Revert "Lightweight Query Factory"

Author: afawcett

fflib/src/classes/fflib_QueryFactory.cls

@@ -61,7 +61,7 @@ public class fflib_QueryFactory { //No explicit sharing declaration - inherit fr
 	**/
 	public Schema.SObjectType table {get; private set;}
 	@testVisible
-	private List<QueryField> fields; 
+	private Set<QueryField> fields; 
 	private String conditionExpression;
 	private Integer limitCount;
 	private Integer offset;
@@ -71,10 +71,9 @@ public class fflib_QueryFactory { //No explicit sharing declaration - inherit fr
 	/* This can optionally be enforced (or not) by calling the setEnforceFLS method prior to calling 
 	/* one of the selectField or selectFieldset methods.
 	**/
-	@TestVisible
-	private Boolean enforceFLS = false;
-	@TestVisible
-	private Boolean lightweight = false;
+	private Boolean enforceFLS;
+	
+	private Boolean sortSelectFields = true;
 
 	/**
 	 * The relationship and  subselectQueryMap variables are used to support subselect queries.  Subselects can be added to 
@@ -86,12 +85,6 @@ public class fflib_QueryFactory { //No explicit sharing declaration - inherit fr
 	private Map<Schema.ChildRelationship, fflib_QueryFactory> subselectQueryMap;
 
 	private QueryField getFieldToken(String fieldName){
-		
-		// FLS will not be enforced, so we are going to take a lot of shortcuts in the name of performance
-		if (this.lightweight) {
-			return new LightweightQueryField(fieldName);
-		}
-		
 		QueryField result;
 		if(!fieldName.contains('.')){ //single field
 			Schema.SObjectField token = fflib_SObjectDescribe.getDescribe(table).getField(fieldName.toLowerCase());
@@ -137,31 +130,19 @@ public class fflib_QueryFactory { //No explicit sharing declaration - inherit fr
 			return false;
 		return ((fflib_QueryFactory)obj).toSOQL() == this.toSOQL();
 	}
-	
+
 	/**
 	 * Construct a new fflib_QueryFactory instance with no options other than the FROM caluse. 
 	 * You *must* call selectField(s) before {@link #toSOQL} will return a valid, runnable query.
 	 * @param table	the SObject to be used in the FROM clause of the resultant query. This sets the value of {@link #table}.
 	**/
 	public fflib_QueryFactory(Schema.SObjectType table){
-		this(table, false);
-	}
-
-	/**
-	 * Construct a new fflib_QueryFactory instance, allowing you to use LightweightQueryFields
-	 * to build the query. This offers significant performance improvement in query build time
-	 * at the expense of FLS enforcement, and up-front field validation.
-	 * @param table the SObject to be used in the FROM clause of the resultant query. This sets the value of {@link #table}.
-	 * @param lightweight a Boolean that specifies whether the LightweightQueryField is to be used when building the query.
-	**/
-	public fflib_QueryFactory(Schema.SObjectType table, Boolean lightweight) {
 		this.table = table;
-		this.fields = new List<QueryField>();
-		this.order = new List<Ordering>();
-		this.lightweight = lightweight;
-		this.enforceFLS = false;
+		fields = new Set<QueryField>();
+		order = new List<Ordering>();
+		enforceFLS = false;
 	}
-	
+
 	/**
 	 * Construct a new fflib_QueryFactory instance with no options other than the FROM clause and the relationship.
 	 * This should be used when constructing a subquery query for addition to a parent query. 
@@ -170,22 +151,10 @@ public class fflib_QueryFactory { //No explicit sharing declaration - inherit fr
 	 * @param relationship the ChildRelationship to be used in the FROM Clause of the resultant Query (when set overrides value of table). This sets the value of {@link #relationship} and {@link #table}.
 	**/
 	private fflib_QueryFactory(Schema.ChildRelationship relationship){
-		this(relationship, false);
-	}
-	
-	/**
-	 * Construct a new fflib_QueryFactory instance with no options other than the FROM clause and the relationship.
-	 * This should be used when constructing a subquery query for addition to a parent query. 
-	 * Objects created with this constructor cannot be added to another object using the subselectQuery method.
-	 * You *must* call selectField(s) before {@link #toSOQL} will return a valid, runnable query.
-	 * @param relationship the ChildRelationship to be used in the FROM Clause of the resultant Query (when set overrides value of table). This sets the value of {@link #relationship} and {@link #table}.
-	 * @param lightweight a Boolean that specifies whether the LightweightQueryField is to be used when building the query.
-	**/
-	private fflib_QueryFactory(Schema.ChildRelationship relationship, Boolean lightweight){
-		this(relationship.getChildSObject(), lightweight);
+		this(relationship.getChildSObject()); 
 		this.relationship = relationship;
 	}
-	
+
 	/**
 	 * This method checks to see if the User has Read Access on {@link #table}. 
 	 * Asserts true if User has access.
@@ -202,9 +171,6 @@ public class fflib_QueryFactory { //No explicit sharing declaration - inherit fr
 	 * @param enforce whether to enforce field level security (read)
 	 **/
 	public fflib_QueryFactory setEnforceFLS(Boolean enforce){
-		if (this.lightweight && enforce) {
-			throw new InvalidOperationException('Calling setEnforceFLS(true) on a "lightweight" QueryFactory instance is not allowed.');
-		}
 		this.enforceFLS = enforce;
 		return this;
 	}
@@ -213,10 +179,10 @@ public class fflib_QueryFactory { //No explicit sharing declaration - inherit fr
 	 * Sets a flag to indicate that this query should have ordered
 	 * query fields in the select statement (this at a small cost to performance).
 	 * If you are processing large query sets, you should switch this off.
-	 * @deprecated Fields are ALWAYS sorted within the generated SOQL, so this method now does nothing.
 	 * @param whether or not select fields should be sorted in the soql statement.
 	 **/
 	public fflib_QueryFactory setSortSelectFields(Boolean doSort){
+		this.sortSelectFields = doSort;
 		return this;
 	}
 
@@ -226,7 +192,7 @@ public class fflib_QueryFactory { //No explicit sharing declaration - inherit fr
 	 * @param fieldName the API name of the field to add to the query's SELECT clause.
 	 **/
 	public fflib_QueryFactory selectField(String fieldName){ 		
-		this.fields.add( getFieldToken(fieldName) );
+		fields.add( getFieldToken(fieldName) );
 		return this;
 	} 
 	/**
@@ -240,41 +206,31 @@ public class fflib_QueryFactory { //No explicit sharing declaration - inherit fr
 			throw new InvalidFieldException(null,this.table);
 		if (enforceFLS) 
 			fflib_SecurityUtils.checkFieldIsReadable(table, field);
-		this.fields.add(getQueryFieldFromToken(field));
+		fields.add( new QueryField(field) );
 		return this;
 	}
-	
-	/**
-	 * Returns the appropriate QueryField implementation, based on the "lightweight" flag
-	 * @param field the {@link Schema.SObjectField} for the QueryField
-	 * @returns either a QueryField, or LightweightQueryField object for the specified SObjectField
-	**/
-	private QueryField getQueryFieldFromToken(Schema.SObjectField field) {
-		QueryField qf;
-		if (this.lightweight)
-			qf = new LightweightQueryField(field);
-		else
-			qf = new QueryField(field);
-		return qf;
-	}
-	
 	/**
 	 * Selects multiple fields. This acts the same as calling {@link #selectField(String)} multiple times.
 	 * @param fieldNames the Set of field API names to select.
 	**/
 	public fflib_QueryFactory selectFields(Set<String> fieldNames){
+		List<String> fieldList = new List<String>();
+		Set<QueryField> toAdd = new Set<QueryField>();
 		for(String fieldName:fieldNames){
-			this.fields.add( getFieldToken(fieldName) );
-		}
+			toAdd.add( getFieldToken(fieldName) );
+		}	
+		fields.addAll(toAdd);
 		return this;
 	}
 	/**
 	 * Selects multiple fields. This acts the same as calling {@link #selectField(String)} multiple times.
 	 * @param fieldNames the List of field API names to select.
 	**/
 	public fflib_QueryFactory selectFields(List<String> fieldNames){
+		Set<QueryField> toAdd = new Set<QueryField>();
 		for(String fieldName:fieldNames)
-			this.fields.add( getFieldToken(fieldName) );
+			toAdd.add( getFieldToken(fieldName) );
+		fields.addAll(toAdd);
 		return this;
 	}
 	/**
@@ -287,8 +243,8 @@ public class fflib_QueryFactory { //No explicit sharing declaration - inherit fr
 			if(token == null)
 				throw new InvalidFieldException();	
 			if (enforceFLS) 
-				fflib_SecurityUtils.checkFieldIsReadable(table, token);
-			this.fields.add(getQueryFieldFromToken(token));
+				fflib_SecurityUtils.checkFieldIsReadable(table, token);	
+			this.fields.add( new QueryField(token) );
 		}
 		return this;
 	}
@@ -302,8 +258,8 @@ public class fflib_QueryFactory { //No explicit sharing declaration - inherit fr
 			if(token == null)
 				throw new InvalidFieldException();
 			if (enforceFLS) 
-				fflib_SecurityUtils.checkFieldIsReadable(table, token);
-			this.fields.add(getQueryFieldFromToken(token));
+				fflib_SecurityUtils.checkFieldIsReadable(table, token);		
+			this.fields.add( new QueryField(token) );
 		}
 		return this;
 	}
@@ -370,13 +326,12 @@ public class fflib_QueryFactory { //No explicit sharing declaration - inherit fr
 	}
 
 	/**
-	 * @deprecated  Replaced by {@link #getSelectedFieldsAsList()}
 	 * @returns the selected fields
 	 **/
 	public Set<QueryField> getSelectedFields() { 
-		return new Set<QueryField>(this.fields);
+		return this.fields;
 	}
-	
+
 	/**
 	 * Add a subquery query to this query.  If a subquery for this relationship already exists, it will be returned.
 	 * If not, a new one will be created and returned.
@@ -465,8 +420,11 @@ public class fflib_QueryFactory { //No explicit sharing declaration - inherit fr
 			return subselectQueryMap.get(relationship);
 		}
 
-		fflib_QueryFactory subselectQuery = new fflib_QueryFactory(relationship, this.lightweight);
+		fflib_QueryFactory subselectQuery = new fflib_QueryFactory(relationship);
 
+		//The child queryFactory should be configured in the same way as the parent by default - can override after if required
+		subSelectQuery.setSortSelectFields(sortSelectFields);
+
 		if(assertIsAccessible){
 			subSelectQuery.assertIsAccessible();
 		}
@@ -543,7 +501,7 @@ public class fflib_QueryFactory { //No explicit sharing declaration - inherit fr
 	**/
     public fflib_QueryFactory addOrdering(SObjectField field, SortOrder direction, Boolean nullsLast){
 		order.add(
-			new Ordering(getQueryFieldFromToken(field), direction, nullsLast)
+			new Ordering(new QueryField(field), direction, nullsLast)
 		);	
 		return this;
     }
@@ -581,7 +539,7 @@ public class fflib_QueryFactory { //No explicit sharing declaration - inherit fr
 	**/
     public fflib_QueryFactory addOrdering(SObjectField field, SortOrder direction){
 		order.add(
-			new Ordering(getQueryFieldFromToken(field), direction)
+			new Ordering(new QueryField(field), direction)
 		);	
 		return this;
     }
@@ -596,20 +554,15 @@ public class fflib_QueryFactory { //No explicit sharing declaration - inherit fr
 		if (fields.size() == 0){
 			if (enforceFLS) fflib_SecurityUtils.checkFieldIsReadable(table, 'Id');
 			result += 'Id  ';
-		} else {
-			// This bit of code de-dupes the list of QueryFields. Since we've moved away from using a Set to back this collection 
-			// (for performance reasons related to https://github.com/financialforcedev/fflib-apex-common/issues/79), we de-dupe
-			// by first sorting the List of QueryField objects (in order of the String representation of the field path),
-			// then making a pass through the list leaving dupes out of the "fieldsToQuery" collection.
-			fields.sort(); // Sorts based on QueryFields's "comparable" implementation
-			// Now that the QueryField list is sorted, we can de-dupe
-			QueryField previousQf = null;
-			for(QueryField field : fields){
-				if (!field.equals(previousQf)) {
-					result += field + ', ';
-				}
-				previousQf = field;
+		}else if(sortSelectFields){
+			List<QueryField> fieldsToQuery = new List<QueryField>(fields);
+			fieldsToQuery.sort(); //delegates to QueryFilter's comparable implementation
+			for(QueryField field:fieldsToQuery){
+				result += field + ', ';
 			}
+		}else{
+			for (QueryField field : fields)
+				result += field + ', ';
 		}
 
 		if(subselectQueryMap != null && !subselectQueryMap.isEmpty()){
@@ -639,7 +592,7 @@ public class fflib_QueryFactory { //No explicit sharing declaration - inherit fr
 	**/
 	public fflib_QueryFactory deepClone(){	
 
-		fflib_QueryFactory clone = new fflib_QueryFactory(this.table, this.lightweight)
+		fflib_QueryFactory clone = new fflib_QueryFactory(this.table)
 			.setLimit(this.limitCount)
 			.setCondition(this.conditionExpression)
 			.setEnforceFLS(this.enforceFLS);
@@ -711,54 +664,8 @@ public class fflib_QueryFactory { //No explicit sharing declaration - inherit fr
 		}
 	}
 
-	public class LightweightQueryField extends QueryField implements Comparable {
-		String fieldName;
-		
-		private LightweightQueryField() {}
-		
-		@TestVisible
-		private LightweightQueryField(String fieldName) {
-			// Convert strings to lowercase so they sort in a case insensitive manner
-			this.fieldName = fieldName.toLowercase();
-		}
-		
-		@TestVisible
-		private LightweightQueryField(Schema.SObjectField field) {
-			// Convert strings to lowercase so they sort in a case insensitive manner
-			this.fieldName = field.getDescribe().getLocalName().toLowercase();
-		}
-		
-		public override String toString() { return this.fieldName; }
-		
-		public override Integer hashCode() {
-			return (this.fieldName == null) ? 0 : this.fieldName.hashCode();
-		}
-		
-		public override Boolean equals(Object obj) {
-			return ((obj != null) 
-				&& (obj instanceof LightweightQueryField) 
-				&& (this.fieldName == ((LightweightQueryField) obj).fieldName));
-		}
-		
-		public override Integer compareTo(Object obj) {
-			if (obj == null || !(obj instanceof LightweightQueryField))
-				return 1;
-			
-			if (this.fieldName == null) {
-				if (((LightweightQueryField) obj).fieldName == null)
-					// Both objects are non-null, but their fieldName is null
-					return 0;
-				else
-					// Our fieldName is null, but theirs isn't
-					return -1;
-			}
-			
-			// Both objects have non-null fieldNames, so just return the result of String.compareTo
-			return this.fieldName.compareTo(((LightweightQueryField) obj).fieldName);
-		}
-	}
 
-	public virtual class QueryField implements Comparable{
+	public class QueryField implements Comparable{
 		List<Schema.SObjectField> fields;
 
 		/**
@@ -774,9 +681,7 @@ public class fflib_QueryFactory { //No explicit sharing declaration - inherit fr
 		public List<SObjectField> getFieldPath(){
 			return fields.clone();
 		}
-		
-		private QueryField() {}
-		
+
 		@testVisible 
 		private QueryField(List<Schema.SObjectField> fields){
 			if(fields == null || fields.size() == 0)
@@ -789,7 +694,7 @@ public class fflib_QueryFactory { //No explicit sharing declaration - inherit fr
 				throw new InvalidFieldException('Invalid field: null');
 			fields = new List<Schema.SObjectField>{ field };
 		}
-		public virtual override String toString(){
+		public override String toString(){
 			String result = '';
 			Integer size = fields.size();
 			for (Integer i=0; i<size; i++)
@@ -807,10 +712,10 @@ public class fflib_QueryFactory { //No explicit sharing declaration - inherit fr
 			}
 			return result;
 		} 
-		public virtual integer hashCode(){
+		public integer hashCode(){
 			return String.valueOf(this.fields).hashCode();
 		}
-		public virtual boolean equals(Object obj){
+		public boolean equals(Object obj){
 			//Easy checks first
 			if(obj == null || !(obj instanceof QueryField))
 				return false;
@@ -839,7 +744,7 @@ public class fflib_QueryFactory { //No explicit sharing declaration - inherit fr
 		 * - QueryFields with more joins give +1, while fewer joins give -1
 		 * - For anything else, compare the toStrings of this and the supplied object.
 		 **/
-		public virtual Integer compareTo(Object o){
+		public Integer compareTo(Object o){
 			if(o == null || !(o instanceof QueryField))
 				return -2; //We can't possibly do a sane comparison against an unknwon type, go athead and let it "win"
 
@@ -869,6 +774,5 @@ public class fflib_QueryFactory { //No explicit sharing declaration - inherit fr
 	}
 	public class InvalidFieldSetException extends Exception{}
 	public class NonReferenceFieldException extends Exception{}
-	public class InvalidSubqueryRelationshipException extends Exception{}
-	public class InvalidOperationException extends Exception{}
-}
+	public class InvalidSubqueryRelationshipException extends Exception{}	
+}