tests fixed

Author: simbo1905

AGENTS.md

@@ -59,6 +59,8 @@ $(command -v mvnd || command -v mvn || command -v ./mvnw) -Dtest=BlahTest#testSo
 $(command -v mvnd || command -v mvn || command -v ./mvnw) -pl json-java21-api-tracker -Dtest=ApiTrackerTest -Djava.util.logging.ConsoleHandler.level=FINE
 ```
 
+IMPORTANT: Fix the method with FINEST logging, then fix the test class with FINER logging, then fix the module with FINE logging, then run the whole suite with INFO logging. THERE IS NO TRIVIAL LOGIC LEFT IN THIS PROJECT TO BE SYSTEMATIC. 
+
 ### Output Visibility Requirements
 
 - You MUST NEVER pipe build or test output to tools (head, tail, grep, etc.) that reduce visibility. Logging uncovers the unexpected; piping hides it. Use the instructions above to zoom in on what you want to see using `-Dtest=BlahTest` and `-Dtest=BlahTest#testSomething` passing the appropriate `Djava.util.logging.ConsoleHandler.level=XXX` to avoid too much outputs

json-java21-schema/src/main/java/io/github/simbo1905/json/schema/JsonSchema.java

@@ -422,6 +422,7 @@ static CompiledRegistry compileWorkStack(JsonValue initialJson,
     Deque<java.net.URI> workStack = new ArrayDeque<>();
     Map<java.net.URI, CompiledRoot> built = new NormalizedUriMap(new LinkedHashMap<>());
     Set<java.net.URI> active = new HashSet<>();
+    Map<java.net.URI, java.net.URI> parentMap = new HashMap<>();
 
     LOG.finest(() -> "compileWorkStack: initialized workStack=" + workStack + ", built=" + built + ", active=" + active);
 
@@ -481,7 +482,7 @@ static CompiledRegistry compileWorkStack(JsonValue initialJson,
           if (refToken instanceof RefToken.RemoteRef remoteRef) {
             LOG.finest(() -> "compileWorkStack: processing RemoteRef object=" + remoteRef + ", base=" + remoteRef.baseUri() + ", target=" + remoteRef.targetUri());
             java.net.URI targetDocUri = normalizeUri(finalCurrentUri, remoteRef.targetUri().toString());
-            boolean scheduled = scheduleRemoteIfUnseen(finalWorkStack, finalBuilt, targetDocUri);
+            boolean scheduled = scheduleRemoteIfUnseen(finalWorkStack, finalBuilt, parentMap, finalCurrentUri, targetDocUri);
             LOG.finer(() -> "compileWorkStack: remote ref scheduled=" + scheduled + ", target=" + targetDocUri);
           }
         }, built, options, compileOptions);
@@ -650,12 +651,23 @@ public String pointer() {
   }
 
   /// Schedule remote document for compilation if not seen before
