Skip to content

Support concurrency of multiple RDATE together with RRULE directives. #716

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.

Sign up for GitHub

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

Open
rotdrop wants to merge 3 commits into
base: master
Choose a base branch
from
+152 −46
Open

Support concurrency of multiple RDATE together with RRULE directives. #716

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8b995f4
Support concurrency of multiple RDATE together with RRULE directives.
rotdrop Jun 20, 2025
cfb111f
Cleanup and optimize a bit.
rotdrop Jun 20, 2025
aeccb21
Fix yet unsupported "RDATE before DTSTART". This has to be supported.
rotdrop Jun 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 68 additions & 20 deletions lib/Recur/EventIterator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,18 +163,26 @@ public function __construct($input, ?string $uid = null, ?\DateTimeZone $timeZon
$this->eventDuration = 0;
}

$this->recurIterators = [];
if (isset($this->masterEvent->RRULE)) {
foreach ($this->masterEvent->RRULE as $rRule) {
$this->recurIterators[] = new RRuleIterator(
$this->masterEvent->RRULE->getParts(),
$this->startDate
);
}
$isRecurring = true;
}
if (isset($this->masterEvent->RDATE)) {
$this->recurIterator = new RDateIterator(
$this->masterEvent->RDATE->getParts(),
$this->startDate
);
} elseif (isset($this->masterEvent->RRULE)) {
$this->recurIterator = new RRuleIterator(
$this->masterEvent->RRULE->getParts(),
$this->startDate
);
} else {
$this->recurIterator = new RRuleIterator(
foreach ($this->masterEvent->RDATE as $rDate) {
$this->recurIterators[] = new RDateIterator(
$rDate->getParts(),
$this->startDate,
);
}
Comment on lines +177 to +182
Copy link
Contributor

@SebastianKrupinski SebastianKrupinski Nov 27, 2025

Choose a reason for hiding this comment

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

Instead of creating an array of iterators would it be easier to combine all the rdate instances in to a single iterator?

$rdateValues = [];
foreach ($this->masterEvent->RDATE as $rdate) {
    $rdateValues = array_merge($rdateValues, $rdate->getParts());
}
$this->recurIterator = new RDateIterator(
    $this->masterEvent->RDATE->getParts(),
    $rdateValues,
    $this->startDate
);

This would reduce the complexity of iterating through RRULE and RDATE significantly, you would only need to rewind, forward, and check 2 iterators, instead of looping through an array of iterators.

Copy link
Author

@rotdrop rotdrop Nov 27, 2025

Choose a reason for hiding this comment

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

Did not look into this for a long time now. I am no longer using upstream Sabre, but rather my own forks with my bug fixes, due to lack of responsiveness of the maintainers of Sabre. No flame intended, but we all have only a limited amount of time available, this applies as well to the maintainers of a software package as to potential contributors (which might be at the same time over-loaded maintainers of other software packages). I'll post a more useful response as soon as I find the time to look into your suggestions.

}
if (empty($this->recurIterators)) {
$this->recurIterators[] = new RRuleIterator(
[
'FREQ' => 'DAILY',
'COUNT' => 1,
Expand Down Expand Up @@ -313,7 +321,9 @@ public function valid(): bool
#[\ReturnTypeWillChange]
public function rewind(): void
{
$this->recurIterator->rewind();
foreach ($this->recurIterators as $iterator) {
$iterator->rewind();
}
// re-creating overridden event index.
$index = [];
foreach ($this->overriddenEvents as $key => $event) {
Expand All @@ -329,6 +339,15 @@ public function rewind(): void
$this->nextDate = null;
$this->currentDate = clone $this->startDate;

$this->currentCandidates = [];
foreach ($this->recurIterators as $index => $iterator) {
if (!$iterator->valid()) {
continue;
}
$this->currentCandidates[$index] = $iterator->current()->getTimeStamp();
}
asort($this->currentCandidates);

$this->next();
}

Expand All @@ -351,13 +370,30 @@ public function next(): void
// We need to do this until we find a date that's not in the
// exception list.
do {
if (!$this->recurIterator->valid()) {
if (empty($this->currentCandidates)) {
$nextDate = null;
break;
}
$nextDate = $this->recurIterator->current();
$this->recurIterator->next();
} while (isset($this->exceptions[$nextDate->getTimeStamp()]));
$nextIndex = array_key_first($this->currentCandidates);
$nextDate = $this->recurIterators[$nextIndex]->current();
$nextStamp = $this->currentCandidates[$nextIndex];

// advance all iterators which match the current timestamp
foreach ($this->currentCandidates as $index => $stamp) {
if ($stamp > $nextStamp) {
break;
}
$iterator = $this->recurIterators[$index];
$iterator->next();
if ($iterator->valid()) {
$this->currentCandidates[$index] = $iterator->current()->getTimeStamp();
asort($this->currentCandidates);
} else {
unset($this->currentCandidates[$index]);
// resort not neccessary
}
}
} while (isset($this->exceptions[$nextStamp]));
}

// $nextDate now contains what rrule thinks is the next one, but an
Expand Down Expand Up @@ -405,15 +441,27 @@ public function fastForward(\DateTimeInterface $dateTime): void
*/
public function isInfinite(): bool
{
return $this->recurIterator->isInfinite();
foreach ($this->recurIterators as $iterator) {
if ($iterator->isInfinite()) {
return true;
}
}
return false;
}

/**
* RRULE parser.
* Array of RRULE parsers.
*
* @var array<int, RRuleIterator|RDateIterator>
*/
protected array $recurIterators;

/**
* Array of current candidate timestamps.
*
* @var RRuleIterator|RDateIterator
* @var array<int, int>
*/
protected \Iterator $recurIterator;
protected array $currentCandidates;

/**
* The duration, in seconds, of the master event.
Expand Down
49 changes: 23 additions & 26 deletions lib/Recur/RDateIterator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

namespace Sabre\VObject\Recur;

use Iterator;
use Sabre\VObject\DateTimeParser;
use Sabre\VObject\InvalidDataException;

Expand Down Expand Up @@ -30,7 +29,10 @@ public function __construct($rrule, \DateTimeInterface $start)
{
$this->startDate = $start;
$this->parseRDate($rrule);
$this->currentDate = clone $this->startDate;
array_unshift($this->dates, \DateTimeImmutable::createFromInterface($this->startDate));
sort($this->dates);
$this->dates = array_values($this->dates);
$this->rewind();
}

/* Implementation of the Iterator interface {{{ */
Expand All @@ -41,8 +43,14 @@ public function current(): ?\DateTimeInterface
if (!$this->valid()) {
return null;
}

return clone $this->currentDate;
if (is_string($this->dates[$this->counter])) {
$this->dates[$this->counter] =
DateTimeParser::parse(
$this->dates[$this->counter],
$this->startDate->getTimezone()
);
}
return $this->dates[$this->counter];
}

/**
Expand All @@ -61,7 +69,7 @@ public function key(): int
#[\ReturnTypeWillChange]
public function valid(): bool
{
return $this->counter <= count($this->dates);
return $this->counter < count($this->dates);
}

/**
Expand All @@ -70,7 +78,6 @@ public function valid(): bool
#[\ReturnTypeWillChange]
public function rewind(): void
{
$this->currentDate = clone $this->startDate;
$this->counter = 0;
}

Expand All @@ -83,15 +90,6 @@ public function rewind(): void
public function next(): void
{
++$this->counter;
if (!$this->valid()) {
return;
}

$this->currentDate =
DateTimeParser::parse(
$this->dates[$this->counter - 1],
$this->startDate->getTimezone()
);
}

/* End of Iterator implementation }}} */
Expand All @@ -112,7 +110,7 @@ public function isInfinite(): bool
*/
public function fastForward(\DateTimeInterface $dt): void
{
while ($this->valid() && $this->currentDate < $dt) {
while ($this->valid() && $this->current() < $dt) {
$this->next();
}
}
Expand All @@ -124,12 +122,6 @@ public function fastForward(\DateTimeInterface $dt): void
*/
protected \DateTimeInterface $startDate;

/**
* The date of the current iteration. You can get this by calling
* ->current().
*/
protected \DateTimeInterface $currentDate;

/**
* The current item in the list.
*
Expand All @@ -143,15 +135,20 @@ public function fastForward(\DateTimeInterface $dt): void
* This method receives a string from an RRULE property, and populates this
* class with all the values.
*
* @param string|array $rdate
* @param string|array $rrule
*/
protected function parseRDate($rdate): void
protected function parseRDate($rdate)
{
if (is_string($rdate)) {
$rdate = explode(',', $rdate);
}

$this->dates = $rdate;
$this->dates = array_map(
fn(string $dateString) => DateTimeParser::parse(
$dateString,
$this->startDate->getTimezone()
),
$rdate
);
}

/**
Expand Down
61 changes: 61 additions & 0 deletions tests/VObject/Recur/EventIterator/MultipleRDateRRuleTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
<?php

namespace Sabre\VObject\Recur\EventIterator;

use PHPUnit\Framework\TestCase;
use Sabre\VObject\Component\VCalendar;
use Sabre\VObject\Reader;

/**
* This is a unit test for Issue #53.
*/
class MultipleRDateRRuleTest extends TestCase
{
public function testExpand(): void
{
$input = <<<ICS
BEGIN:VCALENDAR
VERSION:2.0
BEGIN:VEVENT
UID:2CD5887F7CF4600F7A3B1F8065099E40-240BDA7121B61224
DTSTAMP;VALUE=DATE-TIME:20151014T110604Z
CREATED;VALUE=DATE-TIME:20151014T110245Z
LAST-MODIFIED;VALUE=DATE-TIME:20151014T110541Z
DTSTART;VALUE=DATE-TIME;TZID=Europe/Berlin:20151025T020000
DTEND;VALUE=DATE-TIME;TZID=Europe/Berlin:20151025T013000
SUMMARY:Test
SEQUENCE:2
RRULE:FREQ=DAILY;UNTIL=20151027T225959Z;INTERVAL=1
RDATE;VALUE=DATE-TIME;TZID=Europe/Berlin:20151018T020000,20151020T020000
RDATE;VALUE=DATE-TIME;TZID=Europe/Berlin:20151015T020000,20151017T020000
TRANSP:OPAQUE
CLASS:PUBLIC
END:VEVENT
END:VCALENDAR
ICS;

$vcal = Reader::read($input);
self::assertInstanceOf(VCalendar::class, $vcal);

$vcal = $vcal->expand(new \DateTime('2015-01-01'), new \DateTime('2015-12-01'));

$result = iterator_to_array($vcal->VEVENT);

$utc = new \DateTimeZone('UTC');
$expected = [
new \DateTimeImmutable('2015-10-15', $utc),
new \DateTimeImmutable('2015-10-17', $utc),
new \DateTimeImmutable('2015-10-18', $utc),
new \DateTimeImmutable('2015-10-20', $utc),
new \DateTimeImmutable('2015-10-25T01:00:00.000000+0000', $utc),
new \DateTimeImmutable('2015-10-26T01:00:00.000000+0000', $utc),
new \DateTimeImmutable('2015-10-27T01:00:00.000000+0000', $utc),
];

$result = array_map(function ($ev) {return $ev->DTSTART->getDateTime(); }, $result);

self::assertCount(7, $result);

self::assertEquals($expected, $result);
}
}