In the Solr backend we skipped all query conditions that had an "empty" value. Now we changed the filter query builder in #2826146: Be compatible with Search API DB regarding empty and non-existing values to satisfy the new tests defined in #2767609: Add backend tests for empty value conditions. "Empty" values are now "correctly" handled in the Solr backend.

But now the views integration tests fail. But from my point of view it's correct that they fail because the Search API views integration seems to create query conditions that aren't 100% correct. While the DB backend seems to handle them "gracefully", Solr fails. But I think that's a bug.

I attached an example as screenshot:

"Contains all of this words" with an empty search string results in a Search API query condition like:

field => "name"
value => ""
operator => "="

For whatever reason the DB backend seems to skip that hard condition in this test and returns all results while Solr returns no results.
I think no results is correct because there is no content where the field "name" has an empty value.

If the expected result is to return all results if no keywords are provided, Search API should send different queries to the backends if a keyword is present or not.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mkalkbrenner created an issue. See original summary.

mkalkbrenner’s picture

Issue summary: View changes
mkalkbrenner’s picture

Issue summary: View changes
drunken monkey’s picture

Issue tags: +release target

I haven't tested it, but you're right, an empty string should be ignored by the Views integration, not be added to the Search API query.
The DB backend probably just ignores this because of its "Minimum word length" setting – since a string of length 0 is surely below that, the backend cannot check whether that string is present. So the with the DB backend those tests just seem to pass by accident.

In addition to fixing this, I guess we might also want to add some assertions for the query that was constructed, to ensure such things don't happen again in the future. (Just having tests in the Solr module doesn't really cut it for bugs in the Search API module.)

mkalkbrenner’s picture

From IRC by drunken_monkey:
mkalkbrenner: The bug has to be in \Drupal\search_api\Plugin\views\filter\SearchApiFulltext.
mkalkbrenner: Apparently, query() has to check whether the given value is empty.
mkalkbrenner: I thought Views did that automatically, but apparently not.
mkalkbrenner: Probably take a look into Views' own filter plugins to see where they normally weed out empty strings, and then implement it in that class, too. Maybe acceptExposedInput()?

drunken monkey’s picture

Title: Conversion of views filters into Search API queries might be erroneous » Views tests add empty-value name filter to searches
Component: Views integration » Tests
Status: Active » Needs review
FileSize
646 bytes

Found the problem, just a bug in the Views test.

mkalkbrenner’s picture

Status: Needs review » Reviewed & tested by the community

Thanks. That fixes the test on the Solr backend :-)

  • drunken monkey committed 7431208 on 8.x-1.x
    Issue #2826308 by drunken monkey, mkalkbrenner: Fixed unwanted filter in...
drunken monkey’s picture

Status: Reviewed & tested by the community » Fixed

Good to hear, thanks for trying it out!
Committed.

Status: Fixed » Closed (fixed)

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