diff --git a/src/memoizers/meta-tags-context-memoizer.php b/src/memoizers/meta-tags-context-memoizer.php index ef5f71ebbb6..6c33dd2babf 100644 --- a/src/memoizers/meta-tags-context-memoizer.php +++ b/src/memoizers/meta-tags-context-memoizer.php @@ -132,7 +132,10 @@ public function for_current_page() { * @return Meta_Tags_Context The meta tags context. */ public function get( Indexable $indexable, $page_type ) { - if ( ! isset( $this->cache[ $indexable->id ] ) ) { + // An unsaved indexable has a null id; normalise it to a stable integer cache key. + $cache_key = (int) $indexable->id; + + if ( ! isset( $this->cache[ $cache_key ] ) ) { $blocks = []; $post = null; if ( $indexable->object_type === 'post' ) { @@ -151,10 +154,10 @@ public function get( Indexable $indexable, $page_type ) { $context->presentation = $this->presentation_memoizer->get( $indexable, $context, $page_type ); - $this->cache[ $indexable->id ] = $context; + $this->cache[ $cache_key ] = $context; } - return $this->cache[ $indexable->id ]; + return $this->cache[ $cache_key ]; } /** diff --git a/src/memoizers/presentation-memoizer.php b/src/memoizers/presentation-memoizer.php index fb619564a46..441fd680ea5 100644 --- a/src/memoizers/presentation-memoizer.php +++ b/src/memoizers/presentation-memoizer.php @@ -46,7 +46,10 @@ public function __construct( ContainerInterface $service_container ) { * @return Indexable_Presentation The indexable presentation. */ public function get( Indexable $indexable, Meta_Tags_Context $context, $page_type ) { - if ( ! isset( $this->cache[ $indexable->id ] ) ) { + // An unsaved indexable has a null id; normalise it to a stable integer cache key. + $cache_key = (int) $indexable->id; + + if ( ! isset( $this->cache[ $cache_key ] ) ) { $presentation = $this->container->get( "Yoast\WP\SEO\Presentations\Indexable_{$page_type}_Presentation", ContainerInterface::NULL_ON_INVALID_REFERENCE ); if ( ! $presentation ) { @@ -60,10 +63,10 @@ public function get( Indexable $indexable, Meta_Tags_Context $context, $page_typ ], ); - $this->cache[ $indexable->id ] = $context->presentation; + $this->cache[ $cache_key ] = $context->presentation; } - return $this->cache[ $indexable->id ]; + return $this->cache[ $cache_key ]; } /** diff --git a/tests/Unit/Doubles/Memoizers/Presentation_Memoizer_Double.php b/tests/Unit/Doubles/Memoizers/Presentation_Memoizer_Double.php index 482c2607f09..686cae1db7b 100644 --- a/tests/Unit/Doubles/Memoizers/Presentation_Memoizer_Double.php +++ b/tests/Unit/Doubles/Memoizers/Presentation_Memoizer_Double.php @@ -20,4 +20,13 @@ final class Presentation_Memoizer_Double extends Presentation_Memoizer { public function set_cache( $key, $value ) { $this->cache[ $key ] = $value; } + + /** + * Used to retrieve the internal cache for testing purposes. + * + * @return array The cache. + */ + public function get_cache() { + return $this->cache; + } } diff --git a/tests/Unit/Memoizers/Meta_Tags_Context_Memoizer_Test.php b/tests/Unit/Memoizers/Meta_Tags_Context_Memoizer_Test.php index 31a1a9728c0..db9c10dbb57 100644 --- a/tests/Unit/Memoizers/Meta_Tags_Context_Memoizer_Test.php +++ b/tests/Unit/Memoizers/Meta_Tags_Context_Memoizer_Test.php @@ -260,6 +260,36 @@ public function test_get_without_cache_for_post() { $this->assertEquals( $this->meta_tags_context_mock, $this->instance->get( $this->indexable, 'the_page_type' ) ); } + /** + * Tests getting the meta tags context for an indexable whose id is null, ensuring it is + * cached under a stable integer key instead of a null array offset. + * + * @covers ::get + * + * @return void + */ + public function test_get_caches_when_indexable_id_is_null() { + $this->indexable->id = null; + + $this->meta_tags_context + ->expects( 'of' ) + ->once() + ->andReturn( $this->meta_tags_context_mock ); + + $this->presentation_memoizer + ->expects( 'get' ) + ->once() + ->with( $this->indexable, $this->meta_tags_context_mock, 'the_page_type' ) + ->andReturn( $this->meta_tags_context_mock->presentation ); + + $first = $this->instance->get( $this->indexable, 'the_page_type' ); + $second = $this->instance->get( $this->indexable, 'the_page_type' ); + + $this->assertEquals( $this->meta_tags_context_mock, $first ); + $this->assertSame( $first, $second ); + $this->assertArrayHasKey( 0, $this->instance->get_cache() ); + } + /** * Tests clearing the memoization of a specific indexable. * diff --git a/tests/Unit/Memoizers/Presentation_Memoizer_Test.php b/tests/Unit/Memoizers/Presentation_Memoizer_Test.php index f2b05ab649b..fb3e5c8c5f6 100644 --- a/tests/Unit/Memoizers/Presentation_Memoizer_Test.php +++ b/tests/Unit/Memoizers/Presentation_Memoizer_Test.php @@ -152,6 +152,35 @@ public function test_get_without_cache_and_without_presentation() { $this->assertEquals( $this->meta_tags_context_mock->presentation, $this->instance->get( $this->indexable, $this->meta_tags_context_mock, 'the_page_type' ) ); } + /** + * Tests getting the presentation of an indexable whose id is null, ensuring it is + * cached under a stable integer key instead of a null array offset. + * + * @covers ::get + * + * @return void + */ + public function test_get_caches_when_indexable_id_is_null() { + $this->indexable->id = null; + + $this->container + ->expects( 'get' ) + ->once() + ->andReturn( $this->indexable_presentation ); + + $this->indexable_presentation + ->expects( 'of' ) + ->once() + ->andReturn( $this->meta_tags_context_mock->presentation ); + + $first = $this->instance->get( $this->indexable, $this->meta_tags_context_mock, 'the_page_type' ); + $second = $this->instance->get( $this->indexable, $this->meta_tags_context_mock, 'the_page_type' ); + + $this->assertSame( $this->meta_tags_context_mock->presentation, $first ); + $this->assertSame( $first, $second ); + $this->assertArrayHasKey( 0, $this->instance->get_cache() ); + } + /** * Tests clearing the memoization of an indexable when the indexable is given. *