Bug 1280: Added option to automaticly create parents 36/8736/3
authorTony Tkacik <ttkacik@cisco.com>
Wed, 23 Jul 2014 10:11:06 +0000 (12:11 +0200)
committertpantelis <tpanteli@brocade.com>
Tue, 15 Jul 2014 00:29:08 +0000 (20:29 -0400)
Introduced additional variation of put and merge
which allows user to specify if parents nodes
are to be created. Default behaviour is to not
create parent nodes, but users still have
explicit API for creating them if necessary.

Updated documentation accordingly and added note
that auto-create of parents may potentially
create garbage in data store.

Change-Id: Id4a88b015e05b4717e9c393ddf821c7f93e7e541
Signed-off-by: Tony Tkacik <ttkacik@cisco.com>
opendaylight/md-sal/sal-binding-api/src/main/java/org/opendaylight/controller/md/sal/binding/api/WriteTransaction.java
opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/AbstractReadWriteTransaction.java
opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/AbstractWriteTransaction.java
opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/BindingDataWriteTransactionImpl.java
opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/ForwardedBackwardsCompatibleDataBroker.java
opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/md/sal/binding/impl/test/WriteTransactionTest.java

index 044a700..a6ba2e2 100644 (file)
@@ -22,6 +22,10 @@ public interface WriteTransaction extends AsyncWriteTransaction<InstanceIdentifi
     /**
      * Stores a piece of data at the specified path. This acts as an add / replace
      * operation, which is to say that whole subtree will be replaced by the specified data.
+     * * <p>
+     * This method does not automatically create missing parent nodes. It is equivalent to invoking
+     * {@link #put(LogicalDatastoreType, InstanceIdentifier, DataObject, boolean)}
+     * with <code>createMissingParents</code> set to false.
      * <p>
      * For more information on usage and examples, please see the documentation in {@link AsyncWriteTransaction}.
      * <p>
@@ -39,15 +43,50 @@ public interface WriteTransaction extends AsyncWriteTransaction<InstanceIdentifi
      */
     <T extends DataObject> void put(LogicalDatastoreType store, InstanceIdentifier<T> path, T data);
 
+
+    /**
+     * Stores a piece of data at the specified path. This acts as an add /
+     * replace operation, which is to say that whole subtree will be replaced by
+     * the specified data.
+     * <p>
+     * For more information on usage and examples, please see the documentation
+     * in {@link AsyncWriteTransaction}.
+     * <p>
+     * If you need to make sure that a parent object exists but you do not want
+     * modify its pre-existing state by using put, consider using {@link #merge}
+     * instead.
+     *
+     * Note: Using <code>createMissingParents</code> with value true, may
+     * introduce garbage in data store, or recreate nodes, which were deleted by
+     * previous transaction.
+     *
+     * @param store
+     *            the logical data store which should be modified
+     * @param path
+     *            the data object path
+     * @param data
+     *            the data object to be written to the specified path
+     * @param createMissingParents
+     *            if true, any missing parent nodes will be automatically
+     *            created using a merge operation.
+     * @throws IllegalStateException
+     *             if the transaction has already been submitted
+     */
+    <T extends DataObject> void put(LogicalDatastoreType store, InstanceIdentifier<T> path, T data,
+            boolean createMissingParents);
+
     /**
      * Merges a piece of data with the existing data at a specified path. Any pre-existing data
      * which is not explicitly overwritten will be preserved. This means that if you store a container,
      * its child lists will be merged.
      * <p>
+     * This method does not automatically create missing parent nodes. It is equivalent to invoking
+     * {@link #merge(LogicalDatastoreType, InstanceIdentifier, DataObject, boolean)}
+     * with <code>createMissingParents</code> set to false.
+     * <p>
      * For more information on usage and examples, please see the documentation in {@link AsyncWriteTransaction}.
      *<p>
      * If you require an explicit replace operation, use {@link #put} instead.
-     *
      * @param store
      *            the logical data store which should be modified
      * @param path
@@ -59,6 +98,31 @@ public interface WriteTransaction extends AsyncWriteTransaction<InstanceIdentifi
      */
     <T extends DataObject> void merge(LogicalDatastoreType store, InstanceIdentifier<T> path, T data);
 
