Reconcile pathArgument() and isKeyedEntry() 89/106089/1
authorRobert Varga <robert.varga@pantheon.tech>
Sat, 20 May 2023 23:06:18 +0000 (01:06 +0200)
committerRobert Varga <robert.varga@pantheon.tech>
Sat, 20 May 2023 23:11:23 +0000 (01:11 +0200)
We have a tad of a mess here. isKeyedEntry() is really indicating
whether the pathArgument() is useable. Merge the logic to enable sane
operation:
- pathStep() returns @Nullable NodeIdentifier, effectively acting as
  !isKeyedEntry()
- getPathStep() returns a @NonNull NodeIdentifier or throws.

Based on this, update PathMixin, which is now guaranteed to return a
NodeIdentifier (by deferring to getPathStep()).

JIRA: YANGTOOLS-1413
Change-Id: Ic7f566d37cf0867122a472e879b2665624d5bc74
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
12 files changed:
data/yang-data-util/src/main/java/org/opendaylight/yangtools/yang/data/util/DataSchemaContextNode.java
data/yang-data-util/src/main/java/org/opendaylight/yangtools/yang/data/util/XpathStringParsingPathArgumentBuilder.java
data/yang-data-util/src/main/java/org/opendaylight/yangtools/yang/data/util/impl/legacy/AbstractDataSchemaContextNode.java
data/yang-data-util/src/main/java/org/opendaylight/yangtools/yang/data/util/impl/legacy/AbstractInteriorContextNode.java
data/yang-data-util/src/main/java/org/opendaylight/yangtools/yang/data/util/impl/legacy/AbstractLeafContextNode.java
data/yang-data-util/src/main/java/org/opendaylight/yangtools/yang/data/util/impl/legacy/AbstractListItemContextNode.java
data/yang-data-util/src/main/java/org/opendaylight/yangtools/yang/data/util/impl/legacy/AbstractListLikeContextNode.java
data/yang-data-util/src/main/java/org/opendaylight/yangtools/yang/data/util/impl/legacy/AbstractMixinContextNode.java
data/yang-data-util/src/main/java/org/opendaylight/yangtools/yang/data/util/impl/legacy/ChoiceNodeContextNode.java
data/yang-data-util/src/main/java/org/opendaylight/yangtools/yang/data/util/impl/legacy/DataContainerContextNode.java
data/yang-data-util/src/main/java/org/opendaylight/yangtools/yang/data/util/impl/legacy/LeafListEntryContextNode.java
data/yang-data-util/src/main/java/org/opendaylight/yangtools/yang/data/util/impl/legacy/MapItemContextNode.java

index 35bbd5bf4a5bb6ac6f9172a2260889aecd9da994..c3108321da852dbd75b21637ba60af70960e4cc0 100644 (file)
@@ -12,9 +12,12 @@ import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.jdt.annotation.Nullable;
 import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument;
 import org.opendaylight.yangtools.yang.data.api.schema.ChoiceNode;
+import org.opendaylight.yangtools.yang.data.api.schema.LeafSetEntryNode;
 import org.opendaylight.yangtools.yang.data.api.schema.LeafSetNode;
+import org.opendaylight.yangtools.yang.data.api.schema.MapEntryNode;
 import org.opendaylight.yangtools.yang.data.api.schema.MapNode;
 import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode;
 import org.opendaylight.yangtools.yang.data.api.schema.UnkeyedListNode;
