From a79c9f1bdf335843c29a425da53c15b5e353e5a3 Mon Sep 17 00:00:00 2001 From: DylanB95EST Date: Fri, 29 Oct 2021 17:33:06 +0100 Subject: Clean Up Code around List Nodes Make sure code refers clearly to List (whole) nodes or List elements incl. method names, parameter names, test descriptions etc. Issue-ID: CPS-756 Change-Id: Ic9dae6565c0e84c1ba4c2d6e891d3ea307f589da Signed-off-by: DylanB95EST --- .../spi/impl/CpsDataPersistenceServiceImpl.java | 170 ++++++++++------- ...CpsDataPersistenceServiceIntegrationSpec.groovy | 209 ++++++++++++--------- cps-ri/src/test/resources/data/fragment.sql | 7 +- 3 files changed, 221 insertions(+), 165 deletions(-) (limited to 'cps-ri') 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 2397d3179..8dc6c2f69 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 @@ -91,7 +91,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService private static final Gson GSON = new GsonBuilder().create(); private static final String REG_EX_FOR_OPTIONAL_LIST_INDEX = "(\\[@[\\s\\S]+?]){0,1})"; - private static final String REG_EX_FOR_LIST_NODE_KEY = "\\[(\\@([^/]*?)){0,99}( and)*\\]$"; + private static final String REG_EX_FOR_LIST_ELEMENT_KEY_PREDICATE = "\\[(\\@([^/]*?)){0,99}( and)*\\]$"; @Override public void addChildDataNode(final String dataspaceName, final String anchorName, final String parentXpath, @@ -108,7 +108,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService } @Override - public void addListDataNodes(final String dataspaceName, final String anchorName, final String parentNodeXpath, + public void addListElements(final String dataspaceName, final String anchorName, final String parentNodeXpath, final Collection dataNodes) { final FragmentEntity parentFragment = getFragmentByXpath(dataspaceName, anchorName, parentNodeXpath); final List newFragmentEntities = @@ -296,23 +296,23 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService } private static void replaceDataNodeTree(final FragmentEntity existingFragmentEntity, - final DataNode submittedDataNode) { + final DataNode newDataNode) { - existingFragmentEntity.setAttributes(GSON.toJson(submittedDataNode.getLeaves())); + existingFragmentEntity.setAttributes(GSON.toJson(newDataNode.getLeaves())); final Map existingChildrenByXpath = existingFragmentEntity.getChildFragments() .stream().collect(Collectors.toMap(FragmentEntity::getXpath, childFragmentEntity -> childFragmentEntity)); final Collection updatedChildFragments = new HashSet<>(); - for (final DataNode submittedChildDataNode : submittedDataNode.getChildDataNodes()) { + for (final DataNode newDataNodeChild : newDataNode.getChildDataNodes()) { final FragmentEntity childFragment; - if (isNewDataNode(submittedChildDataNode, existingChildrenByXpath)) { + if (isNewDataNode(newDataNodeChild, existingChildrenByXpath)) { childFragment = convertToFragmentWithAllDescendants( - existingFragmentEntity.getDataspace(), existingFragmentEntity.getAnchor(), submittedChildDataNode); + existingFragmentEntity.getDataspace(), existingFragmentEntity.getAnchor(), newDataNodeChild); } else { - childFragment = existingChildrenByXpath.get(submittedChildDataNode.getXpath()); - replaceDataNodeTree(childFragment, submittedChildDataNode); + childFragment = existingChildrenByXpath.get(newDataNodeChild.getXpath()); + replaceDataNodeTree(childFragment, newDataNodeChild); } updatedChildFragments.add(childFragment); } @@ -322,19 +322,19 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService @Override @Transactional - public void replaceListDataNodes(final String dataspaceName, final String anchorName, final String parentNodeXpath, - final Collection replacementDataNodes) { + public void replaceListContent(final String dataspaceName, final String anchorName, final String parentNodeXpath, + final Collection newListElements) { final FragmentEntity parentEntity = getFragmentByXpath(dataspaceName, anchorName, parentNodeXpath); - final String listNodeXpathPrefix = getListNodeXpathPrefix(replacementDataNodes); + final String listElementXpathPrefix = getListElementXpathPrefix(newListElements); final Map existingListElementFragmentEntitiesByXPath = - extractListElementFragmentEntitiesByXPath(parentEntity.getChildFragments(), listNodeXpathPrefix); - removeExistingListElements(parentEntity.getChildFragments(), existingListElementFragmentEntitiesByXPath); + extractListElementFragmentEntitiesByXPath(parentEntity.getChildFragments(), listElementXpathPrefix); + deleteListElements(parentEntity.getChildFragments(), existingListElementFragmentEntitiesByXPath); final Set updatedChildFragmentEntities = new HashSet<>(); - for (final DataNode replacementDataNode : replacementDataNodes) { - final FragmentEntity existingListNodeElementEntity = - existingListElementFragmentEntitiesByXPath.get(replacementDataNode.getXpath()); - final FragmentEntity entityToBeAdded = getFragmentForReplacement(parentEntity, replacementDataNode, - existingListNodeElementEntity); + for (final DataNode newListElement : newListElements) { + final FragmentEntity existingListElementEntity = + existingListElementFragmentEntitiesByXPath.get(newListElement.getXpath()); + final FragmentEntity entityToBeAdded = getFragmentForReplacement(parentEntity, newListElement, + existingListElementEntity); updatedChildFragmentEntities.add(entityToBeAdded); } @@ -342,85 +342,111 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService fragmentRepository.save(parentEntity); } - private static void removeExistingListElements( + + @Override + @Transactional + public void deleteListDataNode(final String dataspaceName, final String anchorName, + final String targetXpath) { + deleteDataNode(dataspaceName, anchorName, targetXpath, true); + } + + @Override + @Transactional + public void deleteDataNode(final String dataspaceName, final String anchorName, final String targetXpath) { + deleteDataNode(dataspaceName, anchorName, targetXpath, false); + } + + private void deleteDataNode(final String dataspaceName, final String anchorName, final String targetXpath, + final boolean onlySupportListNodeDeletion) { + final String parentNodeXpath = targetXpath.substring(0, targetXpath.lastIndexOf('/')); + final FragmentEntity parentFragmentEntity = getFragmentByXpath(dataspaceName, anchorName, parentNodeXpath); + final String lastXpathElement = targetXpath.substring(targetXpath.lastIndexOf('/')); + final boolean isListElement = Pattern.compile(REG_EX_FOR_LIST_ELEMENT_KEY_PREDICATE) + .matcher(lastXpathElement).find(); + boolean targetExist; + if (isListElement) { + targetExist = deleteDataNode(parentFragmentEntity, targetXpath); + } else { + targetExist = deleteAllListElements(parentFragmentEntity, targetXpath); + final boolean tryToDeleteDataNode = !targetExist && !onlySupportListNodeDeletion; + if (tryToDeleteDataNode) { + targetExist = deleteDataNode(parentFragmentEntity, targetXpath); + } + } + if (!targetExist) { + final String additionalInformation = onlySupportListNodeDeletion + ? "The target is probably not a List." : ""; + throw new DataNodeNotFoundException(parentFragmentEntity.getDataspace().getName(), + parentFragmentEntity.getAnchor().getName(), targetXpath, additionalInformation); + } + } + + private boolean deleteDataNode(final FragmentEntity parentFragmentEntity, final String targetXpath) { + if (parentFragmentEntity.getChildFragments() + .removeIf(fragment -> fragment.getXpath().equals(targetXpath))) { + fragmentRepository.save(parentFragmentEntity); + return true; + } + return false; + } + + + private boolean deleteAllListElements(final FragmentEntity parentFragmentEntity, final String listXpath) { + final String deleteTargetXpathPrefix = listXpath + "["; + if (parentFragmentEntity.getChildFragments() + .removeIf(fragment -> fragment.getXpath().startsWith(deleteTargetXpathPrefix))) { + fragmentRepository.save(parentFragmentEntity); + return true; + } + return false; + } + + private static void deleteListElements( final Collection fragmentEntities, final Map existingListElementFragmentEntitiesByXPath) { fragmentEntities.removeAll(existingListElementFragmentEntitiesByXPath.values()); } - private static String getListNodeXpathPrefix(final Collection replacementDataNodes) { - final String firstChildNodeXpath = replacementDataNodes.iterator().next().getXpath(); + private static String getListElementXpathPrefix(final Collection newListElements) { + final String firstChildNodeXpath = newListElements.iterator().next().getXpath(); return firstChildNodeXpath.substring(0, firstChildNodeXpath.lastIndexOf("[") + 1); } private static FragmentEntity getFragmentForReplacement(final FragmentEntity parentEntity, - final DataNode replacementDataNode, - final FragmentEntity existingListNodeElementEntity) { - if (existingListNodeElementEntity == null) { + final DataNode newListElement, + final FragmentEntity existingListElementEntity) { + if (existingListElementEntity == null) { return convertToFragmentWithAllDescendants( - parentEntity.getDataspace(), parentEntity.getAnchor(), replacementDataNode); + parentEntity.getDataspace(), parentEntity.getAnchor(), newListElement); } - if (replacementDataNode.getChildDataNodes().isEmpty()) { - copyAttributesFromReplacementDataNode(existingListNodeElementEntity, replacementDataNode); - existingListNodeElementEntity.getChildFragments().clear(); + if (newListElement.getChildDataNodes().isEmpty()) { + copyAttributesFromNewListElement(existingListElementEntity, newListElement); + existingListElementEntity.getChildFragments().clear(); } else { - replaceDataNodeTree(existingListNodeElementEntity, replacementDataNode); + replaceDataNodeTree(existingListElementEntity, newListElement); } - return existingListNodeElementEntity; + return existingListElementEntity; } private static boolean isNewDataNode(final DataNode replacementDataNode, - final Map existingListNodeElementsByXpath) { - return !existingListNodeElementsByXpath.containsKey(replacementDataNode.getXpath()); + final Map existingListElementsByXpath) { + return !existingListElementsByXpath.containsKey(replacementDataNode.getXpath()); } - private static void copyAttributesFromReplacementDataNode(final FragmentEntity existingListNodeElementEntity, - final DataNode replacementDataNode) { + private static void copyAttributesFromNewListElement(final FragmentEntity existingListElementEntity, + final DataNode newListElement) { final FragmentEntity replacementFragmentEntity = - FragmentEntity.builder().attributes(GSON.toJson(replacementDataNode.getLeaves())).build(); - existingListNodeElementEntity.setAttributes(replacementFragmentEntity.getAttributes()); + FragmentEntity.builder().attributes(GSON.toJson(newListElement.getLeaves())).build(); + existingListElementEntity.setAttributes(replacementFragmentEntity.getAttributes()); } private static Map extractListElementFragmentEntitiesByXPath( - final Set childEntities, final String listNodeXpathPrefix) { + final Set childEntities, final String listElementXpathPrefix) { return childEntities.stream() - .filter(fragmentEntity -> fragmentEntity.getXpath().startsWith(listNodeXpathPrefix)) + .filter(fragmentEntity -> fragmentEntity.getXpath().startsWith(listElementXpathPrefix)) .collect(Collectors.toMap(FragmentEntity::getXpath, fragmentEntity -> fragmentEntity)); } - @Override - @Transactional - public void deleteListDataNodes(final String dataspaceName, final String anchorName, final String listNodeXpath) { - final String parentNodeXpath = listNodeXpath.substring(0, listNodeXpath.lastIndexOf('/')); - final FragmentEntity parentEntity = getFragmentByXpath(dataspaceName, anchorName, parentNodeXpath); - final String descendantNode = listNodeXpath.substring(listNodeXpath.lastIndexOf('/')); - final Matcher descendantNodeHasListNodeKey = Pattern.compile(REG_EX_FOR_LIST_NODE_KEY).matcher(descendantNode); - - final boolean xpathPointsToAValidChildNodeWithKey = parentEntity.getChildFragments().stream().anyMatch( - fragment -> fragment.getXpath().equals(listNodeXpath)); - - final boolean xpathPointsToAValidChildNodeWithoutKey = parentEntity.getChildFragments().stream().anyMatch( - fragment -> fragment.getXpath().replaceAll(REG_EX_FOR_LIST_NODE_KEY, "").equals(listNodeXpath)); - - if ((descendantNodeHasListNodeKey.find() && xpathPointsToAValidChildNodeWithKey) - || - (!descendantNodeHasListNodeKey.find() && xpathPointsToAValidChildNodeWithoutKey)) { - removeListNodeDescendants(parentEntity, listNodeXpath); - } else { - throw new DataNodeNotFoundException(parentEntity.getDataspace().getName(), - parentEntity.getAnchor().getName(), listNodeXpath); - } - } - - private void removeListNodeDescendants(final FragmentEntity parentFragmentEntity, final String listNodeXpath) { - final Matcher descendantNodeHasListNodeKey = Pattern.compile(REG_EX_FOR_LIST_NODE_KEY).matcher(listNodeXpath); - final String listNodeXpathPrefix = listNodeXpath + (descendantNodeHasListNodeKey.find() ? "" : "["); - if (parentFragmentEntity.getChildFragments() - .removeIf(fragment -> fragment.getXpath().startsWith(listNodeXpathPrefix))) { - fragmentRepository.save(parentFragmentEntity); - } - } - private static boolean isRootXpath(final String xpath) { return "/".equals(xpath) || "".equals(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 144b18b53..85e1155cf 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 @@ -32,7 +32,6 @@ import org.onap.cps.spi.exceptions.DataNodeNotFoundException import org.onap.cps.spi.exceptions.DataspaceNotFoundException import org.onap.cps.spi.model.DataNode import org.onap.cps.spi.model.DataNodeBuilder -import org.spockframework.util.CollectionUtil import org.springframework.beans.factory.annotation.Autowired import org.springframework.test.context.jdbc.Sql @@ -157,20 +156,20 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase { } @Sql([CLEAR_DATA, SET_DATA]) - def 'Add list-node fragment with multiple elements including an element with a child datanode.'() { + def 'Add multiple list elements including an element with a child datanode.'() { given: 'two new data nodes for an existing list' - def listNodeXpaths = ['/parent-201/child-204[@key="B"]', '/parent-201/child-204[@key="C"]'] - def listNodeCollection = buildDataNodeCollection(listNodeXpaths) + def listElementXpaths = ['/parent-201/child-204[@key="B"]', '/parent-201/child-204[@key="C"]'] + def listElements = toDataNodes(listElementXpaths) and: 'a child node for one of the new data nodes' def childDataNode = buildDataNode('/parent-201/child-204[@key="C"]/grand-child-204[@key2="Z"]', [leave:'value'], []) - listNodeCollection.iterator().next().childDataNodes = [childDataNode] + listElements[0].childDataNodes = [childDataNode] when: 'the data nodes (list elements) are added to existing parent node' - objectUnderTest.addListDataNodes(DATASPACE_NAME, ANCHOR_NAME3, '/parent-201', listNodeCollection) + objectUnderTest.addListElements(DATASPACE_NAME, ANCHOR_NAME3, '/parent-201', listElements) then: 'new entries 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.getChildFragments().collect { it.getXpath() } assert allChildXpaths.size() == 5 - assert allChildXpaths.containsAll(listNodeXpaths) + assert allChildXpaths.containsAll(listElementXpaths) and: 'the child node of the new list entry is also present' def dataspaceEntity = dataspaceRepository.getByName(DATASPACE_NAME) def anchorEntity = anchorRepository.getByDataspaceAndName(dataspaceEntity, ANCHOR_NAME3) @@ -179,15 +178,15 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase { } @Sql([CLEAR_DATA, SET_DATA]) - def 'Add list-node fragment error scenario: #scenario.'() { - given: 'list node data fragment as a collection of data nodes' - def listNodeCollection = buildDataNodeCollection(listNodeXpaths) - when: 'list-node elements added to existing parent node' - objectUnderTest.addListDataNodes(DATASPACE_NAME, ANCHOR_NAME3, parentNodeXpath, listNodeCollection) + def 'Add list element error scenario: #scenario.'() { + given: 'list element as a collection of data nodes' + def listElementCollection = toDataNodes(listElementXpaths) + when: 'attempt to add list elements to parent node' + objectUnderTest.addListElements(DATASPACE_NAME, ANCHOR_NAME3, parentNodeXpath, listElementCollection) then: 'a #expectedException is thrown' thrown(expectedException) where: 'following parameters were used' - scenario | parentNodeXpath | listNodeXpaths || expectedException + scenario | parentNodeXpath | listElementXpaths || expectedException 'parent node does not exist' | '/unknown' | ['irrelevant'] || DataNodeNotFoundException 'already existing fragment' | '/parent-201' | ['/parent-201/child-204[@key="A"]'] || AlreadyDefinedException @@ -385,40 +384,40 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase { } @Sql([CLEAR_DATA, SET_DATA]) - def 'Replace list-node content of #scenario.'() { - given: 'list node data fragment as a collection of data nodes' - def listNodeCollection = buildDataNodeCollection(listNodeXpaths) - when: 'list-node elements replaced within the existing parent node' - objectUnderTest.replaceListDataNodes(DATASPACE_NAME, ANCHOR_NAME3, parentXpath, listNodeCollection) - then: 'child list elements are updated as expected, non-list element remains as is' - def parentFragment = fragmentRepository.getById(listNodeFragmentID) + def 'Replace list content of #scenario.'() { + given: 'list element as a collection of data nodes' + def listElementCollection = toDataNodes(listElementXpaths) + when: 'list elements are replaced within the existing parent node' + objectUnderTest.replaceListContent(DATASPACE_NAME, ANCHOR_NAME3, parentXpath, listElementCollection) + then: 'list elements are updated as expected, non-list element remains as is' + def parentFragment = fragmentRepository.getById(listElementFragmentID) def allChildXpaths = parentFragment.getChildFragments().collect { it.getXpath() } assert allChildXpaths.size() == expectedChildXpaths.size() assert allChildXpaths.containsAll(expectedChildXpaths) where: 'following parameters were used' - scenario | listNodeXpaths |parentXpath |listNodeFragmentID || expectedChildXpaths - 'existing list node with non existing key' | ['/parent-201/child-204[@key="B"]'] | '/parent-201' | LIST_DATA_NODE_PARENT201_FRAGMENT_ID || ['/parent-201/child-203', '/parent-201/child-204[@key="B"]'] - 'non existing list node with non existing key' | ['/parent-201/child-205[@key="1"]'] | '/parent-201' | LIST_DATA_NODE_PARENT201_FRAGMENT_ID || ['/parent-201/child-203', '/parent-201/child-204[@key="A"]', '/parent-201/child-204[@key="X"]', '/parent-201/child-205[@key="1"]'] - 'existing list node with 1 existing key' | ['/parent-201/child-204[@key="X"]'] | '/parent-201' | LIST_DATA_NODE_PARENT201_FRAGMENT_ID || ['/parent-201/child-203', '/parent-201/child-204[@key="X"]'] - 'existing list-node with combined keys' | ['/parent-202/child-205[@key="A"]'] | '/parent-202' | LIST_DATA_NODE_PARENT202_FRAGMENT_ID || ['/parent-202/child-206[@key="A"]', '/parent-202/child-205[@key="A"]'] - 'existing grandchild list-node' | ['/parent-200/child-202/grand-child-202[@key="E"]'] | '/parent-200/child-202' | LIST_DATA_NODE_CHILD202_FRAGMENT_ID || ['/parent-200/child-202/grand-child-202[@key="E"]'] - 'existing list node with two list nodes' | ['/parent-201/child-204[@key="new X"]', '/parent-201/child-204[@key="Y"]'] | '/parent-201' | LIST_DATA_NODE_PARENT201_FRAGMENT_ID || ['/parent-201/child-203', '/parent-201/child-204[@key="new X"]', '/parent-201/child-204[@key="Y"]'] - 'existing list node with compounded list node' | ['/parent-202/child-205[@key="A" and @key2="B"]'] | '/parent-202' | LIST_DATA_NODE_PARENT202_FRAGMENT_ID || ['/parent-202/child-206[@key="A"]', '/parent-202/child-205[@key="A" and @key2="B"]'] - 'existing list node with list node with parent with key value' | ['/parent-204[@key="L"]/child-210[@key="N"]'] | '/parent-204[@key="L"]' | LIST_DATA_NODE_PARENT204_FRAGMENT_ID || ['/parent-204[@key="L"]/child-210[@key="N"]'] + scenario | listElementXpaths | parentXpath | listElementFragmentID || expectedChildXpaths + 'existing list element with non existing key' | ['/parent-201/child-204[@key="B"]'] | '/parent-201' | LIST_DATA_NODE_PARENT201_FRAGMENT_ID || ['/parent-201/child-203', '/parent-201/child-204[@key="B"]'] + 'non existing list element with non existing key' | ['/parent-201/child-205[@key="1"]'] | '/parent-201' | LIST_DATA_NODE_PARENT201_FRAGMENT_ID || ['/parent-201/child-203', '/parent-201/child-204[@key="A"]', '/parent-201/child-204[@key="X"]', '/parent-201/child-205[@key="1"]'] + 'list element with 1 existing key' | ['/parent-201/child-204[@key="X"]'] | '/parent-201' | LIST_DATA_NODE_PARENT201_FRAGMENT_ID || ['/parent-201/child-203', '/parent-201/child-204[@key="X"]'] + 'list element with combined keys' | ['/parent-202/child-205[@key="A"]'] | '/parent-202' | LIST_DATA_NODE_PARENT202_FRAGMENT_ID || ['/parent-202/child-206[@key="A"]', '/parent-202/child-205[@key="A"]'] + 'grandchild list element' | ['/parent-200/child-202/grand-child-202[@key="E"]'] | '/parent-200/child-202' | LIST_DATA_NODE_CHILD202_FRAGMENT_ID || ['/parent-200/child-202/grand-child-202[@key="E"]'] + 'list element with two list elements' | ['/parent-201/child-204[@key="new X"]', '/parent-201/child-204[@key="Y"]'] | '/parent-201' | LIST_DATA_NODE_PARENT201_FRAGMENT_ID || ['/parent-201/child-203', '/parent-201/child-204[@key="new X"]', '/parent-201/child-204[@key="Y"]'] + 'list element with compounded list element' | ['/parent-202/child-205[@key="A" and @key2="B"]'] | '/parent-202' | LIST_DATA_NODE_PARENT202_FRAGMENT_ID || ['/parent-202/child-206[@key="A"]', '/parent-202/child-205[@key="A" and @key2="B"]'] + 'list element with list element with parent with key value' | ['/parent-204[@key="L"]/child-210[@key="N"]'] | '/parent-204[@key="L"]' | LIST_DATA_NODE_PARENT204_FRAGMENT_ID || ['/parent-204[@key="L"]/child-210[@key="N"]'] } @Sql([CLEAR_DATA, SET_DATA]) - def 'Replace list-node that has children with #scenario'() { - given: 'list node data fragment with child data node fragments' - def grandChildDataNodes = buildDataNodeCollection(grandChildXpaths) - def listNode = new DataNodeBuilder().withXpath(childXpath).withChildDataNodes(grandChildDataNodes).build() - when: 'list-node elements replaced within the existing parent node' - objectUnderTest.replaceListDataNodes(DATASPACE_NAME, ANCHOR_NAME3, parentXpath, [ listNode ]) - then: 'child list elements are updated as expected with non-list elements remaining as is' - def parentFragment = fragmentRepository.getById(listNodeFragmentId) + def 'Replace list content that has #scenario'() { + given: 'list element with child list element as a collection of data nodes' + def grandChildDataNodes = toDataNodes(grandChildXpaths) + def listElementCollection = new DataNodeBuilder().withXpath(childXpath).withChildDataNodes(grandChildDataNodes).build() + when: 'list elements replaced within the existing parent node' + objectUnderTest.replaceListContent(DATASPACE_NAME, ANCHOR_NAME3, parentXpath, [listElementCollection ]) + then: 'list elements are updated as expected with non-list elements remaining as is' + def parentFragment = fragmentRepository.getById(listElementFragmentId) def allChildXpaths = parentFragment.getChildFragments().collect { it.getXpath() } - assert allChildXpaths.size() == expectedChildXpaths.size() - assert allChildXpaths.containsAll(expectedChildXpaths) + assert allChildXpaths.size() == expectedRemainingChildXpaths.size() + assert allChildXpaths.containsAll(expectedRemainingChildXpaths) and: 'grandchild list elements are updated as expected' def allGrandChildXpaths = parentFragment.getChildFragments().collect(){ it.getChildFragments().collect(){ @@ -429,21 +428,20 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase { assert grandChildXpathsToList.size() == expectedGrandChildXpaths.size() assert grandChildXpathsToList.containsAll(expectedGrandChildXpaths) where: 'the following parameters are used' - scenario | parentXpath | childXpath | grandChildXpaths | expectedChildXpaths | listNodeFragmentId - 'existing grandchild of list node' | '/parent-203' | '/parent-203/child-204[@key="X"]' | ['/parent-203/child-204/grandchild[@key="2"]'] | ['/parent-203/child-203', '/parent-203/child-204[@key="X"]'] | LIST_DATA_NODE_PARENT203_FRAGMENT_ID - 'existing grandchild of list node with two new nodes' | '/parent-203' | '/parent-203/child-204[@key="X"]' | ['/parent-203/child-204/grandchild[@key="2"]' , '/parent-203/child-204/grandchild[@key="3"]'] | ['/parent-203/child-203', '/parent-203/child-204[@key="X"]'] | LIST_DATA_NODE_PARENT203_FRAGMENT_ID - 'existing grandchild with compound list node' | '/parent-205' | '/parent-205/child-205[@key="X"]' | ['/parent-205/child-205/grand-child-206[@key="Y" and @key2="Z"]'] | ['/parent-205/child-205', '/parent-205/child-205[@key="X"]'] | LIST_DATA_NODE_PARENT205_FRAGMENT_ID - 'two existing list node with a new node' | '/parent-205' | '/parent-205/child-205[@key="X"]' | ['/parent-205/child-205/grandchild[@key="A"]'] | ['/parent-205/child-205', '/parent-205/child-205[@key="X"]'] | LIST_DATA_NODE_PARENT205_FRAGMENT_ID + scenario | parentXpath | childXpath | grandChildXpaths | listElementFragmentId || expectedRemainingChildXpaths + 'grandchild of list' | '/parent-203' | '/parent-203/child-204[@key="X"]' | ['/parent-203/child-204/grandchild[@key="2"]'] | LIST_DATA_NODE_PARENT203_FRAGMENT_ID || ['/parent-203/child-203', '/parent-203/child-204[@key="X"]'] + 'grandchild of list with two new element' | '/parent-203' | '/parent-203/child-204[@key="X"]' | ['/parent-203/child-204/grandchild[@key="2"]' , '/parent-203/child-204/grandchild[@key="3"]'] | LIST_DATA_NODE_PARENT203_FRAGMENT_ID || ['/parent-203/child-203', '/parent-203/child-204[@key="X"]'] + 'grandchild with compound list elements' | '/parent-205' | '/parent-205/child-205[@key="X"]' | ['/parent-205/child-205/grand-child-206[@key="Y" and @key2="Z"]'] | LIST_DATA_NODE_PARENT205_FRAGMENT_ID || ['/parent-205/child-205', '/parent-205/child-205[@key="X"]'] } @Sql([CLEAR_DATA, SET_DATA]) - def 'Replace list-node content of #scenario with grandchildren.'() { - given: 'list node data fragment as a collection of data nodes' - def listNodeCollection = buildDataNodeCollection(listNodeXpaths) - when: 'list-node elements replaced within the existing parent node' - objectUnderTest.replaceListDataNodes(DATASPACE_NAME, ANCHOR_NAME3, parentXpath, listNodeCollection) + def 'Replace list content of #scenario with grandchildren.'() { + given: 'list element as a collection of data nodes' + def listElementCollection = toDataNodes(listElementXpaths) + when: 'list elements are replaced within the existing parent node' + objectUnderTest.replaceListContent(DATASPACE_NAME, ANCHOR_NAME3, parentXpath, listElementCollection) then: 'child list elements are updated as expected with non-list elements remaining as is' - def parentFragment = fragmentRepository.getById(listNodeFragmentID) + def parentFragment = fragmentRepository.getById(listElementFragmentID) def allChildXpaths = parentFragment.getChildFragments().collect { it.getXpath() } assert allChildXpaths.size() == expectedChildXpaths.size() assert allChildXpaths.containsAll(expectedChildXpaths) @@ -455,67 +453,94 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase { assert allGrandChildXpaths.size() == expectedGrandChildXpaths.size() assert allGrandChildXpaths.containsAll(expectedGrandChildXpaths) where: 'following parameters were used' - scenario | listNodeXpaths | parentXpath | listNodeFragmentID || expectedChildXpaths | expectedGrandChildXpaths - 'existing list node with existing keys' | ['/parent-203/child-204[@key="X"]'] | '/parent-203' | LIST_DATA_NODE_PARENT203_FRAGMENT_ID || ['/parent-203/child-203', '/parent-203/child-204[@key="X"]'] | [] - 'non existing list node with existing keys' | ['/parent-203/child-204[@key="V"]'] | '/parent-203' | LIST_DATA_NODE_PARENT203_FRAGMENT_ID || ['/parent-203/child-203', '/parent-203/child-204[@key="V"]'] | [] + scenario | listElementXpaths | parentXpath | listElementFragmentID || expectedChildXpaths | expectedGrandChildXpaths + 'existing list element with existing keys' | ['/parent-203/child-204[@key="X"]'] | '/parent-203' | LIST_DATA_NODE_PARENT203_FRAGMENT_ID || ['/parent-203/child-203', '/parent-203/child-204[@key="X"]'] | [] + 'non existing list element with existing keys' | ['/parent-203/child-204[@key="V"]'] | '/parent-203' | LIST_DATA_NODE_PARENT203_FRAGMENT_ID || ['/parent-203/child-203', '/parent-203/child-204[@key="V"]'] | [] } @Sql([CLEAR_DATA, SET_DATA]) - def 'Replace list-node fragment error scenario: #scenario.'() { - given: 'list node data fragment as a collection of data nodes' - def listNodeCollection = buildDataNodeCollection(listNodeXpaths) - when: 'list-node elements were replaced under existing parent node' - objectUnderTest.replaceListDataNodes(DATASPACE_NAME, ANCHOR_NAME3, parentNodeXpath, listNodeCollection) + def 'Replace content error scenario: #scenario.'() { + given: 'list element as a collection of data nodes' + def listElementCollection = toDataNodes(listElementXpaths) + when: 'list elements were replaced under existing parent node' + objectUnderTest.replaceListContent(DATASPACE_NAME, ANCHOR_NAME3, parentNodeXpath, listElementCollection) then: 'a #expectedException is thrown' thrown(expectedException) where: 'following parameters were used' - scenario | parentNodeXpath | listNodeXpaths || expectedException + scenario | parentNodeXpath | listElementXpaths || expectedException 'parent node does not exist' | '/unknown' | ['irrelevant'] || DataNodeNotFoundException } @Sql([CLEAR_DATA, SET_DATA]) - def 'Delete list-node content of #scenario.'() { - given: 'list node data fragments are present in database' - when: 'list-node elements deleted within the existing parent node' - objectUnderTest.deleteListDataNodes(DATASPACE_NAME, ANCHOR_NAME3, listNodeXpaths) - then: 'child list elements are removed as expected, non-list element remains as is' - def parentFragment = fragmentRepository.getById(listNodeFragmentID) - def allChildXpaths = parentFragment.getChildFragments().collect { it.getXpath() } - assert allChildXpaths.size() == expectedChildXpaths.size() - assert allChildXpaths.containsAll(expectedChildXpaths) + def 'Delete list scenario: #scenario.'() { + when: 'deleting list is executed for: #scenario.' + objectUnderTest.deleteListDataNode(DATASPACE_NAME, ANCHOR_NAME3, targetXpaths) + then: 'only the expected children remain' + def parentFragment = fragmentRepository.getById(parentFragmentId) + def remainingChildXpaths = parentFragment.getChildFragments().collect { it.getXpath() } + assert remainingChildXpaths.size() == expectedRemainingChildXpaths.size() + assert remainingChildXpaths.containsAll(expectedRemainingChildXpaths) where: 'following parameters were used' - scenario | listNodeXpaths | listNodeFragmentID || expectedChildXpaths - 'existing list-node with key' | '/parent-203/child-204[@key="A"]' | LIST_DATA_NODE_PARENT203_FRAGMENT_ID || ['/parent-203/child-203', '/parent-203/child-204[@key="X"]'] - 'existing list-node with key' | '/parent-203/child-204[@key="X"]' | LIST_DATA_NODE_PARENT203_FRAGMENT_ID || ['/parent-203/child-203', '/parent-203/child-204[@key="A"]'] - 'existing grand-child list node with keys' | '/parent-203/child-204[@key="X"]/grand-child-204[@key2="Y"]' | LIST_DATA_NODE_PARENT203_FRAGMENT_ID || ['/parent-203/child-203', '/parent-203/child-204[@key="X"]', '/parent-203/child-204[@key="A"]'] - 'existing list-node with combined keys' | '/parent-202/child-205[@key="A" and @key2="B"]' | LIST_DATA_NODE_PARENT202_FRAGMENT_ID || ['/parent-202/child-206[@key="A"]'] - 'existing node with list node variants to delete' | '/parent-203/child-204' | LIST_DATA_NODE_PARENT203_FRAGMENT_ID || ['/parent-203/child-203'] - 'existing grandchild list-node' | '/parent-200/child-202/grand-child-202[@key="D"]' | LIST_DATA_NODE_CHILD202_FRAGMENT_ID || [] + scenario | targetXpaths | parentFragmentId || expectedRemainingChildXpaths + 'list element with key' | '/parent-203/child-204[@key="A"]' | LIST_DATA_NODE_PARENT203_FRAGMENT_ID || ['/parent-203/child-203', '/parent-203/child-204[@key="X"]'] + 'list element with combined keys' | '/parent-202/child-205[@key="A" and @key2="B"]' | LIST_DATA_NODE_PARENT202_FRAGMENT_ID || ['/parent-202/child-206[@key="A"]'] + 'whole list' | '/parent-203/child-204' | LIST_DATA_NODE_PARENT203_FRAGMENT_ID || ['/parent-203/child-203'] + 'list element under list element' | '/parent-203/child-204[@key="X"]/grand-child-204[@key2="Y"]' | LIST_DATA_NODE_PARENT203_FRAGMENT_ID || ['/parent-203/child-203', '/parent-203/child-204[@key="X"]', '/parent-203/child-204[@key="A"]'] } @Sql([CLEAR_DATA, SET_DATA]) - def 'Delete list-node fragment error scenario: #scenario.'() { - given: 'list node data fragments are present in database' - when: 'list-node elements are deleted under existing parent node' - objectUnderTest.deleteListDataNodes(DATASPACE_NAME, ANCHOR_NAME3, listNodeXpaths) - then: 'a #expectedException is thrown' - thrown(expectedException) + def 'Delete list error scenario: #scenario.'() { + when: 'attempting to delete scenario: #scenario.' + objectUnderTest.deleteListDataNode(DATASPACE_NAME, ANCHOR_NAME3, targetXpaths) + then: 'a DataNodeNotFoundException is thrown' + thrown(DataNodeNotFoundException) where: 'following parameters were used' - scenario | listNodeXpaths || expectedException - 'list parent node does not exist' | '/unknown/unknown' || DataNodeNotFoundException - 'list child nodes do not exist' | '/parent-200/unknown' || DataNodeNotFoundException - 'list child nodes with key does not exist' | '/parent-200/unknown[@key="C"]' || DataNodeNotFoundException - 'list grandchild nodes parent does not exist' | '/parent-200/unknown/unknown' || DataNodeNotFoundException - 'non-existing parent with existing list-node' | '/unknown/child-204' || DataNodeNotFoundException - 'non-existing parent with existing list-node & key' | '/unknown/child-204[@key="A"]' || DataNodeNotFoundException - 'valid with non existing key' | '/parent-200/child-202/grand-child-202[@key="A"]' || DataNodeNotFoundException - 'child list node without key' | '/parent-200/child-204/grand-child-204' || DataNodeNotFoundException - 'valid list node with invalid key' | '/parent-203/child-204[@key="C"]' || DataNodeNotFoundException + scenario | targetXpaths + 'whole list, parent node does not exist' | '/unknown/some-child' + 'list element, parent node does not exist' | '/unknown/child-204[@key="A"]' + 'whole list does not exist' | '/parent-200/unknown' + 'list element, list does not exist' | '/parent-200/unknown[@key="C"]' + 'list element, element does not exist' | '/parent-203/child-204[@key="C"]' + 'valid datanode but not a list' | '/parent-200/child-202' + } + + @Sql([CLEAR_DATA, SET_DATA]) + def 'Confirm deletion of #scenario.'() { + given: 'a valid data node' + def dataNode + def dataNodeXpath + when: 'data nodes are deleted' + objectUnderTest.deleteDataNode(DATASPACE_NAME, ANCHOR_NAME3, xpathForDeletion) + then: 'verify data nodes are removed' + try { + dataNode = objectUnderTest.getDataNode(DATASPACE_NAME, ANCHOR_NAME3, getDataNodesXpaths, INCLUDE_ALL_DESCENDANTS) + dataNodeXpath = dataNode.getXpath() + assert dataNodeXpath == expectedXpaths + } catch (DataNodeNotFoundException) { + assert dataNodeXpath == expectedXpaths + } + where: 'following parameters were used' + scenario | xpathForDeletion | getDataNodesXpaths || expectedXpaths + 'child of target' | '/parent-206/child-206' | '/parent-206/child-206' || null + 'child data node, parent still exists' | '/parent-206/child-206' | '/parent-206' || '/parent-206' + 'list element' | '/parent-206/child-206/grand-child-206[@key="A"]' | '/parent-206/child-206/grand-child-206[@key="A"]' || null + 'list element, sibling still exists' | '/parent-206/child-206/grand-child-206[@key="A"]' | '/parent-206/child-206/grand-child-206[@key="X"]' || '/parent-206/child-206/grand-child-206[@key="X"]' + } + @Sql([CLEAR_DATA, SET_DATA]) + def 'Delete data node with #scenario.'() { + when: 'data node is deleted' + objectUnderTest.deleteDataNode(DATASPACE_NAME, ANCHOR_NAME3, datanodeXpath) + then: 'a #expectedException is thrown' + thrown(DataNodeNotFoundException) + where: 'the following parameters were used' + scenario | datanodeXpath + 'valid data node, non existent child node' | '/parent-203/child-non-existent' + 'invalid list element' | '/parent-206/child-206/grand-child-206@key="A"]' } - static Collection buildDataNodeCollection(xpaths) { + static Collection toDataNodes(xpaths) { return xpaths.collect { new DataNodeBuilder().withXpath(it).build() } } diff --git a/cps-ri/src/test/resources/data/fragment.sql b/cps-ri/src/test/resources/data/fragment.sql index f6e887e01..5d3a32832 100755 --- a/cps-ri/src/test/resources/data/fragment.sql +++ b/cps-ri/src/test/resources/data/fragment.sql @@ -68,4 +68,9 @@ INSERT INTO FRAGMENT (ID, DATASPACE_ID, ANCHOR_ID, PARENT_ID, XPATH, ATTRIBUTES) (4222, 1001, 3003, 4221, '/parent-205/child-205', '{}'), (4223, 1001, 3003, 4221, '/parent-205/child-205[@key="X"]', '{"key": "X"}'), (4224, 1001, 3003, 4223, '/parent-205/child-205[@key="X"]/grand-child-206[@key="Y"]', '{"key": "Y", "key2": "Z"}'), - (4225, 1001, 3003, 4223, '/parent-205/child-205[@key="X"]/grand-child-206[@key="Y" and @key2="Z"]', '{"key": "Y", "key2": "Z"}'); \ No newline at end of file + (4225, 1001, 3003, 4223, '/parent-205/child-205[@key="X"]/grand-child-206[@key="Y" and @key2="Z"]', '{"key": "Y", "key2": "Z"}'), + (4226, 1001, 3003, null, '/parent-206', '{"leaf-value": "original"}'), + (4227, 1001, 3003, 4226, '/parent-206/child-206', '{}'), + (4228, 1001, 3003, 4227, '/parent-206/child-206/grand-child-206', '{}'), + (4229, 1001, 3003, 4227, '/parent-206/child-206/grand-child-206[@key="A"]', '{"key": "A"}'), + (4230, 1001, 3003, 4227, '/parent-206/child-206/grand-child-206[@key="X"]', '{"key": "X"}'); \ No newline at end of file -- cgit 1.2.3-korg