diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/operators/TokenOperator.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/operators/TokenOperator.java index b6530c22bfe1..f844060f853d 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/operators/TokenOperator.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/operators/TokenOperator.java @@ -27,20 +27,14 @@ */ package org.hisp.dhis.query.operators; -import static org.hisp.dhis.user.UserSettingKey.DB_LOCALE; -import static org.hisp.dhis.user.UserSettingKey.UI_LOCALE; - -import java.util.Locale; import javax.persistence.criteria.CriteriaBuilder; import javax.persistence.criteria.Predicate; import javax.persistence.criteria.Root; -import org.apache.commons.lang3.ObjectUtils; import org.hibernate.criterion.Criterion; import org.hibernate.criterion.Restrictions; import org.hisp.dhis.hibernate.jsonb.type.JsonbFunctions; import org.hisp.dhis.query.Typed; import org.hisp.dhis.query.planner.QueryPath; -import org.hisp.dhis.user.CurrentUserUtil; /** * @author Henning HÃ¥konsen @@ -76,10 +70,8 @@ public Predicate getPredicate(CriteriaBuilder builder, Root root, QueryPa root.get(queryPath.getPath()), builder.literal(TokenUtils.createRegex(value).toString())), true); - Locale currentLocale = - ObjectUtils.defaultIfNull( - CurrentUserUtil.getUserSetting(DB_LOCALE), CurrentUserUtil.getUserSetting(UI_LOCALE)); - if (currentLocale == null + + if (queryPath.getLocale() == null || !queryPath.getProperty().isTranslatable() || queryPath.getProperty().getTranslationKey() == null) { return defaultSearch; @@ -90,7 +82,7 @@ public Predicate getPredicate(CriteriaBuilder builder, Root root, QueryPa Boolean.class, root.get("translations"), builder.literal("{" + queryPath.getProperty().getTranslationKey() + "}"), - builder.literal(currentLocale.getLanguage()), + builder.literal(queryPath.getLocale().getLanguage()), builder.literal(TokenUtils.createRegex(value).toString())), true); } 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 4d590b379348..6dfbce8fdff6 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 @@ -31,20 +31,27 @@ import java.util.Arrays; import java.util.Iterator; import java.util.List; +import java.util.Locale; import javax.persistence.criteria.Path; import javax.persistence.criteria.Root; import lombok.RequiredArgsConstructor; import org.hisp.dhis.attribute.Attribute; import org.hisp.dhis.common.CodeGenerator; +import org.hisp.dhis.i18n.locale.LocaleManager; import org.hisp.dhis.query.Conjunction; import org.hisp.dhis.query.Criterion; import org.hisp.dhis.query.Disjunction; import org.hisp.dhis.query.Junction; import org.hisp.dhis.query.Query; import org.hisp.dhis.query.Restriction; +import org.hisp.dhis.query.operators.TokenOperator; import org.hisp.dhis.schema.Property; import org.hisp.dhis.schema.Schema; import org.hisp.dhis.schema.SchemaService; +import org.hisp.dhis.setting.SettingKey; +import org.hisp.dhis.setting.SystemSettingManager; +import org.hisp.dhis.user.CurrentUserUtil; +import org.hisp.dhis.user.UserSettingKey; import org.springframework.stereotype.Component; /** @@ -54,6 +61,7 @@ @RequiredArgsConstructor public class DefaultQueryPlanner implements QueryPlanner { private final SchemaService schemaService; + private final SystemSettingManager systemSettingManager; @Override public QueryPlan planQuery(Query query) { @@ -202,6 +210,10 @@ private Query getQuery(Query query, boolean persistedOnly) { Restriction restriction = (Restriction) criterion; restriction.setQueryPath(getQueryPath(query.getSchema(), restriction.getPath())); + if (restriction.getOperator().getClass().isAssignableFrom(TokenOperator.class)) { + setQueryPathLocale(restriction); + } + if (restriction.getQueryPath().isPersisted() && !restriction.getQueryPath().haveAlias() && !Attribute.ObjectType.isValidType(restriction.getQueryPath().getPath())) { @@ -247,6 +259,10 @@ private Junction handleJunction(Query query, Junction queryJunction, boolean per Restriction restriction = (Restriction) criterion; restriction.setQueryPath(getQueryPath(query.getSchema(), restriction.getPath())); + if (restriction.getOperator().getClass().isAssignableFrom(TokenOperator.class)) { + setQueryPathLocale(restriction); + } + if (restriction.getQueryPath().isPersisted() && !restriction.getQueryPath().haveAlias(1) && !Attribute.ObjectType.isValidType(restriction.getQueryPath().getPath())) { @@ -270,4 +286,17 @@ private Junction handleJunction(Query query, Junction queryJunction, boolean per private boolean isFilterByAttributeId(Property curProperty, String propertyName) { return curProperty == null && CodeGenerator.isValidUid(propertyName); } + + private void setQueryPathLocale(Restriction restriction) { + Locale systemLocale = + systemSettingManager.getSystemSetting(SettingKey.DB_LOCALE, LocaleManager.DEFAULT_LOCALE); + Locale currentUserLocale = CurrentUserUtil.getUserSetting(UserSettingKey.DB_LOCALE); + if (currentUserLocale != null && !currentUserLocale.equals(systemLocale)) { + // Use translations jsonb column for querying with the current user locale. + restriction.getQueryPath().setLocale(currentUserLocale); + } else { + // Use default properties for querying. Don't use the translations jsonb column. + restriction.getQueryPath().setLocale(null); + } + } } diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/planner/QueryPath.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/planner/QueryPath.java index 9a6b3f7c272b..9a903c566817 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/planner/QueryPath.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/query/planner/QueryPath.java @@ -30,15 +30,16 @@ import com.google.common.base.Joiner; import com.google.common.base.MoreObjects; import java.util.Arrays; -import lombok.AllArgsConstructor; +import java.util.Locale; import lombok.Getter; +import lombok.RequiredArgsConstructor; import org.hisp.dhis.schema.Property; /** * @author Morten Olav Hansen */ @Getter -@AllArgsConstructor +@RequiredArgsConstructor public class QueryPath { private final Property property; @@ -48,6 +49,12 @@ public class QueryPath { private static final Joiner PATH_JOINER = Joiner.on("."); + /** + * If this locale is not null then the query must use the translations jsonb column instead of + * default properties. + */ + private Locale locale; + public QueryPath(Property property, boolean persisted) { this(property, persisted, new String[0]); } @@ -70,6 +77,14 @@ public boolean haveAlias(int n) { return alias != null && alias.length > n; } + public void setLocale(Locale locale) { + this.locale = locale; + } + + public Locale getLocale() { + return locale; + } + @Override public String toString() { return MoreObjects.toStringHelper(this) diff --git a/dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/query/DefaultQueryServiceTest.java b/dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/query/DefaultQueryServiceTest.java index 422042621a6f..c35b8a212d27 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/query/DefaultQueryServiceTest.java +++ b/dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/query/DefaultQueryServiceTest.java @@ -42,6 +42,7 @@ import org.hisp.dhis.query.planner.QueryPlanner; import org.hisp.dhis.schema.SchemaService; import org.hisp.dhis.schema.descriptors.OrganisationUnitSchemaDescriptor; +import org.hisp.dhis.setting.SystemSettingManager; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -65,9 +66,11 @@ class DefaultQueryServiceTest { @Mock private SchemaService schemaService; + @Mock private SystemSettingManager systemSettingManager; + @BeforeEach public void setUp() { - QueryPlanner queryPlanner = new DefaultQueryPlanner(schemaService); + QueryPlanner queryPlanner = new DefaultQueryPlanner(schemaService, systemSettingManager); subject = new DefaultQueryService( queryParser, queryPlanner, criteriaQueryEngine, inMemoryQueryEngine); diff --git a/dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/query/planner/DefaultQueryPlannerTest.java b/dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/query/planner/DefaultQueryPlannerTest.java index c0268674b7b6..5a289026af85 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/query/planner/DefaultQueryPlannerTest.java +++ b/dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/query/planner/DefaultQueryPlannerTest.java @@ -44,6 +44,7 @@ import org.hisp.dhis.schema.SchemaService; import org.hisp.dhis.schema.descriptors.DataElementSchemaDescriptor; import org.hisp.dhis.schema.descriptors.OrganisationUnitSchemaDescriptor; +import org.hisp.dhis.setting.SystemSettingManager; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -60,9 +61,11 @@ class DefaultQueryPlannerTest { @Mock private SchemaService schemaService; + @Mock private SystemSettingManager systemSettingManager; + @BeforeEach public void setUp() { - this.subject = new DefaultQueryPlanner(schemaService); + this.subject = new DefaultQueryPlanner(schemaService, systemSettingManager); } @Test diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/AbstractDatastoreControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/AbstractDatastoreControllerTest.java index 819b8edf1fc7..c3c5230934d3 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/AbstractDatastoreControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/AbstractDatastoreControllerTest.java @@ -29,23 +29,12 @@ import static org.hisp.dhis.utils.JavaToJson.toJson; import static org.hisp.dhis.web.WebClientUtils.assertStatus; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.List; -import java.util.Locale; import java.util.Map; -import org.hisp.dhis.jsontree.JsonArray; -import org.hisp.dhis.user.User; -import org.hisp.dhis.user.UserSettingKey; -import org.hisp.dhis.user.UserSettingService; import org.hisp.dhis.utils.JavaToJson; import org.hisp.dhis.web.HttpStatus; import org.hisp.dhis.webapi.DhisControllerConvenienceTest; -import org.hisp.dhis.webapi.DhisControllerIntegrationTest; -import org.junit.jupiter.api.Test; -import org.springframework.beans.factory.annotation.Autowired; /** * Base class for testing the {@link DatastoreController} providing helpers to set up entries in the @@ -79,52 +68,4 @@ final void postPet(String key, String name, int age, List eats) { eats == null ? List.of() : eats.stream().map(food -> Map.of("name", food))); postEntry("pets", key, toJson(obj)); } - - static class CrudControllerIntegrationTest extends DhisControllerIntegrationTest { - - @Autowired private UserSettingService userSettingService; - - @Test - void testSearchByToken() { - String id = - assertStatus( - HttpStatus.CREATED, - POST( - "/dataSets/", - "{'name':'My data set', 'shortName': 'MDS', 'periodType':'Monthly'}")); - - PUT( - "/dataSets/" + id + "/translations", - "{'translations': [{'locale':'fr', 'property':'NAME', 'value':'fr dataSet'}]}") - .content(HttpStatus.NO_CONTENT); - - JsonArray translations = - GET("/dataSets/{id}/translations", id).content().getArray("translations"); - assertEquals(1, translations.size()); - - assertTrue( - GET("/dataSets?filter=identifiable:token:fr", id) - .content() - .getArray("dataSets") - .isEmpty()); - User userA = createAndAddUser("userA", null, "ALL"); - userSettingService.saveUserSetting(UserSettingKey.DB_LOCALE, Locale.FRENCH, userA); - injectSecurityContext(userA); - assertTrue( - GET("/dataSets?filter=identifiable:token:bb", id) - .content() - .getArray("dataSets") - .isEmpty()); - assertFalse( - GET("/dataSets?filter=identifiable:token:fr", id) - .content() - .getArray("dataSets") - .isEmpty()); - assertFalse( - GET("/dataSets?filter=identifiable:token:dataSet", id) - .content() - .getArray("dataSets") - .isEmpty()); - } - } } diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CrudControllerIntegrationTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CrudControllerIntegrationTest.java index fe3a8a502385..e546156635b6 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CrudControllerIntegrationTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/CrudControllerIntegrationTest.java @@ -29,14 +29,29 @@ import static org.hisp.dhis.web.WebClientUtils.assertStatus; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import java.util.Locale; import org.hisp.dhis.dataset.DataSet; +import org.hisp.dhis.jsontree.JsonArray; +import org.hisp.dhis.setting.SettingKey; +import org.hisp.dhis.setting.SystemSettingManager; import org.hisp.dhis.user.User; +import org.hisp.dhis.user.UserSettingKey; +import org.hisp.dhis.user.UserSettingService; import org.hisp.dhis.web.HttpStatus; import org.hisp.dhis.webapi.DhisControllerIntegrationTest; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; class CrudControllerIntegrationTest extends DhisControllerIntegrationTest { + + @Autowired private UserSettingService userSettingService; + + @Autowired private SystemSettingManager systemSettingManager; + @Test void testGetNonAccessibleObject() { User admin = getCurrentUser(); @@ -73,4 +88,88 @@ void testGetAccessibleObject() { GET("/dataSets/{id}", id).content(HttpStatus.OK); } + + @Test + @DisplayName("Search by token should use translations column instead of default columns") + void testSearchByToken() { + setUpTranslation(); + User userA = createAndAddUser("userA", null, "ALL"); + userSettingService.saveUserSetting(UserSettingKey.DB_LOCALE, Locale.FRENCH, userA); + injectSecurityContext(userA); + assertTrue( + GET("/dataSets?filter=identifiable:token:bb").content().getArray("dataSets").isEmpty()); + assertFalse( + GET("/dataSets?filter=identifiable:token:fr").content().getArray("dataSets").isEmpty()); + assertFalse( + GET("/dataSets?filter=identifiable:token:dataSet") + .content() + .getArray("dataSets") + .isEmpty()); + } + + @Test + @DisplayName("Search by token should use default properties instead of translations column") + void testSearchTokenDefaultLocale() { + setUpTranslation(); + User userA = createAndAddUser("userA", null, "ALL"); + userSettingService.saveUserSetting(UserSettingKey.DB_LOCALE, Locale.ENGLISH, userA); + injectSecurityContext(userA); + + systemSettingManager.saveSystemSetting(SettingKey.DB_LOCALE, Locale.ENGLISH); + assertTrue( + GET("/dataSets?filter=identifiable:token:bb").content().getArray("dataSets").isEmpty()); + assertTrue( + GET("/dataSets?filter=identifiable:token:fr").content().getArray("dataSets").isEmpty()); + assertTrue( + GET("/dataSets?filter=identifiable:token:dataSet") + .content() + .getArray("dataSets") + .isEmpty()); + + assertFalse( + GET("/dataSets?filter=identifiable:token:my").content().getArray("dataSets").isEmpty()); + } + + @Test + @DisplayName("Search by token should use default properties instead of translations column") + void testSearchTokenWithNullLocale() { + setUpTranslation(); + User userA = createAndAddUser("userA", null, "ALL"); + injectSecurityContext(userA); + + systemSettingManager.saveSystemSetting(SettingKey.DB_LOCALE, Locale.ENGLISH); + assertTrue( + GET("/dataSets?filter=identifiable:token:bb").content().getArray("dataSets").isEmpty()); + assertTrue( + GET("/dataSets?filter=identifiable:token:fr").content().getArray("dataSets").isEmpty()); + assertTrue( + GET("/dataSets?filter=identifiable:token:dataSet") + .content() + .getArray("dataSets") + .isEmpty()); + + assertFalse( + GET("/dataSets?filter=identifiable:token:my").content().getArray("dataSets").isEmpty()); + } + + private void setUpTranslation() { + String id = + assertStatus( + HttpStatus.CREATED, + POST( + "/dataSets/", + "{'name':'My data set', 'shortName': 'MDS', 'periodType':'Monthly'}")); + + PUT( + "/dataSets/" + id + "/translations", + "{'translations': [{'locale':'fr', 'property':'NAME', 'value':'fr dataSet'}]}") + .content(HttpStatus.NO_CONTENT); + + JsonArray translations = + GET("/dataSets/{id}/translations", id).content().getArray("translations"); + assertEquals(1, translations.size()); + + assertTrue( + GET("/dataSets?filter=identifiable:token:fr", id).content().getArray("dataSets").isEmpty()); + } } diff --git a/dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/controller/dataelement/DataElementOperandControllerTest.java b/dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/controller/dataelement/DataElementOperandControllerTest.java index 5ca7c6a1ddef..57dcaba9c997 100644 --- a/dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/controller/dataelement/DataElementOperandControllerTest.java +++ b/dhis-2/dhis-web-api/src/test/java/org/hisp/dhis/webapi/controller/dataelement/DataElementOperandControllerTest.java @@ -72,6 +72,7 @@ import org.hisp.dhis.schema.Schema; import org.hisp.dhis.schema.SchemaService; import org.hisp.dhis.security.acl.AclService; +import org.hisp.dhis.setting.SystemSettingManager; import org.hisp.dhis.user.CurrentUserService; import org.hisp.dhis.user.User; import org.hisp.dhis.webapi.mvc.messageconverter.JsonMessageConverter; @@ -109,6 +110,8 @@ class DataElementOperandControllerTest { @Mock private CategoryService dataElementCategoryService; + @Mock private SystemSettingManager systemSettingManager; + private QueryService queryService; @Mock private CurrentUserService currentUserService; @@ -124,7 +127,7 @@ public void setUp() { QueryService _queryService = new DefaultQueryService( new DefaultJpaQueryParser(schemaService), - new DefaultQueryPlanner(schemaService), + new DefaultQueryPlanner(schemaService, systemSettingManager), mock(JpaCriteriaQueryEngine.class), new InMemoryQueryEngine<>(schemaService, mock(AclService.class), currentUserService)); // Use "spy" on queryService, because we want a partial mock: we only