Daniel Erez has posted comments on this change. Change subject: rest: retrieve disk snapshots list by storage domain ......................................................................
Patch Set 5: (7 comments) http://gerrit.ovirt.org/#/c/26730/5/backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd File backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd: Line 3094: <xs:element ref="disk" minOccurs="0" maxOccurs="1"/> Line 3095: </xs:sequence> Line 3096: </xs:extension> Line 3097: </xs:complexContent> Line 3098: </xs:complexType> > Please remember to fix indentation before merging: two spaces per level. Done Line 3099: Line 3100: <xs:complexType name="DiskSnapshots"> Line 3101: <xs:complexContent> Line 3102: <xs:extension base="BaseDevices"> http://gerrit.ovirt.org/#/c/26730/5/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendStorageDomainDiskSnapshotResource.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendStorageDomainDiskSnapshotResource.java: Line 17: this.storageDomainId = parent.getStorageDomainId().toString(); Line 18: } Line 19: Line 20: @Override Line 21: protected DiskSnapshot deprecatedPopulate(DiskSnapshot model, org.ovirt.engine.core.common.businessentities.Disk entity) { > same comment as in BackendStorageDomainDiskSnapshotsResource.java Done Line 22: DiskSnapshot populatedDisk = doPopulate(model, entity); Line 23: Line 24: // this code generates back-link to the corresponding SD Line 25: populatedDisk.setStorageDomain(new StorageDomain()); http://gerrit.ovirt.org/#/c/26730/5/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendStorageDomainDiskSnapshotsResource.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendStorageDomainDiskSnapshotsResource.java: Line 32: @Override Line 33: protected DiskSnapshot addLinks(DiskSnapshot model, Line 34: Class<? extends BaseResource> suggestedParent, Line 35: String... excludeSubCollectionMembers) { Line 36: return super.addLinks(model, StorageDomain.class, excludeSubCollectionMembers); > why necessary to override addLinks()? Was needed before DiskSnapshot entity - could be removed now. Line 37: } Line 38: Line 39: @Override Line 40: protected DiskSnapshot addLinks(DiskSnapshot model, String... subCollectionMembersToExclude) { Line 37: } Line 38: Line 39: @Override Line 40: protected DiskSnapshot addLinks(DiskSnapshot model, String... subCollectionMembersToExclude) { Line 41: return addLinks(model, StorageDomain.class, subCollectionMembersToExclude); > why necessary to override addLinks()? Done Line 42: } Line 43: Line 44: @Override Line 45: public DiskSnapshots list() { Line 49: Line 50: protected DiskSnapshots mapCollection(List<org.ovirt.engine.core.common.businessentities.Disk> entities) { Line 51: DiskSnapshots collection = new DiskSnapshots(); Line 52: for (org.ovirt.engine.core.common.businessentities.Disk disk : entities) { Line 53: DiskSnapshot diskSnapshot = DiskSnapshotMapper.map(disk, null); > Consider using the mapping locator: Done Line 54: collection.getDiskSnapshots().add(addLinks(populate(diskSnapshot, disk))); Line 55: } Line 56: return collection; Line 57: } Line 57: } Line 58: Line 59: @Override Line 60: protected Response performRemove(String id) { Line 61: DiskImage diskImage = (DiskImage) DiskSnapshotMapper.map(getDeviceSubResource(id).get(), null); > 1) is getDeviceSubResource(id).get() really necessary? you already have the Done Line 62: return performAction(VdcActionType.RemoveDiskSnapshots, new RemoveDiskSnapshotsParameters(diskImage)); Line 63: } Line 64: Line 65: @Override Line 71: protected DiskSnapshot deprecatedPopulate(DiskSnapshot model, org.ovirt.engine.core.common.businessentities.Disk entity) { Line 72: DiskSnapshot populatedDisk = doPopulate(model, entity); Line 73: Line 74: // this code generates back-link to the corresponding SD Line 75: populatedDisk.setStorageDomain(new StorageDomain()); > Move this code into list(). Done Line 76: populatedDisk.getStorageDomain().setId(this.storageDomainId.toString()); Line 77: Line 78: return model; Line 79: } -- To view, visit http://gerrit.ovirt.org/26730 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5359c5df490c7e7e6f9b4d25e794bd07c6877339 Gerrit-PatchSet: 5 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Daniel Erez <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Juan Hernandez <[email protected]> Gerrit-Reviewer: Ori Liel <[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
