From 2de57f7b35764bb814436ec00e7ae66678530b16 Mon Sep 17 00:00:00 2001 From: Daryl Stultz Date: Wed, 8 May 2013 14:32:57 -0400 Subject: [PATCH] Exposed bug 417 (and 408?) --- .../TestInheritQuery.java | 126 +++++++++++++++++- .../model/Warehouse.java | 49 +++++++ .../singleTableInheritance/model/Zone.java | 14 ++ .../model/ZoneExternal.java | 5 + .../model/ZoneInternal.java | 24 ++++ 5 files changed, 213 insertions(+), 5 deletions(-) create mode 100644 src/test/java/com/avaje/tests/singleTableInheritance/model/Warehouse.java create mode 100644 src/test/java/com/avaje/tests/singleTableInheritance/model/ZoneInternal.java diff --git a/src/test/java/com/avaje/tests/singleTableInheritance/TestInheritQuery.java b/src/test/java/com/avaje/tests/singleTableInheritance/TestInheritQuery.java index 048191c10..347b4ecc7 100644 --- a/src/test/java/com/avaje/tests/singleTableInheritance/TestInheritQuery.java +++ b/src/test/java/com/avaje/tests/singleTableInheritance/TestInheritQuery.java @@ -8,16 +8,12 @@ import org.junit.Test; import com.avaje.ebean.BaseTestCase; import com.avaje.ebean.Ebean; -import com.avaje.tests.singleTableInheritance.model.PalletLocation; -import com.avaje.tests.singleTableInheritance.model.PalletLocationExternal; -import com.avaje.tests.singleTableInheritance.model.Zone; -import com.avaje.tests.singleTableInheritance.model.ZoneExternal; +import com.avaje.tests.singleTableInheritance.model.*; public class TestInheritQuery extends BaseTestCase { @Test public void test() { - ZoneExternal zone = new ZoneExternal(); zone.setAttribute("ABC"); Ebean.save(zone); @@ -42,4 +38,124 @@ public class TestInheritQuery extends BaseTestCase { Assert.assertNotNull(rereadZone); Assert.assertTrue(rereadZone instanceof ZoneExternal); } + + @Test + public void testDiscriminator_bug417() { + ZoneInternal zoneInt = new ZoneInternal(); + zoneInt.setAttribute("some zone 1"); + Ebean.save(zoneInt); + + ZoneExternal zoneExt = new ZoneExternal(); + zoneExt.setAttribute("some zone 2"); + Ebean.save(zoneExt); + + // queries of Zone and subclasses as root node of query + + // query abstract class on attribute (root of heirarchy) + List zones = Ebean.find(Zone.class).where().startsWith("attribute", "some zone").findList(); + // select t0.type c0, t0.ID c1, t0.attribute c2, t0.attribute c3 from zones t0 where t0.attribute like ? ; --bind(some zone%) + Assert.assertEquals(2, zones.size()); + Assert.assertTrue(zones.contains(zoneInt)); + Assert.assertTrue(zones.contains(zoneExt)); + + // query internal zones only + // discriminator is in WHERE clause where it belongs + List internalZones = Ebean.find(ZoneInternal.class).where().startsWith("attribute", "some zone").findList(); + // select t0.type c0, t0.ID c1, t0.attribute c2 from zones t0 where t0.type = 'INT' and t0.attribute like ? ; --bind(some zone%) + Assert.assertEquals(1, internalZones.size()); + Assert.assertTrue(internalZones.contains(zoneInt)); + Assert.assertFalse(internalZones.contains(zoneExt)); + Assert.assertTrue(internalZones.get(0) instanceof ZoneInternal); + + // query external zones only + List externalZones = Ebean.find(ZoneExternal.class).where().startsWith("attribute", "some zone").findList(); + // select t0.type c0, t0.ID c1, t0.attribute c2 from zones t0 where t0.type = 'EXT' and t0.attribute like ? ; --bind(some zone%) + Assert.assertEquals(1, externalZones.size()); + Assert.assertTrue(externalZones.contains(zoneExt)); + Assert.assertFalse(externalZones.contains(zoneInt)); + Assert.assertTrue(externalZones.get(0) instanceof ZoneExternal); + + // parents with children of Zones and subclasses + + Warehouse wh = new Warehouse(); + wh.setOfficeZone(zoneInt); // many-to-one + wh.getShippingZones().add(zoneExt); // many-to-many + Ebean.save(wh); + + // JOIN clause, no discriminator + // parent with many-to-one, doesn't put in discriminator, why not, PK sufficient? + // eager join + Warehouse wh2 = Ebean.find(Warehouse.class, wh.getId()); + // select t0.ID c0, t1.type c1, t0.officeZoneId c2 from warehouses t0 left outer join zones t1 on t1.ID = t0.officeZoneId where t0.ID = ? ; --bind(1) + Assert.assertNotNull(wh2); + Assert.assertEquals(wh.getId(), wh2.getId()); + Assert.assertEquals(wh.getOfficeZone(), wh2.getOfficeZone()); + Assert.assertEquals(wh.getOfficeZone().getAttribute(), wh2.getOfficeZone().getAttribute()); + + // before the fix, next assertion runs this lazy query: + + // select t0.ID c0, t1.type c1, t1.ID c2 from warehouses t0 + // left outer join WarehousesShippingZones t1z_ on t1z_.warehouseId = t0.ID + // left outer join zones t1 on t1.ID = t1z_.shippingZoneId + // where t1.type = 'EXT' // this should be in the join clause + // and t0.ID = ? + // order by t0.ID; --bind(1) + + // this works here because we have at least one shipping zone + + Assert.assertEquals(1, wh2.getShippingZones().size()); + Assert.assertTrue(wh2.getShippingZones().contains(zoneExt)); + + // set optional concrete to null to set stage for failure + wh.setOfficeZone(null); + Ebean.save(wh); + + // no discriminator here + wh2 = Ebean.find(Warehouse.class) + .where().eq("id", wh.getId()) + .findUnique(); + + Assert.assertNotNull(wh2); + // discriminator is used here, should be in join + // assuming this "manual" fetch is equivalent to autofetch (i.e., autofetch should work the same way) + // before Daryl's fix + // select t0.ID c0, t1.type c1, t1.ID c2, t1.attribute c3 from warehouses t0 left outer join zones t1 on t1.ID = t0.officeZoneId where t1.type = 'INT' and t0.ID = ? + // todo: after Daryl's fix, not sure if this is proper, no discriminator at all, isn't PK/FK sufficient? + // select t0.ID c0, t1.type c1, t1.ID c2, t1.attribute c3 from warehouses t0 left outer join zones t1 on t1.ID = t0.officeZoneId where t0.ID = ? ; --bind(1) + wh2 = Ebean.find(Warehouse.class) + .fetch("officeZone") + .where().eq("id", wh.getId()) + .findUnique(); + // key assertion #1 - fails due to left join with discriminator in WHERE + Assert.assertNotNull(wh2); + + // clear children to set the stage for left join failure + wh.getShippingZones().clear(); + Ebean.save(wh); + + wh2 = Ebean.find(Warehouse.class, wh.getId()); + Assert.assertNotNull(wh2); + Assert.assertEquals(wh.getId(), wh2.getId()); + Assert.assertEquals(0, wh.getShippingZones().size()); + + // query with lazy load of abstract children + wh = Ebean.find(Warehouse.class) + .where().eq("id", wh.getId()) + .findUnique(); + Assert.assertNotNull(wh); + Assert.assertEquals(wh.getId(), wh2.getId()); + Assert.assertEquals(0, wh.getShippingZones().size()); + + // query with fetch of abstract children + wh = Ebean.find(Warehouse.class) + .fetch("shippingZones") + .where().eq("id", wh.getId()) + .findUnique(); + // key assertion #2 - fails due to left join with discriminator in WHERE + Assert.assertNotNull(wh); + Assert.assertEquals(wh.getId(), wh2.getId()); + Assert.assertEquals(0, wh.getShippingZones().size()); + + + } } \ No newline at end of file diff --git a/src/test/java/com/avaje/tests/singleTableInheritance/model/Warehouse.java b/src/test/java/com/avaje/tests/singleTableInheritance/model/Warehouse.java new file mode 100644 index 000000000..30d66b849 --- /dev/null +++ b/src/test/java/com/avaje/tests/singleTableInheritance/model/Warehouse.java @@ -0,0 +1,49 @@ +package com.avaje.tests.singleTableInheritance.model; + +import java.util.Set; + +import javax.persistence.*; + +@Entity +@Table(name="warehouses") +public class Warehouse { + @Id + @Column(name="ID") + private Integer id; + + @ManyToOne//(optional = false) //todo: should this be nullable with assertions made? + @JoinColumn(name = "officeZoneId") + private ZoneInternal officeZone; + + @ManyToMany(cascade = CascadeType.PERSIST) + @JoinTable(name = "WarehousesShippingZones", + joinColumns = { @JoinColumn(name = "warehouseId", referencedColumnName = "ID") }, + inverseJoinColumns = { @JoinColumn(name = "shippingZoneId", referencedColumnName = "ID") } + ) + private Set shippingZones; + + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + + public ZoneInternal getOfficeZone() { + return officeZone; + } + + public void setOfficeZone(ZoneInternal officeZone) { + this.officeZone = officeZone; + } + + public Set getShippingZones() { + return shippingZones; + } + + public void setShippingZones(Set shippingZones) { + this.shippingZones = shippingZones; + } + +} diff --git a/src/test/java/com/avaje/tests/singleTableInheritance/model/Zone.java b/src/test/java/com/avaje/tests/singleTableInheritance/model/Zone.java index cd0b48f9e..9f0d002ce 100644 --- a/src/test/java/com/avaje/tests/singleTableInheritance/model/Zone.java +++ b/src/test/java/com/avaje/tests/singleTableInheritance/model/Zone.java @@ -28,4 +28,18 @@ public class Zone { this.id = id; } + + public int hashCode() { + if (getId() != null) return getId().hashCode(); + else return super.hashCode(); + } + + @Override + public boolean equals(Object obj) { + if (obj instanceof Zone) { + return ((Zone) obj).getId().equals(getId()); + } else { + return false; + } + } } diff --git a/src/test/java/com/avaje/tests/singleTableInheritance/model/ZoneExternal.java b/src/test/java/com/avaje/tests/singleTableInheritance/model/ZoneExternal.java index 3b1fd1bd8..c3b24734e 100644 --- a/src/test/java/com/avaje/tests/singleTableInheritance/model/ZoneExternal.java +++ b/src/test/java/com/avaje/tests/singleTableInheritance/model/ZoneExternal.java @@ -18,4 +18,9 @@ public class ZoneExternal extends Zone { this.attribute = attribute; } + + @Override + public String toString() { + return "ZoneExternal " + getId() + " \"" + getAttribute() + "\""; + } } diff --git a/src/test/java/com/avaje/tests/singleTableInheritance/model/ZoneInternal.java b/src/test/java/com/avaje/tests/singleTableInheritance/model/ZoneInternal.java new file mode 100644 index 000000000..b7e57bb5e --- /dev/null +++ b/src/test/java/com/avaje/tests/singleTableInheritance/model/ZoneInternal.java @@ -0,0 +1,24 @@ +package com.avaje.tests.singleTableInheritance.model; + +import javax.persistence.DiscriminatorValue; +import javax.persistence.Entity; + +@Entity +@DiscriminatorValue("INT") +public class ZoneInternal extends Zone { + + private String attribute; + + public String getAttribute() { + return attribute; + } + + public void setAttribute(String attribute) { + this.attribute = attribute; + } + + @Override + public String toString() { + return "ZoneInternal " + getId() + " \"" + getAttribute() + "\""; + } +}