Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure post abstraction methods use correct site ID. #1010

Merged
merged 14 commits into from
Feb 13, 2023
Merged

Ensure post abstraction methods use correct site ID. #1010

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
b010154
Track site ID used to create object.
peterwilsoncc Jan 30, 2023
1de7cdf
Add __call magic method to ensure site is correct.
peterwilsoncc Jan 30, 2023
9ace708
Mock `get_current_blog_id()` for all tests.
peterwilsoncc Jan 30, 2023
8085f6f
Mock `get_current_blog_id()` for all tests.
peterwilsoncc Jan 30, 2023
8e6aa89
Group magic methods.
peterwilsoncc Jan 30, 2023
1b9bac3
Only throw notice if WP_DEBUG is true.
peterwilsoncc Jan 30, 2023
3aaf4f5
Methods must be inaccessible for magic method to fire.
peterwilsoncc Feb 1, 2023
9189602
Set the site ID even earlier.
peterwilsoncc Feb 1, 2023
4e0e09f
Note for developers.
peterwilsoncc Feb 1, 2023
b167b0d
Test only magic methods are public.
peterwilsoncc Feb 1, 2023
dc19355
Cache results to avoid switching sites too many times.
peterwilsoncc Feb 1, 2023
1c4c2bf
Define methods available via the magic method.
peterwilsoncc Feb 2, 2023
1948031
Note the need to add public methods to docblock.
peterwilsoncc Feb 2, 2023
3aa72a1
Improve error messsages: i18n and phrasing.
peterwilsoncc Feb 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
210 changes: 154 additions & 56 deletions includes/classes/DistributorPost.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,34 @@
* includes the phrase Distributor to make it clear to developers `use`ing
* the class that they are not using the `WP_Post` object.
*
* Developer note: This class uses the `__call()` magic method to ensure the
* post data is from the correct site on multisite installs. This is to avoid
* repeating code to determine if `switch_to_blog()` is required.
*
* When adding new methods to this class, please ensure that the method is
* protected to ensure the magic method is used. If the method is intended to
* be public, please add it to the `@method` docblock below to ensure it is
* shown in IDEs.
*
* @since x.x.x
*
* @method bool has_blocks()
* @method bool has_block( string $block_name )
* @method int get_the_ID()
* @method string get_permalink()
* @method string get_post_type()
* @method int|false get_post_thumbnail_id()
* @method string|false get_post_thumbnail_url( string $size = 'post-thumbnail' )
* @method string|false get_the_post_thumbnail( string $size = 'post-thumbnail', array $attr = '' )
* @method string get_canonical_url( string $canonical_url = '' )
* @method string get_author_name( string $author_name = '' )
* @method string get_author_link( string $author_link = '' )
* @method array get_meta()
* @method array get_terms()
* @method array get_media()
* @method array post_data()
* @method array to_insert()
* @method string to_json( int $options = 0, int $depth = 512 )
*/
class DistributorPost {
/**
Expand Down Expand Up @@ -94,6 +121,13 @@ class DistributorPost {
*/
public $connection_id = 0;

/**
* The site ID of this post.
*
* @var int
*/
public $site_id = 0;

/**
* The source site data for internal connections.
*
Expand All @@ -110,6 +144,16 @@ class DistributorPost {
*/
private $source_site = [];

/**
* The cache for accessing methods when the site has been switched.
*
* This prevents the need to switch sites multiple times when accessing
* the same method multiple times.
*
* @var array
*/
private $switched_site_cache = [];

/**
* Initialize the DistributorPost object.
*
Expand All @@ -122,7 +166,8 @@ public function __construct( $post ) {
return;
}

$this->post = $post;
$this->site_id = get_current_blog_id();
$this->post = $post;

/*
* The original post ID is listed as excluded post meta and therefore
Expand Down Expand Up @@ -178,6 +223,95 @@ public function __construct( $post ) {
}
}

/**
* Magic method for calling methods on the post object.
*
* This is used to ensure the post object is switched to the correct site before
* running any of the internal methods.
*
* @param string $name Method name.
* @param array $arguments Method arguments.
*/
public function __call( $name, $arguments ) {
$switched = false;
$cache_key = md5( "{$name}::" . wp_json_encode( $arguments ) );
if ( ! method_exists( $this, $name ) ) {
// Emulate default behavior of calling non existent method (a fatal error).
// phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error
trigger_error(
sprintf(
/* translators: %s: method name */
esc_html__( 'Call to undefined method %s', 'distributor' ),
esc_html( __CLASS__ . '::' . $name . '()' )
),
E_USER_ERROR
);
}

if ( get_current_blog_id() !== $this->site_id ) {
if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) {
// phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error -- throwing a warning is the correct behavior.
trigger_error( esc_html__( 'DistributorPost object was called from a switched site.', 'distributor' ), E_USER_WARNING );
}
// array_key_exists as opposed to isset to avoid false negatives.
if ( array_key_exists( $cache_key, $this->switched_site_cache ) ) {
/*
* Avoid switching sites if the result is already cached.
*
* Due to the use of filters within various functions called by
* the helper methods, this data may not be correct at runtime if
* hooks have been added or removed. However, caching is a performance
* compromise to avoid switching sites on every call.
*/
return $this->switched_site_cache[ $cache_key ];
}
switch_to_blog( $this->site_id );
$switched = true;
}
$result = call_user_func_array( array( $this, $name ), $arguments );
$this->switched_site_cache[ $cache_key ] = $result;
if ( $switched ) {
restore_current_blog();
}
return $result;
}

