From: ToineSiebelink Date: Mon, 17 Nov 2025 16:01:33 +0000 (+0000) Subject: Forwarding list parameters : attributes and field consistently X-Git-Url: https://gerrit.onap.org/r/gitweb?a=commitdiff_plain;h=5e789af42450e9684d5ab61cec9ec9928d6d4721;p=cps.git Forwarding list parameters : attributes and field consistently - Treat 'fields' same as 'attributes' i.e. support empty value - Refactored code (removed unnecessary null-check) - Add test for all scenarios in Controller for best readability Issue-ID: CPS-3046 Change-Id: I3d3e59f9b736a74036d73f55af5c6349010aa1e1 Signed-off-by: ToineSiebelink --- diff --git a/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/controller/ProvMnsController.java b/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/controller/ProvMnsController.java index fdfd5615ac..fc7129cb50 100644 --- a/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/controller/ProvMnsController.java +++ b/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/controller/ProvMnsController.java @@ -67,13 +67,14 @@ public class ProvMnsController implements ProvMnS { private final JsonObjectMapper jsonObjectMapper; @Override - public ResponseEntity getMoi(final HttpServletRequest httpServletRequest, final Scope scope, - final String filter, List attributes, - final List fields, - final ClassNameIdGetDataNodeSelectorParameter dataNodeSelector) { + public ResponseEntity getMoi(final HttpServletRequest httpServletRequest, + final Scope scope, + final String filter, + final List attributes, + final List fields, + final ClassNameIdGetDataNodeSelectorParameter dataNodeSelector) { final RequestPathParameters requestPathParameters = parameterMapper.extractRequestParameters(httpServletRequest); - attributes = isAttributesInRequest(httpServletRequest) ? attributes : null; try { final YangModelCmHandle yangModelCmHandle = inventoryPersistence.getYangModelCmHandle( alternateIdMatcher.getCmHandleIdByLongestMatchingAlternateId( @@ -86,9 +87,7 @@ public class ProvMnsController implements ProvMnS { return errorResponseBuilder.buildErrorResponseGet(httpStatus, exception.getDetails()); } final UrlTemplateParameters urlTemplateParameters = parametersBuilder.createUrlTemplateParametersForRead( - scope, filter, attributes, - fields, dataNodeSelector, - yangModelCmHandle, requestPathParameters); + scope, filter, attributes, fields, dataNodeSelector, yangModelCmHandle, requestPathParameters); return dmiRestClient.synchronousGetOperation( RequiredDmiService.DATA, urlTemplateParameters); } catch (final NoAlternateIdMatchFoundException noAlternateIdMatchFoundException) { @@ -225,7 +224,4 @@ public class ProvMnsController implements ProvMnS { return alternateId + " not found"; } - private boolean isAttributesInRequest(final HttpServletRequest httpServletRequest) { - return httpServletRequest.getParameterMap().containsKey("attributes"); - } } diff --git a/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/controller/ProvMnsControllerSpec.groovy b/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/controller/ProvMnsControllerSpec.groovy index 8936cfc0c0..b1435ac38c 100644 --- a/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/controller/ProvMnsControllerSpec.groovy +++ b/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/controller/ProvMnsControllerSpec.groovy @@ -88,9 +88,9 @@ class ProvMnsControllerSpec extends Specification { @Value('${rest.api.provmns-base-path}') def provMnSBasePath - def 'Get resource data request where #scenario'() { + def 'Get resource data request where #scenario.'() { given: 'resource data url' - def getUrl = "$provMnSBasePath/v1/someUriLdnFirstPart/someClassName=someId?attributes=[test,query,param]" + def getUrl = "$provMnSBasePath/v1/someUriLdnFirstPart/someClassName=someId" and: 'alternate Id can be matched' alternateIdMatcher.getCmHandleIdByLongestMatchingAlternateId('/someUriLdnFirstPart/someClassName=someId', "/") >> 'cm-1' and: 'persistence service returns yangModelCmHandle' @@ -102,14 +102,39 @@ class ProvMnsControllerSpec extends Specification { then: 'response status as expected' assert response.status == expectedHttpStatus.value() where: - scenario | dataProducerId | state || expectedHttpStatus - 'cmHandle state is Ready with populated dataProducerId' | 'someDataProducerId' | READY || HttpStatus.OK - 'dataProducerId is blank' | ' ' | READY || HttpStatus.UNPROCESSABLE_ENTITY - 'dataProducerId is null' | null | READY || HttpStatus.UNPROCESSABLE_ENTITY - 'cmHandle state is Advised' | 'someDataProducerId' | ADVISED || HttpStatus.NOT_ACCEPTABLE + scenario | dataProducerId | state || expectedHttpStatus + 'cmHandle state is Ready with populated dataProducerId' | 'someDataProducerId' | READY || HttpStatus.OK + 'dataProducerId is blank' | ' ' | READY || HttpStatus.UNPROCESSABLE_ENTITY + 'dataProducerId is null' | null | READY || HttpStatus.UNPROCESSABLE_ENTITY + 'cmHandle state is Advised' | 'someDataProducerId' | ADVISED || HttpStatus.NOT_ACCEPTABLE + } + + def 'Get resource data request with list parameter: #scenario.'() { + given: 'resource data url' + def getUrl = "$provMnSBasePath/v1/someUriLdnFirstPart/someClassName=someId$parameterInProvMnsRequest" + and: 'alternate Id can be matched' + alternateIdMatcher.getCmHandleIdByLongestMatchingAlternateId('/someUriLdnFirstPart/someClassName=someId', "/") >> 'cm-1' + and: 'persistence service returns yangModelCmHandle' + inventoryPersistence.getYangModelCmHandle('cm-1') >> new YangModelCmHandle(id:'cm-1', dmiServiceName: 'someDmiService', dataProducerIdentifier: 'someDataProducerId', compositeState: new CompositeState(cmHandleState: READY)) + when: 'get data resource request is performed' + mvc.perform(get(getUrl).contentType(MediaType.APPLICATION_JSON)) + then: 'the request to dmi contains the expected url parameters and values and then returns OK' + 1 * dmiRestClient.synchronousGetOperation(*_) >> { arguments -> def urlTemplateParameters = arguments[1] + assert urlTemplateParameters.urlTemplate.contains(expectedParameterInUri) + assert urlTemplateParameters.urlVariables().get(parameterName) == expectedParameterValuesInDmiRequest + return new ResponseEntity<>('Some response from DMI service', HttpStatus.OK) + } + where: 'attributes is populated with the following ' + scenario | parameterName | parameterInProvMnsRequest || expectedParameterInUri || expectedParameterValuesInDmiRequest + 'some attributes' | 'attributes' | '?attributes=value1,value2' || '?attributes={attributes}' || 'value1,value2' + 'empty attributes' | 'attributes' | '?attributes=' || '?attributes={attributes}' || '' + 'no attributes (null)' | 'attributes' | '' || '' || null + 'some fields' | 'fields' | '?fields=value3,value4' || '?fields={fields}' || 'value3,value4' + 'empty fields' | 'fields' | '?fields=' || '?fields={fields}' || '' + 'no fields (null)' | 'fields' | '' || '' || null } - def 'Get resource data request with no match for alternate id'() { + def 'Get resource data request with no match for alternate id.'() { given: 'resource data url' def getUrl = "$provMnSBasePath/v1/someUriLdnFirstPart/someClassName=someId" and: 'alternate Id cannot be matched' @@ -122,7 +147,7 @@ class ProvMnsControllerSpec extends Specification { assert response.status == HttpStatus.NOT_FOUND.value() } - def 'Patch request where #scenario'() { + def 'Patch request where #scenario.'() { given: 'provmns url' def provmnsUrl = "$provMnSBasePath/v1/someUriLdnFirstPart/someClassName=someId" and: 'alternate Id can be matched' @@ -146,7 +171,7 @@ class ProvMnsControllerSpec extends Specification { 'cmHandle state is Advised' | 'someDataProducerId' | ADVISED || HttpStatus.NOT_ACCEPTABLE } - def 'Patch request with no match for alternate id'() { + def 'Patch request with no match for alternate id.'() { given: 'resource data url' def provmnsUrl = "$provMnSBasePath/v1/someUriLdnFirstPart/someClassName=someId" and: 'alternate Id cannot be matched' @@ -162,7 +187,7 @@ class ProvMnsControllerSpec extends Specification { assert response.status == HttpStatus.NOT_FOUND.value() } - def 'Patch request with no permission from coordination management'() { + def 'Patch request with no permission from coordination management.'() { given: 'resource data url' def url = "$provMnSBasePath/v1/someUriLdnFirstPart/someClassName=someId" and: 'alternate Id can be matched' @@ -180,7 +205,7 @@ class ProvMnsControllerSpec extends Specification { assert response.status == HttpStatus.NOT_ACCEPTABLE.value() } - def 'Put resource data request where #scenario'() { + def 'Put resource data request where #scenario.'() { given: 'resource data url' def putUrl = "$provMnSBasePath/v1/someUriLdnFirstPart/someClassName=someId" and: 'alternate Id can be matched' @@ -204,7 +229,7 @@ class ProvMnsControllerSpec extends Specification { 'cmHandle state is Advised' | 'someDataProducerId' | ADVISED || HttpStatus.NOT_ACCEPTABLE } - def 'Put resource data request with no match for alternate id'() { + def 'Put resource data request with no match for alternate id.'() { given: 'resource data url' def putUrl = "$provMnSBasePath/v1/someUriLdnFirstPart/someClassName=someId" and: 'alternate Id cannot be matched' @@ -220,7 +245,7 @@ class ProvMnsControllerSpec extends Specification { assert response.status == HttpStatus.NOT_FOUND.value() } - def 'Put resource data request with no permission from coordination management'() { + def 'Put resource data request with no permission from coordination management.'() { given: 'resource data url' def putUrl = "$provMnSBasePath/v1/someUriLdnFirstPart/someClassName=someId" and: 'alternate Id can be matched' @@ -238,7 +263,7 @@ class ProvMnsControllerSpec extends Specification { assert response.status == HttpStatus.NOT_ACCEPTABLE.value() } - def 'Delete resource data request where #scenario'() { + def 'Delete resource data request where #scenario.'() { given: 'resource data url' def deleteUrl = "$provMnSBasePath/v1/someUriLdnFirstPart/someClassName=someId" and: 'alternate Id can be matched' @@ -259,7 +284,7 @@ class ProvMnsControllerSpec extends Specification { 'cmHandle state is Advised' | 'someDataProducerId' | ADVISED || HttpStatus.NOT_ACCEPTABLE } - def 'Delete resource data request with no match for alternate id'() { + def 'Delete resource data request with no match for alternate id.'() { given: 'resource data url' def deleteUrl = "$provMnSBasePath/v1/someUriLdnFirstPart/someClassName=someId" and: 'alternate Id cannot be matched' @@ -272,7 +297,7 @@ class ProvMnsControllerSpec extends Specification { assert response.status == HttpStatus.NOT_FOUND.value() } - def 'Delete resource data request with no permission from coordination management'() { + def 'Delete resource data request with no permission from coordination management.'() { given: 'resource data url' def deleteUrl = "$provMnSBasePath/v1/someUriLdnFirstPart/someClassName=someId" and: 'alternate Id can be matched' @@ -287,11 +312,11 @@ class ProvMnsControllerSpec extends Specification { assert response.status == HttpStatus.NOT_ACCEPTABLE.value() } - def 'Invalid path passed in to provmns interface, #scenario'() { + def 'Invalid path passed in to provmns interface, #scenario.'() { given: 'an invalid path' def url = "$provMnSBasePath/v1/" + invalidPath when: 'get data resource request is performed' - def response = mvc.perform(get(url).contentType(MediaType.APPLICATION_JSON)).andReturn().response + mvc.perform(get(url).contentType(MediaType.APPLICATION_JSON)).andReturn().response then: 'invalid path exception is thrown' thrown(ServletException) where: diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/provmns/ParametersBuilder.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/provmns/ParametersBuilder.java index 125ac460d5..30b4d21668 100644 --- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/provmns/ParametersBuilder.java +++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/provmns/ParametersBuilder.java @@ -54,15 +54,13 @@ public class ParametersBuilder { final YangModelCmHandle yangModelCmHandle, final RequestPathParameters requestPathParameters) { final String dmiServiceName = yangModelCmHandle.resolveDmiServiceName(DATA); - final String attributesString = removeBrackets(attributes); - final String fieldsString = removeBrackets(fields); return RestServiceUrlTemplateBuilder.newInstance() .fixedPathSegment(requestPathParameters.toAlternateId()) .queryParameter("scopeType", scope.getScopeType() != null ? scope.getScopeType().getValue() : null) .queryParameter("scopeLevel", scope.getScopeLevel() != null ? scope.getScopeLevel().toString() : null) .queryParameter("filter", filter) - .queryParameterWithBlankValue("attributes", attributes == null ? null : attributesString) - .queryParameter("fields", fieldsString.isBlank() ? null : fieldsString) + .queryParameterAllowBlankValue("attributes", attributes == null ? null : String.join(",", attributes)) + .queryParameterAllowBlankValue("fields", fields == null ? null : String.join(",", fields)) .queryParameter("dataNodeSelector", dataNodeSelector.getDataNodeSelector()) .createUrlTemplateParameters(dmiServiceName, "ProvMnS"); } @@ -82,11 +80,4 @@ public class ParametersBuilder { .createUrlTemplateParameters(dmiServiceName, "ProvMnS"); } - private String removeBrackets(final List queryParameterList) { - if (queryParameterList != null) { - final String queryParameterText = queryParameterList.toString(); - return queryParameterText.substring(1, queryParameterText.length() - 1); - } - return ""; - } } diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/utils/http/RestServiceUrlTemplateBuilder.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/utils/http/RestServiceUrlTemplateBuilder.java index 3bc62cb125..6bf82d03d0 100644 --- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/utils/http/RestServiceUrlTemplateBuilder.java +++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/utils/http/RestServiceUrlTemplateBuilder.java @@ -89,7 +89,7 @@ public class RestServiceUrlTemplateBuilder { } /** - * Add a query parameter to the URL. Query parameter could have a blank value. + * Add a query parameter to the URL. Accept blank but ignore null. * Do NOT encode as the builder will take care of encoding * * @param queryParameterName the name of the variable @@ -97,8 +97,8 @@ public class RestServiceUrlTemplateBuilder { * * @return this builder instance */ - public RestServiceUrlTemplateBuilder queryParameterWithBlankValue(final String queryParameterName, - final String queryParameterValue) { + public RestServiceUrlTemplateBuilder queryParameterAllowBlankValue(final String queryParameterName, + final String queryParameterValue) { if (queryParameterValue != null) { queryParameters.put(queryParameterName, queryParameterValue); } diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/provmns/ParametersBuilderSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/provmns/ParametersBuilderSpec.groovy index 67c364c9ac..8b81306822 100644 --- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/provmns/ParametersBuilderSpec.groovy +++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/provmns/ParametersBuilderSpec.groovy @@ -65,25 +65,6 @@ class ParametersBuilderSpec extends Specification{ assert result.urlVariables.keySet()[0] == 'dataNodeSelector' } - def 'Create url template parameters for GET with empty attributes'() { - when: 'Creating URL parameters for GET with empty attributes and fields' - def result = objectUnderTest.createUrlTemplateParametersForRead(new Scope(), - null, - [], - [], - new ClassNameIdGetDataNodeSelectorParameter(dataNodeSelector: 'my dataNodeSelector'), - new YangModelCmHandle(dmiServiceName: 'myDmiService'), - new RequestPathParameters(uriLdnFirstPart:'myPathVariable=myPathValue', className: 'myClassName', id:'myId')) - then: 'the template has the correct result' - assert result.urlTemplate.toString().startsWith('myDmiService/ProvMnS/v1/myPathVariable=myPathValue/myClassName=myId') - and: 'The url variables contains a data node selector and attributes parameters' - assert result.urlVariables.size() == 2 - assert result.urlVariables.keySet()[0] == 'dataNodeSelector' - assert result.urlVariables.keySet()[1] == 'attributes' - and: 'the attributes value is blank' - assert result.urlVariables.attributes == '' - } - def 'Create url template parameters for PUT and PATCH.'() { when: 'Creating URL parameters for PUT (or PATCH)' def result = objectUnderTest.createUrlTemplateParametersForWrite( @@ -95,5 +76,4 @@ class ParametersBuilderSpec extends Specification{ assert result.urlVariables.isEmpty() } - } diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/utils/http/RestServiceUrlTemplateBuilderSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/utils/http/RestServiceUrlTemplateBuilderSpec.groovy index 26d156279f..8874ba5e54 100644 --- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/utils/http/RestServiceUrlTemplateBuilderSpec.groovy +++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/utils/http/RestServiceUrlTemplateBuilderSpec.groovy @@ -51,31 +51,32 @@ class RestServiceUrlTemplateBuilderSpec extends Specification { } def 'Build URL template parameters with empty query parameters.'() { - when: 'the query parameter is given to the builder' - objectUnderTest.queryParameter('param', value) - and: 'the URL template parameters are create' + when: 'a query parameter with value #scenario is added' + objectUnderTest.queryParameter('myParam', value) + and: 'the URL template parameters are created' def result = objectUnderTest.createUrlTemplateParameters('myServer', 'myBasePath') then: 'no parameter gets added' assert result.urlVariables.isEmpty() where: 'the following parameter values are used' - value << [ null, '', ' ' ] + scenario | value + 'empty' | '' + 'blank' | ' ' + 'null' | null } - def 'Build URL template parameters with empty value query parameters.'() { - when: 'the query parameter with a blank value is given to the builder' - objectUnderTest.queryParameterWithBlankValue('myParam', '') + def 'Build URL template parameters allowing blank.'() { + when: 'a query parameter with value #scenario is added while allowing blank values' + objectUnderTest.queryParameterAllowBlankValue('myParam', value) and: 'the URL template parameters are created' def result = objectUnderTest.createUrlTemplateParameters('myServer', 'myBasePath') - then: 'parameter myParam is added with empty value' - assert result.urlVariables() == ['myParam': ''] + then: 'the parameter has the expected value' + assert result.urlVariables().get('myParam') == value + where: + scenario | value + 'has value' | 'my value' + 'empty' | '' + 'blank' | ' ' + 'null' | null } - def 'Build URL template parameters with null value query parameters.'() { - when: 'the query parameter with null value is given to the builder' - objectUnderTest.queryParameterWithBlankValue('myParam', null) - and: 'the URL template parameters are created' - def result = objectUnderTest.createUrlTemplateParameters('myServer', 'myBasePath') - then: 'parameter myParam is not added' - assert result.urlVariables().size() == 0 - } }