configured search_state_class propagates to search builder

[barmintor]

see also #2716

SearchBuilder always uses the default SearchState implementation, regardless of application configuration. This PR allows the SearchBuilder scope to indicate the class to use for search states.

[cbeer]

I wonder if it would be cleaner if we gave Blacklight::SearchService a default_search_state/empty_search_state/etc method?

In 8.x, it looks like we could update with and where to require an existing SearchState instance.

[barmintor]

I think that's a better forward-looking approach; here I'm mostly trying to conform to the existing API (although SearchService and Blacklight::Controller drift a little bit here). I'm totally willing to pivot to empty_search_state early, though, if you're 👍 on adding some API in 7.x

[barmintor]

Another option is to move both search_state_class and search_service_class onto Blacklight::Configuration, and have the controller and search service both delegate there. (the builder class config is already there).

[barmintor]

Adding a commit here that:

• consolidates the class configurations into a dedicated config
• adds a search state factory method to Blacklight::Searchable and Blacklight::SearchService
• removes the direct access to configured search state classes in favor of SearchState#reset

[jcoyne]

Can you explain a bit more about the need for the repository_config? Should that go on a branch that is merged to main prior to a backport to 7.x? Can we separate that into another PR and provide justification for it?

[barmintor]

Sure: Right now there are two classes of object that function in the "scope" role for a SearchBuilder: blacklight controllers and instances of Blacklight::SearchService. Only the controllers currently have access to a configuration of search_state_class.

This presents a problem if the application has implemented a SearchState subclass that overrides/extends the param validation methods, as SearchBuilder uses the default Blacklight::SearchState to build new states.

In the first commit in this PR, I simply exposed a .search_state_class method on Blacklight::SearchService - this just nudged the API expectations of a "scope" for the SearchBuilder. Chris suggested that maybe the right thing to do was to add some API to allow the scope to produce a blank SearchState of the configured class.

In the second commit of this PR, I tried to suggest a way to do that - but if we were to make the search_state_class configuration available, it seems like the right thing to do is to move both search_state_class and search_service_class out of Blacklight::Controller and into the configuration. The introduction of the Configuration::RepositoryConfig class is an attempt to do that in a relatively uncluttered way.

I think we could also drop the second commit and just immediately deprecate the search_state_class methods?

[jcoyne]

@barmintor Hyrax uses the search_state_class, so I'd be hesitant to go down the path of deprecating/removing those methods: https://github.com/samvera/hyrax/blob/828cc4bb2c80e326228c7a4f8c14c366d23e97a4/app/controllers/concerns/hyrax/controller.rb#L13

[barmintor]

@jcoyne let me stash the second commit somewhere else

[barmintor]

@jcoyne ok I've reverted that change - thank you for the Hyrax note. This now just adds search_state_class to SearchService, so that the builder can ask @scope for it.

[jcoyne]

Is there a need to forward port this?

[barmintor]

@jcoyne there is - I'll put a PR together.

[barmintor]

Forward port in #2722