This issue arose from #336697: Optional vid argument for taxonomy_get_term_by_name().
As outlined in that issue, there may be a need for filtering the retrieval of $term (name) from $vocabularies array($A, $B, $C). The suggested solution is to invoke a loop repeatedly calling taxonomy_get_term_by_name() with the $vocabulary constraint set to $A or $B or $C. Within taxonomy_get_term_by_name() is a call to taxonomy_vocabulary_get_names(), which queries the data base for defined $vocabularies and the returns the resulting array.
To make taxonomy_vocabulary_get_names() more performant for a particular page request, the return value should be stored in a static array to minimize the data base query. An open issue is whether the function profile should be changed to include a parameter $reset = FALSE.
@xjm has suggested that this is a patch for 'novice' contributors.
Comments
Comment #1
karschsp CreditAttribution: karschsp commentedHere's an initial stab at this.
Comment #2
Lars Toomre CreditAttribution: Lars Toomre commentedThanks @karschap for your initial stab. I note that you are attempting to change taxonomy_get_term_by_name(). That function is being worked on in #336697: Optional vid argument for taxonomy_get_term_by_name(). The purpose of this issue is to add a static to taxonomy_vocabulary_get_names() which will be called by that modified taxonomy_get_term_by_name().
On your patch attempt, there a couple of other things that would need to be changed. First, you need set $term before returning; otherwise the static storage has no effect. Second, Drupal coding standards call for formatting else statements like:
Comment #4
Lars Toomre CreditAttribution: Lars Toomre commentedForgot to change status.
Comment #5
Lars Toomre CreditAttribution: Lars Toomre commentedCleaning up spammer data.
Comment #6
karschsp CreditAttribution: karschsp commentedGot it (I think)...so, something like the attached?
Comment #8
pgrond CreditAttribution: pgrond commentedLets use &drupal_static for this. Reworked patch attached.
Comment #9
pgrond CreditAttribution: pgrond commentedComment #11
mdupontPatch in #8 is a step in the right direction but still have some issues:
- indentation is incorrect (Drupal coding standards use 2 spaces)
- drupal_static() is called with a default value, so $names will always be set and the underlying conditional block of code will never be run
Here is a minimal patch that fixes these.
Comment #12
pgrond CreditAttribution: pgrond commentedAh yes. Stupid. Thanks for the fix.
Comment #13
arithmetric CreditAttribution: arithmetric commentedComment #15
aroq CreditAttribution: aroq commentedadded static reset function call for "taxonomy_vocabulary_get_names" (in analogy to taxonomy_terms_static_reset() ) in taxonomy_vocabulary_save($vocabulary) and taxonomy_vocabulary_delete($vid) functions to have a cache reset after changing vocabularues.
Comment #16
aroq CreditAttribution: aroq commentedBTW, while checking the code I wondered if reset logic should be placed in TaxonomyVocabularyController->resetCache() method?
Comment #17
mdupont@aroq: the same logic is applied in taxonomy_terms_static_reset() so I guess your approach is the good one.
Comment #18
mdupontLet's get gurus to have a look.
Comment #19
catchtaxonomy_vocabulary_get_names() is a hack because we're using the vocabulary (which is an entity) as a bundle, and you get infinite recursion in hook_entity_info() if you don't have the workaround function.
However while I'd like to see us change vocabularies to configuration in Drupal 8, while they're entities we need to encourage use of this function when you only need a list of all bundle names or mapping vid to name, so adding the static cache here makes sense.
Patch failures in here show it is already tested and the static reset is consistent with terms (which also have meta functions on top of loading).
So I've committed this to 8.x, moving back to 7.x for webchick to consider since there's the same issue there.
Comment #20
karschsp CreditAttribution: karschsp commentedCleaning up whitespace and removing a stray ".". Passes locally.
Comment #21
Lars Toomre CreditAttribution: Lars Toomre commentedLooks good to go.
Comment #22
webchickCommitted and pushed to 7.x. Thanks!
Comment #23
karschsp CreditAttribution: karschsp commentedReally minor. Removing an extra "." from a comment.
Comment #24
Devin Carlson CreditAttribution: Devin Carlson commentedThe above patch simply fixes punctuation.
The extra "." has already been removed from Drupal 7, so the patch should only be committed to Drupal 8.
Comment #25
catchCommitted/pushed to 8.x, thanks!