Merge "Revert "Introduce Hazelcast for alternateId-cmHandle relation pt. 2 - error...
authorToine Siebelink <toine.siebelink@est.tech>
Thu, 8 Feb 2024 16:36:52 +0000 (16:36 +0000)
committerGerrit Code Review <gerrit@onap.org>
Thu, 8 Feb 2024 16:36:52 +0000 (16:36 +0000)
cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/NcmpResponseStatus.java
cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImpl.java
cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServicePropertyHandler.java
cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/utils/CmHandleIdMapper.java
cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImplRegistrationSpec.groovy
cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServicePropertyHandlerSpec.groovy
cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/utils/CmHandleIdMapperSpec.groovy
docs/cps-ncmp-message-status-codes.rst

index e7293b2..b9c834c 100644 (file)
@@ -36,8 +36,7 @@ public enum NcmpResponseStatus {
     SUBSCRIPTION_PENDING("106", "subscription pending for all cm handles"),
     UNKNOWN_ERROR("108", "Unknown error"),
     CM_HANDLE_ALREADY_EXIST("109", "cm-handle already exists"),
-    CM_HANDLE_INVALID_ID("110", "cm-handle has an invalid character(s) in id"),
-    ALTERNATE_ID_ALREADY_ASSOCIATED("111", "cannot re-assign alternate id");
+    CM_HANDLE_INVALID_ID("110", "cm-handle has an invalid character(s) in id");
 
     private final String code;
     private final String message;
index ad1c5cd..05b83b9 100755 (executable)
@@ -24,7 +24,6 @@
 
 package org.onap.cps.ncmp.api.impl;
 
-import static org.onap.cps.ncmp.api.NcmpResponseStatus.ALTERNATE_ID_ALREADY_ASSOCIATED;
 import static org.onap.cps.ncmp.api.NcmpResponseStatus.CM_HANDLES_NOT_FOUND;
 import static org.onap.cps.ncmp.api.NcmpResponseStatus.CM_HANDLES_NOT_READY;
 import static org.onap.cps.ncmp.api.NcmpResponseStatus.CM_HANDLE_ALREADY_EXIST;
@@ -501,35 +500,16 @@ public class NetworkCmProxyDataServiceImpl implements NetworkCmProxyDataService
     private List<CmHandleRegistrationResponse> registerNewCmHandles(final List<YangModelCmHandle> yangModelCmHandles,
                                                                     final Map<String, TrustLevel>
                                                                             initialTrustLevelPerCmHandleId) {
-        final List<CmHandleRegistrationResponse> failureResponses = new ArrayList<>();
-        final List<YangModelCmHandle> acceptedYangModelCmHandles = new ArrayList<>(yangModelCmHandles.size());
-        final Set<String> acceptedCmHandleIds = new HashSet<>(yangModelCmHandles.size());
-        for (final YangModelCmHandle yangModelCmHandle : yangModelCmHandles) {
-            if (cmHandleIdMapper.isDuplicateId(yangModelCmHandle.getId(), yangModelCmHandle.getAlternateId())) {
-                initialTrustLevelPerCmHandleId.remove(yangModelCmHandle.getId());
-                failureResponses.add(CmHandleRegistrationResponse.createFailureResponse(
-                        yangModelCmHandle.getId(), ALTERNATE_ID_ALREADY_ASSOCIATED));
-            } else {
-                acceptedCmHandleIds.add(yangModelCmHandle.getId());
-                acceptedYangModelCmHandles.add(yangModelCmHandle);
-            }
-        }
+        final Set<String> cmHandleIds = initialTrustLevelPerCmHandleId.keySet();
         try {
-            lcmEventsCmHandleStateHandler.initiateStateAdvised(acceptedYangModelCmHandles);
+            lcmEventsCmHandleStateHandler.initiateStateAdvised(yangModelCmHandles);
             trustLevelManager.handleInitialRegistrationOfTrustLevels(initialTrustLevelPerCmHandleId);
-            final List<CmHandleRegistrationResponse> cmHandleRegistrationResponses = CmHandleRegistrationResponse
-                    .createSuccessResponses(acceptedCmHandleIds);
-            cmHandleRegistrationResponses.addAll(failureResponses);
-            return cmHandleRegistrationResponses;
+            return CmHandleRegistrationResponse.createSuccessResponses(cmHandleIds);
         } catch (final AlreadyDefinedException alreadyDefinedException) {
-            final List<CmHandleRegistrationResponse> alreadyDefinedResponses = CmHandleRegistrationResponse
-                    .createFailureResponses(
+            return CmHandleRegistrationResponse.createFailureResponses(
                     alreadyDefinedException.getAlreadyDefinedObjectNames(), CM_HANDLE_ALREADY_EXIST);
-            failureResponses.addAll(alreadyDefinedResponses);
-            return failureResponses;
         } catch (final Exception exception) {
-            return CmHandleRegistrationResponse
-                    .createFailureResponses(initialTrustLevelPerCmHandleId.keySet(), exception);
+            return CmHandleRegistrationResponse.createFailureResponses(cmHandleIds, exception);
         }
     }
 
@@ -576,4 +556,5 @@ public class NetworkCmProxyDataServiceImpl implements NetworkCmProxyDataService
             }
         }
     }
