diff options
author | 2022-09-06 18:29:34 +0100 | |
---|---|---|
committer | 2022-09-12 18:09:43 +0100 | |
commit | 7b6ab50231f5ab39d1476531031437f81328115e (patch) | |
tree | 3939da0046a69ab31d391e14bd2b5af699202baf /cps-ri/src | |
parent | 440dc8aab179f6c8451683f53b019ccd8bd60bdf (diff) |
Handle partial failure
- Removing the transaction boundaries as it was getting rollbacked on
partial failures
- Handled adding the elements in batch and if it fails try them
individually
- Refactored code a bit and when there is partial failure we try one
more time in sequence and even if there are failures we collect the
failures
Issue-ID: CPS-1232
Issue-ID: CPS-1126
Change-Id: I7824c9f37f80cbaeedd5dc06d598ca0e3a69c59b
Signed-off-by: mpriyank <priyank.maheshwari@est.tech>
Diffstat (limited to 'cps-ri/src')
-rw-r--r-- | cps-ri/src/main/java/org/onap/cps/spi/impl/CpsDataPersistenceServiceImpl.java | 67 | ||||
-rwxr-xr-x | cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceIntegrationSpec.groovy | 23 |
2 files changed, 68 insertions, 22 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 61e1d5b56..e02fb7355 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 @@ -52,6 +52,7 @@ import org.onap.cps.spi.entities.FragmentEntity; import org.onap.cps.spi.entities.SchemaSetEntity; import org.onap.cps.spi.entities.YangResourceEntity; import org.onap.cps.spi.exceptions.AlreadyDefinedException; +import org.onap.cps.spi.exceptions.AlreadyDefinedExceptionBatch; import org.onap.cps.spi.exceptions.ConcurrencyException; import org.onap.cps.spi.exceptions.CpsAdminException; import org.onap.cps.spi.exceptions.CpsPathException; @@ -88,51 +89,75 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService Pattern.compile("\\[(\\@([^\\/]{0,9999}))\\]$"); @Override - @Transactional public void addChildDataNode(final String dataspaceName, final String anchorName, final String parentNodeXpath, final DataNode newChildDataNode) { - addChildDataNodes(dataspaceName, anchorName, parentNodeXpath, Collections.singleton(newChildDataNode)); + addNewChildDataNode(dataspaceName, anchorName, parentNodeXpath, newChildDataNode); } @Override - @Transactional public void addListElements(final String dataspaceName, final String anchorName, final String parentNodeXpath, final Collection<DataNode> newListElements) { - addChildDataNodes(dataspaceName, anchorName, parentNodeXpath, newListElements); + addChildrenDataNodes(dataspaceName, anchorName, parentNodeXpath, newListElements); } @Override - @Transactional - public void addListElementsBatch(final String dataspaceName, final String anchorName, final String parentNodeXpath, - final Collection<Collection<DataNode>> newListsElements) { + public void addMultipleLists(final String dataspaceName, final String anchorName, final String parentNodeXpath, + final Collection<Collection<DataNode>> newLists) { + final Collection<String> failedCmHandleIds = new HashSet<>(); + newLists.forEach(newList -> { + try { + addChildrenDataNodes(dataspaceName, anchorName, parentNodeXpath, newList); + } catch (final AlreadyDefinedException e) { + newList.forEach(listElement -> failedCmHandleIds.add((String) listElement.getLeaves().get("id"))); + } + }); - newListsElements.forEach( - newListElement -> addListElements(dataspaceName, anchorName, parentNodeXpath, newListElement)); + if (!failedCmHandleIds.isEmpty()) { + throw new AlreadyDefinedExceptionBatch(failedCmHandleIds); + } } - private void addChildDataNodes(final String dataspaceName, final String anchorName, final String parentNodeXpath, - final Collection<DataNode> newChildren) { + private void addNewChildDataNode(final String dataspaceName, final String anchorName, + final String parentNodeXpath, final DataNode newChild) { final FragmentEntity parentFragmentEntity = getFragmentByXpath(dataspaceName, anchorName, parentNodeXpath); + final FragmentEntity newChildAsFragmentEntity = + convertToFragmentWithAllDescendants(parentFragmentEntity.getDataspace(), + parentFragmentEntity.getAnchor(), newChild); + newChildAsFragmentEntity.setParentId(parentFragmentEntity.getId()); + try { + fragmentRepository.save(newChildAsFragmentEntity); + } catch (final DataIntegrityViolationException e) { + throw AlreadyDefinedException.forDataNode(newChild.getXpath(), anchorName, e); + } + + } + + private void addChildrenDataNodes(final String dataspaceName, final String anchorName, final String parentNodeXpath, + final Collection<DataNode> newChildren) { + final FragmentEntity parentFragmentEntity = getFragmentByXpath(dataspaceName, anchorName, parentNodeXpath); + final List<FragmentEntity> fragmentEntities = new ArrayList<>(newChildren.size()); try { - final List<FragmentEntity> fragmentEntities = new ArrayList<>(); newChildren.forEach(newChildAsDataNode -> { - final FragmentEntity newChildAsFragmentEntity = convertToFragmentWithAllDescendants( - parentFragmentEntity.getDataspace(), - parentFragmentEntity.getAnchor(), - newChildAsDataNode); + final FragmentEntity newChildAsFragmentEntity = + convertToFragmentWithAllDescendants(parentFragmentEntity.getDataspace(), + parentFragmentEntity.getAnchor(), newChildAsDataNode); newChildAsFragmentEntity.setParentId(parentFragmentEntity.getId()); fragmentEntities.add(newChildAsFragmentEntity); }); fragmentRepository.saveAll(fragmentEntities); - } catch (final DataIntegrityViolationException exception) { - final List<String> conflictXpaths = newChildren.stream() - .map(DataNode::getXpath) - .collect(Collectors.toList()); - throw AlreadyDefinedException.forDataNodes(conflictXpaths, anchorName, exception); + } catch (final DataIntegrityViolationException e) { + log.warn("Exception occurred : {} , Batch with size : {} will be retried using individual save operations", + e, fragmentEntities.size()); + retrySavingEachChildIndividually(dataspaceName, anchorName, parentNodeXpath, newChildren); } } + private void retrySavingEachChildIndividually(final String dataspaceName, final String anchorName, + final String parentNodeXpath, final Collection<DataNode> newChildren) { + newChildren.forEach(newChild -> addNewChildDataNode(dataspaceName, anchorName, parentNodeXpath, newChild)); + } + @Override public void storeDataNode(final String dataspaceName, final String anchorName, final DataNode dataNode) { final DataspaceEntity dataspaceEntity = dataspaceRepository.getByName(dataspaceName); 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 acc243b5b..ba9bd6f95 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 @@ -27,6 +27,7 @@ import org.onap.cps.cpspath.parser.PathParsingException import org.onap.cps.spi.CpsDataPersistenceService import org.onap.cps.spi.entities.FragmentEntity import org.onap.cps.spi.exceptions.AlreadyDefinedException +import org.onap.cps.spi.exceptions.AlreadyDefinedExceptionBatch import org.onap.cps.spi.exceptions.AnchorNotFoundException import org.onap.cps.spi.exceptions.CpsAdminException import org.onap.cps.spi.exceptions.CpsPathException @@ -165,7 +166,7 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase { def grandChild = buildDataNode('/parent-201/child-204[@key="NEW1"]/grand-child-204[@key2="NEW1-CHILD"]', [leave:'value'], []) listElements[0].childDataNodes = [grandChild] when: 'the new data node (list elements) are added to an existing parent node' - objectUnderTest.addListElementsBatch(DATASPACE_NAME, ANCHOR_NAME3, '/parent-201', [listElements]) + objectUnderTest.addMultipleLists(DATASPACE_NAME, ANCHOR_NAME3, '/parent-201', [listElements]) then: 'new entries are successfully persisted, parent node now contains 5 children (2 new + 3 existing before)' def parentFragment = fragmentRepository.getById(LIST_DATA_NODE_PARENT201_FRAGMENT_ID) def allChildXpaths = parentFragment.childFragments.collect { it.xpath } @@ -179,6 +180,22 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase { } @Sql([CLEAR_DATA, SET_DATA]) + def 'Add already existing list elements'() { + given: 'two new child list elements for an existing parent' + def listElementXpaths1 = ['/parent-100', '/parent-100/child-001'] + def listElementXpaths2 = ['/parent-200', '/parent-200/child-201'] + def listElements1 = toDataNodesWithId(listElementXpaths1, 'cmhandle1') + def listElements2 = toDataNodesWithId(listElementXpaths2, 'cmhandle2') + when: 'duplicate data node is requested to be added' + objectUnderTest.addMultipleLists(DATASPACE_NAME, ANCHOR_NAME3, '/', [listElements1,listElements2]) + then: 'already defined batch exception is thrown' + def e = thrown(AlreadyDefinedExceptionBatch) + and: 'it contains both cmhandle ids' + assert e.alreadyDefinedCmHandleIds.size() == 2 + assert e.alreadyDefinedCmHandleIds.containsAll(['cmhandle1', 'cmhandle2']) + } + + @Sql([CLEAR_DATA, SET_DATA]) def 'Add list element error scenario: #scenario.'() { given: 'list element as a collection of data nodes' def listElementCollection = toDataNodes(listElementXpaths) @@ -559,6 +576,10 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase { return xpaths.collect { new DataNodeBuilder().withXpath(it).build() } } + static Collection<DataNode> toDataNodesWithId(xpaths, id) { + return xpaths.collect { new DataNodeBuilder().withXpath(it).withLeaves(['id': id]).build() } + } + static DataNode buildDataNode(xpath, leaves, childDataNodes) { return new DataNodeBuilder().withXpath(xpath).withLeaves(leaves).withChildDataNodes(childDataNodes).build() } |