From b3dfa9ba4771b3d98bbfbdb870b8ed063d4dd2ce Mon Sep 17 00:00:00 2001 From: seanbeirne Date: Thu, 20 Feb 2025 11:16:11 +0000 Subject: Change order of CM Handle Reference lookup depending on special character - implemented new algorithm using validator to rule out standard id / prefer alternate id - moved validator imp to cps-service (not in RI) TBC!!! - changed order of characters tested in validator to fail fast (on '=') - added Boolean variation validator method to reduce overhead and prevent logic based on exceptions - improved integration test to cover all scenarios - add performance test for alternate id look up (report only) - ensured all performance test use alternate ids it '=' - added small groovy tests to restore cps-ri code coverage to 0.31 Issue-ID: CPS-2605 Change-Id: Id9c22bb69904b7f5d376b7f8319332428435333e Signed-off-by: ToineSiebelink Signed-off-by: seanbeirne --- .../onap/cps/ncmp/impl/data/DmiDataOperations.java | 34 ++++++++-------------- .../cps/ncmp/impl/utils/AlternateIdMatcher.java | 19 ++++++++++-- .../ncmp/impl/data/DmiDataOperationsSpec.groovy | 32 ++------------------ .../ncmp/impl/utils/AlternateIdMatcherSpec.groovy | 21 +++++++++---- 4 files changed, 45 insertions(+), 61 deletions(-) (limited to 'cps-ncmp-service') diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/data/DmiDataOperations.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/data/DmiDataOperations.java index 189239ceb2..7cb1c44526 100644 --- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/data/DmiDataOperations.java +++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/data/DmiDataOperations.java @@ -1,6 +1,6 @@ /* * ============LICENSE_START======================================================= - * Copyright (C) 2021-2024 Nordix Foundation + * Copyright (C) 2021-2025 Nordix Foundation * Modifications Copyright (C) 2022 Bell Canada * ================================================================================ * Licensed under the Apache License, Version 2.0 (the "License"); @@ -34,7 +34,6 @@ import java.util.Set; import java.util.stream.Collectors; import lombok.RequiredArgsConstructor; import org.onap.cps.api.exceptions.CpsException; -import org.onap.cps.api.exceptions.DataValidationException; import org.onap.cps.ncmp.api.NcmpResponseStatus; import org.onap.cps.ncmp.api.data.models.CmResourceAddress; import org.onap.cps.ncmp.api.data.models.DataOperationRequest; @@ -79,7 +78,7 @@ public class DmiDataOperations { * This method fetches the resource data from the operational data store for a given CM handle * identifier on the specified resource using the DMI client. * - * @param cmResourceAddress Target datastore, CM handle, and resource identifier. + * @param cmResourceAddress Target datastore, CM handle reference, and resource identifier. * @param options Options query string. * @param topic Topic name for triggering asynchronous responses. * @param requestId Request ID for asynchronous responses. @@ -94,7 +93,8 @@ public class DmiDataOperations { final String topic, final String requestId, final String authorization) { - final YangModelCmHandle yangModelCmHandle = resolveYangModelCmHandleFromCmHandleReference(cmResourceAddress); + final YangModelCmHandle yangModelCmHandle = getYangModelCmHandle( + cmResourceAddress.resolveCmHandleReferenceToId()); final CmHandleState cmHandleState = yangModelCmHandle.getCompositeState().getCmHandleState(); validateIfCmHandleStateReady(yangModelCmHandle, cmHandleState); final String jsonRequestBody = getDmiRequestBody(READ, requestId, null, null, yangModelCmHandle); @@ -157,22 +157,22 @@ public class DmiDataOperations { * This method creates the resource data from pass-through running data store for given cm handle * identifier on given resource using dmi client. * - * @param cmHandleId network resource identifier - * @param resourceId resource identifier - * @param operationType operation enum - * @param requestData the request data - * @param dataType data type - * @param authorization contents of Authorization header, or null if not present + * @param cmHandleReference network resource identifier + * @param resourceId resource identifier + * @param operationType operation enum + * @param requestData the request data + * @param dataType data type + * @param authorization contents of Authorization header, or null if not present * @return {@code ResponseEntity} response entity */ - public ResponseEntity writeResourceDataPassThroughRunningFromDmi(final String cmHandleId, + public ResponseEntity writeResourceDataPassThroughRunningFromDmi(final String cmHandleReference, final String resourceId, final OperationType operationType, final String requestData, final String dataType, final String authorization) { final CmResourceAddress cmResourceAddress = - new CmResourceAddress(PASSTHROUGH_RUNNING.getDatastoreName(), cmHandleId, resourceId); + new CmResourceAddress(PASSTHROUGH_RUNNING.getDatastoreName(), cmHandleReference, resourceId); final YangModelCmHandle yangModelCmHandle = getYangModelCmHandle(cmResourceAddress.resolveCmHandleReferenceToId()); @@ -281,16 +281,6 @@ public class DmiDataOperations { }).subscribe(); } - private YangModelCmHandle resolveYangModelCmHandleFromCmHandleReference(final CmResourceAddress cmResourceAddress) { - String cmHandleId = cmResourceAddress.getCmHandleReference(); - try { - return getYangModelCmHandle(cmHandleId); - } catch (final DataValidationException ignored) { - cmHandleId = cmResourceAddress.resolveCmHandleReferenceToId(); - return getYangModelCmHandle(cmHandleId); - } - } - private String createDmiDataOperationRequestAsJsonString( final List dmiDataOperationRequestBodies) { final DmiDataOperationRequest dmiDataOperationRequest = DmiDataOperationRequest.builder() diff --git a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/utils/AlternateIdMatcher.java b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/utils/AlternateIdMatcher.java index b97da2977a..750a5050f2 100644 --- a/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/utils/AlternateIdMatcher.java +++ b/cps-ncmp-service/src/main/java/org/onap/cps/ncmp/impl/utils/AlternateIdMatcher.java @@ -1,6 +1,6 @@ /* * ============LICENSE_START======================================================= - * Copyright (C) 2024 Nordix Foundation + * Copyright (C) 2024-2025 Nordix Foundation * ================================================================================ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -26,6 +26,7 @@ import org.onap.cps.ncmp.api.exceptions.CmHandleNotFoundException; import org.onap.cps.ncmp.exceptions.NoAlternateIdMatchFoundException; import org.onap.cps.ncmp.impl.inventory.InventoryPersistence; import org.onap.cps.ncmp.impl.inventory.models.YangModelCmHandle; +import org.onap.cps.utils.CpsValidator; import org.springframework.stereotype.Service; @Service @@ -33,6 +34,7 @@ import org.springframework.stereotype.Service; public class AlternateIdMatcher { private final InventoryPersistence inventoryPersistence; + private final CpsValidator cpsValidator; /** * Get yang model cm handle that matches longest alternate id by removing elements @@ -64,11 +66,22 @@ public class AlternateIdMatcher { * @return cm handle id string */ public String getCmHandleId(final String cmHandleReference) { + if (cpsValidator.isValidName(cmHandleReference)) { + return getCmHandleIdTryingStandardIdFirst(cmHandleReference); + } + return getCmHandleIdByAlternateId(cmHandleReference); + } + + private String getCmHandleIdByAlternateId(final String cmHandleReference) { + // Please note: because of cm handle id validation rules this case does NOT need to try by (standard) id + return inventoryPersistence.getYangModelCmHandleByAlternateId(cmHandleReference).getId(); + } + + private String getCmHandleIdTryingStandardIdFirst(final String cmHandleReference) { if (inventoryPersistence.isExistingCmHandleId(cmHandleReference)) { return cmHandleReference; - } else { - return inventoryPersistence.getYangModelCmHandleByAlternateId(cmHandleReference).getId(); } + return inventoryPersistence.getYangModelCmHandleByAlternateId(cmHandleReference).getId(); } private String getParentPath(final String path, final String separator) { diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/data/DmiDataOperationsSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/data/DmiDataOperationsSpec.groovy index 01a08e7bb5..3dd2eabe21 100644 --- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/data/DmiDataOperationsSpec.groovy +++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/data/DmiDataOperationsSpec.groovy @@ -1,6 +1,6 @@ /* * ============LICENSE_START======================================================= - * Copyright (C) 2021-2024 Nordix Foundation + * Copyright (C) 2021-2025 Nordix Foundation * Modifications Copyright (C) 2022 Bell Canada * ================================================================================ * Licensed under the Apache License, Version 2.0 (the "License"); @@ -22,8 +22,6 @@ package org.onap.cps.ncmp.impl.data import com.fasterxml.jackson.databind.ObjectMapper -import org.onap.cps.api.exceptions.DataNodeNotFoundException -import org.onap.cps.api.exceptions.DataValidationException import org.onap.cps.events.EventsPublisher import org.onap.cps.ncmp.api.data.models.CmResourceAddress import org.onap.cps.ncmp.api.data.models.DataOperationRequest @@ -86,6 +84,7 @@ class DmiDataOperationsSpec extends DmiOperationsBaseSpec { def 'call get resource data for #expectedDataStore from DMI without topic #scenario.'() { given: 'a cm handle for #cmHandleId' + alternateIdMatcher.getCmHandleId(cmHandleId) >> cmHandleId mockYangModelCmHandleRetrieval(dmiProperties) and: 'a positive response from DMI service when it is called with the expected parameters' def responseFromDmi = Mono.just(new ResponseEntity('{some-key:some-value}', HttpStatus.OK)) @@ -206,33 +205,6 @@ class DmiDataOperationsSpec extends DmiOperationsBaseSpec { CmHandleState.ADVISED || true } - def 'Resolving cm handle references with cm handle id.'() { - given: 'a resource address with a cm handle id' - def cmResourceAddress = new CmResourceAddress('some store', 'cm-handle-id', 'some resource') - and: 'the given cm handle id is available in the inventory' - mockInventoryPersistence.getYangModelCmHandle('cm-handle-id') >> yangModelCmHandle - expect: 'resolving the cm handle id returns the cm handle' - assert objectUnderTest.resolveYangModelCmHandleFromCmHandleReference(cmResourceAddress) == yangModelCmHandle - } - - def 'Resolving cm handle references with alternate id #scenario.'() { - given: 'a resource with a alternate id' - def cmResourceAddress = new CmResourceAddress('some store', alternateId, 'some resource') - and: 'the alternate id cannot be found in the inventory directly and that results in an exception' - mockInventoryPersistence.getYangModelCmHandle(alternateId) >> { throw errorThrownDuringCmHandleIdSearch } - and: 'the alternate id can be matched to a cm handle id' - alternateIdMatcher.getCmHandleId(alternateId) >> 'cm-handle-id' - and: 'that cm handle id is available in the inventory' - mockInventoryPersistence.getYangModelCmHandle('cm-handle-id') >> yangModelCmHandle - expect: 'resolving that cm handle id returns the cm handle' - assert objectUnderTest.resolveYangModelCmHandleFromCmHandleReference(cmResourceAddress) == yangModelCmHandle - where: 'the following alternate ids are used' - scenario | alternateId | errorThrownDuringCmHandleIdSearch - 'alternate id with no special characters' | 'alternate-id' | new DataNodeNotFoundException('','') - 'alternate id with special characters' | 'alternate#id' | new DataValidationException('','') - } - - def extractDataValue(actualDataOperationCloudEvent) { return toTargetEvent(actualDataOperationCloudEvent, DataOperationEvent).data.responses[0] } diff --git a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/utils/AlternateIdMatcherSpec.groovy b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/utils/AlternateIdMatcherSpec.groovy index a6d21afd30..fe7c5e3817 100644 --- a/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/utils/AlternateIdMatcherSpec.groovy +++ b/cps-ncmp-service/src/test/groovy/org/onap/cps/ncmp/impl/utils/AlternateIdMatcherSpec.groovy @@ -1,6 +1,6 @@ /* * ============LICENSE_START======================================================= - * Copyright (C) 2024 Nordix Foundation + * Copyright (C) 2024-2025 Nordix Foundation * ================================================================================ * Licensed under the Apache License, Version 2.0 (the 'License'); * you may not use this file except in compliance with the License. @@ -24,12 +24,20 @@ import org.onap.cps.ncmp.api.exceptions.CmHandleNotFoundException import org.onap.cps.ncmp.exceptions.NoAlternateIdMatchFoundException import org.onap.cps.ncmp.impl.inventory.InventoryPersistence import org.onap.cps.ncmp.impl.inventory.models.YangModelCmHandle +import org.onap.cps.utils.CpsValidatorImpl +import org.springframework.boot.test.context.SpringBootTest +import org.springframework.test.context.ContextConfiguration import spock.lang.Specification +@SpringBootTest +@ContextConfiguration(classes = [InventoryPersistence, CpsValidatorImpl]) class AlternateIdMatcherSpec extends Specification { def mockInventoryPersistence = Mock(InventoryPersistence) - def objectUnderTest = new AlternateIdMatcher(mockInventoryPersistence) + + def mockCpsValidator = Mock(CpsValidatorImpl) + + def objectUnderTest = new AlternateIdMatcher(mockInventoryPersistence, mockCpsValidator) def setup() { given: 'cm handle in the registry with alternate id /a/b' @@ -72,13 +80,14 @@ class AlternateIdMatcherSpec extends Specification { when: 'a cmHandleCmReference is passed in' def result = objectUnderTest.getCmHandleId(cmHandleReference) then: 'the inventory persistence service returns a cm handle (or not)' - mockInventoryPersistence.isExistingCmHandleId(cmHandleReference) >> existingCmHandleIdResponse + mockCpsValidator.isValidName(cmHandleReference) >> existingCmHandleIdAndValidatorResponse + mockInventoryPersistence.isExistingCmHandleId(cmHandleReference) >> existingCmHandleIdAndValidatorResponse mockInventoryPersistence.getYangModelCmHandleByAlternateId(cmHandleReference) >> alternateIdGetResponse and: 'correct result is returned' assert result == cmHandleReference where: 'the following parameters are used' - cmHandleReference | existingCmHandleIdResponse | alternateIdGetResponse - 'ch-1' | true | '' - 'alt-1' | false | new YangModelCmHandle(id: 'alt-1') + cmHandleReference | existingCmHandleIdAndValidatorResponse | alternateIdGetResponse + 'ch-1' | true | null + 'alt=1' | false | new YangModelCmHandle(id: 'alt=1') } } -- cgit