- Issue created by @catch
- 🇬🇧United Kingdom catch
I haven't looked at Dashboard module internals, but pretty sure that also needs the same treatment.
- 🇬🇧United Kingdom catch
@longwave mentioned in slack the possibility of generalising this to all plugins that have settings. We could maybe use it for views config updates if we did that.
Also config entities that store plugin settings could maybe generically do something to, to apply any plugin update plugins they need to. Although they also need to indicate where those plugin settings are stored in their own data structures. Only views knows where area or filter plugins are in its own config entities.
- 🇬🇧United Kingdom catch
For scope something like:
1. Add the update plugin interface, and the updater service called from post updates, and the presave bit, and implement this in block module + search for the update that already exists. That will give us one working implementation of one update and one block plugin storage. We'll need to decide if we make this generically useful for all plugin types or limit it to blocks.
2. Open separate issues for layout builder, navigation, and experience builder to implement the storage update parts.
3. Check for any other block updates in core that follow a similar pattern to the search one and convert those too.
4. If we do make this generically useful for other plugin types and config entities, open follow-ups for those.
- 🇪🇸Spain penyaskito Seville 💃, Spain 🇪🇸, UTC+2 🇪🇺
#4 🐛 Block plugins need to be able to update their settings in multiple different storage implementations Active I haven't looked at Dashboard module internals, but pretty sure that also needs the same treatment.
Confirm, we would need to do the same, and we are not doing anything for this scenario (in this case I'd expect layout builder implementation to handle this for everyone using a SectionStorage plugin).
#0 🐛 Block plugins need to be able to update their settings in multiple different storage implementations Active Conversely, updates are not the only place that this logic needs to run, it also needs to run on presave of all these different config entity types to support shipped config too.
IMHO this will be complex enough that it could be left for a follow-up, and AFAIK we have no precedent of this. Looks like a related-but-not-the-same separate beast.
E.g. if node module implements a config schema change on node_type, the shipped node types config of contrib module X are not updated on installation of module X.
#8 🐛 Block plugins need to be able to update their settings in multiple different storage implementations Active @longwave mentioned in slack the possibility of generalising this to all plugins that have settings.
If we include shipped config on the scope, and per my comment above, wouldn't this be a general problem for any config that has an update to its structure/config schema, and not only plugins?
And I'd expect we would need some kind of tracking of the last hook_update_N/post_update where this shipped config was generated with, which might mean a huge change with no BC layer.
- 🇬🇧United Kingdom catch
IMHO this will be complex enough that it could be left for a follow-up, and AFAIK we have no precedent of this.
We already implement an equivalent presave hook for every* config entity update in core. Views has the most examples of this, which are handled centrally via ViewsConfigUpdater. The search example above also comes with a presave hook. See https://api.drupal.org/api/drupal/core%21modules%21search%21src%21Hook%2...
*except when we forget, although I think Drupal 10/11 are better than 8/9 were.
- 🇦🇺Australia acbramley
if node module implements a config schema change on node_type, the shipped node types config of contrib module X are not updated on installation of module X
We're starting to tackle that now with pre_save hooks (see issue below)
I've opened 📌 Add generic interface + base class for upgrade paths that require config changes Active which is sort of similar to this issue but for config entity changes specifically.
WRT. this issue, Layout builder is going to be really tricky here for overridden entities. That config is stored as a blob in the database. It's a massive headache to update these and requires an entity save or some DB voodoo.
I wonder if instead we have something that applies default config upon loading the plugin? That way when a user edits an entity that has a block plugin in its overridden LB storage, it'll get those config changes applied and then when they save the changes will persist to the db? - 🇬🇧United Kingdom catch
WRT. this issue, Layout builder is going to be really tricky here for overridden entities. That config is stored as a blob in the database. It's a massive headache to update these and requires an entity save or some DB voodoo.
This is true and will also affect the experience builder field in a similar way - however I think we can add the config elements of this, and look at 'plugin configuration stored in entity data' in follow-ups. Just the config aspects are bad enough.
A just-in-time update sounds interesting although wouldn't that then mean the update code needs to be maintained indefinitely?
- 🇬🇧United Kingdom catch
Replying to myself here:
A just-in-time update sounds interesting although wouldn't that then mean the update code needs to be maintained indefinitely?
I thought about this some more, and we already sort of have a pattern for just-in-time updates in the phpass module. For phpass we cannot bulk update user password hashes, otherwise we could reverse engineer people's passwords, so we just have to wait for users to log in again, update the password hashes then, and then one day site will uninstall the module and any users that didn't log in for years will need to reset their password.
For content we could bulk update content, but we don't really want to do this in a mandatory hook_post_update_NAME() because we don't want to enforce potentially hours of downtime on sites with lots of content in a specific minor release.
So what we could do is implement a just-in-time update on entity presave, and that will update content one by one when it's updated.
We've then got the question of what to do with content that isn't manually updated, there are several ways to tackle this.
Ideally we'd have a way to detect which entities and revisions need updating, e.g. at least only those that are using layout builder overrides or whatever the criteria is. And once it's loaded the entity, we should be able to check if anything actually needs saving before saving it. This would reduce overall churn.
For actually running the update there are several options:
1. Drush or console command which can be manually run, loads and saves everything that needs an update.
2. Admin page that does this in a batch.
3. Admin page that queues all the content on the site, or does something similar to search indexing with a watermark + cron.
We also have issues around discussing revision compression/pruning so e.g. if we add a way to prune old non-default revisions, or very old default revisions, that would allow sites to cut down on the amount of content that needs to be updated.
This does imply keeping the just-in-time update path around for a couple of major releases, but that code will be easier to maintain than a hook_post_update_NAME(), which can go horribly wrong (all of Drupal 7 and the first few releases of Drupal 8 testify to this).
- 🇬🇧United Kingdom longwave UK
> a just-in-time update on entity presave
This also has to take place on entity load, because we need to fix up data so it can be handled correctly by anything that wants to consume it - the renderer or whatever else needs the entity; it might never even be saved again without some kind of bulk/batch/queue mechanism. But doing all these checks on load likely comes with a performance penalty, especially if the amount of updates grows over time.
- 🇬🇧United Kingdom catch
This also has to take place on entity load
Ouch, yeah was thinking in terms of config entity presaves but that relies on them being saved before they're loaded, which isn't applicable here for the exact reason we're discussing this at all.
But doing all these checks on load likely comes with a performance penalty, especially if the amount of updates grows over time.
If it's on hook_entity_load() or an equivalent spot, with a lot of isset() checks etc., then it would be cached in the persistent entity cache for most entity types, so it might not be too bad purely in terms of performance impact.
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
we already sort of have a pattern for just-in-time updates in the phpass module
Fascinating! 🤓
Ideally we'd have a way to detect which entities and revisions need updating
Indeed! That's where 📌 Version component prop definitions for SDC and Code components Active is part of the solution, or Experience Builder at least — to know when either a component has changed its "explicit input schema" (SDC: props defined in JSON Schema, block plugin: default configuration and config schema — these require an update path to be provided by the module that provides them), or when the way that a site decides to store data for an SDC (e.g. change which field type is used for storing images — this requires XB itself to figure out the update path — related: #3509115-8: Plan to allow editing props and slots for exposed code components → ).
@longwave asked what I was going to ask in #16, and #17 kind of blows my mind 🤯😄
- 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
FYI: test coverage is under way at 📌 [SPIKE] Prove that it's possible to apply block settings update paths to stored XB component trees Active , which should help vet whatever we come up here — because the test infrastructure that introduces should be trivially updatable to first expect component instance validation errors (and rendering failure) as they do now, but then after applying the update path infrastructure that this issue adds, they should expect success.
- 🇨🇭Switzerland berdir Switzerland
Interesting ideas, but as always, things get a bit more complicated with contrib, with the main challenge being release management/timing where an event invoked by core might be not be the right solution.
A practical example is when we deprecated and replaced the node_type condition plugin with the generic entity bundle plugin derivates. Core provided an upgrade path for block configs. pathauto also uses condition plugins to control which patterns apply to which entity. I did write an update function there, but there were still a bunch of people that did run into problems because they either had old default config in install profiles/modules, might have updated pathauto before core, or reimported fixed config after running updates (a surprisingly common issue).
A solution where a block plugin can trigger an update for all possibly afffected places is IMHO extremely hard, because it would require that people update all contrib and custom code at the same time as core introduces that new update, so those implementations actually get triggered. I think that's not realistic and more complex solutions like discovering those conversion handlers/plugins doesn't seem easier than each implementation writing its own update function. Maybe it could be useful to have some apis on plugins to deal with the conversion, but that won't handle a case when a whole plugin gets deprecated, I guess.
Also, what navigation does with block plugins isn't even config entities, just a single simple config file, so generic config entity based approaches wouldn't work there (fwiw, I think what navigation does is wrong, you shouldn't manage block config in simple config, but that's a different topic)
- 🇬🇧United Kingdom catch
A solution where a block plugin can trigger an update for all possibly afffected places is IMHO extremely hard, because it would require that people update all contrib and custom code at the same time as core introduces that new update, so those implementations actually get triggered.
Yes I think this would be hard, but also this is why #15-#17 are talking about implementing just in time updates - e.g. an API for dealing with old to new conversions that if necessary can run on entity load. Everywhere that needs it would need to implement that API, but it wouldn't be triggered in response to a one-time event.
The problem with that is just-in-time can easily turn into too-late when the specific update code gets removed, so we'd either need to keep these around a lot longer than we currently do for hook_update_N() or add some kind of catch-all, on-demand bulk updater along the lines of search indexing - load all the entities on a site, check if they need updating, update them if they do, but something that can be run via drush/batch as a last resort prior to a major update, not via the regular update system.
- 🇬🇧United Kingdom longwave UK
There is an additional hard problem in the case of interdependent entities: if entity type A depends on entity type B, and both receive some kind of structure update using this mechanism, which order do the updates run in? Does the update code for entity type A have to deal with the old or new structure of entity type B? Perhaps this is a hypothetical issue for now but as @berdir mentioned contrib I am sure we will run into this case sooner or later, if what we are effectively trying to here is provide long term BC for data structures.