GroupingDefinitionDependencySort needs to consider actions

In order to be able to correctly process actions/notifications
which can refer to other groupings, we must properly sort them
within each module.

To do that, GroupingDefinitionDependencySort must consider
uses nodes within them to properly construct the dependency
graph.

JIRA: MDSAL-448
Change-Id: I627702e39b1ab235b1c77ceaa2717ee03b3b2e39
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/yang/types/GroupingDefinitionDependencySort.java b/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/yang/types/GroupingDefinitionDependencySort.java
index f86d03bfd392b1c843d98244e90a4d80ae341a8c..c7aa607bb5cce422f283f1987f27cbb3566a2194 100644 (file)
--- a/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/yang/types/GroupingDefinitionDependencySort.java
+++ b/binding/mdsal-binding-generator-impl/src/main/java/org/opendaylight/mdsal/binding/yang/types/GroupingDefinitionDependencySort.java
@@ -16,12 +16,16 @@ import java.util.Map;
 import java.util.Set;
 import org.opendaylight.yangtools.util.TopologicalSort;
 import org.opendaylight.yangtools.util.TopologicalSort.Node;
+import org.opendaylight.yangtools.yang.model.api.ActionDefinition;
+import org.opendaylight.yangtools.yang.model.api.ActionNodeContainer;
 import org.opendaylight.yangtools.yang.model.api.AugmentationSchemaNode;
 import org.opendaylight.yangtools.yang.model.api.CaseSchemaNode;
 import org.opendaylight.yangtools.yang.model.api.ChoiceSchemaNode;
 import org.opendaylight.yangtools.yang.model.api.DataNodeContainer;
 import org.opendaylight.yangtools.yang.model.api.DataSchemaNode;
 import org.opendaylight.yangtools.yang.model.api.GroupingDefinition;
+import org.opendaylight.yangtools.yang.model.api.NotificationDefinition;
+import org.opendaylight.yangtools.yang.model.api.NotificationNodeContainer;
 import org.opendaylight.yangtools.yang.model.api.SchemaPath;
 import org.opendaylight.yangtools.yang.model.api.UsesNode;
 
@@ -114,8 +118,7 @@ public class GroupingDefinitionDependencySort {
                 ret.addAll(getAllUsesNodes(augment));
             }
         }
-        Set<GroupingDefinition> groupings = container.getGroupings();
-        for (GroupingDefinition groupingDefinition : groupings) {
+        for (GroupingDefinition groupingDefinition : container.getGroupings()) {
             ret.addAll(getAllUsesNodes(groupingDefinition));
         }
         for (DataSchemaNode childNode : container.getChildNodes()) {
@@ -127,6 +130,18 @@ public class GroupingDefinitionDependencySort {
                 }
             }
         }
+        if (container instanceof ActionNodeContainer) {
+            for (ActionDefinition action : ((ActionNodeContainer) container).getActions()) {
+                ret.addAll(getAllUsesNodes(action.getInput()));
+                ret.addAll(getAllUsesNodes(action.getOutput()));
+            }
+        }
+        if (container instanceof NotificationNodeContainer) {
+            for (NotificationDefinition notification : ((NotificationNodeContainer) container).getNotifications()) {
+                ret.addAll(getAllUsesNodes(notification));
+            }
+        }
+
         return ret;
     }
 }

