From 9b8490316a60fc8e63597cf328ec0646d6b325e3 Mon Sep 17 00:00:00 2001 From: Tony Tkacik Date: Wed, 8 Jul 2015 13:23:00 +0200 Subject: [PATCH] Bug 2882: Fix off-by-one usage of cursor. DataTreeCandidates#applyToModification when dealing with cursor-aware transaction entered writen / element twice - once with enter and once with write which resulted in exception. Change-Id: Ifd4023c7ad54f2be1672c42725dd37f4e3ff8282 Signed-off-by: Tony Tkacik --- .../schema/tree/DataTreeCandidateNodes.java | 80 ++++++++++++++++--- .../api/schema/tree/DataTreeCandidates.java | 20 ++++- 2 files changed, 88 insertions(+), 12 deletions(-) diff --git a/yang/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/schema/tree/DataTreeCandidateNodes.java b/yang/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/schema/tree/DataTreeCandidateNodes.java index e1636ae77e..3cc1202a26 100644 --- a/yang/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/schema/tree/DataTreeCandidateNodes.java +++ b/yang/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/schema/tree/DataTreeCandidateNodes.java @@ -11,6 +11,8 @@ import com.google.common.annotations.Beta; import com.google.common.base.Preconditions; import java.util.Collection; import java.util.Iterator; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; @Beta @@ -29,8 +31,8 @@ public final class DataTreeCandidateNodes { cursor.delete(node.getIdentifier()); break; case SUBTREE_MODIFIED: - cursor.enter(node.getIdentifier()); - NodeIterator iterator = new NodeIterator(null, node.getChildNodes().iterator()); + cursor.enter(node.getIdentifier()); + AbstractNodeIterator iterator = new ExitingNodeIterator(null, node.getChildNodes().iterator()); do { iterator = iterator.next(cursor); } while (iterator != null); @@ -46,16 +48,33 @@ public final class DataTreeCandidateNodes { } } - private static final class NodeIterator { + public static void applyRootToCursor(final DataTreeModificationCursor cursor, final DataTreeCandidateNode node) { + switch (node.getModificationType()) { + case DELETE: + throw new IllegalArgumentException("Can not delete root."); + case WRITE: + case SUBTREE_MODIFIED: + AbstractNodeIterator iterator = new RootNonExitingIterator(node.getChildNodes().iterator()); + do { + iterator = iterator.next(cursor); + } while (iterator != null); + break; + case UNMODIFIED: + // No-op + break; + default: + throw new IllegalArgumentException("Unsupported modification " + node.getModificationType()); + } + } + + private static abstract class AbstractNodeIterator { private final Iterator iterator; - private final NodeIterator parent; - NodeIterator(final NodeIterator parent, final Iterator iterator) { - this.parent = Preconditions.checkNotNull(parent); + AbstractNodeIterator(final Iterator iterator) { this.iterator = Preconditions.checkNotNull(iterator); } - NodeIterator next(final DataTreeModificationCursor cursor) { + AbstractNodeIterator next(final DataTreeModificationCursor cursor) { while (iterator.hasNext()) { final DataTreeCandidateNode node = iterator.next(); switch (node.getModificationType()) { @@ -66,7 +85,7 @@ public final class DataTreeCandidateNodes { final Collection children = node.getChildNodes(); if (!children.isEmpty()) { cursor.enter(node.getIdentifier()); - return new NodeIterator(this, children.iterator()); + return new ExitingNodeIterator(this, children.iterator()); } break; case UNMODIFIED: @@ -79,9 +98,52 @@ public final class DataTreeCandidateNodes { throw new IllegalArgumentException("Unsupported modification " + node.getModificationType()); } } + exitNode(cursor); + return getParent(); + } - cursor.exit(); + protected abstract @Nullable AbstractNodeIterator getParent(); + + protected abstract void exitNode(DataTreeModificationCursor cursor); + } + + private static final class RootNonExitingIterator extends AbstractNodeIterator { + + protected RootNonExitingIterator(@Nonnull final Iterator iterator) { + super(iterator); + } + + @Override + protected void exitNode(final DataTreeModificationCursor cursor) { + // Intentional noop. + } + + @Override + protected AbstractNodeIterator getParent() { + return null; + } + + } + + private static final class ExitingNodeIterator extends AbstractNodeIterator { + + private final AbstractNodeIterator parent; + + public ExitingNodeIterator(@Nullable final AbstractNodeIterator parent, + @Nonnull final Iterator iterator) { + super(iterator); + this.parent = parent; + } + + @Override + protected AbstractNodeIterator getParent() { return parent; } + + @Override + protected final void exitNode(final DataTreeModificationCursor cursor) { + cursor.exit(); + } + } } diff --git a/yang/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/schema/tree/DataTreeCandidates.java b/yang/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/schema/tree/DataTreeCandidates.java index cf8051b02f..a75ec5984b 100644 --- a/yang/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/schema/tree/DataTreeCandidates.java +++ b/yang/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/schema/tree/DataTreeCandidates.java @@ -41,9 +41,7 @@ public final class DataTreeCandidates { public static void applyToModification(final DataTreeModification modification, final DataTreeCandidate candidate) { if (modification instanceof CursorAwareDataTreeModification) { - try (DataTreeModificationCursor cursor = ((CursorAwareDataTreeModification) modification).createCursor(candidate.getRootPath())) { - applyToCursor(cursor, candidate); - } + applyToCursorAwareModification((CursorAwareDataTreeModification) modification, candidate); return; } @@ -75,6 +73,22 @@ public final class DataTreeCandidates { } } + private static void applyToCursorAwareModification(final CursorAwareDataTreeModification modification, + final DataTreeCandidate candidate) { + final YangInstanceIdentifier candidatePath = candidate.getRootPath(); + if (candidatePath.isEmpty()) { + try (DataTreeModificationCursor cursor = + ((CursorAwareDataTreeModification) modification).createCursor(candidatePath)) { + DataTreeCandidateNodes.applyRootToCursor(cursor, candidate.getRootNode()); + } + } else { + try (DataTreeModificationCursor cursor = + ((CursorAwareDataTreeModification) modification).createCursor(candidatePath.getParent())) { + DataTreeCandidateNodes.applyToCursor(cursor, candidate.getRootNode()); + } + } + } + private static final class NodeIterator { private final Iterator iterator; private final YangInstanceIdentifier path; -- 2.36.6