From: emaclee Date: Tue, 14 Oct 2025 12:24:53 +0000 (+0100) Subject: Bug: Add check from alternateId cache before saving state X-Git-Tag: 3.7.2~4^2~1 X-Git-Url: https://gerrit.onap.org/r/gitweb?a=commitdiff_plain;h=3f9f3f374210b32ad8394535602e445f69e2f077;p=cps.git Bug: Add check from alternateId cache before saving state - ensured the the alternate ids/cm handle ids were removed from cache as part of the remove cm handle process - upon saving a cm handle state , a check to the cache is performed first Issue-ID: CPS-3007 Change-Id: I399c3bc36244cc49e20f435a84ebc68bd9050654 Signed-off-by: emaclee --- diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/CmHandleRegistrationService.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/CmHandleRegistrationService.java index e254f9bb95..8b34b4f8a9 100644 --- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/CmHandleRegistrationService.java +++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/inventory/CmHandleRegistrationService.java @@ -188,10 +188,10 @@ public class CmHandleRegistrationService { } } } + removeAlternateIdsFromCache(yangModelCmHandles, tobeRemovedCmHandleBatch, notDeletedCmHandles); } yangModelCmHandles.removeIf(yangModelCmHandle -> notDeletedCmHandles.contains(yangModelCmHandle.getId())); updateCmHandleStateBatch(yangModelCmHandles, CmHandleState.DELETED); - removeAlternateIdsFromCache(yangModelCmHandles); dmiPluginRegistrationResponse.setRemovedCmHandles(cmHandleRegistrationResponses); } @@ -410,8 +410,16 @@ public class CmHandleRegistrationService { ncmpServiceCmHandle.getDmiProperties()); } - void removeAlternateIdsFromCache(final Collection yangModelCmHandles) { - for (final YangModelCmHandle yangModelCmHandle : yangModelCmHandles) { + void removeAlternateIdsFromCache(final Collection yangModelCmHandles, + final List toBeRemovedCmHandleBatch, + final Set cmHandlesIdsToExclude) { + final Collection removedYangModelCmHandles = + yangModelCmHandles.stream() + .filter(yangModelCmHandle -> + (new HashSet<>(toBeRemovedCmHandleBatch).contains(yangModelCmHandle.getId()) + && !cmHandlesIdsToExclude.contains(yangModelCmHandle.getId()))) + .toList(); + for (final YangModelCmHandle yangModelCmHandle : removedYangModelCmHandles) { final String cmHandleId = yangModelCmHandle.getId(); final String alternateId = yangModelCmHandle.getAlternateId(); if (StringUtils.isNotBlank(alternateId)) { 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 d024072776..f069407cea 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 @@ -26,6 +26,7 @@ import static org.onap.cps.api.parameters.FetchDescendantsOption.INCLUDE_ALL_DES import static org.onap.cps.api.parameters.FetchDescendantsOption.OMIT_DESCENDANTS; import com.google.common.collect.Lists; +import com.hazelcast.map.IMap; import java.time.OffsetDateTime; import java.util.ArrayList; import java.util.Collection; @@ -52,6 +53,7 @@ import org.onap.cps.ncmp.impl.utils.YangDataConverter; import org.onap.cps.utils.ContentType; import org.onap.cps.utils.CpsValidator; import org.onap.cps.utils.JsonObjectMapper; +import org.springframework.beans.factory.annotation.Qualifier; import org.springframework.stereotype.Component; @Slf4j @@ -62,6 +64,7 @@ public class InventoryPersistenceImpl extends NcmpPersistenceImpl implements Inv private final CpsModuleService cpsModuleService; private final CpsValidator cpsValidator; + private final IMap cmHandleIdPerAlternateId; /** * initialize an inventory persistence object. @@ -76,10 +79,13 @@ public class InventoryPersistenceImpl extends NcmpPersistenceImpl implements Inv final JsonObjectMapper jsonObjectMapper, final CpsAnchorService cpsAnchorService, final CpsModuleService cpsModuleService, - final CpsDataService cpsDataService) { + final CpsDataService cpsDataService, + @Qualifier("cmHandleIdPerAlternateId") + final IMap cmHandleIdPerAlternateId) { super(jsonObjectMapper, cpsAnchorService, cpsDataService); this.cpsModuleService = cpsModuleService; this.cpsValidator = cpsValidator; + this.cmHandleIdPerAlternateId = cmHandleIdPerAlternateId; } @Override @@ -101,18 +107,25 @@ public class InventoryPersistenceImpl extends NcmpPersistenceImpl implements Inv @Override public void saveCmHandleStateBatch(final Map cmHandleStatePerCmHandleId) { final Map cmHandlesJsonDataMap = new HashMap<>(); - cmHandleStatePerCmHandleId.forEach((cmHandleId, compositeState) -> cmHandlesJsonDataMap.put( - getXPathForCmHandleById(cmHandleId), - createStateJsonData(jsonObjectMapper.asJsonString(compositeState)))); - cpsDataService.updateDataNodesAndDescendants(NCMP_DATASPACE_NAME, NCMP_DMI_REGISTRY_ANCHOR, - cmHandlesJsonDataMap, OffsetDateTime.now(), ContentType.JSON); + cmHandleStatePerCmHandleId.forEach((cmHandleId, compositeState) -> { + if (exists(cmHandleId)) { + cmHandlesJsonDataMap.put(getXPathForCmHandleById(cmHandleId), + createStateJsonData(jsonObjectMapper.asJsonString(compositeState))); + } else { + log.warn("Failure to save state for cmHandle id '{}' as it does not exist in cache", cmHandleId); + } + }); + if (!cmHandlesJsonDataMap.isEmpty()) { + cpsDataService.updateDataNodesAndDescendants(NCMP_DATASPACE_NAME, NCMP_DMI_REGISTRY_ANCHOR, + cmHandlesJsonDataMap, OffsetDateTime.now(), ContentType.JSON); + } } @Override public YangModelCmHandle getYangModelCmHandle(final String cmHandleId) { cpsValidator.validateNameCharacters(cmHandleId); final DataNode dataNode = - getCmHandleDataNodeByCmHandleId(cmHandleId, INCLUDE_ALL_DESCENDANTS).iterator().next(); + getCmHandleDataNodeByCmHandleId(cmHandleId, INCLUDE_ALL_DESCENDANTS).iterator().next(); return YangDataConverter.toYangModelCmHandle(dataNode); } @@ -137,7 +150,7 @@ public class InventoryPersistenceImpl extends NcmpPersistenceImpl implements Inv final String moduleRevision) { cpsValidator.validateNameCharacters(cmHandleId, moduleName); return cpsModuleService.getModuleDefinitionsByAnchorAndModule(NFP_OPERATIONAL_DATASTORE_DATASPACE_NAME, - cmHandleId, moduleName, moduleRevision); + cmHandleId, moduleName, moduleRevision); } @Override @@ -209,9 +222,9 @@ public class InventoryPersistenceImpl extends NcmpPersistenceImpl implements Inv } private Collection getYangModelCmHandlesWithDescendantsOption(final Collection - cmHandleIds, + cmHandleIds, final FetchDescendantsOption - fetchDescendantsOption) { + fetchDescendantsOption) { final Collection validCmHandleIds = new ArrayList<>(cmHandleIds.size()); cmHandleIds.forEach(cmHandleId -> { try { @@ -219,7 +232,7 @@ public class InventoryPersistenceImpl extends NcmpPersistenceImpl implements Inv validCmHandleIds.add(cmHandleId); } catch (final DataValidationException dataValidationException) { log.error("DataValidationException in CmHandleId {} to be ignored", - dataValidationException.getMessage()); + dataValidationException.getMessage()); } }); return YangDataConverter.toYangModelCmHandles(getCmHandleDataNodes(validCmHandleIds, fetchDescendantsOption)); @@ -232,4 +245,8 @@ public class InventoryPersistenceImpl extends NcmpPersistenceImpl implements Inv return this.getDataNodes(xpaths, fetchDescendantsOption); } + private boolean exists(final String cmHandleId) { + return cmHandleIdPerAlternateId.containsKey(cmHandleId) || cmHandleIdPerAlternateId.containsValue(cmHandleId); + } + } diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/CmHandleRegistrationServiceSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/CmHandleRegistrationServiceSpec.groovy index 802f30c1e7..979fba1f31 100644 --- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/CmHandleRegistrationServiceSpec.groovy +++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/inventory/CmHandleRegistrationServiceSpec.groovy @@ -441,7 +441,7 @@ class CmHandleRegistrationServiceSpec extends Specification { then: 'it is added to the cache with the expected key' 1 * mockCmHandleIdPerAlternateId.putAll([(expectedKey):'ch-1']) when: 'it is removed from the cache' - objectUnderTest.removeAlternateIdsFromCache([yangModelCmHandle]) + objectUnderTest.removeAlternateIdsFromCache([yangModelCmHandle], ['ch-1'],[].toSet()) then: 'the correct key is deleted from the cache' 1 * mockCmHandleIdPerAlternateId.delete(expectedKey) where: 'the following alternate ids are used' 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 8ad03ad097..ddb4bcd331 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 @@ -23,6 +23,8 @@ package org.onap.cps.ncmp.impl.inventory import com.fasterxml.jackson.databind.ObjectMapper +import com.hazelcast.map.IMap + import java.time.OffsetDateTime import java.time.ZoneOffset import java.time.format.DateTimeFormatter @@ -63,7 +65,9 @@ class InventoryPersistenceImplSpec extends Specification { def mockCpsValidator = Mock(CpsValidator) - def objectUnderTest = new InventoryPersistenceImpl(mockCpsValidator, spiedJsonObjectMapper, mockCpsAnchorService, mockCpsModuleService, mockCpsDataService) + def mockCmHandleIdPerAlternateId = Mock(IMap) + + def objectUnderTest = new InventoryPersistenceImpl(mockCpsValidator, spiedJsonObjectMapper, mockCpsAnchorService, mockCpsModuleService, mockCpsDataService, mockCmHandleIdPerAlternateId) def formattedDateAndTime = DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss.SSSZ") .format(OffsetDateTime.of(2022, 12, 31, 20, 30, 40, 1, ZoneOffset.UTC)) @@ -190,6 +194,8 @@ class InventoryPersistenceImplSpec extends Specification { given: 'a map of cm handles composite states' def compositeState1 = new CompositeState(cmHandleState: cmHandleState, lastUpdateTime: formattedDateAndTime) def compositeState2 = new CompositeState(cmHandleState: cmHandleState, lastUpdateTime: formattedDateAndTime) + and: 'alternate id cache contains the given cm handle reference' + mockCmHandleIdPerAlternateId.containsKey(_) >> true when: 'update cm handle state is invoked with the #scenario state' def cmHandleStateMap = ['Some-Cm-Handle1' : compositeState1, 'Some-Cm-Handle2' : compositeState2] objectUnderTest.saveCmHandleStateBatch(cmHandleStateMap) @@ -202,6 +208,25 @@ class InventoryPersistenceImplSpec extends Specification { 'DELETING' | CmHandleState.DELETING || ['/dmi-registry/cm-handles[@id=\'Some-Cm-Handle1\']':'{"state":{"cm-handle-state":"DELETING","last-update-time":"2022-12-31T20:30:40.000+0000"}}', '/dmi-registry/cm-handles[@id=\'Some-Cm-Handle2\']':'{"state":{"cm-handle-state":"DELETING","last-update-time":"2022-12-31T20:30:40.000+0000"}}'] } + def 'Update cm handle states when #scenario in alternate id cache'() { + given: 'a map of cm handles composite states' + def compositeState = new CompositeState(cmHandleState: CmHandleState.ADVISED, lastUpdateTime: formattedDateAndTime) + def cmHandleStateMap = ['some-cm-handle' : compositeState] + and: 'alternate id cache returns #scenario' + mockCmHandleIdPerAlternateId.containsKey(_) >> keyExists + mockCmHandleIdPerAlternateId.containsValue(_) >> valueExists + when: 'we update the state of a cm handle when #scenario' + objectUnderTest.saveCmHandleStateBatch(cmHandleStateMap) + then: 'update node leaves is invoked correct number of times' + expectedCalls * mockCpsDataService.updateDataNodesAndDescendants(*_) + where: 'the following cm handle ids are used' + scenario | keyExists | valueExists || expectedCalls + 'id exists as key' | true | false || 1 + 'id exists as value'| false | true || 1 + 'id does not exist' | false | false || 0 + + } + def 'Getting module definitions by module'() { given: 'cps module service returns module definition for module name' def moduleDefinitions = [new ModuleDefinition('moduleName','revision','content')]