diff --git a/inc/sitemaps/class-post-type-sitemap-provider.php b/inc/sitemaps/class-post-type-sitemap-provider.php index d018ad0b3ce..df948701978 100644 --- a/inc/sitemaps/class-post-type-sitemap-provider.php +++ b/inc/sitemaps/class-post-type-sitemap-provider.php @@ -93,54 +93,15 @@ public function handles_type( $type ) { * @return array */ public function get_index_links( $max_entries ) { - global $wpdb; - $post_types = WPSEO_Post_Type::get_accessible_post_types(); - $post_types = array_filter( $post_types, [ $this, 'is_valid_post_type' ] ); - $last_modified_times = WPSEO_Sitemaps::get_last_modified_gmt( $post_types, true ); - $index = []; + $post_types = WPSEO_Post_Type::get_accessible_post_types(); + $post_types = array_filter( $post_types, [ $this, 'is_valid_post_type' ] ); + $index_links = []; foreach ( $post_types as $post_type ) { - - $total_count = $this->get_post_type_count( $post_type ); - - if ( $total_count === 0 ) { - continue; - } - - $max_pages = 1; - if ( $total_count > $max_entries ) { - $max_pages = (int) ceil( $total_count / $max_entries ); - } - - $all_dates = []; - - if ( $max_pages > 1 ) { - $all_dates = version_compare( $wpdb->db_version(), '8.0', '>=' ) ? $this->get_all_dates_using_with_clause( $post_type, $max_entries ) : $this->get_all_dates( $post_type, $max_entries ); - } - - for ( $page_counter = 0; $page_counter < $max_pages; $page_counter++ ) { - - $current_page = ( $page_counter === 0 ) ? '' : ( $page_counter + 1 ); - $date = false; - - if ( empty( $current_page ) || $current_page === $max_pages ) { - - if ( ! empty( $last_modified_times[ $post_type ] ) ) { - $date = $last_modified_times[ $post_type ]; - } - } - else { - $date = $all_dates[ $page_counter ]; - } - - $index[] = [ - 'loc' => WPSEO_Sitemaps_Router::get_base_url( $post_type . '-sitemap' . $current_page . '.xml' ), - 'lastmod' => $date, - ]; - } + $index_links = array_merge( $index_links, $this->get_index_links_for_post_type( $post_type, $max_entries ) ); } - return $index; + return $index_links; } /** @@ -155,55 +116,39 @@ public function get_index_links( $max_entries ) { * @throws OutOfBoundsException When an invalid page is requested. */ public function get_sitemap_links( $type, $max_entries, $current_page ) { - - $links = []; - $post_type = $type; + $links = []; + $post_type = $type; + $current_page = max( $current_page, 1 ); if ( ! $this->is_valid_post_type( $post_type ) ) { throw new OutOfBoundsException( 'Invalid sitemap page requested' ); } - $steps = min( 100, $max_entries ); - $offset = ( $current_page > 1 ) ? ( ( $current_page - 1 ) * $max_entries ) : 0; - $total = ( $offset + $max_entries ); - - $post_type_entries = $this->get_post_type_count( $post_type ); - - if ( $total > $post_type_entries ) { - $total = $post_type_entries; - } - if ( $current_page === 1 ) { - $links = array_merge( $links, $this->get_first_links( $post_type ) ); + $links = $this->get_first_links( $post_type ); } - // If total post type count is lower than the offset, an invalid page is requested. - if ( $post_type_entries < $offset ) { - throw new OutOfBoundsException( 'Invalid sitemap page requested' ); - } - - if ( $post_type_entries === 0 ) { - return $links; + // Even if we don't have posts for the page, we might have some first-page links to show. + try { + $page_boundaries = $this->get_page_boundaries( $post_type, $current_page, $max_entries ); + } catch ( OutOfBoundsException $e ) { + if ( count( $links ) > 0 ) { + return $links; + } + throw $e; } - $posts_to_exclude = $this->get_excluded_posts( $type ); - - while ( $total > $offset ) { - - $posts = $this->get_posts( $post_type, $steps, $offset ); - - $offset += $steps; - + $start_post_id = $page_boundaries['start']; + while ( $start_post_id <= $page_boundaries['end'] ) { + $posts = $this->get_posts( $post_type, $max_entries, $start_post_id, $page_boundaries['end'] ); if ( empty( $posts ) ) { - continue; + break; } - foreach ( $posts as $post ) { - - if ( in_array( $post->ID, $posts_to_exclude, true ) ) { - continue; - } + $last_id = (int) current( array_slice( $posts, -1 ) )->ID; + $start_post_id = ( $last_id + 1 ); + foreach ( $posts as $post ) { if ( WPSEO_Meta::get_value( 'meta-robots-noindex', $post->ID ) === '1' ) { continue; } @@ -227,8 +172,6 @@ public function get_sitemap_links( $type, $max_entries, $current_page ) { $links[] = $url; } } - - unset( $post, $url ); } return $links; @@ -308,46 +251,6 @@ protected function get_excluded_posts( $post_type ) { return array_unique( $excluded_posts_ids ); } - /** - * Get count of posts for post type. - * - * @param string $post_type Post type to retrieve count for. - * - * @return int - */ - protected function get_post_type_count( $post_type ) { - - global $wpdb; - - /** - * Filter JOIN query part for type count of post type. - * - * @param string $join SQL part, defaults to empty string. - * @param string $post_type Post type name. - */ - $join_filter = apply_filters( 'wpseo_typecount_join', '', $post_type ); - - /** - * Filter WHERE query part for type count of post type. - * - * @param string $where SQL part, defaults to empty string. - * @param string $post_type Post type name. - */ - $where_filter = apply_filters( 'wpseo_typecount_where', '', $post_type ); - - $where = $this->get_sql_where_clause( $post_type ); - - $sql = " - SELECT COUNT({$wpdb->posts}.ID) - FROM {$wpdb->posts} - {$join_filter} - {$where} - {$where_filter} - "; - - return (int) $wpdb->get_var( $sql ); - } - /** * Produces set of links to prepend at start of first sitemap page. * @@ -356,9 +259,7 @@ protected function get_post_type_count( $post_type ) { * @return array */ protected function get_first_links( $post_type ) { - - $links = []; - $archive_url = false; + $links = []; if ( $post_type === 'page' ) { @@ -384,7 +285,8 @@ protected function get_first_links( $post_type ) { /** * Filter images to be included for the term in XML sitemap. * - * @param array $images Array of image items. + * @param array $images Array of image items. + * * @return array $image_list Array of image items. */ $image_list = apply_filters( 'wpseo_sitemap_urlimages_front_page', $images ); @@ -394,30 +296,29 @@ protected function get_first_links( $post_type ) { $links[] = $front_page; } - elseif ( $post_type !== 'page' ) { + else { /** * Filter the URL Yoast SEO uses in the XML sitemap for this post type archive. * * @param string $archive_url The URL of this archive * @param string $post_type The post type this archive is for. */ - $archive_url = apply_filters( + $archive_url = (string) apply_filters( 'wpseo_sitemap_post_type_archive_link', $this->get_post_type_archive_link( $post_type ), $post_type ); - } - if ( $archive_url ) { + if ( $archive_url !== '' ) { + $links[] = [ + 'loc' => $archive_url, + 'mod' => WPSEO_Sitemaps::get_last_modified_gmt( $post_type ), - $links[] = [ - 'loc' => $archive_url, - 'mod' => WPSEO_Sitemaps::get_last_modified_gmt( $post_type ), - - // Deprecated, kept for backwards data compat. R. - 'chf' => 'daily', - 'pri' => 1, - ]; + // Deprecated, kept for backwards data compat. R. + 'chf' => 'daily', + 'pri' => 1, + ]; + } } /** @@ -499,18 +400,45 @@ protected function is_post_type_archive_indexable( $post_type, $archive_page_id /** * Retrieve set of posts with optimized query routine. * - * @param string $post_type Post type to retrieve. - * @param int $count Count of posts to retrieve. - * @param int $offset Starting offset. + * @param string $post_type Post type to retrieve. + * @param int $count Count of posts to retrieve. + * @param int $offset Starting offset. + * @param int|null $max_post_id The maximum post ID to retrieve. * - * @return object[] + * @return WP_Post[] */ - protected function get_posts( $post_type, $count, $offset ) { + protected function get_posts( $post_type, $count, $offset, $max_post_id = null ) { + $raw_post_data = $this->get_raw_post_data( $post_type, $count, $offset, $max_post_id ); + $posts = []; + $post_ids = []; + foreach ( $raw_post_data as $row_index => $post_data ) { + $post_data->post_type = $post_type; + $sanitized_post = sanitize_post( $post_data, 'raw' ); + $posts[ $row_index ] = new WP_Post( $sanitized_post ); + + $post_ids[] = $sanitized_post->ID; + } + + // We'll need meta values for individual posts, so let's prime the cache with a single query instead of one per post. + update_meta_cache( 'post', $post_ids ); + + return $posts; + } + /** + * Retrieve raw post data for a given post type within a range of post IDs. + * + * @param string $post_type The post type to retrieve posts for. + * @param int $count The maximum number of posts to retrieve. + * @param int $offset The starting post ID. + * @param int|null $max_post_id The maximum post ID to retrieve. + * + * @return stdClass[] + */ + protected function get_raw_post_data( $post_type, $count, $offset, $max_post_id = null ) { global $wpdb; static $filters = []; - if ( ! isset( $filters[ $post_type ] ) ) { // Make sure you're wpdb->preparing everything you throw into this!! $filters[ $post_type ] = [ @@ -536,39 +464,137 @@ protected function get_posts( $post_type, $count, $offset ) { $where_filter = $filters[ $post_type ]['where']; $where = $this->get_sql_where_clause( $post_type ); + $page_limit_clause = ( $max_post_id ) ? "AND {$wpdb->posts}.ID <= {$max_post_id}" : ''; + /* * Optimized query per this thread: * {@link http://wordpress.org/support/topic/plugin-wordpress-seo-by-yoast-performance-suggestion}. * Also see {@link http://explainextended.com/2009/10/23/mysql-order-by-limit-performance-late-row-lookups/}. */ + $sql = " - SELECT l.ID, post_title, post_content, post_name, post_parent, post_author, post_status, post_modified_gmt, post_date, post_date_gmt + SELECT l.ID, post_title, post_content, post_name, post_parent, post_author, post_status, post_modified_gmt, post_date, post_date_gmt, post_password FROM ( SELECT {$wpdb->posts}.ID FROM {$wpdb->posts} {$join_filter} {$where} {$where_filter} - ORDER BY {$wpdb->posts}.post_modified ASC LIMIT %d OFFSET %d + AND {$wpdb->posts}.ID >= %d + {$page_limit_clause} + ORDER BY {$wpdb->posts}.ID ASC LIMIT %d ) o JOIN {$wpdb->posts} l ON l.ID = o.ID "; - $posts = $wpdb->get_results( $wpdb->prepare( $sql, $count, $offset ) ); + // phpcs:disable WordPress.DB.DirectDatabaseQuery.NoCaching,WordPress.DB.DirectDatabaseQuery.DirectQuery -- Can't do this with WP queries. Caching is done on the sitemap level. + $raw_post_data = $wpdb->get_results( $wpdb->prepare( $sql, $offset, $count ) ); + // phpcs:enable - $post_ids = []; + return array_values( $raw_post_data ); + } - foreach ( $posts as $post_index => $post ) { - $post->post_type = $post_type; - $sanitized_post = sanitize_post( $post, 'raw' ); - $posts[ $post_index ] = new WP_Post( $sanitized_post ); + /** + * Gets aggregated data for all pages in a pagination map for a post type. + * Empty pages won't show up in the results. + * Posts with IDs that aren't covered by the pagination map will be added to page 1. + * + * @param array $pagination_map The pagination map to use for grouping results. + * @param string $post_type The post type to get the aggregates for. + * + * @return array{page: int, last_mod_gmt: string}[] An array of aggregates with page. Page numbers start at 1. + */ + private function get_page_aggregates( $pagination_map, string $post_type ) { + global $wpdb; - $post_ids[] = $sanitized_post->ID; + if ( empty( $pagination_map ) ) { + return []; } - update_meta_cache( 'post', $post_ids ); + $where_clause = $this->get_sql_where_clause( $post_type ); + $cases = []; + $replacements = []; + foreach ( $pagination_map as $page_number => $page ) { + $cases[] = 'WHEN ID >= %d AND ID <= %d THEN %d'; + array_push( $replacements, $page['start'], $page['end'], $page_number ); + } - return $posts; + $query = $wpdb->prepare( + 'SELECT + CASE ' . implode( ' ', $cases ) . " ELSE 1 END as page, + MAX(post_modified_gmt) as last_mod_gmt, + COUNT(ID) as count + FROM {$wpdb->posts} + {$where_clause} + GROUP by page + ORDER BY page ASC", + ...$replacements + ); + + return (array) $wpdb->get_results( $query, OBJECT_K ); + } + + /** + * Gets the first id of the post of all pages. + * + * @param string $post_type The post type to retrieve posts for. + * @param int $entries_per_page The maximum number of posts per page. + * @param int $starting_post_id The lowest post ID to get the pages for. + * + * @return string[] + */ + protected function get_raw_page_data( $post_type, $entries_per_page, $starting_post_id ) { + global $wpdb; + $where = $this->get_sql_where_clause( $post_type ); + + // Run a different query for MySQL 8.0 and up. It's ever so slightly faster than the 5.7 alternative and doesn't produce a warning. + // It is, however, not compatible with mysql < 8.0. + if ( version_compare( $wpdb->db_version(), '8.0', '>=' ) ) { + return $wpdb->get_col( + $wpdb->prepare( + " + WITH pages AS (SELECT ROW_NUMBER() OVER (ORDER BY %i.ID) AS n, %i.ID as first_id_on_page + FROM %i USE INDEX ( `type_status_date` ) + {$where} + AND %i.ID >= %d + ORDER BY %i.ID ASC) + SELECT first_id_on_page + FROM `pages` + WHERE MOD(n-1, %d) = 0 + ", + $wpdb->posts, + $wpdb->posts, + $wpdb->posts, + $wpdb->posts, + $starting_post_id, + $wpdb->posts, + $entries_per_page + ) + ); + } + + /* + * Optimized query per this thread: + * {@link http://wordpress.org/support/topic/plugin-wordpress-seo-by-yoast-performance-suggestion}. + * Also see {@link http://explainextended.com/2009/10/23/mysql-order-by-limit-performance-late-row-lookups/}. + */ + $sql = " + SELECT l.ID as first_id_on_page + FROM ( + SELECT {$wpdb->posts}.ID + FROM ( SELECT @rownum:=-1 ) as init + JOIN {$wpdb->posts} USE INDEX (`type_status_date`) + {$where} + AND {$wpdb->posts}.ID >= %d + ORDER BY {$wpdb->posts}.ID ASC + ) + o JOIN {$wpdb->posts} l ON l.ID = o.ID + WHERE (@rownum:=@rownum+1) %% %d = 0 + "; + + // phpcs:disable WordPress.DB.DirectDatabaseQuery.NoCaching,WordPress.DB.DirectDatabaseQuery.DirectQuery -- Can't do this with WP queries. Caching is done on the sitemap level. + return $wpdb->get_col( $wpdb->prepare( $sql, $starting_post_id, $entries_per_page ) ); + // phpcs:enable } /** @@ -581,6 +607,7 @@ protected function get_posts( $post_type, $count, $offset ) { protected function get_sql_where_clause( $post_type ) { global $wpdb; + $posts_to_exclude = $this->get_excluded_posts( $post_type ); $join = ''; $post_statuses = array_map( 'esc_sql', WPSEO_Sitemaps::get_post_statuses( $post_type ) ); @@ -601,7 +628,11 @@ protected function get_sql_where_clause( $post_type ) { AND {$wpdb->posts}.post_date != '0000-00-00 00:00:00' "; - return $wpdb->prepare( $where_clause, $post_type ); + if ( count( $posts_to_exclude ) > 0 ) { + $where_clause .= "AND {$wpdb->posts}.ID NOT IN (" . implode( ', ', array_fill( 0, count( $posts_to_exclude ), '%d' ) ) . ')'; + } + + return $wpdb->prepare( $where_clause, $post_type, ...$posts_to_exclude ); } /** @@ -668,99 +699,320 @@ protected function get_url( $post ) { } /** - * Get all dates for a post type by using the WITH clause for performance. + * Gets the boundaries of a single post sitemap page of a post type. * - * @param string $post_type Post type to retrieve dates for. - * @param int $max_entries Maximum number of entries to retrieve. + * @param string $post_type The post type to get the boundaries for. + * @param int $page_number The page number to get the boundaries for. Starting at 1. + * @param int $max_entries_per_page The maximum number of links that should be visible on a page. * - * @return array Array of dates. + * @return int[] + * @throws OutOfBoundsException When an invalid page is requested. */ - private function get_all_dates_using_with_clause( $post_type, $max_entries ) { - global $wpdb; + private function get_page_boundaries( $post_type, $page_number, $max_entries_per_page ) { + $map = $this->get_pagination_map( $post_type, $max_entries_per_page ); + if ( ! isset( $map[ $page_number ] ) ) { + throw new OutOfBoundsException( 'Invalid sitemap page requested' ); + } - $post_statuses = array_map( 'esc_sql', WPSEO_Sitemaps::get_post_statuses( $post_type ) ); + return $map[ $page_number ]; + } - $replacements = array_merge( - [ - 'ordering', - 'post_modified_gmt', - $wpdb->posts, - 'type_status_date', - 'post_status', - ], - $post_statuses, - [ - 'post_type', + /** + * Gets a full pagination map for a post type. + * The map is an array of page numbers with the starting and ending post id of the page. + * When the map is malformed or missing, it will be rebuilt. + * When the map is incomplete, it will be completed. + * + * @param string $post_type The post type to get the map for. + * @param int $max_entries_per_page The maximum number of links that should be visible on a page. + * + * @return array + */ + private function get_pagination_map( $post_type, $max_entries_per_page ) { + $pagination = get_option( "wpseo_{$post_type}_sitemap_pagination", [] ); + $aggregates = $this->get_aggregates( $post_type ); + + if ( $this->pagination_is_malformed( $pagination ) || $this->map_needs_to_be_rebuilt( $pagination, $aggregates, $max_entries_per_page ) ) { + $pagination_map = $this->create_map( $post_type, - 'post_modified_gmt', - 'post_modified_gmt', - 'ordering', - $max_entries, - ] - ); + $max_entries_per_page, + $aggregates['min_post_id'], + $aggregates['max_post_id'] + ); + $this->save_pagination( $post_type, $pagination_map, $aggregates, $max_entries_per_page ); - //phpcs:disable WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching -- We need to use a direct query here. - //phpcs:disable WordPress.DB.DirectDatabaseQuery.NoCaching -- Reason: No relevant caches. - return $wpdb->get_col( - //phpcs:disable WordPress.DB.PreparedSQLPlaceholders -- %i placeholder is still not recognized. - $wpdb->prepare( - ' - WITH %i AS (SELECT ROW_NUMBER() OVER (ORDER BY %i) AS n, post_modified_gmt - FROM %i USE INDEX ( %i ) - WHERE %i IN (' . implode( ', ', array_fill( 0, count( $post_statuses ), '%s' ) ) . ') - AND %i = %s - ORDER BY %i) - SELECT %i - FROM %i - WHERE MOD(n, %d) = 0; - ', - $replacements + return $pagination_map; + } + + // If the map is outdated, replace the last page with an updated version and complete the map. + if ( $this->map_is_missing_posts( $pagination, $aggregates ) ) { + $pagination_map = $pagination['map']; + // The existing map might be empty. E.g., when a post_type only has password protected posts. + if ( empty( $pagination_map ) ) { + $last_known_page = 1; + $first_id_on_last_page = $aggregates['min_post_id']; + } + else { + $last_known_page = max( array_keys( $pagination_map ) ); + $first_id_on_last_page = $pagination_map[ $last_known_page ]['start']; + } + array_pop( $pagination_map ); + + $final_pages = $this->create_map( + $post_type, + $max_entries_per_page, + $first_id_on_last_page, + $aggregates['max_post_id'], + $last_known_page + ); + + // Loop over pages instead of using array_merge to avoid losing array keys. + foreach ( $final_pages as $page_number => $page_boundaries ) { + $pagination_map[ $page_number ] = $page_boundaries; + } + + $this->save_pagination( $post_type, $pagination_map, $aggregates, $max_entries_per_page ); + + return $pagination_map; + } + + return $pagination['map']; + } + + /** + * Gets a map of the starting and ending post id of sitemap pages for a post_type. + * + * @param string $post_type The post type to create the map for. + * @param int $max_entries_per_page The maximum number of links that should be visible on a page. + * @param int $min_post_id The minimum post id to start the map from. The lowest known post ID when creating a new map. + * @param int $max_post_id The highest known post ID. + * @param int $starting_page The page number to start the map from. Defaults to 1. + * + * @return array + */ + private function create_map( $post_type, $max_entries_per_page, $min_post_id, $max_post_id, $starting_page = 1 ) { + $map = []; + $current_page_id = max( $starting_page, 1 ); + + $page_starting_ids = $this->get_raw_page_data( $post_type, $max_entries_per_page, $min_post_id ); + // If there's no data, we'll create a single page that would encompass all posts in case they would become visible in the sitemap. + if ( empty( $page_starting_ids ) ) { + return [ + $current_page_id => [ + 'start' => $min_post_id, + 'end' => $max_post_id, + ], + ]; + } + + foreach ( $page_starting_ids as $index => $page_starting_id ) { + $next_page_starting_id = isset( $page_starting_ids[ ( $index + 1 ) ] ) ? ( $page_starting_ids[ ( $index + 1 ) ] ) : null; + $map[ $current_page_id ] = [ + 'start' => ( $current_page_id === $page_starting_id ) ? $min_post_id : (int) $page_starting_id, + 'end' => ( $next_page_starting_id ) ? ( $next_page_starting_id - 1 ) : $max_post_id, + ]; + ++$current_page_id; + } + + return $map; + } + + /** + * Checks the structure of the pagination object. + * + * @param array $stored_pagination The stored pagination object to validate. + * + * @return bool + */ + private function pagination_is_malformed( $stored_pagination ): bool { + return ( + ! isset( + $stored_pagination['min_post_id'], + $stored_pagination['max_post_id'], + $stored_pagination['total_number_of_posts'], + $stored_pagination['max_entries_per_page'], + $stored_pagination['map'] ) + || $this->map_is_malformed( $stored_pagination['map'] ) ); } /** - * Get all dates for a post type. + * Checks the structure of the pagination map. + * A map is malformed when it is not an array or when it contains pages with a start ID higher than the end ID. * - * @param string $post_type Post type to retrieve dates for. - * @param int $max_entries Maximum number of entries to retrieve. + * @param array{start: int, end: int} $map The map. * - * @return array Array of dates. + * @return bool */ - private function get_all_dates( $post_type, $max_entries ) { - global $wpdb; + private function map_is_malformed( $map ): bool { + if ( ! is_array( $map ) ) { + return true; + } + foreach ( $map as $page ) { + if ( ! isset( $page['start'], $page['end'] ) || $page['end'] < $page['start'] ) { + return true; + } + } - $post_statuses = array_map( 'esc_sql', WPSEO_Sitemaps::get_post_statuses( $post_type ) ); - $replacements = array_merge( - [ - 'post_modified_gmt', - $wpdb->posts, - 'type_status_date', - 'post_status', - ], - $post_statuses, + return false; + } + + /** + * Checks if the map in a pagination object needs to be rebuilt completely. + * This is the case when: + * 1. posts exists with a lower ID than what the pagination object knows about; + * 2. the map has pages for posts with a higher ID than the highest ID in the database for this post type; + * 3. the total number of posts shrunk since creating the last pagination object. + * This can happen when posts are deleted, but could also hint at bigger site structure changes. + * 4. the site has been migrated to a different domain. + * + * This will trigger some false positives, causing some unnecessary rebuilds, + * but this will cover cases where bigger structural changes have been made that would otherwise go undetected. + * + * @param array $stored_pagination The stored pagination object. + * @param array{min_post_id: int, max_post_id: int, total_number_of_posts: int} $current_aggregates The current aggregates for the post type. + * @param int $max_entries_per_page The maximum number of links that should be visible on a page. + * + * @return bool + */ + private function map_needs_to_be_rebuilt( $stored_pagination, $current_aggregates, $max_entries_per_page ): bool { + return $stored_pagination['min_post_id'] !== $current_aggregates['min_post_id'] + || $stored_pagination['max_post_id'] > $current_aggregates['max_post_id'] + || $stored_pagination['total_number_of_posts'] > $current_aggregates['total_number_of_posts'] + || $stored_pagination['max_entries_per_page'] !== $max_entries_per_page + || $stored_pagination['site_url'] !== get_site_url(); + } + + /** + * Checks if the map in a pagination object is missing posts that have been published since the last update of the pagination object. + * + * @param array $stored_pagination The stored pagination object. + * @param array{min_post_id: int, max_post_id: int, total_number_of_posts: int} $current_aggregates The current aggregates for the post type. + * + * @return bool + */ + private function map_is_missing_posts( $stored_pagination, $current_aggregates ): bool { + return $stored_pagination['max_post_id'] < $current_aggregates['max_post_id']; + } + + /** + * Saves a pagination object to the database. + * + * @param string $post_type The post type to save the pagination object for. + * @param array $pagination_map The map to save as part of the pagination object. + * @param array{min_post_id: int, max_post_id: int, total_number_of_posts: int} $aggregates The aggregates that were used when creating the map. + * @param int $max_entries_per_page The maximum number of links that should be visible on a page. + * + * @return void + */ + private function save_pagination( $post_type, $pagination_map, $aggregates, $max_entries_per_page ): void { + update_option( + "wpseo_{$post_type}_sitemap_pagination", [ - 'post_type', - $post_type, - $max_entries, - 'post_modified_gmt', + 'map' => $pagination_map, + 'min_post_id' => $aggregates['min_post_id'], + 'max_post_id' => $aggregates['max_post_id'], + 'total_number_of_posts' => $aggregates['total_number_of_posts'], + 'max_entries_per_page' => $max_entries_per_page, + 'site_url' => get_site_url(), ] ); + } + + /** + * Gets links to show on the sitemap index/overview for a particular post type. + * + * @param string $post_type The post type to get index links for. + * @param int $max_entries The maximum number of links that should be visible on a page. + * + * @return array{loc: string, lastmod: string} + */ + protected function get_index_links_for_post_type( string $post_type, int $max_entries ): array { + $index_links = []; + $pagination_map = $this->get_pagination_map( $post_type, $max_entries ); + $pages = $this->get_page_aggregates( $pagination_map, $post_type ); + + // If there's no pages with posts, check for any "first links" to create a page 1 with. + if ( empty( $pages ) ) { + $first_links = $this->get_first_links( $post_type ); + if ( ! $first_links ) { + return $index_links; + } + + $last_mod_gmt = array_reduce( $first_links, [ $this, 'reduce_most_recent_last_mod_for_links' ], ( $first_links[0]['mod'] ?? null ) ); - return $wpdb->get_col( - //phpcs:disable WordPress.DB.PreparedSQLPlaceholders -- %i placeholder is still not recognized. + $index_links[] = [ + 'loc' => WPSEO_Sitemaps_Router::get_base_url( $post_type . '-sitemap.xml' ), + 'lastmod' => $last_mod_gmt, + ]; + } + + foreach ( $pages as $page_number => $page ) { + $page_link_suffix = ( $page_number > 1 ) ? $page_number : ''; + $last_mod_gmt = $page->last_mod_gmt; + if ( $page_number === 1 ) { + $first_links = $this->get_first_links( $post_type ); + if ( ! $first_links && ! $last_mod_gmt ) { + continue; + } + + $last_mod_gmt = array_reduce( $first_links, [ $this, 'reduce_most_recent_last_mod_for_links' ], $last_mod_gmt ); + } + + $index_links[] = [ + 'loc' => WPSEO_Sitemaps_Router::get_base_url( $post_type . '-sitemap' . $page_link_suffix . '.xml' ), + 'lastmod' => $last_mod_gmt, + ]; + } + + return $index_links; + } + + /** + * Returns the most recent last modified date for a set of links. + * Intended to be used with array_reduce. + * + * @param string $carry The string representation most recent last modified date. e.g. '2021-02-03 01:02:03'. + * @param array{loc: string, lastmod: string} $link The link to compare to the carry. + * + * @return string The most recent last modified date. + */ + private function reduce_most_recent_last_mod_for_links( $carry, $link ) { + if ( ! isset( $link['mod'] ) ) { + return $carry; + } + + return max( $link['mod'], $carry ); + } + + /** + * Gets current aggregates for a post type. + * + * @param string $post_type The post type to get aggregates for. + * + * @return array{min_post_id: int, max_post_id: int, total_number_of_posts: int} + */ + protected function get_aggregates( $post_type ): array { + global $wpdb; + // phpcs:disable WordPress.DB.DirectDatabaseQuery.NoCaching,WordPress.DB.DirectDatabaseQuery.DirectQuery -- Can't do this with WP queries. Caching is done on the sitemap level. + $aggregates = (array) $wpdb->get_row( $wpdb->prepare( - ' - SELECT %i - FROM ( SELECT @rownum:=0 ) init - JOIN %i USE INDEX( %i ) - WHERE %i IN (' . implode( ', ', array_fill( 0, count( $post_statuses ), '%s' ) ) . ') - AND %i = %s - AND ( @rownum:=@rownum+1 ) %% %d = 0 - ORDER BY %i ASC - ', - $replacements + "SELECT + MIN( ID ) AS min_post_id, + MAX( ID ) AS max_post_id, + COUNT( ID ) AS total_number_of_posts + FROM {$wpdb->posts} + WHERE post_type = %s", + $post_type ) ); + + // phpcs:enable + return [ + 'min_post_id' => (int) $aggregates['min_post_id'], + 'max_post_id' => (int) $aggregates['max_post_id'], + 'total_number_of_posts' => (int) $aggregates['total_number_of_posts'], + ]; } } diff --git a/inc/sitemaps/class-sitemaps.php b/inc/sitemaps/class-sitemaps.php index 2fbb567c1fe..608622b3945 100644 --- a/inc/sitemaps/class-sitemaps.php +++ b/inc/sitemaps/class-sitemaps.php @@ -262,12 +262,15 @@ public function redirect( $query ) { return; } + // This may be the type of sitemap that's being requested or the value "1" for the root sitemap. $type = get_query_var( 'sitemap' ); + // example.com/sitemap-.xml is not a valid request. if ( empty( $type ) ) { return; } + // We don't use "page" numbers for the first (or 0th) page in a sitemap. if ( get_query_var( 'sitemap_n' ) === '1' || get_query_var( 'sitemap_n' ) === '0' ) { wp_safe_redirect( home_url( "/$type-sitemap.xml" ), 301, 'Yoast SEO' ); exit; diff --git a/src/actions/importing/aioseo/aioseo-posts-importing-action.php b/src/actions/importing/aioseo/aioseo-posts-importing-action.php index 86f500eaecb..257bdfdd207 100644 --- a/src/actions/importing/aioseo/aioseo-posts-importing-action.php +++ b/src/actions/importing/aioseo/aioseo-posts-importing-action.php @@ -284,7 +284,7 @@ public function index() { if ( $this->indexable_helper->check_if_default_indexable( $indexable, $check_defaults_fields ) ) { $indexable = $this->map( $indexable, $aioseo_indexable ); - $indexable->save(); + $this->indexable_helper->save_indexable( $indexable ); // To ensure that indexables can be rebuild after a reset, we have to store the data in the postmeta table too. $this->indexable_to_postmeta->map_to_postmeta( $indexable ); diff --git a/src/actions/indexing/abstract-link-indexing-action.php b/src/actions/indexing/abstract-link-indexing-action.php index d25ca433a0f..0ae29336ce3 100644 --- a/src/actions/indexing/abstract-link-indexing-action.php +++ b/src/actions/indexing/abstract-link-indexing-action.php @@ -4,6 +4,7 @@ use wpdb; use Yoast\WP\SEO\Builders\Indexable_Link_Builder; +use Yoast\WP\SEO\Helpers\Indexable_Helper; use Yoast\WP\SEO\Models\SEO_Links; use Yoast\WP\SEO\Repositories\Indexable_Repository; @@ -19,6 +20,13 @@ abstract class Abstract_Link_Indexing_Action extends Abstract_Indexing_Action { */ protected $link_builder; + /** + * The indexable helper. + * + * @var Indexable_Helper + */ + protected $indexable_helper; + /** * The indexable repository. * @@ -36,18 +44,21 @@ abstract class Abstract_Link_Indexing_Action extends Abstract_Indexing_Action { /** * Indexable_Post_Indexing_Action constructor * - * @param Indexable_Link_Builder $link_builder The indexable link builder. - * @param Indexable_Repository $repository The indexable repository. - * @param wpdb $wpdb The WordPress database instance. + * @param Indexable_Link_Builder $link_builder The indexable link builder. + * @param Indexable_Helper $indexable_helper The indexable repository. + * @param Indexable_Repository $repository The indexable repository. + * @param wpdb $wpdb The WordPress database instance. */ public function __construct( Indexable_Link_Builder $link_builder, + Indexable_Helper $indexable_helper, Indexable_Repository $repository, wpdb $wpdb ) { - $this->link_builder = $link_builder; - $this->repository = $repository; - $this->wpdb = $wpdb; + $this->link_builder = $link_builder; + $this->indexable_helper = $indexable_helper; + $this->repository = $repository; + $this->wpdb = $wpdb; } /** @@ -63,7 +74,7 @@ public function index() { $indexable = $this->repository->find_by_id_and_type( $object->id, $object->type ); if ( $indexable ) { $this->link_builder->build( $indexable, $object->content ); - $indexable->save(); + $this->indexable_helper->save_indexable( $indexable ); $indexables[] = $indexable; } diff --git a/src/builders/indexable-builder.php b/src/builders/indexable-builder.php index b882c1f0dd2..f0c4b727004 100644 --- a/src/builders/indexable-builder.php +++ b/src/builders/indexable-builder.php @@ -267,49 +267,6 @@ protected function ensure_indexable( $indexable, $defaults = [] ) { return $indexable; } - /** - * Saves and returns an indexable (on production environments only). - * - * @param Indexable $indexable The indexable. - * @param Indexable|null $indexable_before The indexable before possible changes. - * - * @return Indexable The indexable. - */ - protected function save_indexable( $indexable, $indexable_before = null ) { - $intend_to_save = $this->indexable_helper->should_index_indexables(); - - /** - * Filter: 'wpseo_should_save_indexable' - Allow developers to enable / disable - * saving the indexable when the indexable is updated. Warning: overriding - * the intended action may cause problems when moving from a staging to a - * production environment because indexable permalinks may get set incorrectly. - * - * @param bool $intend_to_save True if YoastSEO intends to save the indexable. - * @param Indexable $indexable The indexable to be saved. - */ - $intend_to_save = \apply_filters( 'wpseo_should_save_indexable', $intend_to_save, $indexable ); - - if ( ! $intend_to_save ) { - return $indexable; - } - - // Save the indexable before running the WordPress hook. - $indexable->save(); - - if ( $indexable_before ) { - /** - * Action: 'wpseo_save_indexable' - Allow developers to perform an action - * when the indexable is updated. - * - * @param Indexable $indexable The saved indexable. - * @param Indexable $indexable_before The indexable before saving. - */ - \do_action( 'wpseo_save_indexable', $indexable, $indexable_before ); - } - - return $indexable; - } - /** * Build and author indexable from an author id if it does not exist yet, or if the author indexable needs to be upgraded. * @@ -372,7 +329,7 @@ public function build( $indexable, $defaults = null ) { $indexable = $this->post_builder->build( $indexable->object_id, $indexable ); // Save indexable, to make sure it can be queried when building related objects like the author indexable and hierarchy. - $indexable = $this->save_indexable( $indexable, $indexable_before ); + $indexable = $this->indexable_helper->save_indexable( $indexable, $indexable_before ); // For attachments, we have to make sure to patch any potentially previously cleaned up SEO links. if ( \is_a( $indexable, Indexable::class ) && $indexable->object_sub_type === 'attachment' ) { @@ -398,7 +355,7 @@ public function build( $indexable, $defaults = null ) { $indexable = $this->term_builder->build( $indexable->object_id, $indexable ); // Save indexable, to make sure it can be queried when building hierarchy. - $indexable = $this->save_indexable( $indexable, $indexable_before ); + $indexable = $this->indexable_helper->save_indexable( $indexable, $indexable_before ); $this->hierarchy_builder->build( $indexable ); @@ -422,7 +379,7 @@ public function build( $indexable, $defaults = null ) { break; } - return $this->save_indexable( $indexable, $indexable_before ); + return $this->indexable_helper->save_indexable( $indexable, $indexable_before ); } catch ( Source_Exception $exception ) { if ( ! $this->is_type_with_no_id( $indexable->object_type ) && ( ! isset( $indexable->object_id ) || \is_null( $indexable->object_id ) ) ) { @@ -449,7 +406,7 @@ public function build( $indexable, $defaults = null ) { // Make sure that the indexing process doesn't get stuck in a loop on this broken indexable. $indexable = $this->version_manager->set_latest( $indexable ); - return $this->save_indexable( $indexable, $indexable_before ); + return $this->indexable_helper->save_indexable( $indexable, $indexable_before ); } catch ( Not_Built_Exception $exception ) { return false; diff --git a/src/builders/indexable-link-builder.php b/src/builders/indexable-link-builder.php index a45944e5348..ca703c2cda7 100644 --- a/src/builders/indexable-link-builder.php +++ b/src/builders/indexable-link-builder.php @@ -6,6 +6,7 @@ use WP_HTML_Tag_Processor; use WPSEO_Image_Utils; use Yoast\WP\SEO\Helpers\Image_Helper; +use Yoast\WP\SEO\Helpers\Indexable_Helper; use Yoast\WP\SEO\Helpers\Options_Helper; use Yoast\WP\SEO\Helpers\Post_Helper; use Yoast\WP\SEO\Helpers\Url_Helper; @@ -40,6 +41,13 @@ class Indexable_Link_Builder { */ protected $image_helper; + /** + * The indexable helper. + * + * @var Indexable_Helper + */ + protected $indexable_helper; + /** * The post helper. * @@ -68,17 +76,20 @@ class Indexable_Link_Builder { * @param Url_Helper $url_helper The URL helper. * @param Post_Helper $post_helper The post helper. * @param Options_Helper $options_helper The options helper. + * @param Indexable_Helper $indexable_helper The indexable helper. */ public function __construct( SEO_Links_Repository $seo_links_repository, Url_Helper $url_helper, Post_Helper $post_helper, - Options_Helper $options_helper + Options_Helper $options_helper, + Indexable_Helper $indexable_helper ) { $this->seo_links_repository = $seo_links_repository; $this->url_helper = $url_helper; $this->post_helper = $post_helper; $this->options_helper = $options_helper; + $this->indexable_helper = $indexable_helper; } /** @@ -108,6 +119,10 @@ public function set_dependencies( * @return SEO_Links[] The created SEO links. */ public function build( $indexable, $content ) { + if ( ! $this->indexable_helper->should_index_indexable( $indexable ) ) { + return []; + } + global $post; if ( $indexable->object_type === 'post' ) { $post_backup = $post; diff --git a/src/builders/primary-term-builder.php b/src/builders/primary-term-builder.php index 445827911ee..a636776a566 100644 --- a/src/builders/primary-term-builder.php +++ b/src/builders/primary-term-builder.php @@ -2,6 +2,7 @@ namespace Yoast\WP\SEO\Builders; +use Yoast\WP\SEO\Helpers\Indexable_Helper; use Yoast\WP\SEO\Helpers\Meta_Helper; use Yoast\WP\SEO\Helpers\Primary_Term_Helper; use Yoast\WP\SEO\Repositories\Primary_Term_Repository; @@ -20,6 +21,13 @@ class Primary_Term_Builder { */ protected $repository; + /** + * The indexable helper. + * + * @var Indexable_Helper + */ + private $indexable_helper; + /** * The primary term helper. * @@ -37,18 +45,21 @@ class Primary_Term_Builder { /** * Primary_Term_Builder constructor. * - * @param Primary_Term_Repository $repository The primary term repository. - * @param Primary_Term_Helper $primary_term The primary term helper. - * @param Meta_Helper $meta The meta helper. + * @param Primary_Term_Repository $repository The primary term repository. + * @param Indexable_Helper $indexable_helper The indexable helper. + * @param Primary_Term_Helper $primary_term The primary term helper. + * @param Meta_Helper $meta The meta helper. */ public function __construct( Primary_Term_Repository $repository, + Indexable_Helper $indexable_helper, Primary_Term_Helper $primary_term, Meta_Helper $meta ) { - $this->repository = $repository; - $this->primary_term = $primary_term; - $this->meta = $meta; + $this->repository = $repository; + $this->indexable_helper = $indexable_helper; + $this->primary_term = $primary_term; + $this->meta = $meta; } /** @@ -75,22 +86,22 @@ public function build( $post_id ) { protected function save_primary_term( $post_id, $taxonomy ) { $term_id = $this->meta->get_value( 'primary_' . $taxonomy, $post_id ); - $term_selected = ! empty( $term_id ); - $primary_term = $this->repository->find_by_post_id_and_taxonomy( $post_id, $taxonomy, $term_selected ); + $term_selected = ! empty( $term_id ); + $primary_term_indexable = $this->repository->find_by_post_id_and_taxonomy( $post_id, $taxonomy, $term_selected ); // Removes the indexable when no term found. if ( ! $term_selected ) { - if ( $primary_term ) { - $primary_term->delete(); + if ( $primary_term_indexable ) { + $primary_term_indexable->delete(); } return; } - $primary_term->term_id = $term_id; - $primary_term->post_id = $post_id; - $primary_term->taxonomy = $taxonomy; - $primary_term->blog_id = \get_current_blog_id(); - $primary_term->save(); + $primary_term_indexable->term_id = $term_id; + $primary_term_indexable->post_id = $post_id; + $primary_term_indexable->taxonomy = $taxonomy; + $primary_term_indexable->blog_id = \get_current_blog_id(); + $this->indexable_helper->save_indexable( $primary_term_indexable ); } } diff --git a/src/helpers/indexable-helper.php b/src/helpers/indexable-helper.php index 81317e4826e..cd2f6bb0aab 100644 --- a/src/helpers/indexable-helper.php +++ b/src/helpers/indexable-helper.php @@ -178,6 +178,28 @@ public function reset_permalink_indexables( $type = null, $subtype = null, $reas } } + /** + * Determines whether indexing the specific indexable is appropriate at this time. + * + * @param Indexable $indexable The indexable. + * + * @return bool Whether indexing the specific indexable is appropriate at this time. + */ + public function should_index_indexable( $indexable ) { + $intend_to_save = $this->should_index_indexables(); + + /** + * Filter: 'wpseo_should_save_indexable' - Allow developers to enable / disable + * saving the indexable when the indexable is updated. Warning: overriding + * the intended action may cause problems when moving from a staging to a + * production environment because indexable permalinks may get set incorrectly. + * + * @param bool $intend_to_save True if YoastSEO intends to save the indexable. + * @param Indexable $indexable The indexable to be saved. + */ + return \apply_filters( 'wpseo_should_save_indexable', $intend_to_save, $indexable ); + } + /** * Determines whether indexing indexables is appropriate at this time. * @@ -262,4 +284,36 @@ public function check_if_default_field( $indexable, $field ) { return false; } + + /** + * Saves and returns an indexable (on production environments only). + * + * Moved from Yoast\WP\SEO\Builders\Indexable_Builder. + * + * @param Indexable $indexable The indexable. + * @param Indexable|null $indexable_before The indexable before possible changes. + * + * @return bool True if default value. + */ + public function save_indexable( $indexable, $indexable_before = null ) { + if ( ! $this->should_index_indexable( $indexable ) ) { + return $indexable; + } + + // Save the indexable before running the WordPress hook. + $indexable->save(); + + if ( $indexable_before ) { + /** + * Action: 'wpseo_save_indexable' - Allow developers to perform an action + * when the indexable is updated. + * + * @param Indexable $indexable The saved indexable. + * @param Indexable $indexable_before The indexable before saving. + */ + \do_action( 'wpseo_save_indexable', $indexable, $indexable_before ); + } + + return $indexable; + } } diff --git a/src/integrations/watchers/indexable-ancestor-watcher.php b/src/integrations/watchers/indexable-ancestor-watcher.php index 88024bbb627..9c16794bfdc 100644 --- a/src/integrations/watchers/indexable-ancestor-watcher.php +++ b/src/integrations/watchers/indexable-ancestor-watcher.php @@ -4,6 +4,7 @@ use Yoast\WP\SEO\Builders\Indexable_Hierarchy_Builder; use Yoast\WP\SEO\Conditionals\Migrations_Conditional; +use Yoast\WP\SEO\Helpers\Indexable_Helper; use Yoast\WP\SEO\Helpers\Permalink_Helper; use Yoast\WP\SEO\Helpers\Post_Type_Helper; use Yoast\WP\SEO\Integrations\Integration_Interface; @@ -39,6 +40,13 @@ class Indexable_Ancestor_Watcher implements Integration_Interface { */ protected $indexable_hierarchy_repository; + /** + * The indexable helper. + * + * @var Indexable_Helper + */ + private $indexable_helper; + /** * Represents the permalink helper. * @@ -59,6 +67,7 @@ class Indexable_Ancestor_Watcher implements Integration_Interface { * @param Indexable_Repository $indexable_repository The indexable repository. * @param Indexable_Hierarchy_Builder $indexable_hierarchy_builder The indexable hierarchy builder. * @param Indexable_Hierarchy_Repository $indexable_hierarchy_repository The indexable hierarchy repository. + * @param Indexable_Helper $indexable_helper The indexable helper. * @param Permalink_Helper $permalink_helper The permalink helper. * @param Post_Type_Helper $post_type_helper The post type helper. */ @@ -66,12 +75,14 @@ public function __construct( Indexable_Repository $indexable_repository, Indexable_Hierarchy_Builder $indexable_hierarchy_builder, Indexable_Hierarchy_Repository $indexable_hierarchy_repository, + Indexable_Helper $indexable_helper, Permalink_Helper $permalink_helper, Post_Type_Helper $post_type_helper ) { $this->indexable_repository = $indexable_repository; $this->indexable_hierarchy_builder = $indexable_hierarchy_builder; $this->indexable_hierarchy_repository = $indexable_hierarchy_repository; + $this->indexable_helper = $indexable_helper; $this->permalink_helper = $permalink_helper; $this->post_type_helper = $post_type_helper; } @@ -177,7 +188,7 @@ protected function update_hierarchy_and_permalink( $indexable ) { $this->indexable_hierarchy_builder->build( $indexable ); $indexable->permalink = $this->permalink_helper->get_permalink_for_indexable( $indexable ); - $indexable->save(); + $this->indexable_helper->save_indexable( $indexable ); } } diff --git a/src/integrations/watchers/indexable-author-watcher.php b/src/integrations/watchers/indexable-author-watcher.php index b224ff9d82b..00960cdafe2 100644 --- a/src/integrations/watchers/indexable-author-watcher.php +++ b/src/integrations/watchers/indexable-author-watcher.php @@ -4,6 +4,7 @@ use Yoast\WP\SEO\Builders\Indexable_Builder; use Yoast\WP\SEO\Conditionals\Migrations_Conditional; +use Yoast\WP\SEO\Helpers\Indexable_Helper; use Yoast\WP\SEO\Integrations\Integration_Interface; use Yoast\WP\SEO\Repositories\Indexable_Repository; @@ -26,6 +27,13 @@ class Indexable_Author_Watcher implements Integration_Interface { */ protected $builder; + /** + * The indexable helper. + * + * @var Indexable_Helper + */ + private $indexable_helper; + /** * Returns the conditionals based on which this loadable should be active. * @@ -38,12 +46,18 @@ public static function get_conditionals() { /** * Indexable_Author_Watcher constructor. * - * @param Indexable_Repository $repository The repository to use. - * @param Indexable_Builder $builder The builder to use. + * @param Indexable_Repository $repository The repository to use. + * @param Indexable_Helper $indexable_helper The indexable helper. + * @param Indexable_Builder $builder The builder to use. */ - public function __construct( Indexable_Repository $repository, Indexable_Builder $builder ) { - $this->repository = $repository; - $this->builder = $builder; + public function __construct( + Indexable_Repository $repository, + Indexable_Helper $indexable_helper, + Indexable_Builder $builder + ) { + $this->repository = $repository; + $this->indexable_helper = $indexable_helper; + $this->builder = $builder; } /** @@ -90,7 +104,7 @@ public function build_indexable( $user_id ) { if ( $indexable ) { $indexable->object_last_modified = \max( $indexable->object_last_modified, \current_time( 'mysql' ) ); - $indexable->save(); + $this->indexable_helper->save_indexable( $indexable ); } } diff --git a/src/integrations/watchers/indexable-home-page-watcher.php b/src/integrations/watchers/indexable-home-page-watcher.php index bbe8a193fca..913780c62b1 100644 --- a/src/integrations/watchers/indexable-home-page-watcher.php +++ b/src/integrations/watchers/indexable-home-page-watcher.php @@ -4,6 +4,7 @@ use Yoast\WP\SEO\Builders\Indexable_Builder; use Yoast\WP\SEO\Conditionals\Migrations_Conditional; +use Yoast\WP\SEO\Helpers\Indexable_Helper; use Yoast\WP\SEO\Integrations\Integration_Interface; use Yoast\WP\SEO\Repositories\Indexable_Repository; @@ -21,6 +22,13 @@ class Indexable_Home_Page_Watcher implements Integration_Interface { */ protected $repository; + /** + * The indexable helper. + * + * @var Indexable_Helper + */ + protected $indexable_helper; + /** * The indexable builder. * @@ -40,12 +48,18 @@ public static function get_conditionals() { /** * Indexable_Home_Page_Watcher constructor. * - * @param Indexable_Repository $repository The repository to use. - * @param Indexable_Builder $builder The post builder to use. + * @param Indexable_Repository $repository The repository to use. + * @param Indexable_Helper $indexable_helper The indexable helper. + * @param Indexable_Builder $builder The post builder to use. */ - public function __construct( Indexable_Repository $repository, Indexable_Builder $builder ) { - $this->repository = $repository; - $this->builder = $builder; + public function __construct( + Indexable_Repository $repository, + Indexable_Helper $indexable_helper, + Indexable_Builder $builder + ) { + $this->repository = $repository; + $this->indexable_helper = $indexable_helper; + $this->builder = $builder; } /** @@ -108,6 +122,11 @@ public function check_option( $old_value, $new_value, $option ) { */ public function build_indexable() { $indexable = $this->repository->find_for_home_page( false ); + + if ( $indexable === false && ! $this->indexable_helper->should_index_indexables() ) { + return; + } + $indexable = $this->builder->build_for_home_page( $indexable ); if ( $indexable ) { diff --git a/src/integrations/watchers/indexable-post-type-archive-watcher.php b/src/integrations/watchers/indexable-post-type-archive-watcher.php index 6b5eb237641..2a2f8edc3c6 100644 --- a/src/integrations/watchers/indexable-post-type-archive-watcher.php +++ b/src/integrations/watchers/indexable-post-type-archive-watcher.php @@ -4,6 +4,7 @@ use Yoast\WP\SEO\Builders\Indexable_Builder; use Yoast\WP\SEO\Conditionals\Migrations_Conditional; +use Yoast\WP\SEO\Helpers\Indexable_Helper; use Yoast\WP\SEO\Integrations\Integration_Interface; use Yoast\WP\SEO\Repositories\Indexable_Repository; @@ -21,6 +22,13 @@ class Indexable_Post_Type_Archive_Watcher implements Integration_Interface { */ protected $repository; + /** + * The indexable helper. + * + * @var Indexable_Helper + */ + private $indexable_helper; + /** * The indexable builder. * @@ -40,12 +48,18 @@ public static function get_conditionals() { /** * Indexable_Post_Type_Archive_Watcher constructor. * - * @param Indexable_Repository $repository The repository to use. - * @param Indexable_Builder $builder The post builder to use. + * @param Indexable_Repository $repository The repository to use. + * @param Indexable_Helper $indexable_helper The indexable helper. + * @param Indexable_Builder $builder The post builder to use. */ - public function __construct( Indexable_Repository $repository, Indexable_Builder $builder ) { - $this->repository = $repository; - $this->builder = $builder; + public function __construct( + Indexable_Repository $repository, + Indexable_Helper $indexable_helper, + Indexable_Builder $builder + ) { + $this->repository = $repository; + $this->indexable_helper = $indexable_helper; + $this->builder = $builder; } /** @@ -127,7 +141,7 @@ public function build_indexable( $post_type ) { if ( $indexable ) { $indexable->object_last_modified = \max( $indexable->object_last_modified, \current_time( 'mysql' ) ); - $indexable->save(); + $this->indexable_helper->save_indexable( $indexable ); } } } diff --git a/src/integrations/watchers/indexable-post-watcher.php b/src/integrations/watchers/indexable-post-watcher.php index c76b6c10124..21564d94c28 100644 --- a/src/integrations/watchers/indexable-post-watcher.php +++ b/src/integrations/watchers/indexable-post-watcher.php @@ -8,6 +8,7 @@ use Yoast\WP\SEO\Builders\Indexable_Link_Builder; use Yoast\WP\SEO\Conditionals\Migrations_Conditional; use Yoast\WP\SEO\Helpers\Author_Archive_Helper; +use Yoast\WP\SEO\Helpers\Indexable_Helper; use Yoast\WP\SEO\Helpers\Post_Helper; use Yoast\WP\SEO\Integrations\Cleanup_Integration; use Yoast\WP\SEO\Integrations\Integration_Interface; @@ -59,6 +60,13 @@ class Indexable_Post_Watcher implements Integration_Interface { */ private $author_archive; + /** + * The indexable helper. + * + * @var Indexable_Helper + */ + private $indexable_helper; + /** * Holds the Post_Helper instance. * @@ -90,6 +98,7 @@ public static function get_conditionals() { * @param Indexable_Hierarchy_Repository $hierarchy_repository The hierarchy repository to use. * @param Indexable_Link_Builder $link_builder The link builder. * @param Author_Archive_Helper $author_archive The author archive helper. + * @param Indexable_Helper $indexable_helper The indexable helper. * @param Post_Helper $post The post helper. * @param Logger $logger The logger. */ @@ -99,6 +108,7 @@ public function __construct( Indexable_Hierarchy_Repository $hierarchy_repository, Indexable_Link_Builder $link_builder, Author_Archive_Helper $author_archive, + Indexable_Helper $indexable_helper, Post_Helper $post, Logger $logger ) { @@ -107,6 +117,7 @@ public function __construct( $this->hierarchy_repository = $hierarchy_repository; $this->link_builder = $link_builder; $this->author_archive = $author_archive; + $this->indexable_helper = $indexable_helper; $this->post = $post; $this->logger = $logger; } @@ -172,8 +183,6 @@ public function updated_indexable( $indexable, $post ) { } $this->update_relations( $post ); - - $indexable->save(); } /** @@ -208,7 +217,7 @@ public function build_indexable( $post_id ) { if ( $post && $indexable && \in_array( $post->post_status, $this->post->get_public_post_statuses(), true ) ) { $this->link_builder->build( $indexable, $post->post_content ); // Save indexable to persist the updated link count. - $indexable->save(); + $this->indexable_helper->save_indexable( $indexable ); $this->updated_indexable( $indexable, $post ); } } catch ( Exception $exception ) { @@ -229,7 +238,7 @@ protected function update_has_public_posts( $indexable ) { $author_indexable = $this->repository->find_by_id_and_type( $indexable->author_id, 'user' ); if ( $author_indexable ) { $author_indexable->has_public_posts = $this->author_archive->author_has_public_posts( $author_indexable->object_id ); - $author_indexable->save(); + $this->indexable_helper->save_indexable( $author_indexable ); $this->reschedule_cleanup_if_author_has_no_posts( $author_indexable ); } @@ -273,7 +282,7 @@ protected function update_relations( $post ) { // Ignore everything that is not an actual indexable. if ( \is_a( $indexable, Indexable::class ) ) { $indexable->object_last_modified = \max( $indexable->object_last_modified, $post->post_modified_gmt ); - $indexable->save(); + $this->indexable_helper->save_indexable( $indexable ); } } } diff --git a/src/integrations/watchers/indexable-term-watcher.php b/src/integrations/watchers/indexable-term-watcher.php index 0dec8710054..338d5cdffb4 100644 --- a/src/integrations/watchers/indexable-term-watcher.php +++ b/src/integrations/watchers/indexable-term-watcher.php @@ -5,6 +5,7 @@ use Yoast\WP\SEO\Builders\Indexable_Builder; use Yoast\WP\SEO\Builders\Indexable_Link_Builder; use Yoast\WP\SEO\Conditionals\Migrations_Conditional; +use Yoast\WP\SEO\Helpers\Indexable_Helper; use Yoast\WP\SEO\Helpers\Site_Helper; use Yoast\WP\SEO\Integrations\Integration_Interface; use Yoast\WP\SEO\Repositories\Indexable_Repository; @@ -35,6 +36,13 @@ class Indexable_Term_Watcher implements Integration_Interface { */ protected $link_builder; + /** + * The indexable helper. + * + * @var Indexable_Helper + */ + private $indexable_helper; + /** * Represents the site helper. * @@ -54,21 +62,24 @@ public static function get_conditionals() { /** * Indexable_Term_Watcher constructor. * - * @param Indexable_Repository $repository The repository to use. - * @param Indexable_Builder $builder The post builder to use. - * @param Indexable_Link_Builder $link_builder The lint builder to use. - * @param Site_Helper $site The site helper. + * @param Indexable_Repository $repository The repository to use. + * @param Indexable_Builder $builder The post builder to use. + * @param Indexable_Link_Builder $link_builder The lint builder to use. + * @param Indexable_Helper $indexable_helper The indexable helper. + * @param Site_Helper $site The site helper. */ public function __construct( Indexable_Repository $repository, Indexable_Builder $builder, Indexable_Link_Builder $link_builder, + Indexable_Helper $indexable_helper, Site_Helper $site ) { - $this->repository = $repository; - $this->builder = $builder; - $this->link_builder = $link_builder; - $this->site = $site; + $this->repository = $repository; + $this->builder = $builder; + $this->link_builder = $link_builder; + $this->indexable_helper = $indexable_helper; + $this->site = $site; } /** @@ -136,6 +147,6 @@ public function build_indexable( $term_id ) { $this->link_builder->build( $indexable, $term->description ); $indexable->object_last_modified = \max( $indexable->object_last_modified, \current_time( 'mysql' ) ); - $indexable->save(); + $this->indexable_helper->save_indexable( $indexable ); } } diff --git a/src/integrations/watchers/primary-term-watcher.php b/src/integrations/watchers/primary-term-watcher.php index 140fc0cbde3..5eeadb18cf3 100644 --- a/src/integrations/watchers/primary-term-watcher.php +++ b/src/integrations/watchers/primary-term-watcher.php @@ -149,13 +149,13 @@ protected function save_primary_term( $post_id, $taxonomy ) { */ public function delete_primary_terms( $post_id ) { foreach ( $this->primary_term->get_primary_term_taxonomies( $post_id ) as $taxonomy ) { - $primary_term = $this->repository->find_by_post_id_and_taxonomy( $post_id, $taxonomy->name, false ); + $primary_term_indexable = $this->repository->find_by_post_id_and_taxonomy( $post_id, $taxonomy->name, false ); - if ( ! $primary_term ) { + if ( ! $primary_term_indexable ) { continue; } - $primary_term->delete(); + $primary_term_indexable->delete(); } } } diff --git a/src/repositories/indexable-hierarchy-repository.php b/src/repositories/indexable-hierarchy-repository.php index 4ff8beb07f4..1d83ed759b7 100644 --- a/src/repositories/indexable-hierarchy-repository.php +++ b/src/repositories/indexable-hierarchy-repository.php @@ -5,6 +5,7 @@ use Yoast\WP\Lib\Model; use Yoast\WP\Lib\ORM; use Yoast\WP\SEO\Builders\Indexable_Hierarchy_Builder; +use Yoast\WP\SEO\Helpers\Indexable_Helper; use Yoast\WP\SEO\Models\Indexable; /** @@ -19,6 +20,13 @@ class Indexable_Hierarchy_Repository { */ protected $builder; + /** + * Represents the indexable helper. + * + * @var Indexable_Helper + */ + protected $indexable_helper; + /** * Sets the hierarchy builder. * @@ -32,6 +40,19 @@ public function set_builder( Indexable_Hierarchy_Builder $builder ) { $this->builder = $builder; } + /** + * Sets the indexable helper. + * + * @required + * + * @param Indexable_Helper $indexable_helper The indexable helper. + * + * @return void + */ + public function set_helper( Indexable_Helper $indexable_helper ) { + $this->indexable_helper = $indexable_helper; + } + /** * Removes all ancestors for an indexable. * @@ -53,6 +74,10 @@ public function clear_ancestors( $indexable_id ) { * @return bool Whether or not the ancestor was added successfully. */ public function add_ancestor( $indexable_id, $ancestor_id, $depth ) { + if ( ! $this->indexable_helper->should_index_indexables() ) { + return false; + } + $hierarchy = $this->query()->create( [ 'indexable_id' => $indexable_id, diff --git a/src/repositories/primary-term-repository.php b/src/repositories/primary-term-repository.php index 244e34d6aea..8fc0c93e93c 100644 --- a/src/repositories/primary-term-repository.php +++ b/src/repositories/primary-term-repository.php @@ -33,17 +33,17 @@ public function find_by_post_id_and_taxonomy( $post_id, $taxonomy, $auto_create /** * Instance of the primary term. * - * @var Primary_Term $primary_term + * @var Primary_Term $primary_term_indexable */ - $primary_term = $this->query() + $primary_term_indexable = $this->query() ->where( 'post_id', $post_id ) ->where( 'taxonomy', $taxonomy ) ->find_one(); - if ( $auto_create && ! $primary_term ) { - $primary_term = $this->query()->create(); + if ( $auto_create && ! $primary_term_indexable ) { + $primary_term_indexable = $this->query()->create(); } - return $primary_term; + return $primary_term_indexable; } } diff --git a/tests/Unit/Actions/Indexing/Abstract_Link_Indexing_Action_Test.php b/tests/Unit/Actions/Indexing/Abstract_Link_Indexing_Action_Test.php index 87a3cb1b6a1..dde441f14a9 100644 --- a/tests/Unit/Actions/Indexing/Abstract_Link_Indexing_Action_Test.php +++ b/tests/Unit/Actions/Indexing/Abstract_Link_Indexing_Action_Test.php @@ -7,6 +7,7 @@ use wpdb; use Yoast\WP\SEO\Actions\Indexing\Abstract_Link_Indexing_Action; use Yoast\WP\SEO\Builders\Indexable_Link_Builder; +use Yoast\WP\SEO\Helpers\Indexable_Helper; use Yoast\WP\SEO\Helpers\Taxonomy_Helper; use Yoast\WP\SEO\Repositories\Indexable_Repository; use Yoast\WP\SEO\Tests\Unit\TestCase; @@ -22,23 +23,30 @@ final class Abstract_Link_Indexing_Action_Test extends TestCase { /** - * The link builder. + * Represents the link builder. * - * @var Indexable_Link_Builder + * @var Mockery\MockInterface|Indexable_Link_Builder */ protected $link_builder; /** - * The post type helper. + * Represents the taxonomy helper. * - * @var Taxonomy_Helper + * @var Mockery\MockInterface|Taxonomy_Helper */ protected $taxonomy_helper; /** - * The indexable repository. + * Represents the indexable helper. * - * @var Indexable_Repository + * @var Mockery\MockInterface|Indexable_Helper + */ + protected $indexable_helper; + + /** + * Represents the indexable repository. + * + * @var Mockery\MockInterface|Indexable_Repository */ protected $repository; @@ -69,6 +77,7 @@ protected function set_up() { $wpdb = (object) [ 'prefix' => 'wp_' ]; $this->link_builder = Mockery::mock( Indexable_Link_Builder::class ); + $this->indexable_helper = Mockery::mock( Indexable_Helper::class ); $this->repository = Mockery::mock( Indexable_Repository::class ); $this->wpdb = Mockery::mock( wpdb::class ); $this->wpdb->term_taxonomy = 'wp_term_taxonomy'; @@ -77,6 +86,7 @@ protected function set_up() { Abstract_Link_Indexing_Action::class, [ $this->link_builder, + $this->indexable_helper, $this->repository, $this->wpdb, ] @@ -95,6 +105,10 @@ public function test_construct() { Indexable_Link_Builder::class, $this->getPropertyValue( $this->instance, 'link_builder' ) ); + $this->assertInstanceOf( + Indexable_Helper::class, + $this->getPropertyValue( $this->instance, 'indexable_helper' ) + ); $this->assertInstanceOf( Indexable_Repository::class, $this->getPropertyValue( $this->instance, 'repository' ) diff --git a/tests/Unit/Actions/Indexing/Post_Link_Indexing_Action_Test.php b/tests/Unit/Actions/Indexing/Post_Link_Indexing_Action_Test.php index f4d79fdd4d2..b11d58b0e94 100644 --- a/tests/Unit/Actions/Indexing/Post_Link_Indexing_Action_Test.php +++ b/tests/Unit/Actions/Indexing/Post_Link_Indexing_Action_Test.php @@ -8,6 +8,7 @@ use wpdb; use Yoast\WP\SEO\Actions\Indexing\Post_Link_Indexing_Action; use Yoast\WP\SEO\Builders\Indexable_Link_Builder; +use Yoast\WP\SEO\Helpers\Indexable_Helper; use Yoast\WP\SEO\Helpers\Post_Type_Helper; use Yoast\WP\SEO\Repositories\Indexable_Repository; use Yoast\WP\SEO\Tests\Unit\Doubles\Models\Indexable_Mock; @@ -24,23 +25,30 @@ final class Post_Link_Indexing_Action_Test extends TestCase { /** - * The link builder. + * Represents the link builder. * - * @var Indexable_Link_Builder + * @var Mockery\MockInterface|Indexable_Link_Builder */ protected $link_builder; /** - * The post type helper. + * Represents the post type helper. * - * @var Post_Type_Helper + * @var Mockery\MockInterface|Post_Type_Helper */ protected $post_type_helper; /** - * The indexable repository. + * Represents the indexable helper. * - * @var Indexable_Repository + * @var Mockery\MockInterface|Indexable_Helper + */ + protected $indexable_helper; + + /** + * Represents the indexable repository. + * + * @var Mockery\MockInterface|Indexable_Repository */ protected $repository; @@ -72,12 +80,14 @@ protected function set_up() { $this->link_builder = Mockery::mock( Indexable_Link_Builder::class ); $this->post_type_helper = Mockery::mock( Post_Type_Helper::class ); + $this->indexable_helper = Mockery::mock( Indexable_Helper::class ); $this->repository = Mockery::mock( Indexable_Repository::class ); $this->wpdb = Mockery::mock( wpdb::class ); $this->wpdb->posts = 'wp_posts'; $this->instance = new Post_Link_Indexing_Action( $this->link_builder, + $this->indexable_helper, $this->repository, $this->wpdb ); @@ -274,7 +284,11 @@ public function test_index() { foreach ( $posts as $post ) { $indexable = Mockery::mock( Indexable_Mock::class ); $indexable->link_count = 10; - $indexable->expects( 'save' )->once(); + + $this->indexable_helper + ->expects( 'save_indexable' ) + ->with( $indexable ) + ->once(); $this->link_builder->expects( 'build' )->with( $indexable, $post->post_content ); @@ -348,7 +362,11 @@ public function test_index_without_link_count() { $indexable = Mockery::mock( Indexable_Mock::class ); $indexable->link_count = null; - $indexable->expects( 'save' )->times( 3 ); + + $this->indexable_helper + ->expects( 'save_indexable' ) + ->with( $indexable ) + ->times( 3 ); $this->repository->expects( 'find_by_id_and_type' )->once()->with( 1, 'post' )->andReturn( $indexable ); $this->repository->expects( 'find_by_id_and_type' )->once()->with( 3, 'post' )->andReturn( $indexable ); diff --git a/tests/Unit/Actions/Indexing/Term_Link_Indexing_Action_Test.php b/tests/Unit/Actions/Indexing/Term_Link_Indexing_Action_Test.php index 8dbbc361a14..be632fb931d 100644 --- a/tests/Unit/Actions/Indexing/Term_Link_Indexing_Action_Test.php +++ b/tests/Unit/Actions/Indexing/Term_Link_Indexing_Action_Test.php @@ -8,6 +8,7 @@ use wpdb; use Yoast\WP\SEO\Actions\Indexing\Term_Link_Indexing_Action; use Yoast\WP\SEO\Builders\Indexable_Link_Builder; +use Yoast\WP\SEO\Helpers\Indexable_Helper; use Yoast\WP\SEO\Helpers\Taxonomy_Helper; use Yoast\WP\SEO\Repositories\Indexable_Repository; use Yoast\WP\SEO\Tests\Unit\Doubles\Models\Indexable_Mock; @@ -24,23 +25,30 @@ final class Term_Link_Indexing_Action_Test extends TestCase { /** - * The link builder. + * Represents the link builder. * - * @var Indexable_Link_Builder + * @var Mockery\MockInterface|Indexable_Link_Builder */ protected $link_builder; /** - * The post type helper. + * Represents the taxonomy helper. * - * @var Taxonomy_Helper + * @var Mockery\MockInterface|Taxonomy_Helper */ protected $taxonomy_helper; /** - * The indexable repository. + * Represents the indexable helper. * - * @var Indexable_Repository + * @var Mockery\MockInterface|Indexable_Helper + */ + protected $indexable_helper; + + /** + * Represents the indexable repository. + * + * @var Mockery\MockInterface|Indexable_Repository */ protected $repository; @@ -72,12 +80,14 @@ protected function set_up() { $this->link_builder = Mockery::mock( Indexable_Link_Builder::class ); $this->taxonomy_helper = Mockery::mock( Taxonomy_Helper::class ); + $this->indexable_helper = Mockery::mock( Indexable_Helper::class ); $this->repository = Mockery::mock( Indexable_Repository::class ); $this->wpdb = Mockery::mock( wpdb::class ); $this->wpdb->term_taxonomy = 'wp_term_taxonomy'; $this->instance = new Term_Link_Indexing_Action( $this->link_builder, + $this->indexable_helper, $this->repository, $this->wpdb ); @@ -320,7 +330,11 @@ public function test_index() { foreach ( $terms as $term ) { $indexable = Mockery::mock( Indexable_Mock::class ); $indexable->link_count = 10; - $indexable->expects( 'save' )->once(); + + $this->indexable_helper + ->expects( 'save_indexable' ) + ->with( $indexable ) + ->once(); $this->link_builder->expects( 'build' )->with( $indexable, $term->description ); @@ -387,7 +401,11 @@ public function test_index_without_link_count() { $indexable = Mockery::mock( Indexable_Mock::class ); $indexable->link_count = null; - $indexable->expects( 'save' )->times( 3 ); + + $this->indexable_helper + ->expects( 'save_indexable' ) + ->with( $indexable ) + ->times( 3 ); $this->repository->expects( 'find_by_id_and_type' )->once()->with( 1, 'term' )->andReturn( $indexable ); $this->repository->expects( 'find_by_id_and_type' )->once()->with( 3, 'term' )->andReturn( $indexable ); diff --git a/tests/Unit/Builders/Indexable_Builder/Abstract_Indexable_Builder_TestCase.php b/tests/Unit/Builders/Indexable_Builder/Abstract_Indexable_Builder_TestCase.php index 7541a510121..c96849448d8 100644 --- a/tests/Unit/Builders/Indexable_Builder/Abstract_Indexable_Builder_TestCase.php +++ b/tests/Unit/Builders/Indexable_Builder/Abstract_Indexable_Builder_TestCase.php @@ -197,6 +197,20 @@ public function expect_save_indexable_skip() { ->andReturnFalse(); } + /** + * Expectation in save_indexable. + * + * @param Indexable_Mock $indexable The indexable to expect. + * + * @return void + */ + public function expect_save_indexable( $indexable ) { + $this->indexable_helper + ->expects( 'save_indexable' ) + ->once() + ->andReturn( $indexable ); + } + /** * Expectations for ensure_indexable method. * diff --git a/tests/Unit/Builders/Indexable_Builder/Build_For_Date_Archive_Test.php b/tests/Unit/Builders/Indexable_Builder/Build_For_Date_Archive_Test.php index 9028fcc254f..de8ea612464 100644 --- a/tests/Unit/Builders/Indexable_Builder/Build_For_Date_Archive_Test.php +++ b/tests/Unit/Builders/Indexable_Builder/Build_For_Date_Archive_Test.php @@ -55,8 +55,7 @@ public function test_build_for_date_archive_with_indexable() { ->with( $this->indexable ) ->andReturn( $this->indexable ); - // Saving is outside the scope of this test. - $this->expect_save_indexable_skip(); + $this->expect_save_indexable( $this->indexable ); $this->assertSame( $this->indexable, $this->instance->build_for_date_archive( $this->indexable ) ); } @@ -77,7 +76,6 @@ private function expect_build( $defaults ) { ->with( $this->indexable ) ->andReturn( $this->indexable ); - // Saving is outside the scope of this test. - $this->expect_save_indexable_skip(); + $this->expect_save_indexable( $this->indexable ); } } diff --git a/tests/Unit/Builders/Indexable_Builder/Build_For_Home_Page_Test.php b/tests/Unit/Builders/Indexable_Builder/Build_For_Home_Page_Test.php index 65732cb73be..9d69050f815 100644 --- a/tests/Unit/Builders/Indexable_Builder/Build_For_Home_Page_Test.php +++ b/tests/Unit/Builders/Indexable_Builder/Build_For_Home_Page_Test.php @@ -56,7 +56,6 @@ public function expect_build( $defaults ) { ->with( $this->indexable ) ->andReturn( $this->indexable ); - // Saving is outside the scope of this test. - $this->expect_save_indexable_skip(); + $this->expect_save_indexable( $this->indexable ); } } diff --git a/tests/Unit/Builders/Indexable_Builder/Build_For_Id_And_Type_Test.php b/tests/Unit/Builders/Indexable_Builder/Build_For_Id_And_Type_Test.php index 38759650731..ad067d37ed4 100644 --- a/tests/Unit/Builders/Indexable_Builder/Build_For_Id_And_Type_Test.php +++ b/tests/Unit/Builders/Indexable_Builder/Build_For_Id_And_Type_Test.php @@ -63,7 +63,6 @@ public function expect_build( $defaults ) { $this->expect_maybe_build_author_indexable(); - // Saving is outside the scope of this test. - $this->expect_save_indexable_skip(); + $this->expect_save_indexable( $this->indexable ); } } diff --git a/tests/Unit/Builders/Indexable_Builder/Build_For_Post_Type_Archive_Test.php b/tests/Unit/Builders/Indexable_Builder/Build_For_Post_Type_Archive_Test.php index ee3bea10515..ba163aa3101 100644 --- a/tests/Unit/Builders/Indexable_Builder/Build_For_Post_Type_Archive_Test.php +++ b/tests/Unit/Builders/Indexable_Builder/Build_For_Post_Type_Archive_Test.php @@ -60,7 +60,6 @@ public function expect_build( $defaults ) { ->with( 'post', $this->indexable ) ->andReturn( $this->indexable ); - // Saving is outside the scope of this test. - $this->expect_save_indexable_skip(); + $this->expect_save_indexable( $this->indexable ); } } diff --git a/tests/Unit/Builders/Indexable_Builder/Build_For_System_Page_Test.php b/tests/Unit/Builders/Indexable_Builder/Build_For_System_Page_Test.php index ff3f2a3c4ca..45c8c77e68f 100644 --- a/tests/Unit/Builders/Indexable_Builder/Build_For_System_Page_Test.php +++ b/tests/Unit/Builders/Indexable_Builder/Build_For_System_Page_Test.php @@ -64,7 +64,6 @@ public function expect_build( $defaults ) { ->with( '404', $this->indexable ) ->andReturn( $this->indexable ); - // Saving is outside the scope of this test. - $this->expect_save_indexable_skip(); + $this->expect_save_indexable( $this->indexable ); } } diff --git a/tests/Unit/Builders/Indexable_Builder/Build_Test.php b/tests/Unit/Builders/Indexable_Builder/Build_Test.php index 7acc294c081..7e470b6fba0 100644 --- a/tests/Unit/Builders/Indexable_Builder/Build_Test.php +++ b/tests/Unit/Builders/Indexable_Builder/Build_Test.php @@ -47,7 +47,7 @@ public function test_build() { $this->expect_maybe_build_author_indexable(); - $this->expect_save_indexable_skip(); + $this->expect_save_indexable( $this->indexable ); $this->assertSame( $this->indexable, $this->instance->build( $this->indexable ) ); } @@ -78,8 +78,7 @@ public function test_build_with_post_attachment() { $this->expect_maybe_build_author_indexable(); - // Saving is outside the scope of this test. - $this->expect_save_indexable_skip(); + $this->expect_save_indexable( $this->indexable ); $result = $this->instance->build( $this->indexable ); @@ -112,7 +111,7 @@ public function test_build_with_term_given() { ->once() ->with( $this->indexable ); - $this->expect_save_indexable_skip(); + $this->expect_save_indexable( $this->indexable ); $this->assertSame( $this->indexable, $this->instance->build( $this->indexable ) ); } @@ -171,7 +170,7 @@ static function ( $indexable ) { } ); - $this->expect_save_indexable_skip(); + $this->expect_save_indexable( $this->indexable ); $result = $this->instance->build( $this->indexable ); @@ -193,7 +192,7 @@ public function test_build_for_id_and_type_with_unknown_type_given() { $this->expect_deep_copy_indexable( $this->indexable ); - $this->expect_save_indexable_skip(); + $this->expect_save_indexable( $this->indexable ); $this->assertSame( $this->indexable, $this->instance->build( $this->indexable ) ); } @@ -259,12 +258,13 @@ static function ( $indexable ) { } ); - $this->expect_save_indexable_skip(); - - $result = $this->instance->build( false, $defaults ); - $expected_indexable = clone $empty_indexable; $expected_indexable->post_status = 'unindexed'; + $expected_indexable->version = 2; + + $this->expect_save_indexable( $expected_indexable ); + + $result = $this->instance->build( false, $defaults ); $this->assertEquals( $empty_indexable, $result ); } @@ -305,12 +305,12 @@ static function ( $indexable ) { } ); - $this->expect_save_indexable_skip(); - - $result = $this->instance->build( $this->indexable ); - $expected_indexable = clone $this->indexable; $expected_indexable->post_status = 'unindexed'; + + $this->expect_save_indexable( $expected_indexable ); + + $result = $this->instance->build( $this->indexable ); $this->assertEquals( $expected_indexable, $result ); } diff --git a/tests/Unit/Builders/Indexable_Builder/Maybe_Build_Author_Indexable_Test.php b/tests/Unit/Builders/Indexable_Builder/Maybe_Build_Author_Indexable_Test.php index ff33e9fb166..f43155a6e81 100644 --- a/tests/Unit/Builders/Indexable_Builder/Maybe_Build_Author_Indexable_Test.php +++ b/tests/Unit/Builders/Indexable_Builder/Maybe_Build_Author_Indexable_Test.php @@ -79,7 +79,7 @@ public function test_build_new_author_indexable() { ->with( $author_indexable->object_id, $author_indexable ) ->andReturn( $author_indexable ); - $this->expect_save_indexable_skip(); + $this->expect_save_indexable( $author_indexable ); $this->instance->exposed_maybe_build_author_indexable( $author_id ); } @@ -122,7 +122,7 @@ public function test_update_author_indexable() { ->with( $author_indexable->object_id, $author_indexable ) ->andReturn( $author_indexable ); - $this->expect_save_indexable_skip(); + $this->expect_save_indexable( $author_indexable ); $this->instance->exposed_maybe_build_author_indexable( $author_id ); } diff --git a/tests/Unit/Builders/Indexable_Builder/Save_Indexable_Test.php b/tests/Unit/Builders/Indexable_Builder/Save_Indexable_Test.php index 4cf7218ea91..a7ef8283afb 100644 --- a/tests/Unit/Builders/Indexable_Builder/Save_Indexable_Test.php +++ b/tests/Unit/Builders/Indexable_Builder/Save_Indexable_Test.php @@ -2,7 +2,6 @@ namespace Yoast\WP\SEO\Tests\Unit\Builders\Indexable_Builder; -use Brain\Monkey; use Mockery; use Yoast\WP\SEO\Tests\Unit\Doubles\Builders\Indexable_Builder_Double; @@ -50,47 +49,11 @@ protected function set_up() { public static function save_indexable_provider() { $before = Mockery::mock( Indexable::class ); return [ - 'Should index and save' => [ - 'indexable_before' => null, - 'should_index_indexables' => true, - 'wpseo_should_save_indexable' => true, - 'save_times' => 1, - 'action_times' => 0, + 'Index with no indexable before' => [ + 'indexable_before' => null, ], - 'Should not index and not save' => [ - 'indexable_before' => null, - 'should_index_indexables' => false, - 'wpseo_should_save_indexable' => false, - 'save_times' => 0, - 'action_times' => 0, - ], - 'Should not index but wpseo_should_save_indexable filter is true, should save' => [ - 'indexable_before' => null, - 'should_index_indexables' => false, - 'wpseo_should_save_indexable' => true, - 'save_times' => 1, - 'action_times' => 0, - ], - 'Should index but wpseo_should_save_indexable filter is false, should not save' => [ - 'indexable_before' => null, - 'should_index_indexables' => true, - 'wpseo_should_save_indexable' => false, - 'save_times' => 0, - 'action_times' => 0, - ], - 'Updating existing indexable' => [ - 'indexable_before' => $before, - 'should_index_indexables' => true, - 'wpseo_should_save_indexable' => true, - 'save_times' => 1, - 'action_times' => 1, - ], - 'Updating existing indexable when should_index_indexables is false' => [ - 'indexable_before' => $before, - 'should_index_indexables' => false, - 'wpseo_should_save_indexable' => true, - 'save_times' => 1, - 'action_times' => 1, + 'Index with indexable before' => [ + 'indexable_before' => $before, ], ]; } @@ -102,33 +65,16 @@ public static function save_indexable_provider() { * * @dataProvider save_indexable_provider * - * @param Indexable_Mock $indexable_before The indexable to expect. - * @param bool $should_index The return value of should_index_indexables method. - * @param bool $wpseo_should_save The return value for wpseo_should_save_indexable. - * @param int $save_times The times save method should be executed. - * @param int $action_times The times wpseo_save_indexable action should be executed. + * @param Indexable_Mock $indexable_before The indexable to expect. * @return void */ - public function test_save_indexable( $indexable_before, $should_index, $wpseo_should_save, $save_times, $action_times ) { + public function test_save_indexable( $indexable_before ) { $this->indexable_helper - ->expects( 'should_index_indexables' ) + ->expects( 'save_indexable' ) ->once() - ->withNoArgs() - ->andReturn( $should_index ); - - Monkey\Filters\expectApplied( 'wpseo_should_save_indexable' ) - ->once() - ->with( $should_index, $this->indexable ) - ->andReturn( $wpseo_should_save ); - - $this->indexable - ->expects( 'save' ) - ->times( $save_times ); - - Monkey\Actions\expectDone( 'wpseo_save_indexable' ) - ->times( $action_times ) - ->with( $this->indexable, $indexable_before ); + ->with( $this->indexable, $indexable_before ) + ->andReturn( $this->indexable ); $result = $this->instance->exposed_save_indexable( $this->indexable, $indexable_before ); diff --git a/tests/Unit/Builders/Indexable_Link_Builder/Abstract_Indexable_Link_Builder_TestCase.php b/tests/Unit/Builders/Indexable_Link_Builder/Abstract_Indexable_Link_Builder_TestCase.php index bfcb349831e..a1258c61dbf 100644 --- a/tests/Unit/Builders/Indexable_Link_Builder/Abstract_Indexable_Link_Builder_TestCase.php +++ b/tests/Unit/Builders/Indexable_Link_Builder/Abstract_Indexable_Link_Builder_TestCase.php @@ -7,6 +7,7 @@ use Yoast\WP\Lib\ORM; use Yoast\WP\SEO\Builders\Indexable_Link_Builder; use Yoast\WP\SEO\Helpers\Image_Helper; +use Yoast\WP\SEO\Helpers\Indexable_Helper; use Yoast\WP\SEO\Helpers\Options_Helper; use Yoast\WP\SEO\Helpers\Post_Helper; use Yoast\WP\SEO\Helpers\Url_Helper; @@ -41,6 +42,13 @@ abstract class Abstract_Indexable_Link_Builder_TestCase extends TestCase { */ protected $image_helper; + /** + * The indexable helper. + * + * @var Mockery\MockInterface|Indexable_Helper + */ + protected $indexable_helper; + /** * The post helper. * @@ -90,12 +98,14 @@ protected function set_up() { $this->image_helper = Mockery::mock( Image_Helper::class ); $this->post_helper = Mockery::mock( Post_Helper::class ); $this->options_helper = Mockery::mock( Options_Helper::class ); + $this->indexable_helper = Mockery::mock( Indexable_Helper::class ); $this->instance = new Indexable_Link_Builder( $this->seo_links_repository, $this->url_helper, $this->post_helper, - $this->options_helper + $this->options_helper, + $this->indexable_helper ); $this->instance->set_dependencies( $this->indexable_repository, $this->image_helper ); diff --git a/tests/Unit/Builders/Indexable_Link_Builder/Build_Test.php b/tests/Unit/Builders/Indexable_Link_Builder/Build_Test.php index 456239f1cad..23de91e49f6 100644 --- a/tests/Unit/Builders/Indexable_Link_Builder/Build_Test.php +++ b/tests/Unit/Builders/Indexable_Link_Builder/Build_Test.php @@ -114,6 +114,7 @@ public function test_build( $content, $link_type, $is_image, $ignore_content_sca $indexable->object_type = 'post'; $indexable->permalink = 'https://site.com/page'; + $this->indexable_helper->expects( 'should_index_indexable' )->once()->andReturn( true ); $this->post_helper->expects( 'get_post' )->once()->with( 2 )->andReturn( 'post' ); Functions\expect( 'setup_postdata' )->once()->with( 'post' ); Functions\expect( 'apply_filters' )->once()->with( 'the_content', $content )->andReturn( $content ); @@ -258,6 +259,7 @@ public function test_build_target_indexable_does_not_exist() { $target_indexable->language = 'nl'; $target_indexable->region = 'NL'; + $this->indexable_helper->expects( 'should_index_indexable' )->once()->andReturn( true ); $this->post_helper->expects( 'get_post' )->once()->with( 2 )->andReturn( 'post' ); Functions\expect( 'setup_postdata' )->once()->with( 'post' ); Filters\expectApplied( 'the_content' )->with( $content )->andReturnFirstArg(); @@ -372,6 +374,8 @@ public function test_build_no_links() { $indexable->object_type = 'page'; $indexable->permalink = 'https://site.com/page'; + $this->indexable_helper->expects( 'should_index_indexable' )->once()->andReturn( true ); + $this->seo_links_repository ->expects( 'find_all_by_indexable_id' ) ->with( $indexable->id ) @@ -400,6 +404,8 @@ public function test_build_ignore_content_scan( $input_content, $output_result ) $indexable->object_id = 2; $indexable->object_type = 'page'; $indexable->permalink = 'https://site.com/page'; + + $this->indexable_helper->expects( 'should_index_indexable' )->once()->andReturn( true ); Functions\expect( 'apply_filters' )->andReturn( true ); $this->seo_links_repository diff --git a/tests/Unit/Builders/Indexable_Link_Builder/Create_Internal_Link_Test.php b/tests/Unit/Builders/Indexable_Link_Builder/Create_Internal_Link_Test.php index 27c7b8c163a..55dc508e7ba 100644 --- a/tests/Unit/Builders/Indexable_Link_Builder/Create_Internal_Link_Test.php +++ b/tests/Unit/Builders/Indexable_Link_Builder/Create_Internal_Link_Test.php @@ -43,6 +43,8 @@ public function test_build_create_internal_link() { $model = new SEO_Links_Mock(); $model->type = SEO_Links::TYPE_INTERNAL_IMAGE; + $this->indexable_helper->expects( 'should_index_indexable' )->once()->andReturn( true ); + Functions\stubs( [ // Executed in build->create_links->create_internal_link. @@ -119,6 +121,8 @@ public function test_build_create_internal_link_disable_attachment_true_file_doe $model->type = SEO_Links::TYPE_INTERNAL_IMAGE; $model->target_post_id = 2; + $this->indexable_helper->expects( 'should_index_indexable' )->once()->andReturn( true ); + // Executed in build->create_links->create_internal_link. Functions\stubs( [ @@ -198,6 +202,8 @@ public function test_build_create_internal_link_disable_attachment_true_file_exi $model->height = null; $model->width = null; + $this->indexable_helper->expects( 'should_index_indexable' )->once()->andReturn( true ); + // Executed in build->create_links->create_internal_link. Functions\stubs( [ @@ -261,6 +267,8 @@ public function test_build_create_internal_link_disable_attachment_true_file_not $model->type = SEO_Links::TYPE_INTERNAL_IMAGE; $model->target_post_id = 3; + $this->indexable_helper->expects( 'should_index_indexable' )->once()->andReturn( true ); + Functions\stubs( [ // Executed in build->create_links->create_internal_link. @@ -339,6 +347,8 @@ public function test_build_create_internal_link_disable_attachment_true_get_atta $model->type = SEO_Links::TYPE_INTERNAL_IMAGE; $model->target_post_id = 2; + $this->indexable_helper->expects( 'should_index_indexable' )->once()->andReturn( true ); + Functions\stubs( [ // Executed in build->create_links. diff --git a/tests/Unit/Builders/Indexable_Link_Builder/Get_Permalink_Test.php b/tests/Unit/Builders/Indexable_Link_Builder/Get_Permalink_Test.php index 4d62f218062..7495510984d 100644 --- a/tests/Unit/Builders/Indexable_Link_Builder/Get_Permalink_Test.php +++ b/tests/Unit/Builders/Indexable_Link_Builder/Get_Permalink_Test.php @@ -25,7 +25,8 @@ protected function set_up() { $this->seo_links_repository, $this->url_helper, $this->post_helper, - $this->options_helper + $this->options_helper, + $this->indexable_helper ); $this->instance->set_dependencies( $this->indexable_repository, $this->image_helper ); diff --git a/tests/Unit/Builders/Indexable_Link_Builder/Update_Incoming_Links_For_Related_Test.php b/tests/Unit/Builders/Indexable_Link_Builder/Update_Incoming_Links_For_Related_Test.php index 952b7aff76c..92a04c7b4c3 100644 --- a/tests/Unit/Builders/Indexable_Link_Builder/Update_Incoming_Links_For_Related_Test.php +++ b/tests/Unit/Builders/Indexable_Link_Builder/Update_Incoming_Links_For_Related_Test.php @@ -27,7 +27,8 @@ protected function set_up() { $this->seo_links_repository, $this->url_helper, $this->post_helper, - $this->options_helper + $this->options_helper, + $this->indexable_helper ); $this->instance->set_dependencies( $this->indexable_repository, $this->image_helper ); diff --git a/tests/Unit/Builders/Primary_Term_Builder_Test.php b/tests/Unit/Builders/Primary_Term_Builder_Test.php index 89bf049dc77..fb11f359791 100644 --- a/tests/Unit/Builders/Primary_Term_Builder_Test.php +++ b/tests/Unit/Builders/Primary_Term_Builder_Test.php @@ -5,6 +5,7 @@ use Brain\Monkey\Functions; use Mockery; use Yoast\WP\SEO\Builders\Primary_Term_Builder; +use Yoast\WP\SEO\Helpers\Indexable_Helper; use Yoast\WP\SEO\Helpers\Meta_Helper; use Yoast\WP\SEO\Helpers\Primary_Term_Helper; use Yoast\WP\SEO\Models\Primary_Term; @@ -37,6 +38,13 @@ final class Primary_Term_Builder_Test extends TestCase { */ private $repository; + /** + * Holds the indexable helper. + * + * @var Mockery\MockInterface|Indexable_Helper + */ + private $indexable_helper; + /** * Holds the primary term helper. * @@ -59,12 +67,18 @@ final class Primary_Term_Builder_Test extends TestCase { protected function set_up() { parent::set_up(); - $this->repository = Mockery::mock( Primary_Term_Repository::class ); - $this->primary_term = Mockery::mock( Primary_Term_Helper::class ); - $this->meta = Mockery::mock( Meta_Helper::class ); - $this->instance = Mockery::mock( + $this->repository = Mockery::mock( Primary_Term_Repository::class ); + $this->indexable_helper = Mockery::mock( Indexable_Helper::class ); + $this->primary_term = Mockery::mock( Primary_Term_Helper::class ); + $this->meta = Mockery::mock( Meta_Helper::class ); + $this->instance = Mockery::mock( Primary_Term_Builder_Double::class, - [ $this->repository, $this->primary_term, $this->meta ] + [ + $this->repository, + $this->indexable_helper, + $this->primary_term, + $this->meta, + ] ) ->shouldAllowMockingProtectedMethods() ->makePartial(); @@ -80,6 +94,7 @@ protected function set_up() { public function test_successfully_creates_primary_term_builder() { $primary_term_builder = new Primary_Term_Builder( $this->repository, + $this->indexable_helper, $this->primary_term, $this->meta ); @@ -89,6 +104,11 @@ public function test_successfully_creates_primary_term_builder() { $this->getPropertyValue( $primary_term_builder, 'repository' ) ); + $this->assertInstanceOf( + Indexable_Helper::class, + $this->getPropertyValue( $primary_term_builder, 'indexable_helper' ) + ); + $this->assertInstanceOf( Primary_Term_Helper::class, $this->getPropertyValue( $primary_term_builder, 'primary_term' ) @@ -168,7 +188,6 @@ public function test_build_empty_taxonomies() { */ public function test_save_primary_term() { $primary_term = Mockery::mock( Primary_Term_Mock::class ); - $primary_term->expects( 'save' )->once(); Functions\expect( 'get_current_blog_id' )->once()->andReturn( 1 ); @@ -183,6 +202,10 @@ public function test_save_primary_term() { ->with( 'primary_category', 1 ) ->andReturn( 1337 ); + $this->indexable_helper + ->expects( 'save_indexable' ) + ->once(); + $this->instance->save_primary_term( 1, 'category' ); $this->assertEquals( 1337, $primary_term->term_id ); @@ -200,7 +223,6 @@ public function test_save_primary_term() { */ public function test_save_primary_term_of_custom_taxonomy() { $primary_term = Mockery::mock( Primary_Term_Mock::class ); - $primary_term->expects( 'save' )->once(); Functions\expect( 'get_current_blog_id' )->once()->andReturn( 1 ); @@ -215,6 +237,10 @@ public function test_save_primary_term_of_custom_taxonomy() { ->with( 'primary_custom', 1 ) ->andReturn( 1337 ); + $this->indexable_helper + ->expects( 'save_indexable' ) + ->once(); + $this->instance->save_primary_term( 1, 'custom' ); $this->assertEquals( 1337, $primary_term->term_id ); diff --git a/tests/Unit/Doubles/Builders/Indexable_Builder_Double.php b/tests/Unit/Doubles/Builders/Indexable_Builder_Double.php index a000f30110b..f09c0299ee5 100644 --- a/tests/Unit/Doubles/Builders/Indexable_Builder_Double.php +++ b/tests/Unit/Doubles/Builders/Indexable_Builder_Double.php @@ -19,7 +19,7 @@ final class Indexable_Builder_Double extends Indexable_Builder { * @return Indexable The indexable. */ public function exposed_save_indexable( $indexable, $indexable_before = null ) { - return $this->save_indexable( $indexable, $indexable_before ); + return $this->indexable_helper->save_indexable( $indexable, $indexable_before ); } /** diff --git a/tests/Unit/Helpers/Indexable_Helper_Test.php b/tests/Unit/Helpers/Indexable_Helper_Test.php index 41b9c44498f..37eaa4655f4 100644 --- a/tests/Unit/Helpers/Indexable_Helper_Test.php +++ b/tests/Unit/Helpers/Indexable_Helper_Test.php @@ -337,4 +337,185 @@ public static function provider_check_if_default_field() { [ 'schema_article_type', 'irrelevant', false ], // Checking for fields that don't have an explicit default will always return false. ]; } + + /** + * Tests should_index_indexable method. + * + * @dataProvider provider_should_index_indexable + * @covers ::should_index_indexable + * @covers ::should_index_indexables + * + * @param bool $is_production_mode Whether is production mode. + * @param bool $should_index_indexables Whether indexables should be created. + * @param bool $should_save_indexable What the "wpseo_should_save_indexable" filter returns. + * @param bool $expected_result If indexable should be indexed. + * + * @return void + */ + public function test_should_index_indexable( $is_production_mode, $should_index_indexables, $should_save_indexable, $expected_result ) { + $indexable = Mockery::mock( Indexable_Mock::class ); + + $this->environment_helper + ->expects( 'is_production_mode' ) + ->andReturn( $is_production_mode ); + + // In order to test not having an overriding "Yoast\WP\SEO\should_index_indexables" filter. + if ( $should_index_indexables === null ) { + $should_index_indexables = $is_production_mode; + } + Monkey\Filters\expectApplied( 'Yoast\WP\SEO\should_index_indexables' ) + ->once() + ->with( $is_production_mode ) + ->andReturn( $should_index_indexables ); + + // In order to test not having an overriding "wpseo_should_save_indexable" filter. + if ( $should_save_indexable === null ) { + $should_save_indexable = $should_index_indexables; + } + Monkey\Filters\expectApplied( 'wpseo_should_save_indexable' ) + ->once() + ->with( $should_index_indexables, $indexable ) + ->andReturn( $should_save_indexable ); + + $result = $this->instance->should_index_indexable( $indexable ); + $this->assertSame( $expected_result, $result ); + } + + /** + * Data provider for test_should_index_indexable. + * + * @return array + */ + public static function provider_should_index_indexable() { + yield 'Not production and no overriding filters' => [ + 'is_production_mode' => false, + 'should_index_indexables' => null, + 'should_save_indexable' => null, + 'expected_result' => false, + ]; + yield 'Not production, but with an overriding filter allowing general indexing' => [ + 'is_production_mode' => false, + 'should_index_indexables' => true, + 'should_save_indexable' => null, + 'expected_result' => true, + ]; + yield 'Not production, but with an overriding filter disallowing general indexing' => [ + 'is_production_mode' => false, + 'should_index_indexables' => false, + 'should_save_indexable' => null, + 'expected_result' => false, + ]; + yield 'Not production, but with an overriding filter allowing indexing of that specific indexable' => [ + 'is_production_mode' => false, + 'should_index_indexables' => null, + 'should_save_indexable' => true, + 'expected_result' => true, + ]; + yield 'Not production, but with an overriding filter disallowing general indexing, but with an overriding filter allowing indexing of that specific indexable' => [ + 'is_production_mode' => false, + 'should_index_indexables' => false, + 'should_save_indexable' => true, + 'expected_result' => true, + ]; + yield 'Production and no overriding filters' => [ + 'is_production_mode' => true, + 'should_index_indexables' => null, + 'should_save_indexable' => null, + 'expected_result' => true, + ]; + yield 'Production, but with an overriding filter disallowing general indexing' => [ + 'is_production_mode' => true, + 'should_index_indexables' => false, + 'should_save_indexable' => null, + 'expected_result' => false, + ]; + yield 'Production, with an overriding filter disallowing general indexing, but with an overriding filter allowing indexing of that specific indexable' => [ + 'is_production_mode' => true, + 'should_index_indexables' => false, + 'should_save_indexable' => true, + 'expected_result' => true, + ]; + yield 'Production, with an overriding filter allowing general indexing, but with an overriding filter disallowing indexing of that specific indexable' => [ + 'is_production_mode' => true, + 'should_index_indexables' => true, + 'should_save_indexable' => false, + 'expected_result' => false, + ]; + } + + /** + * Tests save_indexable method. + * + * @dataProvider provider_save_indexable + * @covers ::save_indexable + * @covers ::should_index_indexable + * @covers ::should_index_indexables + * + * @param bool $should_save Whether the indexable should be saved. + * @param Indexable $indexable_before The indexable how it was before all this. + * @param int $save_times Times that indexable will be saved. + * @param int $action_times Times that the "wpseo_save_indexable" action will be run. + * + * @return void + */ + public function test_save_indexable( $should_save, $indexable_before, $save_times, $action_times ) { + $indexable = Mockery::mock( Indexable_Mock::class ); + + $this->environment_helper + ->expects( 'is_production_mode' ) + ->andReturn( true ); + + Monkey\Filters\expectApplied( 'Yoast\WP\SEO\should_index_indexables' ) + ->once() + ->andReturn( true ); + + Monkey\Filters\expectApplied( 'wpseo_should_save_indexable' ) + ->once( $action_times ) + ->andReturn( $should_save ); + + $indexable + ->expects( 'save' ) + ->times( $save_times ); + + Monkey\Actions\expectDone( 'wpseo_save_indexable' ) + ->times( $action_times ) + ->with( $indexable, $indexable_before ); + + $result = $this->instance->save_indexable( $indexable, $indexable_before ); + $this->assertSame( $result, $indexable ); + } + + /** + * Data provider for test_save_indexable. + * + * @return array + */ + public static function provider_save_indexable() { + $indexable_before = Mockery::mock( Indexable_Mock::class ); + + yield 'Indexable should be saved and there is no past iteration of the indexable passed' => [ + 'should_save' => true, + 'indexable_before' => null, + 'save_times' => 1, + 'action_times' => 0, + ]; + yield 'Indexable should not be saved and there is no past iteration of the indexable passed' => [ + 'should_save' => false, + 'indexable_before' => null, + 'save_times' => 0, + 'action_times' => 0, + ]; + yield 'Indexable should be saved and there is a past iteration of the indexable passed' => [ + 'should_save' => true, + 'indexable_before' => $indexable_before, + 'save_times' => 1, + 'action_times' => 1, + ]; + yield 'Indexable should not be saved and there is a past iteration of the indexable passed' => [ + 'should_save' => false, + 'indexable_before' => $indexable_before, + 'save_times' => 0, + 'action_times' => 0, + ]; + } } diff --git a/tests/Unit/Integrations/Watchers/Indexable_Ancestor_Watcher_Test.php b/tests/Unit/Integrations/Watchers/Indexable_Ancestor_Watcher_Test.php index a1485aa71fa..30065b03a78 100644 --- a/tests/Unit/Integrations/Watchers/Indexable_Ancestor_Watcher_Test.php +++ b/tests/Unit/Integrations/Watchers/Indexable_Ancestor_Watcher_Test.php @@ -6,6 +6,7 @@ use Mockery; use Yoast\WP\SEO\Builders\Indexable_Hierarchy_Builder; use Yoast\WP\SEO\Conditionals\Migrations_Conditional; +use Yoast\WP\SEO\Helpers\Indexable_Helper; use Yoast\WP\SEO\Helpers\Permalink_Helper; use Yoast\WP\SEO\Helpers\Post_Type_Helper; use Yoast\WP\SEO\Integrations\Watchers\Indexable_Ancestor_Watcher; @@ -85,6 +86,7 @@ protected function set_up() { $this->indexable_repository = Mockery::mock( Indexable_Repository::class ); $this->indexable_hierarchy_builder = Mockery::mock( Indexable_Hierarchy_Builder::class ); $this->indexable_hierarchy_repository = Mockery::mock( Indexable_Hierarchy_Repository::class ); + $this->indexable_helper = Mockery::mock( Indexable_Helper::class ); $this->permalink_helper = Mockery::mock( Permalink_Helper::class ); $this->post_type_helper = Mockery::mock( Post_Type_Helper::class ); @@ -92,6 +94,7 @@ protected function set_up() { $this->indexable_repository, $this->indexable_hierarchy_builder, $this->indexable_hierarchy_repository, + $this->indexable_helper, $this->permalink_helper, $this->post_type_helper ); @@ -200,7 +203,6 @@ public function test_reset_children() { $child_indexable = Mockery::mock( Indexable_Mock::class ); $child_indexable->permalink = 'https://example.org/old-child-permalink'; - $child_indexable->expects( 'save' )->once(); $this->indexable_hierarchy_builder->expects( 'build' )->with( $child_indexable ); @@ -221,6 +223,11 @@ public function test_reset_children() { ->with( [ 1 ] ) ->andReturn( [ $child_indexable ] ); + $this->indexable_helper + ->expects( 'save_indexable' ) + ->with( $child_indexable ) + ->once(); + $this->assertTrue( $this->instance->reset_children( $indexable, $indexable_before ) ); } @@ -245,7 +252,6 @@ public function test_reset_children_for_term() { $child_indexable->object_type = 'term'; $child_indexable->permalink = 'https://example.org/old-child-permalink'; $child_indexable->object_id = 23; - $child_indexable->expects( 'save' )->once(); $this->indexable_hierarchy_repository ->expects( 'find_children' ) @@ -305,6 +311,11 @@ public function test_reset_children_for_term() { $this->set_expectations_for_update_hierarchy_and_permalink( $indexable_term_1, $indexable_term_2, $additional_indexable ); + $this->indexable_helper + ->expects( 'save_indexable' ) + ->with( $child_indexable ) + ->once(); + $this->assertTrue( $this->instance->reset_children( $indexable, $indexable_before ) ); $this->assertSame( 'https://example.org/permalink', $indexable_term_1->permalink ); $this->assertSame( 'https://example.org/permalink', $indexable_term_2->permalink ); @@ -444,7 +455,10 @@ private function set_expectations_for_update_hierarchy_and_permalink( ...$indexa ->with( $indexable ) ->andReturn( 'https://example.org/permalink' ); - $indexable->expects( 'save' ); + $this->indexable_helper + ->expects( 'save_indexable' ) + ->with( $indexable ) + ->once(); } } } diff --git a/tests/Unit/Integrations/Watchers/Indexable_Author_Watcher_Test.php b/tests/Unit/Integrations/Watchers/Indexable_Author_Watcher_Test.php index ddac0b2cd55..a831707de35 100644 --- a/tests/Unit/Integrations/Watchers/Indexable_Author_Watcher_Test.php +++ b/tests/Unit/Integrations/Watchers/Indexable_Author_Watcher_Test.php @@ -7,6 +7,7 @@ use Yoast\WP\Lib\ORM; use Yoast\WP\SEO\Builders\Indexable_Builder; use Yoast\WP\SEO\Conditionals\Migrations_Conditional; +use Yoast\WP\SEO\Helpers\Indexable_Helper; use Yoast\WP\SEO\Integrations\Watchers\Indexable_Author_Watcher; use Yoast\WP\SEO\Models\Indexable; use Yoast\WP\SEO\Repositories\Indexable_Repository; @@ -38,6 +39,13 @@ final class Indexable_Author_Watcher_Test extends TestCase { */ private $builder; + /** + * Represents the indexable helper. + * + * @var Mockery\MockInterface|Indexable_Helper + */ + private $indexable_helper; + /** * Represents the instance to test. * @@ -53,9 +61,14 @@ final class Indexable_Author_Watcher_Test extends TestCase { protected function set_up() { parent::set_up(); - $this->repository = Mockery::mock( Indexable_Repository::class ); - $this->builder = Mockery::mock( Indexable_Builder::class ); - $this->instance = new Indexable_Author_Watcher( $this->repository, $this->builder ); + $this->repository = Mockery::mock( Indexable_Repository::class ); + $this->indexable_helper = Mockery::mock( Indexable_Helper::class ); + $this->builder = Mockery::mock( Indexable_Builder::class ); + $this->instance = new Indexable_Author_Watcher( + $this->repository, + $this->indexable_helper, + $this->builder + ); } /** @@ -144,7 +157,6 @@ public function test_build_indexable() { $indexable_mock->orm = Mockery::mock( ORM::class ); $indexable_mock->orm->expects( 'get' )->with( 'object_last_modified' )->andReturn( '1234-12-12 00:00:00' ); $indexable_mock->orm->expects( 'set' )->with( 'object_last_modified', '1234-12-12 12:12:12' ); - $indexable_mock->expects( 'save' )->once(); $this->repository ->expects( 'find_by_id_and_type' ) @@ -158,6 +170,11 @@ public function test_build_indexable() { ->with( $id, 'user', $indexable_mock ) ->andReturn( $indexable_mock ); + $this->indexable_helper + ->expects( 'save_indexable' ) + ->with( $indexable_mock ) + ->once(); + $this->instance->build_indexable( $id ); } @@ -177,7 +194,6 @@ public function test_build_indexable_not_found() { $indexable_mock->orm = Mockery::mock( ORM::class ); $indexable_mock->orm->expects( 'get' )->with( 'object_last_modified' )->andReturn( '1234-12-12 00:00:00' ); $indexable_mock->orm->expects( 'set' )->with( 'object_last_modified', '1234-12-12 12:12:12' ); - $indexable_mock->expects( 'save' )->once(); $this->repository ->expects( 'find_by_id_and_type' ) @@ -191,6 +207,11 @@ public function test_build_indexable_not_found() { ->with( $id, 'user', false ) ->andReturn( $indexable_mock ); + $this->indexable_helper + ->expects( 'save_indexable' ) + ->with( $indexable_mock ) + ->once(); + $this->instance->build_indexable( $id ); } } diff --git a/tests/Unit/Integrations/Watchers/Indexable_Home_Page_Watcher_Test.php b/tests/Unit/Integrations/Watchers/Indexable_Home_Page_Watcher_Test.php index aea176e05d4..5daeae0d21c 100644 --- a/tests/Unit/Integrations/Watchers/Indexable_Home_Page_Watcher_Test.php +++ b/tests/Unit/Integrations/Watchers/Indexable_Home_Page_Watcher_Test.php @@ -6,6 +6,7 @@ use Mockery; use Yoast\WP\SEO\Builders\Indexable_Builder; use Yoast\WP\SEO\Conditionals\Migrations_Conditional; +use Yoast\WP\SEO\Helpers\Indexable_Helper; use Yoast\WP\SEO\Integrations\Watchers\Indexable_Home_Page_Watcher; use Yoast\WP\SEO\Models\Indexable; use Yoast\WP\SEO\Repositories\Indexable_Repository; @@ -30,6 +31,13 @@ final class Indexable_Home_Page_Watcher_Test extends TestCase { */ private $repository; + /** + * Represents the indexable helper. + * + * @var Mockery\MockInterface|Indexable_Helper + */ + private $indexable_helper; + /** * Represents the indexable builder. * @@ -52,9 +60,14 @@ final class Indexable_Home_Page_Watcher_Test extends TestCase { protected function set_up() { parent::set_up(); - $this->repository = Mockery::mock( Indexable_Repository::class ); - $this->builder = Mockery::mock( Indexable_Builder::class ); - $this->instance = new Indexable_Home_Page_Watcher( $this->repository, $this->builder ); + $this->repository = Mockery::mock( Indexable_Repository::class ); + $this->indexable_helper = Mockery::mock( Indexable_Helper::class ); + $this->builder = Mockery::mock( Indexable_Builder::class ); + $this->instance = new Indexable_Home_Page_Watcher( + $this->repository, + $this->indexable_helper, + $this->builder + ); } /** @@ -205,6 +218,11 @@ public function test_build_indexable_without_indexable() { ->with( false ) ->andReturn( false ); + $this->indexable_helper + ->expects( 'should_index_indexables' ) + ->once() + ->andReturn( true ); + $this->builder ->expects( 'build_for_home_page' ) ->once() diff --git a/tests/Unit/Integrations/Watchers/Indexable_Post_Type_Archive_Watcher_Test.php b/tests/Unit/Integrations/Watchers/Indexable_Post_Type_Archive_Watcher_Test.php index 32780c46841..b1a65c97312 100644 --- a/tests/Unit/Integrations/Watchers/Indexable_Post_Type_Archive_Watcher_Test.php +++ b/tests/Unit/Integrations/Watchers/Indexable_Post_Type_Archive_Watcher_Test.php @@ -6,6 +6,7 @@ use Mockery; use Yoast\WP\SEO\Builders\Indexable_Builder; use Yoast\WP\SEO\Conditionals\Migrations_Conditional; +use Yoast\WP\SEO\Helpers\Indexable_Helper; use Yoast\WP\SEO\Integrations\Watchers\Indexable_Post_Type_Archive_Watcher; use Yoast\WP\SEO\Models\Indexable; use Yoast\WP\SEO\Repositories\Indexable_Repository; @@ -32,6 +33,13 @@ final class Indexable_Post_Type_Archive_Watcher_Test extends TestCase { */ private $repository; + /** + * Represents the indexable helper. + * + * @var Mockery\MockInterface|Indexable_Helper + */ + private $indexable_helper; + /** * Represents the indexable builder. * @@ -54,9 +62,14 @@ final class Indexable_Post_Type_Archive_Watcher_Test extends TestCase { protected function set_up() { parent::set_up(); - $this->repository = Mockery::mock( Indexable_Repository::class ); - $this->builder = Mockery::mock( Indexable_Builder::class ); - $this->instance = new Indexable_Post_Type_Archive_Watcher( $this->repository, $this->builder ); + $this->repository = Mockery::mock( Indexable_Repository::class ); + $this->indexable_helper = Mockery::mock( Indexable_Helper::class ); + $this->builder = Mockery::mock( Indexable_Builder::class ); + $this->instance = new Indexable_Post_Type_Archive_Watcher( + $this->repository, + $this->indexable_helper, + $this->builder + ); } /** @@ -101,7 +114,6 @@ public function test_check_option_first_time_save() { $indexable_mock->orm = Mockery::mock( ORM::class ); $indexable_mock->orm->expects( 'get' )->with( 'object_last_modified' )->andReturn( '1234-12-12 00:00:00' ); $indexable_mock->orm->expects( 'set' )->with( 'object_last_modified', '1234-12-12 12:12:12' ); - $indexable_mock->expects( 'save' )->once(); $this->repository ->expects( 'find_for_post_type_archive' ) @@ -115,6 +127,11 @@ public function test_check_option_first_time_save() { ->with( 'my-post-type', $indexable_mock ) ->andReturn( $indexable_mock ); + $this->indexable_helper + ->expects( 'save_indexable' ) + ->with( $indexable_mock ) + ->once(); + $this->assertTrue( $this->instance->check_option( false, [ 'title-ptarchive-my-post-type' => 'baz' ] ) ); } @@ -146,7 +163,6 @@ public function test_update_wpseo_titles_value() { $indexable_mock->orm = Mockery::mock( ORM::class ); $indexable_mock->orm->expects( 'get' )->with( 'object_last_modified' )->andReturn( '1234-12-12 00:00:00' ); $indexable_mock->orm->expects( 'set' )->with( 'object_last_modified', '1234-12-12 12:12:12' ); - $indexable_mock->expects( 'save' )->once(); $this->repository ->expects( 'find_for_post_type_archive' ) @@ -160,6 +176,11 @@ public function test_update_wpseo_titles_value() { ->with( 'my-post-type', $indexable_mock ) ->andReturn( $indexable_mock ); + $this->indexable_helper + ->expects( 'save_indexable' ) + ->with( $indexable_mock ) + ->once(); + $this->assertTrue( $this->instance->check_option( [ 'title-ptarchive-my-post-type' => 'bar' ], [ 'title-ptarchive-my-post-type' => 'baz' ] ) ); } @@ -179,7 +200,6 @@ public function test_update_wpseo_titles_value_new() { $indexable_mock->orm = Mockery::mock( ORM::class ); $indexable_mock->orm->expects( 'get' )->with( 'object_last_modified' )->andReturn( '1234-12-12 00:00:00' ); $indexable_mock->orm->expects( 'set' )->with( 'object_last_modified', '1234-12-12 12:12:12' ); - $indexable_mock->expects( 'save' )->once(); $this->repository ->expects( 'find_for_post_type_archive' ) @@ -193,6 +213,11 @@ public function test_update_wpseo_titles_value_new() { ->with( 'my-post-type', $indexable_mock ) ->andReturn( $indexable_mock ); + $this->indexable_helper + ->expects( 'save_indexable' ) + ->with( $indexable_mock ) + ->once(); + $this->assertTrue( $this->instance->check_option( [], [ 'title-ptarchive-my-post-type' => 'baz' ] ) ); } @@ -212,13 +237,11 @@ public function test_update_wpseo_titles_value_switched() { $indexable_mock->orm = Mockery::mock( ORM::class ); $indexable_mock->orm->expects( 'get' )->with( 'object_last_modified' )->andReturn( '1234-12-12 00:00:00' ); $indexable_mock->orm->expects( 'set' )->with( 'object_last_modified', '1234-12-12 12:12:12' ); - $indexable_mock->expects( 'save' )->once(); $other_indexable_mock = Mockery::mock( Indexable::class ); $other_indexable_mock->orm = Mockery::mock( ORM::class ); $other_indexable_mock->orm->expects( 'get' )->with( 'object_last_modified' )->andReturn( '1234-12-12 00:00:00' ); $other_indexable_mock->orm->expects( 'set' )->with( 'object_last_modified', '1234-12-12 12:12:12' ); - $other_indexable_mock->expects( 'save' )->once(); $this->repository ->expects( 'find_for_post_type_archive' ) @@ -244,6 +267,16 @@ public function test_update_wpseo_titles_value_switched() { ->with( 'other-post-type', $other_indexable_mock ) ->andReturn( $other_indexable_mock ); + $this->indexable_helper + ->expects( 'save_indexable' ) + ->with( $indexable_mock ) + ->once(); + + $this->indexable_helper + ->expects( 'save_indexable' ) + ->with( $other_indexable_mock ) + ->once(); + $this->assertTrue( $this->instance->check_option( [ 'title-ptarchive-my-post-type' => 'baz' ], [ 'title-ptarchive-other-post-type' => 'baz' ] ) ); } @@ -290,7 +323,6 @@ public function test_build_indexable_without_indexable() { $indexable_mock->orm = Mockery::mock( ORM::class ); $indexable_mock->orm->expects( 'get' )->with( 'object_last_modified' )->andReturn( '1234-12-12 00:00:00' ); $indexable_mock->orm->expects( 'set' )->with( 'object_last_modified', '1234-12-12 12:12:12' ); - $indexable_mock->expects( 'save' )->once(); $this->repository ->expects( 'find_for_post_type_archive' ) @@ -304,6 +336,11 @@ public function test_build_indexable_without_indexable() { ->with( 'my-post-type', false ) ->andReturn( $indexable_mock ); + $this->indexable_helper + ->expects( 'save_indexable' ) + ->with( $indexable_mock ) + ->once(); + $this->instance->build_indexable( 'my-post-type' ); } } diff --git a/tests/Unit/Integrations/Watchers/Indexable_Post_Watcher_Test.php b/tests/Unit/Integrations/Watchers/Indexable_Post_Watcher_Test.php index 228572f4bb8..29e3c11fe76 100644 --- a/tests/Unit/Integrations/Watchers/Indexable_Post_Watcher_Test.php +++ b/tests/Unit/Integrations/Watchers/Indexable_Post_Watcher_Test.php @@ -10,6 +10,7 @@ use Yoast\WP\SEO\Builders\Indexable_Link_Builder; use Yoast\WP\SEO\Conditionals\Migrations_Conditional; use Yoast\WP\SEO\Helpers\Author_Archive_Helper; +use Yoast\WP\SEO\Helpers\Indexable_Helper; use Yoast\WP\SEO\Helpers\Post_Helper; use Yoast\WP\SEO\Integrations\Cleanup_Integration; use Yoast\WP\SEO\Integrations\Watchers\Indexable_Post_Watcher; @@ -55,7 +56,7 @@ final class Indexable_Post_Watcher_Test extends TestCase { /** * The link builder. * - * @var Indexable_Link_Builder + * @var Mockery\MockInterface|Indexable_Link_Builder */ protected $link_builder; @@ -66,6 +67,13 @@ final class Indexable_Post_Watcher_Test extends TestCase { */ private $author_archive; + /** + * The indexable helper mock. + * + * @var Mockery\MockInterface|Indexable_Helper + */ + private $indexable_helper; + /** * Represents the class we are testing. * @@ -100,6 +108,7 @@ protected function set_up() { $this->hierarchy_repository = Mockery::mock( Indexable_Hierarchy_Repository::class ); $this->link_builder = Mockery::mock( Indexable_Link_Builder::class ); $this->author_archive = Mockery::mock( Author_Archive_Helper::class ); + $this->indexable_helper = Mockery::mock( Indexable_Helper::class ); $this->post = Mockery::mock( Post_Helper::class ); $this->logger = Mockery::mock( Logger::class ); $this->instance = Mockery::mock( @@ -110,6 +119,7 @@ protected function set_up() { $this->hierarchy_repository, $this->link_builder, $this->author_archive, + $this->indexable_helper, $this->post, $this->logger, ] @@ -219,10 +229,6 @@ public function test_build_indexable() { $indexable_mock = Mockery::mock( Indexable_Mock::class ); - $indexable_mock - ->expects( 'save' ) - ->once(); - $this->repository ->expects( 'find_by_id_and_type' ) ->once() @@ -252,6 +258,11 @@ public function test_build_indexable() { ->once() ->with( $indexable_mock, $post_content ); + $this->indexable_helper + ->expects( 'save_indexable' ) + ->with( $indexable_mock ) + ->once(); + $this->instance ->expects( 'update_has_public_posts' ) ->with( $indexable_mock ) @@ -328,7 +339,6 @@ public function test_build_indexable_does_not_exist() { ]; $indexable_mock = Mockery::mock( Indexable_Mock::class ); - $indexable_mock->expects( 'save' )->once(); $this->repository->expects( 'find_by_id_and_type' )->once()->with( $post_id, 'post', false )->andReturn( false ); $this->builder->expects( 'build_for_id_and_type' )->once()->with( $post_id, 'post', false )->andReturn( $indexable_mock ); @@ -349,6 +359,11 @@ public function test_build_indexable_does_not_exist() { ->once() ->andReturn( [ 'publish' ] ); + $this->indexable_helper + ->expects( 'save_indexable' ) + ->with( $indexable_mock ) + ->once(); + $this->instance ->expects( 'update_has_public_posts' ) ->with( $indexable_mock ) @@ -404,7 +419,11 @@ public function test_update_has_public_posts_with_post() { ->with( 11 ) ->once() ->andReturn( true ); - $author_indexable->expects( 'save' )->once(); + + $this->indexable_helper + ->expects( 'save_indexable' ) + ->with( $author_indexable ) + ->once(); $this->post->expects( 'update_has_public_posts_on_attachments' )->once()->with( 33, null )->andReturnTrue(); @@ -490,8 +509,6 @@ public function test_reschedule_cleanup_when_author_does_not_have_posts() { $author_indexable = Mockery::mock( Indexable_Mock::class ); $author_indexable->object_id = 11; - $author_indexable->expects( 'save' ); - $this->repository->expects( 'find_by_id_and_type' ) ->with( 11, 'user' ) ->once() @@ -499,6 +516,12 @@ public function test_reschedule_cleanup_when_author_does_not_have_posts() { $this->author_archive->expects( 'author_has_public_posts' ) ->with( 11 ) ->andReturnFalse(); + + $this->indexable_helper + ->expects( 'save_indexable' ) + ->with( $author_indexable ) + ->once(); + $this->post->expects( 'update_has_public_posts_on_attachments' ) ->once() ->with( 33, null ) @@ -531,7 +554,6 @@ public function test_update_relations() { $indexable = Mockery::mock( Indexable_Mock::class ); $indexable->object_last_modified = '1234-12-12 00:00:00'; - $indexable->expects( 'save' )->once(); $this->instance ->expects( 'get_related_indexables' ) @@ -539,6 +561,11 @@ public function test_update_relations() { ->with( $post ) ->andReturn( [ $indexable ] ); + $this->indexable_helper + ->expects( 'save_indexable' ) + ->with( $indexable ) + ->once(); + $this->instance->update_relations( $post ); $this->assertSame( '1234-12-12 12:12:12', $indexable->object_last_modified ); diff --git a/tests/Unit/Integrations/Watchers/Indexable_Term_Watcher_Test.php b/tests/Unit/Integrations/Watchers/Indexable_Term_Watcher_Test.php index 194d4b4fbf0..c770d601568 100644 --- a/tests/Unit/Integrations/Watchers/Indexable_Term_Watcher_Test.php +++ b/tests/Unit/Integrations/Watchers/Indexable_Term_Watcher_Test.php @@ -7,6 +7,7 @@ use Yoast\WP\SEO\Builders\Indexable_Builder; use Yoast\WP\SEO\Builders\Indexable_Link_Builder; use Yoast\WP\SEO\Conditionals\Migrations_Conditional; +use Yoast\WP\SEO\Helpers\Indexable_Helper; use Yoast\WP\SEO\Helpers\Site_Helper; use Yoast\WP\SEO\Integrations\Watchers\Indexable_Term_Watcher; use Yoast\WP\SEO\Models\Indexable; @@ -46,6 +47,13 @@ final class Indexable_Term_Watcher_Test extends TestCase { */ protected $link_builder; + /** + * Represents the indexable helper. + * + * @var Mockery\MockInterface|Indexable_Helper + */ + private $indexable_helper; + /** * Represents the site helper. * @@ -68,15 +76,17 @@ final class Indexable_Term_Watcher_Test extends TestCase { protected function set_up() { parent::set_up(); - $this->repository = Mockery::mock( Indexable_Repository::class ); - $this->builder = Mockery::mock( Indexable_Builder::class ); - $this->link_builder = Mockery::mock( Indexable_Link_Builder::class ); - $this->site = Mockery::mock( Site_Helper::class ); + $this->repository = Mockery::mock( Indexable_Repository::class ); + $this->builder = Mockery::mock( Indexable_Builder::class ); + $this->link_builder = Mockery::mock( Indexable_Link_Builder::class ); + $this->indexable_helper = Mockery::mock( Indexable_Helper::class ); + $this->site = Mockery::mock( Site_Helper::class ); $this->instance = new Indexable_Term_Watcher( $this->repository, $this->builder, $this->link_builder, + $this->indexable_helper, $this->site ); } @@ -205,8 +215,9 @@ public function test_build_indexable() { 'This is a term description, with a link.' ); - $indexable - ->expects( 'save' ) + $this->indexable_helper + ->expects( 'save_indexable' ) + ->with( $indexable ) ->once(); $this->instance->build_indexable( 1 ); @@ -369,8 +380,9 @@ public function test_build_does_not_exist() { 'This is a term description, with a link.' ); - $indexable - ->expects( 'save' ) + $this->indexable_helper + ->expects( 'save_indexable' ) + ->with( $indexable ) ->once(); $this->instance->build_indexable( 1 ); diff --git a/tests/Unit/Repositories/Indexable_Hierarchy_Repository_Test.php b/tests/Unit/Repositories/Indexable_Hierarchy_Repository_Test.php index fd47876ecea..3a06e429cbc 100644 --- a/tests/Unit/Repositories/Indexable_Hierarchy_Repository_Test.php +++ b/tests/Unit/Repositories/Indexable_Hierarchy_Repository_Test.php @@ -7,6 +7,7 @@ use wpdb; use Yoast\WP\Lib\ORM; use Yoast\WP\SEO\Builders\Indexable_Hierarchy_Builder; +use Yoast\WP\SEO\Helpers\Indexable_Helper; use Yoast\WP\SEO\Repositories\Indexable_Hierarchy_Repository; use Yoast\WP\SEO\Tests\Unit\Doubles\Models\Indexable_Hierarchy_Mock; use Yoast\WP\SEO\Tests\Unit\Doubles\Models\Indexable_Mock; @@ -36,14 +37,22 @@ final class Indexable_Hierarchy_Repository_Test extends TestCase { */ protected $builder; + /** + * Represents the indexable helper. + * + * @var Mockery\Mock|Indexable_Helper + */ + protected $indexable_helper; + /** * {@inheritDoc} */ protected function set_up() { parent::set_up(); - $this->instance = Mockery::mock( Indexable_Hierarchy_Repository::class )->makePartial(); - $this->builder = Mockery::mock( Indexable_Hierarchy_Builder::class ); + $this->instance = Mockery::mock( Indexable_Hierarchy_Repository::class )->makePartial(); + $this->builder = Mockery::mock( Indexable_Hierarchy_Builder::class ); + $this->indexable_helper = Mockery::mock( Indexable_Helper::class ); } /** @@ -62,6 +71,22 @@ public function test_set_builder() { ); } + /** + * Tests setting the helper object. + * + * @covers ::set_helper + * + * @return void + */ + public function test_set_helper() { + $this->instance->set_helper( $this->indexable_helper ); + + $this->assertInstanceOf( + Indexable_Helper::class, + $this->getPropertyValue( $this->instance, 'indexable_helper' ) + ); + } + /** * Tests the retrieval of the find_ancestors when having already results. * @@ -201,6 +226,13 @@ public function test_clear_ancestors() { * @return void */ public function test_add_ancestor() { + $this->instance->set_helper( $this->indexable_helper ); + + $this->indexable_helper + ->expects( 'should_index_indexables' ) + ->once() + ->andReturnTrue(); + $hierarchy = Mockery::mock( Indexable_Hierarchy_Mock::class ); $hierarchy->indexable_id = 1; $hierarchy->ancestor_id = 2; @@ -209,10 +241,10 @@ public function test_add_ancestor() { Functions\expect( 'get_current_blog_id' )->once()->andReturn( 1 ); - $hierarchy->expects( 'save' )->once()->andReturn( true ); - $orm_object = Mockery::mock( ORM::class )->makePartial(); + $hierarchy->expects( 'save' )->once()->andReturn( true ); + $orm_object ->expects( 'create' ) ->with( diff --git a/tests/WP/Admin/Watchers/Indexable_Ancestor_Watcher_Test.php b/tests/WP/Admin/Watchers/Indexable_Ancestor_Watcher_Test.php index 332dd594f91..21bf8af4e51 100644 --- a/tests/WP/Admin/Watchers/Indexable_Ancestor_Watcher_Test.php +++ b/tests/WP/Admin/Watchers/Indexable_Ancestor_Watcher_Test.php @@ -6,6 +6,7 @@ use WP_Post; use Yoast\WP\Lib\Model; use Yoast\WP\SEO\Builders\Indexable_Hierarchy_Builder; +use Yoast\WP\SEO\Helpers\Indexable_Helper; use Yoast\WP\SEO\Helpers\Permalink_Helper; use Yoast\WP\SEO\Helpers\Post_Type_Helper; use Yoast\WP\SEO\Repositories\Indexable_Hierarchy_Repository; @@ -45,6 +46,13 @@ final class Indexable_Ancestor_Watcher_Test extends TestCase { */ private $indexable_hierarchy_repository; + /** + * Represents the Indexable_Helper. + * + * @var Mockery\MockInterface|Indexable_Helper + */ + private $indexable_helper; + /** * Represents the Permalink_Helper. * @@ -77,6 +85,7 @@ public function set_up() { $this->indexable_repository = Mockery::mock( Indexable_Repository::class ); $this->indexable_hierarchy_builder = Mockery::mock( Indexable_Hierarchy_Builder::class ); $this->indexable_hierarchy_repository = Mockery::mock( Indexable_Hierarchy_Repository::class ); + $this->indexable_helper = Mockery::mock( Indexable_Helper::class ); $this->permalink_helper = Mockery::mock( Permalink_Helper::class ); $this->post_type_helper = Mockery::mock( Post_Type_Helper::class ); @@ -84,6 +93,7 @@ public function set_up() { $this->indexable_repository, $this->indexable_hierarchy_builder, $this->indexable_hierarchy_repository, + $this->indexable_helper, $this->permalink_helper, $this->post_type_helper ); diff --git a/tests/WP/Builders/Indexable_Post_Type_Archive_Builder_Test.php b/tests/WP/Builders/Indexable_Post_Type_Archive_Builder_Test.php index 60fda177480..1b9c7ad3685 100644 --- a/tests/WP/Builders/Indexable_Post_Type_Archive_Builder_Test.php +++ b/tests/WP/Builders/Indexable_Post_Type_Archive_Builder_Test.php @@ -38,6 +38,19 @@ public function set_up(): void { ); } + /** + * Remove the custom post type after each test. + * + * @return void + */ + public function tear_down() { + // Remove possibly set post type. + \unregister_post_type( 'my-custom-post-type' ); + \unregister_post_type( 'my-private-post-type' ); + + parent::tear_down(); + } + /** * Tests the build method's happy path. * diff --git a/tests/WP/Generators/Schema/Organization_Test.php b/tests/WP/Generators/Schema/Organization_Test.php index ec06d78e4ce..51f9220952e 100644 --- a/tests/WP/Generators/Schema/Organization_Test.php +++ b/tests/WP/Generators/Schema/Organization_Test.php @@ -87,6 +87,18 @@ public function set_up() { $this->context = $meta_tags_context_memoizer->get( $built_indexable, 'page' ); } + /** + * Remove the custom post type after each test. + * + * @return void + */ + public function tear_down() { + // Remove possibly set post type. + \unregister_post_type( 'my-custom-post-type' ); + + parent::tear_down(); + } + /** * Tests that the type is properly set if everything is in the default setting. * diff --git a/tests/WP/Generators/Schema/Person_Test.php b/tests/WP/Generators/Schema/Person_Test.php index fe99d947980..c2479cf0c75 100644 --- a/tests/WP/Generators/Schema/Person_Test.php +++ b/tests/WP/Generators/Schema/Person_Test.php @@ -87,6 +87,18 @@ public function set_up() { $this->context = $meta_tags_context_memoizer->get( $built_indexable, 'page' ); } + /** + * Remove the custom post type after each test. + * + * @return void + */ + public function tear_down() { + // Remove possibly set post type. + \unregister_post_type( 'my-custom-post-type' ); + + parent::tear_down(); + } + /** * Tests that the type is properly set if everything is in the default setting. * diff --git a/tests/WP/Repositories/Primary_Term_Repository_Test.php b/tests/WP/Repositories/Primary_Term_Repository_Test.php new file mode 100644 index 00000000000..2ee61ce37f4 --- /dev/null +++ b/tests/WP/Repositories/Primary_Term_Repository_Test.php @@ -0,0 +1,106 @@ +insert( + $wpdb->prefix . 'yoast_primary_term', + [ + 'id' => '123', + 'post_id' => '110', + 'term_id' => '22', + 'taxonomy' => 'category', + 'created_at' => '2024-05-22 10:16:42', + 'updated_at' => '2024-05-22 10:16:42', + 'blog_id' => '1', + ] + ); + + $this->instance = new Primary_Term_Repository(); + } + + /** + * Tears down the test class. + * + * @return void + */ + public function tear_down(): void { + global $wpdb; + + $wpdb->query( "TRUNCATE TABLE {$wpdb->prefix}yoast_primary_term" ); + + parent::tear_down(); + } + + /** + * Tests the query method. + * + * @covers ::query + * + * @return void + */ + public function test_query() { + + $this->assertIsObject( $this->instance->query() ); + } + + /** + * Tests finding one entry by post id and taxonomy. + * + * @covers ::find_by_post_id_and_taxonomy + * + * @return void + */ + public function test_find_by_post_id_and_taxonomy_one_result() { + + $post_id = 110; + $taxonomy = 'category'; + + $result = $this->instance->find_by_post_id_and_taxonomy( $post_id, $taxonomy ); + + $this->assertInstanceOf( Primary_Term::class, $result, 'The result should be a Primary_term instance.' ); + $this->assertEquals( '123', $result->id, 'The id of the one result should be the one we expect.' ); + } + + /** + * Tests finding no entry by post id and taxonomy. + * + * @covers ::find_by_post_id_and_taxonomy + * + * @return void + */ + public function test_find_by_post_id_and_taxonomy_no_result() { + $post_id = 111; + $taxonomy = 'category'; + + $result = $this->instance->find_by_post_id_and_taxonomy( $post_id, $taxonomy, false ); + + $this->assertFalse( $result, 'The result should be false' ); + } +} diff --git a/tests/WP/Sitemaps/Post_Type_Sitemap_Provider_Test.php b/tests/WP/Sitemaps/Post_Type_Sitemap_Provider_Test.php index 68668c8595c..9ebe5a59308 100644 --- a/tests/WP/Sitemaps/Post_Type_Sitemap_Provider_Test.php +++ b/tests/WP/Sitemaps/Post_Type_Sitemap_Provider_Test.php @@ -2,6 +2,7 @@ namespace Yoast\WP\SEO\Tests\WP\Sitemaps; +use OutOfBoundsException; use WPSEO_Options; use WPSEO_Post_Type_Sitemap_Provider; use WPSEO_Sitemaps; @@ -50,10 +51,31 @@ public function set_up() { * @return void */ public function test_get_index_links_no_entries() { + \add_filter( 'wpseo_sitemap_exclude_post_type', '__return_true' ); $index_links = self::$class_instance->get_index_links( 1 ); $this->assertEquals( [], $index_links ); } + /** + * Any post type that has links on the first page of the sitemap should be included in the index. + * + * @covers ::get_index_links + * + * @return void + */ + public function test_get_index_links_no_posts() { + $index_links = \wp_list_pluck( self::$class_instance->get_index_links( 1 ), 'loc' ); + $this->assertSame( \wp_count_posts( 'post' )->publish, 0 ); + $this->assertSame( \wp_count_posts( 'page' )->publish, 0 ); + $this->assertEquals( + [ + 'http://example.org/post-sitemap.xml', + 'http://example.org/page-sitemap.xml', + ], + $index_links + ); + } + /** * Multiple pages of a post-type should result in multiple index entries. * @@ -101,12 +123,10 @@ public function test_get_index_links_multiple_entries_non_paged() { * @return void */ public function test_get_index_links_empty_bucket() { - $this->factory->post->create(); $this->excluded_posts = [ $this->factory->post->create() ]; // Remove this post. $this->factory->post->create(); - \add_filter( 'wpseo_exclude_from_sitemap_by_post_ids', [ $this, 'exclude_post' ] ); \add_filter( 'wpseo_sitemap_entries_per_page', [ $this, 'return_one' ] ); // Fetch the global sitemap. @@ -115,10 +135,16 @@ public function test_get_index_links_empty_bucket() { // Set the page to the second one, which should not contain an entry, but should exist. \set_query_var( 'sitemap_n', '2' ); - // Load the sitemap. + // Load the sitemap to generate and store the pagination map. $sitemaps = new Sitemaps_Double(); $sitemaps->redirect( $GLOBALS['wp_the_query'] ); + $this->expectOutputContains( \get_permalink( $this->excluded_posts[0] ) ); + + // Now exclude the only post on page 2. + \add_filter( 'wpseo_exclude_from_sitemap_by_post_ids', [ $this, 'exclude_post' ] ); + $sitemaps->redirect( $GLOBALS['wp_the_query'] ); + // Expect an empty list to be output. $this->expectOutputContains( 'xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">' . "\r\n" . '' @@ -313,10 +339,15 @@ public function test_get_url() { * @return void */ public function test_regular_post() { - $this->factory->post->create(); + $post_id = $this->factory->post->create(); + + $expected_urls = [ + \trailingslashit( \home_url() ), + \get_permalink( $post_id ), + ]; + $actual_urls = \wp_list_pluck( self::$class_instance->get_sitemap_links( 'post', 100, 1 ), 'loc' ); - // Expect the created post to be in the sitemap list. - $this->assertCount( 1, self::$class_instance->get_sitemap_links( 'post', 100, 0 ) ); + $this->assertSame( $expected_urls, $actual_urls ); } /** @@ -327,19 +358,35 @@ public function test_regular_post() { * @return void */ public function test_password_protected_post() { + // Create a public and password protected post. + $public_post_id = $this->factory->post->create(); + $this->factory->post->create( [ 'post_password' => 'secret' ] ); + + $expected_urls = [ + \trailingslashit( \home_url() ), + \get_permalink( $public_post_id ), + ]; + $actual_urls = \wp_list_pluck( self::$class_instance->get_sitemap_links( 'post', 100, 1 ), 'loc' ); + + $this->assertSame( $expected_urls, $actual_urls ); + } + + /** + * Tests to make sure that there is no sitemap if all posts are password protected. + * + * @covers ::get_sitemap_links + * + * @return void + */ + public function test_only_password_protected_posts() { // Create password protected post. - $this->factory->post->create( - [ - 'post_password' => 'secret', - ] - ); + $this->factory->post->create( [ 'post_password' => 'secret' ] ); + $this->factory->post->create( [ 'post_password' => 'secret' ] ); + $this->factory->post->create( [ 'post_password' => 'secret' ] ); - // Expect the protected post should not be added. - $this->assertCount( - 0, - self::$class_instance->get_sitemap_links( 'post', 100, 0 ), - 'Password protected posts should not be in the sitemap' - ); + $this->expectException( OutOfBoundsException::class ); + + self::$class_instance->get_sitemap_links( 'post', 1, 2 ); } /** @@ -369,7 +416,7 @@ public function test_regular_attachment() { ); // Expect the attchment to be in the list. - $this->assertCount( 1, self::$class_instance->get_sitemap_links( 'attachment', 100, 0 ) ); + $this->assertCount( 1, self::$class_instance->get_sitemap_links( 'attachment', 100, 1 ) ); } /** @@ -384,9 +431,49 @@ public function test_regular_attachment() { public function test_password_protected_post_parent_attachment() { // Enable attachments in the sitemap. WPSEO_Options::set( 'disable-attachment', false ); + $public_post_id = $this->factory->post->create(); + $public_attachment_id = $this->factory->post->create( + [ + 'post_type' => 'attachment', + 'post_parent' => $public_post_id, + ] + ); + // Create password protected post. + $password_protected_post_id = $this->factory->post->create( + [ + 'post_password' => 'secret', + ] + ); + $this->factory->post->create( + [ + 'post_parent' => $password_protected_post_id, + 'post_type' => 'attachment', + 'post_status' => 'inherit', + ] + ); + + $expected_urls = [ + \get_permalink( $public_attachment_id ), + ]; + $actual_urls = \wp_list_pluck( self::$class_instance->get_sitemap_links( 'attachment', 100, 1 ), 'loc' ); + + // Expect the attachment not to be added to the list. + $this->assertSame( $expected_urls, $actual_urls ); + } + + /** + * Tests to make sure that there is no attachment sitemap if all attachments are part of password protected posts. + * + * @covers ::get_sitemap_links + * + * @return void + */ + public function test_only_password_protected_post_parent_attachments() { + // Enable attachments in the sitemap. + WPSEO_Options::set( 'disable-attachment', false ); // Create password protected post. - $post_id = $this->factory->post->create( + $password_protected_post_id = $this->factory->post->create( [ 'post_password' => 'secret', ] @@ -394,14 +481,16 @@ public function test_password_protected_post_parent_attachment() { $this->factory->post->create( [ - 'post_parent' => $post_id, + 'post_parent' => $password_protected_post_id, 'post_type' => 'attachment', 'post_status' => 'inherit', ] ); + $actual_urls = \wp_list_pluck( self::$class_instance->get_sitemap_links( 'attachment', 100, 1 ), 'loc' ); + // Expect the attachment not to be added to the list. - $this->assertCount( 0, self::$class_instance->get_sitemap_links( 'attachment', 100, 0 ) ); + $this->assertCount( 0, $actual_urls ); } /** diff --git a/tests/WP/Sitemaps/Sitemaps_Test.php b/tests/WP/Sitemaps/Sitemaps_Test.php index 532f936d55b..8c6e48dadd9 100644 --- a/tests/WP/Sitemaps/Sitemaps_Test.php +++ b/tests/WP/Sitemaps/Sitemaps_Test.php @@ -162,6 +162,7 @@ public function test_last_modified_post_type() { $this->factory->post->create( $post_args ); $this->assertEquals( $newest_date, WPSEO_Sitemaps::get_last_modified_gmt( [ 'yoast' ] ) ); + \unregister_post_type( 'yoast' ); } /**