From a18a2bc53dbec48c9ec5c4ac632ccbcddb059631 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Jaroslav=20T=C3=B3th?= Date: Thu, 1 Aug 2019 17:17:40 +0200 Subject: [PATCH] Fixing of useless message 'Unknown key : content' (read data) MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit - After reading of data in RFC-8040 with set content query parameter, the INFO log with unknown query parameter was sent (but it was recognized correctly in another part of code). - Refactoring of the readData(..) method, so the parsing of withDefaults query parameter is moved to ReadDataTransactionUtil that contains other parsing logic. - Adding of withDefaults query paramameter to WriteParameters data class. - Added unit tests. Change-Id: I25a01c62a995facce24f29a0dc48048aa1e49f56 Signed-off-by: Jaroslav Tóth --- .../common/context/WriterParameters.java | 12 ++++ .../impl/RestconfDataServiceImpl.java | 38 +----------- .../rests/utils/ReadDataTransactionUtil.java | 45 ++++++-------- .../utils/RestconfDataServiceConstant.java | 2 + .../utils/ReadDataTransactionUtilTest.java | 58 +++++++++++++++++++ 5 files changed, 94 insertions(+), 61 deletions(-) diff --git a/restconf/restconf-common/src/main/java/org/opendaylight/restconf/common/context/WriterParameters.java b/restconf/restconf-common/src/main/java/org/opendaylight/restconf/common/context/WriterParameters.java index 8941f9751c..79bfa12ab2 100644 --- a/restconf/restconf-common/src/main/java/org/opendaylight/restconf/common/context/WriterParameters.java +++ b/restconf/restconf-common/src/main/java/org/opendaylight/restconf/common/context/WriterParameters.java @@ -18,6 +18,7 @@ public final class WriterParameters { private final List> fields; private final boolean prettyPrint; private final boolean tagged; + private final String withDefault; private WriterParameters(final WriterParametersBuilder builder) { this.content = builder.content; @@ -25,6 +26,7 @@ public final class WriterParameters { this.fields = builder.fields; this.prettyPrint = builder.prettyPrint; this.tagged = builder.tagged; + this.withDefault = builder.withDefault; } public String getContent() { @@ -47,12 +49,17 @@ public final class WriterParameters { return this.tagged; } + public String getWithDefault() { + return withDefault; + } + public static class WriterParametersBuilder { private String content; private Integer depth; private List> fields; private boolean prettyPrint; private boolean tagged; + private String withDefault; public WriterParametersBuilder() {} @@ -76,6 +83,11 @@ public final class WriterParameters { return this; } + public WriterParametersBuilder setWithDefault(final String withDefault) { + this.withDefault = withDefault; + return this; + } + public WriterParameters build() { return new WriterParameters(this); } diff --git a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfDataServiceImpl.java b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfDataServiceImpl.java index 55f3cd8ee1..b0dcebd66a 100644 --- a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfDataServiceImpl.java +++ b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/services/impl/RestconfDataServiceImpl.java @@ -103,38 +103,7 @@ public class RestconfDataServiceImpl implements RestconfDataService { final SchemaContextRef schemaContextRef = new SchemaContextRef(this.schemaContextHandler.get()); final InstanceIdentifierContext instanceIdentifier = ParserIdentifier.toInstanceIdentifier( identifier, schemaContextRef.get(), Optional.of(this.mountPointServiceHandler.get())); - - boolean withDefaUsed = false; - String withDefa = null; - - for (final Entry> entry : uriInfo.getQueryParameters().entrySet()) { - switch (entry.getKey()) { - case "with-defaults": - if (!withDefaUsed) { - withDefaUsed = true; - withDefa = entry.getValue().iterator().next(); - } else { - throw new RestconfDocumentedException("With-defaults parameter can be used only once."); - } - break; - default: - LOG.info("Unknown key : {}.", entry.getKey()); - break; - } - } - boolean tagged = false; - if (withDefaUsed) { - if ("report-all-tagged".equals(withDefa)) { - tagged = true; - withDefa = null; - } - if ("report-all".equals(withDefa)) { - withDefa = null; - } - } - - final WriterParameters parameters = ReadDataTransactionUtil.parseUriParameters( - instanceIdentifier, uriInfo, tagged); + final WriterParameters parameters = ReadDataTransactionUtil.parseUriParameters(instanceIdentifier, uriInfo); final DOMMountPoint mountPoint = instanceIdentifier.getMountPoint(); final TransactionChainHandler localTransactionChainHandler; @@ -146,9 +115,8 @@ public class RestconfDataServiceImpl implements RestconfDataService { final TransactionVarsWrapper transactionNode = new TransactionVarsWrapper( instanceIdentifier, mountPoint, localTransactionChainHandler); - final NormalizedNode node = - ReadDataTransactionUtil.readData(identifier, parameters.getContent(), transactionNode, withDefa, - schemaContextRef, uriInfo); + final NormalizedNode node = ReadDataTransactionUtil.readData(identifier, parameters.getContent(), + transactionNode, parameters.getWithDefault(), schemaContextRef, uriInfo); if (identifier != null && identifier.contains(STREAM_PATH) && identifier.contains(STREAM_ACCESS_PATH_PART) && identifier.contains(STREAM_LOCATION_PATH_PART)) { final String value = (String) node.getValue(); 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 014da28275..509562bb28 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 @@ -92,39 +92,15 @@ public final class ReadDataTransactionUtil { /** * Parse parameters from URI request and check their types and values. * - * * @param identifier * {@link InstanceIdentifierContext} * @param uriInfo * URI info - * @param tagged - * set tagged for {@link WriterParameters} * @return {@link WriterParameters} */ - public static @NonNull WriterParameters parseUriParameters(final @NonNull InstanceIdentifierContext identifier, - final @Nullable UriInfo uriInfo, final boolean tagged) { - return parseParams(identifier, uriInfo, tagged); - } - - /** - * Parse parameters from URI request and check their types and values. - * - * - * @param identifier - * {@link InstanceIdentifierContext} - * @param uriInfo - * URI info - * @return {@link WriterParameters} - */ - public static @NonNull WriterParameters parseUriParameters(final @NonNull InstanceIdentifierContext identifier, - final @Nullable UriInfo uriInfo) { - return parseParams(identifier, uriInfo, false); - } - - private static WriterParameters parseParams(final InstanceIdentifierContext identifier, final UriInfo uriInfo, - final boolean tagged) { + public static WriterParameters parseUriParameters(final InstanceIdentifierContext identifier, + final UriInfo uriInfo) { final WriterParametersBuilder builder = new WriterParametersBuilder(); - builder.setTagged(tagged); if (uriInfo == null) { return builder.build(); @@ -145,6 +121,9 @@ public final class ReadDataTransactionUtil { final List depth = uriInfo.getQueryParameters().getOrDefault( RestconfDataServiceConstant.ReadData.DEPTH, Collections.singletonList(RestconfDataServiceConstant.ReadData.UNBOUNDED)); + final List withDefaults = uriInfo.getQueryParameters().getOrDefault( + RestconfDataServiceConstant.ReadData.WITH_DEFAULTS, + Collections.emptyList()); // fields final List fields = uriInfo.getQueryParameters().getOrDefault( RestconfDataServiceConstant.ReadData.FIELDS, @@ -154,6 +133,7 @@ public final class ReadDataTransactionUtil { ParametersUtil.checkParameterCount(content, RestconfDataServiceConstant.ReadData.CONTENT); ParametersUtil.checkParameterCount(depth, RestconfDataServiceConstant.ReadData.DEPTH); ParametersUtil.checkParameterCount(fields, RestconfDataServiceConstant.ReadData.FIELDS); + ParametersUtil.checkParameterCount(fields, RestconfDataServiceConstant.ReadData.WITH_DEFAULTS); // check and set content final String contentValue = content.get(0); @@ -190,6 +170,19 @@ public final class ReadDataTransactionUtil { builder.setFields(ParserFieldsParameter.parseFieldsParameter(identifier, fields.get(0))); } + // check and set withDefaults parameter + if (!withDefaults.isEmpty()) { + switch (withDefaults.get(0)) { + case RestconfDataServiceConstant.ReadData.REPORT_ALL_TAGGED_DEFAULT_VALUE: + builder.setTagged(true); + break; + case RestconfDataServiceConstant.ReadData.REPORT_ALL_DEFAULT_VALUE: + break; + default: + builder.setWithDefault(withDefaults.get(0)); + } + } + return builder.build(); } diff --git a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/RestconfDataServiceConstant.java b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/RestconfDataServiceConstant.java index a39ec65bc0..7cc4d31bcf 100644 --- a/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/RestconfDataServiceConstant.java +++ b/restconf/restconf-nb-rfc8040/src/main/java/org/opendaylight/restconf/nb/rfc8040/rests/utils/RestconfDataServiceConstant.java @@ -46,6 +46,8 @@ public final class RestconfDataServiceConstant { public static final String READ_TYPE_TX = "READ"; public static final String WITH_DEFAULTS = "with-defaults"; + public static final String REPORT_ALL_DEFAULT_VALUE = "report-all"; + public static final String REPORT_ALL_TAGGED_DEFAULT_VALUE = "report-all-tagged"; private ReadData() { throw new UnsupportedOperationException("Util class."); 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 709fc73eff..bb4d19f62f 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 @@ -8,8 +8,10 @@ package org.opendaylight.restconf.nb.rfc8040.rests.utils; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.when; @@ -36,6 +38,7 @@ import org.opendaylight.restconf.common.errors.RestconfError.ErrorTag; import org.opendaylight.restconf.common.errors.RestconfError.ErrorType; import org.opendaylight.restconf.nb.rfc8040.handlers.TransactionChainHandler; import org.opendaylight.restconf.nb.rfc8040.rests.transactions.TransactionVarsWrapper; +import org.opendaylight.restconf.nb.rfc8040.rests.utils.RestconfDataServiceConstant.ReadData; import org.opendaylight.yangtools.yang.common.QName; import org.opendaylight.yangtools.yang.data.api.YangInstanceIdentifier; import org.opendaylight.yangtools.yang.data.api.schema.ContainerNode; @@ -437,4 +440,59 @@ public class ReadDataTransactionUtilTest { assertEquals("Error status code is not correct", 400, e.getErrors().get(0).getErrorTag().getStatusCode()); } } + + /** + * Testing parsing of with-defaults parameter which value doesn't match report-all or report-all-tagged patterns + * - non-reporting setting. + */ + @Test + public void parseUriParametersWithDefaultAndNonTaggedTest() { + // preparation of input data + final UriInfo uriInfo = Mockito.mock(UriInfo.class); + final MultivaluedHashMap parameters = new MultivaluedHashMap<>(); + final String preparedDefaultValue = "sample-default"; + parameters.put(RestconfDataServiceConstant.ReadData.WITH_DEFAULTS, + Collections.singletonList(preparedDefaultValue)); + when(uriInfo.getQueryParameters()).thenReturn(parameters); + + final WriterParameters writerParameters = ReadDataTransactionUtil.parseUriParameters(context, uriInfo); + assertEquals(preparedDefaultValue, writerParameters.getWithDefault()); + assertFalse(writerParameters.isTagged()); + } + + /** + * Testing parsing of with-defaults parameter which value matches 'report-all-tagged' setting - default value should + * be set to {@code null} and tagged flag should be set to {@code true}. + */ + @Test + public void parseUriParametersWithDefaultAndTaggedTest() { + // preparation of input data + final UriInfo uriInfo = Mockito.mock(UriInfo.class); + final MultivaluedHashMap parameters = new MultivaluedHashMap<>(); + parameters.put(RestconfDataServiceConstant.ReadData.WITH_DEFAULTS, + Collections.singletonList(ReadData.REPORT_ALL_TAGGED_DEFAULT_VALUE)); + when(uriInfo.getQueryParameters()).thenReturn(parameters); + + final WriterParameters writerParameters = ReadDataTransactionUtil.parseUriParameters(context, uriInfo); + assertNull(writerParameters.getWithDefault()); + assertTrue(writerParameters.isTagged()); + } + + /** + * Testing parsing of with-defaults parameter which value matches 'report-all' setting - default value should + * be set to {@code null} and tagged flag should be set to {@code false}. + */ + @Test + public void parseUriParametersWithDefaultAndReportAllTest() { + // preparation of input data + final UriInfo uriInfo = Mockito.mock(UriInfo.class); + final MultivaluedHashMap parameters = new MultivaluedHashMap<>(); + parameters.put(RestconfDataServiceConstant.ReadData.WITH_DEFAULTS, + Collections.singletonList(ReadData.REPORT_ALL_DEFAULT_VALUE)); + when(uriInfo.getQueryParameters()).thenReturn(parameters); + + final WriterParameters writerParameters = ReadDataTransactionUtil.parseUriParameters(context, uriInfo); + assertNull(writerParameters.getWithDefault()); + assertFalse(writerParameters.isTagged()); + } } -- 2.36.6