fixing #321 (ref-by-$id not working in remote schema references)

 * added integ test for reproducing the problem
 * test update: `JsonPointerEvaluatorTest#JsonPointerEvaluatorTest()` now expects the source URI to be set as location for remote fetched documents
    - updating `JsonPointerEvaluator#configureBasedOnState()` to pass the test
 * for reference schemas, using the location of the found object (instead of the resolved URI) as schema location
 * changing `JsonPointerEvaluator#query()` to first try to lookup the result schema by $id in the containing document, before trying to
    use it as a json pointer
 * adding some missing `final` modifiers to fields

Author: erosb

core/src/main/java/org/everit/json/schema/loader/JsonPointerEvaluator.java

@@ -1,6 +1,7 @@
 package org.everit.json.schema.loader;
 
 import static java.util.Arrays.asList;
+import static java.util.Collections.emptyList;
 import static java.util.Objects.requireNonNull;
 import static org.everit.json.schema.loader.OrgJsonUtil.toMap;
 
@@ -10,6 +11,7 @@
 import java.io.InputStreamReader;
 import java.io.UncheckedIOException;
 import java.io.UnsupportedEncodingException;
+import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URLDecoder;
 import java.nio.charset.Charset;
@@ -108,10 +110,10 @@ static final JsonPointerEvaluator forDocument(JsonObject document, String fragme
     }
 
     private static JsonObject configureBasedOnState(JsonObject obj, LoadingState callingState, String id) {
+        URI documentURI = validateURI(callingState, id).asJavaURI();
         obj.ls = new LoadingState(callingState.config,
                 callingState.pointerSchemas, obj, obj,
-                validateURI(callingState, id).asJavaURI(),
-                SchemaLocation.empty());
+                documentURI, new SchemaLocation(documentURI, emptyList()));
         return obj;
     }
 
