The title says it all. See the attached screenshot. This is probably because the current_search_check_visibility() function doesn't call FacetapiAdapter::searchExecuted() to see if the search was in fact executed.

current_search-empty-search-error.jpg

CommentFileSizeAuthor
#22 facetapi-1379166-22.patch11.55 KBcpliakas
#18 apachesolr-1379166-18.patch4.57 KBnick_vh
#17 apachesolr-1379166-17.patch2.72 KBcpliakas
#14 apachesolr-1379166-14.patch2.7 KBcpliakas
#7 facetapi-1379166-7.jpg71.3 KBcpliakas
#3 Search | Smartphone compare.jpg14.89 KBnick_vh
#2 facetapi-1379166-2.patch846 bytescpliakas
current_search-empty-search-error.jpg41.98 KBcpliakas

Comments

nick_vh’s picture

nick_vh
he/him
Location Ghent
commented

Can you also mention what version of solr or search api? Just tried to reproduce and this is not happening to me. Possibly a regression from the new current search block? I didn't test it with the latest dev version. Tried with latest dev of apachesolr and latest dev of facetapi (not the current search branch) and could not reproduce.

Maybe little hint on how you got in to this state?

cpliakas’s picture

cpliakas commented
Status: Active » Needs review
StatusFileSize
new facetapi-1379166-2.patch846 bytes

This is the 7.x-1.x branch of Facet API and the 7.x-1.x branch Apache Solr Search Integration (all the latest code). It happens on a search page when you execute no search using the default Current Search Block configuration as the Current Search block (although it could be reproduced with any current search block configuration). This is 100% related to the new Current Search Blocks module and from what I can tell doesn't have anything to do with Search API or Apache Solr Search Integration. The attached patch resolves the issue by using the adapter to ensure that the search module actually executed a search.

Thanks for testing,
Chris

nick_vh’s picture

nick_vh
he/him
Location Ghent
commented
Status: Needs review » Needs work
StatusFileSize
new Search | Smartphone compare.jpg14.89 KB

Doesn't seem to fix the issue. I was able to reproduce and this patch did not fix it.

I am working with the default provided current search block. I did clear the cache and I was using apachesolr search with a default environment and the default search page.

cpliakas’s picture

cpliakas commented

Which check is succeeding? Patch works perfectly for me, so I will need some help to reproduce.

cpliakas’s picture

cpliakas commented

Never mind... ran a git pull and I can replicate now.

cpliakas’s picture

cpliakas commented

Never mind again. Has nothing to do with the software version. It has to do with the "behavior on empty search" configuration. As the image shows, the FacetapiAdapter::searchExecuted() method returns TRUE because an empty search is executed to show enabled facets' blocks under the search box.

This is a bit of an issue. To me, we could either make it so that Apache Solr does not set the "searchExecuted" flag on these types of empty searches, or we could look over all realms in Facet API and check if one of them suppressed the output of facets. To me the Apache Solr modification would probably be the best approach, but I would be curious to hear your thoughts.

Great job testing this issue,
Chris

cpliakas’s picture

cpliakas commented
StatusFileSize
new facetapi-1379166-7.jpg71.3 KB

Image supporting #6.

facetapi-1379166-7.jpg

nick_vh’s picture

nick_vh
he/him
Location Ghent
commented

Hi Cpliakas,

I'll toy around with this and see what I can work with. Give me until tomorrow or so :-)

cpliakas’s picture

cpliakas commented

Take as much time as you need. P.S., finding that setting was unbelievably awesome with the new Apache Solr Search Integration UI!

nick_vh’s picture

nick_vh
he/him
Location Ghent
commented

So I guess the problem is in the adapter specification of apachesolr search

