Update DataTreeChangeTracker

DataTreeChangeTracker is treated as a pure DTO, whereas it has some
characteristics attached, but kept in EditConfig support.

Refactor it to remove collection leaks and duplicate code. Also optimize
current path snapshotting by instatiating YangInstanceIdentifier
eagerly.

Change-Id: I05b4e9e6d2cfd49a289b20b6ee2072fc755a0377
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/ops/DataTreeChangeTracker.java b/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/ops/DataTreeChangeTracker.java
index eb1b26d761b3668c6647ec1d6e3de095222a0f6b..da9be7a5957f3ecbe6a41df57215a3723e821397 100644 (file)
--- a/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/ops/DataTreeChangeTracker.java
+++ b/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/ops/DataTreeChangeTracker.java
@@ -5,58 +5,70 @@
  * 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.netconf.mdsal.connector.ops;
 
-import com.google.common.collect.Lists;
+import static java.util.Objects.requireNonNull;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableList.Builder;
 import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Deque;
 import java.util.List;
 import org.opendaylight.yangtools.yang.data.api.ModifyAction;
+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;
 
 public class DataTreeChangeTracker {
-
+    private final Deque<ModifyAction> actions = new ArrayDeque<>();
+    private final Deque<PathArgument> currentPath = new ArrayDeque<>();
+    private final List<DataTreeChange> dataTreeChanges = new ArrayList<>();
     private final ModifyAction defaultAction;
 
-    private final Deque<ModifyAction> actions;
-    private final Deque<PathArgument> currentPath;
-    private final ArrayList<DataTreeChange> dataTreeChanges;
     private int deleteOperationTracker = 0;
     private int removeOperationTracker = 0;
 
     public DataTreeChangeTracker(final ModifyAction defaultAction) {
-        this.defaultAction = defaultAction;
-        this.currentPath = new ArrayDeque<>();
-        this.actions = new ArrayDeque<>();
-        this.dataTreeChanges = new ArrayList<>();
+        this.defaultAction = requireNonNull(defaultAction);
     }
 
     public void pushAction(final ModifyAction action) {
-        if (ModifyAction.DELETE.equals(action)) {
-            deleteOperationTracker++;
+        switch (action) {
+            case DELETE:
+                deleteOperationTracker++;
+                break;
+            case REMOVE:
+                removeOperationTracker++;
+                break;
+            default:
+                // no-op
         }
 
-        if (ModifyAction.REMOVE.equals(action)) {
-            removeOperationTracker++;
-        }
-        this.actions.push(action);
+        actions.push(action);
     }
 
+    // Returns nullable
     public ModifyAction peekAction() {
-        return this.actions.peekFirst();
+        return actions.peekFirst();
+    }
+
+    public ModifyAction currentAction() {
+        final ModifyAction stack = peekAction();
+        return stack != null ? stack : defaultAction;
     }
 
     public ModifyAction popAction() {
         final ModifyAction popResult = actions.pop();
-        if (ModifyAction.DELETE.equals(popResult)) {
-            deleteOperationTracker--;
-        }
-
-        if (ModifyAction.REMOVE.equals(popResult)) {
-            removeOperationTracker--;
+        switch (popResult) {
+            case DELETE:
+                deleteOperationTracker--;
+                break;
+            case REMOVE:
+                removeOperationTracker--;
+                break;
+            default:
+                // no-op
         }
         return popResult;
     }
@@ -69,18 +81,14 @@ public class DataTreeChangeTracker {
         return removeOperationTracker;
     }
 
-    public void addDataTreeChange(final DataTreeChange change) {
-        dataTreeChanges.add(change);
+    public void addDataTreeChange(final ModifyAction action, final NormalizedNode<?, ?> changeRoot) {
+        dataTreeChanges.add(new DataTreeChange(changeRoot, action, currentPath));
     }
 
-    public ArrayList<DataTreeChange> getDataTreeChanges() {
+    public List<DataTreeChange> getDataTreeChanges() {
         return dataTreeChanges;
     }
 
-    public ModifyAction getDefaultAction() {
-        return defaultAction;
-    }
-
     public void pushPath(final PathArgument pathArgument) {
         currentPath.push(pathArgument);
     }
@@ -89,22 +97,19 @@ public class DataTreeChangeTracker {
         return currentPath.pop();
     }
 
-    public Deque<PathArgument> getCurrentPath() {
-        return currentPath;
-    }
-
-
     public static final class DataTreeChange {
-
         private final NormalizedNode<?, ?> changeRoot;
+        private final YangInstanceIdentifier path;
         private final ModifyAction action;
-        private final List<PathArgument> path;
 
-        public DataTreeChange(final NormalizedNode<?, ?> changeRoot, final ModifyAction action,
-                              final ArrayList<PathArgument> path) {
-            this.changeRoot = changeRoot;
-            this.action = action;
-            this.path = Lists.reverse(path);
+        DataTreeChange(final NormalizedNode<?, ?> changeRoot, final ModifyAction action,
+                final Deque<PathArgument> path) {
+            this.changeRoot = requireNonNull(changeRoot);
+            this.action = requireNonNull(action);
+
+            final Builder<PathArgument> builder = ImmutableList.builderWithExpectedSize(path.size());
+            path.descendingIterator().forEachRemaining(builder::add);
+            this.path = YangInstanceIdentifier.create(builder.build());
         }
 
         public NormalizedNode<?, ?> getChangeRoot() {
@@ -115,7 +120,7 @@ public class DataTreeChangeTracker {
             return action;
         }
 
-        public List<PathArgument> getPath() {
+        public YangInstanceIdentifier getPath() {
             return path;
         }
     }

diff --git a/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/ops/EditConfig.java b/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/ops/EditConfig.java
index 0fc35ee659b7210633489dfbcebdb48a3224f426..1e2106ffcc111eb51843251401861aff32bf5c80 100644 (file)
--- a/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/ops/EditConfig.java
+++ b/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/ops/EditConfig.java
@@ -96,7 +96,7 @@ public final class EditConfig extends AbstractEdit {
 
     private void executeChange(final DOMDataTreeReadWriteTransaction rwtx, final DataTreeChange change)
             throws DocumentedException {
-        final YangInstanceIdentifier path = YangInstanceIdentifier.create(change.getPath());
+        final YangInstanceIdentifier path = change.getPath();
         final NormalizedNode<?, ?> changeData = change.getChangeRoot();
         switch (change.getAction()) {
             case NONE:

diff --git a/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/ops/EditOperationNormalizedNodeStreamWriter.java b/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/ops/EditOperationNormalizedNodeStreamWriter.java
index d64375a887e8b6edeb8990cc5d34827d550cfef7..3420a50092c4c120a4a0cb789fd24e07e1ed512d 100644 (file)
--- a/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/ops/EditOperationNormalizedNodeStreamWriter.java
+++ b/netconf/mdsal-netconf-connector/src/main/java/org/opendaylight/netconf/mdsal/connector/ops/EditOperationNormalizedNodeStreamWriter.java
@@ -8,7 +8,6 @@
 
 package org.opendaylight.netconf.mdsal.connector.ops;
 
-import java.util.ArrayList;
 import java.util.Map;
 import org.opendaylight.netconf.api.xml.XmlNetconfConstants;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.netconf.base._1._0.rev110601.EditConfigInput;
@@ -59,8 +58,7 @@ final class EditOperationNormalizedNodeStreamWriter extends ImmutableNormalizedN
             if (!action.equals(dataTreeChangeTracker.peekAction())) {
                 final LeafNode<Object> node = ImmutableNodes.leafNode(name, value);
                 dataTreeChangeTracker.pushPath(name);
-                dataTreeChangeTracker.addDataTreeChange(new DataTreeChangeTracker.DataTreeChange(node, action,
-                        new ArrayList<>(dataTreeChangeTracker.getCurrentPath())));
+                dataTreeChangeTracker.addDataTreeChange(action, node);
                 getCurrent().removeChild(dataTreeChangeTracker.popPath());
             }
         }
@@ -91,10 +89,9 @@ final class EditOperationNormalizedNodeStreamWriter extends ImmutableNormalizedN
                 && dataTreeChangeTracker.getRemoveOperationTracker() == 0) {
             if (!action.equals(dataTreeChangeTracker.peekAction())) {
                 final LeafSetEntryNode<?> node = Builders.leafSetEntryBuilder().withNodeIdentifier(
-                        new NodeWithValue(name, value)).withValue(value).build();
+                        new NodeWithValue<>(name, value)).withValue(value).build();
                 dataTreeChangeTracker.pushPath(node.getIdentifier());
-                dataTreeChangeTracker.addDataTreeChange(new DataTreeChangeTracker.DataTreeChange(node, action,
-                        new ArrayList<>(dataTreeChangeTracker.getCurrentPath())));
+                dataTreeChangeTracker.addDataTreeChange(action, node);
                 getCurrent().removeChild(dataTreeChangeTracker.popPath());
             }
         }
@@ -161,20 +158,16 @@ final class EditOperationNormalizedNodeStreamWriter extends ImmutableNormalizedN
     // for augments and choices
     private void trackMixinNode(final PathArgument identifier) {
         dataTreeChangeTracker.pushPath(identifier);
-        dataTreeChangeTracker.pushAction(dataTreeChangeTracker.peekAction() != null
-                ? dataTreeChangeTracker.peekAction() : dataTreeChangeTracker.getDefaultAction());
+        dataTreeChangeTracker.pushAction(dataTreeChangeTracker.currentAction());
     }
 
     // for containers, (unkeyed) list entries and yang-modeled-anyxmls
     private void trackDataContainerNode(final PathArgument identifier, final Map<QName, String> attributes) {
         dataTreeChangeTracker.pushPath(identifier);
         final String operation = attributes.get(OPERATION_ATTRIBUTE);
-        if (operation != null) {
-            dataTreeChangeTracker.pushAction(ModifyAction.fromXmlValue(operation));
-        } else {
-            dataTreeChangeTracker.pushAction(dataTreeChangeTracker.peekAction() != null
-                    ? dataTreeChangeTracker.peekAction() : dataTreeChangeTracker.getDefaultAction());
-        }
+        final ModifyAction action = operation != null ? ModifyAction.fromXmlValue(operation)
+                : dataTreeChangeTracker.currentAction();
+        dataTreeChangeTracker.pushAction(action);
     }
 
     @Override
@@ -196,8 +189,7 @@ final class EditOperationNormalizedNodeStreamWriter extends ImmutableNormalizedN
                 //if parent and current actions don't match create a DataTreeChange and add it to the change list
                 //don't add a new child to the parent node
                 if (!currentAction.equals(dataTreeChangeTracker.peekAction())) {
-                    dataTreeChangeTracker.addDataTreeChange(new DataTreeChangeTracker.DataTreeChange(product,
-                            currentAction, new ArrayList<>(dataTreeChangeTracker.getCurrentPath())));
+                    dataTreeChangeTracker.addDataTreeChange(currentAction, product);
                     if (getCurrent() instanceof NormalizedNodeResultBuilder) {
                         dataTreeChangeTracker.popPath();
                         return;