Merge "Handling Yang module upgrade error scenarios"

7 files changed, 108 insertions, 51 deletions

diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImpl.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImpl.java
index 4ec00daea..d1ef7faa8 100755
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImpl.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImpl.java
@@ -39,6 +39,7 @@ import java.text.MessageFormat;
 import java.time.OffsetDateTime;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.EnumMap;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -48,6 +49,7 @@ import java.util.stream.Collectors;
 import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 import org.onap.cps.api.CpsDataService;
+import org.onap.cps.ncmp.api.NcmpResponseStatus;
 import org.onap.cps.ncmp.api.NetworkCmProxyCmHandleQueryService;
 import org.onap.cps.ncmp.api.NetworkCmProxyDataService;
 import org.onap.cps.ncmp.api.impl.events.lcm.LcmEventsCmHandleStateHandler;
@@ -72,7 +74,6 @@ import org.onap.cps.ncmp.api.models.DataOperationRequest;
 import org.onap.cps.ncmp.api.models.DmiPluginRegistration;
 import org.onap.cps.ncmp.api.models.DmiPluginRegistrationResponse;
 import org.onap.cps.ncmp.api.models.NcmpServiceCmHandle;
-import org.onap.cps.ncmp.api.models.UpgradedCmHandles;
 import org.onap.cps.spi.FetchDescendantsOption;
 import org.onap.cps.spi.exceptions.AlreadyDefinedException;
 import org.onap.cps.spi.exceptions.CpsException;
@@ -127,7 +128,6 @@ public class NetworkCmProxyDataServiceImpl implements NetworkCmProxyDataService
             dmiPluginRegistrationResponse.setUpgradedCmHandles(
                     parseAndProcessUpgradedCmHandlesInRegistration(dmiPluginRegistration));
         }
-
         return dmiPluginRegistrationResponse;
     }
 
