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.

Files: 
CommentFileSizeAuthor
#23 taxonomy_vocabulary_get_names-023.patch456 byteskarschsp
PASSED: [[SimpleTest]]: [MySQL] 33,289 pass(es).
[ View ]
#20 taxonomy_vocabulary_get_names_static_cache-with-reset-1274674-20.patch1.99 KBkarschsp
PASSED: [[SimpleTest]]: [MySQL] 36,474 pass(es).
[ View ]
#15 taxonomy_vocabulary_get_names_static_cache-with-reset-1274674-15.patch1.74 KBaroq
PASSED: [[SimpleTest]]: [MySQL] 32,996 pass(es).
[ View ]
#11 taxonomy_vocabulary_get_names_static_cache-1274674-11.patch706 bytesmdupont
FAILED: [[SimpleTest]]: [MySQL] 32,988 pass(es), 2 fail(s), and 2 exception(es).
[ View ]
#8 1274674-5001044.patch707 bytespgrond
FAILED: [[SimpleTest]]: [MySQL] 32,992 pass(es), 6 fail(s), and 6 exception(es).
[ View ]
#6 1274674.003.patch678 byteskarschsp
FAILED: [[SimpleTest]]: [MySQL] 32,989 pass(es), 4 fail(s), and 2 exception(es).
[ View ]
#1 1274674.001.patch628 byteskarschsp
PASSED: [[SimpleTest]]: [MySQL] 32,983 pass(es).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new628 bytes
PASSED: [[SimpleTest]]: [MySQL] 32,983 pass(es).
[ View ]

Here's an initial stab at this.

Thanks @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:

if (...) {
  ...
}
else {
  ...
}

Assigned:Unassigned»
Status:Needs review» Needs work

Forgot to change status.

Assigned:» Unassigned

Cleaning up spammer data.

Status:Needs work» Needs review
StatusFileSize
new678 bytes
FAILED: [[SimpleTest]]: [MySQL] 32,989 pass(es), 4 fail(s), and 2 exception(es).
[ View ]

Got it (I think)...so, something like the attached?

Status:Needs review» Needs work

The last submitted patch, 1274674.003.patch, failed testing.

StatusFileSize
new707 bytes
FAILED: [[SimpleTest]]: [MySQL] 32,992 pass(es), 6 fail(s), and 6 exception(es).
[ View ]

Lets use &drupal_static for this. Reworked patch attached.

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, 1274674-5001044.patch, failed testing.

StatusFileSize
new706 bytes
FAILED: [[SimpleTest]]: [MySQL] 32,988 pass(es), 2 fail(s), and 2 exception(es).
[ View ]

Patch 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.

Ah yes. Stupid. Thanks for the fix.

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, taxonomy_vocabulary_get_names_static_cache-1274674-11.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1.74 KB
PASSED: [[SimpleTest]]: [MySQL] 32,996 pass(es).
[ View ]

added 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.

BTW, while checking the code I wondered if reset logic should be placed in TaxonomyVocabularyController->resetCache() method?

@aroq: the same logic is applied in taxonomy_terms_static_reset() so I guess your approach is the good one.

Status:Needs review» Reviewed & tested by the community

Let's get gurus to have a look.

Version:8.x-dev» 7.x-dev
Status:Reviewed & tested by the community» Needs review
Issue tags:+needs backport to D7

taxonomy_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.

Assigned:Unassigned» karschsp
StatusFileSize
new1.99 KB
PASSED: [[SimpleTest]]: [MySQL] 36,474 pass(es).
[ View ]

Cleaning up whitespace and removing a stray ".". Passes locally.

Status:Needs review» Reviewed & tested by the community

Looks good to go.

Status:Reviewed & tested by the community» Fixed

Committed and pushed to 7.x. Thanks!

Version:7.x-dev» 8.x-dev
Status:Fixed» Needs review
StatusFileSize
new456 bytes
PASSED: [[SimpleTest]]: [MySQL] 33,289 pass(es).
[ View ]

Really minor. Removing an extra "." from a comment.

Status:Needs review» Reviewed & tested by the community

The above patch simply fixes punctuation.

The extra "." has already been removed from Drupal 7, so the patch should only be committed to Drupal 8.

Status:Reviewed & tested by the community» Fixed

Committed/pushed to 8.x, thanks!

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