From: Robert Varga Date: Sun, 24 Oct 2021 09:09:55 +0000 (+0200) Subject: Rework query parameter parsing X-Git-Tag: v2.0.6~27 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?a=commitdiff_plain;h=1f9ce8d4cee5014da9b7651466a26f7caceebe4c;p=netconf.git Rework query parameter parsing Parsing code is structured so that things are not exactly nearby, fix that by things are close by. We also create better utilities to reduce code verbosity. JIRA: NETCONF-773 Change-Id: Id2244c08a1e8bfd27f1da3102c08da5a06691eb5 Signed-off-by: Robert Varga --- diff --git a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/legacy/QueryParameters.java b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/legacy/QueryParameters.java index a928a9ddd1..4fa5a521da 100644 --- a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/legacy/QueryParameters.java +++ b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/legacy/QueryParameters.java @@ -7,6 +7,8 @@ */ package org.opendaylight.restconf.nb.rfc8040.legacy; +import static java.util.Objects.requireNonNull; + import java.util.List; import java.util.Set; import org.eclipse.jdt.annotation.NonNull; @@ -24,9 +26,9 @@ import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; */ public final class QueryParameters { public static final class Builder { + private @NonNull ContentParameter content = ContentParameter.ALL; private List fieldPaths; private List> fields; - private ContentParameter content; private WithDefaultsParameter withDefault; private DepthParameter depth; private boolean prettyPrint; @@ -37,7 +39,7 @@ public final class QueryParameters { } public Builder setContent(final ContentParameter content) { - this.content = content; + this.content = requireNonNull(content); return this; } @@ -82,7 +84,7 @@ public final class QueryParameters { private final List fieldPaths; private final List> fields; private final WithDefaultsParameter withDefault; - private final ContentParameter content; + private final @NonNull ContentParameter content; private final DepthParameter depth; private final boolean prettyPrint; private final boolean tagged; @@ -105,7 +107,7 @@ public final class QueryParameters { return new Builder(); } - public ContentParameter getContent() { + public @NonNull ContentParameter getContent() { return content; } diff --git a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/ReadDataTransactionUtil.java b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/ReadDataTransactionUtil.java index 62b795d7e8..40ab0b3a49 100644 --- a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/ReadDataTransactionUtil.java +++ b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/ReadDataTransactionUtil.java @@ -7,6 +7,7 @@ */ package org.opendaylight.restconf.nb.rfc8040.rests.utils; +import static com.google.common.base.Verify.verifyNotNull; import static org.opendaylight.restconf.nb.rfc8040.utils.parser.ParserFieldsParameter.parseFieldsParameter; import static org.opendaylight.restconf.nb.rfc8040.utils.parser.ParserFieldsParameter.parseFieldsPaths; @@ -85,13 +86,8 @@ import org.opendaylight.yangtools.yang.model.api.RpcDefinition; * */ public final class ReadDataTransactionUtil { - private static final Set ALLOWED_PARAMETERS = Set.of( - ContentParameter.uriName(), - DepthParameter.uriName(), - FieldsParameter.uriName(), - WithDefaultsParameter.uriName()); - private static final List DEFAULT_CONTENT = List.of(ContentParameter.ALL.uriValue()); - private static final List DEFAULT_DEPTH = List.of(DepthParameter.unboundedUriValue()); + private static final Set ALLOWED_PARAMETERS = Set.of(ContentParameter.uriName(), DepthParameter.uriName(), + FieldsParameter.uriName(), WithDefaultsParameter.uriName()); private static final List POSSIBLE_CONTENT = Arrays.stream(ContentParameter.values()) .map(ContentParameter::uriValue) .collect(Collectors.toUnmodifiableList()); @@ -123,50 +119,44 @@ public final class ReadDataTransactionUtil { final MultivaluedMap queryParams = uriInfo.getQueryParameters(); checkParametersTypes(queryParams.keySet(), ALLOWED_PARAMETERS); - // read parameters from URI or set default values - final List content = queryParams.getOrDefault(ContentParameter.uriName(), DEFAULT_CONTENT); - final List depth = queryParams.getOrDefault(DepthParameter.uriName(), DEFAULT_DEPTH); - final List withDefaults = queryParams.getOrDefault(WithDefaultsParameter.uriName(), List.of()); - // fields - final List fields = queryParams.getOrDefault(FieldsParameter.uriName(), List.of()); - - // parameter can be in URI at most once - checkParameterCount(content, ContentParameter.uriName()); - checkParameterCount(depth, DepthParameter.uriName()); - checkParameterCount(fields, FieldsParameter.uriName()); - checkParameterCount(withDefaults, WithDefaultsParameter.uriName()); - // check and set content - final String contentStr = content.get(0); - builder.setContent(RestconfDocumentedException.throwIfNull( - ContentParameter.forUriValue(contentStr), ErrorType.PROTOCOL, ErrorTag.INVALID_VALUE, - "Invalid content parameter: %s, allowed values are %s", contentStr, POSSIBLE_CONTENT)); - - final String depthStr = depth.get(0); - try { - builder.setDepth(DepthParameter.forUriValue(depthStr)); - } catch (IllegalArgumentException e) { - throw new RestconfDocumentedException(e, new RestconfError(ErrorType.PROTOCOL, ErrorTag.INVALID_VALUE, - "Invalid depth parameter: " + depthStr, null, - "The depth parameter must be an integer between 1 and 65535 or \"unbounded\"")); + final String contentStr = getSingleParameter(queryParams, ContentParameter.uriName()); + if (contentStr != null) { + builder.setContent(RestconfDocumentedException.throwIfNull( + ContentParameter.forUriValue(contentStr), ErrorType.PROTOCOL, ErrorTag.INVALID_VALUE, + "Invalid content parameter: %s, allowed values are %s", contentStr, POSSIBLE_CONTENT)); + } + + // check and set depth + final String depthStr = getSingleParameter(queryParams, DepthParameter.uriName()); + if (depthStr != null) { + try { + builder.setDepth(DepthParameter.forUriValue(depthStr)); + } catch (IllegalArgumentException e) { + throw new RestconfDocumentedException(e, new RestconfError(ErrorType.PROTOCOL, ErrorTag.INVALID_VALUE, + "Invalid depth parameter: " + depthStr, null, + "The depth parameter must be an integer between 1 and 65535 or \"unbounded\"")); + } } // check and set fields - if (!fields.isEmpty()) { + final String fieldsStr = getSingleParameter(queryParams, FieldsParameter.uriName()); + if (fieldsStr != null) { + // FIXME: parse a FieldsParameter instead if (identifier.getMountPoint() != null) { - builder.setFieldPaths(parseFieldsPaths(identifier, fields.get(0))); + builder.setFieldPaths(parseFieldsPaths(identifier, fieldsStr)); } else { - builder.setFields(parseFieldsParameter(identifier, fields.get(0))); + builder.setFields(parseFieldsParameter(identifier, fieldsStr)); } } // check and set withDefaults parameter - if (!withDefaults.isEmpty()) { - final String str = withDefaults.get(0); - final WithDefaultsParameter val = WithDefaultsParameter.forUriValue(str); + final String withDefaultsStr = getSingleParameter(queryParams, WithDefaultsParameter.uriName()); + if (withDefaultsStr != null) { + final WithDefaultsParameter val = WithDefaultsParameter.forUriValue(withDefaultsStr); if (val == null) { throw new RestconfDocumentedException(new RestconfError(ErrorType.PROTOCOL, ErrorTag.INVALID_VALUE, - "Invalid with-defaults parameter: " + str, null, + "Invalid with-defaults parameter: " + withDefaultsStr, null, "The with-defaults parameter must be a string in " + POSSIBLE_WITH_DEFAULTS)); } @@ -180,6 +170,7 @@ public final class ReadDataTransactionUtil { builder.setWithDefault(val); } } + return builder.build(); } @@ -249,17 +240,20 @@ public final class ReadDataTransactionUtil { } } - /** - * Check if URI does not contain value for the same parameter more than once. - * - * @param parameterValues URI parameter values - * @param parameterName URI parameter name - */ @VisibleForTesting - static void checkParameterCount(final @NonNull List parameterValues, final @NonNull String parameterName) { - if (parameterValues.size() > 1) { - throw new RestconfDocumentedException( - "Parameter " + parameterName + " can appear at most once in request URI", + static @Nullable String getSingleParameter(final MultivaluedMap params, final String name) { + final var values = params.get(name); + if (values == null) { + return null; + } + + switch (values.size()) { + case 0: + return null; + case 1: + return verifyNotNull(values.get(0)); + default: + throw new RestconfDocumentedException("Parameter " + name + " can appear at most once in request URI", ErrorType.PROTOCOL, ErrorTag.INVALID_VALUE); } } @@ -271,8 +265,7 @@ public final class ReadDataTransactionUtil { * @param allowedParameters allowed parameters for operation */ @VisibleForTesting - static void checkParametersTypes(final @NonNull Set usedParameters, - final @NonNull Set allowedParameters) { + static void checkParametersTypes(final Set usedParameters, final Set allowedParameters) { if (!allowedParameters.containsAll(usedParameters)) { final Set notAllowedParameters = usedParameters.stream() .filter(param -> !allowedParameters.contains(param)) diff --git a/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/ReadDataTransactionUtilTest.java b/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/ReadDataTransactionUtilTest.java index 3761fdaeb0..b57a287c5b 100644 --- a/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/ReadDataTransactionUtilTest.java +++ b/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/ReadDataTransactionUtilTest.java @@ -347,12 +347,9 @@ public class ReadDataTransactionUtilTest { public void parseUriParametersUserDefinedTest() { final UriInfo uriInfo = mock(UriInfo.class); final MultivaluedHashMap parameters = new MultivaluedHashMap<>(); - - final String fields = containerChildQName.getLocalName(); - - parameters.put("content", List.of("config")); - parameters.put("depth", List.of("10")); - parameters.put("fields", List.of(fields)); + parameters.putSingle("content", "config"); + parameters.putSingle("depth", "10"); + parameters.putSingle("fields", containerChildQName.getLocalName()); when(uriInfo.getQueryParameters()).thenReturn(parameters); @@ -380,8 +377,7 @@ public class ReadDataTransactionUtilTest { public void parseUriParametersContentParameterNegativeTest() { final UriInfo uriInfo = mock(UriInfo.class); final MultivaluedHashMap parameters = new MultivaluedHashMap<>(); - - parameters.put("content", List.of("not-allowed-parameter-value")); + parameters.putSingle("content", "not-allowed-parameter-value"); when(uriInfo.getQueryParameters()).thenReturn(parameters); final RestconfDocumentedException ex = assertThrows(RestconfDocumentedException.class, @@ -400,7 +396,7 @@ public class ReadDataTransactionUtilTest { final MultivaluedHashMap parameters = new MultivaluedHashMap<>(); // inserted value is not allowed - parameters.put("depth", List.of("bounded")); + parameters.putSingle("depth", "bounded"); when(uriInfo.getQueryParameters()).thenReturn(parameters); RestconfDocumentedException ex = assertThrows(RestconfDocumentedException.class, @@ -419,7 +415,7 @@ public class ReadDataTransactionUtilTest { final MultivaluedHashMap parameters = new MultivaluedHashMap<>(); // inserted value is too low - parameters.put("depth", List.of("0")); + parameters.putSingle("depth", "0"); when(uriInfo.getQueryParameters()).thenReturn(parameters); RestconfDocumentedException ex = assertThrows(RestconfDocumentedException.class, @@ -438,7 +434,7 @@ public class ReadDataTransactionUtilTest { final MultivaluedHashMap parameters = new MultivaluedHashMap<>(); // inserted value is too high - parameters.put("depth", List.of("65536")); + parameters.putSingle("depth", "65536"); when(uriInfo.getQueryParameters()).thenReturn(parameters); RestconfDocumentedException ex = assertThrows(RestconfDocumentedException.class, @@ -457,7 +453,7 @@ public class ReadDataTransactionUtilTest { // preparation of input data final UriInfo uriInfo = mock(UriInfo.class); final MultivaluedHashMap parameters = new MultivaluedHashMap<>(); - parameters.put("with-defaults", List.of("explicit")); + parameters.putSingle("with-defaults", "explicit"); when(uriInfo.getQueryParameters()).thenReturn(parameters); final QueryParameters writerParameters = ReadDataTransactionUtil.parseUriParameters(context, uriInfo); @@ -473,7 +469,7 @@ public class ReadDataTransactionUtilTest { // preparation of input data final UriInfo uriInfo = mock(UriInfo.class); final MultivaluedHashMap parameters = new MultivaluedHashMap<>(); - parameters.put("with-defaults", List.of("invalid")); + parameters.putSingle("with-defaults", "invalid"); when(uriInfo.getQueryParameters()).thenReturn(parameters); final RestconfDocumentedException ex = assertThrows(RestconfDocumentedException.class, @@ -492,7 +488,7 @@ public class ReadDataTransactionUtilTest { // preparation of input data final UriInfo uriInfo = mock(UriInfo.class); final MultivaluedHashMap parameters = new MultivaluedHashMap<>(); - parameters.put("with-defaults", List.of("report-all-tagged")); + parameters.putSingle("with-defaults", "report-all-tagged"); when(uriInfo.getQueryParameters()).thenReturn(parameters); final QueryParameters writerParameters = ReadDataTransactionUtil.parseUriParameters(context, uriInfo); @@ -509,7 +505,7 @@ public class ReadDataTransactionUtilTest { // preparation of input data final UriInfo uriInfo = mock(UriInfo.class); final MultivaluedHashMap parameters = new MultivaluedHashMap<>(); - parameters.put("with-defaults", List.of("report-all")); + parameters.putSingle("with-defaults", "report-all"); when(uriInfo.getQueryParameters()).thenReturn(parameters); final QueryParameters writerParameters = ReadDataTransactionUtil.parseUriParameters(context, uriInfo); @@ -521,18 +517,22 @@ public class ReadDataTransactionUtilTest { * Test when parameter is present at most once. */ @Test - public void checkParameterCountTest() { - ReadDataTransactionUtil.checkParameterCount(List.of("all"), ContentParameter.uriName()); + public void getSingleParameterTest() { + final MultivaluedHashMap parameters = new MultivaluedHashMap<>(); + parameters.putSingle(ContentParameter.uriName(), "all"); + assertEquals("all", ReadDataTransactionUtil.getSingleParameter(parameters, ContentParameter.uriName())); } /** * Test when parameter is present more than once. */ @Test - public void checkParameterCountNegativeTest() { + public void getSingleParameterNegativeTest() { + final MultivaluedHashMap parameters = new MultivaluedHashMap<>(); + parameters.put(ContentParameter.uriName(), List.of("config", "nonconfig", "all")); + final RestconfDocumentedException ex = assertThrows(RestconfDocumentedException.class, - () -> ReadDataTransactionUtil.checkParameterCount(List.of("config", "nonconfig", "all"), - ContentParameter.uriName())); + () -> ReadDataTransactionUtil.getSingleParameter(parameters, ContentParameter.uriName())); final List errors = ex.getErrors(); assertEquals(1, errors.size()); @@ -541,7 +541,6 @@ public class ReadDataTransactionUtilTest { assertEquals("Error tag is not correct", ErrorTag.INVALID_VALUE, error.getErrorTag()); } - /** * Test when all parameters are allowed. */