Add DataNodeNotFoundException to deleteDataNodes

Current implementation of NCMP handle de-registration relies on
DataNodeNotFoundException being thrown to report errors.

- Make deleteDataNodes throw DataNodeFoundExceptionBatch
- Update performance test timings

Issue-ID: CPS-1481
Signed-off-by: danielhanrahan <daniel.hanrahan@est.tech>
Change-Id: Ib833bdb4a23d24e1784bdaf4e5e5e8a9acb41c54

4 files changed, 50 insertions, 25 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 2159ceae8a..f634008dc6 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
@@ -57,6 +57,7 @@ import org.onap.cps.spi.exceptions.ConcurrencyException;
 import org.onap.cps.spi.exceptions.CpsAdminException;
 import org.onap.cps.spi.exceptions.CpsPathException;
 import org.onap.cps.spi.exceptions.DataNodeNotFoundException;
+import org.onap.cps.spi.exceptions.DataNodeNotFoundExceptionBatch;
 import org.onap.cps.spi.model.DataNode;
 import org.onap.cps.spi.model.DataNodeBuilder;
 import org.onap.cps.spi.repository.AnchorRepository;
@@ -621,23 +622,30 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService
         final DataspaceEntity dataspaceEntity = dataspaceRepository.getByName(dataspaceName);
         final AnchorEntity anchorEntity = anchorRepository.getByDataspaceAndName(dataspaceEntity, anchorName);
 
-        final Collection<String> normalizedXPathsToDelete = new ArrayList<>(xpathsToDelete.size());
-        final Collection<String> normalizedXpathsToPotentialLists = new ArrayList<>();
+        final Collection<String> deleteChecklist = new HashSet<>(xpathsToDelete.size());
         for (final String xpath : xpathsToDelete) {
             try {
-                final CpsPathQuery cpsPathQuery = CpsPathUtil.getCpsPathQuery(xpath);
-                final String normalizedXpath = cpsPathQuery.getNormalizedXpath();
-                normalizedXPathsToDelete.add(normalizedXpath);
-                if (!cpsPathQuery.isPathToListElement()) {
-                    normalizedXpathsToPotentialLists.add(normalizedXpath);
-                }
+                deleteChecklist.add(CpsPathUtil.getNormalizedXpath(xpath));
             } catch (final PathParsingException e) {
                 log.debug("Error parsing xpath \"{}\": {}", xpath, e.getMessage());
             }
         }
 
-        fragmentRepository.deleteByAnchorIdAndXpaths(anchorEntity.getId(), normalizedXPathsToDelete);
-        fragmentRepository.deleteListsByAnchorIdAndXpaths(anchorEntity.getId(), normalizedXpathsToPotentialLists);
+        final Collection<String> xpathsToExistingContainers =
+            fragmentRepository.findAllXpathByAnchorAndXpathIn(anchorEntity, deleteChecklist);
+        deleteChecklist.removeAll(xpathsToExistingContainers);
+
+        final Collection<String> xpathsToExistingLists = deleteChecklist.stream()
+            .filter(xpath -> fragmentRepository.existsByAnchorAndXpathStartsWith(anchorEntity, xpath + "["))
+            .collect(Collectors.toList());
+        deleteChecklist.removeAll(xpathsToExistingLists);
+
+        if (!deleteChecklist.isEmpty()) {
+            throw new DataNodeNotFoundExceptionBatch(dataspaceName, anchorName, deleteChecklist);
+        }
+
+        fragmentRepository.deleteByAnchorIdAndXpaths(anchorEntity.getId(), xpathsToExistingContainers);
+        fragmentRepository.deleteListsByAnchorIdAndXpaths(anchorEntity.getId(), xpathsToExistingLists);
     }
 
     @Override
diff --git a/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentRepository.java b/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentRepository.java
index 8bdb7d985b..51ebcb4127 100755
--- a/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentRepository.java
+++ b/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentRepository.java
@@ -100,4 +100,10 @@ public interface FragmentRepository extends JpaRepository<FragmentEntity, Long>,
         nativeQuery = true)

     List<FragmentExtract> quickFindWithDescendants(@Param("anchorId") int anchorId,

                                                    @Param("xpathRegex") String xpathRegex);

+

+    @Query("SELECT f.xpath FROM FragmentEntity f WHERE f.anchor = :anchor AND f.xpath IN :xpaths")

+    List<String> findAllXpathByAnchorAndXpathIn(@Param("anchor") AnchorEntity anchorEntity,

+                                                @Param("xpaths") Collection<String> xpaths);

