backdrop-contrib / entity_plus

This module wraps in a variety of additional entity-related functionality from various sources. Partial port of D7 Entity API.
https://backdropcms.org/project/entity_plus
GNU General Public License v2.0
3 stars 11 forks source link

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

Open argiepiano opened 2 months ago

argiepiano commented 2 months ago

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 commented 2 months ago

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.

laryn commented 2 months ago

@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 commented 2 months ago

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 commented 2 months ago

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

laryn commented 2 months ago

Maybe 'cmi storage'?

argiepiano commented 2 months ago

Maybe 'cmi storage'?

I like it! Fixed.