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>
(cherry picked from commit 6fca42e6bde88795354a2a2386bc7a6d76aaec47)

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 29d8e6699c603af079010eb57cb839ce8f0ab029..b663345eb376bc799fb3bd88834d4fbcc74803a0 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
@@ -17,12 +17,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;
 
@@ -137,8 +141,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()) {
@@ -150,6 +153,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 748f873ea29eae90257c9fc6e24612e632ccf733..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,46 +26,59 @@ public class GroupingDefinitionDependencySortTest {
     @Rule
     public ExpectedException expException = ExpectedException.none();
 
+    private final GroupingDefinitionDependencySort groupingDefinitionDependencySort =
+            new GroupingDefinitionDependencySort();
+
     @Test
     public void testSortMethod() {
-        GroupingDefinitionDependencySort groupingDefinitionDependencySort = new GroupingDefinitionDependencySort();
-        List<GroupingDefinition> unsortedGroupingDefs = new ArrayList<>();
+
+        final List<GroupingDefinition> unsortedGroupingDefs = new ArrayList<>();
 
         GroupingDefinition grp1 = mock(GroupingDefinition.class);
         doReturn(SchemaPath.create(false, QName.create("", "Cont1"), QName.create("", "Cont2"))).when(grp1).getPath();
         doReturn(QName.create("", "leaf1")).when(grp1).getQName();
-        doReturn(Collections.EMPTY_SET).when(grp1).getUses();
-        doReturn(Collections.EMPTY_SET).when(grp1).getGroupings();
-        doReturn(Collections.EMPTY_SET).when(grp1).getChildNodes();
+        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();
         doReturn(QName.create("", "leaf2")).when(grp2).getQName();
-        doReturn(Collections.EMPTY_SET).when(grp2).getUses();
-        doReturn(Collections.EMPTY_SET).when(grp2).getGroupings();
-        doReturn(Collections.EMPTY_SET).when(grp2).getChildNodes();
+        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();
         doReturn(QName.create("", "leaf3")).when(grp3).getQName();
-        doReturn(Collections.EMPTY_SET).when(grp3).getUses();
-        doReturn(Collections.EMPTY_SET).when(grp3).getGroupings();
-        doReturn(Collections.EMPTY_SET).when(grp3).getChildNodes();
+        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"),
             QName.create("", "List1"))).when(grp4).getPath();
         doReturn(QName.create("", "leaf4")).when(grp4).getQName();
-        doReturn(Collections.EMPTY_SET).when(grp4).getUses();
-        doReturn(Collections.EMPTY_SET).when(grp4).getGroupings();
-        doReturn(Collections.EMPTY_SET).when(grp4).getChildNodes();
+        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();
         doReturn(QName.create("", "leaf5")).when(grp5).getQName();
-        doReturn(Collections.EMPTY_SET).when(grp5).getUses();
-        doReturn(Collections.EMPTY_SET).when(grp5).getGroupings();
-        doReturn(Collections.EMPTY_SET).when(grp5).getChildNodes();
+        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);
@@ -76,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;
+        }
+    }
+}