Merge pull request apex-enterprise-patterns#99 from financialforcedev/selector-and-query-factory-optimisations

Selector and query factory optimisations

Author: afawcett

fflib/src/classes/fflib_QueryFactory.cls

@@ -73,6 +73,8 @@ public class fflib_QueryFactory { //No explicit sharing declaration - inherit fr
 	**/
 	private Boolean enforceFLS;
 
+	private Boolean sortSelectFields = true;
+	
 	/**
 	 * The relationship and  subselectQueryMap variables are used to support subselect queries.  Subselects can be added to 
 	 * a query, as long as it isn't a subselect query itself.  You may have many subselects inside
@@ -173,6 +175,17 @@ public class fflib_QueryFactory { //No explicit sharing declaration - inherit fr
 		return this;
 	}
 
+	/**
+	 * 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.
+	 * @param whether or not select fields should be sorted in the soql statement.
+	 **/
+	public fflib_QueryFactory setSortSelectFields(Boolean doSort){
+		this.sortSelectFields = doSort;
+		return this;
+	}
+	
 	/**
 	 * Selects a single field from the SObject specified in {@link #table}.
 	 * Selecting fields is idempotent, if this field is already selected calling this method will have no additional impact.
@@ -408,6 +421,10 @@ public class fflib_QueryFactory { //No explicit sharing declaration - inherit fr
 		}
 
 		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();
 		}
@@ -537,13 +554,17 @@ public class fflib_QueryFactory { //No explicit sharing declaration - inherit fr
 		if (fields.size() == 0){
 			if (enforceFLS) fflib_SecurityUtils.checkFieldIsReadable(table, 'Id');
 			result += 'Id  ';
-		}else{
+		}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()){
 			for (fflib_QueryFactory childRow : subselectQueryMap.values()){
 				result += ' (' + childRow.toSOQL() + '), ';
@@ -675,57 +696,70 @@ public class fflib_QueryFactory { //No explicit sharing declaration - inherit fr
 		}
 		public override String toString(){
 			String result = '';
-			Iterator<Schema.sObjectField> i = fields.iterator();
-			while(i.hasNext()){
-				String fieldName = i.next().getDescribe().getName();
-				if(fieldName.endsWithIgnoreCase('Id') && i.hasNext())
-					fieldName = fieldName.removeEndIgnoreCase('Id');
-				if(fieldName.endsWithIgnoreCase('__c') && i.hasNext())
-					fieldName = fieldName.removeEndIgnoreCase('__c')+'__r';
-				result += fieldName + (i.hasNext() ? '.' :'');
+			Integer size = fields.size();
+			for (Integer i=0; i<size; i++)
+			{
+				if (i>0)
+				{
+					if (result.endsWithIgnoreCase('Id'))
+						result = result.removeEndIgnoreCase('Id');
+					else if (result.endsWithIgnoreCase('__c')) {
+						result = result.removeEndIgnoreCase('__c') + '__r';
+					}
+					result += '.';
+				}
+				result += fields[i].getDescribe().getName();
 			}
 			return result;
 		} 
 		public integer hashCode(){
 			return String.valueOf(this.fields).hashCode();
 		}
 		public boolean equals(Object obj){
-			if(!(obj instanceof QueryField))
+			//Easy checks first
+			if(obj == null || !(obj instanceof QueryField))
 				return false;
-			if( String.valueOf(((QueryField) obj).fields) != String.valueOf(this.fields))
+
+			if (this === obj)
+				return true;
+
+			//Detailed checks
+			QueryField other = (QueryField)obj;
+			Integer size = fields.size();
+			if (size != other.fields.size())
 				return false;
-			Set<Schema.SObjectField> objFields = new Set<Schema.SObjectField>();
-			objFields.addAll( ((QueryField)obj).fields );
-			objFields.retainAll(this.fields);
-			objFields.removeAll(this.fields);
-			return objFields.size() == 0;
+
+			for (Integer i=0; i<size; i++)
+				if (fields[i] != (other.fields[i]))
+					return false;
+
+			return true;
 		}
 		/**
-		 * Allows sorting QueryField instances, which means we'll get deterministic field ordering by just sorting the parent
-		 * QueryFactory's array when toSOQL'ing.
+		 * Allows sorting QueryField instances.
 		 *
 		 * Returns:
 		 * - Objects that are not QueryField instances as -2, which functions as -1 but with more flair
-		 * - QueryField instances with less joins in their path as -1
-		 * - QueryField instances with an equal number of joins and alphabetically first as an undefined negative integer
-		 * - equals as 0
-		 * - anything else an undefined positive integer (usually, but not always 1)
+		 * - Equivalent QueryFields return 0.
+		 * - QueryFields with more joins give +1, while fewer joins give -1
+		 * - For anything else, compare the toStrings of this and the supplied object.
 		 **/
 		public Integer compareTo(Object o){
-			if(!(o instanceof QueryField))
+			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"
-			QueryField that = (QueryField) o;
-			if(this.fields.size() < that.fields.size()){
+			
+			if (this === o)
+				return 0;
+
+			QueryField other = (QueryField)o;
+			Integer size = fields.size();
+			Integer sizeOther = other.fields.size();
+			if (size < sizeOther)
 				return -1;
-			}else if( this.fields.size() == that.fields.size() ){
-				if(this.equals(that)){
-					return 0;
-				}else{
-					return this.toString().compareTo(that.toString());
-				}
-			}else{
+			if (size > sizeOther)
 				return 1;
-			}
+
+			return this.toString().compareTo(other.toString());
 		}
 	}
 

