Warn instead of failing on stale dynamic search fields

Closes #74.

Author: Philip Z

teaql-core/src/main/java/io/teaql/core/BaseRequest.java

@@ -205,6 +205,14 @@ public SearchCriteria getSearchCriteria() {
         return searchCriteria;
     }
 
+    /**
+     * Replaces the current criteria as one atomic parser operation. Dynamic input adapters use
+     * this to roll back a partially resolved relation path when a later field segment is stale.
+     */
+    public void replaceSearchCriteria(SearchCriteria searchCriteria) {
+        this.searchCriteria = searchCriteria;
+    }
+
     @Override
     public OrderBys getOrderBy() {
         return orderBys;

teaql-query-json/pom.xml

@@ -23,5 +23,11 @@
             <groupId>com.fasterxml.jackson.core</groupId>
             <artifactId>jackson-databind</artifactId>
         </dependency>
+        <dependency>
+            <groupId>junit</groupId>
+            <artifactId>junit</artifactId>
+            <version>4.13.2</version>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 </project>

teaql-query-json/src/main/java/io/teaql/query/json/DynamicSearchHelper.java

@@ -2,9 +2,14 @@
 
 import java.util.Date;
 import java.util.Iterator;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.node.JsonNodeType;
@@ -68,6 +73,53 @@ public void setDateTimeField(boolean dateTimeField) {
 
 public class DynamicSearchHelper {
 
+    public static final String WARNINGS_EXTENSION = "teaql.dynamicSearch.warnings";
+    private static final Logger LOGGER = Logger.getLogger(DynamicSearchHelper.class.getName());
+
+    public static List<DynamicSearchWarning> warningsOf(BaseRequest request) {
+        Object warnings = request.getExtensions().get(WARNINGS_EXTENSION);
+        if (!(warnings instanceof List<?>)) {
+            return Collections.emptyList();
+        }
+        List<DynamicSearchWarning> result = new ArrayList<>();
+        for (Object warning : (List<?>) warnings) {
+            if (warning instanceof DynamicSearchWarning) {
+                result.add((DynamicSearchWarning) warning);
+            }
+        }
+        return Collections.unmodifiableList(result);
+    }
+
+    protected void warnUnknownField(BaseRequest request, String clause, String fieldPath) {
+        DynamicSearchWarning warning =
+                new DynamicSearchWarning(
+                        DynamicSearchWarning.UNKNOWN_FIELD,
+                        request.getTypeName(),
+                        clause,
+                        fieldPath);
+        Object existing = request.getExtensions().get(WARNINGS_EXTENSION);
+        List<DynamicSearchWarning> warnings;
+        if (existing instanceof List<?>) {
+            @SuppressWarnings("unchecked")
+            List<DynamicSearchWarning> typed = (List<DynamicSearchWarning>) existing;
+            warnings = typed;
+        }
+        else {
+            warnings = new ArrayList<>();
+            request.putExtension(WARNINGS_EXTENSION, warnings);
+        }
+        warnings.add(warning);
+        LOGGER.log(
+                Level.WARNING,
+                "Ignored unknown dynamic search field: code={0}, requestType={1}, clause={2}, fieldPath={3}",
+                new Object[] {
+                    warning.getCode(),
+                    warning.getRequestType(),
+                    warning.getClause(),
+                    warning.getFieldPath()
+                });
+    }
+
     protected static JsonNode jsonFromString(String jsonExpr) {
         try {
             ObjectMapper objectMapper = new ObjectMapper();
@@ -129,6 +181,9 @@ public void addJsonFilter(BaseRequest baseRequest, JsonNode jsonNode) {
             String fieldName = field.getKey();
 
             if (!baseRequest.isOneOfSelfField(fieldName)) {
+                if (!fieldName.startsWith("_")) {
+                    warnUnknownField(baseRequest, "FILTER", fieldName);
+                }
                 continue;
             }
             JsonNode fieldValue = field.getValue();
@@ -155,12 +210,31 @@ protected boolean handleChainField(
         if (fieldNames.length < 2) {
             return true; // need to continue
         }
+        SearchCriteria originalCriteria = rootRequest.getSearchCriteria();
         BaseRequest currentRequest = rootRequest;
         for (int i = 0; i < fieldNames.length - 1; i++) {
-            Optional<BaseRequest> optional = currentRequest.subRequestOfFieldName(fieldNames[i]);
+            Optional<BaseRequest> optional;
+            try {
+                optional = currentRequest.subRequestOfFieldName(fieldNames[i]);
+            }
+            catch (IllegalArgumentException exception) {
+                rootRequest.replaceSearchCriteria(originalCriteria);
+                warnUnknownField(rootRequest, "FILTER", fieldName);
+                return false;
+            }
+            if (optional.isEmpty()) {
+                rootRequest.replaceSearchCriteria(originalCriteria);
+                warnUnknownField(rootRequest, "FILTER", fieldName);
+                return false;
+            }
             currentRequest = optional.get();
         }
         final String lastSegmentOfField = fieldNames[fieldNames.length - 1];
+        if (!currentRequest.isOneOfSelfField(lastSegmentOfField)) {
+            rootRequest.replaceSearchCriteria(originalCriteria);
+            warnUnknownField(rootRequest, "FILTER", fieldName);
+            return false;
+        }
         // last segment of field, use it as value
         currentRequest.appendSearchCriteria(
                 currentRequest.createBasicSearchCriteria(
@@ -356,6 +430,7 @@ public void addJsonOrderBy(BaseRequest baseRequest, JsonNode jsonNode) {
         // single text
         if (fieldValue.isTextual()) {
             if (!baseRequest.isOneOfSelfField(fieldValue.asText())) {
+                warnUnknownField(baseRequest, "ORDER_BY", fieldValue.asText());
                 return;
             }
             this.addOrderBy(baseRequest, fieldValue.asText(), false);
@@ -381,6 +456,7 @@ public void addJsonOrderBy(BaseRequest baseRequest, JsonNode jsonNode) {
     protected void addSingleJsonOrderBy(BaseRequest baseRequest, JsonNode jsonValueNode) {
         String field = jsonValueNode.get("field").asText();
         if (!baseRequest.isOneOfSelfField(field)) {
+            warnUnknownField(baseRequest, "ORDER_BY", field);
             return;
         }
         Boolean useAsc = jsonValueNode.get("useAsc").booleanValue();

teaql-query-json/src/main/java/io/teaql/query/json/DynamicSearchWarning.java

@@ -0,0 +1,48 @@
+package io.teaql.query.json;
+
+import java.util.Objects;
+
+/** A non-fatal compatibility warning produced while applying a dynamic search payload. */
+public final class DynamicSearchWarning {
+
+    public static final String UNKNOWN_FIELD = "DYNAMIC_SEARCH_UNKNOWN_FIELD";
+
+    private final String code;
+    private final String requestType;
+    private final String clause;
+    private final String fieldPath;
+
+    public DynamicSearchWarning(
+            String code, String requestType, String clause, String fieldPath) {
+        this.code = Objects.requireNonNull(code);
+        this.requestType = Objects.requireNonNull(requestType);
+        this.clause = Objects.requireNonNull(clause);
+        this.fieldPath = Objects.requireNonNull(fieldPath);
+    }
+
+    public String getCode() {
+        return code;
+    }
+
+    public String getRequestType() {
+        return requestType;
+    }
+
+    public String getClause() {
+        return clause;
+    }
+
+    public String getFieldPath() {
+        return fieldPath;
+    }
+
+    @Override
+    public String toString() {
+        return "DynamicSearchWarning{" +
+                "code='" + code + '\'' +
+                ", requestType='" + requestType + '\'' +
+                ", clause='" + clause + '\'' +
+                ", fieldPath='" + fieldPath + '\'' +
+                '}';
+    }
+}

teaql-query-json/src/main/java/module-info.java

@@ -2,6 +2,7 @@
     requires io.teaql.core;
     requires io.teaql.utils;
     requires com.fasterxml.jackson.databind;
+    requires java.logging;
 
     exports io.teaql.query.json;
 }

teaql-query-json/src/test/java/io/teaql/query/json/DynamicSearchHelperTest.java

@@ -0,0 +1,96 @@
+package io.teaql.query.json;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+import io.teaql.core.BaseRequest;
+import io.teaql.core.Entity;
+import java.util.Optional;
+import org.junit.Test;
+
+public class DynamicSearchHelperTest {
+
+    @Test
+    public void unknownTopLevelFilterIsIgnoredAndRecorded() {
+        StubRequest request = new StubRequest("Order");
+
+        new DynamicSearchHelper()
+                .mergeClauses(
+                        request,
+                        DynamicSearchHelper.jsonFromString("{\"removedField\":\"value\"}"));
+
+        assertTrue(request.getSearchCriteria() == null);
+        assertWarning(request, "FILTER", "removedField");
+    }
+
+    @Test
+    public void unknownRelationOrNestedFieldDoesNotThrow() {
+        StubRequest request = new StubRequest("Order");
+        request.child = new StubRequest("Customer");
+
+        DynamicSearchHelper helper = new DynamicSearchHelper();
+        helper.mergeClauses(
+                request,
+                DynamicSearchHelper.jsonFromString("{\"removedRelation.name\":\"x\"}"));
+        helper.mergeClauses(
+                request,
+                DynamicSearchHelper.jsonFromString("{\"customer.removedField\":\"x\"}"));
+
+        assertEquals(2, DynamicSearchHelper.warningsOf(request).size());
+        assertTrue("stale nested fields must not leave a partial relation filter", request.getSearchCriteria() == null);
+        assertWarning(request, "FILTER", "removedRelation.name");
+        assertWarning(request, "FILTER", "customer.removedField");
+    }
+
+    @Test
+    public void unknownOrderFieldsAreIgnoredAndRecorded() {
+        StubRequest request = new StubRequest("Order");
+
+        new DynamicSearchHelper()
+                .mergeClauses(
+                        request,
+                        DynamicSearchHelper.jsonFromString(
+                                "{\"_orderBy\":[{\"field\":\"oldName\",\"useAsc\":true}]}"));
+
+        assertTrue(request.getOrderBy().isEmpty());
+        assertWarning(request, "ORDER_BY", "oldName");
+    }
+
+    private static void assertWarning(BaseRequest<?> request, String clause, String fieldPath) {
+        assertTrue(
+                DynamicSearchHelper.warningsOf(request).stream()
+                        .anyMatch(
+                                warning ->
+                                        DynamicSearchWarning.UNKNOWN_FIELD.equals(warning.getCode())
+                                                && clause.equals(warning.getClause())
+                                                && fieldPath.equals(warning.getFieldPath())));
+    }
+
+    private static final class StubRequest extends BaseRequest<Entity> {
+        private final String typeName;
+        private StubRequest child;
+
+        private StubRequest(String typeName) {
+            super(Entity.class);
+            this.typeName = typeName;
+        }
+
+        @Override
+        public String getTypeName() {
+            return typeName;
+        }
+
+        @Override
+        public boolean isOneOfSelfField(String propertyName) {
+            return "id".equals(propertyName) || "name".equals(propertyName);
+        }
+
+        @Override
+        public Optional<BaseRequest> subRequestOfFieldName(String fieldName) {
+            if ("customer".equals(fieldName) && child != null) {
+                return Optional.of(child);
+            }
+            throw new IllegalArgumentException("unknown relation");
+        }
+    }
+}