From a8a56843c3dc31f61a01d6eff31cb79a5381398a Mon Sep 17 00:00:00 2001 From: Vladyslav Marchenko Date: Tue, 15 Sep 2020 13:29:46 +0300 Subject: [PATCH] Eliminate unnecessary blocking checks According to RFC6241: 1) "delete" operation must first check if the configuration data currently exists in the configuration datastore. 2) "create" operation must first check if the configuration data does not already exist in the configuration datastore. 3) "replace" operation replaces any related configuration in the configuration datastore. If no such configuration data exists in the configuration datastore, it is created. If request goes via NetconfRestconfStrategy, we do not need to check existence on the controller side, as the implied checks are part of the underlyig behavior. If request goes via MdsalRestconfStrategy, first we need to do "if exists" check. Read requests in this case are collected in a batch and then collecting the results. JIRA: NETCONF-403 Change-Id: I6354c345d099017c1e82878af36ac37bbf5662c9 Signed-off-by: Vladyslav Marchenko Signed-off-by: Robert Varga --- .../transactions/BatchedExistenceCheck.java | 93 +++++++ .../transactions/MdsalRestconfStrategy.java | 94 ++++++- .../transactions/NetconfRestconfStrategy.java | 42 ++- .../rests/transactions/RestconfStrategy.java | 28 +- .../utils/DeleteDataTransactionUtil.java | 25 +- .../rfc8040/rests/utils/FutureCallbackTx.java | 45 +++- .../rests/utils/PatchDataTransactionUtil.java | 170 ++++-------- .../rests/utils/PostDataTransactionUtil.java | 214 ++++----------- .../rests/utils/PutDataTransactionUtil.java | 248 ++++-------------- .../rfc8040/rests/utils/ResponseFactory.java | 8 + .../utils/DeleteDataTransactionUtilTest.java | 22 +- .../utils/PatchDataTransactionUtilTest.java | 39 ++- .../utils/PostDataTransactionUtilTest.java | 7 - .../utils/PutDataTransactionUtilTest.java | 6 +- 14 files changed, 496 insertions(+), 545 deletions(-) create mode 100644 restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/BatchedExistenceCheck.java diff --git a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/BatchedExistenceCheck.java b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/BatchedExistenceCheck.java new file mode 100644 index 0000000000..8cfbab1182 --- /dev/null +++ b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/BatchedExistenceCheck.java @@ -0,0 +1,93 @@ +/* + * Copyright (c) 2017 Pantheon Technologies, s.r.o. and others. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v1.0 which accompanies this distribution, + * and is available at http://www.eclipse.org/legal/epl-v10.html + */ +package org.opendaylight.restconf.nb.rfc8040.rests.transactions; + +import com.google.common.util.concurrent.FutureCallback; +import com.google.common.util.concurrent.MoreExecutors; +import com.google.common.util.concurrent.SettableFuture; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import java.util.AbstractMap.SimpleImmutableEntry; +import java.util.Collection; +import java.util.Map.Entry; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.atomic.AtomicIntegerFieldUpdater; +import org.opendaylight.mdsal.common.api.LogicalDatastoreType; +import org.opendaylight.mdsal.common.api.ReadFailedException; +import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; +import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; + +final class BatchedExistenceCheck { + private static final AtomicIntegerFieldUpdater UPDATER = + AtomicIntegerFieldUpdater.newUpdater(BatchedExistenceCheck.class, "outstanding"); + + private final SettableFuture> future = SettableFuture.create(); + + @SuppressWarnings("unused") + private volatile int outstanding; + + private BatchedExistenceCheck(final int total) { + this.outstanding = total; + } + + static BatchedExistenceCheck start(final RestconfStrategy read, + final LogicalDatastoreType datastore, final YangInstanceIdentifier parentPath, + final Collection> children) { + final BatchedExistenceCheck ret = new BatchedExistenceCheck(children.size()); + for (NormalizedNode child : children) { + final YangInstanceIdentifier path = parentPath.node(child.getIdentifier()); + read.exists(datastore, path).addCallback(new FutureCallback() { + @Override + public void onSuccess(final Boolean result) { + ret.complete(path, result); + } + + @Override + @SuppressFBWarnings("BC_UNCONFIRMED_CAST_OF_RETURN_VALUE") + public void onFailure(final Throwable throwable) { + final Exception e; + if (throwable instanceof Exception) { + e = (Exception) throwable; + } else { + e = new ExecutionException(throwable); + } + + ret.complete(path, ReadFailedException.MAPPER.apply(e)); + } + }, MoreExecutors.directExecutor()); + } + + return ret; + } + + Entry getFailure() throws InterruptedException { + try { + return future.get(); + } catch (ExecutionException e) { + // This should never happen + throw new IllegalStateException(e); + } + } + + @SuppressFBWarnings(value = "UPM_UNCALLED_PRIVATE_METHOD", + justification = "https://github.com/spotbugs/spotbugs/issues/811") + private void complete(final YangInstanceIdentifier childPath, final boolean present) { + final int count = UPDATER.decrementAndGet(this); + if (present) { + future.set(new SimpleImmutableEntry<>(childPath, null)); + } else if (count == 0) { + future.set(null); + } + } + + @SuppressFBWarnings(value = "UPM_UNCALLED_PRIVATE_METHOD", + justification = "https://github.com/spotbugs/spotbugs/issues/811") + private void complete(final YangInstanceIdentifier childPath, final ReadFailedException cause) { + UPDATER.decrementAndGet(this); + future.set(new SimpleImmutableEntry<>(childPath, cause)); + } +} diff --git a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/MdsalRestconfStrategy.java b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/MdsalRestconfStrategy.java index c2eb2fbd9e..3923828a10 100644 --- a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/MdsalRestconfStrategy.java +++ b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/MdsalRestconfStrategy.java @@ -8,20 +8,34 @@ package org.opendaylight.restconf.nb.rfc8040.rests.transactions; import static java.util.Objects.requireNonNull; +import static org.opendaylight.restconf.nb.rfc8040.rests.utils.DeleteDataTransactionUtil.DELETE_TX_TYPE; +import static org.opendaylight.restconf.nb.rfc8040.rests.utils.PostDataTransactionUtil.checkItemDoesNotExists; import com.google.common.util.concurrent.FluentFuture; import com.google.common.util.concurrent.ListenableFuture; +import java.util.Collection; +import java.util.Map; import java.util.Optional; import org.eclipse.jdt.annotation.NonNull; import org.opendaylight.mdsal.common.api.CommitInfo; import org.opendaylight.mdsal.common.api.LogicalDatastoreType; +import org.opendaylight.mdsal.common.api.ReadFailedException; import org.opendaylight.mdsal.dom.api.DOMDataBroker; import org.opendaylight.mdsal.dom.api.DOMDataTreeReadTransaction; import org.opendaylight.mdsal.dom.api.DOMDataTreeReadWriteTransaction; import org.opendaylight.mdsal.dom.api.DOMTransactionChain; +import org.opendaylight.restconf.common.errors.RestconfDocumentedException; +import org.opendaylight.restconf.common.errors.RestconfError; import org.opendaylight.restconf.nb.rfc8040.handlers.TransactionChainHandler; +import org.opendaylight.restconf.nb.rfc8040.rests.utils.DeleteDataTransactionUtil; +import org.opendaylight.restconf.nb.rfc8040.rests.utils.TransactionUtil; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; +import org.opendaylight.yangtools.yang.data.api.schema.LeafSetNode; +import org.opendaylight.yangtools.yang.data.api.schema.MapNode; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; +import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNodeContainer; +import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes; +import org.opendaylight.yangtools.yang.model.api.SchemaContext; /** * Implementation of RESTCONF operations using {@link DOMTransactionChain} and related concepts. @@ -59,9 +73,13 @@ public final class MdsalRestconfStrategy extends RestconfStrategy { @Override public ListenableFuture>> read(final LogicalDatastoreType store, - final YangInstanceIdentifier path) { - try (DOMDataTreeReadTransaction tx = transactionChain.newReadOnlyTransaction()) { - return tx.read(store, path); + final YangInstanceIdentifier path) { + if (rwTx != null) { + return rwTx.read(store, path); + } else { + try (DOMDataTreeReadTransaction tx = transactionChain.newReadOnlyTransaction()) { + return tx.read(store, path); + } } } @@ -72,6 +90,13 @@ public final class MdsalRestconfStrategy extends RestconfStrategy { @Override public void delete(final LogicalDatastoreType store, final YangInstanceIdentifier path) { + DeleteDataTransactionUtil.checkItemExists(this, LogicalDatastoreType.CONFIGURATION, path, + DELETE_TX_TYPE); + rwTx.delete(store, path); + } + + @Override + public void remove(final LogicalDatastoreType store, final YangInstanceIdentifier path) { rwTx.delete(store, path); } @@ -83,14 +108,49 @@ public final class MdsalRestconfStrategy extends RestconfStrategy { @Override public void create(final LogicalDatastoreType store, final YangInstanceIdentifier path, - final NormalizedNode data) { - rwTx.put(store, path, data); + final NormalizedNode data, final SchemaContext schemaContext) { + if (data instanceof MapNode || data instanceof LeafSetNode) { + final NormalizedNode emptySubTree = ImmutableNodes.fromInstanceId(schemaContext, path); + merge(LogicalDatastoreType.CONFIGURATION, YangInstanceIdentifier.create(emptySubTree.getIdentifier()), + emptySubTree); + TransactionUtil.ensureParentsByMerge(path, schemaContext, this); + + final Collection> children = + ((NormalizedNodeContainer) data).getValue(); + final BatchedExistenceCheck check = + BatchedExistenceCheck.start(this, LogicalDatastoreType.CONFIGURATION, path, children); + + for (final NormalizedNode child : children) { + final YangInstanceIdentifier childPath = path.node(child.getIdentifier()); + rwTx.put(store, childPath, child); + } + + // ... finally collect existence checks and abort the transaction if any of them failed. + checkExistence(path, check); + } else { + checkItemDoesNotExists(this, LogicalDatastoreType.CONFIGURATION, path); + TransactionUtil.ensureParentsByMerge(path, schemaContext, this); + rwTx.put(store, path, data); + } } @Override public void replace(final LogicalDatastoreType store, final YangInstanceIdentifier path, - final NormalizedNode data) { - create(store, path, data); + final NormalizedNode data, final SchemaContext schemaContext) { + if (data instanceof MapNode || data instanceof LeafSetNode) { + final NormalizedNode emptySubtree = ImmutableNodes.fromInstanceId(schemaContext, path); + merge(LogicalDatastoreType.CONFIGURATION, YangInstanceIdentifier.create(emptySubtree.getIdentifier()), + emptySubtree); + TransactionUtil.ensureParentsByMerge(path, schemaContext, this); + + for (final NormalizedNode child : ((NormalizedNodeContainer) data).getValue()) { + final YangInstanceIdentifier childPath = path.node(child.getIdentifier()); + rwTx.put(store, childPath, child); + } + } else { + TransactionUtil.ensureParentsByMerge(path, schemaContext, this); + rwTx.put(store, path, data); + } } @Override @@ -107,4 +167,24 @@ public final class MdsalRestconfStrategy extends RestconfStrategy { public TransactionChainHandler getTransactionChainHandler() { return transactionChainHandler; } + + private static void checkExistence(final YangInstanceIdentifier path, final BatchedExistenceCheck check) { + final Map.Entry failure; + try { + failure = check.getFailure(); + } catch (InterruptedException e) { + throw new RestconfDocumentedException("Could not determine the existence of path " + path, e); + } + + if (failure != null) { + final ReadFailedException e = failure.getValue(); + if (e == null) { + throw new RestconfDocumentedException("Data already exists", + RestconfError.ErrorType.PROTOCOL, RestconfError.ErrorTag.DATA_EXISTS, failure.getKey()); + } + + throw new RestconfDocumentedException( + "Could not determine the existence of path " + failure.getKey(), e, e.getErrorList()); + } + } } diff --git a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/NetconfRestconfStrategy.java b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/NetconfRestconfStrategy.java index 51bdd14d09..19a60f376d 100644 --- a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/NetconfRestconfStrategy.java +++ b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/NetconfRestconfStrategy.java @@ -26,7 +26,12 @@ import org.opendaylight.mdsal.dom.api.DOMTransactionChain; import org.opendaylight.netconf.dom.api.NetconfDataTreeService; import org.opendaylight.restconf.nb.rfc8040.handlers.TransactionChainHandler; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; +import org.opendaylight.yangtools.yang.data.api.schema.LeafSetNode; +import org.opendaylight.yangtools.yang.data.api.schema.MapNode; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; +import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNodeContainer; +import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes; +import org.opendaylight.yangtools.yang.model.api.SchemaContext; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -85,6 +90,11 @@ public final class NetconfRestconfStrategy extends RestconfStrategy { resultsFutures.add(netconfService.delete(store, path)); } + @Override + public void remove(final LogicalDatastoreType store, final YangInstanceIdentifier path) { + resultsFutures.add(netconfService.remove(store, path)); + } + @Override public void merge(final LogicalDatastoreType store, final YangInstanceIdentifier path, final NormalizedNode data) { @@ -93,14 +103,36 @@ public final class NetconfRestconfStrategy extends RestconfStrategy { @Override public void create(final LogicalDatastoreType store, final YangInstanceIdentifier path, - final NormalizedNode data) { - resultsFutures.add(netconfService.create(store, path, data, Optional.empty())); + final NormalizedNode data, final SchemaContext schemaContext) { + if (data instanceof MapNode || data instanceof LeafSetNode) { + final NormalizedNode emptySubTree = ImmutableNodes.fromInstanceId(schemaContext, path); + merge(LogicalDatastoreType.CONFIGURATION, YangInstanceIdentifier.create(emptySubTree.getIdentifier()), + emptySubTree); + + for (final NormalizedNode child : ((NormalizedNodeContainer) data).getValue()) { + final YangInstanceIdentifier childPath = path.node(child.getIdentifier()); + resultsFutures.add(netconfService.create(store, childPath, child, Optional.empty())); + } + } else { + resultsFutures.add(netconfService.create(store, path, data, Optional.empty())); + } } @Override public void replace(final LogicalDatastoreType store, final YangInstanceIdentifier path, - final NormalizedNode data) { - resultsFutures.add(netconfService.replace(store, path, data, Optional.empty())); + final NormalizedNode data, final SchemaContext schemaContext) { + if (data instanceof MapNode || data instanceof LeafSetNode) { + final NormalizedNode emptySubTree = ImmutableNodes.fromInstanceId(schemaContext, path); + merge(LogicalDatastoreType.CONFIGURATION, YangInstanceIdentifier.create(emptySubTree.getIdentifier()), + emptySubTree); + + for (final NormalizedNode child : ((NormalizedNodeContainer) data).getValue()) { + final YangInstanceIdentifier childPath = path.node(child.getIdentifier()); + resultsFutures.add(netconfService.replace(store, childPath, child, Optional.empty())); + } + } else { + resultsFutures.add(netconfService.replace(store, path, data, Optional.empty())); + } } @Override @@ -136,7 +168,7 @@ public final class NetconfRestconfStrategy extends RestconfStrategy { @Override public void onFailure(final Throwable cause) { ret.setException(cause instanceof ReadFailedException ? cause - : new ReadFailedException("NETCONF operation failed", cause)); + : new ReadFailedException("NETCONF operation failed", cause)); } }, MoreExecutors.directExecutor()); return FluentFuture.from(ret); diff --git a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/RestconfStrategy.java b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/RestconfStrategy.java index 0c01e6784f..bfecd8893a 100644 --- a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/RestconfStrategy.java +++ b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/transactions/RestconfStrategy.java @@ -21,6 +21,7 @@ import org.opendaylight.netconf.dom.api.NetconfDataTreeService; import org.opendaylight.restconf.nb.rfc8040.handlers.TransactionChainHandler; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; +import org.opendaylight.yangtools.yang.model.api.SchemaContext; /** * Baseline execution strategy for various RESTCONF operations. @@ -96,6 +97,14 @@ public abstract class RestconfStrategy { */ public abstract void delete(LogicalDatastoreType store, YangInstanceIdentifier path); + /** + * Remove data from the datastore. + * + * @param store the logical data store which should be modified + * @param path the data object path + */ + public abstract void remove(LogicalDatastoreType store, YangInstanceIdentifier path); + /** * Merges a piece of data with the existing data at a specified path. * @@ -109,19 +118,24 @@ public abstract class RestconfStrategy { * Stores a piece of data at the specified path. * * @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 path the data object path + * @param data the data object to be merged to the specified path + * @param schemaContext static view of compiled yang files */ - public abstract void create(LogicalDatastoreType store, YangInstanceIdentifier path, NormalizedNode data); + public abstract void create(LogicalDatastoreType store, YangInstanceIdentifier path, NormalizedNode data, + SchemaContext schemaContext); /** * Replace a piece of data at the specified path. * - * @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 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 schemaContext static view of compiled yang files */ - public abstract void replace(LogicalDatastoreType store, YangInstanceIdentifier path, NormalizedNode data); + public abstract void replace(LogicalDatastoreType store, YangInstanceIdentifier path, NormalizedNode data, + SchemaContext schemaContext); + /** * Get transaction chain for creating specific transaction for specific operation. diff --git a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/DeleteDataTransactionUtil.java b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/DeleteDataTransactionUtil.java index 6b3f0a0146..ba6d10b1cd 100644 --- a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/DeleteDataTransactionUtil.java +++ b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/DeleteDataTransactionUtil.java @@ -25,7 +25,7 @@ import org.slf4j.LoggerFactory; */ public final class DeleteDataTransactionUtil { private static final Logger LOG = LoggerFactory.getLogger(DeleteDataTransactionUtil.class); - private static final String DELETE_TX_TYPE = "DELETE"; + public static final String DELETE_TX_TYPE = "DELETE"; private DeleteDataTransactionUtil() { throw new UnsupportedOperationException("Util class."); @@ -39,12 +39,17 @@ public final class DeleteDataTransactionUtil { */ public static Response deleteData(final RestconfStrategy strategy, final YangInstanceIdentifier path) { strategy.prepareReadWriteExecution(); - checkItemExists(strategy, LogicalDatastoreType.CONFIGURATION, path, DELETE_TX_TYPE); - strategy.delete(LogicalDatastoreType.CONFIGURATION, path); + try { + strategy.delete(LogicalDatastoreType.CONFIGURATION, path); + } catch (RestconfDocumentedException e) { + // close transaction if any and pass exception further + strategy.cancel(); + throw e; + } final FluentFuture future = strategy.commit(); final ResponseFactory response = new ResponseFactory(Status.NO_CONTENT); //This method will close transactionChain if any - FutureCallbackTx.addCallback(future, DELETE_TX_TYPE, response, strategy.getTransactionChain()); + FutureCallbackTx.addCallback(future, DELETE_TX_TYPE, response, strategy.getTransactionChain(), path); return response.build(); } @@ -57,21 +62,17 @@ public final class DeleteDataTransactionUtil { * @param path Path to be checked * @param operationType Type of operation (READ, POST, PUT, DELETE...) */ - private static void checkItemExists(final RestconfStrategy strategy, - final LogicalDatastoreType store, final YangInstanceIdentifier path, - final String operationType) { + public static void checkItemExists(final RestconfStrategy strategy, + final LogicalDatastoreType store, final YangInstanceIdentifier path, + final String operationType) { final FluentFuture future = strategy.exists(store, path); final FutureDataFactory response = new FutureDataFactory<>(); - FutureCallbackTx.addCallback(future, operationType, response); if (!response.result) { - // close transaction - strategy.cancel(); - // throw error LOG.trace("Operation via Restconf was not executed because data at {} does not exist", path); throw new RestconfDocumentedException( - "Data does not exist", ErrorType.PROTOCOL, ErrorTag.DATA_MISSING, path); + "Data does not exist", ErrorType.PROTOCOL, ErrorTag.DATA_MISSING, path); } } } diff --git a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/FutureCallbackTx.java b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/FutureCallbackTx.java index 96423dc690..b68cd5177d 100644 --- a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/FutureCallbackTx.java +++ b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/FutureCallbackTx.java @@ -19,11 +19,13 @@ import org.opendaylight.mdsal.dom.api.DOMRpcException; import org.opendaylight.mdsal.dom.api.DOMTransactionChain; import org.opendaylight.mdsal.dom.spi.DefaultDOMRpcResult; import org.opendaylight.mdsal.dom.spi.SimpleDOMActionResult; +import org.opendaylight.netconf.api.DocumentedException; import org.opendaylight.netconf.api.NetconfDocumentedException; import org.opendaylight.restconf.common.errors.RestconfDocumentedException; import org.opendaylight.restconf.common.errors.RestconfError; import org.opendaylight.yangtools.yang.common.RpcError; import org.opendaylight.yangtools.yang.common.RpcResultBuilder; +import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -31,7 +33,6 @@ import org.slf4j.LoggerFactory; * Add callback for future objects and result set to the data factory. */ final class FutureCallbackTx { - private static final Logger LOG = LoggerFactory.getLogger(FutureCallbackTx.class); private FutureCallbackTx() { @@ -53,7 +54,7 @@ final class FutureCallbackTx { // FIXME: this is a *synchronous operation* and has to die static void addCallback(final ListenableFuture listenableFuture, final String txType, final FutureDataFactory dataFactory) throws RestconfDocumentedException { - addCallback(listenableFuture,txType,dataFactory,null); + addCallback(listenableFuture, txType, dataFactory, null, null); } /** @@ -72,9 +73,31 @@ final class FutureCallbackTx { */ // FIXME: this is a *synchronous operation* and has to die static void addCallback(final ListenableFuture listenableFuture, final String txType, - final FutureDataFactory dataFactory, @Nullable final DOMTransactionChain transactionChain) - throws RestconfDocumentedException { + final FutureDataFactory dataFactory, + @Nullable final DOMTransactionChain transactionChain) + throws RestconfDocumentedException { + addCallback(listenableFuture, txType, dataFactory, transactionChain, null); + } + /** + * Add callback to the future object and close transaction chain. + * + * @param listenableFuture + * future object + * @param txType + * type of operation (READ, POST, PUT, DELETE) + * @param dataFactory + * factory setting result + * @param transactionChain + * transaction chain + * @param path + * unique identifier of a particular node instance in the data tree. + * @throws RestconfDocumentedException + * if the Future throws an exception + */ + static void addCallback(final ListenableFuture listenableFuture, final String txType, + final FutureDataFactory dataFactory, @Nullable final DOMTransactionChain transactionChain, + YangInstanceIdentifier path) throws RestconfDocumentedException { try { final T result = listenableFuture.get(); dataFactory.setResult(result); @@ -102,6 +125,20 @@ final class FutureCallbackTx { */ final List causalChain = Throwables.getCausalChain(cause); for (Throwable error : causalChain) { + if (error instanceof DocumentedException) { + final DocumentedException.ErrorTag errorTag = ((DocumentedException) error).getErrorTag(); + if (errorTag.equals(DocumentedException.ErrorTag.DATA_EXISTS)) { + LOG.trace("Operation via Restconf was not executed because data at {} already exists", + path); + throw new RestconfDocumentedException(e, new RestconfError(RestconfError.ErrorType.PROTOCOL, + RestconfError.ErrorTag.DATA_EXISTS, "Data already exists", path)); + } else if (errorTag.equals(DocumentedException.ErrorTag.DATA_MISSING)) { + LOG.trace("Operation via Restconf was not executed because data at {} does not exist", + path); + throw new RestconfDocumentedException(e, new RestconfError(RestconfError.ErrorType.PROTOCOL, + RestconfError.ErrorTag.DATA_MISSING, "Data does not exist", path)); + } + } if (error instanceof NetconfDocumentedException) { throw new RestconfDocumentedException(error.getMessage(), RestconfError.ErrorType.valueOfCaseInsensitive( diff --git a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/PatchDataTransactionUtil.java b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/PatchDataTransactionUtil.java index f9d2f371b9..4d165dc1e7 100644 --- a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/PatchDataTransactionUtil.java +++ b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/PatchDataTransactionUtil.java @@ -26,10 +26,7 @@ import org.opendaylight.restconf.common.patch.PatchStatusContext; import org.opendaylight.restconf.common.patch.PatchStatusEntity; import org.opendaylight.restconf.nb.rfc8040.rests.transactions.RestconfStrategy; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; -import org.opendaylight.yangtools.yang.data.api.schema.MapEntryNode; -import org.opendaylight.yangtools.yang.data.api.schema.MapNode; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; -import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes; import org.opendaylight.yangtools.yang.model.api.EffectiveModelContext; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -63,8 +60,8 @@ public final class PatchDataTransactionUtil { switch (patchEntity.getOperation()) { case CREATE: try { - createDataWithinTransaction(LogicalDatastoreType.CONFIGURATION, - patchEntity.getTargetNode(), patchEntity.getNode(), strategy, schemaContext); + createDataWithinTransaction(patchEntity.getTargetNode(), patchEntity.getNode(), + schemaContext, strategy); editCollection.add(new PatchStatusEntity(patchEntity.getEditId(), true, null)); } catch (final RestconfDocumentedException e) { editCollection.add(new PatchStatusEntity(patchEntity.getEditId(), @@ -74,8 +71,7 @@ public final class PatchDataTransactionUtil { break; case DELETE: try { - deleteDataWithinTransaction(LogicalDatastoreType.CONFIGURATION, patchEntity.getTargetNode(), - strategy); + deleteDataWithinTransaction(patchEntity.getTargetNode(), strategy); editCollection.add(new PatchStatusEntity(patchEntity.getEditId(), true, null)); } catch (final RestconfDocumentedException e) { editCollection.add(new PatchStatusEntity(patchEntity.getEditId(), @@ -85,8 +81,8 @@ public final class PatchDataTransactionUtil { break; case MERGE: try { - mergeDataWithinTransaction(LogicalDatastoreType.CONFIGURATION, - patchEntity.getTargetNode(), patchEntity.getNode(), strategy, schemaContext); + mergeDataWithinTransaction(patchEntity.getTargetNode(), patchEntity.getNode(), + schemaContext, strategy); editCollection.add(new PatchStatusEntity(patchEntity.getEditId(), true, null)); } catch (final RestconfDocumentedException e) { editCollection.add(new PatchStatusEntity(patchEntity.getEditId(), @@ -96,8 +92,8 @@ public final class PatchDataTransactionUtil { break; case REPLACE: try { - replaceDataWithinTransaction(LogicalDatastoreType.CONFIGURATION, - patchEntity.getTargetNode(), patchEntity.getNode(), schemaContext, strategy); + replaceDataWithinTransaction(patchEntity.getTargetNode(), patchEntity.getNode(), + schemaContext, strategy); editCollection.add(new PatchStatusEntity(patchEntity.getEditId(), true, null)); } catch (final RestconfDocumentedException e) { editCollection.add(new PatchStatusEntity(patchEntity.getEditId(), @@ -107,8 +103,7 @@ public final class PatchDataTransactionUtil { break; case REMOVE: try { - removeDataWithinTransaction(LogicalDatastoreType.CONFIGURATION, patchEntity.getTargetNode(), - strategy); + removeDataWithinTransaction(patchEntity.getTargetNode(), strategy); editCollection.add(new PatchStatusEntity(patchEntity.getEditId(), true, null)); } catch (final RestconfDocumentedException e) { editCollection.add(new PatchStatusEntity(patchEntity.getEditId(), @@ -142,175 +137,104 @@ public final class PatchDataTransactionUtil { Lists.newArrayList(e.getErrors())); } - return new PatchStatusContext(context.getPatchId(), ImmutableList.copyOf(editCollection), - true, null); + return new PatchStatusContext(context.getPatchId(), ImmutableList.copyOf(editCollection), true, null); } else { strategy.cancel(); - return new PatchStatusContext(context.getPatchId(), ImmutableList.copyOf(editCollection), - false, null); + return new PatchStatusContext(context.getPatchId(), ImmutableList.copyOf(editCollection), false, null); } } /** * Create data within one transaction, return error if already exists. * - * @param dataStore Datastore to write data to * @param path Path for data to be created * @param payload Data to be created * @param strategy Object that perform the actual DS operations - * @param schemaContext Global schema context */ - private static void createDataWithinTransaction(final LogicalDatastoreType dataStore, - final YangInstanceIdentifier path, + private static void createDataWithinTransaction(final YangInstanceIdentifier path, final NormalizedNode payload, - final RestconfStrategy strategy, - final EffectiveModelContext schemaContext) { - LOG.trace("POST {} within Restconf Patch: {} with payload {}", dataStore.name(), path, payload); - createData(payload, schemaContext, path, strategy, dataStore, true); + final EffectiveModelContext schemaContext, + final RestconfStrategy strategy) { + LOG.trace("POST {} within Restconf Patch: {} with payload {}", LogicalDatastoreType.CONFIGURATION.name(), + path, payload); + createData(payload, path, strategy, schemaContext, true); } /** - * Check if data exists and remove it within one transaction. + * Remove data within one transaction. * - * @param dataStore Datastore to delete data from - * @param path Path for data to be deleted - * @param strategy Object that perform the actual DS operations + * @param path Path for data to be deleted + * @param strategy Object that perform the actual DS operations */ - private static void deleteDataWithinTransaction(final LogicalDatastoreType dataStore, - final YangInstanceIdentifier path, + private static void deleteDataWithinTransaction(final YangInstanceIdentifier path, final RestconfStrategy strategy) { - LOG.trace("Delete {} within Restconf Patch: {}", dataStore.name(), path); - final FluentFuture future = strategy.exists(dataStore, path); - final FutureDataFactory response = new FutureDataFactory<>(); - - FutureCallbackTx.addCallback(future, PATCH_TX_TYPE, response); - - if (!response.result) { - LOG.trace("Operation via Restconf was not executed because data at {} does not exist", path); - throw new RestconfDocumentedException("Data does not exist", ErrorType.PROTOCOL, ErrorTag.DATA_MISSING, - path); - } - - strategy.delete(dataStore, path); + LOG.trace("Delete {} within Restconf Patch: {}", LogicalDatastoreType.CONFIGURATION.name(), path); + strategy.delete(LogicalDatastoreType.CONFIGURATION, path); } /** * Merge data within one transaction. * - * @param dataStore Datastore to merge data to - * @param path Path for data to be merged - * @param payload Data to be merged - * @param strategy Object that perform the actual DS operations - * @param schemaContext Global schema context + * @param path Path for data to be merged + * @param payload Data to be merged + * @param strategy Object that perform the actual DS operations */ - private static void mergeDataWithinTransaction(final LogicalDatastoreType dataStore, - final YangInstanceIdentifier path, + private static void mergeDataWithinTransaction(final YangInstanceIdentifier path, final NormalizedNode payload, - final RestconfStrategy strategy, - final EffectiveModelContext schemaContext) { - LOG.trace("Merge {} within Restconf Patch: {} with payload {}", dataStore.name(), path, payload); + final EffectiveModelContext schemaContext, + final RestconfStrategy strategy) { + LOG.trace("Merge {} within Restconf Patch: {} with payload {}", LogicalDatastoreType.CONFIGURATION.name(), + path, payload); TransactionUtil.ensureParentsByMerge(path, schemaContext, strategy); - strategy.merge(dataStore, path, payload); + strategy.merge(LogicalDatastoreType.CONFIGURATION, path, payload); } /** * Do NOT check if data exists and remove it within one transaction. * - * @param dataStore Datastore to delete data from - * @param path Path for data to be deleted - * @param strategy Object that perform the actual DS operations + * @param path Path for data to be deleted + * @param strategy Object that perform the actual DS operations */ - private static void removeDataWithinTransaction(final LogicalDatastoreType dataStore, - final YangInstanceIdentifier path, + private static void removeDataWithinTransaction(final YangInstanceIdentifier path, final RestconfStrategy strategy) { - LOG.trace("Remove {} within Restconf Patch: {}", dataStore.name(), path); - strategy.delete(dataStore, path); + LOG.trace("Remove {} within Restconf Patch: {}", LogicalDatastoreType.CONFIGURATION.name(), path); + strategy.remove(LogicalDatastoreType.CONFIGURATION, path); } /** * Create data within one transaction, replace if already exists. * - * @param dataStore Datastore to write data to * @param path Path for data to be created * @param payload Data to be created - * @param schemaContext Global schema context * @param strategy Object that perform the actual DS operations */ - private static void replaceDataWithinTransaction(final LogicalDatastoreType dataStore, - final YangInstanceIdentifier path, + private static void replaceDataWithinTransaction(final YangInstanceIdentifier path, final NormalizedNode payload, final EffectiveModelContext schemaContext, final RestconfStrategy strategy) { - LOG.trace("PUT {} within Restconf Patch: {} with payload {}", dataStore.name(), path, payload); - createData(payload, schemaContext, path, strategy, dataStore, false); + LOG.trace("PUT {} within Restconf Patch: {} with payload {}", + LogicalDatastoreType.CONFIGURATION.name(), path, payload); + createData(payload, path, strategy, schemaContext, false); } /** * Create data within one transaction. If {@code errorIfExists} is set to {@code true} then data will be checked * for existence before created, otherwise they will be overwritten. * - * @param payload Data to be created - * @param schemaContext Global schema context + * @param data Data to be created * @param path Path for data to be created * @param strategy Object that perform the actual DS operations - * @param dataStore Datastore to write data to * @param errorIfExists Enable checking for existence of data (throws error if already exists) */ - private static void createData(final NormalizedNode payload, final EffectiveModelContext schemaContext, + private static void createData(final NormalizedNode data, final YangInstanceIdentifier path, final RestconfStrategy strategy, - final LogicalDatastoreType dataStore, final boolean errorIfExists) { - if (payload instanceof MapNode) { - final NormalizedNode emptySubtree = ImmutableNodes.fromInstanceId(schemaContext, path); - strategy.merge(dataStore, YangInstanceIdentifier.create(emptySubtree.getIdentifier()), emptySubtree); - TransactionUtil.ensureParentsByMerge(path, schemaContext, strategy); - for (final MapEntryNode child : ((MapNode) payload).getValue()) { - final YangInstanceIdentifier childPath = path.node(child.getIdentifier()); - - if (errorIfExists) { - checkItemDoesNotExistsWithinTransaction(strategy, dataStore, childPath); - } - - if (errorIfExists) { - strategy.create(dataStore, childPath, child); - } else { - strategy.replace(dataStore, childPath, child); - } - } + final EffectiveModelContext schemaContext, + final boolean errorIfExists) { + if (errorIfExists) { + strategy.create(LogicalDatastoreType.CONFIGURATION, path, data, schemaContext); } else { - if (errorIfExists) { - checkItemDoesNotExistsWithinTransaction(strategy, dataStore, path); - } - - TransactionUtil.ensureParentsByMerge(path, schemaContext, strategy); - if (errorIfExists) { - strategy.create(dataStore, path, payload); - } else { - strategy.replace(dataStore, path, payload); - } - } - } - - /** - * Check if items do NOT already exists at specified {@code path}. Throws {@link RestconfDocumentedException} if - * data already exists. - * - * @param strategy Object that perform the actual DS operations - * @param store Datastore - * @param path Path to be checked - */ - public static void checkItemDoesNotExistsWithinTransaction(final RestconfStrategy strategy, - final LogicalDatastoreType store, - final YangInstanceIdentifier path) { - final FluentFuture future = strategy.exists(store, path); - final FutureDataFactory response = new FutureDataFactory<>(); - - FutureCallbackTx.addCallback(future, PATCH_TX_TYPE, response); - - if (response.result) { - LOG.trace("Operation via Restconf was not executed because data at {} already exists", path); - throw new RestconfDocumentedException("Data already exists", ErrorType.PROTOCOL, ErrorTag.DATA_EXISTS, - path); + strategy.replace(LogicalDatastoreType.CONFIGURATION, path, data, schemaContext); } } } diff --git a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/PostDataTransactionUtil.java b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/PostDataTransactionUtil.java index 55c4b7c46f..d5b39fd8a9 100644 --- a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/PostDataTransactionUtil.java +++ b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/PostDataTransactionUtil.java @@ -27,16 +27,12 @@ import org.opendaylight.restconf.nb.rfc8040.rests.transactions.RestconfStrategy; import org.opendaylight.restconf.nb.rfc8040.rests.utils.RestconfDataServiceConstant.PostPutQueryParameters.Insert; import org.opendaylight.restconf.nb.rfc8040.utils.parser.ParserIdentifier; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; -import org.opendaylight.yangtools.yang.data.api.schema.LeafSetEntryNode; import org.opendaylight.yangtools.yang.data.api.schema.MapEntryNode; import org.opendaylight.yangtools.yang.data.api.schema.MapNode; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; -import org.opendaylight.yangtools.yang.data.api.schema.OrderedLeafSetNode; -import org.opendaylight.yangtools.yang.data.api.schema.OrderedMapNode; +import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNodeContainer; import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes; -import org.opendaylight.yangtools.yang.model.api.DataSchemaNode; import org.opendaylight.yangtools.yang.model.api.EffectiveModelContext; -import org.opendaylight.yangtools.yang.model.api.ListSchemaNode; import org.opendaylight.yangtools.yang.model.api.SchemaContext; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -76,7 +72,7 @@ public final class PostDataTransactionUtil { final URI location = resolveLocation(uriInfo, path, schemaContext, payload.getData()); final ResponseFactory dataFactory = new ResponseFactory(Status.CREATED).location(location); //This method will close transactionChain if any - FutureCallbackTx.addCallback(future, POST_TX_TYPE, dataFactory, strategy.getTransactionChain()); + FutureCallbackTx.addCallback(future, POST_TX_TYPE, dataFactory, strategy.getTransactionChain(), path); return dataFactory.build(); } @@ -102,105 +98,62 @@ public final class PostDataTransactionUtil { return strategy.commit(); } - final DataSchemaNode schemaNode = PutDataTransactionUtil.checkListAndOrderedType(schemaContext, path); + PutDataTransactionUtil.checkListAndOrderedType(schemaContext, path); + final NormalizedNode readData; switch (insert) { case FIRST: - if (schemaNode instanceof ListSchemaNode) { - final NormalizedNode readData = PutDataTransactionUtil.readList(strategy, path.getParent()); - final OrderedMapNode readList = (OrderedMapNode) readData; - if (readList == null || readList.getValue().isEmpty()) { - makePost(path, data, schemaContext, strategy); - return strategy.commit(); - } - - strategy.delete(LogicalDatastoreType.CONFIGURATION, path.getParent().getParent()); - simplePost(LogicalDatastoreType.CONFIGURATION, path, data, schemaContext, strategy); - makePost(path, readData, schemaContext, strategy); - return strategy.commit(); - } else { - final NormalizedNode readData = PutDataTransactionUtil.readList(strategy, path.getParent()); - - final OrderedLeafSetNode readLeafList = (OrderedLeafSetNode) readData; - if (readLeafList == null || readLeafList.getValue().isEmpty()) { - makePost(path, data, schemaContext, strategy); - return strategy.commit(); - } - - strategy.delete(LogicalDatastoreType.CONFIGURATION, path.getParent().getParent()); - simplePost(LogicalDatastoreType.CONFIGURATION, path, data, schemaContext, strategy); - makePost(path, readData, schemaContext, strategy); + readData = PutDataTransactionUtil.readList(strategy, path.getParent().getParent()); + if (readData == null || ((NormalizedNodeContainer) readData).getValue().isEmpty()) { + strategy.replace(LogicalDatastoreType.CONFIGURATION, path, data, schemaContext); return strategy.commit(); } + checkItemDoesNotExists(strategy, LogicalDatastoreType.CONFIGURATION, path); + strategy.remove(LogicalDatastoreType.CONFIGURATION, path.getParent().getParent()); + strategy.replace(LogicalDatastoreType.CONFIGURATION, path, data, schemaContext); + strategy.replace(LogicalDatastoreType.CONFIGURATION, path.getParent().getParent(), readData, + schemaContext); + return strategy.commit(); case LAST: makePost(path, data, schemaContext, strategy); return strategy.commit(); case BEFORE: - if (schemaNode instanceof ListSchemaNode) { - final NormalizedNode readData = PutDataTransactionUtil.readList(strategy, path.getParent()); - final OrderedMapNode readList = (OrderedMapNode) readData; - if (readList == null || readList.getValue().isEmpty()) { - makePost(path, data, schemaContext, strategy); - return strategy.commit(); - } - - insertWithPointListPost(LogicalDatastoreType.CONFIGURATION, path, - data, schemaContext, point, readList, true, strategy); - return strategy.commit(); - } else { - final NormalizedNode readData = PutDataTransactionUtil.readList(strategy, path.getParent()); - - final OrderedLeafSetNode readLeafList = (OrderedLeafSetNode) readData; - if (readLeafList == null || readLeafList.getValue().isEmpty()) { - makePost(path, data, schemaContext, strategy); - return strategy.commit(); - } - - insertWithPointLeafListPost(LogicalDatastoreType.CONFIGURATION, - path, data, schemaContext, point, readLeafList, true, strategy); + readData = PutDataTransactionUtil.readList(strategy, path.getParent().getParent()); + if (readData == null || ((NormalizedNodeContainer) readData).getValue().isEmpty()) { + strategy.replace(LogicalDatastoreType.CONFIGURATION, path, data, schemaContext); return strategy.commit(); } + checkItemDoesNotExists(strategy, LogicalDatastoreType.CONFIGURATION, path); + insertWithPointPost(path, data, schemaContext, point, + (NormalizedNodeContainer>) readData, true, strategy); + return strategy.commit(); case AFTER: - if (schemaNode instanceof ListSchemaNode) { - final NormalizedNode readData = PutDataTransactionUtil.readList(strategy, path.getParent()); - final OrderedMapNode readList = (OrderedMapNode) readData; - if (readList == null || readList.getValue().isEmpty()) { - makePost(path, data, schemaContext, strategy); - return strategy.commit(); - } - - insertWithPointListPost(LogicalDatastoreType.CONFIGURATION, path, - data, schemaContext, point, readList, false, strategy); - return strategy.commit(); - } else { - final NormalizedNode readData = PutDataTransactionUtil.readList(strategy, path.getParent()); - final OrderedLeafSetNode readLeafList = (OrderedLeafSetNode) readData; - if (readLeafList == null || readLeafList.getValue().isEmpty()) { - makePost(path, data, schemaContext, strategy); - return strategy.commit(); - } - - insertWithPointLeafListPost(LogicalDatastoreType.CONFIGURATION, - path, data, schemaContext, point, readLeafList, true, strategy); + readData = PutDataTransactionUtil.readList(strategy, path.getParent().getParent()); + if (readData == null || ((NormalizedNodeContainer) readData).getValue().isEmpty()) { + strategy.replace(LogicalDatastoreType.CONFIGURATION, path, data, schemaContext); return strategy.commit(); } + checkItemDoesNotExists(strategy, LogicalDatastoreType.CONFIGURATION, path); + insertWithPointPost(path, data, schemaContext, point, + (NormalizedNodeContainer>) readData, false, strategy); + return strategy.commit(); default: throw new RestconfDocumentedException( "Used bad value of insert parameter. Possible values are first, last, before or after, but was: " - + insert, RestconfError.ErrorType.PROTOCOL, RestconfError.ErrorTag.BAD_ATTRIBUTE); + + insert, RestconfError.ErrorType.PROTOCOL, RestconfError.ErrorTag.BAD_ATTRIBUTE); } } - private static void insertWithPointLeafListPost(final LogicalDatastoreType datastore, - final YangInstanceIdentifier path, - final NormalizedNode payload, - final EffectiveModelContext schemaContext, final String point, - final OrderedLeafSetNode readLeafList, - final boolean before, final RestconfStrategy strategy) { - strategy.delete(datastore, path.getParent().getParent()); + private static void insertWithPointPost(final YangInstanceIdentifier path, + final NormalizedNode data, + final EffectiveModelContext schemaContext, final String point, + final NormalizedNodeContainer> readList, + final boolean before, final RestconfStrategy strategy) { + final YangInstanceIdentifier parent = path.getParent().getParent(); + strategy.remove(LogicalDatastoreType.CONFIGURATION, parent); final InstanceIdentifierContext instanceIdentifier = - ParserIdentifier.toInstanceIdentifier(point, schemaContext, Optional.empty()); + ParserIdentifier.toInstanceIdentifier(point, schemaContext, Optional.empty()); int lastItemPosition = 0; - for (final LeafSetEntryNode nodeChild : readLeafList.getValue()) { + for (final NormalizedNode nodeChild : readList.getValue()) { if (nodeChild.getIdentifier().equals(instanceIdentifier.getInstanceIdentifier().getLastPathArgument())) { break; } @@ -210,76 +163,27 @@ public final class PostDataTransactionUtil { lastItemPosition++; } int lastInsertedPosition = 0; - final NormalizedNode emptySubtree = - ImmutableNodes.fromInstanceId(schemaContext, path.getParent().getParent()); - strategy.merge(datastore, YangInstanceIdentifier.create(emptySubtree.getIdentifier()), emptySubtree); - for (final LeafSetEntryNode nodeChild : readLeafList.getValue()) { - if (lastInsertedPosition == lastItemPosition) { - checkItemDoesNotExists(strategy, datastore, path); - strategy.create(datastore, path, payload); - } - final YangInstanceIdentifier childPath = path.getParent().getParent().node(nodeChild.getIdentifier()); - checkItemDoesNotExists(strategy, datastore, childPath); - strategy.create(datastore, childPath, nodeChild); - lastInsertedPosition++; - } - } - - private static void insertWithPointListPost(final LogicalDatastoreType datastore, final YangInstanceIdentifier path, - final NormalizedNode payload, - final EffectiveModelContext schemaContext, final String point, - final MapNode readList, final boolean before, - final RestconfStrategy strategy) { - strategy.delete(datastore, path.getParent().getParent()); - final InstanceIdentifierContext instanceIdentifier = - ParserIdentifier.toInstanceIdentifier(point, schemaContext, Optional.empty()); - int lastItemPosition = 0; - for (final MapEntryNode mapEntryNode : readList.getValue()) { - if (mapEntryNode.getIdentifier().equals(instanceIdentifier.getInstanceIdentifier().getLastPathArgument())) { - break; - } - lastItemPosition++; - } - if (!before) { - lastItemPosition++; - } - int lastInsertedPosition = 0; - final NormalizedNode emptySubtree = - ImmutableNodes.fromInstanceId(schemaContext, path.getParent().getParent()); - strategy.merge(datastore, YangInstanceIdentifier.create(emptySubtree.getIdentifier()), emptySubtree); - for (final MapEntryNode mapEntryNode : readList.getValue()) { + final NormalizedNode emptySubtree = ImmutableNodes.fromInstanceId(schemaContext, parent); + strategy.merge(LogicalDatastoreType.CONFIGURATION, + YangInstanceIdentifier.create(emptySubtree.getIdentifier()), emptySubtree); + for (final NormalizedNode nodeChild : readList.getValue()) { if (lastInsertedPosition == lastItemPosition) { - checkItemDoesNotExists(strategy, datastore, path); - strategy.create(datastore, path, payload); + strategy.replace(LogicalDatastoreType.CONFIGURATION, path, data, schemaContext); } - final YangInstanceIdentifier childPath = path.getParent().getParent().node(mapEntryNode.getIdentifier()); - checkItemDoesNotExists(strategy, datastore, childPath); - strategy.create(datastore, childPath, mapEntryNode); + final YangInstanceIdentifier childPath = parent.node(nodeChild.getIdentifier()); + strategy.replace(LogicalDatastoreType.CONFIGURATION, childPath, nodeChild, schemaContext); lastInsertedPosition++; } } private static void makePost(final YangInstanceIdentifier path, final NormalizedNode data, final SchemaContext schemaContext, final RestconfStrategy strategy) { - if (data instanceof MapNode) { - boolean merge = false; - for (final MapEntryNode child : ((MapNode) data).getValue()) { - final YangInstanceIdentifier childPath = path.node(child.getIdentifier()); - checkItemDoesNotExists(strategy, LogicalDatastoreType.CONFIGURATION, childPath); - if (!merge) { - merge = true; - TransactionUtil.ensureParentsByMerge(path, schemaContext, strategy); - final NormalizedNode emptySubTree = ImmutableNodes.fromInstanceId(schemaContext, path); - strategy.merge(LogicalDatastoreType.CONFIGURATION, - YangInstanceIdentifier.create(emptySubTree.getIdentifier()), emptySubTree); - } - strategy.create(LogicalDatastoreType.CONFIGURATION, childPath, child); - } - } else { - checkItemDoesNotExists(strategy, LogicalDatastoreType.CONFIGURATION, path); - - TransactionUtil.ensureParentsByMerge(path, schemaContext, strategy); - strategy.create(LogicalDatastoreType.CONFIGURATION, path, data); + try { + strategy.create(LogicalDatastoreType.CONFIGURATION, path, data, schemaContext); + } catch (RestconfDocumentedException e) { + // close transaction if any and pass exception further + strategy.cancel(); + throw e; } } @@ -311,15 +215,6 @@ public final class PostDataTransactionUtil { .build(); } - private static void simplePost(final LogicalDatastoreType datastore, final YangInstanceIdentifier path, - final NormalizedNode payload, - final SchemaContext schemaContext, final RestconfStrategy strategy) { - checkItemDoesNotExists(strategy, datastore, path); - TransactionUtil.ensureParentsByMerge(path, schemaContext, strategy); - strategy.create(datastore, path, payload); - } - - /** * Check if items do NOT already exists at specified {@code path}. Throws {@link RestconfDocumentedException} if * data already exists. @@ -327,9 +222,8 @@ public final class PostDataTransactionUtil { * @param strategy Object that perform the actual DS operations * @param store Datastore * @param path Path to be checked - * @param operationType Type of operation (READ, POST, PUT, DELETE...) */ - private static void checkItemDoesNotExists(final RestconfStrategy strategy, + public static void checkItemDoesNotExists(final RestconfStrategy strategy, final LogicalDatastoreType store, final YangInstanceIdentifier path) { final FluentFuture future = strategy.exists(store, path); final FutureDataFactory response = new FutureDataFactory<>(); @@ -337,13 +231,9 @@ public final class PostDataTransactionUtil { FutureCallbackTx.addCallback(future, POST_TX_TYPE, response); if (response.result) { - // close transaction - strategy.cancel(); - // throw error LOG.trace("Operation via Restconf was not executed because data at {} already exists", path); throw new RestconfDocumentedException( - "Data already exists", ErrorType.PROTOCOL, ErrorTag.DATA_EXISTS, path); + "Data already exists", ErrorType.PROTOCOL, ErrorTag.DATA_EXISTS, path); } } - } diff --git a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/PutDataTransactionUtil.java b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/PutDataTransactionUtil.java index 3ec3f487a5..0d42f064ba 100644 --- a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/PutDataTransactionUtil.java +++ b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/PutDataTransactionUtil.java @@ -30,12 +30,9 @@ import org.opendaylight.yangtools.yang.common.QName; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifierWithPredicates; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument; -import org.opendaylight.yangtools.yang.data.api.schema.LeafSetEntryNode; -import org.opendaylight.yangtools.yang.data.api.schema.LeafSetNode; import org.opendaylight.yangtools.yang.data.api.schema.MapEntryNode; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; -import org.opendaylight.yangtools.yang.data.api.schema.OrderedLeafSetNode; -import org.opendaylight.yangtools.yang.data.api.schema.OrderedMapNode; +import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNodeContainer; import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes; import org.opendaylight.yangtools.yang.data.util.DataSchemaContextNode; import org.opendaylight.yangtools.yang.data.util.DataSchemaContextTree; @@ -150,16 +147,15 @@ public final class PutDataTransactionUtil { strategy.prepareReadWriteExecution(); final FluentFuture existsFuture = strategy.exists(LogicalDatastoreType.CONFIGURATION, path); - final FutureDataFactory existsResponse = new FutureDataFactory<>(); - FutureCallbackTx.addCallback(existsFuture, PUT_TX_TYPE, existsResponse); - - final ResponseFactory responseFactory = - new ResponseFactory(existsResponse.result ? Status.NO_CONTENT : Status.CREATED); final FluentFuture submitData = submitData(path, schemaContext, strategy, - payload.getData(), insert, point, existsResponse.result); + payload.getData(), insert, point); + final ResponseFactory response = new ResponseFactory(); //This method will close transactionChain if any - FutureCallbackTx.addCallback(submitData, PUT_TX_TYPE, responseFactory, strategy.getTransactionChain()); - return responseFactory.build(); + FutureCallbackTx.addCallback(submitData, PUT_TX_TYPE, response, strategy.getTransactionChain()); + + final FutureDataFactory isExists = new FutureDataFactory<>(); + FutureCallbackTx.addCallback(existsFuture, PUT_TX_TYPE, isExists); + return response.status(isExists.result ? Status.NO_CONTENT : Status.CREATED).build(); } /** @@ -173,93 +169,45 @@ public final class PutDataTransactionUtil { * @param insert query parameter * @return {@link FluentFuture} */ - private static FluentFuture submitData( - final YangInstanceIdentifier path, - final EffectiveModelContext schemaContext, - final RestconfStrategy strategy, - final NormalizedNode data, final Insert insert, final String point, final boolean exists) { + private static FluentFuture submitData(final YangInstanceIdentifier path, + final EffectiveModelContext schemaContext, + final RestconfStrategy strategy, + final NormalizedNode data, + final Insert insert, final String point) { if (insert == null) { - return makePut(path, schemaContext, strategy, data, exists); + return makePut(path, schemaContext, strategy, data); } - final DataSchemaNode schemaNode = checkListAndOrderedType(schemaContext, path); + checkListAndOrderedType(schemaContext, path); + final NormalizedNode readData; switch (insert) { case FIRST: - if (schemaNode instanceof ListSchemaNode) { - final NormalizedNode readData = readList(strategy, path); - final OrderedMapNode readList = (OrderedMapNode) readData; - if (readList == null || readList.getValue().isEmpty()) { - return makePut(path, schemaContext, strategy, data, exists); - } else { - strategy.delete(LogicalDatastoreType.CONFIGURATION, path.getParent()); - simplePut(LogicalDatastoreType.CONFIGURATION, path, strategy, schemaContext, data, exists); - listPut(LogicalDatastoreType.CONFIGURATION, path.getParent(), strategy, - schemaContext, readList, exists); - return strategy.commit(); - } - } else { - final NormalizedNode readData = readList(strategy, path); - - final OrderedLeafSetNode readLeafList = (OrderedLeafSetNode) readData; - if (readLeafList == null || readLeafList.getValue().isEmpty()) { - return makePut(path, schemaContext, strategy, data, exists); - } else { - strategy.delete(LogicalDatastoreType.CONFIGURATION, path.getParent()); - simplePut(LogicalDatastoreType.CONFIGURATION, path, strategy, - schemaContext, data, exists); - listPut(LogicalDatastoreType.CONFIGURATION, path.getParent(), strategy, - schemaContext, readLeafList, exists); - return strategy.commit(); - } + readData = readList(strategy, path.getParent()); + if (readData == null || ((NormalizedNodeContainer) readData).getValue().isEmpty()) { + return makePut(path, schemaContext, strategy, data); } + strategy.remove(LogicalDatastoreType.CONFIGURATION, path.getParent()); + strategy.replace(LogicalDatastoreType.CONFIGURATION, path, data, schemaContext); + strategy.replace(LogicalDatastoreType.CONFIGURATION, path.getParent(), readData, schemaContext); + return strategy.commit(); case LAST: - return makePut(path, schemaContext, strategy, data, exists); + return makePut(path, schemaContext, strategy, data); case BEFORE: - if (schemaNode instanceof ListSchemaNode) { - final NormalizedNode readData = readList(strategy, path); - final OrderedMapNode readList = (OrderedMapNode) readData; - if (readList == null || readList.getValue().isEmpty()) { - return makePut(path, schemaContext, strategy, data, exists); - } else { - insertWithPointListPut(strategy, LogicalDatastoreType.CONFIGURATION, path, - data, schemaContext, point, readList, true, exists); - return strategy.commit(); - } - } else { - final NormalizedNode readData = readList(strategy, path); - - final OrderedLeafSetNode readLeafList = (OrderedLeafSetNode) readData; - if (readLeafList == null || readLeafList.getValue().isEmpty()) { - return makePut(path, schemaContext, strategy, data, exists); - } else { - insertWithPointLeafListPut(strategy, LogicalDatastoreType.CONFIGURATION, - path, data, schemaContext, point, readLeafList, true, exists); - return strategy.commit(); - } + readData = readList(strategy, path.getParent()); + if (readData == null || ((NormalizedNodeContainer) readData).getValue().isEmpty()) { + return makePut(path, schemaContext, strategy, data); } + insertWithPointPut(strategy, path, data, schemaContext, point, + (NormalizedNodeContainer>) readData, true); + return strategy.commit(); case AFTER: - if (schemaNode instanceof ListSchemaNode) { - final NormalizedNode readData = readList(strategy, path); - final OrderedMapNode readList = (OrderedMapNode) readData; - if (readList == null || readList.getValue().isEmpty()) { - return makePut(path, schemaContext, strategy, data, exists); - } else { - insertWithPointListPut(strategy, LogicalDatastoreType.CONFIGURATION, - path, data, schemaContext, point, readList, false, exists); - return strategy.commit(); - } - } else { - final NormalizedNode readData = readList(strategy, path); - - final OrderedLeafSetNode readLeafList = (OrderedLeafSetNode) readData; - if (readLeafList == null || readLeafList.getValue().isEmpty()) { - return makePut(path, schemaContext, strategy, data, exists); - } else { - insertWithPointLeafListPut(strategy, LogicalDatastoreType.CONFIGURATION, - path, data, schemaContext, point, readLeafList, true, exists); - return strategy.commit(); - } + readData = readList(strategy, path.getParent()); + if (readData == null || ((NormalizedNodeContainer) readData).getValue().isEmpty()) { + return makePut(path, schemaContext, strategy, data); } + insertWithPointPut(strategy, path, data, schemaContext, point, + (NormalizedNodeContainer>) readData, false); + return strategy.commit(); default: throw new RestconfDocumentedException( "Used bad value of insert parameter. Possible values are first, last, before or after, " @@ -270,21 +218,21 @@ public final class PutDataTransactionUtil { // FIXME: this method is only called from a context where we are modifying data. This should be part of strategy, // requiring an already-open transaction. It also must return a future, so it can be properly composed. static NormalizedNode readList(final RestconfStrategy strategy, final YangInstanceIdentifier path) { - return ReadDataTransactionUtil.readDataViaTransaction(strategy, LogicalDatastoreType.CONFIGURATION, path, true); + return ReadDataTransactionUtil.readDataViaTransaction(strategy, LogicalDatastoreType.CONFIGURATION, + path,false); } - private static void insertWithPointLeafListPut(final RestconfStrategy strategy, - final LogicalDatastoreType datastore, - final YangInstanceIdentifier path, - final NormalizedNode data, - final EffectiveModelContext schemaContext, final String point, - final OrderedLeafSetNode readLeafList, final boolean before, - final boolean exists) { - strategy.delete(datastore, path.getParent()); + private static void insertWithPointPut(final RestconfStrategy strategy, + final YangInstanceIdentifier path, + final NormalizedNode data, + final EffectiveModelContext schemaContext, final String point, + final NormalizedNodeContainer> readList, + final boolean before) { + strategy.remove(LogicalDatastoreType.CONFIGURATION, path.getParent()); final InstanceIdentifierContext instanceIdentifier = - ParserIdentifier.toInstanceIdentifier(point, schemaContext, Optional.empty()); + ParserIdentifier.toInstanceIdentifier(point, schemaContext, Optional.empty()); int lastItemPosition = 0; - for (final LeafSetEntryNode nodeChild : readLeafList.getValue()) { + for (final NormalizedNode nodeChild : readList.getValue()) { if (nodeChild.getIdentifier().equals(instanceIdentifier.getInstanceIdentifier().getLastPathArgument())) { break; } @@ -295,111 +243,23 @@ public final class PutDataTransactionUtil { } int lastInsertedPosition = 0; final NormalizedNode emptySubtree = ImmutableNodes.fromInstanceId(schemaContext, path.getParent()); - strategy.merge(datastore, YangInstanceIdentifier.create(emptySubtree.getIdentifier()), emptySubtree); - for (final LeafSetEntryNode nodeChild : readLeafList.getValue()) { + strategy.merge(LogicalDatastoreType.CONFIGURATION, YangInstanceIdentifier.create(emptySubtree.getIdentifier()), + emptySubtree); + for (final NormalizedNode nodeChild : readList.getValue()) { if (lastInsertedPosition == lastItemPosition) { - simplePut(datastore, path, strategy, schemaContext, data, exists); + strategy.replace(LogicalDatastoreType.CONFIGURATION, path, data, schemaContext); } final YangInstanceIdentifier childPath = path.getParent().node(nodeChild.getIdentifier()); - if (exists) { - strategy.replace(datastore, childPath, nodeChild); - } else { - strategy.create(datastore, childPath, nodeChild); - } - lastInsertedPosition++; - } - } - - private static void insertWithPointListPut(final RestconfStrategy strategy, - final LogicalDatastoreType datastore, final YangInstanceIdentifier path, - final NormalizedNode data, - final EffectiveModelContext schemaContext, final String point, - final OrderedMapNode readList, final boolean before, - final boolean exists) { - strategy.delete(datastore, path.getParent()); - final InstanceIdentifierContext instanceIdentifier = - ParserIdentifier.toInstanceIdentifier(point, schemaContext, Optional.empty()); - int lastItemPosition = 0; - for (final MapEntryNode mapEntryNode : readList.getValue()) { - if (mapEntryNode.getIdentifier().equals(instanceIdentifier.getInstanceIdentifier().getLastPathArgument())) { - break; - } - lastItemPosition++; - } - if (!before) { - lastItemPosition++; - } - int lastInsertedPosition = 0; - final NormalizedNode emptySubtree = ImmutableNodes.fromInstanceId(schemaContext, path.getParent()); - strategy.merge(datastore, YangInstanceIdentifier.create(emptySubtree.getIdentifier()), emptySubtree); - for (final MapEntryNode mapEntryNode : readList.getValue()) { - if (lastInsertedPosition == lastItemPosition) { - simplePut(datastore, path, strategy, schemaContext, data, exists); - } - final YangInstanceIdentifier childPath = path.getParent().node(mapEntryNode.getIdentifier()); - if (exists) { - strategy.replace(datastore, childPath, mapEntryNode); - } else { - strategy.create(datastore, childPath, mapEntryNode); - } + strategy.replace(LogicalDatastoreType.CONFIGURATION, childPath, nodeChild, schemaContext); lastInsertedPosition++; } } - private static void listPut(final LogicalDatastoreType datastore, final YangInstanceIdentifier path, - final RestconfStrategy strategy, final SchemaContext schemaContext, - final OrderedLeafSetNode payload, final boolean exists) { - final NormalizedNode emptySubtree = ImmutableNodes.fromInstanceId(schemaContext, path); - strategy.merge(datastore, YangInstanceIdentifier.create(emptySubtree.getIdentifier()), emptySubtree); - TransactionUtil.ensureParentsByMerge(path, schemaContext, strategy); - for (final LeafSetEntryNode child : ((LeafSetNode) payload).getValue()) { - final YangInstanceIdentifier childPath = path.node(child.getIdentifier()); - if (exists) { - strategy.replace(datastore, childPath, child); - } else { - strategy.create(datastore, childPath, child); - } - } - } - - private static void listPut(final LogicalDatastoreType datastore, final YangInstanceIdentifier path, - final RestconfStrategy strategy, final SchemaContext schemaContext, - final OrderedMapNode payload, final boolean exists) { - final NormalizedNode emptySubtree = ImmutableNodes.fromInstanceId(schemaContext, path); - strategy.merge(datastore, YangInstanceIdentifier.create(emptySubtree.getIdentifier()), emptySubtree); - TransactionUtil.ensureParentsByMerge(path, schemaContext, strategy); - for (final MapEntryNode child : payload.getValue()) { - final YangInstanceIdentifier childPath = path.node(child.getIdentifier()); - if (exists) { - strategy.replace(datastore, childPath, child); - } else { - strategy.create(datastore, childPath, child); - } - } - } - - private static void simplePut(final LogicalDatastoreType configuration, final YangInstanceIdentifier path, - final RestconfStrategy strategy, final SchemaContext schemaContext, - final NormalizedNode data, final boolean exists) { - TransactionUtil.ensureParentsByMerge(path, schemaContext, strategy); - if (exists) { - strategy.replace(LogicalDatastoreType.CONFIGURATION, path, data); - } else { - strategy.create(LogicalDatastoreType.CONFIGURATION, path, data); - } - } - private static FluentFuture makePut(final YangInstanceIdentifier path, final SchemaContext schemaContext, final RestconfStrategy strategy, - final NormalizedNode data, - final boolean exists) { - TransactionUtil.ensureParentsByMerge(path, schemaContext, strategy); - if (exists) { - strategy.replace(LogicalDatastoreType.CONFIGURATION, path, data); - } else { - strategy.create(LogicalDatastoreType.CONFIGURATION, path, data); - } + final NormalizedNode data) { + strategy.replace(LogicalDatastoreType.CONFIGURATION, path, data, schemaContext); return strategy.commit(); } diff --git a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/ResponseFactory.java b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/ResponseFactory.java index b8c60f571e..da153a4780 100644 --- a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/ResponseFactory.java +++ b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/ResponseFactory.java @@ -18,10 +18,18 @@ final class ResponseFactory extends FutureDataFactory implements Bui private ResponseBuilder responseBuilder; + ResponseFactory() { + } + ResponseFactory(final Status status) { this.responseBuilder = Response.status(status); } + ResponseFactory status(final Status status) { + responseBuilder = Response.status(status); + return this; + } + ResponseFactory location(final URI location) { responseBuilder.location(location); return this; diff --git a/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/DeleteDataTransactionUtilTest.java b/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/DeleteDataTransactionUtilTest.java index 72eb655e53..156100ad25 100644 --- a/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/DeleteDataTransactionUtilTest.java +++ b/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/DeleteDataTransactionUtilTest.java @@ -9,12 +9,11 @@ package org.opendaylight.restconf.nb.rfc8040.rests.utils; import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; -import static org.mockito.Mockito.mock; +import static org.mockito.ArgumentMatchers.any; import static org.opendaylight.yangtools.util.concurrent.FluentFutures.immediateFalseFluentFuture; -import static org.opendaylight.yangtools.util.concurrent.FluentFutures.immediateFluentFuture; import static org.opendaylight.yangtools.util.concurrent.FluentFutures.immediateTrueFluentFuture; -import java.util.Optional; +import com.google.common.util.concurrent.SettableFuture; import javax.ws.rs.core.Response; import javax.ws.rs.core.Response.Status; import org.junit.Before; @@ -25,9 +24,12 @@ import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; import org.opendaylight.mdsal.common.api.CommitInfo; import org.opendaylight.mdsal.common.api.LogicalDatastoreType; +import org.opendaylight.mdsal.common.api.TransactionCommitFailedException; import org.opendaylight.mdsal.dom.api.DOMDataBroker; import org.opendaylight.mdsal.dom.api.DOMDataTreeReadWriteTransaction; import org.opendaylight.mdsal.dom.api.DOMTransactionChain; +import org.opendaylight.netconf.api.DocumentedException; +import org.opendaylight.netconf.api.NetconfDocumentedException; import org.opendaylight.netconf.dom.api.NetconfDataTreeService; import org.opendaylight.restconf.common.context.InstanceIdentifierContext; import org.opendaylight.restconf.common.errors.RestconfDocumentedException; @@ -38,7 +40,6 @@ import org.opendaylight.restconf.nb.rfc8040.rests.transactions.MdsalRestconfStra import org.opendaylight.restconf.nb.rfc8040.rests.transactions.NetconfRestconfStrategy; import org.opendaylight.restconf.nb.rfc8040.rests.transactions.RestconfStrategy; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; -import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; @RunWith(MockitoJUnitRunner.StrictStubs.class) public class DeleteDataTransactionUtilTest { @@ -74,8 +75,6 @@ public class DeleteDataTransactionUtilTest { // assert that data to delete exists Mockito.when(this.transactionChain.newReadWriteTransaction().exists(LogicalDatastoreType.CONFIGURATION, YangInstanceIdentifier.empty())).thenReturn(immediateTrueFluentFuture()); - Mockito.when(this.netconfService.getConfig(YangInstanceIdentifier.empty())) - .thenReturn(immediateFluentFuture(Optional.of(mock(NormalizedNode.class)))); // test delete(new MdsalRestconfStrategy(transactionChainHandler)); delete(new NetconfRestconfStrategy(netconfService)); @@ -89,8 +88,15 @@ public class DeleteDataTransactionUtilTest { // assert that data to delete does NOT exist Mockito.when(this.transactionChain.newReadWriteTransaction().exists(LogicalDatastoreType.CONFIGURATION, YangInstanceIdentifier.empty())).thenReturn(immediateFalseFluentFuture()); - Mockito.when(this.netconfService.getConfig(YangInstanceIdentifier.empty())) - .thenReturn(immediateFluentFuture(Optional.empty())); + final NetconfDocumentedException exception = new NetconfDocumentedException("id", + DocumentedException.ErrorType.RPC, DocumentedException.ErrorTag.from("data-missing"), + DocumentedException.ErrorSeverity.ERROR); + final SettableFuture ret = SettableFuture.create(); + ret.setException(new TransactionCommitFailedException( + String.format("Commit of transaction %s failed", this), exception)); + + Mockito.when(this.netconfService.commit(any())).thenAnswer(invocation -> ret); + // test and assert error deleteFail(new MdsalRestconfStrategy(transactionChainHandler)); deleteFail(new NetconfRestconfStrategy(netconfService)); diff --git a/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/PatchDataTransactionUtilTest.java b/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/PatchDataTransactionUtilTest.java index cb48c80ad5..70851175af 100644 --- a/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/PatchDataTransactionUtilTest.java +++ b/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/PatchDataTransactionUtilTest.java @@ -12,19 +12,17 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doReturn; -import static org.mockito.Mockito.mock; import static org.opendaylight.restconf.common.patch.PatchEditOperation.CREATE; import static org.opendaylight.restconf.common.patch.PatchEditOperation.DELETE; import static org.opendaylight.restconf.common.patch.PatchEditOperation.MERGE; import static org.opendaylight.restconf.common.patch.PatchEditOperation.REMOVE; import static org.opendaylight.restconf.common.patch.PatchEditOperation.REPLACE; import static org.opendaylight.yangtools.util.concurrent.FluentFutures.immediateFalseFluentFuture; -import static org.opendaylight.yangtools.util.concurrent.FluentFutures.immediateFluentFuture; import static org.opendaylight.yangtools.util.concurrent.FluentFutures.immediateTrueFluentFuture; +import com.google.common.util.concurrent.SettableFuture; import java.util.ArrayList; import java.util.List; -import java.util.Optional; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -33,9 +31,12 @@ import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; import org.opendaylight.mdsal.common.api.CommitInfo; import org.opendaylight.mdsal.common.api.LogicalDatastoreType; +import org.opendaylight.mdsal.common.api.TransactionCommitFailedException; import org.opendaylight.mdsal.dom.api.DOMDataBroker; import org.opendaylight.mdsal.dom.api.DOMDataTreeReadWriteTransaction; import org.opendaylight.mdsal.dom.api.DOMTransactionChain; +import org.opendaylight.netconf.api.DocumentedException; +import org.opendaylight.netconf.api.NetconfDocumentedException; import org.opendaylight.netconf.dom.api.NetconfDataTreeService; import org.opendaylight.restconf.common.context.InstanceIdentifierContext; import org.opendaylight.restconf.common.errors.RestconfError; @@ -56,7 +57,6 @@ import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode; import org.opendaylight.yangtools.yang.data.api.schema.LeafNode; import org.opendaylight.yangtools.yang.data.api.schema.MapEntryNode; import org.opendaylight.yangtools.yang.data.api.schema.MapNode; -import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; import org.opendaylight.yangtools.yang.data.impl.schema.Builders; import org.opendaylight.yangtools.yang.model.api.EffectiveModelContext; import org.opendaylight.yangtools.yang.model.api.SchemaNode; @@ -198,12 +198,8 @@ public class PatchDataTransactionUtilTest { public void testPatchDataCreateAndDelete() { doReturn(immediateFalseFluentFuture()).when(this.rwTransaction).exists(LogicalDatastoreType.CONFIGURATION, this.instanceIdContainer); - Mockito.when(this.netconfService.getConfig(this.instanceIdContainer)) - .thenReturn(immediateFluentFuture(Optional.empty())); doReturn(immediateTrueFluentFuture()).when(this.rwTransaction).exists(LogicalDatastoreType.CONFIGURATION, this.targetNodeForCreateAndDelete); - Mockito.when(this.netconfService.getConfig(this.targetNodeForCreateAndDelete)) - .thenReturn(immediateFluentFuture(Optional.of(mock(NormalizedNode.class)))); final PatchEntity entityCreate = new PatchEntity("edit1", CREATE, this.instanceIdContainer, this.buildBaseContainerForTests); @@ -225,8 +221,14 @@ public class PatchDataTransactionUtilTest { public void deleteNonexistentDataTest() { doReturn(immediateFalseFluentFuture()).when(this.rwTransaction).exists(LogicalDatastoreType.CONFIGURATION, this.targetNodeForCreateAndDelete); - Mockito.when(this.netconfService.getConfig(this.targetNodeForCreateAndDelete)) - .thenReturn(immediateFluentFuture(Optional.empty())); + final NetconfDocumentedException exception = new NetconfDocumentedException("id", + DocumentedException.ErrorType.RPC, DocumentedException.ErrorTag.from("data-missing"), + DocumentedException.ErrorSeverity.ERROR); + final SettableFuture ret = SettableFuture.create(); + ret.setException(new TransactionCommitFailedException( + String.format("Commit of transaction %s failed", this), exception)); + + Mockito.when(this.netconfService.commit(any())).thenAnswer(invocation -> ret); final PatchEntity entityDelete = new PatchEntity("edit", DELETE, this.targetNodeForCreateAndDelete); final List entities = new ArrayList<>(); @@ -236,8 +238,8 @@ public class PatchDataTransactionUtilTest { final InstanceIdentifierContext iidContext = new InstanceIdentifierContext<>(this.instanceIdCreateAndDelete, null, null, this.refSchemaCtx); final PatchContext patchContext = new PatchContext(iidContext, entities, "patchD"); - delete(patchContext, new MdsalRestconfStrategy(transactionChainHandler)); - delete(patchContext, new NetconfRestconfStrategy(netconfService)); + deleteMdsal(patchContext, new MdsalRestconfStrategy(transactionChainHandler)); + deleteNetconf(patchContext, new NetconfRestconfStrategy(netconfService)); } @Test @@ -269,7 +271,7 @@ public class PatchDataTransactionUtilTest { assertTrue(patchStatusContext.isOk()); } - private void delete(final PatchContext patchContext, final RestconfStrategy strategy) { + private void deleteMdsal(final PatchContext patchContext, final RestconfStrategy strategy) { final PatchStatusContext patchStatusContext = PatchDataTransactionUtil.patchData(patchContext, strategy, this.refSchemaCtx); @@ -279,4 +281,15 @@ public class PatchDataTransactionUtilTest { assertEquals(RestconfError.ErrorTag.DATA_MISSING, patchStatusContext.getEditCollection().get(0).getEditErrors().get(0).getErrorTag()); } + + private void deleteNetconf(PatchContext patchContext, RestconfStrategy strategy) { + final PatchStatusContext patchStatusContext = + PatchDataTransactionUtil.patchData(patchContext, strategy, this.refSchemaCtx); + + assertFalse(patchStatusContext.isOk()); + assertEquals(RestconfError.ErrorType.PROTOCOL, + patchStatusContext.getGlobalErrors().get(0).getErrorType()); + assertEquals(RestconfError.ErrorTag.DATA_MISSING, + patchStatusContext.getGlobalErrors().get(0).getErrorTag()); + } } diff --git a/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/PostDataTransactionUtilTest.java b/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/PostDataTransactionUtilTest.java index bd0c138ac7..ee4d92a475 100644 --- a/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/PostDataTransactionUtilTest.java +++ b/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/PostDataTransactionUtilTest.java @@ -17,7 +17,6 @@ import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.verify; import static org.opendaylight.yangtools.util.concurrent.FluentFutures.immediateFailedFluentFuture; import static org.opendaylight.yangtools.util.concurrent.FluentFutures.immediateFalseFluentFuture; -import static org.opendaylight.yangtools.util.concurrent.FluentFutures.immediateFluentFuture; import java.io.UnsupportedEncodingException; import java.net.URLDecoder; @@ -152,7 +151,6 @@ public class PostDataTransactionUtilTest { doReturn(immediateFalseFluentFuture()).when(this.readWrite).exists(LogicalDatastoreType.CONFIGURATION, this.iid2); - doReturn(immediateFluentFuture(Optional.empty())).when(this.netconfService).getConfig(this.iid2); final NodeIdentifier identifier = ((ContainerNode) ((Collection) payload.getData().getValue()).iterator().next()).getIdentifier(); final YangInstanceIdentifier node = @@ -171,7 +169,6 @@ public class PostDataTransactionUtilTest { response = PostDataTransactionUtil.postData(this.uriInfo, payload, new NetconfRestconfStrategy(netconfService), this.schema, null, null); assertEquals(201, response.getStatus()); - verify(this.netconfService).getConfig(this.iid2); verify(this.netconfService).create(LogicalDatastoreType.CONFIGURATION, payload.getInstanceIdentifierContext().getInstanceIdentifier(), payload.getData(), Optional.empty()); } @@ -188,7 +185,6 @@ public class PostDataTransactionUtilTest { final YangInstanceIdentifier node = payload.getInstanceIdentifierContext().getInstanceIdentifier().node(identifier); doReturn(immediateFalseFluentFuture()).when(this.readWrite).exists(LogicalDatastoreType.CONFIGURATION, node); - doReturn(immediateFluentFuture(Optional.empty())).when(this.netconfService).getConfig(node); doNothing().when(this.readWrite).put(LogicalDatastoreType.CONFIGURATION, node, entryNode); doReturn(CommitInfo.emptyFluentFuture()).when(this.readWrite).commit(); doReturn(CommitInfo.emptyFluentFuture()).when(this.netconfService).commit(Mockito.any()); @@ -206,7 +202,6 @@ public class PostDataTransactionUtilTest { assertEquals(201, response.getStatus()); assertThat(URLDecoder.decode(response.getLocation().toString(), StandardCharsets.UTF_8), containsString(identifier.getValue(identifier.keySet().iterator().next()).toString())); - verify(this.netconfService).getConfig(node); verify(this.netconfService).create(LogicalDatastoreType.CONFIGURATION, node, entryNode, Optional.empty()); } @@ -219,7 +214,6 @@ public class PostDataTransactionUtilTest { doReturn(immediateFalseFluentFuture()).when(this.readWrite).exists(LogicalDatastoreType.CONFIGURATION, this.iid2); - doReturn(immediateFluentFuture(Optional.empty())).when(this.netconfService).getConfig(this.iid2); final NodeIdentifier identifier = ((ContainerNode) ((Collection) payload.getData().getValue()).iterator().next()).getIdentifier(); final YangInstanceIdentifier node = @@ -251,7 +245,6 @@ public class PostDataTransactionUtilTest { assertTrue(e.getErrors().get(0).getErrorInfo().contains(domException.getMessage())); } - verify(this.netconfService).getConfig(this.iid2); verify(this.netconfService).create(LogicalDatastoreType.CONFIGURATION, payload.getInstanceIdentifierContext().getInstanceIdentifier(), payload.getData(), Optional.empty()); } diff --git a/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/PutDataTransactionUtilTest.java b/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/PutDataTransactionUtilTest.java index 7b436632be..6c95fff1ab 100644 --- a/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/PutDataTransactionUtilTest.java +++ b/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/PutDataTransactionUtilTest.java @@ -245,7 +245,7 @@ public class PutDataTransactionUtilTest { PutDataTransactionUtil.putData(payload, this.schema, new NetconfRestconfStrategy(netconfService), null, null); verify(this.netconfService).getConfig(payload.getInstanceIdentifierContext().getInstanceIdentifier()); - verify(this.netconfService).create(LogicalDatastoreType.CONFIGURATION, + verify(this.netconfService).replace(LogicalDatastoreType.CONFIGURATION, payload.getInstanceIdentifierContext().getInstanceIdentifier(), payload.getData(), Optional.empty()); } @@ -299,7 +299,7 @@ public class PutDataTransactionUtilTest { PutDataTransactionUtil.putData(payload, this.schema, new NetconfRestconfStrategy(netconfService), null, null); verify(this.netconfService).getConfig(payload.getInstanceIdentifierContext().getInstanceIdentifier()); - verify(this.netconfService).create(LogicalDatastoreType.CONFIGURATION, + verify(this.netconfService).replace(LogicalDatastoreType.CONFIGURATION, payload.getInstanceIdentifierContext().getInstanceIdentifier(), payload.getData(), Optional.empty()); } @@ -351,7 +351,7 @@ public class PutDataTransactionUtilTest { PutDataTransactionUtil.putData(payload, this.schema, new NetconfRestconfStrategy(netconfService), null, null); verify(this.netconfService).getConfig(this.iid2); - verify(this.netconfService).create(LogicalDatastoreType.CONFIGURATION, this.iid2, + verify(this.netconfService).replace(LogicalDatastoreType.CONFIGURATION, this.iid2, payload.getData(), Optional.empty()); } -- 2.36.6