From b480777548fdf77c7598a5b2bb67e03d3155bd45 Mon Sep 17 00:00:00 2001
From: akwizgran <akwizgran@users.sourceforge.net>
Date: Tue, 5 Apr 2016 15:44:50 +0100
Subject: [PATCH] Services should throw exceptions for startup errors.
---
.../AndroidNotificationManagerImpl.java | 62 +++++++++++--------
.../api/db/DatabaseComponent.java | 3 +-
.../briarproject/api/lifecycle/Service.java | 12 ++--
.../api/lifecycle/ServiceException.java | 15 +++++
.../src/org/briarproject/db/Database.java | 3 +-
.../db/DatabaseComponentImpl.java | 6 +-
.../lifecycle/LifecycleManagerImpl.java | 27 ++++----
.../plugins/PluginManagerImpl.java | 15 ++---
.../sync/ValidationManagerImpl.java | 6 +-
.../transport/KeyManagerImpl.java | 11 ++--
.../plugins/PluginManagerImplTest.java | 6 +-
.../sync/ValidationManagerImplTest.java | 6 +-
12 files changed, 87 insertions(+), 85 deletions(-)
create mode 100644 briar-api/src/org/briarproject/api/lifecycle/ServiceException.java
diff --git a/briar-android/src/org/briarproject/android/AndroidNotificationManagerImpl.java b/briar-android/src/org/briarproject/android/AndroidNotificationManagerImpl.java
index 172c998d10..3cd074aea5 100644
--- a/briar-android/src/org/briarproject/android/AndroidNotificationManagerImpl.java
+++ b/briar-android/src/org/briarproject/android/AndroidNotificationManagerImpl.java
@@ -10,10 +10,10 @@ import android.support.v4.app.NotificationCompat;
import android.support.v4.app.TaskStackBuilder;
import org.briarproject.R;
-import org.briarproject.android.contact.ConversationActivity;
-import org.briarproject.android.forum.ForumActivity;
import org.briarproject.android.api.AndroidExecutor;
import org.briarproject.android.api.AndroidNotificationManager;
+import org.briarproject.android.contact.ConversationActivity;
+import org.briarproject.android.forum.ForumActivity;
import org.briarproject.api.db.DatabaseExecutor;
import org.briarproject.api.db.DbException;
import org.briarproject.api.event.Event;
@@ -22,6 +22,7 @@ import org.briarproject.api.event.MessageValidatedEvent;
import org.briarproject.api.event.SettingsUpdatedEvent;
import org.briarproject.api.forum.ForumManager;
import org.briarproject.api.lifecycle.Service;
+import org.briarproject.api.lifecycle.ServiceException;
import org.briarproject.api.messaging.MessagingManager;
import org.briarproject.api.settings.Settings;
import org.briarproject.api.settings.SettingsManager;
@@ -31,7 +32,10 @@ import org.briarproject.util.StringUtils;
import java.util.HashMap;
import java.util.Map;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executor;
+import java.util.concurrent.Future;
import java.util.logging.Logger;
import javax.inject.Inject;
@@ -93,37 +97,30 @@ class AndroidNotificationManagerImpl implements AndroidNotificationManager,
}
@Override
- public boolean start() {
- loadSettings();
- return true;
- }
-
- private void loadSettings() {
- dbExecutor.execute(new Runnable() {
- public void run() {
- try {
- settings = settingsManager.getSettings(SETTINGS_NAMESPACE);
- } catch (DbException e) {
- if (LOG.isLoggable(WARNING))
- LOG.log(WARNING, e.toString(), e);
- }
- }
- });
+ public void startService() throws ServiceException {
+ try {
+ settings = settingsManager.getSettings(SETTINGS_NAMESPACE);
+ } catch (DbException e) {
+ throw new ServiceException(e);
+ }
}
@Override
- public boolean stop() {
- clearNotifications();
- return true;
- }
-
- private void clearNotifications() {
- androidExecutor.execute(new Runnable() {
- public void run() {
+ public void stopService() throws ServiceException {
+ Future<Void> f = androidExecutor.submit(new Callable<Void>() {
+ public Void call() {
clearPrivateMessageNotification();
clearForumPostNotification();
+ return null;
}
});
+ try {
+ f.get();
+ } catch (InterruptedException e) {
+ throw new ServiceException(e);
+ } catch (ExecutionException e) {
+ throw new ServiceException(e);
+ }
}
private void clearPrivateMessageNotification() {
@@ -154,6 +151,19 @@ class AndroidNotificationManagerImpl implements AndroidNotificationManager,
}
}
+ private void loadSettings() {
+ dbExecutor.execute(new Runnable() {
+ public void run() {
+ try {
+ settings = settingsManager.getSettings(SETTINGS_NAMESPACE);
+ } catch (DbException e) {
+ if (LOG.isLoggable(WARNING))
+ LOG.log(WARNING, e.toString(), e);
+ }
+ }
+ });
+ }
+
public void showPrivateMessageNotification(final GroupId g) {
androidExecutor.execute(new Runnable() {
public void run() {
diff --git a/briar-api/src/org/briarproject/api/db/DatabaseComponent.java b/briar-api/src/org/briarproject/api/db/DatabaseComponent.java
index 2a627fb8a2..bf6b9f66cd 100644
--- a/briar-api/src/org/briarproject/api/db/DatabaseComponent.java
+++ b/briar-api/src/org/briarproject/api/db/DatabaseComponent.java
@@ -19,7 +19,6 @@ import org.briarproject.api.sync.Offer;
import org.briarproject.api.sync.Request;
import org.briarproject.api.transport.TransportKeys;
-import java.io.IOException;
import java.util.Collection;
import java.util.Map;
@@ -37,7 +36,7 @@ public interface DatabaseComponent {
/**
* Waits for any open transactions to finish and closes the database.
*/
- void close() throws DbException, IOException;
+ void close() throws DbException;
/**
* Starts a new transaction and returns an object representing it.
diff --git a/briar-api/src/org/briarproject/api/lifecycle/Service.java b/briar-api/src/org/briarproject/api/lifecycle/Service.java
index 3c80e6cf68..5489435a4a 100644
--- a/briar-api/src/org/briarproject/api/lifecycle/Service.java
+++ b/briar-api/src/org/briarproject/api/lifecycle/Service.java
@@ -3,14 +3,14 @@ package org.briarproject.api.lifecycle;
public interface Service {
/**
- * Starts the service and returns true if it started successfully.
- * This method must not be called concurrently with {@link #stop()}.
+ * Starts the service.This method must not be called concurrently with
+ * {@link #stopService()}.
*/
- public boolean start();
+ void startService() throws ServiceException;
/**
- * Stops the service and returns true if it stopped successfully.
- * This method must not be called concurrently with {@link #start()}.
+ * Stops the service. This method must not be called concurrently with
+ * {@link #startService()}.
*/
- public boolean stop();
+ void stopService() throws ServiceException;
}
diff --git a/briar-api/src/org/briarproject/api/lifecycle/ServiceException.java b/briar-api/src/org/briarproject/api/lifecycle/ServiceException.java
new file mode 100644
index 0000000000..55a0b939ab
--- /dev/null
+++ b/briar-api/src/org/briarproject/api/lifecycle/ServiceException.java
@@ -0,0 +1,15 @@
+package org.briarproject.api.lifecycle;
+
+/**
+ * An exception that indicates an error starting or stopping a {@link Service}.
+ */
+public class ServiceException extends Exception {
+
+ public ServiceException() {
+ super();
+ }
+
+ public ServiceException(Throwable cause) {
+ super(cause);
+ }
+}
diff --git a/briar-core/src/org/briarproject/db/Database.java b/briar-core/src/org/briarproject/db/Database.java
index bcd1b7f448..981da4f285 100644
--- a/briar-core/src/org/briarproject/db/Database.java
+++ b/briar-core/src/org/briarproject/db/Database.java
@@ -19,7 +19,6 @@ import org.briarproject.api.sync.MessageStatus;
import org.briarproject.api.sync.ValidationManager.Validity;
import org.briarproject.api.transport.TransportKeys;
-import java.io.IOException;
import java.util.Collection;
import java.util.Map;
@@ -41,7 +40,7 @@ interface Database<T> {
* Prevents new transactions from starting, waits for all current
* transactions to finish, and closes the database.
*/
- void close() throws DbException, IOException;
+ void close() throws DbException;
/**
* Starts a new transaction and returns an object representing it.
diff --git a/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java b/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java
index 79ff343255..bdd7e6691a 100644
--- a/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java
+++ b/briar-core/src/org/briarproject/db/DatabaseComponentImpl.java
@@ -50,7 +50,6 @@ import org.briarproject.api.sync.Request;
import org.briarproject.api.sync.ValidationManager.Validity;
import org.briarproject.api.transport.TransportKeys;
-import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
@@ -101,9 +100,6 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
} catch (DbException e) {
if (LOG.isLoggable(WARNING))
LOG.log(WARNING, e.toString(), e);
- } catch (IOException e) {
- if (LOG.isLoggable(WARNING))
- LOG.log(WARNING, e.toString(), e);
}
}
};
@@ -112,7 +108,7 @@ class DatabaseComponentImpl<T> implements DatabaseComponent {
return reopened;
}
- public void close() throws DbException, IOException {
+ public void close() throws DbException {
if (closed.getAndSet(true)) return;
shutdown.removeShutdownHook(shutdownHandle);
db.close();
diff --git a/briar-core/src/org/briarproject/lifecycle/LifecycleManagerImpl.java b/briar-core/src/org/briarproject/lifecycle/LifecycleManagerImpl.java
index bd2d31765c..0e7810eb56 100644
--- a/briar-core/src/org/briarproject/lifecycle/LifecycleManagerImpl.java
+++ b/briar-core/src/org/briarproject/lifecycle/LifecycleManagerImpl.java
@@ -8,8 +8,8 @@ import org.briarproject.api.event.EventBus;
import org.briarproject.api.event.ShutdownEvent;
import org.briarproject.api.lifecycle.LifecycleManager;
import org.briarproject.api.lifecycle.Service;
+import org.briarproject.api.lifecycle.ServiceException;
-import java.io.IOException;
import java.util.List;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.CountDownLatch;
@@ -101,16 +101,11 @@ class LifecycleManagerImpl implements LifecycleManager {
}
for (Service s : services) {
start = System.currentTimeMillis();
- boolean started = s.start();
+ s.startService();
duration = System.currentTimeMillis() - start;
- if (!started) {
- if (LOG.isLoggable(WARNING))
- LOG.warning(s.getClass().getName() + " did not start");
- return SERVICE_ERROR;
- }
if (LOG.isLoggable(INFO)) {
- String name = s.getClass().getName();
- LOG.info("Starting " + name + " took " + duration + " ms");
+ LOG.info("Starting " + s.getClass().getName()
+ + " took " + duration + " ms");
}
}
startupLatch.countDown();
@@ -118,6 +113,9 @@ class LifecycleManagerImpl implements LifecycleManager {
} catch (DbException e) {
if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
return DB_ERROR;
+ } catch (ServiceException e) {
+ if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
+ return SERVICE_ERROR;
} finally {
startStopSemaphore.release();
}
@@ -134,12 +132,9 @@ class LifecycleManagerImpl implements LifecycleManager {
LOG.info("Stopping services");
eventBus.broadcast(new ShutdownEvent());
for (Service s : services) {
- boolean stopped = s.stop();
- if (LOG.isLoggable(INFO)) {
- String name = s.getClass().getName();
- if (stopped) LOG.info("Service stopped: " + name);
- else LOG.warning("Service failed to stop: " + name);
- }
+ s.stopService();
+ if (LOG.isLoggable(INFO))
+ LOG.info("Service stopped: " + s.getClass().getName());
}
for (ExecutorService e : executors) e.shutdownNow();
if (LOG.isLoggable(INFO))
@@ -149,7 +144,7 @@ class LifecycleManagerImpl implements LifecycleManager {
shutdownLatch.countDown();
} catch (DbException e) {
if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
- } catch (IOException e) {
+ } catch (ServiceException e) {
if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
} finally {
startStopSemaphore.release();
diff --git a/briar-core/src/org/briarproject/plugins/PluginManagerImpl.java b/briar-core/src/org/briarproject/plugins/PluginManagerImpl.java
index ef6e65e19d..9f2f85b978 100644
--- a/briar-core/src/org/briarproject/plugins/PluginManagerImpl.java
+++ b/briar-core/src/org/briarproject/plugins/PluginManagerImpl.java
@@ -8,6 +8,7 @@ import org.briarproject.api.event.TransportDisabledEvent;
import org.briarproject.api.event.TransportEnabledEvent;
import org.briarproject.api.lifecycle.IoExecutor;
import org.briarproject.api.lifecycle.Service;
+import org.briarproject.api.lifecycle.ServiceException;
import org.briarproject.api.plugins.ConnectionManager;
import org.briarproject.api.plugins.Plugin;
import org.briarproject.api.plugins.PluginCallback;
@@ -83,7 +84,7 @@ class PluginManagerImpl implements PluginManager, Service {
}
@Override
- public boolean start() {
+ public void startService() throws ServiceException {
// Instantiate and start the simplex plugins
LOG.info("Starting simplex plugins");
Collection<SimplexPluginFactory> sFactories =
@@ -103,15 +104,12 @@ class PluginManagerImpl implements PluginManager, Service {
sLatch.await();
dLatch.await();
} catch (InterruptedException e) {
- LOG.warning("Interrupted while starting plugins");
- Thread.currentThread().interrupt();
- return false;
+ throw new ServiceException(e);
}
- return true;
}
@Override
- public boolean stop() {
+ public void stopService() throws ServiceException {
// Stop the poller
LOG.info("Stopping poller");
poller.stop();
@@ -131,11 +129,8 @@ class PluginManagerImpl implements PluginManager, Service {
try {
latch.await();
} catch (InterruptedException e) {
- LOG.warning("Interrupted while stopping plugins");
- Thread.currentThread().interrupt();
- return false;
+ throw new ServiceException(e);
}
- return true;
}
public Plugin getPlugin(TransportId t) {
diff --git a/briar-core/src/org/briarproject/sync/ValidationManagerImpl.java b/briar-core/src/org/briarproject/sync/ValidationManagerImpl.java
index 89c35459af..a5d02b5cc4 100644
--- a/briar-core/src/org/briarproject/sync/ValidationManagerImpl.java
+++ b/briar-core/src/org/briarproject/sync/ValidationManagerImpl.java
@@ -57,14 +57,12 @@ class ValidationManagerImpl implements ValidationManager, Service,
}
@Override
- public boolean start() {
+ public void startService() {
for (ClientId c : validators.keySet()) getMessagesToValidate(c);
- return true;
}
@Override
- public boolean stop() {
- return true;
+ public void stopService() {
}
@Override
diff --git a/briar-core/src/org/briarproject/transport/KeyManagerImpl.java b/briar-core/src/org/briarproject/transport/KeyManagerImpl.java
index 32a6ee3aad..21d3b941af 100644
--- a/briar-core/src/org/briarproject/transport/KeyManagerImpl.java
+++ b/briar-core/src/org/briarproject/transport/KeyManagerImpl.java
@@ -14,6 +14,7 @@ import org.briarproject.api.event.ContactStatusChangedEvent;
import org.briarproject.api.event.Event;
import org.briarproject.api.event.EventListener;
import org.briarproject.api.lifecycle.Service;
+import org.briarproject.api.lifecycle.ServiceException;
import org.briarproject.api.plugins.PluginConfig;
import org.briarproject.api.plugins.duplex.DuplexPluginFactory;
import org.briarproject.api.plugins.simplex.SimplexPluginFactory;
@@ -32,7 +33,6 @@ import java.util.logging.Logger;
import javax.inject.Inject;
import static java.util.logging.Level.INFO;
-import static java.util.logging.Level.WARNING;
class KeyManagerImpl implements KeyManager, Service, EventListener {
@@ -64,7 +64,7 @@ class KeyManagerImpl implements KeyManager, Service, EventListener {
}
@Override
- public boolean start() {
+ public void startService() throws ServiceException {
Map<TransportId, Integer> transports =
new HashMap<TransportId, Integer>();
for (SimplexPluginFactory f : pluginConfig.getSimplexFactories())
@@ -89,15 +89,12 @@ class KeyManagerImpl implements KeyManager, Service, EventListener {
db.endTransaction(txn);
}
} catch (DbException e) {
- if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
- return false;
+ throw new ServiceException(e);
}
- return true;
}
@Override
- public boolean stop() {
- return true;
+ public void stopService() {
}
public void addContact(Transaction txn, ContactId c, SecretKey master,
diff --git a/briar-tests/src/org/briarproject/plugins/PluginManagerImplTest.java b/briar-tests/src/org/briarproject/plugins/PluginManagerImplTest.java
index 2d2ea5ce2c..872985ff15 100644
--- a/briar-tests/src/org/briarproject/plugins/PluginManagerImplTest.java
+++ b/briar-tests/src/org/briarproject/plugins/PluginManagerImplTest.java
@@ -23,8 +23,6 @@ import java.util.Arrays;
import java.util.concurrent.Executor;
import java.util.concurrent.Executors;
-import static org.junit.Assert.assertTrue;
-
public class PluginManagerImplTest extends BriarTestCase {
@Test
@@ -117,8 +115,8 @@ public class PluginManagerImplTest extends BriarTestCase {
transportPropertyManager, uiCallback);
// Two plugins should be started and stopped
- assertTrue(p.start());
- assertTrue(p.stop());
+ p.startService();
+ p.stopService();
context.assertIsSatisfied();
}
diff --git a/briar-tests/src/org/briarproject/sync/ValidationManagerImplTest.java b/briar-tests/src/org/briarproject/sync/ValidationManagerImplTest.java
index 561bb26ce4..9a5ec0dd38 100644
--- a/briar-tests/src/org/briarproject/sync/ValidationManagerImplTest.java
+++ b/briar-tests/src/org/briarproject/sync/ValidationManagerImplTest.java
@@ -112,7 +112,7 @@ public class ValidationManagerImplTest extends BriarTestCase {
cryptoExecutor);
vm.registerMessageValidator(clientId, validator);
vm.registerIncomingMessageHook(clientId, hook);
- vm.start();
+ vm.startService();
context.assertIsSatisfied();
}
@@ -166,7 +166,7 @@ public class ValidationManagerImplTest extends BriarTestCase {
cryptoExecutor);
vm.registerMessageValidator(clientId, validator);
vm.registerIncomingMessageHook(clientId, hook);
- vm.start();
+ vm.startService();
context.assertIsSatisfied();
}
@@ -223,7 +223,7 @@ public class ValidationManagerImplTest extends BriarTestCase {
cryptoExecutor);
vm.registerMessageValidator(clientId, validator);
vm.registerIncomingMessageHook(clientId, hook);
- vm.start();
+ vm.startService();
context.assertIsSatisfied();
}
--
GitLab