Bug 1003: Restconf - remove whitespace on input 66/7466/1
authortpantelis <tpanteli@brocade.com>
Mon, 26 May 2014 05:47:18 +0000 (01:47 -0400)
committertpantelis <tpanteli@brocade.com>
Mon, 26 May 2014 05:47:18 +0000 (01:47 -0400)
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 <tpanteli@brocade.com>
opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/sal/dom/broker/impl/SchemaAwareRpcBroker.java
opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/rest/impl/JsonReader.java
opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/rest/impl/RestconfDocumentedExceptionMapper.java
opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/rest/impl/XmlReader.java
opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/RestconfError.java
opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/rpc/impl/AbstractRpcExecutor.java
opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/rpc/impl/BrokerRpcExecutor.java
opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/rpc/impl/MountPointRpcExecutor.java
opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/RestconfErrorTest.java

index f47e1efc3fb364d812c08721b3e1b2d78ee6a056..a2c43d0c73ba402f13bd7b604a9629b617637732 100644 (file)
@@ -174,8 +174,12 @@ public class SchemaAwareRpcBroker implements RpcRouter, Identifiable<String>, Ro
         if (potentialImpl != null) {
             return potentialImpl;
         }
         if (potentialImpl != null) {
             return potentialImpl;
         }
+
         potentialImpl = defaultImplementation;
         potentialImpl = defaultImplementation;
-        checkState(potentialImpl != null, "Implementation is not available.");
+        if( potentialImpl == null ) {
+            throw new UnsupportedOperationException( "No implementation for this operation is available." );
+        }
+
         return potentialImpl;
     }
 
         return potentialImpl;
     }
 
@@ -326,7 +330,8 @@ public class SchemaAwareRpcBroker implements RpcRouter, Identifiable<String>, Ro
             SimpleNode<?> routeContainer = inputContainer.getFirstSimpleByName(strategy.getLeaf());
             checkArgument(routeContainer != null, "Leaf %s must be set with value", strategy.getLeaf());
             Object route = routeContainer.getValue();
             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);
             RpcImplementation potential = null;
             if (route != null) {
                 RoutedRpcRegImpl potentialReg = implementations.get(route);
index 4d9958ee6b4720b66fe5fc2a286e754630274e20..593c104dfd63e24efe268358e79bdb5a8a9e5a56 100644 (file)
@@ -87,7 +87,7 @@ class JsonReader {
             }
         } else if (childType.isJsonPrimitive()) {
             JsonPrimitive childPrimitive = childType.getAsJsonPrimitive();
             }
         } 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)));
         }
             parent.addValue(new SimpleNodeWrapper(getNamespaceFor(childName), getLocalNameFor(childName),
                     resolveValueOfElement(value)));
         }
index 456354bbf0eac9c9eab31c0d72800f5f3a6462e6..c13d593a8e9ed4cac3d525026de291a5e2d7d08a 100644 (file)
@@ -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.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;
 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<Restco
                                     .entity( " " ).build();
         }
 
                                     .entity( " " ).build();
         }
 
-        Status status = errors.iterator().next().getErrorTag().getStatusCode();
+        int status = errors.iterator().next().getErrorTag().getStatusCode();
 
         ControllerContext context = ControllerContext.getInstance();
         DataNodeContainer errorsSchemaNode = (DataNodeContainer)context.getRestconfModuleErrorsSchemaNode();
 
         ControllerContext context = ControllerContext.getInstance();
         DataNodeContainer errorsSchemaNode = (DataNodeContainer)context.getRestconfModuleErrorsSchemaNode();
index a75f6b4a85f8277bffa57ad6034abe6bc323f2a0..5b95f0de1ada80408f24670c310dce35a306ab5d 100644 (file)
@@ -165,7 +165,7 @@ public class XmlReader {
                 }
             }
         }
                 }
             }
         }