@@ -365,33 +365,53 @@ public class NetworkCmProxyDataServiceImpl implements NetworkCmProxyDataService
     protected List<CmHandleRegistrationResponse> parseAndProcessUpgradedCmHandlesInRegistration(
             final DmiPluginRegistration dmiPluginRegistration) {
 
-        final UpgradedCmHandles upgradedCmHandles = dmiPluginRegistration.getUpgradedCmHandles();
-        final String moduleSetTag = dmiPluginRegistration.getUpgradedCmHandles().getModuleSetTag();
-        final Map<YangModelCmHandle, CmHandleState> cmHandleStatePerCmHandle =
-                new HashMap<>(upgradedCmHandles.getCmHandles().size());
-        final Collection<String> notReadyCmHandles = new ArrayList<>(upgradedCmHandles.getCmHandles().size());
-        final NcmpServiceCmHandle ncmpServiceCmHandle = new NcmpServiceCmHandle();
-        final String formattedModuleSetTag = MessageFormat.format("new moduleSetTag: {0}", moduleSetTag);
-
-        upgradedCmHandles.getCmHandles().forEach(cmHandleId -> {
-            if (cmHandleQueries.cmHandleHasState(cmHandleId, CmHandleState.READY)) {
-                ncmpServiceCmHandle.setCmHandleId(cmHandleId);
-                ncmpServiceCmHandle.setCompositeState(new CompositeStateBuilder()
-                        .withCmHandleState(CmHandleState.READY)
-                        .withLockReason(MODULE_UPGRADE, formattedModuleSetTag).build());
-                final YangModelCmHandle yangModelCmHandle = YangModelCmHandle.toYangModelCmHandle(
-                        dmiPluginRegistration.getDmiPlugin(),
-                        dmiPluginRegistration.getDmiDataPlugin(),
-                        dmiPluginRegistration.getDmiModelPlugin(),
-                        ncmpServiceCmHandle,
-                        moduleSetTag);
-                cmHandleStatePerCmHandle.put(yangModelCmHandle, CmHandleState.LOCKED);
-            } else {
-                notReadyCmHandles.add(cmHandleId);
+        final List<String> upgradedCmHandleIds = dmiPluginRegistration.getUpgradedCmHandles().getCmHandles();
+
+        final Map<YangModelCmHandle, CmHandleState> acceptedCmHandleStatePerCmHandle
+                = new HashMap<>(upgradedCmHandleIds.size());
+
+        final Map<NcmpResponseStatus, List<String>> failedCmHandlesPerResponseStatus
+                = new EnumMap<>(NcmpResponseStatus.class);
+        final List<String> nonExistingCmHandleIds = new ArrayList<>();
+        final List<String> nonReadyCmHandleIds = new ArrayList<>();
+        final List<String> invalidCmHandleIds = new ArrayList<>();
+
+        upgradedCmHandleIds.forEach(cmHandleId -> {
+            try {
+                if (cmHandleQueries.cmHandleHasState(cmHandleId, CmHandleState.READY)) {
+                    final YangModelCmHandle yangModelCmHandleWithUpgradeDetails
+                            = createYangModelCmHandle(dmiPluginRegistration, cmHandleId);
+                    acceptedCmHandleStatePerCmHandle.put(yangModelCmHandleWithUpgradeDetails, CmHandleState.LOCKED);
+                } else {
+                    nonReadyCmHandleIds.add(cmHandleId);
+                }
+            } catch (final DataNodeNotFoundException dataNodeNotFoundException) {
+                log.error("Unable to find data node for cm handle id : {} , caused by : {}",
+                        cmHandleId, dataNodeNotFoundException.getMessage());
+                nonExistingCmHandleIds.add(cmHandleId);
+            } catch (final DataValidationException dataValidationException) {
+                log.error("Unable to upgrade cm handle id: {}, caused by : {}",
+                        cmHandleId, dataValidationException.getMessage());
+                invalidCmHandleIds.add(cmHandleId);
             }
         });
+        failedCmHandlesPerResponseStatus.put(CM_HANDLES_NOT_READY, nonReadyCmHandleIds);
+        failedCmHandlesPerResponseStatus.put(CM_HANDLES_NOT_FOUND, nonExistingCmHandleIds);
+        failedCmHandlesPerResponseStatus.put(CM_HANDLE_INVALID_ID, invalidCmHandleIds);
+        return mergeAllCmHandleResponses(acceptedCmHandleStatePerCmHandle, failedCmHandlesPerResponseStatus);
+    }
 
-        return prepareAndGetCmHandleUpgradeResponses(cmHandleStatePerCmHandle, notReadyCmHandles);
+    private static YangModelCmHandle createYangModelCmHandle(final DmiPluginRegistration dmiPluginRegistration,
+                                                             final String cmHandleId) {
+        final NcmpServiceCmHandle ncmpServiceCmHandle = new NcmpServiceCmHandle();
+        ncmpServiceCmHandle.setCmHandleId(cmHandleId);
+        final String moduleSetTag = dmiPluginRegistration.getUpgradedCmHandles().getModuleSetTag();
+        final String lockReasonWithModuleSetTag = MessageFormat.format("ModuleSetTag: {0}", moduleSetTag);
+        ncmpServiceCmHandle.setCompositeState(new CompositeStateBuilder().withCmHandleState(CmHandleState.READY)
+                .withLockReason(MODULE_UPGRADE, lockReasonWithModuleSetTag).build());
+        return YangModelCmHandle.toYangModelCmHandle(dmiPluginRegistration.getDmiPlugin(),
+                dmiPluginRegistration.getDmiDataPlugin(), dmiPluginRegistration.getDmiModelPlugin(),
+                ncmpServiceCmHandle, moduleSetTag);
     }
 
     private CmHandleRegistrationResponse deleteCmHandleAndGetCmHandleRegistrationResponse(final String cmHandleId) {
@@ -453,21 +473,22 @@ public class NetworkCmProxyDataServiceImpl implements NetworkCmProxyDataService
             return CmHandleRegistrationResponse.createSuccessResponses(cmHandleIds);
         } catch (final AlreadyDefinedException alreadyDefinedException) {
             return CmHandleRegistrationResponse.createFailureResponses(
-                    alreadyDefinedException.getAlreadyDefinedObjectNames(),
-                    CM_HANDLE_ALREADY_EXIST);
+                    alreadyDefinedException.getAlreadyDefinedObjectNames(), CM_HANDLE_ALREADY_EXIST);
         } catch (final Exception exception) {
             return CmHandleRegistrationResponse.createFailureResponses(cmHandleIds, exception);
         }
     }
 
-    private List<CmHandleRegistrationResponse> prepareAndGetCmHandleUpgradeResponses(final Map<YangModelCmHandle,
-            CmHandleState> cmHandleStatePerCmHandle, final Collection<String> notReadyCmHandles) {
-        final List<CmHandleRegistrationResponse> cmHandleRegistrationResponses
-                = upgradeCmHandles(cmHandleStatePerCmHandle);
-        final List<CmHandleRegistrationResponse> failedCmHandleRegistrationResponses
-                = CmHandleRegistrationResponse.createFailureResponses(notReadyCmHandles, CM_HANDLES_NOT_READY);
-        failedCmHandleRegistrationResponses.forEach(cmHandleRegistrationResponses::add);
-        return cmHandleRegistrationResponses;
+    private List<CmHandleRegistrationResponse> mergeAllCmHandleResponses(
+            final Map<YangModelCmHandle, CmHandleState> acceptedCmHandleStatePerCmHandle,
+            final Map<NcmpResponseStatus, List<String>> failedCmHandlesPerResponseStatus) {
+        final List<CmHandleRegistrationResponse> cmHandleUpgradeResponses
+                = upgradeCmHandles(acceptedCmHandleStatePerCmHandle);
+        failedCmHandlesPerResponseStatus.forEach((ncmpResponseStatus, cmHandleIds) -> {
+            cmHandleIds.forEach(cmHandleId -> cmHandleUpgradeResponses.add(CmHandleRegistrationResponse
+                    .createFailureResponse(cmHandleId, ncmpResponseStatus)));
+        });
+        return cmHandleUpgradeResponses;
     }
 
     private List<CmHandleRegistrationResponse> upgradeCmHandles(final Map<YangModelCmHandle, CmHandleState>
@@ -477,8 +498,9 @@ public class NetworkCmProxyDataServiceImpl implements NetworkCmProxyDataService
         try {
             lcmEventsCmHandleStateHandler.updateCmHandleStateBatch(cmHandleStatePerCmHandle);
             return CmHandleRegistrationResponse.createSuccessResponses(cmHandleIds);
-        } catch (final Exception exception) {
-            return CmHandleRegistrationResponse.createFailureResponses(cmHandleIds, exception);
+        } catch (final Exception e) {
+            log.error("Unable to update cmHandleIds : {} , caused by : {}", cmHandleIds, e.getMessage());
+            return CmHandleRegistrationResponse.createFailureResponses(cmHandleIds, e);
         }
     }
 
diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServicePropertyHandler.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServicePropertyHandler.java
index 401b19036..25ded162b 100644
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServicePropertyHandler.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServicePropertyHandler.java
@@ -73,18 +73,15 @@ public class NetworkCmProxyDataServicePropertyHandler {
                 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());
+                log.error("Unable to find dataNode for cmHandleId : {} , caused by : {}", cmHandleId, e.getMessage());
                 cmHandleRegistrationResponses.add(CmHandleRegistrationResponse
                     .createFailureResponse(cmHandleId, CM_HANDLES_NOT_FOUND));
             } catch (final DataValidationException e) {
-                log.error("Unable to update cm handle : {}, caused by : {}",
-                    cmHandleId, e.getMessage());
+                log.error("Unable to update cm handle : {}, caused by : {}", cmHandleId, e.getMessage());
                 cmHandleRegistrationResponses.add(
                     CmHandleRegistrationResponse.createFailureResponse(cmHandleId, CM_HANDLE_INVALID_ID));
             } catch (final Exception exception) {
-                log.error("Unable to update cmHandle : {} , caused by : {}",
-                    cmHandleId, exception.getMessage());
+                log.error("Unable to update cmHandle : {} , caused by : {}", cmHandleId, exception.getMessage());
                 cmHandleRegistrationResponses.add(
                     CmHandleRegistrationResponse.createFailureResponse(cmHandleId, exception));
             }
diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/CmHandleQueriesImpl.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/CmHandleQueriesImpl.java
index e5cf8edd6..419d0a345 100644
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/CmHandleQueriesImpl.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/CmHandleQueriesImpl.java
@@ -40,6 +40,7 @@ import org.onap.cps.ncmp.api.impl.trustlevel.TrustLevelFilter;
 import org.onap.cps.spi.CpsDataPersistenceService;
 import org.onap.cps.spi.FetchDescendantsOption;
 import org.onap.cps.spi.model.DataNode;
+import org.onap.cps.spi.utils.CpsValidator;
 import org.springframework.stereotype.Component;
 
 @RequiredArgsConstructor
@@ -50,6 +51,7 @@ public class CmHandleQueriesImpl implements CmHandleQueries {
     private static final String ANCESTOR_CM_HANDLES = "/ancestor::cm-handles";
     private final CpsDataPersistenceService cpsDataPersistenceService;
     private final Map<String, TrustLevel> trustLevelPerCmHandle;
+    private final CpsValidator cpsValidator;
 
     @Override
     public Collection<String> queryCmHandleAdditionalProperties(final Map<String, String> privatePropertyQueryPairs) {
@@ -154,6 +156,7 @@ public class CmHandleQueriesImpl implements CmHandleQueries {
     }
 
     private DataNode getCmHandleState(final String cmHandleId) {
+        cpsValidator.validateNameCharacters(cmHandleId);
         final String xpath = NCMP_DMI_REGISTRY_PARENT + "/cm-handles[@id='" + cmHandleId + "']/state";
         return cpsDataPersistenceService.getDataNodes(NCMP_DATASPACE_NAME, NCMP_DMI_REGISTRY_ANCHOR,
                 xpath, OMIT_DESCENDANTS).iterator().next();
diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncTasks.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncTasks.java
index c19dbeb90..facaf155f 100644
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncTasks.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncTasks.java
@@ -73,10 +73,9 @@ public class ModuleSyncTasks {
                     yangModelCmHandle.getCompositeState().setLockReason(null);
                     cmHandelStatePerCmHandle.put(yangModelCmHandle, CmHandleState.READY);
                 } catch (final Exception e) {
-                    log.warn("Processing of {} module sync failed due to reason {}.",
-                            cmHandleId, e.getMessage());
-                    syncUtils.updateLockReasonDetailsAndAttempts(compositeState,
-                            LockReasonCategory.MODULE_SYNC_FAILED, e.getMessage());
+                    log.warn("Processing of {} module sync failed due to reason {}.", cmHandleId, e.getMessage());
+                    syncUtils.updateLockReasonDetailsAndAttempts(compositeState, LockReasonCategory.MODULE_SYNC_FAILED,
+                            e.getMessage());
                     setCmHandleStateLocked(yangModelCmHandle, compositeState.getLockReason());
                     cmHandelStatePerCmHandle.put(yangModelCmHandle, CmHandleState.LOCKED);
                 }
diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/models/CmHandleRegistrationResponse.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/models/CmHandleRegistrationResponse.java
index e007491ce..b0aed5763 100644
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/models/CmHandleRegistrationResponse.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/models/CmHandleRegistrationResponse.java
@@ -26,6 +26,7 @@ import static org.onap.cps.ncmp.api.NcmpResponseStatus.UNKNOWN_ERROR;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
+import java.util.stream.Collectors;
 import lombok.Builder;
 import lombok.Data;
 import lombok.extern.slf4j.Slf4j;
@@ -116,7 +117,8 @@ public class CmHandleRegistrationResponse {
     }
 
     public static List<CmHandleRegistrationResponse> createSuccessResponses(final List<String> cmHandleIds) {
-        return cmHandleIds.stream().map(CmHandleRegistrationResponse::createSuccessResponse).toList();
+        return cmHandleIds.stream().map(CmHandleRegistrationResponse::createSuccessResponse)
+                .collect(Collectors.toList());
     }
 
     public enum Status {
diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImplRegistrationSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImplRegistrationSpec.groovy
index c87efce04..5588ec781 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImplRegistrationSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImplRegistrationSpec.groovy
@@ -72,7 +72,7 @@ class NetworkCmProxyDataServiceImplRegistrationSpec extends Specification {
     def objectUnderTest = getObjectUnderTest()
 
     def 'DMI Registration: Create, Update, Delete & Upgrade operations are processed in the right order'() {
-        given: 'a registration with operations of all three types'
+        given: 'a registration with operations of all types'
             def dmiRegistration = new DmiPluginRegistration(dmiPlugin: 'my-server')
             dmiRegistration.setCreatedCmHandles([new NcmpServiceCmHandle(cmHandleId: 'cmhandle-1', publicProperties: ['publicProp1': 'value'], dmiProperties: [:])])
             dmiRegistration.setUpdatedCmHandles([new NcmpServiceCmHandle(cmHandleId: 'cmhandle-2', publicProperties: ['publicProp1': 'value'], dmiProperties: [:])])
@@ -96,6 +96,38 @@ class NetworkCmProxyDataServiceImplRegistrationSpec extends Specification {
             1 * objectUnderTest.parseAndProcessUpgradedCmHandlesInRegistration(*_)
     }
 
+    def 'DMI Registration upgrade operation with upgrade node state #scenario'() {
+        given: 'a registration with upgrade operation'
+            def dmiRegistration = new DmiPluginRegistration(dmiPlugin: 'my-server')
+            dmiRegistration.setUpgradedCmHandles(new UpgradedCmHandles(cmHandles: ['cmhandle-3'], moduleSetTag: 'some-module-set-tag'))
+        and: 'exception while checking cm handle state'
+            mockCmHandleQueries.cmHandleHasState('cmhandle-3', CmHandleState.READY) >> isReady
+        when: 'registration is processed'
+            def result = objectUnderTest.updateDmiRegistrationAndSyncModule(dmiRegistration)
+        then: 'upgrade operation contains expected error code'
+            assert result.upgradedCmHandles.status[0] == expectedResponseStatus
+        where: 'the following parameters are used'
+            scenario    | isReady || expectedResponseStatus
+            'READY'     | true    || Status.SUCCESS
+            'Not READY' | false   || Status.FAILURE
+    }
+
+    def 'DMI Registration upgrade with exception #scenario'() {
+        given: 'a registration with upgrade operation'
+            def dmiRegistration = new DmiPluginRegistration(dmiPlugin: 'my-server')
+            dmiRegistration.setUpgradedCmHandles(new UpgradedCmHandles(cmHandles: ['cmhandle-3'], moduleSetTag: 'some-module-set-tag'))
+        and: 'exception while checking cm handle state'
+            mockCmHandleQueries.cmHandleHasState('cmhandle-3', CmHandleState.READY) >> { throw exception }
+        when: 'registration is processed'
+            def result = objectUnderTest.updateDmiRegistrationAndSyncModule(dmiRegistration)
+        then: 'upgrade operation contains expected error code'
+            assert result.upgradedCmHandles.ncmpResponseStatus.code[0] == expectedErrorCode
+        where: 'the following parameters are used'
+            scenario               | exception                                                                || expectedErrorCode
+            'data node not found'  | new DataNodeNotFoundException('some-dataspace-name', 'some-anchor-name') || '100'
+            'cm handle is invalid' | new DataValidationException('some error message', 'some error details')  || '110'
+    }
+
     def 'DMI Registration: Response from all operations types are in response'() {
         given: 'a registration with operations of all three types'
             def dmiRegistration = new DmiPluginRegistration(dmiPlugin: 'my-server')
diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/inventory/CmHandleQueriesImplSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/inventory/CmHandleQueriesImplSpec.groovy
index 78b09e6a1..1da3a55a5 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/inventory/CmHandleQueriesImplSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/inventory/CmHandleQueriesImplSpec.groovy
@@ -22,6 +22,7 @@
 package org.onap.cps.ncmp.api.impl.inventory
 
 import org.onap.cps.ncmp.api.impl.trustlevel.TrustLevel
+import org.onap.cps.spi.utils.CpsValidator
 
 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
@@ -40,9 +41,10 @@ import spock.lang.Specification
 
 class CmHandleQueriesImplSpec extends Specification {
     def cpsDataPersistenceService = Mock(CpsDataPersistenceService)
+    def mockCpsValidator = Mock(CpsValidator)
     def trustLevelPerCmHandle = [ 'my completed cm handle': TrustLevel.COMPLETE, 'my untrusted cm handle': TrustLevel.NONE ]
 
-    def objectUnderTest = new CmHandleQueriesImpl(cpsDataPersistenceService, trustLevelPerCmHandle)
+    def objectUnderTest = new CmHandleQueriesImpl(cpsDataPersistenceService, trustLevelPerCmHandle, mockCpsValidator)
 
     @Shared
     def static sampleDataNodes = [new DataNode()]