Consolidated some duplicated update-related code in DepotMarshaller, nixed some

unneeded type parameterization.

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

@@ -480,34 +480,17 @@ public abstract class DepotRepository
      *
      * @throws DatabaseException if any problem is encountered communicating with the database.
      */
-    protected <T extends PersistentRecord> int update (T record)
+    protected int update (PersistentRecord record)
         throws DatabaseException
     {
-        @SuppressWarnings("unchecked") Class<T> pClass = (Class<T>) record.getClass();
+        Class<? extends PersistentRecord> pClass = record.getClass();
         requireNotComputed(pClass, "update");
-
+        DepotMarshaller<? extends PersistentRecord> marsh = _ctx.getMarshaller(pClass);
-        DepotMarshaller<T> marsh = _ctx.getMarshaller(pClass);
+        Key<? extends PersistentRecord> key = marsh.getPrimaryKey(record);
-        Key<T> key = marsh.getPrimaryKey(record);
         if (key == null) {
             throw new IllegalArgumentException("Can't update record with null primary key.");
         }
-
+        return doUpdate(key, new UpdateClause(pClass, key, marsh.getColumnFieldNames(), record));
-        UpdateClause<T> update =
-            new UpdateClause<T>(pClass, key, marsh.getColumnFieldNames(), record);
-        final SQLBuilder builder = _ctx.getSQLBuilder(DepotTypes.getDepotTypes(_ctx, update));
-        builder.newQuery(update);
-
-        return _ctx.invoke(new CachingModifier<T>(record, key, key) {
-            @Override
-            protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
-                PreparedStatement stmt = builder.prepare(conn);
-                try {
-                    return stmt.executeUpdate();
-                } finally {
-                    JDBCUtil.close(stmt);
-                }
-            }
-        });
     }
 
     /**
@@ -525,31 +508,13 @@ public abstract class DepotRepository
     {
         @SuppressWarnings("unchecked") Class<T> pClass = (Class<T>) record.getClass();
         requireNotComputed(pClass, "update");
-
         DepotMarshaller<T> marsh = _ctx.getMarshaller(pClass);
         Key<T> key = marsh.getPrimaryKey(record);
         if (key == null) {
             throw new IllegalArgumentException("Can't update record with null primary key.");
         }
-
+        String[] fields = ColumnExp.toNames(modifiedFields);
-        UpdateClause<T> update = new UpdateClause<T>(
+        return doUpdate(key, new UpdateClause(pClass, key, fields, record));
-            pClass, key, ColumnExp.toNames(modifiedFields), record);
-        final SQLBuilder builder = _ctx.getSQLBuilder(DepotTypes.getDepotTypes(_ctx, update));
-        builder.newQuery(update);
-
-        return _ctx.invoke(new CachingModifier<T>(record, key, key) {
-            @Override
-            protected int 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;
-                try {
-                    return stmt.executeUpdate();
-                } finally {
-                    JDBCUtil.close(stmt);
-                }
-            }
-        });
     }
 
     /**
@@ -594,6 +559,7 @@ public abstract class DepotRepository
         Class<T> type, final WhereClause key, CacheInvalidator invalidator, Object... fieldsValues)
         throws DatabaseException
     {
+        requireNotComputed(type, "updateLiteral");
         if (invalidator instanceof ValidatingCacheInvalidator) {
             ((ValidatingCacheInvalidator)invalidator).validateFlushType(type); // sanity check
         }
@@ -603,38 +569,15 @@ public abstract class DepotRepository
         final String[] fields = new String[fieldsValues.length/2];
         final SQLExpression[] values = new SQLExpression[fields.length];
         for (int ii = 0, idx = 0; ii < fields.length; ii++) {
-            if (fieldsValues[idx] instanceof ColumnExp) {
+            fields[ii] = ((ColumnExp) fieldsValues[idx++]).name;
-                fields[ii] = ((ColumnExp) fieldsValues[idx]).name;
-            } else if (fieldsValues[idx] instanceof String) {
-                fields[ii] = (String) fieldsValues[idx];
-            } else {
-                throw new IllegalArgumentException(
-                    "Field identifier #" + (ii+1) + " is neither String nor ColumnExp");
-            }
-            idx ++;
-
             if (fieldsValues[idx] instanceof SQLExpression) {
-                values[ii] = (SQLExpression) fieldsValues[idx];
+                values[ii] = (SQLExpression) fieldsValues[idx++];
             } else {
-                values[ii] = new ValueExp(fieldsValues[idx]);
+                values[ii] = new ValueExp(fieldsValues[idx++]);
             }
-            idx ++;
         }
-        UpdateClause<T> update = new UpdateClause<T>(type, key, fields, values);
-        final SQLBuilder builder = _ctx.getSQLBuilder(DepotTypes.getDepotTypes(_ctx, update));
-        builder.newQuery(update);
 
-        return _ctx.invoke(new Modifier(invalidator) {
+        return doUpdate(invalidator, new UpdateClause(type, key, fields, values));
-            @Override
-            protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
-                PreparedStatement stmt = builder.prepare(conn);
-                try {
-                    return stmt.executeUpdate();
-                } finally {
-                    JDBCUtil.close(stmt);
-                }
-            }
-        });
     }
 
     /**
@@ -690,7 +633,6 @@ public abstract class DepotRepository
         throws DatabaseException
     {
         requireNotComputed(type, "updateLiteral");
-
         if (invalidator instanceof ValidatingCacheInvalidator) {
             ((ValidatingCacheInvalidator)invalidator).validateFlushType(type); // sanity check
         }
@@ -706,21 +648,7 @@ public abstract class DepotRepository
             ii ++;
         }
 
-        UpdateClause<T> update = new UpdateClause<T>(type, key, fields, values);
+        return doUpdate(invalidator, new UpdateClause(type, key, fields, values));
-        final SQLBuilder builder = _ctx.getSQLBuilder(DepotTypes.getDepotTypes(_ctx, update));
-        builder.newQuery(update);
-
-        return _ctx.invoke(new Modifier(invalidator) {
-            @Override
-            protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
-                PreparedStatement stmt = builder.prepare(conn);
-                try {
-                    return stmt.executeUpdate();
-                } finally {
-                    JDBCUtil.close(stmt);
-                }
-            }
-        });
     }
 
     /**
@@ -740,8 +668,8 @@ public abstract class DepotRepository
 
         final DepotMarshaller<T> marsh = _ctx.getMarshaller(pClass);
         Key<T> key = marsh.hasPrimaryKey() ? marsh.getPrimaryKey(record) : null;
-        final UpdateClause<T> update =
+        final UpdateClause update =
-            new UpdateClause<T>(pClass, key, marsh.getColumnFieldNames(), record);
+            new UpdateClause(pClass, key, marsh.getColumnFieldNames(), record);
         final SQLBuilder builder = _ctx.getSQLBuilder(DepotTypes.getDepotTypes(_ctx, update));
 
         // if our primary key isn't null, we start by trying to update rather than insert
@@ -900,6 +828,26 @@ public abstract class DepotRepository
         }
     }
 
+    /**
+     * A helper method for the various partial update methods.
+     */
+    protected int doUpdate (CacheInvalidator invalidator, UpdateClause update)
+    {
+        final SQLBuilder builder = _ctx.getSQLBuilder(DepotTypes.getDepotTypes(_ctx, update));
+        builder.newQuery(update);
+        return _ctx.invoke(new Modifier(invalidator) {
+            @Override
+            protected int invoke (Connection conn, DatabaseLiaison liaison) throws SQLException {
+                PreparedStatement stmt = builder.prepare(conn);
+                try {
+                    return stmt.executeUpdate();
+                } finally {
+                    JDBCUtil.close(stmt);
+                }
+            }
+        });
+    }
+
     /**
      * If the supplied migration has not already been run, it will be run and if it completes, we
      * will note in the DepotMigrationHistory table that it has been run.

src/java/com/samskivert/depot/clause/SelectClause.java

@@ -34,14 +34,15 @@ import com.samskivert.depot.impl.ExpressionVisitor;
 /**
  * Builds actual SQL given a main persistent type and some {@link QueryClause} objects.
  */
