Bug 7741 - "Please try again" should not result in http code 500

Change ErrorType to TRANSPORT
Change ErrorTag of DocumentedException to RESOURCE_DENIED
Change ErrorSeverity to ERROR
Add mapping in BrokerFacade to 503
Edit RPCError in NetconfReadOnlyTransaction
Add JUnit test for BrokerFacade

Change-Id: I039886d80a89e0ef4d376229617b3b4b7520a04c
Signed-off-by: matus.kubica <matus.kubica@pantheon.tech>

diff --git a/netconf/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/tx/NetconfProxyDOMTransaction.java b/netconf/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/tx/NetconfProxyDOMTransaction.java
index aa05f5415d00288bb55630e13f984e7727891be9..535123bf11ebd6b9b491cf8ac361a1deaaa0fc5f 100644 (file)
--- a/netconf/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/tx/NetconfProxyDOMTransaction.java
+++ b/netconf/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/tx/NetconfProxyDOMTransaction.java
@@ -90,9 +90,11 @@ public class NetconfProxyDOMTransaction implements NetconfDOMTransaction {
             @Override
             public void onComplete(final Throwable failure, final Object success) throws Throwable {
                 if (failure != null) { // ask timeout
-                    Exception exception = new DocumentedException(id + ":Master is down. Please try again.",
-                            DocumentedException.ErrorType.APPLICATION, DocumentedException.ErrorTag.OPERATION_FAILED,
-                            DocumentedException.ErrorSeverity.WARNING);
+                    final Exception exception = new DocumentedException(
+                            id + ":Master is down. Please try again.",
+                            DocumentedException.ErrorType.TRANSPORT,
+                            DocumentedException.ErrorTag.RESOURCE_DENIED,
+                            DocumentedException.ErrorSeverity.ERROR);
                     promise.failure(exception);
                     return;
                 }
@@ -123,9 +125,11 @@ public class NetconfProxyDOMTransaction implements NetconfDOMTransaction {
             @Override
             public void onComplete(final Throwable failure, final Object success) throws Throwable {
                 if (failure != null) { // ask timeout
-                    Exception exception = new DocumentedException(id + ":Master is down. Please try again.",
-                            DocumentedException.ErrorType.APPLICATION, DocumentedException.ErrorTag.OPERATION_FAILED,
-                            DocumentedException.ErrorSeverity.WARNING);
+                    final Exception exception = new DocumentedException(
+                            id + ":Master is down. Please try again.",
+                            DocumentedException.ErrorType.TRANSPORT,
+                            DocumentedException.ErrorTag.RESOURCE_DENIED,
+                            DocumentedException.ErrorSeverity.ERROR);
                     promise.failure(exception);
                     return;
                 }
@@ -144,7 +148,6 @@ public class NetconfProxyDOMTransaction implements NetconfDOMTransaction {
         LOG.trace("{}: Write {} via NETCONF: {} with payload {}", id, store, data.getIdentifier(), data.getNode());
 
         masterContextRef.tell(new PutRequest(store, data), ActorRef.noSender());
-
     }
 
     @Override
@@ -189,9 +192,11 @@ public class NetconfProxyDOMTransaction implements NetconfDOMTransaction {
             @Override
             public void onComplete(final Throwable failure, final Object success) throws Throwable {
                 if (failure != null) { // ask timeout
-                    Exception exception = new DocumentedException(id + ":Master is down. Please try again.",
-                            DocumentedException.ErrorType.APPLICATION, DocumentedException.ErrorTag.OPERATION_FAILED,
-                            DocumentedException.ErrorSeverity.WARNING);
+                    final Exception exception = new DocumentedException(
+                            id + ":Master is down. Please try again.",
+                            DocumentedException.ErrorType.TRANSPORT,
+                            DocumentedException.ErrorTag.RESOURCE_DENIED,
+                            DocumentedException.ErrorSeverity.ERROR);
                     promise.failure(exception);
                     return;
                 }

diff --git a/netconf/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/tx/NetconfReadOnlyTransaction.java b/netconf/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/tx/NetconfReadOnlyTransaction.java
index 9b386c0a477c80809d7ced1027fa5ea8c01150e6..30483377f71fada7050421fc19f912110fe102e2 100644 (file)
--- a/netconf/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/tx/NetconfReadOnlyTransaction.java
+++ b/netconf/netconf-topology-singleton/src/main/java/org/opendaylight/netconf/topology/singleton/impl/tx/NetconfReadOnlyTransaction.java
@@ -16,12 +16,15 @@ import com.google.common.util.concurrent.CheckedFuture;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.SettableFuture;
 import javax.annotation.Nullable;
+import org.opendaylight.controller.config.util.xml.DocumentedException;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.controller.md.sal.common.api.data.ReadFailedException;
 import org.opendaylight.controller.md.sal.dom.api.DOMDataReadOnlyTransaction;
 import org.opendaylight.netconf.sal.connect.util.RemoteDeviceId;
 import org.opendaylight.netconf.topology.singleton.api.NetconfDOMTransaction;
 import org.opendaylight.netconf.topology.singleton.messages.NormalizedNodeMessage;
+import org.opendaylight.yangtools.yang.common.RpcError;
+import org.opendaylight.yangtools.yang.common.RpcResultBuilder;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode;
 import org.slf4j.Logger;
@@ -62,6 +65,18 @@ public class NetconfReadOnlyTransaction implements DOMDataReadOnlyTransaction {
             @Nullable
             @Override
             public ReadFailedException apply(Exception cause) {
+                if (cause.getCause() instanceof DocumentedException) {
+                    final DocumentedException exception = (DocumentedException) cause.getCause();
+                    if (exception.getErrorSeverity() == DocumentedException.ErrorSeverity.ERROR &&
+                            exception.getErrorType() == DocumentedException.ErrorType.TRANSPORT &&
+                            exception.getErrorTag() == DocumentedException.ErrorTag.RESOURCE_DENIED) {
+                        final RpcError error = RpcResultBuilder.newError(
+                                RpcError.ErrorType.TRANSPORT,
+                                exception.getErrorTag().getTagValue(),
+                                exception.getMessage());
+                        return new ReadFailedException("Read from transaction failed", error);
+                    }
+                }
                 return new ReadFailedException("Read from transaction failed", cause);
             }
         });

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 e71934d6642c2c12b31c12f7deaa47a30a6a7735..ffe1f5e42f070d4461283b8c5c12e95e643c35db 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
@@ -44,6 +44,7 @@ import org.opendaylight.netconf.sal.streams.listeners.ListenerAdapter;
 import org.opendaylight.netconf.sal.streams.listeners.NotificationListenerAdapter;
 import org.opendaylight.yangtools.concepts.ListenerRegistration;
 import org.opendaylight.yangtools.yang.common.QName;
+import org.opendaylight.yangtools.yang.common.RpcError;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifierWithPredicates;
@@ -526,6 +527,15 @@ public class BrokerFacade {
                 prepareDataByParamWithDef(optional.get(), path, withDefa);
         } catch (ReadFailedException e) {
             LOG.warn("Error reading {} from datastore {}", path, datastore.name(), e);
+            for (final RpcError error : e.getErrorList()) {
+                if (error.getErrorType() == RpcError.ErrorType.TRANSPORT
+                        && error.getTag().equals(ErrorTag.RESOURCE_DENIED.getTagValue())) {
+                    throw new RestconfDocumentedException(
+                            error.getMessage(),
+                            ErrorType.TRANSPORT,
+                            ErrorTag.RESOURCE_DENIED_TRANSPORT);
+                }
+            }
             throw new RestconfDocumentedException("Error reading data.", e, e.getErrorList());
         }
     }

diff --git a/restconf/sal-rest-connector/src/main/java/org/opendaylight/netconf/sal/restconf/impl/RestconfError.java b/restconf/sal-rest-connector/src/main/java/org/opendaylight/netconf/sal/restconf/impl/RestconfError.java
index f15515665c28d3e424eaf9efb6bd141e71a1ce0f..1a7b6408c47b12349e3b11e4b76d6e350af15a19 100644 (file)
--- a/restconf/sal-rest-connector/src/main/java/org/opendaylight/netconf/sal/restconf/impl/RestconfError.java
+++ b/restconf/sal-rest-connector/src/main/java/org/opendaylight/netconf/sal/restconf/impl/RestconfError.java
@@ -68,7 +68,8 @@ public class RestconfError {
         OPERATION_NOT_SUPPORTED("operation-not-supported", 501 /* Not Implemented */),
         OPERATION_FAILED("operation-failed", 500 /* INTERNAL_SERVER_ERROR */),
         PARTIAL_OPERATION("partial-operation", 500 /* INTERNAL_SERVER_ERROR */),
-        MALFORMED_MESSAGE("malformed-message", 400 /* Bad Request */);
+        MALFORMED_MESSAGE("malformed-message", 400 /* Bad Request */),
+        RESOURCE_DENIED_TRANSPORT("resource-denied-transport", 503 /* Service Unavailable */);
 
         private final String tagValue;
         private final int statusCode;

diff --git a/restconf/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/BrokerFacadeTest.java b/restconf/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/BrokerFacadeTest.java
index e53ba7ffa99b95574da3bd334fafca999387de84..a8442af3b4b9c470ef35060a095b71b988877040 100644 (file)
--- a/restconf/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/BrokerFacadeTest.java
+++ b/restconf/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/BrokerFacadeTest.java
@@ -16,6 +16,7 @@ import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
@@ -68,6 +69,8 @@ import org.opendaylight.restconf.handlers.TransactionChainHandler;
 import org.opendaylight.yang.gen.v1.urn.sal.restconf.event.subscription.rev140708.NotificationOutputTypeGrouping.NotificationOutputType;
 import org.opendaylight.yangtools.concepts.ListenerRegistration;
 import org.opendaylight.yangtools.yang.common.QName;
+import org.opendaylight.yangtools.yang.common.RpcError;
+import org.opendaylight.yangtools.yang.common.RpcResultBuilder;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier;
 import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode;
@@ -165,6 +168,25 @@ public class BrokerFacadeTest {
         this.brokerFacade.readOperationalData(this.instanceID);
     }
 
+    @Test
+    public void test503() throws Exception {
+        final RpcError error = RpcResultBuilder.newError(
+                RpcError.ErrorType.TRANSPORT,
+                ErrorTag.RESOURCE_DENIED.getTagValue(),
+                "Master is down. Please try again.");
+        final ReadFailedException exception503 = new ReadFailedException("Read from transaction failed", error);
+        doReturn(Futures.immediateFailedCheckedFuture(exception503))
+                .when(rTransaction).read(any(LogicalDatastoreType.class), any(YangInstanceIdentifier.class));
+        try {
+            brokerFacade.readConfigurationData(this.instanceID, "explicit");
+            fail("This test should fail.");
+        } catch (final RestconfDocumentedException e) {
+            assertEquals("getErrorTag", ErrorTag.RESOURCE_DENIED_TRANSPORT, e.getErrors().get(0).getErrorTag());
+            assertEquals("getErrorType", ErrorType.TRANSPORT, e.getErrors().get(0).getErrorType());
+            assertEquals("getErrorMessage", "Master is down. Please try again.", e.getErrors().get(0).getErrorMessage());
+        }
+    }
+
     @Test
     public void testInvokeRpc() throws Exception {
         final DOMRpcResult expResult = mock(DOMRpcResult.class);

diff --git a/restconf/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/RestconfErrorTest.java b/restconf/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/RestconfErrorTest.java
index 2fc629f686ebcc02f8e7cefca6a603129e5cfbb6..17049890b5499db7c44545d1af93d0f6ddff8596 100644 (file)
--- a/restconf/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/RestconfErrorTest.java
+++ b/restconf/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/RestconfErrorTest.java
@@ -96,6 +96,7 @@ public class RestconfErrorTest {
         lookUpMap.put("operation-failed", 500);
         lookUpMap.put("partial-operation", 500);
         lookUpMap.put("malformed-message", 400);
+        lookUpMap.put("resource-denied-transport", 503);
 
         for (ErrorTag tag : ErrorTag.values()) {
             Integer expectedStatusCode = lookUpMap.get(tag.getTagValue());