From e92616c6eb8773d1586099bdfbdca67099a89c99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Thu, 7 Jun 2018 10:04:28 +0200 Subject: [PATCH] Peer review --- .../api/v2/FieldContainerResponseFilter.java | 9 +- .../java/sonia/scm/api/v2/JsonFilters.java | 90 ++++++++++--------- .../api/v2/JsonMarshallingResponseFilter.java | 4 +- .../v2/FieldContainerResponseFilterTest.java | 10 +-- .../sonia/scm/api/v2/JsonFiltersTest.java | 28 +++--- ...-test-004.json => filter-test-arrays.json} | 0 ...st-003.json => filter-test-deep-path.json} | 0 ...-test-002.json => filter-test-nested.json} | 0 ...-test-001.json => filter-test-simple.json} | 0 9 files changed, 73 insertions(+), 68 deletions(-) rename scm-webapp/src/test/resources/sonia/scm/api/v2/{filter-test-004.json => filter-test-arrays.json} (100%) rename scm-webapp/src/test/resources/sonia/scm/api/v2/{filter-test-003.json => filter-test-deep-path.json} (100%) rename scm-webapp/src/test/resources/sonia/scm/api/v2/{filter-test-002.json => filter-test-nested.json} (100%) rename scm-webapp/src/test/resources/sonia/scm/api/v2/{filter-test-001.json => filter-test-simple.json} (100%) diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/FieldContainerResponseFilter.java b/scm-webapp/src/main/java/sonia/scm/api/v2/FieldContainerResponseFilter.java index 68d52a9bd6..677d5d955e 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/FieldContainerResponseFilter.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/FieldContainerResponseFilter.java @@ -9,6 +9,7 @@ import javax.ws.rs.container.ContainerResponseContext; import javax.ws.rs.container.ContainerResponseFilter; import javax.ws.rs.core.MultivaluedMap; import javax.ws.rs.ext.Provider; +import java.util.Collection; import java.util.List; import java.util.Optional; import java.util.stream.Collectors; @@ -35,7 +36,7 @@ public class FieldContainerResponseFilter implements ContainerResponseFilter { public void filter(ContainerRequestContext requestContext, ContainerResponseContext responseContext) { Optional entity = getJsonEntity(responseContext); if (entity.isPresent()) { - List fields = extractFieldsFrom(requestContext); + Collection fields = extractFieldsFrom(requestContext); if (!fields.isEmpty()) { JsonFilters.filterByFields(entity.get(), fields); } @@ -54,7 +55,7 @@ public class FieldContainerResponseFilter implements ContainerResponseFilter { return entity instanceof JsonNode; } - private List extractFieldsFrom(ContainerRequestContext requestContext) { + private Collection extractFieldsFrom(ContainerRequestContext requestContext) { return getFieldParameterFrom(requestContext) .orElse(emptyList()) .stream() @@ -62,9 +63,9 @@ public class FieldContainerResponseFilter implements ContainerResponseFilter { .collect(Collectors.toList()); } - private Optional> getFieldParameterFrom(ContainerRequestContext requestContext) { + private Optional> getFieldParameterFrom(ContainerRequestContext requestContext) { MultivaluedMap queryParameters = requestContext.getUriInfo().getQueryParameters(); - List fieldParameters = queryParameters.get(PARAMETER_FIELDS); + Collection fieldParameters = queryParameters.get(PARAMETER_FIELDS); return ofNullable(fieldParameters); } } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/JsonFilters.java b/scm-webapp/src/main/java/sonia/scm/api/v2/JsonFilters.java index a06e3118b7..149825b047 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/JsonFilters.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/JsonFilters.java @@ -5,66 +5,72 @@ import com.fasterxml.jackson.databind.node.ArrayNode; import com.fasterxml.jackson.databind.node.ObjectNode; import com.google.common.collect.Maps; +import java.util.Collection; import java.util.Iterator; import java.util.Map; +import static java.util.Arrays.stream; + public final class JsonFilters { private JsonFilters() { } - public static void filterByFields(JsonNode root, Iterable fields) { - filterNode(createJsonFilterNode(fields), root); + public static void filterByFields(JsonNode root, Collection filterExpressions) { + createJsonFilterNode(filterExpressions).filterNode(root); } - private static JsonFilterNode createJsonFilterNode(Iterable fields) { + private static JsonFilterNode createJsonFilterNode(Collection filterExpressions) { JsonFilterNode rootFilterNode = new JsonFilterNode(); - for (String field : fields) { - appendFilterNode(rootFilterNode, field); - } + filterExpressions.stream() + .map(JsonFilterNode::expressionPartIterator) + .forEach(rootFilterNode::appendFilterExpression); return rootFilterNode; } - private static void appendFilterNode(JsonFilterNode rootFilterNode, String field) { - JsonFilterNode filterNode = rootFilterNode; - for (String part : field.split("\\.")) { - filterNode = filterNode.addOrGet(part); - } - } - - private static void filterNode(JsonFilterNode filter, JsonNode node) { - if (node.isObject()) { - filterObjectNode(filter, (ObjectNode) node); - } else if (node.isArray()) { - filterArrayNode(filter, (ArrayNode) node); - } - } - - private static void filterObjectNode(JsonFilterNode filter, ObjectNode objectNode) { - Iterator> entryIterator = objectNode.fields(); - while (entryIterator.hasNext()) { - Map.Entry entry = entryIterator.next(); - - JsonFilterNode childFilter = filter.get(entry.getKey()); - if (childFilter == null) { - entryIterator.remove(); - } else if (!childFilter.isLeaf()) { - filterNode(childFilter, entry.getValue()); - } - } - } - - private static void filterArrayNode(JsonFilterNode filter, ArrayNode arrayNode) { - for (int i=0; i children = Maps.newHashMap(); - JsonFilterNode addOrGet(String name) { + private static Iterator expressionPartIterator(String filterExpression) { + return stream(filterExpression.split("\\.")).iterator(); + } + + private void appendFilterExpression(Iterator fields) { + if (fields.hasNext()) { + addOrGet(fields.next()).appendFilterExpression(fields); + } + } + + private void filterNode(JsonNode node) { + if (!isLeaf()) { + if (node.isObject()) { + filterObjectNode((ObjectNode) node); + } else if (node.isArray()) { + filterArrayNode((ArrayNode) node); + } + } + } + + private void filterObjectNode(ObjectNode objectNode) { + Iterator> entryIterator = objectNode.fields(); + while (entryIterator.hasNext()) { + Map.Entry entry = entryIterator.next(); + + JsonFilterNode childFilter = get(entry.getKey()); + if (childFilter == null) { + entryIterator.remove(); + } else { + childFilter.filterNode(entry.getValue()); + } + } + } + + private void filterArrayNode(ArrayNode arrayNode) { + arrayNode.forEach(this::filterNode); + } + + private JsonFilterNode addOrGet(String name) { JsonFilterNode child = children.get(name); if (child == null) { child = new JsonFilterNode(); diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/JsonMarshallingResponseFilter.java b/scm-webapp/src/main/java/sonia/scm/api/v2/JsonMarshallingResponseFilter.java index 8903ab8790..c0d9904fda 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/JsonMarshallingResponseFilter.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/JsonMarshallingResponseFilter.java @@ -48,9 +48,7 @@ public class JsonMarshallingResponseFilter implements ContainerResponseFilter { node ); - for (JsonEnricher enricher : enrichers) { - enricher.enrich(context); - } + enrichers.forEach(enricher -> enricher.enrich(context)); } private JsonNode getJsonEntity(ContainerResponseContext responseContext) { diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/FieldContainerResponseFilterTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/FieldContainerResponseFilterTest.java index 35e7f5d4d0..9eb8192b88 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/FieldContainerResponseFilterTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/FieldContainerResponseFilterTest.java @@ -38,7 +38,7 @@ public class FieldContainerResponseFilterTest { @Test public void testFilter() throws IOException { applyFields("one"); - JsonNode node = applyEntity("filter-test-002"); + JsonNode node = applyEntity("filter-test-nested"); filter.filter(requestContext, responseContext); @@ -48,7 +48,7 @@ public class FieldContainerResponseFilterTest { @Test public void testFilterWithMultiple() throws IOException { applyFields("one", "five"); - JsonNode node = applyEntity("filter-test-002"); + JsonNode node = applyEntity("filter-test-nested"); filter.filter(requestContext, responseContext); @@ -58,7 +58,7 @@ public class FieldContainerResponseFilterTest { @Test public void testFilterCommaSeparated() throws IOException { applyFields("one,five"); - JsonNode node = applyEntity("filter-test-002"); + JsonNode node = applyEntity("filter-test-nested"); filter.filter(requestContext, responseContext); @@ -68,7 +68,7 @@ public class FieldContainerResponseFilterTest { @Test public void testFilterEmpty() throws IOException { applyFields(); - JsonNode node = applyEntity("filter-test-002"); + JsonNode node = applyEntity("filter-test-nested"); filter.filter(requestContext, responseContext); @@ -78,7 +78,7 @@ public class FieldContainerResponseFilterTest { @Test public void testFilterNotSet() throws IOException { applyFields((List) null); - JsonNode node = applyEntity("filter-test-002"); + JsonNode node = applyEntity("filter-test-nested"); filter.filter(requestContext, responseContext); diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/JsonFiltersTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/JsonFiltersTest.java index bfa280d821..b60775a73b 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/JsonFiltersTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/JsonFiltersTest.java @@ -10,6 +10,7 @@ import org.junit.Test; import java.io.IOException; import java.net.URL; +import static java.util.Arrays.asList; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -19,9 +20,9 @@ public class JsonFiltersTest { @Test public void testFilterByFields() throws IOException { - JsonNode node = readJson("filter-test-001"); + JsonNode node = readJson("filter-test-simple"); - JsonFilters.filterByFields(node, Lists.newArrayList("one")); + JsonFilters.filterByFields(node, asList("one")); assertEquals(1, node.get("one").intValue()); assertFalse(node.has("two")); @@ -30,9 +31,9 @@ public class JsonFiltersTest { @Test public void testFilterByFieldsWithMultipleFields() throws IOException { - JsonNode node = readJson("filter-test-001"); + JsonNode node = readJson("filter-test-simple"); - JsonFilters.filterByFields(node, Lists.newArrayList("one", "three")); + JsonFilters.filterByFields(node, asList("one", "three")); assertEquals(1, node.get("one").intValue()); assertFalse(node.has("two")); @@ -41,22 +42,22 @@ public class JsonFiltersTest { @Test public void testFilterByFieldsWithNonPrimitive() throws IOException { - JsonNode node = readJson("filter-test-002"); - JsonFilters.filterByFields(node, Lists.newArrayList("two")); + JsonNode node = readJson("filter-test-nested"); + JsonFilters.filterByFields(node, asList("two")); assertEquals("{\"two\":{\"three\":3,\"four\":4}}", objectMapper.writeValueAsString(node)); } @Test public void testFilterByFieldsWithDeepField() throws IOException { - JsonNode node = readJson("filter-test-002"); - JsonFilters.filterByFields(node, Lists.newArrayList("two.three")); + JsonNode node = readJson("filter-test-nested"); + JsonFilters.filterByFields(node, asList("two.three")); assertEquals("{\"two\":{\"three\":3}}", objectMapper.writeValueAsString(node)); } @Test public void testFilterByFieldsWithVeryDeepField() throws IOException { - JsonNode node = readJson("filter-test-003"); - JsonFilters.filterByFields(node, Lists.newArrayList("two.three.four.five")); + JsonNode node = readJson("filter-test-deep-path"); + JsonFilters.filterByFields(node, asList("two.three.four.five")); assertFalse(node.has("one")); String json = objectMapper.writeValueAsString(node.get("two").get("three").get("four").get("five")); assertEquals("{\"six\":6,\"seven\":7}", json); @@ -64,10 +65,10 @@ public class JsonFiltersTest { @Test public void testFilterByFieldsWithArray() throws IOException { - JsonNode node = readJson("filter-test-004"); - JsonFilters.filterByFields(node, Lists.newArrayList("one.two")); + JsonNode node = readJson("filter-test-arrays"); + JsonFilters.filterByFields(node, asList("one.two")); ArrayNode one = (ArrayNode) node.get("one"); - assertEquals(one.size(), 2); + assertEquals(2, one.size()); for (int i=0; i