Test issue for Make ContentEntityDatabaseStorage handle changes in the entity schema definition

Pushed to plach's sandbox - branch is called: 2298525-alexpott

Bot check for the next sub-issue I plan on opening.

Here's the next step I'd like us to focus our reviews on after #2330121: Replace ContentEntityDatabaseStorage::getSchema() with a new EntityTypeListenerInterface implemented by SqlContentEntityStorageSchema. This patch includes that one in order to get a bot check. Once it passes, I'll open the issue for just its own piece.

[Edit: here's that issue: #2333113: Add an EntityDefinitionUpdateManager so that entity handlers can respond (e.g., by updating db schema) to code updates in a controlled way (e.g., from update.php)]

Thanks for the reroll in #57. Rerolling #59 on top of #60 will also be a bit tedious, but I'll work on that, and will post it here once I'm done.

This is a reroll of #59 on top of #60. The "-do-not-test" one doesn't include any of #60. The "-full" one includes everything, but resolves the various conflicts introduced by #60.

Re #62, great! If your new patch is based on #59, then when you post yours, please post an interdiff relative to #59, and hopefully that interdiff won't be too hard to apply to these patches here.

Just a reroll of #72.

Merged in changes from the history of #2333113: Add an EntityDefinitionUpdateManager so that entity handlers can respond (e.g., by updating db schema) to code updates in a controlled way (e.g., from update.php). Also reordered some functions in SqlContentEntityStorageSchema to match their order in HEAD to make the diff easier to review.

The "-do-not-test" patch is rebased on top of #2333113-58: Add an EntityDefinitionUpdateManager so that entity handlers can respond (e.g., by updating db schema) to code updates in a controlled way (e.g., from update.php), so is the last step once that lands.

+++ b/core/lib/Drupal/Core/Entity/EntityManagerInterface.php
@@ -8,11 +8,12 @@
+interface EntityManagerInterface extends PluginManagerInterface, EntityTypeListenerInterface, FieldStorageDefinitionListenerInterface {

This really doesn't seem right to me. Why does EntityManagerInterface know about FieldStorageDefinition anything?

Can't we please create a new class and move all of this stuff out? I don't know at what point EntityManager became the dumping ground for all of this code that only pertains to half of entity types, but this is getting out of control.

EDIT: Ugh, just realized I'm posting to a test-only issue. I'll track down the real one tomorrow...

Ugh, just realized I'm posting to a test-only issue. I'll track down the real one tomorrow...

The real issue with that change is #2333113: Add an EntityDefinitionUpdateManager so that entity handlers can respond (e.g., by updating db schema) to code updates in a controlled way (e.g., from update.php). However, realize that EntityManagerInterface already has a ton of methods that deal with field definitions. So if you want to decouple everything field-related from EntityManager, please open a new issue for that rather than holding up that one. However, I'll point out that isFieldable() is a method of EntityTypeInterface, so I don't think it's entirely unreasonable for EntityManager to deal with field definitions in addition to entity type definitions, but if there's a good way to partition EntityManager into smaller parts, I'm ok with that too.

Removing the system_system_info_alter() change as out of scope for this issue. Seems like can be a separate issue and post-beta. Elsewhere we decided to not add hasData() to entity storage handlers, so if we want to change our mind on that, let's do it in a dedicated issue.

Also fixed a stray merge conflict.

Merged changes from #2333113: Add an EntityDefinitionUpdateManager so that entity handlers can respond (e.g., by updating db schema) to code updates in a controlled way (e.g., from update.php) and fixed #82.

Merged changes from #2333113: Add an EntityDefinitionUpdateManager so that entity handlers can respond (e.g., by updating db schema) to code updates in a controlled way (e.g., from update.php), which is now RTBC. And fixed the cause of #83's exceptions (system.module's hunk in the interdiff).

-1 on the last interdiff: blocking module uninstallation was planned with @alexpott in the last hard problems discussion about this topic we had in Austin. This was meant to prevent modules from breaking while we unify the field purging code.

Makes sense, but seems a different scope than #2337927: SqlContentEntityStorage::onFieldStorageDefinition(Create|Update|Delete)() is broken for base fields, so I opened #2338873: Modules providing non-configurable field storage definitions can be uninstalled, leaving orphaned unpurged data. Maybe having it isolated will also help with troubleshooting those failures?

Heh, I just happened to do this before seeing your comment, so here's my reroll. Curious if yours ends up being identical or not.