From 20bad3ccff1d3e32881db47401c1a9a7400a3618 Mon Sep 17 00:00:00 2001 From: James William Dumay Date: Sat, 25 Mar 2023 08:02:02 +1100 Subject: [PATCH 1/6] set a name for the cloud task based on display name and UUID --- src/CloudTasksQueue.php | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/CloudTasksQueue.php b/src/CloudTasksQueue.php index 33c8dba..1eb766f 100644 --- a/src/CloudTasksQueue.php +++ b/src/CloudTasksQueue.php @@ -142,7 +142,7 @@ protected function pushToCloudTasks($queue, $payload, $delay = 0) // Laravel 7+ jobs have a uuid, but Laravel 6 doesn't have it. // Since we are using and expecting the uuid in some places // we will add it manually here if it's not present yet. - [$payload, $uuid] = $this->withUuid($payload); + [$payload, $uuid, $displayName] = $this->extractPayload($payload); // Since 3.x tasks are released back onto the queue after an exception has // been thrown. This means we lose the native [X-CloudTasks-TaskRetryCount] header @@ -152,6 +152,7 @@ protected function pushToCloudTasks($queue, $payload, $delay = 0) $httpRequest->setBody($payload); $task = $this->createTask(); + $task->setName($this->taskName($uuid, $displayName)); $task->setHttpRequest($httpRequest); // The deadline for requests sent to the app. If the app does not respond by @@ -172,12 +173,17 @@ protected function pushToCloudTasks($queue, $payload, $delay = 0) $createdTask = CloudTasksApi::createTask($queueName, $task); - event((new TaskCreated)->queue($queue)->task($task)); + event((new TaskCreated)->queue($queue)->task($createdTask)); return $uuid; } - private function withUuid(string $payload): array + private function taskName(string $uuid, string $displayName) { + $displayName = str_replace("\\", "-", $displayName); + return sprintf('%s-%s', $uuid, $displayName); + } + + private function extractPayload(string $payload): array { /** @var array $decoded */ $decoded = json_decode($payload, true); @@ -189,6 +195,7 @@ private function withUuid(string $payload): array return [ json_encode($decoded), $decoded['uuid'], + $decoded['displayName'] ]; } From 35e6b274b8c86573b32751cdb38efc4175b985db Mon Sep 17 00:00:00 2001 From: James William Dumay Date: Sat, 25 Mar 2023 08:21:32 +1100 Subject: [PATCH 2/6] Update CloudTasksQueue.php --- src/CloudTasksQueue.php | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/CloudTasksQueue.php b/src/CloudTasksQueue.php index 1eb766f..414f45c 100644 --- a/src/CloudTasksQueue.php +++ b/src/CloudTasksQueue.php @@ -152,7 +152,7 @@ protected function pushToCloudTasks($queue, $payload, $delay = 0) $httpRequest->setBody($payload); $task = $this->createTask(); - $task->setName($this->taskName($uuid, $displayName)); + $task->setName($this->taskName($queueName, $uuid, $displayName)); $task->setHttpRequest($httpRequest); // The deadline for requests sent to the app. If the app does not respond by @@ -178,9 +178,14 @@ protected function pushToCloudTasks($queue, $payload, $delay = 0) return $uuid; } - private function taskName(string $uuid, string $displayName) { + private function taskName(string $queueName, string $uuid, string $displayName) { + $config = $this->config; + $projectId = $config['project']; + $location = $config['location']; + $queueId = $queueName; + // projects/PROJECT_ID/locations/LOCATION_ID/queues/QUEUE_ID/tasks/TASK_ID $displayName = str_replace("\\", "-", $displayName); - return sprintf('%s-%s', $uuid, $displayName); + return sprintf('projects/%s/locations/%s/queues/%s/tasks/%s-%s', $projectId, $location, $queueId, $uuid, $displayName); } private function extractPayload(string $payload): array From 5c5dbc557bdfca8f69e43a2756273db7310fa81e Mon Sep 17 00:00:00 2001 From: James William Dumay Date: Sat, 25 Mar 2023 08:37:17 +1100 Subject: [PATCH 3/6] Update CloudTasksQueue.php --- src/CloudTasksQueue.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/CloudTasksQueue.php b/src/CloudTasksQueue.php index 414f45c..0517293 100644 --- a/src/CloudTasksQueue.php +++ b/src/CloudTasksQueue.php @@ -11,6 +11,7 @@ use Google\Protobuf\Timestamp; use Illuminate\Contracts\Queue\Queue as QueueContract; use Illuminate\Queue\Queue as LaravelQueue; +use Illuminate\Support\Facades\Log; use Illuminate\Support\Str; use Stackkit\LaravelGoogleCloudTasksQueue\Events\TaskCreated; use function Safe\json_decode; @@ -185,7 +186,9 @@ private function taskName(string $queueName, string $uuid, string $displayName) $queueId = $queueName; // projects/PROJECT_ID/locations/LOCATION_ID/queues/QUEUE_ID/tasks/TASK_ID $displayName = str_replace("\\", "-", $displayName); - return sprintf('projects/%s/locations/%s/queues/%s/tasks/%s-%s', $projectId, $location, $queueId, $uuid, $displayName); + $taskName = sprintf('projects/%s/locations/%s/queues/%s/tasks/%s-%s', $projectId, $location, $queueId, $uuid, $displayName); + Log::info('Task name '. $taskName); + return $taskName; } private function extractPayload(string $payload): array From 16735b37cbfdac8fcfd922b5074a9b19e51c2e17 Mon Sep 17 00:00:00 2001 From: James William Dumay Date: Sat, 25 Mar 2023 08:54:33 +1100 Subject: [PATCH 4/6] Update CloudTasksQueue.php --- src/CloudTasksQueue.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/CloudTasksQueue.php b/src/CloudTasksQueue.php index 0517293..b8935ec 100644 --- a/src/CloudTasksQueue.php +++ b/src/CloudTasksQueue.php @@ -153,7 +153,7 @@ protected function pushToCloudTasks($queue, $payload, $delay = 0) $httpRequest->setBody($payload); $task = $this->createTask(); - $task->setName($this->taskName($queueName, $uuid, $displayName)); + $task->setName($this->taskName($queue, $uuid, $displayName)); $task->setHttpRequest($httpRequest); // The deadline for requests sent to the app. If the app does not respond by @@ -185,6 +185,7 @@ private function taskName(string $queueName, string $uuid, string $displayName) $location = $config['location']; $queueId = $queueName; // projects/PROJECT_ID/locations/LOCATION_ID/queues/QUEUE_ID/tasks/TASK_ID + // projects/PROJECT_ID/locations/LOCATION_ID/queues/QUEUE_ID/tasks/TASK_ID $displayName = str_replace("\\", "-", $displayName); $taskName = sprintf('projects/%s/locations/%s/queues/%s/tasks/%s-%s', $projectId, $location, $queueId, $uuid, $displayName); Log::info('Task name '. $taskName); From 1263eddd9bca10dcdb2eb1f7b2d378ab9e5ebfa2 Mon Sep 17 00:00:00 2001 From: James William Dumay Date: Sat, 25 Mar 2023 09:06:11 +1100 Subject: [PATCH 5/6] Update CloudTasksQueue.php --- src/CloudTasksQueue.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/CloudTasksQueue.php b/src/CloudTasksQueue.php index b8935ec..48cf572 100644 --- a/src/CloudTasksQueue.php +++ b/src/CloudTasksQueue.php @@ -188,7 +188,8 @@ private function taskName(string $queueName, string $uuid, string $displayName) // projects/PROJECT_ID/locations/LOCATION_ID/queues/QUEUE_ID/tasks/TASK_ID $displayName = str_replace("\\", "-", $displayName); $taskName = sprintf('projects/%s/locations/%s/queues/%s/tasks/%s-%s', $projectId, $location, $queueId, $uuid, $displayName); - Log::info('Task name '. $taskName); + print ($taskName); + print ($taskName); return $taskName; } From 65e7200de5b4f1134f0f0680e3d2c315083d272d Mon Sep 17 00:00:00 2001 From: Marick van Tuil Date: Sun, 9 Apr 2023 15:10:43 +0200 Subject: [PATCH 6/6] Cleanup and add test --- src/CloudTasksApiFake.php | 14 ++------- src/CloudTasksQueue.php | 63 ++++++++++++++++----------------------- tests/QueueTest.php | 19 ++++++++++++ tests/TestCase.php | 3 +- 4 files changed, 49 insertions(+), 50 deletions(-) diff --git a/src/CloudTasksApiFake.php b/src/CloudTasksApiFake.php index c193e71..f1af5da 100644 --- a/src/CloudTasksApiFake.php +++ b/src/CloudTasksApiFake.php @@ -30,8 +30,6 @@ public function getRetryConfig(string $queueName): RetryConfig public function createTask(string $queueName, Task $task): Task { - $task->setName(Str::uuid()->toString()); - $this->createdTasks[] = compact('queueName', 'task'); return $task; @@ -56,24 +54,16 @@ public function getRetryUntilTimestamp(Task $task): ?int public function assertTaskDeleted(string $taskName): void { - $taskUuids = array_map(function ($fullTaskName) { - return Arr::last(explode('/', $fullTaskName)); - }, $this->deletedTasks); - Assert::assertTrue( - in_array($taskName, $taskUuids), + in_array($taskName, $this->deletedTasks), 'The task [' . $taskName . '] should have been deleted but it is not.' ); } public function assertTaskNotDeleted(string $taskName): void { - $taskUuids = array_map(function ($fullTaskName) { - return Arr::last(explode('/', $fullTaskName)); - }, $this->deletedTasks); - Assert::assertTrue( - ! in_array($taskName, $taskUuids), + ! in_array($taskName, $this->deletedTasks), 'The task [' . $taskName . '] should not have been deleted but it was.' ); } diff --git a/src/CloudTasksQueue.php b/src/CloudTasksQueue.php index 48cf572..c066687 100644 --- a/src/CloudTasksQueue.php +++ b/src/CloudTasksQueue.php @@ -11,7 +11,6 @@ use Google\Protobuf\Timestamp; use Illuminate\Contracts\Queue\Queue as QueueContract; use Illuminate\Queue\Queue as LaravelQueue; -use Illuminate\Support\Facades\Log; use Illuminate\Support\Str; use Stackkit\LaravelGoogleCloudTasksQueue\Events\TaskCreated; use function Safe\json_decode; @@ -140,20 +139,22 @@ protected function pushToCloudTasks($queue, $payload, $delay = 0) $httpRequest->setUrl($this->getHandler()); $httpRequest->setHttpMethod(HttpMethod::POST); + $payload = json_decode($payload, true); + // Laravel 7+ jobs have a uuid, but Laravel 6 doesn't have it. // Since we are using and expecting the uuid in some places // we will add it manually here if it's not present yet. - [$payload, $uuid, $displayName] = $this->extractPayload($payload); + $payload = $this->withUuid($payload); // Since 3.x tasks are released back onto the queue after an exception has // been thrown. This means we lose the native [X-CloudTasks-TaskRetryCount] header // value and need to manually set and update the number of times a task has been attempted. $payload = $this->withAttempts($payload); - $httpRequest->setBody($payload); + $httpRequest->setBody(json_encode($payload)); $task = $this->createTask(); - $task->setName($this->taskName($queue, $uuid, $displayName)); + $task->setName($this->taskName($queue, $payload)); $task->setHttpRequest($httpRequest); // The deadline for requests sent to the app. If the app does not respond by @@ -174,51 +175,39 @@ protected function pushToCloudTasks($queue, $payload, $delay = 0) $createdTask = CloudTasksApi::createTask($queueName, $task); - event((new TaskCreated)->queue($queue)->task($createdTask)); + event((new TaskCreated)->queue($queue)->task($task)); - return $uuid; + return $payload['uuid']; } - private function taskName(string $queueName, string $uuid, string $displayName) { - $config = $this->config; - $projectId = $config['project']; - $location = $config['location']; - $queueId = $queueName; - // projects/PROJECT_ID/locations/LOCATION_ID/queues/QUEUE_ID/tasks/TASK_ID - // projects/PROJECT_ID/locations/LOCATION_ID/queues/QUEUE_ID/tasks/TASK_ID - $displayName = str_replace("\\", "-", $displayName); - $taskName = sprintf('projects/%s/locations/%s/queues/%s/tasks/%s-%s', $projectId, $location, $queueId, $uuid, $displayName); - print ($taskName); - print ($taskName); - return $taskName; - } - - private function extractPayload(string $payload): array + private function withUuid(array $payload): array { - /** @var array $decoded */ - $decoded = json_decode($payload, true); - - if (!isset($decoded['uuid'])) { - $decoded['uuid'] = (string) Str::uuid(); + if (!isset($payload['uuid'])) { + $payload['uuid'] = (string) Str::uuid(); } - return [ - json_encode($decoded), - $decoded['uuid'], - $decoded['displayName'] - ]; + return $payload; } - private function withAttempts(string $payload): string + private function taskName(string $queueName, array $payload): string { - /** @var array $decoded */ - $decoded = json_decode($payload, true); + $displayName = str_replace("\\", '-', $payload['displayName']); - if (!isset($decoded['internal']['attempts'])) { - $decoded['internal']['attempts'] = 0; + return CloudTasksClient::taskName( + $this->config['project'], + $this->config['location'], + $queueName, + $displayName . '-' . $payload['uuid'] + ); + } + + private function withAttempts(array $payload): array + { + if (!isset($payload['internal']['attempts'])) { + $payload['internal']['attempts'] = 0; } - return json_encode($decoded); + return $payload; } /** diff --git a/tests/QueueTest.php b/tests/QueueTest.php index d4ef6aa..8ed8f67 100644 --- a/tests/QueueTest.php +++ b/tests/QueueTest.php @@ -478,4 +478,23 @@ public function test_ignoring_jobs_with_deleted_models() Log::assertLogged('UserJob:John'); CloudTasksApi::assertTaskNotDeleted($job->task->getName()); } + + /** + * @test + */ + public function it_adds_a_task_name_based_on_the_display_name() + { + // Arrange + CloudTasksApi::fake(); + + // Act + $this->dispatch((new SimpleJob())); + + // Assert + CloudTasksApi::assertTaskCreated(function (Task $task, string $queueName): bool { + $uuid = \Safe\json_decode($task->getHttpRequest()->getBody(), true)['uuid']; + + return $task->getName() === 'projects/my-test-project/locations/europe-west6/queues/barbequeue/tasks/Tests-Support-SimpleJob-' . $uuid; + }); + } } diff --git a/tests/TestCase.php b/tests/TestCase.php index 0c17bb7..587e802 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -133,7 +133,8 @@ public function dispatch($job) $payloadAsArray = json_decode($payload, true); $task = $event->task; - request()->headers->set('X-Cloudtasks-Taskname', $task->getName()); + [,,,,,,,$taskName] = explode('/', $task->getName()); + request()->headers->set('X-Cloudtasks-Taskname', $taskName); }); dispatch($job);