From 7afbb975ecf486e2802a229459133c7ee93944d5 Mon Sep 17 00:00:00 2001 From: Arpit Singh Date: Wed, 15 Oct 2025 14:40:00 +0530 Subject: [PATCH] Revert "Inconsistency With JSON Response(List Items) Using ReplaceANode API" - This reverts commit 30d460ae43131d5fef3de041dcbcd2943702724a associated with CPS-2800. - Due to logical errors in the previous patch, any data node added using Replace a node with descendants API gets added at the root level instead of the specific xpath. - Suitable approach is to revert the patch and rework on a new patch set with corrected logic. Issue-ID: CPS-3013 Change-Id: I5bc5441ffed039332b8e64eb9d6037771174de4e Signed-off-by: Arpit Singh --- cps-rest/docs/openapi/cpsData.yml | 4 +- .../cps/rest/controller/DataRestController.java | 8 ++-- .../rest/controller/DataRestControllerSpec.groovy | 21 ---------- .../onap/cps/ri/CpsDataPersistenceServiceImpl.java | 48 ++++++++-------------- .../ri/CpsDataPersistenceServiceImplSpec.groovy | 24 +---------- .../main/java/org/onap/cps/api/CpsDataService.java | 6 +-- .../java/org/onap/cps/impl/CpsDataServiceImpl.java | 6 +-- .../onap/cps/spi/CpsDataPersistenceService.java | 8 +--- docs/api/swagger/cps/openapi.yaml | 7 ---- 9 files changed, 29 insertions(+), 103 deletions(-) diff --git a/cps-rest/docs/openapi/cpsData.yml b/cps-rest/docs/openapi/cpsData.yml index 7ab19828c5..178a68fb77 100644 --- a/cps-rest/docs/openapi/cpsData.yml +++ b/cps-rest/docs/openapi/cpsData.yml @@ -1,7 +1,7 @@ # ============LICENSE_START======================================================= # Copyright (c) 2021-2022 Bell Canada. # Modifications Copyright (C) 2021-2022 Nordix Foundation -# Modifications Copyright (C) 2022-2025 TechMahindra Ltd. +# Modifications Copyright (C) 2022-2024 TechMahindra Ltd. # Modifications Copyright (C) 2022 Deutsche Telekom AG # ================================================================================ # Licensed under the Apache License, Version 2.0 (the "License"); @@ -240,8 +240,6 @@ nodesByDataspaceAndAnchor: responses: '200': $ref: 'components.yml#/components/responses/Ok' - '201': - $ref: 'components.yml#/components/responses/Created' '400': $ref: 'components.yml#/components/responses/BadRequest' '403': diff --git a/cps-rest/src/main/java/org/onap/cps/rest/controller/DataRestController.java b/cps-rest/src/main/java/org/onap/cps/rest/controller/DataRestController.java index 9ba46918c2..b2f323a8d9 100755 --- a/cps-rest/src/main/java/org/onap/cps/rest/controller/DataRestController.java +++ b/cps-rest/src/main/java/org/onap/cps/rest/controller/DataRestController.java @@ -169,11 +169,11 @@ public class DataRestController implements CpsDataApi { if (Boolean.TRUE.equals(dryRunEnabled)) { cpsDataService.validateData(dataspaceName, anchorName, parentNodeXpath, nodeData, contentType); return ResponseEntity.ok().build(); + } else { + cpsDataService.updateDataNodeAndDescendants(dataspaceName, anchorName, parentNodeXpath, + nodeData, toOffsetDateTime(observedTimestamp), contentType); } - final boolean hasNewDataNodes = cpsDataService.updateDataNodeAndDescendants(dataspaceName, anchorName, - parentNodeXpath, nodeData, toOffsetDateTime(observedTimestamp), contentType); - final HttpStatus httpStatus = hasNewDataNodes ? HttpStatus.CREATED : HttpStatus.OK; - return ResponseEntity.status(httpStatus).build(); + return ResponseEntity.status(HttpStatus.OK).build(); } @Override diff --git a/cps-rest/src/test/groovy/org/onap/cps/rest/controller/DataRestControllerSpec.groovy b/cps-rest/src/test/groovy/org/onap/cps/rest/controller/DataRestControllerSpec.groovy index cf1c49ed3e..3762b6e853 100755 --- a/cps-rest/src/test/groovy/org/onap/cps/rest/controller/DataRestControllerSpec.groovy +++ b/cps-rest/src/test/groovy/org/onap/cps/rest/controller/DataRestControllerSpec.groovy @@ -420,27 +420,6 @@ class DataRestControllerSpec extends Specification { 'XML content: some xpath by parent' | '/some/xpath' | MediaType.APPLICATION_XML || '/some/xpath' | requestBodyXml | expectedXmlData | ContentType.XML } - def 'Replace data node tree returns #hasNewNodes for #scenario.'() { - given: 'endpoint to replace node' - def endpoint = "$dataNodeBaseEndpointV2/anchors/$anchorName/nodes" - when: 'put request is performed' - def response = - mvc.perform( - put(endpoint) - .contentType(MediaType.APPLICATION_JSON) - .content(requestBodyJson) - .param('xpath', '')) - .andReturn().response - then: 'the cps data service method is invoked with expected parameters' - 1 * mockCpsDataService.updateDataNodeAndDescendants(dataspaceName, anchorName, '/', expectedJsonData, noTimestamp, ContentType.JSON) >> hasNewNodes - and: 'response status indicates success or creation' - assert response.status == expectedStatus - where: - scenario | hasNewNodes || expectedStatus - 'JSON content: root node updated only' | false || HttpStatus.OK.value() - 'JSON content: root node with new list items' | true || HttpStatus.CREATED.value() - } - def 'Validate data using Replace data node API.'() { given: 'endpoint to replace node' def endpoint = "$dataNodeBaseEndpointV1/anchors/$anchorName/nodes" 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 8b5aed053e..3e80e30c2a 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 @@ -3,7 +3,7 @@ * Copyright (C) 2021-2025 OpenInfra Foundation Europe. All rights reserved. * Modifications Copyright (C) 2021 Pantheon.tech * Modifications Copyright (C) 2020-2022 Bell Canada. - * Modifications Copyright (C) 2022-2025 TechMahindra Ltd. + * Modifications Copyright (C) 2022-2023 TechMahindra Ltd. * ================================================================================ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -151,40 +151,26 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService } @Override - public boolean updateDataNodesAndDescendants(final String dataspaceName, final String anchorName, + public void updateDataNodesAndDescendants(final String dataspaceName, final String anchorName, final Collection updatedDataNodes) { final AnchorEntity anchorEntity = getAnchorEntity(dataspaceName, anchorName); + final Map xpathToUpdatedDataNode = updatedDataNodes.stream() .collect(Collectors.toMap(DataNode::getXpath, dataNode -> dataNode)); + final Collection xpaths = xpathToUpdatedDataNode.keySet(); Collection existingFragmentEntities = getFragmentEntities(anchorEntity, xpaths); - final List newDataNodes = identifyNewDataNodes(updatedDataNodes, existingFragmentEntities); + + logMissingXPaths(xpaths, existingFragmentEntities); + existingFragmentEntities = fragmentRepository.prefetchDescendantsOfFragmentEntities( FetchDescendantsOption.INCLUDE_ALL_DESCENDANTS, existingFragmentEntities); - updateExistingFragments(existingFragmentEntities, xpathToUpdatedDataNode); - addFragmentEntitiesForNewDataNodes(anchorEntity, newDataNodes, existingFragmentEntities); - persistFragmentEntitiesWithRetry(anchorEntity, existingFragmentEntities); - return !newDataNodes.isEmpty(); - } - private void updateExistingFragments(final Collection existingFragmentEntities, - final Map xpathToUpdatedDataNode) { for (final FragmentEntity existingFragmentEntity : existingFragmentEntities) { final DataNode updatedDataNode = xpathToUpdatedDataNode.get(existingFragmentEntity.getXpath()); updateFragmentEntityAndDescendantsWithDataNode(existingFragmentEntity, updatedDataNode); } - } - private void addFragmentEntitiesForNewDataNodes(final AnchorEntity anchorEntity, final List newNodes, - final Collection existingFragmentEntities) { - for (final DataNode newNode : newNodes) { - final FragmentEntity newFragmentEntity = convertToFragmentWithAllDescendants(anchorEntity, newNode); - existingFragmentEntities.add(newFragmentEntity); - } - } - - private void persistFragmentEntitiesWithRetry(final AnchorEntity anchorEntity, - final Collection existingFragmentEntities) { try { fragmentRepository.saveAll(existingFragmentEntities); } catch (final StaleStateException staleStateException) { @@ -192,16 +178,6 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService } } - private List identifyNewDataNodes(final Collection updatedDataNodes, - final Collection existingFragmentEntities) { - final Set existingXpaths = existingFragmentEntities.stream() - .map(FragmentEntity::getXpath) - .collect(Collectors.toSet()); - return updatedDataNodes.stream() - .filter(dataNode -> !existingXpaths.contains(dataNode.getXpath())) - .collect(Collectors.toList()); - } - private void retryUpdateDataNodesIndividually(final AnchorEntity anchorEntity, final Collection fragmentEntities) { final Collection failedXpaths = new HashSet<>(); @@ -712,4 +688,14 @@ public class CpsDataPersistenceServiceImpl implements CpsDataPersistenceService } } + private static void logMissingXPaths(final Collection xpaths, + final Collection existingFragmentEntities) { + final Set existingXPaths = + existingFragmentEntities.stream().map(FragmentEntity::getXpath).collect(Collectors.toSet()); + final Set missingXPaths = + xpaths.stream().filter(xpath -> !existingXPaths.contains(xpath)).collect(Collectors.toSet()); + if (!missingXPaths.isEmpty()) { + log.warn("Cannot update data nodes: Target XPaths {} not found in DB.", missingXPaths); + } + } } diff --git a/cps-ri/src/test/groovy/org/onap/cps/ri/CpsDataPersistenceServiceImplSpec.groovy b/cps-ri/src/test/groovy/org/onap/cps/ri/CpsDataPersistenceServiceImplSpec.groovy index 86097be516..7fa9b2e755 100644 --- a/cps-ri/src/test/groovy/org/onap/cps/ri/CpsDataPersistenceServiceImplSpec.groovy +++ b/cps-ri/src/test/groovy/org/onap/cps/ri/CpsDataPersistenceServiceImplSpec.groovy @@ -2,7 +2,7 @@ * ============LICENSE_START======================================================= * Copyright (c) 2021 Bell Canada. * Modifications Copyright (C) 2021-2023 Nordix Foundation - * Modifications Copyright (C) 2022-2025 TechMahindra Ltd. + * Modifications Copyright (C) 2022-2023 TechMahindra Ltd. * ================================================================================ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -256,28 +256,6 @@ class CpsDataPersistenceServiceImplSpec extends Specification { }}) } - def 'Replace data nodes and add new list item'() { - given: 'The fragment repository returns existing fragment for known XPath' - def existingFragmentEntity = new FragmentEntity(id: 1, xpath: '/bookstore/categories[@code=1]/books[@title="Matilda"]', attributes: '{"title":"Matilda","lang":"English"}', anchor: anchorEntity, childFragments: [] as Set) - mockFragmentRepository.findByAnchorAndXpathIn(_, _ as Set) >> [existingFragmentEntity] - and: 'a data node that represents replacing an attribute from an existing one' - def dataNode1 = new DataNode(xpath: '/bookstore/categories[@code=1]/books[@title="Matilda"]', leaves: ['title': 'Matilda', 'lang': 'French']) - and: 'a data node that represents adding a new list item' - def dataNode2 = new DataNode(xpath: '/bookstore/categories[@code=1]/books[@title="Book 2"]', leaves: ['title': 'Book 2', 'lang': 'English']) - when: 'the fragment entities are updated by the data nodes' - def result = objectUnderTest.updateDataNodesAndDescendants('dataspace', 'anchor', [dataNode1, dataNode2]) - then: 'fragment repository saves the updated new fragments' - 1 * mockFragmentRepository.saveAll({ fragmentEntities -> - { - assert fragmentEntities.size() == 2 - def fragmentEntityPerXpath = fragmentEntities.collectEntries { [it.xpath, it] } - assert fragmentEntityPerXpath.get('/bookstore/categories[@code=1]/books[@title="Matilda"]').attributes.contains('"lang":"French"') - assert fragmentEntityPerXpath.get('/bookstore/categories[@code=1]/books[@title="Book 2"]').attributes.contains('"lang":"English"') - }}) - and: 'result confirms new nodes were inserted' - assert result - } - def createDataNodeAndMockRepositoryMethodSupportingIt(xpath, scenario) { def dataNode = new DataNodeBuilder().withXpath(xpath).build() def fragmentEntity = new FragmentEntity(xpath: xpath, childFragments: []) diff --git a/cps-service/src/main/java/org/onap/cps/api/CpsDataService.java b/cps-service/src/main/java/org/onap/cps/api/CpsDataService.java index a0b79462e9..43c3660b4d 100644 --- a/cps-service/src/main/java/org/onap/cps/api/CpsDataService.java +++ b/cps-service/src/main/java/org/onap/cps/api/CpsDataService.java @@ -147,11 +147,9 @@ public interface CpsDataService { * @param nodeData node data * @param observedTimestamp observedTimestamp * @param contentType JSON/XML content type - * @return true if new data nodes were created during the update; - * false if only existing nodes were updated or no changes were applied */ - boolean updateDataNodeAndDescendants(String dataspaceName, String anchorName, String parentNodeXpath, - String nodeData, OffsetDateTime observedTimestamp, ContentType contentType); + void updateDataNodeAndDescendants(String dataspaceName, String anchorName, String parentNodeXpath, String nodeData, + OffsetDateTime observedTimestamp, ContentType contentType); /** * Replaces multiple existing data nodes' content including descendants in a batch operation. diff --git a/cps-service/src/main/java/org/onap/cps/impl/CpsDataServiceImpl.java b/cps-service/src/main/java/org/onap/cps/impl/CpsDataServiceImpl.java index f680c62a46..ec839e22d6 100644 --- a/cps-service/src/main/java/org/onap/cps/impl/CpsDataServiceImpl.java +++ b/cps-service/src/main/java/org/onap/cps/impl/CpsDataServiceImpl.java @@ -197,17 +197,15 @@ public class CpsDataServiceImpl implements CpsDataService { @Override @Timed(value = "cps.data.service.datanode.descendants.update", description = "Time taken to update a data node and descendants") - public boolean updateDataNodeAndDescendants(final String dataspaceName, final String anchorName, + public void updateDataNodeAndDescendants(final String dataspaceName, final String anchorName, final String parentNodeXpath, final String nodeData, final OffsetDateTime observedTimestamp, final ContentType contentType) { cpsValidator.validateNameCharacters(dataspaceName, anchorName); final Anchor anchor = cpsAnchorService.getAnchor(dataspaceName, anchorName); final Collection dataNodes = dataNodeFactory .createDataNodesWithAnchorParentXpathAndNodeData(anchor, parentNodeXpath, nodeData, contentType); - final boolean hasNewDataNodes = cpsDataPersistenceService - .updateDataNodesAndDescendants(dataspaceName, anchorName, dataNodes); + cpsDataPersistenceService.updateDataNodesAndDescendants(dataspaceName, anchorName, dataNodes); sendDataUpdatedEvent(anchor, parentNodeXpath, Operation.UPDATE, observedTimestamp); - return hasNewDataNodes; } @Override 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 7deb09a01c..551ff29372 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 @@ -3,7 +3,7 @@ * Copyright (C) 2020-2025 OpenInfra Foundation Europe. All rights reserved. * Modifications Copyright (C) 2021 Pantheon.tech * Modifications Copyright (C) 2022 Bell Canada - * Modifications Copyright (C) 2022-2025 TechMahindra Ltd. + * Modifications Copyright (C) 2022-2023 TechMahindra Ltd. * ================================================================================ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -114,12 +114,8 @@ public interface CpsDataPersistenceService { * @param dataspaceName dataspace name * @param anchorName anchor name * @param dataNodes data nodes - * @return true if new data nodes were created during the update; - * false if only existing nodes were updated or no changes were applied - * */ - boolean updateDataNodesAndDescendants(String dataspaceName, String anchorName, final Collection - dataNodes); + void updateDataNodesAndDescendants(String dataspaceName, String anchorName, final Collection dataNodes); /** * Replaces list content by removing all existing elements and inserting the given new elements diff --git a/docs/api/swagger/cps/openapi.yaml b/docs/api/swagger/cps/openapi.yaml index a9e8b439e7..ebf3c63e30 100644 --- a/docs/api/swagger/cps/openapi.yaml +++ b/docs/api/swagger/cps/openapi.yaml @@ -1829,13 +1829,6 @@ paths: schema: type: object description: OK - "201": - content: - application/json: - schema: - example: my-resource - type: string - description: Created "400": content: application/json: -- 2.16.6