Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Connecting different Yii2 apps to the same queue #339

Open
flaviovs opened this issue Jun 11, 2019 · 9 comments
Open

Connecting different Yii2 apps to the same queue #339

flaviovs opened this issue Jun 11, 2019 · 9 comments
Labels
status:ready for adoption Feel free to implement this issue. type:docs Documentation

Comments

@flaviovs
Copy link

What steps will reproduce the problem?

On the producer:

        'queue' => [
            'class' => \yii\queue\db\Queue::class,
            'mutex' => \yii\mutex\PgsqlMutex::class,
            'strictJobType' => false,
            'serializer' => \yii\queue\serializers\JsonSerializer::class,
        ],

On the consumer:

        'queue' => [
            'class' => \yii\queue\db\Queue::class,
            'mutex' => \yii\mutex\PgsqlMutex::class,
            'strictJobType' => false,
            'serializer' => \yii\queue\serializers\JsonSerializer::class,
            'on beforeExec' => function ($ev) {
                print_r($ev);
                $ev->handled = true;
            },
        ],

Then, in the producer:

\Yii::$app->queue->push(['foobar' => 12345]);

What's expected?

Unserialized job data easily accessible somehow to the consumer.

What do you get instead?

Impossible to get direct access to the unserialized job data.

Looking into unserializeMessage() (see below) we see that the unserialized job is returned only if it is a JobInterface object, which obviously it is not (i.e. it is a PHP array just unserialized from a JSON string).

yii2-queue/src/Queue.php

Lines 265 to 281 in ee28f57

public function unserializeMessage($serialized)
{
try {
$job = $this->serializer->unserialize($serialized);
} catch (\Exception $e) {
return [null, new InvalidJobException($serialized, $e->getMessage(), 0, $e)];
}
if ($job instanceof JobInterface) {
return [$job, null];
}
return [null, new InvalidJobException($serialized, sprintf(
'Job must be a JobInterface instance instead of %s.',
VarDumper::dumpAsString($job)
))];
}

The only way I could manage to access the data is to get the (serialized) JSON string using $ev->error->getSerialized(), and then decode it. That does not look right to me.

Not sure of it is a bug, or design decision. However, this issue makes it very difficult to (ironically) two Yii2 apps that do not share the same codebase to communicate.

Additional info

  • As you can see from the examples, I am using Postgres+DB driver, but I guess this an issue common to all drivers.
Q A
Yii version 2.0.20
PHP version 7.3
Operating system Debian (stretch)
@samdark
Copy link
Member

samdark commented Jun 11, 2019

@zhuravljov any idea on how we can improve to solve it?

@larryli
Copy link
Contributor

larryli commented Jun 11, 2019

At first, strictJobType is designed for the third party workers, cannot used in the yii2-queue workers.

Implement your own serializer class if you want to handle custom messages in the workers.

You can simply replace {"event":"foobar"} into {"class":"app\jobs\FoobarJob"}.

@samdark samdark added type:docs Documentation and removed status:under discussion labels Jun 11, 2019
@samdark
Copy link
Member

samdark commented Jun 11, 2019

@larryli so do you think better docs would be enough?

@larryli
Copy link
Contributor

larryli commented Jun 11, 2019

@samdark Yes.

@larryli
Copy link
Contributor

larryli commented Jun 11, 2019

@flaviovs I copy some code from \yii\web\UrlManager to MyJsonSerializer.

So, handle multi-apps jobs:

'queue' => [
    'class' => yii\queue\redis\Queue::class,
    'redis' => 'redis',
    'serializer' => [
        'class' => app\MyJsonSerializer::class,
        'jobAttribute' => 'event',
        'rules' => [
             '<app:\w+>-common-<job:\w+>' => 'app/jobs/common/<job>', // pass $job->app
             '<app:\w+>-<job:\w+>' => 'app/jobs/<app>/<job>',
       ],
    ],
];

push:

Yii::$app->queue->push([
    'event' => 'app1-foobar',
    'foo' => 'bar',
]);

Some code:

        list($route, $params) = $this->parse(ArrayHelper::getValue($unserialized, $this->jobAttribute));
        $route = trim($route, '/');
        $pos = strrpos($route, '/');
        if ($pos === false) {
            $route = ucfirst($route);
        } else {
            $pos++;
            $route = str_replace('/', '\\', substr($route, 0, $pos)) . ucfirst(substr($route, $pos));
        }
        $params['class'] = $route . 'Job';
        if (strpos($params['class'], '-') !== false || !class_exists($params['class'])) {
            throw new UnknownClassException($params['class']);
        }
        foreach ($unserialized as $key => $value) {
            if (!isset($params[$key])) {
                 $params[$key] = $value;
            }
        }
        return Yii::createObject($params);

@samdark samdark added the status:ready for adoption Feel free to implement this issue. label Jun 11, 2019
@flaviovs
Copy link
Author

I think that a clean and simple solution would be to have a externalJobFactory attribute in the queue that, when provided, should point to a callable responsible to instantiate and return a proper JobInterface instance. This callable should receive the unserialized data, and be used by unserializeMessage() when it detects that the unserialized message is not a JobInterface. It is a simple change to implement, and totally BC. I'm willing to provide a PR if you think it is the way to go.

@larryli, strictJobType is needed because we want to explicitly pass arbitrary data. Notice that the goal here is to have two Yii2 apps to communicate without having to have and maintain the same job class in both. I understand that that attribute is meant for external workers, but it would be nice if such external worker could be another Yii application.

BTW, passing ['class' => 'app\Myjob'] would not work, because JsonSerializer raises an exception if it sees a "class" key during serialization.

FYI, I'm testing now the custom serializer workaround. Actually, this is akin to the externalJobFactory solution, except that the logic is hard-coded inside the JSON serializer. It works, but is just not as elegant and general as it should be, IMHO.

@luayessa
Copy link

@flaviovs
I think the better solution is as described in the below steps:

  1. Use JsonSerializer for the producer and the worker queues.
  2. Set the strictJobType value to be false for the producer queue.
  3. Change the value of the classKey property for the producer queue to be something different than the default value. For example:
'serializer' => [
                'class' => \yii\queue\serializers\JsonSerializer::class,
                'classKey' => 'dummyClassKey'  
            ],

Now you can push a JSON message on the queue which has a "class" as a property.

\Yii::$app->producer_queue->push([
            'class' => 'app\jobs\WorkerClass',
            'property1' => 'value1'
]);

@flaviovs
Copy link
Author

Hi @luayessa - thanks for the tip.

For completeness, my primary goal was to have two distinct Yii2 to communicate, and the solution I adopted was actually very simple: just make sure that the job class both in app A and B have the same properties. In other words, they do not need to come from the same codebase, just have the same public attributes. I even made execute() throw an exception in the producer job class, to catch someone pushing an instance of it to the producer queue by accident. Not totally elegant, but it works fine with stock Yii2. The only issue I guess may happen is the consumer crashing if you add a new public attribute to the producer (or remove one from consumer). A way to mitigate such type of is to have a single "payload" attribute that can be used to pass any data. Again, not very elegant but it works.

@brotherbigbao
Copy link

@flaviovs good job

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:ready for adoption Feel free to implement this issue. type:docs Documentation
Projects
None yet
Development

No branches or pull requests

5 participants