8.7.0-rc1 database updates fail on media_library configuration dependencies due to missing form and view modes [#3051826]

Problem/Motivation

(Why the issue was filed, steps to reproduce the problem, etc.)

While attempting to update to core 8.7 rc1 I am running into the following fatal errors related to media_library configuration:


Error: Call to a member function getConfigDependencyName() on null in /var/www/<project>/docroot/core/lib/Drupal/Core/Entity/EntityDisplayBase.php on line 314 #0 /var/www/<project>/docroot/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php(319): Drupal\Core\Entity\EntityDisplayBase->calculateDependencies()
#1 /var/www/<project>/docroot/core/lib/Drupal/Core/Entity/EntityDisplayBase.php(266): Drupal\Core\Config\Entity\ConfigEntityBase->preSave(Object(Drupal\Core\Config\Entity\ConfigEntityStorage))
#2 /var/www/<project>/docroot/core/lib/Drupal/Core/Entity/EntityStorageBase.php(490): Drupal\Core\Entity\EntityDisplayBase->preSave(Object(Drupal\Core\Config\Entity\ConfigEntityStorage))
#3 /var/www/<project>/docroot/core/lib/Drupal/Core/Entity/EntityStorageBase.php(445): Drupal\Core\Entity\EntityStorageBase->doPreSave(Object(Drupal\Core\Entity\Entity\EntityFormDisplay))
#4 /var/www/<project>/docroot/core/lib/Drupal/Core/Config/Entity/ConfigEntityStorage.php(263): Drupal\Core\Entity\EntityStorageBase->save(Object(Drupal\Core\Entity\Entity\EntityFormDisplay))
#5 /var/www/<project>/docroot/core/lib/Drupal/Core/Entity/EntityBase.php(394): Drupal\Core\Config\Entity\ConfigEntityStorage->save(Object(Drupal\Core\Entity\Entity\EntityFormDisplay))
#6 /var/www/<project>/docroot/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php(613): Drupal\Core\Entity\EntityBase->save()
#7 /var/www/<project>/docroot/core/modules/media_library/media_library.module(281): Drupal\Core\Config\Entity\ConfigEntityBase->save()
#8 /var/www/<project>/docroot/core/modules/media_library/media_library.post_update.php(20): _media_library_configure_form_display(Object(Drupal\media\Entity\MediaType))
#9 /var/www/<project>/vendor/drush/drush/src/Commands/core/UpdateDBCommands.php(279): media_library_post_update_display_modes(Array)
#10 /var/www/<project>/vendor/drush/drush/includes/batch.inc(235): Drush\Commands\core\UpdateDBCommands->updateDoOnePostUpdate('media_library_p...', Object(DrushBatchContext))
#11 /var/www/<project>/vendor/drush/drush/includes/batch.inc(183): _drush_batch_worker()
#12 /var/www/<project>/vendor/drush/drush/includes/batch.inc(95): _drush_batch_command('677')
#13 /var/www/<project>/vendor/drush/drush/src/Commands/core/UpdateDBCommands.php(148): drush_batch_command('677')
#14 [internal function]: Drush\Commands\core\UpdateDBCommands->process('677', Array)
#15 /var/www/<project>/vendor/consolidation/annotated-command/src/CommandProcessor.php(257): call_user_func_array(Array, Array)
#16 /var/www/<project>/vendor/consolidation/annotated-command/src/CommandProcessor.php(212): Consolidation\AnnotatedCommand\CommandProcessor->runCommandCallback(Array, Object(Consolidation\AnnotatedCommand\CommandData))
#17 /var/www/<project>/vendor/consolidation/annotated-command/src/CommandProcessor.php(178): Consolidation\AnnotatedCommand\CommandProcessor->validateRunAndAlter(Array, Array, Object(Consolidation\AnnotatedCommand\CommandData))
#18 /var/www/<project>/vendor/consolidation/annotated-command/src/AnnotatedCommand.php(302): Consolidation\AnnotatedCommand\CommandProcessor->process(Object(Symfony\Component\Console\Output\ConsoleOutput), Array, Array, Object(Consolidation\AnnotatedCommand\CommandData))
#19 /var/www/<project>/vendor/symfony/console/Command/Command.php(255): Consolidation\AnnotatedCommand\AnnotatedCommand->execute(Object(Drush\Symfony\LessStrictArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#20 /var/www/<project>/vendor/symfony/console/Application.php(978): Symfony\Component\Console\Command\Command->run(Object(Drush\Symfony\LessStrictArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#21 /var/www/<project>/vendor/symfony/console/Application.php(255): Symfony\Component\Console\Application->doRunCommand(Object(Consolidation\AnnotatedCommand\AnnotatedCommand), Object(Drush\Symfony\LessStrictArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#22 /var/www/<project>/vendor/symfony/console/Application.php(148): Symfony\Component\Console\Application->doRun(Object(Drush\Symfony\LessStrictArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#23 /var/www/<project>/vendor/drush/drush/src/Runtime/Runtime.php(112): Symfony\Component\Console\Application->run(Object(Drush\Symfony\LessStrictArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#24 /var/www/<project>/vendor/drush/drush/src/Runtime/Runtime.php(41): Drush\Runtime\Runtime->doRun(Array)
#25 /var/www/<project>/vendor/drush/drush/drush.php(66): Drush\Runtime\Runtime->run(Array)
#26 /var/www/<project>/vendor/drush/drush/includes/preflight.inc(17): require('/var/www/<project>...')
#27 phar:///usr/local/bin/drush/bin/drush.php(141): drush_main()
#28 /usr/local/bin/drush(10): require('phar:///usr/loc...')
#29 {main}
Error: Call to a member function getConfigDependencyName() on null in Drupal\Core\Entity\EntityDisplayBase->calculateDependencies() (line 314 of /var/www/<project>/docroot/core/lib/Drupal/Core/Entity/EntityDisplayBase.php).

I found a related core bug https://www.drupal.org/project/drupal/issues/2741429 that, when patched, does change the output to be much more helpful but doesn't "fix" the underlying problem here.

As far as I can tell, the config for my Document media type is either corrupted or missing the proper media library configuration based on the output I am getting from the patch in above:


 [error]  Cannot add the config entity dependency with ID 'media.media_library' of type 'entity_form_mode' to core.entity_form_display.media.document.media_library as it does not exist 
 [error]  Update failed: media_library_post_update_display_modes 
 [error]  Update aborted by: media_library_post_update_display_modes 
 [error]  Finished performing updates.

I am running a database update prior to configuration import as part of the 8.7 update. I have tried created a database update to just import the configuration for core.entity_form_display.media.document.media_library so it is present when the database update media_library_post_update_display_modes runs, however this seems to be ineffective.

Proposed resolution

Create the view mode and form mode before creating the displays.

Remaining tasks

(reviews needed, tests to be written or run, documentation to be written, etc.)

Unknown

Comment	File	Size	Author
#7	3051826-media-6-interdiff.txt	1.45 KB	tim.plunkett
#7	3051826-media-6.patch	3.18 KB	tim.plunkett
#7	8.8.x: PHP 7.1 & MySQL 5.7 Patch Failed to Apply
#4	3051826-media-4.patch	1.74 KB	tim.plunkett
#4	8.7.x: PHP 7.1 & MySQL 5.7 26,305 pass, 2 fail

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

30 April 2019 at 21:08

mikemadison created an issue. See original summary.

Comment #2

mikemadison CreditAttribution: mikemadison at Acquia commented 30 April 2019 at 21:09

Comment #3

tim.plunkett

he/him

English

Philadelphia

CreditAttribution: tim.plunkett at Acquia commented 30 April 2019 at 21:13

This code assumes that core.entity_view_mode.media.media_library exists (and it doesn't).

Comment #4

tim.plunkett

he/him

English

Philadelphia

CreditAttribution: tim.plunkett at Acquia commented 30 April 2019 at 21:18

Version:	8.7.x-dev	» 8.8.x-dev
Priority:	Normal	» Critical
Status:	Active	» Needs review

File	Size
3051826-media-4.patch	1.74 KB
8.7.x: PHP 7.1 & MySQL 5.7 26,305 pass, 2 fail

The test added for this update code is MediaLibraryUpdateFormAndViewDisplaysTest

In it, it first sets up a view mode and form mode. Then tests the update path.
That's cheating! :)

This should fully reproduce the error in the issue summary.

Comment #5

xjm

she/her

English

CreditAttribution: xjm at Acquia commented 30 April 2019 at 21:27

Issue tags:

+8.7.0 update

Comment #6

30 April 2019 at 22:01

Status:

Needs review

» Needs work

The last submitted patch, 4: 3051826-media-4.patch, failed testing. View results

Comment #7

tim.plunkett

he/him

English

Philadelphia

CreditAttribution: tim.plunkett at Acquia commented 30 April 2019 at 23:38

Title:	8.7rc1 Database Updates Fail on media_library Configuration Dependencies	» 8.7.0-rc1 database updates fail on media_library configuration dependencies due to missing form and view modes
Status:	Needs work	» Needs review

File	Size
3051826-media-6.patch	3.18 KB
8.8.x: PHP 7.1 & MySQL 5.7 Patch Failed to Apply
3051826-media-6-interdiff.txt	1.45 KB

Here's the fix

Comment #8

tim.plunkett

he/him

English

Philadelphia

CreditAttribution: tim.plunkett at Acquia commented 30 April 2019 at 23:44

Issue summary:	View changes
Related issues:		+#2988433: Automatically create and configure Media Library view and form displays

For the record, #2988433: Automatically create and configure Media Library view and form displays was the issue that added this.

Comment #9

larowlan

🇦🇺🏝.au GMT+10

CreditAttribution: larowlan at PreviousNext commented 30 April 2019 at 23:48

Status:

Needs review

» Reviewed & tested by the community

+++ b/core/modules/media_library/media_library.post_update.php
@@ -12,6 +14,29 @@
+  $values = [
+    'id' => 'media.media_library',
+    'targetEntityType' => 'media',
+    'label' => t('Media library'),
+    'dependencies' => [
+      'enforced' => [
+        'module' => [
+          'media_library',
+        ],
+      ],
+      'module' => [
+        'media',
+      ],
+    ],
+  ];
+  if (!EntityViewMode::load('media.media_library')) {
+    EntityViewMode::create($values)->save();
+  }
+  if (!EntityFormMode::load('media.media_library')) {
+    EntityFormMode::create($values)->save();
+  }

pity we don't have #2367933: Move entity_get_(form_)display() to the entity display repository

RTBC if bot agrees

Comment #10

tim.plunkett

he/him

English

Philadelphia

CreditAttribution: tim.plunkett at Acquia commented 1 May 2019 at 01:53

Wouldn't help this part of the code at all, this is modes not displays :)

Comment #11

larowlan

🇦🇺🏝.au GMT+10

CreditAttribution: larowlan at PreviousNext commented 1 May 2019 at 02:15

🤦‍♂️

Comment #12

1 May 2019 at 02:32

xjm committed 33bd920 on 8.8.x

Issue #3051826 by tim.plunkett, mikemadison: 8.7.0-rc1 database updates...

Comment #13

1 May 2019 at 02:33

xjm committed 90a748f on 8.7.x

Issue #3051826 by tim.plunkett, mikemadison: 8.7.0-rc1 database updates...

Comment #14

xjm

she/her

English

CreditAttribution: xjm at Acquia commented 1 May 2019 at 02:34

Status:

Reviewed & tested by the community

» Fixed

So core.entity_form_mode.media.media_library and core.entity_view_mode.media.media_library are both install config for Media library, dating to the week the module was added. To not exist, they'd have to have been deleted from the site manually? (Presumably that's why they exist in the DB dump for the upgrade path test). And the Media Library wouldn't really work, if I understand this correctly. Not that that is a reason to have a broken update, of course; update functions should be defensive and we shouldn't blindly assume config exists on a site.

The alternative is skipping the update if the modes don't already exist, but that seems worse to me. The worst case of this fix is that someone again gets form and view modes they deliberately deleted from a site. However, th will only happen once when this post-update runs, and the module's after all experimental. So that seems OK to me.

This reminds me of some existing previous issues around how Media and Media Library rely on the existence of things site owners can easily delete through the UI.

Anyway, committed to 8.8.x and cherry-picked to 8.7.x to avoid a broken DB update, which is always the last thing we want to have happen. Thanks @mikemadison for reporting this issue!

Comment #15

pandaski CreditAttribution: pandaski at govCMS (Australian Government Department of Finance) commented 1 May 2019 at 06:25

Automated testing is broken with this commit?

https://www.drupal.org/pift-ci-job/1277504

Comment #16

larowlan

🇦🇺🏝.au GMT+10

CreditAttribution: larowlan at PreviousNext commented 1 May 2019 at 08:12

Looks to be a random fail @Joseph Zhao

Comment #17

xjm

she/her

English

CreditAttribution: xjm at Acquia commented 1 May 2019 at 19:31

Yep, CKEditorIntegrationTest is a frequent visitor to my inbox.

Comment #18

mikemadison CreditAttribution: mikemadison at Acquia commented 1 May 2019 at 20:04

confirming that the patch in #7 fixed my issue. thanks for the quick turn around!

Comment #19

pandaski CreditAttribution: pandaski at govCMS (Australian Government Department of Finance) commented 1 May 2019 at 22:55

Thanks, all good now

Comment #20

15 May 2019 at 22:59

Status:

Fixed

» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

Comment #21

pameeela CreditAttribution: pameeela as a volunteer commented 16 May 2019 at 22:19

@mikemadison Thank you for posting this issue. If you'd like to help us make sure the 8.8.0 update is as smooth as possible, please consider signing up for the beta testing program at https://goo.gl/forms/bMBTMRSY3sKEscUJ3