BUG 1330 - list key counts|values diff in payload and URI 89/8889/3
authorJozef Gloncak <jgloncak@cisco.com>
Thu, 10 Jul 2014 08:02:00 +0000 (10:02 +0200)
committertpantelis <tpanteli@brocade.com>
Wed, 9 Jul 2014 21:14:17 +0000 (17:14 -0400)
Adds validation whether list key counts or list key values are different
between payload and URI. It difference occures then
RestconfDocumentedException is raised.

Change-Id: I93aaa352cfb8daee298ebf16375cb70987328dbb
Signed-off-by: Jozef Gloncak <jgloncak@cisco.com>
opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/ControllerContext.java
opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/RestconfImpl.java
opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/input/to/cnsn/test/RestPutListDataTest.java [new file with mode: 0644]
opendaylight/md-sal/sal-rest-connector/src/test/resources/full-versions/test-module/test-module

index 85c8e595396e9463b8fb7bf93437510041306b09..28e6a3c2472ee3b8723c82dc5d1050215c209bee 100644 (file)
@@ -696,8 +696,8 @@ public class ControllerContext implements SchemaContextListener {
             final int keysSize = listNode.getKeyDefinition().size();
             if( (strings.size() - consumed) < keysSize ) {
                 throw new RestconfDocumentedException(
             final int keysSize = listNode.getKeyDefinition().size();
             if( (strings.size() - consumed) < keysSize ) {
                 throw new RestconfDocumentedException(
-                        "Missing key for list \"" + listNode.getQName().getLocalName() + "\".",
-                        ErrorType.PROTOCOL, ErrorTag.INVALID_VALUE );
+                        "Missing key value for list element '" + listNode.getQName().getLocalName() + "' in the URI.",
+                        ErrorType.PROTOCOL, ErrorTag.DATA_MISSING );
             }
 
             final List<String> uriKeyValues = strings.subList( consumed, consumed + keysSize );
             }
 
             final List<String> uriKeyValues = strings.subList( consumed, consumed + keysSize );
index c8440c0a1692066660b0a6b2025abee6476c28a6..ae48ca7196904a135429bd9aae88626f947924d1 100644 (file)
@@ -52,6 +52,8 @@ import org.opendaylight.yangtools.yang.common.RpcResult;
 import org.opendaylight.yangtools.yang.data.api.CompositeNode;
 import org.opendaylight.yangtools.yang.data.api.InstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.InstanceIdentifier.InstanceIdentifierBuilder;
 import org.opendaylight.yangtools.yang.data.api.CompositeNode;
 import org.opendaylight.yangtools.yang.data.api.InstanceIdentifier;
 import org.opendaylight.yangtools.yang.data.api.InstanceIdentifier.InstanceIdentifierBuilder;
+import org.opendaylight.yangtools.yang.data.api.InstanceIdentifier.NodeIdentifierWithPredicates;
+import org.opendaylight.yangtools.yang.data.api.InstanceIdentifier.PathArgument;
 import org.opendaylight.yangtools.yang.data.api.MutableCompositeNode;
 import org.opendaylight.yangtools.yang.data.api.Node;
 import org.opendaylight.yangtools.yang.data.api.SimpleNode;
 import org.opendaylight.yangtools.yang.data.api.MutableCompositeNode;
 import org.opendaylight.yangtools.yang.data.api.Node;
 import org.opendaylight.yangtools.yang.data.api.SimpleNode;
@@ -674,6 +676,7 @@ public class RestconfImpl implements RestconfService {
 
         MountInstance mountPoint = iiWithData.getMountPoint();
         final CompositeNode value = this.normalizeNode(payload, iiWithData.getSchemaNode(), mountPoint);
 
         MountInstance mountPoint = iiWithData.getMountPoint();
         final CompositeNode value = this.normalizeNode(payload, iiWithData.getSchemaNode(), mountPoint);
+        validateListKeysEqualityInPayloadAndUri(iiWithData, payload);
         RpcResult<TransactionStatus> status = null;
 
         try {
         RpcResult<TransactionStatus> status = null;
 
         try {
@@ -695,6 +698,52 @@ public class RestconfImpl implements RestconfService {
         return Response.status(Status.INTERNAL_SERVER_ERROR).build();
     }
 
         return Response.status(Status.INTERNAL_SERVER_ERROR).build();
     }
 
+    /**
+     * Validates whether keys in {@code payload} are equal to values of keys in
+     * {@code iiWithData} for list schema node
+     *
+     * @throws RestconfDocumentedException
+     *             if key values or key count in payload and URI isn't equal
+     *
+     */
+    private void validateListKeysEqualityInPayloadAndUri(final InstanceIdWithSchemaNode iiWithData,
+            final CompositeNode payload) {
+        if (iiWithData.getSchemaNode() instanceof ListSchemaNode) {
+            final List<QName> keyDefinitions = ((ListSchemaNode) iiWithData.getSchemaNode()).getKeyDefinition();
+            final PathArgument lastPathArgument = iiWithData.getInstanceIdentifier().getLastPathArgument();
+            if (lastPathArgument instanceof NodeIdentifierWithPredicates) {
+                final Map<QName, Object> uriKeyValues = ((NodeIdentifierWithPredicates) lastPathArgument)
+                        .getKeyValues();
+                isEqualUriAndPayloadKeyValues(uriKeyValues, payload, keyDefinitions);
+            }
+        }
+    }
+
+    private void isEqualUriAndPayloadKeyValues(final Map<QName, Object> uriKeyValues, final CompositeNode payload,
+            final List<QName> keyDefinitions) {
+        for (QName keyDefinition : keyDefinitions) {
+            final Object uriKeyValue = uriKeyValues.get(keyDefinition);
+            // should be caught during parsing URI to InstanceIdentifier
+            if (uriKeyValue == null) {
+                throw new RestconfDocumentedException("Missing key " + keyDefinition + " in URI.",
+                        ErrorType.PROTOCOL, ErrorTag.DATA_MISSING);
+            }
+            final List<SimpleNode<?>> payloadKeyValues = payload.getSimpleNodesByName(keyDefinition.getLocalName());
+            if (payloadKeyValues.isEmpty()) {
+                throw new RestconfDocumentedException("Missing key " + keyDefinition.getLocalName()
+                        + " in the message body.", ErrorType.PROTOCOL,
+                        ErrorTag.DATA_MISSING);
+            }
+
+            Object payloadKeyValue = payloadKeyValues.iterator().next().getValue();
+            if (!uriKeyValue.equals(payloadKeyValue)) {
+                throw new RestconfDocumentedException("The value '"+uriKeyValue+ "' for key '" + keyDefinition.getLocalName() +
+                        "' specified in the URI doesn't match the value '" + payloadKeyValue + "' specified in the message body. ",
+                        ErrorType.PROTOCOL, ErrorTag.INVALID_VALUE);
+            }
+        }
+    }
+
     @Override
     public Response createConfigurationData(final String identifier, final CompositeNode payload) {
         if( payload == null ) {
     @Override
     public Response createConfigurationData(final String identifier, final CompositeNode payload) {
         if( payload == null ) {
@@ -1190,9 +1239,9 @@ public class RestconfImpl implements RestconfService {
 
                 if (!foundKey) {
                     throw new RestconfDocumentedException(
 
                 if (!foundKey) {
                     throw new RestconfDocumentedException(
-                            "Missing key in URI \"" + listKey.getLocalName() +
-                            "\" of list \"" + listSchemaNode.getQName().getLocalName() + "\"",
-                            ErrorType.PROTOCOL, ErrorTag.INVALID_VALUE );
+                            "Missing key '" + listKey.getLocalName() +
+                            "' for list '" + listSchemaNode.getQName().getLocalName() + "' in the message body",
+                            ErrorType.PROTOCOL, ErrorTag.DATA_MISSING );
                 }
             }
         }
                 }
             }
         }
diff --git a/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/input/to/cnsn/test/RestPutListDataTest.java b/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/input/to/cnsn/test/RestPutListDataTest.java
new file mode 100644 (file)
index 0000000..f4281c0
--- /dev/null
@@ -0,0 +1,189 @@
+/*
+ * Copyright (c) 2014 Cisco Systems, Inc. and others.  All rights reserved.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ */
+package org.opendaylight.controller.sal.restconf.impl.input.to.cnsn.test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.io.FileNotFoundException;
+import java.net.URI;
+import java.util.List;
+import org.junit.Before;
+import org.junit.Test;
+import org.opendaylight.controller.md.sal.common.api.TransactionStatus;
+import org.opendaylight.controller.sal.restconf.impl.BrokerFacade;
+import org.opendaylight.controller.sal.restconf.impl.CompositeNodeWrapper;
+import org.opendaylight.controller.sal.restconf.impl.ControllerContext;
+import org.opendaylight.controller.sal.restconf.impl.RestconfDocumentedException;
+import org.opendaylight.controller.sal.restconf.impl.RestconfError;
+import org.opendaylight.controller.sal.restconf.impl.RestconfError.ErrorTag;
+import org.opendaylight.controller.sal.restconf.impl.RestconfError.ErrorType;
+import org.opendaylight.controller.sal.restconf.impl.RestconfImpl;
+import org.opendaylight.controller.sal.restconf.impl.SimpleNodeWrapper;
+import org.opendaylight.controller.sal.restconf.impl.test.DummyFuture;
+import org.opendaylight.controller.sal.restconf.impl.test.DummyFuture.Builder;
+import org.opendaylight.controller.sal.restconf.impl.test.DummyRpcResult;
+import org.opendaylight.controller.sal.restconf.impl.test.TestUtils;
+import org.opendaylight.yangtools.yang.common.QName;
+import org.opendaylight.yangtools.yang.data.api.CompositeNode;
+import org.opendaylight.yangtools.yang.data.api.InstanceIdentifier;
+import org.opendaylight.yangtools.yang.data.impl.ImmutableCompositeNode;
+import org.opendaylight.yangtools.yang.data.impl.util.CompositeNodeBuilder;
+import org.opendaylight.yangtools.yang.model.api.SchemaContext;
+
+public class RestPutListDataTest {
+
+    private static BrokerFacade brokerFacade;
+    private static RestconfImpl restconfImpl;
+    private static SchemaContext schemaContextTestModule;
+
+    private static final String TEST_MODULE_NS_STRING = "test:module";
+    private static final URI TEST_MODULE_NS;
+    private static final String TEST_MODULE_REVISION = "2014-01-09";
+
+    static {
+        TEST_MODULE_NS = URI.create("test:module");
+    }
+
+    @Before
+    public void initialize() throws FileNotFoundException {
+        ControllerContext controllerContext = ControllerContext.getInstance();
+        schemaContextTestModule = TestUtils.loadSchemaContext("/full-versions/test-module");
+        controllerContext.setSchemas(schemaContextTestModule);
+        brokerFacade = mock(BrokerFacade.class);
+        restconfImpl = RestconfImpl.getInstance();
+        restconfImpl.setBroker(brokerFacade);
+        restconfImpl.setControllerContext(controllerContext);
+        Builder<TransactionStatus> futureBuilder = new DummyFuture.Builder<TransactionStatus>();
+        futureBuilder.rpcResult(new DummyRpcResult.Builder<TransactionStatus>().result(TransactionStatus.COMMITED)
+                .build());
+        when(brokerFacade.commitConfigurationDataPut(any(InstanceIdentifier.class), any(CompositeNode.class)))
+                .thenReturn(futureBuilder.build());
+    }
+
+    /**
+     * Tests whether no exception is raised if number and values of keys in URI
+     * and payload are equal
+     */
+    @Test
+    public void testValidKeys() {
+        putListDataTest("key1value", "15", "key1value", (short) 15);
+    }
+
+    /**
+     * Tests whether an exception is raised if key values in URI and payload are
+     * different.
+     *
+     * The exception should be raised from validation method
+     * {@code RestconfImpl#validateListEqualityOfListInDataAndUri}
+     */
+    @Test
+    public void testUriAndPayloadKeysDifferent() {
+        try {
+            putListDataTest("key1value", "15", "key1value", (short) 16);
+            fail("RestconfDocumentedException expected");
+        } catch (RestconfDocumentedException e) {
+            verifyException(e, ErrorType.PROTOCOL, ErrorTag.INVALID_VALUE);
+        }
+
+        try {
+            putListDataTest("key1value", "15", "key1value1", (short) 16);
+            fail("RestconfDocumentedException expected");
+        } catch (RestconfDocumentedException e) {
+            verifyException(e, ErrorType.PROTOCOL, ErrorTag.INVALID_VALUE);
+        }
+    }
+
+    /**
+     * Tests whether an exception is raised if URI contains less key values then
+     * payload.
+     *
+     * The exception is raised during {@code InstanceIdentifier} instance is
+     * built from URI
+     */
+    @Test
+    public void testMissingKeysInUri() {
+        try {
+            putListDataTest("key1value", null, "key1value", (short) 15);
+            fail("RestconfDocumentedException expected");
+        } catch (RestconfDocumentedException e) {
+            verifyException(e, ErrorType.PROTOCOL, ErrorTag.DATA_MISSING);
+        }
+    }
+
+    /**
+     * Tests whether an exception is raised if URI contains more key values then
+     * payload.
+     *
+     * The exception should be raised from validation method
+     * {@code RestconfImpl#validateListEqualityOfListInDataAndUri}
+     */
+    @Test
+    public void testMissingKeysInPayload() {
+        try {
+            putListDataTest("key1value", "15", "key1value", null);
+            fail("RestconfDocumentedException expected");
+        } catch (RestconfDocumentedException e) {
+            verifyException(e, ErrorType.PROTOCOL, ErrorTag.DATA_MISSING);
+        }
+        try {
+            putListDataWithWrapperTest("key1value", "15", "key1value", null);
+            fail("RestconfDocumentedException expected");
+        } catch (RestconfDocumentedException e) {
+            // this exception is raised from RestconfImpl.normalizeCompositeNode()
+            verifyException(e, ErrorType.PROTOCOL, ErrorTag.DATA_MISSING);
+        }
+
+    }
+
+    private void verifyException(final RestconfDocumentedException e, final ErrorType errorType, final ErrorTag errorTag) {
+        List<RestconfError> errors = e.getErrors();
+        assertEquals("getErrors() size", 1, errors.size());
+        assertEquals("RestconfError getErrorType()", errorType, errors.get(0).getErrorType());
+        assertEquals("RestconfError getErrorTag()", errorTag, errors.get(0).getErrorTag());
+    }
+
+    public void putListDataTest(final String uriKey1, final String uriKey2, final String payloadKey1,
+            final Short payloadKey2) {
+        QName lstWithCompositeKey = QName.create(TEST_MODULE_NS_STRING, TEST_MODULE_REVISION, "lst-with-composite-key");
+        QName key1 = QName.create(TEST_MODULE_NS_STRING, TEST_MODULE_REVISION, "key1");
+        QName key2 = QName.create(TEST_MODULE_NS_STRING, TEST_MODULE_REVISION, "key2");
+
+        CompositeNodeBuilder<ImmutableCompositeNode> payloadBuilder = ImmutableCompositeNode.builder();
+        payloadBuilder.setQName(lstWithCompositeKey).addLeaf(key1, payloadKey1);
+        if (payloadKey2 != null) {
+            payloadBuilder.addLeaf(key2, payloadKey2);
+        }
+
+        restconfImpl.updateConfigurationData(toUri(uriKey1, uriKey2), payloadBuilder.toInstance());
+    }
+
+    public void putListDataWithWrapperTest(final String uriKey1, final String uriKey2, final String payloadKey1,
+            final Short payloadKey2) {
+        CompositeNodeWrapper payloadBuilder = new CompositeNodeWrapper(TEST_MODULE_NS, "lst-with-composite-key");
+        payloadBuilder.addValue(new SimpleNodeWrapper(TEST_MODULE_NS, "key1", payloadKey1));
+        if (payloadKey2 != null) {
+            payloadBuilder.addValue(new SimpleNodeWrapper(TEST_MODULE_NS, "key2", payloadKey2));
+        }
+        restconfImpl.updateConfigurationData(toUri(uriKey1, uriKey2), payloadBuilder);
+    }
+
+    private String toUri(final String uriKey1, final String uriKey2) {
+        final StringBuilder uriBuilder = new StringBuilder("/test-module:lst-with-composite-key/");
+        uriBuilder.append(uriKey1);
+        if (uriKey2 != null) {
+            uriBuilder.append("/");
+            uriBuilder.append(uriKey2);
+        }
+        return uriBuilder.toString();
+    }
+
+}
index 2e533e720e0760f6abca5b84663c52ce170a4899..baf7d1e9ae7a9f61ccdf8b33cdefe728e4c16cee 100644 (file)
@@ -4,7 +4,7 @@ module test-module {
 
   revision 2014-01-09 {
   }
 
   revision 2014-01-09 {
   }
-  
+
   container interfaces {
     container class {
         leaf name {
   container interfaces {
     container class {
         leaf name {
@@ -18,7 +18,7 @@ module test-module {
         }
     }
   }
         }
     }
   }
-  
+
   container cont {
     container cont1 {
         leaf lf11 {
   container cont {
     container cont1 {
         leaf lf11 {
@@ -34,9 +34,18 @@ module test-module {
             type string;
         }
     }
             type string;
         }
     }
-  }   
-  
+  }
   
   
+  list lst-with-composite-key {
+    key "key1 key2";
+    leaf key1 {
+        type string;
+    }
+    leaf key2 {
+        type uint8;
+    }
+  }
+
   rpc rpc-test {
     input {
       container cont {
   rpc rpc-test {
     input {
       container cont {
@@ -48,16 +57,11 @@ module test-module {
                 type string;
             }
         }
                 type string;
             }
         }
-      }    
+      }
     }
     output {
         container cont-output {
         }
     }
     output {
         container cont-output {
         }
-    } 
-  
+    }
   }
   }
-  
-  
-
 }
\ No newline at end of file
 }
\ No newline at end of file