Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enabling SOQL functions ( closes #163 ) #258

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Enabling SOQL functions ( closes #163 ) #258

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f8fdc9b
163: Adding logic to exclude the SOQL-function on describe-matching a…
rvandenassum Dec 26, 2019
4e99487
Merge pull request #1 from apex-enterprise-patterns/master
foxysolutions Dec 26, 2019
534ebc8
Merge branch 'master' into feature/163-EnablingSOQLFunctions
rvandenassum Dec 26, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 49 additions & 14 deletions fflib/src/classes/fflib_QueryFactory.cls
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,17 +84,51 @@ public class fflib_QueryFactory { //No explicit sharing declaration - inherit fr
private Schema.ChildRelationship relationship;
private Map<Schema.ChildRelationship, fflib_QueryFactory> subselectQueryMap;

private String getFieldPath(String fieldName){
if(!fieldName.contains('.')){ //single field
/**
* Pattern variable to prevent the Pattern to be compiled for each field
*/
private Pattern soqlFunctionPattern { get{
if( soqlFunctionPattern == null ){
soqlFunctionPattern = Pattern.compile( '^([a-zA-Z_]*\\(\\s*)([a-zA-Z0-9_\\.]+)?(\\s*\\)[a-zA-Z ]*)$' );
}
return soqlFunctionPattern;
} set; }

private String getFieldPath( String fieldName ){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By adding this logic inside this method the complexity suddenly becomes very high. I would advise to split this logic into a number of private methods to keep a level of abstraction here in his method.

Copy link
Contributor Author

@foxysolutions foxysolutions Jan 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wimvelzeboer I do agree with you on the increased complexity and length of the method. I've thought of this before, but in my perspective this will only become more difficult to read, since beforeField and afterField are used throughout the function. The methods should then return a list/map or an innerclass to support. That's why I've put emphasize on the comments.

If you have any suggestion how to split and make sure all data is properly fed back to be used, please let me know!

Copy link
Contributor

@wimvelzeboer wimvelzeboer Mar 12, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would maybe split up this method into three parts:
private String getFieldPathForMethod(String fieldName)
private String getFieldPathForRelatedField(String fieldName)
private String getFieldPathForField(String fieldName)

private String getFieldPath( String fieldName )
{
  if (fieldName.contains( '(' )) return getFieldPathForMethod(fieldName);
  if (fieldName.contains( '.' ) ) return getFieldPathForRelatedField(fieldName);
  return getFieldPathForField(fieldName);
}

private getFieldPathForMethod(String methodCall)
{
  Matcher match = this.soqlFunctionPattern.matcher( methodCall );
  if (!match.matches())
  {
    throw new InvalidFieldException( 'The method '+ methodCall + 'does specify a ( function opening, but doesn\'t match the pattern' );
  }
  
  String beforeField = match.group( 1 );
  String fieldName = match.group( 2 );
  String afterField = match.group( 3 );

  // When no fieldName is specified (e.g. COUNT() ), return directly, since nothing to check
  if (String.isBlank( fieldName ))
  {
    return beforeField + afterField;
  }
  return beforeField + getFieldPath(fieldName) + afterField;
}

// Check whether field contains a SOQL function statement - if so, split from field
String beforeField = '';
String afterField = '';
if( fieldName.contains( '(' ) ){
// Splitting string into three groups: [ 'METHOD( ', 'Field__c', ') as c' ]
Matcher match = this.soqlFunctionPattern.matcher( fieldName );

if( match.matches() ){
beforeField = match.group( 1 );
fieldName = match.group( 2 );
afterField = match.group( 3 );

// When no fieldName is specified (e.g. COUNT() ), return directly, since nothing to check
if( String.isBlank( fieldName ) ){
return beforeField + afterField;
}
} else{
throw new InvalidFieldException( 'The field '+ fieldName + 'does specify a ( function opening, but doesn\'t match the pattern' );
}
}

if( !fieldName.contains( '.' ) ){ //single field
Schema.SObjectField token = fflib_SObjectDescribe.getDescribe(table).getField(fieldName.toLowerCase());
if(token == null)
throw new InvalidFieldException(fieldName,this.table);
if (enforceFLS)
fflib_SecurityUtils.checkFieldIsReadable(this.table, token);
return token.getDescribe().getName();
if( token == null ){
throw new InvalidFieldException( fieldName, this.table );
}
if( enforceFLS ){
fflib_SecurityUtils.checkFieldIsReadable( this.table, token );
}
// Once verified field exists and user has access, include function parts back in
return beforeField + token.getDescribe().getName() + afterField;
}

//traversing FK relationship(s)
// Traversing FK relationship(s)
List<String> fieldPath = new List<String>();
Schema.sObjectType lastSObjectType = table;
Iterator<String> i = fieldName.split('\\.').iterator();
Expand All @@ -113,14 +147,15 @@ public class fflib_QueryFactory { //No explicit sharing declaration - inherit fr
}else if(token != null && !i.hasNext()){
fieldPath.add(tokenDescribe.getName());
}else{
if(token == null)
throw new InvalidFieldException(field,lastSObjectType);
else
throw new NonReferenceFieldException(lastSObjectType+'.'+field+' is not a lookup or master-detail field but is used in a cross-object query field.');
if(token == null){
throw new InvalidFieldException( field, lastSObjectType );
} else{
throw new NonReferenceFieldException( lastSObjectType + '.' + field + ' is not a lookup or master-detail field but is used in a cross-object query field.' );
}
}
}

return String.join(fieldPath,'.');
return beforeField + String.join( fieldPath, '.' ) + afterField;
}

@TestVisible
Expand Down Expand Up @@ -763,4 +798,4 @@ 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{}
}
}
31 changes: 30 additions & 1 deletion fflib/src/classes/fflib_QueryFactoryTest.cls
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,35 @@ private class fflib_QueryFactoryTest {
System.assertEquals(1, query.countMatches('Name'), 'Expected one name field in query: '+query );
}

@isTest
static void selectFunction_COUNT() {
fflib_QueryFactory qf = new fflib_QueryFactory(Contact.SObjectType);
qf.selectField( 'COUNT()' );
qf.selectField( 'COUNT( eMail ) as countEmail' );
qf.selectField( 'COUNT_DISTINCT( Name )' );
String query = qf.toSOQL();
System.assertEquals( 'SELECT COUNT( Email ) as countEmail, COUNT(), COUNT_DISTINCT( Name ) FROM Contact', query, 'Expected COUNT fields to be in query '+ query );
}

@isTest
static void selectFunction_ToLabel() {
fflib_QueryFactory qf = new fflib_QueryFactory(Contact.SObjectType);
qf.selectField( 'toLabel(Name)' );
String query = qf.toSOQL();
System.assertEquals( 'SELECT toLabel(Name) FROM Contact', query, 'Expected COUNT fields to be in query '+ query );
}

@isTest
static void selectFunction_Exception() {
fflib_QueryFactory qf = new fflib_QueryFactory(Contact.SObjectType);
try{
qf.selectField( 'AVG( MIN( CreatedDate ) )' );
System.assert( false, 'Expected exception to be thrown' );
} catch( Exception ex ){
System.assert( ex.getMessage().contains( 'does specify a ( function opening' ), 'Expected exception to be thrown' );
}
}

@isTest
static void equalityCheck(){
fflib_QueryFactory qf1 = new fflib_QueryFactory(Contact.SObjectType);
Expand Down Expand Up @@ -847,4 +876,4 @@ private class fflib_QueryFactoryTest {
}
return usr;
}
}
}