From 9ad4931c12bf75c0fcb5448a0d6fca440668aa88 Mon Sep 17 00:00:00 2001 From: Robin Bygrave Date: Wed, 19 Jun 2013 22:35:42 +1200 Subject: [PATCH] Add fix and test for BUG 408 from Eddie --- .../server/deploy/BeanDescriptorManager.java | 32 +++++++++ .../server/deploy/DbSqlContext.java | 2 +- .../server/deploy/TableJoin.java | 10 ++- .../server/deploy/meta/DeployTableJoin.java | 10 +++ .../server/query/DefaultDbSqlContext.java | 11 ++- .../server/query/SqlTreeNodeBean.java | 3 +- .../inheritance/TestInheritanceJoins.java | 72 +++++++++++++++++++ .../inheritance/model/AbstractBaseClass.java | 16 +++++ .../inheritance/model/CalculationResult.java | 64 +++++++++++++++++ .../inheritance/model/Configuration.java | 44 ++++++++++++ .../inheritance/model/Configurations.java | 43 +++++++++++ .../inheritance/model/GroupConfiguration.java | 41 +++++++++++ .../model/ProductConfiguration.java | 36 ++++++++++ 13 files changed, 379 insertions(+), 5 deletions(-) create mode 100644 src/test/java/com/avaje/tests/inheritance/TestInheritanceJoins.java create mode 100644 src/test/java/com/avaje/tests/inheritance/model/AbstractBaseClass.java create mode 100644 src/test/java/com/avaje/tests/inheritance/model/CalculationResult.java create mode 100644 src/test/java/com/avaje/tests/inheritance/model/Configuration.java create mode 100644 src/test/java/com/avaje/tests/inheritance/model/Configurations.java create mode 100644 src/test/java/com/avaje/tests/inheritance/model/GroupConfiguration.java create mode 100644 src/test/java/com/avaje/tests/inheritance/model/ProductConfiguration.java diff --git a/src/main/java/com/avaje/ebeaninternal/server/deploy/BeanDescriptorManager.java b/src/main/java/com/avaje/ebeaninternal/server/deploy/BeanDescriptorManager.java index 702c65a08..787839547 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/deploy/BeanDescriptorManager.java +++ b/src/main/java/com/avaje/ebeaninternal/server/deploy/BeanDescriptorManager.java @@ -610,6 +610,11 @@ public class BeanDescriptorManager implements BeanDescriptorMap { secondaryPropsJoins(info); } + // Set inheritance info + for (DeployBeanInfo info : deplyInfoMap.values()) { + setInheritanceInfo(info); + } + for (DeployBeanInfo info : deplyInfoMap.values()) { DeployBeanDescriptor deployBeanDescriptor = info.getDescriptor(); Integer key = getUniqueHash(deployBeanDescriptor); @@ -617,6 +622,33 @@ public class BeanDescriptorManager implements BeanDescriptorMap { } } + /** + * Sets the inheritance info. ~EMG fix for join problem + * + * @param info the new inheritance info + */ + private void setInheritanceInfo(DeployBeanInfo info) { + for (DeployBeanPropertyAssocOne oneProp : info.getDescriptor().propertiesAssocOne()) { + if (!oneProp.isTransient()) { + DeployBeanInfo assoc = deplyInfoMap.get(oneProp.getTargetType()); + + if (assoc != null){ + oneProp.getTableJoin().setInheritInfo(assoc.getDescriptor().getInheritInfo()); + } + } + } + + for (DeployBeanPropertyAssocMany manyProp : info.getDescriptor().propertiesAssocMany()) { + if (!manyProp.isTransient()) { + DeployBeanInfo assoc = deplyInfoMap.get(manyProp.getTargetType()); + + if (assoc != null){ + manyProp.getTableJoin().setInheritInfo(assoc.getDescriptor().getInheritInfo()); + } + } + } + } + private Integer getUniqueHash(DeployBeanDescriptor deployBeanDescriptor) { int hashCode = deployBeanDescriptor.getFullName().hashCode(); diff --git a/src/main/java/com/avaje/ebeaninternal/server/deploy/DbSqlContext.java b/src/main/java/com/avaje/ebeaninternal/server/deploy/DbSqlContext.java index 482d2c958..48dcbc831 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/deploy/DbSqlContext.java +++ b/src/main/java/com/avaje/ebeaninternal/server/deploy/DbSqlContext.java @@ -8,7 +8,7 @@ public interface DbSqlContext { /** * Add a join to the sql query. */ - public void addJoin(String type, String table, TableJoinColumn[] cols, String a1, String a2); + public void addJoin(String type, String table, TableJoinColumn[] cols, String a1, String a2, String inheritance); public void pushSecondaryTableAlias(String alias); diff --git a/src/main/java/com/avaje/ebeaninternal/server/deploy/TableJoin.java b/src/main/java/com/avaje/ebeaninternal/server/deploy/TableJoin.java index 609ddd93f..069e77c47 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/deploy/TableJoin.java +++ b/src/main/java/com/avaje/ebeaninternal/server/deploy/TableJoin.java @@ -40,6 +40,8 @@ public final class TableJoin { */ private final BeanCascadeInfo cascadeInfo; + private final InheritInfo inheritInfo; + /** * Properties as an array. */ @@ -59,6 +61,7 @@ public final class TableJoin { this.table = InternString.intern(deploy.getTable()); this.type = InternString.intern(deploy.getType()); this.cascadeInfo = deploy.getCascadeInfo(); + this.inheritInfo = deploy.getInheritInfo(); DeployTableJoinColumn[] deployCols = deploy.columns(); this.columns = new TableJoinColumn[deployCols.length]; @@ -167,8 +170,10 @@ public final class TableJoin { public boolean addJoin(boolean forceOuterJoin, String a1, String a2, DbSqlContext ctx) { - ctx.addJoin(forceOuterJoin ? LEFT_OUTER : type, table, columns(), a1, a2); + String inheritance = inheritInfo != null ? inheritInfo.getWhere() : null; + ctx.addJoin(forceOuterJoin?LEFT_OUTER:type, table, columns(), a1, a2, inheritance); + return forceOuterJoin || LEFT_OUTER.equals(type); } @@ -176,6 +181,7 @@ public final class TableJoin { * Explicitly add a (non-outer) join. */ public void addInnerJoin(String a1, String a2, DbSqlContext ctx) { - ctx.addJoin(JOIN, table, columns(), a1, a2); + String inheritance = inheritInfo != null ? inheritInfo.getWhere() : null; + ctx.addJoin(JOIN, table, columns(), a1, a2, inheritance); } } diff --git a/src/main/java/com/avaje/ebeaninternal/server/deploy/meta/DeployTableJoin.java b/src/main/java/com/avaje/ebeaninternal/server/deploy/meta/DeployTableJoin.java index bfbc3b82d..d77c21d0c 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/deploy/meta/DeployTableJoin.java +++ b/src/main/java/com/avaje/ebeaninternal/server/deploy/meta/DeployTableJoin.java @@ -7,6 +7,7 @@ import javax.persistence.JoinColumn; import com.avaje.ebeaninternal.server.core.Message; import com.avaje.ebeaninternal.server.deploy.BeanCascadeInfo; import com.avaje.ebeaninternal.server.deploy.BeanTable; +import com.avaje.ebeaninternal.server.deploy.InheritInfo; import com.avaje.ebeaninternal.server.deploy.TableJoin; /** @@ -48,6 +49,7 @@ public class DeployTableJoin { */ private BeanCascadeInfo cascadeInfo = new BeanCascadeInfo(); + private InheritInfo inheritInfo; /** * Create a DeployTableJoin. @@ -205,4 +207,12 @@ public class DeployTableJoin { return destJoin; } + + public InheritInfo getInheritInfo() { + return inheritInfo; + } + + public void setInheritInfo(InheritInfo inheritInfo) { + this.inheritInfo = inheritInfo; + } } diff --git a/src/main/java/com/avaje/ebeaninternal/server/query/DefaultDbSqlContext.java b/src/main/java/com/avaje/ebeaninternal/server/query/DefaultDbSqlContext.java index 98ea65809..6adbf6e84 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/query/DefaultDbSqlContext.java +++ b/src/main/java/com/avaje/ebeaninternal/server/query/DefaultDbSqlContext.java @@ -93,7 +93,7 @@ public class DefaultDbSqlContext implements DbSqlContext { joinStack.push(node); } - public void addJoin(String type, String table, TableJoinColumn[] cols, String a1, String a2) { + public void addJoin(String type, String table, TableJoinColumn[] cols, String a1, String a2, String inheritance) { if (tableJoins == null) { tableJoins = new HashSet(); @@ -125,6 +125,15 @@ public class DefaultDbSqlContext implements DbSqlContext { sb.append(".").append(pair.getLocalDbColumn()); } + + // add on any inheritance where clause + if (inheritance != null && inheritance.length() > 0){ + sb.append(" and "); + sb.append(a2); + sb.append("."); + sb.append(inheritance); + } + sb.append(" "); } diff --git a/src/main/java/com/avaje/ebeaninternal/server/query/SqlTreeNodeBean.java b/src/main/java/com/avaje/ebeaninternal/server/query/SqlTreeNodeBean.java index a73727211..97f207ef5 100644 --- a/src/main/java/com/avaje/ebeaninternal/server/query/SqlTreeNodeBean.java +++ b/src/main/java/com/avaje/ebeaninternal/server/query/SqlTreeNodeBean.java @@ -423,7 +423,8 @@ public class SqlTreeNodeBean implements SqlTreeNode { public void appendWhere(DbSqlContext ctx) { - if (inheritInfo != null) { + // Only apply inheritance to root node as any join will alreay have the inheritance join include - see TableJoin + if (inheritInfo != null && nodeBeanProp == null) { if (inheritInfo.isRoot()) { // at root of hierarchy so don't bother // adding a where clause because we want diff --git a/src/test/java/com/avaje/tests/inheritance/TestInheritanceJoins.java b/src/test/java/com/avaje/tests/inheritance/TestInheritanceJoins.java new file mode 100644 index 000000000..38521b93f --- /dev/null +++ b/src/test/java/com/avaje/tests/inheritance/TestInheritanceJoins.java @@ -0,0 +1,72 @@ +package com.avaje.tests.inheritance; + +import java.util.List; + +import junit.framework.Assert; + +import org.junit.Test; + +import com.avaje.ebean.BaseTestCase; +import com.avaje.ebean.Ebean; +import com.avaje.ebean.EbeanServer; +import com.avaje.ebean.Query; +import com.avaje.tests.inheritance.model.CalculationResult; +import com.avaje.tests.inheritance.model.Configurations; +import com.avaje.tests.inheritance.model.GroupConfiguration; +import com.avaje.tests.inheritance.model.ProductConfiguration; + +public class TestInheritanceJoins extends BaseTestCase { + + + @Test + public void testAssocOne() { + + EbeanServer server = Ebean.getServer(null); + + final ProductConfiguration pc = new ProductConfiguration(); + pc.setName("PC1"); + server.save(pc); + + final GroupConfiguration gc = new GroupConfiguration(); + gc.setName("GC1"); + server.save(gc); + + CalculationResult r = new CalculationResult(); + final Double charge = 100.0; + r.setCharge(charge); + r.setProductConfiguration(pc); + r.setGroupConfiguration(gc); + server.save(r); + + + Query q = server.createNamedQuery(CalculationResult.class, "loadResult"); + q.setParameter("charge", charge); + + List results = q.findList(); + + Assert.assertTrue(!results.isEmpty()); + } + + @Test + public void testAssocMany() { + Configurations configurations = new Configurations(); + + EbeanServer server = Ebean.getServer(null); + + server.save(configurations); + + + final GroupConfiguration gc = new GroupConfiguration("GC1"); + configurations.add(gc); + + + server.save(gc); + + + Configurations configurationsQueried = server.find(Configurations.class, configurations.getId()); + + List groups = configurationsQueried.getGroupConfigurations(); + + Assert.assertTrue(!groups.isEmpty()); + } +} \ No newline at end of file diff --git a/src/test/java/com/avaje/tests/inheritance/model/AbstractBaseClass.java b/src/test/java/com/avaje/tests/inheritance/model/AbstractBaseClass.java new file mode 100644 index 000000000..482a995cb --- /dev/null +++ b/src/test/java/com/avaje/tests/inheritance/model/AbstractBaseClass.java @@ -0,0 +1,16 @@ +package com.avaje.tests.inheritance.model; + +import javax.persistence.MappedSuperclass; + +@MappedSuperclass +public class AbstractBaseClass { + private String name; + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } +} diff --git a/src/test/java/com/avaje/tests/inheritance/model/CalculationResult.java b/src/test/java/com/avaje/tests/inheritance/model/CalculationResult.java new file mode 100644 index 000000000..6d6303067 --- /dev/null +++ b/src/test/java/com/avaje/tests/inheritance/model/CalculationResult.java @@ -0,0 +1,64 @@ +package com.avaje.tests.inheritance.model; + +import javax.persistence.CascadeType; +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.ManyToOne; +import javax.persistence.NamedQueries; +import javax.persistence.NamedQuery; + +@Entity +@NamedQueries({ + @NamedQuery(name="loadResult", + query="find CalculationResult " + + "fetch productConfiguration "+ + "fetch groupConfiguration "+ + "where charge = :charge") +}) +public class CalculationResult { + + @Id + @Column(name="ID") + private Integer id; + + private double charge; + + @ManyToOne(cascade=CascadeType.PERSIST) + private ProductConfiguration productConfiguration; + + @ManyToOne(cascade=CascadeType.PERSIST) + private GroupConfiguration groupConfiguration; + + public double getCharge() { + return charge; + } + + public void setCharge(double charge) { + this.charge = charge; + } + + public ProductConfiguration getProductConfiguration() { + return productConfiguration; + } + + public void setProductConfiguration(ProductConfiguration productConfiguration) { + this.productConfiguration = productConfiguration; + } + + public GroupConfiguration getGroupConfiguration() { + return groupConfiguration; + } + + public void setGroupConfiguration(GroupConfiguration groupConfiguration) { + this.groupConfiguration = groupConfiguration; + } + + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } +} diff --git a/src/test/java/com/avaje/tests/inheritance/model/Configuration.java b/src/test/java/com/avaje/tests/inheritance/model/Configuration.java new file mode 100644 index 000000000..fff28fc5a --- /dev/null +++ b/src/test/java/com/avaje/tests/inheritance/model/Configuration.java @@ -0,0 +1,44 @@ +package com.avaje.tests.inheritance.model; + +import javax.persistence.Column; +import javax.persistence.DiscriminatorColumn; +import javax.persistence.DiscriminatorType; +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.Inheritance; +import javax.persistence.InheritanceType; +import javax.persistence.ManyToOne; + +@Entity +@Inheritance(strategy=InheritanceType.SINGLE_TABLE) +@DiscriminatorColumn(name="type", discriminatorType=DiscriminatorType.STRING) +public class Configuration extends AbstractBaseClass{ + @Id + @Column(name="ID") + private Integer id; + + + @ManyToOne + private Configurations configurations; + + + public Configuration(){ + super(); + } + + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + + public Configurations getConfigurations() { + return configurations; + } + + public void setConfigurations(Configurations configurations) { + this.configurations = configurations; + } +} diff --git a/src/test/java/com/avaje/tests/inheritance/model/Configurations.java b/src/test/java/com/avaje/tests/inheritance/model/Configurations.java new file mode 100644 index 000000000..cfd7425a1 --- /dev/null +++ b/src/test/java/com/avaje/tests/inheritance/model/Configurations.java @@ -0,0 +1,43 @@ +package com.avaje.tests.inheritance.model; + +import java.util.List; + +import javax.persistence.Column; +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.OneToMany; + +@Entity +public class Configurations { + @Id + @Column(name="ID") + private Integer id; + + @OneToMany + private List groupConfigurations; + + + public Integer getId() { + return id; + } + + + public void setId(Integer id) { + this.id = id; + } + + + public List getGroupConfigurations() { + return groupConfigurations; + } + + + public void setGroupConfigurations(List groupConfigurations) { + this.groupConfigurations = groupConfigurations; + } + + public void add(GroupConfiguration groupConfiguration){ + groupConfiguration.setConfigurations(this); + groupConfigurations.add(groupConfiguration); + } +} diff --git a/src/test/java/com/avaje/tests/inheritance/model/GroupConfiguration.java b/src/test/java/com/avaje/tests/inheritance/model/GroupConfiguration.java new file mode 100644 index 000000000..808819c03 --- /dev/null +++ b/src/test/java/com/avaje/tests/inheritance/model/GroupConfiguration.java @@ -0,0 +1,41 @@ +package com.avaje.tests.inheritance.model; + +import java.util.List; + +import javax.persistence.DiscriminatorValue; +import javax.persistence.Entity; +import javax.persistence.OneToMany; + +@Entity +@DiscriminatorValue("2") +public class GroupConfiguration extends Configuration { + private String groupName; + + @OneToMany(mappedBy="groupConfiguration") + private List results; + + public GroupConfiguration(){ + super(); + } + + public GroupConfiguration(String name){ + super(); + this.groupName = name; + } + + public String getGroupName() { + return groupName; + } + + public void setGroupName(String groupName) { + this.groupName = groupName; + } + + public List getResults() { + return results; + } + + public void setResults(List results) { + this.results = results; + } +} diff --git a/src/test/java/com/avaje/tests/inheritance/model/ProductConfiguration.java b/src/test/java/com/avaje/tests/inheritance/model/ProductConfiguration.java new file mode 100644 index 000000000..bc1dd6a84 --- /dev/null +++ b/src/test/java/com/avaje/tests/inheritance/model/ProductConfiguration.java @@ -0,0 +1,36 @@ +package com.avaje.tests.inheritance.model; + +import java.util.List; + +import javax.persistence.DiscriminatorValue; +import javax.persistence.Entity; +import javax.persistence.OneToMany; + +@Entity +@DiscriminatorValue("1") +public class ProductConfiguration extends Configuration { + private String productName; + + @OneToMany(mappedBy="productConfiguration") + private List results; + + public ProductConfiguration(){ + super(); + } + + public String getProductName() { + return productName; + } + + public void setProductName(String productName) { + this.productName = productName; + } + + public List getResults() { + return results; + } + + public void setResults(List results) { + this.results = results; + } +}