Browse Source

fix(core): Correctly serialize job queue data payloads

Errors were arising in the EmailPlugin whereby the Order entity would be added to the job context and would be incorrectly serialized, leading to a `Cannot set property totalBeforeTax of #<Order> which has only a getter` error.
Michael Bromley 5 years ago
parent
commit
1a9ac07c1a
2 changed files with 138 additions and 1 deletions
  1. 98 0
      packages/core/src/job-queue/job.spec.ts
  2. 40 1
      packages/core/src/job-queue/job.ts

+ 98 - 0
packages/core/src/job-queue/job.spec.ts

@@ -0,0 +1,98 @@
+import { Job } from './job';
+
+describe('Job class', () => {
+    describe('ensuring job data is serializable', () => {
+        it('getters are converted to plain properties', () => {
+            class Order {
+                code = 123;
+                get totalPrice() {
+                    return 42;
+                }
+            }
+
+            const job = new Job({
+                queueName: 'test',
+                data: new Order(),
+            });
+
+            expect(job.data).toEqual({
+                code: 123,
+                totalPrice: 42,
+            });
+        });
+
+        it('getters are converted to plain properties (nested)', () => {
+            class Order {
+                code = 123;
+                get totalPrice() {
+                    return 42;
+                }
+            }
+
+            const data: any = {
+                order: new Order(),
+            };
+
+            const job = new Job({
+                queueName: 'test',
+                data,
+            });
+
+            expect(job.data).toEqual({
+                order: {
+                    code: 123,
+                    totalPrice: 42,
+                },
+            });
+        });
+
+        it('getters are converted to plain properties (nested array)', () => {
+            class Order {
+                code = 123;
+                get totalPrice() {
+                    return 42;
+                }
+            }
+
+            const data: any = {
+                orders: [new Order()],
+            };
+
+            const job = new Job({
+                queueName: 'test',
+                data,
+            });
+
+            expect(job.data).toEqual({
+                orders: [
+                    {
+                        code: 123,
+                        totalPrice: 42,
+                    },
+                ],
+            });
+        });
+
+        it('handles dates', () => {
+            const date = new Date('2020-03-01T10:00:00Z');
+
+            const job = new Job({
+                queueName: 'test',
+                data: date as any,
+            });
+
+            expect(job.data).toEqual(date.toISOString());
+        });
+
+        it('handles dates (nested)', () => {
+            const date = new Date('2020-03-01T10:00:00Z');
+
+            const job = new Job({
+                queueName: 'test',
+                data: { createdAt: date as any },
+            });
+
+            expect(job.data).toEqual({ createdAt: date.toISOString() });
+        });
+    });
+});

+ 40 - 1
packages/core/src/job-queue/job.ts

@@ -1,5 +1,6 @@
 import { JobState } from '@vendure/common/lib/generated-types';
 import { ID } from '@vendure/common/lib/shared-types';
+import { isClassInstance, isObject } from '@vendure/common/lib/shared-utils';
 
 import { JobConfig, JobData } from './types';
 
@@ -98,7 +99,7 @@ export class Job<T extends JobData<T> = any> {
 
     constructor(config: JobConfig<T>) {
         this.queueName = config.queueName;
-        this._data = config.data;
+        this._data = this.ensureDataIsSerializable(config.data);
         this.id = config.id || null;
         this._state = config.state || JobState.PENDING;
         this.retries = config.retries || 0;
@@ -188,4 +189,42 @@ export class Job<T extends JobData<T> = any> {
             listener(this);
         }
     }
+
+    /**
+     * All data in a job must be serializable. This method handles certain problem cases such as when
+     * the data is a class instance with getters. Even though technically the "data" object should
+     * already be serializable per the TS type, in practice data can slip through due to loss of
+     * type safety.
+     */
+    private ensureDataIsSerializable(data: any, output?: any): any {
+        if (data instanceof Date) {
+            return data.toISOString();
+        } else if (isObject(data)) {
+            if (!output) {
+                output = {};
+            }
+            for (const key of Object.keys(data)) {
+                output[key] = this.ensureDataIsSerializable((data as any)[key]);
+            }
+            if (isClassInstance(data)) {
+                const descriptors = Object.getOwnPropertyDescriptors(Object.getPrototypeOf(data));
+                for (const name of Object.keys(descriptors)) {
+                    const descriptor = descriptors[name];
+                    if (typeof descriptor.get === 'function') {
+                        output[name] = (data as any)[name];
+                    }
+                }
+            }
+        } else if (Array.isArray(data)) {
+            if (!output) {
+                output = [];
+            }
+            data.forEach((item, i) => {
+                output[i] = this.ensureDataIsSerializable(item);
+            });
+        } else {
+            return data;
+        }
+        return output;
+    }
 }