Fix unique statement argument propagation

Augmentation to the list with a unique statement from a module different
from the one where the list is defined causes SourceException which
complains that the argument node of the unique statement does not exist.

This turns out to have already been flagged with a FIXME -- this
behaviour has been there since forever.

The correct behaviour is to use a StatementPolicy.copyDeclared() and
adjust QNames in the argument's Descendants.

JIRA: YANGTOOLS-1470
Change-Id: Ib2cd23d50cd881d2ae94dbc0b271b5d481c9f55c
Signed-off-by: Sangwook Ha <sangwook.ha@verizon.com>
(cherry picked from commit 7e849648fed0d0a195f83d43d1a7d015478ad1eb)

diff --git a/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/meta/UniqueStatementSupport.java b/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/meta/UniqueStatementSupport.java
index 5bec0855b7cbbc47f598f91d53e0cfc2ea4e8000..ad3900c2e23f6e7a16f9dfa5e3192796117a9975 100644 (file)
--- a/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/meta/UniqueStatementSupport.java
+++ b/parser/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/meta/UniqueStatementSupport.java
@@ -14,6 +14,7 @@ import com.google.common.base.Splitter;
 import com.google.common.collect.ImmutableBiMap;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import java.util.Collection;
 import java.util.HashSet;
@@ -21,13 +22,15 @@ import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
 import java.util.regex.Pattern;
+import java.util.stream.Stream;
+import org.opendaylight.yangtools.yang.common.QName;
+import org.opendaylight.yangtools.yang.common.QNameModule;
 import org.opendaylight.yangtools.yang.model.api.YangStmtMapping;
 import org.opendaylight.yangtools.yang.model.api.meta.DeclarationReference;
 import org.opendaylight.yangtools.yang.model.api.meta.DeclaredStatement;
 import org.opendaylight.yangtools.yang.model.api.meta.EffectiveStatement;
 import org.opendaylight.yangtools.yang.model.api.stmt.LeafEffectiveStatement;
 import org.opendaylight.yangtools.yang.model.api.stmt.ListEffectiveStatement;
-import org.opendaylight.yangtools.yang.model.api.stmt.SchemaNodeIdentifier;
 import org.opendaylight.yangtools.yang.model.api.stmt.SchemaNodeIdentifier.Absolute;
 import org.opendaylight.yangtools.yang.model.api.stmt.SchemaNodeIdentifier.Descendant;
 import org.opendaylight.yangtools.yang.model.api.stmt.UniqueEffectiveStatement;
@@ -65,14 +68,36 @@ public final class UniqueStatementSupport
         SubstatementValidator.builder(YangStmtMapping.UNIQUE).build();
 
     public UniqueStatementSupport(final YangParserConfiguration config) {
-        // FIXME: This reflects what the current implementation does. We really want to define an adaptArgumentValue(),
-        //        but how that plays with the argument and expectations needs to be investigated.
-        super(YangStmtMapping.UNIQUE, StatementPolicy.contextIndependent(), config, SUBSTATEMENT_VALIDATOR);
+        super(YangStmtMapping.UNIQUE,
+            StatementPolicy.copyDeclared(
+                (copy, current, substatements) -> copy.getArgument().equals(current.getArgument())),
+            config, SUBSTATEMENT_VALIDATOR);
+    }
+
+    @Override
+    public Set<Descendant> adaptArgumentValue(
+            final StmtContext<Set<Descendant>, UniqueStatement, UniqueEffectiveStatement> ctx,
+            final QNameModule targetModule) {
+        // Copy operation to a targetNamespace -- this implies rehosting node-identifiers to target namespace. Check
+        // if that is needed first, though, so as not to copy things unnecessarily.
+        final var origArg = ctx.getArgument();
+        if (allMatch(origArg.stream().flatMap(desc -> desc.getNodeIdentifiers().stream()), targetModule)) {
+            return origArg;
+        }
+
+        return origArg.stream()
+            .map(descendant -> {
+                final var nodeIds = descendant.getNodeIdentifiers();
+                // Only update descendants that need updating
+                return allMatch(nodeIds.stream(), targetModule) ? descendant
+                    : Descendant.of(Lists.transform(nodeIds, nodeId -> nodeId.bindTo(targetModule).intern()));
+            })
+            .collect(ImmutableSet.toImmutableSet());
     }
 
     @Override
     public ImmutableSet<Descendant> parseArgumentValue(final StmtContext<?, ?, ?> ctx, final String value) {
-        final ImmutableSet<Descendant> uniqueConstraints = parseUniqueConstraintArgument(ctx, value);
+        final var uniqueConstraints = parseUniqueConstraintArgument(ctx, value);
         SourceException.throwIf(uniqueConstraints.isEmpty(), ctx,
             "Invalid argument value '%s' of unique statement. The value must contains at least one descendant schema "
                 + "node identifier.", value);
@@ -112,14 +137,18 @@ public final class UniqueStatementSupport
         return EffectiveStatements.createUnique(stmt.declared(), substatements);
     }
 
