Added ability to auto-generate bind variable keys and specify filter logic

[superkensington]

Adds ability to auto-generate bind variable keys by invoking new generateBindVariableKeys method on an AggregateQuery or Query instance. Extends updates made in PR #49.

As a nice consequence of changes made, also adds ability to specify custom filter logic.

Query filters are now stored as a list of QueryFilter instances instead of Strings, allowing for the ability to invoke 'generateBindVariableKeys' at any point. Consequently, filter logic is now explicitly defined and used at the time of overall filter string generation rather than being included at the time a filter is added to the Query.

NOTE -- due to the change in the way the overall filter string in constructed I had to remove the overall sorting of the filters, but did leave the sorting of "OR" filters since that doesn't interfere. If the sorting is important to keep, a different approach to filter structure and logic would need to be taken to allow for it.

[jongpie]

Hi @superkensington - I've been reviewing your changes & testing things out, and I'm planning to make a few additional commits to the PR, including:

• Running prettier on the repo to make the formatting consistent
• Moving tests related to AccessLevel to not be included in the package - trying to create a new user in tests (using your minimumAccessUser() method) will fail in any org that has custom validations or custom required fields on the User object.
  • I'll still keep the tests, but they'll be moved to another test class that only runs in the pipeline/when creating a package version

One area I'm still struggling with conceptually - I can't think of a use case where I would want to actually specify a bind variable's key (or remove/clear keys). It creates more work for anyone building a query, so it feels like it would be better for the bind keys to always be auto-generated (when using bind variables). Can you think of a use case where someone needs to specify the bind key's variable, instead of having it be auto-generated?

My thought is...

• Remove the overloads with the new parameter String bindWithKey
• Remove the new bind methods
  • setBind(String key, Object value)
  • setBinds(Map<String, Object> binds)
  • removeBind(String key)
  • clearBinds()
  • generateBindVariableKeys()
• Update Query and AggregateQuery to have new constructors that indicate if binds should be used

This would indicate at the time of instantiating a query if binds should be (automatically) used, which then simplifies other things a good bit. Let me know what you think - I'm happy to make these changes if you think that makes sense.

[superkensington]

Hi @superkensington - I've been reviewing your changes & testing things out, and I'm planning to make a few additional commits to the PR, including:

• Running prettier on the repo to make the formatting consistent

• Moving tests related to AccessLevel to not be included in the package - trying to create a new user in tests (using your minimumAccessUser() method) will fail in any org that has custom validations or custom required fields on the User object.

  • I'll still keep the tests, but they'll be moved to another test class that only runs in the pipeline/when creating a package version

Makes sense 👍

One area I'm still struggling with conceptually - I can't think of a use case where I would want to actually specify a bind variable's key (or remove/clear keys). It creates more work for anyone building a query, so it feels like it would be better for the bind keys to always be auto-generated (when using bind variables). Can you think of a use case where someone needs to specify the bind key's variable, instead of having it be auto-generated?

With the addition of automatically generating the keys those methods definitely are no longer necessary and I agree that they can be removed.

My thought is...

• Remove the overloads with the new parameter String bindWithKey

• Remove the new bind methods

  • setBind(String key, Object value)
  • setBinds(Map<String, Object> binds)
  • removeBind(String key)
  • clearBinds()
  • generateBindVariableKeys()

• Update Query and AggregateQuery to have new constructors that indicate if binds should be used

This would indicate at the time of instantiating a query if binds should be (automatically) used, which then simplifies other things a good bit. Let me know what you think - I'm happy to make these changes if you think that makes sense.

This all sounds good to me; have at it!