From 8e5ba9eac435b7e427100ca612c46ea13508259d Mon Sep 17 00:00:00 2001 From: Jan-Niclas Loosen Date: Sat, 26 Sep 2026 17:40:32 +0200 Subject: [PATCH] refactor: WriteFailed named constructors, Rollback runner, Errors read-back helpers (H6) - WriteFailed::rolledBack()/partial() replace the 11-parameter constructor (now private); the hint/state of either error are plain $hint/$state. - Rollback::attempt(...steps) runs every undo step and collects failure messages; used by ContentCreate::discard, ContentUpdate::rollBack, MediaUpdate::rollBack and DiviSaver::restore. - Errors::clientMessage/codeOf/fieldsOf replace DiviReplaceTextAbility::reason, the JSON digging in MediaUpdateAbility::failure and ContentCreateAbility::fields. - Errors docblock documents the exception/WP_Error split. - ContentCreateAbility::discard takes the client reason as a parameter (both callers still pass REASON_EXTRAS; changing the Divi text needs a ruling). Error payloads unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/Abilities/Errors.php | 34 +++++++++ src/Abilities/Rollback.php | 33 +++++++++ src/Abilities/WriteFailed.php | 70 ++++++++++++++---- .../Divi/Pages/DiviReplaceTextAbility.php | 14 +--- src/Integrations/Divi/Pages/DiviSaver.php | 31 ++++---- src/Integrations/Media/MediaImporter.php | 19 ++--- src/Integrations/Media/MediaUpdateAbility.php | 33 ++++----- .../Content/ContentCreateAbility.php | 73 ++++++++----------- .../Content/ContentUpdateAbility.php | 35 +++------ .../WordPress/Menus/MenuWriter.php | 25 ++++--- .../Settings/WpSettingsUpdateAbility.php | 15 ++-- .../WordPress/Status/PostCloner.php | 12 +-- .../Status/PostReplaceWithCopyAbility.php | 22 +++--- .../WordPress/Status/StatusWrite.php | 13 ++-- tests/Unit/Abilities/AbilityRegistrarTest.php | 7 +- tests/Unit/Abilities/ErrorsTest.php | 20 +++++ tests/Unit/Abilities/RollbackTest.php | 54 ++++++++++++++ tests/Unit/Abilities/WriteFailedTest.php | 41 ++++++++--- .../Divi/Pages/DiviReplaceTextAbilityTest.php | 2 +- .../Divi/Pages/DiviWriterTest.php | 4 +- .../Media/MediaUploadAbilitiesTest.php | 2 +- .../Menus/MenuSetItemsAbilityTest.php | 4 +- .../Status/PostReplaceWithCopyAbilityTest.php | 2 +- 23 files changed, 358 insertions(+), 207 deletions(-) create mode 100644 src/Abilities/Rollback.php create mode 100644 tests/Unit/Abilities/RollbackTest.php diff --git a/src/Abilities/Errors.php b/src/Abilities/Errors.php index a5d1da7..3b6f6c2 100644 --- a/src/Abilities/Errors.php +++ b/src/Abilities/Errors.php @@ -12,6 +12,14 @@ use WP_Error; * mcp-adapter 0.5 turns a WP_Error into a tool result with isError: true but * forwards only the message text. The message therefore carries the whole * structured error as JSON: {code, message, hint, fields?, usages?, …} (spec §8.2). + * clientMessage(), codeOf() and fieldsOf() read such an error back (batch results that list + * per-item errors, merged validation errors). + * + * How failures travel: pure helpers (e.g. the Divi Compose classes) throw + * InvalidArgumentException, which the caller collects into `fields`; abilities return a + * WP_Error from this factory; writes that may have left something behind throw WriteFailed + * (rolledBack()/partial()), which the AbilityRegistrar maps via toError(); internal steps + * throw RuntimeException, which is logged and becomes internal_error. */ final class Errors { @@ -190,6 +198,32 @@ final class Errors ); } + /** The client message of an error (its JSON "message"), without the JSON around it. */ + public static function clientMessage(WP_Error $error): string + { + $data = json_decode($error->get_error_message(), true); + + return is_array($data) && is_string($data['message'] ?? null) ? $data['message'] : $error->get_error_message(); + } + + /** The error code as the client sees it (without the "lit_" prefix of the WP_Error code). */ + public static function codeOf(WP_Error $error): string + { + return (string) preg_replace('/^lit_/', '', (string) $error->get_error_code()); + } + + /** + * The `fields` of a validation error (input path => problem); [] when it has none. + * + * @return array + */ + public static function fieldsOf(WP_Error $error): array + { + $data = $error->get_error_data(); + + return is_array($data) && is_array($data['fields'] ?? null) ? $data['fields'] : []; + } + public static function internal(): WP_Error { return self::make( diff --git a/src/Abilities/Rollback.php b/src/Abilities/Rollback.php new file mode 100644 index 0000000..2a16166 --- /dev/null +++ b/src/Abilities/Rollback.php @@ -0,0 +1,33 @@ + The messages of the steps that threw, in step order. Never throws. + */ + public static function attempt(callable ...$steps): array + { + $failures = []; + foreach ($steps as $step) { + try { + $step(); + } catch (Throwable $e) { + $failures[] = $e->getMessage(); + } + } + + return $failures; + } +} diff --git a/src/Abilities/WriteFailed.php b/src/Abilities/WriteFailed.php index 4fde2cf..472acdc 100644 --- a/src/Abilities/WriteFailed.php +++ b/src/Abilities/WriteFailed.php @@ -12,27 +12,28 @@ use WP_Error; * Thrown by write paths when saving failed after something may already have been * written. getMessage() is the internal detail for the PHP error log; $reason is the * client-safe German text. The AbilityRegistrar logs the detail and returns toError(): - * lit_write_failed when everything was rolled back, lit_partial_write otherwise. - * $state/$hint customise the partial-write error for writes that are not about a post (see - * Errors::partialWrite()); $details are added to either error. $rolledBackHint replaces the - * default hint of the write_failed error, $rolledBackState its "Nichts wurde gespeichert." - * (for rollbacks that deliberately keep a backup or revision). + * lit_write_failed when everything was rolled back (rolledBack()), lit_partial_write + * otherwise (partial()). + * + * $detail is the log text (default: the cause's message, else the reason); $details are + * added to either error. For rolledBack(), $hint replaces the default hint and $state the + * "Nichts wurde gespeichert." (for rollbacks that deliberately keep a backup or revision, + * see Errors::notSavedKeeping()). For partial(), $state and $hint replace the post-oriented + * defaults for writes that are not about a post (see Errors::partialWrite()). */ final class WriteFailed extends RuntimeException { /** @param array $details Extra error details for the client. */ - public function __construct( + private function __construct( public readonly string $reason, public readonly bool $rolledBack, - public readonly ?int $postId = null, - public readonly ?int $revisionId = null, - ?Throwable $previous = null, - string $detail = '', - public readonly ?string $state = null, - public readonly ?string $hint = null, - public readonly array $details = [], - public readonly ?string $rolledBackHint = null, - public readonly ?string $rolledBackState = null + public readonly ?int $postId, + public readonly ?int $revisionId, + ?Throwable $previous, + string $detail, + public readonly ?string $state, + public readonly ?string $hint, + public readonly array $details ) { $internal = $detail !== '' ? $detail : ($previous !== null ? $previous->getMessage() : $reason); parent::__construct( @@ -42,10 +43,47 @@ final class WriteFailed extends RuntimeException ); } + /** + * Everything was restored (lit_write_failed). $postId/$revisionId only name the entry for + * callers that log it; the client error does not carry them. + * + * @param array $details + */ + public static function rolledBack( + string $reason, + ?Throwable $cause = null, + string $detail = '', + ?string $hint = null, + ?string $state = null, + array $details = [], + ?int $postId = null, + ?int $revisionId = null + ): self { + return new self($reason, true, $postId, $revisionId, $cause, $detail, $state, $hint, $details); + } + + /** + * The rollback was incomplete – something may have been saved (lit_partial_write). + * + * @param array $details + */ + public static function partial( + string $reason, + ?Throwable $cause = null, + string $detail = '', + ?int $postId = null, + ?int $revisionId = null, + ?string $state = null, + ?string $hint = null, + array $details = [] + ): self { + return new self($reason, false, $postId, $revisionId, $cause, $detail, $state, $hint, $details); + } + public function toError(): WP_Error { return $this->rolledBack - ? Errors::writeFailed($this->reason, $this->details, $this->rolledBackHint, $this->rolledBackState) + ? Errors::writeFailed($this->reason, $this->details, $this->hint, $this->state) : Errors::partialWrite( $this->reason, $this->postId, diff --git a/src/Integrations/Divi/Pages/DiviReplaceTextAbility.php b/src/Integrations/Divi/Pages/DiviReplaceTextAbility.php index f7189da..01552fa 100644 --- a/src/Integrations/Divi/Pages/DiviReplaceTextAbility.php +++ b/src/Integrations/Divi/Pages/DiviReplaceTextAbility.php @@ -186,7 +186,7 @@ final class DiviReplaceTextAbility implements Ability $divi = $this->loader->load($postId, true); $problem = $divi instanceof WP_Error ? $divi : $this->loader->writable($divi); if ($problem instanceof WP_Error) { - $skipped[] = ['post_id' => $postId, 'reason' => self::reason($problem)]; + $skipped[] = ['post_id' => $postId, 'reason' => Errors::clientMessage($problem)]; continue; } @@ -320,7 +320,7 @@ final class DiviReplaceTextAbility implements Ability } catch (WriteFailed $e) { self::log($postId, $e); - return ['post_id' => $postId, 'ok' => false, 'error' => self::reason($e->toError())]; + return ['post_id' => $postId, 'ok' => false, 'error' => Errors::clientMessage($e->toError())]; } catch (Throwable $e) { self::log($postId, $e); @@ -343,7 +343,7 @@ final class DiviReplaceTextAbility implements Ability $postId = $input['post_id']; $page = $this->writer->page($input); if ($page instanceof WP_Error) { - return ['post_id' => $postId, 'ok' => false, 'error' => self::reason($page)]; + return ['post_id' => $postId, 'ok' => false, 'error' => Errors::clientMessage($page)]; } $post = $page->post; @@ -475,14 +475,6 @@ final class DiviReplaceTextAbility implements Ability : 'Knoten nicht lesbar (Attribute kein JSON-Objekt) – im Divi-Builder bearbeiten'; } - /** The client message of an error (its JSON "message"), without the JSON around it. */ - private static function reason(WP_Error $error): string - { - $data = json_decode($error->get_error_message(), true); - - return is_array($data) && is_string($data['message'] ?? null) ? $data['message'] : $error->get_error_message(); - } - private static function log(int $postId, Throwable $e): void { Log::failure(sprintf('lit/divi-replace-text on post %d', $postId), $e); diff --git a/src/Integrations/Divi/Pages/DiviSaver.php b/src/Integrations/Divi/Pages/DiviSaver.php index 0ee0c78..f20b3a2 100644 --- a/src/Integrations/Divi/Pages/DiviSaver.php +++ b/src/Integrations/Divi/Pages/DiviSaver.php @@ -6,6 +6,7 @@ namespace LIT\MCP\Integrations\Divi\Pages; use LIT\MCP\Abilities\Errors; use LIT\MCP\Abilities\FollowUp; +use LIT\MCP\Abilities\Rollback; use LIT\MCP\Abilities\WriteFailed; use LIT\MCP\Integrations\WordPress\PostGuard; use LIT\MCP\Integrations\WordPress\RevisionPoint; @@ -78,14 +79,12 @@ class DiviSaver if ($result instanceof WP_Error || $result === 0) { $message = $result instanceof WP_Error ? $result->get_error_message() : 'returned 0'; - throw new WriteFailed( + throw WriteFailed::rolledBack( self::REASON, - true, - $postId, - $revisionId, - null, - sprintf('wp_update_post(%d) refused: %s', $postId, $message), - rolledBackState: self::keeping($revisionId) + detail: sprintf('wp_update_post(%d) refused: %s', $postId, $message), + state: self::keeping($revisionId), + postId: $postId, + revisionId: $revisionId ); } @@ -172,7 +171,7 @@ class DiviSaver private function restore(int $postId, string $before, ?int $revisionId, Throwable $cause): never { $detail = sprintf('Saving Divi content of post %d failed: %s', $postId, $cause->getMessage()); - try { + $failures = Rollback::attempt(function () use ($postId, $before): void { $current = get_post($postId); if ($current instanceof WP_Post && PostGuard::missedSchedule($current)) { throw new RuntimeException('restoring would publish a missed schedule'); @@ -185,19 +184,19 @@ class DiviSaver if ($restored instanceof WP_Error || $restored === 0) { throw new RuntimeException('previous content could not be written back'); } - } catch (Throwable $e) { - $detail .= '; rollback: ' . $e->getMessage(); - throw new WriteFailed(self::REASON, false, $postId, $revisionId, $cause, $detail); + }); + if ($failures !== []) { + $detail .= '; rollback: ' . $failures[0]; + throw WriteFailed::partial(self::REASON, $cause, $detail, $postId, $revisionId); } - throw new WriteFailed( + throw WriteFailed::rolledBack( self::REASON, - true, - $postId, - $revisionId, $cause, $detail, - rolledBackState: self::keeping($revisionId) + state: self::keeping($revisionId), + postId: $postId, + revisionId: $revisionId ); } diff --git a/src/Integrations/Media/MediaImporter.php b/src/Integrations/Media/MediaImporter.php index fc0838a..3cab21e 100644 --- a/src/Integrations/Media/MediaImporter.php +++ b/src/Integrations/Media/MediaImporter.php @@ -261,20 +261,17 @@ class MediaImporter } if ($deleted) { - throw new WriteFailed($reason, true, null, null, null, $detail); + throw WriteFailed::rolledBack($reason, detail: $detail); } - throw new WriteFailed( + throw WriteFailed::partial( $reason, - false, - $attachmentId, - null, - null, - $detail . '; attachment could not be deleted', - sprintf('Medium %d wurde trotzdem angelegt, aber ohne Alt-Text.', $attachmentId), - 'Den Aufruf nicht wiederholen (sonst liegt die Datei doppelt vor). Medium mit lit/media-get prüfen, den ' - . 'Alt-Text in der Mediathek nachtragen und dem Nutzer Bescheid geben.', - ['attachment_id' => $attachmentId] + detail: $detail . '; attachment could not be deleted', + postId: $attachmentId, + state: sprintf('Medium %d wurde trotzdem angelegt, aber ohne Alt-Text.', $attachmentId), + hint: 'Den Aufruf nicht wiederholen (sonst liegt die Datei doppelt vor). Medium mit lit/media-get prüfen, ' + . 'den Alt-Text in der Mediathek nachtragen und dem Nutzer Bescheid geben.', + details: ['attachment_id' => $attachmentId] ); } diff --git a/src/Integrations/Media/MediaUpdateAbility.php b/src/Integrations/Media/MediaUpdateAbility.php index 7b677cd..d784f40 100644 --- a/src/Integrations/Media/MediaUpdateAbility.php +++ b/src/Integrations/Media/MediaUpdateAbility.php @@ -7,6 +7,7 @@ namespace LIT\MCP\Integrations\Media; use LIT\MCP\Abilities\Ability; use LIT\MCP\Abilities\Errors; use LIT\MCP\Abilities\FollowUp; +use LIT\MCP\Abilities\Rollback; use LIT\MCP\Abilities\ToolResult; use LIT\MCP\Abilities\WriteFailed; use LIT\MCP\ChangeLog\ChangeLog; @@ -266,24 +267,20 @@ final class MediaUpdateAbility implements Ability $unrestored = []; $failures = []; foreach (array_reverse($written) as $entry) { - try { - $this->writer->restore($entry['snapshot']); - } catch (Throwable $e) { + $failed = Rollback::attempt(fn () => $this->writer->restore($entry['snapshot'])); + if ($failed !== []) { $unrestored[] = $entry['attachment']; - $failures[] = $e->getMessage(); + $failures = array_merge($failures, $failed); } } $detail = 'Media update failed: ' . $cause->getMessage(); if ($unrestored === []) { - throw new WriteFailed( + throw WriteFailed::rolledBack( self::REASON, - true, - null, - null, $cause, $detail, - rolledBackHint: 'Das Zurücksetzen hat modified der Medien geändert: vor einem neuen Versuch die Medien ' + hint: 'Das Zurücksetzen hat modified der Medien geändert: vor einem neuen Versuch die Medien ' . 'neu lesen (lit/media-get oder lit/media-search, direkt: lit-media-search) und den Stapel mit ' . 'dem aktuellen modified senden; tritt der Fehler wieder auf, den Administrator informieren ' . '(Details im PHP-Error-Log).' @@ -301,17 +298,14 @@ final class MediaUpdateAbility implements Ability ); } - throw new WriteFailed( + throw WriteFailed::partial( self::REASON, - false, - null, - null, $cause, $detail . '; rollback incomplete: ' . implode('; ', $failures), - sprintf('Möglicherweise geändert: Medien %s (siehe changed_ids).', implode(', ', $ids)), - 'Den Aufruf nicht einfach wiederholen. Die unter changed_ids genannten Medien mit lit/media-get prüfen, ' - . 'die Metadaten ggf. korrigieren und dem Nutzer Bescheid geben.', - ['changed_ids' => $ids] + state: sprintf('Möglicherweise geändert: Medien %s (siehe changed_ids).', implode(', ', $ids)), + hint: 'Den Aufruf nicht einfach wiederholen. Die unter changed_ids genannten Medien mit lit/media-get ' + . 'prüfen, die Metadaten ggf. korrigieren und dem Nutzer Bescheid geben.', + details: ['changed_ids' => $ids] ); } @@ -374,12 +368,11 @@ final class MediaUpdateAbility implements Ability */ private static function failure(int $index, array $item, WP_Error $error): array { - $payload = json_decode($error->get_error_message(), true); return [ 'index' => $index, 'id' => PostGuard::id($item['id'] ?? null), - 'code' => (string) preg_replace('/^lit_/', '', (string) $error->get_error_code()), - 'error' => is_array($payload) ? (string) ($payload['message'] ?? '') : $error->get_error_message(), + 'code' => Errors::codeOf($error), + 'error' => Errors::clientMessage($error), ]; } diff --git a/src/Integrations/WordPress/Content/ContentCreateAbility.php b/src/Integrations/WordPress/Content/ContentCreateAbility.php index 3211cac..d9735f1 100644 --- a/src/Integrations/WordPress/Content/ContentCreateAbility.php +++ b/src/Integrations/WordPress/Content/ContentCreateAbility.php @@ -7,12 +7,14 @@ namespace LIT\MCP\Integrations\WordPress\Content; use LIT\MCP\Abilities\Ability; use LIT\MCP\Abilities\Errors; use LIT\MCP\Abilities\Rev; +use LIT\MCP\Abilities\Rollback; use LIT\MCP\Abilities\ToolResult; use LIT\MCP\Abilities\WriteFailed; use LIT\MCP\ChangeLog\ChangeLog; use LIT\MCP\Integrations\WordPress\PostChangeResult; use LIT\MCP\Integrations\WordPress\PostGuard; use LIT\MCP\Integrations\WordPress\Schema\ContentSchema; +use RuntimeException; use Throwable; use WP_Error; use WP_Post; @@ -137,7 +139,7 @@ final class ContentCreateAbility implements Ability // A hook threw – the row may or may not exist and its ID is unknown; nothing is // searched or deleted, the client is told to check (lit_partial_write without id). $this->logPartial($postType, '', 'Anlegen abgebrochen; Entwurf möglicherweise vorhanden'); - throw new WriteFailed(self::REASON_INSERT, false, null, null, $e); + throw WriteFailed::partial(self::REASON_INSERT, $e); } if ($postId instanceof WP_Error) { return Errors::invalidInput('WordPress hat den Eintrag abgelehnt: ' . $postId->get_error_message()); @@ -153,7 +155,7 @@ final class ContentCreateAbility implements Ability try { $layout = $withDivi ? $this->divi?->write($postId, $postType, self::sections($input)) : null; } catch (Throwable $e) { - $this->discard($postId, $postType, $e); + $this->discard($postId, $postType, $e, self::REASON_EXTRAS); } if ($layout instanceof WP_Error) { return $this->refuse($postId, $postType, $layout); @@ -162,7 +164,7 @@ final class ContentCreateAbility implements Ability try { $this->writer->writeExtras($postId, $postType, $prepared, $input); } catch (Throwable $e) { - $this->discard($postId, $postType, $e); + $this->discard($postId, $postType, $e, self::REASON_EXTRAS); } $warnings = array_merge($prepared['warnings'], $layout['warnings'] ?? []); @@ -207,7 +209,7 @@ final class ContentCreateAbility implements Ability return $prepared instanceof WP_Error ? $prepared : $divi; } - $fields = array_merge(self::fields($prepared), self::fields($divi)); + $fields = array_merge(Errors::fieldsOf($prepared), Errors::fieldsOf($divi)); if ($fields === []) { return $prepared; } @@ -217,14 +219,6 @@ final class ContentCreateAbility implements Ability : Errors::invalidInput('Eingaben sind ungültig; nichts wurde angelegt.', $fields); } - /** @return array */ - private static function fields(WP_Error $error): array - { - $data = $error->get_error_data(); - - return is_array($data) && is_array($data['fields'] ?? null) ? $data['fields'] : []; - } - /** * @param array $input * @param list $warnings @@ -254,42 +248,38 @@ final class ContentCreateAbility implements Ability /** * Rolls the brand-new draft back: terms it created, then the draft itself (force – - * it cannot have been used anywhere yet). + * it cannot have been used anywhere yet). $reason is the client text of the error. * * @throws WriteFailed Always. */ - private function discard(int $postId, string $postType, Throwable $cause): never + private function discard(int $postId, string $postType, Throwable $cause, string $reason): never { - $problems = []; - try { - $this->writer->deleteCreatedTerms(); - } catch (Throwable $e) { - $problems[] = $e->getMessage(); - } - - try { - if (! wp_delete_post($postId, true)) { - $problems[] = sprintf('draft %d could not be deleted', $postId); + $problems = Rollback::attempt( + fn () => $this->writer->deleteCreatedTerms(), + static function () use ($postId): void { + try { + $deleted = wp_delete_post($postId, true); + } catch (Throwable $e) { + throw new RuntimeException( + sprintf('draft %d could not be deleted: %s', $postId, $e->getMessage()), + 0, + $e + ); + } + if (! $deleted) { + throw new RuntimeException(sprintf('draft %d could not be deleted', $postId)); + } } - } catch (Throwable $e) { - $problems[] = sprintf('draft %d could not be deleted: %s', $postId, $e->getMessage()); - } + ); $detail = sprintf('Extras of new post %d could not be written: %s', $postId, $cause->getMessage()); if ($problems === []) { - throw new WriteFailed(self::REASON_EXTRAS, true, null, null, $cause, $detail); + throw WriteFailed::rolledBack($reason, $cause, $detail); } $summary = sprintf('Entwurf %d: Anlegen nur teilweise zurückgenommen', $postId); $this->logPartial($postType, (string) $postId, $summary); - throw new WriteFailed( - self::REASON_EXTRAS, - false, - $postId, - null, - $cause, - $detail . '; rollback: ' . implode('; ', $problems) - ); + throw WriteFailed::partial($reason, $cause, $detail . '; rollback: ' . implode('; ', $problems), $postId); } /** @@ -311,15 +301,12 @@ final class ContentCreateAbility implements Ability if (! $deleted) { $summary = sprintf('Entwurf %d: Anlegen nicht zurückgenommen', $postId); $this->logPartial($postType, (string) $postId, $summary); - $fields = $denied instanceof WP_Error ? self::fields($denied) : $denied; - throw new WriteFailed( + $fields = $denied instanceof WP_Error ? Errors::fieldsOf($denied) : $denied; + throw WriteFailed::partial( ($denied instanceof WP_Error ? 'Ungültiger Divi-Inhalt: ' : 'Keine Berechtigung für ') . implode(', ', array_keys($fields)) . '.', - false, - $postId, - null, - null, - sprintf('New post %d refused after the insert; draft could not be deleted', $postId) + detail: sprintf('New post %d refused after the insert; draft could not be deleted', $postId), + postId: $postId ); } diff --git a/src/Integrations/WordPress/Content/ContentUpdateAbility.php b/src/Integrations/WordPress/Content/ContentUpdateAbility.php index 2c6dc7f..1178a9a 100644 --- a/src/Integrations/WordPress/Content/ContentUpdateAbility.php +++ b/src/Integrations/WordPress/Content/ContentUpdateAbility.php @@ -7,6 +7,7 @@ namespace LIT\MCP\Integrations\WordPress\Content; use LIT\MCP\Abilities\Ability; use LIT\MCP\Abilities\Errors; use LIT\MCP\Abilities\Rev; +use LIT\MCP\Abilities\Rollback; use LIT\MCP\Abilities\ToolResult; use LIT\MCP\Abilities\WriteFailed; use LIT\MCP\ChangeLog\ChangeLog; @@ -218,32 +219,21 @@ final class ContentUpdateAbility implements Ability private function rollBack(WP_Post $post, array $before, ?int $revisionId, array $snapshot, Throwable $cause): never { $postId = (int) $post->ID; - $steps = [ + $failures = Rollback::attempt( fn () => $this->writer->deleteCreatedTerms(), fn () => $this->writer->restore($postId, (string) $post->post_type, $snapshot), - fn () => $this->restoreRow($post, $before, $revisionId), - ]; - $failures = []; - foreach ($steps as $step) { - try { - $step(); - } catch (Throwable $e) { - $failures[] = $e->getMessage(); - } - } + fn () => $this->restoreRow($post, $before, $revisionId) + ); $detail = sprintf('Update of post %d failed: %s', $postId, $cause->getMessage()); if ($failures === []) { - throw new WriteFailed( + throw WriteFailed::rolledBack( self::REASON, - true, - $postId, - $revisionId, $cause, $detail, - rolledBackState: $revisionId === null - ? null - : Errors::notSavedKeeping(sprintf('Revision %d', $revisionId)) + state: $revisionId === null ? null : Errors::notSavedKeeping(sprintf('Revision %d', $revisionId)), + postId: $postId, + revisionId: $revisionId ); } @@ -254,13 +244,12 @@ final class ContentUpdateAbility implements Ability sprintf('"%s": Änderung nur teilweise zurückgenommen', $post->post_title), $revisionId ); - throw new WriteFailed( + throw WriteFailed::partial( self::REASON, - false, - $postId, - $revisionId, $cause, - $detail . '; rollback incomplete: ' . implode('; ', $failures) + $detail . '; rollback incomplete: ' . implode('; ', $failures), + $postId, + $revisionId ); } diff --git a/src/Integrations/WordPress/Menus/MenuWriter.php b/src/Integrations/WordPress/Menus/MenuWriter.php index c0806b9..81a842d 100644 --- a/src/Integrations/WordPress/Menus/MenuWriter.php +++ b/src/Integrations/WordPress/Menus/MenuWriter.php @@ -454,19 +454,22 @@ class MenuWriter . ($this->deleted !== [] ? '; already deleted: ' . implode(', ', $this->deleted) : '') . ($problems !== [] ? '; rollback: ' . implode('; ', $problems) : ''); - throw new WriteFailed( - 'Das Menü konnte nicht gespeichert werden.', - $rolledBack, - null, - null, + $reason = 'Das Menü konnte nicht gespeichert werden.'; + $details = ['menu_id' => $menuId, 'previous_items' => MenuTree::build(array_values($current))]; + if ($rolledBack) { + throw WriteFailed::rolledBack($reason, $cause, $detail, details: $details); + } + + throw WriteFailed::partial( + $reason, $cause, $detail, - sprintf('Menü %d ist möglicherweise teilweise geändert worden.', $menuId), - 'Den Aufruf nicht einfach wiederholen. Zuerst den aktuellen Stand mit lit/menu-get prüfen und ihn ggf. ' - . 'mit lit/menu-set-items (beide über mcp-adapter-execute-ability) anhand von previous_items (Stand ' - . 'vorher) wiederherstellen; entfernte Einträge müssen dabei neu angelegt werden. Dem Nutzer Bescheid ' - . 'geben.', - ['menu_id' => $menuId, 'previous_items' => MenuTree::build(array_values($current))] + state: sprintf('Menü %d ist möglicherweise teilweise geändert worden.', $menuId), + hint: 'Den Aufruf nicht einfach wiederholen. Zuerst den aktuellen Stand mit lit/menu-get prüfen und ihn ' + . 'ggf. mit lit/menu-set-items (beide über mcp-adapter-execute-ability) anhand von previous_items ' + . '(Stand vorher) wiederherstellen; entfernte Einträge müssen dabei neu angelegt werden. Dem Nutzer ' + . 'Bescheid geben.', + details: $details ); } diff --git a/src/Integrations/WordPress/Settings/WpSettingsUpdateAbility.php b/src/Integrations/WordPress/Settings/WpSettingsUpdateAbility.php index 6ae6c92..761f674 100644 --- a/src/Integrations/WordPress/Settings/WpSettingsUpdateAbility.php +++ b/src/Integrations/WordPress/Settings/WpSettingsUpdateAbility.php @@ -238,7 +238,7 @@ final class WpSettingsUpdateAbility implements Ability $reason = 'Eine Einstellung konnte nicht gespeichert werden.'; if ($stuck === []) { - throw new WriteFailed($reason, true, null, null, $cause); + throw WriteFailed::rolledBack($reason, $cause); } $this->log->recordPartial( @@ -247,17 +247,14 @@ final class WpSettingsUpdateAbility implements Ability implode(',', $stuck), 'Einstellungen nur teilweise zurückgenommen: ' . implode(', ', $stuck) ); - throw new WriteFailed( + throw WriteFailed::partial( $reason, - false, - null, - null, $cause, $cause->getMessage() . '; not restored: ' . implode(', ', $stuck), - sprintf('Möglicherweise auf dem neuen, nicht vollständigen Stand: %s.', implode(', ', $stuck)), - 'Aktuelle Werte über lit/wp-settings-get (mcp-adapter-execute-ability) prüfen und ggf. erneut auf die ' - . 'gewünschten Werte setzen.', - ['keys' => $stuck] + state: sprintf('Möglicherweise auf dem neuen, nicht vollständigen Stand: %s.', implode(', ', $stuck)), + hint: 'Aktuelle Werte über lit/wp-settings-get (mcp-adapter-execute-ability) prüfen und ggf. erneut auf ' + . 'die gewünschten Werte setzen.', + details: ['keys' => $stuck] ); } diff --git a/src/Integrations/WordPress/Status/PostCloner.php b/src/Integrations/WordPress/Status/PostCloner.php index ed4a991..8d6b5cd 100644 --- a/src/Integrations/WordPress/Status/PostCloner.php +++ b/src/Integrations/WordPress/Status/PostCloner.php @@ -182,13 +182,9 @@ class PostCloner if ($cloneId instanceof WP_Error || (int) $cloneId === 0) { $reason = $cloneId instanceof WP_Error ? $cloneId->get_error_message() : 'unknown error'; - throw new WriteFailed( + throw WriteFailed::rolledBack( $label . ' konnte nicht angelegt werden.', - true, - null, - null, - null, - sprintf('Clone (%s) of post %d could not be created: %s', $markerKey, $originalId, $reason) + detail: sprintf('Clone (%s) of post %d could not be created: %s', $markerKey, $originalId, $reason) ); } @@ -216,10 +212,10 @@ class PostCloner $deleted = wp_delete_post($cloneId, true); $detail = sprintf('Clone %d of post %d failed: %s', $cloneId, (int) $original->ID, $cause->getMessage()); if ($deleted) { - throw new WriteFailed($reason, true, null, null, $cause, $detail . '; clone deleted'); + throw WriteFailed::rolledBack($reason, $cause, $detail . '; clone deleted'); } - throw new WriteFailed($reason, false, $cloneId, null, $cause, $detail . '; clone could not be deleted'); + throw WriteFailed::partial($reason, $cause, $detail . '; clone could not be deleted', $cloneId); } /** diff --git a/src/Integrations/WordPress/Status/PostReplaceWithCopyAbility.php b/src/Integrations/WordPress/Status/PostReplaceWithCopyAbility.php index 69a3646..57cd164 100644 --- a/src/Integrations/WordPress/Status/PostReplaceWithCopyAbility.php +++ b/src/Integrations/WordPress/Status/PostReplaceWithCopyAbility.php @@ -126,7 +126,7 @@ final class PostReplaceWithCopyAbility implements Ability try { $snapshot = $this->cloner->snapshot($original); } catch (Throwable $e) { - throw new WriteFailed(self::REASON, true, $originalId, null, $e); + throw WriteFailed::rolledBack(self::REASON, $e, postId: $originalId); } // Throws WriteFailed (backup deleted again) before anything touches the original. @@ -275,13 +275,12 @@ final class PostReplaceWithCopyAbility implements Ability sprintf('"%s": Austausch nur teilweise zurückgenommen', $original->post_title), $revisionId ); - throw new WriteFailed( + throw WriteFailed::partial( self::REASON, - false, - $originalId, - $revisionId, $cause, $detail . '; rollback incomplete: ' . $e->getMessage(), + $originalId, + $revisionId, sprintf( 'Eintrag %d ist möglicherweise teilweise geändert worden (Stand vorher: Revision %s); die ' . 'vollständige Sicherung des vorherigen Stands liegt als Eintrag %d im Papierkorb.', @@ -297,17 +296,14 @@ final class PostReplaceWithCopyAbility implements Ability ); } - throw new WriteFailed( + throw WriteFailed::rolledBack( self::REASON, - true, - $originalId, - $revisionId, $cause, $detail, - null, - null, - $details, - rolledBackState: Errors::notSavedKeeping(sprintf('Sicherung %d im Papierkorb', $backupId)) + state: Errors::notSavedKeeping(sprintf('Sicherung %d im Papierkorb', $backupId)), + details: $details, + postId: $originalId, + revisionId: $revisionId ); } diff --git a/src/Integrations/WordPress/Status/StatusWrite.php b/src/Integrations/WordPress/Status/StatusWrite.php index c7afc40..94f47a0 100644 --- a/src/Integrations/WordPress/Status/StatusWrite.php +++ b/src/Integrations/WordPress/Status/StatusWrite.php @@ -33,16 +33,13 @@ final class StatusWrite sprintf('"%s": Statusänderung abgebrochen, Stand unklar', $post->post_title) ); - throw new WriteFailed( + throw WriteFailed::partial( $reason, - false, - $postId, - null, $e, - '', - sprintf('Status von Eintrag %d ist möglicherweise schon geändert worden.', $postId), - 'Den Aufruf nicht einfach wiederholen. Zuerst mit lit/content-get (direkt: lit-content-get) Status ' - . 'und Inhalt prüfen und dem Nutzer Bescheid geben.' + postId: $postId, + state: sprintf('Status von Eintrag %d ist möglicherweise schon geändert worden.', $postId), + hint: 'Den Aufruf nicht einfach wiederholen. Zuerst mit lit/content-get (direkt: lit-content-get) ' + . 'Status und Inhalt prüfen und dem Nutzer Bescheid geben.' ); } } diff --git a/tests/Unit/Abilities/AbilityRegistrarTest.php b/tests/Unit/Abilities/AbilityRegistrarTest.php index 4837b58..98a43df 100644 --- a/tests/Unit/Abilities/AbilityRegistrarTest.php +++ b/tests/Unit/Abilities/AbilityRegistrarTest.php @@ -182,7 +182,12 @@ final class AbilityRegistrarTest extends TestCase $previous = ini_set('error_log', $log); $ability = $this->ability(); $ability->expects('execute')->andThrow( - new WriteFailed('Felder konnten nicht gespeichert werden.', false, 5, 811, new RuntimeException('meta /var/www')) + WriteFailed::partial( + 'Felder konnten nicht gespeichert werden.', + new RuntimeException('meta /var/www'), + postId: 5, + revisionId: 811 + ) ); try { diff --git a/tests/Unit/Abilities/ErrorsTest.php b/tests/Unit/Abilities/ErrorsTest.php index 9eda029..b41b5a1 100644 --- a/tests/Unit/Abilities/ErrorsTest.php +++ b/tests/Unit/Abilities/ErrorsTest.php @@ -114,4 +114,24 @@ final class ErrorsTest extends TestCase self::assertStringContainsString('Entwurf', $payload['message']); self::assertArrayNotHasKey('post_id', $payload); } + + public function testClientMessageReadsTheJsonMessageBack(): void + { + self::assertSame('Titel fehlt.', Errors::clientMessage(Errors::invalidInput('Titel fehlt.'))); + self::assertSame('WordPress sagt nein', Errors::clientMessage(new \WP_Error('db', 'WordPress sagt nein'))); + self::assertSame('{"code":"x"}', Errors::clientMessage(new \WP_Error('db', '{"code":"x"}'))); + } + + public function testCodeOfDropsTheLitPrefixOnly(): void + { + self::assertSame('validation_failed', Errors::codeOf(Errors::invalidInput('x'))); + self::assertSame('db_error', Errors::codeOf(new \WP_Error('db_error', 'x'))); + } + + public function testFieldsOfReturnsTheFieldsOrNothing(): void + { + self::assertSame(['title' => 'Pflichtfeld'], Errors::fieldsOf(Errors::invalidInput('x', ['title' => 'Pflichtfeld']))); + self::assertSame([], Errors::fieldsOf(Errors::invalidInput('x'))); + self::assertSame([], Errors::fieldsOf(new \WP_Error('db', 'x', 'kein Array'))); + } } diff --git a/tests/Unit/Abilities/RollbackTest.php b/tests/Unit/Abilities/RollbackTest.php new file mode 100644 index 0000000..fed8579 --- /dev/null +++ b/tests/Unit/Abilities/RollbackTest.php @@ -0,0 +1,54 @@ + 'egal', static fn (): bool => false)); + } +} diff --git a/tests/Unit/Abilities/WriteFailedTest.php b/tests/Unit/Abilities/WriteFailedTest.php index e062e62..b4fd958 100644 --- a/tests/Unit/Abilities/WriteFailedTest.php +++ b/tests/Unit/Abilities/WriteFailedTest.php @@ -13,7 +13,7 @@ final class WriteFailedTest extends TestCase public function testCleanRollbackMapsToWriteFailed(): void { $cause = new RuntimeException('meta kaputt'); - $failure = new WriteFailed('Felder konnten nicht gespeichert werden.', true, 5, null, $cause); + $failure = WriteFailed::rolledBack('Felder konnten nicht gespeichert werden.', $cause, postId: 5); self::assertTrue($failure->rolledBack); self::assertSame($cause, $failure->getPrevious()); @@ -23,16 +23,17 @@ final class WriteFailedTest extends TestCase public function testRolledBackHintReplacesTheDefaultHintOfWriteFailedOnly(): void { - $clean = new WriteFailed('x', true, rolledBackHint: 'Erst neu lesen.'); - $partial = new WriteFailed('x', false, rolledBackHint: 'Erst neu lesen.'); + $clean = WriteFailed::rolledBack('x', hint: 'Erst neu lesen.'); + $partial = WriteFailed::partial('x'); self::assertSame('Erst neu lesen.', $clean->toError()->get_error_data()['hint']); self::assertNotSame('Erst neu lesen.', $partial->toError()->get_error_data()['hint']); + self::assertStringStartsWith('Den Aufruf nicht einfach wiederholen.', $partial->toError()->get_error_data()['hint']); } public function testIncompleteRollbackMapsToPartialWriteWithTheIds(): void { - $error = (new WriteFailed('Felder konnten nicht gespeichert werden.', false, 5, 811))->toError(); + $error = WriteFailed::partial('Felder konnten nicht gespeichert werden.', postId: 5, revisionId: 811)->toError(); self::assertSame('lit_partial_write', $error->get_error_code()); self::assertSame(5, $error->get_error_data()['post_id']); @@ -41,13 +42,12 @@ final class WriteFailedTest extends TestCase public function testIncompleteRollbackCanDescribeItsOwnStateHintAndDetails(): void { - $error = (new WriteFailed( + $error = WriteFailed::partial( 'Menü nicht gespeichert.', - false, state: 'Das Menü ist teilweise geändert.', hint: 'Mit lit/menu-get prüfen.', details: ['menu_id' => 3] - ))->toError(); + )->toError(); self::assertSame('lit_partial_write', $error->get_error_code()); self::assertStringContainsString('Das Menü ist teilweise geändert.', $error->get_error_message()); @@ -59,8 +59,8 @@ final class WriteFailedTest extends TestCase public function testRolledBackStateReplacesNothingWasSavedOfWriteFailedOnly(): void { $state = 'Die Änderung wurde nicht gespeichert (Sicherung 88 bleibt erhalten).'; - $clean = (new WriteFailed('x', true, rolledBackState: $state))->toError(); - $partial = (new WriteFailed('x', false, rolledBackState: $state))->toError(); + $clean = WriteFailed::rolledBack('x', state: $state)->toError(); + $partial = WriteFailed::partial('x')->toError(); self::assertStringContainsString($state, $clean->get_error_message()); self::assertStringNotContainsString('Nichts wurde gespeichert', $clean->get_error_message()); @@ -69,10 +69,31 @@ final class WriteFailedTest extends TestCase public function testCleanRollbackKeepsItsDetails(): void { - $error = (new WriteFailed('Nicht übernommen.', true, details: ['backup_id' => 88]))->toError(); + $error = WriteFailed::rolledBack('Nicht übernommen.', details: ['backup_id' => 88])->toError(); self::assertSame('lit_write_failed', $error->get_error_code()); self::assertSame(88, $error->get_error_data()['backup_id']); self::assertStringContainsString('"backup_id":88', $error->get_error_message()); } + + public function testTheLogMessageIsTheDetailOrTheCauseOrTheReasonWithTheRollbackOutcome(): void + { + $cause = new RuntimeException('meta kaputt'); + + self::assertSame('Detail [rolled back]', WriteFailed::rolledBack('x', $cause, 'Detail')->getMessage()); + self::assertSame('meta kaputt [rollback incomplete]', WriteFailed::partial('x', $cause)->getMessage()); + self::assertSame('Grund [rolled back]', WriteFailed::rolledBack('Grund')->getMessage()); + } + + public function testTheNamedConstructorsKeepTheIdsForCallersThatLogThem(): void + { + $clean = WriteFailed::rolledBack('x', postId: 7, revisionId: 811); + $partial = WriteFailed::partial('x', postId: 7, revisionId: 811); + + self::assertTrue($clean->rolledBack); + self::assertSame([7, 811], [$clean->postId, $clean->revisionId]); + self::assertFalse($partial->rolledBack); + self::assertSame([7, 811], [$partial->postId, $partial->revisionId]); + self::assertArrayNotHasKey('post_id', $clean->toError()->get_error_data()); + } } diff --git a/tests/Unit/Integrations/Divi/Pages/DiviReplaceTextAbilityTest.php b/tests/Unit/Integrations/Divi/Pages/DiviReplaceTextAbilityTest.php index 5d7247f..fd960cd 100644 --- a/tests/Unit/Integrations/Divi/Pages/DiviReplaceTextAbilityTest.php +++ b/tests/Unit/Integrations/Divi/Pages/DiviReplaceTextAbilityTest.php @@ -561,7 +561,7 @@ final class DiviReplaceTextAbilityTest extends TestCase public function testWriteFailedKeepsItsOwnMessage(): void { $ability = $this->ability(); - $this->failures = [new WriteFailed('Die Datenbank hat das Speichern abgelehnt.', true, 7)]; + $this->failures = [WriteFailed::rolledBack('Die Datenbank hat das Speichern abgelehnt.', postId: 7)]; $log = (string) tempnam(sys_get_temp_dir(), 'lit-log'); $previous = ini_set('error_log', $log); try { diff --git a/tests/Unit/Integrations/Divi/Pages/DiviWriterTest.php b/tests/Unit/Integrations/Divi/Pages/DiviWriterTest.php index 4bcc6a9..2eec8b9 100644 --- a/tests/Unit/Integrations/Divi/Pages/DiviWriterTest.php +++ b/tests/Unit/Integrations/Divi/Pages/DiviWriterTest.php @@ -171,7 +171,7 @@ final class DiviWriterTest extends TestCase $content = DiviPageLoaderTest::fixture('synthetic-basic.html'); DiviPageLoaderTest::givenPost($content); $saver = Mockery::mock(DiviSaver::class); - $saver->allows('save')->andThrow(new WriteFailed(DiviSaver::REASON, false, 7, 811)); + $saver->allows('save')->andThrow(WriteFailed::partial(DiviSaver::REASON, postId: 7, revisionId: 811)); $log = Mockery::mock(ChangeLog::class); $log->expects('recordPartial')->with('lit/divi-delete', 'page', '7', 'Divi "Start": Änderung nur teilweise zurückgenommen', 811); $writer = $this->writer($saver, $log); @@ -187,7 +187,7 @@ final class DiviWriterTest extends TestCase $content = DiviPageLoaderTest::fixture('synthetic-basic.html'); DiviPageLoaderTest::givenPost($content); $saver = Mockery::mock(DiviSaver::class); - $saver->allows('save')->andThrow(new WriteFailed(DiviSaver::REASON, true, 7, 811)); + $saver->allows('save')->andThrow(WriteFailed::rolledBack(DiviSaver::REASON, postId: 7, revisionId: 811)); $log = Mockery::mock(ChangeLog::class); $log->expects('recordPartial')->never(); $log->expects('record')->never(); diff --git a/tests/Unit/Integrations/Media/MediaUploadAbilitiesTest.php b/tests/Unit/Integrations/Media/MediaUploadAbilitiesTest.php index c40508f..427b899 100644 --- a/tests/Unit/Integrations/Media/MediaUploadAbilitiesTest.php +++ b/tests/Unit/Integrations/Media/MediaUploadAbilitiesTest.php @@ -108,7 +108,7 @@ final class MediaUploadAbilitiesTest extends TestCase public function testAPartialUploadIsLoggedAsPartialAndRethrown(): void { - $failure = new WriteFailed('Alt-Text fehlt.', false, 55); + $failure = WriteFailed::partial('Alt-Text fehlt.', postId: 55); $importer = Mockery::mock(MediaImporter::class); $importer->allows('fromUrl')->andThrow($failure); $log = Mockery::mock(ChangeLog::class); diff --git a/tests/Unit/Integrations/WordPress/Menus/MenuSetItemsAbilityTest.php b/tests/Unit/Integrations/WordPress/Menus/MenuSetItemsAbilityTest.php index ea29135..db9d870 100644 --- a/tests/Unit/Integrations/WordPress/Menus/MenuSetItemsAbilityTest.php +++ b/tests/Unit/Integrations/WordPress/Menus/MenuSetItemsAbilityTest.php @@ -242,7 +242,7 @@ final class MenuSetItemsAbilityTest extends TestCase { $writer = Mockery::mock(MenuWriter::class); $writer->allows('plan')->andReturn(['errors' => [], 'removals' => [], 'warnings' => []]); - $writer->expects('apply')->andThrow(new WriteFailed('Menü nicht gespeichert.', true)); + $writer->expects('apply')->andThrow(WriteFailed::rolledBack('Menü nicht gespeichert.')); $log = Mockery::mock(ChangeLog::class); $log->shouldNotReceive('recordPartial'); $ability = new MenuSetItemsAbility($writer, new MenuOperations(), $log); @@ -255,7 +255,7 @@ final class MenuSetItemsAbilityTest extends TestCase { $writer = Mockery::mock(MenuWriter::class); $writer->allows('plan')->andReturn(['errors' => [], 'removals' => [], 'warnings' => []]); - $writer->expects('apply')->andThrow(new WriteFailed('Menü nicht gespeichert.', false)); + $writer->expects('apply')->andThrow(WriteFailed::partial('Menü nicht gespeichert.')); $log = Mockery::mock(ChangeLog::class); $log->expects('recordPartial')->with('lit/menu-set-items', 'nav_menu', '3', Mockery::type('string')); $ability = new MenuSetItemsAbility($writer, new MenuOperations(), $log); diff --git a/tests/Unit/Integrations/WordPress/Status/PostReplaceWithCopyAbilityTest.php b/tests/Unit/Integrations/WordPress/Status/PostReplaceWithCopyAbilityTest.php index e330e34..e7d9310 100644 --- a/tests/Unit/Integrations/WordPress/Status/PostReplaceWithCopyAbilityTest.php +++ b/tests/Unit/Integrations/WordPress/Status/PostReplaceWithCopyAbilityTest.php @@ -282,7 +282,7 @@ final class PostReplaceWithCopyAbilityTest extends TestCase { $cloner = Mockery::mock(PostCloner::class); $cloner->allows('snapshot')->andReturn(self::SNAPSHOT); - $cloner->expects('backup')->andThrow(new WriteFailed('Die Sicherung konnte nicht angelegt werden.', true)); + $cloner->expects('backup')->andThrow(WriteFailed::rolledBack('Die Sicherung konnte nicht angelegt werden.')); $cloner->expects('replace')->never(); $cloner->expects('restore')->never(); Functions\expect('wp_save_post_revision')->never();