Decouple WP_Customize_Manager

[utkarshpatel]

When you create multiple instances of WP_Customize_Manager it adds/removes multiple hooks in its constructor which should be executed single time.

So let's say if I want to create two new changeset posts it I will do.

<?php
$data = array(
... // Some changeset data.
);
$manager1 = new WP_Customize_Manager();
$manager1->save_changeset_post( array( 'data' => $data ) );

$manager2 = new WP_Customize_Manager();
$manager2->save_changeset_post( array( 'data' => $data ) );

This will cause wp_ajax_customize_save to add twice with $manager1 and $manager2 it should add only once.

wp_ajax_customize_save is a just example.

See Constructor: ​https://github.com/WordPress/WordPress/blob/5f771393a318d333503d5e13525dfcd543819302/wp-includes/class-wp-customize-manager.php#L229-L369.

So we should decouple WP_Customize_Manager class and maybe extract changeset methods in separate class WP_Customize_Changeset.

[westonruter]

Yes. As ​mentioned previously, the WP_Customize_Manager class needs to be broken up into smaller classes.

[westonruter]

Part of the scope here is to invalidate any cached data when clean_post_cache is called. See ​https://github.com/WP-API/wp-api-customize-endpoints/pull/5#pullrequestreview-40207192

[westonruter]

This can also include factoring out the logic for trashing a changeset, as @dlh has prototyped in ​https://github.com/WP-API/wp-api-customize-endpoints/pull/5

[dlh]

40527.diff​ is a first attempt at a WP_Customize_Changeset class.

Some notes about the patch:

• The scope so far has been to abstract as much changeset code as possible that operates independently of a WP_Customize_Manager instance. I chose this scope in part based on the comment here: ​https://github.com/WP-API/wp-api-customize-endpoints/pull/5#discussion_r118804446.

• There is, of course, room to add more functionality from WP_Customize_Manager -- for instance, more of the logic from save_changeset_post(). But because doing so also might increase the coupling between the two classes, I thought it best to check in first.

• One exception to the above is the publish() method, which accepts a $wp_customize instance.

• There are two named constructors for instantiating an instance based on a UUID or a post ID. Core needs to be able to look up changeset data by both UUID and post ID, and in neither case is there guaranteed to be a corresponding post in the database. I found it easier to handle the logic for what the changeset instance should look like under these different circumstances in the separate constructors rather than a single __construct(), but I'm open to other ideas.

• The patch doesn't try to swap a WP_Customize_Changeset into every location in core where it could be used, so it might not handle all the requirements it would need to yet. Attempting to integrate the class into the in-progress REST API endpoints would also probably uncover additional needs or other functionality that would be helpful.

• comment:3 is addressed in the patch, but not comment:2 yet.

• No unit tests yet.

[dlh]

In 40527.2.diff​:

• Remove the cached copy of changeset data inside WP_Customize_Manager to address comment:3.

• Return null when instantiating a changeset for an invalid post.

• Remove a duplicate call to json_last_error(), and other minor fixes.

Although comment:3 mentions using the clean_cache_post hook to reset the WP_Customize_Manager::_changeset_data property, we also had discussed removing the cached value entirely in the REST API endpoints repo: ​https://github.com/WP-API/wp-api-customize-endpoints/pull/5#discussion_r118804446.

I went with the latter in this patch for its comparative simplicity and because core itself almost always calls WP_Customize_Manager::get_changeset_post_data(), and that already bypasses the property.

Regarding the change to WP_Customize_Changeset::from_post(): Returning null would ensure that the two named constructors will return only changeset instances that contain at least a UUID so that WP_Customize_Changeset::save() can be called reliably.

Returning null also matches the behavior of get_post(), and it removes the appearance of having retrieved a changeset by post ID when the lookup actually failed.

Still, one alternative to returning null would be to still return a WP_Customize_Changeset instance as happened in 40527.diff​, but then return a WP_Error early in WP_Customize_Changeset::save() if the instance doesn't have a post ID or UUID.

[westonruter]

Bumping priority to high for visibility and alignment with 4.9 goals, and given proximity to beta 1 deadline.

[westonruter]

We'll re-visit this, perhaps with Customizer v2 feature plugin during 5.0.