-public class SelectClause<T extends PersistentRecord> implements QueryClause
+public class SelectClause implements QueryClause
 {
     /**
      * Creates a new Query object to generate one or more instances of the specified persistent
      * class, as dictated by the key and query clauses.  A persistence context is supplied for
      * instantiation of marshallers, which may trigger table creations and schema migrations.
      */
-    public SelectClause (Class<T> pClass, String[] fields, Collection<? extends QueryClause> clauses)
+    public SelectClause (Class<? extends PersistentRecord> pClass, String[] fields,
+                         Collection<? extends QueryClause> clauses)
     {
         _pClass = pClass;
         _fields = fields;
@@ -109,7 +110,8 @@ public class SelectClause<T extends PersistentRecord> implements QueryClause
     /**
      * A varargs version of the constructor.
      */
-    public SelectClause (Class<T> pClass, String[] fields, QueryClause... clauses)
+    public SelectClause (Class<? extends PersistentRecord> pClass,
+                         String[] fields, QueryClause... clauses)
     {
         this(pClass, fields, Arrays.asList(clauses));
     }
@@ -124,7 +126,7 @@ public class SelectClause<T extends PersistentRecord> implements QueryClause
         return _disMap.values();
     }
 
-    public Class<T> getPersistentClass ()
+    public Class<? extends PersistentRecord> getPersistentClass ()
     {
         return _pClass;
     }
@@ -224,7 +226,7 @@ public class SelectClause<T extends PersistentRecord> implements QueryClause
     protected Map<String, FieldDefinition> _disMap = Maps.newHashMap();
 
     /** The persistent class this select defines. */
-    protected Class<T> _pClass;
+    protected Class<? extends PersistentRecord> _pClass;
 
     /** The persistent fields to select. */
     protected String[] _fields;

src/java/com/samskivert/depot/impl/BuildVisitor.java

@@ -334,7 +334,7 @@ public abstract class BuildVisitor implements ExpressionVisitor<Void>
         return null;
     }
 
