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

Reply via email to