Ori Liel has uploaded a new change for review. Change subject: restapi: Move VM remove from collection to entity ......................................................................
restapi: Move VM remove from collection to entity This patch moves the method that implements the DELETE operation from the collection interface to the entity interface. This is needed to avoid issues with newer versions of Resteasy. Change-Id: I1d653075475a136eccf4c2ea0bf3921dae3e1bb8 Signed-off-by: Ori Liel <[email protected]> --- M backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/VmResource.java M backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/VmsResource.java M backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmResource.java M backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmsResource.java M backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmResourceTest.java M backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmsResourceTest.java 6 files changed, 157 insertions(+), 165 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/74/42074/1 diff --git a/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/VmResource.java b/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/VmResource.java index a48c579..655540d 100644 --- a/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/VmResource.java +++ b/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/VmResource.java @@ -17,6 +17,7 @@ package org.ovirt.engine.api.resource; import javax.ws.rs.Consumes; +import javax.ws.rs.DELETE; import javax.ws.rs.POST; import javax.ws.rs.Path; import javax.ws.rs.PathParam; @@ -36,6 +37,13 @@ @Path("{action: (start|stop|shutdown|reboot|suspend|detach|migrate|export|move|ticket|cancelmigration|preview_snapshot|commit_snapshot|undo_snapshot|clone|maintenance)}/{oid}") public ActionResource getActionSubresource(@PathParam("action")String action, @PathParam("oid")String oid); + @DELETE + public Response remove(); + + @DELETE + @Consumes({ ApiMediaType.APPLICATION_XML, ApiMediaType.APPLICATION_JSON, ApiMediaType.APPLICATION_X_YAML }) + public Response remove(Action action); + @POST @Consumes({ApiMediaType.APPLICATION_XML, ApiMediaType.APPLICATION_JSON, ApiMediaType.APPLICATION_X_YAML}) @Actionable diff --git a/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/VmsResource.java b/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/VmsResource.java index a9f0f83..0b46b6c 100644 --- a/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/VmsResource.java +++ b/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/VmsResource.java @@ -17,7 +17,6 @@ package org.ovirt.engine.api.resource; import javax.ws.rs.Consumes; -import javax.ws.rs.DELETE; import javax.ws.rs.GET; import javax.ws.rs.Path; import javax.ws.rs.PathParam; @@ -25,7 +24,6 @@ import javax.ws.rs.Produces; import javax.ws.rs.core.Response; -import org.ovirt.engine.api.model.Action; import org.ovirt.engine.api.model.VM; import org.ovirt.engine.api.model.VMs; @@ -48,16 +46,6 @@ @POST @Consumes({ApiMediaType.APPLICATION_XML, ApiMediaType.APPLICATION_JSON, ApiMediaType.APPLICATION_X_YAML}) public Response add(VM vm); - - @DELETE - @Path("{id}") - public Response remove(@PathParam("id") String id); - - @DELETE - @Consumes({ApiMediaType.APPLICATION_XML, ApiMediaType.APPLICATION_JSON, ApiMediaType.APPLICATION_X_YAML}) - @Path("{id}") - public Response remove(@PathParam("id") String id, Action action); - /** * Sub-resource locator method, returns individual VmResource on which the diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmResource.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmResource.java index 1462dcb..cb76b6d 100644 --- a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmResource.java +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmResource.java @@ -6,6 +6,7 @@ import java.util.List; import java.util.Set; +import javax.ws.rs.Consumes; import javax.ws.rs.WebApplicationException; import javax.ws.rs.core.Response; import javax.ws.rs.core.UriInfo; @@ -55,6 +56,7 @@ import org.ovirt.engine.core.common.action.MigrateVmToServerParameters; import org.ovirt.engine.core.common.action.MoveVmParameters; import org.ovirt.engine.core.common.action.RemoveVmFromPoolParameters; +import org.ovirt.engine.core.common.action.RemoveVmParameters; import org.ovirt.engine.core.common.action.RestoreAllSnapshotsParameters; import org.ovirt.engine.core.common.action.RunVmOnceParams; import org.ovirt.engine.core.common.action.RunVmParams; @@ -169,6 +171,26 @@ return vm; } + @Override + public Response remove() { + get(); + return performAction(VdcActionType.RemoveVm, new RemoveVmParameters(asGuid(id), false)); + } + + @Override + @Consumes({ "application/xml", "application/json", "application/x-yaml" }) + public Response remove(Action action) { + get(); + boolean forceRemove = action != null && action.isSetForce() ? action.isForce() : false; + RemoveVmParameters params = new RemoveVmParameters(asGuid(id), forceRemove); + // If detach only is set we do not remove the VM disks + if (action != null && action.isSetVm() && action.getVm().isSetDisks() + && action.getVm().getDisks().isSetDetachOnly()) { + params.setRemoveDisks(false); + } + return performAction(VdcActionType.RemoveVm, params); + } + private void validateParameters(VM incoming) { if (incoming.isSetDomain() && !incoming.getDomain().isSetName()) { throw new WebFaultException(null, diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmsResource.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmsResource.java index 265c5c6..182c327 100644 --- a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmsResource.java +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmsResource.java @@ -12,7 +12,6 @@ import org.apache.commons.lang.ObjectUtils; import org.ovirt.engine.api.common.util.DetailHelper; -import org.ovirt.engine.api.model.Action; import org.ovirt.engine.api.model.Certificate; import org.ovirt.engine.api.model.Configuration; import org.ovirt.engine.api.model.ConfigurationType; @@ -43,7 +42,6 @@ import org.ovirt.engine.core.common.action.AddVmFromSnapshotParameters; import org.ovirt.engine.core.common.action.AddVmParameters; import org.ovirt.engine.core.common.action.ImportVmParameters; -import org.ovirt.engine.core.common.action.RemoveVmParameters; import org.ovirt.engine.core.common.action.VdcActionType; import org.ovirt.engine.core.common.action.VmManagementParametersBase; import org.ovirt.engine.core.common.businessentities.Entities; @@ -537,23 +535,6 @@ new IdQueryParameters(Guid.createGuidFromStringDefaultEmpty(vm.getId())))); Disks disks = disksResource.list(); vm.setDisks(disks); - } - - @Override - public Response performRemove(String id) { - return performAction(VdcActionType.RemoveVm, new RemoveVmParameters(asGuid(id), false)); - } - - @Override - public Response remove(String id, Action action) { - getEntity(id); - boolean forceRemove = action != null && action.isSetForce() ? action.isForce() : false; - RemoveVmParameters params = new RemoveVmParameters(asGuid(id), forceRemove); - // If detach only is set we do not remove the VM disks - if (action != null && action.isSetVm() && action.getVm().isSetDisks() && action.getVm().getDisks().isSetDetachOnly()) { - params.setRemoveDisks(false); - } - return performAction(VdcActionType.RemoveVm, params); } protected VMs mapCollection(List<org.ovirt.engine.core.common.businessentities.VM> entities, boolean isFiltered) { diff --git a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmResourceTest.java b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmResourceTest.java index e28d7f3..0f7f09c 100644 --- a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmResourceTest.java +++ b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmResourceTest.java @@ -10,7 +10,6 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; -import java.util.HashMap; import java.util.List; import javax.ws.rs.WebApplicationException; @@ -28,6 +27,7 @@ import org.ovirt.engine.api.model.CdRoms; import org.ovirt.engine.api.model.Cluster; import org.ovirt.engine.api.model.CreationStatus; +import org.ovirt.engine.api.model.Disks; import org.ovirt.engine.api.model.Display; import org.ovirt.engine.api.model.DisplayType; import org.ovirt.engine.api.model.File; @@ -49,6 +49,7 @@ import org.ovirt.engine.core.common.action.MigrateVmToServerParameters; import org.ovirt.engine.core.common.action.MoveVmParameters; import org.ovirt.engine.core.common.action.RemoveVmFromPoolParameters; +import org.ovirt.engine.core.common.action.RemoveVmParameters; import org.ovirt.engine.core.common.action.RestoreAllSnapshotsParameters; import org.ovirt.engine.core.common.action.RunVmOnceParams; import org.ovirt.engine.core.common.action.RunVmParams; @@ -88,7 +89,6 @@ private static final String FLOPPY_ID = "bar.vfd"; public static final String CERTIFICATE = "O=Redhat,CN=X.Y.Z.Q"; private static final String PAYLOAD_COMTENT = "payload"; - private static HashMap<Integer, String> osNames = new HashMap<>(); public BackendVmResourceTest() { super(new BackendVmResource(GUIDS[0].toString(), new BackendVmsResource())); @@ -969,6 +969,121 @@ verifyActionResponse(resource.maintenance(action)); } + @Test + public void testRemove() throws Exception { + setUriInfo(setUpBasicUriExpectations()); + setUpGetEntityExpectations(); + setUpGetPayloadExpectations(0, 1); + setUpGetBallooningExpectations(); + setUpGetGraphicsExpectations(1); + setUpActionExpectations(VdcActionType.RemoveVm, RemoveVmParameters.class, new String[] { + "VmId", "Force" }, new Object[] { GUIDS[0], Boolean.FALSE }, true, true); + verifyRemove(resource.remove()); + } + + @Test + public void testRemoveForced() throws Exception { + setUriInfo(setUpBasicUriExpectations()); + setUpGetEntityExpectations(); + setUpGetPayloadExpectations(0, 1); + setUpGetBallooningExpectations(); + setUpGetGraphicsExpectations(1); + setUpActionExpectations(VdcActionType.RemoveVm, RemoveVmParameters.class, new String[] { + "VmId", "Force" }, new Object[] { GUIDS[0], Boolean.TRUE }, true, true); + verifyRemove(resource.remove(new Action() { + { + setForce(true); + } + })); + } + + @Test + public void testRemoveDetachOnly() throws Exception { + setUriInfo(setUpBasicUriExpectations()); + setUpGetEntityExpectations(); + setUpGetPayloadExpectations(0, 1); + setUpGetBallooningExpectations(); + setUpGetGraphicsExpectations(1); + setUpActionExpectations(VdcActionType.RemoveVm, RemoveVmParameters.class, new String[] { + "VmId", "RemoveDisks" }, new Object[] { GUIDS[0], Boolean.FALSE }, true, true); + + Action action = new Action(); + action.setVm(new VM()); + action.getVm().setDisks(new Disks()); + action.getVm().getDisks().setDetachOnly(true); + verifyRemove(resource.remove(action)); + } + + @Test + public void testRemoveForcedIncomplete() throws Exception { + setUriInfo(setUpBasicUriExpectations()); + setUpGetEntityExpectations(); + setUpGetPayloadExpectations(0, 1); + setUpGetBallooningExpectations(); + setUpGetGraphicsExpectations(1); + setUpActionExpectations(VdcActionType.RemoveVm, RemoveVmParameters.class, new String[] { + "VmId", "Force" }, new Object[] { GUIDS[0], Boolean.FALSE }, true, true); + verifyRemove(resource.remove(new Action() { + { + } + })); + } + + @Test + public void testRemoveNonExistant() throws Exception { + setUpGetEntityExpectations(VdcQueryType.GetVmByVmId, + IdQueryParameters.class, + new String[] { "Id" }, + new Object[] { GUIDS[0] }, + null); + setUriInfo(setUpBasicUriExpectations()); + control.replay(); + try { + resource.remove(); + fail("expected WebApplicationException"); + } catch (WebApplicationException wae) { + assertNotNull(wae.getResponse()); + assertEquals(404, wae.getResponse().getStatus()); + } + } + + private void setUpGetEntityExpectations() throws Exception { + setUpGetEntityExpectations(VdcQueryType.GetVmByVmId, + IdQueryParameters.class, + new String[] { "Id" }, + new Object[] { GUIDS[0] }, + new org.ovirt.engine.core.common.businessentities.VM()); + } + + @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(); + setUpGetPayloadExpectations(0, 1); + setUpGetBallooningExpectations(); + setUpGetGraphicsExpectations(1); + setUriInfo(setUpActionExpectations(VdcActionType.RemoveVm, + RemoveVmParameters.class, + new String[] { "VmId", "Force" }, + new Object[] { GUIDS[0], Boolean.FALSE }, + canDo, + success)); + try { + resource.remove(); + fail("expected WebApplicationException"); + } catch (WebApplicationException wae) { + verifyFault(wae, detail); + } + } + protected org.ovirt.engine.core.common.businessentities.VM setUpStatisticalExpectations() throws Exception { org.ovirt.engine.core.common.businessentities.VM entity = new org.ovirt.engine.core.common.businessentities.VM(); setUpStatisticalEntityExpectations(entity, entity.getStatisticsData()); diff --git a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmsResourceTest.java b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmsResourceTest.java index b6f83c5..c6a9758 100644 --- a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmsResourceTest.java +++ b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmsResourceTest.java @@ -16,7 +16,6 @@ import org.apache.commons.codec.binary.Base64; import org.junit.Test; -import org.ovirt.engine.api.model.Action; import org.ovirt.engine.api.model.Cluster; import org.ovirt.engine.api.model.Configuration; import org.ovirt.engine.api.model.CreationStatus; @@ -36,7 +35,6 @@ import org.ovirt.engine.core.common.action.AddVmFromSnapshotParameters; import org.ovirt.engine.core.common.action.AddVmParameters; import org.ovirt.engine.core.common.action.ImportVmParameters; -import org.ovirt.engine.core.common.action.RemoveVmParameters; import org.ovirt.engine.core.common.action.VdcActionType; import org.ovirt.engine.core.common.businessentities.ArchitectureType; import org.ovirt.engine.core.common.businessentities.AsyncTaskStatus; @@ -125,136 +123,6 @@ verifyCollection(vms); } finally { accepts.clear(); - } - } - - @Test - public void testRemove() throws Exception { - setUriInfo(setUpBasicUriExpectations()); - mockUniqueOsNames(); - setUpGetEntityExpectations(); - setUpGetPayloadExpectations(1, 0); - setUpGetBallooningExpectations(1); - setUpGetGraphicsExpectations(1); - setUpActionExpectations(VdcActionType.RemoveVm, RemoveVmParameters.class, new String[] { - "VmId", "Force" }, new Object[] { GUIDS[0], Boolean.FALSE }, true, true); - verifyRemove(collection.remove(GUIDS[0].toString())); - } - - protected void setUpGetGraphicsExpectations(int times) throws Exception { - for (int i = 0; i < times; i++) { - setUpGetEntityExpectations(VdcQueryType.GetGraphicsDevices, - IdQueryParameters.class, - new String[] {}, - new Object[] {}, - Arrays.asList(new GraphicsDevice(VmDeviceType.SPICE))); - } - } - - @Test - public void testRemoveForced() throws Exception { - setUriInfo(setUpBasicUriExpectations()); - mockUniqueOsNames(); - setUpGetEntityExpectations(); - setUpGetPayloadExpectations(1, 0); - setUpGetBallooningExpectations(1); - setUpGetGraphicsExpectations(1); - setUpActionExpectations(VdcActionType.RemoveVm, RemoveVmParameters.class, new String[]{ - "VmId", "Force"}, new Object[]{GUIDS[0], Boolean.TRUE}, true, true); - verifyRemove(collection.remove(GUIDS[0].toString(), new Action(){{setForce(true);}})); - } - - @Test - public void testRemoveDetachOnly() throws Exception { - mockUniqueOsNames(); - setUriInfo(setUpBasicUriExpectations()); - setUpGetEntityExpectations(); - setUpGetPayloadExpectations(1, 0); - setUpGetBallooningExpectations(1); - setUpGetGraphicsExpectations(1); - setUpActionExpectations(VdcActionType.RemoveVm, RemoveVmParameters.class, new String[] { - "VmId", "RemoveDisks" }, new Object[] { GUIDS[0], Boolean.FALSE }, true, true); - - Action action = new Action(); - action.setVm(new VM()); - action.getVm().setDisks(new Disks()); - action.getVm().getDisks().setDetachOnly(true); - verifyRemove(collection.remove(GUIDS[0].toString(), action)); - } - - private void mockUniqueOsNames() { - HashMap<Integer, String> uniqueOsNames = new HashMap<>(); - expect(osRepository.getUniqueOsNames()).andReturn(uniqueOsNames); - uniqueOsNames.put(0, "Fedora20"); - } - - @Test - public void testRemoveForcedIncomplete() throws Exception { - setUriInfo(setUpBasicUriExpectations()); - mockUniqueOsNames(); - setUpGetEntityExpectations(); - setUpGetPayloadExpectations(1, 0); - setUpGetBallooningExpectations(1); - setUpGetGraphicsExpectations(1); - setUpActionExpectations(VdcActionType.RemoveVm, RemoveVmParameters.class, new String[] { - "VmId", "Force" }, new Object[] { GUIDS[0], Boolean.FALSE }, true, true); - verifyRemove(collection.remove(GUIDS[0].toString(), new Action() {{ - }})); - } - - @Test - public void testRemoveNonExistant() throws Exception{ - setUpGetEntityExpectations(VdcQueryType.GetVmByVmId, - IdQueryParameters.class, - new String[]{"Id"}, - new Object[]{NON_EXISTANT_GUID}, - null); - setUriInfo(setUpBasicUriExpectations()); - control.replay(); - try { - collection.remove(NON_EXISTANT_GUID.toString()); - fail("expected WebApplicationException"); - } catch (WebApplicationException wae) { - assertNotNull(wae.getResponse()); - assertEquals(404, wae.getResponse().getStatus()); - } - } - - private void setUpGetEntityExpectations() throws Exception { - setUpGetEntityExpectations(VdcQueryType.GetVmByVmId, - IdQueryParameters.class, - new String[]{"Id"}, - new Object[]{GUIDS[0]}, - new org.ovirt.engine.core.common.businessentities.VM()); - } - - @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(); - mockUniqueOsNames(); - setUpGetPayloadExpectations(1, 0); - setUpGetBallooningExpectations(1); - setUpGetGraphicsExpectations(1); - setUriInfo(setUpActionExpectations(VdcActionType.RemoveVm, - RemoveVmParameters.class, - new String[] { "VmId", "Force" }, - new Object[] { GUIDS[0], Boolean.FALSE }, - canDo, - success)); - try { - collection.remove(GUIDS[0].toString()); - fail("expected WebApplicationException"); - } catch (WebApplicationException wae) { - verifyFault(wae, detail); } } @@ -1614,6 +1482,16 @@ super.setUpQueryExpectations(query, failure); } + protected void setUpGetGraphicsExpectations(int times) throws Exception { + for (int i = 0; i < times; i++) { + setUpGetEntityExpectations(VdcQueryType.GetGraphicsDevices, + IdQueryParameters.class, + new String[] {}, + new Object[] {}, + Arrays.asList(new GraphicsDevice(VmDeviceType.SPICE))); + } + } + private List<VmInit> setUpVmInit() { List<VmInit> vminits = new ArrayList<>(NAMES.length); for (int i = 0; i < NAMES.length; i++) { -- To view, visit https://gerrit.ovirt.org/42074 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I1d653075475a136eccf4c2ea0bf3921dae3e1bb8 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ori Liel <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
