From: Tony Tkacik Date: Wed, 23 Jul 2014 10:11:06 +0000 (+0200) Subject: Bug 1280: Added option to automaticly create parents X-Git-Tag: release/helium~449^2 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=45d8b39b7584805d173a143e77dbff671b60f97f Bug 1280: Added option to automaticly create parents 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 --- diff --git a/opendaylight/md-sal/sal-binding-api/src/main/java/org/opendaylight/controller/md/sal/binding/api/WriteTransaction.java b/opendaylight/md-sal/sal-binding-api/src/main/java/org/opendaylight/controller/md/sal/binding/api/WriteTransaction.java index 044a700d84..a6ba2e2799 100644 --- a/opendaylight/md-sal/sal-binding-api/src/main/java/org/opendaylight/controller/md/sal/binding/api/WriteTransaction.java +++ b/opendaylight/md-sal/sal-binding-api/src/main/java/org/opendaylight/controller/md/sal/binding/api/WriteTransaction.java @@ -22,6 +22,10 @@ public interface WriteTransaction extends AsyncWriteTransaction + * This method does not automatically create missing parent nodes. It is equivalent to invoking + * {@link #put(LogicalDatastoreType, InstanceIdentifier, DataObject, boolean)} + * with createMissingParents set to false. *

* For more information on usage and examples, please see the documentation in {@link AsyncWriteTransaction}. *

@@ -39,15 +43,50 @@ public interface WriteTransaction extends AsyncWriteTransaction void put(LogicalDatastoreType store, InstanceIdentifier 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. + *

+ * For more information on usage and examples, please see the documentation + * in {@link AsyncWriteTransaction}. + *

+ * 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 createMissingParents 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 + */ + void put(LogicalDatastoreType store, InstanceIdentifier 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. *

+ * This method does not automatically create missing parent nodes. It is equivalent to invoking + * {@link #merge(LogicalDatastoreType, InstanceIdentifier, DataObject, boolean)} + * with createMissingParents set to false. + *

* For more information on usage and examples, please see the documentation in {@link AsyncWriteTransaction}. *

* 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 void merge(LogicalDatastoreType store, InstanceIdentifier 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. + *

+ * For more information on usage and examples, please see the documentation + * in {@link AsyncWriteTransaction}. + *

+ * 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 + */ + void merge(LogicalDatastoreType store, InstanceIdentifier path, T data, + boolean createMissingParents); + @Override void delete(LogicalDatastoreType store, InstanceIdentifier path); } diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/AbstractReadWriteTransaction.java b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/AbstractReadWriteTransaction.java index 3988bc6960..44be74b424 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/AbstractReadWriteTransaction.java +++ b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/AbstractReadWriteTransaction.java @@ -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 path, final DataObject data) { - final Entry> 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> 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 currentArguments = new ArrayList<>(); diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/AbstractWriteTransaction.java b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/AbstractWriteTransaction.java index a8eef5a3ca..f8c56f95b3 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/AbstractWriteTransaction.java +++ b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/AbstractWriteTransaction.java @@ -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 extends +public abstract class AbstractWriteTransaction extends AbstractForwardedTransaction { private static final Logger LOG = LoggerFactory.getLogger(AbstractWriteTransaction.class); @@ -40,15 +40,36 @@ public class AbstractWriteTransaction extends super(delegate, codec); } - protected final void doPut(final LogicalDatastoreType store, - final InstanceIdentifier path, final DataObject data) { + + public final void put(final LogicalDatastoreType store, + final InstanceIdentifier path, final U data, final boolean createParents) { final Entry> 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 void merge(final LogicalDatastoreType store, + final InstanceIdentifier path, final U data,final boolean createParents) { + + final Entry> 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 extends } } - protected final void doMerge(final LogicalDatastoreType store, - final InstanceIdentifier path, final DataObject data) { - - final Entry> 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) { diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/BindingDataWriteTransactionImpl.java b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/BindingDataWriteTransactionImpl.java index 29790fb60a..e62b4f736a 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/BindingDataWriteTransactionImpl.java +++ b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/BindingDataWriteTransactionImpl.java @@ -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 extends } @Override - public void put(final LogicalDatastoreType store, final InstanceIdentifier path, - final T data) { - doPut(store, path, data); + public void put(final LogicalDatastoreType store, final InstanceIdentifier path, + final U data) { + put(store, path, data,false); } @Override public void merge(final LogicalDatastoreType store, final InstanceIdentifier 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 currentArguments = new ArrayList<>(); + DataNormalizationOperation currentOp = getCodec().getDataNormalizer().getRootOperation(); + Iterator 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 diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/ForwardedBackwardsCompatibleDataBroker.java b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/ForwardedBackwardsCompatibleDataBroker.java index 12f26b09bb..a243e6223a 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/ForwardedBackwardsCompatibleDataBroker.java +++ b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/md/sal/binding/impl/ForwardedBackwardsCompatibleDataBroker.java @@ -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 path, final DataObject data) { boolean previouslyRemoved = posponedRemovedOperational.remove(path); + + @SuppressWarnings({ "rawtypes", "unchecked" }) + final InstanceIdentifier 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 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); } } diff --git a/opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/md/sal/binding/impl/test/WriteTransactionTest.java b/opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/md/sal/binding/impl/test/WriteTransactionTest.java index b577e2af1f..b504837d67 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/md/sal/binding/impl/test/WriteTransactionTest.java +++ b/opendaylight/md-sal/sal-binding-broker/src/test/java/org/opendaylight/controller/md/sal/binding/impl/test/WriteTransactionTest.java @@ -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_PATH = InstanceIdentifier.create(Top.class); private static final TopLevelListKey TOP_LIST_KEY = new TopLevelListKey("foo"); private static final InstanceIdentifier 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 topNode = readTx.read(LogicalDatastoreType.OPERATIONAL, TOP_PATH).get(); + assertTrue("Top node must exists after commit",topNode.isPresent()); + Optional 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 topNode = readTx.read(LogicalDatastoreType.OPERATIONAL, TOP_PATH).get(); + assertTrue("Top node must exists after commit",topNode.isPresent()); + Optional listNode = readTx.read(LogicalDatastoreType.OPERATIONAL, NODE_PATH).get(); + assertTrue("List node must exists after commit",listNode.isPresent()); + } + }