Finished collection caching implementation. Our databases will shortly breath a

great sigh of relief.

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

@@ -75,56 +75,68 @@ public abstract class FindAllQuery<T extends PersistentRecord> extends Query<Lis
             }
 
             _select = new SelectClause<T>(_type, _marsh.getPrimaryKeyFields(), clauses);
-            _builder = ctx.getSQLBuilder(DepotTypes.getDepotTypes(ctx, _select));
+            _qkey = new SimpleCacheKey(_marsh.getTableName() + "Query", _select.toString());
-            _builder.newQuery(_select);
         }
 
         @Override // from Query
         public List<T> getCachedResult (PersistenceContext ctx)
         {
-            return null; // TODO
+            if (_qkey == null) {
+                return null;
+            }
+            _keys = ctx.<KeySet<T>>cacheLookup(_qkey);
+            if (_keys == null) {
+                return null;
+            }
+            _cachedQueries++;
+            _fetchKeys = loadFromCache(ctx, _keys, _entities);
+            return (_fetchKeys.size() == 0) ? resolve(_keys, _entities) : null;
         }
 
         // from Query
         public List<T> invoke (PersistenceContext ctx, Connection conn, DatabaseLiaison liaison)
             throws SQLException
         {
-            Map<Key<T>, T> entities = Maps.newHashMap();
+            // we want this to remain null if our key set came from the cache
-            List<Key<T>> allKeys = Lists.newArrayList();
+            String stmtString = null;
-            Set<Key<T>> fetchKeys = Sets.newHashSet();
 
-            // first load up the primary keys on which we're operating
+            // if we didn't find our key set in the cache, load the keys that match
-            PreparedStatement stmt = _builder.prepare(conn);
+            if (_keys == null) {
-            String stmtString = stmt.toString(); // for debugging
+                List<Key<T>> keys = Lists.newArrayList();
-            try {
+                SQLBuilder builder = ctx.getSQLBuilder(DepotTypes.getDepotTypes(ctx, _select));
-                ResultSet rs = stmt.executeQuery();
+                builder.newQuery(_select);
-                while (rs.next()) {
+                PreparedStatement stmt = builder.prepare(conn);
-                    Key<T> key = _marsh.makePrimaryKey(rs);
+                stmtString = stmt.toString(); // for debugging
-                    allKeys.add(key);
+                try {
+                    ResultSet rs = stmt.executeQuery();
+                    while (rs.next()) {
+                        keys.add(_marsh.makePrimaryKey(rs));
+                    }
+                } finally {
+                    JDBCUtil.close(stmt);
                 }
-            } finally {
+                _keys = KeySet.newKeySet(_type, keys);
-                JDBCUtil.close(stmt);
+                _uncachedQueries++;
-            }
+                if (PersistenceContext.CACHE_DEBUG) {
-            _uncachedQueries++;
+                    log.info("Loaded " + _marsh.getTableName() + " keys", "query", _select,
-
+                             "keys", keysToString(_keys), "cacheable", (_qkey != null));
-            // now fetch any records we can from the cache
+                }
-            loadFromCache(ctx, allKeys, entities, fetchKeys);
+                if (_qkey != null) {
-            _cachedRecords = entities.size();
+                    ctx.cacheStore(_qkey, _keys); // cache the resulting key set
-            _uncachedRecords = fetchKeys.size();
+                }
-
+                // and fetch any records we can from the cache
-            if (PersistenceContext.CACHE_DEBUG) {
+                _fetchKeys = loadFromCache(ctx, _keys, _entities);
-                log.info("Loaded " + _marsh.getTableName(), "query", _select,
-                         "keys", keysToString(allKeys));
             }
 
             // finally load the rest from the database
-            return loadAndResolve(ctx, conn, allKeys, fetchKeys, entities, stmtString);
+            return loadAndResolve(ctx, conn, _keys, _fetchKeys, _entities, stmtString);
         }
 
-        protected CacheKey getCacheKey ()
+        protected SimpleCacheKey _qkey;
-        {
+        protected SelectClause<T> _select;
-            return null;
+        protected KeySet<T> _keys;
-        }
+        protected Set<Key<T>> _fetchKeys;
+        protected Map<Key<T>, T> _entities = Maps.newHashMap();
     }
 
     /**
@@ -138,16 +150,13 @@ public abstract class FindAllQuery<T extends PersistentRecord> extends Query<Lis
         {
             super(ctx, keys.iterator().next().getPersistentClass());
             _keys = keys;
-            _builder = ctx.getSQLBuilder(new DepotTypes(ctx, _type));
         }
 
         @Override // from Query
         public List<T> getCachedResult (PersistenceContext ctx)
         {
             // look up what we can from the cache
-            loadFromCache(ctx, _keys, _entities, _fetchKeys);
+            _fetchKeys = loadFromCache(ctx, _keys, _entities);
-            _cachedRecords = _entities.size();
-            _uncachedRecords = _fetchKeys.size();
 
             // if we found everything, we can just return our result straight away, yay!
             return _fetchKeys.isEmpty() ? resolve(_keys, _entities) : null;
@@ -161,8 +170,8 @@ public abstract class FindAllQuery<T extends PersistentRecord> extends Query<Lis
         }
 
         protected Iterable<Key<T>> _keys;
+        protected Set<Key<T>> _fetchKeys;
         protected Map<Key<T>, T> _entities = Maps.newHashMap();
-        protected Set<Key<T>> _fetchKeys = Sets.newHashSet();
     }
 
     /**
@@ -177,8 +186,6 @@ public abstract class FindAllQuery<T extends PersistentRecord> extends Query<Lis
         {
             super(ctx, type);
             _select = new SelectClause<T>(type, _marsh.getFieldNames(), clauses);
-            _builder = ctx.getSQLBuilder(DepotTypes.getDepotTypes(ctx, _select));
-            _builder.newQuery(_select);
         }
 
         @Override // from Query
@@ -193,7 +200,9 @@ public abstract class FindAllQuery<T extends PersistentRecord> extends Query<Lis
             throws SQLException
         {
             List<T> result = Lists.newArrayList();
-            PreparedStatement stmt = _builder.prepare(conn);
+            SQLBuilder builder = ctx.getSQLBuilder(DepotTypes.getDepotTypes(ctx, _select));
+            builder.newQuery(_select);
+            PreparedStatement stmt = builder.prepare(conn);
             try {
                 ResultSet rs = stmt.executeQuery();
                 while (rs.next()) {
@@ -211,6 +220,8 @@ public abstract class FindAllQuery<T extends PersistentRecord> extends Query<Lis
             // TODO: do we want to cache these results?
             return result;
         }
+
+        protected SelectClause<T> _select;
     }
 
     // from Query
@@ -226,9 +237,10 @@ public abstract class FindAllQuery<T extends PersistentRecord> extends Query<Lis
         _marsh = ctx.getMarshaller(type);
     }
 
-    protected void loadFromCache (PersistenceContext ctx, Iterable<Key<T>> allKeys,
+    protected Set<Key<T>> loadFromCache (PersistenceContext ctx, Iterable<Key<T>> allKeys,
-                                  Map<Key<T>, T> entities, Set<Key<T>> fetchKeys)
+                                         Map<Key<T>, T> entities)
     {
+        Set<Key<T>> fetchKeys = Sets.newHashSet();
         for (Key<T> key : allKeys) {
             T value = ctx.<T>cacheLookup(key);
             if (value != null) {
@@ -238,6 +250,12 @@ public abstract class FindAllQuery<T extends PersistentRecord> extends Query<Lis
             }
             fetchKeys.add(key);
         }
+        if (PersistenceContext.CACHE_DEBUG) {
+            log.info("Loaded from cache " + _marsh.getTableName(), "count", entities.size());
+        }
+        _cachedRecords = entities.size();
+        _uncachedRecords = fetchKeys.size();
+        return fetchKeys;
     }
 
     protected List<T> resolve (Iterable<Key<T>> allKeys, Map<Key<T>, T> entities)
@@ -287,9 +305,11 @@ public abstract class FindAllQuery<T extends PersistentRecord> extends Query<Lis
         throws SQLException
     {
         boolean hasPrimaryKey = _marsh.hasPrimaryKey();
-        _builder.newQuery(new SelectClause<T>(_type, _marsh.getFieldNames(),
+        SelectClause<T> select = new SelectClause<T>(
-                                              KeySet.newKeySet(_type, keys)));
+            _type, _marsh.getFieldNames(), KeySet.newKeySet(_type, keys));
-        PreparedStatement stmt = _builder.prepare(conn);
+        SQLBuilder builder = ctx.getSQLBuilder(DepotTypes.getDepotTypes(ctx, select));
+        builder.newQuery(select);
+        PreparedStatement stmt = builder.prepare(conn);
         try {
             Set<Key<T>> got = Sets.newHashSet();
             ResultSet rs = stmt.executeQuery();
@@ -300,10 +320,7 @@ public abstract class FindAllQuery<T extends PersistentRecord> extends Query<Lis
                 if (entities.put(key, obj) != null) {
                     dups++;
                 }
-                // cache our result if it has a primary key
+                ctx.cacheStore(key, obj.clone()); // cache our result
-                if (hasPrimaryKey) {
-                    ctx.cacheStore(_marsh.getPrimaryKey(obj), obj.clone());
-                }
                 got.add(key);
                 cnt++;
             }
@@ -311,7 +328,11 @@ public abstract class FindAllQuery<T extends PersistentRecord> extends Query<Lis
             // fewer, then complain
             if (cnt > keys.size() || (origStmt != null && cnt < keys.size())) {
                 log.warning("Row count mismatch in second pass", "origQuery", origStmt,
-                            "wanted", keys, "got", got, "dups", dups, new Exception());
+                            // we need toString() here or StringUtil will get smart and dump our
+                            // KeySet using its iterator which results in verbosity
+                            "wanted", KeySet.newKeySet(_type, keys).toString(),
+                            "got", KeySet.newKeySet(_type, got).toString(),
+                            "dups", dups, new Exception());
             }
 
             if (PersistenceContext.CACHE_DEBUG) {
@@ -335,9 +356,7 @@ public abstract class FindAllQuery<T extends PersistentRecord> extends Query<Lis
         return builder.append(")").toString();
     }
 
-    protected SQLBuilder _builder;
-    protected DepotMarshaller<T> _marsh;
     protected Class<T> _type;
-    protected SelectClause<T> _select;
+    protected DepotMarshaller<T> _marsh;
     protected int _cachedQueries, _uncachedQueries, _cachedRecords, _uncachedRecords;
 }

src/java/com/samskivert/depot/Key.java

@@ -189,7 +189,7 @@ public class Key<T extends PersistentRecord> extends WhereClause
     // from CacheKey
     public Serializable getCacheKey ()
     {
-        return _values;
+        return this;
     }
 
     // from ValidatingCacheInvalidator

src/java/com/samskivert/depot/KeySet.java

@@ -20,6 +20,7 @@
 
 package com.samskivert.depot;
 
+import java.io.Serializable;
 import java.util.AbstractCollection;
 import java.util.Arrays;
 import java.util.Collection;
@@ -48,7 +49,7 @@ import static com.samskivert.depot.Log.log;
  * the cache.
  */
 public abstract class KeySet<T extends PersistentRecord> extends WhereClause
-    implements SQLExpression, ValidatingCacheInvalidator, Iterable<Key<T>>
+    implements Serializable, SQLExpression, ValidatingCacheInvalidator, Iterable<Key<T>>
 {
     /**
      * Creates a key set for the supplied persistent record and keys.

src/java/com/samskivert/depot/SimpleCacheKey.java

@@ -22,6 +22,8 @@ package com.samskivert.depot;
 
 import java.io.Serializable;
 
+import com.samskivert.util.ObjectUtil;
+
 /**
  * Convenience class that implements {@link CacheKey} as simply as possibly. This class is
  * typically used when the caller wants to cache a non-obvious query such as a collection,
@@ -85,21 +87,8 @@ public class SimpleCacheKey
             return false;
         }
         SimpleCacheKey other = (SimpleCacheKey) obj;
-        if (_cacheId == null) {
+        return ObjectUtil.equals(_cacheId, other._cacheId) &&
-            if (other._cacheId != null) {
+            ObjectUtil.equals(_cacheKey, other._cacheKey);
-                return false;
-            }
-        } else if (!_cacheId.equals(other._cacheId)) {
-            return false;
-        }
-        if (_cacheKey == null) {
-            if (other._cacheKey != null) {
-                return false;
-            }
-        } else if (!_cacheKey.equals(other._cacheKey)) {
-            return false;
-        }
-        return true;
     }
 
     @Override

src/java/com/samskivert/depot/tests/TestRepository.java

@@ -27,9 +27,10 @@ import java.util.Set;
 
 import com.google.common.collect.Sets;
 
+import com.samskivert.jdbc.StaticConnectionProvider;
 import com.samskivert.util.RandomUtil;
 
-import com.samskivert.jdbc.StaticConnectionProvider;
+import com.samskivert.depot.CacheAdapter;
 import com.samskivert.depot.DepotRepository;
 import com.samskivert.depot.Key;
 import com.samskivert.depot.KeySet;
@@ -40,6 +41,7 @@ import com.samskivert.depot.annotation.Computed;
 import com.samskivert.depot.clause.Where;
 import com.samskivert.depot.expression.LiteralExp;
 import com.samskivert.depot.operator.Conditionals;
+import com.samskivert.depot.util.TestCacheAdapter;
 
 /**
  * A test tool for the Depot repository services.
@@ -60,7 +62,8 @@ public class TestRepository extends DepotRepository
         throws Exception
     {
         PersistenceContext perCtx = new PersistenceContext();
-        perCtx.init("test", new StaticConnectionProvider("depot.properties"), null);
+        perCtx.init("test", new StaticConnectionProvider("depot.properties"),
+                    new TestCacheAdapter());
 
         // tests a bogus rename migration
         // perCtx.registerMigration(TestRecord.class, new SchemaMigration.Rename(1, "foo", "bar"));
@@ -71,6 +74,7 @@ public class TestRepository extends DepotRepository
 
         TestRepository repo = new TestRepository(perCtx);
 
+        System.out.println("Deleting old record.");
         repo.delete(TestRecord.class, 1);
 
         Date now = new Date(System.currentTimeMillis());
@@ -86,7 +90,7 @@ public class TestRepository extends DepotRepository
         record.numbers = new int[] { 9, 0, 2, 1, 0 };
 
         repo.insert(record);
-        System.out.println(repo.load(TestRecord.class, record.recordId));
+        System.out.println("Record: " + repo.load(TestRecord.class, record.recordId));
 
 //         record.age = 25;
 //         record.name = "Bob";
@@ -96,7 +100,7 @@ public class TestRepository extends DepotRepository
         repo.updatePartial(TestRecord.class, record.recordId,
                            TestRecord.AGE, 25, TestRecord.NAME, "Bob",
                            TestRecord.NUMBERS, new int[] { 1, 2, 3, 4, 5 });
-        System.out.println(repo.load(TestRecord.class, record.recordId));
+        System.out.println("Updated " + repo.load(TestRecord.class, record.recordId));
 
         for (int ii = 2; ii < CREATE_RECORDS; ii++) {
             record = new TestRecord();
@@ -116,6 +120,11 @@ public class TestRepository extends DepotRepository
         System.out.println("Load none " + repo.loadAll(none.toCollection()) + ".");
         System.out.println("Delete none " + repo.deleteAll(TestRecord.class, none) + ".");
 
+        // test collection caching
+        Where where = new Where(new Conditionals.GreaterThan(TestRecord.RECORD_ID_C, 100));
+        System.out.println("100 and up: " + repo.findAll(TestRecord.class, where).size());
+        System.out.println("100 and up again: " + repo.findAll(TestRecord.class, where).size());
+
         // test a partial key set
         KeySet<TestRecord> some = KeySet.newSimpleKeySet(
             TestRecord.class, Sets.newHashSet(1, 3, 5, 7, 9));