+    /**
+     * Merges a piece of data with the existing data at a specified path. Any
+     * pre-existing data which is not explicitly overwritten will be preserved.
+     * This means that if you store a container, its child lists will be merged.
+     * <p>
+     * For more information on usage and examples, please see the documentation
+     * in {@link AsyncWriteTransaction}.
+     * <p>
+     * If you require an explicit replace operation, use {@link #put} instead.
+     *
+     * @param store
+     *            the logical data store which should be modified
+     * @param path
+     *            the data object path
+     * @param data
+     *            the data object to be merged to the specified path
+     * @param createMissingParents
+     *            if true, any missing parent nodes will be automatically created
+     *            using a merge operation.
+     * @throws IllegalStateException
+     *             if the transaction has already been submitted
+     */
+    <T extends DataObject> void merge(LogicalDatastoreType store, InstanceIdentifier<T> path, T data,
+            boolean createMissingParents);
+
     @Override
     void delete(LogicalDatastoreType store, InstanceIdentifier<?> path);
 }
index 3988bc6..44be74b 100644 (file)
@@ -10,14 +10,12 @@ package org.opendaylight.controller.md.sal.binding.impl;
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
-import java.util.Map.Entry;
 import java.util.concurrent.ExecutionException;
 
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.controller.md.sal.common.impl.util.compat.DataNormalizationException;
 import org.opendaylight.controller.md.sal.common.impl.util.compat.DataNormalizationOperation;
 import org.opendaylight.controller.md.sal.dom.api.DOMDataReadWriteTransaction;
-import org.opendaylight.yangtools.yang.binding.DataObject;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.InstanceIdentifier.PathArgument;
 import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode;
@@ -34,27 +32,8 @@ public class AbstractReadWriteTransaction extends AbstractWriteTransaction<DOMDa
         super(delegate, codec);
     }
 
