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

Reply via email to