From def70c58910c8de4381870b987366902d3993336 Mon Sep 17 00:00:00 2001 From: Tillmann Gaida Date: Sat, 14 Feb 2015 14:08:33 +0100 Subject: [PATCH] Removed static resources in WebDavServer, FXThreads and Settings with dependency injection. Replaced static references to MainApplication in the context of closing resources with an injected DeferredCloser. Using controller factory for dependency injection into FX controllers. --- .../org/cryptomator/webdav/WebDavServer.java | 7 +- main/pom.xml | 7 - main/ui/pom.xml | 7 + .../org/cryptomator/ui/MainApplication.java | 74 ++++++++--- .../org/cryptomator/ui/MainController.java | 17 ++- .../java/org/cryptomator/ui/MainModule.java | 68 ++++++++++ .../org/cryptomator/ui/UnlockController.java | 27 +++- .../cryptomator/ui/UnlockedController.java | 12 +- .../java/org/cryptomator/ui/model/Vault.java | 63 +++------ .../org/cryptomator/ui/settings/Settings.java | 39 +++--- .../cryptomator/ui/util/DeferredClosable.java | 43 ++++++ .../cryptomator/ui/util/DeferredCloser.java | 123 ++++++++++++++++++ .../org/cryptomator/ui/util/FXThreads.java | 81 +++--------- .../ui/util/SingleInstanceManager.java | 4 +- .../cryptomator/ui/MainApplicationTest.java | 12 ++ .../ui/util/DeferredCloserTest.java | 48 +++++++ 16 files changed, 465 insertions(+), 167 deletions(-) create mode 100644 main/ui/src/main/java/org/cryptomator/ui/MainModule.java create mode 100644 main/ui/src/main/java/org/cryptomator/ui/util/DeferredClosable.java create mode 100644 main/ui/src/main/java/org/cryptomator/ui/util/DeferredCloser.java create mode 100644 main/ui/src/test/java/org/cryptomator/ui/MainApplicationTest.java create mode 100644 main/ui/src/test/java/org/cryptomator/ui/util/DeferredCloserTest.java diff --git a/main/core/src/main/java/org/cryptomator/webdav/WebDavServer.java b/main/core/src/main/java/org/cryptomator/webdav/WebDavServer.java index cabfb0b38..a648fcf5b 100644 --- a/main/core/src/main/java/org/cryptomator/webdav/WebDavServer.java +++ b/main/core/src/main/java/org/cryptomator/webdav/WebDavServer.java @@ -38,16 +38,11 @@ public final class WebDavServer { private static final int MAX_THREADS = 200; private static final int MIN_THREADS = 4; private static final int THREAD_IDLE_SECONDS = 20; - private static final WebDavServer INSTANCE = new WebDavServer(); private final Server server; private final ServerConnector localConnector; private final ContextHandlerCollection servletCollection; - public static WebDavServer getInstance() { - return INSTANCE; - } - - private WebDavServer() { + public WebDavServer() { final BlockingQueue queue = new LinkedBlockingQueue<>(MAX_PENDING_REQUESTS); final ThreadPool tp = new QueuedThreadPool(MAX_THREADS, MIN_THREADS, THREAD_IDLE_SECONDS, queue); server = new Server(tp); diff --git a/main/pom.xml b/main/pom.xml index 7a2228307..5ce5d04e0 100644 --- a/main/pom.xml +++ b/main/pom.xml @@ -34,7 +34,6 @@ 1.10 2.4.4 1.10.19 - 2.2.3 @@ -126,12 +125,6 @@ ${mockito.version} test - - - com.github.axet - desktop - ${axetDesktop.version} - diff --git a/main/ui/pom.xml b/main/ui/pom.xml index 9200379f4..fbcb9671b 100644 --- a/main/ui/pom.xml +++ b/main/ui/pom.xml @@ -60,7 +60,14 @@ com.github.axet desktop + 2.2.3 --> + + + com.google.inject + guice + 3.0 + diff --git a/main/ui/src/main/java/org/cryptomator/ui/MainApplication.java b/main/ui/src/main/java/org/cryptomator/ui/MainApplication.java index 5ac1229d4..b4d62bfa5 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/MainApplication.java +++ b/main/ui/src/main/java/org/cryptomator/ui/MainApplication.java @@ -14,7 +14,6 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.ResourceBundle; import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; import javafx.application.Application; import javafx.application.Platform; @@ -25,14 +24,16 @@ import javafx.stage.Stage; import org.apache.commons.lang3.SystemUtils; import org.cryptomator.ui.model.Vault; -import org.cryptomator.ui.settings.Settings; +import org.cryptomator.ui.MainModule.ControllerFactory; import org.cryptomator.ui.util.ActiveWindowStyleSupport; +import org.cryptomator.ui.util.DeferredCloser; import org.cryptomator.ui.util.SingleInstanceManager; import org.cryptomator.ui.util.SingleInstanceManager.LocalInstance; import org.cryptomator.ui.util.TrayIconUtil; -import org.cryptomator.webdav.WebDavServer; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.google.inject.Guice; +import com.google.inject.Injector; public class MainApplication extends Application { @@ -40,7 +41,38 @@ public class MainApplication extends Application { private static final Logger LOG = LoggerFactory.getLogger(MainApplication.class); - private ExecutorService executorService; + private final CleanShutdownPerformer cleanShutdownPerformer = new CleanShutdownPerformer(); + + private final ExecutorService executorService; + + private final ControllerFactory controllerFactory; + + private final DeferredCloser closer; + + public MainApplication() { + this(getInjector()); + } + + private static Injector getInjector() { + try { + return Guice.createInjector(new MainModule()); + } catch (Exception e) { + throw e; + } + } + + public MainApplication(Injector injector) { + this(injector.getInstance(ExecutorService.class), + injector.getInstance(ControllerFactory.class), + injector.getInstance(DeferredCloser.class)); + } + + public MainApplication(ExecutorService executorService, ControllerFactory controllerFactory, DeferredCloser closer) { + super(); + this.executorService = executorService; + this.controllerFactory = controllerFactory; + this.closer = closer; + } @Override public void start(final Stage primaryStage) throws IOException { @@ -55,12 +87,12 @@ public class MainApplication extends Application { } }); - executorService = Executors.newCachedThreadPool(); + Runtime.getRuntime().addShutdownHook(cleanShutdownPerformer); - WebDavServer.getInstance().start(); chooseNativeStylesheet(); final ResourceBundle rb = ResourceBundle.getBundle("localization"); final FXMLLoader loader = new FXMLLoader(getClass().getResource("/fxml/main.fxml"), rb); + loader.setControllerFactory(controllerFactory); final Parent root = loader.load(); final MainController ctrl = loader.getController(); ctrl.setStage(primaryStage); @@ -83,14 +115,10 @@ public class MainApplication extends Application { Main.OPEN_FILE_HANDLER.complete(file -> handleCommandLineArg(ctrl, file.getAbsolutePath())); } - final LocalInstance cryptomatorGuiInstance = SingleInstanceManager.startLocalInstance(APPLICATION_KEY, executorService); - cryptomatorGuiInstance.registerListener(arg -> handleCommandLineArg(ctrl, arg)); + LocalInstance cryptomatorGuiInstance = closer.closeLater( + SingleInstanceManager.startLocalInstance(APPLICATION_KEY, executorService), LocalInstance::close).get().get(); - Main.addShutdownTask(() -> { - cryptomatorGuiInstance.close(); - Settings.save(); - executorService.shutdown(); - }); + cryptomatorGuiInstance.registerListener(arg -> handleCommandLineArg(ctrl, arg)); } void handleCommandLineArg(final MainController ctrl, String arg) { @@ -131,11 +159,27 @@ public class MainApplication extends Application { private void quit() { Platform.runLater(() -> { - WebDavServer.getInstance().stop(); - Settings.save(); + stop(); Platform.exit(); System.exit(0); }); } + @Override + public void stop() { + closer.close(); + try { + Runtime.getRuntime().removeShutdownHook(cleanShutdownPerformer); + } catch (Exception e) { + + } + } + + private class CleanShutdownPerformer extends Thread { + @Override + public void run() { + closer.close(); + } + } + } diff --git a/main/ui/src/main/java/org/cryptomator/ui/MainController.java b/main/ui/src/main/java/org/cryptomator/ui/MainController.java index 38bd7dc85..087f3d169 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/MainController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/MainController.java @@ -39,6 +39,7 @@ import javafx.stage.Stage; import javafx.stage.WindowEvent; import org.cryptomator.ui.InitializeController.InitializationListener; +import org.cryptomator.ui.MainModule.ControllerFactory; import org.cryptomator.ui.UnlockController.UnlockListener; import org.cryptomator.ui.UnlockedController.LockListener; import org.cryptomator.ui.controls.DirectoryListCell; @@ -47,6 +48,8 @@ import org.cryptomator.ui.settings.Settings; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.google.inject.Inject; + public class MainController implements Initializable, InitializationListener, UnlockListener, LockListener { private static final Logger LOG = LoggerFactory.getLogger(MainController.class); @@ -73,11 +76,22 @@ public class MainController implements Initializable, InitializationListener, Un private ResourceBundle rb; + private final ControllerFactory controllerFactory; + + private final Settings settings; + + @Inject + public MainController(ControllerFactory controllerFactory, Settings settings) { + super(); + this.controllerFactory = controllerFactory; + this.settings = settings; + } + @Override public void initialize(URL url, ResourceBundle rb) { this.rb = rb; - final ObservableList items = FXCollections.observableList(Settings.load().getDirectories()); + final ObservableList items = FXCollections.observableList(settings.getDirectories()); directoryList.setItems(items); directoryList.setCellFactory(this::createDirecoryListCell); directoryList.getSelectionModel().getSelectedItems().addListener(this::selectedDirectoryDidChange); @@ -202,6 +216,7 @@ public class MainController implements Initializable, InitializationListener, Un private T showView(String fxml) { try { final FXMLLoader loader = new FXMLLoader(getClass().getResource(fxml), rb); + loader.setControllerFactory(controllerFactory); final Parent root = loader.load(); contentPane.getChildren().clear(); contentPane.getChildren().add(root); diff --git a/main/ui/src/main/java/org/cryptomator/ui/MainModule.java b/main/ui/src/main/java/org/cryptomator/ui/MainModule.java new file mode 100644 index 000000000..06f294136 --- /dev/null +++ b/main/ui/src/main/java/org/cryptomator/ui/MainModule.java @@ -0,0 +1,68 @@ +/******************************************************************************* + * Copyright (c) 2014 cryptomator.org + * This file is licensed under the terms of the MIT license. + * See the LICENSE.txt file for more info. + * + * Contributors: + * Tillmann Gaida - initial implementation + ******************************************************************************/ +package org.cryptomator.ui; + +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; + +import javax.inject.Singleton; + +import org.cryptomator.ui.settings.Settings; +import org.cryptomator.ui.util.DeferredCloser; +import org.cryptomator.ui.util.DeferredCloser.Closer; +import org.cryptomator.webdav.WebDavServer; + +import javafx.util.Callback; + +import com.google.inject.AbstractModule; +import com.google.inject.Injector; +import com.google.inject.Provides; + +public class MainModule extends AbstractModule { + DeferredCloser deferredCloser = new DeferredCloser(); + + public static interface ControllerFactory extends Callback, Object> { + + } + + @Override + protected void configure() { + bind(DeferredCloser.class).toInstance(deferredCloser); + } + + @Provides + @Singleton + ControllerFactory getControllerFactory(Injector injector) { + return cls -> injector.getInstance(cls); + } + + @Provides + @Singleton + ExecutorService getExec() { + return closeLater(Executors.newCachedThreadPool(), ExecutorService::shutdown); + } + + @Provides + @Singleton + Settings getSettings() { + return closeLater(Settings.load(), Settings::save); + } + + @Provides + @Singleton + WebDavServer getServer() { + final WebDavServer webDavServer = new WebDavServer(); + webDavServer.start(); + return closeLater(webDavServer, WebDavServer::stop); + } + + T closeLater(T object, Closer closer) { + return deferredCloser.closeLater(object, closer).get().get(); + } +} diff --git a/main/ui/src/main/java/org/cryptomator/ui/UnlockController.java b/main/ui/src/main/java/org/cryptomator/ui/UnlockController.java index 7054a4d44..3d37b0b33 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/UnlockController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/UnlockController.java @@ -16,6 +16,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardOpenOption; import java.util.ResourceBundle; +import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; import javafx.application.Platform; @@ -40,9 +41,13 @@ import org.cryptomator.ui.controls.SecPasswordField; import org.cryptomator.ui.model.Vault; import org.cryptomator.ui.util.FXThreads; import org.cryptomator.ui.util.MasterKeyFilter; +import org.cryptomator.ui.util.DeferredCloser; +import org.cryptomator.webdav.WebDavServer; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import com.google.inject.Inject; + public class UnlockController implements Initializable { private static final Logger LOG = LoggerFactory.getLogger(UnlockController.class); @@ -72,6 +77,20 @@ public class UnlockController implements Initializable { @FXML private Label messageLabel; + private final WebDavServer server; + + private final ExecutorService exec; + + private final DeferredCloser closer; + + @Inject + public UnlockController(WebDavServer server, ExecutorService exec, DeferredCloser closer) { + super(); + this.server = server; + this.exec = exec; + this.closer = closer; + } + @Override public void initialize(URL url, ResourceBundle rb) { this.rb = rb; @@ -107,15 +126,15 @@ public class UnlockController implements Initializable { masterKeyInputStream = Files.newInputStream(masterKeyPath, StandardOpenOption.READ); directory.setVerifyFileIntegrity(checkIntegrity.isSelected()); directory.getCryptor().decryptMasterKey(masterKeyInputStream, password); - if (!directory.startServer()) { + if (!directory.startServer(server, closer)) { messageLabel.setText(rb.getString("unlock.messageLabel.startServerFailed")); directory.getCryptor().swipeSensitiveData(); return; } directory.setUnlocked(true); - final Future futureMount = FXThreads.runOnBackgroundThread(directory::mount); - FXThreads.runOnMainThreadWhenFinished(futureMount, this::didUnlockAndMount); - FXThreads.runOnMainThreadWhenFinished(futureMount, (result) -> { + final Future futureMount = exec.submit(() -> directory.mount(closer)); + FXThreads.runOnMainThreadWhenFinished(exec, futureMount, this::didUnlockAndMount); + FXThreads.runOnMainThreadWhenFinished(exec, futureMount, (result) -> { setControlsDisabled(false); }); } catch (DecryptFailedException | IOException ex) { diff --git a/main/ui/src/main/java/org/cryptomator/ui/UnlockedController.java b/main/ui/src/main/java/org/cryptomator/ui/UnlockedController.java index b562cc77e..f7369778d 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/UnlockedController.java +++ b/main/ui/src/main/java/org/cryptomator/ui/UnlockedController.java @@ -29,6 +29,8 @@ import org.cryptomator.crypto.CryptorIOSampling; import org.cryptomator.ui.model.Vault; import org.cryptomator.webdav.WebDavServer; +import com.google.inject.Inject; + public class UnlockedController implements Initializable { private static final int IO_SAMPLING_STEPS = 100; @@ -47,6 +49,14 @@ public class UnlockedController implements Initializable { @FXML private NumberAxis xAxis; + private final WebDavServer server; + + @Inject + public UnlockedController(WebDavServer server) { + super(); + this.server = server; + } + @Override public void initialize(URL url, ResourceBundle rb) { this.rb = rb; @@ -124,7 +134,7 @@ public class UnlockedController implements Initializable { public void setDirectory(Vault directory) { this.directory = directory; - final String msg = String.format(rb.getString("unlocked.messageLabel.runningOnPort"), WebDavServer.getInstance().getPort()); + final String msg = String.format(rb.getString("unlocked.messageLabel.runningOnPort"), server.getPort()); messageLabel.setText(msg); if (directory.getCryptor() instanceof CryptorIOSampling) { diff --git a/main/ui/src/main/java/org/cryptomator/ui/model/Vault.java b/main/ui/src/main/java/org/cryptomator/ui/model/Vault.java index 89d08ef54..3a6186cad 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/model/Vault.java +++ b/main/ui/src/main/java/org/cryptomator/ui/model/Vault.java @@ -6,6 +6,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.text.Normalizer; import java.text.Normalizer.Form; +import java.util.Optional; import javafx.beans.property.ObjectProperty; import javafx.beans.property.SimpleObjectProperty; @@ -14,8 +15,9 @@ import org.apache.commons.lang3.StringUtils; import org.cryptomator.crypto.Cryptor; import org.cryptomator.crypto.SamplingDecorator; import org.cryptomator.crypto.aes256.Aes256Cryptor; -import org.cryptomator.ui.Main; import org.cryptomator.ui.util.MasterKeyFilter; +import org.cryptomator.ui.util.DeferredClosable; +import org.cryptomator.ui.util.DeferredCloser; import org.cryptomator.ui.util.mount.CommandFailedException; import org.cryptomator.ui.util.mount.WebDavMount; import org.cryptomator.ui.util.mount.WebDavMounter; @@ -38,12 +40,11 @@ public class Vault implements Serializable { private final Cryptor cryptor = SamplingDecorator.decorate(new Aes256Cryptor()); private final ObjectProperty unlocked = new SimpleObjectProperty(this, "unlocked", Boolean.FALSE); - private final Runnable shutdownTask = new ShutdownTask(); private final Path path; private boolean verifyFileIntegrity; private String mountName; - private ServletLifeCycleAdapter webDavServlet; - private WebDavMount webDavMount; + private DeferredClosable webDavServlet = DeferredClosable.empty(); + private DeferredClosable webDavMount = DeferredClosable.empty(); public Vault(final Path vaultDirectoryPath) { if (!Files.isDirectory(vaultDirectoryPath) || !vaultDirectoryPath.getFileName().toString().endsWith(VAULT_FILE_EXTENSION)) { @@ -62,34 +63,32 @@ public class Vault implements Serializable { return MasterKeyFilter.filteredDirectory(path).iterator().hasNext(); } - public synchronized boolean startServer() { - if (webDavServlet != null && webDavServlet.isRunning()) { + public synchronized boolean startServer(WebDavServer server, DeferredCloser closer) { + Optional o = webDavServlet.get(); + if (o.isPresent() && o.get().isRunning()) { return false; } - webDavServlet = WebDavServer.getInstance().createServlet(path, verifyFileIntegrity, cryptor, getMountName()); - if (webDavServlet.start()) { - Main.addShutdownTask(shutdownTask); + ServletLifeCycleAdapter servlet = server.createServlet(path, verifyFileIntegrity, cryptor, getMountName()); + if (servlet.start()) { + webDavServlet = closer.closeLater(servlet, ServletLifeCycleAdapter::stop); return true; - } else { - return false; } + return false; } public void stopServer() { - if (webDavServlet != null && webDavServlet.isRunning()) { - Main.removeShutdownTask(shutdownTask); - this.unmount(); - webDavServlet.stop(); - cryptor.swipeSensitiveData(); - } + unmount(); + webDavServlet.close(); + cryptor.swipeSensitiveData(); } - public boolean mount() { - if (webDavServlet == null || !webDavServlet.isRunning()) { + public boolean mount(DeferredCloser closer) { + Optional o = webDavServlet.get(); + if (!o.isPresent() || !o.get().isRunning()) { return false; } try { - webDavMount = WebDavMounter.mount(webDavServlet.getServletUri(), getMountName()); + webDavMount = closer.closeLater(WebDavMounter.mount(o.get().getServletUri(), getMountName()), WebDavMount::unmount); return true; } catch (CommandFailedException e) { LOG.warn("mount failed", e); @@ -97,17 +96,8 @@ public class Vault implements Serializable { } } - public boolean unmount() { - try { - if (webDavMount != null) { - webDavMount.unmount(); - webDavMount = null; - } - return true; - } catch (CommandFailedException e) { - LOG.warn("unmount failed", e); - return false; - } + public void unmount() { + webDavMount.close(); } /* Getter/Setter */ @@ -208,15 +198,4 @@ public class Vault implements Serializable { } } - /* graceful shutdown */ - - private class ShutdownTask implements Runnable { - - @Override - public void run() { - stopServer(); - } - - } - } diff --git a/main/ui/src/main/java/org/cryptomator/ui/settings/Settings.java b/main/ui/src/main/java/org/cryptomator/ui/settings/Settings.java index fd5e5b845..30365ec04 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/settings/Settings.java +++ b/main/ui/src/main/java/org/cryptomator/ui/settings/Settings.java @@ -36,7 +36,6 @@ public class Settings implements Serializable { private static final Path SETTINGS_DIR; private static final String SETTINGS_FILE = "settings.json"; private static final ObjectMapper JSON_OM = new ObjectMapper(); - private static Settings INSTANCE = null; static { final String appdata = System.getenv("APPDATA"); @@ -61,31 +60,25 @@ public class Settings implements Serializable { } public static synchronized Settings load() { - if (INSTANCE == null) { - try { - Files.createDirectories(SETTINGS_DIR); - final Path settingsFile = SETTINGS_DIR.resolve(SETTINGS_FILE); - final InputStream in = Files.newInputStream(settingsFile, StandardOpenOption.READ); - INSTANCE = JSON_OM.readValue(in, Settings.class); - return INSTANCE; - } catch (IOException e) { - LOG.warn("Failed to load settings, creating new one."); - INSTANCE = Settings.defaultSettings(); - } + try { + Files.createDirectories(SETTINGS_DIR); + final Path settingsFile = SETTINGS_DIR.resolve(SETTINGS_FILE); + final InputStream in = Files.newInputStream(settingsFile, StandardOpenOption.READ); + return JSON_OM.readValue(in, Settings.class); + } catch (IOException e) { + LOG.warn("Failed to load settings, creating new one."); + return Settings.defaultSettings(); } - return INSTANCE; } - public static synchronized void save() { - if (INSTANCE != null) { - try { - Files.createDirectories(SETTINGS_DIR); - final Path settingsFile = SETTINGS_DIR.resolve(SETTINGS_FILE); - final OutputStream out = Files.newOutputStream(settingsFile, StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.CREATE); - JSON_OM.writeValue(out, INSTANCE); - } catch (IOException e) { - LOG.error("Failed to save settings.", e); - } + public synchronized void save() { + try { + Files.createDirectories(SETTINGS_DIR); + final Path settingsFile = SETTINGS_DIR.resolve(SETTINGS_FILE); + final OutputStream out = Files.newOutputStream(settingsFile, StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.CREATE); + JSON_OM.writeValue(out, this); + } catch (IOException e) { + LOG.error("Failed to save settings.", e); } } diff --git a/main/ui/src/main/java/org/cryptomator/ui/util/DeferredClosable.java b/main/ui/src/main/java/org/cryptomator/ui/util/DeferredClosable.java new file mode 100644 index 000000000..55f3f516a --- /dev/null +++ b/main/ui/src/main/java/org/cryptomator/ui/util/DeferredClosable.java @@ -0,0 +1,43 @@ +/******************************************************************************* + * Copyright (c) 2014 cryptomator.org + * This file is licensed under the terms of the MIT license. + * See the LICENSE.txt file for more info. + * + * Contributors: + * Tillmann Gaida - initial implementation + ******************************************************************************/ +package org.cryptomator.ui.util; + +import java.util.Optional; + +/** + * Wrapper around an object, which should be closed later - explicitly or by a + * {@link DeferredCloser}. The wrapped object can be accessed as long as the + * resource has not been closed. + * + * @author Tillmann Gaida + * + * @param + * any type + */ +public interface DeferredClosable extends AutoCloseable { + /** + * Returns the wrapped Object. + * + * @return empty if the object has been closed. + */ + public Optional get(); + + /** + * Quietly closes the Object. If the object was closed before, nothing + * happens. + */ + public void close(); + + /** + * @return an empty object. + */ + public static DeferredClosable empty() { + return DeferredCloser.empty(); + } +} \ No newline at end of file diff --git a/main/ui/src/main/java/org/cryptomator/ui/util/DeferredCloser.java b/main/ui/src/main/java/org/cryptomator/ui/util/DeferredCloser.java new file mode 100644 index 000000000..ef0df455a --- /dev/null +++ b/main/ui/src/main/java/org/cryptomator/ui/util/DeferredCloser.java @@ -0,0 +1,123 @@ +/******************************************************************************* + * Copyright (c) 2014 cryptomator.org + * This file is licensed under the terms of the MIT license. + * See the LICENSE.txt file for more info. + * + * Contributors: + * Tillmann Gaida - initial implementation + ******************************************************************************/ +package org.cryptomator.ui.util; + +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.concurrent.ConcurrentSkipListMap; +import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.atomic.AtomicReference; + +import org.cryptomator.ui.MainController; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + *

+ * Tries to bring open-close symmetry in contexts where the resource outlives + * the current scope by introducing a manager, which closes the resources if + * they haven't been closed before. + *

+ * + *

+ * If you have a {@link DeferredCloser} instance present, call + * {@link #closeLater(Object, Closer)} immediately after you have opened the + * resource and return a resource handle. If {@link #close()} is called, the + * resource will be closed. Calling {@link DeferredClosable#close()} on the resource + * handle will also close the resource and prevent a second closing by + * {@link #close()}. + *

+ * + * @author Tillmann Gaida + */ +public class DeferredCloser implements AutoCloseable { + public static interface Closer { + void close(T object) throws Exception; + } + + static class EmptyResource implements DeferredClosable { + @Override + public Optional get() { + return Optional.empty(); + } + + @Override + public void close() { + + } + } + + private static final Logger LOG = LoggerFactory.getLogger(MainController.class); + + final Map> cleanups = new ConcurrentSkipListMap<>(); + + final AtomicLong counter = new AtomicLong(); + + public class ManagedResource implements DeferredClosable { + private final long number = counter.incrementAndGet(); + + private final AtomicReference object = new AtomicReference<>(); + private final Closer closer; + + ManagedResource(T object, Closer closer) { + super(); + this.object.set(object); + this.closer = closer; + } + + public void close() { + final T oldObject = object.getAndSet(null); + if (oldObject != null) { + cleanups.remove(number); + + try { + closer.close(oldObject); + } catch (Exception e) { + LOG.error("exception closing resource", e); + } + } + } + + public Optional get() throws IllegalStateException { + return Optional.ofNullable(object.get()); + } + } + + /** + * Closes all added objects which have not been closed before. + */ + public void close() { + for (ManagedResource closableProvider : cleanups.values()) { + closableProvider.close(); + } + } + + public DeferredClosable closeLater(T object, Closer closer) { + Objects.requireNonNull(object); + Objects.requireNonNull(closer); + final ManagedResource resource = new ManagedResource(object, closer); + cleanups.put(resource.number, resource); + return resource; + } + + public DeferredClosable closeLater(T object) { + Objects.requireNonNull(object); + final ManagedResource resource = new ManagedResource(object, AutoCloseable::close); + cleanups.put(resource.number, resource); + return resource; + } + + private static final EmptyResource EMPTY_RESOURCE = new EmptyResource<>(); + + @SuppressWarnings("unchecked") + public static DeferredClosable empty() { + return (DeferredClosable) EMPTY_RESOURCE; + } +} diff --git a/main/ui/src/main/java/org/cryptomator/ui/util/FXThreads.java b/main/ui/src/main/java/org/cryptomator/ui/util/FXThreads.java index 86a5a18dd..48a305044 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/util/FXThreads.java +++ b/main/ui/src/main/java/org/cryptomator/ui/util/FXThreads.java @@ -10,9 +10,8 @@ ******************************************************************************/ package org.cryptomator.ui.util; -import java.util.concurrent.Callable; +import java.util.Objects; import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; import java.util.concurrent.Future; import javafx.application.Platform; @@ -48,61 +47,14 @@ import javafx.application.Platform; */ public final class FXThreads { - private static final ExecutorService EXECUTOR = Executors.newCachedThreadPool(); private static final CallbackWhenTaskFailed DUMMY_EXCEPTION_CALLBACK = (e) -> { // ignore. }; - private FXThreads() { - throw new AssertionError("Not instantiable."); - } - - /** - * Executes the given task on a background thread. If you want to react on the result on your JavaFX main thread, use - * {@link #runOnMainThreadWhenFinished(Future, CallbackWhenTaskFinished)}. - * - *
-	 * // examples:
-	 * 
-	 * Future<String> futureBookName1 = runOnBackgroundThread(restResource::getBookName);
-	 * 
-	 * Future<String> futureBookName2 = runOnBackgroundThread(() -> {
-	 * 	return restResource.getBookName();
-	 * });
-	 * 
- * - * @param task The task to be executed on a background thread. - * @return A future result object, which you can use in {@link #runOnMainThreadWhenFinished(Future, CallbackWhenTaskFinished)}. - */ - public static Future runOnBackgroundThread(Callable task) { - return EXECUTOR.submit(task); - } - - /** - * Executes the given task on a background thread. If you want to react on the result on your JavaFX main thread, use - * {@link #runOnMainThreadWhenFinished(Future, CallbackWhenTaskFinished)}. - * - *
-	 * // examples:
-	 * 
-	 * Future<?> futureDone1 = runOnBackgroundThread(this::doSomeComplexCalculation);
-	 * 
-	 * Future<?> futureDone2 = runOnBackgroundThread(() -> {
-	 * 	doSomeComplexCalculation();
-	 * });
-	 * 
- * - * @param task The task to be executed on a background thread. - * @return A future result object, which you can use in {@link #runOnMainThreadWhenFinished(Future, CallbackWhenTaskFinished)}. - */ - public static Future runOnBackgroundThread(Runnable task) { - return EXECUTOR.submit(task); - } - /** * Waits for the given task to complete and notifies the given successCallback. If an exception occurs, the callback will never be * called. If you are interested in the exception, use - * {@link #runOnMainThreadWhenFinished(Future, CallbackWhenTaskFinished, CallbackWhenTaskFailed)} instead. + * {@link #runOnMainThreadWhenFinished(ExecutorService, Future, CallbackWhenTaskFinished, CallbackWhenTaskFailed)} instead. * *
 	 * // example:
@@ -111,21 +63,21 @@ public final class FXThreads {
 	 * 	myLabel.setText(bookName);
 	 * });
 	 * 
- * + * @param executor * @param task The task to wait for. * @param successCallback The action to perform, when the task finished. */ - public static void runOnMainThreadWhenFinished(Future task, CallbackWhenTaskFinished successCallback) { - runOnBackgroundThread(() -> { + public static void runOnMainThreadWhenFinished(ExecutorService executor, Future task, CallbackWhenTaskFinished successCallback) { + executor.submit(() -> { return "asd"; }); - FXThreads.runOnMainThreadWhenFinished(task, successCallback, DUMMY_EXCEPTION_CALLBACK); + runOnMainThreadWhenFinished(executor, task, successCallback, DUMMY_EXCEPTION_CALLBACK); } /** * Waits for the given task to complete and notifies the given successCallback. If an exception occurs, the callback will never be * called. If you are interested in the exception, use - * {@link #runOnMainThreadWhenFinished(Future, CallbackWhenTaskFinished, CallbackWhenTaskFailed)} instead. + * {@link #runOnMainThreadWhenFinished(ExecutorService, Future, CallbackWhenTaskFinished, CallbackWhenTaskFailed)} instead. * *
 	 * // example:
@@ -137,14 +89,17 @@ public final class FXThreads {
 	 * });
 	 * 
* + * @param executor + * The service to execute the background task on * @param task The task to wait for. * @param successCallback The action to perform, when the task finished. + * @param exceptionCallback */ - public static void runOnMainThreadWhenFinished(Future task, CallbackWhenTaskFinished successCallback, CallbackWhenTaskFailed exceptionCallback) { - assertParamNotNull(task, "task must not be null."); - assertParamNotNull(successCallback, "successCallback must not be null."); - assertParamNotNull(exceptionCallback, "exceptionCallback must not be null."); - EXECUTOR.execute(() -> { + public static void runOnMainThreadWhenFinished(ExecutorService executor, Future task, CallbackWhenTaskFinished successCallback, CallbackWhenTaskFailed exceptionCallback) { + Objects.requireNonNull(task, "task must not be null."); + Objects.requireNonNull(successCallback, "successCallback must not be null."); + Objects.requireNonNull(exceptionCallback, "exceptionCallback must not be null."); + executor.execute(() -> { try { final T result = task.get(); Platform.runLater(() -> { @@ -158,12 +113,6 @@ public final class FXThreads { }); } - private static void assertParamNotNull(Object param, String msg) { - if (param == null) { - throw new IllegalArgumentException(msg); - } - } - public interface CallbackWhenTaskFinished { void taskFinished(T result); } diff --git a/main/ui/src/main/java/org/cryptomator/ui/util/SingleInstanceManager.java b/main/ui/src/main/java/org/cryptomator/ui/util/SingleInstanceManager.java index 9330d9c69..4b687b7ce 100644 --- a/main/ui/src/main/java/org/cryptomator/ui/util/SingleInstanceManager.java +++ b/main/ui/src/main/java/org/cryptomator/ui/util/SingleInstanceManager.java @@ -1,10 +1,10 @@ /******************************************************************************* - * Copyright (c) 2014 Sebastian Stenzel + * Copyright (c) 2014 cryptomator.org * This file is licensed under the terms of the MIT license. * See the LICENSE.txt file for more info. * * Contributors: - * Sebastian Stenzel - initial API and implementation + * Tillmann Gaida - initial implementation ******************************************************************************/ package org.cryptomator.ui.util; diff --git a/main/ui/src/test/java/org/cryptomator/ui/MainApplicationTest.java b/main/ui/src/test/java/org/cryptomator/ui/MainApplicationTest.java new file mode 100644 index 000000000..affbe1c53 --- /dev/null +++ b/main/ui/src/test/java/org/cryptomator/ui/MainApplicationTest.java @@ -0,0 +1,12 @@ +package org.cryptomator.ui; + +import static org.junit.Assert.*; + +import org.junit.Test; + +public class MainApplicationTest { + @Test + public void testInjection() throws Exception { + new MainApplication(); + } +} diff --git a/main/ui/src/test/java/org/cryptomator/ui/util/DeferredCloserTest.java b/main/ui/src/test/java/org/cryptomator/ui/util/DeferredCloserTest.java new file mode 100644 index 000000000..a876fbee8 --- /dev/null +++ b/main/ui/src/test/java/org/cryptomator/ui/util/DeferredCloserTest.java @@ -0,0 +1,48 @@ +package org.cryptomator.ui.util; + +import static org.junit.Assert.*; +import static org.mockito.Mockito.*; + +import java.io.Closeable; + +import org.junit.Test; + +public class DeferredCloserTest { + @Test + public void testBasicFunctionality() throws Exception { + DeferredCloser closer = new DeferredCloser(); + + final Closeable obj = mock(Closeable.class); + + final DeferredClosable resource = closer.closeLater(obj); + + assertTrue(resource.get().isPresent()); + assertTrue(resource.get().get() == obj); + + closer.close(); + + assertFalse(resource.get().isPresent()); + verify(obj).close(); + } + + @Test + public void testAutoremoval() throws Exception { + DeferredCloser closer = new DeferredCloser(); + + final DeferredClosable resource = closer.closeLater(mock(Closeable.class)); + final DeferredClosable resource2 = closer.closeLater(mock(Closeable.class)); + + resource.close(); + + assertFalse(resource.get().isPresent()); + assertEquals(1, closer.cleanups.size()); + + assertTrue(resource2.get().isPresent()); + + closer.close(); + + assertFalse(resource2.get().isPresent()); + + assertEquals(0, closer.cleanups.size()); + } +}