Problem/Motivation

When upgrading from UI Patterns version 1.x to 2.x in an existing project, I successfully migrated my existing patterns using the ui-patterns:migrate Drush command (thanks to the ui_patterns_legacy submodule). However, after completing the migration, my website went down, displaying the following error:

Le site Web a rencontré une erreur inattendue.

LogicException: The database connection is not serializable. This probably means you are serializing an object that has an indirect reference to the database connection. Adjust your code so that is not necessary. Alternatively, look at DependencySerializationTrait as a temporary solution. in Drupal\Core\Database\Connection->__sleep() (line 1920 of core/lib/Drupal/Core/Database/Connection.php).

After investigating, I discovered that the error was caused by all components with props of the enum type.

Steps to reproduce

Migrating from UI patterns 1.x to UI patterns 2.x and use components with props type enum.

Proposed resolution

The EnumPropType plugin class (located in src/Plugin/UiPatterns/PropType/EnumPropType.php) should use DependencySerializationTrait trait to ensure proper serialization.

CommentFileSizeAuthor
#2 3471426-fix-ui-patterns-enum-serialization.patch1.58 KBb.khouy

Issue fork ui_patterns-3471426

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

b.khouy created an issue. See original summary.

b.khouy’s picture

b.khouy
Location 🇲🇦 Morocco
commented
StatusFileSize
new 3471426-fix-ui-patterns-enum-serialization.patch1.58 KB
b.khouy’s picture

b.khouy
Location 🇲🇦 Morocco
commented
Status: Active » Needs review
b.khouy’s picture

b.khouy
Location 🇲🇦 Morocco
commented
Issue summary: View changes
b.khouy’s picture

b.khouy
Location 🇲🇦 Morocco
commented
Assigned: b.khouy » Unassigned
pdureau’s picture

pdureau commented
Title: Enum prop type serialization issue » [2.0.0-beta2] Enum prop type serialization issue
Status: Needs review » Needs work

Thanks a lot Brahim. Can you do a MR instead of a patch?

Why would EnumPropType be the only one needing DependencySerializationTrait?

b.khouy’s picture

b.khouy
Location 🇲🇦 Morocco
commented

I'm not sure if this also affects other complex prop types, as I only have a few components in my project, and they all use primitive prop types (boolean, string, etc.) and enum prop type.
I will test with other prop types and create a merge request.

b.khouy opened merge request !190

b.khouy’s picture

b.khouy
Location 🇲🇦 Morocco
commented
Status: Needs work » Reviewed & tested by the community

I tested all existing prop types and found no serialization errors. This makes sense since, for example, the variant prop type uses enums, so fixing the issue with the enum prop type should also resolve it for the variant.

A merge request has been opened.

pdureau’s picture

pdureau commented
Assigned: Unassigned » pdureau
Status: Reviewed & tested by the community » Needs review

the variant prop type uses enums, so fixing the issue with the enum prop type should also resolve it for the variant.

I am not sure about this sentence.

I tested all existing prop types and found no serialization errors.

This is the most important, I will do the review

christian.wiedemann’s picture

christian.wiedemann commented

The trait DependencySerializationTrait solves two seralization issues. (Correct me if I am wrong.) First if the plugin uses services and second if the plugin uses the translateable markup. EnumPropType uses translateable markup so this is propable the reason for the bug because summary method uses translateable markup.

So I would move this PropTypePluginBase.

pdureau’s picture

pdureau commented
Assigned: pdureau » Unassigned
Status: Needs review » Needs work

Brahim, can you move the trait use to PropTypePluginBase?

b.khouy’s picture

b.khouy
Location 🇲🇦 Morocco
commented

@christian.wiedemann
You're right, it would be better to handle this in PropTypePluginBase, ensuring a general fix for all prop types.

@pdureau I'll take care of it.

b.khouy’s picture

b.khouy
Location 🇲🇦 Morocco
commented
Assigned: Unassigned » b.khouy
b.khouy’s picture

b.khouy
Location 🇲🇦 Morocco
commented
Assigned: b.khouy » Unassigned
Status: Needs work » Needs review
pdureau’s picture

pdureau commented
Status: Needs review » Needs work

PHPCS is KO

   | ERROR | [x] Use statements should be sorted alphabetically. The first
   |       |     wrong one is
   |       |     Drupal\Component\Plugin\Definition\PluginDefinitionInterface.
   |       |     (SlevomatCodingStandard.Namespaces.AlphabeticallySortedUses.IncorrectlyOrderedUses)

Careful ( I have rebased the branch on git.drupalcode.org, you may need to delete and repull your local)

b.khouy’s picture

b.khouy
Location 🇲🇦 Morocco
commented
Status: Needs work » Needs review
pdureau’s picture

pdureau commented
Status: Needs review » Fixed

  • pdureau committed d3910c26 on 2.0.x authored by b.khouy 
    Issue #3471426 by b.khouy: Use DependencySerializationTrait trait on...
pdureau’s picture

pdureau commented
Status: Fixed » Closed (fixed)