From b8b29c97ee64b395bceb1e100eaf2a6bf115871f Mon Sep 17 00:00:00 2001 From: danielhanrahan Date: Tue, 7 Mar 2023 20:35:14 +0000 Subject: [PATCH] Reduce dataspace/anchor lookups in CpsDataPersistenceService - Remove unneeded calls to DataspaceRepository::getByName - Remove unneeded calls to AnchorRepository::getByDataspaceAndName - Refactor FragmentRepository Issue-ID: CPS-1536 Signed-off-by: danielhanrahan Change-Id: I2121f6247070ee4a7c000e06ec66a6278b758540 --- .../spi/impl/CpsDataPersistenceServiceImpl.java | 167 +++++++++------------ .../cps/spi/repository/FragmentRepository.java | 61 ++------ ...CpsDataPersistenceServiceIntegrationSpec.groovy | 43 ++---- .../spi/impl/CpsDataPersistenceServiceSpec.groovy | 35 ++--- 4 files changed, 111 insertions(+), 195 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 916baa8c2..e3be0137d 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 @@ -80,88 +80,83 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService private final JsonObjectMapper jsonObjectMapper; private final SessionManager sessionManager; - private static final String REG_EX_FOR_OPTIONAL_LIST_INDEX = "(\\[@[\\s\\S]+?]){0,1})"; + private static final String REG_EX_FOR_OPTIONAL_LIST_INDEX = "(\\[@[\\s\\S]+?])?)"; @Override public void addChildDataNode(final String dataspaceName, final String anchorName, final String parentNodeXpath, final DataNode newChildDataNode) { - addNewChildDataNode(dataspaceName, anchorName, parentNodeXpath, newChildDataNode); + final AnchorEntity anchorEntity = getAnchorEntity(dataspaceName, anchorName); + addNewChildDataNode(anchorEntity, parentNodeXpath, newChildDataNode); } @Override public void addChildDataNodes(final String dataspaceName, final String anchorName, final String parentNodeXpath, final Collection dataNodes) { - addChildrenDataNodes(dataspaceName, anchorName, parentNodeXpath, dataNodes); + final AnchorEntity anchorEntity = getAnchorEntity(dataspaceName, anchorName); + addChildrenDataNodes(anchorEntity, parentNodeXpath, dataNodes); } @Override public void addListElements(final String dataspaceName, final String anchorName, final String parentNodeXpath, final Collection newListElements) { - addChildrenDataNodes(dataspaceName, anchorName, parentNodeXpath, newListElements); + final AnchorEntity anchorEntity = getAnchorEntity(dataspaceName, anchorName); + addChildrenDataNodes(anchorEntity, parentNodeXpath, newListElements); } @Override public void addMultipleLists(final String dataspaceName, final String anchorName, final String parentNodeXpath, final Collection> newLists) { + final AnchorEntity anchorEntity = getAnchorEntity(dataspaceName, anchorName); final Collection failedXpaths = new HashSet<>(); - newLists.forEach(newList -> { + for (final Collection newList : newLists) { try { - addChildrenDataNodes(dataspaceName, anchorName, parentNodeXpath, newList); + addChildrenDataNodes(anchorEntity, parentNodeXpath, newList); } catch (final AlreadyDefinedExceptionBatch e) { failedXpaths.addAll(e.getAlreadyDefinedXpaths()); } - }); - + } if (!failedXpaths.isEmpty()) { throw new AlreadyDefinedExceptionBatch(failedXpaths); } - } - private void addNewChildDataNode(final String dataspaceName, final String anchorName, - final String parentNodeXpath, final DataNode newChild) { - final FragmentEntity parentFragmentEntity = - getFragmentEntity(dataspaceName, anchorName, parentNodeXpath); - final FragmentEntity newChildAsFragmentEntity = - convertToFragmentWithAllDescendants(parentFragmentEntity.getDataspace(), - parentFragmentEntity.getAnchor(), newChild); + private void addNewChildDataNode(final AnchorEntity anchorEntity, final String parentNodeXpath, + final DataNode newChild) { + final FragmentEntity parentFragmentEntity = getFragmentEntity(anchorEntity, parentNodeXpath); + final FragmentEntity newChildAsFragmentEntity = convertToFragmentWithAllDescendants(anchorEntity, newChild); newChildAsFragmentEntity.setParentId(parentFragmentEntity.getId()); try { fragmentRepository.save(newChildAsFragmentEntity); } catch (final DataIntegrityViolationException e) { - throw AlreadyDefinedException.forDataNode(newChild.getXpath(), anchorName, e); + throw AlreadyDefinedException.forDataNode(newChild.getXpath(), anchorEntity.getName(), e); } - } - private void addChildrenDataNodes(final String dataspaceName, final String anchorName, final String parentNodeXpath, + private void addChildrenDataNodes(final AnchorEntity anchorEntity, final String parentNodeXpath, final Collection newChildren) { - final FragmentEntity parentFragmentEntity = - getFragmentEntity(dataspaceName, anchorName, parentNodeXpath); + final FragmentEntity parentFragmentEntity = getFragmentEntity(anchorEntity, parentNodeXpath); final List fragmentEntities = new ArrayList<>(newChildren.size()); try { - newChildren.forEach(newChildAsDataNode -> { + for (final DataNode newChildAsDataNode : newChildren) { final FragmentEntity newChildAsFragmentEntity = - convertToFragmentWithAllDescendants(parentFragmentEntity.getDataspace(), - parentFragmentEntity.getAnchor(), newChildAsDataNode); + convertToFragmentWithAllDescendants(anchorEntity, newChildAsDataNode); newChildAsFragmentEntity.setParentId(parentFragmentEntity.getId()); fragmentEntities.add(newChildAsFragmentEntity); - }); + } fragmentRepository.saveAll(fragmentEntities); } catch (final DataIntegrityViolationException e) { log.warn("Exception occurred : {} , While saving : {} children, retrying using individual save operations", e, fragmentEntities.size()); - retrySavingEachChildIndividually(dataspaceName, anchorName, parentNodeXpath, newChildren); + retrySavingEachChildIndividually(anchorEntity, parentNodeXpath, newChildren); } } - private void retrySavingEachChildIndividually(final String dataspaceName, final String anchorName, - final String parentNodeXpath, + private void retrySavingEachChildIndividually(final AnchorEntity anchorEntity, final String parentNodeXpath, final Collection newChildren) { final Collection failedXpaths = new HashSet<>(); for (final DataNode newChild : newChildren) { try { - addNewChildDataNode(dataspaceName, anchorName, parentNodeXpath, newChild); + addNewChildDataNode(anchorEntity, parentNodeXpath, newChild); } catch (final AlreadyDefinedException e) { failedXpaths.add(newChild.getXpath()); } @@ -179,32 +174,26 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService @Override public void storeDataNodes(final String dataspaceName, final String anchorName, final Collection dataNodes) { - final DataspaceEntity dataspaceEntity = dataspaceRepository.getByName(dataspaceName); - final AnchorEntity anchorEntity = anchorRepository.getByDataspaceAndName(dataspaceEntity, anchorName); + final AnchorEntity anchorEntity = getAnchorEntity(dataspaceName, anchorName); final List fragmentEntities = new ArrayList<>(dataNodes.size()); try { for (final DataNode dataNode: dataNodes) { - final FragmentEntity fragmentEntity = convertToFragmentWithAllDescendants(dataspaceEntity, anchorEntity, - dataNode); + final FragmentEntity fragmentEntity = convertToFragmentWithAllDescendants(anchorEntity, dataNode); fragmentEntities.add(fragmentEntity); } fragmentRepository.saveAll(fragmentEntities); } catch (final DataIntegrityViolationException exception) { log.warn("Exception occurred : {} , While saving : {} data nodes, Retrying saving data nodes individually", exception, dataNodes.size()); - storeDataNodesIndividually(dataspaceName, anchorName, dataNodes); + storeDataNodesIndividually(anchorEntity, dataNodes); } } - private void storeDataNodesIndividually(final String dataspaceName, final String anchorName, - final Collection dataNodes) { - final DataspaceEntity dataspaceEntity = dataspaceRepository.getByName(dataspaceName); - final AnchorEntity anchorEntity = anchorRepository.getByDataspaceAndName(dataspaceEntity, anchorName); + private void storeDataNodesIndividually(final AnchorEntity anchorEntity, final Collection dataNodes) { final Collection failedXpaths = new HashSet<>(); for (final DataNode dataNode: dataNodes) { try { - final FragmentEntity fragmentEntity = convertToFragmentWithAllDescendants(dataspaceEntity, anchorEntity, - dataNode); + final FragmentEntity fragmentEntity = convertToFragmentWithAllDescendants(anchorEntity, dataNode); fragmentRepository.save(fragmentEntity); } catch (final DataIntegrityViolationException e) { failedXpaths.add(dataNode.getXpath()); @@ -219,30 +208,25 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService * Convert DataNode object into Fragment and places the result in the fragments placeholder. Performs same action * for all DataNode children recursively. * - * @param dataspaceEntity dataspace * @param anchorEntity anchorEntity * @param dataNodeToBeConverted dataNode * @return a Fragment built from current DataNode */ - private FragmentEntity convertToFragmentWithAllDescendants(final DataspaceEntity dataspaceEntity, - final AnchorEntity anchorEntity, + private FragmentEntity convertToFragmentWithAllDescendants(final AnchorEntity anchorEntity, final DataNode dataNodeToBeConverted) { - final FragmentEntity parentFragment = toFragmentEntity(dataspaceEntity, anchorEntity, dataNodeToBeConverted); + final FragmentEntity parentFragment = toFragmentEntity(anchorEntity, dataNodeToBeConverted); final Builder childFragmentsImmutableSetBuilder = ImmutableSet.builder(); for (final DataNode childDataNode : dataNodeToBeConverted.getChildDataNodes()) { - final FragmentEntity childFragment = - convertToFragmentWithAllDescendants(parentFragment.getDataspace(), parentFragment.getAnchor(), - childDataNode); + final FragmentEntity childFragment = convertToFragmentWithAllDescendants(anchorEntity, childDataNode); childFragmentsImmutableSetBuilder.add(childFragment); } parentFragment.setChildFragments(childFragmentsImmutableSetBuilder.build()); return parentFragment; } - private FragmentEntity toFragmentEntity(final DataspaceEntity dataspaceEntity, - final AnchorEntity anchorEntity, final DataNode dataNode) { + private FragmentEntity toFragmentEntity(final AnchorEntity anchorEntity, final DataNode dataNode) { return FragmentEntity.builder() - .dataspace(dataspaceEntity) + .dataspace(anchorEntity.getDataspace()) .anchor(anchorEntity) .xpath(dataNode.getXpath()) .attributes(jsonObjectMapper.asJsonString(dataNode.getLeaves())) @@ -266,15 +250,13 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService public Collection getDataNodesForMultipleXpaths(final String dataspaceName, final String anchorName, final Collection xpaths, final FetchDescendantsOption fetchDescendantsOption) { - final Collection fragmentEntities = getFragmentEntities(dataspaceName, anchorName, xpaths); + final AnchorEntity anchorEntity = getAnchorEntity(dataspaceName, anchorName); + final Collection fragmentEntities = getFragmentEntities(anchorEntity, xpaths); return toDataNodes(fragmentEntities, fetchDescendantsOption); } - private Collection getFragmentEntities(final String dataspaceName, final String anchorName, + private Collection getFragmentEntities(final AnchorEntity anchorEntity, final Collection xpaths) { - final DataspaceEntity dataspaceEntity = dataspaceRepository.getByName(dataspaceName); - final AnchorEntity anchorEntity = anchorRepository.getByDataspaceAndName(dataspaceEntity, anchorName); - final Collection nonRootXpaths = new HashSet<>(xpaths); final boolean haveRootXpath = nonRootXpaths.removeIf(CpsDataPersistenceServiceImpl::isRootXpath); @@ -290,50 +272,39 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService new HashSet<>(fragmentRepository.findByAnchorAndMultipleCpsPaths(anchorEntity.getId(), normalizedXpaths)); if (haveRootXpath) { - final List fragmentExtracts = fragmentRepository.getTopLevelFragments(dataspaceEntity, - anchorEntity); + final List fragmentExtracts = fragmentRepository.findAllExtractsByAnchor(anchorEntity); fragmentEntities.addAll(FragmentEntityArranger.toFragmentEntityTrees(anchorEntity, fragmentExtracts)); } return fragmentEntities; } - private FragmentEntity getFragmentEntity(final String dataspaceName, final String anchorName, final String xpath) { - final DataspaceEntity dataspaceEntity = dataspaceRepository.getByName(dataspaceName); - final AnchorEntity anchorEntity = anchorRepository.getByDataspaceAndName(dataspaceEntity, anchorName); + private FragmentEntity getFragmentEntity(final AnchorEntity anchorEntity, final String xpath) { final FragmentEntity fragmentEntity; if (isRootXpath(xpath)) { - final List fragmentExtracts = fragmentRepository.getTopLevelFragments(dataspaceEntity, - anchorEntity); + final List fragmentExtracts = fragmentRepository.findAllExtractsByAnchor(anchorEntity); fragmentEntity = FragmentEntityArranger.toFragmentEntityTrees(anchorEntity, fragmentExtracts) .stream().findFirst().orElse(null); } else { - final String normalizedXpath = getNormalizedXpath(xpath); - fragmentEntity = - fragmentRepository.getByDataspaceAndAnchorAndXpath(dataspaceEntity, anchorEntity, normalizedXpath); + fragmentEntity = fragmentRepository.getByAnchorAndXpath(anchorEntity, getNormalizedXpath(xpath)); } if (fragmentEntity == null) { - throw new DataNodeNotFoundException(dataspaceEntity.getName(), anchorEntity.getName(), xpath); + throw new DataNodeNotFoundException(anchorEntity.getDataspace().getName(), anchorEntity.getName(), xpath); } return fragmentEntity; - } private Collection buildFragmentEntitiesFromFragmentExtracts(final AnchorEntity anchorEntity, final String normalizedXpath) { final List fragmentExtracts = - fragmentRepository.findByAnchorIdAndParentXpath(anchorEntity.getId(), normalizedXpath); - log.debug("Fetched {} fragment entities by anchor {} and cps path {}.", - fragmentExtracts.size(), anchorEntity.getName(), normalizedXpath); + fragmentRepository.findByAnchorAndParentXpath(anchorEntity, normalizedXpath); return FragmentEntityArranger.toFragmentEntityTrees(anchorEntity, fragmentExtracts); - } @Override public List queryDataNodes(final String dataspaceName, final String anchorName, final String cpsPath, final FetchDescendantsOption fetchDescendantsOption) { - final DataspaceEntity dataspaceEntity = dataspaceRepository.getByName(dataspaceName); - final AnchorEntity anchorEntity = anchorRepository.getByDataspaceAndName(dataspaceEntity, anchorName); + final AnchorEntity anchorEntity = getAnchorEntity(dataspaceName, anchorName); final CpsPathQuery cpsPathQuery; try { cpsPathQuery = CpsPathUtil.getCpsPathQuery(cpsPath); @@ -441,8 +412,8 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService final CpsPathQuery cpsPathQuery) { final Set ancestorXpath = new HashSet<>(); final Pattern pattern = - Pattern.compile("([\\s\\S]*\\/" + Pattern.quote(cpsPathQuery.getAncestorSchemaNodeIdentifier()) - + REG_EX_FOR_OPTIONAL_LIST_INDEX + "\\/[\\s\\S]*"); + Pattern.compile("([\\s\\S]*/" + Pattern.quote(cpsPathQuery.getAncestorSchemaNodeIdentifier()) + + REG_EX_FOR_OPTIONAL_LIST_INDEX + "/[\\s\\S]*"); for (final FragmentEntity fragmentEntity : fragmentEntities) { final Matcher matcher = pattern.matcher(fragmentEntity.getXpath()); if (matcher.matches()) { @@ -487,7 +458,8 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService @Override public void updateDataLeaves(final String dataspaceName, final String anchorName, final String xpath, final Map updateLeaves) { - final FragmentEntity fragmentEntity = getFragmentEntity(dataspaceName, anchorName, xpath); + final AnchorEntity anchorEntity = getAnchorEntity(dataspaceName, anchorName); + final FragmentEntity fragmentEntity = getFragmentEntity(anchorEntity, xpath); final String currentLeavesAsString = fragmentEntity.getAttributes(); final String mergedLeaves = mergeLeaves(updateLeaves, currentLeavesAsString); fragmentEntity.setAttributes(mergedLeaves); @@ -497,7 +469,8 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService @Override public void updateDataNodeAndDescendants(final String dataspaceName, final String anchorName, final DataNode dataNode) { - final FragmentEntity fragmentEntity = getFragmentEntity(dataspaceName, anchorName, dataNode.getXpath()); + final AnchorEntity anchorEntity = getAnchorEntity(dataspaceName, anchorName); + final FragmentEntity fragmentEntity = getFragmentEntity(anchorEntity, dataNode.getXpath()); updateFragmentEntityAndDescendantsWithDataNode(fragmentEntity, dataNode); try { fragmentRepository.save(fragmentEntity); @@ -511,12 +484,13 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService @Override public void updateDataNodesAndDescendants(final String dataspaceName, final String anchorName, final List updatedDataNodes) { + final AnchorEntity anchorEntity = getAnchorEntity(dataspaceName, anchorName); + final Map xpathToUpdatedDataNode = updatedDataNodes.stream() .collect(Collectors.toMap(DataNode::getXpath, dataNode -> dataNode)); final Collection xpaths = xpathToUpdatedDataNode.keySet(); - final Collection existingFragmentEntities = - getFragmentEntities(dataspaceName, anchorName, xpaths); + final Collection existingFragmentEntities = getFragmentEntities(anchorEntity, xpaths); for (final FragmentEntity existingFragmentEntity : existingFragmentEntities) { final DataNode updatedDataNode = xpathToUpdatedDataNode.get(existingFragmentEntity.getXpath()); @@ -526,45 +500,41 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService try { fragmentRepository.saveAll(existingFragmentEntities); } catch (final StaleStateException staleStateException) { - retryUpdateDataNodesIndividually(dataspaceName, anchorName, existingFragmentEntities); + retryUpdateDataNodesIndividually(anchorEntity, existingFragmentEntities); } } - private void retryUpdateDataNodesIndividually(final String dataspaceName, final String anchorName, + private void retryUpdateDataNodesIndividually(final AnchorEntity anchorEntity, final Collection fragmentEntities) { final Collection failedXpaths = new HashSet<>(); - - fragmentEntities.forEach(dataNodeFragment -> { + for (final FragmentEntity dataNodeFragment : fragmentEntities) { try { fragmentRepository.save(dataNodeFragment); } catch (final StaleStateException e) { failedXpaths.add(dataNodeFragment.getXpath()); } - }); - + } if (!failedXpaths.isEmpty()) { final String failedXpathsConcatenated = String.join(",", failedXpaths); throw new ConcurrencyException("Concurrent Transactions", String.format( "DataNodes : %s in Dataspace :'%s' with Anchor : '%s' are updated by another transaction.", - failedXpathsConcatenated, dataspaceName, anchorName)); + failedXpathsConcatenated, anchorEntity.getDataspace().getName(), anchorEntity.getName())); } } private void updateFragmentEntityAndDescendantsWithDataNode(final FragmentEntity existingFragmentEntity, final DataNode newDataNode) { - existingFragmentEntity.setAttributes(jsonObjectMapper.asJsonString(newDataNode.getLeaves())); final Map existingChildrenByXpath = existingFragmentEntity.getChildFragments().stream() .collect(Collectors.toMap(FragmentEntity::getXpath, childFragmentEntity -> childFragmentEntity)); final Collection updatedChildFragments = new HashSet<>(); - for (final DataNode newDataNodeChild : newDataNode.getChildDataNodes()) { final FragmentEntity childFragment; if (isNewDataNode(newDataNodeChild, existingChildrenByXpath)) { - childFragment = convertToFragmentWithAllDescendants( - existingFragmentEntity.getDataspace(), existingFragmentEntity.getAnchor(), newDataNodeChild); + childFragment = convertToFragmentWithAllDescendants(existingFragmentEntity.getAnchor(), + newDataNodeChild); } else { childFragment = existingChildrenByXpath.get(newDataNodeChild.getXpath()); updateFragmentEntityAndDescendantsWithDataNode(childFragment, newDataNodeChild); @@ -580,7 +550,8 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService @Transactional public void replaceListContent(final String dataspaceName, final String anchorName, final String parentNodeXpath, final Collection newListElements) { - final FragmentEntity parentEntity = getFragmentEntity(dataspaceName, anchorName, parentNodeXpath); + final AnchorEntity anchorEntity = getAnchorEntity(dataspaceName, anchorName); + final FragmentEntity parentEntity = getFragmentEntity(anchorEntity, parentNodeXpath); final String listElementXpathPrefix = getListElementXpathPrefix(newListElements); final Map existingListElementFragmentEntitiesByXPath = extractListElementFragmentEntitiesByXPath(parentEntity.getChildFragments(), listElementXpathPrefix); @@ -591,7 +562,6 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService existingListElementFragmentEntitiesByXPath.get(newListElement.getXpath()); final FragmentEntity entityToBeAdded = getFragmentForReplacement(parentEntity, newListElement, existingListElementEntity); - updatedChildFragmentEntities.add(entityToBeAdded); } parentEntity.getChildFragments().addAll(updatedChildFragmentEntities); @@ -630,15 +600,14 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService return; } - final DataspaceEntity dataspaceEntity = dataspaceRepository.getByName(dataspaceName); - final AnchorEntity anchorEntity = anchorRepository.getByDataspaceAndName(dataspaceEntity, anchorName); + 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.debug("Error parsing xpath \"{}\": {}", xpath, e.getMessage()); + log.warn("Error parsing xpath \"{}\": {}", xpath, e.getMessage()); } } @@ -702,8 +671,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService final DataNode newListElement, final FragmentEntity existingListElementEntity) { if (existingListElementEntity == null) { - return convertToFragmentWithAllDescendants( - parentEntity.getDataspace(), parentEntity.getAnchor(), newListElement); + return convertToFragmentWithAllDescendants(parentEntity.getAnchor(), newListElement); } if (newListElement.getChildDataNodes().isEmpty()) { copyAttributesFromNewListElement(existingListElementEntity, newListElement); @@ -747,4 +715,9 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService } return jsonObjectMapper.asJsonString(currentLeavesAsMap); } + + private AnchorEntity getAnchorEntity(final String dataspaceName, final String anchorName) { + final DataspaceEntity dataspaceEntity = dataspaceRepository.getByName(dataspaceName); + return anchorRepository.getByDataspaceAndName(dataspaceEntity, anchorName); + } } 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 51ebcb412..996e55e77 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 @@ -25,10 +25,7 @@ package org.onap.cps.spi.repository; import java.util.Collection; import java.util.List; import java.util.Optional; -import javax.validation.constraints.NotNull; -import org.checkerframework.checker.nullness.qual.NonNull; import org.onap.cps.spi.entities.AnchorEntity; -import org.onap.cps.spi.entities.DataspaceEntity; import org.onap.cps.spi.entities.FragmentEntity; import org.onap.cps.spi.entities.FragmentExtract; import org.onap.cps.spi.exceptions.DataNodeNotFoundException; @@ -42,56 +39,26 @@ import org.springframework.stereotype.Repository; public interface FragmentRepository extends JpaRepository, FragmentRepositoryCpsPathQuery, FragmentRepositoryMultiPathQuery, FragmentNativeRepository { - Optional findByDataspaceAndAnchorAndXpath(@NonNull DataspaceEntity dataspaceEntity, - @NonNull AnchorEntity anchorEntity, - @NonNull String xpath); + Optional findByAnchorAndXpath(AnchorEntity anchorEntity, String xpath); - default FragmentEntity getByDataspaceAndAnchorAndXpath(@NonNull DataspaceEntity dataspaceEntity, - @NonNull AnchorEntity anchorEntity, - @NonNull String xpath) { - return findByDataspaceAndAnchorAndXpath(dataspaceEntity, anchorEntity, xpath) - .orElseThrow(() -> new DataNodeNotFoundException(dataspaceEntity.getName(), anchorEntity.getName(), xpath)); + default FragmentEntity getByAnchorAndXpath(final AnchorEntity anchorEntity, final String xpath) { + return findByAnchorAndXpath(anchorEntity, xpath).orElseThrow(() -> + new DataNodeNotFoundException(anchorEntity.getDataspace().getName(), anchorEntity.getName(), xpath)); } - @Query( - value = "SELECT * FROM FRAGMENT WHERE anchor_id = :anchor AND dataspace_id = :dataspace AND parent_id is NULL", - nativeQuery = true) - List findRootsByDataspaceAndAnchor(@Param("dataspace") int dataspaceId, - @Param("anchor") int anchorId); - - @Query(value = "SELECT id, anchor_id AS anchorId, xpath, parent_id AS parentId," - + " CAST(attributes AS TEXT) AS attributes" - + " FROM FRAGMENT WHERE anchor_id = :anchorId", - nativeQuery = true) - List findRootsByAnchorId(@Param("anchorId") int anchorId); + boolean existsByAnchorId(int anchorId); - /** - * find top level fragment by anchor. - * - * @param dataspaceEntity dataspace entity - * @param anchorEntity anchor entity - * @return FragmentEntity fragment entity - */ - default List getTopLevelFragments(DataspaceEntity dataspaceEntity, - AnchorEntity anchorEntity) { - final List fragmentExtracts = findRootsByAnchorId(anchorEntity.getId()); - if (fragmentExtracts.isEmpty()) { - throw new DataNodeNotFoundException(dataspaceEntity.getName(), anchorEntity.getName()); - } - return fragmentExtracts; - } + @Query("SELECT f FROM FragmentEntity f WHERE anchor = :anchor") + List findAllExtractsByAnchor(@Param("anchor") AnchorEntity anchorEntity); @Modifying - @Query("DELETE FROM FragmentEntity fe WHERE fe.anchor IN (:anchors)") - void deleteByAnchorIn(@NotNull @Param("anchors") Collection anchorEntities); + @Query("DELETE FROM FragmentEntity WHERE anchor IN (:anchors)") + void deleteByAnchorIn(@Param("anchors") Collection anchorEntities); - @Query(value = "SELECT id, anchor_id AS anchorId, xpath, parent_id AS parentId," - + " CAST(attributes AS TEXT) AS attributes" - + " FROM FRAGMENT WHERE anchor_id = :anchorId" - + " AND ( xpath = :parentXpath OR xpath LIKE CONCAT(:parentXpath,'/%') )", - nativeQuery = true) - List findByAnchorIdAndParentXpath(@Param("anchorId") int anchorId, - @Param("parentXpath") String parentXpath); + @Query("SELECT f FROM FragmentEntity f WHERE anchor = :anchor" + + " AND (xpath = :parentXpath OR xpath LIKE CONCAT(:parentXpath,'/%'))") + List findByAnchorAndParentXpath(@Param("anchor") AnchorEntity anchorEntity, + @Param("parentXpath") String parentXpath); @Query(value = "SELECT id, anchor_id AS anchorId, xpath, parent_id AS parentId," + " CAST(attributes AS TEXT) AS attributes" @@ -101,7 +68,7 @@ public interface FragmentRepository extends JpaRepository, List 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") + @Query("SELECT xpath FROM FragmentEntity f WHERE anchor = :anchor AND xpath IN :xpaths") List findAllXpathByAnchorAndXpathIn(@Param("anchor") AnchorEntity anchorEntity, @Param("xpaths") Collection xpaths); 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 71253278e..25f19f7a8 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 @@ -70,13 +70,6 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase { def static deleteTestChildXpath = "${deleteTestParentXPath}/child-with-slash[@key='a/b']" def static deleteTestGrandChildXPath = "${deleteTestChildXpath}/grandChild" - def expectedLeavesByXpathMap = [ - '/parent-207' : ['parent-leaf': 'parent-leaf value'], - '/parent-207/child-001' : ['first-child-leaf': 'first-child-leaf value'], - '/parent-207/child-002' : ['second-child-leaf': 'second-child-leaf value'], - '/parent-207/child-002/grand-child': ['grand-child-leaf': 'grand-child-leaf value'] - ] - @Sql([CLEAR_DATA, SET_DATA]) def 'Get all datanodes with descendants .'() { when: 'data nodes are retrieved by their xpath' @@ -187,7 +180,7 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase { and: 'the (grand)child node of the new list entry is also present' def dataspaceEntity = dataspaceRepository.getByName(DATASPACE_NAME) def anchorEntity = anchorRepository.getByDataspaceAndName(dataspaceEntity, ANCHOR_NAME3) - def grandChildFragmentEntity = fragmentRepository.findByDataspaceAndAnchorAndXpath(dataspaceEntity, anchorEntity, grandChild.xpath) + def grandChildFragmentEntity = fragmentRepository.findByAnchorAndXpath(anchorEntity, grandChild.xpath) assert grandChildFragmentEntity.isPresent() } @@ -212,7 +205,7 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase { and: 'the new entity is inserted correctly' def dataspaceEntity = dataspaceRepository.getByName(DATASPACE_NAME) def anchorEntity = anchorRepository.getByDataspaceAndName(dataspaceEntity, ANCHOR_HAVING_SINGLE_TOP_LEVEL_FRAGMENT) - fragmentRepository.findByDataspaceAndAnchorAndXpath(dataspaceEntity, anchorEntity, '/parent-200/child-new2').isPresent() + fragmentRepository.findByAnchorAndXpath(anchorEntity, '/parent-200/child-new2').isPresent() } @Sql([CLEAR_DATA, SET_DATA]) @@ -674,27 +667,27 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase { @Sql([CLEAR_DATA, SET_DATA]) def 'Delete data node for an anchor.'() { given: 'a data-node exists for an anchor' - assert fragmentsExistInDB(1001, 3003) + assert fragmentsExistInDB(3003) when: 'data nodes are deleted ' objectUnderTest.deleteDataNodes(DATASPACE_NAME, ANCHOR_NAME3) then: 'all data-nodes are deleted successfully' - assert !fragmentsExistInDB(1001, 3003) + assert !fragmentsExistInDB(3003) } @Sql([CLEAR_DATA, SET_DATA]) def 'Delete data node for multiple anchors.'() { given: 'a data-node exists for an anchor' - assert fragmentsExistInDB(1001, 3001) - assert fragmentsExistInDB(1001, 3003) + assert fragmentsExistInDB(3001) + assert fragmentsExistInDB(3003) when: 'data nodes are deleted ' objectUnderTest.deleteDataNodes(DATASPACE_NAME, ['ANCHOR-001', 'ANCHOR-003']) then: 'all data-nodes are deleted successfully' - assert !fragmentsExistInDB(1001, 3001) - assert !fragmentsExistInDB(1001, 3003) + assert !fragmentsExistInDB(3001) + assert !fragmentsExistInDB(3003) } - def fragmentsExistInDB(dataSpaceId, anchorId) { - !fragmentRepository.findRootsByDataspaceAndAnchor(dataSpaceId, anchorId).isEmpty() + def fragmentsExistInDB(anchorId) { + fragmentRepository.existsByAnchorId(anchorId) } static Collection toDataNodes(xpaths) { @@ -710,19 +703,6 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase { return jsonObjectMapper.convertJsonString(fragmentEntity.attributes, Map.class) } - def static assertLeavesMaps(actualLeavesMap, expectedLeavesMap) { - expectedLeavesMap.forEach((key, value) -> { - def actualValue = actualLeavesMap[key] - if (value instanceof Collection && actualValue instanceof Collection) { - assert value.size() == actualValue.size() - assert value.containsAll(actualValue) - } else { - assert value == actualValue - } - }) - return true - } - def static treeToFlatMapByXpath(Map flatMap, DataNode dataNodeTree) { flatMap.put(dataNodeTree.xpath, dataNodeTree) dataNodeTree.childDataNodes @@ -756,10 +736,9 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase { def getFragmentByXpath(dataspaceName, anchorName, xpath) { def dataspace = dataspaceRepository.getByName(dataspaceName) def anchor = anchorRepository.getByDataspaceAndName(dataspace, anchorName) - return fragmentRepository.findByDataspaceAndAnchorAndXpath(dataspace, anchor, xpath).orElseThrow() + return fragmentRepository.findByAnchorAndXpath(anchor, xpath).orElseThrow() } - def createChildListAllHavingAttributeValue(parentXpath, tag, Collection keys, boolean addGrandChild) { def listElementAsDataNodes = keysToXpaths(parentXpath, keys).collect { new DataNodeBuilder() 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 ba42a083e..e74b4a745 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 @@ -24,6 +24,7 @@ import com.fasterxml.jackson.databind.ObjectMapper import org.hibernate.StaleStateException import org.onap.cps.spi.FetchDescendantsOption import org.onap.cps.spi.entities.AnchorEntity +import org.onap.cps.spi.entities.DataspaceEntity import org.onap.cps.spi.entities.FragmentEntity import org.onap.cps.spi.exceptions.ConcurrencyException import org.onap.cps.spi.exceptions.DataValidationException @@ -48,6 +49,12 @@ class CpsDataPersistenceServiceSpec extends Specification { def objectUnderTest = Spy(new CpsDataPersistenceServiceImpl(mockDataspaceRepository, mockAnchorRepository, mockFragmentRepository, jsonObjectMapper, mockSessionManager)) + def anchorEntity = new AnchorEntity(id: 123, dataspace: new DataspaceEntity(id: 1)) + + def setup() { + mockAnchorRepository.getByDataspaceAndName(_, _) >> anchorEntity + } + def 'Storing data nodes individually when batch operation fails'(){ given: 'two data nodes and supporting repository mock behavior' def dataNode1 = createDataNodeAndMockRepositoryMethodSupportingIt('xpath1','OK') @@ -57,7 +64,7 @@ class CpsDataPersistenceServiceSpec extends Specification { when: 'trying to store data nodes' objectUnderTest.storeDataNodes('dataSpaceName', 'anchorName', [dataNode1, dataNode2]) then: 'the two data nodes are saved individually' - 2 * mockFragmentRepository.save(_); + 2 * mockFragmentRepository.save(_) } def 'Store single data node.'() { @@ -71,7 +78,7 @@ class CpsDataPersistenceServiceSpec extends Specification { def 'Handling of StaleStateException (caused by concurrent updates) during update data node and descendants.'() { given: 'the fragment repository returns a fragment entity' - mockFragmentRepository.getByDataspaceAndAnchorAndXpath(*_) >> { + mockFragmentRepository.getByAnchorAndXpath(*_) >> { def fragmentEntity = new FragmentEntity() fragmentEntity.setChildFragments([new FragmentEntity()] as Set) return fragmentEntity @@ -93,8 +100,6 @@ class CpsDataPersistenceServiceSpec extends Specification { '/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' @@ -144,10 +149,7 @@ class CpsDataPersistenceServiceSpec extends Specification { } def 'Retrieving multiple data nodes.'() { - given: 'db contains an anchor' - def anchorEntity = new AnchorEntity(id:123) - mockAnchorRepository.getByDataspaceAndName(*_) >> anchorEntity - and: 'fragment repository returns a collection of fragments' + given: 'fragment repository returns a collection of fragments' def fragmentEntity1 = new FragmentEntity(xpath: '/xpath1', childFragments: []) def fragmentEntity2 = new FragmentEntity(xpath: '/xpath2', childFragments: []) mockFragmentRepository.findByAnchorAndMultipleCpsPaths(123, ['/xpath1', '/xpath2'] as Set) >> [fragmentEntity1, fragmentEntity2] @@ -182,7 +184,7 @@ class CpsDataPersistenceServiceSpec extends Specification { def 'update data node leaves: #scenario'(){ given: 'A node exists for the given xpath' - mockFragmentRepository.getByDataspaceAndAnchorAndXpath(_, _, '/some/xpath') >> new FragmentEntity(xpath: '/some/xpath', attributes: existingAttributes) + mockFragmentRepository.getByAnchorAndXpath(_, '/some/xpath') >> new FragmentEntity(xpath: '/some/xpath', attributes: existingAttributes) when: 'the node leaves are updated' objectUnderTest.updateDataLeaves('some-dataspace', 'some-anchor', '/some/xpath', newAttributes as Map) then: 'the fragment entity saved has the original and new attributes' @@ -200,8 +202,7 @@ class CpsDataPersistenceServiceSpec extends Specification { } def 'update data node and descendants: #scenario'(){ - given: 'mocked responses' - mockAnchorRepository.getByDataspaceAndName(_, _) >> new AnchorEntity(id:123) + given: 'the fragment repository returns fragment entities related to the xpath inputs' mockFragmentRepository.findByAnchorAndMultipleCpsPaths(_, [] as Set) >> [] mockFragmentRepository.findByAnchorAndMultipleCpsPaths(_, ['/test/xpath'] as Set) >> [new FragmentEntity(xpath: '/test/xpath', childFragments: [])] when: 'replace data node tree' @@ -217,10 +218,8 @@ class CpsDataPersistenceServiceSpec extends Specification { def 'update data nodes and descendants'() { 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) + new FragmentEntity(xpath: '/test/xpath1', childFragments: [], anchor: anchorEntity), + new FragmentEntity(xpath: '/test/xpath2', childFragments: [], anchor: anchorEntity)] 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,7 +238,7 @@ class CpsDataPersistenceServiceSpec extends Specification { def createDataNodeAndMockRepositoryMethodSupportingIt(xpath, scenario) { def dataNode = new DataNodeBuilder().withXpath(xpath).build() def fragmentEntity = new FragmentEntity(xpath: xpath, childFragments: []) - mockFragmentRepository.getByDataspaceAndAnchorAndXpath(_, _, xpath) >> fragmentEntity + mockFragmentRepository.getByAnchorAndXpath(_, xpath) >> fragmentEntity if ('EXCEPTION' == scenario) { mockFragmentRepository.save(fragmentEntity) >> { throw new StaleStateException("concurrent updates") } } @@ -256,7 +255,7 @@ class CpsDataPersistenceServiceSpec extends Specification { dataNodes.add(dataNode) def fragmentEntity = new FragmentEntity(xpath: xpath, childFragments: []) fragmentEntities.add(fragmentEntity) - mockFragmentRepository.getByDataspaceAndAnchorAndXpath(_, _, xpath) >> fragmentEntity + mockFragmentRepository.getByAnchorAndXpath(_, xpath) >> fragmentEntity if ('EXCEPTION' == scenario) { mockFragmentRepository.save(fragmentEntity) >> { throw new StaleStateException("concurrent updates") } } @@ -266,8 +265,6 @@ class CpsDataPersistenceServiceSpec extends Specification { } def mockFragmentWithJson(json) { - def anchorEntity = new AnchorEntity(id:123) - mockAnchorRepository.getByDataspaceAndName(*_) >> anchorEntity def fragmentEntity = new FragmentEntity(xpath: '/parent-01', childFragments: [], attributes: json) mockFragmentRepository.findByAnchorAndMultipleCpsPaths(123, ['/parent-01'] as Set) >> [fragmentEntity] } -- 2.16.6