#1837 - Inheritance on both sides of @ManyToOne confuses columns (#1843)

* #1837 - Inheritance on both sides of @ManyToOne confuses columns

* #1837 - Inheritance on both sides of @ManyToOne confuses columns

Tidy up whitespace in SQL

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

@@ -5,6 +5,7 @@ import io.ebean.bean.EntityBeanIntercept;
 import io.ebean.bean.ObjectGraphNode;
 import io.ebean.bean.PersistenceContext;
 import io.ebeaninternal.server.core.OrmQueryRequest;
+import io.ebeaninternal.server.deploy.BeanPropertyAssocOne;
 
 /**
  * Controls the loading of reference objects for a query instance.
@@ -47,6 +48,11 @@ public interface LoadContext {
    */
   void register(String path, EntityBeanIntercept ebi);
 
+  /**
+   * Register a Bean with inheritance.
+   */
+  void register(String path, EntityBeanIntercept ebi, BeanPropertyAssocOne<?> property);
+
   /**
    * Register a collection for lazy loading.
    */

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

@@ -57,14 +57,13 @@ class AssocOneHelpRefInherit extends AssocOneHelp {
     boolean disableLazyLoading = ctx.isDisableLazyLoading();
     Object ref = desc.contextRef(pc, ctx.isReadOnly(), disableLazyLoading, id);
     if (!disableLazyLoading) {
-      ctx.register(property.name, ((EntityBean) ref)._ebean_getIntercept());
+      ctx.registerBeanInherit(property, ((EntityBean) ref)._ebean_getIntercept());
     }
     return ref;
   }
 
   @Override
   void appendFrom(DbSqlContext ctx, SqlJoinType joinType) {
-
     // add join to support the discriminator column
     String relativePrefix = ctx.getRelativePrefix(property.name);
     property.tableJoin.addJoin(joinType, relativePrefix, ctx);

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

@@ -60,6 +60,11 @@ public interface DbReadContext {
    */
   void register(String path, EntityBeanIntercept ebi);
 
+  /**
+   * Register a reference with inheritance for lazy loading.
+   */
+  void registerBeanInherit(BeanPropertyAssocOne<?> property, EntityBeanIntercept ebi);
+
   /**
    * Register a collection for lazy loading.
    */

src/main/java/io/ebeaninternal/server/loadcontext/DLoadBaseContext.java

@@ -33,7 +33,6 @@ abstract class DLoadBaseContext {
   final boolean queryFetch;
 
   DLoadBaseContext(DLoadContext parent, BeanDescriptor<?> desc, String path, int defaultBatchSize, OrmQueryProperties queryProps) {
-
     this.parent = parent;
     this.serverName = parent.getEbeanServer().getName();
     this.desc = desc;
@@ -41,7 +40,6 @@ abstract class DLoadBaseContext {
     this.fullPath = parent.getFullPath(path);
     this.hitCache = parent.isBeanCacheGet() && desc.isBeanCaching();
     this.objectGraphNode = parent.getObjectGraphNode(path);
-
     this.queryFetch = queryProps != null && queryProps.isQueryFetch();
     this.firstBatchSize = initFirstBatchSize(defaultBatchSize, queryProps);
     this.secondaryBatchSize = initSecondaryBatchSize(defaultBatchSize, firstBatchSize, queryProps);

src/main/java/io/ebeaninternal/server/loadcontext/DLoadContext.java

@@ -19,6 +19,7 @@ import io.ebeaninternal.server.deploy.BeanDescriptor;
 import io.ebeaninternal.server.deploy.BeanProperty;
 import io.ebeaninternal.server.deploy.BeanPropertyAssoc;
 import io.ebeaninternal.server.deploy.BeanPropertyAssocMany;
+import io.ebeaninternal.server.deploy.BeanPropertyAssocOne;
 import io.ebeaninternal.server.el.ElPropertyValue;
 import io.ebeaninternal.server.querydefn.OrmQueryProperties;
 
@@ -287,6 +288,11 @@ public class DLoadContext implements LoadContext {
     getBeanContext(path).register(ebi);
   }
 
+  @Override
+  public void register(String path, EntityBeanIntercept ebi, BeanPropertyAssocOne<?> property) {
+    getBeanContextWithInherit(path, property).register(ebi);
+  }
+
   @Override
   public void register(String path, BeanCollection<?> bc) {
     getManyContext(path).register(bc);
@@ -299,6 +305,11 @@ public class DLoadContext implements LoadContext {
     return beanMap.computeIfAbsent(path, p -> createBeanContext(p, defaultBatchSize, null));
   }
 
+  DLoadBeanContext getBeanContextWithInherit(String path, BeanPropertyAssocOne<?> property) {
+    String key = path + ":" + property.getTargetDescriptor().getName();
+    return beanMap.computeIfAbsent(key, p -> createBeanContext(property, path, defaultBatchSize, null));
+  }
+
   private void registerSecondaryNode(boolean many, OrmQueryProperties props) {
 
     int batchSize;
@@ -331,11 +342,14 @@ public class DLoadContext implements LoadContext {
   }
 
   private DLoadBeanContext createBeanContext(String path, int batchSize, OrmQueryProperties queryProps) {
-
     BeanPropertyAssoc<?> p = (BeanPropertyAssoc<?>) getBeanProperty(rootDescriptor, path);
     return new DLoadBeanContext(this, p.getTargetDescriptor(), path, batchSize, queryProps);
   }
 
+  private DLoadBeanContext createBeanContext(BeanPropertyAssoc<?> property, String path, int batchSize, OrmQueryProperties queryProps) {
+    return new DLoadBeanContext(this, property.getTargetDescriptor(), path, batchSize, queryProps);
+  }
+
   private BeanProperty getBeanProperty(BeanDescriptor<?> desc, String path) {
     return desc.findPropertyFromPath(path);
   }

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

@@ -21,6 +21,7 @@ import io.ebeaninternal.server.core.SpiOrmQueryRequest;
 import io.ebeaninternal.server.deploy.BeanCollectionHelpFactory;
 import io.ebeaninternal.server.deploy.BeanDescriptor;
 import io.ebeaninternal.server.deploy.BeanPropertyAssocMany;
+import io.ebeaninternal.server.deploy.BeanPropertyAssocOne;
 import io.ebeaninternal.server.deploy.DbReadContext;
 import io.ebeaninternal.server.type.DataBind;
 import io.ebeaninternal.server.type.DataReader;
@@ -635,8 +636,13 @@ public class CQuery<T> implements DbReadContext, CancelableQuery, SpiProfileTran
   }
 
   @Override
-  public void register(String path, EntityBeanIntercept ebi) {
+  public void registerBeanInherit(BeanPropertyAssocOne<?> property, EntityBeanIntercept ebi) {
+    String path = getPath(property.getName());
+    request.getGraphContext().register(path, ebi, property);
+  }
 
+  @Override
+  public void register(String path, EntityBeanIntercept ebi) {
     path = getPath(path);
     request.getGraphContext().register(path, ebi);
   }

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

@@ -595,7 +595,7 @@ class SqlTreeNodeBean implements SqlTreeNode {
           ctx.append(" and");
         }
         ctx.append(" ").append(ctx.getTableAlias(prefix)).append(".");
-        ctx.append(inheritInfo.getWhere()).append(" ");
+        ctx.append(inheritInfo.getWhere());
       }
     }
 

src/test/java/org/tests/aggregateformula/TestAggregateInheritFormula.java

@@ -34,6 +34,6 @@ public class TestAggregateInheritFormula {
     final List<String> sql = LoggedSqlCollector.stop();
 
     assertThat(segments).hasSize(1);
-    assertThat(sql.get(0)).contains("select t0.segment_id_zat, min(t0.status_id) from iaf_segment t0 where t0.ptype = 'target'  and t0.segment_id_zat = ? group by t0.segment_id_zat");
+    assertThat(sql.get(0)).contains("select t0.segment_id_zat, min(t0.status_id) from iaf_segment t0 where t0.ptype = 'target' and t0.segment_id_zat = ? group by t0.segment_id_zat");
   }
 }

src/test/java/org/tests/inheritance/bothsides/SourceA.java

@@ -0,0 +1,22 @@
+package org.tests.inheritance.bothsides;
+
+import javax.persistence.CascadeType;
+import javax.persistence.Entity;
+import javax.persistence.ManyToOne;
+
+@Entity
+public class SourceA extends SourceBase {
+
+  @ManyToOne(cascade = CascadeType.ALL)
+  private Target1 target;
+
+  public SourceA(String name, Target1 target, int pos) {
+    super(name, pos);
+    this.target = target;
+  }
+
+  public Target1 getTarget() {
+    return target;
+  }
+
+}

src/test/java/org/tests/inheritance/bothsides/SourceB.java

@@ -0,0 +1,20 @@
+package org.tests.inheritance.bothsides;
+
+import javax.persistence.CascadeType;
+import javax.persistence.Entity;
+import javax.persistence.ManyToOne;
+
+@Entity
+public class SourceB extends SourceBase {
+
+  @ManyToOne(cascade = CascadeType.PERSIST)
+  private Target2 target;
+
+  public SourceB(String name, Target2 target, int pos) {
+    super(name, pos);
+    this.target = target;
+  }
+
+  public Target2 getTarget() { return target;}
+
+}

src/test/java/org/tests/inheritance/bothsides/SourceBase.java

@@ -0,0 +1,26 @@
+package org.tests.inheritance.bothsides;
+
+import javax.persistence.Entity;
+import javax.persistence.Inheritance;
+
+@Entity
+@Inheritance
+public abstract class SourceBase extends WithAutoGeneratedUUID {
+
+  private String name;
+
+  private int pos;
+
+  public SourceBase(String name, int pos) {
+    this.name = name;
+    this.pos = pos;
+  }
+
+  public String getName() {
+    return name;
+  }
+
+  public int getPos() {
+    return pos;
+  }
+}

src/test/java/org/tests/inheritance/bothsides/Target1.java

@@ -0,0 +1,13 @@
+package org.tests.inheritance.bothsides;
+
+import javax.persistence.Entity;
+import javax.persistence.Inheritance;
+
+@Entity
+@Inheritance
+public class Target1 extends TargetBase {
+
+  public Target1(String name) {
+    super(name);
+  }
+}

src/test/java/org/tests/inheritance/bothsides/Target2.java

@@ -0,0 +1,10 @@
+package org.tests.inheritance.bothsides;
+
+import javax.persistence.Entity;
+
+@Entity
+public class Target2 extends TargetBase {
+  public Target2(String name) {
+    super(name);
+  }
+}

src/test/java/org/tests/inheritance/bothsides/TargetBase.java

@@ -0,0 +1,18 @@
+package org.tests.inheritance.bothsides;
+
+import javax.persistence.Entity;
+import javax.persistence.Inheritance;
+
+@Entity
+@Inheritance
+public abstract class TargetBase extends WithAutoGeneratedUUID {
+
+  private String name;
+
+  public TargetBase(String name) {
+    this.name = name;
+  }
+
+  public String getName() { return name;}
+
+}

src/test/java/org/tests/inheritance/bothsides/TestInheritanceBothSides.java

@@ -0,0 +1,148 @@
+package org.tests.inheritance.bothsides;
+
+import io.ebean.BaseTestCase;
+import io.ebean.DB;
+import io.ebean.Database;
+import io.ebean.Query;
+import org.ebeantest.LoggedSqlCollector;
+import org.junit.Test;
+
+import java.util.List;
+import java.util.stream.Collectors;
+
+import static java.util.Arrays.asList;
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class TestInheritanceBothSides extends BaseTestCase {
+
+  @Test
+  public void selectSourceBaseSql() {
+
+    final Query<SourceBase> query = DB.find(SourceBase.class).orderBy("pos");
+    query.findList();
+
+    //assertThat(sqlOf(query)).contains("select t0.dtype, t0.id, t0.name, t0.pos, t0.target_id, t0.target_id from source_base t0 order by t0.pos");
+    assertThat(sqlOf(query)).contains("select t0.dtype, t0.id, t0.name, t0.pos, t0.target_id, t0.target_id from source_base t0 left join target_base t1 on t1.id = t0.target_id and t1.dtype = 'Target1'  order by t0.pos");
+  }
+
+  @Test
+  public void selectSourceASql() {
+
+    final Query<SourceA> query = DB.find(SourceA.class).orderBy("pos");
+    query.findList();
+
+    //assertThat(sqlOf(query)).contains("select t0.id, t0.name, t0.pos, t0.target_id from source_base t0 where t0.dtype = 'SourceA'  order by t0.pos");
+    assertThat(sqlOf(query)).contains("select t0.id, t0.name, t0.pos, t0.target_id from source_base t0 left join target_base t1 on t1.id = t0.target_id and t1.dtype = 'Target1'  where t0.dtype = 'SourceA' order by t0.pos");
+  }
+
+  @Test
+  public void selectSourceAWithJoin() {
+
+    final Query<SourceA> query = DB.find(SourceA.class).fetch("target", "name").orderBy("pos");
+    query.findList();
+
+    assertThat(sqlOf(query)).contains("select t0.id, t0.name, t0.pos, t1.id, t1.name from source_base t0 left join target_base t1 on t1.id = t0.target_id and t1.dtype = 'Target1'  where t0.dtype = 'SourceA' order by t0.pos");
+  }
+
+  @Test
+  public void test() {
+
+    Target1 target1 = new Target1("target 1");
+    Target2 target2 = new Target2("target 2");
+    Target1 target1b = new Target1("target 1b");
+
+    SourceA sourceA = new SourceA("source a", target1, 1);
+    SourceB sourceB = new SourceB("source b", target2, 2);
+    SourceA sourceA2 = new SourceA("source a2", target1b, 3);
+
+    DB.saveAll(asList(sourceA, sourceB, sourceA2));
+
+    final Database db = DB.getDefault();
+
+    final SourceBase foundA = DB.find(SourceBase.class, db.getBeanId(sourceA));
+    final SourceBase foundB = DB.find(SourceBase.class, db.getBeanId(sourceB));
+
+    assertSourceBaseEqual(foundA, sourceA);
+    assertThat(foundA).isInstanceOf(SourceA.class);
+    assertSourceAEqual((SourceA) foundA, sourceA);
+
+    assertSourceBaseEqual(foundB, sourceB);
+    assertThat(foundB).isInstanceOf(SourceB.class);
+    assertSourceBEqual((SourceB) foundB, sourceB);
+
+    assertFetchAllDoubleLazyLoading();
+    assertFetchAllSourceAs();
+  }
+
+  private void assertFetchAllSourceAs() {
+
+    LoggedSqlCollector.start();
+
+    final List<SourceA> sourceAList = DB.find(SourceA.class)
+      .fetch("target", "name")
+      .orderBy("pos")
+      .findList();
+
+    final String joinedNames = sourceAList.stream()
+      .map(it -> "|" + it.getName() + "|" + it.getTarget().getName())
+      .collect(Collectors.joining());
+
+    assertThat(joinedNames).isEqualTo("|source a|target 1|source a2|target 1b");
+
+    final List<String> sql = LoggedSqlCollector.stop();
+    assertThat(sql).hasSize(1);
+    assertThat(sql.get(0)).contains("select t0.id, t0.name, t0.pos, t1.id, t1.name from source_base t0 left join target_base t1 on t1.id = t0.target_id and t1.dtype = 'Target1'  where t0.dtype = 'SourceA' order by t0.pos");
+  }
+
+  /**
+   * Expect separate lazy loading for SourceAs and SourceBs.
+   */
+  private void assertFetchAllDoubleLazyLoading() {
+
+    LoggedSqlCollector.start();
+
+    final List<SourceBase> sources = DB.find(SourceBase.class).orderBy("pos").findList();
+    for (SourceBase source : sources) {
+      if (source instanceof SourceA) {
+        SourceA a = (SourceA) source;
+        final Target1 target = a.getTarget();
+        System.out.println("read target 1 " + target.getId() + " " + target.getName());
+
+      } else if (source instanceof SourceB) {
+        SourceB b = (SourceB) source;
+        final Target2 target = b.getTarget();
+        System.out.println("read target 2 " + target.getId() + " " + target.getName());
+      }
+    }
+
+    final List<String> sql = LoggedSqlCollector.stop();
+
+    assertThat(sql).hasSize(3);
+    //assertThat(sql.get(0)).contains("select t0.dtype, t0.id, t0.name, t0.pos, t0.target_id, t0.target_id from source_base t0 order by t0.pos");
+    assertThat(sql.get(0)).contains("select t0.dtype, t0.id, t0.name, t0.pos, t0.target_id, t0.target_id from source_base t0 left join target_base t1 on t1.id = t0.target_id and t1.dtype = 'Target1'  order by t0.pos");
+    assertThat(sql.get(1)).contains("select t0.id, t0.name from target_base t0 where t0.dtype = 'Target1' and t0.id ");
+    assertThat(sql.get(2)).contains("select t0.id, t0.name from target_base t0 where t0.dtype = 'Target2' and t0.id = ?");
+  }
+
+  private void assertSourceBaseEqual(SourceBase foundA, SourceBase sourceA) {
+    assertThat(foundA.getId()).isEqualTo(sourceA.getId());
+    assertThat(foundA.getName()).isEqualTo(sourceA.getName());
+  }
+
+  private void assertSourceAEqual(SourceA found, SourceA source) {
+    assertThat(found.getTarget().getId()).isEqualTo(source.getTarget().getId());
+    assertThat(found.getTarget().getName()).isEqualTo(source.getTarget().getName());
+    assertThat(found.getTarget().getClass()).isEqualTo(source.getTarget().getClass());
+  }
+
+  private void assertSourceBEqual(SourceB found, SourceB source) {
+
+    final Target2 target = found.getTarget();
+    final String name = target.getName();
+    assertThat(name).isEqualTo(source.getTarget().getName());
+
+    assertThat(found.getTarget().getId()).isEqualTo(source.getTarget().getId());
+    assertThat(found.getTarget().getName()).isEqualTo(source.getTarget().getName());
+    assertThat(found.getTarget().getClass()).isEqualTo(source.getTarget().getClass());
+  }
+}

src/test/java/org/tests/inheritance/bothsides/WithAutoGeneratedUUID.java

@@ -0,0 +1,20 @@
+package org.tests.inheritance.bothsides;
+
+import javax.persistence.Id;
+import javax.persistence.MappedSuperclass;
+import java.util.UUID;
+
+@MappedSuperclass
+public class WithAutoGeneratedUUID {
+
+  @Id
+  UUID id;
+
+  public UUID getId() {
+    return id;
+  }
+
+  public void setId(UUID id) {
+    this.id = id;
+  }
+}