Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DX] Add normalized project data into the database. #5905

Open
klonos opened this issue Jan 2, 2023 · 10 comments
Open

[DX] Add normalized project data into the database. #5905

klonos opened this issue Jan 2, 2023 · 10 comments

Comments

@klonos
Copy link
Member

klonos commented Jan 2, 2023

This is related to issues like #2548.

Add project data into the database. Make it normalized and easily accessible for database queries.

To follow up on #1: The system table already has the following columns:

  • filename
  • name
  • type
  • owner
  • status
  • bootstrap
  • schema_version
  • weight
  • info

We'd like to propose adding two additional tables: projects and project_info, where data that applies to all project types can live in the projects table, and data that applies to only one type of project (or only one specific project!) can live in the project_info table.

projects could have the following columns:

  • name (machine name of project - PRIMARY KEY)
  • label
  • description
  • backdrop
  • hidden
  • required
  • package
  • tags
  • stylesheets
  • stylesheets-conditional
  • scripts
  • version
  • dependencies
  • bootstrap
  • php

project_info can have the following columns:

  • name (machine name of project - PRIMARY KEY)
  • key
  • value

Original issue:

It took a bit of pause and head scratching while reviewing the PR for #5820. There was a check for ($info_array['type'] == 'module' || $info_array['type'] == 'profile') and then a call to system_rebuild_module_data() in order to get the human-readable names of projects, and I thought "why not do the same for themes and layout templates as well?". Then I remembered all these inconsistencies we have all over the place, which urged me to file this issue here.

I would like to propose the following:

  1. Have separate columns for the properties in the .info files of projects - not a single info column with a BLOB of serialized data as we currently have it (which forces developers to unserialize it each time). The serialized data in that BLOB contains duplicate information that can already be found in other columns of that table (namely project name and project type).
  2. Add data from .info files of layout templates to this table as well.
  3. [DX] Add a project_get_info helper function. #2385
  4. [DX] Does system_get_info need to be updated to include 'layout' as $type? #2384
  5. ...
@klonos
Copy link
Member Author

klonos commented Jan 2, 2023

PS: once this is done, we should revisit #5820 and the implementation in core/modules/system/views/views_handler_field_system_info.inc to ensure that we're able to pull info for all project types (should be a follow-up).

Various comments from that issue:

... Not all settings are provided, and some will only return a value for modules (not for themes). Empty strings are returned if the setting doesn't exist.

Layout template information is not stored in the system table, and therefore is unavailable to this handler. ...

One question, why one common field with a select list rather than individual fields? Not saying I prefer it the other way, but I find Views fields seem to lean toward granular options. Any particular reasoning here?

Thanks @docwilmot. The reason it's done this way is that Views field handlers are abstractions of select queries. Since the information handled by this handler comes from a single column (info in the table system), and since info contains a serialized array with ALL properties in the info file, you can't really do a select query for a single property. You get the whole info column at once, which has to be unserialized.

@jenlampton
Copy link
Member

jenlampton commented Jan 3, 2023

There are a few reasons info data isn't separated out into individual columns in the system table:

  1. The system table is intended to track the current state of various things on the system. It's not a project data table. What's in the system table is only what the system needs to know to operate. If we make substantial changes to the system table there could be serious consequences (tagging with "needs benchmarks").

If we want to create a store of project data, I'd prefer that we add a new project data table, so that we aren't interfering with system operations. (also tagging with "needs more feedback") The views handlers should probably use this new table as the base table, and join to system only when enabled or schema_version data is necessary.

  1. the info keys will be different for different types of projects, modules won't have layout info, themes won't have module info, etc.

I don't think it would make sense to make a database column for everything that might appear in the info file for any type of project, so as a rule of thumb, if it's not available for all project types, I think we should leave that information in info. Examples would include: base_theme, regions, default region,settings, engine, preview, dependencies, configure, etc). We can make exceptions for particularly valuable information as use-cases arise.

