Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
There is a "facet api name" key in the facet definitions that Search API sets as the human readable label of the field. It should be the machine readable name of the field so it can be passed to field_info_field(). Examples of where this is used is in the FacetapiDependencyBundle class.
Comments
Comment #1
drunken monkeyAh, OK, then that explains #1215526: Add support for the "Bundle" dependency plugin, right? Should have read the issues in the other order …
Comment #2
cpliakas CreditAttribution: cpliakas commentedI actually think this is different because even if the "field api name" key isn't set, the form should still be displayed. However, this would prevent the "A content type this field is attached to must be active" option from being posted.
Comment #3
drunken monkeyIs it 'field api name' or 'facet api name'? In case of the former, this seems to be fixed.
Comment #4
drunken monkeyOr, should this only be set for Field API fields? We currently set it for all properties … This will probably mess up the dependencies, right?
Comment #5
cpliakas CreditAttribution: cpliakas commentedYou are right... "facet api name" is a typo, and doesn't exist. This is why there are bugs in my modules. The "field api name" key should only be set for entity fields, as this is the only way Facet API knows we are dealing with a Facet that has field data.
I'm up for modifying the architecture if it doesn't make sense, I just want to be super cognizant of API changes at this point since usage is starting to increase pretty heavily.
Good catch!
~Chris
Comment #6
cpliakas CreditAttribution: cpliakas commentedTo reiterate, the "field api name" should not be set for properties. Do you see a need for a "field api property" or something similar?
Comment #7
drunken monkeyAs long as the Facet API has no use for it (which doesn't seem to have), no, I don't.
OK, then we have to differentiate between the two in the hook …
I'll see what we can do there.
Comment #8
drunken monkeyComment #9
drunken monkeyComment #10
drunken monkeyOK, this is now fixed, sort of. There is a simple way of telling that via an entity metadata wrapper (provided by the Entity API). The code for getting the wrapper isn't that easy, however:
Well, it works. It's just executed every time the hook is called, for all indexed fields, which isn't really optimal. Is there any way of caching these values (I know, that was already asked) or would we have to implement that ourselves? In the latter case, we should definitely do that.
I also think we should get rid of the
$field_callbacks
variable in the hook and just set always set a few values (type, entity type, …) and letsearch_api_facet_map_callback()
directly figure out the rest.Comment #11
cpliakas CreditAttribution: cpliakas commentedRegarding caching, my internal testing of the never released 6.x-1.x branch showed that database caching was actually a slower option than iterating over all hook implementations and collecting the various arrays. Obviously this would be different with the use case posted above, but I do want to be careful of not making things slower for core Search or Apache Solr Search Integration. I'd definitely be open to discussions about implementing caching in Facet API, however.
Let's just remember there there are two hard things in computer science: cache invalidation, naming things, and off-by-one errors.
Comment #12
drunken monkeyHehe, gotta remember that one. :D
However, in the previous facet solution I also cached the data (directly on the facet objects) and didn't really have a problem there. But as we already need the information in the hook, where we have no facet objects yet, this isn't really possible here. We'd have to keep a cache of all indexed Field API fields for each index, or something like that.
But for other data, would there be a good way of storing/caching such information in the facet objects?
Ah, come to think of it – most of the code is unnecessary anyways, as we don't have to check this at all for nested fields. Changing this …