Bug 2882: Fix off-by-one usage of cursor. 97/23897/1
authorTony Tkacik <ttkacik@cisco.com>
Wed, 8 Jul 2015 11:23:00 +0000 (13:23 +0200)
committerTony Tkacik <ttkacik@cisco.com>
Wed, 8 Jul 2015 11:23:00 +0000 (13:23 +0200)
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 <ttkacik@cisco.com>
yang/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/schema/tree/DataTreeCandidateNodes.java
yang/yang-data-api/src/main/java/org/opendaylight/yangtools/yang/data/api/schema/tree/DataTreeCandidates.java

index e1636ae77e8707baccc837e5f721ad78cbc30a55..3cc1202a26b83b86276145eb413dc616066fcc73 100644 (file)
@@ -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<DataTreeCandidateNode> iterator;
-        private final NodeIterator parent;
 
-        NodeIterator(final NodeIterator parent, final Iterator<DataTreeCandidateNode> iterator) {
-            this.parent = Preconditions.checkNotNull(parent);
+        AbstractNodeIterator(final Iterator<DataTreeCandidateNode> 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<DataTreeCandidateNode> 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<DataTreeCandidateNode> 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<DataTreeCandidateNode> iterator) {
+            super(iterator);
+            this.parent = parent;
+        }
+
+        @Override
+        protected AbstractNodeIterator getParent() {
             return parent;
         }
+
+        @Override
+        protected final void exitNode(final DataTreeModificationCursor cursor) {
+            cursor.exit();
+        }
+
     }
 }
index cf8051b02f557d9cfd281e9047a54b8337a06631..a75ec5984b7fa5f197e5656a28c67f5d7d701154 100644 (file)
@@ -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<DataTreeCandidateNode> iterator;
         private final YangInstanceIdentifier path;