+
 }
index 0520f45..13b3fca 100644 (file)
@@ -22,7 +22,6 @@
 
 package org.onap.cps.ncmp.api.impl;
 
-import static org.onap.cps.ncmp.api.NcmpResponseStatus.ALTERNATE_ID_ALREADY_ASSOCIATED;
 import static org.onap.cps.ncmp.api.NcmpResponseStatus.CM_HANDLES_NOT_FOUND;
 import static org.onap.cps.ncmp.api.NcmpResponseStatus.CM_HANDLE_INVALID_ID;
 import static org.onap.cps.ncmp.api.impl.NetworkCmProxyDataServicePropertyHandler.PropertyType.DMI_PROPERTY;
@@ -82,17 +81,11 @@ public class NetworkCmProxyDataServicePropertyHandler {
         for (final NcmpServiceCmHandle ncmpServiceCmHandle : ncmpServiceCmHandles) {
             final String cmHandleId = ncmpServiceCmHandle.getCmHandleId();
             try {
-                if (cmHandleIdMapper.isDuplicateId(ncmpServiceCmHandle.getCmHandleId(),
-                        ncmpServiceCmHandle.getAlternateId())) {
-                    cmHandleRegistrationResponses.add(CmHandleRegistrationResponse.createFailureResponse(cmHandleId,
-                            ALTERNATE_ID_ALREADY_ASSOCIATED));
-                } else {
-                    final DataNode existingCmHandleDataNode = inventoryPersistence.getCmHandleDataNode(cmHandleId)
-                            .iterator().next();
-                    updateAlternateId(existingCmHandleDataNode, ncmpServiceCmHandle);
-                    processUpdates(existingCmHandleDataNode, ncmpServiceCmHandle);
-                    cmHandleRegistrationResponses.add(CmHandleRegistrationResponse.createSuccessResponse(cmHandleId));
-                }
+                final DataNode existingCmHandleDataNode = inventoryPersistence.getCmHandleDataNode(cmHandleId)
+                        .iterator().next();
+                updateAlternateId(existingCmHandleDataNode, ncmpServiceCmHandle);
+                processUpdates(existingCmHandleDataNode, ncmpServiceCmHandle);
+                cmHandleRegistrationResponses.add(CmHandleRegistrationResponse.createSuccessResponse(cmHandleId));
             } catch (final DataNodeNotFoundException e) {
                 log.error("Unable to find dataNode for cmHandleId : {} , caused by : {}", cmHandleId, e.getMessage());
                 cmHandleRegistrationResponses.add(
index 40c620f..a88adbd 100644 (file)
@@ -55,10 +55,15 @@ public class CmHandleIdMapper {
 
 
     private boolean addMappingWithValidation(final String cmHandleId, final String alternateId) {
-        if (StringUtils.isBlank(alternateId)) {
+        if (alternateIdPerCmHandleId.containsKey(cmHandleId)) {
+            final String originalAlternateId = alternateIdPerCmHandleId.get(cmHandleId);
+            if (!originalAlternateId.equals(alternateId)) {
+                log.warn("Alternate id update ignored, cannot update cm handle {}, already has an alternate id of {}",
+                        cmHandleId, originalAlternateId);
+            }
             return false;
         }
-        if (isDuplicateId(cmHandleId, alternateId)) {
+        if (StringUtils.isBlank(alternateId)) {
             return false;
         }
         alternateIdPerCmHandleId.put(cmHandleId, alternateId);
@@ -77,32 +82,10 @@ public class CmHandleIdMapper {
         }
     }
 
-    /**
-     * Check if alternate id is already used.
-     *
-     * @param cmHandleId the cmHandle id.
-     * @param alternateId the alternate id.
-     * @return boolean true if the alternate id is already used.
-     */
-    public boolean isDuplicateId(final String cmHandleId, final String alternateId) {
-        if (StringUtils.isBlank(alternateId)) {
-            return false;
-        }
-        if (cmHandleIdPerAlternateId.get(alternateId) != null) {
-            log.warn("The given alternate id was added to the cache already: {}", alternateId);
-            return true;
-        }
-        if (alternateIdPerCmHandleId.get(cmHandleId) != null) {
-            log.warn("The given cmhandle id was added to the cache already: {}", cmHandleId);
-            return true;
-        }
-        return false;
-    }
-
     private void initializeCache() {
         if (!cacheIsInitialized) {
             networkCmProxyCmHandleQueryService.getAllCmHandles().forEach(cmHandle ->
-                    addMappingWithValidation(cmHandle.getCmHandleId(), cmHandle.getAlternateId())
+                addMappingWithValidation(cmHandle.getCmHandleId(), cmHandle.getAlternateId())
             );
             log.info("Alternate ID cache initialized from DB with {} cm handle/alternate id pairs ",
                     alternateIdPerCmHandleId.size());
index faa2225..c7ac8ab 100644 (file)
 
 package org.onap.cps.ncmp.api.impl
 
+import static org.onap.cps.ncmp.api.models.CmHandleRegistrationResponse.Status
+import static org.onap.cps.ncmp.api.NcmpResponseStatus.CM_HANDLES_NOT_FOUND
+import static org.onap.cps.ncmp.api.NcmpResponseStatus.CM_HANDLE_ALREADY_EXIST
+import static org.onap.cps.ncmp.api.NcmpResponseStatus.CM_HANDLE_INVALID_ID
+import static org.onap.cps.ncmp.api.NcmpResponseStatus.UNKNOWN_ERROR
+
+import org.onap.cps.ncmp.api.impl.trustlevel.TrustLevelManager
+import org.onap.cps.ncmp.api.impl.utils.CmHandleIdMapper
+import org.onap.cps.ncmp.api.models.UpgradedCmHandles
 import com.fasterxml.jackson.databind.ObjectMapper
 import com.hazelcast.map.IMap
 import org.onap.cps.api.CpsDataService
 import org.onap.cps.api.CpsModuleService
-import org.onap.cps.ncmp.api.NcmpResponseStatus
 import org.onap.cps.ncmp.api.NetworkCmProxyCmHandleQueryService
 import org.onap.cps.ncmp.api.impl.events.lcm.LcmEventsCmHandleStateHandler
 import org.onap.cps.ncmp.api.impl.exception.DmiRequestException
-import org.onap.cps.ncmp.api.impl.inventory.CmHandleQueries
-import org.onap.cps.ncmp.api.impl.inventory.CmHandleState
-import org.onap.cps.ncmp.api.impl.inventory.InventoryPersistence
 import org.onap.cps.ncmp.api.impl.operations.DmiDataOperations
 import org.onap.cps.ncmp.api.impl.trustlevel.TrustLevel
-import org.onap.cps.ncmp.api.impl.trustlevel.TrustLevelManager
-import org.onap.cps.ncmp.api.impl.utils.CmHandleIdMapper
 import org.onap.cps.ncmp.api.impl.yangmodels.YangModelCmHandle
+import org.onap.cps.ncmp.api.impl.inventory.CmHandleQueries
+import org.onap.cps.ncmp.api.impl.inventory.CmHandleState
+import org.onap.cps.ncmp.api.impl.inventory.InventoryPersistence
 import org.onap.cps.ncmp.api.models.CmHandleRegistrationResponse
 import org.onap.cps.ncmp.api.models.DmiPluginRegistration
 import org.onap.cps.ncmp.api.models.NcmpServiceCmHandle
-import org.onap.cps.ncmp.api.models.UpgradedCmHandles
 import org.onap.cps.spi.exceptions.AlreadyDefinedException
 import org.onap.cps.spi.exceptions.DataNodeNotFoundException
 import org.onap.cps.spi.exceptions.DataValidationException
@@ -48,12 +53,6 @@ import org.onap.cps.spi.exceptions.SchemaSetNotFoundException
 import org.onap.cps.utils.JsonObjectMapper
 import spock.lang.Specification
 
-import static org.onap.cps.ncmp.api.NcmpResponseStatus.CM_HANDLES_NOT_FOUND
-import static org.onap.cps.ncmp.api.NcmpResponseStatus.CM_HANDLE_ALREADY_EXIST
-import static org.onap.cps.ncmp.api.NcmpResponseStatus.CM_HANDLE_INVALID_ID
-import static org.onap.cps.ncmp.api.NcmpResponseStatus.UNKNOWN_ERROR
-import static org.onap.cps.ncmp.api.models.CmHandleRegistrationResponse.Status
-
 class NetworkCmProxyDataServiceImplRegistrationSpec extends Specification {
 
     def ncmpServiceCmHandle = new NcmpServiceCmHandle(cmHandleId: 'some-cm-handle-id')
@@ -442,21 +441,6 @@ class NetworkCmProxyDataServiceImplRegistrationSpec extends Specification {
             1 * mockCmHandleIdMapper.addMapping('cmhandle1', 'my-alternate-id-1')
     }
 
-    def 'Attempting to register a cmhandle with an already cached id.'() {
-        given: 'a registration of a cmhandle with an alternate id'
-            def dmiPluginRegistration = new DmiPluginRegistration(dmiPlugin: 'my-server')
-            dmiPluginRegistration.createdCmHandles = [new NcmpServiceCmHandle(cmHandleId: 'ch-1', alternateId: 'my alternate id')]
-        and: 'one of the ids are duplicated'
-            mockCmHandleIdMapper.isDuplicateId('ch-1', 'my alternate id') >> true
-        when: 'registration is attempted'
-            def response = objectUnderTest.updateDmiRegistrationAndSyncModule(dmiPluginRegistration)
-        then: 'a response is received'
-            assert response != null
-        and: 'the cmhandle has a failed state with the appropriate NCMP response status'
-            assert Status.FAILURE == response.createdCmHandles[0].status
-            assert NcmpResponseStatus.ALTERNATE_ID_ALREADY_ASSOCIATED == response.createdCmHandles[0].ncmpResponseStatus
-    }
-
     def getObjectUnderTest() {
         return Spy(new NetworkCmProxyDataServiceImpl(spiedJsonObjectMapper, mockDmiDataOperations,
                 mockNetworkCmProxyDataServicePropertyHandler, mockInventoryPersistence, mockCmHandleQueries,
index 6ae62cf..f94c34c 100644 (file)
@@ -26,7 +26,6 @@ package org.onap.cps.ncmp.api.impl
 import com.fasterxml.jackson.databind.ObjectMapper
 import org.onap.cps.ncmp.api.impl.utils.CmHandleIdMapper
 
-import static org.onap.cps.ncmp.api.NcmpResponseStatus.ALTERNATE_ID_ALREADY_ASSOCIATED
 import static org.onap.cps.ncmp.api.impl.ncmppersistence.NcmpPersistence.NCMP_DATASPACE_NAME
 import static org.onap.cps.ncmp.api.impl.ncmppersistence.NcmpPersistence.NCMP_DMI_REGISTRY_ANCHOR
 import static org.onap.cps.ncmp.api.NcmpResponseStatus.CM_HANDLES_NOT_FOUND
@@ -220,24 +219,6 @@ class NetworkCmProxyDataServicePropertyHandlerSpec extends Specification {
             1 * mockCmHandleIdMapper.removeMapping(cmHandleId)
     }
 
-    def 'Attempt to update a cmhandle with an already cached alternate id.'() {
-        given: 'cm handles request'
-            def cmHandleUpdateRequest = [new NcmpServiceCmHandle(cmHandleId: cmHandleId, alternateId: 'my alternate id')]
-        and: 'the id already added to the alternate id cache'
-            mockCmHandleIdMapper.isDuplicateId(cmHandleId, 'my alternate id') >> true
-        when: 'update data node leaves is called using correct parameters'
-            def response = objectUnderTest.updateCmHandleProperties(cmHandleUpdateRequest)
-        then: 'one failed registration response'
-            response.size() == 1
-        and: 'it has expected error details'
-            with(response.get(0)) {
-                assert it.status == Status.FAILURE
-                assert it.cmHandle == cmHandleId
-                assert it.ncmpResponseStatus == ALTERNATE_ID_ALREADY_ASSOCIATED
-                assert it.errorText.contains('cannot re-assign')
-            }
-    }
-
     def convertToProperties(expectedPropertiesAfterUpdateAsMap) {
         def properties = [].withDefault { [:] }
         expectedPropertiesAfterUpdateAsMap.forEach(property ->
index 1b323a2..55ccdf3 100644 (file)
@@ -43,7 +43,7 @@ class CmHandleIdMapperSpec extends Specification {
         ((Logger) LoggerFactory.getLogger(CmHandleIdMapper.class)).addAppender(logger)
         logger.start()
         mockCpsCmHandlerQueryService.getAllCmHandles() >> []
-        assert objectUnderTest.addMapping('cached cmhandle id', 'cached alternate id')
+        assert objectUnderTest.addMapping('my cmhandle id', 'my alternate id')
     }
 
     void cleanup() {
@@ -52,9 +52,9 @@ class CmHandleIdMapperSpec extends Specification {
 
     def 'Checking entries in the cache.'() {
         expect: 'the alternate id can be converted to cmhandle id'
-            assert objectUnderTest.alternateIdToCmHandleId('cached alternate id') == 'cached cmhandle id'
+            assert objectUnderTest.alternateIdToCmHandleId('my alternate id') == 'my cmhandle id'
         and: 'the cmhandle id can be converted to alternate id'
-            assert objectUnderTest.cmHandleIdToAlternateId('cached cmhandle id') == 'cached alternate id'
+            assert objectUnderTest.cmHandleIdToAlternateId('my cmhandle id') == 'my alternate id'
     }
 
     def 'Attempt adding #scenario alternate id.'() {
@@ -71,11 +71,11 @@ class CmHandleIdMapperSpec extends Specification {
 
     def 'Remove an entry from the cache.'() {
         when: 'removing an entry'
-            objectUnderTest.removeMapping('cached cmhandle id')
+            objectUnderTest.removeMapping('my cmhandle id')
         then: 'converting alternate id returns null'
-            assert objectUnderTest.alternateIdToCmHandleId('cached alternate id') == null
+            assert objectUnderTest.alternateIdToCmHandleId('my alternate id') == null
         and: 'converting cmhandle id returns null'
-            assert objectUnderTest.cmHandleIdToAlternateId('cached cmhandle id') == null
+            assert objectUnderTest.cmHandleIdToAlternateId('my cmhandle id') == null
     }
 
     def 'Attempt to remove a non-existing entry from the cache.'() {
@@ -89,22 +89,22 @@ class CmHandleIdMapperSpec extends Specification {
 
     def 'Cannot update existing alternate id.'() {
         given: 'attempt to update an existing alternate id'
-            objectUnderTest.addMapping('cached cmhandle id', 'other id')
+            objectUnderTest.addMapping('my cmhandle id', 'other id')
         expect: 'still returns the original alternate id'
-            assert objectUnderTest.cmHandleIdToAlternateId('cached cmhandle id') == 'cached alternate id'
+            assert objectUnderTest.cmHandleIdToAlternateId('my cmhandle id') == 'my alternate id'
         and: 'converting other alternate id returns null'
             assert objectUnderTest.alternateIdToCmHandleId('other id') == null
         and: 'a warning is logged with the original alternate id'
             def lastLoggingEvent = logger.list[1]
             assert lastLoggingEvent.level == Level.WARN
-            assert lastLoggingEvent.formattedMessage.contains('id was added to the cache already')
+            assert lastLoggingEvent.formattedMessage.contains('my alternate id')
     }
 
     def 'Update existing alternate id with the same value.'() {
         expect: 'update an existing alternate id with the same value returns false (no update)'
-            assert objectUnderTest.addMapping('cached cmhandle id', 'cached alternate id') == false
+            assert objectUnderTest.addMapping('my cmhandle id', 'my alternate id') == false
         and: 'conversion still returns the original alternate id'
-            assert objectUnderTest.cmHandleIdToAlternateId('cached cmhandle id') == 'cached alternate id'
+            assert objectUnderTest.cmHandleIdToAlternateId('my cmhandle id') == 'my alternate id'
     }
 
     def 'Initializing cache #scenario.'() {
@@ -123,16 +123,4 @@ class CmHandleIdMapperSpec extends Specification {
             'without alternate id'    | [new NcmpServiceCmHandle(cmHandleId: 'ch-1')]                       || null                | null
             'with blank alternate id' | [new NcmpServiceCmHandle(cmHandleId: 'ch-1', alternateId: ' ')]     || null                | null
     }
-
-    def 'Checking caches for duplicated values when: #scenario.'() {
-        expect: 'duplicate checks works as intended'
-            assert objectUnderTest.isDuplicateId(cmHandleId, alternateId) == expectDuplicate
-        where: 'the following values are given'
-            scenario                                             | cmHandleId           | alternateId           || expectDuplicate
-            'new cm handle'                                      | 'new ch-1'           | 'alt-1'               || false
-            'cm handle with already assigned other alternate id' | 'cached cmhandle id' | 'alt-1'               || true
-            'alternate id already assigned to other cm handle'   | 'ch-1'               | 'cached alternate id' || true
-            'no alternate id provided'                           | 'ch-1'               | null                  || false
-            'cm handle with already assigned same alternate id'  | 'ch-1'               | 'alt-1'               || false
-    }
 }
\ No newline at end of file
index 6b930a8..20a5ae3 100644 (file)
@@ -38,8 +38,6 @@ CPS-NCMP Message Status Codes
     +-----------------+------------------------------------------------------+-----------------------------------+
     | 110             | cm-handle has an invalid character(s) in id          | Inventory                         |
     +-----------------+------------------------------------------------------+-----------------------------------+
-    | 111             | cannot re-assign alternate id                        | Inventory                         |
-    +-----------------+------------------------------------------------------+-----------------------------------+
 
 .. note::