From 7c1e3a1bb4991450c7ae661ca86317a8e4e9d3ac Mon Sep 17 00:00:00 2001
From: catch <catch@35733.no-reply.drupal.org>
Date: Wed, 20 Dec 2023 22:11:38 +0000
Subject: [PATCH] Issue #2183565 by plopesc, alexpott, jcnventura, DuaelFr,
melvinlouwerse, grndlvl, cesarmiquel, Sweetchuck, yogeshmpawar, BetoAveiga,
Sivaji_Ganesh_Jojodae, drugan, marvil07, smustgrave, poker10, catch, bbrala:
Avoid loading all terms on the taxonomy overview form
---
.../taxonomy/src/Form/OverviewTerms.php | 61 ++++++++++++-------
.../taxonomy_test/taxonomy_test.module | 12 ++++
.../src/Functional/TaxonomyTermPagerTest.php | 45 +++++++++++++-
3 files changed, 96 insertions(+), 22 deletions(-)
diff --git a/core/modules/taxonomy/src/Form/OverviewTerms.php b/core/modules/taxonomy/src/Form/OverviewTerms.php
index 4e7b1f9c168d..62fc76a6b61b 100644
--- a/core/modules/taxonomy/src/Form/OverviewTerms.php
+++ b/core/modules/taxonomy/src/Form/OverviewTerms.php
@@ -158,7 +158,9 @@ public function buildForm(array $form, FormStateInterface $form_state, Vocabular
$delta = 0;
$term_deltas = [];
- $tree = $this->storageController->loadTree($taxonomy_vocabulary->id(), 0, NULL, TRUE);
+ // Terms are not loaded to avoid excessive memory consumption for large
+ // vocabularies. Needed terms are loaded explicitly afterward.
+ $tree = $this->storageController->loadTree($taxonomy_vocabulary->id(), 0, NULL, FALSE);
$tree_index = 0;
$complete_tree = NULL;
do {
@@ -179,8 +181,8 @@ public function buildForm(array $form, FormStateInterface $form_state, Vocabular
}
// Do not let a term start the page that is not at the root.
- $term = $tree[$tree_index];
- if (isset($term->depth) && ($term->depth > 0) && !isset($back_step)) {
+ $raw_term = $tree[$tree_index];
+ if (isset($raw_term->depth) && ($raw_term->depth > 0) && !isset($back_step)) {
$back_step = 0;
while ($parent_term = $tree[--$tree_index]) {
$before_entries--;
@@ -195,7 +197,7 @@ public function buildForm(array $form, FormStateInterface $form_state, Vocabular
$back_step = $back_step ?? 0;
// Continue rendering the tree until we reach the a new root item.
- if ($page_entries >= $page_increment + $back_step + 1 && $term->depth == 0 && $root_entries > 1) {
+ if ($page_entries >= $page_increment + $back_step + 1 && $raw_term->depth == 0 && $root_entries > 1) {
$complete_tree = TRUE;
// This new item at the root level is the first item on the next page.
$after_entries++;
@@ -208,20 +210,30 @@ public function buildForm(array $form, FormStateInterface $form_state, Vocabular
// Finally, if we've gotten down this far, we're rendering a term on this
// page.
$page_entries++;
- $term_deltas[$term->id()] = isset($term_deltas[$term->id()]) ? $term_deltas[$term->id()] + 1 : 0;
- $key = 'tid:' . $term->id() . ':' . $term_deltas[$term->id()];
+ $term_deltas[$raw_term->tid] = isset($term_deltas[$raw_term->tid]) ? $term_deltas[$raw_term->tid] + 1 : 0;
+ $key = 'tid:' . $raw_term->tid . ':' . $term_deltas[$raw_term->tid];
// Keep track of the first term displayed on this page.
if ($page_entries == 1) {
- $form['#first_tid'] = $term->id();
+ $form['#first_tid'] = $raw_term->tid;
}
// Keep a variable to make sure at least 2 root elements are displayed.
- if ($term->parents[0] == 0) {
+ if ($raw_term->parents[0] == 0) {
$root_entries++;
}
- $current_page[$key] = $term;
+ $current_page[$key] = $raw_term;
} while (isset($tree[++$tree_index]));
+ // Load all the terms we're going to display and set the weight and parents
+ // from the tree.
+ $terms = $this->storageController->loadMultiple(array_keys($term_deltas));
+ $current_page = array_map(function ($raw_term) use ($terms) {
+ $term = $terms[$raw_term->tid];
+ $term->depth = $raw_term->depth;
+ $term->parents = $raw_term->parents;
+ return $term;
+ }, $current_page);
+
// Because we didn't use a pager query, set the necessary pager variables.
$total_entries = $before_entries + $page_entries + $after_entries;
$this->pagerManager->createPager($total_entries, $page_increment);
@@ -518,7 +530,9 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
$vocabulary = $form_state->get(['taxonomy', 'vocabulary']);
$changed_terms = [];
- $tree = $this->storageController->loadTree($vocabulary->id(), 0, NULL, TRUE);
+ // Terms are not loaded to avoid excessive memory consumption for large
+ // vocabularies. Needed terms are loaded explicitly afterward.
+ $tree = $this->storageController->loadTree($vocabulary->id(), 0, NULL, FALSE);
if (empty($tree)) {
return;
@@ -526,14 +540,14 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
// Build a list of all terms that need to be updated on previous pages.
$weight = 0;
- $term = $tree[0];
- while ($term->id() != $form['#first_tid']) {
- if ($term->parents[0] == 0 && $term->getWeight() != $weight) {
- $term->setWeight($weight);
- $changed_terms[$term->id()] = $term;
+ $raw_term = $tree[0];
+ $term_weights = [];
+ while ($raw_term->tid != $form['#first_tid']) {
+ if ($raw_term->parents[0] == 0 && $raw_term->weight != $weight) {
+ $term_weights[$raw_term->tid] = $weight;
}
$weight++;
- $term = $tree[$weight];
+ $raw_term = $tree[$weight];
}
// Renumber the current page weights and assign any new parents.
@@ -565,14 +579,19 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
// Build a list of all terms that need to be updated on following pages.
for ($weight; $weight < count($tree); $weight++) {
- $term = $tree[$weight];
- if ($term->parents[0] == 0 && $term->getWeight() != $weight) {
- $term->parent->target_id = $term->parents[0];
- $term->setWeight($weight);
- $changed_terms[$term->id()] = $term;
+ $raw_term = $tree[$weight];
+ if ($raw_term->parents[0] == 0 && $raw_term->weight != $weight) {
+ $term_weights[$raw_term->tid] = $weight;
}
}
+ // Load all the items that need to be updated at once.
+ $terms = $this->storageController->loadMultiple(array_keys($term_weights));
+ foreach ($terms as $term) {
+ $term->setWeight($term_weights[$term->id()]);
+ $changed_terms[$term->id()] = $term;
+ }
+
if (!empty($changed_terms)) {
$pending_term_ids = $this->storageController->getTermIdsWithPendingRevisions();
diff --git a/core/modules/taxonomy/tests/modules/taxonomy_test/taxonomy_test.module b/core/modules/taxonomy/tests/modules/taxonomy_test/taxonomy_test.module
index e6bf91b46947..7735ea345bff 100644
--- a/core/modules/taxonomy/tests/modules/taxonomy_test/taxonomy_test.module
+++ b/core/modules/taxonomy/tests/modules/taxonomy_test/taxonomy_test.module
@@ -46,3 +46,15 @@ function taxonomy_test_form_taxonomy_term_form_alter(&$form, FormStateInterface
$form['relations']['parent']['#disabled'] = TRUE;
}
}
+
+/**
+ * Implements hook_ENTITY_TYPE_load() for the taxonomy term.
+ */
+function taxonomy_test_taxonomy_term_load($entities) {
+ $value = \Drupal::state()->get(__FUNCTION__);
+ // Only record loaded terms is the test has set this to an empty array.
+ if (is_array($value)) {
+ $value = array_merge($value, array_keys($entities));
+ \Drupal::state()->set(__FUNCTION__, array_unique($value));
+ }
+}
diff --git a/core/modules/taxonomy/tests/src/Functional/TaxonomyTermPagerTest.php b/core/modules/taxonomy/tests/src/Functional/TaxonomyTermPagerTest.php
index 5233169832b2..e46dc8ceff68 100644
--- a/core/modules/taxonomy/tests/src/Functional/TaxonomyTermPagerTest.php
+++ b/core/modules/taxonomy/tests/src/Functional/TaxonomyTermPagerTest.php
@@ -14,7 +14,7 @@ class TaxonomyTermPagerTest extends TaxonomyTestBase {
*
* @var array
*/
- protected static $modules = ['taxonomy'];
+ protected static $modules = ['taxonomy', 'taxonomy_test'];
/**
* {@inheritdoc}
@@ -73,4 +73,47 @@ public function testTaxonomyTermOverviewPager() {
$this->assertSession()->responseMatches('|<nav class="pager" [^>]*>|');
}
+ /**
+ * Tests that overview page only loads the necessary terms.
+ */
+ public function testTaxonomyTermOverviewTermLoad() {
+ // Set limit to 3 terms per page.
+ $this->config('taxonomy.settings')
+ ->set('terms_per_page_admin', '3')
+ ->save();
+
+ $state = $this->container->get('state');
+
+ // Create 5 terms.
+ for ($x = 0; $x <= 10; $x++) {
+ $this->createTerm($this->vocabulary, ['weight' => $x]);
+ }
+
+ // Check the overview page.
+ $state->set('taxonomy_test_taxonomy_term_load', []);
+ $this->drupalGet('admin/structure/taxonomy/manage/' . $this->vocabulary->id() . '/overview');
+ $loaded_terms = $state->get('taxonomy_test_taxonomy_term_load');
+ $this->assertCount(4, $loaded_terms);
+
+ // Check the overview page for submit callback.
+ $state->set('taxonomy_test_taxonomy_term_load', []);
+ $this->submitForm([], 'Save');
+ $loaded_terms = $state->get('taxonomy_test_taxonomy_term_load');
+ $this->assertCount(4, $loaded_terms);
+
+ $this->drupalGet('admin/structure/taxonomy/manage/' . $this->vocabulary->id() . '/overview', ['query' => ['page' => 2]]);
+ $state->set('taxonomy_test_taxonomy_term_load', []);
+ $this->submitForm([], 'Save');
+ $loaded_terms = $state->get('taxonomy_test_taxonomy_term_load');
+ $this->assertCount(4, $loaded_terms);
+
+ // Adding a new term with weight < 0 implies that all root terms are updated.
+ $this->createTerm($this->vocabulary, ['weight' => -1]);
+ $this->drupalGet('admin/structure/taxonomy/manage/' . $this->vocabulary->id() . '/overview', ['query' => ['page' => 2]]);
+ $state->set('taxonomy_test_taxonomy_term_load', []);
+ $this->submitForm([], 'Save');
+ $loaded_terms = $state->get('taxonomy_test_taxonomy_term_load');
+ $this->assertCount(12, $loaded_terms);
+ }
+
}
--
GitLab