When updating the alias through pathauto_taxonomy_term_update_alias() it uses taxonomy_get_tree(), which will load the entire vocabulary. When dealing with vocabularies with many terms, this will add a very heavy load, both CPU, DB and memory, and may even cause memory limit breaks. Using taxonomy_get_children() fixes this.

CommentFileSizeAuthor
#8 interdiff.txt635 bytesolli
#8 pathauto-2259527-using_taxonomy_get_children-8.patch618 bytesolli
#1 pathauto-2259527-using_taxonomy_get_children-1.patch584 bytesfrakke
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

frakke’s picture

frakke CreditAttribution: frakke commented
FileSize
pathauto-2259527-using_taxonomy_get_children-1.patch584 bytes
frakke’s picture

frakke CreditAttribution: frakke commented
Status: Active » Needs review

jvandyk queued 1: pathauto-2259527-using_taxonomy_get_children-1.patch for re-testing.

jvandyk’s picture

jvandyk CreditAttribution: jvandyk commented
Status: Needs review » Reviewed & tested by the community

This seems like a smart change for sites with many terms.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented
Parent issue: » #2305515: Plan for Pathauto 7.x-1.3 release
joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet as a volunteer commented

Would there be a regression if you have more than 2 levels as I think that only get's the direct children?

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented
Status: Reviewed & tested by the community » Needs review 
+++ b/pathauto.module
@@ -655,7 +655,7 @@ function pathauto_taxonomy_term_update_alias(stdClass $term, $op, array $options
+    foreach (taxonomy_get_children($term->tid) as $subterm) {

Why aren't we using $term->vid as the second parameter here?

olli’s picture

olli CreditAttribution: olli commented
FileSize
pathauto-2259527-using_taxonomy_get_children-8.patch618 bytes
interdiff.txt635 bytes

Re #6: I think you're right. Current code sets 'alias children' to FALSE before recursive call because it already gets all levels from taxonomy_get_tree(). Here's a patch that doesn't set 'alias children'.

Re #7: I don't know but I guess we can add that since taxonomy_get_tree() also returned terms by $term->vid.

Note that taxonomy_get_children() always loads entities while taxonomy_get_tree() didn't.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented
Status: Needs review » Closed (duplicate)
Related issues: +#2114323: Field based aliases incorrectly generated for taxonomy term children when updating top level term

Let's merge this with #2114323: Field based aliases incorrectly generated for taxonomy term children when updating top level term which also adds tests for the taxonomy children aliasing functionality.