Juan Hernandez has uploaded a new change for review. Change subject: restapi: Move cluster remove from collection to entity ......................................................................
restapi: Move cluster remove from collection to entity Currently the methods implementing the DELETE operation are part of the collection interfaces. This causes an issue with recent version of Resteasy that implement the JAX-RS specification strictly. The issue is that the remove resource method and the subresource locator use the same URI pattern "/cluster/{id}". According to the JAX-RS specification when two methods use the same URI pattern the resource method should be used and the resource locator should be ignored. See section 3.7.2, step 2.h of the resource matching algorithm. To avoid this problem this patch moves the remove method from the collection resource to the entity resource, but only for the cluster entity. Subsequent patches will move all the other remove methods. Change-Id: Ife8a3eb08ab379ffc0ea7b41c49f164cb55c15f2 Signed-off-by: Juan Hernandez <juan.hernan...@redhat.com> --- M backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/ClusterResource.java M backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/ClustersResource.java M backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendCollectionResource.java M backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendClusterResource.java M backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendClustersResource.java M backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendClusterResourceTest.java M backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendClustersResourceTest.java M backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendDataCenterClusterResourceTest.java M backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendDataCenterClustersResourceTest.java 9 files changed, 167 insertions(+), 141 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/83/41783/1 diff --git a/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/ClusterResource.java b/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/ClusterResource.java index c8e1227..1f0b735 100644 --- a/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/ClusterResource.java +++ b/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/ClusterResource.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.Produces; @@ -30,6 +31,8 @@ @Produces({ApiMediaType.APPLICATION_XML, ApiMediaType.APPLICATION_JSON, ApiMediaType.APPLICATION_X_YAML}) public interface ClusterResource extends UpdatableResource<Cluster> { + @DELETE + Response remove(); @Path("networks") public AssignedNetworksResource getAssignedNetworksSubResource(); diff --git a/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/ClustersResource.java b/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/ClustersResource.java index 9199624..0f9b50c 100644 --- a/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/ClustersResource.java +++ b/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/ClustersResource.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; @@ -38,10 +37,6 @@ @POST @Consumes({ApiMediaType.APPLICATION_XML, ApiMediaType.APPLICATION_JSON, ApiMediaType.APPLICATION_X_YAML}) public Response add(Cluster cluster); - - @DELETE - @Path("{id}") - public Response remove(@PathParam("id") String id); /** * Sub-resource locator method, returns individual ClusterResource on which the diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendCollectionResource.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendCollectionResource.java index 1010733..8d8d6ff 100644 --- a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendCollectionResource.java +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendCollectionResource.java @@ -47,7 +47,9 @@ return performRemove(id); } - protected abstract Response performRemove(String id); + protected Response performRemove(String id) { + throw new UnsupportedOperationException(); + } protected List<Q> getBackendCollection(SearchType searchType) { return getBackendCollection(searchType, QueryHelper.getConstraint(getUriInfo(), "", modelType)); diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendClusterResource.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendClusterResource.java index 53cbec6..a0b68a4 100644 --- a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendClusterResource.java +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendClusterResource.java @@ -16,6 +16,7 @@ import org.ovirt.engine.core.common.action.ManagementNetworkOnClusterOperationParameters; import org.ovirt.engine.core.common.action.VdcActionParametersBase; import org.ovirt.engine.core.common.action.VdcActionType; +import org.ovirt.engine.core.common.action.VdsGroupParametersBase; import org.ovirt.engine.core.common.businessentities.VDSGroup; import org.ovirt.engine.core.common.queries.GetPermissionsForObjectParameters; import org.ovirt.engine.core.common.queries.IdQueryParameters; @@ -128,4 +129,10 @@ throw new WebApplicationException(Response.status(Response.Status.NOT_FOUND).build()); } } + + @Override + public Response remove() { + get(); + return performAction(VdcActionType.RemoveVdsGroup, new VdsGroupParametersBase(asGuid(id))); + } } diff --git a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendClustersResource.java b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendClustersResource.java index 3631ee3..0c9f92a 100644 --- a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendClustersResource.java +++ b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendClustersResource.java @@ -11,7 +11,6 @@ import org.ovirt.engine.api.resource.ClustersResource; import org.ovirt.engine.core.common.action.ManagementNetworkOnClusterOperationParameters; import org.ovirt.engine.core.common.action.VdcActionType; -import org.ovirt.engine.core.common.action.VdsGroupParametersBase; import org.ovirt.engine.core.common.businessentities.StoragePool; import org.ovirt.engine.core.common.businessentities.VDSGroup; import org.ovirt.engine.core.common.interfaces.SearchType; @@ -103,11 +102,6 @@ final Guid managementNetworkId = managementNetworkFinder.getManagementNetworkId(cluster, dataCenter.getId()); return new ManagementNetworkOnClusterOperationParameters(clusterEntity, managementNetworkId); - } - - @Override - public Response performRemove(String id) { - return performAction(VdcActionType.RemoveVdsGroup, new VdsGroupParametersBase(asGuid(id))); } protected Clusters mapCollection(List<VDSGroup> entities) { diff --git a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendClusterResourceTest.java b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendClusterResourceTest.java index 86ed258..c73cfad 100644 --- a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendClusterResourceTest.java +++ b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendClusterResourceTest.java @@ -9,6 +9,7 @@ import org.ovirt.engine.api.model.Cluster; import org.ovirt.engine.core.common.action.ManagementNetworkOnClusterOperationParameters; import org.ovirt.engine.core.common.action.VdcActionType; +import org.ovirt.engine.core.common.action.VdsGroupParametersBase; import org.ovirt.engine.core.common.businessentities.VDSGroup; import org.ovirt.engine.core.common.businessentities.network.Network; import org.ovirt.engine.core.common.queries.IdQueryParameters; @@ -157,6 +158,68 @@ } } + @Test + public void testRemove() throws Exception { + setUpGetEntityExpectations(1); + setUriInfo( + setUpActionExpectations( + VdcActionType.RemoveVdsGroup, + VdsGroupParametersBase.class, + new String[]{"VdsGroupId"}, + new Object[]{GUIDS[0]}, + true, + true + ) + ); + verifyRemove(resource.remove()); + } + + @Test + public void testRemoveNonExistant() throws Exception{ + setUpGetEntityExpectations(1, true); + control.replay(); + try { + resource.remove(); + fail("expected WebApplicationException"); + } + catch (WebApplicationException wae) { + assertNotNull(wae.getResponse()); + assertEquals(404, wae.getResponse().getStatus()); + } + } + + @Test + public void testRemoveCantDo() throws Exception { + setUpGetEntityExpectations(1); + doTestBadRemove(false, true, CANT_DO); + } + + @Test + public void testRemoveFailed() throws Exception { + setUpGetEntityExpectations(1); + doTestBadRemove(true, false, FAILURE); + } + + protected void doTestBadRemove(boolean canDo, boolean success, String detail) throws Exception { + setUriInfo( + setUpActionExpectations( + VdcActionType.RemoveVdsGroup, + VdsGroupParametersBase.class, + new String[] { "VdsGroupId" }, + new Object[] { GUIDS[0] }, + canDo, + success + ) + ); + try { + resource.remove(); + fail("expected WebApplicationException"); + } + catch (WebApplicationException wae) { + verifyFault(wae, detail); + } + } + protected void setUpGetEntityExpectations(int times) throws Exception { setUpGetEntityExpectations(times, false); } diff --git a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendClustersResourceTest.java b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendClustersResourceTest.java index ddff6b6..2314412 100644 --- a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendClustersResourceTest.java +++ b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendClustersResourceTest.java @@ -12,7 +12,6 @@ import org.ovirt.engine.api.model.Version; import org.ovirt.engine.core.common.action.ManagementNetworkOnClusterOperationParameters; import org.ovirt.engine.core.common.action.VdcActionType; -import org.ovirt.engine.core.common.action.VdsGroupParametersBase; import org.ovirt.engine.core.common.businessentities.StoragePool; import org.ovirt.engine.core.common.businessentities.VDSGroup; import org.ovirt.engine.core.common.interfaces.SearchType; @@ -30,30 +29,6 @@ super(new BackendClustersResource(), SearchType.Cluster, "Clusters : "); } - @Test - public void testRemove() throws Exception { - setUpGetEntityExpectations(); - setUriInfo(setUpActionExpectations(VdcActionType.RemoveVdsGroup, - VdsGroupParametersBase.class, - new String[] { "VdsGroupId" }, - new Object[] { GUIDS[0] }, - true, - true)); - verifyRemove(collection.remove(GUIDS[0].toString())); - } - - @Test - public void testRemoveNonExistant() throws Exception{ - setUpGetEntityExpectations(NON_EXISTANT_GUID, true); - 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(Guid entityId, Boolean returnNull) throws Exception { setUpGetEntityExpectations(VdcQueryType.GetVdsGroupById, @@ -61,37 +36,6 @@ new String[] { "Id" }, new Object[] { entityId }, returnNull ? null : getEntity(0)); - } - - private void setUpGetEntityExpectations() throws Exception { - setUpGetEntityExpectations(GUIDS[0], false); - } - - @Test - public void testRemoveCantDo() throws Exception { - setUpGetEntityExpectations(); - doTestBadRemove(false, true, CANT_DO); - } - - @Test - public void testRemoveFailed() throws Exception { - setUpGetEntityExpectations(); - doTestBadRemove(true, false, FAILURE); - } - - protected void doTestBadRemove(boolean canDo, boolean success, String detail) throws Exception { - setUriInfo(setUpActionExpectations(VdcActionType.RemoveVdsGroup, - VdsGroupParametersBase.class, - new String[] { "VdsGroupId" }, - new Object[] { GUIDS[0] }, - canDo, - success)); - try { - collection.remove(GUIDS[0].toString()); - fail("expected WebApplicationException"); - } catch (WebApplicationException wae) { - verifyFault(wae, detail); - } } @Test diff --git a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendDataCenterClusterResourceTest.java b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendDataCenterClusterResourceTest.java index 344420a..0c07a80 100644 --- a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendDataCenterClusterResourceTest.java +++ b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendDataCenterClusterResourceTest.java @@ -12,6 +12,7 @@ import org.ovirt.engine.api.model.Cluster; import org.ovirt.engine.core.common.action.VdcActionType; import org.ovirt.engine.core.common.action.VdsGroupOperationParameters; +import org.ovirt.engine.core.common.action.VdsGroupParametersBase; import org.ovirt.engine.core.common.businessentities.VDSGroup; import org.ovirt.engine.core.common.businessentities.network.Network; import org.ovirt.engine.core.common.queries.IdQueryParameters; @@ -204,6 +205,96 @@ } } + @Test + public void testRemove() throws Exception { + setUpEntityQueryExpectations( + VdcQueryType.GetVdsGroupsByStoragePoolId, + IdQueryParameters.class, + new String[] { "Id" }, + new Object[] { dataCenterId }, + setUpVDSGroups(), + null + ); + setUriInfo( + setUpActionExpectations( + VdcActionType.RemoveVdsGroup, + VdsGroupParametersBase.class, + new String[] { "VdsGroupId" }, + new Object[] { GUIDS[0] }, + true, + true + ) + ); + verifyRemove(resource.remove()); + } + + @Test + public void testRemoveNonExistant() throws Exception { + setUpEntityQueryExpectations( + VdcQueryType.GetVdsGroupsByStoragePoolId, + IdQueryParameters.class, + new String[] { "Id" }, + new Object[] { dataCenterId }, + new ArrayList<VDSGroup>(), + null + ); + control.replay(); + try { + resource.remove(); + fail("expected WebApplicationException"); + } + catch (WebApplicationException wae) { + assertNotNull(wae.getResponse()); + assertEquals(404, wae.getResponse().getStatus()); + } + } + + @Test + public void testRemoveCantDo() throws Exception { + setUpEntityQueryExpectations( + VdcQueryType.GetVdsGroupsByStoragePoolId, + IdQueryParameters.class, + new String[] { "Id" }, + new Object[] { dataCenterId }, + setUpVDSGroups(), + null + ); + doTestBadRemove(false, true, CANT_DO); + } + + @Test + public void testRemoveFailed() throws Exception { + setUpEntityQueryExpectations( + VdcQueryType.GetVdsGroupsByStoragePoolId, + IdQueryParameters.class, + new String[] { "Id" }, + new Object[] { dataCenterId }, + setUpVDSGroups(), + null + ); + doTestBadRemove(true, false, FAILURE); + } + + protected void doTestBadRemove(boolean canDo, boolean success, String detail) throws Exception { + setUriInfo( + setUpActionExpectations( + VdcActionType.RemoveVdsGroup, + VdsGroupParametersBase.class, + new String[] { "VdsGroupId" }, + new Object[] { GUIDS[0] }, + canDo, + success + ) + ); + try { + resource.remove(); + fail("expected WebApplicationException"); + } + catch (WebApplicationException wae) { + verifyFault(wae, detail); + } + } + protected void setUpGetEntityExpectations(int times) throws Exception { setUpGetEntityExpectations(times, false); } diff --git a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendDataCenterClustersResourceTest.java b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendDataCenterClustersResourceTest.java index 9a27627..98c88a3 100644 --- a/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendDataCenterClustersResourceTest.java +++ b/backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendDataCenterClustersResourceTest.java @@ -13,7 +13,6 @@ import org.ovirt.engine.api.model.Version; import org.ovirt.engine.core.common.action.ManagementNetworkOnClusterOperationParameters; import org.ovirt.engine.core.common.action.VdcActionType; -import org.ovirt.engine.core.common.action.VdsGroupParametersBase; import org.ovirt.engine.core.common.businessentities.StoragePool; import org.ovirt.engine.core.common.businessentities.VDSGroup; import org.ovirt.engine.core.common.queries.IdQueryParameters; @@ -29,78 +28,6 @@ public BackendDataCenterClustersResourceTest() { super(new BackendDataCenterClustersResource(dataCenterId.toString()), null, ""); - } - - @Test - public void testRemove() throws Exception { - setUpEntityQueryExpectations(VdcQueryType.GetVdsGroupsByStoragePoolId, - IdQueryParameters.class, - new String[] { "Id" }, - new Object[] { dataCenterId }, - setUpVDSGroups(), - null); - setUriInfo(setUpActionExpectations(VdcActionType.RemoveVdsGroup, - VdsGroupParametersBase.class, - new String[] { "VdsGroupId" }, - new Object[] { GUIDS[0] }, - true, - true)); - verifyRemove(collection.remove(GUIDS[0].toString())); - } - - @Test - public void testRemoveNonExistant() throws Exception{ - setUpEntityQueryExpectations(VdcQueryType.GetVdsGroupsByStoragePoolId, - IdQueryParameters.class, - new String[] { "Id" }, - new Object[] { dataCenterId }, - setUpVDSGroups(), - null); - control.replay(); - try { - collection.remove(NON_EXISTANT_GUID.toString()); - fail("expected WebApplicationException"); - } catch (WebApplicationException wae) { - assertNotNull(wae.getResponse()); - assertEquals(404, wae.getResponse().getStatus()); - } - } - - @Test - public void testRemoveCantDo() throws Exception { - setUpEntityQueryExpectations(VdcQueryType.GetVdsGroupsByStoragePoolId, - IdQueryParameters.class, - new String[] { "Id" }, - new Object[] { dataCenterId }, - setUpVDSGroups(), - null); - doTestBadRemove(false, true, CANT_DO); - } - - @Test - public void testRemoveFailed() throws Exception { - setUpEntityQueryExpectations(VdcQueryType.GetVdsGroupsByStoragePoolId, - IdQueryParameters.class, - new String[] { "Id" }, - new Object[] { dataCenterId }, - setUpVDSGroups(), - null); - doTestBadRemove(true, false, FAILURE); - } - - protected void doTestBadRemove(boolean canDo, boolean success, String detail) throws Exception { - setUriInfo(setUpActionExpectations(VdcActionType.RemoveVdsGroup, - VdsGroupParametersBase.class, - new String[] { "VdsGroupId" }, - new Object[] { GUIDS[0] }, - canDo, - success)); - try { - collection.remove(GUIDS[0].toString()); - fail("expected WebApplicationException"); - } catch (WebApplicationException wae) { - verifyFault(wae, detail); - } } @Test -- To view, visit https://gerrit.ovirt.org/41783 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ife8a3eb08ab379ffc0ea7b41c49f164cb55c15f2 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