OPENJPA-806: Refactor different domain paths into a single list in QueryDefinitionImpl.

git-svn-id: https://svn.apache.org/repos/asf/openjpa/trunk@724564 13f79535-47bb-0310-9956-ffa450edef68

openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/criteria/TestCriteria.java

@@ -118,7 +118,9 @@ public class TestCriteria extends SingleEMFTestCase {
 		    .where(photo.key().like("egret"));
 		
 		
-		String jpql = "select i.name, VALUE(p) from Item i join i.photos p where KEY(p) like 'egret'";
+		String jpql = "select i.name, VALUE(p)"
+			        + " from Item i join i.photos p"
+			        + " where KEY(p) like 'egret'";
 		compare(jpql, qdef);
 	}
 	
@@ -444,6 +446,33 @@ public class TestCriteria extends SingleEMFTestCase {
 		compare(jpql, e);
 	}
 	
+	public void testCorrelatedSubquerySpecialCase1() {
+		DomainObject o = qb.createQueryDefinition(Order.class);
+		DomainObject a = qb.createSubqueryDefinition(o.get("customer").get("accounts"));
+		o.select(o)
+		 .where(o.literal(10000).lessThan(a.select(a.get("balance")).all()));
+		
+		String jpql = "select o from Order o"
+			        + " where 10000 < ALL "
+			        + " (select a.balance from o.customer c join o.customer.accounts a)";
+		
+		compare(jpql, o);
+	}
+	
+	public void testCorrelatedSubquerySpecialCase2() {
+		DomainObject o = qb.createQueryDefinition(Order.class);
+		DomainObject c = o.join("customer");
+		DomainObject a = qb.createSubqueryDefinition(c.get("accounts"));
+		o.select(o)
+		 .where(o.literal(10000).lessThan(a.select(a.get("balance")).all()));
+		
+		String jpql = "select o from Order o JOIN o.customer c"
+			        + " where 10000 < ALL "
+			        + " (select a.balance from c.accounts a)";
+		
+		compare(jpql, o);
+	}
+	
 	public void testRecursiveDefinitionIsNotAllowed() {
 		DomainObject q = qb.createQueryDefinition(Customer.class);
 		q.where(q.exists().and(q.get("name").equal("wrong")));

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/AbstractDomainObject.java

@@ -18,7 +18,6 @@
  */
 package org.apache.openjpa.persistence.query;
 
-import java.util.ArrayList;
 import java.util.Calendar;
 import java.util.Date;
 import java.util.List;
@@ -43,38 +42,12 @@ import javax.persistence.Subquery;
  */
 public abstract class AbstractDomainObject extends AbstractPath 
 	implements DomainObject {
-	private final QueryDefinitionImpl _owner;
-	private List<JoinPath> _joins;
-	private List<FetchPath> _fetchJoins;
 	
 	protected AbstractDomainObject(QueryDefinitionImpl owner, 
 		AbstractPath parent, PathOperator op, Object part2) {
-		super(parent, op, part2);
+		super(owner, parent, op, part2);
-		_owner = owner;
 	}
 
-	/**
-	 * Gets the QueryDefinition that created this path.
-	 * @return
-	 */
-	public QueryDefinitionImpl getOwner() {
-		return _owner;
-	}
-	
-	/**
-	 * Gets the fetch joins associated with this path. Can be null.
-	 */
-	public List<FetchPath> getFetchJoins() {
-		return _fetchJoins;
-	}
-	
-	/**
-	 * Gets the joins associated with this path. Can be null.
-	 */
-	public List<JoinPath> getJoins() {
-		return _joins;
-	}
-	
 	/**
 	 * Adding a root adds a root domain to the owning query. 
 	 */
@@ -100,49 +73,32 @@ public abstract class AbstractDomainObject extends AbstractPath
 	 * Derives a path from this path by joining the given field.
 	 * Also the joined path becomes a domain of the owning query.
 	 */
-	public DomainObject join(String attribute) {
+	public DomainObject join(String attr) {
-		return join(attribute, PathOperator.INNER);
+		return _owner.addDomain(new JoinPath(this, PathOperator.INNER, attr));
 	}
 	
 	/**
 	 * Derives a path from this path by outer joining the given field.
 	 * Also the joined path becomes a domain of the owning query.
 	 */
-	public DomainObject leftJoin(String attribute) {
+	public DomainObject leftJoin(String attr) {
-		return join(attribute, PathOperator.OUTER);
+		return _owner.addDomain(new JoinPath(this, PathOperator.OUTER, attr));
-	}
-	
-	protected DomainObject join(String attr, PathOperator joinType) {
-		JoinPath join = new JoinPath(this, joinType, attr);
-		if (_joins == null) {
-			_joins = new ArrayList<JoinPath>();
-		}
-		_joins.add(join);
-		return join;
 	}
 	
 	/**
 	 * Derives a path from this path by fetch joining the given field.
 	 */
-	public FetchJoinObject joinFetch(String attribute) {
+	public FetchJoinObject joinFetch(String attr) {
-		return fetchJoin(attribute, PathOperator.FETCH_INNER);
+		return _owner.addDomain(new FetchPath(this, PathOperator.FETCH_INNER, 
+			attr));
 	}
 
 	/**
 	 * Derives a path from this path by fetch joining the given field.
 	 */
-	public FetchJoinObject leftJoinFetch(String attribute) {
+	public FetchJoinObject leftJoinFetch(String attr) {
-		return fetchJoin(attribute, PathOperator.FETCH_OUTER);
+		return _owner.addDomain(new FetchPath(this, PathOperator.FETCH_OUTER, 
-	}
+			attr));
-
-	private FetchJoinObject fetchJoin(String attr, PathOperator joinType) {
-		NavigationPath path = new NavigationPath(_owner, this, attr);
-		FetchPath join = new FetchPath(path, joinType);
-		if (_fetchJoins == null) {
-			_fetchJoins = new ArrayList<FetchPath>();
-		}
-		_fetchJoins.add(join);
-		return join;
 	}
 
 	/**
@@ -376,19 +332,4 @@ public abstract class AbstractDomainObject extends AbstractPath
 	public QueryDefinition where(Predicate predicate) {
 		return _owner.where(predicate);
 	}
-	
-	// -----------------------------------------------------------------------
-	// contract for conversion to JPQL.
-	// -----------------------------------------------------------------------
-	/**
-	 * Sets alias for this domain and all its joins.
-	 */
-	public void setAlias(AliasContext ctx) {
-		ctx.getAlias(this);
-		if (_joins != null)
-			for (JoinPath join : _joins)
-				join.setAlias(ctx);
-	}
-	
-
 }

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/AbstractPath.java

@@ -18,6 +18,8 @@
  */
 package org.apache.openjpa.persistence.query;
 
+import java.util.LinkedList;
+
 import javax.persistence.Aggregate;
 import javax.persistence.Expression;
 import javax.persistence.PathExpression;
@@ -45,8 +47,10 @@ abstract class AbstractPath extends ExpressionImpl implements
 	protected final AbstractPath  _parent;
 	protected final Object 		  _part2;
 	protected final PathOperator  _operator;
+	protected final QueryDefinitionImpl _owner;
 	
-	protected AbstractPath(AbstractPath parent, PathOperator op, Object part2) {
+	protected AbstractPath(QueryDefinitionImpl owner, AbstractPath parent, PathOperator op, Object part2) {
+		_owner = owner;
 		_parent = parent;
 		_part2  = part2;
 		_operator = op;
@@ -55,6 +59,10 @@ abstract class AbstractPath extends ExpressionImpl implements
 	// ------------------------------------------------------------------------
 	// Path related functions.
 	// ------------------------------------------------------------------------
+	
+	final QueryDefinitionImpl getOwner() {
+		return _owner;
+	}
 	/**
 	 * Gets the parent from which this receiver has been derived. Can be null
 	 * for a root path.
@@ -112,4 +120,17 @@ abstract class AbstractPath extends ExpressionImpl implements
 	public Expression type() {
 		return new TypeExpression(this);
 	}
+	
+	LinkedList<AbstractPath> split() {
+		return _split(this, new LinkedList<AbstractPath>());
+	}
+	
+	private LinkedList<AbstractPath> _split(AbstractPath path, 
+		LinkedList<AbstractPath> list) {
+		if (path == null)
+			return list;
+		_split(path.getParent(), list);
+		list.add(path);
+		return list;
+	}
 }

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/AbstractVisitable.java

@@ -39,10 +39,6 @@ abstract class AbstractVisitable implements Visitable {
 		throw new UnsupportedOperationException(this.getClass().getName());
 	}
 
-	public void setAlias(AliasContext ctx) {
-		throw new UnsupportedOperationException(this.getClass().getName());
-	}
-	
 	public String asJoinable(AliasContext ctx) {
 		throw new UnsupportedOperationException(this.getClass().getName());
 	}

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/AliasContext.java

@@ -49,7 +49,13 @@ class AliasContext {
 		String alias = _aliases.get(path);
 		if (alias != null)
 			return alias;
-		alias = path.getAliasHint(this).substring(0,1).toLowerCase();
+		return setAlias(path);
+	}
+	
+	public String setAlias(ExpressionImpl path) {
+		if (_aliases.containsKey(path))
+			return _aliases.get(path);
+		String alias = path.getAliasHint(this).substring(0,1).toLowerCase();
 		int i = 2;
 		while (_aliases.containsValue(alias)) {
 			alias = alias.substring(0,1) + i;

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/ExpressionImpl.java

@@ -462,14 +462,7 @@ abstract class ExpressionImpl extends AbstractVisitable
 	//
 	// Visitable/Selectable implementation
 	//
-	public void setAlias(AliasContext ctx) {
-		ctx.getAlias(this);
-	}
-	
 	public String getAliasHint(AliasContext ctx) {
 		return "o";
 	}
-	
-	public abstract String asExpression(AliasContext ctx);
-	public abstract String asProjection(AliasContext ctx);
 }

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/FetchPath.java

@@ -18,6 +18,8 @@
  */
 package org.apache.openjpa.persistence.query;
 
+import static org.apache.openjpa.persistence.query.PathOperator.NAVIGATION;
+
 import javax.persistence.FetchJoinObject;
 
 /**
@@ -26,18 +28,21 @@ import javax.persistence.FetchJoinObject;
  * @author Pinaki Poddar
  *
  */
-public class FetchPath extends AbstractVisitable 
+public class FetchPath extends AbstractDomainObject 
     implements FetchJoinObject, Visitable {
-	private NavigationPath _path;
+	FetchPath(AbstractDomainObject parent, PathOperator joinType, String attr) {
-	private PathOperator _joinType;
+		super(parent.getOwner(), parent, joinType, attr);
+	}
 	
+	@Override
+	public String asJoinable(AliasContext ctx) {
+		StringBuffer tmp = new StringBuffer(getOperator().toString());
+		tmp.append(getParent().asProjection(ctx))
+		   .append(NAVIGATION)
+		   .append(getLastSegment())
+		   .append(" ");
+		
+		return tmp.toString();
+	}
 
-	FetchPath(NavigationPath path, PathOperator joinType) {
-		_path = path;
-		_joinType = joinType;
-	}
-	
-	public String asExpression(AliasContext ctx) {
-		return _joinType + " " + _path.asExpression(ctx);
-	}
 }

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/JoinPath.java

@@ -49,17 +49,12 @@ public class JoinPath extends AbstractDomainObject implements DomainObject {
 		
 	@Override
 	public String asJoinable(AliasContext ctx) {
-		StringBuffer tmp = new StringBuffer(getOperator().toString());
+		return new StringBuffer(getOperator().toString())
-		tmp.append(getParent().asProjection(ctx))
+		   .append(getParent().asProjection(ctx))
 		   .append(NAVIGATION)
 		   .append(getLastSegment())
 		   .append(" ")
-		   .append(ctx.getAlias(this));
+		   .append(ctx.getAlias(this)).toString();
-		
-		if (getJoins() != null)
-			for (JoinPath join : getJoins())
-				tmp.append(join.asJoinable(ctx));
-		return tmp.toString();
 	}
 	
 	@Override
@@ -75,4 +70,9 @@ public class JoinPath extends AbstractDomainObject implements DomainObject {
 			return ctx.getAlias(this);
 		return getParent().asProjection(ctx) + NAVIGATION + getLastSegment();
 	}
+	
+	public String toString() {
+		return getOperator() + getParent().toString() + "*" + getLastSegment();
+	}
+
 }

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/NavigationPath.java

@@ -37,6 +37,11 @@ class NavigationPath extends AbstractDomainObject implements PathExpression {
 		return (String)super.getLastSegment();
 	}
 	
+	@Override
+	public String getAliasHint(AliasContext ctx) {
+		return getLastSegment();
+	}
+
 	@Override
 	public String asProjection(AliasContext ctx) {
 		AbstractPath parent = getParent();
@@ -59,4 +64,9 @@ class NavigationPath extends AbstractDomainObject implements PathExpression {
 	public String asJoinable(AliasContext ctx) {
 		return asProjection(ctx) + " " + ctx.getAlias(this);
 	}
+	
+	public String toString() {
+		return getParent().toString()+"."+getLastSegment();
+	}
+
 }

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/OperatorPath.java

@@ -55,4 +55,9 @@ public class OperatorPath extends AbstractDomainObject {
 	public String asJoinable(AliasContext ctx) {
 		throw new UnsupportedOperationException();
 	}
+	
+	public String toString() {
+		return getOperator() + "(" + getParent().toString() + ")";
+	}
+
 }

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/QueryBuilderImpl.java

@@ -18,17 +18,12 @@
  */
 package org.apache.openjpa.persistence.query;
 
-import java.io.Serializable;
-
 import javax.persistence.DomainObject;
 import javax.persistence.PathExpression;
 import javax.persistence.QueryBuilder;
 import javax.persistence.QueryDefinition;
 
-import org.apache.openjpa.meta.ClassMetaData;
 import org.apache.openjpa.meta.MetaDataRepository;
-import org.apache.openjpa.persistence.EntityManagerFactoryImpl;
-import org.apache.openjpa.persistence.OpenJPAEntityManagerFactory;
 import org.apache.openjpa.persistence.OpenJPAEntityManagerFactorySPI;
 
 /**

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/QueryDefinitionImpl.java

@@ -23,7 +23,10 @@ import java.sql.Timestamp;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Calendar;
+import java.util.Collections;
+import java.util.Comparator;
 import java.util.Date;
+import java.util.LinkedList;
 import java.util.List;
 
 import javax.persistence.CaseExpression;
@@ -79,16 +82,34 @@ public class QueryDefinitionImpl extends ExpressionImpl
 	
 	public DomainObject addSubqueryRoot(PathExpression path) {
 		AbstractPath impl = (AbstractPath)path;
-		AbstractDomainObject newRoot = new NavigationPath(this,  
+		LinkedList<AbstractPath> paths = impl.split();
-				impl.getParent(), impl.getLastSegment().toString());
+		QueryDefinitionImpl owner = impl.getOwner();
-		addDomain(newRoot);
+		int i = 0;
+		while (i < paths.size() && owner.hasDomain(paths.get(i))) {
+			i++;
+		}
+		
+		AbstractPath next = paths.get(i);
+		DomainObject newRoot = new NavigationPath(this, 
+				next.getParent(), next.getLastSegment().toString());
+		addDomain((AbstractDomainObject)newRoot);
+		i++;
+		for (; i < paths.size(); i++) {
+			next = paths.get(i);
+			newRoot = newRoot.join(next.getLastSegment().toString());
+		}
 		return newRoot;
 	}
 	
-	protected void addDomain(AbstractDomainObject path) {
+	boolean hasDomain(PathExpression path) {
+		return _domains != null && _domains.contains(path);
+	}
+	
+	protected <T extends AbstractDomainObject> T addDomain(T path) {
 		if (_domains == null)
 			_domains = new ArrayList<AbstractDomainObject>();
 		_domains.add(path);
+		return path;
 	}
 
 	public Subquery all() {
@@ -138,6 +159,8 @@ public class QueryDefinitionImpl extends ExpressionImpl
 	public QueryDefinition groupBy(PathExpression... pathExprs) {
 		if (_groupBys == null) {
 			_groupBys = new ArrayList<PathExpression>();
+		} else {
+			_groupBys.clear();
 		}
 		for (PathExpression e : pathExprs)
 			_groupBys.add(e);
@@ -147,6 +170,8 @@ public class QueryDefinitionImpl extends ExpressionImpl
 	public QueryDefinition groupBy(List<PathExpression> pathExprList) {
 		if (_groupBys == null) {
 			_groupBys = new ArrayList<PathExpression>();
+		} else {
+			_groupBys.clear();
 		}
 		for (PathExpression e : pathExprList)
 			_groupBys.add(e);
@@ -336,38 +361,17 @@ public class QueryDefinitionImpl extends ExpressionImpl
 		return _projections;
 	}
 
-	/**
-	 * 
-	 */
 	@Override
 	public String asExpression(AliasContext ctx) {
 		ctx.push(this);
 		StringBuffer buffer = new StringBuffer();
 		registerDomains(ctx);
-
+		String select = _distinct ? "SELECT DISTINCT " : "SELECT ";
-		fillBuffer(_distinct ? "SELECT DISTINCT " : "SELECT ", buffer, ctx, 
+		fillBuffer(select, buffer, ctx, getProjections(), Visit.PROJECTION);
-			getProjections(), Visit.PROJECTION);
+		fillBuffer(" FROM ", buffer, ctx, _domains, Visit.JOINABLE);
-		buffer.append(" FROM ");
+		fillBuffer(" WHERE ", buffer, ctx, _where);
-		for (int i=0; _domains != null && i < _domains.size(); i++) {
-			buffer.append(_domains.get(i).asJoinable(ctx));
-			fillBuffer(" ", buffer, ctx, _domains.get(i).getJoins(), 
-					Visit.JOINABLE);
-			fillBuffer(" ", buffer, ctx, _domains.get(i).getFetchJoins(), 
-					Visit.EXPRESSION);
-			if (i != _domains.size()-1)
-				buffer.append(",");
-		}
-		if (_where != null) {
-			buffer.append(" WHERE ")
-			      .append(((Visitable)_where).asExpression(ctx));
-		}
-		
 		fillBuffer(" GROUP BY ", buffer, ctx, _groupBys, Visit.EXPRESSION);
-		
+		fillBuffer(" HAVING ", buffer, ctx, _having);
-		if (_having != null) {
-			buffer.append(" HAVING ")
-			      .append(((Visitable)_having).asExpression(ctx));
-		}
 		fillBuffer(" ORDER BY ", buffer, ctx, _orderBys, Visit.EXPRESSION);
 		
 		return buffer.toString();
@@ -391,28 +395,50 @@ public class QueryDefinitionImpl extends ExpressionImpl
 			case EXPRESSION : buffer.append(v.asExpression(ctx))
 				                    .append(i != list.size()-1 ? ", " : " ");
 				break;
-			case JOINABLE   : buffer.append(v.asJoinable(ctx)); 
+			case JOINABLE   : buffer.append(v.asJoinable(ctx))
+								    .append(i > 0 && v instanceof RootPath ? 
+									"," : " ");
 				break;
 			}
 		}
 	}
 	
+	public void fillBuffer(String header, StringBuffer buffer, AliasContext ctx,
+			Predicate p) {
+		if (p == null)
+			return;
+		Visitable v = (Visitable)p;
+		buffer.append(header);
+		buffer.append(v.asExpression(ctx));
+	}
+	
 	/**
-	 * Registers each domain with an alias.
+	 * Registers each domain with an alias. Also set alias for order by items
-	 * @param ctx
+	 * that are projected.
 	 */
 	private void registerDomains(AliasContext ctx) {
 		if (_domains != null) {
+			Collections.sort(_domains, new DomainSorter());
 			for (AbstractDomainObject domain : _domains) {
-				domain.setAlias(ctx);
+				ctx.setAlias(domain);
 			}
 		}
 		if (_orderBys != null) {
 			for (OrderableItem o : _orderBys) {
 				ExpressionImpl e = o.getExpression();
 				if (_projections != null && _projections.contains(e))
-					e.setAlias(ctx);
+					ctx.setAlias(e);
 			}
 		}
 	}
+	
+	static class DomainSorter implements Comparator<AbstractDomainObject> {
+		static List<Class> _order = Arrays.asList(new Class[] {
+				RootPath.class, NavigationPath.class, OperatorPath.class, 
+				JoinPath.class, FetchPath.class, } );
+		
+		public int compare(AbstractDomainObject a, AbstractDomainObject b) {
+			return _order.indexOf(a.getClass()) - _order.indexOf(b.getClass());
+		}
+	}
 }

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/RootPath.java

@@ -38,4 +38,8 @@ public class RootPath extends AbstractDomainObject implements DomainObject {
 	public String asProjection(AliasContext ctx) {
 		return ctx.getAlias(this);
 	}
+	
+	public String toString() {
+		return getLastSegment().getSimpleName();
+	}
 }

openjpa-persistence/src/main/java/org/apache/openjpa/persistence/query/Visitable.java

@@ -29,24 +29,23 @@ import java.io.Serializable;
  */
 public interface Visitable extends Serializable {
 	/**
-	 * Get a JPQL fragment as used in a WHERE clause.
+	 * Get a JPQL fragment as used in WHERE clause.
 	 */
 	String asExpression(AliasContext ctx);
 	
 	/**
-	 * Gets the string representation in a SELECT projection.
+	 * Gets the string representation in SELECT projection.
 	 */
 	String asProjection(AliasContext ctx);
 	
 	/**
-	 * Sets alias.
+	 * Gets the string representation in FROM clause.
 	 */
-	void setAlias(AliasContext ctx);
-	
-	String getAliasHint(AliasContext ctx);
-	
 	String asJoinable(AliasContext ctx);
-
+	
-
+	/**
+	 * Gets the hint to be used while creating alias.
+	 */
+	String getAliasHint(AliasContext ctx);
 
 }