Fix error reporting for PUT/POST

If a datastore failure occurs for PUT or POST, restconf reports the
error message "Problem while PUT operations" which is neither useful
nor grammatically correct.

It used to report the underlying error info however this was broken by
https://git.opendaylight.org/gerrit/#/c/40235/ which changed the code to
use a Future callback with a CountDownLatch but doesn't propagate errors
from the Future. I don't understand why the code was changed in this manner
when it worked fine and was much simpler using checkedGet. I changed the
code back to use checkedGet, in several places.

Change-Id: I2e917d7eedc569702cdf3a54c4aa0321fe229ca1
Signed-off-by: Tom Pantelis <tpanteli@brocade.com>

diff --git a/restconf/sal-rest-connector/src/main/java/org/opendaylight/netconf/sal/restconf/impl/BrokerFacade.java b/restconf/sal-rest-connector/src/main/java/org/opendaylight/netconf/sal/restconf/impl/BrokerFacade.java
index 940932025103c1880167d5cc1bcd923f3d9e2382..e71934d6642c2c12b31c12f7deaa47a30a6a7735 100644 (file)
--- a/restconf/sal-rest-connector/src/main/java/org/opendaylight/netconf/sal/restconf/impl/BrokerFacade.java
+++ b/restconf/sal-rest-connector/src/main/java/org/opendaylight/netconf/sal/restconf/impl/BrokerFacade.java
@@ -9,13 +9,13 @@ package org.opendaylight.netconf.sal.restconf.impl;
 
 import static org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType.CONFIGURATION;
 import static org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType.OPERATIONAL;
+
 import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.util.concurrent.CheckedFuture;
 import com.google.common.util.concurrent.FutureCallback;
 import com.google.common.util.concurrent.Futures;
