Allon Mureinik has posted comments on this change. Change subject: engine, dao: Introduce ISCSI Bond entity, tables and dao ......................................................................
Patch Set 7: (6 comments) See inline. I'd also like to have a test case for GetIscsiBondsByStoragePoolIdQuery, which is not a trivial DAO call. http://gerrit.ovirt.org/#/c/22951/7/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/IscsiBond.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/IscsiBond.java: Line 28: networkIds = new LinkedList<Guid>(); Line 29: } Line 30: Line 31: public IscsiBond(Guid id, Guid storagePoolId, String name, String description) { Line 32: networkIds = new LinkedList<Guid>(); I'd just call this() instead. Line 33: this.id = id; Line 34: this.storagePoolId = storagePoolId; Line 35: this.name = name; Line 36: this.description = description; Line 57: @NotNull(message = "VALIDATION.ISCSI_BOND.NAME.NOT_NULL", groups = { CreateEntity.class, UpdateEntity.class }) Line 58: @Size(min = 1, max = 50, message = "VALIDATION.ISCSI_BOND.NAME.MAX", Line 59: groups = { CreateEntity.class, UpdateEntity.class }) Line 60: @ValidI18NName(message = "VALIDATION.ISCSI_BOND.NAME.INVALID.CHARACTER", Line 61: groups = { CreateEntity.class, UpdateEntity.class }) shouldn't these be on the data members? Line 62: public String getName() { Line 63: return name; Line 64: } Line 65: Line 78: public void setDescription(String description) { Line 79: this.description = description; Line 80: } Line 81: Line 82: public List<Guid> getNetworkIds() { Does GWT properly handle having a List here and not a concrete type? Line 83: return networkIds; Line 84: } Line 85: Line 86: public void setNetworkIds(List<Guid> networkIds) { http://gerrit.ovirt.org/#/c/22951/7/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/VdcQueryType.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/VdcQueryType.java: Line 224: GetStoragePoolsByClusterService(VdcQueryAuthType.User), Line 225: GetStorageDomainListById, Line 226: GetLunsByVgId, Line 227: GetPermittedStorageDomainsByStoragePoolId(VdcQueryAuthType.User), Line 228: GetIscsiBondsByStoragePoolId, These two are strictly admin queries, right? Line 229: Line 230: // Event Notification Line 231: GetEventSubscribersBySubscriberIdGrouped, Line 232: http://gerrit.ovirt.org/#/c/22951/7/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/IscsiBondDaoDbFacadeImpl.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/IscsiBondDaoDbFacadeImpl.java: Line 29: return getCallsHandler().executeReadList("GetNetworksByIscsiBondId", new RowMapper<Guid>() { Line 30: @Override Line 31: public Guid mapRow(ResultSet rs, int index) throws SQLException { Line 32: return new Guid((UUID) rs.getObject(1)); Line 33: } just use createGuidMapper() Line 34: }, getCustomMapSqlParameterSource().addValue("iscsi_bond_id", iscsiBondId)); Line 35: } Line 36: Line 37: @Override Line 55: return getCallsHandler().executeReadList("GetConnectionsByIscsiBondId", new RowMapper<String>() { Line 56: @Override Line 57: public String mapRow(ResultSet rs, int index) throws SQLException { Line 58: return rs.getString(1); Line 59: } I wonder if we should add a StringMapper next to IntegerMapper and BooleanMapper in the base class. Line 60: }, getCustomMapSqlParameterSource().addValue("iscsi_bond_id", iscsiBondId)); Line 61: } Line 62: Line 63: @Override -- To view, visit http://gerrit.ovirt.org/22951 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I12313c02810a2f0e75016bdd78b44da43f2154d4 Gerrit-PatchSet: 7 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Sergey Gotliv <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Maor Lipchuk <[email protected]> Gerrit-Reviewer: Sergey Gotliv <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
