diff --git a/lib/Client/ClientInterface.php b/lib/Client/ClientInterface.php index 88e9680..cf57ab6 100644 --- a/lib/Client/ClientInterface.php +++ b/lib/Client/ClientInterface.php @@ -17,6 +17,10 @@ interface ClientInterface public function capabilities(): array; /** + * Unsuccessful command completion throws ImapException. For streamed + * results, completion is checked as the returned generator is consumed. + * + * @throws ImapException * @template TResult * @param CommandInterface $command * @return TResult diff --git a/lib/Client/Protocol/Command/CreateCommand.php b/lib/Client/Protocol/Command/CreateCommand.php index b95928a..08b4f1f 100644 --- a/lib/Client/Protocol/Command/CreateCommand.php +++ b/lib/Client/Protocol/Command/CreateCommand.php @@ -5,7 +5,7 @@ declare(strict_types=1); namespace KTXM\ProviderImap\Client\Protocol\Command; use KTXM\ProviderImap\Client\Protocol\StringEncoder; -use KTXM\ProviderImap\Client\Result\CommandStatusResult; +use KTXM\ProviderImap\Client\Result\CommandCompletion; use KTXM\ProviderImap\Client\ImapException; use KTXM\ProviderImap\Client\Protocol\RequestFrame; use KTXM\ProviderImap\Client\Protocol\Response\TaggedResponse; @@ -14,7 +14,7 @@ use KTXM\ProviderImap\Client\Protocol\SessionContext; use KTXM\ProviderImap\Client\Protocol\SessionState; /** - * @implements CommandInterface + * @implements CommandInterface */ final class CreateCommand implements CommandInterface { @@ -42,7 +42,7 @@ final class CreateCommand implements CommandInterface return new RequestFrame(sprintf('CREATE %s', StringEncoder::quote($this->mailbox))); } - public function handle(ResponseStream $responses, SessionContext $context): CommandStatusResult + public function handle(ResponseStream $responses, SessionContext $context): CommandCompletion { unset($context); @@ -52,7 +52,7 @@ final class CreateCommand implements CommandInterface throw new ImapException('CREATE failed: ' . $response->text()); } - return new CommandStatusResult($response->status(), $response->text()); + return new CommandCompletion($response->status(), $response->text()); } } diff --git a/lib/Client/Protocol/Command/DeleteCommand.php b/lib/Client/Protocol/Command/DeleteCommand.php index f2c22d8..66a669b 100644 --- a/lib/Client/Protocol/Command/DeleteCommand.php +++ b/lib/Client/Protocol/Command/DeleteCommand.php @@ -5,7 +5,7 @@ declare(strict_types=1); namespace KTXM\ProviderImap\Client\Protocol\Command; use KTXM\ProviderImap\Client\Protocol\StringEncoder; -use KTXM\ProviderImap\Client\Result\CommandStatusResult; +use KTXM\ProviderImap\Client\Result\CommandCompletion; use KTXM\ProviderImap\Client\ImapException; use KTXM\ProviderImap\Client\Protocol\RequestFrame; use KTXM\ProviderImap\Client\Protocol\Response\TaggedResponse; @@ -14,7 +14,7 @@ use KTXM\ProviderImap\Client\Protocol\SessionContext; use KTXM\ProviderImap\Client\Protocol\SessionState; /** - * @implements CommandInterface + * @implements CommandInterface */ final class DeleteCommand implements CommandInterface { @@ -42,7 +42,7 @@ final class DeleteCommand implements CommandInterface return new RequestFrame(sprintf('DELETE %s', StringEncoder::quote($this->mailbox))); } - public function handle(ResponseStream $responses, SessionContext $context): CommandStatusResult + public function handle(ResponseStream $responses, SessionContext $context): CommandCompletion { if ($context->selectedMailbox() === $this->mailbox) { $context->setSelectedMailbox(null); @@ -55,7 +55,7 @@ final class DeleteCommand implements CommandInterface throw new ImapException('DELETE failed: ' . $response->text()); } - return new CommandStatusResult($response->status(), $response->text()); + return new CommandCompletion($response->status(), $response->text()); } } diff --git a/lib/Client/Protocol/Command/LoginCommand.php b/lib/Client/Protocol/Command/LoginCommand.php index c932a0e..610f6ec 100644 --- a/lib/Client/Protocol/Command/LoginCommand.php +++ b/lib/Client/Protocol/Command/LoginCommand.php @@ -5,7 +5,7 @@ declare(strict_types=1); namespace KTXM\ProviderImap\Client\Protocol\Command; use KTXM\ProviderImap\Client\Protocol\StringEncoder; -use KTXM\ProviderImap\Client\Result\CommandStatusResult; +use KTXM\ProviderImap\Client\Result\CommandCompletion; use KTXM\ProviderImap\Client\ImapException; use KTXM\ProviderImap\Client\Protocol\RequestFrame; use KTXM\ProviderImap\Client\Protocol\Response\TaggedResponse; @@ -14,7 +14,7 @@ use KTXM\ProviderImap\Client\Protocol\SessionContext; use KTXM\ProviderImap\Client\Protocol\SessionState; /** - * @implements CommandInterface + * @implements CommandInterface */ final class LoginCommand implements CommandInterface { @@ -44,7 +44,7 @@ final class LoginCommand implements CommandInterface )); } - public function handle(ResponseStream $responses, SessionContext $context): CommandStatusResult + public function handle(ResponseStream $responses, SessionContext $context): CommandCompletion { foreach ($responses as $response) { if ($response instanceof TaggedResponse) { @@ -54,7 +54,7 @@ final class LoginCommand implements CommandInterface $context->setState(SessionState::Authenticated); - return new CommandStatusResult($response->status(), $response->text()); + return new CommandCompletion($response->status(), $response->text()); } } diff --git a/lib/Client/Protocol/Command/LogoutCommand.php b/lib/Client/Protocol/Command/LogoutCommand.php index c933024..c3fc6b9 100644 --- a/lib/Client/Protocol/Command/LogoutCommand.php +++ b/lib/Client/Protocol/Command/LogoutCommand.php @@ -4,7 +4,7 @@ declare(strict_types=1); namespace KTXM\ProviderImap\Client\Protocol\Command; -use KTXM\ProviderImap\Client\Result\CommandStatusResult; +use KTXM\ProviderImap\Client\Result\CommandCompletion; use KTXM\ProviderImap\Client\ImapException; use KTXM\ProviderImap\Client\Protocol\RequestFrame; use KTXM\ProviderImap\Client\Protocol\Response\TaggedResponse; @@ -13,7 +13,7 @@ use KTXM\ProviderImap\Client\Protocol\SessionContext; use KTXM\ProviderImap\Client\Protocol\SessionState; /** - * @implements CommandInterface + * @implements CommandInterface */ final class LogoutCommand implements CommandInterface { @@ -38,7 +38,7 @@ final class LogoutCommand implements CommandInterface return new RequestFrame('LOGOUT'); } - public function handle(ResponseStream $responses, SessionContext $context): CommandStatusResult + public function handle(ResponseStream $responses, SessionContext $context): CommandCompletion { foreach ($responses as $response) { if ($response instanceof TaggedResponse) { @@ -50,7 +50,7 @@ final class LogoutCommand implements CommandInterface $context->setState(SessionState::Logout); $context->connection()->disconnect(); - return new CommandStatusResult($response->status(), $response->text()); + return new CommandCompletion($response->status(), $response->text()); } } diff --git a/lib/Client/Protocol/Command/NoopCommand.php b/lib/Client/Protocol/Command/NoopCommand.php index 09f9a73..3bc06d7 100644 --- a/lib/Client/Protocol/Command/NoopCommand.php +++ b/lib/Client/Protocol/Command/NoopCommand.php @@ -4,7 +4,7 @@ declare(strict_types=1); namespace KTXM\ProviderImap\Client\Protocol\Command; -use KTXM\ProviderImap\Client\Result\CommandStatusResult; +use KTXM\ProviderImap\Client\Result\CommandCompletion; use KTXM\ProviderImap\Client\ImapException; use KTXM\ProviderImap\Client\Protocol\RequestFrame; use KTXM\ProviderImap\Client\Protocol\Response\TaggedResponse; @@ -13,7 +13,7 @@ use KTXM\ProviderImap\Client\Protocol\SessionContext; use KTXM\ProviderImap\Client\Protocol\SessionState; /** - * @implements CommandInterface + * @implements CommandInterface */ final class NoopCommand implements CommandInterface { @@ -38,7 +38,7 @@ final class NoopCommand implements CommandInterface return new RequestFrame('NOOP'); } - public function handle(ResponseStream $responses, SessionContext $context): CommandStatusResult + public function handle(ResponseStream $responses, SessionContext $context): CommandCompletion { unset($context); @@ -48,7 +48,7 @@ final class NoopCommand implements CommandInterface throw new ImapException('NOOP failed: ' . $response->text()); } - return new CommandStatusResult($response->status(), $response->text()); + return new CommandCompletion($response->status(), $response->text()); } } diff --git a/lib/Client/Protocol/Command/RenameCommand.php b/lib/Client/Protocol/Command/RenameCommand.php index 5a2118c..badd315 100644 --- a/lib/Client/Protocol/Command/RenameCommand.php +++ b/lib/Client/Protocol/Command/RenameCommand.php @@ -5,7 +5,7 @@ declare(strict_types=1); namespace KTXM\ProviderImap\Client\Protocol\Command; use KTXM\ProviderImap\Client\Protocol\StringEncoder; -use KTXM\ProviderImap\Client\Result\CommandStatusResult; +use KTXM\ProviderImap\Client\Result\CommandCompletion; use KTXM\ProviderImap\Client\ImapException; use KTXM\ProviderImap\Client\Protocol\RequestFrame; use KTXM\ProviderImap\Client\Protocol\Response\TaggedResponse; @@ -14,7 +14,7 @@ use KTXM\ProviderImap\Client\Protocol\SessionContext; use KTXM\ProviderImap\Client\Protocol\SessionState; /** - * @implements CommandInterface + * @implements CommandInterface */ final class RenameCommand implements CommandInterface { @@ -47,7 +47,7 @@ final class RenameCommand implements CommandInterface )); } - public function handle(ResponseStream $responses, SessionContext $context): CommandStatusResult + public function handle(ResponseStream $responses, SessionContext $context): CommandCompletion { foreach ($responses as $response) { if ($response instanceof TaggedResponse) { @@ -59,7 +59,7 @@ final class RenameCommand implements CommandInterface $context->setSelectedMailbox($this->toMailbox); } - return new CommandStatusResult($response->status(), $response->text()); + return new CommandCompletion($response->status(), $response->text()); } } diff --git a/lib/Client/Protocol/Command/StartTlsCommand.php b/lib/Client/Protocol/Command/StartTlsCommand.php index bcc1b9b..d351d2d 100644 --- a/lib/Client/Protocol/Command/StartTlsCommand.php +++ b/lib/Client/Protocol/Command/StartTlsCommand.php @@ -4,7 +4,7 @@ declare(strict_types=1); namespace KTXM\ProviderImap\Client\Protocol\Command; -use KTXM\ProviderImap\Client\Result\CommandStatusResult; +use KTXM\ProviderImap\Client\Result\CommandCompletion; use KTXM\ProviderImap\Client\ImapException; use KTXM\ProviderImap\Client\Protocol\RequestFrame; use KTXM\ProviderImap\Client\Protocol\Response\TaggedResponse; @@ -13,7 +13,7 @@ use KTXM\ProviderImap\Client\Protocol\SessionContext; use KTXM\ProviderImap\Client\Protocol\SessionState; /** - * @implements CommandInterface + * @implements CommandInterface */ final class StartTlsCommand implements CommandInterface { @@ -34,7 +34,7 @@ final class StartTlsCommand implements CommandInterface return new RequestFrame('STARTTLS'); } - public function handle(ResponseStream $responses, SessionContext $context): CommandStatusResult + public function handle(ResponseStream $responses, SessionContext $context): CommandCompletion { foreach ($responses as $response) { if ($response instanceof TaggedResponse) { @@ -45,7 +45,7 @@ final class StartTlsCommand implements CommandInterface $context->connection()->upgradeToTls(); $context->replaceCapabilities(); - return new CommandStatusResult($response->status(), $response->text()); + return new CommandCompletion($response->status(), $response->text()); } } diff --git a/lib/Client/Protocol/Command/StoreCommand.php b/lib/Client/Protocol/Command/StoreCommand.php index 32fd15a..2157e5a 100644 --- a/lib/Client/Protocol/Command/StoreCommand.php +++ b/lib/Client/Protocol/Command/StoreCommand.php @@ -4,7 +4,7 @@ declare(strict_types=1); namespace KTXM\ProviderImap\Client\Protocol\Command; -use KTXM\ProviderImap\Client\Result\CommandStatusResult; +use KTXM\ProviderImap\Client\Result\CommandCompletion; use KTXM\ProviderImap\Client\Protocol\Command\Argument\MessageTarget; use KTXM\ProviderImap\Client\Protocol\IdentifierMode; use KTXM\ProviderImap\Client\ImapException; @@ -16,7 +16,7 @@ use KTXM\ProviderImap\Client\Protocol\SessionContext; use KTXM\ProviderImap\Client\Protocol\SessionState; /** - * @implements CommandInterface + * @implements CommandInterface */ final class StoreCommand implements CommandInterface { @@ -82,7 +82,7 @@ final class StoreCommand implements CommandInterface )); } - public function handle(ResponseStream $responses, SessionContext $context): CommandStatusResult + public function handle(ResponseStream $responses, SessionContext $context): CommandCompletion { if ($context->selectedMailbox() === null) { throw new ImapException('STORE requires a selected mailbox.'); @@ -94,7 +94,7 @@ final class StoreCommand implements CommandInterface throw new ImapException('STORE failed: ' . $response->text()); } - return new CommandStatusResult($response->status(), $response->text()); + return new CommandCompletion($response->status(), $response->text()); } } diff --git a/lib/Client/Result/CommandStatusResult.php b/lib/Client/Result/CommandCompletion.php similarity index 75% rename from lib/Client/Result/CommandStatusResult.php rename to lib/Client/Result/CommandCompletion.php index 89e57ef..51ed861 100644 --- a/lib/Client/Result/CommandStatusResult.php +++ b/lib/Client/Result/CommandCompletion.php @@ -4,7 +4,10 @@ declare(strict_types=1); namespace KTXM\ProviderImap\Client\Result; -final class CommandStatusResult +/** + * Successful command output; unsuccessful completion is reported by exception. + */ +final class CommandCompletion { public function __construct( private readonly string $status, @@ -20,9 +23,4 @@ final class CommandStatusResult { return $this->text; } - - public function isOk(): bool - { - return $this->status === 'OK'; - } } \ No newline at end of file diff --git a/lib/Client/Result/MessageTransferResult.php b/lib/Client/Result/MessageTransferResult.php index a94159c..d90e755 100644 --- a/lib/Client/Result/MessageTransferResult.php +++ b/lib/Client/Result/MessageTransferResult.php @@ -4,6 +4,9 @@ declare(strict_types=1); namespace KTXM\ProviderImap\Client\Result; +/** + * Successful command output; unsuccessful completion is reported by exception. + */ final class MessageTransferResult { /** @@ -33,11 +36,6 @@ final class MessageTransferResult return $this->text; } - public function isOk(): bool - { - return $this->status === 'OK'; - } - /** * @return list, text:string}> */ diff --git a/lib/Service/Remote/RemoteMailService.php b/lib/Service/Remote/RemoteMailService.php index 5ad3737..a96ae00 100644 --- a/lib/Service/Remote/RemoteMailService.php +++ b/lib/Service/Remote/RemoteMailService.php @@ -220,11 +220,7 @@ class RemoteMailService */ public function collectionCreate(string $name): Mailbox { - $result = $this->imapClient()->perform(new CreateCommand($name)); - - if (!$result->isOk()) { - throw new ImapException('Failed to create mailbox: ' . $name); - } + $this->imapClient()->perform(new CreateCommand($name)); // Attempt to refetch the new mailbox from the server $mailbox = $this->collectionFetch($name); @@ -240,11 +236,7 @@ class RemoteMailService */ public function collectionRename(string $oldName, string $newName): Mailbox { - $result = $this->imapClient()->perform(new RenameCommand($oldName, $newName)); - - if (!$result->isOk()) { - throw new ImapException('Failed to rename mailbox: ' . $oldName . ' to ' . $newName); - } + $this->imapClient()->perform(new RenameCommand($oldName, $newName)); $mailbox = $this->collectionFetch($newName); @@ -260,11 +252,7 @@ class RemoteMailService */ public function collectionDestroy(string $name): bool { - $result = $this->imapClient()->perform(new DeleteCommand($name)); - - if (!$result->isOk()) { - throw new ImapException('Failed to delete mailbox: ' . $name); - } + $this->imapClient()->perform(new DeleteCommand($name)); return true; } @@ -617,20 +605,14 @@ class RemoteMailService MessageTarget::uid(SequenceSet::items(...array_values($uids))), $targetCollection, )); - if ($response->isOk()) { - $this->imapClient()->perform(new StoreCommand( - MessageTarget::uid(SequenceSet::items(...array_values($uids))), - ['\\Deleted'], - '+', - )); - $this->imapClient()->perform(new ExpungeCommand( - MessageTarget::uid(SequenceSet::items(...array_values($uids))), - )); - } - } - - if (!$response->isOk()) { - throw new ImapException('Failed to move messages: ' . implode(', ', $response->responseCodes())); + $this->imapClient()->perform(new StoreCommand( + MessageTarget::uid(SequenceSet::items(...array_values($uids))), + ['\\Deleted'], + '+', + )); + $this->imapClient()->perform(new ExpungeCommand( + MessageTarget::uid(SequenceSet::items(...array_values($uids))), + )); } // construct operation result as a map of source UID to boolean or destination UID, depending on server support @@ -660,10 +642,6 @@ class RemoteMailService $targetCollection, )); - if (!$response->isOk()) { - throw new ImapException('Failed to copy messages: ' . implode(', ', $response->responseCodes())); - } - // construct operation result as a map of source UID to boolean or destination UID, depending on server support $map = $response->copyUidMap(); if ($map === []) { diff --git a/tests/php/Unit/CommandCompletionTest.php b/tests/php/Unit/CommandCompletionTest.php new file mode 100644 index 0000000..d493dbc --- /dev/null +++ b/tests/php/Unit/CommandCompletionTest.php @@ -0,0 +1,109 @@ +createStub(ConnectionInterface::class); + $connection->method('readLine')->willReturnCallback(static function () use (&$lines): string { + return array_shift($lines) ?? throw new \RuntimeException('Unexpected response read'); + }); + $connection->method('write')->willReturnCallback(function (string $wire) use (&$lines, $failure, $move, $failureStatus): void { + [$tag, $command] = explode(' ', trim($wire), 2); + $this->commands[] = $command; + $operation = str_starts_with($command, 'UID ') ? explode(' ', $command)[1] : explode(' ', $command)[0]; + if ($operation === 'CAPABILITY') { + $lines[] = '* CAPABILITY IMAP4rev1 UIDPLUS' . ($move ? ' MOVE' : '') . "\r\n"; + } + $status = $operation === $failure ? $failureStatus : 'OK'; + $text = $status === 'OK' ? 'Completed' : 'Denied'; + if ($status === 'OK' && in_array($operation, ['COPY', 'MOVE'], true)) { + $text = '[COPYUID 7 42 142] Completed'; + } + $lines[] = "$tag $status $text\r\n"; + }); + $factory = $this->createStub(ConnectionFactoryInterface::class); + $factory->method('create')->willReturn($connection); + $client = new Client($factory); + $client->connect(new ConnectionConfig('localhost')); + $this->commands = []; + return $client; + } + + private function service(Client $client): RemoteMailService + { + return new class($this->createStub(Service::class), $client) extends RemoteMailService { + public function __construct(Service $service, private readonly Client $client) + { + parent::__construct($service); + } + + public function imapClient(): Client + { + return $this->client; + } + }; + } + + public function testSuccessfulCommandReturnsCompletionText(): void + { + $result = $this->client()->perform(new CreateCommand('Archive')); + $this->assertInstanceOf(CommandCompletion::class, $result); + $this->assertSame('OK', $result->status()); + $this->assertSame('Completed', $result->text()); + } + + public static function failures(): iterable + { + foreach (['NO', 'BAD'] as $status) { + yield "create $status" => ['collectionCreate', ['Archive'], 'CREATE', false, ['CREATE "Archive"'], $status]; + yield "rename $status" => ['collectionRename', ['INBOX', 'Archive'], 'RENAME', false, ['RENAME "INBOX" "Archive"'], $status]; + yield "delete $status" => ['collectionDestroy', ['Archive'], 'DELETE', false, ['DELETE "Archive"'], $status]; + yield "copy $status" => ['entityCopy', ['Archive', 'INBOX', 42], 'COPY', false, ['SELECT "INBOX"', 'UID COPY 42 "Archive"'], $status]; + yield "move $status" => ['entityMove', ['Archive', 'INBOX', 42], 'MOVE', true, ['SELECT "INBOX"', 'UID MOVE 42 "Archive"'], $status]; + yield "fallback copy $status" => ['entityMove', ['Archive', 'INBOX', 42], 'COPY', false, ['SELECT "INBOX"', 'UID COPY 42 "Archive"'], $status]; + yield "fallback store $status" => ['entityMove', ['Archive', 'INBOX', 42], 'STORE', false, ['SELECT "INBOX"', 'UID COPY 42 "Archive"', 'UID STORE 42 +FLAGS.SILENT (\\Deleted)'], $status]; + } + } + + #[DataProvider('failures')] + public function testFailuresPropagateWithoutExecutingLaterCommands(string $method, array $arguments, string $failure, bool $move, array $commands, string $status): void + { + $service = $this->service($this->client($failure, $move, $status)); + try { + $service->$method(...$arguments); + $this->fail('Expected command failure'); + } catch (ImapException $exception) { + $this->assertSame($failure . ' failed: Denied', $exception->getMessage()); + } + $this->assertSame($commands, $this->commands); + } + + public function testMoveFallbackDeletesOnlyAfterCopySucceeds(): void + { + $result = $this->service($this->client())->entityMove('Archive', 'INBOX', 42); + $this->assertSame([42 => '142'], $result); + $this->assertSame([ + 'SELECT "INBOX"', + 'UID COPY 42 "Archive"', + 'UID STORE 42 +FLAGS.SILENT (\\Deleted)', + 'UID EXPUNGE 42', + ], $this->commands); + } +}