Issue#1779 Invalid sparse fields should return 4xx (#1801)

yahoo · Jan 27, 2021 · 37a7860 · 37a7860
1 parent e5354ea
commit 37a7860
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 3 deletions.
diff --git a/elide-core/src/main/java/com/yahoo/elide/jsonapi/EntityProjectionMaker.java b/elide-core/src/main/java/com/yahoo/elide/jsonapi/EntityProjectionMaker.java
@@ -10,6 +10,7 @@
 import com.yahoo.elide.core.RequestScope;
 import com.yahoo.elide.core.dictionary.EntityDictionary;
 import com.yahoo.elide.core.exceptions.InvalidCollectionException;
+import com.yahoo.elide.core.exceptions.InvalidValueException;
 import com.yahoo.elide.core.filter.expression.FilterExpression;
 import com.yahoo.elide.core.pagination.PaginationImpl;
 import com.yahoo.elide.core.request.Attribute;
@@ -23,6 +24,7 @@
 import com.yahoo.elide.generated.parsers.CoreParser;
 import com.yahoo.elide.jsonapi.parser.JsonApiParser;
 import com.google.common.collect.Sets;
+
 import org.apache.commons.lang3.tuple.Pair;
 import lombok.Builder;
 import lombok.Data;
@@ -330,9 +332,13 @@ private Set<Attribute> getSparseAttributes(Type<?> entityClass) {
         Set<String> sparseFieldsForEntity = sparseFields.get(dictionary.getJsonAliasFor(entityClass));
         if (sparseFieldsForEntity == null || sparseFieldsForEntity.isEmpty()) {
             sparseFieldsForEntity = allAttributes;
+        } else {
+            Set<String> allRelationships = new LinkedHashSet<>(dictionary.getRelationships(entityClass));
+            validateSparseFields(sparseFieldsForEntity, allAttributes, allRelationships, entityClass);
+            sparseFieldsForEntity = Sets.intersection(allAttributes, sparseFieldsForEntity);
         }
 
-        return Sets.intersection(allAttributes, sparseFieldsForEntity).stream()
+        return sparseFieldsForEntity.stream()
                 .map(attributeName -> Attribute.builder()
                     .name(attributeName)
                     .type(dictionary.getType(entityClass, attributeName))
@@ -346,10 +352,12 @@ private Map<String, EntityProjection> getSparseRelationships(Type<?> entityClass
 
         if (sparseFieldsForEntity == null || sparseFieldsForEntity.isEmpty()) {
             sparseFieldsForEntity = allRelationships;
+        } else {
+            Set<String> allAttributes = new LinkedHashSet<>(dictionary.getAttributes(entityClass));
+            validateSparseFields(sparseFieldsForEntity, allAttributes, allRelationships, entityClass);
+            sparseFieldsForEntity = Sets.intersection(allRelationships, sparseFieldsForEntity);
         }
 
-        sparseFieldsForEntity = Sets.intersection(allRelationships, sparseFieldsForEntity);
-
         return sparseFieldsForEntity.stream()
                 .collect(Collectors.toMap(
                         Function.identity(),
@@ -365,6 +373,18 @@ private Map<String, EntityProjection> getSparseRelationships(Type<?> entityClass
                 ));
     }
 
+    private void validateSparseFields(Set<String> sparseFieldsForEntity, Set<String> allAttributes,
+                    Set<String> allRelationships, Type<?> entityClass) {
+        String unknownSparseFields = sparseFieldsForEntity.stream()
+                        .filter(field -> !(allAttributes.contains(field) || allRelationships.contains(field)))
+                        .collect(Collectors.joining(", "));
+
+        if (!unknownSparseFields.isEmpty()) {
+            throw new InvalidValueException(String.format("%s does not contain the fields: [%s]",
+                            dictionary.getJsonAliasFor(entityClass), unknownSparseFields));
+        }
+    }
+
     private Map<String, EntityProjection> getRequiredRelationships(Type<?> entityClass) {
         return Stream.concat(
                 getIncludedRelationships(entityClass).entrySet().stream(),

diff --git a/elide-core/src/test/java/com/yahoo/elide/jsonapi/EntityProjectionMakerTest.java b/elide-core/src/test/java/com/yahoo/elide/jsonapi/EntityProjectionMakerTest.java
@@ -8,10 +8,13 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
 import com.yahoo.elide.core.Path;
 import com.yahoo.elide.core.RequestScope;
 import com.yahoo.elide.core.TestRequestScope;
 import com.yahoo.elide.core.dictionary.EntityDictionary;
+import com.yahoo.elide.core.exceptions.InvalidValueException;
 import com.yahoo.elide.core.filter.expression.FilterExpression;
 import com.yahoo.elide.core.filter.predicates.InPredicate;
 import com.yahoo.elide.core.pagination.PaginationImpl;
@@ -756,6 +759,34 @@ public void testRootCollectionWithTypedFilter() {
         projectionEquals(expected, actual);
     }
 
+    @Test
+    public void testInvalidSparseFields() {
+        MultivaluedMap<String, String> queryParams = new MultivaluedHashMap<>();
+        queryParams.add("fields[book]", "publisher,bookTitle,bookName"); // Invalid Fields: bookTitle & bookName
+        String path = "/book";
+
+        RequestScope scope = new TestRequestScope(dictionary, path, queryParams);
+        EntityProjectionMaker maker = new EntityProjectionMaker(dictionary, scope);
+
+        Exception e = assertThrows(InvalidValueException.class, () -> maker.parsePath(path));
+        assertEquals("Invalid value: book does not contain the fields: [bookTitle, bookName]", e.getMessage());
+    }
+
+    @Test
+    public void testInvalidSparseFieldsNested() {
+        MultivaluedMap<String, String> queryParams = new MultivaluedHashMap<>();
+        queryParams.add("fields[book]", "publisher,title");
+        queryParams.add("fields[publisher]", "name,cost"); // Invalid Fields: cost
+        queryParams.add("include", "publisher");
+        String path = "/book";
+
+        RequestScope scope = new TestRequestScope(dictionary, path, queryParams);
+        EntityProjectionMaker maker = new EntityProjectionMaker(dictionary, scope);
+
+        Exception e = assertThrows(InvalidValueException.class, () -> maker.parsePath(path));
+        assertEquals("Invalid value: publisher does not contain the fields: [cost]", e.getMessage());
+    }
+
     private void projectionEquals(EntityProjection projection1, EntityProjection projection2) {
         assertEquals(projection1.getType(), projection2.getType());
         assertEquals(projection1.getSorting(), projection2.getSorting());

diff --git a/...m/yahoo/elide/datastores/aggregation/integration/AggregationDataStoreIntegrationTest.java b/...m/yahoo/elide/datastores/aggregation/integration/AggregationDataStoreIntegrationTest.java
@@ -902,6 +902,18 @@ public void testDynamicAggregationModel() {
             .body("meta.page.limit", equalTo(500));
     }
 
+    @Test
+    public void testInvalidSparseFields() {
+        String expectedError = "Invalid value: orderDetails does not contain the fields: [orderValue, customerState]";
+        String getPath = "/orderDetails?fields[orderDetails]=orderValue,customerState,orderMonth";
+        given()
+            .when()
+            .get(getPath)
+            .then()
+            .statusCode(HttpStatus.SC_BAD_REQUEST)
+            .body("errors.detail", hasItems(expectedError));
+    }
+
     @Test
     public void missingClientFilterTest() {
         String expectedError = "Querying deliveryDetails requires a mandatory filter:"