diff options
6 files changed, 104 insertions, 122 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..3600d6da32 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 @@ -22,13 +22,12 @@ package org.onap.cps.ncmp.impl.inventory; import java.util.ArrayList; import java.util.Collection; -import java.util.HashSet; import java.util.Set; +import java.util.stream.Collectors; 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. * @@ -109,13 +55,13 @@ public class AlternateIdChecker { public Collection<String> getIdsOfCmHandlesWithRejectedAlternateId( final Collection<NcmpServiceCmHandle> newNcmpServiceCmHandles, final Operation operation) { - final Set<String> acceptedAlternateIds = new HashSet<>(newNcmpServiceCmHandles.size()); + final Set<String> assignedAlternateIds = getAlternateIdsAlreadyInDb(newNcmpServiceCmHandles); final Collection<String> rejectedCmHandleIds = new ArrayList<>(); for (final NcmpServiceCmHandle ncmpServiceCmHandle : newNcmpServiceCmHandles) { final String cmHandleId = ncmpServiceCmHandle.getCmHandleId(); final String proposedAlternateId = ncmpServiceCmHandle.getAlternateId(); - if (isProposedAlternateIdAcceptable(proposedAlternateId, operation, acceptedAlternateIds, cmHandleId)) { - acceptedAlternateIds.add(proposedAlternateId); + if (isProposedAlternateIdAcceptable(proposedAlternateId, operation, assignedAlternateIds, cmHandleId)) { + assignedAlternateIds.add(proposedAlternateId); } else { rejectedCmHandleIds.add(cmHandleId); } @@ -124,28 +70,46 @@ public class AlternateIdChecker { } private boolean isProposedAlternateIdAcceptable(final String proposedAlternateId, final Operation operation, - final Set<String> acceptedAlternateIds, final String cmHandleId) { - if (StringUtils.isEmpty(proposedAlternateId)) { + final Set<String> assignedAlternateIds, final String cmHandleId) { + if (StringUtils.isBlank(proposedAlternateId)) { return true; } - if (acceptedAlternateIds.contains(proposedAlternateId)) { - log.warn("Alternate id update ignored, cannot update cm handle {}, alternate id is already " - + "assigned to a different cm handle (in this batch)", cmHandleId); + 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 (Operation.CREATE.equals(operation)) { - return canApplyAlternateId(cmHandleId, NO_CURRENT_ALTERNATE_ID, proposedAlternateId); + if (assignedAlternateIds.contains(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) { - try { - inventoryPersistence.getCmHandleDataNodeByAlternateId(alternateId); - } catch (final DataNodeNotFoundException dataNodeNotFoundException) { - return false; + private Set<String> getAlternateIdsAlreadyInDb(final Collection<NcmpServiceCmHandle> ncmpServiceCmHandles) { + final Set<String> alternateIdsToCheck = ncmpServiceCmHandles.stream() + .map(NcmpServiceCmHandle::getAlternateId) + .filter(StringUtils::isNotBlank) + .collect(Collectors.toSet()); + return inventoryPersistence.getCmHandleDataNodesByAlternateIds(alternateIdsToCheck).stream() + .map(dataNode -> (String) dataNode.getLeaves().get("alternate-id")) + .collect(Collectors.toSet()); + } + + 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 true; + return NO_CURRENT_ALTERNATE_ID; } } diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/InventoryPersistence.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/InventoryPersistence.java index beef752ef1..a0d3a3eaee 100644 --- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/InventoryPersistence.java +++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/InventoryPersistence.java @@ -130,6 +130,14 @@ public interface InventoryPersistence extends NcmpPersistence { DataNode getCmHandleDataNodeByAlternateId(String alternateId); /** + * Get data nodes for the given batch of alternate ids. + * + * @param alternateIds alternate IDs + * @return data nodes + */ + Collection<DataNode> getCmHandleDataNodesByAlternateIds(Collection<String> alternateIds); + + /** * Get collection of data nodes of given cm handles. * * @param cmHandleIds collection of cmHandle IDs diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/InventoryPersistenceImpl.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/InventoryPersistenceImpl.java index 083b25db3d..42123f1a26 100644 --- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/InventoryPersistenceImpl.java +++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/InventoryPersistenceImpl.java @@ -32,6 +32,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; import lombok.extern.slf4j.Slf4j; import org.onap.cps.api.CpsAnchorService; import org.onap.cps.api.CpsDataService; @@ -184,6 +185,15 @@ public class InventoryPersistenceImpl extends NcmpPersistenceImpl implements Inv } @Override + public Collection<DataNode> getCmHandleDataNodesByAlternateIds(final Collection<String> alternateIds) { + if (alternateIds.isEmpty()) { + return Collections.emptyList(); + } + final String cpsPathForCmHandlesByAlternateIds = getCpsPathForCmHandlesByAlternateIds(alternateIds); + return cmHandleQueryService.queryNcmpRegistryByCpsPath(cpsPathForCmHandlesByAlternateIds, OMIT_DESCENDANTS); + } + + @Override public Collection<DataNode> getCmHandleDataNodes(final Collection<String> cmHandleIds) { final Collection<String> xpaths = new ArrayList<>(cmHandleIds.size()); cmHandleIds.forEach(cmHandleId -> xpaths.add(getXPathForCmHandleById(cmHandleId))); @@ -212,6 +222,11 @@ public class InventoryPersistenceImpl extends NcmpPersistenceImpl implements Inv return NCMP_DMI_REGISTRY_PARENT + "/cm-handles[@alternate-id='" + alternateId + "']"; } + private static String getCpsPathForCmHandlesByAlternateIds(final Collection<String> alternateIds) { + return alternateIds.stream().collect(Collectors.joining("' or @alternate-id='", + NCMP_DMI_REGISTRY_PARENT + "/cm-handles[@alternate-id='", "']")); + } + private static String createStateJsonData(final String state) { return "{\"state\":" + state + "}"; } 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..b976ff4284 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,74 +24,41 @@ 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' - mockInventoryPersistenceService.getCmHandleDataNodeByAlternateId(_) >> - { args -> altAlreadyInDb.contains(args[0]) ? new DataNode() : throwDataNodeNotFoundException() } + mockInventoryPersistenceService.getCmHandleDataNodesByAlternateIds(_ as Collection<String>) >> + { args -> args[0].stream().filter(altId -> altAlreadyInDb.contains(altId)).map(altId -> new DataNode(leaves: ["alternate-id": altId])).toList() } when: 'the batch of new cm handles is checked' def result = objectUnderTest.getIdsOfCmHandlesWithRejectedAlternateId(batch, AlternateIdChecker.Operation.CREATE) 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(_) >> - { args -> altAlreadyInDb.equals(args[0]) ? new DataNode() : throwDataNodeNotFoundException() } + mockInventoryPersistenceService.getCmHandleDataNodesByAlternateIds(_ as Collection<String>) >> + { args -> args[0].stream().filter(altId -> altAlreadyInDb == altId).map(altId -> new DataNode(leaves: ["alternate-id": altId])).toList() } mockInventoryPersistenceService.getYangModelCmHandle(_) >> new YangModelCmHandle(alternateId: altAlreadyInDb) when: 'the batch of cm handle updates is checked' def result = objectUnderTest.getIdsOfCmHandlesWithRejectedAlternateId(batch, AlternateIdChecker.Operation.UPDATE) @@ -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.getCmHandleDataNodesByAlternateIds(_) >> [] + 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('', '') } } diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/InventoryPersistenceImplSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/InventoryPersistenceImplSpec.groovy index e60bacbdc5..fdf12a880d 100644 --- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/InventoryPersistenceImplSpec.groovy +++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/InventoryPersistenceImplSpec.groovy @@ -297,7 +297,7 @@ class InventoryPersistenceImplSpec extends Specification { 1 * mockCpsDataService.getDataNodes(NCMP_DATASPACE_NAME, NCMP_DMI_REGISTRY_ANCHOR, expectedXPath, INCLUDE_ALL_DESCENDANTS) } - def 'Get cm handle data node'() { + def 'Get cm handle data node by alternate id'() { given: 'expected xPath to get cmHandle data node' def expectedXPath = '/dmi-registry/cm-handles[@alternate-id=\'alternate id\']' and: 'query service is invoked with expected xpath' @@ -316,6 +316,22 @@ class InventoryPersistenceImplSpec extends Specification { assert thrownException.getMessage().contains('DataNode not found') } + def 'Get multiple cm handle data nodes by alternate ids'() { + given: 'expected xPath to get cmHandle data node' + def expectedXPath = "/dmi-registry/cm-handles[@alternate-id='A' or @alternate-id='B']" + when: 'getting the cm handle data node' + objectUnderTest.getCmHandleDataNodesByAlternateIds(['A', 'B']) + then: 'query service is invoked with expected xpath' + 1 * mockCmHandleQueries.queryNcmpRegistryByCpsPath(expectedXPath, OMIT_DESCENDANTS) + } + + def 'Get multiple cm handle data nodes by alternate ids, passing empty collection'() { + when: 'getting the cm handle data node for no alternate ids' + objectUnderTest.getCmHandleDataNodesByAlternateIds([]) + then: 'query service is not invoked' + 0 * mockCmHandleQueries.queryNcmpRegistryByCpsPath(_, _) + } + def 'Get CM handles that has given module names'() { when: 'the method to get cm handles is called' objectUnderTest.getCmHandleIdsWithGivenModules(['sample-module-name']) diff --git a/docs/release-notes.rst b/docs/release-notes.rst index 1652997f60..08d1ac4c48 100644 --- a/docs/release-notes.rst +++ b/docs/release-notes.rst @@ -46,6 +46,7 @@ Features -------- 3.5.2 - `CPS-2326 <https://jira.onap.org/browse/CPS-2326>`_ Uplift liquibase-core dependency to 4.28.0 + - `CPS-2366 <https://jira.onap.org/browse/CPS-2366>`_ Improve registration performance with use of alternateID Version: 3.5.1 ============== |