As written as one in the suggestions in #2044393: Changes in search query functionality:

Unify whether/where the query checks the validity of method parameters. I.e., we currently validate input for fields() and sort(), but not for condition(). (If we want to check there, too, the filter needs to be aware of the index, though – but that might make sense in any case.) Either we should check in all the methods, where it makes sense, or just once in execute() (would eliminate the possibility that other methods throw exceptions – probably a DX plus), or completely leave that to the service classes (bad UX because of code duplication).

However, I'd now argue that while moving the validation out of the query class entirely does lead to a bit of code duplication, but on the other hand it allows backends to easily change the definition of what constitutes "valid" values for all of the query data. This is important when features, e.g., introduce new fields (e.g., #1197538: Random sort in Views), new operators (something like #1783746: Add support for "between" operator (though we want to have that in the Search API itself now anyways)), etc.
Also, most of the values are currently double-checked, since the backend plugins tend to not trust the validation of the query class anyways.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

drunken monkey created an issue. See original summary.

drunken monkey’s picture

Status: Active » Needs review
FileSize
14.44 KB

Something like this.

(Note: This would also lead to changes in the Solr backend, and maybe other backends, becoming necessary.)

borisson_’s picture

Status: Needs review » Reviewed & tested by the community

This change makes a lot of sense even though this'd break solr other backends, I think doing this now (before the official 8.0.0 release) is a good time.

drunken monkey’s picture

Status: Reviewed & tested by the community » Fixed

Thanks for your input, good to hear others also think that this makes sense!
Committed.

  • drunken monkey committed 4796b44 on 8.x-1.x
    Issue #2604874 by drunken monkey: Moved the query validation logic to...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.