Merge branch 'master' of https://github.com/dastultz/avaje-ebeanorm into dastultz-master

src/test/java/com/avaje/tests/model/basic/MUser.java

@@ -3,10 +3,7 @@ package com.avaje.tests.model.basic;
 import java.util.ArrayList;
 import java.util.List;
 
-import javax.persistence.CascadeType;
-import javax.persistence.Entity;
-import javax.persistence.Id;
-import javax.persistence.ManyToMany;
+import javax.persistence.*;
 
 @Entity
 public class MUser {
@@ -21,6 +18,9 @@ public class MUser {
 	@ManyToMany(mappedBy="users",cascade=CascadeType.ALL)
 	List<MRole> roles;
 	
+	@ManyToOne
+	private MUserType userType;
+	
     public MUser() {
         
     }
@@ -59,5 +59,13 @@ public class MUser {
 	    }
 	    roles.add(role);
 	}
+
+	public MUserType getUserType() {
+		return userType;
+	}
+	
+	public void setUserType(MUserType userType) {
+		this.userType = userType;
+	}
 	
 }

src/test/java/com/avaje/tests/model/basic/MUserType.java

@@ -0,0 +1,42 @@
+package com.avaje.tests.model.basic;
+
+import javax.persistence.Entity;
+import javax.persistence.Id;
+
+@Entity
+public class MUserType {
+
+	@Id
+	Integer id;
+
+	String name;
+
+	public MUserType() {
+		super();
+	}
+
+	public MUserType(String name) {
+		this.name = name;
+	}
+	
+	public Integer getId() {
+		return id;
+	}
+
+	
+	public void setId(Integer id) {
+		this.id = id;
+	}
+
+	
+	public String getName() {
+		return name;
+	}
+
+	
+	public void setName(String name) {
+		this.name = name;
+	}
+
+	
+}

src/test/java/com/avaje/tests/query/orderby/TestOrderByWithDistinct.java

@@ -0,0 +1,82 @@
+package com.avaje.tests.query.orderby;
+
+import java.util.List;
+
+import junit.framework.Assert;
+
+import org.junit.Test;
+
+import com.avaje.ebean.*;
+import com.avaje.tests.model.basic.*;
+
+
+public class TestOrderByWithDistinct extends BaseTestCase {
+	
+	@Test
+	public void test() {
+		/*
+		 * Original conversation:
+		 * https://groups.google.com/forum/?fromgroups=#!topic/ebean/uuvi1btdCDQ%5B1-25-false%5D
+		 * 
+		 * This test exposes what may be a general problem with columns required by the order by phrase being omitted from the select.
+		 * I'm not sure this exposes all causes of the problem.
+		 */
+		
+		MUserType ut = new MUserType("md");
+		Ebean.save(ut);
+		MUser user1 = new MUser("one");
+		user1.setUserType(ut);
+		Ebean.save(user1);
+		MUser user2 = new MUser("two");
+		user2.setUserType(ut);
+		Ebean.save(user2);
+		
+		MRole roleA = new MRole("A");
+		Ebean.save(roleA);
+		MRole roleB = new MRole("B");
+		Ebean.save(roleB);
+		
+		user1.addRole(roleA);
+		Ebean.save(user1);
+		user2.addRole(roleB);
+		Ebean.save(user2);
+		
+		Query<MUser> query = Ebean.find(MUser.class)
+				.where()
+				.eq("roles.roleName", "A")
+				.orderBy("userType.name, userName");
+		List<MUser> list = query.findList();
+		
+		/*
+		 * Produces this query:
+				 select distinct t0.userid c0, t0.user_name c1, t0.user_type_id c2 
+				 from muser t0 
+				 join mrole_muser u1z_ on u1z_.muser_userid = t0.userid  
+				 join mrole u1 on u1.roleid = u1z_.mrole_roleid  
+				 left outer join muser_type t1 on t1.id = t0.user_type_id  
+				 where t1.name = ?  and u1.role_name = ?  
+				 order by t1.name, t0.user_name
+			t1.name is not in select 
+		 */
+		
+		Assert.assertEquals(1, list.size());
+		Assert.assertEquals(user1, list.get(0));
+		
+		// repeat with slight variation, not sure this really produces a different execution path
+		// this problem also manifests when autofetch eliminates properties from the select that aren't used in the objects
+		// still need them to be present for purpose of order by
+		// so here I'm "simulating" a scenario where autofetch has dropped userType.name
+		query = Ebean.find(MUser.class)
+				.setAutofetch(false)
+				.select("userName")
+				.where()
+				.eq("roles.roleName", "A")
+				.orderBy("userType.name");
+		list = query.findList();
+				
+		Assert.assertEquals(1, list.size());
+		Assert.assertEquals(user1, list.get(0));
+		
+	}
+
+}

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<Zone> 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<ZoneInternal> 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<ZoneExternal> 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());
+		
+		
+	}
 }

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<ZoneExternal> 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<ZoneExternal> getShippingZones() {
+		return shippingZones;
+	}
+
+	public void setShippingZones(Set<ZoneExternal> shippingZones) {
+		this.shippingZones = shippingZones;
+	}
+	
+}

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;
+		}
+	}
 }

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() + "\"";
+	}
 }

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() + "\"";
+	}
+}