From de0bcaf3e8b2422932e25c6c4d21ab4a8aa3e1b3 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Tue, 1 Sep 2020 07:21:06 +0200 Subject: [PATCH 1/4] use static instance of slf4j instead of lombok annotation --- .../java/sonia/scm/web/i18n/I18nServlet.java | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) 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 2748153b7d..903250c68b 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 @@ -21,7 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.web.i18n; @@ -31,14 +31,15 @@ import com.fasterxml.jackson.databind.node.ObjectNode; import com.github.legman.Subscribe; import com.google.common.annotations.VisibleForTesting; import com.google.inject.Singleton; -import lombok.extern.slf4j.Slf4j; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import sonia.scm.NotFoundException; import sonia.scm.SCMContext; import sonia.scm.Stage; -import sonia.scm.lifecycle.RestartEvent; import sonia.scm.cache.Cache; import sonia.scm.cache.CacheManager; import sonia.scm.filter.WebElement; +import sonia.scm.lifecycle.RestartEvent; import sonia.scm.plugin.PluginLoader; import javax.inject.Inject; @@ -63,16 +64,17 @@ import static sonia.scm.NotFoundException.notFound; */ @Singleton @WebElement(value = I18nServlet.PATTERN, regex = true) -@Slf4j public class I18nServlet extends HttpServlet { + private static final Logger LOG = LoggerFactory.getLogger(I18nServlet.class); + public static final String PLUGINS_JSON = "plugins.json"; public static final String PATTERN = "/locales/[a-z\\-A-Z]*/" + PLUGINS_JSON; public static final String CACHE_NAME = "sonia.cache.plugins.translations"; private final ClassLoader classLoader; private final Cache cache; - private static ObjectMapper objectMapper = new ObjectMapper(); + private final ObjectMapper objectMapper = new ObjectMapper(); @Inject @@ -83,7 +85,7 @@ public class I18nServlet extends HttpServlet { @Subscribe(async = false) public void handleRestartEvent(RestartEvent event) { - log.debug("Clear cache on restart event with reason {}", event.getReason()); + LOG.debug("Clear cache on restart event with reason {}", event.getReason()); cache.clear(); } @@ -108,21 +110,20 @@ public class I18nServlet extends HttpServlet { try (PrintWriter out = response.getWriter()) { String path = req.getServletPath(); Function> jsonFileProvider = usedPath -> Optional.empty(); - BiConsumer createdJsonFileConsumer = (usedPath, jsonNode) -> log.debug("A json File is created from the path {}", usedPath); + BiConsumer createdJsonFileConsumer = (usedPath, jsonNode) -> LOG.debug("A json File is created from the path {}", usedPath); if (isProductionStage()) { - log.debug("In Production Stage get the plugin translations from the cache"); - jsonFileProvider = usedPath -> Optional.ofNullable( - cache.get(usedPath)); + LOG.debug("In Production Stage get the plugin translations from the cache"); + jsonFileProvider = usedPath -> Optional.ofNullable(cache.get(usedPath)); createdJsonFileConsumer = createdJsonFileConsumer - .andThen((usedPath, jsonNode) -> log.debug("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); } objectMapper.writeValue(out, getCollectedJson(path, jsonFileProvider, createdJsonFileConsumer)); } catch (IOException e) { - log.error("Error on getting the translation of the plugins", e); + LOG.error("Error on getting the translation of the plugins", e); response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); } catch (NotFoundException e) { - log.error("Plugin translations are not found", e); + LOG.error("Plugin translations are not found", e); response.setStatus(HttpServletResponse.SC_NOT_FOUND); } } @@ -140,7 +141,7 @@ public class I18nServlet extends HttpServlet { */ @VisibleForTesting protected Optional collectJsonFile(String path) { - log.debug("Collect plugin translations from path {} for every plugin", path); + LOG.debug("Collect plugin translations from path {} for every plugin", path); JsonNode mergedJsonNode = null; try { Enumeration resources = classLoader.getResources(path.replaceFirst("/", "")); @@ -154,7 +155,7 @@ public class I18nServlet extends HttpServlet { } } } catch (IOException e) { - log.error("Error on loading sources from {}", path, e); + LOG.error("Error on loading sources from {}", path, e); return Optional.empty(); } return Optional.ofNullable(mergedJsonNode); From 8d65bf75f3fc3ce3ce32317205c8d3f7356280fe Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Tue, 1 Sep 2020 07:53:21 +0200 Subject: [PATCH 2/4] refactor I18nServlet to avoid stacktrace logging on unknown language --- .../java/sonia/scm/web/i18n/I18nServlet.java | 108 +++++++++--------- .../sonia/scm/web/i18n/I18nServletTest.java | 26 ++--- 2 files changed, 65 insertions(+), 69 deletions(-) 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 903250c68b..4671915a9d 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 @@ -33,7 +33,6 @@ import com.google.common.annotations.VisibleForTesting; import com.google.inject.Singleton; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import sonia.scm.NotFoundException; import sonia.scm.SCMContext; import sonia.scm.Stage; import sonia.scm.cache.Cache; @@ -52,11 +51,6 @@ import java.net.URL; import java.util.Enumeration; import java.util.Iterator; import java.util.Optional; -import java.util.function.BiConsumer; -import java.util.function.Function; - -import static sonia.scm.ContextEntry.ContextBuilder.entity; -import static sonia.scm.NotFoundException.notFound; /** @@ -89,45 +83,54 @@ public class I18nServlet extends HttpServlet { cache.clear(); } - private JsonNode getCollectedJson(String path, - Function> jsonFileProvider, - BiConsumer createdJsonFileConsumer) { - return Optional.ofNullable(jsonFileProvider.apply(path) - .orElseGet(() -> { - Optional createdFile = collectJsonFile(path); - createdFile.ifPresent(map -> createdJsonFileConsumer.accept(path, map)); - return createdFile.orElse(null); - } - )).orElseThrow(() -> notFound(entity("jsonprovider", path))); - } - @VisibleForTesting @Override - protected void doGet(HttpServletRequest req, HttpServletResponse response) { + protected void doGet(HttpServletRequest request, HttpServletResponse response) { + String path = request.getServletPath(); + try { + Optional json = findJson(path); + if (json.isPresent()) { + write(response, json.get()); + } else { + LOG.debug("could not find translation at {}", path); + response.setStatus(HttpServletResponse.SC_NOT_FOUND); + } + } catch (IOException ex) { + LOG.error("Error on getting the translation of the plugins", ex); + response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + } + } + + private void write(HttpServletResponse response, JsonNode jsonNode) throws IOException { response.setCharacterEncoding("UTF-8"); response.setContentType("application/json"); response.setHeader("Cache-Control", "no-cache"); - try (PrintWriter out = response.getWriter()) { - String path = req.getServletPath(); - Function> jsonFileProvider = usedPath -> Optional.empty(); - BiConsumer createdJsonFileConsumer = (usedPath, jsonNode) -> LOG.debug("A json File is created from the path {}", usedPath); - if (isProductionStage()) { - LOG.debug("In Production Stage get the plugin translations from the cache"); - jsonFileProvider = usedPath -> Optional.ofNullable(cache.get(usedPath)); - createdJsonFileConsumer = createdJsonFileConsumer - .andThen((usedPath, jsonNode) -> LOG.debug("Put the created json File in the cache with the key {}", usedPath)) - .andThen(cache::put); - } - 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); - } catch (NotFoundException e) { - LOG.error("Plugin translations are not found", e); - response.setStatus(HttpServletResponse.SC_NOT_FOUND); + + try (PrintWriter writer = response.getWriter()) { + objectMapper.writeValue(writer, jsonNode); } } + public Optional findJson(String path) throws IOException { + if (isProductionStage()) { + return findJsonCached(path); + } + return collectJsonFile(path); + } + + private Optional findJsonCached(String path) throws IOException { + JsonNode jsonNode = cache.get(path); + if (jsonNode != null) { + LOG.debug("return json node from cache for path {}", path); + return Optional.of(jsonNode); + } + + LOG.debug("collect json for path {}", path); + Optional collected = collectJsonFile(path); + collected.ifPresent(node -> cache.put(path, node)); + return collected; + } + @VisibleForTesting protected boolean isProductionStage() { return SCMContext.getContext().getStage() == Stage.PRODUCTION; @@ -140,46 +143,39 @@ public class I18nServlet extends HttpServlet { * @return a collected Json File as JsonNode from the given path from all plugins in the class path */ @VisibleForTesting - protected Optional collectJsonFile(String path) { + protected Optional collectJsonFile(String path) throws IOException { LOG.debug("Collect plugin translations from path {} for every plugin", path); JsonNode mergedJsonNode = null; - try { - Enumeration resources = classLoader.getResources(path.replaceFirst("/", "")); - while (resources.hasMoreElements()) { - URL url = resources.nextElement(); - JsonNode jsonNode = objectMapper.readTree(url); - if (mergedJsonNode != null) { - merge(mergedJsonNode, jsonNode); - } else { - mergedJsonNode = jsonNode; - } + Enumeration resources = classLoader.getResources(path.replaceFirst("/", "")); + while (resources.hasMoreElements()) { + URL url = resources.nextElement(); + JsonNode jsonNode = objectMapper.readTree(url); + if (mergedJsonNode != null) { + merge(mergedJsonNode, jsonNode); + } else { + mergedJsonNode = jsonNode; } - } catch (IOException e) { - LOG.error("Error on loading sources from {}", path, e); - return Optional.empty(); } + return Optional.ofNullable(mergedJsonNode); } /** * Merge the updateNode into the mainNode and return it. - * + *

* This is not a deep merge. * - * @param mainNode the main node + * @param mainNode the main node * @param updateNode the update node * @return the merged mainNode */ @VisibleForTesting protected 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) { mergeNode(updateNode, fieldName, jsonNode); } else { 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 4770a54d91..32c3fcc772 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 @@ -21,7 +21,7 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.web.i18n; import com.fasterxml.jackson.databind.JsonNode; @@ -40,7 +40,6 @@ import org.mockito.MockSettings; import org.mockito.internal.creation.MockSettingsImpl; import org.mockito.invocation.InvocationOnMock; import org.mockito.junit.MockitoJUnitRunner; -import sonia.scm.lifecycle.RestartEvent; import sonia.scm.cache.Cache; import sonia.scm.cache.CacheManager; import sonia.scm.event.ScmEventBus; @@ -87,7 +86,7 @@ public class I18nServletTest { "}", "}" ); - private static String SVN_PLUGIN_JSON = json( + private static final String SVN_PLUGIN_JSON = json( "{", "'scm-svn-plugin': {", "'information': {", @@ -116,11 +115,11 @@ public class I18nServletTest { private I18nServlet servlet; @Mock - private Cache cache; + private Cache cache; + private Enumeration resources; @Before - @SuppressWarnings("unchecked") public void init() throws IOException { resources = Collections.enumeration(Lists.newArrayList( createFileFromString(SVN_PLUGIN_JSON).toURI().toURL(), @@ -128,7 +127,7 @@ public class I18nServletTest { createFileFromString(HG_PLUGIN_JSON).toURI().toURL() )); when(pluginLoader.getUberClassLoader()).thenReturn(classLoader); - when(cacheManager.getCache(I18nServlet.CACHE_NAME)).thenReturn(cache); + when(cacheManager.getCache(I18nServlet.CACHE_NAME)).thenReturn(cache); MockSettings settings = new MockSettingsImpl<>(); settings.useConstructor(pluginLoader, cacheManager); settings.defaultAnswer(InvocationOnMock::callRealMethod); @@ -145,7 +144,6 @@ public class I18nServletTest { } @Test - @SuppressWarnings("unchecked") public void shouldFailWith404OnMissingResources() throws IOException { String path = "/locales/de/plugins.json"; HttpServletRequest request = mock(HttpServletRequest.class); @@ -153,7 +151,9 @@ public class I18nServletTest { PrintWriter writer = mock(PrintWriter.class); when(response.getWriter()).thenReturn(writer); when(request.getServletPath()).thenReturn(path); - when(classLoader.getResources("locales/de/plugins.json")).thenThrow(IOException.class); + when(classLoader.getResources("locales/de/plugins.json")).thenReturn( + I18nServlet.class.getClassLoader().getResources("something/not/available") + ); servlet.doGet(request, response); @@ -161,9 +161,10 @@ public class I18nServletTest { } @Test - @SuppressWarnings("unchecked") public void shouldFailWith500OnIOException() throws IOException { + when(servlet.isProductionStage()).thenReturn(false); HttpServletRequest request = mock(HttpServletRequest.class); + when(request.getServletPath()).thenReturn("/locales/de/plugins.json"); HttpServletResponse response = mock(HttpServletResponse.class); doThrow(IOException.class).when(response).getWriter(); @@ -173,7 +174,7 @@ public class I18nServletTest { } @Test - @SuppressWarnings("unchecked") + @SuppressWarnings("UnstableApiUsage") public void inDevelopmentStageShouldNotUseCache() throws IOException { String path = "/locales/de/plugins.json"; when(servlet.isProductionStage()).thenReturn(false); @@ -193,7 +194,7 @@ public class I18nServletTest { } @Test - @SuppressWarnings("unchecked") + @SuppressWarnings("UnstableApiUsage") public void inProductionStageShouldUseCache() throws IOException { String path = "/locales/de/plugins.json"; when(servlet.isProductionStage()).thenReturn(true); @@ -216,7 +217,7 @@ public class I18nServletTest { } @Test - @SuppressWarnings("unchecked") + @SuppressWarnings("UnstableApiUsage") public void inProductionStageShouldGetFromCache() throws IOException { String path = "/locales/de/plugins.json"; when(servlet.isProductionStage()).thenReturn(true); @@ -245,7 +246,6 @@ public class I18nServletTest { } @Test - @SuppressWarnings("unchecked") public void shouldCollectJsonFile() throws IOException { String path = "locales/de/plugins.json"; when(classLoader.getResources(path)).thenReturn(resources); From dcfd0d0d163bced6a90ed74a56d4840c6b3acff0 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Tue, 1 Sep 2020 08:34:01 +0200 Subject: [PATCH 3/4] refactor I18nServletTest Use JUnit5 and only api methods --- .../java/sonia/scm/web/i18n/I18nServlet.java | 29 +-- .../sonia/scm/web/i18n/I18nServletTest.java | 203 ++++++++---------- 2 files changed, 95 insertions(+), 137 deletions(-) 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 4671915a9d..11ec8b5978 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 @@ -33,7 +33,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.inject.Singleton; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import sonia.scm.SCMContext; +import sonia.scm.SCMContextProvider; import sonia.scm.Stage; import sonia.scm.cache.Cache; import sonia.scm.cache.CacheManager; @@ -66,13 +66,15 @@ public class I18nServlet extends HttpServlet { public static final String PATTERN = "/locales/[a-z\\-A-Z]*/" + PLUGINS_JSON; public static final String CACHE_NAME = "sonia.cache.plugins.translations"; + private final SCMContextProvider context; private final ClassLoader classLoader; private final Cache cache; private final ObjectMapper objectMapper = new ObjectMapper(); @Inject - public I18nServlet(PluginLoader pluginLoader, CacheManager cacheManager) { + public I18nServlet(SCMContextProvider context, PluginLoader pluginLoader, CacheManager cacheManager) { + this.context = context; this.classLoader = pluginLoader.getUberClassLoader(); this.cache = cacheManager.getCache(CACHE_NAME); } @@ -133,17 +135,11 @@ public class I18nServlet extends HttpServlet { @VisibleForTesting protected boolean isProductionStage() { - return SCMContext.getContext().getStage() == Stage.PRODUCTION; + return context.getStage() == Stage.PRODUCTION; } - /** - * 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 JsonNode from the given path from all plugins in the class path - */ @VisibleForTesting - protected Optional collectJsonFile(String path) throws IOException { + private Optional collectJsonFile(String path) throws IOException { LOG.debug("Collect plugin translations from path {} for every plugin", path); JsonNode mergedJsonNode = null; Enumeration resources = classLoader.getResources(path.replaceFirst("/", "")); @@ -160,18 +156,7 @@ public class I18nServlet extends HttpServlet { return Optional.ofNullable(mergedJsonNode); } - - /** - * Merge the updateNode into the mainNode and return it. - *

- * This is not a deep merge. - * - * @param mainNode the main node - * @param updateNode the update node - * @return the merged mainNode - */ - @VisibleForTesting - protected JsonNode merge(JsonNode mainNode, JsonNode updateNode) { + private JsonNode merge(JsonNode mainNode, JsonNode updateNode) { Iterator fieldNames = updateNode.fieldNames(); while (fieldNames.hasNext()) { String fieldName = fieldNames.next(); 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 32c3fcc772..495c233906 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 @@ -26,43 +26,38 @@ package sonia.scm.web.i18n; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; -import com.google.common.base.Charsets; -import com.google.common.io.Files; +import com.github.legman.EventBus; 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.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.api.io.TempDir; import org.mockito.Mock; -import org.mockito.MockSettings; -import org.mockito.internal.creation.MockSettingsImpl; -import org.mockito.invocation.InvocationOnMock; -import org.mockito.junit.MockitoJUnitRunner; +import org.mockito.junit.jupiter.MockitoExtension; +import sonia.scm.SCMContextProvider; +import sonia.scm.Stage; import sonia.scm.cache.Cache; import sonia.scm.cache.CacheManager; -import sonia.scm.event.ScmEventBus; import sonia.scm.lifecycle.RestartEventFactory; import sonia.scm.plugin.PluginLoader; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import java.io.File; -import java.io.FileOutputStream; +import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.PrintWriter; import java.net.URL; -import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; +import java.nio.file.Path; +import java.util.Arrays; import java.util.Collections; import java.util.Enumeration; -import java.util.Optional; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.*; -@RunWith(MockitoJUnitRunner.Silent.class) -public class I18nServletTest { +@ExtendWith(MockitoExtension.class) +class I18nServletTest { private static final String GIT_PLUGIN_JSON = json( "{", @@ -75,6 +70,7 @@ public class I18nServletTest { "}", "}" ); + private static final String HG_PLUGIN_JSON = json( "{", "'scm-hg-plugin': {", @@ -86,6 +82,7 @@ public class I18nServletTest { "}", "}" ); + private static final String SVN_PLUGIN_JSON = json( "{", "'scm-svn-plugin': {", @@ -100,56 +97,45 @@ public class I18nServletTest { return String.join("\n", parts ).replaceAll("'", "\""); } - @Rule - public TemporaryFolder temporaryFolder = new TemporaryFolder(); + @Mock + private SCMContextProvider context; @Mock private PluginLoader pluginLoader; - @Mock - private CacheManager cacheManager; - @Mock private ClassLoader classLoader; - private I18nServlet servlet; + @Mock + private CacheManager cacheManager; @Mock private Cache cache; - private Enumeration resources; + private I18nServlet servlet; - @Before - public void init() throws IOException { - resources = Collections.enumeration(Lists.newArrayList( - createFileFromString(SVN_PLUGIN_JSON).toURI().toURL(), - createFileFromString(GIT_PLUGIN_JSON).toURI().toURL(), - createFileFromString(HG_PLUGIN_JSON).toURI().toURL() - )); + + @BeforeEach + void init() { when(pluginLoader.getUberClassLoader()).thenReturn(classLoader); when(cacheManager.getCache(I18nServlet.CACHE_NAME)).thenReturn(cache); - MockSettings settings = new MockSettingsImpl<>(); - settings.useConstructor(pluginLoader, cacheManager); - settings.defaultAnswer(InvocationOnMock::callRealMethod); - servlet = mock(I18nServlet.class, settings); + servlet = new I18nServlet(context, pluginLoader, cacheManager); } @Test - public void shouldCleanCacheOnRestartEvent() { - ScmEventBus.getInstance().register(servlet); - - ScmEventBus.getInstance().post(RestartEventFactory.create(I18nServlet.class, "Restart to reload the plugin resources")); + void shouldCleanCacheOnRestartEvent() { + EventBus eventBus = new EventBus("forTestingOnly"); + eventBus.register(servlet); + eventBus.post(RestartEventFactory.create(I18nServlet.class, "Restart to reload the plugin resources")); verify(cache).clear(); } @Test - public void shouldFailWith404OnMissingResources() throws IOException { + void shouldFailWith404OnMissingResources() 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(classLoader.getResources("locales/de/plugins.json")).thenReturn( I18nServlet.class.getClassLoader().getResources("something/not/available") @@ -161,98 +147,96 @@ public class I18nServletTest { } @Test - public void shouldFailWith500OnIOException() throws IOException { - when(servlet.isProductionStage()).thenReturn(false); + void shouldFailWith500OnIOException() throws IOException { + stage(Stage.DEVELOPMENT); HttpServletRequest request = mock(HttpServletRequest.class); when(request.getServletPath()).thenReturn("/locales/de/plugins.json"); HttpServletResponse response = mock(HttpServletResponse.class); - doThrow(IOException.class).when(response).getWriter(); + when(classLoader.getResources("locales/de/plugins.json")).thenThrow(new IOException("failed")); servlet.doGet(request, response); verify(response).setStatus(500); } + private void stage(Stage stage) { + when(context.getStage()).thenReturn(stage); + } + @Test - @SuppressWarnings("UnstableApiUsage") - public void inDevelopmentStageShouldNotUseCache() throws IOException { - String path = "/locales/de/plugins.json"; - when(servlet.isProductionStage()).thenReturn(false); + void inDevelopmentStageShouldNotUseCache(@TempDir Path temp) throws IOException { + stage(Stage.DEVELOPMENT); + mockResources(temp, "locales/de/plugins.json"); HttpServletRequest request = mock(HttpServletRequest.class); + when(request.getServletPath()).thenReturn("/locales/de/plugins.json"); + HttpServletResponse response = mock(HttpServletResponse.class); - File file = temporaryFolder.newFile(); - PrintWriter writer = new PrintWriter(new FileOutputStream(file)); - when(response.getWriter()).thenReturn(writer); - when(request.getServletPath()).thenReturn(path); - when(classLoader.getResources("locales/de/plugins.json")).thenReturn(resources); + String json = doGetString(request, response); - servlet.doGet(request, response); - - String json = Files.readLines(file, Charset.defaultCharset()).get(0); assertJson(json); verify(cache, never()).get(any()); } - @Test - @SuppressWarnings("UnstableApiUsage") - public void inProductionStageShouldUseCache() throws IOException { - String path = "/locales/de/plugins.json"; - when(servlet.isProductionStage()).thenReturn(true); - HttpServletRequest request = mock(HttpServletRequest.class); - HttpServletResponse response = mock(HttpServletResponse.class); - File file = temporaryFolder.newFile(); - PrintWriter writer = new PrintWriter(new FileOutputStream(file)); + private String doGetString(HttpServletRequest request, HttpServletResponse response) throws IOException { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + PrintWriter writer = new PrintWriter(baos); when(response.getWriter()).thenReturn(writer); - when(request.getServletPath()).thenReturn(path); - when(classLoader.getResources("locales/de/plugins.json")).thenReturn(resources); servlet.doGet(request, response); - String json = Files.readLines(file, Charset.defaultCharset()).get(0); - assertJson(json); - verify(cache).get(path); - verify(cache).put(eq(path), any()); + writer.flush(); + return baos.toString(StandardCharsets.UTF_8.name()); + } + private void mockResources(Path directory, String resourcePath) throws IOException { + Enumeration resources = Collections.enumeration( + Arrays.asList( + toURL(directory, "git.json", GIT_PLUGIN_JSON), + toURL(directory, "hg.json", HG_PLUGIN_JSON), + toURL(directory, "svn.json", SVN_PLUGIN_JSON) + ) + ); + when(classLoader.getResources(resourcePath)).thenReturn(resources); + } + + private URL toURL(Path directory, String name, String content) throws IOException { + Path file = directory.resolve(name); + java.nio.file.Files.write(file, content.getBytes(StandardCharsets.UTF_8)); + return file.toUri().toURL(); + } + + @Test + void shouldGetFromCacheInProductionStage() throws IOException { + String path = "/locales/de/plugins.json"; + stage(Stage.PRODUCTION); + HttpServletRequest request = mock(HttpServletRequest.class); + when(request.getServletPath()).thenReturn(path); + HttpServletResponse response = mock(HttpServletResponse.class); + + ObjectMapper mapper = new ObjectMapper(); + JsonNode jsonNode = mapper.readTree(GIT_PLUGIN_JSON); + when(cache.get(path)).thenReturn(jsonNode); + + String json = doGetString(request, response); + assertThat(json).contains("scm-git-plugin").doesNotContain("scm-hg-plugin"); verifyHeaders(response); } @Test - @SuppressWarnings("UnstableApiUsage") - public void inProductionStageShouldGetFromCache() throws IOException { + void shouldStoreToCacheInProductionStage(@TempDir Path temp) throws IOException { String path = "/locales/de/plugins.json"; - when(servlet.isProductionStage()).thenReturn(true); + mockResources(temp, "locales/de/plugins.json"); + stage(Stage.PRODUCTION); HttpServletRequest request = mock(HttpServletRequest.class); - HttpServletResponse response = mock(HttpServletResponse.class); - File file = temporaryFolder.newFile(); - PrintWriter writer = new PrintWriter(new FileOutputStream(file)); - when(response.getWriter()).thenReturn(writer); when(request.getServletPath()).thenReturn(path); - when(classLoader.getResources("locales/de/plugins.json")).thenReturn(resources); - ObjectMapper objectMapper = new ObjectMapper(); - JsonNode node = objectMapper.readTree(GIT_PLUGIN_JSON); - node = servlet.merge(node, objectMapper.readTree(HG_PLUGIN_JSON)); - node = servlet.merge(node, objectMapper.readTree(SVN_PLUGIN_JSON)); - when(cache.get(path)).thenReturn(node); + HttpServletResponse response = mock(HttpServletResponse.class); - servlet.doGet(request, response); + String json = doGetString(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); - assertJson(json); + verify(cache).put(any(String.class), any(JsonNode.class)); verifyHeaders(response); - } - - @Test - public void shouldCollectJsonFile() throws IOException { - String path = "locales/de/plugins.json"; - when(classLoader.getResources(path)).thenReturn(resources); - - Optional jsonNodeOptional = servlet.collectJsonFile("/" + path); - - assertJson(jsonNodeOptional.orElse(null)); + assertJson(json); } private void verifyHeaders(HttpServletResponse response) { @@ -261,22 +245,11 @@ public class I18nServletTest { verify(response).setHeader("Cache-Control", "no-cache"); } - public void assertJson(JsonNode actual) throws IOException { - assertJson(actual.toString()); - } - - private void assertJson(String actual) throws IOException { + private void assertJson(String actual) { assertThat(actual) .isNotEmpty() .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))); } - - private File createFileFromString(String json) throws IOException { - File file = temporaryFolder.newFile(); - Files.write(json.getBytes(Charsets.UTF_8), file); - return file; - } - } From 397f524aab2fe48538b8103d175119c0acda9dd9 Mon Sep 17 00:00:00 2001 From: Sebastian Sdorra Date: Tue, 1 Sep 2020 08:42:48 +0200 Subject: [PATCH 4/4] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c5dec347ce..8d96e623d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - JWT token timeout is now handled properly ([#1297](https://github.com/scm-manager/scm-manager/pull/1297)) - Fix text-overflow in danger zone ([#1298](https://github.com/scm-manager/scm-manager/pull/1298)) - Fix plugin installation error if previously a plugin was installed with the same dependency which is still pending. ([#1300](https://github.com/scm-manager/scm-manager/pull/1300)) +- Fix logging of large stacktrace for unknown language ([#1313](https://github.com/scm-manager/scm-manager/pull/1313)) ## [2.4.0] - 2020-08-14 ### Added