-    protected final void doPutWithEnsureParents(final LogicalDatastoreType store, final InstanceIdentifier<?> path, final DataObject data) {
-        final Entry<org.opendaylight.yangtools.yang.data.api.InstanceIdentifier, NormalizedNode<?, ?>> normalized = getCodec()
-                .toNormalizedNode(path, data);
-
-        final org.opendaylight.yangtools.yang.data.api.InstanceIdentifier normalizedPath = normalized.getKey();
-        ensureParentsByMerge(store, normalizedPath, path);
-        LOG.debug("Tx: {} : Putting data {}", getDelegate().getIdentifier(), normalizedPath);
-        doPut(store, path, data);
-    }
-
-    protected final void doMergeWithEnsureParents(final LogicalDatastoreType store, final InstanceIdentifier<?> path, final DataObject data) {
-        final Entry<org.opendaylight.yangtools.yang.data.api.InstanceIdentifier, NormalizedNode<?, ?>> normalized = getCodec()
-                .toNormalizedNode(path, data);
-
-        final org.opendaylight.yangtools.yang.data.api.InstanceIdentifier normalizedPath = normalized.getKey();
-        ensureParentsByMerge(store, normalizedPath, path);
-        LOG.debug("Tx: {} : Merge data {}", getDelegate().getIdentifier(), normalizedPath);
-        doMerge(store, path, data);
-    }
-
-    private final void ensureParentsByMerge(final LogicalDatastoreType store,
+    @Override
+    protected final void ensureParentsByMerge(final LogicalDatastoreType store,
             final org.opendaylight.yangtools.yang.data.api.InstanceIdentifier normalizedPath,
             final InstanceIdentifier<?> path) {
         List<PathArgument> currentArguments = new ArrayList<>();
index a8eef5a..f8c56f9 100644 (file)
@@ -30,7 +30,7 @@ import com.google.common.util.concurrent.CheckedFuture;
  * Abstract Base Transaction for transactions which are backed by
  * {@link DOMDataWriteTransaction}
  */
-public class AbstractWriteTransaction<T extends DOMDataWriteTransaction> extends
+public abstract class AbstractWriteTransaction<T extends DOMDataWriteTransaction> extends
         AbstractForwardedTransaction<T> {
 
     private static final Logger LOG = LoggerFactory.getLogger(AbstractWriteTransaction.class);
@@ -40,15 +40,36 @@ public class AbstractWriteTransaction<T extends DOMDataWriteTransaction> extends
         super(delegate, codec);
     }
 
-    protected final void doPut(final LogicalDatastoreType store,
-            final InstanceIdentifier<?> path, final DataObject data) {
+
+    public final <U extends DataObject> void put(final LogicalDatastoreType store,
+            final InstanceIdentifier<U> path, final U data, final boolean createParents) {
        final Entry<org.opendaylight.yangtools.yang.data.api.InstanceIdentifier, NormalizedNode<?, ?>> normalized = getCodec()
                 .toNormalizedNode(path, data);
-        ensureListParentIfNeeded(store,path,normalized);
+        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) {
+
+        final Entry<org.opendaylight.yangtools.yang.data.api.InstanceIdentifier, 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.
@@ -106,14 +127,16 @@ public class AbstractWriteTransaction<T extends DOMDataWriteTransaction> extends
         }
     }
 
-    protected final void doMerge(final LogicalDatastoreType store,
-            final InstanceIdentifier<?> path, final DataObject data) {
-
-        final Entry<org.opendaylight.yangtools.yang.data.api.InstanceIdentifier, NormalizedNode<?, ?>> normalized = getCodec()
-                .toNormalizedNode(path, data);
-        ensureListParentIfNeeded(store,path,normalized);
-        getDelegate().merge(store, normalized.getKey(), normalized.getValue());
-    }
+    /**
+     * Subclasses of this class are required to implement creation of parent
+     * nodes based on behaviour of their underlying transaction.
+     *
+     * @param store
+     * @param key
+     * @param path
+     */
+    protected abstract void ensureParentsByMerge(LogicalDatastoreType store,
+            org.opendaylight.yangtools.yang.data.api.InstanceIdentifier key, InstanceIdentifier<?> path);
 
     protected final void doDelete(final LogicalDatastoreType store,
             final InstanceIdentifier<?> path) {
index 29790fb..e62b4f7 100644 (file)
@@ -7,15 +7,23 @@
  */
 package org.opendaylight.controller.md.sal.binding.impl;
 
+import java.util.ArrayList;
+import java.util.Iterator;
+import java.util.List;
+
 import org.opendaylight.controller.md.sal.binding.api.WriteTransaction;
 import org.opendaylight.controller.md.sal.common.api.TransactionStatus;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.controller.md.sal.common.api.data.TransactionCommitFailedException;
 import org.opendaylight.controller.md.sal.common.impl.service.AbstractDataTransaction;
+import org.opendaylight.controller.md.sal.common.impl.util.compat.DataNormalizationException;
+import org.opendaylight.controller.md.sal.common.impl.util.compat.DataNormalizationOperation;
 import org.opendaylight.controller.md.sal.dom.api.DOMDataWriteTransaction;
 import org.opendaylight.yangtools.yang.binding.DataObject;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 import org.opendaylight.yangtools.yang.common.RpcResult;
+import org.opendaylight.yangtools.yang.data.api.InstanceIdentifier.PathArgument;
+
 import com.google.common.util.concurrent.CheckedFuture;
 import com.google.common.util.concurrent.ListenableFuture;
 
@@ -27,15 +35,37 @@ class BindingDataWriteTransactionImpl<T extends DOMDataWriteTransaction> extends
     }
 
     @Override
-    public <T extends DataObject> void put(final LogicalDatastoreType store, final InstanceIdentifier<T> path,
-                                           final T data) {
-        doPut(store, path, data);
+    public <U extends DataObject> void put(final LogicalDatastoreType store, final InstanceIdentifier<U> path,
+                                           final U data) {
+        put(store, path, data,false);
     }
 
     @Override
     public <T extends DataObject> void merge(final LogicalDatastoreType store, final InstanceIdentifier<T> path,
                                              final T data) {
-        doMerge(store, path, data);
+        merge(store, path, data,false);
+    }
+
+
+    @Override
+    protected void ensureParentsByMerge(final LogicalDatastoreType store,
+            final org.opendaylight.yangtools.yang.data.api.InstanceIdentifier normalizedPath, final InstanceIdentifier<?> path) {
+        List<PathArgument> currentArguments = new ArrayList<>();
+        DataNormalizationOperation<?> currentOp = getCodec().getDataNormalizer().getRootOperation();
+        Iterator<PathArgument> iterator = normalizedPath.getPathArguments().iterator();
+        while (iterator.hasNext()) {
+            PathArgument currentArg = iterator.next();
+            try {
+                currentOp = currentOp.getChild(currentArg);
+            } catch (DataNormalizationException e) {
+                throw new IllegalArgumentException(String.format("Invalid child encountered in path %s", path), e);
+            }
+            currentArguments.add(currentArg);
+            org.opendaylight.yangtools.yang.data.api.InstanceIdentifier currentPath = org.opendaylight.yangtools.yang.data.api.InstanceIdentifier.create(
+                    currentArguments);
+
+            getDelegate().merge(store, currentPath, currentOp.createDefault(currentArg));
+        }
     }
 
     @Override
index 12f26b0..a243e62 100644 (file)
@@ -55,6 +55,7 @@ import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.ListeningExecutorService;
 
+@SuppressWarnings("deprecation")
 public class ForwardedBackwardsCompatibleDataBroker extends AbstractForwardedDataBroker implements DataProviderService, AutoCloseable {
 
     private static final Logger LOG = LoggerFactory.getLogger(ForwardedBackwardsCompatibleDataBroker.class);
@@ -213,10 +214,13 @@ public class ForwardedBackwardsCompatibleDataBroker extends AbstractForwardedDat
         @Override
         public void putOperationalData(final InstanceIdentifier<? extends DataObject> path, final DataObject data) {
             boolean previouslyRemoved = posponedRemovedOperational.remove(path);
+
+            @SuppressWarnings({ "rawtypes", "unchecked" })
+            final InstanceIdentifier<DataObject> castedPath = (InstanceIdentifier) path;
             if(previouslyRemoved) {
-                doPutWithEnsureParents(LogicalDatastoreType.OPERATIONAL, path, data);
+                put(LogicalDatastoreType.OPERATIONAL, castedPath, data,true);
             } else {
-                doMergeWithEnsureParents(LogicalDatastoreType.OPERATIONAL, path, data);
+                merge(LogicalDatastoreType.OPERATIONAL, castedPath, data,true);
             }
         }
 
@@ -231,10 +235,12 @@ public class ForwardedBackwardsCompatibleDataBroker extends AbstractForwardedDat
                 created.put(path, data);
             }
             updated.put(path, data);
+            @SuppressWarnings({"rawtypes","unchecked"})
+            final InstanceIdentifier<DataObject> castedPath = (InstanceIdentifier) path;
             if(previouslyRemoved) {
-                doPutWithEnsureParents(LogicalDatastoreType.CONFIGURATION, path, data);
+                put(LogicalDatastoreType.CONFIGURATION, castedPath, data,true);
             } else {
-                doMergeWithEnsureParents(LogicalDatastoreType.CONFIGURATION, path, data);
+                merge(LogicalDatastoreType.CONFIGURATION, castedPath, data,true);
             }
         }
 
index b577e2a..b504837 100644 (file)
@@ -7,12 +7,16 @@
  */
 package org.opendaylight.controller.md.sal.binding.impl.test;
 
+import static org.junit.Assert.assertTrue;
+
 import java.util.concurrent.ExecutionException;
 
 import org.junit.Test;
+import org.opendaylight.controller.md.sal.binding.api.ReadOnlyTransaction;
 import org.opendaylight.controller.md.sal.binding.api.WriteTransaction;
 import org.opendaylight.controller.md.sal.binding.test.AbstractDataBrokerTest;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
+import org.opendaylight.controller.md.sal.common.api.data.TransactionCommitFailedException;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.list.rev140701.Top;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.list.rev140701.TopBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.list.rev140701.two.level.list.TopLevelList;
@@ -20,19 +24,49 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controll
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.controller.md.sal.test.list.rev140701.two.level.list.TopLevelListKey;
 import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
 
+import com.google.common.base.Optional;
+
 
 public class WriteTransactionTest extends AbstractDataBrokerTest {
 
     private static final InstanceIdentifier<Top> TOP_PATH = InstanceIdentifier.create(Top.class);
     private static final TopLevelListKey TOP_LIST_KEY = new TopLevelListKey("foo");
     private static final InstanceIdentifier<TopLevelList> NODE_PATH = TOP_PATH.child(TopLevelList.class, TOP_LIST_KEY);
-
+    private static final TopLevelList NODE = new TopLevelListBuilder().setKey(TOP_LIST_KEY).build();
     @Test
     public void test() throws InterruptedException, ExecutionException {
         WriteTransaction writeTx = getDataBroker().newWriteOnlyTransaction();
         writeTx.put(LogicalDatastoreType.OPERATIONAL, TOP_PATH, new TopBuilder().build());
-        writeTx.put(LogicalDatastoreType.OPERATIONAL, NODE_PATH, new TopLevelListBuilder().setKey(TOP_LIST_KEY).build());
+        writeTx.put(LogicalDatastoreType.OPERATIONAL, NODE_PATH, NODE);
         writeTx.submit().get();
     }
 
+    @Test
+    public void testPutCreateParentsSuccess() throws TransactionCommitFailedException, InterruptedException, ExecutionException {
+
+        WriteTransaction writeTx = getDataBroker().newWriteOnlyTransaction();
+        writeTx.put(LogicalDatastoreType.OPERATIONAL, NODE_PATH, NODE,true);
+        writeTx.submit().checkedGet();
+
+        ReadOnlyTransaction readTx = getDataBroker().newReadOnlyTransaction();
+        Optional<Top> topNode = readTx.read(LogicalDatastoreType.OPERATIONAL, TOP_PATH).get();
+        assertTrue("Top node must exists after commit",topNode.isPresent());
+        Optional<TopLevelList> listNode = readTx.read(LogicalDatastoreType.OPERATIONAL, NODE_PATH).get();
+        assertTrue("List node must exists after commit",listNode.isPresent());
+    }
+
+    @Test
+    public void testMergeCreateParentsSuccess() throws TransactionCommitFailedException, InterruptedException, ExecutionException {
+
+        WriteTransaction writeTx = getDataBroker().newWriteOnlyTransaction();
+        writeTx.merge(LogicalDatastoreType.OPERATIONAL, NODE_PATH, NODE,true);
+        writeTx.submit().checkedGet();
+
+        ReadOnlyTransaction readTx = getDataBroker().newReadOnlyTransaction();
+        Optional<Top> topNode = readTx.read(LogicalDatastoreType.OPERATIONAL, TOP_PATH).get();
+        assertTrue("Top node must exists after commit",topNode.isPresent());
+        Optional<TopLevelList> listNode = readTx.read(LogicalDatastoreType.OPERATIONAL, NODE_PATH).get();
+        assertTrue("List node must exists after commit",listNode.isPresent());
+    }
+
 }

©2013 OpenDaylight, A Linux Foundation Collaborative Project. All Rights Reserved.
OpenDaylight is a registered trademark of The OpenDaylight Project, Inc.
Linux Foundation and OpenDaylight are registered trademarks of the Linux Foundation.
Linux is a registered trademark of Linus Torvalds.