function apachesolr_do_query(DrupalSolrQueryInterface $current_query, $page = 0) {
  if (!is_object($current_query)) {
    throw new Exception(t('NULL query object in function apachesolr_do_query()'));
  }
  // Allow modules to alter the query prior to statically caching it.
  // This can e.g. be used to add available sorts.
  $searcher = $current_query->getSearcher();

  if (module_exists('facetapi')) {
    // Gets enabled facets, adds filter queries to $params.
    $adapter = facetapi_adapter_load($searcher);
    if ($adapter) {
      // Realm could be added but we want all the facets
      $adapter->addActiveFilters($current_query);
    }
  }

You suggest we should explicitly tell Facetapi there is no current search? Sounds a bit odd because it used to work properly? Is there no way you can check if the current search is empty you can hide the block or not show anything? I mean, the logic is a bit weird that I have to tell facetapi I did not execute a search while I actually did execute a search?
How can apachesolr tell facetapi it will execute an empty search? Is it as simple as $adapter->searchExecuted(false); ?

The function for an empty search is as follows (separate for speed optimizations)

/**
 * Execute a search with zero results rows so as to populate facets.
 */
function apachesolr_search_run_empty($name, array $params = array(), $base_path = '', $solr) {
  $query = apachesolr_drupal_query($name, $params, '', $base_path, $solr);
  $query->addParam('rows', '0');
  $solr_id = $query->solr('getId');
  list($final_query, $response) = apachesolr_do_query($query, 0);
  apachesolr_has_searched($solr_id, TRUE);
}
nick_vh’s picture

nick_vh
he/him
Location Ghent
commented

Ah, I see there is also our own implemented function for searchExecuted(). Continuing on my investigation!

/**
   * Returns a boolean flagging whether $this->_searcher executed a search.
   */
  public function searchExecuted() {
    // Initial check - has ANY solr query run in our environment.
    $env_id = $this->info['instance'];
    $this_has_searched = apachesolr_has_searched($env_id);
    // Secondary check - do we have results for this searcher?
    $this_has_searched = $this_has_searched && apachesolr_static_response_cache($this->getSearcher());
    return $this_has_searched;
  }

I am still not very clear how apachesolr should solve this. If we say that search was not executed, there won't be any facets even though we want them to appear (we just do not want the current block, since there is no facet or search entered yet). Maybe we should have a way to surpress only the current search blocks?
My thought is that facetapi should hide the block if nothing is inside the facet block actually

cpliakas’s picture

cpliakas commented

The original idea of the FacetapiAdapter::searchExecuted() method is that a search was actually executed by the user, not executed programmatically. For example with the core search, this can be done simply by checking whether the user submitted search keywords or not. This logic may or may not be flawed. An example of where this logic might not work is the taxonomy search page where an empty query is executed to list nodes containing the various terms. We would probably want the ability to have facets on this page, and maybe even a current search block.

To me the empty query in this specific instance is unique and should probably use FacetapiAdapter::searchExecuted(FALSE). A search query isn't really being executed from a user's standpoint, just to get the facet counts to display below the search box if no query was executed. Like you said, maybe this will prevent facets from being displayed in which case we might have to take approach 2 in mentioned in #6.

nick_vh’s picture

nick_vh
he/him
Location Ghent
commented

I think we are going a bit out of scope here.

we could look over all realms in Facet API and check if one of them suppressed the output of facets

The only thing we want to resolve is that the current search block does not show when the empty query was sent. How can apachesolr make this happen without suppressing all the facet blocks?

I suppose #1382092: Allow facets to be configured per search page as opposed to per environment was openend because of this issue?

cpliakas’s picture

cpliakas commented
StatusFileSize
new apachesolr-1379166-14.patch2.7 KB

I think we are going a bit out of scope here.

Agreed. The attached patch is a proof-of-concept against Apache Solr that coupled with the patch in #2 seems to work for me. It takes the approach of storing the status of the searchExecuted() method in a class property and providing a setSearchExecuted method that developers can use to manually override the status. It has the additional performance benefit of only calculating whether a search was executed once, because there are multiple calls to this method per page.

cpliakas’s picture

cpliakas commented
Status: Needs work » Needs review

Please excuse typos in comments.

cpliakas’s picture

cpliakas commented
Project: Facet API » Apache Solr Search
Component: Custom Search Blocks » Code
Assigned: Unassigned » cpliakas
Issue tags: +Facet API integration

As per discussions with Nick_vh on IRC, committing patch in #2 to Facet API and moving this issue over to Apache Solr. Assigning to myself for testing the patch.

Patch committed to Facet API at http://drupalcode.org/project/facetapi.git/commit/795d1a6.

cpliakas’s picture

cpliakas commented
StatusFileSize
new apachesolr-1379166-17.patch2.72 KB

Revised patch with same functionality, fixed bad grammar in comments. Seems to work for the use cases illustrated above.

nick_vh’s picture

nick_vh
he/him
Location Ghent
commented
StatusFileSize
new apachesolr-1379166-18.patch4.57 KB

I adjusted the patch so it becomes more clear as to when it is being called. I am still not able to actually browse facets. They just disappear when I set the searchExecuted to False. Only the current search block should disappear. Everything does work when I set them to "Show facets under search block"

nick_vh’s picture

nick_vh
he/him
Location Ghent
commented
Status: Needs review » Needs work

I should probably restore the comments also. But that is for a follow-up patch

cpliakas’s picture

cpliakas commented

I understand the overall goal, its just that my logic is flawed and my testing is bad :-(. I thought about this on the way to work, and maybe the approach I proposed is getting too complex. What about moving this back to Facet API and adding a setting to the block configuration "Display on empty search" or something of that nature that defaults to FALSE, and then I can add a condition to the current_search_check_visibility() function that hides the block if the setting is unchecked and no search keys were passed. Seems like that might be a simpler approach, and it is also configurable so we aren't making any assumptions.

cpliakas’s picture

cpliakas commented
Project: Apache Solr Search » Facet API

As per discussions with Nick on Skype, moving back to Facet API to be tackled there by taking the approach mentioned in #20 above.

cpliakas’s picture

cpliakas commented
Component: Code » Custom Search Blocks
Status: Needs work » Needs review
StatusFileSize
new facetapi-1379166-22.patch11.55 KB

The attached patch adds the setting. I had to reconfigure the structure of the settings, so you have to run update.php to convert the settings to the new structure. Otherwise you wil get nasty notice errors.

cpliakas’s picture

cpliakas commented
Status: Needs review » Fixed

Committed at http://drupalcode.org/project/facetapi.git/commit/61cb8bc.

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

Anonymous’s picture

(not verified) commented
Issue summary: View changes

Updated issue summary.