From: Robert Varga Date: Mon, 14 Mar 2022 12:29:08 +0000 (+0100) Subject: Fix inferred statements over undeclared statements X-Git-Tag: v8.0.2~12 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=yangtools.git;a=commitdiff_plain;h=0f34ef9c1d749b903a476b84c3c54e5e98225669 Fix inferred statements over undeclared statements We must not assume we can access declared() statement. In order to do that, we need to change how createInferredEffective() operates and push it down to ReactorStmtCtx. JIRA: YANGTOOLS-1407 Change-Id: I56823cb2ad497adcccaffa3e3386e45b0bb08089 Signed-off-by: Robert Varga --- diff --git a/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/AbstractResumedStatement.java b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/AbstractResumedStatement.java index 17ce23e150..1358f284ff 100644 --- a/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/AbstractResumedStatement.java +++ b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/AbstractResumedStatement.java @@ -106,27 +106,29 @@ abstract class AbstractResumedStatement, E ext @Override final E createEffective(final StatementFactory factory) { - return createEffective(factory, this); + return createEffective(factory, this, streamDeclared(), streamEffective()); } // Creates EffectiveStatement through full materialization and assumes declared statement presence private @NonNull E createEffective(final StatementFactory factory, - final StatementContextBase ctx) { + final StatementContextBase ctx, final Stream> declared, + final Stream> effective) { // Statement reference count infrastructure makes an assumption that effective statement is only built after // the declared statement is already done. Statements tracked by this class always have a declared view, and // we need to ensure that is built before we touch effective substatements. // // Once the effective substatement stream has been exhausted, reference counting will triggers a sweep, hence // the substatements may be gone by the time the factory attempts to acquire the declared statement. - declared(); + ctx.declared(); - return factory.createEffective(ctx, ctx.streamDeclared(), ctx.streamEffective()); + return factory.createEffective(ctx, declared, effective); } @Override final E createInferredEffective(final StatementFactory factory, - final InferredStatementContext ctx) { - return createEffective(factory, ctx); + final InferredStatementContext ctx, final Stream> declared, + final Stream> effective) { + return createEffective(factory, ctx, declared, effective); } /** 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 b1693ae1d5..9349e3ff6c 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 @@ -87,7 +87,7 @@ final class InferredStatementContext, E extend private final @NonNull StatementContextBase prototype; private final @NonNull StatementContextBase parent; - private final @NonNull StmtContext originalCtx; + private final @NonNull ReactorStmtCtx originalCtx; private final QNameModule targetModule; private final A argument; @@ -122,7 +122,10 @@ final class InferredStatementContext, E extend this.argument = targetModule == null ? prototype.argument() : prototype.definition().adaptArgumentValue(prototype, targetModule); this.targetModule = targetModule; - this.originalCtx = prototype.getOriginalCtx().orElse(prototype); + + final var origCtx = prototype.getOriginalCtx().orElse(prototype); + verify(origCtx instanceof ReactorStmtCtx, "Unexpected original %s", origCtx); + this.originalCtx = (ReactorStmtCtx) origCtx; // Mark prototype as blocking statement cleanup prototype.incRef(); @@ -212,12 +215,18 @@ final class InferredStatementContext, E extend // If we have not materialized we do not have a difference in effective substatements, hence we can forward // towards the source of the statement. accessSubstatements(); - return substatements == null ? tryToReusePrototype(factory) : createInferredEffective(factory, this); + return substatements == null ? tryToReusePrototype(factory) : createInferredEffective(factory); + } + + private @NonNull E createInferredEffective(final @NonNull StatementFactory factory) { + return createInferredEffective(factory, this, streamDeclared(), streamEffective()); } @Override - E createInferredEffective(final StatementFactory factory, final InferredStatementContext ctx) { - return prototype.createInferredEffective(factory, ctx); + E createInferredEffective(final StatementFactory factory, final InferredStatementContext ctx, + final Stream> declared, + final Stream> effective) { + return originalCtx.createInferredEffective(factory, ctx, declared, effective); } private @NonNull E tryToReusePrototype(final @NonNull StatementFactory factory) { @@ -279,7 +288,8 @@ final class InferredStatementContext, E extend prototype.decRef(); // Values are the effective copies, hence this efficiently deals with recursion. - return internAlongCopyAxis(factory, factory.createEffective(this, declared.stream(), effective.stream())); + return internAlongCopyAxis(factory, + originalCtx.createInferredEffective(factory, this, declared.stream(), effective.stream())); } private @NonNull E tryToReuseSubstatements(final @NonNull StatementFactory factory, @@ -293,7 +303,7 @@ final class InferredStatementContext, E extend // Fall back to full instantiation, which populates our substatements. Then check if we should be reusing // the substatement list, as this operation turned out to not affect them. - final E effective = createInferredEffective(factory, this); + final E effective = createInferredEffective(factory); // Since we have forced instantiation to deal with this case, we also need to reset the 'modified' flag setUnmodified(); diff --git a/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ReactorStmtCtx.java b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ReactorStmtCtx.java index e0507835b9..73aef53d66 100644 --- a/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ReactorStmtCtx.java +++ b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ReactorStmtCtx.java @@ -17,6 +17,7 @@ import java.util.Collection; import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.stream.Stream; import org.eclipse.jdt.annotation.NonNull; import org.eclipse.jdt.annotation.Nullable; import org.opendaylight.yangtools.yang.common.Empty; @@ -42,6 +43,7 @@ import org.opendaylight.yangtools.yang.parser.spi.meta.ModelProcessingPhase; import org.opendaylight.yangtools.yang.parser.spi.meta.ModelProcessingPhase.ExecutionOrder; import org.opendaylight.yangtools.yang.parser.spi.meta.NamespaceBehaviour.Registry; import org.opendaylight.yangtools.yang.parser.spi.meta.ParserNamespace; +import org.opendaylight.yangtools.yang.parser.spi.meta.StatementFactory; import org.opendaylight.yangtools.yang.parser.spi.meta.StmtContext; import org.opendaylight.yangtools.yang.parser.spi.meta.StmtContext.Mutable; import org.opendaylight.yangtools.yang.parser.spi.meta.StmtContextUtils; @@ -396,6 +398,20 @@ abstract class ReactorStmtCtx, E extends Effec abstract @NonNull E createEffective(); + /** + * Routing of the request to build an effective statement from {@link InferredStatementContext} towards the original + * definition site. This is needed to pick the correct instantiation method: for declared statements we will + * eventually land in {@link AbstractResumedStatement}, for underclared statements that will be + * {@link UndeclaredStmtCtx}. + * + * @param factory Statement factory + * @param ctx Inferred statement context, i.e. where the effective statement is instantiated + * @return Built effective stateue + */ + abstract @NonNull E createInferredEffective(@NonNull StatementFactory factory, + @NonNull InferredStatementContext ctx, Stream> declared, + Stream> effective); + /** * Attach an effective copy of this statement. This essentially acts as a map, where we make a few assumptions: *
    diff --git a/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ReplicaStatementContext.java b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ReplicaStatementContext.java index 5843492618..16bab426be 100644 --- a/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ReplicaStatementContext.java +++ b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/ReplicaStatementContext.java @@ -12,6 +12,7 @@ import static java.util.Objects.requireNonNull; import java.util.Collection; import java.util.List; import java.util.Optional; +import java.util.stream.Stream; import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.yangtools.yang.common.QNameModule; import org.opendaylight.yangtools.yang.model.api.meta.DeclaredStatement; @@ -21,6 +22,7 @@ import org.opendaylight.yangtools.yang.parser.spi.meta.CopyHistory; import org.opendaylight.yangtools.yang.parser.spi.meta.CopyType; import org.opendaylight.yangtools.yang.parser.spi.meta.NamespaceBehaviour.StorageNodeType; import org.opendaylight.yangtools.yang.parser.spi.meta.ParserNamespace; +import org.opendaylight.yangtools.yang.parser.spi.meta.StatementFactory; import org.opendaylight.yangtools.yang.parser.spi.meta.StatementNamespace; import org.opendaylight.yangtools.yang.parser.spi.meta.StatementSupport; import org.opendaylight.yangtools.yang.parser.spi.meta.StmtContext; @@ -49,6 +51,13 @@ final class ReplicaStatementContext, E extends return source.buildEffective(); } + @Override + E createInferredEffective(final StatementFactory factory, final InferredStatementContext ctx, + final Stream> declared, + final Stream> effective) { + return source.createInferredEffective(factory, ctx, declared, effective); + } + @Override ReactorStmtCtx unmodifiedEffectiveSource() { return source.unmodifiedEffectiveSource(); diff --git a/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/StatementContextBase.java b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/StatementContextBase.java index 438c6e6365..eb072d9a31 100644 --- a/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/StatementContextBase.java +++ b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/StatementContextBase.java @@ -445,19 +445,6 @@ abstract class StatementContextBase, E extends abstract @NonNull E createEffective(@NonNull StatementFactory factory); - /** - * Routing of the request to build an effective statement from {@link InferredStatementContext} towards the original - * definition site. This is needed to pick the correct instantiation method: for declared statements we will - * eventually land in {@link AbstractResumedStatement}, for underclared statements that will be - * {@link UndeclaredStmtCtx}. - * - * @param factory Statement factory - * @param ctx Inferred statement context, i.e. where the effective statement is instantiated - * @return Built effective stateue - */ - abstract @NonNull E createInferredEffective(@NonNull StatementFactory factory, - @NonNull InferredStatementContext ctx); - /** * Return a stream of declared statements which can be built into an {@link EffectiveStatement}, as per * {@link StmtContext#buildEffective()} contract. diff --git a/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/UndeclaredStmtCtx.java b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/UndeclaredStmtCtx.java index 75b3aefb2d..beef8d92fb 100644 --- a/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/UndeclaredStmtCtx.java +++ b/parser/yang-parser-reactor/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/UndeclaredStmtCtx.java @@ -123,8 +123,12 @@ final class UndeclaredStmtCtx, E extends Effec } @Override - E createInferredEffective(final StatementFactory factory, final InferredStatementContext ctx) { - return createEffective(factory, new ForwardingUndeclaredCurrent<>(ctx), ctx.streamEffective()); + E createInferredEffective(final StatementFactory factory, final InferredStatementContext ctx, + final Stream> declared, + final Stream> effective) { + final long declaredCount = declared.count(); + verify(declaredCount == 0, "Unexpected non-empty declared statements in %s", ctx); + return createEffective(factory, new ForwardingUndeclaredCurrent<>(ctx), effective); } /* diff --git a/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1407Test.java b/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1407Test.java new file mode 100644 index 0000000000..2fa809bcf1 --- /dev/null +++ b/parser/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1407Test.java @@ -0,0 +1,17 @@ +/* + * 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 YT1407Test extends AbstractYangTest { + @Test + public void testUsedUndeclaredCase() { + assertEffectiveModelDir("/bugs/YT1407"); + } +} diff --git a/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1407/ietf-lisp-address-types.yang b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1407/ietf-lisp-address-types.yang new file mode 100644 index 0000000000..1827e95187 --- /dev/null +++ b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1407/ietf-lisp-address-types.yang @@ -0,0 +1,14 @@ +module ietf-lisp-address-types { + namespace "urn:ietf:params:xml:ns:yang:ietf-lisp-address-types"; + prefix laddr; + + grouping lisp-address { + choice address { + container null-address { + leaf address { + type empty; + } + } + } + } +} diff --git a/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1407/odl-inet-binary-types.yang b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1407/odl-inet-binary-types.yang new file mode 100644 index 0000000000..ee7ae5f21e --- /dev/null +++ b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1407/odl-inet-binary-types.yang @@ -0,0 +1,10 @@ +module odl-inet-binary-types { + namespace "urn:opendaylight:lfm:inet-binary-types"; + prefix "inet-binary"; + + typedef ipv4-address-binary { + type binary { + length "4"; + } + } +} diff --git a/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1407/odl-lisp-address-types.yang b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1407/odl-lisp-address-types.yang new file mode 100644 index 0000000000..b4372eb648 --- /dev/null +++ b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1407/odl-lisp-address-types.yang @@ -0,0 +1,19 @@ +module odl-lisp-address-types { + namespace "urn:opendaylight:lfm:lisp-binary-address-types"; + prefix "lisp-binary"; + + import ietf-lisp-address-types { prefix laddr; } + import odl-inet-binary-types { prefix bin; } + + grouping augmented-lisp-address { + uses laddr:lisp-address { + augment "address" { + case ipv4-binary { + leaf ipv4-binary { + type bin:ipv4-address-binary; + } + } + } + } + } +} diff --git a/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1407/odl-lisp-proto.yang b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1407/odl-lisp-proto.yang new file mode 100644 index 0000000000..fa20e9d328 --- /dev/null +++ b/parser/yang-parser-rfc7950/src/test/resources/bugs/YT1407/odl-lisp-proto.yang @@ -0,0 +1,22 @@ +module odl-lisp-proto { + namespace "urn:opendaylight:lfm:lisp-proto"; + prefix "lisp-proto"; + + import odl-lisp-address-types { prefix odl-lisp-address; } + + grouping eid-container { + container eid { + uses odl-lisp-address:augmented-lisp-address; + } + } + + grouping eid-list { + list eid-item { + key "eid-item-id"; + leaf eid-item-id { + type string; + } + uses eid-container; + } + } +}