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.

Sign up for GitHub

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

[4.x dev] Added support for handler-url and headers per job #139

Merged
merged 8 commits into from
Mar 31, 2024
Merged

[4.x dev] Added support for handler-url and headers per job #139

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8bb520a
[4.x-dev] Can use the task handler url from the job, if available. Ot…
aaajeetee Feb 27, 2024
b461e1e
[4.x-dev] A job can also have task headers, which are merged with any…
aaajeetee Feb 27, 2024
7a0f7df
[4.x-dev] Can set the HandlerUrl using a Closure callback. If given, …
aaajeetee Mar 13, 2024
e642299
Merge branch '4.x-dev' into 4.x-dev
aaajeetee Mar 13, 2024
fa0d675
[4.x-dev] Import class
aaajeetee Mar 13, 2024
4cf5598
[4.x-dev] Added missing test attribute
aaajeetee Mar 27, 2024
829f720
[4.x-dev] Added "setTaskHeadersUsing()" method to allow a closure to …
aaajeetee Mar 27, 2024
4b224ae
Merge branch '4.x-dev' into 4.x-dev
aaajeetee Mar 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 25 additions & 11 deletions src/CloudTasksQueue.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ public function push($job, $data = '', $queue = null)
$this->createPayload($job, $queue, $data),
$queue,
null,
function ($payload, $queue) {
return $this->pushRaw($payload, $queue);
function ($payload, $queue) use ($job) {
return $this->pushRaw($payload, $queue, ['job' => $job]);
}
);
}
Expand All @@ -73,8 +73,9 @@ function ($payload, $queue) {
public function pushRaw($payload, $queue = null, array $options = [])
{
$delay = ! empty($options['delay']) ? $options['delay'] : 0;
$job = $options['job'] ?? null;

return $this->pushToCloudTasks($queue, $payload, $delay);
return $this->pushToCloudTasks($queue, $payload, $delay, $job);
}

/**
Expand All @@ -93,8 +94,8 @@ public function later($delay, $job, $data = '', $queue = null)
$this->createPayload($job, $queue, $data),
$queue,
$delay,
function ($payload, $queue, $delay) {
return $this->pushToCloudTasks($queue, $payload, $delay);
function ($payload, $queue, $delay) use ($job) {
return $this->pushToCloudTasks($queue, $payload, $delay, $job);
}
);
}
Expand All @@ -105,9 +106,10 @@ function ($payload, $queue, $delay) {
* @param string|null $queue
* @param string $payload
* @param \DateTimeInterface|\DateInterval|int $delay
* @param string|object $job
* @return string
*/
protected function pushToCloudTasks($queue, $payload, $delay = 0)
protected function pushToCloudTasks($queue, $payload, $delay, mixed $job)
{
$queue = $queue ?: $this->config['queue'];

Expand All @@ -122,7 +124,7 @@ protected function pushToCloudTasks($queue, $payload, $delay = 0)
connectionName: $this->getConnectionName(),
);

$this->addPayloadToTask($payload, $task);
$this->addPayloadToTask($payload, $task, $job);

// The deadline for requests sent to the app. If the app does not respond by
// this deadline then the request is cancelled and the attempt is marked as
Expand Down Expand Up @@ -184,9 +186,16 @@ private function enrichPayloadWithInternalData(
return $payload;
}

public function addPayloadToTask(array $payload, Task $task): Task
/** @param string|object $job */
public function addPayloadToTask(array $payload, Task $task, mixed $job): Task
{
$headers = value($this->headers, $payload) ?: [];
if ($job instanceof HasTaskHeaders) {
$headers = [
...$headers,
...$job->taskHeaders(),
];
}

if (!empty($this->config['app_engine'])) {
$path = \Safe\parse_url(route('cloud-tasks.handle-task'), PHP_URL_PATH);
Expand All @@ -206,7 +215,7 @@ public function addPayloadToTask(array $payload, Task $task): Task
$task->setAppEngineHttpRequest($appEngineRequest);
} else {
$httpRequest = new HttpRequest();
$httpRequest->setUrl($this->getHandler());
$httpRequest->setUrl($this->getHandler($job));
$httpRequest->setBody(json_encode($payload));
$httpRequest->setHttpMethod(HttpMethod::POST);
$httpRequest->setHeaders($headers);
Expand Down Expand Up @@ -237,13 +246,18 @@ public function release(CloudTasksJob $job, int $delay = 0): void

$payload = $job->getRawBody();

$options = ['delay' => $delay];
$options = ['delay' => $delay, 'job' => $job];

$this->pushRaw($payload, $job->getQueue(), $options);
}

public function getHandler(): string
/** @param string|object $job */
public function getHandler(mixed $job): string
{
if ($job instanceof HasTaskHandlerUrl) {
return $job->taskHandlerUrl();
Copy link
Member

Choose a reason for hiding this comment

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

Let's say there is a staging and production environment. How can I use a separate high performance queue in production (because it's high traffic) but use the same single queue setup in staging?
Or when both environments do have a high-performance queue, they need to point to different URLs which will need a configuration and environment variable. How can we make that possible?
I feel like you'd encounter that situation quickly, which is why I toyed with the idea of putting this into the config/queue.php settings:

Code snippet 
'cloudtasks' => [
    'handler'               => env('STACKKIT_CLOUD_TASKS_HANDLER', ''),
    'service_account_email' => env('STACKKIT_CLOUD_TASKS_SERVICE_EMAIL', ''),
    'signed_audience'       => env('STACKKIT_CLOUD_TASKS_SIGNED_AUDIENCE', true),
    // The default handler
    'handler'               => env('STACKKIT_CLOUD_TASKS_HANDLER'),
    // Queue specific handlers
    'queue_handlers'        => [
      'low_performance' => env('STACKKIT_CLOUD_TASKS_LOW_PERF_HANDLER', env('STACKKIT_CLOUD_TASKS_HANDLER')),
      'high_performance' => env('STACKKIT_CLOUD_TASKS_HIGH_PERF_HANDLER', env('STACKKIT_CLOUD_TASKS_HANDLER')),
    ]
],

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I understand the need and use case, but is this something this package needs to take care of, or is this logic that can be put in the taskHandlerUrl() method of the job? Because I can imagine that jobs A and B will behave differently than jobs C and D, url wise (and environment) wise.

I do like the option to have multiple handlers in the config, but personally I would not hard-code these. Other than a 'default'.

Something like this:

'cloudtasks' => [
    'handler'               => env('STACKKIT_CLOUD_TASKS_HANDLER', ''),
    'service_account_email' => env('STACKKIT_CLOUD_TASKS_SERVICE_EMAIL', ''),
    'signed_audience'       => env('STACKKIT_CLOUD_TASKS_SIGNED_AUDIENCE', true),
    'handlers'              => [
      'default' => env('STACKKIT_CLOUD_TASKS_HANDLER'),
      // add your own handler urls here
    ],
],

The package or project that implements this cloud-tasks package is free to update the config as they desire and add other handlers.

With this config setup, the taskHandlerUrl() implementation of the job can fetch the appropriate handler url from the config if needed. Or add other logic to return the right handler url for that job. That's up to the job.

The main 'problem' I got with these hardcoded handlers: not all projects have "low performance" or "high performance".
Without going into too much detail: my use case is that I have 2 different handlers: the default and one with a static outgoing IP address. This does not fit the hardcoded performance handlers.

Let me know what you think of this, or perhaps have other suggestion(s) based on this.

}

if (empty($this->config['handler'])) {
$this->config['handler'] = request()->getSchemeAndHttpHost();
}
Expand Down
10 changes: 10 additions & 0 deletions src/HasTaskHandlerUrl.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?php

declare(strict_types=1);

namespace Stackkit\LaravelGoogleCloudTasksQueue;

interface HasTaskHandlerUrl
{
public function taskHandlerUrl(): string;
}
11 changes: 11 additions & 0 deletions src/HasTaskHeaders.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

declare(strict_types=1);

namespace Stackkit\LaravelGoogleCloudTasksQueue;

interface HasTaskHeaders
{
/** @return array<string, mixed> */
public function taskHeaders(): array;
Copy link
Member

Choose a reason for hiding this comment

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

Nice, I really like this. I think we can remove the Queue::connection()->setTaskHeaders(...) so there is a single unified way to add task headers. If tasks require the same set of headers, a trait can be created:

trait WithDefaultTaskHeaders
{
    public function taskHeaders(): array
    {
        return [];
    }
}

Much more clear where the task headers are defined rather than a global method somewhere in a service provider.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I personally also want to have 1 way to add task headers, but was doubting if I should remove the setTaskHeaders() on the connection or not.

Will do so now.

}
40 changes: 38 additions & 2 deletions tests/QueueTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@
use Illuminate\Support\Facades\Queue;
use PHPUnit\Framework\Attributes\Test;
use Stackkit\LaravelGoogleCloudTasksQueue\CloudTasksApi;
use Stackkit\LaravelGoogleCloudTasksQueue\CloudTasksQueue;
use Stackkit\LaravelGoogleCloudTasksQueue\Events\JobReleased;
use Tests\Support\CustomHandlerUrlJob;
use Tests\Support\CustomHeadersJob;
use Tests\Support\FailingJob;
use Tests\Support\FailingJobWithExponentialBackoff;
use Tests\Support\JobOutput;
Expand Down Expand Up @@ -59,7 +60,7 @@ public function it_posts_to_the_handler()
}

#[Test]
public function it_posts_to_the_correct_handler_url()
public function it_posts_to_the_configured_handler_url()
{
// Arrange
$this->setConfigValue('handler', 'https://docker.for.mac.localhost:8081');
Expand All @@ -74,6 +75,22 @@ public function it_posts_to_the_correct_handler_url()
});
}

