From a7d14636dcd7115aeec31a03f4fe32c6edf9767c Mon Sep 17 00:00:00 2001 From: Mohamed Karray Date: Mon, 22 Oct 2018 17:37:10 +0200 Subject: [PATCH] use JsonNode in the I18nServlet --- .../java/sonia/scm/util/JacksonUtils.java | 49 ++++++++++ .../java/sonia/scm/web/i18n/I18nServlet.java | 62 ++++++------- .../sonia/scm/web/i18n/I18nServletTest.java | 93 +++++++++---------- 3 files changed, 124 insertions(+), 80 deletions(-) create mode 100644 scm-webapp/src/main/java/sonia/scm/util/JacksonUtils.java diff --git a/scm-webapp/src/main/java/sonia/scm/util/JacksonUtils.java b/scm-webapp/src/main/java/sonia/scm/util/JacksonUtils.java new file mode 100644 index 0000000000..3847b1fb55 --- /dev/null +++ b/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 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; + } +} diff --git a/scm-webapp/src/main/java/sonia/scm/web/i18n/I18nServlet.java b/scm-webapp/src/main/java/sonia/scm/web/i18n/I18nServlet.java index d0c78bf1f5..16e8c974e7 100644 --- a/scm-webapp/src/main/java/sonia/scm/web/i18n/I18nServlet.java +++ b/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 Cache cache; + private final ClassLoader classLoader; + private final Cache 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, - Function> jsonFileProvider, - BiConsumer createdJsonFileConsumer) { + private JsonNode getCollectedJson(String path, + Function> jsonFileProvider, + BiConsumer createdJsonFileConsumer) { return Optional.ofNullable(jsonFileProvider.apply(path) .orElseGet(() -> { - Optional createdFile = collectJsonFile(path); + Optional 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> jsonFileProvider = usedPath -> Optional.empty(); - BiConsumer createdJsonFileConsumer = (usedPath, foundJsonMap) -> log.info("A json File is created from the path {}", usedPath); + Function> jsonFileProvider = usedPath -> Optional.empty(); + BiConsumer 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 collectJsonFile(String path) { - log.info("Collect plugin translations from path {} for every plugin", path); - Map result = null; + protected Optional collectJsonFile(String path) { + log.debug("Collect plugin translations from path {} for every plugin", path); + JsonNode mergedJsonNode = null; try { - Enumeration resources = uberClassLoader.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)); + Enumeration resources = classLoader.getResources(path.replaceFirst("/", "")); + while (resources.hasMoreElements()) { + URL url = resources.nextElement(); + 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); } } diff --git a/scm-webapp/src/test/java/sonia/scm/web/i18n/I18nServletTest.java b/scm-webapp/src/test/java/sonia/scm/web/i18n/I18nServletTest.java index b8deb4b069..243a9bd6ed 100644 --- a/scm-webapp/src/test/java/sonia/scm/web/i18n/I18nServletTest.java +++ b/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); - 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)); + doThrow(IOException.class).when(response).getWriter(); 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); - ArgumentCaptor captor = ArgumentCaptor.forClass(String.class); - doCallRealMethod().when(writer).write(captor.capture()); + when(classLoader.getResources("locales/de/plugins.json")).thenReturn(resources); 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); - ArgumentCaptor captor = ArgumentCaptor.forClass(String.class); - doCallRealMethod().when(writer).write(captor.capture()); + when(classLoader.getResources("locales/de/plugins.json")).thenReturn(resources); 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); - ArgumentCaptor captor = ArgumentCaptor.forClass(String.class); - doCallRealMethod().when(writer).write(captor.capture()); - Map cachedMap = jsonStringToMap(GIT_PLUGIN_JSON); - cachedMap.putAll(jsonStringToMap(HG_PLUGIN_JSON)); - cachedMap.putAll(jsonStringToMap(SVN_PLUGIN_JSON)); - when(cache.get(path)).thenReturn(cachedMap); + when(classLoader.getResources("locales/de/plugins.json")).thenReturn(resources); + ObjectMapper objectMapper = new ObjectMapper(); + JsonNode node = objectMapper.readTree(GIT_PLUGIN_JSON); + node = JacksonUtils.merge(node, objectMapper.readTree(HG_PLUGIN_JSON)); + node = JacksonUtils.merge(node, objectMapper.readTree(SVN_PLUGIN_JSON)); + when(cache.get(path)).thenReturn(node); + 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 mapOptional = servlet.collectJsonFile("/" + path); + Optional jsonNodeOptional = servlet.collectJsonFile("/" + path); - assertJsonMap(mapOptional.orElse(null)); + assertJson(jsonNodeOptional.orElse(null)); } - @SuppressWarnings("unchecked") - public void assertJsonMap(Map actual) throws IOException { + public void assertJson(JsonNode actual) throws IOException { + assertJson(actual.toString()); + } + + public void assertJson(String actual) throws IOException { assertThat(actual) .isNotEmpty() - .containsAllEntriesOf(jsonStringToMap(GIT_PLUGIN_JSON)) - .containsAllEntriesOf(jsonStringToMap(HG_PLUGIN_JSON)) - .containsAllEntriesOf(jsonStringToMap(SVN_PLUGIN_JSON)); + .contains(StringUtils.deleteWhitespace(GIT_PLUGIN_JSON.substring(1, GIT_PLUGIN_JSON.length() - 1))) + .contains(StringUtils.deleteWhitespace(HG_PLUGIN_JSON.substring(1, HG_PLUGIN_JSON.length() - 1))) + .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); - } - }