Fix CollisionDomain chaining

A simple augmentation of RPC input fails becase we end up having a
secondary (augment) being attached to another secondary (input), which
triggers a verify error.

Generalize the concept of having secondaries, so that we can have a
secondary -> secondary transition.

Change-Id: I2c1ca5b5473d3db56516385f4957ff11dcf512a0
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/impl/reactor/CollisionDomain.java b/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/impl/reactor/CollisionDomain.java
index 99b31b2e5501f41d067dc5cba75f2654fa6f251b..c79485fd75da9e022eec049a94562e217f5add99 100644 (file)
--- a/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/impl/reactor/CollisionDomain.java
+++ b/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/generator/impl/reactor/CollisionDomain.java
@@ -8,7 +8,6 @@
 package org.opendaylight.mdsal.binding.generator.impl.reactor;
 
 import static com.google.common.base.Preconditions.checkState;
-import static com.google.common.base.Verify.verify;
 import static java.util.Objects.requireNonNull;
 
 import com.google.common.base.MoreObjects;
@@ -28,9 +27,17 @@ import org.opendaylight.yangtools.yang.model.api.stmt.SchemaNodeIdentifier;
 
 final class CollisionDomain {
     abstract class Member {
+        private List<Secondary> secondaries = List.of();
         private String currentPackage;
         private String currentClass;
 
+        final void addSecondary(final Secondary secondary) {
+            if (secondaries.isEmpty()) {
+                secondaries = new ArrayList<>();
+            }
+            secondaries.add(requireNonNull(secondary));
+        }
+
         final @NonNull String currentClass() {
             if (currentClass == null) {
                 currentClass = computeCurrentClass();
@@ -53,6 +60,11 @@ final class CollisionDomain {
             solved = false;
             currentClass = null;
             currentPackage = null;
+
+            for (Secondary secondary : secondaries) {
+                secondary.primaryConflict();
+            }
+
             return true;
         }
 
@@ -68,7 +80,6 @@ final class CollisionDomain {
 
     private class Primary extends Member {
         private ClassNamingStrategy strategy;
-        private List<Secondary> secondaries = List.of();
 
         Primary(final ClassNamingStrategy strategy) {
             this.strategy = requireNonNull(strategy);
@@ -84,12 +95,6 @@ final class CollisionDomain {
             return packageString(strategy.nodeIdentifier());
         }
 
-        final void addSecondary(final Secondary secondary) {
-            if (secondaries.isEmpty()) {
-                secondaries = new ArrayList<>();
-            }
-            secondaries.add(requireNonNull(secondary));
-        }
 
         @Override
         final boolean signalConflict() {
@@ -99,11 +104,7 @@ final class CollisionDomain {
             }
 
             strategy = newStrategy;
-            super.signalConflict();
-            for (Secondary secondary : secondaries) {
-                secondary.primaryConflict();
-            }
-            return true;
+            return super.signalConflict();
         }
 
         @Override
@@ -120,9 +121,9 @@ final class CollisionDomain {
 
     private abstract class Secondary extends Member {
         private final String classSuffix;
-        final Primary classPrimary;
+        final Member classPrimary;
 
-        Secondary(final Primary primary, final String classSuffix) {
+        Secondary(final Member primary, final String classSuffix) {
             this.classPrimary = requireNonNull(primary);
             this.classSuffix = requireNonNull(classSuffix);
             primary.addSecondary(this);
@@ -144,7 +145,7 @@ final class CollisionDomain {
     }
 
     private final class LeafSecondary extends Secondary {
-        LeafSecondary(final Primary classPrimary, final String classSuffix) {
+        LeafSecondary(final Member classPrimary, final String classSuffix) {
             super(classPrimary, classSuffix);
         }
 
@@ -158,7 +159,7 @@ final class CollisionDomain {
     private final class SuffixSecondary extends Secondary {
         private final AbstractQName packageSuffix;
 
-        SuffixSecondary(final Primary primaryClass, final String classSuffix, final AbstractQName packageSuffix) {
+        SuffixSecondary(final Member primaryClass, final String classSuffix, final AbstractQName packageSuffix) {
             super(primaryClass, classSuffix);
             this.packageSuffix = requireNonNull(packageSuffix);
         }
@@ -172,7 +173,7 @@ final class CollisionDomain {
     private final class AugmentSecondary extends Secondary {
         private final SchemaNodeIdentifier packageSuffix;
 
-        AugmentSecondary(final Primary primary, final String classSuffix, final SchemaNodeIdentifier packageSuffix) {
+        AugmentSecondary(final Member primary, final String classSuffix, final SchemaNodeIdentifier packageSuffix) {
             super(primary, classSuffix);
             this.packageSuffix = requireNonNull(packageSuffix);
         }
@@ -203,21 +204,16 @@ final class CollisionDomain {
     }
 
     @NonNull Member addSecondary(final Member primary, final String classSuffix) {
-        return addMember(new LeafSecondary(castPrimary(primary), classSuffix));
+        return addMember(new LeafSecondary(primary, classSuffix));
     }
 
     @NonNull Member addSecondary(final Member primary, final String classSuffix, final AbstractQName packageSuffix) {
-        return addMember(new SuffixSecondary(castPrimary(primary), classSuffix, packageSuffix));
+        return addMember(new SuffixSecondary(primary, classSuffix, packageSuffix));
     }
 
     @NonNull Member addSecondary(final Member classPrimary, final String classSuffix,
             final SchemaNodeIdentifier packageSuffix) {
-        return addMember(new AugmentSecondary(castPrimary(classPrimary), classSuffix, packageSuffix));
-    }
-
-    private static @NonNull Primary castPrimary(final Member primary) {
-        verify(primary instanceof Primary, "Unexpected primary %s", primary);
-        return (Primary) primary;
+        return addMember(new AugmentSecondary(classPrimary, classSuffix, packageSuffix));
     }
 
     /*

diff --git a/binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/generator/impl/GeneratedTypesTest.java b/binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/generator/impl/GeneratedTypesTest.java
index c96f08314d56c4e3478feeee9cfee4fa7d5e83ac..caf1d27a5dd7f335a68260c56af553bdf2801515 100644 (file)
--- a/binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/generator/impl/GeneratedTypesTest.java
+++ b/binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/generator/impl/GeneratedTypesTest.java
@@ -371,4 +371,11 @@ public class GeneratedTypesTest {
         assertEquals(11, genTypesCount);
         assertEquals(3, genTOsCount);
     }
+
+    @Test
+    public void testAugmentRpcInput() {
+        final List<GeneratedType> genTypes = DefaultBindingGenerator.generateFor(YangParserTestUtils.parseYangResource(
+            "/augment-rpc-input.yang"));
+        assertEquals(5, genTypes.size());
+    }
 }

diff --git a/binding/mdsal-binding-generator-impl/src/test/resources/augment-rpc-input.yang b/binding/mdsal-binding-generator-impl/src/test/resources/augment-rpc-input.yang
new file mode 100644 (file)
index 0000000..98001c0
--- /dev/null
+++ b/binding/mdsal-binding-generator-impl/src/test/resources/augment-rpc-input.yang
@@ -0,0 +1,10 @@
+module foo {
+  namespace foo;
+  prefix foo;
+
+  rpc foo;
+
+  augment /foo/input {
+    container bar;
+  }
+}