fflib/src/classes/fflib_QueryFactoryTest.cls

@@ -630,6 +630,7 @@ private class fflib_QueryFactoryTest {
 			User.SObjectType.fields.Name
 		});
 		System.assertEquals(-2, qf.compareTo(otherType));
+		System.assertEquals(-2, qf.compareTo(null));
 		System.assertEquals(0, qf.compareTo(qf));
 		System.assertEquals(
 			0, 
@@ -774,6 +775,35 @@ private class fflib_QueryFactoryTest {
 		System.assert(qf.getSubselectQueries().get(0).getOrderings().size() == 0);
 		System.assert(qf2.getSubselectQueries().get(0).getOrderings().size() == 1);
 	}
+	
+	@isTest
+	static void testSoql_unsortedSelectFields(){
+		//Given
+		fflib_QueryFactory qf = new fflib_QueryFactory(User.SObjectType);
+		qf.selectFields(new List<String>{
+			'Id',
+			'FirstName',
+			'LastName',
+			'CreatedBy.Name',
+			'CreatedBy.Manager',
+			'LastModifiedBy.Email'
+		});
+
+		qf.setSortSelectFields(false);
+
+		String orderedQuery =
+			'SELECT '
+			+'FirstName, Id, LastName, ' //less joins come first, alphabetically
+			+'CreatedBy.ManagerId, CreatedBy.Name, LastModifiedBy.Email ' //alphabetical on the same number of joins'
+			+'FROM User';
+
+		//When
+		String actualSoql = qf.toSOQL();
+
+		//Then		
+		System.assertNotEquals(orderedQuery, actualSoql);
+	}
+
 
 	public static User createTestUser_noAccess(){
 		User usr;

fflib/src/classes/fflib_SObjectSelector.cls

@@ -59,6 +59,12 @@ public abstract with sharing class fflib_SObjectSelector
     * Order by field
     **/
     	private String m_orderBy;
+
+    /**
+     * Sort the query fields in the select statement (defaults to true, at the expense of performance).
+     * Switch this off if you need more performant queries.
+     **/
+    private Boolean m_sortSelectFields;
 
    /**
    * Describe helper
@@ -106,10 +112,24 @@ public abstract with sharing class fflib_SObjectSelector
      * @param includeFieldSetFields Set to true if the Selector queries are to include Fieldset fields as well 
      **/
     public fflib_SObjectSelector(Boolean includeFieldSetFields, Boolean enforceCRUD, Boolean enforceFLS)
+    {
+        this(includeFieldSetFields, enforceCRUD, enforceFLS, true);
+    }
+
+    /**
+     * Constructs the Selector
+     *
+     * @param includeFieldSetFields Set to true if the Selector queries are to include Fieldset fields as well 
+     * @param enforceCRUD Enforce CRUD security
+     * @param enforeFLS Enforce Field Level Security
+     * @param sortSelectFields Set to false if selecting many columns to skip sorting select fields and improve performance
+     **/
+    public fflib_SObjectSelector(Boolean includeFieldSetFields, Boolean enforceCRUD, Boolean enforceFLS, Boolean sortSelectFields)
     {
         m_includeFieldSetFields = includeFieldSetFields;
         m_enforceCRUD = enforceCRUD;
         m_enforceFLS = enforceFLS;
+        m_sortSelectFields = sortSelectFields;
     }
 
     /**
@@ -366,7 +386,8 @@ public abstract with sharing class fflib_SObjectSelector
         if(includeSelectorFields)
         {
 	        // select the Selector fields and Fieldsets and set order
-	        queryFactory.selectFields(new Set<SObjectField>(getSObjectFieldList()));
+            queryFactory.selectFields(getSObjectFieldList());
+
 	        List<Schema.FieldSet> fieldSetList = getSObjectFieldSetList();
 	        if(m_includeFieldSetFields && fieldSetList != null)
 	            for(Schema.FieldSet fieldSet : fieldSetList)
@@ -394,7 +415,9 @@ public abstract with sharing class fflib_SObjectSelector
                 fieldSortOrder = fflib_QueryFactory.SortOrder.ASCENDING;
             queryFactory.addOrdering(fieldNamePart, fieldSortOrder);
         }           
-                
+        
+        queryFactory.setSortSelectFields(m_sortSelectFields);           
+
         return queryFactory;    
     }    
 }

fflib/src/classes/fflib_SObjectSelectorTest.cls

@@ -144,7 +144,7 @@ private with sharing class fflib_SObjectSelectorTest
 			return; // Abort the test if unable to create a user with low enough acess
 		System.runAs(testUser)
 		{					
-			Testfflib_SObjectSelector selector = new Testfflib_SObjectSelector(false, false, false);
+			Testfflib_SObjectSelector selector = new Testfflib_SObjectSelector(false, false, false, true);
 			try
 			{
 				List<Account> result = (List<Account>) selector.selectSObjectsById(idSet);
@@ -205,16 +205,50 @@ private with sharing class fflib_SObjectSelectorTest
 		}
 	}
 
+
+	@isTest
+	static void testWithoutSorting()
+	{
+		//Given
+		Testfflib_SObjectSelector selector = new Testfflib_SObjectSelector(false, false, false, false);
+		fflib_QueryFactory qf = selector.newQueryFactory();
+		
+		Set<String> expectedSelectFields = new Set<String>{ 'Name', 'Id', 'AccountNumber', 'AnnualRevenue' };
+		if (UserInfo.isMultiCurrencyOrganization())
+		{
+			expectedSelectFields.add('CurrencyIsoCode');
+		}
+
+		//When
+		String soql = qf.toSOQL();
+
+		//Then
+		Pattern soqlPattern = Pattern.compile('SELECT (.*) FROM Account ORDER BY Name DESC NULLS FIRST , AnnualRevenue ASC NULLS FIRST ');
+		Matcher soqlMatcher = soqlPattern.matcher(soql);
+		soqlMatcher.matches();
+
+		List<String> actualSelectFields = soqlMatcher.group(1).deleteWhiteSpace().split(',');
+		System.assertEquals(expectedSelectFields, new Set<String>(actualSelectFields));
+	}
+	
+	private static void assertEqualsSelectFields(String expectedSelectFields, String actualSelectFields)
+	{
+		Set<String> expected = new Set<String>(expectedSelectFields.deleteWhiteSpace().split(','));
+		Set<String> actual = new Set<String>(actualSelectFields.deleteWhiteSpace().split(','));
+
+		System.assertEquals(expected, actual);
+	}
+	
 	private class Testfflib_SObjectSelector extends fflib_SObjectSelector
 	{
 		public Testfflib_SObjectSelector()
 		{
 			super();
 		}
 
-		public Testfflib_SObjectSelector(Boolean includeFieldSetFields, Boolean enforceCRUD, Boolean enforceFLS)
+		public Testfflib_SObjectSelector(Boolean includeFieldSetFields, Boolean enforceCRUD, Boolean enforceFLS, Boolean sortSelectFields)
 		{
-			super(includeFieldSetFields, enforceCRUD, enforceFLS);
+			super(includeFieldSetFields, enforceCRUD, enforceFLS, sortSelectFields);
 		}
 
 		public List<Schema.SObjectField> getSObjectFieldList()