dereuromark / cakephp-queue

Queue plugin for CakePHP - simple, pure PHP and without dependencies.
MIT License
306 stars 137 forks source link

Prepare major release v8 #410

Closed dereuromark closed 7 months ago

dereuromark commented 8 months ago

Wait a few more days for any other major change that could be added

Then:

Also inform others maybe ( https://github.com/dereuromark/cakephp-queue/network/dependents )

So far changelogs are

### Improvements

This new major ships with a few improvements:

* Cleaned up DB migration files using a single snapshot. It is expected that all previous migrations have been run for existing installs.
* Fix up data field to be of JSON type. Conversion to array is now happening automatically, so make sure you are not using manual encode/decode anywhere. Also make sure that you are not using deprecated serialize/unserialize, as now everything is using JSON.
* Support DTOs for `$data` input in `createJob()`
* Support Config object input for `$options` in `createJob()`

For more info on changes coming from v7 see included upgrade guide in docs/ folder.

**Full Changelog**: https://github.com/dereuromark/cakephp-queue/compare/7.2.2...8.0.0
dereuromark commented 8 months ago

I updated the sandbox already with 0 changes needed: https://sandbox.dereuromark.de/sandbox/queue-examples All works well :)

The only thing I find a bit annoying is the manual arrays to be used everywhere without clear info on what options are possible. Here, value objects or DTOs could also be handy.

But I dont want to couple the plugin now to more complexity. For now I use those on user land side:

    <dto name="EventUpdateNotificationQueueData" immutable="true">
        <field name="eventId" type="int" required="true"/>
        <field name="mailer" type="string" required="true"/>
        <field name="type" type="string" required="true"/>
    </dto>

and

$data = OrderUpdateNotificationQueueDataDto::createFromArray([
    OrderUpdateNotificationQueueDataDto::FIELD_ORDER_ID => $order->id,
    OrderUpdateNotificationQueueDataDto::FIELD_TYPE => 'request',
])->toArray();
$this->getTableLocator()->get('Queue.QueuedJobs')->createJob('OrderUpdateNotification', $data);

as well as this inside the queue task:

    public function run(array $data, int $jobId): void
    {
        $queueData = OrderUpdateNotificationQueueDataDto::createFromArray($data);

        $order = $this->fetchTable('Orders')->get($queueData->getOrderId());
        $this->getMailer($queueData->getMailer())->send($queueData->getType(), [$order]);
    }
dereuromark commented 8 months ago

@LordSimal Could we use annotations to auto-work with DTOs maybe even?

LordSimal commented 8 months ago

if the Task constructor would be empty I'd rather use that to set properties via constructor property promotion and do something like

$this->getTableLocator()->get('Queue.QueuedJobs')->createJob('OrderUpdateNotification', $arg1, $arg2, $arg3);

There is no need to pass down the data to the run method since a task can't be run differently. Don't know if you wanna go this way though.

dereuromark commented 8 months ago

How is the object serialized and unserialized regarding payload data?

LordSimal commented 8 months ago

well since there is only json encoding anymore its just a json array of all the args. If a object is being passed down as arg you'd have to check if it can be json encoded by checking the JsonSerializable interface. If not, throw an exception.

dereuromark commented 8 months ago

Yeah, the serialize process is the easy one, but how would it be assembled back?

LordSimal commented 8 months ago
new $className(...$arrayFromDataCol)

?

LordSimal commented 8 months ago

and as far as I know if you easily json_encode an object it should also be rather easily json_decoded

dereuromark commented 8 months ago
public function createJob(string $jobTask, ?array $data = null, array $config = []): QueuedJob {

still has the configs as 3rd arg they also also still magic string assoc arrays so far

I guess we need a functioning prototype to check on pro/con analysis.

We can wait until then or just move on for now, and see if that can be a new minor in the future.

LordSimal commented 8 months ago

actually i was wrong. You can't transform a json back into a object (like it was done with serializing)

So the interface is just to tell PHP how to transform a object into JSON, not the other way around.

LordSimal commented 8 months ago

well it doesn't need to be a variadic type like mentioned above. Could still be just an array of args as the 2nd argument of createJob

dereuromark commented 8 months ago

actually i was wrong. You can't transform a json back into a object (like it was done with serializing)

That's why my favorite would be the DTOs, since they work like a charm for serialize and unserialize.

well it doesn't need to be a variadic type like mentioned above. Could still be just an array of args as the 2nd argument of createJob

Yep

One more IDE and Stan friendly way could be:

// QueueConfig class
$config = $queuedJobsTable->newConfig()
    ->setPriority(1)
    ->setReference('foo');
$queuedJobsTable->createJob('OrderUpdateNotification', $data, $config);

As those options are fixed.

The rest of the dynamic payload will be a bit more complicated to deal with. Even if inside the plugin, they could be extended and would then need a different class.

LordSimal commented 8 months ago

well this would make creating the config more IDE friendly but users would have to create a dto foreach task do get the same level of IDE integration. I personally wouldn't like that since I don't want to create classes just for "type-safety".

dereuromark commented 7 months ago

See https://github.com/dereuromark/cakephp-queue/pull/412 So no, nothing extra needs to be done, just call the fluent interface.

dereuromark commented 7 months ago
Index: src/Queue/Processor.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/Queue/Processor.php b/src/Queue/Processor.php
--- a/src/Queue/Processor.php   (revision a2041884e6c571099ff556e425e099b9b863f92f)
+++ b/src/Queue/Processor.php   (date 1709848438300)
@@ -204,14 +204,21 @@
        try {
            $this->time = time();

-           $data = $queuedJob->data;
+           $data = (array)$queuedJob->data;
+           if (!empty($data['_dtoClass'])) {
+               /** @var \CakeDto\Dto\FromArrayToArrayInterface $class */
+               $class = $data['_dtoClass'];
+               unset($data['_dtoClass']);
+               $data = $class::createFromArray($data);
+           }
+
            $task = $this->loadTask($taskName);
            $traits = class_uses($task);
            if ($this->container && $traits && in_array(ServicesTrait::class, $traits, true)) {
                /** @phpstan-ignore-next-line */
                $task->setContainer($this->container);
            }
-           $task->run((array)$data, $queuedJob->id);
+           $task->run($data, $queuedJob->id);
        } catch (Throwable $e) {
            $return = false;

Index: composer.json
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/composer.json b/composer.json
--- a/composer.json (revision a2041884e6c571099ff556e425e099b9b863f92f)
+++ b/composer.json (date 1709848658594)
@@ -28,7 +28,8 @@
    "require": {
        "php": ">=8.1",
        "brick/varexporter": "^0.4.0",
-       "cakephp/cakephp": "^5.0.0"
+       "cakephp/cakephp": "^5.0.0",
+       "dereuromark/cakephp-dto": "^2.2.0"
    },
    "require-dev": {
        "cakedc/cakephp-phpstan": "^3.0.0",
Index: tests/test_app/src/Queue/Task/FooTask.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/tests/test_app/src/Queue/Task/FooTask.php b/tests/test_app/src/Queue/Task/FooTask.php
--- a/tests/test_app/src/Queue/Task/FooTask.php (revision a2041884e6c571099ff556e425e099b9b863f92f)
+++ b/tests/test_app/src/Queue/Task/FooTask.php (date 1709848576051)
@@ -3,6 +3,7 @@

 namespace TestApp\Queue\Task;

+use CakeDto\Dto\FromArrayToArrayInterface;
 use Queue\Queue\AddInterface;
 use Queue\Queue\ServicesTrait;
 use Queue\Queue\Task;
@@ -29,13 +30,15 @@
     * This function is executed, when a worker is executing a task.
     * The return parameter will determine, if the task will be marked completed, or be requeued.
     *
-    * @param array<string, mixed> $data The array passed to QueuedJobsTable::createJob()
+    * @param \TestApp\Dto\FooTaskDto|null $data
     * @param int $jobId The id of the QueuedJob entity
     *
     * @return void
     */
-   public function run(array $data, int $jobId): void {
-       $this->io->out('CakePHP Foo Example.');
+   public function run(?FromArrayToArrayInterface $data, int $jobId): void {
+       assert($data !== null);
+
+       $this->io->out('CakePHP Foo Example: ' . $data->getFoo());
        $testService = $this->getService(TestService::class);
        $test = $testService->output();
        $this->io->out($test);
Index: src/Queue/TaskInterface.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/Queue/TaskInterface.php b/src/Queue/TaskInterface.php
--- a/src/Queue/TaskInterface.php   (revision a2041884e6c571099ff556e425e099b9b863f92f)
+++ b/src/Queue/TaskInterface.php   (date 1709848658606)
@@ -3,6 +3,8 @@

 namespace Queue\Queue;

+use CakeDto\Dto\FromArrayToArrayInterface;
+
 /**
  * Any task needs to at least implement run().
  * The add() is mainly only for CLI adding purposes and optional.
@@ -17,11 +19,11 @@
    /**
     * Main execution of the task.
     *
-    * @param array<string, mixed> $data The array passed to QueuedJobsTable::createJob()
+    * @param \CakeDto\Dto\FromArrayToArrayInterface|null $data The array passed to QueuedJobsTable::createJob()
     * @param int $jobId The id of the QueuedJob entity
     *
     * @return void
     */
-   public function run(array $data, int $jobId): void;
+   public function run(?FromArrayToArrayInterface $data, int $jobId): void;

 }
Index: src/Model/Table/QueuedJobsTable.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/Model/Table/QueuedJobsTable.php b/src/Model/Table/QueuedJobsTable.php
--- a/src/Model/Table/QueuedJobsTable.php   (revision a2041884e6c571099ff556e425e099b9b863f92f)
+++ b/src/Model/Table/QueuedJobsTable.php   (date 1709848576055)
@@ -13,6 +13,7 @@
 use Cake\ORM\Query\SelectQuery;
 use Cake\ORM\Table;
 use Cake\Validation\Validator;
+use CakeDto\Dto\FromArrayToArrayInterface;
 use InvalidArgumentException;
 use Queue\Config\JobConfig;
 use Queue\Model\Entity\QueuedJob;
@@ -219,16 +220,22 @@
     * - reference: An optional reference string
     *
     * @param string $jobTask Job task name or FQCN
-    * @param array<string, mixed>|null $data Array of data
+    * @param \CakeDto\Dto\FromArrayToArrayInterface|null $data DTO like object or null
     * @param \Queue\Config\JobConfig|array<string, mixed> $config Config to save along with the job
     *
     * @return \Queue\Model\Entity\QueuedJob Saved job entity
     */
-   public function createJob(string $jobTask, ?array $data = null, array|JobConfig $config = []): QueuedJob {
+   public function createJob(string $jobTask, ?FromArrayToArrayInterface $data = null, array|JobConfig $config = []): QueuedJob {
        if (!$config instanceof JobConfig) {
            $config = $this->createConfig()->fromArray($config);
        }

+       if ($data) {
+           $class = get_class($data);
+           $data = $data->toArray();
+           $data['_dtoClass'] = $class;
+       }
+
        $queuedJob = [
            'job_task' => $this->jobTask($jobTask),
            'data' => $data,
Index: tests/test_app/src/Dto/FooTaskDto.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/tests/test_app/src/Dto/FooTaskDto.php b/tests/test_app/src/Dto/FooTaskDto.php
new file mode 100644
--- /dev/null   (date 1709848315125)
+++ b/tests/test_app/src/Dto/FooTaskDto.php (date 1709848315125)
@@ -0,0 +1,42 @@
+<?php
+declare(strict_types=1);
+
+namespace TestApp\Dto;
+
+use CakeDto\Dto\FromArrayToArrayInterface;
+
+class FooTaskDto implements FromArrayToArrayInterface {
+
+   protected array $data;
+
+   /**
+    * @param array $data
+    */
+   public function __construct(array $data) {
+       $this->data = $data;
+   }
+
+   /**
+    * @return string
+    */
+   public function getFoo(): string {
+       return 'foo';
+   }
+
+   /**
+    * @param array $array
+    *
+    * @return static
+    */
+   public static function createFromArray(array $array): static {
+       return new static($array);
+   }
+
+   /**
+    * @return array
+    */
+   public function toArray(): array {
+       return $this->data;
+   }
+
+}

would take it a bit too far, hard-requiring DTOs everywhere. I think for now having the option of using $data as DTO the way shown above seems fair enough - as opt in.

dereuromark commented 7 months ago

@MSeven @inoas @houseoftech @Graziel @jiru @challgren @stmeyer @netstyler @drmonkeyninja @mfrascati @charukiewicz @TeckniX @garas @repher @JacobAGTyler @luke83 @mstroink @kburton-dev @DeeFuse

You have, based on history, also contributed once upon a time - and potentially still using it. Maybe you have some feedback prior to releasing the v8 stable, as well.