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 044a700d84f26b968cb29f6fe3cd6a80e0dbf0ec..a6ba2e2799b99f884c119dae6ef67e44bd85a562 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 3988bc6960266654e86110f3c3c96764b8db854d..44be74b4248b0d25ee40d484d17ff8c96e82d6a5 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 a8eef5a3cae2687f6cc552454407e0bb8bf9eb97..f8c56f95b3aaa26848faca888085bf790446f503 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 29790fb60a1a22209047acd44d1057087b707194..e62b4f736a944c180f47a79e3eff132011452491 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 12f26b09bb2cf1645d645438302e705c3c0489ce..a243e6223ac3525a29d496581cf25b04257d1513 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 b577e2af1f2c845586615985fa77ec1169abd291..b504837d6780aa344eb706a786733b321a2fa65e 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());
+    }
+
 }