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>

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 61e1d5b569..e02fb7355b 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 acc243b5b4..ba9bd6f95a 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()
     }