Alon Bar-Lev has posted comments on this change. Change subject: aaa: Added usage of AuthRecord.VALID_TO ......................................................................
Patch Set 18: (7 comments) http://gerrit.ovirt.org/#/c/26975/18/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 116: return failCanDoAction(VdcBllMessages.USER_CANNOT_LOGIN_SESSION_MISSING); Line 117: } Line 118: Line 119: SessionDataContainer.getInstance().setSoftLimit(DateUtils.addMinutes(new Date(), Line 120: Config.<Integer> getValue(ConfigValues.UserSessionTimeOutInterval))); why don't you call refresh? Line 121: Line 122: int maxSessionInterval = Config.<Integer> getValue(ConfigValues.MaxSessionTimeoutInterval); Line 123: Date validTo = maxSessionInterval != 0 ? DateUtils.addMinutes(new Date(), maxSessionInterval) : null; Line 124: try { Line 118: Line 119: SessionDataContainer.getInstance().setSoftLimit(DateUtils.addMinutes(new Date(), Line 120: Config.<Integer> getValue(ConfigValues.UserSessionTimeOutInterval))); Line 121: Line 122: int maxSessionInterval = Config.<Integer> getValue(ConfigValues.MaxSessionTimeoutInterval); this is not an interval... 3rd time... it is a max session DURATION. Line 123: Date validTo = maxSessionInterval != 0 ? DateUtils.addMinutes(new Date(), maxSessionInterval) : null; Line 124: try { Line 125: Date fromExtension = Line 126: new SimpleDateFormat("yyyy-MM-dd HH:mm:ssZ").parse(authRecord.<String> get(AuthRecord.VALID_TO)); Line 125: Date fromExtension = Line 126: new SimpleDateFormat("yyyy-MM-dd HH:mm:ssZ").parse(authRecord.<String> get(AuthRecord.VALID_TO)); Line 127: if (validTo != null) { Line 128: validTo = validTo.compareTo(fromExtension) < 0 ? validTo : fromExtension; Line 129: } else validTo = fromExtension Line 130: } catch (ParseException e) { Line 131: log.warn("Error parsing AuthRecord.VALID_TO . Default VALID_TO value will be set on session"); Line 132: } Line 133: SessionDataContainer.getInstance().setHardLimit(validTo); http://gerrit.ovirt.org/#/c/26975/18/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 143: if (iterator.next().getKey().equals(sessionId)) { Line 144: iterator.remove(); Line 145: break; Line 146: } Line 147: } can you please explain why isn't this plain: sessionInfoList.remove(sessionId) ? Line 148: } Line 149: Line 150: /** Line 151: * Will run the process of cleaning expired sessions. Line 272: } Line 273: Line 274: private void refresh(SessionInfo sessionInfo) { Line 275: Date now = new Date(); Line 276: synchronized (sessionInfo) { why do you syncrhonize? Line 277: Date softLimit = (Date) sessionInfo.contentOfSession.get(SOFT_LIMIT_PARAMETER_NAME); Line 278: if (softLimit == null) { Line 279: softLimit = now; Line 280: } Line 280: } Line 281: softLimit = Line 282: DateUtils.addMinutes(softLimit, Line 283: Config.<Integer> getValue(ConfigValues.UserSessionTimeOutInterval)); Line 284: sessionInfo.contentOfSession.put(SOFT_LIMIT_PARAMETER_NAME, softLimit); I do not understand.... should be: userSessionTimeoutInterval = Config.<Integer> getValue(ConfigValues.UserSessionTimeOutInterval) if (userSessionTimeoutInterval > 0) { Date next = new Date(); DateUtils.addMinutes(next, userSessionTimeoutInterval); sessionInfo.contentOfSession.put(SOFT_LIMIT_PARAMETER_NAME, next); } now, I also recommend of storing long (getTime()) instead of Date object... but this is your call. Line 285: } Line 286: } Line 287: http://gerrit.ovirt.org/#/c/26975/18/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java: Line 1700: BootMenuSupported, Line 1701: Line 1702: @TypeConverterAttribute(Integer.class) Line 1703: @DefaultValueAttribute("0") Line 1704: MaxSessionTimeoutInterval, Duration or anything else but interval! Line 1705: Line 1706: Invalid -- 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: 18 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
