refactor: simlplify calculateProcessEvery (#6)

* refactor nextRunAt and processEvery

* reduce complexity of calculateProcessEvery

Co-authored-by: Aras Abbasi <a.abbasi@cognigy.com>

package.json

@@ -57,27 +57,29 @@
     "mongodb": "~3.6.2"
   },
   "devDependencies": {
-    "standard-version": "^9.0.0",
+    "@hokify/eslint-config": "^0.5.10",
     "@types/chai": "^4.2.14",
-    "@types/node": "^14.14.0",
+    "@types/debug": "^4.1.5",
+    "@types/human-interval": "^1.0.0",
     "@types/mocha": "^8.0.3",
-    "@types/sinon": "^9.0.8",
-    "ts-node": "^9.0.0",
-    "typescript": "^4.0.3",
-    "mongodb-memory-server": "^6.9.2",
-    "@hokify/eslint-config": "^0.5.10",
-    "eslint": "7.11.0",
-    "prettier": "^2.1.2",
     "@types/mongodb": "^3.5.29",
+    "@types/node": "^14.14.0",
+    "@types/sinon": "^9.0.8",
     "blanket": "1.2.3",
+    "chai": "^4.2.0",
     "coveralls": "3.1.0",
     "delay": "4.4.0",
-    "chai": "^4.2.0",
+    "eslint": "7.11.0",
     "jsdoc": "3.6.6",
     "jsdoc-template": "https://github.com/braintree/jsdoc-template",
     "mocha": "8.2.0",
     "mocha-lcov-reporter": "1.3.0",
+    "mongodb-memory-server": "^6.9.2",
+    "prettier": "^2.1.2",
     "q": "1.5.1",
-    "sinon": "9.2.0"
+    "sinon": "9.2.0",
+    "standard-version": "^9.0.0",
+    "ts-node": "^9.0.0",
+    "typescript": "^4.0.3"
   }
 }

src/index.ts

@@ -1,7 +1,6 @@
 import { EventEmitter } from 'events';
 import * as debug from 'debug';
 
-import * as humanInterval from 'human-interval';
 import { Db, FilterQuery, MongoClientOptions, SortOptionObject } from 'mongodb';
 import { Job } from './Job';
 import { JobProcessor } from './JobProcessor';
@@ -13,6 +12,7 @@ import { filterUndefined } from './utils/filterUndefined';
 import { JobPriority, parsePriority } from './utils/priority';
 import { IAgendaStatus } from './types/AgendaStatus';
 import { IJobParameters } from './types/JobParameters';
+import { calculateProcessEvery } from './utils/processEvery';
 
 const log = debug('agenda');
 