-        return data;
+        return data == null ? null : data.trim();
     }
 
     private String getAdditionalData(XMLEvent event) throws XMLStreamException {
     }
 
     private String getAdditionalData(XMLEvent event) throws XMLStreamException {
index 9220f8bd8ff3cc3d28bb9f54b0701be451623f44..cc279cbd58e68cfc98634364a30002051f698441 100644 (file)
@@ -10,8 +10,6 @@ package org.opendaylight.controller.sal.restconf.impl;
 import java.io.PrintWriter;
 import java.io.StringWriter;
 
 import java.io.PrintWriter;
 import java.io.StringWriter;
 
-import javax.ws.rs.core.Response.Status;
-
 import org.opendaylight.yangtools.yang.common.RpcError;
 
 import com.google.common.base.Preconditions;
 import org.opendaylight.yangtools.yang.common.RpcError;
 
 import com.google.common.base.Preconditions;
@@ -52,30 +50,30 @@ public class RestconfError {
     }
 
     public static enum ErrorTag {
     }
 
     public static enum ErrorTag {
-        IN_USE( "in-use", Status.fromStatusCode(409)),
-        INVALID_VALUE( "invalid-value", Status.fromStatusCode(400)),
-        TOO_BIG( "too-big", Status.fromStatusCode(413)),
-        MISSING_ATTRIBUTE( "missing-attribute", Status.fromStatusCode(400)),
-        BAD_ATTRIBUTE( "bad-attribute", Status.fromStatusCode(400)),
-        UNKNOWN_ATTRIBUTE( "unknown-attribute", Status.fromStatusCode(400)),
-        BAD_ELEMENT( "bad-element", Status.fromStatusCode(400)),
-        UNKNOWN_ELEMENT( "unknown-element", Status.fromStatusCode(400)),
-        UNKNOWN_NAMESPACE( "unknown-namespace", Status.fromStatusCode(400)),
-        ACCESS_DENIED( "access-denied", Status.fromStatusCode(403)),
-        LOCK_DENIED( "lock-denied", Status.fromStatusCode(409)),
-        RESOURCE_DENIED( "resource-denied", Status.fromStatusCode(409)),
-        ROLLBACK_FAILED( "rollback-failed", Status.fromStatusCode(500)),
-        DATA_EXISTS( "data-exists", Status.fromStatusCode(409)),
-        DATA_MISSING( "data-missing", Status.fromStatusCode(409)),
-        OPERATION_NOT_SUPPORTED( "operation-not-supported", Status.fromStatusCode(501)),
-        OPERATION_FAILED( "operation-failed", Status.fromStatusCode(500)),
-        PARTIAL_OPERATION( "partial-operation", Status.fromStatusCode(500)),
-        MALFORMED_MESSAGE( "malformed-message", Status.fromStatusCode(400));
+        IN_USE( "in-use", 409 /*Conflict*/ ),
+        INVALID_VALUE( "invalid-value", 400 /*Bad Request*/ ),
+        TOO_BIG( "too-big", 413 /*Request Entity Too Large*/ ),
+        MISSING_ATTRIBUTE( "missing-attribute", 400 /*Bad Request*/ ),
+        BAD_ATTRIBUTE( "bad-attribute", 400 /*Bad Request*/ ),
+        UNKNOWN_ATTRIBUTE( "unknown-attribute", 400 /*Bad Request*/ ),
+        BAD_ELEMENT( "bad-element", 400 /*Bad Request*/ ),
+        UNKNOWN_ELEMENT( "unknown-element", 400 /*Bad Request*/ ),
+        UNKNOWN_NAMESPACE( "unknown-namespace", 400 /*Bad Request*/ ),
+        ACCESS_DENIED( "access-denied", 403 /*Forbidden*/ ),
+        LOCK_DENIED( "lock-denied", 409 /*Conflict*/ ),
+        RESOURCE_DENIED( "resource-denied", 409 /*Conflict*/ ),
+        ROLLBACK_FAILED( "rollback-failed", 500 /*INTERNAL_SERVER_ERROR*/ ),
+        DATA_EXISTS( "data-exists", 409 /*Conflict*/ ),
+        DATA_MISSING( "data-missing", 409 /*Conflict*/ ),
+        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*/ );
 
         private final String tagValue;
 
         private final String tagValue;
-        private final Status statusCode;
+        private final int statusCode;
 
 
-        ErrorTag(final String tagValue, final Status statusCode) {
+        ErrorTag(final String tagValue, final int statusCode) {
             this.tagValue = tagValue;
             this.statusCode = statusCode;
         }
             this.tagValue = tagValue;
             this.statusCode = statusCode;
         }
@@ -94,7 +92,7 @@ public class RestconfError {
             }
         }
 
             }
         }
 
