From a3e8d92c291a61395e91554004af318f70090ecf Mon Sep 17 00:00:00 2001 From: Renu Kumari Date: Wed, 14 Jul 2021 14:26:33 -0400 Subject: [PATCH] Fixed inconsistent data issue with replaceNode Issue-ID: CPS-501 Signed-off-by: Renu Kumari Change-Id: Ic4785d97013729b80f81aca3de4430bdaa8155fa --- .../spi/impl/CpsDataPersistenceServiceImpl.java | 68 ++++++++++++++------ .../spi/impl/CpsDataPersistenceServiceSpec.groovy | 50 +++++++++++++-- .../impl/CpsDataPersistenceServiceUnitSpec.groovy | 72 ++++++++++++++++++++++ .../cps/spi/exceptions/ConcurrencyException.java | 27 ++++++++ 4 files changed, 195 insertions(+), 22 deletions(-) create mode 100644 cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceUnitSpec.groovy create mode 100644 cps-service/src/main/java/org/onap/cps/spi/exceptions/ConcurrencyException.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 844ad8474..81205a1da 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 @@ -37,6 +37,8 @@ import java.util.Set; import java.util.regex.Pattern; import java.util.stream.Collectors; import javax.transaction.Transactional; +import lombok.extern.slf4j.Slf4j; +import org.hibernate.StaleStateException; import org.onap.cps.cpspath.parser.CpsPathQuery; import org.onap.cps.cpspath.parser.CpsPathQueryType; import org.onap.cps.spi.CpsDataPersistenceService; @@ -45,28 +47,40 @@ 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.AlreadyDefinedException; +import org.onap.cps.spi.exceptions.ConcurrencyException; import org.onap.cps.spi.exceptions.CpsPathException; import org.onap.cps.spi.model.DataNode; import org.onap.cps.spi.model.DataNodeBuilder; import org.onap.cps.spi.repository.AnchorRepository; import org.onap.cps.spi.repository.DataspaceRepository; import org.onap.cps.spi.repository.FragmentRepository; -import org.springframework.beans.factory.annotation.Autowired; import org.springframework.dao.DataIntegrityViolationException; import org.springframework.stereotype.Service; @Service +@Slf4j public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService { - @Autowired private DataspaceRepository dataspaceRepository; - @Autowired private AnchorRepository anchorRepository; - @Autowired private FragmentRepository fragmentRepository; + /** + * Constructor. + * + * @param dataspaceRepository dataspaceRepository + * @param anchorRepository anchorRepository + * @param fragmentRepository fragmentRepository + */ + public CpsDataPersistenceServiceImpl(final DataspaceRepository dataspaceRepository, + final AnchorRepository anchorRepository, final FragmentRepository fragmentRepository) { + this.dataspaceRepository = dataspaceRepository; + this.anchorRepository = anchorRepository; + this.fragmentRepository = fragmentRepository; + } + private static final Gson GSON = new GsonBuilder().create(); private static final String REG_EX_FOR_OPTIONAL_LIST_INDEX = "(\\[@\\S+?]){0,1})"; @@ -247,18 +261,41 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService } @Override - public void replaceDataNodeTree(final String dataspaceName, final String anchorName, final DataNode dataNode) { + public void replaceDataNodeTree(final String dataspaceName, final String anchorName, + final DataNode dataNode) { final var fragmentEntity = getFragmentByXpath(dataspaceName, anchorName, dataNode.getXpath()); - removeExistingDescendants(fragmentEntity); + replaceDataNodeTree(fragmentEntity, dataNode); + try { + fragmentRepository.save(fragmentEntity); + } catch (final StaleStateException staleStateException) { + throw new ConcurrencyException("Concurrent Transactions", + String.format("dataspace :'%s', Anchor : '%s' and xpath: '%s' is updated by another transaction.", + dataspaceName, anchorName, dataNode.getXpath()), + staleStateException); + } + } - fragmentEntity.setAttributes(GSON.toJson(dataNode.getLeaves())); - final Set childFragmentEntities = dataNode.getChildDataNodes().stream().map( - childDataNode -> convertToFragmentWithAllDescendants( - fragmentEntity.getDataspace(), fragmentEntity.getAnchor(), childDataNode) - ).collect(Collectors.toUnmodifiableSet()); - fragmentEntity.setChildFragments(childFragmentEntities); + private void replaceDataNodeTree(final FragmentEntity existingFragmentEntity, final DataNode submittedDataNode) { - fragmentRepository.save(fragmentEntity); + existingFragmentEntity.setAttributes(GSON.toJson(submittedDataNode.getLeaves())); + + final Map existingChildrenByXpath = existingFragmentEntity.getChildFragments() + .stream().collect(Collectors.toMap(FragmentEntity::getXpath, childFragmentEntity -> childFragmentEntity)); + + final var updatedChildFragments = new HashSet(); + + for (final DataNode submittedChildDataNode : submittedDataNode.getChildDataNodes()) { + final FragmentEntity childFragment; + if (existingChildrenByXpath.containsKey(submittedChildDataNode.getXpath())) { + childFragment = existingChildrenByXpath.get(submittedChildDataNode.getXpath()); + replaceDataNodeTree(childFragment, submittedChildDataNode); + } else { + childFragment = convertToFragmentWithAllDescendants( + existingFragmentEntity.getDataspace(), existingFragmentEntity.getAnchor(), submittedChildDataNode); + } + updatedChildFragments.add(childFragment); + } + existingFragmentEntity.setChildFragments(updatedChildFragments); } @Override @@ -285,11 +322,6 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService } } - private void removeExistingDescendants(final FragmentEntity fragmentEntity) { - fragmentEntity.setChildFragments(Collections.emptySet()); - fragmentRepository.save(fragmentEntity); - } - private static boolean isRootXpath(final String xpath) { return "/".equals(xpath) || "".equals(xpath); } 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 0ad67d541..3a6947379 100755 --- 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 @@ -21,6 +21,8 @@ */ package org.onap.cps.spi.impl +import org.onap.cps.spi.exceptions.ConcurrencyException + import static org.onap.cps.spi.FetchDescendantsOption.INCLUDE_ALL_DESCENDANTS import static org.onap.cps.spi.FetchDescendantsOption.OMIT_DESCENDANTS @@ -305,13 +307,53 @@ class CpsDataPersistenceServiceSpec extends CpsPersistenceSpecBase { def updatedLeaves = getLeavesMap(updatedFragment) assert updatedLeaves.size() == 1 assert updatedLeaves.'leaf-value' == 'new' + and: 'existing child entry is not updated as content is same' + def childFragment = updatedFragment.getChildFragments().iterator().next() + childFragment.getXpath() == '/parent-200/child-201/grand-child' + def childLeaves = getLeavesMap(childFragment) + assert childLeaves.'leaf-value' == 'original' + } + + @Sql([CLEAR_DATA, SET_DATA]) + def 'Replace data node tree with same descendants but changed leaf value.'() { + given: 'data node object with leaves updated, having child with old content' + def submittedDataNode = buildDataNode("/parent-200/child-201", ['leaf-value': 'new'], [ + buildDataNode("/parent-200/child-201/grand-child", ['leaf-value': 'new'], []) + ]) + when: 'update is performed including descendants' + objectUnderTest.replaceDataNodeTree(DATASPACE_NAME, ANCHOR_FOR_DATA_NODES_WITH_LEAVES, submittedDataNode) + then: 'leaves have been updated for selected data node' + def updatedFragment = fragmentRepository.getOne(UPDATE_DATA_NODE_FRAGMENT_ID) + def updatedLeaves = getLeavesMap(updatedFragment) + assert updatedLeaves.size() == 1 + assert updatedLeaves.'leaf-value' == 'new' + and: 'existing child entry is updated with the new content' + def childFragment = updatedFragment.getChildFragments().iterator().next() + childFragment.getXpath() == '/parent-200/child-201/grand-child' + def childLeaves = getLeavesMap(childFragment) + assert childLeaves.'leaf-value' == 'new' + } + + @Sql([CLEAR_DATA, SET_DATA]) + def 'Replace data node tree with different descendants xpath'() { + given: 'data node object with leaves updated, having child with old content' + def submittedDataNode = buildDataNode("/parent-200/child-201", ['leaf-value': 'new'], [ + buildDataNode("/parent-200/child-201/grand-child-new", ['leaf-value': 'new'], []) + ]) + when: 'update is performed including descendants' + objectUnderTest.replaceDataNodeTree(DATASPACE_NAME, ANCHOR_FOR_DATA_NODES_WITH_LEAVES, submittedDataNode) + then: 'leaves have been updated for selected data node' + def updatedFragment = fragmentRepository.getOne(UPDATE_DATA_NODE_FRAGMENT_ID) + def updatedLeaves = getLeavesMap(updatedFragment) + assert updatedLeaves.size() == 1 + assert updatedLeaves.'leaf-value' == 'new' and: 'previously attached child entry is removed from database' fragmentRepository.findById(UPDATE_DATA_NODE_SUB_FRAGMENT_ID).isEmpty() - and: 'new child entry with same content is created' + and: 'new child entry is persisted' def childFragment = updatedFragment.getChildFragments().iterator().next() + childFragment.getXpath() == '/parent-200/child-201/grand-child-new' def childLeaves = getLeavesMap(childFragment) - assert childFragment.getId() != UPDATE_DATA_NODE_SUB_FRAGMENT_ID - assert childLeaves.'leaf-value' == 'original' + assert childLeaves.'leaf-value' == 'new' } @Sql([CLEAR_DATA, SET_DATA]) @@ -320,7 +362,7 @@ class CpsDataPersistenceServiceSpec extends CpsPersistenceSpecBase { def submittedDataNode = buildDataNode(xpath, ['leaf-name': 'leaf-value'], []) when: 'attempt to update data node for #scenario' objectUnderTest.replaceDataNodeTree(dataspaceName, anchorName, submittedDataNode) - then: 'a #expectedException is thrown' + then: 'a #expectedException is thrown' thrown(expectedException) where: 'the following data is used' scenario | dataspaceName | anchorName | xpath || expectedException diff --git a/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceUnitSpec.groovy b/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceUnitSpec.groovy new file mode 100644 index 000000000..5257e62a6 --- /dev/null +++ b/cps-ri/src/test/groovy/org/onap/cps/spi/impl/CpsDataPersistenceServiceUnitSpec.groovy @@ -0,0 +1,72 @@ +/* + * ============LICENSE_START======================================================= + * Copyright (c) 2021 Bell Canada. + * ================================================================================ + * 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. + * ============LICENSE_END========================================================= +*/ + +package org.onap.cps.spi.impl + +import org.hibernate.StaleStateException +import org.onap.cps.spi.entities.FragmentEntity +import org.onap.cps.spi.exceptions.ConcurrencyException +import org.onap.cps.spi.model.DataNodeBuilder +import org.onap.cps.spi.repository.AnchorRepository +import org.onap.cps.spi.repository.DataspaceRepository +import org.onap.cps.spi.repository.FragmentRepository +import spock.lang.Specification + + +class CpsDataPersistenceServiceUnitSpec extends Specification { + + def mockDataspaceRepository = Mock(DataspaceRepository) + def mockAnchorRepository = Mock(AnchorRepository) + def mockFragmentRepository = Mock(FragmentRepository) + + def objectUnderTest = new CpsDataPersistenceServiceImpl( + mockDataspaceRepository, mockAnchorRepository, mockFragmentRepository) + + def 'Handling of StaleStateException (caused by concurrent updates) during data node tree update.'() { + + def parentXpath = 'parent-01' + def myDataspaceName = 'my-dataspace' + def myAnchorName = 'my-anchor' + + given: 'data node object' + def submittedDataNode = new DataNodeBuilder() + .withXpath(parentXpath) + .withLeaves(['leaf-name': 'leaf-value']) + .build() + and: 'fragment to be updated' + mockFragmentRepository.getByDataspaceAndAnchorAndXpath(_, _, _) >> { + def fragmentEntity = new FragmentEntity() + fragmentEntity.setXpath(parentXpath) + fragmentEntity.setChildFragments(Collections.emptySet()) + return fragmentEntity + } + and: 'data node is concurrently updated by another transaction' + mockFragmentRepository.save(_) >> { throw new StaleStateException("concurrent updates") } + + when: 'attempt to update data node' + objectUnderTest.replaceDataNodeTree(myDataspaceName, myAnchorName, submittedDataNode) + + then: 'concurrency exception is thrown' + def concurrencyException = thrown(ConcurrencyException) + assert concurrencyException.getDetails().contains(myDataspaceName) + assert concurrencyException.getDetails().contains(myAnchorName) + assert concurrencyException.getDetails().contains(parentXpath) + } + + +} diff --git a/cps-service/src/main/java/org/onap/cps/spi/exceptions/ConcurrencyException.java b/cps-service/src/main/java/org/onap/cps/spi/exceptions/ConcurrencyException.java new file mode 100644 index 000000000..3a8a94b97 --- /dev/null +++ b/cps-service/src/main/java/org/onap/cps/spi/exceptions/ConcurrencyException.java @@ -0,0 +1,27 @@ +/* + * ============LICENSE_START======================================================= + * Copyright (c) 2021 Bell Canada. + * ================================================================================ + * 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. + * ============LICENSE_END========================================================= + */ + +package org.onap.cps.spi.exceptions; + +public class ConcurrencyException extends CpsException { + + public ConcurrencyException(final String message, final String details, final Throwable cause) { + super(message, details, cause); + } + +} -- 2.16.6