/**
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

::__get() and ::__isset() methods are unchanged but have been relocated to group all the magic methods together.

* Magic getter method.
*
* This method is used to get the value of the `source_site` property and
* populate it if needs be. For internal connections the post permalink is
* updated with live data.
*
* @param string $name Property name.
* @return mixed
*/
public function __get( $name ) {
if ( in_array( $name, array( 'source_site', 'original_post_url' ), true ) ) {
$this->populate_source_site();
}

return $this->$name;
}

/**
* Magic isset method.
*
* This method is used to check if the `source_site` property is set and
* populate it if needs be.
*
* @param string $name Property name.
* @return bool
*/
public function __isset( $name ) {
if ( 'source_site' === $name && empty( $this->source_site ) ) {
$this->populate_source_site();
return ! empty( $this->source_site );
}

return isset( $this->$name );
}

/**
* Populate the source site data for internal connections.
*
Expand Down Expand Up @@ -232,42 +366,6 @@ protected function populate_source_site() {
}
}

/**
* Magic getter method.
*
* This method is used to get the value of the `source_site` property and
* populate it if needs be. For internal connections the post permalink is
* updated with live data.
*
* @param string $name Property name.
* @return mixed
*/
public function __get( $name ) {
if ( in_array( $name, array( 'source_site', 'original_post_url' ), true ) ) {
$this->populate_source_site();
}

return $this->$name;
}

/**
* Magic isset method.
*
* This method is used to check if the `source_site` property is set and
* populate it if needs be.
*
* @param string $name Property name.
* @return bool
*/
public function __isset( $name ) {
if ( 'source_site' === $name && empty( $this->source_site ) ) {
$this->populate_source_site();
return ! empty( $this->source_site );
}

return isset( $this->$name );
}

