[PP-1] Consider not storing the ComponentTreeStructure data type as a JSON blob

Would it be possible to document what are the benefits and downsides of using a flat table over a JSON blob? Based on the issue summary, it isn't clear to me what are the trade-offs involved in this decision.

Wim Leers → credited longwave → .

Thoughts:

• I agree a JSON blob is overkill and a table is in principle sufficient.
• … but Drupal's Entity Field system assumes every field prop corresponds to a data type (true here too), and that data type must be a single column in the database (not true here), and is always a primitive (the data type class must implement \Drupal\Core\TypedData\PrimitiveInterface if that data type is used for a field property — AFAICT)
• So then IMHO the question becomes: how do we store such a table in a single DB column?

The only exception I can think of (right now, in the little time I have this morning while writing this up to ensure we follow through!): the PathItem field type, which essentially is a computed field type that stores its data in another database table and does the necessary additional DB queries.

Maybe that can work? That maybe actually gets closer to @effulgentsia's original proposal at ✨ JSON-based data storage proposal for component-based page building Active , where he proposed to do some deduplication stuff that most of us wanted to defer to later?

We could then literally have a single DB table for all XB uses, by expanding the columns from

parent,slot,delta,uuid,component

(@longwave's comment)
to:

entity_type_id,entity_id,field_name,parent,slot,delta,uuid,component

… which would not violate the 3.2.4 Facilitating `component props` changes section in docs/data-model.md.

One benefit is this becomes trivial:

An upgrade path for a `component` would require logic somewhat like this:

1. SQL query to search the _tree_ JSON blob for uses of this `component`, capture the UUIDs.

SELECT uuid FROM table WHERE component = "provider:person-card"

@Wim Leers additional note: we also have to consider langcode for asymmetric translations

@lauriii One of the advantages here is it would (at least partially) solve the same problem of non-portable JSON queries (which are already in the XB code base) that 📌 Calculate field and component dependencies on save and store them in an easy to retrieve format Active is also trying to remove - same general problem as #5.

… but Drupal's Entity Field system assumes every field prop corresponds to a data type (true here too), and that data type must be a single column in the database (not true here), and is always a primitive (the data type class must implement \Drupal\Core\TypedData\PrimitiveInterface if that data type is used for a field property — AFAICT)

I'm not sure exactly which data isn't fitting in a single column, but could just that column be JSON (storing a flat array of whatever doesn't fit) without undermining the goal of the issue?

#5++

#6++

#7: an XB field must store a tree of components + sources for their props values. This issue proposes to store that tree as a list, so not as a single column in a single row, but as multiple columns in multiple rows. That's what I was trying to convey in #4, and is also why I pointed to @effulgentsia's "deduplication" proposal (we could implement this by storing a single value for each XB field revision's tree, with that value pointing to a foreign key in another table, where @longwave's proposed columns could then be used).

I think this proposal makes sense. My initial thinking behind JSON for this was thinking we wanted to store it as a tree. But since we've already changed that to a flat JSON representation, I don't think JSON for this part has any special value.

Drupal's Entity Field system assumes every field prop corresponds to a data type and that data type must be a single column in the database

I think what we could do is store this as a multi-valued field. If we want to keep tree and values as a single field, we could do that by each item containing the following columns: component_instance_id, component, parent, slot, delta_in_slot, props, where that last one is the JSON props source/expression/values for that component instance.

However, at some point, I think it will make sense for us to model this as two fields: in other words, pull the props column out of the above suggestion and make it its own separate field from the field representing the tree. Not sure if we want to jump to the two field implementation as part of this issue or continue to keep it as one field until we separately discuss the pros and cons of two fields vs one.

#4

We could then literally have a single DB table for all XB uses, by expanding the columns from

entity_type_id,entity_id,field_name,parent,slot,delta,uuid,component

I'm not sure that would with revisions and sync/async translations?

Removed duplicate related item.

I incorporated this into 📌 Refactor the XB field type to be multi-valued, to de-jsonify the tree, and to reference the field_union type of the prop values Active .

Repurposing this as a spike to explore storing the tree in a flat structure and unblocking 📌 [PP-1] Spike: Explore storing a hash lookup of component inputs Postponed which would basically be the proposal in comment #9 here, but instead of props we'd store the lookup hash

Additionally there's scope to use CTEs here like we do in Entity hierarchy v5 to retrieve an ordered tree in a single SQL query should we need it. That module has views integration to let people do things like 'is child of' and 'is parent of' in a single query - which might be attractive for issues like 🌱 [META] Support alternative renderings of prop data added for the 'full' view mode such as for search indexing or newsletters Active

Picking this spike up

👏🙏

Complication not yet discussed here: ✨ Content templates, part 3b: store exposed slot subtrees on individual entities Active . But perhaps it's worth first doing this issue without complicating it with that aspect?

Note: we could choose to tackle 📌 Remove root UUID Active as part of this, by equating \Drupal\experience_builder\Plugin\DataType\ComponentTreeStructure::ROOT_UUID with NULL for the parent column.

I defer to @larowlan.

This would also unblock ✨ ComponentTreeItem should automatically purge subtrees for slots that are no longer exposed Active for ContentTemplate support.

I think it's worth doing this first, but having ✨ Content templates, part 3b: store exposed slot subtrees on individual entities Active in mind while doing it.

There are probably two ways that issue could be done, assuming row-per-component is implemented here:

1. Keep a single field, but add a column for 'slot' (or whatever else is necessary), meaning deltas would span across multiple slots.

2. Add a separate field for each exposed slot.

If those are indeed the two options and there is not some other third option, then they're both compatible with this issue (I think), so it should be fine to do this first without having to pick one in advance.

I intend to get to the bottom of that one tomorrow, but it is reassuring to read you do not anticipate problems with this implementation order 😊

Just caught up on #3519352 — high-level responses at #3519352-38: Content templates, part 3b: store exposed slot subtrees on individual entities → .

Makes me want to #22+++++++++++++ even more than I could've anticipated 😄 — as in: "do this first, but keep #3519352 in mind".

Pushed up some work in progress. Will continue with the TranslationTest next as that's the big unknown

cross post

Draft MR up, should be passing linting

I think now is good time to check in and decide if we want to get this mergeable

Referring back to the spike objectives

1. Determine if this is feasible - yes definitely - TranslationTest is passing, rendering is working
2. Consider the implications for API consumers like JSON:API - If we add a normalizer for the ComponentInputs typed data this becomes pretty simple
3. Find out any possible downsides - Alex mentioned that some components won't require all their props to be translatable. This model will allow us to do that on a per component basis as we're one row per instance. I think this also makes purging slots and ContentTemplate tree merging a lot simpler
4. Size the remaining work that would be required to implement this - I think there's probably 1 or 2 day left to clean up the failing tests (there's a lot!) - many tests are constructing the data in the old (tree+input) format and need to move to the multi-cardinality format.

Assigning to Wim to get a +1 keep going before going any further.

Awesome! Thank you for summarizing the current status as well 🙏😊 Excited to review this!

Find out any possible downsides - Alex mentioned that some components won't require all their props to be translatable. This model will allow us to do that on a per component basis as we're one row per instance.

These seem potential benefits of this approach rather than downsides 🤔 Have we considered what are the downsides with this approach? How does this impact performance, flexibility, storage requirements, and developer experience?

I'm also curious, how does having one row per instance help with prop translation? Might be worth explaining the change in more detail in the issue summary because I feel like I'm missing something.

I'm also curious, how does having one row per instance help with prop translation? Might be worth explaining the change in more detail in the issue summary because I feel like I'm missing something.

This bit is specifically what I wanted @larowlan and @effulgentsia to discuss. Per #28, they did discuss that, and @larowlan got the (a)symmetrical translation support working as he mentions in #28.1.

I'd be fine with explaining that in the updated docs/data-model.md rather than the issue summary. I think @larowlan just hasn't gotten to that point yet 😊

Linking right parent.

How does this impact performance, flexibility, storage requirements, and developer experience?

On performance and storage, for saves it's obviously more rows to write than the JSON blob, but because it will be in a transaction, it should not make much difference. Reading will get the rows in a single query so should be neutral.

There are already additional rows to write on entity saves due to 📌 [PP-1] Evaluate storing XB field type's "deps_*" columns in separate table Active , which denormalizes some of the existing information in the JSON field. I think this MR (or a follow-up) will make that completely unnecessary because there's already a column with the component in it. So it should actually be the same, or similar, amounts of database rows with less duplication overall.

Any kind of auditing or bulk operation on components should be easier - e.g. to find all entities using a specific component, you can query the component column (including from custom views admin listings or similar), this means no bespoke integration with 📌 [PP-1] Evaluate storing XB field type's "deps_*" columns in separate table Active to worry about, no JSON queries.

For DX, if someone is looking around the database tables, it will be a more familiar structure than a JSON blob + separate denormalized dependency lookup table. Apart from looking in the database or working on XB itself, it should be transparent anyway.

For storage requirements, it should help compatibility with ✨ Add way to "intern" large field item values to reduce database size by 10x to 100x for sites with many entity revisions and/or languages Active because instead of a single large field that could have a single character change, it will be possible to intern per-component - e.g. when there's a long text input. Otherwise a change to any part of the tree would preclude interning. If/when we add revision pruning outside XB, it may be possible to do something like only prune revisions when they are not removing the last remaining instance of an 'interned' value, so that the full history of changes to individual fields can be preserved, just thought about this now and not sure if it will hold up, but it would be a nice bonus if so. That definitely wouldn't be possible when all the content is in the single JSON blob.

Views queries on prop values will still be hard, because that will need to query the JSON column, which will run into all the same issues discussed on 📌 [PP-1] Evaluate storing XB field type's "deps_*" columns in separate table Active with regard to JSON indexes, cross database support etc.. However, it will be a simpler JSON structure to query than with the current schema, so can only make that better rather than worse. I don't think views queries on values for this field are a high or even medium priority, and approaches like 📌 Spike: Explore storing component inputs in separate columns (aka field union) Active , while they would avoid JSON, introduce a lot of cross-table JOINs which can have their own problems and complexity. So for me the single JSON column with values is the right balance.

It will make things easier for MongoDB, because MongoDB can just store this in jsonb (along with everything else for the entity/revision), and use it natively instead of having to deal with the duplication and custom schema in 📌 [PP-1] Evaluate storing XB field type's "deps_*" columns in separate table Active . So while it moves the main field away from a document model, the overall XB database schema moves closer to one again - back to only field storage without custom relational SQL layers on top.

For flexibility, the actual change should be neutral in terms of XB itself. However when it's combined with 📌 Version component prop definitions for SDC and Code components Active we are getting very close to a 'dynamic field union' or 'no entity paragraphs' data model which could potentially be re-usable or adaptable outside of XB itself - e.g. for structured, multi-column fields (think a CD content type with a track listing field, where the track listing has song title, duration, musician entity references etc.). Those structured fields can then be incorporated into XB layouts. Without the dynamic field union, we will either see people continue to use paragraphs or similar models, or try to shoe-horn those kinds of things into dynamic slots even if they should be structured fields.

This is a long list of benefits or neutral effects, with no actual downsides, but I think that's right.

The big bet we made with XB >1 year ago was that ✨ Add "json" as core data type Active would get fixed, which with the last core minor release (11.2) prior to XB 1.0 in the final stages now, is guaranteed to not happen.

So the primary goal of this MR: decomposing the 2 big JSON blobs per revision (tree and inputs) that we've been discussing for >1 year in many places, most recently in 📌 Calculate field and component dependencies on save and store them in an easy to retrieve format Active . This MR must do it in a manner where:

• everything that already works today continues to work
• all technical debt and missing features listed in 🌱 [META] Production-ready data storage Active are either solved, become simpler, or at minimum, remain equally achievable

The approach @larowlan went with is option 1 that @catch listed in #22.

Reviewed this entire 71 files, +1908, -1680 MR 😄

Posted 67 remarks, of which 19 were trivial comment nits that I already applied. The remainder are either observations to help the next reviewer (ℹ️), excitement/admiration/applause (🤩) or questions 😄

Light bulb moments

While reviewing, these were the most important "aha!" moments:

1. The (a)symmetrical translatability support concerns in #31 are solved by core's require_all_groups_for_translation, which is used by the image field type already 👍
2. This MR's new ValidParentAndSlotConstraintValidator allows trivially adding support for ContentTemplates: there's already a @todo, the remaining work to make it fully validatable is trivial, and makes ✨ Content templates, part 3b: store exposed slot subtrees on individual entities Active completely obsolete AFAICT! This MR's introduction of storing every component instance separately, with an optional parent_uuid and optional slot is what makes this easy (instead of very hard in HEAD) 👍

   IOW: this does address the meta (#3520449)'s .

3. A single additional normalizer (for the ComponentInputs data type) that would be trivial to implement would be sufficient to achieve JSON:API read/write support for content entities with XB fields! Which means XB would be able to leap ahead of LB on that front already, because 📌 [PP-1] Expose Layout Builder data to REST and JSON:API Postponed remains unsolved. The difference is that XB started with strict schema + validation, so the big "ensure the normalization can be interpreted by front-end developers" problem is already solved. (It'd even comply with the recommended best practice at 📌 Discourage @FieldType-level normalizers, encourage @DataType-level normalizers, to strengthen the API-First ecosystem Needs work .)
   EDIT: that's what @larowlan independently concluded in #28.2! 👍

Issues we could close if this lands

AFAICT this allows us to close, @larowlan should confirm:

1. 📌 Remove root UUID Active
2. 📌 Add typed value-objects for the component tree structure Active
3. ✨ Content templates, part 3b: store exposed slot subtrees on individual entities Active

Questions

1. While I know it was not a goal of this issue, his does not yet address the meta (#3520449)'s .

   AFAICT this MR does not make that neither more difficult nor easier to achieve. In HEAD, it would have required some extra per-component instance metadata. Here, it would, too. In HEAD, it'd probably have had to live in the data contained in the ComponentInputs data type. With this MR in, that would still have to be the case.

   So I think this MR is neutral for that.

   EDIT: that seems to match what @larowlan independently wrote in #28.3 👍

Conclusion

A very strong "+1, yes please! 😄" to @larowlan's .

I think option #1 vs #2 from #22 is still to be determined in ✨ Content templates, part 3b: store exposed slot subtrees on individual entities Active . I think there might be some confusion between 'slot' and 'exposed slot' in my original comment either between me and Wim in #34 or possibly just by me.

If we did option #1, I was thinking we would probably have to add an additional 'exposed slot' column to map the row to the exposed slot in ✨ Content templates, part 3b: store exposed slot subtrees on individual entities Active (in that issue, not this one). e.g. all the data for all the exposed slots in one field instance.

If we did option #2, then the schema added here would be unchanged, instead each exposed slot would have a field instance.

Or am I misunderstanding and this already implements #1 because slot === exposed slot in this context? If so the confusion is all mine.

@catch in #33:

Even if there are more rows, writing additional rows in a transaction shouldn't make a lot of difference

Good reassurance, this I'm less familiar with 🙏👍

storage requirements, it should help compatibility with […] because instead of a single large field that could have a single character change

Exactly!

ot removing the last remaining instance of an 'interned' value, so that the full history of changes to individual fields can be preserved, just thought about this now and not sure if it will hold up, but it would be a nice bonus if so.

Wow, great insight — that would be amazing! 🤩

Any kind of auditing or bulk operation on components should be easier - e.g. to find all entities using a specific component, you can query the component column (including from custom views admin listings or similar

Exactly! IOW: this would basically solve 📌 [later phase] Make Component `audit` operation scalable Postponed , and would allow us omitting config: experience_builder.component.* dependencies from 📌 [PP-1] Evaluate storing XB field type's "deps_*" columns in separate table Active 's component_tree_depencies table. Because that's essentially duplicating that same bit of information now.

For DX, if someone is looking around the database tables, it will be a more familiar structure than a JSON blob + separate denormalized dependency lookup table. Apart from looking in the database or working on XB itself, it should be transparent anyway.

💯 — this MR single-handedly makes XB much easier to wrap your head around. Much becomes deducable.

However, it will be a simpler JSON structure to query than with the current schema, so can only make that better rather than worse.

Indeed! To be more precise:

• from 1 massive JSON blob per XB field revision
• to 1 small JSON blob (with one less level of nesting too) per component instance per XB field revision

However when it's combined with […]

That's what I'm starting work on right now! 🤓🥳

This is a long list of benefits or neutral effects, with no actual downsides, but I think that's right.

💯

@catch in #35:

You're right that that choice is not finalized in this MR. But this bit in ValidParentAndSlotConstraintValidator definitely lean towards option #22.1 aka storing an exposed slot name in the slot field property/column of an XB field item:

    // @todo 👉️ For 'bonsai' trees, the parent may not be in the corresponding
    //   parent list but in a ContentTemplate for the parent entity.

Which at least I interpreted as @larowlan intending to implement it this way in this MR, which would make almost true: it's not quite implemented yet, but AFAICT @larowlan intends it to be.

This is why I tried to deduce how @larowlan intends to implement this and the required code would indeed be trivial.

Perhaps we're both missing something? :D

Good reassurance, this I'm less familiar with

There's some background in 🐛 Configuration management performance regression - slow config:import Needs work . I don't think we were actively making use of this behaviour (or necessarily knew about it at all) until recently, for entities it's been like this since we added transaction support to core. Either way the write to one field table happens in a single query anyway (just double checked), so it may even behave exactly the same before/after in terms of write performance or at least very similarly.

The spike has proven itself, as I already said in my review in #34, but especially now that it's green!

Excited to review this 85 files, +3644, -3221 diff in depth tomorrow! 😄

@larowlan indicated he'd be doing a self-review first thing tomorrow, so leaving assigned to him for that. 👍

Did another pass and was able to
* Remove the new UniqueConstraint validator in favour of putting it into ComponentTreeStructure
* Noticed we weren't using ComponentTreeStructure on field default values, was tricky, but was able to add that
* Made validation property paths consistently use dot notations e.g. something.1.uuid instead of a mix of that and square brackets - something[1][uuid]
* Another pass on docs

Added comments to the MR to help reviewers

This is ready for reviews now

🥳🤓

I think the true test of a well-designed system (besides adoption, vibrant ecosystem, etc. — all of which by definition cannot yet exist) is:

Well of course it works like that, that all looks like it makes total sense.

IOW: when it seems so evident that the data and logic is structured in this particular way, that it's hard to imagine any other way.

I think XB is now entering that territory with this MR. 🤓🤩

It's amazing to see how this single (massive!) MR completely transforms the most essential part of XB (the way it stores component trees in both content entities as fields and in config entities), and we can be confident that it'll A) work (thanks to tests), B) scale to many different uses (thanks to the 3 different config entity types that use it, and kinda 4).

If it were so evident that this was the right approach, we would've written it like this at the very start. But we didn't. It required iteration. And the uses, the tests, the validation logic all needed to be structured in a sufficiently sensible manner for this massive refactor (by a single (very capable) person over the course of a few days!!!) to be achievable.

IMHO this is the biggest achievement yet of the team working on XB's back end. And it's thanks to literally all of those people having contributed — including all the front end work that uses that back end and that end-to-end tests are exercising the back end through the front end.

See the remaining open threads — they highlight some of the crucial improvements.

Bonus delight: @larowlan's fine test string choices:

🤣

It also allowed me to:

1. close #3471026-13: Harden UUID validation in ComponentTreeStructureConstraintValidator →
2. reduce scope and stable blocker tag for #3495625-14: Remove ComponentTreeItemList::ROOT_UUID from hydration and client-to-server conversion →
3. clarify #3499632-13: [PP-1] Remove `ClientServerConversionTrait` (C→S) and `ComponentTreeItemList::getClientSideRepresentation()` (S→C) in favor of (de)normalizers →
4. reduced scope and precise remaining steps for #3519352-41: [PP-1] Content templates, part 3b: store exposed slot subtrees on individual entities →
5. create a nicely scoped follow-up for the last crucial DX improvement for config management: 📌 Ensure predictable config export order of config-defined component trees Active

Finally, this is what a single simple component tree on a content entity now looks like in the DB:

… and that component_id column will evolve in the next issue @larowlan is working on: 📌 Version component prop definitions for SDC and Code components Active .

Awesome to see this land!

Full credit to Jim Morrison poetry for the test strings.

There's a minor issue here https://git.drupalcode.org/project/experience_builder/-/merge_requests/1... around DX consistency from a late change, I've reopened this to revert that (separate MR). Creating that now

Opened a follow up for a couple of things spotted in review

Thanks for the posthumous review/confirmation of the tweaks I made to your epic MR, and merged the follow-up 👍 😊

This somehow introduced a regression for default_content's export functionality: 📌 Default content exports are invalid and hence are not correct after importing Active

Which would've been far less painful for @justafish to run into if the validation this MR introduced caught the problematic bits, created 📌 Tighten validation of `parent_uuid` and `slot` on XB fields to match the strictness of config Active for that.