Remove deserialization and entity manager merge (#72)

* Add PHP 8.3 to CI tasks * Remove deserialization and entity manager merge * Fix code style * Fix test cases to represent real usecases * Allow JMS serializer 5.0 * Fix compatibility issues * Add deprecation if serializer still defined * Set requirement to ORM only
sulu · Jun 10, 2024 · 629e540 · 629e540
1 parent 42b35db
commit 629e540
Show file tree

Hide file tree

Showing 13 changed files with 103 additions and 98 deletions.
diff --git a/.github/workflows/test-application.yaml b/.github/workflows/test-application.yaml
@@ -9,7 +9,7 @@ on:
 
 jobs:
     test:
-        name: 'PHP ${{ matrix.php-version }} (${{ matrix.dependency-versions }}, Lint ${{ matrix.lint }})'
+        name: 'PHP ${{ matrix.php-version }} (${{ matrix.dependency-versions }})'
         runs-on: ubuntu-latest
 
         env:
@@ -20,35 +20,36 @@ jobs:
             matrix:
                 include:
                     - php-version: '7.2'
-                      lint: false
                       dependency-versions: 'lowest'
                       tools: 'composer:v2'
                       env:
                           SYMFONY_DEPRECATIONS_HELPER: disabled
 
                     - php-version: '7.4'
-                      lint: true
                       dependency-versions: 'highest'
                       tools: 'composer:v2'
                       env:
                           SYMFONY_DEPRECATIONS_HELPER: weak
 
                     - php-version: '8.0'
-                      lint: false
                       dependency-versions: 'highest'
                       tools: 'composer:v2'
                       env:
                           SYMFONY_DEPRECATIONS_HELPER: weak
 
                     - php-version: '8.1'
-                      lint: false
                       dependency-versions: 'highest'
                       tools: 'composer:v2'
                       env:
                           SYMFONY_DEPRECATIONS_HELPER: weak
 
                     - php-version: '8.2'
-                      lint: false
+                      dependency-versions: 'highest'
+                      tools: 'composer:v2'
+                      env:
+                          SYMFONY_DEPRECATIONS_HELPER: weak
+
+                    - php-version: '8.3'
                       dependency-versions: 'highest'
                       tools: 'composer:v2'
                       env:
@@ -74,6 +75,9 @@ jobs:
                   extensions: 'mysql, gd'
                   tools: ${{ matrix.tools }}
 
+            - name: Remove not required tooling
+              run: composer remove php-cs-fixer/shim "*phpstan*" --dev --no-interaction --no-update
+
             - name: Install composer dependencies
               uses: ramsey/composer-install@v1
               with:
@@ -83,10 +87,44 @@ jobs:
             - name: Bootstrap test environment
               run: composer bootstrap-test-environment
 
-            - name: Lint code
-              if: ${{ matrix.lint }}
-              run: composer lint
-
             - name: Execute test cases
               run: composer test
               env: ${{ matrix.env }}
+
+    lint:
+        name: 'PHP Lint'
+        runs-on: ubuntu-latest
+
+        env:
+            DATABASE_URL: 'mysql://root:[email protected]/sulu_automation_test?serverVersion=5.7'
+
+        services:
+            mysql:
+                image: mysql:5.7
+                env:
+                    MYSQL_ROOT_PASSWORD: root
+                ports:
+                    - 3306:3306
+                options: --health-cmd="mysqladmin ping" --health-interval=10s --health-timeout=5s --health-retries=5
+
+        steps:
+            - name: Checkout project
+              uses: actions/checkout@v2
+
+            - name: Install and configure PHP
+              uses: shivammathur/setup-php@v2
+              with:
+                  php-version: 8.3
+                  extensions: 'mysql, gd'
+                  tools: composer:v2
+
+            - name: Install composer dependencies
+              uses: ramsey/composer-install@v1
+              with:
+                  dependency-versions: highest
+
+            - name: Bootstrap test environment
+              run: composer bootstrap-test-environment
+
+            - name: Lint code
+              run: composer lint
diff --git a/.gitignore b/.gitignore
@@ -27,3 +27,6 @@ Tests/Application/.env.test.local
 # php-cs-fixer
 .php-cs-fixer.cache
 php-cs-fixer
+
+.env.local
+.env.*.local
diff --git a/Controller/TaskController.php b/Controller/TaskController.php
@@ -14,14 +14,12 @@
 use Doctrine\ORM\EntityManagerInterface;
 use FOS\RestBundle\Routing\ClassResourceInterface;
 use FOS\RestBundle\View\ViewHandlerInterface;
-use JMS\Serializer\DeserializationContext;
 use JMS\Serializer\SerializerInterface;
 use Sulu\Bundle\AutomationBundle\Admin\AutomationAdmin;
 use Sulu\Bundle\AutomationBundle\Entity\Task;
 use Sulu\Bundle\AutomationBundle\Exception\TaskNotFoundException;
 use Sulu\Bundle\AutomationBundle\TaskHandler\AutomationTaskHandlerInterface;
 use Sulu\Bundle\AutomationBundle\Tasks\Manager\TaskManagerInterface;
-use Sulu\Bundle\AutomationBundle\Tasks\Model\TaskInterface;
 use Sulu\Bundle\AutomationBundle\Tasks\Model\TaskRepositoryInterface as AutomationTaskRepositoryInterface;
 use Sulu\Component\Rest\AbstractRestController;
 use Sulu\Component\Rest\ListBuilder\Doctrine\DoctrineListBuilderFactoryInterface;
@@ -93,7 +91,7 @@ class TaskController extends AbstractRestController implements ClassResourceInte
     protected $entityManager;
 
     /**
-     * @var SerializerInterface
+     * @var SerializerInterface|null
      */
     protected $serializer;
 
@@ -112,7 +110,7 @@ public function __construct(
         RestHelperInterface $doctrineRestHelper,
         TaskManagerInterface $taskManager,
         EntityManagerInterface $entityManager,
-        SerializerInterface $serializer,
+        ?SerializerInterface $serializer,
         FieldDescriptorFactoryInterface $fieldDescriptorFactory,
         AutomationTaskRepositoryInterface $automationTaskRepository
     ) {
@@ -125,6 +123,11 @@ public function __construct(
         $this->taskManager = $taskManager;
         $this->entityManager = $entityManager;
         $this->serializer = $serializer;
+
+        if (null !== $serializer) {
+            @trigger_deprecation('sulu/automation-bundle', '2.1.2', 'The "%s" class not longer should be constructed with a serializer.', self::class);
+        }
+
         $this->fieldDescriptorFactory = $fieldDescriptorFactory;
         $this->automationTaskRepository = $automationTaskRepository;
     }
@@ -298,27 +301,14 @@ public function getCountAction(Request $request): Response
      */
     public function postAction(Request $request): Response
     {
-        $data = \array_merge(
-            [
-                'scheme' => $request->getScheme(),
-                'host' => $request->getHost(),
-                'entityId' => $request->get('entityId'),
-                'entityClass' => $request->get('entityClass'),
-                'locale' => $request->get('locale'),
-            ],
-            \array_filter($request->request->all())
-        );
-
-        $context = DeserializationContext::create();
-        $context->setGroups(['api']);
-
-        /** @var TaskInterface $task */
-        $task = $this->serializer->deserialize(
-            (string) \json_encode($data),
-            Task::class,
-            'json',
-            $context
-        );
+        $task = new Task();
+        $task->setScheme($request->getScheme());
+        $task->setHost($request->getHost());
+        $task->setEntityId((string) $request->query->get('entityId'));
+        $task->setEntityClass((string) $request->query->get('entityClass'));
+        $task->setLocale((string) $request->query->get('locale'));
+        $task->setHandlerClass((string) $request->request->get('handlerClass'));
+        $task->setSchedule(new \DateTime((string) $request->request->get('schedule')));
 
         $this->taskManager->create($task);
 
@@ -332,32 +322,16 @@ public function postAction(Request $request): Response
      */
     public function putAction(string $id, Request $request): Response
     {
-        $data = \array_merge(
-            [
-                'id' => $id,
-                'scheme' => $request->getScheme(),
-                'host' => $request->getHost(),
-                'entityId' => $request->get('entityId'),
-                'entityClass' => $request->get('entityClass'),
-                'locale' => $request->get('locale'),
-            ],
-            \array_filter($request->request->all())
-        );
-
-        $context = DeserializationContext::create();
-        $context->setGroups(['api']);
-
-        /** @var TaskInterface $task */
-        $task = $this->serializer->deserialize(
-            (string) \json_encode($data),
-            Task::class,
-            'json',
-            $context
-        );
+        /** @var Task $task */
+        $task = $this->taskManager->findById($id);
+        $task->setScheme($request->getScheme());
+        $task->setHost($request->getHost());
+        $task->setLocale((string) $request->query->get('locale'));
+        $task->setHandlerClass((string) $request->request->get('handlerClass'));
+        $task->setSchedule(new \DateTime((string) $request->request->get('schedule')));
 
         $task = $this->taskManager->update($task);
 
-        $this->entityManager->merge($task);
         $this->entityManager->flush();
 
         return $this->handleView($this->view($task));
@@ -396,11 +370,9 @@ public function cdeleteAction(Request $request): Response
     /**
      * Returns field-descriptors for task-entity.
      *
-     * @param string $type
-     *
      * @return FieldDescriptorInterface[]
      */
-    private function getFieldDescriptors(string $type = null): array
+    private function getFieldDescriptors(?string $type = null): array
     {
         $fieldDescriptors = $this->fieldDescriptorFactory->getFieldDescriptors(Task::RESOURCE_KEY);
         if (!$fieldDescriptors) {

diff --git a/Entity/DoctrineTaskRepository.php b/Entity/DoctrineTaskRepository.php
@@ -19,6 +19,8 @@
 
 /**
  * Task-Repository implementation for doctrine.
+ *
+ * @extends EntityRepository<TaskInterface>
  */
 class DoctrineTaskRepository extends EntityRepository implements TaskRepositoryInterface
 {
@@ -62,7 +64,7 @@ public function findByTaskId(string $id): ?TaskInterface
         return $task;
     }
 
-    public function countFutureTasks(string $entityClass, string $entityId, string $locale = null): int
+    public function countFutureTasks(string $entityClass, string $entityId, ?string $locale = null): int
     {
         $queryBuilder = $this->createQueryBuilder('task')
             ->select('COUNT(task.id)')
@@ -86,7 +88,7 @@ public function countFutureTasks(string $entityClass, string $entityId, string $
         return (int) $result;
     }
 
-    public function countPendingTasks(string $entityClass, string $entityId, string $locale = null): int
+    public function countPendingTasks(string $entityClass, string $entityId, ?string $locale = null): int
     {
         $queryBuilder = $this->_em->createQueryBuilder()
             ->select('COUNT(taskExecution.uuid)')

diff --git a/Metadata/FormMetadataLoader.php b/Metadata/FormMetadataLoader.php
@@ -54,7 +54,7 @@ public function getMetadata(string $key, string $locale, array $metadataOptions)
             return null;
         }
 
-        /** @var FormMetaData $form */
+        /** @var FormMetadata $form */
         $form = new FormMetadata();
         $form->setKey(self::TASK_DETAILS_VIEW);
 

diff --git a/PageTree/PageTreeRouteUpdateHandler.php b/PageTree/PageTreeRouteUpdateHandler.php
@@ -12,7 +12,6 @@
 namespace Sulu\Bundle\AutomationBundle\PageTree;
 
 use Doctrine\ORM\EntityManagerInterface;
-use Exception;
 use Sulu\Bundle\AutomationBundle\TaskHandler\AutomationTaskHandlerInterface;
 use Sulu\Bundle\AutomationBundle\TaskHandler\TaskHandlerConfiguration;
 use Sulu\Bundle\PageBundle\Document\BasePageDocument;
@@ -83,7 +82,7 @@ public function handle($workload)
 
             $this->documentManager->flush();
             $this->entityManager->commit();
-        } catch (Exception $exception) {
+        } catch (\Exception $exception) {
             $this->entityManager->rollback();
 
             throw $exception;

diff --git a/Resources/config/services.xml b/Resources/config/services.xml
@@ -21,7 +21,7 @@
             <argument type="service" id="sulu_core.doctrine_rest_helper"/>
             <argument type="service" id="sulu_automation.tasks.manager"/>
             <argument type="service" id="doctrine.orm.entity_manager"/>
-            <argument type="service" id="jms_serializer"/>
+            <argument>null</argument>
             <argument type="service" id="sulu_core.list_builder.field_descriptor_factory"/>
             <argument type="service" id="sulu.repository.task"/>
 

diff --git a/Tasks/Model/TaskInterface.php b/Tasks/Model/TaskInterface.php
@@ -25,8 +25,6 @@ public function getId(): string;
 
     /**
      * Set id.
-     *
-     * @return TaskInterface
      */
     public function setId(string $id): self;
 
@@ -40,9 +38,6 @@ public function getHandlerClass(): string;
      */
     public function getSchedule(): \DateTime;
 
-    /**
-     * @return TaskInterface
-     */
     public function setSchedule(\DateTime $schedule): self;
 
     /**
@@ -67,8 +62,6 @@ public function getTaskId(): ?string;
 
     /**
      * Set taskId.
-     *
-     * @return TaskInterface
      */
     public function setTaskId(?string $taskId): self;
 

diff --git a/Tasks/Model/TaskRepositoryInterface.php b/Tasks/Model/TaskRepositoryInterface.php
@@ -46,12 +46,12 @@ public function findByTaskId(string $id): ?TaskInterface;
      *
      * Count tasks which have a schedule date in the future
      */
-    public function countFutureTasks(string $entityClass, string $entityId, string $locale = null): int;
+    public function countFutureTasks(string $entityClass, string $entityId, ?string $locale = null): int;
 
     /**
      * Count pending tasks which have not been executed yet.
      */
-    public function countPendingTasks(string $entityClass, string $entityId, string $locale = null): int;
+    public function countPendingTasks(string $entityClass, string $entityId, ?string $locale = null): int;
 
     /**
      * Revert given task-entity.

diff --git a/Tests/Application/bin/console.php b/Tests/Application/bin/console.php
@@ -33,7 +33,7 @@
     if (\class_exists(Debug::class)) {
         Debug::enable();
     } else {
-        \Symfony\Component\Debug\Debug::enable();
+        Symfony\Component\Debug\Debug::enable();
     }
 }
 

diff --git a/Tests/Functional/Controller/TaskControllerTest.php b/Tests/Functional/Controller/TaskControllerTest.php
@@ -270,13 +270,10 @@ public function testPost(
 
         $this->client->request(
             'POST',
-            '/api/tasks',
+            '/api/tasks?locale=' . $locale . '&entityClass=' . $entityClass . '&entityId=' . $entityId,
             [
                 'handlerClass' => $handlerClass,
                 'schedule' => $date->format('Y-m-d\TH:i:s'),
-                'entityClass' => $entityClass,
-                'entityId' => $entityId,
-                'locale' => $locale,
             ]
         );
         $this->assertHttpStatusCode(200, $this->client->getResponse());
@@ -310,13 +307,10 @@ public function testPut(
 
         $this->client->request(
             'PUT',
-            '/api/tasks/' . $postData['id'],
+            '/api/tasks/' . $postData['id'] . '?locale=' . $locale . '&entityClass=' . $entityClass . '&entityId=' . $postData['entityId'],
             [
                 'handlerClass' => $handlerClass,
-                'entityId' => $postData['entityId'],
                 'taskId' => $postData['taskId'],
-                'entityClass' => $entityClass,
-                'locale' => $locale,
                 'schedule' => $date->format('Y-m-d\TH:i:s'),
             ]
         );