Merge pull request #5727 from google/enhancement/#5711-update-validation

google · Aug 24, 2022 · 511042e · 511042e
2 parents bd1aeba + dbd3d9e
commit 511042e
Show file tree

Hide file tree

Showing 4 changed files with 101 additions and 42 deletions.
diff --git a/includes/Core/Modules/Module.php b/includes/Core/Modules/Module.php
@@ -338,6 +338,9 @@ final protected function execute_data_request( Data_Request $data ) {
 			$datapoint    = $this->get_datapoint_definition( "{$data->method}:{$data->datapoint}" );
 			$oauth_client = $this->get_oauth_client_for_datapoint( $datapoint );
 
+			// Always reset this property first to ensure it is only set true for the current request.
+			$this->is_using_shared_credentials = false;
+
 			$this->validate_datapoint_scopes( $datapoint, $oauth_client );
 			$this->validate_base_scopes( $oauth_client );
 

diff --git a/includes/Core/Validation/Exception/Invalid_Report_Dimensions_Exception.php b/includes/Core/Validation/Exception/Invalid_Report_Dimensions_Exception.php
@@ -0,0 +1,24 @@
+<?php
+/**
+ * Class Google\Site_Kit\Core\Validation\Exception\Invalid_Report_Dimensions_Exception
+ *
+ * @package   Google\Site_Kit\Core\Validation\Exception
+ * @copyright 2022 Google LLC
+ * @license   https://www.apache.org/licenses/LICENSE-2.0 Apache License 2.0
+ * @link      https://sitekit.withgoogle.com
+ */
+
+namespace Google\Site_Kit\Core\Validation\Exception;
+
+use DomainException;
+
+/**
+ * Exception thrown when dimensions are invalid for a report request.
+ *
+ * @since n.e.x.t
+ * @access private
+ * @ignore
+ */
+class Invalid_Report_Dimensions_Exception extends DomainException {
+
+}
diff --git a/includes/Core/Validation/Exception/Invalid_Report_Metrics_Exception.php b/includes/Core/Validation/Exception/Invalid_Report_Metrics_Exception.php
@@ -0,0 +1,24 @@
+<?php
+/**
+ * Class Google\Site_Kit\Core\Validation\Exception\Invalid_Report_Metrics_Exception
+ *
+ * @package   Google\Site_Kit\Core\Validation\Exception
+ * @copyright 2022 Google LLC
+ * @license   https://www.apache.org/licenses/LICENSE-2.0 Apache License 2.0
+ * @link      https://sitekit.withgoogle.com
+ */
+
+namespace Google\Site_Kit\Core\Validation\Exception;
+
+use DomainException;
+
+/**
+ * Exception thrown when metrics are invalid for a report request.
+ *
+ * @since n.e.x.t
+ * @access private
+ * @ignore
+ */
+class Invalid_Report_Metrics_Exception extends DomainException {
+
+}
diff --git a/includes/Modules/Analytics.php b/includes/Modules/Analytics.php
@@ -23,6 +23,8 @@
 use Google\Site_Kit\Core\Modules\Module_With_Owner;
 use Google\Site_Kit\Core\Modules\Module_With_Owner_Trait;
 use Google\Site_Kit\Core\REST_API\Exception\Invalid_Datapoint_Exception;
+use Google\Site_Kit\Core\Validation\Exception\Invalid_Report_Metrics_Exception;
+use Google\Site_Kit\Core\Validation\Exception\Invalid_Report_Dimensions_Exception;
 use Google\Site_Kit\Core\Authentication\Google_Proxy;
 use Google\Site_Kit\Core\Assets\Asset;
 use Google\Site_Kit\Core\Assets\Script;
@@ -543,20 +545,12 @@ function ( $dimension_def ) {
 					);
 
 					if ( ! empty( $dimensions ) ) {
-						$invalid_dimensions_error_message = $this->validate_report_dimensions(
-							array_map(
-								function ( $dimension_def ) {
-									return $dimension_def->getName();
-								},
-								$dimensions
-							)
-						);
-
-						if ( isset( $invalid_dimensions_error_message ) ) {
+						try {
+							$this->validate_report_dimensions( $dimensions );
+						} catch ( Invalid_Report_Dimensions_Exception $exception ) {
 							return new WP_Error(
-								'invalid_dimensions',
-								$invalid_dimensions_error_message,
-								array( 'status' => 400 )
+								'invalid_analytics_report_dimensions',
+								$exception->getMessage()
 							);
 						}
 
@@ -658,20 +652,12 @@ function ( $metric_def ) {
 					);
 
 					if ( ! empty( $metrics ) ) {
-						$invalid_metrics_error_message = $this->validate_report_metrics(
-							array_map(
-								function ( $metric_def ) {
-									return $metric_def->getExpression();
-								},
-								$metrics
-							)
-						);
-
-						if ( isset( $invalid_metrics_error_message ) ) {
+						try {
+							$this->validate_report_metrics( $metrics );
+						} catch ( Invalid_Report_Metrics_Exception $exception ) {
 							return new WP_Error(
-								'invalid_metrics',
-								$invalid_metrics_error_message,
-								array( 'status' => 400 )
+								'invalid_analytics_report_metrics',
+								$exception->getMessage()
 							);
 						}
 
@@ -1370,9 +1356,8 @@ public function check_service_entity_access() {
 	 *
 	 * @since n.e.x.t
 	 *
-	 * @param array $metrics The metrics to validate.
-	 *
-	 * @return null|WP_Error NULL if metrics are valid, otherwise WP_Error.
+	 * @param Google_Service_AnalyticsReporting_Metric[] $metrics The metrics to validate.
+	 * @throws Invalid_Report_Metrics_Exception Thrown if the metrics are invalid.
 	 */
 	protected function validate_report_metrics( $metrics ) {
 		if ( false === $this->is_using_shared_credentials ) {
@@ -1395,32 +1380,43 @@ protected function validate_report_metrics( $metrics ) {
 			)
 		);
 
-		$invalid_metrics = array_diff( $metrics, $valid_metrics );
+		$invalid_metrics = array_diff(
+			array_map(
+				function ( $metric ) {
+					return $metric->getExpression();
+				},
+				$metrics
+			),
+			$valid_metrics
+		);
 
 		if ( count( $invalid_metrics ) > 0 ) {
-			return sprintf(
+			$message = sprintf(
 				/* translators: %s is replaced with a comma separated list of the invalid metrics. */
 				_n(
 					'Unsupported metric requested: %s',
 					'Unsupported metrics requested: %s',
 					count( $invalid_metrics ),
 					'google-site-kit'
 				),
-				implode( ', ', $invalid_metrics )
+				join(
+					/* translators: used between list items, there is a space after the comma. */
+					__( ', ', 'google-site-kit' ),
+					$invalid_metrics
+				)
 			);
-		}
 
-		return null;
+			throw new Invalid_Report_Metrics_Exception( $message );
+		}
 	}
 
 	/**
 	 * Validates the report dimensions.
 	 *
 	 * @since n.e.x.t
 	 *
-	 * @param array $dimensions The dimensions to validate.
-	 *
-	 * @return null|WP_Error NULL if dimensions are valid, otherwise WP_Error.
+	 * @param Google_Service_AnalyticsReporting_Dimension[] $dimensions The dimensions to validate.
+	 * @throws Invalid_Report_Dimensions_Exception Thrown if the dimensions are invalid.
 	 */
 	protected function validate_report_dimensions( $dimensions ) {
 		if ( false === $this->is_using_shared_credentials ) {
@@ -1440,22 +1436,34 @@ protected function validate_report_dimensions( $dimensions ) {
 			)
 		);
 
-		$invalid_dimensions = array_diff( $dimensions, $valid_dimensions );
+		$invalid_dimensions = array_diff(
+			array_map(
+				function ( $dimension ) {
+					return $dimension->getName();
+				},
+				$dimensions
+			),
+			$valid_dimensions
+		);
 
 		if ( count( $invalid_dimensions ) > 0 ) {
-			return sprintf(
+			$message = sprintf(
 				/* translators: %s is replaced with a comma separated list of the invalid dimensions. */
 				_n(
 					'Unsupported dimension requested: %s',
 					'Unsupported dimensions requested: %s',
 					count( $invalid_dimensions ),
 					'google-site-kit'
 				),
-				implode( ', ', $invalid_dimensions )
+				join(
+					/* translators: used between list items, there is a space after the comma. */
+					__( ', ', 'google-site-kit' ),
+					$invalid_dimensions
+				)
 			);
-		}
 
-		return null;
+			throw new Invalid_Report_Dimensions_Exception( $message );
+		}
 	}
 
 }