From be2000ec2d21151b42cb559ef881695eb32e35e9 Mon Sep 17 00:00:00 2001 From: PatrikBuhr Date: Mon, 16 Mar 2020 12:33:29 +0100 Subject: [PATCH] Handling of A1 STD 1.1 Handling of Http status and bodies from A1 error responses so that the information is passed nack to the agent NBI. Change-Id: Ia3e571b37d4a425410f21168d04f5ef442d14607 Issue-ID: NONRTRIC-155 Signed-off-by: PatrikBuhr --- .../org/oransc/policyagent/clients/A1Client.java | 1 + .../policyagent/clients/A1ClientFactory.java | 54 ++++--- .../{StdA1Client.java => StdA1ClientVersion1.java} | 6 +- .../policyagent/clients/StdA1ClientVersion2.java | 54 +++++++ .../policyagent/controllers/PolicyController.java | 51 +++++-- .../policyagent/exceptions/ServiceException.java | 1 - .../org/oransc/policyagent/repository/Lock.java | 24 ++-- .../oransc/policyagent/tasks/RicSupervision.java | 7 +- .../policyagent/tasks/RicSynchronizationTask.java | 2 +- .../policyagent/tasks/ServiceSupervision.java | 2 +- .../org/oransc/policyagent/ApplicationTest.java | 92 ++++++++++-- .../policyagent/clients/A1ClientFactoryTest.java | 157 ++++++--------------- .../policyagent/clients/StdA1ClientTest.java | 4 +- .../policyagent/utils/MockA1ClientFactory.java | 8 +- 14 files changed, 277 insertions(+), 186 deletions(-) rename policy-agent/src/main/java/org/oransc/policyagent/clients/{StdA1Client.java => StdA1ClientVersion1.java} (95%) create mode 100644 policy-agent/src/main/java/org/oransc/policyagent/clients/StdA1ClientVersion2.java diff --git a/policy-agent/src/main/java/org/oransc/policyagent/clients/A1Client.java b/policy-agent/src/main/java/org/oransc/policyagent/clients/A1Client.java index ace486b9..4936893c 100644 --- a/policy-agent/src/main/java/org/oransc/policyagent/clients/A1Client.java +++ b/policy-agent/src/main/java/org/oransc/policyagent/clients/A1Client.java @@ -32,6 +32,7 @@ public interface A1Client { public enum A1ProtocolType { UNKNOWN, // STD_V1, // + STD_V1_1, // version 1.1 OSC_V1, // SDNC_OSC, // SDNC_ONAP diff --git a/policy-agent/src/main/java/org/oransc/policyagent/clients/A1ClientFactory.java b/policy-agent/src/main/java/org/oransc/policyagent/clients/A1ClientFactory.java index 636a69f6..8f9169dd 100644 --- a/policy-agent/src/main/java/org/oransc/policyagent/clients/A1ClientFactory.java +++ b/policy-agent/src/main/java/org/oransc/policyagent/clients/A1ClientFactory.java @@ -22,6 +22,7 @@ package org.oransc.policyagent.clients; import org.oransc.policyagent.clients.A1Client.A1ProtocolType; import org.oransc.policyagent.configuration.ApplicationConfig; +import org.oransc.policyagent.exceptions.ServiceException; import org.oransc.policyagent.repository.Ric; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -61,27 +62,40 @@ public class A1ClientFactory { */ public Mono createA1Client(Ric ric) { return getProtocolVersion(ric) // - .flatMap(version -> createA1Client(ric, version)); + .flatMap(version -> createA1ClientMono(ric, version)); } - private Mono createA1Client(Ric ric, A1ProtocolType version) { + A1Client createClient(Ric ric, A1ProtocolType version) { if (version == A1ProtocolType.STD_V1) { - return Mono.just(createStdA1ClientImpl(ric)); + return new StdA1ClientVersion1(ric.getConfig()); + } else if (version == A1ProtocolType.STD_V1_1) { + return new StdA1ClientVersion2(ric.getConfig()); } else if (version == A1ProtocolType.OSC_V1) { - return Mono.just(createOscA1Client(ric)); + return new OscA1Client(ric.getConfig()); } else if (version == A1ProtocolType.SDNC_OSC) { - return Mono.just(createSdncOscA1Client(ric)); - } else { // A1ProtocolType.SDNC_ONAP - return Mono.just(createSdncOnapA1Client(ric)); + return new SdncOscA1Client(ric.getConfig(), appConfig.getA1ControllerBaseUrl(), + appConfig.getA1ControllerUsername(), appConfig.getA1ControllerPassword()); + } else if (version == A1ProtocolType.SDNC_ONAP) { + return new SdncOnapA1Client(ric.getConfig(), appConfig.getA1ControllerBaseUrl(), + appConfig.getA1ControllerUsername(), appConfig.getA1ControllerPassword()); + } else { + logger.error("Unhandled protocol: {}", version); + return null; } } + private Mono createA1ClientMono(Ric ric, A1ProtocolType version) { + A1Client c = createClient(ric, version); + return c != null ? Mono.just(c) : Mono.error(new ServiceException("Unhandled protocol: " + version)); + } + private Mono getProtocolVersion(Ric ric) { if (ric.getProtocolVersion() == A1ProtocolType.UNKNOWN) { - return fetchVersion(createSdncOnapA1Client(ric)) // - .onErrorResume(notUsed -> fetchVersion(createSdncOscA1Client(ric))) // - .onErrorResume(notUsed -> fetchVersion(createOscA1Client(ric))) // - .onErrorResume(notUsed -> fetchVersion(createStdA1ClientImpl(ric))) // + return fetchVersion(createClient(ric, A1ProtocolType.STD_V1)) // + .onErrorResume(notUsed -> fetchVersion(createClient(ric, A1ProtocolType.STD_V1_1))) // + .onErrorResume(notUsed -> fetchVersion(createClient(ric, A1ProtocolType.OSC_V1))) // + .onErrorResume(notUsed -> fetchVersion(createClient(ric, A1ProtocolType.SDNC_OSC))) // + .onErrorResume(notUsed -> fetchVersion(createClient(ric, A1ProtocolType.SDNC_ONAP))) // .doOnNext(ric::setProtocolVersion) .doOnNext(version -> logger.debug("Recover ric: {}, protocol version:{}", ric.name(), version)) // .doOnError(notUsed -> logger.warn("Could not get protocol version from RIC: {}", ric.name())); // @@ -90,24 +104,6 @@ public class A1ClientFactory { } } - protected A1Client createOscA1Client(Ric ric) { - return new OscA1Client(ric.getConfig()); - } - - protected A1Client createStdA1ClientImpl(Ric ric) { - return new StdA1Client(ric.getConfig()); - } - - protected A1Client createSdncOscA1Client(Ric ric) { - return new SdncOscA1Client(ric.getConfig(), appConfig.getA1ControllerBaseUrl(), - appConfig.getA1ControllerUsername(), appConfig.getA1ControllerPassword()); - } - - protected A1Client createSdncOnapA1Client(Ric ric) { - return new SdncOnapA1Client(ric.getConfig(), appConfig.getA1ControllerBaseUrl(), - appConfig.getA1ControllerUsername(), appConfig.getA1ControllerPassword()); - } - private Mono fetchVersion(A1Client a1Client) { return Mono.just(a1Client) // .flatMap(client -> a1Client.getProtocolVersion()); diff --git a/policy-agent/src/main/java/org/oransc/policyagent/clients/StdA1Client.java b/policy-agent/src/main/java/org/oransc/policyagent/clients/StdA1ClientVersion1.java similarity index 95% rename from policy-agent/src/main/java/org/oransc/policyagent/clients/StdA1Client.java rename to policy-agent/src/main/java/org/oransc/policyagent/clients/StdA1ClientVersion1.java index 73adecee..f8ea2ee9 100644 --- a/policy-agent/src/main/java/org/oransc/policyagent/clients/StdA1Client.java +++ b/policy-agent/src/main/java/org/oransc/policyagent/clients/StdA1ClientVersion1.java @@ -29,7 +29,7 @@ import org.springframework.web.util.UriComponentsBuilder; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; -public class StdA1Client implements A1Client { +public class StdA1ClientVersion1 implements A1Client { private static final String URL_PREFIX = "/A1-P/v1"; private static final String POLICY_TYPES_URI = "/policytypes"; @@ -51,12 +51,12 @@ public class StdA1Client implements A1Client { private final AsyncRestClient restClient; - public StdA1Client(RicConfig ricConfig) { + public StdA1ClientVersion1(RicConfig ricConfig) { String baseUrl = ricConfig.baseUrl() + URL_PREFIX; this.restClient = new AsyncRestClient(baseUrl); } - public StdA1Client(AsyncRestClient restClient) { + public StdA1ClientVersion1(AsyncRestClient restClient) { this.restClient = restClient; } diff --git a/policy-agent/src/main/java/org/oransc/policyagent/clients/StdA1ClientVersion2.java b/policy-agent/src/main/java/org/oransc/policyagent/clients/StdA1ClientVersion2.java new file mode 100644 index 00000000..cd8c6849 --- /dev/null +++ b/policy-agent/src/main/java/org/oransc/policyagent/clients/StdA1ClientVersion2.java @@ -0,0 +1,54 @@ +/*- + * ========================LICENSE_START================================= + * O-RAN-SC + * %% + * Copyright (C) 2019 Nordix Foundation + * %% + * 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.oransc.policyagent.clients; + +import java.util.Arrays; +import java.util.List; + +import org.oransc.policyagent.configuration.RicConfig; +import reactor.core.publisher.Mono; + +public class StdA1ClientVersion2 extends StdA1ClientVersion1 { + + public StdA1ClientVersion2(RicConfig ricConfig) { + super(ricConfig); + } + + public StdA1ClientVersion2(AsyncRestClient restClient) { + super(restClient); + } + + @Override + public Mono> getPolicyTypeIdentities() { + return Mono.just(Arrays.asList("")); + } + + @Override + public Mono getPolicyTypeSchema(String policyTypeId) { + return Mono.just("{}"); + } + + @Override + public Mono getProtocolVersion() { + return getPolicyIdentities() // + .flatMap(x -> Mono.just(A1ProtocolType.STD_V1_1)); + } +} diff --git a/policy-agent/src/main/java/org/oransc/policyagent/controllers/PolicyController.java b/policy-agent/src/main/java/org/oransc/policyagent/controllers/PolicyController.java index 987cd97a..bc3aa81c 100644 --- a/policy-agent/src/main/java/org/oransc/policyagent/controllers/PolicyController.java +++ b/policy-agent/src/main/java/org/oransc/policyagent/controllers/PolicyController.java @@ -32,6 +32,8 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; +import lombok.Getter; + import org.oransc.policyagent.clients.A1ClientFactory; import org.oransc.policyagent.exceptions.ServiceException; import org.oransc.policyagent.repository.ImmutablePolicy; @@ -53,12 +55,24 @@ import org.springframework.web.bind.annotation.PutMapping; import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestParam; import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.reactive.function.client.WebClientResponseException; import reactor.core.publisher.Mono; @RestController @Api(tags = "A1 Policy Management") public class PolicyController { + public static class RejectionException extends Exception { + private static final long serialVersionUID = 1L; + @Getter + private final HttpStatus status; + + public RejectionException(String message, HttpStatus status) { + super(message); + this.status = status; + } + } + @Autowired private Rics rics; @Autowired @@ -177,7 +191,8 @@ public class PolicyController { .flatMap(client -> client.deletePolicy(policy)) // .doOnNext(notUsed -> ric.getLock().unlockBlocking()) // .doOnError(notUsed -> ric.getLock().unlockBlocking()) // - .flatMap(notUsed -> Mono.just(new ResponseEntity<>(HttpStatus.NO_CONTENT))); + .flatMap(notUsed -> Mono.just(new ResponseEntity<>(HttpStatus.NO_CONTENT))) + .onErrorResume(this::handleException); } catch (ServiceException e) { return Mono.just(new ResponseEntity<>(HttpStatus.NOT_FOUND)); } @@ -190,10 +205,10 @@ public class PolicyController { @ApiResponse(code = 201, message = "Policy created", response = Object.class), // @ApiResponse(code = 200, message = "Policy updated", response = Object.class), // @ApiResponse(code = 423, message = "RIC is locked", response = String.class), // - @ApiResponse(code = 404, message = "RIC or policy type is not found", response = String.class), // - @ApiResponse(code = 405, message = "Change is not allowed", response = String.class)}) + @ApiResponse(code = 404, message = "RIC or policy type is not found", response = String.class) // + }) public Mono> putPolicy( // - @RequestParam(name = "type", required = true) String typeName, // + @RequestParam(name = "type", required = false, defaultValue = "") String typeName, // @RequestParam(name = "instance", required = true) String instanceId, // @RequestParam(name = "ric", required = true) String ricName, // @RequestParam(name = "service", required = true) String service, // @@ -223,20 +238,39 @@ public class PolicyController { .doOnNext(notUsed -> ric.getLock().unlockBlocking()) // .doOnError(t -> ric.getLock().unlockBlocking()) // .flatMap(notUsed -> Mono.just(new ResponseEntity<>(isCreate ? HttpStatus.CREATED : HttpStatus.OK))) // - .onErrorResume(t -> Mono.just(new ResponseEntity<>(t.getMessage(), HttpStatus.METHOD_NOT_ALLOWED))); + .onErrorResume(this::handleException); } return ric == null || type == null ? Mono.just(new ResponseEntity<>(HttpStatus.NOT_FOUND)) : Mono.just(new ResponseEntity<>(HttpStatus.LOCKED)); // Recovering } + @SuppressWarnings({"unchecked"}) + private Mono> createResponseEntity(String message, HttpStatus status) { + ResponseEntity re = new ResponseEntity<>((T) message, status); + return Mono.just(re); + } + + private Mono> handleException(Throwable throwable) { + if (throwable instanceof WebClientResponseException) { + WebClientResponseException e = (WebClientResponseException) throwable; + return createResponseEntity(e.getResponseBodyAsString(), e.getStatusCode()); + } else if (throwable instanceof RejectionException) { + RejectionException e = (RejectionException) throwable; + return createResponseEntity(e.getMessage(), e.getStatus()); + } else { + return createResponseEntity(throwable.getMessage(), HttpStatus.INTERNAL_SERVER_ERROR); + } + } + private Mono validateModifiedPolicy(Policy policy) { // Check that ric is not updated Policy current = this.policies.get(policy.id()); if (current != null && !current.ric().name().equals(policy.ric().name())) { - return Mono.error(new Exception("Policy cannot change RIC, policyId: " + current.id() + // + RejectionException e = new RejectionException("Policy cannot change RIC, policyId: " + current.id() + // ", RIC name: " + current.ric().name() + // - ", new name: " + policy.ric().name())); + ", new name: " + policy.ric().name(), HttpStatus.CONFLICT); + return Mono.error(e); } return Mono.just("OK"); } @@ -297,7 +331,8 @@ public class PolicyController { return a1ClientFactory.createA1Client(policy.ric()) // .flatMap(client -> client.getPolicyStatus(policy)) // - .flatMap(status -> Mono.just(new ResponseEntity<>(status, HttpStatus.OK))); + .flatMap(status -> Mono.just(new ResponseEntity<>(status, HttpStatus.OK))) + .onErrorResume(this::handleException); } catch (ServiceException e) { return Mono.just(new ResponseEntity<>(e.getMessage(), HttpStatus.NOT_FOUND)); } diff --git a/policy-agent/src/main/java/org/oransc/policyagent/exceptions/ServiceException.java b/policy-agent/src/main/java/org/oransc/policyagent/exceptions/ServiceException.java index c3443ede..c5b604e2 100644 --- a/policy-agent/src/main/java/org/oransc/policyagent/exceptions/ServiceException.java +++ b/policy-agent/src/main/java/org/oransc/policyagent/exceptions/ServiceException.java @@ -29,5 +29,4 @@ public class ServiceException extends Exception { public ServiceException(String message, Exception originalException) { super(message, originalException); } - } diff --git a/policy-agent/src/main/java/org/oransc/policyagent/repository/Lock.java b/policy-agent/src/main/java/org/oransc/policyagent/repository/Lock.java index 70d00cb0..9f02f089 100644 --- a/policy-agent/src/main/java/org/oransc/policyagent/repository/Lock.java +++ b/policy-agent/src/main/java/org/oransc/policyagent/repository/Lock.java @@ -59,11 +59,16 @@ public class Lock { @Override public void run() { - while (true) { - for (LockRequest request : consume()) { - request.callback.success(request.lock); + try { + while (true) { + for (LockRequest request : consume()) { + request.callback.success(request.lock); + } + waitForNewEntries(); } - waitForNewEntries(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + logger.error("Interrupted {}", e.getMessage()); } } @@ -74,14 +79,9 @@ public class Lock { } @SuppressWarnings("java:S2274") - private synchronized void waitForNewEntries() { - try { - if (this.lockRequestQueue.isEmpty()) { - this.wait(); - } - } catch (InterruptedException e) { - logger.warn("waitForUnlock interrupted", e); - Thread.currentThread().interrupt(); + private synchronized void waitForNewEntries() throws InterruptedException { + if (this.lockRequestQueue.isEmpty()) { + this.wait(); } } } diff --git a/policy-agent/src/main/java/org/oransc/policyagent/tasks/RicSupervision.java b/policy-agent/src/main/java/org/oransc/policyagent/tasks/RicSupervision.java index d6013de7..d368fc46 100644 --- a/policy-agent/src/main/java/org/oransc/policyagent/tasks/RicSupervision.java +++ b/policy-agent/src/main/java/org/oransc/policyagent/tasks/RicSupervision.java @@ -47,6 +47,7 @@ import reactor.core.publisher.Mono; */ @Component @EnableScheduling +@SuppressWarnings("squid:S2629") // Invoke method(s) only conditionally public class RicSupervision { private static final Logger logger = LoggerFactory.getLogger(RicSupervision.class); @@ -72,9 +73,11 @@ public class RicSupervision { @Scheduled(fixedRate = 1000 * 60) public void checkAllRics() { logger.debug("Checking Rics starting"); - createTask().subscribe(ric -> logger.debug("Ric: {} checked", ric.ric.name()), // + createTask().subscribe( // + ric -> logger.debug("Ric: {} checked", ric.ric.name()), // null, // - () -> logger.debug("Checking Rics completed")); + () -> logger.debug("Checking Rics completed") // + ); } private Flux createTask() { diff --git a/policy-agent/src/main/java/org/oransc/policyagent/tasks/RicSynchronizationTask.java b/policy-agent/src/main/java/org/oransc/policyagent/tasks/RicSynchronizationTask.java index 05812d43..a42135ef 100644 --- a/policy-agent/src/main/java/org/oransc/policyagent/tasks/RicSynchronizationTask.java +++ b/policy-agent/src/main/java/org/oransc/policyagent/tasks/RicSynchronizationTask.java @@ -55,7 +55,7 @@ import reactor.core.publisher.Mono; *

* Notify subscribing services */ -@SuppressWarnings("squid:S2629") // Invoke method(s) only conditionally. +@SuppressWarnings("squid:S2629") // Invoke method(s) only conditionally public class RicSynchronizationTask { private static final Logger logger = LoggerFactory.getLogger(RicSynchronizationTask.class); diff --git a/policy-agent/src/main/java/org/oransc/policyagent/tasks/ServiceSupervision.java b/policy-agent/src/main/java/org/oransc/policyagent/tasks/ServiceSupervision.java index 4be26eb9..26509925 100644 --- a/policy-agent/src/main/java/org/oransc/policyagent/tasks/ServiceSupervision.java +++ b/policy-agent/src/main/java/org/oransc/policyagent/tasks/ServiceSupervision.java @@ -47,6 +47,7 @@ import reactor.core.publisher.Mono; */ @Component @EnableScheduling +@SuppressWarnings("squid:S2629") // Invoke method(s) only conditionally public class ServiceSupervision { private static final Logger logger = LoggerFactory.getLogger(ServiceSupervision.class); private final Services services; @@ -116,7 +117,6 @@ public class ServiceSupervision { .map(nothing -> policy)); } - @SuppressWarnings("squid:S2629") // Invoke method(s) only conditionally private Mono handleDeleteFromRicFailure(Policy policy, Throwable e) { logger.warn("Could not delete policy: {} from ric: {}", policy.id(), policy.ric().name(), e); return Mono.empty(); diff --git a/policy-agent/src/test/java/org/oransc/policyagent/ApplicationTest.java b/policy-agent/src/test/java/org/oransc/policyagent/ApplicationTest.java index a5bf3cb7..5b6c5ba2 100644 --- a/policy-agent/src/test/java/org/oransc/policyagent/ApplicationTest.java +++ b/policy-agent/src/test/java/org/oransc/policyagent/ApplicationTest.java @@ -23,6 +23,8 @@ package org.oransc.policyagent; import static org.assertj.core.api.Assertions.assertThat; import static org.awaitility.Awaitility.await; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doReturn; import com.google.gson.Gson; import com.google.gson.GsonBuilder; @@ -30,6 +32,7 @@ import com.google.gson.JsonArray; import com.google.gson.JsonElement; import com.google.gson.JsonParser; +import java.nio.charset.StandardCharsets; import java.time.Duration; import java.time.Instant; import java.util.ArrayList; @@ -189,11 +192,19 @@ public class ApplicationTest { @Test public void testGetRics() throws Exception { - addRic("kista_1"); - this.addPolicyType("type1", "kista_1"); + addRic("ric1"); + this.addPolicyType("type1", "ric1"); String url = "/rics?policyType=type1"; String rsp = restClient().get(url).block(); - assertThat(rsp).contains("kista_1"); + assertThat(rsp).contains("ric1"); + + // nameless type for ORAN A1 1.1 + addRic("ric2"); + this.addPolicyType("", "ric2"); + url = "/rics?policyType="; + rsp = restClient().get(url).block(); + assertThat(rsp).contains("ric2"); + assertThat(rsp).doesNotContain("ric1"); // Non existing policy type url = "/rics?policyType=XXXX"; @@ -237,9 +248,12 @@ public class ApplicationTest { } private String putPolicyUrl(String serviceName, String ricName, String policyTypeName, String policyInstanceId) { - String url = "/policy?type=" + policyTypeName + "&instance=" + policyInstanceId + "&ric=" + ricName - + "&service=" + serviceName; - return url; + if (policyTypeName.isEmpty()) { + return "/policy?instance=" + policyInstanceId + "&ric=" + ricName + "&service=" + serviceName; + } else { + return "/policy?instance=" + policyInstanceId + "&ric=" + ricName + "&service=" + serviceName + "&type=" + + policyTypeName; + } } @Test @@ -281,6 +295,61 @@ public class ApplicationTest { this.rics.getRic(ricName).setState(Ric.RicState.IDLE); } + @Test + /** + * Test that HttpStatus and body from failing REST call to A1 is passed on to + * the caller. + * + * @throws ServiceException + */ + public void testErrorFromRIC() throws ServiceException { + putService("service1"); + addPolicyType("type1", "ric1"); + + String url = putPolicyUrl("service1", "ric1", "type1", "id1"); + MockA1Client a1Client = a1ClientFactory.getOrCreateA1Client("ric1"); + HttpStatus httpStatus = HttpStatus.INTERNAL_SERVER_ERROR; + String responseBody = "Refused"; + byte[] responseBodyBytes = responseBody.getBytes(StandardCharsets.UTF_8); + + WebClientResponseException a1Exception = new WebClientResponseException(httpStatus.value(), "statusText", null, + responseBodyBytes, StandardCharsets.UTF_8, null); + doReturn(Mono.error(a1Exception)).when(a1Client).putPolicy(any()); + + // PUT Policy + testErrorCode(restClient().put(url, "{}"), httpStatus, responseBody); + + // DELETE POLICY + this.addPolicy("instance1", "type1", "service1", "ric1"); + doReturn(Mono.error(a1Exception)).when(a1Client).deletePolicy(any()); + testErrorCode(restClient().delete("/policy?instance=instance1"), httpStatus, responseBody); + + // GET STATUS + this.addPolicy("instance1", "type1", "service1", "ric1"); + doReturn(Mono.error(a1Exception)).when(a1Client).getPolicyStatus(any()); + testErrorCode(restClient().get("/policy_status?instance=instance1"), httpStatus, responseBody); + + // Check that empty response body is OK + a1Exception = new WebClientResponseException(httpStatus.value(), "", null, null, null, null); + doReturn(Mono.error(a1Exception)).when(a1Client).getPolicyStatus(any()); + testErrorCode(restClient().get("/policy_status?instance=instance1"), httpStatus); + } + + @Test + public void testPutTypelessPolicy() throws Exception { + putService("service1"); + addPolicyType("", "ric1"); + String url = putPolicyUrl("service1", "ric1", "", "id1"); + restClient().put(url, jsonString()).block(); + + String rsp = restClient().get("/policies").block(); + List info = parseList(rsp, PolicyInfo.class); + assertThat(info).size().isEqualTo(1); + PolicyInfo policyInfo = info.get(0); + assertThat(policyInfo.id.equals("id1")).isTrue(); + assertThat(policyInfo.type.equals("")).isTrue(); + } + @Test public void testRefuseToUpdatePolicy() throws Exception { // Test that only the json can be changed for a already created policy @@ -291,7 +360,7 @@ public class ApplicationTest { // Try change ric1 -> ricXXX String urlWrongRic = putPolicyUrl("service1", "ricXXX", "type1", "instance1"); - testErrorCode(restClient().put(urlWrongRic, jsonString()), HttpStatus.METHOD_NOT_ALLOWED); + testErrorCode(restClient().put(urlWrongRic, jsonString()), HttpStatus.CONFLICT); } @Test @@ -601,16 +670,21 @@ public class ApplicationTest { } private void testErrorCode(Mono request, HttpStatus expStatus) { + testErrorCode(request, expStatus, ""); + } + + private void testErrorCode(Mono request, HttpStatus expStatus, String responseContains) { StepVerifier.create(request) // .expectSubscription() // - .expectErrorMatches(t -> checkWebClientError(t, expStatus)) // + .expectErrorMatches(t -> checkWebClientError(t, expStatus, responseContains)) // .verify(); } - private boolean checkWebClientError(Throwable t, HttpStatus expStatus) { + private boolean checkWebClientError(Throwable t, HttpStatus expStatus, String responseContains) { assertTrue(t instanceof WebClientResponseException); WebClientResponseException e = (WebClientResponseException) t; assertThat(e.getStatusCode()).isEqualTo(expStatus); + assertThat(e.getResponseBodyAsString()).contains(responseContains); return true; } diff --git a/policy-agent/src/test/java/org/oransc/policyagent/clients/A1ClientFactoryTest.java b/policy-agent/src/test/java/org/oransc/policyagent/clients/A1ClientFactoryTest.java index 6122c191..a7495b9f 100644 --- a/policy-agent/src/test/java/org/oransc/policyagent/clients/A1ClientFactoryTest.java +++ b/policy-agent/src/test/java/org/oransc/policyagent/clients/A1ClientFactoryTest.java @@ -20,18 +20,13 @@ package org.oransc.policyagent.clients; -import static ch.qos.logback.classic.Level.WARN; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; -import ch.qos.logback.classic.spi.ILoggingEvent; -import ch.qos.logback.core.read.ListAppender; - import java.util.Vector; import org.junit.jupiter.api.BeforeEach; @@ -43,7 +38,6 @@ import org.oransc.policyagent.clients.A1Client.A1ProtocolType; import org.oransc.policyagent.configuration.ApplicationConfig; import org.oransc.policyagent.configuration.ImmutableRicConfig; import org.oransc.policyagent.repository.Ric; -import org.oransc.policyagent.utils.LoggingUtils; import reactor.core.publisher.Mono; import reactor.test.StepVerifier; @@ -57,16 +51,19 @@ public class A1ClientFactoryTest { private ApplicationConfig applicationConfigMock; @Mock - A1Client stdA1ClientMock; + A1Client clientMock1; + + @Mock + A1Client clientMock2; @Mock - A1Client oscA1ClientMock; + A1Client clientMock3; @Mock - A1Client sdncOscA1ClientMock; + A1Client clientMock4; @Mock - A1Client sdncOnapA1ClientMock; + A1Client clientMock5; private ImmutableRicConfig ricConfig = ImmutableRicConfig.builder().name(RIC_NAME).baseUrl("baseUrl").managedElementIds(new Vector<>()).build(); @@ -80,136 +77,66 @@ public class A1ClientFactoryTest { } @Test - public void createStd_ok() { - whenGetProtocolVersionSdncOnapA1ClientThrowException(); - whenGetProtocolVersionSdncOscA1ClientThrowException(); - whenGetProtocolVersionOscA1ClientThrowException(); - whenGetProtocolVersionStdA1ClientReturnCorrectProtocol(); + public void getProtocolVersion_ok() { + whenGetProtocolVersionThrowException(clientMock1); + whenGetProtocolVersionReturn(clientMock2, A1ProtocolType.STD_V1); + doReturn(clientMock1, clientMock2).when(factoryUnderTest).createClient(any(), any()); - StepVerifier.create(factoryUnderTest.createA1Client(ric)) // - .expectSubscription() // - .expectNext(stdA1ClientMock) // - .verifyComplete(); + A1Client client = factoryUnderTest.createA1Client(ric).block(); + assertEquals(clientMock2, client, "Not correct client returned"); assertEquals(A1ProtocolType.STD_V1, ric.getProtocolVersion(), "Not correct protocol"); } @Test - public void createOsc_ok() { - whenGetProtocolVersionSdncOnapA1ClientThrowException(); - whenGetProtocolVersionSdncOscA1ClientThrowException(); - whenGetProtocolVersionOscA1ClientReturnCorrectProtocol(); - - StepVerifier.create(factoryUnderTest.createA1Client(ric)) // - .expectSubscription() // - .expectNext(oscA1ClientMock) // - .verifyComplete(); - - assertEquals(A1ProtocolType.OSC_V1, ric.getProtocolVersion(), "Not correct protocol"); - } - - @Test - public void createSdncOsc_ok() { - whenGetProtocolVersionSdncOnapA1ClientThrowException(); - whenGetProtocolVersionSdncOscA1ClientReturnCorrectProtocol(); - - StepVerifier.create(factoryUnderTest.createA1Client(ric)) // - .expectSubscription() // - .expectNext(sdncOscA1ClientMock) // - .verifyComplete(); - - assertEquals(A1ProtocolType.SDNC_OSC, ric.getProtocolVersion(), "Not correct protocol"); - } - - @Test - public void createSdncOnap_ok() { - whenGetProtocolVersionSdncOnapA1ClientReturnCorrectProtocol(); + public void getProtocolVersion_ok_Last() { + whenGetProtocolVersionThrowException(clientMock1, clientMock2, clientMock3, clientMock4); + whenGetProtocolVersionReturn(clientMock5, A1ProtocolType.STD_V1_1); + doReturn(clientMock1, clientMock2, clientMock3, clientMock4, clientMock5).when(factoryUnderTest) + .createClient(any(), any()); - StepVerifier.create(factoryUnderTest.createA1Client(ric)) // - .expectSubscription() // - .expectNext(sdncOnapA1ClientMock) // - .verifyComplete(); + A1Client client = factoryUnderTest.createA1Client(ric).block(); - assertEquals(A1ProtocolType.SDNC_ONAP, ric.getProtocolVersion(), "Not correct protocol"); + assertEquals(clientMock5, client, "Not correct client returned"); + assertEquals(A1ProtocolType.STD_V1_1, ric.getProtocolVersion(), "Not correct protocol"); } @Test - public void createWithNoProtocol_error() { - whenGetProtocolVersionSdncOnapA1ClientThrowException(); - whenGetProtocolVersionSdncOscA1ClientThrowException(); - whenGetProtocolVersionOscA1ClientThrowException(); - whenGetProtocolVersionStdA1ClientThrowException(); + public void getProtocolVersion_error() { + whenGetProtocolVersionThrowException(clientMock1, clientMock2, clientMock3, clientMock4, clientMock5); + doReturn(clientMock1, clientMock2, clientMock3, clientMock4, clientMock5).when(factoryUnderTest) + .createClient(any(), any()); - final ListAppender logAppender = LoggingUtils.getLogListAppender(A1ClientFactory.class, WARN); StepVerifier.create(factoryUnderTest.createA1Client(ric)) // .expectSubscription() // - .expectErrorMatches( - throwable -> throwable instanceof Exception && throwable.getMessage().equals(EXCEPTION_MESSAGE)) + .expectErrorMatches(throwable -> throwable.getMessage().equals(EXCEPTION_MESSAGE)) // .verify(); - assertEquals(WARN, logAppender.list.get(0).getLevel(), "Warning not logged"); - assertTrue(logAppender.list.toString().contains("Could not get protocol version from RIC: " + RIC_NAME), - "Correct message not logged"); - assertEquals(A1ProtocolType.UNKNOWN, ric.getProtocolVersion(), "Not correct protocol"); } - @Test - public void createWithProtocolInRic_noTrialAndError() { - doReturn(stdA1ClientMock).when(factoryUnderTest).createStdA1ClientImpl(any(Ric.class)); - - ric.setProtocolVersion(A1ProtocolType.STD_V1); - - StepVerifier.create(factoryUnderTest.createA1Client(ric)) // - .expectSubscription() // - .expectNext(stdA1ClientMock) // - .verifyComplete(); - - assertEquals(A1ProtocolType.STD_V1, ric.getProtocolVersion(), "Not correct protocol"); - - verifyNoMoreInteractions(sdncOnapA1ClientMock); - verifyNoMoreInteractions(sdncOscA1ClientMock); - verifyNoMoreInteractions(oscA1ClientMock); - verifyNoMoreInteractions(stdA1ClientMock); - } - - private void whenGetProtocolVersionSdncOnapA1ClientThrowException() { - doReturn(sdncOnapA1ClientMock).when(factoryUnderTest).createSdncOnapA1Client(ric); - when(sdncOnapA1ClientMock.getProtocolVersion()).thenReturn(Mono.error(new Exception(EXCEPTION_MESSAGE))); - } - - private void whenGetProtocolVersionSdncOnapA1ClientReturnCorrectProtocol() { - doReturn(sdncOnapA1ClientMock).when(factoryUnderTest).createSdncOnapA1Client(any(Ric.class)); - when(sdncOnapA1ClientMock.getProtocolVersion()).thenReturn(Mono.just(A1ProtocolType.SDNC_ONAP)); - } - - private void whenGetProtocolVersionSdncOscA1ClientThrowException() { - doReturn(sdncOscA1ClientMock).when(factoryUnderTest).createSdncOscA1Client(any(Ric.class)); - when(sdncOscA1ClientMock.getProtocolVersion()).thenReturn(Mono.error(new Exception(EXCEPTION_MESSAGE))); - } - - private void whenGetProtocolVersionSdncOscA1ClientReturnCorrectProtocol() { - doReturn(sdncOscA1ClientMock).when(factoryUnderTest).createSdncOscA1Client(any(Ric.class)); - when(sdncOscA1ClientMock.getProtocolVersion()).thenReturn(Mono.just(A1ProtocolType.SDNC_OSC)); + private A1Client createClient(A1ProtocolType version) { + return factoryUnderTest.createClient(ric, version); } - private void whenGetProtocolVersionOscA1ClientThrowException() { - doReturn(oscA1ClientMock).when(factoryUnderTest).createOscA1Client(any(Ric.class)); - when(oscA1ClientMock.getProtocolVersion()).thenReturn(Mono.error(new Exception(EXCEPTION_MESSAGE))); + @Test + public void create_check_types() { + assertTrue(createClient(A1ProtocolType.STD_V1) instanceof StdA1ClientVersion1); + assertTrue(createClient(A1ProtocolType.STD_V1_1) instanceof StdA1ClientVersion2); + assertTrue(createClient(A1ProtocolType.OSC_V1) instanceof OscA1Client); + assertTrue(createClient(A1ProtocolType.SDNC_ONAP) instanceof SdncOnapA1Client); + assertTrue(createClient(A1ProtocolType.SDNC_OSC) instanceof SdncOscA1Client); + assertTrue(createClient(A1ProtocolType.UNKNOWN) == null); } - private void whenGetProtocolVersionOscA1ClientReturnCorrectProtocol() { - doReturn(oscA1ClientMock).when(factoryUnderTest).createOscA1Client(any(Ric.class)); - when(oscA1ClientMock.getProtocolVersion()).thenReturn(Mono.just(A1ProtocolType.OSC_V1)); + private void whenGetProtocolVersionThrowException(A1Client... clientMocks) { + for (A1Client clientMock : clientMocks) { + when(clientMock.getProtocolVersion()).thenReturn(Mono.error(new Exception(EXCEPTION_MESSAGE))); + } } - private void whenGetProtocolVersionStdA1ClientThrowException() { - doReturn(stdA1ClientMock).when(factoryUnderTest).createStdA1ClientImpl(any(Ric.class)); - when(stdA1ClientMock.getProtocolVersion()).thenReturn(Mono.error(new Exception(EXCEPTION_MESSAGE))); + private void whenGetProtocolVersionReturn(A1Client clientMock, A1ProtocolType protocol) { + when(clientMock.getProtocolVersion()).thenReturn(Mono.just(protocol)); } - private void whenGetProtocolVersionStdA1ClientReturnCorrectProtocol() { - doReturn(stdA1ClientMock).when(factoryUnderTest).createStdA1ClientImpl(any(Ric.class)); - when(stdA1ClientMock.getProtocolVersion()).thenReturn(Mono.just(A1ProtocolType.STD_V1)); - } } diff --git a/policy-agent/src/test/java/org/oransc/policyagent/clients/StdA1ClientTest.java b/policy-agent/src/test/java/org/oransc/policyagent/clients/StdA1ClientTest.java index d5dd3b73..68b56f3e 100644 --- a/policy-agent/src/test/java/org/oransc/policyagent/clients/StdA1ClientTest.java +++ b/policy-agent/src/test/java/org/oransc/policyagent/clients/StdA1ClientTest.java @@ -56,14 +56,14 @@ public class StdA1ClientTest { private static final String POLICY_JSON_INVALID = "\"policyId\":\"policy1\"}"; private static final String POLICY_TYPE = "typeName"; - StdA1Client clientUnderTest; + StdA1ClientVersion1 clientUnderTest; @Mock AsyncRestClient asyncRestClientMock; @BeforeEach public void init() { - clientUnderTest = new StdA1Client(asyncRestClientMock); + clientUnderTest = new StdA1ClientVersion1(asyncRestClientMock); } @Test diff --git a/policy-agent/src/test/java/org/oransc/policyagent/utils/MockA1ClientFactory.java b/policy-agent/src/test/java/org/oransc/policyagent/utils/MockA1ClientFactory.java index ec86a82f..52682fac 100644 --- a/policy-agent/src/test/java/org/oransc/policyagent/utils/MockA1ClientFactory.java +++ b/policy-agent/src/test/java/org/oransc/policyagent/utils/MockA1ClientFactory.java @@ -21,6 +21,7 @@ package org.oransc.policyagent.utils; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; import java.lang.invoke.MethodHandles; import java.util.HashMap; @@ -33,6 +34,7 @@ import org.oransc.policyagent.repository.PolicyTypes; import org.oransc.policyagent.repository.Ric; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import reactor.core.publisher.Mono; public class MockA1ClientFactory extends A1ClientFactory { private static final Logger logger = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); @@ -45,14 +47,14 @@ public class MockA1ClientFactory extends A1ClientFactory { } @Override - protected A1Client createStdA1ClientImpl(Ric ric) { - return getOrCreateA1Client(ric.name()); + public Mono createA1Client(Ric ric) { + return Mono.just(getOrCreateA1Client(ric.name())); } public MockA1Client getOrCreateA1Client(String ricName) { if (!clients.containsKey(ricName)) { logger.debug("Creating client for RIC: {}", ricName); - MockA1Client client = new MockA1Client(policyTypes); + MockA1Client client = spy(new MockA1Client(policyTypes)); clients.put(ricName, client); } return clients.get(ricName); -- 2.16.6