fflib-apex-common icon indicating copy to clipboard operation
fflib-apex-common copied to clipboard

Proposed integration point for a new filter builder and suggested API

Open SWhalen-gh opened this issue 5 years ago • 11 comments
trafficstars

this is a continuation of a conversation started here: As a developer, I'd love advanced WHERE conditions ...

Currently the “fflib_QueryFactory” does not have an object oriented representation of a WHERE clause, and implementing one would be very valuable. Building a WHERE clause by hand is tedious and error-prone, and distracts the developer from their primary business task. Adding functionality for managing the WHERE clause would enable the queryFactory to centralize common logic related to quotes, commas, and conjunctions that many developers are solving in multiple places. It could also be a place to add more specialized logic related to how a filter should behave if Shield Encryption is enabled, or when to apply ‘EscapeSingleQuotes’.

@daveespo you suggested proposing an integration point and a skeleton API. As a starting point, the QueryFactory could have a new FilterBuilder object available:

Private FilterBuilder whereClause;
Public getWhereClause {return whereClause;}
Public setWhereClause (FilterBuilder whereClause) {this.whereClause = whereClause;}

The FilterBuilder instance could be initialized in the fflib_QueryFactory constructor, and then added to the rest of the query in the same area as the current ‘conditionExpression’:

if(conditionExpression != null)
     result += ' WHERE '+conditionExpression;
If (conditionExpression ==null) && (whereClause != null)
     result += ' WHERE '+ whereClause.toString();

Generally, a WHERE clause is a list of 1 or more conditions, separated by conjunctions. A condition is a structure consisting of a field, operator, and value. There is opportunity for many helpful overloads that default in some common operators (“=”) and conjunctions (“AND”).

Here is a small portion of an implementation that shows the declaration of the list of conditions, three constructors, and two variations of the “.add” method:

public class FilterBuilder {
    private list<FilterBuilder.condition> conditions = new list<FilterBuilder.condition>();
    public filterBuilder() {}
    
    public filterBuilder(SObjectField field, object value) {
        filterBuilder.condition c = new filterBuilder.condition(field, operator.eq, value);
        conditions.add(c);
    }
    public filterBuilder(condition c) {
        conditions.add(c);            
    }
    public filterBuilder add(condition c) {
        conditions.add(c);
        return this;
    }
    public filterBuilder add(conjunction conj, SObjectField field, operator op, object value) {
        filterBuilder.condition c = 
            new filterBuilder.condition(conj, field, op, value, false);
        conditions.add(c);
        return this;
    }

SWhalen-gh avatar Aug 05 '20 00:08 SWhalen-gh

We at Johnson & Johnson already implemented a dynamic/object-oriented (WHERE) query builder. It’s based on the well known Criteria class from Java. At this moment we are working on extending the functionality, when that’s done we will propose that change via a PR.

@daveespo, @SWhalen-gh

wimvelzeboer avatar Aug 05 '20 07:08 wimvelzeboer

@SWhalen-gh -- this is a good proposal

Does FilterBuilder need any carnal knowledge of the state of fflib_QueryFactory? I'm thinking of things like the SObject that the QueryFactory is constructed for and any subselect/relationships that are registered?

It seems like we should have an interface for QueryFactory to understand .. something along the lines of:

public interface FilterClause{
  public String toWhereClause();
}

FilterBuilder could be used to create a FilterClause and that would ensure that the line is clear between the methods used to create the the conditions and the methods used by fflib_QueryFactory

The modifications to fflib_QueryFactory would boil down to something like adding a single new method:

public class fflib_QueryFactory .... {
  public fflib_QueryFactory setCondition(FilterClause clause){
    this.conditionExpression = clause.toWhereClause();
   ...
  }

We could even start without modifying fflib_QueryFactory since you could accomplish the same thing by doing

qf.setCondition(clause.toWhereClause());

Point being: It seems like the proposal will have minimal or no impact to fflib_QueryFactory which is a good start.

@wimvelzeboer -- I looked at Hibernate's Criteria class .. definitely a rich set of features and a good source of inspiration

daveespo avatar Aug 13 '20 21:08 daveespo

@daveespo, SOLID suggestion!!! :wink:

john-storey-devops avatar Aug 13 '20 21:08 john-storey-devops

@stohn777 I am waiting until the pull-request for splitting the domain and trigger handler get merged, then I can raise the PR with the criteria class and have a discussion about that proposal.

I will include examples on how you can use the same criteria feature for Selector and Domain classes. Where you can use the same criteria definition for SOQL queries and filtering records inside a domain. Best would be to have a separate class where you store the filter clauses for a specific SObjectType and re-use that for both the Selector and the Domain.

CC: @daveespo

wimvelzeboer avatar Aug 14 '20 08:08 wimvelzeboer

@daveespo I don't see any need for the FilterBuilder to have knowledge or access to the state of the QueryBuilder. My local implementation has total separation, and my test code looks similar to your example above, i.e.:

