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

Aggregated optimization changes #2744

Merged
merged 24 commits into from
Mar 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
4034445
fix(core): self-referencing relations `Not unique table/alias`
monrostar Mar 15, 2024
16cfc07
perf(core): upgrade sql requests for more performant memory usage wit…
monrostar Mar 15, 2024
469a02b
perf(core): Upgrade EntityHydrator to add more performance to any hyd…
monrostar Mar 15, 2024
10c0798
fix(core): update self-related manual joins with deep eager relations
monrostar Mar 16, 2024
4824d86
chore(core): clean up code
monrostar Mar 16, 2024
5eb6391
fix(core): update test cases for new TestEntities order
monrostar Mar 16, 2024
0750065
feat(core): optimization for assignToChannels method
monrostar Mar 16, 2024
dc463ba
feat(core): fix changes for new getAssignedEntityChannels method
monrostar Mar 16, 2024
4486cd2
Merge branch 'perf/performance-improvement-in-entity-hydrator' into f…
monrostar Mar 16, 2024
949dc54
Merge branch 'perf/optimize-assign-to-channels' into feat/aggregated-prs
monrostar Mar 16, 2024
85821b7
Merge branch 'perf/product-soft-delete-performance-update' into feat/…
monrostar Mar 16, 2024
c376eef
feat(core): add optimization into findOneInChannel and more refactoring
monrostar Mar 16, 2024
661c2e9
perf(core): optimize translatable-saver.ts query to get existingTrans…
monrostar Mar 17, 2024
eaca1c9
fix(core): Added processing of deeply nested relations that lack Reve…
monrostar Mar 17, 2024
11e3904
fix(core): fix condition for tree type of metadata in list-query-builder
monrostar Mar 17, 2024
d1494bd
Merge branch 'minor' into feat/aggregated-prs
monrostar Mar 19, 2024
c92bf6d
feat(core): Added edge case handling with customFields as well as dep…
monrostar Mar 19, 2024
b54f0f0
feat(core): Clean up code and remove joinEagerRelations helper
monrostar Mar 19, 2024
7af0d4a
feat(core): Fix import in the unit test case
monrostar Mar 19, 2024
5f836b9
feat(core): Fix import in the unit test case
monrostar Mar 19, 2024
0dfb4f6
feat(core): Fix condition for tree type of all relation entities
monrostar Mar 19, 2024
8a15a26
chore(core): Code style fixes
monrostar Mar 20, 2024
d6c50d5
docs(core): Update docs for joinTreeRelationsDynamically
monrostar Mar 20, 2024
a444894
chore(core): Remove unused import
monrostar Mar 20, 2024
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
Expand Up @@ -17,7 +17,10 @@ import { isEntityCreateOrUpdateMutation } from '../utils/is-entity-create-or-upd

