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.

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(core): Keep order of nested relations during hydration (#2864) #2865

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { Injectable } from '@nestjs/common';
import { Type } from '@vendure/common/lib/shared-types';
import { isObject } from '@vendure/common/lib/shared-utils';
import { unique } from '@vendure/common/lib/unique';
import { SelectQueryBuilder } from 'typeorm';

Expand All @@ -14,6 +13,7 @@ import { TranslatorService } from '../translator/translator.service';
import { joinTreeRelationsDynamically } from '../utils/tree-relations-qb-joiner';

import { HydrateOptions } from './entity-hydrator-types';
import { mergeDeep } from './merge-deep';

/**
* @description
Expand Down Expand Up @@ -134,7 +134,7 @@ export class EntityHydrator {
const hydrated = await hydratedQb.getOne();
const propertiesToAdd = unique(missingRelations.map(relation => relation.split('.')[0]));
for (const prop of propertiesToAdd) {
(target as any)[prop] = this.mergeDeep((target as any)[prop], hydrated[prop]);
(target as any)[prop] = mergeDeep((target as any)[prop], hydrated[prop]);
}

const relationsWithEntities = missingRelations.map(relation => ({
Expand Down Expand Up @@ -306,51 +306,4 @@ export class EntityHydrator {
? input[0]?.hasOwnProperty('translations') ?? false
: input?.hasOwnProperty('translations') ?? false;
}

/**
* Merges properties into a target entity. This is needed for the cases in which a
* property already exists on the target, but the hydrated version also contains that
* property with a different set of properties. This prevents the original target
* entity from having data overwritten.
*/
private mergeDeep<T extends { [key: string]: any }>(a: T | undefined, b: T): T {
if (!a) {
return b;
}
if (Array.isArray(a) && Array.isArray(b) && a.length === b.length && a.length > 1) {
if (a[0].hasOwnProperty('id')) {
// If the array contains entities, we can use the id to match them up
// so that we ensure that we don't merge properties from different entities
// with the same index.
const aIds = a.map(e => e.id);
const bIds = b.map(e => e.id);
if (JSON.stringify(aIds) !== JSON.stringify(bIds)) {
// The entities in the arrays are not in the same order, so we can't
// safely merge them. We need to sort the `b` array so that the entities
// are in the same order as the `a` array.
const idToIndexMap = new Map();
a.forEach((item, index) => {
idToIndexMap.set(item.id, index);
});
b.sort((_a, _b) => {
return idToIndexMap.get(_a.id) - idToIndexMap.get(_b.id);
});
}
}
}
for (const [key, value] of Object.entries(b)) {
if (Object.getOwnPropertyDescriptor(b, key)?.writable) {
if (Array.isArray(value)) {
(a as any)[key] = value.map((v, index) =>
this.mergeDeep(a?.[key]?.[index], b[key][index]),
);
} else if (isObject(value)) {
(a as any)[key] = this.mergeDeep(a?.[key], b[key]);
} else {
(a as any)[key] = b[key];
}
}
}
return a ?? b;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import { describe, expect, it } from 'vitest';

import { Order, Sale } from '../../../entity';

import { mergeDeep } from './merge-deep';

describe('mergeDeep()', () => {
// https://github.com/vendure-ecommerce/vendure/issues/2864
it('should sync the order of sub relations', () => {
const prefetched = new Order({
lines: [
{
id: 'line1',
sales: [new Sale({ id: 'sale-of-line-1' })],
},
{
id: 'line2',
sales: [new Sale({ id: 'sale-of-line-2' })],
},
],
});

const hydrationFetched = new Order({
lines: [
{
id: 'line2',
productVariant: { id: 'variant-of-line-2' },
},
{
id: 'line1',
productVariant: { id: 'variant-of-line-1' },
},
],
});

const merged = mergeDeep(prefetched, hydrationFetched);
const line1 = merged.lines.find(l => l.id === 'line1');
const line2 = merged.lines.find(l => l.id === 'line2');

expect(line1?.sales[0].id).toBe('sale-of-line-1');
expect(line1?.productVariant?.id).toBe('variant-of-line-1');
expect(line2?.sales[0].id).toBe('sale-of-line-2');
expect(line2?.productVariant?.id).toBe('variant-of-line-2');
});
});
44 changes: 44 additions & 0 deletions packages/core/src/service/helpers/entity-hydrator/merge-deep.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { isObject } from '@vendure/common/lib/shared-utils';

/**
* Merges properties into a target entity. This is needed for the cases in which a
* property already exists on the target, but the hydrated version also contains that
* property with a different set of properties. This prevents the original target
* entity from having data overwritten.
*/
export function mergeDeep<T extends { [key: string]: any }>(a: T | undefined, b: T): T {
if (!a) {
return b;
}
if (Array.isArray(a) && Array.isArray(b) && a.length === b.length && a.length > 1) {
if (a[0].hasOwnProperty('id')) {
// If the array contains entities, we can use the id to match them up
// so that we ensure that we don't merge properties from different entities
// with the same index.
const aIds = a.map(e => e.id);
const bIds = b.map(e => e.id);
if (JSON.stringify(aIds) !== JSON.stringify(bIds)) {
// The entities in the arrays are not in the same order, so we can't
// safely merge them. We need to sort the `b` array so that the entities
// are in the same order as the `a` array.
const idToIndexMap = new Map();
a.forEach((item, index) => {
idToIndexMap.set(item.id, index);
});
b.sort((_a, _b) => {
return idToIndexMap.get(_a.id) - idToIndexMap.get(_b.id);
});
}
}
}
for (const [key, value] of Object.entries(b)) {
if (Object.getOwnPropertyDescriptor(b, key)?.writable) {
if (Array.isArray(value) || isObject(value)) {
(a as any)[key] = mergeDeep(a?.[key], b[key]);

Check warning

Code scanning / CodeQL

Prototype-polluting function Medium

Properties are copied from
b
to
here
without guarding against prototype pollution.
} else {
(a as any)[key] = b[key];

Check warning

Code scanning / CodeQL

Prototype-polluting function Medium

Properties are copied from
b
to
here
without guarding against prototype pollution.
}
}
}
return a ?? b;
}
Loading