Merge "Improve performance of deleteDataNodes SQL"

2 files changed, 46 insertions, 43 deletions

diff --git a/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentNativeRepositoryImpl.java b/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentNativeRepositoryImpl.java
index 0e4d359da..5c5458a03 100644
--- a/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentNativeRepositoryImpl.java
+++ b/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentNativeRepositoryImpl.java
@@ -21,8 +21,11 @@
 package org.onap.cps.spi.repository;
 
 import java.util.Collection;
+import java.util.Collections;
+import java.util.stream.Collectors;
 import javax.persistence.EntityManager;
 import javax.persistence.PersistenceContext;
+import javax.persistence.Query;
 import lombok.RequiredArgsConstructor;
 
 @RequiredArgsConstructor
@@ -40,55 +43,55 @@ public class FragmentNativeRepositoryImpl implements FragmentNativeRepository {
     @PersistenceContext
     private final EntityManager entityManager;
 
-    private final TempTableCreator tempTableCreator;
-
     @Override
     public void deleteFragmentEntity(final long fragmentEntityId) {
         entityManager.createNativeQuery(
-                DROP_FRAGMENT_CONSTRAINT
-                    + ADD_FRAGMENT_CONSTRAINT_WITH_CASCADE
-                    + "DELETE FROM fragment WHERE id = ?;"
-                    + DROP_FRAGMENT_CONSTRAINT
-                    + ADD_ORIGINAL_FRAGMENT_CONSTRAINT)
+                addFragmentConstraintWithDeleteCascade("DELETE FROM fragment WHERE id = ?"))
             .setParameter(1, fragmentEntityId)
             .executeUpdate();
     }
 
     @Override
-    // Accept security hotspot as temporary table name in SQL query is created internally, not from user input.
-    @SuppressWarnings("squid:S2077")
     public void deleteByAnchorIdAndXpaths(final int anchorId, final Collection<String> xpaths) {
-        if (!xpaths.isEmpty()) {
-            final String tempTableName = tempTableCreator.createTemporaryTable("xpathsToDelete", xpaths, "xpath");
-            entityManager.createNativeQuery(
-                    DROP_FRAGMENT_CONSTRAINT
-                        + ADD_FRAGMENT_CONSTRAINT_WITH_CASCADE
-                        + "DELETE FROM fragment f USING " + tempTableName + " t"
-                        + " WHERE f.anchor_id = :anchorId AND f.xpath = t.xpath;"
-                        + DROP_FRAGMENT_CONSTRAINT
-                        + ADD_ORIGINAL_FRAGMENT_CONSTRAINT)
-                .setParameter("anchorId", anchorId)
-                .executeUpdate();
-        }
+        final String queryString = addFragmentConstraintWithDeleteCascade(
+            "DELETE FROM fragment f WHERE f.anchor_id = ? AND (f.xpath IN (:parameterPlaceholders))");
+        executeUpdateWithAnchorIdAndCollection(queryString, anchorId, xpaths);
     }
 
     @Override
-    // Accept security hotspot as temporary table name in SQL query is created internally, not from user input.
+    public void deleteListsByAnchorIdAndXpaths(final int anchorId, final Collection<String> listXpaths) {
+        final Collection<String> listXpathPatterns =
+            listXpaths.stream().map(listXpath -> listXpath + "[%").collect(Collectors.toSet());
+        final String queryString = addFragmentConstraintWithDeleteCascade(
+            "DELETE FROM fragment f WHERE f.anchor_id = ? AND (f.xpath LIKE ANY (array[:parameterPlaceholders]))");
+        executeUpdateWithAnchorIdAndCollection(queryString, anchorId, listXpathPatterns);
+    }
+
+    // Accept security hotspot as placeholders in SQL query are created internally, not from user input.
     @SuppressWarnings("squid:S2077")
