From ed6c05157f60328b0215bde544f7a4e9894fd15f Mon Sep 17 00:00:00 2001 From: lukegleeson Date: Thu, 18 Aug 2022 17:47:24 +0100 Subject: Performance Improvement: Batch Update DataNodes Implemented methods to perform a batch operation on updating datanodes Refactored replace data node(s) tree methods to update data node(s) and descendants Issue-ID: CPS-1203 Signed-off-by: lukegleeson Change-Id: I365d657422b19c9ce384110c9a23d041eaed06f4 --- .../ncmp/api/inventory/InventoryPersistence.java | 19 ++++- .../api/inventory/InventoryPersistenceSpec.groovy | 19 ++++- .../cps/rest/controller/DataRestController.java | 2 +- .../rest/controller/DataRestControllerSpec.groovy | 6 +- .../spi/impl/CpsDataPersistenceServiceImpl.java | 48 +++++++++---- ...CpsDataPersistenceServiceIntegrationSpec.groovy | 42 +++++------ .../spi/impl/CpsDataPersistenceServiceSpec.groovy | 83 +++++++++++++++------- .../main/java/org/onap/cps/api/CpsDataService.java | 18 ++++- .../org/onap/cps/api/impl/CpsDataServiceImpl.java | 29 +++++++- .../onap/cps/spi/CpsDataPersistenceService.java | 13 +++- .../cps/api/impl/CpsDataServiceImplSpec.groovy | 45 ++++++++++-- 11 files changed, 245 insertions(+), 79 deletions(-) diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/InventoryPersistence.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/InventoryPersistence.java index 14fc6d698a..c059ece0d3 100644 --- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/InventoryPersistence.java +++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/inventory/InventoryPersistence.java @@ -28,6 +28,8 @@ import static org.onap.cps.spi.FetchDescendantsOption.INCLUDE_ALL_DESCENDANTS; import java.time.OffsetDateTime; import java.util.Collection; +import java.util.HashMap; +import java.util.Map; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.onap.cps.api.CpsDataService; @@ -91,11 +93,26 @@ public class InventoryPersistence { public void saveCmHandleState(final String cmHandleId, final CompositeState compositeState) { final String cmHandleJsonData = String.format("{\"state\":%s}", jsonObjectMapper.asJsonString(compositeState)); - cpsDataService.replaceNodeTree(NCMP_DATASPACE_NAME, NCMP_DMI_REGISTRY_ANCHOR, + cpsDataService.updateDataNodeAndDescendants(NCMP_DATASPACE_NAME, NCMP_DMI_REGISTRY_ANCHOR, String.format(CM_HANDLE_XPATH_TEMPLATE, cmHandleId), cmHandleJsonData, OffsetDateTime.now()); } + /** + * Save all cm handles states in batch. + * + * @param cmHandleStates contains cm handle id and updated state + */ + public void saveCmHandleStates(final Map cmHandleStates) { + final Map cmHandlesJsonDataMap = new HashMap<>(); + cmHandleStates.entrySet().stream().forEach(cmHandleEntry -> + cmHandlesJsonDataMap.put(String.format(CM_HANDLE_XPATH_TEMPLATE, cmHandleEntry.getKey()), + String.format("{\"state\":%s}", + jsonObjectMapper.asJsonString(cmHandleEntry.getValue())))); + cpsDataService.updateDataNodesAndDescendants(NCMP_DATASPACE_NAME, NCMP_DMI_REGISTRY_ANCHOR, + cmHandlesJsonDataMap, OffsetDateTime.now()); + } + /** * This method retrieves DMI service name, DMI properties and the state for a given cm handle. * @param cmHandleId the id of the cm handle diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/inventory/InventoryPersistenceSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/inventory/InventoryPersistenceSpec.groovy index f9ca676f3b..7ffec1ab09 100644 --- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/inventory/InventoryPersistenceSpec.groovy +++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/inventory/InventoryPersistenceSpec.groovy @@ -43,7 +43,6 @@ import java.time.format.DateTimeFormatter import static org.onap.cps.ncmp.api.impl.constants.DmiRegistryConstants.NO_TIMESTAMP import static org.onap.cps.spi.FetchDescendantsOption.INCLUDE_ALL_DESCENDANTS -import static org.onap.cps.spi.FetchDescendantsOption.OMIT_DESCENDANTS class InventoryPersistenceSpec extends Specification { @@ -146,7 +145,7 @@ class InventoryPersistenceSpec extends Specification { when: 'update cm handle state is invoked with the #scenario state' objectUnderTest.saveCmHandleState(cmHandleId, compositeState) then: 'update node leaves is invoked with the correct params' - 1 * mockCpsDataService.replaceNodeTree('NCMP-Admin', 'ncmp-dmi-registry', '/dmi-registry/cm-handles[@id=\'Some-Cm-Handle\']', expectedJsonData, _ as OffsetDateTime) + 1 * mockCpsDataService.updateDataNodeAndDescendants('NCMP-Admin', 'ncmp-dmi-registry', '/dmi-registry/cm-handles[@id=\'Some-Cm-Handle\']', expectedJsonData, _ as OffsetDateTime) where: 'the following states are used' scenario | cmHandleState || expectedJsonData 'READY' | CmHandleState.READY || '{"state":{"cm-handle-state":"READY","last-update-time":"2022-12-31T20:30:40.000+0000"}}' @@ -154,6 +153,22 @@ class InventoryPersistenceSpec extends Specification { 'DELETING' | CmHandleState.DELETING || '{"state":{"cm-handle-state":"DELETING","last-update-time":"2022-12-31T20:30:40.000+0000"}}' } + def 'Update Cm Handles with #scenario States'() { + given: 'a map of cm handles composite states' + def compositeState1 = new CompositeState(cmHandleState: cmHandleState, lastUpdateTime: formattedDateAndTime) + def compositeState2 = new CompositeState(cmHandleState: cmHandleState, lastUpdateTime: formattedDateAndTime) + when: 'update cm handle state is invoked with the #scenario state' + def cmHandleStateMap = ['Some-Cm-Handle1' : compositeState1, 'Some-Cm-Handle2' : compositeState2] + objectUnderTest.saveCmHandleStates(cmHandleStateMap) + then: 'update node leaves is invoked with the correct params' + 1 * mockCpsDataService.updateDataNodesAndDescendants('NCMP-Admin', 'ncmp-dmi-registry', cmHandlesJsonDataMap, _ as OffsetDateTime) + where: 'the following states are used' + scenario | cmHandleState || cmHandlesJsonDataMap + 'READY' | CmHandleState.READY || ['/dmi-registry/cm-handles[@id=\'Some-Cm-Handle1\']':'{"state":{"cm-handle-state":"READY","last-update-time":"2022-12-31T20:30:40.000+0000"}}', '/dmi-registry/cm-handles[@id=\'Some-Cm-Handle2\']':'{"state":{"cm-handle-state":"READY","last-update-time":"2022-12-31T20:30:40.000+0000"}}'] + 'LOCKED' | CmHandleState.LOCKED || ['/dmi-registry/cm-handles[@id=\'Some-Cm-Handle1\']':'{"state":{"cm-handle-state":"LOCKED","last-update-time":"2022-12-31T20:30:40.000+0000"}}', '/dmi-registry/cm-handles[@id=\'Some-Cm-Handle2\']':'{"state":{"cm-handle-state":"LOCKED","last-update-time":"2022-12-31T20:30:40.000+0000"}}'] + '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 'Get module definitions'() { given: 'cps module service returns a collection of module definitions' def moduleDefinitions = [new ModuleDefinition('moduleName','revision','content')] diff --git a/cps-rest/src/main/java/org/onap/cps/rest/controller/DataRestController.java b/cps-rest/src/main/java/org/onap/cps/rest/controller/DataRestController.java index b78d383394..8dea2c02c2 100755 --- a/cps-rest/src/main/java/org/onap/cps/rest/controller/DataRestController.java +++ b/cps-rest/src/main/java/org/onap/cps/rest/controller/DataRestController.java @@ -101,7 +101,7 @@ public class DataRestController implements CpsDataApi { public ResponseEntity replaceNode(final String dataspaceName, final String anchorName, final Object jsonData, final String parentNodeXpath, final String observedTimestamp) { cpsDataService - .replaceNodeTree(dataspaceName, anchorName, parentNodeXpath, + .updateDataNodeAndDescendants(dataspaceName, anchorName, parentNodeXpath, jsonObjectMapper.asJsonString(jsonData), toOffsetDateTime(observedTimestamp)); return new ResponseEntity<>(HttpStatus.OK); } diff --git a/cps-rest/src/test/groovy/org/onap/cps/rest/controller/DataRestControllerSpec.groovy b/cps-rest/src/test/groovy/org/onap/cps/rest/controller/DataRestControllerSpec.groovy index 6f415bd451..75a3fcf008 100755 --- a/cps-rest/src/test/groovy/org/onap/cps/rest/controller/DataRestControllerSpec.groovy +++ b/cps-rest/src/test/groovy/org/onap/cps/rest/controller/DataRestControllerSpec.groovy @@ -273,7 +273,7 @@ class DataRestControllerSpec extends Specification { .param('xpath', inputXpath)) .andReturn().response then: 'the service method is invoked with expected parameters' - 1 * mockCpsDataService.replaceNodeTree(dataspaceName, anchorName, xpathServiceParameter, expectedJsonData, noTimestamp) + 1 * mockCpsDataService.updateDataNodeAndDescendants(dataspaceName, anchorName, xpathServiceParameter, expectedJsonData, noTimestamp) and: 'response status indicates success' response.status == HttpStatus.OK.value() where: @@ -283,7 +283,7 @@ class DataRestControllerSpec extends Specification { 'some xpath by parent' | '/some/xpath' || '/some/xpath' } - def 'Replace data node tree with observedTimestamp.'() { + def 'Update data node and descendants with observedTimestamp.'() { given: 'endpoint to replace node' def endpoint = "$dataNodeBaseEndpoint/anchors/$anchorName/nodes" when: 'put request is performed' @@ -296,7 +296,7 @@ class DataRestControllerSpec extends Specification { .param('observed-timestamp', observedTimestamp)) .andReturn().response then: 'the service method is invoked with expected parameters' - expectedApiCount * mockCpsDataService.replaceNodeTree(dataspaceName, anchorName, '/', expectedJsonData, + expectedApiCount * mockCpsDataService.updateDataNodeAndDescendants(dataspaceName, anchorName, '/', expectedJsonData, { it == DateTimeUtility.toOffsetDateTime(observedTimestamp) }) and: 'response status indicates success' response.status == expectedHttpStatus.value() diff --git a/cps-ri/src/main/java/org/onap/cps/spi/impl/CpsDataPersistenceServiceImpl.java b/cps-ri/src/main/java/org/onap/cps/spi/impl/CpsDataPersistenceServiceImpl.java index 9443355981..c4a2c2fe98 100644 --- a/cps-ri/src/main/java/org/onap/cps/spi/impl/CpsDataPersistenceServiceImpl.java +++ b/cps-ri/src/main/java/org/onap/cps/spi/impl/CpsDataPersistenceServiceImpl.java @@ -26,6 +26,7 @@ import static org.onap.cps.spi.FetchDescendantsOption.INCLUDE_ALL_DESCENDANTS; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet.Builder; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.HashMap; @@ -104,14 +105,16 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService final Collection newChildren) { final FragmentEntity parentFragmentEntity = getFragmentByXpath(dataspaceName, anchorName, parentNodeXpath); try { - for (final DataNode newChildAsDataNode : newChildren) { + final List fragmentEntities = new ArrayList<>(); + newChildren.forEach(newChildAsDataNode -> { final FragmentEntity newChildAsFragmentEntity = convertToFragmentWithAllDescendants( - parentFragmentEntity.getDataspace(), - parentFragmentEntity.getAnchor(), - newChildAsDataNode); + parentFragmentEntity.getDataspace(), + parentFragmentEntity.getAnchor(), + newChildAsDataNode); newChildAsFragmentEntity.setParentId(parentFragmentEntity.getId()); - fragmentRepository.save(newChildAsFragmentEntity); - } + fragmentEntities.add(newChildAsFragmentEntity); + }); + fragmentRepository.saveAll(fragmentEntities); } catch (final DataIntegrityViolationException exception) { final List conflictXpaths = newChildren.stream() .map(DataNode::getXpath) @@ -288,9 +291,10 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService } @Override - public void replaceDataNodeTree(final String dataspaceName, final String anchorName, final DataNode dataNode) { + public void updateDataNodeAndDescendants(final String dataspaceName, final String anchorName, + final DataNode dataNode) { final FragmentEntity fragmentEntity = getFragmentByXpath(dataspaceName, anchorName, dataNode.getXpath()); - replaceDataNodeTree(fragmentEntity, dataNode); + updateFragmentEntityAndDescendantsWithDataNode(fragmentEntity, dataNode); try { fragmentRepository.save(fragmentEntity); } catch (final StaleStateException staleStateException) { @@ -301,8 +305,27 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService } } - private void replaceDataNodeTree(final FragmentEntity existingFragmentEntity, - final DataNode newDataNode) { + @Override + public void updateDataNodesAndDescendants(final String dataspaceName, + final String anchorName, + final List dataNodes) { + final Map dataNodeFragmentEntityMap = dataNodes.stream() + .collect(Collectors.toMap( + dataNode -> dataNode, dataNode -> getFragmentByXpath(dataspaceName, anchorName, dataNode.getXpath()))); + dataNodeFragmentEntityMap.forEach( + (dataNode, fragmentEntity) -> updateFragmentEntityAndDescendantsWithDataNode(fragmentEntity, dataNode)); + try { + fragmentRepository.saveAll(dataNodeFragmentEntityMap.values()); + } catch (final StaleStateException staleStateException) { + throw new ConcurrencyException("Concurrent Transactions", + String.format("A data node in dataspace :'%s' with Anchor : '%s' is updated by another transaction.", + dataspaceName, anchorName), + staleStateException); + } + } + + private void updateFragmentEntityAndDescendantsWithDataNode(final FragmentEntity existingFragmentEntity, + final DataNode newDataNode) { existingFragmentEntity.setAttributes(jsonObjectMapper.asJsonString(newDataNode.getLeaves())); @@ -318,10 +341,11 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService existingFragmentEntity.getDataspace(), existingFragmentEntity.getAnchor(), newDataNodeChild); } else { childFragment = existingChildrenByXpath.get(newDataNodeChild.getXpath()); - replaceDataNodeTree(childFragment, newDataNodeChild); + updateFragmentEntityAndDescendantsWithDataNode(childFragment, newDataNodeChild); } updatedChildFragments.add(childFragment); } + existingFragmentEntity.getChildFragments().clear(); existingFragmentEntity.getChildFragments().addAll(updatedChildFragments); } @@ -457,7 +481,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService copyAttributesFromNewListElement(existingListElementEntity, newListElement); existingListElementEntity.getChildFragments().clear(); } else { - replaceDataNodeTree(existingListElementEntity, newListElement); + updateFragmentEntityAndDescendantsWithDataNode(existingListElementEntity, newListElement); } return existingListElementEntity; } diff --git a/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceIntegrationSpec.groovy b/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceIntegrationSpec.groovy index 6f780fc508..fee489d18b 100755 --- a/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceIntegrationSpec.groovy +++ b/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceIntegrationSpec.groovy @@ -55,8 +55,8 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase { static final long ID_DATA_NODE_WITH_DESCENDANTS = 4001 static final String XPATH_DATA_NODE_WITH_DESCENDANTS = '/parent-1' static final String XPATH_DATA_NODE_WITH_LEAVES = '/parent-100' - static final long UPDATE_DATA_NODE_FRAGMENT_ID = 4202L - static final long UPDATE_DATA_NODE_SUB_FRAGMENT_ID = 4203L + static final long DATA_NODE_202_FRAGMENT_ID = 4202L + static final long CHILD_OF_DATA_NODE_202_FRAGMENT_ID = 4203L static final long LIST_DATA_NODE_PARENT201_FRAGMENT_ID = 4206L static final long LIST_DATA_NODE_PARENT203_FRAGMENT_ID = 4214L static final long LIST_DATA_NODE_PARENT202_FRAGMENT_ID = 4211L @@ -258,14 +258,14 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase { objectUnderTest.updateDataLeaves(DATASPACE_NAME, ANCHOR_FOR_DATA_NODES_WITH_LEAVES, "/parent-200/child-201", ['leaf-value': 'new']) then: 'leaves are updated for selected data node' - def updatedFragment = fragmentRepository.getById(UPDATE_DATA_NODE_FRAGMENT_ID) + def updatedFragment = fragmentRepository.getById(DATA_NODE_202_FRAGMENT_ID) def updatedLeaves = getLeavesMap(updatedFragment) assert updatedLeaves.size() == 1 assert updatedLeaves.'leaf-value' == 'new' and: 'existing child entry remains as is' def childFragment = updatedFragment.childFragments.iterator().next() def childLeaves = getLeavesMap(childFragment) - assert childFragment.id == UPDATE_DATA_NODE_SUB_FRAGMENT_ID + assert childFragment.id == CHILD_OF_DATA_NODE_202_FRAGMENT_ID assert childLeaves.'leaf-value' == 'original' } @@ -283,32 +283,32 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase { } @Sql([CLEAR_DATA, SET_DATA]) - def 'Replace data node tree with descendants removal.'() { + def 'Update data node and descendants by removing descendants.'() { given: 'data node object with leaves updated, no children' def submittedDataNode = buildDataNode("/parent-200/child-201", ['leaf-value': 'new'], []) - when: 'replace data node tree is performed' - objectUnderTest.replaceDataNodeTree(DATASPACE_NAME, ANCHOR_FOR_DATA_NODES_WITH_LEAVES, submittedDataNode) + when: 'update data nodes and descendants is performed' + objectUnderTest.updateDataNodeAndDescendants(DATASPACE_NAME, ANCHOR_FOR_DATA_NODES_WITH_LEAVES, submittedDataNode) then: 'leaves have been updated for selected data node' - def updatedFragment = fragmentRepository.getById(UPDATE_DATA_NODE_FRAGMENT_ID) + def updatedFragment = fragmentRepository.getById(DATA_NODE_202_FRAGMENT_ID) def updatedLeaves = getLeavesMap(updatedFragment) assert updatedLeaves.size() == 1 assert updatedLeaves.'leaf-value' == 'new' and: 'updated entry has no children' updatedFragment.childFragments.isEmpty() and: 'previously attached child entry is removed from database' - fragmentRepository.findById(UPDATE_DATA_NODE_SUB_FRAGMENT_ID).isEmpty() + fragmentRepository.findById(CHILD_OF_DATA_NODE_202_FRAGMENT_ID).isEmpty() } @Sql([CLEAR_DATA, SET_DATA]) - def 'Replace data node tree with descendants.'() { + def 'Update data node and descendants with new descendants'() { given: 'data node object with leaves updated, having child with old content' def submittedDataNode = buildDataNode("/parent-200/child-201", ['leaf-value': 'new'], [ buildDataNode("/parent-200/child-201/grand-child", ['leaf-value': 'original'], []) ]) when: 'update is performed including descendants' - objectUnderTest.replaceDataNodeTree(DATASPACE_NAME, ANCHOR_FOR_DATA_NODES_WITH_LEAVES, submittedDataNode) + objectUnderTest.updateDataNodeAndDescendants(DATASPACE_NAME, ANCHOR_FOR_DATA_NODES_WITH_LEAVES, submittedDataNode) then: 'leaves have been updated for selected data node' - def updatedFragment = fragmentRepository.getById(UPDATE_DATA_NODE_FRAGMENT_ID) + def updatedFragment = fragmentRepository.getById(DATA_NODE_202_FRAGMENT_ID) def updatedLeaves = getLeavesMap(updatedFragment) assert updatedLeaves.size() == 1 assert updatedLeaves.'leaf-value' == 'new' @@ -320,15 +320,15 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase { } @Sql([CLEAR_DATA, SET_DATA]) - def 'Replace data node tree with same descendants but changed leaf value.'() { + def 'Update data node and descendants with same descendants but changed leaf value.'() { given: 'data node object with leaves updated, having child with old content' def submittedDataNode = buildDataNode("/parent-200/child-201", ['leaf-value': 'new'], [ buildDataNode("/parent-200/child-201/grand-child", ['leaf-value': 'new'], []) ]) when: 'update is performed including descendants' - objectUnderTest.replaceDataNodeTree(DATASPACE_NAME, ANCHOR_FOR_DATA_NODES_WITH_LEAVES, submittedDataNode) + objectUnderTest.updateDataNodeAndDescendants(DATASPACE_NAME, ANCHOR_FOR_DATA_NODES_WITH_LEAVES, submittedDataNode) then: 'leaves have been updated for selected data node' - def updatedFragment = fragmentRepository.getById(UPDATE_DATA_NODE_FRAGMENT_ID) + def updatedFragment = fragmentRepository.getById(DATA_NODE_202_FRAGMENT_ID) def updatedLeaves = getLeavesMap(updatedFragment) assert updatedLeaves.size() == 1 assert updatedLeaves.'leaf-value' == 'new' @@ -340,20 +340,20 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase { } @Sql([CLEAR_DATA, SET_DATA]) - def 'Replace data node tree with different descendants xpath'() { + def 'Update data node and descendants with different descendants xpath'() { given: 'data node object with leaves updated, having child with old content' def submittedDataNode = buildDataNode("/parent-200/child-201", ['leaf-value': 'new'], [ buildDataNode("/parent-200/child-201/grand-child-new", ['leaf-value': 'new'], []) ]) when: 'update is performed including descendants' - objectUnderTest.replaceDataNodeTree(DATASPACE_NAME, ANCHOR_FOR_DATA_NODES_WITH_LEAVES, submittedDataNode) + objectUnderTest.updateDataNodeAndDescendants(DATASPACE_NAME, ANCHOR_FOR_DATA_NODES_WITH_LEAVES, submittedDataNode) then: 'leaves have been updated for selected data node' - def updatedFragment = fragmentRepository.getById(UPDATE_DATA_NODE_FRAGMENT_ID) + def updatedFragment = fragmentRepository.getById(DATA_NODE_202_FRAGMENT_ID) def updatedLeaves = getLeavesMap(updatedFragment) assert updatedLeaves.size() == 1 assert updatedLeaves.'leaf-value' == 'new' and: 'previously attached child entry is removed from database' - fragmentRepository.findById(UPDATE_DATA_NODE_SUB_FRAGMENT_ID).isEmpty() + fragmentRepository.findById(CHILD_OF_DATA_NODE_202_FRAGMENT_ID).isEmpty() and: 'new child entry is persisted' def childFragment = updatedFragment.childFragments.iterator().next() childFragment.xpath == '/parent-200/child-201/grand-child-new' @@ -362,11 +362,11 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase { } @Sql([CLEAR_DATA, SET_DATA]) - def 'Replace data node tree error scenario: #scenario.'() { + def 'Update data node and descendants error scenario: #scenario.'() { given: 'data node object' def submittedDataNode = buildDataNode(xpath, ['leaf-name': 'leaf-value'], []) when: 'attempt to update data node for #scenario' - objectUnderTest.replaceDataNodeTree(dataspaceName, anchorName, submittedDataNode) + objectUnderTest.updateDataNodeAndDescendants(dataspaceName, anchorName, submittedDataNode) then: 'a #expectedException is thrown' thrown(expectedException) where: 'the following data is used' diff --git a/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceSpec.groovy b/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceSpec.groovy index bde2f3de9f..1bbf358e54 100644 --- a/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceSpec.groovy +++ b/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceSpec.groovy @@ -28,6 +28,7 @@ import org.onap.cps.spi.entities.SchemaSetEntity import org.onap.cps.spi.entities.YangResourceEntity import org.onap.cps.spi.exceptions.ConcurrencyException import org.onap.cps.spi.exceptions.DataValidationException +import org.onap.cps.spi.model.DataNode import org.onap.cps.spi.model.DataNodeBuilder import org.onap.cps.spi.repository.AnchorRepository import org.onap.cps.spi.repository.DataspaceRepository @@ -67,40 +68,40 @@ class CpsDataPersistenceServiceSpec extends Specification { )] as Set - def 'Handling of StaleStateException (caused by concurrent updates) during data node tree update.'() { - - def parentXpath = '/parent-01' - def myDataspaceName = 'my-dataspace' - def myAnchorName = 'my-anchor' - - given: 'data node object' - def submittedDataNode = new DataNodeBuilder() - .withXpath(parentXpath) - .withLeaves(['leaf-name': 'leaf-value']) - .build() - and: 'fragment to be updated' - mockFragmentRepository.getByDataspaceAndAnchorAndXpath(_, _, _) >> { + def 'Handling of StaleStateException (caused by concurrent updates) during update data node and descendants.'() { + given: 'the fragment repository returns a fragment entity' + mockFragmentRepository.getByDataspaceAndAnchorAndXpath(*_) >> { def fragmentEntity = new FragmentEntity() - fragmentEntity.setXpath(parentXpath) - fragmentEntity.setChildFragments(Collections.emptySet()) + fragmentEntity.setChildFragments([new FragmentEntity()] as Set) return fragmentEntity } - and: 'data node is concurrently updated by another transaction' + and: 'a data node is concurrently updated by another transaction' mockFragmentRepository.save(_) >> { throw new StaleStateException("concurrent updates") } + when: 'attempt to update data node with submitted data nodes' + objectUnderTest.updateDataNodeAndDescendants('some-dataspace', 'some-anchor', new DataNodeBuilder().withXpath('/some/xpath').build()) + then: 'concurrency exception is thrown' + def concurrencyException = thrown(ConcurrencyException) + assert concurrencyException.getDetails().contains('some-dataspace') + assert concurrencyException.getDetails().contains('some-anchor') + assert concurrencyException.getDetails().contains('/some/xpath') + } - when: 'attempt to update data node' - objectUnderTest.replaceDataNodeTree(myDataspaceName, myAnchorName, submittedDataNode) - + def 'Handling of StaleStateException (caused by concurrent updates) during update data nodes and descendants.'() { + given: 'the fragment repository returns a list of fragment entities' + mockFragmentRepository.getByDataspaceAndAnchorAndXpath(*_) >> new FragmentEntity() + and: 'a data node is concurrently updated by another transaction' + mockFragmentRepository.saveAll(*_) >> { throw new StaleStateException("concurrent updates") } + when: 'attempt to update data node with submitted data nodes' + objectUnderTest.updateDataNodesAndDescendants('some-dataspace', 'some-anchor', []) then: 'concurrency exception is thrown' def concurrencyException = thrown(ConcurrencyException) - assert concurrencyException.getDetails().contains(myDataspaceName) - assert concurrencyException.getDetails().contains(myAnchorName) - assert concurrencyException.getDetails().contains(parentXpath) + assert concurrencyException.getDetails().contains('some-dataspace') + assert concurrencyException.getDetails().contains('some-anchor') } def 'Retrieving a data node with a property JSON value of #scenario'() { given: 'a fragment with a property JSON value of #scenario' - mockFragmentRepository.getByDataspaceAndAnchorAndXpath(_, _, _) >> { + mockFragmentRepository.getByDataspaceAndAnchorAndXpath(*_) >> { new FragmentEntity(childFragments: Collections.emptySet(), attributes: "{\"some attribute\": ${dataString}}", anchor: new AnchorEntity(schemaSet: new SchemaSetEntity(yangResources: yangResourceSet ))) @@ -128,11 +129,11 @@ class CpsDataPersistenceServiceSpec extends Specification { def 'Retrieving a data node with invalid JSON'() { given: 'a fragment with invalid JSON' - mockFragmentRepository.getByDataspaceAndAnchorAndXpath(_, _, _) >> { + mockFragmentRepository.getByDataspaceAndAnchorAndXpath(*_) >> { new FragmentEntity(childFragments: Collections.emptySet(), attributes: '{invalid json') } when: 'getting the data node represented by this fragment' - def dataNode = objectUnderTest.getDataNode('my-dataspace', 'my-anchor', + objectUnderTest.getDataNode('my-dataspace', 'my-anchor', '/parent-01', FetchDescendantsOption.INCLUDE_ALL_DESCENDANTS) then: 'a data validation exception is thrown' thrown(DataValidationException) @@ -160,4 +161,36 @@ class CpsDataPersistenceServiceSpec extends Specification { then: 'the session manager method to lock anchor is invoked with same parameters' 1 * mockSessionManager.lockAnchor('mySessionId', 'myDataspaceName', 'myAnchorName', 123L) } + + def 'update data node and descendants: #scenario'(){ + given: 'mocked responses' + mockFragmentRepository.getByDataspaceAndAnchorAndXpath(_, _, '/test/xpath') >> new FragmentEntity(xpath: '/test/xpath', childFragments: []) + when: 'replace data node tree' + objectUnderTest.updateDataNodesAndDescendants('dataspaceName', 'anchorName', dataNodes) + then: 'call fragment repository save all method' + 1 * mockFragmentRepository.saveAll({fragmentEntities -> assert fragmentEntities as List == expectedFragmentEntities}) + where: 'the following Data Type is passed' + scenario | dataNodes || expectedFragmentEntities + 'empty data node list' | [] || [] + 'one data node in list' | [new DataNode(xpath: '/test/xpath', leaves: ['id': 'testId'], childDataNodes: [])] || [new FragmentEntity(xpath: '/test/xpath', attributes: '{"id":"testId"}', childFragments: [])] + } + + def 'update data nodes and descendants'() { + given: 'the fragment repository returns a fragment entity related to the xpath input' + mockFragmentRepository.getByDataspaceAndAnchorAndXpath(_, _, '/test/xpath1') >> new FragmentEntity(xpath: '/test/xpath1', childFragments: []) + mockFragmentRepository.getByDataspaceAndAnchorAndXpath(_, _, '/test/xpath2') >> new FragmentEntity(xpath: '/test/xpath2', childFragments: []) + and: 'some data nodes with descendants' + def dataNode1 = new DataNode(xpath: '/test/xpath1', leaves: ['id': 'testId1'], childDataNodes: [new DataNode(xpath: '/test/xpath1/child', leaves: ['id': 'childTestId1'])]) + def dataNode2 = new DataNode(xpath: '/test/xpath2', leaves: ['id': 'testId2'], childDataNodes: [new DataNode(xpath: '/test/xpath2/child', leaves: ['id': 'childTestId2'])]) + when: 'the fragment entities are update by the data nodes' + objectUnderTest.updateDataNodesAndDescendants('dataspaceName', 'anchorName', [dataNode1, dataNode2]) + then: 'call fragment repository save all method is called with the updated fragments' + 1 * mockFragmentRepository.saveAll({fragmentEntities -> { + fragmentEntities.containsAll([ + new FragmentEntity(xpath: '/test/xpath1', attributes: '{"id":"testId1"}', childFragments: [new FragmentEntity(xpath: '/test/xpath1/child', attributes: '{"id":"childTestId1"}', childFragments: [])]), + new FragmentEntity(xpath: '/test/xpath2', attributes: '{"id":"testId2"}', childFragments: [new FragmentEntity(xpath: '/test/xpath2/child', attributes: '{"id":"childTestId2"}', childFragments: [])]) + ]) + assert fragmentEntities.size() == 2 + }}) + } } \ No newline at end of file diff --git a/cps-service/src/main/java/org/onap/cps/api/CpsDataService.java b/cps-service/src/main/java/org/onap/cps/api/CpsDataService.java index cde25a9f98..decf67d24e 100644 --- a/cps-service/src/main/java/org/onap/cps/api/CpsDataService.java +++ b/cps-service/src/main/java/org/onap/cps/api/CpsDataService.java @@ -24,6 +24,7 @@ package org.onap.cps.api; import java.time.OffsetDateTime; import java.util.Collection; +import java.util.Map; import org.onap.cps.spi.FetchDescendantsOption; import org.onap.cps.spi.model.DataNode; @@ -93,7 +94,7 @@ public interface CpsDataService { OffsetDateTime observedTimestamp); /** - * Replaces existing data node content including descendants. + * Replaces an existing data node's content including descendants. * * @param dataspaceName dataspace name * @param anchorName anchor name @@ -101,8 +102,19 @@ public interface CpsDataService { * @param jsonData json data * @param observedTimestamp observedTimestamp */ - void replaceNodeTree(String dataspaceName, String anchorName, String parentNodeXpath, String jsonData, - OffsetDateTime observedTimestamp); + void updateDataNodeAndDescendants(String dataspaceName, String anchorName, String parentNodeXpath, String jsonData, + OffsetDateTime observedTimestamp); + + /** + * Replaces multiple existing data nodes' content including descendants in a batch operation. + * + * @param dataspaceName dataspace name + * @param anchorName anchor name + * @param nodesJsonData map of xpath and node JSON data + * @param observedTimestamp observedTimestamp + */ + void updateDataNodesAndDescendants(String dataspaceName, String anchorName, Map nodesJsonData, + OffsetDateTime observedTimestamp); /** * Replaces list content by removing all existing elements and inserting the given new elements as json diff --git a/cps-service/src/main/java/org/onap/cps/api/impl/CpsDataServiceImpl.java b/cps-service/src/main/java/org/onap/cps/api/impl/CpsDataServiceImpl.java index 7bdc2c166a..092fd31fcf 100755 --- a/cps-service/src/main/java/org/onap/cps/api/impl/CpsDataServiceImpl.java +++ b/cps-service/src/main/java/org/onap/cps/api/impl/CpsDataServiceImpl.java @@ -28,6 +28,9 @@ import static org.onap.cps.notification.Operation.UPDATE; import java.time.OffsetDateTime; import java.util.Collection; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; import lombok.AllArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.onap.cps.api.CpsAdminService; @@ -142,14 +145,27 @@ public class CpsDataServiceImpl implements CpsDataService { } @Override - public void replaceNodeTree(final String dataspaceName, final String anchorName, final String parentNodeXpath, - final String jsonData, final OffsetDateTime observedTimestamp) { + public void updateDataNodeAndDescendants(final String dataspaceName, final String anchorName, + final String parentNodeXpath, final String jsonData, + final OffsetDateTime observedTimestamp) { CpsValidator.validateNameCharacters(dataspaceName, anchorName); final DataNode dataNode = buildDataNode(dataspaceName, anchorName, parentNodeXpath, jsonData); - cpsDataPersistenceService.replaceDataNodeTree(dataspaceName, anchorName, dataNode); + cpsDataPersistenceService.updateDataNodeAndDescendants(dataspaceName, anchorName, dataNode); processDataUpdatedEventAsync(dataspaceName, anchorName, parentNodeXpath, UPDATE, observedTimestamp); } + @Override + public void updateDataNodesAndDescendants(final String dataspaceName, final String anchorName, + final Map nodesJsonData, + final OffsetDateTime observedTimestamp) { + CpsValidator.validateNameCharacters(dataspaceName, anchorName); + final List dataNodes = buildDataNodes(dataspaceName, anchorName, nodesJsonData); + cpsDataPersistenceService.updateDataNodesAndDescendants(dataspaceName, anchorName, dataNodes); + nodesJsonData.keySet().forEach(nodeXpath -> + processDataUpdatedEventAsync(dataspaceName, anchorName, nodeXpath, + UPDATE, observedTimestamp)); + } + @Override public void replaceListContent(final String dataspaceName, final String anchorName, final String parentNodeXpath, final String jsonData, final OffsetDateTime observedTimestamp) { @@ -209,6 +225,13 @@ public class CpsDataServiceImpl implements CpsDataService { .build(); } + private List buildDataNodes(final String dataspaceName, final String anchorName, + final Map nodesJsonData) { + return nodesJsonData.entrySet().stream().map(nodeJsonData -> + buildDataNode(dataspaceName, anchorName, nodeJsonData.getKey(), + nodeJsonData.getValue())).collect(Collectors.toList()); + } + private Collection buildDataNodes(final String dataspaceName, final String anchorName, final String parentNodeXpath, diff --git a/cps-service/src/main/java/org/onap/cps/spi/CpsDataPersistenceService.java b/cps-service/src/main/java/org/onap/cps/spi/CpsDataPersistenceService.java index b27a2976d0..686f0f3fee 100644 --- a/cps-service/src/main/java/org/onap/cps/spi/CpsDataPersistenceService.java +++ b/cps-service/src/main/java/org/onap/cps/spi/CpsDataPersistenceService.java @@ -90,13 +90,22 @@ public interface CpsDataPersistenceService { void updateDataLeaves(String dataspaceName, String anchorName, String xpath, Map leaves); /** - * Replaces existing data node content including descendants. + * Replaces an existing data node's content including descendants. * * @param dataspaceName dataspace name * @param anchorName anchor name * @param dataNode data node */ - void replaceDataNodeTree(String dataspaceName, String anchorName, DataNode dataNode); + void updateDataNodeAndDescendants(String dataspaceName, String anchorName, DataNode dataNode); + + /** + * Replaces multiple existing data nodes' content including descendants in a batch operation. + * + * @param dataspaceName dataspace name + * @param anchorName anchor name + * @param dataNodes data nodes + */ + void updateDataNodesAndDescendants(String dataspaceName, String anchorName, final List dataNodes); /** * Replaces list content by removing all existing elements and inserting the given new elements diff --git a/cps-service/src/test/groovy/org/onap/cps/api/impl/CpsDataServiceImplSpec.groovy b/cps-service/src/test/groovy/org/onap/cps/api/impl/CpsDataServiceImplSpec.groovy index 6c995fa85e..cb352bccec 100644 --- a/cps-service/src/test/groovy/org/onap/cps/api/impl/CpsDataServiceImplSpec.groovy +++ b/cps-service/src/test/groovy/org/onap/cps/api/impl/CpsDataServiceImplSpec.groovy @@ -262,13 +262,13 @@ class CpsDataServiceImplSpec extends Specification { } - def 'Replace data node: #scenario.'() { + def 'Replace data node using singular data node: #scenario.'() { given: 'schema set for given anchor and dataspace references test-tree model' setupSchemaSetMocks('test-tree.yang') when: 'replace data method is invoked with json data #jsonData and parent node xpath #parentNodeXpath' - objectUnderTest.replaceNodeTree(dataspaceName, anchorName, parentNodeXpath, jsonData, observedTimestamp) + objectUnderTest.updateDataNodeAndDescendants(dataspaceName, anchorName, parentNodeXpath, jsonData, observedTimestamp) then: 'the persistence service method is invoked with correct parameters' - 1 * mockCpsDataPersistenceService.replaceDataNodeTree(dataspaceName, anchorName, + 1 * mockCpsDataPersistenceService.updateDataNodeAndDescendants(dataspaceName, anchorName, { dataNode -> dataNode.xpath == expectedNodeXpath }) and: 'data updated event is sent to notification service' 1 * mockNotificationService.processDataUpdatedEvent(dataspaceName, anchorName, parentNodeXpath, Operation.UPDATE, observedTimestamp) @@ -278,13 +278,46 @@ class CpsDataServiceImplSpec extends Specification { 'level 2 node' | '/test-tree' | '{"branch": [{"name":"Name"}]}' || '/test-tree/branch[@name=\'Name\']' } - def 'Replace data node with invalid #scenario.'() { + def 'Replace data node using multiple data nodes: #scenario.'() { + given: 'schema set for given anchor and dataspace references test-tree model' + setupSchemaSetMocks('test-tree.yang') + when: 'replace data method is invoked with a map of xpaths and json data' + objectUnderTest.updateDataNodesAndDescendants(dataspaceName, anchorName, nodesJsonData, observedTimestamp) + then: 'the persistence service method is invoked with correct parameters' + 1 * mockCpsDataPersistenceService.updateDataNodesAndDescendants(dataspaceName, anchorName, + { dataNode -> dataNode.xpath == expectedNodeXpath}) + and: 'data updated event is sent to notification service' + 1 * mockNotificationService.processDataUpdatedEvent(dataspaceName, anchorName, nodesJsonData.keySet()[0], Operation.UPDATE, observedTimestamp) + 1 * mockNotificationService.processDataUpdatedEvent(dataspaceName, anchorName, nodesJsonData.keySet()[1], Operation.UPDATE, observedTimestamp) + where: 'following parameters were used' + scenario | nodesJsonData || expectedNodeXpath + 'top level node' | ['/' : '{"test-tree": {"branch": []}}', '/test-tree' : '{"branch": [{"name":"Name"}]}'] || ["/test-tree", "/test-tree/branch[@name='Name']"] + 'level 2 node' | ['/test-tree' : '{"branch": [{"name":"Name"}]}', '/test-tree/branch[@name=\'Name\']':'{"nest":{"name":"nestName"}}'] || ["/test-tree/branch[@name='Name']", "/test-tree/branch[@name='Name']/nest"] + } + + def 'Replace data node using singular data node with invalid #scenario.'() { + when: 'replace data method is invoked with invalid #scenario' + objectUnderTest.updateDataNodeAndDescendants(dataspaceName, anchorName, '/', _ as String, observedTimestamp) + then: 'a data validation exception is thrown' + thrown(DataValidationException) + and: 'the persistence service method is not invoked' + 0 * mockCpsDataPersistenceService.updateDataNodeAndDescendants(*_) + and: 'data updated event is not sent to notification service' + 0 * mockNotificationService.processDataUpdatedEvent(*_) + where: 'the following parameters are used' + scenario | dataspaceName | anchorName + 'dataspace name' | 'dataspace names with spaces' | 'anchorName' + 'anchor name' | 'dataspaceName' | 'anchor name with spaces' + 'dataspace and anchor name' | 'dataspace name with spaces' | 'anchor name with spaces' + } + + def 'Replace data node using multiple data nodes with invalid #scenario.'() { when: 'replace data method is invoked with invalid #scenario' - objectUnderTest.replaceNodeTree(dataspaceName, anchorName, '/', _ as String, observedTimestamp) + objectUnderTest.updateDataNodesAndDescendants(dataspaceName, anchorName, ['/': _ as String], observedTimestamp) then: 'a data validation exception is thrown' thrown(DataValidationException) and: 'the persistence service method is not invoked' - 0 * mockCpsDataPersistenceService.replaceDataNodeTree(*_) + 0 * mockCpsDataPersistenceService.updateDataNodesAndDescendants(*_) and: 'data updated event is not sent to notification service' 0 * mockNotificationService.processDataUpdatedEvent(*_) where: 'the following parameters are used' -- cgit 1.2.3-korg