From 79d73247b013d7b3494fb760bc9e3bcabad40e49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A5l=20Edman?= Date: Sat, 27 Apr 2024 09:09:27 +0200 Subject: [PATCH] throw on invalid timer type value --- dist/activity/Activity.js | 8 +- dist/eventDefinitions/TimerEventDefinition.js | 31 +-- docs/TimerEventDefinition.md | 34 +-- src/activity/Activity.js | 4 +- src/eventDefinitions/TimerEventDefinition.js | 29 +-- test/activity/Activity-test.js | 15 ++ .../TimerEventDefinition-test.js | 92 +++---- test/feature/timers-feature.js | 236 ++++++++++-------- 8 files changed, 238 insertions(+), 211 deletions(-) diff --git a/dist/activity/Activity.js b/dist/activity/Activity.js index 806db06..36b0591 100644 --- a/dist/activity/Activity.js +++ b/dist/activity/Activity.js @@ -242,6 +242,7 @@ Object.defineProperties(Activity.prototype, { } }); Activity.prototype.activate = function activate() { + if (this[kActivated]) return; this[kActivated] = true; this.addInboundListeners(); return this._consumeInbound(); @@ -892,8 +893,7 @@ Activity.prototype._publishEvent = function publishEvent(state, content, propert }), { ...properties, type: state, - mandatory: state === 'error', - persistent: 'persistent' in properties ? properties.persistent : state !== 'stop' + mandatory: state === 'error' }); }; Activity.prototype._onStop = function onStop(message) { @@ -908,7 +908,9 @@ Activity.prototype._onStop = function onStop(message) { broker.cancel('_format-consumer'); if (this.extensions) this.extensions.deactivate((0, _messageHelper.cloneMessage)(message)); if (running) { - this._publishEvent('stop', this._createMessage()); + this._publishEvent('stop', this._createMessage(), { + persistent: false + }); } }; Activity.prototype._consumeApi = function consumeApi() { diff --git a/dist/eventDefinitions/TimerEventDefinition.js b/dist/eventDefinitions/TimerEventDefinition.js index 1b5934c..b3f20e1 100644 --- a/dist/eventDefinitions/TimerEventDefinition.js +++ b/dist/eventDefinitions/TimerEventDefinition.js @@ -9,6 +9,7 @@ var _piso = require("@0dep/piso"); const kStopped = Symbol.for('stopped'); const kTimerContent = Symbol.for('timerContent'); const kTimer = Symbol.for('timer'); +const timerTypes = ['timeDuration', 'timeDate', 'timeCycle']; function TimerEventDefinition(activity, eventDefinition) { const type = this.type = eventDefinition.type || 'TimerEventDefinition'; this.activity = activity; @@ -81,7 +82,6 @@ TimerEventDefinition.prototype.execute = function execute(executeMessage) { broker.publish('execution', 'execute.timer', (0, _messageHelper.cloneContent)(timerContent)); broker.publish('event', 'activity.timer', (0, _messageHelper.cloneContent)(timerContent)); if (this.stopped) return; - if (timerContent.timeout === undefined) return this._debug(`waiting for ${timerContent.timerType || 'signal'}`); if (timerContent.timeout <= 0) return this._completed(); const timers = this.environment.timers.register(timerContent); const delay = timerContent.timeout; @@ -222,28 +222,22 @@ TimerEventDefinition.prototype._getTimers = function getTimers(executeMessage) { expireAt: new Date(content.expireAt) }) }; - let parseErr; - for (const t of ['timeDuration', 'timeDate', 'timeCycle']) { - if (t in content) result[t] = content[t];else if (t in this) result[t] = this.environment.resolveExpression(this[t], executeMessage);else continue; + for (const timerType of timerTypes) { + if (timerType in content) result[timerType] = content[timerType];else if (timerType in this) result[timerType] = this.environment.resolveExpression(this[timerType], executeMessage);else continue; let expireAtDate, repeat; - const timerStr = result[t]; + const timerStr = result[timerType]; if (timerStr) { - try { - const { - repeat: parsedRepeat, - expireAt: parsedExpireAt - } = this.parse(t, timerStr); - repeat = parsedRepeat; - expireAtDate = parsedExpireAt; - } catch (err) { - parseErr = err; - } + const { + repeat: parsedRepeat, + expireAt: parsedExpireAt + } = this.parse(timerType, timerStr); + repeat = parsedRepeat; + expireAtDate = parsedExpireAt; } else { expireAtDate = new Date(); } - if (!expireAtDate) continue; if (!('expireAt' in result) || result.expireAt > expireAtDate) { - result.timerType = t; + result.timerType = timerType; result.expireAt = expireAtDate; result.repeat = repeat; } @@ -255,9 +249,6 @@ TimerEventDefinition.prototype._getTimers = function getTimers(executeMessage) { } else if (!Object.keys(result).length) { result.timeout = 0; } - if (!('timeout' in result) && parseErr) { - this.logger.warn(`<${this.activity.id}> failed to parse timer: ${parseErr.message}`); - } if (content.inbound && 'repeat' in content.inbound[0]) { result.repeat = content.inbound[0].repeat; } diff --git a/docs/TimerEventDefinition.md b/docs/TimerEventDefinition.md index 4ae6449..8c353fc 100644 --- a/docs/TimerEventDefinition.md +++ b/docs/TimerEventDefinition.md @@ -2,11 +2,11 @@ TimerEventDefinition behaviour. -# TimerEventDefinition events +## TimerEventDefinition events The timer event definition publish a number of events. -## `activity.timer` +### `activity.timer` Fired when the timer is started. @@ -19,7 +19,7 @@ Object with properties. A subset: - `startedAt`: timer started at date - `expireAt`: timer expires at date -## `activity.timeout` +### `activity.timeout` Fired when the timer has timed out or was cancelled. @@ -29,41 +29,41 @@ Object with `activity.timer` properties and some: - `stoppedAt`: stopped at date - `runningTime`: running for milliseconds -# `timeDuration` +## `timeDuration` -Default support for ISO8601 duration. Will set a timer (`setTimeout`) for the duration and then complete when timed out. Invalid ISI8601 duration will stall the execution and wait for cancel. +Default support for ISO8601 duration. Will set a timer (`setTimeout`) for the duration and then complete when timed out. Invalid ISI8601 duration will throw and stop the execution. -Uses [`@0dep/piso`](https://www.npmjs.com/package/@0dep/piso) to parse duration and repetitions. Consequently also [ISO8601 intervals](https://en.wikipedia.org/wiki/ISO_8601) are allowed. +Uses [`@0dep/piso`](https://www.npmjs.com/package/@0dep/piso) to parse duration and repetitions. Consequently [ISO8601 intervals](https://en.wikipedia.org/wiki/ISO_8601) are supported. -# `timeDate` +## `timeDate` -Behaves the same as `timeDuration`. Due date will timeout immediately. An invalid date will stall the execution and wait for cancel. +Behaves the same as `timeDuration`. Due date will timeout immediately. An invalid date, like `2023-02-29`, will throw and stop the execution. Uses [`@0dep/piso`](https://www.npmjs.com/package/@0dep/piso) to parse date according to [ISO8601](https://en.wikipedia.org/wiki/ISO_8601). -# `timeCycle` - -Default support for ISO8601 repeating interval. - -If another format is used, e.g. cron, the event definition will wait until cancelled. There are several modules to handle time cycles and this project tries to keep the number of dependencies to a minimum. +## `timeCycle` Time cycles are parsed with [`@0dep/piso`](https://www.npmjs.com/package/@0dep/piso) that also handles ISO8601 intervals. -# Combined `timeDuration` and `timeDate` +If another format is used, e.g. cron, you need to handle that by [extending the behavior](#set-your-own-timeout). There are several modules to handle time cycles and this project tries to keep the number of dependencies to a minimum. + +## Combined `timeDuration` and `timeDate` The shortest timeout will be picked to start the timer. -# Set your own timeout +## Set your own timeout If the parent event start message has an `expireAt` date or `timeout` positive integer property a timer will be started. See how to format these messages [here](/docs/Extension.md). -# Api +Another alternative is to override the [parse function](#timereventdefinitionparsetimertype-value). + +## Api Timer event definition api. -## `TimerEventDefinition.parse(timerType, value)` +### `TimerEventDefinition.parse(timerType, value)` Parse timer value into expire date. diff --git a/src/activity/Activity.js b/src/activity/Activity.js index 1a9c77f..4e8321c 100644 --- a/src/activity/Activity.js +++ b/src/activity/Activity.js @@ -233,6 +233,7 @@ Object.defineProperties(Activity.prototype, { }); Activity.prototype.activate = function activate() { + if (this[kActivated]) return; this[kActivated] = true; this.addInboundListeners(); return this._consumeInbound(); @@ -877,7 +878,6 @@ Activity.prototype._publishEvent = function publishEvent(state, content, propert ...properties, type: state, mandatory: state === 'error', - persistent: 'persistent' in properties ? properties.persistent : state !== 'stop', }); }; @@ -897,7 +897,7 @@ Activity.prototype._onStop = function onStop(message) { if (this.extensions) this.extensions.deactivate(cloneMessage(message)); if (running) { - this._publishEvent('stop', this._createMessage()); + this._publishEvent('stop', this._createMessage(), { persistent: false }); } }; diff --git a/src/eventDefinitions/TimerEventDefinition.js b/src/eventDefinitions/TimerEventDefinition.js index aa42d7e..71fdee7 100644 --- a/src/eventDefinitions/TimerEventDefinition.js +++ b/src/eventDefinitions/TimerEventDefinition.js @@ -5,6 +5,8 @@ const kStopped = Symbol.for('stopped'); const kTimerContent = Symbol.for('timerContent'); const kTimer = Symbol.for('timer'); +const timerTypes = ['timeDuration', 'timeDate', 'timeCycle']; + export default function TimerEventDefinition(activity, eventDefinition) { const type = (this.type = eventDefinition.type || 'TimerEventDefinition'); this.activity = activity; @@ -80,7 +82,6 @@ TimerEventDefinition.prototype.execute = function execute(executeMessage) { if (this.stopped) return; - if (timerContent.timeout === undefined) return this._debug(`waiting for ${timerContent.timerType || 'signal'}`); if (timerContent.timeout <= 0) return this._completed(); const timers = this.environment.timers.register(timerContent); @@ -219,29 +220,23 @@ TimerEventDefinition.prototype._getTimers = function getTimers(executeMessage) { ...('expireAt' in content && { expireAt: new Date(content.expireAt) }), }; - let parseErr; - for (const t of ['timeDuration', 'timeDate', 'timeCycle']) { - if (t in content) result[t] = content[t]; - else if (t in this) result[t] = this.environment.resolveExpression(this[t], executeMessage); + for (const timerType of timerTypes) { + if (timerType in content) result[timerType] = content[timerType]; + else if (timerType in this) result[timerType] = this.environment.resolveExpression(this[timerType], executeMessage); else continue; let expireAtDate, repeat; - const timerStr = result[t]; + const timerStr = result[timerType]; if (timerStr) { - try { - const { repeat: parsedRepeat, expireAt: parsedExpireAt } = this.parse(t, timerStr); - repeat = parsedRepeat; - expireAtDate = parsedExpireAt; - } catch (err) { - parseErr = err; - } + const { repeat: parsedRepeat, expireAt: parsedExpireAt } = this.parse(timerType, timerStr); + repeat = parsedRepeat; + expireAtDate = parsedExpireAt; } else { expireAtDate = new Date(); } - if (!expireAtDate) continue; if (!('expireAt' in result) || result.expireAt > expireAtDate) { - result.timerType = t; + result.timerType = timerType; result.expireAt = expireAtDate; result.repeat = repeat; } @@ -255,10 +250,6 @@ TimerEventDefinition.prototype._getTimers = function getTimers(executeMessage) { result.timeout = 0; } - if (!('timeout' in result) && parseErr) { - this.logger.warn(`<${this.activity.id}> failed to parse timer: ${parseErr.message}`); - } - if (content.inbound && 'repeat' in content.inbound[0]) { result.repeat = content.inbound[0].repeat; } diff --git a/test/activity/Activity-test.js b/test/activity/Activity-test.js index 2de6b23..9f80587 100644 --- a/test/activity/Activity-test.js +++ b/test/activity/Activity-test.js @@ -2853,6 +2853,21 @@ describe('Activity', () => { recovered.getApi().signal(); expect(recovered.counters).to.have.property('taken', 2); }); + + it('resume if not running activates activity', () => { + const activity = getActivity(undefined, SignalTaskBehaviour); + activity.resume(); + + expect(activity.isRunning).to.be.false; + }); + + it('resume on resume if not running activates activity once', () => { + const activity = getActivity(undefined, SignalTaskBehaviour); + activity.resume(); + activity.resume(); + + expect(activity.isRunning).to.be.false; + }); }); describe('evaluateOutbound()', () => { diff --git a/test/eventDefinitions/TimerEventDefinition-test.js b/test/eventDefinitions/TimerEventDefinition-test.js index 472907b..0debeba 100644 --- a/test/eventDefinitions/TimerEventDefinition-test.js +++ b/test/eventDefinitions/TimerEventDefinition-test.js @@ -144,7 +144,7 @@ describe('TimerEventDefinition', () => { }); }); - it('invalid ISO duration executes stalls execution', () => { + it('invalid ISO duration executes throws', () => { const definition = new TimerEventDefinition(event, { type: 'bpmn:TimerEventDefinition', behaviour: { @@ -161,26 +161,19 @@ describe('TimerEventDefinition', () => { { noAck: true }, ); - event.broker.subscribeTmp( - 'execution', - 'execute.error', - () => { - throw new Error('Should not throw'); - }, - { noAck: true }, - ); - - definition.execute({ - fields: {}, - content: { - executionId: 'event_1_0', - index: 0, - parent: { - id: 'bound', - executionId: 'event_1', + expect(() => { + definition.execute({ + fields: {}, + content: { + executionId: 'event_1_0', + index: 0, + parent: { + id: 'bound', + executionId: 'event_1', + }, }, - }, - }); + }); + }).to.throw(RangeError); expect(event.environment.timers.executing).to.be.empty; }); @@ -587,7 +580,7 @@ describe('TimerEventDefinition', () => { expect(event.environment.timers.executing, 'no of executing timers').to.have.length(0); }); - it('invalid date stalls', () => { + it('invalid date throws', () => { const definition = new TimerEventDefinition(event, { type: 'bpmn:TimerEventDefinition', behaviour: { @@ -614,26 +607,19 @@ describe('TimerEventDefinition', () => { { noAck: true }, ); - event.broker.subscribeTmp( - 'execution', - 'execute.error', - () => { - throw new Error('Should not throw'); - }, - { noAck: true }, - ); - - definition.execute({ - fields: {}, - content: { - executionId: 'event_1_0', - index: 0, - parent: { - id: 'bound', - executionId: 'event_1', + expect(() => { + definition.execute({ + fields: {}, + content: { + executionId: 'event_1_0', + index: 0, + parent: { + id: 'bound', + executionId: 'event_1', + }, }, - }, - }); + }); + }).to.throw(RangeError); expect(event.environment.timers.executing, 'no of executing timers').to.have.length(0); }); @@ -778,17 +764,19 @@ describe('TimerEventDefinition', () => { { noAck: true }, ); - definition.execute({ - fields: {}, - content: { - executionId: 'event_1_0', - index: 0, - parent: { - id: 'bound', - executionId: 'event_1', + expect(() => { + definition.execute({ + fields: {}, + content: { + executionId: 'event_1_0', + index: 0, + parent: { + id: 'bound', + executionId: 'event_1', + }, }, - }, - }); + }); + }).to.throw(RangeError); expect(event.environment.timers.executing, 'no of executing timers').to.have.length(0); }); @@ -1699,7 +1687,7 @@ describe('TimerEventDefinition', () => { const definition = new TimerEventDefinition(event, { type: 'bpmn:TimerEventDefinition', behaviour: { - timeCycle: '* * * * * * 5', + timeCycle: 'P1Y', }, }); @@ -1730,7 +1718,7 @@ describe('TimerEventDefinition', () => { const definition = new TimerEventDefinition(event, { type: 'bpmn:TimerEventDefinition', behaviour: { - timeCycle: '0 0/5 * 1/1 * ? *', + timeCycle: '2024-01-01/P1Y', }, }); diff --git a/test/feature/timers-feature.js b/test/feature/timers-feature.js index 3578d71..4a3af5c 100644 --- a/test/feature/timers-feature.js +++ b/test/feature/timers-feature.js @@ -943,104 +943,6 @@ Feature('Timers', () => { }); }); - Scenario('faulty timer expression', () => { - let context, definition; - after(() => { - expect(definition?.environment.timers.executing).to.have.length(0); - }); - - Given('a source with a faulty timer expression', async () => { - const source = ` - - - - - \${true === "false'} - - - - `; - - context = await testHelpers.context(source); - definition = new Definition(context, { - expressions: { resolveExpression }, - }); - }); - - let errored; - When('definition is ran', () => { - errored = definition.waitFor('error'); - definition.run(); - }); - - Then('run fails', async () => { - const err = await errored; - expect(err.content.error).to.match(/syntax/i); - }); - }); - - Scenario('timer delay does not fit into a 32-bit signed integer', () => { - before(ck.reset); - - let context, definition; - Given('a source with a timer expression of one year', async () => { - const source = ` - - - - - P1Y - - - - `; - - context = await testHelpers.context(source); - definition = new Definition(context); - }); - - let timer; - When('definition is ran', () => { - ck.freeze(2023, 4, 23); - timer = definition.waitFor('activity.timer'); - definition.run(); - }); - - Then('activity timer is emitted', async () => { - const timerEvent = await timer; - expect(timerEvent.content.expireAt.getFullYear()).to.equal(2024); - }); - - let state; - When('run is stopped and state is saved', () => { - definition.stop(); - state = definition.getState(); - }); - - Then('timers are cleared', () => { - expect(definition?.environment.timers.executing).to.have.length(0); - }); - - let end; - When('run is resumed one year later', () => { - ck.travel(new Date(2024, 4, 23)); - - definition = new Definition(context.clone()).recover(state); - - end = definition.waitFor('leave'); - - definition.resume(); - }); - - Then('run completes', () => { - return end; - }); - }); - Scenario('timer bound to user task is resumed on wait', () => { before(ck.reset); @@ -1228,4 +1130,142 @@ Feature('Timers', () => { return end; }); }); + + Scenario('faulty timer expression', () => { + let context, definition; + after(() => { + expect(definition?.environment.timers.executing).to.have.length(0); + }); + + Given('a source with a faulty timer expression', async () => { + const source = ` + + + + + \${true === "false'} + + + + `; + + context = await testHelpers.context(source); + definition = new Definition(context, { + expressions: { resolveExpression }, + }); + }); + + let errored; + When('definition is ran', () => { + errored = definition.waitFor('error'); + definition.run(); + }); + + Then('run fails', async () => { + const err = await errored; + expect(err.content.error).to.match(/syntax/i); + }); + }); + + ['2023-12-32', 'R-3/P1Y', 'P0.5YT1.5M', '2023-12-01/32'].forEach((timer) => { + Scenario(`faulty timer value "${timer}"`, () => { + let context, definition; + after(() => { + expect(definition?.environment.timers.executing).to.have.length(0); + }); + + Given('a source with a faulty timer expression', async () => { + const source = ` + + + + + \${environment.variables.timer} + + + + `; + + context = await testHelpers.context(source); + definition = new Definition(context, { + variables: { timer }, + }); + }); + + let errored; + When('definition is ran', () => { + errored = definition.waitFor('error'); + definition.run(); + }); + + Then('run fails', async () => { + const err = await errored; + expect(err.content.error).to.be.instanceof(RangeError); + }); + }); + }); + + Scenario('timer delay does not fit into a 32-bit signed integer', () => { + before(ck.reset); + + let context, definition; + Given('a source with a timer expression of one year', async () => { + const source = ` + + + + + P1Y + + + + `; + + context = await testHelpers.context(source); + definition = new Definition(context); + }); + + let timer; + When('definition is ran', () => { + ck.freeze(2023, 4, 23); + timer = definition.waitFor('activity.timer'); + definition.run(); + }); + + Then('activity timer is emitted', async () => { + const timerEvent = await timer; + expect(timerEvent.content.expireAt.getFullYear()).to.equal(2024); + }); + + let state; + When('run is stopped and state is saved', () => { + definition.stop(); + state = definition.getState(); + }); + + Then('timers are cleared', () => { + expect(definition?.environment.timers.executing).to.have.length(0); + }); + + let end; + When('run is resumed one year later', () => { + ck.travel(new Date(2024, 4, 23)); + + definition = new Definition(context.clone()).recover(state); + + end = definition.waitFor('leave'); + + definition.resume(); + }); + + Then('run completes', () => { + return end; + }); + }); });