Unified Query and Modifier under Operation which cleans up some niggles in

PersistenceContext.

src/java/com/samskivert/jdbc/depot/DepotMarshaller.java

@@ -576,7 +576,7 @@ public class DepotMarshaller<T extends PersistentRecord>
         // check to see if our schema version table exists, create it if not
         ctx.invoke(new Modifier() {
             @Override
-            public int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+            public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
                 liaison.createTableIfMissing(
                     conn, SCHEMA_VERSION_TABLE,
                     new String[] { P_COLUMN, V_COLUMN, MV_COLUMN },
@@ -695,7 +695,7 @@ public class DepotMarshaller<T extends PersistentRecord>
         final Iterable<Index> indexen = _indexes.values();
         ctx.invoke(new Modifier() {
             @Override
-            public int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+            public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
                 // create the table
                 String[] primaryKeyColumns = null;
                 if (_pkColumns != null) {
@@ -801,8 +801,8 @@ public class DepotMarshaller<T extends PersistentRecord>
         if (hasPrimaryKey() && metaData.pkName == null) {
             log.info("Adding primary key.");
             ctx.invoke(new Modifier() {
-                @Override
+                @Override public Integer invoke (Connection conn, DatabaseLiaison liaison)
-                public int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+                    throws SQLException {
                     liaison.addPrimaryKey(
                         conn, getTableName(), fieldsToColumns(getPrimaryKeyFields()));
                     return 0;
@@ -813,8 +813,8 @@ public class DepotMarshaller<T extends PersistentRecord>
             final String pkName = metaData.pkName;
             log.info("Dropping primary key: " + pkName);
             ctx.invoke(new Modifier() {
-                @Override
+                @Override public Integer invoke (Connection conn, DatabaseLiaison liaison)
-                public int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+                    throws SQLException {
                     liaison.dropPrimaryKey(conn, getTableName(), pkName);
                     return 0;
                 }
@@ -831,8 +831,8 @@ public class DepotMarshaller<T extends PersistentRecord>
             }
             // but this is a new, named index, so we create it
             ctx.invoke(new Modifier() {
-                @Override
+                @Override public Integer invoke (Connection conn, DatabaseLiaison liaison)
-                public int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+                    throws SQLException {
                     liaison.addIndexToTable(
                         conn, getTableName(), fieldsToColumns(index.fields()),
                         ixName, index.unique());
@@ -867,8 +867,8 @@ public class DepotMarshaller<T extends PersistentRecord>
             final String[] colArr = colSet.toArray(new String[colSet.size()]);
             final String fName = indexName;
             ctx.invoke(new Modifier() {
-                @Override
+                @Override public Integer invoke (Connection conn, DatabaseLiaison liaison)
-                public int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+                    throws SQLException {
                     liaison.addIndexToTable(conn, getTableName(), colArr, fName, true);
                     return 0;
                 }
@@ -889,8 +889,8 @@ public class DepotMarshaller<T extends PersistentRecord>
 
             // but not this one, so let's create it
             ctx.invoke(new Modifier() {
-                @Override
+                @Override public Integer invoke (Connection conn, DatabaseLiaison liaison)
-                public int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+                    throws SQLException {
                     builder.addFullTextSearch(conn, DepotMarshaller.this, recordFts);
                     return 0;
                 }
@@ -935,8 +935,8 @@ public class DepotMarshaller<T extends PersistentRecord>
             // initialValue which will use the new column name to obtain the sequence name which
             // ain't going to work either; we punt!
             ctx.invoke(new Modifier() {
-                @Override
+                @Override public Integer invoke (Connection conn, DatabaseLiaison liaison)
-                public int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+                    throws SQLException {
                     valgen.create(conn, liaison);
                     return 0;
                 }
@@ -980,7 +980,8 @@ public class DepotMarshaller<T extends PersistentRecord>
     }
 
     protected abstract class SimpleModifier extends Modifier {
-        @Override public int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+        @Override
+        public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
             Statement stmt = conn.createStatement();
             try {
                 return invoke(liaison, stmt);

src/java/com/samskivert/jdbc/depot/DepotRepository.java

@@ -319,8 +319,8 @@ public abstract class DepotRepository
 
         if (forUpdate) {
             _ctx.invoke(new Modifier(null) {
-                @Override
+                @Override public Integer invoke (Connection conn, DatabaseLiaison liaison)
-                public int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+                    throws SQLException {
                     PreparedStatement stmt = builder.prepare(conn);
                     try {
                         ResultSet rs = stmt.executeQuery();
@@ -375,7 +375,7 @@ public abstract class DepotRepository
         // key will be null if record was supplied without a primary key
         return _ctx.invoke(new CachingModifier<T>(record, key, key) {
             @Override
-            public int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+            public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
                 // if needed, update our modifier's key so that it can cache our results
                 Set<String> identityFields = Collections.emptySet();
                 if (_key == null) {
@@ -427,7 +427,7 @@ public abstract class DepotRepository
 
         return _ctx.invoke(new CachingModifier<T>(record, key, key) {
             @Override
-            public int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+            public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
                 PreparedStatement stmt = builder.prepare(conn);
                 try {
                     return stmt.executeUpdate();
@@ -464,7 +464,7 @@ public abstract class DepotRepository
 
         return _ctx.invoke(new CachingModifier<T>(record, key, key) {
             @Override
-            public int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+            public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
                 PreparedStatement stmt = builder.prepare(conn);
                 // clear out _result so that we don't rewrite this partial record to the cache
                 _result = null;
@@ -604,7 +604,7 @@ public abstract class DepotRepository
 
         return _ctx.invoke(new Modifier(invalidator) {
             @Override
-            public int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+            public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
                 PreparedStatement stmt = builder.prepare(conn);
                 try {
                     return stmt.executeUpdate();
@@ -735,7 +735,7 @@ public abstract class DepotRepository
 
         return _ctx.invoke(new Modifier(invalidator) {
             @Override
-            public int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+            public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
                 PreparedStatement stmt = builder.prepare(conn);
                 try {
                     return stmt.executeUpdate();
@@ -773,7 +773,7 @@ public abstract class DepotRepository
         final boolean[] created = new boolean[1];
         _ctx.invoke(new CachingModifier<T>(record, key, key) {
             @Override
-            public int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+            public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
                 PreparedStatement stmt = null;
                 try {
                     if (_key != null) {
@@ -897,7 +897,7 @@ public abstract class DepotRepository
 
         return _ctx.invoke(new Modifier(invalidator) {
             @Override
-            public int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+            public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
                 PreparedStatement stmt = builder.prepare(conn);
                 try {
                     return stmt.executeUpdate();

src/java/com/samskivert/jdbc/depot/Modifier.java

@@ -29,7 +29,7 @@ import com.samskivert.jdbc.DatabaseLiaison;
 /**
  * Encapsulates a modification of persistent objects.
  */
-public abstract class Modifier
+public abstract class Modifier implements Operation<Integer>
 {
     /**
      * A simple modifier that executes a single SQL statement. No cache flushing is done as a
@@ -41,7 +41,8 @@ public abstract class Modifier
             super(null);
         }
 
-        @Override public int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+        @Override
+        public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
             Statement stmt = conn.createStatement();
             try {
                 return stmt.executeUpdate(createQuery(liaison));
@@ -102,7 +103,7 @@ public abstract class Modifier
      * Overriden to perform the actual database modifications represented by this object; should
      * return the number of modified rows.
      */
-    public abstract int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException;
+    public abstract Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException;
 
     /**
      * Constructs a {@link Modifier} without a cache invalidator.

src/java/com/samskivert/jdbc/depot/Operation.java

@@ -0,0 +1,38 @@
+//
+// $Id$
+//
+// samskivert library - useful routines for java programs
+// Copyright (C) 2001-2008 Michael Bayne
+// 
+// This library is free software; you can redistribute it and/or modify it
+// under the terms of the GNU Lesser General Public License as published
+// by the Free Software Foundation; either version 2.1 of the License, or
+// (at your option) any later version.
+//
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+// Lesser General Public License for more details.
+//
+// You should have received a copy of the GNU Lesser General Public
+// License along with this library; if not, write to the Free Software
+// Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+
+package com.samskivert.jdbc.depot;
+
+import java.sql.Connection;
+import java.sql.SQLException;
+
+import com.samskivert.jdbc.DatabaseLiaison;
+
+/**
+ * An abstraction that encompasses both {@link Query} and {@link Modifier} operations.
+ */
+public interface Operation<T>
+{
+    /**
+     * Performs the actual JDBC interactions associated with this operation.
+     */
+    public T invoke (Connection conn, DatabaseLiaison liaison)
+        throws SQLException;
+}

src/java/com/samskivert/jdbc/depot/PersistenceContext.java

@@ -273,7 +273,7 @@ public class PersistenceContext
             log.debug("cache miss [key=" + key + "]");
         }
         // otherwise, perform the query
-        @SuppressWarnings("unchecked") T result = (T) invoke(query, null, true);
+        T result = invoke(query, true);
         // and let the caller figure out if it wants to cache itself somehow
         query.updateCache(this, result);
         return result;
@@ -286,7 +286,7 @@ public class PersistenceContext
         throws DatabaseException
     {
         modifier.cacheInvalidation(this);
-        int rows = (Integer) invoke(null, modifier, true);
+        int rows = invoke(modifier, true);
         if (rows > 0) {
             modifier.cacheUpdate(this);
         }
@@ -522,15 +522,15 @@ public class PersistenceContext
     /**
      * Internal invoke method that takes care of transient retries for both queries and modifiers.
      */
-    protected <T> Object invoke (Query<T> query, Modifier modifier, boolean retryOnTransientFailure)
+    protected <T> T invoke (Operation<T> op, boolean retryOnTransientFailure)
         throws DatabaseException
     {
         checkAreInitialized(); // le check du sanity
 
-        boolean isReadOnlyQuery = (query != null);
+        boolean isReadOnly = !(op instanceof Modifier);
         Connection conn;
         try {
-            conn = _conprov.getConnection(_ident, isReadOnlyQuery);
+            conn = _conprov.getConnection(_ident, isReadOnly);
         } catch (PersistenceException pe) {
             throw new DatabaseException(pe.getMessage(), pe.getCause());
         }
@@ -541,19 +541,12 @@ public class PersistenceContext
         // that someone else may be using
         synchronized (conn) {
             try {
-                if (isReadOnlyQuery) {
+                // if this becomes more complex than this single statement, then this should turn
-                    // if this becomes more complex than this single statement, then this should
+                // into a method call that contains the complexity
-                    // turn into a method call that contains the complexity
+                return op.invoke(conn, _liaison);
-                    return query.invoke(conn, _liaison);
-
-                } else {
-                    // if this becomes more complex than this single statement, then this should
-                    // turn into a method call that contains the complexity
-                    return modifier.invoke(conn, _liaison);
-                }
 
             } catch (SQLException sqe) {
-                if (!isReadOnlyQuery) {
+                if (!isReadOnly) {
                     // convert this exception to a DuplicateKeyException if appropriate
                     if (_liaison.isDuplicateRowException(sqe)) {
                         throw new DuplicateKeyException(sqe.getMessage());
@@ -561,7 +554,7 @@ public class PersistenceContext
                 }
 
                 // let the provider know that the connection failed
-                _conprov.connectionFailed(_ident, isReadOnlyQuery, conn, sqe);
+                _conprov.connectionFailed(_ident, isReadOnly, conn, sqe);
                 conn = null;
 
                 if (retryOnTransientFailure && _liaison.isTransientException(sqe)) {
@@ -572,18 +565,16 @@ public class PersistenceContext
                     log.info("Transient failure executing op, retrying [error=" + msg + "].");
 
                 } else {
-                    String msg = isReadOnlyQuery ?
+                    throw new DatabaseException("Operation failure " + op, sqe);
-                        "Query failure " + query : "Modifier failure " + modifier;
-                    throw new DatabaseException(msg, sqe);
                 }
 
             } finally {
-                _conprov.releaseConnection(_ident, isReadOnlyQuery, conn);
+                _conprov.releaseConnection(_ident, isReadOnly, conn);
             }
         }
 
         // if we got here, we want to retry a transient failure
-        return invoke(query, modifier, false);
+        return invoke(op, false);
     }
 
     protected void checkAreInitialized ()

src/java/com/samskivert/jdbc/depot/Query.java

@@ -29,7 +29,7 @@ import com.samskivert.jdbc.depot.PersistenceContext.CacheListener;
 /**
  * The base of all read-only queries.
  */
-public interface Query<T>
+public interface Query<T> extends Operation<T>
 {
     /** A simple base class for non-complex queries. */
     public abstract class Trivial<T> implements Query<T>
@@ -60,12 +60,6 @@ public interface Query<T>
      */
     public CacheKey getCacheKey ();
 
-    /**
-     * Performs the actual JDBC operations associated with this query.
-     */
-    public T invoke (Connection conn, DatabaseLiaison liaison)
-        throws SQLException;
-
     /**
      * Overriden by subclasses to perform special operations when the query would return a cache
      * hit. The value may be mutated, modified, or null may be return to force a database hit.

src/java/com/samskivert/jdbc/depot/SchemaMigration.java

@@ -52,7 +52,8 @@ public abstract class SchemaMigration extends Modifier
             _columnName = columnName;
         }
 
-        @Override public int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+        @Override
+        public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
             if (!liaison.tableContainsColumn(conn, _tableName, _columnName)) {
                 // we'll accept this inconsistency
                 log.warning(_tableName + "." + _columnName + " already dropped.");
@@ -77,7 +78,8 @@ public abstract class SchemaMigration extends Modifier
             _newColumnName = newColumnName;
         }
 
-        @Override public int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+        @Override
+        public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
             if (!liaison.tableContainsColumn(conn, _tableName, _oldColumnName)) {
                 if (liaison.tableContainsColumn(conn, _tableName, _newColumnName)) {
                     // we'll accept this inconsistency
@@ -133,7 +135,8 @@ public abstract class SchemaMigration extends Modifier
             _fieldName = fieldName;
         }
 
-        @Override public int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+        @Override
+        public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
             log.info("Updating type of '" + _fieldName + "' in " + _tableName);
             return liaison.changeColumn(conn, _tableName, _fieldName, _newColumnDef.getType(),
                 _newColumnDef.isNullable(), _newColumnDef.isUnique(),
@@ -171,7 +174,8 @@ public abstract class SchemaMigration extends Modifier
             _defaultValue = defaultValue;
         }
 
-        @Override public int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+        @Override
+        public Integer invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
             // override the default value in the column definition with the one provided
             ColumnDefinition defColumnDef = new ColumnDefinition(
                 _newColumnDef.getType(), _newColumnDef.isNullable(),