+    private static boolean allMatch(final Stream<QName> qnames, final QNameModule module) {
+        return qnames.allMatch(qname -> module.equals(qname.getModule()));
+    }
+
     private static ImmutableSet<Descendant> parseUniqueConstraintArgument(final StmtContext<?, ?, ?> ctx,
             final String argumentValue) {
         // deal with 'line-break' rule, which is either "\n" or "\r\n", but not "\r"
         final String nocrlf = CRLF_PATTERN.matcher(argumentValue).replaceAll("\n");
 
-        final Set<Descendant> uniqueConstraintNodes = new HashSet<>();
-        for (final String uniqueArgToken : SEP_SPLITTER.split(nocrlf)) {
-            final SchemaNodeIdentifier nodeIdentifier = ArgumentUtils.nodeIdentifierFromPath(ctx, uniqueArgToken);
+        final var uniqueConstraintNodes = new HashSet<Descendant>();
+        for (var uniqueArgToken : SEP_SPLITTER.split(nocrlf)) {
+            final var nodeIdentifier = ArgumentUtils.nodeIdentifierFromPath(ctx, uniqueArgToken);
             SourceException.throwIf(nodeIdentifier instanceof Absolute, ctx,
                 "Unique statement argument '%s' contains schema node identifier '%s' which is not in the descendant "
                     + "node identifier form.", argumentValue, uniqueArgToken);

diff --git a/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1470Test.java b/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1470Test.java
new file mode 100644 (file)
index 0000000..2645d4e
--- /dev/null
+++ b/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1470Test.java
@@ -0,0 +1,17 @@
+/*
+ * Copyright (c) 2022 Verizon 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.stmt;
+
+import org.junit.Test;
+
+public class YT1470Test extends AbstractYangTest {
+    @Test
+    public void testUniqueInAugmentedList() {
+        assertEffectiveModelDir("/bugs/YT1470");
+    }
+}

diff --git a/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1470/bar.yang b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1470/bar.yang
new file mode 100644 (file)
index 0000000..06c0ca1
--- /dev/null
+++ b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1470/bar.yang
@@ -0,0 +1,18 @@
+module bar {
+  namespace "urn:bar";
+  prefix "bar";
+
+  grouping bar-group {
+    list bar-list {
+      key bar-leaf1;
+      unique bar-leaf2;
+
+      leaf bar-leaf1 {
+        type string;
+      }
+      leaf bar-leaf2 {
+        type string;
+      }
+    }
+  }
+}

diff --git a/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1470/foo.yang b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1470/foo.yang
new file mode 100644 (file)
index 0000000..decd2c2
--- /dev/null
+++ b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1470/foo.yang
@@ -0,0 +1,18 @@
+module foo {
+  namespace "urn:foo";
+  prefix "foo";
+
+  import bar {
+    prefix bar;
+  }
+
+  container foo {
+    uses bar:bar-group;
+  }
+
+  augment "/foo:foo/foo:bar-list" {
+    leaf foo-leaf {
+      type string;
+    }
+  }
+}