From e13d2970392b4bc62ab2ddf89cba6454709a8174 Mon Sep 17 00:00:00 2001 From: Peter Kajsa Date: Thu, 22 Sep 2016 13:57:39 +0200 Subject: [PATCH] Bug 6329: Parser fails when target node of uses-augment is an unknown node Yang parser fails when target node of uses-augment is an unknown node. It is not quite clear whether such yang model is valid according to RFC6020 or not, but yang parser failure causes lots of trouble, because such augment is used widely in some yang models. So this patch prevents failure of yang parser and rather introduces a warning and augmentation is not performed, when target node is an unknown node. Change-Id: I7f1c5d7b3ef0898ca098466702d7cb1ad7a0f30f Signed-off-by: Peter Kajsa --- .../stmt/reactor/BuildGlobalContext.java | 57 +++++++++++-------- .../stmt/reactor/SourceSpecificContext.java | 9 +-- .../stmt/rfc6020/AugmentStatementImpl.java | 23 ++++++-- .../yang/parser/stmt/rfc6020/Utils.java | 4 +- .../yang/stmt/AugmentToExtensionTest.java | 2 - .../incorrect-path/augment-module.yang | 2 +- 6 files changed, 59 insertions(+), 38 deletions(-) diff --git a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/BuildGlobalContext.java b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/BuildGlobalContext.java index 62822474d7..6480ab9af0 100644 --- a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/BuildGlobalContext.java +++ b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/BuildGlobalContext.java @@ -20,6 +20,7 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.function.Predicate; import javax.annotation.Nonnull; @@ -90,7 +91,7 @@ class BuildGlobalContext extends NamespaceStorageSupport implements NamespaceBeh Preconditions.checkNotNull(statementParserMode, "Statement parser mode must not be null."); this.enabledSemanticVersions = statementParserMode == StatementParserMode.SEMVER_MODE; - for (Entry> validationBundle : supportedValidation.entrySet()) { + for (final Entry> validationBundle : supportedValidation.entrySet()) { addToNs(ValidationBundlesNamespace.class, validationBundle.getKey(), validationBundle.getValue()); } @@ -130,7 +131,7 @@ class BuildGlobalContext extends NamespaceStorageSupport implements NamespaceBeh final Class type) { NamespaceBehaviourWithListeners potential = supportedNamespaces.get(type); if (potential == null) { - NamespaceBehaviour potentialRaw = supports.get(currentPhase).getNamespaceBehaviour(type); + final NamespaceBehaviour potentialRaw = supports.get(currentPhase).getNamespaceBehaviour(type); if (potentialRaw != null) { potential = createNamespaceContext(potentialRaw); supportedNamespaces.put(type, potential); @@ -152,7 +153,7 @@ class BuildGlobalContext extends NamespaceStorageSupport implements NamespaceBeh private > NamespaceBehaviourWithListeners createNamespaceContext( final NamespaceBehaviour potentialRaw) { if (potentialRaw instanceof DerivedNamespaceBehaviour) { - VirtualNamespaceContext derivedContext = new VirtualNamespaceContext( + final VirtualNamespaceContext derivedContext = new VirtualNamespaceContext( (DerivedNamespaceBehaviour) potentialRaw); getNamespaceBehaviour(((DerivedNamespaceBehaviour) potentialRaw).getDerivedFrom()).addDerivedNamespace( derivedContext); @@ -164,7 +165,7 @@ class BuildGlobalContext extends NamespaceStorageSupport implements NamespaceBeh public StatementDefinitionContext getStatementDefinition(final QName name) { StatementDefinitionContext potential = definitions.get(name); if (potential == null) { - StatementSupport potentialRaw = supports.get(currentPhase).getStatementDefinition(name); + final StatementSupport potentialRaw = supports.get(currentPhase).getStatementDefinition(name); if (potentialRaw != null) { potential = new StatementDefinitionContext<>(potentialRaw); definitions.put(name, potential); @@ -174,7 +175,7 @@ class BuildGlobalContext extends NamespaceStorageSupport implements NamespaceBeh } public EffectiveModelContext build() throws SourceException, ReactorException { - for (ModelProcessingPhase phase : PHASE_EXECUTION_ORDER) { + for (final ModelProcessingPhase phase : PHASE_EXECUTION_ORDER) { startPhase(phase); loadPhaseStatements(); completePhaseActions(); @@ -185,15 +186,15 @@ class BuildGlobalContext extends NamespaceStorageSupport implements NamespaceBeh private EffectiveModelContext transform() { Preconditions.checkState(finishedPhase == ModelProcessingPhase.EFFECTIVE_MODEL); - List> rootStatements = new ArrayList<>(sources.size()); - for (SourceSpecificContext source : sources) { + final List> rootStatements = new ArrayList<>(sources.size()); + for (final SourceSpecificContext source : sources) { rootStatements.add(source.getRoot().buildDeclared()); } return new EffectiveModelContext(rootStatements); } public EffectiveSchemaContext buildEffective() throws ReactorException { - for (ModelProcessingPhase phase : PHASE_EXECUTION_ORDER) { + for (final ModelProcessingPhase phase : PHASE_EXECUTION_ORDER) { startPhase(phase); loadPhaseStatements(); completePhaseActions(); @@ -204,18 +205,18 @@ class BuildGlobalContext extends NamespaceStorageSupport implements NamespaceBeh private EffectiveSchemaContext transformEffective() throws ReactorException { Preconditions.checkState(finishedPhase == ModelProcessingPhase.EFFECTIVE_MODEL); - List> rootStatements = new ArrayList<>(sources.size()); - List> rootEffectiveStatements = new ArrayList<>(sources.size()); + final List> rootStatements = new ArrayList<>(sources.size()); + final List> rootEffectiveStatements = new ArrayList<>(sources.size()); SourceIdentifier sourceId = null; try { - for (SourceSpecificContext source : sources) { + for (final SourceSpecificContext source : sources) { final RootStatementContext root = source.getRoot(); sourceId = Utils.createSourceIdentifier(root); rootStatements.add(root.buildDeclared()); rootEffectiveStatements.add(root.buildEffective()); } - } catch (SourceException ex) { + } catch (final SourceException ex) { throw new SomeModifiersUnresolvedException(currentPhase, sourceId, ex); } finally { RecursiveObjectLeaker.cleanup(); @@ -226,7 +227,7 @@ class BuildGlobalContext extends NamespaceStorageSupport implements NamespaceBeh private void startPhase(final ModelProcessingPhase phase) { Preconditions.checkState(Objects.equals(finishedPhase, phase.getPreviousPhase())); - for (SourceSpecificContext source : sources) { + for (final SourceSpecificContext source : sources) { source.startPhase(phase); } currentPhase = phase; @@ -234,10 +235,10 @@ class BuildGlobalContext extends NamespaceStorageSupport implements NamespaceBeh private void loadPhaseStatements() throws ReactorException { Preconditions.checkState(currentPhase != null); - for (SourceSpecificContext source : sources) { + for (final SourceSpecificContext source : sources) { try { source.loadStatements(); - } catch (SourceException ex) { + } catch (final SourceException ex) { final SourceIdentifier sourceId = Utils.createSourceIdentifier(source.getRoot()); throw new SomeModifiersUnresolvedException(currentPhase, sourceId, ex); } @@ -247,12 +248,16 @@ class BuildGlobalContext extends NamespaceStorageSupport implements NamespaceBeh private SomeModifiersUnresolvedException addSourceExceptions(final List sourcesToProgress) { boolean addedCause = false; SomeModifiersUnresolvedException buildFailure = null; - for (SourceSpecificContext failedSource : sourcesToProgress) { - final SourceException sourceEx = failedSource.failModifiers(currentPhase); + for (final SourceSpecificContext failedSource : sourcesToProgress) { + final Optional optSourceEx = failedSource.failModifiers(currentPhase); + if (!optSourceEx.isPresent()) { + continue; + } + final SourceException sourceEx = optSourceEx.get(); // Workaround for broken logging implementations which ignore // suppressed exceptions - Throwable cause = sourceEx.getCause() != null ? sourceEx.getCause() : sourceEx; + final Throwable cause = sourceEx.getCause() != null ? sourceEx.getCause() : sourceEx; if (LOG.isDebugEnabled()) { LOG.error("Failed to parse YANG from source {}", failedSource, sourceEx); } else { @@ -264,7 +269,7 @@ class BuildGlobalContext extends NamespaceStorageSupport implements NamespaceBeh LOG.error("{} additional errors reported:", suppressed.length); int i = 1; - for (Throwable t : suppressed) { + for (final Throwable t : suppressed) { // FIXME: this should be configured in the appender, really if (LOG.isDebugEnabled()) { LOG.error("Error {}: {}", i, t.getMessage(), t); @@ -289,18 +294,18 @@ class BuildGlobalContext extends NamespaceStorageSupport implements NamespaceBeh private void completePhaseActions() throws ReactorException { Preconditions.checkState(currentPhase != null); - List sourcesToProgress = Lists.newArrayList(sources); + final List sourcesToProgress = Lists.newArrayList(sources); SourceIdentifier sourceId = null; try { boolean progressing = true; while (progressing) { // We reset progressing to false. progressing = false; - Iterator currentSource = sourcesToProgress.iterator(); + final Iterator currentSource = sourcesToProgress.iterator(); while (currentSource.hasNext()) { - SourceSpecificContext nextSourceCtx = currentSource.next(); + final SourceSpecificContext nextSourceCtx = currentSource.next(); sourceId = Utils.createSourceIdentifier(nextSourceCtx.getRoot()); - PhaseCompletionProgress sourceProgress = nextSourceCtx.tryToCompletePhase(currentPhase); + final PhaseCompletionProgress sourceProgress = nextSourceCtx.tryToCompletePhase(currentPhase); switch (sourceProgress) { case FINISHED: currentSource.remove(); @@ -317,12 +322,14 @@ class BuildGlobalContext extends NamespaceStorageSupport implements NamespaceBeh } } } - } catch (SourceException e) { + } catch (final SourceException e) { throw new SomeModifiersUnresolvedException(currentPhase, sourceId, e); } if (!sourcesToProgress.isEmpty()) { final SomeModifiersUnresolvedException buildFailure = addSourceExceptions(sourcesToProgress); - throw buildFailure; + if (buildFailure != null) { + throw buildFailure; + } } } diff --git a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/SourceSpecificContext.java b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/SourceSpecificContext.java index 7adb0d6f1d..5c8f67f312 100644 --- a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/SourceSpecificContext.java +++ b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/reactor/SourceSpecificContext.java @@ -19,6 +19,7 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.Objects; +import java.util.Optional; import javax.annotation.Nullable; import org.opendaylight.yangtools.concepts.Mutable; import org.opendaylight.yangtools.yang.common.QName; @@ -281,7 +282,7 @@ public class SourceSpecificContext implements NamespaceStorageNode, NamespaceBeh + finishedPhase + "]"; } - SourceException failModifiers(final ModelProcessingPhase identifier) { + Optional failModifiers(final ModelProcessingPhase identifier) { final List exceptions = new ArrayList<>(); for (final ModifierImpl mod : modifiers.get(identifier)) { try { @@ -291,11 +292,11 @@ public class SourceSpecificContext implements NamespaceStorageNode, NamespaceBeh } } - final String message = String.format("Yang model processing phase %s failed", identifier); if (exceptions.isEmpty()) { - return new InferenceException(message, root.getStatementSourceReference()); + return Optional.empty(); } + final String message = String.format("Yang model processing phase %s failed", identifier); final InferenceException e = new InferenceException(message, root.getStatementSourceReference(), exceptions.get(0)); final Iterator it = exceptions.listIterator(1); @@ -303,7 +304,7 @@ public class SourceSpecificContext implements NamespaceStorageNode, NamespaceBeh e.addSuppressed(it.next()); } - return e; + return Optional.of(e); } void loadStatements() throws SourceException { diff --git a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/AugmentStatementImpl.java b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/AugmentStatementImpl.java index 8d2f5e0aeb..82f208303d 100644 --- a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/AugmentStatementImpl.java +++ b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/AugmentStatementImpl.java @@ -130,7 +130,7 @@ public class AugmentStatementImpl extends AbstractDeclaredStatement> failed) { + /* + * Do not fail, if it is an uses-augment to an unknown node. + */ + if (Rfc6020Mapping.USES == augmentNode.getParentContext().getPublicDefinition()) { + final StatementContextBase targetNode = Utils.findNode(getSearchRoot(augmentNode), + augmentNode.getStatementArgument()); + if (Utils.isUnknownNode(targetNode)) { + augmentNode.setIsSupportedToBuildEffective(false); + LOG.warn( + "Uses-augment to unknown node {}. Augmentation has not been performed. At line: {}", + augmentNode.getStatementArgument(), augmentNode.getStatementSourceReference()); + return; + } + } + throw new InferenceException(augmentNode.getStatementSourceReference(), - "Augment target '%s' not found", augmentNode.getStatementArgument()); + "Augment target '%s' not found", augmentNode.getStatementArgument()); } }); } private static Mutable getSearchRoot(final Mutable augmentContext) { - Mutable parent = augmentContext.getParentContext(); + final Mutable parent = augmentContext.getParentContext(); // Augment is in uses - we need to augment instantiated nodes in parent. - if (Rfc6020Mapping.USES.equals(parent.getPublicDefinition())) { + if (Rfc6020Mapping.USES == parent.getPublicDefinition()) { return parent.getParentContext(); } return parent; diff --git a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/Utils.java b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/Utils.java index abd4ce7c74..05de618fa2 100644 --- a/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/Utils.java +++ b/yang/yang-parser-impl/src/main/java/org/opendaylight/yangtools/yang/parser/stmt/rfc6020/Utils.java @@ -579,7 +579,7 @@ public final class Utils { } public static boolean isUnknownNode(final StmtContext stmtCtx) { - return stmtCtx.getPublicDefinition().getDeclaredRepresentationClass() + return stmtCtx != null && stmtCtx.getPublicDefinition().getDeclaredRepresentationClass() .isAssignableFrom(UnknownStatementImpl.class); } @@ -697,7 +697,7 @@ public final class Utils { return false; } - public static SourceIdentifier createSourceIdentifier(RootStatementContext root) { + public static SourceIdentifier createSourceIdentifier(final RootStatementContext root) { final QNameModule qNameModule = root.getFromNamespace(ModuleCtxToModuleQName.class, root); if (qNameModule != null) { // creates SourceIdentifier for a module diff --git a/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/stmt/AugmentToExtensionTest.java b/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/stmt/AugmentToExtensionTest.java index 4e3a4e5fd5..071c6f51ed 100644 --- a/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/stmt/AugmentToExtensionTest.java +++ b/yang/yang-parser-impl/src/test/java/org/opendaylight/yangtools/yang/stmt/AugmentToExtensionTest.java @@ -11,7 +11,6 @@ import static org.junit.Assert.assertTrue; import java.net.URISyntaxException; import java.util.Set; -import org.junit.Ignore; import org.junit.Test; import org.opendaylight.yangtools.yang.common.QName; import org.opendaylight.yangtools.yang.model.api.AugmentationSchema; @@ -36,7 +35,6 @@ public class AugmentToExtensionTest { * */ @Test - @Ignore public void testCorrectPathIntoUnsupportedTarget() throws URISyntaxException, SourceException, ReactorException { try { diff --git a/yang/yang-parser-impl/src/test/resources/augment-to-extension-test/incorrect-path/augment-module.yang b/yang/yang-parser-impl/src/test/resources/augment-to-extension-test/incorrect-path/augment-module.yang index 17b3b4c72d..7370e12df8 100644 --- a/yang/yang-parser-impl/src/test/resources/augment-to-extension-test/incorrect-path/augment-module.yang +++ b/yang/yang-parser-impl/src/test/resources/augment-to-extension-test/incorrect-path/augment-module.yang @@ -28,7 +28,7 @@ module augment-module { container my-container { uses my-grouping { - augment "my-extension-name/input/a" { + augment "my-extension-name-a/input" { leaf-list my-leaf-list { type string; } -- 2.36.6