--- a/storage/core/src/main/java/com/redhat/thermostat/storage/core/DbService.java	Fri Feb 08 12:52:59 2013 -0500
+++ b/storage/core/src/main/java/com/redhat/thermostat/storage/core/DbService.java	Fri Feb 08 19:10:32 2013 +0100
@@ -40,11 +40,27 @@
 import com.redhat.thermostat.storage.core.ConnectionException;
 import com.redhat.thermostat.storage.core.Connection.ConnectionListener;
 
+/**
+ * DbService provides API for handling database (i.e. {@link Storage})
+ * connections. Once {@link DbService#connect()} has been called, the current
+ * instance can be retrieved as service. This registered service instance could
+ * then be used to disconnect from storage.
+ * 
+ * @see {@link DbServiceFactory}
+ */
 @Service
 public interface DbService {
 
     /**
-     * Connects to the given database.
+     * Connects to {@link Storage} and registers {@link Storage} instance which
+     * was used for the connection as a service.
+     * 
+     * <br/>
+     * <br/>
+     * <strong>Pre:</strong> Neither DbService nor Storage are registered as
+     * services. <br/>
+     * <strong>Post:</strong> Both DbService and Storage are registered as
+     * services.
      * 
      * @throws ConnectionException
      *             If DB connection cannot be established.
@@ -52,7 +68,15 @@
     void connect() throws ConnectionException;
 
     /**
-     * Disconnects from the database.
+     * Disconnects from {@link Storage}.
+     * 
+     * <br/>
+     * <br/>
+     * <strong>Pre:</strong> Both DbService and Storage are registered as
+     * services.
+     * <br/>
+     * <strong>Post:</strong> Neither DbService nor Storage are registered as
+     * services.
      * 
      * @throws ConnectionException
      */
@@ -65,20 +89,23 @@
      *             if not connected to storage.
      */
     String getConnectionUrl();
-    
+
     /**
-     * Registers the supplied ConnectionListener to be notified
-     * when the status of the database connection changes.
-     * @param listener - the listener to be registered
+     * Registers the supplied ConnectionListener to be notified when the status
+     * of the database connection changes.
+     * 
+     * @param listener
+     *            - the listener to be registered
      */
     void addConnectionListener(ConnectionListener listener);
-    
+
     /**
-     * Unregisters the supplied ConnectionListener if it was
-     * previously registered via {@link #addConnectionListener(ConnectionListener)}.
-     * @param listener - the listener to be unregistered
+     * Unregisters the supplied ConnectionListener if it was previously
+     * registered via {@link #addConnectionListener(ConnectionListener)}.
+     * 
+     * @param listener
+     *            - the listener to be unregistered
      */
     void removeConnectionListener(ConnectionListener listener);
-    
+
 }
-

--- a/storage/core/src/main/java/com/redhat/thermostat/storage/core/DbServiceFactory.java	Fri Feb 08 12:52:59 2013 -0500
+++ b/storage/core/src/main/java/com/redhat/thermostat/storage/core/DbServiceFactory.java	Fri Feb 08 19:10:32 2013 +0100
@@ -44,9 +44,39 @@
  * classes are won't be.
  *
  */
