Peer review

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<JsonNode> entity = getJsonEntity(responseContext);
     if (entity.isPresent()) {
-      List<String> fields = extractFieldsFrom(requestContext);
+      Collection<String> 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<String> extractFieldsFrom(ContainerRequestContext requestContext) {
+  private Collection<String> extractFieldsFrom(ContainerRequestContext requestContext) {
     return getFieldParameterFrom(requestContext)
       .orElse(emptyList())
       .stream()
@@ -62,9 +63,9 @@ public class FieldContainerResponseFilter implements ContainerResponseFilter {
       .collect(Collectors.toList());
   }
 
-  private Optional<List<String>> getFieldParameterFrom(ContainerRequestContext requestContext) {
+  private Optional<Collection<String>> getFieldParameterFrom(ContainerRequestContext requestContext) {
     MultivaluedMap<String, String> queryParameters = requestContext.getUriInfo().getQueryParameters();
-    List<String> fieldParameters = queryParameters.get(PARAMETER_FIELDS);
+    Collection<String> fieldParameters = queryParameters.get(PARAMETER_FIELDS);
     return ofNullable(fieldParameters);
   }
 }

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<String> fields) {
+  public static void filterByFields(JsonNode root, Collection<String> filterExpressions) {
-    filterNode(createJsonFilterNode(fields), root);
+    createJsonFilterNode(filterExpressions).filterNode(root);
   }
 
-  private static JsonFilterNode createJsonFilterNode(Iterable<String> fields) {
+  private static JsonFilterNode createJsonFilterNode(Collection<String> filterExpressions) {
     JsonFilterNode rootFilterNode = new JsonFilterNode();
-    for (String field : fields) {
+    filterExpressions.stream()
-      appendFilterNode(rootFilterNode, field);
+      .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<Map.Entry<String,JsonNode>> entryIterator = objectNode.fields();
-    while (entryIterator.hasNext()) {
-      Map.Entry<String,JsonNode> 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<arrayNode.size(); i++) {
-      filterNode(filter, arrayNode.get(i));
-    }
-  }
-
   private static class JsonFilterNode {
 
     private final Map<String,JsonFilterNode> children = Maps.newHashMap();
 
-    JsonFilterNode addOrGet(String name) {
+    private static Iterator<String> expressionPartIterator(String filterExpression) {
+      return stream(filterExpression.split("\\.")).iterator();
+    }
+
+    private void appendFilterExpression(Iterator<String> 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<Map.Entry<String,JsonNode>> entryIterator = objectNode.fields();
+      while (entryIterator.hasNext()) {
+        Map.Entry<String,JsonNode> 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();

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) {
+    enrichers.forEach(enricher -> enricher.enrich(context));
-      enricher.enrich(context);
-    }
   }
 
   private JsonNode getJsonEntity(ContainerResponseContext responseContext) {

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);
 

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");
+    JsonNode node = readJson("filter-test-nested");
-    JsonFilters.filterByFields(node, Lists.newArrayList("two"));
+    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");
+    JsonNode node = readJson("filter-test-nested");
-    JsonFilters.filterByFields(node, Lists.newArrayList("two.three"));
+    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");
+    JsonNode node = readJson("filter-test-deep-path");
-    JsonFilters.filterByFields(node, Lists.newArrayList("two.three.four.five"));
+    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");
+    JsonNode node = readJson("filter-test-arrays");
-    JsonFilters.filterByFields(node, Lists.newArrayList("one.two"));
+    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<one.size(); i++) {
       JsonNode childOfOne = one.get(i);
       assertFalse(childOfOne.has("three"));
@@ -79,5 +80,4 @@ public class JsonFiltersTest {
     URL resource = Resources.getResource("sonia/scm/api/v2/" + name + ".json");
     return objectMapper.readTree(resource);
   }
-
 }