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.

Sign up for GitHub

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

fix(events-producer): serialize and deserialize custom events #2987

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

fix(events-producer): serialize and deserialize custom events #2987

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
80b1daa
fix(events-producer): stringify values in args
roggervalf Dec 28, 2024
23be50e
refactor: deserialize custom event object
roggervalf Dec 29, 2024
6b035f0
Merge branch 'master' into event-producer-fix-args-stringify
manast Jan 23, 2025
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
2 changes: 1 addition & 1 deletion src/classes/queue-events-producer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export class QueueEventsProducer extends QueueBase {
const args: any[] = ['MAXLEN', '~', maxEvents, '*', 'event', eventName];

for (const [key, value] of Object.entries(restArgs)) {
args.push(key, value);
args.push(key, JSON.stringify(value));
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we are digging ourselves more into this rabbit hole, as it would have been much simpler to just allow sending strings as payloads for the custom events and let the user serialize and deserialise as they see fit, this will never be satisfactory for all cases out there...

}

await client.xadd(key, ...args);
Expand Down
31 changes: 26 additions & 5 deletions src/classes/queue-events.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
array2obj,
clientCommandMessageReg,
isRedisInstance,
parseObjectValues,
QUEUE_EVENT_SUFFIX,
} from '../utils';
import { QueueBase } from './queue-base';
Expand Down Expand Up @@ -298,20 +299,40 @@ export class QueueEvents extends QueueBase {
id = events[i][0];
const args = array2obj(events[i][1]);

let { event, ...restArgs } = args;

//
// TODO: we may need to have a separate xtream for progress data
// to avoid this hack.
switch (args.event) {
switch (event) {
case 'active':
case 'added':
case 'cleaned':
case 'debounced': // TODO: to be removed in next breaking change
case 'deduplicated':
case 'delayed':
case 'duplicated':
case 'error':
case 'failed':
case 'paused':
case 'removed':
case 'resumed':
case 'retries-exhausted':
case 'stalled':
case 'waiting':
case 'waiting-children':
break;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

avoid to do a deserialization for these events that do not need it

Copy link
Contributor

Choose a reason for hiding this comment

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

Fastest here would be to use a Set object that is global for this module and that provides fast check for membership.

case 'progress':
args.data = JSON.parse(args.data);
restArgs.data = JSON.parse(restArgs.data);
break;
case 'completed':
args.returnvalue = JSON.parse(args.returnvalue);
restArgs.returnvalue = JSON.parse(restArgs.returnvalue);
break;
default:
restArgs = parseObjectValues(restArgs);
break;
}

const { event, ...restArgs } = args;

if (event === 'drained') {
this.emit(event, id);
} else {
Expand Down
47 changes: 46 additions & 1 deletion tests/test_events.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1269,7 +1269,7 @@ describe('events', function () {
});

describe('when publishing custom events', function () {
it('emits waiting when a job has been added', async () => {
it('emits custom event', async () => {
const queueName2 = `test-${v4()}`;
const queueEventsProducer = new QueueEventsProducer(queueName2, {
connection,
Expand Down Expand Up @@ -1311,5 +1311,50 @@ describe('events', function () {
await queueEvents2.close();
await removeAllQueueData(new IORedis(redisHost), queueName2);
});

describe('when published event is an object', function () {
it('deserialize event', async () => {
const queueName2 = `test-${v4()}`;
const queueEventsProducer = new QueueEventsProducer(queueName2, {
connection,
prefix,
});
const queueEvents2 = new QueueEvents(queueName2, {
autorun: false,
connection,
prefix,
lastEventId: '0-0',
});
await queueEvents2.waitUntilReady();

interface CustomListener extends QueueEventsListener {
example: (args: { custom: { foo: string } }, id: string) => void;
}
const customEvent = new Promise<void>(resolve => {
queueEvents2.on<CustomListener>('example', async ({ custom }) => {
await delay(250);
await expect(custom.foo).to.be.equal('value');
resolve();
});
});

interface CustomEventPayload {
eventName: string;
custom: { foo: string };
}

await queueEventsProducer.publishEvent<CustomEventPayload>({
eventName: 'example',
custom: { foo: 'value' },
});

queueEvents2.run();
await customEvent;

await queueEventsProducer.close();
await queueEvents2.close();
await removeAllQueueData(new IORedis(redisHost), queueName2);
});
});
});
});
Loading