use JsonNode in the I18nServlet

scm-webapp/src/main/java/sonia/scm/util/JacksonUtils.java

@@ -0,0 +1,49 @@
+package sonia.scm.util;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.node.ObjectNode;
+
+import java.util.Iterator;
+
+public class JacksonUtils {
+
+  private JacksonUtils() {
+  }
+
+  public static JsonNode merge(JsonNode mainNode, JsonNode updateNode) {
+    Iterator<String> fieldNames = updateNode.fieldNames();
+
+    while (fieldNames.hasNext()) {
+
+      String fieldName = fieldNames.next();
+      JsonNode jsonNode = mainNode.get(fieldName);
+
+      if (jsonNode != null) {
+        if (jsonNode.isObject()) {
+          merge(jsonNode, updateNode.get(fieldName));
+        } else if (jsonNode.isArray()) {
+          for (int i = 0; i < jsonNode.size(); i++) {
+            merge(jsonNode.get(i), updateNode.get(fieldName).get(i));
+          }
+        }
+      } else {
+        if (mainNode instanceof ObjectNode) {
+          // Overwrite field
+          JsonNode value = updateNode.get(fieldName);
+          if (value.isNull()) {
+            continue;
+          }
+          if (value.isIntegralNumber() && value.toString().equals("0")) {
+            continue;
+          }
+          if (value.isFloatingPointNumber() && value.toString().equals("0.0")) {
+            continue;
+          }
+          ((ObjectNode) mainNode).put(fieldName, value);
+        }
+      }
+    }
+
+    return mainNode;
+  }
+}

scm-webapp/src/main/java/sonia/scm/web/i18n/I18nServlet.java

@@ -1,6 +1,7 @@
 package sonia.scm.web.i18n;
 
 
+import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.github.legman.Subscribe;
 import com.google.inject.Singleton;
@@ -13,7 +14,7 @@ import sonia.scm.cache.Cache;
 import sonia.scm.cache.CacheManager;
 import sonia.scm.filter.WebElement;
 import sonia.scm.plugin.PluginLoader;
-import sonia.scm.plugin.UberClassLoader;
+import sonia.scm.util.JacksonUtils;
 
 import javax.inject.Inject;
 import javax.servlet.http.HttpServlet;
@@ -22,11 +23,7 @@ import javax.servlet.http.HttpServletResponse;
 import java.io.IOException;
 import java.io.PrintWriter;
 import java.net.URL;
-import java.nio.file.Files;
-import java.nio.file.Paths;
 import java.util.Enumeration;
-import java.util.HashMap;
-import java.util.Map;
 import java.util.Optional;
 import java.util.function.BiConsumer;
 import java.util.function.Function;
