From d5ed0b8baa3ca64ded819a1f5a923639785884ad Mon Sep 17 00:00:00 2001 From: Jan Bernitt Date: Mon, 14 Oct 2024 15:13:41 +0200 Subject: [PATCH 1/7] feat: settings closed set with validation [DHIS2-18217] --- .../org/hisp/dhis/setting/LazySettings.java | 33 ++++++- .../java/org/hisp/dhis/setting/Settings.java | 2 + .../org/hisp/dhis/setting/SystemSettings.java | 15 ++++ .../dhis/setting/SystemSettingsService.java | 9 +- .../hisp/dhis/translation/Translatable.java | 2 +- .../table/DefaultAnalyticsTableGenerator.java | 18 +--- .../dxf2/metadata/jobs/MetadataSyncJob.java | 9 +- .../setting/DefaultSystemSettingsService.java | 31 ++++++- ...faultSystemSettingsTranslationService.java | 2 + .../hisp/dhis/setting/SystemSettingsTest.java | 86 +++++++++++++++++++ .../MetadataSystemSettingServiceTest.java | 2 +- .../hisp/dhis/dxf2/sync/SyncUtilsTest.java | 2 +- .../setting/SystemSettingsServiceTest.java | 54 ++++++------ .../controller/SystemSettingsController.java | 71 +++++++++++---- 14 files changed, 263 insertions(+), 73 deletions(-) diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/setting/LazySettings.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/setting/LazySettings.java index 8fcfce920a3e..ee9e88bc5b1e 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/setting/LazySettings.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/setting/LazySettings.java @@ -65,6 +65,7 @@ import org.hisp.dhis.jsontree.JsonMap; import org.hisp.dhis.jsontree.JsonMixed; import org.hisp.dhis.jsontree.JsonValue; +import org.hisp.dhis.translation.Translatable; /** * {@link SystemSettings} or {@link UserSettings} represented by a set of keys and their values. @@ -86,6 +87,8 @@ final class LazySettings implements SystemSettings, UserSettings { private static final Set CONFIDENTIAL_KEYS = new HashSet<>(); + private static final Set TRANSLATABLE_KEYS = new HashSet<>(); + private static final Map DEFAULTS_SYSTEM_SETTINGS = extractDefaults(SystemSettings.class); private static final Map DEFAULTS_USER_SETTINGS = @@ -106,6 +109,10 @@ static boolean isConfidential(@Nonnull String key) { return CONFIDENTIAL_KEYS.contains(key); } + static boolean isTranslatable(@Nonnull String key) { + return TRANSLATABLE_KEYS.contains(key); + } + @Nonnull static LazySettings of(Class type, @Nonnull Map settings) { return of(type, settings, (k, v) -> v); @@ -243,12 +250,15 @@ public JsonMap toJson(boolean includeConfidential, @Nonnull Set) + return Enum.valueOf(((Enum) defaultValue).getDeclaringClass(), value) != null; + return true; + } catch (Exception ex) { + return false; + } + } + private int indexOf(String key) { if (keys.length == 0) return -1; int i = Arrays.binarySearch(keys, key); @@ -334,6 +363,8 @@ private static Map extractDefaults(ClassThis method is for internal use only. Therefore, if the key does not exist no exception is + * thrown but an error is logged. + * * @param key of the setting to insert or update * @param value of the setting, null or empty to delete */ @@ -57,8 +62,10 @@ public interface SystemSettingsService extends SystemSettingsProvider { * Saves the given system setting key and value. * * @param settings the new values, null or empty values delete the setting + * @throws NotFoundException if any of the provided keys is not a key contained in {@link + * SystemSettings#keysWithDefaults()} */ - void putAll(@Nonnull Map settings); + void putAll(@Nonnull Map settings) throws NotFoundException, BadRequestException; /** * Deletes the system setting with the given name. diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/translation/Translatable.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/translation/Translatable.java index b9df9468a313..8f16722bc5b7 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/translation/Translatable.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/translation/Translatable.java @@ -44,7 +44,7 @@ public @interface Translatable { /** Property name for enabling translation */ @Nonnull - String propertyName(); + String propertyName() default ""; /** * Translation key for storing translation in json format. If not defined then property name is diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/DefaultAnalyticsTableGenerator.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/DefaultAnalyticsTableGenerator.java index bea7af23b130..28ce71a55689 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/DefaultAnalyticsTableGenerator.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/DefaultAnalyticsTableGenerator.java @@ -27,7 +27,6 @@ */ package org.hisp.dhis.analytics.table; -import static java.util.Map.entry; import static org.hisp.dhis.common.collection.CollectionUtils.emptyIfNull; import static org.hisp.dhis.scheduling.JobProgress.FailurePolicy.SKIP_STAGE; import static org.hisp.dhis.util.DateUtils.toLongDate; @@ -48,7 +47,6 @@ import org.hisp.dhis.analytics.cache.OutliersCache; import org.hisp.dhis.resourcetable.ResourceTableService; import org.hisp.dhis.scheduling.JobProgress; -import org.hisp.dhis.setting.Settings; import org.hisp.dhis.setting.SystemSettingsService; import org.hisp.dhis.system.util.Clock; import org.springframework.stereotype.Service; @@ -118,19 +116,11 @@ public void generateAnalyticsTables(AnalyticsTableUpdateParams params0, JobProgr private void updateLastSuccessfulSystemSettings(AnalyticsTableUpdateParams params, Clock clock) { if (params.isLatestUpdate()) { - settingsService.putAll( - Map.ofEntries( - entry( - "keyLastSuccessfulLatestAnalyticsPartitionUpdate", - Settings.valueOf(params.getStartTime())), - entry("keyLastSuccessfulLatestAnalyticsPartitionRuntime", clock.time()))); + settingsService.put("keyLastSuccessfulLatestAnalyticsPartitionUpdate", params.getStartTime()); + settingsService.put("keyLastSuccessfulLatestAnalyticsPartitionRuntime", clock.time()); } else { - settingsService.putAll( - Map.ofEntries( - entry( - "keyLastSuccessfulAnalyticsTablesUpdate", - Settings.valueOf(params.getStartTime())), - entry("keyLastSuccessfulAnalyticsTablesRuntime", clock.time()))); + settingsService.put("keyLastSuccessfulAnalyticsTablesUpdate", params.getStartTime()); + settingsService.put("keyLastSuccessfulAnalyticsTablesRuntime", clock.time()); } } diff --git a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/jobs/MetadataSyncJob.java b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/jobs/MetadataSyncJob.java index 0cd5ba4b8bbf..c92d885aa35b 100644 --- a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/jobs/MetadataSyncJob.java +++ b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/jobs/MetadataSyncJob.java @@ -28,11 +28,9 @@ package org.hisp.dhis.dxf2.metadata.jobs; import static java.lang.String.format; -import static java.util.Map.entry; import java.util.Date; import java.util.List; -import java.util.Map; import java.util.Set; import lombok.AllArgsConstructor; import lombok.extern.slf4j.Slf4j; @@ -50,7 +48,6 @@ import org.hisp.dhis.scheduling.JobProgress; import org.hisp.dhis.scheduling.JobType; import org.hisp.dhis.scheduling.parameters.MetadataSyncJobParameters; -import org.hisp.dhis.setting.Settings; import org.hisp.dhis.setting.SystemSettingsService; import org.springframework.retry.support.RetryTemplate; import org.springframework.stereotype.Component; @@ -204,10 +201,8 @@ private void updateMetadataVersionFailureDetails(MetadataRetryContext retryConte if (version != null) { MetadataVersion metadataVersion = (MetadataVersion) version; - settingsService.putAll( - Map.ofEntries( - entry("keyMetadataFailedVersion", metadataVersion.getName()), - entry("keyMetadataLastFailedTime", Settings.valueOf(new Date())))); + settingsService.put("keyMetadataFailedVersion", metadataVersion.getName()); + settingsService.put("keyMetadataLastFailedTime", new Date()); } } diff --git a/dhis-2/dhis-services/dhis-service-setting/src/main/java/org/hisp/dhis/setting/DefaultSystemSettingsService.java b/dhis-2/dhis-services/dhis-service-setting/src/main/java/org/hisp/dhis/setting/DefaultSystemSettingsService.java index b89f8a51343d..7e581b16f6d3 100644 --- a/dhis-2/dhis-services/dhis-service-setting/src/main/java/org/hisp/dhis/setting/DefaultSystemSettingsService.java +++ b/dhis-2/dhis-services/dhis-service-setting/src/main/java/org/hisp/dhis/setting/DefaultSystemSettingsService.java @@ -31,6 +31,7 @@ import java.io.Serializable; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; import javax.annotation.CheckForNull; @@ -39,6 +40,8 @@ import lombok.extern.slf4j.Slf4j; import org.hisp.dhis.common.IndirectTransactional; import org.hisp.dhis.common.NonTransactional; +import org.hisp.dhis.feedback.BadRequestException; +import org.hisp.dhis.feedback.NotFoundException; import org.jasypt.encryption.pbe.PBEStringEncryptor; import org.jasypt.exceptions.EncryptionOperationNotPossibleException; import org.springframework.beans.factory.annotation.Qualifier; @@ -107,13 +110,22 @@ private SystemSettings getAllSettings() { @Override @Transactional public void put(@Nonnull String key, @CheckForNull Serializable value) { - putAll(Map.of(key, Settings.valueOf(value))); + try { + putAll(Map.of(key, Settings.valueOf(value))); + } catch (NotFoundException | BadRequestException ex) { + // Note: This is a compromise as otherwise the exception would propagate + // to lots of places that have not yet been adjusted to using the feedback exceptions + log.error("Unable to put setting", ex); + } } @Override @Transactional - public void putAll(@Nonnull Map settings) { + public void putAll(@Nonnull Map settings) + throws NotFoundException, BadRequestException { if (settings.isEmpty()) return; + validateAll(settings); + Set deletes = new HashSet<>(); for (Map.Entry e : settings.entrySet()) { String key = e.getKey(); @@ -129,6 +141,21 @@ public void putAll(@Nonnull Map settings) { allSettings = null; // invalidate } + private void validateAll(@Nonnull Map settings) + throws NotFoundException, BadRequestException { + Set allowed = SystemSettings.keysWithDefaults(); + List illegal = + settings.keySet().stream().filter(key -> !allowed.contains(key)).toList(); + if (!illegal.isEmpty()) + throw new NotFoundException("Setting does not exist: " + String.join(",", illegal)); + SystemSettings empty = SystemSettings.of(Map.of()); + for (Map.Entry e : settings.entrySet()) { + if (!empty.isValid(e.getKey(), e.getValue())) + throw new BadRequestException( + "Setting %s cannot have value %s".formatted(e.getKey(), e.getValue())); + } + } + @Override @Transactional public void deleteAll(@Nonnull Set keys) { diff --git a/dhis-2/dhis-services/dhis-service-setting/src/main/java/org/hisp/dhis/setting/DefaultSystemSettingsTranslationService.java b/dhis-2/dhis-services/dhis-service-setting/src/main/java/org/hisp/dhis/setting/DefaultSystemSettingsTranslationService.java index 5fcdba589432..b63ed0363bb7 100644 --- a/dhis-2/dhis-services/dhis-service-setting/src/main/java/org/hisp/dhis/setting/DefaultSystemSettingsTranslationService.java +++ b/dhis-2/dhis-services/dhis-service-setting/src/main/java/org/hisp/dhis/setting/DefaultSystemSettingsTranslationService.java @@ -77,6 +77,8 @@ private void init() { public void saveSystemSettingTranslation( @Nonnull String key, @Nonnull String locale, String translation) throws ForbiddenException, BadRequestException { + if (!SystemSettings.isTranslatable(key)) + throw new BadRequestException("Not translatable: " + key); DatastoreEntry e = datastore.getEntry(NS, key); if (translation == null || translation.isEmpty()) { if (e == null) return; diff --git a/dhis-2/dhis-services/dhis-service-setting/src/test/java/org/hisp/dhis/setting/SystemSettingsTest.java b/dhis-2/dhis-services/dhis-service-setting/src/test/java/org/hisp/dhis/setting/SystemSettingsTest.java index 5ba2d9ea19a7..7da0ae9681bf 100644 --- a/dhis-2/dhis-services/dhis-service-setting/src/test/java/org/hisp/dhis/setting/SystemSettingsTest.java +++ b/dhis-2/dhis-services/dhis-service-setting/src/test/java/org/hisp/dhis/setting/SystemSettingsTest.java @@ -31,8 +31,10 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import java.time.LocalDateTime; import java.util.Date; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Set; import org.hisp.dhis.analytics.AnalyticsCacheTtlMode; @@ -55,6 +57,15 @@ class SystemSettingsTest { private static final List CONFIDENTIAL_KEYS = List.of("keyEmailPassword", "keyRemoteInstancePassword", "recaptchaSite", "recaptchaSecret"); + private static final List TRANSLATABLE_KEYS = + List.of( + "applicationTitle", + "keyApplicationIntro", + "keyApplicationNotification", + "keyApplicationFooter", + "keyApplicationRightFooter", + "loginPopup"); + @Test void testIsConfidential() { CONFIDENTIAL_KEYS.forEach( @@ -65,6 +76,16 @@ void testIsConfidential() { assertFalse(SystemSettings.isConfidential("keyEmailHostName")); } + @Test + void testIsTranslatable() { + TRANSLATABLE_KEYS.forEach( + key -> + assertTrue( + SystemSettings.isTranslatable(key), "%s should be translatable".formatted(key))); + + assertFalse(SystemSettings.isTranslatable("recaptchaSite")); + } + @Test void testKeysWithDefaults() { Set keys = SystemSettings.keysWithDefaults(); @@ -128,4 +149,69 @@ void testAsDate() { Map.of("keyLastSuccessfulResourceTablesUpdate", Settings.valueOf(new Date(123456L)))); assertEquals(new Date(123456L), settings.getLastSuccessfulResourceTablesUpdate()); } + + @Test + void testAsLocale() { + assertEquals( + Locale.forLanguageTag("fr"), SystemSettings.of(Map.of("keyUiLocale", "fr")).getUiLocale()); + } + + @Test + void testIsValid_Boolean() { + SystemSettings settings = SystemSettings.of(Map.of()); + assertTrue(settings.isValid("keyEmailTls", "true")); + assertTrue(settings.isValid("keyEmailTls", "false")); + assertFalse(settings.isValid("keyEmailTls", "hello")); + assertFalse(settings.isValid("keyEmailTls", "1")); + } + + @Test + void testIsValid_Int() { + SystemSettings settings = SystemSettings.of(Map.of()); + assertTrue(settings.isValid("keyEmailPort", "1")); + assertTrue(settings.isValid("keyEmailPort", "42")); + assertTrue(settings.isValid("keyEmailPort", "-567")); + assertFalse(settings.isValid("keyEmailPort", "hello")); + assertFalse(settings.isValid("keyEmailPort", "true")); + } + + @Test + void testIsValid_Double() { + SystemSettings settings = SystemSettings.of(Map.of()); + assertTrue(settings.isValid("factorDeviation", "1")); + assertTrue(settings.isValid("factorDeviation", "42.5")); + assertTrue(settings.isValid("factorDeviation", "-0.567")); + assertFalse(settings.isValid("factorDeviation", "hello")); + assertFalse(settings.isValid("factorDeviation", "true")); + } + + @Test + void testIsValid_Locale() { + SystemSettings settings = SystemSettings.of(Map.of()); + assertTrue(settings.isValid("keyDbLocale", "fr")); + assertTrue(settings.isValid("keyDbLocale", "de_DE")); + assertFalse(settings.isValid("keyDbLocale", "hello")); + assertFalse(settings.isValid("keyDbLocale", "true")); + assertFalse(settings.isValid("keyDbLocale", "42")); + } + + @Test + void testIsValid_Enum() { + SystemSettings settings = SystemSettings.of(Map.of()); + assertTrue(settings.isValid("keyCacheStrategy", "NO_CACHE")); + assertTrue(settings.isValid("keyCacheStrategy", "CACHE_1_HOUR")); + assertFalse(settings.isValid("keyCacheStrategy", "hello")); + assertFalse(settings.isValid("keyCacheStrategy", "true")); + assertFalse(settings.isValid("keyCacheStrategy", "42")); + } + + @Test + void testIsValid_Date() { + SystemSettings settings = SystemSettings.of(Map.of()); + assertTrue(settings.isValid("keyLastMonitoringRun", "42789654")); + String date = LocalDateTime.of(2020, 12, 12, 0, 0).toString(); + assertTrue(settings.isValid("keyLastMonitoringRun", date)); + assertFalse(settings.isValid("keyLastMonitoringRun", "hello")); + assertFalse(settings.isValid("keyLastMonitoringRun", "true")); + } } diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/dxf2/metadata/sync/MetadataSystemSettingServiceTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/dxf2/metadata/sync/MetadataSystemSettingServiceTest.java index 4a43bf2effcf..e20e00d4a3d6 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/dxf2/metadata/sync/MetadataSystemSettingServiceTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/dxf2/metadata/sync/MetadataSystemSettingServiceTest.java @@ -55,7 +55,7 @@ class MetadataSystemSettingServiceTest extends PostgresIntegrationTestBase { @Autowired DefaultMetadataSystemSettingService metadataSystemSettingService; @BeforeEach - public void setup() { + public void setup() throws Exception { settingsService.putAll( Map.ofEntries( entry("keyRemoteInstanceUrl", "http://localhost:9080"), diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/dxf2/sync/SyncUtilsTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/dxf2/sync/SyncUtilsTest.java index 05615156de3c..648273614ec9 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/dxf2/sync/SyncUtilsTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/dxf2/sync/SyncUtilsTest.java @@ -56,7 +56,7 @@ class SyncUtilsTest extends PostgresIntegrationTestBase { @Autowired SystemSettingsService settingsService; @Test - void getRemoteInstanceTest() { + void getRemoteInstanceTest() throws Exception { settingsService.putAll( Map.ofEntries( entry("keyRemoteInstanceUsername", USERNAME), diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/setting/SystemSettingsServiceTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/setting/SystemSettingsServiceTest.java index 33b9cc2cf12a..04821e4697ed 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/setting/SystemSettingsServiceTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/setting/SystemSettingsServiceTest.java @@ -27,8 +27,6 @@ */ package org.hisp.dhis.setting; -import static java.util.stream.Collectors.toMap; -import static java.util.stream.Collectors.toSet; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -36,8 +34,6 @@ import java.util.Locale; import java.util.Map; import java.util.Set; -import java.util.stream.IntStream; -import java.util.stream.Stream; import org.hisp.dhis.test.integration.PostgresIntegrationTestBase; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -63,63 +59,67 @@ void setUp() { } @Test - void testSaveSystemSettings() { - settingsService.putAll(Map.of("k0", "v0", "k1", "v1")); + void testSaveSystemSettings() throws Exception { + settingsService.putAll(Map.of("keyUiLocale", "de", "keyDbLocale", "fr")); SystemSettings settings = settingsService.getCurrentSettings(); - assertEquals(Set.of("k0", "k1"), settings.keys()); - assertEquals(Map.of("k0", "v0", "k1", "v1"), settings.toMap()); + assertEquals(Set.of("keyUiLocale", "keyDbLocale"), settings.keys()); + assertEquals(Map.of("keyUiLocale", "de", "keyDbLocale", "fr"), settings.toMap()); } @Test void testSaveSystemSetting_Date() { - settingsService.put("keyDate", new Date(123456L)); + settingsService.put("lastSuccessfulDataStatistics", new Date(123456L)); SystemSettings settings = settingsService.getCurrentSettings(); - assertEquals(new Date(123456L), settings.asDate("keyDate", new Date(0L))); + assertEquals(new Date(123456L), settings.asDate("lastSuccessfulDataStatistics", new Date(0L))); } @Test void testSaveSystemSetting_Boolean() { - settingsService.put("keyBoolean", true); + settingsService.put("keyEmailTls", true); SystemSettings settings = settingsService.getCurrentSettings(); - assertTrue(settings.asBoolean("keyBoolean", false)); + assertTrue(settings.asBoolean("keyEmailTls", false)); } @Test void testSaveSystemSetting_Int() { - settingsService.put("keyInt", 42); + settingsService.put("maxPasswordLength", 42); SystemSettings settings = settingsService.getCurrentSettings(); - assertEquals(42, settings.asInt("keyInt", -1)); + assertEquals(42, settings.asInt("maxPasswordLength", -1)); } @Test void testSaveSystemSetting_Locale() { - settingsService.put("keyLocale", Locale.forLanguageTag("en-GB")); + settingsService.put("keyUiLocale", Locale.forLanguageTag("en-GB")); SystemSettings settings = settingsService.getCurrentSettings(); - assertEquals(Locale.forLanguageTag("en-GB"), settings.asLocale("keyLocale", Locale.FRENCH)); + assertEquals(Locale.forLanguageTag("en-GB"), settings.asLocale("keyUiLocale", Locale.FRENCH)); } @Test - void testDeleteSystemSettings() { + void testSaveSystemSetting_Double() { + settingsService.put("factorDeviation", 42.5d); + + SystemSettings settings = settingsService.getCurrentSettings(); + assertEquals(42.5d, settings.asDouble("factorDeviation", -1)); + } + + @Test + void testDeleteSystemSettings() throws Exception { Map allSettings = - IntStream.range(0, 20) - .mapToObj(String::valueOf) - .collect(toMap(i -> "key" + i, i -> "value" + i)); + Map.ofEntries( + Map.entry("keyCustomJs", "JS"), + Map.entry("keyCustomCss", "CSS"), + Map.entry("keyStyle", "style")); settingsService.putAll(allSettings); - Set deletedKeys = - Stream.of(1, 4, 5, 7, 9, 12, 16, 32, 64).map(i -> "key" + i).collect(toSet()); + Set deletedKeys = Set.of("keyCustomJs", "keyStyle"); settingsService.deleteAll(deletedKeys); SystemSettings settings = settingsService.getCurrentSettings(); - Set expectedKeys = - Stream.of(0, 2, 3, 6, 8, 10, 11, 13, 14, 15, 17, 18, 19) - .map(i -> "key" + i) - .collect(toSet()); - assertEquals(expectedKeys, settings.keys()); + assertEquals(Set.of("keyCustomCss"), settings.keys()); } } diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/SystemSettingsController.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/SystemSettingsController.java index aa28f05ca4a4..4a9cd3fabcf2 100644 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/SystemSettingsController.java +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/SystemSettingsController.java @@ -39,6 +39,7 @@ import java.util.Map; import java.util.Optional; import java.util.Set; +import javax.annotation.Nonnull; import lombok.AllArgsConstructor; import org.apache.commons.lang3.StringUtils; import org.hisp.dhis.common.DhisApiVersion; @@ -47,6 +48,8 @@ import org.hisp.dhis.feedback.BadRequestException; import org.hisp.dhis.feedback.ConflictException; import org.hisp.dhis.feedback.ForbiddenException; +import org.hisp.dhis.feedback.NotFoundException; +import org.hisp.dhis.jsontree.Json; import org.hisp.dhis.jsontree.JsonMap; import org.hisp.dhis.jsontree.JsonMixed; import org.hisp.dhis.security.RequiresAuthority; @@ -89,8 +92,9 @@ public class SystemSettingsController { @RequiresAuthority(anyOf = F_SYSTEM_SETTING) @ResponseBody public WebMessage setSystemSettingPlain( - @PathVariable("key") String key, @RequestParam(value = "value") String value) { - settingsService.put(key, value); + @PathVariable("key") String key, @RequestParam(value = "value") String value) + throws NotFoundException, BadRequestException { + settingsService.putAll(Map.of(key, value)); return ok("System setting '" + key + "' set to value '" + value + "'."); } @@ -98,7 +102,8 @@ public WebMessage setSystemSettingPlain( @RequiresAuthority(anyOf = F_SYSTEM_SETTING) @ResponseBody public WebMessage setSystemSettingPlainBody( - @PathVariable("key") String key, @RequestBody String value) { + @PathVariable("key") String key, @RequestBody String value) + throws NotFoundException, BadRequestException { return setSystemSettingPlain(key, value); } @@ -107,27 +112,32 @@ public WebMessage setSystemSettingPlainBody( @ResponseBody public WebMessage setSystemSettingJson( @PathVariable("key") String key, @Language("json") @RequestBody String value) - throws ConflictException { + throws ConflictException, NotFoundException, BadRequestException { return setSystemSettingPlain(key, toJavaString(JsonMixed.of(value))); } @PostMapping(consumes = ContextUtils.CONTENT_TYPE_JSON) @RequiresAuthority(anyOf = F_SYSTEM_SETTING) @ResponseBody - public WebMessage setSystemSettingsJson(@RequestBody Map settings) { + public WebMessage setSystemSettingsJson(@RequestBody Map settings) + throws NotFoundException, BadRequestException { settingsService.putAll(settings); return ok("System settings imported"); } @GetMapping(value = "/{key}", produces = TEXT_PLAIN_VALUE) public ResponseEntity getSystemSettingPlain(@PathVariable("key") String key) - throws ForbiddenException, ConflictException { + throws ForbiddenException, ConflictException, NotFoundException { + checkExists(key); if (SystemSettings.isConfidential(key) && !getCurrentUserDetails().isSuper()) throw new ForbiddenException("Setting is marked as confidential"); - return ResponseEntity.ok() - .headers(noCacheNoStoreMustRevalidate()) - .body( - toJavaString(settingsService.getCurrentSettings().toJson(true, Set.of(key)).get(key))); + + String value = ""; + // Note: This exception is added in for backwards compatibility + if (SystemSettings.isTranslatable(key)) value = getSystemSettingTranslation(key); + if (value.isEmpty()) + value = toJavaString(settingsService.getCurrentSettings().toJson(true, Set.of(key)).get(key)); + return ResponseEntity.ok().headers(noCacheNoStoreMustRevalidate()).body(value); } @GetMapping(produces = APPLICATION_JSON_VALUE) @@ -140,18 +150,27 @@ public ResponseEntity getSystemSettingsJson() { @GetMapping(value = "/{key}", produces = APPLICATION_JSON_VALUE) public ResponseEntity> getSystemSettingJson( @PathVariable("key") String key, @CurrentUser UserDetails currentUser) - throws ForbiddenException { + throws ForbiddenException, NotFoundException { + checkExists(key); if (SystemSettings.isConfidential(key) && !currentUser.isSuper()) throw new ForbiddenException("Setting is marked as confidential"); - return ResponseEntity.ok() - .headers(noCacheNoStoreMustRevalidate()) - .body(settingsService.getCurrentSettings().toJson(true, Set.of(key))); + + JsonMap value = Json.ofNull().asMap(JsonMixed.class); + if (SystemSettings.isTranslatable(key)) { + String translation = getSystemSettingTranslation(key); + if (!translation.isEmpty()) { + value = Json.object(obj -> obj.addString(key, translation)).asMap(JsonMixed.class); + } + } + if (value.isNull()) value = settingsService.getCurrentSettings().toJson(true, Set.of(key)); + return ResponseEntity.ok().headers(noCacheNoStoreMustRevalidate()).body(value); } @DeleteMapping("/{key}") @RequiresAuthority(anyOf = F_SYSTEM_SETTING) @ResponseStatus(value = NO_CONTENT) - public void removeSystemSetting(@PathVariable("key") String key) { + public void removeSystemSetting(@PathVariable("key") String key) throws NotFoundException { + checkExists(key); settingsService.deleteAll(Set.of(key)); } @@ -171,9 +190,9 @@ public void removeSystemSetting(@RequestParam("key") Set keys) { @GetMapping(value = "/{key}", params = "locale", produces = TEXT_PLAIN_VALUE) public @ResponseBody ResponseEntity getSystemSettingTranslation( @PathVariable("key") String key, @RequestParam("locale") String locale) - throws ForbiddenException, ConflictException { - if (locale == null || locale.isEmpty()) - locale = UserSettings.getCurrentSettings().getUserUiLocale().getLanguage(); + throws ForbiddenException, ConflictException, NotFoundException { + checkExists(key); + if (locale == null || locale.isEmpty()) locale = getUserUiLanguage(); Optional translation = settingsTranslationService.getSystemSettingTranslation(key, locale); if (translation.isPresent()) @@ -181,6 +200,13 @@ public void removeSystemSetting(@RequestParam("key") Set keys) { return getSystemSettingPlain(key); } + @Nonnull + private String getSystemSettingTranslation(String key) { + return settingsTranslationService + .getSystemSettingTranslation(key, getUserUiLanguage()) + .orElse(""); + } + @PostMapping( value = "/{key}", params = {"locale", "value"}) @@ -218,4 +244,13 @@ public void removeSystemSettingTranslation( throws ForbiddenException, BadRequestException { settingsTranslationService.saveSystemSettingTranslation(key, locale, StringUtils.EMPTY); } + + private static void checkExists(String key) throws NotFoundException { + if (!SystemSettings.keysWithDefaults().contains(key)) + throw new NotFoundException("Setting does not exist"); + } + + private static String getUserUiLanguage() { + return UserSettings.getCurrentSettings().getUserUiLocale().getLanguage(); + } } From 2b707f81ffe1e3356d74ceb4777ad4ddbbef9711 Mon Sep 17 00:00:00 2001 From: Jan Bernitt Date: Tue, 15 Oct 2024 14:29:36 +0200 Subject: [PATCH 2/7] fix: set timezone for date string format --- .../src/main/java/org/hisp/dhis/setting/LazySettings.java | 2 +- .../java/org/hisp/dhis/setting/SystemSettingsTest.java | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/setting/LazySettings.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/setting/LazySettings.java index ee9e88bc5b1e..f8a0f6722197 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/setting/LazySettings.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/setting/LazySettings.java @@ -251,7 +251,7 @@ public JsonMap toJson(boolean includeConfidential, @Nonnull Set assertFalse(asJson.exists(key))); } From 3421c15be5eb6b916562df7092fe7373192429ee Mon Sep 17 00:00:00 2001 From: Jan Bernitt Date: Tue, 15 Oct 2024 17:15:21 +0200 Subject: [PATCH 3/7] fix: tests --- .../SystemSettingsTranslationService.java | 2 +- ...faultSystemSettingsTranslationService.java | 2 +- .../hisp/dhis/setting/SystemSettingsTest.java | 3 +- .../SystemSettingsTranslationServiceTest.java | 78 +++++++++---------- .../controller/LoginConfigControllerTest.java | 12 +-- .../SystemSettingsControllerTest.java | 6 +- .../controller/SystemSettingsController.java | 47 +++++------ 7 files changed, 76 insertions(+), 74 deletions(-) diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/setting/SystemSettingsTranslationService.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/setting/SystemSettingsTranslationService.java index bc7a4a15cd1b..f71ca07c346f 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/setting/SystemSettingsTranslationService.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/setting/SystemSettingsTranslationService.java @@ -48,7 +48,7 @@ public interface SystemSettingsTranslationService { * @param locale locale of the translation (should be a language tag) * @param translation translation text, null or empty to delete */ - void saveSystemSettingTranslation( + void putSystemSettingTranslation( @Nonnull String key, @Nonnull String locale, @CheckForNull String translation) throws ForbiddenException, BadRequestException; diff --git a/dhis-2/dhis-services/dhis-service-setting/src/main/java/org/hisp/dhis/setting/DefaultSystemSettingsTranslationService.java b/dhis-2/dhis-services/dhis-service-setting/src/main/java/org/hisp/dhis/setting/DefaultSystemSettingsTranslationService.java index b63ed0363bb7..7025b4d3acec 100644 --- a/dhis-2/dhis-services/dhis-service-setting/src/main/java/org/hisp/dhis/setting/DefaultSystemSettingsTranslationService.java +++ b/dhis-2/dhis-services/dhis-service-setting/src/main/java/org/hisp/dhis/setting/DefaultSystemSettingsTranslationService.java @@ -74,7 +74,7 @@ private void init() { @Override @Transactional - public void saveSystemSettingTranslation( + public void putSystemSettingTranslation( @Nonnull String key, @Nonnull String locale, String translation) throws ForbiddenException, BadRequestException { if (!SystemSettings.isTranslatable(key)) diff --git a/dhis-2/dhis-services/dhis-service-setting/src/test/java/org/hisp/dhis/setting/SystemSettingsTest.java b/dhis-2/dhis-services/dhis-service-setting/src/test/java/org/hisp/dhis/setting/SystemSettingsTest.java index c9520cbd97db..c8bb5f99a802 100644 --- a/dhis-2/dhis-services/dhis-service-setting/src/test/java/org/hisp/dhis/setting/SystemSettingsTest.java +++ b/dhis-2/dhis-services/dhis-service-setting/src/test/java/org/hisp/dhis/setting/SystemSettingsTest.java @@ -113,7 +113,8 @@ void testToJson() { assertFalse(booleanValue.as(JsonBoolean.class).booleanValue()); JsonString dateValue = asJson.get("keyLastMetaDataSyncSuccess"); assertTrue(dateValue.isString()); - assertEquals("1970-01-01T01:00:00.000", dateValue.string()); + assertEquals("1970-01-01T", dateValue.string().substring(0, 11)); + assertEquals(":00:00.000", dateValue.string().substring(13)); JsonString enumValue = asJson.get("keyCacheStrategy"); assertTrue(enumValue.isString()); assertEquals(CacheStrategy.CACHE_1_MINUTE, enumValue.parsed(CacheStrategy::valueOf)); diff --git a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/setting/SystemSettingsTranslationServiceTest.java b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/setting/SystemSettingsTranslationServiceTest.java index b6e99af4b786..42d237bc9f4a 100644 --- a/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/setting/SystemSettingsTranslationServiceTest.java +++ b/dhis-2/dhis-test-integration/src/test/java/org/hisp/dhis/setting/SystemSettingsTranslationServiceTest.java @@ -47,69 +47,69 @@ class SystemSettingsTranslationServiceTest extends PostgresIntegrationTestBase { @Test void testSaveSystemSettingTranslation_Add() throws Exception { - putTranslation("key1", "en", "text-en"); - putTranslation("key1", "de", "text-de"); - putTranslation("key2", "en", "text2-en"); - putTranslation("key2", "it", "text2-it"); - - assertTranslation("key1", "en", "text-en"); - assertTranslation("key1", "de", "text-de"); - assertNoTranslation("key1", "it"); - assertTranslation("key2", "en", "text2-en"); - assertTranslation("key2", "it", "text2-it"); - assertNoTranslation("key2", "de"); + putTranslation("applicationTitle", "en", "text-en"); + putTranslation("applicationTitle", "de", "text-de"); + putTranslation("loginPopup", "en", "text2-en"); + putTranslation("loginPopup", "it", "text2-it"); + + assertTranslation("applicationTitle", "en", "text-en"); + assertTranslation("applicationTitle", "de", "text-de"); + assertNoTranslation("applicationTitle", "it"); + assertTranslation("loginPopup", "en", "text2-en"); + assertTranslation("loginPopup", "it", "text2-it"); + assertNoTranslation("loginPopup", "de"); } @Test void testSaveSystemSettingTranslation_Update() throws Exception { - putTranslation("key1", "en", "text-en"); - putTranslation("key1", "de", "text-de"); + putTranslation("applicationTitle", "en", "text-en"); + putTranslation("applicationTitle", "de", "text-de"); - assertTranslation("key1", "en", "text-en"); - assertTranslation("key1", "de", "text-de"); + assertTranslation("applicationTitle", "en", "text-en"); + assertTranslation("applicationTitle", "de", "text-de"); - putTranslation("key1", "en", "new-text-en"); - assertTranslation("key1", "en", "new-text-en"); - assertTranslation("key1", "de", "text-de"); + putTranslation("applicationTitle", "en", "new-text-en"); + assertTranslation("applicationTitle", "en", "new-text-en"); + assertTranslation("applicationTitle", "de", "text-de"); } @Test void testSaveSystemSettingTranslation_DeleteNull() throws Exception { - putTranslation("key1", "en", "text-en"); - putTranslation("key1", "de", "text-de"); + putTranslation("applicationTitle", "en", "text-en"); + putTranslation("applicationTitle", "de", "text-de"); - putTranslation("key1", "en", null); - assertNoTranslation("key1", "en"); - assertTranslation("key1", "de", "text-de"); + putTranslation("applicationTitle", "en", null); + assertNoTranslation("applicationTitle", "en"); + assertTranslation("applicationTitle", "de", "text-de"); - putTranslation("key1", "de", null); - assertNoTranslation("key1", "en"); - assertNoTranslation("key1", "de"); + putTranslation("applicationTitle", "de", null); + assertNoTranslation("applicationTitle", "en"); + assertNoTranslation("applicationTitle", "de"); - putTranslation("key1", "it", null); - assertNoTranslation("key1", "it"); + putTranslation("applicationTitle", "it", null); + assertNoTranslation("applicationTitle", "it"); } @Test void testSaveSystemSettingTranslation_DeleteEmpty() throws Exception { - putTranslation("key1", "en", "text-en"); - putTranslation("key1", "de", "text-de"); + putTranslation("applicationTitle", "en", "text-en"); + putTranslation("applicationTitle", "de", "text-de"); - putTranslation("key1", "en", ""); - assertNoTranslation("key1", "en"); - assertTranslation("key1", "de", "text-de"); + putTranslation("applicationTitle", "en", ""); + assertNoTranslation("applicationTitle", "en"); + assertTranslation("applicationTitle", "de", "text-de"); - putTranslation("key1", "de", ""); - assertNoTranslation("key1", "en"); - assertNoTranslation("key1", "de"); + putTranslation("applicationTitle", "de", ""); + assertNoTranslation("applicationTitle", "en"); + assertNoTranslation("applicationTitle", "de"); - putTranslation("key1", "it", ""); - assertNoTranslation("key1", "it"); + putTranslation("applicationTitle", "it", ""); + assertNoTranslation("applicationTitle", "it"); } private void putTranslation(String key, String locale, String text) throws ForbiddenException, BadRequestException { - settingsTranslationService.saveSystemSettingTranslation(key, locale, text); + settingsTranslationService.putSystemSettingTranslation(key, locale, text); } private void assertTranslation(String key, String locale, String expected) { diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/LoginConfigControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/LoginConfigControllerTest.java index 463d37d7012f..feb42cb5a48b 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/LoginConfigControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/LoginConfigControllerTest.java @@ -79,27 +79,27 @@ void shouldGetLoginConfig() throws Exception { addGoogleProvider("testClientId"); settingsService.put("applicationTitle", "DHIS2"); - settingsTranslationService.saveSystemSettingTranslation( + settingsTranslationService.putSystemSettingTranslation( "applicationTitle", "no", "Distrikstshelsesinformasjonssystem versjon 2"); settingsService.put("loginPopup", "TEXT"); - settingsTranslationService.saveSystemSettingTranslation( + settingsTranslationService.putSystemSettingTranslation( "loginPopup", "no", "tekst"); settingsService.put("keyApplicationFooter", "APPLICATION_FOOTER"); - settingsTranslationService.saveSystemSettingTranslation( + settingsTranslationService.putSystemSettingTranslation( "keyApplicationFooter", "no", "Søknadsbunntekst"); settingsService.put("keyApplicationRightFooter", "APPLICATION_RIGHT_FOOTER"); - settingsTranslationService.saveSystemSettingTranslation( + settingsTranslationService.putSystemSettingTranslation( "keyApplicationRightFooter", "no", "Høyre søknadsbunntekst"); settingsService.put("keyApplicationIntro", "APPLICATION_INTRO"); - settingsTranslationService.saveSystemSettingTranslation( + settingsTranslationService.putSystemSettingTranslation( "keyApplicationIntro", "no", "Søknadsintroduksjon"); settingsService.put("keyApplicationNotification", "APPLICATION_NOTIFICATION"); - settingsTranslationService.saveSystemSettingTranslation( + settingsTranslationService.putSystemSettingTranslation( "keyApplicationNotification", "no", "Søknadsmelding"); settingsService.put("keyFlag", "FLAG_IMAGE"); diff --git a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/SystemSettingsControllerTest.java b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/SystemSettingsControllerTest.java index 96080372f1bd..8a22905ce0a2 100644 --- a/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/SystemSettingsControllerTest.java +++ b/dhis-2/dhis-test-web-api/src/test/java/org/hisp/dhis/webapi/controller/SystemSettingsControllerTest.java @@ -73,13 +73,13 @@ void testSetSystemSettingOrTranslation_Setting() { @Test void testSetSystemSettingOrTranslation_Translation() { - assertStatus(HttpStatus.OK, POST("/systemSettings/keyUiLocale?value=de")); + assertStatus(HttpStatus.OK, POST("/systemSettings/applicationTitle?value=Hello")); assertWebMessage( "OK", 200, "OK", - "Translation for system setting 'keyUiLocale' and locale: 'de' set to: 'Sprache'", - POST("/systemSettings/keyUiLocale?locale=de&value=Sprache").content(HttpStatus.OK)); + "Translation for system setting 'applicationTitle' and locale: 'de' set to: 'Ahoi'", + POST("/systemSettings/applicationTitle?locale=de&value=Ahoi").content(HttpStatus.OK)); } @Test diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/SystemSettingsController.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/SystemSettingsController.java index 4a9cd3fabcf2..bcd510399401 100644 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/SystemSettingsController.java +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/SystemSettingsController.java @@ -37,7 +37,6 @@ import static org.springframework.http.MediaType.TEXT_PLAIN_VALUE; import java.util.Map; -import java.util.Optional; import java.util.Set; import javax.annotation.Nonnull; import lombok.AllArgsConstructor; @@ -91,7 +90,7 @@ public class SystemSettingsController { @PostMapping(value = "/{key}", params = "value") @RequiresAuthority(anyOf = F_SYSTEM_SETTING) @ResponseBody - public WebMessage setSystemSettingPlain( + public WebMessage putSystemSettingPlain( @PathVariable("key") String key, @RequestParam(value = "value") String value) throws NotFoundException, BadRequestException { settingsService.putAll(Map.of(key, value)); @@ -101,25 +100,25 @@ public WebMessage setSystemSettingPlain( @PostMapping(value = "/{key}", consumes = TEXT_PLAIN_VALUE) @RequiresAuthority(anyOf = F_SYSTEM_SETTING) @ResponseBody - public WebMessage setSystemSettingPlainBody( + public WebMessage putSystemSettingPlainBody( @PathVariable("key") String key, @RequestBody String value) throws NotFoundException, BadRequestException { - return setSystemSettingPlain(key, value); + return putSystemSettingPlain(key, value); } @PostMapping(value = "/{key}", consumes = APPLICATION_JSON_VALUE) @RequiresAuthority(anyOf = F_SYSTEM_SETTING) @ResponseBody - public WebMessage setSystemSettingJson( + public WebMessage putSystemSettingJson( @PathVariable("key") String key, @Language("json") @RequestBody String value) throws ConflictException, NotFoundException, BadRequestException { - return setSystemSettingPlain(key, toJavaString(JsonMixed.of(value))); + return putSystemSettingPlain(key, toJavaString(JsonMixed.of(value))); } @PostMapping(consumes = ContextUtils.CONTENT_TYPE_JSON) @RequiresAuthority(anyOf = F_SYSTEM_SETTING) @ResponseBody - public WebMessage setSystemSettingsJson(@RequestBody Map settings) + public WebMessage putSystemSettingsJson(@RequestBody Map settings) throws NotFoundException, BadRequestException { settingsService.putAll(settings); return ok("System settings imported"); @@ -128,7 +127,7 @@ public WebMessage setSystemSettingsJson(@RequestBody Map setting @GetMapping(value = "/{key}", produces = TEXT_PLAIN_VALUE) public ResponseEntity getSystemSettingPlain(@PathVariable("key") String key) throws ForbiddenException, ConflictException, NotFoundException { - checkExists(key); + checkKeyExists(key); if (SystemSettings.isConfidential(key) && !getCurrentUserDetails().isSuper()) throw new ForbiddenException("Setting is marked as confidential"); @@ -151,7 +150,7 @@ public ResponseEntity getSystemSettingsJson() { public ResponseEntity> getSystemSettingJson( @PathVariable("key") String key, @CurrentUser UserDetails currentUser) throws ForbiddenException, NotFoundException { - checkExists(key); + checkKeyExists(key); if (SystemSettings.isConfidential(key) && !currentUser.isSuper()) throw new ForbiddenException("Setting is marked as confidential"); @@ -170,7 +169,7 @@ public ResponseEntity> getSystemSettingJson( @RequiresAuthority(anyOf = F_SYSTEM_SETTING) @ResponseStatus(value = NO_CONTENT) public void removeSystemSetting(@PathVariable("key") String key) throws NotFoundException { - checkExists(key); + checkKeyExists(key); settingsService.deleteAll(Set.of(key)); } @@ -191,13 +190,15 @@ public void removeSystemSetting(@RequestParam("key") Set keys) { public @ResponseBody ResponseEntity getSystemSettingTranslation( @PathVariable("key") String key, @RequestParam("locale") String locale) throws ForbiddenException, ConflictException, NotFoundException { - checkExists(key); + checkKeyExists(key); + if (!SystemSettings.isTranslatable(key)) return getSystemSettingPlain(key); if (locale == null || locale.isEmpty()) locale = getUserUiLanguage(); - Optional translation = - settingsTranslationService.getSystemSettingTranslation(key, locale); - if (translation.isPresent()) - return ResponseEntity.ok().headers(noCacheNoStoreMustRevalidate()).body(translation.get()); - return getSystemSettingPlain(key); + + String value = settingsTranslationService.getSystemSettingTranslation(key, locale).orElse(""); + if (value.isEmpty()) + value = + toJavaString(settingsService.getCurrentSettings().toJson(false, Set.of(key)).get(key)); + return ResponseEntity.ok().headers(noCacheNoStoreMustRevalidate()).body(value); } @Nonnull @@ -212,14 +213,14 @@ private String getSystemSettingTranslation(String key) { params = {"locale", "value"}) @RequiresAuthority(anyOf = F_SYSTEM_SETTING) @ResponseBody - public WebMessage setSystemSettingTranslation( + public WebMessage putSystemSettingTranslation( @PathVariable("key") String key, @RequestParam("locale") String locale, @RequestParam("value") String value) throws ForbiddenException, BadRequestException { if (value == null) throw new BadRequestException("Value must be specified as query param or as payload"); - settingsTranslationService.saveSystemSettingTranslation(key, locale, value); + settingsTranslationService.putSystemSettingTranslation(key, locale, value); return ok( "Translation for system setting '%s' and locale: '%s' set to: '%s'" .formatted(key, locale, value)); @@ -228,12 +229,12 @@ public WebMessage setSystemSettingTranslation( @PostMapping(value = "/{key}", params = "locale", consumes = TEXT_PLAIN_VALUE) @RequiresAuthority(anyOf = F_SYSTEM_SETTING) @ResponseBody - public WebMessage setSystemSettingTranslationBody( + public WebMessage putSystemSettingTranslationBody( @PathVariable("key") String key, @RequestParam("locale") String locale, @RequestBody String value) throws ForbiddenException, BadRequestException { - return setSystemSettingTranslation(key, locale, value); + return putSystemSettingTranslation(key, locale, value); } @DeleteMapping(value = "/{key}", params = "locale") @@ -242,12 +243,12 @@ public WebMessage setSystemSettingTranslationBody( public void removeSystemSettingTranslation( @PathVariable("key") String key, @RequestParam("locale") String locale) throws ForbiddenException, BadRequestException { - settingsTranslationService.saveSystemSettingTranslation(key, locale, StringUtils.EMPTY); + settingsTranslationService.putSystemSettingTranslation(key, locale, StringUtils.EMPTY); } - private static void checkExists(String key) throws NotFoundException { + private static void checkKeyExists(String key) throws NotFoundException { if (!SystemSettings.keysWithDefaults().contains(key)) - throw new NotFoundException("Setting does not exist"); + throw new NotFoundException("Setting does not exist: " + key); } private static String getUserUiLanguage() { From 3691731b0b5c6141763db0d49909af82f1d3751e Mon Sep 17 00:00:00 2001 From: Jan Bernitt Date: Tue, 15 Oct 2024 17:30:28 +0200 Subject: [PATCH 4/7] fix: e2e tests --- .../java/org/hisp/dhis/systemsettings/SystemSettingsTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/systemsettings/SystemSettingsTests.java b/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/systemsettings/SystemSettingsTests.java index 4bdfdc974e2d..5110a64b5a69 100644 --- a/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/systemsettings/SystemSettingsTests.java +++ b/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/systemsettings/SystemSettingsTests.java @@ -223,7 +223,7 @@ void deleteSystemSetting() { response.validate().statusCode(200); - assertEquals(StringUtils.EMPTY, response.getAsString()); + assertEquals(ENGLISH_INTRO, response.getAsString()); } @Test From fe73a0fcf6057d118442b540869f3ccac9354f52 Mon Sep 17 00:00:00 2001 From: Jan Bernitt Date: Wed, 16 Oct 2024 09:51:54 +0200 Subject: [PATCH 5/7] feat: user settings validation and co --- .../hisp/dhis/user/UserSettingsService.java | 17 ++++++++-- .../ui/locale/UserSettingLocaleManager.java | 30 ++++++----------- .../dhis/setting/DefaultStyleManager.java | 9 +++++- .../dhis/user/DefaultUserSettingsService.java | 32 ++++++++++++++++--- .../hooks/UserObjectBundleHook.java | 10 +++--- .../setting/DefaultSystemSettingsService.java | 2 +- .../CrudControllerIntegrationTest.java | 2 +- .../controller/UserSettingsController.java | 15 +++++++-- .../webapi/controller/user/MeController.java | 22 ++++--------- .../controller/user/UserController.java | 7 +++- 10 files changed, 91 insertions(+), 55 deletions(-) diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/user/UserSettingsService.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/user/UserSettingsService.java index f29c2d02aa60..aaaf5f7f7863 100644 --- a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/user/UserSettingsService.java +++ b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/user/UserSettingsService.java @@ -31,6 +31,8 @@ import java.util.Map; import javax.annotation.CheckForNull; import javax.annotation.Nonnull; +import org.hisp.dhis.feedback.BadRequestException; +import org.hisp.dhis.feedback.ConflictException; import org.hisp.dhis.feedback.NotFoundException; import org.hisp.dhis.setting.UserSettings; @@ -62,8 +64,11 @@ public interface UserSettingsService { * * @param key the user setting key. * @param value the setting value, null or empty to delete + * @throws NotFoundException when a setting with the key does not exist for users + * @throws BadRequestException when the value isn't valid for the key */ - void put(@Nonnull String key, @CheckForNull Serializable value); + void put(@Nonnull String key, @CheckForNull Serializable value) + throws NotFoundException, BadRequestException; /** * Saves the name/value pair as a user setting connected to user. @@ -71,18 +76,24 @@ public interface UserSettingsService { * @param key the user setting key. * @param value the setting value, null or empty to delete * @param username owner/target of the settings update + * @throws NotFoundException when a setting with the key does not exist for users + * @throws BadRequestException when the value isn't valid for the key + * @throws ConflictException when no user with the given username exists */ void put(@Nonnull String key, @CheckForNull Serializable value, @Nonnull String username) - throws NotFoundException; + throws NotFoundException, BadRequestException, ConflictException; /** * Updates all settings in the provided collection. * * @param settings settings to store, null or empty values are deleted * @param username owner/target of the settings update + * @throws NotFoundException when a setting with the key does not exist for users + * @throws BadRequestException when the value isn't valid for the key + * @throws ConflictException when no user with the given username exists */ void putAll(@Nonnull Map settings, @Nonnull String username) - throws NotFoundException; + throws NotFoundException, BadRequestException, ConflictException; /** * Deletes all settings of a user. If no such user exists this has no effect. diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/i18n/ui/locale/UserSettingLocaleManager.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/i18n/ui/locale/UserSettingLocaleManager.java index f86d43c6d31e..02a7f74f4fc6 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/i18n/ui/locale/UserSettingLocaleManager.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/i18n/ui/locale/UserSettingLocaleManager.java @@ -27,12 +27,13 @@ */ package org.hisp.dhis.i18n.ui.locale; -import static com.google.common.base.Preconditions.checkNotNull; - import java.util.ArrayList; import java.util.List; import java.util.Locale; import javax.annotation.CheckForNull; +import lombok.RequiredArgsConstructor; +import org.hisp.dhis.feedback.BadRequestException; +import org.hisp.dhis.feedback.NotFoundException; import org.hisp.dhis.i18n.locale.LocaleManager; import org.hisp.dhis.i18n.ui.resourcebundle.ResourceBundleManager; import org.hisp.dhis.i18n.ui.resourcebundle.ResourceBundleManagerException; @@ -45,28 +46,12 @@ * @author Torgeir Lorange Ostby */ @Component("org.hisp.dhis.i18n.locale.LocaleManager") +@RequiredArgsConstructor public class UserSettingLocaleManager implements LocaleManager { - // ------------------------------------------------------------------------- - // Dependencies - // ------------------------------------------------------------------------- private final UserSettingsService userSettingsService; - private final ResourceBundleManager resourceBundleManager; - public UserSettingLocaleManager( - UserSettingsService userSettingsService, ResourceBundleManager resourceBundleManager) { - checkNotNull(userSettingsService); - checkNotNull(resourceBundleManager); - - this.userSettingsService = userSettingsService; - this.resourceBundleManager = resourceBundleManager; - } - - // ------------------------------------------------------------------------- - // LocaleManager implementation - // ------------------------------------------------------------------------- - @Override public Locale getCurrentLocale() { @@ -84,7 +69,12 @@ public Locale getCurrentLocale() { @Override public void setCurrentLocale(Locale locale) { - userSettingsService.put("keyUiLocale", locale); + try { + userSettingsService.put("keyUiLocale", locale); + } catch (NotFoundException | BadRequestException ex) { + // this should never happen as this key-value combination is valid + throw new IllegalArgumentException(ex); + } } @Override diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/setting/DefaultStyleManager.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/setting/DefaultStyleManager.java index 94f4adc6635c..8badf05e6830 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/setting/DefaultStyleManager.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/setting/DefaultStyleManager.java @@ -37,6 +37,8 @@ import java.util.SortedMap; import java.util.TreeMap; import lombok.RequiredArgsConstructor; +import org.hisp.dhis.feedback.BadRequestException; +import org.hisp.dhis.feedback.NotFoundException; import org.hisp.dhis.i18n.I18n; import org.hisp.dhis.i18n.I18nManager; import org.hisp.dhis.user.CurrentUserUtil; @@ -77,7 +79,12 @@ public void setSystemStyle(String style) { @Override public void setUserStyle(String style) { - userSettingsService.put("keyStyle", style); + try { + userSettingsService.put("keyStyle", style); + } catch (NotFoundException | BadRequestException ex) { + // this should never happen as this key-value combination is valid + throw new IllegalArgumentException(ex); + } } @Override diff --git a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/user/DefaultUserSettingsService.java b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/user/DefaultUserSettingsService.java index f202fc6c4135..9a9cefe689ad 100644 --- a/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/user/DefaultUserSettingsService.java +++ b/dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/user/DefaultUserSettingsService.java @@ -31,6 +31,7 @@ import java.io.Serializable; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.NoSuchElementException; import java.util.Optional; @@ -38,6 +39,8 @@ import javax.annotation.CheckForNull; import javax.annotation.Nonnull; import lombok.RequiredArgsConstructor; +import org.hisp.dhis.feedback.BadRequestException; +import org.hisp.dhis.feedback.ConflictException; import org.hisp.dhis.feedback.NotFoundException; import org.hisp.dhis.setting.SessionUserSettings; import org.hisp.dhis.setting.Settings; @@ -110,10 +113,11 @@ private UserSettings getUserSettingsInternal(@Nonnull String username, boolean i @Override @Transactional - public void put(@Nonnull String key, Serializable value) { + public void put(@Nonnull String key, Serializable value) + throws NotFoundException, BadRequestException { try { put(key, value, CurrentUserUtil.getCurrentUsername()); - } catch (NotFoundException ex) { + } catch (ConflictException ex) { // we know the user exists so this should never happen throw new NoSuchElementException(ex); } @@ -122,17 +126,20 @@ public void put(@Nonnull String key, Serializable value) { @Override @Transactional public void put(@Nonnull String key, @CheckForNull Serializable value, @Nonnull String username) - throws NotFoundException { + throws NotFoundException, BadRequestException, ConflictException { putAll(Map.of(key, Settings.valueOf(value)), username); } @Override @Transactional public void putAll(@Nonnull Map settings, @Nonnull String username) - throws NotFoundException { + throws NotFoundException, BadRequestException, ConflictException { + if (settings.isEmpty()) return; + validateAll(settings); + User user = userStore.getUserByUsername(username); if (user == null) - throw new NotFoundException( + throw new ConflictException( "%s with username %s could not be found." .formatted(User.class.getSimpleName(), username)); Set deletes = new HashSet<>(); @@ -149,6 +156,21 @@ public void putAll(@Nonnull Map settings, @Nonnull String userna updateSession(username); } + private void validateAll(@Nonnull Map settings) + throws NotFoundException, BadRequestException { + Set allowed = UserSettings.keysWithDefaults(); + List illegal = + settings.keySet().stream().filter(key -> !allowed.contains(key)).toList(); + if (!illegal.isEmpty()) + throw new NotFoundException("Setting does not exist: " + String.join(",", illegal)); + UserSettings empty = UserSettings.of(Map.of()); + for (Map.Entry e : settings.entrySet()) { + if (!empty.isValid(e.getKey(), e.getValue())) + throw new BadRequestException( + "Not a valid value for setting %s: %s".formatted(e.getKey(), e.getValue())); + } + } + @Override @Transactional public void deleteAll(@Nonnull String username) { diff --git a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/UserObjectBundleHook.java b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/UserObjectBundleHook.java index f4de028aea5a..a32a9334ec3f 100644 --- a/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/UserObjectBundleHook.java +++ b/dhis-2/dhis-services/dhis-service-dxf2/src/main/java/org/hisp/dhis/dxf2/metadata/objectbundle/hooks/UserObjectBundleHook.java @@ -29,7 +29,6 @@ import java.util.HashSet; import java.util.Map; -import java.util.NoSuchElementException; import java.util.Objects; import java.util.Set; import java.util.function.Consumer; @@ -41,6 +40,8 @@ import org.hisp.dhis.dxf2.metadata.objectbundle.ObjectBundle; import org.hisp.dhis.external.conf.ConfigurationKey; import org.hisp.dhis.external.conf.DhisConfigurationProvider; +import org.hisp.dhis.feedback.BadRequestException; +import org.hisp.dhis.feedback.ConflictException; import org.hisp.dhis.feedback.ErrorCode; import org.hisp.dhis.feedback.ErrorReport; import org.hisp.dhis.feedback.NotFoundException; @@ -300,9 +301,10 @@ private void updateUserSettings(User user) { if (settings == null) return; try { userSettingsService.putAll(settings, user.getUsername()); - } catch (NotFoundException ex) { - // we know the user exists so this should never happen - throw new NoSuchElementException(ex); + } catch (NotFoundException | ConflictException | BadRequestException ex) { + // this should never happen as this key-value combination should be valid and the user does + // exist + throw new IllegalArgumentException(ex); } } } diff --git a/dhis-2/dhis-services/dhis-service-setting/src/main/java/org/hisp/dhis/setting/DefaultSystemSettingsService.java b/dhis-2/dhis-services/dhis-service-setting/src/main/java/org/hisp/dhis/setting/DefaultSystemSettingsService.java index 7e581b16f6d3..ebd091a2b4db 100644 --- a/dhis-2/dhis-services/dhis-service-setting/src/main/java/org/hisp/dhis/setting/DefaultSystemSettingsService.java +++ b/dhis-2/dhis-services/dhis-service-setting/src/main/java/org/hisp/dhis/setting/DefaultSystemSettingsService.java @@ -152,7 +152,7 @@ private void validateAll(@Nonnull Map settings) for (Map.Entry e : settings.entrySet()) { if (!empty.isValid(e.getKey(), e.getValue())) throw new BadRequestException( - "Setting %s cannot have value %s".formatted(e.getKey(), e.getValue())); + "Not a valid value for setting %s: %s".formatted(e.getKey(), e.getValue())); } } 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 2656f9fae1d5..d4677f098c61 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 @@ -143,7 +143,7 @@ void testSearchTokenDefaultLocale() { @Test @DisplayName("Search by token should use default properties instead of translations column") - void testSearchTokenWithNullLocale() { + void testSearchTokenWithNullLocale() throws Exception { setUpTranslation(); doInTransaction(() -> settingsService.put("keyDbLocale", Locale.ENGLISH)); settingsService.clearCurrentSettings(); diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/UserSettingsController.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/UserSettingsController.java index 3a6b305fc3d6..f5b84a3bfddf 100644 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/UserSettingsController.java +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/UserSettingsController.java @@ -41,6 +41,7 @@ import org.hisp.dhis.common.OpenApi; import org.hisp.dhis.common.UID; import org.hisp.dhis.dxf2.webmessage.WebMessage; +import org.hisp.dhis.feedback.BadRequestException; import org.hisp.dhis.feedback.ConflictException; import org.hisp.dhis.feedback.ForbiddenException; import org.hisp.dhis.feedback.NotFoundException; @@ -102,6 +103,7 @@ public class UserSettingsController { String userId, HttpServletResponse response) throws ForbiddenException, ConflictException, NotFoundException { + checkKeyExists(key); response.setHeader( ContextUtils.HEADER_CACHE_CONTROL, CacheControl.noCache().cachePrivate().getHeaderValue()); @@ -112,14 +114,15 @@ public class UserSettingsController { } @PostMapping(value = "/{key}") - public WebMessage setUserSettingByKey( + public WebMessage putUserSettingByKey( @PathVariable(value = "key") String key, @RequestParam(value = "user", required = false) String username, @OpenApi.Param({UID.class, User.class}) @RequestParam(value = "userId", required = false) String userId, @RequestParam(required = false) String value, @RequestBody(required = false) String valuePayload) - throws ForbiddenException, ConflictException, NotFoundException { + throws ForbiddenException, ConflictException, NotFoundException, BadRequestException { + checkKeyExists(key); String newValue = firstNonNull(value, valuePayload); @@ -137,7 +140,8 @@ public void deleteUserSettingByKey( @RequestParam(value = "user", required = false) String username, @OpenApi.Param({UID.class, User.class}) @RequestParam(value = "userId", required = false) String userId) - throws ForbiddenException, NotFoundException { + throws ForbiddenException, NotFoundException, ConflictException, BadRequestException { + checkKeyExists(key); if (username == null) username = getUsername(userId); userSettingsService.put(key, null, username); } @@ -172,4 +176,9 @@ private UserSettings getUserSettings(String userId, String username, boolean use if (username == null) username = getUsername(userId); return userSettingsService.getUserSettings(username, useFallback); } + + private static void checkKeyExists(String key) throws NotFoundException { + if (!UserSettings.keysWithDefaults().contains(key)) + throw new NotFoundException("Setting does not exist: " + key); + } } diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/user/MeController.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/user/MeController.java index 7aefdcf5871b..fbcedc684843 100644 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/user/MeController.java +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/user/MeController.java @@ -81,7 +81,6 @@ import org.hisp.dhis.user.User; import org.hisp.dhis.user.UserDetails; import org.hisp.dhis.user.UserService; -import org.hisp.dhis.user.UserSettingsService; import org.hisp.dhis.webapi.mvc.annotation.ApiVersion; import org.hisp.dhis.webapi.service.ContextService; import org.hisp.dhis.webapi.webdomain.Dashboard; @@ -133,8 +132,6 @@ public class MeController { @Nonnull private final NodeService nodeService; - @Nonnull private final UserSettingsService userSettingsService; - @Nonnull private final PasswordValidationService passwordValidationService; @Nonnull private final ProgramService programService; @@ -209,7 +206,7 @@ private boolean fieldsContains(String key, List fields) { @GetMapping("/dataApprovalWorkflows") public ResponseEntity getCurrentUserDataApprovalWorkflows( - HttpServletResponse response, @CurrentUser(required = true) User user) { + @CurrentUser(required = true) User user) { ObjectNode objectNode = userControllerUtils.getUserDataApprovalWorkflows(user); return ResponseEntity.ok(objectNode); } @@ -288,7 +285,7 @@ public ResponseEntity hasAuthority( @GetMapping(value = "/settings/{key}", produces = APPLICATION_JSON_VALUE) public @ResponseBody JsonValue getSetting(@PathVariable String key) { - return UserSettings.getCurrentSettings().toJson(false).get(key); + return UserSettings.getCurrentSettings().toJson(false, Set.of(key)).get(key); } @OpenApi.Document(group = OpenApi.Document.GROUP_MANAGE) @@ -321,9 +318,7 @@ public void changePassword( @OpenApi.Document(group = OpenApi.Document.GROUP_MANAGE) @PostMapping(value = "/verifyPassword", consumes = "text/*") public @ResponseBody RootNode verifyPasswordText( - @RequestBody String password, - HttpServletResponse response, - @CurrentUser(required = true) User currentUser) + @RequestBody String password, @CurrentUser(required = true) User currentUser) throws ConflictException { return verifyPasswordInternal(password, currentUser); } @@ -331,9 +326,7 @@ public void changePassword( @OpenApi.Document(group = OpenApi.Document.GROUP_MANAGE) @PostMapping(value = "/validatePassword", consumes = "text/*") public @ResponseBody RootNode validatePasswordText( - @RequestBody String password, - HttpServletResponse response, - @CurrentUser(required = true) User currentUser) + @RequestBody String password, @CurrentUser(required = true) User currentUser) throws ConflictException { return validatePasswordInternal(password, currentUser); } @@ -341,16 +334,13 @@ public void changePassword( @OpenApi.Document(group = OpenApi.Document.GROUP_MANAGE) @PostMapping(value = "/verifyPassword", consumes = APPLICATION_JSON_VALUE) public @ResponseBody RootNode verifyPasswordJson( - @RequestBody Map body, - HttpServletResponse response, - @CurrentUser(required = true) User currentUser) + @RequestBody Map body, @CurrentUser(required = true) User currentUser) throws ConflictException { return verifyPasswordInternal(body.get("password"), currentUser); } @GetMapping("/dashboard") - public @ResponseBody Dashboard getDashboard( - HttpServletResponse response, @CurrentUser(required = true) User currentUser) { + public @ResponseBody Dashboard getDashboard(HttpServletResponse response) { Dashboard dashboard = new Dashboard(); dashboard.setUnreadMessageConversations(messageService.getUnreadMessageConversationCount()); dashboard.setUnreadInterpretations(interpretationService.getNewInterpretationCount()); diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/user/UserController.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/user/UserController.java index d9ae26576aef..4c5645b48996 100644 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/user/UserController.java +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/user/UserController.java @@ -76,6 +76,7 @@ import org.hisp.dhis.dxf2.webmessage.WebMessage; import org.hisp.dhis.dxf2.webmessage.WebMessageException; import org.hisp.dhis.dxf2.webmessage.WebMessageUtils; +import org.hisp.dhis.feedback.BadRequestException; import org.hisp.dhis.feedback.ConflictException; import org.hisp.dhis.feedback.ErrorCode; import org.hisp.dhis.feedback.ErrorReport; @@ -417,7 +418,11 @@ public void resetToInvite(@PathVariable String id, HttpServletRequest request) @ResponseBody public WebMessage replicateUser( @PathVariable String uid, HttpServletRequest request, HttpServletResponse response) - throws IOException, ForbiddenException, ConflictException, NotFoundException { + throws IOException, + ForbiddenException, + ConflictException, + NotFoundException, + BadRequestException { User existingUser = userService.getUser(uid); if (existingUser == null) { return conflict("User not found: " + uid); From d6a9c3a6cc513b1bfad35f736037961d31970b2f Mon Sep 17 00:00:00 2001 From: Jan Bernitt Date: Wed, 16 Oct 2024 09:56:45 +0200 Subject: [PATCH 6/7] chore: code format --- .../java/org/hisp/dhis/systemsettings/SystemSettingsTests.java | 1 - 1 file changed, 1 deletion(-) diff --git a/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/systemsettings/SystemSettingsTests.java b/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/systemsettings/SystemSettingsTests.java index 5110a64b5a69..18afa65962a9 100644 --- a/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/systemsettings/SystemSettingsTests.java +++ b/dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/systemsettings/SystemSettingsTests.java @@ -35,7 +35,6 @@ import com.google.gson.JsonObject; import io.restassured.http.ContentType; import io.restassured.response.ValidatableResponse; -import org.apache.commons.lang3.StringUtils; import org.hisp.dhis.ApiTest; import org.hisp.dhis.helpers.TestCleanUp; import org.hisp.dhis.test.e2e.actions.LoginActions; From fc16c96316d876877f28d0666e67c443fdf68da8 Mon Sep 17 00:00:00 2001 From: Jan Bernitt Date: Wed, 16 Oct 2024 12:13:02 +0200 Subject: [PATCH 7/7] fix: always route to translation endpoint for all accept headers --- .../dhis/webapi/controller/SystemSettingsController.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/SystemSettingsController.java b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/SystemSettingsController.java index bcd510399401..962b16ffbd6b 100644 --- a/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/SystemSettingsController.java +++ b/dhis-2/dhis-web-api/src/main/java/org/hisp/dhis/webapi/controller/SystemSettingsController.java @@ -62,6 +62,7 @@ import org.hisp.dhis.webapi.mvc.annotation.ApiVersion; import org.hisp.dhis.webapi.utils.ContextUtils; import org.intellij.lang.annotations.Language; +import org.springframework.http.MediaType; import org.springframework.http.ResponseEntity; import org.springframework.stereotype.Controller; import org.springframework.web.bind.annotation.DeleteMapping; @@ -186,7 +187,7 @@ public void removeSystemSetting(@RequestParam("key") Set keys) { */ - @GetMapping(value = "/{key}", params = "locale", produces = TEXT_PLAIN_VALUE) + @GetMapping(value = "/{key}", params = "locale") public @ResponseBody ResponseEntity getSystemSettingTranslation( @PathVariable("key") String key, @RequestParam("locale") String locale) throws ForbiddenException, ConflictException, NotFoundException { @@ -198,7 +199,10 @@ public void removeSystemSetting(@RequestParam("key") Set keys) { if (value.isEmpty()) value = toJavaString(settingsService.getCurrentSettings().toJson(false, Set.of(key)).get(key)); - return ResponseEntity.ok().headers(noCacheNoStoreMustRevalidate()).body(value); + return ResponseEntity.ok() + .contentType(MediaType.TEXT_PLAIN) + .headers(noCacheNoStoreMustRevalidate()) + .body(value); } @Nonnull