[BUG] Blank alternateId overwrites existing 13/138813/4
authordanielhanrahan <daniel.hanrahan@est.tech>
Mon, 19 Aug 2024 19:34:45 +0000 (20:34 +0100)
committerDaniel Hanrahan <daniel.hanrahan@est.tech>
Wed, 21 Aug 2024 11:15:23 +0000 (11:15 +0000)
During CM-handle update, a blank "" alternateId will overwrite
an already-set alternateId.
Additionally during registration, if an alternateId consisting of
whitespace is supplied, it will be persisted with whitespace.
This fixes both issues by using isBlank/isNotBlank consistently.

Issue-ID: CPS-2372
Signed-off-by: danielhanrahan <daniel.hanrahan@est.tech>
Change-Id: I44e536cf87a31c6e975a82ed37769c1d2548653a

cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/CmHandleRegistrationService.java
cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/CmHandleRegistrationServicePropertyHandler.java
cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/models/YangModelCmHandle.java
docs/release-notes.rst
integration-test/src/test/groovy/org/onap/cps/integration/functional/ncmp/CmHandleUpdateSpec.groovy [new file with mode: 0644]

index d9f7e38..d7e16fc 100644 (file)
@@ -210,7 +210,8 @@ public class CmHandleRegistrationService {
         final DmiPluginRegistrationResponse dmiPluginRegistrationResponse) {
 
         final List<String> cmHandleIds = dmiPluginRegistration.getUpgradedCmHandles().getCmHandles();
-        final String upgradedModuleSetTag = dmiPluginRegistration.getUpgradedCmHandles().getModuleSetTag();
+        final String upgradedModuleSetTag =
+                StringUtils.trimToEmpty(dmiPluginRegistration.getUpgradedCmHandles().getModuleSetTag());
         final Map<YangModelCmHandle, CmHandleState> acceptedCmHandleStatePerCmHandle
             = new HashMap<>(cmHandleIds.size());
         final List<CmHandleRegistrationResponse> cmHandleUpgradeResponses = new ArrayList<>(cmHandleIds.size());
index c9981db..308ead1 100644 (file)
@@ -44,6 +44,7 @@ import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
+import org.apache.commons.lang3.StringUtils;
 import org.onap.cps.api.CpsDataService;
 import org.onap.cps.ncmp.api.inventory.models.CmHandleRegistrationResponse;
 import org.onap.cps.ncmp.api.inventory.models.NcmpServiceCmHandle;
@@ -56,7 +57,6 @@ import org.onap.cps.spi.model.DataNodeBuilder;
 import org.onap.cps.utils.ContentType;
 import org.onap.cps.utils.JsonObjectMapper;
 import org.springframework.stereotype.Service;
-import org.springframework.util.StringUtils;
 
 @Slf4j
 @Service
@@ -112,8 +112,7 @@ public class CmHandleRegistrationServicePropertyHandler {
 
     private void processUpdates(final DataNode existingCmHandleDataNode,
                                 final NcmpServiceCmHandle updatedNcmpServiceCmHandle) {
-        setAndUpdateCmHandleField(
-            updatedNcmpServiceCmHandle.getCmHandleId(), "alternate-id", updatedNcmpServiceCmHandle.getAlternateId());
+        updateAlternateId(updatedNcmpServiceCmHandle);
         updateDataProducerIdentifier(existingCmHandleDataNode, updatedNcmpServiceCmHandle);
         if (!updatedNcmpServiceCmHandle.getPublicProperties().isEmpty()) {
             updateProperties(existingCmHandleDataNode, PUBLIC_PROPERTY,
@@ -124,13 +123,20 @@ public class CmHandleRegistrationServicePropertyHandler {
         }
     }
 
+    private void updateAlternateId(final NcmpServiceCmHandle ncmpServiceCmHandle) {
+        final String newAlternateId = ncmpServiceCmHandle.getAlternateId();
+        if (StringUtils.isNotBlank(newAlternateId)) {
+            setAndUpdateCmHandleField(ncmpServiceCmHandle.getCmHandleId(), "alternate-id", newAlternateId);
+        }
+    }
+
     private void updateDataProducerIdentifier(final DataNode cmHandleDataNode,
                                               final NcmpServiceCmHandle ncmpServiceCmHandle) {
         final String newDataProducerIdentifier = ncmpServiceCmHandle.getDataProducerIdentifier();
-        if (StringUtils.hasText(newDataProducerIdentifier)) {
+        if (StringUtils.isNotBlank(newDataProducerIdentifier)) {
             final YangModelCmHandle yangModelCmHandle = YangDataConverter.toYangModelCmHandle(cmHandleDataNode);
             final String existingDataProducerIdentifier = yangModelCmHandle.getDataProducerIdentifier();
-            if (StringUtils.hasText(existingDataProducerIdentifier)) {
+            if (StringUtils.isNotBlank(existingDataProducerIdentifier)) {
                 if (!existingDataProducerIdentifier.equals(newDataProducerIdentifier)) {
                     log.warn("Unable to update dataProducerIdentifier for cmHandle {}. "
                             + "Value for dataProducerIdentifier has been set previously.",
index b10155c..76ee286 100644 (file)
@@ -18,7 +18,6 @@
  *  ============LICENSE_END=========================================================
  */
 
-
 package org.onap.cps.ncmp.impl.inventory.models;
 
 import com.fasterxml.jackson.annotation.JsonInclude;
@@ -126,10 +125,9 @@ public class YangModelCmHandle {
         yangModelCmHandle.setDmiServiceName(dmiServiceName);
         yangModelCmHandle.setDmiDataServiceName(dmiDataServiceName);
         yangModelCmHandle.setDmiModelServiceName(dmiModelServiceName);
-        yangModelCmHandle.setModuleSetTag(moduleSetTag == null ? StringUtils.EMPTY : moduleSetTag);
-        yangModelCmHandle.setAlternateId(alternateId == null ? StringUtils.EMPTY : alternateId);
-        yangModelCmHandle.setDataProducerIdentifier(
-            dataProducerIdentifier == null ? StringUtils.EMPTY : dataProducerIdentifier);
+        yangModelCmHandle.setModuleSetTag(StringUtils.trimToEmpty(moduleSetTag));
+        yangModelCmHandle.setAlternateId(StringUtils.trimToEmpty(alternateId));
+        yangModelCmHandle.setDataProducerIdentifier(StringUtils.trimToEmpty(dataProducerIdentifier));
         yangModelCmHandle.setDmiProperties(asYangModelCmHandleProperties(ncmpServiceCmHandle.getDmiProperties()));
         yangModelCmHandle.setPublicProperties(asYangModelCmHandleProperties(
                 ncmpServiceCmHandle.getPublicProperties()));
index 08d1ac4..829b28a 100644 (file)
@@ -41,6 +41,7 @@ Bug Fixes
 3.5.2
     - `CPS-2306 <https://jira.onap.org/browse/CPS-2306>`_ Update response message for data validation failure and make it consistent across APIs
     - `CPS-2319 <https://jira.onap.org/browse/CPS-2319>`_ Fix "Create a node" and "Add List Elements" APIs response code
+    - `CPS-2372 <https://jira.onap.org/browse/CPS-2372>`_ Blank alternate ID overwrites existing one
 
 Features
 --------
diff --git a/integration-test/src/test/groovy/org/onap/cps/integration/functional/ncmp/CmHandleUpdateSpec.groovy b/integration-test/src/test/groovy/org/onap/cps/integration/functional/ncmp/CmHandleUpdateSpec.groovy
new file mode 100644 (file)
index 0000000..2d1588e
--- /dev/null
@@ -0,0 +1,89 @@
+/*
+ *  ============LICENSE_START=======================================================
+ *  Copyright (C) 2024 Nordix Foundation
+ *  ================================================================================
+ *  Licensed under the Apache License, Version 2.0 (the 'License');
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *        http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an 'AS IS' BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ *
+ *  SPDX-License-Identifier: Apache-2.0
+ *  ============LICENSE_END=========================================================
+ */
+
+package org.onap.cps.integration.functional.ncmp
+
+import org.onap.cps.integration.base.CpsIntegrationSpecBase
+import org.onap.cps.ncmp.api.NcmpResponseStatus
+import org.onap.cps.ncmp.api.inventory.NetworkCmProxyInventoryFacade
+import org.onap.cps.ncmp.api.inventory.models.CmHandleRegistrationResponse
+import org.onap.cps.ncmp.api.inventory.models.DmiPluginRegistration
+import org.onap.cps.ncmp.api.inventory.models.NcmpServiceCmHandle
+
+class CmHandleUpdateSpec extends CpsIntegrationSpecBase {
+
+    NetworkCmProxyInventoryFacade objectUnderTest
+
+    def setup() {
+        objectUnderTest = networkCmProxyInventoryFacade
+    }
+
+    def 'Update of CM-handle with new or unchanged alternate ID succeeds.'() {
+        given: 'DMI will return modules when requested'
+            dmiDispatcher1.moduleNamesPerCmHandleId = ['ch-1': ['M1', 'M2']]
+        and: "existing CM-handle with alternate ID: $oldAlternateId"
+            registerCmHandle(DMI1_URL, 'ch-1', NO_MODULE_SET_TAG, oldAlternateId)
+
+        when: "CM-handle is registered for update with new alternate ID: $newAlternateId"
+            def cmHandleToUpdate = new NcmpServiceCmHandle(cmHandleId: 'ch-1', alternateId: newAlternateId)
+            def dmiPluginRegistrationResponse =
+                    objectUnderTest.updateDmiRegistrationAndSyncModule(new DmiPluginRegistration(dmiPlugin: DMI1_URL, updatedCmHandles: [cmHandleToUpdate]))
+
+        then: 'registration gives successful response'
+            assert dmiPluginRegistrationResponse.updatedCmHandles == [CmHandleRegistrationResponse.createSuccessResponse('ch-1')]
+
+        and: 'the CM-handle has expected alternate ID'
+            assert objectUnderTest.getNcmpServiceCmHandle('ch-1').alternateId == expectedAlternateId
+
+        cleanup: 'deregister CM handles'
+            deregisterCmHandle(DMI1_URL, 'ch-1')
+
+        where:
+            oldAlternateId | newAlternateId || expectedAlternateId
+            ''             | ''             || ''
+            ''             | 'new'          || 'new'
+            'old'          | 'old'          || 'old'
+            'old'          | null           || 'old'
+            'old'          | ''             || 'old'
+            'old'          | '  '           || 'old'
+    }
+
+    def 'Update of CM-handle with previously set alternate ID fails.'() {
+        given: 'DMI will return modules when requested'
+            dmiDispatcher1.moduleNamesPerCmHandleId = ['ch-1': ['M1', 'M2']]
+        and: 'existing CM-handle with alternate ID'
+            registerCmHandle(DMI1_URL, 'ch-1', NO_MODULE_SET_TAG, 'original')
+
+        when: 'a CM-handle is registered for update with new alternate ID'
+            def cmHandleToUpdate = new NcmpServiceCmHandle(cmHandleId: 'ch-1', alternateId: 'new')
+            def dmiPluginRegistrationResponse =
+                    objectUnderTest.updateDmiRegistrationAndSyncModule(new DmiPluginRegistration(dmiPlugin: DMI1_URL, updatedCmHandles: [cmHandleToUpdate]))
+
+        then: 'registration gives failure response, due to alternate ID being already associated'
+            assert dmiPluginRegistrationResponse.updatedCmHandles == [CmHandleRegistrationResponse.createFailureResponse('ch-1', NcmpResponseStatus.ALTERNATE_ID_ALREADY_ASSOCIATED)]
+
+        and: 'the CM-handle still has the old alternate ID'
+            assert objectUnderTest.getNcmpServiceCmHandle('ch-1').alternateId == 'original'
+
+        cleanup: 'deregister CM handles'
+            deregisterCmHandle(DMI1_URL, 'ch-1')
+    }
+
+}