From 7f2ea5cc2fc2e3255a397c1a78116eaf597e0a88 Mon Sep 17 00:00:00 2001 From: Mike Jolley Date: Fri, 14 Jun 2019 13:43:29 +0100 Subject: [PATCH] Fix tests for batches --- phpunit.xml | 3 + .../Version4/AbstractController.php | 134 ++++++++++-------- .../Version4/AbstractObjectsController.php | 1 - .../Version4/AbstractTermsContoller.php | 15 +- src/Controllers/Version4/BatchTrait.php | 98 +++++++------ .../Version4/CustomerDownloads.php | 21 +-- src/Controllers/Version4/Customers.php | 15 +- .../Version4/ProductAttributes.php | 15 +- src/Controllers/Version4/ProductReviews.php | 15 +- .../Version4/ProductVariations.php | 46 +++--- src/Controllers/Version4/Settings.php | 14 +- src/Controllers/Version4/SettingsOptions.php | 34 ++--- src/Controllers/Version4/Taxes.php | 15 +- src/Controllers/Version4/Webhooks.php | 15 +- unit-tests/Tests/Version4/Data.php | 30 +++- 15 files changed, 215 insertions(+), 256 deletions(-) diff --git a/phpunit.xml b/phpunit.xml index 9119520176d..8f9df0b9fb5 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -13,5 +13,8 @@ ./unit-tests/Tests + + ./unit-tests/Tests/Version4 + diff --git a/src/Controllers/Version4/AbstractController.php b/src/Controllers/Version4/AbstractController.php index a9b6b06b7ea..d10479673e2 100644 --- a/src/Controllers/Version4/AbstractController.php +++ b/src/Controllers/Version4/AbstractController.php @@ -24,6 +24,7 @@ use \WP_REST_Controller; * @version 2.6.0 */ abstract class AbstractController extends WP_REST_Controller { + use BatchTrait; /** * Endpoint namespace. @@ -41,76 +42,97 @@ abstract class AbstractController extends WP_REST_Controller { /** * Register route for items requests. + * + * @param array $methods Supported methods. read, create. */ - protected function register_items_route() { + protected function register_items_route( $methods = [ 'read', 'create' ] ) { + $routes = []; + $routes['schema'] = [ $this, 'get_public_item_schema' ]; + + if ( in_array( 'read', $methods, true ) ) { + $routes[] = array( + 'methods' => \WP_REST_Server::READABLE, + 'callback' => array( $this, 'get_items' ), + 'permission_callback' => array( $this, 'get_items_permissions_check' ), + 'args' => $this->get_collection_params(), + ); + } + + if ( in_array( 'create', $methods, true ) ) { + $routes[] = array( + 'methods' => \WP_REST_Server::CREATABLE, + 'callback' => array( $this, 'create_item' ), + 'permission_callback' => array( $this, 'create_item_permissions_check' ), + 'args' => $this->get_endpoint_args_for_item_schema( \WP_REST_Server::CREATABLE ), + ); + } + register_rest_route( $this->namespace, '/' . $this->rest_base, - array( - array( - 'methods' => \WP_REST_Server::READABLE, - 'callback' => array( $this, 'get_items' ), - 'permission_callback' => array( $this, 'get_items_permissions_check' ), - 'args' => $this->get_collection_params(), - ), - array( - 'methods' => \WP_REST_Server::CREATABLE, - 'callback' => array( $this, 'create_item' ), - 'permission_callback' => array( $this, 'create_item_permissions_check' ), - 'args' => $this->get_endpoint_args_for_item_schema( \WP_REST_Server::CREATABLE ), - ), - 'schema' => array( $this, 'get_public_item_schema' ), - ), + $routes, true ); } /** * Register route for item create/get/delete/update requests. + * + * @param array $methods Supported methods. read, create. */ - protected function register_item_route() { + protected function register_item_route( $methods = [ 'read', 'edit', 'delete' ] ) { + $routes = []; + $routes['schema'] = [ $this, 'get_public_item_schema' ]; + $routes['args'] = [ + 'id' => [ + 'description' => __( 'Unique identifier for the resource.', 'woocommerce' ), + 'type' => 'integer', + ], + ]; + + if ( in_array( 'read', $methods, true ) ) { + $routes[] = array( + 'methods' => \WP_REST_Server::READABLE, + 'callback' => array( $this, 'get_item' ), + 'permission_callback' => array( $this, 'get_item_permissions_check' ), + 'args' => array( + 'context' => $this->get_context_param( + array( + 'default' => 'view', + ) + ), + ), + ); + } + + if ( in_array( 'edit', $methods, true ) ) { + $routes[] = array( + 'methods' => \WP_REST_Server::EDITABLE, + 'callback' => array( $this, 'update_item' ), + 'permission_callback' => array( $this, 'update_item_permissions_check' ), + 'args' => $this->get_endpoint_args_for_item_schema( \WP_REST_Server::EDITABLE ), + ); + } + + if ( in_array( 'delete', $methods, true ) ) { + $routes[] = array( + 'methods' => \WP_REST_Server::DELETABLE, + 'callback' => array( $this, 'delete_item' ), + 'permission_callback' => array( $this, 'delete_item_permissions_check' ), + 'args' => array( + 'force' => array( + 'default' => false, + 'description' => __( 'Whether to bypass trash and force deletion.', 'woocommerce' ), + 'type' => 'boolean', + ), + ), + ); + } + register_rest_route( $this->namespace, '/' . $this->rest_base . '/(?P[\d]+)', - array( - 'args' => array( - 'id' => array( - 'description' => __( 'Unique identifier for the resource.', 'woocommerce' ), - 'type' => 'integer', - ), - ), - array( - 'methods' => \WP_REST_Server::READABLE, - 'callback' => array( $this, 'get_item' ), - 'permission_callback' => array( $this, 'get_item_permissions_check' ), - 'args' => array( - 'context' => $this->get_context_param( - array( - 'default' => 'view', - ) - ), - ), - ), - array( - 'methods' => \WP_REST_Server::EDITABLE, - 'callback' => array( $this, 'update_item' ), - 'permission_callback' => array( $this, 'update_item_permissions_check' ), - 'args' => $this->get_endpoint_args_for_item_schema( \WP_REST_Server::EDITABLE ), - ), - array( - 'methods' => \WP_REST_Server::DELETABLE, - 'callback' => array( $this, 'delete_item' ), - 'permission_callback' => array( $this, 'delete_item_permissions_check' ), - 'args' => array( - 'force' => array( - 'default' => false, - 'description' => __( 'Whether to bypass trash and force deletion.', 'woocommerce' ), - 'type' => 'boolean', - ), - ), - ), - 'schema' => array( $this, 'get_public_item_schema' ), - ), + $routes, true ); } diff --git a/src/Controllers/Version4/AbstractObjectsController.php b/src/Controllers/Version4/AbstractObjectsController.php index efca889a057..f3eb35c278b 100644 --- a/src/Controllers/Version4/AbstractObjectsController.php +++ b/src/Controllers/Version4/AbstractObjectsController.php @@ -13,7 +13,6 @@ defined( 'ABSPATH' ) || exit; * CRUD Object Controller. */ abstract class AbstractObjectsController extends AbstractController { - use BatchTrait; /** * If object is hierarchical. diff --git a/src/Controllers/Version4/AbstractTermsContoller.php b/src/Controllers/Version4/AbstractTermsContoller.php index fea0f2ce815..956d83c9474 100644 --- a/src/Controllers/Version4/AbstractTermsContoller.php +++ b/src/Controllers/Version4/AbstractTermsContoller.php @@ -117,20 +117,7 @@ abstract class AbstractTermsContoller extends AbstractController { true ); - register_rest_route( - $this->namespace, - '/' . $this->rest_base . '/batch', - array( - array( - 'methods' => \WP_REST_Server::EDITABLE, - 'callback' => array( $this, 'batch_items' ), - 'permission_callback' => array( $this, 'batch_items_permissions_check' ), - 'args' => $this->get_endpoint_args_for_item_schema( \WP_REST_Server::EDITABLE ), - ), - 'schema' => array( $this, 'get_public_batch_schema' ), - ), - true - ); + $this->register_batch_route(); } /** diff --git a/src/Controllers/Version4/BatchTrait.php b/src/Controllers/Version4/BatchTrait.php index 44125054470..40a50491d23 100644 --- a/src/Controllers/Version4/BatchTrait.php +++ b/src/Controllers/Version4/BatchTrait.php @@ -19,23 +19,37 @@ trait BatchTrait { * @return array Of \WP_Error or \WP_REST_Response. */ public function batch_items( $request ) { - $items = array_filter( $request->get_params() ); + $items = $request->get_params(); $limit = $this->check_batch_limit( $items ); if ( is_wp_error( $limit ) ) { return $limit; } - $response = []; $batches = [ 'create', 'update', 'delete' ]; + $response = []; + foreach ( $batches as $batch ) { - if ( ! isset( $items[ $batch ] ) ) { - continue; - } - $items[ $batch ] = wp_parse_id_list( $items[ $batch ] ); - $response[ $batch ] = $this->{"batch_$batch"}( $items[ $batch ] ); + $response[ $batch ] = $this->{"batch_$batch"}( $this->get_batch_of_items_from_request( $request, $batch ) ); } - return $response; + return array_filter( $response ); + } + + /** + * Get batch of items from requst. + * + * @param \WP_REST_Request $request Full details about the request. + * @param string $batch_type Batch type; one of create, update, delete. + * @return array + */ + protected function get_batch_of_items_from_request( $request, $batch_type ) { + $params = $request->get_params(); + + if ( ! isset( $params[ $batch_type ] ) ) { + return array(); + } + + return array_filter( $params[ $batch_type ] ); } /** @@ -44,7 +58,9 @@ trait BatchTrait { * @param \WP_REST_Request $request Full details about the request. * @return boolean|\WP_Error */ - abstract public function batch_items_permissions_check( $request ); + public function batch_items_permissions_check( $request ) { + return update_items_permissions_check( $request ); + } /** * Register route for batch requests. @@ -123,16 +139,15 @@ trait BatchTrait { * @return bool|\WP_Error */ protected function check_batch_limit( $items ) { - $limit = apply_filters( 'woocommerce_rest_batch_items_limit', 100, $this->get_normalized_rest_base() ); - $total = 0; + $limit = apply_filters( 'woocommerce_rest_batch_items_limit', 100, $this->get_normalized_rest_base() ); + $total = 0; + $batches = [ 'create', 'update', 'delete' ]; - $batches = [ 'create', 'update', 'delete' ]; foreach ( $batches as $batch ) { if ( ! isset( $items[ $batch ] ) ) { continue; } - $items[ $batch ] = wp_parse_id_list( $items[ $batch ] ); - $total = $total + count( $items[ $batch ] ); + $total = $total + count( $items[ $batch ] ); } if ( $total > $limit ) { @@ -162,40 +177,42 @@ trait BatchTrait { /** * Batch create items. * - * @param array $items Array of item ids. + * @param array $items Array of items. * @return array Response data. */ protected function batch_create( $items ) { - $response = []; + $batch_response = []; - foreach ( $items as $id ) { - $item = new \WP_REST_Request( 'POST' ); - $item->set_default_params( $this->get_default_params() ); - $item->set_body_params( $item ); - $item_response = $this->create_item( $item ); - $response[] = $this->format_response( $id, $item_response ); + foreach ( $items as $item ) { + $request = new \WP_REST_Request( 'POST' ); + $request->set_default_params( $this->get_default_params() ); + $request->set_body_params( $item ); + + $response = $this->create_item( $request ); + $batch_response[] = $this->format_response( 0, $response ); } - return $response; + return $batch_response; } /** * Batch update items. * - * @param array $items Array of item ids. + * @param array $items Array of items. * @return array Response data. */ protected function batch_update( $items ) { - $response = []; + $batch_response = []; - foreach ( $items as $id ) { - $item = new \WP_REST_Request( 'PUT' ); - $item->set_body_params( $item ); - $item_response = $this->update_item( $item ); - $response[] = $this->format_response( $id, $item_response ); + foreach ( $items as $item ) { + $request = new \WP_REST_Request( 'PUT' ); + $request->set_body_params( $item ); + + $response = $this->update_item( $request ); + $batch_response[] = $this->format_response( $item['id'], $response ); } - return $response; + return $batch_response; } /** @@ -205,21 +222,22 @@ trait BatchTrait { * @return array Response data. */ protected function batch_delete( $items ) { - $response = []; + $batch_response = []; + $items = wp_parse_id_list( $items ); foreach ( $items as $id ) { - $item = new \WP_REST_Request( 'DELETE' ); - $item->set_query_params( + $request = new \WP_REST_Request( 'DELETE' ); + $request->set_query_params( [ 'id' => $id, 'force' => true, ] ); - $item_response = $this->delete_item( $item ); - $response[] = $this->format_response( $id, $item_response ); + $response = $this->delete_item( $request ); + $batch_response[] = $this->format_response( $id, $response ); } - return $response; + return $batch_response; } /** @@ -237,12 +255,12 @@ trait BatchTrait { */ global $wp_rest_server; - if ( is_wp_error( $response ) ) { - return $this->format_error_response( $response ); + if ( ! is_wp_error( $response ) ) { + return $wp_rest_server->response_to_data( $response, '' ); } else { return array( 'id' => $id, - 'error' => $wp_rest_server->response_to_data( $response, '' ), + 'error' => $this->format_error_response( $response ), ); } } diff --git a/src/Controllers/Version4/CustomerDownloads.php b/src/Controllers/Version4/CustomerDownloads.php index e03f6ec175d..588da40d3b5 100644 --- a/src/Controllers/Version4/CustomerDownloads.php +++ b/src/Controllers/Version4/CustomerDownloads.php @@ -27,26 +27,7 @@ class CustomerDownloads extends AbstractController { * Register the routes for customers. */ public function register_routes() { - register_rest_route( - $this->namespace, - '/' . $this->rest_base, - array( - 'args' => array( - 'customer_id' => array( - 'description' => __( 'Unique identifier for the resource.', 'woocommerce' ), - 'type' => 'integer', - ), - ), - array( - 'methods' => \WP_REST_Server::READABLE, - 'callback' => array( $this, 'get_items' ), - 'permission_callback' => array( $this, 'get_items_permissions_check' ), - 'args' => $this->get_collection_params(), - ), - 'schema' => array( $this, 'get_public_item_schema' ), - ), - true - ); + $this->register_items_route( [ 'read' ] ); } /** diff --git a/src/Controllers/Version4/Customers.php b/src/Controllers/Version4/Customers.php index 4e0234b5138..41d6a07f4a8 100644 --- a/src/Controllers/Version4/Customers.php +++ b/src/Controllers/Version4/Customers.php @@ -115,20 +115,7 @@ class Customers extends AbstractController { true ); - register_rest_route( - $this->namespace, - '/' . $this->rest_base . '/batch', - array( - array( - 'methods' => \WP_REST_Server::EDITABLE, - 'callback' => array( $this, 'batch_items' ), - 'permission_callback' => array( $this, 'batch_items_permissions_check' ), - 'args' => $this->get_endpoint_args_for_item_schema( \WP_REST_Server::EDITABLE ), - ), - 'schema' => array( $this, 'get_public_batch_schema' ), - ), - true - ); + $this->register_batch_route(); } /** diff --git a/src/Controllers/Version4/ProductAttributes.php b/src/Controllers/Version4/ProductAttributes.php index 70f955ea928..53c9ef2aada 100644 --- a/src/Controllers/Version4/ProductAttributes.php +++ b/src/Controllers/Version4/ProductAttributes.php @@ -105,20 +105,7 @@ class ProductAttributes extends AbstractController { true ); - register_rest_route( - $this->namespace, - '/' . $this->rest_base . '/batch', - array( - array( - 'methods' => \WP_REST_Server::EDITABLE, - 'callback' => array( $this, 'batch_items' ), - 'permission_callback' => array( $this, 'batch_items_permissions_check' ), - 'args' => $this->get_endpoint_args_for_item_schema( \WP_REST_Server::EDITABLE ), - ), - 'schema' => array( $this, 'get_public_batch_schema' ), - ), - true - ); + $this->register_batch_route(); } /** diff --git a/src/Controllers/Version4/ProductReviews.php b/src/Controllers/Version4/ProductReviews.php index 1a1b87be436..f6eb8600bbb 100644 --- a/src/Controllers/Version4/ProductReviews.php +++ b/src/Controllers/Version4/ProductReviews.php @@ -113,20 +113,7 @@ class ProductReviews extends AbstractController { true ); - register_rest_route( - $this->namespace, - '/' . $this->rest_base . '/batch', - array( - array( - 'methods' => \WP_REST_Server::EDITABLE, - 'callback' => array( $this, 'batch_items' ), - 'permission_callback' => array( $this, 'batch_items_permissions_check' ), - 'args' => $this->get_endpoint_args_for_item_schema( \WP_REST_Server::EDITABLE ), - ), - 'schema' => array( $this, 'get_public_batch_schema' ), - ), - true - ); + $this->register_batch_route(); } /** diff --git a/src/Controllers/Version4/ProductVariations.php b/src/Controllers/Version4/ProductVariations.php index d0da8692edc..9d81af75af3 100644 --- a/src/Controllers/Version4/ProductVariations.php +++ b/src/Controllers/Version4/ProductVariations.php @@ -725,37 +725,35 @@ class ProductVariations extends Products { } /** - * Bulk create, update and delete items. + * Get batch of items from requst. * - * @since 3.0.0 * @param \WP_REST_Request $request Full details about the request. - * @return array Of \WP_Error or \WP_REST_Response. + * @param string $batch_type Batch type; one of create, update, delete. + * @return array */ - public function batch_items( $request ) { - $items = array_filter( $request->get_params() ); - $params = $request->get_url_params(); - $product_id = $params['product_id']; - $body_params = array(); + protected function get_batch_of_items_from_request( $request, $batch_type ) { + $params = $request->get_params(); + $url_params = $request->get_url_params(); + $product_id = $url_params['product_id']; - foreach ( array( 'update', 'create', 'delete' ) as $batch_type ) { - if ( ! empty( $items[ $batch_type ] ) ) { - $injected_items = array(); - foreach ( $items[ $batch_type ] as $item ) { - $injected_items[] = is_array( $item ) ? array_merge( - array( - 'product_id' => $product_id, - ), - $item - ) : $item; - } - $body_params[ $batch_type ] = $injected_items; + if ( ! isset( $params[ $batch_type ] ) ) { + return array(); + } + + $items = array_filter( $params[ $batch_type ] ); + + if ( 'update' === $batch_type || 'create' === $batch_type ) { + foreach ( $items as $key => $item ) { + $items[ $key ] = array_merge( + array( + 'product_id' => $product_id, + ), + $item + ); } } - $request = new \WP_REST_Request( $request->get_method() ); - $request->set_body_params( $body_params ); - - return parent::batch_items( $request ); + return array_filter( $items ); } /** diff --git a/src/Controllers/Version4/Settings.php b/src/Controllers/Version4/Settings.php index 0d29ed833ff..8110ad64cae 100644 --- a/src/Controllers/Version4/Settings.php +++ b/src/Controllers/Version4/Settings.php @@ -42,19 +42,7 @@ class Settings extends AbstractController { ), true ); - register_rest_route( - $this->namespace, - '/' . $this->rest_base . '/batch', - array( - array( - 'methods' => \WP_REST_Server::EDITABLE, - 'callback' => array( $this, 'batch_items' ), - 'permission_callback' => array( $this, 'update_items_permissions_check' ), - ), - 'schema' => array( $this, 'get_public_batch_schema' ), - ), - true - ); + $this->register_batch_route(); } /** diff --git a/src/Controllers/Version4/SettingsOptions.php b/src/Controllers/Version4/SettingsOptions.php index af2e525efe5..6fba4a8ab1e 100644 --- a/src/Controllers/Version4/SettingsOptions.php +++ b/src/Controllers/Version4/SettingsOptions.php @@ -274,30 +274,32 @@ class SettingsOptions extends AbstractController { } /** - * Bulk create, update and delete items. + * Get batch of items from requst. * - * @since 3.0.0 * @param \WP_REST_Request $request Full details about the request. - * @return array Of \WP_Error or \WP_REST_Response. + * @param string $batch_type Batch type; one of create, update, delete. + * @return array */ - public function batch_items( $request ) { - // Get the request params. - $items = array_filter( $request->get_params() ); + protected function get_batch_of_items_from_request( $request, $batch_type ) { + $params = $request->get_params(); - /* + if ( ! isset( $params[ $batch_type ] ) ) { + return array(); + } + + /** * Since our batch settings update is group-specific and matches based on the route, * we inject the URL parameters (containing group) into the batch items */ - if ( ! empty( $items['update'] ) ) { - $to_update = array(); - foreach ( $items['update'] as $item ) { - $to_update[] = array_merge( $request->get_url_params(), $item ); + $items = array_filter( $params[ $batch_type ] ); + + if ( 'update' === $batch_type ) { + foreach ( $items as $key => $item ) { + $items[ $key ] = array_merge( $request->get_url_params(), $item ); } - $request = new \WP_REST_Request( $request->get_method() ); - $request->set_body_params( array( 'update' => $to_update ) ); } - return parent::batch_items( $request ); + return array_filter( $items ); } /** @@ -345,8 +347,7 @@ class SettingsOptions extends AbstractController { /** * Prepare a single setting object for response. * - * @since 3.0.0 - * @param object $item Setting object. + * @param object $item Setting object. * @param \WP_REST_Request $request Request object. * @return \WP_REST_Response $response Response data. */ @@ -363,7 +364,6 @@ class SettingsOptions extends AbstractController { /** * Prepare links for the request. * - * @since 3.0.0 * @param string $setting_id Setting ID. * @param string $group_id Group ID. * @return array Links for the given setting. diff --git a/src/Controllers/Version4/Taxes.php b/src/Controllers/Version4/Taxes.php index 5f974dd5dbb..595cede149e 100644 --- a/src/Controllers/Version4/Taxes.php +++ b/src/Controllers/Version4/Taxes.php @@ -89,20 +89,7 @@ class Taxes extends AbstractController { true ); - register_rest_route( - $this->namespace, - '/' . $this->rest_base . '/batch', - array( - array( - 'methods' => \WP_REST_Server::EDITABLE, - 'callback' => array( $this, 'batch_items' ), - 'permission_callback' => array( $this, 'batch_items_permissions_check' ), - 'args' => $this->get_endpoint_args_for_item_schema( \WP_REST_Server::EDITABLE ), - ), - 'schema' => array( $this, 'get_public_batch_schema' ), - ), - true - ); + $this->register_batch_route(); } /** diff --git a/src/Controllers/Version4/Webhooks.php b/src/Controllers/Version4/Webhooks.php index ececa580c0a..77886eb0703 100644 --- a/src/Controllers/Version4/Webhooks.php +++ b/src/Controllers/Version4/Webhooks.php @@ -110,20 +110,7 @@ class Webhooks extends AbstractController { true ); - register_rest_route( - $this->namespace, - '/' . $this->rest_base . '/batch', - array( - array( - 'methods' => \WP_REST_Server::EDITABLE, - 'callback' => array( $this, 'batch_items' ), - 'permission_callback' => array( $this, 'batch_items_permissions_check' ), - 'args' => $this->get_endpoint_args_for_item_schema( \WP_REST_Server::EDITABLE ), - ), - 'schema' => array( $this, 'get_public_batch_schema' ), - ), - true - ); + $this->register_batch_route(); } /** diff --git a/unit-tests/Tests/Version4/Data.php b/unit-tests/Tests/Version4/Data.php index f57849732a3..15b14b95ef7 100644 --- a/unit-tests/Tests/Version4/Data.php +++ b/unit-tests/Tests/Version4/Data.php @@ -12,7 +12,7 @@ defined( 'ABSPATH' ) || exit; /** * WC Tests API Data */ -class Data { +class Data extends \WC_REST_Unit_Test_Case { /** * Endpoints. @@ -21,6 +21,34 @@ class Data { */ protected $endpoint = '/wc/v4/data'; + /** + * User variable. + * + * @var WP_User + */ + protected static $user; + + /** + * Setup once before running tests. + * + * @param object $factory Factory object. + */ + public static function wpSetUpBeforeClass( $factory ) { + self::$user = $factory->user->create( + array( + 'role' => 'administrator', + ) + ); + } + + /** + * Setup test class. + */ + public function setUp() { + parent::setUp(); + wp_set_current_user( self::$user ); + } + /** * Test that the list of data endpoints includes download-ips. */