#2044 - Using @AttributeOverrides breaking generated SQL (when override nullable and no column name)

src/main/java/io/ebeaninternal/server/deploy/BeanEmbeddedMetaFactory.java

@@ -3,6 +3,7 @@ package io.ebeaninternal.server.deploy;
 import io.ebean.config.BeanNotRegisteredException;
 import io.ebeaninternal.server.deploy.meta.DeployBeanPropertyAssocOne;
 
+import javax.persistence.Column;
 import java.util.Map;
 
 /**
@@ -15,7 +16,6 @@ class BeanEmbeddedMetaFactory {
    * Create BeanProperties for embedded beans using the deployment specific DB column name and table alias.
    */
   public static BeanEmbeddedMeta create(BeanDescriptorMap owner, DeployBeanPropertyAssocOne<?> prop) {
-
     // we can get a BeanDescriptor for an Embedded bean
     // and know that it is NOT recursive, as Embedded beans are
     // only allow to hold simple scalar types...
@@ -28,23 +28,17 @@ class BeanEmbeddedMetaFactory {
 
     // deployment override information (column names)
     String columnPrefix = prop.getColumnPrefix();
-    Map<String, String> propColMap = prop.getDeployEmbedded().getPropertyColumnMap();
+    Map<String, Column> propColMap = prop.getDeployEmbedded().getPropertyColumnMap();
 
     BeanProperty[] sourceProperties = targetDesc.propertiesNonTransient();
     BeanProperty[] embeddedProperties = new BeanProperty[sourceProperties.length];
 
     for (int i = 0; i < sourceProperties.length; i++) {
       String propertyName = sourceProperties[i].getName();
-      String dbColumn = propColMap.get(propertyName);
-      if (dbColumn == null) {
-        // dbColumn not overridden so take original
-        dbColumn = sourceProperties[i].getDbColumn();
-        if (columnPrefix != null) {
-          dbColumn = columnPrefix + dbColumn;
-        }
-      }
-
-      BeanPropertyOverride overrides = new BeanPropertyOverride(dbColumn);
+      Column column = propColMap.get(propertyName);
+      String dbColumn = dbColumn(columnPrefix, column, sourceProperties[i]);
+      boolean dbNullable = dbNullable(column, sourceProperties[i]);
+      BeanPropertyOverride overrides = new BeanPropertyOverride(dbColumn, dbNullable);
       if (sourceProperties[i] instanceof BeanPropertyAssocOne) {
         embeddedProperties[i] = new BeanPropertyAssocOne((BeanPropertyAssocOne)sourceProperties[i], overrides);
       } else {
@@ -54,4 +48,13 @@ class BeanEmbeddedMetaFactory {
 
     return new BeanEmbeddedMeta(embeddedProperties);
   }
+
+  private static String dbColumn(String prefix, Column override, BeanProperty source) {
+    String dbCol = (override != null && !override.name().isEmpty()) ? override.name() : source.getDbColumn();
+    return prefix == null ? dbCol : prefix + dbCol;
+  }
+
+  private static boolean dbNullable(Column override, BeanProperty source) {
+    return (override != null && !override.nullable()) ? override.nullable() : source.isNullable();
+  }
 }

src/main/java/io/ebeaninternal/server/deploy/BeanProperty.java

@@ -274,7 +274,6 @@ public class BeanProperty implements ElPropertyValue, Property, STreeProperty {
   }
 
   public BeanProperty(BeanDescriptor<?> descriptor, DeployBeanProperty deploy) {
-
     this.descriptor = descriptor;
     this.name = InternString.intern(deploy.getName());
     this.propertyIndex = deploy.getPropertyIndex();
@@ -382,9 +381,10 @@ public class BeanProperty implements ElPropertyValue, Property, STreeProperty {
   public BeanProperty(BeanProperty source, BeanPropertyOverride override) {
 
     this.descriptor = source.descriptor;
-    this.name = InternString.intern(source.getName());
     this.propertyIndex = source.propertyIndex;
+    this.name = source.getName();
     this.dbColumn = InternString.intern(override.getDbColumn());
+    this.nullable = override.isDbNullable();
     // override with sqlFormula not currently supported
     this.sqlFormulaJoin = null;
     this.sqlFormulaSelect = null;
@@ -418,7 +418,6 @@ public class BeanProperty implements ElPropertyValue, Property, STreeProperty {
     this.dbRead = source.isDbRead();
     this.dbInsertable = source.isDbInsertable();
     this.dbUpdatable = source.isDbUpdatable();
-    this.nullable = source.isNullable();
     this.unique = source.isUnique();
     this.naturalKey = source.isNaturalKey();
     this.dbLength = source.getDbLength();

src/main/java/io/ebeaninternal/server/deploy/BeanPropertyOverride.java

@@ -12,16 +12,22 @@ import io.ebeaninternal.server.core.InternString;
 class BeanPropertyOverride {
 
   private final String dbColumn;
+  private final boolean dbNullable;
 
-  BeanPropertyOverride(String dbColumn) {
+  BeanPropertyOverride(String dbColumn, boolean dbNullable) {
     this.dbColumn = InternString.intern(dbColumn);
+    this.dbNullable = dbNullable;
   }
 
-  public String getDbColumn() {
+  String getDbColumn() {
     return dbColumn;
   }
 
-  public String replace(String src, String srcDbColumn) {
+  boolean isDbNullable() {
+    return dbNullable;
+  }
+
+  String replace(String src, String srcDbColumn) {
     return StringHelper.replaceString(src, srcDbColumn, dbColumn);
   }
 }

src/main/java/io/ebeaninternal/server/deploy/meta/DeployBeanEmbedded.java

@@ -1,5 +1,6 @@
 package io.ebeaninternal.server.deploy.meta;
 
+import javax.persistence.Column;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -8,28 +9,26 @@ import java.util.Map;
  * <p>
  * Typically collects the overridden column names mapped
  * to the Embedded bean.
- * </p>
  */
 public class DeployBeanEmbedded {
 
   /**
    * A map of property names to dbColumns.
    */
-  private final Map<String, String> propMap = new HashMap<>();
+  private final Map<String, Column> propMap = new HashMap<>();
 
   /**
    * Set a Map of property names to dbColumns.
    */
-  public void putAll(Map<String, String> propertyColumnMap) {
+  public void putAll(Map<String, Column> propertyColumnMap) {
     propMap.putAll(propertyColumnMap);
   }
 
   /**
    * Return a map of property names to dbColumns.
    */
-  public Map<String, String> getPropertyColumnMap() {
+  public Map<String, Column> getPropertyColumnMap() {
     return propMap;
   }
 
-
 }

src/main/java/io/ebeaninternal/server/deploy/parse/AnnotationParser.java

@@ -11,6 +11,7 @@ import javax.persistence.CascadeType;
 import javax.persistence.Column;
 import javax.validation.groups.Default;
 import java.util.HashMap;
+import java.util.Map;
 import java.util.Set;
 import java.util.UUID;
 
@@ -89,9 +90,9 @@ public abstract class AnnotationParser extends AnnotationBase {
 
     Set<AttributeOverride> attrOverrides = annotationAttributeOverrides(prop);
     if (!attrOverrides.isEmpty()) {
-      HashMap<String, String> propMap = new HashMap<>(attrOverrides.size());
+      Map<String, Column> propMap = new HashMap<>(attrOverrides.size());
       for (AttributeOverride attrOverride : attrOverrides) {
-        propMap.put(attrOverride.name(), attrOverride.column().name());
+        propMap.put(attrOverride.name(), attrOverride.column());
       }
       prop.getDeployEmbedded().putAll(propMap);
     }

src/test/ddl-review/h2-create-all.sql

@@ -1,4 +1,4 @@
--- Generated by ebean unknown at 2020-08-24T21:06:38.731290Z
+-- Generated by ebean unknown at 2020-08-24T21:11:54.586122Z
 create table asimple_bean (
   id                            bigint generated by default as identity not null,
   name                          varchar(255),
@@ -1430,6 +1430,19 @@ create table eperson (
   constraint pk_eperson primary key (id)
 );
 
+create table eperson2 (
+  id                            bigint generated by default as identity not null,
+  name                          varchar(255),
+  notes                         varchar(255),
+  street                        varchar(255),
+  suburb                        varchar(255),
+  city                          varchar(255) not null,
+  status                        varchar(3) not null,
+  version                       bigint not null,
+  constraint ck_eperson2_status check ( status in ('ONE','TWO')),
+  constraint pk_eperson2 primary key (id)
+);
+
 create table e_person_online (
   id                            bigint generated by default as identity not null,
   email                         varchar(127),

src/test/ddl-review/h2-drop-all.sql

@@ -1,4 +1,4 @@
--- Generated by ebean unknown at 2020-08-24T21:06:38.731290Z
+-- Generated by ebean unknown at 2020-08-24T21:11:54.586122Z
 alter table bar drop constraint if exists fk_bar_foo_id;
 drop index if exists ix_bar_foo_id;
 
@@ -1189,6 +1189,8 @@ drop table if exists eper_addr;
 
 drop table if exists eperson;
 
+drop table if exists eperson2;
+
 drop table if exists e_person_online;
 
 drop table if exists esimple;

src/test/java/org/tests/model/embedded/EPerson2.java

@@ -0,0 +1,71 @@
+package org.tests.model.embedded;
+
+import javax.persistence.AttributeOverride;
+import javax.persistence.AttributeOverrides;
+import javax.persistence.Column;
+import javax.persistence.Embedded;
+import javax.persistence.Entity;
+import javax.persistence.Id;
+import javax.persistence.Version;
+
+@Entity
+public class EPerson2 {
+
+  @Id
+  long id;
+
+  @Version
+  long version;
+
+  String name;
+
+  String notes;
+
+  @Embedded
+  @AttributeOverrides({
+    @AttributeOverride(name = "city", column = @Column(nullable = false)),
+    @AttributeOverride(name = "status", column = @Column(nullable = false))
+  })
+  EAddress address;
+
+  public long getId() {
+    return id;
+  }
+
+  public void setId(long id) {
+    this.id = id;
+  }
+
+  public long getVersion() {
+    return version;
+  }
+
+  public void setVersion(long version) {
+    this.version = version;
+  }
+
+  public String getName() {
+    return name;
+  }
+
+  public void setName(String name) {
+    this.name = name;
+  }
+
+  public String getNotes() {
+    return notes;
+  }
+
+  public void setNotes(String notes) {
+    this.notes = notes;
+  }
+
+  public EAddress getAddress() {
+    return address;
+  }
+
+  public void setAddress(EAddress address) {
+    this.address = address;
+  }
+
+}

src/test/java/org/tests/model/embedded/TestEmbeddedNullableOverride.java

@@ -0,0 +1,33 @@
+package org.tests.model.embedded;
+
+import io.ebean.BaseTestCase;
+import io.ebean.DB;
+import org.junit.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.Assert.assertNotNull;
+
+public class TestEmbeddedNullableOverride extends BaseTestCase {
+
+  @Test
+  public void test() {
+
+    EPerson2 person = new EPerson2();
+    person.setName("foo");
+
+    EAddress address = new EAddress();
+    address.setCity("myCity");
+    address.setStatus(EAddressStatus.TWO);
+    person.setAddress(address);
+
+    DB.save(person);
+
+    final EPerson2 found = DB.find(EPerson2.class, person.getId());
+
+    assertNotNull(found);
+    assertNotNull(found.getAddress());
+    assertThat(found.getAddress().getCity()).isEqualTo("myCity");
+    assertThat(found.getAddress().getStatus()).isEqualTo(EAddressStatus.TWO);
+    assertThat(found.getAddress().getStreet()).isNull();
+  }
+}