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 <pkajsa@cisco.com>

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 62822474d7042f4659c9bde7afcaced862b000ef..6480ab9af0d2f5431a4ef7b8de2912591ff53295 100644 (file)
--- 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<ValidationBundleType, Collection<?>> validationBundle : supportedValidation.entrySet()) {
+        for (final Entry<ValidationBundleType, Collection<?>> validationBundle : supportedValidation.entrySet()) {
             addToNs(ValidationBundlesNamespace.class, validationBundle.getKey(), validationBundle.getValue());
         }
 
@@ -130,7 +131,7 @@ class BuildGlobalContext extends NamespaceStorageSupport implements NamespaceBeh
             final Class<N> type) {
         NamespaceBehaviourWithListeners<?, ?, ?> potential = supportedNamespaces.get(type);
         if (potential == null) {
-            NamespaceBehaviour<K, V, N> potentialRaw = supports.get(currentPhase).getNamespaceBehaviour(type);
+            final NamespaceBehaviour<K, V, N> 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 <K, V, N extends IdentifierNamespace<K, V>> NamespaceBehaviourWithListeners<K, V, N> createNamespaceContext(
             final NamespaceBehaviour<K, V, N> 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<DeclaredStatement<?>> rootStatements = new ArrayList<>(sources.size());
-        for (SourceSpecificContext source : sources) {
+        final List<DeclaredStatement<?>> 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<DeclaredStatement<?>> rootStatements = new ArrayList<>(sources.size());
-        List<EffectiveStatement<?, ?>> rootEffectiveStatements = new ArrayList<>(sources.size());
+        final List<DeclaredStatement<?>> rootStatements = new ArrayList<>(sources.size());
+        final List<EffectiveStatement<?, ?>> 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<SourceSpecificContext> sourcesToProgress) {
         boolean addedCause = false;
         SomeModifiersUnresolvedException buildFailure = null;
-        for (SourceSpecificContext failedSource : sourcesToProgress) {
-            final SourceException sourceEx = failedSource.failModifiers(currentPhase);
+        for (final SourceSpecificContext failedSource : sourcesToProgress) {
+            final Optional<SourceException> 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<SourceSpecificContext> sourcesToProgress = Lists.newArrayList(sources);
+        final List<SourceSpecificContext> sourcesToProgress = Lists.newArrayList(sources);
         SourceIdentifier sourceId = null;
         try {
             boolean progressing = true;
             while (progressing) {
                 // We reset progressing to false.
                 progressing = false;
-                Iterator<SourceSpecificContext> currentSource = sourcesToProgress.iterator();
+                final Iterator<SourceSpecificContext> 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 7adb0d6f1d8a8068a3dae535fcb9f7a4f9d6e9af..5c8f67f31294bf8358e20ba17be2ee48dff48a9a 100644 (file)
--- 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<SourceException> failModifiers(final ModelProcessingPhase identifier) {
         final List<SourceException> 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<SourceException> 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 8d2f5e0aeb8207948b78c569eddc8b111b5d8c3d..82f208303d8e04d15399d350c472ae3d072646c9 100644 (file)
--- 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<SchemaNodeId
                         AugmentUtils.copyFromSourceToTarget(augmentSourceCtx, augmentTargetCtx);
                         augmentTargetCtx.addEffectiveSubstatement(augmentSourceCtx);
                         updateAugmentOrder(augmentSourceCtx);
-                    } catch (SourceException e) {
+                    } catch (final SourceException e) {
                         LOG.debug("Failed to add augmentation {} defined at {}",
                             augmentTargetCtx.getStatementSourceReference(),
                                 augmentSourceCtx.getStatementSourceReference(), e);
@@ -152,16 +152,31 @@ public class AugmentStatementImpl extends AbstractDeclaredStatement<SchemaNodeId
 
                 @Override
                 public void prerequisiteFailed(final Collection<? extends ModelActionBuilder.Prerequisite<?>> 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 abd4ce7c749e533dded6d2ccc1168c8a41a92d53..05de618fa2bebb27add0b511c3843a165717cfb3 100644 (file)
--- 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 4e3a4e5fd5de3c841b821aa8ae4418860b5c319a..071c6f51ed97239123b17dbd7366a567f6e20afa 100644 (file)
--- 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 17b3b4c72d4337634bfd2befea6decb366fae065..7370e12df85a13bc7b7f4c6c0419f63920a9a81d 100644 (file)
--- 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;
                 }