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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.7] Add queue payload customization feature #23606

Closed
wants to merge 1 commit into from
Closed

[5.7] Add queue payload customization feature #23606

wants to merge 1 commit into from

Conversation

AaronJan
Copy link
Contributor

Goal

Allow developer to customize payload of queue job.

Why

Developer using message queue to decouple logic into different services, some of these services may not be PHP application.

Here is how a Redis queue payload looks like:

{\"displayName\":\"App\\\\Jobs\\\\TestQueue\",\"job\":\"Illuminate\\\\Queue\\\\CallQueuedHandler@call\",\"maxTries\":null,\"timeout\":null,\"timeoutAt\":null,\"data\":{\"commandName\":\"App\\\\Jobs\\\\TestQueue\",\"command\":\"O:18:\\\"App\\\\Jobs\\\\TestQueue\\\":7:{s:6:\\\"\\u0000*\\u0000job\\\";N;s:10:\\\"connection\\\";N;s:5:\\\"queue\\\";N;s:15:\\\"chainConnection\\\";N;s:10:\\\"chainQueue\\\";N;s:5:\\\"delay\\\";N;s:7:\\\"chained\\\";a:0:{}}\"},\"id\":\"SNjXfGlL1QdB3G3CyCnfXI3pHjC2kNK8\",\"attempts\":0}

Use NodeJS to generating this? No way.

To achieve communication between different languages, the only way is customizing payload (in an out) to a simpler JSON (or other format) without the overhead, like this:

{
    "productId": 1,
    "cost": "151.15"
}

Just let another adapter to translate this back to job handler (using payload, connection name and queue name, it's possible).

And that's what I did for my project (using RabbitMQ).

Changes

  • I added a PayloadSerializer interface to handle serialization that is hard-coded originally.

  • Allow developer configure PayloadSerializer in config/queue.php (Connection => PayloadSerializer).

  • Backward compatible.

Discussion

This is a huge change, but maybe few developers are really need this feature.

What are you think about this? 馃槂

* @param int $retryAfter
* @param int|null $blockFor
* @param \Illuminate\Contracts\Redis\Factory $redis
* @param string $default
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep 2 spaces in thr phpdoc for param annotations, like it was before, everywhere.

$this->redis = $redis;
$this->default = $default;
$this->blockFor = $blockFor;
$this->redis = $redis;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't align these.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code style updated.

@taylorotwell
Copy link
Member

Honestly way to many changes. Something like this should be able to be accomplished easier.

@AaronJan
Copy link
Contributor Author

@taylorotwell I knew this would happen, faster than I expect.. 馃槯

I accept this, but how to accomplish this exactly?

@AaronJan
Copy link
Contributor Author

@GrahamCampbell
Thanks for the reviews, I forgot to change my format setting.

Do you use PhpStorm? How to accomplish this?

Please keep 2 spaces in thr phpdoc for param annotations, like it was before, everywhere.

@AaronJan AaronJan mentioned this pull request Mar 19, 2018
@francislavoie
Copy link

francislavoie commented Mar 19, 2018

FWIW I'd really love to see this implemented in some form or another.

I did some research on the topic, seems like implementing Serializable on jobs would fix some of the problems here, but I haven't gotten around to trying that yet.

laravel/ideas#1007

@AaronJan
Copy link
Contributor Author

AaronJan commented Mar 19, 2018

@francislavoie

Finally I'm not alone. 馃槃

Serializable only gives some flexibility, by introducing PayloadSerializer gives you full control of payload.

You can serialize job to some really simple and readable JSON string (even protobuf), so other program language can process it, or you can unserialize it back, and return the essential attributes to Laravel like this:

<?php

public function unserialize($raw, $connectionName, $queueName)
{
    $parameters = json_decode($raw);

    if ($connectionName === 'awesome' && $queueName === 'laravel') {
        $jobClass = 'a';
    } else {
        $jobClass = 'b';
    }

    return [
        'displayName' => 'My RabbitMQ Job',
        'job' => 'Illuminate\Queue\CallQueuedHandler@call',
        'data' => [
            'commandName' => $jobClass,
            'command' => serialize(new $jobClass($parameters['product_id'])),
        ],
    ];
}

Last serialize part is a little bit nasty, but I think we can figure it out, that's why I need some discussion.

This PR is backward compatible, even the Redis queue that is using Lua.

At last, if Taylor doesn't approve this PR, maybe these's no point to do it this way. 馃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants