From 9270546119b5f63ded27fb847f0f0700df4dddd4 Mon Sep 17 00:00:00 2001 From: danielhanrahan Date: Thu, 18 May 2023 10:18:10 +0100 Subject: [PATCH] Allow getDataNodes to read whole lists(ep 1) - getDataNodes can now retrieve list data nodes. Issue-ID: CPS-1696 Signed-off-by: arpitsingh Change-Id: I320a368d6cb73599d3f7c13fe9b8ab7d0cc69470 --- .../spi/impl/CpsDataPersistenceServiceImpl.java | 68 +++++++++++++--------- .../cps/spi/repository/FragmentRepository.java | 15 ++++- .../spi/impl/CpsDataPersistenceServiceSpec.groovy | 1 + docs/release-notes.rst | 2 +- .../CpsDataServiceIntegrationSpec.groovy | 58 +++++++++++++----- .../integration/performance/cps/GetPerfTest.groovy | 11 ++-- 6 files changed, 102 insertions(+), 53 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 f4afe3d73..f904e8bdd 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 @@ -120,7 +120,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService newChildAsFragmentEntity.setParentId(parentFragmentEntity.getId()); try { fragmentRepository.save(newChildAsFragmentEntity); - } catch (final DataIntegrityViolationException e) { + } catch (final DataIntegrityViolationException dataIntegrityViolationException) { throw AlreadyDefinedException.forDataNodes(Collections.singletonList(newChild.getXpath()), anchorEntity.getName()); } @@ -138,9 +138,9 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService fragmentEntities.add(newChildAsFragmentEntity); } fragmentRepository.saveAll(fragmentEntities); - } catch (final DataIntegrityViolationException e) { + } catch (final DataIntegrityViolationException dataIntegrityViolationException) { log.warn("Exception occurred : {} , While saving : {} children, retrying using individual save operations", - e, fragmentEntities.size()); + dataIntegrityViolationException, fragmentEntities.size()); retrySavingEachChildIndividually(anchorEntity, parentNodeXpath, newChildren); } } @@ -151,7 +151,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService for (final DataNode newChild : newChildren) { try { addNewChildDataNode(anchorEntity, parentNodeXpath, newChild); - } catch (final AlreadyDefinedException e) { + } catch (final AlreadyDefinedException alreadyDefinedException) { failedXpaths.add(newChild.getXpath()); } } @@ -184,7 +184,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService try { final FragmentEntity fragmentEntity = convertToFragmentWithAllDescendants(anchorEntity, dataNode); fragmentRepository.save(fragmentEntity); - } catch (final DataIntegrityViolationException e) { + } catch (final DataIntegrityViolationException dataIntegrityViolationException) { failedXpaths.add(dataNode.getXpath()); } } @@ -251,22 +251,28 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService private Collection getFragmentEntities(final AnchorEntity anchorEntity, final Collection xpaths) { - final Collection nonRootXpaths = new HashSet<>(xpaths); - final boolean haveRootXpath = nonRootXpaths.removeIf(CpsDataPersistenceServiceImpl::isRootXpath); + final Collection normalizedXpaths = getNormalizedXpaths(xpaths); - final Collection normalizedXpaths = new HashSet<>(nonRootXpaths.size()); - for (final String xpath : nonRootXpaths) { - try { - normalizedXpaths.add(CpsPathUtil.getNormalizedXpath(xpath)); - } catch (final PathParsingException e) { - log.warn("Error parsing xpath \"{}\": {}", xpath, e.getMessage()); + final boolean haveRootXpath = normalizedXpaths.removeIf(CpsDataPersistenceServiceImpl::isRootXpath); + + final List fragmentEntities = fragmentRepository.findByAnchorAndXpathIn(anchorEntity, + normalizedXpaths); + + for (final FragmentEntity fragmentEntity : fragmentEntities) { + normalizedXpaths.remove(fragmentEntity.getXpath()); + } + + for (final String xpath : normalizedXpaths) { + if (!CpsPathUtil.isPathToListElement(xpath)) { + fragmentEntities.addAll(fragmentRepository.findListByAnchorAndXpath(anchorEntity, xpath)); } } + if (haveRootXpath) { - normalizedXpaths.addAll(fragmentRepository.findAllXpathByAnchorAndParentIdIsNull(anchorEntity)); + fragmentEntities.addAll(fragmentRepository.findRootsByAnchorId(anchorEntity.getId())); } - return fragmentRepository.findByAnchorAndXpathIn(anchorEntity, normalizedXpaths); + return fragmentEntities; } private FragmentEntity getFragmentEntity(final AnchorEntity anchorEntity, final String xpath) { @@ -293,8 +299,8 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService final CpsPathQuery cpsPathQuery; try { cpsPathQuery = CpsPathUtil.getCpsPathQuery(cpsPath); - } catch (final PathParsingException e) { - throw new CpsPathException(e.getMessage()); + } catch (final PathParsingException pathParsingException) { + throw new CpsPathException(pathParsingException.getMessage()); } Collection fragmentEntities; @@ -337,11 +343,23 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService } try { return CpsPathUtil.getNormalizedXpath(xpathSource); - } catch (final PathParsingException e) { - throw new CpsPathException(e.getMessage()); + } catch (final PathParsingException pathParsingException) { + throw new CpsPathException(pathParsingException.getMessage()); } } + private static Collection getNormalizedXpaths(final Collection xpaths) { + final Collection normalizedXpaths = new HashSet<>(xpaths.size()); + for (final String xpath : xpaths) { + try { + normalizedXpaths.add(getNormalizedXpath(xpath)); + } catch (final CpsPathException cpsPathException) { + log.warn("Error parsing xpath \"{}\": {}", xpath, cpsPathException.getMessage()); + } + } + return normalizedXpaths; + } + @Override public String startSession() { return sessionManager.startSession(); @@ -450,7 +468,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService for (final FragmentEntity dataNodeFragment : fragmentEntities) { try { fragmentRepository.save(dataNodeFragment); - } catch (final StaleStateException e) { + } catch (final StaleStateException staleStateException) { failedXpaths.add(dataNodeFragment.getXpath()); } } @@ -542,15 +560,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService final AnchorEntity anchorEntity = getAnchorEntity(dataspaceName, anchorName); - final Collection deleteChecklist = new HashSet<>(xpathsToDelete.size()); - for (final String xpath : xpathsToDelete) { - try { - deleteChecklist.add(CpsPathUtil.getNormalizedXpath(xpath)); - } catch (final PathParsingException e) { - log.warn("Error parsing xpath \"{}\": {}", xpath, e.getMessage()); - } - } - + final Collection deleteChecklist = getNormalizedXpaths(xpathsToDelete); final Collection xpathsToExistingContainers = fragmentRepository.findAllXpathByAnchorAndXpathIn(anchorEntity, deleteChecklist); if (onlySupportListDeletion) { 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 303af5bc4..7d5be13a5 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 @@ -58,6 +58,17 @@ public interface FragmentRepository extends JpaRepository, return findByAnchorIdAndXpathIn(anchorEntity.getId(), xpaths.toArray(new String[0])); } + @Query(value = "SELECT * FROM fragment WHERE anchor_id = :anchorId \n" + + "AND xpath LIKE :escapedXpath||'[@%]' AND xpath NOT LIKE :escapedXpath||'[@%]/%[@%]'", + nativeQuery = true) + List findListByAnchorIdAndEscapedXpath(@Param("anchorId") long anchorId, + @Param("escapedXpath") String escapedXpath); + + default List findListByAnchorAndXpath(final AnchorEntity anchorEntity, final String xpath) { + final String escapedXpath = EscapeUtils.escapeForSqlLike(xpath); + return findListByAnchorIdAndEscapedXpath(anchorEntity.getId(), escapedXpath); + } + @Query(value = "SELECT fragment.* FROM fragment JOIN anchor ON anchor.id = fragment.anchor_id " + "WHERE dataspace_id = :dataspaceId AND xpath = ANY (:xpaths)", nativeQuery = true) List findByDataspaceIdAndXpathIn(@Param("dataspaceId") int dataspaceId, @@ -110,7 +121,7 @@ public interface FragmentRepository extends JpaRepository, boolean existsByAnchorAndXpathStartsWith(AnchorEntity anchorEntity, String xpath); - @Query("SELECT xpath FROM FragmentEntity WHERE anchor = :anchor AND parentId IS NULL") - List findAllXpathByAnchorAndParentIdIsNull(@Param("anchor") AnchorEntity anchorEntity); + @Query(value = "SELECT * FROM fragment WHERE anchor_id = :anchorId AND parent_id IS NULL", nativeQuery = true) + List findRootsByAnchorId(@Param("anchorId") long anchorId); } 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 cb554faee..c72c3046e 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 @@ -56,6 +56,7 @@ class CpsDataPersistenceServiceSpec extends Specification { def setup() { mockAnchorRepository.getByDataspaceAndName(_, _) >> anchorEntity mockFragmentRepository.prefetchDescendantsOfFragmentEntities(_, _) >> { fetchDescendantsOption, fragmentEntities -> fragmentEntities } + mockFragmentRepository.findListByAnchorAndXpath(_, [] as Set) >> [] } def 'Storing data nodes individually when batch operation fails'(){ diff --git a/docs/release-notes.rst b/docs/release-notes.rst index 6b3546142..66dde1cfb 100755 --- a/docs/release-notes.rst +++ b/docs/release-notes.rst @@ -42,7 +42,7 @@ Bug Fixes Features -------- -3.3.6 + - `CPS-1696 `_ Get Data Node to return entire List data node. Version: 3.3.5 diff --git a/integration-test/src/test/groovy/org/onap/cps/integration/functional/CpsDataServiceIntegrationSpec.groovy b/integration-test/src/test/groovy/org/onap/cps/integration/functional/CpsDataServiceIntegrationSpec.groovy index a3f14397c..6b556d3bc 100644 --- a/integration-test/src/test/groovy/org/onap/cps/integration/functional/CpsDataServiceIntegrationSpec.groovy +++ b/integration-test/src/test/groovy/org/onap/cps/integration/functional/CpsDataServiceIntegrationSpec.groovy @@ -113,23 +113,49 @@ class CpsDataServiceIntegrationSpec extends FunctionalSpecBase { restoreBookstoreDataAnchor(1) } + def 'Get whole list data' () { + def xpathForWholeList = "/bookstore/categories" + when: 'get data nodes for bookstore container' + def dataNodes = objectUnderTest.getDataNodes(FUNCTIONAL_TEST_DATASPACE_1, BOOKSTORE_ANCHOR_1, xpathForWholeList, FetchDescendantsOption.INCLUDE_ALL_DESCENDANTS) + then: 'the tree consist ouf of #expectNumberOfDataNodes data nodes' + assert dataNodes.size() == 5 + and: 'each datanode contains the list node xpath partially in its xpath' + dataNodes.each {dataNode -> + assert dataNode.xpath.contains(xpathForWholeList) + } + } + + def 'Read (multiple) data nodes with #scenario' () { + when: 'attempt to get data nodes using multiple valid xpaths' + def dataNodes = objectUnderTest.getDataNodesForMultipleXpaths(FUNCTIONAL_TEST_DATASPACE_1, BOOKSTORE_ANCHOR_1, xpath, OMIT_DESCENDANTS) + then: 'expected numer of data nodes are returned' + dataNodes.size() == expectedNumberOfDataNodes + where: 'the following data was used' + scenario | xpath | expectedNumberOfDataNodes + 'container-node xpath' | ['/bookstore'] | 1 + 'list-item' | ['/bookstore/categories[@code=1]'] | 1 + 'parent-list xpath' | ['/bookstore/categories'] | 5 + 'child-list xpath' | ['/bookstore/categories[@code=1]/books'] | 2 + 'both parent and child list xpath' | ['/bookstore/categories', '/bookstore/categories[@code=1]/books'] | 7 + } + def 'Add and Delete a (container) data node using #scenario.'() { - when: 'the new datanode is saved' - objectUnderTest.saveData(FUNCTIONAL_TEST_DATASPACE_1, BOOKSTORE_ANCHOR_1 , parentXpath, json, now) - then: 'it can be retrieved by its normalized xpath' - def result = objectUnderTest.getDataNodes(FUNCTIONAL_TEST_DATASPACE_1, BOOKSTORE_ANCHOR_1, normalizedXpathToNode, DIRECT_CHILDREN_ONLY) - assert result.size() == 1 - assert result[0].xpath == normalizedXpathToNode - and: 'there is now one extra datanode' - assert originalCountBookstoreChildNodes + 1 == countDataNodesInBookstore() - when: 'the new datanode is deleted' - objectUnderTest.deleteDataNode(FUNCTIONAL_TEST_DATASPACE_1, BOOKSTORE_ANCHOR_1, normalizedXpathToNode, now) - then: 'the original number of data nodes is restored' - assert originalCountBookstoreChildNodes == countDataNodesInBookstore() - where: - scenario | parentXpath | json || normalizedXpathToNode - 'normalized parent xpath' | '/bookstore' | '{"webinfo": {"domain-name":"ourbookstore.com", "contact-email":"info@ourbookstore.com" }}' || "/bookstore/webinfo" - 'non-normalized parent xpath' | '/bookstore/categories[ @code="1"]' | '{"books": {"title":"new" }}' || "/bookstore/categories[@code='1']/books[@title='new']" + when: 'the new datanode is saved' + objectUnderTest.saveData(FUNCTIONAL_TEST_DATASPACE_1, BOOKSTORE_ANCHOR_1 , parentXpath, json, now) + then: 'it can be retrieved by its normalized xpath' + def result = objectUnderTest.getDataNodes(FUNCTIONAL_TEST_DATASPACE_1, BOOKSTORE_ANCHOR_1, normalizedXpathToNode, DIRECT_CHILDREN_ONLY) + assert result.size() == 1 + assert result[0].xpath == normalizedXpathToNode + and: 'there is now one extra datanode' + assert originalCountBookstoreChildNodes + 1 == countDataNodesInBookstore() + when: 'the new datanode is deleted' + objectUnderTest.deleteDataNode(FUNCTIONAL_TEST_DATASPACE_1, BOOKSTORE_ANCHOR_1, normalizedXpathToNode, now) + then: 'the original number of data nodes is restored' + assert originalCountBookstoreChildNodes == countDataNodesInBookstore() + where: + scenario | parentXpath | json || normalizedXpathToNode + 'normalized parent xpath' | '/bookstore' | '{"webinfo": {"domain-name":"ourbookstore.com", "contact-email":"info@ourbookstore.com" }}' || "/bookstore/webinfo" + 'non-normalized parent xpath' | '/bookstore/categories[ @code="1"]' | '{"books": {"title":"new" }}' || "/bookstore/categories[@code='1']/books[@title='new']" } def 'Attempt to create a top level data node using root.'() { diff --git a/integration-test/src/test/groovy/org/onap/cps/integration/performance/cps/GetPerfTest.groovy b/integration-test/src/test/groovy/org/onap/cps/integration/performance/cps/GetPerfTest.groovy index eee87dd7c..8323ef9f8 100644 --- a/integration-test/src/test/groovy/org/onap/cps/integration/performance/cps/GetPerfTest.groovy +++ b/integration-test/src/test/groovy/org/onap/cps/integration/performance/cps/GetPerfTest.groovy @@ -74,11 +74,12 @@ class GetPerfTest extends CpsPerfTestBase { then: 'all data is read within #durationLimit ms' recordAndAssertPerformance("Read datatrees using ${scenario}", durationLimit, durationInMillis) where: 'the following xpaths are used' - scenario | anchorPrefix | xpath || durationLimit | expectedNumberOfDataNodes - 'bookstore root' | 'bookstore' | '/' || 200 | 78 - 'bookstore top element' | 'bookstore' | '/bookstore' || 200 | 78 - 'openroadm root' | 'openroadm' | '/' || 600 | 1 + 50 * 86 - 'openroadm top element' | 'openroadm' | '/openroadm-devices' || 600 | 1 + 50 * 86 + scenario | anchorPrefix | xpath || durationLimit | expectedNumberOfDataNodes + 'bookstore root' | 'bookstore' | '/' || 200 | 78 + 'bookstore top element' | 'bookstore' | '/bookstore' || 200 | 78 + 'openroadm root' | 'openroadm' | '/' || 600 | 1 + 50 * 86 + 'openroadm top element' | 'openroadm' | '/openroadm-devices' || 600 | 1 + 50 * 86 + 'openroadm whole list' | 'openroadm' | '/openroadm-devices/openroadm-device' || 600 | 50 * 86 } } -- 2.16.6