Skip to content

Commit

Permalink
[RAM] Remove third party RRule library, replace with own timezone-com…
Browse files Browse the repository at this point in the history
…pliant lib (#152873)

## Summary

Closes #152630

~Adds a fix for the weird UTC-but-not-really expected inputs in
rrule.js~

This PR removes the third-party `rrule` package and replaces it with
`@kbn/rrule`.

The third party RRule library's functions produced different results
depending on what system timezone you ran it in. It would output local
timestamps in UTC, making it impossible to do reliable math on them.
It's now replaced with our own library that passes all of our own tests
for the limited cross-section of the RRule spec that we need to support.
It's possible that it wouldn't stand up to the rigor of more complex
RRule queries, but it supports the ones that our Recurrence Scheduler UI
supports just fine.

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
Zacqary and kibanamachine authored Jul 2, 2023
1 parent dea3423 commit e31ede2
Show file tree
Hide file tree
Showing 47 changed files with 1,640 additions and 285 deletions.
1 change: 1 addition & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,7 @@ examples/response_stream @elastic/ml-ui
packages/kbn-rison @elastic/kibana-operations
x-pack/plugins/rollup @elastic/platform-deployment-management
examples/routing_example @elastic/kibana-core
packages/kbn-rrule @elastic/response-ops
packages/kbn-rule-data-utils @elastic/security-detections-response @elastic/actionable-observability @elastic/response-ops
x-pack/plugins/rule_registry @elastic/response-ops @elastic/actionable-observability
x-pack/plugins/runtime_fields @elastic/platform-deployment-management
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -555,6 +555,7 @@
"@kbn/rison": "link:packages/kbn-rison",
"@kbn/rollup-plugin": "link:x-pack/plugins/rollup",
"@kbn/routing-example-plugin": "link:examples/routing_example",
"@kbn/rrule": "link:packages/kbn-rrule",
"@kbn/rule-data-utils": "link:packages/kbn-rule-data-utils",
"@kbn/rule-registry-plugin": "link:x-pack/plugins/rule_registry",
"@kbn/runtime-fields-plugin": "link:x-pack/plugins/runtime_fields",
Expand Down Expand Up @@ -965,7 +966,6 @@
"require-in-the-middle": "^6.0.0",
"reselect": "^4.1.6",
"rison-node": "1.0.2",
"rrule": "2.6.4",
"rxjs": "^7.5.5",
"safe-squel": "^5.12.5",
"seedrandom": "^3.0.5",
Expand Down
70 changes: 70 additions & 0 deletions packages/kbn-rrule/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
# @kbn/rrule

A rewrite of [rrule.js](https://github.com/jakubroztocil/rrule) using `moment-timezone` for timezone support. Ensures that we always get the same outputs no matter what local timezone the executing system is set to.

Differences from library on Github:

- It is **recommended** to generate input Dates from UTC timestamps, but not required. This implementation will perform calculations on inputted Dates accurate to their corresponding Unix timestamps.
- Timezones IDs are required. They're very important for dealing with things like day-of-week changes or DST.
- `inc` argument from `between`, `before`, `after` is removed, and is computed as if it were `true`
- SECONDLY frequency is not implemented.
- This implementation may not accurately support the entire [iCalendar RRULE RFC](https://www.rfc-editor.org/rfc/rfc5545). It is known to work for common scenarios configurable in the Recurrence Scheduler UI, plus some other more complicated ones. See `rrule.test.ts` for known working configurations.

Known not to work are mostly edge cases:

- Manually configuring `setpos` with any frequency besides `MONTHLY`
- `wkst` doesn't seem to have an effect on anything (I was also unable to get it to affect anything in the original library though)
- Setting `byyearday` on anything besides `Frequency.YEARLY`, setting `bymonthday` on anything besides `MONTHLY`, and other similar odd situations

## Constructor

Create an RRule with the following options:

```ts
new RRule({
dtstart: Date; // Recommended to generate this from a UTC timestamp, but this impl
tzid: string; // Takes a Moment.js timezone string. Recommended to use a country and city for DST accuracy, e.g. America/Phoenix and America/Denver are both in Mountain time but Phoenix doesn't observe DST
freq?: Frequency; // Defaults to YEARLY
interval?: number; // Every x freq, e.g. 1 and YEARLY is every 1 year, 2 and WEEKLY is every 2 weeks
until?: Date; // Recur until this date
count?: number; // Number of times this rule should recur until it stops
wkst?: Weekday | number; // Start of week, defaults to Monday
// The following, if not provided, will be automatically derived from the dtstart
byweekday?: Weekday[] | string[]; // Day(s) of the week to recur, OR nth-day-of-month strings, e.g. "+2TU" second Tuesday of month, "-1FR" last Friday of the month, which will get internally converted to a byweekday/bysetpos combination
bysetpos?: number[]; // Positive or negative integer affecting nth day of the month, eg -2 combined with byweekday of FR is 2nd to last Friday of the month. Best not to set this manually and just use byweekday.
byyearday?: number[]; // Day(s) of the year that this rule should recur, e.g. 32 is Feb 1. Respects leap years.
bymonth?: number[]; // Month(s) of the year that this rule should recur
bymonthday?: number[]; // Day(s) of the momth to recur
byhour?: number[]; // Hour(s) of the day to recur
byminute?: number[]; // Minute(s) of the hour to recur
bysecond?: number[]; // Seconds(s) of the day to recur
});
```

## Methods

### `RRule.prototype.all(limit?: number)`

returns `Date[] & { hasMore?: boolean}`

Returns an array all dates matching the rule. By default, limited to 10000 iterations. Pass something to `limit` to change this.

If it hits the limit, the array will come back with the property `hasMore: true`.

### `RRule.prototype.before(dt: Date)`

returns `Date | null`

Returns the last recurrence before `dt`, or `null` if there is none.

### RRule.prototype.after(dt: Date)`

returns `Date | null`

Returns the last recurrence after `dt`, or `null` if there is none.

### RRule.prototype.between(start: Date, end: Date)`

returns `Date[]`

Returns an array of all dates between `start` and `end`.
11 changes: 11 additions & 0 deletions packages/kbn-rrule/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

export { RRule, Frequency, Weekday } from './rrule';
export type { Options } from './rrule';
export declare type WeekdayStr = 'MO' | 'TU' | 'WE' | 'TH' | 'FR' | 'SA' | 'SU';
13 changes: 13 additions & 0 deletions packages/kbn-rrule/jest.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

module.exports = {
preset: '@kbn/test',
rootDir: '../..',
roots: ['<rootDir>/packages/kbn-rrule'],
};
5 changes: 5 additions & 0 deletions packages/kbn-rrule/kibana.jsonc
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"type": "shared-common",
"id": "@kbn/rrule",
"owner": "@elastic/response-ops"
}
6 changes: 6 additions & 0 deletions packages/kbn-rrule/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"name": "@kbn/rrule",
"private": true,
"version": "1.0.0",
"license": "SSPL-1.0 OR Elastic License 2.0"
}
Loading

0 comments on commit e31ede2

Please sign in to comment.