Fix for #166 - Incorrect join with @OneToOne optional on one side and

NOT optional on other side
This commit is contained in:
rbygrave
2014-07-10 03:05:52 +12:00
parent f2d422ff5a
commit 4e2bc6cce6
9 changed files with 220 additions and 49 deletions
@@ -877,7 +877,7 @@ public class BeanDescriptorManager implements BeanDescriptorMap {
if (!tableJoin.hasJoinColumns()) {
// define Join as the inverse of the mappedBy property
DeployTableJoin otherTableJoin = mappedAssocOne.getTableJoin();
otherTableJoin.copyTo(tableJoin, true, tableJoin.getTable());
otherTableJoin.copyWithoutType(tableJoin, true, tableJoin.getTable());
}
}
@@ -17,10 +17,6 @@ import com.avaje.ebeaninternal.server.query.SqlJoinType;
*/
public final class TableJoin {
public static final String LEFT_OUTER = "left outer join";
public static final String JOIN = "join";
/**
* Flag set when the imported key maps to the primary key. This occurs for
* intersection tables (ManyToMany).
@@ -4,11 +4,9 @@ import java.util.ArrayList;
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;
import com.avaje.ebeaninternal.server.query.SqlJoinType;
/**
@@ -175,48 +173,40 @@ public class DeployTableJoin {
return type == SqlJoinType.OUTER;
}
private void setType(SqlJoinType type) {
public void setType(SqlJoinType type) {
this.type = type;
}
/**
* Set the type of join.
*/
public void setType(String joinType) {
joinType = joinType.toUpperCase();
if (joinType.equalsIgnoreCase(TableJoin.JOIN)) {
type = SqlJoinType.INNER;
} else if (joinType.indexOf("LEFT") > -1) {
type = SqlJoinType.OUTER;
} else if (joinType.indexOf("OUTER") > -1) {
type = SqlJoinType.OUTER;
} else if (joinType.indexOf("INNER") > -1) {
type = SqlJoinType.INNER;
} else {
throw new RuntimeException(Message.msg("join.type.unknown", joinType));
}
}
public DeployTableJoin createInverse(String tableName) {
DeployTableJoin inverse = new DeployTableJoin();
return copyTo(inverse, true, tableName);
return copyInternal(inverse, true, tableName, true);
}
public DeployTableJoin copyTo(DeployTableJoin destJoin, boolean reverse, String tableName) {
return copyInternal(destJoin, reverse, tableName, true);
}
public DeployTableJoin copyWithoutType(DeployTableJoin destJoin, boolean reverse, String tableName) {
return copyInternal(destJoin, reverse, tableName, false);
}
private DeployTableJoin copyInternal(DeployTableJoin destJoin, boolean reverse, String tableName, boolean withType) {
destJoin.setTable(tableName);
destJoin.setType(type);
if (withType) {
destJoin.setType(type);
}
destJoin.setColumns(columns(), reverse);
return destJoin;
}
public InheritInfo getInheritInfo() {
return inheritInfo;
}
public InheritInfo getInheritInfo() {
return inheritInfo;
}
public void setInheritInfo(InheritInfo inheritInfo) {
this.inheritInfo = inheritInfo;
}
public void setInheritInfo(InheritInfo inheritInfo) {
this.inheritInfo = inheritInfo;
}
}
@@ -18,12 +18,12 @@ import com.avaje.ebean.config.TableName;
import com.avaje.ebeaninternal.server.deploy.BeanDescriptorManager;
import com.avaje.ebeaninternal.server.deploy.BeanProperty;
import com.avaje.ebeaninternal.server.deploy.BeanTable;
import com.avaje.ebeaninternal.server.deploy.TableJoin;
import com.avaje.ebeaninternal.server.deploy.meta.DeployBeanProperty;
import com.avaje.ebeaninternal.server.deploy.meta.DeployBeanPropertyAssocMany;
import com.avaje.ebeaninternal.server.deploy.meta.DeployTableJoin;
import com.avaje.ebeaninternal.server.deploy.meta.DeployTableJoinColumn;
import com.avaje.ebeaninternal.server.lib.util.StringHelper;
import com.avaje.ebeaninternal.server.query.SqlJoinType;
/**
* Read the deployment annotation for Assoc Many beans.
@@ -160,7 +160,7 @@ public class AnnotationAssocManys extends AnnotationParser {
DeployTableJoin destJoin = prop.getTableJoin();
destJoin.addJoinColumn(false, joinTable.inverseJoinColumns(), prop.getBeanTable());
intJoin.setType(TableJoin.LEFT_OUTER);
intJoin.setType(SqlJoinType.OUTER);
// reverse join from dest back to intersection
DeployTableJoin inverseDest = destJoin.createInverse(intTableName);
@@ -216,7 +216,7 @@ public class AnnotationAssocManys extends AnnotationParser {
intTableName = getM2MJoinTableName(localTable, otherTable);
intJoin.setTable(intTableName);
intJoin.setType(TableJoin.LEFT_OUTER);
intJoin.setType(SqlJoinType.OUTER);
}
DeployTableJoin destJoin = prop.getTableJoin();
@@ -282,7 +282,7 @@ public class AnnotationAssocManys extends AnnotationParser {
manyProp.setManyToMany(true);
manyProp.setModifyListenMode(ModifyListenMode.ALL);
manyProp.setBeanTable(assoc);
manyProp.getTableJoin().setType(TableJoin.LEFT_OUTER);
manyProp.getTableJoin().setType(SqlJoinType.OUTER);
}
private void readToOne(OneToMany propAnn, DeployBeanPropertyAssocMany<?> manyProp) {
@@ -307,7 +307,7 @@ public class AnnotationAssocManys extends AnnotationParser {
}
manyProp.setBeanTable(assoc);
manyProp.getTableJoin().setType(TableJoin.LEFT_OUTER);
manyProp.getTableJoin().setType(SqlJoinType.OUTER);
}
@@ -19,10 +19,10 @@ import com.avaje.ebean.annotation.Where;
import com.avaje.ebean.config.NamingConvention;
import com.avaje.ebeaninternal.server.deploy.BeanDescriptorManager;
import com.avaje.ebeaninternal.server.deploy.BeanTable;
import com.avaje.ebeaninternal.server.deploy.TableJoin;
import com.avaje.ebeaninternal.server.deploy.meta.DeployBeanProperty;
import com.avaje.ebeaninternal.server.deploy.meta.DeployBeanPropertyAssocOne;
import com.avaje.ebeaninternal.server.lib.util.StringHelper;
import com.avaje.ebeaninternal.server.query.SqlJoinType;
/**
* Read the deployment annotations for Associated One beans.
@@ -99,7 +99,7 @@ public class AnnotationAssocOnes extends AnnotationParser {
if (notNull != null) {
prop.setNullable(false);
// overrides optional attribute of ManyToOne etc
prop.getTableJoin().setType(TableJoin.JOIN);
prop.getTableJoin().setType(SqlJoinType.INNER);
}
}
@@ -2,10 +2,10 @@ package com.avaje.ebeaninternal.server.deploy.parse;
import java.util.HashMap;
import com.avaje.ebeaninternal.server.deploy.TableJoin;
import com.avaje.ebeaninternal.server.deploy.meta.DeployBeanDescriptor;
import com.avaje.ebeaninternal.server.deploy.meta.DeployBeanPropertyAssocOne;
import com.avaje.ebeaninternal.server.deploy.meta.DeployTableJoin;
import com.avaje.ebeaninternal.server.query.SqlJoinType;
/**
* Wraps information about a bean during deployment parsing.
@@ -58,7 +58,7 @@ public class DeployBeanInfo<T> {
if (tableJoin == null) {
tableJoin = new DeployTableJoin();
tableJoin.setTable(tableName);
tableJoin.setType(TableJoin.JOIN);
tableJoin.setType(SqlJoinType.INNER);
descriptor.addTableJoin(tableJoin);
tableJoinMap.put(key, tableJoin);
@@ -71,13 +71,8 @@ public class DeployBeanInfo<T> {
*/
public void setBeanJoinType(DeployBeanPropertyAssocOne<?> beanProp, boolean outerJoin) {
String joinType = TableJoin.JOIN;
if (outerJoin){// && util.isUseOneToOneOptional()) {
joinType = TableJoin.LEFT_OUTER;
}
DeployTableJoin tableJoin = beanProp.getTableJoin();
tableJoin.setType(joinType);
tableJoin.setType(outerJoin ? SqlJoinType.OUTER : SqlJoinType.INNER);
}
}
@@ -0,0 +1,37 @@
package com.avaje.tests.model.onetoone;
import javax.persistence.Entity;
import javax.persistence.OneToOne;
import javax.persistence.Table;
import com.avaje.tests.model.BaseModel;
@Entity
@Table(name="oto_account")
public class Account extends BaseModel {
public static final Finder<Long,Account> find = new Finder<Long,Account>(Long.class, Account.class);
String name;
@OneToOne(mappedBy = "account",optional = true)
User user;
public String getName() {
return name;
}
public void setName(String name) {
this.name = name;
}
public User getUser() {
return user;
}
public void setUser(User user) {
this.user = user;
}
}
@@ -0,0 +1,117 @@
package com.avaje.tests.model.onetoone;
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.config.GlobalProperties;
public class TestOneToOneOptionalRelationship extends BaseTestCase {
@Test
public void test() {
GlobalProperties.put("ebean.search.packages", "com.avaje.tests.model.onetoone");
Account account = new Account();
account.setName("AC234");
account.save();
LoggedSqlCollector.start();
Account fetchedAccount = Account.find.byId(account.getId());
Assert.assertNotNull(fetchedAccount);
List<String> loggedSql = LoggedSqlCollector.stop();
Assert.assertEquals(1, loggedSql.size());
// select t0.id c0, t0.name c1, t0.version c2, t0.when_created c3, t0.when_updated c4, t1.id c5
// from oto_account t0
// join oto_user t1 on t1.account_id = t0.id
// where t0.id = ?
String sql = loggedSql.get(0);
Assert.assertTrue(sql.contains("select t0.id c0, t0.name c1"));
Assert.assertTrue(sql.contains(" from oto_account t0 left outer join oto_user t1 on t1.account_id = t0.id where t0.id = ?"));
}
@Test
public void testWithUser() {
GlobalProperties.put("ebean.search.packages", "com.avaje.tests.model.onetoone");
Account account = new Account();
account.setName("AC678");
account.save();
User user = new User();
user.setName("Geoff");
user.setAccount(account);
user.save();
LoggedSqlCollector.start();
Account fetchedAccount = Account.find.byId(account.getId());
Assert.assertNotNull(fetchedAccount);
Assert.assertNotNull(fetchedAccount.getUser());
Assert.assertEquals(user.getId(), fetchedAccount.getUser().getId());
Assert.assertEquals(user.getName(), fetchedAccount.getUser().getName());
List<String> loggedSql = LoggedSqlCollector.stop();
Assert.assertEquals(2, loggedSql.size());
// select t0.id c0, t0.name c1, t0.version c2, t0.when_created c3, t0.when_updated c4, t1.id c5
// from oto_account t0
// join oto_user t1 on t1.account_id = t0.id
// where t0.id = ?
String sql = loggedSql.get(0);
Assert.assertTrue(sql.contains("select t0.id c0, t0.name c1"));
Assert.assertTrue(sql.contains(" from oto_account t0 left outer join oto_user t1 on t1.account_id = t0.id where t0.id = ?"));
String lazyLoadSql = loggedSql.get(1);
Assert.assertTrue(lazyLoadSql.contains("select t0.id c0, t0.name c1, t0.version c2, t0.when_created c3, t0.when_updated c4, t0.account_id c5 from oto_user t0 where t0.id = ?"));
}
@Test
public void testWithUserFetch() {
GlobalProperties.put("ebean.search.packages", "com.avaje.tests.model.onetoone");
Account account = new Account();
account.setName("AC786");
account.save();
User user = new User();
user.setName("Jane");
user.setAccount(account);
user.save();
LoggedSqlCollector.start();
Account fetchedAccount = Account.find.fetch("user").setId(account.getId()).findUnique();
Assert.assertNotNull(fetchedAccount);
Assert.assertNotNull(fetchedAccount.getUser());
Assert.assertEquals(user.getId(), fetchedAccount.getUser().getId());
Assert.assertEquals(user.getName(), fetchedAccount.getUser().getName());
List<String> loggedSql = LoggedSqlCollector.stop();
Assert.assertEquals(1, loggedSql.size());
// select t0.id c0, t0.name c1, t0.version c2, t0.when_created c3, t0.when_updated c4, t1.id c5
// from oto_account t0
// join oto_user t1 on t1.account_id = t0.id
// where t0.id = ?
String sql = loggedSql.get(0);
Assert.assertTrue(sql.contains("select t0.id c0, t0.name c1"));
Assert.assertTrue(sql.contains(" from oto_account t0 left outer join oto_user t1 on t1.account_id = t0.id where t0.id = ?"));
}
}
@@ -0,0 +1,36 @@
package com.avaje.tests.model.onetoone;
import javax.persistence.Entity;
import javax.persistence.OneToOne;
import javax.persistence.Table;
import com.avaje.tests.model.BaseModel;
@Entity
@Table(name="oto_user")
public class User extends BaseModel {
public static final Finder<Long,User> find = new Finder<Long,User>(Long.class, User.class);
String name;
@OneToOne(optional = false)
Account account;
public String getName() {
return name;
}
public void setName(String name) {
this.name = name;
}
public Account getAccount() {
return account;
}
public void setAccount(Account account) {
this.account = account;
}
}