From: Robert Varga Date: Wed, 6 Jun 2018 11:37:04 +0000 (+0200) Subject: Fix SchemaContextUtil RevisionAwareXPath resolution X-Git-Tag: v2.0.6~2 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=1636eaae9a133d79301b7ebb02ae8d6a5fc38117;p=yangtools.git Fix SchemaContextUtil RevisionAwareXPath resolution The mapping of yang-model-api SchemaNode hierarchy to YANG namespaces is confusing, which is compounded by naming -- invoking notions of equivalence when there is none. This is very much evident when dealing with children identified by QName in various contexts -- they usually work, but break down when choice/case nodes are encountered, because their addressability is different based on context -- they are addressable via 'node identifier', but are invisble to XPath contexts. Document the difference in extant DataNodeContainer and DataSchemaNode API and introduce DataNodeContainer.findDataNode(), which follows RFC7950 'data tree', not 'schema tree'. Change-Id: Ic570da8dd97a686423c7e2b28e0db0d9c6907271 JIRA: YANGTOOLS-885 Signed-off-by: Robert Varga --- diff --git a/yang/yang-model-api/src/main/java/org/opendaylight/yangtools/yang/model/api/DataNodeContainer.java b/yang/yang-model-api/src/main/java/org/opendaylight/yangtools/yang/model/api/DataNodeContainer.java index 308697a7cf..60e076d22c 100644 --- a/yang/yang-model-api/src/main/java/org/opendaylight/yangtools/yang/model/api/DataNodeContainer.java +++ b/yang/yang-model-api/src/main/java/org/opendaylight/yangtools/yang/model/api/DataNodeContainer.java @@ -7,7 +7,14 @@ */ package org.opendaylight.yangtools.yang.model.api; +import static com.google.common.base.Preconditions.checkArgument; +import static java.util.Objects.requireNonNull; + +import com.google.common.annotations.Beta; +import java.util.Arrays; import java.util.Collection; +import java.util.Iterator; +import java.util.NoSuchElementException; import java.util.Optional; import java.util.Set; import javax.annotation.Nullable; @@ -17,7 +24,6 @@ import org.opendaylight.yangtools.yang.common.QName; * Node which can contains other nodes. */ public interface DataNodeContainer { - /** * Returns set of all newly defined types within this DataNodeContainer. * @@ -26,9 +32,13 @@ public interface DataNodeContainer { Set> getTypeDefinitions(); /** - * Returns set of all child nodes defined within this DataNodeContainer. - * Although the return type is a collection, each node is guaranteed to - * be present at most once. + * Returns set of all child nodes defined within this DataNodeContainer. Although the return type is a collection, + * each node is guaranteed to be present at most once. + * + *

+ * Note that the nodes returned are NOT {@code data nodes}, but rather {@link DataSchemaNode}s, + * hence {@link ChoiceSchemaNode} and {@link CaseSchemaNode} are present instead of their children. This + * is consistent with {@code schema tree}. * * @return child nodes in lexicographical order */ @@ -44,11 +54,15 @@ public interface DataNodeContainer { /** * Returns the child node corresponding to the specified name. * - * @param name - * QName of child - * @return child node of this DataNodeContainer if child with given name is present, null otherwise + *

+ * Note that the nodes searched are NOT {@code data nodes}, but rather {@link DataSchemaNode}s, + * hence {@link ChoiceSchemaNode} and {@link CaseSchemaNode} are returned instead of their matching children. This + * is consistent with {@code schema tree}. * + * @param name QName of child + * @return child node of this DataNodeContainer if child with given name is present, null otherwise * @deprecated Use {@link #findDataChildByName(QName)} instead. + * @throws NullPointerException if {@code name} is null */ @Deprecated @Nullable default DataSchemaNode getDataChildByName(final QName name) { @@ -58,10 +72,13 @@ public interface DataNodeContainer { /** * Returns the child node corresponding to the specified name. * - * @param name - * QName of child + *

+ * Note that the nodes searched are NOT {@code data nodes}, but rather {@link DataSchemaNode}s, + * hence {@link ChoiceSchemaNode} and {@link CaseSchemaNode} are returned instead of their matching children. + * + * @param name QName of child * @return child node of this DataNodeContainer if child with given name is present, empty otherwise - * @throws NullPointerException if name is null + * @throws NullPointerException if {@code name} is null */ Optional findDataChildByName(QName name); @@ -71,4 +88,78 @@ public interface DataNodeContainer { * @return Set of all uses nodes defined within this DataNodeContainer */ Set getUses(); + + /** + * Returns a {@code data node} identified by a QName. This method is distinct from + * {@link #findDataChildByName(QName)} in that it skips over {@link ChoiceSchemaNode}s and {@link CaseSchemaNode}s, + * hence mirroring layout of the {@code data tree}, not {@code schema tree}. + * + * @param name QName identifier of the data node + * @return Direct or indirect child of this DataNodeContainer which is a {@code data node}, empty otherwise + * @throws NullPointerException if {@code name} is null + */ + @Beta + default Optional findDataTreeChild(final QName name) { + // First we try to find a direct child and check if it is a data node (as per RFC7950) + final Optional optDataChild = findDataChildByName(name); + if (HelperMethods.isDataNode(optDataChild)) { + return optDataChild; + } + + // There either is no such node present, or there are Choice/CaseSchemaNodes with the same name involved, + // hence we have to resort to a full search. + for (DataSchemaNode child : getChildNodes()) { + if (child instanceof ChoiceSchemaNode) { + for (CaseSchemaNode choiceCase : ((ChoiceSchemaNode) child).getCases().values()) { + final Optional caseChild = choiceCase.findDataTreeChild(name); + if (caseChild.isPresent()) { + return caseChild; + } + } + } + } + + return Optional.empty(); + } + + /** + * Returns a {@code data node} identified by a series of QNames. This is equivalent to incrementally calling + * {@link #findDataTreeChild(QName)}. + * + * @param path Series of QNames towards identifying the requested data node + * @return Direct or indirect child of this DataNodeContainer which is a {@code data node}, empty otherwise + * @throws IllegalArgumentException if {@code path} is determined to go beyond a not-container-nor-list node. + * @throws NoSuchElementException if {@code path} is empty + * @throws NullPointerException if {@code path} is null or contains a null + */ + @Beta + default Optional findDataTreeChild(final QName... path) { + return findDataTreeChild(Arrays.asList(path)); + } + + /** + * Returns a {@code data node} identified by a series of QNames. This is equivalent to incrementally calling + * {@link #findDataTreeChild(QName)}. + * + * @param path Series of QNames towards identifying the requested data node + * @return Direct or indirect child of this DataNodeContainer which is a {@code data node}, empty otherwise + * @throws IllegalArgumentException if {@code path} is determined to go beyond a not-container-nor-list node. + * @throws NoSuchElementException if {@code path} is empty + * @throws NullPointerException if {@code path} is null or contains a null + */ + @Beta + default Optional findDataTreeChild(final Iterable path) { + final Iterator it = path.iterator(); + DataNodeContainer parent = this; + do { + final Optional optChild = parent.findDataTreeChild(requireNonNull(it.next())); + if (!optChild.isPresent() || !it.hasNext()) { + return optChild; + } + + final DataSchemaNode child = optChild.get(); + checkArgument(child instanceof DataNodeContainer, "Path %s extends beyond terminal child %s", path, child); + parent = (DataNodeContainer) child; + } while (true); + } } diff --git a/yang/yang-model-api/src/main/java/org/opendaylight/yangtools/yang/model/api/DataSchemaNode.java b/yang/yang-model-api/src/main/java/org/opendaylight/yangtools/yang/model/api/DataSchemaNode.java index c0e98d6256..ab6650eb5a 100644 --- a/yang/yang-model-api/src/main/java/org/opendaylight/yangtools/yang/model/api/DataSchemaNode.java +++ b/yang/yang-model-api/src/main/java/org/opendaylight/yangtools/yang/model/api/DataSchemaNode.java @@ -8,7 +8,10 @@ package org.opendaylight.yangtools.yang.model.api; /** - * Data Schema Node represents abstract supertype from which all data tree definitions are derived. + * Data Schema Node represents abstract supertype from which all data tree definitions are derived. Unlike what + * the name would suggest, this interface corresponds more to RFC7950 {@code data definition statement} than to + * {@code data node}, yet it notably does not include {@link UsesNode} and {@link AugmentationSchemaNode}, which are + * resolved separately. * *

* Common interface is composed of {@link #isConfiguration()}, governing validity in config/operation data stores diff --git a/yang/yang-model-api/src/main/java/org/opendaylight/yangtools/yang/model/api/HelperMethods.java b/yang/yang-model-api/src/main/java/org/opendaylight/yangtools/yang/model/api/HelperMethods.java new file mode 100644 index 0000000000..054f5cd6e4 --- /dev/null +++ b/yang/yang-model-api/src/main/java/org/opendaylight/yangtools/yang/model/api/HelperMethods.java @@ -0,0 +1,33 @@ +/* + * Copyright (c) 2018 Pantheon Technologies, 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.model.api; + +import java.util.Optional; +import org.eclipse.jdt.annotation.NonNullByDefault; + +/** + * Package-internal helper methods for use in interface default methods. + * + * @author Robert Varga + */ +@NonNullByDefault +final class HelperMethods { + private HelperMethods() { + + } + + static boolean isDataNode(final Optional optNode) { + return optNode.isPresent() && isDataNode(optNode.get()); + } + + private static boolean isDataNode(final DataSchemaNode node) { + return node instanceof ContainerSchemaNode || node instanceof LeafSchemaNode + || node instanceof LeafListSchemaNode || node instanceof ListSchemaNode + || node instanceof AnyDataSchemaNode || node instanceof AnyXmlSchemaNode; + } +} diff --git a/yang/yang-model-api/src/main/java/org/opendaylight/yangtools/yang/model/api/SchemaContext.java b/yang/yang-model-api/src/main/java/org/opendaylight/yangtools/yang/model/api/SchemaContext.java index 9a5c88593c..b4189cc6ec 100644 --- a/yang/yang-model-api/src/main/java/org/opendaylight/yangtools/yang/model/api/SchemaContext.java +++ b/yang/yang-model-api/src/main/java/org/opendaylight/yangtools/yang/model/api/SchemaContext.java @@ -7,6 +7,7 @@ */ package org.opendaylight.yangtools.yang.model.api; +import com.google.common.annotations.Beta; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; import java.net.URI; @@ -210,4 +211,10 @@ public interface SchemaContext extends ContainerSchemaNode { default Optional getWhenCondition() { return Optional.empty(); } + + @Beta + @Override + default Optional findDataTreeChild(final QName name) { + return findModule(name.getModule()).flatMap(mod -> mod.findDataTreeChild(name)); + } } diff --git a/yang/yang-model-util/src/main/java/org/opendaylight/yangtools/yang/model/util/SchemaContextUtil.java b/yang/yang-model-util/src/main/java/org/opendaylight/yangtools/yang/model/util/SchemaContextUtil.java index bdf8a672af..2d2240c382 100644 --- a/yang/yang-model-util/src/main/java/org/opendaylight/yangtools/yang/model/util/SchemaContextUtil.java +++ b/yang/yang-model-util/src/main/java/org/opendaylight/yangtools/yang/model/util/SchemaContextUtil.java @@ -124,6 +124,18 @@ public final class SchemaContextUtil { * Non-conditional Revision Aware XPath, or null if the * DataSchemaNode is not present in Schema Context. */ + // FIXME: This entire method is ill-defined, as the resolution process depends on where the XPath is defined -- + // notably RPCs, actions and notifications modify the data tree temporarily. See sections 6.4.1 and 9.9.2 + // of RFC7950. + // + // Most notably we need to understand whether the XPath is being resolved in the data tree, or as part of + // a notification/action/RPC, as then the SchemaContext grows tentative nodes ... which could be addressed + // via a derived SchemaContext (i.e. this class would have to have a + // + // SchemaContext notificationSchemaContext(SchemaContext delegate, NotificationDefinition notif) + // + // which would then be passed in to a method similar to this one. In static contexts, like MD-SAL codegen, + // that feels like an overkill. public static SchemaNode findDataSchemaNode(final SchemaContext context, final Module module, final RevisionAwareXPath nonCondXPath) { Preconditions.checkArgument(context != null, "Schema Context reference cannot be NULL"); @@ -135,10 +147,12 @@ public final class SchemaContextUtil { Preconditions.checkArgument(strXPath.indexOf('[') == -1, "Revision Aware XPath may not contain a condition"); if (nonCondXPath.isAbsolute()) { - final List qnamedPath = xpathToQNamePath(context, module, strXPath); - if (qnamedPath != null) { - return findNodeInSchemaContext(context, qnamedPath); - } + final List path = xpathToQNamePath(context, module, strXPath); + + // We do not have enough information about resolution context, hence cannot account for actions, RPCs + // and notifications. We therefore attempt to make a best estimate, but this can still fail. + final Optional pureData = context.findDataTreeChild(path); + return pureData.isPresent() ? pureData.get() : findNodeInSchemaContext(context, path); } } return null; @@ -181,6 +195,18 @@ public final class SchemaContextUtil { * given relative Revision Aware XPath, otherwise will return * null. */ + // FIXME: This entire method is ill-defined, as the resolution process depends on where the XPath is defined -- + // notably RPCs, actions and notifications modify the data tree temporarily. See sections 6.4.1 and 9.9.2 + // of RFC7950. + // + // Most notably we need to understand whether the XPath is being resolved in the data tree, or as part of + // a notification/action/RPC, as then the SchemaContext grows tentative nodes ... which could be addressed + // via a derived SchemaContext (i.e. this class would have to have a + // + // SchemaContext notificationSchemaContext(SchemaContext delegate, NotificationDefinition notif) + // + // which would then be passed in to a method similar to this one. In static contexts, like MD-SAL codegen, + // that feels like an overkill. public static SchemaNode findDataSchemaNodeForRelativeXPath(final SchemaContext context, final Module module, final SchemaNode actualSchemaNode, final RevisionAwareXPath relativeXPath) { Preconditions.checkArgument(context != null, "Schema Context reference cannot be NULL"); @@ -195,9 +221,10 @@ public final class SchemaContextUtil { if (actualNodePath != null) { final Iterable qnamePath = resolveRelativeXPath(context, module, relativeXPath, actualSchemaNode); - if (qnamePath != null) { - return findNodeInSchemaContext(context, qnamePath); - } + // We do not have enough information about resolution context, hence cannot account for actions, RPCs + // and notifications. We therefore attempt to make a best estimate, but this can still fail. + final Optional pureData = context.findDataTreeChild(qnamePath); + return pureData.isPresent() ? pureData.get() : findNodeInSchemaContext(context, qnamePath); } return null; } diff --git a/yang/yang-model-util/src/test/java/org/opendaylight/yangtools/yang/model/util/SchemaContextUtilTest.java b/yang/yang-model-util/src/test/java/org/opendaylight/yangtools/yang/model/util/SchemaContextUtilTest.java index 580ec72c53..f455b253b9 100644 --- a/yang/yang-model-util/src/test/java/org/opendaylight/yangtools/yang/model/util/SchemaContextUtilTest.java +++ b/yang/yang-model-util/src/test/java/org/opendaylight/yangtools/yang/model/util/SchemaContextUtilTest.java @@ -37,6 +37,7 @@ public class SchemaContextUtilTest { public void testFindDummyData() { MockitoAnnotations.initMocks(this); doReturn(Optional.empty()).when(mockSchemaContext).findModule(any(QNameModule.class)); + doReturn(Optional.empty()).when(mockSchemaContext).findDataTreeChild(any(Iterable.class)); doReturn("test").when(mockModule).getName(); doReturn("test").when(mockModule).getPrefix(); diff --git a/yang/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/parser/rfc7950/repo/SchemaContextUtilTest.java b/yang/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/parser/rfc7950/repo/SchemaContextUtilTest.java index 2491a885a1..acd7f70080 100644 --- a/yang/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/parser/rfc7950/repo/SchemaContextUtilTest.java +++ b/yang/yang-parser-rfc7950/src/test/java/org/opendaylight/yangtools/yang/parser/rfc7950/repo/SchemaContextUtilTest.java @@ -55,6 +55,7 @@ public class SchemaContextUtilTest { public void testFindDummyData() { MockitoAnnotations.initMocks(this); doReturn(Optional.empty()).when(mockSchemaContext).findModule(any(QNameModule.class)); + doReturn(Optional.empty()).when(mockSchemaContext).findDataTreeChild(any(Iterable.class)); doReturn(URI.create("dummy")).when(mockModule).getNamespace(); doReturn(Optional.empty()).when(mockModule).getRevision();