Fix handling of blank moduleSetTag

Handling of blank moduleSetTag is very inconsistent.
Sometimes it will be returned as null, '' or 'not-specified'.
Specifically in the case of MODULE_UPGRADE_FAILED, on the retry,
a blank moduleSetTag will be returned as 'not-specified', leading
to incorrect upgrade.

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

3 files changed, 10 insertions, 8 deletions

diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleOperationsUtils.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleOperationsUtils.java
index 22bbc7383..7b2e3d589 100644
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleOperationsUtils.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleOperationsUtils.java
@@ -38,7 +38,6 @@ 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.ncmp.api.impl.inventory.CmHandleQueries;
 import org.onap.cps.ncmp.api.impl.inventory.CmHandleState;
 import org.onap.cps.ncmp.api.impl.inventory.CompositeState;
@@ -62,7 +61,7 @@ public class ModuleOperationsUtils {
     private final DmiDataOperations dmiDataOperations;
     private final JsonObjectMapper jsonObjectMapper;
     private static final String RETRY_ATTEMPT_KEY = "attempt";
-    public static final String MODULE_SET_TAG_KEY = "moduleSetTag";
+    private static final String MODULE_SET_TAG_KEY = "moduleSetTag";
     public static final String MODULE_SET_TAG_MESSAGE_FORMAT = "Upgrade to ModuleSetTag: {0}";
     private static final String UPGRADE_FORMAT = "Upgrade to ModuleSetTag: %s";
     private static final String LOCK_REASON_DETAILS_MSG_FORMAT = UPGRADE_FORMAT + " Attempt #%d failed: %s";
@@ -134,10 +133,10 @@ public class ModuleOperationsUtils {
         if (!compositeStateDetails.isEmpty() && compositeStateDetails.containsKey(RETRY_ATTEMPT_KEY)) {
             attempt = 1 + Integer.parseInt(compositeStateDetails.get(RETRY_ATTEMPT_KEY));
         }
-        final String moduleSetTag = compositeStateDetails.get(MODULE_SET_TAG_KEY);
+        final String moduleSetTag = compositeStateDetails.getOrDefault(MODULE_SET_TAG_KEY, "");
         compositeState.setLockReason(CompositeState.LockReason.builder()
-                .details(String.format(LOCK_REASON_DETAILS_MSG_FORMAT, StringUtils.isNotBlank(moduleSetTag)
-                        ? moduleSetTag : "not-specified", attempt, errorMessage)).lockReasonCategory(lockReasonCategory)
+                .details(String.format(LOCK_REASON_DETAILS_MSG_FORMAT, moduleSetTag, attempt, errorMessage))
+                .lockReasonCategory(lockReasonCategory)
                 .build());
     }
 
@@ -228,6 +227,10 @@ public class ModuleOperationsUtils {
                 .getLockReasonCategory()));
     }
 
+    public static String getUpgradedModuleSetTagFromLockReason(final CompositeState.LockReason lockReason) {
+        return getLockedCompositeStateDetails(lockReason).getOrDefault(MODULE_SET_TAG_KEY, "");
+    }
+
     private String getFirstResource(final Object responseBody) {
         final String jsonObjectAsString = jsonObjectMapper.asJsonString(responseBody);
         final JsonNode overallJsonNode = jsonObjectMapper.convertToJsonNode(jsonObjectAsString);
diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncService.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncService.java
index 72d322605..d4ef5c612 100644
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncService.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncService.java
@@ -187,8 +187,7 @@ public class ModuleSyncService {
                                    final CompositeState compositeState,
                                    final boolean inUpgrade) {
         if (inUpgrade) {
-            return ModuleOperationsUtils.getLockedCompositeStateDetails(compositeState.getLockReason())
-                    .get(ModuleOperationsUtils.MODULE_SET_TAG_KEY);
+            return ModuleOperationsUtils.getUpgradedModuleSetTagFromLockReason(compositeState.getLockReason());
         }
         return yangModelCmHandle.getModuleSetTag();
     }
diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleOperationsUtilsSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleOperationsUtilsSpec.groovy
index 827a548ae..44bc18200 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleOperationsUtilsSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleOperationsUtilsSpec.groovy
@@ -125,7 +125,7 @@ class ModuleOperationsUtilsSpec extends Specification{
         where:
             scenario               | moduleSetTag       || expectedDetails
             'a module set tag'     | 'someModuleSetTag' || 'someModuleSetTag'
-            'empty module set tag' | ''                 || 'not-specified'
+            'empty module set tag' | ''                 || ''
     }
 
     def 'Get all locked cm-Handles where lock reasons are model sync failed or upgrade'() {