Improve performance for update data nodes

- Extract method getFragmentEntities from getDataNodes
- Remove unused code from getFragmentEntity
- Use bulk getFragmentEntities in updateDataNodesAndDescendants
- Update performance test timings

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

2 files changed, 38 insertions, 13 deletions

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)
     }
 }