Follow-up to #2183231: Make ContentEntityDatabaseStorage generate static database schemas for content entities:

Problem/Motivation

Right, now both the SqlContentEntityStorage and the schema handler have to work together based on a known table mapping, what makes the line between both object blurry.

Proposed resolution

- Make the SqlContentEntityStorage table mapping agnostic and get the table mapping from the schema handler
- As suggested by yched in #2183231: Make ContentEntityDatabaseStorage generate static database schemas for content entities we could create multiple table mapping classes for the different table mapping cases also.

Remaining tasks

- Agree on the exact way forward and do it.

User interface changes

-

API changes

None, only some base classes change a little

Comments

fago’s picture

Status:Active» Postponed
yched’s picture

Basically, what differentiates an SQL storage strategy from another is entirely contained in the mapping layout (per field tables, per bundle tables...)
So having switchable mapping classes would allow alternate SQL storage strategies in contrib, while using the same EntityDatabaseStorageController.

yched’s picture

Related : crossposting from #2079019-10: Make Views use SqlEntityStorageInterface

I'd think TableMappingInterface are the objects Views should be taking its informations from ? In other words, IMO TableMappingInterface should be "the official business object to turn to when you need info about the SQL layout of an entity typed stored in an SqlEntityStorageInterface".

That probably requires working a bit on the API and DX of TableMappingInterface.
That might also mean moving the logic in _fieldTableName(), _fieldSqlSchema() & friends out of ContentEntityDatabaseStorage and into a base class for TableMappingInterface.

#2183231: Make ContentEntityDatabaseStorage generate static database schemas for content entities got in (yay!) and added TableMappingInterface. IMO this isssue here is where we refine what we want to do exactly with that concept, and adjust its APIs accordingly.

fago’s picture

agreed. I think I this should be postponed on #1498720: [meta] Make the entity storage system handle changes in the entity and field schema definitions now. It technically isn't, but that issue is going to the touch the same area quite a bit.

yched’s picture

plach’s picture

Nope, the other issue does not touch the storage class at all. The goal here is implementing the approach you suggested of moving the table mapping initialization logic that currently lives in CEDB::getTableMapping() to the table mapping instantiation itself (or lazy initialize it, details tbd). This also aims to make CEDB table layout agnostic and make it rely only on the provided table mapping.

plach’s picture

plach’s picture

Issue tags:+entity storage
andypost’s picture