From: Jozef Gloncak Date: Thu, 10 Jul 2014 08:02:00 +0000 (+0200) Subject: BUG 1330 - list key counts|values diff in payload and URI X-Git-Tag: release/helium~490^2 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=3b721d88ed1083a463ecb73a6050de4bfedf1a78;hp=ce89a7ff4059902375f73aee481920191cdb84a5 BUG 1330 - list key counts|values diff in payload and URI 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 --- diff --git a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/ControllerContext.java b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/ControllerContext.java index 85c8e59539..28e6a3c247 100644 --- a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/ControllerContext.java +++ b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/ControllerContext.java @@ -696,8 +696,8 @@ public class ControllerContext implements SchemaContextListener { 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 uriKeyValues = strings.subList( consumed, consumed + keysSize ); diff --git a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/RestconfImpl.java b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/RestconfImpl.java index c8440c0a16..ae48ca7196 100644 --- a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/RestconfImpl.java +++ b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/restconf/impl/RestconfImpl.java @@ -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.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; @@ -674,6 +676,7 @@ public class RestconfImpl implements RestconfService { MountInstance mountPoint = iiWithData.getMountPoint(); final CompositeNode value = this.normalizeNode(payload, iiWithData.getSchemaNode(), mountPoint); + validateListKeysEqualityInPayloadAndUri(iiWithData, payload); RpcResult status = null; try { @@ -695,6 +698,52 @@ public class RestconfImpl implements RestconfService { 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 keyDefinitions = ((ListSchemaNode) iiWithData.getSchemaNode()).getKeyDefinition(); + final PathArgument lastPathArgument = iiWithData.getInstanceIdentifier().getLastPathArgument(); + if (lastPathArgument instanceof NodeIdentifierWithPredicates) { + final Map uriKeyValues = ((NodeIdentifierWithPredicates) lastPathArgument) + .getKeyValues(); + isEqualUriAndPayloadKeyValues(uriKeyValues, payload, keyDefinitions); + } + } + } + + private void isEqualUriAndPayloadKeyValues(final Map uriKeyValues, final CompositeNode payload, + final List 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> 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 ) { @@ -1190,9 +1239,9 @@ public class RestconfImpl implements RestconfService { 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 index 0000000000..f4281c037c --- /dev/null +++ b/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/input/to/cnsn/test/RestPutListDataTest.java @@ -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 futureBuilder = new DummyFuture.Builder(); + futureBuilder.rpcResult(new DummyRpcResult.Builder().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 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 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(); + } + +} diff --git a/opendaylight/md-sal/sal-rest-connector/src/test/resources/full-versions/test-module/test-module b/opendaylight/md-sal/sal-rest-connector/src/test/resources/full-versions/test-module/test-module index 2e533e720e..baf7d1e9ae 100644 --- a/opendaylight/md-sal/sal-rest-connector/src/test/resources/full-versions/test-module/test-module +++ b/opendaylight/md-sal/sal-rest-connector/src/test/resources/full-versions/test-module/test-module @@ -4,7 +4,7 @@ module test-module { revision 2014-01-09 { } - + container interfaces { container class { leaf name { @@ -18,7 +18,7 @@ module test-module { } } } - + container cont { container cont1 { leaf lf11 { @@ -34,9 +34,18 @@ module test-module { type string; } } - } - + } + list lst-with-composite-key { + key "key1 key2"; + leaf key1 { + type string; + } + leaf key2 { + type uint8; + } + } + rpc rpc-test { input { container cont { @@ -48,16 +57,11 @@ module test-module { type string; } } - } + } } output { container cont-output { } - } - + } } - - - - } \ No newline at end of file