diff options
3 files changed, 72 insertions, 52 deletions
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 2fba30ee24..dd2d3652ea 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 @@ -120,7 +120,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService private void addNewChildDataNode(final String dataspaceName, final String anchorName, final String parentNodeXpath, final DataNode newChild) { final FragmentEntity parentFragmentEntity = - getFragmentWithoutDescendantsByXpath(dataspaceName, anchorName, parentNodeXpath); + getFragmentEntity(dataspaceName, anchorName, parentNodeXpath); final FragmentEntity newChildAsFragmentEntity = convertToFragmentWithAllDescendants(parentFragmentEntity.getDataspace(), parentFragmentEntity.getAnchor(), newChild); @@ -136,7 +136,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService private void addChildrenDataNodes(final String dataspaceName, final String anchorName, final String parentNodeXpath, final Collection<DataNode> newChildren) { final FragmentEntity parentFragmentEntity = - getFragmentWithoutDescendantsByXpath(dataspaceName, anchorName, parentNodeXpath); + getFragmentEntity(dataspaceName, anchorName, parentNodeXpath); final List<FragmentEntity> fragmentEntities = new ArrayList<>(newChildren.size()); try { newChildren.forEach(newChildAsDataNode -> { @@ -265,6 +265,12 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService public Collection<DataNode> getDataNodesForMultipleXpaths(final String dataspaceName, final String anchorName, final Collection<String> xpaths, final FetchDescendantsOption fetchDescendantsOption) { + final Collection<FragmentEntity> fragmentEntities = getFragmentEntities(dataspaceName, anchorName, xpaths); + return toDataNodes(fragmentEntities, fetchDescendantsOption); + } + + private Collection<FragmentEntity> getFragmentEntities(final String dataspaceName, final String anchorName, + final Collection<String> xpaths) { final DataspaceEntity dataspaceEntity = dataspaceRepository.getByName(dataspaceName); final AnchorEntity anchorEntity = anchorRepository.getByDataspaceAndName(dataspaceEntity, anchorName); @@ -276,7 +282,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService try { normalizedXpaths.add(CpsPathUtil.getNormalizedXpath(xpath)); } catch (final PathParsingException e) { - log.warn("Error parsing xpath \"{}\" in getDataNodesForMultipleXpaths: {}", xpath, e.getMessage()); + log.warn("Error parsing xpath \"{}\": {}", xpath, e.getMessage()); } } final Collection<FragmentEntity> fragmentEntities = @@ -288,17 +294,10 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService fragmentEntities.addAll(FragmentEntityArranger.toFragmentEntityTrees(anchorEntity, fragmentExtracts)); } - return toDataNodes(fragmentEntities, fetchDescendantsOption); + return fragmentEntities; } - private FragmentEntity getFragmentWithoutDescendantsByXpath(final String dataspaceName, - final String anchorName, - final String xpath) { - return getFragmentByXpath(dataspaceName, anchorName, xpath, FetchDescendantsOption.OMIT_DESCENDANTS); - } - - private FragmentEntity getFragmentByXpath(final String dataspaceName, final String anchorName, - final String xpath, final FetchDescendantsOption fetchDescendantsOption) { + private FragmentEntity getFragmentEntity(final String dataspaceName, final String anchorName, final String xpath) { final DataspaceEntity dataspaceEntity = dataspaceRepository.getByName(dataspaceName); final AnchorEntity anchorEntity = anchorRepository.getByDataspaceAndName(dataspaceEntity, anchorName); final FragmentEntity fragmentEntity; @@ -309,13 +308,8 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService .stream().findFirst().orElse(null); } else { final String normalizedXpath = getNormalizedXpath(xpath); - if (FetchDescendantsOption.OMIT_DESCENDANTS.equals(fetchDescendantsOption)) { - fragmentEntity = - fragmentRepository.getByDataspaceAndAnchorAndXpath(dataspaceEntity, anchorEntity, normalizedXpath); - } else { - fragmentEntity = buildFragmentEntitiesFromFragmentExtracts(anchorEntity, normalizedXpath) - .stream().findFirst().orElse(null); - } + fragmentEntity = + fragmentRepository.getByDataspaceAndAnchorAndXpath(dataspaceEntity, anchorEntity, normalizedXpath); } if (fragmentEntity == null) { throw new DataNodeNotFoundException(dataspaceEntity.getName(), anchorEntity.getName(), xpath); @@ -491,7 +485,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService @Override public void updateDataLeaves(final String dataspaceName, final String anchorName, final String xpath, final Map<String, Serializable> updateLeaves) { - final FragmentEntity fragmentEntity = getFragmentWithoutDescendantsByXpath(dataspaceName, anchorName, xpath); + final FragmentEntity fragmentEntity = getFragmentEntity(dataspaceName, anchorName, xpath); final String currentLeavesAsString = fragmentEntity.getAttributes(); final String mergedLeaves = mergeLeaves(updateLeaves, currentLeavesAsString); fragmentEntity.setAttributes(mergedLeaves); @@ -501,8 +495,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService @Override public void updateDataNodeAndDescendants(final String dataspaceName, final String anchorName, final DataNode dataNode) { - final FragmentEntity fragmentEntity = - getFragmentWithoutDescendantsByXpath(dataspaceName, anchorName, dataNode.getXpath()); + final FragmentEntity fragmentEntity = getFragmentEntity(dataspaceName, anchorName, dataNode.getXpath()); updateFragmentEntityAndDescendantsWithDataNode(fragmentEntity, dataNode); try { fragmentRepository.save(fragmentEntity); @@ -514,21 +507,24 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService } @Override - public void updateDataNodesAndDescendants(final String dataspaceName, - final String anchorName, - final List<DataNode> dataNodes) { - - final Map<DataNode, FragmentEntity> dataNodeFragmentEntityMap = dataNodes.stream() - .collect(Collectors.toMap( - dataNode -> dataNode, - dataNode -> - getFragmentWithoutDescendantsByXpath(dataspaceName, anchorName, dataNode.getXpath()))); - dataNodeFragmentEntityMap.forEach( - (dataNode, fragmentEntity) -> updateFragmentEntityAndDescendantsWithDataNode(fragmentEntity, dataNode)); + public void updateDataNodesAndDescendants(final String dataspaceName, final String anchorName, + final List<DataNode> updatedDataNodes) { + final Map<String, DataNode> xpathToUpdatedDataNode = updatedDataNodes.stream() + .collect(Collectors.toMap(DataNode::getXpath, dataNode -> dataNode)); + + final Collection<String> xpaths = xpathToUpdatedDataNode.keySet(); + final Collection<FragmentEntity> existingFragmentEntities = + getFragmentEntities(dataspaceName, anchorName, xpaths); + + for (final FragmentEntity existingFragmentEntity : existingFragmentEntities) { + final DataNode updatedDataNode = xpathToUpdatedDataNode.get(existingFragmentEntity.getXpath()); + updateFragmentEntityAndDescendantsWithDataNode(existingFragmentEntity, updatedDataNode); + } + try { - fragmentRepository.saveAll(dataNodeFragmentEntityMap.values()); + fragmentRepository.saveAll(existingFragmentEntities); } catch (final StaleStateException staleStateException) { - retryUpdateDataNodesIndividually(dataspaceName, anchorName, dataNodeFragmentEntityMap.values()); + retryUpdateDataNodesIndividually(dataspaceName, anchorName, existingFragmentEntities); } } @@ -582,8 +578,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService @Transactional public void replaceListContent(final String dataspaceName, final String anchorName, final String parentNodeXpath, final Collection<DataNode> newListElements) { - final FragmentEntity parentEntity = - getFragmentWithoutDescendantsByXpath(dataspaceName, anchorName, parentNodeXpath); + final FragmentEntity parentEntity = getFragmentEntity(dataspaceName, anchorName, parentNodeXpath); final String listElementXpathPrefix = getListElementXpathPrefix(newListElements); final Map<String, FragmentEntity> existingListElementFragmentEntitiesByXPath = extractListElementFragmentEntitiesByXPath(parentEntity.getChildFragments(), listElementXpathPrefix); @@ -631,7 +626,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService try { normalizedXpaths.add(CpsPathUtil.getNormalizedXpath(xpath)); } catch (final PathParsingException e) { - log.debug("Error parsing xpath \"{}\" in deleteDataNodes: {}", xpath, e.getMessage()); + log.debug("Error parsing xpath \"{}\": {}", xpath, e.getMessage()); } } @@ -666,7 +661,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService } else { parentNodeXpath = CpsPathUtil.getNormalizedParentXpath(targetXpath); } - parentFragmentEntity = getFragmentWithoutDescendantsByXpath(dataspaceName, anchorName, parentNodeXpath); + parentFragmentEntity = getFragmentEntity(dataspaceName, anchorName, parentNodeXpath); if (CpsPathUtil.isPathToListElement(targetXpath)) { targetDeleted = deleteDataNode(parentFragmentEntity, targetXpath); } else { 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 ac66e8c023..ba42a083ea 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 @@ -88,15 +88,17 @@ class CpsDataPersistenceServiceSpec extends Specification { } def 'Handling of StaleStateException (caused by concurrent updates) during update data nodes and descendants.'() { - given: 'the system contains and can update one datanode' - def dataNode1 = createDataNodeAndMockRepositoryMethodSupportingIt('/node1', 'OK') - and: 'the system contains two more datanodes that throw an exception while updating' - def dataNode2 = createDataNodeAndMockRepositoryMethodSupportingIt('/node2', 'EXCEPTION') - def dataNode3 = createDataNodeAndMockRepositoryMethodSupportingIt('/node3', 'EXCEPTION') + given: 'the system can update one datanode and has two more datanodes that throw an exception while updating' + def dataNodes = createDataNodesAndMockRepositoryMethodSupportingThem([ + '/node1': 'OK', + '/node2': 'EXCEPTION', + '/node3': 'EXCEPTION']) + and: 'db contains an anchor' + mockAnchorRepository.getByDataspaceAndName(*_) >> new AnchorEntity(id:123) and: 'the batch update will therefore also fail' mockFragmentRepository.saveAll(*_) >> { throw new StaleStateException("concurrent updates") } when: 'attempt batch update data nodes' - objectUnderTest.updateDataNodesAndDescendants('some-dataspace', 'some-anchor', [dataNode1, dataNode2, dataNode3]) + objectUnderTest.updateDataNodesAndDescendants('some-dataspace', 'some-anchor', dataNodes) then: 'concurrency exception is thrown' def thrown = thrown(ConcurrencyException) assert thrown.message == 'Concurrent Transactions' @@ -199,7 +201,9 @@ class CpsDataPersistenceServiceSpec extends Specification { def 'update data node and descendants: #scenario'(){ given: 'mocked responses' - mockFragmentRepository.getByDataspaceAndAnchorAndXpath(_, _, '/test/xpath') >> new FragmentEntity(xpath: '/test/xpath', childFragments: []) + mockAnchorRepository.getByDataspaceAndName(_, _) >> new AnchorEntity(id:123) + mockFragmentRepository.findByAnchorAndMultipleCpsPaths(_, [] as Set) >> [] + mockFragmentRepository.findByAnchorAndMultipleCpsPaths(_, ['/test/xpath'] as Set) >> [new FragmentEntity(xpath: '/test/xpath', childFragments: [])] when: 'replace data node tree' objectUnderTest.updateDataNodesAndDescendants('dataspaceName', 'anchorName', dataNodes) then: 'call fragment repository save all method' @@ -211,9 +215,12 @@ class CpsDataPersistenceServiceSpec extends Specification { } 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: []) + given: 'the fragment repository returns fragment entities related to the xpath inputs' + mockFragmentRepository.findByAnchorAndMultipleCpsPaths(_, ['/test/xpath1', '/test/xpath2'] as Set) >> [ + new FragmentEntity(xpath: '/test/xpath1', childFragments: []), + new FragmentEntity(xpath: '/test/xpath2', childFragments: [])] + and: 'db contains an anchor' + mockAnchorRepository.getByDataspaceAndName(*_) >> new AnchorEntity(id:123) 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'])]) @@ -239,6 +246,25 @@ class CpsDataPersistenceServiceSpec extends Specification { return dataNode } + def createDataNodesAndMockRepositoryMethodSupportingThem(Map<String, String> xpathToScenarioMap) { + def dataNodes = [] + def fragmentEntities = [] + xpathToScenarioMap.each { + def xpath = it.key + def scenario = it.value + def dataNode = new DataNodeBuilder().withXpath(xpath).build() + dataNodes.add(dataNode) + def fragmentEntity = new FragmentEntity(xpath: xpath, childFragments: []) + fragmentEntities.add(fragmentEntity) + mockFragmentRepository.getByDataspaceAndAnchorAndXpath(_, _, xpath) >> fragmentEntity + if ('EXCEPTION' == scenario) { + mockFragmentRepository.save(fragmentEntity) >> { throw new StaleStateException("concurrent updates") } + } + } + mockFragmentRepository.findByAnchorAndMultipleCpsPaths(_, xpathToScenarioMap.keySet()) >> fragmentEntities + return dataNodes + } + def mockFragmentWithJson(json) { def anchorEntity = new AnchorEntity(id:123) mockAnchorRepository.getByDataspaceAndName(*_) >> anchorEntity diff --git a/cps-ri/src/test/groovy/org/onap/cps/spi/performance/CpsDataPersistenceServicePerfTest.groovy b/cps-ri/src/test/groovy/org/onap/cps/spi/performance/CpsDataPersistenceServicePerfTest.groovy index cfd1093a99..3562419c05 100644 --- a/cps-ri/src/test/groovy/org/onap/cps/spi/performance/CpsDataPersistenceServicePerfTest.groovy +++ b/cps-ri/src/test/groovy/org/onap/cps/spi/performance/CpsDataPersistenceServicePerfTest.groovy @@ -108,7 +108,6 @@ class CpsDataPersistenceServicePerfTest extends CpsPersistencePerfSpecBase { stopWatch.stop() def readDurationInMillis = stopWatch.getTotalTimeMillis() then: 'read duration is under #allowedDuration milliseconds' - assert readDurationInMillis < allowedDuration recordAndAssertPerformance("Query many descendants by cpspath (${scenario})", allowedDuration, readDurationInMillis) and: 'data node is returned with all the descendants populated' assert result.size() == NUMBER_OF_CHILDREN @@ -153,7 +152,7 @@ class CpsDataPersistenceServicePerfTest extends CpsPersistencePerfSpecBase { objectUnderTest.updateDataNodesAndDescendants(PERF_DATASPACE, PERF_ANCHOR, dataNodes) stopWatch.stop() def updateDurationInMillis = stopWatch.getTotalTimeMillis() - then: 'update duration is under 1400 milliseconds' - recordAndAssertPerformance('Update data nodes without descendants', 1400, updateDurationInMillis) + then: 'update duration is under 600 milliseconds' + recordAndAssertPerformance('Update data nodes without descendants', 600, updateDurationInMillis) } } |