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

Add entity info key to indicate if exportable entities are stored as json instead of DB #175

Open
argiepiano opened this issue Jun 10, 2024 · 6 comments · May be fixed by #176
Open

Add entity info key to indicate if exportable entities are stored as json instead of DB #175

argiepiano opened this issue Jun 10, 2024 · 6 comments · May be fixed by #176
Labels
has pr maintainer review requested Request for maintainer review.

Comments

@argiepiano
Copy link
Collaborator

argiepiano commented Jun 10, 2024

It would be helpful to add a key to the array returned by hook_entity_info() to indicate that an exportable entity (aka "configuration entity") is stored as a json config file instead of in the database. This would allow us to avoid some fatal errors such as the one that happens in _entity_plus_defaults_rebuild() in the if statement that checks for $info['schema_fields_sql']['base table']. Since the entity is stored as json, it doesn't define hook_schema() and it doesn't have $info['schema_fields_sql']['base table']

This key could be called 'json storage'. If TRUE, then we could avoid the error I described above by skipping the if statement. Plus this may also provide other benefits (for example to Entity UI).

Some background on this:

_entity_plus_defaults_rebuild() (called _entity_defaults_rebuild in D7) was added a long time ago in D7. Before it, exportable entities could be defined in code, and would not be store in the database unless they were overridden. After the patch, this function stored all "in code" exportable entities to the database on cache clear or module enable/disable.

The problem I'm facing is that I'm working on the module Entity Plus CMI to create a Rules submodule. This submodule would save/load Rules configurations from json instead of database, making them available to the CMI api of Backdrop. Entity Plus CMI overrides EntityPlusControllerExportable. The main difference between exportable entities stored in the DB and those stored in config is that the second one doesn't use hook_schema(). Rather, it's "fields" are defined by including the fields to be read/saved in hook_entity_property_info().

This is working great, except that, when _entity_plus_defaults_rebuild()runs upon cache clear, we get a fatal error because the info for the entity doesn't contain $info['schema_fields_sql']['base table']. As a temporary fix, I have manually created a "dummy" array element called schema_fields_sql and a subarray base table in the entity definition array to fool that function.

Eventually, if we defined this key, providing a dummy info array element would be unnecessary, and it will provide other benefits to the Entity Plus code. It would allow to seamlessly move configuration entities such as rules_config and others (e.g. those provided by Search API) to config.

@argiepiano argiepiano changed the title Add entity info key to indicate exportable entities are stored as json instead of DB Add entity info key to indicate if exportable entities are stored as json instead of DB Jun 10, 2024
argiepiano added a commit to argiepiano/entity_plus that referenced this issue Jun 10, 2024
@argiepiano
Copy link
Collaborator Author

PR #176 added and ready for review. This is a very simple addition that opens the door for more development of entities stored as config files.

@argiepiano argiepiano added maintainer review requested Request for maintainer review. has pr labels Jun 10, 2024
@laryn
Copy link
Member

laryn commented Jun 10, 2024

@argiepiano This is a cursory review to be sure, but given this:

It would allow to seamlessly move configuration entities such as rules_config and others (e.g. those provided by Search API) to config.

I am wondering about the key being called "disk storage" -- config may not always be stored on disk as of 1.28.0, right?

@argiepiano
Copy link
Collaborator Author

I am wondering about the key being called "disk storage" -- config may not always be stored on disk as of 1.28.0, right?

Right. Perhaps 'json storage' then?

BTW I found a problem with the PR. Working on it.

@argiepiano
Copy link
Collaborator Author

Logic problem (parenthesis missing) fixed. @laryn, I'm open to suggestions re: the new key name.

@laryn
Copy link
Member

laryn commented Jun 10, 2024

Maybe 'cmi storage'?

@argiepiano
Copy link
Collaborator Author

Maybe 'cmi storage'?

I like it! Fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has pr maintainer review requested Request for maintainer review.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants