Alona Kaplan has posted comments on this change. Change subject: webadmin: fluent UiAction ......................................................................
Patch Set 45: (10 comments) https://gerrit.ovirt.org/#/c/36848/45//COMMIT_MSG Commit Message: Line 5: CommitDate: 2015-05-05 10:14:54 +0300 Line 6: Line 7: webadmin: fluent UiAction Line 8: Line 9: Thid patch includes action classes that can assist running vdc action > This patch... Done Line 10: (and may be queries in the future) in a fluent way. Line 11: Line 12: An action can be dependent on the previous action result or can run Line 13: parallely to it. Line 11: Line 12: An action can be dependent on the previous action result or can run Line 13: parallely to it. Line 14: Line 15: The mechanism takes care of staring the model progress when running the > ...starting the model progress... Done Line 16: first action and stopping in after the last action result was return. Line 17: Line 18: It also takes care of collecting the actions failures and displaying them Line 19: in one error popup. https://gerrit.ovirt.org/#/c/36848/45/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/IFrontendEventsHandler.java File frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/IFrontendEventsHandler.java: Line 25: void runQueryFailed(List<VdcQueryReturnValue> returnValue); Line 26: Line 27: void publicConnectionClosed(Exception ex); Line 28: Line 29: public static interface MessageFormatter { > No need for "public" or "static" in nested interface declaration, it can be Done Line 30: public String format(String message); Line 31: } https://gerrit.ovirt.org/#/c/36848/45/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/action/UiAction.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/action/UiAction.java: Line 32: * executed. Line 33: * Line 34: * <code>ActionFlowState</code> is shared between all the action in the flow. Line 35: */ Line 36: public abstract class UiAction { > Very helpful Javadoc! :) Line 37: Line 38: private UiAction nextAction; Line 39: private SimpleAction finalAction; Line 40: private Model model; Line 101: * @param actionFlowState Line 102: * the state of the flow this action wants to join. Line 103: */ Line 104: public void runParallelAction(ActionFlowState actionFlowState) { Line 105: actionFlowState.setBranchCounter(actionFlowState.getBranchCounter() + 1); > Could be a bit simpler, e.g. actionFlowState.incBranchCounter() - just an i Done Line 106: runAction(actionFlowState); Line 107: } Line 108: Line 109: abstract void internalRunAction(); https://gerrit.ovirt.org/#/c/36848/45/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/action/UiVdcAction.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/action/UiVdcAction.java: Line 75: private IFrontendActionAsyncCallback createCallback() { Line 76: return new IFrontendActionAsyncCallback() { Line 77: @Override Line 78: public void executed(FrontendActionAsyncResult result) { Line 79: VdcReturnValueBase returnValueBase = result.getReturnValue(); > I'd suggest to rename this simply to "returnValue" for better readability. Done Line 80: if (returnValueBase == null || !returnValueBase.getSucceeded()) { Line 81: if (!showErrorDialogOnFirstFailure && returnValueBase != null) { Line 82: getActionFlowState().addFailure(actionType, returnValueBase); Line 83: } Line 82: getActionFlowState().addFailure(actionType, returnValueBase); Line 83: } Line 84: Line 85: // Reset the next action Line 86: then(null); > This ensures that next action won't be executed (even if it's already set) yes Line 87: } Line 88: Line 89: runNextAction(); Line 90: } https://gerrit.ovirt.org/#/c/36848/45/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/action/UiVdcMultipleAction.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/action/UiVdcMultipleAction.java: Line 26: * 2. actionB, actionC and actionD will be executed in a parallel manner just after the result of actionA is returned. Line 27: * 3. actionE/F will be executed parallely after actionD's result is returned. 4. actionG is the <code>final</code>, it Line 28: * will be executed just after the result of all the action's in the flow was returned. Line 29: * Line 30: * actionA->actionB ->actionC ->actionD->actionE ->actionF ->actionG > I think the Javadoc formatting is a bit off here, compared to UiVdcAction.j Done Line 31: * Line 32: * The failures are collected and displayed after finishing the execution of the flow. Line 33: */ Line 34: public class UiVdcMultipleAction extends UiAction { Line 48: * @param showErrorDialog Line 49: * if true, in case of a error, the error dialog will be displayed immediately. Otherwise, the error will Line 50: * be collected and displayed with all the other errors at the end of the flow. Line 51: */ Line 52: public UiVdcMultipleAction(VdcActionType actionType, > Maybe worth adjusting above Javadoc to match parameters. Done Line 53: Collection<? extends VdcActionParametersBase> parametersList, Line 54: Model model, Line 55: boolean waitForResult, Line 56: boolean runNextInCaseOfError) { https://gerrit.ovirt.org/#/c/36848/45/frontend/webadmin/modules/uicommonweb/src/test/java/org/ovirt/engine/ui/uicommonweb/action/AsyncUiActionTest.java File frontend/webadmin/modules/uicommonweb/src/test/java/org/ovirt/engine/ui/uicommonweb/action/AsyncUiActionTest.java: Line 22: Line 23: protected VdcActionType ACTION_TYPE = VdcActionType.Unknown; Line 24: Line 25: @Rule Line 26: public UiCommonSetup setup = new UiCommonSetup(); > +1 for using UiCommonSetup :-) :) Line 27: Line 28: @Captor Line 29: protected ArgumentCaptor<C> callbackCaptor; Line 30: -- To view, visit https://gerrit.ovirt.org/36848 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I262282d00bbb16a65f36a2463d3ffe8dbf6594c6 Gerrit-PatchSet: 45 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com> Gerrit-Reviewer: Greg Sheremeta <gsher...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Lior Vernia <lver...@redhat.com> Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com> Gerrit-Reviewer: Yevgeny Zaspitsky <yzasp...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
