Refactor DbJson Jackson handling adding DatabaseConfig.setJsonDirtyByDefault()

Adds the ability to change the default "jsonDirtyByDefault" configuration setting used with DbJson Jackson properties. Currently these default to being assumed dirty and this allows us to change that to be assumed not dirty (which is very likely the better default).

ebean-api/src/main/java/io/ebean/config/DatabaseConfig.java

@@ -193,6 +193,12 @@ public class DatabaseConfig {
    */
   private JsonConfig.Include jsonInclude = JsonConfig.Include.ALL;
 
+  /**
+   * When true then by default DbJson beans are assumed to be dirty.
+   * I believe we want to change this default to false in the future.
+   */
+  private boolean jsonDirtyByDefault = true;
+
   /**
    * The database platform name. Used to imply a DatabasePlatform to use.
    */
@@ -737,6 +743,26 @@ public class DatabaseConfig {
     this.jsonInclude = jsonInclude;
   }
 
+  /**
+   * Return true if DbJson beans are assumed dirty by default.
+   * <p>
+   * That is, when true beans that do not implement ModifyAwareType are by
+   * default assumed to be dirty and included in updates.
+   */
+  public boolean isJsonDirtyByDefault() {
+    return jsonDirtyByDefault;
+  }
+
+  /**
+   * Set to false if we want DbJson beans to not be assumed to be dirty.
+   * <p>
+   * That is, when true beans that do not implement ModifyAwareType are by
+   * default assumed to be dirty and included in updates.
+   */
+  public void setJsonDirtyByDefault(boolean jsonDirtyByDefault) {
+    this.jsonDirtyByDefault = jsonDirtyByDefault;
+  }
+
   /**
    * Return the name of the Database.
    */
@@ -2909,6 +2935,7 @@ public class DatabaseConfig {
     jsonInclude = p.getEnum(JsonConfig.Include.class, "jsonInclude", jsonInclude);
     jsonDateTime = p.getEnum(JsonConfig.DateTime.class, "jsonDateTime", jsonDateTime);
     jsonDate = p.getEnum(JsonConfig.Date.class, "jsonDate", jsonDate);
+    jsonDirtyByDefault = p.getBoolean("jsonDirtyByDefault", jsonDirtyByDefault);
 
     runMigration = p.getBoolean("migration.run", runMigration);
     ddlGenerate = p.getBoolean("ddl.generate", ddlGenerate);
@@ -3369,7 +3396,7 @@ public class DatabaseConfig {
     this.loadModuleInfo = loadModuleInfo;
   }
 
-    public enum UuidVersion {
+  public enum UuidVersion {
     VERSION4,
     VERSION1,
     VERSION1RND

ebean-core/src/main/java/io/ebeaninternal/server/type/CheckMarkedDirty.java

@@ -1,26 +0,0 @@
-package io.ebeaninternal.server.type;
-
-import io.ebean.ModifyAwareType;
-
-/**
- * Check dirty state of json value which might be modify aware.
- */
-class CheckMarkedDirty {
-
-  /**
-   * Return true if the value should be considered dirty (and included in an update).
-   */
-  static boolean isDirty(Object value) {
-    if (value instanceof ModifyAwareType) {
-      ModifyAwareType modifyAware = (ModifyAwareType) value;
-      if (modifyAware.isMarkedDirty()) {
-        // reset the dirty state (consider not dirty after update)
-        modifyAware.setMarkedDirty(false);
-        return true;
-      } else {
-        return false;
-      }
-    }
-    return true;
-  }
-}

ebean-core/src/main/java/io/ebeaninternal/server/type/DefaultTypeManager.java

@@ -92,6 +92,7 @@ public final class DefaultTypeManager implements TypeManager {
   private final Object objectMapper;
   private final boolean objectMapperPresent;
   private final boolean postgres;
+  private final TypeJsonManager jsonManager;
   private final boolean offlineMigrationGeneration;
   private final EnumType defaultEnumType;
 
@@ -131,11 +132,11 @@ public final class DefaultTypeManager implements TypeManager {
     this.typeMap = new ConcurrentHashMap<>();
     this.nativeMap = new ConcurrentHashMap<>();
     this.logicalMap = new ConcurrentHashMap<>();
+    this.postgres = isPostgres(config.getDatabasePlatform());
     this.objectMapperPresent = config.getClassLoadConfig().isJacksonObjectMapperPresent();
     this.objectMapper = (objectMapperPresent) ? initObjectMapper(config) : null;
-
+    this.jsonManager = (objectMapperPresent) ? new TypeJsonManager(postgres, objectMapper, config.isJsonDirtyByDefault()) : null;
     this.extraTypeFactory = new DefaultTypeFactory(config);
-    this.postgres = isPostgres(config.getDatabasePlatform());
     this.arrayTypeListFactory = arrayTypeListFactory(config.getDatabasePlatform());
     this.arrayTypeSetFactory = arrayTypeSetFactory(config.getDatabasePlatform());
     this.offlineMigrationGeneration = DbOffline.isGenerateMigration();
@@ -425,7 +426,7 @@ public final class DefaultTypeManager implements TypeManager {
     if (objectMapper == null) {
       throw new IllegalArgumentException("Type [" + type + "] unsupported for @DbJson mapping - Jackson ObjectMapper not present");
     }
-    return ScalarTypeJsonObjectMapper.createTypeFor(postgres, (AnnotatedField) prop.getJacksonField(), (ObjectMapper) objectMapper, dbType, docType);
+    return ScalarTypeJsonObjectMapper.createTypeFor(jsonManager, (AnnotatedField) prop.getJacksonField(), dbType, docType);
   }
 
   /**

ebean-core/src/main/java/io/ebeaninternal/server/type/ScalarTypeJsonCollection.java

@@ -61,7 +61,7 @@ abstract class ScalarTypeJsonCollection<T> extends ScalarTypeBase<T> implements
    */
   @Override
   public boolean isDirty(Object value) {
-    return CheckMarkedDirty.isDirty(value);
+    return TypeJsonManager.checkIsDirty(value);
   }
 
   @Override

ebean-core/src/main/java/io/ebeaninternal/server/type/ScalarTypeJsonMap.java

@@ -123,7 +123,7 @@ public abstract class ScalarTypeJsonMap extends ScalarTypeBase<Map> {
    */
   @Override
   public boolean isDirty(Object value) {
-    return CheckMarkedDirty.isDirty(value);
+    return TypeJsonManager.checkIsDirty(value);
   }
 
   @Override

ebean-core/src/main/java/io/ebeaninternal/server/type/ScalarTypeJsonObjectMapper.java

@@ -7,7 +7,6 @@ import com.fasterxml.jackson.databind.JavaType;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.ObjectWriter;
 import com.fasterxml.jackson.databind.introspect.AnnotatedField;
-import io.ebean.config.dbplatform.DbPlatformType;
 import io.ebean.core.type.DataBinder;
 import io.ebean.core.type.DataReader;
 import io.ebean.core.type.DocPropertyType;
@@ -30,38 +29,23 @@ import java.util.Set;
 /**
  * Supports @DbJson properties using Jackson ObjectMapper.
  */
-public class ScalarTypeJsonObjectMapper {
+class ScalarTypeJsonObjectMapper {
 
   /**
    * Create and return the appropriate ScalarType.
    */
-  public static ScalarType<?> createTypeFor(boolean postgres, AnnotatedField field, ObjectMapper objectMapper,
-                                            int dbType, DocPropertyType docType) {
-
+  static ScalarType<?> createTypeFor(TypeJsonManager jsonManager, AnnotatedField field, int dbType, DocPropertyType docType) {
     Class<?> type = field.getRawType();
-    String pgType = getPostgresType(postgres, dbType);
     if (Set.class.equals(type)) {
-      return new OmSet(objectMapper, field, dbType, pgType, docType);
+      return new OmSet(jsonManager, field, dbType, docType);
     }
     if (List.class.equals(type)) {
-      return new OmList(objectMapper, field, dbType, pgType, docType);
+      return new OmList(jsonManager, field, dbType, docType);
     }
     if (Map.class.equals(type)) {
-      return new OmMap(objectMapper, field, dbType, pgType);
+      return new OmMap(jsonManager, field, dbType);
     }
-    return new GenericObject(objectMapper, field, dbType, pgType);
-  }
-
-  private static String getPostgresType(boolean postgres, int dbType) {
-    if (postgres) {
-      switch (dbType) {
-        case DbPlatformType.JSON:
-          return PostgresHelper.JSON_TYPE;
-        case DbPlatformType.JSONB:
-          return PostgresHelper.JSONB_TYPE;
-      }
-    }
-    return null;
+    return new GenericObject(jsonManager, field, dbType, type);
   }
 
   /**
@@ -69,8 +53,8 @@ public class ScalarTypeJsonObjectMapper {
    */
   private static class GenericObject extends Base<Object> {
 
-    public GenericObject(ObjectMapper objectMapper, AnnotatedField field, int dbType, String pgType) {
-      super(Object.class, objectMapper, field, dbType, pgType, DocPropertyType.OBJECT);
+    GenericObject(TypeJsonManager jsonManager, AnnotatedField field, int dbType, Class<?> rawType) {
+      super(Object.class, jsonManager, field, dbType, DocPropertyType.OBJECT, rawType);
     }
   }
 
@@ -80,8 +64,8 @@ public class ScalarTypeJsonObjectMapper {
   @SuppressWarnings("rawtypes")
   private static class OmSet extends Base<Set> {
 
-    public OmSet(ObjectMapper objectMapper, AnnotatedField field, int dbType, String pgType, DocPropertyType docType) {
-      super(Set.class, objectMapper, field, dbType, pgType, docType);
+    OmSet(TypeJsonManager jsonManager, AnnotatedField field, int dbType, DocPropertyType docType) {
+      super(Set.class, jsonManager, field, dbType, docType);
     }
 
     @Override
@@ -98,8 +82,8 @@ public class ScalarTypeJsonObjectMapper {
   @SuppressWarnings("rawtypes")
   private static class OmList extends Base<List> {
 
-    public OmList(ObjectMapper objectMapper, AnnotatedField field, int dbType, String pgType, DocPropertyType docType) {
-      super(List.class, objectMapper, field, dbType, pgType, docType);
+    OmList(TypeJsonManager jsonManager, AnnotatedField field, int dbType, DocPropertyType docType) {
+      super(List.class, jsonManager, field, dbType, docType);
     }
 
     @Override
@@ -116,8 +100,8 @@ public class ScalarTypeJsonObjectMapper {
   @SuppressWarnings("rawtypes")
   private static class OmMap extends Base<Map> {
 
-    public OmMap(ObjectMapper objectMapper, AnnotatedField field, int dbType, String pgType) {
-      super(Map.class, objectMapper, field, dbType, pgType, DocPropertyType.OBJECT);
+    OmMap(TypeJsonManager jsonManager, AnnotatedField field, int dbType) {
+      super(Map.class, jsonManager, field, dbType, DocPropertyType.OBJECT);
     }
 
     @Override
@@ -135,24 +119,23 @@ public class ScalarTypeJsonObjectMapper {
   private static abstract class Base<T> extends ScalarTypeBase<T> {
 
     private final ObjectWriter objectWriter;
-
     private final ObjectMapper objectReader;
-
     private final JavaType deserType;
-
     private final String pgType;
-
     private final DocPropertyType docType;
+    private final TypeJsonManager.DirtyHandler dirtyHandler;
 
-    /**
-     * Construct given the object mapper, property type and DB type for storage.
-     */
-    public Base(Class<T> cls, ObjectMapper objectMapper, AnnotatedField field, int dbType, String pgType, DocPropertyType docType) {
+    Base(Class<T> cls, TypeJsonManager jsonManager, AnnotatedField field, int dbType, DocPropertyType docType) {
+      this(cls, jsonManager, field, dbType, docType, cls);
+    }
+
+    Base(Class<T> cls, TypeJsonManager jsonManager, AnnotatedField field, int dbType, DocPropertyType docType, Class<?> rawType) {
       super(cls, false, dbType);
-      this.pgType = pgType;
+      this.objectReader = jsonManager.objectMapper();
+      this.pgType = jsonManager.postgresType(dbType);
       this.docType = docType;
-      this.objectReader = objectMapper;
-      final JacksonTypeHelper helper = new JacksonTypeHelper(field, objectMapper);
+      this.dirtyHandler = jsonManager.dirtyHandler(cls, rawType);
+      final JacksonTypeHelper helper = new JacksonTypeHelper(field, objectReader);
       this.deserType = helper.type();
       this.objectWriter = helper.objectWriter();
     }
@@ -170,7 +153,7 @@ public class ScalarTypeJsonObjectMapper {
      */
     @Override
     public boolean isDirty(Object value) {
-      return CheckMarkedDirty.isDirty(value);
+      return dirtyHandler.isDirty(value);
     }
 
     @Override

ebean-core/src/main/java/io/ebeaninternal/server/type/ScalarTypePostgresHstore.java

@@ -33,7 +33,7 @@ public class ScalarTypePostgresHstore extends ScalarTypeBase<Map> {
 
   @Override
   public boolean isDirty(Object value) {
-    return CheckMarkedDirty.isDirty(value);
+    return TypeJsonManager.checkIsDirty(value);
   }
 
   @SuppressWarnings("unchecked")

ebean-core/src/main/java/io/ebeaninternal/server/type/TypeJsonManager.java

@@ -0,0 +1,98 @@
+package io.ebeaninternal.server.type;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import io.ebean.ModifyAwareType;
+import io.ebean.config.DatabaseConfig;
+import io.ebean.config.dbplatform.DbPlatformType;
+
+class TypeJsonManager {
+
+  interface DirtyHandler {
+    boolean isDirty(Object value);
+  }
+
+  private final boolean postgres;
+  private final ObjectMapper objectMapper;
+  private final DirtyHandler defaultHandler;
+  private final DirtyHandler modifyAwareHandler;
+
+  TypeJsonManager(boolean postgres, Object objectMapper, boolean defaultDirty) {
+    this.postgres = postgres;
+    this.objectMapper = (ObjectMapper) objectMapper;
+    this.defaultHandler = new DefaultHandler(defaultDirty);
+    this.modifyAwareHandler = new ModifyAwareHandler();
+  }
+
+  ObjectMapper objectMapper() {
+    return objectMapper;
+  }
+
+  String postgresType(int dbType) {
+    if (postgres) {
+      switch (dbType) {
+        case DbPlatformType.JSON:
+          return PostgresHelper.JSON_TYPE;
+        case DbPlatformType.JSONB:
+          return PostgresHelper.JSONB_TYPE;
+      }
+    }
+    return null;
+  }
+
+  /**
+   * Return the DirtyHandler to use.
+   */
+  DirtyHandler dirtyHandler(Class<?> cls, Class<?> rawType) {
+    if (!Object.class.equals(cls) || ModifyAwareType.class.isAssignableFrom(rawType)) {
+      // Set, List and Map are modify aware
+      return modifyAwareHandler;
+    }
+    return defaultHandler;
+  }
+
+  /**
+   * Return true if the value should be considered dirty (and included in an update).
+   */
+  static boolean checkIsDirty(Object value) {
+    if (value instanceof ModifyAwareType) {
+      return checkModifyAware(value);
+    }
+    return true;
+  }
+
+  private static boolean checkModifyAware(Object value) {
+    ModifyAwareType modifyAware = (ModifyAwareType) value;
+    if (modifyAware.isMarkedDirty()) {
+      // reset the dirty state (consider not dirty after update)
+      modifyAware.setMarkedDirty(false);
+      return true;
+    } else {
+      return false;
+    }
+  }
+
+  static final class ModifyAwareHandler implements DirtyHandler {
+    @Override
+    public boolean isDirty(Object value) {
+      return checkModifyAware(value);
+    }
+  }
+
+  /**
+   * Effectively constant based on {@link DatabaseConfig#isJsonDirtyByDefault()}
+   */
+  static final class DefaultHandler implements DirtyHandler {
+
+    private final boolean dirty;
+
+    DefaultHandler(boolean dirty) {
+      this.dirty = dirty;
+    }
+
+    @Override
+    public boolean isDirty(Object value) {
+      return dirty;
+    }
+  }
+
+}

ebean-core/src/test/java/io/ebean/config/ServerConfigTest.java

@@ -62,6 +62,7 @@ public class ServerConfigTest {
     props.setProperty("dbOffline", "true");
     props.setProperty("jsonDateTime", "MILLIS");
     props.setProperty("jsonDate", "MILLIS");
+    props.setProperty("jsonDirtyByDefault", "false");
     props.setProperty("autoReadOnlyDataSource", "true");
     props.setProperty("disableL2Cache", "true");
     props.setProperty("notifyL2CacheInForeground", "true");
@@ -103,6 +104,9 @@ public class ServerConfigTest {
     assertEquals(PlatformConfig.DbUuid.BINARY, serverConfig.getPlatformConfig().getDbUuid());
     assertEquals(JsonConfig.DateTime.MILLIS, serverConfig.getJsonDateTime());
     assertEquals(JsonConfig.Date.MILLIS, serverConfig.getJsonDate());
+    assertFalse(serverConfig.isJsonDirtyByDefault());
+    serverConfig.setJsonDirtyByDefault(true);
+    assertTrue(serverConfig.isJsonDirtyByDefault());
 
     assertEquals("r0,users,orgs", serverConfig.getEnabledL2Regions());
 
@@ -155,6 +159,7 @@ public class ServerConfigTest {
     assertFalse(serverConfig.isIdGeneratorAutomatic());
     assertEquals(JsonConfig.DateTime.ISO8601, serverConfig.getJsonDateTime());
     assertEquals(JsonConfig.Date.ISO8601, serverConfig.getJsonDate());
+    assertTrue(serverConfig.isJsonDirtyByDefault());
     assertTrue(serverConfig.getPlatformConfig().isCaseSensitiveCollation());
     assertTrue(serverConfig.isAutoLoadModuleInfo());
 

ebean-core/src/test/java/org/tests/json/TestDbJson_Jackson3.java

@@ -9,6 +9,7 @@ import org.tests.model.json.EBasicJsonList;
 import org.tests.model.json.PlainBean;
 import org.tests.model.json.PlainBeanDirtyAware;
 
+import java.util.Arrays;
 import java.util.List;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -59,12 +60,14 @@ public class TestDbJson_Jackson3 extends BaseTestCase {
     EBasicJsonList bean = new EBasicJsonList();
     bean.setName("p1");
     bean.setPlainBean(contentBean);
+    bean.setBeanList(Arrays.asList(contentBean));
 
     DB.save(bean);
     final EBasicJsonList found = DB.find(EBasicJsonList.class, bean.getId());
     // json bean not modified but not aware
     // ideally don't load the json content if we are not going to modify it
     found.setName("p1-mod");
+    found.setBeanList(null);
 
     LoggedSql.start();
     DB.save(found);