Include a facet for missing

Do we still not handle missing in 7.x?

With facetapi, we've cut the "Include a facet for missing" functionality.

So, seems like we need to add this in to to Facet API I'm not sure if this is something every back-end would support, or whether this is something apachesolr would need to enable for certain facets.

I'm not clear on the functionality. Is this for cases where a term has been deleted, yet it remains in the list of available facets? Let me know if I am way of here.

No, the common use cases I had to far:

find all nodes not in any OG group.
find all nodes of a given type not in any book.

So it is for finding documents where a certain property (e.g. gid or bid) is not present.

Ah, I see. Thanks for the explanation. How was this represented in the old faceted interface?

http://drupalcode.org/project/apachesolr.git/blob/refs/heads/6.x-1.x:/ap...

~line 862:

    foreach ($response->facet_counts->facet_fields->$facet_field as $facet => $count) {
      $options = array();
      $exclude = FALSE;

      // Solr sends this back if it's empty.
      if ($facet == '_empty_') {
        $exclude = TRUE;
        $facet = '[* TO *]';
        $options['html'] = TRUE;
      }

      if ($facet_callback && function_exists($facet_callback)) {
        $facet_text = $facet_callback($facet, $options);
      }
      elseif ($exclude) {
        $facet_text = theme('placeholder', t('Missing this field'));
      }
...

The '_empty_' string I think actually comes from the json_decode(), not Solr. It's because the KEY in the facet response is an empty string for the "missing" facet (the value is the number of matches).

So there would be a few little pieces:

1. Facet API, would need to generically map the value '_empty_' to something like the theme('placeholder', t('Missing this field')); above.
2. Facet APi would have to decide how to represent this in the URL. e.g. as '_empty_' or something else.
3. Facet API needs to either offer this for all back-ends/facets, or perhaps have it as something in the facet info array?
4. Apache Solr and other back ends would need to know how to detect the magic value from the URL to generate the correct fq syntax.

As a side note, I had to fake up this behavior by patching the Zend Decoder.php we ship with apachesolr:

                foreach ($members as $key => $value) {
                    if ($key === '') {
                      // Acquia patch to resolve http://drupal.org/node/465528
                      // Mimic PHP 5.2 behavior
                      $key = '_empty_';
                    }
                    $result->$key = $value;
                }

see also: http://us2.php.net/manual/en/function.json-decode.php#95935

OK interesting, and thanks for the code. I am trying to figure out whether this is a per-facet settings. I am also trying to figure out whether this functionality is backend agnostic enough to be included in Facet API. If it is not, then Facet API obviously it needs to provide the mechanism for Apache Solr to add this functionality as a perk of using that backend. Would be curious to hear your opinions on this, because I am still wrapping my head around it.

Well, I'm pretty sure Lucene would be able to support it too, but not sure about e.g. the Elastic search front end to Lucene (not seeing any explicit support in the term facet docs), and a quick look at Sphinx I don't see it.

In term of UI, see also discussion at: https://issues.apache.org/jira/browse/SOLR-2168

For the velocity engine, Erik decided on a UI of <em>&lt;no ${field.name}&gt;</em>

Marc points out this example which I didn't read closely enough which indecates that Elastic search does have this - note "missing"
http://www.elasticsearch.org/guide/reference/api/search/facets/

"facets" : {
  "tags" : {
    "_type" : "terms",
    "missing" : 0,
    "terms" : [ {
      "term" : "foo",
      "count" : 2
    }, {
      "term" : "bar",
      "count" : 2
    }, {
      "term" : "baz",
      "count" : 1
    } ]
  }
}

related: #784282: OR faceting with facet.missing won't work

Apparently a Solr issue that you can't to facet missing filters as part of an OR facet. Just confirmed on Solr 3.1 also that it fails.

So, at least for the Solr backend, can we add validation to prevent this combination?

Here are paired patches for facetapi and apachsolr.

This make a variety of changes, including having the map callback return an array with possible #markup and #html keys.

oops - there was some cruft left in the apachesolr patch from when I was figuring out the html bit.

One more minor fix - check that 'facet missing allowed' is TRUE in apachesolr.

The patch does not apply to HEAD, will spend a little time re-rolling the patch. I think that the overall direction is good though.

Facet API patch re-rolled, changing to a task.

I tested the patch and it does seem to work. I do have a coupe of thoughts on it, though.

First, I'm not a big fan of the FacetapiAdapter::supportsFacetMissing() method. I would rather see that as an "allow missing" key in the hook_facetapi_searcher_info() definition for the adapter and then have an allowMissing() method as a wrapper around that setting so the adapter doesn't have to override it. That way the backend settings are set in one location and it reduces the amount of code in the implementing adapter.

Second, I see the need for the FacetapiAdapter::getFacetFormValidators() functionality. There is actually parallel work going on at #1158254: Allow plugins to add settings to the facet configuration forms and provide default values that would eliminate the need for this method while allowing the adapter to add a validation handlers like you would in any hook_form_alter(). The plugins are using a paradigm where they have a settingsFrom() method, so I think keeping that consistency will keep things more transparent. See the FacetapiDependencyBundle::settingsForm() for a working example.

Overall I really like the direction of the patch and (finally) see the use case and the need. Marking as postponed because I want to complete the issue posted above, and I also want to complete #1158386: Rework the UI of the facet display settings form to imrpove the site builder exprience because to me it is a bigger priority and it works in the same areas of the patch for this issue.

The other issues are resolved. Marking the task as "needs work" so we can finish this off.

refreshed patches.

Created feature branch for each project and committed these patches for further iteration.

Apache Solr: 912758-missing
Facet API: 912758-missing

Just to get the conversation back in the public, I am concerned that the map callback has to determine whether the field is #markup or #html and has to return the array depending on whether a value is empty. This is a tough issue that @pwolanin spent a lot of time on, so there may be no way around it.

I agree it's ugly, but fundamentally I think the map callback is the only place where sufficient information exists to know that a certain value is safe HTML, or is supposed to be plain text. This may vary per value, (e.g. we want to use an EM tag for the missing facet) so it's not something you can set overall per facet.

We also have to pass this information back via getMappedValue() so we can properly display the mapped value in the breadcrumb and current search block.

It's not super ugly. I am trying out a solution that is very much based on yours but with some minor differences. Instead of the facet passing "facet missing allowed", what if we passed the actual "missing value"? That way we could leave the map callbacks as they are and then add the #html if FacetapiFacetProcessor::mapValues() after the array_walk() function invocation? So something like the following:

  protected function mapValues(array $build) {
    if ($this->facet['map callback']) {

      // Maps values.
      $this->map = call_user_func($this->facet['map callback'], array_keys($build), $this->facet['map options']);
      array_walk($build, 'facetapi_replace_ids', $this->map);

      // Test for an empty value if supported.
      if (FALSE !== $this->facet['missing value'] && isset($build[$this->facet['missing value']])) {
        $build[$this->facet['missing value']]['#html'] = TRUE;
        // @todo Get field's display name ($display_name) from $this->facet['field api name']
        $build[$this->facet['missing value']]['#markup'] = theme(
          'facetapi_facet_missing', array('field_name' => $display_name)
        );
        // Adds to the map property so it shows up in breadcrumbs
        $this->map[$this->facet['missing value']] = $build[$this->facet['missing value']]['#markup'];
      }
    }
    return $build;
  }

Well, no, I don't think that's right. Only in this particular case did the back-end decide the missing facet mapped values needs HTML. It's not generically a property of the missing facet.

Also, there is no reason that any given facet value might have similar markup depending on the map callback. e.g. imagine I alter the map callback for author name from uid and it returns something like:

Posts by <em>@name</em>

An alternative that's slightly uglier, but would allow the typical map callback to return scalar values would be to use an is_array() test in facetapi_replace_ids().

Regarding #27, I think you are right in that we would be adding more ugliness.

Regarding #26, regardless of whether the missing value returns HTML or not the sanitization / rendering should have been done in theme_facetapi_facet_missing(), so passing HTML as TRUE is fine in all instances as to not double encode anything.

I see the issue with the breadcrumbs, though. $this->map would have to contain #html and #markup, and getMappedValue() would have to return an array with the same keys as well, like you have.

note that apachesolr is doing $map[$key]['#markup'] = field_filter_xss($key); for all values to pass through the same things as the filed module, hence all have to be marked with #html as TRUE.

Here's a new patch that implements the is_array() check. Also replaces the facetapi_replace_ids() callback and array_walk() with a foreach().

Also, there's no requirement that the map callback use the theme function facetapi supplies - it can do something else to format the missing value. I just added that as a convenience.

I think what you proposed works, so let's commit. Good work, and commit to the 7.x-1.x branches of both modules when ready. Deleting the topic branches.

committed these patches per IRC discussion. Adds a isset() check to the admin form to avoid notices.