From 35811e172de21bf18aaf6cdc3687500d5f835278 Mon Sep 17 00:00:00 2001 From: Kirill Hramov Date: Wed, 5 Aug 2026 13:01:20 +0300 Subject: [PATCH 1/7] Add `needReinstall` status to `ApplicationInstallation` - Introduced `markAsNeedReinstall` method for transitioning `new` installations to `needReinstall` status. - Added `ApplicationInstallationMarkedNeedReinstallEvent` domain event. - Implemented repository method for finding stale installations. - Updated guards and allowed transitions in `applicationUninstalled` logic. - Introduced associated tests and documentation updates. --- .tasks/92/mark-old-installations-plan.md | 178 ++++++++++++++++++ .../Entity/ApplicationInstallation.php | 24 ++- .../ApplicationInstallationRepository.php | 41 ++++ 3 files changed, 242 insertions(+), 1 deletion(-) create mode 100644 .tasks/92/mark-old-installations-plan.md diff --git a/.tasks/92/mark-old-installations-plan.md b/.tasks/92/mark-old-installations-plan.md new file mode 100644 index 00000000..497cca3a --- /dev/null +++ b/.tasks/92/mark-old-installations-plan.md @@ -0,0 +1,178 @@ +## План по issue #92: фоновая очистка зависших установок — статус needReinstall + +### Summary + +Issue #92 — фоновая очистка зависших установок в статусе `new`, до которых не дошёл `ONAPPINSTALL`. +Реализуется в два этапа: сначала добавление нового статуса `needReinstall` в SDK, затем использование его в этой библиотеке. + +### Scope + +**В scope:** +- Новый статус `needReinstall` в SDK (`ApplicationInstallationStatus`) +- Метод `markAsNeedReinstall()` в SDK-интерфейсе и entity +- Обновление guard в `applicationUninstalled()` для `needReinstall` +- Новое доменное событие `ApplicationInstallationMarkedNeedReinstallEvent` +- UseCase `MarkOldInstallations` (поиск + перевод в `needReinstall`) +- Repository метод для поиска зависших установок (в локальном репо, потом в SDK) +- Console-команда `bitrix24:installations:mark-old` +- Unit + Functional тесты +- Документация и CHANGELOG + +**Out of scope:** +- Проверка реального состояния портала через SDK (`--verify-portal`) +- Автоматическое планирование (cron, worker) — задача потребителя +- Изменение `Bitrix24AccountStatus` — статус `needReinstall` только у `ApplicationInstallation` +- UseCase переустановки — достаточно существующего `Install`, который через `applicationUninstalled` удалит `needReinstall` установку + +### Target Contract + +1. **Машина состояний ApplicationInstallation после изменений:** +``` +new → active (applicationInstalled) +new → blocked (markAsBlocked) +new → needReinstall (markAsNeedReinstall) ← НОВОЕ +blocked → active (markAsActive) +blocked → deleted (applicationUninstalled) +active → blocked (markAsBlocked) +active → deleted (applicationUninstalled) +needReinstall → deleted (applicationUninstalled) ← НОВОЕ +``` + +2. **markOldInstallations flow:** +``` +Console command (ttl=3600 по дефолту) + → MarkOldInstallations\Command(ttl) + → Handler::handle() + → repository.findStaleInstallations(status=new, olderThan=NOW()-ttl) + → для каждого: installation.markAsNeedReinstall(comment) + → repository.save(installation) + → flusher.flush(installation) + → событие ApplicationInstallationMarkedNeedReinstallEvent диспатчится +``` + +3. **Переустановка после markOldInstallations:** +``` +Install\Command (повторная установка) + → Install\Handler::handle() + → findByBitrix24AccountMemberId находит installation в needReinstall + → deactivateCurrentInstallation() + → applicationUninstalled(null) // needReinstall → deleted (guard обновлён) + → bitrix24Account.applicationUninstalled(null) // new → deleted (уже работает) + → создание новой пары +``` + +### Implementation Changes + +#### PR 1: SDK (bitrix24/b24phpsdk) + +**Файлы SDK:** + +| Файл | Изменение | +|---|---| +| `src/Application/Contracts/ApplicationInstallations/Entity/ApplicationInstallationStatus.php` | Добавить `case needReinstall = 'needReinstall'` | +| `src/Application/Contracts/ApplicationInstallations/Entity/ApplicationInstallationInterface.php` | Добавить метод `markAsNeedReinstall(?string $comment): void` + обновить docblock `applicationUninstalled` | +| `src/Application/Contracts/ApplicationInstallations/Events/ApplicationInstallationMarkedNeedReinstallEvent.php` | Новое событие (id, updatedAt, comment) | + +#### PR 2: bitrix24-php-lib (этот репозиторий) + +**Домен — `src/ApplicationInstallations/Entity/ApplicationInstallation.php`:** + +| Изменение | Детали | +|---|---| +| `markAsNeedReinstall(?string $comment)` | Переход `new → needReinstall`, emit `ApplicationInstallationMarkedNeedReinstallEvent` | +| Guard в `applicationUninstalled()` | Добавить `needReinstall` в допустимые статусы | + +**Событие — `src/ApplicationInstallations/Entity/ApplicationInstallationMarkedNeedReinstallEvent.php`:** + +```php +readonly class ApplicationInstallationMarkedNeedReinstallEvent { + public function __construct( + public Uuid $id, + public CarbonImmutable $updatedAt, + public ?string $comment + ) {} +} +``` + +**Repository — `src/ApplicationInstallations/Infrastructure/Repository/StaleInstallationFinderInterface.php`:** + +```php +interface StaleInstallationFinderInterface { + public function findStaleInstallations( + ApplicationInstallationStatus $status, + CarbonImmutable $olderThan, + ?string $memberId = null + ): array; +} +``` + +**Repository — `src/ApplicationInstallations/Infrastructure/Doctrine/StaleInstallationFinder.php`:** + +- JOIN к `Bitrix24Account` для memberId +- WHERE `status = :status AND createdAt < :olderThan` +- Опциональный фильтр `AND b24.memberId = :memberId` + +**Repository — `tests/Helpers/ApplicationInstallations/InMemoryStaleInstallationFinder.php`:** + +- In-memory реализация `StaleInstallationFinderInterface` для тестов + +**UseCase — `src/ApplicationInstallations/UseCase/MarkOldInstallations/`:** + +| Файл | Содержание | +|---|---| +| `Command.php` | `public function __construct(public int $ttlInSeconds = self::DEFAULT_TTL)` + `DEFAULT_TTL = 3600` | +| `Handler.php` | Находит зависшие через `StaleInstallationFinderInterface`, вызывает `markAsNeedReinstall()`, сохраняет, flush | + +**Console — `src/Console/MarkOldInstallationsCommand.php`:** + +```php +protected static $defaultName = 'bitrix24:installations:mark-old'; +// --ttl=3600 (опционально, дефолт из Command::DEFAULT_TTL) +``` + +**Документация — `src/ApplicationInstallations/Docs/application-installations.md`:** + +- Добавить секцию «Stale Installation Cleanup» с описанием flow +- Sequence diagram для markOldInstallations +- Sequence diagram для reinstall после needReinstall +- Обновить секцию Follow-Up — отметить что issue #92 реализован + +**CHANGELOG.md:** + +- Новая запись: feature — mark old installations as needReinstall via background command + +### Test Cases + +#### Unit Tests + +**MarkOldInstallations/HandlerTest:** +1. Находит одну зависшую установку → переводит в `needReinstall`, событие диспатчится +2. Находит несколько зависших → все переведены +3. Нет зависших → ничего не делает, flush не вызван +4. TTL = 0 → находит все `new` установки + +**MarkOldInstallations/CommandTest:** +1. Дефолтный TTL = 3600 +2. Кастомный TTL передаётся корректно + +**Entity/ApplicationInstallation — markAsNeedReinstall:** +1. `new → needReinstall` — успех, событие создано +2. `active → needReinstall` — exception +3. `blocked → needReinstall` — exception +4. `needReinstall → deleted` через `applicationUninstalled(null)` — успех + +#### Functional Tests + +**MarkOldInstallations/HandlerTest:** +1. Создаём `new` установку с `createdAt = NOW() - 2h`, запускаем с TTL=3600 → статус `needReinstall` +2. Создаём `new` установку с `createdAt = NOW() - 30m`, запускаем с TTL=3600 → статус остаётся `new` +3. Запускаем Install повторно → старая `needReinstall` установка удаляется, новая создаётся + +### Assumptions + +- `needReinstall` добавляется только в `ApplicationInstallationStatus`, не в `Bitrix24AccountStatus` +- Мастер-аккаунт при markOld не меняет статус — остаётся в `new` +- `applicationUninstalled(null)` из `needReinstall → deleted` не требует блокировки +- Метод `findStaleInstallations` сначала живёт в локальном репозитории этой библиотеки, потом переносится в SDK-интерфейс отдельным PR +- Scheduling (cron/worker) — ответственность потребителя библиотеки +- Константа TTL по умолчанию = 3600 секунд, находится в Command diff --git a/src/ApplicationInstallations/Entity/ApplicationInstallation.php b/src/ApplicationInstallations/Entity/ApplicationInstallation.php index bda0eea2..6463c170 100644 --- a/src/ApplicationInstallations/Entity/ApplicationInstallation.php +++ b/src/ApplicationInstallations/Entity/ApplicationInstallation.php @@ -13,6 +13,7 @@ use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Events\ApplicationInstallationContactPersonLinkedEvent; use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Events\ApplicationInstallationCreatedEvent; use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Events\ApplicationInstallationFinishedEvent; +use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Events\ApplicationInstallationMarkedNeedReinstallEvent; use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Events\ApplicationInstallationUnblockedEvent; use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Events\ApplicationInstallationUninstalledEvent; use Bitrix24\SDK\Application\PortalLicenseFamily; @@ -166,10 +167,11 @@ public function applicationUninstalled(?string $applicationToken = null): void if ( ApplicationInstallationStatus::active !== $this->status && ApplicationInstallationStatus::blocked !== $this->status + && ApplicationInstallationStatus::needReinstall !== $this->status ) { throw new LogicException( sprintf( - 'installation was interrupted because status must be in active or blocked, but your status is %s', + 'installation was interrupted because status must be in active, blocked or needReinstall, but your status is %s', $this->status->value ) ); @@ -241,6 +243,26 @@ public function markAsBlocked(?string $comment): void ); } + public function markAsNeedReinstall(?string $comment): void + { + if (ApplicationInstallationStatus::new !== $this->status) { + throw new LogicException(sprintf( + 'you can mark application installation as need reinstall only in status new, but your status is «%s»', + $this->status->value + )); + } + + $this->status = ApplicationInstallationStatus::needReinstall; + $this->comment = $comment; + $this->updatedAt = new CarbonImmutable(); + + $this->events[] = new ApplicationInstallationMarkedNeedReinstallEvent( + $this->id, + new CarbonImmutable(), + $this->comment + ); + } + #[\Override] public function getApplicationStatus(): ApplicationStatus { diff --git a/src/ApplicationInstallations/Infrastructure/Doctrine/ApplicationInstallationRepository.php b/src/ApplicationInstallations/Infrastructure/Doctrine/ApplicationInstallationRepository.php index 28717da4..02082a3e 100644 --- a/src/ApplicationInstallations/Infrastructure/Doctrine/ApplicationInstallationRepository.php +++ b/src/ApplicationInstallations/Infrastructure/Doctrine/ApplicationInstallationRepository.php @@ -11,6 +11,7 @@ use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Exceptions\ApplicationInstallationNotFoundException; use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Repository\ApplicationInstallationRepositoryInterface; use Bitrix24\SDK\Core\Exceptions\InvalidArgumentException; +use Carbon\CarbonImmutable; use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\EntityRepository; use Doctrine\ORM\Query\Expr\Join; @@ -184,4 +185,44 @@ public function findByBitrix24AccountMemberId(string $memberId): ?ApplicationIns ->getOneOrNullResult() ; } + + public function findStaleInstallations( + ApplicationInstallationStatus $status, + CarbonImmutable $olderThan, + ?string $memberId = null + ): array { + $queryBuilder = $this->getEntityManager()->getRepository(ApplicationInstallation::class) + ->createQueryBuilder('ai') + ; + + $queryBuilder + ->leftJoin( + Bitrix24Account::class, + 'b24', + Join::WITH, + 'ai.bitrix24AccountId = b24.id AND b24.isMasterAccount = true' + ) + ->where('ai.status = :status') + ->andWhere('ai.createdAt < :olderThan') + ->setParameter('status', $status) + ->setParameter('olderThan', $olderThan) + ; + + if (null !== $memberId) { + if ('' === trim($memberId)) { + throw new InvalidArgumentException('memberId cannot be empty'); + } + + $queryBuilder + ->andWhere('b24.memberId = :memberId') + ->setParameter('memberId', $memberId) + ; + } + + return $queryBuilder + ->orderBy('ai.createdAt', 'ASC') + ->getQuery() + ->getResult() + ; + } } From e43cc25da5b43218773e918229c8a200d0863b9d Mon Sep 17 00:00:00 2001 From: Kirill Hramov Date: Thu, 6 Aug 2026 19:03:03 +0300 Subject: [PATCH 2/7] Introduce `MarkOldInstallations` use case for handling stale application installations - Added console command `bitrix24:installations:mark-old` to process stale `new` installations, marking them with `needReinstall` status. - Implemented `Workflow`, `Handler`, and configuration classes to support TTL-based filtering and dry-run mode. - Enhanced repository with methods for fetching stale installations. - Integrated logging for tracking progress and results. --- .../Console/MarkOldInstallationsCommand.php | 137 ++++++++++++++++++ .../UseCase/MarkOldInstallations/Command.php | 23 +++ .../UseCase/MarkOldInstallations/Handler.php | 76 ++++++++++ .../MarkOldInstallationsConfig.php | 24 +++ .../MarkOldInstallationsResult.php | 15 ++ .../UseCase/MarkOldInstallations/Workflow.php | 54 +++++++ 6 files changed, 329 insertions(+) create mode 100644 src/ApplicationInstallations/Console/MarkOldInstallationsCommand.php create mode 100644 src/ApplicationInstallations/UseCase/MarkOldInstallations/Command.php create mode 100644 src/ApplicationInstallations/UseCase/MarkOldInstallations/Handler.php create mode 100644 src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsConfig.php create mode 100644 src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsResult.php create mode 100644 src/ApplicationInstallations/UseCase/MarkOldInstallations/Workflow.php diff --git a/src/ApplicationInstallations/Console/MarkOldInstallationsCommand.php b/src/ApplicationInstallations/Console/MarkOldInstallationsCommand.php new file mode 100644 index 00000000..b713f7b6 --- /dev/null +++ b/src/ApplicationInstallations/Console/MarkOldInstallationsCommand.php @@ -0,0 +1,137 @@ +addArgument( + 'ttl', + InputArgument::OPTIONAL, + sprintf('Time to live in seconds (default: %d)', self::DEFAULT_TTL), + (string) self::DEFAULT_TTL + ) + ->addOption( + 'member-id', + 'm', + InputOption::VALUE_OPTIONAL, + 'Filter by specific portal member ID' + ) + ->addOption( + 'dry-run', + null, + InputOption::VALUE_NONE, + 'Show what would be marked without making changes' + ) + ->setHelp( + <<<'HELP' +The bitrix24:installations:mark-old command finds pending installations +in status "new" older than the given TTL and marks them as "needReinstall". + +Mark installations older than 1 hour (default): + php bin/console bitrix24:installations:mark-old + +Mark installations older than 30 minutes: + php bin/console bitrix24:installations:mark-old 1800 + +Mark installations for a specific portal: + php bin/console bitrix24:installations:mark-old 3600 --member-id=xxxxxxxxxxxxxxxxx + +Dry run — show what would be marked: + php bin/console bitrix24:installations:mark-old --dry-run +HELP + ) + ; + } + + #[\Override] + protected function execute(InputInterface $input, OutputInterface $output): int + { + $this->io = new SymfonyStyle($input, $output); + + $config = $this->parseInput($input); + if (null === $config) { + return Command::FAILURE; + } + + $result = $this->workflow->run($config); + + return $this->renderResult($result); + } + + private function parseInput(InputInterface $input): ?MarkOldInstallationsConfig + { + $ttl = (int) $input->getArgument('ttl'); + $memberId = $input->getOption('member-id'); + $dryRun = (bool) $input->getOption('dry-run'); + + try { + return new MarkOldInstallationsConfig($ttl, $memberId, $dryRun); + } catch (InvalidArgumentException $invalidArgumentException) { + $this->io->error($invalidArgumentException->getMessage()); + } + + return null; + } + + private function renderResult(MarkOldInstallationsResult $result): int + { + if ($result->dryRun) { + $this->io->note(sprintf('Dry-run mode: %d stale installation(s) found.', count($result->staleInstallations))); + + foreach ($result->staleInstallations as $staleInstallation) { + $this->io->text(sprintf( + ' - installation %s, created at %s', + $staleInstallation->getId()->toRfc4122(), + $staleInstallation->getCreatedAt()->toAtomString() + )); + } + + $this->io->newLine(); + $this->io->note('No changes were made. Remove --dry-run to mark installations.'); + + return 0; + } + + if (0 === $result->processedCount) { + $this->io->success('No stale installations found.'); + + return 0; + } + + $this->io->success(sprintf('Processed %d stale installation(s)', $result->processedCount)); + + return $result->processedCount; + } +} diff --git a/src/ApplicationInstallations/UseCase/MarkOldInstallations/Command.php b/src/ApplicationInstallations/UseCase/MarkOldInstallations/Command.php new file mode 100644 index 00000000..ce3116ad --- /dev/null +++ b/src/ApplicationInstallations/UseCase/MarkOldInstallations/Command.php @@ -0,0 +1,23 @@ +ttlInSeconds < 0) { + throw new InvalidArgumentException('TTL in seconds must be a non-negative integer.'); + } + + if (null !== $this->memberId && '' === trim($this->memberId)) { + throw new InvalidArgumentException('Member ID must be a non-empty string.'); + } + } +} diff --git a/src/ApplicationInstallations/UseCase/MarkOldInstallations/Handler.php b/src/ApplicationInstallations/UseCase/MarkOldInstallations/Handler.php new file mode 100644 index 00000000..eb9c9bc5 --- /dev/null +++ b/src/ApplicationInstallations/UseCase/MarkOldInstallations/Handler.php @@ -0,0 +1,76 @@ +subSeconds($command->ttlInSeconds); + + return $this->applicationInstallationRepository->findStaleInstallations( + ApplicationInstallationStatus::new, + $olderThan, + $command->memberId + ); + } + + /** + * @throws LogicException + */ + public function handle(Command $command): int + { + $this->logger->info('ApplicationInstallations.MarkOldInstallations.start', [ + 'ttlInSeconds' => $command->ttlInSeconds, + 'memberId' => $command->memberId, + ]); + + $staleInstallations = $this->findStaleInstallations($command); + + $processedCount = 0; + + foreach ($staleInstallations as $staleInstallation) { + $staleInstallation->markAsNeedReinstall( + sprintf('installation timed out without ONAPPINSTALL, TTL = %d seconds', $command->ttlInSeconds) + ); + + $this->applicationInstallationRepository->save($staleInstallation); + $this->flusher->flush($staleInstallation); + + ++$processedCount; + + $this->logger->info('ApplicationInstallations.MarkOldInstallations.marked', [ + 'installationId' => $staleInstallation->getId()->toRfc4122(), + 'createdAt' => $staleInstallation->getCreatedAt()->toAtomString(), + ]); + } + + $this->logger->info('ApplicationInstallations.MarkOldInstallations.finish', [ + 'processedCount' => $processedCount, + ]); + + return $processedCount; + } +} diff --git a/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsConfig.php b/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsConfig.php new file mode 100644 index 00000000..579aa274 --- /dev/null +++ b/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsConfig.php @@ -0,0 +1,24 @@ +ttlInSeconds < 0) { + throw new InvalidArgumentException('TTL in seconds must be a non-negative integer.'); + } + + if (null !== $this->memberId && '' === trim($this->memberId)) { + throw new InvalidArgumentException('Member ID must be a non-empty string.'); + } + } +} diff --git a/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsResult.php b/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsResult.php new file mode 100644 index 00000000..8837c8d9 --- /dev/null +++ b/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsResult.php @@ -0,0 +1,15 @@ +logger->info('ApplicationInstallations.MarkOldInstallations.Workflow.start', [ + 'ttlInSeconds' => $config->ttlInSeconds, + 'memberId' => $config->memberId, + 'dryRun' => $config->dryRun, + ]); + + if ($config->dryRun) { + $olderThan = new CarbonImmutable(); + $olderThan = $olderThan->subSeconds($config->ttlInSeconds); + + $staleInstallations = $this->applicationInstallationRepository->findStaleInstallations( + ApplicationInstallationStatus::new, + $olderThan, + $config->memberId + ); + + $this->logger->info('ApplicationInstallations.MarkOldInstallations.Workflow.dryRun', [ + 'foundCount' => count($staleInstallations), + ]); + + return new MarkOldInstallationsResult(0, true, $staleInstallations); + } + + $command = new Command($config->ttlInSeconds, $config->memberId); + $processedCount = $this->handler->handle($command); + + $this->logger->info('ApplicationInstallations.MarkOldInstallations.Workflow.finish', [ + 'processedCount' => $processedCount, + ]); + + return new MarkOldInstallationsResult($processedCount, false); + } +} From 4d9e2cb264452b184188917dcdb57b04e3acb539 Mon Sep 17 00:00:00 2001 From: Kirill Hramov Date: Fri, 7 Aug 2026 18:04:16 +0300 Subject: [PATCH 3/7] Remove `processedCount` from `MarkOldInstallations` workflow and handler - Simplified `Handler` and `Workflow` by eliminating the unused `processedCount` tracking. - Updated result structures, logging, and console output accordingly to reflect the removal. - Streamlined data flow and improved maintainability across the use case. --- .../Console/MarkOldInstallationsCommand.php | 10 ++-------- .../UseCase/MarkOldInstallations/Handler.php | 12 ++---------- .../MarkOldInstallationsResult.php | 4 +--- .../UseCase/MarkOldInstallations/Workflow.php | 10 ++++------ 4 files changed, 9 insertions(+), 27 deletions(-) diff --git a/src/ApplicationInstallations/Console/MarkOldInstallationsCommand.php b/src/ApplicationInstallations/Console/MarkOldInstallationsCommand.php index b713f7b6..6c989bff 100644 --- a/src/ApplicationInstallations/Console/MarkOldInstallationsCommand.php +++ b/src/ApplicationInstallations/Console/MarkOldInstallationsCommand.php @@ -124,14 +124,8 @@ private function renderResult(MarkOldInstallationsResult $result): int return 0; } - if (0 === $result->processedCount) { - $this->io->success('No stale installations found.'); + $this->io->success('Stale installations marked as needReinstall. See event subscriber for details.'); - return 0; - } - - $this->io->success(sprintf('Processed %d stale installation(s)', $result->processedCount)); - - return $result->processedCount; + return 0; } } diff --git a/src/ApplicationInstallations/UseCase/MarkOldInstallations/Handler.php b/src/ApplicationInstallations/UseCase/MarkOldInstallations/Handler.php index eb9c9bc5..c88ce5fb 100644 --- a/src/ApplicationInstallations/UseCase/MarkOldInstallations/Handler.php +++ b/src/ApplicationInstallations/UseCase/MarkOldInstallations/Handler.php @@ -40,7 +40,7 @@ private function findStaleInstallations(Command $command): array /** * @throws LogicException */ - public function handle(Command $command): int + public function handle(Command $command): void { $this->logger->info('ApplicationInstallations.MarkOldInstallations.start', [ 'ttlInSeconds' => $command->ttlInSeconds, @@ -49,8 +49,6 @@ public function handle(Command $command): int $staleInstallations = $this->findStaleInstallations($command); - $processedCount = 0; - foreach ($staleInstallations as $staleInstallation) { $staleInstallation->markAsNeedReinstall( sprintf('installation timed out without ONAPPINSTALL, TTL = %d seconds', $command->ttlInSeconds) @@ -59,18 +57,12 @@ public function handle(Command $command): int $this->applicationInstallationRepository->save($staleInstallation); $this->flusher->flush($staleInstallation); - ++$processedCount; - $this->logger->info('ApplicationInstallations.MarkOldInstallations.marked', [ 'installationId' => $staleInstallation->getId()->toRfc4122(), 'createdAt' => $staleInstallation->getCreatedAt()->toAtomString(), ]); } - $this->logger->info('ApplicationInstallations.MarkOldInstallations.finish', [ - 'processedCount' => $processedCount, - ]); - - return $processedCount; + $this->logger->info('ApplicationInstallations.MarkOldInstallations.finish'); } } diff --git a/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsResult.php b/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsResult.php index 8837c8d9..1d0db623 100644 --- a/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsResult.php +++ b/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsResult.php @@ -7,9 +7,7 @@ readonly class MarkOldInstallationsResult { public function __construct( - public int $processedCount, public bool $dryRun, public array $staleInstallations = [] - ) - {} + ) {} } diff --git a/src/ApplicationInstallations/UseCase/MarkOldInstallations/Workflow.php b/src/ApplicationInstallations/UseCase/MarkOldInstallations/Workflow.php index 528c929b..2ff3f1a2 100644 --- a/src/ApplicationInstallations/UseCase/MarkOldInstallations/Workflow.php +++ b/src/ApplicationInstallations/UseCase/MarkOldInstallations/Workflow.php @@ -39,16 +39,14 @@ public function run(MarkOldInstallationsConfig $config): MarkOldInstallationsRes 'foundCount' => count($staleInstallations), ]); - return new MarkOldInstallationsResult(0, true, $staleInstallations); + return new MarkOldInstallationsResult(true, $staleInstallations); } $command = new Command($config->ttlInSeconds, $config->memberId); - $processedCount = $this->handler->handle($command); + $this->handler->handle($command); - $this->logger->info('ApplicationInstallations.MarkOldInstallations.Workflow.finish', [ - 'processedCount' => $processedCount, - ]); + $this->logger->info('ApplicationInstallations.MarkOldInstallations.Workflow.finish'); - return new MarkOldInstallationsResult($processedCount, false); + return new MarkOldInstallationsResult(false); } } From f192c40aa210540a9d3581843c61e55cc62edb55 Mon Sep 17 00:00:00 2001 From: Kirill Hramov Date: Fri, 7 Aug 2026 18:12:16 +0300 Subject: [PATCH 4/7] Track processed installations in `MarkOldInstallations` workflow - Introduced `processedInstallations` tracking to capture and log details of installations marked as `needReinstall`. - Added `MarkOldInstallationsCollector` for event collection during the workflow. - Enhanced console output with detailed information about processed installations. --- .../Console/MarkOldInstallationsCommand.php | 17 +++++++++- .../MarkOldInstallationsCollector.php | 26 ++++++++++++++++ .../MarkOldInstallationsResult.php | 3 +- .../UseCase/MarkOldInstallations/Workflow.php | 31 ++++++++++++++++--- 4 files changed, 71 insertions(+), 6 deletions(-) create mode 100644 src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsCollector.php diff --git a/src/ApplicationInstallations/Console/MarkOldInstallationsCommand.php b/src/ApplicationInstallations/Console/MarkOldInstallationsCommand.php index 6c989bff..5b9dc80c 100644 --- a/src/ApplicationInstallations/Console/MarkOldInstallationsCommand.php +++ b/src/ApplicationInstallations/Console/MarkOldInstallationsCommand.php @@ -124,7 +124,22 @@ private function renderResult(MarkOldInstallationsResult $result): int return 0; } - $this->io->success('Stale installations marked as needReinstall. See event subscriber for details.'); + $count = count($result->processedInstallations); + if (0 === $count) { + $this->io->success('No stale installations found.'); + + return 0; + } + + $this->io->success(sprintf('Marked %d installation(s) as needReinstall:', $count)); + + foreach ($result->processedInstallations as $event) { + $this->io->text(sprintf( + ' - installation %s, marked at %s', + $event->applicationInstallationId->toRfc4122(), + $event->timestamp->toAtomString() + )); + } return 0; } diff --git a/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsCollector.php b/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsCollector.php new file mode 100644 index 00000000..7a8d8505 --- /dev/null +++ b/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsCollector.php @@ -0,0 +1,26 @@ +events[] = $event; + } + + /** + * @return ApplicationInstallationMarkedNeedReinstallEvent[] + */ + public function getEvents(): array + { + return $this->events; + } +} diff --git a/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsResult.php b/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsResult.php index 1d0db623..79b22354 100644 --- a/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsResult.php +++ b/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsResult.php @@ -8,6 +8,7 @@ { public function __construct( public bool $dryRun, - public array $staleInstallations = [] + public array $staleInstallations = [], + public array $processedInstallations = [] ) {} } diff --git a/src/ApplicationInstallations/UseCase/MarkOldInstallations/Workflow.php b/src/ApplicationInstallations/UseCase/MarkOldInstallations/Workflow.php index 2ff3f1a2..e98a95a2 100644 --- a/src/ApplicationInstallations/UseCase/MarkOldInstallations/Workflow.php +++ b/src/ApplicationInstallations/UseCase/MarkOldInstallations/Workflow.php @@ -5,15 +5,18 @@ namespace Bitrix24\Lib\ApplicationInstallations\UseCase\MarkOldInstallations; use Bitrix24\Lib\ApplicationInstallations\Infrastructure\Doctrine\ApplicationInstallationRepository; +use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Events\ApplicationInstallationMarkedNeedReinstallEvent; use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Entity\ApplicationInstallationStatus; use Carbon\CarbonImmutable; use Psr\Log\LoggerInterface; +use Symfony\Component\EventDispatcher\EventDispatcherInterface; readonly class Workflow { public function __construct( private Handler $handler, private ApplicationInstallationRepository $applicationInstallationRepository, + private EventDispatcherInterface $eventDispatcher, private LoggerInterface $logger ) {} @@ -39,14 +42,34 @@ public function run(MarkOldInstallationsConfig $config): MarkOldInstallationsRes 'foundCount' => count($staleInstallations), ]); - return new MarkOldInstallationsResult(true, $staleInstallations); + return new MarkOldInstallationsResult(true, staleInstallations: $staleInstallations); } $command = new Command($config->ttlInSeconds, $config->memberId); - $this->handler->handle($command); - $this->logger->info('ApplicationInstallations.MarkOldInstallations.Workflow.finish'); + $collector = new MarkOldInstallationsCollector(); + $listener = $collector->add(...); - return new MarkOldInstallationsResult(false); + $this->eventDispatcher->addListener( + ApplicationInstallationMarkedNeedReinstallEvent::class, + $listener + ); + + try { + $this->handler->handle($command); + } finally { + $this->eventDispatcher->removeListener( + ApplicationInstallationMarkedNeedReinstallEvent::class, + $listener + ); + } + + $processedInstallations = $collector->getEvents(); + + $this->logger->info('ApplicationInstallations.MarkOldInstallations.Workflow.finish', [ + 'processedCount' => count($processedInstallations), + ]); + + return new MarkOldInstallationsResult(false, processedInstallations: $processedInstallations); } } From 43a36bbc359a9861a8edf62ec326773882e331c8 Mon Sep 17 00:00:00 2001 From: Kirill Hramov Date: Tue, 11 Aug 2026 14:03:28 +0300 Subject: [PATCH 5/7] Remove `memberId` filtering and `dry-run` mode from `MarkOldInstallations` workflow - Simplified the `MarkOldInstallations` workflow by removing `memberId` support and dry-run functionality. - Updated console command, configuration, and result classes to reflect the changes. - Streamlined repository queries and reduced unnecessary logic for improved maintainability. --- .../Console/MarkOldInstallationsCommand.php | 40 +------------------ .../ApplicationInstallationRepository.php | 20 +--------- .../UseCase/MarkOldInstallations/Command.php | 5 --- .../UseCase/MarkOldInstallations/Handler.php | 4 +- .../MarkOldInstallationsConfig.php | 6 --- .../MarkOldInstallationsResult.php | 7 +++- .../UseCase/MarkOldInstallations/Workflow.php | 27 +------------ 7 files changed, 10 insertions(+), 99 deletions(-) diff --git a/src/ApplicationInstallations/Console/MarkOldInstallationsCommand.php b/src/ApplicationInstallations/Console/MarkOldInstallationsCommand.php index 5b9dc80c..8a4c79b5 100644 --- a/src/ApplicationInstallations/Console/MarkOldInstallationsCommand.php +++ b/src/ApplicationInstallations/Console/MarkOldInstallationsCommand.php @@ -12,7 +12,6 @@ use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; -use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Style\SymfonyStyle; @@ -42,18 +41,6 @@ protected function configure(): void sprintf('Time to live in seconds (default: %d)', self::DEFAULT_TTL), (string) self::DEFAULT_TTL ) - ->addOption( - 'member-id', - 'm', - InputOption::VALUE_OPTIONAL, - 'Filter by specific portal member ID' - ) - ->addOption( - 'dry-run', - null, - InputOption::VALUE_NONE, - 'Show what would be marked without making changes' - ) ->setHelp( <<<'HELP' The bitrix24:installations:mark-old command finds pending installations @@ -64,12 +51,6 @@ protected function configure(): void Mark installations older than 30 minutes: php bin/console bitrix24:installations:mark-old 1800 - -Mark installations for a specific portal: - php bin/console bitrix24:installations:mark-old 3600 --member-id=xxxxxxxxxxxxxxxxx - -Dry run — show what would be marked: - php bin/console bitrix24:installations:mark-old --dry-run HELP ) ; @@ -93,11 +74,9 @@ protected function execute(InputInterface $input, OutputInterface $output): int private function parseInput(InputInterface $input): ?MarkOldInstallationsConfig { $ttl = (int) $input->getArgument('ttl'); - $memberId = $input->getOption('member-id'); - $dryRun = (bool) $input->getOption('dry-run'); try { - return new MarkOldInstallationsConfig($ttl, $memberId, $dryRun); + return new MarkOldInstallationsConfig($ttl); } catch (InvalidArgumentException $invalidArgumentException) { $this->io->error($invalidArgumentException->getMessage()); } @@ -107,23 +86,6 @@ private function parseInput(InputInterface $input): ?MarkOldInstallationsConfig private function renderResult(MarkOldInstallationsResult $result): int { - if ($result->dryRun) { - $this->io->note(sprintf('Dry-run mode: %d stale installation(s) found.', count($result->staleInstallations))); - - foreach ($result->staleInstallations as $staleInstallation) { - $this->io->text(sprintf( - ' - installation %s, created at %s', - $staleInstallation->getId()->toRfc4122(), - $staleInstallation->getCreatedAt()->toAtomString() - )); - } - - $this->io->newLine(); - $this->io->note('No changes were made. Remove --dry-run to mark installations.'); - - return 0; - } - $count = count($result->processedInstallations); if (0 === $count) { $this->io->success('No stale installations found.'); diff --git a/src/ApplicationInstallations/Infrastructure/Doctrine/ApplicationInstallationRepository.php b/src/ApplicationInstallations/Infrastructure/Doctrine/ApplicationInstallationRepository.php index 02082a3e..066ea233 100644 --- a/src/ApplicationInstallations/Infrastructure/Doctrine/ApplicationInstallationRepository.php +++ b/src/ApplicationInstallations/Infrastructure/Doctrine/ApplicationInstallationRepository.php @@ -188,37 +188,19 @@ public function findByBitrix24AccountMemberId(string $memberId): ?ApplicationIns public function findStaleInstallations( ApplicationInstallationStatus $status, - CarbonImmutable $olderThan, - ?string $memberId = null + CarbonImmutable $olderThan ): array { $queryBuilder = $this->getEntityManager()->getRepository(ApplicationInstallation::class) ->createQueryBuilder('ai') ; $queryBuilder - ->leftJoin( - Bitrix24Account::class, - 'b24', - Join::WITH, - 'ai.bitrix24AccountId = b24.id AND b24.isMasterAccount = true' - ) ->where('ai.status = :status') ->andWhere('ai.createdAt < :olderThan') ->setParameter('status', $status) ->setParameter('olderThan', $olderThan) ; - if (null !== $memberId) { - if ('' === trim($memberId)) { - throw new InvalidArgumentException('memberId cannot be empty'); - } - - $queryBuilder - ->andWhere('b24.memberId = :memberId') - ->setParameter('memberId', $memberId) - ; - } - return $queryBuilder ->orderBy('ai.createdAt', 'ASC') ->getQuery() diff --git a/src/ApplicationInstallations/UseCase/MarkOldInstallations/Command.php b/src/ApplicationInstallations/UseCase/MarkOldInstallations/Command.php index ce3116ad..0eacf9f9 100644 --- a/src/ApplicationInstallations/UseCase/MarkOldInstallations/Command.php +++ b/src/ApplicationInstallations/UseCase/MarkOldInstallations/Command.php @@ -10,14 +10,9 @@ { public function __construct( public int $ttlInSeconds, - public ?string $memberId = null, ) { if ($this->ttlInSeconds < 0) { throw new InvalidArgumentException('TTL in seconds must be a non-negative integer.'); } - - if (null !== $this->memberId && '' === trim($this->memberId)) { - throw new InvalidArgumentException('Member ID must be a non-empty string.'); - } } } diff --git a/src/ApplicationInstallations/UseCase/MarkOldInstallations/Handler.php b/src/ApplicationInstallations/UseCase/MarkOldInstallations/Handler.php index c88ce5fb..4f816659 100644 --- a/src/ApplicationInstallations/UseCase/MarkOldInstallations/Handler.php +++ b/src/ApplicationInstallations/UseCase/MarkOldInstallations/Handler.php @@ -32,8 +32,7 @@ private function findStaleInstallations(Command $command): array return $this->applicationInstallationRepository->findStaleInstallations( ApplicationInstallationStatus::new, - $olderThan, - $command->memberId + $olderThan ); } @@ -44,7 +43,6 @@ public function handle(Command $command): void { $this->logger->info('ApplicationInstallations.MarkOldInstallations.start', [ 'ttlInSeconds' => $command->ttlInSeconds, - 'memberId' => $command->memberId, ]); $staleInstallations = $this->findStaleInstallations($command); diff --git a/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsConfig.php b/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsConfig.php index 579aa274..c8601e5e 100644 --- a/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsConfig.php +++ b/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsConfig.php @@ -10,15 +10,9 @@ { public function __construct( public int $ttlInSeconds, - public ?string $memberId = null, - public bool $dryRun = false, ) { if ($this->ttlInSeconds < 0) { throw new InvalidArgumentException('TTL in seconds must be a non-negative integer.'); } - - if (null !== $this->memberId && '' === trim($this->memberId)) { - throw new InvalidArgumentException('Member ID must be a non-empty string.'); - } } } diff --git a/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsResult.php b/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsResult.php index 79b22354..51d91b55 100644 --- a/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsResult.php +++ b/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsResult.php @@ -4,11 +4,14 @@ namespace Bitrix24\Lib\ApplicationInstallations\UseCase\MarkOldInstallations; +use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Events\ApplicationInstallationMarkedNeedReinstallEvent; + readonly class MarkOldInstallationsResult { + /** + * @param ApplicationInstallationMarkedNeedReinstallEvent[] $processedInstallations + */ public function __construct( - public bool $dryRun, - public array $staleInstallations = [], public array $processedInstallations = [] ) {} } diff --git a/src/ApplicationInstallations/UseCase/MarkOldInstallations/Workflow.php b/src/ApplicationInstallations/UseCase/MarkOldInstallations/Workflow.php index e98a95a2..69b63f51 100644 --- a/src/ApplicationInstallations/UseCase/MarkOldInstallations/Workflow.php +++ b/src/ApplicationInstallations/UseCase/MarkOldInstallations/Workflow.php @@ -4,10 +4,7 @@ namespace Bitrix24\Lib\ApplicationInstallations\UseCase\MarkOldInstallations; -use Bitrix24\Lib\ApplicationInstallations\Infrastructure\Doctrine\ApplicationInstallationRepository; use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Events\ApplicationInstallationMarkedNeedReinstallEvent; -use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Entity\ApplicationInstallationStatus; -use Carbon\CarbonImmutable; use Psr\Log\LoggerInterface; use Symfony\Component\EventDispatcher\EventDispatcherInterface; @@ -15,7 +12,6 @@ { public function __construct( private Handler $handler, - private ApplicationInstallationRepository $applicationInstallationRepository, private EventDispatcherInterface $eventDispatcher, private LoggerInterface $logger ) {} @@ -24,28 +20,9 @@ public function run(MarkOldInstallationsConfig $config): MarkOldInstallationsRes { $this->logger->info('ApplicationInstallations.MarkOldInstallations.Workflow.start', [ 'ttlInSeconds' => $config->ttlInSeconds, - 'memberId' => $config->memberId, - 'dryRun' => $config->dryRun, ]); - if ($config->dryRun) { - $olderThan = new CarbonImmutable(); - $olderThan = $olderThan->subSeconds($config->ttlInSeconds); - - $staleInstallations = $this->applicationInstallationRepository->findStaleInstallations( - ApplicationInstallationStatus::new, - $olderThan, - $config->memberId - ); - - $this->logger->info('ApplicationInstallations.MarkOldInstallations.Workflow.dryRun', [ - 'foundCount' => count($staleInstallations), - ]); - - return new MarkOldInstallationsResult(true, staleInstallations: $staleInstallations); - } - - $command = new Command($config->ttlInSeconds, $config->memberId); + $command = new Command($config->ttlInSeconds); $collector = new MarkOldInstallationsCollector(); $listener = $collector->add(...); @@ -70,6 +47,6 @@ public function run(MarkOldInstallationsConfig $config): MarkOldInstallationsRes 'processedCount' => count($processedInstallations), ]); - return new MarkOldInstallationsResult(false, processedInstallations: $processedInstallations); + return new MarkOldInstallationsResult($processedInstallations); } } From 1e1cb234af75283c3cb6abbff191b29906116b14 Mon Sep 17 00:00:00 2001 From: Kirill Hramov Date: Tue, 11 Aug 2026 17:32:43 +0300 Subject: [PATCH 6/7] Add unit and functional tests for `MarkOldInstallations` use case - Introduced `ConfigTest` to validate TTL configurations and edge cases. - Added `HandlerTest` to ensure correct identification and status updates for stale installations. - Implemented `WorkflowTest` for end-to-end flow validation of the `MarkOldInstallations` use case. - Updated `Install/HandlerTest` to test reinstall behavior after `needReinstall` transition. --- .tasks/92/mark-old-installations-plan.md | 136 ++++++++-------- .../UseCase/Install/HandlerTest.php | 30 ++++ .../MarkOldInstallations/HandlerTest.php | 150 ++++++++++++++++++ .../MarkOldInstallations/WorkflowTest.php | 135 ++++++++++++++++ .../MarkOldInstallations/ConfigTest.php | 43 +++++ 5 files changed, 422 insertions(+), 72 deletions(-) create mode 100644 tests/Functional/ApplicationInstallations/UseCase/MarkOldInstallations/HandlerTest.php create mode 100644 tests/Functional/ApplicationInstallations/UseCase/MarkOldInstallations/WorkflowTest.php create mode 100644 tests/Unit/ApplicationInstallations/UseCase/MarkOldInstallations/ConfigTest.php diff --git a/.tasks/92/mark-old-installations-plan.md b/.tasks/92/mark-old-installations-plan.md index 497cca3a..c8c524a5 100644 --- a/.tasks/92/mark-old-installations-plan.md +++ b/.tasks/92/mark-old-installations-plan.md @@ -9,16 +9,18 @@ Issue #92 — фоновая очистка зависших установок **В scope:** - Новый статус `needReinstall` в SDK (`ApplicationInstallationStatus`) -- Метод `markAsNeedReinstall()` в SDK-интерфейсе и entity +- Метод `markAsNeedReinstall()` в SDK entity и интерфейсе - Обновление guard в `applicationUninstalled()` для `needReinstall` - Новое доменное событие `ApplicationInstallationMarkedNeedReinstallEvent` -- UseCase `MarkOldInstallations` (поиск + перевод в `needReinstall`) -- Repository метод для поиска зависших установок (в локальном репо, потом в SDK) +- UseCase `MarkOldInstallations` (Workflow → Handler, поиск + перевод в `needReinstall`) +- Repository метод `findStaleInstallations()` в существующем `ApplicationInstallationRepository` - Console-команда `bitrix24:installations:mark-old` - Unit + Functional тесты - Документация и CHANGELOG **Out of scope:** +- Фильтр по `memberId` — ищем все зависшие установки независимо от портала +- `dry-run` режим — убрали, не нужен - Проверка реального состояния портала через SDK (`--verify-portal`) - Автоматическое планирование (cron, worker) — задача потребителя - Изменение `Bitrix24AccountStatus` — статус `needReinstall` только у `ApplicationInstallation` @@ -41,13 +43,17 @@ needReinstall → deleted (applicationUninstalled) ← НОВОЕ 2. **markOldInstallations flow:** ``` Console command (ttl=3600 по дефолту) - → MarkOldInstallations\Command(ttl) - → Handler::handle() - → repository.findStaleInstallations(status=new, olderThan=NOW()-ttl) - → для каждого: installation.markAsNeedReinstall(comment) - → repository.save(installation) - → flusher.flush(installation) - → событие ApplicationInstallationMarkedNeedReinstallEvent диспатчится + → Workflow.run(Config) + → регистрация listener на ApplicationInstallationMarkedNeedReinstallEvent + → Handler::handle(Command) + → repository.findStaleInstallations(status=new, olderThan=NOW()-ttl) + → для каждого: installation.markAsNeedReinstall(comment) + → repository.save(installation) + → flusher.flush(installation) + → событие ApplicationInstallationMarkedNeedReinstallEvent диспатчится через EventDispatcher + → unregister listener + → collector.getEvents() → Result(processedInstallations) + → Console рендерит список переведённых установок ``` 3. **Переустановка после markOldInstallations:** @@ -56,8 +62,9 @@ Install\Command (повторная установка) → Install\Handler::handle() → findByBitrix24AccountMemberId находит installation в needReinstall → deactivateCurrentInstallation() + → markAsBlocked пропускается (только для status=new) → applicationUninstalled(null) // needReinstall → deleted (guard обновлён) - → bitrix24Account.applicationUninstalled(null) // new → deleted (уже работает) + → удаляются ВСЕ аккаунты портала (не только master), см. Install/Handler.php:125-138 → создание новой пары ``` @@ -71,7 +78,9 @@ Install\Command (повторная установка) |---|---| | `src/Application/Contracts/ApplicationInstallations/Entity/ApplicationInstallationStatus.php` | Добавить `case needReinstall = 'needReinstall'` | | `src/Application/Contracts/ApplicationInstallations/Entity/ApplicationInstallationInterface.php` | Добавить метод `markAsNeedReinstall(?string $comment): void` + обновить docblock `applicationUninstalled` | -| `src/Application/Contracts/ApplicationInstallations/Events/ApplicationInstallationMarkedNeedReinstallEvent.php` | Новое событие (id, updatedAt, comment) | +| `src/Application/Contracts/ApplicationInstallations/Events/ApplicationInstallationMarkedNeedReinstallEvent.php` | Новое событие (`applicationInstallationId: Uuid`, `timestamp: CarbonImmutable`, `comment: ?string`) | + +> Временно изменения применены прямо в `vendor/bitrix24/b24phpsdk/`, будут формализованы в SDK PR. #### PR 2: bitrix24-php-lib (этот репозиторий) @@ -80,99 +89,82 @@ Install\Command (повторная установка) | Изменение | Детали | |---|---| | `markAsNeedReinstall(?string $comment)` | Переход `new → needReinstall`, emit `ApplicationInstallationMarkedNeedReinstallEvent` | -| Guard в `applicationUninstalled()` | Добавить `needReinstall` в допустимые статусы | - -**Событие — `src/ApplicationInstallations/Entity/ApplicationInstallationMarkedNeedReinstallEvent.php`:** +| Guard в `applicationUninstalled()` | Добавить `needReinstall` в допустимые статусы (прямой переход `needReinstall → deleted`, без `blocked`) | -```php -readonly class ApplicationInstallationMarkedNeedReinstallEvent { - public function __construct( - public Uuid $id, - public CarbonImmutable $updatedAt, - public ?string $comment - ) {} -} -``` +**Repository — `src/ApplicationInstallations/Infrastructure/Doctrine/ApplicationInstallationRepository.php`:** -**Repository — `src/ApplicationInstallations/Infrastructure/Repository/StaleInstallationFinderInterface.php`:** - -```php -interface StaleInstallationFinderInterface { - public function findStaleInstallations( - ApplicationInstallationStatus $status, - CarbonImmutable $olderThan, - ?string $memberId = null - ): array; -} -``` - -**Repository — `src/ApplicationInstallations/Infrastructure/Doctrine/StaleInstallationFinder.php`:** - -- JOIN к `Bitrix24Account` для memberId +Метод `findStaleInstallations(ApplicationInstallationStatus $status, CarbonImmutable $olderThan): array`: +- Без `memberId` фильтра +- Без JOIN к `Bitrix24Account` - WHERE `status = :status AND createdAt < :olderThan` -- Опциональный фильтр `AND b24.memberId = :memberId` - -**Repository — `tests/Helpers/ApplicationInstallations/InMemoryStaleInstallationFinder.php`:** - -- In-memory реализация `StaleInstallationFinderInterface` для тестов +- ORDER BY `createdAt ASC` **UseCase — `src/ApplicationInstallations/UseCase/MarkOldInstallations/`:** | Файл | Содержание | |---|---| -| `Command.php` | `public function __construct(public int $ttlInSeconds = self::DEFAULT_TTL)` + `DEFAULT_TTL = 3600` | -| `Handler.php` | Находит зависшие через `StaleInstallationFinderInterface`, вызывает `markAsNeedReinstall()`, сохраняет, flush | +| `Command.php` | `public int $ttlInSeconds` + валидация (>= 0) | +| `Handler.php` | `handle(Command): void` — находит зависшие через `findStaleInstallations`, вызывает `markAsNeedReinstall()`, сохраняет, flush | +| `MarkOldInstallationsConfig.php` | `public int $ttlInSeconds` + валидация | +| `MarkOldInstallationsResult.php` | `public array $processedInstallations` (массив `ApplicationInstallationMarkedNeedReinstallEvent`) | +| `MarkOldInstallationsCollector.php` | Коллектор событий — `add(event)`, `getEvents(): array` | +| `Workflow.php` | Оркестратор: регистрирует listener на EventDispatcher → `handler->handle()` → unregister → `Result(processedInstallations)` | -**Console — `src/Console/MarkOldInstallationsCommand.php`:** +**Console — `src/ApplicationInstallations/Console/MarkOldInstallationsCommand.php`:** ```php -protected static $defaultName = 'bitrix24:installations:mark-old'; -// --ttl=3600 (опционально, дефолт из Command::DEFAULT_TTL) +#[AsCommand(name: 'bitrix24:installations:mark-old')] +class MarkOldInstallationsCommand extends Command +{ + public const DEFAULT_TTL = 3600; + // аргумент: ttl (опциональный, дефолт DEFAULT_TTL) + // рендерит список переведённых установок + count +} ``` **Документация — `src/ApplicationInstallations/Docs/application-installations.md`:** - - Добавить секцию «Stale Installation Cleanup» с описанием flow - Sequence diagram для markOldInstallations - Sequence diagram для reinstall после needReinstall +- Обновить state machine — новые переходы - Обновить секцию Follow-Up — отметить что issue #92 реализован **CHANGELOG.md:** - - Новая запись: feature — mark old installations as needReinstall via background command ### Test Cases -#### Unit Tests +#### Unit Tests — ✅ реализованы + +**MarkOldInstallations/ConfigTest** (`tests/Unit/ApplicationInstallations/UseCase/MarkOldInstallations/ConfigTest.php`): +1. TTL = 0, 3600, 1800 — успех +2. TTL = -1 — InvalidArgumentException -**MarkOldInstallations/HandlerTest:** -1. Находит одну зависшую установку → переводит в `needReinstall`, событие диспатчится -2. Находит несколько зависших → все переведены -3. Нет зависших → ничего не делает, flush не вызван -4. TTL = 0 → находит все `new` установки +> Entity-тесты (`markAsNeedReinstall` transitions) — идут в SDK, не в этот репо. +> Handler/Workflow unit-тесты убраны — это functional concern, моки репы/Handler здесь неуместны. -**MarkOldInstallations/CommandTest:** -1. Дефолтный TTL = 3600 -2. Кастомный TTL передаётся корректно +#### Functional Tests — ✅ реализованы -**Entity/ApplicationInstallation — markAsNeedReinstall:** -1. `new → needReinstall` — успех, событие создано -2. `active → needReinstall` — exception -3. `blocked → needReinstall` — exception -4. `needReinstall → deleted` через `applicationUninstalled(null)` — успех +**MarkOldInstallations/HandlerTest** (`tests/Functional/ApplicationInstallations/UseCase/MarkOldInstallations/HandlerTest.php`): +1. Stale installation (`createdAt = NOW() - 2h`) → TTL=3600 → статус `needReinstall`, событие `ApplicationInstallationMarkedNeedReinstallEvent` диспатчнуто +2. Fresh installation (`createdAt = NOW`) → TTL=3600 → статус остаётся `new` +3. No stale installations → ничего не происходит, событие не диспатчится -#### Functional Tests +**MarkOldInstallations/WorkflowTest** (`tests/Functional/ApplicationInstallations/UseCase/MarkOldInstallations/WorkflowTest.php`): +1. Full flow: stale installation → `Result.processedInstallations` содержит событие с правильным ID +2. No stale installations → `Result` с пустым массивом `processedInstallations` -**MarkOldInstallations/HandlerTest:** -1. Создаём `new` установку с `createdAt = NOW() - 2h`, запускаем с TTL=3600 → статус `needReinstall` -2. Создаём `new` установку с `createdAt = NOW() - 30m`, запускаем с TTL=3600 → статус остаётся `new` -3. Запускаем Install повторно → старая `needReinstall` установка удаляется, новая создаётся +**Install/HandlerTest** — добавлен тест `testReinstallOverNeedReinstallInstallationDeletesOldEntitiesAndCreatesNewPendingPair`: +- Reinstall поверх `needReinstall` → старая `deleted`, новая пара создана +- `markAsBlocked` пропущен (срабатывает только для `new`), сразу `applicationUninstalled(null)` → `deleted` +- Событие `ApplicationInstallationBlockedEvent` НЕ диспатчнуто ### Assumptions - `needReinstall` добавляется только в `ApplicationInstallationStatus`, не в `Bitrix24AccountStatus` - Мастер-аккаунт при markOld не меняет статус — остаётся в `new` - `applicationUninstalled(null)` из `needReinstall → deleted` не требует блокировки -- Метод `findStaleInstallations` сначала живёт в локальном репозитории этой библиотеки, потом переносится в SDK-интерфейс отдельным PR +- Метод `findStaleInstallations` живёт в локальном репозитории этой библиотеки (не в SDK-интерфейсе) - Scheduling (cron/worker) — ответственность потребителя библиотеки -- Константа TTL по умолчанию = 3600 секунд, находится в Command +- Константа TTL по умолчанию = 3600 секунд, находится в Console command (`DEFAULT_TTL`) +- При reinstall удаляются ВСЕ аккаунты портала (master + child), а не только master — см. `Install/Handler.php:125-138` diff --git a/tests/Functional/ApplicationInstallations/UseCase/Install/HandlerTest.php b/tests/Functional/ApplicationInstallations/UseCase/Install/HandlerTest.php index e3b6103b..e1eb3944 100644 --- a/tests/Functional/ApplicationInstallations/UseCase/Install/HandlerTest.php +++ b/tests/Functional/ApplicationInstallations/UseCase/Install/HandlerTest.php @@ -152,6 +152,36 @@ public function testReinstallOverPendingInstallationDeletesOldEntitiesAndCreates self::assertNotContains(ApplicationInstallationFinishedEvent::class, $events); } + #[Test] + public function testReinstallOverNeedReinstallInstallationDeletesOldEntitiesAndCreatesNewPendingPair(): void + { + $memberId = Uuid::v4()->toRfc4122(); + $bitrix24Account = $this->createAccount($memberId); + $applicationInstallation = $this->createInstallation($bitrix24Account->getId()); + $applicationInstallation->markAsNeedReinstall('installation timed out without ONAPPINSTALL'); + + $this->entityManager->persist($bitrix24Account); + $this->entityManager->persist($applicationInstallation); + $this->entityManager->flush(); + $this->entityManager->clear(); + + $this->handler->handle($this->createCommand(null, $memberId)); + $this->entityManager->clear(); + + /** @var ApplicationInstallation $deletedInstallation */ + $deletedInstallation = $this->entityManager->find(ApplicationInstallation::class, $applicationInstallation->getId()); + $currentInstallation = $this->installationRepository->findByBitrix24AccountMemberId($memberId); + + self::assertNotNull($currentInstallation); + self::assertSame(ApplicationInstallationStatus::deleted, $deletedInstallation->getStatus()); + self::assertSame(ApplicationInstallationStatus::new, $currentInstallation->getStatus()); + self::assertNotSame($applicationInstallation->getId()->toRfc4122(), $currentInstallation->getId()->toRfc4122()); + + $events = $this->eventDispatcher->getOrphanedEvents(); + self::assertContains(ApplicationInstallationUninstalledEvent::class, $events); + self::assertNotContains(ApplicationInstallationBlockedEvent::class, $events); + } + private function createCommand(?string $applicationToken = null, ?string $memberId = null): Command { return new Command( diff --git a/tests/Functional/ApplicationInstallations/UseCase/MarkOldInstallations/HandlerTest.php b/tests/Functional/ApplicationInstallations/UseCase/MarkOldInstallations/HandlerTest.php new file mode 100644 index 00000000..e152a5fc --- /dev/null +++ b/tests/Functional/ApplicationInstallations/UseCase/MarkOldInstallations/HandlerTest.php @@ -0,0 +1,150 @@ +entityManager = EntityManagerFactory::get(); + $this->eventDispatcher = new TraceableEventDispatcher(new EventDispatcher(), new Stopwatch()); + $this->installationRepository = new ApplicationInstallationRepository($this->entityManager); + + $this->handler = new Handler( + $this->installationRepository, + new Flusher($this->entityManager, $this->eventDispatcher), + new NullLogger() + ); + } + + #[Test] + public function testStaleInstallationIsMarkedAsNeedReinstall(): void + { + $bitrix24Account = $this->createAccount(); + $installation = $this->createInstallation($bitrix24Account->getId()); + + $this->entityManager->persist($bitrix24Account); + $this->entityManager->persist($installation); + $this->entityManager->flush(); + + $this->backdateCreatedAt($installation->getId(), new CarbonImmutable('-2 hours')); + + $this->handler->handle(new Command(3600)); + $this->entityManager->clear(); + + $updated = $this->installationRepository->getById($installation->getId()); + + self::assertSame(ApplicationInstallationStatus::needReinstall, $updated->getStatus()); + + $events = $this->eventDispatcher->getOrphanedEvents(); + self::assertContains(ApplicationInstallationMarkedNeedReinstallEvent::class, $events); + } + + #[Test] + public function testFreshInstallationStaysNew(): void + { + $bitrix24Account = $this->createAccount(); + $installation = $this->createInstallation($bitrix24Account->getId()); + + $this->entityManager->persist($bitrix24Account); + $this->entityManager->persist($installation); + $this->entityManager->flush(); + $this->entityManager->clear(); + + $this->handler->handle(new Command(3600)); + + $updated = $this->installationRepository->getById($installation->getId()); + + self::assertSame(ApplicationInstallationStatus::new, $updated->getStatus()); + } + + #[Test] + public function testNoStaleInstallationsDoesNothing(): void + { + $this->handler->handle(new Command(3600)); + + $events = $this->eventDispatcher->getOrphanedEvents(); + self::assertNotContains(ApplicationInstallationMarkedNeedReinstallEvent::class, $events); + } + + private function createAccount(): Bitrix24Account + { + return new Bitrix24Account( + Uuid::v7(), + 1, + true, + Uuid::v4()->toRfc4122(), + 'example.bitrix24.test', + new AuthToken('access', 'refresh', 3600, time() + 3600), + 1, + new Scope(['crm']), + true + ); + } + + private function createInstallation(Uuid $bitrix24AccountId): ApplicationInstallation + { + return new ApplicationInstallation( + Uuid::v7(), + $bitrix24AccountId, + new ApplicationStatus('F'), + PortalLicenseFamily::free, + 10, + null, + null, + null, + 'lead-1', + 'install' + ); + } + + private function backdateCreatedAt(Uuid $installationId, CarbonImmutable $createdAt): void + { + $this->entityManager->createQuery( + 'UPDATE ' . ApplicationInstallation::class . ' ai SET ai.createdAt = :createdAt WHERE ai.id = :id' + ) + ->setParameter('createdAt', $createdAt) + ->setParameter('id', $installationId, 'uuid') + ->execute(); + } +} diff --git a/tests/Functional/ApplicationInstallations/UseCase/MarkOldInstallations/WorkflowTest.php b/tests/Functional/ApplicationInstallations/UseCase/MarkOldInstallations/WorkflowTest.php new file mode 100644 index 00000000..2fcc92da --- /dev/null +++ b/tests/Functional/ApplicationInstallations/UseCase/MarkOldInstallations/WorkflowTest.php @@ -0,0 +1,135 @@ +entityManager = EntityManagerFactory::get(); + $this->eventDispatcher = new EventDispatcher(); + $this->installationRepository = new ApplicationInstallationRepository($this->entityManager); + + $handler = new Handler( + $this->installationRepository, + new Flusher($this->entityManager, $this->eventDispatcher), + new NullLogger() + ); + + $this->workflow = new Workflow( + $handler, + $this->eventDispatcher, + new NullLogger() + ); + } + + #[Test] + public function testFullFlowReturnsResultWithProcessedInstallations(): void + { + $bitrix24Account = $this->createAccount(); + $installation = $this->createInstallation($bitrix24Account->getId()); + + $this->entityManager->persist($bitrix24Account); + $this->entityManager->persist($installation); + $this->entityManager->flush(); + + $this->backdateCreatedAt($installation->getId(), new CarbonImmutable('-2 hours')); + + $result = $this->workflow->run(new MarkOldInstallationsConfig(3600)); + $this->entityManager->clear(); + + self::assertCount(1, $result->processedInstallations); + self::assertTrue( + $installation->getId()->equals($result->processedInstallations[0]->applicationInstallationId) + ); + + $updated = $this->installationRepository->getById($installation->getId()); + self::assertSame(ApplicationInstallationStatus::needReinstall, $updated->getStatus()); + } + + #[Test] + public function testNoStaleInstallationsReturnsEmptyResult(): void + { + $result = $this->workflow->run(new MarkOldInstallationsConfig(3600)); + + self::assertSame([], $result->processedInstallations); + } + + private function createAccount(): Bitrix24Account + { + return new Bitrix24Account( + Uuid::v7(), + 1, + true, + Uuid::v4()->toRfc4122(), + 'example.bitrix24.test', + new AuthToken('access', 'refresh', 3600, time() + 3600), + 1, + new Scope(['crm']), + true + ); + } + + private function createInstallation(Uuid $bitrix24AccountId): ApplicationInstallation + { + return new ApplicationInstallation( + Uuid::v7(), + $bitrix24AccountId, + new ApplicationStatus('F'), + PortalLicenseFamily::free, + 10, + null, + null, + null, + 'lead-1', + 'install' + ); + } + + private function backdateCreatedAt(Uuid $installationId, CarbonImmutable $createdAt): void + { + $this->entityManager->createQuery( + 'UPDATE ' . ApplicationInstallation::class . ' ai SET ai.createdAt = :createdAt WHERE ai.id = :id' + ) + ->setParameter('createdAt', $createdAt) + ->setParameter('id', $installationId, 'uuid') + ->execute(); + } +} diff --git a/tests/Unit/ApplicationInstallations/UseCase/MarkOldInstallations/ConfigTest.php b/tests/Unit/ApplicationInstallations/UseCase/MarkOldInstallations/ConfigTest.php new file mode 100644 index 00000000..2157e674 --- /dev/null +++ b/tests/Unit/ApplicationInstallations/UseCase/MarkOldInstallations/ConfigTest.php @@ -0,0 +1,43 @@ +ttlInSeconds); + } + + #[Test] + public function testNegativeTtlThrowsException(): void + { + $this->expectException(InvalidArgumentException::class); + + new MarkOldInstallationsConfig(-1); + } + + public static function validTtlProvider(): \Generator + { + yield 'zero' => [0]; + yield 'one_hour' => [3600]; + yield 'thirty_minutes' => [1800]; + } +} From 39f68b051323ccddacc483edb9bcef3a5e38cc7b Mon Sep 17 00:00:00 2001 From: Kirill Hramov Date: Tue, 18 Aug 2026 18:00:03 +0300 Subject: [PATCH 7/7] Refactor `MarkOldInstallations` use case: replace batch workflow with single-command aggregation - Removed `Workflow`, `Config`, `Result`, and `Collector` classes for simplified logic. - Introduced `MarkAsNeedReinstall` use case to handle single-installation operations with robust validation. - Updated console command to perform batch orchestration and handle race conditions with inline validation. - Added new functional tests for `MarkAsNeedReinstall` use case to ensure correctness and reliability. --- .tasks/92/mark-old-installations-plan.md | 78 +++++----- .../Console/MarkOldInstallationsCommand.php | 89 ++++++++---- .../UseCase/MarkAsNeedReinstall/Command.php | 15 ++ .../UseCase/MarkAsNeedReinstall/Handler.php | 47 ++++++ .../UseCase/MarkOldInstallations/Command.php | 18 --- .../UseCase/MarkOldInstallations/Handler.php | 66 --------- .../MarkOldInstallationsCollector.php | 26 ---- .../MarkOldInstallationsConfig.php | 18 --- .../MarkOldInstallationsResult.php | 17 --- .../UseCase/MarkOldInstallations/Workflow.php | 52 ------- .../ApplicationInstallationRepositoryTest.php | 83 +++++++++++ .../HandlerTest.php | 72 ++++------ .../MarkOldInstallations/WorkflowTest.php | 135 ------------------ .../MarkOldInstallations/ConfigTest.php | 43 ------ 14 files changed, 272 insertions(+), 487 deletions(-) create mode 100644 src/ApplicationInstallations/UseCase/MarkAsNeedReinstall/Command.php create mode 100644 src/ApplicationInstallations/UseCase/MarkAsNeedReinstall/Handler.php delete mode 100644 src/ApplicationInstallations/UseCase/MarkOldInstallations/Command.php delete mode 100644 src/ApplicationInstallations/UseCase/MarkOldInstallations/Handler.php delete mode 100644 src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsCollector.php delete mode 100644 src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsConfig.php delete mode 100644 src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsResult.php delete mode 100644 src/ApplicationInstallations/UseCase/MarkOldInstallations/Workflow.php rename tests/Functional/ApplicationInstallations/UseCase/{MarkOldInstallations => MarkAsNeedReinstall}/HandlerTest.php (59%) delete mode 100644 tests/Functional/ApplicationInstallations/UseCase/MarkOldInstallations/WorkflowTest.php delete mode 100644 tests/Unit/ApplicationInstallations/UseCase/MarkOldInstallations/ConfigTest.php diff --git a/.tasks/92/mark-old-installations-plan.md b/.tasks/92/mark-old-installations-plan.md index c8c524a5..8b745f8b 100644 --- a/.tasks/92/mark-old-installations-plan.md +++ b/.tasks/92/mark-old-installations-plan.md @@ -40,22 +40,23 @@ active → deleted (applicationUninstalled) needReinstall → deleted (applicationUninstalled) ← НОВОЕ ``` -2. **markOldInstallations flow:** +2. **markOldInstallations flow (после рефакторинга по ревью):** ``` Console command (ttl=3600 по дефолту) - → Workflow.run(Config) - → регистрация listener на ApplicationInstallationMarkedNeedReinstallEvent - → Handler::handle(Command) - → repository.findStaleInstallations(status=new, olderThan=NOW()-ttl) - → для каждого: installation.markAsNeedReinstall(comment) - → repository.save(installation) - → flusher.flush(installation) - → событие ApplicationInstallationMarkedNeedReinstallEvent диспатчится через EventDispatcher - → unregister listener - → collector.getEvents() → Result(processedInstallations) - → Console рендерит список переведённых установок + → валидация ttl inline (>= 0) + → repository.findStaleInstallations(status=new, olderThan=NOW()-ttl) // query side, CQRS read + → foreach stale: + → MarkAsNeedReinstall\Handler::handle(Command(installationId, comment)) // command side + → getById(installationId) // identity map: без доп. SQL, ре-проверка guard от race с ONAPPINSTALL + → installation.markAsNeedReinstall(comment) + → repository.save + flusher.flush → событие ApplicationInstallationMarkedNeedReinstallEvent + → catch LogicException → skip (race condition: статус изменился параллельно) + → рендер: помеченные ID + count + пропущенные ID ``` +> Рефакторинг по итогам ревью: use case — единоразовая операция над одним агрегатом (без foreach), +> батч-оркестрация вынесена в Console command. Workflow/Config/Result/Collector удалены. + 3. **Переустановка после markOldInstallations:** ``` Install\Command (повторная установка) @@ -70,7 +71,7 @@ Install\Command (повторная установка) ### Implementation Changes -#### PR 1: SDK (bitrix24/b24phpsdk) +#### PR 1: SDK (bitrix24/b24phpsdk) — разбит на issues #576–#580 **Файлы SDK:** @@ -99,26 +100,27 @@ Install\Command (повторная установка) - WHERE `status = :status AND createdAt < :olderThan` - ORDER BY `createdAt ASC` -**UseCase — `src/ApplicationInstallations/UseCase/MarkOldInstallations/`:** +**UseCase — `src/ApplicationInstallations/UseCase/MarkAsNeedReinstall/`** (единоразовая операция над одним агрегатом): | Файл | Содержание | |---|---| -| `Command.php` | `public int $ttlInSeconds` + валидация (>= 0) | -| `Handler.php` | `handle(Command): void` — находит зависшие через `findStaleInstallations`, вызывает `markAsNeedReinstall()`, сохраняет, flush | -| `MarkOldInstallationsConfig.php` | `public int $ttlInSeconds` + валидация | -| `MarkOldInstallationsResult.php` | `public array $processedInstallations` (массив `ApplicationInstallationMarkedNeedReinstallEvent`) | -| `MarkOldInstallationsCollector.php` | Коллектор событий — `add(event)`, `getEvents(): array` | -| `Workflow.php` | Оркестратор: регистрирует listener на EventDispatcher → `handler->handle()` → unregister → `Result(processedInstallations)` | +| `Command.php` | `installationId: Uuid`, `comment: ?string` | +| `Handler.php` | `handle(Command): void` — `getById` → `markAsNeedReinstall(comment)` → save → flush → лог. Комментарий у `getById` про identity map | + +**Удалено (после рефакторинга по ревью):** весь `UseCase/MarkOldInstallations/` — Workflow, Handler, Command, MarkOldInstallationsConfig, MarkOldInstallationsResult, MarkOldInstallationsCollector. -**Console — `src/ApplicationInstallations/Console/MarkOldInstallationsCommand.php`:** +**Console — `src/ApplicationInstallations/Console/MarkOldInstallationsCommand.php`** (батч-оркестратор): ```php #[AsCommand(name: 'bitrix24:installations:mark-old')] class MarkOldInstallationsCommand extends Command { public const DEFAULT_TTL = 3600; - // аргумент: ttl (опциональный, дефолт DEFAULT_TTL) - // рендерит список переведённых установок + count + // аргумент: ttl (опциональный, дефолт DEFAULT_TTL), валидация inline + // конструктор: ApplicationInstallationRepository + MarkAsNeedReinstall\Handler + // findStaleInstallations(new, NOW()-ttl) → foreach → handler->handle(new Command(id, comment)) + // catch LogicException → skip (race condition с ONAPPINSTALL) + // рендер: помеченные ID + count, пропущенные ID } ``` @@ -134,37 +136,39 @@ class MarkOldInstallationsCommand extends Command ### Test Cases -#### Unit Tests — ✅ реализованы - -**MarkOldInstallations/ConfigTest** (`tests/Unit/ApplicationInstallations/UseCase/MarkOldInstallations/ConfigTest.php`): -1. TTL = 0, 3600, 1800 — успех -2. TTL = -1 — InvalidArgumentException +#### Unit Tests — удалены после рефакторинга +> ConfigTest удалён вместе с MarkOldInstallationsConfig (валидация TTL теперь inline в Console command, консоль не тестируем unit-тестами). > Entity-тесты (`markAsNeedReinstall` transitions) — идут в SDK, не в этот репо. -> Handler/Workflow unit-тесты убраны — это functional concern, моки репы/Handler здесь неуместны. #### Functional Tests — ✅ реализованы -**MarkOldInstallations/HandlerTest** (`tests/Functional/ApplicationInstallations/UseCase/MarkOldInstallations/HandlerTest.php`): -1. Stale installation (`createdAt = NOW() - 2h`) → TTL=3600 → статус `needReinstall`, событие `ApplicationInstallationMarkedNeedReinstallEvent` диспатчнуто -2. Fresh installation (`createdAt = NOW`) → TTL=3600 → статус остаётся `new` -3. No stale installations → ничего не происходит, событие не диспатчится +**MarkAsNeedReinstall/HandlerTest** (`tests/Functional/ApplicationInstallations/UseCase/MarkAsNeedReinstall/HandlerTest.php`): +1. Pending installation в `new` → handle → статус `needReinstall`, comment сохранён, событие `ApplicationInstallationMarkedNeedReinstallEvent` диспатчнуто +2. Active installation → `LogicException` +3. Неизвестный installationId → `ApplicationInstallationNotFoundException` -**MarkOldInstallations/WorkflowTest** (`tests/Functional/ApplicationInstallations/UseCase/MarkOldInstallations/WorkflowTest.php`): -1. Full flow: stale installation → `Result.processedInstallations` содержит событие с правильным ID -2. No stale installations → `Result` с пустым массивом `processedInstallations` +**ApplicationInstallationRepositoryTest** — добавлен `testFindStaleInstallationsReturnsOnlyOldEnoughNewOnes`: +- Old `new`-installation (createdAt=NOW()-2h, TTL=3600) → найдена +- Fresh `new`-installation → не найдена +- Old `active`-installation (createdAt=NOW()-2h) → не найдена (статус не тот) +- `backdateCreatedAt` через DQL UPDATE (createdAt readonly в конструкторе) **Install/HandlerTest** — добавлен тест `testReinstallOverNeedReinstallInstallationDeletesOldEntitiesAndCreatesNewPendingPair`: - Reinstall поверх `needReinstall` → старая `deleted`, новая пара создана - `markAsBlocked` пропущен (срабатывает только для `new`), сразу `applicationUninstalled(null)` → `deleted` - Событие `ApplicationInstallationBlockedEvent` НЕ диспатчнуто +> Удалены после рефакторинга: MarkOldInstallations/ConfigTest, HandlerTest, WorkflowTest (unit + functional). + ### Assumptions - `needReinstall` добавляется только в `ApplicationInstallationStatus`, не в `Bitrix24AccountStatus` - Мастер-аккаунт при markOld не меняет статус — остаётся в `new` - `applicationUninstalled(null)` из `needReinstall → deleted` не требует блокировки -- Метод `findStaleInstallations` живёт в локальном репозитории этой библиотеки (не в SDK-интерфейсе) +- Метод `findStaleInstallations` временно живёт в локальном репозитории; в SDK-интерфейс пойдёт отдельным issue ([b24phpsdk#579](https://github.com/bitrix24/b24phpsdk/issues/579)) - Scheduling (cron/worker) — ответственность потребителя библиотеки - Константа TTL по умолчанию = 3600 секунд, находится в Console command (`DEFAULT_TTL`) - При reinstall удаляются ВСЕ аккаунты портала (master + child), а не только master — см. `Install/Handler.php:125-138` +- Use case обрабатывает ровно один агрегат; повторная загрузка через `getById` не даёт доп. SQL (Doctrine identity map) и защищает guard от race с ONAPPINSTALL +- SDK issues: [#576](https://github.com/bitrix24/b24phpsdk/issues/576) (enum), [#577](https://github.com/bitrix24/b24phpsdk/issues/577) (interface method), [#578](https://github.com/bitrix24/b24phpsdk/issues/578) (event), [#579](https://github.com/bitrix24/b24phpsdk/issues/579) (repo interface), [#580](https://github.com/bitrix24/b24phpsdk/issues/580) (reference impl + tests) diff --git a/src/ApplicationInstallations/Console/MarkOldInstallationsCommand.php b/src/ApplicationInstallations/Console/MarkOldInstallationsCommand.php index 8a4c79b5..79379709 100644 --- a/src/ApplicationInstallations/Console/MarkOldInstallationsCommand.php +++ b/src/ApplicationInstallations/Console/MarkOldInstallationsCommand.php @@ -4,16 +4,19 @@ namespace Bitrix24\Lib\ApplicationInstallations\Console; -use Bitrix24\Lib\ApplicationInstallations\UseCase\MarkOldInstallations\MarkOldInstallationsConfig; -use Bitrix24\Lib\ApplicationInstallations\UseCase\MarkOldInstallations\Workflow; -use Bitrix24\Lib\ApplicationInstallations\UseCase\MarkOldInstallations\MarkOldInstallationsResult; -use Bitrix24\SDK\Core\Exceptions\InvalidArgumentException; +use Bitrix24\Lib\ApplicationInstallations\Infrastructure\Doctrine\ApplicationInstallationRepository; +use Bitrix24\Lib\ApplicationInstallations\UseCase\MarkAsNeedReinstall\Command as MarkAsNeedReinstallCommand; +use Bitrix24\Lib\ApplicationInstallations\UseCase\MarkAsNeedReinstall\Handler; +use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Entity\ApplicationInstallationStatus; +use Bitrix24\SDK\Core\Exceptions\LogicException; +use Carbon\CarbonImmutable; use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Style\SymfonyStyle; +use Symfony\Component\Uid\Uuid; #[AsCommand( name: 'bitrix24:installations:mark-old', @@ -26,7 +29,8 @@ class MarkOldInstallationsCommand extends Command private ?SymfonyStyle $io = null; public function __construct( - private readonly Workflow $workflow + private readonly ApplicationInstallationRepository $applicationInstallationRepository, + private readonly Handler $markAsNeedReinstallHandler ) { parent::__construct(); } @@ -61,48 +65,75 @@ protected function execute(InputInterface $input, OutputInterface $output): int { $this->io = new SymfonyStyle($input, $output); - $config = $this->parseInput($input); - if (null === $config) { + $ttl = (int) $input->getArgument('ttl'); + if ($ttl < 0) { + $this->io->error('TTL in seconds must be a non-negative integer.'); + return Command::FAILURE; } - $result = $this->workflow->run($config); + $olderThan = new CarbonImmutable(); + $olderThan = $olderThan->subSeconds($ttl); + + $staleInstallations = $this->applicationInstallationRepository->findStaleInstallations( + ApplicationInstallationStatus::new, + $olderThan + ); - return $this->renderResult($result); + return $this->processStaleInstallations($staleInstallations, $ttl); } - private function parseInput(InputInterface $input): ?MarkOldInstallationsConfig + private function processStaleInstallations(array $staleInstallations, int $ttl): int { - $ttl = (int) $input->getArgument('ttl'); + if ([] === $staleInstallations) { + $this->io->success('No stale installations found.'); + + return Command::SUCCESS; + } + + $comment = sprintf('installation timed out without ONAPPINSTALL, TTL = %d seconds', $ttl); + + $markedIds = []; + $failedIds = []; - try { - return new MarkOldInstallationsConfig($ttl); - } catch (InvalidArgumentException $invalidArgumentException) { - $this->io->error($invalidArgumentException->getMessage()); + foreach ($staleInstallations as $staleInstallation) { + $installationId = $staleInstallation->getId(); + + try { + $this->markAsNeedReinstallHandler->handle( + new MarkAsNeedReinstallCommand($installationId, $comment) + ); + + $markedIds[] = $installationId; + } catch (LogicException) { + // Installation changed status concurrently (e.g. ONAPPINSTALL arrived) — skip it. + $failedIds[] = $installationId; + } } - return null; + return $this->renderResult($markedIds, $failedIds); } - private function renderResult(MarkOldInstallationsResult $result): int + /** + * @param Uuid[] $markedIds + * @param Uuid[] $failedIds + */ + private function renderResult(array $markedIds, array $failedIds): int { - $count = count($result->processedInstallations); - if (0 === $count) { - $this->io->success('No stale installations found.'); + $this->io->success(sprintf('Marked %d installation(s) as needReinstall:', count($markedIds))); - return 0; + foreach ($markedIds as $installationId) { + $this->io->text(sprintf(' - installation %s', $installationId->toRfc4122())); } - $this->io->success(sprintf('Marked %d installation(s) as needReinstall:', $count)); + if ([] !== $failedIds) { + $this->io->warning(sprintf('Skipped %d installation(s) changed status concurrently:', count($failedIds))); - foreach ($result->processedInstallations as $event) { - $this->io->text(sprintf( - ' - installation %s, marked at %s', - $event->applicationInstallationId->toRfc4122(), - $event->timestamp->toAtomString() - )); + foreach ($failedIds as $installationId) { + $this->io->text(sprintf(' - installation %s', $installationId->toRfc4122())); + } } - return 0; + return Command::SUCCESS; } } diff --git a/src/ApplicationInstallations/UseCase/MarkAsNeedReinstall/Command.php b/src/ApplicationInstallations/UseCase/MarkAsNeedReinstall/Command.php new file mode 100644 index 00000000..5429631f --- /dev/null +++ b/src/ApplicationInstallations/UseCase/MarkAsNeedReinstall/Command.php @@ -0,0 +1,15 @@ +logger->info('ApplicationInstallations.MarkAsNeedReinstall.start', [ + 'installationId' => $command->installationId->toRfc4122(), + ]); + + // getById does not produce an extra SQL query when the entity is already + // managed by Doctrine's identity map (e.g. loaded by findStaleInstallations + // in the same session). Re-loading here also re-checks the status guard + // against a race with a concurrently arriving ONAPPINSTALL. + $applicationInstallation = $this->applicationInstallationRepository->getById($command->installationId); + assert($applicationInstallation instanceof AggregateRootEventsEmitterInterface); + + $applicationInstallation->markAsNeedReinstall($command->comment); + + $this->applicationInstallationRepository->save($applicationInstallation); + $this->flusher->flush($applicationInstallation); + + $this->logger->info('ApplicationInstallations.MarkAsNeedReinstall.finish', [ + 'installationId' => $command->installationId->toRfc4122(), + 'createdAt' => $applicationInstallation->getCreatedAt()->toAtomString(), + ]); + } +} diff --git a/src/ApplicationInstallations/UseCase/MarkOldInstallations/Command.php b/src/ApplicationInstallations/UseCase/MarkOldInstallations/Command.php deleted file mode 100644 index 0eacf9f9..00000000 --- a/src/ApplicationInstallations/UseCase/MarkOldInstallations/Command.php +++ /dev/null @@ -1,18 +0,0 @@ -ttlInSeconds < 0) { - throw new InvalidArgumentException('TTL in seconds must be a non-negative integer.'); - } - } -} diff --git a/src/ApplicationInstallations/UseCase/MarkOldInstallations/Handler.php b/src/ApplicationInstallations/UseCase/MarkOldInstallations/Handler.php deleted file mode 100644 index 4f816659..00000000 --- a/src/ApplicationInstallations/UseCase/MarkOldInstallations/Handler.php +++ /dev/null @@ -1,66 +0,0 @@ -subSeconds($command->ttlInSeconds); - - return $this->applicationInstallationRepository->findStaleInstallations( - ApplicationInstallationStatus::new, - $olderThan - ); - } - - /** - * @throws LogicException - */ - public function handle(Command $command): void - { - $this->logger->info('ApplicationInstallations.MarkOldInstallations.start', [ - 'ttlInSeconds' => $command->ttlInSeconds, - ]); - - $staleInstallations = $this->findStaleInstallations($command); - - foreach ($staleInstallations as $staleInstallation) { - $staleInstallation->markAsNeedReinstall( - sprintf('installation timed out without ONAPPINSTALL, TTL = %d seconds', $command->ttlInSeconds) - ); - - $this->applicationInstallationRepository->save($staleInstallation); - $this->flusher->flush($staleInstallation); - - $this->logger->info('ApplicationInstallations.MarkOldInstallations.marked', [ - 'installationId' => $staleInstallation->getId()->toRfc4122(), - 'createdAt' => $staleInstallation->getCreatedAt()->toAtomString(), - ]); - } - - $this->logger->info('ApplicationInstallations.MarkOldInstallations.finish'); - } -} diff --git a/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsCollector.php b/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsCollector.php deleted file mode 100644 index 7a8d8505..00000000 --- a/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsCollector.php +++ /dev/null @@ -1,26 +0,0 @@ -events[] = $event; - } - - /** - * @return ApplicationInstallationMarkedNeedReinstallEvent[] - */ - public function getEvents(): array - { - return $this->events; - } -} diff --git a/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsConfig.php b/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsConfig.php deleted file mode 100644 index c8601e5e..00000000 --- a/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsConfig.php +++ /dev/null @@ -1,18 +0,0 @@ -ttlInSeconds < 0) { - throw new InvalidArgumentException('TTL in seconds must be a non-negative integer.'); - } - } -} diff --git a/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsResult.php b/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsResult.php deleted file mode 100644 index 51d91b55..00000000 --- a/src/ApplicationInstallations/UseCase/MarkOldInstallations/MarkOldInstallationsResult.php +++ /dev/null @@ -1,17 +0,0 @@ -logger->info('ApplicationInstallations.MarkOldInstallations.Workflow.start', [ - 'ttlInSeconds' => $config->ttlInSeconds, - ]); - - $command = new Command($config->ttlInSeconds); - - $collector = new MarkOldInstallationsCollector(); - $listener = $collector->add(...); - - $this->eventDispatcher->addListener( - ApplicationInstallationMarkedNeedReinstallEvent::class, - $listener - ); - - try { - $this->handler->handle($command); - } finally { - $this->eventDispatcher->removeListener( - ApplicationInstallationMarkedNeedReinstallEvent::class, - $listener - ); - } - - $processedInstallations = $collector->getEvents(); - - $this->logger->info('ApplicationInstallations.MarkOldInstallations.Workflow.finish', [ - 'processedCount' => count($processedInstallations), - ]); - - return new MarkOldInstallationsResult($processedInstallations); - } -} diff --git a/tests/Functional/ApplicationInstallations/Infrastructure/Doctrine/ApplicationInstallationRepositoryTest.php b/tests/Functional/ApplicationInstallations/Infrastructure/Doctrine/ApplicationInstallationRepositoryTest.php index 925d0aca..5d0b2e2b 100644 --- a/tests/Functional/ApplicationInstallations/Infrastructure/Doctrine/ApplicationInstallationRepositoryTest.php +++ b/tests/Functional/ApplicationInstallations/Infrastructure/Doctrine/ApplicationInstallationRepositoryTest.php @@ -6,14 +6,19 @@ use Bitrix24\Lib\ApplicationInstallations\Entity\ApplicationInstallation; use Bitrix24\Lib\ApplicationInstallations\Infrastructure\Doctrine\ApplicationInstallationRepository; +use Bitrix24\Lib\Bitrix24Accounts\Entity\Bitrix24Account; use Bitrix24\SDK\Application\ApplicationStatus; use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Entity\ApplicationInstallationInterface; use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Entity\ApplicationInstallationStatus; use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Repository\ApplicationInstallationRepositoryInterface; use Bitrix24\SDK\Application\PortalLicenseFamily; +use Bitrix24\SDK\Core\Credentials\AuthToken; +use Bitrix24\SDK\Core\Credentials\Scope; use Bitrix24\SDK\Tests\Application\Contracts\ApplicationInstallations\Repository\ApplicationInstallationRepositoryInterfaceTest; use Bitrix24\SDK\Tests\Application\Contracts\TestRepositoryFlusherInterface; use Carbon\CarbonImmutable; +use Doctrine\ORM\EntityManagerInterface; +use PHPUnit\Framework\Attributes\Test; use Symfony\Component\EventDispatcher\EventDispatcher; use Symfony\Component\Uid\Uuid; use PHPUnit\Framework\Attributes\CoversClass; @@ -72,5 +77,83 @@ protected function createRepositoryFlusherImplementation(): TestRepositoryFlushe return new FlusherDecorator(new Flusher($entityManager, $eventDispatcher)); } + #[Test] + public function testFindStaleInstallationsReturnsOnlyOldEnoughNewOnes(): void + { + $entityManager = EntityManagerFactory::get(); + $repository = new ApplicationInstallationRepository($entityManager); + + $oldInstallation = $this->persistInstallation($entityManager); + $this->backdateCreatedAt($entityManager, $oldInstallation->getId(), new CarbonImmutable('-2 hours')); + + $freshInstallation = $this->persistInstallation($entityManager); + $activeInstallation = $this->persistInstallation($entityManager); + $activeInstallation->applicationInstalled(Uuid::v7()->toRfc4122()); + $this->backdateCreatedAt($entityManager, $activeInstallation->getId(), new CarbonImmutable('-2 hours')); + $entityManager->flush(); + $entityManager->clear(); + + $found = $repository->findStaleInstallations( + ApplicationInstallationStatus::new, + (new CarbonImmutable())->subSeconds(3600) + ); + + $foundIds = array_map( + static fn (ApplicationInstallationInterface $installation): string => $installation->getId()->toRfc4122(), + $found + ); + + self::assertContains($oldInstallation->getId()->toRfc4122(), $foundIds); + self::assertNotContains($freshInstallation->getId()->toRfc4122(), $foundIds); + self::assertNotContains($activeInstallation->getId()->toRfc4122(), $foundIds); + } + + private function persistInstallation(EntityManagerInterface $entityManager): ApplicationInstallation + { + $bitrix24Account = new Bitrix24Account( + Uuid::v7(), + 1, + true, + Uuid::v4()->toRfc4122(), + 'example.bitrix24.test', + new AuthToken('access', 'refresh', 3600, time() + 3600), + 1, + new Scope(['crm']), + true + ); + + $applicationInstallation = new ApplicationInstallation( + Uuid::v7(), + $bitrix24Account->getId(), + new ApplicationStatus('F'), + PortalLicenseFamily::free, + 10, + null, + null, + null, + 'lead-1', + 'install' + ); + + $entityManager->persist($bitrix24Account); + $entityManager->persist($applicationInstallation); + $entityManager->flush(); + + return $applicationInstallation; + } + + private function backdateCreatedAt( + EntityManagerInterface $entityManager, + Uuid $installationId, + CarbonImmutable $createdAt + ): void { + $entityManager->createQuery( + 'UPDATE ' . ApplicationInstallation::class . ' ai SET ai.createdAt = :createdAt WHERE ai.id = :id' + ) + ->setParameter('createdAt', $createdAt) + ->setParameter('id', $installationId, 'uuid') + ->execute(); + } + } diff --git a/tests/Functional/ApplicationInstallations/UseCase/MarkOldInstallations/HandlerTest.php b/tests/Functional/ApplicationInstallations/UseCase/MarkAsNeedReinstall/HandlerTest.php similarity index 59% rename from tests/Functional/ApplicationInstallations/UseCase/MarkOldInstallations/HandlerTest.php rename to tests/Functional/ApplicationInstallations/UseCase/MarkAsNeedReinstall/HandlerTest.php index e152a5fc..930c24eb 100644 --- a/tests/Functional/ApplicationInstallations/UseCase/MarkOldInstallations/HandlerTest.php +++ b/tests/Functional/ApplicationInstallations/UseCase/MarkAsNeedReinstall/HandlerTest.php @@ -2,23 +2,23 @@ declare(strict_types=1); -namespace Bitrix24\Lib\Tests\Functional\ApplicationInstallations\UseCase\MarkOldInstallations; +namespace Bitrix24\Lib\Tests\Functional\ApplicationInstallations\UseCase\MarkAsNeedReinstall; use Bitrix24\Lib\ApplicationInstallations\Entity\ApplicationInstallation; use Bitrix24\Lib\ApplicationInstallations\Infrastructure\Doctrine\ApplicationInstallationRepository; -use Bitrix24\Lib\ApplicationInstallations\UseCase\MarkOldInstallations\Command; -use Bitrix24\Lib\ApplicationInstallations\UseCase\MarkOldInstallations\Handler; +use Bitrix24\Lib\ApplicationInstallations\UseCase\MarkAsNeedReinstall\Command; +use Bitrix24\Lib\ApplicationInstallations\UseCase\MarkAsNeedReinstall\Handler; use Bitrix24\Lib\Bitrix24Accounts\Entity\Bitrix24Account; -use Bitrix24\Lib\Bitrix24Accounts\Infrastructure\Doctrine\Bitrix24AccountRepository; use Bitrix24\Lib\Services\Flusher; use Bitrix24\Lib\Tests\EntityManagerFactory; use Bitrix24\SDK\Application\ApplicationStatus; use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Entity\ApplicationInstallationStatus; use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Events\ApplicationInstallationMarkedNeedReinstallEvent; +use Bitrix24\SDK\Application\Contracts\ApplicationInstallations\Exceptions\ApplicationInstallationNotFoundException; use Bitrix24\SDK\Application\PortalLicenseFamily; use Bitrix24\SDK\Core\Credentials\AuthToken; use Bitrix24\SDK\Core\Credentials\Scope; -use Carbon\CarbonImmutable; +use Bitrix24\SDK\Core\Exceptions\LogicException; use Doctrine\ORM\EntityManagerInterface; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\Test; @@ -58,58 +58,45 @@ protected function setUp(): void } #[Test] - public function testStaleInstallationIsMarkedAsNeedReinstall(): void + public function testPendingInstallationIsMarkedAsNeedReinstall(): void { - $bitrix24Account = $this->createAccount(); - $installation = $this->createInstallation($bitrix24Account->getId()); + $installation = $this->persistInstallation(); - $this->entityManager->persist($bitrix24Account); - $this->entityManager->persist($installation); - $this->entityManager->flush(); - - $this->backdateCreatedAt($installation->getId(), new CarbonImmutable('-2 hours')); - - $this->handler->handle(new Command(3600)); + $this->handler->handle(new Command($installation->getId(), 'installation timed out without ONAPPINSTALL')); $this->entityManager->clear(); $updated = $this->installationRepository->getById($installation->getId()); self::assertSame(ApplicationInstallationStatus::needReinstall, $updated->getStatus()); + self::assertSame('installation timed out without ONAPPINSTALL', $updated->getComment()); $events = $this->eventDispatcher->getOrphanedEvents(); self::assertContains(ApplicationInstallationMarkedNeedReinstallEvent::class, $events); } #[Test] - public function testFreshInstallationStaysNew(): void + public function testActiveInstallationThrowsLogicException(): void { - $bitrix24Account = $this->createAccount(); - $installation = $this->createInstallation($bitrix24Account->getId()); - - $this->entityManager->persist($bitrix24Account); - $this->entityManager->persist($installation); + $installation = $this->persistInstallation(); + $installation->applicationInstalled(Uuid::v7()->toRfc4122()); $this->entityManager->flush(); - $this->entityManager->clear(); - - $this->handler->handle(new Command(3600)); - $updated = $this->installationRepository->getById($installation->getId()); + $this->expectException(LogicException::class); - self::assertSame(ApplicationInstallationStatus::new, $updated->getStatus()); + $this->handler->handle(new Command($installation->getId(), 'installation timed out without ONAPPINSTALL')); } #[Test] - public function testNoStaleInstallationsDoesNothing(): void + public function testUnknownInstallationIdThrowsNotFoundException(): void { - $this->handler->handle(new Command(3600)); + $this->expectException(ApplicationInstallationNotFoundException::class); - $events = $this->eventDispatcher->getOrphanedEvents(); - self::assertNotContains(ApplicationInstallationMarkedNeedReinstallEvent::class, $events); + $this->handler->handle(new Command(Uuid::v7(), 'installation timed out without ONAPPINSTALL')); } - private function createAccount(): Bitrix24Account + private function persistInstallation(): ApplicationInstallation { - return new Bitrix24Account( + $bitrix24Account = new Bitrix24Account( Uuid::v7(), 1, true, @@ -120,13 +107,10 @@ private function createAccount(): Bitrix24Account new Scope(['crm']), true ); - } - private function createInstallation(Uuid $bitrix24AccountId): ApplicationInstallation - { - return new ApplicationInstallation( + $installation = new ApplicationInstallation( Uuid::v7(), - $bitrix24AccountId, + $bitrix24Account->getId(), new ApplicationStatus('F'), PortalLicenseFamily::free, 10, @@ -136,15 +120,11 @@ private function createInstallation(Uuid $bitrix24AccountId): ApplicationInstall 'lead-1', 'install' ); - } - private function backdateCreatedAt(Uuid $installationId, CarbonImmutable $createdAt): void - { - $this->entityManager->createQuery( - 'UPDATE ' . ApplicationInstallation::class . ' ai SET ai.createdAt = :createdAt WHERE ai.id = :id' - ) - ->setParameter('createdAt', $createdAt) - ->setParameter('id', $installationId, 'uuid') - ->execute(); + $this->entityManager->persist($bitrix24Account); + $this->entityManager->persist($installation); + $this->entityManager->flush(); + + return $installation; } } diff --git a/tests/Functional/ApplicationInstallations/UseCase/MarkOldInstallations/WorkflowTest.php b/tests/Functional/ApplicationInstallations/UseCase/MarkOldInstallations/WorkflowTest.php deleted file mode 100644 index 2fcc92da..00000000 --- a/tests/Functional/ApplicationInstallations/UseCase/MarkOldInstallations/WorkflowTest.php +++ /dev/null @@ -1,135 +0,0 @@ -entityManager = EntityManagerFactory::get(); - $this->eventDispatcher = new EventDispatcher(); - $this->installationRepository = new ApplicationInstallationRepository($this->entityManager); - - $handler = new Handler( - $this->installationRepository, - new Flusher($this->entityManager, $this->eventDispatcher), - new NullLogger() - ); - - $this->workflow = new Workflow( - $handler, - $this->eventDispatcher, - new NullLogger() - ); - } - - #[Test] - public function testFullFlowReturnsResultWithProcessedInstallations(): void - { - $bitrix24Account = $this->createAccount(); - $installation = $this->createInstallation($bitrix24Account->getId()); - - $this->entityManager->persist($bitrix24Account); - $this->entityManager->persist($installation); - $this->entityManager->flush(); - - $this->backdateCreatedAt($installation->getId(), new CarbonImmutable('-2 hours')); - - $result = $this->workflow->run(new MarkOldInstallationsConfig(3600)); - $this->entityManager->clear(); - - self::assertCount(1, $result->processedInstallations); - self::assertTrue( - $installation->getId()->equals($result->processedInstallations[0]->applicationInstallationId) - ); - - $updated = $this->installationRepository->getById($installation->getId()); - self::assertSame(ApplicationInstallationStatus::needReinstall, $updated->getStatus()); - } - - #[Test] - public function testNoStaleInstallationsReturnsEmptyResult(): void - { - $result = $this->workflow->run(new MarkOldInstallationsConfig(3600)); - - self::assertSame([], $result->processedInstallations); - } - - private function createAccount(): Bitrix24Account - { - return new Bitrix24Account( - Uuid::v7(), - 1, - true, - Uuid::v4()->toRfc4122(), - 'example.bitrix24.test', - new AuthToken('access', 'refresh', 3600, time() + 3600), - 1, - new Scope(['crm']), - true - ); - } - - private function createInstallation(Uuid $bitrix24AccountId): ApplicationInstallation - { - return new ApplicationInstallation( - Uuid::v7(), - $bitrix24AccountId, - new ApplicationStatus('F'), - PortalLicenseFamily::free, - 10, - null, - null, - null, - 'lead-1', - 'install' - ); - } - - private function backdateCreatedAt(Uuid $installationId, CarbonImmutable $createdAt): void - { - $this->entityManager->createQuery( - 'UPDATE ' . ApplicationInstallation::class . ' ai SET ai.createdAt = :createdAt WHERE ai.id = :id' - ) - ->setParameter('createdAt', $createdAt) - ->setParameter('id', $installationId, 'uuid') - ->execute(); - } -} diff --git a/tests/Unit/ApplicationInstallations/UseCase/MarkOldInstallations/ConfigTest.php b/tests/Unit/ApplicationInstallations/UseCase/MarkOldInstallations/ConfigTest.php deleted file mode 100644 index 2157e674..00000000 --- a/tests/Unit/ApplicationInstallations/UseCase/MarkOldInstallations/ConfigTest.php +++ /dev/null @@ -1,43 +0,0 @@ -ttlInSeconds); - } - - #[Test] - public function testNegativeTtlThrowsException(): void - { - $this->expectException(InvalidArgumentException::class); - - new MarkOldInstallationsConfig(-1); - } - - public static function validTtlProvider(): \Generator - { - yield 'zero' => [0]; - yield 'one_hour' => [3600]; - yield 'thirty_minutes' => [1800]; - } -}