-    public Void visit (SelectClause<? extends PersistentRecord> selectClause)
+    public Void visit (SelectClause selectClause)
     {
         Class<? extends PersistentRecord> pClass = selectClause.getPersistentClass();
         boolean isInner = _innerClause;
@@ -432,7 +432,7 @@ public abstract class BuildVisitor implements ExpressionVisitor<Void>
         return null;
     }
 
-    public Void visit (UpdateClause<? extends PersistentRecord> updateClause)
+    public Void visit (UpdateClause updateClause)
     {
         if (updateClause.getWhereClause() == null) {
             throw new IllegalArgumentException(

src/java/com/samskivert/depot/impl/ExpressionEvaluator.java

@@ -299,12 +299,12 @@ public class ExpressionEvaluator
         throw new IllegalArgumentException("Can't evaluate expression: " + fromOverride);
     }
 
-    public Object visit (SelectClause<? extends PersistentRecord> selectClause)
+    public Object visit (SelectClause selectClause)
     {
         throw new IllegalArgumentException("Can't evaluate expression: " + selectClause);
     }
 
-    public Object visit (UpdateClause<? extends PersistentRecord> updateClause)
+    public Object visit (UpdateClause updateClause)
     {
         throw new IllegalArgumentException("Can't evaluate expression: " + updateClause);
     }

src/java/com/samskivert/depot/impl/ExpressionVisitor.java

@@ -83,8 +83,8 @@ public interface ExpressionVisitor<T>
     public T visit (WhereClause where);
     public T visit (Key.Expression<? extends PersistentRecord> key);
     public T visit (Exists<? extends PersistentRecord> exists);
-    public T visit (SelectClause<? extends PersistentRecord> selectClause);
+    public T visit (SelectClause selectClause);
-    public T visit (UpdateClause<? extends PersistentRecord> updateClause);
+    public T visit (UpdateClause updateClause);
     public T visit (DeleteClause deleteClause);
     public T visit (InsertClause insertClause);
     public T visit (CreateIndexClause createIndexClause);

src/java/com/samskivert/depot/impl/FindAllKeysQuery.java

@@ -54,7 +54,7 @@ public class FindAllKeysQuery<T extends PersistentRecord> extends Query<List<Key
     {
         _forUpdate = forUpdate;
         _marsh = ctx.getMarshaller(type);
-        _select = new SelectClause<T>(type, _marsh.getPrimaryKeyFields(), clauses);
+        _select = new SelectClause(type, _marsh.getPrimaryKeyFields(), clauses);
         _builder = ctx.getSQLBuilder(DepotTypes.getDepotTypes(ctx, _select));
         _builder.newQuery(_select);
     }
@@ -101,5 +101,5 @@ public class FindAllKeysQuery<T extends PersistentRecord> extends Query<List<Key
     protected boolean _forUpdate;
     protected SQLBuilder _builder;
     protected DepotMarshaller<T> _marsh;
-    protected SelectClause<T> _select;
+    protected SelectClause _select;
 }

src/java/com/samskivert/depot/impl/FindAllQuery.java

@@ -84,7 +84,7 @@ public abstract class FindAllQuery<T extends PersistentRecord> extends Query<Lis
                 }
             }
 
