Extend facetapi_map_bundle() to all entity types

Thanks for posting. On a mini-vacation, but will review early next week.

Thanks for the contribution!
~Chris

Hi scor.

I really like the direction of the patch and it definitely works as-is, however I am marking as "needs work" because I would like to make use of the "map options" key in the facet definition if possible so we can explicitly pass which entities the facet contains bundle information for. This would take enough of the guess work out of things IMO and not force the backends to change their field naming schemes. The resulting facet definition would look similar to the code below:

$facet['bundle'] = array(
  ...,
  'map callback' => 'facetapi_map_bundle',
  'map options' => array('node'),
  ...,
);

We also want to maintain backwards compatibility for the Apache Solr Search Integration and Faceted Navigation for Search modules (I don't think Search API uses this callback), so we should add a second parameter to the facetapi_map_bundle() function that defaults to array('node'). Would be curious to get your thoughts on this approach.

Thanks for the work on this,
Chris

Bumping this because I just ran into this problem as well. Hopefully I'll find some time to look into this

Thanks Nick! I think that scor's patch is really close, and adding the suggestions in #3 should get us to where we need to be. Of course if you feel another direction is required, I would love to hear your thoughts.

Thanks,
Chris

Changing to a task.

What would be the conflict if we do not use the parameters?
I don't really feel we should add more complexity since this callback just returns names of bundles.

Conflicting bundle names. If you aren't passing the entity you are getting bundle names for, then the machine readable bundle names have to be unique across all entities. To me this would be a huge WTF and an arbitrary limitation. I personally don't think that passing the entity through the options is adding too much complexity.

But then you would disable the ease of use when adding an entity type that is not node? It might make more sense to key the values differently?
Suppose we do add the array argument. Can we then pass an array without entity types to get a list of all of them? Or should the module make a list of all entity types and pass it to the callback?

I am still not understanding the difficulty or complexity here. If you are mapping a bundle in a non-node entity, just pass the entity as a map option, for example 'map options' => array('myentity') or 'map options' => array('entity' => array('myentity')) if the function should support multiple entities at one time. I would consider that to be pretty trivial. Looking at apachesolr_default_node_facet_info() you should know what entity the bundle belongs to so it would be really easy to pass it via the mapping options. To me requiring an extra line of code is worth preventing arbitrary name conflicts and mapping inconsistencies.

After doing a little bit of research, it seems all of Apache Solr Search Integration's custom mapping callbacks are already leveraging map options pretty heavily, so if anything this would be maintaing the status quo.

Attached is a patch that illustrates the idea. I couldn't set the default $options in the function definition because the map options are initialized generically as an empty array in the facetapi_get_facet_info() function. I also removed the array_keys() call in the foreach loop because it is slower than just iterating over the array.

Usage of the map callback for non-node bundles is illustrated below:

$facet['bundle'] = array(
  ...,
  'map callback' => 'facetapi_map_bundle',
  'map options' => array('entities' => array('my-entity-1', 'my-entity-2'),
  ...,
);

This only introduces the conflicting bundle machine names in the edge case where data from multiple entities is stored in the same index.

Ugh. Ignore patch in #12. Forgot to re-diff my changes.

Ok, I'll admit. This is not too shabby ;-)
You can go ahead with this one, at least then there is support for multiple entities

Thanks!

Nick_vh, thanks for your feedback and flexibility. Marking as RTBC to give others a last chance to veto.

~Chris

Committed at http://drupalcode.org/project/facetapi.git/commit/67758f1.