From e51312f71f5c2e964bc660b5ffb6f5089777ef51 Mon Sep 17 00:00:00 2001 From: Tom Pantelis Date: Wed, 14 Jun 2017 14:42:41 -0400 Subject: [PATCH] Remove UriInfo from JSONRestconfService API methods https://git.opendaylight.org/gerrit/#/c/48369/ added a UriInfo parameter to several JSONRestconfService API methods in Carbon. However this was an undocumented API change which will break downstream users. Further, there is no available implementation of UriInfo so each user would have to provide an implementation which would pretty much render the API unusable. We do not need to expose UriInfo in this API so I've remove the parameters. We'll need to cherry-pick to stable/carbon for SR1 to minimize breakage. Change-Id: I74285413c2a3a92510398a57ea82567b646c6d27 Signed-off-by: Tom Pantelis --- .../sal/restconf/api/JSONRestconfService.java | 7 +- .../impl/JSONRestconfServiceImpl.java | 181 +++++++++++++++--- .../test/JSONRestconfServiceImplTest.java | 69 +------ 3 files changed, 166 insertions(+), 91 deletions(-) diff --git a/restconf/sal-rest-connector/src/main/java/org/opendaylight/netconf/sal/restconf/api/JSONRestconfService.java b/restconf/sal-rest-connector/src/main/java/org/opendaylight/netconf/sal/restconf/api/JSONRestconfService.java index 3550ddee87..df3e3f1afb 100644 --- a/restconf/sal-rest-connector/src/main/java/org/opendaylight/netconf/sal/restconf/api/JSONRestconfService.java +++ b/restconf/sal-rest-connector/src/main/java/org/opendaylight/netconf/sal/restconf/api/JSONRestconfService.java @@ -9,7 +9,6 @@ package org.opendaylight.netconf.sal.restconf.api; import com.google.common.base.Optional; import javax.annotation.Nonnull; -import javax.ws.rs.core.UriInfo; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; import org.opendaylight.yangtools.yang.common.OperationFailedException; @@ -32,7 +31,7 @@ public interface JSONRestconfService { * @param payload the payload data in JSON format. * @throws OperationFailedException if the request fails. */ - void put(String uriPath, @Nonnull String payload, UriInfo uriInfo) throws OperationFailedException; + void put(String uriPath, @Nonnull String payload) throws OperationFailedException; /** * Issues a restconf POST request to the configuration data store. @@ -42,7 +41,7 @@ public interface JSONRestconfService { * @param payload the payload data in JSON format. * @throws OperationFailedException if the request fails. */ - void post(String uriPath, @Nonnull String payload, UriInfo uriInfo) throws OperationFailedException; + void post(String uriPath, @Nonnull String payload) throws OperationFailedException; /** * Issues a restconf DELETE request to the configuration data store. @@ -62,7 +61,7 @@ public interface JSONRestconfService { * @return an Optional containing the data in JSON format if present. * @throws OperationFailedException if the request fails. */ - Optional get(String uriPath, LogicalDatastoreType datastoreType, UriInfo uriInfo) + Optional get(String uriPath, LogicalDatastoreType datastoreType) throws OperationFailedException; /** diff --git a/restconf/sal-rest-connector/src/main/java/org/opendaylight/netconf/sal/restconf/impl/JSONRestconfServiceImpl.java b/restconf/sal-rest-connector/src/main/java/org/opendaylight/netconf/sal/restconf/impl/JSONRestconfServiceImpl.java index cb7e662581..f6359a68b8 100644 --- a/restconf/sal-rest-connector/src/main/java/org/opendaylight/netconf/sal/restconf/impl/JSONRestconfServiceImpl.java +++ b/restconf/sal-rest-connector/src/main/java/org/opendaylight/netconf/sal/restconf/impl/JSONRestconfServiceImpl.java @@ -14,9 +14,15 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; import java.lang.annotation.Annotation; +import java.net.URI; import java.nio.charset.StandardCharsets; +import java.util.Collections; import java.util.List; import javax.ws.rs.core.MediaType; +import javax.ws.rs.core.MultivaluedHashMap; +import javax.ws.rs.core.MultivaluedMap; +import javax.ws.rs.core.PathSegment; +import javax.ws.rs.core.UriBuilder; import javax.ws.rs.core.UriInfo; import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType; import org.opendaylight.netconf.sal.rest.impl.JsonNormalizedNodeBodyReader; @@ -36,12 +42,13 @@ import org.slf4j.LoggerFactory; * @author Thomas Pantelis */ public class JSONRestconfServiceImpl implements JSONRestconfService, AutoCloseable { - private final static Logger LOG = LoggerFactory.getLogger(JSONRestconfServiceImpl.class); + private static final Logger LOG = LoggerFactory.getLogger(JSONRestconfServiceImpl.class); private static final Annotation[] EMPTY_ANNOTATIONS = new Annotation[0]; + @SuppressWarnings("checkstyle:IllegalCatch") @Override - public void put(final String uriPath, final String payload, final UriInfo uriInfo) throws OperationFailedException { + public void put(final String uriPath, final String payload) throws OperationFailedException { Preconditions.checkNotNull(payload, "payload can't be null"); LOG.debug("put: uriPath: {}, payload: {}", uriPath, payload); @@ -53,14 +60,15 @@ public class JSONRestconfServiceImpl implements JSONRestconfService, AutoCloseab LOG.debug("Parsed NormalizedNode: {}", context.getData()); try { - RestconfImpl.getInstance().updateConfigurationData(uriPath, context, uriInfo); + RestconfImpl.getInstance().updateConfigurationData(uriPath, context, new SimpleUriInfo(uriPath)); } catch (final Exception e) { propagateExceptionAs(uriPath, e, "PUT"); } } + @SuppressWarnings("checkstyle:IllegalCatch") @Override - public void post(final String uriPath, final String payload, final UriInfo uriInfo) + public void post(final String uriPath, final String payload) throws OperationFailedException { Preconditions.checkNotNull(payload, "payload can't be null"); @@ -73,12 +81,13 @@ public class JSONRestconfServiceImpl implements JSONRestconfService, AutoCloseab LOG.debug("Parsed NormalizedNode: {}", context.getData()); try { - RestconfImpl.getInstance().createConfigurationData(uriPath, context, uriInfo); + RestconfImpl.getInstance().createConfigurationData(uriPath, context, new SimpleUriInfo(uriPath)); } catch (final Exception e) { propagateExceptionAs(uriPath, e, "POST"); } } + @SuppressWarnings("checkstyle:IllegalCatch") @Override public void delete(final String uriPath) throws OperationFailedException { LOG.debug("delete: uriPath: {}", uriPath); @@ -90,14 +99,16 @@ public class JSONRestconfServiceImpl implements JSONRestconfService, AutoCloseab } } + @SuppressWarnings("checkstyle:IllegalCatch") @Override - public Optional get(final String uriPath, final LogicalDatastoreType datastoreType, final UriInfo uriInfo) + public Optional get(final String uriPath, final LogicalDatastoreType datastoreType) throws OperationFailedException { LOG.debug("get: uriPath: {}", uriPath); try { NormalizedNodeContext readData; - if(datastoreType == LogicalDatastoreType.CONFIGURATION) { + final SimpleUriInfo uriInfo = new SimpleUriInfo(uriPath); + if (datastoreType == LogicalDatastoreType.CONFIGURATION) { readData = RestconfImpl.getInstance().readConfigurationData(uriPath, uriInfo); } else { readData = RestconfImpl.getInstance().readOperationalData(uriPath, uriInfo); @@ -109,7 +120,7 @@ public class JSONRestconfServiceImpl implements JSONRestconfService, AutoCloseab return result; } catch (final Exception e) { - if(!isDataMissing(e)) { + if (!isDataMissing(e)) { propagateExceptionAs(uriPath, e, "GET"); } @@ -118,8 +129,10 @@ public class JSONRestconfServiceImpl implements JSONRestconfService, AutoCloseab } } + @SuppressWarnings("checkstyle:IllegalCatch") @Override - public Optional invokeRpc(final String uriPath, final Optional input) throws OperationFailedException { + public Optional invokeRpc(final String uriPath, final Optional input) + throws OperationFailedException { Preconditions.checkNotNull(uriPath, "uriPath can't be null"); final String actualInput = input.isPresent() ? input.get() : null; @@ -129,9 +142,10 @@ public class JSONRestconfServiceImpl implements JSONRestconfService, AutoCloseab String output = null; try { NormalizedNodeContext outputContext; - if(actualInput != null) { + if (actualInput != null) { final InputStream entityStream = new ByteArrayInputStream(actualInput.getBytes(StandardCharsets.UTF_8)); - final NormalizedNodeContext inputContext = JsonNormalizedNodeBodyReader.readFrom(uriPath, entityStream, true); + final NormalizedNodeContext inputContext = + JsonNormalizedNodeBodyReader.readFrom(uriPath, entityStream, true); LOG.debug("Parsed YangInstanceIdentifier: {}", inputContext.getInstanceIdentifierContext() .getInstanceIdentifier()); @@ -142,7 +156,7 @@ public class JSONRestconfServiceImpl implements JSONRestconfService, AutoCloseab outputContext = RestconfImpl.getInstance().invokeRpc(uriPath, "", null); } - if(outputContext.getData() != null) { + if (outputContext.getData() != null) { output = toJson(outputContext); } } catch (final Exception e) { @@ -160,16 +174,16 @@ public class JSONRestconfServiceImpl implements JSONRestconfService, AutoCloseab final NormalizedNodeJsonBodyWriter writer = new NormalizedNodeJsonBodyWriter(); final ByteArrayOutputStream outputStream = new ByteArrayOutputStream(); writer.writeTo(readData, NormalizedNodeContext.class, null, EMPTY_ANNOTATIONS, - MediaType.APPLICATION_JSON_TYPE, null, outputStream ); + MediaType.APPLICATION_JSON_TYPE, null, outputStream); return outputStream.toString(StandardCharsets.UTF_8.name()); } - private static boolean isDataMissing(final Exception e) { + private static boolean isDataMissing(final Exception exception) { boolean dataMissing = false; - if (e instanceof RestconfDocumentedException) { - final RestconfDocumentedException rde = (RestconfDocumentedException)e; - if(!rde.getErrors().isEmpty()) { - if(rde.getErrors().get(0).getErrorTag() == ErrorTag.DATA_MISSING) { + if (exception instanceof RestconfDocumentedException) { + final RestconfDocumentedException rde = (RestconfDocumentedException)exception; + if (!rde.getErrors().isEmpty()) { + if (rde.getErrors().get(0).getErrorTag() == ErrorTag.DATA_MISSING) { dataMissing = true; } } @@ -178,22 +192,24 @@ public class JSONRestconfServiceImpl implements JSONRestconfService, AutoCloseab return dataMissing; } - private static void propagateExceptionAs(final String uriPath, final Exception e, final String operation) throws OperationFailedException { - LOG.debug("Error for uriPath: {}", uriPath, e); + private static void propagateExceptionAs(final String uriPath, final Exception exception, final String operation) + throws OperationFailedException { + LOG.debug("Error for uriPath: {}", uriPath, exception); - if(e instanceof RestconfDocumentedException) { - throw new OperationFailedException(String.format("%s failed for URI %s", operation, uriPath), e.getCause(), - toRpcErrors(((RestconfDocumentedException)e).getErrors())); + if (exception instanceof RestconfDocumentedException) { + throw new OperationFailedException(String.format( + "%s failed for URI %s", operation, uriPath), exception.getCause(), + toRpcErrors(((RestconfDocumentedException)exception).getErrors())); } - throw new OperationFailedException(String.format("%s failed for URI %s", operation, uriPath), e); + throw new OperationFailedException(String.format("%s failed for URI %s", operation, uriPath), exception); } private static RpcError[] toRpcErrors(final List from) { final RpcError[] to = new RpcError[from.size()]; - int i = 0; - for(final RestconfError e: from) { - to[i++] = RpcResultBuilder.newError(toRpcErrorType(e.getErrorType()), e.getErrorTag().getTagValue(), + int index = 0; + for (final RestconfError e: from) { + to[index++] = RpcResultBuilder.newError(toRpcErrorType(e.getErrorType()), e.getErrorTag().getTagValue(), e.getErrorMessage()); } @@ -201,7 +217,7 @@ public class JSONRestconfServiceImpl implements JSONRestconfService, AutoCloseab } private static ErrorType toRpcErrorType(final RestconfError.ErrorType errorType) { - switch(errorType) { + switch (errorType) { case TRANSPORT: { return ErrorType.TRANSPORT; } @@ -216,4 +232,113 @@ public class JSONRestconfServiceImpl implements JSONRestconfService, AutoCloseab } } } + + private static class SimpleUriInfo implements UriInfo { + private final String path; + private final MultivaluedMap queryParams; + + SimpleUriInfo(String path) { + this(path, new MultivaluedHashMap<>()); + } + + SimpleUriInfo(String path, MultivaluedMap queryParams) { + this.path = path; + this.queryParams = queryParams; + } + + @Override + public String getPath() { + return path; + } + + @Override + public String getPath(boolean decode) { + return path; + } + + @Override + public List getPathSegments() { + throw new UnsupportedOperationException(); + } + + @Override + public List getPathSegments(boolean decode) { + throw new UnsupportedOperationException(); + } + + @Override + public URI getRequestUri() { + return URI.create(path); + } + + @Override + public UriBuilder getRequestUriBuilder() { + return UriBuilder.fromUri(getRequestUri()); + } + + @Override + public URI getAbsolutePath() { + return getRequestUri(); + } + + @Override + public UriBuilder getAbsolutePathBuilder() { + return UriBuilder.fromUri(getAbsolutePath()); + } + + @Override + public URI getBaseUri() { + return URI.create(""); + } + + @Override + public UriBuilder getBaseUriBuilder() { + return UriBuilder.fromUri(getBaseUri()); + } + + @Override + public MultivaluedMap getPathParameters() { + return new MultivaluedHashMap<>(); + } + + @Override + public MultivaluedMap getPathParameters(boolean decode) { + return getPathParameters(); + } + + @Override + public MultivaluedMap getQueryParameters() { + return queryParams; + } + + @Override + public MultivaluedMap getQueryParameters(boolean decode) { + return getQueryParameters(); + } + + @Override + public List getMatchedURIs() { + return Collections.emptyList(); + } + + @Override + public List getMatchedURIs(boolean decode) { + return getMatchedURIs(); + } + + @Override + public List getMatchedResources() { + return Collections.emptyList(); + } + + @Override + public URI resolve(URI uri) { + return uri; + } + + @Override + public URI relativize(URI uri) { + return uri; + } + } } diff --git a/restconf/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/JSONRestconfServiceImplTest.java b/restconf/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/JSONRestconfServiceImplTest.java index fa788c61a0..7d6ff79d36 100644 --- a/restconf/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/JSONRestconfServiceImplTest.java +++ b/restconf/sal-rest-connector/src/test/java/org/opendaylight/controller/sal/restconf/impl/test/JSONRestconfServiceImplTest.java @@ -29,13 +29,9 @@ import com.google.common.util.concurrent.Futures; import java.io.FileNotFoundException; import java.io.IOException; import java.nio.charset.StandardCharsets; -import java.util.HashSet; import java.util.List; import java.util.Map; -import javax.ws.rs.core.MultivaluedMap; import javax.ws.rs.core.Response.Status; -import javax.ws.rs.core.UriBuilder; -import javax.ws.rs.core.UriInfo; import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; @@ -138,11 +134,7 @@ public class JSONRestconfServiceImplTest { when(result.getStatus()).thenReturn(Status.OK); final String uriPath = "ietf-interfaces:interfaces/interface/eth0"; final String payload = loadData("/parts/ietf-interfaces_interfaces.json"); - final UriInfo uriInfo = Mockito.mock(UriInfo.class); - final MultivaluedMap value = Mockito.mock(MultivaluedMap.class); - Mockito.when(value.entrySet()).thenReturn(new HashSet<>()); - Mockito.when(uriInfo.getQueryParameters()).thenReturn(value); - this.service.put(uriPath, payload, uriInfo); + this.service.put(uriPath, payload); final ArgumentCaptor capturedPath = ArgumentCaptor.forClass(YangInstanceIdentifier.class); final ArgumentCaptor capturedNode = ArgumentCaptor.forClass(NormalizedNode.class); @@ -175,11 +167,7 @@ public class JSONRestconfServiceImplTest { final String uriPath = "ietf-interfaces:interfaces/yang-ext:mount/test-module:cont/cont1"; final String payload = loadData("/full-versions/testCont1Data.json"); - final UriInfo uriInfo = Mockito.mock(UriInfo.class); - final MultivaluedMap value = Mockito.mock(MultivaluedMap.class); - Mockito.when(value.entrySet()).thenReturn(new HashSet<>()); - Mockito.when(uriInfo.getQueryParameters()).thenReturn(value); - this.service.put(uriPath, payload, uriInfo); + this.service.put(uriPath, payload); final ArgumentCaptor capturedPath = ArgumentCaptor.forClass(YangInstanceIdentifier.class); final ArgumentCaptor capturedNode = ArgumentCaptor.forClass(NormalizedNode.class); @@ -209,11 +197,7 @@ public class JSONRestconfServiceImplTest { final String uriPath = "ietf-interfaces:interfaces/interface/eth0"; final String payload = loadData("/parts/ietf-interfaces_interfaces.json"); - final UriInfo uriInfo = Mockito.mock(UriInfo.class); - final MultivaluedMap value = Mockito.mock(MultivaluedMap.class); - Mockito.when(value.entrySet()).thenReturn(new HashSet<>()); - Mockito.when(uriInfo.getQueryParameters()).thenReturn(value); - this.service.put(uriPath, payload, uriInfo); + this.service.put(uriPath, payload); } @SuppressWarnings("rawtypes") @@ -226,13 +210,7 @@ public class JSONRestconfServiceImplTest { final String uriPath = null; final String payload = loadData("/parts/ietf-interfaces_interfaces_absolute_path.json"); - final UriInfo uriInfo = Mockito.mock(UriInfo.class); - final MultivaluedMap value = Mockito.mock(MultivaluedMap.class); - Mockito.when(value.entrySet()).thenReturn(new HashSet<>()); - Mockito.when(uriInfo.getQueryParameters()).thenReturn(value); - final UriBuilder uriBuilder = UriBuilder.fromPath(""); - Mockito.when(uriInfo.getBaseUriBuilder()).thenReturn(uriBuilder); - this.service.post(uriPath, payload, uriInfo); + this.service.post(uriPath, payload); final ArgumentCaptor capturedPath = ArgumentCaptor.forClass(YangInstanceIdentifier.class); final ArgumentCaptor capturedNode = ArgumentCaptor.forClass(NormalizedNode.class); @@ -272,13 +250,7 @@ public class JSONRestconfServiceImplTest { final String uriPath = "ietf-interfaces:interfaces/yang-ext:mount/test-module:cont"; final String payload = loadData("/full-versions/testCont1Data.json"); - final UriInfo uriInfo = Mockito.mock(UriInfo.class); - final MultivaluedMap value = Mockito.mock(MultivaluedMap.class); - Mockito.when(value.entrySet()).thenReturn(new HashSet<>()); - Mockito.when(uriInfo.getQueryParameters()).thenReturn(value); - final UriBuilder uriBuilder = UriBuilder.fromPath(""); - Mockito.when(uriInfo.getBaseUriBuilder()).thenReturn(uriBuilder); - this.service.post(uriPath, payload, uriInfo); + this.service.post(uriPath, payload); final ArgumentCaptor capturedPath = ArgumentCaptor.forClass(YangInstanceIdentifier.class); final ArgumentCaptor capturedNode = ArgumentCaptor.forClass(NormalizedNode.class); @@ -303,15 +275,8 @@ public class JSONRestconfServiceImplTest { final String uriPath = null; final String payload = loadData("/parts/ietf-interfaces_interfaces_absolute_path.json"); - final UriInfo uriInfo = Mockito.mock(UriInfo.class); - final MultivaluedMap value = Mockito.mock(MultivaluedMap.class); - Mockito.when(value.entrySet()).thenReturn(new HashSet<>()); - Mockito.when(uriInfo.getQueryParameters()).thenReturn(value); - final UriBuilder uriBuilder = UriBuilder.fromPath(""); - Mockito.when(uriInfo.getBaseUriBuilder()).thenReturn(uriBuilder); - try { - this.service.post(uriPath, payload, uriInfo); + this.service.post(uriPath, payload); } catch (final OperationFailedException e) { assertNotNull(e.getCause()); throw e.getCause(); @@ -356,21 +321,13 @@ public class JSONRestconfServiceImplTest { doReturn(null).when(brokerFacade).readConfigurationData(notNull(YangInstanceIdentifier.class), Mockito.anyString()); final String uriPath = "ietf-interfaces:interfaces"; - final UriInfo uriInfo = Mockito.mock(UriInfo.class); - final MultivaluedMap value = Mockito.mock(MultivaluedMap.class); - Mockito.when(value.entrySet()).thenReturn(new HashSet<>()); - Mockito.when(uriInfo.getQueryParameters()).thenReturn(value); - this.service.get(uriPath, LogicalDatastoreType.CONFIGURATION, uriInfo); + this.service.get(uriPath, LogicalDatastoreType.CONFIGURATION); } @Test(expected=OperationFailedException.class) public void testGetFailure() throws Exception { final String invalidUriPath = "/ietf-interfaces:interfaces/invalid"; - final UriInfo uriInfo = Mockito.mock(UriInfo.class); - final MultivaluedMap value = Mockito.mock(MultivaluedMap.class); - Mockito.when(value.entrySet()).thenReturn(new HashSet<>()); - Mockito.when(uriInfo.getQueryParameters()).thenReturn(value); - this.service.get(invalidUriPath, LogicalDatastoreType.CONFIGURATION, uriInfo); + this.service.get(invalidUriPath, LogicalDatastoreType.CONFIGURATION); } @SuppressWarnings("rawtypes") @@ -465,14 +422,8 @@ public class JSONRestconfServiceImplTest { } final String uriPath = "/ietf-interfaces:interfaces/interface/eth0"; - final UriInfo uriInfo = Mockito.mock(UriInfo.class); - final MultivaluedMap value = Mockito.mock(MultivaluedMap.class); - Mockito.when(value.entrySet()).thenReturn(new HashSet<>()); - Mockito.when(uriInfo.getQueryParameters()).thenReturn(value); - Mockito.when(uriInfo.getQueryParameters(false)).thenReturn(value); - Mockito.when(value.getFirst("depth")).thenReturn(""); - - final Optional optionalResp = this.service.get(uriPath, datastoreType, uriInfo); + + final Optional optionalResp = this.service.get(uriPath, datastoreType); assertEquals("Response present", true, optionalResp.isPresent()); final String jsonResp = optionalResp.get(); -- 2.36.6