From 0736b4a6b26e83d598057aa68c305271e2f8a656 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Mon, 25 Oct 2021 00:56:32 +0200 Subject: [PATCH] Refactor QueryParams.newReadDataParams() Follow the same layout as other methods, removing the need for getSingleParameter() and checkParametersTypes(). Also differentiate between unknown and illegal parameters. JIRA: NETCONF-773 Change-Id: I8c5db7d1f77f07cfc8a3be28ec3382fd3fb080ae Signed-off-by: Robert Varga --- .../rfc8040/databind/jaxrs/QueryParams.java | 159 ++++++++---------- .../databind/jaxrs/QueryParamsTest.java | 27 +-- 2 files changed, 73 insertions(+), 113 deletions(-) diff --git a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/databind/jaxrs/QueryParams.java b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/databind/jaxrs/QueryParams.java index fef0b87572..f3c766bb20 100644 --- a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/databind/jaxrs/QueryParams.java +++ b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/databind/jaxrs/QueryParams.java @@ -20,7 +20,6 @@ import java.util.Map.Entry; import java.util.Set; import java.util.function.Function; import java.util.stream.Collectors; -import javax.ws.rs.core.MultivaluedMap; import javax.ws.rs.core.UriInfo; import org.eclipse.jdt.annotation.NonNull; import org.eclipse.jdt.annotation.Nullable; @@ -64,7 +63,7 @@ public final class QueryParams { FilterParam filter = null; boolean skipNotificationData = false; - for (final Entry> entry : uriInfo.getQueryParameters().entrySet()) { + for (Entry> entry : uriInfo.getQueryParameters().entrySet()) { final String paramName = entry.getKey(); final List paramValues = entry.getValue(); @@ -81,7 +80,8 @@ public final class QueryParams { // FIXME: this should be properly encapsulated in SkipNotificatioDataParameter skipNotificationData = Boolean.parseBoolean(optionalParam(paramName, paramValues)); } else { - throw new RestconfDocumentedException("Bad parameter used with notifications: " + paramName); + throw new RestconfDocumentedException("Bad parameter used with notifications: " + paramName, + ErrorType.PROTOCOL, ErrorTag. UNKNOWN_ATTRIBUTE); } } catch (IllegalArgumentException e) { throw new RestconfDocumentedException("Invalid " + paramName + " value: " + e.getMessage(), e); @@ -114,78 +114,73 @@ public final class QueryParams { * @return {@link ReadDataParams} */ public static @NonNull ReadDataParams newReadDataParams(final UriInfo uriInfo) { - // check only allowed parameters - final MultivaluedMap queryParams = uriInfo.getQueryParameters(); - checkParametersTypes(queryParams.keySet(), ALLOWED_PARAMETERS); - - // check and set content - final String contentStr = getSingleParameter(queryParams, ContentParam.uriName()); - final ContentParam content = contentStr == null ? ContentParam.ALL - : RestconfDocumentedException.throwIfNull(ContentParam.forUriValue(contentStr), - ErrorType.PROTOCOL, ErrorTag.INVALID_VALUE, - "Invalid content parameter: %s, allowed values are %s", contentStr, POSSIBLE_CONTENT); - - // check and set depth - final DepthParam depth; - final String depthStr = getSingleParameter(queryParams, DepthParam.uriName()); - if (depthStr != null) { - try { - depth = DepthParam.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\"")); - } - } else { - depth = null; - } - - // check and set fields - final FieldsParam fields; - final String fieldsStr = getSingleParameter(queryParams, FieldsParam.uriName()); - if (fieldsStr != null) { - try { - fields = FieldsParam.parse(fieldsStr); - } catch (ParseException e) { - throw new RestconfDocumentedException(e, new RestconfError(ErrorType.PROTOCOL, ErrorTag.INVALID_VALUE, - "Invalid filds parameter: " + fieldsStr)); - } - } else { - fields = null; - } + ContentParam content = ContentParam.ALL; + DepthParam depth = null; + FieldsParam fields = null; + WithDefaultsParam withDefaults = null; + boolean tagged = false; - // check and set withDefaults parameter - final WithDefaultsParam withDefaults; - final boolean tagged; + for (Entry> entry : uriInfo.getQueryParameters().entrySet()) { + final String paramName = entry.getKey(); + final List paramValues = entry.getValue(); - final String withDefaultsStr = getSingleParameter(queryParams, WithDefaultsParam.uriName()); - if (withDefaultsStr != null) { - final WithDefaultsParam val = WithDefaultsParam.forUriValue(withDefaultsStr); - if (val == null) { - throw new RestconfDocumentedException(new RestconfError(ErrorType.PROTOCOL, ErrorTag.INVALID_VALUE, - "Invalid with-defaults parameter: " + withDefaultsStr, null, - "The with-defaults parameter must be a string in " + POSSIBLE_WITH_DEFAULTS)); - } + if (paramName.equals(ContentParam.uriName())) { + final String str = optionalParam(paramName, paramValues); + if (str != null) { + content = RestconfDocumentedException.throwIfNull(ContentParam.forUriValue(str), + ErrorType.PROTOCOL, ErrorTag.INVALID_VALUE, + "Invalid content parameter: %s, allowed values are %s", str, POSSIBLE_CONTENT); + } + } else if (paramName.equals(DepthParam.uriName())) { + final String str = optionalParam(paramName, paramValues); + try { + depth = DepthParam.forUriValue(str); + } catch (IllegalArgumentException e) { + throw new RestconfDocumentedException(e, new RestconfError(ErrorType.PROTOCOL, + ErrorTag.INVALID_VALUE, "Invalid depth parameter: " + str, null, + "The depth parameter must be an integer between 1 and 65535 or \"unbounded\"")); + } + } else if (paramName.equals(FieldsParam.uriName())) { + final String str = optionalParam(paramName, paramValues); + if (str != null) { + try { + fields = FieldsParam.parse(str); + } catch (ParseException e) { + throw new RestconfDocumentedException(e, new RestconfError(ErrorType.PROTOCOL, + ErrorTag.INVALID_VALUE, "Invalid filds parameter: " + str)); + } + } + } else if (paramName.equals(WithDefaultsParam.uriName())) { + final String str = optionalParam(paramName, paramValues); + if (str != null) { + final WithDefaultsParam val = WithDefaultsParam.forUriValue(str); + if (val == null) { + throw new RestconfDocumentedException(new RestconfError(ErrorType.PROTOCOL, + ErrorTag.INVALID_VALUE, "Invalid with-defaults parameter: " + str, null, + "The with-defaults parameter must be a string in " + POSSIBLE_WITH_DEFAULTS)); + } - switch (val) { - case REPORT_ALL: - withDefaults = null; - tagged = false; - break; - case REPORT_ALL_TAGGED: - withDefaults = null; - tagged = true; - break; - default: - withDefaults = val; - tagged = false; + switch (val) { + case REPORT_ALL: + withDefaults = null; + tagged = false; + break; + case REPORT_ALL_TAGGED: + withDefaults = null; + tagged = true; + break; + default: + withDefaults = val; + tagged = false; + } + } + } else { + // FIXME: recognize pretty-print here + throw new RestconfDocumentedException("Not allowed parameter for read operation: " + paramName, + ErrorType.PROTOCOL, ErrorTag.UNKNOWN_ATTRIBUTE); } - } else { - withDefaults = null; - tagged = false; } - // FIXME: recognize pretty-print here return ReadDataParams.of(content, depth, fields, withDefaults, tagged, false); } @@ -212,7 +207,7 @@ public final class QueryParams { } } else { throw new RestconfDocumentedException("Bad parameter for post: " + uriName, - ErrorType.PROTOCOL, ErrorTag.BAD_ELEMENT); + ErrorType.PROTOCOL, ErrorTag.UNKNOWN_ATTRIBUTE); } } @@ -223,30 +218,8 @@ public final class QueryParams { } } - /** - * Check if URI does not contain not allowed parameters for specified operation. - * - * @param usedParameters parameters used in URI request - * @param allowedParameters allowed parameters for operation - */ @VisibleForTesting - static void checkParametersTypes(final Set usedParameters, final Set allowedParameters) { - if (!allowedParameters.containsAll(usedParameters)) { - final Set notAllowedParameters = usedParameters.stream() - .filter(param -> !allowedParameters.contains(param)) - .collect(Collectors.toSet()); - throw new RestconfDocumentedException("Not allowed parameters for read operation: " + notAllowedParameters, - ErrorType.PROTOCOL, ErrorTag.INVALID_VALUE); - } - } - - @VisibleForTesting - static @Nullable String getSingleParameter(final MultivaluedMap params, final String name) { - final var values = params.get(name); - return values == null ? null : optionalParam(name, values); - } - - private static @Nullable String optionalParam(final String name, final List values) { + static @Nullable String optionalParam(final String name, final List values) { switch (values.size()) { case 0: return null; diff --git a/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/databind/jaxrs/QueryParamsTest.java b/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/databind/jaxrs/QueryParamsTest.java index bdf83183d2..4f2b51d38c 100644 --- a/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/databind/jaxrs/QueryParamsTest.java +++ b/restconf/restconf-nb-rfc8040/src/test/java/org/opendaylight/restconf/nb/rfc8040/databind/jaxrs/QueryParamsTest.java @@ -57,22 +57,17 @@ public class QueryParamsTest { * Test when parameter is present at most once. */ @Test - public void getSingleParameterTest() { - final MultivaluedHashMap parameters = new MultivaluedHashMap<>(); - parameters.putSingle(ContentParam.uriName(), "all"); - assertEquals("all", QueryParams.getSingleParameter(parameters, ContentParam.uriName())); + public void optionalParamTest() { + assertEquals("all", QueryParams.optionalParam(ContentParam.uriName(), List.of("all"))); } /** * Test when parameter is present more than once. */ @Test - public void getSingleParameterNegativeTest() { - final MultivaluedHashMap parameters = new MultivaluedHashMap<>(); - parameters.put(ContentParam.uriName(), List.of("config", "nonconfig", "all")); - + public void optionalParamMultipleTest() { final RestconfDocumentedException ex = assertThrows(RestconfDocumentedException.class, - () -> QueryParams.getSingleParameter(parameters, ContentParam.uriName())); + () -> QueryParams.optionalParam(ContentParam.uriName(), List.of("config", "nonconfig", "all"))); final List errors = ex.getErrors(); assertEquals(1, errors.size()); @@ -81,23 +76,15 @@ public class QueryParamsTest { assertEquals("Error tag is not correct", ErrorTag.INVALID_VALUE, error.getErrorTag()); } - /** - * Test when all parameters are allowed. - */ - @Test - public void checkParametersTypesTest() { - QueryParams.checkParametersTypes(Set.of("content"), - Set.of(ContentParam.uriName(), DepthParam.uriName())); - } - /** * Test when not allowed parameter type is used. */ @Test public void checkParametersTypesNegativeTest() { + mockQueryParameter("not-allowed-parameter", "does-not-matter"); + final RestconfDocumentedException ex = assertThrows(RestconfDocumentedException.class, - () -> QueryParams.checkParametersTypes(Set.of("not-allowed-parameter"), - Set.of(ContentParam.uriName(), DepthParam.uriName()))); + () -> QueryParams.newWriteDataParams(uriInfo)); final List errors = ex.getErrors(); assertEquals(1, errors.size()); -- 2.36.6