From c3acce135d19955f72616c4c956668bb539f80f2 Mon Sep 17 00:00:00 2001 From: tpantelis Date: Mon, 26 May 2014 01:47:18 -0400 Subject: [PATCH] Bug 1003: Restconf - remove whitespace on input JsonReader, XmlReader: - Trimmed whitespace on leaf data input AbsractRpcExecutor, BrokerRpcExecutor, MountPointRpcExecutor: - Modified to handle IllegalArgumentEx and UnsupportedOperationEx thrown from invokeRpc to throw appropriate ResconfDocumentedEx. ResconfDocumentedExeptionMapper, RestconfError, RestconfErrorTest: - I discovered that the Response.Status.NOT_IMPLEMENTED jaxrs enum is defined in the enum class that is used at compile time but isn't defined in the run time enum class provided by jersey. So I changed RestconfError.ErrorTag to store the integer status code (501) instead of the enum. Ideally compile and run time should use the same lib. - SchemaAwareRpcBroker: - Modified to throw UnsupportedOperationEx if no RPC impl is found so the restconf front-end can yield the appropriate 501 (Not Implemented) status code. Change-Id: Ibfa1dc7ff1526b6d352b9f4e6be2aae0d19ab655 Signed-off-by: tpantelis --- .../dom/broker/impl/SchemaAwareRpcBroker.java | 9 +++- .../controller/sal/rest/impl/JsonReader.java | 2 +- .../RestconfDocumentedExceptionMapper.java | 3 +- .../controller/sal/rest/impl/XmlReader.java | 2 +- .../sal/restconf/impl/RestconfError.java | 46 +++++++++-------- .../rpc/impl/AbstractRpcExecutor.java | 22 +++++++++ .../restconf/rpc/impl/BrokerRpcExecutor.java | 6 ++- .../rpc/impl/MountPointRpcExecutor.java | 8 +-- .../restconf/impl/test/RestconfErrorTest.java | 49 +++++++++---------- 9 files changed, 86 insertions(+), 61 deletions(-) diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/impl/SchemaAwareRpcBroker.java b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/impl/SchemaAwareRpcBroker.java index f47e1efc3f..a2c43d0c73 100644 --- a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/impl/SchemaAwareRpcBroker.java +++ b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/impl/SchemaAwareRpcBroker.java @@ -174,8 +174,12 @@ public class SchemaAwareRpcBroker implements RpcRouter, Identifiable, Ro if (potentialImpl != null) { return potentialImpl; } + potentialImpl = defaultImplementation; - checkState(potentialImpl != null, "Implementation is not available."); + if( potentialImpl == null ) { + throw new UnsupportedOperationException( "No implementation for this operation is available." ); + } + return potentialImpl; } @@ -326,7 +330,8 @@ public class SchemaAwareRpcBroker implements RpcRouter, Identifiable, Ro SimpleNode routeContainer = inputContainer.getFirstSimpleByName(strategy.getLeaf()); checkArgument(routeContainer != null, "Leaf %s must be set with value", strategy.getLeaf()); Object route = routeContainer.getValue(); - checkArgument(route instanceof InstanceIdentifier); + checkArgument(route instanceof InstanceIdentifier, + "The routed node %s is not an instance identifier", route); RpcImplementation potential = null; if (route != null) { RoutedRpcRegImpl potentialReg = implementations.get(route); diff --git a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/rest/impl/JsonReader.java b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/rest/impl/JsonReader.java index 4d9958ee6b..593c104dfd 100644 --- a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/rest/impl/JsonReader.java +++ b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/rest/impl/JsonReader.java @@ -87,7 +87,7 @@ class JsonReader { } } else if (childType.isJsonPrimitive()) { JsonPrimitive childPrimitive = childType.getAsJsonPrimitive(); - String value = childPrimitive.getAsString(); + String value = childPrimitive.getAsString().trim(); parent.addValue(new SimpleNodeWrapper(getNamespaceFor(childName), getLocalNameFor(childName), resolveValueOfElement(value))); } diff --git a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/rest/impl/RestconfDocumentedExceptionMapper.java b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/rest/impl/RestconfDocumentedExceptionMapper.java index 456354bbf0..c13d593a8e 100644 --- a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/rest/impl/RestconfDocumentedExceptionMapper.java +++ b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/rest/impl/RestconfDocumentedExceptionMapper.java @@ -21,7 +21,6 @@ import javax.ws.rs.core.Context; import javax.ws.rs.core.HttpHeaders; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; -import javax.ws.rs.core.Response.Status; import javax.ws.rs.ext.ExceptionMapper; import javax.ws.rs.ext.Provider; import javax.xml.parsers.DocumentBuilderFactory; @@ -100,7 +99,7 @@ public class RestconfDocumentedExceptionMapper implements ExceptionMapper invokeRpc( CompositeNode rpcRequest ) + throws RestconfDocumentedException { + try { + return getRpcResult( invokeRpcUnchecked( rpcRequest ) ); + } + catch( IllegalArgumentException e ) { + throw new RestconfDocumentedException( + e.getMessage(), ErrorType.PROTOCOL, ErrorTag.INVALID_VALUE ); + } + catch( UnsupportedOperationException e ) { + throw new RestconfDocumentedException( + e.getMessage(), ErrorType.RPC, ErrorTag.OPERATION_NOT_SUPPORTED ); + } + catch( Exception e ) { + throw new RestconfDocumentedException( + "The operation encountered an unexpected error while executing.", e ); + } + } + + protected abstract Future> invokeRpcUnchecked( CompositeNode rpcRequest ); + protected RpcResult getRpcResult( Future> fromFuture ) { try { diff --git a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/rpc/impl/BrokerRpcExecutor.java b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/rpc/impl/BrokerRpcExecutor.java index 249b657d49..e23e95faab 100644 --- a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/rpc/impl/BrokerRpcExecutor.java +++ b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/rpc/impl/BrokerRpcExecutor.java @@ -7,6 +7,8 @@ */ package org.opendaylight.controller.sal.restconf.rpc.impl; +import java.util.concurrent.Future; + import org.opendaylight.controller.sal.restconf.impl.BrokerFacade; import org.opendaylight.yangtools.yang.common.RpcResult; import org.opendaylight.yangtools.yang.data.api.CompositeNode; @@ -22,7 +24,7 @@ public class BrokerRpcExecutor extends AbstractRpcExecutor { } @Override - public RpcResult invokeRpc(CompositeNode rpcRequest) { - return getRpcResult( broker.invokeRpc( getRpcDefinition().getQName(), rpcRequest ) ); + protected Future> invokeRpcUnchecked( CompositeNode rpcRequest ) { + return broker.invokeRpc( getRpcDefinition().getQName(), rpcRequest ); } } \ No newline at end of file diff --git a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/rpc/impl/MountPointRpcExecutor.java b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/rpc/impl/MountPointRpcExecutor.java index da19a0034d..26cb3b8102 100644 --- a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/rpc/impl/MountPointRpcExecutor.java +++ b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/rpc/impl/MountPointRpcExecutor.java @@ -7,8 +7,9 @@ */ package org.opendaylight.controller.sal.restconf.rpc.impl; +import java.util.concurrent.Future; + import org.opendaylight.controller.sal.core.api.mount.MountInstance; -import org.opendaylight.controller.sal.restconf.impl.RestconfDocumentedException; import org.opendaylight.yangtools.yang.common.RpcResult; import org.opendaylight.yangtools.yang.data.api.CompositeNode; import org.opendaylight.yangtools.yang.model.api.RpcDefinition; @@ -30,8 +31,7 @@ public class MountPointRpcExecutor extends AbstractRpcExecutor { } @Override - public RpcResult invokeRpc( CompositeNode rpcRequest ) - throws RestconfDocumentedException { - return getRpcResult( mountPoint.rpc( getRpcDefinition().getQName(), rpcRequest ) ); + protected Future> invokeRpcUnchecked( CompositeNode rpcRequest ) { + return mountPoint.rpc( getRpcDefinition().getQName(), rpcRequest ); } } \ No newline at end of file diff --git a/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/RestconfErrorTest.java b/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/RestconfErrorTest.java index 70ad7683a4..a1b8732554 100644 --- a/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/RestconfErrorTest.java +++ b/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/RestconfErrorTest.java @@ -15,8 +15,6 @@ import static org.junit.Assert.assertThat; import java.util.HashMap; import java.util.Map; -import javax.ws.rs.core.Response.Status; - import org.hamcrest.BaseMatcher; import org.hamcrest.Description; import org.hamcrest.Matcher; @@ -88,33 +86,34 @@ public class RestconfErrorTest { @Test public void testErrorTagStatusCodes() { - Map lookUpMap = new HashMap(); - - lookUpMap.put( "in-use", Status.fromStatusCode(409)); - lookUpMap.put( "invalid-value", Status.fromStatusCode(400)); - lookUpMap.put( "too-big", Status.fromStatusCode(413)); - lookUpMap.put( "missing-attribute", Status.fromStatusCode(400)); - lookUpMap.put( "bad-attribute", Status.fromStatusCode(400)); - lookUpMap.put( "unknown-attribute", Status.fromStatusCode(400)); - lookUpMap.put( "bad-element", Status.fromStatusCode(400)); - lookUpMap.put( "unknown-element", Status.fromStatusCode(400)); - lookUpMap.put( "unknown-namespace", Status.fromStatusCode(400)); - lookUpMap.put( "access-denied", Status.fromStatusCode(403)); - lookUpMap.put( "lock-denied", Status.fromStatusCode(409)); - lookUpMap.put( "resource-denied", Status.fromStatusCode(409)); - lookUpMap.put( "rollback-failed", Status.fromStatusCode(500)); - lookUpMap.put( "data-exists", Status.fromStatusCode(409)); - lookUpMap.put( "data-missing", Status.fromStatusCode(409)); - lookUpMap.put( "operation-not-supported", Status.fromStatusCode(501)); - lookUpMap.put( "operation-failed", Status.fromStatusCode(500)); - lookUpMap.put( "partial-operation", Status.fromStatusCode(500)); - lookUpMap.put( "malformed-message", Status.fromStatusCode(400)); + Map lookUpMap = new HashMap(); + + lookUpMap.put( "in-use", 409); + lookUpMap.put( "invalid-value", 400); + lookUpMap.put( "too-big", 413); + lookUpMap.put( "missing-attribute", 400); + lookUpMap.put( "bad-attribute", 400); + lookUpMap.put( "unknown-attribute", 400); + lookUpMap.put( "bad-element", 400); + lookUpMap.put( "unknown-element", 400); + lookUpMap.put( "unknown-namespace", 400); + lookUpMap.put( "access-denied", 403); + lookUpMap.put( "lock-denied", 409); + lookUpMap.put( "resource-denied", 409); + lookUpMap.put( "rollback-failed", 500); + lookUpMap.put( "data-exists", 409); + lookUpMap.put( "data-missing", 409); + lookUpMap.put( "operation-not-supported", 501); + lookUpMap.put( "operation-failed", 500); + lookUpMap.put( "partial-operation", 500); + lookUpMap.put( "malformed-message", 400); for( ErrorTag tag : ErrorTag.values() ) { - Status expectedStatusCode = lookUpMap.get( tag.getTagValue() ); + Integer expectedStatusCode = lookUpMap.get( tag.getTagValue() ); assertNotNull( "Failed to find " + tag.getTagValue(), expectedStatusCode ); - assertEquals( "Status Code does not match", expectedStatusCode, tag.getStatusCode() ); + assertEquals( "Status Code does not match", expectedStatusCode, + Integer.valueOf( tag.getStatusCode() ) ); } } -- 2.36.6