Forráskód Böngészése

fix(core): Fix circular reference error in email sending job

Fixes to #3277
Michael Bromley 1 éve
szülő
commit
02bcdba6ee

+ 11 - 0
packages/core/e2e/entity-serialization.e2e-spec.ts

@@ -214,6 +214,17 @@ describe('Entity serialization', () => {
         assertCanBeSerialized(shippingLine);
     });
 
+    it('serialize Order with nested ShippingMethod', async () => {
+        const ctx = await createCtx();
+        const order = await server.app
+            .get(OrderService)
+            .findOne(ctx, 1, ['lines', 'shippingLines.shippingMethod']);
+
+        expect(order).not.toBeNull();
+        expect(order instanceof Order).toBe(true);
+        assertCanBeSerialized(order);
+    });
+
     function assertCanBeSerialized(entity: any) {
         try {
             const json = JSON.stringify(entity);

+ 8 - 0
packages/core/src/config/config.service.ts

@@ -146,4 +146,12 @@ export class ConfigService implements VendureConfig {
         }
         return definedCustomFields;
     }
+
+    /**
+     * This is a precaution against attempting to JSON.stringify() a reference to
+     * this class, which can lead to a circular reference error.
+     */
+    protected toJSON() {
+        return {};
+    }
 }

+ 8 - 0
packages/core/src/config/shipping-method/shipping-eligibility-checker.ts

@@ -113,6 +113,14 @@ export class ShippingEligibilityChecker<
         }
         return true;
     }
+
+    /**
+     * This is a precaution against attempting to JSON.stringify() a reference to
+     * this class, which can lead to a circular reference error.
+     */
+    protected toJSON() {
+        return {};
+    }
 }
 
 /**

+ 1 - 1
packages/core/src/entity/shipping-method/shipping-method.entity.ts

@@ -102,7 +102,7 @@ export class ShippingMethod
      * This is a fix for https://github.com/vendure-ecommerce/vendure/issues/3277,
      * to prevent circular references which cause the JSON.stringify() to fail.
      */
-    protected toJSON() {
+    protected toJSON(): any {
         return omit(this, ['allCheckers', 'allCalculators'] as any);
     }
 }

+ 69 - 25
packages/core/src/job-queue/job.spec.ts

@@ -116,36 +116,80 @@ describe('Job class', () => {
                 name: 'parent',
                 child: {
                     name: 'child',
-                    parent: {
-                        child: {
-                            name: 'child',
-                            parent: {
-                                child: {
-                                    name: 'child',
-                                    parent: {
-                                        child: {
-                                            name: 'child',
-                                            parent: {
-                                                child: {
-                                                    name: 'child',
-                                                    parent: {
-                                                        child: '[max depth reached]',
-                                                        name: '[max depth reached]',
-                                                    },
-                                                },
-                                                name: 'parent',
-                                            },
-                                        },
-                                        name: 'parent',
-                                    },
-                                },
-                                name: 'parent',
+                    parent: '[circular *child.parent]',
+                },
+            });
+        });
+
+        it('handles objects with deep cycles', () => {
+            const parent = {
+                name: 'parent',
+                child1: {
+                    name: 'child1',
+                    child2: {
+                        name: 'child2',
+                        child3: {
+                            name: 'child3',
+                            child4: {
+                                name: 'child4',
+                                parent: {} as any,
+                            },
+                        },
+                    },
+                },
+            };
+            parent.child1.child2.child3.child4.parent = parent;
+
+            const job = new Job({
+                queueName: 'test',
+                data: parent,
+            });
+
+            expect(job.data).toEqual({
+                name: 'parent',
+                child1: {
+                    name: 'child1',
+                    child2: {
+                        name: 'child2',
+                        child3: {
+                            name: 'child3',
+                            child4: {
+                                name: 'child4',
+                                parent: '[circular *child1.child2.child3.child4.parent]',
                             },
                         },
-                        name: 'parent',
                     },
                 },
             });
         });
+
+        it('handles class instances with cycles', async () => {
+            class Parent {
+                name = 'parent';
+                child = new Child();
+            }
+
+            class Child {
+                name = 'child';
+                parent = undefined as Parent | undefined;
+            }
+
+            const parent = new Parent();
+            const child = parent.child;
+            child.parent = parent;
+
+            const job = new Job({
+                queueName: 'test',
+                data: parent as any,
+            });
+
+            expect(job.data).toEqual({
+                name: 'parent',
+                child: {
+                    name: 'child',
+                    parent: '[circular *child.parent]',
+                },
+            });
+        });
     });
 });

+ 48 - 16
packages/core/src/job-queue/job.ts

@@ -231,27 +231,55 @@ export class Job<T extends JobData<T> = any> {
      * already be serializable per the TS type, in practice data can slip through due to loss of
      * type safety.
      */
-    private ensureDataIsSerializable(data: any, depth = 0): any {
+    private ensureDataIsSerializable(
+        data: any,
+        depth = 0,
+        seen = new WeakMap<any, string[]>(),
+        path: string[] = [],
+    ): any {
         if (10 < depth) {
             return '[max depth reached]';
         }
-        depth++;
-        let output: any;
+        if (data === null || data === undefined) {
+            return data;
+        }
+        // Handle Date objects
         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], depth);
+        }
+
+        if (typeof data === 'object' && data !== null) {
+            const seenData = seen.get(data);
+            if (seenData && seenData.length < path.length) {
+                return `[circular *${path.join('.')}]`;
             }
-            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];
+            seen.set(data, path);
+        }
+
+        depth++;
+        let output: any;
+        if (isObject(data)) {
+            output = {};
+            // If the object has a `.toJSON()` function defined, then
+            // prefer it to any other type of serialization.
+            if (this.hasToJSONFunction(data)) {
+                output = data.toJSON();
+            } else {
+                for (const key of Object.keys(data)) {
+                    output[key] = this.ensureDataIsSerializable(
+                        (data as any)[key],
+                        depth,
+                        seen,
+                        path.concat(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];
+                        }
                     }
                 }
             }
@@ -260,11 +288,15 @@ export class Job<T extends JobData<T> = any> {
                 output = [];
             }
             data.forEach((item, i) => {
-                output[i] = this.ensureDataIsSerializable(item, depth);
+                output[i] = this.ensureDataIsSerializable(item, depth, seen, path.concat(i.toString()));
             });
         } else {
             return data;
         }
         return output;
     }
+
+    private hasToJSONFunction(obj: any): obj is { toJSON(): any } {
+        return typeof obj?.toJSON === 'function';
+    }
 }