/**
* Determines whether the post has blocks.
*
Expand All @@ -279,7 +377,7 @@ public function __isset( $name ) {
*
* @return bool Whether the post has blocks.
*/
public function has_blocks() {
protected function has_blocks() {
return has_blocks( $this->post->post_content );
}

Expand All @@ -295,7 +393,7 @@ public function has_blocks() {
* @param string $block_name Full block type to look for.
* @return bool Whether the post content contains the specified block.
*/
public function has_block( $block_name ) {
protected function has_block( $block_name ) {
return has_block( $block_name, $this->post->post_content );
}

Expand All @@ -304,7 +402,7 @@ public function has_block( $block_name ) {
*
* @return int Post ID.
*/
public function get_the_id() {
protected function get_the_id() {
return $this->post->ID;
}

Expand All @@ -313,7 +411,7 @@ public function get_the_id() {
*
* @return string Post permalink.
*/
public function get_permalink() {
protected function get_permalink() {
return get_permalink( $this->post );
}

Expand All @@ -322,7 +420,7 @@ public function get_permalink() {
*
* @return string Post type.
*/
public function get_post_type() {
protected function get_post_type() {
return get_post_type( $this->post );
}

Expand All @@ -331,7 +429,7 @@ public function get_post_type() {
*
* @return int|false Post thumbnail ID or false if no thumbnail is set.
*/
public function get_post_thumbnail_id() {
protected function get_post_thumbnail_id() {
return get_post_thumbnail_id( $this->post );
}

Expand All @@ -341,7 +439,7 @@ public function get_post_thumbnail_id() {
* @param string $size Thumbnail size. Defaults to 'post-thumbnail'.
* @return string|false The post's thumbnail URL or false if no thumbnail is set.
*/
public function get_post_thumbnail_url( $size = 'post-thumbnail' ) {
protected function get_post_thumbnail_url( $size = 'post-thumbnail' ) {
return get_the_post_thumbnail_url( $this->post, $size );
}

Expand All @@ -352,7 +450,7 @@ public function get_post_thumbnail_url( $size = 'post-thumbnail' ) {
* @param array $attr Optional. Attributes for the image markup. Default empty.
* @return string|false The post's thumbnail HTML or false if no thumbnail is set.
*/
public function get_the_post_thumbnail( $size = 'post-thumbnail', $attr = '' ) {
protected function get_the_post_thumbnail( $size = 'post-thumbnail', $attr = '' ) {
return get_the_post_thumbnail( $this->post, $size, $attr );
}

Expand All @@ -367,7 +465,7 @@ public function get_the_post_thumbnail( $size = 'post-thumbnail', $attr = '' ) {
* original source URL.
* @return string The post's canonical URL.
*/
public function get_canonical_url( $canonical_url = '' ) {
protected function get_canonical_url( $canonical_url = '' ) {
if (
$this->is_source
|| $this->original_deleted
Expand All @@ -394,7 +492,7 @@ public function get_canonical_url( $canonical_url = '' ) {
* author name does not need to be replaced by the original source name.
* @return string The post's author name.
*/
public function get_author_name( $author_name = '' ) {
protected function get_author_name( $author_name = '' ) {
$settings = Utils\get_settings();

if (
Expand Down Expand Up @@ -425,7 +523,7 @@ public function get_author_name( $author_name = '' ) {
* author link does not need to be replaced by the original source name.
* @return string The post's author link.
*/
public function get_author_link( $author_link = '' ) {
protected function get_author_link( $author_link = '' ) {
$settings = Utils\get_settings();

if (
Expand All @@ -451,7 +549,7 @@ public function get_author_link( $author_link = '' ) {
*
* @return array Array of meta data.
*/
public function get_meta() {
protected function get_meta() {
return Utils\prepare_meta( $this->post->ID );
}

Expand All @@ -462,7 +560,7 @@ public function get_meta() {
* @type WP_Term[] Post terms keyed by taxonomy.
* }
*/
public function get_terms() {
protected function get_terms() {
return Utils\prepare_taxonomy_terms( $this->post->ID );
}

Expand All @@ -471,7 +569,7 @@ public function get_terms() {
*
* @return array
*/
public function get_media() {
protected function get_media() {
$post_id = $this->post->ID;
if ( $this->has_blocks() ) {
$raw_media = $this->parse_media_blocks();
Expand Down Expand Up @@ -511,7 +609,7 @@ public function get_media() {
*
* @return WP_Post[] Array of media posts.
*/
public function parse_media_blocks() {
protected function parse_media_blocks() {
$found = false;

// Note: changes to the cache key or group should be reflected in `includes/settings.php`
Expand Down Expand Up @@ -555,7 +653,7 @@ public function parse_media_blocks() {
* @param array $block Block to parse.
* @return int[] Array of media attachment IDs.
*/
private function parse_blocks_for_attachment_id( $block ) {
protected function parse_blocks_for_attachment_id( $block ) {
$media_blocks = array(
'core/image' => 'id',
'core/audio' => 'id',
Expand Down Expand Up @@ -614,7 +712,7 @@ private function parse_blocks_for_attachment_id( $block ) {
* @type array $distributor_meta Post meta.
* }
*/
public function post_data() {
protected function post_data() {
return [
'title' => html_entity_decode( get_the_title( $this->post->ID ), ENT_QUOTES, get_bloginfo( 'charset' ) ),
'slug' => $this->post->post_name,
Expand Down Expand Up @@ -647,7 +745,7 @@ public function post_data() {
* @type array $distributor_media Media data.
* }
*/
public function to_insert() {
protected function to_insert() {
$insert = [];
$post_data = $this->post_data();
$key_mappings = [
Expand Down Expand Up @@ -677,7 +775,7 @@ public function to_insert() {
* @param int $depth Optional. Maximum depth to walk through $data. Default 512.
* @return string JSON encoded post data.
*/
public function to_json( $options = 0, $depth = 512 ) {
protected function to_json( $options = 0, $depth = 512 ) {
$post_data = $this->post_data();

/*
Expand Down
Loading