Alon Bar-Lev has posted comments on this change. Change subject: aaa: Added usage of AuthRecord.VALID_TO ......................................................................
Patch Set 14: (7 comments) http://gerrit.ovirt.org/#/c/26975/14/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/Backend.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/Backend.java: Line 217 Line 218 Line 219 Line 220 Line 221 once again... I do not understand why you need to get this interval at Backend now. http://gerrit.ovirt.org/#/c/26975/14/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/LoginBaseCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/LoginBaseCommand.java: Line 120: // backward compatibility with session expiration when session was expired after moving Line 121: // from new generation to old generation and then got expired - this happened after Line 122: // 2 * UserSessionTimeoutInterval minutes Line 123: SessionDataContainer.getInstance().setSoftLimit(DateUtils.addMinutes(new Date(), Line 124: 2 * Config.<Integer> getValue(ConfigValues.UserSessionTimeOutInterval))); if it is 0 then you need to set soft limit to infinite Line 125: Line 126: int maxSessionInterval = Config.<Integer> getValue(ConfigValues.MaxSessionTimeoutInterval); Line 127: Date validTo = Line 128: DateUtils.addMinutes(new Date(), maxSessionInterval == 0 ? Integer.MAX_VALUE : maxSessionInterval); Line 122: // 2 * UserSessionTimeoutInterval minutes Line 123: SessionDataContainer.getInstance().setSoftLimit(DateUtils.addMinutes(new Date(), Line 124: 2 * Config.<Integer> getValue(ConfigValues.UserSessionTimeOutInterval))); Line 125: Line 126: int maxSessionInterval = Config.<Integer> getValue(ConfigValues.MaxSessionTimeoutInterval); MaxSessionTime - has nothing to do with interval :) Line 127: Date validTo = Line 128: DateUtils.addMinutes(new Date(), maxSessionInterval == 0 ? Integer.MAX_VALUE : maxSessionInterval); Line 129: try { Line 130: Date fromExtension = Line 130: Date fromExtension = Line 131: new SimpleDateFormat("yyyy-MM-dd HH:mm:ssZ").parse(authRecord.<String> get(AuthRecord.VALID_TO)); Line 132: validTo = validTo.compareTo(fromExtension) < 0 ? validTo : fromExtension; Line 133: } catch (ParseException e) { Line 134: log.warn("Error parsing AuthRecord.VALID_TO . Default VALID_TO value will be set on session"); debug exception? Line 135: } Line 136: SessionDataContainer.getInstance().setHardLimit(validTo); Line 137: } Line 138: return true; http://gerrit.ovirt.org/#/c/26975/14/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/session/SessionDataContainer.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/session/SessionDataContainer.java: Line 25: this.sessionId = sessionId; Line 26: } Line 27: } Line 28: Line 29: private List<SessionInfo> sessionInfoList = new ArrayList<>(); why do you need ArrayList while SessionId is unique and you can iterate over the concurent map? Line 30: Line 31: private static final String USER_PARAMETER_NAME = "user"; Line 32: private static final String PASSWORD_PARAMETER_NAME = "password"; Line 33: private static final String HARD_LIMIT_PARAMETER_NAME = "hard_limit"; Line 115: Line 116: } Line 117: value = sessionInfo.sessionMap.get(key); Line 118: } Line 119: return value; this I do not understand at all... please explain in words... should have been something like: if (refresh) { if (softLimit != null) { softLimit = now + interval; } } why is the above so complex? why do we need LAST_REFRESH? BTW: I think we can remove the *2 as we are only keeping session alive not disconnecting. Line 120: } Line 121: Line 122: public final void setData(String sessionId, String key, Object value) { Line 123: SessionInfo sessionInfo = getSessionInfo(sessionId); Line 190: } else if (softLimit != null) { Line 191: if (softLimit.before(now)) { Line 192: iterator.remove(); Line 193: } Line 194: } this I understand! so why don't you put null instead of using MAX_INT if no limit? Line 195: } Line 196: } Line 197: Line 198: /** -- To view, visit http://gerrit.ovirt.org/26975 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I53e4a371c1bae8d2480ddd2af921a560c6fe9a85 Gerrit-PatchSet: 14 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
