Skip to content

Commit

Permalink
Fix for #166 - Incorrect join with @OnetoOne optional on one side and
Browse files Browse the repository at this point in the history
NOT optional on other side
  • Loading branch information
rbygrave committed Jul 9, 2014
1 parent f2d422f commit 4e2bc6c
Show file tree
Hide file tree
Showing 9 changed files with 220 additions and 49 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -877,7 +877,7 @@ private void checkMappedByOneToOne(DeployBeanInfo<?> info, DeployBeanPropertyAss
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());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,6 @@
*/
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).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,9 @@

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;

/**
Expand Down Expand Up @@ -175,48 +173,40 @@ public boolean isOuterJoin() {
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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@
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.
Expand Down Expand Up @@ -160,7 +160,7 @@ private void readJoinTable(JoinTable joinTable, DeployBeanPropertyAssocMany<?> p
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);
Expand Down Expand Up @@ -216,7 +216,7 @@ private void manyToManyDefaultJoins(DeployBeanPropertyAssocMany<?> prop) {
intTableName = getM2MJoinTableName(localTable, otherTable);

intJoin.setTable(intTableName);
intJoin.setType(TableJoin.LEFT_OUTER);
intJoin.setType(SqlJoinType.OUTER);
}

DeployTableJoin destJoin = prop.getTableJoin();
Expand Down Expand Up @@ -282,7 +282,7 @@ private void readToMany(ManyToMany propAnn, DeployBeanPropertyAssocMany<?> manyP
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) {
Expand All @@ -307,7 +307,7 @@ private void readToOne(OneToMany propAnn, DeployBeanPropertyAssocMany<?> manyPro
}

manyProp.setBeanTable(assoc);
manyProp.getTableJoin().setType(TableJoin.LEFT_OUTER);
manyProp.getTableJoin().setType(SqlJoinType.OUTER);
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@
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.
Expand Down Expand Up @@ -99,7 +99,7 @@ private void readAssocOne(DeployBeanPropertyAssocOne<?> prop) {
if (notNull != null) {
prop.setNullable(false);
// overrides optional attribute of ManyToOne etc
prop.getTableJoin().setType(TableJoin.JOIN);
prop.getTableJoin().setType(SqlJoinType.INNER);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@

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.
Expand Down Expand Up @@ -58,7 +58,7 @@ public DeployTableJoin getTableJoin(String tableName) {
if (tableJoin == null) {
tableJoin = new DeployTableJoin();
tableJoin.setTable(tableName);
tableJoin.setType(TableJoin.JOIN);
tableJoin.setType(SqlJoinType.INNER);
descriptor.addTableJoin(tableJoin);

tableJoinMap.put(key, tableJoin);
Expand All @@ -71,13 +71,8 @@ public DeployTableJoin getTableJoin(String tableName) {
*/
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);
}

}
37 changes: 37 additions & 0 deletions src/test/java/com/avaje/tests/model/onetoone/Account.java
Original file line number Diff line number Diff line change
@@ -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;
}

}
Original file line number Diff line number Diff line change
@@ -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 = ?"));
}
}
36 changes: 36 additions & 0 deletions src/test/java/com/avaje/tests/model/onetoone/User.java
Original file line number Diff line number Diff line change
@@ -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;
}

}

0 comments on commit 4e2bc6c

Please sign in to comment.