Do not issue empty list merges

List nodes undergo atomatic lifecycle, i.e. they are created
as needed. This means we do issue an empty merge when a list
entry is written from binding -- reducing the number of operations
issued in typical operation.

Also mark BindingToNormalizedNodeCodec.getDefaultNodeFor() as
deprecated, because it should never have been public in the first
place.

JIRA: MDSAL-383
Change-Id: Ie41dbd9d238990f3e136043b87d002bf55557501
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/AbstractWriteTransaction.java b/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/AbstractWriteTransaction.java
index 059b498440e84221944fe302431577b22995b5de..f1162ab30659ff0fc014b25d1b835b4fdeb56286 100644 (file)
--- a/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/AbstractWriteTransaction.java
+++ b/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/AbstractWriteTransaction.java
@@ -7,7 +7,8 @@
  */
 package org.opendaylight.mdsal.binding.dom.adapter;
 
-import com.google.common.base.Preconditions;
+import static com.google.common.base.Preconditions.checkArgument;
+
 import com.google.common.util.concurrent.FluentFuture;
 import java.util.Map.Entry;
 import java.util.Optional;
@@ -16,7 +17,6 @@ import org.opendaylight.mdsal.common.api.CommitInfo;
 import org.opendaylight.mdsal.common.api.LogicalDatastoreType;
 import org.opendaylight.mdsal.dom.api.DOMDataTreeWriteTransaction;
 import org.opendaylight.yangtools.yang.binding.DataObject;
-import org.opendaylight.yangtools.yang.binding.Identifiable;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode;
@@ -32,76 +32,30 @@ public abstract class AbstractWriteTransaction<T extends DOMDataTreeWriteTransac
         super(delegate, codec);
     }
 
