Michael Pasternak has posted comments on this change.
Change subject: API : API Refactoring
......................................................................
Patch Set 2: I would prefer that you didn't submit this
(17 inline comments)
Eli,
I'm suggesting already asking pre-integration to verify this patch (after
fixing comments).
thanks.
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendAssignedTagsResource.java
Line 98: }
Line 99: return handleError(new EntityNotFoundException(name), false);
Line 100: }
Line 101:
Line 102: public <T> tags lookupTagById(T id) {
no need to make this method generic, you know ahead the this is UUID.
Line 103: return getEntity(tags.class, VdcQueryType.GetTagByTagId, new
GetTagByTagIdParameters((Guid) id), id.toString(), true);
Line 104: }
Line 105:
Line 106: protected class TagIdResolver extends EntityIdResolver<Guid> {
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendDevicesResource.java
Line 74:
Line 75: DeviceIdResolver(String name) {
Line 76: this.name = name;
Line 77: }
Line 78: // TODO Auto-generated method stub
remove this comment
Line 79: private Q lookupEntity(Guid id, String name) {
Line 80: for (Q entity : getBackendCollection(queryType,
queryParams)) {
Line 81: if (matchEntity(entity, id) || matchEntity(entity,
name)) {
Line 82: return entity;
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendNetworksResource.java
Line 55: public org.ovirt.engine.core.common.businessentities.Network
lookupNetwork(Guid id) {
Line 56: return lookupNetwork(id, null);
Line 57: }
Line 58:
Line 59: public <T >org.ovirt.engine.core.common.businessentities.Network
lookupNetwork(T id, String name) {
no need to make this method generic, you know ahead the this is UUID.
Line 60: for (org.ovirt.engine.core.common.businessentities.Network
entity : getBackendCollection(queryType, getQueryParameters())) {
Line 61: if ((id != null && id.equals(entity.getId())) ||
Line 62: (name != null && name.equals(entity.getname()))) {
Line 63: return entity;
Line 65: }
Line 66: return null;
Line 67: }
Line 68:
Line 69: public <T> org.ovirt.engine.core.common.businessentities.Network
lookupNetwork(T id, String name, String dataCenterId) {
no need to make this method generic, you know ahead the this is UUID.
Line 70: for (org.ovirt.engine.core.common.businessentities.Network
entity : getBackendCollection(queryType, getQueryParameters())) {
Line 71: if ((id != null && id.equals(entity.getId())) ||
Line 72: (name != null && name.equals(entity.getname()))
Line 73: && (entity.getstorage_pool_id()!=null) &&
(entity.getstorage_pool_id().toString().equals(dataCenterId))) {
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendResource.java
Line 328: // // trivial class construction
Line 329: // }
Line 330: // return params;
Line 331: // }
Line 332:
please remove this code/comment
Line 333:
Line 334: protected <T> VdcQueryParametersBase getQueryParams(Class<?
extends VdcQueryParametersBase> queryParamsClass, T id) {
Line 335: VdcQueryParametersBase params = null;
Line 336: try {
Line 344: // for (Class<?> clazz : ctr.getParameterTypes()){
Line 345: // if (clazz == T)
Line 346: // }
Line 347: // }
Line 348:
please remove this code/comment
Line 349: /**
Line 350: * Convert a string to a Guid, or return a 404 response.
Line 351: *
Line 352: * If an invalid UUID is supplied to a sub-resource locator, this
Line 390: return entity;
Line 391: }
Line 392: }
Line 393:
Line 394: protected abstract class EntityResolver {
should implement IResolver?...
Line 395:
Line 396: public abstract Object lookupEntity(Guid id) throws
BackendFailureException;
Line 397:
Line 398: public Object resolve(Guid id) throws BackendFailureException
{
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendSubResource.java
Line 35: Q entity = getEntity(entityType, query, params, id, true);
Line 36: return addLinks(populate(map(entity, null), entity),
suggestedParentType);
Line 37: }
Line 38:
Line 39: protected Q getEntity(EntityIdResolver<Guid> entityResolver,
boolean notFoundAs404) {
is it always UUID? i'm not sure ... (please make it generic as well)
Line 40: try {
Line 41: return entityResolver.resolve(guid);
Line 42: } catch (Exception e) {
Line 43: return handleError(entityType, e, notFoundAs404);
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendAttachedStorageDomainsResource.java
Line 90: SearchType.StorageDomain,
Line 91: "Storage: name=" + name).getId();
Line 92: }
Line 93:
Line 94: protected <T> storage_domains lookupStorageDomainById(T
storageDomainId) {
no need to make this method generic, you know ahead the this is UUID.
Line 95: return getEntity(storage_domains.class,
Line 96:
VdcQueryType.GetStorageDomainByIdAndStoragePoolId,
Line 97: new
StorageDomainAndPoolQueryParameters((Guid) storageDomainId, dataCenterId),
Line 98: storageDomainId.toString());
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendGroupsResourceBase.java
Line 155: getSearchPattern("*")));
Line 156:
Line 157: }
Line 158:
Line 159: public <T> ad_groups lookupGroupById(T id) {
no need to make this method generic, you know ahead the this is UUID.
Line 160: return getEntity(ad_groups.class,
Line 161: VdcQueryType.GetAdGroupById,
Line 162: new GetAdGroupByIdParameters((Guid) id),
Line 163: id.toString(),
Line 158:
Line 159: public <T> ad_groups lookupGroupById(T id) {
Line 160: return getEntity(ad_groups.class,
Line 161: VdcQueryType.GetAdGroupById,
Line 162: new GetAdGroupByIdParameters((Guid) id),
no need for this cast
Line 163: id.toString(),
Line 164: true);
Line 165: }
Line 166:
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendHostNicResource.java
Line 79: public StatisticsResource getStatisticsResource() {
Line 80: EntityIdResolver<Guid> resolver = new EntityIdResolver<Guid>()
{
Line 81: @Override
Line 82: public VdsNetworkInterface lookupEntity(Guid id) throws
BackendFailureException {
Line 83: return parent.lookupInterface(id.toString());
this method was receiving UUID and noting is changed, so why do you pass now
id.toString()?
Line 84: }
Line 85: };
Line 86: HostNicStatisticalQuery query = new
HostNicStatisticalQuery(resolver, newModel(id));
Line 87: return inject(new BackendStatisticsResource<HostNIC,
VdsNetworkInterface>(entityType, guid, query));
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendNicResource.java
Line 33: @Override
Line 34: public StatisticsResource getStatisticsResource() {
Line 35: EntityIdResolver<Guid> resolver = new EntityIdResolver<Guid>()
{
Line 36: @Override
Line 37: public VmNetworkInterface lookupEntity(Guid id)
you renamed guid arg in to id, what about all the places it is used? - i
suggest you revert this change.
Line 38: throws BackendFailureException {
Line 39: return collection.lookupEntity(guid);
Line 40: }
Line 41: };
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendSnapshotsResource.java
Line 90: org.ovirt.engine.core.common.businessentities.VM vm =
getEntity(org.ovirt.engine.core.common.businessentities.VM.class,
VdcQueryType.GetVmConfigurationBySnapshot, new
GetVmConfigurationBySnapshotQueryParams(asGuid(snapshot.getId())), null);
Line 91: return vm;
Line 92: }
Line 93:
Line 94: protected <T>
org.ovirt.engine.core.common.businessentities.Snapshot getSnapshotById(T id) {
no need to make this method generic, you know ahead the this is UUID.
Line 95: //TODO: move to 'GetSnapshotBySnapshotId' once Backend
supplies it.
Line 96: for (org.ovirt.engine.core.common.businessentities.Snapshot
snapshot : getBackendCollection(VdcQueryType.GetAllVmSnapshotsByVmId,
Line 97: new GetAllVmSnapshotsByVmIdParameters(parentId))) {
Line 98: if (snapshot.getId().equals(id)) {
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendUsersResourceBase.java
Line 154: protected User mapAdUser(AdUser adUser) {
Line 155: return getMapper(AdUser.class, User.class).map(adUser, null);
Line 156: }
Line 157:
Line 158: public <T> DbUser lookupUserById(T id) {
no need to make this method generic, you know ahead the this is UUID.
Line 159: return getEntity(DbUser.class,
Line 160: VdcQueryType.GetDbUserByUserId,
Line 161: new GetDbUserByUserIdParameters((Guid) id),
Line 162: id.toString());
Line 157:
Line 158: public <T> DbUser lookupUserById(T id) {
Line 159: return getEntity(DbUser.class,
Line 160: VdcQueryType.GetDbUserByUserId,
Line 161: new GetDbUserByUserIdParameters((Guid) id),
no need for this cast
Line 162: id.toString());
Line 163: }
Line 164:
Line 165: protected class UserIdResolver extends EntityIdResolver<Guid> {
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmDiskResource.java
Line 37: EntityIdResolver<Guid> resolver = new EntityIdResolver<Guid>()
{
Line 38:
Line 39: @Override
Line 40: public org.ovirt.engine.core.common.businessentities.Disk
lookupEntity(
Line 41: Guid id) throws BackendFailureException {
you renamed guid arg in to id, but what about all the places it is used, - i
suggest you revert this chnage
Line 42: return collection.lookupEntity(guid);
Line 43: }
Line 44: };
Line 45: DiskStatisticalQuery query = new
DiskStatisticalQuery(resolver, newModel(id));
--
To view, visit http://gerrit.ovirt.org/10096
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic1c7100e31be004fdb325cb00e20042aa1b30df6
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Eli Mesika <[email protected]>
Gerrit-Reviewer: Eli Mesika <[email protected]>
Gerrit-Reviewer: Eyal Edri <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[email protected]>
Gerrit-Reviewer: Ohad Basan <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