I'll add a list of properties that are consistent across all projects to the Original Post, and we can adjust as needed.

@yorkshire-pudding
Copy link
Member

How about a project data table with common fields and a project info table with project_name, info_type, info_value fields that could be easily joined to the project table (assuming project data table has project_name also)?
I think file_metadata table works on a similar basis.

@jenlampton
Copy link
Member

jenlampton commented Jan 3, 2023

Brilliant! Why didn't I think of that? :)

@klonos I rewrote the issue and moved your original content to the bottom. I hope that's okay :)

@jenlampton jenlampton changed the title [DX] Consistency and sanity in the [system] table of the database [DX] Add normalized project data into the database. Jan 3, 2023
@klonos klonos removed their assignment Jan 4, 2023
@klonos
Copy link
Member Author

klonos commented Jan 4, 2023

Thanks @yorkshire-pudding and @jenlampton for chiming in. Yes, we seem to be heading to a good direction with this 👍🏼

@klonos
Copy link
Member Author

klonos commented Jan 4, 2023

...since both tables will be holding project info (or data from the .info files of projects), I would like these to be something more descriptive/self-explanatory, such as project_info (for the standard things that are common across all projects) and project_misc_info or project_extra_info (for things specific per project type, or any additional info that may come after we have #119).

@yorkshire-pudding
Copy link
Member

I wonder whether project_common_info and project_extra_info would be suitably descriptive?

@argiepiano
Copy link

The proposal here makes sense from the point of view of a developer - serialized table columns are just wicked to work with. But on the other hand I wonder if there is really a huge benefit to a site architect or user, beyond the creation of Views filtering for module and theme properties? Is this something that has been added to D8/D9? Is there really a need to add stuff to the database?

I can see how a similar move (denormalizing serialized data) may benefit projects like Ubercart's attributes and product variations; the creation of pivot tables can provide so much flexibility to site admins (I'm working on a custom module for this), but I don't think the benefit is really that big for something like module properties, no?

@klonos
Copy link
Member Author

klonos commented Jan 4, 2023

The proposal here makes sense from the point of view of a developer - serialized table columns are just wicked to work with.

@argiepiano This issue is marked as [DX] precisely because it is a developer-focused improvement - not a [UX] issue. It is meant to simplify (and reduce?) core and most likely also contrib code.

But on the other hand I wonder if there is really a huge benefit to a site architect or user, beyond the creation of Views filtering for module and theme properties?

See issues like #2385, #2384 and the code of the respective functions in core. There are so many module-/theme-/layout-related functions and other code that are inconsistent throughout core that we won't know until we've started cleaning things up, and this is one of the first steps. The system table doesn't include info about layout templates, doesn't treat modules/themes/layouts the same, and in general makes it harder to consolidate and simplify these functions that retrieve information about projects, which is used all over the place in the core admin UI and code in general.

Is this something that has been added to D8/D9?

The system table was removed in D8/D9 (change record: https://www.drupal.org/node/1813642), and module/theme status is being tracked in config instead (core.extension.yml). We have #5163, #4079, #3860, and some other related issues to do some of these things in Backdrop (only whatever is relevant/applicable/useful to us).

Is there really a need to add stuff to the database?

I've asked this before somewhere, but AFAIK views handlers are meant to retrieve things from the db - they have not been created to retrieve things from config files. https://www.drupal.org/project/views_system for instance seems to be adding its own table in the database, and then populating it with (normalized) data from .info files (see https://git.drupalcode.org/project/views_system/-/blob/7.x-4.x/views_system.install). So unless there is a way to "proxy" data directly from .info files without having to get them into the database first, then the answer to your question would be "nope, no need". Unfortunately, we don't have that (yet?).

@avpaderno
Copy link
Member

I would like to see this implemented too.

It surely helps creating views handling projects data, but (in general) also any module that needs to handle that data for any reason.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants