Font Library: Use data or src file to define font collection data (#5…

…7734) * Add ability to use data rather than a file to define font collection * format php * avoid returning data when getting the font collection config * fix rest endpoint get_font_collection --------- Co-authored-by: Matias Benedetto <[email protected]>
WordPress · Jan 11, 2024 · 2762ec4 · 2762ec4
1 parent 940f0fe
commit 2762ec4
Show file tree

Hide file tree

Showing 6 changed files with 156 additions and 27 deletions.
diff --git a/lib/experimental/fonts/font-library/class-wp-font-collection.php b/lib/experimental/fonts/font-library/class-wp-font-collection.php
@@ -35,7 +35,7 @@ class WP_Font_Collection {
 	 * @since 6.5.0
 	 *
 	 * @param array $config Font collection config options.
-	 *                      See {@see wp_register_font_collection()} for the supported fields.
+	 *  See {@see wp_register_font_collection()} for the supported fields.
 	 * @throws Exception If the required parameters are missing.
 	 */
 	public function __construct( $config ) {
@@ -51,8 +51,8 @@ public function __construct( $config ) {
 			throw new Exception( 'Font Collection config name is required as a non-empty string.' );
 		}
 
-		if ( empty( $config['src'] ) || ! is_string( $config['src'] ) ) {
-			throw new Exception( 'Font Collection config "src" option is required as a non-empty string.' );
+		if ( ( empty( $config['src'] ) || ! is_string( $config['src'] ) ) && ( empty( $config['data'] ) ) ) {
+			throw new Exception( 'Font Collection config "src" option OR "data" option is required.' );
 		}
 
 		$this->config = $config;
@@ -63,21 +63,59 @@ public function __construct( $config ) {
 	 *
 	 * @since 6.5.0
 	 *
-	 * @return array An array containing the font collection config.
+	 * @return array {
+	 *     An array of font collection config.
+	 *
+	 *     @type string $id          The font collection's unique ID.
+	 *     @type string $name        The font collection's name.
+	 *     @type string $description The font collection's description.
+	 * }
 	 */
 	public function get_config() {
-		return $this->config;
+		return array(
+			'id'          => $this->config['id'],
+			'name'        => $this->config['name'],
+			'description' => $this->config['description'] ?? '',
+		);
 	}
 
 	/**
-	 * Gets the font collection data.
+	 * Gets the font collection config and data.
+	 *
+	 * This function returns an array containing the font collection's unique ID,
+	 * name, and its data as a PHP array.
 	 *
 	 * @since 6.5.0
 	 *
-	 * @return array|WP_Error An array containing the list of font families in theme.json format on success,
+	 * @return array {
+	 *     An array of font collection config and data.
+	 *
+	 *     @type string $id          The font collection's unique ID.
+	 *     @type string $name        The font collection's name.
+	 *     @type string $description The font collection's description.
+	 *     @type array  $data        The font collection's data as a PHP array.
+	 * }
+	 */
+	public function get_config_and_data() {
+		$config_and_data         = $this->get_config();
+		$config_and_data['data'] = $this->load_data();
+		return $config_and_data;
+	}
+
+	/**
+	 * Loads the font collection data.
+	 *
+	 * @since 6.5.0
+	 *
+	 * @return array|WP_Error An array containing the list of font families in font-collection.json format on success,
 	 *                        else an instance of WP_Error on failure.
 	 */
-	public function get_data() {
+	public function load_data() {
+
+		if ( ! empty( $this->config['data'] ) ) {
+			return $this->config['data'];
+		}
+
 		// If the src is a URL, fetch the data from the URL.
 		if ( str_contains( $this->config['src'], 'http' ) && str_contains( $this->config['src'], '://' ) ) {
 			if ( ! wp_http_validate_url( $this->config['src'] ) ) {
@@ -104,9 +142,6 @@ public function get_data() {
 			}
 		}
 
-		$collection_data         = $this->get_config();
-		$collection_data['data'] = $data;
-		unset( $collection_data['src'] );
-		return $collection_data;
+		return $data;
 	}
 }
diff --git a/lib/experimental/fonts/font-library/class-wp-rest-font-collections-controller.php b/lib/experimental/fonts/font-library/class-wp-rest-font-collections-controller.php
@@ -77,13 +77,16 @@ public function get_font_collection( $request ) {
 			$collection->add_data( array( 'status' => 404 ) );
 			return $collection;
 		}
-		$collection_with_data = $collection->get_data();
+		$config_and_data = $collection->get_config_and_data();
+		$collection_data = $config_and_data['data'];
+
 		// If there was an error getting the collection data, return the error.
-		if ( is_wp_error( $collection_with_data ) ) {
-			$collection_with_data->add_data( array( 'status' => 500 ) );
-			return $collection_with_data;
+		if ( is_wp_error( $collection_data ) ) {
+			$collection_data->add_data( array( 'status' => 500 ) );
+			return $collection_data;
 		}
-		return new WP_REST_Response( $collection_with_data );
+
+		return new WP_REST_Response( $config_and_data );
 	}
 
 	/**
@@ -96,7 +99,7 @@ public function get_font_collection( $request ) {
 	public function get_font_collections() {
 		$collections = array();
 		foreach ( WP_Font_Library::get_font_collections() as $collection ) {
-			$collections[] = $collection->get_config();
+			$collections[] = $collection->get_config_and_data();
 		}
 
 		return new WP_REST_Response( $collections, 200 );

diff --git a/lib/experimental/fonts/font-library/font-library.php b/lib/experimental/fonts/font-library/font-library.php
@@ -50,7 +50,8 @@ function gutenberg_init_font_library_routes() {
 	 *     Font collection associative array of configuration options.
 	 *
 	 *     @type string $id             The font collection's unique ID.
-	 *     @type string $src The font collection's data JSON file.
+	 *     @type string $src            The font collection's data as a JSON file path.
+	 *     @type array  $data           The font collection's data as a PHP array.
 	 * }
 	 * @return WP_Font_Collection|WP_Error A font collection is it was registered
 	 *                                     successfully, else WP_Error.

diff --git a/phpunit/tests/fonts/font-library/wpFontCollection/__construct.php b/phpunit/tests/fonts/font-library/wpFontCollection/__construct.php
@@ -84,7 +84,7 @@ public function data_should_throw_exception() {
 					'name'        => 'My Collection',
 					'description' => 'My collection description',
 				),
-				'Font Collection config "src" option is required as a non-empty string.',
+				'Font Collection config "src" option OR "data" option is required.',
 			),
 
 		);

diff --git a/phpunit/tests/fonts/font-library/wpFontCollection/getConfig.php b/phpunit/tests/fonts/font-library/wpFontCollection/getConfig.php
@@ -0,0 +1,76 @@
+<?php
+/**
+ * Test WP_Font_Collection::get_config().
+ *
+ * @package WordPress
+ * @subpackage Font Library
+ *
+ * @group fonts
+ * @group font-library
+ *
+ * @covers WP_Font_Collection::get_config
+ */
+class Tests_Fonts_WpFontCollection_GetConfig extends WP_UnitTestCase {
+	/**
+	 * @dataProvider data_should_get_config
+	 *
+	 * @param array $config Font collection config options.
+	 * @param array $expected_data Expected data.
+	 */
+	public function test_should_get_config( $config, $expected_data ) {
+		$collection = new WP_Font_Collection( $config );
+		$this->assertSame( $expected_data, $collection->get_config() );
+	}
+
+	/**
+	 * Data provider.
+	 *
+	 * @return array[]
+	 */
+	public function data_should_get_config() {
+		$mock_file = wp_tempnam( 'my-collection-data-' );
+		file_put_contents( $mock_file, '{"this is mock data":true}' );
+
+		return array(
+			'with a file' => array(
+				'config'        => array(
+					'id'          => 'my-collection',
+					'name'        => 'My Collection',
+					'description' => 'My collection description',
+					'src'         => $mock_file,
+				),
+				'expected_data' => array(
+					'id'          => 'my-collection',
+					'name'        => 'My Collection',
+					'description' => 'My collection description',
+				),
+			),
+			'with a url'  => array(
+				'config'        => array(
+					'id'          => 'my-collection-with-url',
+					'name'        => 'My Collection with URL',
+					'description' => 'My collection description',
+					'src'         => 'https://localhost/fonts/mock-font-collection.json',
+				),
+				'expected_data' => array(
+					'id'          => 'my-collection-with-url',
+					'name'        => 'My Collection with URL',
+					'description' => 'My collection description',
+				),
+			),
+			'with data'   => array(
+				'config'        => array(
+					'id'          => 'my-collection',
+					'name'        => 'My Collection',
+					'description' => 'My collection description',
+					'data'        => array( 'this is mock data' => true ),
+				),
+				'expected_data' => array(
+					'id'          => 'my-collection',
+					'name'        => 'My Collection',
+					'description' => 'My collection description',
+				),
+			),
+		);
+	}
+}
diff --git a/...font-library/wpFontCollection/getData.php → ...ary/wpFontCollection/getConfigAndData.php b/...font-library/wpFontCollection/getData.php → ...ary/wpFontCollection/getConfigAndData.php
@@ -1,16 +1,16 @@
 <?php
 /**
- * Test WP_Font_Collection::get_data().
+ * Test WP_Font_Collection::get_config_and_data().
  *
  * @package WordPress
  * @subpackage Font Library
  *
  * @group fonts
  * @group font-library
  *
- * @covers WP_Font_Collection::get_data
+ * @covers WP_Font_Collection::get_config_and_data
  */
-class Tests_Fonts_WpFontCollection_GetData extends WP_UnitTestCase {
+class Tests_Fonts_WpFontCollection_GetConfigAndData extends WP_UnitTestCase {
 
 	public function set_up() {
 		parent::set_up();
@@ -47,22 +47,22 @@ public function mock_request( $preempt, $args, $url ) {
 	}
 
 	/**
-	 * @dataProvider data_should_get_data
+	 * @dataProvider data_should_get_config_and_data
 	 *
 	 * @param array $config Font collection config options.
 	 * @param array $expected_data Expected data.
 	 */
-	public function test_should_get_data( $config, $expected_data ) {
+	public function test_should_get_config_and_data( $config, $expected_data ) {
 		$collection = new WP_Font_Collection( $config );
-		$this->assertSame( $expected_data, $collection->get_data() );
+		$this->assertSame( $expected_data, $collection->get_config_and_data() );
 	}
 
 	/**
 	 * Data provider.
 	 *
 	 * @return array[]
 	 */
-	public function data_should_get_data() {
+	public function data_should_get_config_and_data() {
 		$mock_file = wp_tempnam( 'my-collection-data-' );
 		file_put_contents( $mock_file, '{"this is mock data":true}' );
 
@@ -98,6 +98,20 @@ public function data_should_get_data() {
 					),
 				),
 			),
+			'with data'   => array(
+				'config'        => array(
+					'id'          => 'my-collection',
+					'name'        => 'My Collection',
+					'description' => 'My collection description',
+					'data'        => array( 'this is mock data' => true ),
+				),
+				'expected_data' => array(
+					'id'          => 'my-collection',
+					'name'        => 'My Collection',
+					'description' => 'My collection description',
+					'data'        => array( 'this is mock data' => true ),
+				),
+			),
 		);
 	}
 }