Problem/Motivation

We avoid serializing the database connection, if it is serialized we throw an exception.
If you're building a form that needs a database queue instance, you cannot inject it as it will result in 'The database connection is not serializable' as the queue doesn't manage the connection reference on sleep/wakeup.

Proposed resolution

Make Queue use DependencySerializationTrait

Remaining tasks

Review

User interface changes

None

API changes

None

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Bug because injection throws Exception
Issue priority Normal
Disruption None
CommentFileSizeAuthor
#19 queue-serialize-boom-2481729.pass_.patch3.45 KBlarowlan
#12 queue-serialize-boom-2481729.pass_.patch3.45 KBlarowlan
#12 queue-serialize-boom-2481729.fail_.patch3.45 KBlarowlan
#12 interdiff.txt523 byteslarowlan
#5 queue-serialize-boom-2481729.pass_.patch3.45 KBlarowlan
#5 queue-serialize-boom-2481729.fail_.patch2.84 KBlarowlan
#3 queue-injection.patch624 byteslarowlan
queue-injection.patch1.25 KBlarowlan
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented
Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, queue-injection.patch, failed testing.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented
Status: Needs work » Needs review
FileSize
queue-injection.patch624 bytes

meh

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented
FileSize
queue-serialize-boom-2481729.fail_.patch2.84 KB
queue-serialize-boom-2481729.pass_.patch3.45 KB

failing test

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented
Issue summary: View changes

The last submitted patch, 5: queue-serialize-boom-2481729.fail_.patch, failed testing.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost at Skilld commented
Status: Needs review » Reviewed & tested by the community

nice

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/modules/system/src/Tests/Queue/QueueSerializationTest.php
@@ -0,0 +1,108 @@
+use Drupal\Core\DependencyInjection\DependencySerializationTrait;
...
+class QueueSerializationTest extends KernelTestBase implements FormInterface {
+
+  use DependencySerializationTrait;

Why does this class need this trait?

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented
Status: Needs work » Reviewed & tested by the community

Because it's a form as well as a test, but it doesn't extend from FormBase

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented

To extrapolate - the intent of the test is to check form serializing, but because the form is also a test, it gets a $this->container property, which can't be serialized, so we add that trait to get past that.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented
Status: Reviewed & tested by the community » Needs review
FileSize
interdiff.txt523 bytes
queue-serialize-boom-2481729.fail_.patch3.45 KB
queue-serialize-boom-2481729.pass_.patch3.45 KB

demonstrating reason for the trait
interdiff is between these two patches

Status: Needs review » Needs work

The last submitted patch, 12: queue-serialize-boom-2481729.pass_.patch, failed testing.

Status: Needs work » Needs review

larowlan queued 12: queue-serialize-boom-2481729.pass_.patch for re-testing.

The last submitted patch, 12: queue-serialize-boom-2481729.fail_.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 12: queue-serialize-boom-2481729.pass_.patch, failed testing.

Status: Needs work » Needs review

larowlan queued 12: queue-serialize-boom-2481729.pass_.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 12: queue-serialize-boom-2481729.pass_.patch, failed testing.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented
Status: Needs work » Needs review
FileSize
queue-serialize-boom-2481729.pass_.patch3.45 KB

no idea

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented
Status: Needs review » Reviewed & tested by the community

Same file passes, back to rtbc

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed 843670c and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation to the issue summary.

  • alexpott committed 843670c on 8.0.x 
    Issue #2481729 by larowlan: Cannot inject DatabaseQueue instances into...

Status: Fixed » Closed (fixed)

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