Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 @@ -24,6 +24,7 @@ import type {
ILinkFieldOptions,
IConditionalRollupFieldOptions,
IConditionalLookupOptions,
IRollupFieldOptions,
IGetFieldsQuery,
IFilter,
IFilterItem,
Expand Down Expand Up @@ -145,11 +146,22 @@ export class FieldOpenApiService {
return true;
}

private async validateConditionalRollupAggregation(hostTableId: string, field: IFieldInstance) {
const options = field.options as IConditionalRollupFieldOptions | undefined;
const expression = options?.expression;
const lookupFieldId = options?.lookupFieldId;
const foreignTableId = options?.foreignTableId;
private normalizeCellValueType(rawCellType: unknown): CellValueType {
if (
typeof rawCellType === 'string' &&
Object.values(CellValueType).includes(rawCellType as CellValueType)
) {
return rawCellType as CellValueType;
}
return CellValueType.String;
}

private async isRollupAggregationSupported(params: {
expression?: IRollupFieldOptions['expression'];
lookupFieldId?: string;
foreignTableId?: string;
}): Promise<boolean> {
const { expression, lookupFieldId, foreignTableId } = params;

if (!expression || !lookupFieldId || !foreignTableId) {
return false;
Expand All @@ -164,20 +176,42 @@ export class FieldOpenApiService {
return false;
}

const rawCellType = foreignField.cellValueType as string;
const availableTypes = new Set<string>(Object.values(CellValueType));
const cellValueType = availableTypes.has(rawCellType)
? (rawCellType as CellValueType)
: CellValueType.String;
const cellValueType = this.normalizeCellValueType(foreignField.cellValueType);
return isRollupFunctionSupportedForCellValueType(expression, cellValueType);
}

private async validateRollupAggregation(field: IFieldInstance): Promise<boolean> {
if (!field.lookupOptions || !isLinkLookupOptions(field.lookupOptions)) {
return false;
}

const aggregationSupported = isRollupFunctionSupportedForCellValueType(
const options = field.options as IRollupFieldOptions | undefined;
return this.isRollupAggregationSupported({
expression: options?.expression,
lookupFieldId: field.lookupOptions.lookupFieldId,
foreignTableId: field.lookupOptions.foreignTableId,
});
}

private async validateConditionalRollupAggregation(hostTableId: string, field: IFieldInstance) {
const options = field.options as IConditionalRollupFieldOptions | undefined;
const expression = options?.expression;
const lookupFieldId = options?.lookupFieldId;
const foreignTableId = options?.foreignTableId;

const aggregationSupported = await this.isRollupAggregationSupported({
expression,
cellValueType
);
lookupFieldId,
foreignTableId,
});
if (!aggregationSupported) {
return false;
}

if (!foreignTableId) {
return false;
}

return await this.validateFilterFieldReferences(hostTableId, foreignTableId, options?.filter);
}

Expand Down Expand Up @@ -206,6 +240,38 @@ export class FieldOpenApiService {
return this.validateFilterFieldReferences(tableId, foreignTableId, meta?.filter);
}

private async isFieldConfigurationValid(
tableId: string,
field: IFieldInstance
): Promise<boolean> {
if (
field.lookupOptions &&
field.type !== FieldType.ConditionalRollup &&
!field.isConditionalLookup
) {
const lookupValid = await this.validateLookupField(field);
if (!lookupValid) {
return false;
}

if (field.type === FieldType.Rollup) {
return await this.validateRollupAggregation(field);
}

return true;
}

if (field.type === FieldType.ConditionalRollup) {
return await this.validateConditionalRollupAggregation(tableId, field);
}

if (field.isConditionalLookup) {
return await this.validateConditionalLookup(tableId, field);
}

return true;
}

private async findConditionalFilterDependentFields(startFieldIds: readonly string[]): Promise<
Array<{
id: string;
Expand Down Expand Up @@ -536,24 +602,8 @@ export class FieldOpenApiService {
});
}

let hasError = false;

if (
field.lookupOptions &&
field.type !== FieldType.ConditionalRollup &&
!field.isConditionalLookup
) {
const isValid = await this.validateLookupField(field);
hasError = !isValid;
} else if (field.type === FieldType.ConditionalRollup) {
const isValid = await this.validateConditionalRollupAggregation(tableId, field);
hasError = !isValid;
} else if (field.isConditionalLookup) {
const isValid = await this.validateConditionalLookup(tableId, field);
hasError = !isValid;
}

await this.markError(tableId, field, hasError);
const isValid = await this.isFieldConfigurationValid(tableId, field);
await this.markError(tableId, field, !isValid);
}

async restoreReference(references: string[]) {
Expand Down Expand Up @@ -1060,14 +1110,7 @@ export class FieldOpenApiService {
});
for (const raw of dependentFieldRaws) {
const instance = createFieldInstanceByRaw(raw);
let isValid = true;

if (instance.type === FieldType.ConditionalRollup) {
isValid = await this.validateConditionalRollupAggregation(raw.tableId, instance);
} else if (instance.isConditionalLookup) {
isValid = await this.validateConditionalLookup(raw.tableId, instance);
}

const isValid = await this.isFieldConfigurationValid(raw.tableId, instance);
await this.markError(raw.tableId, instance, !isValid);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
DbFieldType,
SortFunc,
isLinkLookupOptions,
normalizeConditionalLimit,
} from '@teable/core';
import type { Knex } from 'knex';
import { match } from 'ts-pattern';
Expand Down Expand Up @@ -1283,8 +1284,9 @@ export class FieldCteVisitor implements IFieldVisitor<ICteResult> {
aggregateSourceQuery.orderByRaw(orderByClause);
}

if (supportsOrdering && typeof limit === 'number' && Number.isFinite(limit) && limit > 0) {
aggregateSourceQuery.limit(limit);
if (supportsOrdering) {
const resolvedLimit = normalizeConditionalLimit(limit);
aggregateSourceQuery.limit(resolvedLimit);
}

const aggregateQuery = this.qb.client
Expand Down Expand Up @@ -1446,9 +1448,8 @@ export class FieldCteVisitor implements IFieldVisitor<ICteResult> {
aggregateSourceQuery.orderByRaw(orderByClause);
}

if (typeof limit === 'number' && limit > 0) {
aggregateSourceQuery.limit(limit);
}
const resolvedLimit = normalizeConditionalLimit(limit);
aggregateSourceQuery.limit(resolvedLimit);

const aggregateQuery = this.qb.client
.queryBuilder()
Expand Down
105 changes: 105 additions & 0 deletions apps/nestjs-backend/test/conditional-lookup.e2e-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3393,4 +3393,109 @@ describe('OpenAPI Conditional Lookup field (e2e)', () => {
expect(bobSummary.fields[scoresWithoutExcludedField.id]).toEqual([7]);
});
});