@@ -45,29 +42,29 @@ public class I18nServlet extends HttpServlet {
   public static final String PATTERN = PATH + "/[a-z\\-A-Z]*/" + PLUGINS_JSON;
   public static final String CACHE_NAME = "sonia.cache.plugins.translations";
 
-  private final UberClassLoader uberClassLoader;
+  private final ClassLoader classLoader;
-  private final Cache<String, Map> cache;
+  private final Cache<String, JsonNode> cache;
   private static ObjectMapper objectMapper = new ObjectMapper();
 
 
   @Inject
   public I18nServlet(PluginLoader pluginLoader, CacheManager cacheManager) {
-    this.uberClassLoader = (UberClassLoader) pluginLoader.getUberClassLoader();
+    this.classLoader = pluginLoader.getUberClassLoader();
     this.cache = cacheManager.getCache(CACHE_NAME);
   }
 
   @Subscribe(async = false)
   public void handleRestartEvent(RestartEvent event) {
-    log.info("Clear cache on restart event with reason {}", event.getReason());
+    log.debug("Clear cache on restart event with reason {}", event.getReason());
     cache.clear();
   }
 
-  private Map getCollectedJson(String path,
+  private JsonNode getCollectedJson(String path,
-                               Function<String, Optional<Map>> jsonFileProvider,
+                                    Function<String, Optional<JsonNode>> jsonFileProvider,
-                               BiConsumer<String, Map> createdJsonFileConsumer) {
+                                    BiConsumer<String, JsonNode> createdJsonFileConsumer) {
     return Optional.ofNullable(jsonFileProvider.apply(path)
       .orElseGet(() -> {
-          Optional<Map> createdFile = collectJsonFile(path);
+          Optional<JsonNode> createdFile = collectJsonFile(path);
           createdFile.ifPresent(map -> createdJsonFileConsumer.accept(path, map));
           return createdFile.orElse(null);
         }
@@ -76,21 +73,20 @@ public class I18nServlet extends HttpServlet {
 
   @Override
   protected void doGet(HttpServletRequest req, HttpServletResponse response) {
-    try {
+    try (PrintWriter out = response.getWriter()) {
       response.setContentType("application/json");
-      PrintWriter out = response.getWriter();
       String path = req.getServletPath();
-      Function<String, Optional<Map>> jsonFileProvider = usedPath -> Optional.empty();
+      Function<String, Optional<JsonNode>> jsonFileProvider = usedPath -> Optional.empty();
-      BiConsumer<String, Map> createdJsonFileConsumer = (usedPath, foundJsonMap) -> log.info("A json File is created from the path {}", usedPath);
+      BiConsumer<String, JsonNode> createdJsonFileConsumer = (usedPath, jsonNode) -> log.debug("A json File is created from the path {}", usedPath);
       if (isProductionStage()) {
-        log.info("In Production Stage get the plugin translations from the cache");
+        log.debug("In Production Stage get the plugin translations from the cache");
         jsonFileProvider = usedPath -> Optional.ofNullable(
           cache.get(usedPath));
         createdJsonFileConsumer = createdJsonFileConsumer
-          .andThen((usedPath, map) -> log.info("Put the created json File in the cache with the key {}", usedPath))
+          .andThen((usedPath, jsonNode) -> log.debug("Put the created json File in the cache with the key {}", usedPath))
           .andThen(cache::put);
       }
-      out.write(objectMapper.writeValueAsString(getCollectedJson(path, jsonFileProvider, createdJsonFileConsumer)));
+      objectMapper.writeValue(out, getCollectedJson(path, jsonFileProvider, createdJsonFileConsumer));
     } catch (IOException e) {
       log.error("Error on getting the translation of the plugins", e);
       response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
@@ -105,27 +101,29 @@ public class I18nServlet extends HttpServlet {
   }
 
   /**
-   * Return a collected Json File as map from the given path from all plugins in the class path
+   * Return a collected Json File as JsonNode from the given path from all plugins in the class path
    *
    * @param path the searched resource path
-   * @return a collected Json File as map from the given path from all plugins in the class path
+   * @return a collected Json File as JsonNode from the given path from all plugins in the class path
    */
-  protected Optional<Map> collectJsonFile(String path) {
+  protected Optional<JsonNode> collectJsonFile(String path) {
-    log.info("Collect plugin translations from path {} for every plugin", path);
+    log.debug("Collect plugin translations from path {} for every plugin", path);
-    Map result = null;
+    JsonNode mergedJsonNode = null;
     try {
-      Enumeration<URL> resources = uberClassLoader.getResources(path.replaceFirst("/", ""));
+      Enumeration<URL> resources = classLoader.getResources(path.replaceFirst("/", ""));
-      if (resources.hasMoreElements()) {
-        result = new HashMap();
       while (resources.hasMoreElements()) {
         URL url = resources.nextElement();
-          result.putAll(objectMapper.readValue(Files.readAllBytes(Paths.get(url.getPath())), Map.class));
+        JsonNode jsonNode = objectMapper.readTree(url);
+        if (mergedJsonNode != null) {
+          JacksonUtils.merge(mergedJsonNode, jsonNode);
+        } else {
+          mergedJsonNode = jsonNode;
         }
       }
     } catch (IOException e) {
       log.error("Error on loading sources from {}", path, e);
       return Optional.empty();
     }
-    return Optional.ofNullable(result);
+    return Optional.ofNullable(mergedJsonNode);
   }
 }

scm-webapp/src/test/java/sonia/scm/web/i18n/I18nServletTest.java

@@ -1,17 +1,18 @@
 package sonia.scm.web.i18n;
 
+import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.github.sdorra.shiro.ShiroRule;
 import com.github.sdorra.shiro.SubjectAware;
 import com.google.common.base.Charsets;
 import com.google.common.io.Files;
+import org.apache.commons.lang3.StringUtils;
 import org.assertj.core.util.Lists;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 import org.junit.runner.RunWith;
-import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
 import org.mockito.MockSettings;
 import org.mockito.internal.creation.MockSettingsImpl;
@@ -22,23 +23,23 @@ import sonia.scm.cache.Cache;
 import sonia.scm.cache.CacheManager;
 import sonia.scm.event.ScmEventBus;
 import sonia.scm.plugin.PluginLoader;
-import sonia.scm.plugin.UberClassLoader;
+import sonia.scm.util.JacksonUtils;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import java.io.File;
+import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.PrintWriter;
 import java.net.URL;
+import java.nio.charset.Charset;
 import java.util.Collections;
 import java.util.Enumeration;
-import java.util.Map;
 import java.util.Optional;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
-import static org.mockito.Mockito.doCallRealMethod;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
@@ -60,7 +61,7 @@ public class I18nServletTest {
     "      \"replace\" : \"Push\"\n" +
     "    }\n" +
     "  }\n" +
-    "}\n";
+    "}";
   private static final String HG_PLUGIN_JSON = "{\n" +
     "  \"scm-hg-plugin\": {\n" +
     "    \"information\": {\n" +
@@ -69,14 +70,14 @@ public class I18nServletTest {
     "      \"replace\" : \"Push\"\n" +
     "    }\n" +
     "  }\n" +
-    "}\n";
+    "}";
   private static String SVN_PLUGIN_JSON = "{\n" +
     "  \"scm-svn-plugin\": {\n" +
     "    \"information\": {\n" +
     "      \"checkout\" : \"Checkout\"\n" +
     "    }\n" +
     "  }\n" +
-    "}\n";
+    "}";
 
   @Rule
   public TemporaryFolder temporaryFolder = new TemporaryFolder();
@@ -88,7 +89,7 @@ public class I18nServletTest {
   CacheManager cacheManager;
 
   @Mock
-  UberClassLoader uberClassLoader;
+  ClassLoader classLoader;
 
   I18nServlet servlet;
 
@@ -104,7 +105,7 @@ public class I18nServletTest {
       createFileFromString(GIT_PLUGIN_JSON).toURL(),
       createFileFromString(HG_PLUGIN_JSON).toURL()
     ));
-    when(pluginLoader.getUberClassLoader()).thenReturn(uberClassLoader);
+    when(pluginLoader.getUberClassLoader()).thenReturn(classLoader);
     when(cacheManager.getCache(I18nServlet.CACHE_NAME)).thenReturn(cache);
     MockSettings settings = new MockSettingsImpl<>();
     settings.useConstructor(pluginLoader, cacheManager);
@@ -130,7 +131,7 @@ public class I18nServletTest {
     PrintWriter writer = mock(PrintWriter.class);
     when(response.getWriter()).thenReturn(writer);
     when(request.getServletPath()).thenReturn(path);
-    when(uberClassLoader.getResources("locales/de/plugins.json")).thenThrow(IOException.class);
+    when(classLoader.getResources("locales/de/plugins.json")).thenThrow(IOException.class);
 
     servlet.doGet(request, response);
 
@@ -140,14 +141,9 @@ public class I18nServletTest {
   @Test
   @SuppressWarnings("unchecked")
   public void shouldFailWith500OnIOException() throws IOException {
-    String path = "/locales/de/plugins.json";
     HttpServletRequest request = mock(HttpServletRequest.class);
     HttpServletResponse response = mock(HttpServletResponse.class);
-    PrintWriter writer = mock(PrintWriter.class);
+    doThrow(IOException.class).when(response).getWriter();
-    when(response.getWriter()).thenReturn(writer);
-    when(request.getServletPath()).thenReturn(path);
-    when(uberClassLoader.getResources("locales/de/plugins.json")).thenReturn(resources);
-    doThrow(IOException.class).when(writer).write(any(String.class));
 
     servlet.doGet(request, response);
 
@@ -161,16 +157,16 @@ public class I18nServletTest {
     when(servlet.isProductionStage()).thenReturn(false);
     HttpServletRequest request = mock(HttpServletRequest.class);
     HttpServletResponse response = mock(HttpServletResponse.class);
-    PrintWriter writer = mock(PrintWriter.class);
+    File file = temporaryFolder.newFile();
+    PrintWriter writer = new PrintWriter(new FileOutputStream(file));
     when(response.getWriter()).thenReturn(writer);
     when(request.getServletPath()).thenReturn(path);
-    when(uberClassLoader.getResources("locales/de/plugins.json")).thenReturn(resources);
+    when(classLoader.getResources("locales/de/plugins.json")).thenReturn(resources);
-    ArgumentCaptor<String> captor = ArgumentCaptor.forClass(String.class);
-    doCallRealMethod().when(writer).write(captor.capture());
 
     servlet.doGet(request, response);
 
-    assertJsonMap(jsonStringToMap(captor.getValue()));
+    String json = Files.readLines(file, Charset.defaultCharset()).get(0);
+    assertJson(json);
     verify(cache, never()).get(any());
   }
 
@@ -181,16 +177,16 @@ public class I18nServletTest {
     when(servlet.isProductionStage()).thenReturn(true);
     HttpServletRequest request = mock(HttpServletRequest.class);
     HttpServletResponse response = mock(HttpServletResponse.class);
-    PrintWriter writer = mock(PrintWriter.class);
+    File file = temporaryFolder.newFile();
+    PrintWriter writer = new PrintWriter(new FileOutputStream(file));
     when(response.getWriter()).thenReturn(writer);
     when(request.getServletPath()).thenReturn(path);
-    when(uberClassLoader.getResources("locales/de/plugins.json")).thenReturn(resources);
+    when(classLoader.getResources("locales/de/plugins.json")).thenReturn(resources);
-    ArgumentCaptor<String> captor = ArgumentCaptor.forClass(String.class);
-    doCallRealMethod().when(writer).write(captor.capture());
 
     servlet.doGet(request, response);
 
-    assertJsonMap(jsonStringToMap(captor.getValue()));
+    String json = Files.readLines(file, Charset.defaultCharset()).get(0);
+    assertJson(json);
     verify(cache).get(path);
     verify(cache).put(eq(path), any());
   }
@@ -202,41 +198,47 @@ public class I18nServletTest {
     when(servlet.isProductionStage()).thenReturn(true);
     HttpServletRequest request = mock(HttpServletRequest.class);
     HttpServletResponse response = mock(HttpServletResponse.class);
-    PrintWriter writer = mock(PrintWriter.class);
+    File file = temporaryFolder.newFile();
+    PrintWriter writer = new PrintWriter(new FileOutputStream(file));
     when(response.getWriter()).thenReturn(writer);
     when(request.getServletPath()).thenReturn(path);
-    when(uberClassLoader.getResources("locales/de/plugins.json")).thenReturn(resources);
+    when(classLoader.getResources("locales/de/plugins.json")).thenReturn(resources);
-    ArgumentCaptor<String> captor = ArgumentCaptor.forClass(String.class);
+    ObjectMapper objectMapper = new ObjectMapper();
-    doCallRealMethod().when(writer).write(captor.capture());
+    JsonNode node = objectMapper.readTree(GIT_PLUGIN_JSON);
-    Map cachedMap = jsonStringToMap(GIT_PLUGIN_JSON);
+    node = JacksonUtils.merge(node, objectMapper.readTree(HG_PLUGIN_JSON));
-    cachedMap.putAll(jsonStringToMap(HG_PLUGIN_JSON));
+    node = JacksonUtils.merge(node, objectMapper.readTree(SVN_PLUGIN_JSON));
-    cachedMap.putAll(jsonStringToMap(SVN_PLUGIN_JSON));
+    when(cache.get(path)).thenReturn(node);
-    when(cache.get(path)).thenReturn(cachedMap);
+
     servlet.doGet(request, response);
+
+    String json = Files.readLines(file, Charset.defaultCharset()).get(0);
     verify(servlet, never()).collectJsonFile(path);
     verify(cache, never()).put(eq(path), any());
     verify(cache).get(path);
-    assertJsonMap(jsonStringToMap(captor.getValue()));
+    assertJson(json);
   }
 
   @Test
   @SuppressWarnings("unchecked")
   public void shouldCollectJsonFile() throws IOException {
     String path = "locales/de/plugins.json";
+    when(classLoader.getResources(path)).thenReturn(resources);
 
-    when(uberClassLoader.getResources(path)).thenReturn(resources);
+    Optional<JsonNode> jsonNodeOptional = servlet.collectJsonFile("/" + path);
-    Optional<Map> mapOptional = servlet.collectJsonFile("/" + path);
 
-    assertJsonMap(mapOptional.orElse(null));
+    assertJson(jsonNodeOptional.orElse(null));
   }
 
-  @SuppressWarnings("unchecked")
+  public void assertJson(JsonNode actual) throws IOException {
-  public void assertJsonMap(Map actual) throws IOException {
+    assertJson(actual.toString());
+  }
+
+  public void assertJson(String actual) throws IOException {
     assertThat(actual)
       .isNotEmpty()
-      .containsAllEntriesOf(jsonStringToMap(GIT_PLUGIN_JSON))
+      .contains(StringUtils.deleteWhitespace(GIT_PLUGIN_JSON.substring(1, GIT_PLUGIN_JSON.length() - 1)))
-      .containsAllEntriesOf(jsonStringToMap(HG_PLUGIN_JSON))
+      .contains(StringUtils.deleteWhitespace(HG_PLUGIN_JSON.substring(1, HG_PLUGIN_JSON.length() - 1)))
-      .containsAllEntriesOf(jsonStringToMap(SVN_PLUGIN_JSON));
+      .contains(StringUtils.deleteWhitespace(SVN_PLUGIN_JSON.substring(1, SVN_PLUGIN_JSON.length() - 1)));
   }
 
   public File createFileFromString(String json) throws IOException {
@@ -245,9 +247,4 @@ public class I18nServletTest {
     return file;
   }
 
-  private Map jsonStringToMap(String fileAsString) throws IOException {
-    ObjectMapper mapper = new ObjectMapper();
-    return mapper.readValue(fileAsString, Map.class);
-  }
-
 }