From 9dc2521b497bec737e2c32398c5f2764f5cdb1a6 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Tue, 26 Oct 2021 14:16:37 +0200 Subject: [PATCH] Unify parameter parsing We have a number of different ways to parse a query parameter, make the contracts more consistent. JIRA: NETCONF-773 Change-Id: I7c3090998efaeb9b7d713e595640a6d1e156fc65 Signed-off-by: Robert Varga --- .../restconf/nb/rfc8040/ContentParam.java | 7 +- .../restconf/nb/rfc8040/FieldsParam.java | 8 + .../restconf/nb/rfc8040/InsertParam.java | 7 +- .../restconf/nb/rfc8040/PointParam.java | 10 +- .../nb/rfc8040/WithDefaultsParam.java | 6 +- .../rfc8040/databind/jaxrs/QueryParams.java | 148 +++++++----------- 6 files changed, 81 insertions(+), 105 deletions(-) diff --git a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/ContentParam.java b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/ContentParam.java index 6ed8e20f1f..75f2255ad4 100644 --- a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/ContentParam.java +++ b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/ContentParam.java @@ -10,7 +10,6 @@ package org.opendaylight.restconf.nb.rfc8040; import static java.util.Objects.requireNonNull; import org.eclipse.jdt.annotation.NonNull; -import org.eclipse.jdt.annotation.Nullable; /** * Enumeration of possible {@code content} values as defined by @@ -55,8 +54,7 @@ public enum ContentParam implements RestconfQueryParam { return uriValue; } - // Note: returns null of unknowns - public static @Nullable ContentParam forUriValue(final String uriValue) { + public static @NonNull ContentParam forUriValue(final String uriValue) { switch (uriValue) { case "all": return ALL; @@ -65,7 +63,8 @@ public enum ContentParam implements RestconfQueryParam { case "nonconfig": return NONCONFIG; default: - return null; + throw new IllegalArgumentException( + "Value can be 'all', 'config' or 'non-config', not '" + uriValue + "'"); } } } diff --git a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/FieldsParam.java b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/FieldsParam.java index d6b96e1cac..3142a9372c 100644 --- a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/FieldsParam.java +++ b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/FieldsParam.java @@ -113,6 +113,14 @@ public final class FieldsParam implements RestconfQueryParam { return new FieldsParameterParser().parse(str); } + public static FieldsParam forUriValue(final String uriValue) { + try { + return parse(uriValue); + } catch (ParseException e) { + throw new IllegalArgumentException(e.getMessage() + " [at offset " + e.getErrorOffset() + "]", e); + } + } + @Override public Class<@NonNull FieldsParam> javaClass() { return FieldsParam.class; diff --git a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/InsertParam.java b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/InsertParam.java index 4c60198c86..829a06a8b7 100644 --- a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/InsertParam.java +++ b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/InsertParam.java @@ -10,7 +10,6 @@ package org.opendaylight.restconf.nb.rfc8040; import static java.util.Objects.requireNonNull; import org.eclipse.jdt.annotation.NonNull; -import org.eclipse.jdt.annotation.Nullable; /** * Enumeration of possible {@code insert} values as defined by @@ -59,8 +58,7 @@ public enum InsertParam implements RestconfQueryParam { return uriValue; } - // Note: returns null of unknowns - public static @Nullable InsertParam forUriValue(final String uriValue) { + public static @NonNull InsertParam forUriValue(final String uriValue) { switch (uriValue) { case "after": return AFTER; @@ -71,7 +69,8 @@ public enum InsertParam implements RestconfQueryParam { case "last": return LAST; default: - return null; + throw new IllegalArgumentException( + "Value can be 'after', 'before', 'first' or 'last', not '" + uriValue + "'"); } } } \ No newline at end of file diff --git a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/PointParam.java b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/PointParam.java index 9e9c3b9b4b..a80135f291 100644 --- a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/PointParam.java +++ b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/PointParam.java @@ -10,20 +10,18 @@ package org.opendaylight.restconf.nb.rfc8040; import static java.util.Objects.requireNonNull; import org.eclipse.jdt.annotation.NonNull; -import org.eclipse.jdt.annotation.NonNullByDefault; /** * This class represents a {@code point} parameter as defined in * RFC8040 section 4.8.4. */ -@NonNullByDefault public final class PointParam implements RestconfQueryParam { // API consistency: must not be confused with enum constants @SuppressWarnings("checkstyle:ConstantName") - public static final String uriName = "point"; + public static final @NonNull String uriName = "point"; // FIXME: This should be ApiPath - private final String value; + private final @NonNull String value; private PointParam(final String value) { this.value = requireNonNull(value); @@ -44,11 +42,11 @@ public final class PointParam implements RestconfQueryParam { return value; } - public static PointParam forUriValue(final String uriValue) { + public static @NonNull PointParam forUriValue(final String uriValue) { return new PointParam(uriValue); } - public String value() { + public @NonNull String value() { return value; } } diff --git a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/WithDefaultsParam.java b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/WithDefaultsParam.java index aa9f7ac045..6caa650c83 100644 --- a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/WithDefaultsParam.java +++ b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/WithDefaultsParam.java @@ -11,7 +11,6 @@ import static java.util.Objects.requireNonNull; import java.net.URI; import org.eclipse.jdt.annotation.NonNull; -import org.eclipse.jdt.annotation.Nullable; /** * Enumeration of possible {@code with-defaults} parameter values as defined by @@ -62,7 +61,7 @@ public enum WithDefaultsParam implements RestconfQueryParam { return uriValue; } - public static @Nullable WithDefaultsParam forUriValue(final String uriValue) { + public static @NonNull WithDefaultsParam forUriValue(final String uriValue) { switch (uriValue) { case "explicit": return EXPLICIT; @@ -73,7 +72,8 @@ public enum WithDefaultsParam implements RestconfQueryParam { case "trim": return TRIM; default: - return null; + throw new IllegalArgumentException( + "Value can be 'explicit', 'report-all', 'report-all-tagged' or 'trim', not '" + uriValue + "'"); } } 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 9d7b7b459b..f2e2998a6d 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 @@ -11,13 +11,10 @@ import static java.util.Objects.requireNonNull; import com.google.common.annotations.Beta; import com.google.common.annotations.VisibleForTesting; -import java.text.ParseException; -import java.util.Arrays; import java.util.List; import java.util.Map.Entry; import java.util.Set; import java.util.function.Function; -import java.util.stream.Collectors; import javax.ws.rs.core.UriInfo; import org.eclipse.jdt.annotation.NonNull; import org.eclipse.jdt.annotation.Nullable; @@ -47,20 +44,15 @@ import org.opendaylight.yangtools.yang.common.ErrorType; @Beta public final class QueryParams { - private static final List POSSIBLE_CONTENT = Arrays.stream(ContentParam.values()) - .map(ContentParam::paramValue) - .collect(Collectors.toUnmodifiableList()); - private static final List POSSIBLE_WITH_DEFAULTS = Arrays.stream(WithDefaultsParam.values()) - .map(WithDefaultsParam::paramValue) - .collect(Collectors.toUnmodifiableList()); private static final Set KNOWN_PARAMS = Set.of( // Read data ContentParam.uriName, DepthParam.uriName, FieldsParam.uriName, WithDefaultsParam.uriName, + PrettyPrintParam.uriName, // Modify data InsertParam.uriName, PointParam.uriName, // Notifications - FilterParam.uriName, StartTimeParam.uriName, StopTimeParam.uriName ,"odl-skip-notification-data"); - + FilterParam.uriName, StartTimeParam.uriName, StopTimeParam.uriName, + LeafNodesOnlyParam.uriName, SkipNotificationDataParam.uriName); private QueryParams() { // Utility class @@ -141,66 +133,51 @@ public final class QueryParams { final String paramName = entry.getKey(); final List paramValues = entry.getValue(); - switch (paramName) { - case ContentParam.uriName: - final String contentStr = optionalParam(paramName, paramValues); - if (contentStr != null) { - content = RestconfDocumentedException.throwIfNull(ContentParam.forUriValue(contentStr), - ErrorType.PROTOCOL, ErrorTag.INVALID_VALUE, - "Invalid content parameter: %s, allowed values are %s", contentStr, POSSIBLE_CONTENT); - } - break; - case DepthParam.uriName: - final String depthStr = optionalParam(paramName, paramValues); - 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\"")); - } - break; - case FieldsParam.uriName: - final String fieldsStr = optionalParam(paramName, paramValues); - if (fieldsStr != null) { + try { + switch (paramName) { + case ContentParam.uriName: + content = optionalParam(ContentParam::forUriValue, paramName, paramValues); + break; + case DepthParam.uriName: + final String depthStr = optionalParam(paramName, paramValues); try { - fields = FieldsParam.parse(fieldsStr); - } catch (ParseException e) { + depth = DepthParam.forUriValue(depthStr); + } catch (IllegalArgumentException e) { throw new RestconfDocumentedException(e, new RestconfError(ErrorType.PROTOCOL, - ErrorTag.INVALID_VALUE, "Invalid filds parameter: " + fieldsStr)); + ErrorTag.INVALID_VALUE, "Invalid depth parameter: " + depthStr, null, + "The depth parameter must be an integer between 1 and 65535 or \"unbounded\"")); } - } - break; - case WithDefaultsParam.uriName: - final String withDefaultsStr = optionalParam(paramName, paramValues); - 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)); - } - - switch (val) { - case REPORT_ALL: - withDefaults = null; - tagged = false; - break; - case REPORT_ALL_TAGGED: - withDefaults = null; - tagged = true; - break; - default: - withDefaults = val; - tagged = false; + break; + case FieldsParam.uriName: + fields = optionalParam(FieldsParam::forUriValue, paramName, paramValues); + break; + case WithDefaultsParam.uriName: + final var defaultsVal = optionalParam(WithDefaultsParam::forUriValue, paramName, paramValues); + if (defaultsVal != null) { + switch (defaultsVal) { + case REPORT_ALL: + withDefaults = null; + tagged = false; + break; + case REPORT_ALL_TAGGED: + withDefaults = null; + tagged = true; + break; + default: + withDefaults = defaultsVal; + tagged = false; + } } - } - break; - case PrettyPrintParam.uriName: - prettyPrint = optionalParam(PrettyPrintParam::forUriValue, paramName, paramValues); - break; - default: - throw unhandledParam("read", paramName); + break; + case PrettyPrintParam.uriName: + prettyPrint = optionalParam(PrettyPrintParam::forUriValue, paramName, paramValues); + break; + default: + throw unhandledParam("read", paramName); + } + } catch (IllegalArgumentException e) { + throw new RestconfDocumentedException("Invalid " + paramName + " value: " + e.getMessage(), + ErrorType.PROTOCOL, ErrorTag.INVALID_VALUE, e); } } @@ -212,28 +189,23 @@ public final class QueryParams { PointParam point = null; for (final Entry> entry : uriInfo.getQueryParameters().entrySet()) { - final String uriName = entry.getKey(); + final String paramName = entry.getKey(); final List paramValues = entry.getValue(); - switch (uriName) { - case InsertParam.uriName: - final String instartStr = optionalParam(uriName, paramValues); - if (instartStr != null) { - insert = InsertParam.forUriValue(instartStr); - if (insert == null) { - throw new RestconfDocumentedException( - "Unrecognized insert parameter value '" + instartStr + "'", ErrorType.PROTOCOL, - ErrorTag.BAD_ELEMENT); - } - } - break; - case PointParam.uriName: - final String pointStr = optionalParam(uriName, paramValues); - if (pointStr != null) { - point = PointParam.forUriValue(pointStr); - } - break; - default: - throw unhandledParam("write", uriName); + + try { + switch (paramName) { + case InsertParam.uriName: + insert = optionalParam(InsertParam::forUriValue, paramName, paramValues); + break; + case PointParam.uriName: + point = optionalParam(PointParam::forUriValue, paramName, paramValues); + break; + default: + throw unhandledParam("write", paramName); + } + } catch (IllegalArgumentException e) { + throw new RestconfDocumentedException("Invalid " + paramName + " value: " + e.getMessage(), + ErrorType.PROTOCOL, ErrorTag.INVALID_VALUE, e); } } -- 2.36.6