From: ToineSiebelink Date: Tue, 6 Jan 2026 16:45:12 +0000 (+0000) Subject: REMOVE (#)/attribute validation based on content-type X-Git-Url: https://gerrit.onap.org/r/gitweb?a=commitdiff_plain;h=a12a5bfb3e7f1b26409f060c4c265243a65ef911;p=cps.git REMOVE (#)/attribute validation based on content-type - removed validation and test (as agreed in meeting with stakeholder today) - Add test for last missing code branch :-D (and cleaned it up!) Issue-ID: CPS-3099 Change-Id: If5f4aae7d51df9854f7212ec76148f13aaf5b6b6 Signed-off-by: ToineSiebelink --- diff --git a/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/controller/ProvMnSController.java b/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/controller/ProvMnSController.java index 54204c8521..33acffc2ae 100644 --- a/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/controller/ProvMnSController.java +++ b/cps-ncmp-rest/src/main/java/org/onap/cps/ncmp/rest/controller/ProvMnSController.java @@ -28,8 +28,6 @@ import static org.onap.cps.ncmp.impl.provmns.ParameterMapper.NO_OP; import io.netty.handler.timeout.TimeoutException; import jakarta.servlet.http.HttpServletRequest; import java.util.List; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.onap.cps.api.exceptions.DataValidationException; @@ -69,7 +67,6 @@ import org.springframework.web.bind.annotation.RestController; @Slf4j public class ProvMnSController implements ProvMnS { - private static final Pattern STANDARD_ATTRIBUTE_PATTERN = Pattern.compile("[^#]/attributes"); private static final String NO_AUTHORIZATION = null; private static final String PROVMNS_NOT_SUPPORTED_ERROR_MESSAGE = "Registered DMI does not support the ProvMnS interface."; @@ -114,9 +111,6 @@ public class ProvMnSController implements ProvMnS { throw new ProvMnSException(httpServletRequest.getMethod(), HttpStatus.PAYLOAD_TOO_LARGE, title, NO_OP); } final RequestParameters requestParameters = parameterMapper.extractRequestParameters(httpServletRequest); - for (final PatchItem patchItem : patchItems) { - validateAttributeReference(httpServletRequest.getMethod(), httpServletRequest.getContentType(), patchItem); - } try { final YangModelCmHandle yangModelCmHandle = getAndValidateYangModelCmHandle(requestParameters); checkPermissionForEachPatchItem(requestParameters, patchItems, yangModelCmHandle); @@ -235,23 +229,4 @@ public class ProvMnSController implements ProvMnS { return provMnSException; } - private void validateAttributeReference(final String httpMethodName, - final String contentType, - final PatchItem patchItem) { - final String path = patchItem.getPath(); - boolean attributesReferenceIncorrect = false; - if (path.contains("#/attributes") && "application/json-patch+json".equals(contentType)) { - attributesReferenceIncorrect = true; - } else { - final Matcher matcher = STANDARD_ATTRIBUTE_PATTERN.matcher(path); - if ("application/3gpp-json-patch+json".equals(contentType) && matcher.find()) { - attributesReferenceIncorrect = true; - } - } - if (attributesReferenceIncorrect) { - throw new ProvMnSException(httpMethodName, HttpStatus.BAD_REQUEST, - "Invalid path for content-type " + contentType, NO_OP); - } - } - } diff --git a/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/controller/ProvMnSControllerSpec.groovy b/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/controller/ProvMnSControllerSpec.groovy index b1d9f70b30..11a45563f5 100644 --- a/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/controller/ProvMnSControllerSpec.groovy +++ b/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/controller/ProvMnSControllerSpec.groovy @@ -1,6 +1,6 @@ /* * ============LICENSE_START======================================================= - * Copyright (C) 2025 OpenInfra Foundation Europe. All rights reserved. + * Copyright (C) 2025-2026 OpenInfra Foundation Europe. All rights reserved. * ================================================================================ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -366,27 +366,4 @@ class ProvMnSControllerSpec extends Specification { assert response.contentAsString.contains('"title":"/myClass=id1 not found"') } - def 'Patch request with invalid attribute reference: #scenario.'() { - given: 'provMns url' - def provMnsUrl = "$provMnSBasePath/v1/myClass=id1" - and: 'alternate Id can be matched' - mockAlternateIdMatcher.getCmHandleIdByLongestMatchingAlternateId('/myClass=id1', "/") >> 'ch-1' - and: 'persistence service returns yangModelCmHandle' - mockInventoryPersistence.getYangModelCmHandle('ch-1') >> new YangModelCmHandle(id:'ch-1', dmiServiceName: 'someDmiService', dataProducerIdentifier: 'someDataProducerId', compositeState: new CompositeState(cmHandleState: READY)) - and: 'dmi provides a response' - mockDmiRestClient.synchronousPatchOperation(*_) >> new ResponseEntity<>('Response from DMI service', HttpStatus.OK) - when: 'patch request is performed' - def response = mvc.perform(patch(provMnsUrl) - .contentType(contentType) - .content('[{"op":"replace","path":"className' + attributeReference + '"}]')) - .andReturn().response - then: 'response status is BAD REQUEST (400)' - assert response.status == HttpStatus.BAD_REQUEST.value() - and: 'response contains expected message for error case' - assert response.contentAsString.contains('"title":"Invalid path for content-type ' + contentType + '"') - where: 'following scenarios' - scenario | contentType | attributeReference - '3gpp without hash' | 'application/3gpp-json-patch+json' | '/attributes/attr1' - 'standard with hash' | "application/json-patch+json" | '#/attributes/attr1' - } } diff --git a/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/util/CmHandleStateMapperSpec.groovy b/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/util/CmHandleStateMapperSpec.groovy index c01b9508b1..82816c51b4 100644 --- a/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/util/CmHandleStateMapperSpec.groovy +++ b/cps-ncmp-rest/src/test/groovy/org/onap/cps/ncmp/rest/util/CmHandleStateMapperSpec.groovy @@ -1,6 +1,6 @@ /* * ============LICENSE_START======================================================= - * Copyright (C) 2022-2023 Nordix Foundation + * Copyright (C) 2022-2026 OpenInfra Foundation Europe * ================================================================================ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,6 +21,7 @@ package org.onap.cps.ncmp.rest.util import org.mapstruct.factory.Mappers +import org.onap.cps.ncmp.api.inventory.models.CompositeState import org.onap.cps.ncmp.api.inventory.models.CompositeStateBuilder import org.onap.cps.ncmp.api.inventory.DataStoreSyncState import org.onap.cps.ncmp.api.inventory.models.CmHandleState @@ -36,8 +37,7 @@ import static org.onap.cps.ncmp.api.inventory.models.LockReasonCategory.MODULE_S class CmHandleStateMapperSpec extends Specification { - def formattedDateAndTime = DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss.SSSZ") - .format(OffsetDateTime.of(2022, 12, 31, 20, 30, 40, 1, ZoneOffset.UTC)) + def formattedDateAndTime = DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss.SSSZ").format(OffsetDateTime.of(2022, 12, 31, 20, 30, 40, 1, ZoneOffset.UTC)) def objectUnderTest = Mappers.getMapper(CmHandleStateMapper) def 'Composite State to CmHandleCompositeState'() { @@ -61,24 +61,30 @@ class CmHandleStateMapperSpec extends Specification { assert result.dataSyncState.operational.getSyncState() != null } - def 'Handling null state.'() { + def 'Convert composite datastore that is null.'() { expect: 'converting null returns null' - CmHandleStateMapper.toDataStores(null) == null + assert CmHandleStateMapper.toDataStores(null) == null } - def 'Internal to External Lock Reason Mapping of #scenario'() { + def 'Convert empty composite datastore.'() { + when: 'converting a empty data stores object' + def result = CmHandleStateMapper.toDataStores(new CompositeState.DataStores()) + then: 'result not null but both datastores are null' + assert result.operational == null + assert result.running == null + } + + def 'Internal to External Lock Reason Mapping of #scenario.'() { given: 'a LOCKED composite state with locked reason of #scenario' - def compositeState = new CompositeStateBuilder() - .withCmHandleState(CmHandleState.LOCKED) - .withLockReason(lockReason, '').build() + def compositeState = new CompositeStateBuilder().withCmHandleState(CmHandleState.LOCKED).withLockReason(lockReason, '').build() when: 'the composite state is mapped to a CMHandle composite state' - def result = objectUnderTest.toCmHandleCompositeStateExternalLockReason(compositeState) + def result = objectUnderTest.toCmHandleCompositeStateExternalLockReason(compositeState) then: 'the composite state contains the expected lock Reason and details' - result.getLockReason().getReason() == (expectedExternalLockReason as String) - where: - scenario | lockReason || expectedExternalLockReason - 'MODULE_SYNC_FAILED' | MODULE_SYNC_FAILED || MODULE_SYNC_FAILED - 'null value' | null || LOCKED_MISBEHAVING + assert result.getLockReason().getReason() == (expectedExternalLockReason as String) + where: 'following lock states are used' + scenario | lockReason || expectedExternalLockReason + 'MODULE_SYNC_FAILED' | MODULE_SYNC_FAILED || MODULE_SYNC_FAILED + 'null value' | null || LOCKED_MISBEHAVING } }