Feature/cache improvements 2 (#1067)

* New Mode to control queryCache behavior

* Collections form the cache have to be read only

* findCount not queried in query cache

* return modifiable collections from the cache

* FIX: Compile errors

* No effective code change: reformat code / javadoc

src/main/java/io/ebean/CacheMode.java

@@ -0,0 +1,57 @@
+package io.ebean;
+/**
+ * Enum to control the different cache modes for queryCache (and maybe later) beanCache.
+ * 
+ * If cache is enabled, you must be careful, what you do with the returned collection.
+ * By default the returned collections are read only and you will get an exception if you try
+ * to change them.
+ * If you add ".setReadOnly(false)" to your query, you'll get a collection that is a clone from the
+ * one in the cache. That means, changing does not affect the cache
+ * 
+ * @author Roland Praml, FOCONIS AG
+ */
+public enum CacheMode {
+  /**
+   * Do not use cache.
+   */
+  OFF(false, false),
+  
+  /**
+   * Use the cace (query & store the resut). 
+   */
+  ON(true, true), 
+  
+  /**
+   * Do not read from cache, but write retrived value to cache.
+   * Use this, if you want to get the fresh value from database and a CacheMode.ON query will follow.
+   */
+  RECACHE(false, true),
+  
+  /**
+   * Query the cache for value. If it is there, use it, otherwise hit database but do NOT put the value
+   * into the cache. (this mode is for completeness. There's probably no use case for this)
+   */
+  QUERY_ONLY(true,false);
+  
+  private boolean get;
+  private boolean put;
+
+  private CacheMode(boolean get, boolean put) {
+    this.get = get;
+    this.put = put;
+  }
+  
+  /**
+   * Retruns <code>true</code> if value is read from cache.
+   */
+  public boolean isGet() {
+    return get;
+  }
+  
+  /**
+   * Returns <code>true</code> if value (from database) is written to cache.
+   */
+  public boolean isPut() {
+    return put;
+  }
+}

src/main/java/io/ebean/ExpressionList.java

@@ -445,12 +445,21 @@ public interface ExpressionList<T> {
   Query<T> setUseCache(boolean useCache);
 
   /**
-   * Set to true to use the query for executing this query.
+   * Set the {@link CacheMode} to use the query for executing this query.
    *
    * @see Query#setUseQueryCache(boolean)
    */
-  Query<T> setUseQueryCache(boolean useCache);
+  Query<T> setUseQueryCache(CacheMode useCache);
 
+  /**
+   * Calls {@link #setUseQueryCache(CacheMode)} with <code>ON</code> or <code>OFF</code>.
+   * 
+   * @see Query#setUseQueryCache(CacheMode)
+   */
+  default Query<T> setUseQueryCache(boolean enabled) {
+    return setUseQueryCache(enabled ? CacheMode.ON : CacheMode.OFF);
+  }
+  
   /**
    * Set to true if this query should execute against the doc store.
    * <p>

src/main/java/io/ebean/Query.java

@@ -1325,10 +1325,17 @@ public interface Query<T> {
   Query<T> setUseCache(boolean useCache);
 
   /**
-   * Set this to true to use the query cache.
+   * Set the {@link CacheMode} to use the query for executing this query.
    */
-  Query<T> setUseQueryCache(boolean useQueryCache);
-
+  Query<T> setUseQueryCache(CacheMode useQueryCache);
+  
+  /**
+   * Calls {@link #setUseQueryCache(CacheMode)} with <code>ON</code> or <code>OFF</code>.
+   */
+  default Query<T> setUseQueryCache(boolean enabled) {
+    return setUseQueryCache(enabled ? CacheMode.ON : CacheMode.OFF);
+  }
+  
   /**
    * Set to true if this query should execute against the doc store.
    * <p>

src/main/java/io/ebean/bean/BeanCollection.java

@@ -226,4 +226,9 @@ public interface BeanCollection<E> extends Serializable {
    * additions and removals have been processed.
    */
   void modifyReset();
+
+  /**
+   * Return a shallow copy of this collection that is modifiable.
+   */
+  BeanCollection<E> getShallowCopy();
 }

src/main/java/io/ebean/common/AbstractBeanCollection.java

@@ -213,4 +213,16 @@ abstract class AbstractBeanCollection<E> implements BeanCollection<E> {
   boolean holdsModifications() {
     return modifyHolder != null && modifyHolder.hasModifications();
   }
+  
+  /**
+   * Copies all relevant properties for a clone. See {@link #getShallowCopy()}
+   * @param other
+   */
+  protected void setFromOriginal(AbstractBeanCollection<E> other) {
+    this.disableLazyLoad = other.disableLazyLoad;
+    this.ebeanServerName = other.ebeanServerName;
+    this.loader = other.loader;
+    this.ownerBean = other.ownerBean;
+    this.propertyName = other.propertyName;
+  }
 }

src/main/java/io/ebean/common/BeanList.java

@@ -541,4 +541,11 @@ public final class BeanList<E> extends AbstractBeanCollection<E> implements List
     }
 
   }
