Mercurial > hg > release > thermostat-0.5
changeset 793:03f70aded260
Add diagnostics to connect command failure and support web storage in shell.
This patch adds some diagnostics to the connect command. I.e. if a connection
fails due to existing connection the URL for this existing connection is
printed. This wasn't the case earlier.
It also adds better error handling/diagnostics to the connect command and the
launcher.
Example:
(before)
$ thermostat shell
Thermostat > connect -d blah
<Exception thrown>
(after)
$ thermostat shell
Thermostat > connect -d blah
Unrecognized storage URL blah
Thermostat >
It also adds web client bundles to connect command. This way we can support web
storage connections without adding bundles to all commands. This is a temp fix
for now.
Reviewed-by: neugens
Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2012-November/004146.html
line wrap: on
line diff
--- a/common/core/src/main/java/com/redhat/thermostat/common/DbService.java Tue Nov 20 11:48:04 2012 +0100 +++ b/common/core/src/main/java/com/redhat/thermostat/common/DbService.java Tue Nov 20 14:57:05 2012 +0100 @@ -43,15 +43,23 @@ /** * Connects to the given database. * - * @throws ConnectionException If DB connection cannot be established. + * @throws ConnectionException + * If DB connection cannot be established. */ void connect() throws ConnectionException; - - + /** * Disconnects from the database. * * @throws ConnectionException */ void disconnect() throws ConnectionException; -} \ No newline at end of file + + /** + * @returns the storage URL which was used for connection. + * + * @throws IllegalStateException + * if not connected to storage. + */ + String getConnectionUrl(); +} \ No newline at end of file
--- a/common/core/src/main/java/com/redhat/thermostat/common/internal/DbServiceImpl.java Tue Nov 20 11:48:04 2012 +0100 +++ b/common/core/src/main/java/com/redhat/thermostat/common/internal/DbServiceImpl.java Tue Nov 20 14:57:05 2012 +0100 @@ -45,6 +45,7 @@ import com.redhat.thermostat.common.dao.DAOFactoryImpl; import com.redhat.thermostat.storage.config.StartupConfiguration; import com.redhat.thermostat.storage.core.ConnectionException; +import com.redhat.thermostat.storage.core.StorageException; import com.redhat.thermostat.storage.core.StorageProvider; import com.redhat.thermostat.storage.core.StorageProviderUtil; @@ -55,43 +56,64 @@ private DAOFactory daoFactory; private BundleContext context; + private String dbUrl; - DbServiceImpl(String username, String password, String dbUrl) { - this(FrameworkUtil.getBundle(DbService.class).getBundleContext(), getDAOFactory(username, password, dbUrl)); + DbServiceImpl(String username, String password, String dbUrl) throws StorageException { + this(FrameworkUtil.getBundle(DbService.class).getBundleContext(), getDAOFactory(username, password, dbUrl), dbUrl); } - DbServiceImpl(BundleContext context, DAOFactory daoFactory) { + // for testing + DbServiceImpl(BundleContext context, DAOFactory daoFactory, String dbUrl) { this.daoFactory = daoFactory; this.context = context; + this.dbUrl = dbUrl; } public void connect() throws ConnectionException { - daoFactory.getConnection().connect(); - registration = context.registerService(DbService.class, this, null); - daoFactory.registerDAOsAndStorageAsOSGiServices(); + try { + daoFactory.getConnection().connect(); + registration = context.registerService(DbService.class, this, null); + daoFactory.registerDAOsAndStorageAsOSGiServices(); + } catch (Exception cause) { + throw new ConnectionException(cause); + } } public void disconnect() throws ConnectionException { - daoFactory.unregisterDAOsAndStorageAsOSGiServices(); - daoFactory.getConnection().disconnect(); - registration.unregister(); + try { + daoFactory.unregisterDAOsAndStorageAsOSGiServices(); + daoFactory.getConnection().disconnect(); + registration.unregister(); + } catch (Exception cause) { + throw new ConnectionException(cause); + } } + @Override + public String getConnectionUrl() { + return dbUrl; + } + /** * Factory method for creating a DbService instance. * * @param username * @param password * @param dbUrl - * @return + * @return a DbService instance + * @throws StorageException if no storage provider exists for the given {@code dbUrl}. */ - public static DbService create(String username, String password, String dbUrl) { + public static DbService create(String username, String password, String dbUrl) throws StorageException { return new DbServiceImpl(username, password, dbUrl); } - private static DAOFactory getDAOFactory(String username, String password, String dbUrl) { + private static DAOFactory getDAOFactory(String username, String password, String dbUrl) throws StorageException { StartupConfiguration config = new ConnectionConfiguration(dbUrl, username, password); StorageProvider prov = StorageProviderUtil.getStorageProvider(config); + if (prov == null) { + // no suitable provider found + throw new StorageException("No storage found for URL " + dbUrl); + } return new DAOFactoryImpl(prov); }
--- a/common/core/src/test/java/com/redhat/thermostat/common/internal/DbServiceTest.java Tue Nov 20 11:48:04 2012 +0100 +++ b/common/core/src/test/java/com/redhat/thermostat/common/internal/DbServiceTest.java Tue Nov 20 14:57:05 2012 +0100 @@ -36,6 +36,7 @@ package com.redhat.thermostat.common.internal; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; @@ -68,7 +69,7 @@ daoFactory = mock(DAOFactory.class); when(daoFactory.getConnection()).thenReturn(connection); - dbService = new DbServiceImpl(context, daoFactory); + dbService = new DbServiceImpl(context, daoFactory, "http://someUrl.ignored.com"); } @After @@ -122,4 +123,12 @@ // disconnect unregisters DbService assertNull(context.getServiceReference(DbService.class)); } + + @Test + public void canGetStorageUrl() { + String connectionURL = "http://test.example.com:8082"; + + dbService = new DbServiceImpl(context, null, connectionURL); + assertEquals(connectionURL, dbService.getConnectionUrl()); + } }
--- a/distribution/config/commands/connect.properties Tue Nov 20 11:48:04 2012 +0100 +++ b/distribution/config/commands/connect.properties Tue Nov 20 14:57:05 2012 +0100 @@ -1,7 +1,12 @@ -# ConnectCommand is provided by the tools bundle, which is a bootstrap bundle, and requires no other bundles. -bundles = +# ConnectCommand is provided by the tools bundle, which is a bootstrap bundle. +# In order to support web storage connections we add web bundles here +bundles = thermostat-web-common-0.5.0-SNAPSHOT.jar, \ + thermostat-web-client-0.5.0-SNAPSHOT.jar, \ + httpcomponents-core.jar, \ + httpcomponents-client.jar, \ + gson.jar -description = persistently connect to a database +description = persistently connect to storage usage = connect -d <url> [-u <username>] [-p <password>]
--- a/distribution/config/commands/disconnect.properties Tue Nov 20 11:48:04 2012 +0100 +++ b/distribution/config/commands/disconnect.properties Tue Nov 20 14:57:05 2012 +0100 @@ -1,9 +1,9 @@ # DisconnectCommand is provided by the tools bundle, which is a bootstrap bundle, and requires no other bundles. bundles = -description = disconnect from the currently used database +description = disconnect from the currently used storage usage = thermostat disconnect # No options necessary for this command -#options = \ No newline at end of file +#options =
--- a/launcher/src/main/java/com/redhat/thermostat/launcher/internal/LauncherImpl.java Tue Nov 20 11:48:04 2012 +0100 +++ b/launcher/src/main/java/com/redhat/thermostat/launcher/internal/LauncherImpl.java Tue Nov 20 14:57:05 2012 +0100 @@ -71,6 +71,7 @@ import com.redhat.thermostat.launcher.CommonCommandOptions; import com.redhat.thermostat.launcher.Launcher; import com.redhat.thermostat.storage.core.ConnectionException; +import com.redhat.thermostat.storage.core.StorageException; import com.redhat.thermostat.utils.keyring.Keyring; public class LauncherImpl implements Launcher { @@ -279,12 +280,17 @@ } String username = ctx.getArguments().getArgument(CommonCommandOptions.USERNAME_ARG); String password = ctx.getArguments().getArgument(CommonCommandOptions.PASSWORD_ARG); - DbService service = dbServiceFactory.createDbService(username, password, dbUrl); try { + // this may throw storage exception + DbService service = dbServiceFactory.createDbService(username, password, dbUrl); // This registers the DbService if all goes well service.connect(); + } catch (StorageException ex) { + throw new CommandException("Unsupported storage URL: " + dbUrl); } catch (ConnectionException ex) { - throw new CommandException("Could not connect to: " + dbUrl, ex); + String error = ex.getMessage(); + String message = ( error == null ? "" : " Error: " + error ); + throw new CommandException("Could not connect to: " + dbUrl + message, ex); } } }
--- a/tools/src/main/java/com/redhat/thermostat/tools/LocaleResources.java Tue Nov 20 11:48:04 2012 +0100 +++ b/tools/src/main/java/com/redhat/thermostat/tools/LocaleResources.java Tue Nov 20 14:57:05 2012 +0100 @@ -51,6 +51,8 @@ COMMAND_CONNECT_ALREADY_CONNECTED, COMMAND_CONNECT_FAILED_TO_CONNECT, + COMMAND_CONNECT_INVALID_STORAGE, + COMMAND_CONNECT_ERROR, COMMAND_DISCONNECT_NOT_CONNECTED, COMMAND_DISCONNECT_ERROR,
--- a/tools/src/main/java/com/redhat/thermostat/tools/cli/ConnectCommand.java Tue Nov 20 11:48:04 2012 +0100 +++ b/tools/src/main/java/com/redhat/thermostat/tools/cli/ConnectCommand.java Tue Nov 20 14:57:05 2012 +0100 @@ -46,6 +46,7 @@ import com.redhat.thermostat.common.utils.OSGIUtils; import com.redhat.thermostat.launcher.CommonCommandOptions; import com.redhat.thermostat.storage.core.ConnectionException; +import com.redhat.thermostat.storage.core.StorageException; import com.redhat.thermostat.tools.LocaleResources; import com.redhat.thermostat.utils.keyring.Keyring; @@ -79,7 +80,7 @@ DbService service = OSGIUtils.getInstance().getServiceAllowNull(DbService.class); if (service != null) { // Already connected, bail out - throw new CommandException(translator.localize(LocaleResources.COMMAND_CONNECT_ALREADY_CONNECTED)); + throw new CommandException(translator.localize(LocaleResources.COMMAND_CONNECT_ALREADY_CONNECTED, service.getConnectionUrl())); } if (prefs == null) { prefs = new ClientPreferences(OSGIUtils.getInstance().getService(Keyring.class)); @@ -90,11 +91,16 @@ } String username = ctx.getArguments().getArgument(CommonCommandOptions.USERNAME_ARG); String password = ctx.getArguments().getArgument(CommonCommandOptions.PASSWORD_ARG); - service = dbServiceFactory.createDbService(username, password, dbUrl); try { + // may throw StorageException if storage url is not supported + service = dbServiceFactory.createDbService(username, password, dbUrl); service.connect(); + } catch (StorageException ex) { + throw new CommandException(translator.localize(LocaleResources.COMMAND_CONNECT_INVALID_STORAGE, dbUrl)); } catch (ConnectionException ex) { - throw new CommandException(translator.localize(LocaleResources.COMMAND_CONNECT_FAILED_TO_CONNECT, dbUrl), ex); + String error = ex.getMessage(); + String message = ( error == null ? "" : " " + translator.localize(LocaleResources.COMMAND_CONNECT_ERROR, error) ); + throw new CommandException(translator.localize(LocaleResources.COMMAND_CONNECT_FAILED_TO_CONNECT, dbUrl + message), ex); } }
--- a/tools/src/main/resources/com/redhat/thermostat/tools/strings.properties Tue Nov 20 11:48:04 2012 +0100 +++ b/tools/src/main/resources/com/redhat/thermostat/tools/strings.properties Tue Nov 20 14:57:05 2012 +0100 @@ -7,8 +7,10 @@ VM_CPU_SERVICE_NOT_AVAILABLE = Unable to access vm cpu information (VmCpuStats not available) VM_MEMORY_SERVICE_NOT_AVAILABLE = Unable to access vm memory information (VmCpuStats not available) -COMMAND_CONNECT_ALREADY_CONNECTED = Already connected to storage. Please use disconnect command to disconnect. +COMMAND_CONNECT_ALREADY_CONNECTED = Already connected to storage: URL = {0}\nPlease use disconnect command to disconnect. COMMAND_CONNECT_FAILED_TO_CONNECT = Could not connect to db {0} +COMMAND_CONNECT_INVALID_STORAGE = Unrecognized storage URL {0} +COMMAND_CONNECT_ERROR = Error: {0} COMMAND_DISCONNECT_NOT_CONNECTED = Not connected to storage. You may use the connect command for establishing connections. COMMAND_DISCONNECT_ERROR = Failed to disconnect from database.
--- a/tools/src/test/java/com/redhat/thermostat/tools/cli/ConnectCommandTest.java Tue Nov 20 11:48:04 2012 +0100 +++ b/tools/src/test/java/com/redhat/thermostat/tools/cli/ConnectCommandTest.java Tue Nov 20 14:57:05 2012 +0100 @@ -106,19 +106,21 @@ } @Test - public void verifyConnectedThrowsException() { + public void verifyConnectedThrowsExceptionWithDiagnosticMessage() { + String dbUrl = "fluff"; DbService dbService = mock(DbService.class); OSGIUtils utils = mock(OSGIUtils.class); PowerMockito.mockStatic(OSGIUtils.class); when(OSGIUtils.getInstance()).thenReturn(utils); when(utils.getServiceAllowNull(DbService.class)).thenReturn(dbService); + when(dbService.getConnectionUrl()).thenReturn(dbUrl); SimpleArguments args = new SimpleArguments(); - args.addArgument("--dbUrl", "fluff"); + args.addArgument("--dbUrl", dbUrl); try { cmd.run(cmdCtxFactory.createContext(args)); } catch (CommandException e) { - assertEquals(translator.localize(LocaleResources.COMMAND_CONNECT_ALREADY_CONNECTED), e.getMessage()); + assertEquals(translator.localize(LocaleResources.COMMAND_CONNECT_ALREADY_CONNECTED, dbUrl), e.getMessage()); } }