You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by se...@apache.org on 2010/10/12 21:46:36 UTC

svn commit: r1021897 - in /commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl: GenericKeyedObjectPool.java GenericObjectPool.java SoftReferenceObjectPool.java StackKeyedObjectPool.java

Author: sebb
Date: Tue Oct 12 19:46:36 2010
New Revision: 1021897

URL: http://svn.apache.org/viewvc?rev=1021897&view=rev
Log:
Factories are immutable and non-null.
Remove ctors that don't provide factories
Check that ctor factory parameter is not null

Modified:
    commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java
    commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericObjectPool.java
    commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/SoftReferenceObjectPool.java
    commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/StackKeyedObjectPool.java

Modified: commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java
URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java?rev=1021897&r1=1021896&r2=1021897&view=diff
==============================================================================
--- commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java (original)
+++ commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericKeyedObjectPool.java Tue Oct 12 19:46:36 2010
@@ -314,17 +314,6 @@ public class GenericKeyedObjectPool<K,V>
     //--- constructors -----------------------------------------------
 
     /**
-     * Create a new <code>GenericKeyedObjectPool</code> with no factory.
-     *
-     * @see #GenericKeyedObjectPool(KeyedPoolableObjectFactory)
-     */
-    public GenericKeyedObjectPool() {
-        this(null, DEFAULT_MAX_ACTIVE, DEFAULT_WHEN_EXHAUSTED_ACTION, DEFAULT_MAX_WAIT, DEFAULT_MAX_IDLE, 
-                DEFAULT_TEST_ON_BORROW, DEFAULT_TEST_ON_RETURN, DEFAULT_TIME_BETWEEN_EVICTION_RUNS_MILLIS,
-                DEFAULT_NUM_TESTS_PER_EVICTION_RUN, DEFAULT_MIN_EVICTABLE_IDLE_TIME_MILLIS, DEFAULT_TEST_WHILE_IDLE);
-    }
-
-    /**
      * Create a new <code>GenericKeyedObjectPool</code> using the specified values.
      * @param factory the <code>KeyedPoolableObjectFactory</code> to use to create, validate, and destroy
      * objects if not <code>null</code>
@@ -560,12 +549,16 @@ public class GenericKeyedObjectPool<K,V>
      * @param testWhileIdle whether or not to validate objects in the idle object eviction thread, if any
      * (see {@link #setTestWhileIdle})
      * @param lifo whether or not the pools behave as LIFO (last in first out) queues (see {@link #setLifo})
+     * @throws IllegalArgumentException if the factory is null
      * @since Pool 1.4
      */
     public GenericKeyedObjectPool(KeyedPoolableObjectFactory<K,V> factory, int maxActive, WhenExhaustedAction whenExhaustedAction,
             long maxWait, int maxIdle, int maxTotal, int minIdle, boolean testOnBorrow, boolean testOnReturn,
             long timeBetweenEvictionRunsMillis, int numTestsPerEvictionRun, long minEvictableIdleTimeMillis,
             boolean testWhileIdle, boolean lifo) {
+        if (factory == null) {
+            throw new IllegalArgumentException("factory must not be null");
+        }
         _factory = factory;
         _maxActive = maxActive;
         _lifo = lifo;
@@ -1485,21 +1478,19 @@ public class GenericKeyedObjectPool<K,V>
         try {
             addObjectToPool(key, obj, true);
         } catch (Exception e) {
-            if (_factory != null) {
-                try {
-                    _factory.destroyObject(key, obj);
-                } catch (Exception e2) {
-                    // swallowed
-                }
-                // TODO: Correctness here depends on control in addObjectToPool.
-                // These two methods should be refactored, removing the
-                // "behavior flag", decrementNumActive, from addObjectToPool.
-                ObjectQueue pool = (_poolMap.get(key));
-                if (pool != null) {
-                    synchronized(this) {
-                        pool.decrementActiveCount();
-                        allocate();
-                    }
+            try {
+                _factory.destroyObject(key, obj);
+            } catch (Exception e2) {
+                // swallowed
+            }
+            // TODO: Correctness here depends on control in addObjectToPool.
+            // These two methods should be refactored, removing the
+            // "behavior flag", decrementNumActive, from addObjectToPool.
+            ObjectQueue pool = (_poolMap.get(key));
+            if (pool != null) {
+                synchronized(this) {
+                    pool.decrementActiveCount();
+                    allocate();
                 }
             }
         }
@@ -1619,15 +1610,11 @@ public class GenericKeyedObjectPool<K,V>
      *
      * @param key the key a new instance should be added to
      * @throws Exception when {@link KeyedPoolableObjectFactory#makeObject} fails.
-     * @throws IllegalStateException when no {@link #setFactory factory} has been set or after {@link #close} has been
      * called on this pool.
      */
     @Override
     public void addObject(K key) throws Exception {
         assertOpen();
-        if (_factory == null) {
-            throw new IllegalStateException("Cannot add objects without a factory.");
-        }
         V obj = _factory.makeObject(key);
         try {
             assertOpen();
@@ -2533,7 +2520,7 @@ public class GenericKeyedObjectPool<K,V>
     private int _totalInternalProcessing = 0;
 
     /** My {@link KeyedPoolableObjectFactory}. */
-    private KeyedPoolableObjectFactory<K,V> _factory = null;
+    private final KeyedPoolableObjectFactory<K,V> _factory;
 
     /**
      * My idle object eviction {@link TimerTask}, if any.

Modified: commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericObjectPool.java
URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericObjectPool.java?rev=1021897&r1=1021896&r2=1021897&view=diff
==============================================================================
--- commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericObjectPool.java (original)
+++ commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/GenericObjectPool.java Tue Oct 12 19:46:36 2010
@@ -298,15 +298,6 @@ public class GenericObjectPool<T> extend
     //--- constructors -----------------------------------------------
 
     /**
-     * Create a new <tt>GenericObjectPool</tt> with default properties.
-     */
-    public GenericObjectPool() {
-        this(null, DEFAULT_MAX_ACTIVE, DEFAULT_WHEN_EXHAUSTED_ACTION, DEFAULT_MAX_WAIT, DEFAULT_MAX_IDLE,
-                DEFAULT_MIN_IDLE, DEFAULT_TEST_ON_BORROW, DEFAULT_TEST_ON_RETURN, DEFAULT_TIME_BETWEEN_EVICTION_RUNS_MILLIS,
-                DEFAULT_NUM_TESTS_PER_EVICTION_RUN, DEFAULT_MIN_EVICTABLE_IDLE_TIME_MILLIS, DEFAULT_TEST_WHILE_IDLE);
-    }
-
-    /**
      * Create a new <tt>GenericObjectPool</tt> using the specified factory.
      * @param factory the (possibly <tt>null</tt>)PoolableObjectFactory to use to create, validate and destroy objects
      */
@@ -527,12 +518,16 @@ public class GenericObjectPool<T> extend
      * remain in the pool. (see {@link #setSoftMinEvictableIdleTimeMillis})
      * @param lifo whether or not objects are returned in last-in-first-out order from the idle object pool
      * (see {@link #setLifo})
+     * @throws IllegalArgumentException if the factory is null
      * @since Pool 1.4
      */
     public GenericObjectPool(PoolableObjectFactory<T> factory, int maxActive, WhenExhaustedAction whenExhaustedAction,
             long maxWait, int maxIdle, int minIdle, boolean testOnBorrow, boolean testOnReturn, long timeBetweenEvictionRunsMillis,
             int numTestsPerEvictionRun, long minEvictableIdleTimeMillis, boolean testWhileIdle,
             long softMinEvictableIdleTimeMillis, boolean lifo) {
+        if (factory == null) {
+            throw new IllegalArgumentException("factory must not be null");
+        }
         _factory = factory;
         _maxActive = maxActive;
         _lifo = lifo;
@@ -1203,9 +1198,7 @@ public class GenericObjectPool<T> extend
     @Override
     public void invalidateObject(T obj) throws Exception {
         try {
-            if (_factory != null) {
-                _factory.destroyObject(obj);
-            }
+            _factory.destroyObject(obj);
         } finally {
             synchronized (this) {
                 _numActive--;
@@ -1309,19 +1302,17 @@ public class GenericObjectPool<T> extend
         try {
             addObjectToPool(obj, true);
         } catch (Exception e) {
-            if (_factory != null) {
-                try {
-                    _factory.destroyObject(obj);
-                } catch (Exception e2) {
-                    // swallowed
-                }
-                // TODO: Correctness here depends on control in addObjectToPool.
-                // These two methods should be refactored, removing the
-                // "behavior flag", decrementNumActive, from addObjectToPool.
-                synchronized(this) {
-                    _numActive--;
-                    allocate();
-                }
+            try {
+                _factory.destroyObject(obj);
+            } catch (Exception e2) {
+                // swallowed
+            }
+            // TODO: Correctness here depends on control in addObjectToPool.
+            // These two methods should be refactored, removing the
+            // "behavior flag", decrementNumActive, from addObjectToPool.
+            synchronized(this) {
+                _numActive--;
+                allocate();
             }
         }
     }
@@ -1555,9 +1546,6 @@ public class GenericObjectPool<T> extend
     @Override
     public void addObject() throws Exception {
         assertOpen();
-        if (_factory == null) {
-            throw new IllegalStateException("Cannot add objects without a factory.");
-        }
         T obj = _factory.makeObject();
         try {
             assertOpen();
@@ -1931,7 +1919,7 @@ public class GenericObjectPool<T> extend
     private CursorableLinkedList<ObjectTimestampPair<T>>.Cursor _evictionCursor = null;
 
     /** My {@link PoolableObjectFactory}. */
-    private PoolableObjectFactory<T> _factory = null;
+    private final PoolableObjectFactory<T> _factory;
 
     /**
      * The number of objects {@link #borrowObject} borrowed

Modified: commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/SoftReferenceObjectPool.java
URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/SoftReferenceObjectPool.java?rev=1021897&r1=1021896&r2=1021897&view=diff
==============================================================================
--- commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/SoftReferenceObjectPool.java (original)
+++ commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/SoftReferenceObjectPool.java Tue Oct 12 19:46:36 2010
@@ -334,10 +334,10 @@ public class SoftReferenceObjectPool<T> 
     }
 
     /** My pool. */
-    private List<SoftReference<T>> _pool = null;
+    private final List<SoftReference<T>> _pool;
 
     /** My {@link PoolableObjectFactory}. */
-    private PoolableObjectFactory<T> _factory = null;
+    private final PoolableObjectFactory<T> _factory;
 
     /**
      * Queue of broken references that might be able to be removed from <code>_pool</code>.

Modified: commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/StackKeyedObjectPool.java
URL: http://svn.apache.org/viewvc/commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/StackKeyedObjectPool.java?rev=1021897&r1=1021896&r2=1021897&view=diff
==============================================================================
--- commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/StackKeyedObjectPool.java (original)
+++ commons/proper/pool/trunk/src/java/org/apache/commons/pool/impl/StackKeyedObjectPool.java Tue Oct 12 19:46:36 2010
@@ -47,45 +47,7 @@ import org.apache.commons.pool.PoolUtils
  * @since Pool 1.0
  */
 public class StackKeyedObjectPool<K,V> extends BaseKeyedObjectPool<K,V> implements KeyedObjectPool<K,V> {
-    /**
-     * Create a new pool using no factory.
-     * Clients must first set the {@link #setFactory factory} or
-     * may populate the pool using {@link #returnObject returnObject}
-     * before they can be {@link #borrowObject borrowed}.
-     *
-     * @see #StackKeyedObjectPool(KeyedPoolableObjectFactory)
-     */
-    public StackKeyedObjectPool() {
-        this((KeyedPoolableObjectFactory<K,V>)null,DEFAULT_MAX_SLEEPING,DEFAULT_INIT_SLEEPING_CAPACITY);
-    }
 
-    /**
-     * Create a new pool using no factory.
-     * Clients must first set the {@link #setFactory factory} or
-     * may populate the pool using {@link #returnObject returnObject}
-     * before they can be {@link #borrowObject borrowed}.
-     *
-     * @param max cap on the number of "sleeping" instances in the pool
-     * @see #StackKeyedObjectPool(KeyedPoolableObjectFactory, int)
-     */
-    public StackKeyedObjectPool(int max) {
-        this((KeyedPoolableObjectFactory<K,V>)null,max,DEFAULT_INIT_SLEEPING_CAPACITY);
-    }
-
-    /**
-     * Create a new pool using no factory.
-     * Clients must first set the {@link #setFactory factory} or
-     * may populate the pool using {@link #returnObject returnObject}
-     * before they can be {@link #borrowObject borrowed}.
-     *
-     * @param max cap on the number of "sleeping" instances in the pool
-     * @param init initial size of the pool (this specifies the size of the container,
-     *             it does not cause the pool to be pre-populated.)
-     * @see #StackKeyedObjectPool(KeyedPoolableObjectFactory, int, int)
-     */
-    public StackKeyedObjectPool(int max, int init) {
-        this((KeyedPoolableObjectFactory<K,V>)null,max,init);
-    }
 
     /**
      * Create a new <code>SimpleKeyedObjectPool</code> using
@@ -122,6 +84,9 @@ public class StackKeyedObjectPool<K,V> e
      *             it does not cause the pool to be pre-populated.)
      */
     public StackKeyedObjectPool(KeyedPoolableObjectFactory<K,V> factory, int max, int init) {
+        if (factory == null) {
+            throw new IllegalArgumentException("factory must not be null");
+        }
         _factory = factory;
         _maxSleeping = (max < 0 ? DEFAULT_MAX_SLEEPING : max);
         _initSleepingCapacity = (init < 1 ? DEFAULT_INIT_SLEEPING_CAPACITY : init);
@@ -152,14 +117,10 @@ public class StackKeyedObjectPool<K,V> e
                 obj = stack.pop();
                 _totIdle--;
             } else {
-                if(null == _factory) {
-                    throw new NoSuchElementException("pools without a factory cannot create new objects as needed.");
-                } else {
-                    obj = _factory.makeObject(key);
-                    newlyMade = true;
-                }
+                obj = _factory.makeObject(key);
+                newlyMade = true;
             }
-            if (null != _factory && null != obj) {
+            if (null != obj) {
                 try {
                     _factory.activateObject(key, obj);
                     if (!_factory.validateObject(key, obj)) {
@@ -199,26 +160,22 @@ public class StackKeyedObjectPool<K,V> e
     @Override
     public synchronized void returnObject(K key, V obj) throws Exception {
         decrementActiveCount(key);
-        if (null != _factory) {
-            if (_factory.validateObject(key, obj)) {
-                try {
-                    _factory.passivateObject(key, obj);
-                } catch (Exception ex) {
-                    _factory.destroyObject(key, obj);
-                    return;
-                }
-            } else {
+        if (_factory.validateObject(key, obj)) {
+            try {
+                _factory.passivateObject(key, obj);
+            } catch (Exception ex) {
+                _factory.destroyObject(key, obj);
                 return;
             }
+        } else {
+            return;
         }
 
         if (isClosed()) {
-            if (null != _factory) {
-                try {
-                    _factory.destroyObject(key, obj);
-                } catch (Exception e) {
-                    // swallowed
-                }
+            try {
+                _factory.destroyObject(key, obj);
+            } catch (Exception e) {
+                // swallowed
             }
             return;
         }
@@ -238,12 +195,10 @@ public class StackKeyedObjectPool<K,V> e
             } else {
                 staleObj = obj;
             }
-            if(null != _factory) {
-                try {
-                    _factory.destroyObject(key, staleObj);
-                } catch (Exception e) {
-                    // swallowed
-                }
+            try {
+                _factory.destroyObject(key, staleObj);
+            } catch (Exception e) {
+                // swallowed
             }
         }
         stack.push(obj);
@@ -256,9 +211,7 @@ public class StackKeyedObjectPool<K,V> e
     @Override
     public synchronized void invalidateObject(K key, V obj) throws Exception {
         decrementActiveCount(key);
-        if(null != _factory) {
-            _factory.destroyObject(key,obj);
-        }
+        _factory.destroyObject(key,obj);
         notifyAll(); // _totalActive has changed
     }
 
@@ -269,14 +222,10 @@ public class StackKeyedObjectPool<K,V> e
      *
      * @param key the key a new instance should be added to
      * @throws Exception when {@link KeyedPoolableObjectFactory#makeObject} fails.
-     * @throws IllegalStateException when no {@link #_factory} has been set or after {@link #close} has been called on this pool.
      */
     @Override
     public synchronized void addObject(K key) throws Exception {
         assertOpen();
-        if (_factory == null) {
-            throw new IllegalStateException("Cannot add objects without a factory.");
-        }
         V obj = _factory.makeObject(key);
         try {
             if (!_factory.validateObject(key, obj)) {
@@ -404,13 +353,11 @@ public class StackKeyedObjectPool<K,V> e
         if(null == stack) {
             return;
         } else {
-            if(null != _factory) {
-                for (V v : stack) {
-                    try {
-                        _factory.destroyObject(key, v);
-                    } catch(Exception e) {
-                        // ignore error, keep destroying the rest
-                    }
+            for (V v : stack) {
+                try {
+                    _factory.destroyObject(key, v);
+                } catch(Exception e) {
+                    // ignore error, keep destroying the rest
                 }
             }
             _totIdle -= stack.size();
@@ -457,6 +404,7 @@ public class StackKeyedObjectPool<K,V> e
      * @return the {@link KeyedPoolableObjectFactory} used by this pool to manage object instances.
      * @since 1.5.5
      */
+    // TODO does this still need to be synchronised?
     public synchronized KeyedPoolableObjectFactory<K,V> getFactory() {
         return _factory;
     }