From 23c5afa5153577de85a0282e31477a624333e4e3 Mon Sep 17 00:00:00 2001 From: Sangwook Ha Date: Sat, 3 Dec 2022 15:19:45 -0800 Subject: [PATCH] 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 (cherry picked from commit 7e849648fed0d0a195f83d43d1a7d015478ad1eb) --- .../stmt/meta/UniqueStatementSupport.java | 45 +++++++++++++++---- .../yangtools/yang/stmt/YT1470Test.java | 17 +++++++ .../src/test/resources/bugs/YT1470/bar.yang | 18 ++++++++ .../src/test/resources/bugs/YT1470/foo.yang | 18 ++++++++ 4 files changed, 90 insertions(+), 8 deletions(-) create mode 100644 parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1470Test.java create mode 100644 parser/yang-parser-rfc7950/src/test/resources/bugs/YT1470/bar.yang create mode 100644 parser/yang-parser-rfc7950/src/test/resources/bugs/YT1470/foo.yang 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 5bec0855b7..ad3900c2e2 100644 --- 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 adaptArgumentValue( + final StmtContext, 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 parseArgumentValue(final StmtContext ctx, final String value) { - final ImmutableSet 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 qnames, final QNameModule module) { + return qnames.allMatch(qname -> module.equals(qname.getModule())); + } + private static ImmutableSet 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 uniqueConstraintNodes = new HashSet<>(); - for (final String uniqueArgToken : SEP_SPLITTER.split(nocrlf)) { - final SchemaNodeIdentifier nodeIdentifier = ArgumentUtils.nodeIdentifierFromPath(ctx, uniqueArgToken); + final var uniqueConstraintNodes = new HashSet(); + 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 index 0000000000..2645d4eeb2 --- /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 index 0000000000..06c0ca1688 --- /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 index 0000000000..decd2c28df --- /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; + } + } +} -- 2.36.6