InstanceValidator: improve test, documentation, error message

Also rewrite a schema for a more simple example

Signed-off-by: Francis Galiegue <[email protected]>

src/main/java/com/github/fge/jsonschema/processors/validation/InstanceValidator.java

@@ -61,6 +61,26 @@ public final class InstanceValidator
     private final MessageBundle validationMessages;
     private final Processor<SchemaContext, ValidatorList> keywordBuilder;
 
+    /*
+     * It is possible to trigger a validation loop if there is a repeated
+     * triplet schema ID/schema pointer/instance pointer while we validate;
+     * example schema:
+     *
+     * { "oneOf": [ {}, { "$ref": "#" } ] }
+     *
+     * Whatever the data, validation will end up validating, for a same pointer
+     * into the instance, the following pointers into the schema:
+     *
+     * "" -> "/oneOf/0" -> "/oneOf/1" -> "" <-- LOOP
+     *
+     * This is not a JSON Reference loop here, but truly a validation loop.
+     *
+     * We therefore use this set to record the triplets seen by using an
+     * Equivalence over FullData which detects this. This is helped by the fact
+     * that SchemaTree now implements equals()/hashCode() in -core; since this
+     * class is instantiated for each instance validation, we are certain that
+     * what instance pointer is seen is the one of the instance we validate.
+     */
     private final Set<Equivalence.Wrapper<FullData>> visited
         = Sets.newLinkedHashSet();
 
@@ -88,7 +108,10 @@ public FullData process(final ProcessingReport report,
             final ProcessingMessage message = input.newMessage()
                 .put("domain", "validation")
                 .setMessage(errmsg)
-                .put("visited", node);
+                .putArgument("alreadyVisited", toJson(input))
+                .putArgument("instancePointer",
+                    input.getInstance().getPointer().toString())
+                .put("validationPath", node);
             throw new ProcessingException(message);
         }
 
@@ -231,15 +254,16 @@ public String toString()
         return "instance validator";
     }
 
-    private static JsonNode toJson(final FullData data)
+    private static String toJson(final FullData data)
     {
         final SchemaTree tree = data.getSchema();
         final URI baseUri = tree.getLoadingRef().getLocator();
         try {
+            // TODO: there should be an easier way to do that...
             final URITemplate template = new URITemplate(baseUri + "{+ptr}");
             final VariableMap vars = VariableMap.newBuilder().addScalarValue(
                 "ptr", tree.getPointer()).freeze();
-            return JacksonUtils.nodeFactory().textNode(template.toString(vars));
+            return template.toString(vars);
         } catch (URITemplateException e) {
             throw new IllegalStateException("wtf??", e);
         }

src/main/resources/com/github/fge/jsonschema/validator/validation.properties

@@ -78,5 +78,4 @@ err.format.UUID.invalid = input string "%s" is not a valid UUID
 #
 # Other messages
 #
-err.common.validationLoop = validation loop: same schema visited more than \
-  once for the same instance
+err.common.validationLoop = validation loop: schema "%s" visited twice for pointer "%s" into validated instance

src/test/java/com/github/fge/jsonschema/processors/validation/ValidationProcessorTest.java

@@ -26,10 +26,13 @@
 import com.github.fge.jackson.JacksonUtils;
 import com.github.fge.jackson.JsonLoader;
 import com.github.fge.jackson.NodeType;
+import com.github.fge.jackson.jsonpointer.JsonPointer;
+import com.github.fge.jackson.jsonpointer.JsonPointerException;
 import com.github.fge.jsonschema.cfg.ValidationConfiguration;
 import com.github.fge.jsonschema.core.exceptions.ProcessingException;
 import com.github.fge.jsonschema.core.keyword.syntax.checkers.SyntaxChecker;
 import com.github.fge.jsonschema.core.processing.Processor;
+import com.github.fge.jsonschema.core.report.ProcessingMessage;
 import com.github.fge.jsonschema.core.report.ProcessingReport;
 import com.github.fge.jsonschema.core.tree.CanonicalSchemaTree;
 import com.github.fge.jsonschema.core.tree.JsonTree;
@@ -49,8 +52,11 @@
 import org.testng.annotations.Test;
 
 import java.io.IOException;
+import java.net.URI;
+import java.util.Arrays;
 import java.util.concurrent.atomic.AtomicInteger;
 
+import static com.github.fge.jsonschema.matchers.ProcessingMessageAssert.assertMessage;
 import static org.mockito.Mockito.mock;
 import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertTrue;
@@ -120,23 +126,30 @@ public void childrenAreExploredOnDemandEvenIfContainerFails()
 
     @Test(timeOut = 1000)
     public void circularReferencingDuringValidationIsDetected()
-        throws IOException, ProcessingException
+        throws IOException, ProcessingException, JsonPointerException
     {
         final JsonNode schemaNode
             = JsonLoader.fromResource("/other/issue102.json");
         final JsonSchemaFactory factory = JsonSchemaFactory.byDefault();
         final JsonValidator validator = factory.getValidator();
         final MessageBundle bundle
             = MessageBundles.getBundle(JsonSchemaValidationBundle.class);
-        final String expectedMsg
-            = bundle.getMessage("err.common.validationLoop");
 
         try {
             validator.validate(schemaNode,
                 JacksonUtils.nodeFactory().nullNode());
             fail("No exception thrown!");
         } catch (ProcessingException e) {
-            assertEquals(e.getProcessingMessage().getMessage(), expectedMsg);
+            final URI uri = URI.create("#/oneOf/0");
+            final ProcessingMessage message = e.getProcessingMessage();
+            final String expectedMessage
+                = bundle.printf("err.common.validationLoop", uri, "");
+            assertMessage(message)
+                .hasMessage(expectedMessage)
+                .hasField("alreadyVisited", uri)
+                .hasField("instancePointer", JsonPointer.empty().toString())
+                .hasField("validationPath",
+                    Arrays.asList("#", "#/oneOf/0", "#/oneOf/1"));
         }
         assertTrue(true);
     }

src/test/resources/other/issue102.json

@@ -1,30 +1 @@
-{
-    "$schema": "http://json-schema.org/draft-04/schema#",
-    "definitions": {
-        "S": {
-            "allOf": [
-                {
-                    "type": "string",
-                    "enum": ["a"]
-                },
-                {
-                    "oneOf": [
-                        {
-                            "$ref": "#/definitions/S"
-                        },
-                        {
-                            "type": "string",
-                            "enum": [""]
-                        }
-                    ]
-                },
-                {
-                    "type": "string",
-                    "enum": ["b"]
-                }
-            ],
-            "additionalItems": false
-        }
-    },
-    "$ref": "#/definitions/S"
-}
+{ "oneOf": [ {}, { "$ref": "#" } ] }