+
+  @Override
+  public BeanCollection<E> getShallowCopy() {
+    BeanList<E> copy = new BeanList<>(new CopyOnFirstWriteList<>(list));
+    copy.setFromOriginal(this);
+    return copy;
+  }
 }

src/main/java/io/ebean/common/BeanMap.java

@@ -340,4 +340,10 @@ public final class BeanMap<K, E> extends AbstractBeanCollection<E> implements Ma
     return map.values();
   }
 
+  @Override
+  public BeanCollection<E> getShallowCopy() {
+    BeanMap<K, E> copy = new BeanMap<>(new LinkedHashMap<>(map));
+    copy.setFromOriginal(this);
+    return copy;
+  }
 }

src/main/java/io/ebean/common/BeanSet.java

@@ -377,5 +377,11 @@ public final class BeanSet<E> extends AbstractBeanCollection<E> implements Set<E
       throw new IllegalStateException("This collection is in ReadOnly mode");
     }
   }
-
+  
+  @Override
+  public BeanCollection<E> getShallowCopy() {
+    BeanSet<E> copy = new BeanSet<>(new LinkedHashSet<>(set));
+    copy.setFromOriginal(this);
+    return copy;
+  }
 }

src/main/java/io/ebean/common/CopyOnFirstWriteList.java

