message overrides v2

Feuermagier · Jan 12, 2025 · 3165947 · 3165947
1 parent 3381ab0
commit 3165947
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 21 deletions.
diff --git a/autograder-api/src/main/java/de/firemage/autograder/api/AbstractLinter.java b/autograder-api/src/main/java/de/firemage/autograder/api/AbstractLinter.java
@@ -26,7 +26,7 @@ class Builder {
         private ClassLoader classLoader;
         private int maxProblemsPerCheck = -1;
         private List<FluentResource> messageOverrides = new ArrayList<>();
-        private Map<AbstractProblemType, List<String>> conditionalOverrides = new HashMap<>();
+        private Map<AbstractProblemType, List<FluentResource>> conditionalOverrides = new HashMap<>();
 
         private Builder(Locale locale) {
             this.locale = locale;
@@ -88,28 +88,25 @@ public List<FluentResource> getMessageOverrides() {
 
         /**
          * Add a message override that only applies if the message was emitted for the specified problem type.
-         * Conditional overrides override all other overrides. The (problemType, key) pair must be unique.
+         * Conditional overrides override all other overrides.
          * @param problemType
-         * @param key
-         * @param value
+         * @param bundle
          * @return this
          */
-        public Builder conditionalOverride(AbstractProblemType problemType, String key, String value) {
-            this.conditionalOverrides.computeIfAbsent(problemType, k -> new ArrayList<>()).add(key + " = " + value);
+        public Builder conditionalOverride(AbstractProblemType problemType, FluentResource bundle) {
+            this.conditionalOverrides.computeIfAbsent(problemType, k -> new ArrayList<>()).add(bundle);
             return this;
         }
 
         /**
-         * Sets all conditional overrides, discarding all previously set conditional overrides.
-         * @param conditionalOverrides
-         * @return
+         * @see #conditionalOverride(AbstractProblemType, FluentResource)
          */
-        public Builder setConditionalOverrides(Map<AbstractProblemType, List<String>> conditionalOverrides) {
-            this.conditionalOverrides = new HashMap<>(conditionalOverrides);
+        public Builder conditionalOverride(Map<AbstractProblemType, FluentResource> bundles) {
+            bundles.forEach(this::conditionalOverride);
             return this;
         }
 
-        public Map<AbstractProblemType, List<String>> getConditionalOverrides() {
+        public Map<AbstractProblemType, List<FluentResource>> getConditionalOverrides() {
             return this.conditionalOverrides;
         }
     }

diff --git a/autograder-api/src/main/java/de/firemage/autograder/api/FluentBuilder.java b/autograder-api/src/main/java/de/firemage/autograder/api/FluentBuilder.java
@@ -0,0 +1,40 @@
+package de.firemage.autograder.api;
+
+import fluent.bundle.FluentResource;
+import fluent.syntax.parser.FTLParser;
+import fluent.syntax.parser.FTLStream;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Helper for building FluentResources from strings.
+ */
+public class FluentBuilder {
+    private final List<String> lines = new ArrayList<>();
+
+    public static FluentResource ofSingle(String id, String value) {
+        return build(id + " = " + value);
+    }
+
+    public FluentBuilder message(String id, String value) {
+        lines.add(id + " = " + value);
+        return this;
+    }
+
+    public FluentResource build() {
+        return build(String.join("\n", lines));
+    }
+
+    private static FluentResource build(String content) {
+        if (content.isBlank()) {
+            return new FluentResource(List.of(), List.of(), List.of());
+        }
+
+        var bundle = FTLParser.parse(FTLStream.of(content));
+        if (bundle.hasErrors()) {
+            throw new IllegalStateException("Could not parse the fluent resource: " + bundle.errors().toString());
+        }
+        return bundle;
+    }
+}
diff --git a/autograder-core/src/main/java/de/firemage/autograder/core/Translations.java b/autograder-core/src/main/java/de/firemage/autograder/core/Translations.java
@@ -19,7 +19,7 @@ public class Translations implements AbstractTranslations {
     private final FluentBundle mainTranslations;
     private final Map<AbstractProblemType, FluentBundle> conditionalTranslations;
 
-    public Translations(Locale locale, List<FluentResource> mainOverrides, Map<AbstractProblemType, List<String>> conditionalOverrides) {
+    public Translations(Locale locale, List<FluentResource> mainOverrides, Map<AbstractProblemType, List<FluentResource>> conditionalOverrides) {
         String filename = switch (locale.getLanguage()) {
             case "de" -> "/strings.de.ftl";
             case "en" -> "/strings.en.ftl";
@@ -51,8 +51,9 @@ public Translations(Locale locale, List<FluentResource> mainOverrides, Map<Abstr
             this.conditionalTranslations = new HashMap<>();
             for (var entry : conditionalOverrides.entrySet()) {
                 var conditionalBuilder = FluentBundle.builder(locale, ICUFunctionFactory.INSTANCE);
-                var content = String.join("\n", entry.getValue());
-                conditionalBuilder.addResource(FTLParser.parse(FTLStream.of(content)));
+                for (var resource : entry.getValue()) {
+                    conditionalBuilder.addResourceOverriding(resource);
+                }
                 this.conditionalTranslations.put(entry.getKey(), conditionalBuilder.build());
             }
         } catch (IOException e) {

diff --git a/autograder-core/src/test/java/de/firemage/autograder/core/TestMessageOverriding.java b/autograder-core/src/test/java/de/firemage/autograder/core/TestMessageOverriding.java
@@ -1,6 +1,7 @@
 package de.firemage.autograder.core;
 
 import de.firemage.autograder.api.AbstractLinter;
+import de.firemage.autograder.api.FluentBuilder;
 import fluent.syntax.parser.FTLParser;
 import fluent.syntax.parser.FTLStream;
 import org.junit.jupiter.api.Test;
@@ -57,7 +58,7 @@ void testNonOverriddenMessageWithOverrides() {
     @Test
     void testConditionalMessageOverride() {
         var builder = AbstractLinter.builder(Locale.ENGLISH)
-                .conditionalOverride(ProblemType.AVOID_LABELS, "avoid-labels", "Foo Bar");
+                .conditionalOverride(ProblemType.AVOID_LABELS, FluentBuilder.ofSingle("avoid-labels", "Foo Bar"));
         var linter = new Linter(builder);
         String msg = linter.translateMessage(new LocalizedMessageForProblem(new LocalizedMessage("avoid-labels"), ProblemType.AVOID_LABELS));
         assertEquals("Foo Bar", msg);
@@ -66,8 +67,8 @@ void testConditionalMessageOverride() {
     @Test
     void testMultipleConditionalMessageOverrides() {
         var builder = AbstractLinter.builder(Locale.ENGLISH)
-                .conditionalOverride(ProblemType.AVOID_SHADOWING, "avoid-shadowing", "1234")
-                .conditionalOverride(ProblemType.AVOID_LABELS, "avoid-labels", "Foo Bar");
+                .conditionalOverride(ProblemType.AVOID_SHADOWING, FluentBuilder.ofSingle("avoid-shadowing", "1234"))
+                .conditionalOverride(ProblemType.AVOID_LABELS, FluentBuilder.ofSingle("avoid-labels", "Foo Bar"));
         var linter = new Linter(builder);
 
         String msg = linter.translateMessage(new LocalizedMessageForProblem(new LocalizedMessage("avoid-labels"), ProblemType.AVOID_LABELS));
@@ -80,8 +81,8 @@ void testMultipleConditionalMessageOverrides() {
     @Test
     void testMultipleConditionalMessageOverridesForSameProblemType() {
         var builder = AbstractLinter.builder(Locale.ENGLISH)
-                .conditionalOverride(ProblemType.AVOID_SHADOWING, "avoid-shadowing", "1234")
-                .conditionalOverride(ProblemType.AVOID_SHADOWING, "avoid-labels", "Foo Bar");
+                .conditionalOverride(ProblemType.AVOID_SHADOWING, FluentBuilder.ofSingle("avoid-shadowing", "1234"))
+                .conditionalOverride(ProblemType.AVOID_SHADOWING, FluentBuilder.ofSingle("avoid-labels", "Foo Bar"));
         var linter = new Linter(builder);
 
         String msg = linter.translateMessage(new LocalizedMessageForProblem(new LocalizedMessage("avoid-labels"), ProblemType.AVOID_SHADOWING));
@@ -94,7 +95,7 @@ void testMultipleConditionalMessageOverridesForSameProblemType() {
     @Test
     void testConditionalMessageOverrideForSameProblemType() {
         var builder = AbstractLinter.builder(Locale.ENGLISH)
-                .conditionalOverride(ProblemType.AVOID_SHADOWING, "avoid-shadowing", "1234");
+                .conditionalOverride(ProblemType.AVOID_SHADOWING, FluentBuilder.ofSingle("avoid-shadowing", "1234"));
         var linter = new Linter(builder);
 
         String msg = linter.translateMessage(new LocalizedMessageForProblem(new LocalizedMessage("avoid-labels"), ProblemType.AVOID_SHADOWING));