Skip to content

REST API: Correctly store multiple value meta on autosaves - #12788

Open
jigneshbhavani wants to merge 1 commit into
WordPress:trunkfrom
jigneshbhavani:fix/65754-autosave-multi-value-meta
Open

REST API: Correctly store multiple value meta on autosaves#12788
jigneshbhavani wants to merge 1 commit into
WordPress:trunkfrom
jigneshbhavani:fix/65754-autosave-multi-value-meta

Conversation

@jigneshbhavani

@jigneshbhavani jigneshbhavani commented Jul 31, 2026

Copy link
Copy Markdown

Problem

WP_REST_Autosaves_Controller::create_post_autosave() stores every revisioned meta key with a single call:

update_metadata( 'post', $revision_id, $meta_key, wp_slash( $meta[ $meta_key ] ) );

update_metadata() called without a $prev_value updates every row that shares the meta key. For a meta key registered with single => false the autosave revision already has one row per value, copied there by _wp_copy_post_meta(). Each of those rows is then overwritten with the complete array, so a key with two values ends up as:

meta_key | meta_value
foo      | a:2:{i:0;s:3:"bar";i:1;s:3:"qux";}
foo      | a:2:{i:0;s:3:"bar";i:1;s:3:"qux";}

instead of one row holding bar and another holding qux.

There is a second, smaller problem in the change detection a few lines above:

$old_meta = get_metadata_raw( 'post', $post_id, $meta_key, true );
$new_meta = $meta[ $meta_key ] ?? '';

$single is hardcoded to true, so for a multiple value key the stored data is read back as a single string and compared against an array. That comparison can never match, so the autosave is always considered different from the post even when nothing actually changed, and a revision is written on every request. This is not in the ticket report, but it has the same cause, so it is fixed here as well. Happy to split it out if that is preferred.

Fix

Whether a key is single is now taken from the single argument it was registered with, rather than from whether the submitted value happens to be an array. That distinction matters: a key registered with single => true can legitimately hold an array, and the existing test_update_item_with_json_meta covers exactly that case. Keys with no registration keep the previous single behaviour.

For multiple value keys the stored rows are replaced the same way _wp_copy_post_meta() writes them in the first place, with delete_metadata() followed by one add_metadata() per value. The change detection reads back with the matching $single argument so the comparison is like for like.

Single value meta is unaffected and still goes through update_metadata().

Testing instructions

Register a revisionable, multiple value post meta field:

add_action(
	'init',
	function () {
		register_post_meta(
			'post',
			'foo',
			array(
				'show_in_rest'      => true,
				'revisions_enabled' => true,
				'single'            => false,
				'type'              => 'string',
			)
		);
	}
);

Add two values to a post, then open it in the block editor, change the content and wait for an autosave:

wp post meta add 1 foo bar
wp post meta add 1 foo baz

Inspect the POST /wp-json/wp/v2/posts/1/autosaves response. Before this change the returned meta.foo does not reflect the submitted values and the revision rows each contain the whole serialized array. After it, each value is stored in its own row and comes back correctly.

Automated coverage is included:

phpunit --group restapi-autosave

test_update_item_with_multiple_value_meta fails on trunk with:

Failed asserting that two arrays are identical.
-    0 => 'bar'
-    1 => 'qux'
+    0 => Array ( 0 => 'bar', 1 => 'qux' )
+    1 => Array ( 0 => 'bar', 1 => 'qux' )

and passes with this change. The full restapi, revision and meta groups pass locally.

Trac ticket: https://core.trac.wordpress.org/ticket/65754

Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Locating the cause in create_post_autosave(), drafting the fix and the regression test, and drafting this description. I reviewed the change, confirmed the failure and the fix locally against the restapi, restapi-autosave, revision and meta groups, and I take responsibility for the code.


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

`WP_REST_Autosaves_Controller::create_post_autosave()` wrote every revisioned
meta key with `update_metadata()`. Without a `$prev_value` that updates every
row sharing the meta key, so a meta key registered with `single => false` ended
up with each of its rows holding the entire array of values instead of one value
each.

Values are now stored the way `_wp_copy_post_meta()` stores them, by replacing
the existing rows and adding one row per value. Whether a key is treated as
single is taken from its registration rather than from the type of the value,
since a key registered with `single => true` may legitimately hold an array.

The change detection above it hardcoded `$single` to `true` when reading the
stored value, so a multiple value key was always compared as a single string
against an array and never matched, marking every autosave as different.

Props jigneshbhavani, wongjn.
Fixes #65754.
@github-actions

Copy link
Copy Markdown

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @jigneshbhavani.

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@github-actions

Copy link
Copy Markdown

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

@irozum irozum left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This correctly fixes both problems described on the ticket: update_metadata() without a $prev_value was overwriting every row of a multi-value meta key with the whole submitted array, and the change-detection above it hardcoded single => true so a multi-value key could never compare equal and always forced a new autosave revision. Keying $is_single_meta off the meta key's actual registration rather than the shape of the submitted value is the right call — it's what correctly keeps test_update_item_with_json_meta (a single => true key holding an array) working unchanged.

Ran restapi-autosave (41 tests), meta (458 tests), and revision (45 tests) — all green — plus PHPCS lint:errors and PHPStan, both clean. I also ran the full restapi group; there's one unrelated failure (Test_oEmbed_Controller::test_proxy_with_classic_embed_provider), which I confirmed also fails on trunk without this PR checked out, so it's a pre-existing flake, not a regression here.

I traced the one edge case the fix has to get right: wp_post_revision_meta_keys() builds its list from the same $registered_meta array this PR queries, so every key it returns should already have a 'single' entry — except when a plugin adds an unregistered key via the wp_post_revision_meta_keys filter. The ! isset( $registered_meta[ $meta_key ]['single'] ) fallback to single-mode handles exactly that case, matching the old hardcoded-true behavior for anything not actually registered. Good attention to that detail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants