diff --git a/includes/Core/Modules/Module.php b/includes/Core/Modules/Module.php index 9a83db3b3b9..2a57d51f38f 100644 --- a/includes/Core/Modules/Module.php +++ b/includes/Core/Modules/Module.php @@ -232,154 +232,6 @@ final public function set_data( $datapoint, $data ) { ); } - /** - * Gets data for multiple datapoints in one go. - * - * When needing to fetch multiple pieces of data at once, this method provides a more performant approach than - * {@see Module::get_data()} by combining multiple external requests into a single one. - * - * @since 1.0.0 - * - * @param \stdClass[]|Data_Request[] $datasets List of datapoints with data attached. - * @return array List of responses. Each item is either the response data, or a WP_Error on failure. - */ - final public function get_batch_data( array $datasets ) { - // Ensure all services are initialized. - try { - $this->get_service( 'default' ); - } catch ( Exception $e ) { - // Internal error. - if ( ! is_array( $this->google_services ) ) { - return array(); - } - } - - $restore_defer = $this->with_client_defer( true ); - - $datapoint_definitions = $this->get_datapoint_definitions(); - $service_batches = array(); - - $data_requests = array(); - $results = array(); - foreach ( $datasets as $dataset ) { - if ( ! $dataset instanceof Data_Request ) { - $dataset = new Data_Request( - 'GET', - 'modules', - $dataset->identifier, - $dataset->datapoint, - (array) $dataset->data, - $dataset->key - ); - } - - /* @var Data_Request $dataset Request object. */ - if ( $this->slug !== $dataset->identifier ) { - continue; - } - - $definition_key = "{$dataset->method}:{$dataset->datapoint}"; - if ( ! isset( $datapoint_definitions[ $definition_key ] ) ) { - continue; - } - - $key = $dataset->key ?: wp_rand(); - $data_requests[ $key ] = $dataset; - $datapoint = $dataset->datapoint; - - try { - $this->validate_data_request( $dataset ); - $request = $this->create_data_request( $dataset ); - } catch ( Exception $e ) { - $request = $this->exception_to_error( $e, $datapoint ); - } - - if ( is_wp_error( $request ) ) { - $results[ $key ] = $request; - continue; - } - - if ( $request instanceof Closure ) { - try { - $response = $request(); - $results[ $key ] = $response; - - if ( ! is_wp_error( $response ) ) { - $results[ $key ] = $this->parse_data_response( $dataset, $response ); - } - } catch ( Exception $e ) { - $results[ $key ] = $this->exception_to_error( $e, $datapoint ); - } - continue; - } - - $datapoint_service = $datapoint_definitions[ $definition_key ]['service']; - if ( empty( $datapoint_service ) ) { - continue; - } - - if ( ! isset( $service_batches[ $datapoint_service ] ) ) { - $service_batches[ $datapoint_service ] = $this->google_services[ $datapoint_service ]->createBatch(); - } - - $service_batches[ $datapoint_service ]->add( $request, $key ); - $results[ $key ] = $definition_key; - } - - foreach ( $service_batches as $service_identifier => $batch ) { - try { - $batch_results = $batch->execute(); - } catch ( Exception $e ) { - // Set every result of this batch to the exception. - foreach ( $results as $key => $definition_key ) { - if ( is_wp_error( $definition_key ) ) { - continue; - } - - $datapoint_service = ! empty( $datapoint_definitions[ $definition_key ] ) - ? $datapoint_definitions[ $definition_key ]['service'] - : null; - - if ( is_string( $definition_key ) && $service_identifier === $datapoint_service ) { - $results[ $key ] = $this->exception_to_error( $e, explode( ':', $definition_key, 2 )[1] ); - } - } - continue; - } - - foreach ( $batch_results as $key => $result ) { - if ( 0 === strpos( $key, 'response-' ) ) { - $key = substr( $key, 9 ); - } - if ( ! isset( $results[ $key ] ) || ! is_string( $results[ $key ] ) ) { - continue; - } - - if ( ! $result instanceof Exception ) { - $results[ $key ] = $result; - $results[ $key ] = $this->parse_data_response( $data_requests[ $key ], $result ); - } else { - $definition_key = $results[ $key ]; - $results[ $key ] = $this->exception_to_error( $result, explode( ':', $definition_key, 2 )[1] ); - } - } - } - - $restore_defer(); - - // Cache the results for storybook. - if ( - ! empty( $results ) - && null !== $this->context->input()->filter( INPUT_GET, 'datacache' ) - && current_user_can( 'manage_options' ) - ) { - $cache = new Cache(); - $cache->cache_batch_results( $datasets, $results ); - } - - return $results; - } - /** * Returns the list of datapoints the class provides data for. * diff --git a/includes/Core/REST_API/REST_Routes.php b/includes/Core/REST_API/REST_Routes.php index b14af2c5189..d0360f9c240 100644 --- a/includes/Core/REST_API/REST_Routes.php +++ b/includes/Core/REST_API/REST_Routes.php @@ -167,69 +167,6 @@ private function get_routes() { }; $routes = array( - // TODO: This route is super-complex to use and needs to be simplified. - new REST_Route( - 'data', - array( - array( - 'methods' => WP_REST_Server::CREATABLE, - 'callback' => function( WP_REST_Request $request ) { - if ( ! $request['request'] ) { - return new WP_Error( 'no_data_requested', __( 'Missing request data.', 'google-site-kit' ), array( 'status' => 400 ) ); - } - - // Datasets are expected to be objects but the REST API parses the JSON into an array. - $datasets = array_map( - function ( $dataset_array ) { - return (object) $dataset_array; - }, - $request['request'] - ); - - $modules = $this->modules->get_active_modules(); - - $responses = array(); - foreach ( $modules as $module ) { - $filtered_datasets = array_filter( - $datasets, - function( $dataset ) use ( $module ) { - return 'modules' === $dataset->type && $module->slug === $dataset->identifier; // phpcs:ignore WordPress.NamingConventions.ValidVariableName - } - ); - if ( empty( $filtered_datasets ) ) { - continue; - } - $additional_responses = $module->get_batch_data( $filtered_datasets ); - if ( is_array( $additional_responses ) ) { - $responses = array_merge( $responses, $additional_responses ); - } - } - $responses = array_map( - function ( $response ) { - if ( is_wp_error( $response ) ) { - return $this->error_to_response( $response ); - } - return $response; - }, - $responses - ); - - return new WP_REST_Response( $responses ); - }, - 'permission_callback' => $can_view_insights, - 'args' => array( - 'request' => array( - 'type' => 'array', - 'description' => __( 'List of request objects.', 'google-site-kit' ), - 'required' => true, - 'items' => array( - 'type' => 'object', - ), - ), - ), - ), - ) - ), // TODO: Remove this and replace usage with calls to wp/v1/posts. new REST_Route( 'core/search/data/post-search', @@ -363,38 +300,4 @@ function ( $response ) { */ return apply_filters( 'googlesitekit_rest_routes', $routes ); } - - /** - * Converts a WP_Error to its response representation. - * - * Adapted from \WP_REST_Server::error_to_response - * - * @since 1.2.0 - * - * @param WP_Error $error Error to transform. - * - * @return array - */ - protected function error_to_response( WP_Error $error ) { - $errors = array(); - - foreach ( (array) $error->errors as $code => $messages ) { - foreach ( (array) $messages as $message ) { - $errors[] = array( - 'code' => $code, - 'message' => $message, - 'data' => $error->get_error_data( $code ), - ); - } - } - - $data = $errors[0]; - if ( count( $errors ) > 1 ) { - // Remove the primary error. - array_shift( $errors ); - $data['additional_errors'] = $errors; - } - - return $data; - } } diff --git a/includes/Modules/Analytics_4.php b/includes/Modules/Analytics_4.php index 651f7604a04..9d014f1b590 100644 --- a/includes/Modules/Analytics_4.php +++ b/includes/Modules/Analytics_4.php @@ -385,21 +385,14 @@ protected function create_data_request( Data_Request $data ) { ); } - return function() use ( $data ) { - $requests = array(); - - foreach ( $data['propertyIDs'] as $property_id ) { - $requests[] = new Data_Request( - 'GET', - 'modules', - self::MODULE_SLUG, - 'webdatastreams', - array( 'propertyID' => $property_id ), - $property_id - ); - } + $analyticsadmin = $this->get_service( 'analyticsadmin' ); + $batch_request = $analyticsadmin->createBatch(); + foreach ( $data['propertyIDs'] as $property_id ) { + $batch_request->add( $analyticsadmin->properties_webDataStreams->listPropertiesWebDataStreams( self::normalize_property_id( $property_id ) ) ); // phpcs:ignore WordPress.NamingConventions.ValidVariableName.UsedPropertyNotSnakeCase + } - return $this->get_batch_data( $requests ); + return function() use ( $batch_request ) { + return $batch_request->execute(); }; } @@ -445,6 +438,8 @@ function( $property ) { return self::filter_property_with_ids( $response ); case 'GET:webdatastreams': return array_map( array( self::class, 'filter_webdatastream_with_ids' ), $response->getWebDataStreams() ); + case 'GET:webdatastreams-batch': + return self::parse_webdatastreams_batch( $response ); } return parent::parse_data_response( $data, $response ); @@ -622,6 +617,28 @@ public static function filter_webdatastream_with_ids( $webdatastream ) { return $obj; } + /** + * Parses a response, adding the _id and _propertyID params and converting to an array keyed by the propertyID and web datastream IDs. + * + * @since n.e.x.t + * + * @param GoogleAnalyticsAdminV1alphaListWebDataStreamsResponse[] $response Array of GoogleAnalyticsAdminV1alphaListWebDataStreamsResponse objects. + * @return \stdClass[] Array of models containing _id and _propertyID attributes, keyed by the propertyID. + */ + public static function parse_webdatastreams_batch( $response ) { + $mapped = array(); + foreach ( $response as $single_response ) { + $webdatastreams = $single_response->getWebDataStreams(); + foreach ( $webdatastreams as $webdatastream ) { + $value = self::filter_webdatastream_with_ids( $webdatastream ); + $key = $value->_propertyID; // phpcs:ignore WordPress.NamingConventions.ValidVariableName.UsedPropertyNotSnakeCase + $mapped[ $key ] = isset( $mapped[ $key ] ) ? $mapped[ $key ] : array(); + $mapped[ $key ][] = $value; + } + } + return $mapped; + } + /** * Normalizes account ID and returns it. * diff --git a/tests/phpunit/integration/Core/Modules/ModuleTest.php b/tests/phpunit/integration/Core/Modules/ModuleTest.php index 42b12f61c40..38b096eac6f 100644 --- a/tests/phpunit/integration/Core/Modules/ModuleTest.php +++ b/tests/phpunit/integration/Core/Modules/ModuleTest.php @@ -135,47 +135,6 @@ public function test_set_data() { $this->assertEquals( 2, $method->getNumberOfRequiredParameters() ); } - public function test_get_batch_data() { - // get_batch_data is a wrapper for the protected execute_data_request method. - $method = new \ReflectionMethod( self::MODULE_CLASS_NAME, 'get_batch_data' ); - // Make assertions that affect backwards compatibility - $this->assertTrue( $method->isPublic() ); - // Number of parameters can increase while preserving B/C, but not decrease - $this->assertEquals( 1, $method->getNumberOfParameters() ); - // Number of required parameters can decrease while preserving B/C, but not increase - $this->assertEquals( 1, $method->getNumberOfRequiredParameters() ); - - $module = new FakeModule( new Context( GOOGLESITEKIT_PLUGIN_MAIN_FILE ) ); - $batch_responses = $module->get_batch_data( - array( - new Data_Request( - 'GET', - 'modules', - $module->slug, - 'test-request', - array( 'foo' => 'bar' ), - 'request-1' - ), - new Data_Request( - 'GET', - 'modules', - $module->slug, - 'test-request', - array( 'bar' => 'baz' ), - 'request-2' - ), - ) - ); - $response = $batch_responses['request-1']; - $this->assertEquals( 'GET', $response->method ); - $this->assertEquals( 'test-request', $response->datapoint ); - $this->assertEquals( array( 'foo' => 'bar' ), (array) $response->data ); - $response = $batch_responses['request-2']; - $this->assertEquals( 'GET', $response->method ); - $this->assertEquals( 'test-request', $response->datapoint ); - $this->assertEquals( array( 'bar' => 'baz' ), (array) $response->data ); - } - public function test_get_datapoints() { $module = new FakeModule( new Context( GOOGLESITEKIT_PLUGIN_MAIN_FILE ) ); diff --git a/tests/phpunit/integration/Core/REST_API/REST_RoutesTest.php b/tests/phpunit/integration/Core/REST_API/REST_RoutesTest.php index a8ade73e6ca..26db6d6595b 100644 --- a/tests/phpunit/integration/Core/REST_API/REST_RoutesTest.php +++ b/tests/phpunit/integration/Core/REST_API/REST_RoutesTest.php @@ -48,7 +48,6 @@ public function test_register() { '/' . REST_Routes::REST_ROOT . '/core/modules/data/info', '/' . REST_Routes::REST_ROOT . '/core/modules/data/activation', '/' . REST_Routes::REST_ROOT . '/modules/(?P[a-z0-9\\-]+)/data/(?P[a-z\\-]+)', - '/' . REST_Routes::REST_ROOT . '/data', '/' . REST_Routes::REST_ROOT . '/modules/(?P[a-z0-9\\-]+)/data/notifications', '/' . REST_Routes::REST_ROOT . '/modules/(?P[a-z0-9\\-]+)/data/settings', '/' . REST_Routes::REST_ROOT . '/core/search/data/post-search',