Add explicit query.setUnmodifiable(true) [for readOnly=true + disableLazyLoad=true]

Although this is ok, makes me think that another option is just to have query.setReadOnly(true)
to mean ... readOnly + disableLazyLoad + error reading unloaded property or collection. As in,
readOnly true without these extra things does not that good [as in the existing readOnly does
not seem very good/safe/useful to use].

ebean-api/src/main/java/io/ebean/QueryBuilder.java

@@ -387,6 +387,11 @@ public interface QueryBuilder<SELF extends QueryBuilder<SELF, T>, T> extends Que
    */
   SELF setUseDocStore(boolean useDocStore);
 
+  /**
+   * When set to true when you want the returned beans to be read only.
+   */
+  SELF setUnmodifiable(boolean unmodifiable);
+
   /**
    * When set to true when you want the returned beans to be read only.
    */

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

@@ -240,4 +240,9 @@ public interface BeanCollection<E> extends Serializable, ToStringAware {
    * Return a shallow copy of this collection that is modifiable.
    */
   BeanCollection<E> shallowCopy();
+
+  /**
+   * Freeze the collection returning an unmodifiable version.
+   */
+  Object freeze();
 }

ebean-api/src/main/java/io/ebean/bean/EntityBeanIntercept.java

@@ -29,12 +29,6 @@ public interface EntityBeanIntercept extends Serializable {
    */
   void setPersistenceContext(PersistenceContext persistenceContext);
 
-  /**
-   * When accessing a property that is not loaded, throw a IllegalStateException
-   * when true or invoke lazy loading.
-   */
-  void errorOnLazyLoad(boolean lazyLoadAsError);
-
   /**
    * Turn on profile collection.
    */

ebean-api/src/main/java/io/ebean/bean/InterceptBase.java

@@ -7,7 +7,6 @@ abstract class InterceptBase implements EntityBeanIntercept {
 
   final EntityBean owner;
   boolean fullyLoadedBean;
-  boolean errorOnLazyLoad;
 
   /**
    * Create with a given entity.
@@ -28,11 +27,6 @@ abstract class InterceptBase implements EntityBeanIntercept {
     return owner;
   }
 
-  @Override
-  public final void errorOnLazyLoad(boolean errorOnLazyLoad) {
-    this.errorOnLazyLoad = errorOnLazyLoad;
-  }
-
   @Override
   public final boolean isFullyLoadedBean() {
     return fullyLoadedBean;

ebean-api/src/main/java/io/ebean/bean/InterceptReadOnly.java

@@ -8,7 +8,7 @@ import java.util.*;
  * EntityBeanIntercept optimised for read only use.
  * <p>
  * For the read only use this intercept doesn't need to hold any state that is normally
- * required for updates such as per property changed, loaded, dirty state, original values
+ * required for updates such as per property changed, dirty state, original values
  * bean state etc.
  */
 public final class InterceptReadOnly extends InterceptBase {
@@ -91,7 +91,7 @@ public final class InterceptReadOnly extends InterceptBase {
   @Override
   public boolean isPartial() {
     for (boolean flag : flags) {
-      if (flag) {
+      if (!flag) {
         return true;
       }
     }
@@ -412,9 +412,8 @@ public final class InterceptReadOnly extends InterceptBase {
 
   @Override
   public void preGetter(int propertyIndex) {
-    if (errorOnLazyLoad && !flags[propertyIndex]) {
-      final String property = property(propertyIndex);
-      throw new IllegalStateException("Property not loaded: " + property);
+    if (!flags[propertyIndex]) {
+      throw new IllegalStateException("Property not loaded: " + property(propertyIndex));
     }
   }
 

ebean-api/src/main/java/io/ebean/bean/InterceptReadWrite.java

@@ -650,10 +650,6 @@ public final class InterceptReadWrite extends InterceptBase {
 
   @Override
   public void loadBean(int loadProperty) {
-    if (errorOnLazyLoad) {
-      final String property = property(loadProperty);
-      throw new IllegalStateException("Lazy loading not allowed for " + property);
-    }
     lock.lock();
     try {
       if (beanLoader == null) {
@@ -661,7 +657,7 @@ public final class InterceptReadWrite extends InterceptBase {
         if (database == null) {
           throw new PersistenceException(ebeanServerName == null ? "No registered default server" : "Database [" + ebeanServerName + "] is not registered");
         }
-        // For stand alone reference bean or after deserialisation lazy load
+        // For stand-alone reference bean or after deserialisation lazy load
         // using the ebeanServer. Synchronise only on the bean.
         loadBeanInternal(loadProperty, database.pluginApi().beanLoader());
         return;

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

@@ -39,6 +39,11 @@ public final class BeanList<E> extends AbstractBeanCollection<E> implements List
     super(loader, ownerBean, propertyName);
   }
 
+  @Override
+  public List<E> freeze() {
+    return list == null ? null : Collections.unmodifiableList(list);
+  }
+
   @Override
   public void toString(ToStringBuilder builder) {
     builder.addCollection(list);

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

@@ -37,6 +37,11 @@ public final class BeanMap<K, E> extends AbstractBeanCollection<E> implements Ma
     super(ebeanServer, ownerBean, propertyName);
   }
 
+  @Override
+  public Map<K, E> freeze() {
+    return map == null ? null : Collections.unmodifiableMap(map);
+  }
+
   @Override
   public void toString(ToStringBuilder builder) {
     if (map == null || map.isEmpty()) {

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

@@ -3,10 +3,7 @@ package io.ebean.common;
 import io.ebean.bean.*;
 
 import java.io.Serializable;
-import java.util.Collection;
-import java.util.Iterator;
-import java.util.LinkedHashSet;
-import java.util.Set;
+import java.util.*;
 
 /**
  * Set capable of lazy loading and modification aware.
@@ -38,6 +35,11 @@ public final class BeanSet<E> extends AbstractBeanCollection<E> implements Set<E
     super(loader, ownerBean, propertyName);
   }
 
+  @Override
+  public Set<E> freeze() {
+    return set == null ? null : Collections.unmodifiableSet(set);
+  }
+
   @Override
   public void toString(ToStringBuilder builder) {
     builder.addCollection(set);

ebean-core/src/main/java/io/ebeaninternal/api/SpiQuery.java

@@ -784,6 +784,11 @@ public interface SpiQuery<T> extends Query<T>, SpiQueryFetch, TxnProfileEventCod
    */
   Boolean isReadOnly();
 
+  /**
+   * Return true if the beans returned by this query should be unmodifiable.
+   */
+  boolean isUnmodifiable();
+
   /**
    * Return the query timeout.
    */

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

@@ -687,14 +687,14 @@ public class BeanDescriptor<T> implements BeanType<T>, STreeType, SpiBeanType {
     }
   }
 
+  @Override
   public void freeze(EntityBean entityBean) {
     for (BeanProperty beanProperty : propertiesMutable) {
       beanProperty.freeze(entityBean);
     }
-//    for (BeanPropertyAssocMany<?> many : propertiesMany) {
-//      many.freeze(entityBean);
-//    }
-//    entityBean._ebean_getIntercept().freeze();
+    for (BeanPropertyAssocMany<?> many : propertiesMany) {
+      many.freeze(entityBean);
+    }
   }
 
   public void metricPersistBatch(PersistRequest.Type type, long startNanos, int size) {

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

@@ -1165,4 +1165,15 @@ public class BeanPropertyAssocMany<T> extends BeanPropertyAssoc<T> implements ST
       return false;
     }
   }
+
+  @Override
+  public void freeze(EntityBean entityBean) {
+    Object value = getValue(entityBean);
+    if (value instanceof BeanCollection) {
+      setValue(entityBean, ((BeanCollection<?>) value).freeze());
+    } else if (value == null) {
+      // make it an error to access the collection (no lazy loading allowed)
+      entityBean._ebean_getIntercept().setPropertyUnloaded(propertyIndex);
+    }
+  }
 }

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

@@ -513,6 +513,11 @@ final class DefaultFetchGroupQuery<T> implements SpiFetchGroupQuery<T>, SpiQuery
     throw new RuntimeException("EB102: Only select() and fetch() clause is allowed on FetchGroup");
   }
 
+  @Override
+  public Query<T> setUnmodifiable(boolean unmodifiable) {
+    throw new RuntimeException("EB102: Only select() and fetch() clause is allowed on FetchGroup");
+  }
+
   @Override
   public Query<T> setReadOnly(boolean readOnly) {
     throw new RuntimeException("EB102: Only select() and fetch() clause is allowed on FetchGroup");

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

@@ -34,7 +34,7 @@ public interface STreePropertyAssocMany extends STreePropertyAssoc {
   BeanCollection<?> createReference(EntityBean localBean, boolean forceNewReference);
 
   /**
-   * Populate the collection for read-only disabled lazy loading (aka Java Collections non mutable empty collection).
+   * Populate the collection for read-only disabled lazy loading (aka Java Collections non-mutable empty collection).
    */
   void createEmptyReference(EntityBean localBean);
 

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

@@ -99,6 +99,11 @@ public interface STreeType {
    */
   void postLoad(Object localBean);
 
+  /**
+   * Freeze the properties of the entity.
+   */
+  void freeze(EntityBean entityBean);
+
   /**
    * Return the base table to use given the temporalMode.
    */

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

@@ -45,12 +45,12 @@ public final class SqlTreeBuilder {
    */
   private final boolean rawNoId;
   private final boolean disableLazyLoad;
-  private final boolean readOnly;
   private final SpiQuery.TemporalMode temporalMode;
   private SqlTreeNode rootNode;
   private boolean sqlDistinct;
   private final boolean platformDistinctNoLobs;
   private final SqlTreeCommon common;
+  private final boolean unmodifiable;
 
   /**
    * Construct for RawSql query.
@@ -60,7 +60,7 @@ public final class SqlTreeBuilder {
     this.desc = request.descriptor();
     this.rawNoId = rawNoId;
     this.disableLazyLoad = request.query().isDisableLazyLoading();
-    this.readOnly = Boolean.TRUE.equals(request.query().isReadOnly());
+    this.unmodifiable = request.query().isUnmodifiable();
     this.query = null;
     this.subQuery = false;
     this.distinctOnPlatform = false;
@@ -72,7 +72,7 @@ public final class SqlTreeBuilder {
     this.manyWhereJoins = null;
     this.alias = null;
     this.ctx = null;
-    this.common = new SqlTreeCommon(temporalMode, disableLazyLoad, readOnly, null);
+    this.common = new SqlTreeCommon(temporalMode, disableLazyLoad, unmodifiable, null);
   }
 
   /**
@@ -87,7 +87,7 @@ public final class SqlTreeBuilder {
     this.query = request.query();
     this.temporalMode = SpiQuery.TemporalMode.of(query);
     this.disableLazyLoad = query.isDisableLazyLoading();
-    this.readOnly = Boolean.TRUE.equals(query.isReadOnly());
+    this.unmodifiable = query.isUnmodifiable();
     this.subQuery = Type.SQ_EXISTS == query.type()
       || Type.SQ_EX == query.type()
       || Type.ID_LIST == query.type()
@@ -105,7 +105,7 @@ public final class SqlTreeBuilder {
     CQueryDraftSupport draftSupport = builder.draftSupport(query);
     String colAlias = subQuery ? null : columnAliasPrefix;
     this.ctx = new DefaultDbSqlContext(alias, colAlias, historySupport, draftSupport, fromForUpdate);
-    this.common = new SqlTreeCommon(temporalMode, disableLazyLoad, readOnly, includeJoin);
+    this.common = new SqlTreeCommon(temporalMode, disableLazyLoad, unmodifiable, includeJoin);
   }
 
   /**

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

@@ -7,13 +7,13 @@ final class SqlTreeCommon {
 
   private final SpiQuery.TemporalMode temporalMode;
   private final boolean disableLazyLoad;
-  private final boolean readOnly;
+  private final boolean unmodifiable;
   private final TableJoin includeJoin;
 
-  SqlTreeCommon(SpiQuery.TemporalMode temporalMode, boolean disableLazyLoad, boolean readOnly, TableJoin includeJoin) {
+  SqlTreeCommon(SpiQuery.TemporalMode temporalMode, boolean disableLazyLoad, boolean unmodifiable, TableJoin includeJoin) {
     this.temporalMode = temporalMode;
     this.disableLazyLoad = disableLazyLoad;
-    this.readOnly = readOnly;
+    this.unmodifiable = unmodifiable;
     this.includeJoin = includeJoin;
   }
 
@@ -25,8 +25,8 @@ final class SqlTreeCommon {
     return disableLazyLoad;
   }
 
-  boolean readOnly() {
-    return readOnly;
+  boolean unmodifiable() {
+    return unmodifiable;
   }
 
   TableJoin includeJoin() {

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

@@ -31,7 +31,7 @@ class SqlTreeLoadBean implements SqlTreeLoad {
   final boolean readId;
   private final boolean readIdNormal;
   private final boolean disableLazyLoad;
-  private final boolean readOnlyNoIntercept;
+  private final boolean unmodifiable;
   private final InheritInfo inheritInfo;
   final String prefix;
   private final Map<String, String> pathMap;
@@ -54,7 +54,7 @@ class SqlTreeLoadBean implements SqlTreeLoad {
     this.readId = node.readId;
     this.readIdNormal = readId && !temporalVersions;
     this.disableLazyLoad = node.disableLazyLoad;
-    this.readOnlyNoIntercept = disableLazyLoad && node.readOnly;
+    this.unmodifiable = node.unmodifiable;
     this.partialObject = node.partialObject;
     this.properties = node.properties;
     this.pathMap = node.pathMap;
@@ -162,7 +162,7 @@ class SqlTreeLoadBean implements SqlTreeLoad {
 
     void initBeanType() throws SQLException {
       localDesc = desc;
-      localBean = desc.createEntityBean2(readOnlyNoIntercept);
+      localBean = desc.createEntityBean2(unmodifiable);
       localIdBinder = idBinder;
     }
 
@@ -280,6 +280,9 @@ class SqlTreeLoadBean implements SqlTreeLoad {
           if (!partialObject) {
             ebi.setFullyLoadedBean(true);
           }
+          if (unmodifiable) {
+            localDesc.freeze(localBean);
+          }
         } else if (!partialObject) {
           ebi.setFullyLoadedBean(true);
         } else if (readId && !usingContextBean) {
@@ -302,9 +305,7 @@ class SqlTreeLoadBean implements SqlTreeLoad {
       boolean forceNewReference = queryMode == Mode.REFRESH_BEAN;
       for (STreePropertyAssocMany many : localDesc.propsMany()) {
         if (many != loadingChildProperty) {
-          if (readOnlyNoIntercept) {
-            many.createEmptyReference(localBean);
-          } else {
+          if (!unmodifiable) {
             // create a proxy for the many (deferred fetching)
             BeanCollection<?> ref = many.createReference(localBean, forceNewReference);
             if (ref != null) {

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

@@ -38,7 +38,7 @@ class SqlTreeNodeBean implements SqlTreeNode {
   final boolean readId;
   final boolean readIdNormal;
   final boolean disableLazyLoad;
-  final boolean readOnly;
+  final boolean unmodifiable;
   final InheritInfo inheritInfo;
   final String prefix;
   final Map<String, String> pathMap;
@@ -92,7 +92,7 @@ class SqlTreeNodeBean implements SqlTreeNode {
     this.readId = !aggregationRoot && withId && desc.hasId();
     this.readIdNormal = readId && !temporalVersions;
     this.disableLazyLoad = common.disableLazyLoad() || !readIdNormal || desc.isRawSqlBased();
-    this.readOnly = common.readOnly();
+    this.unmodifiable = common.unmodifiable();
     this.partialObject = props.isPartialObject();
     this.properties = props.props();
     this.children = myChildren == null ? Collections.emptyList() : myChildren;

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

@@ -133,6 +133,7 @@ public class DefaultOrmQuery<T> extends AbstractQuery implements SpiQuery<T> {
   private CacheMode useBeanCache = CacheMode.AUTO;
   private CacheMode useQueryCache = CacheMode.OFF;
   private Boolean readOnly;
+  private boolean unmodifiable;
   private PersistenceContextScope persistenceContextScope;
 
   /**
@@ -514,6 +515,15 @@ public class DefaultOrmQuery<T> extends AbstractQuery implements SpiQuery<T> {
     if (!useDocStore) {
       createExtraJoinsToSupportManyWhereClause();
     }
+    if (unmodifiable) {
+      readOnly = Boolean.TRUE;
+      disableLazyLoading = true;
+      persistenceContextScope = PersistenceContextScope.QUERY;
+    } else if (disableLazyLoading && Boolean.TRUE.equals(readOnly)) {
+      // "upgrade" this to unmodifiable? hmmm.
+      unmodifiable = true;
+      persistenceContextScope = PersistenceContextScope.QUERY;
+    }
     return markQueryJoins();
   }
 
@@ -763,6 +773,7 @@ public class DefaultOrmQuery<T> extends AbstractQuery implements SpiQuery<T> {
     copy.nativeSql = nativeSql;
     copy.useBeanCache = useBeanCache;
     copy.useQueryCache = useQueryCache;
+    copy.unmodifiable = unmodifiable;
     copy.readOnly = readOnly;
     if (detail != null) {
       copy.detail = detail.copy();
@@ -1276,11 +1287,22 @@ public class DefaultOrmQuery<T> extends AbstractQuery implements SpiQuery<T> {
     return versionsEnd;
   }
 
+  @Override
+  public Query<T> setUnmodifiable(boolean unmodifiable) {
+    this.unmodifiable = unmodifiable;
+    return this;
+  }
+
   @Override
   public final Boolean isReadOnly() {
     return readOnly;
   }
 
+  @Override
+  public boolean isUnmodifiable() {
+    return unmodifiable;
+  }
+
   @Override
   public final Query<T> setReadOnly(boolean readOnly) {
     this.readOnly = readOnly;

ebean-querybean/src/main/java/io/ebean/typequery/QueryBean.java

@@ -509,6 +509,12 @@ public abstract class QueryBean<T, R extends QueryBean<T, R>> implements IQueryB
     return root;
   }
 
+  @Override
+  public R setUnmodifiable(boolean unmodifiable) {
+    query.setUnmodifiable(unmodifiable);
+    return root;
+  }
+
   @Override
   public final R setReadOnly(boolean readOnly) {
     query.setReadOnly(readOnly);

ebean-test/src/test/java/org/tests/query/TestQueryOrderById.java

@@ -1,26 +1,65 @@
 package org.tests.query;
 
-import io.ebean.bean.BeanCollection;
+import io.ebean.DB;
+import io.ebean.Query;
 import io.ebean.bean.EntityBean;
 import io.ebean.bean.EntityBeanIntercept;
 import io.ebean.bean.InterceptReadOnly;
 import io.ebean.xtest.BaseTestCase;
-import io.ebean.DB;
-import io.ebean.Query;
 import org.junit.jupiter.api.Test;
 import org.tests.model.basic.Address;
 import org.tests.model.basic.Contact;
 import org.tests.model.basic.Customer;
 import org.tests.model.basic.ResetBasicData;
 
-import java.util.Collections;
+import java.io.*;
 import java.util.List;
 
-import static org.assertj.core.api.Assertions.*;
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 class TestQueryOrderById extends BaseTestCase {
 
+  @Test
+  void unmodifiableResult() throws IOException, ClassNotFoundException {
+    ResetBasicData.reset();
+
+    List<Customer> result = DB.find(Customer.class)
+      .setUnmodifiable(true)
+      .select("id,name")
+      .fetch("contacts")
+      .orderBy("id")
+      .findList();
+
+    assertThat(result).isNotEmpty();
+    assertThatThrownBy(() -> result.add(new Customer()))
+      .isInstanceOf(IllegalStateException.class)
+      .hasMessageContaining("ReadOnly");
+
+    Customer customer = result.get(0);
+    List<Contact> contacts = customer.getContacts();
+    assertThat(contacts).isNotEmpty();
+    assertThatThrownBy(() -> contacts.add(new Contact()))
+      .isInstanceOf(UnsupportedOperationException.class);
+
+    assertThatThrownBy(customer::getOrders)
+      .isInstanceOf(IllegalStateException.class)
+      .hasMessageContaining("Property not loaded: orders");
+
+    ByteArrayOutputStream os = new ByteArrayOutputStream();
+    ObjectOutputStream oos = new ObjectOutputStream(os);
+
+    oos.writeObject(customer);
+    oos.flush();
+    oos.close();
+
+    ByteArrayInputStream is = new ByteArrayInputStream(os.toByteArray());
+    ObjectInputStream ois = new ObjectInputStream(is);
+
+    Customer read = (Customer) ois.readObject();
+    assertThat(read.getName()).isEqualTo(customer.getName());
+  }
+
   @Test
   void immutableResult() {
     ResetBasicData.reset();
@@ -38,12 +77,8 @@ class TestQueryOrderById extends BaseTestCase {
 
     List<Contact> contacts = result.get(0).getContacts();
     assertThat(contacts).isNotEmpty();
-    assertThat(contacts).isInstanceOf(BeanCollection.class);
-    BeanCollection<?> bc = (BeanCollection<?>)contacts;
-    assertThat(bc.isReadOnly()).isTrue();
     assertThatThrownBy(() -> contacts.add(new Contact()))
-      .isInstanceOf(IllegalStateException.class)
-      .hasMessageContaining("ReadOnly");
+      .isInstanceOf(UnsupportedOperationException.class);
   }
 
   @Test
@@ -70,24 +105,27 @@ class TestQueryOrderById extends BaseTestCase {
     EntityBeanIntercept intercept = ((EntityBean) customer)._ebean_getIntercept();
     int pos = intercept.findProperty("billingAddress");
     assertThat(intercept.isLoadedProperty(pos)).isFalse();
-    assertThat(customer.getBillingAddress())
+    assertThatThrownBy(() -> customer.getBillingAddress())
       .describedAs("Not loaded property returns null")
-      .isNull();
+      .isInstanceOf(IllegalStateException.class)
+      .hasMessageContaining("Property not loaded: billingAddress");
 
     assertThatThrownBy(() -> customer.setBillingAddress(new Address()))
       .describedAs("Not allowed to mutate a readOnly bean")
       .isInstanceOf(IllegalStateException.class)
-      .hasMessageContaining("ReadOnly");
-
-    intercept.errorOnLazyLoad(true);
+      .hasMessageContaining("Property not loaded: billingAddress");
 
     assertThatThrownBy(customer::getBillingAddress)
       .isInstanceOf(IllegalStateException.class)
       .hasMessageContaining("Property not loaded: billingAddress");
 
     assertThat(intercept).isInstanceOf(InterceptReadOnly.class);
-    assertThat(customer.getOrders()).isSameAs(Collections.EMPTY_LIST);
-    assertThat(customer.getContacts()).isSameAs(Collections.EMPTY_LIST);
+    assertThatThrownBy(customer::getOrders)
+      .isInstanceOf(IllegalStateException.class)
+      .hasMessageContaining("Property not loaded: orders");
+    assertThatThrownBy(customer::getContacts)
+      .isInstanceOf(IllegalStateException.class)
+      .hasMessageContaining("Property not loaded: contacts");
   }
 
   @Test