Alexander Wels has posted comments on this change.

Change subject: Introduction of filters to unify AAA flows for UI and REST-API
......................................................................


Patch Set 38:

(2 comments)

http://gerrit.ovirt.org/#/c/28022/38/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 482: 
Line 483:     private static String addSessionToContext(VdcQueryParametersBase 
parameters) {
Line 484:         String sessionId = parameters.getSessionId();
Line 485:         if (StringUtils.isEmpty(sessionId)) {
Line 486:             sessionId = ThreadLocalParamsContainer.getSessionId();
> I would like to believe we always have parameters... and can get rid of the
I was more worried about getting a null pointer exception when we call 
setSessionId(sessionId) where sessionId is null.
Line 487:         }
Line 488:         ThreadLocalParamsContainer.setSessionId(sessionId);
Line 489:         return sessionId;
Line 490:     }


http://gerrit.ovirt.org/#/c/28022/38/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/GenericApiGWTServiceImpl.java
File 
frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/server/gwt/GenericApiGWTServiceImpl.java:

Line 188:         params.setActionType(loginType);
Line 189:         VdcLoginReturnValueBase returnValue = 
(VdcLoginReturnValueBase) getBackend().login(params);
Line 190:         if (returnValue.getSucceeded()) {
Line 191:             
this.getThreadLocalResponse().addHeader("OVIRT-SSO-TOKEN", 
getSession().getId()); //$NON-NLS-1$
Line 192:             getSession().setAttribute("ovirt_aaa_engineSessionId", 
returnValue.getSessionId()); //$NON-NLS-1$)
> can you please suggest where to put it? if there is already proper place...
Actually looks like SessionUtils already defines the constant, why not simply 
use that one?
Line 193:         }
Line 194:         return returnValue;
Line 195:     }
Line 196: 


-- 
To view, visit http://gerrit.ovirt.org/28022
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia5536d123b6407acf41b6946dde796bd67d1e073
Gerrit-PatchSet: 38
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: Alexander Wels <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Barak Azulay <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[email protected]>
Gerrit-Reviewer: Vojtech Szocs <[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

Reply via email to