Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Functions like hook_apachesolr_query_prepare() or hook_apachesolr_query_alter() need to be aware of the current search page!
Currently it's not easy to implement custom sorts or queries for dedicated search pages.
I created a patch that add a method getSearchPageId() to the query class.
Everything has been achieved in way that does not break the existing API.
BTW this patch is essential to make Apache Solr Multilingual functional for different search pages.
Comment | File | Size | Author |
---|---|---|---|
#35 | 1918012-35.patch | 12.62 KB | mkalkbrenner |
#34 | 1918012-34.patch | 11.66 KB | kevin.dutra |
#26 | 918012-26.patch | 11.72 KB | Nick_vh |
#25 | 918012-25.patch | 11.72 KB | Nick_vh |
#21 | 918012-21.patch | 10.88 KB | Nick_vh |
Comments
Comment #1
mkalkbrennerComment #2
pwolanin CreditAttribution: pwolanin commentedMarkus, can you give more specific examples of your use case?
Right now I 'd think about creating a search environment for each set of custom alterations, and then using that env for one or more pages (or blocks, etc).
Comment #3
mkalkbrennerIn Apache Solr Multilingual you can for example define if the search results should be limited to the current language by default. Without that patch the query could only be altered for all search pages. With that patch this could be done individually per search page:
Another use case is that you use hook_apachesolr_query_prepare($query) to modify the sorts.
There's also noe easy way to implement different sorts for different search pages which looks like a standard feature.
Comment #4
mkalkbrennerTo be more specific, this is the example code in apachsesolr.api.php:
This will modify the sort for ALL search pages.
With my patch you can to something like this without breaking the existing API and existing implementations of different contrib modules:
I think the motivation for this patch is obvious, because hook_apachesolr_query_prepare is older than the concept of search pages.
Comment #5
pwolanin CreditAttribution: pwolanin commentedSo, I would suggest, as above, using environments like:
Comment #6
klonos...coming from #1702354: Apache Solr Multilingual 7.x Roadmap
Comment #7
mkalkbrennerPeter, you're right that you can use environments as workaround for that example.
But I don't see what's wrong in having the possibility to distinguish between different search pages in hooks like query_alter or query_prepare. The patch doesn't break anything.
In a default installation you have two search pages for one environment (core search, taxonomy) and one mlt block.
In the hooks query_alter and query_prepare there's no common way to target one of them using the existing properties of the query object:
core search and taxonomy use the same name 'apachesolr', the base_path could be modified, the mlt block has a dedicated name 'apachesolr_mlt' but might occur on different pages.
Comment #8
pwolanin CreditAttribution: pwolanin commented@mkalkbrenner - so... what if I want to do some alterations specific to the MLT block?
I think there are a couple options:
1) Use only environments as I suggested above.
2) Add some kind of "context" or "source" property or method to the query object.
#2 would basically depend on people correctly setting the value when constructing the query
Comment #9
mkalkbrennerIn apachesolr the site administrator can create environments and search pages using these environments.
The current dev version of apachesolr_multilingual adds index related options to the environment forms and query related options to the search page form. To access theses settings at runtime, the second part requires the patch we discuss here.
Without that patch, we will implement the same approach in apachesolr_multiligual, but we need much more code and it will be harder to maintain.
To access the environment id, we already use exactly the same code you posted in #5.
That's what we want. In fact the environment id is already part of the query object. The environment Id in combination with the page id will describe full context.
Using my patch, the site administrator doesn't have to care about setting the right context manually. It will be set automatically.
If you think about developers as "people", the context will be set if the use the apachesolr functions to create a query object.
If they create a query manually, you're right that they have to set the context manually. But if they don't, that doesn't matter! The query runs in a "global" or - let's say - "environment only" context as before without my patch. It' won't break anything.
I'm still convinced that a "complete" context for the query object will be a benefit for apachesolr.
If you're not satisfied with the page id property, I'm also fine with a context property containing environment id and page id.
Comment #10
pwolanin CreditAttribution: pwolanin commentednot all "contexts" are pages - e.g. the MLT block
This patch needs more docs and probably more discussion/work, but I think this would be more useful and generic than linking it to page ID.
Comment #11
mkalkbrennerThank you Peter.
This approach is fine for me and you're right that the generic approach is more useful.
I tested this patch and it works well with apachesolr multilingual, beside that I had to fix a small bug in the interface definition that caused a fatal error:
Comment #13
mkalkbrennerBTW these hooks benefit from the context as well:
hook_apachesolr_search_result_alter($document, &$extra, DrupalSolrQueryInterface $query)
hook_apachesolr_process_results(&$results, DrupalSolrQueryInterface $query)
Comment #14
mkalkbrenner#11: 1918012.patch queued for re-testing.
Comment #16
pwolanin CreditAttribution: pwolanin commentedupdating title
the test error is:
Comment #17
Nick_vhadd these functions to the Dummy_Solr.php file in the test directory
Comment #18
mkalkbrennerDummySolr doesn't implement the modified interface DrupalSolrQueryInterface.
Comment #19
mkalkbrennerThe patch in #11 discovered two already existing bugs in the test cases where a string has been passed instead of an object:
needs to be
Comment #21
Nick_vhComment #23
Nick_vh#21: 918012-21.patch queued for re-testing.
Comment #25
Nick_vhComment #26
Nick_vhMake the function definition more consistent and use NULL. The issue with the test was that the dummy solr file was not included
Comment #27
Nick_vhCommitted and pushed to 7.x-1.x - thanks!
Comment #28
pwolanin CreditAttribution: pwolanin commentedI'm worried that the interface change is an API break that we should not be introducing.
Comment #29
mkalkbrennerPeter, can you explain where you see an API change that breaks anything?
From my point of view we only add additional methods.
All exiting code in contributed apachesolr modules should still work without any changes!
Comment #30
pwolanin CreditAttribution: pwolanin commentedok, so I guess the constructor is not part of the interface?
Comment #31
pwolanin CreditAttribution: pwolanin commentedSorry, didn't mean to flip the status again.
As you saw with the test class, anyone implementing a version of this that doesn't inherit from the normal class will have their code break because it won't match the interface. However, I doubt anyone is doing that in reality.
Comment #32
mkalkbrennerRight, the constructor is not part of the interface.
You introduced getSearcher and getName with your patch, which aren't related to this issue.
But that doesn't really matter because getContext and addContext require the API change anyway.
So the additional methods in the interaface are an API change which could break a contrib module if someone implemented his own DrupalSolrQuery class.
But I second your opinion:
And if it's the case it will be easy to fix as you can see at the fixed tests.
So I vote for that kind of "API enhancement" for the next release.
BTW my initial patch didn't change the interface at all.
Comment #33
pwolanin CreditAttribution: pwolanin commentedok, well we can put it in the release notes... back to needing to be ported then
Comment #34
kevin.dutra CreditAttribution: kevin.dutra commentedComment #35
mkalkbrennerPatch #34 does not apply to latest 6.x-3.x-dev anymore. Here's an updated version.
Comment #36
mkalkbrennerComment #38
mkalkbrennerComment #40
rahul_sankrit CreditAttribution: rahul_sankrit as a volunteer commentedI am using Apache Solr autocomplete module for solr auto search but suggestion is not coming in search box, I did debug and got fatal error like this :
Debug url : example.com/apachesolr_autocomplete?query=test (this is the search key)
Fatal error: Call to undefined method SolrBaseQuery::get_fq() in /var/www/html/drupal/sites/all/modules/contributed/apachesolr/apachesolr.module on line 2805
I am using : Apache Solr framework - 6.x-3.0-rc1 and Apache Solr autocomplete - 6.x-1.1
Comment #41
mkalkbrennerSorry, we don't support 6.x anymore. I even don't have an environment to reproduce the issue.