WordPress.org

Make WordPress Core

Ticket #49693: 49693-5.patch

File 49693-5.patch, 7.6 KB (added by aidvu, 5 months ago)

Better approach, that also correctly limits to 24 hours per day (for intervals less than a day), and to INTERVAL / HOUR_IN_SECONDS for longer intervals.

  • src/wp-includes/cron.php

     
    220220        }
    221221
    222222        $key = md5( serialize( $event->args ) );
     223        $crons = (array) _get_cron_array();
     224
     225        // Skip the duplicate check if we're doing cron.
     226        if ( ! wp_doing_cron() ) {
     227                // Minimal check interval is a day.
     228                $check_interval = DAY_IN_SECONDS;
     229                if ( $check_interval < $event->interval ) {
     230                        $check_interval = $event->interval;
     231                }
     232
     233                // Always allow 24 of the same event per day, if interval is less than a day.
     234                // Otherwise, calculate max events based on interval and 24 events per day.
     235                $allowed_occurrences = floor( $check_interval / HOUR_IN_SECONDS );
     236
     237                $min_timestamp = $event->timestamp - ( $check_interval / 2 );
     238                $max_timestamp = $event->timestamp + ( $check_interval / 2 );
     239                $duplicate_occurrences = 0;
     240
     241                foreach ( $crons as $event_timestamp => $cron ) {
     242                        // Skip events before the check interval.
     243                        if ( $event_timestamp < $min_timestamp ) {
     244                                continue;
     245                        }
     246
     247                        // Stop checking when we're after interval max timestamp.
     248                        if ( $event_timestamp > $max_timestamp ) {
     249                                break;
     250                        }
     251
     252                        // Check if an event with same hook and args exists.
     253                        if ( ! isset( $cron[ $event->hook ][ $key ] ) ) {
     254                                continue;
     255                        }
     256
     257                        $cron_event = $cron[ $event->hook ][ $key ];
     258
     259                        // Count as duplicate if event also has the same interval and schedule.
     260                        if ( $cron_event['interval'] === $event->interval && $cron_event['schedule'] === $event->schedule ) {
     261                                $duplicate_occurrences++;
     262                        }
     263                }
     264
     265                if ( $allowed_occurrences <= $duplicate_occurrences ) {
     266                        return false;
     267                }
     268        }
    223269
    224         $crons = _get_cron_array();
    225270        $crons[ $event->timestamp ][ $event->hook ][ $key ] = array(
    226271                'schedule' => $event->schedule,
    227272                'args'     => $event->args,
    228273                'interval' => $event->interval,
    229274        );
    230275        uksort( $crons, 'strnatcasecmp' );
     276
    231277        return _set_cron_array( $crons );
    232278}
    233279
  • tests/phpunit/tests/cron.php

     
    534534         * @ticket 45976.
    535535         */
    536536        function test_get_scheduled_event_recurring() {
    537                 $hook     = __FUNCTION__;
    538                 $args     = array( 'arg1' );
    539                 $ts_late  = strtotime( '+30 minutes' );
    540                 $ts_next  = strtotime( '+3 minutes' );
    541                 $schedule = 'hourly';
    542                 $interval = HOUR_IN_SECONDS;
     537                $hook         = __FUNCTION__;
     538                $args         = array( 'arg1' );
     539                $ts_late      = strtotime( '+30 minutes' );
     540                $ts_next      = strtotime( '+3 minutes' );
     541                $schedule_one = 'hourly';
     542                $schedule_two = 'daily';
     543                $interval_one = HOUR_IN_SECONDS;
     544                $interval_two = DAY_IN_SECONDS;
    543545
    544546                $expected1 = (object) array(
    545547                        'hook'      => $hook,
    546548                        'timestamp' => $ts_late,
    547                         'schedule'  => $schedule,
     549                        'schedule'  => $schedule_one,
    548550                        'args'      => $args,
    549                         'interval'  => $interval,
     551                        'interval'  => $interval_one,
    550552                );
    551553
    552554                $expected2 = (object) array(
    553555                        'hook'      => $hook,
    554556                        'timestamp' => $ts_next,
    555                         'schedule'  => $schedule,
     557                        'schedule'  => $schedule_two,
    556558                        'args'      => $args,
    557                         'interval'  => $interval,
     559                        'interval'  => $interval_two,
    558560                );
    559561
    560562                // Schedule late running event.
    561                 wp_schedule_event( $ts_late, $schedule, $hook, $args );
     563                wp_schedule_event( $ts_late, $schedule_one, $hook, $args );
    562564                // Schedule next running event.
    563                 wp_schedule_event( $ts_next, $schedule, $hook, $args );
     565                wp_schedule_event( $ts_next, $schedule_two, $hook, $args );
    564566
    565567                // Late running, timestamp specified.
    566568                $this->assertEquals( $expected1, wp_get_scheduled_event( $hook, $args, $ts_late ) );
     
    680682                $this->assertTrue( wp_schedule_single_event( $ts2, $hook, $args ) );
    681683                $this->assertTrue( wp_schedule_single_event( $ts3, $hook, $args ) );
    682684        }
     685
     686        /**
     687         * Recurring events have a max event limiter. Make sure it's honored properly.
     688         *
     689         * @ticket 49693
     690         */
     691        function test_duplicate_recurring_event() {
     692                $hook     = __FUNCTION__;
     693                $args     = array( 'arg1' );
     694                $schedule = 'weekly';
     695
     696                // Add the max number of events for a 'weekly' schedule.
     697                // Weekly schedule, max events is 24 (per day) * 7 (days of week).
     698                $max_events = WEEK_IN_SECONDS / HOUR_IN_SECONDS;
     699                for ( $i = 0; $i < $max_events; $i++ ) {
     700                        $timestamp_next = strtotime( "+$i minutes" );
     701                        // Scheduling the same recurring event (hook and args), but with different timestamp should fail.
     702                        $this->assertTrue( wp_schedule_event( $timestamp_next, $schedule, $hook, $args ) );
     703                }
     704                $expected = _get_cron_array();
     705
     706                // Scheduling the event $max_events + 1 time should fail.
     707                $this->assertFalse( wp_schedule_event( strtotime( '+300 minutes' ), $schedule, $hook, $args ) );
     708
     709                // Check cron option is unchanged.
     710                $this->assertEquals( $expected, _get_cron_array() );
     711                $this->assertCount( $max_events, _get_cron_array() );
     712        }
     713
     714        /**
     715         * Recurring events that have the same hook but different args or schedule are allowed.
     716         *
     717         * @ticket 49693
     718         */
     719        function test_not_duplicate_recurring_event() {
     720                $hook         = __FUNCTION__;
     721                $args_one     = array( 'arg1' );
     722                $args_two     = array( 'arg2' );
     723                $timestamp    = strtotime( '+60 minutes' );
     724                $schedule_one = 'hourly';
     725                $schedule_two = 'daily';
     726
     727                // Schedule recurring event.
     728                $this->assertNotFalse( wp_schedule_event( $timestamp, $schedule_one, $hook, $args_one ) );
     729
     730                // Schedule recurring event as above, but with different schedule is allowed.
     731                $this->assertNotFalse( wp_schedule_event( $timestamp, $schedule_two, $hook, $args_one ) );
     732
     733                // Schedule recurring event as above, but with different args is allowed.
     734                $this->assertNotFalse( wp_schedule_event( $timestamp, $schedule_one, $hook, $args_two ) );
     735        }
     736
     737        /**
     738         * Make sure that rescheduling still works with the recurring event limiter when running CRON.
     739         *
     740         * @ticket 49693
     741         */
     742        function test_reschedule_recurring_event() {
     743                $hook   = __FUNCTION__;
     744                $ts_one = strtotime( '+30 minutes' );
     745
     746                // Pretend CRON is running
     747                add_filter( 'wp_doing_cron', '__return_true' );
     748
     749                // Confirm there's no events.
     750                $this->assertEmpty( _get_cron_array() );
     751
     752                // Add an event.
     753                $this->assertTrue( wp_schedule_event( $ts_one, 'hourly', $hook ) );
     754
     755                // Reschedule it.
     756                $this->assertNotFalse( wp_reschedule_event( $ts_one, 'hourly', $hook ) );
     757
     758                // Make sure the original event is still there.
     759                $this->assertNotFalse( wp_get_scheduled_event( $hook, array(), $ts_one ) );
     760
     761                // Make sure the rescheduled event is also there.
     762                $this->assertEquals( 2, count( _get_cron_array() ) );
     763        }
     764
     765        /**
     766         * Make sure that the limiter applies to event rescheduling if we're not running CRON.
     767         *
     768         * @ticket 49693
     769         */
     770        function test_not_reschedule_recurring_event() {
     771                $hook     = __FUNCTION__;
     772                $schedule = 'hourly';
     773
     774                // Add the max number of events for an 'hourly' schedule.
     775                $max_events = DAY_IN_SECONDS / HOUR_IN_SECONDS; // Hourly schedule, max events per day is 24.
     776                for ( $i = 0; $i < $max_events; $i++ ) {
     777                        $timestamp_next = strtotime( "+$i minutes" );
     778                        // Scheduling the same recurring event (hook and args), but with different timestamp should fail.
     779                        $this->assertTrue( wp_schedule_event( $timestamp_next, $schedule, $hook ) );
     780                }
     781                $expected = _get_cron_array();
     782
     783                // Rescheduling the event fails because of the limiter.
     784                $this->assertFalse( wp_reschedule_event( strtotime( '+120 minutes' ), $schedule, $hook ) );
     785
     786                // Check cron option is unchanged.
     787                $this->assertEquals( $expected, _get_cron_array() );
     788                $this->assertCount( $max_events, _get_cron_array() );
     789        }
    683790}