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.
Merge request link
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Issue fork drupal-3416826
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:
Comments
Comment #3
longwaveComment #4
smustgrave CreditAttribution: smustgrave at Mobomo commentedLeft a small nitpicky but MR appears to have a failure.
Comment #5
longwaveUnfortunately we can't add a return type because this is already extended in contrib.
Added a change record and improved the deprecation notice.
Comment #6
smustgrave CreditAttribution: smustgrave at Mobomo commentedAh yea see what you mean about not being able to use the typehints. In that case rest looks fine.
Comment #7
quietone CreditAttribution: quietone at PreviousNext commentedI'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;
Comment #8
solideogloria CreditAttribution: solideogloria commentedHave 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.
Comment #10
longwaveThanks, I did not see that issue earlier. I think that one can be closed as duplicate because:
Otherwise the implementations are the same except for comments. Crediting fgm here for the work done over there.
Comment #11
solideogloria CreditAttribution: solideogloria commentedOkay, thanks. I closed it as a duplicate.
Comment #12
SpokjeComment #15
catchCommitted/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.
Comment #16
longwaveAs 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
Comment #17
catchThat's enough I think we should backport just the interface to 10.2.x.
Comment #20
longwaveAdds the interface, applies it to QueueDatabaseFactory, also backports the comment fixes.
Comment #21
catchThat looks like just the right amount to backport.
Comment #22
alexpottCan 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.
Comment #23
longwaveCan'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...Comment #24
alexpottCommitted a5826e6 and pushed to 10.2.x. Thanks!