Juan Hernandez has uploaded a new change for review. Change subject: restapi: Move VM Disk remove from collection to entity ......................................................................
restapi: Move VM Disk remove from collection to entity Previous changes moved the remove methods from collection to entity interfaces, except the device related ones as those required larger changes that posed a risk for the migration to WildFly, those were handled in a different and less nice way. This completes the movement for the VM disk resource in a nicer way. Change-Id: I72f35d4a2ed77b249ec142c7b27782694e312efc Signed-off-by: Juan Hernandez <juan.hernan...@redhat.com> --- M backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmDiskResource.java M backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmDisksResource.java M backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmDiskResourceTest.java M backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmDisksResourceTest.java 4 files changed, 123 insertions(+), 98 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/79/42279/1 diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmDiskResource.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmDiskResource.java index cfa2f43..6cc1473 100644 --- a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmDiskResource.java +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmDiskResource.java @@ -13,38 +13,48 @@ import org.ovirt.engine.api.resource.StatisticsResource; import org.ovirt.engine.api.resource.VmDiskResource; import org.ovirt.engine.core.common.VdcObjectType; +import org.ovirt.engine.core.common.action.AttachDetachVmDiskParameters; import org.ovirt.engine.core.common.action.ExportRepoImageParameters; import org.ovirt.engine.core.common.action.HotPlugDiskToVmParameters; import org.ovirt.engine.core.common.action.MoveDiskParameters; import org.ovirt.engine.core.common.action.MoveDisksParameters; +import org.ovirt.engine.core.common.action.RemoveDiskParameters; import org.ovirt.engine.core.common.action.VdcActionType; import org.ovirt.engine.core.common.queries.GetPermissionsForObjectParameters; import org.ovirt.engine.core.common.queries.IdQueryParameters; import org.ovirt.engine.core.common.queries.VdcQueryType; import org.ovirt.engine.core.compat.Guid; +public class BackendVmDiskResource + extends BackendDeviceResource<Disk, Disks, org.ovirt.engine.core.common.businessentities.storage.Disk> + implements VmDiskResource { -public class BackendVmDiskResource extends BackendDeviceResource<Disk, Disks, org.ovirt.engine.core.common.businessentities.storage.Disk> implements VmDiskResource { - protected BackendVmDiskResource(String id, - AbstractBackendReadOnlyDevicesResource<Disk, Disks, org.ovirt.engine.core.common.businessentities.storage.Disk> collection, - VdcActionType updateType, - ParametersProvider<Disk, org.ovirt.engine.core.common.businessentities.storage.Disk> updateParametersProvider, - String[] requiredUpdateFields, - String... subCollections) { - super(Disk.class, - org.ovirt.engine.core.common.businessentities.storage.Disk.class, - collection.asGuidOr404(id), - collection, - updateType, - updateParametersProvider, - requiredUpdateFields, - SUB_COLLECTIONS); + private Guid vmId; + + protected BackendVmDiskResource( + Guid vmId, + String diskId, + AbstractBackendReadOnlyDevicesResource<Disk, Disks, org.ovirt.engine.core.common.businessentities.storage.Disk> collection, + VdcActionType updateType, + ParametersProvider<Disk, org.ovirt.engine.core.common.businessentities.storage.Disk> updateParametersProvider, + String[] requiredUpdateFields, + String... subCollections) { + super( + Disk.class, + org.ovirt.engine.core.common.businessentities.storage.Disk.class, + collection.asGuidOr404(diskId), + collection, + updateType, + updateParametersProvider, + requiredUpdateFields, + SUB_COLLECTIONS + ); + this.vmId = vmId; } @Override public StatisticsResource getStatisticsResource() { EntityIdResolver<Guid> resolver = new EntityIdResolver<Guid>() { - @Override public org.ovirt.engine.core.common.businessentities.storage.Disk lookupEntity( Guid guid) throws BackendFailureException { @@ -52,7 +62,7 @@ } }; DiskStatisticalQuery query = new DiskStatisticalQuery(resolver, newModel(id)); - return inject(new BackendStatisticsResource<Disk, org.ovirt.engine.core.common.businessentities.storage.Disk>(entityType, guid, query)); + return inject(new BackendStatisticsResource<>(entityType, guid, query)); } @Override @@ -141,10 +151,20 @@ return super.update(resource);//explicit call solves REST-Easy confusion } - // The code to remove the disk should be here, but moving it from the collection resource requires too many - // changes, so it is temporarily kept there. + @Override + public Response remove() { + get(); + return performAction(VdcActionType.RemoveDisk, new RemoveDiskParameters(guid)); + } + @Override public Response remove(Action action) { - return ((BackendVmDisksResource) collection).deprecatedRemove(id, action); + get(); + if (action.isSetDetach() && action.isDetach()) { + return performAction(VdcActionType.DetachDiskFromVm, new AttachDetachVmDiskParameters(vmId, guid)); + } + else { + return performAction(VdcActionType.RemoveDisk, new RemoveDiskParameters(guid)); + } } } diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmDisksResource.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmDisksResource.java index e2a0735..d7818bc 100644 --- a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmDisksResource.java +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmDisksResource.java @@ -6,7 +6,6 @@ import org.apache.commons.lang.BooleanUtils; import org.ovirt.engine.api.common.util.DetailHelper; -import org.ovirt.engine.api.model.Action; import org.ovirt.engine.api.model.Disk; import org.ovirt.engine.api.model.Disks; import org.ovirt.engine.api.model.Snapshot; @@ -81,29 +80,20 @@ } } - // This method should be part of the entity resource, but moving it requires too many changes, so it will be - // temporarily kept here. A new method is introduced in the entity resource that calls this one, and this - // one is removed from the collection interface. - @Deprecated - public Response deprecatedRemove(String id, Action action) { - getEntity(id); //verifies that entity exists, returns 404 otherwise. - if (action.isSetDetach() && action.isDetach()) { - return performAction(VdcActionType.DetachDiskFromVm, - new AttachDetachVmDiskParameters(parentId, Guid.createGuidFromStringDefaultEmpty(id))); - } else { - return remove(id); - } - } - @Override @SingleEntityResource public VmDiskResource getDeviceSubResource(String id) { - return inject(new BackendVmDiskResource(id, - this, - updateType, - getUpdateParametersProvider(), - getRequiredUpdateFields(), - subCollections)); + return inject( + new BackendVmDiskResource( + parentId, + id, + this, + updateType, + getUpdateParametersProvider(), + getRequiredUpdateFields(), + subCollections + ) + ); } @Override diff --git a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmDiskResourceTest.java b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmDiskResourceTest.java index acf0965..c7dd9ce 100644 --- a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmDiskResourceTest.java +++ b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmDiskResourceTest.java @@ -21,12 +21,16 @@ import org.ovirt.engine.api.model.Statistic; import org.ovirt.engine.api.model.StorageDomain; import org.ovirt.engine.api.resource.VmDiskResource; +import org.ovirt.engine.core.common.VdcObjectType; +import org.ovirt.engine.core.common.action.AttachDetachVmDiskParameters; import org.ovirt.engine.core.common.action.ExportRepoImageParameters; import org.ovirt.engine.core.common.action.HotPlugDiskToVmParameters; import org.ovirt.engine.core.common.action.MoveDisksParameters; +import org.ovirt.engine.core.common.action.RemoveDiskParameters; import org.ovirt.engine.core.common.action.UpdateVmDiskParameters; import org.ovirt.engine.core.common.action.VdcActionParametersBase; import org.ovirt.engine.core.common.action.VdcActionType; +import org.ovirt.engine.core.common.asynctasks.EntityInfo; import org.ovirt.engine.core.common.businessentities.StorageDomainStatus; import org.ovirt.engine.core.common.businessentities.StorageDomainType; import org.ovirt.engine.core.common.businessentities.storage.DiskStorageType; @@ -39,11 +43,9 @@ import org.ovirt.engine.core.compat.Guid; public class BackendVmDiskResourceTest - extends AbstractBackendSubResourceTest<Disk, org.ovirt.engine.core.common.businessentities.storage.Disk, BackendVmDiskResource> { + extends AbstractBackendSubResourceTest<Disk, org.ovirt.engine.core.common.businessentities.storage.Disk, BackendVmDiskResource> { protected static final Guid DISK_ID = GUIDS[1]; - - protected static BackendVmDisksResource collection; public BackendVmDiskResourceTest() { super((BackendVmDiskResource)getCollection().getDeviceSubResource(DISK_ID.toString())); @@ -232,6 +234,73 @@ } } + @Test + public void testRemove() throws Exception { + setUriInfo(setUpBasicUriExpectations()); + setUpEntityQueryExpectations(1); + setUriInfo( + setUpActionExpectations( + VdcActionType.RemoveDisk, + RemoveDiskParameters.class, + new String[] { "DiskId" }, + new Object[] { DISK_ID }, + true, + true + ) + ); + verifyRemove(resource.remove()); + } + + @Test + public void testDetach() throws Exception { + setUriInfo(setUpBasicUriExpectations()); + setUpEntityQueryExpectations(1); + setUriInfo( + setUpActionExpectations( + VdcActionType.DetachDiskFromVm, + AttachDetachVmDiskParameters.class, + new String[] { "VmId", "EntityInfo" }, + new Object[] { PARENT_ID, new EntityInfo(VdcObjectType.Disk, DISK_ID) }, + true, + true + ) + ); + Action action = new Action(); + action.setDetach(true); + verifyRemove(resource.remove(action)); + } + + @Test + public void testRemoveCantDo() throws Exception { + doTestBadRemove(false, true, CANT_DO); + } + + @Test + public void testRemoveFailed() throws Exception { + doTestBadRemove(true, false, FAILURE); + } + + protected void doTestBadRemove(boolean canDo, boolean success, String detail) throws Exception { + setUriInfo(setUpBasicUriExpectations()); + setUpEntityQueryExpectations(1); + setUriInfo( + setUpActionExpectations( + VdcActionType.RemoveDisk, + RemoveDiskParameters.class, + new String[] { "DiskId" }, + new Object[] { DISK_ID }, + canDo, + success + ) + ); + try { + resource.remove(); + fail("expected WebApplicationException"); + } catch (WebApplicationException wae) { + verifyFault(wae, detail); + } + } + protected DiskImage setUpStatisticalExpectations() throws Exception { DiskImage entity = control.createMock(DiskImage.class); expect(entity.getId()).andReturn(DISK_ID).anyTimes(); diff --git a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmDisksResourceTest.java b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmDisksResourceTest.java index 8f6c5a4..ffb95ba 100644 --- a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmDisksResourceTest.java +++ b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmDisksResourceTest.java @@ -7,7 +7,6 @@ import javax.ws.rs.core.Response; import org.junit.Test; -import org.ovirt.engine.api.model.Action; import org.ovirt.engine.api.model.CreationStatus; import org.ovirt.engine.api.model.Disk; import org.ovirt.engine.api.model.LogicalUnit; @@ -19,7 +18,6 @@ import org.ovirt.engine.core.common.VdcObjectType; import org.ovirt.engine.core.common.action.AddDiskParameters; import org.ovirt.engine.core.common.action.AttachDetachVmDiskParameters; -import org.ovirt.engine.core.common.action.RemoveDiskParameters; import org.ovirt.engine.core.common.action.VdcActionType; import org.ovirt.engine.core.common.asynctasks.EntityInfo; import org.ovirt.engine.core.common.businessentities.AsyncTaskStatus; @@ -61,50 +59,12 @@ } } - @Test - public void testRemove() throws Exception { - setUpGetEntityExpectations(); - setUriInfo(setUpActionExpectations(VdcActionType.RemoveDisk, - RemoveDiskParameters.class, - new String[] { "DiskId" }, - new Object[] { GUIDS[0] }, - true, - true)); - verifyRemove(collection.deprecatedRemove(GUIDS[0].toString())); - } - private void setUpGetEntityExpectations() { setUpEntityQueryExpectations(VdcQueryType.GetAllDisksByVmId, IdQueryParameters.class, new String[] { "Id" }, new Object[] { PARENT_ID }, getEntityList()); - } - - @Test - public void testRemoveCantDo() throws Exception { - doTestBadRemove(false, true, CANT_DO); - } - - @Test - public void testRemoveFailed() throws Exception { - doTestBadRemove(true, false, FAILURE); - } - - protected void doTestBadRemove(boolean canDo, boolean success, String detail) throws Exception { - setUpGetEntityExpectations(); - setUriInfo(setUpActionExpectations(VdcActionType.RemoveDisk, - RemoveDiskParameters.class, - new String[] { "DiskId" }, - new Object[] { GUIDS[0] }, - canDo, - success)); - try { - collection.deprecatedRemove(GUIDS[0].toString()); - fail("expected WebApplicationException"); - } catch (WebApplicationException wae) { - verifyFault(wae, detail); - } } @Test @@ -164,20 +124,6 @@ true); Response response = collection.add(model); assertEquals(200, response.getStatus()); - } - - @Test - public void testDetachDisk() throws Exception { - setUpGetEntityExpectations(); - setUriInfo(setUpActionExpectations(VdcActionType.DetachDiskFromVm, - AttachDetachVmDiskParameters.class, - new String[] { "VmId", "EntityInfo" }, - new Object[] { PARENT_ID, new EntityInfo(VdcObjectType.Disk, DISK_ID) }, - true, - true)); - Action action = new Action(); - action.setDetach(true); - verifyRemove(collection.deprecatedRemove(DISK_ID.toString(), action)); } private void doTestAddAsync(AsyncTaskStatusEnum asyncStatus, CreationStatus creationStatus) throws Exception { -- To view, visit https://gerrit.ovirt.org/42279 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I72f35d4a2ed77b249ec142c7b27782694e312efc Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Juan Hernandez <juan.hernan...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches