From 1650c77c9ba3edb3fed7ee0924c991b5cc677cb4 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Mon, 9 Nov 2026 15:39:07 +0000 Subject: [PATCH 1/8] feat: expose signer and workflow transitions as native webhooks Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Events/SignerRejectedWebhookEvent.php | 64 +++++++++++++++++++ lib/Events/SignerSignedWebhookEvent.php | 62 ++++++++++++++++++ lib/Events/SigningRequestCanceledEvent.php | 48 ++++++++++++++ lib/Events/SigningRequestReadyEvent.php | 48 ++++++++++++++ lib/Service/FileStatusService.php | 16 +++++ lib/Service/RequestSignatureService.php | 10 ++- lib/Service/SignFileService.php | 6 ++ .../SignatureRejectionService.php | 43 +++++++++++++ .../Unit/Service/FileStatusServiceTest.php | 20 +++++- .../SignatureRejectionServiceTest.php | 45 +++++++++++++ 10 files changed, 359 insertions(+), 3 deletions(-) create mode 100644 lib/Events/SignerRejectedWebhookEvent.php create mode 100644 lib/Events/SignerSignedWebhookEvent.php create mode 100644 lib/Events/SigningRequestCanceledEvent.php create mode 100644 lib/Events/SigningRequestReadyEvent.php diff --git a/lib/Events/SignerRejectedWebhookEvent.php b/lib/Events/SignerRejectedWebhookEvent.php new file mode 100644 index 0000000000..b685c8e9fc --- /dev/null +++ b/lib/Events/SignerRejectedWebhookEvent.php @@ -0,0 +1,64 @@ +getUuid() === '' || $document->getUuid() === '' + || $signer->getId() === null + || $signer->getStatusEnum() !== SignRequestStatus::REJECTED + || $signer->getRejectedAt() === null) { + return null; + } + + $identity = implode(':', [$root->getUuid(), $document->getUuid(), $signer->getId(), $signer->getRejectedAt()->format('U.u')]); + return new self( + $root->getUuid(), + $root->isEnvelope() ? 'envelope' : 'document', + $document->getUuid(), + $signer->getId(), + $workflowCanceled, + 'libresign.signer.rejected.v1:' . hash('sha256', $identity), + ); + } + + #[\Override] + public function getWebhookSerializable(): array { + return [ + 'contractVersion' => self::CONTRACT_VERSION, + 'eventId' => $this->eventId, + 'requestUuid' => $this->requestUuid, + 'requestType' => $this->requestType, + 'documentUuid' => $this->documentUuid, + 'signerId' => $this->signerId, + 'status' => 'rejected', + 'workflowCanceled' => $this->workflowCanceled, + ]; + } +} diff --git a/lib/Events/SignerSignedWebhookEvent.php b/lib/Events/SignerSignedWebhookEvent.php new file mode 100644 index 0000000000..2925082c8a --- /dev/null +++ b/lib/Events/SignerSignedWebhookEvent.php @@ -0,0 +1,62 @@ +getUuid() === '' || $document->getUuid() === '' + || $signer->getId() === null || $signer->getSigned() === null + || !is_string($signer->getSignedHash()) || $signer->getSignedHash() === '') { + return null; + } + + $identity = implode(':', [$root->getUuid(), $document->getUuid(), $signer->getId(), $signer->getSignedHash()]); + return new self( + $root->getUuid(), + $root->isEnvelope() ? 'envelope' : 'document', + $document->getUuid(), + $signer->getId(), + 'libresign.signer.signed.v1:' . hash('sha256', $identity), + ); + } + + #[\Override] + public function getWebhookSerializable(): array { + return [ + 'contractVersion' => self::CONTRACT_VERSION, + 'eventId' => $this->eventId, + 'requestUuid' => $this->requestUuid, + 'requestType' => $this->requestType, + 'documentUuid' => $this->documentUuid, + 'signerId' => $this->signerId, + 'status' => 'signed', + ]; + } +} diff --git a/lib/Events/SigningRequestCanceledEvent.php b/lib/Events/SigningRequestCanceledEvent.php new file mode 100644 index 0000000000..ac2b3eb416 --- /dev/null +++ b/lib/Events/SigningRequestCanceledEvent.php @@ -0,0 +1,48 @@ +hasParent() + || $previousStatus === FileStatus::CANCELED->value + || $file->getStatus() !== FileStatus::CANCELED->value + || $file->getUuid() === '') { + return null; + } + + return new self($file->getUuid(), $file->isEnvelope() ? 'envelope' : 'document'); + } + + #[\Override] + public function getWebhookSerializable(): array { + return [ + 'contractVersion' => self::CONTRACT_VERSION, + 'eventId' => 'libresign.workflow.canceled.v1:' . $this->requestUuid, + 'requestUuid' => $this->requestUuid, + 'requestType' => $this->requestType, + 'status' => 'canceled', + ]; + } +} diff --git a/lib/Events/SigningRequestReadyEvent.php b/lib/Events/SigningRequestReadyEvent.php new file mode 100644 index 0000000000..5a184f4262 --- /dev/null +++ b/lib/Events/SigningRequestReadyEvent.php @@ -0,0 +1,48 @@ +hasParent() + || $previousStatus !== FileStatus::DRAFT->value + || $file->getStatus() !== FileStatus::ABLE_TO_SIGN->value + || $file->getUuid() === '') { + return null; + } + + return new self($file->getUuid(), $file->isEnvelope() ? 'envelope' : 'document'); + } + + #[\Override] + public function getWebhookSerializable(): array { + return [ + 'contractVersion' => self::CONTRACT_VERSION, + 'eventId' => 'libresign.workflow.ready.v1:' . $this->requestUuid, + 'requestUuid' => $this->requestUuid, + 'requestType' => $this->requestType, + 'status' => 'ready_to_sign', + ]; + } +} diff --git a/lib/Service/FileStatusService.php b/lib/Service/FileStatusService.php index b95bca2457..832bdd224b 100644 --- a/lib/Service/FileStatusService.php +++ b/lib/Service/FileStatusService.php @@ -13,8 +13,10 @@ use OCA\Libresign\Db\File as FileEntity; use OCA\Libresign\Db\FileMapper; use OCA\Libresign\Enum\FileStatus; +use OCA\Libresign\Events\SigningRequestReadyEvent; use OCA\Libresign\Service\SignRequest\StatusCacheService; use OCP\AppFramework\Db\DoesNotExistException; +use OCP\EventDispatcher\IEventDispatcher; class FileStatusService { public const POLICY_SNAPSHOT_FROZEN_AT = 'policy_snapshot_frozen_at'; @@ -22,6 +24,7 @@ class FileStatusService { public function __construct( private FileMapper $fileMapper, private StatusCacheService $statusCacheService, + private IEventDispatcher $eventDispatcher, ) { } @@ -33,6 +36,7 @@ public function updateFileStatusIfUpgrade(FileEntity $file, int $newStatus): Fil $this->touchStatusChangedAt($file); $this->fileMapper->update($file); $this->statusCacheService->setStatus($file->getUuid(), $newStatus); + $this->dispatchReadyTransition($file, $currentStatus); if ($file->hasParent()) { $this->propagateStatusToParent($file->getParentFileId()); @@ -42,6 +46,18 @@ public function updateFileStatusIfUpgrade(FileEntity $file, int $newStatus): Fil return $file; } + /** Called after creating a ready request and persisting its signers. */ + public function dispatchCreatedReadyRequest(FileEntity $file): void { + $this->dispatchReadyTransition($file, FileStatus::DRAFT->value); + } + + private function dispatchReadyTransition(FileEntity $file, int $previousStatus): void { + $event = SigningRequestReadyEvent::fromTransition($file, $previousStatus); + if ($event !== null) { + $this->eventDispatcher->dispatchTyped($event); + } + } + public function invalidateCachedStatus(FileEntity $file): void { $this->statusCacheService->invalidateStatus($file->getUuid()); } diff --git a/lib/Service/RequestSignatureService.php b/lib/Service/RequestSignatureService.php index 35009a3fe4..5e1f5df258 100644 --- a/lib/Service/RequestSignatureService.php +++ b/lib/Service/RequestSignatureService.php @@ -139,7 +139,8 @@ public function saveFiles(array $data): array { } public function save(array $data): FileEntity { - $file = $this->saveFile($data); + $newlyCreated = false; + $file = $this->saveFile($data, $newlyCreated); if (!isset($data['status'])) { $data['status'] = $file->getStatus(); } @@ -149,6 +150,9 @@ public function save(array $data): FileEntity { $this->propagateSignersToChildren($file, $data); $this->saveVisibleElements($data, $file); }, $this->db); + if ($newlyCreated) { + $this->fileStatusService->dispatchCreatedReadyRequest($file); + } $this->notifications->flushCommitted(); return $file; @@ -363,7 +367,8 @@ private function createFileForEnvelope( * uploadedFile?: array * } $data */ - public function saveFile(array $data): FileEntity { + public function saveFile(array $data, ?bool &$newlyCreated = null): FileEntity { + $newlyCreated = false; if (!empty($data['uuid'])) { $file = $this->fileMapper->getByUuid($data['uuid']); $this->filePolicyApplier->syncCoreFlowPolicies($file, $data); @@ -433,6 +438,7 @@ public function saveFile(array $data): FileEntity { $this->filePolicyApplier->applyAll($file, $data); $this->fileMapper->insert($file); + $newlyCreated = true; return $file; } diff --git a/lib/Service/SignFileService.php b/lib/Service/SignFileService.php index a230d1a5df..7a7395955d 100644 --- a/lib/Service/SignFileService.php +++ b/lib/Service/SignFileService.php @@ -31,6 +31,7 @@ use OCA\Libresign\Enum\IdentifyMethodRequirement; use OCA\Libresign\Enum\SignRequestStatus; use OCA\Libresign\Events\SignedEventFactory; +use OCA\Libresign\Events\SignerSignedWebhookEvent; use OCA\Libresign\Events\SigningRequestCompletedEvent; use OCA\Libresign\Exception\FooterStampUnavailableException; use OCA\Libresign\Exception\LibresignException; @@ -1018,6 +1019,11 @@ protected function dispatchSignedEvent(): void { $certificateSerialNumber, ); $this->eventDispatcher->dispatchTyped($event); + $root = $this->getCompletionTarget($this->libreSignFile); + $publicEvent = SignerSignedWebhookEvent::afterSigning($root, $this->libreSignFile, $this->signRequest); + if ($publicEvent !== null) { + $this->eventDispatcher->dispatchTyped($publicEvent); + } } protected function identifyEngine(File $file): ISignEngineHandler { diff --git a/lib/Service/SignatureRejection/SignatureRejectionService.php b/lib/Service/SignatureRejection/SignatureRejectionService.php index b5de4b2282..70f086a14a 100644 --- a/lib/Service/SignatureRejection/SignatureRejectionService.php +++ b/lib/Service/SignatureRejection/SignatureRejectionService.php @@ -16,6 +16,9 @@ use OCA\Libresign\Enum\SignatureRejectionCommentMode; use OCA\Libresign\Enum\SignRequestStatus; use OCA\Libresign\Events\SignatureRejectedEvent; +use OCA\Libresign\Events\SignerRejectedWebhookEvent; +use OCA\Libresign\Events\SigningRequestCanceledEvent; +use OCA\Libresign\Events\SigningRequestCompletedEvent; use OCA\Libresign\Exception\LibresignException; use OCA\Libresign\Service\File\SigningWorkflowStatusResolver; use OCA\Libresign\Service\FileStatusService; @@ -77,7 +80,12 @@ public function reject( // always the signer's call and no policy can take that choice away. $commentIsPrivate = $normalizedComment !== null && $privateComment; + $root = $libreSignFile->hasParent() + ? $this->fileMapper->getById($libreSignFile->getParentFileId()) + : $libreSignFile; + $previousRootStatus = $root->getStatus(); $activatedSigners = $this->persistRejection($libreSignFile, $signRequest, $normalizedComment, $commentIsPrivate, $config->cancelsWorkflow()); + $this->dispatchPublicRejectionEvents($libreSignFile, $signRequest, $previousRootStatus); // Record the committed rejection before notification errors can interrupt this flow. // The event reports the status the rejection left, as the API does: a workflow that // continues after rejections is still canceled once every signer rejected. @@ -357,6 +365,41 @@ private function normalizeComment(?string $comment, SignatureRejectionCommentMod return $trimmed; } + private function dispatchPublicRejectionEvents( + FileEntity $document, + SignRequestEntity $signer, + int $previousRootStatus, + ): void { + // Signing-state transactions have committed. A webhook scheduling failure + // must not undo a persisted rejection. + try { + $root = $document->hasParent() + ? $this->fileMapper->getById($document->getParentFileId()) + : $document; + $rejected = SignerRejectedWebhookEvent::afterRejection( + $root, $document, $signer, $this->isWorkflowCanceled($root), + ); + if ($rejected !== null) { + $this->eventDispatcher->dispatchTyped($rejected); + } + $canceled = SigningRequestCanceledEvent::fromTransition($root, $previousRootStatus); + if ($canceled !== null) { + $this->eventDispatcher->dispatchTyped($canceled); + } + // Under a continue-on-rejection policy, the final rejection may + // finish a workflow that already contains a valid signature. + $completed = SigningRequestCompletedEvent::fromTransition($root, $previousRootStatus); + if ($completed !== null) { + $this->eventDispatcher->dispatchTyped($completed); + } + } catch (\Throwable $e) { + $this->logger->error('Unable to enqueue LibreSign rejection webhook', [ + 'exception' => $e, + 'fileId' => $document->getId(), + ]); + } + } + private function dispatchRejectedEvent( SignRequestEntity $signRequest, FileEntity $libreSignFile, diff --git a/tests/php/Unit/Service/FileStatusServiceTest.php b/tests/php/Unit/Service/FileStatusServiceTest.php index 6bbe403d11..a8bbcac8a6 100644 --- a/tests/php/Unit/Service/FileStatusServiceTest.php +++ b/tests/php/Unit/Service/FileStatusServiceTest.php @@ -12,8 +12,10 @@ use OCA\Libresign\Db\File as FileEntity; use OCA\Libresign\Db\FileMapper; use OCA\Libresign\Enum\FileStatus; +use OCA\Libresign\Events\SigningRequestReadyEvent; use OCA\Libresign\Service\FileStatusService; use OCA\Libresign\Service\SignRequest\StatusCacheService; +use OCP\EventDispatcher\IEventDispatcher; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; @@ -22,11 +24,13 @@ class FileStatusServiceTest extends TestCase { private FileMapper|MockObject $fileMapper; private StatusCacheService|MockObject $statusCacheService; private FileStatusService $service; + private IEventDispatcher&MockObject $eventDispatcher; protected function setUp(): void { $this->fileMapper = $this->createMock(FileMapper::class); $this->statusCacheService = $this->createMock(StatusCacheService::class); - $this->service = new FileStatusService($this->fileMapper, $this->statusCacheService); + $this->eventDispatcher = $this->createMock(IEventDispatcher::class); + $this->service = new FileStatusService($this->fileMapper, $this->statusCacheService, $this->eventDispatcher); } public function testQueuedSigningStatusIsNotPublishedBeforeCommit(): void { @@ -49,6 +53,20 @@ public function testCommittedEnqueueDoesNotOverwriteNewerSigningProgress(): void $this->service->onQueuedJobCommitted($file); } + public function testReadyWebhookIsEmittedOnceForRootDraftTransition(): void { + $file = new FileEntity(); + $file->setId(11); + $file->setUuid('ready-uuid'); + $file->setStatus(FileStatus::DRAFT->value); + $this->eventDispatcher->expects($this->once())->method('dispatchTyped') + ->with($this->callback(static function ($event): bool { + return $event instanceof SigningRequestReadyEvent + && $event->getWebhookSerializable()['status'] === 'ready_to_sign'; + })); + $this->service->updateFileStatusIfUpgrade($file, FileStatus::ABLE_TO_SIGN->value); + $this->service->updateFileStatusIfUpgrade($file, FileStatus::ABLE_TO_SIGN->value); + } + #[DataProvider('dataFileStatusUpgrade')] public function testUpdateFileStatusIfUpgrade(int $currentStatus, int $newStatus, bool $shouldUpdate): void { $file = new FileEntity(); diff --git a/tests/php/Unit/Service/SignatureRejection/SignatureRejectionServiceTest.php b/tests/php/Unit/Service/SignatureRejection/SignatureRejectionServiceTest.php index 6d34ac914a..74c66c3533 100644 --- a/tests/php/Unit/Service/SignatureRejection/SignatureRejectionServiceTest.php +++ b/tests/php/Unit/Service/SignatureRejection/SignatureRejectionServiceTest.php @@ -15,6 +15,9 @@ use OCA\Libresign\Enum\FileStatus; use OCA\Libresign\Enum\SignRequestStatus; use OCA\Libresign\Events\SignatureRejectedEvent; +use OCA\Libresign\Events\SignerRejectedWebhookEvent; +use OCA\Libresign\Events\SigningRequestCanceledEvent; +use OCA\Libresign\Events\SigningRequestCompletedEvent; use OCA\Libresign\Exception\LibresignException; use OCA\Libresign\Service\File\SigningWorkflowStatusResolver; use OCA\Libresign\Service\FileStatusService; @@ -285,6 +288,48 @@ public function testPrivateFlagIsIgnoredWhenThereIsNoComment(): void { $this->assertFalse($signRequest->getRejectionCommentPrivate()); } + #[DataProvider('publicRejectionOutcomes')] + public function testCommittedRejectionEmitsOnePublicBusinessEvent(bool $cancel): void { + $this->withPolicy(self::policy($cancel ? 'cancel' : 'continue', 'optional')); + $file = $this->file(); + $file->setUuid('request-uuid'); + $signer = $this->signRequest(); + $signer->setUuid('secret-invitation'); + $events = []; + $this->eventDispatcher->method('dispatchTyped')->willReturnCallback(static function ($event) use (&$events): void { + $events[] = $event; + }); + $this->getService()->reject($file, $signer, 'private explanation', true); + $rejections = array_values(array_filter($events, static fn ($event): bool => $event instanceof SignerRejectedWebhookEvent)); + $cancellations = array_values(array_filter($events, static fn ($event): bool => $event instanceof SigningRequestCanceledEvent)); + $this->assertCount(1, $rejections); + $this->assertCount($cancel ? 1 : 0, $cancellations); + $this->assertSame($cancel, $rejections[0]->getWebhookSerializable()['workflowCanceled']); + $this->assertStringNotContainsString('private explanation', json_encode($rejections[0]->getWebhookSerializable())); + } + + /** @return array */ + public static function publicRejectionOutcomes(): array { + return [ + 'continue' => [false], + 'cancel' => [true], + ]; + } + + public function testRejectionCanCompletePreviouslySignedWorkflow(): void { + $this->withPolicy(self::policy('continue')); + $file = $this->file(FileStatus::PARTIAL_SIGNED->value); + $file->setUuid('completed-by-rejection'); + $this->signingWorkflowStatusResolver->method('resolve')->willReturn(FileStatus::SIGNED); + $events = []; + $this->eventDispatcher->method('dispatchTyped')->willReturnCallback(static function ($event) use (&$events): void { + $events[] = $event; + }); + $this->getService()->reject($file, $this->signRequest()); + $this->assertSame(FileStatus::SIGNED->value, $file->getStatus()); + $this->assertCount(1, array_filter($events, static fn ($event): bool => $event instanceof SigningRequestCompletedEvent)); + } + public function testRejectionIsPersistedWithItsTimestampAndComment(): void { $this->withPolicy(self::policy(commentMode: 'optional')); $signRequest = $this->signRequest(); From 75a368c8be367dc58b1e2d4e500f3a5b4523f49c Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Mon, 9 Nov 2026 15:39:23 +0000 Subject: [PATCH 2/8] feat: publish persisted notification channel outcomes via webhook Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- ...NotificationDeliveryStatusChangedEvent.php | 65 +++++++ .../NotificationSubmissionService.php | 27 +++ .../Unit/Events/NativeWebhookEventsTest.php | 177 ++++++++++++++++++ .../NotificationSubmissionServiceTest.php | 4 +- .../NotificationDeliveryServiceTest.php | 4 + 5 files changed, 276 insertions(+), 1 deletion(-) create mode 100644 lib/Events/NotificationDeliveryStatusChangedEvent.php create mode 100644 tests/php/Unit/Events/NativeWebhookEventsTest.php diff --git a/lib/Events/NotificationDeliveryStatusChangedEvent.php b/lib/Events/NotificationDeliveryStatusChangedEvent.php new file mode 100644 index 0000000000..3f0393d395 --- /dev/null +++ b/lib/Events/NotificationDeliveryStatusChangedEvent.php @@ -0,0 +1,65 @@ +getState()); + if ($root->getUuid() === '' || $delivery->getId() === null || $delivery->getAttemptCount() < 1 + || $state === null + || in_array($state, [NotificationDeliveryState::PENDING, NotificationDeliveryState::SENDING], true)) { + return null; + } + + return new self( + $root->getUuid(), + $root->isEnvelope() ? 'envelope' : 'document', + $delivery->getId(), + $delivery->getChannel(), + $state->value, + $delivery->getAttemptCount(), + 'libresign.notification.delivery.v1:' . $delivery->getId() . ':' . $delivery->getAttemptCount() . ':' . $state->value, + ); + } + + #[\Override] + public function getWebhookSerializable(): array { + return [ + 'contractVersion' => self::CONTRACT_VERSION, + 'eventId' => $this->eventId, + 'requestUuid' => $this->requestUuid, + 'requestType' => $this->requestType, + 'deliveryId' => $this->deliveryId, + 'channel' => $this->channel, + 'state' => $this->state, + 'attempt' => $this->attempt, + ]; + } +} diff --git a/lib/Service/Notification/NotificationSubmissionService.php b/lib/Service/Notification/NotificationSubmissionService.php index d9702450c0..a08f4072ea 100644 --- a/lib/Service/Notification/NotificationSubmissionService.php +++ b/lib/Service/Notification/NotificationSubmissionService.php @@ -8,12 +8,15 @@ namespace OCA\Libresign\Service\Notification; +use OCA\Libresign\Db\FileMapper; use OCA\Libresign\Db\NotificationAttemptMapper; use OCA\Libresign\Db\NotificationDelivery; use OCA\Libresign\Db\NotificationDeliveryMapper; use OCA\Libresign\Enum\NotificationDeliveryState; +use OCA\Libresign\Events\NotificationDeliveryStatusChangedEvent; use OCA\Libresign\Exception\NotificationPreparationException; use OCP\AppFramework\Db\TTransactional; +use OCP\EventDispatcher\IEventDispatcher; use OCP\IDBConnection; use Psr\Log\LoggerInterface; @@ -29,6 +32,8 @@ public function __construct( private NotificationFailureClassifier $failureClassifier, private NotificationHistoryService $history, private LoggerInterface $logger, + private FileMapper $fileMapper, + private IEventDispatcher $eventDispatcher, ) { } @@ -90,7 +95,29 @@ public function submit(NotificationDelivery $delivery, NotificationMessage $mess $this->attempts->finish($attempt->getId(), $outcome, $reason); $this->deliveries->finish($delivery->getId(), $outcome, $retryAt); }, $this->db); + $this->dispatchDeliveryStatusWebhook($delivery); } return true; } + + private function dispatchDeliveryStatusWebhook(NotificationDelivery $delivery): void { + // A webhook failure must not mask a persisted provider result or its original error. + try { + $persisted = $this->deliveries->findById($delivery->getId()); + $file = $this->fileMapper->getById($persisted->getFileId()); + $root = $file->hasParent() + ? $this->fileMapper->getById($file->getParentFileId()) + : $file; + $event = NotificationDeliveryStatusChangedEvent::fromPersistedDelivery($root, $persisted); + if ($event !== null) { + $this->eventDispatcher->dispatchTyped($event); + } + } catch (\Throwable $e) { + $this->logger->warning('Unable to enqueue notification outcome webhook', [ + 'exception' => $e, + 'deliveryId' => $delivery->getId(), + ]); + } + } + } diff --git a/tests/php/Unit/Events/NativeWebhookEventsTest.php b/tests/php/Unit/Events/NativeWebhookEventsTest.php new file mode 100644 index 0000000000..1998f504c2 --- /dev/null +++ b/tests/php/Unit/Events/NativeWebhookEventsTest.php @@ -0,0 +1,177 @@ +setUuid($uuid); + $file->setNodeType($envelope ? 'envelope' : 'file'); + return $file; + } + + /** @return array */ + public static function readyTransitions(): array { + return [ + 'draft to ready' => [FileStatus::DRAFT, FileStatus::ABLE_TO_SIGN, false, true], + 'repeated ready' => [FileStatus::ABLE_TO_SIGN, FileStatus::ABLE_TO_SIGN, false, false], + 'draft to signed' => [FileStatus::DRAFT, FileStatus::SIGNED, false, false], + 'child draft to ready' => [FileStatus::DRAFT, FileStatus::ABLE_TO_SIGN, true, false], + ]; + } + + #[DataProvider('readyTransitions')] + public function testReadyRequiresRealRootTransition(FileStatus $previous, FileStatus $next, bool $child, bool $emitted): void { + $file = self::file(); + $file->setStatus($next->value); + if ($child) { + $file->setParentFileId(42); + } + $event = SigningRequestReadyEvent::fromTransition($file, $previous->value); + if (!$emitted) { + self::assertNull($event); + return; + } + self::assertInstanceOf(IWebhookCompatibleEvent::class, $event); + self::assertSame([ + 'contractVersion' => 1, + 'eventId' => 'libresign.workflow.ready.v1:workflow-uuid', + 'requestUuid' => 'workflow-uuid', + 'requestType' => 'document', + 'status' => 'ready_to_sign', + ], $event->getWebhookSerializable()); + } + + /** @return array */ + public static function canceledTransitions(): array { + return [ + 'ready to canceled' => [FileStatus::ABLE_TO_SIGN, FileStatus::CANCELED, false, true], + 'already canceled' => [FileStatus::CANCELED, FileStatus::CANCELED, false, false], + 'not canceled' => [FileStatus::ABLE_TO_SIGN, FileStatus::PARTIAL_SIGNED, false, false], + 'canceled envelope child' => [FileStatus::ABLE_TO_SIGN, FileStatus::CANCELED, true, false], + ]; + } + + #[DataProvider('canceledTransitions')] + public function testCanceledRequiresAggregateTransition(FileStatus $previous, FileStatus $next, bool $child, bool $emitted): void { + $file = self::file('envelope-uuid', true); + $file->setStatus($next->value); + if ($child) { + $file->setParentFileId(42); + } + $event = SigningRequestCanceledEvent::fromTransition($file, $previous->value); + if (!$emitted) { + self::assertNull($event); + return; + } + self::assertInstanceOf(IWebhookCompatibleEvent::class, $event); + self::assertSame([ + 'contractVersion' => 1, + 'eventId' => 'libresign.workflow.canceled.v1:envelope-uuid', + 'requestUuid' => 'envelope-uuid', + 'requestType' => 'envelope', + 'status' => 'canceled', + ], $event->getWebhookSerializable()); + } + + public function testSignerSignedHasStableNonSecretCorrelation(): void { + $root = self::file('envelope-uuid', true); + $document = self::file('child-uuid'); + $document->setParentFileId(42); + $signer = new SignRequest(); + $signer->setId(52); + $signer->setUuid('SECRET-INVITATION-UUID'); + $signer->setSigned(new \DateTime('2026-11-09T11:00:00Z')); + $signer->setSignedHash('content-hash'); + $event = SignerSignedWebhookEvent::afterSigning($root, $document, $signer); + self::assertInstanceOf(IWebhookCompatibleEvent::class, $event); + $payload = $event->getWebhookSerializable(); + self::assertSame('envelope', $payload['requestType']); + self::assertSame('envelope-uuid', $payload['requestUuid']); + self::assertSame('child-uuid', $payload['documentUuid']); + self::assertSame(52, $payload['signerId']); + self::assertSame('signed', $payload['status']); + self::assertSame($payload, SignerSignedWebhookEvent::afterSigning($root, $document, $signer)->getWebhookSerializable()); + self::assertStringNotContainsString('SECRET-INVITATION-UUID', json_encode($payload)); + $signer->setSignedHash(null); + self::assertNull(SignerSignedWebhookEvent::afterSigning($root, $document, $signer)); + } + + public function testRejectedEventDoesNotDiscloseCommentsOrInvitation(): void { + $root = self::file('workflow-uuid'); + $signer = new SignRequest(); + $signer->setId(9); + $signer->setUuid('SECRET-INVITATION'); + $signer->setRejectedAt(new \DateTime('2026-11-09T11:00:00Z')); + $signer->setStatusEnum(SignRequestStatus::REJECTED); + $signer->setRejectionComment('PRIVATE CASE DETAILS'); + $signer->setRejectionCommentPrivate(true); + $event = SignerRejectedWebhookEvent::afterRejection($root, $root, $signer, true); + self::assertInstanceOf(IWebhookCompatibleEvent::class, $event); + $p = $event->getWebhookSerializable(); + self::assertSame(true, $p['workflowCanceled']); + self::assertSame('rejected', $p['status']); + self::assertSame(9, $p['signerId']); + self::assertSame($p, SignerRejectedWebhookEvent::afterRejection($root, $root, $signer, true)->getWebhookSerializable()); + self::assertStringNotContainsString('PRIVATE CASE DETAILS', json_encode($p)); + self::assertStringNotContainsString('SECRET-INVITATION', json_encode($p)); + $signer->setStatusEnum(SignRequestStatus::SIGNED); + self::assertNull(SignerRejectedWebhookEvent::afterRejection($root, $root, $signer, false)); + } + + /** @return array */ + public static function persistedDeliveryOutcomes(): array { + return [ + 'pending' => [NotificationDeliveryState::PENDING, 0, false], + 'claimed' => [NotificationDeliveryState::SENDING, 1, false], + 'accepted by adapter' => [NotificationDeliveryState::ACCEPTED, 1, true], + 'unknown result' => [NotificationDeliveryState::UNKNOWN, 2, true], + 'failed' => [NotificationDeliveryState::FAILED, 2, true], + 'temporary failure' => [NotificationDeliveryState::TEMPORARY_FAILURE, 2, true], + ]; + } + + #[DataProvider('persistedDeliveryOutcomes')] + public function testDeliveryReportsOnlyPersistedAttemptOutcomes(NotificationDeliveryState $state, int $attempt, bool $emitted): void { + $delivery = new NotificationDelivery(); + $delivery->setId(15); + $delivery->setEventKey(str_repeat('a', 64)); + $delivery->setRecipientEnc('ENCRYPTED_RECIPIENT'); + $delivery->setChannel('mail'); + $delivery->setState($state->value); + $delivery->setAttemptCount($attempt); + $event = NotificationDeliveryStatusChangedEvent::fromPersistedDelivery(self::file(), $delivery); + if (!$emitted) { + self::assertNull($event); + return; + } + self::assertInstanceOf(IWebhookCompatibleEvent::class, $event); + $p = $event->getWebhookSerializable(); + self::assertSame($state->value, $p['state']); + self::assertSame($attempt, $p['attempt']); + self::assertSame(15, $p['deliveryId']); + self::assertSame('mail', $p['channel']); + self::assertStringNotContainsString('ENCRYPTED_RECIPIENT', json_encode($p)); + } +} diff --git a/tests/php/Unit/Service/Notification/NotificationSubmissionServiceTest.php b/tests/php/Unit/Service/Notification/NotificationSubmissionServiceTest.php index 32c82fc91d..be5d824d45 100644 --- a/tests/php/Unit/Service/Notification/NotificationSubmissionServiceTest.php +++ b/tests/php/Unit/Service/Notification/NotificationSubmissionServiceTest.php @@ -9,6 +9,7 @@ namespace OCA\Libresign\Tests\Unit\Service\Notification; use OCA\Libresign\Db\File; +use OCA\Libresign\Db\FileMapper; use OCA\Libresign\Db\NotificationAttempt; use OCA\Libresign\Db\NotificationAttemptMapper; use OCA\Libresign\Db\NotificationDelivery; @@ -24,6 +25,7 @@ use OCA\Libresign\Service\Notification\NotificationHistoryService; use OCA\Libresign\Service\Notification\NotificationMessage; use OCA\Libresign\Service\Notification\NotificationSubmissionService; +use OCP\EventDispatcher\IEventDispatcher; use OCP\IDBConnection; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; @@ -65,7 +67,7 @@ static function (?\DateTime $due) use ($expectRetry, $attemptNumber): bool { $db->expects($this->never())->method('rollBack'); $channel = $this->createMock(NotificationChannel::class); $channel->method('deliver')->willThrowException($failure); - $service = new NotificationSubmissionService($deliveries, $attempts, $db, new NotificationFailureClassifier(), new NotificationHistoryService($this->createMock(SignRequestMapper::class)), $this->createMock(LoggerInterface::class)); + $service = new NotificationSubmissionService($deliveries, $attempts, $db, new NotificationFailureClassifier(), new NotificationHistoryService($this->createMock(SignRequestMapper::class)), $this->createMock(LoggerInterface::class), $this->createMock(FileMapper::class), $this->createMock(IEventDispatcher::class)); $this->expectException(get_class($failure)); $service->submit($delivery, $this->message(), $channel); } diff --git a/tests/php/Unit/Service/NotificationDeliveryServiceTest.php b/tests/php/Unit/Service/NotificationDeliveryServiceTest.php index fbd8fae78d..e0af462066 100644 --- a/tests/php/Unit/Service/NotificationDeliveryServiceTest.php +++ b/tests/php/Unit/Service/NotificationDeliveryServiceTest.php @@ -10,6 +10,7 @@ use OCA\Libresign\BackgroundJob\DeliverNotification; use OCA\Libresign\Db\File; +use OCA\Libresign\Db\FileMapper; use OCA\Libresign\Db\NotificationAttempt; use OCA\Libresign\Db\NotificationAttemptMapper; use OCA\Libresign\Db\NotificationDelivery; @@ -31,6 +32,7 @@ use OCA\Libresign\Service\NotificationDeliveryService; use OCA\Libresign\Service\Worker\WorkerActivationService; use OCP\BackgroundJob\IJobList; +use OCP\EventDispatcher\IEventDispatcher; use OCP\IConfig; use OCP\IDBConnection; use OCP\Notification\IManager; @@ -79,6 +81,8 @@ protected function setUp(): void { new NotificationFailureClassifier(), new NotificationHistoryService($this->createMock(\OCA\Libresign\Db\SignRequestMapper::class)), $this->createMock(LoggerInterface::class), + $this->createMock(FileMapper::class), + $this->createMock(IEventDispatcher::class), ), new NotificationEnvelopeCodec($handoffCrypto, $this->createMock(IManager::class)), $this->mode, From 2b7705b860cd9f71a3ad8f941bd1918171cf0c47 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Mon, 9 Nov 2026 15:45:39 +0000 Subject: [PATCH 3/8] test: verify notification outcome event follows persisted submission Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../NotificationSubmissionServiceTest.php | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/tests/php/Unit/Service/Notification/NotificationSubmissionServiceTest.php b/tests/php/Unit/Service/Notification/NotificationSubmissionServiceTest.php index be5d824d45..5ffae31b82 100644 --- a/tests/php/Unit/Service/Notification/NotificationSubmissionServiceTest.php +++ b/tests/php/Unit/Service/Notification/NotificationSubmissionServiceTest.php @@ -17,6 +17,7 @@ use OCA\Libresign\Db\SignRequest; use OCA\Libresign\Db\SignRequestMapper; use OCA\Libresign\Enum\NotificationDeliveryState; +use OCA\Libresign\Events\NotificationDeliveryStatusChangedEvent; use OCA\Libresign\Exception\NotificationPreparationException; use OCA\Libresign\Exception\NotificationTransportException; use OCA\Libresign\Service\Notification\MailPayload; @@ -105,4 +106,55 @@ public function testGenericTransportWrapperCannotMakeNestedPreparationSafe(): vo public function testNestedPreparationFailureCannotMakeTransportOutcomeSafe(): void { $this->executeFailure(new NotificationTransportException('synthetic uncertain outcome', NotificationDeliveryState::UNKNOWN, 'smtp_outcome_unknown', new NotificationPreparationException('preparation')), 1, NotificationDeliveryState::UNKNOWN, false); } + + public function testAcceptedOutcomeWebhookIsDispatchedOnlyAfterLedgerCommit(): void { + $delivery = new NotificationDelivery(); + $delivery->setId(9); + $delivery->setFileId(25); + $delivery->setChannel('mail'); + $delivery->setAttemptCount(1); + $persisted = new NotificationDelivery(); + $persisted->setId(9); + $persisted->setFileId(25); + $persisted->setChannel('mail'); + $persisted->setAttemptCount(1); + $persisted->setState(NotificationDeliveryState::ACCEPTED->value); + $attempt = new NotificationAttempt(); + $attempt->setId(28); + + $deliveries = $this->createMock(NotificationDeliveryMapper::class); + $deliveries->method('claim')->with(9)->willReturn(true); + $deliveries->method('findById')->with(9)->willReturn($persisted); + $deliveries->expects($this->once())->method('finish')->with(9, NotificationDeliveryState::ACCEPTED, null); + $attempts = $this->createMock(NotificationAttemptMapper::class); + $attempts->method('begin')->willReturn($attempt); + $attempts->expects($this->once())->method('finish')->with(28, NotificationDeliveryState::ACCEPTED, null); + $db = $this->createMock(IDBConnection::class); + $committed = 0; + $db->expects($this->exactly(2))->method('commit')->willReturnCallback(static function () use (&$committed): void { + $committed++; + }); + $root = new File(); + $root->setUuid('public-workflow-uuid'); + $files = $this->createMock(FileMapper::class); + $files->method('getById')->with(25)->willReturn($root); + $dispatcher = $this->createMock(IEventDispatcher::class); + $dispatcher->expects($this->once())->method('dispatchTyped')->with($this->callback( + static function ($event) use (&$committed): bool { + return $committed === 2 + && $event instanceof NotificationDeliveryStatusChangedEvent + && $event->getWebhookSerializable()['state'] === 'accepted' + && $event->getWebhookSerializable()['requestUuid'] === 'public-workflow-uuid'; + }, + )); + $channel = $this->createMock(NotificationChannel::class); + $channel->expects($this->once())->method('deliver'); + $history = new NotificationHistoryService($this->createMock(SignRequestMapper::class)); + $service = new NotificationSubmissionService( + $deliveries, $attempts, $db, new NotificationFailureClassifier(), + $history, $this->createMock(LoggerInterface::class), $files, $dispatcher, + ); + $this->assertTrue($service->submit($delivery, $this->message(), $channel)); + } + } From f95997c75309a9a75934390754886e65654cb4aa Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Mon, 9 Nov 2026 18:06:15 +0000 Subject: [PATCH 4/8] refactor: isolate persisted notification webhook publishing Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../NotificationDeliveryEventPublisher.php | 44 +++++++++ .../NotificationSubmissionService.php | 28 +----- ...NotificationDeliveryEventPublisherTest.php | 98 +++++++++++++++++++ .../NotificationSubmissionServiceTest.php | 66 ++++++------- .../NotificationDeliveryServiceTest.php | 5 +- 5 files changed, 177 insertions(+), 64 deletions(-) create mode 100644 lib/Service/Notification/NotificationDeliveryEventPublisher.php create mode 100644 tests/php/Unit/Service/Notification/NotificationDeliveryEventPublisherTest.php diff --git a/lib/Service/Notification/NotificationDeliveryEventPublisher.php b/lib/Service/Notification/NotificationDeliveryEventPublisher.php new file mode 100644 index 0000000000..8b1b764b0b --- /dev/null +++ b/lib/Service/Notification/NotificationDeliveryEventPublisher.php @@ -0,0 +1,44 @@ +deliveries->findById($deliveryId); + $file = $this->files->getById($delivery->getFileId()); + $root = $file->hasParent() + ? $this->files->getById($file->getParentFileId()) + : $file; + $event = NotificationDeliveryStatusChangedEvent::fromPersistedDelivery($root, $delivery); + if ($event !== null) { + $this->events->dispatchTyped($event); + } + } catch (\Exception $e) { + $this->logger->warning('Unable to publish persisted notification outcome', [ + 'deliveryId' => $deliveryId, + 'errorType' => get_debug_type($e), + ]); + } + } +} diff --git a/lib/Service/Notification/NotificationSubmissionService.php b/lib/Service/Notification/NotificationSubmissionService.php index a08f4072ea..20105dfacc 100644 --- a/lib/Service/Notification/NotificationSubmissionService.php +++ b/lib/Service/Notification/NotificationSubmissionService.php @@ -8,15 +8,12 @@ namespace OCA\Libresign\Service\Notification; -use OCA\Libresign\Db\FileMapper; use OCA\Libresign\Db\NotificationAttemptMapper; use OCA\Libresign\Db\NotificationDelivery; use OCA\Libresign\Db\NotificationDeliveryMapper; use OCA\Libresign\Enum\NotificationDeliveryState; -use OCA\Libresign\Events\NotificationDeliveryStatusChangedEvent; use OCA\Libresign\Exception\NotificationPreparationException; use OCP\AppFramework\Db\TTransactional; -use OCP\EventDispatcher\IEventDispatcher; use OCP\IDBConnection; use Psr\Log\LoggerInterface; @@ -32,8 +29,7 @@ public function __construct( private NotificationFailureClassifier $failureClassifier, private NotificationHistoryService $history, private LoggerInterface $logger, - private FileMapper $fileMapper, - private IEventDispatcher $eventDispatcher, + private NotificationDeliveryEventPublisher $eventPublisher, ) { } @@ -95,29 +91,9 @@ public function submit(NotificationDelivery $delivery, NotificationMessage $mess $this->attempts->finish($attempt->getId(), $outcome, $reason); $this->deliveries->finish($delivery->getId(), $outcome, $retryAt); }, $this->db); - $this->dispatchDeliveryStatusWebhook($delivery); + $this->eventPublisher->publish($delivery->getId()); } return true; } - private function dispatchDeliveryStatusWebhook(NotificationDelivery $delivery): void { - // A webhook failure must not mask a persisted provider result or its original error. - try { - $persisted = $this->deliveries->findById($delivery->getId()); - $file = $this->fileMapper->getById($persisted->getFileId()); - $root = $file->hasParent() - ? $this->fileMapper->getById($file->getParentFileId()) - : $file; - $event = NotificationDeliveryStatusChangedEvent::fromPersistedDelivery($root, $persisted); - if ($event !== null) { - $this->eventDispatcher->dispatchTyped($event); - } - } catch (\Throwable $e) { - $this->logger->warning('Unable to enqueue notification outcome webhook', [ - 'exception' => $e, - 'deliveryId' => $delivery->getId(), - ]); - } - } - } diff --git a/tests/php/Unit/Service/Notification/NotificationDeliveryEventPublisherTest.php b/tests/php/Unit/Service/Notification/NotificationDeliveryEventPublisherTest.php new file mode 100644 index 0000000000..733deb1727 --- /dev/null +++ b/tests/php/Unit/Service/Notification/NotificationDeliveryEventPublisherTest.php @@ -0,0 +1,98 @@ +deliveries = $this->createMock(NotificationDeliveryMapper::class); + $this->files = $this->createMock(FileMapper::class); + $this->dispatcher = $this->createMock(IEventDispatcher::class); + $this->logger = $this->createMock(LoggerInterface::class); + $this->publisher = new NotificationDeliveryEventPublisher( + $this->deliveries, $this->files, $this->dispatcher, $this->logger, + ); + } + + private function delivery(string $state = 'accepted'): NotificationDelivery { + $delivery = new NotificationDelivery(); + $delivery->setId(9); + $delivery->setFileId(25); + $delivery->setState($state); + $delivery->setAttemptCount(1); + $delivery->setChannel('mail'); + $delivery->setRecipientEnc('PRIVATE_RECIPIENT'); + return $delivery; + } + + public function testPublishedOutcomeUsesRootOfEnvelopeWithoutDisclosingRecipient(): void { + $this->deliveries->method('findById')->with(9)->willReturn($this->delivery()); + $child = new File(); + $child->setParentFileId(10); + $root = new File(); + $root->setUuid('root-uuid'); + $root->setNodeType('envelope'); + $this->files->method('getById')->willReturnMap([[25, $child], [10, $root]]); + $this->dispatcher->expects($this->once())->method('dispatchTyped') + ->with($this->callback(static function ($event): bool { + if (!$event instanceof NotificationDeliveryStatusChangedEvent) { + return false; + } + $payload = $event->getWebhookSerializable(); + return $payload['requestUuid'] === 'root-uuid' + && $payload['requestType'] === 'envelope' + && !str_contains(json_encode($payload), 'PRIVATE_RECIPIENT'); + })); + $this->publisher->publish(9); + } + + public function testPendingDeliveryCannotPublishAnOutcome(): void { + $this->deliveries->method('findById')->willReturn($this->delivery(NotificationDeliveryState::PENDING->value)); + $root = new File(); + $root->setUuid('root-uuid'); + $this->files->method('getById')->willReturn($root); + $this->dispatcher->expects($this->never())->method('dispatchTyped'); + $this->publisher->publish(9); + } + + public function testExpectedRepositoryFailureIsLoggedWithoutSensitiveData(): void { + $this->deliveries->method('findById') + ->willThrowException(new \RuntimeException('PRIVATE_RECIPIENT')); + $this->dispatcher->expects($this->never())->method('dispatchTyped'); + $this->logger->expects($this->once())->method('warning') + ->with($this->anything(), $this->callback(static function (array $context): bool { + return !str_contains(json_encode($context), 'PRIVATE_RECIPIENT') + && $context['deliveryId'] === 9; + })); + $this->publisher->publish(9); + } + + public function testProgrammingErrorIsNotSwallowed(): void { + $this->deliveries->method('findById')->willThrowException(new \TypeError('programming error')); + $this->expectException(\TypeError::class); + $this->publisher->publish(9); + } +} diff --git a/tests/php/Unit/Service/Notification/NotificationSubmissionServiceTest.php b/tests/php/Unit/Service/Notification/NotificationSubmissionServiceTest.php index 5ffae31b82..e8c98ee46d 100644 --- a/tests/php/Unit/Service/Notification/NotificationSubmissionServiceTest.php +++ b/tests/php/Unit/Service/Notification/NotificationSubmissionServiceTest.php @@ -9,7 +9,6 @@ namespace OCA\Libresign\Tests\Unit\Service\Notification; use OCA\Libresign\Db\File; -use OCA\Libresign\Db\FileMapper; use OCA\Libresign\Db\NotificationAttempt; use OCA\Libresign\Db\NotificationAttemptMapper; use OCA\Libresign\Db\NotificationDelivery; @@ -17,16 +16,15 @@ use OCA\Libresign\Db\SignRequest; use OCA\Libresign\Db\SignRequestMapper; use OCA\Libresign\Enum\NotificationDeliveryState; -use OCA\Libresign\Events\NotificationDeliveryStatusChangedEvent; use OCA\Libresign\Exception\NotificationPreparationException; use OCA\Libresign\Exception\NotificationTransportException; use OCA\Libresign\Service\Notification\MailPayload; use OCA\Libresign\Service\Notification\NotificationChannel; +use OCA\Libresign\Service\Notification\NotificationDeliveryEventPublisher; use OCA\Libresign\Service\Notification\NotificationFailureClassifier; use OCA\Libresign\Service\Notification\NotificationHistoryService; use OCA\Libresign\Service\Notification\NotificationMessage; use OCA\Libresign\Service\Notification\NotificationSubmissionService; -use OCP\EventDispatcher\IEventDispatcher; use OCP\IDBConnection; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; @@ -68,7 +66,7 @@ static function (?\DateTime $due) use ($expectRetry, $attemptNumber): bool { $db->expects($this->never())->method('rollBack'); $channel = $this->createMock(NotificationChannel::class); $channel->method('deliver')->willThrowException($failure); - $service = new NotificationSubmissionService($deliveries, $attempts, $db, new NotificationFailureClassifier(), new NotificationHistoryService($this->createMock(SignRequestMapper::class)), $this->createMock(LoggerInterface::class), $this->createMock(FileMapper::class), $this->createMock(IEventDispatcher::class)); + $service = new NotificationSubmissionService($deliveries, $attempts, $db, new NotificationFailureClassifier(), new NotificationHistoryService($this->createMock(SignRequestMapper::class)), $this->createMock(LoggerInterface::class), $this->createMock(NotificationDeliveryEventPublisher::class)); $this->expectException(get_class($failure)); $service->submit($delivery, $this->message(), $channel); } @@ -107,54 +105,54 @@ public function testNestedPreparationFailureCannotMakeTransportOutcomeSafe(): vo $this->executeFailure(new NotificationTransportException('synthetic uncertain outcome', NotificationDeliveryState::UNKNOWN, 'smtp_outcome_unknown', new NotificationPreparationException('preparation')), 1, NotificationDeliveryState::UNKNOWN, false); } - public function testAcceptedOutcomeWebhookIsDispatchedOnlyAfterLedgerCommit(): void { + public function testSuccessfulSubmissionPublishesAfterLedgerCommit(): void { $delivery = new NotificationDelivery(); $delivery->setId(9); - $delivery->setFileId(25); - $delivery->setChannel('mail'); - $delivery->setAttemptCount(1); - $persisted = new NotificationDelivery(); - $persisted->setId(9); - $persisted->setFileId(25); - $persisted->setChannel('mail'); - $persisted->setAttemptCount(1); - $persisted->setState(NotificationDeliveryState::ACCEPTED->value); $attempt = new NotificationAttempt(); $attempt->setId(28); $deliveries = $this->createMock(NotificationDeliveryMapper::class); - $deliveries->method('claim')->with(9)->willReturn(true); - $deliveries->method('findById')->with(9)->willReturn($persisted); + $deliveries->method('claim')->willReturn(true); $deliveries->expects($this->once())->method('finish')->with(9, NotificationDeliveryState::ACCEPTED, null); $attempts = $this->createMock(NotificationAttemptMapper::class); $attempts->method('begin')->willReturn($attempt); $attempts->expects($this->once())->method('finish')->with(28, NotificationDeliveryState::ACCEPTED, null); $db = $this->createMock(IDBConnection::class); - $committed = 0; - $db->expects($this->exactly(2))->method('commit')->willReturnCallback(static function () use (&$committed): void { - $committed++; + $commits = 0; + $db->method('commit')->willReturnCallback(static function () use (&$commits): void { + $commits++; }); - $root = new File(); - $root->setUuid('public-workflow-uuid'); - $files = $this->createMock(FileMapper::class); - $files->method('getById')->with(25)->willReturn($root); - $dispatcher = $this->createMock(IEventDispatcher::class); - $dispatcher->expects($this->once())->method('dispatchTyped')->with($this->callback( - static function ($event) use (&$committed): bool { - return $committed === 2 - && $event instanceof NotificationDeliveryStatusChangedEvent - && $event->getWebhookSerializable()['state'] === 'accepted' - && $event->getWebhookSerializable()['requestUuid'] === 'public-workflow-uuid'; - }, - )); + $publisher = $this->createMock(NotificationDeliveryEventPublisher::class); + $publisher->expects($this->once())->method('publish')->with(9) + ->willReturnCallback(function () use (&$commits): void { + $this->assertSame(2, $commits); + }); $channel = $this->createMock(NotificationChannel::class); $channel->expects($this->once())->method('deliver'); - $history = new NotificationHistoryService($this->createMock(SignRequestMapper::class)); $service = new NotificationSubmissionService( $deliveries, $attempts, $db, new NotificationFailureClassifier(), - $history, $this->createMock(LoggerInterface::class), $files, $dispatcher, + new NotificationHistoryService($this->createMock(SignRequestMapper::class)), + $this->createMock(LoggerInterface::class), $publisher, ); $this->assertTrue($service->submit($delivery, $this->message(), $channel)); } + public function testAlreadyClaimedDeliveryNeverSubmitsOrPublishes(): void { + $delivery = new NotificationDelivery(); + $delivery->setId(9); + $deliveries = $this->createMock(NotificationDeliveryMapper::class); + $deliveries->method('claim')->willReturn(false); + $channel = $this->createMock(NotificationChannel::class); + $channel->expects($this->never())->method('deliver'); + $publisher = $this->createMock(NotificationDeliveryEventPublisher::class); + $publisher->expects($this->never())->method('publish'); + $service = new NotificationSubmissionService( + $deliveries, $this->createMock(NotificationAttemptMapper::class), + $this->createMock(IDBConnection::class), new NotificationFailureClassifier(), + new NotificationHistoryService($this->createMock(SignRequestMapper::class)), + $this->createMock(LoggerInterface::class), $publisher, + ); + $this->assertFalse($service->submit($delivery, $this->message(), $channel)); + } + } diff --git a/tests/php/Unit/Service/NotificationDeliveryServiceTest.php b/tests/php/Unit/Service/NotificationDeliveryServiceTest.php index e0af462066..2e6bbff6f0 100644 --- a/tests/php/Unit/Service/NotificationDeliveryServiceTest.php +++ b/tests/php/Unit/Service/NotificationDeliveryServiceTest.php @@ -10,7 +10,6 @@ use OCA\Libresign\BackgroundJob\DeliverNotification; use OCA\Libresign\Db\File; -use OCA\Libresign\Db\FileMapper; use OCA\Libresign\Db\NotificationAttempt; use OCA\Libresign\Db\NotificationAttemptMapper; use OCA\Libresign\Db\NotificationDelivery; @@ -32,7 +31,6 @@ use OCA\Libresign\Service\NotificationDeliveryService; use OCA\Libresign\Service\Worker\WorkerActivationService; use OCP\BackgroundJob\IJobList; -use OCP\EventDispatcher\IEventDispatcher; use OCP\IConfig; use OCP\IDBConnection; use OCP\Notification\IManager; @@ -81,8 +79,7 @@ protected function setUp(): void { new NotificationFailureClassifier(), new NotificationHistoryService($this->createMock(\OCA\Libresign\Db\SignRequestMapper::class)), $this->createMock(LoggerInterface::class), - $this->createMock(FileMapper::class), - $this->createMock(IEventDispatcher::class), + $this->createMock(\OCA\Libresign\Service\Notification\NotificationDeliveryEventPublisher::class), ), new NotificationEnvelopeCodec($handoffCrypto, $this->createMock(IManager::class)), $this->mode, From 569f720234d32579b44e07ce7515808a5dfa1688 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Mon, 9 Nov 2026 18:07:43 +0000 Subject: [PATCH 5/8] fix: publish ready workflow events after signer persistence Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- lib/Service/FileStatusService.php | 15 +++---- lib/Service/RequestSignatureService.php | 41 ++++++++++++------- .../Unit/Service/FileStatusServiceTest.php | 11 +++++ .../Service/RequestSignatureServiceTest.php | 33 +++++++++++++++ 4 files changed, 76 insertions(+), 24 deletions(-) diff --git a/lib/Service/FileStatusService.php b/lib/Service/FileStatusService.php index 832bdd224b..6070e1cc71 100644 --- a/lib/Service/FileStatusService.php +++ b/lib/Service/FileStatusService.php @@ -28,7 +28,7 @@ public function __construct( ) { } - public function updateFileStatusIfUpgrade(FileEntity $file, int $newStatus): FileEntity { + public function updateFileStatusIfUpgrade(FileEntity $file, int $newStatus, bool $publishReady = true): FileEntity { $currentStatus = $file->getStatus(); if ($newStatus > $currentStatus) { $this->markPolicySnapshotFrozen($file, $newStatus); @@ -36,7 +36,9 @@ public function updateFileStatusIfUpgrade(FileEntity $file, int $newStatus): Fil $this->touchStatusChangedAt($file); $this->fileMapper->update($file); $this->statusCacheService->setStatus($file->getUuid(), $newStatus); - $this->dispatchReadyTransition($file, $currentStatus); + if ($publishReady && $currentStatus === FileStatus::DRAFT->value) { + $this->dispatchReadyRequest($file); + } if ($file->hasParent()) { $this->propagateStatusToParent($file->getParentFileId()); @@ -46,13 +48,8 @@ public function updateFileStatusIfUpgrade(FileEntity $file, int $newStatus): Fil return $file; } - /** Called after creating a ready request and persisting its signers. */ - public function dispatchCreatedReadyRequest(FileEntity $file): void { - $this->dispatchReadyTransition($file, FileStatus::DRAFT->value); - } - - private function dispatchReadyTransition(FileEntity $file, int $previousStatus): void { - $event = SigningRequestReadyEvent::fromTransition($file, $previousStatus); + public function dispatchReadyRequest(FileEntity $file): void { + $event = SigningRequestReadyEvent::fromTransition($file, FileStatus::DRAFT->value); if ($event !== null) { $this->eventDispatcher->dispatchTyped($event); } diff --git a/lib/Service/RequestSignatureService.php b/lib/Service/RequestSignatureService.php index 5e1f5df258..18017bd84b 100644 --- a/lib/Service/RequestSignatureService.php +++ b/lib/Service/RequestSignatureService.php @@ -139,8 +139,8 @@ public function saveFiles(array $data): array { } public function save(array $data): FileEntity { - $newlyCreated = false; - $file = $this->saveFile($data, $newlyCreated); + $becameReady = false; + $file = $this->saveFile($data, $becameReady); if (!isset($data['status'])) { $data['status'] = $file->getStatus(); } @@ -150,10 +150,10 @@ public function save(array $data): FileEntity { $this->propagateSignersToChildren($file, $data); $this->saveVisibleElements($data, $file); }, $this->db); - if ($newlyCreated) { - $this->fileStatusService->dispatchCreatedReadyRequest($file); - } $this->notifications->flushCommitted(); + if ($becameReady) { + $this->fileStatusService->dispatchReadyRequest($file); + } return $file; } @@ -227,15 +227,18 @@ public function saveEnvelope(array $data): array { }, $this->db); $this->notifications->flushCommitted(); } - - return [ - 'envelope' => $envelope, - 'files' => $files, - ]; } catch (\Throwable $e) { $this->rollbackEnvelopeCreation($envelope, $files, $createdNodes); throw $e; } + + if ($envelope->getStatus() === FileStatus::ABLE_TO_SIGN->value) { + $this->fileStatusService->dispatchReadyRequest($envelope); + } + return [ + 'envelope' => $envelope, + 'files' => $files, + ]; } private function processFileData(array $fileData, ?IUser $userManager, array $settings): Node { @@ -367,16 +370,20 @@ private function createFileForEnvelope( * uploadedFile?: array * } $data */ - public function saveFile(array $data, ?bool &$newlyCreated = null): FileEntity { - $newlyCreated = false; + public function saveFile(array $data, ?bool &$becameReady = null): FileEntity { + $becameReady = false; if (!empty($data['uuid'])) { $file = $this->fileMapper->getByUuid($data['uuid']); $this->filePolicyApplier->syncCoreFlowPolicies($file, $data); + $previousStatus = $file->getStatus(); if (!empty($data['name'])) { $file->setName($data['name']); $this->fileService->update($file); } - return $this->fileStatusService->updateFileStatusIfUpgrade($file, $data['status'] ?? 0); + $updated = $this->fileStatusService->updateFileStatusIfUpgrade($file, $data['status'] ?? 0, false); + $becameReady = $previousStatus === FileStatus::DRAFT->value + && $updated->getStatus() === FileStatus::ABLE_TO_SIGN->value; + return $updated; } $nodeId = null; if (isset($data['file']['fileNode']) && $data['file']['fileNode'] instanceof Node) { @@ -395,7 +402,11 @@ public function saveFile(array $data, ?bool &$newlyCreated = null): FileEntity { } if ($file !== null) { $this->filePolicyApplier->syncAllPolicies($file, $data); - return $this->fileStatusService->updateFileStatusIfUpgrade($file, $data['status'] ?? 0); + $previousStatus = $file->getStatus(); + $updated = $this->fileStatusService->updateFileStatusIfUpgrade($file, $data['status'] ?? 0, false); + $becameReady = $previousStatus === FileStatus::DRAFT->value + && $updated->getStatus() === FileStatus::ABLE_TO_SIGN->value; + return $updated; } } @@ -438,7 +449,7 @@ public function saveFile(array $data, ?bool &$newlyCreated = null): FileEntity { $this->filePolicyApplier->applyAll($file, $data); $this->fileMapper->insert($file); - $newlyCreated = true; + $becameReady = $file->getStatus() === FileStatus::ABLE_TO_SIGN->value; return $file; } diff --git a/tests/php/Unit/Service/FileStatusServiceTest.php b/tests/php/Unit/Service/FileStatusServiceTest.php index a8bbcac8a6..b262ebeac7 100644 --- a/tests/php/Unit/Service/FileStatusServiceTest.php +++ b/tests/php/Unit/Service/FileStatusServiceTest.php @@ -67,6 +67,17 @@ public function testReadyWebhookIsEmittedOnceForRootDraftTransition(): void { $this->service->updateFileStatusIfUpgrade($file, FileStatus::ABLE_TO_SIGN->value); } + public function testReadyEventCanBeDeferredUntilSignerPersistenceFinishes(): void { + $file = new FileEntity(); + $file->setId(11); + $file->setUuid('ready-uuid'); + $file->setStatus(FileStatus::DRAFT->value); + $this->service->updateFileStatusIfUpgrade($file, FileStatus::ABLE_TO_SIGN->value, false); + $this->eventDispatcher->expects($this->once())->method('dispatchTyped') + ->with($this->isInstanceOf(SigningRequestReadyEvent::class)); + $this->service->dispatchReadyRequest($file); + } + #[DataProvider('dataFileStatusUpgrade')] public function testUpdateFileStatusIfUpgrade(int $currentStatus, int $newStatus, bool $shouldUpdate): void { $file = new FileEntity(); diff --git a/tests/php/Unit/Service/RequestSignatureServiceTest.php b/tests/php/Unit/Service/RequestSignatureServiceTest.php index 7af9e9f6de..b1ff72a24e 100644 --- a/tests/php/Unit/Service/RequestSignatureServiceTest.php +++ b/tests/php/Unit/Service/RequestSignatureServiceTest.php @@ -318,6 +318,39 @@ public function testANewRequestCreatedReadyToSignStartsWithItsPolicySnapshotFroz $this->assertSame(['freeze:' . \OCA\Libresign\Enum\FileStatus::ABLE_TO_SIGN->value, 'insert'], $calls); } + public function testReadyRequestEventIsPublishedAfterSignerTransaction(): void { + $file = new \OCA\Libresign\Db\File(); + $file->setUuid('ready-uuid'); + $file->setStatus(\OCA\Libresign\Enum\FileStatus::ABLE_TO_SIGN->value); + $service = $this->getService(['saveFile']); + $service->method('saveFile')->willReturnCallback( + static function (array $data, ?bool &$becameReady) use ($file): \OCA\Libresign\Db\File { + $becameReady = true; + return $file; + }, + ); + $this->fileStatusService->expects($this->once())->method('dispatchReadyRequest')->with($file); + $this->assertSame($file, $service->save(['status' => $file->getStatus()])); + } + + public function testReadyRequestEventIsNotPublishedWhenSignerPersistenceFails(): void { + $file = new \OCA\Libresign\Db\File(); + $file->setUuid('ready-uuid'); + $file->setStatus(\OCA\Libresign\Enum\FileStatus::ABLE_TO_SIGN->value); + $service = $this->getService(['saveFile']); + $service->method('saveFile')->willReturnCallback( + static function (array $data, ?bool &$becameReady) use ($file): \OCA\Libresign\Db\File { + $becameReady = true; + return $file; + }, + ); + $this->signerValidator->method('normalizeRequestSigners') + ->willThrowException(new \RuntimeException('invalid signer')); + $this->fileStatusService->expects($this->never())->method('dispatchReadyRequest'); + $this->expectException(\RuntimeException::class); + $service->save(['status' => $file->getStatus(), 'signers' => [['identifyMethods' => []]]]); + } + public function testSaveFilesUsesSaveForSingleFile(): void { $service = $this->getService(['save']); From 1dd4f1d47788ebb1e6bc218589cb8f3d4d0146f6 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Mon, 9 Nov 2026 18:08:02 +0000 Subject: [PATCH 6/8] refactor: narrow rejection error handling and isolate event failures Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../SignatureRejectionService.php | 77 +++++++++++-------- .../SignatureRejectionServiceTest.php | 50 ++++++++++++ 2 files changed, 97 insertions(+), 30 deletions(-) diff --git a/lib/Service/SignatureRejection/SignatureRejectionService.php b/lib/Service/SignatureRejection/SignatureRejectionService.php index 70f086a14a..d8562e8285 100644 --- a/lib/Service/SignatureRejection/SignatureRejectionService.php +++ b/lib/Service/SignatureRejection/SignatureRejectionService.php @@ -24,6 +24,7 @@ use OCA\Libresign\Service\FileStatusService; use OCA\Libresign\Service\IdentifyMethodService; use OCA\Libresign\Service\SequentialSigningService; +use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Utility\ITimeFactory; use OCP\EventDispatcher\IEventDispatcher; use OCP\IDBConnection; @@ -144,16 +145,18 @@ private function persistRejection( } $this->db->commit(); - } catch (\Throwable $e) { - $this->db->rollBack(); - $this->restoreState($libreSignFile, $signRequests, $previousSignerStatuses, $previousFileStatus); - $this->logger->error('Failed to register the signature rejection: ' . $e->getMessage(), [ - 'exception' => $e, + } catch (\Exception $e) { + $this->rollBackRejection($libreSignFile, $signRequests, $previousSignerStatuses, $previousFileStatus); + $this->logger->error('Failed to register the signature rejection', [ + 'errorType' => get_debug_type($e), 'signRequestId' => $signRequest->getId(), ]); // TRANSLATORS Error shown when the rejection could not be saved and nothing was changed. throw new LibresignException($this->l10n->t('It was not possible to register the rejection. Nothing was changed.')); + } catch (\Error $e) { + $this->rollBackRejection($libreSignFile, $signRequests, $previousSignerStatuses, $previousFileStatus); + throw $e; } return $activatedSigners; @@ -188,8 +191,7 @@ private function collectSignRequestsToReject(FileEntity $libreSignFile, SignRequ $envelopeId, ); $collected[$envelopeSignRequest->getId()] ??= $envelopeSignRequest; - } catch (\Throwable) { - // The envelope itself carries no signature request for this signer. + } catch (DoesNotExistException) { } // A document this signer already signed keeps its signature. @@ -293,6 +295,16 @@ private function cancelWorkflow(FileEntity $libreSignFile): void { * @param list $signRequests * @param list $previousSignerStatuses */ + private function rollBackRejection( + FileEntity $file, + array $signRequests, + array $previousSignerStatuses, + int $previousFileStatus, + ): void { + $this->db->rollBack(); + $this->restoreState($file, $signRequests, $previousSignerStatuses, $previousFileStatus); + } + private function restoreState( FileEntity $libreSignFile, array $signRequests, @@ -370,33 +382,36 @@ private function dispatchPublicRejectionEvents( SignRequestEntity $signer, int $previousRootStatus, ): void { - // Signing-state transactions have committed. A webhook scheduling failure - // must not undo a persisted rejection. try { $root = $document->hasParent() ? $this->fileMapper->getById($document->getParentFileId()) : $document; - $rejected = SignerRejectedWebhookEvent::afterRejection( - $root, $document, $signer, $this->isWorkflowCanceled($root), - ); - if ($rejected !== null) { - $this->eventDispatcher->dispatchTyped($rejected); - } - $canceled = SigningRequestCanceledEvent::fromTransition($root, $previousRootStatus); - if ($canceled !== null) { - $this->eventDispatcher->dispatchTyped($canceled); - } - // Under a continue-on-rejection policy, the final rejection may - // finish a workflow that already contains a valid signature. - $completed = SigningRequestCompletedEvent::fromTransition($root, $previousRootStatus); - if ($completed !== null) { - $this->eventDispatcher->dispatchTyped($completed); - } - } catch (\Throwable $e) { - $this->logger->error('Unable to enqueue LibreSign rejection webhook', [ - 'exception' => $e, + $events = [ + SignerRejectedWebhookEvent::afterRejection($root, $document, $signer, $this->isWorkflowCanceled($root)), + SigningRequestCanceledEvent::fromTransition($root, $previousRootStatus), + SigningRequestCompletedEvent::fromTransition($root, $previousRootStatus), + ]; + } catch (\Exception $e) { + $this->logger->warning('Unable to prepare LibreSign rejection events', [ 'fileId' => $document->getId(), + 'errorType' => get_debug_type($e), ]); + return; + } + + foreach ($events as $event) { + if ($event === null) { + continue; + } + try { + $this->eventDispatcher->dispatchTyped($event); + } catch (\Exception $e) { + $this->logger->warning('Unable to publish LibreSign rejection event', [ + 'fileId' => $document->getId(), + 'eventType' => $event::class, + 'errorType' => get_debug_type($e), + ]); + } } } @@ -419,8 +434,10 @@ private function dispatchRejectedEvent( )); } } - } catch (\Throwable $e) { - $this->logger->error('Error dispatching SignatureRejectedEvent: ' . $e->getMessage(), ['exception' => $e]); + } catch (\Exception $e) { + $this->logger->error('Error dispatching SignatureRejectedEvent', [ + 'errorType' => get_debug_type($e), + ]); } } } diff --git a/tests/php/Unit/Service/SignatureRejection/SignatureRejectionServiceTest.php b/tests/php/Unit/Service/SignatureRejection/SignatureRejectionServiceTest.php index 74c66c3533..aa801dd53c 100644 --- a/tests/php/Unit/Service/SignatureRejection/SignatureRejectionServiceTest.php +++ b/tests/php/Unit/Service/SignatureRejection/SignatureRejectionServiceTest.php @@ -316,6 +316,56 @@ public static function publicRejectionOutcomes(): array { ]; } + public function testProgrammingErrorDuringRejectionRollsBackAndRemainsVisible(): void { + $this->withPolicy(self::policy('cancel')); + $signer = $this->signRequest(); + $this->signRequestMapper->method('update') + ->willThrowException(new \TypeError('programming error')); + $this->db->expects($this->once())->method('rollBack'); + $this->db->expects($this->never())->method('commit'); + $this->eventDispatcher->expects($this->never())->method('dispatchTyped'); + $this->expectException(\TypeError::class); + try { + $this->getService()->reject($this->file(), $signer); + } finally { + $this->assertSame(SignRequestStatus::ABLE_TO_SIGN, $signer->getStatusEnum()); + } + } + + public function testFailedPublicRejectionEventDoesNotSuppressCancellationEvent(): void { + $this->withPolicy(self::policy('cancel')); + $file = $this->file(); + $file->setUuid('workflow-uuid'); + $signer = $this->signRequest(); + $events = []; + $this->eventDispatcher->method('dispatchTyped')->willReturnCallback( + static function ($event) use (&$events): void { + $events[] = $event; + if ($event instanceof SignerRejectedWebhookEvent) { + throw new \RuntimeException('private recipient information'); + } + }, + ); + $this->logger->expects($this->once())->method('warning') + ->with($this->anything(), $this->callback(static function (array $context): bool { + return $context['errorType'] === 'RuntimeException' + && !str_contains(json_encode($context), 'private recipient information'); + })); + $this->getService()->reject($file, $signer); + $this->assertCount(1, array_filter($events, static fn ($event): bool => $event instanceof SigningRequestCanceledEvent)); + } + + public function testProgrammingErrorDuringPublicEventDispatchIsNotSuppressed(): void { + $this->withPolicy(self::policy('cancel')); + $file = $this->file(); + $file->setUuid('workflow-uuid'); + $this->db->expects($this->once())->method('commit'); + $this->db->expects($this->never())->method('rollBack'); + $this->eventDispatcher->method('dispatchTyped')->willThrowException(new \TypeError('programming error')); + $this->expectException(\TypeError::class); + $this->getService()->reject($file, $this->signRequest()); + } + public function testRejectionCanCompletePreviouslySignedWorkflow(): void { $this->withPolicy(self::policy('continue')); $file = $this->file(FileStatus::PARTIAL_SIGNED->value); From 1f04265fc75190c1b0da0a51dbe4537e6b8e7378 Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Tue, 10 Nov 2026 01:30:16 +0000 Subject: [PATCH 7/8] test: verify notification outcomes through persisted ledger Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../NotificationDeliveryServiceTest.php | 67 +++++++++++++++++++ .../NotificationSubmissionServiceTest.php | 18 ++--- 2 files changed, 72 insertions(+), 13 deletions(-) diff --git a/tests/php/Integration/Service/NotificationDeliveryServiceTest.php b/tests/php/Integration/Service/NotificationDeliveryServiceTest.php index 439efad08b..ce25508582 100644 --- a/tests/php/Integration/Service/NotificationDeliveryServiceTest.php +++ b/tests/php/Integration/Service/NotificationDeliveryServiceTest.php @@ -10,18 +10,23 @@ use OCA\Libresign\BackgroundJob\DeliverNotification; use OCA\Libresign\Db\File; +use OCA\Libresign\Db\FileMapper; use OCA\Libresign\Db\NotificationAttemptMapper; use OCA\Libresign\Db\NotificationDeliveryMapper; use OCA\Libresign\Db\SignRequest; +use OCA\Libresign\Enum\FileStatus; use OCA\Libresign\Enum\NotificationDeliveryState; +use OCA\Libresign\Events\NotificationDeliveryStatusChangedEvent; use OCA\Libresign\Exception\MailTransportException; use OCA\Libresign\Service\Notification\GatewayPayload; use OCA\Libresign\Service\Notification\MailPayload; use OCA\Libresign\Service\Notification\NotificationChannel; use OCA\Libresign\Service\Notification\NotificationMessage; +use OCA\Libresign\Service\Notification\NotificationSubmissionService; use OCA\Libresign\Service\NotificationDeliveryService; use OCP\AppFramework\Db\DoesNotExistException; use OCP\BackgroundJob\IJobList; +use OCP\EventDispatcher\IEventDispatcher; use OCP\IDBConnection; use OCP\Server; @@ -180,6 +185,68 @@ public function testRealDatabaseMakesOneSmtpSubmissionAndRecordsAcceptance(): vo $this->assertSame(NotificationDeliveryState::ACCEPTED->value, $attempts[0]->getState()); } + public function testSubmittedOutcomeEventReflectsCommittedLedgerWithoutRecipientData(): void { + $fileMapper = Server::get(FileMapper::class); + $file = new File(); + $file->setUuid('notification-workflow-' . bin2hex(random_bytes(12))); + $file->setNodeId(random_int(1000000, 1999999)); + $file->setUserId('synthetic-owner'); + $file->setName('notification-contract.pdf'); + $file->setCreatedAt(new \DateTime('now', new \DateTimeZone('UTC'))); + $file->setStatus(FileStatus::ABLE_TO_SIGN->value); + $file = $fileMapper->insert($file); + + $source = 'committed-event-' . bin2hex(random_bytes(10)); + $key = hash('sha256', 'outcome-' . $source); + $dispatcher = Server::get(IEventDispatcher::class); + $db = Server::get(IDBConnection::class); + $observed = []; + $listener = function (NotificationDeliveryStatusChangedEvent $event) use (&$observed, $db, $key): void { + $delivery = $this->deliveries->findByEventKey($key); + $attempts = $this->attempts->findForDelivery($delivery->getId()); + $observed[] = [ + 'inTransaction' => $db->inTransaction(), + 'ledgerState' => $delivery->getState(), + 'attemptStates' => array_map(static fn ($attempt): string => $attempt->getState(), $attempts), + 'payload' => $event->getWebhookSerializable(), + ]; + }; + $dispatcher->addListener(NotificationDeliveryStatusChangedEvent::class, $listener); + + try { + $delivery = $this->deliveries->register( + $key, hash('sha256', 'correlation-' . $source), 'sign_request', 'mail', + $file->getId(), null, 'synthetic-owner', 'PRIVATE_ENCRYPTED_RECIPIENT', + ); + $this->keys[] = $key; + $channel = new class implements NotificationChannel { + public function supports(string $channel): bool { + return $channel === 'mail'; + } + public function deliver(NotificationMessage $message): void { + } + }; + $this->assertTrue(Server::get(NotificationSubmissionService::class)->submit( + $delivery, + new NotificationMessage($source, 'sign_request', 'mail', $file, $this->request(), 'synthetic@example.invalid', new MailPayload()), + $channel, + )); + $this->assertCount(1, $observed); + $this->assertFalse($observed[0]['inTransaction']); + $this->assertSame(NotificationDeliveryState::ACCEPTED->value, $observed[0]['ledgerState']); + $this->assertSame([NotificationDeliveryState::ACCEPTED->value], $observed[0]['attemptStates']); + $this->assertSame($file->getUuid(), $observed[0]['payload']['requestUuid']); + $this->assertSame(NotificationDeliveryState::ACCEPTED->value, $observed[0]['payload']['state']); + $this->assertSame(1, $observed[0]['payload']['attempt']); + $serialized = json_encode($observed[0]['payload'], JSON_THROW_ON_ERROR); + $this->assertStringNotContainsString('synthetic@example.invalid', $serialized); + $this->assertStringNotContainsString('PRIVATE_ENCRYPTED_RECIPIENT', $serialized); + } finally { + $dispatcher->removeListener(NotificationDeliveryStatusChangedEvent::class, $listener); + $fileMapper->delete($file); + } + } + public function testAmbiguousSubmissionNeverRetriesEvenWhenEventReplayed(): void { $source = 'integration-unknown-' . bin2hex(random_bytes(8)); $key = $this->recordKey(101, 201, $source); diff --git a/tests/php/Unit/Service/Notification/NotificationSubmissionServiceTest.php b/tests/php/Unit/Service/Notification/NotificationSubmissionServiceTest.php index e8c98ee46d..0f21d47fac 100644 --- a/tests/php/Unit/Service/Notification/NotificationSubmissionServiceTest.php +++ b/tests/php/Unit/Service/Notification/NotificationSubmissionServiceTest.php @@ -105,7 +105,7 @@ public function testNestedPreparationFailureCannotMakeTransportOutcomeSafe(): vo $this->executeFailure(new NotificationTransportException('synthetic uncertain outcome', NotificationDeliveryState::UNKNOWN, 'smtp_outcome_unknown', new NotificationPreparationException('preparation')), 1, NotificationDeliveryState::UNKNOWN, false); } - public function testSuccessfulSubmissionPublishesAfterLedgerCommit(): void { + public function testSuccessfulSubmissionRequestsOutcomePublication(): void { $delivery = new NotificationDelivery(); $delivery->setId(9); $attempt = new NotificationAttempt(); @@ -113,27 +113,19 @@ public function testSuccessfulSubmissionPublishesAfterLedgerCommit(): void { $deliveries = $this->createMock(NotificationDeliveryMapper::class); $deliveries->method('claim')->willReturn(true); - $deliveries->expects($this->once())->method('finish')->with(9, NotificationDeliveryState::ACCEPTED, null); $attempts = $this->createMock(NotificationAttemptMapper::class); $attempts->method('begin')->willReturn($attempt); - $attempts->expects($this->once())->method('finish')->with(28, NotificationDeliveryState::ACCEPTED, null); - $db = $this->createMock(IDBConnection::class); - $commits = 0; - $db->method('commit')->willReturnCallback(static function () use (&$commits): void { - $commits++; - }); $publisher = $this->createMock(NotificationDeliveryEventPublisher::class); - $publisher->expects($this->once())->method('publish')->with(9) - ->willReturnCallback(function () use (&$commits): void { - $this->assertSame(2, $commits); - }); + $publisher->expects($this->once())->method('publish')->with(9); $channel = $this->createMock(NotificationChannel::class); $channel->expects($this->once())->method('deliver'); + $service = new NotificationSubmissionService( - $deliveries, $attempts, $db, new NotificationFailureClassifier(), + $deliveries, $attempts, $this->createMock(IDBConnection::class), new NotificationFailureClassifier(), new NotificationHistoryService($this->createMock(SignRequestMapper::class)), $this->createMock(LoggerInterface::class), $publisher, ); + $this->assertTrue($service->submit($delivery, $this->message(), $channel)); } From 2a149945a0e3f1ae2583714c5d4b2100e13ac93a Mon Sep 17 00:00:00 2001 From: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> Date: Tue, 10 Nov 2026 01:35:56 +0000 Subject: [PATCH 8/8] test: keep notification workflow fixture UUID within SQL schema Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com> --- .../php/Integration/Service/NotificationDeliveryServiceTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/php/Integration/Service/NotificationDeliveryServiceTest.php b/tests/php/Integration/Service/NotificationDeliveryServiceTest.php index ce25508582..926af12a62 100644 --- a/tests/php/Integration/Service/NotificationDeliveryServiceTest.php +++ b/tests/php/Integration/Service/NotificationDeliveryServiceTest.php @@ -188,7 +188,7 @@ public function testRealDatabaseMakesOneSmtpSubmissionAndRecordsAcceptance(): vo public function testSubmittedOutcomeEventReflectsCommittedLedgerWithoutRecipientData(): void { $fileMapper = Server::get(FileMapper::class); $file = new File(); - $file->setUuid('notification-workflow-' . bin2hex(random_bytes(12))); + $file->setUuid(bin2hex(random_bytes(16))); $file->setNodeId(random_int(1000000, 1999999)); $file->setUserId('synthetic-owner'); $file->setName('notification-contract.pdf');