diff --git a/binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/generator/impl/Mdsal448Test.java b/binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/generator/impl/Mdsal448Test.java
new file mode 100644 (file)
index 0000000..14d66b1
--- /dev/null
+++ b/binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/generator/impl/Mdsal448Test.java
@@ -0,0 +1,52 @@
+/*
+ * Copyright (c) 2019 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.mdsal.binding.generator.impl;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+
+import com.google.common.collect.Iterables;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Set;
+import org.junit.Test;
+import org.opendaylight.mdsal.binding.model.api.Type;
+import org.opendaylight.mdsal.binding.yang.types.GroupingDefinitionDependencySort;
+import org.opendaylight.yangtools.yang.model.api.GroupingDefinition;
+import org.opendaylight.yangtools.yang.model.api.SchemaContext;
+import org.opendaylight.yangtools.yang.test.util.YangParserTestUtils;
+
+public class Mdsal448Test {
+
+    @Test
+    public void groupingSortIncludesActions() {
+        final SchemaContext context = YangParserTestUtils.parseYangResource("/mdsal448.yang");
+        final Set<GroupingDefinition> groupings = context.findModule("mdsal448").get().getGroupings();
+        assertEquals(2, groupings.size());
+
+        final List<GroupingDefinition> ordered = sortGroupings(Iterables.get(groupings, 0),
+            Iterables.get(groupings, 1));
+        assertEquals(2, ordered.size());
+        // "the-grouping" needs to be first
+        assertEquals("the-grouping", ordered.get(0).getQName().getLocalName());
+        assertEquals("action-grouping", ordered.get(1).getQName().getLocalName());
+
+        // Sort needs to be stable
+        final List<GroupingDefinition> reverse = sortGroupings(Iterables.get(groupings, 1),
+            Iterables.get(groupings, 0));
+        assertEquals(ordered, reverse);
+
+        final List<Type> types = new BindingGeneratorImpl().generateTypes(context);
+        assertNotNull(types);
+        assertEquals(9, types.size());
+    }
+
+    private static List<GroupingDefinition> sortGroupings(GroupingDefinition... groupings) {
+        return new GroupingDefinitionDependencySort().sort(Arrays.asList(groupings));
+    }
+}

diff --git a/binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/yang/types/GroupingDefinitionDependencySortTest.java b/binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/yang/types/GroupingDefinitionDependencySortTest.java
index 1acfedc7f3a12bb22970db3ce32adc518bb164c4..cbf5dfca742a9b333e12d0314f4f0095176a9453 100644 (file)
--- a/binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/yang/types/GroupingDefinitionDependencySortTest.java
+++ b/binding/mdsal-binding-generator-impl/src/test/java/org/opendaylight/mdsal/binding/yang/types/GroupingDefinitionDependencySortTest.java
@@ -26,10 +26,12 @@ public class GroupingDefinitionDependencySortTest {
     @Rule
     public ExpectedException expException = ExpectedException.none();
 
+    private final GroupingDefinitionDependencySort groupingDefinitionDependencySort =
+            new GroupingDefinitionDependencySort();
+
     @Test
     public void testSortMethod() {
-        final GroupingDefinitionDependencySort groupingDefinitionDependencySort =
-                new GroupingDefinitionDependencySort();
+
         final List<GroupingDefinition> unsortedGroupingDefs = new ArrayList<>();
 
         GroupingDefinition grp1 = mock(GroupingDefinition.class);
@@ -38,6 +40,8 @@ public class GroupingDefinitionDependencySortTest {
         doReturn(Collections.emptySet()).when(grp1).getUses();
         doReturn(Collections.emptySet()).when(grp1).getGroupings();
         doReturn(Collections.emptySet()).when(grp1).getChildNodes();
+        doReturn(Collections.emptySet()).when(grp1).getActions();
+        doReturn(Collections.emptySet()).when(grp1).getNotifications();
 
         GroupingDefinition grp2 = mock(GroupingDefinition.class);
         doReturn(SchemaPath.create(false, QName.create("", "Cont1"))).when(grp2).getPath();
@@ -45,6 +49,8 @@ public class GroupingDefinitionDependencySortTest {
         doReturn(Collections.emptySet()).when(grp2).getUses();
         doReturn(Collections.emptySet()).when(grp2).getGroupings();
         doReturn(Collections.emptySet()).when(grp2).getChildNodes();
+        doReturn(Collections.emptySet()).when(grp2).getActions();
+        doReturn(Collections.emptySet()).when(grp2).getNotifications();
 
         GroupingDefinition grp3 = mock(GroupingDefinition.class);
         doReturn(SchemaPath.create(false, QName.create("", "Cont1"), QName.create("", "Cont2"))).when(grp3).getPath();
@@ -52,6 +58,8 @@ public class GroupingDefinitionDependencySortTest {
         doReturn(Collections.emptySet()).when(grp3).getUses();
         doReturn(Collections.emptySet()).when(grp3).getGroupings();
         doReturn(Collections.emptySet()).when(grp3).getChildNodes();
+        doReturn(Collections.emptySet()).when(grp3).getActions();
+        doReturn(Collections.emptySet()).when(grp3).getNotifications();
 
         GroupingDefinition grp4 = mock(GroupingDefinition.class);
         doReturn(SchemaPath.create(false, QName.create("", "Cont1"), QName.create("", "Cont2"),
@@ -60,6 +68,8 @@ public class GroupingDefinitionDependencySortTest {
         doReturn(Collections.emptySet()).when(grp4).getUses();
         doReturn(Collections.emptySet()).when(grp4).getGroupings();
         doReturn(Collections.emptySet()).when(grp4).getChildNodes();
+        doReturn(Collections.emptySet()).when(grp4).getActions();
+        doReturn(Collections.emptySet()).when(grp4).getNotifications();
 
         GroupingDefinition grp5 = mock(GroupingDefinition.class);
         doReturn(SchemaPath.create(false, QName.create("", "Cont1"))).when(grp5).getPath();
@@ -67,6 +77,8 @@ public class GroupingDefinitionDependencySortTest {
         doReturn(Collections.emptySet()).when(grp5).getUses();
         doReturn(Collections.emptySet()).when(grp5).getGroupings();
         doReturn(Collections.emptySet()).when(grp5).getChildNodes();
+        doReturn(Collections.emptySet()).when(grp5).getActions();
+        doReturn(Collections.emptySet()).when(grp5).getNotifications();
 
         unsortedGroupingDefs.add(grp1);
         unsortedGroupingDefs.add(grp1);
@@ -77,7 +89,10 @@ public class GroupingDefinitionDependencySortTest {
 
         List<GroupingDefinition> sortedGroupingDefs = groupingDefinitionDependencySort.sort(unsortedGroupingDefs);
         assertNotNull(sortedGroupingDefs);
+    }
 
+    @Test
+    public void testNullSort() {
         expException.expect(IllegalArgumentException.class);
         expException.expectMessage("Set of Type Definitions cannot be NULL!");
         groupingDefinitionDependencySort.sort(null);

diff --git a/binding/mdsal-binding-generator-impl/src/test/resources/mdsal448.yang b/binding/mdsal-binding-generator-impl/src/test/resources/mdsal448.yang
new file mode 100644 (file)
index 0000000..d2d1fc4
--- /dev/null
+++ b/binding/mdsal-binding-generator-impl/src/test/resources/mdsal448.yang
@@ -0,0 +1,35 @@
+module mdsal448 {
+    yang-version "1.1";
+    namespace "urn:example:test";
+    prefix "test";
+
+    grouping the-grouping {
+        leaf the-leaf {
+            type string;
+        }
+    }
+
+    grouping action-grouping {
+        action action-with-grouping {
+            input {
+                leaf leaf1 {
+                    type string;
+                }
+
+                uses the-grouping;
+            }
+        }
+    }
+
+    container network {
+        list node {
+            key "id";
+
+            leaf id {
+                type string;
+            }
+
+            uses action-grouping;
+        }
+    }
+}