+/**
+ * Factory for creating new {@link DbService} instances.
+ * 
+ * @see {@link DbService}, {@link StorageProvider}.
+ *
+ */
 public class DbServiceFactory {
 
-    public DbService createDbService(String username, String password, String dbUrl) {
+    /**
+     * Creates a <strong>new</strong> {@link DbService} instance which can be
+     * used to establish a connection with {@link Storage}. Note that the actual
+     * {@link StorageProvider} which will be used for the connection is looked
+     * up based on registered StorageProvider OSGi services and URLs they are
+     * able to handle. If a {@link DbService} instance is already registered as
+     * a service, users are encouraged to use that instance for disconnecting
+     * from storage.
+     * 
+     * @param username
+     *            The username to use for the connection to storage.
+     * @param password
+     *            The password to use for the connection to storage.
+     * @param dbUrl
+     *            The URL to the storage endpoint. For example
+     *            {@code mongodb://127.0.0.1:27518} or
+     *            {@code https://storage.example.com/storage}.
+     * @return A new {@link DbService} instance which can be used for
+     *         establishing new storage connections.
+     * @throws StorageException
+     *             If no matching {@link StorageProvider} could be found for the
+     *             given dbUrl.
+     */
+    public DbService createDbService(String username, String password,
+            String dbUrl) throws StorageException {
         return DbServiceImpl.create(username, password, dbUrl);
     }
 }

--- a/storage/core/src/main/java/com/redhat/thermostat/storage/core/StorageProvider.java	Fri Feb 08 12:52:59 2013 -0500
+++ b/storage/core/src/main/java/com/redhat/thermostat/storage/core/StorageProvider.java	Fri Feb 08 19:10:32 2013 +0100
@@ -38,13 +38,42 @@
 
 import com.redhat.thermostat.storage.config.StartupConfiguration;
 
+/**
+ * Factory for creating a new {@link Storage} instance.
+ * 
+ */
 public interface StorageProvider {
 
+    /**
+     * Creates a new {@link Storage}.
+     * 
+     * @return The new instance.
+     */
     Storage createStorage();
-    
+
+    /**
+     * Sets the to-be-used configuration of this StorageProvider. Called prior
+     * {@link StorageProvider#canHandleProtocol()}.
+     * 
+     * @param config
+     */
     void setConfig(StartupConfiguration config);
-    
+
+    /**
+     * Method which determines if this StorageProvider can handle the given
+     * protocol as set via
+     * {@link StorageProvider#setConfig(StartupConfiguration)}.
+     * 
+     * <br/>
+     * <br/>
+     * <strong>Pre:</strong> Configuration has been set via
+     * {@link StorageProvider#setConfig(StartupConfiguration)}.
+     * 
+     * @return true if this StorageProvider can handle the protocol prefix of
+     *         the given StartupConfiguration. I.e.
+     *         {@link StorageProvider#createStorage()} can be safely called
+     *         given this config. false otherwise.
+     */
     boolean canHandleProtocol();
 
 }
-

--- a/storage/core/src/main/java/com/redhat/thermostat/storage/internal/DbServiceImpl.java	Fri Feb 08 12:52:59 2013 -0500
+++ b/storage/core/src/main/java/com/redhat/thermostat/storage/internal/DbServiceImpl.java	Fri Feb 08 19:10:32 2013 +0100
@@ -80,16 +80,21 @@
     }
 
     public void connect() throws ConnectionException {
+        // Storage and DbService must not be registered
+        // as service
+        ensureConnectPreCondition();
         try {
-            storage.getConnection().connect();
+            this.storage.getConnection().connect();
             dbServiceReg = context.registerService(DbService.class, this, null);
-            storageReg = context.registerService(Storage.class.getName(), storage, null);
+            storageReg = context.registerService(Storage.class.getName(), this.storage, null);
         } catch (Exception cause) {
             throw new ConnectionException(cause);
         }
     }
     
     public void disconnect() throws ConnectionException {
+        // DbService and Storage must be registered as service at this point
+        ensureDisconnectPrecondition();
         try {
             storage.getConnection().disconnect();
             storageReg.unregister();
@@ -117,6 +122,32 @@
         return new DbServiceImpl(username, password, dbUrl);
     }
 
+    @SuppressWarnings("rawtypes")
+    private void ensureDisconnectPrecondition() {
+        ServiceReference dbServiceReference = context
+                .getServiceReference(DbService.class);
+        ServiceReference storageReference = context
+                .getServiceReference(Storage.class);
+        if (dbServiceReference == null || storageReference == null) {
+            throw new IllegalStateException(
+                    "DbService or Storage not registered as service when "
+                            + "trying to disconnect");
+        }
+    }
+
+    @SuppressWarnings("rawtypes")
+    private void ensureConnectPreCondition() {
+        ServiceReference dbServiceReference = context
+                .getServiceReference(DbService.class);
+        ServiceReference storageReference = context
+                .getServiceReference(Storage.class);
+        if (dbServiceReference != null || storageReference != null) {
+            throw new IllegalStateException(
+                    "DbService or Storage already registered as service when "
+                            + "trying to connect");
+        }
+    }
+
     private static Storage createStorage(BundleContext context, String username, String password, String dbUrl) throws StorageException {
         StartupConfiguration config = new ConnectionConfiguration(dbUrl, username, password);
         StorageProvider prov = getStorageProvider(context, config);
@@ -127,6 +158,7 @@
         return prov.createStorage();
     }
     
+    @SuppressWarnings({ "rawtypes", "unchecked" })
     private static StorageProvider getStorageProvider(BundleContext context, StartupConfiguration config) {
         try {
             ServiceReference[] refs = context.getServiceReferences(StorageProvider.class.getName(), null);

--- a/storage/core/src/test/java/com/redhat/thermostat/storage/internal/DbServiceImplTest.java	Fri Feb 08 12:52:59 2013 -0500
+++ b/storage/core/src/test/java/com/redhat/thermostat/storage/internal/DbServiceImplTest.java	Fri Feb 08 19:10:32 2013 +0100
@@ -40,6 +40,7 @@
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -48,6 +49,7 @@
 import org.junit.Before;
 import org.junit.Test;
 import org.osgi.framework.ServiceReference;
+import org.osgi.framework.ServiceRegistration;
 
 import com.redhat.thermostat.storage.core.Connection;
 import com.redhat.thermostat.storage.core.Connection.ConnectionListener;
@@ -111,6 +113,50 @@
         // make sure we really get the same instance
         assertTrue(storage.equals(context.getService(storageRef)));
     }
+    
+    @SuppressWarnings("rawtypes")
+    @Test
+    public void testConnectEnforcesPreCond() {
+        ServiceRegistration reg = context.registerService(DbService.class, dbService, null);
+        try {
+            dbService.connect();
+            fail("connect should check if db service is already registered");
+        } catch (IllegalStateException e) {
+            // pass
+            reg.unregister();
+        }
+        reg = context.registerService(Storage.class, storage, null);
+        try {
+            dbService.connect();
+            fail("connect should check if storage service is already registered");
+        } catch (IllegalStateException e) {
+            // pass
+            reg.unregister();
+        }
+    }
+    
+    @SuppressWarnings("rawtypes")
+    @Test
+    public void testDisConnectEnforcesPreCond() {
+        ServiceRegistration reg = context.registerService(DbService.class, dbService, null);
+        try {
+            // Storage == null
+            dbService.disconnect();
+            fail("disconnect should check if storage service is already registered");
+        } catch (IllegalStateException e) {
+            // pass
+            reg.unregister();
+        }
+        reg = context.registerService(Storage.class, storage, null);
+        try {
+            // DbService == null
+            dbService.disconnect();
+            fail("disconnect should check if db service is already registered");
+        } catch (IllegalStateException e) {
+            // pass
+            reg.unregister();
+        }
+    }
 
     @Test
     public void testDisconnect() {