From: Robert Varga Date: Tue, 17 May 2022 10:57:32 +0000 (+0200) Subject: Fix statement prerequisites and materialization X-Git-Tag: v9.0.0~50 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=yangtools.git;a=commitdiff_plain;h=a257eeb5333f6719db8afed7258fa0ce3dbdc0ae Fix statement prerequisites and materialization We are firing onStatementAdded() when copying statements, which means that: a) a new inference requirement may be created during inferred statemenent materialization b) the inference requirement may be immediately available in effective model In order to deal with this, ModifierImpl has to always go through a bootstrap, so that it prerequisites only trigger once an action is registered. Furthermore InferredStatementContext needs always instantiate the map for partial instantiations so as to deal with namespace-driven instantiations happening while full instantiation is going on. JIRA: YANGTOOLS-1434 Change-Id: I86dae587c1fe5804cd983c194903e1975f257408 Signed-off-by: Robert Varga --- diff --git a/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/InferredStatementContext.java b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/InferredStatementContext.java index f5a00a1499..af2b5bbe56 100644 --- a/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/InferredStatementContext.java +++ b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/InferredStatementContext.java @@ -12,6 +12,7 @@ import static java.util.Objects.requireNonNull; import com.google.common.base.VerifyException; import com.google.common.collect.ImmutableList; +import com.google.common.collect.Maps; import com.google.common.collect.Streams; import java.util.ArrayList; import java.util.Collection; @@ -474,7 +475,8 @@ final class InferredStatementContext, E extend private List> ensureEffectiveSubstatements() { accessSubstatements(); return substatements instanceof List ? castEffective(substatements) - : initializeSubstatements(castMaterialized(substatements)); + // We have either not started or have only partially-materialized statements, ensure full materialization + : initializeSubstatements(); } @Override @@ -529,22 +531,31 @@ final class InferredStatementContext, E extend return count; } - private List> initializeSubstatements( - final Map, ReactorStmtCtx> materializedSchemaTree) { - final Collection> declared = prototype.mutableDeclaredSubstatements(); - final Collection> effective = prototype.mutableEffectiveSubstatements(); + private List> initializeSubstatements() { + final var declared = prototype.mutableDeclaredSubstatements(); + final var effective = prototype.mutableEffectiveSubstatements(); + + // We are about to instantiate some substatements. The simple act of materializing them may end up triggering + // namespace lookups, which in turn can materialize copies by themselves, running ahead of our materialization. + // We therefore need a meeting place for, which are the partially-materialized substatements. If we do not have + // them yet, instantiate them and we need to populate them as well. + final int expectedSize = declared.size() + effective.size(); + var materializedSchemaTree = castMaterialized(substatements); + if (materializedSchemaTree == null) { + substatements = materializedSchemaTree = Maps.newHashMapWithExpectedSize(expectedSize); + } - final var buffer = new ArrayList>(declared.size() + effective.size()); - for (final Mutable stmtContext : declared) { + final var buffer = new ArrayList>(expectedSize); + for (var stmtContext : declared) { if (stmtContext.isSupportedByFeatures()) { copySubstatement(stmtContext, buffer, materializedSchemaTree); } } - for (final Mutable stmtContext : effective) { + for (var stmtContext : effective) { copySubstatement(stmtContext, buffer, materializedSchemaTree); } - final List> ret = beforeAddEffectiveStatementUnsafe(ImmutableList.of(), buffer.size()); + final var ret = beforeAddEffectiveStatementUnsafe(ImmutableList.of(), buffer.size()); ret.addAll(buffer); substatements = ret; setModified(); @@ -570,10 +581,12 @@ final class InferredStatementContext, E extend // // We could also perform a Map.containsKey() and perform a bulk add, but that would mean the statement order // against parent would change -- and we certainly do not want that to happen. - final ReactorStmtCtx materialized = findMaterialized(materializedSchemaTree, substatement); + final var materialized = findMaterialized(materializedSchemaTree, substatement); if (materialized == null) { copySubstatement(substatement).ifPresent(copy -> { - buffer.add(ensureCompletedPhase(copy)); + final var cast = ensureCompletedPhase(copy); + materializedSchemaTree.put(substatement, cast); + buffer.add(cast); }); } else { buffer.add(materialized); diff --git a/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ModifierImpl.java b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ModifierImpl.java index 1fbb0f4bb8..6e31b1adb2 100644 --- a/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ModifierImpl.java +++ b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ModifierImpl.java @@ -116,7 +116,7 @@ final class ModifierImpl implements ModelActionBuilder { PhaseFinished phaseFin = new PhaseFinished<>(); addReq(phaseFin); - contextImpl(context).addPhaseCompletedListener(phase, phaseFin); + addBootstrap(() -> contextImpl(context).addPhaseCompletedListener(phase, phaseFin)); return phaseFin; } diff --git a/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1434Test.java b/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1434Test.java new file mode 100644 index 0000000000..44123b7799 --- /dev/null +++ b/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1434Test.java @@ -0,0 +1,22 @@ +/* + * Copyright (c) 2022 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.yangtools.yang.stmt; + +import org.junit.Test; + +public class YT1434Test extends AbstractYangTest { + @Test + public void testUniqueViaAugment() { + assertEffectiveModel("/bugs/YT1434/foo.yang"); + } + + @Test + public void testUniqueViaUses() { + assertEffectiveModel("/bugs/YT1434/bar.yang"); + } +} diff --git a/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1434/bar.yang b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1434/bar.yang new file mode 100644 index 0000000000..370908b15b --- /dev/null +++ b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1434/bar.yang @@ -0,0 +1,27 @@ +module bar { + namespace bar; + prefix bar; + + container foo { + uses bar; + } + + grouping bar { + list bar { + key one; + unique "two three"; + + leaf one { + type string; + } + + leaf two { + type string; + } + + leaf three { + type string; + } + } + } +} diff --git a/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1434/foo.yang b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1434/foo.yang new file mode 100644 index 0000000000..ef6765f130 --- /dev/null +++ b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1434/foo.yang @@ -0,0 +1,25 @@ +module foo { + namespace foo; + prefix foo; + + container foo; + + augment /foo { + list bar { + key one; + unique "two three"; + + leaf one { + type string; + } + + leaf two { + type string; + } + + leaf three { + type string; + } + } + } +}