refactor I18nServlet to avoid stacktrace logging on unknown language

This commit is contained in:
Sebastian Sdorra
2020-09-01 07:53:21 +02:00
parent de0bcaf3e8
commit 8d65bf75f3
2 changed files with 65 additions and 69 deletions

View File

@@ -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<String, Optional<JsonNode>> jsonFileProvider,
BiConsumer<String, JsonNode> createdJsonFileConsumer) {
return Optional.ofNullable(jsonFileProvider.apply(path)
.orElseGet(() -> {
Optional<JsonNode> 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<JsonNode> 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<String, Optional<JsonNode>> jsonFileProvider = usedPath -> Optional.empty();
BiConsumer<String, JsonNode> 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<JsonNode> findJson(String path) throws IOException {
if (isProductionStage()) {
return findJsonCached(path);
}
return collectJsonFile(path);
}
private Optional<JsonNode> 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<JsonNode> 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<JsonNode> collectJsonFile(String path) {
protected Optional<JsonNode> collectJsonFile(String path) throws IOException {
LOG.debug("Collect plugin translations from path {} for every plugin", path);
JsonNode mergedJsonNode = null;
try {
Enumeration<URL> 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<URL> 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 <code>updateNode</code> into the <code>mainNode</code> and return it.
*
* <p>
* 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<String> fieldNames = updateNode.fieldNames();
while (fieldNames.hasNext()) {
String fieldName = fieldNames.next();
JsonNode jsonNode = mainNode.get(fieldName);
if (jsonNode != null) {
mergeNode(updateNode, fieldName, jsonNode);
} else {

View File

@@ -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<String, JsonNode> cache;
private Enumeration<URL> 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.<String, JsonNode>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);