Search code examples
concurrencyjavafx-8

Task chaining in JavaFX8: Start next Task after onSucceeded finished on previous task


I'm rather new to JavaFX8 and facing the following problem. In my current App, which is for document processing/editing, I have two rather expensive tasks. Opening a document and saving a document.

My app has the buttons "import next", "export current" and "export current and import next". For Import and Export, I have two Task of the following structure:

    private class Export extends Task<Void> {
    public Export() {
        this.setOnRunning(event -> {
            // do stuff (change cursor etc)
        });

        this.setOnFailed(event -> {
            // do stuff, eg. show error box
        });

        this.setOnSucceeded(event -> {
            // do stuff
        });
    }

    @Override
    protected Void call() throws Exception {
        // do expensive stuff
        return null;
    }
}

I submit the task using the Executors.newSingleThreadExecutor();.

For the functionality "export current and import next", my goal is to submit the Export and Import tasks to the executor, but my Import tasks should only run if the export-task was sucessful and the EventHandler given in setOnSucceedded (whichs runs on the GUI thread) finished. If the export fails, it does not make any sense to load the next document because user interaction is needed. How can this be achieved?

First I tired to but the entire logic/error handling in the call method, but this does not work as I cannot change the GUI from this method (i.e. to show an error-box).

As workaround, I'm manually submitting the import-task on the last line of my setOnSucceeded in the export-task, but this is not very flexible, because I want to be sure this task exports only (without subsequent import)...


Solution

  • Don't call the handler property methods setOnXXX in your Task subclass constructor. These actually set a property on the task, so if you also call those methods from elsewhere you will replace the functionality you're implementing in the class itself, rather than add to it.

    Instead, override the protected convenience methods:

    public class Export extends Task<Void> {
    
        @Override
        protected void succeeded() {
            super.succeeded();
            // do stuff...
        }
    
        @Override
        protected void running() {
            super.running();
            // do stuff...
        }
    
        @Override
        protected void failed() {
            super.failed();
            // do stuff...
        }
    
        @Override
        protected Void call() {
            // do expensive stuff....
            return null ;
        }
    }
    

    Now you can safely use setOnXXX(...) externally to the Export class without breaking its functionality:

    Export export = new Export();
    export.setOnSucceeded(e -> {
        Import import = new Import();
        executor.submit(import);
    });
    executor.submit(export);
    

    This puts the logic for chaining the tasks at the point where you actually create them, which would seem to be the correct place to do it.

    Note that another way to provide multiple handlers for the change of state is to register listeners with the stateProperty():

    Export export = new Export();
    export.stateProperty().addListener((obs, oldState, newState) -> {
        if (newState == Worker.State.SUCCEEDED) {
            // ...
        }
    });
    

    From testing, it appears the order of execution of these different mechanisms is:

    1. state listeners
    2. the onSucceeded handler
    3. the Task.succeeded method

    All are executed on the FX Application Thread.

    So if you want the code in the Task subclass to be executed before the handler added externally, do

    public class Export extends Task<Void> {
    
        public Export() {
            stateProperty().addListener((obs, oldState, newState) -> {
                if (newState == Worker.State.RUNNING) {
                    // do stuff
                } else if (newState == Worker.State.SUCCEEDED) {
                    // do stuff
                } else if (newState == Worker.State.FAILED) {
                    // do stuff
                }
            });
        }
    
        @Override
        public Void call() {
            // ...
        }
    }
    

    Finally, you could implement the entire logic in your call method: if you need to interact with the UI you can wrap those calls in a Platform.runLater(() -> {});. However, separating the functionality into different tasks as you have done is probably cleaner anyway.