From 32b112dc64848d9c1cf0ea227109ca9a1265e2e7 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Tue, 26 Nov 2019 12:27:57 +0100 Subject: [PATCH] Refactor AbstractEffectiveModule AbstractEffectiveModule has a number of special cases in its constructor, effectively adding behavior based on expected subclass. Furthermore its layout does not correctly propagate declared/effective substatements towards superclass, leading to partially-populated data/schema tree namespaces. This patch moves the special handling bits to subclasses, making AbstractEffectiveModule properly agnostic of whether it is used as a ModuleEffectiveStatement or SubmoduleEffectiveStatement. For the mechanics of including submodule effective statements into the main module we introduce ModuleStmtContext, which performs the semantic addition, thus all StmtContexts are properly visible as if they were declared in the main module. JIRA: YANGTOOLS-1042 Change-Id: Ice9609c2db82981ec9156d076f8a02bc637181b9 Signed-off-by: Robert Varga --- .../rfc7950/stmt/AbstractEffectiveModule.java | 126 ++-------- .../module/ModuleEffectiveStatementImpl.java | 26 ++- .../stmt/module/ModuleStmtContext.java | 221 ++++++++++++++++++ .../SubmoduleEffectiveStatementImpl.java | 74 +++++- .../yangtools/yang/stmt/YT1042Test.java | 32 +++ .../src/test/resources/bugs/YT1042/bar.yang | 13 ++ .../src/test/resources/bugs/YT1042/foo.yang | 10 + 7 files changed, 381 insertions(+), 121 deletions(-) create mode 100644 yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/module/ModuleStmtContext.java create mode 100644 yang/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1042Test.java create mode 100644 yang/yang-parser-rfc7950/src/test/resources/bugs/YT1042/bar.yang create mode 100644 yang/yang-parser-rfc7950/src/test/resources/bugs/YT1042/foo.yang diff --git a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/AbstractEffectiveModule.java b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/AbstractEffectiveModule.java index 411615c44f..85721fee9a 100644 --- a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/AbstractEffectiveModule.java +++ b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/AbstractEffectiveModule.java @@ -7,16 +7,13 @@ */ package org.opendaylight.yangtools.yang.parser.rfc7950.stmt; -import static com.google.common.base.Preconditions.checkState; import static java.util.Objects.requireNonNull; import com.google.common.annotations.Beta; import com.google.common.base.MoreObjects; -import com.google.common.base.Verify; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Iterables; import java.net.URI; import java.util.ArrayList; import java.util.HashSet; @@ -26,7 +23,6 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; -import java.util.stream.Collectors; import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.yangtools.concepts.SemVer; import org.opendaylight.yangtools.openconfig.model.api.OpenConfigVersionEffectiveStatement; @@ -34,7 +30,6 @@ import org.opendaylight.yangtools.yang.common.QName; import org.opendaylight.yangtools.yang.common.Revision; import org.opendaylight.yangtools.yang.common.YangVersion; import org.opendaylight.yangtools.yang.model.api.AugmentationSchemaNode; -import org.opendaylight.yangtools.yang.model.api.DataNodeContainer; import org.opendaylight.yangtools.yang.model.api.DataSchemaNode; import org.opendaylight.yangtools.yang.model.api.Deviation; import org.opendaylight.yangtools.yang.model.api.ExtensionDefinition; @@ -45,31 +40,24 @@ import org.opendaylight.yangtools.yang.model.api.Module; import org.opendaylight.yangtools.yang.model.api.ModuleImport; import org.opendaylight.yangtools.yang.model.api.NotificationDefinition; import org.opendaylight.yangtools.yang.model.api.RpcDefinition; -import org.opendaylight.yangtools.yang.model.api.SchemaNode; import org.opendaylight.yangtools.yang.model.api.TypeDefinition; import org.opendaylight.yangtools.yang.model.api.UnknownSchemaNode; import org.opendaylight.yangtools.yang.model.api.UsesNode; -import org.opendaylight.yangtools.yang.model.api.YangStmtMapping; 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.BelongsToEffectiveStatement; import org.opendaylight.yangtools.yang.model.api.stmt.ContactEffectiveStatement; import org.opendaylight.yangtools.yang.model.api.stmt.OrganizationEffectiveStatement; -import org.opendaylight.yangtools.yang.model.api.stmt.PrefixEffectiveStatement; -import org.opendaylight.yangtools.yang.model.api.stmt.SubmoduleEffectiveStatement; -import org.opendaylight.yangtools.yang.model.api.stmt.SubmoduleStatement; +import org.opendaylight.yangtools.yang.model.api.stmt.PrefixStatement; import org.opendaylight.yangtools.yang.model.api.stmt.TypedefEffectiveStatement; import org.opendaylight.yangtools.yang.model.api.stmt.YangVersionEffectiveStatement; import org.opendaylight.yangtools.yang.model.api.stmt.compat.NotificationNodeContainerCompat; -import org.opendaylight.yangtools.yang.parser.spi.meta.MutableStatement; 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.source.IncludedSubmoduleNameToModuleCtx; +import org.opendaylight.yangtools.yang.parser.spi.meta.StmtContextUtils; import org.opendaylight.yangtools.yang.parser.spi.source.SourceException; @Beta public abstract class AbstractEffectiveModule> extends - AbstractSchemaEffectiveDocumentedNode implements Module, MutableStatement, + AbstractSchemaEffectiveDocumentedNode implements Module, NotificationNodeContainerCompat { private final String name; private final String prefix; @@ -92,32 +80,13 @@ public abstract class AbstractEffectiveModule publicChildNodes; private final SemVer semanticVersion; - private Set>> - submoduleContextsToBuild; - private ImmutableSet submodules; - private boolean sealed; - - protected AbstractEffectiveModule(final StmtContext> ctx) { + protected AbstractEffectiveModule( + final @NonNull StmtContext> ctx, + final @NonNull String prefix) { super(ctx); this.name = argument(); - - final EffectiveStatement parentOfPrefix; - if (ctx.getPublicDefinition() == YangStmtMapping.SUBMODULE) { - final Optional optParent = - findFirstEffectiveSubstatement(BelongsToEffectiveStatement.class); - SourceException.throwIf(!optParent.isPresent(), ctx.getStatementSourceReference(), - "Unable to find belongs-to statement in submodule %s.", ctx.getStatementArgument()); - parentOfPrefix = optParent.get(); - } else { - parentOfPrefix = this; - } - - final Optional<@NonNull PrefixEffectiveStatement> prefixStmt = parentOfPrefix.findFirstEffectiveSubstatement( - PrefixEffectiveStatement.class); - SourceException.throwIf(!prefixStmt.isPresent(), ctx.getStatementSourceReference(), - "Unable to resolve prefix for module or submodule %s.", ctx.getStatementArgument()); - this.prefix = prefixStmt.get().argument(); + this.prefix = requireNonNull(prefix); this.yangVersion = findFirstEffectiveSubstatementArgument(YangVersionEffectiveStatement.class) .orElse(YangVersion.VERSION_1); this.semanticVersion = findFirstEffectiveSubstatementArgument(OpenConfigVersionEffectiveStatement.class) @@ -127,65 +96,6 @@ public abstract class AbstractEffectiveModule> substatementsOfSubmodules; - final Map> includedSubmodulesMap = ctx - .getAllFromCurrentStmtCtxNamespace(IncludedSubmoduleNameToModuleCtx.class); - - if (includedSubmodulesMap == null || includedSubmodulesMap.isEmpty()) { - this.submodules = ImmutableSet.of(); - this.submoduleContextsToBuild = ImmutableSet.of(); - substatementsOfSubmodules = ImmutableList.of(); - } else if (YangStmtMapping.MODULE.equals(ctx.getPublicDefinition())) { - /* - * Aggregation of substatements from submodules should be done only - * for modules. In case of submodules it does not make sense because - * of possible circular chains of includes between submodules. - */ - final Set submodulesInit = new HashSet<>(); - final List> substatementsOfSubmodulesInit = new ArrayList<>(); - for (final StmtContext submoduleCtx : includedSubmodulesMap.values()) { - final EffectiveStatement submodule = submoduleCtx.buildEffective(); - Verify.verify(submodule instanceof SubmoduleEffectiveStatement); - Verify.verify(submodule instanceof Module, "Submodule statement %s is not a Module", submodule); - submodulesInit.add((Module) submodule); - substatementsOfSubmodulesInit.addAll(submodule.effectiveSubstatements().stream() - .filter(sub -> sub instanceof SchemaNode || sub instanceof DataNodeContainer) - .collect(Collectors.toList())); - } - - this.submodules = ImmutableSet.copyOf(submodulesInit); - this.submoduleContextsToBuild = ImmutableSet.of(); - substatementsOfSubmodules = ImmutableList.copyOf(substatementsOfSubmodulesInit); - } else { - /* - * Because of possible circular chains of includes between submodules we can - * collect only submodule contexts here and then build them during - * sealing of this statement. - */ - final Set>> - submoduleContextsInit = new HashSet<>(); - for (final StmtContext submoduleCtx : includedSubmodulesMap.values()) { - submoduleContextsInit.add( - (StmtContext>)submoduleCtx); - } - - this.submoduleContextsToBuild = ImmutableSet.copyOf(submoduleContextsInit); - substatementsOfSubmodules = ImmutableList.of(); - } - - if (!submoduleContextsToBuild.isEmpty()) { - ((Mutable) ctx).addMutableStmtToSeal(this); - sealed = false; - } else { - sealed = true; - } - - // init substatements collections - final List> effectiveSubstatements = new ArrayList<>(); - effectiveSubstatements.addAll(effectiveSubstatements()); - effectiveSubstatements.addAll(substatementsOfSubmodules); - final List unknownNodesInit = new ArrayList<>(); final Set augmentationsInit = new LinkedHashSet<>(); final Set importsInit = new HashSet<>(); @@ -202,7 +112,7 @@ public abstract class AbstractEffectiveModule> mutableTypeDefinitions = new LinkedHashSet<>(); final Set mutablePublicChildNodes = new LinkedHashSet<>(); - for (final EffectiveStatement effectiveStatement : effectiveSubstatements) { + for (final EffectiveStatement effectiveStatement : effectiveSubstatements()) { if (effectiveStatement instanceof UnknownSchemaNode) { unknownNodesInit.add((UnknownSchemaNode) effectiveStatement); } @@ -312,13 +222,6 @@ public abstract class AbstractEffectiveModule getSubmodules() { - checkState(sealed, "Attempt to get base submodules from unsealed submodule effective statement %s", - getQNameModule()); - return submodules; - } - @Override public Set getFeatures() { return features; @@ -402,13 +305,10 @@ public abstract class AbstractEffectiveModule (Module) ctx.buildEffective())); - submoduleContextsToBuild = ImmutableSet.of(); - sealed = true; - } + protected static final @NonNull String findPrefix(final @NonNull StmtContext ctx, + final String type, final String name) { + return SourceException.throwIfNull( + StmtContextUtils.firstAttributeOf(ctx.declaredSubstatements(), PrefixStatement.class), + ctx.getStatementSourceReference(), "Unable to resolve prefix for %s %s.", type, name); } } diff --git a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/module/ModuleEffectiveStatementImpl.java b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/module/ModuleEffectiveStatementImpl.java index eb0810db85..ad883e20a6 100644 --- a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/module/ModuleEffectiveStatementImpl.java +++ b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/module/ModuleEffectiveStatementImpl.java @@ -11,6 +11,7 @@ import static com.google.common.base.Verify.verifyNotNull; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableMap.Builder; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Maps; import java.util.Map; import java.util.Map.Entry; @@ -19,6 +20,7 @@ import java.util.Optional; import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.yangtools.yang.common.QName; import org.opendaylight.yangtools.yang.common.QNameModule; +import org.opendaylight.yangtools.yang.model.api.Module; import org.opendaylight.yangtools.yang.model.api.meta.IdentifierNamespace; import org.opendaylight.yangtools.yang.model.api.stmt.ExtensionEffectiveStatement; import org.opendaylight.yangtools.yang.model.api.stmt.ExtensionEffectiveStatementNamespace; @@ -52,10 +54,13 @@ final class ModuleEffectiveStatementImpl extends AbstractEffectiveModule prefixToModule; private final ImmutableMap namespaceToPrefix; private final @NonNull QNameModule qnameModule; + private final ImmutableSet submodules; - ModuleEffectiveStatementImpl(final StmtContext ctx) { - super(ctx); - qnameModule = verifyNotNull(ctx.getFromNamespace(ModuleCtxToModuleQName.class, ctx)); + private ModuleEffectiveStatementImpl(final @NonNull ModuleStmtContext ctx) { + super(ctx, findPrefix(ctx.delegate(), "module", ctx.getStatementArgument())); + submodules = ctx.getSubmodules(); + + qnameModule = verifyNotNull(ctx.getFromNamespace(ModuleCtxToModuleQName.class, ctx.delegate())); final String localPrefix = findFirstEffectiveSubstatementArgument(PrefixEffectiveStatement.class).get(); final Builder prefixToModuleBuilder = ImmutableMap.builder(); @@ -78,10 +83,10 @@ final class ModuleEffectiveStatementImpl extends AbstractEffectiveModule> submodules = + final Map> includedSubmodules = ctx.getAllFromCurrentStmtCtxNamespace(IncludedSubmoduleNameToModuleCtx.class); - nameToSubmodule = submodules == null ? ImmutableMap.of() - : ImmutableMap.copyOf(Maps.transformValues(submodules, + nameToSubmodule = includedSubmodules == null ? ImmutableMap.of() + : ImmutableMap.copyOf(Maps.transformValues(includedSubmodules, submodule -> (SubmoduleEffectiveStatement) submodule.buildEffective())); final Map> extensions = @@ -98,6 +103,10 @@ final class ModuleEffectiveStatementImpl extends AbstractEffectiveModule ctx) { + this(ModuleStmtContext.create(ctx)); + } + @Override public @NonNull QNameModule localQNameModule() { return qnameModule; @@ -108,6 +117,11 @@ final class ModuleEffectiveStatementImpl extends AbstractEffectiveModule getSubmodules() { + return submodules; + } + @Override @SuppressWarnings("unchecked") public > Optional> getNamespaceContents( diff --git a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/module/ModuleStmtContext.java b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/module/ModuleStmtContext.java new file mode 100644 index 0000000000..0e7e70b3a7 --- /dev/null +++ b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/module/ModuleStmtContext.java @@ -0,0 +1,221 @@ +/* + * 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.yangtools.yang.parser.rfc7950.stmt.module; + +import static com.google.common.base.Verify.verify; +import static java.util.Objects.requireNonNull; + +import com.google.common.collect.ForwardingObject; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import java.util.ArrayList; +import java.util.Collection; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; +import org.eclipse.jdt.annotation.NonNull; +import org.eclipse.jdt.annotation.Nullable; +import org.opendaylight.yangtools.yang.common.YangVersion; +import org.opendaylight.yangtools.yang.model.api.DataNodeContainer; +import org.opendaylight.yangtools.yang.model.api.Module; +import org.opendaylight.yangtools.yang.model.api.SchemaNode; +import org.opendaylight.yangtools.yang.model.api.SchemaPath; +import org.opendaylight.yangtools.yang.model.api.meta.EffectiveStatement; +import org.opendaylight.yangtools.yang.model.api.meta.IdentifierNamespace; +import org.opendaylight.yangtools.yang.model.api.meta.StatementDefinition; +import org.opendaylight.yangtools.yang.model.api.meta.StatementSource; +import org.opendaylight.yangtools.yang.model.api.stmt.ModuleEffectiveStatement; +import org.opendaylight.yangtools.yang.model.api.stmt.ModuleStatement; +import org.opendaylight.yangtools.yang.parser.spi.meta.CopyHistory; +import org.opendaylight.yangtools.yang.parser.spi.meta.ModelProcessingPhase; +import org.opendaylight.yangtools.yang.parser.spi.meta.StmtContext; +import org.opendaylight.yangtools.yang.parser.spi.source.IncludedSubmoduleNameToModuleCtx; +import org.opendaylight.yangtools.yang.parser.spi.source.StatementSourceReference; + +/** + * A concentrator {@link StmtContext}, which makes it appear as though as all effective statements in submodules + * are included in it. + */ +final class ModuleStmtContext extends ForwardingObject + implements StmtContext { + + private final @NonNull StmtContext delegate; + private final @NonNull ImmutableList> effectiveSubstatements; + private final @NonNull ImmutableSet submodules; + + private ModuleStmtContext(final StmtContext delegate, + final Collection> submodules) { + this.delegate = requireNonNull(delegate); + + final List> statements = new ArrayList<>(delegate.effectiveSubstatements()); + final Set subs = new LinkedHashSet<>(submodules.size()); + for (StmtContext submoduleCtx : submodules) { + final EffectiveStatement submodule = submoduleCtx.buildEffective(); + verify(submodule instanceof Module, "Submodule statement %s is not a Module", submodule); + subs.add((Module) submodule); + + for (StmtContext stmt : submoduleCtx.allSubstatements()) { + if (stmt.isSupportedByFeatures()) { + final EffectiveStatement effective = stmt.buildEffective(); + if (effective instanceof SchemaNode || effective instanceof DataNodeContainer) { + statements.add(stmt); + } + } + } + } + + this.effectiveSubstatements = ImmutableList.copyOf(statements); + this.submodules = ImmutableSet.copyOf(subs); + } + + static @NonNull ModuleStmtContext create( + final StmtContext delegate) { + final Map> includedSubmodules = delegate.getAllFromCurrentStmtCtxNamespace( + IncludedSubmoduleNameToModuleCtx.class); + return new ModuleStmtContext(delegate, includedSubmodules == null || includedSubmodules.isEmpty() + ? ImmutableList.of() : includedSubmodules.values()); + } + + @Override + protected @NonNull StmtContext delegate() { + return delegate; + } + + ImmutableSet getSubmodules() { + return submodules; + } + + @Override + public ImmutableList> effectiveSubstatements() { + return effectiveSubstatements; + } + + @Override + public ModuleEffectiveStatement buildEffective() { + throw new UnsupportedOperationException("Attempted to instantiate proxy context " + this); + } + + @Override + public ModuleStatement buildDeclared() { + return delegate.buildDeclared(); + } + + @Override + public StatementSource getStatementSource() { + return delegate.getStatementSource(); + } + + @Override + public StatementSourceReference getStatementSourceReference() { + return delegate.getStatementSourceReference(); + } + + @Override + public StatementDefinition getPublicDefinition() { + return delegate.getPublicDefinition(); + } + + @Override + public StmtContext getParentContext() { + return delegate.getParentContext(); + } + + @Override + public String rawStatementArgument() { + return delegate.rawStatementArgument(); + } + + @Override + public @Nullable String getStatementArgument() { + return delegate.getStatementArgument(); + } + + @Override + public @NonNull Optional getSchemaPath() { + return delegate.getSchemaPath(); + } + + @Override + public boolean isConfiguration() { + return delegate.isConfiguration(); + } + + @Override + public boolean isEnabledSemanticVersioning() { + return delegate.isEnabledSemanticVersioning(); + } + + @Override + public > @NonNull V getFromNamespace(final Class type, + final T key) { + return delegate.getFromNamespace(type, key); + } + + @Override + public > Map getAllFromNamespace(final Class type) { + return delegate.getAllFromNamespace(type); + } + + @Override + public > Map getAllFromCurrentStmtCtxNamespace( + final Class type) { + return delegate.getAllFromCurrentStmtCtxNamespace(type); + } + + @Override + public StmtContext getRoot() { + return delegate.getRoot(); + } + + @Override + public Collection> declaredSubstatements() { + return delegate.declaredSubstatements(); + } + + @Override + public boolean isSupportedToBuildEffective() { + return delegate.isSupportedToBuildEffective(); + } + + @Override + public boolean isSupportedByFeatures() { + return delegate.isSupportedByFeatures(); + } + + @Override + public Collection> getEffectOfStatement() { + return delegate.getEffectOfStatement(); + } + + @Override + public CopyHistory getCopyHistory() { + return delegate.getCopyHistory(); + } + + @Override + public Optional> getOriginalCtx() { + return delegate.getOriginalCtx(); + } + + @Override + public Optional> getPreviousCopyCtx() { + return delegate.getPreviousCopyCtx(); + } + + @Override + public ModelProcessingPhase getCompletedPhase() { + return delegate.getCompletedPhase(); + } + + @Override + public @NonNull YangVersion getRootVersion() { + return delegate.getRootVersion(); + } +} diff --git a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/submodule/SubmoduleEffectiveStatementImpl.java b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/submodule/SubmoduleEffectiveStatementImpl.java index a531553961..90edc78867 100644 --- a/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/submodule/SubmoduleEffectiveStatementImpl.java +++ b/yang/yang-parser-rfc7950/src/main/java/org/opendaylight/yangtools/yang/parser/rfc7950/stmt/submodule/SubmoduleEffectiveStatementImpl.java @@ -7,27 +7,45 @@ */ package org.opendaylight.yangtools.yang.parser.rfc7950.stmt.submodule; +import static com.google.common.base.Preconditions.checkState; import static org.opendaylight.yangtools.yang.parser.spi.meta.StmtContextUtils.firstAttributeOf; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; +import java.util.HashSet; +import java.util.Map; import java.util.Objects; import java.util.Optional; +import java.util.Set; +import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.yangtools.yang.common.QNameModule; import org.opendaylight.yangtools.yang.common.Revision; +import org.opendaylight.yangtools.yang.model.api.Module; +import org.opendaylight.yangtools.yang.model.api.meta.EffectiveStatement; import org.opendaylight.yangtools.yang.model.api.stmt.BelongsToStatement; import org.opendaylight.yangtools.yang.model.api.stmt.RevisionEffectiveStatement; import org.opendaylight.yangtools.yang.model.api.stmt.SubmoduleEffectiveStatement; import org.opendaylight.yangtools.yang.model.api.stmt.SubmoduleStatement; import org.opendaylight.yangtools.yang.parser.rfc7950.stmt.AbstractEffectiveModule; +import org.opendaylight.yangtools.yang.parser.spi.meta.MutableStatement; 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; +import org.opendaylight.yangtools.yang.parser.spi.source.IncludedSubmoduleNameToModuleCtx; import org.opendaylight.yangtools.yang.parser.spi.source.ModuleNameToModuleQName; +import org.opendaylight.yangtools.yang.parser.spi.source.SourceException; final class SubmoduleEffectiveStatementImpl extends AbstractEffectiveModule - implements SubmoduleEffectiveStatement { + implements SubmoduleEffectiveStatement, MutableStatement { private final QNameModule qnameModule; + private Set>> submoduleContexts; + private ImmutableSet submodules; + private boolean sealed; + SubmoduleEffectiveStatementImpl(final StmtContext ctx) { - super(ctx); + super(ctx, findSubmodulePrefix(ctx)); final String belongsToModuleName = firstAttributeOf(ctx.declaredSubstatements(), BelongsToStatement.class); final QNameModule belongsToModuleQName = ctx.getFromNamespace(ModuleNameToModuleQName.class, @@ -36,6 +54,33 @@ final class SubmoduleEffectiveStatementImpl extends AbstractEffectiveModule submoduleRevision = findFirstEffectiveSubstatementArgument( RevisionEffectiveStatement.class); this.qnameModule = QNameModule.create(belongsToModuleQName.getNamespace(), submoduleRevision).intern(); + + /* + * Because of possible circular chains of includes between submodules we can + * collect only submodule contexts here and then build them during + * sealing of this statement. + */ + final Map> includedSubmodulesMap = ctx.getAllFromCurrentStmtCtxNamespace( + IncludedSubmoduleNameToModuleCtx.class); + if (includedSubmodulesMap != null) { + final Set>> + submoduleContextsInit = new HashSet<>(); + for (final StmtContext submoduleCtx : includedSubmodulesMap.values()) { + submoduleContextsInit.add( + (StmtContext>)submoduleCtx); + } + submoduleContexts = ImmutableSet.copyOf(submoduleContextsInit); + } else { + submoduleContexts = ImmutableSet.of(); + } + + if (!submoduleContexts.isEmpty()) { + ((Mutable) ctx).addMutableStmtToSeal(this); + sealed = false; + } else { + submodules = ImmutableSet.of(); + sealed = true; + } } @Override @@ -43,6 +88,13 @@ final class SubmoduleEffectiveStatementImpl extends AbstractEffectiveModule getSubmodules() { + checkState(sealed, "Attempt to get base submodules from unsealed submodule effective statement %s", + qnameModule); + return submodules; + } + @Override public int hashCode() { return Objects.hash(getName(), getYangVersion(), qnameModule); @@ -60,4 +112,22 @@ final class SubmoduleEffectiveStatementImpl extends AbstractEffectiveModule (Module) ctx.buildEffective())); + submoduleContexts = ImmutableSet.of(); + sealed = true; + } + } + + private static @NonNull String findSubmodulePrefix(final StmtContext ctx) { + final String name = ctx.getStatementArgument(); + final StmtContext belongsTo = SourceException.throwIfNull( + StmtContextUtils.findFirstDeclaredSubstatement(ctx, BelongsToStatement.class), + ctx.getStatementSourceReference(), "Unable to find belongs-to statement in submodule %s.", name); + return findPrefix(belongsTo, "submodule", name); + } } diff --git a/yang/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1042Test.java b/yang/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1042Test.java new file mode 100644 index 0000000000..390ecdad04 --- /dev/null +++ b/yang/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/stmt/YT1042Test.java @@ -0,0 +1,32 @@ +/* + * 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.yangtools.yang.stmt; + +import static org.hamcrest.Matchers.isA; +import static org.hamcrest.Matchers.startsWith; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; + +import org.junit.Test; +import org.opendaylight.yangtools.yang.parser.spi.meta.ReactorException; +import org.opendaylight.yangtools.yang.parser.spi.source.SourceException; + +public class YT1042Test { + @Test + public void testSubmoduleConflict() throws Exception { + try { + StmtTestUtils.parseYangSources("/bugs/YT1042"); + fail("Sechema assembly should have failed"); + } catch (ReactorException e) { + final Throwable cause = e.getCause(); + assertThat(cause, isA(SourceException.class)); + assertThat(cause.getMessage(), + startsWith("Cannot add data tree child with name (foo)foo, a conflicting child already exists")); + } + } +} diff --git a/yang/yang-parser-rfc7950/src/test/resources/bugs/YT1042/bar.yang b/yang/yang-parser-rfc7950/src/test/resources/bugs/YT1042/bar.yang new file mode 100644 index 0000000000..bdc3acd979 --- /dev/null +++ b/yang/yang-parser-rfc7950/src/test/resources/bugs/YT1042/bar.yang @@ -0,0 +1,13 @@ +submodule bar { + belongs-to foo { + prefix foo; + } + + choice bar { + case foo { + leaf foo { + type string; + } + } + } +} diff --git a/yang/yang-parser-rfc7950/src/test/resources/bugs/YT1042/foo.yang b/yang/yang-parser-rfc7950/src/test/resources/bugs/YT1042/foo.yang new file mode 100644 index 0000000000..0249391da3 --- /dev/null +++ b/yang/yang-parser-rfc7950/src/test/resources/bugs/YT1042/foo.yang @@ -0,0 +1,10 @@ +module foo { + namespace foo; + prefix foo; + + include bar; + + container foo { + + } +} -- 2.36.6