From 8accdd881808e0086e46a7e4388fa43c7997aa72 Mon Sep 17 00:00:00 2001 From: "aravind.est" Date: Fri, 29 Sep 2023 09:06:35 +0100 Subject: [PATCH] Fix error messaging format in API response Error messaging format has been changed to be identical with all endpoints. Issue-ID: NONRTRIC-913 Signed-off-by: aravind.est Change-Id: Iba0ef643af29bbde63873890d88dad925a53c194 --- .../rappmanager/rest/RappInstanceController.java | 2 +- .../oransc/rappmanager/service/RappService.java | 62 +++++++++---------- .../rappmanager/service/RappServiceTest.java | 69 +++++++++++++++------- 3 files changed, 77 insertions(+), 56 deletions(-) diff --git a/rapp-manager-application/src/main/java/com/oransc/rappmanager/rest/RappInstanceController.java b/rapp-manager-application/src/main/java/com/oransc/rappmanager/rest/RappInstanceController.java index a6e83dc..ee48f84 100755 --- a/rapp-manager-application/src/main/java/com/oransc/rappmanager/rest/RappInstanceController.java +++ b/rapp-manager-application/src/main/java/com/oransc/rappmanager/rest/RappInstanceController.java @@ -100,7 +100,7 @@ public class RappInstanceController { } @DeleteMapping("{rapp_instance_id}") - public ResponseEntity deleteRappInstance(@PathVariable("rapp_id") String rappId, + public ResponseEntity deleteRappInstance(@PathVariable("rapp_id") String rappId, @PathVariable("rapp_instance_id") UUID rappInstanceId) { return rappCacheService.getRapp(rappId).filter(rApp -> rApp.getRappInstances().containsKey(rappInstanceId)) .map(rApp -> rappService.deleteRappInstance(rApp, rappInstanceId)).orElseThrow( diff --git a/rapp-manager-application/src/main/java/com/oransc/rappmanager/service/RappService.java b/rapp-manager-application/src/main/java/com/oransc/rappmanager/service/RappService.java index 9ddcd8b..278bcbd 100755 --- a/rapp-manager-application/src/main/java/com/oransc/rappmanager/service/RappService.java +++ b/rapp-manager-application/src/main/java/com/oransc/rappmanager/service/RappService.java @@ -53,15 +53,13 @@ public class RappService { if (acmDeployer.primeRapp(rapp) && dmeDeployer.primeRapp(rapp)) { rapp.setState(RappState.PRIMED); return ResponseEntity.ok().build(); - } else { - rapp.setState(RappState.COMMISSIONED); - return ResponseEntity.status(HttpStatus.BAD_GATEWAY).build(); } - } else { - return ResponseEntity.badRequest() - .body(String.format(STATE_TRANSITION_NOT_PERMITTED, RappState.PRIMED.name(), - rapp.getState().name())); + rapp.setState(RappState.COMMISSIONED); + return ResponseEntity.status(HttpStatus.BAD_GATEWAY).build(); } + throw new RappHandlerException(HttpStatus.BAD_REQUEST, + String.format(STATE_TRANSITION_NOT_PERMITTED, rapp.getState().name(), RappState.PRIMED.name())); + } public ResponseEntity deprimeRapp(Rapp rapp) { @@ -71,18 +69,17 @@ public class RappService { if (acmDeployer.deprimeRapp(rapp) && dmeDeployer.deprimeRapp(rapp)) { rapp.setState(RappState.COMMISSIONED); return ResponseEntity.ok().build(); - } else { - rapp.setState(RappState.PRIMED); - return ResponseEntity.status(HttpStatus.BAD_GATEWAY).build(); } + rapp.setState(RappState.PRIMED); + return ResponseEntity.status(HttpStatus.BAD_GATEWAY).build(); + } + if (!rapp.getRappInstances().isEmpty()) { + throw new RappHandlerException(HttpStatus.BAD_REQUEST, + "Unable to deprime as there are active rapp instances."); } else { - if (!rapp.getRappInstances().isEmpty()) { - return ResponseEntity.badRequest().body("Unable to deprime as there are active rapp instances,"); - } else { - return ResponseEntity.badRequest() - .body(String.format(STATE_TRANSITION_NOT_PERMITTED, RappState.COMMISSIONED.name(), - rapp.getState().name())); - } + throw new RappHandlerException(HttpStatus.BAD_REQUEST, + String.format(STATE_TRANSITION_NOT_PERMITTED, RappState.COMMISSIONED.name(), + rapp.getState().name())); } } @@ -90,15 +87,15 @@ public class RappService { if (rApp.getRappInstances().isEmpty() && rApp.getState().equals(RappState.COMMISSIONED)) { rappCacheService.deleteRapp(rApp); return ResponseEntity.ok().build(); + } + if (!rApp.getRappInstances().isEmpty()) { + throw new RappHandlerException(HttpStatus.BAD_REQUEST, + String.format("Unable to delete %s as there are active rApp instances.", rApp.getName())); } else { - if (!rApp.getRappInstances().isEmpty()) { - return ResponseEntity.badRequest() - .body("Unable to delete '" + rApp.getName() + "' as there are active rApp instances."); - } else { - return ResponseEntity.badRequest().body("Unable to delete '" + rApp.getName() - + "' as the rApp is not in COMMISSIONED state."); - } + throw new RappHandlerException(HttpStatus.BAD_REQUEST, + String.format("Unable to delete %s as the rApp is not in COMMISSIONED state.", rApp.getName())); } + } public ResponseEntity deployRappInstance(Rapp rapp, RappInstance rappInstance) { @@ -110,11 +107,11 @@ public class RappService { return ResponseEntity.accepted().build(); } return ResponseEntity.status(HttpStatus.BAD_GATEWAY).build(); - } else { - return ResponseEntity.badRequest() - .body(String.format(STATE_TRANSITION_NOT_PERMITTED, rappInstance.getState().name(), - RappInstanceState.DEPLOYED.name())); } + throw new RappHandlerException(HttpStatus.BAD_REQUEST, + String.format("Unable to deploy rApp instance %s as it is not in UNDEPLOYED state", + rappInstance.getRappInstanceId())); + } public ResponseEntity undeployRappInstance(Rapp rapp, RappInstance rappInstance) { @@ -126,14 +123,13 @@ public class RappService { return ResponseEntity.accepted().build(); } return ResponseEntity.status(HttpStatus.BAD_GATEWAY).build(); - } else { - return ResponseEntity.badRequest() - .body(String.format(STATE_TRANSITION_NOT_PERMITTED, rappInstance.getState().name(), - RappInstanceState.UNDEPLOYED.name())); } + throw new RappHandlerException(HttpStatus.BAD_REQUEST, + String.format("Unable to undeploy rApp instance %s as it is not in DEPLOYED state", + rappInstance.getRappInstanceId())); } - public ResponseEntity deleteRappInstance(Rapp rApp, UUID rappInstanceId) { + public ResponseEntity deleteRappInstance(Rapp rApp, UUID rappInstanceId) { if (rApp.getRappInstances().get(rappInstanceId).getState().equals(RappInstanceState.UNDEPLOYED)) { rappInstanceStateMachine.deleteRappInstance(rApp.getRappInstances().get(rappInstanceId)); rApp.getRappInstances().remove(rappInstanceId); diff --git a/rapp-manager-application/src/test/java/com/oransc/rappmanager/service/RappServiceTest.java b/rapp-manager-application/src/test/java/com/oransc/rappmanager/service/RappServiceTest.java index ea9d3d0..d9be9b7 100755 --- a/rapp-manager-application/src/test/java/com/oransc/rappmanager/service/RappServiceTest.java +++ b/rapp-manager-application/src/test/java/com/oransc/rappmanager/service/RappServiceTest.java @@ -24,7 +24,6 @@ import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMock import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.http.HttpStatus; -import org.springframework.http.ResponseEntity; @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) @AutoConfigureMockMvc @@ -52,6 +51,8 @@ class RappServiceTest { private final String validRappFile = "valid-rapp-package.csar"; + private final String STATE_TRANSITION_NOT_PERMITTED = "State transition from %s to %s is not permitted."; + @Test void testPrimeRapp() { @@ -67,7 +68,12 @@ class RappServiceTest { void testPrimeRappInvalidState() { Rapp rapp = Rapp.builder().rappId(UUID.randomUUID()).name("").packageName(validRappFile) .packageLocation(validCsarFileLocation).state(RappState.PRIMING).build(); - assertEquals(HttpStatus.BAD_REQUEST, rappService.primeRapp(rapp).getStatusCode()); + RappHandlerException rappHandlerException = + assertThrows(RappHandlerException.class, () -> rappService.primeRapp(rapp)); + assertEquals(HttpStatus.BAD_REQUEST, rappHandlerException.getStatusCode()); + assertEquals(String.format(STATE_TRANSITION_NOT_PERMITTED, RappState.PRIMING, RappState.PRIMED), + rappHandlerException.getMessage()); + assertEquals(RappState.PRIMING, rapp.getState()); } @Test @@ -125,7 +131,11 @@ class RappServiceTest { void testDeprimeRappInvalidState() { Rapp rapp = Rapp.builder().rappId(UUID.randomUUID()).name("").packageName(validRappFile) .packageLocation(validCsarFileLocation).state(RappState.COMMISSIONED).build(); - assertEquals(HttpStatus.BAD_REQUEST, rappService.deprimeRapp(rapp).getStatusCode()); + RappHandlerException rappHandlerException = + assertThrows(RappHandlerException.class, () -> rappService.deprimeRapp(rapp)); + assertEquals(HttpStatus.BAD_REQUEST, rappHandlerException.getStatusCode()); + assertEquals(String.format(STATE_TRANSITION_NOT_PERMITTED, RappState.COMMISSIONED, RappState.COMMISSIONED), + rappHandlerException.getMessage()); assertEquals(RappState.COMMISSIONED, rapp.getState()); } @@ -134,7 +144,9 @@ class RappServiceTest { Rapp rapp = Rapp.builder().rappId(UUID.randomUUID()).name("").packageName(validRappFile) .packageLocation(validCsarFileLocation).state(RappState.PRIMED) .rappInstances(Map.of(UUID.randomUUID(), new RappInstance())).build(); - assertEquals(HttpStatus.BAD_REQUEST, rappService.deprimeRapp(rapp).getStatusCode()); + RappHandlerException rappHandlerException = + assertThrows(RappHandlerException.class, () -> rappService.deprimeRapp(rapp)); + assertEquals(HttpStatus.BAD_REQUEST, rappHandlerException.getStatusCode()); assertEquals(RappState.PRIMED, rapp.getState()); } @@ -179,13 +191,15 @@ class RappServiceTest { Rapp rapp = Rapp.builder().rappId(UUID.randomUUID()).name("").packageName(validRappFile) .packageLocation(validCsarFileLocation).state(RappState.PRIMED).build(); RappInstance rappInstance = new RappInstance(); - RappInstanceState rappInstanceState = RappInstanceState.DEPLOYED; - rappInstance.setState(rappInstanceState); + rappInstance.setState(RappInstanceState.DEPLOYED); rappInstanceStateMachine.onboardRappInstance(rappInstance.getRappInstanceId()); - ResponseEntity responseEntity = rappService.deployRappInstance(rapp, rappInstance); - assertEquals(HttpStatus.BAD_REQUEST, responseEntity.getStatusCode()); - assertEquals("State transition from " + rappInstanceState + " to DEPLOYED is not permitted.", - responseEntity.getBody()); + RappHandlerException rappHandlerException = + assertThrows(RappHandlerException.class, () -> rappService.deployRappInstance(rapp, rappInstance)); + assertEquals(HttpStatus.BAD_REQUEST, rappHandlerException.getStatusCode()); + assertEquals(String.format("Unable to deploy rApp instance %s as it is not in UNDEPLOYED state", + rappInstance.getRappInstanceId()), rappHandlerException.getMessage()); + assertEquals(RappState.PRIMED, rapp.getState()); + } @Test @@ -237,7 +251,9 @@ class RappServiceTest { when(acmDeployer.undeployRappInstance(any(), any())).thenReturn(true); when(smeDeployer.undeployRappInstance(any(), any())).thenReturn(false); when(dmeDeployer.undeployRappInstance(any(), any())).thenReturn(true); - assertEquals(HttpStatus.BAD_REQUEST, rappService.undeployRappInstance(rapp, rappInstance).getStatusCode()); + RappHandlerException rappHandlerException = + assertThrows(RappHandlerException.class, () -> rappService.undeployRappInstance(rapp, rappInstance)); + assertEquals(HttpStatus.BAD_REQUEST, rappHandlerException.getStatusCode()); } @Test @@ -260,12 +276,17 @@ class RappServiceTest { .packageLocation(validCsarFileLocation).state(RappState.PRIMED).build(); RappInstance rappInstance = new RappInstance(); rappInstance.setState(RappInstanceState.DEPLOYED); + UUID rappInstanceId = rappInstance.getRappInstanceId(); HashMap rAppInstanceMap = new HashMap<>(); - rAppInstanceMap.put(rappInstance.getRappInstanceId(), rappInstance); + rAppInstanceMap.put(rappInstanceId, rappInstance); rapp.setRappInstances(rAppInstanceMap); rappInstanceStateMachine.onboardRappInstance(rappInstance.getRappInstanceId()); - assertThrows(RappHandlerException.class, - () -> rappService.deleteRappInstance(rapp, rappInstance.getRappInstanceId())); + RappHandlerException rappHandlerException = + assertThrows(RappHandlerException.class, () -> rappService.deleteRappInstance(rapp, rappInstanceId)); + assertEquals(HttpStatus.BAD_REQUEST, rappHandlerException.getStatusCode()); + assertEquals(String.format("Unable to delete rApp instance %s as it is not in UNDEPLOYED state", + rappInstance.getRappInstanceId()), rappHandlerException.getMessage()); + assertEquals(RappState.PRIMED, rapp.getState()); } @Test @@ -280,10 +301,12 @@ class RappServiceTest { String rAppName = "rAppInPrimed"; Rapp rApp = Rapp.builder().rappId(UUID.randomUUID()).name(rAppName).packageName(validRappFile) .packageLocation(validCsarFileLocation).state(RappState.PRIMED).build(); - ResponseEntity responseEntity = rappService.deleteRapp(rApp); - assertEquals(HttpStatus.BAD_REQUEST, responseEntity.getStatusCode()); - assertEquals("Unable to delete '" + rAppName + "' as the rApp is not in COMMISSIONED state.", - responseEntity.getBody()); + RappHandlerException rappHandlerException = + assertThrows(RappHandlerException.class, () -> rappService.deleteRapp(rApp)); + assertEquals(HttpStatus.BAD_REQUEST, rappHandlerException.getStatusCode()); + assertEquals(String.format("Unable to delete %s as the rApp is not in COMMISSIONED state.", rAppName), + rappHandlerException.getMessage()); + assertEquals(RappState.PRIMED, rApp.getState()); } @Test @@ -295,9 +318,11 @@ class RappServiceTest { rappInstance.setState(RappInstanceState.DEPLOYED); rappInstanceStateMachine.onboardRappInstance(rappInstance.getRappInstanceId()); rApp.setRappInstances(Map.of(rappInstance.getRappInstanceId(), rappInstance)); - ResponseEntity responseEntity = rappService.deleteRapp(rApp); - assertEquals(HttpStatus.BAD_REQUEST, responseEntity.getStatusCode()); - assertEquals("Unable to delete '" + rAppName + "' as there are active rApp instances.", - responseEntity.getBody()); + RappHandlerException rappHandlerException = + assertThrows(RappHandlerException.class, () -> rappService.deleteRapp(rApp)); + assertEquals(HttpStatus.BAD_REQUEST, rappHandlerException.getStatusCode()); + assertEquals(String.format("Unable to delete %s as there are active rApp instances.", rAppName), + rappHandlerException.getMessage()); + assertEquals(RappState.PRIMED, rApp.getState()); } } -- 2.16.6