Fix YANG export with duplicate imports

When we have two submodules not agreeing on the prefix used to import a
module, we get a nasty splat from StatementPrefixResolver. Fix this by
detecting when this is happens and skip to full resolution magic.

JIRA: YANGTOOLS-1313
Change-Id: I99cc3ebc2f560590d05b51287d08429aaf7d6582
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
Signed-off-by: Dominik Vrbovsky <dominik.vrbovsky@pantheon.tech>
(cherry picked from commit d08e5552beb9180f582bf419fdcf120052876411)

diff --git a/yang/yang-model-export/src/main/java/org/opendaylight/yangtools/yang/model/export/StatementPrefixResolver.java b/yang/yang-model-export/src/main/java/org/opendaylight/yangtools/yang/model/export/StatementPrefixResolver.java
index 34332c7b66f4ee35de28e5a990942eb612f05e0b..bf242599c7d6c6e2d71bee55597da98f0c91ad10 100644 (file)
--- a/yang/yang-model-export/src/main/java/org/opendaylight/yangtools/yang/model/export/StatementPrefixResolver.java
+++ b/yang/yang-model-export/src/main/java/org/opendaylight/yangtools/yang/model/export/StatementPrefixResolver.java
@@ -12,6 +12,7 @@ import static com.google.common.base.Verify.verify;
 import static java.util.Objects.requireNonNull;
 
 import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.HashMultimap;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableMap.Builder;
 import com.google.common.collect.Maps;
