Skip to content

Commit

Permalink
fix(core): Keep order of nested relations during hydration (#2864) (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
jnugh authored May 31, 2024
1 parent b3c29e7 commit b325a83
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 49 deletions.
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]);
} else {
(a as any)[key] = b[key];
}
}
}
return a ?? b;
}

0 comments on commit b325a83

Please sign in to comment.