From a9b2f2db718b7505c6cc0782672a063c9e8399c6 Mon Sep 17 00:00:00 2001 From: Jan-Niclas Loosen Date: Fri, 25 Sep 2026 23:11:18 +0200 Subject: [PATCH] fix: typed write failures, template rollback, parent and term cleanup WriteFailed maps to lit_write_failed / lit_partial_write (partial writes are logged with result partial); old template read before the update; row marker set before wp_update_post; created terms deleted on rollback; parent must be a readable non-descendant of a hierarchical type; featured image must be readable; snapshot keeps empty values; meta comparison after sanitize_meta; follow-up failures (listener, change log) only add warnings. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01At3V6SjwbMe6eMzVGW3Hs7 --- src/Abilities/AbilityRegistrar.php | 4 + src/Abilities/Errors.php | 38 +++++ src/Abilities/FollowUp.php | 30 ++++ src/Abilities/WriteFailed.php | 41 +++++ .../Content/ContentCreateAbility.php | 108 ++++++++++--- .../Content/ContentUpdateAbility.php | 150 +++++++++++++----- .../WordPress/Content/PostWriter.php | 54 ++++++- .../WordPress/Schema/MetaFieldSource.php | 11 +- .../WordPress/Schema/TermAssigner.php | 66 ++++++-- tests/Unit/Abilities/AbilityRegistrarTest.php | 22 +++ tests/Unit/Abilities/ErrorsTest.php | 34 ++++ tests/Unit/Abilities/FollowUpTest.php | 46 ++++++ tests/Unit/Abilities/WriteFailedTest.php | 32 ++++ .../Content/ContentCreateAbilityTest.php | 142 +++++++++++++++-- .../Content/ContentUpdateAbilityTest.php | 136 +++++++++++++--- .../WordPress/Content/PostWriterTest.php | 109 ++++++++++++- .../Schema/RegisteredMetaSourceTest.php | 13 ++ .../WordPress/Schema/TermAssignerTest.php | 28 +++- 18 files changed, 934 insertions(+), 130 deletions(-) create mode 100644 src/Abilities/FollowUp.php create mode 100644 src/Abilities/WriteFailed.php create mode 100644 tests/Unit/Abilities/FollowUpTest.php create mode 100644 tests/Unit/Abilities/WriteFailedTest.php diff --git a/src/Abilities/AbilityRegistrar.php b/src/Abilities/AbilityRegistrar.php index 015b2b0..01c6f78 100644 --- a/src/Abilities/AbilityRegistrar.php +++ b/src/Abilities/AbilityRegistrar.php @@ -71,6 +71,10 @@ final class AbilityRegistrar 'execute_callback' => static function (mixed $input = null) use ($ability): mixed { try { return $ability->execute(is_array($input) ? $input : []); + } catch (WriteFailed $e) { + $message = sprintf('[wp-lit-mcp] %s failed: %s', $ability->name(), $e); + error_log($message); // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log + return $e->toError(); } catch (Throwable $e) { $message = sprintf('[wp-lit-mcp] %s failed: %s', $ability->name(), $e); error_log($message); // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log diff --git a/src/Abilities/Errors.php b/src/Abilities/Errors.php index cb722cf..f201cd3 100644 --- a/src/Abilities/Errors.php +++ b/src/Abilities/Errors.php @@ -98,6 +98,44 @@ final class Errors return self::make('unsupported', $message, $hint, 400); } + /** A write failed and was rolled back completely. */ + public static function writeFailed(string $reason): WP_Error + { + return self::make( + 'write_failed', + trim('Speichern fehlgeschlagen: ' . $reason) . ' Nichts wurde gespeichert.', + 'Eingaben prüfen, ggf. korrigieren und erneut versuchen; tritt der Fehler wieder auf, den Administrator ' + . 'informieren (Details im PHP-Error-Log der Website).', + 500 + ); + } + + /** + * A write failed and could not be rolled back completely – something may have been saved. + */ + public static function partialWrite(string $reason, ?int $postId = null, ?int $revisionId = null): WP_Error + { + $state = $postId === null + ? 'Möglicherweise wurde trotzdem ein Entwurf angelegt.' + : sprintf('Eintrag %d ist möglicherweise teilweise geändert bzw. angelegt worden', $postId) + . ($revisionId === null ? '.' : sprintf(' (Stand vorher: Revision %d).', $revisionId)); + $details = array_filter( + ['post_id' => $postId, 'revision_id' => $revisionId], + static fn (?int $id): bool => $id !== null + ); + + return self::make( + 'partial_write', + trim('Speichern fehlgeschlagen: ' . $reason) . ' Die Änderung konnte nicht vollständig zurückgenommen ' + . 'werden. ' . $state, + 'Den Aufruf nicht einfach wiederholen. Zuerst den aktuellen Stand prüfen (lit-content-get, bei einem ' + . 'neuen Entwurf lit-content-query) und ggf. über lit-revisions-list den vorherigen Stand ' + . 'wiederherstellen; dem Nutzer Bescheid geben.', + 500, + $details + ); + } + public static function internal(): WP_Error { return self::make( diff --git a/src/Abilities/FollowUp.php b/src/Abilities/FollowUp.php new file mode 100644 index 0000000..4c641bb --- /dev/null +++ b/src/Abilities/FollowUp.php @@ -0,0 +1,30 @@ + $warnings Receives $warning when the step fails. + */ + public static function run(callable $step, string $context, string $warning, array &$warnings): mixed + { + try { + return $step(); + } catch (Throwable $e) { + $message = sprintf('[wp-lit-mcp] %s failed: %s', $context, $e); + error_log($message); // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log + $warnings[] = $warning; + + return null; + } + } +} diff --git a/src/Abilities/WriteFailed.php b/src/Abilities/WriteFailed.php new file mode 100644 index 0000000..eb6b4eb --- /dev/null +++ b/src/Abilities/WriteFailed.php @@ -0,0 +1,41 @@ +getMessage() : $reason); + parent::__construct( + sprintf('%s [%s]', $internal, $rolledBack ? 'rolled back' : 'rollback incomplete'), + 0, + $previous + ); + } + + public function toError(): WP_Error + { + return $this->rolledBack + ? Errors::writeFailed($this->reason) + : Errors::partialWrite($this->reason, $this->postId, $this->revisionId); + } +} diff --git a/src/Integrations/WordPress/Content/ContentCreateAbility.php b/src/Integrations/WordPress/Content/ContentCreateAbility.php index 360b1b3..caf158f 100644 --- a/src/Integrations/WordPress/Content/ContentCreateAbility.php +++ b/src/Integrations/WordPress/Content/ContentCreateAbility.php @@ -6,12 +6,13 @@ namespace LIT\MCP\Integrations\WordPress\Content; use LIT\MCP\Abilities\Ability; use LIT\MCP\Abilities\Errors; +use LIT\MCP\Abilities\FollowUp; use LIT\MCP\Abilities\Rev; use LIT\MCP\Abilities\ToolResult; +use LIT\MCP\Abilities\WriteFailed; use LIT\MCP\ChangeLog\ChangeLog; use LIT\MCP\Integrations\WordPress\PostGuard; use LIT\MCP\Integrations\WordPress\Schema\ContentSchema; -use RuntimeException; use Throwable; use WP_Error; use WP_Post; @@ -19,11 +20,15 @@ use WP_Post; /** * lit/content-create: new entry, always a draft. All or nothing (spec §8.2): input is * validated before the insert; if fields, terms or the featured image cannot be written - * afterwards, the brand-new draft is deleted again and the error is rethrown (the - * AbilityRegistrar logs it and reports lit_internal_error). + * afterwards, created terms and the brand-new draft are deleted again and a WriteFailed + * is thrown (AbilityRegistrar → lit_write_failed, or lit_partial_write if that cleanup + * failed or the insert itself threw; partial writes are logged with result "partial"). */ final class ContentCreateAbility implements Ability { + private const REASON_INSERT = 'WordPress hat beim Anlegen des Eintrags einen Fehler gemeldet.'; + private const REASON_EXTRAS = 'Felder, Begriffe oder Beitragsbild konnten nicht gespeichert werden.'; + public function __construct( private ContentSchema $schema, private PostWriter $writer, @@ -101,7 +106,14 @@ final class ContentCreateAbility implements Ability 'post_status' => 'draft', 'post_author' => get_current_user_id(), ]); - $postId = wp_insert_post(wp_slash($data), true); + try { + $postId = wp_insert_post(wp_slash($data), true); + } catch (Throwable $e) { + // 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); + } if ($postId instanceof WP_Error) { return Errors::invalidInput('WordPress hat den Eintrag abgelehnt: ' . $postId->get_error_message()); } @@ -110,42 +122,94 @@ final class ContentCreateAbility implements Ability try { $this->writer->writeExtras($postId, $postType, $prepared, $input); } catch (Throwable $e) { - $this->discard($postId, $e); + $this->discard($postId, $postType, $e); } - do_action('lit_mcp_after_save', $postId, $this->name(), $input); + + return $this->result($postId, $postType, $input, $prepared['warnings']); + } + + /** + * @param array $input + * @param list $warnings + * @return array + */ + private function result(int $postId, string $postType, array $input, array $warnings): array + { + // The draft is saved – failures from here on are logged and reported as warnings only. + FollowUp::run( + fn () => do_action('lit_mcp_after_save', $postId, $this->name(), $input), + $this->name() . ' lit_mcp_after_save', + 'Eine Erweiterung ist nach dem Speichern auf einen Fehler gestoßen; der Entwurf ist gespeichert.', + $warnings + ); $post = get_post($postId); $title = $post instanceof WP_Post ? (string) $post->post_title : (string) $input['title']; $summary = sprintf('Entwurf "%s" angelegt', $title); - $logId = $this->log->record($this->name(), $postType, (string) $postId, $summary); + $logId = FollowUp::run( + fn () => $this->log->record($this->name(), $postType, (string) $postId, $summary), + $this->name() . ' change log', + 'Änderungsprotokoll konnte nicht geschrieben werden; der Entwurf ist gespeichert.', + $warnings + ); return ToolResult::ok(sprintf('"%s" als Entwurf angelegt (ID %d).', $title, $postId)) ->changed((string) $postId, $postType, 'created', $title) ->rev($post instanceof WP_Post ? Rev::ofPost($post) : null) - ->logId($logId) + ->logId(is_int($logId) ? $logId : null) ->urls((string) get_preview_post_link($postId), (string) get_edit_post_link($postId, 'raw')) - ->warnings($prepared['warnings']) + ->warnings($warnings) ->toArray(); } /** - * Deletes the draft this call just created (it cannot have been used anywhere yet). + * Rolls the brand-new draft back: terms it created, then the draft itself (force – + * it cannot have been used anywhere yet). * - * @throws RuntimeException Always – the original failure, noting whether the draft is gone. + * @throws WriteFailed Always. */ - private function discard(int $postId, Throwable $cause): never + private function discard(int $postId, string $postType, Throwable $cause): never { - $deleted = wp_delete_post($postId, true); + $problems = []; + try { + $this->writer->deleteCreatedTerms(); + } catch (Throwable $e) { + $problems[] = $e->getMessage(); + } - throw new RuntimeException( - sprintf( - 'Extras of new post %d could not be written (%s); draft %s.', - $postId, - $cause->getMessage(), - $deleted ? 'deleted again' : 'could NOT be deleted' - ), - 0, - $cause + try { + if (! wp_delete_post($postId, true)) { + $problems[] = 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); + } + + $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) + ); + } + + private function logPartial(string $postType, string $postId, string $summary): void + { + $ignored = []; + FollowUp::run( + fn () => $this->log->record($this->name(), $postType, $postId, $summary, null, null, 'partial'), + $this->name() . ' change log', + '', + $ignored ); } } diff --git a/src/Integrations/WordPress/Content/ContentUpdateAbility.php b/src/Integrations/WordPress/Content/ContentUpdateAbility.php index d62349f..31a74cb 100644 --- a/src/Integrations/WordPress/Content/ContentUpdateAbility.php +++ b/src/Integrations/WordPress/Content/ContentUpdateAbility.php @@ -6,8 +6,10 @@ namespace LIT\MCP\Integrations\WordPress\Content; use LIT\MCP\Abilities\Ability; use LIT\MCP\Abilities\Errors; +use LIT\MCP\Abilities\FollowUp; use LIT\MCP\Abilities\Rev; use LIT\MCP\Abilities\ToolResult; +use LIT\MCP\Abilities\WriteFailed; use LIT\MCP\ChangeLog\ChangeLog; use LIT\MCP\Integrations\WordPress\PostGuard; use LIT\MCP\Integrations\WordPress\RevisionPoint; @@ -23,8 +25,9 @@ use WP_Post; * principle 7). All or nothing (spec §8.2): input is validated first; if fields, * terms or the featured image fail after the post row was updated, the row is * restored from the restore-point revision (plus old values for columns revisions - * do not cover), the extras get their old values back, and the error is rethrown - * (the AbilityRegistrar logs it and reports lit_internal_error). + * do not cover), the extras get their old values back, created terms are deleted, and + * a WriteFailed is thrown (AbilityRegistrar → lit_write_failed, or lit_partial_write + * when the rollback was incomplete; partial writes are logged with result "partial"). */ final class ContentUpdateAbility implements Ability { @@ -32,6 +35,8 @@ final class ContentUpdateAbility implements Ability 'title', 'content', 'excerpt', 'slug', 'parent', 'menu_order', 'template', 'terms', 'fields', 'featured_image', ]; + private const REASON = 'Felder, Begriffe, Beitragsbild oder Kerndaten konnten nicht gespeichert werden.'; + /** Post columns a revision stores (and wp_restore_post_revision() restores). */ private const REVISIONED = ['post_title', 'post_content', 'post_excerpt']; @@ -133,71 +138,137 @@ final class ContentUpdateAbility implements Ability $postId = (int) $post->ID; $snapshot = $this->writer->snapshot($postId, $postType, $prepared, $input); + $before = $this->columnsBefore($post, $data); $revisionId = RevisionPoint::capture($postId); - $rowUpdated = false; + $rowTouched = false; try { if ($data !== []) { + // Set before the call: a throwing save_post hook runs after the row was written. + $rowTouched = true; $result = wp_update_post(wp_slash(array_merge($data, ['ID' => $postId])), true); if ($result instanceof WP_Error) { return Errors::invalidInput( 'WordPress hat die Änderung abgelehnt: ' . $result->get_error_message() ); } - $rowUpdated = true; } $this->writer->writeExtras($postId, $postType, $prepared, $input); } catch (Throwable $e) { - $this->rollBack($post, $rowUpdated ? $data : [], $revisionId, $snapshot, $e); + $this->rollBack($post, $rowTouched ? $before : [], $revisionId, $snapshot, $e); } - do_action('lit_mcp_after_save', $postId, $this->name(), $input); + + return $this->result($post, $input, $prepared['warnings'], $revisionId); + } + + /** + * @param array $input + * @param list $warnings + * @return array + */ + private function result(WP_Post $post, array $input, array $warnings, ?int $revisionId): array + { + $postId = (int) $post->ID; + $postType = (string) $post->post_type; + + // The change is saved – failures from here on are logged and reported as warnings only. + FollowUp::run( + fn () => do_action('lit_mcp_after_save', $postId, $this->name(), $input), + $this->name() . ' lit_mcp_after_save', + 'Eine Erweiterung ist nach dem Speichern auf einen Fehler gestoßen; die Änderung ist gespeichert.', + $warnings + ); $updated = get_post($postId); $updated = $updated instanceof WP_Post ? $updated : $post; $changedKeys = array_keys(array_intersect_key($input, array_flip(self::CHANGE_KEYS))); $summary = sprintf('"%s" geändert: %s', $updated->post_title, implode(', ', $changedKeys)); - $logId = $this->log->record($this->name(), $postType, (string) $postId, $summary, $revisionId); + $logId = FollowUp::run( + fn () => $this->log->record($this->name(), $postType, (string) $postId, $summary, $revisionId), + $this->name() . ' change log', + 'Änderungsprotokoll konnte nicht geschrieben werden; die Änderung ist gespeichert.', + $warnings + ); return ToolResult::ok(sprintf('%s (Status unverändert: %s).', $summary, $updated->post_status)) ->changed((string) $postId, $postType, 'updated', (string) $updated->post_title) ->rev(Rev::ofPost($updated)) ->revisionId($revisionId) - ->logId($logId) + ->logId(is_int($logId) ? $logId : null) ->urls((string) get_preview_post_link($updated), (string) get_edit_post_link($postId, 'raw')) - ->warnings($prepared['warnings']) + ->warnings($warnings) ->toArray(); } /** - * @param array $data Columns that were written to the post row. - * @param array $snapshot Output of PostWriter::snapshot(). - * @throws RuntimeException Always – the original failure plus the rollback outcome. + * Values of the columns about to be written, read before the update (the template + * lives in post meta, so it must be read now – afterwards it is the new one). + * + * @param array $data + * @return array */ - private function rollBack(WP_Post $post, array $data, ?int $revisionId, array $snapshot, Throwable $cause): never + private function columnsBefore(WP_Post $post, array $data): array { + $before = []; + foreach (array_keys($data) as $column) { + $before[$column] = $column === 'page_template' + ? ((string) get_page_template_slug($post) ?: PostWriter::DEFAULT_TEMPLATE) + : $post->{$column}; + } + + return $before; + } + + /** + * @param array $before Pre-update values of the columns that were (possibly) written. + * @param array $snapshot Output of PostWriter::snapshot(). + * @throws WriteFailed Always – rolledBack tells whether everything was restored. + */ + private function rollBack(WP_Post $post, array $before, ?int $revisionId, array $snapshot, Throwable $cause): never + { + $postId = (int) $post->ID; + $steps = [ + fn () => $this->writer->deleteCreatedTerms(), + fn () => $this->writer->restore($postId, (string) $post->post_type, $snapshot), + fn () => $this->restoreRow($post, $before, $revisionId), + ]; $failures = []; - try { - $this->writer->restore((int) $post->ID, (string) $post->post_type, $snapshot); - } catch (Throwable $e) { - $failures[] = $e->getMessage(); + foreach ($steps as $step) { + try { + $step(); + } catch (Throwable $e) { + $failures[] = $e->getMessage(); + } } - try { - $this->restoreRow($post, $data, $revisionId); - } catch (Throwable $e) { - $failures[] = $e->getMessage(); + $detail = sprintf('Update of post %d failed: %s', $postId, $cause->getMessage()); + if ($failures === []) { + throw new WriteFailed(self::REASON, true, $postId, $revisionId, $cause, $detail); } - throw new RuntimeException( - sprintf( - 'Update of post %d failed (%s); %s.', - (int) $post->ID, - $cause->getMessage(), - $failures === [] ? 'changes rolled back' : 'rollback incomplete: ' . implode('; ', $failures) + $ignored = []; + FollowUp::run( + fn () => $this->log->record( + $this->name(), + (string) $post->post_type, + (string) $postId, + sprintf('"%s": Änderung nur teilweise zurückgenommen', $post->post_title), + $revisionId, + null, + 'partial' ), - 0, - $cause + $this->name() . ' change log', + '', + $ignored + ); + throw new WriteFailed( + self::REASON, + false, + $postId, + $revisionId, + $cause, + $detail . '; rollback incomplete: ' . implode('; ', $failures) ); } @@ -207,35 +278,28 @@ final class ContentUpdateAbility implements Ability * saved or confirmed it). Everything else – and everything when revisions are off, * where the newest revision may be stale – gets the value read before the update. * - * @param array $data + * @param array $before * @throws RuntimeException */ - private function restoreRow(WP_Post $post, array $data, ?int $revisionId): void + private function restoreRow(WP_Post $post, array $before, ?int $revisionId): void { - if ($data === []) { + if ($before === []) { return; } - $old = []; - foreach (array_keys($data) as $column) { - $old[$column] = $column === 'page_template' - ? ((string) get_page_template_slug($post) ?: PostWriter::DEFAULT_TEMPLATE) - : $post->{$column}; - } - - $fromRevision = array_values(array_intersect(array_keys($old), self::REVISIONED)); + $fromRevision = array_values(array_intersect(array_keys($before), self::REVISIONED)); if ($revisionId !== null && $fromRevision !== [] && wp_revisions_enabled($post)) { $restored = wp_restore_post_revision($revisionId, $fromRevision); if ($restored && ! is_wp_error($restored)) { - $old = array_diff_key($old, array_flip($fromRevision)); + $before = array_diff_key($before, array_flip($fromRevision)); } } - if ($old === []) { + if ($before === []) { return; } - $result = wp_update_post(wp_slash(array_merge($old, ['ID' => (int) $post->ID])), true); + $result = wp_update_post(wp_slash(array_merge($before, ['ID' => (int) $post->ID])), true); if ($result instanceof WP_Error || ! $result) { $message = $result instanceof WP_Error ? $result->get_error_message() : 'unknown error'; throw new RuntimeException('Post row could not be restored: ' . $message); diff --git a/src/Integrations/WordPress/Content/PostWriter.php b/src/Integrations/WordPress/Content/PostWriter.php index 529a304..0a401dd 100644 --- a/src/Integrations/WordPress/Content/PostWriter.php +++ b/src/Integrations/WordPress/Content/PostWriter.php @@ -7,6 +7,7 @@ namespace LIT\MCP\Integrations\WordPress\Content; use LIT\MCP\Abilities\Errors; use LIT\MCP\Integrations\WordPress\Schema\ContentSchema; use LIT\MCP\Integrations\WordPress\Schema\FieldValidator; +use LIT\MCP\Integrations\WordPress\Schema\MetaFieldSource; use LIT\MCP\Integrations\WordPress\Schema\TermAssigner; use RuntimeException; use Throwable; @@ -16,7 +17,8 @@ use WP_Post; /** * Shared validation and write logic of lit/content-create and lit/content-update. * prepare() validates everything before the first write (all or nothing, spec §8.2); - * snapshot()/restore() let lit/content-update undo the extras when a later write fails. + * snapshot()/restore() let lit/content-update undo the extras when a later write fails; + * deleteCreatedTerms() removes terms the last writeExtras() created by name. * Not final so the abilities can be unit-tested with a mock. */ class PostWriter @@ -34,6 +36,9 @@ class PostWriter /** wp_insert_post() ignores an empty page_template; this value selects the default template. */ public const DEFAULT_TEMPLATE = 'default'; + /** @var array Terms created by the last writeExtras(): term ID => taxonomy. */ + private array $createdTerms = []; + public function __construct( private ContentSchema $schema, private FieldValidator $validator, @@ -138,7 +143,8 @@ class PostWriter */ public function writeExtras(int $postId, string $postType, array $prepared, array $input): void { - $this->terms->assign($postId, $prepared['terms']); + $this->createdTerms = []; + $this->createdTerms = $this->terms->assign($postId, $prepared['terms']); $this->writeFields($postId, $postType, $prepared['fields']); if (array_key_exists('featured_image', $input)) { @@ -146,6 +152,18 @@ class PostWriter } } + /** + * Rollback helper: deletes the terms the last writeExtras() created by name. + * + * @throws RuntimeException + */ + public function deleteCreatedTerms(): void + { + $created = $this->createdTerms; + $this->createdTerms = []; + $this->terms->deleteTerms($created); + } + /** * Current values of everything writeExtras() would change – call before the first write. * @@ -159,8 +177,10 @@ class PostWriter $fields = []; foreach (array_keys($prepared['fields']) as $key) { $definition = $definitions[$key]; - $old = $this->schema->sourceOf($definition)->read($postId, $definition); - $fields[$key] = in_array($old, ['', [], null, false], true) ? null : $old; + $source = $this->schema->sourceOf($definition); + // null = "was not set" → restore() clears it; '', false, [] are real values and are rewritten. + $absent = $source instanceof MetaFieldSource && ! metadata_exists('post', $postId, (string) $key); + $fields[$key] = $absent ? null : $source->read($postId, $definition); } $terms = []; @@ -251,6 +271,10 @@ class PostWriter return 'Inhaltstyp unterstützt kein Beitragsbild.'; } + if (! current_user_can('read_post', $imageId)) { + return 'Keine Berechtigung für dieses Medium.'; + } + return wp_attachment_is_image($imageId) ? null : 'Kein Bild aus der Mediathek.'; } @@ -262,11 +286,25 @@ class PostWriter return null; } - $parent = get_post($parentId); + if (! is_post_type_hierarchical($postType)) { + return 'Inhaltstyp ist nicht hierarchisch; parent ist nicht möglich.'; + } - return $parent instanceof WP_Post && $parent->post_type === $postType - ? null - : 'Übergeordneter Eintrag muss existieren und denselben Inhaltstyp haben.'; + $postId = (int) ($input['post_id'] ?? 0); + if ($postId > 0 && $parentId === $postId) { + return 'Ein Eintrag kann nicht sich selbst übergeordnet sein.'; + } + + $parent = get_post($parentId); + if (! $parent instanceof WP_Post || $parent->post_type !== $postType) { + return 'Übergeordneter Eintrag muss existieren und denselben Inhaltstyp haben.'; + } + + if ($postId > 0 && in_array($postId, array_map('intval', get_post_ancestors($parent)), true)) { + return 'Übergeordneter Eintrag ist diesem Eintrag untergeordnet (Zirkelbezug).'; + } + + return current_user_can('read_post', $parentId) ? null : 'Keine Berechtigung für den übergeordneten Eintrag.'; } /** diff --git a/src/Integrations/WordPress/Schema/MetaFieldSource.php b/src/Integrations/WordPress/Schema/MetaFieldSource.php index 328c5eb..460f67d 100644 --- a/src/Integrations/WordPress/Schema/MetaFieldSource.php +++ b/src/Integrations/WordPress/Schema/MetaFieldSource.php @@ -39,7 +39,8 @@ abstract class MetaFieldSource implements FieldSource // update_post_meta() also returns false when the value is unchanged – only a // stored value that still differs is a failure. - if (update_post_meta($postId, $key, wp_slash($value)) === false && ! $this->holds($postId, $key, $value)) { + $updated = update_post_meta($postId, $key, wp_slash($value)); + if ($updated === false && ! $this->holds($postId, $postType, $key, $value)) { throw new RuntimeException(sprintf('Meta "%s" of post %d could not be saved.', $key, $postId)); } } @@ -53,8 +54,14 @@ abstract class MetaFieldSource implements FieldSource } } - private function holds(int $postId, string $key, mixed $value): bool + /** + * Whether the stored value already is what update_post_meta() would have stored – + * compared after sanitize_meta(), like WordPress does, so registered sanitize + * callbacks do not look like failures. + */ + private function holds(int $postId, string $postType, string $key, mixed $value): bool { + $value = sanitize_meta($key, $value, 'post', $postType); $stored = get_post_meta($postId, $key, false); if (! is_array($stored) || count($stored) !== 1) { return false; diff --git a/src/Integrations/WordPress/Schema/TermAssigner.php b/src/Integrations/WordPress/Schema/TermAssigner.php index 5e5bee4..b788c49 100644 --- a/src/Integrations/WordPress/Schema/TermAssigner.php +++ b/src/Integrations/WordPress/Schema/TermAssigner.php @@ -49,31 +49,69 @@ final class TermAssigner } /** + * Replaces the post's terms per taxonomy, creating terms given by name. If this + * fails, the terms it created in this call are deleted again before it throws. + * * @param array> $resolved Output of resolve(). + * @return array Created terms: term ID => taxonomy (for a later rollback). * @throws RuntimeException */ - public function assign(int $postId, array $resolved): void + public function assign(int $postId, array $resolved): array { - foreach ($resolved as $taxonomy => $items) { - $ids = []; - foreach ($items as $item) { - if (is_int($item)) { - $ids[] = $item; - continue; + $created = []; + try { + foreach ($resolved as $taxonomy => $items) { + $ids = []; + foreach ($items as $item) { + if (is_int($item)) { + $ids[] = $item; + continue; + } + + $term = wp_insert_term($item, $taxonomy); + if (is_wp_error($term)) { + throw new RuntimeException('Term could not be created: ' . $term->get_error_message()); + } + $ids[] = (int) $term['term_id']; + $created[(int) $term['term_id']] = $taxonomy; } - $created = wp_insert_term($item, $taxonomy); - if (is_wp_error($created)) { - throw new RuntimeException('Term could not be created: ' . $created->get_error_message()); + $result = wp_set_object_terms($postId, $ids, $taxonomy, false); + if (is_wp_error($result)) { + throw new RuntimeException('Terms could not be assigned: ' . $result->get_error_message()); } - $ids[] = (int) $created['term_id']; } + } catch (RuntimeException $e) { + try { + $this->deleteTerms($created); + } catch (RuntimeException $cleanup) { + throw new RuntimeException($e->getMessage() . '; ' . $cleanup->getMessage(), 0, $e); + } + throw $e; + } - $result = wp_set_object_terms($postId, $ids, $taxonomy, false); - if (is_wp_error($result)) { - throw new RuntimeException('Terms could not be assigned: ' . $result->get_error_message()); + return $created; + } + + /** + * Deletes terms (e.g. the ones assign() created). A term that no longer exists is fine. + * + * @param array $terms Term ID => taxonomy. + * @throws RuntimeException Naming every term that could not be deleted. + */ + public function deleteTerms(array $terms): void + { + $failed = []; + foreach ($terms as $termId => $taxonomy) { + $result = wp_delete_term((int) $termId, $taxonomy); + if ($result !== true && $result !== false) { + $failed[] = sprintf('%s %d', $taxonomy, $termId); } } + + if ($failed !== []) { + throw new RuntimeException('Created terms could not be deleted: ' . implode(', ', $failed)); + } } /** diff --git a/tests/Unit/Abilities/AbilityRegistrarTest.php b/tests/Unit/Abilities/AbilityRegistrarTest.php index 155db29..d58a19c 100644 --- a/tests/Unit/Abilities/AbilityRegistrarTest.php +++ b/tests/Unit/Abilities/AbilityRegistrarTest.php @@ -7,6 +7,7 @@ namespace LIT\MCP\Tests\Unit\Abilities; use Brain\Monkey\Functions; use LIT\MCP\Abilities\Ability; use LIT\MCP\Abilities\AbilityRegistrar; +use LIT\MCP\Abilities\WriteFailed; use LIT\MCP\Integrations\Integration; use LIT\MCP\Integrations\IntegrationRegistry; use LIT\MCP\Server\RequestContext; @@ -156,6 +157,27 @@ final class AbilityRegistrarTest extends TestCase unlink($log); } + public function testWriteFailedBecomesItsOwnErrorAndTheDetailIsLogged(): void + { + $log = (string) tempnam(sys_get_temp_dir(), 'lit-mcp-log'); + $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')) + ); + + try { + $result = ($this->registrarFor($ability)->argsFor($ability)['execute_callback'])([]); + } finally { + ini_set('error_log', (string) $previous); + } + + self::assertSame('lit_partial_write', $result->get_error_code()); + self::assertStringNotContainsString('/var/www', $result->get_error_message()); + self::assertStringContainsString('meta /var/www', (string) file_get_contents($log)); + unlink($log); + } + public function testRegisterAbilitiesCallsWordPressForEachAbility(): void { $ability = $this->ability(); diff --git a/tests/Unit/Abilities/ErrorsTest.php b/tests/Unit/Abilities/ErrorsTest.php index 0012c02..20fbd43 100644 --- a/tests/Unit/Abilities/ErrorsTest.php +++ b/tests/Unit/Abilities/ErrorsTest.php @@ -55,4 +55,38 @@ final class ErrorsTest extends TestCase self::assertNotSame('', json_decode($error->get_error_message(), true)['hint']); } } + + public function testWriteFailedSaysNothingWasSaved(): void + { + $error = Errors::writeFailed('Felder konnten nicht gespeichert werden.'); + + $payload = json_decode($error->get_error_message(), true); + self::assertSame('lit_write_failed', $error->get_error_code()); + self::assertStringContainsString('Nichts wurde gespeichert.', $payload['message']); + self::assertStringContainsString('Felder konnten nicht gespeichert werden.', $payload['message']); + self::assertStringContainsString('erneut', $payload['hint']); + } + + public function testPartialWriteNamesWhatMayHaveBeenSavedAndWarnsAgainstRetrying(): void + { + $error = Errors::partialWrite('Felder konnten nicht gespeichert werden.', 5, 811); + + $payload = json_decode($error->get_error_message(), true); + self::assertSame('lit_partial_write', $error->get_error_code()); + self::assertStringContainsString('Eintrag 5', $payload['message']); + self::assertStringContainsString('Revision 811', $payload['message']); + self::assertStringContainsString('lit-content-get', $payload['hint']); + self::assertStringContainsString('lit-revisions-list', $payload['hint']); + self::assertStringContainsString('nicht einfach wiederholen', $payload['hint']); + self::assertSame(5, $payload['post_id']); + self::assertSame(811, $payload['revision_id']); + } + + public function testPartialWriteWithoutIdSaysADraftMayExist(): void + { + $payload = json_decode(Errors::partialWrite('Anlegen fehlgeschlagen.')->get_error_message(), true); + + self::assertStringContainsString('Entwurf', $payload['message']); + self::assertArrayNotHasKey('post_id', $payload); + } } diff --git a/tests/Unit/Abilities/FollowUpTest.php b/tests/Unit/Abilities/FollowUpTest.php new file mode 100644 index 0000000..48e9817 --- /dev/null +++ b/tests/Unit/Abilities/FollowUpTest.php @@ -0,0 +1,46 @@ + 7, 'ctx', 'Warnung', $warnings)); + self::assertSame([], $warnings); + } + + public function testAFailingStepIsLoggedAndBecomesAWarning(): void + { + $log = (string) tempnam(sys_get_temp_dir(), 'lit-mcp-log'); + $previous = ini_set('error_log', $log); + $warnings = ['alt']; + + try { + $result = FollowUp::run( + static function (): void { + throw new RuntimeException('hook kaputt'); + }, + 'lit/content-create after_save', + 'Warnung', + $warnings + ); + } finally { + ini_set('error_log', (string) $previous); + } + + self::assertNull($result); + self::assertSame(['alt', 'Warnung'], $warnings); + self::assertStringContainsString('lit/content-create after_save', (string) file_get_contents($log)); + self::assertStringContainsString('hook kaputt', (string) file_get_contents($log)); + unlink($log); + } +} diff --git a/tests/Unit/Abilities/WriteFailedTest.php b/tests/Unit/Abilities/WriteFailedTest.php new file mode 100644 index 0000000..c0b2e4d --- /dev/null +++ b/tests/Unit/Abilities/WriteFailedTest.php @@ -0,0 +1,32 @@ +rolledBack); + self::assertSame($cause, $failure->getPrevious()); + self::assertStringContainsString('meta kaputt', $failure->getMessage()); + self::assertSame('lit_write_failed', $failure->toError()->get_error_code()); + } + + public function testIncompleteRollbackMapsToPartialWriteWithTheIds(): void + { + $error = (new WriteFailed('Felder konnten nicht gespeichert werden.', false, 5, 811))->toError(); + + self::assertSame('lit_partial_write', $error->get_error_code()); + self::assertSame(5, $error->get_error_data()['post_id']); + self::assertSame(811, $error->get_error_data()['revision_id']); + } +} diff --git a/tests/Unit/Integrations/WordPress/Content/ContentCreateAbilityTest.php b/tests/Unit/Integrations/WordPress/Content/ContentCreateAbilityTest.php index 42051c8..09ab3b3 100644 --- a/tests/Unit/Integrations/WordPress/Content/ContentCreateAbilityTest.php +++ b/tests/Unit/Integrations/WordPress/Content/ContentCreateAbilityTest.php @@ -8,17 +8,21 @@ use Brain\Monkey\Actions; use Brain\Monkey\Functions; use LIT\MCP\Abilities\Errors; use LIT\MCP\Abilities\Rev; +use LIT\MCP\Abilities\WriteFailed; use LIT\MCP\ChangeLog\ChangeLog; use LIT\MCP\Integrations\WordPress\Content\ContentCreateAbility; use LIT\MCP\Integrations\WordPress\Content\PostWriter; use LIT\MCP\Integrations\WordPress\Schema\ContentSchema; use LIT\MCP\Tests\Unit\TestCase; use Mockery; +use Mockery\MockInterface; use RuntimeException; use WP_Post; final class ContentCreateAbilityTest extends TestCase { + private const INPUT = ['post_type' => 'lit_person', 'title' => 'Anna', 'fields' => ['role' => 'Leitung']]; + private function schema(): ContentSchema { Functions\when('get_post_type_object')->justReturn((object) ['cap' => (object) ['create_posts' => 'edit_pages']]); @@ -29,6 +33,38 @@ final class ContentCreateAbilityTest extends TestCase return $schema; } + /** @return PostWriter&MockInterface */ + private function writer(array $warnings = []): PostWriter + { + $writer = Mockery::mock(PostWriter::class); + $writer->allows('prepare')->andReturn(['fields' => ['role' => 'Leitung'], 'terms' => [], 'warnings' => $warnings]); + $writer->allows('postData')->andReturn(['post_title' => 'Anna']); + Functions\when('get_current_user_id')->justReturn(3); + + return $writer; + } + + private function execute(PostWriter $writer, ChangeLog $log): WriteFailed + { + try { + (new ContentCreateAbility($this->schema(), $writer, $log))->execute(self::INPUT); + } catch (WriteFailed $e) { + return $e; + } + + self::fail('WriteFailed expected'); + } + + private function stubResultLinks(): WP_Post + { + $post = new WP_Post(['ID' => 42, 'post_title' => 'Anna', 'post_type' => 'lit_person']); + Functions\when('get_post')->justReturn($post); + Functions\when('get_preview_post_link')->justReturn('https://kunde.de/?p=42&preview=true'); + Functions\when('get_edit_post_link')->justReturn('https://kunde.de/wp-admin/post.php?post=42&action=edit'); + + return $post; + } + public function testInvalidInputWritesNothing(): void { $writer = Mockery::mock(PostWriter::class); @@ -45,7 +81,7 @@ final class ContentCreateAbilityTest extends TestCase { $schema = $this->schema(); $prepared = ['fields' => ['role' => 'Leitung'], 'terms' => [], 'warnings' => ['w']]; - $input = ['post_type' => 'lit_person', 'title' => 'Anna', 'fields' => ['role' => 'Leitung']]; + $input = self::INPUT; $writer = Mockery::mock(PostWriter::class); $writer->allows('prepare')->with('lit_person', $input, true)->andReturn($prepared); $writer->allows('postData')->andReturn(['post_title' => 'Anna']); @@ -58,10 +94,7 @@ final class ContentCreateAbilityTest extends TestCase 'post_author' => 3, ], true)->andReturn(42); Actions\expectDone('lit_mcp_after_save')->once()->with(42, 'lit/content-create', $input); - $post = new WP_Post(['ID' => 42, 'post_title' => 'Anna', 'post_type' => 'lit_person']); - Functions\when('get_post')->justReturn($post); - Functions\when('get_preview_post_link')->justReturn('https://kunde.de/?p=42&preview=true'); - Functions\when('get_edit_post_link')->justReturn('https://kunde.de/wp-admin/post.php?post=42&action=edit'); + $post = $this->stubResultLinks(); $log = Mockery::mock(ChangeLog::class); $log->expects('record')->with('lit/content-create', 'lit_person', '42', 'Entwurf "Anna" angelegt')->andReturn(7); @@ -75,28 +108,103 @@ final class ContentCreateAbilityTest extends TestCase self::assertSame(Rev::ofPost($post), $result['rev']); } - public function testFailingExtrasDeleteTheNewDraftAndNothingIsLogged(): void + public function testFailingExtrasDeleteCreatedTermsAndTheNewDraft(): void { - $schema = $this->schema(); - $input = ['post_type' => 'lit_person', 'title' => 'Anna', 'fields' => ['role' => 'Leitung']]; - $writer = Mockery::mock(PostWriter::class); - $writer->allows('prepare')->andReturn(['fields' => ['role' => 'Leitung'], 'terms' => [], 'warnings' => []]); - $writer->allows('postData')->andReturn(['post_title' => 'Anna']); + $writer = $this->writer(); $writer->expects('writeExtras')->andThrow(new RuntimeException('Pods could not save the fields.')); - Functions\when('get_current_user_id')->justReturn(3); + $writer->expects('deleteCreatedTerms'); Functions\when('wp_insert_post')->justReturn(42); Functions\expect('wp_delete_post')->once()->with(42, true)->andReturn(new WP_Post(['ID' => 42])); Actions\expectDone('lit_mcp_after_save')->never(); $log = Mockery::mock(ChangeLog::class); $log->expects('record')->never(); + $failure = $this->execute($writer, $log); + + self::assertTrue($failure->rolledBack); + self::assertStringContainsString('Pods could not save the fields.', $failure->getMessage()); + self::assertSame('lit_write_failed', $failure->toError()->get_error_code()); + } + + public function testADraftThatCannotBeDeletedIsAPartialWriteAndLogged(): void + { + $writer = $this->writer(); + $writer->expects('writeExtras')->andThrow(new RuntimeException('meta kaputt')); + $writer->expects('deleteCreatedTerms'); + Functions\when('wp_insert_post')->justReturn(42); + Functions\when('wp_delete_post')->justReturn(false); + $log = Mockery::mock(ChangeLog::class); + $log->expects('record') + ->with('lit/content-create', 'lit_person', '42', Mockery::type('string'), null, null, 'partial') + ->andReturn(9); + + $failure = $this->execute($writer, $log); + + self::assertFalse($failure->rolledBack); + self::assertSame(42, $failure->postId); + self::assertStringContainsString('Eintrag 42', $failure->toError()->get_error_message()); + } + + public function testLeftoverCreatedTermsMakeItAPartialWrite(): void + { + $writer = $this->writer(); + $writer->expects('writeExtras')->andThrow(new RuntimeException('meta kaputt')); + $writer->expects('deleteCreatedTerms')->andThrow(new RuntimeException('term 15 bleibt')); + Functions\when('wp_insert_post')->justReturn(42); + Functions\when('wp_delete_post')->justReturn(new WP_Post(['ID' => 42])); + $log = Mockery::mock(ChangeLog::class); + $log->expects('record')->andReturn(9); + + $failure = $this->execute($writer, $log); + + self::assertFalse($failure->rolledBack); + self::assertStringContainsString('term 15 bleibt', $failure->getMessage()); + } + + public function testAnInsertThatThrowsIsAPartialWriteWithoutId(): void + { + $writer = $this->writer(); + Functions\when('wp_insert_post')->alias(static function (): never { + throw new RuntimeException('save_post hook kaputt'); + }); + $log = Mockery::mock(ChangeLog::class); + $log->expects('record') + ->with('lit/content-create', 'lit_person', '', Mockery::type('string'), null, null, 'partial') + ->andReturn(9); + + $failure = $this->execute($writer, $log); + + self::assertFalse($failure->rolledBack); + self::assertNull($failure->postId); + self::assertSame('lit_partial_write', $failure->toError()->get_error_code()); + } + + public function testFailuresAfterTheWriteOnlyAddWarnings(): void + { + $writer = $this->writer(['w']); + $writer->allows('writeExtras'); + Functions\when('wp_insert_post')->justReturn(42); + $this->stubResultLinks(); + Actions\expectDone('lit_mcp_after_save')->once()->whenHappen(static function (): void { + throw new RuntimeException('listener kaputt'); + }); + $log = Mockery::mock(ChangeLog::class); + $log->expects('record')->andThrow(new RuntimeException('db weg')); + $file = (string) tempnam(sys_get_temp_dir(), 'lit-mcp-log'); + $previous = ini_set('error_log', $file); + try { - (new ContentCreateAbility($schema, $writer, $log))->execute($input); - self::fail('RuntimeException expected'); - } catch (RuntimeException $e) { - self::assertStringContainsString('Pods could not save the fields.', $e->getMessage()); - self::assertStringContainsString('deleted', $e->getMessage()); + $result = (new ContentCreateAbility($this->schema(), $writer, $log))->execute(self::INPUT); + } finally { + ini_set('error_log', (string) $previous); } + + self::assertTrue($result['ok']); + self::assertNull($result['log_id']); + self::assertCount(3, $result['warnings']); + self::assertStringContainsString('listener kaputt', (string) file_get_contents($file)); + self::assertStringContainsString('db weg', (string) file_get_contents($file)); + unlink($file); } public function testStatusIsNotAnInputAndPermissionNeedsCreateRights(): void diff --git a/tests/Unit/Integrations/WordPress/Content/ContentUpdateAbilityTest.php b/tests/Unit/Integrations/WordPress/Content/ContentUpdateAbilityTest.php index 0d716c7..12ef0f3 100644 --- a/tests/Unit/Integrations/WordPress/Content/ContentUpdateAbilityTest.php +++ b/tests/Unit/Integrations/WordPress/Content/ContentUpdateAbilityTest.php @@ -7,6 +7,7 @@ namespace LIT\MCP\Tests\Unit\Integrations\WordPress\Content; use Brain\Monkey\Actions; use Brain\Monkey\Functions; use LIT\MCP\Abilities\Rev; +use LIT\MCP\Abilities\WriteFailed; use LIT\MCP\ChangeLog\ChangeLog; use LIT\MCP\Integrations\WordPress\Content\ContentUpdateAbility; use LIT\MCP\Integrations\WordPress\Content\PostWriter; @@ -142,9 +143,9 @@ final class ContentUpdateAbilityTest extends TestCase /** * @param array $data - * @return array{PostWriter, array, array} + * @return array{PostWriter&\Mockery\MockInterface, array} */ - private function failingWriter(array $data): array + private function failingWriter(array $data, bool $restoreWorks = true): array { $input = ['post_id' => 5, 'rev' => Rev::ofPost($this->post), 'title' => 'Unser Team', 'fields' => ['a' => 1]]; $snapshot = ['fields' => ['a' => 0], 'terms' => []]; @@ -152,66 +153,159 @@ final class ContentUpdateAbilityTest extends TestCase $writer->allows('postData')->andReturn($data); $writer->allows('prepare')->andReturn(['fields' => ['a' => 1], 'terms' => [], 'warnings' => []]); $writer->expects('snapshot')->with(5, 'page', Mockery::type('array'), $input)->andReturn($snapshot); - $writer->expects('writeExtras')->andThrow(new RuntimeException('meta kaputt')); - $writer->expects('restore')->with(5, 'page', $snapshot); + $writer->allows('writeExtras')->andThrow(new RuntimeException('meta kaputt')); + $restore = $writer->expects('restore')->with(5, 'page', $snapshot); + if (! $restoreWorks) { + $restore->andThrow(new RuntimeException('restore kaputt')); + } + $writer->expects('deleteCreatedTerms'); Functions\when('wp_save_post_revision')->justReturn(811); Functions\when('wp_get_post_revisions')->justReturn([811 => new WP_Post(['ID' => 811])]); - Functions\when('get_page_template_slug')->justReturn(''); Actions\expectDone('lit_mcp_after_save')->never(); - return [$writer, $input, $snapshot]; + return [$writer, $input]; } /** @param array $input */ - private function executeExpectingRollback(PostWriter $writer, array $input): void + private function executeExpectingFailure(PostWriter $writer, array $input, ?ChangeLog $log = null): WriteFailed { - $log = Mockery::mock(ChangeLog::class); - $log->expects('record')->never(); + if ($log === null) { + $log = Mockery::mock(ChangeLog::class); + $log->expects('record')->never(); + } try { $this->ability($writer, $log)->execute($input); - self::fail('RuntimeException expected'); - } catch (RuntimeException $e) { + } catch (WriteFailed $e) { self::assertStringContainsString('meta kaputt', $e->getMessage()); - self::assertStringContainsString('rolled back', $e->getMessage()); + return $e; } + + self::fail('WriteFailed expected'); + } + + /** @return list> Captured wp_update_post() calls. */ + private function captureUpdates(array &$updates): void + { + Functions\when('wp_update_post')->alias(static function (array $data) use (&$updates): int { + $updates[] = $data; + return 5; + }); } public function testFailingExtrasRestoreTheRevisionAndTheOldValues(): void { [$writer, $input] = $this->failingWriter(['post_title' => 'Unser Team', 'post_name' => 'unser-team']); $updates = []; - Functions\when('wp_update_post')->alias(static function (array $data) use (&$updates): int { - $updates[] = $data; - return 5; - }); + $this->captureUpdates($updates); Functions\when('wp_revisions_enabled')->justReturn(true); Functions\expect('wp_restore_post_revision')->once()->with(811, ['post_title'])->andReturn(5); - $this->executeExpectingRollback($writer, $input); + $failure = $this->executeExpectingFailure($writer, $input); + self::assertTrue($failure->rolledBack); + self::assertSame('lit_write_failed', $failure->toError()->get_error_code()); self::assertSame([ ['post_title' => 'Unser Team', 'post_name' => 'unser-team', 'ID' => 5], ['post_name' => 'team', 'ID' => 5], ], $updates); } - public function testWithoutRevisionsTheOldColumnValuesAreWrittenBack(): void + public function testWithoutRevisionsTheOldColumnValuesIncludingTheOldTemplateAreWrittenBack(): void { [$writer, $input] = $this->failingWriter(['post_title' => 'Unser Team', 'page_template' => 'tpl-wide.php']); + $template = 'tpl-alt.php'; $updates = []; - Functions\when('wp_update_post')->alias(static function (array $data) use (&$updates): int { + Functions\when('get_page_template_slug')->alias(static function () use (&$template): string { + return $template; + }); + Functions\when('wp_update_post')->alias(static function (array $data) use (&$updates, &$template): int { $updates[] = $data; + $template = (string) ($data['page_template'] ?? $template); return 5; }); Functions\when('wp_revisions_enabled')->justReturn(false); Functions\expect('wp_restore_post_revision')->never(); - $this->executeExpectingRollback($writer, $input); + $this->executeExpectingFailure($writer, $input); self::assertSame([ ['post_title' => 'Unser Team', 'page_template' => 'tpl-wide.php', 'ID' => 5], - ['post_title' => 'Team', 'page_template' => 'default', 'ID' => 5], + ['post_title' => 'Team', 'page_template' => 'tpl-alt.php', 'ID' => 5], ], $updates); } + + public function testAnUpdateThatThrowsStillRestoresTheRow(): void + { + [$writer, $input] = $this->failingWriter(['post_name' => 'unser-team']); + $writer->shouldNotReceive('writeExtras'); + $updates = []; + Functions\when('wp_update_post')->alias(static function (array $data) use (&$updates): int { + $updates[] = $data; + if (count($updates) === 1) { + throw new RuntimeException('meta kaputt im save_post-Hook'); + } + return 5; + }); + + $failure = $this->executeExpectingFailure($writer, $input); + + self::assertTrue($failure->rolledBack); + self::assertSame(['post_name' => 'team', 'ID' => 5], $updates[1]); + } + + public function testAnIncompleteRollbackIsAPartialWriteAndLogged(): void + { + [$writer, $input] = $this->failingWriter(['post_title' => 'Unser Team'], false); + $updates = []; + $this->captureUpdates($updates); + Functions\when('wp_revisions_enabled')->justReturn(false); + $log = Mockery::mock(ChangeLog::class); + $log->expects('record') + ->with('lit/content-update', 'page', '5', Mockery::type('string'), 811, null, 'partial') + ->andReturn(12); + + $failure = $this->executeExpectingFailure($writer, $input, $log); + + self::assertFalse($failure->rolledBack); + self::assertStringContainsString('restore kaputt', $failure->getMessage()); + $error = $failure->toError(); + self::assertSame('lit_partial_write', $error->get_error_code()); + self::assertSame(5, $error->get_error_data()['post_id']); + self::assertSame(811, $error->get_error_data()['revision_id']); + } + + public function testFailuresAfterTheWriteOnlyAddWarnings(): void + { + $input = ['post_id' => 5, 'rev' => Rev::ofPost($this->post), 'title' => 'Unser Team']; + $writer = Mockery::mock(PostWriter::class); + $writer->allows('postData')->andReturn(['post_title' => 'Unser Team']); + $writer->allows('prepare')->andReturn(['fields' => [], 'terms' => [], 'warnings' => []]); + $writer->allows('snapshot')->andReturn(['fields' => [], 'terms' => []]); + $writer->allows('writeExtras'); + Functions\when('wp_save_post_revision')->justReturn(811); + Functions\when('wp_get_post_revisions')->justReturn([811 => new WP_Post(['ID' => 811])]); + Functions\when('wp_update_post')->justReturn(5); + Functions\when('get_preview_post_link')->justReturn(''); + Functions\when('get_edit_post_link')->justReturn(''); + Actions\expectDone('lit_mcp_after_save')->once()->whenHappen(static function (): void { + throw new RuntimeException('listener kaputt'); + }); + $log = Mockery::mock(ChangeLog::class); + $log->expects('record')->andThrow(new RuntimeException('db weg')); + $file = (string) tempnam(sys_get_temp_dir(), 'lit-mcp-log'); + $previous = ini_set('error_log', $file); + + try { + $result = $this->ability($writer, $log)->execute($input); + } finally { + ini_set('error_log', (string) $previous); + } + + self::assertTrue($result['ok']); + self::assertNull($result['log_id']); + self::assertCount(2, $result['warnings']); + self::assertStringContainsString('listener kaputt', (string) file_get_contents($file)); + unlink($file); + } } diff --git a/tests/Unit/Integrations/WordPress/Content/PostWriterTest.php b/tests/Unit/Integrations/WordPress/Content/PostWriterTest.php index 2f36b00..2ef9e04 100644 --- a/tests/Unit/Integrations/WordPress/Content/PostWriterTest.php +++ b/tests/Unit/Integrations/WordPress/Content/PostWriterTest.php @@ -10,6 +10,7 @@ use LIT\MCP\Integrations\WordPress\Schema\ContentSchema; use LIT\MCP\Integrations\WordPress\Schema\FieldDefinition; use LIT\MCP\Integrations\WordPress\Schema\FieldSource; use LIT\MCP\Integrations\WordPress\Schema\FieldValidator; +use LIT\MCP\Integrations\WordPress\Schema\RegisteredMetaSource; use LIT\MCP\Integrations\WordPress\Schema\TermAssigner; use LIT\MCP\Tests\Unit\TestCase; use Mockery; @@ -31,6 +32,9 @@ final class PostWriterTest extends TestCase Functions\when('get_object_taxonomies')->justReturn([]); Functions\when('post_type_supports')->justReturn(false); Functions\when('get_post')->justReturn(new WP_Post(['ID' => 3, 'post_type' => 'post'])); + Functions\when('is_post_type_hierarchical')->justReturn(true); + Functions\when('get_post_ancestors')->justReturn([]); + Functions\when('current_user_can')->justReturn(true); $schema = Mockery::mock(ContentSchema::class); $schema->allows('fields')->andReturn([ 'role' => new FieldDefinition('role', 'Funktion', 'string', 'pods', 'writable', true), @@ -53,6 +57,53 @@ final class PostWriterTest extends TestCase self::assertStringContainsString('tpl-wide.php', $fields['template']); } + private function writerWithoutFields(): PostWriter + { + Functions\when('get_object_taxonomies')->justReturn([]); + $schema = Mockery::mock(ContentSchema::class); + $schema->allows('fields')->andReturn([]); + + return new PostWriter($schema, new FieldValidator(), new TermAssigner()); + } + + /** @param array $input */ + private function errorFor(PostWriter $writer, array $input, string $path): string + { + $result = $writer->prepare('page', $input, false); + self::assertFalse(is_array($result), $path . ' should be rejected'); + + return (string) $result->get_error_data()['fields'][$path]; + } + + public function testParentMustBeAReadableNonDescendantEntryOfAHierarchicalType(): void + { + $writer = $this->writerWithoutFields(); + Functions\when('get_post')->justReturn(new WP_Post(['ID' => 8, 'post_type' => 'page'])); + Functions\when('get_post_ancestors')->justReturn([5, 1]); + Functions\when('current_user_can')->justReturn(true); + + Functions\when('is_post_type_hierarchical')->justReturn(false); + self::assertStringContainsString('hierarchisch', $this->errorFor($writer, ['parent' => 8], 'parent')); + + Functions\when('is_post_type_hierarchical')->justReturn(true); + self::assertStringContainsString('selbst', $this->errorFor($writer, ['post_id' => 8, 'parent' => 8], 'parent')); + self::assertStringContainsString('untergeordnet', $this->errorFor($writer, ['post_id' => 5, 'parent' => 8], 'parent')); + self::assertIsArray($writer->prepare('page', ['post_id' => 9, 'parent' => 8], false)); + + Functions\when('current_user_can')->justReturn(false); + self::assertStringContainsString('Berechtigung', $this->errorFor($writer, ['parent' => 8], 'parent')); + } + + public function testFeaturedImageMustBeReadable(): void + { + $writer = $this->writerWithoutFields(); + Functions\when('post_type_supports')->justReturn(true); + Functions\when('wp_attachment_is_image')->justReturn(true); + Functions\expect('current_user_can')->once()->with('read_post', 9)->andReturn(false); + + self::assertStringContainsString('Berechtigung', $this->errorFor($writer, ['featured_image' => 9], 'featured_image')); + } + public function testKnownDefaultAndEmptyTemplatesAreValid(): void { $this->stubTheme(); @@ -141,7 +192,7 @@ final class PostWriterTest extends TestCase $pods->expects('write')->with(5, 'lit_person', ['role' => 'Alt'], $definitions); $sample = Mockery::mock(FieldSource::class); $sample->allows('read')->with(5, $city)->andReturn(''); - $sample->expects('write')->with(5, 'lit_person', ['city' => null], $definitions); + $sample->expects('write')->with(5, 'lit_person', ['city' => ''], $definitions); $schema = Mockery::mock(ContentSchema::class); $schema->allows('fields')->andReturn($definitions); $schema->allows('sourceOf')->andReturnUsing(static fn (FieldDefinition $f) => $f->source === 'pods' ? $pods : $sample); @@ -157,7 +208,7 @@ final class PostWriterTest extends TestCase ); self::assertSame( - ['fields' => ['role' => 'Alt', 'city' => null], 'terms' => ['category' => [2, 3]], 'featured_image' => 7], + ['fields' => ['role' => 'Alt', 'city' => ''], 'terms' => ['category' => [2, 3]], 'featured_image' => 7], $snapshot ); @@ -166,6 +217,60 @@ final class PostWriterTest extends TestCase $writer->restore(5, 'lit_person', $snapshot); } + public function testSnapshotTellsAbsentMetaFromStoredEmptyValues(): void + { + $kept = new FieldDefinition('kept', 'kept', 'string', 'meta', 'writable'); + $absent = new FieldDefinition('absent', 'absent', 'string', 'meta', 'writable'); + $flag = new FieldDefinition('flag', 'flag', 'boolean', 'pods', 'writable'); + $pods = Mockery::mock(FieldSource::class); + $pods->allows('read')->andReturn(false); + $schema = Mockery::mock(ContentSchema::class); + $schema->allows('fields')->andReturn(['absent' => $absent, 'flag' => $flag, 'kept' => $kept]); + $schema->allows('sourceOf')->andReturnUsing( + static fn (FieldDefinition $f) => $f->source === 'pods' ? $pods : new RegisteredMetaSource() + ); + Functions\when('get_post_meta')->justReturn(''); + Functions\when('metadata_exists')->alias(static fn (string $type, int $id, string $key): bool => $key === 'kept'); + + $snapshot = (new PostWriter($schema, new FieldValidator(), new TermAssigner()))->snapshot( + 5, + 'lit_person', + ['fields' => ['kept' => 'x', 'absent' => 'y', 'flag' => true], 'terms' => [], 'warnings' => []], + [] + ); + + self::assertSame(['kept' => '', 'absent' => null, 'flag' => false], $snapshot['fields']); + } + + public function testTermsCreatedByWriteExtrasCanBeDeletedAgain(): void + { + $role = new FieldDefinition('role', 'Funktion', 'string', 'pods', 'writable'); + $pods = Mockery::mock(FieldSource::class); + $pods->allows('write')->andThrow(new RuntimeException('pods kaputt')); + $schema = Mockery::mock(ContentSchema::class); + $schema->allows('fields')->andReturn(['role' => $role]); + $schema->allows('sourceOf')->andReturn($pods); + Functions\when('wp_insert_term')->justReturn(['term_id' => 15]); + Functions\when('wp_set_object_terms')->justReturn([15]); + $writer = new PostWriter($schema, new FieldValidator(), new TermAssigner()); + + try { + $writer->writeExtras( + 5, + 'lit_person', + ['fields' => ['role' => 'x'], 'terms' => ['category' => ['Neu']], 'warnings' => []], + [] + ); + self::fail('RuntimeException expected'); + } catch (RuntimeException $e) { + self::assertSame('pods kaputt', $e->getMessage()); + } + + Functions\expect('wp_delete_term')->once()->with(15, 'category')->andReturn(true); + $writer->deleteCreatedTerms(); + $writer->deleteCreatedTerms(); // nothing left – no second call + } + public function testRestoreTriesEveryStepAndReportsFailures(): void { $role = new FieldDefinition('role', 'Funktion', 'string', 'pods', 'writable'); diff --git a/tests/Unit/Integrations/WordPress/Schema/RegisteredMetaSourceTest.php b/tests/Unit/Integrations/WordPress/Schema/RegisteredMetaSourceTest.php index 306fe06..b00cf0a 100644 --- a/tests/Unit/Integrations/WordPress/Schema/RegisteredMetaSourceTest.php +++ b/tests/Unit/Integrations/WordPress/Schema/RegisteredMetaSourceTest.php @@ -94,6 +94,7 @@ final class RegisteredMetaSourceTest extends TestCase public function testUnchangedValueIsNotAFailure(): void { $field = new FieldDefinition('count', 'count', 'integer', 'meta', 'writable'); + Functions\when('sanitize_meta')->returnArg(2); Functions\expect('update_post_meta')->once()->with(5, 'count', 3)->andReturn(false); Functions\expect('get_post_meta')->once()->with(5, 'count', false)->andReturn(['3']); @@ -101,9 +102,21 @@ final class RegisteredMetaSourceTest extends TestCase self::addToAssertionCount(1); } + public function testTheComparisonUsesTheSanitizedValue(): void + { + $field = new FieldDefinition('slug_meta', 'slug_meta', 'string', 'meta', 'writable'); + Functions\expect('sanitize_meta')->once()->with('slug_meta', ' Trier ', 'post', 'event')->andReturn('trier'); + Functions\when('update_post_meta')->justReturn(false); + Functions\when('get_post_meta')->justReturn(['trier']); + + (new RegisteredMetaSource())->write(5, 'event', ['slug_meta' => ' Trier '], ['slug_meta' => $field]); + self::addToAssertionCount(1); + } + public function testFailedUpdateThrows(): void { $field = new FieldDefinition('count', 'count', 'integer', 'meta', 'writable'); + Functions\when('sanitize_meta')->returnArg(2); Functions\when('update_post_meta')->justReturn(false); Functions\when('get_post_meta')->justReturn(['2']); diff --git a/tests/Unit/Integrations/WordPress/Schema/TermAssignerTest.php b/tests/Unit/Integrations/WordPress/Schema/TermAssignerTest.php index cb4ceb6..d590a57 100644 --- a/tests/Unit/Integrations/WordPress/Schema/TermAssignerTest.php +++ b/tests/Unit/Integrations/WordPress/Schema/TermAssignerTest.php @@ -69,7 +69,33 @@ final class TermAssignerTest extends TestCase Functions\expect('wp_insert_term')->once()->with('Neu', 'category')->andReturn(['term_id' => 15]); Functions\expect('wp_set_object_terms')->once()->with(5, [4, 15], 'category', false)->andReturn([4, 15]); - (new TermAssigner())->assign(5, ['category' => [4, 'Neu']]); + self::assertSame([15 => 'category'], (new TermAssigner())->assign(5, ['category' => [4, 'Neu']])); + } + + public function testAFailedAssignmentDeletesTheTermsItCreated(): void + { + Functions\when('wp_insert_term')->justReturn(['term_id' => 15]); + Functions\when('wp_set_object_terms')->justReturn(new WP_Error('x', 'kaputt')); + Functions\expect('wp_delete_term')->once()->with(15, 'category')->andReturn(true); + + $this->expectException(RuntimeException::class); + + (new TermAssigner())->assign(5, ['category' => ['Neu']]); + } + + public function testDeleteTermsAcceptsAlreadyMissingTermsAndThrowsOnRealFailures(): void + { + Functions\when('wp_delete_term')->alias(static fn (int $id): mixed => match ($id) { + 15 => true, + 16 => false, + default => new WP_Error('x', 'kaputt'), + }); + $assigner = new TermAssigner(); + + $assigner->deleteTerms([15 => 'category', 16 => 'category']); + + $this->expectException(RuntimeException::class); + $assigner->deleteTerms([15 => 'category', 17 => 'post_tag']); } public function testAssignFailureThrows(): void