Allow empty arrays to be cached (#31077)

* Allow empty arrays to be cached

* Fix second check

* Add tests for attribute function changes

* Use InvokedRecorder to explicitly assert invocation count.

Provides more friendly failure messages and self documenting code.

* Code format fixes

plugins/woocommerce/includes/abstracts/abstract-wc-data.php

@@ -590,14 +590,14 @@ abstract class WC_Data {
 		if ( ! $force_read ) {
 			if ( ! empty( $this->cache_group ) ) {
 				$cached_meta  = wp_cache_get( $cache_key, $this->cache_group );
-				$cache_loaded = ! empty( $cached_meta );
+				$cache_loaded = is_array( $cached_meta );
 			}
 		}
 
 		// We filter the raw meta data again when loading from cache, in case we cached in an earlier version where filter conditions were different.
 		$raw_meta_data = $cache_loaded ? $this->data_store->filter_raw_meta_data( $this, $cached_meta ) : $this->data_store->read_meta( $this );
 
-		if ( $raw_meta_data ) {
+		if ( is_array( $raw_meta_data ) ) {
 			foreach ( $raw_meta_data as $meta ) {
 				$this->meta_data[] = new WC_Meta_Data(
 					array(

plugins/woocommerce/includes/wc-attribute-functions.php

@@ -95,7 +95,7 @@ function wc_get_attribute_taxonomy_ids() {
 	$cache_key   = $prefix . 'ids';
 	$cache_value = wp_cache_get( $cache_key, 'woocommerce-attributes' );
 
-	if ( $cache_value ) {
+	if ( false !== $cache_value ) {
 		return $cache_value;
 	}
 
@@ -117,7 +117,7 @@ function wc_get_attribute_taxonomy_labels() {
 	$cache_key   = $prefix . 'labels';
 	$cache_value = wp_cache_get( $cache_key, 'woocommerce-attributes' );
 
-	if ( $cache_value ) {
+	if ( false !== $cache_value ) {
 		return $cache_value;
 	}
 
@@ -722,7 +722,7 @@ function wc_attribute_taxonomy_slug( $attribute_name ) {
 	$cache_key   = $prefix . 'slug-' . $attribute_name;
 	$cache_value = wp_cache_get( $cache_key, 'woocommerce-attributes' );
 
-	if ( $cache_value ) {
+	if ( false !== $cache_value ) {
 		return $cache_value;
 	}
 

plugins/woocommerce/tests/php/includes/abstracts/class-wc-abstract-data-test.php

@@ -0,0 +1,75 @@
+<?php
+/**
+ * Class WC_Data file.
+ *
+ * @package WooCommerce\Tests\Abstracts
+ */
+
+/**
+ * Class WC_Data.
+ */
+class WC_Data_Test extends WC_Unit_Test_Case {
+
+	/**
+	 * Test that create is called on data store when new object is saved.
+	 */
+	public function test_data_store_called_on_save() {
+		$data_store = $this->getMockBuilder( WC_Object_Data_Store_Interface::class )->getMock();
+		$data_store->expects( $this->once() )
+			->method( 'create' )
+			->with(
+			   $this->isInstanceOf( WC_Data::class )
+			);
+		$data_store->expects( $this->once() )
+			->method( 'update' )
+			->with(
+			   $this->isInstanceOf( WC_Data::class )
+			);
+		$data_store->expects( $this->once() )
+			->method( 'delete' )
+			->with(
+			   $this->isInstanceOf( WC_Data::class )
+			);
+
+		$data_object = new class( $data_store ) extends WC_Data {
+			public function __construct( $data_store ) {
+				$this->data_store = $data_store;
+			}
+		};
+		$data_object->save();
+		$data_object->set_id( 1 );
+		$data_object->save();
+		$data_object->delete();
+	}
+
+	/**
+	 * Test that cache is used when reading meta data.
+	 */
+	public function test_meta_data_cache() {
+		$raw_meta_data = [];
+		$data_store = $this->getMockBuilder( WC_Data_Store_WP::class )->getMock();
+		$data_store->expects( $this->once() )
+			->method( 'filter_raw_meta_data' )
+			->with(
+			   $this->isInstanceOf( WC_Data::class ),
+			   $raw_meta_data
+			);
+		$data_store->expects( $this->once() )
+			->method( 'read_meta' )
+			->with(
+			   $this->isInstanceOf( WC_Data::class )
+			)
+			->willReturn( $raw_meta_data );
+
+		$data_object = new class( $data_store ) extends WC_Data {
+			protected $cache_group = 'object_name';
+			public function __construct( $data_store ) {
+				$this->id = 1;
+				$this->data_store = $data_store;
+			}
+		};
+		$meta_data = $data_object->get_meta_data();
+		$this->assertEquals( [], $meta_data );
+		$data_object->read_meta_data();
+	}
+}

plugins/woocommerce/tests/php/includes/wc-attribute-functions-test.php

@@ -0,0 +1,125 @@
+<?php
+/**
+ * Attribute functions tests
+ *
+ * @package WooCommerce\Tests\Functions.
+ */
+
+use \PHPUnit\Framework\MockObject\Matcher\InvokedRecorder;
+
+/**
+ * Class WC_Formatting_Functions_Test
+ */
+class WC_Attribute_Functions_Test extends \WC_Unit_Test_Case {
+
+	/**
+	 * Mock object to spy on filter.
+	 *
+	 * @var InvokedRecorder
+	 */
+	protected $filter_recorder;
+
+	/**
+	 * Set up.
+	 */
+	public function setUp() {
+		parent::setUp();
+
+		// Tests will use this to verify the correct call count.
+		$this->filter_recorder = $this->any();
+
+		$filter_mock = $this->getMockBuilder( stdClass::class )
+			->setMethods( [ '__invoke' ] )
+			->getMock();
+		$filter_mock->expects( $this->filter_recorder )
+			->method( '__invoke' )
+			->will( $this->returnArgument( 0 ) );
+
+		add_filter( 'woocommerce_attribute_taxonomies', $filter_mock );
+		add_filter( 'sanitize_taxonomy_name', $filter_mock );
+	}
+
+	/**
+	 * Tear down.
+	 */
+	public function tearDown() {
+		remove_all_filters( 'woocommerce_attribute_taxonomies' );
+		remove_all_filters( 'sanitize_taxonomy_name' );
+
+		parent::tearDown();
+	}
+
+	/**
+	 * Test wc_get_attribute_taxonomy_ids() function.
+	 * Even empty arrays should be cached.
+	 */
+	public function test_wc_get_attribute_taxonomy_ids() {
+		$ids = wc_get_attribute_taxonomy_ids();
+		$this->assertEquals( [], $ids );
+		$this->assertEquals(
+			1,
+			$this->filter_recorder->getInvocationCount(),
+			'Filter `woocommerce_attribute_taxonomies` should have been triggered once after fetching all attribute taxonomies.'
+		);
+		$ids = wc_get_attribute_taxonomy_ids();
+		$this->assertEquals( [], $ids );
+		$this->assertEquals(
+			1,
+			$this->filter_recorder->getInvocationCount(),
+			'Filter `woocommerce_attribute_taxonomies` should not be triggered a second time because the results should be loaded from the cache.'
+		);
+	}
+
+	/**
+	 * Test wc_get_attribute_taxonomy_labels() function.
+	 * Even empty arrays should be cached.
+	 */
+	public function test_wc_get_attribute_taxonomy_labels() {
+		$labels = wc_get_attribute_taxonomy_labels();
+		$this->assertEquals( [], $labels );
+		$this->assertEquals(
+			1,
+			$this->filter_recorder->getInvocationCount(),
+			'Filter `woocommerce_attribute_taxonomies` should have been triggered once after fetching all attribute taxonomies.'
+		);
+		$labels = wc_get_attribute_taxonomy_labels();
+		$this->assertEquals( [], $labels );
+		$this->assertEquals(
+			1,
+			$this->filter_recorder->getInvocationCount(),
+			'Filter `woocommerce_attribute_taxonomies` should not be triggered a second time because the results should be loaded from the cache.'
+		);
+	}
+
+	/**
+	 * Test wc_attribute_taxonomy_slug() function.
+	 * Even empty strings should be cached.
+	 *
+	 * @dataProvider get_attribute_names_and_slugs
+	 */
+	public function test_wc_get_attribute_taxonomy_slug( $name, $expected_slug ) {
+		$slug = wc_attribute_taxonomy_slug( $name );
+		$this->assertEquals( $expected_slug, $slug );
+		$this->assertEquals(
+			1,
+			$this->filter_recorder->getInvocationCount(),
+			'Filter `sanitize_taxonomy_name` should have been triggered once.'
+		);
+		$slug = wc_attribute_taxonomy_slug( $name );
+		$this->assertEquals( $expected_slug, $slug );
+		$this->assertEquals(
+			1,
+			$this->filter_recorder->getInvocationCount(),
+			'Filter `sanitize_taxonomy_name` should not be triggered a second time because the slug should be loaded from the cache.'
+		);
+	}
+
+	public function get_attribute_names_and_slugs() {
+		return [
+			[ 'Dash Me', 'dash-me' ],
+			[ '', '' ],
+			[ 'pa_SubStr', 'substr' ],
+			[ 'ĂnîC°Dę', 'anicde' ],
+		];
+	}
+}