From 337c8d436186d87742caefc95aeee7ba2a36b687 Mon Sep 17 00:00:00 2001 From: sourabh_sourabh Date: Tue, 1 Nov 2022 14:40:25 +0000 Subject: [PATCH] Node API - GET Method performance issue - Modified toDataNode call based on fetch descendants option. - Used fragment extract to build fragment entity. - Modified data set to have correct parent id for descendants. Reviewers : Toine, Priyank and Joe Issue-ID: CPS-1171 Signed-off-by: sourabh_sourabh Change-Id: I27a537fe72dd396722e6cfde7d8c454ed2579ec0 Signed-off-by: sourabh_sourabh --- .../spi/impl/CpsDataPersistenceServiceImpl.java | 57 +++++++++++++++++----- .../CpsDataPersistenceQueryDataNodeSpec.groovy | 21 ++++---- cps-ri/src/test/resources/data/cps-path-query.sql | 6 +-- .../notification/NotificationPublisherSpec.groovy | 4 +- 4 files changed, 60 insertions(+), 28 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 5fe646ff8..dc848e657 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 @@ -232,22 +232,13 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService if (isRootXpath(xpath)) { return fragmentRepository.findFirstRootByDataspaceAndAnchor(dataspaceEntity, anchorEntity); } else { - final String normalizedXpath; - try { - normalizedXpath = CpsPathUtil.getNormalizedXpath(xpath); - } catch (final PathParsingException e) { - throw new CpsPathException(e.getMessage()); - } + final String normalizedXpath = getNormalizedXpath(xpath); final FragmentEntity fragmentEntity; if (FetchDescendantsOption.OMIT_DESCENDANTS.equals(fetchDescendantsOption)) { fragmentEntity = fragmentRepository.getByDataspaceAndAnchorAndXpath(dataspaceEntity, anchorEntity, normalizedXpath); } else { - final List fragmentExtracts = - fragmentRepository.findByAnchorIdAndParentXpath(anchorEntity.getId(), normalizedXpath); - log.debug("Fetched {} fragment entities by anchor {} and cps path {}.", - fragmentExtracts.size(), anchorName, xpath); - fragmentEntity = FragmentEntityArranger.toFragmentEntityTree(anchorEntity, fragmentExtracts); + fragmentEntity = buildFragmentEntityFromFragmentExtracts(anchorEntity, normalizedXpath); } if (fragmentEntity == null) { throw new DataNodeNotFoundException(dataspaceEntity.getName(), anchorEntity.getName(), xpath); @@ -256,6 +247,17 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService } } + private FragmentEntity buildFragmentEntityFromFragmentExtracts(final AnchorEntity anchorEntity, + final String normalizedXpath) { + final FragmentEntity fragmentEntity; + final List fragmentExtracts = + fragmentRepository.findByAnchorIdAndParentXpath(anchorEntity.getId(), normalizedXpath); + log.debug("Fetched {} fragment entities by anchor {} and cps path {}.", + fragmentExtracts.size(), anchorEntity.getName(), normalizedXpath); + fragmentEntity = FragmentEntityArranger.toFragmentEntityTree(anchorEntity, fragmentExtracts); + return fragmentEntity; + } + @Override public List queryDataNodes(final String dataspaceName, final String anchorName, final String cpsPath, final FetchDescendantsOption fetchDescendantsOption) { @@ -274,8 +276,37 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService fragmentEntities = ancestorXpaths.isEmpty() ? Collections.emptyList() : fragmentRepository.findAllByAnchorAndXpathIn(anchorEntity, ancestorXpaths); } - return fragmentEntities.stream().map(fragmentEntity -> toDataNode(fragmentEntity, fetchDescendantsOption)) - .collect(Collectors.toUnmodifiableList()); + return createDataNodesFromFragmentEntities(fetchDescendantsOption, anchorEntity, + fragmentEntities); + } + + private List createDataNodesFromFragmentEntities(final FetchDescendantsOption fetchDescendantsOption, + final AnchorEntity anchorEntity, + final List fragmentEntities) { + final List dataNodes = new ArrayList<>(fragmentEntities.size()); + for (final FragmentEntity proxiedFragmentEntity : fragmentEntities) { + final DataNode dataNode; + if (FetchDescendantsOption.OMIT_DESCENDANTS.equals(fetchDescendantsOption)) { + dataNode = toDataNode(proxiedFragmentEntity, fetchDescendantsOption); + } else { + final String normalizedXpath = getNormalizedXpath(proxiedFragmentEntity.getXpath()); + final FragmentEntity unproxiedFragmentEntity = buildFragmentEntityFromFragmentExtracts(anchorEntity, + normalizedXpath); + dataNode = toDataNode(unproxiedFragmentEntity, fetchDescendantsOption); + } + dataNodes.add(dataNode); + } + return Collections.unmodifiableList(dataNodes); + } + + private static String getNormalizedXpath(final String xpathSource) { + final String normalizedXpath; + try { + normalizedXpath = CpsPathUtil.getNormalizedXpath(xpathSource); + } catch (final PathParsingException e) { + throw new CpsPathException(e.getMessage()); + } + return normalizedXpath; } @Override diff --git a/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceQueryDataNodeSpec.groovy b/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceQueryDataNodeSpec.groovy index be2f8febf..56e388335 100644 --- a/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceQueryDataNodeSpec.groovy +++ b/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceQueryDataNodeSpec.groovy @@ -149,18 +149,19 @@ class CpsDataPersistenceQueryDataNodeSpec extends CpsPersistenceSpecBase { result.size() == expectedXPaths.size() for (int i = 0; i < result.size(); i++) { assert result[i].getXpath() == expectedXPaths[i] + assert result[i].childDataNodes.size() == expectedNumberOfChildren[i] } where: 'the following data is used' - scenario | cpsPath || expectedXPaths - 'multiple list-ancestors' | '//book/ancestor::categories' || ["/shops/shop[@id='1']/categories[@code='1']", "/shops/shop[@id='1']/categories[@code='2']"] - 'one ancestor with list value' | '//book/ancestor::categories[@code=1]' || ["/shops/shop[@id='1']/categories[@code='1']"] - 'top ancestor' | '//shop[@id=1]/ancestor::shops' || ['/shops'] - 'list with index value in the xpath prefix' | '//categories[@code=1]/book/ancestor::shop[@id=1]' || ["/shops/shop[@id='1']"] - 'ancestor with parent list' | '//book/ancestor::shop[@id=1]/categories[@code=2]' || ["/shops/shop[@id='1']/categories[@code='2']"] - 'ancestor with parent' | '//phonenumbers[@type="mob"]/ancestor::info/contact' || ["/shops/shop[@id='3']/info/contact"] - 'ancestor combined with text condition' | '//book/title[text()="Dune"]/ancestor::shop' || ["/shops/shop[@id='1']"] - 'ancestor with parent that does not exist' | '//book/ancestor::parentDoesNoExist/categories' || [] - 'ancestor does not exist' | '//book/ancestor::ancestorDoesNotExist' || [] + scenario | cpsPath || expectedXPaths || expectedNumberOfChildren + 'multiple list-ancestors' | '//book/ancestor::categories' || ["/shops/shop[@id='1']/categories[@code='1']", "/shops/shop[@id='1']/categories[@code='2']"] || [1, 1] + 'one ancestor with list value' | '//book/ancestor::categories[@code=1]' || ["/shops/shop[@id='1']/categories[@code='1']"] || [1] + 'top ancestor' | '//shop[@id=1]/ancestor::shops' || ['/shops'] || [5] + 'list with index value in the xpath prefix' | '//categories[@code=1]/book/ancestor::shop[@id=1]' || ["/shops/shop[@id='1']"] || [3] + 'ancestor with parent list' | '//book/ancestor::shop[@id=1]/categories[@code=2]' || ["/shops/shop[@id='1']/categories[@code='2']"] || [1] + 'ancestor with parent' | '//phonenumbers[@type="mob"]/ancestor::info/contact' || ["/shops/shop[@id='3']/info/contact"] || [3] + 'ancestor combined with text condition' | '//book/title[text()="Dune"]/ancestor::shop' || ["/shops/shop[@id='1']"] || [3] + 'ancestor with parent that does not exist' | '//book/ancestor::parentDoesNoExist/categories' || [] || [] + 'ancestor does not exist' | '//book/ancestor::ancestorDoesNotExist' || [] || [] } def 'Cps Path query with syntax error throws a CPS Path Exception.'() { diff --git a/cps-ri/src/test/resources/data/cps-path-query.sql b/cps-ri/src/test/resources/data/cps-path-query.sql index fa711cbbf..18fd74a07 100644 --- a/cps-ri/src/test/resources/data/cps-path-query.sql +++ b/cps-ri/src/test/resources/data/cps-path-query.sql @@ -73,6 +73,6 @@ INSERT INTO FRAGMENT (ID, DATASPACE_ID, ANCHOR_ID, PARENT_ID, XPATH, ATTRIBUTES) (14, 1001, 1003, 12, '/shops/shop[@id=''3'']/categories[@code=''2'']', '{"id" : 2, "type" : "garden centre", "name": "outdoor plants"}'), (16, 1001, 1003, 1, '/shops/shop[@id=''3'']/info', null), (17, 1001, 1003, 1, '/shops/shop[@id=''3'']/info/contact', null), - (18, 1001, 1003, 1, '/shops/shop[@id=''3'']/info/contact/website', '{"address" : "myshop.ie"}'), - (19, 1001, 1003, 12, '/shops/shop[@id=''3'']/info/contact/phonenumbers[@type=''mob'']', '{"type" : "mob", "number" : "123123456"}'), - (20, 1001, 1003, 12, '/shops/shop[@id=''3'']/info/contact/phonenumbers[@type=''landline'']', '{"type" : "landline", "number" : "012123456"}'); + (18, 1001, 1003, 17, '/shops/shop[@id=''3'']/info/contact/website', '{"address" : "myshop.ie"}'), + (19, 1001, 1003, 17, '/shops/shop[@id=''3'']/info/contact/phonenumbers[@type=''mob'']', '{"type" : "mob", "number" : "123123456"}'), + (20, 1001, 1003, 17, '/shops/shop[@id=''3'']/info/contact/phonenumbers[@type=''landline'']', '{"type" : "landline", "number" : "012123456"}'); diff --git a/cps-service/src/test/groovy/org/onap/cps/notification/NotificationPublisherSpec.groovy b/cps-service/src/test/groovy/org/onap/cps/notification/NotificationPublisherSpec.groovy index f215c6dc0..6cd9ae1b2 100644 --- a/cps-service/src/test/groovy/org/onap/cps/notification/NotificationPublisherSpec.groovy +++ b/cps-service/src/test/groovy/org/onap/cps/notification/NotificationPublisherSpec.groovy @@ -1,7 +1,8 @@ /* * ============LICENSE_START======================================================= * Copyright (C) 2021 Bell Canada. All rights reserved. - * ================================================================================ + * Modifications Copyright (C) 2021-2022 Nordix Foundation + * ================================================================================ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at @@ -20,7 +21,6 @@ package org.onap.cps.notification import org.apache.kafka.clients.producer.ProducerRecord -import org.apache.kafka.clients.producer.RecordMetadata import org.onap.cps.event.model.Content import org.onap.cps.event.model.CpsDataUpdatedEvent import org.spockframework.spring.SpringBean -- 2.16.6