When I revert facetapi feature components or clear the cache I get these warnings:

array_search() expects parameter 2 to be array, null given in facetapi.block.inc:44

This problem originates in facetapi_get_delta_map() which does not validate the results it gets back from the cache:

<?php
function facetapi_get_delta_map() {
 
$map = &drupal_static(__FUNCTION__);
  if (
NULL === $map) {
    if (
$data = cache_get('facetapi:delta_map')) {
     
$map = $data->data;
    }
    else {
     
// Build cache data...
   
}
  }
  return
$map;
}
?>

In my case $data->data is NULL and this empty value is returned, causing the subsequent array_search() warning.

For reference, this is what is currently in my delta_map persistent cache:

[pieter@archlinux docroot (13657 *)]$ drush eval "var_dump(cache_get('facetapi:delta_map'));"
class stdClass#19 (5) {
  public $cid =>
  string(18) "facetapi:delta_map"
  public $data =>
  NULL
  public $created =>
  string(10) "1359378127"
  public $expire =>
  string(2) "-1"
  public $serialized =>
  string(1) "1"
}
Files: 
CommentFileSizeAuthor
#9 facetapi-1900974-9.patch750 bytespjcdawkins
PASSED: [[SimpleTest]]: [MySQL] 566 pass(es).
[ View ]
#1 1900974-1-facetapi-rebuild_delta_map_cache.patch808 bytespfrenssen
PASSED: [[SimpleTest]]: [MySQL] 566 pass(es).
[ View ]

Comments

pfrenssen’s picture

Status:Active» Needs review
StatusFileSize
new808 bytes
PASSED: [[SimpleTest]]: [MySQL] 566 pass(es).
[ View ]

Here's a patch, it applies both to 7.x-1.x and 7.x-2.x.

kenorb’s picture

Status:Needs review» Reviewed & tested by the community
nicksanta’s picture

I have applied this patch and it works perfectly.

gmclelland’s picture

The patch in #1 solved the same problem for me. Thanks!

kenorb’s picture

Status:Reviewed & tested by the community» Patch (to be ported)
cpliakas’s picture

Status:Patch (to be ported)» Fixed

Thanks guys! Committed to all versions of Facet API.

pfrenssen, congrats on becoming the 36th code contributor to Facet API!

Thanks,
Chris

pfrenssen’s picture

Yay! Thanks :-)

Status:Fixed» Closed (fixed)

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

pjcdawkins’s picture

Status:Closed (fixed)» Needs review
StatusFileSize
new750 bytes
PASSED: [[SimpleTest]]: [MySQL] 566 pass(es).
[ View ]

I still get this error, with version 1.3 (after the above patch was committed).

I think we need something like the attached.

cpliakas’s picture

Status:Needs review» Needs work

Thanks for the patch, but if possible I'd like to maintain the logic as there is usually a reason why things are coded the way they are. I am definitely open to change it, but there needs to be a reason why.

I think I see the issue, though. When you are assigning a value in a condition with two clauses, you need to wrap the assignment in parens. For example:

<?php
if (($data = cache_get('facetapi:delta_map')) && !empty($data->data)) {
?>

Notice the parens around $data = cache_get('facetapi:delta_map'). If possible I'd like to start there and then explore other techniques if necessary.

Thanks for the contributions,
Chris

pjcdawkins’s picture

Status:Needs work» Needs review

Yes, I can see that the cache_get line is wrong in the current code, because the variable assignment isn't wrapped in (). But that's not relevant.

We just need to be certain that facetapi_get_delta_map() always returns an array.

In the function at the moment:

<?php
     
// $map is NULL

     
foreach (facetapi_get_realm_info() as $realm_name => $realm_info) {

       
// [ ... various nested loops]

       
$map[facetapi_hash_delta($delta)] = $delta;
      }

     
// $map might now be an array, or it might be NULL
?>

If any of the nested loops don't run, then $map never gets initialized as an array.

If we add $map = array(); then $map, and its cached version, will always be an array.

We can remove the && !empty($data->data) because an empty array might be a valid result (and it was done incorrectly anyway).

So I still think my patch in #9 is valid.

cpliakas’s picture

pjcdawkins,

Thanks for the explanation. The reasoning makes sense, and I will take a closer look at the patch.

Thanks,
Chris

Yaron Tal’s picture

Issue summary:View changes
Status:Needs review» Reviewed & tested by the community

Just adding the $map = array(); part would already solve the warning.
Getting the cache logic working is also needed because cache is not working atm.
cache_get() will return a valid cache object or FALSE, so rewriting it as done in #9 should fix this.
Tested and warning is fixed and caching working.

marcelovani’s picture

Thats right, the patch #9 works. Thanks.

EC-ENTR’s picture

The problem is that the hook facetapi_facetapi_realm_info() is coded in facetapi.facetapi.inc file which is only loaded when a search box is present on the page. This hook goes into Drupal cache when cache is cleared.

The problem is that if you clear Drupal cache from a page where this file is loaded, the hook is present and thus saved into cache. If not, it will be absent from the cache. Thus facetapi_get_delta_map() will return nothing.

The true solution is not to prevent a PHP error when that happens, but to prevent the hook from "disappearing" in the first place.

The easy solution is to move all hooks from the include file to the module file. Another solution is to use a module_load_include() in the module to always load the include file when the module is enabled.

You may NOT use this hook in order to load the include file:

hook_hook_info

  • cpliakas committed 3a2287b on 7.x-1.x authored by pjcdawkins
    Issue #1900974 by pjcdawkins, marcelovani: facetapi_get_block_info()...

  • cpliakas committed 0466e00 on authored by pjcdawkins
    Issue #1900974 by pjcdawkins, marcelovani: facetapi_get_block_info()...
cpliakas’s picture

Status:Reviewed & tested by the community» Fixed

Status:Fixed» Closed (fixed)

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