Plugin design

[merlindorin]

Feature

Add the ability to register one or more plugins (like retry, unique, etc.) to a worker.

Complexity

The design should be very simple because the registration of a plugin is nothing more than a register method that let the plugin to register a bunch of listener.

Reason

With the actual design (really flexible, thanks to events), it's possible to do everything outside of php-resque. The reason of a pluggable design is to let the community the ability to create plugins with a real simple usage.

Sample

In one of my last sample project, I show how to inject a container into a job using events. If we have a pluggable interface, people who want ioc just have to register php-resque-plugin-container-aware like that :

Resque\Worker::register(new ContainerAwarePlugin(new Pimple\Container()));
Resque\Worker::register(new RetryPlugin());
Resque\Worker::register(new LonerPlugin());

[scones]

perhaps also https://github.com/mjphaynes/php-resque/issues/77

for custom shutdown handling

[scones]

Thinking about this a bit more: I'd prefer this being done on the worker instance level, not the class.

Easier to test, easier to mock.

[merlindorin]

Naive implementation (I didn't check it), what do you think ?

diff --git c/src/Resque/Worker.php w/src/Resque/Worker.php
index 19da728..8cbafbc 100644
--- c/src/Resque/Worker.php
+++ w/src/Resque/Worker.php
@@ -11,7 +11,10 @@

 namespace Resque;

+use http\Env\Response;
 use Resque\Helpers\Stats;
+use Resque\Plugin\RegisterInterface;
+use Resque\Plugin\UnRegisterInterface;

 /**
  * Resque worker class
@@ -135,6 +138,21 @@ class Worker
      */
     protected $logger = null;

+    public static $plugins = array();
+
+    public static function registerPlugin($plugin) {
+        if($plugin instanceof RegisterInterface) {
+            $plugin->register();
+        }
+        self::$plugins[get_class($plugin)] = $plugin;
+    }
+
+    public static function unregisterPlugin($plugin) {
+        if($plugin instanceof UnRegisterInterface) {
+            $plugin->unregister();
+        }
+        unset(self::$plugins[get_class($plugin)]);
+    }
     /**
      * Get the Redis key
      *

And a sample usage : Eoko.zip

[merlindorin]

As I said... I don't think this feature should be more complicated

[scones]

This is on class level again (static)

also the Response is unused

i'd rather put down the plugin on every Worker. This is a little overhead for multi worker environments in the future, but only in the initialization phase.

I am just still trying to wrap my head around this line: https://github.com/mjphaynes/php-resque/blob/935824e5f13afa8824b980cc47934d37ebdd6e22/src/Resque/Event.php#L90 i guess this has to be patched for that as well, to prevent multible executions (again with multi worker environments).

---

i guess i am thinking to complicated again. i will have a look at your solution :)

[merlindorin]

I think I see your problem. You want to create multilpe worker using $worker = new Resque\Worker($queue, $blocking); $worker->work(); multiple time?

If it's that, in a docker environment, the best practice is to have a single process / worker on each container and let swarm, kubernetes or other the ability to scale by creating new container.

If I'm right, class level is not a problem. If not, can you give me more information for your use case? (or a sample <3)

[scones]

My train of thought was the following:

assumption:

• it is hard or near impossible to write a test for method call including a static method invocation (in php) without testing the whole framework

conclusion:

• one should avoid static methods
• not introduce new static methods

conclusion:

• plugins should be registered on the instance, not the class

assumption 2 (#56):

• there is an request for COUNT to start several workers at once.

conclusion:

• having the plugin registration on instance level yields several registered plugins, which yields several invocations

• as plugins shall use the hooks to communicate with the library framework, the events for the plugins will get duplicated and executed more than once

• depending on the plugin's job, this can be a really bad thing (re-queuing a failed job 5 times might be bad for financial transactions)

Then i realised my first assumption was wrong. One can isolate methods with static invocations for testing.

The second assumption is still valid, but not critical for this feature, but only for #56.

[merlindorin]

I clearly understand the problem of manipulating static... without a full refacto, this is the actual design of this library... and should be taken "like that". From my point of view, it's not a bad design if we don't try to run multiple worker on the same running code. And I think it's a good practice to keep a worker instance "unique".

The count issue #56 talk about the project of chrisboulton/php-resque. In this library... it's a fork of the worker. COUNT=5 means 5 fork... so the static design still works.

Your first assumption is good... it's more simple to test injection rather than static call... in this project, static is the design :/.

In the end, we can enforce the isolation of the plugin in a worker instance using an Plugin->init() method in the plugin that is call in the worker constructor (I don't think it's interesting because with the actual static design... this will change nothing). For memo, registration does not mean initialization. We can registrate without initialize (not the code above because I registrate and initialize FYI).

What do you think?

Thx for the feedback :D.