Michael Pasternak has posted comments on this change.
Change subject: restapi: Add "All-Content" Header.
......................................................................
Patch Set 6: (10 inline comments)
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendCollectionResource.java
Line 97: protected List<Q> getBackendCollection(VdcQueryType query,
VdcQueryParametersBase queryParams) {
Line 98: return getBackendCollection(entityType, query, queryParams);
Line 99: }
Line 100:
Line 101: protected <T> Response performCreate(VdcActionType task,
should be final as well?
Line 102: VdcActionParametersBase taskParams,
Line 103: IResolver<T, Q> entityResolver,
Line 104: boolean block) {
Line 105: return performCreate(task, taskParams, entityResolver, block,
null);
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendSubResource.java
Line 70: entity = getEntity(entityResolver, false);
Line 71: return entity;
Line 72: }
Line 73:
Line 74: protected R performUpdate(R incoming,
should be final as well.
Line 75: EntityIdResolver<Guid> entityResolver,
Line 76: VdcActionType update,
Line 77: ParametersProvider<R, Q> updateProvider) {
Line 78: // REVISIT maintain isolation across retrievals and update
Line 87: VdcActionType update,
Line 88: ParametersProvider<R, Q> updateProvider) {
Line 89: Q entity = getEntity(entityResolver, true);
Line 90:
Line 91: validateUpdate(incoming, map(entity));
i'd take getEntity() and validateUpdate() out of doUpdate() to performUpdate()
as this part of the code should not be changed
Line 92:
Line 93: performAction(update, updateProvider.getParameters(incoming,
entity));
Line 94:
Line 95: entity = getEntity(entityResolver, false);
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendResource.java
Line 253: */
Line 254: protected boolean isPopulate() {
Line 255: List<String> populates =
httpHeaders.getRequestHeader(POPULATE);
Line 256: if (populates != null && populates.size() > 0) {
Line 257: return Boolean.valueOf(populates.get(0));
please add .booleanValue() here to save boxing.
Line 258: } else {
Line 259: return false;
Line 260: }
Line 261: }
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendStorageDomainResource.java
Line 183:
model.setStatus(StatusUtils.create(StorageDomainStatus.UNATTACHED));
Line 184: } else {
Line 185: model.setStatus(null);
Line 186: }
Line 187: return model;
no need to return super.deprecatedPopulate() ?
Line 188: }
Line 189:
Line 190: private List<LogicalUnit> getIncomingLuns(Storage storage) {
Line 191: //user may pass the LUNs under Storage, or
Storage-->VolumeGroup; both are supported.
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmPoolResource.java
Line 68:
VdcObjectType.VmPool));
Line 69: }
Line 70:
Line 71: @Override
Line 72: protected VmPool doPopulate(VmPool pool, vm_pools entity) {
should become doDeprecatedPopulate() and new doPopulate() should be added
Line 73: return parent.doPopulate(pool, entity);
Line 74: }
Line 75:
Line 76: protected VM mapToVM(VmPool model) {
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmsResource.java
Line 486:
Line 487: @Override
Line 488: protected VM doPopulate(VM model,
org.ovirt.engine.core.common.businessentities.VM entity) {
Line 489: setPayload(model);
Line 490: setBallooning(model);
both ^ should stay in deprecatedPopulate()
Line 491: setCertificateInfo(model);
Line 492: return model;
Line 493: }
....................................................
File
backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendVmsResourceTest.java
Line 199: }
Line 200:
Line 201: private void doTestAddAsync(AsyncTaskStatusEnum asyncStatus,
CreationStatus creationStatus) throws Exception {
Line 202: setUriInfo(setUpBasicUriExpectations());
Line 203: setUriInfo(setUpBasicUriExpectations());
you already have setUriInfo() at line 202
Line 204: setUpGetPayloadExpectations(1, 0);
Line 205: setUpGetBallooningExpectations(1, 0);
Line 206: setUpGetCertuficateExpectations(1, 0);
Line 207:
setUpEntityQueryExpectations(VdcQueryType.GetVdsGroupByVdsGroupId,
Line 247: @Test
Line 248: public void testAddFromScratch() throws Exception {
Line 249: setUriInfo(setUpBasicUriExpectations());
Line 250: setUpHttpHeaderExpectations("Expect", "201-created");
Line 251: setUriInfo(setUpBasicUriExpectations());
you already have setUriInfo() at line 249
Line 252: setUpGetPayloadExpectations(2, 0);
Line 253: setUpGetBallooningExpectations(2, 0);
Line 254: setUpGetCertuficateExpectations(2, 0);
Line 255: setUpEntityQueryExpectations(VdcQueryType.GetVmByVmId,
....................................................
Commit Message
Line 3: AuthorDate: 2012-12-09 17:27:31 +0200
Line 4: Commit: Ori Liel <ol...@redhat.com>
Line 5: CommitDate: 2013-01-07 13:00:51 +0200
Line 6:
Line 7: restapi: Add "All-Content" Header.
please add BZ id + link
Line 8:
Line 9: Added header 'All-Content' (values: 'true'/'false') that states whether
the entity fetched
Line 10: should contain all possible information, or only the basic inforamtion
(which is most of
Line 11: the information, but there's still a difference).
--
To view, visit http://gerrit.ovirt.org/9815
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2b0faf2843ec368fafd5270dc23d711d0710bd48
Gerrit-PatchSet: 6
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ori Liel <ol...@redhat.com>
Gerrit-Reviewer: Michael Pasternak <mpast...@redhat.com>
Gerrit-Reviewer: Ori Liel <ol...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