describe('limit enforcement', () => {
const limitCap = Number(process.env.CONDITIONAL_QUERY_MAX_LIMIT ?? '5000');
const totalActive = limitCap + 2;
let foreign: ITableFullVo;
let host: ITableFullVo;
let titleId: string;
let statusId: string;
let statusFilterId: string;
let lookupFieldId: string;
const activeTitles = Array.from({ length: totalActive }, (_, idx) => `Active ${idx + 1}`);

beforeAll(async () => {
foreign = await createTable(baseId, {
name: 'ConditionalLookup_Limit_Foreign',
fields: [
{ name: 'Title', type: FieldType.SingleLineText, options: {} } as IFieldRo,
{ name: 'Status', type: FieldType.SingleLineText, options: {} } as IFieldRo,
],
records: [
...activeTitles.map((title) => ({
fields: { Title: title, Status: 'Active' },
})),
{ fields: { Title: 'Closed Item', Status: 'Closed' } },
],
});
titleId = foreign.fields.find((field) => field.name === 'Title')!.id;
statusId = foreign.fields.find((field) => field.name === 'Status')!.id;

host = await createTable(baseId, {
name: 'ConditionalLookup_Limit_Host',
fields: [{ name: 'StatusFilter', type: FieldType.SingleLineText, options: {} } as IFieldRo],
records: [{ fields: { StatusFilter: 'Active' } }],
});
statusFilterId = host.fields.find((field) => field.name === 'StatusFilter')!.id;
});

afterAll(async () => {
await permanentDeleteTable(baseId, host.id);
await permanentDeleteTable(baseId, foreign.id);
});

it('rejects creating a conditional lookup with limit beyond configured maximum', async () => {
const statusMatchFilter: IFilter = {
conjunction: 'and',
filterSet: [
{
fieldId: statusId,
operator: 'is',
value: { type: 'field', fieldId: statusFilterId },
},
],
};

await createField(
host.id,
{
name: 'TooManyRecords',
type: FieldType.SingleLineText,
isLookup: true,
isConditionalLookup: true,
lookupOptions: {
foreignTableId: foreign.id,
lookupFieldId: titleId,
filter: statusMatchFilter,
limit: limitCap + 1,
} as ILookupOptionsRo,
} as IFieldRo,
400
);
});

it('caps resolved lookup results to the maximum limit when limit is omitted', async () => {
const statusMatchFilter: IFilter = {
conjunction: 'and',
filterSet: [
{
fieldId: statusId,
operator: 'is',
value: { type: 'field', fieldId: statusFilterId },
},
],
};

const lookupField = await createField(host.id, {
name: 'Limited Titles',
type: FieldType.SingleLineText,
isLookup: true,
isConditionalLookup: true,
lookupOptions: {
foreignTableId: foreign.id,
lookupFieldId: titleId,
filter: statusMatchFilter,
} as ILookupOptionsRo,
} as IFieldRo);
lookupFieldId = lookupField.id;

const record = await getRecord(host.id, host.records[0].id);
const values = record.fields[lookupFieldId] as string[];
expect(Array.isArray(values)).toBe(true);
expect(values.length).toBe(limitCap);
expect(values).toEqual(activeTitles.slice(0, limitCap));
expect(values).not.toContain(activeTitles[limitCap]);
});
});
});
Loading