Merge pull request #404 from woocommerce/issue_401_throw_exception

Provide better error message when saving an action with $args > 191 char
woocommerce · Dec 19, 2019 · b9f3c86 · b9f3c86
2 parents 9f015cb + 13072f5
commit b9f3c86
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 5 deletions.
diff --git a/classes/abstracts/ActionScheduler_Store.php b/classes/abstracts/ActionScheduler_Store.php
@@ -15,6 +15,9 @@ abstract class ActionScheduler_Store extends ActionScheduler_Store_Deprecated {
 	/** @var ActionScheduler_Store */
 	private static $store = NULL;
 
+	/** @var int */
+	private static $max_index_length = 191;
+
 	/**
 	 * @param ActionScheduler_Action $action
 	 * @param DateTime $scheduled_date Optional Date of the first instance
@@ -210,6 +213,21 @@ protected function validate_schedule( $schedule, $action_id ) {
 		}
 	}
 
+	/**
+	 * InnoDB indexes have a maximum size of 767 bytes by default, which is only 191 characters with utf8mb4.
+	 *
+	 * Previously, AS wasn't concerned about args length, as we used the (unindex) post_content column. However,
+	 * as we prepare to move to custom tables, and can use an indexed VARCHAR column instead, we want to warn
+	 * developers of this impending requirement.
+	 *
+	 * @param ActionScheduler_Action $action
+	 */
+	protected function validate_action( ActionScheduler_Action $action ) {
+		if ( strlen( json_encode( $action->get_args() ) ) > self::$max_index_length ) {
+			throw new InvalidArgumentException( __( 'ActionScheduler_Action::$args too long. To ensure the args column can be indexed, action args should not be more than 191 characters when encoded as JSON.', 'action-scheduler' ) );
+		}
+	}
+
 	/**
 	 * Cancel pending actions by hook.
 	 *

diff --git a/classes/data-stores/ActionScheduler_DBStore.php b/classes/data-stores/ActionScheduler_DBStore.php
@@ -29,6 +29,9 @@ public function init() {
 	 */
 	public function save_action( ActionScheduler_Action $action, \DateTime $date = null ) {
 		try {
+
+			$this->validate_action( $action );
+
 			/** @var \wpdb $wpdb */
 			global $wpdb;
 			$data = [

diff --git a/classes/data-stores/ActionScheduler_wpPostStore.php b/classes/data-stores/ActionScheduler_wpPostStore.php
@@ -12,9 +12,6 @@ class ActionScheduler_wpPostStore extends ActionScheduler_Store {
 	/** @var DateTimeZone */
 	protected $local_timezone = NULL;
 
-	/** @var int */
-	private static $max_index_length = 191;
-
 	public function save_action( ActionScheduler_Action $action, DateTime $scheduled_date = NULL ){
 		try {
 			$this->validate_action( $action );
@@ -835,8 +832,11 @@ public function migration_dependencies_met( $setting ) {
 	 * @param ActionScheduler_Action $action
 	 */
 	protected function validate_action( ActionScheduler_Action $action ) {
-		if ( strlen( json_encode( $action->get_args() ) ) > self::$max_index_length ) {
-			_doing_it_wrong( 'ActionScheduler_Action::$args', sprintf( 'To ensure the action args column can be indexed, action args should not be more than %d characters when encoded as JSON. Support for strings longer than this will be removed in a future version.', self::$max_index_length ), '2.1.0' );
+		try {
+			parent::validate_action( $action );
+		} catch ( Exception $e ) {
+			$message = sprintf( __( '%s Support for strings longer than this will be removed in a future version.', 'action-scheduler' ), $e->getMessage() );
+			_doing_it_wrong( 'ActionScheduler_Action::$args', $message, '2.1.0' );
 		}
 	}