Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow alternate storage engine #455

Merged
merged 2 commits into from
Feb 12, 2020
Merged

Conversation

rrennick
Copy link
Contributor

@rrennick rrennick commented Feb 10, 2020

Fixes #418

This PR

  • Updates the base store class to allow a datastore to set the maximum $args length on an action
  • Updates the table store to support a maximum of 8000 characters encoded. The maximum record length is 65535 bytes (excluding *text fields). 8000 * 4 (for utf8mb4) = 32000 bytes. 8000 is the largest we should go without switching to a *text field.
  • It does not remove the deprecated warning on the args length in the post store

Testing Procedure

  • Switch to AS tag 2.2.5
  • Deactivate WC 4.0 beta and WC Subscriptions 3.0 if on those versions
  • Add the following snippet
add_action( 'init', function() {
	$action_id = as_schedule_single_action( time() + 50, 'test_action', range( 100, 150 ) );
	error_log( 'test_action: ' . $action_id );
	$found_action = as_next_scheduled_action( 'test_action', range( 100, 150 ) );
	error_log( 'Schedule: ' . gmdate( 'Y-m-d H:i:s O', $found_action ) );
} );
  • This will log to the debug log
[10-Feb-2020 19:48:04 UTC] PHP Notice:  ActionScheduler_Action::$args was called <strong>incorrectly</strong>. To ensure the action args column can be indexed, action args should not be more than 191 characters when encoded as JSON. Support for strings longer than this will be removed in a future version. Please see <a href="https://wordpress.org/support/article/debugging-in-wordpress/">Debugging in WordPress</a> for more information. (This message was added in version 2.1.0.) in /Sites/ron/wp-includes/functions.php on line 4986
[10-Feb-2020 19:48:04 UTC] test_action: 11084
[10-Feb-2020 19:48:04 UTC] Schedule: 2020-02-10 19:48:48 +0000
  • If your migration was already complete wp option delete action_scheduler_migration_status
  • Switch to this branch
  • Refresh the dashboard then disable the snippet above
  • Debug log should have
[10-Feb-2020 19:48:04 UTC] test_action: 12345
[10-Feb-2020 19:48:04 UTC] Schedule: 2020-02-10 19:48:48 +0000
  • After allowing the migration to run

  • Check that test actions created on 2.2.5 are migrated (the original action ID can be found in the migration entry on the action log).

  • Check that test actions created on this branch are there

  • go to Tools -> Scheduled actions

  • enter 149 in the search box

  • test_action actions should be found

@rrennick rrennick added type: enhancement The issue is a request for an enhancement. Status: Needs Review labels Feb 10, 2020
@rrennick rrennick added this to the 3.0.2 milestone Feb 10, 2020
@rrennick rrennick force-pushed the allow_alternate_storage_engine branch from 5a636f5 to 9710fa8 Compare February 10, 2020 20:32
@rrennick rrennick force-pushed the allow_alternate_storage_engine branch from 9710fa8 to 1cff963 Compare February 10, 2020 20:57
@rrennick
Copy link
Contributor Author

@peterfabian Can you give this one a check with webhooks?

@rrennick rrennick requested a review from thenbrent February 10, 2020 21:40
@peterfabian
Copy link
Contributor

Thanks for the ping, I'll check it out a bit later today.

@peterfabian
Copy link
Contributor

I tested this by creating a bunch of actions with AS 2.2.5 + WC 3.9.1, some with long and some with short args. All the actions have been migrated correctly. Also new actions with long args seem to work correctly, webhooks are firing, filtering and searching in the Scheduled Actions list works, too.
Great work!

Copy link
Contributor

@peterfabian peterfabian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@rrennick rrennick merged commit 90454ed into master Feb 12, 2020
@rrennick rrennick deleted the allow_alternate_storage_engine branch February 12, 2020 15:11
@rrennick
Copy link
Contributor Author

Thanks @peterfabian

@tillkruss
Copy link
Contributor

@rrennick: Are the extended_args searchable?

@rrennick
Copy link
Contributor Author

Are the extended_args searchable?

Yes, with the search parameter. When looking for the next scheduled action, the passed args hash is compared to the DB args hash.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement The issue is a request for an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrations fails because args too long
3 participants