From 96012636456683534c38cc234ec310a0459605ff Mon Sep 17 00:00:00 2001 From: JaniruTEC Date: Sun, 11 Apr 2021 02:35:32 +0200 Subject: [PATCH] Added an additional layer of abstraction to error reports Fixed var names Added abstract base class for controllers Added base interface for error components Added base interface for error component builders Generified scene providers Made GenericError classes and InvalidMountPointException classes depend on those base interfaces/classes --- .../ui/error/AbstractErrorController.java | 35 +++++++++++ .../ui/error/ErrorComponentBase.java | 53 ++++++++++++++++ .../org/cryptomator/ui/error/ErrorModule.java | 19 ++---- .../org/cryptomator/ui/error/ErrorReport.java | 14 +++++ .../ui/error/GenericErrorComponent.java | 61 +++++++------------ .../ui/error/GenericErrorController.java | 30 ++------- .../InvalidMountPointExceptionComponent.java | 42 +++++++++++++ .../ui/fxapp/FxApplicationModule.java | 7 ++- .../UnlockInvalidMountPointController.java | 27 +++----- .../cryptomator/ui/unlock/UnlockModule.java | 6 -- .../cryptomator/ui/unlock/UnlockScoped.java | 2 +- .../cryptomator/ui/unlock/UnlockWorkflow.java | 22 +++---- .../main/resources/fxml/generic_error.fxml | 2 +- 13 files changed, 197 insertions(+), 123 deletions(-) create mode 100644 main/ui/src/main/java/org/cryptomator/ui/error/AbstractErrorController.java create mode 100644 main/ui/src/main/java/org/cryptomator/ui/error/ErrorComponentBase.java create mode 100644 main/ui/src/main/java/org/cryptomator/ui/error/ErrorReport.java create mode 100644 main/ui/src/main/java/org/cryptomator/ui/error/InvalidMountPointExceptionComponent.java diff --git a/main/ui/src/main/java/org/cryptomator/ui/error/AbstractErrorController.java b/main/ui/src/main/java/org/cryptomator/ui/error/AbstractErrorController.java new file mode 100644 index 000000000..c6390b66c --- /dev/null +++ b/main/ui/src/main/java/org/cryptomator/ui/error/AbstractErrorController.java @@ -0,0 +1,35 @@ +package org.cryptomator.ui.error; + +import org.cryptomator.ui.common.FxController; + +import javax.annotation.Nullable; +import javafx.fxml.FXML; +import javafx.scene.Scene; +import javafx.stage.Stage; + +public class AbstractErrorController implements FxController { + + protected final Stage window; + protected final Scene returnScene; + + public AbstractErrorController(@ErrorReport Stage window, @ErrorReport @Nullable Scene returnScene) { + this.returnScene = returnScene; + this.window = window; + } + + @FXML + public void back() { + if (this.returnScene != null) { + this.window.setScene(this.returnScene); + } + } + + @FXML + public void close() { + this.window.close(); + } + + public boolean isReturnScenePresent() { + return this.returnScene != null; + } +} \ No newline at end of file diff --git a/main/ui/src/main/java/org/cryptomator/ui/error/ErrorComponentBase.java b/main/ui/src/main/java/org/cryptomator/ui/error/ErrorComponentBase.java new file mode 100644 index 000000000..3405ddb6d --- /dev/null +++ b/main/ui/src/main/java/org/cryptomator/ui/error/ErrorComponentBase.java @@ -0,0 +1,53 @@ +package org.cryptomator.ui.error; + +import dagger.BindsInstance; + +import javax.annotation.Nullable; +import javax.inject.Named; +import javafx.application.Platform; +import javafx.scene.Scene; +import javafx.stage.Stage; + +public interface ErrorComponentBase { + + @ErrorReport + Stage window(); + + @Named("errorScene") + Scene errorScene(); + + @ErrorReport + Throwable cause(); + + @ErrorReport + @Nullable + Scene previousScene(); + + default void showErrorScene() { + if (Platform.isFxApplicationThread()) { + show(); + } else { + Platform.runLater(this::show); + } + } + + private void show() { + Stage stage = window(); + stage.setScene(errorScene()); + stage.show(); + } + + interface BuilderBase { + + @BindsInstance + B window(@ErrorReport Stage window); + + @BindsInstance + B cause(@ErrorReport Throwable cause); + + @BindsInstance + B returnToScene(@ErrorReport @Nullable Scene returnScene); + + C build(); + } +} \ No newline at end of file diff --git a/main/ui/src/main/java/org/cryptomator/ui/error/ErrorModule.java b/main/ui/src/main/java/org/cryptomator/ui/error/ErrorModule.java index b27a3271e..361b9daec 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/error/ErrorModule.java +++ b/main/ui/src/main/java/org/cryptomator/ui/error/ErrorModule.java @@ -1,15 +1,11 @@ package org.cryptomator.ui.error; -import dagger.Binds; import dagger.Module; import dagger.Provides; -import dagger.multibindings.IntoMap; import org.cryptomator.ui.common.DefaultSceneFactory; import org.cryptomator.ui.common.FxController; -import org.cryptomator.ui.common.FxControllerKey; import org.cryptomator.ui.common.FxmlFile; import org.cryptomator.ui.common.FxmlLoaderFactory; -import org.cryptomator.ui.common.FxmlScene; import javax.inject.Named; import javax.inject.Provider; @@ -30,23 +26,16 @@ abstract class ErrorModule { @Provides @Named("stackTrace") - static String provideStackTrace(Throwable cause) { + static String provideStackTrace(@ErrorReport Throwable cause) { // TODO deduplicate VaultDetailUnknownErrorController.java ByteArrayOutputStream baos = new ByteArrayOutputStream(); cause.printStackTrace(new PrintStream(baos)); return baos.toString(StandardCharsets.UTF_8); } - @Binds - @IntoMap - @FxControllerKey(GenericErrorController.class) - abstract FxController bindErrorController(GenericErrorController controller); - @Provides - @FxmlScene(FxmlFile.GENERIC_ERROR) - static Scene provideErrorScene(FxmlLoaderFactory fxmlLoaders) { - return fxmlLoaders.createScene(FxmlFile.GENERIC_ERROR); + @Named("errorScene") + static Scene provideErrorScene(FxmlLoaderFactory fxmlLoaders, @ErrorReport FxmlFile file) { + return fxmlLoaders.createScene(file); } - - } diff --git a/main/ui/src/main/java/org/cryptomator/ui/error/ErrorReport.java b/main/ui/src/main/java/org/cryptomator/ui/error/ErrorReport.java new file mode 100644 index 000000000..ba61b37ea --- /dev/null +++ b/main/ui/src/main/java/org/cryptomator/ui/error/ErrorReport.java @@ -0,0 +1,14 @@ +package org.cryptomator.ui.error; + +import javax.inject.Qualifier; +import java.lang.annotation.Documented; +import java.lang.annotation.Retention; + +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +@Qualifier +@Documented +@Retention(RUNTIME) +public @interface ErrorReport { + +} \ No newline at end of file diff --git a/main/ui/src/main/java/org/cryptomator/ui/error/GenericErrorComponent.java b/main/ui/src/main/java/org/cryptomator/ui/error/GenericErrorComponent.java index f24c0f6f1..5a47b678c 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/error/GenericErrorComponent.java +++ b/main/ui/src/main/java/org/cryptomator/ui/error/GenericErrorComponent.java @@ -1,51 +1,32 @@ package org.cryptomator.ui.error; -import dagger.BindsInstance; +import dagger.Binds; +import dagger.Module; +import dagger.Provides; import dagger.Subcomponent; +import dagger.multibindings.IntoMap; +import org.cryptomator.ui.common.FxController; +import org.cryptomator.ui.common.FxControllerKey; import org.cryptomator.ui.common.FxmlFile; -import org.cryptomator.ui.common.FxmlScene; -import javax.annotation.Nullable; -import javafx.application.Platform; -import javafx.scene.Scene; -import javafx.stage.Stage; - -@Subcomponent(modules = {ErrorModule.class}) -public interface GenericErrorComponent { - - Stage window(); - - @FxmlScene(FxmlFile.GENERIC_ERROR) - Scene scene(); - - default void showErrorScene() { - if (Platform.isFxApplicationThread()) { - show(); - } else { - Platform.runLater(this::show); - } - } - - private void show() { - Stage stage = window(); - stage.setScene(scene()); - stage.show(); - } +@Subcomponent(modules = {ErrorModule.class, GenericErrorComponent.GenericErrorModule.class}) +public interface GenericErrorComponent extends ErrorComponentBase { @Subcomponent.Builder - interface Builder { + interface Builder extends BuilderBase { /* Handled by base interface */ } - @BindsInstance - Builder cause(Throwable cause); + @Module + abstract class GenericErrorModule { - @BindsInstance - Builder window(Stage window); - - @BindsInstance - Builder returnToScene(@Nullable Scene previousScene); - - GenericErrorComponent build(); + @Provides + @ErrorReport + static FxmlFile provideFxmlFile() { + return FxmlFile.GENERIC_ERROR; + } + @Binds + @IntoMap + @FxControllerKey(GenericErrorController.class) + abstract FxController bindController(GenericErrorController controller); } - -} +} \ No newline at end of file diff --git a/main/ui/src/main/java/org/cryptomator/ui/error/GenericErrorController.java b/main/ui/src/main/java/org/cryptomator/ui/error/GenericErrorController.java index 86b99cd95..b6edbc9dc 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/error/GenericErrorController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/error/GenericErrorController.java @@ -1,46 +1,24 @@ package org.cryptomator.ui.error; -import org.cryptomator.ui.common.FxController; - import javax.annotation.Nullable; import javax.inject.Inject; import javax.inject.Named; -import javafx.fxml.FXML; import javafx.scene.Scene; import javafx.stage.Stage; -public class GenericErrorController implements FxController { +public class GenericErrorController extends AbstractErrorController { private final String stackTrace; - private final Scene previousScene; - private final Stage window; @Inject - GenericErrorController(@Named("stackTrace") String stackTrace, @Nullable Scene previousScene, Stage window) { + GenericErrorController(@ErrorReport Stage window, @ErrorReport @Nullable Scene previousScene, @Named("stackTrace") String stackTrace) { + super(window, previousScene); this.stackTrace = stackTrace; - this.previousScene = previousScene; - this.window = window; - } - - @FXML - public void back() { - if (previousScene != null) { - window.setScene(previousScene); - } - } - - @FXML - public void close() { - window.close(); } /* Getter/Setter */ - public boolean isPreviousScenePresent() { - return previousScene != null; - } - public String getStackTrace() { - return stackTrace; + return this.stackTrace; } } diff --git a/main/ui/src/main/java/org/cryptomator/ui/error/InvalidMountPointExceptionComponent.java b/main/ui/src/main/java/org/cryptomator/ui/error/InvalidMountPointExceptionComponent.java new file mode 100644 index 000000000..316e46f26 --- /dev/null +++ b/main/ui/src/main/java/org/cryptomator/ui/error/InvalidMountPointExceptionComponent.java @@ -0,0 +1,42 @@ +package org.cryptomator.ui.error; + +import dagger.Binds; +import dagger.BindsInstance; +import dagger.Module; +import dagger.Provides; +import dagger.Subcomponent; +import dagger.multibindings.IntoMap; +import org.cryptomator.common.vaults.Vault; +import org.cryptomator.ui.common.FxController; +import org.cryptomator.ui.common.FxControllerKey; +import org.cryptomator.ui.common.FxmlFile; +import org.cryptomator.ui.unlock.UnlockInvalidMountPointController; +import org.cryptomator.ui.unlock.UnlockScoped; + +@Subcomponent(modules = {ErrorModule.class, InvalidMountPointExceptionComponent.InvalidMountPointExceptionModule.class}) +@UnlockScoped +public interface InvalidMountPointExceptionComponent extends ErrorComponentBase { + + @Subcomponent.Builder + interface Builder extends BuilderBase { + + @BindsInstance + Builder vault(@ErrorReport Vault vault); + + } + + @Module + abstract class InvalidMountPointExceptionModule { + + @Provides + @ErrorReport + static FxmlFile provideFxmlFile() { + return FxmlFile.UNLOCK_INVALID_MOUNT_POINT; + } + + @Binds + @IntoMap + @FxControllerKey(UnlockInvalidMountPointController.class) + abstract FxController bindController(UnlockInvalidMountPointController controller); + } +} \ No newline at end of file diff --git a/main/ui/src/main/java/org/cryptomator/ui/fxapp/FxApplicationModule.java b/main/ui/src/main/java/org/cryptomator/ui/fxapp/FxApplicationModule.java index aa939354a..951215a08 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/fxapp/FxApplicationModule.java +++ b/main/ui/src/main/java/org/cryptomator/ui/fxapp/FxApplicationModule.java @@ -9,8 +9,9 @@ import dagger.Binds; import dagger.Module; import dagger.Provides; import org.apache.commons.lang3.SystemUtils; -import org.cryptomator.ui.error.GenericErrorComponent; import org.cryptomator.ui.common.StageFactory; +import org.cryptomator.ui.error.GenericErrorComponent; +import org.cryptomator.ui.error.InvalidMountPointExceptionComponent; import org.cryptomator.ui.lock.LockComponent; import org.cryptomator.ui.mainwindow.MainWindowComponent; import org.cryptomator.ui.preferences.PreferencesComponent; @@ -26,8 +27,8 @@ import java.io.UncheckedIOException; import java.util.Collections; import java.util.List; -@Module(includes = {UpdateCheckerModule.class}, subcomponents = {MainWindowComponent.class, PreferencesComponent.class, UnlockComponent.class, LockComponent.class, QuitComponent.class, GenericErrorComponent.class}) -abstract class FxApplicationModule { +@Module(includes = {UpdateCheckerModule.class}, subcomponents = {MainWindowComponent.class, PreferencesComponent.class, UnlockComponent.class, LockComponent.class, QuitComponent.class, GenericErrorComponent.class, InvalidMountPointExceptionComponent.class}) +public abstract class FxApplicationModule { @Provides @Named("windowIcons") diff --git a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java index 1850953c9..986cd1e08 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockInvalidMountPointController.java @@ -1,49 +1,38 @@ package org.cryptomator.ui.unlock; -import dagger.Lazy; import org.cryptomator.common.vaults.MountPointRequirement; import org.cryptomator.common.vaults.Vault; -import org.cryptomator.ui.common.FxController; -import org.cryptomator.ui.common.FxmlFile; -import org.cryptomator.ui.common.FxmlScene; +import org.cryptomator.ui.error.AbstractErrorController; +import org.cryptomator.ui.error.ErrorReport; +import javax.annotation.Nullable; import javax.inject.Inject; -import javafx.fxml.FXML; import javafx.scene.Scene; import javafx.stage.Stage; //At the current point in time only the CustomMountPointChooser may cause this window to be shown. @UnlockScoped -public class UnlockInvalidMountPointController implements FxController { +public class UnlockInvalidMountPointController extends AbstractErrorController { - private final Stage window; - private final Lazy unlockScene; private final Vault vault; @Inject - UnlockInvalidMountPointController(@UnlockWindow Stage window, @FxmlScene(FxmlFile.UNLOCK) Lazy unlockScene, @UnlockWindow Vault vault) { - this.window = window; - this.unlockScene = unlockScene; + UnlockInvalidMountPointController(@ErrorReport Stage window, @ErrorReport @Nullable Scene previousScene, @ErrorReport Vault vault) { + super(window, previousScene); this.vault = vault; } - @FXML - public void back() { - window.setScene(unlockScene.get()); - } - /* Getter/Setter */ public String getMountPoint() { - return vault.getVaultSettings().getCustomMountPath().orElse("AUTO"); + return this.vault.getVaultSettings().getCustomMountPath().orElse("AUTO"); } public boolean getMustExist() { - MountPointRequirement requirement = vault.getVolume().orElseThrow(() -> new IllegalStateException("Invalid Mountpoint without a Volume?!")).getMountPointRequirement(); + MountPointRequirement requirement = this.vault.getVolume().orElseThrow(() -> new IllegalStateException("Invalid Mountpoint without a Volume?!")).getMountPointRequirement(); assert requirement != MountPointRequirement.NONE; //An invalid MountPoint with no required MountPoint doesn't seem sensible assert requirement != MountPointRequirement.PARENT_OPT_MOUNT_POINT; //Not implemented anywhere (yet) return requirement == MountPointRequirement.EMPTY_MOUNT_POINT; } - } diff --git a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockModule.java b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockModule.java index 2ed17fdff..da6f7de1f 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockModule.java +++ b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockModule.java @@ -130,10 +130,4 @@ abstract class UnlockModule { @IntoMap @FxControllerKey(UnlockSuccessController.class) abstract FxController bindUnlockSuccessController(UnlockSuccessController controller); - - @Binds - @IntoMap - @FxControllerKey(UnlockInvalidMountPointController.class) - abstract FxController bindUnlockInvalidMountPointController(UnlockInvalidMountPointController controller); - } diff --git a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockScoped.java b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockScoped.java index 4899d2246..aa9a0db7a 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockScoped.java +++ b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockScoped.java @@ -8,6 +8,6 @@ import java.lang.annotation.RetentionPolicy; @Scope @Documented @Retention(RetentionPolicy.RUNTIME) -@interface UnlockScoped { +public @interface UnlockScoped { } diff --git a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java index efac020ea..76ae9db82 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java +++ b/main/ui/src/main/java/org/cryptomator/ui/unlock/UnlockWorkflow.java @@ -10,11 +10,12 @@ import org.cryptomator.common.vaults.Volume.VolumeException; import org.cryptomator.cryptolib.api.InvalidPassphraseException; import org.cryptomator.integrations.keychain.KeychainAccessException; import org.cryptomator.ui.common.Animations; -import org.cryptomator.ui.error.GenericErrorComponent; import org.cryptomator.ui.common.FxmlFile; import org.cryptomator.ui.common.FxmlScene; import org.cryptomator.ui.common.UserInteractionLock; import org.cryptomator.ui.common.VaultService; +import org.cryptomator.ui.error.GenericErrorComponent; +import org.cryptomator.ui.error.InvalidMountPointExceptionComponent; import org.cryptomator.ui.unlock.UnlockModule.PasswordEntry; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -56,11 +57,11 @@ public class UnlockWorkflow extends Task { private final KeychainManager keychain; private final Lazy unlockScene; private final Lazy successScene; - private final Lazy invalidMountPointScene; private final GenericErrorComponent.Builder genericErrorBuilder; + private final InvalidMountPointExceptionComponent.Builder invalidMountPointExceptionBuilder; @Inject - UnlockWorkflow(@UnlockWindow Stage window, @UnlockWindow Vault vault, VaultService vaultService, AtomicReference password, @Named("savePassword") AtomicBoolean savePassword, @Named("savedPassword") Optional savedPassword, UserInteractionLock passwordEntryLock, KeychainManager keychain, @FxmlScene(FxmlFile.UNLOCK) Lazy unlockScene, @FxmlScene(FxmlFile.UNLOCK_SUCCESS) Lazy successScene, @FxmlScene(FxmlFile.UNLOCK_INVALID_MOUNT_POINT) Lazy invalidMountPointScene, GenericErrorComponent.Builder genericErrorBuilder) { + UnlockWorkflow(@UnlockWindow Stage window, @UnlockWindow Vault vault, VaultService vaultService, AtomicReference password, @Named("savePassword") AtomicBoolean savePassword, @Named("savedPassword") Optional savedPassword, UserInteractionLock passwordEntryLock, KeychainManager keychain, @FxmlScene(FxmlFile.UNLOCK) Lazy unlockScene, @FxmlScene(FxmlFile.UNLOCK_SUCCESS) Lazy successScene, GenericErrorComponent.Builder genericErrorBuilder, InvalidMountPointExceptionComponent.Builder invalidMountPointExceptionBuilder) { this.window = window; this.vault = vault; this.vaultService = vaultService; @@ -71,8 +72,8 @@ public class UnlockWorkflow extends Task { this.keychain = keychain; this.unlockScene = unlockScene; this.successScene = successScene; - this.invalidMountPointScene = invalidMountPointScene; this.genericErrorBuilder = genericErrorBuilder; + this.invalidMountPointExceptionBuilder = invalidMountPointExceptionBuilder; setOnFailed(event -> { Throwable throwable = event.getSource().getException(); @@ -172,26 +173,23 @@ public class UnlockWorkflow extends Task { } else { LOG.error("Unlock failed. Mountpoint doesn't exist (needs to be a folder): {}", cause.getMessage()); } - showInvalidMountPointScene(); + showInvalidMountPointScene(cause); return; } else if (cause instanceof FileAlreadyExistsException) { LOG.error("Unlock failed. Mountpoint already exists: {}", cause.getMessage()); - showInvalidMountPointScene(); + showInvalidMountPointScene(cause); return; } else if (cause instanceof DirectoryNotEmptyException) { LOG.error("Unlock failed. Mountpoint not an empty directory: {}", cause.getMessage()); - showInvalidMountPointScene(); + showInvalidMountPointScene(cause); return; } else { handleGenericError(impExc); } } - private void showInvalidMountPointScene() { - Platform.runLater(() -> { - window.setScene(invalidMountPointScene.get()); - window.show(); - }); + private void showInvalidMountPointScene(Throwable e) { + invalidMountPointExceptionBuilder.cause(e).window(window).returnToScene(window.getScene()).vault(vault).build().showErrorScene(); } private void handleGenericError(Throwable e) { diff --git a/main/ui/src/main/resources/fxml/generic_error.fxml b/main/ui/src/main/resources/fxml/generic_error.fxml index cfc745e1e..6dda01183 100644 --- a/main/ui/src/main/resources/fxml/generic_error.fxml +++ b/main/ui/src/main/resources/fxml/generic_error.fxml @@ -36,7 +36,7 @@ -