Serialized array values in post meta are broken when distributing to internal connections

[rmarscher]

The \Distributor\Utils\prepare_meta() function is used to put the post's meta into the $meta var. It uses the WP core function maybe_unserialize() to set the values to the unserialized version.

\Distributor\Utils\set_meta() is used to set the meta on the new post. However, when it encounters an array for the value, it loops through the array adding post meta with the same key for each value. It doesn't set the whole array to the meta value. The original value is then lost.

I understand that it is difficult to make distribution work for every scenario so custom hooks should be used to control the distribution process. In \Distributor\InternalConnections\NetworkSiteConnection, the push() method sets up the posts slightly different than the pull() method. With the pull method, the meta, taxonomies and media are prepared as properties of the $post object. In the push method, they are just storied in local variables:

distributor/includes/classes/InternalConnections/NetworkSiteConnection.php

Lines 68 to 70 in 6923bcd

	$media = \Distributor\Utils\prepare_media( $post_id );
	$terms = \Distributor\Utils\prepare_taxonomy_terms( $post_id );
	$meta = \Distributor\Utils\prepare_meta( $post_id );

I think the push method should be updated to store them in the same properties as the pull method uses. Like this:

		$post->media = \Distributor\Utils\prepare_media( $post_id );
		$post->terms = \Distributor\Utils\prepare_taxonomy_terms( $post_id );
		$post->meta  = \Distributor\Utils\prepare_meta( $post_id );

And then later, use those properties when setting the meta, taxonomy and media. That way, the dt_push_post_args filter can be used to manipulate the meta values.

It seems that the push and pull methods could potentially be refactored to use a shared method that distributes the post. I'll submit a pull request that fixes this.

Here are the filters I used to fix my use case:

// Fixes an issue that Distributor has with serialized arrays in post meta
function serialize_post_meta_arrays($post) {
	if (!$post || !$post->meta) {
		return $post;
	}
	$meta = $post->meta;
	$serialized_meta = [];
	foreach ($meta as $key => $value) {
		$serialized_meta[$key] = $value;
		if (is_array($value)) {
			$serialized_meta[$key] = serialize($value);
		}
	}
	$post->meta = $serialized_meta;
	return $post;
}
add_filter( 'dt_push_post_args', function($post_array, $post, $args, $connection) {
	serialize_post_meta_arrays($post);
	return $post_array;
}, 10, 4 );
add_filter( 'dt_pull_post_args', function($post_array, $remote_post_id, $post, $connection) {
	serialize_post_meta_arrays($post);
	return $post_array;
}, 10, 4 );

[tlovett1]

Interesting. I see your point.

I feel like the solution may be to never unserialize. We can't unserialize in prepare then serialize in set because in set we would never know if the original meta was stored serialized or as different values under the same key.

Thoughts?

[rmarscher]

@tlovett1 Sorry for the slow response. I think not unserializing would work. Maybe @adamsilverstein can comment if there was a problem that was fixed by adding the use of maybe_unserialize and if changing it back to not unserialize will cause other issues.
See bebfba0

[rmarscher]

Oops... I read the "blame" logs wrong there. Sorry about the mention @adamsilverstein The maybe_unserialize was in place earlier than that commit. Seems to me that the unserialize isn't necessary.

[helen]

I am still wrapping my head around all of the meta handling but want to crossref #200 / #225 and #258 / #259 here because this is all interrelated. I think we've solved this issue by adding a level of hierarchy to the post meta array but that's broken interop between 1.3.4 and older versions of Distributor (which we probably won't fix but it is important to note). I think the point about the difference in handling between push and pull might also still stand.