refactor: unify command complition checking

Signed-off-by: Sebastian Krupinski <krupinski01@gmail.com>
This commit is contained in:
2026-09-25 12:14:37 -04:00
parent 1b68c99521
commit 6a36e91cb4
21 changed files with 132 additions and 80 deletions
@@ -4,10 +4,10 @@ declare(strict_types=1);
namespace KTXM\ProviderImap\Client\Protocol\Command;
use KTXM\ProviderImap\Client\Protocol\CompletionChecker;
use KTXM\ProviderImap\Client\Protocol\Parser\ResponseCodeParser;
use KTXM\ProviderImap\Client\Protocol\StringEncoder;
use KTXM\ProviderImap\Client\ImapException;
use KTXM\ProviderImap\Client\CommandFailedException;
use KTXM\ProviderImap\Client\Protocol\RequestFrame;
use KTXM\ProviderImap\Client\Protocol\Response\ContinuationResponse;
use KTXM\ProviderImap\Client\Protocol\Response\TaggedResponse;
@@ -81,14 +81,7 @@ final class AppendCommand implements CommandInterface
}
if ($response instanceof TaggedResponse) {
if (!$response->isOk()) {
throw new CommandFailedException(
'APPEND',
$response->status(),
$response->text(),
$this->responseCodeParser->parse($response->text()),
);
}
CompletionChecker::assertSuccess($this->name(), $response);
return $this->parseAppendUid($response->text());
}
@@ -4,6 +4,7 @@ declare(strict_types=1);
namespace KTXM\ProviderImap\Client\Protocol\Command;
use KTXM\ProviderImap\Client\Protocol\CompletionChecker;
use KTXM\ProviderImap\Client\Result\CapabilityResult;
use KTXM\ProviderImap\Client\ImapException;
use KTXM\ProviderImap\Client\Protocol\RequestFrame;
@@ -49,9 +50,7 @@ final class CapabilityCommand implements CommandInterface
}
if ($response instanceof TaggedResponse) {
if (!$response->isOk()) {
throw new ImapException('CAPABILITY failed: ' . $response->text());
}
CompletionChecker::assertSuccess($this->name(), $response);
}
}
@@ -4,6 +4,7 @@ declare(strict_types=1);
namespace KTXM\ProviderImap\Client\Protocol\Command;
use KTXM\ProviderImap\Client\Protocol\CompletionChecker;
use KTXM\ProviderImap\Client\Protocol\StringEncoder;
use KTXM\ProviderImap\Client\Result\CommandCompletion;
use KTXM\ProviderImap\Client\ImapException;
@@ -48,9 +49,7 @@ final class CreateCommand implements CommandInterface
foreach ($responses as $response) {
if ($response instanceof TaggedResponse) {
if (!$response->isOk()) {
throw new ImapException('CREATE failed: ' . $response->text());
}
CompletionChecker::assertSuccess($this->name(), $response);
return new CommandCompletion($response->status(), $response->text());
}
@@ -4,6 +4,7 @@ declare(strict_types=1);
namespace KTXM\ProviderImap\Client\Protocol\Command;
use KTXM\ProviderImap\Client\Protocol\CompletionChecker;
use KTXM\ProviderImap\Client\Protocol\StringEncoder;
use KTXM\ProviderImap\Client\Result\CommandCompletion;
use KTXM\ProviderImap\Client\ImapException;
@@ -51,9 +52,7 @@ final class DeleteCommand implements CommandInterface
foreach ($responses as $response) {
if ($response instanceof TaggedResponse) {
if (!$response->isOk()) {
throw new ImapException('DELETE failed: ' . $response->text());
}
CompletionChecker::assertSuccess($this->name(), $response);
return new CommandCompletion($response->status(), $response->text());
}
@@ -4,6 +4,7 @@ declare(strict_types=1);
namespace KTXM\ProviderImap\Client\Protocol\Command;
use KTXM\ProviderImap\Client\Protocol\CompletionChecker;
use KTXM\ProviderImap\Client\Protocol\Command\Argument\MessageTarget;
use KTXM\ProviderImap\Client\Protocol\IdentifierMode;
use KTXM\ProviderImap\Client\ImapException;
@@ -89,11 +90,7 @@ final class ExpungeCommand implements CommandInterface
}
if ($response instanceof TaggedResponse) {
if (!$response->isOk()) {
throw new ImapException($this->sequenceSet === null
? 'EXPUNGE failed: ' . $response->text()
: 'UID EXPUNGE failed: ' . $response->text());
}
CompletionChecker::assertSuccess($this->sequenceSet === null ? 'EXPUNGE' : 'UID EXPUNGE', $response);
return $expunged;
}
+3 -6
View File
@@ -4,6 +4,7 @@ declare(strict_types=1);
namespace KTXM\ProviderImap\Client\Protocol\Command;
use KTXM\ProviderImap\Client\Protocol\CompletionChecker;
use KTXM\ProviderImap\Client\Protocol\StringEncoder;
use KTXM\ProviderImap\Client\Protocol\Parser\ListResponseParser;
use KTXM\ProviderImap\Client\Protocol\Parser\StatusResponseParser;
@@ -91,9 +92,7 @@ final class ListCommand implements CommandInterface
}
if ($response instanceof TaggedResponse) {
if (!$response->isOk()) {
throw new ImapException('LIST failed: ' . $response->text());
}
CompletionChecker::assertSuccess($this->name(), $response);
return;
}
@@ -127,9 +126,7 @@ final class ListCommand implements CommandInterface
}
if ($response instanceof TaggedResponse) {
if (!$response->isOk()) {
throw new ImapException('LIST failed: ' . $response->text());
}
CompletionChecker::assertSuccess($this->name(), $response);
foreach ($mailboxes as $mailbox) {
yield $mailbox;
+2 -3
View File
@@ -4,6 +4,7 @@ declare(strict_types=1);
namespace KTXM\ProviderImap\Client\Protocol\Command;
use KTXM\ProviderImap\Client\Protocol\CompletionChecker;
use KTXM\ProviderImap\Client\Protocol\StringEncoder;
use KTXM\ProviderImap\Client\Result\CommandCompletion;
use KTXM\ProviderImap\Client\ImapException;
@@ -48,9 +49,7 @@ final class LoginCommand implements CommandInterface
{
foreach ($responses as $response) {
if ($response instanceof TaggedResponse) {
if (!$response->isOk()) {
throw new ImapException('LOGIN failed: ' . $response->text());
}
CompletionChecker::assertSuccess($this->name(), $response);
$context->setState(SessionState::Authenticated);
@@ -4,6 +4,7 @@ declare(strict_types=1);
namespace KTXM\ProviderImap\Client\Protocol\Command;
use KTXM\ProviderImap\Client\Protocol\CompletionChecker;
use KTXM\ProviderImap\Client\Result\CommandCompletion;
use KTXM\ProviderImap\Client\ImapException;
use KTXM\ProviderImap\Client\Protocol\RequestFrame;
@@ -42,9 +43,7 @@ final class LogoutCommand implements CommandInterface
{
foreach ($responses as $response) {
if ($response instanceof TaggedResponse) {
if (!$response->isOk()) {
throw new ImapException('LOGOUT failed: ' . $response->text());
}
CompletionChecker::assertSuccess($this->name(), $response);
$context->setSelectedMailbox(null);
$context->setState(SessionState::Logout);
@@ -4,13 +4,13 @@ declare(strict_types=1);
namespace KTXM\ProviderImap\Client\Protocol\Command;
use KTXM\ProviderImap\Client\Protocol\CompletionChecker;
use KTXM\ProviderImap\Client\Protocol\Parser\ResponseCodeParser;
use KTXM\ProviderImap\Client\Protocol\StringEncoder;
use KTXM\ProviderImap\Client\Result\MessageTransferResult;
use KTXM\ProviderImap\Client\Protocol\Command\Argument\MessageTarget;
use KTXM\ProviderImap\Client\Protocol\IdentifierMode;
use KTXM\ProviderImap\Client\ImapException;
use KTXM\ProviderImap\Client\CommandFailedException;
use KTXM\ProviderImap\Client\Protocol\RequestFrame;
use KTXM\ProviderImap\Client\Protocol\Response\TaggedResponse;
use KTXM\ProviderImap\Client\Protocol\Response\UntaggedResponse;
@@ -114,14 +114,7 @@ final class MessageTransferCommand implements CommandInterface
$highestModSeq,
);
if (!$response->isOk()) {
throw new CommandFailedException(
$this->operation,
$response->status(),
$response->text(),
$this->responseCodeParser->parse($response->text()),
);
}
CompletionChecker::assertSuccess($this->name(), $response);
return new MessageTransferResult(
$response->status(),
+2 -3
View File
@@ -4,6 +4,7 @@ declare(strict_types=1);
namespace KTXM\ProviderImap\Client\Protocol\Command;
use KTXM\ProviderImap\Client\Protocol\CompletionChecker;
use KTXM\ProviderImap\Client\Result\CommandCompletion;
use KTXM\ProviderImap\Client\ImapException;
use KTXM\ProviderImap\Client\Protocol\RequestFrame;
@@ -44,9 +45,7 @@ final class NoopCommand implements CommandInterface
foreach ($responses as $response) {
if ($response instanceof TaggedResponse) {
if (!$response->isOk()) {
throw new ImapException('NOOP failed: ' . $response->text());
}
CompletionChecker::assertSuccess($this->name(), $response);
return new CommandCompletion($response->status(), $response->text());
}
@@ -4,6 +4,7 @@ declare(strict_types=1);
namespace KTXM\ProviderImap\Client\Protocol\Command;
use KTXM\ProviderImap\Client\Protocol\CompletionChecker;
use KTXM\ProviderImap\Client\Protocol\StringEncoder;
use KTXM\ProviderImap\Client\Result\CommandCompletion;
use KTXM\ProviderImap\Client\ImapException;
@@ -51,9 +52,7 @@ final class RenameCommand implements CommandInterface
{
foreach ($responses as $response) {
if ($response instanceof TaggedResponse) {
if (!$response->isOk()) {
throw new ImapException('RENAME failed: ' . $response->text());
}
CompletionChecker::assertSuccess($this->name(), $response);
if ($context->selectedMailbox() === $this->fromMailbox) {
$context->setSelectedMailbox($this->toMailbox);
@@ -4,6 +4,7 @@ declare(strict_types=1);
namespace KTXM\ProviderImap\Client\Protocol\Command;
use KTXM\ProviderImap\Client\Protocol\CompletionChecker;
use KTXM\ProviderImap\Client\Result\SearchResult;
use KTXM\ProviderImap\Client\Protocol\IdentifierMode;
use KTXM\ProviderImap\Client\ImapException;
@@ -74,9 +75,7 @@ final class SearchCommand implements CommandInterface
}
if ($response instanceof TaggedResponse) {
if (!$response->isOk()) {
throw new ImapException('SEARCH failed: ' . $response->text());
}
CompletionChecker::assertSuccess($this->name(), $response);
return new SearchResult($matches, $this->identifierMode);
}
@@ -4,6 +4,7 @@ declare(strict_types=1);
namespace KTXM\ProviderImap\Client\Protocol\Command;
use KTXM\ProviderImap\Client\Protocol\CompletionChecker;
use KTXM\ProviderImap\Client\Protocol\StringEncoder;
use KTXM\ProviderImap\Client\ImapException;
use KTXM\ProviderImap\Client\Mailbox;
@@ -76,9 +77,7 @@ final class SelectCommand implements CommandInterface
}
if ($response instanceof TaggedResponse) {
if (!$response->isOk()) {
throw new ImapException($this->name() . ' failed: ' . $response->text());
}
CompletionChecker::assertSuccess($this->name(), $response);
if (str_contains(strtoupper($response->text()), 'READ-ONLY')) {
$readOnly = true;
+2 -3
View File
@@ -4,6 +4,7 @@ declare(strict_types=1);
namespace KTXM\ProviderImap\Client\Protocol\Command;
use KTXM\ProviderImap\Client\Protocol\CompletionChecker;
use KTXM\ProviderImap\Client\Result\SortResult;
use KTXM\ProviderImap\Client\Protocol\IdentifierMode;
use KTXM\ProviderImap\Client\ImapException;
@@ -82,9 +83,7 @@ final class SortCommand implements CommandInterface
}
if ($response instanceof TaggedResponse) {
if (!$response->isOk()) {
throw new ImapException('SORT failed: ' . $response->text());
}
CompletionChecker::assertSuccess($this->name(), $response);
return new SortResult($matches, $this->identifierMode);
}
@@ -4,6 +4,7 @@ declare(strict_types=1);
namespace KTXM\ProviderImap\Client\Protocol\Command;
use KTXM\ProviderImap\Client\Protocol\CompletionChecker;
use KTXM\ProviderImap\Client\Result\CommandCompletion;
use KTXM\ProviderImap\Client\ImapException;
use KTXM\ProviderImap\Client\Protocol\RequestFrame;
@@ -38,9 +39,7 @@ final class StartTlsCommand implements CommandInterface
{
foreach ($responses as $response) {
if ($response instanceof TaggedResponse) {
if (!$response->isOk()) {
throw new ImapException('STARTTLS failed: ' . $response->text());
}
CompletionChecker::assertSuccess($this->name(), $response);
$context->connection()->upgradeToTls();
$context->replaceCapabilities();
@@ -4,6 +4,7 @@ declare(strict_types=1);
namespace KTXM\ProviderImap\Client\Protocol\Command;
use KTXM\ProviderImap\Client\Protocol\CompletionChecker;
use KTXM\ProviderImap\Client\Protocol\StringEncoder;
use KTXM\ProviderImap\Client\Protocol\Parser\StatusResponseParser;
use KTXM\ProviderImap\Client\Result\MailboxStatusResult;
@@ -71,9 +72,7 @@ final class StatusCommand implements CommandInterface
}
if ($response instanceof TaggedResponse) {
if (!$response->isOk()) {
throw new ImapException('STATUS failed: ' . $response->text());
}
CompletionChecker::assertSuccess($this->name(), $response);
return new MailboxStatusResult($mailbox, $items);
}
+2 -3
View File
@@ -4,6 +4,7 @@ declare(strict_types=1);
namespace KTXM\ProviderImap\Client\Protocol\Command;
use KTXM\ProviderImap\Client\Protocol\CompletionChecker;
use KTXM\ProviderImap\Client\Result\CommandCompletion;
use KTXM\ProviderImap\Client\Protocol\Command\Argument\MessageTarget;
use KTXM\ProviderImap\Client\Protocol\IdentifierMode;
@@ -90,9 +91,7 @@ final class StoreCommand implements CommandInterface
foreach ($responses as $response) {
if ($response instanceof TaggedResponse) {
if (!$response->isOk()) {
throw new ImapException('STORE failed: ' . $response->text());
}
CompletionChecker::assertSuccess($this->name(), $response);
return new CommandCompletion($response->status(), $response->text());
}
+31
View File
@@ -0,0 +1,31 @@
<?php
declare(strict_types=1);
namespace KTXM\ProviderImap\Client\Protocol;
use KTXM\ProviderImap\Client\CommandFailedException;
use KTXM\ProviderImap\Client\ImapException;
use KTXM\ProviderImap\Client\Protocol\Parser\ResponseCodeParser;
use KTXM\ProviderImap\Client\Protocol\Response\TaggedResponse;
final class CompletionChecker
{
public static function assertSuccess(string $command, TaggedResponse $response): void
{
if ($response->isOk()) {
return;
}
if (!in_array($response->status(), ['NO', 'BAD'], true)) {
throw new ImapException($command . ' received an invalid completion status: ' . $response->status());
}
throw new CommandFailedException(
$command,
$response->status(),
$response->text(),
(new ResponseCodeParser())->parse($response->text()),
);
}
}
+1 -3
View File
@@ -49,9 +49,7 @@ final class FetchResultReader
}
if ($response instanceof TaggedResponse) {
if (!$response->isOk()) {
throw new ImapException('FETCH failed: ' . $response->text());
}
CompletionChecker::assertSuccess('FETCH', $response);
return;
}
+7 -6
View File
@@ -129,9 +129,12 @@ final class ProtocolReader
if (str_starts_with($trimmed, $tag . ' ')) {
$parts = preg_split('/\s+/', $trimmed, 3) ?: [];
$status = strtoupper($parts[1] ?? '');
if ($status === 'NO' || $status === 'BAD') {
throw new ImapException(sprintf('FETCH command failed: %s', $trimmed));
}
CompletionChecker::assertSuccess('FETCH', new TaggedResponse(
$tag,
$status,
$parts[2] ?? '',
$trimmed,
));
return null; // Tagged OK without ever finding a literal → UID not found
}
@@ -145,9 +148,7 @@ final class ProtocolReader
$response = $this->readResponse();
if ($response instanceof TaggedResponse && $response->tag() === $tag) {
if (!$response->isOk()) {
throw new ImapException(sprintf('FETCH failed: %s', $response->text()));
}
CompletionChecker::assertSuccess('FETCH', $response);
return $response;
}
}
+58 -3
View File
@@ -7,6 +7,9 @@ namespace KTXT\ProviderImap\Tests\Unit;
use KTXM\ProviderImap\Client\{CommandFailedException, ConnectionConfig, ImapException};
use KTXM\ProviderImap\Client\Protocol\Command\{AppendCommand, CopyCommand, MoveCommand, CommandInterface};
use KTXM\ProviderImap\Client\Protocol\Command\Argument\MessageTarget;
use KTXM\ProviderImap\Client\Protocol\Command\Argument\ListReturnOptions;
use KTXM\ProviderImap\Client\Protocol\Command as Commands;
use KTXM\ProviderImap\Client\Protocol\{CompletionChecker, ProtocolReader};
use KTXM\ProviderImap\Client\Protocol\{ResponseStream, SessionContext};
use KTXM\ProviderImap\Client\Protocol\Response\TaggedResponse;
use KTXM\ProviderImap\Client\Transport\ConnectionInterface;
@@ -28,12 +31,32 @@ final class CommandFailedExceptionTest extends TestCase
'COPY' => new CopyCommand(MessageTarget::uid(42), 'Archive'),
'MOVE' => new MoveCommand(MessageTarget::uid(42), 'Archive'),
'APPEND' => new AppendCommand('Archive', 'body'),
'CAPABILITY' => new Commands\CapabilityCommand(),
'CREATE' => new Commands\CreateCommand('Archive'),
'DELETE' => new Commands\DeleteCommand('Archive'),
'RENAME' => new Commands\RenameCommand('INBOX', 'Archive'),
'LOGIN' => new Commands\LoginCommand('user', 'password'),
'LOGOUT' => new Commands\LogoutCommand(),
'NOOP' => new Commands\NoopCommand(),
'STARTTLS' => new Commands\StartTlsCommand(),
'SELECT' => new Commands\SelectCommand('INBOX', false),
'EXAMINE' => new Commands\SelectCommand('INBOX', true),
'STATUS' => new Commands\StatusCommand('INBOX'),
'SEARCH' => new Commands\SearchCommand(),
'SORT' => new Commands\SortCommand(['DATE']),
'STORE' => new Commands\StoreCommand(MessageTarget::uid(42), ['\\Seen']),
'EXPUNGE' => new Commands\ExpungeCommand(),
'UID EXPUNGE' => new Commands\ExpungeCommand(MessageTarget::uid(42)),
'LIST' => new Commands\ListCommand(),
'LIST-STATUS' => new Commands\ListCommand(returnOptions: ListReturnOptions::status('MESSAGES')),
'FETCH' => new Commands\FetchOneCommand(42),
'FETCH-MANY' => new Commands\FetchManyCommand(),
};
}
public static function failures(): iterable
{
foreach (['COPY', 'MOVE', 'APPEND'] as $command) {
foreach (['COPY', 'MOVE', 'APPEND', 'CAPABILITY', 'CREATE', 'DELETE', 'RENAME', 'LOGIN', 'LOGOUT', 'NOOP', 'STARTTLS', 'SELECT', 'EXAMINE', 'STATUS', 'SEARCH', 'SORT', 'STORE', 'EXPUNGE', 'UID EXPUNGE', 'LIST', 'LIST-STATUS', 'FETCH', 'FETCH-MANY'] as $command) {
foreach (['NO', 'BAD'] as $status) {
foreach ([
['[trycreate] Create destination first', ['name' => 'TRYCREATE', 'arguments' => [], 'text' => 'Create destination first']],
@@ -41,7 +64,7 @@ final class CommandFailedExceptionTest extends TestCase
['Denied', null],
['[Malformed', null],
] as [$text, $code]) {
yield [$command, $status, $text, $code];
yield $command . ' ' . $status . ' ' . $text => [$command, $status, $text, $code];
}
}
}
@@ -55,9 +78,13 @@ final class CommandFailedExceptionTest extends TestCase
});
try {
$this->command($command)->handle($responses, $this->context());
$result = $this->command($command)->handle($responses, $this->context());
if ($result instanceof \Generator) {
iterator_to_array($result);
}
$this->fail('Expected server failure');
} catch (ImapException $exception) {
$command = match ($command) { 'LIST-STATUS' => 'LIST', 'FETCH-MANY' => 'FETCH', default => $command };
$this->assertInstanceOf(CommandFailedException::class, $exception);
$this->assertSame($command . ' failed: ' . $text, $exception->getMessage());
$this->assertSame($command, $exception->command());
@@ -88,4 +115,32 @@ final class CommandFailedExceptionTest extends TestCase
});
$this->assertSame(42, $this->command('APPEND')->handle($responses, $this->context()));
}
public function testDownloadFailuresUseStructuredExceptions(): void
{
foreach (['readUntilFetchLiteral', 'readToEnd'] as $method) {
foreach (['NO', 'BAD'] as $status) {
$connection = $this->createStub(ConnectionInterface::class);
$connection->method('readLine')->willReturn("A1 $status [LIMIT 10] Denied\r\n");
try {
(new ProtocolReader($connection))->$method('A1');
$this->fail('Expected download failure');
} catch (CommandFailedException $exception) {
$this->assertSame('FETCH', $exception->command());
$this->assertSame($status, $exception->status());
$this->assertSame(['name' => 'LIMIT', 'arguments' => ['10'], 'text' => 'Denied'], $exception->responseCode());
}
}
}
}
public function testCheckerAcceptsOkAndRejectsMalformedStatusAsProtocolError(): void
{
CompletionChecker::assertSuccess('NOOP', new TaggedResponse('A1', 'OK', 'Done', 'A1 OK Done'));
try {
CompletionChecker::assertSuccess('NOOP', new TaggedResponse('A1', 'INVALID', 'Bad status', 'A1 INVALID Bad status'));
$this->fail('Expected malformed status error');
} catch (ImapException $exception) {
$this->assertNotInstanceOf(CommandFailedException::class, $exception);
}
}
}