Overview

As +1'd by @catch in #3457504-46: XB field type: calculate all dependencies, store them, surface in new Component "Audit" operation, this is a step to happen after #3457504: XB field type: calculate all dependencies, store them, surface in new Component "Audit" operation.

The intent is:

IOW:

Proposed resolution

Either:

  1. Change the deps_* properties to computed properties, store their data in a separate DB table

    See prior discussion:

  2. OR don't do this, and just do it as part of #3477428: [PP-1] Refactor (or decide not to) the XB field type to be multi-valued, to de-jsonify the tree, and to reference the field_union type of the prop values.

    User interface changes

    None, except … a measurably faster UI, and hence perhaps live usage counts at /admin/appearance/component?

CommentFileSizeAuthor
#15 component usage listing.png120.9 KBwim leers
Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

wim leers created an issue. See original summary.

wim leers credited catch.

wim leers’s picture

Title: [PP-1] Evaluate storing XB field type's "deps_*" columns in separate table » [PP-1] Evaluate storing XB field type's "deps_*" columns in separate table to allow efficient querying
larowlan’s picture

I can knock up a proof of concept for this if you like

wim leers’s picture

Title: [PP-1] Evaluate storing XB field type's "deps_*" columns in separate table to allow efficient querying » Evaluate storing XB field type's "deps_*" columns in separate table to allow efficient querying
Status: Postponed » Active
wim leers’s picture

Assigned: Unassigned » larowlan

larowlan’s picture

POC in draft MR
Need to move ComponentAuditTest to a Kernel test, mocking database calls is the trigger to move it from a Unit test

FieldTypeUninstallValidatorTest and ComponentAuditControllerTest are passing with this stored for querying.

wim leers’s picture

🥳🤓

larowlan’s picture

Title: Evaluate storing XB field type's "deps_*" columns in separate table to allow efficient querying » Store XB field type's "deps_*" columns in separate table to allow efficient querying
Assigned: larowlan » Unassigned
Status: Active » Needs review

New title as we want to proceed with this.

Here's the explain for selecting all entities using e.g. a particular component. We use the same buildDependencyQuery code path for the uninstall validator so the query is the same with only the params changed.

+--+-----------+------------+----------+----+--------------------------------+----------+-------+-----------+----+--------+-----------+
|id|select_type|table       |partitions|type|possible_keys                   |key       |key_len|ref        |rows|filtered|Extra      |
+--+-----------+------------+----------+----+--------------------------------+----------+-------+-----------+----+--------+-----------+
|1 |SIMPLE     |dependencies|null      |ref |PRIMARY,source_entity,dependency|dependency|267    |const,const|1   |100     |Using index|
+--+-----------+------------+----------+----+--------------------------------+----------+-------+-----------+----+--------+-----------+

This is ready for review now

wim leers’s picture

Assigned: Unassigned » wim leers
wim leers’s picture

Assigned: wim leers » Unassigned
Status: Needs review » Reviewed & tested by the community
Related issues: +#3498525: [META] Allow Canvas to be used on any content entity type (bundle), as long as it has certain entity type characteristics

This looks great!

Scariest/hardest to review were
ComponentTreeDependencyRepository and ComponentTreeDependencyRepositoryTest.

But that test is … impressively leading by example 🤩 I think @catch will be quite happy with #12 😄 My literally only question is Why not use a MERGE query?. Not worth holding this big leap forward up for though.

Plus, it puts is in a much better place to do #3498525: [META] Allow Canvas to be used on any content entity type (bundle), as long as it has certain entity type characteristics later already 🎉


The only failure here is unrelated to this MR: it's due to 11.1.7 being used on GitLab CI by default now, and that's being fixed over at #3523978: [11.1.7 regression] JavaScriptComponentValidationTest::testInvalidSlotIdentifiedByConfigSchema fails.

wim leers’s picture

Issue summary: View changes
StatusFileSize
new120.9 KB

None, except … a measurably faster UI, and hence perhaps live usage counts at /admin/appearance/component?

Implemented:

… which now definitely means we'll need to do #3522953: [later phase] Make Component `audit` operation performant/scalable — but that's intentional 😊

  • wim leers committed 57155bf4 on 0.x authored by larowlan
    Issue #3521202 by wim leers, larowlan, catch: Store XB field type's "...
wim leers’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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