Yair Zaslavsky has posted comments on this change. Change subject: aaa: Added usage of AuthRecord.VALID_TO ......................................................................
Patch Set 19: (8 comments) http://gerrit.ovirt.org/#/c/26975/19/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 121: int userSessionHardLimit = Config.<Integer> getValue(ConfigValues.UserSessionHardLimit); Line 122: Date validTo = userSessionHardLimit != 0 ? DateUtils.addMinutes(new Date(), userSessionHardLimit) : null; Line 123: try { Line 124: Date fromExtension = Line 125: new SimpleDateFormat("yyyy-MM-dd HH:mm:ssZ").parse(authRecord.<String> get(AuthRecord.VALID_TO)); > what if extension does not return VALID_TO? I guess you have null pointer e done. I'll catch Exception instead of ParseException. Line 126: if (validTo != null) { Line 127: validTo = validTo.compareTo(fromExtension) < 0 ? validTo : fromExtension; Line 128: } else { Line 129: validTo = fromExtension; Line 128: } else { Line 129: validTo = fromExtension; Line 130: } Line 131: } catch (ParseException e) { Line 132: log.warn("Error parsing AuthRecord.VALID_TO . Default VALID_TO value will be set on session"); > again, debug exception? Done Line 133: } Line 134: SessionDataContainer.getInstance().setHardLimit(validTo); Line 135: } Line 136: return true; http://gerrit.ovirt.org/#/c/26975/19/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 ConcurrentMap<String, SessionInfo> sessionInfoMap = new ConcurrentHashMap<>(); the map is a map from setting id to SessionInfo. 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: sessionInfo = currentSessionInfo; Line 116: break; Line 117: } Line 118: } Line 119: return sessionInfo; > why the key cannot be session id? Done Line 120: } Line 121: Line 122: /** Line 123: * Remove the cached data of session The sessionId is retrieved from Line 141: while (iterator.hasNext()) { Line 142: if (iterator.next().getKey().equals(sessionId)) { Line 143: iterator.remove(); Line 144: break; Line 145: } > I still don't understand why the key cannot be the session id Done Line 146: } Line 147: } Line 148: Line 149: /** Line 271: } Line 272: Line 273: private void refresh(SessionInfo sessionInfo) { Line 274: Date now = new Date(); Line 275: synchronized (sessionInfo) { > why synchornize? worse case it will be updated twice Done Line 276: Date softLimit = (Date) sessionInfo.contentOfSession.get(SOFT_LIMIT_PARAMETER_NAME); Line 277: if (softLimit == null) { Line 278: softLimit = now; Line 279: } Line 275: synchronized (sessionInfo) { Line 276: Date softLimit = (Date) sessionInfo.contentOfSession.get(SOFT_LIMIT_PARAMETER_NAME); Line 277: if (softLimit == null) { Line 278: softLimit = now; Line 279: } > why do you still get the old value? Done Line 280: softLimit = Line 281: DateUtils.addMinutes(softLimit, Line 282: Config.<Integer> getValue(ConfigValues.UserSessionTimeOutInterval)); Line 283: sessionInfo.contentOfSession.put(SOFT_LIMIT_PARAMETER_NAME, softLimit); Line 279: } Line 280: softLimit = Line 281: DateUtils.addMinutes(softLimit, Line 282: Config.<Integer> getValue(ConfigValues.UserSessionTimeOutInterval)); Line 283: sessionInfo.contentOfSession.put(SOFT_LIMIT_PARAMETER_NAME, softLimit); > if UserSessionTimeOutInterval is 0 you should not put anything in session Done Line 284: } Line 285: } Line 286: -- 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: 19 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
