From: Robert Varga Date: Wed, 19 Apr 2017 11:10:28 +0000 (+0200) Subject: BUG-7868: perform checks before starting modifications X-Git-Tag: release/nitrogen~138 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=a48011ae1a964b522eb08c0fdbf4449a00b63b22;p=netconf.git BUG-7868: perform checks before starting modifications The codepath for makeNormalPost() performs sub-optimal emulation of create(). This patch changes the logic to first check for presence and then perform modifications. This saves cycles as continuous modify/read cycles incur heavy penalty on DataTree and also allows batching of modification requests. Change-Id: Ic8e18ada094334948170f2a147da806db7db1a16 Signed-off-by: Robert Varga (cherry picked from commit 3583194dd53ca09383fee9e50299a36a622895a1) --- 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 2d72b78fae..39b9b1b93f 100644 --- 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 @@ -17,6 +17,7 @@ import com.google.common.util.concurrent.CheckedFuture; import com.google.common.util.concurrent.FutureCallback; import com.google.common.util.concurrent.Futures; import java.util.ArrayList; +import java.util.Collection; import java.util.Iterator; import java.util.List; import java.util.concurrent.CountDownLatch; @@ -562,13 +563,13 @@ public class BrokerFacade { Builders.containerBuilder((ContainerSchemaNode) baseSchemaNode); buildCont(builder, (ContainerNode) result, baseSchemaCtxTree, path, trim); return builder.build(); - } else { - final DataContainerNodeAttrBuilder builder = - Builders.mapEntryBuilder((ListSchemaNode) baseSchemaNode); - buildMapEntryBuilder(builder, (MapEntryNode) result, baseSchemaCtxTree, path, trim, - ((ListSchemaNode) baseSchemaNode).getKeyDefinition()); - return builder.build(); } + + final DataContainerNodeAttrBuilder builder = + Builders.mapEntryBuilder((ListSchemaNode) baseSchemaNode); + buildMapEntryBuilder(builder, (MapEntryNode) result, baseSchemaCtxTree, path, trim, + ((ListSchemaNode) baseSchemaNode).getKeyDefinition()); + return builder.build(); } private void buildMapEntryBuilder(final DataContainerNodeAttrBuilder builder, @@ -690,91 +691,92 @@ public class BrokerFacade { final SchemaContext schemaContext, final String insert, final String point) { if (insert == null) { makeNormalPost(rWTransaction, datastore, path, payload, schemaContext); - } else { - final DataSchemaNode schemaNode = checkListAndOrderedType(schemaContext, path); - checkItemDoesNotExists(rWTransaction, datastore, path); - switch (insert) { - case "first": - if(schemaNode instanceof ListSchemaNode){ - final OrderedMapNode readList = - (OrderedMapNode) this.readConfigurationData(path.getParent().getParent()); - if (readList == null || readList.getValue().isEmpty()) { - simplePostPut(rWTransaction, datastore, path, payload, schemaContext); - } else { - rWTransaction.delete(datastore, path.getParent().getParent()); - simplePostPut(rWTransaction, datastore, path, payload, schemaContext); - makeNormalPost(rWTransaction, datastore, path.getParent().getParent(), readList, - schemaContext); - } + return; + } + + final DataSchemaNode schemaNode = checkListAndOrderedType(schemaContext, path); + checkItemDoesNotExists(rWTransaction, datastore, path); + switch (insert) { + case "first": + if(schemaNode instanceof ListSchemaNode){ + final OrderedMapNode readList = + (OrderedMapNode) this.readConfigurationData(path.getParent().getParent()); + if (readList == null || readList.getValue().isEmpty()) { + simplePostPut(rWTransaction, datastore, path, payload, schemaContext); } else { - final OrderedLeafSetNode readLeafList = - (OrderedLeafSetNode) readConfigurationData(path.getParent()); - if (readLeafList == null || readLeafList.getValue().isEmpty()) { - simplePostPut(rWTransaction, datastore, path, payload, schemaContext); - } else { - rWTransaction.delete(datastore, path.getParent()); - simplePostPut(rWTransaction, datastore, path, payload, schemaContext); - makeNormalPost(rWTransaction, datastore, path.getParent().getParent(), readLeafList, - schemaContext); - } + rWTransaction.delete(datastore, path.getParent().getParent()); + simplePostPut(rWTransaction, datastore, path, payload, schemaContext); + makeNormalPost(rWTransaction, datastore, path.getParent().getParent(), readList, + schemaContext); } - break; - case "last": - simplePostPut(rWTransaction, datastore, path, payload, schemaContext); - break; - case "before": - if(schemaNode instanceof ListSchemaNode){ - final OrderedMapNode readList = - (OrderedMapNode) this.readConfigurationData(path.getParent().getParent()); - if (readList == null || readList.getValue().isEmpty()) { - simplePostPut(rWTransaction, datastore, path, payload, schemaContext); - } else { - insertWithPointListPost(rWTransaction, datastore, path, payload, schemaContext, point, - readList, - true); - } + } else { + final OrderedLeafSetNode readLeafList = + (OrderedLeafSetNode) readConfigurationData(path.getParent()); + if (readLeafList == null || readLeafList.getValue().isEmpty()) { + simplePostPut(rWTransaction, datastore, path, payload, schemaContext); } else { - final OrderedLeafSetNode readLeafList = - (OrderedLeafSetNode) readConfigurationData(path.getParent()); - if (readLeafList == null || readLeafList.getValue().isEmpty()) { - simplePostPut(rWTransaction, datastore, path, payload, schemaContext); - } else { - insertWithPointLeafListPost(rWTransaction, datastore, path, payload, schemaContext, point, - readLeafList, true); - } + rWTransaction.delete(datastore, path.getParent()); + simplePostPut(rWTransaction, datastore, path, payload, schemaContext); + makeNormalPost(rWTransaction, datastore, path.getParent().getParent(), readLeafList, + schemaContext); } - break; - case "after": - if (schemaNode instanceof ListSchemaNode) { - final OrderedMapNode readList = - (OrderedMapNode) this.readConfigurationData(path.getParent().getParent()); - if (readList == null || readList.getValue().isEmpty()) { - simplePostPut(rWTransaction, datastore, path, payload, schemaContext); - } else { - insertWithPointListPost(rWTransaction, datastore, path, payload, schemaContext, point, - readList, - false); - } + } + break; + case "last": + simplePostPut(rWTransaction, datastore, path, payload, schemaContext); + break; + case "before": + if(schemaNode instanceof ListSchemaNode){ + final OrderedMapNode readList = + (OrderedMapNode) this.readConfigurationData(path.getParent().getParent()); + if (readList == null || readList.getValue().isEmpty()) { + simplePostPut(rWTransaction, datastore, path, payload, schemaContext); } else { - final OrderedLeafSetNode readLeafList = - (OrderedLeafSetNode) readConfigurationData(path.getParent()); - if (readLeafList == null || readLeafList.getValue().isEmpty()) { - simplePostPut(rWTransaction, datastore, path, payload, schemaContext); - } else { - insertWithPointLeafListPost(rWTransaction, datastore, path, payload, schemaContext, point, - readLeafList, false); - } + insertWithPointListPost(rWTransaction, datastore, path, payload, schemaContext, point, + readList, + true); } - break; - default: - throw new RestconfDocumentedException( - "Used bad value of insert parameter. Possible values are first, last, before or after, " - + "but was: " + insert); - } + } else { + final OrderedLeafSetNode readLeafList = + (OrderedLeafSetNode) readConfigurationData(path.getParent()); + if (readLeafList == null || readLeafList.getValue().isEmpty()) { + simplePostPut(rWTransaction, datastore, path, payload, schemaContext); + } else { + insertWithPointLeafListPost(rWTransaction, datastore, path, payload, schemaContext, point, + readLeafList, true); + } + } + break; + case "after": + if (schemaNode instanceof ListSchemaNode) { + final OrderedMapNode readList = + (OrderedMapNode) this.readConfigurationData(path.getParent().getParent()); + if (readList == null || readList.getValue().isEmpty()) { + simplePostPut(rWTransaction, datastore, path, payload, schemaContext); + } else { + insertWithPointListPost(rWTransaction, datastore, path, payload, schemaContext, point, + readList, + false); + } + } else { + final OrderedLeafSetNode readLeafList = + (OrderedLeafSetNode) readConfigurationData(path.getParent()); + if (readLeafList == null || readLeafList.getValue().isEmpty()) { + simplePostPut(rWTransaction, datastore, path, payload, schemaContext); + } else { + insertWithPointLeafListPost(rWTransaction, datastore, path, payload, schemaContext, point, + readLeafList, false); + } + } + break; + default: + throw new RestconfDocumentedException( + "Used bad value of insert parameter. Possible values are first, last, before or after, " + + "but was: " + insert); } } - private void insertWithPointLeafListPost(final DOMDataReadWriteTransaction rWTransaction, + private static 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) { @@ -807,7 +809,7 @@ public class BrokerFacade { } } - private void insertWithPointListPost(final DOMDataReadWriteTransaction rWTransaction, + private static 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) { @@ -861,28 +863,30 @@ public class BrokerFacade { throw new RestconfDocumentedException("Insert parameter can be used only with list or leaf-list"); } - private void makeNormalPost(final DOMDataReadWriteTransaction rWTransaction, final LogicalDatastoreType datastore, - final YangInstanceIdentifier path, final NormalizedNode payload, final SchemaContext schemaContext) { + private static void makeNormalPost(final DOMDataReadWriteTransaction rWTransaction, + final LogicalDatastoreType datastore, final YangInstanceIdentifier path, final NormalizedNode payload, + final SchemaContext schemaContext) { + final Collection> children; if (payload instanceof MapNode) { - final NormalizedNode emptySubtree = ImmutableNodes.fromInstanceId(schemaContext, path); - rWTransaction.merge(datastore, YangInstanceIdentifier.create(emptySubtree.getIdentifier()), emptySubtree); - ensureParentsByMerge(datastore, path, rWTransaction, schemaContext); - for (final MapEntryNode child : ((MapNode) payload).getValue()) { - final YangInstanceIdentifier childPath = path.node(child.getIdentifier()); - checkItemDoesNotExists(rWTransaction, datastore, childPath); - rWTransaction.put(datastore, childPath, child); - } + children = ((MapNode) payload).getValue(); } else if (payload instanceof LeafSetNode) { - final NormalizedNode emptySubtree = ImmutableNodes.fromInstanceId(schemaContext, path); - rWTransaction.merge(datastore, YangInstanceIdentifier.create(emptySubtree.getIdentifier()), emptySubtree); - ensureParentsByMerge(datastore, path, rWTransaction, schemaContext); - for (final LeafSetEntryNode child : ((LeafSetNode) payload).getValue()) { - final YangInstanceIdentifier childPath = path.node(child.getIdentifier()); - checkItemDoesNotExists(rWTransaction, datastore, childPath); - rWTransaction.put(datastore, childPath, child); - } + children = ((LeafSetNode) payload).getValue(); } else { simplePostPut(rWTransaction, datastore, path, payload, schemaContext); + return; + } + + // We are putting multiple children, we really need a create() operation, but until we have that we make do + // with a two-step process of verifying if the children exist and then putting them in. + for (final NormalizedNode child : children) { + checkItemDoesNotExists(rWTransaction, datastore, path.node(child.getIdentifier())); + } + + final NormalizedNode emptySubtree = ImmutableNodes.fromInstanceId(schemaContext, path); + rWTransaction.merge(datastore, YangInstanceIdentifier.create(emptySubtree.getIdentifier()), emptySubtree); + ensureParentsByMerge(datastore, path, rWTransaction, schemaContext); + for (final NormalizedNode child : children) { + rWTransaction.put(datastore, path.node(child.getIdentifier()), child); } } @@ -1054,7 +1058,7 @@ public class BrokerFacade { } } - private void insertWithPointLeafListPut(final DOMDataReadWriteTransaction rWTransaction, + private static void insertWithPointLeafListPut(final DOMDataReadWriteTransaction rWTransaction, final LogicalDatastoreType datastore, final YangInstanceIdentifier path, final NormalizedNode payload, final SchemaContext schemaContext, final String point, final OrderedLeafSetNode readLeafList, final boolean before) { @@ -1085,7 +1089,7 @@ public class BrokerFacade { } } - private void insertWithPointListPut(final DOMDataReadWriteTransaction rWTransaction, + private static void insertWithPointListPut(final DOMDataReadWriteTransaction rWTransaction, final LogicalDatastoreType datastore, final YangInstanceIdentifier path, final NormalizedNode payload, final SchemaContext schemaContext, final String point, final OrderedMapNode readList, final boolean before) {