Cache field metadata while processing common query results (#23353)
## Context `CommonResultGettersService` post-processes API query results after they are loaded. It recursively walks records and relations, identifies field metadata, and runs result handlers such as file URL signing. Production profiling of tail-latency requests showed local CPU concentrated in this service when processing large nested result sets. Before this change: - Field name maps were rebuilt for each recursive record-array call. A repeated one-to-many relation rebuilt the same child-object map once per parent. - Every record's keys were scanned three times, once for handlers, once for relations, and once for the metadata passed to handlers. - Each scan resolved names through an ID lookup. ORM-only keys such as join columns have no matching field metadata, and the non-throwing lookup handled those misses by throwing and catching an exception internally. This work is small for one record, but multiplies across every nested record and can block the Node.js event loop for large responses. ## What changed - Create an invocation-local processing context from the metadata maps already supplied to the service. - Build a `field name -> field metadata` map once per distinct object type. - Share that context across root records and recursive relation processing. - Scan each record once and reuse the resolved metadata for handlers and relations. - Resolve record keys with direct `Map.get` calls, so keys without metadata are skipped without entering an exception path. For example, when the same child object type is visited under 200 parent records, field-map preparation drops from 201 builds to 2, one for each distinct object type. Per-record metadata scans drop from three to one. ## Why this is safe - The cache exists only for one public `processRecord` or `processRecordArray` invocation. It is not stored on the singleton service and cannot retain metadata across requests or workspaces. - Handler selection, execution order, and existing duplicate-handler behavior are preserved. - Relation traversal order and relation-type behavior are unchanged. - Record keys without field metadata remain in the returned object. - Query selection, database access, pagination, and response shape are unchanged. ## Expected impact This removes repeated metadata preparation, array allocation, and exception construction from the hot path. The improvement should be most visible in API tail latency and event-loop delay for wide nested responses. Small responses should see little change. This does not reduce database time or the size of large responses. Response fan-out remains a separate concern if those requests are still too expensive after this optimization. ## Tests - Verify field handlers still run and fields without metadata are preserved. - Verify nested one-to-many records keep their output and ordering. - Verify field metadata is resolved once per distinct object type within a single invocation, and rebuilt on the next invocation.
This commit is contained in:
Weiko
+142
-12
@@ -1,10 +1,12 @@
|
||||
import {
|
||||
FieldMetadataType,
|
||||
FileFolder,
|
||||
RelationType,
|
||||
type ObjectRecord,
|
||||
} from 'twenty-shared/types';
|
||||
|
||||
import { CommonResultGettersService } from 'src/engine/api/common/common-result-getters/common-result-getters.service';
|
||||
import { getFlatFieldsFromFlatObjectMetadata } from 'src/engine/api/graphql/workspace-schema-builder/utils/get-flat-fields-for-flat-object-metadata.util';
|
||||
import { type FileUrlService } from 'src/engine/core-modules/file/file-url/file-url.service';
|
||||
import { createEmptyFlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/constant/create-empty-flat-entity-maps.constant';
|
||||
import { type FlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/types/flat-entity-maps.type';
|
||||
@@ -14,6 +16,22 @@ import { type FlatFieldMetadata } from 'src/engine/metadata-modules/flat-field-m
|
||||
import { getFlatObjectMetadataMock } from 'src/engine/metadata-modules/flat-object-metadata/__mocks__/get-flat-object-metadata.mock';
|
||||
import { type FlatObjectMetadata } from 'src/engine/metadata-modules/flat-object-metadata/types/flat-object-metadata.type';
|
||||
|
||||
jest.mock(
|
||||
'src/engine/api/graphql/workspace-schema-builder/utils/get-flat-fields-for-flat-object-metadata.util',
|
||||
() => {
|
||||
const actual = jest.requireActual(
|
||||
'src/engine/api/graphql/workspace-schema-builder/utils/get-flat-fields-for-flat-object-metadata.util',
|
||||
);
|
||||
|
||||
return {
|
||||
...actual,
|
||||
getFlatFieldsFromFlatObjectMetadata: jest.fn(
|
||||
actual.getFlatFieldsFromFlatObjectMetadata,
|
||||
),
|
||||
};
|
||||
},
|
||||
);
|
||||
|
||||
const createField = ({
|
||||
id,
|
||||
objectMetadataId,
|
||||
@@ -28,31 +46,39 @@ const createField = ({
|
||||
...overrides,
|
||||
});
|
||||
|
||||
const buildFlatFieldMetadataMaps = (
|
||||
fields: FlatFieldMetadata[],
|
||||
): FlatEntityMaps<FlatFieldMetadata> =>
|
||||
fields.reduce(
|
||||
(maps, field) =>
|
||||
const buildFlatEntityMaps = <
|
||||
TFlatEntity extends FlatFieldMetadata | FlatObjectMetadata,
|
||||
>(
|
||||
flatEntities: TFlatEntity[],
|
||||
): FlatEntityMaps<TFlatEntity> =>
|
||||
flatEntities.reduce(
|
||||
(maps, flatEntity) =>
|
||||
addFlatEntityToFlatEntityMapsOrThrow({
|
||||
flatEntity: field,
|
||||
flatEntity,
|
||||
flatEntityMaps: maps,
|
||||
}),
|
||||
createEmptyFlatEntityMaps() as FlatEntityMaps<FlatFieldMetadata>,
|
||||
createEmptyFlatEntityMaps() as FlatEntityMaps<TFlatEntity>,
|
||||
);
|
||||
|
||||
describe('CommonResultGettersService', () => {
|
||||
const signFileByIdUrl = jest.fn(
|
||||
async ({ fileId }: { fileId: string }) => `signed-${fileId}`,
|
||||
);
|
||||
const service = new CommonResultGettersService({
|
||||
const fileUrlService = {
|
||||
signFileByIdUrl,
|
||||
} as unknown as FileUrlService);
|
||||
} as unknown as FileUrlService;
|
||||
|
||||
let service: CommonResultGettersService;
|
||||
|
||||
beforeEach(() => {
|
||||
service = new CommonResultGettersService(fileUrlService);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
jest.clearAllMocks();
|
||||
});
|
||||
|
||||
it('runs a field handler once when multiple record fields share its type', async () => {
|
||||
it('runs a shared field handler once and preserves fields without metadata', async () => {
|
||||
const objectMetadataId = 'document-object-id';
|
||||
const filesField = createField({
|
||||
id: 'document-files-field-id',
|
||||
@@ -73,7 +99,7 @@ describe('CommonResultGettersService', () => {
|
||||
namePlural: 'documents',
|
||||
fieldIds: [filesField.id, attachmentsField.id],
|
||||
});
|
||||
const flatFieldMetadataMaps = buildFlatFieldMetadataMaps([
|
||||
const flatFieldMetadataMaps = buildFlatEntityMaps([
|
||||
filesField,
|
||||
attachmentsField,
|
||||
]);
|
||||
@@ -95,9 +121,10 @@ describe('CommonResultGettersService', () => {
|
||||
extension: 'pdf',
|
||||
},
|
||||
],
|
||||
ownerId: 'owner-id',
|
||||
};
|
||||
|
||||
await service.processRecord(
|
||||
const result = await service.processRecord(
|
||||
record,
|
||||
objectMetadata,
|
||||
flatObjectMetadataMaps,
|
||||
@@ -105,6 +132,25 @@ describe('CommonResultGettersService', () => {
|
||||
'workspace-id',
|
||||
);
|
||||
|
||||
expect(result).toEqual({
|
||||
...record,
|
||||
files: [
|
||||
{
|
||||
fileId: 'file-id',
|
||||
label: 'document',
|
||||
extension: 'pdf',
|
||||
url: 'signed-file-id',
|
||||
},
|
||||
],
|
||||
attachments: [
|
||||
{
|
||||
fileId: 'attachment-id',
|
||||
label: 'attachment',
|
||||
extension: 'pdf',
|
||||
url: 'signed-attachment-id',
|
||||
},
|
||||
],
|
||||
});
|
||||
expect(signFileByIdUrl).toHaveBeenCalledTimes(2);
|
||||
expect(signFileByIdUrl).toHaveBeenCalledWith({
|
||||
fileId: 'file-id',
|
||||
@@ -117,4 +163,88 @@ describe('CommonResultGettersService', () => {
|
||||
fileFolder: FileFolder.FilesField,
|
||||
});
|
||||
});
|
||||
|
||||
it('resolves field metadata once per object type within a single invocation', async () => {
|
||||
const companyObjectMetadataId = 'company-object-id';
|
||||
const personObjectMetadataId = 'person-object-id';
|
||||
const companyNameField = createField({
|
||||
id: 'company-name-field-id',
|
||||
name: 'name',
|
||||
objectMetadataId: companyObjectMetadataId,
|
||||
type: FieldMetadataType.TEXT,
|
||||
});
|
||||
const companyPeopleField = createField({
|
||||
id: 'company-people-field-id',
|
||||
name: 'people',
|
||||
objectMetadataId: companyObjectMetadataId,
|
||||
type: FieldMetadataType.RELATION,
|
||||
relationTargetObjectMetadataId: personObjectMetadataId,
|
||||
settings: { relationType: RelationType.ONE_TO_MANY },
|
||||
});
|
||||
const personNameField = createField({
|
||||
id: 'person-name-field-id',
|
||||
name: 'name',
|
||||
objectMetadataId: personObjectMetadataId,
|
||||
type: FieldMetadataType.TEXT,
|
||||
});
|
||||
const companyObjectMetadata = getFlatObjectMetadataMock({
|
||||
id: companyObjectMetadataId,
|
||||
universalIdentifier: 'company-object-universal-identifier',
|
||||
nameSingular: 'company',
|
||||
namePlural: 'companies',
|
||||
fieldIds: [companyNameField.id, companyPeopleField.id],
|
||||
});
|
||||
const personObjectMetadata = getFlatObjectMetadataMock({
|
||||
id: personObjectMetadataId,
|
||||
universalIdentifier: 'person-object-universal-identifier',
|
||||
nameSingular: 'person',
|
||||
namePlural: 'people',
|
||||
fieldIds: [personNameField.id],
|
||||
});
|
||||
const flatFieldMetadataMaps = buildFlatEntityMaps([
|
||||
companyNameField,
|
||||
companyPeopleField,
|
||||
personNameField,
|
||||
]);
|
||||
const flatObjectMetadataMaps = buildFlatEntityMaps([
|
||||
companyObjectMetadata,
|
||||
personObjectMetadata,
|
||||
]);
|
||||
const records: ObjectRecord[] = [
|
||||
{
|
||||
id: 'company-1',
|
||||
name: 'Acme',
|
||||
people: [
|
||||
{ id: 'person-1', name: 'Alice' },
|
||||
{ id: 'person-2', name: 'Bob' },
|
||||
],
|
||||
},
|
||||
{
|
||||
id: 'company-2',
|
||||
name: 'Globex',
|
||||
people: [{ id: 'person-3', name: 'Carol' }],
|
||||
},
|
||||
];
|
||||
|
||||
const result = await service.processRecordArray(
|
||||
records,
|
||||
companyObjectMetadata,
|
||||
flatObjectMetadataMaps,
|
||||
flatFieldMetadataMaps,
|
||||
'workspace-id',
|
||||
);
|
||||
|
||||
expect(result).toEqual(records);
|
||||
expect(getFlatFieldsFromFlatObjectMetadata).toHaveBeenCalledTimes(2);
|
||||
|
||||
await service.processRecordArray(
|
||||
records,
|
||||
companyObjectMetadata,
|
||||
flatObjectMetadataMaps,
|
||||
flatFieldMetadataMaps,
|
||||
'workspace-id',
|
||||
);
|
||||
|
||||
expect(getFlatFieldsFromFlatObjectMetadata).toHaveBeenCalledTimes(4);
|
||||
});
|
||||
});
|
||||
|
||||
+94
-78
@@ -13,18 +13,24 @@ import { type QueryResultGetterHandlerInterface } from 'src/engine/api/graphql/w
|
||||
import { FilesFieldQueryResultGetterHandler } from 'src/engine/api/common/common-result-getters/handlers/field-handlers/files-field-query-result-getter.handler';
|
||||
import { RichTextFieldQueryResultGetterHandler } from 'src/engine/api/common/common-result-getters/handlers/field-handlers/rich-text-field-query-result-getter.handler';
|
||||
import { WorkspaceMemberQueryResultGetterHandler } from 'src/engine/api/graphql/workspace-query-runner/factories/query-result-getters/handlers/workspace-member-query-result-getter.handler';
|
||||
import { getFlatFieldsFromFlatObjectMetadata } from 'src/engine/api/graphql/workspace-schema-builder/utils/get-flat-fields-for-flat-object-metadata.util';
|
||||
import { FileUrlService } from 'src/engine/core-modules/file/file-url/file-url.service';
|
||||
import { type FlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/types/flat-entity-maps.type';
|
||||
import { findFlatEntityByIdInFlatEntityMapsOrThrow } from 'src/engine/metadata-modules/flat-entity/utils/find-flat-entity-by-id-in-flat-entity-maps-or-throw.util';
|
||||
import { findFlatEntityByIdInFlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/utils/find-flat-entity-by-id-in-flat-entity-maps.util';
|
||||
import { type FlatFieldMetadata } from 'src/engine/metadata-modules/flat-field-metadata/types/flat-field-metadata.type';
|
||||
import {
|
||||
buildFieldMapsFromFlatObjectMetadata,
|
||||
type FieldMapsForObject,
|
||||
} from 'src/engine/metadata-modules/flat-field-metadata/utils/build-field-maps-from-flat-object-metadata.util';
|
||||
import { isFlatFieldMetadataOfType } from 'src/engine/metadata-modules/flat-field-metadata/utils/is-flat-field-metadata-of-type.util';
|
||||
import { type FlatObjectMetadata } from 'src/engine/metadata-modules/flat-object-metadata/types/flat-object-metadata.type';
|
||||
|
||||
type ResultProcessingContext = {
|
||||
flatObjectMetadataMaps: FlatEntityMaps<FlatObjectMetadata>;
|
||||
flatFieldMetadataMaps: FlatEntityMaps<FlatFieldMetadata>;
|
||||
workspaceId: string;
|
||||
fieldMetadataByNameByObjectMetadataId: Map<
|
||||
string,
|
||||
ReadonlyMap<string, FlatFieldMetadata>
|
||||
>;
|
||||
};
|
||||
|
||||
// TODO: find a way to prevent conflict between handlers executing logic on object relations
|
||||
// And this factory that is also executing logic on object relations
|
||||
// Right now the factory will override any change made on relations by the handlers
|
||||
@@ -66,60 +72,66 @@ export class CommonResultGettersService {
|
||||
]);
|
||||
}
|
||||
|
||||
public async processRecordArray(
|
||||
public processRecordArray(
|
||||
recordArray: ObjectRecord[],
|
||||
flatObjectMetadata: FlatObjectMetadata,
|
||||
flatObjectMetadataMaps: FlatEntityMaps<FlatObjectMetadata>,
|
||||
flatFieldMetadataMaps: FlatEntityMaps<FlatFieldMetadata>,
|
||||
workspaceId: string,
|
||||
) {
|
||||
const fieldMaps = buildFieldMapsFromFlatObjectMetadata(
|
||||
): Promise<ObjectRecord[]> {
|
||||
return this.processRecordArrayWithContext(recordArray, flatObjectMetadata, {
|
||||
flatObjectMetadataMaps,
|
||||
flatFieldMetadataMaps,
|
||||
flatObjectMetadata,
|
||||
);
|
||||
|
||||
return await Promise.all(
|
||||
recordArray.map(
|
||||
async (record: ObjectRecord) =>
|
||||
await this.processRecord(
|
||||
record,
|
||||
flatObjectMetadata,
|
||||
flatObjectMetadataMaps,
|
||||
flatFieldMetadataMaps,
|
||||
workspaceId,
|
||||
fieldMaps,
|
||||
),
|
||||
),
|
||||
);
|
||||
workspaceId,
|
||||
fieldMetadataByNameByObjectMetadataId: new Map(),
|
||||
});
|
||||
}
|
||||
|
||||
public async processRecord(
|
||||
public processRecord(
|
||||
record: ObjectRecord,
|
||||
flatObjectMetadata: FlatObjectMetadata,
|
||||
flatObjectMetadataMaps: FlatEntityMaps<FlatObjectMetadata>,
|
||||
flatFieldMetadataMaps: FlatEntityMaps<FlatFieldMetadata>,
|
||||
workspaceId: string,
|
||||
fieldMapsForObject?: FieldMapsForObject,
|
||||
): Promise<ObjectRecord> {
|
||||
const fieldMaps =
|
||||
fieldMapsForObject ??
|
||||
buildFieldMapsFromFlatObjectMetadata(
|
||||
flatFieldMetadataMaps,
|
||||
flatObjectMetadata,
|
||||
);
|
||||
return this.processRecordWithContext(record, flatObjectMetadata, {
|
||||
flatObjectMetadataMaps,
|
||||
flatFieldMetadataMaps,
|
||||
workspaceId,
|
||||
fieldMetadataByNameByObjectMetadataId: new Map(),
|
||||
});
|
||||
}
|
||||
|
||||
const { fieldIdByName } = fieldMaps;
|
||||
private processRecordArrayWithContext(
|
||||
recordArray: ObjectRecord[],
|
||||
flatObjectMetadata: FlatObjectMetadata,
|
||||
context: ResultProcessingContext,
|
||||
): Promise<ObjectRecord[]> {
|
||||
return Promise.all(
|
||||
recordArray.map((record) =>
|
||||
this.processRecordWithContext(record, flatObjectMetadata, context),
|
||||
),
|
||||
);
|
||||
}
|
||||
|
||||
private async processRecordWithContext(
|
||||
record: ObjectRecord,
|
||||
flatObjectMetadata: FlatObjectMetadata,
|
||||
context: ResultProcessingContext,
|
||||
): Promise<ObjectRecord> {
|
||||
const fieldMetadataByName = this.getOrBuildFieldMetadataByName(
|
||||
flatObjectMetadata,
|
||||
context,
|
||||
);
|
||||
const recordFieldMetadataList = Object.keys(record)
|
||||
.map((recordFieldName) => fieldMetadataByName.get(recordFieldName))
|
||||
.filter(isDefined);
|
||||
|
||||
const fieldHandlers = new Set(
|
||||
Object.keys(record)
|
||||
.map((recordFieldName) =>
|
||||
findFlatEntityByIdInFlatEntityMaps({
|
||||
flatEntityId: fieldIdByName[recordFieldName],
|
||||
flatEntityMaps: flatFieldMetadataMaps,
|
||||
}),
|
||||
recordFieldMetadataList
|
||||
.map((recordFieldMetadata) =>
|
||||
this.fieldHandlers.get(recordFieldMetadata.type),
|
||||
)
|
||||
.filter(isDefined)
|
||||
.map((fieldMetadata) => this.fieldHandlers.get(fieldMetadata.type))
|
||||
.filter(isDefined),
|
||||
);
|
||||
|
||||
@@ -128,17 +140,13 @@ export class CommonResultGettersService {
|
||||
...fieldHandlers,
|
||||
];
|
||||
|
||||
const relationFields = Object.keys(record)
|
||||
.map((recordFieldName) =>
|
||||
findFlatEntityByIdInFlatEntityMaps({
|
||||
flatEntityId: fieldIdByName[recordFieldName],
|
||||
flatEntityMaps: flatFieldMetadataMaps,
|
||||
}),
|
||||
)
|
||||
.filter(isDefined)
|
||||
.filter((fieldMetadata) =>
|
||||
isFlatFieldMetadataOfType(fieldMetadata, FieldMetadataType.RELATION),
|
||||
);
|
||||
const relationFields = recordFieldMetadataList.filter(
|
||||
(recordFieldMetadata) =>
|
||||
isFlatFieldMetadataOfType(
|
||||
recordFieldMetadata,
|
||||
FieldMetadataType.RELATION,
|
||||
),
|
||||
);
|
||||
|
||||
const relationFieldsProcessedMap = {} as Record<
|
||||
string,
|
||||
@@ -159,50 +167,61 @@ export class CommonResultGettersService {
|
||||
const targetFlatObjectMetadata =
|
||||
findFlatEntityByIdInFlatEntityMapsOrThrow({
|
||||
flatEntityId: relationField.relationTargetObjectMetadataId,
|
||||
flatEntityMaps: flatObjectMetadataMaps,
|
||||
flatEntityMaps: context.flatObjectMetadataMaps,
|
||||
});
|
||||
|
||||
relationFieldsProcessedMap[relationField.name] =
|
||||
relationField.settings?.relationType === RelationType.ONE_TO_MANY
|
||||
? await this.processRecordArray(
|
||||
? await this.processRecordArrayWithContext(
|
||||
record[relationField.name],
|
||||
targetFlatObjectMetadata,
|
||||
flatObjectMetadataMaps,
|
||||
flatFieldMetadataMaps,
|
||||
workspaceId,
|
||||
context,
|
||||
)
|
||||
: await this.processRecord(
|
||||
: await this.processRecordWithContext(
|
||||
record[relationField.name],
|
||||
targetFlatObjectMetadata,
|
||||
flatObjectMetadataMaps,
|
||||
flatFieldMetadataMaps,
|
||||
workspaceId,
|
||||
context,
|
||||
);
|
||||
}
|
||||
|
||||
const fieldMetadata = Object.keys(record)
|
||||
.map((recordFieldName) =>
|
||||
findFlatEntityByIdInFlatEntityMaps({
|
||||
flatEntityId: fieldIdByName[recordFieldName],
|
||||
flatEntityMaps: flatFieldMetadataMaps,
|
||||
}),
|
||||
)
|
||||
.filter(isDefined);
|
||||
|
||||
const objectRecordProcessedWithoutRelationFields =
|
||||
await this.processObjectRecordWithoutRelationFields(
|
||||
record,
|
||||
workspaceId,
|
||||
context.workspaceId,
|
||||
handlers,
|
||||
fieldMetadata,
|
||||
recordFieldMetadataList,
|
||||
);
|
||||
|
||||
const processedRecord = {
|
||||
return {
|
||||
...objectRecordProcessedWithoutRelationFields,
|
||||
...relationFieldsProcessedMap,
|
||||
};
|
||||
}
|
||||
|
||||
return processedRecord;
|
||||
private getOrBuildFieldMetadataByName(
|
||||
flatObjectMetadata: FlatObjectMetadata,
|
||||
context: ResultProcessingContext,
|
||||
): ReadonlyMap<string, FlatFieldMetadata> {
|
||||
const cachedFieldMetadataByName =
|
||||
context.fieldMetadataByNameByObjectMetadataId.get(flatObjectMetadata.id);
|
||||
|
||||
if (isDefined(cachedFieldMetadataByName)) {
|
||||
return cachedFieldMetadataByName;
|
||||
}
|
||||
|
||||
const fieldMetadataByName = new Map(
|
||||
getFlatFieldsFromFlatObjectMetadata(
|
||||
flatObjectMetadata,
|
||||
context.flatFieldMetadataMaps,
|
||||
).map((fieldMetadata) => [fieldMetadata.name, fieldMetadata]),
|
||||
);
|
||||
|
||||
context.fieldMetadataByNameByObjectMetadataId.set(
|
||||
flatObjectMetadata.id,
|
||||
fieldMetadataByName,
|
||||
);
|
||||
|
||||
return fieldMetadataByName;
|
||||
}
|
||||
|
||||
private async processObjectRecordWithoutRelationFields(
|
||||
@@ -228,10 +247,7 @@ export class CommonResultGettersService {
|
||||
objectType: string,
|
||||
): QueryResultGetterHandlerInterface {
|
||||
return (
|
||||
(this.objectHandlers.get(objectType) || {
|
||||
handle: (result: ObjectRecord): Promise<ObjectRecord> =>
|
||||
Promise.resolve(result),
|
||||
}) ?? {
|
||||
this.objectHandlers.get(objectType) ?? {
|
||||
handle: (result: ObjectRecord): Promise<ObjectRecord> =>
|
||||
Promise.resolve(result),
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user