Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id A188F200B26 for ; Mon, 27 Jun 2016 23:13:19 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id A07EA160A54; Mon, 27 Jun 2016 21:13:19 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 74DED160A62 for ; Mon, 27 Jun 2016 23:13:18 +0200 (CEST) Received: (qmail 21678 invoked by uid 500); 27 Jun 2016 21:13:17 -0000 Mailing-List: contact commits-help@karaf.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@karaf.apache.org Delivered-To: mailing list commits@karaf.apache.org Received: (qmail 21580 invoked by uid 99); 27 Jun 2016 21:13:17 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 27 Jun 2016 21:13:17 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 76ABADFFF8; Mon, 27 Jun 2016 21:13:17 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: cschneider@apache.org To: commits@karaf.apache.org Message-Id: X-Mailer: ASF-Git Admin Mailer Subject: karaf git commit: [KARAF-4597] Avoid npe and exception logging in main module tests Date: Mon, 27 Jun 2016 21:13:17 +0000 (UTC) archived-at: Mon, 27 Jun 2016 21:13:19 -0000 Repository: karaf Updated Branches: refs/heads/master feaf69e1b -> 79a8da425 [KARAF-4597] Avoid npe and exception logging in main module tests Project: http://git-wip-us.apache.org/repos/asf/karaf/repo Commit: http://git-wip-us.apache.org/repos/asf/karaf/commit/79a8da42 Tree: http://git-wip-us.apache.org/repos/asf/karaf/tree/79a8da42 Diff: http://git-wip-us.apache.org/repos/asf/karaf/diff/79a8da42 Branch: refs/heads/master Commit: 79a8da42599329a1d6d893a285281b16dfb0f4a5 Parents: feaf69e Author: Christian Schneider Authored: Mon Jun 27 23:12:42 2016 +0200 Committer: Christian Schneider Committed: Mon Jun 27 23:13:10 2016 +0200 ---------------------------------------------------------------------- .../org/apache/karaf/main/InstanceHelper.java | 4 +- .../apache/karaf/main/lock/DefaultJDBCLock.java | 41 +++++++++----------- .../org/apache/karaf/main/MainLockingTest.java | 2 - .../karaf/main/TimeoutShutdownActivator.java | 2 +- .../karaf/main/lock/BaseJDBCLockTest.java | 7 ++-- .../karaf/main/lock/DefaultJDBCLockTest.java | 15 ++++++- .../karaf/main/lock/DerbyJDBCLockTest.java | 12 ++++++ .../karaf/main/lock/MySQLJDBCLockTest.java | 12 ++++++ .../karaf/main/lock/OracleJDBCLockTest.java | 7 ++++ .../karaf/main/lock/PostgreSQLJDBCLockTest.java | 12 ++++++ 10 files changed, 84 insertions(+), 30 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/karaf/blob/79a8da42/main/src/main/java/org/apache/karaf/main/InstanceHelper.java ---------------------------------------------------------------------- diff --git a/main/src/main/java/org/apache/karaf/main/InstanceHelper.java b/main/src/main/java/org/apache/karaf/main/InstanceHelper.java index 309f1ad..8b9ebaa 100644 --- a/main/src/main/java/org/apache/karaf/main/InstanceHelper.java +++ b/main/src/main/java/org/apache/karaf/main/InstanceHelper.java @@ -152,7 +152,9 @@ public class InstanceHelper { port = shutdownSocket.getLocalPort(); } if (portFile != null) { - Writer w = new OutputStreamWriter(new FileOutputStream(portFile)); + File portF = new File(portFile); + portF.getParentFile().mkdirs(); + Writer w = new OutputStreamWriter(new FileOutputStream(portF)); w.write(Integer.toString(port)); w.close(); } http://git-wip-us.apache.org/repos/asf/karaf/blob/79a8da42/main/src/main/java/org/apache/karaf/main/lock/DefaultJDBCLock.java ---------------------------------------------------------------------- diff --git a/main/src/main/java/org/apache/karaf/main/lock/DefaultJDBCLock.java b/main/src/main/java/org/apache/karaf/main/lock/DefaultJDBCLock.java index a6d5a29..1fe4fe9 100644 --- a/main/src/main/java/org/apache/karaf/main/lock/DefaultJDBCLock.java +++ b/main/src/main/java/org/apache/karaf/main/lock/DefaultJDBCLock.java @@ -152,30 +152,20 @@ public class DefaultJDBCLock implements Lock { * @return true, if the table exists else false */ boolean schemaExist(String tableName) { - ResultSet rs = null; - boolean schemaExists = false; try { DatabaseMetaData metadata = getConnection().getMetaData(); - rs = metadata.getTables(null, null, tableName, new String[] {"TABLE"}); - schemaExists = rs.next(); - if (schemaExists == false) { - // try table name in lower case - rs = metadata.getTables(null, null, tableName.toLowerCase(), new String[] {"TABLE"}); - schemaExists = rs.next(); - } - /* - if (schemaExists == false) { - // try table name in upper case - rs = getConnection().getMetaData().getTables(null, null, tableName.toUpperCase(), new String[] {"TABLE"}); - schemaExists = rs.next(); - } - */ + return metadata != null && (checkTableExists(tableName.toLowerCase(), metadata) // + || checkTableExists(tableName.toLowerCase(), metadata)); } catch (Exception ignore) { - LOG.log(Level.SEVERE, "Error testing for db table", ignore); - } finally { - closeSafely(rs); + return false; + //throw new RuntimeException("Error testing for db table", ignore); + } + } + + private boolean checkTableExists(String tableName, DatabaseMetaData metadata) throws SQLException { + try (ResultSet rs = metadata.getTables(null, null, tableName, new String[] {"TABLE"})) { + return rs.next(); } - return schemaExists; } /* @@ -203,7 +193,7 @@ public class DefaultJDBCLock implements Lock { lockAquired = preparedStatement.execute(); } catch (Exception e) { // Do we want to display this message everytime??? - LOG.log(Level.WARNING, "Failed to acquire database lock", e); + log(Level.WARNING, "Failed to acquire database lock", e); } finally { closeSafely(preparedStatement); } @@ -222,13 +212,20 @@ public class DefaultJDBCLock implements Lock { int rows = preparedStatement.executeUpdate(); lockUpdated = (rows == 1); } catch (Exception e) { - LOG.log(Level.WARNING, "Failed to update database lock", e); + log(Level.WARNING, "Failed to update database lock", e); } finally { closeSafely(preparedStatement); } return lockUpdated; } + + /** + * Can be overridden to suppress logs in tests + */ + public void log(Level level, String msg, Exception e) { + LOG.log(level, msg, e); + } /* * (non-Javadoc) http://git-wip-us.apache.org/repos/asf/karaf/blob/79a8da42/main/src/test/java/org/apache/karaf/main/MainLockingTest.java ---------------------------------------------------------------------- diff --git a/main/src/test/java/org/apache/karaf/main/MainLockingTest.java b/main/src/test/java/org/apache/karaf/main/MainLockingTest.java index 69c5d15..78d5a32 100644 --- a/main/src/test/java/org/apache/karaf/main/MainLockingTest.java +++ b/main/src/test/java/org/apache/karaf/main/MainLockingTest.java @@ -63,8 +63,6 @@ public class MainLockingTest { bundle.start(); - Thread.sleep(2000); - FrameworkStartLevel sl = framework.adapt(FrameworkStartLevel.class); MockLock lock = (MockLock) main.getLock(); http://git-wip-us.apache.org/repos/asf/karaf/blob/79a8da42/main/src/test/java/org/apache/karaf/main/TimeoutShutdownActivator.java ---------------------------------------------------------------------- diff --git a/main/src/test/java/org/apache/karaf/main/TimeoutShutdownActivator.java b/main/src/test/java/org/apache/karaf/main/TimeoutShutdownActivator.java index 2cfe07d..cd063dc 100644 --- a/main/src/test/java/org/apache/karaf/main/TimeoutShutdownActivator.java +++ b/main/src/test/java/org/apache/karaf/main/TimeoutShutdownActivator.java @@ -23,7 +23,7 @@ import org.osgi.framework.BundleContext; public class TimeoutShutdownActivator implements BundleActivator { - public static int TIMEOUT = 10000; + public static int TIMEOUT = 1000; public void start(BundleContext context) throws Exception { System.err.println("Starting timeout activator"); http://git-wip-us.apache.org/repos/asf/karaf/blob/79a8da42/main/src/test/java/org/apache/karaf/main/lock/BaseJDBCLockTest.java ---------------------------------------------------------------------- diff --git a/main/src/test/java/org/apache/karaf/main/lock/BaseJDBCLockTest.java b/main/src/test/java/org/apache/karaf/main/lock/BaseJDBCLockTest.java index d352bf1..d4d697e 100644 --- a/main/src/test/java/org/apache/karaf/main/lock/BaseJDBCLockTest.java +++ b/main/src/test/java/org/apache/karaf/main/lock/BaseJDBCLockTest.java @@ -68,7 +68,7 @@ public abstract class BaseJDBCLockTest { @Before public void setUp() throws Exception { - connection = EasyMock.createMock(Connection.class); + connection = EasyMock.createNiceMock(Connection.class); metaData = EasyMock.createMock(DatabaseMetaData.class); resultSet = EasyMock.createMock(ResultSet.class); preparedStatement = EasyMock.createMock(PreparedStatement.class); @@ -88,12 +88,13 @@ public abstract class BaseJDBCLockTest { public void initShouldCreateTheSchemaIfItNotExists() throws Exception { expect(connection.isClosed()).andReturn(false); connection.setAutoCommit(false); - expect(connection.getMetaData()).andReturn(metaData); + expect(connection.getMetaData()).andReturn(metaData).anyTimes(); expect(metaData.getTables((String) isNull(), (String) isNull(), anyString(), aryEq(new String[]{"TABLE"}))).andReturn(resultSet); expect(metaData.getTables((String) isNull(), (String) isNull(), anyString(), aryEq(new String[]{"TABLE"}))).andReturn(resultSet); expect(resultSet.next()).andReturn(false); expect(resultSet.next()).andReturn(false); resultSet.close(); + expectLastCall().atLeastOnce(); expect(connection.isClosed()).andReturn(false); expect(connection.createStatement()).andReturn(statement); expect(statement.execute("CREATE TABLE " + tableName + " (MOMENT " + momentDatatype + ", NODE " + nodeDatatype + ")" + createTableStmtSuffix)).andReturn(false); @@ -115,7 +116,7 @@ public abstract class BaseJDBCLockTest { expect(metaData.getTables((String) isNull(), (String) isNull(), anyString(), aryEq(new String[]{"TABLE"}))).andReturn(resultSet); expect(resultSet.next()).andReturn(true); resultSet.close(); - + expectLastCall().atLeastOnce(); replay(connection, metaData, statement, preparedStatement, resultSet); lock = createLock(props); http://git-wip-us.apache.org/repos/asf/karaf/blob/79a8da42/main/src/test/java/org/apache/karaf/main/lock/DefaultJDBCLockTest.java ---------------------------------------------------------------------- diff --git a/main/src/test/java/org/apache/karaf/main/lock/DefaultJDBCLockTest.java b/main/src/test/java/org/apache/karaf/main/lock/DefaultJDBCLockTest.java index 9a3451b..951c458 100644 --- a/main/src/test/java/org/apache/karaf/main/lock/DefaultJDBCLockTest.java +++ b/main/src/test/java/org/apache/karaf/main/lock/DefaultJDBCLockTest.java @@ -21,6 +21,9 @@ package org.apache.karaf.main.lock; import static org.junit.Assert.assertEquals; import java.sql.Connection; +import java.sql.SQLException; +import java.util.logging.Level; + import org.apache.felix.utils.properties.Properties; import org.junit.Before; @@ -54,15 +57,25 @@ public class DefaultJDBCLockTest extends BaseJDBCLockTest { long getCurrentTimeMillis() { return 1; } + + @Override + public void log(Level level, String msg, Exception e) { + // Suppress log + } }; } @Test - public void createConnectionShouldConcatinateOptionsCorrect() { + public void createConnectionShouldConcatinateOptionsCorrect() throws SQLException { props.put("karaf.lock.jdbc.url", this.url + ";dataEncryption=false"); lock = new DefaultJDBCLock(props) { @Override + boolean schemaExists() { + return true; + } + + @Override Connection doCreateConnection(String driver, String url, String username, String password) { assertEquals(this.driver, driver); assertEquals(this.url + ";create=true", url); http://git-wip-us.apache.org/repos/asf/karaf/blob/79a8da42/main/src/test/java/org/apache/karaf/main/lock/DerbyJDBCLockTest.java ---------------------------------------------------------------------- diff --git a/main/src/test/java/org/apache/karaf/main/lock/DerbyJDBCLockTest.java b/main/src/test/java/org/apache/karaf/main/lock/DerbyJDBCLockTest.java index b2bee85..fd96dcc 100644 --- a/main/src/test/java/org/apache/karaf/main/lock/DerbyJDBCLockTest.java +++ b/main/src/test/java/org/apache/karaf/main/lock/DerbyJDBCLockTest.java @@ -21,6 +21,8 @@ package org.apache.karaf.main.lock; import static org.junit.Assert.assertEquals; import java.sql.Connection; +import java.util.logging.Level; + import org.apache.felix.utils.properties.Properties; import org.junit.Before; @@ -54,6 +56,11 @@ public class DerbyJDBCLockTest extends BaseJDBCLockTest { long getCurrentTimeMillis() { return 1; } + + @Override + public void log(Level level, String msg, Exception e) { + // Suppress log + } }; } @@ -63,6 +70,11 @@ public class DerbyJDBCLockTest extends BaseJDBCLockTest { lock = new DerbyJDBCLock(props) { @Override + boolean schemaExists() { + return true; + } + + @Override Connection doCreateConnection(String driver, String url, String username, String password) { assertEquals(this.driver, driver); assertEquals(this.url + ";create=true", url); http://git-wip-us.apache.org/repos/asf/karaf/blob/79a8da42/main/src/test/java/org/apache/karaf/main/lock/MySQLJDBCLockTest.java ---------------------------------------------------------------------- diff --git a/main/src/test/java/org/apache/karaf/main/lock/MySQLJDBCLockTest.java b/main/src/test/java/org/apache/karaf/main/lock/MySQLJDBCLockTest.java index c49d71d..f9daad1 100644 --- a/main/src/test/java/org/apache/karaf/main/lock/MySQLJDBCLockTest.java +++ b/main/src/test/java/org/apache/karaf/main/lock/MySQLJDBCLockTest.java @@ -21,6 +21,8 @@ package org.apache.karaf.main.lock; import static org.junit.Assert.assertEquals; import java.sql.Connection; +import java.util.logging.Level; + import org.apache.felix.utils.properties.Properties; import org.junit.Before; @@ -54,6 +56,11 @@ public class MySQLJDBCLockTest extends BaseJDBCLockTest { long getCurrentTimeMillis() { return 1; } + + @Override + public void log(Level level, String msg, Exception e) { + // Suppress log + } }; } @@ -63,6 +70,11 @@ public class MySQLJDBCLockTest extends BaseJDBCLockTest { lock = new MySQLJDBCLock(props) { @Override + boolean schemaExists() { + return true; + } + + @Override Connection doCreateConnection(String driver, String url, String username, String password) { assertEquals(this.driver, driver); assertEquals(this.url + "&createDatabaseIfNotExist=true", url); http://git-wip-us.apache.org/repos/asf/karaf/blob/79a8da42/main/src/test/java/org/apache/karaf/main/lock/OracleJDBCLockTest.java ---------------------------------------------------------------------- diff --git a/main/src/test/java/org/apache/karaf/main/lock/OracleJDBCLockTest.java b/main/src/test/java/org/apache/karaf/main/lock/OracleJDBCLockTest.java index 30b2400..1209464 100644 --- a/main/src/test/java/org/apache/karaf/main/lock/OracleJDBCLockTest.java +++ b/main/src/test/java/org/apache/karaf/main/lock/OracleJDBCLockTest.java @@ -26,6 +26,8 @@ import static org.junit.Assert.*; import java.sql.Connection; import java.sql.SQLException; +import java.util.logging.Level; + import org.apache.felix.utils.properties.Properties; import org.junit.Before; @@ -60,6 +62,11 @@ public class OracleJDBCLockTest extends BaseJDBCLockTest { long getCurrentTimeMillis() { return 1; } + + @Override + public void log(Level level, String msg, Exception e) { + // Suppress log + } }; } http://git-wip-us.apache.org/repos/asf/karaf/blob/79a8da42/main/src/test/java/org/apache/karaf/main/lock/PostgreSQLJDBCLockTest.java ---------------------------------------------------------------------- diff --git a/main/src/test/java/org/apache/karaf/main/lock/PostgreSQLJDBCLockTest.java b/main/src/test/java/org/apache/karaf/main/lock/PostgreSQLJDBCLockTest.java index 6d4121f..079c7f1 100644 --- a/main/src/test/java/org/apache/karaf/main/lock/PostgreSQLJDBCLockTest.java +++ b/main/src/test/java/org/apache/karaf/main/lock/PostgreSQLJDBCLockTest.java @@ -23,6 +23,8 @@ import org.junit.Test; import java.sql.Connection; import java.sql.SQLException; +import java.util.logging.Level; + import org.apache.felix.utils.properties.Properties; import static org.easymock.EasyMock.*; @@ -58,6 +60,11 @@ public class PostgreSQLJDBCLockTest extends BaseJDBCLockTest { long getCurrentTimeMillis() { return 1; } + + @Override + public void log(Level level, String msg, Exception e) { + // Suppress log + } }; } @@ -67,6 +74,11 @@ public class PostgreSQLJDBCLockTest extends BaseJDBCLockTest { lock = new PostgreSQLJDBCLock(props) { @Override + boolean schemaExists() { + return true; + } + + @Override Connection doCreateConnection(String driver, String url, String username, String password) { assertEquals(this.driver, driver); assertEquals(this.url, url);