Alexander Wels has posted comments on this change. Change subject: webadmin: Store rest api session id in http session ......................................................................
Patch Set 3: (10 comments) http://gerrit.ovirt.org/#/c/25987/3/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/Frontend.java File frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/Frontend.java: Line 877: getOperationManager().setLoggedIn(false); Line 878: } Line 879: } Line 880: Line 881: public void storeValue(final String key, final String value) { > Please consider using consistent names across different layers/components, Done Line 882: getOperationManager().store(key, value); Line 883: } Line 884: Line 885: public void retrieveValue(final String key, final StorageCallback callback) { Line 881: public void storeValue(final String key, final String value) { Line 882: getOperationManager().store(key, value); Line 883: } Line 884: Line 885: public void retrieveValue(final String key, final StorageCallback callback) { > For consistency, this could be named "retrieveFromHttpSession". Done Line 886: getOperationManager().retrieve(key, callback); Line 887: } Line 888: Line 889: // TODO: Externalize to a better location, should support translation via http://gerrit.ovirt.org/#/c/25987/3/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/CommunicationProvider.java File frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/CommunicationProvider.java: Line 40: */ Line 41: void storeInHttpSession(String key, String value, StorageCallback callback); Line 42: Line 43: /** Line 44: * Retrieve the value associated with the key. > ... from {@code HttpSession} on the server side. Done Line 45: * @param key The key Line 46: * @param the callback to call with the result. Line 47: */ Line 48: void retrieveFromHttpSession(String key, StorageCallback callback); http://gerrit.ovirt.org/#/c/25987/3/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/OperationProcessor.java File frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/OperationProcessor.java: Line 271: * Default store where the caller doesn't care about the result. Line 272: * @param key The key Line 273: * @param value The value to store. Line 274: */ Line 275: public void store(final String key, final String value) { > For consistency, this could be named "storeInHttpSession". Done Line 276: store(key, value, new StorageCallback() { Line 277: Line 278: @Override Line 279: public void onSuccess(String result) { Line 287: Line 288: }); Line 289: } Line 290: Line 291: public void store(final String key, final String value, final StorageCallback callback) { > For consistency, this could be named "storeInHttpSession". Done Line 292: communicationProvider.storeInHttpSession(key, value, callback); Line 293: } Line 294: Line 295: public void retrieve(final String key, final StorageCallback callback) { Line 291: public void store(final String key, final String value, final StorageCallback callback) { Line 292: communicationProvider.storeInHttpSession(key, value, callback); Line 293: } Line 294: Line 295: public void retrieve(final String key, final StorageCallback callback) { > For consistency, this could be named "retrieveFromHttpSession". Done Line 296: communicationProvider.retrieveFromHttpSession(key, callback); Line 297: } http://gerrit.ovirt.org/#/c/25987/3/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/VdcOperationManager.java File frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/VdcOperationManager.java: Line 156: * Store a value on the back-end in the session. Line 157: * @param key The key. Line 158: * @param value The value. Line 159: */ Line 160: public void store(final String key, final String value) { > For consistency, this could be named "storeInHttpSession". Done Line 161: processor.store(key, value); Line 162: } Line 163: Line 164: /** Line 165: * Retrieve a stored value from the back-end session. Line 166: * @param key The key. Line 167: * @param callback The callback to call with the value. Line 168: */ Line 169: public void retrieve(final String key, final StorageCallback callback) { > For consistency, this could be named "retrieveFromHttpSession". Done Line 170: processor.retrieve(key, callback); Line 171: } http://gerrit.ovirt.org/#/c/25987/3/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/gwtservices/GenericApiGWTService.java File frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/gwtservices/GenericApiGWTService.java: Line 46: public VdcReturnValueBase Login(String userName, String password, String profileName, VdcActionType loginType); Line 47: Line 48: public void storeInHttpSession(String key, String value); Line 49: Line 50: public String getFromHttpSession(String key); > For consistency, this could be named "retrieveFromHttpSession". Done http://gerrit.ovirt.org/#/c/25987/3/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/plugin/restapi/RestApiSessionManager.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/plugin/restapi/RestApiSessionManager.java: Line 167: //be null. If it is null then reuseSession was called from an automatic login (as restApiSessionId is null Line 168: //can we can utilize the async call to retrieve it from the backend. Line 169: if (getSessionId() != null) { Line 170: RestApiSessionAcquiredEvent.fire(eventBus, getSessionId()); Line 171: scheduleKeepAliveHeartbeat(); > Hm, to avoid duplicating code, I suggest to create small methods: Done Line 172: } else { Line 173: getSessionId(new StorageCallback() { Line 174: Line 175: @Override -- To view, visit http://gerrit.ovirt.org/25987 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6fe9af54054aefe694876b1805a4a44f9bba0482 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Einav Cohen <eco...@redhat.com> Gerrit-Reviewer: Greg Sheremeta <gsher...@redhat.com> Gerrit-Reviewer: Vojtech Szocs <vsz...@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
