diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/attribute/Attribute.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/attribute/Attribute.java index 46a71fa0de36..f146a0702d4c 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/attribute/Attribute.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/attribute/Attribute.java @@ -173,10 +173,6 @@ public static ObjectType valueOf(Class type) { return stream(values()).filter(t -> t.type == type).findFirst().orElse(null); } - public static boolean isValidType(String type) { - return stream(values()).anyMatch(t -> t.getPropertyName().equals(type)); - } - public String getPropertyName() { return CaseFormat.UPPER_UNDERSCORE.converterTo(CaseFormat.LOWER_CAMEL).convert(name()) + "Attribute"; diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/JpaCriteriaQueryEngine.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/JpaCriteriaQueryEngine.java index deef7b14f00f..54929699adc7 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/JpaCriteriaQueryEngine.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/JpaCriteriaQueryEngine.java @@ -244,14 +244,13 @@ private InternalHibernateGenericStore getStore(Class Predicate buildPredicates(CriteriaBuilder builder, Root root, Query query) { Predicate junction = builder.conjunction(); - query.getAliases().forEach(alias -> root.join(alias).alias(alias)); if (!query.getCriterions().isEmpty()) { junction = getJpaJunction(builder, query.getRootJunctionType()); for (org.hisp.dhis.query.Criterion criterion : query.getCriterions()) { addPredicate(builder, root, junction, criterion); } } - + query.getAliases().forEach(alias -> root.get(alias).alias(alias)); return junction; } diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/operators/EqualOperator.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/operators/EqualOperator.java index fa86b25a0b73..3e664cdfbd36 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/operators/EqualOperator.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/operators/EqualOperator.java @@ -30,7 +30,6 @@ import java.util.Collection; import java.util.Date; import javax.persistence.criteria.CriteriaBuilder; -import javax.persistence.criteria.Join; import javax.persistence.criteria.Predicate; import javax.persistence.criteria.Root; import org.hibernate.criterion.Criterion; @@ -86,13 +85,6 @@ public Predicate getPredicate(CriteriaBuilder builder, Root root, QueryPa return builder.equal(builder.size(root.get(queryPath.getPath())), value); } - if (queryPath.haveAlias()) { - for (Join join : root.getJoins()) { - if (join.getAlias().equals(queryPath.getAlias()[0])) { - return builder.equal(join.get(queryPath.getProperty().getFieldName()), args.get(0)); - } - } - } return builder.equal(root.get(queryPath.getPath()), args.get(0)); } diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/operators/InOperator.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/operators/InOperator.java index 33ebef282f58..79b98fd9ed88 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/operators/InOperator.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/operators/InOperator.java @@ -30,7 +30,6 @@ import java.util.Collection; import java.util.Date; import javax.persistence.criteria.CriteriaBuilder; -import javax.persistence.criteria.Join; import javax.persistence.criteria.Predicate; import javax.persistence.criteria.Root; import org.hibernate.criterion.Criterion; @@ -80,13 +79,6 @@ public Predicate getPredicate(CriteriaBuilder builder, Root root, QueryPa getCollectionArgs().get(0))); } - if (queryPath.haveAlias()) { - for (Join join : root.getJoins()) { - if (join.getAlias().equals(queryPath.getAlias()[0])) { - return join.get(queryPath.getProperty().getFieldName()).in(getCollectionArgs().get(0)); - } - } - } return root.get(queryPath.getPath()).in(getCollectionArgs().get(0)); } diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/operators/NotEqualOperator.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/operators/NotEqualOperator.java index 81f33c265805..6c755276d5da 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/operators/NotEqualOperator.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/operators/NotEqualOperator.java @@ -28,7 +28,6 @@ package org.hisp.dhis.query.operators; import javax.persistence.criteria.CriteriaBuilder; -import javax.persistence.criteria.Join; import javax.persistence.criteria.Predicate; import javax.persistence.criteria.Root; import org.hibernate.criterion.Criterion; @@ -65,13 +64,6 @@ public Predicate getPredicate(CriteriaBuilder builder, Root root, QueryPa return builder.notEqual(builder.size(root.get(queryPath.getPath())), value); } - if (queryPath.haveAlias()) { - for (Join join : root.getJoins()) { - if (join.getAlias().equals(queryPath.getAlias()[0])) { - return builder.equal(join.get(queryPath.getProperty().getFieldName()), args.get(0)); - } - } - } return builder.notEqual(root.get(queryPath.getPath()), args.get(0)); } diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/planner/DefaultQueryPlanner.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/planner/DefaultQueryPlanner.java index e2a6716665b8..1ee49bf5964c 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/planner/DefaultQueryPlanner.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/planner/DefaultQueryPlanner.java @@ -34,7 +34,6 @@ import javax.persistence.criteria.Path; import javax.persistence.criteria.Root; import lombok.AllArgsConstructor; -import org.hisp.dhis.attribute.Attribute; import org.hisp.dhis.common.CodeGenerator; import org.hisp.dhis.query.Conjunction; import org.hisp.dhis.query.Criterion; @@ -181,13 +180,33 @@ public Path getQueryPath(Root root, Schema schema, String path) { */ private Query getQuery(Query query, boolean persistedOnly) { Query pQuery = Query.from(query.getSchema(), query.getRootJunctionType()); - Iterator criterions = query.getCriterions().iterator(); + Iterator iterator = query.getCriterions().iterator(); - while (criterions.hasNext()) { - Criterion criterion = criterions.next(); - if (addJunction(criterion, pQuery, persistedOnly) - || addRestriction(query, criterion, pQuery)) { - criterions.remove(); + while (iterator.hasNext()) { + Criterion criterion = iterator.next(); + + if (criterion instanceof Junction) { + Junction junction = handleJunction(pQuery, (Junction) criterion, persistedOnly); + + if (!junction.getCriterions().isEmpty()) { + pQuery.getAliases().addAll(junction.getAliases()); + pQuery.add(junction); + } + + if (((Junction) criterion).getCriterions().isEmpty()) { + iterator.remove(); + } + } else if (criterion instanceof Restriction) { + Restriction restriction = (Restriction) criterion; + restriction.setQueryPath(getQueryPath(query.getSchema(), restriction.getPath())); + + if (restriction.getQueryPath().isPersisted() && !restriction.getQueryPath().haveAlias()) { + pQuery + .getAliases() + .addAll(Arrays.asList(((Restriction) criterion).getQueryPath().getAlias())); + pQuery.getCriterions().add(criterion); + iterator.remove(); + } } } @@ -226,12 +245,10 @@ private Junction handleJunction(Query query, Junction queryJunction, boolean per Restriction restriction = (Restriction) criterion; restriction.setQueryPath(getQueryPath(query.getSchema(), restriction.getPath())); - if (restriction.getQueryPath().isPersisted() && !isAttributeFilter(query, restriction)) { - if (restriction.getQueryPath().haveAlias()) { - criteriaJunction - .getAliases() - .addAll(Arrays.asList(restriction.getQueryPath().getAlias())); - } + if (restriction.getQueryPath().isPersisted() && !restriction.getQueryPath().haveAlias(1)) { + criteriaJunction + .getAliases() + .addAll(Arrays.asList(((Restriction) criterion).getQueryPath().getAlias())); criteriaJunction.getCriterions().add(criterion); iterator.remove(); } else if (persistedOnly) { @@ -249,58 +266,4 @@ private Junction handleJunction(Query query, Junction queryJunction, boolean per private boolean isFilterByAttributeId(Property curProperty, String propertyName) { return curProperty == null && CodeGenerator.isValidUid(propertyName); } - - /** - * Handle attribute filter such as /attributes?fields=id,name&filter=userAttribute:eq:true - * - * @return true if attribute filter - */ - private boolean isAttributeFilter(Query query, Restriction restriction) { - return query.getSchema().getKlass().isAssignableFrom(Attribute.class) - && Attribute.ObjectType.isValidType(restriction.getQueryPath().getPath()); - } - - /** - * Add given criterion which is an instance of {@link Junction} to given {@link Query} pQuery. If - * successfully added, return TRUE indicating that the given {@link Criterion} criterion should be - * removed. - */ - private boolean addJunction(Criterion criterion, Query pQuery, boolean persistedOnly) { - if (!(criterion instanceof Junction)) { - return false; - } - boolean shouldRemove = false; - Junction junction = handleJunction(pQuery, (Junction) criterion, persistedOnly); - if (!junction.getCriterions().isEmpty()) { - pQuery.getAliases().addAll(junction.getAliases()); - pQuery.add(junction); - } - if (((Junction) criterion).getCriterions().isEmpty()) { - shouldRemove = true; - } - return shouldRemove; - } - - /** - * Add given criterion which is an instance of {@link Restriction} to given {@link Query} pQuery. - * If successfully added, return TRUE indicating that the given {@link Criterion} criterion should - * be removed. - */ - private boolean addRestriction(Query query, Criterion criterion, Query pQuery) { - if (!(criterion instanceof Restriction)) { - return false; - } - boolean shouldRemove = false; - Restriction restriction = (Restriction) criterion; - restriction.setQueryPath(getQueryPath(query.getSchema(), restriction.getPath())); - - if (restriction.getQueryPath().isPersisted() && !isAttributeFilter(query, restriction)) { - pQuery.getCriterions().add(criterion); - shouldRemove = true; - if (restriction.getQueryPath().haveAlias()) { - pQuery.getAliases().addAll(Arrays.asList(restriction.getQueryPath().getAlias())); - } - } - return shouldRemove; - } } diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/query/QueryServiceTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/query/QueryServiceTest.java index 5ea781db6666..24aff96ce11e 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/query/QueryServiceTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/query/QueryServiceTest.java @@ -39,9 +39,6 @@ import java.util.Date; import java.util.List; import java.util.stream.Collectors; -import org.hibernate.Session; -import org.hibernate.SessionFactory; -import org.hibernate.stat.Statistics; import org.hisp.dhis.common.IdentifiableObject; import org.hisp.dhis.common.IdentifiableObjectManager; import org.hisp.dhis.common.ValueType; @@ -69,8 +66,6 @@ class QueryServiceTest extends SingleSetupIntegrationTestBase { @Autowired private IdentifiableObjectManager identifiableObjectManager; - @Autowired private SessionFactory sessionFactory; - @BeforeEach void createDataElements() { DataElement dataElementA = createDataElement('A'); @@ -550,22 +545,8 @@ void testCriteriaAndRootJunctionDEG() { query.add(Restrictions.eq("dataElements.id", "deabcdefghD")); query.add(Restrictions.eq("dataElements.id", "deabcdefghE")); query.add(Restrictions.eq("dataElements.id", "deabcdefghF")); - Session session = sessionFactory.getCurrentSession(); - Statistics statistics = session.getSessionFactory().getStatistics(); - statistics.setStatisticsEnabled(true); - List objects = queryService.query(query); - String[] queries = statistics.getQueries(); - boolean findQuery = false; - for (String q : queries) { - if (q.equals( - "select generatedAlias0 from DataElementGroup as generatedAlias0 inner join generatedAlias0.members as members where ( members.uid=:param0 ) and ( members.uid=:param1 ) and ( members.uid=:param2 ) and ( members.uid=:param3 ) and ( members.uid=:param4 ) and ( members.uid=:param5 )")) { - findQuery = true; - break; - } - } - assertTrue(findQuery); + List objects = queryService.query(query); assertTrue(objects.isEmpty()); - statistics.setStatisticsEnabled(false); } @Test diff --git a/dhis-2/dhis-test-web-api/pom.xml b/dhis-2/dhis-test-web-api/pom.xml index 67cfd90365a3..43ed1e7632d2 100644 --- a/dhis-2/dhis-test-web-api/pom.xml +++ b/dhis-2/dhis-test-web-api/pom.xml @@ -251,11 +251,6 @@ spring-tx test - - org.hibernate - hibernate-core - test - com.google.code.gson gson diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/OptionControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/OptionControllerTest.java index 242d7097ea14..ff53ed49daa2 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/OptionControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/OptionControllerTest.java @@ -27,23 +27,14 @@ */ package org.hisp.dhis.webapi.controller; -import static org.hisp.dhis.web.WebClientUtils.assertStatus; import static org.junit.jupiter.api.Assertions.assertEquals; -import org.hibernate.Session; -import org.hibernate.SessionFactory; -import org.hibernate.stat.Statistics; import org.hisp.dhis.jsontree.JsonResponse; import org.hisp.dhis.web.HttpStatus; import org.hisp.dhis.webapi.DhisControllerConvenienceTest; -import org.hisp.dhis.webapi.json.domain.JsonOptionSet; import org.junit.jupiter.api.Test; -import org.springframework.beans.factory.annotation.Autowired; class OptionControllerTest extends DhisControllerConvenienceTest { - - @Autowired private SessionFactory sessionFactory; - @Test void testUpdateOptionWithSortOrderGap() { // Create OptionSet with two Options @@ -78,48 +69,4 @@ void testUpdateOptionWithSortOrderGap() { assertEquals("Uh4HvjK6zg3", response.getString("options[1].id").string()); assertEquals(2, response.getNumber("options[1].sortOrder").intValue()); } - - @Test - void testQueryOptionsByOptionSetIds() { - Session session = sessionFactory.getCurrentSession(); - String id = - assertStatus( - HttpStatus.CREATED, - POST( - "/optionSets/", - "{'name': 'test', 'version': 2, 'valueType': 'TEXT', 'description':'desc' }")); - assertStatus( - HttpStatus.CREATED, - POST( - "/options/", - "{'optionSet': { 'id':'" - + id - + "'}, 'id':'Uh4HvjK6zg3', 'code': 'A', 'name': 'Anna', 'description': 'this-is-a'}")); - assertStatus( - HttpStatus.CREATED, - POST( - "/options/", - "{'optionSet': { 'id':'" - + id - + "'},'id':'BQMei56UBl6','code': 'B', 'name': 'Betta', 'description': 'this-is-b'}")); - Statistics statistics = session.getSessionFactory().getStatistics(); - statistics.setStatisticsEnabled(true); - assertEquals(0, statistics.getQueryExecutionCount()); - JsonOptionSet set = - GET(String.format("/options?filter=optionSet.id:in:[%s,%s]", id, "TESTUIDA")) - .content() - .as(JsonOptionSet.class); - assertEquals(2, set.getOptions().size()); - - // Verify that there is no n+1 queries to Option table - int count = 0; - for (String q : statistics.getQueries()) { - if (q.contains("from Option")) { - count++; - } - } - assertEquals(2, count); - - statistics.setStatisticsEnabled(false); - } }