Michael Pasternak has posted comments on this change.
Change subject: restapi: add storage server connections resource
......................................................................
Patch Set 28: I would prefer that you didn't submit this
(10 inline comments)
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/StorageServerConnectionQueryParametersBase.java
Line 20: }
Line 21:
Line 22: /**
Line 23: * Used by REST because AbstractBackendResource has id member
Line 24: * that is always assumed to be Guid
in general you could send id.toString() from rest ..., backend should be
agnostic to client constraints in this meaning
Line 25: *
Line 26: * @param serverConnectionId
Line 27: */
Line 28: public StorageServerConnectionQueryParametersBase(Guid
serverConnectionId) {
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendStorageServerConnectionResource.java
Line 29: }
Line 30:
Line 31: @Override
Line 32: public Storage update(Storage connection) {
Line 33: validateParameters(connection, "host.id|name");
same comment as in other file on this, i'd prefer to get rid of it asap (before
3.3 goes out), actually would be nice to drop it when this feature goes out,
before people start using it/writing tests ..
Line 34: validateEnums(Storage.class, connection);
Line 35: return performUpdate(connection,
Line 36: new
QueryIdResolver<String>(VdcQueryType.GetStorageServerConnectionById,
Line 37:
StorageServerConnectionQueryParametersBase.class),
Line 57: return new
StorageServerConnectionParametersBase(connection, hostId);
Line 58: }
Line 59:
Line 60: private Guid getHostId(Host host) {
Line 61: // presence of host ID or name already validated
comment in p/s 25
Line 62: return host.isSetId()
Line 63: ? new Guid(host.getId())
Line 64: : host.isSetName()
Line 65: ? getEntity(VDS.class,
....................................................
File
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/BackendStorageServerConnectionsResource.java
Line 45: StorageConnections collection = new StorageConnections();
Line 46: for
(org.ovirt.engine.core.common.businessentities.StorageServerConnections entity
: entities) {
Line 47: Storage connection = map(entity);
Line 48: if (connection != null) {
Line 49:
collection.getStorageConnections().add(addLinks(connection));
comment in p/s 25
Line 50: }
Line 51: }
Line 52: return collection;
Line 53: }
Line 53: }
Line 54:
Line 55: @Override
Line 56: public Response add(Storage storage) {
Line 57: validateParameters(storage, "address", "type", "host.id|name");
it has to go out in 3.3 (any chance it won't?), i wouldn't want exposing
redundant constraint
Line 58: // map to backend object
Line 59: StorageServerConnections storageConnection =
Line 60: getMapper(Storage.class,
StorageServerConnections.class).map(storage, null);
Line 61: Guid hostId = getHostId(storage.getHost());
Line 78: getAddParams(storageConnection, hostId),
Line 79: ENTITY_RETRIEVER);
Line 80: }
Line 81:
Line 82: private Guid getHostId(Host host) {
it is used in various places and since all storage related code does not use
common parent, i'd suggest moving this method to utility class
Line 83: // presence of host ID or name already validated
Line 84: return host.isSetId()
Line 85: ? new Guid(host.getId())
Line 86: : host.isSetName()
Line 96: return params;
Line 97: }
Line 98:
Line 99: @Override
Line 100: public Response remove(@PathParam("id") String id, Host host) {
please remove annotation
Line 101: validateParameters(host, "id|name");
Line 102: this.host = host;
Line 103: return super.remove(id);
Line 104: }
....................................................
File
backend/manager/modules/restapi/types/src/main/java/org/ovirt/engine/api/restapi/types/StorageDomainMapper.java
Line 148: return model;
Line 149: }
Line 150:
Line 151: @Mapping(from = Storage.class, to =
StorageServerConnections.class)
Line 152: public static StorageServerConnections map(Storage model,
StorageServerConnections template) {
please add test for this mapping in the corresponding mapper test
thanks.
Line 153: StorageServerConnections entity = template != null ? template
: new StorageServerConnections();
Line 154: org.ovirt.engine.core.common.businessentities.StorageType
storageType = null;
Line 155: if (model.getType() != null) {
Line 156: storageType =
map(StorageType.fromValue(model.getType()),null);
Line 224: }
Line 225:
Line 226: @Mapping(from =
org.ovirt.engine.core.common.businessentities.StorageServerConnections.class,
Line 227: to = org.ovirt.engine.api.model.Storage.class)
Line 228: public static Storage map(StorageServerConnections entity,
Storage template) {
same
Line 229: Storage model = template != null ? template : new Storage();
Line 230: model.setId(entity.getid());
Line 231: model.setType(map(entity.getstorage_type(), null));
Line 232: if (entity.getstorage_type() ==
org.ovirt.engine.core.common.businessentities.StorageType.ISCSI) {
....................................................
Commit Message
Line 20: 6. Add ability to get a specific connection by its id.
Line 21: Url is the same as item 4.
Line 22: 7. REST unitests
Line 23: Feature page:
Line 24: www.ovirt.org/Features/Manage_Storage_Connections
1. please describe new URIs in rsdl_meta.yaml
2. please describe this feature in the FeatureHelper
thanks
Line 25:
Line 26: Change-Id: If6bc32ead098390723825872f6fb292097d52835
--
To view, visit http://gerrit.ovirt.org/16617
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: If6bc32ead098390723825872f6fb292097d52835
Gerrit-PatchSet: 28
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Alissa Bonas <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Michael Pasternak <[email protected]>
Gerrit-Reviewer: Ori Liel <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
