From 7022a80c954018d37f36ef62f76ccb78e0b60ea4 Mon Sep 17 00:00:00 2001 From: Markus Kreusch Date: Thu, 14 Jul 2016 13:58:17 +0200 Subject: [PATCH] Improved error handling * Created AsyncTaskService to build async UI operations which always log uncaught exceptions * Changed all executor service invocations in the UI to invocations of AsyncTaskService * Improved error handling in some other places, especially try-with-resources * Unlocking read/write locks in NioFile when opening of a channel fails --- .../common/ConsumerThrowingException.java | 2 +- .../common/RunnableThrowingException.java | 2 +- .../org/cryptomator/common/StackTrace.java | 56 ++++++ .../common/SupplierThrowingException.java | 8 + .../java/org/cryptomator/io/FileContents.java | 5 +- .../filesystem/crypto/Masterkeys.java | 13 +- .../inmem/InMemoryReadableFile.java | 6 +- .../cryptomator/filesystem/nio/NioFile.java | 28 ++- .../filesystem/nio/NioFileTest.java | 21 ++- .../cryptomator/ui/CryptomatorComponent.java | 4 + .../ui/controllers/UnlockController.java | 11 +- .../ui/controllers/UnlockedController.java | 46 ++--- .../ui/controllers/UpgradeController.java | 41 ++--- .../ui/controllers/WelcomeController.java | 41 ++--- .../cryptomator/ui/util/AsyncTaskService.java | 174 ++++++++++++++++++ 15 files changed, 351 insertions(+), 107 deletions(-) create mode 100644 main/commons/src/main/java/org/cryptomator/common/StackTrace.java create mode 100644 main/commons/src/main/java/org/cryptomator/common/SupplierThrowingException.java create mode 100644 main/ui/src/main/java/org/cryptomator/ui/util/AsyncTaskService.java diff --git a/main/commons/src/main/java/org/cryptomator/common/ConsumerThrowingException.java b/main/commons/src/main/java/org/cryptomator/common/ConsumerThrowingException.java index 649a2b6b6..242c48572 100644 --- a/main/commons/src/main/java/org/cryptomator/common/ConsumerThrowingException.java +++ b/main/commons/src/main/java/org/cryptomator/common/ConsumerThrowingException.java @@ -1,7 +1,7 @@ package org.cryptomator.common; @FunctionalInterface -public interface ConsumerThrowingException { +public interface ConsumerThrowingException { void accept(T t) throws E; diff --git a/main/commons/src/main/java/org/cryptomator/common/RunnableThrowingException.java b/main/commons/src/main/java/org/cryptomator/common/RunnableThrowingException.java index ec453b886..6af091bf1 100644 --- a/main/commons/src/main/java/org/cryptomator/common/RunnableThrowingException.java +++ b/main/commons/src/main/java/org/cryptomator/common/RunnableThrowingException.java @@ -1,7 +1,7 @@ package org.cryptomator.common; @FunctionalInterface -public interface RunnableThrowingException { +public interface RunnableThrowingException { void run() throws T; diff --git a/main/commons/src/main/java/org/cryptomator/common/StackTrace.java b/main/commons/src/main/java/org/cryptomator/common/StackTrace.java new file mode 100644 index 000000000..8d7b64c8a --- /dev/null +++ b/main/commons/src/main/java/org/cryptomator/common/StackTrace.java @@ -0,0 +1,56 @@ +/******************************************************************************* + * Copyright (c) 2016 Markus Kreusch and others. + * This file is licensed under the terms of the MIT license. + * See the LICENSE.txt file for more info. + * + * Contributors: + * Markus Kreusch - initial implementation + *******************************************************************************/ +package org.cryptomator.common; + +import java.util.stream.Stream; + +/** + * Utility to print stack traces while analyzing issues. + * + * @author Markus Kreusch + */ +public class StackTrace { + + public static void print(String message) { + Thread thread = Thread.currentThread(); + System.err.println(stackTraceFor(message, thread)); + } + + private static String stackTraceFor(String message, Thread thread) { + StringBuilder result = new StringBuilder(); + appendMessageAndThreadName(result, message, thread); + appendStackTrace(thread, result); + return result.toString(); + } + + private static void appendStackTrace(Thread thread, StringBuilder result) { + Stream.of(thread.getStackTrace()) // + .skip(4) // + .forEach(stackTraceElement -> append(stackTraceElement, result)); + } + + private static void appendMessageAndThreadName(StringBuilder result, String message, Thread thread) { + result // + .append('[') // + .append(thread.getName()) // + .append("] ") // + .append(message); + } + + private static void append(StackTraceElement stackTraceElement, StringBuilder result) { + String className = stackTraceElement.getClassName(); + String methodName = stackTraceElement.getMethodName(); + String fileName = stackTraceElement.getFileName(); + int lineNumber = stackTraceElement.getLineNumber(); + result.append('\n') // + .append(className).append(':').append(methodName) // + .append(" (").append(fileName).append(':').append(lineNumber).append(')'); + } + +} diff --git a/main/commons/src/main/java/org/cryptomator/common/SupplierThrowingException.java b/main/commons/src/main/java/org/cryptomator/common/SupplierThrowingException.java new file mode 100644 index 000000000..ed73a51d0 --- /dev/null +++ b/main/commons/src/main/java/org/cryptomator/common/SupplierThrowingException.java @@ -0,0 +1,8 @@ +package org.cryptomator.common; + +@FunctionalInterface +public interface SupplierThrowingException { + + T get() throws E; + +} diff --git a/main/filesystem-api/src/main/java/org/cryptomator/io/FileContents.java b/main/filesystem-api/src/main/java/org/cryptomator/io/FileContents.java index 6152f65cb..32e5adac7 100644 --- a/main/filesystem-api/src/main/java/org/cryptomator/io/FileContents.java +++ b/main/filesystem-api/src/main/java/org/cryptomator/io/FileContents.java @@ -4,6 +4,7 @@ import java.io.IOException; import java.io.Reader; import java.io.UncheckedIOException; import java.nio.channels.Channels; +import java.nio.channels.ReadableByteChannel; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; @@ -28,7 +29,9 @@ public final class FileContents { * @return The file's content interpreted in this FileContents' charset. */ public String readContents(File file) { - try (Reader reader = Channels.newReader(file.openReadable(), charset.newDecoder(), -1)) { + try ( // + ReadableByteChannel channel = file.openReadable(); // + Reader reader = Channels.newReader(channel, charset.newDecoder(), -1)) { return IOUtils.toString(reader); } catch (IOException e) { throw new UncheckedIOException(e); diff --git a/main/filesystem-crypto/src/main/java/org/cryptomator/filesystem/crypto/Masterkeys.java b/main/filesystem-crypto/src/main/java/org/cryptomator/filesystem/crypto/Masterkeys.java index c58c31384..9e8578dc2 100644 --- a/main/filesystem-crypto/src/main/java/org/cryptomator/filesystem/crypto/Masterkeys.java +++ b/main/filesystem-crypto/src/main/java/org/cryptomator/filesystem/crypto/Masterkeys.java @@ -16,6 +16,7 @@ import java.io.InputStream; import java.io.UncheckedIOException; import java.nio.ByteBuffer; import java.nio.channels.Channels; +import java.nio.channels.ReadableByteChannel; import javax.inject.Inject; import javax.inject.Provider; @@ -52,10 +53,14 @@ class Masterkeys { public Cryptor decrypt(Folder vaultLocation, CharSequence passphrase) throws InvalidPassphraseException { File masterkeyFile = vaultLocation.file(MASTERKEY_FILENAME); Cryptor cryptor = cryptorProvider.get(); + boolean success = false; try { readMasterKey(masterkeyFile, cryptor, passphrase); - } catch (UncheckedIOException e) { - cryptor.destroy(); + success = true; + } finally { + if (!success) { + cryptor.destroy(); + } } return cryptor; } @@ -86,7 +91,9 @@ class Masterkeys { /* I/O */ private static void readMasterKey(File file, Cryptor cryptor, CharSequence passphrase) throws UncheckedIOException, InvalidPassphraseException { - try (InputStream in = Channels.newInputStream(file.openReadable())) { + try ( // + ReadableByteChannel channel = file.openReadable(); // + InputStream in = Channels.newInputStream(channel)) { final byte[] fileContents = IOUtils.toByteArray(in); cryptor.readKeysFromMasterkeyFile(fileContents, passphrase); } catch (IOException e) { diff --git a/main/filesystem-inmemory/src/main/java/org/cryptomator/filesystem/inmem/InMemoryReadableFile.java b/main/filesystem-inmemory/src/main/java/org/cryptomator/filesystem/inmem/InMemoryReadableFile.java index 6ba8d326b..90eab31f3 100644 --- a/main/filesystem-inmemory/src/main/java/org/cryptomator/filesystem/inmem/InMemoryReadableFile.java +++ b/main/filesystem-inmemory/src/main/java/org/cryptomator/filesystem/inmem/InMemoryReadableFile.java @@ -64,8 +64,10 @@ class InMemoryReadableFile implements ReadableFile { @Override public void close() throws UncheckedIOException { - open.set(false); - readLock.unlock(); + if (open.get()) { + open.set(false); + readLock.unlock(); + } } } diff --git a/main/filesystem-nio/src/main/java/org/cryptomator/filesystem/nio/NioFile.java b/main/filesystem-nio/src/main/java/org/cryptomator/filesystem/nio/NioFile.java index 5e38a8e25..e1afc2718 100644 --- a/main/filesystem-nio/src/main/java/org/cryptomator/filesystem/nio/NioFile.java +++ b/main/filesystem-nio/src/main/java/org/cryptomator/filesystem/nio/NioFile.java @@ -30,13 +30,21 @@ class NioFile extends NioNode implements File { @Override public ReadableFile openReadable() throws UncheckedIOException { if (lock.getWriteHoldCount() > 0) { - throw new IllegalStateException("Current thread is currently writing this file"); + throw new IllegalStateException("Current thread is currently writing " + path); } if (lock.getReadHoldCount() > 0) { - throw new IllegalStateException("Current thread is already reading this file"); + throw new IllegalStateException("Current thread is already reading " + path); } lock.readLock().lock(); - return instanceFactory.readableNioFile(path, sharedChannel, this::unlockReadLock); + ReadableFile result = null; + try { + result = instanceFactory.readableNioFile(path, sharedChannel, this::unlockReadLock); + } finally { + if (result == null) { + unlockReadLock(); + } + } + return result; } private void unlockReadLock() { @@ -46,13 +54,21 @@ class NioFile extends NioNode implements File { @Override public WritableFile openWritable() throws UncheckedIOException { if (lock.getWriteHoldCount() > 0) { - throw new IllegalStateException("Current thread is already writing this file"); + throw new IllegalStateException("Current thread is already writing " + path); } if (lock.getReadHoldCount() > 0) { - throw new IllegalStateException("Current thread is currently reading this file"); + throw new IllegalStateException("Current thread is currently reading " + path); } lockWriteLock(); - return instanceFactory.writableNioFile(fileSystem(), path, sharedChannel, this::unlockWriteLock); + WritableFile result = null; + try { + result = instanceFactory.writableNioFile(fileSystem(), path, sharedChannel, this::unlockWriteLock); + } finally { + if (result == null) { + unlockWriteLock(); + } + } + return result; } // visible for testing diff --git a/main/filesystem-nio/src/test/java/org/cryptomator/filesystem/nio/NioFileTest.java b/main/filesystem-nio/src/test/java/org/cryptomator/filesystem/nio/NioFileTest.java index 489849f24..ec4269fc9 100644 --- a/main/filesystem-nio/src/test/java/org/cryptomator/filesystem/nio/NioFileTest.java +++ b/main/filesystem-nio/src/test/java/org/cryptomator/filesystem/nio/NioFileTest.java @@ -99,10 +99,11 @@ public class NioFileTest { @Test public void testOpenReadableInvokedBeforeInvokingAfterCloseOperationThrowsIllegalStateException() { + when(instanceFactory.readableNioFile(same(path), same(channel), any())).thenReturn(mock(ReadableNioFile.class)); inTest.openReadable(); thrown.expect(IllegalStateException.class); - thrown.expectMessage("already reading this file"); + thrown.expectMessage("already reading " + path); inTest.openReadable(); } @@ -111,7 +112,7 @@ public class NioFileTest { public void testOpenReadableInvokedAfterAfterCloseOperationCreatesNewReadableFile() { ReadableNioFile readableNioFile = mock(ReadableNioFile.class); ArgumentCaptor captor = ArgumentCaptor.forClass(Runnable.class); - when(instanceFactory.readableNioFile(same(path), same(channel), captor.capture())).thenReturn(null, readableNioFile); + when(instanceFactory.readableNioFile(same(path), same(channel), captor.capture())).thenReturn(mock(ReadableNioFile.class), readableNioFile); inTest.openReadable(); captor.getValue().run(); @@ -122,10 +123,11 @@ public class NioFileTest { @Test public void testOpenReadableInvokedBeforeInvokingAfterCloseOperationOfOpenWritableThrowsIllegalStateException() { + when(instanceFactory.writableNioFile(same(fileSystem), same(path), same(channel), any())).thenReturn(mock(WritableNioFile.class)); inTest.openWritable(); thrown.expect(IllegalStateException.class); - thrown.expectMessage("currently writing this file"); + thrown.expectMessage("currently writing " + path); inTest.openReadable(); } @@ -133,7 +135,7 @@ public class NioFileTest { @Test public void testOpenReadableInvokedAfterInvokingAfterCloseOperationWorks() { ArgumentCaptor captor = ArgumentCaptor.forClass(Runnable.class); - when(instanceFactory.writableNioFile(same(fileSystem), same(path), same(channel), captor.capture())).thenReturn(null); + when(instanceFactory.writableNioFile(same(fileSystem), same(path), same(channel), captor.capture())).thenReturn(mock(WritableNioFile.class)); inTest.openWritable(); captor.getValue().run(); @@ -154,7 +156,7 @@ public class NioFileTest { public void testOpenWritableInvokedAfterAfterCloseOperationCreatesNewWritableFile() { WritableNioFile writableNioFile = mock(WritableNioFile.class); ArgumentCaptor captor = ArgumentCaptor.forClass(Runnable.class); - when(instanceFactory.writableNioFile(same(fileSystem), same(path), same(channel), captor.capture())).thenReturn(null, writableNioFile); + when(instanceFactory.writableNioFile(same(fileSystem), same(path), same(channel), captor.capture())).thenReturn(mock(WritableNioFile.class), writableNioFile); inTest.openWritable(); captor.getValue().run(); @@ -165,28 +167,31 @@ public class NioFileTest { @Test public void testOpenWritableInvokedBeforeInvokingAfterCloseOperationThrowsIllegalStateException() { + when(instanceFactory.writableNioFile(same(fileSystem), same(path), same(channel), any())).thenReturn(mock(WritableNioFile.class)); inTest.openWritable(); thrown.expect(IllegalStateException.class); - thrown.expectMessage("already writing this file"); + thrown.expectMessage("already writing " + path); inTest.openWritable(); } @Test public void testOpenWritableInvokedBeforeInvokingAfterCloseOperationFromOpenReadableThrowsIllegalStateException() { + when(instanceFactory.readableNioFile(same(path), same(channel), any())).thenReturn(mock(ReadableNioFile.class)); inTest.openReadable(); thrown.expect(IllegalStateException.class); - thrown.expectMessage("currently reading this file"); + thrown.expectMessage("currently reading " + path); inTest.openWritable(); } @Test public void testOpenWritableInvokedAfterInvokingAfterCloseOperationWorks() { + ReadableNioFile readableNioFile = mock(ReadableNioFile.class); ArgumentCaptor captor = ArgumentCaptor.forClass(Runnable.class); - when(instanceFactory.readableNioFile(same(path), same(channel), captor.capture())).thenReturn(null); + when(instanceFactory.readableNioFile(same(path), same(channel), captor.capture())).thenReturn(readableNioFile); inTest.openReadable(); captor.getValue().run(); diff --git a/main/ui/src/main/java/org/cryptomator/ui/CryptomatorComponent.java b/main/ui/src/main/java/org/cryptomator/ui/CryptomatorComponent.java index b32935193..095caeb89 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/CryptomatorComponent.java +++ b/main/ui/src/main/java/org/cryptomator/ui/CryptomatorComponent.java @@ -14,6 +14,7 @@ import javax.inject.Singleton; import org.cryptomator.ui.controllers.MainController; import org.cryptomator.ui.settings.Localization; +import org.cryptomator.ui.util.AsyncTaskService; import org.cryptomator.ui.util.DeferredCloser; import dagger.Component; @@ -21,6 +22,9 @@ import dagger.Component; @Singleton @Component(modules = CryptomatorModule.class) interface CryptomatorComponent { + + AsyncTaskService asyncTaskService(); + ExecutorService executorService(); DeferredCloser deferredCloser(); diff --git a/main/ui/src/main/java/org/cryptomator/ui/controllers/UnlockController.java b/main/ui/src/main/java/org/cryptomator/ui/controllers/UnlockController.java index 759227123..d17175dc7 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/controllers/UnlockController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/controllers/UnlockController.java @@ -11,7 +11,6 @@ package org.cryptomator.ui.controllers; import java.net.URL; import java.util.Comparator; import java.util.Optional; -import java.util.concurrent.ExecutorService; import javax.inject.Inject; @@ -27,6 +26,7 @@ import org.cryptomator.ui.controls.SecPasswordField; import org.cryptomator.ui.model.Vault; import org.cryptomator.ui.settings.Localization; import org.cryptomator.ui.settings.Settings; +import org.cryptomator.ui.util.AsyncTaskService; import org.fxmisc.easybind.EasyBind; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -56,7 +56,7 @@ public class UnlockController extends LocalizedFXMLViewController { private static final Logger LOG = LoggerFactory.getLogger(UnlockController.class); private final Application app; - private final ExecutorService exec; + private final AsyncTaskService asyncTaskService; private final Lazy frontendFactory; private final Settings settings; private final WindowsDriveLetters driveLetters; @@ -65,10 +65,10 @@ public class UnlockController extends LocalizedFXMLViewController { private Optional listener = Optional.empty(); @Inject - public UnlockController(Application app, Localization localization, ExecutorService exec, Lazy frontendFactory, Settings settings, WindowsDriveLetters driveLetters) { + public UnlockController(Application app, Localization localization, AsyncTaskService asyncTaskService, Lazy frontendFactory, Settings settings, WindowsDriveLetters driveLetters) { super(localization); this.app = app; - this.exec = exec; + this.asyncTaskService = asyncTaskService; this.frontendFactory = frontendFactory; this.settings = settings; this.driveLetters = driveLetters; @@ -275,8 +275,7 @@ public class UnlockController extends LocalizedFXMLViewController { progressIndicator.setVisible(true); downloadsPageLink.setVisible(false); CharSequence password = passwordField.getCharacters(); - exec.submit(() -> this.unlock(vault.get(), password)); - + asyncTaskService.asyncTaskOf(() -> this.unlock(vault.get(), password)).run(); } private void unlock(Vault vault, CharSequence password) { diff --git a/main/ui/src/main/java/org/cryptomator/ui/controllers/UnlockedController.java b/main/ui/src/main/java/org/cryptomator/ui/controllers/UnlockedController.java index 9cc58ecf1..c0e671d8e 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/controllers/UnlockedController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/controllers/UnlockedController.java @@ -10,7 +10,6 @@ package org.cryptomator.ui.controllers; import java.net.URL; import java.util.Optional; -import java.util.concurrent.ExecutorService; import javax.inject.Inject; import javax.inject.Provider; @@ -19,6 +18,7 @@ import org.cryptomator.frontend.CommandFailedException; import org.cryptomator.ui.model.Vault; import org.cryptomator.ui.settings.Localization; import org.cryptomator.ui.util.ActiveWindowStyleSupport; +import org.cryptomator.ui.util.AsyncTaskService; import org.fxmisc.easybind.EasyBind; import javafx.animation.Animation; @@ -52,16 +52,16 @@ public class UnlockedController extends LocalizedFXMLViewController { private final Stage macWarningsWindow = new Stage(); private final MacWarningsController macWarningsController; - private final ExecutorService exec; + private final AsyncTaskService asyncTaskService; private final ObjectProperty vault = new SimpleObjectProperty<>(); private Optional listener = Optional.empty(); private Timeline ioAnimation; @Inject - public UnlockedController(Localization localization, Provider macWarningsControllerProvider, ExecutorService exec) { + public UnlockedController(Localization localization, Provider macWarningsControllerProvider, AsyncTaskService asyncTaskService) { super(localization); this.macWarningsController = macWarningsControllerProvider.get(); - this.exec = exec; + this.asyncTaskService = asyncTaskService; macWarningsController.vault.bind(this.vault); } @@ -116,18 +116,14 @@ public class UnlockedController extends LocalizedFXMLViewController { @FXML private void didClickLockVault(ActionEvent event) { - exec.submit(() -> { - try { - vault.get().unmount(); - } catch (CommandFailedException e) { - Platform.runLater(() -> { - messageLabel.setText(localization.getString("unlocked.label.unmountFailed")); - }); - return; - } + asyncTaskService.asyncTaskOf(() -> { + vault.get().unmount(); vault.get().deactivateFrontend(); - listener.ifPresent(this::invokeListenerLater); - }); + }).onError(CommandFailedException.class, () -> { + messageLabel.setText(localization.getString("unlocked.label.unmountFailed")); + }).andFinally(() -> { + listener.ifPresent(listener -> listener.didLock(this)); + }).run(); } @FXML @@ -142,15 +138,11 @@ public class UnlockedController extends LocalizedFXMLViewController { @FXML private void didClickRevealVault(ActionEvent event) { - exec.submit(() -> { - try { - vault.get().reveal(); - } catch (CommandFailedException e) { - Platform.runLater(() -> { - messageLabel.setText(localization.getString("unlocked.label.revealFailed")); - }); - } - }); + asyncTaskService.asyncTaskOf(() -> { + vault.get().reveal(); + }).onError(CommandFailedException.class, () -> { + messageLabel.setText(localization.getString("unlocked.label.revealFailed")); + }).run(); } @FXML @@ -258,12 +250,6 @@ public class UnlockedController extends LocalizedFXMLViewController { this.listener = Optional.ofNullable(listener); } - private void invokeListenerLater(LockListener listener) { - Platform.runLater(() -> { - listener.didLock(this); - }); - } - @FunctionalInterface interface LockListener { void didLock(UnlockedController ctrl); diff --git a/main/ui/src/main/java/org/cryptomator/ui/controllers/UpgradeController.java b/main/ui/src/main/java/org/cryptomator/ui/controllers/UpgradeController.java index db92d7df4..729f3fdf9 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/controllers/UpgradeController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/controllers/UpgradeController.java @@ -3,7 +3,6 @@ package org.cryptomator.ui.controllers; import java.net.URL; import java.util.Objects; import java.util.Optional; -import java.util.concurrent.ExecutorService; import javax.inject.Inject; @@ -13,11 +12,9 @@ import org.cryptomator.ui.model.UpgradeStrategy; import org.cryptomator.ui.model.UpgradeStrategy.UpgradeFailedException; import org.cryptomator.ui.model.Vault; import org.cryptomator.ui.settings.Localization; +import org.cryptomator.ui.util.AsyncTaskService; import org.fxmisc.easybind.EasyBind; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import javafx.application.Platform; import javafx.beans.property.ObjectProperty; import javafx.beans.property.SimpleObjectProperty; import javafx.event.ActionEvent; @@ -28,19 +25,17 @@ import javafx.scene.control.ProgressIndicator; public class UpgradeController extends LocalizedFXMLViewController { - private static final Logger LOG = LoggerFactory.getLogger(UpgradeController.class); - final ObjectProperty vault = new SimpleObjectProperty<>(); final ObjectProperty> strategy = new SimpleObjectProperty<>(); private final UpgradeStrategies strategies; - private final ExecutorService exec; + private final AsyncTaskService asyncTaskService; private Optional listener = Optional.empty(); @Inject - public UpgradeController(Localization localization, UpgradeStrategies strategies, ExecutorService exec) { + public UpgradeController(Localization localization, UpgradeStrategies strategies, AsyncTaskService asyncTaskService) { super(localization); this.strategies = strategies; - this.exec = exec; + this.asyncTaskService = asyncTaskService; } @FXML @@ -103,26 +98,22 @@ public class UpgradeController extends LocalizedFXMLViewController { Vault v = Objects.requireNonNull(vault.getValue()); passwordField.setDisable(true); progressIndicator.setVisible(true); - exec.submit(() -> { - if (!instruction.isApplicable(v)) { - LOG.error("No upgrade needed for " + v.path().getValue()); - throw new IllegalStateException("No ugprade needed for " + v.path().getValue()); - } - try { - instruction.upgrade(v, passwordField.getCharacters()); - Platform.runLater(this::showNextUpgrade); - } catch (UpgradeFailedException e) { - Platform.runLater(() -> { + asyncTaskService // + .asyncTaskOf(() -> { + if (!instruction.isApplicable(v)) { + throw new IllegalStateException("No ugprade needed for " + v.path().getValue()); + } + instruction.upgrade(v, passwordField.getCharacters()); + }) // + .onSuccess(this::showNextUpgrade) // + .onError(UpgradeFailedException.class, e -> { errorLabel.setText(e.getLocalizedMessage()); - }); - } finally { - Platform.runLater(() -> { + }) // + .andFinally(() -> { progressIndicator.setVisible(false); passwordField.setDisable(false); passwordField.swipe(); - }); - } - }); + }).run(); } private void showNextUpgrade() { diff --git a/main/ui/src/main/java/org/cryptomator/ui/controllers/WelcomeController.java b/main/ui/src/main/java/org/cryptomator/ui/controllers/WelcomeController.java index b7c41776a..143a43ef9 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/controllers/WelcomeController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/controllers/WelcomeController.java @@ -8,14 +8,12 @@ ******************************************************************************/ package org.cryptomator.ui.controllers; -import java.io.IOException; import java.io.InputStream; import java.net.URL; import java.util.Comparator; import java.util.HashMap; import java.util.Map; import java.util.Optional; -import java.util.concurrent.ExecutorService; import javax.inject.Inject; import javax.inject.Named; @@ -31,6 +29,7 @@ import org.apache.commons.io.IOUtils; import org.apache.commons.lang3.SystemUtils; import org.cryptomator.ui.settings.Localization; import org.cryptomator.ui.settings.Settings; +import org.cryptomator.ui.util.AsyncTaskService; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -54,15 +53,15 @@ public class WelcomeController extends LocalizedFXMLViewController { private final Application app; private final Settings settings; private final Comparator semVerComparator; - private final ExecutorService executor; + private final AsyncTaskService asyncTaskService; @Inject - public WelcomeController(Application app, Localization localization, Settings settings, @Named("SemVer") Comparator semVerComparator, ExecutorService executor) { + public WelcomeController(Application app, Localization localization, Settings settings, @Named("SemVer") Comparator semVerComparator, AsyncTaskService asyncTaskService) { super(localization); this.app = app; this.settings = settings; this.semVerComparator = semVerComparator; - this.executor = executor; + this.asyncTaskService = asyncTaskService; } @FXML @@ -82,7 +81,7 @@ public class WelcomeController extends LocalizedFXMLViewController { if (areUpdatesManagedExternally()) { checkForUpdatesContainer.setVisible(false); } else if (settings.isCheckForUpdatesEnabled()) { - executor.execute(this::checkForUpdates); + this.checkForUpdates(); } } @@ -100,16 +99,14 @@ public class WelcomeController extends LocalizedFXMLViewController { } private void checkForUpdates() { - Platform.runLater(() -> { - checkForUpdatesStatus.setText(localization.getString("welcome.checkForUpdates.label.currentlyChecking")); - checkForUpdatesIndicator.setVisible(true); - }); - final HttpClient client = new HttpClient(); - final HttpMethod method = new GetMethod("https://cryptomator.org/downloads/latestVersion.json"); - client.getParams().setParameter(HttpClientParams.USER_AGENT, "Cryptomator VersionChecker/" + applicationVersion().orElse("SNAPSHOT")); - client.getParams().setCookiePolicy(CookiePolicy.IGNORE_COOKIES); - client.getParams().setConnectionManagerTimeout(5000); - try { + checkForUpdatesStatus.setText(localization.getString("welcome.checkForUpdates.label.currentlyChecking")); + checkForUpdatesIndicator.setVisible(true); + asyncTaskService.asyncTaskOf(() -> { + final HttpClient client = new HttpClient(); + final HttpMethod method = new GetMethod("https://cryptomator.org/downloads/latestVersion.json"); + client.getParams().setParameter(HttpClientParams.USER_AGENT, "Cryptomator VersionChecker/" + applicationVersion().orElse("SNAPSHOT")); + client.getParams().setCookiePolicy(CookiePolicy.IGNORE_COOKIES); + client.getParams().setConnectionManagerTimeout(5000); client.executeMethod(method); final InputStream responseBodyStream = method.getResponseBodyAsStream(); if (method.getStatusCode() == HttpStatus.SC_OK && responseBodyStream != null) { @@ -121,14 +118,10 @@ public class WelcomeController extends LocalizedFXMLViewController { this.compareVersions(map); } } - } catch (IOException e) { - // no error handling required. Maybe next time the version check is successful. - } finally { - Platform.runLater(() -> { - checkForUpdatesStatus.setText(""); - checkForUpdatesIndicator.setVisible(false); - }); - } + }).andFinally(() -> { + checkForUpdatesStatus.setText(""); + checkForUpdatesIndicator.setVisible(false); + }).run(); } private Optional applicationVersion() { diff --git a/main/ui/src/main/java/org/cryptomator/ui/util/AsyncTaskService.java b/main/ui/src/main/java/org/cryptomator/ui/util/AsyncTaskService.java new file mode 100644 index 000000000..e61a5b774 --- /dev/null +++ b/main/ui/src/main/java/org/cryptomator/ui/util/AsyncTaskService.java @@ -0,0 +1,174 @@ +package org.cryptomator.ui.util; + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.ExecutorService; + +import javax.inject.Inject; +import javax.inject.Singleton; + +import org.cryptomator.common.ConsumerThrowingException; +import org.cryptomator.common.RunnableThrowingException; +import org.cryptomator.common.SupplierThrowingException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import javafx.application.Platform; + +@Singleton +public class AsyncTaskService { + + private static final Logger LOG = LoggerFactory.getLogger(AsyncTaskService.class); + + private final ExecutorService executor; + + @Inject + public AsyncTaskService(ExecutorService executor) { + this.executor = executor; + } + + public AsyncTaskWithoutSuccessHandler asyncTaskOf(RunnableThrowingException task) { + return new AsyncTaskImpl<>(() -> { + task.run(); + return null; + }); + } + + public AsyncTaskWithoutSuccessHandler asyncTaskOf(SupplierThrowingException task) { + return new AsyncTaskImpl<>(task); + } + + private class AsyncTaskImpl implements AsyncTaskWithoutSuccessHandler { + + private final SupplierThrowingException task; + + private ConsumerThrowingException successHandler = value -> { + }; + private List> errorHandlers = new ArrayList<>(); + private RunnableThrowingException finallyHandler = () -> { + }; + + public AsyncTaskImpl(SupplierThrowingException task) { + this.task = task; + } + + @Override + public AsyncTaskWithoutErrorHandler onSuccess(ConsumerThrowingException handler) { + successHandler = handler; + return this; + } + + @Override + public AsyncTaskWithoutErrorHandler onSuccess(RunnableThrowingException handler) { + return onSuccess(result -> handler.run()); + } + + @SuppressWarnings({"unchecked", "rawtypes"}) + @Override + public AsyncTaskWithoutErrorHandler onError(Class type, ConsumerThrowingException handler) { + errorHandlers.add((ErrorHandler) new ErrorHandler<>(type, handler)); + return this; + } + + @Override + public AsyncTaskWithoutErrorHandler onError(Class type, RunnableThrowingException handler) { + return onError(type, error -> handler.run()); + } + + @Override + public AsyncTask andFinally(RunnableThrowingException handler) { + finallyHandler = handler; + return this; + } + + @Override + public void run() { + errorHandlers.add(ErrorHandler.LOGGING_HANDLER); + executor.execute(() -> logExceptions(() -> { + try { + ResultType result = task.get(); + Platform.runLater(() -> { + try { + successHandler.accept(result); + } catch (Throwable e) { + LOG.error("Uncaught exception", e); + } + }); + } catch (Throwable e) { + ErrorHandler errorHandler = errorHandlerFor(e); + Platform.runLater(toRunnableLoggingException(() -> errorHandler.accept(e))); + } finally { + Platform.runLater(toRunnableLoggingException(finallyHandler)); + } + })); + } + + private ErrorHandler errorHandlerFor(Throwable e) { + return errorHandlers.stream().filter(handler -> handler.handles(e)).findFirst().get(); + } + + } + + private static Runnable toRunnableLoggingException(RunnableThrowingException delegate) { + return () -> logExceptions(delegate); + } + + private static void logExceptions(RunnableThrowingException delegate) { + try { + delegate.run(); + } catch (Throwable e) { + LOG.error("Uncaught exception", e); + } + } + + private static class ErrorHandler implements ConsumerThrowingException { + + public static final ErrorHandler LOGGING_HANDLER = new ErrorHandler(Throwable.class, error -> { + LOG.error("Uncaught exception", error); + }); + + private final Class type; + private final ConsumerThrowingException delegate; + + public ErrorHandler(Class type, ConsumerThrowingException delegate) { + this.type = type; + this.delegate = delegate; + } + + public boolean handles(Throwable error) { + return type.isInstance(error); + } + + @Override + public void accept(ErrorType error) throws Throwable { + delegate.accept(error); + } + + } + + public interface AsyncTaskWithoutSuccessHandler extends AsyncTaskWithoutErrorHandler { + + AsyncTaskWithoutErrorHandler onSuccess(ConsumerThrowingException handler); + + AsyncTaskWithoutErrorHandler onSuccess(RunnableThrowingException handler); + + } + + public interface AsyncTaskWithoutErrorHandler extends AsyncTaskWithoutFinallyHandler { + + AsyncTaskWithoutErrorHandler onError(Class type, ConsumerThrowingException handler); + + AsyncTaskWithoutErrorHandler onError(Class type, RunnableThrowingException handler); + + } + + public interface AsyncTaskWithoutFinallyHandler extends AsyncTask { + + AsyncTask andFinally(RunnableThrowingException handler); + + } + + public interface AsyncTask extends Runnable { + } + +}