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 <robert.varga@pantheon.tech>

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 fef0b8757226d96b39abf6ccbde0753dd019cc36..f3c766bb20d67838d4be7298134f16b969133d1d 100644 (file)
--- 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<String, List<String>> entry : uriInfo.getQueryParameters().entrySet()) {
+        for (Entry<String, List<String>> entry : uriInfo.getQueryParameters().entrySet()) {
             final String paramName = entry.getKey();
             final List<String> 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<String, String> 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<String, List<String>> entry : uriInfo.getQueryParameters().entrySet()) {
+            final String paramName = entry.getKey();
+            final List<String> 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<String> usedParameters, final Set<String> allowedParameters) {
-        if (!allowedParameters.containsAll(usedParameters)) {
-            final Set<String> 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<String, String> 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<String> values) {
+    static @Nullable String optionalParam(final String name, final List<String> 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 bdf83183d289249951193ebf64aeb7392ba3ba0e..4f2b51d38cbd469e9421e2c71e2ab76c1d56a153 100644 (file)
--- 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<String, String> 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<String, String> 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<RestconfError> 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<RestconfError> errors = ex.getErrors();
         assertEquals(1, errors.size());