From fba95cecdb2c4fc37917233dff3e40b07f4f1bf6 Mon Sep 17 00:00:00 2001
From: Robert Varga <robert.varga@pantheon.tech>
Date: Tue, 16 Apr 2024 16:03:32 +0200
Subject: [PATCH] Fix DependencyResolver

We have a problem with submodules: a submodule cannot be resolved until
its belongs-to module is resolved -- but a module cannot be resolved
until its include submodules are resolved.

Fix this by partially restoring previous logic -- i.e. we consider
submodules resolved without considering belongs-to. We then add a
separate pass to invalidate resolved modules.

JIRA: YANGTOOLS-1572
Change-Id: I3b54912fe0b9ed86b98037b0ce4551a021663199
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
---
 .../yang/parser/repo/DependencyResolver.java  | 36 ++++++++++++-------
 .../parser/repo/DependencyResolverTest.java   |  4 +--
 2 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/parser/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/repo/DependencyResolver.java b/parser/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/repo/DependencyResolver.java
index 52afb83080..53ca4e17ca 100644
--- a/parser/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/repo/DependencyResolver.java
+++ b/parser/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/repo/DependencyResolver.java
@@ -10,6 +10,7 @@ package org.opendaylight.yangtools.yang.parser.repo;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMultimap;
 import com.google.common.collect.Sets;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
@@ -22,10 +23,9 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 /**
- * Inter-module dependency resolved. Given a set of schema source identifiers and their
- * corresponding dependency information, the {@link #create(Map)} method creates a
- * a view of how consistent the dependencies are. In particular, this detects whether
- * any imports are unsatisfied.
+ * Inter-module dependency resolved. Given a set of schema source identifiers and their corresponding dependency
+ * information, the {@link #create(Map)} method creates a a view of how consistent the dependencies are. In particular,
+ * this detects whether any import/include/belongs-to statements are unsatisfied.
  */
 // FIXME: improve this class to track and expose how wildcard imports were resolved.
 //        That information will allow us to track "damage" to dependency resolution
@@ -37,10 +37,12 @@ abstract class DependencyResolver {
     private final ImmutableList<SourceIdentifier> unresolvedSources;
     private final ImmutableMultimap<SourceIdentifier, SourceDependency> unsatisfiedImports;
 
-    protected DependencyResolver(final Map<SourceIdentifier, SourceInfo> depInfo) {
+    DependencyResolver(final Map<SourceIdentifier, SourceInfo> depInfo) {
         final var resolved = Sets.<SourceIdentifier>newHashSetWithExpectedSize(depInfo.size());
         final var pending = new HashMap<>(depInfo);
+        final var submodules = new ArrayList<Submodule>();
 
+        // First pass: resolve 'include' and 'import' statements for each source
         boolean progress;
         do {
             progress = false;
@@ -54,10 +56,27 @@ abstract class DependencyResolver {
                     resolved.add(sourceId);
                     it.remove();
                     progress = true;
+
+                    // Stash submodules for second pass
+                    if (dep instanceof Submodule submodule) {
+                        submodules.add(submodule);
+                    }
                 }
             }
         } while (progress);
 
+        // Second pass: validate 'belongs-to' in submodules
+        for (var submodule : submodules) {
+            final var belongsTo = submodule.belongsTo();
+            if (!isKnown(resolved, belongsTo)) {
+                // belongs-to check failed, move the source back to pending
+                final var sourceId = submodule.sourceId();
+                LOG.debug("Source {} is missing belongs-to {}", sourceId, belongsTo);
+                pending.put(sourceId, submodule);
+                resolved.remove(sourceId);
+            }
+        }
+
         resolvedSources = ImmutableList.copyOf(resolved);
         unresolvedSources = ImmutableList.copyOf(pending.keySet());
 
@@ -125,13 +144,6 @@ abstract class DependencyResolver {
                 return false;
             }
         }
-        if (info instanceof Submodule submodule) {
-            final var dep = submodule.belongsTo();
-            if (!isKnown(resolved, dep)) {
-                LOG.debug("Source {} is missing belongs-to {}", info.sourceId(), dep);
-                return false;
-            }
-        }
         return true;
     }
 
diff --git a/parser/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/parser/repo/DependencyResolverTest.java b/parser/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/parser/repo/DependencyResolverTest.java
index 11fb9e1b6e..dde16145b9 100644
--- a/parser/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/parser/repo/DependencyResolverTest.java
+++ b/parser/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/parser/repo/DependencyResolverTest.java
@@ -55,10 +55,10 @@ class DependencyResolverTest {
             "/model/baz.yang");
         assertThat(resolved.resolvedSources()).containsExactlyInAnyOrder(
             new SourceIdentifier("bar", "2013-07-03"),
-            new SourceIdentifier("baz", "2013-02-27"));
-        assertThat(resolved.unresolvedSources()).containsExactlyInAnyOrder(
+            new SourceIdentifier("baz", "2013-02-27"),
             new SourceIdentifier("foo", "2013-02-27"),
             new SourceIdentifier("subfoo", "2013-02-27"));
+        assertEquals(List.of(), resolved.unresolvedSources());
         assertEquals(ImmutableMultimap.of(), resolved.unsatisfiedImports());
     }
 
-- 
2.36.6