@@ -50,11 +53,13 @@ public interface DataSchemaContextNode {
     */
     sealed interface PathMixin extends DataSchemaContextNode permits AbstractMixinContextNode {
         /**
-         * The mixed-in {@link PathArgument}.
+         * The mixed-in {@link NodeIdentifier}.
          *
-         * @return Mixed-in PathArgument
+         * @return Mixed-in NodeIdentifier
          */
-        @NonNull PathArgument mixinPathArgument();
+        default @NonNull NodeIdentifier mixinPathStep() {
+            return getPathStep();
+        }
     }
 
     /**
@@ -66,12 +71,27 @@ public interface DataSchemaContextNode {
 
     @NonNull DataSchemaNode getDataSchemaNode();
 
-    // FIXME: YANGTOOLS-1413: this idea is wrong -- if does the wrong thing for items of leaf-list and keyed list
-    //                        because those identifiers need a value.
-    @NonNull PathArgument pathArgument();
+    /**
+     * Return the fixed {@link YangInstanceIdentifier} step, if available. This method returns {@code null} for contexts
+     * like {@link MapEntryNode} and {@link LeafSetEntryNode}, where the step depends on the actual node value.
+     *
+     * @return A {@link NodeIdentifier}, or {@code null}
+     */
+    @Nullable NodeIdentifier pathStep();
 
-    // FIXME: YANGTOOLS-1413: document this method and (most likely) split it out to a separate interface
-    boolean isKeyedEntry();
+    /**
+     * Return the fixed {@link YangInstanceIdentifier} step.
+     *
+     * @return A {@link NodeIdentifier}
+     * @throws UnsupportedOperationException if this node does not have fixed step
+     */
+    default @NonNull NodeIdentifier getPathStep() {
+        final var arg = pathStep();
+        if (arg != null) {
+            return arg;
+        }
+        throw new UnsupportedOperationException(this + " does not have a fixed path step");
+    }
 
     /**
      * Find a child node identifier by its {@link PathArgument}.
index c80d102d01389e340cc973940f14828c09a66729..881912dca7f36edb3836c08ef6c2b5cb75049f3c 100644 (file)
@@ -18,6 +18,7 @@ import org.eclipse.jdt.annotation.NonNull;
 import org.opendaylight.yangtools.concepts.Mutable;
 import org.opendaylight.yangtools.yang.common.QName;
 import org.opendaylight.yangtools.yang.common.QNameModule;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifierWithPredicates;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeWithValue;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument;
@@ -115,7 +116,7 @@ final class XpathStringParsingPathArgumentBuilder implements Mutable {
         current = current.getChild(name);
         checkValid(current != null, "%s is not correct schema node identifier.", name);
         while (current instanceof PathMixin mixin) {
-            product.add(mixin.mixinPathArgument());
+            product.add(mixin.mixinPathStep());
             current = current.getChild(name);
         }
         stack.enterDataTree(name);
@@ -138,10 +139,12 @@ final class XpathStringParsingPathArgumentBuilder implements Mutable {
      * @return PathArgument representing node selection with predictes
      */
     private PathArgument computeIdentifierWithPredicate(final QName name) {
-        final DataSchemaContextNode currentNode = nextContextNode(name);
-        checkValid(currentNode.isKeyedEntry(), "Entry %s does not allow specifying predicates.", name);
+        final var currentNode = nextContextNode(name);
+        if (currentNode.pathStep() != null) {
+            throw iae("Entry %s does not allow specifying predicates.", name);
+        }
 
-        ImmutableMap.Builder<QName, Object> keyValues = ImmutableMap.builder();
+        final var keyValues = ImmutableMap.<QName, Object>builder();
         while (!allCharactersConsumed() && PRECONDITION_START == currentChar()) {
             skipCurrentChar();
             skipWhitespaces();
@@ -183,10 +186,13 @@ final class XpathStringParsingPathArgumentBuilder implements Mutable {
         return tmp.resolveLeafref(type);
     }
 
-    private PathArgument computeIdentifier(final QName name) {
-        DataSchemaContextNode currentNode = nextContextNode(name);
-        checkValid(!currentNode.isKeyedEntry(), "Entry %s requires key or value predicate to be present", name);
-        return currentNode.pathArgument();
+    private @NonNull NodeIdentifier computeIdentifier(final QName name) {
+        final var currentNode = nextContextNode(name);
+        final var currentArg = currentNode.pathStep();
+        if (currentArg == null) {
+            throw iae("Entry %s requires key or value predicate to be present", name);
+        }
+        return currentArg;
     }
 
     /**
index c723717df921f54c115f2f5530f5e74c2ade6f27..b7cc3e2bc62105d7154eac41d208c1d6f221d9c5 100644 (file)
@@ -15,6 +15,7 @@ import java.util.Set;
 import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.jdt.annotation.Nullable;
 import org.opendaylight.yangtools.yang.common.QName;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument;
 import org.opendaylight.yangtools.yang.data.util.DataSchemaContextNode;
 import org.opendaylight.yangtools.yang.model.api.AnydataSchemaNode;
@@ -35,14 +36,13 @@ import org.opendaylight.yangtools.yang.model.util.SchemaInferenceStack;
  * since the mapping is not one-to-one.
  */
 public abstract class AbstractDataSchemaContextNode implements DataSchemaContextNode {
-    // FIXME: YANGTOOLS-1413: this field should not be needed
-    private final @NonNull PathArgument pathArgument;
+    private final @Nullable NodeIdentifier pathStep;
 
     final @NonNull DataSchemaNode dataSchemaNode;
 
-    AbstractDataSchemaContextNode(final PathArgument pathArgument, final DataSchemaNode dataSchemaNode) {
+    AbstractDataSchemaContextNode(final NodeIdentifier pathStep, final DataSchemaNode dataSchemaNode) {
         this.dataSchemaNode = requireNonNull(dataSchemaNode);
-        this.pathArgument = requireNonNull(pathArgument);
+        this.pathStep = pathStep;
     }
 
     @Override
@@ -51,13 +51,8 @@ public abstract class AbstractDataSchemaContextNode implements DataSchemaContext
     }
 
     @Override
-    public final PathArgument pathArgument() {
-        return pathArgument;
-    }
-
-    @Override
-    public boolean isKeyedEntry() {
-        return false;
+    public final NodeIdentifier pathStep() {
+        return pathStep;
     }
 
     Set<QName> qnameIdentifiers() {
index be5a0833424ad8863e2daf49e9935a8742ffcd80..948d3e9cfc416d743f98fab71fbc11cde393554f 100644 (file)
@@ -7,11 +7,11 @@
  */
 package org.opendaylight.yangtools.yang.data.util.impl.legacy;
 
-import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier;
 import org.opendaylight.yangtools.yang.model.api.DataSchemaNode;
 
 abstract class AbstractInteriorContextNode extends AbstractDataSchemaContextNode {
-    AbstractInteriorContextNode(final PathArgument pathArgument, final DataSchemaNode schema) {
-        super(pathArgument, schema);
+    AbstractInteriorContextNode(final NodeIdentifier pathStep, final DataSchemaNode schema) {
+        super(pathStep, schema);
     }
 }
\ No newline at end of file
index 6c04c33aa00818fda5e34c16b428b4cabd8e1e91..5db89c8080f6491a2db051ef91ed8661ebb60e63 100644 (file)
@@ -8,14 +8,15 @@
 package org.opendaylight.yangtools.yang.data.util.impl.legacy;
 
 import org.opendaylight.yangtools.yang.common.QName;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument;
 import org.opendaylight.yangtools.yang.data.util.DataSchemaContextNode;
 import org.opendaylight.yangtools.yang.model.api.DataSchemaNode;
 import org.opendaylight.yangtools.yang.model.util.SchemaInferenceStack;
 
 abstract class AbstractLeafContextNode extends AbstractDataSchemaContextNode {
-    AbstractLeafContextNode(final PathArgument pathArgument, final DataSchemaNode schema) {
-        super(pathArgument, schema);
+    AbstractLeafContextNode(final NodeIdentifier pathStep, final DataSchemaNode schema) {
+        super(pathStep, schema);
     }
 
     @Override
index ffaea49fc5e5f57eefd7d83f240dd9beee042e5f..e47896a2dba8225f3e31dde202aaf8470240cf01 100644 (file)
@@ -7,7 +7,7 @@
  */
 package org.opendaylight.yangtools.yang.data.util.impl.legacy;
 
-import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier;
 import org.opendaylight.yangtools.yang.model.api.DataNodeContainer;
 import org.opendaylight.yangtools.yang.model.api.DataSchemaNode;
 import org.opendaylight.yangtools.yang.model.util.SchemaInferenceStack;
@@ -17,9 +17,9 @@ import org.opendaylight.yangtools.yang.model.util.SchemaInferenceStack;
  * {@link ListItemContextNode}.
  */
 abstract class AbstractListItemContextNode extends DataContainerContextNode {
-    AbstractListItemContextNode(final PathArgument pathArgument, final DataNodeContainer container,
+    AbstractListItemContextNode(final NodeIdentifier pathStep, final DataNodeContainer container,
             final DataSchemaNode schema) {
-        super(pathArgument, container, schema);
+        super(pathStep, container, schema);
     }
 
     @Override
index 6737e73bada4fe26756f3edbdd75925274bc083e..0bac68bbb91af662cf48477a3926eba5793dfa41 100644 (file)
@@ -25,13 +25,13 @@ abstract sealed class AbstractListLikeContextNode extends AbstractMixinContextNo
     }
 
     @Override
-    protected final DataSchemaContextNode enterChild(final QName child, final SchemaInferenceStack stack) {
+    final DataSchemaContextNode enterChild(final QName child, final SchemaInferenceStack stack) {
         // Stack is already pointing to the corresponding statement, now we are just working with the child
         return getChild(child);
     }
 
     @Override
-    protected final DataSchemaContextNode enterChild(final PathArgument child, final SchemaInferenceStack stack) {
+    final DataSchemaContextNode enterChild(final PathArgument child, final SchemaInferenceStack stack) {
         return getChild(child);
     }
 }
index 422daa092a829c20720b37f565c861a8c2909cd3..be0d221a7217b6af6c482c9e26e0a1dd9f581273 100644 (file)
@@ -8,7 +8,6 @@
 package org.opendaylight.yangtools.yang.data.util.impl.legacy;
 
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier;
-import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument;
 import org.opendaylight.yangtools.yang.data.util.DataSchemaContextNode.PathMixin;
 import org.opendaylight.yangtools.yang.model.api.DataSchemaNode;
 
@@ -17,9 +16,4 @@ public abstract sealed class AbstractMixinContextNode extends AbstractInteriorCo
     AbstractMixinContextNode(final DataSchemaNode schema) {
         super(NodeIdentifier.create(schema.getQName()), schema);
     }
-
-    @Override
-    public PathArgument mixinPathArgument() {
-        return pathArgument();
-    }
 }
\ No newline at end of file
index a473ceada4782b58228a0d0135b6e5dc1083c404..9457633e24bfe9435ed09660d057c6426daf5345 100644 (file)
@@ -33,7 +33,7 @@ final class ChoiceNodeContextNode extends AbstractMixinContextNode {
         for (var caze : schema.getCases()) {
             for (var cazeChild : caze.getChildNodes()) {
                 final var childOp = AbstractDataSchemaContextNode.of(cazeChild);
-                byArgBuilder.put(childOp.pathArgument(), childOp);
+                byArgBuilder.put(childOp.getPathStep(), childOp);
                 childToCaseBuilder.put(childOp, caze.getQName());
                 for (QName qname : childOp.qnameIdentifiers()) {
                     byQNameBuilder.put(qname, childOp);
@@ -62,12 +62,12 @@ final class ChoiceNodeContextNode extends AbstractMixinContextNode {
     }
 
     @Override
-    protected DataSchemaContextNode enterChild(final QName child, final SchemaInferenceStack stack) {
+    DataSchemaContextNode enterChild(final QName child, final SchemaInferenceStack stack) {
         return pushToStack(getChild(child), stack);
     }
 
     @Override
-    protected DataSchemaContextNode enterChild(final PathArgument child, final SchemaInferenceStack stack) {
+    DataSchemaContextNode enterChild(final PathArgument child, final SchemaInferenceStack stack) {
         return pushToStack(getChild(child), stack);
     }
 
index d09d05711aee23d6043bd4e70da8f1d2dd4e6262..fddf2cd6959099e4fc84a595b0704dc6d30c5a58 100644 (file)
@@ -14,6 +14,7 @@ import java.util.concurrent.ConcurrentMap;
 import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.jdt.annotation.Nullable;
 import org.opendaylight.yangtools.yang.common.QName;
+import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument;
 import org.opendaylight.yangtools.yang.data.util.DataSchemaContextNode;
 import org.opendaylight.yangtools.yang.model.api.DataNodeContainer;
@@ -25,39 +26,37 @@ abstract class DataContainerContextNode extends AbstractInteriorContextNode {
     private final ConcurrentMap<QName, AbstractDataSchemaContextNode> byQName = new ConcurrentHashMap<>();
     private final DataNodeContainer container;
 
-    DataContainerContextNode(final PathArgument pathArgument, final DataNodeContainer container,
+    DataContainerContextNode(final NodeIdentifier pathStep, final DataNodeContainer container,
             final DataSchemaNode schema) {
-        super(pathArgument, schema);
+        super(pathStep, schema);
         this.container = requireNonNull(container);
     }
 
     @Override
-    public AbstractDataSchemaContextNode getChild(final PathArgument child) {
-        var potential = byArg.get(child);
-        if (potential != null) {
-            return potential;
+    public final AbstractDataSchemaContextNode getChild(final PathArgument child) {
+        final var existing = byArg.get(child);
+        if (existing != null) {
+            return existing;
         }
-        potential = fromLocalSchema(child);
-        return register(potential);
+        return register(fromLocalSchema(child));
     }
 
     @Override
-    public AbstractDataSchemaContextNode getChild(final QName child) {
-        AbstractDataSchemaContextNode potential = byQName.get(child);
-        if (potential != null) {
-            return potential;
+    public final AbstractDataSchemaContextNode getChild(final QName child) {
+        var existing = byQName.get(child);
+        if (existing != null) {
+            return existing;
         }
-        potential = fromLocalSchemaAndQName(container, child);
-        return register(potential);
+        return register(fromLocalSchemaAndQName(container, child));
     }
 
     @Override
-    protected final DataSchemaContextNode enterChild(final QName child, final SchemaInferenceStack stack) {
+    final DataSchemaContextNode enterChild(final QName child, final SchemaInferenceStack stack) {
         return pushToStack(getChild(child), stack);
     }
 
     @Override
-    protected final DataSchemaContextNode enterChild(final PathArgument child, final SchemaInferenceStack stack) {
+    final DataSchemaContextNode enterChild(final PathArgument child, final SchemaInferenceStack stack) {
         return pushToStack(getChild(child), stack);
     }
 
@@ -73,7 +72,7 @@ abstract class DataContainerContextNode extends AbstractInteriorContextNode {
         return fromSchemaAndQNameChecked(container, child.getNodeType());
     }
 
-    protected AbstractDataSchemaContextNode fromLocalSchemaAndQName(final DataNodeContainer schema,
+    private static AbstractDataSchemaContextNode fromLocalSchemaAndQName(final DataNodeContainer schema,
             final QName child) {
         return fromSchemaAndQNameChecked(schema, child);
     }
@@ -81,7 +80,7 @@ abstract class DataContainerContextNode extends AbstractInteriorContextNode {
     private AbstractDataSchemaContextNode register(final AbstractDataSchemaContextNode potential) {
         if (potential != null) {
             // FIXME: use putIfAbsent() to make sure we do not perform accidental overrwrites
-            byArg.put(potential.pathArgument(), potential);
+            byArg.put(potential.getPathStep(), potential);
             for (QName qname : potential.qnameIdentifiers()) {
                 byQName.put(qname, potential);
             }
index bf87e571f5f93a41f15d828dee6becd501bf0681..297a166b48d1907bb20003f287281b327969fb6a 100644 (file)
@@ -7,21 +7,13 @@
  */
 package org.opendaylight.yangtools.yang.data.util.impl.legacy;
 
-import org.opendaylight.yangtools.yang.common.Empty;
-import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeWithValue;
 import org.opendaylight.yangtools.yang.data.util.DataSchemaContextNode.SimpleValue;
 import org.opendaylight.yangtools.yang.model.api.LeafListSchemaNode;
 import org.opendaylight.yangtools.yang.model.util.SchemaInferenceStack;
 
 public final class LeafListEntryContextNode extends AbstractLeafContextNode implements SimpleValue {
     LeafListEntryContextNode(final LeafListSchemaNode schema) {
-        // FIXME: YANGTOOLS-1413: Empty() here is NOT NICE -- it assumes the list is of such entries...
-        super(new NodeWithValue<>(schema.getQName(), Empty.value()), schema);
-    }
-
-    @Override
-    public boolean isKeyedEntry() {
-        return true;
+        super(null, schema);
     }
 
     @Override
index 2f9ded4bb9ea61d4bba226de16a3b411c9e3ebec..d4b4f5746db693d28b56255591afe00082cab9cb 100644 (file)
@@ -7,17 +7,10 @@
  */
 package org.opendaylight.yangtools.yang.data.util.impl.legacy;
 
-import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifierWithPredicates;
 import org.opendaylight.yangtools.yang.model.api.ListSchemaNode;
 
 final class MapItemContextNode extends AbstractListItemContextNode {
     MapItemContextNode(final ListSchemaNode schema) {
-        // FIXME: YANGTOOLS-1413: this is wrong: we have no predicates at all!
-        super(NodeIdentifierWithPredicates.of(schema.getQName()), schema, schema);
-    }
-
-    @Override
-    public boolean isKeyedEntry() {
-        return true;
+        super(null, schema, schema);
     }
 }
\ No newline at end of file