-  static boolean scheduleRemoteIfUnseen(Deque<java.net.URI> workStack, Map<java.net.URI, CompiledRoot> built, java.net.URI targetDocUri) {
+  static boolean scheduleRemoteIfUnseen(Deque<java.net.URI> workStack,
+                                        Map<java.net.URI, CompiledRoot> built,
+                                        Map<java.net.URI, java.net.URI> parentMap,
+                                        java.net.URI currentDocUri,
+                                        java.net.URI targetDocUri) {
     LOG.finer(() -> "scheduleRemoteIfUnseen: target=" + targetDocUri + ", workStack.size=" + workStack.size() + ", built.size=" + built.size());
     LOG.finest(() -> "scheduleRemoteIfUnseen: targetDocUri object=" + targetDocUri + ", scheme=" + targetDocUri.getScheme() + ", host=" + targetDocUri.getHost() + ", path=" + targetDocUri.getPath());
     LOG.finest(() -> "scheduleRemoteIfUnseen: workStack object=" + workStack + ", contents=" + workStack.stream().map(Object::toString).collect(java.util.stream.Collectors.joining(", ", "[", "]")));
     LOG.finest(() -> "scheduleRemoteIfUnseen: built map object=" + built + ", keys=" + built.keySet() + ", size=" + built.size());
 
+    // Detect remote cycles by walking parent chain
+    if (formsRemoteCycle(parentMap, currentDocUri, targetDocUri)) {
+      String cycleMessage = "ERROR: CYCLE: remote $ref cycle current=" + currentDocUri + ", target=" + targetDocUri;
+      LOG.severe(() -> cycleMessage);
+      throw new IllegalArgumentException(cycleMessage);
+    }
+
     // Check if already built or already in work stack
     boolean alreadyBuilt = built.containsKey(targetDocUri);
     boolean inWorkStack = workStack.contains(targetDocUri);
@@ -667,13 +679,37 @@ static boolean scheduleRemoteIfUnseen(Deque<java.net.URI> workStack, Map<java.ne
       return false;
     }
 
+    // Track parent chain for cycle detection before scheduling child
+    parentMap.putIfAbsent(targetDocUri, currentDocUri);
+
     // Add to work stack
     workStack.push(targetDocUri);
     LOG.finer(() -> "scheduleRemoteIfUnseen: scheduled remote document: " + targetDocUri);
     LOG.finest(() -> "scheduleRemoteIfUnseen: workStack after push=" + workStack + ", contents=" + workStack.stream().map(Object::toString).collect(java.util.stream.Collectors.joining(", ", "[", "]")));
     return true;
   }
 
+  private static boolean formsRemoteCycle(Map<java.net.URI, java.net.URI> parentMap,
+                                          java.net.URI currentDocUri,
+                                          java.net.URI targetDocUri) {
+    if (currentDocUri.equals(targetDocUri)) {
+      return true;
+    }
+
+    java.net.URI cursor = currentDocUri;
+    while (cursor != null) {
+      java.net.URI parent = parentMap.get(cursor);
+      if (parent == null) {
+        break;
+      }
+      if (parent.equals(targetDocUri)) {
+        return true;
+      }
+      cursor = parent;
+    }
+    return false;
+  }
+
   /// Detect and throw on compile-time cycles
   static void detectAndThrowCycle(Set<java.net.URI> active, java.net.URI docUri, String pathTrail) {
     LOG.finest(() -> "detectAndThrowCycle: active set=" + active + ", docUri=" + docUri + ", pathTrail='" + pathTrail + "'");
@@ -1464,6 +1500,7 @@ private static final class Session {
       final Map<String, JsonSchema> definitions = new LinkedHashMap<>();
       final Map<String, JsonSchema> compiledByPointer = new LinkedHashMap<>();
       final Map<String, JsonValue> rawByPointer = new LinkedHashMap<>();
+      final Map<java.net.URI, java.net.URI> parentMap = new LinkedHashMap<>();
       JsonSchema currentRootSchema;
       Options currentOptions;
       long totalFetchedBytes;
@@ -1473,7 +1510,8 @@ private static final class Session {
     private static java.net.URI stripFragment(java.net.URI uri) {
       String s = uri.toString();
       int i = s.indexOf('#');
-      return i >= 0 ? java.net.URI.create(s.substring(0, i)) : uri;
+      java.net.URI base = i >= 0 ? java.net.URI.create(s.substring(0, i)) : uri;
+      return base.normalize();
     }
     // removed static mutable state; state now lives in Session
 
@@ -1953,13 +1991,26 @@ private static JsonSchema compileInternalWithContext(Session session, JsonValue
           // Handle remote refs by adding to work stack
           if (refToken instanceof RefToken.RemoteRef remoteRef) {
             LOG.finer(() -> "Remote ref detected: " + remoteRef.targetUri());
-            // Get document URI without fragment
             java.net.URI targetDocUri = stripFragment(remoteRef.targetUri());
-            if (!seenUris.contains(targetDocUri)) {
+            LOG.fine(() -> "Remote ref scheduling from docUri=" + docUri + " to target=" + targetDocUri);
+            LOG.finest(() -> "Remote ref parentMap before cycle check: " + session.parentMap);
+            if (formsRemoteCycle(session.parentMap, docUri, targetDocUri)) {
+              String cycleMessage = "ERROR: CYCLE: remote $ref cycle current=" + docUri + ", target=" + targetDocUri;
+              LOG.severe(() -> cycleMessage);
+              throw new IllegalArgumentException(cycleMessage);
+            }
+            boolean alreadySeen = seenUris.contains(targetDocUri);
+            LOG.finest(() -> "Remote ref alreadySeen=" + alreadySeen + " for target=" + targetDocUri);
+            if (!alreadySeen) {
               workStack.push(new WorkItem(targetDocUri));
               seenUris.add(targetDocUri);
+              session.parentMap.putIfAbsent(targetDocUri, docUri);
               LOG.finer(() -> "Added to work stack: " + targetDocUri);
+            } else {
+              session.parentMap.putIfAbsent(targetDocUri, docUri);
+              LOG.finer(() -> "Remote ref already scheduled or compiled: " + targetDocUri);
             }
+            LOG.finest(() -> "Remote ref parentMap after scheduling: " + session.parentMap);
             LOG.finest(() -> "compileInternalWithContext: Creating RefSchema for remote ref " + remoteRef.targetUri());
 
             LOG.fine(() -> "Creating RefSchema for remote ref " + remoteRef.targetUri() +

json-java21-schema/src/test/java/io/github/simbo1905/json/schema/JsonSchemaRemoteRefTest.java

@@ -295,18 +295,18 @@ void detects_cross_document_cycle() {
         ));
         final var options = JsonSchema.CompileOptions.remoteDefaults(fetcher);
 
-        LOG.finer(() -> "Compiling schema expecting cycle resolution");
-        final var schema = JsonSchema.compile(
-            toJson("""
-            {"$ref":"file:///JsonSchemaRemoteRefTest/a.json"}
-            """),
-            JsonSchema.Options.DEFAULT,
-            options
-        );
-
-        final var result = schema.validate(toJson("true"));
-        logResult("validate-true", result);
-        assertThat(result.valid()).isTrue();
+        LOG.finer(() -> "Compiling schema expecting cycle detection");
+        try (CapturedLogs logs = captureLogs(java.util.logging.Level.SEVERE)) {
+            assertThatThrownBy(() -> JsonSchema.compile(
+                toJson("""
+                {"$ref":"file:///JsonSchemaRemoteRefTest/a.json"}
+                """),
+                JsonSchema.Options.DEFAULT,
+                options
+            )).isInstanceOf(IllegalArgumentException.class)
+                .hasMessageContaining("ERROR: CYCLE: remote $ref cycle");
+            assertThat(logs.lines().stream().anyMatch(line -> line.startsWith("ERROR: CYCLE:"))).isTrue();
+        }
     }
 
     @Test