Extra superfluous edit-config RPC sent - Netconf-482

Extra superfluous edit-config RPC sent just prior to correct edit-config RPC for top-level lists
POST & PUT to certain top-level lists in a mounted device's model returns a 500 error via Restconf,
although in some cases (device-dependent) the config appears to have committed.
This problem first appeared in Boron-SR3, and continues in Carbon SR-1.

Change-Id: I473e6e1f34d6f2bb9aa63f12b90fc479e4a8e3cd
Signed-off-by: Atul Gosain <agosain@luminanetworks.com>

diff --git a/restconf/sal-rest-connector/src/main/java/org/opendaylight/netconf/sal/restconf/impl/BrokerFacade.java b/restconf/sal-rest-connector/src/main/java/org/opendaylight/netconf/sal/restconf/impl/BrokerFacade.java
index 63f49e320c48888b2ef8ac2b7ebb5b936d97e9b8..282aba9c51eceaab7ccb13db77a1c1ae38992ed6 100644 (file)
--- a/restconf/sal-rest-connector/src/main/java/org/opendaylight/netconf/sal/restconf/impl/BrokerFacade.java
+++ b/restconf/sal-rest-connector/src/main/java/org/opendaylight/netconf/sal/restconf/impl/BrokerFacade.java
@@ -88,6 +88,8 @@ public class BrokerFacade {
     private DOMDataBroker domDataBroker;
     private DOMNotificationService domNotification;
 
+    private ThreadLocal<Boolean> isMounted = new ThreadLocal<>();
+
     private BrokerFacade() {}
 
     public void setRpcService(final DOMRpcService router) {
@@ -236,12 +238,13 @@ public class BrokerFacade {
         Preconditions.checkNotNull(payload);
 
         checkPreconditions();
-
+        isMounted.set(false);
         final DOMDataReadWriteTransaction newReadWriteTransaction = this.domDataBroker.newReadWriteTransaction();
         final Status status = readDataViaTransaction(newReadWriteTransaction, CONFIGURATION, path) != null ? Status.OK
                 : Status.CREATED;
         final CheckedFuture<Void, TransactionCommitFailedException> future = putDataViaTransaction(
                 newReadWriteTransaction, CONFIGURATION, path, payload, globalSchema, insert, point);
+        isMounted.remove();
         return new PutResult(status, future);
     }
 
@@ -271,7 +274,7 @@ public class BrokerFacade {
         Preconditions.checkNotNull(mountPoint);
         Preconditions.checkNotNull(path);
         Preconditions.checkNotNull(payload);
-
+        isMounted.set(true);
         final Optional<DOMDataBroker> domDataBrokerService = mountPoint.getService(DOMDataBroker.class);
         if (domDataBrokerService.isPresent()) {
             final DOMDataReadWriteTransaction newReadWriteTransaction =
@@ -281,9 +284,11 @@ public class BrokerFacade {
             final CheckedFuture<Void, TransactionCommitFailedException> future = putDataViaTransaction(
                     newReadWriteTransaction, CONFIGURATION, path, payload, mountPoint.getSchemaContext(), insert,
                     point);
+            isMounted.remove();
             return new PutResult(status, future);
         }
         final String errMsg = "DOM data broker service isn't available for mount point " + path;
+        isMounted.remove();
         LOG.warn(errMsg);
         throw new RestconfDocumentedException(errMsg);
     }
@@ -465,20 +470,29 @@ public class BrokerFacade {
     public CheckedFuture<Void, TransactionCommitFailedException> commitConfigurationDataPost(
             final SchemaContext globalSchema, final YangInstanceIdentifier path, final NormalizedNode<?, ?> payload,
             final String insert, final String point) {
+        isMounted.set(false);
         checkPreconditions();
-        return postDataViaTransaction(this.domDataBroker.newReadWriteTransaction(), CONFIGURATION, path, payload,
+        CheckedFuture<Void, TransactionCommitFailedException> future =
+                postDataViaTransaction(this.domDataBroker.newReadWriteTransaction(), CONFIGURATION, path, payload,
                 globalSchema, insert, point);
+        isMounted.remove();
+        return future;
     }
 
     public CheckedFuture<Void, TransactionCommitFailedException> commitConfigurationDataPost(
             final DOMMountPoint mountPoint, final YangInstanceIdentifier path, final NormalizedNode<?, ?> payload,
             final String insert, final String point) {
+        isMounted.set(true);
         final Optional<DOMDataBroker> domDataBrokerService = mountPoint.getService(DOMDataBroker.class);
         if (domDataBrokerService.isPresent()) {
-            return postDataViaTransaction(domDataBrokerService.get().newReadWriteTransaction(), CONFIGURATION, path,
+            CheckedFuture<Void, TransactionCommitFailedException> future =
+                    postDataViaTransaction(domDataBrokerService.get().newReadWriteTransaction(), CONFIGURATION, path,
                     payload, mountPoint.getSchemaContext(), insert, point);
+            isMounted.remove();
+            return future;
         }
         final String errMsg = "DOM data broker service isn't available for mount point " + path;
+        isMounted.remove();
         LOG.warn(errMsg);
         throw new RestconfDocumentedException(errMsg);
     }
@@ -797,7 +811,7 @@ public class BrokerFacade {
         }
     }
 
-    private static void insertWithPointLeafListPost(final DOMDataReadWriteTransaction rwTransaction,
+    private void insertWithPointLeafListPost(final DOMDataReadWriteTransaction rwTransaction,
             final LogicalDatastoreType datastore, final YangInstanceIdentifier path, final NormalizedNode<?, ?> payload,
             final SchemaContext schemaContext, final String point, final OrderedLeafSetNode<?> readLeafList,
             final boolean before) {
@@ -830,7 +844,7 @@ public class BrokerFacade {
         }
     }
 
-    private static void insertWithPointListPost(final DOMDataReadWriteTransaction rwTransaction,
+    private void insertWithPointListPost(final DOMDataReadWriteTransaction rwTransaction,
             final LogicalDatastoreType datastore,
             final YangInstanceIdentifier path, final NormalizedNode<?, ?> payload, final SchemaContext schemaContext,
             final String point, final MapNode readList, final boolean before) {
@@ -885,7 +899,7 @@ public class BrokerFacade {
         throw new RestconfDocumentedException("Insert parameter can be used only with list or leaf-list");
     }
 
-    private static void makeNormalPost(final DOMDataReadWriteTransaction rwTransaction,
+    private void makeNormalPost(final DOMDataReadWriteTransaction rwTransaction,
             final LogicalDatastoreType datastore, final YangInstanceIdentifier path, final NormalizedNode<?, ?> payload,
             final SchemaContext schemaContext) {
         final Collection<? extends NormalizedNode<?, ?>> children;
@@ -900,8 +914,11 @@ public class BrokerFacade {
 
         final NormalizedNode<?, ?> emptySubtree = ImmutableNodes.fromInstanceId(schemaContext, path);
         if (children.isEmpty()) {
-            rwTransaction.merge(datastore, YangInstanceIdentifier.create(emptySubtree.getIdentifier()), emptySubtree);
-            ensureParentsByMerge(datastore, path, rwTransaction, schemaContext);
+            if (isMounted != null && !isMounted.get()) {
+                rwTransaction.merge(datastore, YangInstanceIdentifier.create(emptySubtree.getIdentifier()),
+                                    emptySubtree);
+                ensureParentsByMerge(datastore, path, rwTransaction, schemaContext);
+            }
             return;
         }
 
@@ -910,8 +927,10 @@ public class BrokerFacade {
 
         // ... now enqueue modifications. This relies on proper ordering of requests, i.e. these will not affect the
         // result of the existence checks...
-        rwTransaction.merge(datastore, YangInstanceIdentifier.create(emptySubtree.getIdentifier()), emptySubtree);
-        ensureParentsByMerge(datastore, path, rwTransaction, schemaContext);
+        if (isMounted != null && !isMounted.get()) {
+            rwTransaction.merge(datastore, YangInstanceIdentifier.create(emptySubtree.getIdentifier()), emptySubtree);
+            ensureParentsByMerge(datastore, path, rwTransaction, schemaContext);
+        }
         for (final NormalizedNode<?, ?> child : children) {
             // FIXME: we really want a create(YangInstanceIdentifier, NormalizedNode) method in the transaction,
             //        as that would allow us to skip the existence checks
@@ -940,11 +959,13 @@ public class BrokerFacade {
         }
     }
 
-    private static void simplePostPut(final DOMDataReadWriteTransaction rwTransaction,
+    private void simplePostPut(final DOMDataReadWriteTransaction rwTransaction,
             final LogicalDatastoreType datastore, final YangInstanceIdentifier path, final NormalizedNode<?, ?> payload,
             final SchemaContext schemaContext) {
         checkItemDoesNotExists(rwTransaction, datastore, path);
-        ensureParentsByMerge(datastore, path, rwTransaction, schemaContext);
+        if (isMounted != null && !isMounted.get()) {
+            ensureParentsByMerge(datastore, path, rwTransaction, schemaContext);
+        }
         rwTransaction.put(datastore, path, payload);
     }
 
@@ -1108,7 +1129,7 @@ public class BrokerFacade {
         }
     }
 
-    private static void insertWithPointLeafListPut(final DOMDataWriteTransaction tx,
+    private void insertWithPointLeafListPut(final DOMDataWriteTransaction tx,
             final LogicalDatastoreType datastore, final YangInstanceIdentifier path, final NormalizedNode<?, ?> payload,
             final SchemaContext schemaContext, final String point, final OrderedLeafSetNode<?> readLeafList,
             final boolean before) {
@@ -1139,7 +1160,7 @@ public class BrokerFacade {
         }
     }
 
-    private static void insertWithPointListPut(final DOMDataWriteTransaction tx, final LogicalDatastoreType datastore,
+    private void insertWithPointListPut(final DOMDataWriteTransaction tx, final LogicalDatastoreType datastore,
             final YangInstanceIdentifier path, final NormalizedNode<?, ?> payload, final SchemaContext schemaContext,
             final String point, final OrderedMapNode readList, final boolean before) {
         tx.delete(datastore, path.getParent());
@@ -1169,12 +1190,15 @@ public class BrokerFacade {
         }
     }
 
-    private static void makePut(final DOMDataWriteTransaction tx, final LogicalDatastoreType datastore,
+    private void makePut(final DOMDataWriteTransaction tx, final LogicalDatastoreType datastore,
             final YangInstanceIdentifier path, final NormalizedNode<?, ?> payload, final SchemaContext schemaContext) {
         if (payload instanceof MapNode) {
             final NormalizedNode<?, ?> emptySubtree = ImmutableNodes.fromInstanceId(schemaContext, path);
-            tx.merge(datastore, YangInstanceIdentifier.create(emptySubtree.getIdentifier()), emptySubtree);
-            ensureParentsByMerge(datastore, path, tx, schemaContext);
+
+            if (isMounted != null && !isMounted.get()) {
+                tx.merge(datastore, YangInstanceIdentifier.create(emptySubtree.getIdentifier()), emptySubtree);
+                ensureParentsByMerge(datastore, path, tx, schemaContext);
+            }
             for (final MapEntryNode child : ((MapNode) payload).getValue()) {
                 final YangInstanceIdentifier childPath = path.node(child.getIdentifier());
                 tx.put(datastore, childPath, child);
@@ -1184,9 +1208,11 @@ public class BrokerFacade {
         }
     }
 
-    private static void simplePut(final LogicalDatastoreType datastore, final YangInstanceIdentifier path,
+    private void simplePut(final LogicalDatastoreType datastore, final YangInstanceIdentifier path,
             final DOMDataWriteTransaction tx, final SchemaContext schemaContext, final NormalizedNode<?, ?> payload) {
-        ensureParentsByMerge(datastore, path, tx, schemaContext);
+        if (isMounted != null && !isMounted.get()) {
+            ensureParentsByMerge(datastore, path, tx, schemaContext);
+        }
         tx.put(datastore, path, payload);
     }