Do not use queue name in Worker::processQueue($queue, $options);

[juriansluiman]

Proposal: why not move the queue instantiation from the worker to the controller, which feels more logical to me.

Current controller:

public function __construct(WorkerInterface $worker)
{ /* ... */ }

public function processAction()
{
    // work
    $this->worker->processQueue($queueName, $options);
    // work
}

Current Worker:

public function __construct(QueuePluginManager $queuePluginManager)
{ /* ... */ }

public function processQueue($queueName, $options)
{
    $queue = $this->queuePluginManager->get($queueName);
    //work
}

NEW PROPOSAL Controller:

public function __construct(WorkerInterface $worker, QueuePluginManager $queuePluginManager)
{ /* ... */ }

public function processAction()
{
    // work
    $queue = $this->queuePluginManager->get($queueName);
    $this->worker->processQueue($queue, $options);
    // work
}

Current Worker:

// No constructor!

public function processQueue(QueueInterface $queue, $options)
{
    //work
}

Ping @basz @bakura10; not started this because many tests should be rewritten and obviously clashes now with the worker listeners PR. So if agreed, I'll do this for v0.4 just after Bas's PR is merged.

[bakura10]

I like the idea !

Sorry, I have nothing more to say about this, but it sounds logicial, processQueue sounds like it expects a queue.

[basz]

I like this. And https://github.com/juriansluiman/SlmQueue/pull/109 would actually benefit from this if we can get this sooner.

The task of de/attaching configured strategies listeners should be somewhere in the controller, no? The controller could make sure the InterruptStrategy is always attached even when it's not configured.

Currently I have an ugly (because it introduces complexity) 'internal' WorkerInitializerListenerAggregate (attached in the worker factory) combined with an ListenerEvents class (to hide it from userland). It works, but is/feels ugly.

[juriansluiman]

The task of de/attaching configured strategies listeners should be somewhere in the controller, no?

@basz still struggling with the current implementation from #109 too. But let's discuss this issue in the listeners PR, as I don't think this change will solve that problem.

[juriansluiman]

OK code updated; 16 tests have errors now, will fix them later.

[juriansluiman]

Tests pass now; good to merge. Just added the changelog stupidity (/jk) to the PR as well, 75833b2 was already good to merge :)

[bakura10]

Perfect. I'm waiting for travis.

No adapter needs changes?

[bakura10]

SlmQueue really looks near perfect. That's bad. People will soon complain because "what, this library hasn't been updated for 6 months, is it dead?"

[juriansluiman]

Only the controller, as the factory is defined per adapter. But because the controller does not change between the adapters, so I think we can work towards a same solution as done with the worker.

/edit:

1. juriansluiman/SlmQueueBeanstalkd#39
2. juriansluiman/SlmQueueSqs#19
3. juriansluiman/SlmQueueDoctrine#71