Problem/Motivation

After #2450993: Rendered Cache Metadata created during the main controller request gets lost is being committed, Collect (contrib module) was broken and I had to implement __sleep() method to avoid serialisation of plugin collections. We can avoid this by unsetting the keys in __sleep() in ConfigEntityBase.

The fix was taken from page_manager: http://cgit.drupalcode.org/page_manager/commit/?id=389e915

Proposed resolution

This ConfigEntityBase kind of knows about the plugin collections, we could possibly implement this in a generic way in __sleep().

Challenge: What we are missing is which property the plugin collection is stored in. But we could consider to just loop over it and compare until we find the matching object?

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#4 2530568-sleep-4.patch940 bytestim.plunkett
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Issue summary: View changes

Updated the issue summary a bit, this is the collect issue: #2527148: Adding processors to the model is broken (was in the sidebar only)

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Title: ConfigEntityBase should automatically unset keys in __sleep() » ConfigEntityBase should automatically unset plugin collection keys in __sleep()
Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented

Alternatively, we could make it so that plugin collections can be safely serialized. We had problems with this before but did in a different way back then.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett at Acquia commented
Status: Active » Needs review
FileSize
2530568-sleep-4.patch940 bytes

Maybe this?

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented

Looks great to me, could use a comment or two to explain what we're doing.

Thinking about how to test this.. we could revert a fix in one of the contrib projects where we had to fix it and make sure it works with this now out of the box?

A test in core is going to be tricky I think but at least we know that nothing seems to be broken..

mr.baileys’s picture

mr.baileys
Primary language Dutch
Location 🇧🇪 (Ghent)
CreditAttribution: mr.baileys at Calibrate commented

This seems to have been implemented in #2650588: Entities with plugin collections should be updated before serialization, so this issue can be closed/marked duplicate?

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs review » Closed (duplicate)

Yes.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett at Acquia commented

Whoops, lost track of this issue and opened the other. Thanks @mr.baileys and @Berdir!