    qf = new fflib_QueryFactory(SetupValue__c.SObjectType).selectFields(testFields); 
    fb= new filterBuilder();
    .add()...
     qf.setCondition(wc.ToString());
     list<sobject> records = Database.query(fb.toSOQL()); 

So, yes, as a starting point there can be no impact to fflib_QueryFactory. That is probably a good design principle to aim for. If a need arose, it would be plausible to deliver the containing QueryFactory to the FilterBuilder, e.g., on the constructor. That would satisfy the possible need for the FilterBuilder to ask for the sObject, while still having zero changes to fflib_QueryFactory.

And, an interface method of "toWhereClause" is a good idea.

SWhalen-gh avatar Aug 14 '20 13:08 SWhalen-gh

I wanted to add some more thoughts about the interface and implementation. I'm eager for the developers I work with use a tool like this because of some opporotunities to centralize the string-building that is occuring, and to embed some best practices into the tool.

The API should be terse. This will help a lot with adoption. The competition against the FilterBuilder is a plain old string, eg: "...name = 'Susan' and age__c > 65" which is tough to beat, in terms of brevity. If that old style feels faster to developers, they won't pick up the tool as much. Also, fluent is really nice, but many useages will be simple equivelancy tests inside of some other processing, eg:

FilterBuilder fb = new FilterBuilder();
For (sting key : CriteraMap.keySet() { //approximate psuedo-code
	fb.add(key, criteriaMap[key]) //very terse!
} 

An example of a carrot that encourages adoption of the tool is removing the task of thinking about data types and XSS. The logic for when to apply String.escapeSingleQuotes logic can be embedded inside the filter-builder engine. A value for a string field that contains single quotes should definitely have escaping applied before parsing and executing; a value that purports to represent an id or a date but which contains single quotes should throw an error without submitting the query to the database layer. There are also generally accepted ways to format dates and times for SOQL purposes, which can be build into the FilterBuilder: https://salesforce.stackexchange.com/questions/4799/using-a-date-for-a-datetime-field-in-a-soql-query-criteria https://developer.salesforce.com/docs/atlas.en-us.soql_sosl.meta/soql_sosl/sforce_api_calls_soql_select_dateformats.htm

I'm hoping that additions like these will add a lot of value to the tool and will make developers get out of the business of managing conjunctions, quotes, canonical date formats, and string concatenation.

SWhalen-gh avatar Sep 04 '20 16:09 SWhalen-gh

@daveespo @wimvelzeboer I have a FilterBuilder implementation I could create a pull-request for it this week, but I don't want to waste or duplicate effort.

SWhalen-gh avatar Oct 19 '20 20:10 SWhalen-gh

I have not, nor plan to, propose an implementation -- I see that @wimvelzeboer had indicated that he had plans for a PR but that was a few months ago

daveespo avatar Oct 20 '20 11:10 daveespo

@SWhalen-gh @daveespo Indeed, after I have finished the PR for the new domain class, I am planning to raise the PR with a filter based on the Java Criteria class. That filter can be used then by the domain and selector. Looking at my backlog I guess it will take me about three weeks before I can work on that.

wimvelzeboer avatar Oct 20 '20 13:10 wimvelzeboer

@wimvelzeboer Really excited to read this, as the comment in the source code "Currently the WHERE clause of the query is manipulated as a single string, and is decidedly less OO-styled than other methods. This is expected to be expanded upon in the future." leaves a lot to be guessed about when exactly this future is :-)

Love the idea to use the JPA Criteria API as inspiration! Are you making any progress? Are there pieces of work you might be able to outsource to someone else (e.g. me) if time is an issue?

fransf-wtax avatar Oct 08 '21 08:10 fransf-wtax

@fransf-wtax Yeah I have raised this PR #313 that includes this Criteria based design. It is also implemented in the add-on package that I have developed, it works on top of apex-common and is available here: fflib-apex-extensions

wimvelzeboer avatar Oct 08 '21 15:10 wimvelzeboer