Ravi Nori has posted comments on this change. Change subject: core : Introduce engine_sessions table and DAO ......................................................................
Patch Set 5: (4 comments) http://gerrit.ovirt.org/#/c/35148/5/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/EngineSession.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/EngineSession.java: Line 11: Line 12: public class EngineSession implements Serializable { Line 13: private static final long serialVersionUID = 6964615561527013329L; Line 14: Line 15: private long id; > Don't you want to move the changes you did in the next patch to this one ? Alon doesnt like that change, will keep these but pass DbUser in Constructor Line 16: Line 17: private String engineSessionId; Line 18: Line 19: private Guid userId; Line 58: public Collection<Guid> getRoleIds() { Line 59: if (roleIds == null) { Line 60: roleIds = Collections.emptyList(); Line 61: } Line 62: return new ArrayList<Guid>(roleIds); > As written before - not that critical but consider set (just to ensure nobo Done Line 63: } Line 64: Line 65: public void setRoleIds(Collection<Guid> roleIds) { Line 66: this.roleIds = roleIds; http://gerrit.ovirt.org/#/c/35148/5/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/EngineSessionDAOTest.java File backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/EngineSessionDAOTest.java: Line 33: // create some test data Line 34: newEngineSession = new EngineSession(); Line 35: newEngineSession.setEngineSessionId(Guid.newGuid().toString()); Line 36: newEngineSession.setUserId(Guid.newGuid()); Line 37: newEngineSession.setRoleIds(new LinkedList<Guid>(Arrays.asList(FixturesTool.EXISTING_GROUP_ID))); > not that it's that critical - but why not HashSet? Done Line 38: newEngineSession.setGroupIds(new LinkedList<Guid>(Arrays.asList(FixturesTool.EXISTING_GROUP_ID))); Line 39: newEngineSession.setUserName(""); Line 40: Line 41: existingEngineSession = dao.get(ID); http://gerrit.ovirt.org/#/c/35148/5/packaging/dbscripts/engine_sessions_sp.sql File packaging/dbscripts/engine_sessions_sp.sql: Line 49: Line 50: END; $procedure$ Line 51: LANGUAGE plpgsql; Line 52: Line 53: Create or replace FUNCTION DeleteEngineSessionBySessionId(v_engine_session_id text) > and we do not need this one, right? Removed Line 54: RETURNS integer Line 55: AS $procedure$ Line 56: DECLARE Line 57: deleted_rows int; -- To view, visit http://gerrit.ovirt.org/35148 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4b4d9cfc3edc6084fc0436ecfd09c82d5ae57f5e Gerrit-PatchSet: 5 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ravi Nori <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: Ravi Nori <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: [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
