#2593 - Tidy SqlTreeBuilder to not add duplicate BeanProperties

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

@@ -224,8 +224,7 @@ public final class SqlTreeBuilder {
    * Recursively build the query tree depending on what leaves in the tree
    * should be included.
    */
-  private SqlTreeNode buildSelectChain(String prefix, STreePropertyAssoc prop,
-                                       STreeType desc, List<SqlTreeNode> joinList) {
+  private SqlTreeNode buildSelectChain(String prefix, STreePropertyAssoc prop, STreeType desc, List<SqlTreeNode> joinList) {
     List<SqlTreeNode> myJoinList = new ArrayList<>();
     List<STreePropertyAssocOne> extraProps = new ArrayList<>();
     for (STreePropertyAssocOne one : desc.propsOne()) {
@@ -256,7 +255,7 @@ public final class SqlTreeBuilder {
       }
       addManyWhereJoins(myJoinList);
     }
-    extraProps.forEach(props::add); //addExtra
+    extraProps.forEach(props::addExtra);
 
     if (!rawSql && manyWhereJoins.isFormulaWithJoin(prefix)) {
       for (String property : manyWhereJoins.getFormulaJoinProperties(prefix)) {

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

@@ -33,6 +33,12 @@ public final class SqlTreeProperties {
     propsList.addAll(Arrays.asList(props));
   }
 
+  public void addExtra(STreePropertyAssocOne prop) {
+    if (!propsList.contains(prop)) {
+      add(prop);
+    }
+  }
+
   public void add(STreeProperty prop) {
     propsList.add(prop);
     propNames.add(prop.name());

ebean-test/src/test/java/org/tests/model/nofk/TestNoFk.java

@@ -5,6 +5,7 @@ import io.ebean.DB;
 import io.ebean.SqlRow;
 import io.ebean.bean.EntityBean;
 import io.ebean.bean.EntityBeanIntercept;
+import io.ebean.test.LoggedSql;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
@@ -142,13 +143,16 @@ public class TestNoFk extends BaseTestCase {
     assertTrue(ownerEbi.isReference());
     assertTrue(ownerEbi.isPartial());
     assertTrue(ownerEbi.isLazyLoadFailure());
-
   }
 
   @Test
   public void testEagerLoadFile() {
+    LoggedSql.start();
     List<EFileNoFk> files = DB.find(EFileNoFk.class).fetch("owner").findList();
     assertThat(files).hasSize(2);
+    List<String> sql = LoggedSql.stop();
+    assertThat(sql).hasSize(1);
+    assertThat(sql.get(0)).contains("select t0.file_name, t0.owner_user_id, t0.owner_soft_del_user_id, t1.user_id, t1.user_name from efile_no_fk t0 left join euser_no_fk t1 on t1.user_id = t0.owner_user_id");
 
     EFileNoFk file1 = files.get(0);
     EFileNoFk file2 = files.get(1);
@@ -175,7 +179,6 @@ public class TestNoFk extends BaseTestCase {
     assertTrue(ownerEbi.isReference());
     assertTrue(ownerEbi.isPartial());
     assertTrue(ownerEbi.isLazyLoadFailure());
-
   }
 
   @Test
@@ -259,8 +262,12 @@ public class TestNoFk extends BaseTestCase {
 
   @Test
   public void testEagerLoadFileSoftDel() {
+    LoggedSql.start();
     List<EFileNoFk> files = DB.find(EFileNoFk.class).fetch("ownerSoftDel").findList();
     assertThat(files).hasSize(2);
+    List<String> sql = LoggedSql.stop();
+    assertThat(sql).hasSize(1);
+    assertThat(sql.get(0)).contains("select t0.file_name, t0.owner_user_id, t0.owner_soft_del_user_id, t1.user_id, t1.user_name, t1.user_id is null from efile_no_fk t0 left join euser_no_fk_soft_del t1 on t1.user_id = t0.owner_soft_del_user_id");
 
     EFileNoFk file1 = files.get(0);
     EFileNoFk file2 = files.get(1);

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

@@ -14,7 +14,6 @@ class TestQueryMultiJoinFetchPath extends BaseTestCase {
 
   @Test
   void test() {
-
     HCustomer c1 = new HCustomer("c1", "c1");
     DB.save(c1);
 
@@ -60,9 +59,9 @@ class TestQueryMultiJoinFetchPath extends BaseTestCase {
 
     assertThat(accesses).hasSize(2);
     if (isH2()) {
-      assertThat(query.getGeneratedSql()).isEqualTo("select t0.dtype, t0.id, t0.accessor_id, t0.principal_id, t2.dtype, t0.access_account_number, t0.accessor_id, t2.dtype, t0.access_account_number, t1.cid, t1.name, t2.dtype, t2.account_number from haccess t0 left join hcustomer t1 on t1.cid = t0.accessor_id left join haccount t2 on t2.account_number = t0.access_account_number and t2.dtype = 'B' left join hcustomer t3 on t3.cid = t0.principal_id where t1.status = ? and t3.status = ? and t0.id in (?,?,?,?,?)");
+      assertThat(query.getGeneratedSql()).isEqualTo("select t0.dtype, t0.id, t0.accessor_id, t0.principal_id, t2.dtype, t0.access_account_number, t1.cid, t1.name, t2.dtype, t2.account_number from haccess t0 left join hcustomer t1 on t1.cid = t0.accessor_id left join haccount t2 on t2.account_number = t0.access_account_number and t2.dtype = 'B' left join hcustomer t3 on t3.cid = t0.principal_id where t1.status = ? and t3.status = ? and t0.id in (?,?,?,?,?)");
     } else {
-      assertThat(query.getGeneratedSql()).contains("select t0.dtype, t0.id, t0.accessor_id, t0.principal_id, t2.dtype, t0.access_account_number, t0.accessor_id, t2.dtype, t0.access_account_number, t1.cid, t1.name, t2.dtype, t2.account_number from haccess t0 left join hcustomer t1 on t1.cid = t0.accessor_id left join haccount t2 on t2.account_number = t0.access_account_number and t2.dtype = 'B' left join hcustomer t3 on t3.cid = t0.principal_id where t1.status = ? and t3.status = ? and t0.id ");
+      assertThat(query.getGeneratedSql()).contains("select t0.dtype, t0.id, t0.accessor_id, t0.principal_id, t2.dtype, t0.access_account_number, t1.cid, t1.name, t2.dtype, t2.account_number from haccess t0 left join hcustomer t1 on t1.cid = t0.accessor_id left join haccount t2 on t2.account_number = t0.access_account_number and t2.dtype = 'B' left join hcustomer t3 on t3.cid = t0.principal_id where t1.status = ? and t3.status = ? and t0.id ");
     }
   }