From ffa06343a33cb2d1bb2f29ecc4ccd64e3409d2cb Mon Sep 17 00:00:00 2001 From: Jamie McClymont Date: Mon, 28 Jan 2019 15:06:13 +1300 Subject: [PATCH] Fix bugs in expansion of events with overridden instances --- inc/RRule.php | 77 ++++++++++----- testing/phpunit/ExpansionTest.php | 153 ++++++++++++++++++++++++++++++ 2 files changed, 205 insertions(+), 25 deletions(-) create mode 100644 testing/phpunit/ExpansionTest.php diff --git a/inc/RRule.php b/inc/RRule.php index 7819a343..51a31ecf 100644 --- a/inc/RRule.php +++ b/inc/RRule.php @@ -1227,6 +1227,26 @@ function expand_event_instances( vComponent $vResource, $range_start = null, $ra $is_date = false; $has_repeats = false; $dtstart_type = 'DTSTART'; + + $components_prefix = []; + $components_base_events = []; + $components_override_events = []; + + foreach ($components AS $k => $comp) { + if ( $comp->GetType() != 'VEVENT' && $comp->GetType() != 'VTODO' && $comp->GetType() != 'VJOURNAL' ) { + // Other types of component (such as VTIMEZONE) go first + $components_prefix[] = $comp; + } else if ($comp->GetProperty('RECURRENCE-ID') === null) { + // This is the base event, we need to handle it first + $components_base_events[] = $comp; + } else { + // This is an override of an event instance, handle it last + $components_override_events[] = $comp; + } + } + + $components = array_merge($components_prefix, $components_base_events, $components_override_events); + foreach( $components AS $k => $comp ) { if ( $comp->GetType() != 'VEVENT' && $comp->GetType() != 'VTODO' && $comp->GetType() != 'VJOURNAL' ) { continue; @@ -1371,36 +1391,43 @@ function expand_event_instances( vComponent $vResource, $range_start = null, $ra $p = $comp->GetProperty('RECURRENCE-ID'); if ( isset($p) && $p->Value() != '') { $recurrence_id = $p->Value(); - if ( !isset($new_components[$recurrence_id]) ) { - // The component we're replacing is outside the range. Unless the replacement - // is *in* the range we will move along to the next one. - $dtstart_prop = $comp->GetProperty($dtstart_type); - if ( !isset($dtstart_prop) ) continue; // No start: no expansion. Note that we consider 'DUE' to be a start if DTSTART is missing - $dtstart = new RepeatRuleDateTime( $dtstart_prop ); - $is_date = $dtstart->isDate(); - if ( $return_floating_times ) $dtstart->setAsFloat(); - $dtstart = $dtstart->FloatOrUTC($return_floating_times); - if ( $dtstart > $end_utc ) continue; // Start after end of range, skip it - $end_type = ($comp->GetType() == 'VTODO' ? 'DUE' : 'DTEND'); - $duration = $comp->GetProperty('DURATION'); - if ( !isset($duration) || $duration->Value() == '' ) { - $instance_end = $comp->GetProperty($end_type); - if ( isset($instance_end) ) { - $dtend = new RepeatRuleDateTime( $instance_end ); - if ( $return_floating_times ) $dtend->setAsFloat(); - $dtend = $dtend->FloatOrUTC($return_floating_times); - } - else { - $dtend = $dtstart + ($is_date ? $dtstart + 86400 : 0 ); - } + + $dtstart_prop = $comp->GetProperty('DTSTART'); + if ( !isset($dtstart_prop) && $comp->GetType() != 'VTODO' ) { + $dtstart_prop = $comp->GetProperty('DUE'); + } + + if ( !isset($new_components[$recurrence_id]) && !isset($dtstart_prop) ) continue; // No start: no expansion. Note that we consider 'DUE' to be a start if DTSTART is missing + $dtstart_rrdt = new RepeatRuleDateTime( $dtstart_prop ); + $is_date = $dtstart_rrdt->isDate(); + if ( $return_floating_times ) $dtstart_rrdt->setAsFloat(); + $dtstart = $dtstart_rrdt->FloatOrUTC($return_floating_times); + if ( !isset($new_components[$recurrence_id]) && $dtstart > $end_utc ) continue; // Start after end of range, skip it + + $end_type = ($comp->GetType() == 'VTODO' ? 'DUE' : 'DTEND'); + $duration = $comp->GetProperty('DURATION'); + + if ( !isset($duration) || $duration->Value() == '' ) { + $instance_end = $comp->GetProperty($end_type); + if ( isset($instance_end) ) { + $dtend_rrdt = new RepeatRuleDateTime( $instance_end ); + if ( $return_floating_times ) $dtend_rrdt->setAsFloat(); + $dtend = $dtend_rrdt->FloatOrUTC($return_floating_times); + + $comp->AddProperty('DURATION', Rfc5545Duration::fromTwoDates($dtstart_rrdt, $dtend_rrdt) ); } else { - $duration = new Rfc5545Duration($duration->Value()); - $dtend = $dtstart + $duration->asSeconds(); + $dtend = $dtstart + ($is_date ? $dtstart + 86400 : 0 ); } - if ( $dtend < $start_utc ) continue; // End before start of range: skip that too. } + else { + $duration = new Rfc5545Duration($duration->Value()); + $dtend = $dtstart + $duration->asSeconds(); + } + + if ( !isset($new_components[$recurrence_id]) && $dtend < $start_utc ) continue; // End before start of range: skip that too. + if ( DEBUG_RRULE ) printf( "Replacing overridden instance at %s\n", $recurrence_id); $new_components[$recurrence_id] = $comp; } diff --git a/testing/phpunit/ExpansionTest.php b/testing/phpunit/ExpansionTest.php new file mode 100644 index 00000000..7d0f36c1 --- /dev/null +++ b/testing/phpunit/ExpansionTest.php @@ -0,0 +1,153 @@ +GetComponents(['VEVENT' => true]); + + $result = array(); + + foreach ($expansion as $k => $instance) { + // The same logic used in freebusy-functions (apart from default timezone + // handling, which isn't really under test here) + + $start_date = new RepeatRuleDateTime($instance->GetProperty('DTSTART')); + + $duration = $instance->GetProperty('DURATION'); + $duration = (!isset($duration) ? 'P1D' : $duration->Value()); + + $end_date = clone($start_date); + $end_date->modify($duration); + + array_push($result, $start_date->UTC() .'/'. $end_date->UTC()); + } + sort($result); + return $result; +} + +final class ExpansionTest extends TestCase +{ + const expected_freebusyish_for_base = [ + '20190121T000000Z/20190121T010000Z', + '20190122T000000Z/20190122T010000Z', + '20190123T000000Z/20190123T010000Z', + '20190124T000000Z/20190124T010000Z', + ]; + + public function testUnmodifiedCal() { + global $base_cal; + + self::assertEquals( + self::expected_freebusyish_for_base, + get_freebusyish($base_cal) + ); + } + + public function testTueRenamed() { + global $tuesday_renamed_cal; + + self::assertEquals( + self::expected_freebusyish_for_base, + get_freebusyish($tuesday_renamed_cal) + ); + } + + public function testTueRenamedSwapped() { + global $tuesday_renamed_cal_order_swapped; + + self::assertEquals( + self::expected_freebusyish_for_base, + get_freebusyish($tuesday_renamed_cal_order_swapped) + ); + } +}