Robustness cleaning of in progress cache

- Removed cm handle entry from in progress map while moving from locked
  to advised and de-registration
- Fixed groovy tests

Issue-ID: CPS-1239
Signed-off-by: sourabh_sourabh <sourabh.sourabh@est.tech>
Change-Id: Ic527c7b5d209e0c7788888942e95ce0c2c59f7c6

5 files changed, 69 insertions, 22 deletions

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 0b58d44191..3a0f3c73b2 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
@@ -22,6 +22,7 @@
 package org.onap.cps.ncmp.api.impl
 
 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.NetworkCmProxyCmHandlerQueryService
@@ -63,6 +64,7 @@ class NetworkCmProxyDataServiceImplRegistrationSpec extends Specification {
     def stubbedNetworkCmProxyCmHandlerQueryService = Stub(NetworkCmProxyCmHandlerQueryService)
     def mockLcmEventsCmHandleStateHandler = Mock(LcmEventsCmHandleStateHandler)
     def mockCpsDataService = Mock(CpsDataService)
+    def mockModuleSyncStartedOnCmHandles = Mock(IMap<String, Object>)
     def objectUnderTest = getObjectUnderTest()
 
     def 'DMI Registration: Create, Update & Delete operations are processed in the right order'() {
@@ -76,6 +78,8 @@ class NetworkCmProxyDataServiceImplRegistrationSpec extends Specification {
             // Spock validated invocation order between multiple then blocks
         then: 'cm-handles are removed first'
             1 * objectUnderTest.parseAndRemoveCmHandlesInDmiRegistration(*_)
+        and: 'de-registered cm handle entry is removed from in progress map'
+            1 * mockModuleSyncStartedOnCmHandles.remove('cmhandle-2')
         then: 'cm-handles are created'
             1 * objectUnderTest.parseAndCreateCmHandlesInDmiRegistrationAndSyncModules(*_)
         then: 'cm-handles are updated'
@@ -268,7 +272,7 @@ class NetworkCmProxyDataServiceImplRegistrationSpec extends Specification {
             'schema-set does not exist'                         | false
     }
 
-    def 'Remove CmHandle: All cm-handles delete requests are processed'() {
+    def 'Remove CmHandle: Partial Success'() {
         given: 'a registration with three cm-handles to be deleted'
             def dmiPluginRegistration = new DmiPluginRegistration(dmiPlugin: 'my-server',
                 removedCmHandles: ['cmhandle1', 'cmhandle2', 'cmhandle3'])
@@ -278,6 +282,11 @@ class NetworkCmProxyDataServiceImplRegistrationSpec extends Specification {
             def response = objectUnderTest.updateDmiRegistrationAndSyncModule(dmiPluginRegistration)
         then: 'a response is received for all cm-handles'
             response.getRemovedCmHandles().size() == 3
+        and: 'successfully de-registered cm handle entries are removed from in progress map'
+            1 * mockModuleSyncStartedOnCmHandles.remove('cmhandle1')
+            1 * mockModuleSyncStartedOnCmHandles.remove('cmhandle3')
+        and: 'failed de-registered cm handle entries should not be removed from in progress map'
+            0 * mockModuleSyncStartedOnCmHandles.remove('cmhandle2')
         and: '1st and 3rd cm-handle deletes successfully'
             with(response.getRemovedCmHandles().get(0)) {
                 assert it.status == Status.SUCCESS
@@ -347,7 +356,8 @@ class NetworkCmProxyDataServiceImplRegistrationSpec extends Specification {
 
     def getObjectUnderTest() {
         return Spy(new NetworkCmProxyDataServiceImpl(spiedJsonObjectMapper, mockDmiDataOperations,
-            mockNetworkCmProxyDataServicePropertyHandler, mockInventoryPersistence, mockCmhandleQueries,
-                stubbedNetworkCmProxyCmHandlerQueryService, mockLcmEventsCmHandleStateHandler, mockCpsDataService))
+                mockNetworkCmProxyDataServicePropertyHandler, mockInventoryPersistence, mockCmhandleQueries,
+                stubbedNetworkCmProxyCmHandlerQueryService, mockLcmEventsCmHandleStateHandler, mockCpsDataService,
+                mockModuleSyncStartedOnCmHandles))
     }
 }
diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImplSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImplSpec.groovy
index def0db32d9..91eb218eaf 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImplSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServiceImplSpec.groovy
@@ -22,6 +22,7 @@
 
 package org.onap.cps.ncmp.api.impl
 
+import com.hazelcast.map.IMap
 import org.onap.cps.ncmp.api.NetworkCmProxyCmHandlerQueryService
 import org.onap.cps.ncmp.api.impl.event.lcm.LcmEventsCmHandleStateHandler
 import org.onap.cps.ncmp.api.impl.yangmodels.YangModelCmHandle
@@ -39,14 +40,7 @@ import org.onap.cps.spi.exceptions.CpsException
 import org.onap.cps.spi.exceptions.DataValidationException
 import org.onap.cps.spi.model.CmHandleQueryServiceParameters
 import spock.lang.Shared
-
 import java.util.stream.Collectors
-
-import static org.onap.cps.ncmp.api.impl.operations.DmiOperations.DataStoreEnum.PASSTHROUGH_OPERATIONAL
-import static org.onap.cps.ncmp.api.impl.operations.DmiOperations.DataStoreEnum.PASSTHROUGH_RUNNING
-import static org.onap.cps.ncmp.api.impl.operations.DmiRequestBody.OperationEnum.CREATE
-import static org.onap.cps.ncmp.api.impl.operations.DmiRequestBody.OperationEnum.UPDATE
-
 import org.onap.cps.utils.JsonObjectMapper
 import com.fasterxml.jackson.databind.ObjectMapper
 import org.onap.cps.api.CpsDataService
@@ -57,6 +51,11 @@ import org.springframework.http.HttpStatus
 import org.springframework.http.ResponseEntity
 import spock.lang.Specification
 
+import static org.onap.cps.ncmp.api.impl.operations.DmiOperations.DataStoreEnum.PASSTHROUGH_OPERATIONAL
+import static org.onap.cps.ncmp.api.impl.operations.DmiOperations.DataStoreEnum.PASSTHROUGH_RUNNING
+import static org.onap.cps.ncmp.api.impl.operations.DmiRequestBody.OperationEnum.CREATE
+import static org.onap.cps.ncmp.api.impl.operations.DmiRequestBody.OperationEnum.UPDATE
+
 class NetworkCmProxyDataServiceImplSpec extends Specification {
 
     def mockCpsDataService = Mock(CpsDataService)
@@ -68,6 +67,7 @@ class NetworkCmProxyDataServiceImplSpec extends Specification {
     def mockDmiPluginRegistration = Mock(DmiPluginRegistration)
     def mockCpsCmHandlerQueryService = Mock(NetworkCmProxyCmHandlerQueryService)
     def mockLcmEventsCmHandleStateHandler = Mock(LcmEventsCmHandleStateHandler)
+    def stubModuleSyncStartedOnCmHandles = Stub(IMap<String, Object>)
 
     def NO_TOPIC = null
     def NO_REQUEST_ID = null
@@ -84,7 +84,8 @@ class NetworkCmProxyDataServiceImplSpec extends Specification {
             mockCmHandleQueries,
             mockCpsCmHandlerQueryService,
             mockLcmEventsCmHandleStateHandler,
-            mockCpsDataService)
+            mockCpsDataService,
+            stubModuleSyncStartedOnCmHandles)
 
     def cmHandleXPath = "/dmi-registry/cm-handles[@id='testCmHandle']"
 
diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/config/embeddedcache/SynchronizationCacheConfigSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/config/embeddedcache/SynchronizationCacheConfigSpec.groovy
index 80aa81b047..4cfc02b9e7 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/config/embeddedcache/SynchronizationCacheConfigSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/config/embeddedcache/SynchronizationCacheConfigSpec.groovy
@@ -17,8 +17,11 @@
  *  SPDX-License-Identifier: Apache-2.0
  *  ============LICENSE_END=========================================================
  */
+
 package org.onap.cps.ncmp.api.impl.config.embeddedcache
+
 import com.hazelcast.core.Hazelcast
+import com.hazelcast.map.IMap
 import org.onap.cps.spi.model.DataNode
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.boot.test.context.SpringBootTest
@@ -34,7 +37,7 @@ class SynchronizationCacheConfigSpec extends Specification {
     private BlockingQueue<DataNode> moduleSyncWorkQueue
 
     @Autowired
-    private Map<String, Object> moduleSyncStartedOnCmHandles
+    private IMap<String, Object> moduleSyncStartedOnCmHandles
 
     @Autowired
     private Map<String, Boolean> dataSyncSemaphores
diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/inventory/sync/ModuleSyncTasksSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/inventory/sync/ModuleSyncTasksSpec.groovy
index 67fb89dbbe..3deab112aa 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/inventory/sync/ModuleSyncTasksSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/inventory/sync/ModuleSyncTasksSpec.groovy
@@ -21,8 +21,10 @@
 
 package org.onap.cps.ncmp.api.inventory.sync
 
+import com.hazelcast.config.Config
+import com.hazelcast.instance.impl.HazelcastInstanceFactory
+import com.hazelcast.map.IMap
 import org.onap.cps.ncmp.api.impl.event.lcm.LcmEventsCmHandleStateHandler
-import org.onap.cps.ncmp.api.impl.utils.YangDataConverter
 import org.onap.cps.ncmp.api.impl.yangmodels.YangModelCmHandle
 import org.onap.cps.ncmp.api.inventory.CmHandleState
 import org.onap.cps.ncmp.api.inventory.CompositeState
@@ -43,9 +45,14 @@ class ModuleSyncTasksSpec extends Specification {
 
     def mockLcmEventsCmHandleStateHandler = Mock(LcmEventsCmHandleStateHandler)
 
+    IMap<String, Object> moduleSyncStartedOnCmHandles = HazelcastInstanceFactory
+            .getOrCreateHazelcastInstance(new Config('hazelcastInstanceName'))
+            .getMap('mapInstanceName')
+
     def batchCount = new AtomicInteger(5)
 
-    def objectUnderTest = new ModuleSyncTasks(mockInventoryPersistence, mockSyncUtils, mockModuleSyncService, mockLcmEventsCmHandleStateHandler)
+    def objectUnderTest = new ModuleSyncTasks(mockInventoryPersistence, mockSyncUtils, mockModuleSyncService,
+            mockLcmEventsCmHandleStateHandler, moduleSyncStartedOnCmHandles)
 
     def 'Module Sync ADVISED cm handles.'() {
         given: 'cm handles in an ADVISED state'
@@ -95,17 +102,39 @@ class ModuleSyncTasksSpec extends Specification {
                     .withLockReason(LockReasonCategory.LOCKED_MODULE_SYNC_FAILED, '').withLastUpdatedTimeNow().build()
             def yangModelCmHandle1 = new YangModelCmHandle(id: 'cm-handle-1', compositeState: lockedState)
             def yangModelCmHandle2 = new YangModelCmHandle(id: 'cm-handle-2', compositeState: lockedState)
+            def expectedCmHandleStatePerCmHandle = [(yangModelCmHandle1): CmHandleState.ADVISED]
+        and: 'clear in progress map'
+            resetModuleSyncStartedOnCmHandles(moduleSyncStartedOnCmHandles)
+        and: 'add cm handle entry into progress map'
+            moduleSyncStartedOnCmHandles.put('cm-handle-1', 'started')
+            moduleSyncStartedOnCmHandles.put('cm-handle-2', 'started')
         and: 'sync utils retry locked cm handle returns #isReadyForRetry'
             mockSyncUtils.isReadyForRetry(lockedState) >>> isReadyForRetry
         when: 'resetting failed cm handles'
             objectUnderTest.resetFailedCmHandles([yangModelCmHandle1, yangModelCmHandle2])
         then: 'updated to state "ADVISED" from "READY" is called as often as there are cm handles ready for retry'
-//            expectedNumberOfInvocationsToSaveCmHandleState * mockLcmEventsCmHandleStateHandler.updateCmHandleState(_, CmHandleState.ADVISED)
+            expectedNumberOfInvocationsToUpdateCmHandleState * mockLcmEventsCmHandleStateHandler.updateCmHandleStateBatch(expectedCmHandleStatePerCmHandle)
+        and: 'after reset performed size of in progress map'
+            assert moduleSyncStartedOnCmHandles.size() == inProgressMapSize
         where:
-            scenario                        | isReadyForRetry || expectedNumberOfInvocationsToSaveCmHandleState
-            'retry locked cm handle once'   | [true, false]   || 1
-            'retry locked cm handle twice'  | [true, true]    || 2
-            'do not retry locked cm handle' | [false, false]  || 0
+            scenario                        | isReadyForRetry | inProgressMapSize || expectedNumberOfInvocationsToUpdateCmHandleState
+            'retry locked cm handle'        | [true, false]   | 1                 || 1
+            'do not retry locked cm handle' | [false, false]  | 2                 || 0
+    }
+
+    def 'Module Sync ADVISED cm handle without entry in progress map.'() {
+        given: 'cm handles in an ADVISED state'
+            def cmHandle1 = advisedCmHandleAsDataNode('cm-handle-1')
+        and: 'the inventory persistence cm handle returns a ADVISED state for the any handle'
+            mockInventoryPersistence.getCmHandleState(_) >> new CompositeState(cmHandleState: CmHandleState.ADVISED)
+        and: 'entry in progress map for other cm handle'
+            moduleSyncStartedOnCmHandles.put('other-cm-handle', 'started')
+        when: 'module sync poll is executed'
+            objectUnderTest.performModuleSync([cmHandle1], batchCount)
+        then: 'module sync service is invoked for cm handle'
+            1 * mockModuleSyncService.syncAndCreateSchemaSetAndAnchor(_) >> { args -> assertYamgModelCmHandleArgument(args, 'cm-handle-1') }
+        and: 'the entry for other cm handle is still in the progress map'
+            assert moduleSyncStartedOnCmHandles.get('other-cm-handle') != null
     }
 
     def advisedCmHandleAsDataNode(cmHandleId) {
@@ -131,4 +160,8 @@ class ModuleSyncTasksSpec extends Specification {
         }
         return true
     }
+
+    def resetModuleSyncStartedOnCmHandles(moduleSyncStartedOnCmHandles) {
+        moduleSyncStartedOnCmHandles.clear();
+    }
 }
diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/inventory/sync/ModuleSyncWatchdogSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/inventory/sync/ModuleSyncWatchdogSpec.groovy
index dd989bf676..a8bbf7c483 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/inventory/sync/ModuleSyncWatchdogSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/inventory/sync/ModuleSyncWatchdogSpec.groovy
@@ -21,10 +21,10 @@
 
 package org.onap.cps.ncmp.api.inventory.sync
 
+import com.hazelcast.map.IMap
 import org.onap.cps.ncmp.api.impl.yangmodels.YangModelCmHandle
 import org.onap.cps.ncmp.api.inventory.sync.executor.AsyncTaskExecutor
 import java.util.concurrent.ArrayBlockingQueue
-import java.util.concurrent.BlockingQueue
 import org.onap.cps.spi.model.DataNode
 import spock.lang.Specification
 
@@ -36,13 +36,13 @@ class ModuleSyncWatchdogSpec extends Specification {
 
     def moduleSyncWorkQueue = new ArrayBlockingQueue(testQueueCapacity)
 
-    def moduleSyncStartedOnCmHandles = [:]
+    def stubModuleSyncStartedOnCmHandles = Stub(IMap<String, Object>)
 
     def mockModuleSyncTasks = Mock(ModuleSyncTasks)
 
     def spiedAsyncTaskExecutor = Spy(AsyncTaskExecutor)
 
-    def objectUnderTest = new ModuleSyncWatchdog(mockSyncUtils, moduleSyncWorkQueue , moduleSyncStartedOnCmHandles,
+    def objectUnderTest = new ModuleSyncWatchdog(mockSyncUtils, moduleSyncWorkQueue , stubModuleSyncStartedOnCmHandles,
             mockModuleSyncTasks, spiedAsyncTaskExecutor)
 
     void setup() {