#[Test]
public function it_posts_to_the_job_handler_url()
{
// Arrange
$this->setConfigValue('handler', 'https://docker.for.mac.localhost:8081');
CloudTasksApi::fake();

// Act
$this->dispatch(new CustomHandlerUrlJob());

// Assert
CloudTasksApi::assertTaskCreated(function (Task $task): bool {
return $task->getHttpRequest()->getUrl() === 'https://example.com/api/my-custom-route';
});
}

#[Test]
public function it_posts_the_serialized_job_payload_to_the_handler()
{
Expand Down Expand Up @@ -493,4 +510,23 @@ public function headers_can_be_added_to_the_task_with_job_context()
return $task->getHttpRequest()->getHeaders()['X-MyHeader'] === SimpleJob::class;
});
}

#[Test]
public function job_headers_can_be_added_to_the_task()
{
// Arrange
CloudTasksApi::fake();

// Act
Queue::connection()->setTaskHeaders([
'X-MyHeader' => 'MyValue',
]);
$this->dispatch((new CustomHeadersJob()));

// Assert
CloudTasksApi::assertTaskCreated(function (Task $task): bool {
$headers = $task->getHttpRequest()->getHeaders();
return $headers['X-MyHeader'] === 'MyValue' && $headers['X-MyJobHeader'] === 'MyJobValue';
});
}
}
27 changes: 27 additions & 0 deletions tests/Support/CustomHandlerUrlJob.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php

