public function search(QueryInterface $query) { is too cluttered

[nickveenhof]

public function search(QueryInterface $query) should probably be split out in the different types of query Solr supports.

It does not make sense that MLT queries are in the same logic as filter queries etc. Perhaps we need to split this out in subclasses? As it is now, it is very confusing to make it work.

[Berdir]

Splitting this huge method up is certainly a good idea, should help with testing and being able to follow the code. Unless we change the Search API query API on top of it, I guess we still need a single entry point.

[VeggieMeat]

Splitting would definitely be helpful, and a single entry point as we have now still works just fine. I've been splitting out the code in the Search API Elasticsearch module as each feature gets enough test coverage to be comfortable, and it has definitely made it a lot easier to follow and track down issues.

[drunken-monkey]

Splitting it into different methods, as we do in the DB backend, is certainly a good idea. We already do that to some degree (creating the keywords, filters, …), but the other parts can of course follow.

However, I'm not sure what you mean with "split this out in subclasses". How would that work? Of course, the class is rather huge, but I think it would only add confusion if we moved part of the code into completely different classes. And even if we did, I guess it should just be into helper classes (like Utility in the Search API), not subclasses – but you probably meant that, I guess?

[nickveenhof]

I did indeed meant exactly what you phrased above. A way to make this simpler and more readable is what we need to strive for. I'm happy to discuss any kind of change but having to dig through the function to see grouping together with more like this seems kind of weird and not logical.

The single point of entry isn't bad but splitting it up requires some thinking. More like this is a type of query, similar to select, while filters could possible be applied to all. Not all features are compatible with each other and this needs to be defined somehow.

[nickveenhof]

Started initial stab here: https://github.com/amateescu/search_api_solr/pull/21/files

Moving grouping away now to solrHelper and hopefully we can get rid of these strange solr query parameter arrays and keep it all in the solarium object

[nickveenhof]

Merged the pull request. The query() function is a bit more readable now and should be a good starting point to see how and if we can move the responsibilities to the respective submodules. This naturally means we need to figure out how we will tackle this