fix: resolve N+1 query issue in view name resolver with DataLoaders (#14037)
## What Fixed N+1 query issue where each view's name resolution triggered a separate DB query to fetch objectMetadataId, even though it was already available in the parent view object. ## How - Added `objectMetadataLoader` to DataloaderService to batch metadata lookups - Updated ViewResolver to use context.loaders (following existing pattern from other metadata resolvers) - Removed unused `getObjectMetadataByViewId` method and its tests Now uses the already-loaded `view.objectMetadataId` directly instead of making unnecessary queries. Much cleaner!
This commit is contained in:
@@ -13,6 +13,7 @@ import { isDefined } from 'twenty-shared/utils';
|
||||
|
||||
import { I18nService } from 'src/engine/core-modules/i18n/i18n.service';
|
||||
import { type I18nContext } from 'src/engine/core-modules/i18n/types/i18n-context.type';
|
||||
import { type IDataloaders } from 'src/engine/dataloaders/dataloader.interface';
|
||||
import { generateMessageId } from 'src/engine/core-modules/i18n/utils/generateMessageId';
|
||||
import { CreateViewInput } from 'src/engine/core-modules/view/dtos/inputs/create-view.input';
|
||||
import { UpdateViewInput } from 'src/engine/core-modules/view/dtos/inputs/update-view.input';
|
||||
@@ -51,14 +52,14 @@ export class ViewResolver {
|
||||
@ResolveField(() => String)
|
||||
async name(
|
||||
@Parent() view: ViewDTO,
|
||||
@Context() context: I18nContext,
|
||||
@Context() context: { loaders: IDataloaders } & I18nContext,
|
||||
@AuthWorkspace() workspace: Workspace,
|
||||
): Promise<string> {
|
||||
if (view.name.includes('{objectLabelPlural}')) {
|
||||
const objectMetadata = await this.viewService.getObjectMetadataByViewId(
|
||||
view.id,
|
||||
workspace.id,
|
||||
);
|
||||
const objectMetadata = await context.loaders.objectMetadataLoader.load({
|
||||
objectMetadataId: view.objectMetadataId,
|
||||
workspaceId: workspace.id,
|
||||
});
|
||||
|
||||
if (objectMetadata) {
|
||||
const translatedObjectLabel = resolveObjectMetadataStandardOverride(
|
||||
|
||||
-106
@@ -14,12 +14,10 @@ import {
|
||||
generateViewUserFriendlyExceptionMessage,
|
||||
} from 'src/engine/core-modules/view/exceptions/view.exception';
|
||||
import { ViewService } from 'src/engine/core-modules/view/services/view.service';
|
||||
import { WorkspaceMetadataCacheService } from 'src/engine/metadata-modules/workspace-metadata-cache/services/workspace-metadata-cache.service';
|
||||
|
||||
describe('ViewService', () => {
|
||||
let viewService: ViewService;
|
||||
let viewRepository: Repository<View>;
|
||||
let workspaceMetadataCacheService: WorkspaceMetadataCacheService;
|
||||
|
||||
const mockView = {
|
||||
id: 'view-id',
|
||||
@@ -56,12 +54,6 @@ describe('ViewService', () => {
|
||||
delete: jest.fn(),
|
||||
},
|
||||
},
|
||||
{
|
||||
provide: WorkspaceMetadataCacheService,
|
||||
useValue: {
|
||||
getExistingOrRecomputeMetadataMaps: jest.fn(),
|
||||
},
|
||||
},
|
||||
],
|
||||
}).compile();
|
||||
|
||||
@@ -69,9 +61,6 @@ describe('ViewService', () => {
|
||||
viewRepository = module.get<Repository<View>>(
|
||||
getRepositoryToken(View, 'core'),
|
||||
);
|
||||
workspaceMetadataCacheService = module.get<WorkspaceMetadataCacheService>(
|
||||
WorkspaceMetadataCacheService,
|
||||
);
|
||||
});
|
||||
|
||||
it('should be defined', () => {
|
||||
@@ -327,99 +316,4 @@ describe('ViewService', () => {
|
||||
expect(result).toEqual(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('getObjectMetadataByViewId', () => {
|
||||
it('should return object metadata for a view', async () => {
|
||||
const viewId = 'view-id';
|
||||
const workspaceId = 'workspace-id';
|
||||
const objectMetadataId = 'object-id';
|
||||
const mockObjectMetadata = {
|
||||
id: objectMetadataId,
|
||||
nameSingular: 'TestObject',
|
||||
namePlural: 'TestObjects',
|
||||
labelSingular: 'Test Object',
|
||||
labelPlural: 'Test Objects',
|
||||
};
|
||||
|
||||
jest.spyOn(viewRepository, 'findOne').mockResolvedValue({
|
||||
objectMetadataId,
|
||||
} as View);
|
||||
|
||||
jest
|
||||
.spyOn(
|
||||
workspaceMetadataCacheService,
|
||||
'getExistingOrRecomputeMetadataMaps',
|
||||
)
|
||||
.mockResolvedValue({
|
||||
objectMetadataMaps: {
|
||||
byId: {
|
||||
[objectMetadataId]: mockObjectMetadata,
|
||||
},
|
||||
idByNameSingular: {},
|
||||
},
|
||||
metadataVersion: 1,
|
||||
} as any);
|
||||
|
||||
const result = await viewService.getObjectMetadataByViewId(
|
||||
viewId,
|
||||
workspaceId,
|
||||
);
|
||||
|
||||
expect(viewRepository.findOne).toHaveBeenCalledWith({
|
||||
where: { id: viewId },
|
||||
select: ['objectMetadataId'],
|
||||
});
|
||||
expect(
|
||||
workspaceMetadataCacheService.getExistingOrRecomputeMetadataMaps,
|
||||
).toHaveBeenCalledWith({ workspaceId });
|
||||
expect(result).toEqual(mockObjectMetadata);
|
||||
});
|
||||
|
||||
it('should return null when view is not found', async () => {
|
||||
const viewId = 'non-existent-id';
|
||||
const workspaceId = 'workspace-id';
|
||||
|
||||
jest.spyOn(viewRepository, 'findOne').mockResolvedValue(null);
|
||||
|
||||
const result = await viewService.getObjectMetadataByViewId(
|
||||
viewId,
|
||||
workspaceId,
|
||||
);
|
||||
|
||||
expect(result).toBeNull();
|
||||
expect(
|
||||
workspaceMetadataCacheService.getExistingOrRecomputeMetadataMaps,
|
||||
).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should return null when object metadata is not in cache', async () => {
|
||||
const viewId = 'view-id';
|
||||
const workspaceId = 'workspace-id';
|
||||
const objectMetadataId = 'object-id';
|
||||
|
||||
jest.spyOn(viewRepository, 'findOne').mockResolvedValue({
|
||||
objectMetadataId,
|
||||
} as View);
|
||||
|
||||
jest
|
||||
.spyOn(
|
||||
workspaceMetadataCacheService,
|
||||
'getExistingOrRecomputeMetadataMaps',
|
||||
)
|
||||
.mockResolvedValue({
|
||||
objectMetadataMaps: {
|
||||
byId: {},
|
||||
idByNameSingular: {},
|
||||
},
|
||||
metadataVersion: 1,
|
||||
} as any);
|
||||
|
||||
const result = await viewService.getObjectMetadataByViewId(
|
||||
viewId,
|
||||
workspaceId,
|
||||
);
|
||||
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -5,8 +5,6 @@ import { isDefined } from 'twenty-shared/utils';
|
||||
import { IsNull, Repository } from 'typeorm';
|
||||
|
||||
import { View } from 'src/engine/core-modules/view/entities/view.entity';
|
||||
import { type ObjectMetadataItemWithFieldMaps } from 'src/engine/metadata-modules/types/object-metadata-item-with-field-maps';
|
||||
import { WorkspaceMetadataCacheService } from 'src/engine/metadata-modules/workspace-metadata-cache/services/workspace-metadata-cache.service';
|
||||
import {
|
||||
ViewException,
|
||||
ViewExceptionCode,
|
||||
@@ -20,7 +18,6 @@ export class ViewService {
|
||||
constructor(
|
||||
@InjectRepository(View, 'core')
|
||||
private readonly viewRepository: Repository<View>,
|
||||
private readonly workspaceMetadataCacheService: WorkspaceMetadataCacheService,
|
||||
) {}
|
||||
|
||||
async findByWorkspaceId(workspaceId: string): Promise<View[]> {
|
||||
@@ -180,25 +177,4 @@ export class ViewService {
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
async getObjectMetadataByViewId(
|
||||
viewId: string,
|
||||
workspaceId: string,
|
||||
): Promise<ObjectMetadataItemWithFieldMaps | null> {
|
||||
const view = await this.viewRepository.findOne({
|
||||
where: { id: viewId },
|
||||
select: ['objectMetadataId'],
|
||||
});
|
||||
|
||||
if (!view) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const { objectMetadataMaps } =
|
||||
await this.workspaceMetadataCacheService.getExistingOrRecomputeMetadataMaps(
|
||||
{ workspaceId },
|
||||
);
|
||||
|
||||
return objectMetadataMaps.byId[view.objectMetadataId] || null;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -5,6 +5,7 @@ import {
|
||||
type IndexFieldMetadataLoaderPayload,
|
||||
type IndexMetadataLoaderPayload,
|
||||
type MorphRelationLoaderPayload,
|
||||
type ObjectMetadataLoaderPayload,
|
||||
type RelationLoaderPayload,
|
||||
} from 'src/engine/dataloaders/dataloader.service';
|
||||
import { type FieldMetadataDTO } from 'src/engine/metadata-modules/field-metadata/dtos/field-metadata.dto';
|
||||
@@ -12,6 +13,7 @@ import { type FieldMetadataEntity } from 'src/engine/metadata-modules/field-meta
|
||||
import { type IndexFieldMetadataDTO } from 'src/engine/metadata-modules/index-metadata/dtos/index-field-metadata.dto';
|
||||
import { type IndexMetadataDTO } from 'src/engine/metadata-modules/index-metadata/dtos/index-metadata.dto';
|
||||
import { type ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity';
|
||||
import { type ObjectMetadataItemWithFieldMaps } from 'src/engine/metadata-modules/types/object-metadata-item-with-field-maps';
|
||||
|
||||
export interface IDataloaders {
|
||||
relationLoader: DataLoader<
|
||||
@@ -48,4 +50,9 @@ export interface IDataloaders {
|
||||
IndexFieldMetadataLoaderPayload,
|
||||
IndexFieldMetadataDTO[]
|
||||
>;
|
||||
|
||||
objectMetadataLoader: DataLoader<
|
||||
ObjectMetadataLoaderPayload,
|
||||
ObjectMetadataItemWithFieldMaps | null
|
||||
>;
|
||||
}
|
||||
|
||||
@@ -8,6 +8,7 @@ import { type IndexMetadataInterface } from 'src/engine/metadata-modules/index-m
|
||||
|
||||
import { I18nService } from 'src/engine/core-modules/i18n/i18n.service';
|
||||
import { type IDataloaders } from 'src/engine/dataloaders/dataloader.interface';
|
||||
import { type ObjectMetadataItemWithFieldMaps } from 'src/engine/metadata-modules/types/object-metadata-item-with-field-maps';
|
||||
import { filterMorphRelationDuplicateFieldsDTO } from 'src/engine/dataloaders/utils/filter-morph-relation-duplicate-fields.util';
|
||||
import { type FieldMetadataDTO } from 'src/engine/metadata-modules/field-metadata/dtos/field-metadata.dto';
|
||||
import { type FieldMetadataEntity } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity';
|
||||
@@ -67,6 +68,11 @@ export type IndexFieldMetadataLoaderPayload = {
|
||||
indexMetadata: Pick<IndexMetadataInterface, 'id'>;
|
||||
};
|
||||
|
||||
export type ObjectMetadataLoaderPayload = {
|
||||
workspaceId: string;
|
||||
objectMetadataId: string;
|
||||
};
|
||||
|
||||
@Injectable()
|
||||
export class DataloaderService {
|
||||
constructor(
|
||||
@@ -82,6 +88,7 @@ export class DataloaderService {
|
||||
const fieldMetadataLoader = this.createFieldMetadataLoader();
|
||||
const indexMetadataLoader = this.createIndexMetadataLoader();
|
||||
const indexFieldMetadataLoader = this.createIndexFieldMetadataLoader();
|
||||
const objectMetadataLoader = this.createObjectMetadataLoader();
|
||||
|
||||
return {
|
||||
relationLoader,
|
||||
@@ -89,6 +96,7 @@ export class DataloaderService {
|
||||
fieldMetadataLoader,
|
||||
indexMetadataLoader,
|
||||
indexFieldMetadataLoader,
|
||||
objectMetadataLoader,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -308,4 +316,25 @@ export class DataloaderService {
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
private createObjectMetadataLoader() {
|
||||
return new DataLoader<
|
||||
ObjectMetadataLoaderPayload,
|
||||
ObjectMetadataItemWithFieldMaps | null
|
||||
>(async (dataLoaderParams: ObjectMetadataLoaderPayload[]) => {
|
||||
const workspaceId = dataLoaderParams[0].workspaceId;
|
||||
const objectMetadataIds = dataLoaderParams.map(
|
||||
(dataLoaderParam) => dataLoaderParam.objectMetadataId,
|
||||
);
|
||||
|
||||
const { objectMetadataMaps } =
|
||||
await this.workspaceMetadataCacheService.getExistingOrRecomputeMetadataMaps(
|
||||
{ workspaceId },
|
||||
);
|
||||
|
||||
return objectMetadataIds.map((objectMetadataId) => {
|
||||
return objectMetadataMaps.byId[objectMetadataId] || null;
|
||||
});
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user