Skip to content

Commit

Permalink
Merge pull request #3755 from google/feature/3644-remove-rest-batch-i…
Browse files Browse the repository at this point in the history 
…nfrastructure

Feature/3644 remove rest batch infrastructure
  • Loading branch information
@eugene-manuilov
eugene-manuilov authored Jul 29, 2021
2 parents 3b76daa + 4713411 commit 47b723d
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 301 deletions.
148 changes: 0 additions & 148 deletions includes/Core/Modules/Module.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
97 changes: 0 additions & 97 deletions includes/Core/REST_API/REST_Routes.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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;
}
}
45 changes: 31 additions & 14 deletions includes/Modules/Analytics_4.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
};
}

Expand Down Expand Up @@ -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 );
Expand Down Expand Up @@ -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.
*
Expand Down
41 changes: 0 additions & 41 deletions tests/phpunit/integration/Core/Modules/ModuleTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 ) );

Expand Down
1 change: 0 additions & 1 deletion tests/phpunit/integration/Core/REST_API/REST_RoutesTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<slug>[a-z0-9\\-]+)/data/(?P<datapoint>[a-z\\-]+)',
'/' . REST_Routes::REST_ROOT . '/data',
'/' . REST_Routes::REST_ROOT . '/modules/(?P<slug>[a-z0-9\\-]+)/data/notifications',
'/' . REST_Routes::REST_ROOT . '/modules/(?P<slug>[a-z0-9\\-]+)/data/settings',
'/' . REST_Routes::REST_ROOT . '/core/search/data/post-search',
Expand Down

0 comments on commit 47b723d

Please sign in to comment.