From 24d2bce8a07c364cb389829be6716a906a431f66 Mon Sep 17 00:00:00 2001 From: Robert O'Rourke Date: Fri, 17 Sep 2021 15:45:33 +0100 Subject: [PATCH 1/6] Add MediaResponse object to support pagination headers This is a first pass at adding support for WordPress 5.8 requiring pagination headers for the media library. Related to #38 --- inc/MediaResponse.php | 35 +++++++++++++++++++++++++++++++++++ inc/Provider.php | 16 +++++++++------- 2 files changed, 44 insertions(+), 7 deletions(-) create mode 100644 inc/MediaResponse.php diff --git a/inc/MediaResponse.php b/inc/MediaResponse.php new file mode 100644 index 0000000..39c226f --- /dev/null +++ b/inc/MediaResponse.php @@ -0,0 +1,35 @@ +media_list = $media_list; + + // Set pagination headers. + if ( ! headers_sent() ) { + header( sprintf( 'X-WP-Total: %d', $total ) ); + header( sprintf( 'X-WP-TotalPages: %d', ceil( $total / $per_page ) ) ); + } + } + + public function get_items() : MediaList { + return $this->media_list; + } + +} diff --git a/inc/Provider.php b/inc/Provider.php index 4e7080d..46fe242 100644 --- a/inc/Provider.php +++ b/inc/Provider.php @@ -11,6 +11,7 @@ use Exception; use RangeException; +use WP_HTTP_Requests_Response; use WP_Query; abstract class Provider { @@ -49,9 +50,10 @@ abstract public function get_name() : string; * @type int $monthnum Optional. One or two digit month number if results are filtered by date. * } * @throws Exception Thrown if an unrecoverable error occurs. - * @return MediaList The collection of Media items. Can be an empty collection if there are no matching results. + * @return MediaResponse The response object containing pagination data and list of Media items. + * Can be an empty collection if there are no matching results. */ - abstract protected function request( array $args ) : MediaList; + abstract protected function request( array $args ) : MediaResponse; public function supports_asset_create() : bool { return false; @@ -135,7 +137,8 @@ final public function request_items( array $args ) : MediaList { $args['paged'] = intval( $args['paged'] ?? 1 ); - $items = $this->request( $args ); + $response = $this->request( $args ); + $items = $response->get_items(); $array = $items->toArray(); if ( ! $array ) { @@ -182,9 +185,9 @@ final public function request_items( array $args ) : MediaList { * @param string $url The URL for the request. * @param array $args The arguments to pass to `wp_remote_request()`. * @throws Exception Thrown if there is an error with the request or its response code is not 200. - * @return string The response body. + * @return WP_HTTP_Requests_Response The response object. */ - final public function remote_request( string $url, array $args ) : string { + final public function remote_request( string $url, array $args ) : WP_HTTP_Requests_Response { $response = wp_remote_request( $url, $args @@ -202,7 +205,6 @@ final public function remote_request( string $url, array $args ) : string { $response_code = wp_remote_retrieve_response_code( $response ); $response_message = wp_remote_retrieve_response_message( $response ); - $response_body = wp_remote_retrieve_body( $response ); if ( 200 !== $response_code ) { $message = sprintf( @@ -220,7 +222,7 @@ final public function remote_request( string $url, array $args ) : string { ); } - return $response_body; + return $response['http_response']; } } From 840423f6d65a6ee4c8b5f5d82990d30b753aaffd Mon Sep 17 00:00:00 2001 From: Robert O'Rourke Date: Mon, 20 Sep 2021 14:40:32 +0100 Subject: [PATCH 2/6] Update LocalProvider too --- inc/LocalProvider.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/inc/LocalProvider.php b/inc/LocalProvider.php index bdc3d35..c4dfefb 100644 --- a/inc/LocalProvider.php +++ b/inc/LocalProvider.php @@ -18,11 +18,11 @@ public function get_name() : string { return apply_filters( 'amf/local_provider/name', __( 'Local Media', 'asset-manager-framework' ) ); } - protected function request( array $args ) : MediaList { + protected function request( array $args ) : MediaResponse { // Call the default core attachment query AJAX handler. // This will return JSON and exit before the return statement below. wp_ajax_query_attachments(); - return new MediaList(); + return new MediaResponse( new MediaList(), 1, 1 ); } public function supports_asset_create() : bool { From 3ccfb96dc4fa77777f3b8b0056e1ea8ade057986 Mon Sep 17 00:00:00 2001 From: Robert O'Rourke Date: Tue, 21 Sep 2021 17:29:00 +0100 Subject: [PATCH 3/6] Split out pagination header sending from response object --- inc/LocalProvider.php | 2 +- inc/MediaResponse.php | 42 ++++++++++++++++++++++++++++++++++-------- inc/Provider.php | 18 ++++++++++++++++++ 3 files changed, 53 insertions(+), 9 deletions(-) diff --git a/inc/LocalProvider.php b/inc/LocalProvider.php index c4dfefb..8f41eff 100644 --- a/inc/LocalProvider.php +++ b/inc/LocalProvider.php @@ -22,7 +22,7 @@ protected function request( array $args ) : MediaResponse { // Call the default core attachment query AJAX handler. // This will return JSON and exit before the return statement below. wp_ajax_query_attachments(); - return new MediaResponse( new MediaList(), 1, 1 ); + return new MediaResponse(); } public function supports_asset_create() : bool { diff --git a/inc/MediaResponse.php b/inc/MediaResponse.php index 39c226f..ddf9bb7 100644 --- a/inc/MediaResponse.php +++ b/inc/MediaResponse.php @@ -18,18 +18,44 @@ class MediaResponse { */ private $media_list; - public function __construct( MediaList $media_list, int $total, int $per_page ) { - $this->media_list = $media_list; - - // Set pagination headers. - if ( ! headers_sent() ) { - header( sprintf( 'X-WP-Total: %d', $total ) ); - header( sprintf( 'X-WP-TotalPages: %d', ceil( $total / $per_page ) ) ); - } + /** + * Total available items. + * + * @var int + */ + private $total; + + /** + * Items requested per page. + * + * @var int + */ + private $per_page; + + /** + * Sets up the MediaResponse object. + * + * @param MediaList|null $media_list The list of returned items. + * @param integer $total The total number of results available. + * @param integer $per_page The number of items requested per page, defaults to 40 in + * media modal requests. + */ + public function __construct( ?MediaList $media_list = null, int $total = 0, int $per_page = 40 ) { + $this->media_list = $media_list ?? new MediaList(); + $this->total = $total; + $this->per_page = $per_page; } public function get_items() : MediaList { return $this->media_list; } + public function get_total() : int { + return $this->total; + } + + public function get_total_pages() : int { + return (int) ceil( $this->total / $this->per_page ); + } + } diff --git a/inc/Provider.php b/inc/Provider.php index 46fe242..d54a9ee 100644 --- a/inc/Provider.php +++ b/inc/Provider.php @@ -138,6 +138,10 @@ final public function request_items( array $args ) : MediaList { $args['paged'] = intval( $args['paged'] ?? 1 ); $response = $this->request( $args ); + + // Send headers for media library pagination. + $this->send_pagination_headers( $response ); + $items = $response->get_items(); $array = $items->toArray(); @@ -225,4 +229,18 @@ final public function remote_request( string $url, array $args ) : WP_HTTP_Reque return $response['http_response']; } + /** + * Sends the pagination headers for the media response. + * + * @param MediaResponse $response The media response object. + * @return void + */ + final protected function send_pagination_headers( MediaResponse $response ) { + if ( headers_sent() ) { + return; + } + header( sprintf( 'X-WP-Total: %d', $response->get_total() ) ); + header( sprintf( 'X-WP-TotalPages: %d', $response->get_total_pages() ) ); + } + } From e580e9b2bf1fae4e75a2349d44bba3be33c94f9a Mon Sep 17 00:00:00 2001 From: Robert O'Rourke Date: Wed, 22 Sep 2021 09:52:11 +0100 Subject: [PATCH 4/6] Update readme example --- readme.md | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/readme.md b/readme.md index 551ed19..9869cb9 100644 --- a/readme.md +++ b/readme.md @@ -85,6 +85,7 @@ use AssetManagerFramework\{ ProviderRegistry Provider, MediaList, + MediaResponse, Image }; @@ -98,7 +99,7 @@ class KittenProvider extends Provider { return __( 'Place Kitten' ); } - protected function request( array $args ) : MediaList { + protected function request( array $args ) : MediaResponse { $kittens = [ 500 => 'Boop', 600 => 'Fuzzy', @@ -119,7 +120,11 @@ class KittenProvider extends Provider { $items[] = $item; } - return new MediaList( ...$items ); + return new MediaResponse( + new MediaList( ...$items ), + count( $kittens ), // Total number of available results. + count( $Kittens ) // Number of items requested per page. + ); } } @@ -133,6 +138,8 @@ Try it and your media library will be much improved: ![Kittens](assets/KittenProvider.png) +The `MediaResponse` object takes a `MediaList` along with the total number of available items and the number of items requested per page. This is to ensure pagination in the media library (introduced in WordPress 5.8) works. + You also have access to provider instances during registration via the `amf/provider` filter, so you could use it to decorate providers: ```php From af07f2067d1babe7452732bc1dfb34a53d6d11e0 Mon Sep 17 00:00:00 2001 From: Robert O'Rourke Date: Wed, 22 Sep 2021 12:58:09 +0100 Subject: [PATCH 5/6] Apply suggestions from code review Co-authored-by: Thorsten Frommen --- inc/MediaResponse.php | 8 ++++---- readme.md | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/inc/MediaResponse.php b/inc/MediaResponse.php index ddf9bb7..ca20004 100644 --- a/inc/MediaResponse.php +++ b/inc/MediaResponse.php @@ -35,10 +35,10 @@ class MediaResponse { /** * Sets up the MediaResponse object. * - * @param MediaList|null $media_list The list of returned items. - * @param integer $total The total number of results available. - * @param integer $per_page The number of items requested per page, defaults to 40 in - * media modal requests. + * @param ?MediaList $media_list The list of returned items. + * @param int $total The total number of results available. + * @param int $per_page The number of items requested per page, defaults to 40 in media + * modal requests. */ public function __construct( ?MediaList $media_list = null, int $total = 0, int $per_page = 40 ) { $this->media_list = $media_list ?? new MediaList(); diff --git a/readme.md b/readme.md index 9869cb9..c0a5d92 100644 --- a/readme.md +++ b/readme.md @@ -123,7 +123,7 @@ class KittenProvider extends Provider { return new MediaResponse( new MediaList( ...$items ), count( $kittens ), // Total number of available results. - count( $Kittens ) // Number of items requested per page. + count( $kittens ) // Number of items requested per page. ); } From a845dfa317041d358671c59c34be477f9ec122ac Mon Sep 17 00:00:00 2001 From: Robert O'Rourke Date: Wed, 22 Sep 2021 14:18:25 +0100 Subject: [PATCH 6/6] Throw exception with bad per page value --- inc/MediaResponse.php | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/inc/MediaResponse.php b/inc/MediaResponse.php index ca20004..a808c6c 100644 --- a/inc/MediaResponse.php +++ b/inc/MediaResponse.php @@ -9,6 +9,8 @@ namespace AssetManagerFramework; +use RangeException; + class MediaResponse { /** @@ -39,8 +41,20 @@ class MediaResponse { * @param int $total The total number of results available. * @param int $per_page The number of items requested per page, defaults to 40 in media * modal requests. + * @throws RangeException If $per_page value is less than or equal to 0. */ public function __construct( ?MediaList $media_list = null, int $total = 0, int $per_page = 40 ) { + // Fail early if provided per page value is not valid. + if ( $per_page <= 0 ) { + throw new RangeException( + sprintf( + /* translators: %d: Items per page value in error message */ + __( 'Media items per page value must be greater than zero, %d given.', 'asset-manager-framework' ), + $per_page + ) + ); + } + $this->media_list = $media_list ?? new MediaList(); $this->total = $total; $this->per_page = $per_page;