Skip to content
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.

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

Add isFinite, first & last to rrule & rruleset #422

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 9 additions & 0 deletions src/parseoptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,15 @@ export function parseOptions (options: Partial<Options>) {

if (!opts.dtstart) opts.dtstart = new Date(new Date().setMilliseconds(0))

// infinite count is handled by null
if (isNumber(opts.count)) {
if (opts.count === Number.POSITIVE_INFINITY) {
opts.count = null
} else if (!Number.isSafeInteger(opts.count)) {
throw new Error('Invalid option: Count can only be a safe integer, null, or positive infinity')
}
}

if (!isPresent(opts.wkst)) {
opts.wkst = RRule.MO.weekday
} else if (isNumber(opts.wkst)) {
Expand Down
44 changes: 44 additions & 0 deletions src/rrule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,10 @@ export default class RRule implements QueryMethods {
return this._iter(new CallbackIterResult('all', {}, iterator))
}

if (!this.isFinite()) {
throw new Error('Calling RRule.all() without an iterator on this RRule would result in an infinite loop as its recurrence set is not finite')
}

let result = this._cacheGet('all') as Date[] | false
if (result === false) {
result = this._iter(new IterResult('all', {}))
Expand Down Expand Up @@ -234,11 +238,51 @@ export default class RRule implements QueryMethods {
return result as Date
}

/**
* Returns the very first occurrence of this rrule if it exists, null otherwise
*
* @returns {Date | null}
*/
first (): Date | null {
return this.after(this.options.dtstart, true) || null
}

/**
* Returns the last occurrence of this rrule if it exists, null otherwise
* @returns {Date | null}
*/
last (): Date | null {
if (!this.isFinite()) {
return null
}

if (this.options.until) {
return this.before(this.options.until, true)
}

const allDates = this.all()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion to use this.before() instead of this.all() here with a max date (new Date(9999, 1, 1), or some other date that is large enough to be agreed upon) so that the values from .all() are not all stored into memory while computing the result.

Copy link
Author

Choose a reason for hiding this comment

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

Date has a max value which seems to be new Date(8640000000000000);, anything after that returns "Invalid Date". We could use that if it's not more expensive (which I think it isn't)

Copy link
Contributor

Choose a reason for hiding this comment

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

Yea, that or a similar value works.

Technically the Iter loop won't works/exits properly with that as the MaxDate in IterResult, since technically tooLate will never be set to true, as new Date(8640000000000001) > new Date(8640000000000000) is false.

Would just round the numbers and go with new Date(8000000000000000); to avoid that issue. Kind of doubt people are using dates that are in the year 255000+.

That said, the Javascript VM is probably going to long before it gets there if for whatever reason the iteration is that large. Would leave it up to code outside of the library to catch malicious/bad input that would let a massive amount of iterations to occur.

Copy link
Author

Choose a reason for hiding this comment

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

Ah yes you're right. But I think it could be a good occasion to fix the exit condition to take NaN dates into account! :)

return allDates[allDates.length - 1] || null
}

/**
* Returns whether the number of recurrence in this set is finite.
* @returns {boolean}
*/
isFinite (): boolean {
return this.options.until != null || this.options.count != null || this.options.interval === 0
}

/**
* Returns the number of recurrences in this set. It will have go trough
* the whole recurrence, if this hasn't been done before.
*
* If the number of recurrence in this set is infinite, Positive infinity will be returned.
*/
count (): number {
if (!this.isFinite()) {
return Number.POSITIVE_INFINITY
}

return this.all().length
}

Expand Down
53 changes: 53 additions & 0 deletions src/rruleset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,59 @@ export default class RRuleSet extends RRule {
return this._exdate.map(e => new Date(e.getTime()))
}

/**
* Returns the very first occurrence of this rrule set if it exists, null otherwise
*
* @returns {Date | null}
*/
first (): Date | null {
let earliestDate = null
for (const rrule of this._rrule) {
if (!earliestDate || rrule.options.dtstart < earliestDate) {
earliestDate = rrule.options.dtstart
}
}

for (const rdate of this._rdate) {
if (!earliestDate || rdate < earliestDate) {
earliestDate = rdate
}
}

if (earliestDate == null) {
return null
}

return this.after(earliestDate, true)
}

/**
* Returns the last occurrence of this rrule set if it exists, null otherwise
* @returns {Date | null}
*/
last (): Date | null {
if (!this.isFinite()) {
return null
}

const allDates = this.all()
return allDates[allDates.length - 1] || null
}

/**
* Returns whether the number of recurrence in this set is finite.
* @returns {boolean}
*/
isFinite (): boolean {
for (const rrule of this._rrule) {
if (!rrule.isFinite()) {
return false
}
}

return true
}

valueOf () {
let result: string[] = []

Expand Down
4 changes: 2 additions & 2 deletions test/lib/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ const assertDatesEqual = function (actual: Date | Date[], expected: Date | Date[
for (let i = 0; i < expected.length; i++) {
const act = actual[i]
const exp = expected[i]
expect(exp instanceof Date ? exp.toString() : exp).to.equal(
act.toString(), msg + (i + 1) + '/' + expected.length)
expect(act.toString()).to.equal(
exp instanceof Date ? exp.toString() : exp, msg + (i + 1) + '/' + expected.length)
}
}

Expand Down
75 changes: 74 additions & 1 deletion test/rrule.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@ describe('RRule', function () {
'FREQ=HOURLY;INTERVAL=0;BYSETPOS=1;BYDAY=MO',
'FREQ=MINUTELY;INTERVAL=0;BYSETPOS=1;BYDAY=MO',
'FREQ=SECONDLY;INTERVAL=0;BYSETPOS=1;BYDAY=MO']
.map((s) => expect(rrulestr(s).count()).to.equal(0))
.map((s) => {
expect(rrulestr(s).count()).to.equal(0);
expect(rrulestr(s).isFinite()).to.equal(true);
})
})

it('does not mutate the passed-in options object', function () {
Expand Down Expand Up @@ -156,6 +159,76 @@ describe('RRule', function () {
]
)

testRecurring('testFirst',
{
rrule: new RRule({
freq: RRule.DAILY,
dtstart: parse('19970902T090000')
}),
method: 'first'
},
[
datetime(1997, 9, 2, 9, 0)
]
)

it('testLastZeroInterval', () => {
const rrule = new RRule({
freq: RRule.DAILY,
dtstart: parse('19970902T090000'),
interval: 0
})

expect(rrule.first()).to.equal(null)
})

testRecurring('testLastFiniteCount',
{
rrule: new RRule({
freq: RRule.DAILY,
dtstart: parse('19970902T090000'),
count: 4
}),
method: 'last'
},
[
datetime(1997, 9, 5, 9, 0)
]
)

testRecurring('testLastFiniteUntil',
{
rrule: new RRule({
freq: RRule.DAILY,
dtstart: parse('19970902T090000'),
until: parse('19970910T090000')
}),
method: 'last'
},
[
datetime(1997, 9, 10, 9, 0)
]
)

it('testLastInfinite', () => {
const rrule = new RRule({
freq: RRule.DAILY,
dtstart: parse('19970902T090000')
})

expect(rrule.last()).to.equal(null)
})

it('testLastZeroInterval', () => {
const rrule = new RRule({
freq: RRule.DAILY,
dtstart: parse('19970902T090000'),
interval: 0
})

expect(rrule.last()).to.equal(null)
})

testRecurring('testYearly',
new RRule({
freq: RRule.YEARLY,
Expand Down
Loading