Introduce a new cache strategy that does collection queries in two stages, but does not cache the keyset. This is a safe strategy, and we use it in places where an out-of-date result set is dangerous or annoying.

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

@@ -78,32 +78,37 @@ public abstract class DepotRepository
 
         /**
          * Resolve this collection query in two steps: first we enumerate the primary keys for
-         * all the records that satisfy the query, then we acquire the actual requested data for
+         * all the records that satisfy the query, then we acquire the actual data corresponding
-         * each key. The cache is consulted and updated in both steps: for the first, see if we've
+         * to each key -- first by consulting the cache, and then only loading from the database
-         * already executed precisely this query and cached the resulting key set. For the second
+         * the records for the keys that were not located in the cache.
-         * phase, search the cache for each key in said set, and retrieve the corresponding data
-         * record from there when possible. Finally execute a database query to retrieve the data
-         * for any keys that were not in the cache, making sure to cache them in the process.
          *
-         * Note: There is currently no cache invalidation of collection queries. If records are
+         * Note: This strategy may not be used on @Computed records, for records that do not in
-         * inserted, deleted or modified, cached keysets will not be updated. The keyset cannot be
+         * fact have a primary key, or for queries that use @FieldOverrides.
-         * guaranteed to be up to date. For sensitive operations, use Cache.NONE.
+         */
+        RECORDS,
+
+        /**
+         * This strategy is identical to {@link #RECORDS}, but we also cache the keyset fetched
+         * in the first pass. This makes it much more efficient, but also less reliable because
+         * there is no invalidation of the keyset query: If records are inserted, deleted or
+         * modified, cached keysets will not be updated.
          *
-         * Note: The KEYS strategy may not be used on @Computed records, for records that do not
+         * Note: This strategy may not be used on @Computed records, for records that do not in
-         * in fact have a primary key, or for queries that use @FieldOverrides.
+         * fact have a primary key, or for queries that use @FieldOverrides.
          */
         KEYS,
 
         /**
-         * This cache strategy is direct and explicit, eschewing the dual phases of the KEYS
+         * This cache strategy is direct and explicit, eschewing the dual phases of the
-         * approach. However, before the database is invoked at all, we consult the cache hoping
+         * {@link #RECORDS} and {@link #KEYS} approaches. However, before the database is invoked
-         * to find the entire result set already stashed away in there, using the entire query
+         * at all, we consult the cache hoping to find the entire result set already stashed away
-         * as the key. If we failed to find it, we execute the query and update the cache with the
+         * in there, using the entire query as the key. If we failed to find it, we execute the
-         * result.
+         * query and update the cache with the result.
          *
-         * This strategy has none of the limitations of KEYS and can be used with key-less and
+         * This strategy has none of the limitations of {@link #KEYS} and can be used with key-less
-         * @Computed records and arbitrarily complicated queries. Note however that as with KEYS,
+         * and @Computed records and arbitrarily complicated queries. Note however that as with
-         * there is currently no automatic invalidation and it is potentially very memory intensive.
+         * {@link #KEYS}, there is no automatic invalidation. It is also potentially very memory
+         * intensive.
          */
         CONTENTS
     };
@@ -345,7 +350,8 @@ public abstract class DepotRepository
     {
         DepotMarshaller<T> marsh = _ctx.getMarshaller(type);
 
-        if (cache == CacheStrategy.KEYS || cache == CacheStrategy.BEST) {
+        switch (cache) {
+        case KEYS: case BEST: case RECORDS:
             String reason = null;
             if (marsh.getTableName() == null) {
                 reason = type + " is computed";
@@ -365,8 +371,9 @@ public abstract class DepotRepository
                 cache = (reason != null) ? CacheStrategy.NONE : CacheStrategy.KEYS;
 
             } else if (reason != null) {
-                // if user explicitly asked for the KEYS strategy and we can't do it, protest
+                // if user explicitly asked for a strategy we can't do, protest
-                throw new IllegalArgumentException("Cannot use KEYS strategy because " + reason);
+                throw new IllegalArgumentException(
+                    "Cannot use " + cache + " strategy because " + reason);
             }
         }
 
@@ -374,8 +381,9 @@ public abstract class DepotRepository
             cache = CacheStrategy.NONE;
         }
 
-        if (cache == CacheStrategy.KEYS) {
+        if (cache == CacheStrategy.KEYS || cache == CacheStrategy.RECORDS) {
-            return _ctx.invoke(new FindAllQuery.WithCache<T>(_ctx, type, clauses));
+            return _ctx.invoke(new FindAllQuery.WithCache<T>(
+                    _ctx, type, clauses, cache == CacheStrategy.KEYS));
         }
         return _ctx.invoke(new FindAllQuery.Explicitly<T>(
                 _ctx, type, clauses, cache == CacheStrategy.CONTENTS));

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

@@ -66,7 +66,7 @@ public abstract class FindAllQuery<T extends PersistentRecord> extends Query<Lis
     public static class WithCache<T extends PersistentRecord> extends FindAllQuery<T>
     {
         public WithCache (PersistenceContext ctx, Class<T> type,
-                          Collection<? extends QueryClause> clauses)
+                          Collection<? extends QueryClause> clauses, boolean cacheKeys)
             throws DatabaseException
         {
             super(ctx, type);
@@ -83,7 +83,11 @@ public abstract class FindAllQuery<T extends PersistentRecord> extends Query<Lis
             }
 
             _select = new SelectClause<T>(_type, _marsh.getPrimaryKeyFields(), clauses);
-            _qkey = new SimpleCacheKey(_marsh.getTableName() + "Keys", _select.toString());
+            if (cacheKeys) {
+                _qkey = new SimpleCacheKey(_marsh.getTableName() + "Keys", _select.toString());
+            } else {
+                _qkey = null;
+            }
         }
 
         @Override // from Query
@@ -127,7 +131,7 @@ public abstract class FindAllQuery<T extends PersistentRecord> extends Query<Lis
                 _uncachedQueries++;
                 if (PersistenceContext.CACHE_DEBUG) {
                     log.info("Loaded " + _marsh.getTableName() + " keys", "query", _select,
-                             "keys", keysToString(_keys), "cacheable", (_qkey != null));
+                             "keys", keysToString(_keys), "cached", (_qkey != null));
                 }
                 if (_qkey != null) {
                     ctx.cacheStore(_qkey, _keys); // cache the resulting key set