@@ -0,0 +1,180 @@
+package io.ebean.common;
+
+import java.io.Serializable;
+import java.util.AbstractList;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Comparator;
+import java.util.List;
+import java.util.function.Predicate;
+import java.util.function.UnaryOperator;
+
+/**
+ * List that copies itself on first write access. Needed to keep memory footprint low and the ability
+ * to modify lists from cache.
+ * 
+ * @author Roland Praml, FOCONIS AG
+ */
+public final class CopyOnFirstWriteList<E> extends AbstractList<E> implements List<E>, Serializable {
+
+  private static final long serialVersionUID = 1L;
+
+  /**
+   * The underlying List implementation.
+   */
+  private List<E> list;
+  
+  
+  public CopyOnFirstWriteList(List<E> list) {
+    super();
+    this.list = list;
+  }
+
+  private volatile boolean copied = false;
+
+  @Override
+  public int size() {
+    return list.size();
+  }
+
+  @Override
+  public boolean isEmpty() {
+    return list.isEmpty();
+  }
+
+  @Override
+  public boolean contains(Object o) {
+    return list.contains(o);
+  }
+
+  @Override
+  public Object[] toArray() {
+    return list.toArray();
+  }
+
+  @Override
+  public <T> T[] toArray(T[] a) {
+    return list.toArray(a);
+  }
+
+  @Override
+  public boolean add(E e) {
+    checkCopyOnWrite();
+    return list.add(e);
+  }
+
+  @Override
+  public boolean remove(Object o) {
+    checkCopyOnWrite();
+    return list.remove(o);
+  }
+
+  @Override
+  public boolean containsAll(Collection<?> c) {
+    return list.containsAll(c);
+  }
+
+  @Override
+  public boolean addAll(Collection<? extends E> c) {
+    checkCopyOnWrite();
+    return list.addAll(c);
+  }
+
+  @Override
+  public boolean addAll(int index, Collection<? extends E> c) {
+    checkCopyOnWrite();
+    return list.addAll(index, c);
+  }
+
+  @Override
+  public boolean removeAll(Collection<?> c) {
+    checkCopyOnWrite();
+    return list.removeAll(c);
+  }
+
+  @Override
+  public boolean retainAll(Collection<?> c) {
+    checkCopyOnWrite();
+    return list.retainAll(c);
+  }
+
+  @Override
+  public void replaceAll(UnaryOperator<E> operator) {
+    checkCopyOnWrite();
+    list.replaceAll(operator);
+  }
+
+  @Override
+  public boolean removeIf(Predicate<? super E> filter) {
+    checkCopyOnWrite();
+    return list.removeIf(filter);
+  }
+
+  @Override
+  public void sort(Comparator<? super E> c) {
+    checkCopyOnWrite();
+    list.sort(c);
+  }
+
+  @Override
+  public void clear() {
+    if (!copied) {
+      list = new ArrayList<>();
+      copied = true;
+    }
+  }
+
+  @Override
+  public boolean equals(Object o) {
+    return list.equals(o);
+  }
+
+  @Override
+  public int hashCode() {
+    return list.hashCode();
+  }
+
+  @Override
+  public E get(int index) {
+    return list.get(index);
+  }
+
+  @Override
+  public E set(int index, E element) {
+    checkCopyOnWrite();
+    return list.set(index, element);
+  }
+
+  @Override
+  public void add(int index, E element) {
+    checkCopyOnWrite();
+    list.add(index, element);
+  }
+
+  @Override
+  public E remove(int index) {
+    checkCopyOnWrite();
+    return list.remove(index);
+  }
+
+  @Override
+  public int indexOf(Object o) {
+    return list.indexOf(o);
+  }
+
+  @Override
+  public int lastIndexOf(Object o) {
+    return list.lastIndexOf(o);
+  }
+  
+  private void checkCopyOnWrite() {
+    if (!copied) {
+      synchronized (this) {
+        if (!copied) {
+          list = new ArrayList<>(list);
+          copied = true;
+        }
+      }
+    }
+  }
+}

src/main/java/io/ebeaninternal/api/SpiQuery.java

@@ -1,5 +1,6 @@
 package io.ebeaninternal.api;
 
+import io.ebean.CacheMode;
 import io.ebean.EbeanServer;
 import io.ebean.ExpressionList;
 import io.ebean.OrderBy;
