Michael Pasternak has posted comments on this change.
Change subject: engine: watchdog - restapi support
......................................................................
Patch Set 36: I would prefer that you didn't submit this
(13 inline comments)
....................................................
File
backend/manager/modules/restapi/interface/common/jaxrs/src/main/java/org/ovirt/engine/api/common/util/LinkHelper.java
Line 319: TYPES.put(VersionCaps.class, map);
Line 320:
Line 321: map = new ParentToCollectionMap(WatchDog.class,
DevicesResource.class, VM.class);
Line 322: map.add(WatchDog.class, ReadOnlyDeviceResource.class,
Template.class);
Line 323: TYPES.put(WatchDog.class, map);
i'd suggest defining it as:
map = new ParentToCollectionMap(DeviceResource.class,
DevicesResource.class);
map.add(DeviceResource.class, DevicesResource.class, Template.class);
map.add(DeviceResource.class, DevicesResource.class, VM.class);
TYPES.put(WatchDog.class, map);
you have good example for this in TYPES.put(NIC.class, map);
Line 324: }
Line 325:
Line 326: /**
Line 327: * Obtain the relative path to a top-level collection
....................................................
File
backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/WatchdogResource.java
Line 2:
Line 3: import org.ovirt.engine.api.model.WatchDog;
Line 4: import org.ovirt.engine.api.model.WatchDogs;
Line 5:
Line 6: public interface WatchdogResource extends DevicesResource<WatchDog,
WatchDogs>{
resource should not be extending a collection interface,
should be "extends DeviceResource<WatchDog>"
Line 7:
Line 8:
....................................................
File
backend/manager/modules/restapi/interface/definition/src/main/java/org/ovirt/engine/api/resource/WatchdogsResource.java
Line 11: import org.jboss.resteasy.annotations.providers.jaxb.Formatted;
Line 12: import org.ovirt.engine.api.model.WatchDog;
Line 13: import org.ovirt.engine.api.model.WatchDogs;
Line 14:
Line 15: @Path("/watchdogs")
no need for path annotation here, it's defined at TemplateResource/VmResource
getResource() methods
Line 16: @Produces({ApiMediaType.APPLICATION_XML,
ApiMediaType.APPLICATION_JSON, ApiMediaType.APPLICATION_X_YAML})
Line 17: public interface WatchdogsResource extends DevicesResource<WatchDog,
WatchDogs>{
Line 18: @POST
Line 19: @Formatted
....................................................
File
backend/manager/modules/restapi/interface/definition/src/main/resources/rsdl_metadata.yaml
Line 352: request:
Line 353: body:
Line 354: parameterType: WatchDog
Line 355: signatures:
Line 356: - mandatoryArguments: {watchdog.action: 'xs:string',
watchdog.model: 'xs:string'}
in the implementation i see that
BackendWatchdogsResource.getRequiredUpdateFields() returns empty array
Line 357: optionalArguments: {}
Line 358: urlparams:
Line 359: headers:
Line 360: Content-Type: {value: application/xml|json, required: true}
....................................................
File
backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/AbstractBackendBaseTest.java
Line 20: import javax.ws.rs.core.UriInfo;
Line 21:
Line 22: import org.easymock.IAnswer;
Line 23: import org.easymock.IMocksControl;
Line 24: import org.easymock.EasyMock;
why do you need this change? you understand that it effects all the tests,
right?
Line 25: import org.jboss.resteasy.specimpl.MultivaluedMapImpl;
Line 26: import org.junit.After;
Line 27: import org.junit.Assert;
Line 28: import org.junit.Before;
....................................................
File
backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/AbstractBackendWatchdogsResourceTest.java
Line 46:
Line 47: @Override
Line 48: /**
Line 49: * This method needed to be overridden, because the superclass is
creating 3 returned VmWatchdog
Line 50: * There can only be 1 or 0.
i'm not sure i understand this, the amount of entities should be as
NAMES.length,
and this is true for all tests, i'd suggest staying with this pattern
Line 51: */
Line 52: protected void setUpQueryExpectations(String query, Object
failure) throws Exception {
Line 53: VdcQueryReturnValue queryResult =
control.createMock(VdcQueryReturnValue.class);
Line 54: SearchParameters params = new SearchParameters(prefix + query,
searchType);
Line 75:
Line 76: @Override
Line 77: /**
Line 78: * This method needed to be overridden because the super
implementation assumes that there can be multiple
Line 79: * watchdogs, while there can be only 0 or 1.
same here
Line 80: */
Line 81: protected void verifyCollection(List<WatchDog> collection) throws
Exception {
Line 82: assertNotNull(collection);
Line 83: assertEquals(1, collection.size());
....................................................
File
backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendWatchdogResourceTest.java
Line 104: new Object[] { PARENT_ID },
Line 105: getEntity(0));
Line 106: }
Line 107: }
Line 108:
i'd also expect testUpdateNotFound() test
Line 109: @Test
Line 110: public void testUpdate() throws Exception {
Line 111: setUpEntityQueryExpectations(2);
Line 112:
setUriInfo(setUpActionExpectations(VdcActionType.UpdateWatchdog,
....................................................
File
backend/manager/modules/restapi/jaxrs/src/test/java/org/ovirt/engine/api/restapi/resource/BackendWatchdogsResourceTest.java
Line 7:
Line 8: public BackendWatchdogsResourceTest() {
Line 9: super(new BackendWatchdogsResource(PARENT_ID,
VdcQueryType.GetWatchdog, new IdQueryParameters(PARENT_ID)));
Line 10: }
Line 11:
please add tests as in the BackendCdRomsResourceTest
thanks
....................................................
File
backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/WatchdogMapper.java
Line 9: public class WatchdogMapper {
Line 10: @Mapping(from = WatchDog.class, to = VmWatchdog.class)
Line 11: public static VmWatchdog map(WatchDog model, VmWatchdog template) {
Line 12: VmWatchdog entity = template == null ? new VmWatchdog() :
template;
Line 13: entity.setAction(
VmWatchdogAction.getByName(model.getAction()));
you should be mapping from /api/restapi/types/WatchdogAction and not using
backend enum
Line 14: entity.setId(new Guid(model.getId()));
Line 15: entity.setModel( VmWatchdogType.getByName(model.getModel()));
Line 16: return entity;
Line 17: }
Line 11: public static VmWatchdog map(WatchDog model, VmWatchdog template) {
Line 12: VmWatchdog entity = template == null ? new VmWatchdog() :
template;
Line 13: entity.setAction(
VmWatchdogAction.getByName(model.getAction()));
Line 14: entity.setId(new Guid(model.getId()));
Line 15: entity.setModel( VmWatchdogType.getByName(model.getModel()));
you should be mapping from /api/restapi/types/WatchdogModel and not using
backend enum
Line 16: return entity;
Line 17: }
Line 18: @Mapping(from = VmWatchdog.class, to = WatchDog.class)
Line 19: public static WatchDog map(VmWatchdog entity, WatchDog template) {
Line 17: }
Line 18: @Mapping(from = VmWatchdog.class, to = WatchDog.class)
Line 19: public static WatchDog map(VmWatchdog entity, WatchDog template) {
Line 20: WatchDog model = template == null ? new WatchDog() : template;
Line 21: model.setAction(entity.getAction() == null ? null :
entity.getAction().name().toLowerCase());
you should be mapping to /api/restapi/types/WatchdogAction and not using
backend enum
Line 22: model.setId(entity.getId().toString());
Line 23: model.setModel(entity.getModel() == null ? null :
entity.getModel().name());
Line 24:
Line 25: return model;
Line 19: public static WatchDog map(VmWatchdog entity, WatchDog template) {
Line 20: WatchDog model = template == null ? new WatchDog() : template;
Line 21: model.setAction(entity.getAction() == null ? null :
entity.getAction().name().toLowerCase());
Line 22: model.setId(entity.getId().toString());
Line 23: model.setModel(entity.getModel() == null ? null :
entity.getModel().name());
you should be mapping to /api/restapi/types/WatchdogModel and not using backend
enum
Line 24:
Line 25: return model;
Line 26: }
--
To view, visit http://gerrit.ovirt.org/13060
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ief5b20ecf2221faf900cadfeafe4c71607a9eca3
Gerrit-PatchSet: 36
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Laszlo Hornyak <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Laszlo Hornyak <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
