Using Query Cache now implies unmodifiable

- Query Cache now holds unmodifiable collections
- Can no longer have readOnly=false with queryCache=true
- Reference beans now also honor unmodifiable
- Effectively no longer does bean cache lookup for reference beans (which it defaulted to when cacheSharableBeans true, e.g. Country entity bean)

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

@@ -620,7 +620,7 @@ public final class DefaultServer implements SpiServer, SpiEbeanServer {
     }
     InheritInfo inheritInfo = desc.inheritInfo();
     if (inheritInfo == null || inheritInfo.isConcrete()) {
-      return (T) desc.contextRef(pc, null, false, id);
+      return (T) desc.contextRef(pc, id);
     }
     return referenceFindOne(type, id, desc);
   }

ebean-core/src/main/java/io/ebeaninternal/server/core/OrmQueryEngine.java

@@ -25,9 +25,9 @@ public interface OrmQueryEngine {
   <T> T findId(OrmQueryRequest<T> request);
 
   /**
-   * Execute the findList, findSet, findMap query returning an appropriate BeanCollection.
+   * Execute the findList, findSet, findMap query returning an appropriate Collection.
    */
-  <T> BeanCollection<T> findMany(OrmQueryRequest<T> request);
+  <T> Object findMany(OrmQueryRequest<T> request);
 
   /**
    * Execute the findSingleAttributeCollection query.

ebean-core/src/main/java/io/ebeaninternal/server/deploy/AssocOneHelp.java

@@ -66,9 +66,8 @@ abstract class AssocOneHelp {
     if (existing != null) {
       return existing;
     }
-    boolean disableLazyLoading = ctx.isDisableLazyLoading();
-    Object ref = target.contextRef(pc, ctx.isReadOnly(), disableLazyLoading, id);
-    if (!disableLazyLoading) {
+    Object ref = target.contextRef(pc, id, ctx.unmodifiable(), ctx.isDisableLazyLoading());
+    if (!ctx.unmodifiable() && !ctx.isDisableLazyLoading()) {
       ctx.register(path, ((EntityBean) ref)._ebean_getIntercept());
     }
     return ref;

ebean-core/src/main/java/io/ebeaninternal/server/deploy/AssocOneHelpRefInherit.java

@@ -53,9 +53,8 @@ final class AssocOneHelpRefInherit extends AssocOneHelp {
       return existing;
     }
     // for inheritance hierarchy create the correct type for this row...
-    boolean disableLazyLoading = ctx.isDisableLazyLoading();
-    Object ref = desc.contextRef(pc, ctx.isReadOnly(), disableLazyLoading, id);
-    if (!disableLazyLoading) {
+    Object ref = desc.contextRef(pc, id, ctx.unmodifiable(), ctx.isDisableLazyLoading());
+    if (!ctx.unmodifiable() && !ctx.isDisableLazyLoading()) {
       ctx.registerBeanInherit(property, ((EntityBean) ref)._ebean_getIntercept());
     }
     return ref;

ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java

@@ -1781,10 +1781,10 @@ public class BeanDescriptor<T> implements BeanType<T>, STreeType, SpiBeanType {
   /**
    * Create a reference with a check for the bean in the persistence context.
    */
-  public EntityBean createReference(Boolean readOnly, Object id, PersistenceContext pc) {
+  public EntityBean createReference(PersistenceContext pc, Object id) {
     Object refBean = contextGet(pc, id);
     if (refBean == null) {
-      refBean = createReference(readOnly, false, id, pc);
+      refBean = createReference(false, false, id, pc);
     }
     return (EntityBean) refBean;
   }
@@ -1793,8 +1793,8 @@ public class BeanDescriptor<T> implements BeanType<T>, STreeType, SpiBeanType {
    * Create a reference bean based on the id.
    */
   @SuppressWarnings("unchecked")
-  public T createReference(Boolean readOnly, boolean disableLazyLoad, Object id, PersistenceContext pc) {
-    if (cacheSharableBeans && !disableLazyLoad && !Boolean.FALSE.equals(readOnly)) {
+  public T createReference(boolean unmodifiable, boolean disableLazyLoad, Object id, PersistenceContext pc) {
+    if (cacheSharableBeans && unmodifiable) {
       CachedBeanData d = cacheHelp.beanCacheGetData(cacheKey(id));
       if (d != null) {
         Object shareableBean = d.getSharableBean();
@@ -1810,18 +1810,18 @@ public class BeanDescriptor<T> implements BeanType<T>, STreeType, SpiBeanType {
       if (inheritInfo != null && !inheritInfo.isConcrete()) {
         return findReferenceBean(id, pc);
       }
-      EntityBean eb = createEntityBean();
+      EntityBean eb = createEntityBean2(unmodifiable);
       id = convertSetId(id, eb);
       EntityBeanIntercept ebi = eb._ebean_getIntercept();
       if (disableLazyLoad) {
         ebi.setDisableLazyLoad(true);
-      } else {
+      } else if (!unmodifiable) {
         ebi.setBeanLoader(refBeanLoader());
       }
       ebi.setReference(idPropertyIndex);
-      if (Boolean.TRUE == readOnly) {
-        ebi.setReadOnly(true);
-      }
+     // if (Boolean.TRUE == readOnly) {
+     //   ebi.setReadOnly(true);
+     // }
       if (pc != null) {
         contextPut(pc, id, eb);
         ebi.setPersistenceContext(pc);
@@ -2046,11 +2046,12 @@ public class BeanDescriptor<T> implements BeanType<T>, STreeType, SpiBeanType {
     return pc.putIfAbsent(rootBeanType, id, localBean);
   }
 
-  /**
-   * Create a reference bean and put it in the persistence context (and return it).
-   */
-  public Object contextRef(PersistenceContext pc, Boolean readOnly, boolean disableLazyLoad, Object id) {
-    return createReference(readOnly, disableLazyLoad, id, pc);
+  public Object contextRef(PersistenceContext pc, Object id) {
+    return createReference(false, false, id, pc);
+  }
+
+  public Object contextRef(PersistenceContext pc, Object id, boolean unmodifiable, boolean disableLazyLoad) {
+    return createReference(unmodifiable, disableLazyLoad, id, pc);
   }
 
   /**

ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptorCacheHelp.java

@@ -280,7 +280,7 @@ final class BeanDescriptorCacheHelp<T> {
     bc.checkEmptyLazyLoad();
     int i = 0;
     for (Object id : idList) {
-      final EntityBean ref = targetDescriptor.createReference(readOnly, id, persistenceContext);
+      final EntityBean ref = targetDescriptor.createReference(persistenceContext, id);
       if (many.hasOrderColumn()) {
         ref._ebean_getIntercept().setSortOrder(++i);
       }

ebean-core/src/main/java/io/ebeaninternal/server/deploy/DbReadContext.java

@@ -107,4 +107,9 @@ public interface DbReadContext {
    * query via a secondary query.
    */
   boolean includeSecondary(BeanPropertyAssocMany<?> many);
+
+  /**
+   * Return true if we are loading unmodifiable beans.
+   */
+  boolean unmodifiable();
 }

ebean-core/src/main/java/io/ebeaninternal/server/deploy/InheritInfo.java

@@ -228,8 +228,8 @@ public final class InheritInfo {
   /**
    * Create an EntityBean for this type.
    */
-  public EntityBean createEntityBean() {
-    return descriptor.createEntityBean();
+  public EntityBean createEntityBean(boolean unmodifiable) {
+    return descriptor.createEntityBean2(unmodifiable);
   }
 
   /**

ebean-core/src/main/java/io/ebeaninternal/server/deploy/id/IdBinderSimple.java

@@ -245,7 +245,8 @@ public final class IdBinderSimple implements IdBinder {
       idValue = scalarType.toBeanType(idValue);
     }
     if (bean != null) {
-      idProperty.setValueIntercept(bean, idValue);
+      // not using interception to support unmodifiable entities
+      idProperty.setValue(bean, idValue);
     }
     return idValue;
   }

ebean-core/src/main/java/io/ebeaninternal/server/query/CQuery.java

@@ -248,6 +248,11 @@ public final class CQuery<T> implements DbReadContext, CancelableQuery, SpiProfi
     return readOnly;
   }
 
+  @Override
+  public boolean unmodifiable() {
+    return unmodifiable;
+  }
+
   @Override
   public void propagateState(Object e) {
     if (Boolean.TRUE.equals(readOnly)) {

ebean-core/src/main/java/io/ebeaninternal/server/query/DefaultOrmQueryEngine.java

@@ -114,7 +114,7 @@ public final class DefaultOrmQueryEngine implements OrmQueryEngine {
   }
 
   @Override
-  public <T> BeanCollection<T> findMany(OrmQueryRequest<T> request) {
+  public <T> Object findMany(OrmQueryRequest<T> request) {
     flushJdbcBatchOnQuery(request);
     BeanFindController finder = request.finder();
 
@@ -129,22 +129,21 @@ public final class DefaultOrmQueryEngine implements OrmQueryEngine {
       result = finder.postProcessMany(request, result);
     }
 
-    SpiQuery<T> query = request.query();
     if (result != null && request.isBeanCachePutMany()) {
       // load the individual beans into the bean cache
       request.descriptor().cacheBeanPutAll(result.actualDetails());
     }
 
     request.mergeCacheHits(result);
-    if (request.isQueryCachePut()) {
-      // load the query result into the query cache
-      result.setReadOnly(true);
-      request.putToQueryCache(result);
-      if (Boolean.FALSE.equals(query.isReadOnly())) {
-        result = result.shallowCopy();
+    Object finalResult = result;
+    if (request.query().isUnmodifiable()) {
+      finalResult = result == null ? null : result.freeze();
+      if (request.isQueryCachePut()) {
+        // load the query result into the query cache
+        request.putToQueryCache(finalResult);
       }
     }
-    return result;
+    return finalResult;
   }
 
   /**

ebean-core/src/main/java/io/ebeaninternal/server/query/SqlTreeLoadBean.java

@@ -111,7 +111,7 @@ class SqlTreeLoadBean implements SqlTreeLoad {
         localIdBinder = idBinder;
         localDesc = desc;
       } else {
-        localBean = localInfo.createEntityBean();
+        localBean = localInfo.createEntityBean(unmodifiable);
         localType = localInfo.getType();
         localIdBinder = localInfo.getIdBinder();
         localDesc = localInfo.desc();

ebean-core/src/main/java/io/ebeaninternal/server/querydefn/DefaultOrmQuery.java

@@ -1307,6 +1307,9 @@ public class DefaultOrmQuery<T> extends AbstractQuery implements SpiQuery<T> {
 
   @Override
   public final Query<T> setReadOnly(boolean readOnly) {
+    if (unmodifiable && !readOnly) {
+      throw new IllegalStateException("Not allowed to set readOnly false on query that is unmodifiable");
+    }
     this.readOnly = readOnly;
     return this;
   }
@@ -1352,6 +1355,9 @@ public class DefaultOrmQuery<T> extends AbstractQuery implements SpiQuery<T> {
   @Override
   public final Query<T> setUseQueryCache(CacheMode useQueryCache) {
     this.useQueryCache = useQueryCache;
+    if (CacheMode.OFF != useQueryCache) {
+      unmodifiable = true;
+    }
     return this;
   }
 

ebean-core/src/test/java/io/ebeaninternal/server/deploy/BeanDescriptorTest.java

@@ -28,7 +28,7 @@ public class BeanDescriptorTest extends BaseTest {
   @Test
   public void createReference() {
 
-    Customer bean = customerDesc.createReference(null, false, 42, null);
+    Customer bean = customerDesc.createReference(false, false, 42, null);
     assertThat(bean.getId()).isEqualTo(42);
     Assertions.assertThat(server().beanState(bean).isReadOnly()).isFalse();
   }
@@ -36,14 +36,14 @@ public class BeanDescriptorTest extends BaseTest {
   @Test
   public void createReference_whenReadOnly() {
 
-    Customer bean = customerDesc.createReference(Boolean.TRUE, false, 42, null);
+    Customer bean = customerDesc.createReference(true, false, 42, null);
     Assertions.assertThat(server().beanState(bean).isReadOnly()).isTrue();
   }
 
   @Test
   public void createReference_whenNotReadOnly() {
 
-    Customer bean = customerDesc.createReference(Boolean.FALSE, false, 42, null);
+    Customer bean = customerDesc.createReference(false, false, 42, null);
     Assertions.assertThat(server().beanState(bean).isReadOnly()).isFalse();
 
     bean = customerDesc.createReference(42, null);
@@ -53,7 +53,7 @@ public class BeanDescriptorTest extends BaseTest {
   @Test
   public void createReference_when_disabledLazyLoad() {
 
-    Customer bean = customerDesc.createReference(Boolean.FALSE, true, 42, null);
+    Customer bean = customerDesc.createReference(false, true, 42, null);
     Assertions.assertThat(server().beanState(bean).isDisableLazyLoad()).isTrue();
   }
 
@@ -78,7 +78,7 @@ public class BeanDescriptorTest extends BaseTest {
 
     BeanDescriptor<Animal> animalDesc = spiEbeanServer().descriptor(Animal.class);
 
-    Animal bean = animalDesc.createReference(Boolean.FALSE, false, dog.getId(), null);
+    Animal bean = animalDesc.createReference(false, false, dog.getId(), null);
     assertThat(bean.getId()).isEqualTo(dog.getId());
   }
 

ebean-test/src/test/java/org/tests/basic/TestQueryWithCache.java

@@ -12,12 +12,13 @@ import org.junit.jupiter.api.Test;
 import org.tests.model.basic.Country;
 import org.tests.model.basic.ResetBasicData;
 
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.*;
 
-public class TestQueryWithCache extends BaseTestCase {
+class TestQueryWithCache extends BaseTestCase {
 
   @Test
-  public void testCountryDeploy() {
+  void testCountryDeploy() {
 
     ResetBasicData.reset();
 
@@ -50,15 +51,13 @@ public class TestQueryWithCache extends BaseTestCase {
     Country nz4 = DB.find(Country.class).setId("NZ").setAutoTune(false).setUseCache(false)
       .findOne();
 
-    assertTrue(nz2 == nz2b);
-    assertTrue(nz2 == nz3);
-    assertTrue(nz3 != nz4);
-
+    assertThat(nz2).isNotSameAs(nz2b); // Changed behaviour with unmodifiable
+    assertThat(nz2).isNotSameAs(nz3); // Changed behaviour with unmodifiable
+    assertThat(nz3).isNotSameAs(nz4);
   }
 
   @Test
-  public void testSkipCache() {
-
+  void testSkipCache() {
     ResetBasicData.reset();
 
     DB.find(Country.class, "NZ");

ebean-test/src/test/java/org/tests/cache/TestCacheBasic.java

@@ -29,7 +29,7 @@ public class TestCacheBasic extends BaseTestCase {
     Country c0 = DB.reference(Country.class, "NZ");
     ServerCacheStatistics statistics = countryCache.statistics(false);
     long hc = statistics.getHitCount();
-    assertEquals(1, hc);
+    assertEquals(0, hc); // Change behaviour, reference() no longer hits cache with unmodifiable
     assertNotNull(c0);
 
     // Country c1 = DB.reference(Country.class, "NZ");

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

@@ -5,7 +5,6 @@ import io.ebean.DB;
 import io.ebean.ExpressionList;
 import io.ebean.annotation.Transactional;
 import io.ebean.annotation.TxIsolation;
-import io.ebean.bean.BeanCollection;
 import io.ebean.cache.ServerCache;
 import io.ebean.test.LoggedSql;
 import io.ebean.xtest.BaseTestCase;
@@ -304,46 +303,39 @@ public class TestQueryCache extends BaseTestCase {
 
   @Test
   @SuppressWarnings("unchecked")
-  public void testReadOnlyFind() {
-
+  void testReadOnlyFind() {
     ResetBasicData.reset();
 
     ServerCache customerCache = DB.cacheManager().queryCache(Customer.class);
     customerCache.clear();
 
-    List<Customer> list = DB.find(Customer.class).setUseQueryCache(true).setReadOnly(true).where()
-      .ilike("name", "Rob").findList();
+    List<Customer> list = DB.find(Customer.class).setUnmodifiable(true) //.setUseQueryCache(true) //.setReadOnly(true)
+      .where().ilike("name", "Rob")
+      .findList();
 
-    BeanCollection<Customer> bc = (BeanCollection<Customer>) list;
-    assertTrue(bc.isReadOnly());
-    assertFalse(bc.isEmpty());
-    assertTrue(!list.isEmpty());
-    assertTrue(DB.beanState(list.get(0)).isReadOnly());
 
-    List<Customer> list2 = DB.find(Customer.class).setUseQueryCache(true).setReadOnly(true).where()
-      .ilike("name", "Rob").findList();
+    assertThat(list).isNotEmpty();
+    assertThat(DB.beanState(list.get(0)).isReadOnly()).isTrue();
+
+    List<Customer> list2 = DB.find(Customer.class).setUseQueryCache(true).setReadOnly(true)
+      .where().ilike("name", "Rob")
+      .findList();
 
     List<Customer> list2B = DB.find(Customer.class).setUseQueryCache(true)
-      // .setReadOnly(true)
-      .where().ilike("name", "Rob").findList();
+      .where().ilike("name", "Rob")
+      .findList();
 
-    assertSame(list, list2);
+    assertThat(list2).isEqualTo(list);
 
     // readOnly defaults to true for query cache
-    assertSame(list, list2B);
+    assertSame(list2, list2B);
 
-    List<Customer> list3 = DB.find(Customer.class).setUseQueryCache(true).setReadOnly(false).where()
-      .ilike("name", "Rob").findList();
-
-    assertNotSame(list, list3);
-    BeanCollection<Customer> bc3 = (BeanCollection<Customer>) list3;
-    assertFalse(bc3.isReadOnly());
-    assertFalse(bc3.isEmpty());
-    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.
-    // assertFalse(DB.beanState(list3.get(0)).isReadOnly());
+    List<Customer> list3 = DB.find(Customer.class).setUseQueryCache(true)//.setReadOnly(false)
+      .where().ilike("name", "Rob")
+      .findList();
 
+    assertSame(list2, list3);
+    assertThat(list3).isNotEmpty();
   }
 
   @Test

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

@@ -68,41 +68,41 @@ public class TestQueryCacheReadOnly extends BaseTestCase {
     EBasicVer account = new EBasicVer("an other junk");
     server.save(account);
 
-    Query<EBasicVer> baseQuery = server.find(EBasicVer.class).setUseQueryCache(CacheMode.ON).setReadOnly(false);
+    Query<EBasicVer> baseQuery = server.find(EBasicVer.class).setUseQueryCache(CacheMode.ON);
 
     List<EBasicVer> alist = baseQuery.findList();
     assertThat(alist).isNotEmpty();
-    alist.clear();
+    assertThatThrownBy(alist::clear).isInstanceOf(UnsupportedOperationException.class);
     alist = baseQuery.findList();
     assertThat(alist).isNotEmpty();
-    alist.clear();
+    assertThatThrownBy(alist::clear).isInstanceOf(UnsupportedOperationException.class);
 
     Map<String,EBasicVer> amap = baseQuery.setMapKey("name").findMap();
     assertThat(amap).isNotEmpty();
-    amap.clear();
+    assertThatThrownBy(amap::clear).isInstanceOf(UnsupportedOperationException.class);
     amap = baseQuery.setMapKey("name").findMap();
     assertThat(amap).isNotEmpty();
-    amap.clear();
+    assertThatThrownBy(amap::clear).isInstanceOf(UnsupportedOperationException.class);
 
     Set<EBasicVer> aset = baseQuery.findSet();
     assertThat(aset).isNotEmpty();
-    aset.clear();
+    assertThatThrownBy(aset::clear).isInstanceOf(UnsupportedOperationException.class);
     aset = baseQuery.findSet();
     assertThat(aset).isNotEmpty();
-    aset.clear();
+    assertThatThrownBy(aset::clear).isInstanceOf(UnsupportedOperationException.class);
 
-    List<Object> attributeList = baseQuery.select("name").findSingleAttributeList();
+    final List<Object> attributeList = baseQuery.select("name").findSingleAttributeList();
     assertThat(attributeList).isNotEmpty();
-    attributeList.clear();
-    attributeList = baseQuery.select("name").findSingleAttributeList();
-    assertThat(attributeList).isNotEmpty();
-    attributeList.clear();
+    assertThatThrownBy(attributeList::clear).isInstanceOf(UnsupportedOperationException.class);
+    final List<Object> attributeList2 = baseQuery.select("name").findSingleAttributeList();
+    assertThat(attributeList2).isNotEmpty();
+    assertThatThrownBy(attributeList2::clear).isInstanceOf(UnsupportedOperationException.class);
 
     List<Object> idList = baseQuery.select("name").findIds();
     assertThat(idList).isNotEmpty();
-    idList.clear();
-    idList = baseQuery.select("name").findIds();
-    assertThat(idList).isNotEmpty();
-    idList.clear();
+    assertThatThrownBy(idList::clear).isInstanceOf(UnsupportedOperationException.class);
+    List<Object> idList2 = baseQuery.select("name").findIds();
+    assertThat(idList2).isNotEmpty();
+    assertThatThrownBy(idList2::clear).isInstanceOf(UnsupportedOperationException.class);
   }
 }

ebean-test/src/test/java/org/tests/model/lazywithid/Tune.java

@@ -1,7 +1,5 @@
 package org.tests.model.lazywithid;
 
-import io.ebean.common.BeanList;
-
 import jakarta.persistence.*;
 import java.util.List;
 
@@ -15,7 +13,7 @@ public class Tune {
   String name;
 
   @OneToMany(cascade = CascadeType.ALL)
-  private List<Looney> loonies = new BeanList<>();
+  private List<Looney> loonies;
 
   public String getName() {
     return name;

ebean-test/src/test/java/org/tests/readaudit/TestReadAudit.java

@@ -132,8 +132,8 @@ public class TestReadAudit extends BaseTestCase {
     assertThat(readAuditLogger.beans).hasSize(2);
 
     Country ref = server.reference(Country.class, "AR");
-    assertThat(readAuditLogger.beans).hasSize(3);
-    assertThat(ref).isSameAs(found2);
+    assertThat(readAuditLogger.beans).hasSize(2);
+    assertThat(ref).isNotSameAs(found2);
   }
 
   @Test
@@ -197,9 +197,9 @@ public class TestReadAudit extends BaseTestCase {
       + " plans:" + readAuditLogger.plans
       + " many:" + readAuditLogger.many);
 
-    assertThat(readAuditPrepare.count).isEqualTo(2);
+    assertThat(readAuditPrepare.count).isEqualTo(1);
     assertThat(readAuditLogger.plans).hasSize(1);
-    assertThat(readAuditLogger.many).hasSize(2);
+    assertThat(readAuditLogger.many).hasSize(1);
   }
 
   @Test