@@ -583,9 +584,9 @@ public interface SpiQuery<T> extends Query<T> {
   boolean isUseBeanCache();
 
   /**
-   * Return true if this query should use/check the query cache.
+   * Return the cache mode if this query should use/check the query cache.
    */
-  boolean isUseQueryCache();
+  CacheMode getUseQueryCache();
 
   /**
    * Return true if the beans from this query should be loaded into the bean

src/main/java/io/ebeaninternal/server/core/DefaultServer.java

@@ -35,6 +35,7 @@ import io.ebean.bean.ObjectGraphNode;
 import io.ebean.bean.PersistenceContext;
 import io.ebean.bean.PersistenceContext.WithOption;
 import io.ebean.cache.ServerCacheManager;
+import io.ebean.common.CopyOnFirstWriteList;
 import io.ebean.config.CurrentTenantProvider;
 import io.ebean.config.EncryptKeyManager;
 import io.ebean.config.ServerConfig;
@@ -99,6 +100,7 @@ import javax.persistence.NonUniqueResultException;
 import javax.persistence.OptimisticLockException;
 import javax.persistence.PersistenceException;
 import javax.sql.DataSource;
+
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Iterator;
@@ -1332,10 +1334,19 @@ public final class DefaultServer implements SpiServer, SpiEbeanServer {
     return findIdsWithCopy(((SpiQuery<T>) query).copy(), t);
   }
 
+  @SuppressWarnings("unchecked")
   @Override
   public <A, T> List<A> findIdsWithCopy(Query<T> query, Transaction t) {
 
     SpiOrmQueryRequest<?> request = createQueryRequest(Type.ID_LIST, query, t);
+    Object result = request.getFromQueryCache();
+    if (result != null) {
+      if (Boolean.FALSE.equals(request.getQuery().isReadOnly())) {
+        return new CopyOnFirstWriteList<>((List<A>) result);
+      } else {
+        return (List<A>) result;
+      }
+    }
     try {
       request.initTransIfRequired();
       return request.findIds();

src/main/java/io/ebeaninternal/server/core/OrmQueryRequest.java

@@ -1,11 +1,13 @@
 package io.ebeaninternal.server.core;
 
+import io.ebean.CacheMode;
 import io.ebean.PersistenceContextScope;
 import io.ebean.QueryIterator;
 import io.ebean.Version;
 import io.ebean.bean.BeanCollection;
 import io.ebean.bean.EntityBean;
 import io.ebean.bean.PersistenceContext;
+import io.ebean.common.CopyOnFirstWriteList;
 import io.ebean.event.BeanFindController;
 import io.ebean.event.BeanQueryAdapter;
 import io.ebean.event.BeanQueryRequest;
@@ -32,6 +34,8 @@ import javax.persistence.PersistenceException;
 import java.sql.SQLException;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -471,12 +475,16 @@ public final class OrmQueryRequest<T> extends BeanRequest implements BeanQueryRe
   @SuppressWarnings("unchecked")
   public Object getFromQueryCache() {
 
-    if (!query.isUseQueryCache()) {
+    if (query.getUseQueryCache() == CacheMode.OFF) {
+      return null;
+    } else {
+      cacheKey = query.queryHash();
+    }
+    
+    if (!query.getUseQueryCache().isGet()) {
       return null;
     }
 
-    cacheKey = query.queryHash();
-
     Object cached = beanDescriptor.queryCacheGet(cacheKey);
 
     if (cached != null && isAuditReads() && readAuditQueryType()) {
@@ -491,6 +499,18 @@ public final class OrmQueryRequest<T> extends BeanRequest implements BeanQueryRe
       }
     }
 
+    if (Boolean.FALSE.equals(query.isReadOnly())) {
+      // return shallow copies if readonly is explicitly set to false
+      if (cached instanceof BeanCollection) {
+        cached = ((BeanCollection<?>)cached).getShallowCopy();
+      } else if (cached instanceof List) {
+        cached = new CopyOnFirstWriteList<>((List)cached);
+      } else if (cached instanceof Set) {
+        cached = new LinkedHashSet<>((Set)cached);
+      } else if (cached instanceof Map) {
+        cached = new LinkedHashMap<>((Map)cached);
+      }
+    }
     return cached;
   }
 

src/main/java/io/ebeaninternal/server/expression/DefaultExpressionList.java

@@ -463,7 +463,7 @@ public class DefaultExpressionList<T> implements SpiExpressionList<T> {
   }
 
   @Override
-  public Query<T> setUseQueryCache(boolean useCache) {
+  public Query<T> setUseQueryCache(CacheMode useCache) {
     return query.setUseQueryCache(useCache);
   }
 

src/main/java/io/ebeaninternal/server/expression/JunctionExpression.java

@@ -742,7 +742,7 @@ class JunctionExpression<T> implements SpiJunction<T>, SpiExpression, Expression
   }
 
   @Override
-  public Query<T> setUseQueryCache(boolean useCache) {
+  public Query<T> setUseQueryCache(CacheMode useCache) {
     return exprList.setUseQueryCache(useCache);
   }
 

src/main/java/io/ebeaninternal/server/query/DefaultOrmQueryEngine.java

@@ -13,7 +13,9 @@ import io.ebeaninternal.server.deploy.BeanDescriptor;
 
 import javax.persistence.PersistenceException;
 import java.sql.SQLException;
+import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 
 /**
@@ -71,7 +73,7 @@ public class DefaultOrmQueryEngine implements OrmQueryEngine {
 
     flushJdbcBatchOnQuery(request);
     int result = queryEngine.findCount(request);
-    if (request.getQuery().isUseQueryCache()) {
+    if (request.getQuery().getUseQueryCache().isPut()) {
       request.putToQueryCache(result);
     }
     return result;
@@ -81,16 +83,28 @@ public class DefaultOrmQueryEngine implements OrmQueryEngine {
   public <A> List<A> findIds(OrmQueryRequest<?> request) {
 
     flushJdbcBatchOnQuery(request);
-    return queryEngine.findIds(request);
+    List<A> result = queryEngine.findIds(request);
+    if (request.getQuery().getUseQueryCache().isPut()) {
+      result = Collections.unmodifiableList(result);
+      request.putToQueryCache(result);
+      if (Boolean.FALSE.equals(request.getQuery().isReadOnly())) {
+        result = new ArrayList<>(result);
+      }
+    }
+    return result;
   }
 
   @Override
   public <A> List<A> findSingleAttributeList(OrmQueryRequest<?> request) {
     flushJdbcBatchOnQuery(request);
     List<A> result = queryEngine.findSingleAttributeList(request);
-    if (!result.isEmpty() && request.getQuery().isUseQueryCache()) {
+    if (!result.isEmpty() && request.getQuery().getUseQueryCache().isPut()) {
       // load the query result into the query cache
+      result = Collections.unmodifiableList(result);
       request.putToQueryCache(result);
+      if (Boolean.FALSE.equals(request.getQuery().isReadOnly())) {
+        result = new ArrayList<>(result);
+      }
     }
     return result;
   }
@@ -137,9 +151,13 @@ public class DefaultOrmQueryEngine implements OrmQueryEngine {
       }
     }
 
-    if (!result.isEmpty() && query.isUseQueryCache()) {
+    if (!result.isEmpty() && query.getUseQueryCache().isPut()) {
       // load the query result into the query cache
+      result.setReadOnly(true);
       request.putToQueryCache(result);
+      if (Boolean.FALSE.equals(query.isReadOnly())) {
+        result = result.getShallowCopy();
+      }
     }
 
     return result;

src/main/java/io/ebeaninternal/server/querydefn/DefaultOrmQuery.java

@@ -1,5 +1,6 @@
 package io.ebeaninternal.server.querydefn;
 
+import io.ebean.CacheMode;
 import io.ebean.EbeanServer;
 import io.ebean.Expression;
 import io.ebean.ExpressionFactory;
@@ -195,7 +196,7 @@ public class DefaultOrmQuery<T> implements SpiQuery<T> {
 
   private boolean excludeBeanCache;
 
-  private Boolean useQueryCache;
+  private CacheMode useQueryCache = CacheMode.OFF;
 
   private Boolean readOnly;
 
@@ -1094,9 +1095,13 @@ public class DefaultOrmQuery<T> implements SpiQuery<T> {
   }
 
   @Override
-  public boolean isUseQueryCache() {
+  public CacheMode getUseQueryCache() {
     // not using L2 cache for asDraft() query
-    return !isAsDraft() && Boolean.TRUE.equals(useQueryCache);
+    if (isAsDraft()) {
+      return CacheMode.OFF;
+    } else {
+      return useQueryCache;
+    }
   }
 
   @Override
@@ -1106,7 +1111,7 @@ public class DefaultOrmQuery<T> implements SpiQuery<T> {
   }
 
   @Override
-  public DefaultOrmQuery<T> setUseQueryCache(boolean useQueryCache) {
+  public DefaultOrmQuery<T> setUseQueryCache(CacheMode useQueryCache) {
     this.useQueryCache = useQueryCache;
     return this;
   }

src/test/java/org/tests/cache/TestQueryCache.java

@@ -1,6 +1,7 @@
 package org.tests.cache;
 
 import io.ebean.BaseTestCase;
+import io.ebean.CacheMode;
 import io.ebean.Ebean;
 import io.ebean.bean.BeanCollection;
 import io.ebean.cache.ServerCache;
@@ -105,13 +106,13 @@ public class TestQueryCache extends BaseTestCase {
     LoggedSqlCollector.start();
 
     int count0 = Ebean.find(EColAB.class)
-      .setUseQueryCache(true)
+      .setUseQueryCache(CacheMode.ON)
       .where()
       .eq("columnB", "count")
       .findCount();
 
     int count1 = Ebean.find(EColAB.class)
-      .setUseQueryCache(true)
+      .setUseQueryCache(CacheMode.ON)
       .where()
       .eq("columnB", "count")
       .findCount();
@@ -124,7 +125,7 @@ public class TestQueryCache extends BaseTestCase {
     // and now, ensure that we hit the database
     LoggedSqlCollector.start();
     int count2 = Ebean.find(EColAB.class)
-        .setUseQueryCache(false)
+        .setUseQueryCache(CacheMode.OFF)
         .where()
         .eq("columnB", "count")
         .findCount();
@@ -140,13 +141,13 @@ public class TestQueryCache extends BaseTestCase {
     LoggedSqlCollector.start();
 
     int count0 = Ebean.find(EColAB.class)
-      .setUseQueryCache(true)
+      .setUseQueryCache(CacheMode.ON)
       .where()
       .eq("columnB", "abc")
       .findCount();
 
     int count1 = Ebean.find(EColAB.class)
-      .setUseQueryCache(true)
+      .setUseQueryCache(CacheMode.ON)
       .where()
       .eq("columnB", "def")
       .findCount();
@@ -157,6 +158,58 @@ public class TestQueryCache extends BaseTestCase {
     assertThat(sql).hasSize(2); // different queries
     
   }
+
+  @Test
+  public void findCountFirstOnThenRecache() {
+    
+
+    LoggedSqlCollector.start();
+
+    int count0 = Ebean.find(EColAB.class)
+      .setUseQueryCache(CacheMode.ON)
+      .where()
+      .eq("columnB", "uvw")
+      .findCount();
+
+    int count1 = Ebean.find(EColAB.class)
+      .setUseQueryCache(CacheMode.RECACHE)
+      .where()
+      .eq("columnB", "uvw")
+      .findCount();
+
+    List<String> sql = LoggedSqlCollector.stop();
+
+    assertThat(count0).isEqualTo(count1);
+    assertThat(sql).hasSize(2); // try recache as second query - it must fetch it
+    
+  }
+  
+  
+  @Test
+  public void findCountFirstRecacheThenOn() {
+    
+
+    LoggedSqlCollector.start();
+
+    int count0 = Ebean.find(EColAB.class)
+      .setUseQueryCache(CacheMode.RECACHE)
+      .where()
+      .eq("columnB", "xyz")
+      .findCount();
+
+    int count1 = Ebean.find(EColAB.class)
+      .setUseQueryCache(CacheMode.ON)
+      .where()
+      .eq("columnB", "xyz")
+      .findCount();
+
+    List<String> sql = LoggedSqlCollector.stop();
+
+    assertThat(count0).isEqualTo(count1);
+    assertThat(sql).hasSize(1); // try recache as first query - second "ON" query must fetch it.
+    
+  }
+
   @Test
   @SuppressWarnings("unchecked")
   public void test() {
@@ -170,7 +223,7 @@ public class TestQueryCache extends BaseTestCase {
       .ilike("name", "Rob").findList();
 
     BeanCollection<Customer> bc = (BeanCollection<Customer>) list;
-    Assert.assertFalse(bc.isReadOnly());
+    Assert.assertTrue(bc.isReadOnly());
     Assert.assertFalse(bc.isEmpty());
     Assert.assertTrue(!list.isEmpty());
     Assert.assertTrue(Ebean.getBeanState(list.get(0)).isReadOnly());
@@ -188,19 +241,60 @@ public class TestQueryCache extends BaseTestCase {
     Assert.assertSame(list, list2B);
 
 
-    // TODO: At this stage setReadOnly(false) does not
-    // create a shallow copy of the List/Set/Map
+   
 
-//    List<Customer> list3 = Ebean.find(Customer.class).setUseQueryCache(true).setReadOnly(false).where()
-//        .ilike("name", "Rob").findList();
-//
-//    Assert.assertNotSame(list, list3);
-//    BeanCollection<Customer> bc3 = (BeanCollection<Customer>) list3;
-//    Assert.assertFalse(bc3.isReadOnly());
-//    Assert.assertFalse(bc3.isEmpty());
-//    Assert.assertTrue(list3.size() > 0);
-//    Assert.assertFalse(Ebean.getBeanState(list3.get(0)).isReadOnly());
+    List<Customer> list3 = Ebean.find(Customer.class).setUseQueryCache(true).setReadOnly(false).where()
+        .ilike("name", "Rob").findList();
+
+    Assert.assertNotSame(list, list3);
+    BeanCollection<Customer> bc3 = (BeanCollection<Customer>) list3;
+    Assert.assertFalse(bc3.isReadOnly());
+    Assert.assertFalse(bc3.isEmpty());
+    Assert.assertTrue(list3.size() > 0);
+    // TODO: At this stage setReadOnly(false) does create a shallow copy of the List/Set/Map, but does not
+    // change the read only state in the entities.
+    // Assert.assertFalse(Ebean.getBeanState(list3.get(0)).isReadOnly());
 
   }
 
+  @Test
+  public void findIds() {
+
+    new EColAB("03", "someId").save();
+    new EColAB("04", "someId").save();
+    new EColAB("05", "someId").save();
+    
+    
+    LoggedSqlCollector.start();
+
+    List<Integer> colA_first = Ebean.find(EColAB.class)
+      .setUseQueryCache(CacheMode.ON)
+      .where()
+      .eq("columnB", "someId")
+      .findIds();
+
+    List<Integer> colA_second = Ebean.find(EColAB.class)
+      .setUseQueryCache(CacheMode.ON)
+      .where()
+      .eq("columnB", "someId")
+      .findIds();
+
+    List<String> sql = LoggedSqlCollector.stop();
+
+    assertThat(colA_first).isSameAs(colA_second);
+    assertThat(colA_first).hasSize(3);
+    assertThat(sql).hasSize(1);
+    
+    // and now, ensure that we hit the database
+    LoggedSqlCollector.start();
+    colA_second = Ebean.find(EColAB.class)
+        .setUseQueryCache(CacheMode.RECACHE)
+        .where()
+        .eq("columnB", "someId")
+        .findIds();
+    sql = LoggedSqlCollector.stop();
+    
+    assertThat(sql).hasSize(1);
+  }
+  
 }

src/test/java/org/tests/cache/TestQueryCacheReadOnly.java

@@ -0,0 +1,111 @@
+package org.tests.cache;
+
+import io.ebean.BaseTestCase;
+import io.ebean.CacheMode;
+import io.ebean.Ebean;
+import io.ebean.EbeanServer;
+import io.ebean.Query;
+
+import org.tests.model.basic.EBasicVer;
+import org.junit.Test;
+
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import static org.assertj.core.api.Assertions.*;
+
+public class TestQueryCacheReadOnly extends BaseTestCase {
+
+  @Test
+  public void testReadOnly() {
+
+    EbeanServer server = Ebean.getServer(null);
+    EBasicVer account = new EBasicVer("an other junk");
+    server.save(account);
+    
+    Query<EBasicVer> baseQuery = server.find(EBasicVer.class).setUseQueryCache(CacheMode.ON);
+
+    List<EBasicVer> alist = baseQuery.findList();
+    assertThat(alist).isNotEmpty();
+    assertThatThrownBy(alist::clear).hasMessageContaining("This collection is in ReadOnly mode");
+    alist = baseQuery.findList();
+    assertThat(alist).isNotEmpty();
+    assertThatThrownBy(alist::clear).hasMessageContaining("This collection is in ReadOnly mode");
+
+    Map<String,EBasicVer> amap = baseQuery.setMapKey("name").findMap();
+    assertThat(amap).isNotEmpty();
+    assertThatThrownBy(amap::clear).hasMessageContaining("This collection is in ReadOnly mode");
+    amap = baseQuery.setMapKey("name").findMap();
+    assertThat(amap).isNotEmpty();
+    assertThatThrownBy(amap::clear).hasMessageContaining("This collection is in ReadOnly mode");
+
+    Set<EBasicVer> aset = baseQuery.findSet();
+    assertThat(aset).isNotEmpty();
+    assertThatThrownBy(aset::clear).hasMessageContaining("This collection is in ReadOnly mode");
+    aset = baseQuery.findSet();
+    assertThat(aset).isNotEmpty();
+    assertThatThrownBy(aset::clear).hasMessageContaining("This collection is in ReadOnly mode");
+
+    // we will get an unmodifiable collection here
+    List<Object> attributeList = baseQuery.select("name").findSingleAttributeList();
+    assertThat(attributeList).isNotEmpty();
+    assertThatThrownBy(attributeList::clear).isInstanceOf(UnsupportedOperationException.class);
+    attributeList = baseQuery.select("name").findSingleAttributeList();
+    assertThat(attributeList).isNotEmpty();
+    assertThatThrownBy(attributeList::clear).isInstanceOf(UnsupportedOperationException.class);
+
+    List<Object> idList = baseQuery.select("name").findIds();
+    assertThat(idList).isNotEmpty();
+    assertThatThrownBy(idList::clear).isInstanceOf(UnsupportedOperationException.class);
+    idList = baseQuery.select("name").findIds();
+    assertThat(idList).isNotEmpty();
+    assertThatThrownBy(idList::clear).isInstanceOf(UnsupportedOperationException.class);
+  }
+
+
+  @Test
+  public void testNotReadOnly() {
+
+    EbeanServer server = Ebean.getServer(null);
+    EBasicVer account = new EBasicVer("an other junk");
+    server.save(account);
+
+    Query<EBasicVer> baseQuery = server.find(EBasicVer.class).setUseQueryCache(CacheMode.ON).setReadOnly(false);
+
+    List<EBasicVer> alist = baseQuery.findList();
+    assertThat(alist).isNotEmpty();
+    alist.clear();
+    alist = baseQuery.findList();
+    assertThat(alist).isNotEmpty();
+    alist.clear();
+
+    Map<String,EBasicVer> amap = baseQuery.setMapKey("name").findMap();
+    assertThat(amap).isNotEmpty();
+    amap.clear(); 
+    amap = baseQuery.setMapKey("name").findMap();
+    assertThat(amap).isNotEmpty();
+    amap.clear(); 
+
+    Set<EBasicVer> aset = baseQuery.findSet();
+    assertThat(aset).isNotEmpty();
+    aset.clear();
+    aset = baseQuery.findSet();
+    assertThat(aset).isNotEmpty();
+    aset.clear();
+
+    List<Object> attributeList = baseQuery.select("name").findSingleAttributeList();
+    assertThat(attributeList).isNotEmpty();
+    attributeList.clear();
+    attributeList = baseQuery.select("name").findSingleAttributeList();
+    assertThat(attributeList).isNotEmpty();
+    attributeList.clear();
+
+    List<Object> idList = baseQuery.select("name").findIds();
+    assertThat(idList).isNotEmpty();
+    idList.clear();
+    idList = baseQuery.select("name").findIds();
+    assertThat(idList).isNotEmpty();
+    idList.clear();
+  }
+}