diff options
8 files changed, 136 insertions, 43 deletions
diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/NcmpResponseStatus.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/NcmpResponseStatus.java index b9c834c559..e7293b2fd9 100644 --- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/NcmpResponseStatus.java +++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/NcmpResponseStatus.java @@ -36,7 +36,8 @@ public enum NcmpResponseStatus { SUBSCRIPTION_PENDING("106", "subscription pending for all cm handles"), UNKNOWN_ERROR("108", "Unknown error"), CM_HANDLE_ALREADY_EXIST("109", "cm-handle already exists"), - CM_HANDLE_INVALID_ID("110", "cm-handle has an invalid character(s) in id"); + CM_HANDLE_INVALID_ID("110", "cm-handle has an invalid character(s) in id"), + ALTERNATE_ID_ALREADY_ASSOCIATED("111", "cannot re-assign alternate id"); private final String code; private final String message; 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 05b83b98e4..ad1c5cdfb9 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 @@ -24,6 +24,7 @@ package org.onap.cps.ncmp.api.impl; +import static org.onap.cps.ncmp.api.NcmpResponseStatus.ALTERNATE_ID_ALREADY_ASSOCIATED; import static org.onap.cps.ncmp.api.NcmpResponseStatus.CM_HANDLES_NOT_FOUND; import static org.onap.cps.ncmp.api.NcmpResponseStatus.CM_HANDLES_NOT_READY; import static org.onap.cps.ncmp.api.NcmpResponseStatus.CM_HANDLE_ALREADY_EXIST; @@ -500,16 +501,35 @@ public class NetworkCmProxyDataServiceImpl implements NetworkCmProxyDataService private List<CmHandleRegistrationResponse> registerNewCmHandles(final List<YangModelCmHandle> yangModelCmHandles, final Map<String, TrustLevel> initialTrustLevelPerCmHandleId) { - final Set<String> cmHandleIds = initialTrustLevelPerCmHandleId.keySet(); + final List<CmHandleRegistrationResponse> failureResponses = new ArrayList<>(); + final List<YangModelCmHandle> acceptedYangModelCmHandles = new ArrayList<>(yangModelCmHandles.size()); + final Set<String> acceptedCmHandleIds = new HashSet<>(yangModelCmHandles.size()); + for (final YangModelCmHandle yangModelCmHandle : yangModelCmHandles) { + if (cmHandleIdMapper.isDuplicateId(yangModelCmHandle.getId(), yangModelCmHandle.getAlternateId())) { + initialTrustLevelPerCmHandleId.remove(yangModelCmHandle.getId()); + failureResponses.add(CmHandleRegistrationResponse.createFailureResponse( + yangModelCmHandle.getId(), ALTERNATE_ID_ALREADY_ASSOCIATED)); + } else { + acceptedCmHandleIds.add(yangModelCmHandle.getId()); + acceptedYangModelCmHandles.add(yangModelCmHandle); + } + } try { - lcmEventsCmHandleStateHandler.initiateStateAdvised(yangModelCmHandles); + lcmEventsCmHandleStateHandler.initiateStateAdvised(acceptedYangModelCmHandles); trustLevelManager.handleInitialRegistrationOfTrustLevels(initialTrustLevelPerCmHandleId); - return CmHandleRegistrationResponse.createSuccessResponses(cmHandleIds); + final List<CmHandleRegistrationResponse> cmHandleRegistrationResponses = CmHandleRegistrationResponse + .createSuccessResponses(acceptedCmHandleIds); + cmHandleRegistrationResponses.addAll(failureResponses); + return cmHandleRegistrationResponses; } catch (final AlreadyDefinedException alreadyDefinedException) { - return CmHandleRegistrationResponse.createFailureResponses( + final List<CmHandleRegistrationResponse> alreadyDefinedResponses = CmHandleRegistrationResponse + .createFailureResponses( alreadyDefinedException.getAlreadyDefinedObjectNames(), CM_HANDLE_ALREADY_EXIST); + failureResponses.addAll(alreadyDefinedResponses); + return failureResponses; } catch (final Exception exception) { - return CmHandleRegistrationResponse.createFailureResponses(cmHandleIds, exception); + return CmHandleRegistrationResponse + .createFailureResponses(initialTrustLevelPerCmHandleId.keySet(), exception); } } @@ -556,5 +576,4 @@ public class NetworkCmProxyDataServiceImpl implements NetworkCmProxyDataService } } } - } diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServicePropertyHandler.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServicePropertyHandler.java index 13b3fcafb1..0520f456a5 100644 --- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServicePropertyHandler.java +++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServicePropertyHandler.java @@ -22,6 +22,7 @@ package org.onap.cps.ncmp.api.impl; +import static org.onap.cps.ncmp.api.NcmpResponseStatus.ALTERNATE_ID_ALREADY_ASSOCIATED; import static org.onap.cps.ncmp.api.NcmpResponseStatus.CM_HANDLES_NOT_FOUND; import static org.onap.cps.ncmp.api.NcmpResponseStatus.CM_HANDLE_INVALID_ID; import static org.onap.cps.ncmp.api.impl.NetworkCmProxyDataServicePropertyHandler.PropertyType.DMI_PROPERTY; @@ -81,11 +82,17 @@ public class NetworkCmProxyDataServicePropertyHandler { for (final NcmpServiceCmHandle ncmpServiceCmHandle : ncmpServiceCmHandles) { final String cmHandleId = ncmpServiceCmHandle.getCmHandleId(); try { - final DataNode existingCmHandleDataNode = inventoryPersistence.getCmHandleDataNode(cmHandleId) - .iterator().next(); - updateAlternateId(existingCmHandleDataNode, ncmpServiceCmHandle); - processUpdates(existingCmHandleDataNode, ncmpServiceCmHandle); - cmHandleRegistrationResponses.add(CmHandleRegistrationResponse.createSuccessResponse(cmHandleId)); + if (cmHandleIdMapper.isDuplicateId(ncmpServiceCmHandle.getCmHandleId(), + ncmpServiceCmHandle.getAlternateId())) { + cmHandleRegistrationResponses.add(CmHandleRegistrationResponse.createFailureResponse(cmHandleId, + ALTERNATE_ID_ALREADY_ASSOCIATED)); + } else { + final DataNode existingCmHandleDataNode = inventoryPersistence.getCmHandleDataNode(cmHandleId) + .iterator().next(); + updateAlternateId(existingCmHandleDataNode, ncmpServiceCmHandle); + processUpdates(existingCmHandleDataNode, ncmpServiceCmHandle); + cmHandleRegistrationResponses.add(CmHandleRegistrationResponse.createSuccessResponse(cmHandleId)); + } } catch (final DataNodeNotFoundException e) { log.error("Unable to find dataNode for cmHandleId : {} , caused by : {}", cmHandleId, e.getMessage()); cmHandleRegistrationResponses.add( diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/utils/CmHandleIdMapper.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/utils/CmHandleIdMapper.java index a88adbd110..40c620f508 100644 --- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/utils/CmHandleIdMapper.java +++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/utils/CmHandleIdMapper.java @@ -55,15 +55,10 @@ public class CmHandleIdMapper { private boolean addMappingWithValidation(final String cmHandleId, final String alternateId) { - if (alternateIdPerCmHandleId.containsKey(cmHandleId)) { - final String originalAlternateId = alternateIdPerCmHandleId.get(cmHandleId); - if (!originalAlternateId.equals(alternateId)) { - log.warn("Alternate id update ignored, cannot update cm handle {}, already has an alternate id of {}", - cmHandleId, originalAlternateId); - } + if (StringUtils.isBlank(alternateId)) { return false; } - if (StringUtils.isBlank(alternateId)) { + if (isDuplicateId(cmHandleId, alternateId)) { return false; } alternateIdPerCmHandleId.put(cmHandleId, alternateId); @@ -82,10 +77,32 @@ public class CmHandleIdMapper { } } + /** + * Check if alternate id is already used. + * + * @param cmHandleId the cmHandle id. + * @param alternateId the alternate id. + * @return boolean true if the alternate id is already used. + */ + public boolean isDuplicateId(final String cmHandleId, final String alternateId) { + if (StringUtils.isBlank(alternateId)) { + return false; + } + if (cmHandleIdPerAlternateId.get(alternateId) != null) { + log.warn("The given alternate id was added to the cache already: {}", alternateId); + return true; + } + if (alternateIdPerCmHandleId.get(cmHandleId) != null) { + log.warn("The given cmhandle id was added to the cache already: {}", cmHandleId); + return true; + } + return false; + } + private void initializeCache() { if (!cacheIsInitialized) { networkCmProxyCmHandleQueryService.getAllCmHandles().forEach(cmHandle -> - addMappingWithValidation(cmHandle.getCmHandleId(), cmHandle.getAlternateId()) + addMappingWithValidation(cmHandle.getCmHandleId(), cmHandle.getAlternateId()) ); log.info("Alternate ID cache initialized from DB with {} cm handle/alternate id pairs ", alternateIdPerCmHandleId.size()); 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 c7ac8ab8b6..faa2225459 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 @@ -21,31 +21,26 @@ package org.onap.cps.ncmp.api.impl -import static org.onap.cps.ncmp.api.models.CmHandleRegistrationResponse.Status -import static org.onap.cps.ncmp.api.NcmpResponseStatus.CM_HANDLES_NOT_FOUND -import static org.onap.cps.ncmp.api.NcmpResponseStatus.CM_HANDLE_ALREADY_EXIST -import static org.onap.cps.ncmp.api.NcmpResponseStatus.CM_HANDLE_INVALID_ID -import static org.onap.cps.ncmp.api.NcmpResponseStatus.UNKNOWN_ERROR - -import org.onap.cps.ncmp.api.impl.trustlevel.TrustLevelManager -import org.onap.cps.ncmp.api.impl.utils.CmHandleIdMapper -import org.onap.cps.ncmp.api.models.UpgradedCmHandles 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.NcmpResponseStatus import org.onap.cps.ncmp.api.NetworkCmProxyCmHandleQueryService import org.onap.cps.ncmp.api.impl.events.lcm.LcmEventsCmHandleStateHandler import org.onap.cps.ncmp.api.impl.exception.DmiRequestException -import org.onap.cps.ncmp.api.impl.operations.DmiDataOperations -import org.onap.cps.ncmp.api.impl.trustlevel.TrustLevel -import org.onap.cps.ncmp.api.impl.yangmodels.YangModelCmHandle 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.InventoryPersistence +import org.onap.cps.ncmp.api.impl.operations.DmiDataOperations +import org.onap.cps.ncmp.api.impl.trustlevel.TrustLevel +import org.onap.cps.ncmp.api.impl.trustlevel.TrustLevelManager +import org.onap.cps.ncmp.api.impl.utils.CmHandleIdMapper +import org.onap.cps.ncmp.api.impl.yangmodels.YangModelCmHandle import org.onap.cps.ncmp.api.models.CmHandleRegistrationResponse import org.onap.cps.ncmp.api.models.DmiPluginRegistration import org.onap.cps.ncmp.api.models.NcmpServiceCmHandle +import org.onap.cps.ncmp.api.models.UpgradedCmHandles import org.onap.cps.spi.exceptions.AlreadyDefinedException import org.onap.cps.spi.exceptions.DataNodeNotFoundException import org.onap.cps.spi.exceptions.DataValidationException @@ -53,6 +48,12 @@ import org.onap.cps.spi.exceptions.SchemaSetNotFoundException import org.onap.cps.utils.JsonObjectMapper import spock.lang.Specification +import static org.onap.cps.ncmp.api.NcmpResponseStatus.CM_HANDLES_NOT_FOUND +import static org.onap.cps.ncmp.api.NcmpResponseStatus.CM_HANDLE_ALREADY_EXIST +import static org.onap.cps.ncmp.api.NcmpResponseStatus.CM_HANDLE_INVALID_ID +import static org.onap.cps.ncmp.api.NcmpResponseStatus.UNKNOWN_ERROR +import static org.onap.cps.ncmp.api.models.CmHandleRegistrationResponse.Status + class NetworkCmProxyDataServiceImplRegistrationSpec extends Specification { def ncmpServiceCmHandle = new NcmpServiceCmHandle(cmHandleId: 'some-cm-handle-id') @@ -441,6 +442,21 @@ class NetworkCmProxyDataServiceImplRegistrationSpec extends Specification { 1 * mockCmHandleIdMapper.addMapping('cmhandle1', 'my-alternate-id-1') } + def 'Attempting to register a cmhandle with an already cached id.'() { + given: 'a registration of a cmhandle with an alternate id' + def dmiPluginRegistration = new DmiPluginRegistration(dmiPlugin: 'my-server') + dmiPluginRegistration.createdCmHandles = [new NcmpServiceCmHandle(cmHandleId: 'ch-1', alternateId: 'my alternate id')] + and: 'one of the ids are duplicated' + mockCmHandleIdMapper.isDuplicateId('ch-1', 'my alternate id') >> true + when: 'registration is attempted' + def response = objectUnderTest.updateDmiRegistrationAndSyncModule(dmiPluginRegistration) + then: 'a response is received' + assert response != null + and: 'the cmhandle has a failed state with the appropriate NCMP response status' + assert Status.FAILURE == response.createdCmHandles[0].status + assert NcmpResponseStatus.ALTERNATE_ID_ALREADY_ASSOCIATED == response.createdCmHandles[0].ncmpResponseStatus + } + def getObjectUnderTest() { return Spy(new NetworkCmProxyDataServiceImpl(spiedJsonObjectMapper, mockDmiDataOperations, mockNetworkCmProxyDataServicePropertyHandler, mockInventoryPersistence, mockCmHandleQueries, diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServicePropertyHandlerSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServicePropertyHandlerSpec.groovy index f94c34c589..6ae62cfc2b 100644 --- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServicePropertyHandlerSpec.groovy +++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServicePropertyHandlerSpec.groovy @@ -26,6 +26,7 @@ package org.onap.cps.ncmp.api.impl import com.fasterxml.jackson.databind.ObjectMapper import org.onap.cps.ncmp.api.impl.utils.CmHandleIdMapper +import static org.onap.cps.ncmp.api.NcmpResponseStatus.ALTERNATE_ID_ALREADY_ASSOCIATED import static org.onap.cps.ncmp.api.impl.ncmppersistence.NcmpPersistence.NCMP_DATASPACE_NAME import static org.onap.cps.ncmp.api.impl.ncmppersistence.NcmpPersistence.NCMP_DMI_REGISTRY_ANCHOR import static org.onap.cps.ncmp.api.NcmpResponseStatus.CM_HANDLES_NOT_FOUND @@ -219,6 +220,24 @@ class NetworkCmProxyDataServicePropertyHandlerSpec extends Specification { 1 * mockCmHandleIdMapper.removeMapping(cmHandleId) } + def 'Attempt to update a cmhandle with an already cached alternate id.'() { + given: 'cm handles request' + def cmHandleUpdateRequest = [new NcmpServiceCmHandle(cmHandleId: cmHandleId, alternateId: 'my alternate id')] + and: 'the id already added to the alternate id cache' + mockCmHandleIdMapper.isDuplicateId(cmHandleId, 'my alternate id') >> true + when: 'update data node leaves is called using correct parameters' + def response = objectUnderTest.updateCmHandleProperties(cmHandleUpdateRequest) + then: 'one failed registration response' + response.size() == 1 + and: 'it has expected error details' + with(response.get(0)) { + assert it.status == Status.FAILURE + assert it.cmHandle == cmHandleId + assert it.ncmpResponseStatus == ALTERNATE_ID_ALREADY_ASSOCIATED + assert it.errorText.contains('cannot re-assign') + } + } + def convertToProperties(expectedPropertiesAfterUpdateAsMap) { def properties = [].withDefault { [:] } expectedPropertiesAfterUpdateAsMap.forEach(property -> diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/utils/CmHandleIdMapperSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/utils/CmHandleIdMapperSpec.groovy index 55ccdf3be5..1b323a216e 100644 --- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/utils/CmHandleIdMapperSpec.groovy +++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/utils/CmHandleIdMapperSpec.groovy @@ -43,7 +43,7 @@ class CmHandleIdMapperSpec extends Specification { ((Logger) LoggerFactory.getLogger(CmHandleIdMapper.class)).addAppender(logger) logger.start() mockCpsCmHandlerQueryService.getAllCmHandles() >> [] - assert objectUnderTest.addMapping('my cmhandle id', 'my alternate id') + assert objectUnderTest.addMapping('cached cmhandle id', 'cached alternate id') } void cleanup() { @@ -52,9 +52,9 @@ class CmHandleIdMapperSpec extends Specification { def 'Checking entries in the cache.'() { expect: 'the alternate id can be converted to cmhandle id' - assert objectUnderTest.alternateIdToCmHandleId('my alternate id') == 'my cmhandle id' + assert objectUnderTest.alternateIdToCmHandleId('cached alternate id') == 'cached cmhandle id' and: 'the cmhandle id can be converted to alternate id' - assert objectUnderTest.cmHandleIdToAlternateId('my cmhandle id') == 'my alternate id' + assert objectUnderTest.cmHandleIdToAlternateId('cached cmhandle id') == 'cached alternate id' } def 'Attempt adding #scenario alternate id.'() { @@ -71,11 +71,11 @@ class CmHandleIdMapperSpec extends Specification { def 'Remove an entry from the cache.'() { when: 'removing an entry' - objectUnderTest.removeMapping('my cmhandle id') + objectUnderTest.removeMapping('cached cmhandle id') then: 'converting alternate id returns null' - assert objectUnderTest.alternateIdToCmHandleId('my alternate id') == null + assert objectUnderTest.alternateIdToCmHandleId('cached alternate id') == null and: 'converting cmhandle id returns null' - assert objectUnderTest.cmHandleIdToAlternateId('my cmhandle id') == null + assert objectUnderTest.cmHandleIdToAlternateId('cached cmhandle id') == null } def 'Attempt to remove a non-existing entry from the cache.'() { @@ -89,22 +89,22 @@ class CmHandleIdMapperSpec extends Specification { def 'Cannot update existing alternate id.'() { given: 'attempt to update an existing alternate id' - objectUnderTest.addMapping('my cmhandle id', 'other id') + objectUnderTest.addMapping('cached cmhandle id', 'other id') expect: 'still returns the original alternate id' - assert objectUnderTest.cmHandleIdToAlternateId('my cmhandle id') == 'my alternate id' + assert objectUnderTest.cmHandleIdToAlternateId('cached cmhandle id') == 'cached alternate id' and: 'converting other alternate id returns null' assert objectUnderTest.alternateIdToCmHandleId('other id') == null and: 'a warning is logged with the original alternate id' def lastLoggingEvent = logger.list[1] assert lastLoggingEvent.level == Level.WARN - assert lastLoggingEvent.formattedMessage.contains('my alternate id') + assert lastLoggingEvent.formattedMessage.contains('id was added to the cache already') } def 'Update existing alternate id with the same value.'() { expect: 'update an existing alternate id with the same value returns false (no update)' - assert objectUnderTest.addMapping('my cmhandle id', 'my alternate id') == false + assert objectUnderTest.addMapping('cached cmhandle id', 'cached alternate id') == false and: 'conversion still returns the original alternate id' - assert objectUnderTest.cmHandleIdToAlternateId('my cmhandle id') == 'my alternate id' + assert objectUnderTest.cmHandleIdToAlternateId('cached cmhandle id') == 'cached alternate id' } def 'Initializing cache #scenario.'() { @@ -123,4 +123,16 @@ class CmHandleIdMapperSpec extends Specification { 'without alternate id' | [new NcmpServiceCmHandle(cmHandleId: 'ch-1')] || null | null 'with blank alternate id' | [new NcmpServiceCmHandle(cmHandleId: 'ch-1', alternateId: ' ')] || null | null } + + def 'Checking caches for duplicated values when: #scenario.'() { + expect: 'duplicate checks works as intended' + assert objectUnderTest.isDuplicateId(cmHandleId, alternateId) == expectDuplicate + where: 'the following values are given' + scenario | cmHandleId | alternateId || expectDuplicate + 'new cm handle' | 'new ch-1' | 'alt-1' || false + 'cm handle with already assigned other alternate id' | 'cached cmhandle id' | 'alt-1' || true + 'alternate id already assigned to other cm handle' | 'ch-1' | 'cached alternate id' || true + 'no alternate id provided' | 'ch-1' | null || false + 'cm handle with already assigned same alternate id' | 'ch-1' | 'alt-1' || false + } }
\ No newline at end of file diff --git a/docs/cps-ncmp-message-status-codes.rst b/docs/cps-ncmp-message-status-codes.rst index 20a5ae3c11..6b930a8a4b 100644 --- a/docs/cps-ncmp-message-status-codes.rst +++ b/docs/cps-ncmp-message-status-codes.rst @@ -38,6 +38,8 @@ CPS-NCMP Message Status Codes +-----------------+------------------------------------------------------+-----------------------------------+ | 110 | cm-handle has an invalid character(s) in id | Inventory | +-----------------+------------------------------------------------------+-----------------------------------+ + | 111 | cannot re-assign alternate id | Inventory | + +-----------------+------------------------------------------------------+-----------------------------------+ .. note:: |