From d94951e2d26e87275f7af0aacec97eb805c6c1e3 Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Sat, 20 Jun 2015 04:09:47 -0400 Subject: [PATCH] Bug 3822: Improve error reporting for restconf PUT A runtime exception can be emitted by the netconf mount point which should be reported to the user, otherwise you get a 500 response with no error info which isn't very helpful. Also the fucntionality to output the error-info field was ommitted with the conversion from CompositeNode to NormalizedNode so I re-implemeneted it. It was originally ommitted with a TODO b/c the NormalizedNodeStreamWriters validate against the schema and error-info is defined as an empty container in the restconf yang. So there's no way to create a ContainerNode to represent the error-info data that conforms to the schema. To work around this, I created a leaf node and special-cased error-info in the stream writer to elide schema validation. I also added a regression unit test for the case where the URL contains an identityref. Change-Id: I93b4aea25c829af1232d539180f02dd61e252d50 Signed-off-by: Tom Pantelis --- .../RestconfDocumentedExceptionMapper.java | 159 +++++++++++++++++- .../sal/restconf/impl/RestconfImpl.java | 6 +- .../impl/test/RestGetOperationTest.java | 47 +++++- ...RestconfDocumentedExceptionMapperTest.java | 64 +------ .../test-module/test-module.yang | 29 +++- 5 files changed, 228 insertions(+), 77 deletions(-) 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 721864f973..2e4e00de90 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 @@ -12,6 +12,7 @@ import com.google.common.base.Charsets; import com.google.common.base.Preconditions; import com.google.common.base.Throwables; import com.google.common.collect.Iterables; +import com.google.gson.stream.JsonWriter; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.OutputStreamWriter; @@ -24,6 +25,7 @@ import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; import javax.ws.rs.ext.ExceptionMapper; import javax.ws.rs.ext.Provider; +import javax.xml.XMLConstants; import javax.xml.stream.FactoryConfigurationError; import javax.xml.stream.XMLOutputFactory; import javax.xml.stream.XMLStreamException; @@ -35,6 +37,7 @@ import org.opendaylight.controller.sal.restconf.impl.NormalizedNodeContext; import org.opendaylight.controller.sal.restconf.impl.RestconfDocumentedException; import org.opendaylight.controller.sal.restconf.impl.RestconfError; import org.opendaylight.yangtools.yang.common.QName; +import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.AugmentationIdentifier; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifier; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.NodeIdentifierWithPredicates; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier.PathArgument; @@ -45,7 +48,9 @@ import org.opendaylight.yangtools.yang.data.api.schema.MapNode; import org.opendaylight.yangtools.yang.data.api.schema.NormalizedNode; import org.opendaylight.yangtools.yang.data.api.schema.stream.NormalizedNodeStreamWriter; import org.opendaylight.yangtools.yang.data.api.schema.stream.NormalizedNodeWriter; +import org.opendaylight.yangtools.yang.data.codec.gson.JSONCodecFactory; import org.opendaylight.yangtools.yang.data.codec.gson.JSONNormalizedNodeStreamWriter; +import org.opendaylight.yangtools.yang.data.codec.gson.JsonWriterFactory; import org.opendaylight.yangtools.yang.data.impl.codec.xml.XMLStreamNormalizedNodeStreamWriter; import org.opendaylight.yangtools.yang.data.impl.schema.Builders; import org.opendaylight.yangtools.yang.data.impl.schema.ImmutableNodes; @@ -188,7 +193,15 @@ public class RestconfDocumentedExceptionMapper implements ExceptionMapper pathContext = errorsNode.getInstanceIdentifierContext(); final ByteArrayOutputStream outStream = new ByteArrayOutputStream(); - XMLStreamWriter xmlWriter; + final XMLStreamWriter xmlWriter; try { xmlWriter = XML_FACTORY.createXMLStreamWriter(outStream, "UTF-8"); } catch (final XMLStreamException e) { @@ -262,8 +296,33 @@ public class RestconfDocumentedExceptionMapper implements ExceptionMapper parameters = new ArrayList<>(); - final Date revision = new SimpleDateFormat("yyyy-MM-dd").parse("2014-01-09"); - final URI uri = new URI("test:module"); - final QName qNameCont = QName.create(uri, revision, "cont"); - final QName qNameList = QName.create(uri, revision, "lst1"); - final QName qNameKeyList = QName.create(uri, revision, "lf11"); + final QName qNameCont = newTestModuleQName("cont"); + final QName qNameList = newTestModuleQName("lst1"); + final QName qNameKeyList = newTestModuleQName("lf11"); parameters.add(new YangInstanceIdentifier.NodeIdentifier(qNameCont)); parameters.add(new YangInstanceIdentifier.NodeIdentifier(qNameList)); @@ -226,6 +227,12 @@ public class RestGetOperationTest extends JerseyTest { return YangInstanceIdentifier.create(parameters); } + private QName newTestModuleQName(String localPart) throws Exception { + final Date revision = new SimpleDateFormat("yyyy-MM-dd").parse("2014-01-09"); + final URI uri = new URI("test:module"); + return QName.create(uri, revision, localPart); + } + @Test public void getDataMountPointIntoHighestElement() throws UnsupportedEncodingException, URISyntaxException, ParseException { @@ -242,6 +249,30 @@ public class RestGetOperationTest extends JerseyTest { assertEquals(200, get(uri, MediaType.APPLICATION_XML)); } + @SuppressWarnings("unchecked") + @Test + public void getDataWithIdentityrefInURL() throws Exception { + setControllerContext(schemaContextTestModule); + + QName moduleQN = newTestModuleQName("module"); + ImmutableMap keyMap = ImmutableMap.builder() + .put(newTestModuleQName("type"), newTestModuleQName("test-identity")) + .put(newTestModuleQName("name"), "foo").build(); + YangInstanceIdentifier iid = YangInstanceIdentifier.builder().node(newTestModuleQName("modules")) + .node(moduleQN).nodeWithKey(moduleQN, keyMap).build(); + @SuppressWarnings("rawtypes") + NormalizedNode data = ImmutableMapNodeBuilder.create().withNodeIdentifier( + new NodeIdentifier(moduleQN)).withChild(ImmutableNodes.mapEntryBuilder() + .withNodeIdentifier(new NodeIdentifierWithPredicates(moduleQN, keyMap)) + .withChild(ImmutableNodes.leafNode(newTestModuleQName("type"), newTestModuleQName("test-identity"))) + .withChild(ImmutableNodes.leafNode(newTestModuleQName("name"), "foo")) + .withChild(ImmutableNodes.leafNode(newTestModuleQName("data"), "bar")).build()).build(); + when(brokerFacade.readConfigurationData(iid)).thenReturn(data); + + String uri = "/config/test-module:modules/module/test-module:test-identity/foo"; + assertEquals(200, get(uri, MediaType.APPLICATION_XML)); + } + // /modules @Test public void getModulesTest() throws UnsupportedEncodingException, FileNotFoundException { diff --git a/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/RestconfDocumentedExceptionMapperTest.java b/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/RestconfDocumentedExceptionMapperTest.java index 0244aa7f2d..9edd9525a3 100644 --- a/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/RestconfDocumentedExceptionMapperTest.java +++ b/opendaylight/md-sal/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/RestconfDocumentedExceptionMapperTest.java @@ -18,7 +18,6 @@ import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.reset; import static org.mockito.Mockito.when; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.Maps; import com.google.common.io.ByteStreams; import com.google.gson.JsonArray; @@ -68,7 +67,6 @@ 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.w3c.dom.Document; -import org.w3c.dom.Element; import org.w3c.dom.Node; import org.w3c.dom.NodeList; @@ -85,60 +83,6 @@ public class RestconfDocumentedExceptionMapperTest extends JerseyTest { void verifyJson(JsonElement errorInfoElement); } - static class ComplexErrorInfoVerifier implements ErrorInfoVerifier { - - Map expErrorInfo; - - public ComplexErrorInfoVerifier(final Map expErrorInfo) { - this.expErrorInfo = expErrorInfo; - } - - @Override - public void verifyXML(final Node errorInfoNode) { - - final Map mutableExpMap = Maps.newHashMap(expErrorInfo); - final NodeList childNodes = errorInfoNode.getChildNodes(); - for (int i = 0; i < childNodes.getLength(); i++) { - final Node child = childNodes.item(i); - if (child instanceof Element) { - final String expValue = mutableExpMap.remove(child.getNodeName()); - assertNotNull("Found unexpected \"error-info\" child node: " + child.getNodeName(), expValue); - assertEquals("Text content for \"error-info\" child node " + child.getNodeName(), expValue, - child.getTextContent()); - } - } - - if (!mutableExpMap.isEmpty()) { - fail("Missing \"error-info\" child nodes: " + mutableExpMap); - } - } - - @Override - public void verifyJson(final JsonElement errorInfoElement) { - - assertTrue("\"error-info\" Json element is not an Object", errorInfoElement.isJsonObject()); - - final Map actualErrorInfo = Maps.newHashMap(); - for (final Entry entry : errorInfoElement.getAsJsonObject().entrySet()) { - final String leafName = entry.getKey(); - final JsonElement leafElement = entry.getValue(); - actualErrorInfo.put(leafName, leafElement.getAsString()); - } - - final Map mutableExpMap = Maps.newHashMap(expErrorInfo); - for (final Entry actual : actualErrorInfo.entrySet()) { - final String expValue = mutableExpMap.remove(actual.getKey()); - assertNotNull("Found unexpected \"error-info\" child node: " + actual.getKey(), expValue); - assertEquals("Text content for \"error-info\" child node " + actual.getKey(), expValue, - actual.getValue()); - } - - if (!mutableExpMap.isEmpty()) { - fail("Missing \"error-info\" child nodes: " + mutableExpMap); - } - } - } - static class SimpleErrorInfoVerifier implements ErrorInfoVerifier { String expTextContent; @@ -432,18 +376,16 @@ public class RestconfDocumentedExceptionMapperTest extends JerseyTest { } @Test - @Ignore // TODO : we are not supported "error-info" element yet public void testToJsonResponseWithErrorInfo() throws Exception { final String errorInfo = "
1.2.3.4
123"; testJsonResponse(new RestconfDocumentedException(new RestconfError(ErrorType.APPLICATION, ErrorTag.INVALID_VALUE, "mock error", "mock-app-tag", errorInfo)), Status.BAD_REQUEST, ErrorType.APPLICATION, ErrorTag.INVALID_VALUE, "mock error", "mock-app-tag", - new ComplexErrorInfoVerifier(ImmutableMap.of("session-id", "123", "address", "1.2.3.4"))); + new SimpleErrorInfoVerifier(errorInfo)); } @Test - @Ignore //TODO : we are not supporting "error-info" yet public void testToJsonResponseWithExceptionCause() throws Exception { final Exception cause = new Exception("mock exception cause"); @@ -634,18 +576,16 @@ public class RestconfDocumentedExceptionMapperTest extends JerseyTest { } @Test - @Ignore // TODO : we are not supporting "error-info" node yet public void testToXMLResponseWithErrorInfo() throws Exception { final String errorInfo = "
1.2.3.4
123"; testXMLResponse(new RestconfDocumentedException(new RestconfError(ErrorType.APPLICATION, ErrorTag.INVALID_VALUE, "mock error", "mock-app-tag", errorInfo)), Status.BAD_REQUEST, ErrorType.APPLICATION, ErrorTag.INVALID_VALUE, "mock error", "mock-app-tag", - new ComplexErrorInfoVerifier(ImmutableMap.of("session-id", "123", "address", "1.2.3.4"))); + new SimpleErrorInfoVerifier(errorInfo)); } @Test - @Ignore // TODO : we are not supporting "error-info" node yet public void testToXMLResponseWithExceptionCause() throws Exception { final Exception cause = new Exception("mock exception cause"); diff --git a/opendaylight/md-sal/sal-rest-connector/src/test/resources/full-versions/test-module/test-module.yang b/opendaylight/md-sal/sal-rest-connector/src/test/resources/full-versions/test-module/test-module.yang index efc5e2c4ec..2cc78b38fa 100644 --- a/opendaylight/md-sal/sal-rest-connector/src/test/resources/full-versions/test-module/test-module.yang +++ b/opendaylight/md-sal/sal-rest-connector/src/test/resources/full-versions/test-module/test-module.yang @@ -5,6 +5,12 @@ module test-module { revision 2014-01-09 { } + identity module-type { + } + + identity test-identity { + } + container interfaces { container class { leaf name { @@ -35,7 +41,28 @@ module test-module { } } } - + + container modules { + list module { + key "type name"; + leaf name { + type string; + mandatory true; + } + + leaf type { + type identityref { + base module-type; + } + mandatory true; + } + + leaf data { + type string; + } + } + } + list lst-with-composite-key { key "key1 key2"; leaf key1 { -- 2.36.6