From 06d19ee334e77dbc06d317709d415d5564e1884f Mon Sep 17 00:00:00 2001 From: danielhanrahan Date: Wed, 21 Feb 2024 17:31:44 +0000 Subject: [PATCH] Refactor YangDataConverter.convertCmHandleToYangModel This method redundantly takes cmHandleId as a parameter, even though the DataNode representing a CM-handle is guaranteed to have the ID in a leaf called 'id'. Issue-ID: CPS-245 Signed-off-by: danielhanrahan Change-Id: Ib10097b6e13f7188fd933d6f6913110ec664273e --- .../api/impl/NetworkCmProxyCmHandleQueryServiceImpl.java | 3 +-- .../impl/NetworkCmProxyDataServicePropertyHandler.java | 3 +-- .../ncmp/api/impl/inventory/InventoryPersistenceImpl.java | 2 +- .../api/impl/inventory/sync/ModuleOperationsUtils.java | 4 +--- .../cps/ncmp/api/impl/inventory/sync/ModuleSyncTasks.java | 2 +- .../onap/cps/ncmp/api/impl/utils/YangDataConverter.java | 15 +++++---------- .../NetworkCmProxyDataServicePropertyHandlerSpec.groovy | 10 +++++----- .../impl/inventory/InventoryPersistenceImplSpec.groovy | 6 +++--- .../cps/ncmp/api/impl/utils/YangDataConverterSpec.groovy | 11 ++++++----- 9 files changed, 24 insertions(+), 32 deletions(-) diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyCmHandleQueryServiceImpl.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyCmHandleQueryServiceImpl.java index 6f92a9efe..8890d14ae 100644 --- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyCmHandleQueryServiceImpl.java +++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyCmHandleQueryServiceImpl.java @@ -241,8 +241,7 @@ public class NetworkCmProxyCmHandleQueryServiceImpl implements NetworkCmProxyCmH } private NcmpServiceCmHandle createNcmpServiceCmHandle(final DataNode dataNode) { - return convertYangModelCmHandleToNcmpServiceCmHandle(YangDataConverter - .convertCmHandleToYangModel(dataNode, dataNode.getLeaves().get("id").toString())); + return convertYangModelCmHandleToNcmpServiceCmHandle(YangDataConverter.convertCmHandleToYangModel(dataNode)); } private Collection executeQueries(final CmHandleQueryServiceParameters cmHandleQueryServiceParameters, diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServicePropertyHandler.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServicePropertyHandler.java index 84075a489..1478c86c2 100644 --- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServicePropertyHandler.java +++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServicePropertyHandler.java @@ -106,8 +106,7 @@ public class NetworkCmProxyDataServicePropertyHandler { private void updateAlternateId(final DataNode existingCmHandleDataNode, final NcmpServiceCmHandle ncmpServiceCmHandle) { final YangModelCmHandle yangModelCmHandle = - YangDataConverter.convertCmHandleToYangModel(existingCmHandleDataNode, - ncmpServiceCmHandle.getCmHandleId()); + YangDataConverter.convertCmHandleToYangModel(existingCmHandleDataNode); final String currentAlternateId = yangModelCmHandle.getAlternateId(); final String newAlternateId = ncmpServiceCmHandle.getAlternateId(); if (alternateIdChecker.canApplyAlternateId(ncmpServiceCmHandle.getCmHandleId(), diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/InventoryPersistenceImpl.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/InventoryPersistenceImpl.java index 08ab15eaa..3ae3dd911 100644 --- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/InventoryPersistenceImpl.java +++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/InventoryPersistenceImpl.java @@ -109,7 +109,7 @@ public class InventoryPersistenceImpl extends NcmpPersistenceImpl implements Inv public YangModelCmHandle getYangModelCmHandle(final String cmHandleId) { cpsValidator.validateNameCharacters(cmHandleId); final DataNode dataNode = getCmHandleDataNodeByCmHandleId(cmHandleId).iterator().next(); - return YangDataConverter.convertCmHandleToYangModel(dataNode, cmHandleId); + return YangDataConverter.convertCmHandleToYangModel(dataNode); } @Override diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleOperationsUtils.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleOperationsUtils.java index 9f95131cf..794ca5b1b 100644 --- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleOperationsUtils.java +++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleOperationsUtils.java @@ -241,9 +241,7 @@ public class ModuleOperationsUtils { private List convertCmHandlesDataNodesToYangModelCmHandles( final List cmHandlesAsDataNodeList) { - return cmHandlesAsDataNodeList.stream() - .map(cmHandle -> YangDataConverter.convertCmHandleToYangModel(cmHandle, - cmHandle.getLeaves().get("id").toString())).toList(); + return cmHandlesAsDataNodeList.stream().map(YangDataConverter::convertCmHandleToYangModel).toList(); } private boolean isRetryDue(final CompositeState.LockReason compositeStateLockReason, final OffsetDateTime time) { diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncTasks.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncTasks.java index 804653a16..18aac7a6c 100644 --- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncTasks.java +++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/inventory/sync/ModuleSyncTasks.java @@ -65,7 +65,7 @@ public class ModuleSyncTasks { for (final DataNode cmHandleAsDataNode : cmHandlesAsDataNodes) { final String cmHandleId = String.valueOf(cmHandleAsDataNode.getLeaves().get("id")); final YangModelCmHandle yangModelCmHandle = - YangDataConverter.convertCmHandleToYangModel(cmHandleAsDataNode, cmHandleId); + YangDataConverter.convertCmHandleToYangModel(cmHandleAsDataNode); final CompositeState compositeState = inventoryPersistence.getCmHandleState(cmHandleId); final boolean inUpgrade = ModuleOperationsUtils.inUpgradeOrUpgradeFailed(compositeState); try { diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/utils/YangDataConverter.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/utils/YangDataConverter.java index c77e9aa7a..395414297 100644 --- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/utils/YangDataConverter.java +++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/api/impl/utils/YangDataConverter.java @@ -20,13 +20,13 @@ package org.onap.cps.ncmp.api.impl.utils; -import java.util.ArrayList; import java.util.Collection; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.Collectors; import lombok.AccessLevel; import lombok.NoArgsConstructor; import lombok.extern.slf4j.Slf4j; @@ -76,12 +76,11 @@ public class YangDataConverter { /** * This method convert cm handle data node to yang model cm handle. * @param cmHandleDataNode the datanode of the cm handle - * @param cmHandleId the id of the cm handle * @return yang model cm handle */ - public static YangModelCmHandle convertCmHandleToYangModel(final DataNode cmHandleDataNode, - final String cmHandleId) { + public static YangModelCmHandle convertCmHandleToYangModel(final DataNode cmHandleDataNode) { final NcmpServiceCmHandle ncmpServiceCmHandle = new NcmpServiceCmHandle(); + final String cmHandleId = cmHandleDataNode.getLeaves().get("id").toString(); ncmpServiceCmHandle.setCmHandleId(cmHandleId); populateCmHandleDetails(cmHandleDataNode, ncmpServiceCmHandle); return YangModelCmHandle.toYangModelCmHandle( @@ -101,12 +100,8 @@ public class YangDataConverter { */ public static Collection convertDataNodesToYangModelCmHandles( final Collection cmHandleDataNodes) { - final Collection yangModelCmHandles = new ArrayList<>(cmHandleDataNodes.size()); - cmHandleDataNodes.forEach(dataNode -> { - final String cmHandleId = extractCmHandleIdFromXpath(dataNode.getXpath()); - yangModelCmHandles.add(convertCmHandleToYangModel(dataNode, cmHandleId)); - }); - return yangModelCmHandles; + return cmHandleDataNodes.stream().map(YangDataConverter::convertCmHandleToYangModel) + .collect(Collectors.toList()); } /** diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServicePropertyHandlerSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServicePropertyHandlerSpec.groovy index d822f2e63..e00a42674 100644 --- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServicePropertyHandlerSpec.groovy +++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/NetworkCmProxyDataServicePropertyHandlerSpec.groovy @@ -56,7 +56,7 @@ class NetworkCmProxyDataServicePropertyHandlerSpec extends Specification { new DataNodeBuilder().withXpath("/dmi-registry/cm-handles[@id='${cmHandleId}']/additional-properties[@name='additionalProp2']").withLeaves(['name': 'additionalProp2', 'value': 'additionalValue2']).build(), new DataNodeBuilder().withXpath("/dmi-registry/cm-handles[@id='${cmHandleId}']/public-properties[@name='publicProp3']").withLeaves(['name': 'publicProp3', 'value': 'publicValue3']).build(), new DataNodeBuilder().withXpath("/dmi-registry/cm-handles[@id='${cmHandleId}']/public-properties[@name='publicProp4']").withLeaves(['name': 'publicProp4', 'value': 'publicValue4']).build()] - def static cmHandleDataNodeAsCollection = [new DataNode(xpath: cmHandleXpath, childDataNodes: propertyDataNodes)] + def static cmHandleDataNodeAsCollection = [new DataNode(xpath: cmHandleXpath, childDataNodes: propertyDataNodes, leaves: ['id': cmHandleId])] def 'Update CM Handle Public Properties: #scenario'() { given: 'the CPS service return a CM handle' @@ -105,7 +105,7 @@ class NetworkCmProxyDataServicePropertyHandlerSpec extends Specification { def 'Update CM Handle Properties, remove all properties: #scenario'() { given: 'the CPS service return a CM handle' - def cmHandleDataNode = new DataNode(xpath: cmHandleXpath, childDataNodes: originalPropertyDataNodes) + def cmHandleDataNode = new DataNode(xpath: cmHandleXpath, leaves: ['id': cmHandleId], childDataNodes: originalPropertyDataNodes) mockInventoryPersistence.getCmHandleDataNodeByCmHandleId(cmHandleId) >> [cmHandleDataNode] and: 'an update cm handle request that removes all public properties(existing and non-existing)' def cmHandleUpdateRequest = [new NcmpServiceCmHandle(cmHandleId: cmHandleId, publicProperties: ['publicProp3': null, 'publicProp4': null])] @@ -182,9 +182,9 @@ class NetworkCmProxyDataServicePropertyHandlerSpec extends Specification { def 'Update CM Handle Alternate ID with #scenario'() { given: 'an existing cm handle' - DataNode existingCmHandleDataNode = new DataNode(xpath: cmHandleXpath) + DataNode existingCmHandleDataNode = new DataNode(xpath: cmHandleXpath, leaves: ['id': cmHandleId]) and: 'an update request with an alternate id' - def ncmpServiceCmHandle = new NcmpServiceCmHandle(cmHandleId: cmHandleId, alternateId: 'alt-1' ) + def ncmpServiceCmHandle = new NcmpServiceCmHandle(cmHandleId: cmHandleId, alternateId: 'alt-1') when: 'update alternate id method is called with the update request' objectUnderTest.updateAlternateId(existingCmHandleDataNode, ncmpServiceCmHandle) then: 'the update node leaves method is invoked as many times as expected' @@ -202,7 +202,7 @@ class NetworkCmProxyDataServicePropertyHandlerSpec extends Specification { def 'Alternate ID removed from cache when persisting fails.'() { given: 'an existing data node and an update request with an alternate id' def ncmpServiceCmHandle = new NcmpServiceCmHandle(cmHandleId: cmHandleId, alternateId: 'alt-1') - DataNode existingCmHandleDataNode = new DataNode(xpath: cmHandleXpath, leaves: ['alternate-id': null]) + DataNode existingCmHandleDataNode = new DataNode(xpath: cmHandleXpath, leaves: ['id': cmHandleId, 'alternate-id': null]) and: 'an applicable alternate id for the cm handle' mockAlternateIdChecker.canApplyAlternateId(cmHandleId, '','alt-1') >> true and: 'but an exception occurs while saving' diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/inventory/InventoryPersistenceImplSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/inventory/InventoryPersistenceImplSpec.groovy index 83acb2238..cbc985f78 100644 --- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/inventory/InventoryPersistenceImplSpec.groovy +++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/inventory/InventoryPersistenceImplSpec.groovy @@ -71,7 +71,7 @@ class InventoryPersistenceImplSpec extends Specification { .format(OffsetDateTime.of(2022, 12, 31, 20, 30, 40, 1, ZoneOffset.UTC)) def cmHandleId = 'some-cm-handle' - def leaves = ["dmi-service-name":"common service name","dmi-data-service-name":"data service name","dmi-model-service-name":"model service name"] + def leaves = ["id":cmHandleId,"dmi-service-name":"common service name","dmi-data-service-name":"data service name","dmi-model-service-name":"model service name"] def xpath = "/dmi-registry/cm-handles[@id='some-cm-handle']" def cmHandleId2 = 'another-cm-handle' @@ -119,7 +119,7 @@ class InventoryPersistenceImplSpec extends Specification { def "Handling missing service names as null."() { given: 'the cps data service returns a data node from the DMI registry with empty child and leaf attributes' - def dataNode = new DataNode(childDataNodes:[], leaves: [:]) + def dataNode = new DataNode(childDataNodes:[], leaves: ['id':cmHandleId]) mockCpsDataService.getDataNodes(NCMP_DATASPACE_NAME, NCMP_DMI_REGISTRY_ANCHOR, xpath, INCLUDE_ALL_DESCENDANTS) >> [dataNode] when: 'retrieving the yang modelled cm handle' def result = objectUnderTest.getYangModelCmHandle(cmHandleId) @@ -133,7 +133,7 @@ class InventoryPersistenceImplSpec extends Specification { def "Retrieve multiple YangModelCmHandles"() { given: 'the cps data service returns 2 data nodes from the DMI registry' - def dataNodes = [new DataNode(xpath: xpath), new DataNode(xpath: xpath2)] + def dataNodes = [new DataNode(xpath: xpath, leaves: ['id': cmHandleId]), new DataNode(xpath: xpath2, leaves: ['id': cmHandleId2])] mockCpsDataService.getDataNodesForMultipleXpaths(NCMP_DATASPACE_NAME, NCMP_DMI_REGISTRY_ANCHOR, [xpath, xpath2] , INCLUDE_ALL_DESCENDANTS) >> dataNodes when: 'retrieving the yang modelled cm handle' def results = objectUnderTest.getYangModelCmHandles([cmHandleId, cmHandleId2]) diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/utils/YangDataConverterSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/utils/YangDataConverterSpec.groovy index 6d733dc47..0ae348c07 100644 --- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/utils/YangDataConverterSpec.groovy +++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/api/impl/utils/YangDataConverterSpec.groovy @@ -1,6 +1,6 @@ /* * ============LICENSE_START======================================================== - * Copyright (C) 2022-2023 Nordix Foundation + * Copyright (C) 2022-2024 Nordix Foundation * ================================================================================ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -31,9 +31,10 @@ class YangDataConverterSpec extends Specification{ leaves: ['name': 'dmiProp1', 'value': 'dmiValue1']) def dataNodePublicProperties = new DataNode(xpath:'/public-properties[@name="pubProp1"]', leaves: ['name': 'pubProp1', 'value': 'pubValue1']) - def dataNodeCmHandle = new DataNode(childDataNodes:[dataNodeAdditionalProperties, dataNodePublicProperties]) + def dataNodeCmHandle = new DataNode(leaves:['id':'sample-id'], childDataNodes:[dataNodeAdditionalProperties, dataNodePublicProperties]) when: 'the dataNode is converted' - def yangModelCmHandle = YangDataConverter.convertCmHandleToYangModel(dataNodeCmHandle,'sample-id') + def yangModelCmHandle = + YangDataConverter.convertCmHandleToYangModel(dataNodeCmHandle) then: 'the converted object has the correct id' assert yangModelCmHandle.id == 'sample-id' and: 'the additional (dmi, private) properties are included' @@ -46,8 +47,8 @@ class YangDataConverterSpec extends Specification{ def 'Convert multiple cm handle data nodes'(){ given: 'two data nodes in a collection' - def dataNodes = [new DataNode(xpath:'/dmi-registry/cm-handles[@id=\'some-cm-handle\']'), - new DataNode(xpath:'/dmi-registry/cm-handles[@id=\'another-cm-handle\']')] + def dataNodes = [new DataNode(xpath:'/dmi-registry/cm-handles[@id=\'some-cm-handle\']', leaves: ['id':'some-cm-handle']), + new DataNode(xpath:'/dmi-registry/cm-handles[@id=\'another-cm-handle\']', leaves: ['id':'another-cm-handle'])] when: 'the data nodes are converted' def yangModelCmHandles = YangDataConverter.convertDataNodesToYangModelCmHandles(dataNodes) then: 'verify both have returned and CmHandleIds are correct' -- 2.16.6