Michael Pasternak has posted comments on this change.
Change subject: restapi : Add /applications sub-collection under vm(#926928)
......................................................................
Patch Set 5: I would prefer that you didn't submit this
(12 inline comments)
1. please add rsdl_meta.yaml records
2. please add this as 3.3 feature in the BackednCapabilitiesResource
....................................................
File
backend/manager/modules/restapi/interface/common/jaxrs/src/main/java/org/ovirt/engine/api/common/util/LinkHelper.java
Line 194:
Line 195: map = new ParentToCollectionMap(DeviceResource.class,
DevicesResource.class, VM.class);
Line 196: TYPES.put(CdRom.class, map);
Line 197:
Line 198: map = new ParentToCollectionMap(DeviceResource.class,
ReadOnlyDevicesResource.class, VM.class);
this is not device
Line 199: TYPES.put(Application.class, map);
Line 200:
Line 201: map = new
ParentToCollectionMap(VmReportedDeviceResource.class,
VmReportedDevicesResource.class, VM.class);
Line 202: TYPES.put(ReportedDevice.class, map);
....................................................
File
backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/VmResource.java
Line 110: @Path("cancelmigration")
Line 111: public Response cancelMigration(Action action);
Line 112:
Line 113: @Path("applications")
Line 114: public ReadOnlyDevicesResource<Application, Applications>
getApplicationsResource();
application/s is/are not devices, also they're not up-datable.
Line 115:
Line 116: @Path("cdroms")
Line 117: public DevicesResource<CdRom, CdRoms> getCdRomsResource();
Line 118:
....................................................
File
backend/manager/modules/restapi/interface/definition/src/main/resources/api.xsd
Line 2332: <xs:element name="nics" type="Nics"/>
Line 2333:
Line 2334: <xs:complexType name="Application">
Line 2335: <xs:complexContent>
Line 2336: <xs:extension base="BaseDevice">
this is not device, inherit from BaseResource instead
Line 2337: <xs:sequence>
Line 2338: <xs:element name="appName" type="xs:string" minOccurs="0"/>
Line 2339: </xs:sequence>
Line 2340: </xs:extension>
Line 2334: <xs:complexType name="Application">
Line 2335: <xs:complexContent>
Line 2336: <xs:extension base="BaseDevice">
Line 2337: <xs:sequence>
Line 2338: <xs:element name="appName" type="xs:string" minOccurs="0"/>
"name" from BaseResource should be used
Line 2339: </xs:sequence>
Line 2340: </xs:extension>
Line 2341: </xs:complexContent>
Line 2342: </xs:complexType>
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmApplicationResource.java
Line 8: import org.ovirt.engine.api.resource.DeviceResource;
Line 9:
Line 10: public class BackendVmApplicationResource extends
BackendReadOnlyDeviceResource<Application, Applications, VM> implements
DeviceResource<Application>{
Line 11:
Line 12: public static final String CURRENT_CONSTRAINT_PARAMETER =
"current";
not relevant in this context
Line 13:
Line 14: public BackendVmApplicationResource(Class<Application> modelType,
Line 15: Class<VM> entityType,
Line 16: final Guid guid,
Line 21:
Line 22: @Override
Line 23: protected Application map(VM entity) {
Line 24: try {
Line 25: int index = getIndex(id);
id should be UUID (see BackendDomainResource for more details)
Line 26: String[] apps = entity.getAppList() == null ? new
String[0] : entity.getAppList().split(",");
Line 27: if (index > apps.length) {
Line 28: return notFound();
Line 29: }
Line 28: return notFound();
Line 29: }
Line 30: VM entityVM = new VM();
Line 31: entityVM.setAppList(apps[index]);
Line 32: Application app = map(entityVM, null);
why not just sending apps[indx] to mapper?
Line 33: app.setId(id);
Line 34: return app;
Line 35: } catch(Exception e) {
Line 36: return notFound();
Line 43: return Integer.parseInt(indexStr);
Line 44: }
Line 45:
Line 46: @Override
Line 47: public Application update(Application resource) {
not relevant in this resource, 'applications' are not updatable
Line 48: return resource;
Line 49: }
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendVmApplicationsResource.java
Line 26: queryParams);
Line 27: }
Line 28:
Line 29: @Override
Line 30: protected Applications mapCollection(List<VM> entities, boolean
addLinks) {
i'm not follow, you working on single vm in this context, why List<VM> ??
Line 31: Applications collection = instantiate(collectionType);
Line 32: List<Application> list = getList(collection);
Line 33: int id = 0;
Line 34: for (VM entity : entities) {
....................................................
File
backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/ApplicationMapper.java
Line 8: public class ApplicationMapper {
Line 9:
Line 10: public static final Guid APP_ID = Guid.Empty;
Line 11:
Line 12: @Mapping(from = Application.class, to = VM.class)
this direction is not needed as apps are read only (server->client)
Line 13: public static VM map(Application model, VM template) {
Line 14: VM entity = template != null ? template : new VM();
Line 15: if (!StringHelper.isNullOrEmpty(model.getAppName())) {
Line 16: entity.getDynamicData().setAppList(model.getAppName());
Line 20:
Line 21: @Mapping(from = VM.class, to = Application.class)
Line 22: public static Application map(VM entity, Application template) {
Line 23: Application model = template != null ? template : new
Application();
Line 24: model.setId(APP_ID.toString());
you should be producing UUID here from the app.name (see DomainMapper)
Line 25: if
(!StringHelper.isNullOrEmpty(entity.getDynamicData().getAppList())) {
Line 26: model.setAppName(entity.getDynamicData().getAppList());
Line 27: }
Line 28: return model;
Line 22: public static Application map(VM entity, Application template) {
Line 23: Application model = template != null ? template : new
Application();
Line 24: model.setId(APP_ID.toString());
Line 25: if
(!StringHelper.isNullOrEmpty(entity.getDynamicData().getAppList())) {
Line 26: model.setAppName(entity.getDynamicData().getAppList());
this is a list of apps AFAIU, not as single app ....
Line 27: }
Line 28: return model;
Line 29: }
Line 30:
--
To view, visit http://gerrit.ovirt.org/13493
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b7db49d2cc7b47c39ef3d4fff261a99c24dae42
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ravi Nori <rn...@redhat.com>
Gerrit-Reviewer: Michael Pasternak <mpast...@redhat.com>
Gerrit-Reviewer: Ravi Nori <rn...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
