From: <svn...@os...> - 2012-03-11 10:55:24
|
Author: aaime Date: 2012-03-11 03:55:17 -0700 (Sun, 11 Mar 2012) New Revision: 38626 Modified: trunk/modules/library/jdbc/src/main/java/org/geotools/jdbc/JDBCFeatureStore.java trunk/modules/library/jdbc/src/test/java/org/geotools/jdbc/JDBCConnectionLifecycleTest.java trunk/modules/library/jdbc/src/test/java/org/geotools/jdbc/JDBCGeographyTest.java trunk/modules/library/jdbc/src/test/java/org/geotools/jdbc/JDBCTestSetup.java trunk/modules/library/jdbc/src/test/java/org/geotools/jdbc/JDBCUDTTest.java Log: [GEOT-4053] JDBCFeatureStore.removeFeatures does not close to the database connection it opened Modified: trunk/modules/library/jdbc/src/main/java/org/geotools/jdbc/JDBCFeatureStore.java =================================================================== --- trunk/modules/library/jdbc/src/main/java/org/geotools/jdbc/JDBCFeatureStore.java 2012-03-11 10:48:20 UTC (rev 38625) +++ trunk/modules/library/jdbc/src/main/java/org/geotools/jdbc/JDBCFeatureStore.java 2012-03-11 10:55:17 UTC (rev 38626) @@ -350,42 +350,50 @@ super.modifyFeatures(innerTypes, values, filter); } else { // let's grab the connection - Connection cx = getDataStore().getConnection(getState()); + Connection cx = null; + Transaction tx = getState().getTransaction(); + try { + cx = getDataStore().getConnection(tx); - // we want to support a "batch" update, but we need to be weary of locks - SimpleFeatureType featureType = getSchema(); - try { - getDataStore().ensureAuthorization(featureType, preFilter, getTransaction(), cx); - } - catch (SQLException e) { - throw (IOException) new IOException().initCause( e ); - } - ContentState state = getEntry().getState(transaction); - ReferencedEnvelope bounds = new ReferencedEnvelope( getSchema().getCoordinateReferenceSystem() ); - if( state.hasListener() ){ - // gather bounds before modification - ReferencedEnvelope before = getBounds( new DefaultQuery( getSchema().getTypeName(), preFilter ) ); - if( before != null && !before.isEmpty() ){ - bounds = before; + // we want to support a "batch" update, but we need to be weary of locks + SimpleFeatureType featureType = getSchema(); + try { + getDataStore().ensureAuthorization(featureType, preFilter, getTransaction(), cx); } - } - try { - getDataStore().update(getSchema(), innerTypes, values, preFilter, cx); - } catch(SQLException e) { - throw (IOException) (new IOException(e.getMessage()).initCause(e)); - } - - if( state.hasListener() ){ - // gather any updated bounds due to a geometry modification - for( Object value : values ){ - if( value instanceof Geometry ){ - Geometry geometry = (Geometry) value; - bounds.expandToInclude( geometry.getEnvelopeInternal() ); + catch (SQLException e) { + throw (IOException) new IOException().initCause( e ); + } + ContentState state = getEntry().getState(transaction); + ReferencedEnvelope bounds = new ReferencedEnvelope( getSchema().getCoordinateReferenceSystem() ); + if( state.hasListener() ){ + // gather bounds before modification + ReferencedEnvelope before = getBounds( new DefaultQuery( getSchema().getTypeName(), preFilter ) ); + if( before != null && !before.isEmpty() ){ + bounds = before; } } - // issue notificaiton - FeatureEvent event = new FeatureEvent(this, Type.CHANGED, bounds, preFilter ); - state.fireFeatureEvent( event ); + try { + getDataStore().update(getSchema(), innerTypes, values, preFilter, cx); + } catch(SQLException e) { + throw (IOException) (new IOException(e.getMessage()).initCause(e)); + } + + if( state.hasListener() ){ + // gather any updated bounds due to a geometry modification + for( Object value : values ){ + if( value instanceof Geometry ){ + Geometry geometry = (Geometry) value; + bounds.expandToInclude( geometry.getEnvelopeInternal() ); + } + } + // issue notificaiton + FeatureEvent event = new FeatureEvent(this, Type.CHANGED, bounds, preFilter ); + state.fireFeatureEvent( event ); + } + } finally { + if(tx == null || tx == Transaction.AUTO_COMMIT) { + getDataStore().closeSafe(cx); + } } } } @@ -402,30 +410,39 @@ super.removeFeatures(filter); } else { // let's grab the connection - Connection cx = getDataStore().getConnection(getState()); + Transaction tx = getState().getTransaction(); + Connection cx = null; - // we want to support a "batch" delete, but we need to be weary of locks - SimpleFeatureType featureType = getSchema(); try { - getDataStore().ensureAuthorization(featureType, preFilter, getTransaction(), cx); - } - catch (SQLException e) { - throw (IOException) new IOException().initCause( e ); - } - ContentState state = getEntry().getState(transaction); - ReferencedEnvelope bounds = new ReferencedEnvelope( getSchema().getCoordinateReferenceSystem() ); - if( state.hasListener() ){ - // gather bounds before modification - ReferencedEnvelope before = getBounds( new DefaultQuery( getSchema().getTypeName(), preFilter ) ); - if( before != null && !before.isEmpty() ){ - bounds = before; + cx = getDataStore().getConnection(tx); + + // we want to support a "batch" delete, but we need to be weary of locks + SimpleFeatureType featureType = getSchema(); + try { + getDataStore().ensureAuthorization(featureType, preFilter, getTransaction(), cx); + } + catch (SQLException e) { + throw (IOException) new IOException().initCause( e ); } - } - getDataStore().delete(featureType, preFilter, cx); - if( state.hasListener() ){ - // issue notification - FeatureEvent event = new FeatureEvent(this, Type.REMOVED, bounds, preFilter ); - state.fireFeatureEvent( event ); + ContentState state = getEntry().getState(transaction); + ReferencedEnvelope bounds = new ReferencedEnvelope( getSchema().getCoordinateReferenceSystem() ); + if( state.hasListener() ){ + // gather bounds before modification + ReferencedEnvelope before = getBounds( new DefaultQuery( getSchema().getTypeName(), preFilter ) ); + if( before != null && !before.isEmpty() ){ + bounds = before; + } + } + getDataStore().delete(featureType, preFilter, cx); + if( state.hasListener() ){ + // issue notification + FeatureEvent event = new FeatureEvent(this, Type.REMOVED, bounds, preFilter ); + state.fireFeatureEvent( event ); + } + } finally { + if(tx == null || tx == Transaction.AUTO_COMMIT) { + getDataStore().closeSafe(cx); + } } } } Modified: trunk/modules/library/jdbc/src/test/java/org/geotools/jdbc/JDBCConnectionLifecycleTest.java =================================================================== --- trunk/modules/library/jdbc/src/test/java/org/geotools/jdbc/JDBCConnectionLifecycleTest.java 2012-03-11 10:48:20 UTC (rev 38625) +++ trunk/modules/library/jdbc/src/test/java/org/geotools/jdbc/JDBCConnectionLifecycleTest.java 2012-03-11 10:55:17 UTC (rev 38626) @@ -89,6 +89,7 @@ // and now do a rollback t.rollback(); assertTrue(mockListener.onRollbackCalled); + t.close(); } public void testConnectionReleased() throws IOException { Modified: trunk/modules/library/jdbc/src/test/java/org/geotools/jdbc/JDBCGeographyTest.java =================================================================== --- trunk/modules/library/jdbc/src/test/java/org/geotools/jdbc/JDBCGeographyTest.java 2012-03-11 10:48:20 UTC (rev 38625) +++ trunk/modules/library/jdbc/src/test/java/org/geotools/jdbc/JDBCGeographyTest.java 2012-03-11 10:55:17 UTC (rev 38626) @@ -172,6 +172,7 @@ f.setDefaultGeometry(point); fw.write(); + fw.close(); Filter filter = ff.equals(ff.property("name"), ff.literal("append")); Query q = new Query(tname("geopoint"), filter); Modified: trunk/modules/library/jdbc/src/test/java/org/geotools/jdbc/JDBCTestSetup.java =================================================================== --- trunk/modules/library/jdbc/src/test/java/org/geotools/jdbc/JDBCTestSetup.java 2012-03-11 10:48:20 UTC (rev 38625) +++ trunk/modules/library/jdbc/src/test/java/org/geotools/jdbc/JDBCTestSetup.java 2012-03-11 10:55:17 UTC (rev 38626) @@ -16,6 +16,8 @@ */ package org.geotools.jdbc; +import static junit.framework.Assert.*; + import java.io.BufferedReader; import java.io.ByteArrayInputStream; import java.io.IOException; @@ -98,8 +100,19 @@ } public void tearDown() throws Exception { + final String leakMessage = "Expected no active connection, either there is a connection leak " + + "or you forgot to close some object holding onto connections in the tests (e.g., a reader, an iterator)"; if(dataSource instanceof BasicDataSource) { - ((BasicDataSource) dataSource).close(); + BasicDataSource bds = (BasicDataSource) dataSource; + assertEquals(leakMessage, 0, bds.getNumActive()); + } else if(dataSource instanceof DBCPDataSource) { + BasicDataSource bds = (BasicDataSource) ((DBCPDataSource) dataSource).getWrapped(); + assertEquals(leakMessage, 0, bds.getNumActive()); + } + + if(dataSource instanceof BasicDataSource) { + BasicDataSource bds = (BasicDataSource) dataSource; + bds.close(); } else if(dataSource instanceof ManageableDataSource) { ((ManageableDataSource) dataSource).close(); } Modified: trunk/modules/library/jdbc/src/test/java/org/geotools/jdbc/JDBCUDTTest.java =================================================================== --- trunk/modules/library/jdbc/src/test/java/org/geotools/jdbc/JDBCUDTTest.java 2012-03-11 10:48:20 UTC (rev 38625) +++ trunk/modules/library/jdbc/src/test/java/org/geotools/jdbc/JDBCUDTTest.java 2012-03-11 10:55:17 UTC (rev 38626) @@ -75,6 +75,7 @@ f.setAttribute(aname("ut"), "34cd"); w.write(); + w.close(); assertEquals(count+1, dataStore.getFeatureSource(tname("udt")).getCount(Query.ALL)); } |