Posted by cpliakas on May 5, 2011 at 1:19am
3 followers
Jump to:
| Project: | Facet API |
| Version: | 7.x-1.x-dev |
| Component: | Code |
| Category: | task |
| Priority: | normal |
| Assigned: | cpliakas |
| Status: | closed (fixed) |
Issue Summary
Currently facet builds are initialized on demand. This architecture is problematic because some of the data isn't available when rendering breadcrumbs and the Current Search block. This leads to nasty methods such as FacetapiAdapter::mapActiveItem().
An alternative approach would be to initialize the builds of all enabled facets when the first realm is rendered. This will ensure all required data is available during any point of the facet building process.
Comments
#1
Fixed typo in title.
#2
Created feature branch at http://drupal.org/node/808232/git-instructions/initialization-flow-1147510.
#3
Changing title to more accurately reflect the task. It would be better to move the initial processing outside of the FacetapiAdapter::buildRealm() method anyways.
#4
We discussed making the facet info key the same as the alias, and therefore each field alias would be unique. It seemed like that might help with the rendering problem?
#5
It helps a little, but that isn't the crux of the issue. We really need two things by the time the breadcrumbs and current search block are loaded: the mapped values and the hierarchical relationships which a unique alias wont really help with. The facet build simply needs to be initialized earlier in the page flow. They can't be built too early otherwise that don't get the various contexts set by the backend (i.e. wheter a search has been executed), so you can't accurately determine whether to build the facets or not.
In the initialization-flow-1147510 branch, I added a bunch of comments. The reason for the API change in #1152368: Implement API change to prevent the need for overrides and allowing Facet API to better control application flow is that the refactored FacetapiAdapter::addActiveFilters() method seems to be a great place to initialize the build. It happens late enough in the application flow so that it can be loaded on demand, and it is invoked before all other facets are rendered. Prior to the API change, adapters had to override this method making it the responsibility of the backend to initiate the building process. Not ideal by any means.
The one sticking point I have is that it might be running a little too early in that it doesn't know whether a search has been executed. Looking at the apachesolr_search_run() and apachesolr_search_run_empty() functions, it appear that apachesolr_has_searched() is called after apachesolr_do_query() which means the adapter won't have the proper contexts set in FacetapiAdapter::addActiveFilters(). Would it be possible to put the apachesolr_has_searched() before apachesolr_search_run*(), or would that cause problems. If that could be done, this entire issue could be solved.
#6
Regarding the apachesolr_has_searched() comment, never mind. It seems that apachesolr_static_response_cache() is part of the searchExecuted() method which obviously can't be set before the search is executed. Again, yet another chicken and egg problem.
#7
I think this is as good as it is going to get. At least it paves the way to solving some of the other issues. Committed at 4cbedd2.
#8
Automatically closed -- issue fixed for 2 weeks with no activity.