Juan Hernandez has posted comments on this change. Change subject: restapi: adjust api to new graphics representation ......................................................................
Patch Set 1: (11 comments) http://gerrit.ovirt.org/#/c/35281/1/backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/model/DisplayType.java File backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/model/DisplayType.java: Line 50: return GraphicsType.VNC; Line 51: default: Line 52: return null; Line 53: } Line 54: } These two methods should go in a WhateverMapper class, not in the type itself. Consider creating a new DisplayMapper class, and put the methods there. http://gerrit.ovirt.org/#/c/35281/1/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendInstanceTypeResource.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendInstanceTypeResource.java: Line 114: updateParams.setUpdateRngDevice(true); Line 115: updateParams.setRngDevice(RngDeviceMapper.map(incoming.getRngDevice(), null)); Line 116: } Line 117: Line 118: BackendTemplatesResource.setGraphicsToParams(incoming, updateParams); Can this method be moved to a new DisplayHelper class? Line 119: Line 120: return getMapper(modelType, UpdateVmTemplateParameters.class).map(incoming, updateParams); Line 121: } Line 122: } http://gerrit.ovirt.org/#/c/35281/1/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendInstanceTypesResource.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendInstanceTypesResource.java: Line 156: } else { Line 157: instanceType.setDisplay(null); Line 158: } Line 159: } Line 160: } Can this be moved to a new DisplayHelper class? Line 161: http://gerrit.ovirt.org/#/c/35281/1/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendTemplateResource.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendTemplateResource.java: Line 138: params.setUpdateRngDevice(true); Line 139: params.setRngDevice(RngDeviceMapper.map(incoming.getRngDevice(), null)); Line 140: } Line 141: Line 142: BackendTemplatesResource.setGraphicsToParams(incoming, params); Can this method be moved to a new DisplayHelper class? Line 143: Line 144: return getMapper(modelType, UpdateVmTemplateParameters.class).map(incoming, params); Line 145: } Line 146: } http://gerrit.ovirt.org/#/c/35281/1/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendTemplatesResource.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendTemplatesResource.java: Line 289: params.getGraphicsDevices().put(newGraphicsType, Line 290: new GraphicsDevice(newGraphicsType.getCorrespondingDeviceType())); Line 291: } Line 292: } Line 293: } Can these two methods be moved to a new DisplayHelper class? Line 294: http://gerrit.ovirt.org/#/c/35281/1/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmResource.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmResource.java: Line 538: params.setUpdateRngDevice(true); Line 539: params.setRngDevice(RngDeviceMapper.map(incoming.getRngDevice(), null)); Line 540: } Line 541: Line 542: BackendVmsResource.setGraphicsToParams(incoming, params); Can this method be moved to a new DisplayHelper class? Line 543: Line 544: if (incoming.isSetInstanceType() && (incoming.getInstanceType().isSetId() || incoming.getInstanceType().isSetName())) { Line 545: updated.setInstanceTypeId(lookupInstanceTypeId(incoming.getInstanceType())); Line 546: } else if (incoming.isSetInstanceType()) { http://gerrit.ovirt.org/#/c/35281/1/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmsResource.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmsResource.java: Line 347: params.getGraphicsDevices().put(newGraphicsType, Line 348: new GraphicsDevice(newGraphicsType.getCorrespondingDeviceType())); Line 349: } Line 350: } Line 351: } Can this be moved o a new DisplayHelper class, similar to VmHelper? Line 352: Line 353: private HashMap<Guid, DiskImage> getDisksToClone(Disks disks, Guid templateId) { Line 354: HashMap<Guid, DiskImage> disksMap = new HashMap<Guid, DiskImage>(); Line 355: Line 563: } else { Line 564: vm.setDisplay(null); Line 565: } Line 566: } Line 567: } Can this be made static and moved to DisplayHelper as well? Line 568: Line 569: protected void setPayload(VM vm) { Line 570: try { Line 571: VmPayload payload = getEntity(VmPayload.class, http://gerrit.ovirt.org/#/c/35281/1/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/util/VmHelper.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/util/VmHelper.java: Line 45: } Line 46: } Line 47: Line 48: return graphicsTypes; Line 49: } Create a new DisplayHelper class similar to this one and move this method there, if possible. Line 50: http://gerrit.ovirt.org/#/c/35281/1/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/TemplateMapper.java File backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/TemplateMapper.java: Line 379: model.getDisplay().setAllowOverride(entity.isAllowConsoleReconnect()); Line 380: model.getDisplay().setSmartcardEnabled(entity.isSmartcardEnabled()); Line 381: model.getDisplay().setKeyboardLayout(entity.getVncKeyboardLayout()); Line 382: model.getDisplay().setFileTransferEnabled(entity.isSpiceFileTransferEnabled()); Line 383: model.getDisplay().setCopyPasteEnabled(entity.isSpiceCopyPasteEnabled()); Can we move all this display mapping logic to a new DisplayMapper class? Line 384: if (entity.getClusterArch() != null) { Line 385: model.getCpu().setArchitecture(CPUMapper.map(entity.getClusterArch(), null)); Line 386: } Line 387: if (entity.getCreationDate() != null) { http://gerrit.ovirt.org/#/c/35281/1/backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/VmMapper.java File backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/VmMapper.java: Line 691: params.setRunOnceGraphics(graphics); Line 692: } Line 693: } Line 694: } Line 695: } Can we move all the display mapping logic to a new DisplayMapper class? Line 696: Line 697: @Mapping(from = String.class, to = CustomProperties.class) Line 698: public static CustomProperties map(String entity, CustomProperties template) { Line 699: CustomProperties model = template != null ? template : new CustomProperties(); -- To view, visit http://gerrit.ovirt.org/35281 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibed6bbbce8a36dff82a0903c3478cfb656589fb4 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Frank Kobzik <[email protected]> Gerrit-Reviewer: Frank Kobzik <[email protected]> Gerrit-Reviewer: Juan Hernandez <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