@@ -152,6 +154,10 @@ public QueryResult query() {
         if (fragment.isEmpty()) {
             return new QueryResult(document, document);
         }
+        JsonObject foundById = ReferenceLookup.lookupObjById(document, fragment);
+        if (foundById != null) {
+            return new QueryResult(document, foundById);
+        }
         String[] path = fragment.split("/");
         if ((path[0] == null) || !path[0].startsWith("#")) {
             throw new IllegalArgumentException("JSON pointers must start with a '#'");

core/src/main/java/org/everit/json/schema/loader/ReferenceLookup.java

@@ -20,7 +20,7 @@ class ReferenceKnot {
 
     private Schema referredSchema;
 
-    private List<ReferenceSchema.Builder> refs = new ArrayList<>(1);
+    private final List<ReferenceSchema.Builder> refs = new ArrayList<>(1);
 
     ReferenceSchema.Builder initReference(String refValue) {
         ReferenceSchema.Builder builder = new ReferenceSchema.Builder().refValue(refValue);
@@ -67,21 +67,32 @@ static Map<String, Object> extend(Map<String, Object> additional, Map<String, Ob
         return rawObj;
     }
 
-    private LoadingState ls;
-
-    private SchemaClient schemaClient;
-
-    public ReferenceLookup(LoadingState ls) {
-        this.ls = requireNonNull(ls, "ls cannot be null");
-        this.schemaClient = ls.config.schemaClient;
-    }
-
-    private Map<String, Object> doExtend(Map<String, Object> additional, Map<String, Object> original) {
-        if (ls.specVersion() == SpecificationVersion.DRAFT_4) {
-            return extend(additional, original);
-        } else {
-            return original;
+    static JsonObject lookupObjById(JsonValue val, String idAttrVal) {
+        String idKeyword = val.ls.specVersion().idKeyword();
+        if (val instanceof JsonObject) {
+            JsonObject obj = (JsonObject) val;
+            if (obj.containsKey(idKeyword)
+                    && obj.require(idKeyword).typeOfValue() == String.class
+                    && obj.require(idKeyword).requireString().equals(idAttrVal)) {
+                return obj;
+            }
+            for (String key : obj.keySet()) {
+                JsonObject maybeFound = lookupObjById(obj.require(key), idAttrVal);
+                if (maybeFound != null) {
+                    return maybeFound;
+                }
+            }
+        } else if (val instanceof JsonArray) {
+            JsonArray arr = (JsonArray) val;
+            for (int i = 0; i < arr.length(); ++i) {
+                JsonObject maybeFound = lookupObjById(arr.at(i), idAttrVal);
+                if (maybeFound != null) {
+                    return maybeFound;
+                }
+            }
         }
+
+        return null;
     }
 
     /**
@@ -106,6 +117,23 @@ static URI withoutFragment(final String fullUri) {
         }
     }
 
+    private final LoadingState ls;
+
+    private final SchemaClient schemaClient;
+
+    public ReferenceLookup(LoadingState ls) {
+        this.ls = requireNonNull(ls, "ls cannot be null");
+        this.schemaClient = ls.config.schemaClient;
+    }
+
+    private Map<String, Object> doExtend(Map<String, Object> additional, Map<String, Object> original) {
+        if (ls.specVersion() == SpecificationVersion.DRAFT_4) {
+            return extend(additional, original);
+        } else {
+            return original;
+        }
+    }
+
     Map<String, Object> withoutRef(JsonObject original) {
         Map<String, Object> rawObj = new HashMap<>();
         original.keySet().stream()
@@ -114,34 +142,6 @@ Map<String, Object> withoutRef(JsonObject original) {
         return rawObj;
     }
 
-    private JsonObject lookupObjById(JsonValue val, String idAttrVal) {
-        String idKeyword = val.ls.specVersion().idKeyword();
-        if (val instanceof JsonObject) {
-            JsonObject obj = (JsonObject) val;
-            if (obj.containsKey(idKeyword)
-                    && obj.require(idKeyword).typeOfValue() == String.class
-                    && obj.require(idKeyword).requireString().equals(idAttrVal)) {
-                return obj;
-            }
-            for (String key : obj.keySet()) {
-                JsonObject maybeFound = lookupObjById(obj.require(key), idAttrVal);
-                if (maybeFound != null) {
-                    return maybeFound;
-                }
-            }
-        } else if (val instanceof JsonArray) {
-            JsonArray arr = (JsonArray) val;
-            for (int i = 0; i < arr.length(); ++i) {
-                JsonObject maybeFound = lookupObjById(arr.at(i), idAttrVal);
-                if (maybeFound != null) {
-                    return maybeFound;
-                }
-            }
-        }
-
-        return null;
-    }
-
     private Schema.Builder<?> performQueryEvaluation(String mapKey, JsonPointerEvaluator pointerEvaluator) {
         String absolutePointer = ReferenceResolver.resolve(ls.id, mapKey).toString();
         if (ls.pointerSchemas.containsKey(absolutePointer)) {
@@ -174,13 +174,14 @@ Schema.Builder<?> lookup(String relPointerString, JsonObject ctx) {
 
         URI resolutionScope = !isSameDocumentRef(absPointerString) ? withoutFragment(absPointerString) : ls.id;
         JsonObject containingDocument = result.getContainingDocument();
+        SchemaLocation resultLocation = result.getQueryResult().ls.pointerToCurrentObj;
         SchemaLoader childLoader = ls.initNewDocumentLoader()
-                .pointerToCurrentObj(SchemaLocation.parseURI(absPointerString))
+                .pointerToCurrentObj(resultLocation)
                 .resolutionScope(resolutionScope)
                 .schemaJson(result.getQueryResult())
                 .rootSchemaJson(containingDocument).build();
         Schema referredSchema = childLoader.load().build();
-        refBuilder.schemaLocation(SchemaLocation.parseURI(absPointerString));
+        refBuilder.schemaLocation(resultLocation);
         knot.resolveWith(referredSchema);
         return refBuilder;
     }

core/src/test/java/org/everit/json/schema/loader/JsonPointerEvaluatorTest.java

@@ -14,6 +14,7 @@
 
 import java.io.InputStream;
 import java.net.URI;
+import java.net.URISyntaxException;
 import java.util.HashMap;
 
 import org.everit.json.schema.ResourceLoader;
@@ -79,7 +80,7 @@ private LoadingState createLoadingState(SchemaClient schemaClient, String ref) {
     }
 
     @Test
-    public void remoteDocumentSuccess() {
+    public void remoteDocumentSuccess() throws URISyntaxException {
         SchemaClient schemaClient = mock(SchemaClient.class);
         when(schemaClient.get("http://localhost:1234/hello")).thenReturn(rootSchemaJsonAsStream());
         JsonPointerEvaluator pointer = JsonPointerEvaluator
@@ -88,7 +89,8 @@ public void remoteDocumentSuccess() {
         JsonObject actual = pointer.query().getQueryResult().requireObject();
         assertEquals("dummy schema at #/definitions/Bar", actual.require("description").requireString());
         assertEquals("http://localhost:1234/folder/", actual.ls.id.toString());
-        assertEquals(new SchemaLocation(asList("definitions", "Bar")), actual.ls.pointerToCurrentObj);
+        assertEquals(new SchemaLocation(new URI("http://localhost:1234/hello"), asList("definitions", "Bar")),
+                actual.ls.pointerToCurrentObj);
     }
 
     @Test

tests/vanilla/src/main/resources/org/everit/json/schema/issues/issue321/remotes/schema.json

@@ -0,0 +1,9 @@
+{
+  "$schema": "http://json-schema.org/draft-07/schema",
+  "$defs": {
+    "a": {
+      "$id": "#a",
+      "type": "integer"
+    }
+  }
+}

tests/vanilla/src/main/resources/org/everit/json/schema/issues/issue321/schema.json

@@ -0,0 +1,7 @@
+{
+  "properties": {
+    "a": {
+      "$ref": "http://localhost:1234/schema.json#a"
+    }
+  }
+}

tests/vanilla/src/main/resources/org/everit/json/schema/issues/issue321/subject-valid.json

@@ -0,0 +1,3 @@
+{
+  "a": 42
+}