pass dispatchAfterCommit into custom worker constructor

[oprudkyi]

align constructors for custom workers and RabbitMQQueue

so there no need to map somehow constructor in case of custom worker

'worker' => env('RABBITMQ_WORKER', App\Services\CustomRabbitMQQueue::class),

...

use VladimirYuldashev\LaravelQueueRabbitMQ\Queue\RabbitMQQueue;

class CustomRabbitMQQueue extends RabbitMQQueue
{

[M-Porter]

I believe the intention for this was for custom workers to extend Queue and implement QueueContract.

That being said, it is a good idea to make those calls consistent.

[oprudkyi]

Hi @M-Porter
yes,

we added some tracing into RabbitMQQueue by inheriting it and incompatible constructor make things clumsy

[khepin]

In its current form, this would be a breaking change and will therefore have to wait on a major version release.
If we wanted to allow a similar behavior prior to that, we'd need to offer a different way of achieving the same result.

[oprudkyi]

@khepin well, 13.0.2 also introduces incompatibility with some versions of laravel 9
and as for me it's fix for broken compatibility in v13 , basically the same what 13.0.1 did for 13.0.0 https://github.com/vyuldashev/laravel-queue-rabbitmq/blob/master/CHANGELOG-13x.md

[oprudkyi]

@khepin what if there will be some new option

'worker' => env('RABBITMQ_WORKER', App\Services\CustomRabbitMQQueue::class),
'custom_worker_type' =>  'with_dispatch_after_commit',

this will allow to have fix without introduction of breaking changes

[adm-bome]

Does this work ????

• No library code adjustments, only adjustments in your own Class and config.

<?php

namespace App\Queue;

use Illuminate\Support\Arr;
use VladimirYuldashev\LaravelQueueRabbitMQ\Queue\RabbitMQQueue;

class CustomRabbitMQQueue extends RabbitMQQueue
{

    public function __construct(
        AbstractConnection $connection,
        string $default,
        array $options = []
    ) {
        $dispatchAfterCommit = Arr::pull($options, 'after_commit', false);
        parent::__construct($connection, $default, $dispatchAfterCommit, $options);
    }

and in the connection config ( move the after_commit option to the queue options)

#/config/queue.php
'connections' => [
    // ...

    'rabbitmq' => [
        // ...
        
        // 'after_commit' => true
        'options' => [

            'queue' => [
                // ...
                'after_commit' => true
                'job' => \App\Queue\Jobs\RabbitMQJob::class,
            ],
        ],
    ],

    // ... 

[adm-bome]

@khepin
And I agree to align the constructor calls.

When dispatchAfterCommit was introduced into Laravel and eventually was implemented into this library... the contructor calls had to be already aligned. This library checks if the Worker class (Queue) is an instance of RabibitMQQueue:class, so the call is wrong because the constructor of the RabibitMQQueue:class is already different.

If people are depending on the constructor of the RabibitMQQueue:class, people had already noticed issues and had to correct this with a sort of sollution I presented above.

[oprudkyi]

Hi @adm-bome
everything works (currently we're using fork, I think using own RabbitMQConnector is even more simpler option ) , but this PR will allow to avoid introducing yet another dependency in our code

[adm-bome]

I think using own RabbitMQConnector is even more simpler option

Not really, if you want to replace all classes then use a fork or write your own implementation of a driver 😄

The contructors calls must align with the default constructor of RabibitMQQueue:class.

As i mentioned above _the contructor calls had to be already aligned_ and your problem should have never emerge.
When this was noticed in the beginning, when review-ing the initial merge of the feature dispatchAfterCommit, all was good for now.

[adm-bome]

Closing this, because the connector was refactored and the Factory uses a different approach for the constructor (one single config object)