declare(strict_types=1);

namespace Tests\Support;

use Illuminate\Bus\Queueable;
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Foundation\Bus\Dispatchable;
use Illuminate\Queue\InteractsWithQueue;
use Illuminate\Queue\SerializesModels;
use Stackkit\LaravelGoogleCloudTasksQueue\HasTaskHandlerUrl;

class CustomHandlerUrlJob implements ShouldQueue, HasTaskHandlerUrl
{
use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;

public function handle(): void
{
event(new JobOutput('CustomHandlerUrlJob:success'));
}

public function taskHandlerUrl(): string
{
return 'https://example.com/api/my-custom-route';
}
}
30 changes: 30 additions & 0 deletions tests/Support/CustomHeadersJob.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?php

declare(strict_types=1);

namespace Tests\Support;

use Illuminate\Bus\Queueable;
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Foundation\Bus\Dispatchable;
use Illuminate\Queue\InteractsWithQueue;
use Illuminate\Queue\SerializesModels;
use Stackkit\LaravelGoogleCloudTasksQueue\HasTaskHeaders;

class CustomHeadersJob implements ShouldQueue, HasTaskHeaders
{
use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;

public function handle(): void
{
event(new JobOutput('CustomHandlerUrlJob:success'));
}

/** @inheritdoc */
public function taskHeaders(): array
{
return [
'X-MyJobHeader' => 'MyJobValue',
];
}
}
Loading