diff options
9 files changed, 107 insertions, 83 deletions
diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/data/DmiDataOperations.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/data/DmiDataOperations.java index 41348aea3f..1e66ff6cd2 100644 --- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/data/DmiDataOperations.java +++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/data/DmiDataOperations.java @@ -52,7 +52,7 @@ import org.onap.cps.ncmp.impl.models.DmiRequestBody; import org.onap.cps.ncmp.impl.utils.http.RestServiceUrlTemplateBuilder; import org.onap.cps.ncmp.impl.utils.http.UrlTemplateParameters; import org.onap.cps.spi.exceptions.CpsException; -import org.onap.cps.spi.exceptions.DataNodeNotFoundException; +import org.onap.cps.spi.exceptions.DataValidationException; import org.onap.cps.utils.JsonObjectMapper; import org.springframework.http.ResponseEntity; import org.springframework.stereotype.Service; @@ -285,7 +285,7 @@ public class DmiDataOperations { String cmHandleId = cmResourceAddress.getCmHandleReference(); try { return getYangModelCmHandle(cmHandleId); - } catch (final DataNodeNotFoundException ignored) { + } catch (final DataValidationException ignored) { cmHandleId = cmResourceAddress.resolveCmHandleReferenceToId(); return getYangModelCmHandle(cmHandleId); } diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/data/DmiDataOperationsSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/data/DmiDataOperationsSpec.groovy index 71054dce41..0d1cfb7c2d 100644 --- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/data/DmiDataOperationsSpec.groovy +++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/data/DmiDataOperationsSpec.groovy @@ -36,6 +36,7 @@ import org.onap.cps.ncmp.impl.utils.AlternateIdMatcher import org.onap.cps.ncmp.impl.utils.http.UrlTemplateParameters import org.onap.cps.ncmp.utils.TestUtils import org.onap.cps.spi.exceptions.DataNodeNotFoundException +import org.onap.cps.spi.exceptions.DataValidationException import org.onap.cps.utils.JsonObjectMapper import org.spockframework.spring.SpringBean import org.springframework.beans.factory.annotation.Autowired @@ -214,17 +215,21 @@ class DmiDataOperationsSpec extends DmiOperationsBaseSpec { assert objectUnderTest.resolveYangModelCmHandleFromCmHandleReference(cmResourceAddress) == yangModelCmHandle } - def 'Resolving cm handle references with alternate id.'() { + def 'Resolving cm handle references with alternate id #scenario.'() { given: 'a resource with a alternate id' - def cmResourceAddress = new CmResourceAddress('some store', 'alternate-id', 'some resource') - and: 'the alternate id cannot be found in the inventory directly and that results in a data node not found exception' - mockInventoryPersistence.getYangModelCmHandle('alternate-id') >> { throw new DataNodeNotFoundException('','') } + def cmResourceAddress = new CmResourceAddress('some store', alternateId, 'some resource') + and: 'the alternate id cannot be found in the inventory directly and that results in an exception' + mockInventoryPersistence.getYangModelCmHandle(alternateId) >> { throw errorThrownDuringCmHandleIdSearch } and: 'the alternate id can be matched to a cm handle id' - alternateIdMatcher.getCmHandleId('alternate-id') >> 'cm-handle-id' + alternateIdMatcher.getCmHandleId(alternateId) >> 'cm-handle-id' and: 'that cm handle id is available in the inventory' mockInventoryPersistence.getYangModelCmHandle('cm-handle-id') >> yangModelCmHandle expect: 'resolving that cm handle id returns the cm handle' assert objectUnderTest.resolveYangModelCmHandleFromCmHandleReference(cmResourceAddress) == yangModelCmHandle + where: 'the following alternate ids are used' + scenario | alternateId | errorThrownDuringCmHandleIdSearch + 'alternate id with no special characters' | 'alternate-id' | new DataNodeNotFoundException('','') + 'alternate id with special characters' | 'alternate#id' | new DataValidationException('','') } diff --git a/cps-rest/src/main/java/org/onap/cps/rest/controller/QueryRestController.java b/cps-rest/src/main/java/org/onap/cps/rest/controller/QueryRestController.java index b425333f9e..55a1886ce7 100644 --- a/cps-rest/src/main/java/org/onap/cps/rest/controller/QueryRestController.java +++ b/cps-rest/src/main/java/org/onap/cps/rest/controller/QueryRestController.java @@ -25,9 +25,9 @@ package org.onap.cps.rest.controller; import io.micrometer.core.annotation.Timed; import java.util.ArrayList; import java.util.Collection; -import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; import lombok.RequiredArgsConstructor; import org.onap.cps.api.CpsAnchorService; import org.onap.cps.api.CpsQueryService; @@ -126,17 +126,8 @@ public class QueryRestController implements CpsQueryApi { : (int) Math.ceil((double) totalAnchors / paginationOption.getPageSize()); } - private Map<String, List<DataNode>> groupDataNodesPerAnchor(final Collection<DataNode> dataNodes) { - final Map<String, List<DataNode>> dataNodesMapForAnchor = new HashMap<>(); - for (final DataNode dataNode : dataNodes) { - List<DataNode> dataNodesInAnchor = dataNodesMapForAnchor.get(dataNode.getAnchorName()); - if (dataNodesInAnchor == null) { - dataNodesInAnchor = new ArrayList<>(); - dataNodesMapForAnchor.put(dataNode.getAnchorName(), dataNodesInAnchor); - } - dataNodesInAnchor.add(dataNode); - } - return dataNodesMapForAnchor; + private static Map<String, List<DataNode>> groupDataNodesPerAnchor(final Collection<DataNode> dataNodes) { + return dataNodes.stream().collect(Collectors.groupingBy(DataNode::getAnchorName)); } private ResponseEntity<Object> executeNodesByDataspaceQueryAndCreateResponse(final String dataspaceName, diff --git a/cps-ri/src/main/java/org/onap/cps/ri/CpsDataPersistenceServiceImpl.java b/cps-ri/src/main/java/org/onap/cps/ri/CpsDataPersistenceServiceImpl.java index bdbdc7cf36..cacdba93cc 100644 --- a/cps-ri/src/main/java/org/onap/cps/ri/CpsDataPersistenceServiceImpl.java +++ b/cps-ri/src/main/java/org/onap/cps/ri/CpsDataPersistenceServiceImpl.java @@ -39,8 +39,6 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.TreeMap; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import java.util.stream.Collectors; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; @@ -81,8 +79,6 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService private final JsonObjectMapper jsonObjectMapper; private final SessionManager sessionManager; - private static final String REG_EX_FOR_OPTIONAL_LIST_INDEX = "(\\[@.+?])?)"; - @Override public void storeDataNodes(final String dataspaceName, final String anchorName, final Collection<DataNode> dataNodes) { @@ -228,13 +224,8 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService final FetchDescendantsOption fetchDescendantsOption) { final AnchorEntity anchorEntity = getAnchorEntity(dataspaceName, anchorName); final CpsPathQuery cpsPathQuery = getCpsPathQuery(cpsPath); - - Collection<FragmentEntity> fragmentEntities; - fragmentEntities = fragmentRepository.findByAnchorAndCpsPath(anchorEntity, cpsPathQuery); - if (cpsPathQuery.hasAncestorAxis()) { - final Collection<String> ancestorXpaths = processAncestorXpath(fragmentEntities, cpsPathQuery); - fragmentEntities = fragmentRepository.findByAnchorAndXpathIn(anchorEntity, ancestorXpaths); - } + final Collection<FragmentEntity> fragmentEntities = + fragmentRepository.findByAnchorAndCpsPath(anchorEntity, cpsPathQuery); return createDataNodesFromFragmentEntities(fetchDescendantsOption, fragmentEntities); } @@ -246,7 +237,6 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService final PaginationOption paginationOption) { final DataspaceEntity dataspaceEntity = dataspaceRepository.getByName(dataspaceName); final CpsPathQuery cpsPathQuery = getCpsPathQuery(cpsPath); - final List<Long> anchorIds; if (paginationOption == NO_PAGINATION) { anchorIds = Collections.emptyList(); @@ -256,17 +246,8 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService return Collections.emptyList(); } } - Collection<FragmentEntity> fragmentEntities = - fragmentRepository.findByDataspaceAndCpsPath(dataspaceEntity, cpsPathQuery, anchorIds); - - if (cpsPathQuery.hasAncestorAxis()) { - final Collection<String> ancestorXpaths = processAncestorXpath(fragmentEntities, cpsPathQuery); - if (anchorIds.isEmpty()) { - fragmentEntities = fragmentRepository.findByDataspaceAndXpathIn(dataspaceEntity, ancestorXpaths); - } else { - fragmentEntities = fragmentRepository.findByAnchorIdsAndXpathIn(anchorIds, ancestorXpaths); - } - } + final Collection<FragmentEntity> fragmentEntities = + fragmentRepository.findByDataspaceAndCpsPath(dataspaceEntity, cpsPathQuery, anchorIds); return createDataNodesFromFragmentEntities(fetchDescendantsOption, fragmentEntities); } @@ -668,21 +649,6 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService .collect(Collectors.toMap(FragmentEntity::getXpath, fragmentEntity -> fragmentEntity)); } - private static Set<String> processAncestorXpath(final Collection<FragmentEntity> fragmentEntities, - final CpsPathQuery cpsPathQuery) { - final Set<String> ancestorXpath = new HashSet<>(); - final Pattern pattern = - Pattern.compile("(.*/" + Pattern.quote(cpsPathQuery.getAncestorSchemaNodeIdentifier()) - + REG_EX_FOR_OPTIONAL_LIST_INDEX + "/.*"); - for (final FragmentEntity fragmentEntity : fragmentEntities) { - final Matcher matcher = pattern.matcher(fragmentEntity.getXpath()); - if (matcher.matches()) { - ancestorXpath.add(matcher.group(1)); - } - } - return ancestorXpath; - } - private static boolean isRootXpath(final String xpath) { return "/".equals(xpath) || "".equals(xpath); } diff --git a/cps-ri/src/main/java/org/onap/cps/ri/repository/FragmentQueryBuilder.java b/cps-ri/src/main/java/org/onap/cps/ri/repository/FragmentQueryBuilder.java index b8bbf59c23..e35440e29a 100644 --- a/cps-ri/src/main/java/org/onap/cps/ri/repository/FragmentQueryBuilder.java +++ b/cps-ri/src/main/java/org/onap/cps/ri/repository/FragmentQueryBuilder.java @@ -30,8 +30,10 @@ import java.util.List; import java.util.Map; import java.util.Queue; import lombok.RequiredArgsConstructor; +import org.apache.commons.text.StringSubstitutor; import org.onap.cps.cpspath.parser.CpsPathPrefixType; import org.onap.cps.cpspath.parser.CpsPathQuery; +import org.onap.cps.cpspath.parser.CpsPathUtil; import org.onap.cps.ri.models.AnchorEntity; import org.onap.cps.ri.models.DataspaceEntity; import org.onap.cps.ri.models.FragmentEntity; @@ -43,6 +45,7 @@ import org.springframework.stereotype.Component; @RequiredArgsConstructor @Component public class FragmentQueryBuilder { + private static final String DESCENDANT_PATH = "//"; @PersistenceContext private EntityManager entityManager; @@ -58,9 +61,10 @@ public class FragmentQueryBuilder { final StringBuilder sqlStringBuilder = new StringBuilder(); final Map<String, Object> queryParameters = new HashMap<>(); - sqlStringBuilder.append("SELECT fragment.* FROM fragment"); + addSearchPrefix(cpsPathQuery, sqlStringBuilder); addWhereClauseForAnchor(anchorEntity, sqlStringBuilder, queryParameters); addNodeSearchConditions(cpsPathQuery, sqlStringBuilder, queryParameters, false); + addSearchSuffix(cpsPathQuery, sqlStringBuilder, queryParameters); return getQuery(sqlStringBuilder.toString(), queryParameters, FragmentEntity.class); } @@ -78,13 +82,14 @@ public class FragmentQueryBuilder { final StringBuilder sqlStringBuilder = new StringBuilder(); final Map<String, Object> queryParameters = new HashMap<>(); - sqlStringBuilder.append("SELECT fragment.* FROM fragment"); + addSearchPrefix(cpsPathQuery, sqlStringBuilder); if (anchorIdsForPagination.isEmpty()) { addWhereClauseForDataspace(dataspaceEntity, sqlStringBuilder, queryParameters); } else { addWhereClauseForAnchorIds(anchorIdsForPagination, sqlStringBuilder, queryParameters); } addNodeSearchConditions(cpsPathQuery, sqlStringBuilder, queryParameters, true); + addSearchSuffix(cpsPathQuery, sqlStringBuilder, queryParameters); return getQuery(sqlStringBuilder.toString(), queryParameters, FragmentEntity.class); } @@ -143,7 +148,8 @@ public class FragmentQueryBuilder { final Map<String, Object> queryParameters, final boolean acrossAnchors) { addAbsoluteParentXpathSearchCondition(cpsPathQuery, sqlStringBuilder, queryParameters, acrossAnchors); - addXpathSearchCondition(cpsPathQuery, sqlStringBuilder, queryParameters); + sqlStringBuilder.append(" AND "); + addXpathSearchCondition(cpsPathQuery, sqlStringBuilder, queryParameters, "baseXpath"); addLeafConditions(cpsPathQuery, sqlStringBuilder); addTextFunctionCondition(cpsPathQuery, sqlStringBuilder, queryParameters); addContainsFunctionCondition(cpsPathQuery, sqlStringBuilder, queryParameters); @@ -151,13 +157,35 @@ public class FragmentQueryBuilder { private static void addXpathSearchCondition(final CpsPathQuery cpsPathQuery, final StringBuilder sqlStringBuilder, - final Map<String, Object> queryParameters) { - sqlStringBuilder.append(" AND (xpath LIKE :escapedXpath OR " - + "(xpath LIKE :escapedXpath||'[@%]' AND xpath NOT LIKE :escapedXpath||'[@%]/%[@%]'))"); + final Map<String, Object> queryParameters, + final String parameterName) { + queryParameters.put(parameterName, escapeXpathForSqlLike(cpsPathQuery)); + final String sqlForXpathLikeContainerOrList = """ + ( + (xpath LIKE :${xpathParamName}) + OR + (xpath LIKE :${xpathParamName}||'[@%]' AND xpath NOT LIKE :${xpathParamName}||'[@%]/%[@%]') + ) + """; + sqlStringBuilder.append(substitute(sqlForXpathLikeContainerOrList, Map.of("xpathParamName", parameterName))); + } + + /** + * Returns a pattern suitable for use in an SQL LIKE expression, matching the xpath (absolute or descendant). + * For an absolute path such as "/bookstore/categories[@name='10% off']", + * the output would be "/bookstore/categories[@name='10\% off']". + * For a descendant path such as "//categories[@name='10% off']", + * the output would be "%/categories[@name='10\% off']". + * Note: percent sign '%' means match anything in SQL LIKE, while underscore '_' means match any single character. + * + * @param cpsPathQuery Cps Path Query + * @return a pattern suitable for use in an SQL LIKE expression. + */ + private static String escapeXpathForSqlLike(final CpsPathQuery cpsPathQuery) { if (CpsPathPrefixType.ABSOLUTE.equals(cpsPathQuery.getCpsPathPrefixType())) { - queryParameters.put("escapedXpath", EscapeUtils.escapeForSqlLike(cpsPathQuery.getXpathPrefix())); + return EscapeUtils.escapeForSqlLike(cpsPathQuery.getXpathPrefix()); } else { - queryParameters.put("escapedXpath", "%/" + EscapeUtils.escapeForSqlLike(cpsPathQuery.getDescendantName())); + return "%/" + EscapeUtils.escapeForSqlLike(cpsPathQuery.getDescendantName()); } } @@ -261,6 +289,55 @@ public class FragmentQueryBuilder { } } + private static void addSearchPrefix(final CpsPathQuery cpsPathQuery, final StringBuilder sqlStringBuilder) { + if (cpsPathQuery.hasAncestorAxis()) { + sqlStringBuilder.append(""" + WITH RECURSIVE ancestors AS ( + SELECT parentFragment.* FROM fragment parentFragment + WHERE parentFragment.id IN ( + SELECT parent_id FROM fragment"""); + } else { + sqlStringBuilder.append("SELECT fragment.* FROM fragment"); + } + } + + private static void addSearchSuffix(final CpsPathQuery cpsPathQuery, + final StringBuilder sqlStringBuilder, + final Map<String, Object> queryParameters) { + if (cpsPathQuery.hasAncestorAxis()) { + sqlStringBuilder.append(""" + ) + UNION + SELECT fragment.* + FROM fragment + JOIN ancestors ON ancestors.parent_id = fragment.id + ) + SELECT * FROM ancestors + WHERE"""); + + final String ancestorPath = DESCENDANT_PATH + cpsPathQuery.getAncestorSchemaNodeIdentifier(); + final CpsPathQuery ancestorCpsPathQuery = CpsPathUtil.getCpsPathQuery(ancestorPath); + addAncestorNodeSearchCondition(ancestorCpsPathQuery, sqlStringBuilder, queryParameters); + } + } + + private static void addAncestorNodeSearchCondition(final CpsPathQuery ancestorCpsPathQuery, + final StringBuilder sqlStringBuilder, + final Map<String, Object> queryParameters) { + if (ancestorCpsPathQuery.hasLeafConditions()) { + final String pathWithoutSlashes = ancestorCpsPathQuery.getNormalizedXpath().substring(2); + queryParameters.put("ancestorXpath", "%/" + EscapeUtils.escapeForSqlLike(pathWithoutSlashes)); + sqlStringBuilder.append(" xpath LIKE :ancestorXpath"); + } else { + addXpathSearchCondition(ancestorCpsPathQuery, sqlStringBuilder, queryParameters, "ancestorXpath"); + } + } + + private static <V> String substitute(final String template, final Map<String, V> valueMap) { + final StringSubstitutor stringSubstitutor = new StringSubstitutor(valueMap); + return stringSubstitutor.replace(template); + } + private static void setQueryParameters(final Query query, final Map<String, Object> queryParameters) { for (final Map.Entry<String, Object> queryParameter : queryParameters.entrySet()) { query.setParameter(queryParameter.getKey(), queryParameter.getValue()); diff --git a/cps-ri/src/main/java/org/onap/cps/ri/repository/FragmentRepository.java b/cps-ri/src/main/java/org/onap/cps/ri/repository/FragmentRepository.java index a8c1fd2d4e..d95d322d37 100755 --- a/cps-ri/src/main/java/org/onap/cps/ri/repository/FragmentRepository.java +++ b/cps-ri/src/main/java/org/onap/cps/ri/repository/FragmentRepository.java @@ -26,7 +26,6 @@ package org.onap.cps.ri.repository; import java.util.Collection; import java.util.List; import org.onap.cps.ri.models.AnchorEntity; -import org.onap.cps.ri.models.DataspaceEntity; import org.onap.cps.ri.models.FragmentEntity; import org.onap.cps.ri.utils.EscapeUtils; import org.springframework.data.jpa.repository.JpaRepository; @@ -63,21 +62,6 @@ public interface FragmentRepository extends JpaRepository<FragmentEntity, Long>, 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 IN (:xpaths)", nativeQuery = true) - List<FragmentEntity> findByDataspaceIdAndXpathIn(@Param("dataspaceId") int dataspaceId, - @Param("xpaths") Collection<String> xpaths); - - default List<FragmentEntity> findByDataspaceAndXpathIn(final DataspaceEntity dataspaceEntity, - final Collection<String> xpaths) { - return findByDataspaceIdAndXpathIn(dataspaceEntity.getId(), xpaths); - } - - @Query(value = "SELECT * FROM fragment WHERE anchor_id IN (:anchorIds)" - + " AND xpath IN (:xpaths)", nativeQuery = true) - List<FragmentEntity> findByAnchorIdsAndXpathIn(@Param("anchorIds") Collection<Long> anchorIds, - @Param("xpaths") Collection<String> xpaths); - @Modifying @Query(value = "DELETE FROM fragment WHERE anchor_id IN (:anchorIds)", nativeQuery = true) void deleteByAnchorIdIn(@Param("anchorIds") Collection<Long> anchorIds); diff --git a/csit/plans/cps/test.properties b/csit/plans/cps/test.properties index e7b9519c2d..52e82bdb85 100644 --- a/csit/plans/cps/test.properties +++ b/csit/plans/cps/test.properties @@ -21,7 +21,7 @@ DMI_SERVICE_URL=http://$LOCAL_IP:$DMI_PORT DOCKER_REPO=nexus3.onap.org:10003 CPS_VERSION=latest -DMI_VERSION=1.5.1-SNAPSHOT-latest +DMI_VERSION=latest ADVISED_MODULES_SYNC_SLEEP_TIME_MS=2000 CMHANDLE_DATA_SYNC_SLEEP_TIME_MS=2000 diff --git a/docker-compose/docker-compose.yml b/docker-compose/docker-compose.yml index feb58d849d..ae34fc3606 100644 --- a/docker-compose/docker-compose.yml +++ b/docker-compose/docker-compose.yml @@ -142,7 +142,7 @@ services: ncmp-dmi-plugin-demo-and-csit-stub: container_name: ${NCMP_DMI_PLUGIN_DEMO_AND_CSIT_STUB_CONTAINER_NAME:-ncmp-dmi-plugin-demo-and-csit-stub} - image: ${DOCKER_REPO:-nexus3.onap.org:10003}/onap/dmi-plugin-demo-and-csit-stub:${DMI_DEMO_STUB_VERSION:-latest} + image: ${DOCKER_REPO:-nexus3.onap.org:10003}/onap/dmi-stub:${DMI_DEMO_STUB_VERSION:-latest} ports: - ${DMI_DEMO_STUB_PORT:-8784}:8092 environment: diff --git a/integration-test/src/test/groovy/org/onap/cps/integration/functional/cps/QueryServiceIntegrationSpec.groovy b/integration-test/src/test/groovy/org/onap/cps/integration/functional/cps/QueryServiceIntegrationSpec.groovy index 5c2a4fc665..3b49cfc415 100644 --- a/integration-test/src/test/groovy/org/onap/cps/integration/functional/cps/QueryServiceIntegrationSpec.groovy +++ b/integration-test/src/test/groovy/org/onap/cps/integration/functional/cps/QueryServiceIntegrationSpec.groovy @@ -224,6 +224,7 @@ class QueryServiceIntegrationSpec extends FunctionalSpecBase { 'ancestor with parent that does not exist' | '//books/ancestor::parentDoesNoExist/categories' || [] 'ancestor does not exist' | '//books/ancestor::ancestorDoesNotExist' || [] 'ancestor combined with contains condition' | '//books[contains(@title,"Mat")]/ancestor::bookstore' || ["/bookstore"] + 'ancestor is the same as search target' | '//books/ancestor::books' || [] } def 'Query for attribute by cps path of type ancestor with #scenario descendants.'() { |