-    public final <U extends DataObject> void put(final LogicalDatastoreType store,
-            final InstanceIdentifier<U> path, final U data, final boolean createParents) {
-        Preconditions.checkArgument(!path.isWildcarded(), "Cannot put data into wildcarded path %s", path);
+    public final <U extends DataObject> void put(final LogicalDatastoreType store, final InstanceIdentifier<U> path,
+            final U data, final boolean createParents) {
+        checkArgument(!path.isWildcarded(), "Cannot put data into wildcarded path %s", path);
 
         final Entry<YangInstanceIdentifier, NormalizedNode<?, ?>> normalized = getCodec().toNormalizedNode(path, data);
         if (createParents) {
             ensureParentsByMerge(store, normalized.getKey(), path);
-        } else {
-            ensureListParentIfNeeded(store,path,normalized);
         }
 
         getDelegate().put(store, normalized.getKey(), normalized.getValue());
     }
 
-    public final <U extends DataObject> void merge(final LogicalDatastoreType store,
-            final InstanceIdentifier<U> path, final U data,final boolean createParents) {
-        Preconditions.checkArgument(!path.isWildcarded(), "Cannot merge data into wildcarded path %s", path);
+    public final <U extends DataObject> void merge(final LogicalDatastoreType store, final InstanceIdentifier<U> path,
+            final U data,final boolean createParents) {
+        checkArgument(!path.isWildcarded(), "Cannot merge data into wildcarded path %s", path);
 
         final Entry<YangInstanceIdentifier, NormalizedNode<?, ?>> normalized = getCodec().toNormalizedNode(path, data);
         if (createParents) {
             ensureParentsByMerge(store, normalized.getKey(), path);
-        } else {
-            ensureListParentIfNeeded(store,path,normalized);
         }
 
         getDelegate().merge(store, normalized.getKey(), normalized.getValue());
     }
 
-    /**
-     * Ensures list parent if item is list, otherwise noop.
-     *
-     * <p>
-     * One of properties of binding specification is that it is imposible
-     * to represent list as a whole and thus it is impossible to write
-     * empty variation of MapNode without creating parent node, with
-     * empty list.
-     *
-     * <p>
-     * This actually makes writes such as
-     * <pre>
-     * put("Nodes", new NodesBuilder().build());
-     * put("Nodes/Node[key]", new NodeBuilder().setKey("key").build());
-     * </pre>
-     * To result in three DOM operations:
-     * <pre>
-     * put("/nodes",domNodes);
-     * merge("/nodes/node",domNodeList);
-     * put("/nodes/node/node[key]",domNode);
-     * </pre>
-     *
-     * <p>
-     * In order to allow that to be inserted if necessary, if we know
-     * item is list item, we will try to merge empty MapNode or OrderedNodeMap
-     * to ensure list exists.
-     *
-     * @param store Data Store type
-     * @param path Path to data (Binding Aware)
-     * @param normalized Normalized version of data to be written
-     */
-    private void ensureListParentIfNeeded(final LogicalDatastoreType store, final InstanceIdentifier<?> path,
-            final Entry<YangInstanceIdentifier, NormalizedNode<?, ?>> normalized) {
-        if (Identifiable.class.isAssignableFrom(path.getTargetType())) {
-            final YangInstanceIdentifier parentMapPath = normalized.getKey().getParent();
-            Preconditions.checkArgument(parentMapPath != null, "Map path %s does not have a parent", path);
-
-            final NormalizedNode<?, ?> emptyParent = getCodec().getDefaultNodeFor(parentMapPath);
-            getDelegate().merge(store, parentMapPath, emptyParent);
-        }
-    }
-
     /**
      * Use {@link YangInstanceIdentifier#getParent()} instead.
      */
@@ -111,8 +65,8 @@ public abstract class AbstractWriteTransaction<T extends DOMDataTreeWriteTransac
     }
 
     /**
-     * Subclasses of this class are required to implement creation of parent nodes based on
-     * behaviour of their underlying transaction.
+     * Subclasses of this class are required to implement creation of parent nodes based on behaviour of their
+     * underlying transaction.
      *
      * @param store an instance of LogicalDatastoreType
      * @param domPath an instance of YangInstanceIdentifier
@@ -127,9 +81,8 @@ public abstract class AbstractWriteTransaction<T extends DOMDataTreeWriteTransac
         }
     }
 
-    protected final void doDelete(final LogicalDatastoreType store,
-            final InstanceIdentifier<?> path) {
-        Preconditions.checkArgument(!path.isWildcarded(), "Cannot delete wildcarded path %s", path);
+    protected final void doDelete(final LogicalDatastoreType store, final InstanceIdentifier<?> path) {
+        checkArgument(!path.isWildcarded(), "Cannot delete wildcarded path %s", path);
 
         final YangInstanceIdentifier normalized = getCodec().toYangInstanceIdentifierBlocking(path);
         getDelegate().delete(store, normalized);
@@ -142,5 +95,4 @@ public abstract class AbstractWriteTransaction<T extends DOMDataTreeWriteTransac
     protected final boolean doCancel() {
         return getDelegate().cancel();
     }
-
 }

diff --git a/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/BindingToNormalizedNodeCodec.java b/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/BindingToNormalizedNodeCodec.java
index 7f7a3c205daeaa24db02f6568840435a6c027792..e99ce6094501ab97843e03e7434ee7977c3e3b81 100644 (file)
--- a/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/BindingToNormalizedNodeCodec.java
+++ b/binding/mdsal-binding-dom-adapter/src/main/java/org/opendaylight/mdsal/binding/dom/adapter/BindingToNormalizedNodeCodec.java
@@ -449,6 +449,14 @@ public class BindingToNormalizedNodeCodec implements BindingCodecTreeFactory,
         return ImmutableNodes.fromInstanceId(runtimeContext().getSchemaContext(), parentPath);
     }
 
+    /**
+     * This method creates an empty list container of a particular type.
+     *
+     * @deprecated This method is not generally useful, as empty lists do not convey information in YANG (they are
+     *             equivalent to non-present lists). It also leaks implementation details to a broader scope and should
+     *             never have been public in the first place.
+     */
+    @Deprecated
     public NormalizedNode<?, ?> getDefaultNodeFor(final YangInstanceIdentifier parentMapPath) {
         final BindingCodecTreeNode<?> mapCodec = requireNonNull(
                 codecRegistry.getCodecContext().getSubtreeCodec(parentMapPath),