@@ -42,7 +43,7 @@ final class StatementPrefixResolver {
         private final Collection<Entry<DeclaredStatement<?>, String>> statements;
 
         Conflict(final Collection<Entry<DeclaredStatement<?>, String>> entries) {
-            this.statements = requireNonNull(entries);
+            statements = requireNonNull(entries);
         }
 
         @Nullable String findPrefix(final DeclaredStatement<?> stmt) {
@@ -66,11 +67,11 @@ final class StatementPrefixResolver {
     private final Map<QNameModule, ?> lookup;
 
     private StatementPrefixResolver(final Map<QNameModule, String> map) {
-        this.lookup = ImmutableMap.copyOf(map);
+        lookup = ImmutableMap.copyOf(map);
     }
 
     private StatementPrefixResolver(final ImmutableMap<QNameModule, ?> map) {
-        this.lookup = requireNonNull(map);
+        lookup = requireNonNull(map);
     }
 
     static StatementPrefixResolver forModule(final ModuleEffectiveStatement module) {
@@ -89,7 +90,15 @@ final class StatementPrefixResolver {
             indexPrefixes(prefixToNamespaces, submodule.getAll(QNameModuleToPrefixNamespace.class), submodule);
         }
 
-        // Stage two: resolve first order of conflicts, potentially completely resolving mappings...
+        // Stage two: see what QNameModule -> prefix mappings there are. We will need to understand this in step three
+        final Multimap<QNameModule, String> namespaceToPrefixes = HashMultimap.create();
+        for (Entry<String, Multimap<QNameModule, EffectiveStatement<?,?>>> entry : prefixToNamespaces.entrySet()) {
+            for (QNameModule namespace : entry.getValue().keySet()) {
+                namespaceToPrefixes.put(namespace, entry.getKey());
+            }
+        }
+
+        // Stage three: resolve first order of conflicts, potentially completely resolving mappings...
         final Builder<QNameModule, Object> builder = ImmutableMap.builderWithExpectedSize(prefixToNamespaces.size());
 
         // ... first resolve unambiguous mappings ...
@@ -99,8 +108,12 @@ final class StatementPrefixResolver {
             final Entry<String, Multimap<QNameModule, EffectiveStatement<?, ?>>> entry = it.next();
             final Multimap<QNameModule, EffectiveStatement<?, ?>> modules = entry.getValue();
             if (modules.size() == 1) {
-                builder.put(modules.keys().iterator().next(), entry.getKey());
-                it.remove();
+                // Careful now: the namespace needs to be unambiguous
+                final QNameModule namespace = modules.keys().iterator().next();
+                if (namespaceToPrefixes.get(namespace).size() == 1) {
+                    builder.put(namespace, entry.getKey());
+                    it.remove();
+                }
             }
         }
 

diff --git a/yang/yang-model-export/src/test/java/org/opendaylight/yangtools/yang/model/export/YT1313Test.java b/yang/yang-model-export/src/test/java/org/opendaylight/yangtools/yang/model/export/YT1313Test.java
new file mode 100644 (file)
index 0000000..cb33cd9
--- /dev/null
+++ b/yang/yang-model-export/src/test/java/org/opendaylight/yangtools/yang/model/export/YT1313Test.java
@@ -0,0 +1,27 @@
+/*
+ * Copyright (c) 2021 PANTHEON.tech, s.r.o. and others.  All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.opendaylight.yangtools.yang.model.export;
+
+import static org.junit.Assert.assertNotNull;
+
+import java.net.URI;
+import org.junit.Test;
+import org.opendaylight.yangtools.yang.common.QNameModule;
+import org.opendaylight.yangtools.yang.model.api.stmt.ModuleEffectiveStatement;
+import org.opendaylight.yangtools.yang.test.util.YangParserTestUtils;
+
+public class YT1313Test {
+    @Test
+    public void testSubmoduleImportPrefixes() {
+        final ModuleEffectiveStatement bar = YangParserTestUtils.parseYangResourceDirectory("/bugs/yt1313")
+            .getModuleStatements().get(QNameModule.create(URI.create("bar")));
+
+        final StatementPrefixResolver resolver = StatementPrefixResolver.forModule(bar);
+        assertNotNull(resolver);
+    }
+}

diff --git a/yang/yang-model-export/src/test/resources/bugs/yt1313/bar-one.yang b/yang/yang-model-export/src/test/resources/bugs/yt1313/bar-one.yang
new file mode 100644 (file)
index 0000000..9dc622d
--- /dev/null
+++ b/yang/yang-model-export/src/test/resources/bugs/yt1313/bar-one.yang
@@ -0,0 +1,13 @@
+submodule bar-one {
+  belongs-to bar {
+    prefix bar;
+  }
+
+  import foo {
+    prefix foo1;
+  }
+
+  leaf one {
+    type foo1:foo;
+  }
+}

diff --git a/yang/yang-model-export/src/test/resources/bugs/yt1313/bar-two.yang b/yang/yang-model-export/src/test/resources/bugs/yt1313/bar-two.yang
new file mode 100644 (file)
index 0000000..5503b5c
--- /dev/null
+++ b/yang/yang-model-export/src/test/resources/bugs/yt1313/bar-two.yang
@@ -0,0 +1,13 @@
+submodule bar-two {
+  belongs-to bar {
+    prefix bar;
+  }
+
+  import foo {
+    prefix foo2;
+  }
+
+  leaf two {
+    type foo2:foo;
+  }
+}

diff --git a/yang/yang-model-export/src/test/resources/bugs/yt1313/bar.yang b/yang/yang-model-export/src/test/resources/bugs/yt1313/bar.yang
new file mode 100644 (file)
index 0000000..6df4097
--- /dev/null
+++ b/yang/yang-model-export/src/test/resources/bugs/yt1313/bar.yang
@@ -0,0 +1,8 @@
+module bar {
+  namespace bar;
+  prefix bar;
+
+  include bar-one;
+  include bar-two;
+}
+  

diff --git a/yang/yang-model-export/src/test/resources/bugs/yt1313/foo.yang b/yang/yang-model-export/src/test/resources/bugs/yt1313/foo.yang
new file mode 100644 (file)
index 0000000..e34ae2f
--- /dev/null
+++ b/yang/yang-model-export/src/test/resources/bugs/yt1313/foo.yang
@@ -0,0 +1,9 @@
+module foo {
+  namespace foo;
+  prefix foo;
+
+  typedef foo {
+    type string;
+  }
+}
+