-    public void deleteListsByAnchorIdAndXpaths(final int anchorId, final Collection<String> xpaths) {
-        if (!xpaths.isEmpty()) {
-            final String tempTableName = tempTableCreator.createTemporaryTable("xpathsToDelete", xpaths, "xpath");
-            entityManager.createNativeQuery(
-                DROP_FRAGMENT_CONSTRAINT
-                    + ADD_FRAGMENT_CONSTRAINT_WITH_CASCADE
-                    + "DELETE FROM fragment f USING " + tempTableName + " t"
-                    + " WHERE f.anchor_id = :anchorId AND f.xpath LIKE CONCAT(t.xpath, :xpathListPattern);"
-                    + DROP_FRAGMENT_CONSTRAINT
-                    + ADD_ORIGINAL_FRAGMENT_CONSTRAINT)
-                .setParameter("anchorId", anchorId)
-                .setParameter("xpathListPattern", "[%%")
-                .executeUpdate();
+    private void executeUpdateWithAnchorIdAndCollection(final String sqlTemplate, final int anchorId,
+                                                        final Collection<String> collection) {
+        if (!collection.isEmpty()) {
+            final String parameterPlaceholders = String.join(",", Collections.nCopies(collection.size(), "?"));
+            final String queryStringWithParameterPlaceholders =
+                sqlTemplate.replaceFirst(":parameterPlaceholders\\b", parameterPlaceholders);
+
+            final Query query = entityManager.createNativeQuery(queryStringWithParameterPlaceholders);
+            query.setParameter(1, anchorId);
+            int parameterIndex = 2;
+            for (final String parameterValue : collection) {
+                query.setParameter(parameterIndex++, parameterValue);
+            }
+            query.executeUpdate();
         }
     }
 
+    private static String addFragmentConstraintWithDeleteCascade(final String queryString) {
+        return DROP_FRAGMENT_CONSTRAINT
+            + ADD_FRAGMENT_CONSTRAINT_WITH_CASCADE
+            + queryString + ";"
+            + DROP_FRAGMENT_CONSTRAINT
+            + ADD_ORIGINAL_FRAGMENT_CONSTRAINT;
+    }
+
 }
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 db09382ab..8e74b6222 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
@@ -67,8 +67,8 @@ class CpsDataPersistenceServiceDeletePerfTest extends CpsPersistencePerfSpecBase
             objectUnderTest.deleteDataNodes(PERF_DATASPACE, PERF_ANCHOR, xpathsToDelete)
             stopWatch.stop()
             def deleteDurationInMillis = stopWatch.getTotalTimeMillis()
-        then: 'delete duration is under 300 milliseconds'
-            recordAndAssertPerformance('Batch delete 100 children', 300, deleteDurationInMillis)
+        then: 'delete duration is under 250 milliseconds'
+            recordAndAssertPerformance('Batch delete 100 children', 250, deleteDurationInMillis)
     }
 
     def 'Delete 50 grandchildren (that have no descendants)'() {
@@ -97,8 +97,8 @@ class CpsDataPersistenceServiceDeletePerfTest extends CpsPersistencePerfSpecBase
             objectUnderTest.deleteDataNodes(PERF_DATASPACE, PERF_ANCHOR, xpathsToDelete)
             stopWatch.stop()
             def deleteDurationInMillis = stopWatch.getTotalTimeMillis()
-        then: 'delete duration is under 300 milliseconds'
-            recordAndAssertPerformance('Batch delete 500 grandchildren', 300, deleteDurationInMillis)
+        then: 'delete duration is under 125 milliseconds'
+            recordAndAssertPerformance('Batch delete 500 grandchildren', 125, deleteDurationInMillis)
     }
 
     @Sql([CLEAR_DATA, PERF_TEST_DATA])
@@ -135,8 +135,8 @@ class CpsDataPersistenceServiceDeletePerfTest extends CpsPersistencePerfSpecBase
             objectUnderTest.deleteDataNodes(PERF_DATASPACE, PERF_ANCHOR, xpathsToDelete)
             stopWatch.stop()
             def deleteDurationInMillis = stopWatch.getTotalTimeMillis()
-        then: 'delete duration is under 350 milliseconds'
-            recordAndAssertPerformance('Batch delete 100 whole lists', 350, deleteDurationInMillis)
+        then: 'delete duration is under 250 milliseconds'
+            recordAndAssertPerformance('Batch delete 100 whole lists', 250, deleteDurationInMillis)
     }
 
     def 'Delete 10 list elements'() {
@@ -165,8 +165,8 @@ class CpsDataPersistenceServiceDeletePerfTest extends CpsPersistencePerfSpecBase
             objectUnderTest.deleteDataNodes(PERF_DATASPACE, PERF_ANCHOR, xpathsToDelete)
             stopWatch.stop()
             def deleteDurationInMillis = stopWatch.getTotalTimeMillis()
-        then: 'delete duration is under 300 milliseconds'
-            recordAndAssertPerformance('Batch delete 500 lists elements', 300, deleteDurationInMillis)
+        then: 'delete duration is under 125 milliseconds'
+            recordAndAssertPerformance('Batch delete 500 lists elements', 125, deleteDurationInMillis)
     }
 
     @Sql([CLEAR_DATA, PERF_TEST_DATA])