Unable to change state from LOCKED to ADVISED

-Refactor Cm Handle Registration to deal with null
pointer during Module Sync Retry
-Add Last Update Time to cm handle registration
-Add Fetch Descendants Option as paramater
for get misbehaving locked cm handles
-Fixes issue when state that goes to advised
after retry mechanism won't be retried again

Issue-ID: CPS-1097
Change-Id: Iffe1cef3479a796ea5c78b293a0bd24a86d13efd
Signed-off-by: DylanB95EST <dylan.byrne@est.tech>
Signed-off-by: sourabh_sourabh <sourabh.sourabh@est.tech>

7 files changed, 59 insertions, 40 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 f8cab4f1ca..f44c28c713 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
@@ -51,6 +51,7 @@ import org.onap.cps.ncmp.api.impl.operations.DmiOperations;
 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;
 import org.onap.cps.ncmp.api.inventory.InventoryPersistence;
 import org.onap.cps.ncmp.api.inventory.sync.ModuleSyncService;
 import org.onap.cps.ncmp.api.models.CmHandleQueryApiParameters;
@@ -231,13 +232,14 @@ public class NetworkCmProxyDataServiceImpl implements NetworkCmProxyDataService
         List<CmHandleRegistrationResponse> cmHandleRegistrationResponses = new ArrayList<>();
         try {
             cmHandleRegistrationResponses = dmiPluginRegistration.getCreatedCmHandles().stream()
-                .map(cmHandle ->
-                    YangModelCmHandle.toYangModelCmHandle(
+                .map(cmHandle -> {
+                    setCompositeStateToAdvised(cmHandle);
+                    return YangModelCmHandle.toYangModelCmHandle(
                         dmiPluginRegistration.getDmiPlugin(),
                         dmiPluginRegistration.getDmiDataPlugin(),
                         dmiPluginRegistration.getDmiModelPlugin(),
-                        CmHandleState.ADVISED,
-                        cmHandle)
+                        cmHandle);
+                    }
                 )
                 .map(this::registerNewCmHandle)
                 .collect(Collectors.toList());
@@ -250,6 +252,13 @@ public class NetworkCmProxyDataServiceImpl implements NetworkCmProxyDataService
         return cmHandleRegistrationResponses;
     }
 
+    private void setCompositeStateToAdvised(final NcmpServiceCmHandle ncmpServiceCmHandle) {
+        final CompositeState compositeState = new CompositeState();
+        compositeState.setCmHandleState(CmHandleState.ADVISED);
+        compositeState.setLastUpdateTimeNow();
+        ncmpServiceCmHandle.setCompositeState(compositeState);
+    }
+
     protected List<CmHandleRegistrationResponse> parseAndRemoveCmHandlesInDmiRegistration(
             final List<String> tobeRemovedCmHandles) {
         final List<CmHandleRegistrationResponse> cmHandleRegistrationResponses =
diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/utils/YangDataConverter.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/utils/YangDataConverter.java
index 82ea00eb32..95ff48a9c2 100644
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/utils/YangDataConverter.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/utils/YangDataConverter.java
@@ -78,8 +78,7 @@ public class YangDataConverter {
                 (String) cmHandleDataNode.getLeaves().get("dmi-service-name"),
                 (String) cmHandleDataNode.getLeaves().get("dmi-data-service-name"),
                 (String) cmHandleDataNode.getLeaves().get("dmi-model-service-name"),
-                ncmpServiceCmHandle.getCompositeState().getCmHandleState(),
-                ncmpServiceCmHandle
+            ncmpServiceCmHandle
         );
     }
 
diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/yangmodels/YangModelCmHandle.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/yangmodels/YangModelCmHandle.java
index 5b719054a6..65e03f1f9d 100644
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/yangmodels/YangModelCmHandle.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/yangmodels/YangModelCmHandle.java
@@ -34,9 +34,7 @@ import lombok.Getter;
 import lombok.NoArgsConstructor;
 import lombok.Setter;
 import org.onap.cps.ncmp.api.impl.operations.RequiredDmiService;
-import org.onap.cps.ncmp.api.inventory.CmHandleState;
 import org.onap.cps.ncmp.api.inventory.CompositeState;
-import org.onap.cps.ncmp.api.inventory.CompositeStateBuilder;
 import org.onap.cps.ncmp.api.models.NcmpServiceCmHandle;
 import org.onap.cps.utils.CpsValidator;
 
@@ -70,8 +68,6 @@ public class YangModelCmHandle {
     @JsonProperty("public-properties")
     private List<Property> publicProperties;
 
-    private static final CompositeStateBuilder compositeStateBuilder = new CompositeStateBuilder();
-
     /**
      * Create a yangModelCmHandle.
      *
@@ -84,7 +80,6 @@ public class YangModelCmHandle {
     public static YangModelCmHandle toYangModelCmHandle(final String dmiServiceName,
                                                         final String dmiDataServiceName,
                                                         final String dmiModelServiceName,
-                                                        final CmHandleState cmHandleState,
                                                         final NcmpServiceCmHandle ncmpServiceCmHandle) {
         CpsValidator.validateNameCharacters(ncmpServiceCmHandle.getCmHandleId());
         final YangModelCmHandle yangModelCmHandle = new YangModelCmHandle();
@@ -95,8 +90,7 @@ public class YangModelCmHandle {
         yangModelCmHandle.setDmiProperties(asYangModelCmHandleProperties(ncmpServiceCmHandle.getDmiProperties()));
         yangModelCmHandle.setPublicProperties(asYangModelCmHandleProperties(
                 ncmpServiceCmHandle.getPublicProperties()));
-        compositeStateBuilder.withCmHandleState(cmHandleState);
-        yangModelCmHandle.setCompositeState(compositeStateBuilder.build());
+        yangModelCmHandle.setCompositeState(ncmpServiceCmHandle.getCompositeState());
         return yangModelCmHandle;
     }
 
diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/CompositeStateBuilder.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/CompositeStateBuilder.java
index f4d96389de..012ba5ede2 100644
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/CompositeStateBuilder.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/CompositeStateBuilder.java
@@ -1,7 +1,7 @@
 /*
  * ============LICENSE_START=======================================================
  * Copyright (C) 2022 Bell Canada
- * Copyright (C) 2022 Nordix Foundation.
+ * Modifications Copyright (C) 2022 Nordix Foundation.
  * ================================================================================
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -115,18 +115,13 @@ public class CompositeStateBuilder {
                 .get("cm-handle-state"));
         for (final DataNode stateChildNode : dataNode.getChildDataNodes()) {
             if (stateChildNode.getXpath().endsWith("/lock-reason")) {
-                this.lockReason = new LockReason(LockReasonCategory.valueOf(
-                        (String) stateChildNode.getLeaves().get("reason")),
-                        (String) stateChildNode.getLeaves().get("details"));
+                this.lockReason = getLockReason(stateChildNode);
             }
             if (stateChildNode.getXpath().endsWith("/datastores")) {
                 for (final DataNode dataStoreNodes : stateChildNode.getChildDataNodes()) {
                     Operational operationalDataStore = null;
                     if (dataStoreNodes.getXpath().contains("/operational")) {
-                        operationalDataStore = Operational.builder()
-                            .syncState(SyncState.valueOf((String) dataStoreNodes.getLeaves().get("sync-state")))
-                            .lastSyncTime((String) dataStoreNodes.getLeaves().get("last-sync-time"))
-                            .build();
+                        operationalDataStore = getOperationalDataStore(dataStoreNodes);
                     }
                     this.datastores = DataStores.builder().operationalDataStore(operationalDataStore).build();
                 }
@@ -135,4 +130,18 @@ public class CompositeStateBuilder {
         return this;
     }
 
+    private Operational getOperationalDataStore(final DataNode dataStoreNodes) {
+        return Operational.builder()
+                .syncState(SyncState.valueOf((String) dataStoreNodes.getLeaves().get("sync-state")))
+                .lastSyncTime((String) dataStoreNodes.getLeaves().get("last-sync-time"))
+                .build();
+    }
+
+    private LockReason getLockReason(final DataNode stateChildNode) {
+        final boolean isLockReasonExists = stateChildNode.getLeaves().containsKey("reason");
+        return new LockReason(isLockReasonExists
+                ? LockReasonCategory.valueOf((String) stateChildNode.getLeaves().get("reason"))
+                : null, (String) stateChildNode.getLeaves().get("details"));
+    }
+
 }
diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/InventoryPersistence.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/InventoryPersistence.java
index 1985bd959a..c24063cb45 100644
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/InventoryPersistence.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/InventoryPersistence.java
@@ -60,7 +60,7 @@ public class InventoryPersistence {
      */
     public CompositeState getCmHandleState(final String cmHandleId) {
         final DataNode stateAsDataNode = cpsDataService.getDataNode(NCMP_DATASPACE_NAME, NCMP_DMI_REGISTRY_ANCHOR,
-                String.format(XPATH_TO_CM_HANDLE, cmHandleId) + "/state",
+            String.format(XPATH_TO_CM_HANDLE, cmHandleId) + "/state",
             FetchDescendantsOption.INCLUDE_ALL_DESCENDANTS);
         return compositeStateBuilder.fromDataNode(stateAsDataNode).build();
     }
@@ -96,11 +96,13 @@ public class InventoryPersistence {
      * Method to return data nodes representing the cm handles.
      *
      * @param cpsPath cps path for which the cmHandle is requested
+     * @param fetchDescendantsOption defines the scope of data to fetch: either single node or all the descendant nodes
      * @return a list of data nodes representing the cm handles.
      */
-    public List<DataNode> getCmHandleDataNodesByCpsPath(final String cpsPath) {
+    public List<DataNode> getCmHandleDataNodesByCpsPath(final String cpsPath,
+                                                        final FetchDescendantsOption fetchDescendantsOption) {
         return cpsDataPersistenceService.queryDataNodes(
-                NCMP_DATASPACE_NAME, NCMP_DMI_REGISTRY_ANCHOR, cpsPath, FetchDescendantsOption.OMIT_DESCENDANTS);
+            NCMP_DATASPACE_NAME, NCMP_DMI_REGISTRY_ANCHOR, cpsPath, fetchDescendantsOption);
     }
 
     /**
@@ -111,9 +113,9 @@ public class InventoryPersistence {
      */
     public List<DataNode> getCmHandlesByIdAndState(final String cmHandleId, final CmHandleState cmHandleState) {
         return cpsDataPersistenceService.queryDataNodes(NCMP_DATASPACE_NAME,
-                NCMP_DMI_REGISTRY_ANCHOR, "//cm-handles[@id='" + cmHandleId + "']/state[@cm-handle-state=\""
-                        + cmHandleState + "\"]/ancestor::cm-handles",
-                FetchDescendantsOption.OMIT_DESCENDANTS);
+            NCMP_DMI_REGISTRY_ANCHOR, "//cm-handles[@id='" + cmHandleId + "']/state[@cm-handle-state=\""
+                + cmHandleState + "\"]/ancestor::cm-handles",
+            FetchDescendantsOption.OMIT_DESCENDANTS);
     }
 
     /**
@@ -123,9 +125,9 @@ public class InventoryPersistence {
      */
     public List<DataNode> getCmHandlesByOperationalSyncState(final SyncState syncState) {
         return cpsDataPersistenceService.queryDataNodes(NCMP_DATASPACE_NAME,
-                NCMP_DMI_REGISTRY_ANCHOR, "//state/datastores"
-                        + "/operational[@sync-state=\"" + syncState + "\"]/ancestor::cm-handles",
-                FetchDescendantsOption.OMIT_DESCENDANTS);
+            NCMP_DMI_REGISTRY_ANCHOR, "//state/datastores"
+                + "/operational[@sync-state=\"" + syncState + "\"]/ancestor::cm-handles",
+            FetchDescendantsOption.OMIT_DESCENDANTS);
     }
 
     /**
@@ -145,4 +147,4 @@ public class InventoryPersistence {
             FetchDescendantsOption.INCLUDE_ALL_DESCENDANTS);
     }
 
-}
+}
\ No newline at end of file
diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/sync/ModuleSyncWatchdog.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/sync/ModuleSyncWatchdog.java
index c920649b8b..e590ca1cd0 100644
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/sync/ModuleSyncWatchdog.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/sync/ModuleSyncWatchdog.java
@@ -27,7 +27,6 @@ import lombok.extern.slf4j.Slf4j;
 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;
-import org.onap.cps.ncmp.api.inventory.CompositeState.LockReason;
 import org.onap.cps.ncmp.api.inventory.InventoryPersistence;
 import org.onap.cps.ncmp.api.inventory.LockReasonCategory;
 import org.springframework.scheduling.annotation.Scheduled;
@@ -77,14 +76,19 @@ public class ModuleSyncWatchdog {
     @Scheduled(fixedDelayString = "${timers.locked-modules-sync.sleep-time-ms:300000}")
     public void executeLockedMisbehavingCmHandlePoll() {
         final List<YangModelCmHandle> lockedMisbehavingCmHandles = syncUtils.getLockedMisbehavingYangModelCmHandles();
-        for (final YangModelCmHandle lockedMisbehavingModelCmHandle: lockedMisbehavingCmHandles) {
-            final CompositeState updatedCompositeState = lockedMisbehavingModelCmHandle.getCompositeState();
-            updatedCompositeState.setCmHandleState(CmHandleState.ADVISED);
-            updatedCompositeState.setLastUpdateTimeNow();
-            updatedCompositeState.setLockReason(LockReason.builder()
-                .details(updatedCompositeState.getLockReason().getDetails()).build());
+        for (final YangModelCmHandle lockedMisbehavingModelCmHandle : lockedMisbehavingCmHandles) {
+            final CompositeState compositeState = lockedMisbehavingModelCmHandle.getCompositeState();
+            setCompositeStateToAdvisedAndRetainOldLockReasonDetails(compositeState);
             log.debug("Locked misbehaving cm handle {} is being recycled", lockedMisbehavingModelCmHandle.getId());
-            inventoryPersistence.saveCmHandleState(lockedMisbehavingModelCmHandle.getId(), updatedCompositeState);
+            inventoryPersistence.saveCmHandleState(lockedMisbehavingModelCmHandle.getId(), compositeState);
         }
     }
+
+    private void setCompositeStateToAdvisedAndRetainOldLockReasonDetails(final CompositeState compositeState) {
+        compositeState.setCmHandleState(CmHandleState.ADVISED);
+        compositeState.setLastUpdateTimeNow();
+        final String oldLockReasonDetails = compositeState.getLockReason().getDetails();
+        compositeState.setLockReason(CompositeState.LockReason.builder()
+                .details(oldLockReasonDetails).build());
+    }
 }
diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/sync/SyncUtils.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/sync/SyncUtils.java
index b5456ab149..42edcb7ec8 100644
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/sync/SyncUtils.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/sync/SyncUtils.java
@@ -43,6 +43,7 @@ import org.onap.cps.ncmp.api.inventory.CompositeState;
 import org.onap.cps.ncmp.api.inventory.InventoryPersistence;
 import org.onap.cps.ncmp.api.inventory.LockReasonCategory;
 import org.onap.cps.ncmp.api.inventory.SyncState;
+import org.onap.cps.spi.FetchDescendantsOption;
 import org.onap.cps.spi.model.DataNode;
 import org.onap.cps.utils.JsonObjectMapper;
 import org.springframework.http.ResponseEntity;
@@ -111,7 +112,8 @@ public class SyncUtils {
      */
     public List<YangModelCmHandle> getLockedMisbehavingYangModelCmHandles() {
         final List<DataNode> lockedCmHandleAsDataNodeList = inventoryPersistence.getCmHandleDataNodesByCpsPath(
-            "//lock-reason[@reason=\"LOCKED_MISBEHAVING\"]/ancestor::cm-handles");
+            "//lock-reason[@reason=\"LOCKED_MISBEHAVING\"]/ancestor::cm-handles",
+            FetchDescendantsOption.INCLUDE_ALL_DESCENDANTS);
         return lockedCmHandleAsDataNodeList.stream()
             .map(cmHandle -> YangDataConverter.convertCmHandleToYangModel(cmHandle,
                 cmHandle.getLeaves().get("id").toString())).collect(Collectors.toList());