Skip to content

Commit

Permalink
fix: added fractional offset support (#685)
Browse files Browse the repository at this point in the history
  • Loading branch information
rharshit82 authored Sep 25, 2023
1 parent 0317684 commit ce78478
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 29 deletions.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,10 @@ Parameter Based
- `onTick` - [REQUIRED] - The function to fire at the specified time. If an `onComplete` callback was provided, `onTick` will receive it as an argument. `onTick` may call `onComplete` when it has finished its work.
- `onComplete` - [OPTIONAL] - A function that will fire when the job is stopped with `job.stop()`, and may also be called by `onTick` at the end of each run.
- `start` - [OPTIONAL] - Specifies whether to start the job just before exiting the constructor. By default this is set to false. If left at default you will need to call `job.start()` in order to start the job (assuming `job` is the variable you set the cronjob to). This does not immediately fire your `onTick` function, it just gives you more control over the behavior of your jobs.
- `timeZone` - [OPTIONAL] - Specify the time zone for the execution. This will modify the actual time relative to your time zone. If the time zone is invalid, an error is thrown. By default (if this is omitted) the local time zone will be used. You can check all time zones available at [Moment Timezone Website](http://momentjs.com/timezone/). Probably don't use both `timeZone` and `utcOffset` together or weird things may happen.
- `timeZone` - [OPTIONAL] - Specify the time zone for the execution. This will modify the actual time relative to your time zone. If the time zone is invalid, an error is thrown. By default (if this is omitted) the local time zone will be used. You can check the various time zones format accepted in the [Luxon documentation](https://github.com/moment/luxon/blob/master/docs/zones.md#specifying-a-zone). Note: This parameter supports minutes offsets, e.g. `UTC+5:30`. **Warning**: Probably don't use both `timeZone` and `utcOffset` together or weird things may happen.
- `context` - [OPTIONAL] - The context within which to execute the onTick method. This defaults to the cronjob itself allowing you to call `this.stop()`. However, if you change this you'll have access to the functions and values within your context object.
- `runOnInit` - [OPTIONAL] - This will immediately fire your `onTick` function as soon as the requisite initialization has happened. This option is set to `false` by default for backwards compatibility.
- `utcOffset` - [OPTIONAL] - This allows you to specify the offset of your time zone rather than using the `timeZone` param. This should be an integer amount representing the number of minutes offset (like `120` for +2 hours or `-90` for -1.5 hours) Probably don't use both `timeZone` and `utcOffset` together or weird things may happen.
- `utcOffset` - [OPTIONAL] - This allows you to specify the offset of your time zone rather than using the `timeZone` param. This should be an integer representing the number of minutes offset (like `120` for +2 hours or `-90` for -1.5 hours). **Warning**: Minutes offsets < 60 and >-60 will be treated as an offset in hours. This means a minute offset of `30` means an offset of +30 hours. Use the `timeZone` param in this case. This behavior [is planned to be removed in V3](https://github.com/kelektiv/node-cron/pull/685#issuecomment-1676417917). **Warning**: Probably don't use both `timeZone` and `utcOffset` together or weird things may happen.
- `unrefTimeout` - [OPTIONAL] - If you have code that keeps the event loop running and want to stop the node process when that finishes regardless of the state of your cronjob, you can do so making use of this parameter. This is off by default and cron will run as if it needs to control the event loop. For more information take a look at [timers#timers_timeout_unref](https://nodejs.org/api/timers.html#timers_timeout_unref) from the NodeJS docs.
- `start` - Runs your job.
- `stop` - Stops your job.
Expand Down
23 changes: 16 additions & 7 deletions lib/time.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,17 +152,26 @@ function CronTime(luxon) {
}

if (typeof this.utcOffset !== 'undefined') {
let offset =
const offsetHours = parseInt(
this.utcOffset >= 60 || this.utcOffset <= -60
? this.utcOffset / 60
: this.utcOffset;
offset = parseInt(offset);
: this.utcOffset
);

const offsetMins =
this.utcOffset >= 60 || this.utcOffset <= -60
? Math.abs(this.utcOffset - offsetHours * 60)
: 0;
const offsetMinsStr = offsetMins >= 10 ? offsetMins : '0' + offsetMins;

let utcZone = 'UTC';
if (offset < 0) {
utcZone += offset;
} else if (offset > 0) {
utcZone += `+${offset}`;

if (parseInt(this.utcOffset) < 0) {
utcZone += `${
offsetHours === 0 ? '-0' : offsetHours
}:${offsetMinsStr}`;
} else {
utcZone += `+${offsetHours}:${offsetMinsStr}`;
}

date = date.setZone(utcZone);
Expand Down
102 changes: 94 additions & 8 deletions tests/cron.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ describe('cron', () => {
});

describe('with timezone', () => {
it('should run a job using cron syntax', () => {
it('should run a job using cron syntax with a timezone', () => {
const clock = sinon.useFakeTimers();
const callback = jest.fn();
const luxon = require('luxon');
Expand Down Expand Up @@ -436,6 +436,46 @@ describe('cron', () => {
expect(callback).toHaveBeenCalledTimes(1);
});

it('should run a job using cron syntax with a "UTC+HH:mm" offset as timezone', () => {
const clock = sinon.useFakeTimers();
const callback = jest.fn();
const luxon = require('luxon');

// Current time
const d = luxon.DateTime.local();

// Current time with zone offset
let zone = 'UTC+5:30';
let t = luxon.DateTime.local().setZone(zone);

// If current offset is UTC+5:30, switch to UTC+6:30..
if (t.hour === d.hour && t.minute === d.minute) {
zone = 'UTC+6:30';
t = t.setZone(zone);
}
expect(`${d.hour}:${d.minute}`).not.toBe(`${t.hour}:${t.minute}`);

// If t = 59s12m then t.setSeconds(60)
// becomes 00s13m so we're fine just doing
// this and no testRun callback.
t = t.plus({ seconds: 1 });
// Run a job designed to be executed at a given
// time in `zone`, making sure that it is a different
// hour than local time.
const job = new cron.CronJob(
t.second + ' ' + t.minute + ' ' + t.hour + ' * * *',
callback,
null,
true,
zone
);

clock.tick(1000);
clock.restore();
job.stop();
expect(callback).toHaveBeenCalledTimes(1);
});

it('should run a job using a date', () => {
const luxon = require('luxon');
let zone = 'America/Chicago';
Expand Down Expand Up @@ -828,18 +868,64 @@ describe('cron', () => {
expect(callback).toHaveBeenCalledTimes(1);
});

it('should run a job using cron syntax with numeric format utcOffset with minute support', () => {
const clock = sinon.useFakeTimers();
const callback = jest.fn();
const luxon = require('luxon');
// Current time
const t = luxon.DateTime.local();

/**
* in order to avoid the minute offset being treated as hours (when `-60 < utcOffset < 60`) regardless of the local timezone,
* and the maximum possible offset being +14:00, we simply add 80 minutes to that offset.
* this implicit & undocumented behavior is planned to be removed in V3 anyway:
* https://github.com/kelektiv/node-cron/pull/685#issuecomment-1676417917
*/
const minutesOffset = 14 * 60 + 80; // 920

// UTC Offset decreased by minutesOffset
const utcOffset = t.offset - minutesOffset;

const job = new cron.CronJob(
t.second + ' ' + t.minute + ' ' + t.hour + ' * * *',
callback,
null,
true,
null,
null,
null,
utcOffset
);

// tick 1 sec before minutesOffset
clock.tick(1000 * minutesOffset * 60 - 1);
expect(callback).toHaveBeenCalledTimes(0);

clock.tick(1);
clock.restore();
job.stop();
expect(callback).toHaveBeenCalledTimes(1);
});

/**
* this still works implicitly (without minute support) because the string conversion
* to integer removes everything after the colon, i.e. '(+/-)HH:mm' becomes (+/-)HH,
* but this is an undocumented behavior that will be removed in V3:
* https://github.com/kelektiv/node-cron/pull/685#issuecomment-1676394391
*/
it('should run a job using cron syntax with string format utcOffset', () => {
const clock = sinon.useFakeTimers();
const callback = jest.fn();
const luxon = require('luxon');
// Current time
const t = luxon.DateTime.local();
// UTC Offset decreased by an hour (string format '(+/-)HH:mm')
const utcOffset = t.offset - 60;
let utcOffsetString = utcOffset > 0 ? '+' : '-';
utcOffsetString += ('0' + Math.floor(Math.abs(utcOffset) / 60)).slice(-2);
utcOffsetString += ':';
utcOffsetString += ('0' + (utcOffset % 60)).slice(-2);
// We support only HH support in offset as we support string offset in Timezone.
const minutesOffset = t.offset - Math.floor((t.offset - 60) / 60) * 60;
const utcOffset = t.offset - minutesOffset;
const utcOffsetString = `${utcOffset > 0 ? '+' : '-'}${(
'0' + Math.floor(Math.abs(utcOffset) / 60)
).slice(-2)}:${('0' + (utcOffset % 60)).slice(-2)}`;

const job = new cron.CronJob(
t.second + ' ' + t.minute + ' ' + t.hour + ' * * *',
Expand All @@ -852,8 +938,8 @@ describe('cron', () => {
utcOffsetString
);

// tick 1 sec before an hour
clock.tick(1000 * 60 * 60 - 1);
// tick 1 sec before minutesOffset
clock.tick(1000 * 60 * minutesOffset - 1);
expect(callback).toHaveBeenCalledTimes(0);

// tick 1 sec
Expand Down
12 changes: 0 additions & 12 deletions tests/crontime.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -648,18 +648,6 @@ describe('crontime', () => {
clock.restore();
});

it('should accept 4 as a valid UTC offset', () => {
const clock = sinon.useFakeTimers();

const cronTime = new cron.CronTime('0 11 * * *', null, 5);
const expected = luxon.DateTime.local().plus({ hours: 6 }).toSeconds();
const actual = cronTime.sendAt().toSeconds();

expect(actual).toEqual(expected);

clock.restore();
});

it('should detect real date in the past', () => {
const clock = sinon.useFakeTimers();

Expand Down

0 comments on commit ce78478

Please sign in to comment.