-            _select = new SelectClause<T>(_type, _marsh.getPrimaryKeyFields(), clauses);
+            _select = new SelectClause(_type, _marsh.getPrimaryKeyFields(), clauses);
             switch(strategy) {
             case SHORT_KEYS: case LONG_KEYS:
                 _qkey = new SimpleCacheKey(_marsh.getTableName() + "Keys", _select.toString());
@@ -159,7 +159,7 @@ public abstract class FindAllQuery<T extends PersistentRecord> extends Query<Lis
 
         protected SimpleCacheKey _qkey;
         protected CacheCategory _category;
-        protected SelectClause<T> _select;
+        protected SelectClause _select;
         protected KeySet<T> _keys;
         protected Set<Key<T>> _fetchKeys;
         protected Map<Key<T>, T> _entities = Maps.newHashMap();
@@ -212,7 +212,7 @@ public abstract class FindAllQuery<T extends PersistentRecord> extends Query<Lis
         {
             super(ctx, type);
 
-            _select = new SelectClause<T>(type, _marsh.getFieldNames(), clauses);
+            _select = new SelectClause(type, _marsh.getFieldNames(), clauses);
 
             if (cachedContents) {
                 _qkey = new SimpleCacheKey(_marsh.getTableName() + "Contents", _select.toString());
@@ -258,7 +258,7 @@ public abstract class FindAllQuery<T extends PersistentRecord> extends Query<Lis
             return result;
         }
 
-        protected SelectClause<T> _select;
+        protected SelectClause _select;
         protected SimpleCacheKey _qkey;
     }
 
@@ -343,7 +343,7 @@ public abstract class FindAllQuery<T extends PersistentRecord> extends Query<Lis
                                 Map<Key<T>, T> entities, String origStmt)
         throws SQLException
     {
-        SelectClause<T> select = new SelectClause<T>(
+        SelectClause select = new SelectClause(
             _type, _marsh.getFieldNames(), KeySet.newKeySet(_type, keys));
         SQLBuilder builder = ctx.getSQLBuilder(DepotTypes.getDepotTypes(ctx, select));
         builder.newQuery(select);

src/java/com/samskivert/depot/impl/FindOneQuery.java

@@ -52,7 +52,7 @@ public class FindOneQuery<T extends PersistentRecord> extends Query<T>
     {
         _strategy = strategy;
         _marsh = ctx.getMarshaller(type);
-        _select = new SelectClause<T>(type, _marsh.getFieldNames(), clauses);
+        _select = new SelectClause(type, _marsh.getFieldNames(), clauses);
         WhereClause where = _select.getWhereClause();
         if (where != null) {
             _select.getWhereClause().validateQueryType(type); // sanity check
@@ -135,7 +135,7 @@ public class FindOneQuery<T extends PersistentRecord> extends Query<T>
 
     protected DepotRepository.CacheStrategy _strategy;
     protected DepotMarshaller<T> _marsh;
-    protected SelectClause<T> _select;
+    protected SelectClause _select;
     protected SQLBuilder _builder;
     protected int _cachedRecords;
 }

src/java/com/samskivert/depot/impl/clause/UpdateClause.java

@@ -32,11 +32,11 @@ import com.samskivert.depot.impl.ExpressionVisitor;
 /**
  * Builds actual SQL given a main persistent type and some {@link QueryClause} objects.
  */
-public class UpdateClause<T extends PersistentRecord>
+public class UpdateClause
     implements QueryClause
 {
     public UpdateClause (Class<? extends PersistentRecord> pClass, WhereClause where,
-                         String[] fields, T pojo)
+                         String[] fields, PersistentRecord pojo)
     {
         _pClass = pClass;
         _where = where;
@@ -70,7 +70,7 @@ public class UpdateClause<T extends PersistentRecord>
         return _values;
     }
 
-    public Object getPojo ()
+    public PersistentRecord getPojo ()
     {
         return _pojo;
     }
@@ -113,5 +113,5 @@ public class UpdateClause<T extends PersistentRecord>
     protected SQLExpression[] _values;
 
     /** The object from which to fetch values, or null. */
-    protected Object _pojo;
+    protected PersistentRecord _pojo;
 }

src/java/com/samskivert/depot/operator/Exists.java

@@ -32,7 +32,7 @@ import com.samskivert.depot.impl.ExpressionVisitor;
 public class Exists<T extends PersistentRecord>
     implements SQLOperator
 {
-    public Exists (SelectClause<T> clause)
+    public Exists (SelectClause clause)
     {
         _clause = clause;
     }
@@ -47,7 +47,7 @@ public class Exists<T extends PersistentRecord>
         _clause.addClasses(classSet);
     }
 
-    public SelectClause<T> getSubClause ()
+    public SelectClause getSubClause ()
     {
         return _clause;
     }
@@ -58,5 +58,5 @@ public class Exists<T extends PersistentRecord>
         return "Exists(" + _clause + ")";
     }
 
-    protected SelectClause<T> _clause;
+    protected SelectClause _clause;
 }