Static lists like ArrayList or EagerLoadedDataList should support SearchFilter syntax for filter/exclude

[patricknelson]

Request for enhancement to ArrayList originally discussed in (and subsequently taken from) core dev DL and setup as an issue here for tracking.

It appears that array lists do not actually utilize filters. I was racking my brain trying to figure out why the heck this filter wouldn't work at all:

return $list->filter([
    "IntegerField:GreaterThan" => 123,
])->first();

After inspecting the ->filter() method, I noticed this core class didn't seem to handle this in the same way as DataList. It seemed to just do a straight direct comparison... is this an oversight and, if so, should this be added into core? The consensus thus far seems to be that this enhancement would, in fact, be desirable 😄

Third party library for consideration to help handle this for us (per @kinglozzer's recommendation): http://elliotswebsite.com/Pinq/ and https://github.com/TimeToogo/Pinq

Acceptance Criteria

• ArrayList supports SearchFilter syntax for its filter(), filterAny(), exclude(), and find() methods
• EagerLoadedDataList supports searchfilter syntax for its filter(), filterAny(), exclude(), excludeAny(), and find() methods
• Custom SearchFilter implementations can be used
• Document the prefered approach of creating new filters that work with both SQL and non-SQL filtering
• Document how to register new filters that work with DataList and other non-SQL list.
• Docs are updated to remove any mention of this functionality not working and mention, in the appropriate places, that this does work.

Notes

• There is an idea in the comments of this issue for adding API to the SearchFilter classes themselves to declare whether a record "matches" that filter.
• There is an implementation of ArrayList which uses SearchFilters in a link in one of the comments on this thread that might provide useful inspiration.

PRs

• NEW Enable ArrayList and EagerLoadedList to use search filters #10910
• NEW Enable ArrayList and EagerLoadedList to use search filters #10925
• DOC Document changes from silverstripe/silverstripe-framework#10925 developer-docs#335

[sminnee]

To do this, you’d probably want something that provides a tester function that’s called on each object in the list So in addition to SearchFilter::apply(DataQuery $query), you’d have SearchFilter::matches($record) which is passed an object.

Then you could implement matches on all the current search filters.

Finally, in ArrayList, you'd want to replicate the filter() / addFilter() code from DataList that builds SearchFilter objects.

Then in the ArrayList iterator (which I’ve refactored to a generator in another PR, which will be really helpful for this) can have some code along the lines of:

function getIterator() {
  foreach($this->items as $record) {
    if($this->recordMatchesFilters($record)) {
      yield $record;
    }
  }
}

protected function matchesFilters($record) {
  foreach($this->filters as $filter) {
    if(!$this->filters->matches($record) return false;
  }
  return true;
}

[sminnee]

While you're at it, it might make sense to refactor the SearchFilter classes to have PerRecordFilter and DataQueryFilter as two separate interfaces. That way you can choose to support 1 or both approaches on each searchfilter class. It would also mean that things like filterByCallback could be implemented as a PerRecordFilter and have DataList handle those (in admittedly a lower-performance manner)

[GuySartorelli]

I created a module last year that provides this functionality: https://github.com/signify-nz/silverstripe-searchfilter-arraylist
I wish I had seen this ticket at the time, I probably would have just done what Sminnee suggested instead and had it added into core 😅
Still might, but in the meantime if anyone comes across this issue and wants the functionality, the module is there.

[patricknelson]

You definitely should @GuySartorelli. Just checked that out and I'm wondering: Can that functionality be implemented as an extension to ArrayList instead or is a subclass it required for the functionality?

Anyway, would be awesome to see this supported in core without the added complexity of rolling your own or having to use another module/extension that you'd have to support over the years (I mean, hell, this issue/PR has been open for nearly 6yrs! I'm feeling old).

[GuySartorelli]

Can that functionality be implemented as an extension to ArrayList instead or is a subclass it required for the functionality?

There aren't any extension hooks in ArrayList, so an Extension wouldn't be able to alter the way filtering works.

[michalkleiner]

We can add hooks as an enhancement as well as look into bringing that into the core, happy to review either.

[GuySartorelli]

I think bringing the functionality into core would be the more beneficial of those. I don't think hooks in ArrayList would be particularly useful - any project-specific changes to the functionality of that class are probably better off as subclasses and it's an Injectable class. Granted there are places where they're instantiated using the new keyword so injection doesn't work everywhere but that's something I intend to address separately anyway.

[GuySartorelli]

I've widened the scope of this issue to include the new (will be included in silverstripe/framework 5.1.0) EagerLoadedDataList which should also support this syntax.