Problem/Motivation

There is only one queue backend in core, QueueDatabaseFactory. This provides a get() method which must be implemented for QueueFactory to be able to construct a queue.

Other queue backends presumably implement this as well otherwise the code would not work, but this should be done via an interface.

Adding an interface would also help in the parent issue #3416357: Convert QueueFactory to use a service locator where we want to inject only queue backend services into QueueFactory; without an interface (or perhaps base class) there is no way of identifying them.

Steps to reproduce

Proposed resolution

Add a QueueFactoryInterface and implement it in QueueDatabaseFactory, similar to CacheFactoryInterface.

Notify contrib queue backends in QueueFactory if the interface is not implemented.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3416826

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:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

longwave created an issue. See original summary.

longwave’s picture

Status: Active » Needs review
Issue tags: +Needs change record
smustgrave’s picture

Status: Needs review » Needs work

Left a small nitpicky but MR appears to have a failure.

longwave’s picture

Status: Needs work » Needs review
Issue tags: -Needs change record

Unfortunately we can't add a return type because this is already extended in contrib.

Added a change record and improved the deprecation notice.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

Ah yea see what you mean about not being able to use the typehints. In that case rest looks fine.

quietone’s picture

I'm triaging RTBC issues. I read the IS, the comments and the MR. I didn't find any unanswered questions or other work to do.

Leaving at RTBC;

solideogloria’s picture

Status: Reviewed & tested by the community » Needs review
Related issues: +#2824389: Add a QueueFactoryInterface

Have a look at this issue, figure out which should be closed as a duplicate, and check if any changes from the duplicate issue should be incorporated.

longwave credited fgm.

longwave’s picture

Status: Needs review » Reviewed & tested by the community

Thanks, I did not see that issue earlier. I think that one can be closed as duplicate because:

  • That one defines a new (but unused) queue.memory service, which I don't see the point in
  • That one also adds a return type, but we can't do that without breaking backward compatibility

Otherwise the implementations are the same except for comments. Crediting fgm here for the work done over there.

solideogloria’s picture

Okay, thanks. I closed it as a duplicate.

Spokje’s picture

Issue tags: +Symfony 7

  • catch committed db20bb73 on 10.3.x
    Issue #3416826 by longwave, smustgrave, solideogloria, fgm: Queue...

  • catch committed 2281800f on 11.x
    Issue #3416826 by longwave, smustgrave, solideogloria, fgm: Queue...
catch’s picture

Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 11.x and cherry-picked to 10.3.x, thanks!

I asked @longwave about potentially backporting this to 10.2 so contrib could use it earlier for 10/11 compatibility, but as far as we know the only contrib module affected is redis, so should probably see if it's a real issue before backporting API additions to a patch release.

longwave’s picture

As there is no interface it is hard to search contrib but I did find QueueFactory references in the following (amongst a lot of false positives):

https://www.drupal.org/project/aws_sqs
https://www.drupal.org/project/beanstalkd
https://www.drupal.org/project/mongodb
https://www.drupal.org/project/rabbitmq
https://www.drupal.org/project/redis

catch’s picture

Version: 10.3.x-dev » 10.2.x-dev
Status: Fixed » Patch (to be ported)

That's enough I think we should backport just the interface to 10.2.x.

longwave’s picture

Status: Patch (to be ported) » Needs review

Adds the interface, applies it to QueueDatabaseFactory, also backports the comment fixes.

catch’s picture

Status: Needs review » Reviewed & tested by the community

That looks like just the right amount to backport.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

Can we add typehints (both param and return) to the interface and implementation. I think adding a new interface with typehints is more valuable than the backport to 10.2.x. Given there is no interface I think adding typehints should be allowed.

longwave’s picture

Status: Needs work » Reviewed & tested by the community

Can't be done without breaking at least one contrib module, and perhaps custom code, as per previous comment on the MR.

If we add types to the interface, we need to add them to QueueDatabaseFactory. queue_unique extends QueueDatabaseFactory and so that will break: https://git.drupalcode.org/project/queue_unique/-/blob/8.x-2.x/src/Uniqu...

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed a5826e6 and pushed to 10.2.x. Thanks!

  • alexpott committed a5826e68 on 10.2.x
    Issue #3416826 by longwave, catch, smustgrave, solideogloria, fgm: Queue...

Status: Fixed » Closed (fixed)

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