-        public Status getStatusCode() {
+        public int getStatusCode() {
             return statusCode;
         }
     }
             return statusCode;
         }
     }
index 0bc8428d76f85fae184b577040ee4d52f9cc682d..417cca653365bf96e925925226ee313513a79417 100644 (file)
@@ -30,6 +30,28 @@ public abstract class AbstractRpcExecutor implements RpcExecutor {
         return rpcDef;
     }
 
         return rpcDef;
     }
 
+    @Override
+    public RpcResult<CompositeNode> 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<RpcResult<CompositeNode>> invokeRpcUnchecked( CompositeNode rpcRequest );
+
     protected RpcResult<CompositeNode> getRpcResult(
                                             Future<RpcResult<CompositeNode>> fromFuture ) {
         try {
     protected RpcResult<CompositeNode> getRpcResult(
                                             Future<RpcResult<CompositeNode>> fromFuture ) {
         try {
index 249b657d496218b40c4893c22d9f16ebc48f1b92..e23e95faab21e2cc129a745ba4959dbaa7da5ef3 100644 (file)
@@ -7,6 +7,8 @@
 */
 package org.opendaylight.controller.sal.restconf.rpc.impl;
 
 */
 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;
 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
     }
 
     @Override
-    public RpcResult<CompositeNode> invokeRpc(CompositeNode rpcRequest) {
-        return getRpcResult( broker.invokeRpc( getRpcDefinition().getQName(), rpcRequest ) );
+    protected Future<RpcResult<CompositeNode>> invokeRpcUnchecked( CompositeNode rpcRequest ) {
+        return broker.invokeRpc( getRpcDefinition().getQName(), rpcRequest );
     }
 }
\ No newline at end of file
     }
 }
\ No newline at end of file
index da19a0034de26103afd793e0ebb4a5d002f3bffa..26cb3b81028637c1b513e5d89aad837923684a79 100644 (file)
@@ -7,8 +7,9 @@
 */
 package org.opendaylight.controller.sal.restconf.rpc.impl;
 
 */
 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.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;
 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
     }
 
     @Override
-    public RpcResult<CompositeNode> invokeRpc( CompositeNode rpcRequest )
-                                                   throws RestconfDocumentedException {
-        return getRpcResult( mountPoint.rpc( getRpcDefinition().getQName(), rpcRequest ) );
+    protected Future<RpcResult<CompositeNode>> invokeRpcUnchecked( CompositeNode rpcRequest ) {
+        return mountPoint.rpc( getRpcDefinition().getQName(), rpcRequest );
     }
 }
\ No newline at end of file
     }
 }
\ No newline at end of file
index 70ad7683a4cb6b5bd1a965c2d924de97b6ffd4af..a1b87325540e51a65b5918a9674148e9e2cbea26 100644 (file)
@@ -15,8 +15,6 @@ import static org.junit.Assert.assertThat;
 import java.util.HashMap;
 import java.util.Map;
 
 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;
 import org.hamcrest.BaseMatcher;
 import org.hamcrest.Description;
 import org.hamcrest.Matcher;
@@ -88,33 +86,34 @@ public class RestconfErrorTest {
     @Test
     public void testErrorTagStatusCodes()
     {
     @Test
     public void testErrorTagStatusCodes()
     {
-        Map<String,Status> lookUpMap = new HashMap<String,Status>();
-
-        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<String,Integer> lookUpMap = new HashMap<String,Integer>();
+
+        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() )
         {
 
         for( ErrorTag tag : ErrorTag.values() )
         {
-            Status expectedStatusCode = lookUpMap.get( tag.getTagValue() );
+            Integer expectedStatusCode = lookUpMap.get( tag.getTagValue() );
             assertNotNull( "Failed to find " + tag.getTagValue(), expectedStatusCode );
             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() ) );
         }
     }
 
         }
     }