@@ -90,8 +90,7 @@ export class Agenda extends EventEmitter {
 
 		this.attrs = {
 			name: config.name || '',
-			processEvery:
-				(config.processEvery && humanInterval(config.processEvery)) || humanInterval('5 seconds'),
+			processEvery: calculateProcessEvery(config.processEvery),
 			defaultConcurrency: config.defaultConcurrency || 5,
 			maxConcurrency: config.maxConcurrency || 20,
 			defaultLockLimit: config.defaultLockLimit || 0,
@@ -158,7 +157,7 @@ export class Agenda extends EventEmitter {
 
 	processEvery(time: string | number): Agenda {
 		log('Agenda.processEvery(%d)', time);
-		this.attrs.processEvery = humanInterval(time);
+		this.attrs.processEvery = calculateProcessEvery(time);
 		return this;
 	}
 

src/utils/date.ts → src/utils/isValidDate.ts

@@ -1,6 +1,6 @@
-export function isValidDate(date: Date): boolean {
+export function isValidDate(date: unknown): date is Date {
 	// An invalid date object returns NaN for getTime() and NaN is the only
 	// object not strictly equal to itself.
 	// eslint-disable-next-line no-self-compare
-	return new Date(date).getTime() === new Date(date).getTime();
+	return date !== null && new Date(date as string).getTime() === new Date(date as string).getTime();
 }

src/utils/nextRunAt.ts

@@ -1,11 +1,10 @@
 import { CronTime } from 'cron';
 import * as moment from 'moment-timezone';
-// what's the difference to regular moment package?
 import * as humanInterval from 'human-interval';
 import * as date from 'date.js';
 import * as debug from 'debug';
-import { IJobParameters } from '../types/JobParameters';
-import { isValidDate } from './date';
+import type { IJobParameters } from '../types/JobParameters';
+import { isValidDate } from './isValidDate';
 
 const log = debug('agenda:nextRunAt');
 
@@ -15,9 +14,15 @@ const dateForTimezone = (timezoneDate: Date, timezone?: string): moment.Moment =
 		momentDate.tz(timezone);
 	}
 
-	return momentDate; // .utc(false).toDate();
+	return momentDate;
 };
 
+export function isValidHumanInterval(value: unknown): value is string {
+	const transformedValue = humanInterval(value as string);
+	// eslint-disable-next-line no-restricted-globals
+	return typeof transformedValue === 'number' && isNaN(transformedValue) === false;
+}
+
 /**
  * Internal method that computes the interval
  * @returns {undefined}
@@ -26,33 +31,38 @@ export const computeFromInterval = (attrs: IJobParameters): Date => {
 	const previousNextRunAt = attrs.nextRunAt || new Date();
 	log('[%s:%s] computing next run via interval [%s]', attrs.name, attrs._id, attrs.repeatInterval);
 	const lastRun = dateForTimezone(attrs.lastRunAt || new Date(), attrs.repeatTimezone);
-	let result: Date;
-	try {
-		const cronTime = new CronTime(attrs.repeatInterval);
-		let nextDate = cronTime._getNextDateFrom(lastRun);
-		if (
-			nextDate.valueOf() === lastRun.valueOf() ||
-			nextDate.valueOf() <= previousNextRunAt.valueOf()
-		) {
-			// Handle cronTime giving back the same date for the next run time
-			nextDate = cronTime._getNextDateFrom(
-				dateForTimezone(new Date(lastRun.valueOf() + 1000), attrs.repeatTimezone)
-			);
-		}
+	let nextRunAt: Date | null = null;
 
-		result = nextDate;
-		// Either `xo` linter or Node.js 8 stumble on this line if it isn't just ignored
-	} catch (error) {
-		// eslint-disable-line no-unused-vars
-		// Nope, humanInterval then!
-		if (!attrs.lastRunAt && humanInterval(attrs.repeatInterval)) {
-			result = new Date(lastRun.valueOf());
+	if (typeof attrs.repeatInterval === 'string') {
+		try {
+			const cronTime = new CronTime(attrs.repeatInterval);
+			let nextDate: Date = cronTime._getNextDateFrom(lastRun);
+			if (
+				nextDate.valueOf() === lastRun.valueOf() ||
+				nextDate.valueOf() <= previousNextRunAt.valueOf()
+			) {
+				// Handle cronTime giving back the same date for the next run time
+				nextDate = cronTime._getNextDateFrom(
+					dateForTimezone(new Date(lastRun.valueOf() + 1000), attrs.repeatTimezone)
+				);
+			}
+
+			nextRunAt = nextDate;
+
+			// eslint-disable-next-line no-empty
+		} catch (error) {}
+	}
+
+	if (isValidHumanInterval(attrs.repeatInterval)) {
+		if (!attrs.lastRunAt) {
+			nextRunAt = new Date(lastRun.valueOf());
 		} else {
-			result = new Date(lastRun.valueOf() + humanInterval(attrs.repeatInterval));
+			const intervalValue = humanInterval(attrs.repeatInterval) as number;
+			nextRunAt = new Date(lastRun.valueOf() + intervalValue);
 		}
 	}
 
-	if (!isValidDate(result)) {
+	if (!isValidDate(nextRunAt)) {
 		log(
 			'[%s:%s] failed to calculate nextRunAt due to invalid repeat interval',
 			attrs.name,
@@ -61,7 +71,7 @@ export const computeFromInterval = (attrs: IJobParameters): Date => {
 		throw new Error('failed to calculate nextRunAt due to invalid repeat interval');
 	}
 
-	return result;
+	return nextRunAt;
 };
 
 /**
@@ -74,14 +84,17 @@ export function computeFromRepeatAt(attrs: IJobParameters): Date {
 
 	// If you do not specify offset date for below test it will fail for ms
 	const offset = Date.now();
+
 	if (offset === date(attrs.repeatAt, offset).valueOf()) {
 		log('[%s:%s] failed to calculate repeatAt due to invalid format', attrs.name, attrs._id);
 		// this.attrs.nextRunAt = undefined;
 		// this.fail('failed to calculate repeatAt time due to invalid format');
 		throw new Error('failed to calculate repeatAt time due to invalid format');
-	} else if (nextDate.valueOf() === lastRun.valueOf()) {
+	}
+
+	if (nextDate.valueOf() === lastRun.valueOf()) {
 		return date('tomorrow at ', attrs.repeatAt);
-	} else {
-		return date(attrs.repeatAt);
 	}
+
+	return date(attrs.repeatAt);
 }

src/utils/processEvery.ts

@@ -0,0 +1,6 @@
+import * as humanInterval from 'human-interval';
+
+export function calculateProcessEvery(input: number | string = 5000): number {
+	if (typeof input === 'number') return input;
+	return (humanInterval(input) as number) || 5000;
+}