From f2d422ff5aee083f231b33a59d66f5a2a0309f57 Mon Sep 17 00:00:00 2001 From: rbygrave Date: Thu, 10 Jul 2014 01:20:51 +1200 Subject: [PATCH] Test case for #165 - Incorrect join used when query language with where expression has a many property and ends with a foreign key column --- .../java/com/avaje/tests/model/BaseModel.java | 60 +++++++++ .../tests/model/basic/ResetBasicData.java | 1 + .../model/converstation/Conversation.java | 62 +++++++++ .../tests/model/converstation/Group.java | 46 +++++++ .../tests/model/converstation/Message.java | 56 ++++++++ .../model/converstation/Participation.java | 60 +++++++++ .../avaje/tests/model/converstation/User.java | 55 ++++++++ .../other/TestQueryConversationRowCount.java | 125 ++++++++++++++++++ 8 files changed, 465 insertions(+) create mode 100644 src/test/java/com/avaje/tests/model/BaseModel.java create mode 100644 src/test/java/com/avaje/tests/model/converstation/Conversation.java create mode 100644 src/test/java/com/avaje/tests/model/converstation/Group.java create mode 100644 src/test/java/com/avaje/tests/model/converstation/Message.java create mode 100644 src/test/java/com/avaje/tests/model/converstation/Participation.java create mode 100644 src/test/java/com/avaje/tests/model/converstation/User.java create mode 100644 src/test/java/com/avaje/tests/query/other/TestQueryConversationRowCount.java diff --git a/src/test/java/com/avaje/tests/model/BaseModel.java b/src/test/java/com/avaje/tests/model/BaseModel.java new file mode 100644 index 000000000..638cb3aa5 --- /dev/null +++ b/src/test/java/com/avaje/tests/model/BaseModel.java @@ -0,0 +1,60 @@ +package com.avaje.tests.model; + +import java.sql.Timestamp; + +import javax.persistence.Id; +import javax.persistence.MappedSuperclass; +import javax.persistence.Version; + +import com.avaje.ebean.Model; +import com.avaje.ebean.annotation.CreatedTimestamp; +import com.avaje.ebean.annotation.UpdatedTimestamp; + +@MappedSuperclass +public class BaseModel extends Model { + + @Id + Long id; + + @Version + Long version; + + @CreatedTimestamp + Timestamp whenCreated; + + @UpdatedTimestamp + Timestamp whenUpdated; + + public Long getId() { + return id; + } + + public void setId(Long id) { + this.id = id; + } + + public Long getVersion() { + return version; + } + + public void setVersion(Long version) { + this.version = version; + } + + public Timestamp getWhenCreated() { + return whenCreated; + } + + public void setWhenCreated(Timestamp whenCreated) { + this.whenCreated = whenCreated; + } + + public Timestamp getWhenUpdated() { + return whenUpdated; + } + + public void setWhenUpdated(Timestamp whenUpdated) { + this.whenUpdated = whenUpdated; + } + +} diff --git a/src/test/java/com/avaje/tests/model/basic/ResetBasicData.java b/src/test/java/com/avaje/tests/model/basic/ResetBasicData.java index f6b586edb..afb8ca66b 100644 --- a/src/test/java/com/avaje/tests/model/basic/ResetBasicData.java +++ b/src/test/java/com/avaje/tests/model/basic/ResetBasicData.java @@ -298,6 +298,7 @@ public class ResetBasicData { List details = new ArrayList(); details.add(new OrderDetail(product1, 3, 10.50)); details.add(new OrderDetail(product3, 40, 2.10)); + details.add(new OrderDetail(product1, 5, 10.00)); order.setDetails(details); order.addShipment(new OrderShipment()); diff --git a/src/test/java/com/avaje/tests/model/converstation/Conversation.java b/src/test/java/com/avaje/tests/model/converstation/Conversation.java new file mode 100644 index 000000000..63088bfe2 --- /dev/null +++ b/src/test/java/com/avaje/tests/model/converstation/Conversation.java @@ -0,0 +1,62 @@ +package com.avaje.tests.model.converstation; + +import java.util.List; + +import javax.persistence.Entity; +import javax.persistence.ManyToOne; +import javax.persistence.OneToMany; +import javax.persistence.Table; + +import com.avaje.tests.model.BaseModel; + +@Entity +@Table(name="c_conversation") +public class Conversation extends BaseModel { + + String title; + + boolean open; + + @ManyToOne + Group group; + + @OneToMany(mappedBy="conversation") + List participants; + + @OneToMany(mappedBy="conversation") + List messages; + + + public String getTitle() { + return title; + } + + public void setTitle(String title) { + this.title = title; + } + + public boolean isOpen() { + return open; + } + + public void setOpen(boolean open) { + this.open = open; + } + + public Group getGroup() { + return group; + } + + public void setGroup(Group group) { + this.group = group; + } + + public List getMessages() { + return messages; + } + + public void setMessages(List messages) { + this.messages = messages; + } + +} diff --git a/src/test/java/com/avaje/tests/model/converstation/Group.java b/src/test/java/com/avaje/tests/model/converstation/Group.java new file mode 100644 index 000000000..4359d5ea4 --- /dev/null +++ b/src/test/java/com/avaje/tests/model/converstation/Group.java @@ -0,0 +1,46 @@ +package com.avaje.tests.model.converstation; + +import java.util.List; + +import javax.persistence.Entity; +import javax.persistence.OneToMany; +import javax.persistence.Table; + +import com.avaje.tests.model.BaseModel; + +@Entity +@Table(name="c_group") +public class Group extends BaseModel { + + boolean inactive; + + String name; + + @OneToMany(mappedBy="group") + List users; + + public boolean isInactive() { + return inactive; + } + + public void setInactive(boolean inactive) { + this.inactive = inactive; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public List getUsers() { + return users; + } + + public void setUsers(List users) { + this.users = users; + } + +} diff --git a/src/test/java/com/avaje/tests/model/converstation/Message.java b/src/test/java/com/avaje/tests/model/converstation/Message.java new file mode 100644 index 000000000..ec0b34ebe --- /dev/null +++ b/src/test/java/com/avaje/tests/model/converstation/Message.java @@ -0,0 +1,56 @@ +package com.avaje.tests.model.converstation; + +import javax.persistence.Entity; +import javax.persistence.ManyToOne; +import javax.persistence.Table; + +import com.avaje.tests.model.BaseModel; + +@Entity +@Table(name="c_message") +public class Message extends BaseModel { + + String title; + + String body; + + @ManyToOne + Conversation conversation; + + @ManyToOne + User user; + + + public Conversation getConversation() { + return conversation; + } + + public void setConversation(Conversation conversation) { + this.conversation = conversation; + } + + public String getTitle() { + return title; + } + + public void setTitle(String title) { + this.title = title; + } + + public String getBody() { + return body; + } + + public void setBody(String body) { + this.body = body; + } + + public User getUser() { + return user; + } + + public void setUser(User user) { + this.user = user; + } + +} diff --git a/src/test/java/com/avaje/tests/model/converstation/Participation.java b/src/test/java/com/avaje/tests/model/converstation/Participation.java new file mode 100644 index 000000000..0cf30b28d --- /dev/null +++ b/src/test/java/com/avaje/tests/model/converstation/Participation.java @@ -0,0 +1,60 @@ +package com.avaje.tests.model.converstation; + +import javax.persistence.Entity; +import javax.persistence.ManyToOne; +import javax.persistence.Table; + +import com.avaje.tests.model.BaseModel; + +@Entity +@Table(name="c_participation") +public class Participation extends BaseModel { + + public enum Type { + Moderator, + Member + } + + Integer rating; + + Type type; + + @ManyToOne(optional=false) + Conversation conversation; + + @ManyToOne(optional=false) + User user; + + public Conversation getConversation() { + return conversation; + } + + public void setConversation(Conversation conversation) { + this.conversation = conversation; + } + + public Type getType() { + return type; + } + + public void setType(Type type) { + this.type = type; + } + + public Integer getRating() { + return rating; + } + + public void setRating(Integer rating) { + this.rating = rating; + } + + public User getUser() { + return user; + } + + public void setUser(User user) { + this.user = user; + } + +} diff --git a/src/test/java/com/avaje/tests/model/converstation/User.java b/src/test/java/com/avaje/tests/model/converstation/User.java new file mode 100644 index 000000000..0b94cabeb --- /dev/null +++ b/src/test/java/com/avaje/tests/model/converstation/User.java @@ -0,0 +1,55 @@ +package com.avaje.tests.model.converstation; + +import javax.persistence.Entity; +import javax.persistence.ManyToOne; +import javax.persistence.Table; + +import com.avaje.tests.model.BaseModel; + +@Entity +@Table(name="c_user") +public class User extends BaseModel { + + boolean inactive; + + String name; + + String email; + + @ManyToOne + Group group; + + + public boolean isInactive() { + return inactive; + } + + public void setInactive(boolean inactive) { + this.inactive = inactive; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public String getEmail() { + return email; + } + + public void setEmail(String email) { + this.email = email; + } + + public Group getGroup() { + return group; + } + + public void setGroup(Group group) { + this.group = group; + } + +} diff --git a/src/test/java/com/avaje/tests/query/other/TestQueryConversationRowCount.java b/src/test/java/com/avaje/tests/query/other/TestQueryConversationRowCount.java new file mode 100644 index 000000000..9e2d9520e --- /dev/null +++ b/src/test/java/com/avaje/tests/query/other/TestQueryConversationRowCount.java @@ -0,0 +1,125 @@ +package com.avaje.tests.query.other; + +import java.util.List; + +import org.avaje.ebeantest.LoggedSqlCollector; +import org.junit.Assert; +import org.junit.Test; + +import com.avaje.ebean.BaseTestCase; +import com.avaje.ebean.Ebean; +import com.avaje.ebean.Query; +import com.avaje.tests.model.converstation.Conversation; + +public class TestQueryConversationRowCount extends BaseTestCase { + + @Test + public void test() { + + //"find conversation where club = :clubId and ( ( isPublic = :false and participants.user.id = :userId ) or isPublic = :true ) order by createdAt desc "; + //"find conversation where groupId = :groupId and ( ( open = :false and participants.user.id = :userId ) or open = :true ) order by whenCreated desc "; + + Long groupId = 1L; + Long userId = 1L; + + Query query = Ebean.find(Conversation.class) + .where().eq("group.id", groupId) + .disjunction() + .conjunction() + .eq("open", false).eq("participants.user.id", userId) + .endJunction() + .eq("open", true) + .endJunction() + .orderBy("whenCreated desc"); + + query.findList(); + String generatedSql = query.getGeneratedSql(); + + // select distinct t0.id c0, t0.title c1, t0.open c2, t0.version c3, t0.when_created c4, t0.when_updated c5, t0.group_id c6, t0.when_created + // from c_conversation t0 + // left outer join c_participation u1 on u1.conversation_id = t0.id + // where t0.group_id = ? and ((t0.open = ? and u1.user_id = ? ) or t0.open = ? ) + // order by t0.when_created desc; + + Assert.assertTrue(generatedSql.contains("select distinct t0.id c0, t0.title c1, t0.open")); + Assert.assertTrue(generatedSql.contains("left outer join c_participation u1 on u1.conversation_id = t0.id")); + Assert.assertTrue(generatedSql.contains("where t0.group_id = ? and ((t0.open = ? and u1.user_id = ? ) or t0.open = ? )")); + + + LoggedSqlCollector.start(); + query.findRowCount(); + + // select count(*) from ( + // select distinct t0.id c0 + // from c_conversation t0 + // left outer join c_participation u1 on u1.conversation_id = t0.id + // where t0.group_id = ? and ((t0.open = ? and u1.user_id = ? ) or t0.open = ? ) + // ); --bind(1,true,1,true) + + List loggedSql = LoggedSqlCollector.stop(); + Assert.assertEquals(1, loggedSql.size()); + + String countSql = loggedSql.get(0); + + Assert.assertTrue(countSql.contains("select count(*) from ( select distinct t0.id c0 from c_conversation t0 left outer join c_participation u1 on u1.conversation_id = t0.id where t0.group_id = ? and ((t0.open = ? and u1.user_id = ? ) or t0.open = ? )")); + } + + @Test + public void testUsingQueryLanguage() { + + //"find conversation where club = :clubId and ( ( isPublic = :false and participants.user.id = :userId ) or isPublic = :true ) order by createdAt desc "; + String qry = "find conversation where group.id = :groupId and ( ( open = :false and participants.user.id = :userId ) or open = :true ) order by whenCreated desc "; + + Long groupId = 1L; + Long userId = 1L; + + Query query = Ebean.createQuery(Conversation.class, qry); + query.setParameter("groupId", groupId); + query.setParameter("userId", userId); + query.setParameter("false", false); + query.setParameter("true", true); + + query.findList(); + String generatedSql = query.getGeneratedSql(); + + // ACTUAL: + // select distinct t0.id c0, t0.title c1, t0.open c2, t0.version c3, t0.when_created c4, t0.when_updated c5, t0.group_id c6, t0.when_created + // from c_conversation t0 + // join c_participation t1 on t1.conversation_id = t0.id + // where t0.group_id = ? and ( ( t0.open = ? and t1.user_id = ? ) or t0.open = ? ) + // order by t0.when_created desc; --bind(1, false, 1, true, ) + + // SHOULD BE: + // select distinct t0.id c0, t0.title c1, t0.open c2, t0.version c3, t0.when_created c4, t0.when_updated c5, t0.group_id c6, t0.when_created + // from c_conversation t0 + // left outer join c_participation u1 on u1.conversation_id = t0.id + // where t0.group_id = ? and ((t0.open = ? and u1.user_id = ? ) or t0.open = ? ) + // order by t0.when_created desc; + + Assert.assertTrue(generatedSql.contains("select distinct t0.id c0, t0.title c1, t0.open")); + + + // THE NEXT ASSERT CURRENTLY FAILS: +// Assert.assertTrue(generatedSql.contains("left outer join c_participation u1 on u1.conversation_id = t0.id")); +// Assert.assertTrue(generatedSql.contains("where t0.group_id = ? and ((t0.open = ? and u1.user_id = ? ) or t0.open = ? )")); +// +// +// LoggedSqlCollector.start(); +// query.findRowCount(); +// +// // select count(*) from ( +// // select distinct t0.id c0 +// // from c_conversation t0 +// // left outer join c_participation u1 on u1.conversation_id = t0.id +// // where t0.group_id = ? and ((t0.open = ? and u1.user_id = ? ) or t0.open = ? ) +// // ); --bind(1,true,1,true) +// +// List loggedSql = LoggedSqlCollector.stop(); +// Assert.assertEquals(1, loggedSql.size()); +// +// String countSql = loggedSql.get(0); +// +// Assert.assertTrue(countSql.contains("select count(*) from ( select distinct t0.id c0 from c_conversation t0 left outer join c_participation u1 on u1.conversation_id = t0.id where t0.group_id = ? and ((t0.open = ? and u1.user_id = ? ) or t0.open = ? )")); + } + +}