+

+    boolean existsByAnchorAndXpathStartsWith(AnchorEntity anchorEntity, String xpath);

 }

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 bb80a13206..28916b1c4a 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
@@ -41,7 +41,6 @@ import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.test.context.jdbc.Sql
 
 import javax.validation.ConstraintViolationException
-import java.nio.file.Path
 
 import static org.onap.cps.spi.FetchDescendantsOption.INCLUDE_ALL_DESCENDANTS
 import static org.onap.cps.spi.FetchDescendantsOption.OMIT_DESCENDANTS
@@ -57,7 +56,6 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase {
     static final String SET_DATA = '/data/fragment.sql'
     static long ID_DATA_NODE_WITH_DESCENDANTS = 4001
     static String XPATH_DATA_NODE_WITH_DESCENDANTS = '/parent-1'
-    static String XPATH_DATA_NODE_WITH_LEAVES = '/parent-207'
     static long DATA_NODE_202_FRAGMENT_ID = 4202L
     static long CHILD_OF_DATA_NODE_202_FRAGMENT_ID = 4203L
     static long LIST_DATA_NODE_PARENT201_FRAGMENT_ID = 4206L
@@ -309,6 +307,7 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase {
             '2 unique nodes with duplicate xpath'  | ["/parent-200", "/parent-202", "/parent-200"]   || 2
             'list element with key (single quote)' | ["/parent-201/child-204[@key='A']"]             || 1
             'list element with key (double quote)' | ['/parent-201/child-204[@key="A"]']             || 1
+            'whole list (not implemented)'         | ["/parent-201/child-204"]                       || 0
             'non-existing xpath'                   | ["/NO-XPATH"]                                   || 0
             'existing and non-existing xpaths'     | ["/parent-200", "/NO-XPATH", "/parent-201"]     || 2
             'invalid xpath'                        | ["INVALID XPATH"]                               || 0
@@ -585,11 +584,23 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase {
             'whole list'                      | ['/parent-203/child-204']                                              || ['/parent-203/child-203']
             'list and element in same list'   | ['/parent-203/child-204', '/parent-203/child-204[@key="A"]']           || ['/parent-203/child-203']
             'list element under list element' | ['/parent-203/child-204[@key="B"]/grand-child-204[@key2="Y"]']         || ["/parent-203/child-203", "/parent-203/child-204[@key='A']", "/parent-203/child-204[@key='B']"]
-            'valid but non-existing xpath'    | ['/non-existing', '/parent-203/child-204']                             || ['/parent-203/child-203']
             'invalid xpath'                   | ['INVALID XPATH', '/parent-203/child-204']                             || ['/parent-203/child-203']
     }
 
     @Sql([CLEAR_DATA, SET_DATA])
+    def 'Delete multiple data nodes error scenario: #scenario.'() {
+        when: 'deleting nodes is executed for: #scenario.'
+            objectUnderTest.deleteDataNodes(dataspaceName, anchorName, targetXpaths)
+        then: 'a #expectedException is thrown'
+            thrown(expectedException)
+        where: 'the following data is used'
+            scenario                 | dataspaceName  | anchorName     | targetXpaths            || expectedException
+            'non-existing dataspace' | 'NO DATASPACE' | 'not relevant' | ['/not relevant']       || DataspaceNotFoundException
+            'non-existing anchor'    | DATASPACE_NAME | 'NO ANCHOR'    | ['/not relevant']       || AnchorNotFoundException
+            'non-existing datanode'  | DATASPACE_NAME | ANCHOR_NAME3   | ['/NON-EXISTING-XPATH'] || DataNodeNotFoundException
+    }
+
+    @Sql([CLEAR_DATA, SET_DATA])
     def 'Delete data nodes with "/"-token in list key value: #scenario. (CPS-1409)'() {
         given: 'a data nodes with list-element child with "/" in index value (and grandchild)'
             def grandChild = new DataNodeBuilder().withXpath(deleteTestGrandChildXPath).build()
diff --git a/cps-ri/src/test/groovy/org/onap/cps/spi/performance/CpsDataPersistenceServiceDeletePerfTest.groovy b/cps-ri/src/test/groovy/org/onap/cps/spi/performance/CpsDataPersistenceServiceDeletePerfTest.groovy
index a08d8c66d2..eb138b98be 100644
--- a/cps-ri/src/test/groovy/org/onap/cps/spi/performance/CpsDataPersistenceServiceDeletePerfTest.groovy
+++ b/cps-ri/src/test/groovy/org/onap/cps/spi/performance/CpsDataPersistenceServiceDeletePerfTest.groovy
@@ -94,8 +94,8 @@ class CpsDataPersistenceServiceDeletePerfTest extends CpsPersistencePerfSpecBase
             objectUnderTest.deleteDataNodes(PERF_DATASPACE, PERF_ANCHOR, xpathsToDelete)
             stopWatch.stop()
             def deleteDurationInMillis = stopWatch.getTotalTimeMillis()
-        then: 'delete duration is under 125 milliseconds'
-            recordAndAssertPerformance('Batch delete 500 grandchildren', 125, deleteDurationInMillis)
+        then: 'delete duration is under 75 milliseconds'
+            recordAndAssertPerformance('Batch delete 500 grandchildren', 75, deleteDurationInMillis)
     }
 
     @Sql([CLEAR_DATA, PERF_TEST_DATA])
@@ -105,8 +105,8 @@ class CpsDataPersistenceServiceDeletePerfTest extends CpsPersistencePerfSpecBase
             createLineage(objectUnderTest, 150, 50, true)
             stopWatch.stop()
             def setupDurationInMillis = stopWatch.getTotalTimeMillis()
-        then: 'setup duration is under 10 seconds'
-            recordAndAssertPerformance('Setup lists', 10_000, setupDurationInMillis)
+        then: 'setup duration is under 5 seconds'
+            recordAndAssertPerformance('Setup lists', 5_000, setupDurationInMillis)
     }
 
     def 'Delete 5 whole lists'() {
@@ -132,8 +132,8 @@ class CpsDataPersistenceServiceDeletePerfTest extends CpsPersistencePerfSpecBase
             objectUnderTest.deleteDataNodes(PERF_DATASPACE, PERF_ANCHOR, xpathsToDelete)
             stopWatch.stop()
             def deleteDurationInMillis = stopWatch.getTotalTimeMillis()
-        then: 'delete duration is under 250 milliseconds'
-            recordAndAssertPerformance('Batch delete 100 whole lists', 250, deleteDurationInMillis)
+        then: 'delete duration is under 500 milliseconds'
+            recordAndAssertPerformance('Batch delete 100 whole lists', 500, deleteDurationInMillis)
     }
 
     def 'Delete 10 list elements'() {
@@ -190,8 +190,8 @@ class CpsDataPersistenceServiceDeletePerfTest extends CpsPersistencePerfSpecBase
             objectUnderTest.deleteDataNodes(PERF_DATASPACE, PERF_ANCHOR, [PERF_TEST_PARENT])
             stopWatch.stop()
             def deleteDurationInMillis = stopWatch.getTotalTimeMillis()
-        then: 'delete duration is under 250 milliseconds'
-            recordAndAssertPerformance('Batch delete one large node', 250, deleteDurationInMillis)
+        then: 'delete duration is under 200 milliseconds'
+            recordAndAssertPerformance('Batch delete one large node', 200, deleteDurationInMillis)
     }
 
     @Sql([CLEAR_DATA, PERF_TEST_DATA])
@@ -204,8 +204,8 @@ class CpsDataPersistenceServiceDeletePerfTest extends CpsPersistencePerfSpecBase
             objectUnderTest.deleteDataNode(PERF_DATASPACE, PERF_ANCHOR, '/')
             stopWatch.stop()
             def deleteDurationInMillis = stopWatch.getTotalTimeMillis()
-        then: 'delete duration is under 300 milliseconds'
-            recordAndAssertPerformance('Delete root node', 300, deleteDurationInMillis)
+        then: 'delete duration is under 250 milliseconds'
+            recordAndAssertPerformance('Delete root node', 250, deleteDurationInMillis)
     }
 
     @Sql([CLEAR_DATA, PERF_TEST_DATA])
@@ -218,8 +218,8 @@ class CpsDataPersistenceServiceDeletePerfTest extends CpsPersistencePerfSpecBase
             objectUnderTest.deleteDataNodes(PERF_DATASPACE, PERF_ANCHOR)
             stopWatch.stop()
             def deleteDurationInMillis = stopWatch.getTotalTimeMillis()
-        then: 'delete duration is under 300 milliseconds'
-            recordAndAssertPerformance('Delete data nodes for anchor', 300, deleteDurationInMillis)
+        then: 'delete duration is under 250 milliseconds'
+            recordAndAssertPerformance('Delete data nodes for anchor', 250, deleteDurationInMillis)
     }
 
     @Sql([CLEAR_DATA, PERF_TEST_DATA])