feat: settings closed set with validation [DHIS2-18217]

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<String> CONFIDENTIAL_KEYS = new HashSet<>();
+  private static final Set<String> TRANSLATABLE_KEYS = new HashSet<>();
+
   private static final Map<String, Serializable> DEFAULTS_SYSTEM_SETTINGS =
       extractDefaults(SystemSettings.class);
   private static final Map<String, Serializable> 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<? extends Settings> type, @Nonnull Map<String, String> settings) {
     return of(type, settings, (k, v) -> v);
@@ -243,12 +250,15 @@ public JsonMap<JsonMixed> toJson(boolean includeConfidential, @Nonnull Set<Strin
         .asMap(JsonMixed.class);
   }
 
+  private static final DateTimeFormatter ISO_DATE_TIME =
+      DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss.SSS");
+
   @Nonnull
   private JsonValue asJson(String key) {
     Serializable defaultValue = getDefault(key);
     if (defaultValue instanceof String s) return Json.of(asString(key, s));
     if (defaultValue instanceof Date d)
-      return Json.of(DateTimeFormatter.ISO_INSTANT.format(asDate(key, d).toInstant()));
+      return Json.of(ISO_DATE_TIME.format(asDate(key, d).toInstant()));
     if (defaultValue instanceof Double d) return Json.of(asDouble(key, d));
     if (defaultValue instanceof Number n) return Json.of(asInt(key, n.intValue()));
     if (defaultValue instanceof Boolean b) return Json.of(asBoolean(key, b));
@@ -262,6 +272,25 @@ private JsonValue asJson(String key) {
     return Json.of(value);
   }
 
+  @Override
+  public boolean isValid(String key, String value) {
+    Serializable defaultValue = getDefault(key);
+    if (defaultValue == null || defaultValue instanceof String) return true;
+    if (defaultValue instanceof Boolean) return "true".equals(value) || "false".equals(value);
+    try {
+      // Note: The != null is just a dummy test the parse yielded anything
+      if (defaultValue instanceof Double) return Double.valueOf(value) != null;
+      if (defaultValue instanceof Number) return Integer.valueOf(value) != null;
+      if (defaultValue instanceof Date) return parseDate(value) != null;
+      if (defaultValue instanceof Locale) return LocaleUtils.toLocale(value) != null;
+      if (defaultValue instanceof Enum<?>)
+        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<String, Serializable> extractDefaults(Class<? extends Setting
                 defaults.put(key, defaultValue);
                 if (lastDefault[0].isAnnotationPresent(Confidential.class))
                   CONFIDENTIAL_KEYS.add(key);
+                if (lastDefault[0].isAnnotationPresent(Translatable.class))
+                  TRANSLATABLE_KEYS.add(key);
               }
               return args[1];
             });

dhis-2/dhis-api/src/main/java/org/hisp/dhis/setting/Settings.java

@@ -114,4 +114,6 @@ static String valueOf(@CheckForNull Serializable value) {
   double asDouble(@Nonnull String key, double defaultValue);
 
   boolean asBoolean(@Nonnull String key, boolean defaultValue);
+
+  boolean isValid(String key, String value);
 }

dhis-2/dhis-api/src/main/java/org/hisp/dhis/setting/SystemSettings.java

@@ -49,6 +49,7 @@
 import org.hisp.dhis.period.RelativePeriodEnum;
 import org.hisp.dhis.scheduling.JobConfiguration;
 import org.hisp.dhis.security.LoginPageLayout;
+import org.hisp.dhis.translation.Translatable;
 
 /**
  * A complete set of system settings.
@@ -106,6 +107,14 @@ static boolean isConfidential(@Nonnull String key) {
     return LazySettings.isConfidential(key);
   }
 
+  /**
+   * @param key a setting name
+   * @return true, if it can have a translation, false otherwise
+   */
+  static boolean isTranslatable(@Nonnull String key) {
+    return LazySettings.isTranslatable(key);
+  }
+
   /*
   settings used in core
    */
@@ -134,22 +143,27 @@ default String getTrackerDashboardLayout() {
     return asString("keyTrackerDashboardLayout", "");
   }
 
+  @Translatable
   default String getApplicationTitle() {
     return asString("applicationTitle", "DHIS 2");
   }
 
+  @Translatable
   default String getApplicationIntro() {
     return asString("keyApplicationIntro", "");
   }
 
+  @Translatable
   default String getApplicationNotification() {
     return asString("keyApplicationNotification", "");
   }
 
+  @Translatable
   default String getApplicationFooter() {
     return asString("keyApplicationFooter", "");
   }
 
+  @Translatable
   default String getApplicationRightFooter() {
     return asString("keyApplicationRightFooter", "");
   }
@@ -674,6 +688,7 @@ default int getTrackedEntityMaxLimit() {
     return asInt("KeyTrackedEntityMaxLimit", 50000);
   }
 
+  @Translatable
   default String getLoginPopup() {
     return asString("loginPopup", "");
   }

dhis-2/dhis-api/src/main/java/org/hisp/dhis/setting/SystemSettingsService.java

@@ -32,6 +32,8 @@
 import java.util.Set;
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
+import org.hisp.dhis.feedback.BadRequestException;
+import org.hisp.dhis.feedback.NotFoundException;
 
 /**
  * @author Jan Bernitt (refactored version)
@@ -48,6 +50,9 @@ public interface SystemSettingsService extends SystemSettingsProvider {
    * Saves a single setting with {@link Settings#valueOf(Serializable)} applied to the provided
    * value.
    *
+   * <p>This 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<String, String> settings);
+  void putAll(@Nonnull Map<String, String> settings) throws NotFoundException, BadRequestException;
 
   /**
    * Deletes the system setting with the given name.

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

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());
     }
   }
 

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());
     }
   }
 

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<String, String> settings) {
+  public void putAll(@Nonnull Map<String, String> settings)
+      throws NotFoundException, BadRequestException {
     if (settings.isEmpty()) return;
+    validateAll(settings);
+
     Set<String> deletes = new HashSet<>();
     for (Map.Entry<String, String> e : settings.entrySet()) {
       String key = e.getKey();
@@ -129,6 +141,21 @@ public void putAll(@Nonnull Map<String, String> settings) {
     allSettings = null; // invalidate
   }
 
+  private void validateAll(@Nonnull Map<String, String> settings)
+      throws NotFoundException, BadRequestException {
+    Set<String> allowed = SystemSettings.keysWithDefaults();
+    List<String> 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<String, String> 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<String> keys) {

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;