Refactoring alternate ID checker (CPS-2366 #1)

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

2 files changed, 49 insertions, 103 deletions

diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/AlternateIdChecker.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/AlternateIdChecker.java
index 1096980e1d..f8e13b76d2 100644
--- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/AlternateIdChecker.java
+++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/AlternateIdChecker.java
@@ -28,7 +28,6 @@ import lombok.RequiredArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.lang3.StringUtils;
 import org.onap.cps.ncmp.api.inventory.models.NcmpServiceCmHandle;
-import org.onap.cps.ncmp.impl.inventory.models.YangModelCmHandle;
 import org.onap.cps.spi.exceptions.DataNodeNotFoundException;
 import org.springframework.stereotype.Service;
 
@@ -46,59 +45,6 @@ public class AlternateIdChecker {
     private static final String NO_CURRENT_ALTERNATE_ID = "";
 
     /**
-     * Check if the alternate can be applied to the given cm handle (id).
-     * Conditions:
-     * - proposed alternate is blank (it wil be ignored)
-     * - proposed alternate is same as current (no change)
-     * - proposed alternate is not in use for a different cm handle (in the DB)
-     *
-     * @param cmHandleId cm handle id
-     * @param proposedAlternateId proposed alternate id
-     * @return true if the new alternate id not in use or equal to current alternate id, false otherwise
-     */
-    public boolean canApplyAlternateId(final String cmHandleId, final String proposedAlternateId) {
-        String currentAlternateId = "";
-        try {
-            final YangModelCmHandle yangModelCmHandle = inventoryPersistence.getYangModelCmHandle(cmHandleId);
-            currentAlternateId = yangModelCmHandle.getAlternateId();
-        } catch (final DataNodeNotFoundException dataNodeNotFoundException) {
-            // work with blank current alternate id
-        }
-        return this.canApplyAlternateId(cmHandleId, currentAlternateId, proposedAlternateId);
-    }
-
-    /**
-     * Check if the alternate can be applied to the given cm handle.
-     * Conditions:
-     * - proposed alternate is blank (it wil be ignored)
-     * - proposed alternate is same as current (no change)
-     * - proposed alternate is not in use for a different cm handle (in the DB)
-     *
-     * @param cmHandleId   cm handle id
-     * @param currentAlternateId current alternate id
-     * @param proposedAlternateId new alternate id
-     * @return true if the new alternate id not in use or equal to current alternate id, false otherwise
-     */
-    public boolean canApplyAlternateId(final String cmHandleId,
-                                       final String currentAlternateId,
-                                       final String proposedAlternateId) {
-        if (StringUtils.isBlank(currentAlternateId)) {
-            if (alternateIdAlreadyInDb(proposedAlternateId)) {
-                log.warn("Alternate id update ignored, cannot update cm handle {}, alternate id is already "
-                    + "assigned to a different cm handle", cmHandleId);
-                return false;
-            }
-            return true;
-        }
-        if (currentAlternateId.equals(proposedAlternateId)) {
-            return true;
-        }
-        log.warn("Alternate id update ignored, cannot update cm handle {}, already has an alternate id of {}",
-            cmHandleId, currentAlternateId);
-        return false;
-    }
-
-    /**
      * Check all alternate ids of a batch of cm handles.
      * Includes cross-checks in the batch itself for duplicates. Only the first entry encountered wil be accepted.
      *
@@ -125,7 +71,7 @@ public class AlternateIdChecker {
 
     private boolean isProposedAlternateIdAcceptable(final String proposedAlternateId, final Operation operation,
                                                     final Set<String> acceptedAlternateIds, final String cmHandleId) {
-        if (StringUtils.isEmpty(proposedAlternateId)) {
+        if (StringUtils.isBlank(proposedAlternateId)) {
             return true;
         }
         if (acceptedAlternateIds.contains(proposedAlternateId)) {
@@ -133,10 +79,21 @@ public class AlternateIdChecker {
                 + "assigned to a different cm handle (in this batch)", cmHandleId);
             return false;
         }
-        if (Operation.CREATE.equals(operation)) {
-            return canApplyAlternateId(cmHandleId, NO_CURRENT_ALTERNATE_ID, proposedAlternateId);
+        final String currentAlternateId = getCurrentAlternateId(operation, cmHandleId);
+        if (currentAlternateId.equals(proposedAlternateId)) {
+            return true;
+        }
+        if (StringUtils.isNotBlank(currentAlternateId)) {
+            log.warn("Alternate id update ignored, cannot update cm handle {}, already has an alternate id of {}",
+                    cmHandleId, currentAlternateId);
+            return false;
+        }
+        if (alternateIdAlreadyInDb(proposedAlternateId)) {
+            log.warn("Alternate id update ignored, cannot update cm handle {}, alternate id is already "
+                    + "assigned to a different cm handle", cmHandleId);
+            return false;
         }
-        return canApplyAlternateId(cmHandleId, proposedAlternateId);
+        return true;
     }
 
     private boolean alternateIdAlreadyInDb(final String alternateId) {
@@ -148,4 +105,15 @@ public class AlternateIdChecker {
         return true;
     }
 
+    private String getCurrentAlternateId(final Operation operation, final String cmHandleId) {
+        if (operation == Operation.UPDATE) {
+            try {
+                return inventoryPersistence.getYangModelCmHandle(cmHandleId).getAlternateId();
+            } catch (final DataNodeNotFoundException dataNodeNotFoundException) {
+                // work with blank current alternate id
+            }
+        }
+        return NO_CURRENT_ALTERNATE_ID;
+    }
+
 }
diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/AlternateIdCheckerSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/AlternateIdCheckerSpec.groovy
index b086e58de8..6642532ac2 100644
--- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/AlternateIdCheckerSpec.groovy
+++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/AlternateIdCheckerSpec.groovy
@@ -24,50 +24,16 @@ import org.onap.cps.ncmp.api.inventory.models.NcmpServiceCmHandle
 import org.onap.cps.ncmp.impl.inventory.models.YangModelCmHandle
 import org.onap.cps.spi.exceptions.DataNodeNotFoundException
 import org.onap.cps.spi.model.DataNode
-import org.onap.cps.spi.model.DataNodeBuilder
 import spock.lang.Specification
 
 class AlternateIdCheckerSpec extends Specification {
 
     def mockInventoryPersistenceService = Mock(InventoryPersistence)
-    def someDataNode = new DataNodeBuilder().build()
-    def dataNodeFoundException = new DataNodeNotFoundException('', '')
 
     def objectUnderTest = new AlternateIdChecker(mockInventoryPersistenceService)
 
-    def 'Check new cm handle with new alternate id.'() {
-        given: 'inventory persistence can not find cm handle id'
-            mockInventoryPersistenceService.getYangModelCmHandle('ch 1') >> {throw dataNodeFoundException}
-        and: 'inventory persistence can not find alternate id'
-            mockInventoryPersistenceService.getCmHandleDataNodeByAlternateId('alternate id') >> {throw dataNodeFoundException}
-        expect: 'mapping can be added'
-             assert objectUnderTest.canApplyAlternateId('ch 1', 'alternate id')
-    }
-
-    def 'Check new cm handle with used alternate id.'() {
-        given: 'inventory persistence can not find cm handle id'
-            mockInventoryPersistenceService.getYangModelCmHandle('ch 1') >> {throw dataNodeFoundException}
-        and: 'inventory persistence can find alternate id'
-            mockInventoryPersistenceService.getCmHandleDataNodeByAlternateId('alternate id') >> { someDataNode }
-        expect: 'mapping can not be added'
-            assert objectUnderTest.canApplyAlternateId('ch 1', 'alternate id') == false
-    }
-
-    def 'Check for existing cm handle with #currentAlternateId.'() {
-        given: 'a cm handle with the #currentAlternateId'
-            def yangModelCmHandle = new YangModelCmHandle(alternateId: currentAlternateId)
-        and: 'inventory service finds the cm handle'
-            mockInventoryPersistenceService.getYangModelCmHandle('my cm handle') >> yangModelCmHandle
-        expect: 'add mapping returns expected result'
-            assert canAdd == objectUnderTest.canApplyAlternateId('my cm handle', 'same alternate id')
-        where: 'following alternate ids is used'
-            currentAlternateId   || canAdd
-            'same alternate id'  || true
-            'other alternate id' || false
-    }
-
     def 'Check a batch of created cm handles with #scenario.'() {
-        given: 'a batch of 2 new cm handles alternate id ids #alt1 and #alt2'
+        given: 'a batch of 2 new cm handles with alternate ids #alt1 and #alt2'
             def batch = [new NcmpServiceCmHandle(cmHandleId: 'ch-1', alternateId: alt1),
                          new NcmpServiceCmHandle(cmHandleId: 'ch-2', alternateId: alt2)]
         and: 'the database already contains cm handle(s) with these alternate ids: #altAlreadyInDb'
@@ -78,16 +44,17 @@ class AlternateIdCheckerSpec extends Specification {
         then: 'the result contains ids of the rejected cm handles'
             assert result == expectedRejectedCmHandleIds
         where: 'the following alternate ids are used'
-            scenario                          | alt1   | alt2   | altAlreadyInDb  || expectedRejectedCmHandleIds
-            'blank alternate ids'             | ''     | ''     | ['dont matter'] || []
-            'null alternate ids'              | null   | null   | ['dont matter'] || []
-            'new alternate ids'               | 'fdn1' | 'fdn2' | ['other fdn']   || []
-            'one already used alternate id'   | 'fdn1' | 'fdn2' | ['fdn1']        || ['ch-1']
-            'duplicate alternate id in batch' | 'fdn1' | 'fdn1' | ['dont matter'] || ['ch-2']
+            scenario                          | alt1   | alt2   | altAlreadyInDb   || expectedRejectedCmHandleIds
+            'blank alternate ids'             | ''     | ''     | ['dont matter']  || []
+            'null alternate ids'              | null   | null   | ['dont matter']  || []
+            'new alternate ids'               | 'fdn1' | 'fdn2' | ['other fdn']    || []
+            'one already used alternate id'   | 'fdn1' | 'fdn2' | ['fdn1']         || ['ch-1']
+            'two already used alternate ids'  | 'fdn1' | 'fdn2' | ['fdn1', 'fdn2'] || ['ch-1', 'ch-2']
+            'duplicate alternate id in batch' | 'fdn1' | 'fdn1' | ['dont matter']  || ['ch-2']
     }
 
     def 'Check a batch of updates to existing cm handles with #scenario.'() {
-        given: 'a batch of 1 existing cm handle update alternate id to #proposedAlt'
+        given: 'a batch of 1 existing cm handle to update alternate id to #proposedAlt'
             def batch = [new NcmpServiceCmHandle(cmHandleId: 'ch-1', alternateId: proposedAlt)]
         and: 'the database already contains a cm handle with alternate id: #altAlreadyInDb'
             mockInventoryPersistenceService.getCmHandleDataNodeByAlternateId(_) >>
@@ -104,9 +71,20 @@ class AlternateIdCheckerSpec extends Specification {
             'used different alternate id' | 'otherFdn'  | 'fdn1'         || ['ch-1']
     }
 
-    def throwDataNodeNotFoundException() {
-        // cannot 'return' an exception in conditional stub behavior, so hence a method call that will always throw this exception
-        throw dataNodeFoundException
+    def 'Check update of non-existing cm handle.'() {
+        given: 'a batch of 1 non-existing cm handle to update alternate id'
+            def batch = [new NcmpServiceCmHandle(cmHandleId: 'non-existing', alternateId: 'altId')]
+        and: 'the database does not contain any cm handles'
+            mockInventoryPersistenceService.getCmHandleDataNodeByAlternateId(_) >> { throwDataNodeNotFoundException() }
+            mockInventoryPersistenceService.getYangModelCmHandle(_) >> { throwDataNodeNotFoundException() }
+        when: 'the batch of cm handle updates is checked'
+            def result = objectUnderTest.getIdsOfCmHandlesWithRejectedAlternateId(batch, AlternateIdChecker.Operation.UPDATE)
+        then: 'the result has no rejected cm handles'
+            assert result.empty
+    }
+
+    static throwDataNodeNotFoundException() {
+        throw new DataNodeNotFoundException('', '')
     }
 
 }