Problem:

Invalid argument supplied for foreach() service.inc:1638

SearchApiQueryFilter::getTags() can sometimes return NULL. That happens with Search API Sqaved Searches where SearchApiQueryFilter objects are serialized into the database. Old serialized objects have NULL on their tags property.

The method_exists() call also does not make sense in createFilterQueries() because the type hint to SearchApiQueryFilterInterface already enforces that the getTags() method is always there.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

klausi created an issue. See original summary.

klausi’s picture

Status: Active » Needs review
FileSize
625 bytes

Patch.

drunken monkey’s picture

Project: Search API Solr » Search API
Component: Code » Framework
FileSize
411 bytes

Thanks for reporting this issue! Seems we forgot to think about old serialized queries/filters when implementing this. Easy to miss, I guess.
However, instead of checking for an illegal return value in the Solr backend, I think we should just eliminate the root cause: the getTags() says it'll always return an array, so it should. Patch attached.

Regading removal of method_exists(): that's still needed in case someone updates the Solr module but has an outdated Search API module. "Don't do that!" is of course the right answer, but I think we both know that it'll still happen.

klausi’s picture

Status: Needs review » Needs work

Makes sense!

+++ b/includes/query.inc
@@ -1071,7 +1071,8 @@ public function hasTag($tag) {
   public function &getTags() {
-    return $this->tags;
+    // Tags can sometimes be NULL for old serialized query filter objects.
+    return $this->tags ? $this->tags : array();

So the method returns by reference, but you return an array which is not a reference to $this->tags.

You should set $this->tags to the empty array first and then return that.

drunken monkey’s picture

Status: Needs work » Needs review
FileSize
461 bytes
425 bytes

Correct, of course – thanks for catching that!
Otherwise, would you say this is RTBC?

  • drunken monkey committed 7891a93 on 7.x-1.x
    Issue #2837745 by drunken monkey, klausi: Fixed NULL tags on old...
drunken monkey’s picture

Status: Needs review » Fixed

Should be fine, so: committed.
Thanks again for reporting!

Status: Fixed » Closed (fixed)

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