WordPress.org

Make WordPress Core

Opened 5 years ago

Last modified 6 months ago

#18997 reviewing feature request

Adding wp_unschedule_hook function

Reported by: arena Owned by: chriscct7
Milestone: Future Release Priority: normal
Severity: normal Version: 3.3
Component: Cron API Keywords: has-patch needs-testing
Focuses: Cc:

Description

Unschedule all previously scheduled cron job for a hook.

Can be usefull for plugins when deactivating to clean up the cron queue

The $hook parameter is required, so that the events can be identified.

@param string $hook Action hook, the execution of which will be unscheduled.

Attachments (6)

#18997.2.patch (929 bytes) - added by arena 5 years ago.
#18997.patch (929 bytes) - added by arena 5 years ago.
#cron.patch (4.2 KB) - added by arena 5 years ago.
18997.patch (905 bytes) - added by mordauk 2 years ago.
18997-tests.patch (1.1 KB) - added by mordauk 2 years ago.
18997-refreshed-mordauks-patch.patch (2.5 KB) - added by jrf 9 months ago.

Download all attachments as: .zip

Change History (33)

#1 @SergeyBiryukov
5 years ago

Isn't that what wp_clear_scheduled_hook() already does?

#2 @dd32
5 years ago

Isn't that what wp_clear_scheduled_hook() already does?

That clears by hook + args, It's common place to hook multiple "instances" of a job with varying parameters (ie. Post ID to process) to a single hook, there's no API to clear all events for that hook at present. I've seen this requested before, it might be on trac, or elsewhere, I'm not sure.

#3 @SergeyBiryukov
5 years ago

That makes sense, I had a feeling I must have missed something.

#4 @arena
5 years ago

  • Keywords dev-feedback removed

dd32 is right :

"there's no API to clear all events for a hook at present"

i coded it myself for a plugin but think it ought to be in the API

#5 @arena
5 years ago

just giving a bump to this ticket to have it included in 3.4

thank you

#6 @Viper007Bond
5 years ago

Patch seems to be okay although it will need to be cleaned up to meet WordPress coding standards.

#7 follow-up: @arena
5 years ago

Done !

#8 in reply to: ↑ 7 @Viper007Bond
5 years ago

Replying to arena:

Done !

Just one more minor nitpick:

if ( empty( $crons[$timestamp] ) )
	unset( $crons[$timestamp] );

We do if's like that (line break in there) and with some extra spaces inside of the empty(). :)

@arena
5 years ago

@arena
5 years ago

#9 @arena
5 years ago

Done !

#10 @SergeyBiryukov
5 years ago

  • Milestone changed from Awaiting Review to 3.4

#11 @nacin
5 years ago

Perhaps too clever, or slower, but instead of if ( empty ) unset();, we could also hold off until before we call _set_cron_array() and then do $cron = array_filter( $cron ); That will clear out empty values.

#12 follow-up: @arena
5 years ago

Nacin, your point is interesting.

May be this should be done once for all in

_set_cron_array()

simplifying all other functions calling _set_cron_array()

#13 in reply to: ↑ 12 @arena
5 years ago

Replying to arena:

Nacin, your point is interesting.

May be this should be done once for all in

_set_cron_array()

simplifying all other functions calling _set_cron_array()

since nacin tip, i reviewed the cron api and made some code changes :

  • to have _get_cron_array closer to _set_cron_array when possible
  • generalization of test empty( $crons ) after _get_cron_array()
  • added a '$action' argument in function _set_cron_array()

patch is named #cron

@arena
5 years ago

#14 @nacin
4 years ago

  • Milestone changed from 3.4 to Future Release

A good idea, but too late for 3.4.

#15 follow-up: @arena
4 years ago

not fair, nacin opened 5 months ago ...

#16 in reply to: ↑ 15 @nacin
4 years ago

Replying to arena:

not fair, nacin opened 5 months ago ...

Cron had a few changes already in 3.4, specifically microtime locking. This however digs a bit deeper into the API and deserves a more thorough review. If we end up setting or getting the cron array out of order, we risk a bug or race condition. I think the patch will be great for 3.5.

#17 @nacin
4 years ago

  • Keywords needs-unit-tests added

I am fine with reviving %2318997.patch for 3.4, but we will first need unit tests: http://unit-tests.trac.wordpress.org/browser/wp-testcase/test_cron.php.

Version 0, edited 4 years ago by nacin (next)

#18 @arena
4 years ago

  • Resolution set to maybelater
  • Status changed from new to closed

#19 @Viper007Bond
4 years ago

  • Resolution maybelater deleted
  • Status changed from closed to reopened

Please don't close valid tickets. Even if you no longer desire the feature, I'm sure others do. Thanks. :)

@mordauk
2 years ago

#20 @mordauk
2 years ago

18997.patch is a refreshed patch that improves some formatting, adds brackets, and uses the src directory structure.

@mordauk
2 years ago

#21 @mordauk
2 years ago

  • Keywords needs-unit-tests removed

18997-tests.patch introduces the test_unschedule_hook() test.

#22 @mordauk
2 years ago

  • Keywords needs-testing added

#23 @jpswade
2 years ago

Had a problem today where I accidentally scheduled the function that adds the schedule.

Although I spotted and fixed the error, it was impossible to remove the hook without this function.

http://wordpress.stackexchange.com/questions/39681/delete-all-scheduled-events-with-a-particular-hook

This case here outlines the same issue.

#24 @chriscct7
10 months ago

  • Owner changed from arena to chriscct7
  • Status changed from reopened to reviewing

#25 @jrf
9 months ago

What can be done to move this ticket forward ? I'm all for this.

I've tested @mordauk's patch & unit tests and both seem to work as expected.

Uploading refreshed patch - which includes both his original patch + the unit test - now just in case that's the issue. Props should go to @mordauk.

#26 @chriscct7
9 months ago

@jrf not much needs to be done. I'm going to give it a quick run through, and then it will be milestoned for 4.5

This ticket was mentioned in Slack in #core by chriscct7. View the logs.


6 months ago

Note: See TracTickets for help on using tickets.