diff --git a/CHANGELOG.md b/CHANGELOG.md index 4530610..9c8f2b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ Latest ------ ## Changes +* [#181](https://github.com/cleverage/ui-process-bundle/issues/181) Fix the deprecations revealed by the tests, which now fail on deprecations: `ProcessExecutionRepository::getLastProcessExecution()` passes the `\SortDirection` enum to `orderBy()` when doctrine/orm accepts it (3.7 or later, where a string direction is deprecated, [#152](https://github.com/cleverage/ui-process-bundle/issues/152)); `ProcessExecutionRepository::hasLogs()` counts the log records without an arbitrary join (`WITH` deprecated by doctrine/orm 3.7, same result); the date filters of the log records (`createdAt`) and of the process executions (`startDate`) use the `single_text` widget (not configuring it is deprecated by Symfony 6.4, already the default since Symfony 7.0); `User::eraseCredentials()` is marked `#[\Deprecated]`, as required by Symfony 7.3 (with Symfony 6.4 and PHP 8.4 or later, which still call it, set `security.erase_credentials: false` to avoid the deprecation). * [#180](https://github.com/cleverage/ui-process-bundle/issues/180) Define the public API (Symfony BC promise, as [cleverage/process-bundle#279](https://github.com/cleverage/process-bundle/issues/279)): `ProcessExecutionManager`, `ProcessExecutionRepository`, `ProcessScheduleRepository`, `ProcessExecuteMessage`, `ProcessExecutionNotification` and `CleverAgeUiProcessBundle` are marked `@final since 3.3`, they will be `final` in 4.0: extending them triggers a deprecation with the Symfony `DebugClassLoader`. The entities stay open (overriding them: [#37](https://github.com/cleverage/ui-process-bundle/issues/37)). The infrastructure classes are marked `@internal`: the extension, `Configuration`, the commands, the controllers, the EasyAdmin fields and filters, the form types, the Messenger handlers, the Monolog handlers, `HttpProcessExecuteValueResolver`, `HttpProcessExecutionAuthenticator`, `ProcessExecutionStopChecker`, `CronScheduler`, `ProcessEventSubscriber` and the Twig extensions, runtimes and components. * [#179](https://github.com/cleverage/ui-process-bundle/issues/179) `CleverAgeUiProcessExtension::findServices()` loads the service definition files with a glob pattern instead of a `Finder` (same files, as [cleverage/process-bundle#271](https://github.com/cleverage/process-bundle/issues/271)). Add tests. * [#172](https://github.com/cleverage/ui-process-bundle/issues/172) Prefix the route names of the bundle with `cleverage_ui_process_`, as required by the Symfony best practices for bundles: `cleverage_ui_process_login`, `cleverage_ui_process_logout`, `cleverage_ui_process_list`, `cleverage_ui_process_launch`, `cleverage_ui_process_http_execute`, and the dashboard `cleverage_ui_process`, so that the routes generated by EasyAdmin for the CRUD pages are `cleverage_ui_process__`. The former names (`process`, `process__`, `process_login`, `process_logout`, `process_list`, `process_launch`, `http_process_execute`) are deprecated route aliases (new `config/routes/deprecated_aliases.yaml`, loaded by the documented import, and internal `DeprecatedEasyAdminRouteAliasLoader` for the routes generated by EasyAdmin), removed in 4.0; `?routeName=` URLs keep working. Document that `easyadmin.routes` generates the routes of every dashboard. Update documentation and UPGRADE.md, add tests. diff --git a/composer.json b/composer.json index 8ea6225..59189eb 100644 --- a/composer.json +++ b/composer.json @@ -90,6 +90,7 @@ "require-dev": { "doctrine/doctrine-fixtures-bundle": "^3 || ^4", "friendsofphp/php-cs-fixer": "*", + "masterminds/html5": "^2.7.5", "phpstan/extension-installer": "*", "phpstan/phpstan": "*", "phpstan/phpstan-doctrine": "*", diff --git a/phpunit.xml.dist b/phpunit.xml.dist index 54aa561..e00cece 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -1,14 +1,16 @@ + failOnWarning="true" + failOnDeprecation="true" + displayDetailsOnTestsThatTriggerDeprecations="true"> tests @@ -18,7 +20,7 @@ - + src diff --git a/rector.php b/rector.php index 3ca9cbf..20b2669 100644 --- a/rector.php +++ b/rector.php @@ -4,6 +4,7 @@ use Rector\Config\RectorConfig; use Rector\DeadCode\Rector\ClassMethod\RemoveUnusedConstructorParamRector; +use Rector\DeadCode\Rector\If_\RemoveAlwaysTrueIfConditionRector; use Rector\Symfony\Set\SymfonySetList; use Rector\ValueObject\PhpVersion; @@ -26,5 +27,9 @@ RemoveUnusedConstructorParamRector::class => [ __DIR__.'/src/Controller/Admin/ProcessExecutionCrudController.php', ], + // Symfony version checks of the test kernel, always true with the installed version only + RemoveAlwaysTrueIfConditionRector::class => [ + __DIR__.'/tests/App/TestKernel.php', + ], ]) ; diff --git a/src/Controller/Admin/LogRecordCrudController.php b/src/Controller/Admin/LogRecordCrudController.php index c37352f..e08b84c 100644 --- a/src/Controller/Admin/LogRecordCrudController.php +++ b/src/Controller/Admin/LogRecordCrudController.php @@ -28,6 +28,7 @@ use EasyCorp\Bundle\EasyAdminBundle\Field\DateTimeField; use EasyCorp\Bundle\EasyAdminBundle\Field\TextField; use EasyCorp\Bundle\EasyAdminBundle\Filter\ChoiceFilter; +use EasyCorp\Bundle\EasyAdminBundle\Filter\DateTimeFilter; use Monolog\Level; use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\Security\Http\Attribute\IsGranted; @@ -106,6 +107,10 @@ public function configureFilters(Filters $filters): Filters array_map(static fn ($value) => 'enum.log_level.'.strtolower((string) $value), Level::NAMES) )) ->setFormTypeOption('translation_domain', 'enums'), - )->add('message')->add('context')->add('createdAt'); + )->add('message')->add('context')->add( + // The value_type_options set by EasyAdmin replace the widget of its date filter type, whose default is + // deprecated with Symfony 6.4 (single_text since Symfony 7.0) + DateTimeFilter::new('createdAt')->setFormTypeOption('value_type_options.widget', 'single_text') + ); } } diff --git a/src/Controller/Admin/ProcessExecutionCrudController.php b/src/Controller/Admin/ProcessExecutionCrudController.php index 5e8cf6f..fdbf4ee 100644 --- a/src/Controller/Admin/ProcessExecutionCrudController.php +++ b/src/Controller/Admin/ProcessExecutionCrudController.php @@ -29,6 +29,7 @@ use EasyCorp\Bundle\EasyAdminBundle\Field\ArrayField; use EasyCorp\Bundle\EasyAdminBundle\Field\DateTimeField; use EasyCorp\Bundle\EasyAdminBundle\Field\TextField; +use EasyCorp\Bundle\EasyAdminBundle\Filter\DateTimeFilter; use EasyCorp\Bundle\EasyAdminBundle\Router\AdminUrlGenerator; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Response; @@ -252,7 +253,9 @@ public function configureFilters(Filters $filters): Filters { return $filters ->add('code') - ->add('startDate') + // The value_type_options set by EasyAdmin replace the widget of its date filter type, whose default is + // deprecated with Symfony 6.4 (single_text since Symfony 7.0) + ->add(DateTimeFilter::new('startDate')->setFormTypeOption('value_type_options.widget', 'single_text')) ->add( ProcessExecutionDurationFilter::new('duration', 'Duration (in seconds)') ->setFormTypeOption('mapped', false) diff --git a/src/Entity/User.php b/src/Entity/User.php index 91a20be..9eee0ae 100644 --- a/src/Entity/User.php +++ b/src/Entity/User.php @@ -173,6 +173,10 @@ public function setToken(?string $token): self return $this; } + /** + * Required by UserInterface before Symfony 8.0: no temporary sensitive data is stored on the user. + */ + #[\Deprecated] public function eraseCredentials(): void { // If you store any temporary, sensitive data on the user, clear it here diff --git a/src/Repository/ProcessExecutionRepository.php b/src/Repository/ProcessExecutionRepository.php index 18a1e17..f8e5ec2 100644 --- a/src/Repository/ProcessExecutionRepository.php +++ b/src/Repository/ProcessExecutionRepository.php @@ -17,6 +17,7 @@ use CleverAge\UiProcessBundle\Entity\ProcessExecution; use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\EntityRepository; +use Doctrine\ORM\QueryBuilder; use Doctrine\Persistence\ManagerRegistry; /** @@ -80,7 +81,7 @@ public function getLastProcessExecution(string $code): ?ProcessExecution /** @var ProcessExecution|null $processExecution */ $processExecution = $qb->select('pe') ->where($qb->expr()->eq('pe.code', $qb->expr()->literal($code))) - ->orderBy('pe.startDate', 'DESC') + ->orderBy('pe.startDate', $this->descending()) ->setMaxResults(1) ->getQuery() ->getOneOrNullResult(); @@ -90,13 +91,24 @@ public function getLastProcessExecution(string $code): ?ProcessExecution public function hasLogs(ProcessExecution $processExecution): bool { - $qb = $this->createQueryBuilder('pe') + $qb = $this->getEntityManager()->createQueryBuilder() ->select('count(lr.id)') - ->join(LogRecord::class, 'lr', 'WITH', 'lr.processExecution = pe') - ->where('pe.id = :id') - ->setParameter('id', $processExecution->getId() - ); + ->from(LogRecord::class, 'lr') + ->where('lr.processExecution = :processExecution') + ->setParameter('processExecution', $processExecution->getId()); return (int) $qb->getQuery()->getSingleScalarResult() > 0; } + + /** + * doctrine/orm 3.7 deprecates the string sort directions in favor of the \SortDirection enum (PHP 8.6, polyfilled), + * which the previous versions do not accept (the enum may exist with them, e.g. on PHP 8.6). + */ + private function descending(): \SortDirection|string + { + static $acceptsSortDirection = null; + $acceptsSortDirection ??= str_contains((string) (new \ReflectionMethod(QueryBuilder::class, 'orderBy'))->getParameters()[1]->getType(), 'SortDirection'); + + return $acceptsSortDirection ? \SortDirection::Descending : 'DESC'; + } } diff --git a/tests/App/TestKernel.php b/tests/App/TestKernel.php index a3a174b..68a7c74 100644 --- a/tests/App/TestKernel.php +++ b/tests/App/TestKernel.php @@ -14,6 +14,9 @@ namespace CleverAge\UiProcessBundle\Tests\App; use CleverAge\ProcessBundle\CleverAgeProcessBundle; +use CleverAge\ProcessBundle\Registry\TaskRegistry; +use CleverAge\ProcessBundle\Task\DummyTask; +use CleverAge\ProcessBundle\Task\File\FileReaderTask; use CleverAge\UiProcessBundle\CleverAgeUiProcessBundle; use Doctrine\Bundle\DoctrineBundle\DoctrineBundle; use Doctrine\Bundle\MigrationsBundle\DoctrineMigrationsBundle; @@ -25,6 +28,7 @@ use Symfony\Bundle\SecurityBundle\SecurityBundle; use Symfony\Bundle\TwigBundle\TwigBundle; use Symfony\Component\HttpKernel\Kernel; +use Symfony\Component\Yaml\Yaml; use Symfony\UX\TwigComponent\TwigComponentBundle; use Twig\Extra\TwigExtraBundle\TwigExtraBundle; @@ -36,6 +40,14 @@ class TestKernel extends Kernel { use MicroKernelTrait; + /** + * Service of the tasks used by config/processes.yaml, by code. + */ + private const TASK_SERVICES = [ + 'dummy' => DummyTask::class, + 'file_reader' => FileReaderTask::class, + ]; + private static ?string $runDir = null; public function registerBundles(): iterable @@ -71,6 +83,14 @@ public function getLogDir(): string protected function configureContainer(\Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator $container): void { $container->import(__DIR__.'/config/packages.yaml'); + $container->extension('clever_age_process', $this->getProcessConfigurations()); + if (Kernel::VERSION_ID >= 70300) { + // Not setting it is deprecated since Symfony 7.3: its default value changes in 8.0 + $container->extension('framework', ['property_info' => ['with_constructor_extractor' => true]]); + } else { + // Symfony 6.4 calls User::eraseCredentials(), marked #[\Deprecated] for Symfony 7.3 (deprecation since PHP 8.4) + $container->extension('security', ['erase_credentials' => false]); + } // Doctrine ORM 2 (lowest dependencies): lazy ghost proxies, as configured by the Symfony recipe, the legacy // proxies cannot unset the readonly properties of the entities (ProcessExecution::$code...), see // CheckDoctrineLazyGhostObjectsPass @@ -84,6 +104,31 @@ protected function configureRoutes(\Symfony\Component\Routing\Loader\Configurato $routes->import(__DIR__.'/config/routes.yaml'); } + /** + * The processes of config/processes.yaml, whose tasks are referenced by code (task: ): by their service with + * cleverage/process-bundle < 5.4. + * + * @return array + */ + private function getProcessConfigurations(): array + { + /** @var array{configurations: array>}>} $configurations */ + $configurations = Yaml::parseFile(__DIR__.'/config/processes.yaml'); + if (class_exists(TaskRegistry::class)) { + return $configurations; + } + foreach ($configurations['configurations'] as $processCode => $process) { + foreach ($process['tasks'] as $taskCode => $task) { + \assert(\is_string($task['task']) && isset(self::TASK_SERVICES[$task['task']])); + $task['service'] = '@'.self::TASK_SERVICES[$task['task']]; + unset($task['task']); + $configurations['configurations'][$processCode]['tasks'][$taskCode] = $task; + } + } + + return $configurations; + } + public static function getRunDir(): string { return self::$runDir ??= sys_get_temp_dir().'/ui_process_bundle_tests_'.getmypid(); diff --git a/tests/App/config/packages.yaml b/tests/App/config/packages.yaml index 600e425..a57c681 100644 --- a/tests/App/config/packages.yaml +++ b/tests/App/config/packages.yaml @@ -7,8 +7,18 @@ framework: secret: 'test' test: true http_method_override: false + # Defaults of Symfony 7.0, not setting them is deprecated with Symfony 6.4 + handle_all_throwables: true + php_errors: + log: true + uid: + default_uuid_version: 7 + time_based_uuid_version: 7 session: storage_factory_id: session.storage.factory.mock_file + handler_id: null + cookie_secure: auto + cookie_samesite: lax router: utf8: true messenger: @@ -53,93 +63,6 @@ monolog: twig: default_path: '%kernel.project_dir%/templates' -clever_age_process: - configurations: - test.process: - description: 'Process used by the functional tests' - tasks: - data: - service: '@CleverAge\ProcessBundle\Task\DummyTask' - test.form: - description: 'Process launched with the UI form, text input' - entry_point: data - options: - ui: - ui_launch_mode: form - entrypoint_type: text - default: - input: 'default input' - context: - - key: 'key1' - value: 'value1' - tasks: - data: - service: '@CleverAge\ProcessBundle\Task\DummyTask' - test.form_constraints: - description: 'Process launched with the UI form, with constraints on the context' - entry_point: data - options: - ui: - ui_launch_mode: form - constraints: - - Collection: - fields: - input: ~ - context: - - Collection: - fields: - key1: - - NotBlank: ~ - tasks: - data: - service: '@CleverAge\ProcessBundle\Task\DummyTask' - test.upload: - description: 'Process launched with the UI form, file input' - entry_point: data - options: - ui: - ui_launch_mode: form - entrypoint_type: file - tasks: - data: - service: '@CleverAge\ProcessBundle\Task\DummyTask' - test.private: - public: false - tasks: - data: - service: '@CleverAge\ProcessBundle\Task\DummyTask' - test.failing: - description: 'Process failing on initialization (missing required option)' - tasks: - data: - service: '@CleverAge\ProcessBundle\Task\ConstantOutputTask' - test.notification: - public: false - options: - notification: - enabled: true - statuses: [finish] - tasks: - data: - service: '@CleverAge\ProcessBundle\Task\DummyTask' - test.notification_failing: - public: false - options: - notification: - enabled: true - tasks: - data: - service: '@CleverAge\ProcessBundle\Task\ConstantOutputTask' - test.logs_retention: - public: false - options: - logs: - database_max_days: 90 - file_max_days: 365 - tasks: - data: - service: '@CleverAge\ProcessBundle\Task\DummyTask' - twig_component: anonymous_template_directory: 'components/' defaults: diff --git a/tests/App/config/processes.yaml b/tests/App/config/processes.yaml new file mode 100644 index 0000000..aefe603 --- /dev/null +++ b/tests/App/config/processes.yaml @@ -0,0 +1,91 @@ +# Processes of the functional tests, loaded by TestKernel: the tasks are referenced by code (task: ), replaced +# by their service with cleverage/process-bundle < 5.4 +configurations: + test.process: + description: 'Process used by the functional tests' + tasks: + data: + task: dummy + test.form: + description: 'Process launched with the UI form, text input' + entry_point: data + options: + ui: + ui_launch_mode: form + entrypoint_type: text + default: + input: 'default input' + context: + - key: 'key1' + value: 'value1' + tasks: + data: + task: dummy + test.form_constraints: + description: 'Process launched with the UI form, with constraints on the context' + entry_point: data + options: + ui: + ui_launch_mode: form + constraints: + - Collection: + fields: + input: ~ + context: + - Collection: + fields: + key1: + - NotBlank: ~ + tasks: + data: + task: dummy + test.upload: + description: 'Process launched with the UI form, file input' + entry_point: data + options: + ui: + ui_launch_mode: form + entrypoint_type: file + tasks: + data: + task: dummy + test.private: + public: false + tasks: + data: + task: dummy + test.failing: + description: 'Process failing on execution (missing file)' + tasks: + data: + task: file_reader + options: + filename: '/nonexistent/file.txt' + test.notification: + public: false + options: + notification: + enabled: true + statuses: [finish] + tasks: + data: + task: dummy + test.notification_failing: + public: false + options: + notification: + enabled: true + tasks: + data: + task: file_reader + options: + filename: '/nonexistent/file.txt' + test.logs_retention: + public: false + options: + logs: + database_max_days: 90 + file_max_days: 365 + tasks: + data: + task: dummy diff --git a/tests/Command/UserCreateCommandTest.php b/tests/Command/UserCreateCommandTest.php index 1ffe9b1..e2f835b 100644 --- a/tests/Command/UserCreateCommandTest.php +++ b/tests/Command/UserCreateCommandTest.php @@ -25,6 +25,9 @@ use Symfony\Component\PasswordHasher\Hasher\PasswordHasherFactory; use Symfony\Component\PasswordHasher\Hasher\UserPasswordHasher; use Symfony\Component\PasswordHasher\Hasher\UserPasswordHasherInterface; +use Symfony\Component\Validator\Constraints\Email; +use Symfony\Component\Validator\Constraints\EmailValidator; +use Symfony\Component\Validator\ConstraintValidatorFactory; use Symfony\Component\Validator\Validation; #[CoversClass(UserCreateCommand::class)] @@ -127,7 +130,10 @@ private function createCommand(EntityManagerInterface $entityManager): Command { $application = new Application(); $application->addCommands([new UserCreateCommand( - Validation::createValidator(), + // HTML5 email validation, as configured by the applications (the "loose" default is deprecated with Symfony 6.4) + Validation::createValidatorBuilder() + ->setConstraintValidatorFactory(new ConstraintValidatorFactory([EmailValidator::class => new EmailValidator(Email::VALIDATION_MODE_HTML5)])) + ->getValidator(), $this->passwordHasher, $entityManager, )]); diff --git a/tests/Entity/UserTest.php b/tests/Entity/UserTest.php index d030b08..7f92d45 100644 --- a/tests/Entity/UserTest.php +++ b/tests/Entity/UserTest.php @@ -58,7 +58,13 @@ public function testGettersAndSetters(): void self::assertSame('token', $user->getToken()); self::assertSame(['ROLE_USER', 'ROLE_ADMIN'], $user->getRoles()); - $user->eraseCredentials(); + // Marked #[\Deprecated] (Symfony 7.3): calling it triggers a deprecation with PHP 8.4 or later + set_error_handler(static fn (): bool => true, \E_USER_DEPRECATED); + try { + $user->eraseCredentials(); + } finally { + restore_error_handler(); + } self::assertSame('hashed', $user->getPassword()); $user->setFirstname(null)->setLastname(null)->setTimezone(null)->setLocale(null)->setToken(null); diff --git a/tests/Functional/HttpProcessExecuteTest.php b/tests/Functional/HttpProcessExecuteTest.php index a291bfd..fb6c487 100644 --- a/tests/Functional/HttpProcessExecuteTest.php +++ b/tests/Functional/HttpProcessExecuteTest.php @@ -138,7 +138,7 @@ public function testFailingSynchronousExecution(): void self::assertResponseStatusCodeSame(500); $content = json_decode((string) $this->client->getResponse()->getContent(), true); self::assertIsString($content); - self::assertStringContainsString('output', $content); + self::assertStringContainsString('File does not exists', $content); // The id of the failed process execution, to find its logs in the UI $execution = $this->getEntityManager()->getRepository(ProcessExecution::class)->findOneBy(['code' => 'test.failing']); self::assertInstanceOf(ProcessExecution::class, $execution); diff --git a/tests/Functional/LogRecordCrudTest.php b/tests/Functional/LogRecordCrudTest.php index 3bd0b29..1684e50 100644 --- a/tests/Functional/LogRecordCrudTest.php +++ b/tests/Functional/LogRecordCrudTest.php @@ -32,6 +32,8 @@ use CleverAge\UiProcessBundle\Manager\ProcessConfigurationsManager; use CleverAge\UiProcessBundle\Monolog\Handler\DoctrineProcessHandler; use CleverAge\UiProcessBundle\Monolog\Handler\ProcessHandler; +use CleverAge\UiProcessBundle\Notifier\NotificationTrigger; +use CleverAge\UiProcessBundle\Notifier\ProcessExecutionNotifier; use CleverAge\UiProcessBundle\Repository\ProcessExecutionRepository; use CleverAge\UiProcessBundle\Routing\DeprecatedEasyAdminRouteAliasLoader; use CleverAge\UiProcessBundle\Security\HttpProcessExecutionAuthenticator; @@ -71,6 +73,8 @@ #[UsesClass(ProcessExecutionRepository::class)] #[UsesClass(DoctrineProcessHandler::class)] #[UsesClass(ProcessHandler::class)] +#[UsesClass(NotificationTrigger::class)] +#[UsesClass(ProcessExecutionNotifier::class)] class LogRecordCrudTest extends FunctionalTestCase { public function testIndexAndProcessFilter(): void diff --git a/tests/Functional/ProcessListTest.php b/tests/Functional/ProcessListTest.php index 04bacbe..42520b0 100644 --- a/tests/Functional/ProcessListTest.php +++ b/tests/Functional/ProcessListTest.php @@ -96,10 +96,24 @@ public function testDeprecatedRouteName(): void { $this->login(); - $crawler = $this->client->request('GET', '/process?routeName=process_list'); + $deprecations = []; + set_error_handler(static function (int $errno, string $errstr) use (&$deprecations): bool { + $deprecations[] = $errstr; + + return true; + }, \E_USER_DEPRECATED); + try { + $crawler = $this->client->request('GET', '/process?routeName=process_list'); + } finally { + restore_error_handler(); + } self::assertResponseIsSuccessful(); self::assertStringContainsString('test.process', $crawler->filter('table')->text()); + self::assertContains( + 'Since cleverage/ui-process-bundle 3.3: The "process_list" route alias is deprecated, use the "cleverage_ui_process_list" route instead. It will be removed in 4.0.', + $deprecations + ); } public function testUsersMenuIsReservedToAdmins(): void diff --git a/tests/bootstrap.php b/tests/bootstrap.php new file mode 100644 index 0000000..aebf034 --- /dev/null +++ b/tests/bootstrap.php @@ -0,0 +1,19 @@ +