Juan Hernandez has uploaded a new change for review. Change subject: restapi: Adjust display details when adding or updating VMs ......................................................................
restapi: Adjust display details when adding or updating VMs A previous change modifed the way the display information is populated to support the new concept of "graphics devices". But this change didn't take into account that the display information needs to be adjusted for backwards compatibility not only when retrieving VMs, but also when adding or modifying them, as those operations also return the representation of the VM. As a result when a VM is added or updated the display type field isn't populated and that casuses issues for some clients that expect this field. In particular this breaks the "rbovirt" Ruby client. This patch changes the RESTAPI so that the display adjustment is also performed when adding or updating VMs. Change-Id: I35c8d28b9278294f780b073ee173520d501797f8 Related: https://bugzilla.redhat.com/1033547 Related: https://gerrit.ovirt.org/35281 Signed-off-by: Juan Hernandez <[email protected]> --- 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 4 files changed, 60 insertions(+), 34 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/79/40679/1 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 0f53f82..1a7f613 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 @@ -110,17 +110,21 @@ } else { vm = performGet(VdcQueryType.GetVmByVmId, new IdQueryParameters(guid)); } - DisplayHelper.adjustDisplayData(this, vm); - return removeRestrictedInfo(vm); + + if (vm != null) { + DisplayHelper.adjustDisplayData(this, vm); + removeRestrictedInfo(vm); + } + + return vm; } - private VM removeRestrictedInfo(VM vm) { + private void removeRestrictedInfo(VM vm) { // Filtered users are not allowed to view host related information - if (vm != null && isFiltered()) { + if (isFiltered()) { vm.setHost(null); vm.setPlacementPolicy(null); } - return vm; } @Override @@ -145,11 +149,19 @@ incoming.setPlacementPolicy(null); } - return removeRestrictedInfo( - performUpdate(incoming, - new QueryIdResolver<Guid>(VdcQueryType.GetVmByVmId, IdQueryParameters.class), - VdcActionType.UpdateVm, - new UpdateParametersProvider())); + VM vm = performUpdate( + incoming, + new QueryIdResolver<Guid>(VdcQueryType.GetVmByVmId, IdQueryParameters.class), + VdcActionType.UpdateVm, + new UpdateParametersProvider() + ); + + if (vm != null) { + DisplayHelper.adjustDisplayData(this, vm); + removeRestrictedInfo(vm); + } + + return vm; } private void validateParameters(VM incoming) { 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 c86cd1b..9025558 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 @@ -178,7 +178,14 @@ } } } - return removeRestrictedInfoFromResponse(response); + + VM result = (VM) response.getEntity(); + if (result != null) { + DisplayHelper.adjustDisplayData(this, result); + removeRestrictedInfo(result); + } + + return response; } private boolean shouldMakeCreatorExplicitOwner() { @@ -211,16 +218,8 @@ diskImagesByImageId); } - private Response removeRestrictedInfoFromResponse(Response response) { + private VM removeRestrictedInfo(VM vm) { if (isFiltered()) { - VM vm = (VM) response.getEntity(); - removeRestrictedInfoFromVM(vm); - } - return response; - } - - private VM removeRestrictedInfoFromVM(VM vm) { - if (vm != null) { vm.setHost(null); vm.setPlacementPolicy(null); } @@ -582,10 +581,7 @@ for (org.ovirt.engine.core.common.businessentities.VM entity : entities) { VM vm = map(entity); DisplayHelper.adjustDisplayData(this, vm); - // Filtered users are not allowed to view host related information - if (isFiltered) { - removeRestrictedInfoFromVM(vm); - } + removeRestrictedInfo(vm); collection.getVMs().add(addLinks(populate(vm, entity))); } } 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 5234b4d..e28d7f3 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 @@ -237,7 +237,7 @@ setUpGetPayloadExpectations(0, 2); setUpGetBallooningExpectations(); setUpGetBallooningExpectations(); - setUpGetGraphicsExpectations(1); + setUpGetGraphicsExpectations(2); setUpGetConsoleExpectations(new int[]{0}); setUpGetVmOvfExpectations(new int[]{0}); setUpGetVirtioScsiExpectations(new int[] {0}); @@ -267,12 +267,13 @@ setUpGetBallooningExpectations(); setUpGetBallooningExpectations(); - setUpGetGraphicsExpectations(1); + setUpGetGraphicsExpectations(2); setUpGetConsoleExpectations(new int[]{0}); setUpGetVmOvfExpectations(new int[]{0}); setUpGetVirtioScsiExpectations(new int[] {0}); setUpGetSoundcardExpectations(new int[] {0}); setUpGetRngDeviceExpectations(new int[]{0}); + setUriInfo(setUpActionExpectations(VdcActionType.UpdateVm, VmManagementParametersBase.class, @@ -304,9 +305,9 @@ public void testUpdateVmPolicy() throws Exception { setUpGetEntityExpectations(3); setUpEntityQueryExpectations(VdcQueryType.GetVdsGroupByVdsGroupId, - IdQueryParameters.class, - new String[] { "Id" }, - new Object[] { GUIDS[2] }, + IdQueryParameters.class, + new String[]{"Id"}, + new Object[]{GUIDS[2] }, getVdsGroupEntity()); setUpGetPayloadExpectations(0, 2); @@ -318,6 +319,7 @@ setUpGetVirtioScsiExpectations(new int[] {0}); setUpGetSoundcardExpectations(new int[] {0}); setUpGetRngDeviceExpectations(new int[]{0}); + setUpGetGraphicsExpectations(1); setUpEntityQueryExpectations(VdcQueryType.GetVdsStaticByName, NameQueryParameters.class, new String[] { "Name" }, @@ -360,7 +362,7 @@ setUpGetVirtioScsiExpectations(0); setUpGetSoundcardExpectations(0); setUpGetRngDeviceExpectations(0); - setUpGetGraphicsExpectations(1); + setUpGetGraphicsExpectations(2); setUriInfo(setUpActionExpectations(VdcActionType.ChangeVMCluster, ChangeVMClusterParameters.class, new String[]{"ClusterId", "VmId"}, @@ -1141,8 +1143,8 @@ for (int i = 0; i < times; i++) { setUpGetEntityExpectations(VdcQueryType.GetGraphicsDevices, IdQueryParameters.class, - new String[] { "Id" }, - new Object[] { GUIDS[i] }, + new String[] {}, + new Object[] {}, Arrays.asList(new GraphicsDevice(VmDeviceType.SPICE))); } } 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 df91b2c..b6f83c5 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 @@ -145,8 +145,8 @@ for (int i = 0; i < times; i++) { setUpGetEntityExpectations(VdcQueryType.GetGraphicsDevices, IdQueryParameters.class, - new String[] { "Id" }, - new Object[] { GUIDS[i] }, + new String[] {}, + new Object[] {}, Arrays.asList(new GraphicsDevice(VmDeviceType.SPICE))); } } @@ -277,6 +277,7 @@ setUriInfo(setUpBasicUriExpectations()); setUpGetPayloadExpectations(1, 0); setUpGetBallooningExpectations(1, 0); + setUpGetGraphicsExpectations(1); setUpGetConsoleExpectations(new int[]{0}); setUpGetVirtioScsiExpectations(new int[]{0}); setUpGetSoundcardExpectations(new int[]{0}); @@ -334,6 +335,7 @@ setUpGetSoundcardExpectations(new int[]{0, 0}); setUpGetRngDeviceExpectations(new int[]{0, 0}); setUpGetBallooningExpectations(2, 0); + setUpGetGraphicsExpectations(1); setUpGetCertuficateExpectations(2, 0); setUpEntityQueryExpectations(VdcQueryType.GetVmByVmId, IdQueryParameters.class, @@ -383,6 +385,7 @@ setUriInfo(setUpBasicUriExpectations()); setUpGetPayloadExpectations(2, 0); setUpGetBallooningExpectations(2, 0); + setUpGetGraphicsExpectations(1); setUpGetConsoleExpectations(new int[]{0, 0}); setUpGetVmOvfExpectations(new int[]{0, 0}); setUpGetVirtioScsiExpectations(new int[]{0, 0}); @@ -486,6 +489,7 @@ setUriInfo(setUpBasicUriExpectations()); setUpGetPayloadExpectations(1, 2); setUpGetBallooningExpectations(1, 2); + setUpGetGraphicsExpectations(1); setUpGetConsoleExpectations(new int[]{2}); setUpGetVmOvfExpectations(new int[]{2}); setUpGetVirtioScsiExpectations(new int[]{2}); @@ -532,6 +536,7 @@ setUriInfo(setUpBasicUriExpectations()); setUpGetPayloadExpectations(1, 2); setUpGetBallooningExpectations(1, 2); + setUpGetGraphicsExpectations(1); setUpGetCertuficateExpectations(1, 2); setUpGetConsoleExpectations(2, 2); setUpGetVmOvfExpectations(2); @@ -569,6 +574,7 @@ setUriInfo(setUpBasicUriExpectations()); setUpGetPayloadExpectations(1, 2); setUpGetBallooningExpectations(1, 2); + setUpGetGraphicsExpectations(1); setUpGetConsoleExpectations(2); setUpGetVmOvfExpectations(2); setUpGetVirtioScsiExpectations(2); @@ -609,6 +615,7 @@ setUriInfo(setUpBasicUriExpectations()); setUpGetPayloadExpectations(1, 2); setUpGetBallooningExpectations(1, 2); + setUpGetGraphicsExpectations(1); setUpGetCertuficateExpectations(1, 2); setUpGetConsoleExpectations(new int[]{2}); setUpGetVmOvfExpectations(new int[]{2}); @@ -658,6 +665,7 @@ setUriInfo(setUpBasicUriExpectations()); setUpGetPayloadExpectations(1, 2); setUpGetBallooningExpectations(1, 2); + setUpGetGraphicsExpectations(1); setUpGetCertuficateExpectations(1, 2); setUpGetConsoleExpectations(new int[] { 2 }); setUpGetVirtioScsiExpectations(new int[]{2}); @@ -697,6 +705,7 @@ setUriInfo(setUpBasicUriExpectations()); setUpGetPayloadExpectations(1, 3); setUpGetBallooningExpectations(1, 3); + setUpGetGraphicsExpectations(1); setUpGetCertuficateExpectations(1, 3); setUpGetConsoleExpectations(new int[] { 3 }); setUpGetVmOvfExpectations(new int[] { 3 }); @@ -742,6 +751,7 @@ setUriInfo(setUpBasicUriExpectations()); setUpGetPayloadExpectations(1, 2); setUpGetBallooningExpectations(1, 2); + setUpGetGraphicsExpectations(1); setUpGetCertuficateExpectations(1, 2); setUpGetConsoleExpectations(new int[]{2}); setUpGetVmOvfExpectations(new int[]{2}); @@ -782,6 +792,7 @@ setUriInfo(setUpBasicUriExpectations()); setUpGetPayloadExpectations(1, 2); setUpGetBallooningExpectations(1, 2); + setUpGetGraphicsExpectations(1); setUpGetCertuficateExpectations(1, 2); setUpGetConsoleExpectations(new int[]{2}); setUpGetVmOvfExpectations(new int[]{2}); @@ -882,6 +893,7 @@ setUriInfo(setUpBasicUriExpectations()); setUpGetPayloadExpectations(1, 2); setUpGetBallooningExpectations(1, 2); + setUpGetGraphicsExpectations(1); setUpGetConsoleExpectations(new int[]{2}); setUpGetVmOvfExpectations(new int[]{2}); setUpGetVirtioScsiExpectations(new int[]{2}); @@ -938,6 +950,7 @@ setUriInfo(setUpBasicUriExpectations()); setUpGetPayloadExpectations(1, 2); setUpGetBallooningExpectations(1, 2); + setUpGetGraphicsExpectations(1); setUpGetConsoleExpectations(new int[]{2}); setUpGetVmOvfExpectations(new int[]{2}); setUpGetVirtioScsiExpectations(new int[]{2}); @@ -981,6 +994,7 @@ setUriInfo(setUpBasicUriExpectations()); setUpGetPayloadExpectations(1, 2); setUpGetBallooningExpectations(1, 2); + setUpGetGraphicsExpectations(1); setUpGetConsoleExpectations(new int[]{2}); setUpGetVmOvfExpectations(new int[]{2}); setUpGetVirtioScsiExpectations(new int[]{2}); @@ -1041,6 +1055,7 @@ setUriInfo(setUpBasicUriExpectations()); setUpGetPayloadExpectations(1, 2); setUpGetBallooningExpectations(1, 2); + setUpGetGraphicsExpectations(1); setUpGetCertuficateExpectations(1, 2); setUpGetConsoleExpectations(new int[]{2}); setUpGetVmOvfExpectations(new int[]{2}); @@ -1094,6 +1109,7 @@ setUriInfo(setUpBasicUriExpectations()); setUpGetPayloadExpectations(1, 2); setUpGetBallooningExpectations(1, 2); + setUpGetGraphicsExpectations(1); setUpGetCertuficateExpectations(1, 2); setUpGetConsoleExpectations(new int[]{2}); -- To view, visit https://gerrit.ovirt.org/40679 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I35c8d28b9278294f780b073ee173520d501797f8 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Juan Hernandez <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