@Injectable()
export class BaseDataService {
constructor(private apollo: Apollo, private serverConfigService: ServerConfigService) {}
constructor(
private apollo: Apollo,
private serverConfigService: ServerConfigService,
) {}

private get customFields(): Map<string, CustomFieldConfig[]> {
return this.serverConfigService.customFieldsMap;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,10 @@ export class AssetsComponent {
@Input()
updatePermissions: string | string[] | Permission | Permission[];

constructor(private modalService: ModalService, private changeDetector: ChangeDetectorRef) {}
constructor(
private modalService: ModalService,
private changeDetector: ChangeDetectorRef,
) {}

selectAssets() {
this.modalService
Expand Down
37 changes: 16 additions & 21 deletions packages/core/e2e/shop-order.e2e-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,8 @@ describe('Shop orders', () => {
},
);

const result = await shopClient.query<CodegenShop.GetAvailableCountriesQuery>(
GET_AVAILABLE_COUNTRIES,
);
const result =
await shopClient.query<CodegenShop.GetAvailableCountriesQuery>(GET_AVAILABLE_COUNTRIES);
expect(result.availableCountries.length).toBe(countries.items.length - 1);
expect(result.availableCountries.find(c => c.id === AT.id)).toBeUndefined();
});
Expand Down Expand Up @@ -1519,9 +1518,8 @@ describe('Shop orders', () => {
id: shippingMethods[1].id,
});

const activeOrderResult = await shopClient.query<CodegenShop.GetActiveOrderQuery>(
GET_ACTIVE_ORDER,
);
const activeOrderResult =
await shopClient.query<CodegenShop.GetActiveOrderQuery>(GET_ACTIVE_ORDER);

const order = activeOrderResult.activeOrder!;

Expand All @@ -1533,9 +1531,8 @@ describe('Shop orders', () => {
});

it('shipping method is preserved after adjustOrderLine', async () => {
const activeOrderResult = await shopClient.query<CodegenShop.GetActiveOrderQuery>(
GET_ACTIVE_ORDER,
);
const activeOrderResult =
await shopClient.query<CodegenShop.GetActiveOrderQuery>(GET_ACTIVE_ORDER);
activeOrder = activeOrderResult.activeOrder!;
const { adjustOrderLine } = await shopClient.query<
CodegenShop.AdjustItemQuantityMutation,
Expand Down Expand Up @@ -1709,9 +1706,10 @@ describe('Shop orders', () => {
expect(addPaymentToOrder.errorCode).toBe(ErrorCode.PAYMENT_FAILED_ERROR);
expect((addPaymentToOrder as any).paymentErrorMessage).toBe('Something went horribly wrong');

const result = await shopClient.query<CodegenShop.GetActiveOrderPaymentsQuery>(
GET_ACTIVE_ORDER_PAYMENTS,
);
const result =
await shopClient.query<CodegenShop.GetActiveOrderPaymentsQuery>(
GET_ACTIVE_ORDER_PAYMENTS,
);
const payment = result.activeOrder!.payments![1];
expect(result.activeOrder!.payments!.length).toBe(2);
expect(payment.method).toBe(testErrorPaymentMethod.code);
Expand Down Expand Up @@ -2059,9 +2057,8 @@ describe('Shop orders', () => {
},
});

const { activeOrder } = await shopClient.query<CodegenShop.GetCustomerAddressesQuery>(
GET_ACTIVE_ORDER_ADDRESSES,
);
const { activeOrder } =
await shopClient.query<CodegenShop.GetCustomerAddressesQuery>(GET_ACTIVE_ORDER_ADDRESSES);

expect(activeOrder!.customer!.addresses).toEqual([]);
});
Expand All @@ -2087,9 +2084,8 @@ describe('Shop orders', () => {
},
});

const { activeOrder } = await shopClient.query<CodegenShop.GetCustomerOrdersQuery>(
GET_ACTIVE_ORDER_ORDERS,
);
const { activeOrder } =
await shopClient.query<CodegenShop.GetCustomerOrdersQuery>(GET_ACTIVE_ORDER_ORDERS);

expect(activeOrder!.customer!.orders.items).toEqual([]);
});
Expand Down Expand Up @@ -2328,9 +2324,8 @@ describe('Shop orders', () => {

beforeAll(async () => {
// First we will remove all ShippingMethods and set up 2 specialized ones
const { shippingMethods } = await adminClient.query<Codegen.GetShippingMethodListQuery>(
GET_SHIPPING_METHOD_LIST,
);
const { shippingMethods } =
await adminClient.query<Codegen.GetShippingMethodListQuery>(GET_SHIPPING_METHOD_LIST);
for (const method of shippingMethods.items) {
await adminClient.query<
Codegen.DeleteShippingMethodMutation,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ export class CustomFieldRelationResolverService {
const translated: any = Array.isArray(result)
? result.map(r => (this.isTranslatable(r) ? this.translator.translate(r, ctx) : r))
: this.isTranslatable(result)
? this.translator.translate(result, ctx)
: result;
? this.translator.translate(result, ctx)
: result;

return translated;
}
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/api/resolvers/admin/collection.resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,8 @@ export class CollectionResolver {
): Promise<Translated<Collection>> {
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()
Expand Down
56 changes: 40 additions & 16 deletions packages/core/src/connection/transactional-connection.ts
Original file line number Diff line number Diff line change
@@ -1,24 +1,25 @@
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,
DataSource,
EntityManager,
EntitySchema,
FindOneOptions,
FindOptionsUtils,
FindManyOptions,
ObjectLiteral,
ObjectType,
Repository,
SelectQueryBuilder,
} from 'typeorm';
import { FindManyOptions } from 'typeorm/find-options/FindManyOptions';

import { RequestContext } from '../api/common/request-context';
import { TransactionIsolationLevel } from '../api/decorators/transaction.decorator';
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';
Expand All @@ -38,7 +39,7 @@ import { GetEntityOrThrowOptions } from './types';
@Injectable()
export class TransactionalConnection {
constructor(
@InjectConnection() private connection: Connection,
@InjectDataSource() private dataSource: DataSource,
private transactionWrapper: TransactionWrapper,
) {}

Expand All @@ -48,8 +49,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;
}

/**
Expand Down Expand Up @@ -275,16 +276,26 @@ export class TransactionalConnection {
channelId: ID,
options: FindOneOptions<T> = {},
) {
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,
});

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;
});
}

/**
Expand All @@ -305,11 +316,24 @@ 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<VendureEntity>,
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,
});

return qb
.leftJoin('entity.channels', 'channel')
.andWhere('entity.id IN (:...ids)', { ids })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ import { InternalServerError } from '../../../common/error/errors';
import { TransactionalConnection } from '../../../connection/transactional-connection';
import { VendureEntity } from '../../../entity/base/base.entity';
import { ProductVariant } from '../../../entity/product-variant/product-variant.entity';
import { ListQueryBuilder } from '../list-query-builder/list-query-builder';
import { ProductPriceApplicator } from '../product-price-applicator/product-price-applicator';
import { TranslatorService } from '../translator/translator.service';
import { joinTreeRelationsDynamically } from '../utils/tree-relations-qb-joiner';

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

Expand Down Expand Up @@ -80,7 +80,6 @@ export class EntityHydrator {
private connection: TransactionalConnection,
private productPriceApplicator: ProductPriceApplicator,
private translator: TranslatorService,
private listQueryBuilder: ListQueryBuilder,
) {}

/**
Expand Down Expand Up @@ -122,15 +121,15 @@ export class EntityHydrator {
const hydratedQb: SelectQueryBuilder<any> = this.connection
.getRepository(ctx, target.constructor)
.createQueryBuilder(target.constructor.name);
const processedRelations = this.listQueryBuilder.joinTreeRelationsDynamically(
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]));
Expand Down
Loading
Loading