I am facing the same issue, the reasoning here makes sense and the patch works, +1
----
I believe if we want to follow "best practices" from the drupal core ecosystem, it should work along those lines:
- query DB with default_langcode = 1 - similar to \Drupal\taxonomy\TermStorage::loadTree does (or use it directly)
- load terms via \Drupal\Core\Entity\EntityStorageInterface::loadMultiple
- and pass every term through \Drupal\Core\Entity\EntityRepository::getTranslationFromContext
But of course this would have a performance impact.
Looking at _term_reference_tree_get_children and the yelling comment there - "DO NOT LOAD TAXONOMY TERMS HERE" - I suspect this module makes a deliberate compromize? It probably bypasses some concepts from entity loading and getTranslationFromContext (like language fallback mechanisms etc). But again, maybe it's "by design" for this module...
#30 This issue happens when a user duplicates a paragraph that contains a file field and then removes a file reference in one of the paragraphs in the form (so not really the file entity itself).
This is most probably not restricted to paragraphs though. Maybe now that hook_entity_duplicate is here ( https://www.drupal.org/node/3268812 → ) the file module should implement this logic.
Thanks @liam morland, didn't see that issue, I'm closing this as duplicate then
I don't think I can generate these URLs. As mentioned in #47, they are most likely stale asset requests hitting PROD. We've also seen some China-hosted sites cloning our pages, which could explain some of these requests.
Another possibility is that bots are manipulating query strings, but I find the first explanation more likely.
In any case, these requests are problematic because they’re filling up the logs with errors.
FWIW, this does not seem to solve the case I encounter in
🐛
"Only file JavaScript/CSS assets can be optimized" errors in logs
Active
I just tried the patch here, and remove the one from that issue, hoping to close as duplicate.
When I try one of the URLs hitting prod locally, I see $include_libraries contain only libs that should be aggregated but the $delta = 2, which selects a $group inside $groups.
The hash_equals condition passes.
And here is the selected $group:
Array
(
[type] => external
[attributes] => Array
(
[defer] => 1
)
[group] => -100
[version] => 1.x
[cache] => 1
[preprocess] =>
[license] => Array
(
[name] => GPL-2.0-or-later
[url] => https://www.drupal.org/licensing/faq
[gpl-compatible] => 1
)
[scope] => footer
[items] => Array
(
[0] => Array
(
[type] => external
[attributes] => Array
(
[defer] => 1
)
[group] => -100
[data] => https://webtools.europa.eu/load.js
[version] => 1.x
[weight] => 0.0004
[cache] => 1
[preprocess] =>
[license] => Array
(
[name] => GPL-2.0-or-later
[url] => https://www.drupal.org/licensing/faq
[gpl-compatible] => 1
)
[scope] => footer
)
)
)
Note: this is from https://github.com/openeuropa/oe_webtools
So I'm confused... not sure there's anything wrong with that module, and what scenarios this issue here fixes?
But I believe we are missing some kind of validation on delta. See MR in #3536795.
FWIW I did not test yet but it may be that this is now solved by 🐛 Only file JavaScript assets with preprocessing enabled can be optimized. Active .
Per #11-12, what about centralizing code then?
Update \Drupal\paragraphs\Entity\Paragraph::createDuplicate to something like
$duplicate = parent::createDuplicate();
if (version_compare(\Drupal::VERSION, '11.2.0', '<')) {
/** @var \Drupal\paragraphs\Hook\EntityHooks $service */
$service = \Drupal::service(\Drupal\paragraphs\Hook\EntityHooks::class);
$service->duplicate($duplicate, $this);
}
Then it's clear that this code can be removed once <11.2 is no longer supported.
Phpunit tests are failing, probably because of the above, so moving back to needs work
Noticed the same with layout_paragraphs, D11.2 added a hook to react on entity clone in
🐛
It is not possible to react to an entity being duplicated
Needs work
Maybe we should leverage this?
I created another branch and MR (134) but did not test extensively.
Here is a pdf export of xhprof install process on the 1st project.
Here are some metrics on 2 of our projects.
1. One uses drupal 10.5.6, has 2050 configs in config/sync, and has 26 languages:
- baseline (without patches): 5m27s
- #3559030 only (disable cache.config): 2m36s
- #3559249 only (skip cache tag): 2m42s
- both patches: 2m32s
Worth noting that this project is also worse because it contains a hook_install in one of the custom modules which builds a batch and creates 827 taxonomy terms and so calls MemoryBackend::invalidateTags 827 times and iterates through the complete/full 50K+ config array.
In total, MemoryBackend::invalidateTags is called 192241 times on that project during install (checked with xhprof).
2. Another (smaller) project, on D11.2.8, 1800 configs in sync, and 26 languages
without patch: 02m03s
with #3559030 patch: 1m26s
With a 50K array, it adds up quickly. This takes 43 seconds on my machine.
use Drupal\Component\Datetime\TimeInterface;
use Drupal\Core\Cache\MemoryBackend;
$backend = new MemoryBackend(\Drupal::service(TimeInterface::class));
// Create 50K cache entries.
for ($i = 0; $i < 50_000; $i++) {
$backend->set('item_' . $i, []);
}
// Invalidate x times.
$start = microtime(true);
for ($i = 0; $i < 10_000; $i++) {
$backend->invalidateTags(['foobar']);
}
var_dump(microtime(true) - $start);
It seems this is causing failures, https://git.drupalcode.org/issue/drupal-3559030/-/jobs/7370601
Filter Entity Reference Web (Drupal\Tests\views_ui\Functional\FilterEntityReferenceWeb)
✘ Filter ui
┐
├ InvalidArgumentException: Input "options[reference_views][view][view_and_display]" cannot take "test_entity_reference:entity_reference" as a value (possible values: "views_test_entity_reference_filtered_display:entity_reference").
│
│ /builds/issue/drupal-3559030/vendor/symfony/dom-crawler/Field/ChoiceFormField.php:126
│ /builds/issue/drupal-3559030/vendor/behat/mink-browserkit-driver/src/BrowserKitDriver.php:422
│ /builds/issue/drupal-3559030/vendor/behat/mink/src/Element/NodeElement.php:118
│ /builds/issue/drupal-3559030/core/tests/Drupal/Tests/UiHelperTrait.php:107
│ /builds/issue/drupal-3559030/core/modules/views_ui/tests/src/Functional/FilterEntityReferenceWebTest.php:110
┴herved → changed the visibility of the branch 3252244-disable-config-cache to hidden.
Let's split the scope and move #7 to a separate issue, 📌 Installing from existing config causes massive cache.config population and slow installs Active .
Thank you @berdir!
I rebased both branches, it seems tests are passing.
Tried summarizing the solution in the IS, but feel free to adjust if needed, thanks.
Creted 📌 Investigate whether LanguageRequestSubscriber::onContainerInitializeSubrequestFinished() is still required and appropriate Active for LanguageRequestSubscriber::onContainerInitialize
herved → changed the visibility of the branch 3549397-delete-language-subscriber to hidden.
Yes I'm using mysql 8 on ddev - ddev/ddev-dbserver-mysql-8.0:v1.24.10
Hi, just passing by to mention I encountered a similar issue in
#3447145-11: Improve database performance when computing ModerationStateFieldItemList →
, see comments 11-15.
For some reason it happens after container restart.
Interesting... the slow query issue I described in #11-12 only happens after a container rebuild (ddev restart)
So when I rebuild the site from prod, import db etc, after that the query is very fast (0.1ms)
But if I then do ddev restart, queries are suddenly very slow (~50-60ms).
This might relate to
🐛
Stale table statistics can result in extremely slow queries
Active
to some extent, as running ANALYZE table on those fixes it, even after ddev restarts
ANALYZE TABLE content_moderation_state_field_revision;
ANALYZE TABLE content_moderation_state_revision;
Maybe we could still consider making the field case_sensitive, as that seems to avoid the issue and using LIKE may not be intended?
I hope I'm not hijacking your issue @mparker17, it seems you're on postgres which can behave very differently I suppose.
I can create a separate issue for mysql if needed.
I opened a new MR, could you maybe give that a try? just to see if that makes any difference on postgres.
Thanks!
I just stumbled on this on a D11.2 project, which has a custom formatter for node titles and retrieves the moderation state using $node->get('moderation_state')->value;.
I noticed the the cumulative time spend on this query for a given page is very high, 24 calls on 1 page for a total of 1.30 seconds, 40% of the page load!
The query is from \Drupal\content_moderation\Plugin\Field\ModerationStateFieldItemList::loadContentModerationStateRevision
PHP SPX snapshot
Here is the raw (my)SQL and explain
SELECT base_table.revision_id AS revision_id, base_table.id AS id
FROM content_moderation_state_revision base_table
INNER JOIN content_moderation_state_field_revision content_moderation_state_field_revision ON content_moderation_state_field_revision.revision_id = base_table.revision_id
WHERE (content_moderation_state_field_revision.content_entity_type_id LIKE 'node' ESCAPE '\\')
AND (content_moderation_state_field_revision.content_entity_id = '19112')
AND (content_moderation_state_field_revision.content_entity_revision_id = '204309')
AND (content_moderation_state_field_revision.workflow = 'wf_editorial')
AND (content_moderation_state_field_revision.langcode = 'en')
ORDER BY base_table.revision_id DESC
1 SIMPLE base_table NULL index PRIMARY PRIMARY 4 NULL 1 100.00 Backward index scan
1 SIMPLE content_moderation_state_field_revision NULL eq_ref PRIMARY,content_moderation_state__lookup,content_moderation_state__09628d8dbc,content_moderation_state__content_entity_revision_id PRIMARY 18 base_table.revision_id,const 1 100.00 Using where
This query takes ~50-60ms and the amount of records in the table is 64K, what should be very small for mysql to handle.
I tried adding the suggestion here first and adding the index but that made no difference.
However when I replace LIKE 'node' ESCAPE '\\' with a simple equal, it goes down to pretty much nothing (~0.1ms).
It seems drupal transforms equal operators to like in \Drupal\Core\Entity\Query\Sql\Condition::translateCondition
// If a field explicitly requests that queries should not be case
// sensitive, use the LIKE operator, otherwise keep =.
I suspect the case sensitivity currently set to false is not really intended?
Applying ->setSetting('case_sensitive', TRUE) on the base field and a hook_update fixes it.
MR created
This probably deserves a mention in the release note that the dependency on js_cookie module was removed and sites can uninstall it themselves if unused.
The MR addresses the issue, I can confirm notifications are not sent anymore when an admin creates a new OAuth2 client request.
I'm fine with addressing the points I raised in follow ups, as being out of scope.
I'm having a similar issue on a new project already on ui_patterns 2.0.10, it didn't even use ui_patterns 1.
But when applying composer update which gets ui_patterns 2.0.12, I see all the updates and it leaves ui_patterns_legacy enabled which I of course don't need.
$ ddev drush updb
------------- ---------------------------------------------- --------------- -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Module Update ID Type Description
------------- ---------------------------------------------- --------------- -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
ui_patterns 10201 hook_update_n 10201 - UI Patterns 2 upgrade, step 1. Before executing the update, convert your UI Patterns 1 patterns into UI Patterns 2 SDC components! @see https://project.pages.drupalcode.org/ui_patterns/2-authors/3-migration-from-UIP1/
ui_patterns 10202 hook_update_n 10202 - UI Patterns 2 upgrade, step 2. Enable ui_patterns_legacy and ui_patterns_blocks modules.
ui_patterns 10203 hook_update_n 10203 - UI Patterns 2 upgrade, step 3. Update configuration from UIP1 to UIP2.
ui_patterns 10204 hook_update_n 10204 - UI Patterns 2 upgrade, step 4. Uninstall UI Patterns 1 only modules.
ui_patterns update_layout_builder_override_ui_patterns_2 post-update Convert layout builder overrides into UI Patterns 2. @SuppressWarnings("PHPMD.NPathComplexity") @SuppressWarnings("PHPMD.CyclomaticComplexity") @SuppressWarnings("PHPMD.ExcessiveMethodLength")
------------- ---------------------------------------------- --------------- -----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
┌ Do you wish to run the specified pending updates? ───────────┐
│ Yes │
└──────────────────────────────────────────────────────────────┘
> [notice] Update started: ui_patterns_update_10201
> [notice] Update completed: ui_patterns_update_10201
> [notice] Update started: ui_patterns_update_10202
> [notice] Update completed: ui_patterns_update_10202
> [notice] Update started: ui_patterns_update_10203
> [warning] Undefined array key "type" ConfigurationConverter.php:413
> [warning] Undefined array key "type" ConfigurationConverter.php:413
> [warning] Undefined array key "type" ConfigurationConverter.php:413
> [warning] Undefined array key "type" ConfigurationConverter.php:413
> [warning] Undefined array key "type" ConfigurationConverter.php:413
> [warning] Undefined array key "type" ConfigurationConverter.php:413
> [warning] Undefined array key "type" ConfigurationConverter.php:413
> [warning] Undefined array key "type" ConfigurationConverter.php:413
> [warning] Undefined array key "type" ConfigurationConverter.php:413
> [warning] Undefined array key "type" ConfigurationConverter.php:413
> [warning] Undefined array key "type" ConfigurationConverter.php:413
> [warning] Undefined array key "type" ConfigurationConverter.php:413
> [warning] Undefined array key "type" ConfigurationConverter.php:413
> [notice] Update completed: ui_patterns_update_10203
> [notice] Update started: ui_patterns_update_10204
> [notice] Update completed: ui_patterns_update_10204
> [notice] Update started: ui_patterns_post_update_update_layout_builder_override_ui_patterns_2
> [notice] No entities to update.
> [notice] Update completed: ui_patterns_post_update_update_layout_builder_override_ui_patterns_2
[success] Finished performing updates.
I think all this should only run if we are actually coming from ui_patterns 1.x.
I did an attempt to fork this module and implement this idea.
I wanted to modernize the code first though and found some issues so if you are interested here is the branch: https://github.com/vever001/taxonomy_entity_index/commits/fork-wip/
In any case, this module uses the approach prior to
🐛
Fix IndexTidDepth views argument plugin and TaxonomyIndexTidDepth views filter plugin performance
Fixed
to without UNION which explains the reported issue here.
These commits specifically might be of interest:
- https://github.com/vever001/taxonomy_entity_index/commit/26995693eb8a7d3...
- https://github.com/vever001/taxonomy_entity_index/commit/a46e2319f602ed0...
- https://github.com/vever001/taxonomy_entity_index/commit/500facf8fdf3b0d...
Eventually go with another approach, because even with UNION queries I am still left with ~500-200ms query times.
It seems fetching all children separately and passing them to the final query is faster in my case.
#46 Maybe you are hitting the same issue I reported in
🐛
"Only file JavaScript/CSS assets can be optimized" errors in logs
Active
This can happen when old aggregates are requested or by manipulating the include or delta parameter (so the encoded include param contains an external library).
#5 Indeed that's probably the most pragmatic way to approach this.
I do notice however that https://git.drupalcode.org/project/taxonomy_entity_index/-/blob/8.x-1.x/... overrides the query method entirely and so - if I'm not mistaken - discards all the improvements that were made in core using union queries, from
🐛
Fix IndexTidDepth views argument plugin and TaxonomyIndexTidDepth views filter plugin performance
Fixed
.
I don't use taxonomy_entity_index, but on my project even union queries does not result in satisfactory query times, hence my suggestion to index the hierarchy as well. But yes, surely, I need to have a deeper look into this and collect more data before implementing it.
I'm in a similar situation. So far I a weighing a few options:
1. Propose a patch in this module
One solution could be index the hierarchy (all parents terms) as well in the table for faster queries.
I'm thinking to:
- Add either a is_parent boolean or hierarchy_depth int column in the table which would keep track of term’s relative position in the hierarchy (0 = directly assigned term; >0 = number of parent levels above the assigned term).
- Expose an option in the admin form to "Index hierarchy"
- When enabled, expose a new views filter and argument that will use it, optionally select how many sub-levels to look for.
- Update existing views filter and arguments to filter on hierarchy_depth = 0
- Add a hook_update to add the column and update indexes
I don't know if this project would be open for this. But if so I'd be happy to work on that.
2. Create a fork of this project in drupal.org or keep it in my project.
3. Another option I am considering is to leverage search_api but it comes with some drawbacks and less flexibility.
4. I also found https://git.drupalcode.org/project/taxonomy_parents_index which seems like a perfect match for me, but doesn't seem maintained anymore. It also looks much more minimal and won't solve issues such as ✨ Taxonomy Index for unpublished entities Needs work as I'm trying to hit 2 birds in 1 stone.
herved → changed the visibility of the branch 3555472-regression-messages-are to hidden.
I can confirm the issue. One example is to use node links.
Functionally the patch works and solves the issue. I left a few nitpicks in the MR.
But it would be nice to add a test for this, possibly under in modules/extra_field_plus_example/tests ?
Layout Library module doesn't have to be used for this to happen and using LB overrides is enough to cause this.
But I do see that layout_library section storage plugin defines the view_mode context as well, so this will fix all scenarios.
https://git.drupalcode.org/project/layout_library/-/blob/8.x-1.x/src/Plu...
The code originates from
#3069861: [3.x] Settings do not show up in layout_builder →
, specifically comments #19-26.
It seems these gathered_contexts are coming from LB section storage plugins.
Looking at \Drupal\layout_builder\Plugin\SectionStorage\DefaultsSectionStorage it does define the display context but that is not the case for \Drupal\layout_builder\Plugin\SectionStorage\OverridesSectionStorage which is used when using Allow each content item to have its layout customized..
So I think the current code does not account for this case when that option in used. I'm updating the IS to reflect that.
Both section storage plugins define the view_mode context so the fix looks correct / appears to work and will work in both cases as the goal is to retrieve it to build the extra_field settings form.
But we should add tests - most likely in modules/extra_field_plus_example/tests/src/FunctionalJavascript/ExtraFieldPluginPlusExampleNodeLabelFormattedFunctionalJavascriptTest.php - especially since this doesn't look so robust.
herved → changed the visibility of the branch 3508123-facetsexposedfilterssearchapiqueryalter-implements-a to hidden.
Why do we even need this hook?
Since query type plugins are responsible to set search_api_facets option in the query.
See e.g. \Drupal\facets\Plugin\facets\query_type\SearchApiString::execute which does
$options['search_api_facets'][$field_identifier] = $this->getFacetOptions();
The hook currently just overrides what query type plugins already set, with the same array... but potentially breaking/overriding what custom query types may set.
I'll create a new MR to simply drop the hook to see what phpunit says.
If I'm not mistaken 🐛 Make exposure of translation meta fields conditional Needs work is the parent issue and would solve this.
CI failure looks related to https://git.drupalcode.org/project/htmx/-/commit/71890ab8a300e408ba7a7fd...
herved → changed the visibility of the branch 3416735-register-streamwrappers to hidden.
Found another possible regression, see
#3416735-10: Stream wrappers not registered when installing module's default config →
Which I suspect is coming from ::updateKernel call at the very end of ModuleInstaller::doInstall
I stumbled on this issue because I found a scenario where stream wrappers are not registered properly.
When more than 1 module is enabled, stream wrappers are not registered, see https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co... which calls $this->updateKernel([]) and doesn't call \Drupal::service('stream_wrapper_manager')->register(); after it.
Notice that just above it we have a call to ::updateKernel and then it registers stream wrappers.
That particular code seems to originate from
📌
Add the ability to install multiple modules and only do a single container rebuild to ModuleInstaller
Active
.
This can cause all kind of issues.
In my case, I have a config import that has to enable more than 1 module and delete an image style, and noticed the image style folder is not being deleted, which should happen in \Drupal\image\Entity\ImageStyle::postDelete but stream wrappers are not registered.
I can confirm the issue and was able to replicate. The changes LGTM and address the issue. +1
Makes sense to me, an error is too "aggressive", so +1, thanks.
Here is a static patch for composer
This issue primarily focuses on ensuring that the locked and hidden states of facet processors are respected. Currently, facets_exposed_filters do not honor these settings. Some projects, including mine, rely on custom processors that depend on those features.
I believe the MR addresses the issue, but we could definitely add some test coverage.
I’d appreciate maintainer feedback on whether this is headed in the right direction.
While working on it, I realized the way FacetsFilter deals with the hierarchy processor and settings needed some adjustments.
I wonder why is the hierarchy processor locked in the first place, and if we could/should make it standard/unlocked. But that would involve a lot of changes also for facets blocks.
Just a thought, but the issue title is no longer accurate since the PHP TypeError doesn't happen anymore on 3.0.x.
So this now adds return type to SearchApiDisplay::getCount which could be done but this could affect BC and I wonder if we should apply the same to other ::getCount implementations for consistency? as mentioned in #13
I assume the reasoning behind this makes sense, but I noticed it can have side effects.
The added checkbox class breaks the styling on one of our projects that uses Bootstrap 3 and didn’t account for it.
I wonder if this should instead use the Drupal.theme JS API.
Isn’t form-check Bootstrap-specific?
Also, does this follow classes from core/modules/system/templates/form-element.html.twig? if so, why isn’t form-item included?
There is a conflict in the MR, recent changes on 3.x affected this and the return type was removed to fix phpstan.
See https://git.drupalcode.org/project/facets/-/commit/28332851b927d94f4deb3...
I think we could either:
- close this
- or add the return type to ::getCount in FacetSourcePluginInterface, FacetSourcePluginBase and SearchApiDisplay, but that would affect backwards compatibility I assume.