From 7c93c0e48a16169ee1eed6018348191eae907e03 Mon Sep 17 00:00:00 2001 From: rlanvin Date: Sun, 13 Jan 2019 10:00:43 +0000 Subject: [PATCH] Rewrite the core algorithm to use a generator Drop compatibility with PHP < 5.6 Ref #43 --- .travis.yml | 6 - CHANGELOG.md | 4 +- README.md | 6 +- composer.json | 2 +- src/RRule.php | 313 ++++++++++++++++------------------------- src/RRuleInterface.php | 2 +- src/RSet.php | 50 ++----- tests/RSetTest.php | 2 + 8 files changed, 145 insertions(+), 240 deletions(-) diff --git a/.travis.yml b/.travis.yml index f839146..a2f803d 100755 --- a/.travis.yml +++ b/.travis.yml @@ -1,15 +1,9 @@ language: php php: - - 5.4 - - 5.5 - 5.6 - 7.0 - 7.1 - 7.2 -matrix: - include: - - php: 5.3 - dist: precise install: - composer install -n script: diff --git a/CHANGELOG.md b/CHANGELOG.md index eebf2cd..17b7ddb 100755 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,8 @@ # Changelog -## Unreleased +## [Unreleased] -- n/a +- Rewrite the core algorithm to use a native PHP generator, drop compability with PHP < 5.6 [#43](https://github.com/rlanvin/php-rrule/issues/43) ## [1.6.3] - 2019-01-13 diff --git a/README.md b/README.md index 31ac198..c4e68f5 100755 --- a/README.md +++ b/README.md @@ -32,7 +32,7 @@ Complete documentation and more examples are available in [the wiki](https://git ## Requirements -- PHP >= 5.3 +- PHP >= 5.6 - [intl extension](http://php.net/manual/en/book.intl.php) is recommended for `humanReadable()` but not strictly required ## Installation @@ -41,12 +41,12 @@ The recommended way is to install the lib [through Composer](http://getcomposer. Simply run `composer require rlanvin/php-rrule` for it to be automatically installed and included in your `composer.json`. -Alternatively, just add this to your `composer.json` file and then run `composer install` (you can replace `1.*` by any version selector, or even `dev-master` for the latest development version). +Alternatively, just add this to your `composer.json` file and then run `composer install` (you can replace `2.*` by any version selector, or even `dev-master` for the latest development version). ```JSON { "require": { - "rlanvin/php-rrule": "1.*" + "rlanvin/php-rrule": "2.*" } } ``` diff --git a/composer.json b/composer.json index 5f783c0..f84255b 100755 --- a/composer.json +++ b/composer.json @@ -6,7 +6,7 @@ "homepage": "https://github.com/rlanvin/php-rrule", "license": "MIT", "require": { - "php": ">=5.3.0" + "php": ">=5.6.0" }, "suggest": { "ext-intl": "Intl extension is needed for humanReadable()" diff --git a/src/RRule.php b/src/RRule.php index 7b0187d..22bd86e 100755 --- a/src/RRule.php +++ b/src/RRule.php @@ -997,52 +997,52 @@ class RRule implements RRuleInterface // Note: if cache is complete, we could probably avoid completely calling iterate() // and instead iterate directly on the $this->cache array - /** @internal */ - protected $current = 0; - /** @internal */ - protected $key = 0; + // /** @internal */ + // protected $current = 0; + // /** @internal */ + // protected $key = 0; - /** - * @internal - */ - public function rewind() - { - $this->current = $this->iterate(true); - $this->key = 0; - } + // /** + // * @internal + // */ + // public function rewind() + // { + // $this->current = $this->iterate(true); + // $this->key = 0; + // } - /** - * @internal - */ - public function current() - { - return $this->current; - } + // /** + // * @internal + // */ + // public function current() + // { + // return $this->current; + // } - /** - * @internal - */ - public function key() - { - return $this->key; - } + // /** + // * @internal + // */ + // public function key() + // { + // return $this->key; + // } - /** - * @internal - */ - public function next() - { - $this->current = $this->iterate(); - $this->key += 1; - } + // /** + // * @internal + // */ + // public function next() + // { + // $this->current = $this->iterate(); + // $this->key += 1; + // } - /** - * @internal - */ - public function valid() - { - return $this->current !== null; - } + // /** + // * @internal + // */ + // public function valid() + // { + // return $this->current !== null; + // } /////////////////////////////////////////////////////////////////////////////// // ArrayAccess interface @@ -1445,44 +1445,6 @@ class RRule implements RRuleInterface } } - // Variables for iterate() method, that will persist to allow iterate() - // to resume where it stopped. For PHP >= 5.5, these would be local variables - // inside a generator method using yield. However since we are compatible with - // PHP 5.3 and 5.4, they have to be implemented this way. - // - // The original implementation used static local variables inside the class - // method, which I think was cleaner scope-wise, but sadly this didn't work - // when multiple instances of RRule existed and are iterated at the same time - // (such as in a ruleset) - // - // DO NOT USE OUTSIDE OF iterate() - - /** @internal */ - private $_year = null; - /** @internal */ - private $_month = null; - /** @internal */ - private $_day = null; - /** @internal */ - private $_hour = null; - /** @internal */ - private $_minute = null; - /** @internal */ - private $_second = null; - - /** @internal */ - private $_dayset = null; - /** @internal */ - private $_masks = null; - /** @internal */ - private $_timeset = null; - /** @internal */ - private $_dtstart = null; - /** @internal */ - private $_total = 0; - /** @internal */ - private $_use_cache = true; - /** * This is the main method, where all of the magic happens. * @@ -1531,115 +1493,78 @@ class RRule implements RRuleInterface * @param $reset (bool) Whether to restart the iteration, or keep going * @return \DateTime|null */ - protected function iterate($reset = false) + public function getIterator() { - // for readability's sake, and because scope of the variables should be local anyway - $year = & $this->_year; - $month = & $this->_month; - $day = & $this->_day; - $hour = & $this->_hour; - $minute = & $this->_minute; - $second = & $this->_second; - $dayset = & $this->_dayset; - $masks = & $this->_masks; - $timeset = & $this->_timeset; - $dtstart = & $this->_dtstart; - $total = & $this->_total; - $use_cache = & $this->_use_cache; - - if ( $reset ) { - $this->_year = $this->_month = $this->_day = null; - $this->_hour = $this->_minute = $this->_second = null; - $this->_dayset = $this->_masks = $this->_timeset = null; - $this->_dtstart = null; - $this->_total = 0; - $this->_use_cache = true; - reset($this->cache); - } + $total = 0; + $occurrence = null; + $dtstart = null; + $dayset = null; // go through the cache first - if ( $use_cache ) { - while ( ($occurrence = current($this->cache)) !== false ) { - // echo "Cache hit\n"; - $dtstart = $occurrence; - next($this->cache); - $total += 1; - return clone $occurrence; // since DateTime is not immutable, avoid any problem - } - reset($this->cache); - // now set use_cache to false to skip the all thing on next iteration - // and start filling the cache instead - $use_cache = false; - // if the cache as been used up completely and we now there is nothing else - if ( $total === $this->total ) { - // echo "Cache used up, nothing else to compute\n"; - return null; - } - // echo "Cache used up with occurrences remaining\n"; - if ( $dtstart ) { - $dtstart = clone $dtstart; // since DateTime is not immutable, avoid any problem - // so we skip the last occurrence of the cache - if ( $this->freq === self::SECONDLY ) { - $dtstart->modify('+'.$this->interval.'second'); - } - else { - $dtstart->modify('+1second'); - } - } + foreach ( $this->cache as $occurrence ) { + yield clone $occurrence; // since DateTime is not immutable, avoid any problem + + $total += 1; } - // stop once $total has reached COUNT - if ( $this->count && $total >= $this->count ) { - $this->total = $total; - return null; + // if the cache as been used up completely and we now there is nothing else, + // we can stop the generator + if ( $total === $this->total ) { + return; // end generator + } + + if ( $occurrence ) { + $dtstart = clone $occurrence; // since DateTime is not immutable, clone to avoid any problem + // so we skip the last occurrence of the cache + if ( $this->freq === self::SECONDLY ) { + $dtstart->modify('+'.$this->interval.'second'); + } + else { + $dtstart->modify('+1second'); + } } if ( $dtstart === null ) { $dtstart = clone $this->dtstart; } - if ( $year === null ) { - if ( $this->freq === self::WEEKLY ) { - // we align the start date to the WKST, so we can then - // simply loop by adding +7 days. The Python lib does some - // calculation magic at the end of the loop (when incrementing) - // to realign on first pass. - $tmp = clone $dtstart; - $tmp->modify('-'.pymod($dtstart->format('N') - $this->wkst,7).'days'); - list($year,$month,$day,$hour,$minute,$second) = explode(' ',$tmp->format('Y n j G i s')); - unset($tmp); - } - else { - list($year,$month,$day,$hour,$minute,$second) = explode(' ',$dtstart->format('Y n j G i s')); - } - // remove leading zeros - $minute = (int) $minute; - $second = (int) $second; + if ( $this->freq === self::WEEKLY ) { + // we align the start date to the WKST, so we can then + // simply loop by adding +7 days. The Python lib does some + // calculation magic at the end of the loop (when incrementing) + // to realign on first pass. + $tmp = clone $dtstart; + $tmp->modify('-'.pymod($dtstart->format('N') - $this->wkst,7).'days'); + list($year,$month,$day,$hour,$minute,$second) = explode(' ',$tmp->format('Y n j G i s')); + unset($tmp); } + else { + list($year,$month,$day,$hour,$minute,$second) = explode(' ',$dtstart->format('Y n j G i s')); + } + // remove leading zeros + $minute = (int) $minute; + $second = (int) $second; // we initialize the timeset - if ( $timeset == null ) { - if ( $this->freq < self::HOURLY ) { - // daily, weekly, monthly or yearly - // we don't need to calculate a new timeset - $timeset = $this->timeset; + if ( $this->freq < self::HOURLY ) { + // daily, weekly, monthly or yearly + // we don't need to calculate a new timeset + $timeset = $this->timeset; + } + else { + // initialize empty if it's not going to occurs on the first iteration + if ( + ($this->freq >= self::HOURLY && $this->byhour && ! in_array($hour, $this->byhour)) + || ($this->freq >= self::MINUTELY && $this->byminute && ! in_array($minute, $this->byminute)) + || ($this->freq >= self::SECONDLY && $this->bysecond && ! in_array($second, $this->bysecond)) + ) { + $timeset = array(); } else { - // initialize empty if it's not going to occurs on the first iteration - if ( - ($this->freq >= self::HOURLY && $this->byhour && ! in_array($hour, $this->byhour)) - || ($this->freq >= self::MINUTELY && $this->byminute && ! in_array($minute, $this->byminute)) - || ($this->freq >= self::SECONDLY && $this->bysecond && ! in_array($second, $this->bysecond)) - ) { - $timeset = array(); - } - else { - $timeset = $this->getTimeSet($hour, $minute, $second); - } + $timeset = $this->getTimeSet($hour, $minute, $second); } } - // while (true) { $max_cycles = self::$REPEAT_CYCLES[$this->freq <= self::DAILY ? $this->freq : self::DAILY]; for ( $i = 0; $i < $max_cycles; $i++ ) { // 1. get an array of all days in the next interval (day, month, week, etc.) @@ -1687,7 +1612,7 @@ class RRule implements RRuleInterface $filtered_set = array(); - // filter out the days based on the BY*** rules + // filter out the days based on the BYXXX rules foreach ( $dayset as $yearday ) { if ( $this->bymonth && ! in_array($masks['yearday_to_month'][$yearday], $this->bymonth) ) { continue; @@ -1729,6 +1654,8 @@ class RRule implements RRuleInterface // if BYSETPOS is set, we need to expand the timeset to filter by pos // so we make a special loop to return while generating + // TODO this is not needed with a generator anymore + // we can yield directly within the loop if ( $this->bysetpos && $timeset ) { $filtered_set = array(); foreach ( $this->bysetpos as $pos ) { @@ -1767,48 +1694,58 @@ class RRule implements RRuleInterface // at the same time, we check the end condition and return null if // we need to stop if ( $this->bysetpos && $timeset ) { - while ( ($occurrence = current($dayset)) !== false ) { - + // while ( ($occurrence = current($dayset)) !== false ) { + foreach ( $dayset as $occurrence ) { // consider end conditions if ( $this->until && $occurrence > $this->until ) { $this->total = $total; // save total for count() cache - return null; + return; } - next($dayset); + // next($dayset); if ( $occurrence >= $dtstart ) { // ignore occurrences before DTSTART + if ( $this->count && $total >= $this->count ) { + $this->total = $total; + return; + } $total += 1; - $this->cache[] = $occurrence; - return clone $occurrence; // yield + $this->cache[] = clone $occurrence; + yield clone $occurrence; // yield } } } else { // normal loop, without BYSETPOS - while ( ($yearday = current($dayset)) !== false ) { + // while ( ($yearday = current($dayset)) !== false ) { + foreach ( $dayset as $yearday ) { $occurrence = \DateTime::createFromFormat( 'Y z', "$year $yearday", $this->dtstart->getTimezone() ); - while ( ($time = current($timeset)) !== false ) { + // while ( ($time = current($timeset)) !== false ) { + foreach ( $timeset as $time ) { $occurrence->setTime($time[0], $time[1], $time[2]); // consider end conditions if ( $this->until && $occurrence > $this->until ) { $this->total = $total; // save total for count() cache - return null; + return; } - next($timeset); + // next($timeset); if ( $occurrence >= $dtstart ) { // ignore occurrences before DTSTART + if ( $this->count && $total >= $this->count ) { + $this->total = $total; + return; + } $total += 1; - $this->cache[] = $occurrence; - return clone $occurrence; // yield + $this->cache[] = clone $occurrence; + yield clone $occurrence; // yield } } - reset($timeset); - next($dayset); + // reset($timeset); + // next($dayset); } } @@ -1876,7 +1813,7 @@ class RRule implements RRuleInterface if ( ! $found ) { $this->total = $total; // save total for count cache - return null; // stop the iterator + return; // stop the iterator } $timeset = $this->getTimeSet($hour, $minute, $second); @@ -1910,7 +1847,7 @@ class RRule implements RRuleInterface if ( ! $found ) { $this->total = $total; // save total for count cache - return null; // stop the iterator + return; // stop the iterator } $timeset = $this->getTimeSet($hour, $minute, $second); @@ -1951,7 +1888,7 @@ class RRule implements RRuleInterface if ( ! $found ) { $this->total = $total; // save total for count cache - return null; // stop the iterator + return; // stop the iterator } $timeset = $this->getTimeSet($hour, $minute, $second); @@ -1965,7 +1902,7 @@ class RRule implements RRuleInterface } $this->total = $total; // save total for count cache - return null; // stop the iterator + return; // stop the iterator } /////////////////////////////////////////////////////////////////////////////// diff --git a/src/RRuleInterface.php b/src/RRuleInterface.php index 7eaca85..0c8af1b 100755 --- a/src/RRuleInterface.php +++ b/src/RRuleInterface.php @@ -14,7 +14,7 @@ namespace RRule; /** * Common interface for RRule and RSet objects */ -interface RRuleInterface extends \Iterator, \ArrayAccess, \Countable +interface RRuleInterface extends \ArrayAccess, \Countable, \IteratorAggregate { /** * Return all the occurrences in an array of \DateTime. diff --git a/src/RSet.php b/src/RSet.php index 128cf6f..c47a075 100755 --- a/src/RSet.php +++ b/src/RSet.php @@ -585,15 +585,6 @@ class RSet implements RRuleInterface protected $exlist_heap = null; protected $exlist_iterator = null; - // local variables for iterate() (see comment in RRule about that) - - /** @internal */ - private $_previous_occurrence = null; - /** @internal */ - private $_total = 0; - /** @internal */ - private $_use_cache = 0; - /** * This method will iterate over a bunch of different iterators (rrules and arrays), * keeping the results *in order*, while never attempting to merge or sort @@ -609,34 +600,15 @@ class RSet implements RRuleInterface * @param $reset (bool) Whether to restart the iteration, or keep going * @return \DateTime|null */ - protected function iterate($reset = false) + public function getIterator() { - $previous_occurrence = & $this->_previous_occurrence; - $total = & $this->_total; - $use_cache = & $this->_use_cache; + $previous_occurrence = null; + $total = 0; - if ( $reset ) { - $this->_previous_occurrence = null; - $this->_total = 0; - $this->_use_cache = true; - reset($this->cache); - } + foreach ( $this->cache as $occurrence ) { + yield clone $occurrence; // since DateTime is not immutable, avoid any problem - // go through the cache first - if ( $use_cache ) { - while ( ($occurrence = current($this->cache)) !== false ) { - next($this->cache); - $total += 1; - return clone $occurrence; - } - reset($this->cache); - // now set use_cache to false to skip the all thing on next iteration - // and start filling the cache instead - $use_cache = false; - // if the cache as been used up completely and we now there is nothing else - if ( $total === $this->total ) { - return null; - } + $total += 1; } if ( $this->rlist_heap === null ) { @@ -645,7 +617,7 @@ class RSet implements RRuleInterface $this->rlist_iterator = new \MultipleIterator(\MultipleIterator::MIT_NEED_ANY); $this->rlist_iterator->attachIterator(new \ArrayIterator($this->rdates)); foreach ( $this->rrules as $rrule ) { - $this->rlist_iterator->attachIterator($rrule); + $this->rlist_iterator->attachIterator($rrule->getIterator()); } $this->rlist_iterator->rewind(); @@ -655,7 +627,7 @@ class RSet implements RRuleInterface $this->exlist_iterator->attachIterator(new \ArrayIterator($this->exdates)); foreach ( $this->exrules as $rrule ) { - $this->exlist_iterator->attachIterator($rrule); + $this->exlist_iterator->attachIterator($rrule->getIterator()); } $this->exlist_iterator->rewind(); } @@ -717,11 +689,11 @@ class RSet implements RRuleInterface } $total += 1; - $this->cache[] = $occurrence; - return clone $occurrence; // = yield + $this->cache[] = clone $occurrence; + yield clone $occurrence; // = yield } $this->total = $total; // save total for count cache - return null; // stop the iterator + return; // stop the iterator } } \ No newline at end of file diff --git a/tests/RSetTest.php b/tests/RSetTest.php index bb99047..be41623 100755 --- a/tests/RSetTest.php +++ b/tests/RSetTest.php @@ -190,6 +190,8 @@ class RSetTest extends TestCase 'BYDAY' => 'TU, TH', 'DTSTART' => date_create('1997-09-02 09:00') )); + $this->assertEquals(6, count($rset)); + $rset->addExdate('1997-09-04 09:00:00'); $rset->addExdate('1997-09-11 09:00:00'); $rset->addExdate('1997-09-18 09:00:00');