Alexander Wels has posted comments on this change. Change subject: webadmin: Fix UI plugin REST API / Engine session refresh issue ......................................................................
Patch Set 3: (3 comments) I think I didn't communicate what I was thinking properly. My thought was that VdcOperationManager would check when it should fire an 'need a refresh' event. (Basically what the method checks for in RestApiManager now). This way the RestApiManager just receives a 'need a refresh' event and doesn't care where it came from or what the source of the event was. It also doesn't know about VdcOperations or actions or queries, as it is not important. Also the number of events fired should be lower (conditional on a 'refresh' or action). And the event itself will be smaller since it doesn't contain any objects. http://gerrit.ovirt.org/#/c/36622/3/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/VdcOperationExecuted.java File frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/communication/VdcOperationExecuted.java: Line 5: /** Line 6: * Event triggered when {@link VdcOperationManager} executes an {@linkplain VdcOperation operation}. Line 7: */ Line 8: @GenEvent Line 9: public class VdcOperationExecuted { I was thinking more of a 'SessionRefreshEvent' with no contents, just the fact that the event is fired is an indication that we need a refresh. Line 10: Line 11: VdcOperation<?, ?> operation; Line 12: http://gerrit.ovirt.org/#/c/36622/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 79: Line 80: if (isAllowedToExecute) { Line 81: if (operationCanBeAdded && operationQueue.add(operation)) { Line 82: processor.processOperation(this); Line 83: VdcOperationExecutedEvent.fire(eventBus, operation); Instead of always firing this and having the receiver figure out if it should do something, why not determine if we should fire the event here, and have the receiver just do what it needs to do when it receives the event. Line 84: } Line 85: } Line 86: Line 87: return isAllowedToExecute; http://gerrit.ovirt.org/#/c/36622/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 109: refreshRestApiSession = true; Line 110: } Line 111: } Line 112: Line 113: boolean engineSessionRefreshed(VdcOperation<?, ?> operation) { I would move this method into the VdcOperationManager to decide when to fire the event. Line 114: // Actions always refresh the Engine session Line 115: if (operation.isAction()) { Line 116: return true; Line 117: } -- To view, visit http://gerrit.ovirt.org/36622 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2ffd3198239040f67313238a837af98db16e79ea Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Vojtech Szocs <vsz...@redhat.com> Gerrit-Reviewer: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Einav Cohen <eco...@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
