From a096a7faa35b345c765102201a5a09cc03ef541a Mon Sep 17 00:00:00 2001 From: ToineSiebelink Date: Thu, 20 Oct 2022 18:34:29 +0100 Subject: [PATCH] Read Performance Improvement - Using Native Query - Native query for FragmentExtracts - Convert FragmentExtracts to tree of FragmentEntity - Native Query now used for all Gets with descendants (orignal hibernate option only used when descendanst ommited) - Added error handling for not-found on native query - Ommit descendants by default on many udpate use-cases (this might have a signifcant perf. improvemnt impact too) - Improved legacy tests for delete use-cases - Corrected performace test expectation - Fix TTL test realizing TTL resolution is whole seconds! Issue-ID: CPS-1301 Signed-off-by: ToineSiebelink Change-Id: I658ac1b7b7036f01050f30bdf9e5bd175725ef1d --- .../SynchronizationCacheConfigSpec.groovy | 33 ++++++--- .../cps/spi/entities/FragmentEntityArranger.java | 79 ++++++++++++++++++++++ .../org/onap/cps/spi/entities/FragmentExtract.java | 34 ++++++++++ .../spi/impl/CpsDataPersistenceServiceImpl.java | 61 ++++++++++++----- .../cps/spi/repository/FragmentRepository.java | 9 +++ .../FragmentRepositoryCpsPathQueryImpl.java | 2 +- ...CpsDataPersistenceServiceIntegrationSpec.groovy | 35 +++++----- .../spi/impl/CpsDataPersistenceServiceSpec.groovy | 53 ++++++--------- .../onap/cps/spi/impl/CpsToDataNodePerfSpec.groovy | 18 ++--- cps-ri/src/test/resources/application.yml | 1 + .../onap/cps/spi/CpsDataPersistenceService.java | 1 - 11 files changed, 238 insertions(+), 88 deletions(-) create mode 100644 cps-ri/src/main/java/org/onap/cps/spi/entities/FragmentEntityArranger.java create mode 100644 cps-ri/src/main/java/org/onap/cps/spi/entities/FragmentExtract.java diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/config/embeddedcache/SynchronizationCacheConfigSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/config/embeddedcache/SynchronizationCacheConfigSpec.groovy index c16d6b69b..868ee7a70 100644 --- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/config/embeddedcache/SynchronizationCacheConfigSpec.groovy +++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/config/embeddedcache/SynchronizationCacheConfigSpec.groovy @@ -74,17 +74,30 @@ class SynchronizationCacheConfigSpec extends Specification { assert dataSyncSemaphoresConfig.asyncBackupCount == 3 } - def 'Time to Live Verify for Module Sync and Data Sync Semaphore'() { - when: 'the keys are inserted with a TTL' - moduleSyncStartedOnCmHandles.put('testKeyModuleSync', 'toBeExpired' as Object, 1000, TimeUnit.MILLISECONDS) - dataSyncSemaphores.put('testKeyDataSync', Boolean.TRUE, 1000, TimeUnit.MILLISECONDS) - then: 'the entries are present in the map' + def 'Time to Live Verify for Module Sync Semaphore'() { + when: 'the key is inserted with a TTL of 1 second (Hazelcast TTL resolution is seconds!)' + moduleSyncStartedOnCmHandles.put('testKeyModuleSync', 'toBeExpired' as Object, 1, TimeUnit.SECONDS) + then: 'the entry is present in the map' assert moduleSyncStartedOnCmHandles.get('testKeyModuleSync') != null + and: 'the entry expires in less then 2 seconds' + waitMax2SecondsForKeyExpiration(moduleSyncStartedOnCmHandles, 'testKeyModuleSync') + } + + def 'Time to Live Verify for Data Sync Semaphore'() { + when: 'the key is inserted with a TTL of 1 second' + dataSyncSemaphores.put('testKeyDataSync', Boolean.TRUE, 1, TimeUnit.SECONDS) + then: 'the entry is present in the map' assert dataSyncSemaphores.get('testKeyDataSync') != null - and: 'we wait for the key expiration' - sleep(1500) - and: 'the keys should be expired as TTL elapsed' - assert moduleSyncStartedOnCmHandles.get('testKeyModuleSync') == null - assert dataSyncSemaphores.get('testKeyDataSync') == null + and: 'the entry expires in less then 2 seconds' + waitMax2SecondsForKeyExpiration(dataSyncSemaphores, 'testKeyDataSync') } + + def waitMax2SecondsForKeyExpiration(map, key) { + def count = 0 + while ( map.get(key)!=null && ++count <= 20 ) { + sleep(100) + } + return count < 20 // Should have expired in less the 20 x 100ms = 2 seconds! + } + } diff --git a/cps-ri/src/main/java/org/onap/cps/spi/entities/FragmentEntityArranger.java b/cps-ri/src/main/java/org/onap/cps/spi/entities/FragmentEntityArranger.java new file mode 100644 index 000000000..50187a487 --- /dev/null +++ b/cps-ri/src/main/java/org/onap/cps/spi/entities/FragmentEntityArranger.java @@ -0,0 +1,79 @@ +/* + * ============LICENSE_START======================================================= + * Copyright (C) 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 + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + * ============LICENSE_END========================================================= + */ + +package org.onap.cps.spi.entities; + +import java.util.Collection; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import lombok.AccessLevel; +import lombok.NoArgsConstructor; + +@NoArgsConstructor(access = AccessLevel.PRIVATE) +public class FragmentEntityArranger { + + /** + * Convert a collection of (related) FragmentExtracts into a FragmentEntity (tree) with descendants. + * Multiple top level nodes not yet support. If found only the first top level element is returned + * + * @param anchorEntity the anchor(entity) all the fragments belong to + * @param fragmentExtracts FragmentExtracts to convert + * @return a FragmentEntity (tree) with descendants, null if none found. + */ + public static FragmentEntity toFragmentEntityTree(final AnchorEntity anchorEntity, + final Collection fragmentExtracts) { + final Map fragmentEntityPerId = new HashMap<>(); + for (final FragmentExtract fragmentExtract : fragmentExtracts) { + final FragmentEntity fragmentEntity = toFragmentEntity(anchorEntity, fragmentExtract); + fragmentEntityPerId.put(fragmentEntity.getId(), fragmentEntity); + } + return reuniteChildrenWithTheirParents(fragmentEntityPerId); + } + + private static FragmentEntity toFragmentEntity(final AnchorEntity anchorEntity, + final FragmentExtract fragmentExtract) { + final FragmentEntity fragmentEntity = new FragmentEntity(); + fragmentEntity.setAnchor(anchorEntity); + fragmentEntity.setId(fragmentExtract.getId()); + fragmentEntity.setXpath(fragmentExtract.getXpath()); + fragmentEntity.setAttributes(fragmentExtract.getAttributes()); + fragmentEntity.setParentId(fragmentExtract.getParentId()); + fragmentEntity.setChildFragments(new HashSet<>()); + return fragmentEntity; + } + + private static FragmentEntity reuniteChildrenWithTheirParents(final Map fragmentEntityPerId) { + final Collection fragmentEntitiesWithoutParentInResultSet = new HashSet<>(); + for (final FragmentEntity fragmentEntity : fragmentEntityPerId.values()) { + final FragmentEntity parentFragmentEntity = fragmentEntityPerId.get(fragmentEntity.getParentId()); + if (parentFragmentEntity == null) { + fragmentEntitiesWithoutParentInResultSet.add(fragmentEntity); + } else { + parentFragmentEntity.getChildFragments().add(fragmentEntity); + } + } + if (fragmentEntitiesWithoutParentInResultSet.iterator().hasNext()) { + return fragmentEntitiesWithoutParentInResultSet.iterator().next(); + } + return null; + } + +} diff --git a/cps-ri/src/main/java/org/onap/cps/spi/entities/FragmentExtract.java b/cps-ri/src/main/java/org/onap/cps/spi/entities/FragmentExtract.java new file mode 100644 index 000000000..52c1a603b --- /dev/null +++ b/cps-ri/src/main/java/org/onap/cps/spi/entities/FragmentExtract.java @@ -0,0 +1,34 @@ +/* + * ============LICENSE_START======================================================= + * Copyright (C) 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 + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + * ============LICENSE_END========================================================= + */ + +package org.onap.cps.spi.entities; + +public interface FragmentExtract { + + Long getId(); + + Long getAnchorId(); + + String getXpath(); + + Long getParentId(); + + String getAttributes(); +} 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 06c12a89d..2a4a19226 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 @@ -51,6 +51,8 @@ import org.onap.cps.spi.cache.AnchorDataCacheEntry; 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.FragmentEntityArranger; +import org.onap.cps.spi.entities.FragmentExtract; import org.onap.cps.spi.entities.SchemaSetEntity; import org.onap.cps.spi.entities.YangResourceEntity; import org.onap.cps.spi.exceptions.AlreadyDefinedException; @@ -120,7 +122,8 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService private void addNewChildDataNode(final String dataspaceName, final String anchorName, final String parentNodeXpath, final DataNode newChild) { - final FragmentEntity parentFragmentEntity = getFragmentByXpath(dataspaceName, anchorName, parentNodeXpath); + final FragmentEntity parentFragmentEntity = + getFragmentWithoutDescendantsByXpath(dataspaceName, anchorName, parentNodeXpath); final FragmentEntity newChildAsFragmentEntity = convertToFragmentWithAllDescendants(parentFragmentEntity.getDataspace(), parentFragmentEntity.getAnchor(), newChild); @@ -135,7 +138,8 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService private void addChildrenDataNodes(final String dataspaceName, final String anchorName, final String parentNodeXpath, final Collection newChildren) { - final FragmentEntity parentFragmentEntity = getFragmentByXpath(dataspaceName, anchorName, parentNodeXpath); + final FragmentEntity parentFragmentEntity = + getFragmentWithoutDescendantsByXpath(dataspaceName, anchorName, parentNodeXpath); final List fragmentEntities = new ArrayList<>(newChildren.size()); try { newChildren.forEach(newChildAsDataNode -> { @@ -219,19 +223,25 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService @Override public DataNode getDataNode(final String dataspaceName, final String anchorName, final String xpath, final FetchDescendantsOption fetchDescendantsOption) { - final FragmentEntity fragmentEntity = getFragmentByXpath(dataspaceName, anchorName, xpath); + final FragmentEntity fragmentEntity = getFragmentByXpath(dataspaceName, anchorName, xpath, + fetchDescendantsOption); final DataNode dataNode = toDataNode(fragmentEntity, fetchDescendantsOption); dataNode.setModuleNamePrefix(getRootModuleNamePrefix(fragmentEntity.getAnchor())); return dataNode; } + private FragmentEntity getFragmentWithoutDescendantsByXpath(final String dataspaceName, + final String anchorName, + final String xpath) { + return getFragmentByXpath(dataspaceName, anchorName, xpath, FetchDescendantsOption.OMIT_DESCENDANTS); + } + private FragmentEntity getFragmentByXpath(final String dataspaceName, final String anchorName, - final String xpath) { + final String xpath, final FetchDescendantsOption fetchDescendantsOption) { final DataspaceEntity dataspaceEntity = dataspaceRepository.getByName(dataspaceName); final AnchorEntity anchorEntity = anchorRepository.getByDataspaceAndName(dataspaceEntity, anchorName); if (isRootXpath(xpath)) { - return fragmentRepository.findFirstRootByDataspaceAndAnchor( - dataspaceEntity, anchorEntity); + return fragmentRepository.findFirstRootByDataspaceAndAnchor(dataspaceEntity, anchorEntity); } else { final String normalizedXpath; try { @@ -239,9 +249,21 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService } catch (final PathParsingException e) { throw new CpsPathException(e.getMessage()); } - - return fragmentRepository.getByDataspaceAndAnchorAndXpath( - dataspaceEntity, anchorEntity, normalizedXpath); + 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); + } + if (fragmentEntity == null) { + throw new DataNodeNotFoundException(dataspaceEntity.getName(), anchorEntity.getName(), xpath); + } + return fragmentEntity; } } @@ -316,7 +338,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService } private String getRootModuleNamePrefix(final AnchorEntity anchorEntity) { - final String cachedModuleNamePrefix = getModuleNamePrefixFromCache(anchorEntity); + final String cachedModuleNamePrefix = getModuleNamePrefixFromCache(anchorEntity.getName()); if (cachedModuleNamePrefix != null) { return cachedModuleNamePrefix; } @@ -344,9 +366,9 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService } } - private String getModuleNamePrefixFromCache(final AnchorEntity anchorEntity) { - if (anchorDataCache.containsKey(anchorEntity.getName())) { - final AnchorDataCacheEntry anchorDataCacheEntry = anchorDataCache.get(anchorEntity.getName()); + private String getModuleNamePrefixFromCache(final String anchorName) { + if (anchorDataCache.containsKey(anchorName)) { + final AnchorDataCacheEntry anchorDataCacheEntry = anchorDataCache.get(anchorName); return anchorDataCacheEntry.hasProperty(TOP_LEVEL_MODULE_PREFIX_PROPERTY_NAME) ? anchorDataCacheEntry.getProperty(TOP_LEVEL_MODULE_PREFIX_PROPERTY_NAME).toString() : null; } @@ -366,7 +388,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService @Override public void updateDataLeaves(final String dataspaceName, final String anchorName, final String xpath, final Map leaves) { - final FragmentEntity fragmentEntity = getFragmentByXpath(dataspaceName, anchorName, xpath); + final FragmentEntity fragmentEntity = getFragmentWithoutDescendantsByXpath(dataspaceName, anchorName, xpath); fragmentEntity.setAttributes(jsonObjectMapper.asJsonString(leaves)); fragmentRepository.save(fragmentEntity); } @@ -374,7 +396,8 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService @Override public void updateDataNodeAndDescendants(final String dataspaceName, final String anchorName, final DataNode dataNode) { - final FragmentEntity fragmentEntity = getFragmentByXpath(dataspaceName, anchorName, dataNode.getXpath()); + final FragmentEntity fragmentEntity = + getFragmentWithoutDescendantsByXpath(dataspaceName, anchorName, dataNode.getXpath()); updateFragmentEntityAndDescendantsWithDataNode(fragmentEntity, dataNode); try { fragmentRepository.save(fragmentEntity); @@ -393,7 +416,8 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService final Map dataNodeFragmentEntityMap = dataNodes.stream() .collect(Collectors.toMap( dataNode -> dataNode, - dataNode -> getFragmentByXpath(dataspaceName, anchorName, dataNode.getXpath()))); + dataNode -> + getFragmentWithoutDescendantsByXpath(dataspaceName, anchorName, dataNode.getXpath()))); dataNodeFragmentEntityMap.forEach( (dataNode, fragmentEntity) -> updateFragmentEntityAndDescendantsWithDataNode(fragmentEntity, dataNode)); try { @@ -453,7 +477,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 = getFragmentByXpath(dataspaceName, anchorName, parentNodeXpath); + final FragmentEntity parentEntity = + getFragmentWithoutDescendantsByXpath(dataspaceName, anchorName, parentNodeXpath); final String listElementXpathPrefix = getListElementXpathPrefix(newListElements); final Map existingListElementFragmentEntitiesByXPath = extractListElementFragmentEntitiesByXPath(parentEntity.getChildFragments(), listElementXpathPrefix); @@ -507,7 +532,7 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService } else { parentNodeXpath = targetXpath.substring(0, targetXpath.lastIndexOf('/')); } - parentFragmentEntity = getFragmentByXpath(dataspaceName, anchorName, parentNodeXpath); + parentFragmentEntity = getFragmentWithoutDescendantsByXpath(dataspaceName, anchorName, parentNodeXpath); final String lastXpathElement = targetXpath.substring(targetXpath.lastIndexOf('/')); final boolean isListElement = REG_EX_PATTERN_FOR_LIST_ELEMENT_KEY_PREDICATE .matcher(lastXpathElement).find(); 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 c48c79ef6..112ebfda7 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 @@ -30,6 +30,7 @@ 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; import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.data.jpa.repository.Modifying; @@ -69,4 +70,12 @@ public interface FragmentRepository extends JpaRepository, @Modifying @Query("DELETE FROM FragmentEntity fe WHERE fe.anchor IN (:anchors)") void deleteByAnchorIn(@NotNull @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); } diff --git a/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentRepositoryCpsPathQueryImpl.java b/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentRepositoryCpsPathQueryImpl.java index 654c1c085..1d61416cf 100644 --- a/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentRepositoryCpsPathQueryImpl.java +++ b/cps-ri/src/main/java/org/onap/cps/spi/repository/FragmentRepositoryCpsPathQueryImpl.java @@ -57,7 +57,7 @@ public class FragmentRepositoryCpsPathQueryImpl implements FragmentRepositoryCps if (cpsPathQuery.hasLeafConditions()) { sqlStringBuilder.append(" AND attributes @> :leafDataAsJson\\:\\:jsonb"); queryParameters.put("leafDataAsJson", jsonObjectMapper.asJsonString( - cpsPathQuery.getLeavesData())); + cpsPathQuery.getLeavesData())); } addTextFunctionCondition(cpsPathQuery, sqlStringBuilder, queryParameters); 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 5e15ca795..412c5aa7b 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 @@ -531,28 +531,25 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase { def 'Confirm deletion of #scenario.'() { given: 'a valid data node' def dataNode - def dataNodeXpath - when: 'data nodes are deleted' + and: '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.xpath - assert dataNodeXpath == expectedXpaths - } catch (DataNodeNotFoundException) { - assert dataNodeXpath == expectedXpaths + when: 'verify data nodes are removed' + objectUnderTest.getDataNode(DATASPACE_NAME, ANCHOR_NAME3, xpathForDeletion, INCLUDE_ALL_DESCENDANTS) + then: + thrown(DataNodeNotFoundException) + and: 'some related object is not deleted' + if (xpathSurvivor!=null) { + dataNode = objectUnderTest.getDataNode(DATASPACE_NAME, ANCHOR_NAME3, xpathSurvivor, INCLUDE_ALL_DESCENDANTS) + assert dataNode.xpath == xpathSurvivor } 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']" - 'container node' | '/parent-206' | '/parent-206' || null - 'container list node' | '/parent-206[@key="A"]' | '/parent-206[@key="B"]' || "/parent-206[@key='B']" - 'root node with xpath /' | '/' | '/' || null - 'root node with xpath passed as blank' | '' | '' || null - + scenario | xpathForDeletion || xpathSurvivor + 'child data node, parent still exists' | '/parent-206/child-206' || '/parent-206' + 'list element, sibling still exists' | '/parent-206/child-206/grand-child-206[@key="A"]' || "/parent-206/child-206/grand-child-206[@key='X']" + 'container node' | '/parent-206' || null + 'container list node' | '/parent-206[@key="A"]' || "/parent-206[@key='B']" + 'root node with xpath /' | '/' || null + 'root node with xpath passed as blank' | '' || null } @Sql([CLEAR_DATA, SET_DATA]) 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 3b15b7607..b124925aa 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 @@ -25,8 +25,7 @@ import org.onap.cps.spi.FetchDescendantsOption import org.onap.cps.spi.cache.AnchorDataCacheEntry import org.onap.cps.spi.entities.AnchorEntity import org.onap.cps.spi.entities.FragmentEntity -import org.onap.cps.spi.entities.SchemaSetEntity -import org.onap.cps.spi.entities.YangResourceEntity +import org.onap.cps.spi.entities.FragmentExtract import org.onap.cps.spi.exceptions.ConcurrencyException import org.onap.cps.spi.exceptions.DataValidationException import org.onap.cps.spi.model.DataNode @@ -52,25 +51,6 @@ class CpsDataPersistenceServiceSpec extends Specification { def objectUnderTest = new CpsDataPersistenceServiceImpl( mockDataspaceRepository, mockAnchorRepository, mockFragmentRepository, jsonObjectMapper, mockSessionManager, mockAnchorDataCache) - @Shared - def NEW_RESOURCE_CONTENT = 'module stores {\n' + - ' yang-version 1.1;\n' + - ' namespace "org:onap:ccsdk:sample";\n' + - '\n' + - ' prefix book-store;\n' + - '\n' + - ' revision "2020-09-15" {\n' + - ' description\n' + - ' "Sample Model";\n' + - ' }' + - '}' - - @Shared - def yangResourceSet = [new YangResourceEntity(moduleName: 'moduleName', content: NEW_RESOURCE_CONTENT, - fileName: 'sampleYangResource' - )] as Set - - def 'Handling of StaleStateException (caused by concurrent updates) during update data node and descendants.'() { given: 'the fragment repository returns a fragment entity' mockFragmentRepository.getByDataspaceAndAnchorAndXpath(*_) >> { @@ -107,16 +87,12 @@ class CpsDataPersistenceServiceSpec extends Specification { and: 'it contains the failed datanodes' assert thrown.details.contains('/node2') assert thrown.details.contains('/node3') - } + def 'Retrieving a data node with a property JSON value of #scenario'() { - given: 'a fragment with a property JSON value of #scenario' - mockFragmentRepository.getByDataspaceAndAnchorAndXpath(*_) >> { - new FragmentEntity(childFragments: Collections.emptySet(), - attributes: "{\"some attribute\": ${dataString}}", - anchor: new AnchorEntity(schemaSet: new SchemaSetEntity(yangResources: yangResourceSet ))) - } + given: 'the db has a fragment with an attribute property JSON value of #scenario' + mockFragmentWithJson("{\"some attribute\": ${dataString}}") when: 'getting the data node represented by this fragment' def dataNode = objectUnderTest.getDataNode('my-dataspace', 'my-anchor', '/parent-01', FetchDescendantsOption.INCLUDE_ALL_DESCENDANTS) @@ -140,9 +116,7 @@ class CpsDataPersistenceServiceSpec extends Specification { def 'Retrieving a data node with invalid JSON'() { given: 'a fragment with invalid JSON' - mockFragmentRepository.getByDataspaceAndAnchorAndXpath(*_) >> { - new FragmentEntity(childFragments: Collections.emptySet(), attributes: '{invalid json') - } + mockFragmentWithJson('{invalid json') when: 'getting the data node represented by this fragment' objectUnderTest.getDataNode('my-dataspace', 'my-anchor', '/parent-01', FetchDescendantsOption.INCLUDE_ALL_DESCENDANTS) @@ -214,4 +188,21 @@ class CpsDataPersistenceServiceSpec extends Specification { } return dataNode } + + def mockFragmentWithJson(json) { + def anchorName = 'some anchor' + def anchorDataCacheEntry = new AnchorDataCacheEntry() + anchorDataCacheEntry.setProperty(objectUnderTest.TOP_LEVEL_MODULE_PREFIX_PROPERTY_NAME, 'some prefix') + mockAnchorDataCache.containsKey(anchorName) >> true + mockAnchorDataCache.get(anchorName) >> anchorDataCacheEntry + def mockAnchor = Mock(AnchorEntity) + mockAnchor.getId() >> 123 + mockAnchor.getName() >> anchorName + mockAnchorRepository.getByDataspaceAndName(*_) >> mockAnchor + def mockFragmentExtract = Mock(FragmentExtract) + mockFragmentExtract.getId() >> 456 + mockFragmentExtract.getAttributes() >> json + mockFragmentRepository.findByAnchorIdAndParentXpath(*_) >> [mockFragmentExtract] + } + } diff --git a/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsToDataNodePerfSpec.groovy b/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsToDataNodePerfSpec.groovy index 5b2802813..283be6b9b 100644 --- a/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsToDataNodePerfSpec.groovy +++ b/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsToDataNodePerfSpec.groovy @@ -22,20 +22,23 @@ package org.onap.cps.ri.performance import org.apache.commons.lang3.time.StopWatch import org.onap.cps.spi.CpsDataPersistenceService +import org.onap.cps.spi.impl.CpsDataPersistenceServiceImpl import org.onap.cps.spi.impl.CpsPersistenceSpecBase import org.onap.cps.spi.model.DataNode import org.onap.cps.spi.model.DataNodeBuilder +import org.onap.cps.spi.repository.FragmentRepository import org.springframework.beans.factory.annotation.Autowired import org.springframework.test.context.jdbc.Sql import static org.onap.cps.spi.FetchDescendantsOption.INCLUDE_ALL_DESCENDANTS class CpsToDataNodePerfSpec extends CpsPersistenceSpecBase { + static final String SET_DATA = '/data/fragment.sql' + @Autowired CpsDataPersistenceService objectUnderTest - static final String SET_DATA = '/data/fragment.sql' - static final String XPATH_DATA_NODE_WITH_DESCENDANTS = '/parent-1' + def XPATH_DATA_NODE_WITH_DESCENDANTS = '/parent-1' @Sql([CLEAR_DATA, SET_DATA]) def 'Get data node by xpath with all descendants with many children'() { @@ -48,14 +51,13 @@ class CpsToDataNodePerfSpec extends CpsPersistenceSpecBase { when: 'data node is requested with all descendants' def readStopWatch = new StopWatch() readStopWatch.start() - def result = objectUnderTest.getDataNode( - DATASPACE_NAME, ANCHOR_NAME1, XPATH_DATA_NODE_WITH_DESCENDANTS, INCLUDE_ALL_DESCENDANTS) + def result = objectUnderTest.getDataNode(DATASPACE_NAME, ANCHOR_NAME1, XPATH_DATA_NODE_WITH_DESCENDANTS, INCLUDE_ALL_DESCENDANTS) readStopWatch.stop() def readDurationInMillis = readStopWatch.getTime() - then : 'setup duration is under 8 seconds' + then: 'setup duration is under 8 seconds' assert setupDurationInMillis < 8000 - and: 'read duration is under 6 seconds' - assert readDurationInMillis < 6000 + and: 'read duration is under 1500 milliseconds' + assert readDurationInMillis < 1500 and: 'data node is returned with all the descendants populated' assert countDataNodes(result) == 1533 } @@ -86,4 +88,4 @@ class CpsToDataNodePerfSpec extends CpsPersistenceSpecBase { } return nodeCount } -} \ No newline at end of file +} diff --git a/cps-ri/src/test/resources/application.yml b/cps-ri/src/test/resources/application.yml index e835b77b1..4f40aeaa0 100644 --- a/cps-ri/src/test/resources/application.yml +++ b/cps-ri/src/test/resources/application.yml @@ -25,6 +25,7 @@ spring: enable_lazy_load_no_trans: true dialect: org.hibernate.dialect.PostgreSQLDialect format_sql: true + show_sql: false datasource: url: ${DB_URL} diff --git a/cps-service/src/main/java/org/onap/cps/spi/CpsDataPersistenceService.java b/cps-service/src/main/java/org/onap/cps/spi/CpsDataPersistenceService.java index cd0cefcbf..8d4df20b8 100644 --- a/cps-service/src/main/java/org/onap/cps/spi/CpsDataPersistenceService.java +++ b/cps-service/src/main/java/org/onap/cps/spi/CpsDataPersistenceService.java @@ -89,7 +89,6 @@ public interface CpsDataPersistenceService { DataNode getDataNode(String dataspaceName, String anchorName, String xpath, FetchDescendantsOption fetchDescendantsOption); - /** * Updates leaves for existing data node. * -- 2.16.6