From: Peter Kajsa Date: Tue, 5 Jan 2016 09:14:00 +0000 (+0100) Subject: BUG-4295: instantiate MERGE operations lazily X-Git-Tag: release/beryllium~21 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=yangtools.git;a=commitdiff_plain;h=bf405586fc69c3781311cfb8ac19ba93b670ec8d BUG-4295: instantiate MERGE operations lazily This patch reworks how merges are done in a DataTreeModification by moving the logic to SchemaAwareApplyOperation, which is the final recipient of the resulting ModifiedNode. This way the code is co-located and can be specialized based on information available for that particular node, and the container merge code is cleanly separated from the leaf node code, which turns each merge into a write. When a merge occurs on a previously-written node, we graft all merged children onto the write, using recursion only when necessary. checkPresentChild method renamed. Fix of ModifiedNode and added unit test. Change-Id: I674e3d2150e796472e831abdcfa0fad582b69759 Signed-off-by: Robert Varga Signed-off-by: Filip.Gregor Signed-off-by: Peter Kajsa --- diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/AbstractNodeContainerModificationStrategy.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/AbstractNodeContainerModificationStrategy.java index b7f2f6003c..59ed52d04a 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/AbstractNodeContainerModificationStrategy.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/AbstractNodeContainerModificationStrategy.java @@ -11,8 +11,10 @@ import static com.google.common.base.Preconditions.checkArgument; import com.google.common.base.Optional; import com.google.common.base.Preconditions; +import com.google.common.base.Verify; import java.util.Collection; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; +import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNodeContainer; import org.opendaylight.yangtools.yang.data.api.schema.tree.ConflictingModificationAppliedException; @@ -58,6 +60,21 @@ abstract class AbstractNodeContainerModificationStrategy extends SchemaAwareAppl } } + protected void recursivelyVerifyStructure(NormalizedNode value) { + final NormalizedNodeContainer container = (NormalizedNodeContainer) value; + for (final Object child : container.getValue()) { + checkArgument(child instanceof NormalizedNode); + final NormalizedNode castedChild = (NormalizedNode) child; + final Optional childOp = getChild(castedChild.getIdentifier()); + if (childOp.isPresent()) { + childOp.get().recursivelyVerifyStructure(castedChild); + } else { + throw new SchemaValidationFailedException( + String.format("Child %s is not valid child according to schema.", castedChild.getIdentifier())); + } + } + } + @Override protected TreeNode applyWrite(final ModifiedNode modification, final Optional currentMeta, final Version version) { @@ -131,12 +148,100 @@ abstract class AbstractNodeContainerModificationStrategy extends SchemaAwareAppl } @Override - protected TreeNode applyMerge(final ModifiedNode modification, final TreeNode currentMeta, - final Version version) { - // For Node Containers - merge is same as subtree change - we only replace children. + protected TreeNode applyMerge(final ModifiedNode modification, final TreeNode currentMeta, final Version version) { + /* + * The node which we are merging exists. We now need to expand any child operations implied by the value. Once + * we do that, ModifiedNode children will look like this node were a TOUCH and we will let applyTouch() do the + * heavy lifting of applying the children recursively (either through here or through applyWrite(). + */ + final NormalizedNode value = modification.getWrittenValue(); + + Verify.verify(value instanceof NormalizedNodeContainer, "Attempted to merge non-container %s", value); + @SuppressWarnings({"unchecked", "rawtypes"}) + final Collection> children = ((NormalizedNodeContainer) value).getValue(); + for (NormalizedNode c : children) { + final PathArgument id = c.getIdentifier(); + modification.modifyChild(id, resolveChildOperation(id).getChildPolicy(), version); + } return applyTouch(modification, currentMeta, version); } + private void mergeChildrenIntoModification(final ModifiedNode modification, + final Collection> children, final Version version) { + for (NormalizedNode c : children) { + final ModificationApplyOperation childOp = resolveChildOperation(c.getIdentifier()); + final ModifiedNode childNode = modification.modifyChild(c.getIdentifier(), childOp.getChildPolicy(), version); + childOp.mergeIntoModifiedNode(childNode, c, version); + } + } + + @Override + final void mergeIntoModifiedNode(final ModifiedNode modification, final NormalizedNode value, + final Version version) { + @SuppressWarnings({ "unchecked", "rawtypes" }) + final Collection> children = ((NormalizedNodeContainer)value).getValue(); + + switch (modification.getOperation()) { + case NONE: + // Fresh node, just record a MERGE with a value + recursivelyVerifyStructure(value); + modification.updateValue(LogicalOperation.MERGE, value); + return; + case TOUCH: + + mergeChildrenIntoModification(modification, children, version); + // We record empty merge value, since real children merges + // are already expanded. This is needed to satisfy non-null for merge + // original merge value can not be used since it mean different + // order of operation - parent changes are always resolved before + // children ones, and having node in TOUCH means children was modified + // before. + modification.updateValue(LogicalOperation.MERGE, createEmptyValue(value)); + return; + case MERGE: + // Merging into an existing node. Merge data children modifications (maybe recursively) and mark as MERGE, + // invalidating cached snapshot + mergeChildrenIntoModification(modification, children, version); + modification.updateOperationType(LogicalOperation.MERGE); + return; + case DELETE: + // Delete performs a data dependency check on existence of the node. Performing a merge on DELETE means we + // are really performing a write. One thing that ruins that are any child modifications. If there are any, + // we will perform a read() to get the current state of affairs, turn this into into a WRITE and then + // append any child entries. + if (!modification.getChildren().isEmpty()) { + // Version does not matter here as we'll throw it out + final Optional current = apply(modification, modification.getOriginal(), Version.initial()); + if (current.isPresent()) { + modification.updateValue(LogicalOperation.WRITE, current.get().getData()); + mergeChildrenIntoModification(modification, children, version); + return; + } + } + + modification.updateValue(LogicalOperation.WRITE, value); + return; + case WRITE: + // We are augmenting a previous write. We'll just walk value's children, get the corresponding ModifiedNode + // and run recursively on it + mergeChildrenIntoModification(modification, children, version); + modification.updateOperationType(LogicalOperation.WRITE); + return; + } + + throw new IllegalArgumentException("Unsupported operation " + modification.getOperation()); + } + + @SuppressWarnings({"rawtypes", "unchecked"}) + private NormalizedNode createEmptyValue(NormalizedNode value, + Collection> children) { + NormalizedNodeContainerBuilder builder = createBuilder(value); + for (NormalizedNode child : children) { + builder.removeChild(child.getIdentifier()); + } + return builder.build(); + } + @Override protected TreeNode applyTouch(final ModifiedNode modification, final TreeNode currentMeta, final Version version) { /* @@ -220,4 +325,6 @@ abstract class AbstractNodeContainerModificationStrategy extends SchemaAwareAppl @SuppressWarnings("rawtypes") protected abstract NormalizedNodeContainerBuilder createBuilder(NormalizedNode original); + + protected abstract NormalizedNode createEmptyValue(NormalizedNode original); } diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/AbstractValueNodeModificationStrategy.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/AbstractValueNodeModificationStrategy.java index bd23a4e46c..e4b80b2fb5 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/AbstractValueNodeModificationStrategy.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/AbstractValueNodeModificationStrategy.java @@ -72,4 +72,25 @@ abstract class AbstractValueNodeModificationStrategy e final Optional current) throws IncorrectDataStructureException { throw new IncorrectDataStructureException(path, "Subtree modification is not allowed."); } -} \ No newline at end of file + + @Override + void mergeIntoModifiedNode(final ModifiedNode node, final NormalizedNode value, final Version version) { + + switch (node.getOperation()) { + // Delete performs a data dependency check on existence of the node. Performing a merge + // on DELETE means we + // are really performing a write. + case DELETE: + case WRITE: + node.write(value); + break; + default: + node.updateValue(LogicalOperation.MERGE, value); + } + } + + @Override + void recursivelyVerifyStructure(NormalizedNode value) { + verifyStructure(value, false); + } +} diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/AlwaysFailOperation.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/AlwaysFailOperation.java index b91d28f9b7..cd2455695a 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/AlwaysFailOperation.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/AlwaysFailOperation.java @@ -34,7 +34,8 @@ final class AlwaysFailOperation extends ModificationApplyOperation { } @Override - void checkApplicable(final YangInstanceIdentifier path,final NodeModification modification, final Optional storeMetadata) { + void checkApplicable(final YangInstanceIdentifier path,final NodeModification modification, + final Optional storeMetadata) { throw new IllegalStateException("Schema Context is not available."); } @@ -52,4 +53,14 @@ final class AlwaysFailOperation extends ModificationApplyOperation { ChildTrackingPolicy getChildPolicy() { throw new IllegalStateException("Schema Context is not available."); } + + @Override + void mergeIntoModifiedNode(final ModifiedNode node, final NormalizedNode value, final Version version) { + throw new IllegalStateException("Schema Context is not available."); + } + + @Override + void recursivelyVerifyStructure(NormalizedNode value) { + throw new IllegalStateException("Schema Context is not available."); + } } \ No newline at end of file diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/AugmentationModificationStrategy.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/AugmentationModificationStrategy.java index 83d460c09b..91fd2e557c 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/AugmentationModificationStrategy.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/AugmentationModificationStrategy.java @@ -33,6 +33,13 @@ final class AugmentationModificationStrategy extends AbstractDataNodeContainerMo return ImmutableAugmentationNodeBuilder.create((AugmentationNode) original); } + @Override + protected NormalizedNode createEmptyValue(NormalizedNode original) { + checkArgument(original instanceof AugmentationNode); + return ImmutableAugmentationNodeBuilder.create() + .withNodeIdentifier(((AugmentationNode) original).getIdentifier()).build(); + } + private static AugmentationSchema createAugmentProxy(final AugmentationSchema schema, final DataNodeContainer resolved) { final Set realChildSchemas = new HashSet<>(); for(final DataSchemaNode augChild : schema.getChildNodes()) { diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ChoiceModificationStrategy.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ChoiceModificationStrategy.java index 38edd7bb2f..62c6ebcad9 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ChoiceModificationStrategy.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ChoiceModificationStrategy.java @@ -135,4 +135,11 @@ final class ChoiceModificationStrategy extends AbstractNodeContainerModification enforceCases(ret); return ret; } + + @Override + protected NormalizedNode createEmptyValue(NormalizedNode original) { + checkArgument(original instanceof ChoiceNode); + return ImmutableChoiceNodeBuilder.create().withNodeIdentifier(((ChoiceNode) original).getIdentifier()).build(); + } } + diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ContainerModificationStrategy.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ContainerModificationStrategy.java index 58671b8c3e..8e28fdd868 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ContainerModificationStrategy.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ContainerModificationStrategy.java @@ -8,6 +8,8 @@ package org.opendaylight.yangtools.yang.data.impl.schema.tree; import static com.google.common.base.Preconditions.checkArgument; + +import com.google.common.base.Preconditions; import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; import org.opendaylight.yangtools.yang.data.api.schema.tree.TreeType; @@ -30,4 +32,11 @@ class ContainerModificationStrategy extends AbstractDataNodeContainerModificatio checkArgument(original instanceof ContainerNode); return ImmutableContainerNodeBuilder.create((ContainerNode) original); } + + @Override + protected NormalizedNode createEmptyValue(NormalizedNode original) { + Preconditions.checkArgument(original instanceof ContainerNode); + return ImmutableContainerNodeBuilder.create().withNodeIdentifier(((ContainerNode) original).getIdentifier()) + .build(); + } } diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ListEntryModificationStrategy.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ListEntryModificationStrategy.java index 30b17b1c0e..f2ba17be37 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ListEntryModificationStrategy.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ListEntryModificationStrategy.java @@ -54,4 +54,11 @@ final class ListEntryModificationStrategy extends AbstractDataNodeContainerModif checkArgument(original instanceof MapEntryNode); return ImmutableMapEntryNodeBuilder.create((MapEntryNode) original); } + + @Override + protected NormalizedNode createEmptyValue(NormalizedNode original) { + checkArgument(original instanceof MapEntryNode); + return ImmutableMapEntryNodeBuilder.create().withNodeIdentifier(((MapEntryNode) original).getIdentifier()) + .build(); + } } \ No newline at end of file diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/MinMaxElementsValidation.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/MinMaxElementsValidation.java index 932f20a810..ede5bcc067 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/MinMaxElementsValidation.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/MinMaxElementsValidation.java @@ -114,17 +114,17 @@ final class MinMaxElementsValidation extends SchemaAwareApplyOperation { for (final ModifiedNode modChild : modification.getChildren()) { switch (modChild.getOperation()) { case WRITE: - if (!modChild.getOriginal().isPresent()) { + if (!checkOriginalPresent(modChild)) { result++; } break; case MERGE: - if (!current.isPresent()) { + if (!checkOriginalPresent(modChild)) { result++; } break; case DELETE: - if (modChild.getOriginal().isPresent()) { + if (checkOriginalPresent(modChild)) { result--; } break; @@ -139,6 +139,10 @@ final class MinMaxElementsValidation extends SchemaAwareApplyOperation { return result; } + private static boolean checkOriginalPresent(ModifiedNode child) { + return child.getOriginal().isPresent(); + } + @Override protected void checkTouchApplicable(final YangInstanceIdentifier path, final NodeModification modification, final Optional current) throws DataValidationFailedException { @@ -191,4 +195,14 @@ final class MinMaxElementsValidation extends SchemaAwareApplyOperation { protected ChildTrackingPolicy getChildPolicy() { return delegate.getChildPolicy(); } + + @Override + void mergeIntoModifiedNode(final ModifiedNode node, final NormalizedNode value, final Version version) { + delegate.mergeIntoModifiedNode(node, value, version); + } + + @Override + void recursivelyVerifyStructure(NormalizedNode value) { + delegate.recursivelyVerifyStructure(value); + } } diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ModificationApplyOperation.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ModificationApplyOperation.java index cdeaa7c01a..933bf9c570 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ModificationApplyOperation.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ModificationApplyOperation.java @@ -88,6 +88,17 @@ abstract class ModificationApplyOperation implements StoreTreeNode value, Version version); + /** * Returns a suboperation for specified tree node * @@ -96,4 +107,6 @@ abstract class ModificationApplyOperation implements StoreTreeNode getChild(PathArgument child); + + abstract void recursivelyVerifyStructure(NormalizedNode value); } diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ModifiedNode.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ModifiedNode.java index 23674daf32..cae3e94d1c 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ModifiedNode.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/ModifiedNode.java @@ -16,7 +16,7 @@ import javax.annotation.Nonnull; import javax.annotation.concurrent.NotThreadSafe; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; -import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNodes; +import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNodeContainer; import org.opendaylight.yangtools.yang.data.api.schema.tree.ModificationType; import org.opendaylight.yangtools.yang.data.api.schema.tree.StoreTreeNode; import org.opendaylight.yangtools.yang.data.api.schema.tree.spi.TreeNode; @@ -26,11 +26,14 @@ import org.opendaylight.yangtools.yang.data.api.schema.tree.spi.Version; /** * Node Modification Node and Tree * - * Tree which structurally resembles data tree and captures client modifications - * to the data store tree. + * Tree which structurally resembles data tree and captures client modifications to the data store tree. This tree is + * lazily created and populated via {@link #modifyChild(PathArgument)} and {@link TreeNode} which represents original + * state as tracked by {@link #getOriginal()}. * - * This tree is lazily created and populated via {@link #modifyChild(PathArgument)} - * and {@link TreeNode} which represents original state as tracked by {@link #getOriginal()}. + * The contract is that the state information exposed here preserves the temporal ordering of whatever modifications + * were executed. A child's effects pertain to data node as modified by its ancestors. This means that in order to + * reconstruct the effective data node presentation, it is sufficient to perform a depth-first pre-order traversal of + * the tree. */ @NotThreadSafe final class ModifiedNode extends NodeModification implements StoreTreeNode { @@ -69,28 +72,31 @@ final class ModifiedNode extends NodeModification implements StoreTreeNode getWrittenValue() { - return value; - } - @Override public PathArgument getIdentifier() { return identifier; } + @Override + LogicalOperation getOperation() { + return operation; + } + @Override Optional getOriginal() { return original; } - @Override - LogicalOperation getOperation() { - return operation; + /** + * Return the value which was written to this node. The returned object is only valid for + * {@link LogicalOperation#MERGE} and {@link LogicalOperation#WRITE}. + * operations. It should only be consulted when this modification is going to end up being + * {@link ModificationType#WRITE}. + * + * @return Currently-written value + */ + NormalizedNode getWrittenValue() { + return value; } /** @@ -137,15 +143,8 @@ final class ModifiedNode extends NodeModification implements StoreTreeNode> childData = ((NormalizedNodeContainer)value).getChild(child); + if (childData.isPresent()) { + newlyCreated.updateValue(LogicalOperation.MERGE, childData.get()); + } + } + children.put(child, newlyCreated); return newlyCreated; } @@ -238,39 +249,10 @@ final class ModifiedNode extends NodeModification implements StoreTreeNode value) { - pushWrite(value); + updateValue(LogicalOperation.WRITE, value); children.clear(); } - // Promote the node to write, but do not lose children - void pushWrite(final NormalizedNode value) { - clearSnapshot(); - updateOperationType(LogicalOperation.WRITE); - this.value = value; - } - - void merge(final NormalizedNode value) { - clearSnapshot(); - updateOperationType(LogicalOperation.MERGE); - - /* - * Blind overwrite of any previous data is okay, no matter whether the node - * is simple or complex type. - * - * If this is a simple or complex type with unkeyed children, this merge will - * be turned into a write operation, overwriting whatever was there before. - * - * If this is a container with keyed children, there are two possibilities: - * - if it existed before, this value will never be consulted and the children - * will get explicitly merged onto the original data. - * - if it did not exist before, this value will be used as a seed write and - * children will be merged into it. - * In either case we rely on OperationWithModification to manipulate the children - * before calling this method, so unlike a write we do not want to clear them. - */ - this.value = value; - } - /** * Seal the modification node and prune any children which has not been modified. * @@ -314,7 +296,7 @@ final class ModifiedNode extends NodeModification implements StoreTreeNode value) { + this.value = Preconditions.checkNotNull(value); + updateOperationType(type); + } + /** * Return the physical modification done to data. May return null if the * operation has not been applied to the underlying tree. This is different @@ -345,30 +338,6 @@ final class ModifiedNode extends NodeModification implements StoreTreeNode value) { - /* - * We are instantiating an "equivalent" of this node. Currently the only callsite does not care - * about the actual iteration order, so we do not have to specify the same tracking policy as - * we were instantiated with. Since this is the only time we need to know that policy (it affects - * only things in constructor), we do not want to retain it (saves some memory on per-instance - * basis). - * - * We could reconstruct it using two instanceof checks (to undo what the constructor has done), - * which would give perfect results. The memory saving would be at most 32 bytes of a short-lived - * object, so let's not bother with that. - */ - final ModifiedNode ret = new ModifiedNode(getIdentifier(), Optional.absent(), ChildTrackingPolicy.UNORDERED); - ret.write(value); - return ret; - } - public static ModifiedNode createUnmodified(final TreeNode metadataTree, final ChildTrackingPolicy childPolicy) { return new ModifiedNode(metadataTree.getIdentifier(), Optional.of(metadataTree), childPolicy); } diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/OperationWithModification.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/OperationWithModification.java index fbfb499c6d..d22854dc88 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/OperationWithModification.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/OperationWithModification.java @@ -12,7 +12,6 @@ import com.google.common.base.Optional; import com.google.common.base.Preconditions; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; -import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNodeContainer; import org.opendaylight.yangtools.yang.data.api.schema.tree.spi.TreeNode; import org.opendaylight.yangtools.yang.data.api.schema.tree.spi.Version; @@ -40,48 +39,6 @@ final class OperationWithModification { applyOperation.verifyStructure(value, false); } - private void recursiveMerge(final NormalizedNode data, final Version version) { - if (data instanceof NormalizedNodeContainer) { - @SuppressWarnings({ "rawtypes", "unchecked" }) - final NormalizedNodeContainer> dataContainer = - (NormalizedNodeContainer) data; - - /* - * If there was write before on this node and it is of NormalizedNodeContainer type merge would overwrite - * our changes. So we create write modifications from data children to retain children created by previous - * write operation. These writes will then be pushed down in the tree while there are merge modifications - * on these children - */ - if (modification.getOperation() == LogicalOperation.WRITE) { - @SuppressWarnings({ "rawtypes", "unchecked" }) - final NormalizedNodeContainer> oldDataContainer = - (NormalizedNodeContainer) modification.getWrittenValue(); - for (final NormalizedNode c : oldDataContainer.getValue()) { - final PathArgument childId = c.getIdentifier(); - - // Acquire the child operation type if available, fall back to NONE - final Optional maybeChild = modification.getChild(childId); - if (maybeChild.isPresent()) { - final ModifiedNode child = maybeChild.get(); - final LogicalOperation op = child.getOperation(); - if (op == LogicalOperation.TOUCH || op == LogicalOperation.NONE) { - child.pushWrite(c); - } - } else { - // Not present, issue a write - forChild(childId, version).write(c); - } - } - } - for (final NormalizedNode child : dataContainer.getValue()) { - final PathArgument childId = child.getIdentifier(); - forChild(childId, version).recursiveMerge(child, version); - } - } - - modification.merge(data); - } - void merge(final NormalizedNode data, final Version version) { /* * A merge operation will end up overwriting parts of the tree, retaining others. We want to @@ -89,11 +46,9 @@ final class OperationWithModification { * written. In order to do that, we first pretend the data was written, run verification and * then perform the merge -- with the explicit assumption that adding the newly-validated * data with the previously-validated data will not result in invalid data. - * - * FIXME: Should be this moved to recursive merge and run for each node? */ - applyOperation.verifyStructure(data, false); - recursiveMerge(data, version); + applyOperation.verifyStructure(data, true); + applyOperation.mergeIntoModifiedNode(modification, data, version); } void delete() { @@ -150,15 +105,4 @@ final class OperationWithModification { final ModifiedNode modification) { return new OperationWithModification(operation, modification); } - - private OperationWithModification forChild(final PathArgument childId, final Version version) { - final Optional maybeChildOp = applyOperation.getChild(childId); - Preconditions.checkArgument(maybeChildOp.isPresent(), - "Attempted to apply operation to non-existent child %s", childId); - - final ModificationApplyOperation childOp = maybeChildOp.get(); - final ModifiedNode childMod = modification.modifyChild(childId, childOp.getChildPolicy(), version); - - return from(childOp, childMod); - } } diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/OrderedLeafSetModificationStrategy.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/OrderedLeafSetModificationStrategy.java index 2e57dbbfc5..488cc2281f 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/OrderedLeafSetModificationStrategy.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/OrderedLeafSetModificationStrategy.java @@ -40,6 +40,13 @@ final class OrderedLeafSetModificationStrategy extends AbstractNodeContainerModi return ImmutableOrderedLeafSetNodeBuilder.create((OrderedLeafSetNode) original); } + @Override + protected NormalizedNode createEmptyValue(NormalizedNode original) { + checkArgument(original instanceof OrderedLeafSetNode); + return ImmutableOrderedLeafSetNodeBuilder.create() + .withNodeIdentifier(((OrderedLeafSetNode) original).getIdentifier()).build(); + } + @Override public Optional getChild(final YangInstanceIdentifier.PathArgument identifier) { if (identifier instanceof YangInstanceIdentifier.NodeWithValue) { diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/OrderedMapModificationStrategy.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/OrderedMapModificationStrategy.java index 686258a790..c69a170f24 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/OrderedMapModificationStrategy.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/OrderedMapModificationStrategy.java @@ -39,6 +39,13 @@ final class OrderedMapModificationStrategy extends AbstractNodeContainerModifica return ImmutableOrderedMapNodeBuilder.create((OrderedMapNode) original); } + @Override + protected NormalizedNode createEmptyValue(NormalizedNode original) { + checkArgument(original instanceof OrderedMapNode); + return ImmutableOrderedMapNodeBuilder.create().withNodeIdentifier(((OrderedMapNode) original).getIdentifier()) + .build(); + } + @Override public Optional getChild(final YangInstanceIdentifier.PathArgument identifier) { if (identifier instanceof YangInstanceIdentifier.NodeIdentifierWithPredicates) { diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/RootModificationApplyOperation.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/RootModificationApplyOperation.java index 693ace4df2..6fc704c053 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/RootModificationApplyOperation.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/RootModificationApplyOperation.java @@ -99,11 +99,21 @@ abstract class RootModificationApplyOperation extends ModificationApplyOperation getDelegate().verifyStructure(modification, verifyChildren); } + @Override + void recursivelyVerifyStructure(NormalizedNode value) { + getDelegate().recursivelyVerifyStructure(value); + } + @Override final ChildTrackingPolicy getChildPolicy() { return getDelegate().getChildPolicy(); } + @Override + final void mergeIntoModifiedNode(final ModifiedNode node, final NormalizedNode value, final Version version) { + getDelegate().mergeIntoModifiedNode(node, value, version); + } + /** * Return the underlying delegate. * diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/SchemaAwareApplyOperation.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/SchemaAwareApplyOperation.java index f07844c435..a511ddf064 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/SchemaAwareApplyOperation.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/SchemaAwareApplyOperation.java @@ -16,7 +16,6 @@ import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.Augmentat import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument; import org.opendaylight.yangtools.yang.data.api.schema.tree.ConflictingModificationAppliedException; import org.opendaylight.yangtools.yang.data.api.schema.tree.DataValidationFailedException; -import org.opendaylight.yangtools.yang.data.api.schema.tree.IncorrectDataStructureException; import org.opendaylight.yangtools.yang.data.api.schema.tree.ModificationType; import org.opendaylight.yangtools.yang.data.api.schema.tree.TreeType; import org.opendaylight.yangtools.yang.data.api.schema.tree.spi.TreeNode; @@ -169,8 +168,8 @@ abstract class SchemaAwareApplyOperation extends ModificationApplyOperation { checkNotConflicting(path, original.get(), current.get()); } else if(original.isPresent()) { throw new ConflictingModificationAppliedException(path,"Node was deleted by other transaction."); - } else if(current.isPresent()) { - throw new ConflictingModificationAppliedException(path,"Node was created by other transaction."); + } else if (current.isPresent()) { + throw new ConflictingModificationAppliedException(path, "Node was created by other transaction."); } } @@ -203,7 +202,7 @@ abstract class SchemaAwareApplyOperation extends ModificationApplyOperation { // This is a slight optimization: a merge on a non-existing node equals to a write if (currentMeta.isPresent()) { - result = applyMerge(modification,currentMeta.get(), version); + result = applyMerge(modification, currentMeta.get(), version); } else { modification.resolveModificationType(ModificationType.WRITE); result = applyWrite(modification, currentMeta, version); diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/StructuralContainerModificationStrategy.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/StructuralContainerModificationStrategy.java index f2eefd293c..be35b5aa67 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/StructuralContainerModificationStrategy.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/StructuralContainerModificationStrategy.java @@ -103,11 +103,21 @@ final class StructuralContainerModificationStrategy extends ModificationApplyOpe delegate.verifyStructure(modification, verifyChildren); } + @Override + void recursivelyVerifyStructure(NormalizedNode value) { + delegate.recursivelyVerifyStructure(value); + } + @Override ChildTrackingPolicy getChildPolicy() { return delegate.getChildPolicy(); } + @Override + void mergeIntoModifiedNode(ModifiedNode modification, NormalizedNode value, Version version) { + delegate.mergeIntoModifiedNode(modification, value, version); + } + @Override public Optional getChild(final PathArgument child) { return delegate.getChild(child); diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/UnkeyedListItemModificationStrategy.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/UnkeyedListItemModificationStrategy.java index 6439ed67cb..1e27090058 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/UnkeyedListItemModificationStrategy.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/UnkeyedListItemModificationStrategy.java @@ -27,4 +27,11 @@ final class UnkeyedListItemModificationStrategy extends AbstractDataNodeContaine checkArgument(original instanceof UnkeyedListEntryNode); return ImmutableUnkeyedListEntryNodeBuilder.create((UnkeyedListEntryNode) original); } + + @Override + protected NormalizedNode createEmptyValue(NormalizedNode original) { + checkArgument(original instanceof UnkeyedListEntryNode); + return ImmutableUnkeyedListEntryNodeBuilder.create() + .withNodeIdentifier(((UnkeyedListEntryNode) original).getIdentifier()).build(); + } } \ No newline at end of file diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/UnkeyedListModificationStrategy.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/UnkeyedListModificationStrategy.java index 32d73c9b61..dacba5aefe 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/UnkeyedListModificationStrategy.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/UnkeyedListModificationStrategy.java @@ -14,7 +14,6 @@ import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgum import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; import org.opendaylight.yangtools.yang.data.api.schema.UnkeyedListEntryNode; import org.opendaylight.yangtools.yang.data.api.schema.tree.IncorrectDataStructureException; -import org.opendaylight.yangtools.yang.data.api.schema.tree.ModificationType; import org.opendaylight.yangtools.yang.data.api.schema.tree.TreeType; import org.opendaylight.yangtools.yang.data.api.schema.tree.spi.MutableTreeNode; import org.opendaylight.yangtools.yang.data.api.schema.tree.spi.TreeNode; @@ -38,10 +37,8 @@ final class UnkeyedListModificationStrategy extends SchemaAwareApplyOperation { } @Override - protected TreeNode applyMerge(final ModifiedNode modification, final TreeNode currentMeta, - final Version version) { - modification.resolveModificationType(ModificationType.WRITE); - return applyWrite(modification, Optional.of(currentMeta), version); + protected TreeNode applyMerge(final ModifiedNode modification, final TreeNode currentMeta, final Version version) { + throw new IllegalStateException("Invalid merge into unkeyed list"); } @Override @@ -127,9 +124,20 @@ final class UnkeyedListModificationStrategy extends SchemaAwareApplyOperation { } + @Override + void recursivelyVerifyStructure(NormalizedNode value) { + // NOOP + } + @Override protected void checkTouchApplicable(final YangInstanceIdentifier path, final NodeModification modification, final Optional current) throws IncorrectDataStructureException { throw new IncorrectDataStructureException(path, "Subtree modification is not allowed."); } + + @Override + void mergeIntoModifiedNode(final ModifiedNode node, final NormalizedNode value, final Version version) { + // Unkeyed lists are always replaced + node.write(value); + } } diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/UnorderedLeafSetModificationStrategy.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/UnorderedLeafSetModificationStrategy.java index 8e001d0c7c..e52325ea79 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/UnorderedLeafSetModificationStrategy.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/UnorderedLeafSetModificationStrategy.java @@ -34,6 +34,13 @@ final class UnorderedLeafSetModificationStrategy extends AbstractNodeContainerMo return ImmutableLeafSetNodeBuilder.create((LeafSetNode) original); } + @Override + protected NormalizedNode createEmptyValue(NormalizedNode original) { + checkArgument(original instanceof LeafSetNode); + return ImmutableLeafSetNodeBuilder.create().withNodeIdentifier(((LeafSetNode) original).getIdentifier()) + .build(); + } + @Override public Optional getChild(final YangInstanceIdentifier.PathArgument identifier) { if (identifier instanceof YangInstanceIdentifier.NodeWithValue) { diff --git a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/UnorderedMapModificationStrategy.java b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/UnorderedMapModificationStrategy.java index a264e06f63..601939bc97 100644 --- a/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/UnorderedMapModificationStrategy.java +++ b/yang/yang-data-impl/src/main/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/UnorderedMapModificationStrategy.java @@ -34,6 +34,12 @@ final class UnorderedMapModificationStrategy extends AbstractNodeContainerModifi return ImmutableMapNodeBuilder.create((MapNode) original); } + @Override + protected NormalizedNode createEmptyValue(final NormalizedNode original) { + checkArgument(original instanceof MapNode); + return ImmutableMapNodeBuilder.create().withNodeIdentifier(((MapNode) original).getIdentifier()).build(); + } + @Override public Optional getChild(final YangInstanceIdentifier.PathArgument identifier) { if (identifier instanceof YangInstanceIdentifier.NodeIdentifierWithPredicates) { diff --git a/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/Bug4295Test.java b/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/Bug4295Test.java new file mode 100644 index 0000000000..9ac489c01b --- /dev/null +++ b/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/Bug4295Test.java @@ -0,0 +1,192 @@ +/* + * Copyright (c) 2015 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.data.impl.schema.tree; + +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableMap.Builder; +import java.io.File; +import java.net.URI; +import org.junit.Before; +import org.junit.Test; +import org.opendaylight.yangtools.yang.common.QName; +import org.opendaylight.yangtools.yang.common.QNameModule; +import org.opendaylight.yangtools.yang.common.SimpleDateFormatUtil; +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.NodeIdentifierWithPredicates; +import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode; +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.tree.DataTreeModification; +import org.opendaylight.yangtools.yang.data.api.schema.tree.DataValidationFailedException; +import org.opendaylight.yangtools.yang.data.api.schema.tree.TipProducingDataTree; +import org.opendaylight.yangtools.yang.data.api.schema.tree.TreeType; +import org.opendaylight.yangtools.yang.data.impl.RetestUtils; +import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes; +import org.opendaylight.yangtools.yang.data.impl.schema.builder.api.CollectionNodeBuilder; +import org.opendaylight.yangtools.yang.data.impl.schema.builder.api.DataContainerNodeAttrBuilder; +import org.opendaylight.yangtools.yang.data.impl.schema.builder.impl.ImmutableContainerNodeBuilder; +import org.opendaylight.yangtools.yang.model.api.SchemaContext; + +public class Bug4295Test { + + private TipProducingDataTree inMemoryDataTree; + private SchemaContext context; + private QName root; + private QName subRoot; + private QName outerList; + private QName innerList; + private QName oId; + private QName iId; + private QName oLeaf; + private QName iLeaf; + private QNameModule foo; + + @Before + public void init() throws Exception { + final File resourceFile = new File(Bug4295Test.class.getResource("/bug-4295/foo.yang") + .toURI()); + context = RetestUtils.parseYangSources(resourceFile); + foo = QNameModule.create(new URI("foo"), SimpleDateFormatUtil.getRevisionFormat().parse("1970-01-01")); + root = QName.create(foo, "root"); + subRoot = QName.create(foo, "sub-root"); + outerList = QName.create(foo, "outer-list"); + innerList = QName.create(foo, "inner-list"); + oId = QName.create(foo, "o-id"); + iId = QName.create(foo, "i-id"); + oLeaf = QName.create(foo, "o"); + iLeaf = QName.create(foo, "i"); + inMemoryDataTree = InMemoryDataTreeFactory.getInstance().create(TreeType.OPERATIONAL); + inMemoryDataTree.setSchemaContext(context); + } + + @Test + public void test() throws DataValidationFailedException { + firstModification(); + secondModification(1); + secondModification(2); + secondModification(3); + } + + + private void firstModification() throws DataValidationFailedException { + /* MERGE */ + MapNode outerListNode = ImmutableNodes.mapNodeBuilder().withNodeIdentifier(NodeIdentifier.create(outerList)) + .withChild(createOuterListEntry("1", "o-1")) + .withChild(createOuterListEntry("2", "o-2")) + .withChild(createOuterListEntry("3", "o-3")) + .build(); + ContainerNode rootContainerNode = createRootContainerBuilder() + .withChild( + createSubRootContainerBuilder() + .withChild(outerListNode) + .build()) + .build(); + YangInstanceIdentifier path = YangInstanceIdentifier.of(root); + DataTreeModification modification = inMemoryDataTree.takeSnapshot().newModification(); + modification.merge(path, rootContainerNode); + + /* WRITE INNER LIST WITH ENTRIES*/ + MapNode innerListNode = createInnerListBuilder() + .withChild(createInnerListEntry("a", "i-a")) + .withChild(createInnerListEntry("b", "i-b")) + .build(); + path = YangInstanceIdentifier.of(root).node(subRoot).node(outerList).node(createOuterListEntryPath("2")).node(innerList); + modification.write(path, innerListNode); + + /* COMMIT */ + modification.ready(); + inMemoryDataTree.validate(modification); + inMemoryDataTree.commit(inMemoryDataTree.prepare(modification)); + } + + private void secondModification(int testScenarioNumber) throws DataValidationFailedException { + /* MERGE */ + MapNode outerListNode = ImmutableNodes.mapNodeBuilder().withNodeIdentifier(NodeIdentifier.create(outerList)) + .withChild(createOuterListEntry("3", "o-3")) + .withChild(createOuterListEntry("4", "o-4")) + .withChild(createOuterListEntry("5", "o-5")) + .build(); + + ContainerNode rootContainerNode = createRootContainerBuilder() + .withChild( + createSubRootContainerBuilder() + .withChild(outerListNode) + .build()) + .build(); + + YangInstanceIdentifier path = YangInstanceIdentifier.of(root); + DataTreeModification modification = inMemoryDataTree.takeSnapshot().newModification(); + modification.merge(path, rootContainerNode); + + if (testScenarioNumber == 1) { + /* WRITE EMPTY INNER LIST */ + writeEmptyInnerList(modification, "2"); + } else if (testScenarioNumber == 2) { + /* WRITE INNER LIST ENTRY */ + MapEntryNode innerListEntryA = createInnerListEntry("a", "i-a-2"); + path = YangInstanceIdentifier.of(root).node(subRoot).node(outerList).node(createOuterListEntryPath("2")) + .node(innerList).node(createInnerListEntryPath("a")); + modification.write(path, innerListEntryA); + } else if (testScenarioNumber == 3) { + /* WRITE INNER LIST WITH ENTRIES */ + MapNode innerListNode = createInnerListBuilder().withChild(createInnerListEntry("a", "i-a-3")) + .withChild(createInnerListEntry("c", "i-c")).build(); + path = YangInstanceIdentifier.of(root).node(subRoot).node(outerList).node(createOuterListEntryPath("2")) + .node(innerList); + modification.write(path, innerListNode); + } + + /* COMMIT */ + modification.ready(); + inMemoryDataTree.validate(modification); + inMemoryDataTree.commit(inMemoryDataTree.prepare(modification)); + } + + private void writeEmptyInnerList(DataTreeModification modification, String outerListEntryKey) { + YangInstanceIdentifier path = YangInstanceIdentifier.of(root).node(subRoot).node(outerList) + .node(createOuterListEntryPath(outerListEntryKey)).node(innerList); + modification.write(path, createInnerListBuilder().build()); + } + + private DataContainerNodeAttrBuilder createRootContainerBuilder() { + return ImmutableContainerNodeBuilder.create().withNodeIdentifier( + new YangInstanceIdentifier.NodeIdentifier(root)); + } + + private DataContainerNodeAttrBuilder createSubRootContainerBuilder() { + return ImmutableContainerNodeBuilder.create().withNodeIdentifier( + new YangInstanceIdentifier.NodeIdentifier(subRoot)); + } + + private CollectionNodeBuilder createInnerListBuilder() { + return ImmutableNodes.mapNodeBuilder().withNodeIdentifier(NodeIdentifier.create(innerList)); + } + + private NodeIdentifierWithPredicates createInnerListEntryPath(String keyValue) { + Builder builder = ImmutableMap.builder(); + ImmutableMap keys = builder.put(iId, keyValue).build(); + return new YangInstanceIdentifier.NodeIdentifierWithPredicates(innerList, keys); + } + + private NodeIdentifierWithPredicates createOuterListEntryPath(String keyValue) { + Builder builder = ImmutableMap.builder(); + ImmutableMap keys = builder.put(oId, keyValue).build(); + return new YangInstanceIdentifier.NodeIdentifierWithPredicates(outerList, keys); + } + + private MapEntryNode createOuterListEntry(String keyValue, String leafValue) { + return ImmutableNodes.mapEntryBuilder(outerList, oId, keyValue) + .withChild(ImmutableNodes.leafNode(oLeaf, leafValue)).build(); + } + + private MapEntryNode createInnerListEntry(String keyValue, String leafValue) { + return ImmutableNodes.mapEntryBuilder(innerList, iId, keyValue) + .withChild(ImmutableNodes.leafNode(iLeaf, leafValue)).build(); + } +} \ No newline at end of file diff --git a/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/Bug4454Test.java b/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/Bug4454Test.java index 92906c6e72..1a835c85b3 100644 --- a/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/Bug4454Test.java +++ b/yang/yang-data-impl/src/test/java/org/opendaylight/yangtools/yang/data/impl/schema/tree/Bug4454Test.java @@ -328,7 +328,7 @@ public class Bug4454Test { .node(mapEntryPath2).build(); key.clear(); - key.put(MIN_MAX_KEY_LEAF_QNAME, "bar"); + key.put(MIN_MAX_KEY_LEAF_QNAME, "baz"); mapEntryPath2 = new YangInstanceIdentifier.NodeIdentifierWithPredicates(MIN_MAX_LIST_QNAME, key); diff --git a/yang/yang-data-impl/src/test/resources/bug-4295/foo.yang b/yang/yang-data-impl/src/test/resources/bug-4295/foo.yang new file mode 100644 index 0000000000..be6abf3bf8 --- /dev/null +++ b/yang/yang-data-impl/src/test/resources/bug-4295/foo.yang @@ -0,0 +1,27 @@ +module foo { + namespace "foo"; + prefix foo; + + container root { + container sub-root { + list outer-list { + key "o-id"; + leaf o-id { + type string; + } + list inner-list { + key "i-id"; + leaf i-id { + type string; + } + leaf i { + type string; + } + } + leaf o { + type string; + } + } + } + } +}