From ceda6a0b4aa498ae236092cf36d396c004c61cd7 Mon Sep 17 00:00:00 2001 From: danielhanrahan Date: Tue, 28 Feb 2023 16:25:08 +0000 Subject: [PATCH] Add DataNodeNotFoundException to deleteDataNodes Current implementation of NCMP handle de-registration relies on DataNodeNotFoundException being thrown to report errors. - Make deleteDataNodes throw DataNodeFoundExceptionBatch - Update performance test timings Issue-ID: CPS-1481 Signed-off-by: danielhanrahan Change-Id: Ib833bdb4a23d24e1784bdaf4e5e5e8a9acb41c54 --- .../spi/impl/CpsDataPersistenceServiceImpl.java | 28 ++++++++++------ .../cps/spi/repository/FragmentRepository.java | 6 ++++ ...CpsDataPersistenceServiceIntegrationSpec.groovy | 17 ++++++++-- .../CpsDataPersistenceServiceDeletePerfTest.groovy | 24 +++++++------- .../exceptions/DataNodeNotFoundExceptionBatch.java | 38 ++++++++++++++++++++++ 5 files changed, 88 insertions(+), 25 deletions(-) create mode 100644 cps-service/src/main/java/org/onap/cps/spi/exceptions/DataNodeNotFoundExceptionBatch.java 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 2159ceae8..f634008dc 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 @@ -57,6 +57,7 @@ import org.onap.cps.spi.exceptions.ConcurrencyException; import org.onap.cps.spi.exceptions.CpsAdminException; import org.onap.cps.spi.exceptions.CpsPathException; import org.onap.cps.spi.exceptions.DataNodeNotFoundException; +import org.onap.cps.spi.exceptions.DataNodeNotFoundExceptionBatch; import org.onap.cps.spi.model.DataNode; import org.onap.cps.spi.model.DataNodeBuilder; import org.onap.cps.spi.repository.AnchorRepository; @@ -621,23 +622,30 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService final DataspaceEntity dataspaceEntity = dataspaceRepository.getByName(dataspaceName); final AnchorEntity anchorEntity = anchorRepository.getByDataspaceAndName(dataspaceEntity, anchorName); - final Collection normalizedXPathsToDelete = new ArrayList<>(xpathsToDelete.size()); - final Collection normalizedXpathsToPotentialLists = new ArrayList<>(); + final Collection deleteChecklist = new HashSet<>(xpathsToDelete.size()); for (final String xpath : xpathsToDelete) { try { - final CpsPathQuery cpsPathQuery = CpsPathUtil.getCpsPathQuery(xpath); - final String normalizedXpath = cpsPathQuery.getNormalizedXpath(); - normalizedXPathsToDelete.add(normalizedXpath); - if (!cpsPathQuery.isPathToListElement()) { - normalizedXpathsToPotentialLists.add(normalizedXpath); - } + deleteChecklist.add(CpsPathUtil.getNormalizedXpath(xpath)); } catch (final PathParsingException e) { log.debug("Error parsing xpath \"{}\": {}", xpath, e.getMessage()); } } - fragmentRepository.deleteByAnchorIdAndXpaths(anchorEntity.getId(), normalizedXPathsToDelete); - fragmentRepository.deleteListsByAnchorIdAndXpaths(anchorEntity.getId(), normalizedXpathsToPotentialLists); + final Collection xpathsToExistingContainers = + fragmentRepository.findAllXpathByAnchorAndXpathIn(anchorEntity, deleteChecklist); + deleteChecklist.removeAll(xpathsToExistingContainers); + + final Collection xpathsToExistingLists = deleteChecklist.stream() + .filter(xpath -> fragmentRepository.existsByAnchorAndXpathStartsWith(anchorEntity, xpath + "[")) + .collect(Collectors.toList()); + deleteChecklist.removeAll(xpathsToExistingLists); + + if (!deleteChecklist.isEmpty()) { + throw new DataNodeNotFoundExceptionBatch(dataspaceName, anchorName, deleteChecklist); + } + + fragmentRepository.deleteByAnchorIdAndXpaths(anchorEntity.getId(), xpathsToExistingContainers); + fragmentRepository.deleteListsByAnchorIdAndXpaths(anchorEntity.getId(), xpathsToExistingLists); } @Override 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 8bdb7d985..51ebcb412 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 @@ -100,4 +100,10 @@ public interface FragmentRepository extends JpaRepository, nativeQuery = true) 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") + List findAllXpathByAnchorAndXpathIn(@Param("anchor") AnchorEntity anchorEntity, + @Param("xpaths") Collection xpaths); + + boolean existsByAnchorAndXpathStartsWith(AnchorEntity anchorEntity, String xpath); } diff --git a/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceIntegrationSpec.groovy b/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceIntegrationSpec.groovy index bb80a1320..28916b1c4 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 @@ -41,7 +41,6 @@ import org.springframework.beans.factory.annotation.Autowired import org.springframework.test.context.jdbc.Sql import javax.validation.ConstraintViolationException -import java.nio.file.Path import static org.onap.cps.spi.FetchDescendantsOption.INCLUDE_ALL_DESCENDANTS import static org.onap.cps.spi.FetchDescendantsOption.OMIT_DESCENDANTS @@ -57,7 +56,6 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase { static final String SET_DATA = '/data/fragment.sql' static long ID_DATA_NODE_WITH_DESCENDANTS = 4001 static String XPATH_DATA_NODE_WITH_DESCENDANTS = '/parent-1' - static String XPATH_DATA_NODE_WITH_LEAVES = '/parent-207' static long DATA_NODE_202_FRAGMENT_ID = 4202L static long CHILD_OF_DATA_NODE_202_FRAGMENT_ID = 4203L static long LIST_DATA_NODE_PARENT201_FRAGMENT_ID = 4206L @@ -309,6 +307,7 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase { '2 unique nodes with duplicate xpath' | ["/parent-200", "/parent-202", "/parent-200"] || 2 'list element with key (single quote)' | ["/parent-201/child-204[@key='A']"] || 1 'list element with key (double quote)' | ['/parent-201/child-204[@key="A"]'] || 1 + 'whole list (not implemented)' | ["/parent-201/child-204"] || 0 'non-existing xpath' | ["/NO-XPATH"] || 0 'existing and non-existing xpaths' | ["/parent-200", "/NO-XPATH", "/parent-201"] || 2 'invalid xpath' | ["INVALID XPATH"] || 0 @@ -585,10 +584,22 @@ class CpsDataPersistenceServiceIntegrationSpec extends CpsPersistenceSpecBase { 'whole list' | ['/parent-203/child-204'] || ['/parent-203/child-203'] 'list and element in same list' | ['/parent-203/child-204', '/parent-203/child-204[@key="A"]'] || ['/parent-203/child-203'] 'list element under list element' | ['/parent-203/child-204[@key="B"]/grand-child-204[@key2="Y"]'] || ["/parent-203/child-203", "/parent-203/child-204[@key='A']", "/parent-203/child-204[@key='B']"] - 'valid but non-existing xpath' | ['/non-existing', '/parent-203/child-204'] || ['/parent-203/child-203'] 'invalid xpath' | ['INVALID XPATH', '/parent-203/child-204'] || ['/parent-203/child-203'] } + @Sql([CLEAR_DATA, SET_DATA]) + def 'Delete multiple data nodes error scenario: #scenario.'() { + when: 'deleting nodes is executed for: #scenario.' + objectUnderTest.deleteDataNodes(dataspaceName, anchorName, targetXpaths) + then: 'a #expectedException is thrown' + thrown(expectedException) + where: 'the following data is used' + scenario | dataspaceName | anchorName | targetXpaths || expectedException + 'non-existing dataspace' | 'NO DATASPACE' | 'not relevant' | ['/not relevant'] || DataspaceNotFoundException + 'non-existing anchor' | DATASPACE_NAME | 'NO ANCHOR' | ['/not relevant'] || AnchorNotFoundException + 'non-existing datanode' | DATASPACE_NAME | ANCHOR_NAME3 | ['/NON-EXISTING-XPATH'] || DataNodeNotFoundException + } + @Sql([CLEAR_DATA, SET_DATA]) def 'Delete data nodes with "/"-token in list key value: #scenario. (CPS-1409)'() { given: 'a data nodes with list-element child with "/" in index value (and grandchild)' diff --git a/cps-ri/src/test/groovy/org/onap/cps/spi/performance/CpsDataPersistenceServiceDeletePerfTest.groovy b/cps-ri/src/test/groovy/org/onap/cps/spi/performance/CpsDataPersistenceServiceDeletePerfTest.groovy index a08d8c66d..eb138b98b 100644 --- a/cps-ri/src/test/groovy/org/onap/cps/spi/performance/CpsDataPersistenceServiceDeletePerfTest.groovy +++ b/cps-ri/src/test/groovy/org/onap/cps/spi/performance/CpsDataPersistenceServiceDeletePerfTest.groovy @@ -94,8 +94,8 @@ class CpsDataPersistenceServiceDeletePerfTest extends CpsPersistencePerfSpecBase objectUnderTest.deleteDataNodes(PERF_DATASPACE, PERF_ANCHOR, xpathsToDelete) stopWatch.stop() def deleteDurationInMillis = stopWatch.getTotalTimeMillis() - then: 'delete duration is under 125 milliseconds' - recordAndAssertPerformance('Batch delete 500 grandchildren', 125, deleteDurationInMillis) + then: 'delete duration is under 75 milliseconds' + recordAndAssertPerformance('Batch delete 500 grandchildren', 75, deleteDurationInMillis) } @Sql([CLEAR_DATA, PERF_TEST_DATA]) @@ -105,8 +105,8 @@ class CpsDataPersistenceServiceDeletePerfTest extends CpsPersistencePerfSpecBase createLineage(objectUnderTest, 150, 50, true) stopWatch.stop() def setupDurationInMillis = stopWatch.getTotalTimeMillis() - then: 'setup duration is under 10 seconds' - recordAndAssertPerformance('Setup lists', 10_000, setupDurationInMillis) + then: 'setup duration is under 5 seconds' + recordAndAssertPerformance('Setup lists', 5_000, setupDurationInMillis) } def 'Delete 5 whole lists'() { @@ -132,8 +132,8 @@ class CpsDataPersistenceServiceDeletePerfTest extends CpsPersistencePerfSpecBase objectUnderTest.deleteDataNodes(PERF_DATASPACE, PERF_ANCHOR, xpathsToDelete) stopWatch.stop() def deleteDurationInMillis = stopWatch.getTotalTimeMillis() - then: 'delete duration is under 250 milliseconds' - recordAndAssertPerformance('Batch delete 100 whole lists', 250, deleteDurationInMillis) + then: 'delete duration is under 500 milliseconds' + recordAndAssertPerformance('Batch delete 100 whole lists', 500, deleteDurationInMillis) } def 'Delete 10 list elements'() { @@ -190,8 +190,8 @@ class CpsDataPersistenceServiceDeletePerfTest extends CpsPersistencePerfSpecBase objectUnderTest.deleteDataNodes(PERF_DATASPACE, PERF_ANCHOR, [PERF_TEST_PARENT]) stopWatch.stop() def deleteDurationInMillis = stopWatch.getTotalTimeMillis() - then: 'delete duration is under 250 milliseconds' - recordAndAssertPerformance('Batch delete one large node', 250, deleteDurationInMillis) + then: 'delete duration is under 200 milliseconds' + recordAndAssertPerformance('Batch delete one large node', 200, deleteDurationInMillis) } @Sql([CLEAR_DATA, PERF_TEST_DATA]) @@ -204,8 +204,8 @@ class CpsDataPersistenceServiceDeletePerfTest extends CpsPersistencePerfSpecBase objectUnderTest.deleteDataNode(PERF_DATASPACE, PERF_ANCHOR, '/') stopWatch.stop() def deleteDurationInMillis = stopWatch.getTotalTimeMillis() - then: 'delete duration is under 300 milliseconds' - recordAndAssertPerformance('Delete root node', 300, deleteDurationInMillis) + then: 'delete duration is under 250 milliseconds' + recordAndAssertPerformance('Delete root node', 250, deleteDurationInMillis) } @Sql([CLEAR_DATA, PERF_TEST_DATA]) @@ -218,8 +218,8 @@ class CpsDataPersistenceServiceDeletePerfTest extends CpsPersistencePerfSpecBase objectUnderTest.deleteDataNodes(PERF_DATASPACE, PERF_ANCHOR) stopWatch.stop() def deleteDurationInMillis = stopWatch.getTotalTimeMillis() - then: 'delete duration is under 300 milliseconds' - recordAndAssertPerformance('Delete data nodes for anchor', 300, deleteDurationInMillis) + then: 'delete duration is under 250 milliseconds' + recordAndAssertPerformance('Delete data nodes for anchor', 250, deleteDurationInMillis) } @Sql([CLEAR_DATA, PERF_TEST_DATA]) diff --git a/cps-service/src/main/java/org/onap/cps/spi/exceptions/DataNodeNotFoundExceptionBatch.java b/cps-service/src/main/java/org/onap/cps/spi/exceptions/DataNodeNotFoundExceptionBatch.java new file mode 100644 index 000000000..f38c41b40 --- /dev/null +++ b/cps-service/src/main/java/org/onap/cps/spi/exceptions/DataNodeNotFoundExceptionBatch.java @@ -0,0 +1,38 @@ +/* + * ============LICENSE_START======================================================= + * Copyright (C) 2023 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.exceptions; + +import java.util.Collection; +import lombok.Getter; + +@SuppressWarnings("squid:S110") // Team agreed to accept 6 levels of inheritance for CPS Exceptions +public class DataNodeNotFoundExceptionBatch extends DataNodeNotFoundException { + + @Getter + private final Collection notFoundXpaths; + + public DataNodeNotFoundExceptionBatch(final String dataspaceName, final String anchorName, + final Collection notFoundXpaths) { + super(dataspaceName, anchorName); + this.notFoundXpaths = notFoundXpaths; + } + +} -- 2.16.6