From 40344450006eaf0e8cc3bef885f406101b912a46 Mon Sep 17 00:00:00 2001 From: Eugene Nitsenko Date: Fri, 15 Mar 2024 16:56:40 +0100 Subject: [PATCH 01/20] fix(core): self-referencing relations `Not unique table/alias` --- .../test-plugins/list-query-plugin.ts | 29 ++++++++++++++----- .../core/e2e/list-query-builder.e2e-spec.ts | 5 ++++ .../list-query-builder/list-query-builder.ts | 12 +++++++- 3 files changed, 37 insertions(+), 9 deletions(-) diff --git a/packages/core/e2e/fixtures/test-plugins/list-query-plugin.ts b/packages/core/e2e/fixtures/test-plugins/list-query-plugin.ts index b87cf2ca73..5fa72c22f0 100644 --- a/packages/core/e2e/fixtures/test-plugins/list-query-plugin.ts +++ b/packages/core/e2e/fixtures/test-plugins/list-query-plugin.ts @@ -144,6 +144,12 @@ export class TestEntity extends VendureEntity implements Translatable, HasCustom @Column(() => TestEntityCustomFields) customFields: TestEntityCustomFields; + + @ManyToOne(() => TestEntity, (type) => type.parent) + parent: TestEntity | null; + + @Column('int', { nullable: true }) + parentId: ID | null; } @Entity() @@ -188,6 +194,7 @@ export class ListQueryResolver { .build(TestEntity, args.options, { ctx, relations: [ + 'parent', 'orderRelation', 'orderRelation.customer', 'customFields.relation', @@ -274,6 +281,7 @@ const apiExtensions = gql` nullableId: ID nullableDate: DateTime customFields: TestEntityCustomFields! + parent: TestEntity } type TestEntityList implements PaginatedList { @@ -324,9 +332,9 @@ export class ListQueryPlugin implements OnApplicationBootstrap { ) {} async onApplicationBootstrap() { - const count = await this.connection.getRepository(TestEntity).count(); + const count = await this.connection.rawConnection.getRepository(TestEntity).count(); if (count === 0) { - const testEntities = await this.connection.getRepository(TestEntity).save([ + const testEntities = await this.connection.rawConnection.getRepository(TestEntity).save([ new TestEntity({ label: 'A', description: 'Lorem ipsum', // 11 @@ -392,6 +400,11 @@ export class ListQueryPlugin implements OnApplicationBootstrap { }), ]); + // test entity with self-referencing relation without tree structure decorator + testEntities[0].parent = testEntities[1]; + testEntities[3].parent = testEntities[1]; + await this.connection.rawConnection.getRepository(TestEntity).save([testEntities[0], testEntities[3]]); + const translations: any = { A: { [LanguageCode.en]: 'apple', [LanguageCode.de]: 'apfel' }, B: { [LanguageCode.en]: 'bike', [LanguageCode.de]: 'fahrrad' }, @@ -408,7 +421,7 @@ export class ListQueryPlugin implements OnApplicationBootstrap { }; for (const testEntity of testEntities) { - await this.connection.getRepository(TestEntityPrice).save([ + await this.connection.rawConnection.getRepository(TestEntityPrice).save([ new TestEntityPrice({ price: testEntity.description.length, channelId: 1, @@ -424,7 +437,7 @@ export class ListQueryPlugin implements OnApplicationBootstrap { for (const code of [LanguageCode.en, LanguageCode.de]) { const translation = translations[testEntity.label][code]; if (translation) { - await this.connection.getRepository(TestEntityTranslation).save( + await this.connection.rawConnection.getRepository(TestEntityTranslation).save( new TestEntityTranslation({ name: translation, base: testEntity, @@ -436,13 +449,13 @@ export class ListQueryPlugin implements OnApplicationBootstrap { if (nestedData[testEntity.label]) { for (const nestedContent of nestedData[testEntity.label]) { - await this.connection.getRepository(CustomFieldRelationTestEntity).save( + await this.connection.rawConnection.getRepository(CustomFieldRelationTestEntity).save( new CustomFieldRelationTestEntity({ parent: testEntity, data: nestedContent.data, }), ); - await this.connection.getRepository(CustomFieldOtherRelationTestEntity).save( + await this.connection.rawConnection.getRepository(CustomFieldOtherRelationTestEntity).save( new CustomFieldOtherRelationTestEntity({ parent: testEntity, data: nestedContent.data, @@ -452,7 +465,7 @@ export class ListQueryPlugin implements OnApplicationBootstrap { } } } else { - const testEntities = await this.connection.getRepository(TestEntity).find(); + const testEntities = await this.connection.rawConnection.getRepository(TestEntity).find(); const ctx = await this.requestContextService.create({ apiType: 'admin' }); const customers = await this.connection.rawConnection.getRepository(Customer).find(); let i = 0; @@ -463,7 +476,7 @@ export class ListQueryPlugin implements OnApplicationBootstrap { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion const order = await this.orderService.create(ctx, customer.user!.id); testEntity.orderRelation = order; - await this.connection.getRepository(TestEntity).save(testEntity); + await this.connection.rawConnection.getRepository(TestEntity).save(testEntity); } catch (e: any) { Logger.error(e); } diff --git a/packages/core/e2e/list-query-builder.e2e-spec.ts b/packages/core/e2e/list-query-builder.e2e-spec.ts index 1c5a0a40be..0071c115a0 100644 --- a/packages/core/e2e/list-query-builder.e2e-spec.ts +++ b/packages/core/e2e/list-query-builder.e2e-spec.ts @@ -1285,6 +1285,7 @@ describe('ListQueryBuilder', () => { id: 'T_1', label: 'A', name: 'apple', + parent: { id: 'T_2'}, orderRelation: { customer: { firstName: 'Hayden', @@ -1297,6 +1298,7 @@ describe('ListQueryBuilder', () => { id: 'T_4', label: 'D', name: 'dog', + parent: { id: 'T_2'}, orderRelation: { customer: { firstName: 'Hayden', @@ -1412,6 +1414,9 @@ const GET_LIST_WITH_ORDERS = gql` id label name + parent { + id + } orderRelation { id customer { diff --git a/packages/core/src/service/helpers/list-query-builder/list-query-builder.ts b/packages/core/src/service/helpers/list-query-builder/list-query-builder.ts index 06b307635e..5cc145d023 100644 --- a/packages/core/src/service/helpers/list-query-builder/list-query-builder.ts +++ b/packages/core/src/service/helpers/list-query-builder/list-query-builder.ts @@ -696,7 +696,17 @@ export class ListQueryBuilder implements OnApplicationBootstrap { currentAlias: string, ) => { const currentMetadataIsTreeType = metadata.treeType; - if (!currentParentIsTreeType && !currentMetadataIsTreeType) { + let currentMetadataHasOneOrMoreSelfReferencingRelations = false; + // Check if the current entity has one or more self-referencing relations + // to determine if it is a tree type or has tree relations. + for (const relation of currentMetadata.relations) { + if (relation.inverseEntityMetadata === currentMetadata) { + currentMetadataHasOneOrMoreSelfReferencingRelations = true; + break; + } + } + + if (!currentParentIsTreeType && !currentMetadataIsTreeType && !currentMetadataHasOneOrMoreSelfReferencingRelations) { return; } From 16cfc071716cd588e68bedf3aca273b25999a44c Mon Sep 17 00:00:00 2001 From: Eugene Nitsenko Date: Fri, 15 Mar 2024 17:13:23 +0100 Subject: [PATCH 02/20] perf(core): upgrade sql requests for more performant memory usage with big datasets --- .../core/src/service/services/product-option-group.service.ts | 4 ++++ packages/core/src/service/services/product.service.ts | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/packages/core/src/service/services/product-option-group.service.ts b/packages/core/src/service/services/product-option-group.service.ts index e411a5e815..5b7cea9b3a 100644 --- a/packages/core/src/service/services/product-option-group.service.ts +++ b/packages/core/src/service/services/product-option-group.service.ts @@ -133,6 +133,8 @@ export class ProductOptionGroupService { */ async deleteGroupAndOptionsFromProduct(ctx: RequestContext, id: ID, productId: ID) { const optionGroup = await this.connection.getEntityOrThrow(ctx, ProductOptionGroup, id, { + relationLoadStrategy: 'query', + loadEagerRelations: false, relations: ['options', 'product'], }); const deletedOptionGroup = new ProductOptionGroup(optionGroup); @@ -165,6 +167,8 @@ export class ProductOptionGroupService { // hard delete const product = await this.connection.getRepository(ctx, Product).findOne({ + relationLoadStrategy: 'query', + loadEagerRelations: false, where: { id: productId }, relations: ['optionGroups'], }); diff --git a/packages/core/src/service/services/product.service.ts b/packages/core/src/service/services/product.service.ts index 6ab0c1d96c..40c9510c07 100644 --- a/packages/core/src/service/services/product.service.ts +++ b/packages/core/src/service/services/product.service.ts @@ -271,6 +271,8 @@ export class ProductService { async softDelete(ctx: RequestContext, productId: ID): Promise { const product = await this.connection.getEntityOrThrow(ctx, Product, productId, { + relationLoadStrategy: 'query', + loadEagerRelations: false, channelId: ctx.channelId, relations: ['variants', 'optionGroups'], }); @@ -449,6 +451,8 @@ export class ProductService { private async getProductWithOptionGroups(ctx: RequestContext, productId: ID): Promise { const product = await this.connection.getRepository(ctx, Product).findOne({ + relationLoadStrategy: 'query', + loadEagerRelations: false, where: { id: productId, deletedAt: IsNull() }, relations: ['optionGroups', 'variants', 'variants.options'], }); From 469a02b44fbaa516057dda7c849069d8326bc9c9 Mon Sep 17 00:00:00 2001 From: Eugene Nitsenko Date: Fri, 15 Mar 2024 17:36:10 +0100 Subject: [PATCH 03/20] perf(core): Upgrade EntityHydrator to add more performance to any hydrate call and even move with many relations --- .../entity-hydrator/entity-hydrator.service.ts | 14 ++++++++++++-- .../list-query-builder/list-query-builder.ts | 2 +- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/packages/core/src/service/helpers/entity-hydrator/entity-hydrator.service.ts b/packages/core/src/service/helpers/entity-hydrator/entity-hydrator.service.ts index 7d2497a1c7..72a701a518 100644 --- a/packages/core/src/service/helpers/entity-hydrator/entity-hydrator.service.ts +++ b/packages/core/src/service/helpers/entity-hydrator/entity-hydrator.service.ts @@ -12,6 +12,8 @@ import { ProductPriceApplicator } from '../product-price-applicator/product-pric import { TranslatorService } from '../translator/translator.service'; import { HydrateOptions } from './entity-hydrator-types'; +import { ListQueryBuilder } from '../list-query-builder/list-query-builder'; +import { SelectQueryBuilder } from 'typeorm'; /** * @description @@ -78,6 +80,7 @@ export class EntityHydrator { private connection: TransactionalConnection, private productPriceApplicator: ProductPriceApplicator, private translator: TranslatorService, + private listQueryBuilder: ListQueryBuilder, ) {} /** @@ -116,10 +119,17 @@ export class EntityHydrator { } if (missingRelations.length) { - const hydrated = await this.connection.getRepository(ctx, target.constructor).findOne({ + const hydratedQb: SelectQueryBuilder = this.connection + .getRepository(ctx, target.constructor) + .createQueryBuilder(target.constructor.name) + const processedRelations = this.listQueryBuilder + .joinTreeRelationsDynamically(hydratedQb, target.constructor, missingRelations) + hydratedQb.setFindOptions({ + relationLoadStrategy: 'query', where: { id: target.id }, - relations: missingRelations, + relations: missingRelations.filter(relationPath => !processedRelations.has(relationPath)), }); + 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 as any)[prop]); diff --git a/packages/core/src/service/helpers/list-query-builder/list-query-builder.ts b/packages/core/src/service/helpers/list-query-builder/list-query-builder.ts index 06b307635e..f747b724b3 100644 --- a/packages/core/src/service/helpers/list-query-builder/list-query-builder.ts +++ b/packages/core/src/service/helpers/list-query-builder/list-query-builder.ts @@ -681,7 +681,7 @@ export class ListQueryBuilder implements OnApplicationBootstrap { * @template T extends VendureEntity The type of the entity for which relations are being joined. This type parameter * should extend VendureEntity to ensure compatibility with Vendure's data access layer. */ - private joinTreeRelationsDynamically( + public joinTreeRelationsDynamically( qb: SelectQueryBuilder, entity: EntityTarget, requestedRelations: string[] = [], From 10c07988d1f94b7df1cdcafb704f0d2ada3dfd8d Mon Sep 17 00:00:00 2001 From: Eugene Nitsenko Date: Sat, 16 Mar 2024 16:22:08 +0100 Subject: [PATCH 04/20] fix(core): update self-related manual joins with deep eager relations --- .../test-plugins/list-query-plugin.ts | 7 +-- .../core/e2e/list-query-builder.e2e-spec.ts | 14 +++++- .../list-query-builder/list-query-builder.ts | 48 +++++++++++-------- 3 files changed, 44 insertions(+), 25 deletions(-) diff --git a/packages/core/e2e/fixtures/test-plugins/list-query-plugin.ts b/packages/core/e2e/fixtures/test-plugins/list-query-plugin.ts index 5fa72c22f0..5a3ade28c2 100644 --- a/packages/core/e2e/fixtures/test-plugins/list-query-plugin.ts +++ b/packages/core/e2e/fixtures/test-plugins/list-query-plugin.ts @@ -212,8 +212,9 @@ export class ListQueryResolver { item.activePrice = item.prices.find(p => p.channelId === 1)!.price; } } + const i = items.map(i => translateDeep(i, ctx.languageCode, ['parent'])); return { - items: items.map(i => translateDeep(i, ctx.languageCode)), + items: i, totalItems, }; }); @@ -222,7 +223,7 @@ export class ListQueryResolver { @Query() testEntitiesGetMany(@Ctx() ctx: RequestContext, @Args() args: any) { return this.listQueryBuilder - .build(TestEntity, args.options, { ctx, relations: ['prices'] }) + .build(TestEntity, args.options, { ctx, relations: ['prices', 'parent'] }) .getMany() .then(items => { for (const item of items) { @@ -231,7 +232,7 @@ export class ListQueryResolver { item.activePrice = item.prices.find(p => p.channelId === 1)!.price; } } - return items.map(i => translateDeep(i, ctx.languageCode)); + return items.map(i => translateDeep(i, ctx.languageCode, ['parent'])); }); } } diff --git a/packages/core/e2e/list-query-builder.e2e-spec.ts b/packages/core/e2e/list-query-builder.e2e-spec.ts index 0071c115a0..6d18f3f959 100644 --- a/packages/core/e2e/list-query-builder.e2e-spec.ts +++ b/packages/core/e2e/list-query-builder.e2e-spec.ts @@ -1285,7 +1285,11 @@ describe('ListQueryBuilder', () => { id: 'T_1', label: 'A', name: 'apple', - parent: { id: 'T_2'}, + parent: { + id: 'T_2', + label: 'B', + name: 'bike', + }, orderRelation: { customer: { firstName: 'Hayden', @@ -1298,7 +1302,11 @@ describe('ListQueryBuilder', () => { id: 'T_4', label: 'D', name: 'dog', - parent: { id: 'T_2'}, + parent: { + id: 'T_2', + label: 'B', + name: 'bike', + }, orderRelation: { customer: { firstName: 'Hayden', @@ -1416,6 +1424,8 @@ const GET_LIST_WITH_ORDERS = gql` name parent { id + label + name } orderRelation { id diff --git a/packages/core/src/service/helpers/list-query-builder/list-query-builder.ts b/packages/core/src/service/helpers/list-query-builder/list-query-builder.ts index 5cc145d023..6083797e61 100644 --- a/packages/core/src/service/helpers/list-query-builder/list-query-builder.ts +++ b/packages/core/src/service/helpers/list-query-builder/list-query-builder.ts @@ -660,6 +660,29 @@ export class ListQueryBuilder implements OnApplicationBootstrap { return qb.expressionMap.joinAttributes.some(ja => ja.alias.name === alias); } + /** + * @description + * Check if the current entity has one or more self-referencing relations + * to determine if it is a tree type or has tree relations. + * @param metadata + * @private + */ + private isTreeEntityMetadata(metadata: EntityMetadata): boolean { + if (metadata.treeType !== undefined) { + return true; + } + + for (const relation of metadata.relations) { + if (relation.isTreeParent || relation.isTreeChildren) { + return true; + } + if (relation.inverseEntityMetadata === metadata) { + return true; + } + } + return false + } + /** * Dynamically joins tree relations and their eager relations to a query builder. This method is specifically * designed for entities utilizing TypeORM tree decorators (@TreeParent, @TreeChildren) and aims to address @@ -686,27 +709,16 @@ export class ListQueryBuilder implements OnApplicationBootstrap { entity: EntityTarget, requestedRelations: string[] = [], ): Set { - const metadata = qb.connection.getMetadata(entity); + const sourceMetadata = qb.connection.getMetadata(entity); + const isTreeSourceMetadata = this.isTreeEntityMetadata(sourceMetadata) const processedRelations = new Set(); const processRelation = ( currentMetadata: EntityMetadata, - currentParentIsTreeType: boolean, currentPath: string, currentAlias: string, ) => { - const currentMetadataIsTreeType = metadata.treeType; - let currentMetadataHasOneOrMoreSelfReferencingRelations = false; - // Check if the current entity has one or more self-referencing relations - // to determine if it is a tree type or has tree relations. - for (const relation of currentMetadata.relations) { - if (relation.inverseEntityMetadata === currentMetadata) { - currentMetadataHasOneOrMoreSelfReferencingRelations = true; - break; - } - } - - if (!currentParentIsTreeType && !currentMetadataIsTreeType && !currentMetadataHasOneOrMoreSelfReferencingRelations) { + if (!this.isTreeEntityMetadata(currentMetadata) && !isTreeSourceMetadata) { return; } @@ -729,16 +741,13 @@ export class ListQueryBuilder implements OnApplicationBootstrap { qb.leftJoinAndSelect(`${currentAlias}.${part}`, nextAlias); } - const isTreeParent = relationMetadata.isTreeParent; - const isTreeChildren = relationMetadata.isTreeChildren; - const isTree = isTreeParent || isTreeChildren; + const isTree = this.isTreeEntityMetadata(relationMetadata.inverseEntityMetadata); if (isTree) { relationMetadata.inverseEntityMetadata.relations.forEach(subRelation => { if (subRelation.isEager) { processRelation( relationMetadata.inverseEntityMetadata, - !!relationMetadata.inverseEntityMetadata.treeType, subRelation.propertyPath, nextAlias, ); @@ -749,7 +758,6 @@ export class ListQueryBuilder implements OnApplicationBootstrap { if (nextPath) { processRelation( relationMetadata.inverseEntityMetadata, - !!relationMetadata.inverseEntityMetadata.treeType, nextPath, nextAlias, ); @@ -759,7 +767,7 @@ export class ListQueryBuilder implements OnApplicationBootstrap { }; requestedRelations.forEach(relationPath => { - processRelation(metadata, !!metadata.treeType, relationPath, qb.alias); + processRelation(sourceMetadata, relationPath, qb.alias); }); return processedRelations; From 4824d869085c0a916e6e78bd95be645edc7cf8b1 Mon Sep 17 00:00:00 2001 From: Eugene Nitsenko Date: Sat, 16 Mar 2024 16:25:41 +0100 Subject: [PATCH 05/20] chore(core): clean up code --- packages/core/e2e/fixtures/test-plugins/list-query-plugin.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/core/e2e/fixtures/test-plugins/list-query-plugin.ts b/packages/core/e2e/fixtures/test-plugins/list-query-plugin.ts index 5a3ade28c2..3221b02b6c 100644 --- a/packages/core/e2e/fixtures/test-plugins/list-query-plugin.ts +++ b/packages/core/e2e/fixtures/test-plugins/list-query-plugin.ts @@ -212,9 +212,8 @@ export class ListQueryResolver { item.activePrice = item.prices.find(p => p.channelId === 1)!.price; } } - const i = items.map(i => translateDeep(i, ctx.languageCode, ['parent'])); return { - items: i, + items: items.map(i => translateDeep(i, ctx.languageCode, ['parent'])), totalItems, }; }); From 5eb6391197781fef5108b12a36fe1ace0bd2c17b Mon Sep 17 00:00:00 2001 From: Eugene Nitsenko Date: Sat, 16 Mar 2024 17:55:55 +0100 Subject: [PATCH 06/20] fix(core): update test cases for new TestEntities order --- packages/core/e2e/list-query-builder.e2e-spec.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/core/e2e/list-query-builder.e2e-spec.ts b/packages/core/e2e/list-query-builder.e2e-spec.ts index 6d18f3f959..7a3b54d02f 100644 --- a/packages/core/e2e/list-query-builder.e2e-spec.ts +++ b/packages/core/e2e/list-query-builder.e2e-spec.ts @@ -12,6 +12,7 @@ import { ListQueryPlugin } from './fixtures/test-plugins/list-query-plugin'; import { LanguageCode, SortOrder } from './graphql/generated-e2e-admin-types'; import { assertThrowsWithMessage } from './utils/assert-throws-with-message'; import { fixPostgresTimezone } from './utils/fix-pg-timezone'; +import { sortById } from './utils/test-order-utils'; fixPostgresTimezone(); @@ -55,14 +56,14 @@ describe('ListQueryBuilder', () => { expect(testEntities.totalItems).toBe(6); expect(getItemLabels(testEntities.items)).toEqual(['A', 'B', 'C', 'D', 'E', 'F']); - expect(testEntities.items.map((i: any) => i.name)).toEqual([ + expect(testEntities.items.map((i: any) => i.name)).toEqual(expect.arrayContaining([ 'apple', 'bike', 'cake', 'dog', 'egg', 'baum', // if default en lang does not exist, use next available lang - ]); + ])); }); it('all de', async () => { @@ -76,14 +77,14 @@ describe('ListQueryBuilder', () => { expect(testEntities.totalItems).toBe(6); expect(getItemLabels(testEntities.items)).toEqual(['A', 'B', 'C', 'D', 'E', 'F']); - expect(testEntities.items.map((i: any) => i.name)).toEqual([ + expect(testEntities.items.map((i: any) => i.name)).toEqual(expect.arrayContaining([ 'apfel', 'fahrrad', 'kuchen', 'hund', 'egg', // falls back to en translation when de doesn't exist 'baum', - ]); + ])); }); it('take', async () => { @@ -1207,9 +1208,9 @@ describe('ListQueryBuilder', () => { // https://github.com/vendure-ecommerce/vendure/issues/1586 it('using the getMany() of the resulting QueryBuilder', async () => { const { testEntitiesGetMany } = await adminClient.query(GET_ARRAY_LIST, {}); - expect(testEntitiesGetMany.sort((a: any, b: any) => a.id - b.id).map((x: any) => x.price)).toEqual([ - 11, 9, 22, 14, 13, 33, - ]); + const actualPrices = testEntitiesGetMany.sort(sortById).map((x: any) => x.price).sort((a: number, b: number) => a - b); + const expectedPrices = [11, 9, 22, 14, 13, 33].sort((a, b) => a - b); + expect(actualPrices).toEqual(expectedPrices); }); // https://github.com/vendure-ecommerce/vendure/issues/1611 From 0750065429e3cc5eeb8d17127d68b04f0356381e Mon Sep 17 00:00:00 2001 From: Eugene Nitsenko Date: Sat, 16 Mar 2024 21:30:01 +0100 Subject: [PATCH 07/20] feat(core): optimization for assignToChannels method --- .../src/service/services/channel.service.ts | 74 +++++++++++++++---- 1 file changed, 61 insertions(+), 13 deletions(-) diff --git a/packages/core/src/service/services/channel.service.ts b/packages/core/src/service/services/channel.service.ts index cf29667a17..da6842cfe3 100644 --- a/packages/core/src/service/services/channel.service.ts +++ b/packages/core/src/service/services/channel.service.ts @@ -11,7 +11,7 @@ import { import { DEFAULT_CHANNEL_CODE } from '@vendure/common/lib/shared-constants'; import { ID, PaginatedList, Type } from '@vendure/common/lib/shared-types'; import { unique } from '@vendure/common/lib/unique'; -import { FindOneOptions } from 'typeorm'; +import { FindOneOptions, FindOptionsWhere } from 'typeorm'; import { RelationPaths } from '../../api'; import { RequestContext } from '../../api/common/request-context'; @@ -124,6 +124,31 @@ export class ChannelService { return entity; } + /** + * This method is used to bypass a bug with Typeorm when working with ManyToMany relationships. + * For some reason, a regular query does not return all the channels that an entity has. + * This is a most optimized way to get all the channels that an entity has. + * + * @param ctx - The RequestContext object. + * @param entityType - The type of the entity. + * @param entityId - The ID of the entity. + * @returns A promise that resolves to an array of objects, each containing a channel ID. + * @private + */ + private async getAssignedEntityChannels(ctx: RequestContext, entityType: Type, entityId: T['id']): Promise<{ channelId: ID }[]> { + const entityMetadata = this.connection.rawConnection.getMetadata(entityType); + const channelsRelation = entityMetadata.relations.find(r => r.type === Channel); + if (!channelsRelation) { + throw new InternalServerError(`Could not find the join table for the channels relation of entity ${entityMetadata.targetName}`); + } + const junctionColumnName = channelsRelation.joinColumns.find(jc => jc.referencedColumn?.entityMetadata === entityMetadata)?.databaseName; + if (!junctionColumnName) { + throw new InternalServerError(`Could not find the junction column for the channels relation of entity ${entityMetadata.targetName}`); + } + const sqlToGetAssignedChannelIds = `SELECT "channelId" as "channelId" FROM "${channelsRelation?.joinTableName}" WHERE "${junctionColumnName}" = $1`; + return await this.connection.getRepository(ctx, entityType).query(sqlToGetAssignedChannelIds, [entityId]); + } + /** * @description * Assigns the entity to the given Channels and saves. @@ -134,7 +159,7 @@ export class ChannelService { entityId: ID, channelIds: ID[], ): Promise { - const relations = ['channels']; + const relations = []; // This is a work-around for https://github.com/vendure-ecommerce/vendure/issues/1391 // A better API would be to allow the consumer of this method to supply an entity instance // so that this join could be done prior to invoking this method. @@ -143,13 +168,24 @@ export class ChannelService { relations.push('lines', 'shippingLines'); } const entity = await this.connection.getEntityOrThrow(ctx, entityType, entityId, { + loadEagerRelations: false, + relationLoadStrategy: 'query', + where: { + id: entityId, + } as FindOptionsWhere, relations, }); - for (const id of channelIds) { - const channel = await this.connection.getEntityOrThrow(ctx, Channel, id); - entity.channels.push(channel); - } - await this.connection.getRepository(ctx, entityType).save(entity as any, { reload: false }); + const assignedChannels = await this.getAssignedEntityChannels(ctx, entityType, entityId); + + const newChannelIds = channelIds.filter(id => !assignedChannels.some(ec => idsAreEqual(ec.channelId, id))) + + await this.connection + .getRepository(ctx, entityType) + .createQueryBuilder() + .relation('channels') + .of(entity.id) + .add(newChannelIds); + this.eventBus.publish(new ChangeChannelEvent(ctx, entity, channelIds, 'assigned', entityType)); return entity; } @@ -165,16 +201,28 @@ export class ChannelService { channelIds: ID[], ): Promise { const entity = await this.connection.getRepository(ctx, entityType).findOne({ - where: { id: entityId }, - relations: ['channels'], - } as FindOneOptions); + loadEagerRelations: false, + relationLoadStrategy: 'query', + where: { + id: entityId, + } as FindOptionsWhere, + }) if (!entity) { return; } - for (const id of channelIds) { - entity.channels = entity.channels.filter(c => !idsAreEqual(c.id, id)); + const assignedChannels = await this.getAssignedEntityChannels(ctx, entityType, entityId); + + const existingChannelIds = channelIds.filter(id => assignedChannels.some(ec => idsAreEqual(ec.channelId, id))); + + if (!existingChannelIds.length) { + return } - await this.connection.getRepository(ctx, entityType).save(entity as any, { reload: false }); + await this.connection + .getRepository(ctx, entityType) + .createQueryBuilder() + .relation('channels') + .of(entity.id) + .remove(existingChannelIds); this.eventBus.publish(new ChangeChannelEvent(ctx, entity, channelIds, 'removed', entityType)); return entity; } From dc463bab761d49a86f80a479338a45afb132ca84 Mon Sep 17 00:00:00 2001 From: Eugene Nitsenko Date: Sat, 16 Mar 2024 21:57:53 +0100 Subject: [PATCH 08/20] feat(core): fix changes for new getAssignedEntityChannels method --- .../src/service/services/channel.service.ts | 36 +++++++++++-------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/packages/core/src/service/services/channel.service.ts b/packages/core/src/service/services/channel.service.ts index da6842cfe3..c31cbb2fc9 100644 --- a/packages/core/src/service/services/channel.service.ts +++ b/packages/core/src/service/services/channel.service.ts @@ -11,17 +11,12 @@ import { import { DEFAULT_CHANNEL_CODE } from '@vendure/common/lib/shared-constants'; import { ID, PaginatedList, Type } from '@vendure/common/lib/shared-types'; import { unique } from '@vendure/common/lib/unique'; -import { FindOneOptions, FindOptionsWhere } from 'typeorm'; +import { FindOptionsWhere } from 'typeorm'; import { RelationPaths } from '../../api'; import { RequestContext } from '../../api/common/request-context'; import { ErrorResultUnion, isGraphQlErrorResult } from '../../common/error/error-result'; -import { - ChannelNotFoundError, - EntityNotFoundError, - InternalServerError, - UserInputError, -} from '../../common/error/errors'; +import { ChannelNotFoundError, EntityNotFoundError, InternalServerError, UserInputError } from '../../common/error/errors'; import { LanguageNotAvailableError } from '../../common/error/generated-graphql-admin-errors'; import { createSelfRefreshingCache, SelfRefreshingCache } from '../../common/self-refreshing-cache'; import { ChannelAware, ListQueryOptions } from '../../common/types/common-types'; @@ -136,17 +131,28 @@ export class ChannelService { * @private */ private async getAssignedEntityChannels(ctx: RequestContext, entityType: Type, entityId: T['id']): Promise<{ channelId: ID }[]> { - const entityMetadata = this.connection.rawConnection.getMetadata(entityType); - const channelsRelation = entityMetadata.relations.find(r => r.type === Channel); + const repository = this.connection.getRepository(ctx, entityType); + + const metadata = repository.metadata; + const channelsRelation = metadata.findRelationWithPropertyPath('channels'); + if (!channelsRelation) { - throw new InternalServerError(`Could not find the join table for the channels relation of entity ${entityMetadata.targetName}`); + throw new InternalServerError(`Could not find the channels relation for entity ${metadata.name}`); } - const junctionColumnName = channelsRelation.joinColumns.find(jc => jc.referencedColumn?.entityMetadata === entityMetadata)?.databaseName; - if (!junctionColumnName) { - throw new InternalServerError(`Could not find the junction column for the channels relation of entity ${entityMetadata.targetName}`); + + const junctionTableName = channelsRelation.junctionEntityMetadata?.tableName; + const junctionColumnName = channelsRelation.junctionEntityMetadata?.columns[0].databaseName; + const inverseJunctionColumnName = channelsRelation.junctionEntityMetadata?.inverseColumns[0].databaseName; + + if (!junctionTableName || !junctionColumnName || !inverseJunctionColumnName) { + throw new InternalServerError(`Could not find necessary join table information for the channels relation of entity ${metadata.name}`); } - const sqlToGetAssignedChannelIds = `SELECT "channelId" as "channelId" FROM "${channelsRelation?.joinTableName}" WHERE "${junctionColumnName}" = $1`; - return await this.connection.getRepository(ctx, entityType).query(sqlToGetAssignedChannelIds, [entityId]); + + return await this.connection.getRepository(ctx, entityType).createQueryBuilder() + .select(`channel.${inverseJunctionColumnName}`, 'channelId') + .from(junctionTableName, 'channel') + .where(`channel.${junctionColumnName} = :entityId`, { entityId }) + .execute(); } /** From c376eef36ee0edda658f7fbd372f9917e07c6816 Mon Sep 17 00:00:00 2001 From: Eugene Nitsenko Date: Sat, 16 Mar 2024 23:18:05 +0100 Subject: [PATCH 09/20] feat(core): add optimization into findOneInChannel and more refactoring --- .../resolvers/admin/collection.resolver.ts | 3 +- .../connection/transactional-connection.ts | 56 +++++-- .../entity/asset/orderable-asset.entity.ts | 2 +- .../entity-hydrator.service.ts | 8 +- .../list-query-builder/list-query-builder.ts | 135 +---------------- .../helpers/utils/tree-relations-qb-joiner.ts | 137 ++++++++++++++++++ 6 files changed, 188 insertions(+), 153 deletions(-) create mode 100644 packages/core/src/service/helpers/utils/tree-relations-qb-joiner.ts diff --git a/packages/core/src/api/resolvers/admin/collection.resolver.ts b/packages/core/src/api/resolvers/admin/collection.resolver.ts index 8f73bd2fa6..4cc8a02795 100644 --- a/packages/core/src/api/resolvers/admin/collection.resolver.ts +++ b/packages/core/src/api/resolvers/admin/collection.resolver.ts @@ -101,7 +101,8 @@ export class CollectionResolver { ): Promise> { const { input } = args; this.configurableOperationCodec.decodeConfigurableOperationIds(CollectionFilter, input.filters); - return this.collectionService.create(ctx, input); + const collection = await this.collectionService.create(ctx, input); + return collection; } @Transaction() diff --git a/packages/core/src/connection/transactional-connection.ts b/packages/core/src/connection/transactional-connection.ts index 948e04e3d6..37f6d60e76 100644 --- a/packages/core/src/connection/transactional-connection.ts +++ b/packages/core/src/connection/transactional-connection.ts @@ -9,7 +9,7 @@ import { FindOptionsUtils, ObjectLiteral, ObjectType, - Repository, + Repository, SelectQueryBuilder, } from 'typeorm'; import { FindManyOptions } from 'typeorm/find-options/FindManyOptions'; @@ -22,6 +22,9 @@ import { VendureEntity } from '../entity/base/base.entity'; import { TransactionWrapper } from './transaction-wrapper'; import { GetEntityOrThrowOptions } from './types'; +import { DataSource } from 'typeorm/data-source/DataSource'; +import { InjectDataSource } from '@nestjs/typeorm/dist/common/typeorm.decorators'; +import { joinTreeRelationsDynamically } from '../service/helpers/utils/tree-relations-qb-joiner'; /** * @description @@ -38,7 +41,7 @@ import { GetEntityOrThrowOptions } from './types'; @Injectable() export class TransactionalConnection { constructor( - @InjectConnection() private connection: Connection, + @InjectDataSource() private dataSource: DataSource, private transactionWrapper: TransactionWrapper, ) {} @@ -48,8 +51,8 @@ export class TransactionalConnection { * performed with this connection will not be performed within any outer * transactions. */ - get rawConnection(): Connection { - return this.connection; + get rawConnection(): DataSource { + return this.dataSource; } /** @@ -275,16 +278,28 @@ export class TransactionalConnection { channelId: ID, options: FindOneOptions = {}, ) { - const qb = this.getRepository(ctx, entity).createQueryBuilder('entity').setFindOptions(options); - if (options.loadEagerRelations !== false) { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - FindOptionsUtils.joinEagerRelations(qb, qb.alias, qb.expressionMap.mainAlias!.metadata); + const qb = this.getRepository(ctx, entity).createQueryBuilder('entity') + + if (Array.isArray(options.relations) && options.relations.length > 0) { + const joinedRelations = joinTreeRelationsDynamically(qb, entity, options.relations); + // Remove any relations which are related to the 'collection' tree, as these are handled separately + // to avoid duplicate joins. + options.relations = options.relations.filter(relationPath => !joinedRelations.has(relationPath)); } + qb.setFindOptions({ + relationLoadStrategy: 'query', // default to query strategy for maximum performance + ...options + }); + + FindOptionsUtils.joinEagerRelations(qb, qb.alias, qb.expressionMap.mainAlias!.metadata) + qb.leftJoin('entity.channels', '__channel') .andWhere('entity.id = :id', { id }) .andWhere('__channel.id = :channelId', { channelId }); - return qb.getOne().then(result => result ?? undefined); + return qb.getOne().then(result => { + return result ?? undefined + }); } /** @@ -305,11 +320,26 @@ export class TransactionalConnection { return Promise.resolve([]); } - const qb = this.getRepository(ctx, entity).createQueryBuilder('entity').setFindOptions(options); - if (options.loadEagerRelations !== false) { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion - FindOptionsUtils.joinEagerRelations(qb, qb.alias, qb.expressionMap.mainAlias!.metadata); + const qb = this.getRepository(ctx, entity).createQueryBuilder('entity'); + + if (Array.isArray(options.relations) && options.relations.length > 0) { + const joinedRelations = joinTreeRelationsDynamically( + qb as SelectQueryBuilder, + entity, + options.relations, + ); + // Remove any relations which are related to the 'collection' tree, as these are handled separately + // to avoid duplicate joins. + options.relations = options.relations.filter(relationPath => !joinedRelations.has(relationPath)); } + + qb.setFindOptions({ + relationLoadStrategy: 'query', // default to query strategy for maximum performance + ...options + }); + + FindOptionsUtils.joinEagerRelations(qb, qb.alias, qb.expressionMap.mainAlias!.metadata) + return qb .leftJoin('entity.channels', 'channel') .andWhere('entity.id IN (:...ids)', { ids }) diff --git a/packages/core/src/entity/asset/orderable-asset.entity.ts b/packages/core/src/entity/asset/orderable-asset.entity.ts index 507385f49a..e8b7b6b922 100644 --- a/packages/core/src/entity/asset/orderable-asset.entity.ts +++ b/packages/core/src/entity/asset/orderable-asset.entity.ts @@ -24,7 +24,7 @@ export abstract class OrderableAsset extends VendureEntity implements Orderable assetId: ID; @Index() - @ManyToOne(type => Asset, { eager: true, onDelete: 'CASCADE' }) + @ManyToOne(type => Asset, { eager: true, onDelete: 'CASCADE' }) asset: Asset; @Column() diff --git a/packages/core/src/service/helpers/entity-hydrator/entity-hydrator.service.ts b/packages/core/src/service/helpers/entity-hydrator/entity-hydrator.service.ts index 72a701a518..06fe0d30f9 100644 --- a/packages/core/src/service/helpers/entity-hydrator/entity-hydrator.service.ts +++ b/packages/core/src/service/helpers/entity-hydrator/entity-hydrator.service.ts @@ -12,8 +12,8 @@ import { ProductPriceApplicator } from '../product-price-applicator/product-pric import { TranslatorService } from '../translator/translator.service'; import { HydrateOptions } from './entity-hydrator-types'; -import { ListQueryBuilder } from '../list-query-builder/list-query-builder'; import { SelectQueryBuilder } from 'typeorm'; +import { joinTreeRelationsDynamically } from '../utils/tree-relations-qb-joiner'; /** * @description @@ -80,7 +80,6 @@ export class EntityHydrator { private connection: TransactionalConnection, private productPriceApplicator: ProductPriceApplicator, private translator: TranslatorService, - private listQueryBuilder: ListQueryBuilder, ) {} /** @@ -122,12 +121,11 @@ export class EntityHydrator { const hydratedQb: SelectQueryBuilder = this.connection .getRepository(ctx, target.constructor) .createQueryBuilder(target.constructor.name) - const processedRelations = this.listQueryBuilder - .joinTreeRelationsDynamically(hydratedQb, target.constructor, missingRelations) + const joinedRelations = joinTreeRelationsDynamically(hydratedQb, target.constructor, missingRelations) hydratedQb.setFindOptions({ relationLoadStrategy: 'query', where: { id: target.id }, - relations: missingRelations.filter(relationPath => !processedRelations.has(relationPath)), + relations: missingRelations.filter(relationPath => !joinedRelations.has(relationPath)), }); const hydrated = await hydratedQb.getOne(); const propertiesToAdd = unique(missingRelations.map(relation => relation.split('.')[0])); diff --git a/packages/core/src/service/helpers/list-query-builder/list-query-builder.ts b/packages/core/src/service/helpers/list-query-builder/list-query-builder.ts index 8545b57627..27c9b8262c 100644 --- a/packages/core/src/service/helpers/list-query-builder/list-query-builder.ts +++ b/packages/core/src/service/helpers/list-query-builder/list-query-builder.ts @@ -31,6 +31,7 @@ import { getColumnMetadata, getEntityAlias } from './connection-utils'; import { getCalculatedColumns } from './get-calculated-columns'; import { parseFilterParams, WhereGroup } from './parse-filter-params'; import { parseSortParams } from './parse-sort-params'; +import { joinTreeRelationsDynamically } from '../utils/tree-relations-qb-joiner'; /** * @description @@ -269,7 +270,7 @@ export class ListQueryBuilder implements OnApplicationBootstrap { // and requires special handling to ensure that only the necessary relations are joined. // This is bypassed an issue in TypeORM where it would join the same relation multiple times. // See https://github.com/typeorm/typeorm/issues/9936 for more context. - const processedRelations = this.joinTreeRelationsDynamically(qb, entity, relations); + const processedRelations = joinTreeRelationsDynamically(qb, entity, relations); // Remove any relations which are related to the 'collection' tree, as these are handled separately // to avoid duplicate joins. @@ -634,142 +635,10 @@ export class ListQueryBuilder implements OnApplicationBootstrap { } } - /** - * These method are designed to address specific challenges encountered with TypeORM - * when dealing with complex relation structures, particularly around the 'collection' - * entity and other similar entities, and they nested relations ('parent', 'children'). The need for these custom - * implementations arises from limitations in handling deeply nested relations and ensuring - * efficient query generation without duplicate joins, as discussed in TypeORM issue #9936. - * See https://github.com/typeorm/typeorm/issues/9936 for more context. - */ - - /** - * Verifies if a relation has already been joined in a query builder to prevent duplicate joins. - * This method ensures query efficiency and correctness by maintaining unique joins within the query builder. - * - * @param {SelectQueryBuilder} qb The query builder instance where the joins are being added. - * @param {string} alias The join alias to check for uniqueness. This alias is used to determine if the relation - * has already been joined to avoid adding duplicate join statements. - * @returns boolean Returns true if the relation has already been joined (based on the alias), false otherwise. - * @template T extends VendureEntity The entity type for which the query builder is configured. - */ private isRelationAlreadyJoined( qb: SelectQueryBuilder, alias: string, ): boolean { return qb.expressionMap.joinAttributes.some(ja => ja.alias.name === alias); } - - /** - * @description - * Check if the current entity has one or more self-referencing relations - * to determine if it is a tree type or has tree relations. - * @param metadata - * @private - */ - private isTreeEntityMetadata(metadata: EntityMetadata): boolean { - if (metadata.treeType !== undefined) { - return true; - } - - for (const relation of metadata.relations) { - if (relation.isTreeParent || relation.isTreeChildren) { - return true; - } - if (relation.inverseEntityMetadata === metadata) { - return true; - } - } - return false - } - - /** - * Dynamically joins tree relations and their eager relations to a query builder. This method is specifically - * designed for entities utilizing TypeORM tree decorators (@TreeParent, @TreeChildren) and aims to address - * the challenge of efficiently managing deeply nested relations and avoiding duplicate joins. The method - * automatically handles the joining of related entities marked with tree relation decorators and eagerly - * loaded relations, ensuring efficient data retrieval and query generation. - * - * The method iterates over the requested relations paths, joining each relation dynamically. For tree relations, - * it also recursively joins all associated eager relations. This approach avoids the manual specification of joins - * and leverages TypeORM's relation metadata to automate the process. - * - * @param {SelectQueryBuilder} qb The query builder instance to which the relations will be joined. - * @param {EntityTarget} entity The target entity class or schema name. This parameter is used to access - * the entity's metadata and analyze its relations. - * @param {string[]} requestedRelations An array of strings representing the relation paths to be dynamically joined. - * Each string in the array should denote a path to a relation (e.g., 'parent.parent.children'). - * @returns {Set} A Set containing the paths of relations that were dynamically joined. This set can be used - * to track which relations have been processed and potentially avoid duplicate processing. - * @template T extends VendureEntity The type of the entity for which relations are being joined. This type parameter - * should extend VendureEntity to ensure compatibility with Vendure's data access layer. - */ - public joinTreeRelationsDynamically( - qb: SelectQueryBuilder, - entity: EntityTarget, - requestedRelations: string[] = [], - ): Set { - const sourceMetadata = qb.connection.getMetadata(entity); - const isTreeSourceMetadata = this.isTreeEntityMetadata(sourceMetadata) - const processedRelations = new Set(); - - const processRelation = ( - currentMetadata: EntityMetadata, - currentPath: string, - currentAlias: string, - ) => { - if (!this.isTreeEntityMetadata(currentMetadata) && !isTreeSourceMetadata) { - return; - } - - const parts = currentPath.split('.'); - const part = parts.shift(); - - if (!part || !currentMetadata) return; - - const relationMetadata = currentMetadata.findRelationWithPropertyPath(part); - if (relationMetadata) { - const isEager = relationMetadata.isEager; - let joinConnector = '_'; - if (isEager) { - joinConnector = '__'; - } - const nextAlias = `${currentAlias}${joinConnector}${part}`; - const nextPath = parts.join('.'); - - if (!this.isRelationAlreadyJoined(qb, nextAlias)) { - qb.leftJoinAndSelect(`${currentAlias}.${part}`, nextAlias); - } - - const isTree = this.isTreeEntityMetadata(relationMetadata.inverseEntityMetadata); - - if (isTree) { - relationMetadata.inverseEntityMetadata.relations.forEach(subRelation => { - if (subRelation.isEager) { - processRelation( - relationMetadata.inverseEntityMetadata, - subRelation.propertyPath, - nextAlias, - ); - } - }); - } - - if (nextPath) { - processRelation( - relationMetadata.inverseEntityMetadata, - nextPath, - nextAlias, - ); - } - processedRelations.add(currentPath); - } - }; - - requestedRelations.forEach(relationPath => { - processRelation(sourceMetadata, relationPath, qb.alias); - }); - - return processedRelations; - } } diff --git a/packages/core/src/service/helpers/utils/tree-relations-qb-joiner.ts b/packages/core/src/service/helpers/utils/tree-relations-qb-joiner.ts new file mode 100644 index 0000000000..cc6c05c86b --- /dev/null +++ b/packages/core/src/service/helpers/utils/tree-relations-qb-joiner.ts @@ -0,0 +1,137 @@ +/** + * Verifies if a relation has already been joined in a query builder to prevent duplicate joins. + * This method ensures query efficiency and correctness by maintaining unique joins within the query builder. + * + * @param {SelectQueryBuilder} qb The query builder instance where the joins are being added. + * @param {string} alias The join alias to check for uniqueness. This alias is used to determine if the relation + * has already been joined to avoid adding duplicate join statements. + * @returns boolean Returns true if the relation has already been joined (based on the alias), false otherwise. + * @template T extends VendureEntity The entity type for which the query builder is configured. + */ +import { VendureEntity } from '../../../entity'; +import { EntityMetadata, SelectQueryBuilder } from 'typeorm'; +import { EntityTarget } from 'typeorm/common/EntityTarget'; + +/** + * @description + * Check if the current entity has one or more self-referencing relations + * to determine if it is a tree type or has tree relations. + * @param metadata + * @private + */ +function isTreeEntityMetadata(metadata: EntityMetadata): boolean { + if (metadata.treeType !== undefined) { + return true; + } + + for (const relation of metadata.relations) { + if (relation.isTreeParent || relation.isTreeChildren) { + return true; + } + if (relation.inverseEntityMetadata === metadata) { + return true; + } + } + return false +} + +/** + * These method are designed to address specific challenges encountered with TypeORM + * when dealing with complex relation structures, particularly around the 'collection' + * entity and other similar entities, and they nested relations ('parent', 'children'). The need for these custom + * implementations arises from limitations in handling deeply nested relations and ensuring + * efficient query generation without duplicate joins, as discussed in TypeORM issue #9936. + * See https://github.com/typeorm/typeorm/issues/9936 for more context. + * + * Dynamically joins tree relations and their eager relations to a query builder. This method is specifically + * designed for entities utilizing TypeORM tree decorators (@TreeParent, @TreeChildren) and aims to address + * the challenge of efficiently managing deeply nested relations and avoiding duplicate joins. The method + * automatically handles the joining of related entities marked with tree relation decorators and eagerly + * loaded relations, ensuring efficient data retrieval and query generation. + * + * The method iterates over the requested relations paths, joining each relation dynamically. For tree relations, + * it also recursively joins all associated eager relations. This approach avoids the manual specification of joins + * and leverages TypeORM's relation metadata to automate the process. + * + * @param {SelectQueryBuilder} qb The query builder instance to which the relations will be joined. + * @param {EntityTarget} entity The target entity class or schema name. This parameter is used to access + * the entity's metadata and analyze its relations. + * @param {string[]} requestedRelations An array of strings representing the relation paths to be dynamically joined. + * Each string in the array should denote a path to a relation (e.g., 'parent.parent.children'). + * @returns {Set} A Set containing the paths of relations that were dynamically joined. This set can be used + * to track which relations have been processed and potentially avoid duplicate processing. + * @template T extends VendureEntity The type of the entity for which relations are being joined. This type parameter + * should extend VendureEntity to ensure compatibility with Vendure's data access layer. + */ +export function joinTreeRelationsDynamically( + qb: SelectQueryBuilder, + entity: EntityTarget, + requestedRelations: string[] = [], +): Set { + const processedRelations = new Set(); + if (!requestedRelations.length) { + return processedRelations; + } + + const sourceMetadata = qb.connection.getMetadata(entity); + const isTreeSourceMetadata = isTreeEntityMetadata(sourceMetadata) + + const processRelation = ( + currentMetadata: EntityMetadata, + currentPath: string, + currentAlias: string, + ) => { + if (!isTreeEntityMetadata(currentMetadata) && !isTreeSourceMetadata) { + return; + } + + const parts = currentPath.split('.'); + const part = parts.shift(); + + if (!part || !currentMetadata) return; + + const relationMetadata = currentMetadata.findRelationWithPropertyPath(part); + if (relationMetadata) { + const isEager = relationMetadata.isEager; + let joinConnector = '_'; + if (isEager) { + joinConnector = '__'; + } + const nextAlias = `${currentAlias}${joinConnector}${part}`; + const nextPath = parts.join('.'); + + if (!qb.expressionMap.joinAttributes.some(ja => ja.alias.name === nextAlias)) { + qb.leftJoinAndSelect(`${currentAlias}.${part}`, nextAlias); + } + + const isTree = isTreeEntityMetadata(relationMetadata.inverseEntityMetadata); + + if (isTree) { + relationMetadata.inverseEntityMetadata.relations.forEach(subRelation => { + if (subRelation.isEager) { + processRelation( + relationMetadata.inverseEntityMetadata, + subRelation.propertyPath, + nextAlias, + ); + } + }); + } + + if (nextPath) { + processRelation( + relationMetadata.inverseEntityMetadata, + nextPath, + nextAlias, + ); + } + processedRelations.add(currentPath); + } + }; + + requestedRelations.forEach(relationPath => { + processRelation(sourceMetadata, relationPath, qb.alias); + }); + + return processedRelations; +} From 661c2e9d4d7652a36d980b9eb657f084a43e155d Mon Sep 17 00:00:00 2001 From: Eugene Nitsenko Date: Sun, 17 Mar 2024 09:53:45 +0100 Subject: [PATCH 10/20] perf(core): optimize translatable-saver.ts query to get existingTranslations without any eager relations --- .../src/common/utilities/create-updated-translatable.spec.ts | 2 ++ .../service/helpers/translatable-saver/translatable-saver.ts | 2 ++ 2 files changed, 4 insertions(+) diff --git a/packages/admin-ui/src/lib/core/src/common/utilities/create-updated-translatable.spec.ts b/packages/admin-ui/src/lib/core/src/common/utilities/create-updated-translatable.spec.ts index 219c7bd1c7..19914c06fa 100644 --- a/packages/admin-ui/src/lib/core/src/common/utilities/create-updated-translatable.spec.ts +++ b/packages/admin-ui/src/lib/core/src/common/utilities/create-updated-translatable.spec.ts @@ -1,8 +1,10 @@ import { DeepPartial } from '@vendure/common/lib/shared-types'; +import { describe, expect, it, beforeEach } from 'vitest'; import { CustomFieldConfig, LanguageCode, ProductDetailFragment } from '../generated-types'; import { createUpdatedTranslatable } from './create-updated-translatable'; +import { fail } from 'assert'; /* eslint-disable @typescript-eslint/no-non-null-assertion */ describe('createUpdatedTranslatable()', () => { diff --git a/packages/core/src/service/helpers/translatable-saver/translatable-saver.ts b/packages/core/src/service/helpers/translatable-saver/translatable-saver.ts index d6563215e4..1560eed689 100644 --- a/packages/core/src/service/helpers/translatable-saver/translatable-saver.ts +++ b/packages/core/src/service/helpers/translatable-saver/translatable-saver.ts @@ -94,6 +94,8 @@ export class TranslatableSaver { async update(options: UpdateTranslatableOptions): Promise { const { ctx, entityType, translationType, input, beforeSave, typeOrmSubscriberData } = options; const existingTranslations = await this.connection.getRepository(ctx, translationType).find({ + relationLoadStrategy: 'query', + loadEagerRelations: false, where: { base: { id: input.id } }, relations: ['base'], } as FindManyOptions>); From eaca1c9d1e66a67c961978b7cdc4b9019ea369a6 Mon Sep 17 00:00:00 2001 From: Eugene Nitsenko Date: Sun, 17 Mar 2024 10:43:12 +0100 Subject: [PATCH 11/20] fix(core): Added processing of deeply nested relations that lack ReverseSide relation for list-query-builder --- .../helpers/utils/tree-relations-qb-joiner.ts | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/packages/core/src/service/helpers/utils/tree-relations-qb-joiner.ts b/packages/core/src/service/helpers/utils/tree-relations-qb-joiner.ts index cc6c05c86b..a8231e43cd 100644 --- a/packages/core/src/service/helpers/utils/tree-relations-qb-joiner.ts +++ b/packages/core/src/service/helpers/utils/tree-relations-qb-joiner.ts @@ -58,7 +58,7 @@ function isTreeEntityMetadata(metadata: EntityMetadata): boolean { * the entity's metadata and analyze its relations. * @param {string[]} requestedRelations An array of strings representing the relation paths to be dynamically joined. * Each string in the array should denote a path to a relation (e.g., 'parent.parent.children'). - * @returns {Set} A Set containing the paths of relations that were dynamically joined. This set can be used + * @returns {Map} A Map containing the paths of relations that were dynamically joined with their aliases. This map can be used * to track which relations have been processed and potentially avoid duplicate processing. * @template T extends VendureEntity The type of the entity for which relations are being joined. This type parameter * should extend VendureEntity to ensure compatibility with Vendure's data access layer. @@ -67,21 +67,23 @@ export function joinTreeRelationsDynamically( qb: SelectQueryBuilder, entity: EntityTarget, requestedRelations: string[] = [], -): Set { - const processedRelations = new Set(); +): Map { + const joinedRelations = new Map(); if (!requestedRelations.length) { - return processedRelations; + return joinedRelations; } const sourceMetadata = qb.connection.getMetadata(entity); - const isTreeSourceMetadata = isTreeEntityMetadata(sourceMetadata) + const sourceMetadataIsTree = isTreeEntityMetadata(sourceMetadata) const processRelation = ( currentMetadata: EntityMetadata, + parentMetadataIsTree: boolean, currentPath: string, currentAlias: string, ) => { - if (!isTreeEntityMetadata(currentMetadata) && !isTreeSourceMetadata) { + const currentMetadataIsTree = isTreeEntityMetadata(currentMetadata) && sourceMetadataIsTree; + if (!currentMetadataIsTree && !parentMetadataIsTree) { return; } @@ -104,13 +106,14 @@ export function joinTreeRelationsDynamically( qb.leftJoinAndSelect(`${currentAlias}.${part}`, nextAlias); } - const isTree = isTreeEntityMetadata(relationMetadata.inverseEntityMetadata); + const inverseEntityMetadataIsTree = isTreeEntityMetadata(relationMetadata.inverseEntityMetadata); - if (isTree) { + if (parentMetadataIsTree || inverseEntityMetadataIsTree || currentMetadataIsTree) { relationMetadata.inverseEntityMetadata.relations.forEach(subRelation => { if (subRelation.isEager) { processRelation( relationMetadata.inverseEntityMetadata, + currentMetadataIsTree, subRelation.propertyPath, nextAlias, ); @@ -118,20 +121,21 @@ export function joinTreeRelationsDynamically( }); } - if (nextPath) { + if (nextPath && nextPath !== "") { processRelation( relationMetadata.inverseEntityMetadata, + currentMetadataIsTree, nextPath, nextAlias, ); } - processedRelations.add(currentPath); + joinedRelations.set(currentPath, nextAlias); } }; requestedRelations.forEach(relationPath => { - processRelation(sourceMetadata, relationPath, qb.alias); + processRelation(sourceMetadata, sourceMetadataIsTree, relationPath, qb.alias); }); - return processedRelations; + return joinedRelations; } From 11e39049bc3573b60d7f781dcf58c60a44fec953 Mon Sep 17 00:00:00 2001 From: Eugene Nitsenko Date: Sun, 17 Mar 2024 10:56:02 +0100 Subject: [PATCH 12/20] fix(core): fix condition for tree type of metadata in list-query-builder --- .../core/src/service/helpers/utils/tree-relations-qb-joiner.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/service/helpers/utils/tree-relations-qb-joiner.ts b/packages/core/src/service/helpers/utils/tree-relations-qb-joiner.ts index a8231e43cd..ea8899fb12 100644 --- a/packages/core/src/service/helpers/utils/tree-relations-qb-joiner.ts +++ b/packages/core/src/service/helpers/utils/tree-relations-qb-joiner.ts @@ -82,7 +82,7 @@ export function joinTreeRelationsDynamically( currentPath: string, currentAlias: string, ) => { - const currentMetadataIsTree = isTreeEntityMetadata(currentMetadata) && sourceMetadataIsTree; + const currentMetadataIsTree = isTreeEntityMetadata(currentMetadata) || sourceMetadataIsTree; if (!currentMetadataIsTree && !parentMetadataIsTree) { return; } From c92bf6ded3a04810869329fdf0015ad9cb997635 Mon Sep 17 00:00:00 2001 From: Eugene Nitsenko Date: Tue, 19 Mar 2024 18:22:18 +0100 Subject: [PATCH 13/20] feat(core): Added edge case handling with customFields as well as depth of eager relations --- .../helpers/utils/tree-relations-qb-joiner.ts | 112 +++++++++++------- 1 file changed, 70 insertions(+), 42 deletions(-) diff --git a/packages/core/src/service/helpers/utils/tree-relations-qb-joiner.ts b/packages/core/src/service/helpers/utils/tree-relations-qb-joiner.ts index ea8899fb12..9d6f7376b2 100644 --- a/packages/core/src/service/helpers/utils/tree-relations-qb-joiner.ts +++ b/packages/core/src/service/helpers/utils/tree-relations-qb-joiner.ts @@ -8,10 +8,11 @@ * @returns boolean Returns true if the relation has already been joined (based on the alias), false otherwise. * @template T extends VendureEntity The entity type for which the query builder is configured. */ -import { VendureEntity } from '../../../entity'; import { EntityMetadata, SelectQueryBuilder } from 'typeorm'; import { EntityTarget } from 'typeorm/common/EntityTarget'; +import { VendureEntity } from '../../../entity'; + /** * @description * Check if the current entity has one or more self-referencing relations @@ -32,7 +33,7 @@ function isTreeEntityMetadata(metadata: EntityMetadata): boolean { return true; } } - return false + return false; } /** @@ -58,6 +59,7 @@ function isTreeEntityMetadata(metadata: EntityMetadata): boolean { * the entity's metadata and analyze its relations. * @param {string[]} requestedRelations An array of strings representing the relation paths to be dynamically joined. * Each string in the array should denote a path to a relation (e.g., 'parent.parent.children'). + * @param maxEagerDepth The maximum depth of eager relations to join. This parameter is used to limit the depth of eager relations. * @returns {Map} A Map containing the paths of relations that were dynamically joined with their aliases. This map can be used * to track which relations have been processed and potentially avoid duplicate processing. * @template T extends VendureEntity The type of the entity for which relations are being joined. This type parameter @@ -67,6 +69,7 @@ export function joinTreeRelationsDynamically( qb: SelectQueryBuilder, entity: EntityTarget, requestedRelations: string[] = [], + maxEagerDepth: number = 1, ): Map { const joinedRelations = new Map(); if (!requestedRelations.length) { @@ -74,67 +77,92 @@ export function joinTreeRelationsDynamically( } const sourceMetadata = qb.connection.getMetadata(entity); - const sourceMetadataIsTree = isTreeEntityMetadata(sourceMetadata) + const sourceMetadataIsTree = isTreeEntityMetadata(sourceMetadata); const processRelation = ( currentMetadata: EntityMetadata, parentMetadataIsTree: boolean, currentPath: string, currentAlias: string, + parentPath?: string[], + eagerDepth: number = 0, // Текущий уровень глубины для eager связей ) => { + if (currentPath === '') { + return; + } + parentPath = parentPath?.filter(p => p !== ''); const currentMetadataIsTree = isTreeEntityMetadata(currentMetadata) || sourceMetadataIsTree; if (!currentMetadataIsTree && !parentMetadataIsTree) { return; } const parts = currentPath.split('.'); - const part = parts.shift(); + let part = parts.shift(); if (!part || !currentMetadata) return; + if (part === 'customFields' && parts.length > 0) { + const relation = parts.shift(); + if (!relation) return; + part += `.${relation}`; + } + const relationMetadata = currentMetadata.findRelationWithPropertyPath(part); - if (relationMetadata) { - const isEager = relationMetadata.isEager; - let joinConnector = '_'; - if (isEager) { - joinConnector = '__'; - } - const nextAlias = `${currentAlias}${joinConnector}${part}`; - const nextPath = parts.join('.'); - - if (!qb.expressionMap.joinAttributes.some(ja => ja.alias.name === nextAlias)) { - qb.leftJoinAndSelect(`${currentAlias}.${part}`, nextAlias); - } - - const inverseEntityMetadataIsTree = isTreeEntityMetadata(relationMetadata.inverseEntityMetadata); - - if (parentMetadataIsTree || inverseEntityMetadataIsTree || currentMetadataIsTree) { - relationMetadata.inverseEntityMetadata.relations.forEach(subRelation => { - if (subRelation.isEager) { - processRelation( - relationMetadata.inverseEntityMetadata, - currentMetadataIsTree, - subRelation.propertyPath, - nextAlias, - ); - } - }); - } - - if (nextPath && nextPath !== "") { - processRelation( - relationMetadata.inverseEntityMetadata, - currentMetadataIsTree, - nextPath, - nextAlias, - ); - } - joinedRelations.set(currentPath, nextAlias); + + if (!relationMetadata) { + return; + } + + let joinConnector = '_'; + if (relationMetadata.isEager) { + joinConnector = '__'; + } + const nextAlias = `${currentAlias}${joinConnector}${part.replace(/\./g, '_')}`; + const nextPath = parts.join('.'); + const fullPath = [...(parentPath || []), part].join('.'); + if (!qb.expressionMap.joinAttributes.some(ja => ja.alias.name === nextAlias)) { + qb.leftJoinAndSelect(`${currentAlias}.${part}`, nextAlias); + joinedRelations.set(fullPath, nextAlias); + } + + const inverseEntityMetadataIsTree = isTreeEntityMetadata(relationMetadata.inverseEntityMetadata); + + if (!parentMetadataIsTree && !inverseEntityMetadataIsTree && !currentMetadataIsTree) { + return; + } + + const newEagerDepth = relationMetadata.isEager ? eagerDepth + 1 : eagerDepth; + + if (newEagerDepth <= maxEagerDepth) { + relationMetadata.inverseEntityMetadata.relations.forEach(subRelation => { + if (subRelation.isEager) { + processRelation( + relationMetadata.inverseEntityMetadata, + sourceMetadataIsTree, + subRelation.propertyPath, + nextAlias, + [fullPath], + newEagerDepth, + ); + } + }); + } + + if (nextPath) { + processRelation( + relationMetadata.inverseEntityMetadata, + currentMetadataIsTree || parentMetadataIsTree, + nextPath, + nextAlias, + [fullPath], + ); } }; requestedRelations.forEach(relationPath => { - processRelation(sourceMetadata, sourceMetadataIsTree, relationPath, qb.alias); + if (!joinedRelations.has(relationPath)) { + processRelation(sourceMetadata, sourceMetadataIsTree, relationPath, qb.alias); + } }); return joinedRelations; From b54f0f0cc5b9db3f38f8cfc5993e7b4052635efa Mon Sep 17 00:00:00 2001 From: Eugene Nitsenko Date: Tue, 19 Mar 2024 18:25:45 +0100 Subject: [PATCH 14/20] feat(core): Clean up code and remove joinEagerRelations helper --- .../connection/transactional-connection.ts | 21 ++++++++----------- .../helpers/utils/tree-relations-qb-joiner.ts | 2 +- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/packages/core/src/connection/transactional-connection.ts b/packages/core/src/connection/transactional-connection.ts index 37f6d60e76..8e748973df 100644 --- a/packages/core/src/connection/transactional-connection.ts +++ b/packages/core/src/connection/transactional-connection.ts @@ -1,5 +1,6 @@ import { Injectable } from '@nestjs/common'; import { InjectConnection } from '@nestjs/typeorm'; +import { InjectDataSource } from '@nestjs/typeorm/dist/common/typeorm.decorators'; import { ID, Type } from '@vendure/common/lib/shared-types'; import { Connection, @@ -9,8 +10,10 @@ import { FindOptionsUtils, ObjectLiteral, ObjectType, - Repository, SelectQueryBuilder, + Repository, + SelectQueryBuilder, } from 'typeorm'; +import { DataSource } from 'typeorm/data-source/DataSource'; import { FindManyOptions } from 'typeorm/find-options/FindManyOptions'; import { RequestContext } from '../api/common/request-context'; @@ -19,12 +22,10 @@ import { TRANSACTION_MANAGER_KEY } from '../common/constants'; import { EntityNotFoundError } from '../common/error/errors'; import { ChannelAware, SoftDeletable } from '../common/types/common-types'; import { VendureEntity } from '../entity/base/base.entity'; +import { joinTreeRelationsDynamically } from '../service/helpers/utils/tree-relations-qb-joiner'; import { TransactionWrapper } from './transaction-wrapper'; import { GetEntityOrThrowOptions } from './types'; -import { DataSource } from 'typeorm/data-source/DataSource'; -import { InjectDataSource } from '@nestjs/typeorm/dist/common/typeorm.decorators'; -import { joinTreeRelationsDynamically } from '../service/helpers/utils/tree-relations-qb-joiner'; /** * @description @@ -278,7 +279,7 @@ export class TransactionalConnection { channelId: ID, options: FindOneOptions = {}, ) { - const qb = this.getRepository(ctx, entity).createQueryBuilder('entity') + const qb = this.getRepository(ctx, entity).createQueryBuilder('entity'); if (Array.isArray(options.relations) && options.relations.length > 0) { const joinedRelations = joinTreeRelationsDynamically(qb, entity, options.relations); @@ -288,17 +289,15 @@ export class TransactionalConnection { } qb.setFindOptions({ relationLoadStrategy: 'query', // default to query strategy for maximum performance - ...options + ...options, }); - FindOptionsUtils.joinEagerRelations(qb, qb.alias, qb.expressionMap.mainAlias!.metadata) - qb.leftJoin('entity.channels', '__channel') .andWhere('entity.id = :id', { id }) .andWhere('__channel.id = :channelId', { channelId }); return qb.getOne().then(result => { - return result ?? undefined + return result ?? undefined; }); } @@ -335,11 +334,9 @@ export class TransactionalConnection { qb.setFindOptions({ relationLoadStrategy: 'query', // default to query strategy for maximum performance - ...options + ...options, }); - FindOptionsUtils.joinEagerRelations(qb, qb.alias, qb.expressionMap.mainAlias!.metadata) - return qb .leftJoin('entity.channels', 'channel') .andWhere('entity.id IN (:...ids)', { ids }) diff --git a/packages/core/src/service/helpers/utils/tree-relations-qb-joiner.ts b/packages/core/src/service/helpers/utils/tree-relations-qb-joiner.ts index 9d6f7376b2..4a9511c6e2 100644 --- a/packages/core/src/service/helpers/utils/tree-relations-qb-joiner.ts +++ b/packages/core/src/service/helpers/utils/tree-relations-qb-joiner.ts @@ -85,7 +85,7 @@ export function joinTreeRelationsDynamically( currentPath: string, currentAlias: string, parentPath?: string[], - eagerDepth: number = 0, // Текущий уровень глубины для eager связей + eagerDepth: number = 0, ) => { if (currentPath === '') { return; From 7af0d4a53cfc5cf4baf09a752f92b10add7ee22f Mon Sep 17 00:00:00 2001 From: Eugene Nitsenko Date: Tue, 19 Mar 2024 18:39:28 +0100 Subject: [PATCH 15/20] feat(core): Fix import in the unit test case --- .../src/common/utilities/create-updated-translatable.spec.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/admin-ui/src/lib/core/src/common/utilities/create-updated-translatable.spec.ts b/packages/admin-ui/src/lib/core/src/common/utilities/create-updated-translatable.spec.ts index 19914c06fa..b75ac07680 100644 --- a/packages/admin-ui/src/lib/core/src/common/utilities/create-updated-translatable.spec.ts +++ b/packages/admin-ui/src/lib/core/src/common/utilities/create-updated-translatable.spec.ts @@ -4,7 +4,6 @@ import { describe, expect, it, beforeEach } from 'vitest'; import { CustomFieldConfig, LanguageCode, ProductDetailFragment } from '../generated-types'; import { createUpdatedTranslatable } from './create-updated-translatable'; -import { fail } from 'assert'; /* eslint-disable @typescript-eslint/no-non-null-assertion */ describe('createUpdatedTranslatable()', () => { From 5f836b9fc8f920a25ee4bc3269a35968375c0553 Mon Sep 17 00:00:00 2001 From: Eugene Nitsenko Date: Tue, 19 Mar 2024 18:43:44 +0100 Subject: [PATCH 16/20] feat(core): Fix import in the unit test case --- .../src/common/utilities/create-updated-translatable.spec.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/admin-ui/src/lib/core/src/common/utilities/create-updated-translatable.spec.ts b/packages/admin-ui/src/lib/core/src/common/utilities/create-updated-translatable.spec.ts index b75ac07680..219c7bd1c7 100644 --- a/packages/admin-ui/src/lib/core/src/common/utilities/create-updated-translatable.spec.ts +++ b/packages/admin-ui/src/lib/core/src/common/utilities/create-updated-translatable.spec.ts @@ -1,5 +1,4 @@ import { DeepPartial } from '@vendure/common/lib/shared-types'; -import { describe, expect, it, beforeEach } from 'vitest'; import { CustomFieldConfig, LanguageCode, ProductDetailFragment } from '../generated-types'; From 0dfb4f61582fd09fa8ef404f9091d068ba5b415d Mon Sep 17 00:00:00 2001 From: Eugene Nitsenko Date: Tue, 19 Mar 2024 19:13:03 +0100 Subject: [PATCH 17/20] feat(core): Fix condition for tree type of all relation entities --- .../service/helpers/utils/tree-relations-qb-joiner.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/core/src/service/helpers/utils/tree-relations-qb-joiner.ts b/packages/core/src/service/helpers/utils/tree-relations-qb-joiner.ts index 4a9511c6e2..c98cf182ff 100644 --- a/packages/core/src/service/helpers/utils/tree-relations-qb-joiner.ts +++ b/packages/core/src/service/helpers/utils/tree-relations-qb-joiner.ts @@ -91,8 +91,9 @@ export function joinTreeRelationsDynamically( return; } parentPath = parentPath?.filter(p => p !== ''); - const currentMetadataIsTree = isTreeEntityMetadata(currentMetadata) || sourceMetadataIsTree; - if (!currentMetadataIsTree && !parentMetadataIsTree) { + const currentMetadataIsTree = + isTreeEntityMetadata(currentMetadata) || sourceMetadataIsTree || parentMetadataIsTree; + if (!currentMetadataIsTree) { return; } @@ -127,7 +128,7 @@ export function joinTreeRelationsDynamically( const inverseEntityMetadataIsTree = isTreeEntityMetadata(relationMetadata.inverseEntityMetadata); - if (!parentMetadataIsTree && !inverseEntityMetadataIsTree && !currentMetadataIsTree) { + if (!currentMetadataIsTree && !inverseEntityMetadataIsTree) { return; } @@ -138,7 +139,7 @@ export function joinTreeRelationsDynamically( if (subRelation.isEager) { processRelation( relationMetadata.inverseEntityMetadata, - sourceMetadataIsTree, + currentMetadataIsTree, subRelation.propertyPath, nextAlias, [fullPath], @@ -151,7 +152,7 @@ export function joinTreeRelationsDynamically( if (nextPath) { processRelation( relationMetadata.inverseEntityMetadata, - currentMetadataIsTree || parentMetadataIsTree, + currentMetadataIsTree, nextPath, nextAlias, [fullPath], From 8a15a26a955f00943a486ec177aef951aa177f70 Mon Sep 17 00:00:00 2001 From: Eugene Nitsenko Date: Wed, 20 Mar 2024 09:42:07 +0100 Subject: [PATCH 18/20] chore(core): Code style fixes --- packages/core/src/connection/transactional-connection.ts | 6 ++---- packages/core/src/entity/asset/orderable-asset.entity.ts | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/core/src/connection/transactional-connection.ts b/packages/core/src/connection/transactional-connection.ts index 8e748973df..8ce1eaecf7 100644 --- a/packages/core/src/connection/transactional-connection.ts +++ b/packages/core/src/connection/transactional-connection.ts @@ -3,18 +3,16 @@ import { InjectConnection } from '@nestjs/typeorm'; import { InjectDataSource } from '@nestjs/typeorm/dist/common/typeorm.decorators'; import { ID, Type } from '@vendure/common/lib/shared-types'; import { - Connection, + DataSource, EntityManager, EntitySchema, FindOneOptions, - FindOptionsUtils, + FindManyOptions, ObjectLiteral, ObjectType, Repository, SelectQueryBuilder, } from 'typeorm'; -import { DataSource } from 'typeorm/data-source/DataSource'; -import { FindManyOptions } from 'typeorm/find-options/FindManyOptions'; import { RequestContext } from '../api/common/request-context'; import { TransactionIsolationLevel } from '../api/decorators/transaction.decorator'; diff --git a/packages/core/src/entity/asset/orderable-asset.entity.ts b/packages/core/src/entity/asset/orderable-asset.entity.ts index e8b7b6b922..507385f49a 100644 --- a/packages/core/src/entity/asset/orderable-asset.entity.ts +++ b/packages/core/src/entity/asset/orderable-asset.entity.ts @@ -24,7 +24,7 @@ export abstract class OrderableAsset extends VendureEntity implements Orderable assetId: ID; @Index() - @ManyToOne(type => Asset, { eager: true, onDelete: 'CASCADE' }) + @ManyToOne(type => Asset, { eager: true, onDelete: 'CASCADE' }) asset: Asset; @Column() From d6c50d5a32853af31adac0aed9a053dc30c589c4 Mon Sep 17 00:00:00 2001 From: Eugene Nitsenko Date: Wed, 20 Mar 2024 09:47:19 +0100 Subject: [PATCH 19/20] docs(core): Update docs for joinTreeRelationsDynamically --- .../helpers/utils/tree-relations-qb-joiner.ts | 55 +++++++------------ 1 file changed, 21 insertions(+), 34 deletions(-) diff --git a/packages/core/src/service/helpers/utils/tree-relations-qb-joiner.ts b/packages/core/src/service/helpers/utils/tree-relations-qb-joiner.ts index c98cf182ff..35d4e7fbdc 100644 --- a/packages/core/src/service/helpers/utils/tree-relations-qb-joiner.ts +++ b/packages/core/src/service/helpers/utils/tree-relations-qb-joiner.ts @@ -1,13 +1,3 @@ -/** - * Verifies if a relation has already been joined in a query builder to prevent duplicate joins. - * This method ensures query efficiency and correctness by maintaining unique joins within the query builder. - * - * @param {SelectQueryBuilder} qb The query builder instance where the joins are being added. - * @param {string} alias The join alias to check for uniqueness. This alias is used to determine if the relation - * has already been joined to avoid adding duplicate join statements. - * @returns boolean Returns true if the relation has already been joined (based on the alias), false otherwise. - * @template T extends VendureEntity The entity type for which the query builder is configured. - */ import { EntityMetadata, SelectQueryBuilder } from 'typeorm'; import { EntityTarget } from 'typeorm/common/EntityTarget'; @@ -37,33 +27,30 @@ function isTreeEntityMetadata(metadata: EntityMetadata): boolean { } /** - * These method are designed to address specific challenges encountered with TypeORM - * when dealing with complex relation structures, particularly around the 'collection' - * entity and other similar entities, and they nested relations ('parent', 'children'). The need for these custom - * implementations arises from limitations in handling deeply nested relations and ensuring - * efficient query generation without duplicate joins, as discussed in TypeORM issue #9936. - * See https://github.com/typeorm/typeorm/issues/9936 for more context. + * Dynamically joins tree relations and their eager counterparts in a TypeORM SelectQueryBuilder, addressing + * challenges of managing deeply nested relations and optimizing query efficiency. It leverages TypeORM tree + * decorators (@TreeParent, @TreeChildren) to automate joins of self-related entities, including those marked for eager loading. + * The process avoids duplicate joins and manual join specifications by using relation metadata. + * + * @param {SelectQueryBuilder} qb - The query builder instance for joining relations. + * @param {EntityTarget} entity - The target entity class or schema name, used to access entity metadata. + * @param {string[]} [requestedRelations=[]] - An array of relation paths (e.g., 'parent.children') to join dynamically. + * @param {number} [maxEagerDepth=1] - Limits the depth of eager relation joins to avoid excessively deep joins. + * @returns {Map} - A Map of joined relation paths to their aliases, aiding in tracking and preventing duplicates. + * @template T - The entity type, extending VendureEntity for compatibility with Vendure's data layer. * - * Dynamically joins tree relations and their eager relations to a query builder. This method is specifically - * designed for entities utilizing TypeORM tree decorators (@TreeParent, @TreeChildren) and aims to address - * the challenge of efficiently managing deeply nested relations and avoiding duplicate joins. The method - * automatically handles the joining of related entities marked with tree relation decorators and eagerly - * loaded relations, ensuring efficient data retrieval and query generation. + * Usage Notes: + * - Only entities utilizing TypeORM tree decorators and having nested relations are supported. + * - The `maxEagerDepth` parameter controls the recursion depth for eager relations, preventing performance issues. * - * The method iterates over the requested relations paths, joining each relation dynamically. For tree relations, - * it also recursively joins all associated eager relations. This approach avoids the manual specification of joins - * and leverages TypeORM's relation metadata to automate the process. + * For more context on the issue this function addresses, refer to TypeORM issue #9936: + * https://github.com/typeorm/typeorm/issues/9936 * - * @param {SelectQueryBuilder} qb The query builder instance to which the relations will be joined. - * @param {EntityTarget} entity The target entity class or schema name. This parameter is used to access - * the entity's metadata and analyze its relations. - * @param {string[]} requestedRelations An array of strings representing the relation paths to be dynamically joined. - * Each string in the array should denote a path to a relation (e.g., 'parent.parent.children'). - * @param maxEagerDepth The maximum depth of eager relations to join. This parameter is used to limit the depth of eager relations. - * @returns {Map} A Map containing the paths of relations that were dynamically joined with their aliases. This map can be used - * to track which relations have been processed and potentially avoid duplicate processing. - * @template T extends VendureEntity The type of the entity for which relations are being joined. This type parameter - * should extend VendureEntity to ensure compatibility with Vendure's data access layer. + * Example: + * ```typescript + * const qb = repository.createQueryBuilder("entity"); + * joinTreeRelationsDynamically(qb, EntityClass, ["parent.children"], 2); + * ``` */ export function joinTreeRelationsDynamically( qb: SelectQueryBuilder, From a444894edab3b52b2f23fb0b58684377db1dbc0b Mon Sep 17 00:00:00 2001 From: Eugene Nitsenko Date: Wed, 20 Mar 2024 09:56:02 +0100 Subject: [PATCH 20/20] chore(core): Remove unused import --- packages/core/src/connection/transactional-connection.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/core/src/connection/transactional-connection.ts b/packages/core/src/connection/transactional-connection.ts index 8ce1eaecf7..63366fc4a2 100644 --- a/packages/core/src/connection/transactional-connection.ts +++ b/packages/core/src/connection/transactional-connection.ts @@ -1,5 +1,4 @@ import { Injectable } from '@nestjs/common'; -import { InjectConnection } from '@nestjs/typeorm'; import { InjectDataSource } from '@nestjs/typeorm/dist/common/typeorm.decorators'; import { ID, Type } from '@vendure/common/lib/shared-types'; import {