-import com.google.common.util.concurrent.ListenableFuture;
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
@@ -103,7 +103,7 @@ public class BrokerFacade {
     }
 
     private void checkPreconditions() {
-        if ((this.context == null) || (this.domDataBroker == null)) {
+        if (this.context == null || this.domDataBroker == null) {
             throw new RestconfDocumentedException(Status.SERVICE_UNAVAILABLE);
         }
     }
@@ -519,38 +519,15 @@ public class BrokerFacade {
     private NormalizedNode<?, ?> readDataViaTransaction(final DOMDataReadTransaction transaction,
             final LogicalDatastoreType datastore, final YangInstanceIdentifier path, final String withDefa) {
         LOG.trace("Read {} via Restconf: {}", datastore.name(), path);
-        final ListenableFuture<Optional<NormalizedNode<?, ?>>> listenableFuture = transaction.read(datastore, path);
-        final ReadDataResult<NormalizedNode<?, ?>> readData = new ReadDataResult<>();
-        final CountDownLatch responseWaiter = new CountDownLatch(1);
-
-        Futures.addCallback(listenableFuture, new FutureCallback<Optional<NormalizedNode<?, ?>>>() {
-
-            @Override
-            public void onSuccess(final Optional<NormalizedNode<?, ?>> result) {
-                handlingCallback(null, datastore, path, result, readData);
-                responseWaiter.countDown();
-            }
-
-            @Override
-            public void onFailure(final Throwable t) {
-                responseWaiter.countDown();
-                handlingCallback(t, datastore, path, null, null);
-            }
-        });
 
         try {
-            responseWaiter.await();
-        } catch (final Exception e) {
-            final String msg = "Problem while waiting for response";
-            LOG.warn(msg);
-            throw new RestconfDocumentedException(msg, e);
-        }
-        if (withDefa == null) {
-            return readData.getResult();
-        } else {
-            return prepareDataByParamWithDef(readData.getResult(), path, withDefa);
+            final Optional<NormalizedNode<?, ?>> optional = transaction.read(datastore, path).checkedGet();
+            return !optional.isPresent() ? null : withDefa == null ? optional.get() :
+                prepareDataByParamWithDef(optional.get(), path, withDefa);
+        } catch (ReadFailedException e) {
+            LOG.warn("Error reading {} from datastore {}", path, datastore.name(), e);
+            throw new RestconfDocumentedException("Error reading data.", e, e.getErrorList());
         }
-
     }
 
     private NormalizedNode<?, ?> prepareDataByParamWithDef(final NormalizedNode<?, ?> result,
@@ -611,12 +588,12 @@ public class BrokerFacade {
                     builder.withChild(leafBuilder.build());
                 } else {
                     if (trim) {
-                        if ((defaultVal == null) || !defaultVal.equals(nodeVal)) {
+                        if (defaultVal == null || !defaultVal.equals(nodeVal)) {
                             leafBuilder.withValue(((LeafNode) child).getValue());
                             builder.withChild(leafBuilder.build());
                         }
                     } else {
-                        if ((defaultVal != null) && defaultVal.equals(nodeVal)) {
+                        if (defaultVal != null && defaultVal.equals(nodeVal)) {
                             leafBuilder.withValue(((LeafNode) child).getValue());
                             builder.withChild(leafBuilder.build());
                         }
@@ -662,12 +639,12 @@ public class BrokerFacade {
                 final NormalizedNodeAttrBuilder<NodeIdentifier, Object, LeafNode<Object>> leafBuilder =
                         Builders.leafBuilder((LeafSchemaNode) childSchema);
                 if (trim) {
-                    if ((defaultVal == null) || !defaultVal.equals(nodeVal)) {
+                    if (defaultVal == null || !defaultVal.equals(nodeVal)) {
                         leafBuilder.withValue(((LeafNode) child).getValue());
                         builder.withChild(leafBuilder.build());
                     }
                 } else {
-                    if ((defaultVal != null) && defaultVal.equals(nodeVal)) {
+                    if (defaultVal != null && defaultVal.equals(nodeVal)) {
                         leafBuilder.withValue(((LeafNode) child).getValue());
                         builder.withChild(leafBuilder.build());
                     }
@@ -711,7 +688,7 @@ public class BrokerFacade {
                     if(schemaNode instanceof ListSchemaNode){
                         final OrderedMapNode readList =
                                 (OrderedMapNode) this.readConfigurationData(path.getParent().getParent());
-                        if ((readList == null) || readList.getValue().isEmpty()) {
+                        if (readList == null || readList.getValue().isEmpty()) {
                             simplePostPut(rWTransaction, datastore, path, payload, schemaContext);
                         } else {
                             rWTransaction.delete(datastore, path.getParent().getParent());
@@ -722,7 +699,7 @@ public class BrokerFacade {
                     } else {
                         final OrderedLeafSetNode readLeafList =
                                 (OrderedLeafSetNode) readConfigurationData(path.getParent());
-                        if ((readLeafList == null) || readLeafList.getValue().isEmpty()) {
+                        if (readLeafList == null || readLeafList.getValue().isEmpty()) {
                             simplePostPut(rWTransaction, datastore, path, payload, schemaContext);
                         } else {
                             rWTransaction.delete(datastore, path.getParent());
@@ -739,7 +716,7 @@ public class BrokerFacade {
                     if(schemaNode instanceof ListSchemaNode){
                         final OrderedMapNode readList =
                                 (OrderedMapNode) this.readConfigurationData(path.getParent().getParent());
-                        if ((readList == null) || readList.getValue().isEmpty()) {
+                        if (readList == null || readList.getValue().isEmpty()) {
                             simplePostPut(rWTransaction, datastore, path, payload, schemaContext);
                         } else {
                             insertWithPointListPost(rWTransaction, datastore, path, payload, schemaContext, point,
@@ -749,7 +726,7 @@ public class BrokerFacade {
                     } else {
                         final OrderedLeafSetNode<?> readLeafList =
                                 (OrderedLeafSetNode<?>) readConfigurationData(path.getParent());
-                        if ((readLeafList == null) || readLeafList.getValue().isEmpty()) {
+                        if (readLeafList == null || readLeafList.getValue().isEmpty()) {
                             simplePostPut(rWTransaction, datastore, path, payload, schemaContext);
                         } else {
                             insertWithPointLeafListPost(rWTransaction, datastore, path, payload, schemaContext, point,
@@ -761,7 +738,7 @@ public class BrokerFacade {
                     if (schemaNode instanceof ListSchemaNode) {
                         final OrderedMapNode readList =
                                 (OrderedMapNode) this.readConfigurationData(path.getParent().getParent());
-                        if ((readList == null) || readList.getValue().isEmpty()) {
+                        if (readList == null || readList.getValue().isEmpty()) {
                             simplePostPut(rWTransaction, datastore, path, payload, schemaContext);
                         } else {
                             insertWithPointListPost(rWTransaction, datastore, path, payload, schemaContext, point,
@@ -771,7 +748,7 @@ public class BrokerFacade {
                     } else {
                         final OrderedLeafSetNode<?> readLeafList =
                                 (OrderedLeafSetNode<?>) readConfigurationData(path.getParent());
-                        if ((readLeafList == null) || readLeafList.getValue().isEmpty()) {
+                        if (readLeafList == null || readLeafList.getValue().isEmpty()) {
                             simplePostPut(rWTransaction, datastore, path, payload, schemaContext);
                         } else {
                             insertWithPointLeafListPost(rWTransaction, datastore, path, payload, schemaContext, point,
@@ -907,6 +884,17 @@ public class BrokerFacade {
         rWTransaction.put(datastore, path, payload);
     }
 
+    private boolean doesItemExist(final DOMDataReadWriteTransaction rWTransaction,
+            final LogicalDatastoreType store, final YangInstanceIdentifier path) {
+        try {
+            return rWTransaction.exists(store, path).checkedGet();
+        } catch (ReadFailedException e) {
+            rWTransaction.cancel();
+            throw new RestconfDocumentedException("Could not determine the existence of path " + path,
+                    e, e.getErrorList());
+        }
+    }
+
     /**
      * Check if item already exists. Throws error if it does NOT already exist.
      * @param rWTransaction Current transaction
@@ -915,33 +903,7 @@ public class BrokerFacade {
      */
     private void checkItemExists(final DOMDataReadWriteTransaction rWTransaction,
                                  final LogicalDatastoreType store, final YangInstanceIdentifier path) {
-        final CountDownLatch responseWaiter = new CountDownLatch(1);
-        final ReadDataResult<Boolean> readData = new ReadDataResult<>();
-        final CheckedFuture<Boolean, ReadFailedException> future = rWTransaction.exists(store, path);
-
-        Futures.addCallback(future, new FutureCallback<Boolean>() {
-            @Override
-            public void onSuccess(@Nullable final Boolean result) {
-                handlingCallback(null, store, path, Optional.of(result), readData);
-                responseWaiter.countDown();
-            }
-
-            @Override
-            public void onFailure(final Throwable t) {
-                responseWaiter.countDown();
-                handlingCallback(t, store, path, null, null);
-            }
-        });
-
-        try {
-            responseWaiter.await();
-        } catch (final Exception e) {
-            final String msg = "Problem while waiting for response";
-            LOG.warn(msg);
-            throw new RestconfDocumentedException(msg, e);
-        }
-
-        if ((readData.getResult() == null) || !readData.getResult()) {
+        if (!doesItemExist(rWTransaction, store, path)) {
             final String errMsg = "Operation via Restconf was not executed because data does not exist";
             LOG.trace("{}:{}", errMsg, path);
             rWTransaction.cancel();
@@ -958,33 +920,7 @@ public class BrokerFacade {
      */
     private void checkItemDoesNotExists(final DOMDataReadWriteTransaction rWTransaction,
                                         final LogicalDatastoreType store, final YangInstanceIdentifier path) {
-        final CountDownLatch responseWaiter = new CountDownLatch(1);
-        final ReadDataResult<Boolean> readData = new ReadDataResult<>();
-        final CheckedFuture<Boolean, ReadFailedException> future = rWTransaction.exists(store, path);
-
-        Futures.addCallback(future, new FutureCallback<Boolean>() {
-            @Override
-            public void onSuccess(@Nullable final Boolean result) {
-                handlingCallback(null, store, path, Optional.of(result), readData);
-                responseWaiter.countDown();
-            }
-
-            @Override
-            public void onFailure(final Throwable t) {
-                responseWaiter.countDown();
-                handlingCallback(t, store, path, null, null);
-            }
-        });
-
-        try {
-            responseWaiter.await();
-        } catch (final Exception e) {
-            final String msg = "Problem while waiting for response";
-            LOG.warn(msg);
-            throw new RestconfDocumentedException(msg, e);
-        }
-
-        if (readData.getResult()) {
+        if (doesItemExist(rWTransaction, store, path)) {
             final String errMsg = "Operation via Restconf was not executed because data already exists";
             LOG.trace("{}:{}", errMsg, path);
             rWTransaction.cancel();
@@ -1035,7 +971,7 @@ public class BrokerFacade {
                     if (schemaNode instanceof ListSchemaNode) {
                         final OrderedMapNode readList =
                                 (OrderedMapNode) this.readConfigurationData(path.getParent());
-                        if ((readList == null) || readList.getValue().isEmpty()) {
+                        if (readList == null || readList.getValue().isEmpty()) {
                             simplePut(datastore, path, rWTransaction, schemaContext, payload);
                         } else {
                             rWTransaction.delete(datastore, path.getParent());
@@ -1045,7 +981,7 @@ public class BrokerFacade {
                     } else {
                         final OrderedLeafSetNode<?> readLeafList =
                                 (OrderedLeafSetNode<?>) readConfigurationData(path.getParent());
-                        if ((readLeafList == null) || readLeafList.getValue().isEmpty()) {
+                        if (readLeafList == null || readLeafList.getValue().isEmpty()) {
                             simplePut(datastore, path, rWTransaction, schemaContext, payload);
                         } else {
                             rWTransaction.delete(datastore, path.getParent());
@@ -1062,7 +998,7 @@ public class BrokerFacade {
                     if (schemaNode instanceof ListSchemaNode) {
                         final OrderedMapNode readList =
                                 (OrderedMapNode) this.readConfigurationData(path.getParent());
-                        if ((readList == null) || readList.getValue().isEmpty()) {
+                        if (readList == null || readList.getValue().isEmpty()) {
                             simplePut(datastore, path, rWTransaction, schemaContext, payload);
                         } else {
                             insertWithPointListPut(rWTransaction, datastore, path, payload, schemaContext, point,
@@ -1071,7 +1007,7 @@ public class BrokerFacade {
                     } else {
                         final OrderedLeafSetNode<?> readLeafList =
                                 (OrderedLeafSetNode<?>) readConfigurationData(path.getParent());
-                        if ((readLeafList == null) || readLeafList.getValue().isEmpty()) {
+                        if (readLeafList == null || readLeafList.getValue().isEmpty()) {
                             simplePut(datastore, path, rWTransaction, schemaContext, payload);
                         } else {
                             insertWithPointLeafListPut(rWTransaction, datastore, path, payload, schemaContext, point,
@@ -1083,7 +1019,7 @@ public class BrokerFacade {
                     if (schemaNode instanceof ListSchemaNode) {
                         final OrderedMapNode readList =
                                 (OrderedMapNode) this.readConfigurationData(path.getParent());
-                        if ((readList == null) || readList.getValue().isEmpty()) {
+                        if (readList == null || readList.getValue().isEmpty()) {
                             simplePut(datastore, path, rWTransaction, schemaContext, payload);
                         } else {
                             insertWithPointListPut(rWTransaction, datastore, path, payload, schemaContext, point,
@@ -1092,7 +1028,7 @@ public class BrokerFacade {
                     } else {
                         final OrderedLeafSetNode<?> readLeafList =
                                 (OrderedLeafSetNode<?>) readConfigurationData(path.getParent());
-                        if ((readLeafList == null) || readLeafList.getValue().isEmpty()) {
+                        if (readLeafList == null || readLeafList.getValue().isEmpty()) {
                             simplePut(datastore, path, rWTransaction, schemaContext, payload);
                         } else {
                             insertWithPointLeafListPut(rWTransaction, datastore, path, payload, schemaContext, point,
@@ -1226,47 +1162,6 @@ public class BrokerFacade {
         this.domDataBroker = domDataBroker;
     }
 
-    /**
-     * Helper class for result of transaction commit callback.
-     * @param <T> Type of result
-     */
-    private final class ReadDataResult<T> {
-        T result = null;
-
-        T getResult() {
-            return this.result;
-        }
-
-        void setResult(final T result) {
-            this.result = result;
-        }
-    }
-
-    /**
-     * Set result from transaction commit callback.
-     * @param t Throwable if transaction commit failed
-     * @param datastore Datastore from which data are read
-     * @param path Path from which data are read
-     * @param result Result of read from {@code datastore}
-     * @param readData Result value which will be set
-     * @param <X> Result type
-     */
-    protected final static <X> void handlingCallback(final Throwable t, final LogicalDatastoreType datastore,
-                                                     final YangInstanceIdentifier path, final Optional<X> result,
-                                                     final ReadDataResult<X> readData) {
-        if (t != null) {
-            LOG.warn("Exception by reading {} via Restconf: {}", datastore.name(), path, t);
-            throw new RestconfDocumentedException("Problem to get data from transaction.", t);
-        } else {
-            LOG.debug("Reading result data from transaction.");
-            if (result != null) {
-                if (result.isPresent()) {
-                    readData.setResult(result.get());
-                }
-            }
-        }
-    }
-
     public void registerToListenNotification(final NotificationListenerAdapter listener) {
         checkPreconditions();
 

diff --git a/restconf/sal-rest-connector/src/main/java/org/opendaylight/netconf/sal/restconf/impl/RestconfImpl.java b/restconf/sal-rest-connector/src/main/java/org/opendaylight/netconf/sal/restconf/impl/RestconfImpl.java
index 928abe0eaf61c13431975d96de2a9bbea6d68073..a09dbcc4a4378b3d7cca0545a877d305d9ad3215 100644 (file)
--- a/restconf/sal-rest-connector/src/main/java/org/opendaylight/netconf/sal/restconf/impl/RestconfImpl.java
+++ b/restconf/sal-rest-connector/src/main/java/org/opendaylight/netconf/sal/restconf/impl/RestconfImpl.java
@@ -11,15 +11,16 @@ package org.opendaylight.netconf.sal.restconf.impl;
 import com.google.common.base.CharMatcher;
 import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
+import com.google.common.base.Predicates;
 import com.google.common.base.Splitter;
 import com.google.common.base.Strings;
+import com.google.common.base.Throwables;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
 import com.google.common.util.concurrent.CheckedFuture;
-import com.google.common.util.concurrent.FutureCallback;
 import com.google.common.util.concurrent.Futures;
 import java.net.URI;
 import java.net.URISyntaxException;
@@ -37,7 +38,6 @@ import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
 import java.util.concurrent.CancellationException;
-import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutionException;
 import javax.ws.rs.core.Context;
 import javax.ws.rs.core.Response;
@@ -81,6 +81,7 @@ 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.MapNode;
 import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode;
+import org.opendaylight.yangtools.yang.data.api.schema.tree.ModifiedNodeDoesNotExistException;
 import org.opendaylight.yangtools.yang.data.impl.schema.Builders;
 import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes;
 import org.opendaylight.yangtools.yang.data.impl.schema.builder.api.CollectionNodeBuilder;
@@ -469,12 +470,12 @@ public class RestconfImpl implements RestconfService {
 
         RpcDefinition resultNodeSchema = null;
         final NormalizedNode<?, ?> resultData = result.getResult();
-        if ((result != null) && (result.getResult() != null)) {
+        if (result != null && result.getResult() != null) {
             resultNodeSchema = (RpcDefinition) payload.getInstanceIdentifierContext().getSchemaNode();
         }
 
         return new NormalizedNodeContext(
-                new InstanceIdentifierContext<RpcDefinition>(null, resultNodeSchema, mountPoint, schemaContext),
+                new InstanceIdentifierContext<>(null, resultNodeSchema, mountPoint, schemaContext),
                 resultData, QueryParametersParser.parseWriterParameters(uriInfo));
     }
 
@@ -484,7 +485,7 @@ public class RestconfImpl implements RestconfService {
         }
         try {
             final DOMRpcResult retValue = response.get();
-            if ((retValue.getErrors() == null) || retValue.getErrors().isEmpty()) {
+            if (retValue.getErrors() == null || retValue.getErrors().isEmpty()) {
                 return retValue;
             }
             LOG.debug("RpcError message", retValue.getErrors());
@@ -522,10 +523,10 @@ public class RestconfImpl implements RestconfService {
     }
 
     private static void validateInput(final SchemaNode inputSchema, final NormalizedNodeContext payload) {
-        if ((inputSchema != null) && (payload.getData() == null)) {
+        if (inputSchema != null && payload.getData() == null) {
             // expected a non null payload
             throw new RestconfDocumentedException("Input is required.", ErrorType.PROTOCOL, ErrorTag.MALFORMED_MESSAGE);
-        } else if ((inputSchema == null) && (payload.getData() != null)) {
+        } else if (inputSchema == null && payload.getData() != null) {
             // did not expect any input
             throw new RestconfDocumentedException("No input expected.", ErrorType.PROTOCOL, ErrorTag.MALFORMED_MESSAGE);
         }
@@ -545,7 +546,7 @@ public class RestconfImpl implements RestconfService {
             throw new RestconfDocumentedException(errMsg, ErrorType.APPLICATION, ErrorTag.OPERATION_FAILED);
         }
 
-        final YangInstanceIdentifier pathIdentifier = ((YangInstanceIdentifier) pathValue);
+        final YangInstanceIdentifier pathIdentifier = (YangInstanceIdentifier) pathValue;
         String streamName = (String) CREATE_DATA_SUBSCR;
         NotificationOutputType outputType = null;
         if (!pathIdentifier.isEmpty()) {
@@ -590,7 +591,7 @@ public class RestconfImpl implements RestconfService {
 
     @Override
     public NormalizedNodeContext invokeRpc(final String identifier, final String noPayload, final UriInfo uriInfo) {
-        if ((noPayload != null) && !CharMatcher.WHITESPACE.matchesAllOf(noPayload)) {
+        if (noPayload != null && !CharMatcher.WHITESPACE.matchesAllOf(noPayload)) {
             throw new RestconfDocumentedException("Content must be empty.", ErrorType.PROTOCOL, ErrorTag.INVALID_VALUE);
         }
 
@@ -745,24 +746,24 @@ public class RestconfImpl implements RestconfService {
     @Override
     public Response updateConfigurationData(final String identifier, final NormalizedNodeContext payload,
             final UriInfo uriInfo) {
-        boolean insert_used = false;
-        boolean point_used = false;
+        boolean insertUsed = false;
+        boolean pointUsed = false;
         String insert = null;
         String point = null;
 
         for (final Entry<String, List<String>> entry : uriInfo.getQueryParameters().entrySet()) {
             switch (entry.getKey()) {
                 case "insert":
-                    if (!insert_used) {
-                        insert_used = true;
+                    if (!insertUsed) {
+                        insertUsed = true;
                         insert = entry.getValue().iterator().next();
                     } else {
                         throw new RestconfDocumentedException("Insert parameter can be used only once.");
                     }
                     break;
                 case "point":
-                    if (!point_used) {
-                        point_used = true;
+                    if (!pointUsed) {
+                        pointUsed = true;
                         point = entry.getValue().iterator().next();
                     } else {
                         throw new RestconfDocumentedException("Point parameter can be used only once.");
@@ -773,10 +774,10 @@ public class RestconfImpl implements RestconfService {
             }
         }
 
-        if (point_used && !insert_used) {
+        if (pointUsed && !insertUsed) {
             throw new RestconfDocumentedException("Point parameter can't be used without Insert parameter.");
         }
-        if (point_used && (insert.equals("first") || insert.equals("last"))) {
+        if (pointUsed && (insert.equals("first") || insert.equals("last"))) {
             throw new RestconfDocumentedException(
                     "Point parameter can be used only with 'after' or 'before' values of Insert parameter.");
         }
@@ -809,7 +810,7 @@ public class RestconfImpl implements RestconfService {
          * (and having to document the behavior).
          */
         PutResult result = null;
-        final TryOfPutData tryPutData = new TryOfPutData();
+        int tries = 2;
         while (true) {
             if (mountPoint != null) {
 
@@ -819,56 +820,23 @@ public class RestconfImpl implements RestconfService {
                 result = this.broker.commitConfigurationDataPut(this.controllerContext.getGlobalSchema(), normalizedII,
                         payload.getData(), insert, point);
             }
-            final CountDownLatch waiter = new CountDownLatch(1);
-            Futures.addCallback(result.getFutureOfPutData(), new FutureCallback<Void>() {
-
-                @Override
-                public void onSuccess(final Void result) {
-                    handlingLoggerPut(null, tryPutData, identifier);
-                    waiter.countDown();
-                }
-
-                @Override
-                public void onFailure(final Throwable t) {
-                    waiter.countDown();
-                    handlingLoggerPut(t, tryPutData, identifier);
-                }
-            });
 
             try {
-                waiter.await();
-            } catch (final Exception e) {
-                final String msg = "Problem while waiting for response";
-                LOG.warn(msg);
-                throw new RestconfDocumentedException(msg, e);
-            }
-
-            if (tryPutData.isDone()) {
-                break;
-            } else {
-                throw new RestconfDocumentedException("Problem while PUT operations");
-            }
-        }
-
-        return Response.status(result.getStatus()).build();
-    }
+                result.getFutureOfPutData().checkedGet();
+                return Response.status(result.getStatus()).build();
+            } catch (TransactionCommitFailedException e) {
+                if (e instanceof OptimisticLockFailedException) {
+                    if (--tries <= 0) {
+                        LOG.debug("Got OptimisticLockFailedException on last try - failing " + identifier);
+                        throw new RestconfDocumentedException(e.getMessage(), e, e.getErrorList());
+                    }
 
-    protected void handlingLoggerPut(final Throwable t, final TryOfPutData tryPutData, final String identifier) {
-        if (t != null) {
-            if (t instanceof OptimisticLockFailedException) {
-                if (tryPutData.countGet() <= 0) {
-                    LOG.debug("Got OptimisticLockFailedException on last try - failing " + identifier);
-                    throw new RestconfDocumentedException(t.getMessage(), t);
+                    LOG.debug("Got OptimisticLockFailedException - trying again " + identifier);
+                } else {
+                    LOG.debug("Update failed for " + identifier, e);
+                    throw new RestconfDocumentedException(e.getMessage(), e, e.getErrorList());
                 }
-                LOG.debug("Got OptimisticLockFailedException - trying again " + identifier);
-                tryPutData.countDown();
-            } else {
-                LOG.debug("Update ConfigDataStore fail " + identifier, t);
-                throw new RestconfDocumentedException(t.getMessage(), t);
             }
-        } else {
-            LOG.trace("PUT Successful " + identifier);
-            tryPutData.done();
         }
     }
 
@@ -911,7 +879,7 @@ public class RestconfImpl implements RestconfService {
         final NormalizedNode<?, ?> data = payload.getData();
         if (schemaNode instanceof ListSchemaNode) {
             final List<QName> keyDefinitions = ((ListSchemaNode) schemaNode).getKeyDefinition();
-            if ((lastPathArgument instanceof NodeIdentifierWithPredicates) && (data instanceof MapEntryNode)) {
+            if (lastPathArgument instanceof NodeIdentifierWithPredicates && data instanceof MapEntryNode) {
                 final Map<QName, Object> uriKeyValues =
                         ((NodeIdentifierWithPredicates) lastPathArgument).getKeyValues();
                 isEqualUriAndPayloadKeyValues(uriKeyValues, (MapEntryNode) data, keyDefinitions);
@@ -986,24 +954,24 @@ public class RestconfImpl implements RestconfService {
         final InstanceIdentifierContext<?> iiWithData = payload.getInstanceIdentifierContext();
         final YangInstanceIdentifier normalizedII = iiWithData.getInstanceIdentifier();
 
-        boolean insert_used = false;
-        boolean point_used = false;
+        boolean insertUsed = false;
+        boolean pointUsed = false;
         String insert = null;
         String point = null;
 
         for (final Entry<String, List<String>> entry : uriInfo.getQueryParameters().entrySet()) {
             switch (entry.getKey()) {
                 case "insert":
-                    if (!insert_used) {
-                        insert_used = true;
+                    if (!insertUsed) {
+                        insertUsed = true;
                         insert = entry.getValue().iterator().next();
                     } else {
                         throw new RestconfDocumentedException("Insert parameter can be used only once.");
                     }
                     break;
                 case "point":
-                    if (!point_used) {
-                        point_used = true;
+                    if (!pointUsed) {
+                        pointUsed = true;
                         point = entry.getValue().iterator().next();
                     } else {
                         throw new RestconfDocumentedException("Point parameter can be used only once.");
@@ -1014,10 +982,10 @@ public class RestconfImpl implements RestconfService {
             }
         }
 
-        if (point_used && !insert_used) {
+        if (pointUsed && !insertUsed) {
             throw new RestconfDocumentedException("Point parameter can't be used without Insert parameter.");
         }
-        if (point_used && (insert.equals("first") || insert.equals("last"))) {
+        if (pointUsed && (insert.equals("first") || insert.equals("last"))) {
             throw new RestconfDocumentedException(
                     "Point parameter can be used only with 'after' or 'before' values of Insert parameter.");
         }
@@ -1031,30 +999,17 @@ public class RestconfImpl implements RestconfService {
                     payload.getData(), insert, point);
         }
 
-        final CountDownLatch waiter = new CountDownLatch(1);
-        Futures.addCallback(future, new FutureCallback<Void>() {
-
-            @Override
-            public void onSuccess(final Void result) {
-                handlerLoggerPost(null, uriInfo);
-                waiter.countDown();
-            }
-
-            @Override
-            public void onFailure(final Throwable t) {
-                waiter.countDown();
-                handlerLoggerPost(t, uriInfo);
-            }
-        });
-
         try {
-            waiter.await();
-        } catch (final Exception e) {
-            final String msg = "Problem while waiting for response";
-            LOG.warn(msg);
-            throw new RestconfDocumentedException(msg, e);
+            future.checkedGet();
+        } catch (final RestconfDocumentedException e) {
+            throw e;
+        } catch (TransactionCommitFailedException e) {
+            LOG.info("Error creating data " + (uriInfo != null ? uriInfo.getPath() : ""), e);
+            throw new RestconfDocumentedException(e.getMessage(), e, e.getErrorList());
         }
 
+        LOG.trace("Successfuly created data.");
+
         final ResponseBuilder responseBuilder = Response.status(Status.NO_CONTENT);
         // FIXME: Provide path to result.
         final URI location = resolveLocation(uriInfo, "", mountPoint, normalizedII);
@@ -1064,16 +1019,6 @@ public class RestconfImpl implements RestconfService {
         return responseBuilder.build();
     }
 
-    protected void handlerLoggerPost(final Throwable t, final UriInfo uriInfo) {
-        if (t != null) {
-            final String errMsg = "Error creating data ";
-            LOG.warn(errMsg + (uriInfo != null ? uriInfo.getPath() : ""), t);
-            throw new RestconfDocumentedException(errMsg, t);
-        } else {
-            LOG.trace("Successfuly create data.");
-        }
-    }
-
     private URI resolveLocation(final UriInfo uriInfo, final String uriBehindBase, final DOMMountPoint mountPoint,
             final YangInstanceIdentifier normalizedII) {
         if (uriInfo == null) {
@@ -1105,51 +1050,22 @@ public class RestconfImpl implements RestconfService {
             future = this.broker.commitConfigurationDataDelete(normalizedII);
         }
 
-        final CountDownLatch waiter = new CountDownLatch(1);
-        final ResultOperation result = new ResultOperation();
-        Futures.addCallback(future, new FutureCallback<Void>() {
-
-            @Override
-            public void onSuccess(final Void result) {
-                LOG.trace("Successfuly delete data.");
-                waiter.countDown();
-            }
-
-            @Override
-            public void onFailure(final Throwable t) {
-                waiter.countDown();
-                result.setFailed(t);
+        try {
+            future.checkedGet();
+        } catch (TransactionCommitFailedException e) {
+            final Optional<Throwable> searchedException = Iterables.tryFind(Throwables.getCausalChain(e),
+                    Predicates.instanceOf(ModifiedNodeDoesNotExistException.class));
+            if (searchedException.isPresent()) {
+                throw new RestconfDocumentedException("Data specified for delete doesn't exist.", ErrorType.APPLICATION,
+                        ErrorTag.DATA_MISSING);
             }
 
-        });
-
-        try {
-            waiter.await();
-        } catch (final Exception e) {
-            final String msg = "Problem while waiting for response";
-            LOG.warn(msg);
-            throw new RestconfDocumentedException(msg, e);
-        }
-        if (result.failed() != null) {
-            final Throwable t = result.failed();
             final String errMsg = "Error while deleting data";
-            LOG.info(errMsg, t);
-            throw new RestconfDocumentedException(errMsg, RestconfError.ErrorType.APPLICATION,
-                    RestconfError.ErrorTag.OPERATION_FAILED, t);
-        }
-        return Response.status(Status.OK).build();
-    }
-
-    private class ResultOperation {
-        private Throwable t = null;
-
-        public void setFailed(final Throwable t) {
-            this.t = t;
+            LOG.info(errMsg, e);
+            throw new RestconfDocumentedException(e.getMessage(), e, e.getErrorList());
         }
 
-        public Throwable failed() {
-            return this.t;
-        }
+        return Response.status(Status.OK).build();
     }
 
     /**
@@ -1239,9 +1155,9 @@ public class RestconfImpl implements RestconfService {
         final String value = event.getValue();
         if (value.contains(".")) {
             numOf_ms = numOf_ms + ".";
-            final int lastChar = value.contains("Z") ? value.indexOf("Z") : (value.contains("+") ? value.indexOf("+")
-                    : (value.contains("-") ? value.indexOf("-") : value.length()));
-            for (int i = 0; i < (lastChar - value.indexOf(".") - 1); i++) {
+            final int lastChar = value.contains("Z") ? value.indexOf("Z") : value.contains("+") ? value.indexOf("+")
+                    : value.contains("-") ? value.indexOf("-") : value.length();
+            for (int i = 0; i < lastChar - value.indexOf(".") - 1; i++) {
                 numOf_ms = numOf_ms + "S";
             }
         }
@@ -1299,7 +1215,7 @@ public class RestconfImpl implements RestconfService {
             throw new RestconfDocumentedException("Stream name is empty.", ErrorType.PROTOCOL, ErrorTag.INVALID_VALUE);
         }
         final List<NotificationListenerAdapter> listeners = Notificator.getNotificationListenerFor(streamName);
-        if ((listeners == null) || listeners.isEmpty()) {
+        if (listeners == null || listeners.isEmpty()) {
             throw new RestconfDocumentedException("Stream was not found.", ErrorType.PROTOCOL,
                     ErrorTag.UNKNOWN_ELEMENT);
         }
@@ -1501,14 +1417,14 @@ public class RestconfImpl implements RestconfService {
                 Builders.mapEntryBuilder(listModuleSchemaNode);
 
         List<DataSchemaNode> instanceDataChildrenByName =
-                ControllerContext.findInstanceDataChildrenByName((listModuleSchemaNode), "name");
+                ControllerContext.findInstanceDataChildrenByName(listModuleSchemaNode, "name");
         final DataSchemaNode nameSchemaNode = Iterables.getFirst(instanceDataChildrenByName, null);
         Preconditions.checkState(nameSchemaNode instanceof LeafSchemaNode);
         moduleNodeValues
                 .withChild(Builders.leafBuilder((LeafSchemaNode) nameSchemaNode).withValue(module.getName()).build());
 
         instanceDataChildrenByName =
-                ControllerContext.findInstanceDataChildrenByName((listModuleSchemaNode), "revision");
+                ControllerContext.findInstanceDataChildrenByName(listModuleSchemaNode, "revision");
         final DataSchemaNode revisionSchemaNode = Iterables.getFirst(instanceDataChildrenByName, null);
         Preconditions.checkState(revisionSchemaNode instanceof LeafSchemaNode);
         final String revision = REVISION_FORMAT.format(module.getRevision());
@@ -1516,20 +1432,20 @@ public class RestconfImpl implements RestconfService {
                 .withChild(Builders.leafBuilder((LeafSchemaNode) revisionSchemaNode).withValue(revision).build());
 
         instanceDataChildrenByName =
-                ControllerContext.findInstanceDataChildrenByName((listModuleSchemaNode), "namespace");
+                ControllerContext.findInstanceDataChildrenByName(listModuleSchemaNode, "namespace");
         final DataSchemaNode namespaceSchemaNode = Iterables.getFirst(instanceDataChildrenByName, null);
         Preconditions.checkState(namespaceSchemaNode instanceof LeafSchemaNode);
         moduleNodeValues.withChild(Builders.leafBuilder((LeafSchemaNode) namespaceSchemaNode)
                 .withValue(module.getNamespace().toString()).build());
 
         instanceDataChildrenByName =
-                ControllerContext.findInstanceDataChildrenByName((listModuleSchemaNode), "feature");
+                ControllerContext.findInstanceDataChildrenByName(listModuleSchemaNode, "feature");
         final DataSchemaNode featureSchemaNode = Iterables.getFirst(instanceDataChildrenByName, null);
         Preconditions.checkState(featureSchemaNode instanceof LeafListSchemaNode);
         final ListNodeBuilder<Object, LeafSetEntryNode<Object>> featuresBuilder =
                 Builders.leafSetBuilder((LeafListSchemaNode) featureSchemaNode);
         for (final FeatureDefinition feature : module.getFeatures()) {
-            featuresBuilder.withChild(Builders.leafSetEntryBuilder(((LeafListSchemaNode) featureSchemaNode))
+            featuresBuilder.withChild(Builders.leafSetEntryBuilder((LeafListSchemaNode) featureSchemaNode)
                     .withValue(feature.getQName().getLocalName()).build());
         }
         moduleNodeValues.withChild(featuresBuilder.build());
@@ -1545,33 +1461,33 @@ public class RestconfImpl implements RestconfService {
                 Builders.mapEntryBuilder(listStreamSchemaNode);
 
         List<DataSchemaNode> instanceDataChildrenByName =
-                ControllerContext.findInstanceDataChildrenByName((listStreamSchemaNode), "name");
+                ControllerContext.findInstanceDataChildrenByName(listStreamSchemaNode, "name");
         final DataSchemaNode nameSchemaNode = Iterables.getFirst(instanceDataChildrenByName, null);
         Preconditions.checkState(nameSchemaNode instanceof LeafSchemaNode);
         streamNodeValues.withChild(Builders.leafBuilder((LeafSchemaNode) nameSchemaNode).withValue(streamName).build());
 
         instanceDataChildrenByName =
-                ControllerContext.findInstanceDataChildrenByName((listStreamSchemaNode), "description");
+                ControllerContext.findInstanceDataChildrenByName(listStreamSchemaNode, "description");
         final DataSchemaNode descriptionSchemaNode = Iterables.getFirst(instanceDataChildrenByName, null);
         Preconditions.checkState(descriptionSchemaNode instanceof LeafSchemaNode);
         streamNodeValues.withChild(
                 Builders.leafBuilder((LeafSchemaNode) nameSchemaNode).withValue("DESCRIPTION_PLACEHOLDER").build());
 
         instanceDataChildrenByName =
-                ControllerContext.findInstanceDataChildrenByName((listStreamSchemaNode), "replay-support");
+                ControllerContext.findInstanceDataChildrenByName(listStreamSchemaNode, "replay-support");
         final DataSchemaNode replaySupportSchemaNode = Iterables.getFirst(instanceDataChildrenByName, null);
         Preconditions.checkState(replaySupportSchemaNode instanceof LeafSchemaNode);
         streamNodeValues.withChild(Builders.leafBuilder((LeafSchemaNode) replaySupportSchemaNode)
                 .withValue(Boolean.valueOf(true)).build());
 
         instanceDataChildrenByName =
-                ControllerContext.findInstanceDataChildrenByName((listStreamSchemaNode), "replay-log-creation-time");
+                ControllerContext.findInstanceDataChildrenByName(listStreamSchemaNode, "replay-log-creation-time");
         final DataSchemaNode replayLogCreationTimeSchemaNode = Iterables.getFirst(instanceDataChildrenByName, null);
         Preconditions.checkState(replayLogCreationTimeSchemaNode instanceof LeafSchemaNode);
         streamNodeValues.withChild(
                 Builders.leafBuilder((LeafSchemaNode) replayLogCreationTimeSchemaNode).withValue("").build());
 
-        instanceDataChildrenByName = ControllerContext.findInstanceDataChildrenByName((listStreamSchemaNode), "events");
+        instanceDataChildrenByName = ControllerContext.findInstanceDataChildrenByName(listStreamSchemaNode, "events");
         final DataSchemaNode eventsSchemaNode = Iterables.getFirst(instanceDataChildrenByName, null);
         Preconditions.checkState(eventsSchemaNode instanceof LeafSchemaNode);
         streamNodeValues.withChild(Builders.leafBuilder((LeafSchemaNode) eventsSchemaNode).withValue("").build());
@@ -1595,7 +1511,7 @@ public class RestconfImpl implements RestconfService {
             if (dataChild instanceof LeafSetNode) {
                 leafSet = (LeafSetNode) dataChild;
             } else if (dataChild instanceof AugmentationNode) {
-                outputType = (String) (((AugmentationNode) dataChild).getValue()).iterator().next().getValue();
+                outputType = (String) ((AugmentationNode) dataChild).getValue().iterator().next().getValue();
             }
         }
 
@@ -1643,25 +1559,4 @@ public class RestconfImpl implements RestconfService {
 
         return Futures.immediateCheckedFuture(defaultDOMRpcResult);
     }
-
-    private class TryOfPutData {
-        int tries = 2;
-        boolean done = false;
-
-        void countDown() {
-            this.tries--;
-        }
-
-        void done() {
-            this.done = true;
-        }
-
-        boolean isDone() {
-            return this.done;
-        }
-
-        int countGet() {
-            return this.tries;
-        }
-    }
 }

diff --git a/restconf/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/JSONRestconfServiceImplTest.java b/restconf/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/JSONRestconfServiceImplTest.java
index 6729ea1b01286d69008f7f292708bddf9148e4fa..fa788c61a01afd4708311277f5770f3dff0d1a26 100644 (file)
--- a/restconf/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/JSONRestconfServiceImplTest.java
+++ b/restconf/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/JSONRestconfServiceImplTest.java
@@ -22,6 +22,7 @@ import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.reset;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
+
 import com.google.common.base.Optional;
 import com.google.common.io.Resources;
 import com.google.common.util.concurrent.Futures;
@@ -293,7 +294,7 @@ public class JSONRestconfServiceImplTest {
         verifyLeafNode(actualNode, TEST_LF12_QNAME, "lf12 data");
     }
 
-    @Test
+    @Test(expected = TransactionCommitFailedException.class)
     public void testPostFailure() throws Throwable {
         doReturn(Futures.immediateFailedCheckedFuture(new TransactionCommitFailedException("mock"))).when(brokerFacade)
                 .commitConfigurationDataPost(any(SchemaContext.class), any(YangInstanceIdentifier.class),
@@ -308,7 +309,13 @@ public class JSONRestconfServiceImplTest {
         Mockito.when(uriInfo.getQueryParameters()).thenReturn(value);
         final UriBuilder uriBuilder = UriBuilder.fromPath("");
         Mockito.when(uriInfo.getBaseUriBuilder()).thenReturn(uriBuilder);
-        this.service.post(uriPath, payload, uriInfo);
+
+        try {
+            this.service.post(uriPath, payload, uriInfo);
+        } catch (final OperationFailedException e) {
+            assertNotNull(e.getCause());
+            throw e.getCause();
+        }
     }
 
     @Test