diff --git a/AMW_business/src/main/java/ch/puzzle/itc/mobiliar/business/resourcegroup/boundary/CopyResource.java b/AMW_business/src/main/java/ch/puzzle/itc/mobiliar/business/resourcegroup/boundary/CopyResource.java index 52a6e4431..2b3669030 100644 --- a/AMW_business/src/main/java/ch/puzzle/itc/mobiliar/business/resourcegroup/boundary/CopyResource.java +++ b/AMW_business/src/main/java/ch/puzzle/itc/mobiliar/business/resourcegroup/boundary/CopyResource.java @@ -56,7 +56,7 @@ /** * A boundary for copy resourcess - * + * * @author cweber */ @Stateless @@ -110,21 +110,33 @@ public List loadResourceGroupsForType(Integer typeId, ResourceEnt */ public CopyResourceResult doCopyResource(Integer targetResourceId, Integer originResourceId, ForeignableOwner actingOwner) throws ForeignableOwnerViolationException, AMWException { - // Load resources ResourceEntity targetResource = commonDomainService.getResourceEntityById(targetResourceId); ResourceEntity originResource = commonDomainService.getResourceEntityById(originResourceId); - if (!originResource.getResourceType().equals(targetResource.getResourceType())) { + return this.doCopyResource(targetResource, originResource, actingOwner); + } + + /** + * @param targetResourceEntity + * @param originResourceEntity + * @return result if copy was successful, contains a list with error messages if copy fails + * @throws ResourceNotFoundException + */ + public CopyResourceResult doCopyResource(ResourceEntity targetResourceEntity, ResourceEntity originResourceEntity, ForeignableOwner actingOwner) + throws ForeignableOwnerViolationException, AMWException { + + if (!originResourceEntity.getResourceType().equals(targetResourceEntity.getResourceType())) { throw new AMWException("Target and origin Resource are not of the same ResourceType"); } - if(!permissionBoundary.canCopyFromSpecificResource(originResource, targetResource.getResourceGroup())){ + if(!permissionBoundary.canCopyFromSpecificResource(originResourceEntity, targetResourceEntity.getResourceGroup())){ throw new NotAuthorizedException("Permission Denied"); } - return copyResourceDomainService.copyFromOriginToTargetResource(originResource, targetResource, actingOwner); + return copyResourceDomainService.copyFromOriginToTargetResource(originResourceEntity, targetResourceEntity, actingOwner); } + /** * Creates a new Release of an existing Resource * diff --git a/AMW_business/src/test/java/ch/puzzle/itc/mobiliar/business/resourcegroup/boundary/CopyResourceTest.java b/AMW_business/src/test/java/ch/puzzle/itc/mobiliar/business/resourcegroup/boundary/CopyResourceTest.java index 01f9305ba..f50e84776 100644 --- a/AMW_business/src/test/java/ch/puzzle/itc/mobiliar/business/resourcegroup/boundary/CopyResourceTest.java +++ b/AMW_business/src/test/java/ch/puzzle/itc/mobiliar/business/resourcegroup/boundary/CopyResourceTest.java @@ -24,6 +24,7 @@ import ch.puzzle.itc.mobiliar.builders.ResourceGroupEntityBuilder; import ch.puzzle.itc.mobiliar.business.domain.commons.CommonDomainService; import ch.puzzle.itc.mobiliar.business.foreignable.entity.ForeignableOwner; +import ch.puzzle.itc.mobiliar.business.foreignable.entity.ForeignableOwnerViolationException; import ch.puzzle.itc.mobiliar.business.integration.entity.util.ResourceTypeEntityBuilder; import ch.puzzle.itc.mobiliar.business.resourcegroup.control.CopyResourceDomainService; import ch.puzzle.itc.mobiliar.business.resourcegroup.entity.ResourceEntity; @@ -31,6 +32,7 @@ import ch.puzzle.itc.mobiliar.business.resourcegroup.entity.ResourceTypeEntity; import ch.puzzle.itc.mobiliar.business.security.boundary.PermissionBoundary; import ch.puzzle.itc.mobiliar.common.exception.AMWException; +import ch.puzzle.itc.mobiliar.common.exception.NotAuthorizedException; import org.junit.Before; import org.junit.Test; import org.mockito.InjectMocks; @@ -40,9 +42,8 @@ import java.util.HashSet; import java.util.Set; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; +import static org.junit.Assert.assertThrows; +import static org.mockito.Mockito.*; public class CopyResourceTest { @@ -84,11 +85,28 @@ public void shouldNotAllowCopyFromResourceOfDifferentType() throws Exception { // when // then copyResource.doCopyResource(targetResourceId, originResourceId, ForeignableOwner.getSystemOwner()); + } + + @Test + public void shouldThrowNotAuthorizedExceptionWhenPermissionIsDenied() throws ForeignableOwnerViolationException, AMWException { + // given + ResourceEntity originResourceEntity = mock(ResourceEntity.class); + ResourceEntity targetResourceEntity = mock(ResourceEntity.class); + ResourceTypeEntity resourceType = mock(ResourceTypeEntity.class); + when(targetResourceEntity.getResourceType()).thenReturn(resourceType); + when(originResourceEntity.getResourceType()).thenReturn(resourceType); + // when + assertThrows(NotAuthorizedException.class, () -> { + copyResource.doCopyResource(targetResourceEntity, originResourceEntity, ForeignableOwner.AMW); + }); + + // then + verify(copyResourceDomainService, never()).copyFromOriginToTargetResource(originResourceEntity, targetResourceEntity, ForeignableOwner.AMW); } @Test - public void shouldInvokePermissionSeviceAndCopyResourceDomainServiceWithCorrectParams() throws Exception { + public void shouldInvokePermissionServiceAndCopyResourceDomainServiceWithCorrectParams() throws Exception { //given String targetResourceName = "target"; @@ -118,4 +136,4 @@ public void shouldInvokePermissionSeviceAndCopyResourceDomainServiceWithCorrectP verify(copyResourceDomainService, times(1)).copyFromOriginToTargetResource(originResource, targetResource, ForeignableOwner.getSystemOwner()); } -} \ No newline at end of file +} diff --git a/AMW_rest/src/main/java/ch/mobi/itc/mobiliar/rest/resources/ResourcesRest.java b/AMW_rest/src/main/java/ch/mobi/itc/mobiliar/rest/resources/ResourcesRest.java index d7f2ae232..6c0c5f24e 100644 --- a/AMW_rest/src/main/java/ch/mobi/itc/mobiliar/rest/resources/ResourcesRest.java +++ b/AMW_rest/src/main/java/ch/mobi/itc/mobiliar/rest/resources/ResourcesRest.java @@ -625,30 +625,25 @@ public Response copyFromResource(@ApiParam(value = "The target ResourceGroup (to @ApiParam(value = "The target ReleaseName (to)") @PathParam("releaseName") String targetReleaseName, @ApiParam(value = "The origin ResourceGroup (from)") @QueryParam("originResourceGroupName") String originResourceGroupName, @ApiParam(value = "The origin ReleaseName (from)") @QueryParam("originReleaseName") String originReleaseName) throws ValidationException { - ResourceEntity targetResource = resourceLocator.getResourceByGroupNameAndRelease(targetResourceGroupName, targetReleaseName); - if (targetResource == null) { + + ResourceEntity targetResourceEntity = resourceLocator.getResourceByGroupNameAndRelease(targetResourceGroupName, targetReleaseName); + if (targetResourceEntity == null) { return Response.status(NOT_FOUND).entity(new ExceptionDto("Target Resource not found")).build(); } - ResourceEntity originResource = resourceLocator.getResourceByGroupNameAndRelease(originResourceGroupName, originReleaseName); - if (originResource == null) { + ResourceEntity originResourceEntity = resourceLocator.getResourceByGroupNameAndRelease(originResourceGroupName, originReleaseName); + if (originResourceEntity == null) { return Response.status(NOT_FOUND).entity(new ExceptionDto("Origin Resource not found")).build(); } - if ((!originResource.getResourceType().isApplicationResourceType() && !originResource.getResourceType().isApplicationServerResourceType()) - || (!targetResource.getResourceType().isApplicationResourceType() && !targetResource.getResourceType().isApplicationServerResourceType())) { - return Response.status(BAD_REQUEST).entity(new ExceptionDto("Only Application or ApplicationServer ResourceTypes are allowed")).build(); - } - CopyResourceResult copyResourceResult; try { - copyResourceResult = copyResource.doCopyResource(targetResource.getId(), originResource.getId(), ForeignableOwner.getSystemOwner()); + CopyResourceResult copyResourceResult = copyResource.doCopyResource(targetResourceEntity, originResourceEntity, ForeignableOwner.getSystemOwner()); + if (!copyResourceResult.isSuccess()) { + return Response.status(BAD_REQUEST).entity(new ExceptionDto("Copy from " + originResourceEntity.getName() + " failed")).build(); + } + return Response.ok().build(); } catch (ForeignableOwnerViolationException | AMWException e) { return Response.status(BAD_REQUEST).entity(new ExceptionDto(e.getMessage())).build(); } - - if (!copyResourceResult.isSuccess()) { - return Response.status(BAD_REQUEST).entity(new ExceptionDto("Copy from " + originResource.getName() + " failed")).build(); - } - return Response.ok().build(); } @Path("/{resourceGroupName}/{releaseName}/dependencies/") diff --git a/AMW_rest/src/test/java/ch/mobi/itc/mobiliar/rest/resources/ResourcesRestTest.java b/AMW_rest/src/test/java/ch/mobi/itc/mobiliar/rest/resources/ResourcesRestTest.java index c054cb1ba..6f6f5a70d 100644 --- a/AMW_rest/src/test/java/ch/mobi/itc/mobiliar/rest/resources/ResourcesRestTest.java +++ b/AMW_rest/src/test/java/ch/mobi/itc/mobiliar/rest/resources/ResourcesRestTest.java @@ -26,6 +26,7 @@ import java.util.List; import ch.mobi.itc.mobiliar.rest.dtos.*; +import ch.mobi.itc.mobiliar.rest.exceptions.ExceptionDto; import ch.puzzle.itc.mobiliar.business.deploy.boundary.DeploymentBoundary; import ch.puzzle.itc.mobiliar.business.environment.entity.ContextEntity; import ch.puzzle.itc.mobiliar.business.foreignable.entity.ForeignableOwner; @@ -62,16 +63,12 @@ import javax.ws.rs.core.Response; import static ch.puzzle.itc.mobiliar.common.util.ApplicationServerContainer.APPSERVERCONTAINER; -import static javax.ws.rs.core.Response.Status.BAD_REQUEST; -import static javax.ws.rs.core.Response.Status.CREATED; +import static javax.ws.rs.core.Response.Status.*; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; - -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; +import static org.mockito.Mockito.*; public class ResourcesRestTest { @@ -214,7 +211,7 @@ public void getBatchJobInventoryEmpty() { Integer type = 2305; List list = new ArrayList<>(); when(serverViewMock.getServers(Mockito.anyString(), Mockito.anyString(), Mockito.anyString(), Mockito.anyString(), Mockito.anyString(), Mockito.anyBoolean())).thenReturn(list); - + // when BatchJobInventoryDTO result = rest.getBatchJobInventar(env, type, null, null, null, null, null); @@ -357,7 +354,7 @@ public void shouldInvokeResourcesWithRightArgumentsOnGetClosestPastRelease() thr verify(resourcePropertiesMock).getResourceProperties(resourceGroupName, closestRelease.getName(), env); verify(resourceTemplatesRestMock).getResourceTemplates(resourceGroupName, closestRelease.getName(), ""); } - + @Test public void shouldInvokeBoundaryWithRightArgumentsOnGetApplicationsWithVersionForRelease() { // given @@ -377,6 +374,55 @@ public void shouldInvokeBoundaryWithRightArgumentsOnGetApplicationsWithVersionFo verify(deploymentBoundaryMock, times(1)).getVersions(appServer, contextIds, release); } + @Test + public void shouldNotAllowCopyFromWhenTargetNotFound() throws ValidationException { + // given + when(resourceLocatorMock.getResourceByGroupNameAndRelease(anyString(), anyString())).thenReturn(null); + + // when + final Response response = rest.copyFromResource("targetResourceGroupName", "targetReleaseName", "originResourceGroupName", "originReleaseName"); + + // then + assertThat(response.getStatus(), is(NOT_FOUND.getStatusCode())); + assertThat(((ExceptionDto) response.getEntity()).getMessage(), is("Target Resource not found")); + } + + @Test + public void shouldNotAllowCopyFromWhenOriginNotFound() throws ValidationException { + // given + ResourceEntity targetResourceEntity = mock(ResourceEntity.class); + when(resourceLocatorMock.getResourceByGroupNameAndRelease("targetResourceGroupName", "targetReleaseName")).thenReturn(targetResourceEntity); + when(resourceLocatorMock.getResourceByGroupNameAndRelease("originResourceGroupName", "originReleaseName")).thenReturn(null); + + // when + final Response response = rest.copyFromResource("targetResourceGroupName", "targetReleaseName", "originResourceGroupName", "originReleaseName"); + + // then + assertThat(response.getStatus(), is(NOT_FOUND.getStatusCode())); + assertThat(((ExceptionDto) response.getEntity()).getMessage(), is("Origin Resource not found")); + } + + @Test + public void shouldReturnBadRequestWhenCopyFromDoesNotSucceed() throws ValidationException, ForeignableOwnerViolationException, AMWException { + // given + ResourceEntity targetResourceEntity = mock(ResourceEntity.class); + when(resourceLocatorMock.getResourceByGroupNameAndRelease("targetResourceGroupName", "targetReleaseName")).thenReturn(targetResourceEntity); + ResourceEntity originResourceEntity = mock(ResourceEntity.class); + when(originResourceEntity.getName()).thenReturn("Origin"); + when(resourceLocatorMock.getResourceByGroupNameAndRelease("originResourceGroupName", "originReleaseName")).thenReturn(originResourceEntity); + + CopyResourceResult copyResourceResult = mock(CopyResourceResult.class); + when(copyResourceResult.isSuccess()).thenReturn(false); + when(copyResourceMock.doCopyResource(targetResourceEntity, originResourceEntity, ForeignableOwner.getSystemOwner())).thenReturn(copyResourceResult); + + // when + final Response response = rest.copyFromResource("targetResourceGroupName", "targetReleaseName", "originResourceGroupName", "originReleaseName"); + + // then + assertThat(response.getStatus(), is(BAD_REQUEST.getStatusCode())); + assertThat(((ExceptionDto) response.getEntity()).getMessage(), is("Copy from Origin failed")); + } + @Test public void shouldNotAllowCopyFromResourceOfDifferentTypes() throws ValidationException, ForeignableOwnerViolationException, AMWException { // given @@ -407,7 +453,7 @@ public void shouldNotAllowCopyFromResourceOfDifferentTypes() throws ValidationEx when(resourceLocatorMock.getResourceByGroupNameAndRelease(targetResourceGroupName, targetReleaseName)).thenReturn(target); when(resourceLocatorMock.getResourceByGroupNameAndRelease(originResourceGroupName, originReleaseName)).thenReturn(origin); - when(copyResourceMock.doCopyResource(target.getId(), origin.getId(), ForeignableOwner.getSystemOwner())).thenThrow(new AMWException("Target and origin Resource are not of the same ResourceType")); + when(copyResourceMock.doCopyResource(target, origin, ForeignableOwner.getSystemOwner())).thenThrow(new AMWException("Target and origin Resource are not of the same ResourceType")); // when Response response = rest.copyFromResource(targetResourceGroupName, targetReleaseName, originResourceGroupName, originReleaseName); @@ -418,7 +464,7 @@ public void shouldNotAllowCopyFromResourceOfDifferentTypes() throws ValidationEx } @Test - public void shouldNotAllowCopyFromResourceOfNodeType() throws ValidationException { + public void shouldAllowCopyFromResourceOfNodeType() throws ValidationException, ForeignableOwnerViolationException, AMWException { // given String originResourceGroupName = "Origin"; String originReleaseName = "From"; @@ -445,12 +491,15 @@ public void shouldNotAllowCopyFromResourceOfNodeType() throws ValidationExceptio when(resourceLocatorMock.getResourceByGroupNameAndRelease(targetResourceGroupName, targetReleaseName)).thenReturn(target); when(resourceLocatorMock.getResourceByGroupNameAndRelease(originResourceGroupName, originReleaseName)).thenReturn(origin); + CopyResourceResult copyResourceResult = mock(CopyResourceResult.class); + when(copyResourceResult.isSuccess()).thenReturn(true); + when(copyResourceMock.doCopyResource(target, origin, ForeignableOwner.getSystemOwner())).thenReturn(copyResourceResult); // when Response response = rest.copyFromResource(targetResourceGroupName, targetReleaseName, originResourceGroupName, originReleaseName); // then - assertEquals(BAD_REQUEST.getStatusCode(), response.getStatus()); + assertEquals(OK.getStatusCode(), response.getStatus()); } @Test @@ -472,4 +521,4 @@ public void shouldFilterOutAppServerContainer() { assertThat(filteredGroups.get(0).getName(), is(wanted.getName())); } -} \ No newline at end of file +}