JabRefExecutorService improvement

[Brainsucker92]

I made a very minor update on the current JabRefExecutorService to support the very modern way of handling asynchronous tasks by using CompletableFuture objects. Using CompletableFuture instad of the usual Future object allows chaining of several asynchronous tasks very easily. This is a much more flexible way of dealing with asynchronous operations. I highly recomment using those methods I added instead of the old ones.

For those that are unfamiliar with the CompletableFuture pattern, they might want to have a look at my personal project where I used this to split and chain some very expensive calculations which gave me a very simple but huge increase in throughput.

• Change in CHANGELOG.md described
• Tests created for changes
• Manually tested changed features in running JabRef
• Screenshots added in PR description (for bigger UI changes)
• Ensured that the git commit message is a good one
• Check documentation status (Issue created for outdated help page at help.jabref.org?)

[tobiasdiez]

This is a good idea! The JabRefExecutorService is deprecated but you can replace the Futures in https://github.com/JabRef/jabref/blob/master/src/main/java/org/jabref/gui/util/DefaultTaskExecutor.java by CompletableFutures. In particular, these thenRun methods could be improved using the completables (maybe after a bit of refactoring):

jabref/src/main/java/org/jabref/gui/util/BackgroundTask.java

Lines 172 to 217 in fbbbda4

	public <T> BackgroundTask<T> then(Function<V, BackgroundTask<T>> nextTaskFactory) {
	return new BackgroundTask<T>() {
	@Override
	protected T call() throws Exception {
	V result = BackgroundTask.this.call();
	BackgroundTask<T> nextTask = nextTaskFactory.apply(result);
	EasyBind.subscribe(nextTask.progressProperty(), this::updateProgress);
	return nextTask.call();
	}
	};
	}
	/**
	* Creates a {@link BackgroundTask} that first runs this task and based on the result runs a second task.
	*
	* @param nextOperation the function that performs the next operation
	* @param <T> type of the return value of the second task
	*/
	public <T> BackgroundTask<T> thenRun(Function<V, T> nextOperation) {
	return new BackgroundTask<T>() {
	@Override
	protected T call() throws Exception {
	V result = BackgroundTask.this.call();
	BackgroundTask<T> nextTask = BackgroundTask.wrap(() -> nextOperation.apply(result));
	EasyBind.subscribe(nextTask.progressProperty(), this::updateProgress);
	return nextTask.call();
	}
	};
	}
	/**
	* Creates a {@link BackgroundTask} that first runs this task and based on the result runs a second task.
	*
	* @param nextOperation the function that performs the next operation
	*/
	public BackgroundTask<Void> thenRun(Consumer<V> nextOperation) {
	return new BackgroundTask<Void>() {
	@Override
	protected Void call() throws Exception {
	V result = BackgroundTask.this.call();
	BackgroundTask<Void> nextTask = BackgroundTask.wrap(() -> nextOperation.accept(result));
	EasyBind.subscribe(nextTask.progressProperty(), this::updateProgress);
	return nextTask.call();
	}
	};
	}

[Brainsucker92]

Oh, I didn't know the JabRefExecutorService was deprecated. So I'll simply add the @Deprecated annotation. And continue working on the files you suggested.

Actually I was currently working on the AbbreviateAction class as well, to apply the changes from the JabRefExecutorService. I noticed this class is clearly violating the single responsibility principle, which is why I was planning to make a bigger update to this file. But that's topic for another PR, I guess.

[tobiasdiez]

Oh, I just saw that these changes are also included in the other PR of yours. Please see my comments there and maybe try to separate both PRs to make them easier to review.

[koppor]

I think, @tobiasdiez meant this PR #5441. Since we should meet in person to discuss that, I think, we should focus on urgent issues. I am aware that @Brainsucker92 invested much energy in refactoring and making up his mind. This PR is not lost, but frozen to be picked up at a later time.