Forwarding list parameters : attributes and field consistently 53/142453/1
authorToineSiebelink <toine.siebelink@est.tech>
Mon, 17 Nov 2025 16:01:33 +0000 (16:01 +0000)
committerToineSiebelink <toine.siebelink@est.tech>
Mon, 17 Nov 2025 16:24:35 +0000 (16:24 +0000)
- 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 <toine.siebelink@est.tech>
cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/controller/ProvMnsController.java
cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/controller/ProvMnsControllerSpec.groovy
cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/provmns/ParametersBuilder.java
cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/utils/http/RestServiceUrlTemplateBuilder.java
cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/provmns/ParametersBuilderSpec.groovy
cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/utils/http/RestServiceUrlTemplateBuilderSpec.groovy

index fdfd561..fc7129c 100644 (file)
@@ -67,13 +67,14 @@ public class ProvMnsController implements ProvMnS {
     private final JsonObjectMapper jsonObjectMapper;
 
     @Override
-    public ResponseEntity<Object> getMoi(final HttpServletRequest httpServletRequest, final Scope scope,
-                                                   final String filter, List<String> attributes,
-                                                   final List<String> fields,
-                                                   final ClassNameIdGetDataNodeSelectorParameter dataNodeSelector) {
+    public ResponseEntity<Object> getMoi(final HttpServletRequest httpServletRequest,
+                                         final Scope scope,
+                                         final String filter,
+                                         final List<String> attributes,
+                                         final List<String> 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");
-    }
 }
index 8936cfc..b1435ac 100644 (file)
@@ -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:
index 125ac46..30b4d21 100644 (file)
@@ -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<String> queryParameterList) {
-        if (queryParameterList != null) {
-            final String queryParameterText = queryParameterList.toString();
-            return queryParameterText.substring(1, queryParameterText.length() - 1);
-        }
-        return "";
-    }
 }
index 3bc62cb..6bf82d0 100644 (file)
@@ -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);
         }
index 67c364c..8b81306 100644 (file)
@@ -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()
     }
 
-
 }
index 26d1562..8874ba5 100644 (file)
@@ -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
-    }
 }