From 015ef56110a7c8b06bdc86001d20cd17cb929e07 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 4 Sep 2024 09:29:05 +0100 Subject: [PATCH 01/28] wip --- packages/backend-core/src/sql/sql.ts | 50 +++++++- .../server/src/api/controllers/row/views.ts | 14 ++- .../src/api/controllers/view/viewsV2.ts | 51 ++++++-- .../src/api/routes/tests/viewV2.spec.ts | 71 +++++++++++- packages/server/src/sdk/app/rows/search.ts | 3 + .../src/sdk/app/rows/search/internal/sqs.ts | 23 +++- packages/server/src/sdk/app/views/index.ts | 109 ++++++++++++++---- packages/shared-core/src/helpers/index.ts | 1 + packages/shared-core/src/helpers/schema.ts | 4 + packages/shared-core/src/helpers/views.ts | 39 +++++++ packages/types/src/api/web/app/rows.ts | 1 + packages/types/src/documents/app/view.ts | 18 +-- packages/types/src/sdk/row.ts | 9 +- packages/types/src/sdk/search.ts | 2 + packages/types/src/shared/typeUtils.ts | 23 ++++ 15 files changed, 358 insertions(+), 60 deletions(-) create mode 100644 packages/shared-core/src/helpers/views.ts diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index e6738d4b36..b2fec5fe57 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -10,10 +10,12 @@ import { } from "./utils" import SqlTableQueryBuilder from "./sqlTable" import { + Aggregation, AnySearchFilter, ArrayOperator, BasicOperator, BBReferenceFieldMetadata, + CalculationType, FieldSchema, FieldType, INTERNAL_TABLE_SOURCE_ID, @@ -789,6 +791,38 @@ class InternalBuilder { return query.countDistinct(`${aliased}.${primary[0]} as total`) } + addAggregations( + query: Knex.QueryBuilder, + aggregations: Aggregation[] + ): Knex.QueryBuilder { + const fields = this.query.resource?.fields || [] + if (fields.length > 0) { + query = query.groupBy(fields.map(field => `${this.table.name}.${field}`)) + } + for (const aggregation of aggregations) { + const op = aggregation.calculationType + const field = `${this.table.name}.${aggregation.field} as ${aggregation.name}` + switch (op) { + case CalculationType.COUNT: + query = query.count(field) + break + case CalculationType.SUM: + query = query.sum(field) + break + case CalculationType.AVG: + query = query.avg(field) + break + case CalculationType.MIN: + query = query.min(field) + break + case CalculationType.MAX: + query = query.max(field) + break + } + } + return query + } + addSorting(query: Knex.QueryBuilder): Knex.QueryBuilder { let { sort } = this.query const primaryKey = this.table.primary @@ -1172,10 +1206,18 @@ class InternalBuilder { } } - // if counting, use distinct count, else select - query = !counting - ? query.select(this.generateSelectStatement()) - : this.addDistinctCount(query) + const aggregations = this.query.resource?.aggregations || [] + if (counting) { + query = this.addDistinctCount(query) + } else if (aggregations.length > 0) { + query = query.select( + this.knex.raw("ROW_NUMBER() OVER (ORDER BY (SELECT 0)) as _id") + ) + query = this.addAggregations(query, aggregations) + } else { + query = query.select(this.generateSelectStatement()) + } + // have to add after as well (this breaks MS-SQL) if (this.client !== SqlClient.MS_SQL && !counting) { query = this.addSorting(query) diff --git a/packages/server/src/api/controllers/row/views.ts b/packages/server/src/api/controllers/row/views.ts index d2541dfa25..ba48bc5664 100644 --- a/packages/server/src/api/controllers/row/views.ts +++ b/packages/server/src/api/controllers/row/views.ts @@ -7,8 +7,9 @@ import { RowSearchParams, SearchFilterKey, LogicalOperator, + Aggregation, } from "@budibase/types" -import { dataFilters } from "@budibase/shared-core" +import { dataFilters, helpers } from "@budibase/shared-core" import sdk from "../../../sdk" import { db, context, features } from "@budibase/backend-core" import { enrichSearchContext } from "./utils" @@ -27,7 +28,7 @@ export async function searchView( ctx.throw(400, `This method only supports viewsV2`) } - const viewFields = Object.entries(view.schema || {}) + const viewFields = Object.entries(helpers.views.basicFields(view)) .filter(([_, value]) => value.visible) .map(([key]) => key) const { body } = ctx.request @@ -74,6 +75,14 @@ export async function searchView( user: sdk.users.getUserContextBindings(ctx.user), }) + const aggregations: Aggregation[] = Object.entries( + helpers.views.calculationFields(view) + ).map(([name, { field, calculationType }]) => ({ + name, + calculationType, + field, + })) + const searchOptions: RequiredKeys & RequiredKeys> = { tableId: view.tableId, @@ -84,6 +93,7 @@ export async function searchView( bookmark: body.bookmark, paginate: body.paginate, countRows: body.countRows, + aggregations, } const result = await sdk.rows.search(searchOptions) diff --git a/packages/server/src/api/controllers/view/viewsV2.ts b/packages/server/src/api/controllers/view/viewsV2.ts index 4208772fa6..18306ba245 100644 --- a/packages/server/src/api/controllers/view/viewsV2.ts +++ b/packages/server/src/api/controllers/view/viewsV2.ts @@ -8,8 +8,45 @@ import { ViewResponse, ViewResponseEnriched, ViewV2, + BasicViewUIFieldMetadata, + ViewCalculationFieldMetadata, } from "@budibase/types" import { builderSocket, gridSocket } from "../../../websockets" +import { helpers } from "@budibase/shared-core" + +function stripUnknownFields( + field: ViewUIFieldMetadata +): RequiredKeys { + if (helpers.views.isCalculationField(field)) { + const strippedField: RequiredKeys = { + order: field.order, + width: field.width, + visible: field.visible, + readonly: field.readonly, + icon: field.icon, + calculationType: field.calculationType, + field: field.field, + } + return strippedField + } else { + const strippedField: RequiredKeys = { + order: field.order, + width: field.width, + visible: field.visible, + readonly: field.readonly, + icon: field.icon, + } + return strippedField + } +} + +function stripUndefinedFields(obj: Record): void { + Object.keys(obj) + .filter(key => obj[key] === undefined) + .forEach(key => { + delete obj[key] + }) +} async function parseSchema(view: CreateViewRequest) { if (!view.schema) { @@ -18,18 +55,8 @@ async function parseSchema(view: CreateViewRequest) { const finalViewSchema = view.schema && Object.entries(view.schema).reduce((p, [fieldName, schemaValue]) => { - const fieldSchema: RequiredKeys = { - order: schemaValue.order, - width: schemaValue.width, - visible: schemaValue.visible, - readonly: schemaValue.readonly, - icon: schemaValue.icon, - } - Object.entries(fieldSchema) - .filter(([, val]) => val === undefined) - .forEach(([key]) => { - delete fieldSchema[key as keyof ViewUIFieldMetadata] - }) + const fieldSchema = stripUnknownFields(schemaValue) + stripUndefinedFields(fieldSchema) p[fieldName] = fieldSchema return p }, {} as Record>) diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 356f01dee0..72f55c16d2 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -18,6 +18,7 @@ import { ViewV2, SearchResponse, BasicOperator, + CalculationType, } from "@budibase/types" import { generator, mocks } from "@budibase/backend-core/tests" import { DatabaseName, getDatasource } from "../../../integrations/tests/utils" @@ -32,13 +33,13 @@ import { import sdk from "../../../sdk" describe.each([ - ["lucene", undefined], + // ["lucene", undefined], ["sqs", undefined], - [DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)], - [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)], - [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)], - [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)], - [DatabaseName.ORACLE, getDatasource(DatabaseName.ORACLE)], + // [DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)], + // [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)], + // [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)], + // [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)], + // [DatabaseName.ORACLE, getDatasource(DatabaseName.ORACLE)], ])("/v2/views (%s)", (name, dsProvider) => { const config = setup.getConfig() const isSqs = name === "sqs" @@ -1978,6 +1979,64 @@ describe.each([ }) ) }) + + describe("calculations", () => { + let table: Table + let rows: Row[] + + beforeAll(async () => { + table = await config.api.table.save( + saveTableRequest({ + schema: { + quantity: { + type: FieldType.NUMBER, + name: "quantity", + }, + price: { + type: FieldType.NUMBER, + name: "price", + }, + }, + }) + ) + + rows = await Promise.all( + Array.from({ length: 10 }, () => + config.api.row.save(table._id!, { + quantity: generator.natural({ min: 1, max: 10 }), + price: generator.natural({ min: 1, max: 10 }), + }) + ) + ) + }) + + it.only("should be able to search by calculations", async () => { + const view = await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), + schema: { + "Quantity Sum": { + visible: true, + calculationType: CalculationType.SUM, + field: "quantity", + }, + }, + }) + + const response = await config.api.viewV2.search(view.id, { + query: {}, + }) + + expect(response.rows).toHaveLength(1) + expect(response.rows).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + "Quantity Sum": rows.reduce((acc, r) => acc + r.quantity, 0), + }), + ]) + ) + }) + }) }) describe("permissions", () => { diff --git a/packages/server/src/sdk/app/rows/search.ts b/packages/server/src/sdk/app/rows/search.ts index 3d5de2e6cb..95cbc919a1 100644 --- a/packages/server/src/sdk/app/rows/search.ts +++ b/packages/server/src/sdk/app/rows/search.ts @@ -46,6 +46,9 @@ export async function search( paginate: options.paginate, fields: options.fields, countRows: options.countRows, + aggregations: options.aggregations + ?.map(a => `${a.field}:${a.calculationType}`) + .join(", "), }) const isExternalTable = isExternalTableID(options.tableId) diff --git a/packages/server/src/sdk/app/rows/search/internal/sqs.ts b/packages/server/src/sdk/app/rows/search/internal/sqs.ts index fb140e3c14..93c6cab2ea 100644 --- a/packages/server/src/sdk/app/rows/search/internal/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/internal/sqs.ts @@ -48,6 +48,7 @@ import { } from "@budibase/shared-core" import { isSearchingByRowID } from "../utils" import tracer from "dd-trace" +import { cloneDeep } from "lodash" const builder = new sql.Sql(SqlClient.SQL_LITE) const SQLITE_COLUMN_LIMIT = 2000 @@ -285,7 +286,7 @@ export async function search( table: Table, opts?: { retrying?: boolean } ): Promise> { - let { paginate, query, ...params } = options + let { paginate, query, ...params } = cloneDeep(options) const allTables = await sdk.tables.getAllInternalTables() const allTablesMap = buildTableMap(allTables) @@ -303,6 +304,21 @@ export async function search( ...cleanupFilters(query, table, allTables), documentType: DocumentType.ROW, } + + let fields = options.fields + if (fields === undefined) { + fields = buildInternalFieldList(table, allTables, { relationships }) + } else { + fields = fields.map(f => mapToUserColumn(f)) + } + + if (options.aggregations) { + options.aggregations = options.aggregations.map(a => { + a.field = mapToUserColumn(a.field) + return a + }) + } + const request: QueryJson = { endpoint: { // not important, we query ourselves @@ -317,9 +333,7 @@ export async function search( tables: allTablesMap, columnPrefix: USER_COLUMN_PREFIX, }, - resource: { - fields: buildInternalFieldList(table, allTables, { relationships }), - }, + resource: { fields, aggregations: options.aggregations }, relationships, } @@ -426,6 +440,7 @@ export async function search( if (err.status === 400 && msg?.match(MISSING_COLUMN_REGEX)) { return { rows: [] } } + throw err throw new Error(`Unable to search by SQL - ${msg}`, { cause: err }) } } diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index ed624c2b5c..01c2d3d2fa 100644 --- a/packages/server/src/sdk/app/views/index.ts +++ b/packages/server/src/sdk/app/views/index.ts @@ -1,8 +1,9 @@ import { + BasicViewUIFieldMetadata, RenameColumn, + Table, TableSchema, View, - ViewUIFieldMetadata, ViewV2, ViewV2Enriched, } from "@budibase/types" @@ -38,31 +39,84 @@ export async function getEnriched(viewId: string): Promise { return pickApi(tableId).getEnriched(viewId) } +async function guardCalculationViewSchema( + table: Table, + view: Omit +) { + const calculationFields = helpers.views.calculationFields(view) + for (const calculationFieldName of Object.keys(calculationFields)) { + const schema = calculationFields[calculationFieldName] + const targetSchema = table.schema[schema.field] + if (!targetSchema) { + throw new HTTPError( + `Calculation field "${calculationFieldName}" references field "${schema.field}" which does not exist in the table schema`, + 400 + ) + } + + if (!helpers.schema.isNumeric(targetSchema)) { + throw new HTTPError( + `Calculation field "${calculationFieldName}" references field "${schema.field}" which is not a numeric field`, + 400 + ) + } + } + + const groupByFields = helpers.views.basicFields(view) + for (const groupByFieldName of Object.keys(groupByFields)) { + const targetSchema = table.schema[groupByFieldName] + if (!targetSchema) { + throw new HTTPError( + `Group by field "${groupByFieldName}" does not exist in the table schema`, + 400 + ) + } + } +} + async function guardViewSchema( tableId: string, view: Omit ) { - const viewSchema = view.schema || {} const table = await sdk.tables.getTable(tableId) + if (helpers.views.isCalculationView(view)) { + await guardCalculationViewSchema(table, view) + } + + await checkReadonlyFields(table, view) + checkRequiredFields(table, view) + checkDisplayField(view) +} + +async function checkReadonlyFields( + table: Table, + view: Omit +) { + const viewSchema = view.schema || {} for (const field of Object.keys(viewSchema)) { - const tableSchemaField = table.schema[field] - if (!tableSchemaField) { + const viewFieldSchema = viewSchema[field] + if (helpers.views.isCalculationField(viewFieldSchema)) { + continue + } + + const tableFieldSchema = table.schema[field] + if (!tableFieldSchema) { throw new HTTPError( `Field "${field}" is not valid for the requested table`, 400 ) } - if (viewSchema[field].readonly) { + if (viewFieldSchema.readonly) { if ( !(await features.isViewReadonlyColumnsEnabled()) && - !(tableSchemaField as ViewUIFieldMetadata).readonly + !(tableFieldSchema as BasicViewUIFieldMetadata).readonly ) { throw new HTTPError(`Readonly fields are not enabled`, 400) } - if (!viewSchema[field].visible) { + if (!viewFieldSchema.visible) { throw new HTTPError( `Field "${field}" must be visible if you want to make it readonly`, 400 @@ -70,18 +124,33 @@ async function guardViewSchema( } } } +} - const existingView = - table?.views && (table.views[view.name] as ViewV2 | undefined) +function checkDisplayField(view: Omit) { + if (view.primaryDisplay) { + const viewSchemaField = view.schema?.[view.primaryDisplay] + if (!viewSchemaField?.visible) { + throw new HTTPError( + `You can't hide "${view.primaryDisplay}" because it is the display column.`, + 400 + ) + } + } +} + +function checkRequiredFields( + table: Table, + view: Omit +) { + const existingView = table.views?.[view.name] as ViewV2 | undefined for (const field of Object.values(table.schema)) { if (!helpers.schema.isRequired(field.constraints)) { continue } - const viewSchemaField = viewSchema[field.name] - const existingViewSchema = - existingView?.schema && existingView.schema[field.name] + const viewSchemaField = view.schema?.[field.name] + const existingViewSchema = existingView?.schema?.[field.name] if (!viewSchemaField && !existingViewSchema?.visible) { // Supporting existing configs with required columns but hidden in views continue @@ -94,24 +163,16 @@ async function guardViewSchema( ) } - if (viewSchemaField.readonly) { + if ( + helpers.views.isBasicViewField(viewSchemaField) && + viewSchemaField.readonly + ) { throw new HTTPError( `You can't make "${field.name}" readonly because it is a required field.`, 400 ) } } - - if (view.primaryDisplay) { - const viewSchemaField = viewSchema[view.primaryDisplay] - - if (!viewSchemaField?.visible) { - throw new HTTPError( - `You can't hide "${view.primaryDisplay}" because it is the display column.`, - 400 - ) - } - } } export async function create( diff --git a/packages/shared-core/src/helpers/index.ts b/packages/shared-core/src/helpers/index.ts index c33ff2ab1e..503f71e4eb 100644 --- a/packages/shared-core/src/helpers/index.ts +++ b/packages/shared-core/src/helpers/index.ts @@ -2,3 +2,4 @@ export * from "./helpers" export * from "./integrations" export * as cron from "./cron" export * as schema from "./schema" +export * as views from "./views" diff --git a/packages/shared-core/src/helpers/schema.ts b/packages/shared-core/src/helpers/schema.ts index d0035cc305..c9200fee18 100644 --- a/packages/shared-core/src/helpers/schema.ts +++ b/packages/shared-core/src/helpers/schema.ts @@ -45,3 +45,7 @@ export function decodeNonAscii(str: string): string { String.fromCharCode(parseInt(p1, 16)) ) } + +export function isNumeric(field: FieldSchema) { + return field.type === FieldType.NUMBER || field.type === FieldType.BIGINT +} diff --git a/packages/shared-core/src/helpers/views.ts b/packages/shared-core/src/helpers/views.ts new file mode 100644 index 0000000000..0364ccff41 --- /dev/null +++ b/packages/shared-core/src/helpers/views.ts @@ -0,0 +1,39 @@ +import { + BasicViewUIFieldMetadata, + ViewCalculationFieldMetadata, + ViewUIFieldMetadata, + ViewV2, +} from "@budibase/types" +import { pickBy } from "lodash" + +export function isCalculationField( + field: ViewUIFieldMetadata +): field is ViewCalculationFieldMetadata { + return "calculationType" in field +} + +export function isBasicViewField( + field: ViewUIFieldMetadata +): field is BasicViewUIFieldMetadata { + return !isCalculationField(field) +} + +type UnsavedViewV2 = Omit + +export function isCalculationView(view: UnsavedViewV2) { + return Object.values(view.schema || {}).some(isCalculationField) +} + +export function calculationFields(view: UnsavedViewV2) { + if (!isCalculationView(view)) { + throw new Error("View is not a calculation view") + } + return pickBy(view.schema || {}, isCalculationField) +} + +export function basicFields(view: UnsavedViewV2) { + if (!isCalculationView(view)) { + throw new Error("View is not a calculation view") + } + return pickBy(view.schema || {}, field => !isCalculationField(field)) +} diff --git a/packages/types/src/api/web/app/rows.ts b/packages/types/src/api/web/app/rows.ts index ce6f6f672d..5d49ac1812 100644 --- a/packages/types/src/api/web/app/rows.ts +++ b/packages/types/src/api/web/app/rows.ts @@ -26,6 +26,7 @@ export interface SearchViewRowRequest | "paginate" | "query" | "countRows" + | "aggregations" > {} export interface SearchRowResponse { diff --git a/packages/types/src/documents/app/view.ts b/packages/types/src/documents/app/view.ts index 24dad0bcca..539c1e0d23 100644 --- a/packages/types/src/documents/app/view.ts +++ b/packages/types/src/documents/app/view.ts @@ -33,10 +33,19 @@ export interface View { groupBy?: string } -export type ViewUIFieldMetadata = UIFieldMetadata & { +export interface BasicViewUIFieldMetadata extends UIFieldMetadata { readonly?: boolean } +export interface ViewCalculationFieldMetadata extends BasicViewUIFieldMetadata { + calculationType: CalculationType + field: string +} + +export type ViewUIFieldMetadata = + | BasicViewUIFieldMetadata + | ViewCalculationFieldMetadata + export enum CalculationType { SUM = "sum", AVG = "avg", @@ -45,11 +54,6 @@ export enum CalculationType { MAX = "max", } -export type ViewCalculationFieldMetadata = ViewUIFieldMetadata & { - calculationType: CalculationType - field: string -} - export interface ViewV2 { version: 2 id: string @@ -62,7 +66,7 @@ export interface ViewV2 { order?: SortOrder type?: SortType } - schema?: Record + schema?: Record } export type ViewSchema = ViewCountOrSumSchema | ViewStatisticsSchema diff --git a/packages/types/src/sdk/row.ts b/packages/types/src/sdk/row.ts index 6850359cc3..8ee0338731 100644 --- a/packages/types/src/sdk/row.ts +++ b/packages/types/src/sdk/row.ts @@ -1,8 +1,14 @@ import { SortOrder, SortType } from "../api" import { SearchFilters } from "./search" -import { Row } from "../documents" +import { CalculationType, Row } from "../documents" import { WithRequired } from "../shared" +export interface Aggregation { + name: string + calculationType: CalculationType + field: string +} + export interface SearchParams { tableId?: string query?: SearchFilters @@ -18,6 +24,7 @@ export interface SearchParams { indexer?: () => Promise rows?: Row[] countRows?: boolean + aggregations?: Aggregation[] } // when searching for rows we want a more extensive search type that requires certain properties diff --git a/packages/types/src/sdk/search.ts b/packages/types/src/sdk/search.ts index 6feea40766..ba3c388480 100644 --- a/packages/types/src/sdk/search.ts +++ b/packages/types/src/sdk/search.ts @@ -2,6 +2,7 @@ import { Operation } from "./datasources" import { Row, Table, DocumentType } from "../documents" import { SortOrder, SortType } from "../api" import { Knex } from "knex" +import { Aggregation } from "./row" export enum BasicOperator { EQUAL = "equal", @@ -143,6 +144,7 @@ export interface QueryJson { } resource?: { fields: string[] + aggregations?: Aggregation[] } filters?: SearchFilters sort?: SortJson diff --git a/packages/types/src/shared/typeUtils.ts b/packages/types/src/shared/typeUtils.ts index c7ecebed0a..dbb3fc2553 100644 --- a/packages/types/src/shared/typeUtils.ts +++ b/packages/types/src/shared/typeUtils.ts @@ -4,6 +4,29 @@ export type DeepPartial = { export type ISO8601 = string +/** + * RequiredKeys make it such that you _must_ assign a value to every key in the + * type. It differs subtly from Required in that it doesn't change the type + * of the fields, you can specify undefined as a value and that's fine. + * + * Example: + * + * ```ts + * interface Foo { + * bar: string + * baz?: string + * } + * + * type FooRequiredKeys = RequiredKeys + * type FooRequired = Required + * + * const a: FooRequiredKeys = { bar: "hello", baz: undefined } + * const b: FooRequired = { bar: "hello", baz: undefined } + * ``` + * + * In this code, a passes type checking whereas b does not. This is because + * Required makes baz non-optional. + */ export type RequiredKeys = { [K in keyof Required]: T[K] } From fc44b38fc5e5a2cab4a920e4cf0aac6d6d855247 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 20 Sep 2024 11:52:06 +0100 Subject: [PATCH 02/28] Fix tests. --- .../server/src/api/routes/tests/viewV2.spec.ts | 16 ++++++++-------- packages/shared-core/src/helpers/views.ts | 6 ------ 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 6769c80867..9313e3ab4d 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -37,13 +37,13 @@ import { import sdk from "../../../sdk" describe.each([ - // ["lucene", undefined], + ["lucene", undefined], ["sqs", undefined], - // [DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)], - // [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)], - // [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)], - // [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)], - // [DatabaseName.ORACLE, getDatasource(DatabaseName.ORACLE)], + [DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)], + [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)], + [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)], + [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)], + [DatabaseName.ORACLE, getDatasource(DatabaseName.ORACLE)], ])("/v2/views (%s)", (name, dsProvider) => { const config = setup.getConfig() const isSqs = name === "sqs" @@ -2295,7 +2295,7 @@ describe.each([ ) }) - describe("calculations", () => { + describe.skip("calculations", () => { let table: Table let rows: Row[] @@ -2325,7 +2325,7 @@ describe.each([ ) }) - it.only("should be able to search by calculations", async () => { + it("should be able to search by calculations", async () => { const view = await config.api.viewV2.create({ tableId: table._id!, name: generator.guid(), diff --git a/packages/shared-core/src/helpers/views.ts b/packages/shared-core/src/helpers/views.ts index 0364ccff41..c65bc4882d 100644 --- a/packages/shared-core/src/helpers/views.ts +++ b/packages/shared-core/src/helpers/views.ts @@ -25,15 +25,9 @@ export function isCalculationView(view: UnsavedViewV2) { } export function calculationFields(view: UnsavedViewV2) { - if (!isCalculationView(view)) { - throw new Error("View is not a calculation view") - } return pickBy(view.schema || {}, isCalculationField) } export function basicFields(view: UnsavedViewV2) { - if (!isCalculationView(view)) { - throw new Error("View is not a calculation view") - } return pickBy(view.schema || {}, field => !isCalculationField(field)) } From c5db1d1da305e350804685e59877780aafa45011 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 20 Sep 2024 16:37:23 +0100 Subject: [PATCH 03/28] Got a test passing but I hate it a bit. --- .../src/api/controllers/row/utils/basic.ts | 16 +++++++++++++++- .../src/api/controllers/row/utils/utils.ts | 4 +++- .../server/src/api/routes/tests/viewV2.spec.ts | 16 ++++++++-------- packages/server/src/db/linkedRows/index.ts | 16 ++++++++++++---- packages/server/src/sdk/app/rows/search.ts | 1 + .../src/sdk/app/rows/search/internal/sqs.ts | 11 ++++++++--- .../server/src/utilities/rowProcessor/index.ts | 9 +++++++++ 7 files changed, 56 insertions(+), 17 deletions(-) diff --git a/packages/server/src/api/controllers/row/utils/basic.ts b/packages/server/src/api/controllers/row/utils/basic.ts index 9c49f5636a..20a1cd0742 100644 --- a/packages/server/src/api/controllers/row/utils/basic.ts +++ b/packages/server/src/api/controllers/row/utils/basic.ts @@ -1,5 +1,12 @@ // need to handle table name + field or just field, depending on if relationships used -import { FieldSchema, FieldType, Row, Table, JsonTypes } from "@budibase/types" +import { + FieldSchema, + FieldType, + Row, + Table, + JsonTypes, + Aggregation, +} from "@budibase/types" import { helpers, PROTECTED_EXTERNAL_COLUMNS, @@ -84,12 +91,14 @@ export function basicProcessing({ tables, isLinked, sqs, + aggregations, }: { row: Row table: Table tables: Table[] isLinked: boolean sqs?: boolean + aggregations?: Aggregation[] }): Row { const thisRow: Row = {} // filter the row down to what is actually the row (not joined) @@ -108,6 +117,11 @@ export function basicProcessing({ thisRow[fieldName] = value } } + + for (let aggregation of aggregations || []) { + thisRow[aggregation.name] = row[aggregation.name] + } + let columns: string[] = Object.keys(table.schema) if (!sqs) { thisRow._id = generateIdForRow(row, table, isLinked) diff --git a/packages/server/src/api/controllers/row/utils/utils.ts b/packages/server/src/api/controllers/row/utils/utils.ts index ac305e70b6..1fc7221294 100644 --- a/packages/server/src/api/controllers/row/utils/utils.ts +++ b/packages/server/src/api/controllers/row/utils/utils.ts @@ -2,6 +2,7 @@ import * as utils from "../../../../db/utils" import { context } from "@budibase/backend-core" import { + Aggregation, Ctx, DatasourcePlusQueryResponse, FieldType, @@ -129,7 +130,7 @@ export async function sqlOutputProcessing( table: Table, tables: Record, relationships: RelationshipsJson[], - opts?: { sqs?: boolean } + opts?: { sqs?: boolean; aggregations?: Aggregation[] } ): Promise { if (isKnexEmptyReadResponse(rows)) { return [] @@ -150,6 +151,7 @@ export async function sqlOutputProcessing( tables: Object.values(tables), isLinked: false, sqs: opts?.sqs, + aggregations: opts?.aggregations, }) if (thisRow._id == null) { throw new Error("Unable to generate row ID for SQL rows") diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index aaaf03c113..dc940c5ace 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -37,13 +37,13 @@ import { import sdk from "../../../sdk" describe.each([ - ["lucene", undefined], + // ["lucene", undefined], ["sqs", undefined], - [DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)], - [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)], - [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)], - [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)], - [DatabaseName.ORACLE, getDatasource(DatabaseName.ORACLE)], + // [DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)], + // [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)], + // [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)], + // [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)], + // [DatabaseName.ORACLE, getDatasource(DatabaseName.ORACLE)], ])("/v2/views (%s)", (name, dsProvider) => { const config = setup.getConfig() const isSqs = name === "sqs" @@ -2215,7 +2215,7 @@ describe.each([ ) }) - describe.skip("calculations", () => { + describe("calculations", () => { let table: Table let rows: Row[] @@ -2245,7 +2245,7 @@ describe.each([ ) }) - it("should be able to search by calculations", async () => { + it.only("should be able to search by calculations", async () => { const view = await config.api.viewV2.create({ tableId: table._id!, name: generator.guid(), diff --git a/packages/server/src/db/linkedRows/index.ts b/packages/server/src/db/linkedRows/index.ts index 2c8d1f77ac..4222f9b5e4 100644 --- a/packages/server/src/db/linkedRows/index.ts +++ b/packages/server/src/db/linkedRows/index.ts @@ -20,10 +20,11 @@ import { Row, Table, TableSchema, - ViewFieldMetadata, + ViewUIFieldMetadata, ViewV2, } from "@budibase/types" import sdk from "../../sdk" +import { helpers } from "@budibase/shared-core" export { IncludeDocs, getLinkDocuments, createLinkView } from "./linkUtils" @@ -264,12 +265,19 @@ export async function squashLinks( FeatureFlag.ENRICHED_RELATIONSHIPS ) - let viewSchema: Record = {} - if (options?.fromViewId && allowRelationshipSchemas) { + let viewSchema: Record = {} + if (options?.fromViewId) { const view = Object.values(table.views || {}).find( (v): v is ViewV2 => sdk.views.isV2(v) && v.id === options?.fromViewId ) - viewSchema = view?.schema || {} + + if (view && helpers.views.isCalculationView(view)) { + return enriched + } + + if (allowRelationshipSchemas && view) { + viewSchema = view.schema || {} + } } // will populate this as we find them diff --git a/packages/server/src/sdk/app/rows/search.ts b/packages/server/src/sdk/app/rows/search.ts index 95cbc919a1..dbf0cefd51 100644 --- a/packages/server/src/sdk/app/rows/search.ts +++ b/packages/server/src/sdk/app/rows/search.ts @@ -37,6 +37,7 @@ export async function search( return await tracer.trace("search", async span => { span?.addTags({ tableId: options.tableId, + viewId: options.viewId, query: options.query, sort: options.sort, sortOrder: options.sortOrder, diff --git a/packages/server/src/sdk/app/rows/search/internal/sqs.ts b/packages/server/src/sdk/app/rows/search/internal/sqs.ts index 188c95fb5c..b5bf8e752f 100644 --- a/packages/server/src/sdk/app/rows/search/internal/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/internal/sqs.ts @@ -386,8 +386,9 @@ export async function search( // make sure JSON columns corrected const processed = builder.convertJsonStringColumns( table, - await sqlOutputProcessing(rows, table!, allTablesMap, relationships, { + await sqlOutputProcessing(rows, table, allTablesMap, relationships, { sqs: true, + aggregations: options.aggregations, }) ) @@ -406,11 +407,16 @@ export async function search( preserveLinks: true, squash: true, fromViewId: options.viewId, + aggregations: options.aggregations, }) // check if we need to pick specific rows out if (options.fields) { - const fields = [...options.fields, ...PROTECTED_INTERNAL_COLUMNS] + const fields = [ + ...options.fields, + ...PROTECTED_INTERNAL_COLUMNS, + ...(options.aggregations || []).map(a => a.name), + ] finalRows = finalRows.map((r: any) => pick(r, fields)) } @@ -440,6 +446,5 @@ export async function search( return { rows: [] } } throw err - throw new Error(`Unable to search by SQL - ${msg}`, { cause: err }) } } diff --git a/packages/server/src/utilities/rowProcessor/index.ts b/packages/server/src/utilities/rowProcessor/index.ts index f6cf44d6d6..73768fdd57 100644 --- a/packages/server/src/utilities/rowProcessor/index.ts +++ b/packages/server/src/utilities/rowProcessor/index.ts @@ -11,6 +11,7 @@ import { import { InternalTables } from "../../db/utils" import { TYPE_TRANSFORM_MAP } from "./map" import { + Aggregation, AutoFieldSubType, FieldType, IdentityType, @@ -250,6 +251,7 @@ export async function outputProcessing( fromRow?: Row skipBBReferences?: boolean fromViewId?: string + aggregations?: Aggregation[] } = { squash: true, preserveLinks: false, @@ -357,6 +359,7 @@ export async function outputProcessing( fromViewId: opts?.fromViewId, }) } + // remove null properties to match internal API const isExternal = isExternalTableID(table._id!) if (isExternal || (await features.flags.isEnabled("SQS"))) { @@ -385,9 +388,15 @@ export async function outputProcessing( const tableFields = Object.keys(table.schema).filter( f => table.schema[f].visible !== false ) + const fields = [...tableFields, ...protectedColumns].map(f => f.toLowerCase() ) + + for (const aggregation of opts.aggregations || []) { + fields.push(aggregation.name.toLowerCase()) + } + for (const row of enriched) { for (const key of Object.keys(row)) { if (!fields.includes(key.toLowerCase())) { From 51774b3434a344e0ab8e766b96ad2e840ce44033 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 24 Sep 2024 12:30:45 +0100 Subject: [PATCH 04/28] Working on plumbing 'source' all the way through our code. --- .../backend-core/src/context/mainContext.ts | 19 +++- packages/backend-core/src/context/types.ts | 3 +- packages/backend-core/src/docIds/params.ts | 14 +-- packages/backend-core/src/sql/sql.ts | 3 - .../api/controllers/row/ExternalRequest.ts | 89 ++++++++++--------- .../src/api/controllers/row/external.ts | 19 ++-- .../src/api/controllers/row/utils/utils.ts | 13 ++- .../server/src/api/controllers/row/views.ts | 7 +- packages/server/src/db/utils.ts | 8 +- .../middleware/triggerRowActionAuthorised.ts | 2 +- .../server/src/sdk/app/permissions/index.ts | 6 +- packages/server/src/sdk/app/rowActions.ts | 6 +- .../server/src/sdk/app/rows/queryUtils.ts | 7 +- packages/server/src/sdk/app/rows/rows.ts | 5 +- packages/server/src/sdk/app/rows/search.ts | 58 +++++++----- .../src/sdk/app/rows/search/external.ts | 7 +- .../server/src/sdk/app/rows/search/utils.ts | 6 +- .../src/sdk/app/rows/tests/queryUtils.spec.ts | 20 ++--- packages/server/src/sdk/app/rows/utils.ts | 35 +++----- packages/server/src/sdk/app/tables/utils.ts | 4 + packages/server/src/sdk/app/views/index.ts | 19 +++- packages/types/src/sdk/row.ts | 5 +- yarn.lock | 64 +------------ 23 files changed, 204 insertions(+), 215 deletions(-) diff --git a/packages/backend-core/src/context/mainContext.ts b/packages/backend-core/src/context/mainContext.ts index a52a17dd53..25b273e51c 100644 --- a/packages/backend-core/src/context/mainContext.ts +++ b/packages/backend-core/src/context/mainContext.ts @@ -10,7 +10,7 @@ import { StaticDatabases, DEFAULT_TENANT_ID, } from "../constants" -import { Database, IdentityContext, Snippet, App } from "@budibase/types" +import { Database, IdentityContext, Snippet, App, Table } from "@budibase/types" import { ContextMap } from "./types" let TEST_APP_ID: string | null = null @@ -394,3 +394,20 @@ export function setFeatureFlags(key: string, value: Record) { context.featureFlagCache ??= {} context.featureFlagCache[key] = value } + +export function getTableForView(viewId: string): Table | undefined { + const context = getCurrentContext() + if (!context) { + return + } + return context.viewToTableCache?.[viewId] +} + +export function setTableForView(viewId: string, table: Table) { + const context = getCurrentContext() + if (!context) { + return + } + context.viewToTableCache ??= {} + context.viewToTableCache[viewId] = table +} diff --git a/packages/backend-core/src/context/types.ts b/packages/backend-core/src/context/types.ts index fe6072e85c..ee84b49459 100644 --- a/packages/backend-core/src/context/types.ts +++ b/packages/backend-core/src/context/types.ts @@ -1,4 +1,4 @@ -import { IdentityContext, Snippet, VM } from "@budibase/types" +import { IdentityContext, Snippet, Table, VM } from "@budibase/types" import { OAuth2Client } from "google-auth-library" import { GoogleSpreadsheet } from "google-spreadsheet" @@ -21,4 +21,5 @@ export type ContextMap = { featureFlagCache?: { [key: string]: Record } + viewToTableCache?: Record } diff --git a/packages/backend-core/src/docIds/params.ts b/packages/backend-core/src/docIds/params.ts index 093724b55e..016604b69b 100644 --- a/packages/backend-core/src/docIds/params.ts +++ b/packages/backend-core/src/docIds/params.ts @@ -6,7 +6,7 @@ import { ViewName, } from "../constants" import { getProdAppID } from "./conversions" -import { DatabaseQueryOpts } from "@budibase/types" +import { DatabaseQueryOpts, VirtualDocumentType } from "@budibase/types" /** * If creating DB allDocs/query params with only a single top level ID this can be used, this @@ -66,9 +66,8 @@ export function getQueryIndex(viewName: ViewName) { /** * Check if a given ID is that of a table. - * @returns {boolean} */ -export const isTableId = (id: string) => { +export const isTableId = (id: string): boolean => { // this includes datasource plus tables return ( !!id && @@ -77,13 +76,16 @@ export const isTableId = (id: string) => { ) } +export function isViewId(id: string): boolean { + return !!id && id.startsWith(`${VirtualDocumentType.VIEW}${SEPARATOR}`) +} + /** * Check if a given ID is that of a datasource or datasource plus. - * @returns {boolean} */ -export const isDatasourceId = (id: string) => { +export const isDatasourceId = (id: string): boolean => { // this covers both datasources and datasource plus - return id && id.startsWith(`${DocumentType.DATASOURCE}${SEPARATOR}`) + return !!id && id.startsWith(`${DocumentType.DATASOURCE}${SEPARATOR}`) } /** diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index 023bfd5d8a..97fc4124fb 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -1274,9 +1274,6 @@ class InternalBuilder { if (counting) { query = this.addDistinctCount(query) } else if (aggregations.length > 0) { - query = query.select( - this.knex.raw("ROW_NUMBER() OVER (ORDER BY (SELECT 0)) as _id") - ) query = this.addAggregations(query, aggregations) } else { query = query.select(this.generateSelectStatement()) diff --git a/packages/server/src/api/controllers/row/ExternalRequest.ts b/packages/server/src/api/controllers/row/ExternalRequest.ts index 9c9bd0b284..a52d7abcd1 100644 --- a/packages/server/src/api/controllers/row/ExternalRequest.ts +++ b/packages/server/src/api/controllers/row/ExternalRequest.ts @@ -19,6 +19,7 @@ import { SortJson, SortType, Table, + ViewV2, } from "@budibase/types" import { breakExternalTableId, @@ -159,17 +160,41 @@ function isEditableColumn(column: FieldSchema) { export class ExternalRequest { private readonly operation: T - private readonly tableId: string - private datasource?: Datasource - private tables: { [key: string]: Table } = {} + private readonly source: Table | ViewV2 + private datasource: Datasource - constructor(operation: T, tableId: string, datasource?: Datasource) { - this.operation = operation - this.tableId = tableId - this.datasource = datasource - if (datasource && datasource.entities) { - this.tables = datasource.entities + public static async for( + operation: T, + source: Table | ViewV2, + opts: { datasource?: Datasource } = {} + ) { + if (!opts.datasource) { + if (sdk.views.isView(source)) { + const table = await sdk.views.getTable(source.id) + opts.datasource = await sdk.datasources.get(table.sourceId!) + } else { + opts.datasource = await sdk.datasources.get(source.sourceId!) + } } + + return new ExternalRequest(operation, source, opts.datasource) + } + + private get tables(): { [key: string]: Table } { + if (!this.datasource.entities) { + throw new Error("Datasource does not have entities") + } + return this.datasource.entities + } + + private constructor( + operation: T, + source: Table | ViewV2, + datasource: Datasource + ) { + this.operation = operation + this.source = source + this.datasource = datasource } private prepareFilters( @@ -290,20 +315,6 @@ export class ExternalRequest { return this.tables[tableName] } - // seeds the object with table and datasource information - async retrieveMetadata( - datasourceId: string - ): Promise<{ tables: Record; datasource: Datasource }> { - if (!this.datasource) { - this.datasource = await sdk.datasources.get(datasourceId) - if (!this.datasource || !this.datasource.entities) { - throw "No tables found, fetch tables before query." - } - this.tables = this.datasource.entities - } - return { tables: this.tables, datasource: this.datasource } - } - async getRow(table: Table, rowId: string): Promise { const response = await getDatasourceAndQuery({ endpoint: getEndpoint(table._id!, Operation.READ), @@ -619,24 +630,16 @@ export class ExternalRequest { } async run(config: RunConfig): Promise> { - const { operation, tableId } = this - if (!tableId) { - throw new Error("Unable to run without a table ID") - } - let { datasourceId, tableName } = breakExternalTableId(tableId) - let datasource = this.datasource - if (!datasource) { - const { datasource: ds } = await this.retrieveMetadata(datasourceId) - datasource = ds - } - const tables = this.tables - const table = tables[tableName] - let isSql = isSQL(datasource) - if (!table) { - throw new Error( - `Unable to process query, table "${tableName}" not defined.` - ) + const { operation } = this + let table: Table + if (sdk.views.isView(this.source)) { + table = await sdk.views.getTable(this.source.id) + } else { + table = this.source } + + let isSql = isSQL(this.datasource) + // look for specific components of config which may not be considered acceptable let { id, row, filters, sort, paginate, rows } = cleanupConfig( config, @@ -687,8 +690,8 @@ export class ExternalRequest { } let json: QueryJson = { endpoint: { - datasourceId: datasourceId!, - entityId: tableName, + datasourceId: this.datasource._id!, + entityId: table.name, operation, }, resource: { @@ -714,7 +717,7 @@ export class ExternalRequest { }, meta: { table, - tables: tables, + tables: this.tables, }, } diff --git a/packages/server/src/api/controllers/row/external.ts b/packages/server/src/api/controllers/row/external.ts index bd5201c05c..fb992059f4 100644 --- a/packages/server/src/api/controllers/row/external.ts +++ b/packages/server/src/api/controllers/row/external.ts @@ -17,6 +17,7 @@ import { Row, Table, UserCtx, + ViewV2, } from "@budibase/types" import sdk from "../../../sdk" import * as utils from "./utils" @@ -29,29 +30,29 @@ import { generateIdForRow } from "./utils" export async function handleRequest( operation: T, - tableId: string, + source: Table | ViewV2, opts?: RunConfig ): Promise> { - return new ExternalRequest(operation, tableId, opts?.datasource).run( - opts || {} - ) + return ( + await ExternalRequest.for(operation, source, { + datasource: opts?.datasource, + }) + ).run(opts || {}) } export async function patch(ctx: UserCtx) { - const { tableId, viewId } = utils.getSourceId(ctx) - + const source = await utils.getSource(ctx) const { _id, ...rowData } = ctx.request.body - const table = await sdk.tables.getTable(tableId) const { row: dataToUpdate } = await inputProcessing( ctx.user?._id, - cloneDeep(table), + cloneDeep(source), rowData ) const validateResult = await sdk.rows.utils.validate({ row: dataToUpdate, - tableId, + source, }) if (!validateResult.valid) { throw { validation: validateResult.errors } diff --git a/packages/server/src/api/controllers/row/utils/utils.ts b/packages/server/src/api/controllers/row/utils/utils.ts index 1fc7221294..91c0fc966f 100644 --- a/packages/server/src/api/controllers/row/utils/utils.ts +++ b/packages/server/src/api/controllers/row/utils/utils.ts @@ -1,6 +1,6 @@ import * as utils from "../../../../db/utils" -import { context } from "@budibase/backend-core" +import { context, docIds } from "@budibase/backend-core" import { Aggregation, Ctx, @@ -9,6 +9,7 @@ import { RelationshipsJson, Row, Table, + ViewV2, } from "@budibase/types" import { processDates, @@ -78,7 +79,7 @@ export function getSourceId(ctx: Ctx): { tableId: string; viewId?: string } { // top priority, use the URL first if (ctx.params?.sourceId) { const { sourceId } = ctx.params - if (utils.isViewID(sourceId)) { + if (docIds.isViewId(sourceId)) { return { tableId: utils.extractViewInfoFromID(sourceId).tableId, viewId: sourceId, @@ -97,6 +98,14 @@ export function getSourceId(ctx: Ctx): { tableId: string; viewId?: string } { throw new Error("Unable to find table ID in request") } +export async function getSource(ctx: Ctx): Promise { + const { tableId, viewId } = getSourceId(ctx) + if (viewId) { + return sdk.views.get(viewId) + } + return sdk.tables.getTable(tableId) +} + export async function validate( opts: { row: Row } & ({ tableId: string } | { table: Table }) ) { diff --git a/packages/server/src/api/controllers/row/views.ts b/packages/server/src/api/controllers/row/views.ts index f9565f0e82..7008c5e0be 100644 --- a/packages/server/src/api/controllers/row/views.ts +++ b/packages/server/src/api/controllers/row/views.ts @@ -84,11 +84,8 @@ export async function searchView( })) const searchOptions: RequiredKeys & - RequiredKeys< - Pick - > = { - tableId: view.tableId, - viewId: view.id, + RequiredKeys> = { + sourceId: view.id, query: enrichedQuery, fields: viewFields, ...getSortOptions(body, view), diff --git a/packages/server/src/db/utils.ts b/packages/server/src/db/utils.ts index 043394e7a6..6c1065e847 100644 --- a/packages/server/src/db/utils.ts +++ b/packages/server/src/db/utils.ts @@ -1,4 +1,4 @@ -import { context, db as dbCore, utils } from "@budibase/backend-core" +import { context, db as dbCore, docIds, utils } from "@budibase/backend-core" import { DatabaseQueryOpts, Datasource, @@ -318,12 +318,8 @@ export function generateViewID(tableId: string) { }${SEPARATOR}${tableId}${SEPARATOR}${newid()}` } -export function isViewID(viewId: string) { - return viewId?.split(SEPARATOR)[0] === VirtualDocumentType.VIEW -} - export function extractViewInfoFromID(viewId: string) { - if (!isViewID(viewId)) { + if (!docIds.isViewId(viewId)) { throw new Error("Unable to extract table ID, is not a view ID") } const split = viewId.split(SEPARATOR) diff --git a/packages/server/src/middleware/triggerRowActionAuthorised.ts b/packages/server/src/middleware/triggerRowActionAuthorised.ts index 17f22b7000..3e903ba907 100644 --- a/packages/server/src/middleware/triggerRowActionAuthorised.ts +++ b/packages/server/src/middleware/triggerRowActionAuthorised.ts @@ -15,7 +15,7 @@ export function triggerRowActionAuthorised( const rowActionId: string = ctx.params[actionPath] const isTableId = docIds.isTableId(sourceId) - const isViewId = utils.isViewID(sourceId) + const isViewId = docIds.isViewId(sourceId) if (!isTableId && !isViewId) { ctx.throw(400, `'${sourceId}' is not a valid source id`) } diff --git a/packages/server/src/sdk/app/permissions/index.ts b/packages/server/src/sdk/app/permissions/index.ts index a6e81652ee..d5e4aefe3a 100644 --- a/packages/server/src/sdk/app/permissions/index.ts +++ b/packages/server/src/sdk/app/permissions/index.ts @@ -1,10 +1,10 @@ -import { db, roles } from "@budibase/backend-core" +import { db, docIds, roles } from "@budibase/backend-core" import { PermissionLevel, PermissionSource, VirtualDocumentType, } from "@budibase/types" -import { extractViewInfoFromID, isViewID } from "../../../db/utils" +import { extractViewInfoFromID } from "../../../db/utils" import { CURRENTLY_SUPPORTED_LEVELS, getBasePermissions, @@ -20,7 +20,7 @@ type ResourcePermissions = Record< export async function getInheritablePermissions( resourceId: string ): Promise { - if (isViewID(resourceId)) { + if (docIds.isViewId(resourceId)) { return await getResourcePerms(extractViewInfoFromID(resourceId).tableId) } } diff --git a/packages/server/src/sdk/app/rowActions.ts b/packages/server/src/sdk/app/rowActions.ts index 9a7d402df0..21c256eacb 100644 --- a/packages/server/src/sdk/app/rowActions.ts +++ b/packages/server/src/sdk/app/rowActions.ts @@ -1,11 +1,11 @@ -import { context, HTTPError, utils } from "@budibase/backend-core" +import { context, docIds, HTTPError, utils } from "@budibase/backend-core" import { AutomationTriggerStepId, SEPARATOR, TableRowActions, VirtualDocumentType, } from "@budibase/types" -import { generateRowActionsID, isViewID } from "../../db/utils" +import { generateRowActionsID } from "../../db/utils" import automations from "./automations" import { definitions as TRIGGER_DEFINITIONS } from "../../automations/triggerInfo" import * as triggers from "../../automations/triggers" @@ -155,7 +155,7 @@ export async function update( async function guardView(tableId: string, viewId: string) { let view - if (isViewID(viewId)) { + if (docIds.isViewId(viewId)) { view = await sdk.views.get(viewId) } if (!view || view.tableId !== tableId) { diff --git a/packages/server/src/sdk/app/rows/queryUtils.ts b/packages/server/src/sdk/app/rows/queryUtils.ts index 65f400a1d9..a88763215e 100644 --- a/packages/server/src/sdk/app/rows/queryUtils.ts +++ b/packages/server/src/sdk/app/rows/queryUtils.ts @@ -53,8 +53,8 @@ export const removeInvalidFilters = ( } export const getQueryableFields = async ( - fields: string[], - table: Table + table: Table, + fields?: string[] ): Promise => { const extractTableFields = async ( table: Table, @@ -110,6 +110,9 @@ export const getQueryableFields = async ( "_id", // Querying by _id is always allowed, even if it's never part of the schema ] + if (fields === undefined) { + fields = Object.keys(table.schema) + } result.push(...(await extractTableFields(table, fields, [table._id!]))) return result diff --git a/packages/server/src/sdk/app/rows/rows.ts b/packages/server/src/sdk/app/rows/rows.ts index c61b8692ed..ef25d06baf 100644 --- a/packages/server/src/sdk/app/rows/rows.ts +++ b/packages/server/src/sdk/app/rows/rows.ts @@ -1,9 +1,8 @@ -import { db as dbCore, context } from "@budibase/backend-core" +import { db as dbCore, context, docIds } from "@budibase/backend-core" import { Database, Row } from "@budibase/types" import { extractViewInfoFromID, getRowParams, - isViewID, } from "../../../db/utils" import { isExternalTableID } from "../../../integrations/utils" import * as internal from "./internal" @@ -26,7 +25,7 @@ export async function getAllInternalRows(appId?: string) { function pickApi(tableOrViewId: string) { let tableId = tableOrViewId - if (isViewID(tableOrViewId)) { + if (docIds.isViewId(tableOrViewId)) { tableId = extractViewInfoFromID(tableOrViewId).tableId } diff --git a/packages/server/src/sdk/app/rows/search.ts b/packages/server/src/sdk/app/rows/search.ts index dbf0cefd51..fa01a6cf13 100644 --- a/packages/server/src/sdk/app/rows/search.ts +++ b/packages/server/src/sdk/app/rows/search.ts @@ -4,6 +4,8 @@ import { RowSearchParams, SearchResponse, SortOrder, + Table, + ViewV2, } from "@budibase/types" import { isExternalTableID } from "../../../integrations/utils" import * as internal from "./search/internal" @@ -12,7 +14,7 @@ import { ExportRowsParams, ExportRowsResult } from "./search/types" import { dataFilters } from "@budibase/shared-core" import sdk from "../../index" import { searchInputMapping } from "./search/utils" -import { features } from "@budibase/backend-core" +import { features, docIds } from "@budibase/backend-core" import tracer from "dd-trace" import { getQueryableFields, removeInvalidFilters } from "./queryUtils" @@ -36,8 +38,7 @@ export async function search( ): Promise> { return await tracer.trace("search", async span => { span?.addTags({ - tableId: options.tableId, - viewId: options.viewId, + sourceId: options.sourceId, query: options.query, sort: options.sort, sortOrder: options.sortOrder, @@ -52,20 +53,18 @@ export async function search( .join(", "), }) - const isExternalTable = isExternalTableID(options.tableId) options.query = dataFilters.cleanupQuery(options.query || {}) options.query = dataFilters.fixupFilterArrays(options.query) - span?.addTags({ + span.addTags({ cleanedQuery: options.query, - isExternalTable, }) if ( !dataFilters.hasFilters(options.query) && options.query.onEmptyFilter === EmptyFilterOption.RETURN_NONE ) { - span?.addTags({ emptyQuery: true }) + span.addTags({ emptyQuery: true }) return { rows: [], } @@ -75,34 +74,47 @@ export async function search( options.sortOrder = options.sortOrder.toLowerCase() as SortOrder } - const table = await sdk.tables.getTable(options.tableId) - options = searchInputMapping(table, options) + let source: Table | ViewV2 + let table: Table + if (docIds.isTableId(options.sourceId)) { + source = await sdk.tables.getTable(options.sourceId) + table = source + options = searchInputMapping(source, options) + } else if (docIds.isViewId(options.sourceId)) { + source = await sdk.views.get(options.sourceId) + table = await sdk.tables.getTable(source.tableId) + options = searchInputMapping(table, options) - if (options.query) { - const tableFields = Object.keys(table.schema).filter( - f => table.schema[f].visible !== false - ) - - const queriableFields = await getQueryableFields( - options.fields?.filter(f => tableFields.includes(f)) ?? tableFields, - table - ) - options.query = removeInvalidFilters(options.query, queriableFields) + span.addTags({ + tableId: table._id, + }) + } else { + throw new Error(`Invalid source ID: ${options.sourceId}`) } + if (options.query) { + const visibleFields = ( + options.fields || Object.keys(table.schema) + ).filter(field => table.schema[field].visible) + + const queryableFields = await getQueryableFields(table, visibleFields) + options.query = removeInvalidFilters(options.query, queryableFields) + } + + const isExternalTable = isExternalTableID(table._id!) let result: SearchResponse if (isExternalTable) { span?.addTags({ searchType: "external" }) - result = await external.search(options, table) + result = await external.search(options, source) } else if (await features.flags.isEnabled("SQS")) { span?.addTags({ searchType: "sqs" }) - result = await internal.sqs.search(options, table) + result = await internal.sqs.search(options, source) } else { span?.addTags({ searchType: "lucene" }) - result = await internal.lucene.search(options, table) + result = await internal.lucene.search(options, source) } - span?.addTags({ + span.addTags({ foundRows: result.rows.length, totalRows: result.totalRows, }) diff --git a/packages/server/src/sdk/app/rows/search/external.ts b/packages/server/src/sdk/app/rows/search/external.ts index 0ff25a00e4..5584b3f110 100644 --- a/packages/server/src/sdk/app/rows/search/external.ts +++ b/packages/server/src/sdk/app/rows/search/external.ts @@ -9,6 +9,7 @@ import { SortJson, SortOrder, Table, + ViewV2, } from "@budibase/types" import * as exporters from "../../../../api/controllers/view/exporters" import { handleRequest } from "../../../../api/controllers/row/external" @@ -60,9 +61,8 @@ function getPaginationAndLimitParameters( export async function search( options: RowSearchParams, - table: Table + source: Table | ViewV2 ): Promise> { - const { tableId } = options const { countRows, paginate, query, ...params } = options const { limit } = params let bookmark = @@ -112,10 +112,9 @@ export async function search( : Promise.resolve(undefined), ]) - let processed = await outputProcessing(table, rows, { + let processed = await outputProcessing(source, rows, { preserveLinks: true, squash: true, - fromViewId: options.viewId, }) let hasNextPage = false diff --git a/packages/server/src/sdk/app/rows/search/utils.ts b/packages/server/src/sdk/app/rows/search/utils.ts index 5ffc065353..d727e58887 100644 --- a/packages/server/src/sdk/app/rows/search/utils.ts +++ b/packages/server/src/sdk/app/rows/search/utils.ts @@ -9,6 +9,7 @@ import { SearchResponse, Row, RowSearchParams, + ViewV2, } from "@budibase/types" import { db as dbCore, context } from "@budibase/backend-core" import { utils } from "@budibase/shared-core" @@ -83,10 +84,7 @@ function userColumnMapping(column: string, options: RowSearchParams) { // maps through the search parameters to check if any of the inputs are invalid // based on the table schema, converts them to something that is valid. export function searchInputMapping(table: Table, options: RowSearchParams) { - if (!table?.schema) { - return options - } - for (let [key, column] of Object.entries(table.schema)) { + for (let [key, column] of Object.entries(table.schema || {})) { switch (column.type) { case FieldType.BB_REFERENCE_SINGLE: { const subtype = column.subtype diff --git a/packages/server/src/sdk/app/rows/tests/queryUtils.spec.ts b/packages/server/src/sdk/app/rows/tests/queryUtils.spec.ts index aabc359484..f399801f1e 100644 --- a/packages/server/src/sdk/app/rows/tests/queryUtils.spec.ts +++ b/packages/server/src/sdk/app/rows/tests/queryUtils.spec.ts @@ -203,7 +203,7 @@ describe("query utils", () => { }, }) - const result = await getQueryableFields(Object.keys(table.schema), table) + const result = await getQueryableFields(table) expect(result).toEqual(["_id", "name", "age"]) }) @@ -216,7 +216,7 @@ describe("query utils", () => { }, }) - const result = await getQueryableFields(Object.keys(table.schema), table) + const result = await getQueryableFields(table) expect(result).toEqual(["_id", "name"]) }) @@ -245,7 +245,7 @@ describe("query utils", () => { }) const result = await config.doInContext(config.appId, () => { - return getQueryableFields(Object.keys(table.schema), table) + return getQueryableFields(table) }) expect(result).toEqual([ "_id", @@ -282,7 +282,7 @@ describe("query utils", () => { }) const result = await config.doInContext(config.appId, () => { - return getQueryableFields(Object.keys(table.schema), table) + return getQueryableFields(table) }) expect(result).toEqual(["_id", "name", "aux.name", "auxTable.name"]) }) @@ -313,7 +313,7 @@ describe("query utils", () => { }) const result = await config.doInContext(config.appId, () => { - return getQueryableFields(Object.keys(table.schema), table) + return getQueryableFields(table) }) expect(result).toEqual(["_id", "name"]) }) @@ -381,7 +381,7 @@ describe("query utils", () => { it("includes nested relationship fields from main table", async () => { const result = await config.doInContext(config.appId, () => { - return getQueryableFields(Object.keys(table.schema), table) + return getQueryableFields(table) }) expect(result).toEqual([ "_id", @@ -398,7 +398,7 @@ describe("query utils", () => { it("includes nested relationship fields from aux 1 table", async () => { const result = await config.doInContext(config.appId, () => { - return getQueryableFields(Object.keys(aux1.schema), aux1) + return getQueryableFields(aux1) }) expect(result).toEqual([ "_id", @@ -420,7 +420,7 @@ describe("query utils", () => { it("includes nested relationship fields from aux 2 table", async () => { const result = await config.doInContext(config.appId, () => { - return getQueryableFields(Object.keys(aux2.schema), aux2) + return getQueryableFields(aux2) }) expect(result).toEqual([ "_id", @@ -474,7 +474,7 @@ describe("query utils", () => { it("includes nested relationship fields from main table", async () => { const result = await config.doInContext(config.appId, () => { - return getQueryableFields(Object.keys(table.schema), table) + return getQueryableFields(table) }) expect(result).toEqual([ "_id", @@ -488,7 +488,7 @@ describe("query utils", () => { it("includes nested relationship fields from aux table", async () => { const result = await config.doInContext(config.appId, () => { - return getQueryableFields(Object.keys(aux.schema), aux) + return getQueryableFields(aux) }) expect(result).toEqual([ "_id", diff --git a/packages/server/src/sdk/app/rows/utils.ts b/packages/server/src/sdk/app/rows/utils.ts index bc09116b3b..3899009f13 100644 --- a/packages/server/src/sdk/app/rows/utils.ts +++ b/packages/server/src/sdk/app/rows/utils.ts @@ -13,16 +13,14 @@ import { TableSchema, SqlClient, ArrayOperator, + ViewV2, } from "@budibase/types" import { makeExternalQuery } from "../../../integrations/base/query" import { Format } from "../../../api/controllers/view/exporters" import sdk from "../.." -import { - extractViewInfoFromID, - isRelationshipColumn, - isViewID, -} from "../../../db/utils" +import { extractViewInfoFromID, isRelationshipColumn } from "../../../db/utils" import { isSQL } from "../../../integrations/utils" +import { docIds } from "@budibase/backend-core" const SQL_CLIENT_SOURCE_MAP: Record = { [SourceName.POSTGRES]: SqlClient.POSTGRES, @@ -142,37 +140,32 @@ function isForeignKey(key: string, table: Table) { } export async function validate({ - tableId, + source, row, - table, }: { - tableId?: string + source: Table | ViewV2 row: Row - table?: Table }): Promise<{ valid: boolean errors: Record }> { - let fetchedTable: Table | undefined - if (!table && tableId) { - fetchedTable = await sdk.tables.getTable(tableId) - } else if (table) { - fetchedTable = table - } - if (fetchedTable === undefined) { - throw new Error("Unable to fetch table for validation") + let table: Table + if (sdk.views.isView(source)) { + table = await sdk.views.getTable(source.id) + } else { + table = source } const errors: Record = {} const disallowArrayTypes = [ FieldType.ATTACHMENT_SINGLE, FieldType.BB_REFERENCE_SINGLE, ] - for (let fieldName of Object.keys(fetchedTable.schema)) { - const column = fetchedTable.schema[fieldName] + for (let fieldName of Object.keys(table.schema)) { + const column = table.schema[fieldName] const constraints = cloneDeep(column.constraints) const type = column.type // foreign keys are likely to be enriched - if (isForeignKey(fieldName, fetchedTable)) { + if (isForeignKey(fieldName, table)) { continue } // formulas shouldn't validated, data will be deleted anyway @@ -323,7 +316,7 @@ export function isArrayFilter(operator: any): operator is ArrayOperator { } export function tryExtractingTableAndViewId(tableOrViewId: string) { - if (isViewID(tableOrViewId)) { + if (docIds.isViewId(tableOrViewId)) { return { tableId: extractViewInfoFromID(tableOrViewId).tableId, viewId: tableOrViewId, diff --git a/packages/server/src/sdk/app/tables/utils.ts b/packages/server/src/sdk/app/tables/utils.ts index b8e3d888af..7a8096fb0a 100644 --- a/packages/server/src/sdk/app/tables/utils.ts +++ b/packages/server/src/sdk/app/tables/utils.ts @@ -9,3 +9,7 @@ export function isExternal(opts: { table?: Table; tableId?: string }): boolean { } return false } + +export function isTable(table: any): table is Table { + return table.type === "table" +} diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index de3579f7fd..f911385dc1 100644 --- a/packages/server/src/sdk/app/views/index.ts +++ b/packages/server/src/sdk/app/views/index.ts @@ -9,7 +9,7 @@ import { ViewV2ColumnEnriched, ViewV2Enriched, } from "@budibase/types" -import { HTTPError } from "@budibase/backend-core" +import { context, HTTPError } from "@budibase/backend-core" import { helpers, PROTECTED_EXTERNAL_COLUMNS, @@ -40,6 +40,23 @@ export async function getEnriched(viewId: string): Promise { return pickApi(tableId).getEnriched(viewId) } +export async function getTable(viewId: string): Promise
{ + const cached = context.getTableForView(viewId) + if (cached) { + return cached + } + const { tableId } = utils.extractViewInfoFromID(viewId) + const table = await sdk.tables.getTable(tableId) + context.setTableForView(viewId, table) + return table +} + +export function isView(view: any): view is ViewV2 { + return ( + view.version === 2 && "id" in view && "tableId" in view && "name" in view + ) +} + async function guardCalculationViewSchema( table: Table, view: Omit diff --git a/packages/types/src/sdk/row.ts b/packages/types/src/sdk/row.ts index f81d56c082..2b6ff3a6c6 100644 --- a/packages/types/src/sdk/row.ts +++ b/packages/types/src/sdk/row.ts @@ -10,8 +10,7 @@ export interface Aggregation { } export interface SearchParams { - tableId?: string - viewId?: string + sourceId?: string query?: SearchFilters paginate?: boolean bookmark?: string | number @@ -30,7 +29,7 @@ export interface SearchParams { // when searching for rows we want a more extensive search type that requires certain properties export interface RowSearchParams - extends WithRequired {} + extends WithRequired {} export interface SearchResponse { rows: T[] diff --git a/yarn.lock b/yarn.lock index cd850e833d..01466f66b2 100644 --- a/yarn.lock +++ b/yarn.lock @@ -17796,21 +17796,11 @@ periscopic@^3.1.0: estree-walker "^3.0.0" is-reference "^3.0.0" -pg-cloudflare@^1.1.1: - version "1.1.1" - resolved "https://registry.yarnpkg.com/pg-cloudflare/-/pg-cloudflare-1.1.1.tgz#e6d5833015b170e23ae819e8c5d7eaedb472ca98" - integrity sha512-xWPagP/4B6BgFO+EKz3JONXv3YDgvkbVrGw2mTo3D6tVDQRh1e7cqVGvyR3BE+eQgAvx1XhW/iEASj4/jCWl3Q== - pg-connection-string@2.5.0, pg-connection-string@^2.5.0: version "2.5.0" resolved "https://registry.yarnpkg.com/pg-connection-string/-/pg-connection-string-2.5.0.tgz#538cadd0f7e603fc09a12590f3b8a452c2c0cf34" integrity sha512-r5o/V/ORTA6TmUnyWZR9nCj1klXCO2CEKNRlVuJptZe85QuhFayC7WeMic7ndayT5IRIR0S0xFxFi2ousartlQ== -pg-connection-string@^2.6.4: - version "2.6.4" - resolved "https://registry.yarnpkg.com/pg-connection-string/-/pg-connection-string-2.6.4.tgz#f543862adfa49fa4e14bc8a8892d2a84d754246d" - integrity sha512-v+Z7W/0EO707aNMaAEfiGnGL9sxxumwLl2fJvCQtMn9Fxsg+lPpPkdcyBSv/KFgpGdYkMfn+EI1Or2EHjpgLCA== - pg-int8@1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/pg-int8/-/pg-int8-1.0.1.tgz#943bd463bf5b71b4170115f80f8efc9a0c0eb78c" @@ -17821,21 +17811,11 @@ pg-pool@^3.6.0: resolved "https://registry.yarnpkg.com/pg-pool/-/pg-pool-3.6.0.tgz#3190df3e4747a0d23e5e9e8045bcd99bda0a712e" integrity sha512-clFRf2ksqd+F497kWFyM21tMjeikn60oGDmqMT8UBrynEwVEX/5R5xd2sdvdo1cZCFlguORNpVuqxIj+aK4cfQ== -pg-pool@^3.6.2: - version "3.6.2" - resolved "https://registry.yarnpkg.com/pg-pool/-/pg-pool-3.6.2.tgz#3a592370b8ae3f02a7c8130d245bc02fa2c5f3f2" - integrity sha512-Htjbg8BlwXqSBQ9V8Vjtc+vzf/6fVUuak/3/XXKA9oxZprwW3IMDQTGHP+KDmVL7rtd+R1QjbnCFPuTHm3G4hg== - pg-protocol@*, pg-protocol@^1.6.0: version "1.6.0" resolved "https://registry.yarnpkg.com/pg-protocol/-/pg-protocol-1.6.0.tgz#4c91613c0315349363af2084608db843502f8833" integrity sha512-M+PDm637OY5WM307051+bsDia5Xej6d9IR4GwJse1qA1DIhiKlksvrneZOYQq42OM+spubpcNYEo2FcKQrDk+Q== -pg-protocol@^1.6.1: - version "1.6.1" - resolved "https://registry.yarnpkg.com/pg-protocol/-/pg-protocol-1.6.1.tgz#21333e6d83b01faaebfe7a33a7ad6bfd9ed38cb3" - integrity sha512-jPIlvgoD63hrEuihvIg+tJhoGjUsLPn6poJY9N5CnlPd91c2T18T/9zBtLxZSb1EhYxBRoZJtzScCaWlYLtktg== - pg-types@^2.1.0, pg-types@^2.2.0: version "2.2.0" resolved "https://registry.yarnpkg.com/pg-types/-/pg-types-2.2.0.tgz#2d0250d636454f7cfa3b6ae0382fdfa8063254a3" @@ -17860,19 +17840,6 @@ pg@8.10.0: pg-types "^2.1.0" pgpass "1.x" -pg@^8.12.0: - version "8.12.0" - resolved "https://registry.yarnpkg.com/pg/-/pg-8.12.0.tgz#9341724db571022490b657908f65aee8db91df79" - integrity sha512-A+LHUSnwnxrnL/tZ+OLfqR1SxLN3c/pgDztZ47Rpbsd4jUytsTtwQo/TLPRzPJMp/1pbhYVhH9cuSZLAajNfjQ== - dependencies: - pg-connection-string "^2.6.4" - pg-pool "^3.6.2" - pg-protocol "^1.6.1" - pg-types "^2.1.0" - pgpass "1.x" - optionalDependencies: - pg-cloudflare "^1.1.1" - pgpass@1.x: version "1.0.5" resolved "https://registry.yarnpkg.com/pgpass/-/pgpass-1.0.5.tgz#9b873e4a564bb10fa7a7dbd55312728d422a223d" @@ -20786,16 +20753,7 @@ string-similarity@^4.0.4: resolved "https://registry.yarnpkg.com/string-similarity/-/string-similarity-4.0.4.tgz#42d01ab0b34660ea8a018da8f56a3309bb8b2a5b" integrity sha512-/q/8Q4Bl4ZKAPjj8WerIBJWALKkaPRfrvhfF8k/B23i4nzrlRj2/go1m90In7nG/3XDSbOo0+pu6RvCTM9RGMQ== -"string-width-cjs@npm:string-width@^4.2.0": - version "4.2.3" - resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" - integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== - dependencies: - emoji-regex "^8.0.0" - is-fullwidth-code-point "^3.0.0" - strip-ansi "^6.0.1" - -"string-width@^1.0.2 || 2 || 3 || 4", string-width@^4.0.0, string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.2, string-width@^4.2.3: +"string-width-cjs@npm:string-width@^4.2.0", "string-width@^1.0.2 || 2 || 3 || 4", string-width@^4.0.0, string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.2, string-width@^4.2.3: version "4.2.3" resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== @@ -20886,7 +20844,7 @@ stringify-object@^3.2.1: is-obj "^1.0.1" is-regexp "^1.0.0" -"strip-ansi-cjs@npm:strip-ansi@^6.0.1": +"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@^6.0.0, strip-ansi@^6.0.1: version "6.0.1" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== @@ -20900,13 +20858,6 @@ strip-ansi@^5.0.0, strip-ansi@^5.1.0, strip-ansi@^5.2.0: dependencies: ansi-regex "^4.1.0" -strip-ansi@^6.0.0, strip-ansi@^6.0.1: - version "6.0.1" - resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" - integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== - dependencies: - ansi-regex "^5.0.1" - strip-ansi@^7.0.1: version "7.0.1" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-7.0.1.tgz#61740a08ce36b61e50e65653f07060d000975fb2" @@ -22862,7 +22813,7 @@ worker-farm@1.7.0: dependencies: errno "~0.1.7" -"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0": +"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@^7.0.0: version "7.0.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== @@ -22880,15 +22831,6 @@ wrap-ansi@^5.1.0: string-width "^3.0.0" strip-ansi "^5.0.0" -wrap-ansi@^7.0.0: - version "7.0.0" - resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" - integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== - dependencies: - ansi-styles "^4.0.0" - string-width "^4.1.0" - strip-ansi "^6.0.0" - wrap-ansi@^8.1.0: version "8.1.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-8.1.0.tgz#56dc22368ee570face1b49819975d9b9a5ead214" From fc9b54cb858c743f0530947b7aa984e974e6ceb9 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 24 Sep 2024 13:01:33 +0100 Subject: [PATCH 05/28] Mostly solving type errors around passing the view all the way down, got a fair few left. --- .../src/api/controllers/row/external.ts | 21 +++--- .../server/src/api/controllers/row/index.ts | 9 +-- .../src/api/controllers/row/utils/utils.ts | 16 ++--- packages/server/src/sdk/app/rows/external.ts | 65 +++++++++++++------ packages/server/src/sdk/app/rows/rows.ts | 13 ++-- .../src/sdk/app/rows/search/external.ts | 13 ++-- .../server/src/sdk/app/rows/search/utils.ts | 1 - packages/server/src/sdk/app/rows/utils.ts | 8 +-- .../src/utilities/rowProcessor/index.ts | 13 ++-- 9 files changed, 89 insertions(+), 70 deletions(-) diff --git a/packages/server/src/api/controllers/row/external.ts b/packages/server/src/api/controllers/row/external.ts index fb992059f4..2d4012dfcf 100644 --- a/packages/server/src/api/controllers/row/external.ts +++ b/packages/server/src/api/controllers/row/external.ts @@ -42,6 +42,7 @@ export async function handleRequest( export async function patch(ctx: UserCtx) { const source = await utils.getSource(ctx) + const table = await utils.getTableFromSource(source) const { _id, ...rowData } = ctx.request.body const { row: dataToUpdate } = await inputProcessing( @@ -58,11 +59,11 @@ export async function patch(ctx: UserCtx) { throw { validation: validateResult.errors } } - const beforeRow = await sdk.rows.external.getRow(tableId, _id, { + const beforeRow = await sdk.rows.external.getRow(table._id!, _id, { relationships: true, }) - const response = await handleRequest(Operation.UPDATE, tableId, { + const response = await handleRequest(Operation.UPDATE, source, { id: breakRowIdField(_id), row: dataToUpdate, }) @@ -70,7 +71,7 @@ export async function patch(ctx: UserCtx) { // The id might have been changed, so the refetching would fail. Recalculating the id just in case const updatedId = generateIdForRow({ ...beforeRow, ...dataToUpdate }, table) || _id - const row = await sdk.rows.external.getRow(tableId, updatedId, { + const row = await sdk.rows.external.getRow(table._id!, updatedId, { relationships: true, }) @@ -78,7 +79,6 @@ export async function patch(ctx: UserCtx) { outputProcessing(table, row, { squash: true, preserveLinks: true, - fromViewId: viewId, }), outputProcessing(table, beforeRow, { squash: true, @@ -95,9 +95,9 @@ export async function patch(ctx: UserCtx) { } export async function destroy(ctx: UserCtx) { - const { tableId } = utils.getSourceId(ctx) + const source = await utils.getSource(ctx) const _id = ctx.request.body._id - const { row } = await handleRequest(Operation.DELETE, tableId, { + const { row } = await handleRequest(Operation.DELETE, source, { id: breakRowIdField(_id), includeSqlRelationships: IncludeRelationship.EXCLUDE, }) @@ -106,11 +106,11 @@ export async function destroy(ctx: UserCtx) { export async function bulkDestroy(ctx: UserCtx) { const { rows } = ctx.request.body - const { tableId } = utils.getSourceId(ctx) + const source = await utils.getSource(ctx) let promises: Promise<{ row: Row; table: Table }>[] = [] for (let row of rows) { promises.push( - handleRequest(Operation.DELETE, tableId, { + handleRequest(Operation.DELETE, source, { id: breakRowIdField(row._id), includeSqlRelationships: IncludeRelationship.EXCLUDE, }) @@ -125,6 +125,7 @@ export async function bulkDestroy(ctx: UserCtx) { export async function fetchEnrichedRow(ctx: UserCtx) { const id = ctx.params.rowId + const source = await utils.getSource(ctx) const { tableId } = utils.getSourceId(ctx) const { datasourceId, tableName } = breakExternalTableId(tableId) const datasource: Datasource = await sdk.datasources.get(datasourceId) @@ -132,7 +133,7 @@ export async function fetchEnrichedRow(ctx: UserCtx) { ctx.throw(400, "Datasource has not been configured for plus API.") } const tables = datasource.entities - const response = await handleRequest(Operation.READ, tableId, { + const response = await handleRequest(Operation.READ, source, { id, datasource, includeSqlRelationships: IncludeRelationship.INCLUDE, @@ -156,7 +157,7 @@ export async function fetchEnrichedRow(ctx: UserCtx) { // don't support composite keys right now const linkedIds = links.map((link: Row) => breakRowIdField(link._id!)[0]) const primaryLink = linkedTable.primary?.[0] as string - const relatedRows = await handleRequest(Operation.READ, linkedTableId!, { + const relatedRows = await handleRequest(Operation.READ, linkedTable, { tables, filters: { oneOf: { diff --git a/packages/server/src/api/controllers/row/index.ts b/packages/server/src/api/controllers/row/index.ts index 2e5785157d..ac2c09e7a6 100644 --- a/packages/server/src/api/controllers/row/index.ts +++ b/packages/server/src/api/controllers/row/index.ts @@ -221,7 +221,7 @@ export async function search(ctx: Ctx) { const searchParams: RowSearchParams = { ...ctx.request.body, query: enrichedQuery, - tableId, + sourceId: tableId, } ctx.status = 200 @@ -229,14 +229,15 @@ export async function search(ctx: Ctx) { } export async function validate(ctx: Ctx) { - const { tableId } = utils.getSourceId(ctx) + const source = await utils.getSource(ctx) + const table = await utils.getTableFromSource(source) // external tables are hard to validate currently - if (isExternalTableID(tableId)) { + if (isExternalTableID(table._id!)) { ctx.body = { valid: true, errors: {} } } else { ctx.body = await sdk.rows.utils.validate({ row: ctx.request.body, - tableId, + source, }) } } diff --git a/packages/server/src/api/controllers/row/utils/utils.ts b/packages/server/src/api/controllers/row/utils/utils.ts index 91c0fc966f..e5397ed4a5 100644 --- a/packages/server/src/api/controllers/row/utils/utils.ts +++ b/packages/server/src/api/controllers/row/utils/utils.ts @@ -106,19 +106,11 @@ export async function getSource(ctx: Ctx): Promise
{ return sdk.tables.getTable(tableId) } -export async function validate( - opts: { row: Row } & ({ tableId: string } | { table: Table }) -) { - let fetchedTable: Table - if ("tableId" in opts) { - fetchedTable = await sdk.tables.getTable(opts.tableId) - } else { - fetchedTable = opts.table +export async function getTableFromSource(source: Table | ViewV2) { + if (sdk.views.isView(source)) { + return await sdk.views.getTable(source.id) } - return sdk.rows.utils.validate({ - ...opts, - table: fetchedTable, - }) + return source } function fixBooleanFields({ row, table }: { row: Row; table: Table }) { diff --git a/packages/server/src/sdk/app/rows/external.ts b/packages/server/src/sdk/app/rows/external.ts index 7630e5638b..24ec2302e7 100644 --- a/packages/server/src/sdk/app/rows/external.ts +++ b/packages/server/src/sdk/app/rows/external.ts @@ -1,5 +1,11 @@ -import { IncludeRelationship, Operation, Row } from "@budibase/types" -import { HTTPError } from "@budibase/backend-core" +import { + IncludeRelationship, + Operation, + Row, + Table, + ViewV2, +} from "@budibase/types" +import { docIds, HTTPError } from "@budibase/backend-core" import { handleRequest } from "../../../api/controllers/row/external" import { breakRowIdField } from "../../../integrations/utils" import sdk from "../../../sdk" @@ -12,11 +18,21 @@ import isEqual from "lodash/fp/isEqual" import { tryExtractingTableAndViewId } from "./utils" export async function getRow( - tableId: string, + sourceId: string | Table | ViewV2, rowId: string, opts?: { relationships?: boolean } ) { - const response = await handleRequest(Operation.READ, tableId, { + let source: Table | ViewV2 + if (typeof sourceId === "string") { + if (docIds.isViewId(sourceId)) { + source = await sdk.views.get(sourceId) + } else { + source = await sdk.tables.getTable(sourceId) + } + } else { + source = sourceId + } + const response = await handleRequest(Operation.READ, source, { id: breakRowIdField(rowId), includeSqlRelationships: opts?.relationships ? IncludeRelationship.INCLUDE @@ -27,45 +43,50 @@ export async function getRow( } export async function save( - tableOrViewId: string, + sourceId: string, inputs: Row, userId: string | undefined ) { - const { tableId, viewId } = tryExtractingTableAndViewId(tableOrViewId) - const table = await sdk.tables.getTable(tableId) + const { tableId, viewId } = tryExtractingTableAndViewId(sourceId) + let source: Table | ViewV2 + if (viewId) { + source = await sdk.views.get(viewId) + } else { + source = await sdk.tables.getTable(tableId) + } + const { table: updatedTable, row } = await inputProcessing( userId, - cloneDeep(table), + cloneDeep(source), inputs ) const validateResult = await sdk.rows.utils.validate({ row, - tableId, + source, }) if (!validateResult.valid) { throw { validation: validateResult.errors } } - const response = await handleRequest(Operation.CREATE, tableId, { + const response = await handleRequest(Operation.CREATE, source, { row, }) - if (!isEqual(table, updatedTable)) { + if (sdk.tables.isTable(source) && !isEqual(source, updatedTable)) { await sdk.tables.saveTable(updatedTable) } const rowId = response.row._id if (rowId) { - const row = await getRow(tableId, rowId, { + const row = await getRow(source, rowId, { relationships: true, }) return { ...response, - row: await outputProcessing(table, row, { + row: await outputProcessing(source, row, { preserveLinks: true, squash: true, - fromViewId: viewId, }), } } else { @@ -76,7 +97,14 @@ export async function save( export async function find(tableOrViewId: string, rowId: string): Promise { const { tableId, viewId } = tryExtractingTableAndViewId(tableOrViewId) - const row = await getRow(tableId, rowId, { + let source: Table | ViewV2 + if (viewId) { + source = await sdk.views.get(viewId) + } else { + source = await sdk.tables.getTable(tableId) + } + + const row = await getRow(source, rowId, { relationships: true, }) @@ -84,11 +112,10 @@ export async function find(tableOrViewId: string, rowId: string): Promise { throw new HTTPError("Row not found", 404) } - const table = await sdk.tables.getTable(tableId) - // Preserving links, as the outputProcessing does not support external rows yet and we don't need it in this use case - return await outputProcessing(table, row, { + // Preserving links, as the outputProcessing does not support external rows + // yet and we don't need it in this use case + return await outputProcessing(source, row, { squash: true, preserveLinks: true, - fromViewId: viewId, }) } diff --git a/packages/server/src/sdk/app/rows/rows.ts b/packages/server/src/sdk/app/rows/rows.ts index ef25d06baf..25bdf1fd4f 100644 --- a/packages/server/src/sdk/app/rows/rows.ts +++ b/packages/server/src/sdk/app/rows/rows.ts @@ -1,9 +1,6 @@ import { db as dbCore, context, docIds } from "@budibase/backend-core" import { Database, Row } from "@budibase/types" -import { - extractViewInfoFromID, - getRowParams, -} from "../../../db/utils" +import { extractViewInfoFromID, getRowParams } from "../../../db/utils" import { isExternalTableID } from "../../../integrations/utils" import * as internal from "./internal" import * as external from "./external" @@ -36,13 +33,13 @@ function pickApi(tableOrViewId: string) { } export async function save( - tableOrViewId: string, + sourceId: string, row: Row, userId: string | undefined ) { - return pickApi(tableOrViewId).save(tableOrViewId, row, userId) + return pickApi(sourceId).save(sourceId, row, userId) } -export async function find(tableOrViewId: string, rowId: string) { - return pickApi(tableOrViewId).find(tableOrViewId, rowId) +export async function find(sourceId: string, rowId: string) { + return pickApi(sourceId).find(sourceId, rowId) } diff --git a/packages/server/src/sdk/app/rows/search/external.ts b/packages/server/src/sdk/app/rows/search/external.ts index 5584b3f110..a41ae8dcda 100644 --- a/packages/server/src/sdk/app/rows/search/external.ts +++ b/packages/server/src/sdk/app/rows/search/external.ts @@ -106,9 +106,9 @@ export async function search( includeSqlRelationships: IncludeRelationship.INCLUDE, } const [{ rows, rawResponseSize }, totalRows] = await Promise.all([ - handleRequest(Operation.READ, tableId, parameters), + handleRequest(Operation.READ, source, parameters), countRows - ? handleRequest(Operation.COUNT, tableId, parameters) + ? handleRequest(Operation.COUNT, source, parameters) : Promise.resolve(undefined), ]) @@ -200,7 +200,7 @@ export async function exportRows( } let result = await search( - { tableId, query: requestQuery, sort, sortOrder }, + { sourceId: table._id!, query: requestQuery, sort, sortOrder }, table ) let rows: Row[] = [] @@ -256,10 +256,10 @@ export async function exportRows( } export async function fetch(tableId: string): Promise { - const response = await handleRequest(Operation.READ, tableId, { + const table = await sdk.tables.getTable(tableId) + const response = await handleRequest(Operation.READ, table, { includeSqlRelationships: IncludeRelationship.INCLUDE, }) - const table = await sdk.tables.getTable(tableId) return await outputProcessing(table, response.rows, { preserveLinks: true, squash: true, @@ -267,7 +267,8 @@ export async function fetch(tableId: string): Promise { } export async function fetchRaw(tableId: string): Promise { - const response = await handleRequest(Operation.READ, tableId, { + const table = await sdk.tables.getTable(tableId) + const response = await handleRequest(Operation.READ, table, { includeSqlRelationships: IncludeRelationship.INCLUDE, }) return response.rows diff --git a/packages/server/src/sdk/app/rows/search/utils.ts b/packages/server/src/sdk/app/rows/search/utils.ts index d727e58887..6548f963b8 100644 --- a/packages/server/src/sdk/app/rows/search/utils.ts +++ b/packages/server/src/sdk/app/rows/search/utils.ts @@ -9,7 +9,6 @@ import { SearchResponse, Row, RowSearchParams, - ViewV2, } from "@budibase/types" import { db as dbCore, context } from "@budibase/backend-core" import { utils } from "@budibase/shared-core" diff --git a/packages/server/src/sdk/app/rows/utils.ts b/packages/server/src/sdk/app/rows/utils.ts index 3899009f13..f17d3e4a03 100644 --- a/packages/server/src/sdk/app/rows/utils.ts +++ b/packages/server/src/sdk/app/rows/utils.ts @@ -21,6 +21,7 @@ import sdk from "../.." import { extractViewInfoFromID, isRelationshipColumn } from "../../../db/utils" import { isSQL } from "../../../integrations/utils" import { docIds } from "@budibase/backend-core" +import { getTableFromSource } from "../../../api/controllers/row/utils" const SQL_CLIENT_SOURCE_MAP: Record = { [SourceName.POSTGRES]: SqlClient.POSTGRES, @@ -149,12 +150,7 @@ export async function validate({ valid: boolean errors: Record }> { - let table: Table - if (sdk.views.isView(source)) { - table = await sdk.views.getTable(source.id) - } else { - table = source - } + const table = await getTableFromSource(source) const errors: Record = {} const disallowArrayTypes = [ FieldType.ATTACHMENT_SINGLE, diff --git a/packages/server/src/utilities/rowProcessor/index.ts b/packages/server/src/utilities/rowProcessor/index.ts index 73768fdd57..9aa53e18c1 100644 --- a/packages/server/src/utilities/rowProcessor/index.ts +++ b/packages/server/src/utilities/rowProcessor/index.ts @@ -19,6 +19,7 @@ import { RowAttachment, Table, User, + ViewV2, } from "@budibase/types" import { cloneDeep } from "lodash/fp" import { @@ -34,7 +35,11 @@ import { PROTECTED_INTERNAL_COLUMNS, } from "@budibase/shared-core" import { processString } from "@budibase/string-templates" -import { isUserMetadataTable } from "../../api/controllers/row/utils" +import { + getTableFromSource, + isUserMetadataTable, +} from "../../api/controllers/row/utils" +import sdk from "../../sdk" export * from "./utils" export * from "./attachments" @@ -170,11 +175,12 @@ export function coerce(row: any, type: string) { */ export async function inputProcessing( userId: string | null | undefined, - table: Table, + source: Table | ViewV2, row: Row, opts?: AutoColumnProcessingOpts ) { const clonedRow = cloneDeep(row) + const table = await getTableFromSource(source) const dontCleanseKeys = ["type", "_id", "_rev", "tableId"] for (const [key, value] of Object.entries(clonedRow)) { @@ -243,14 +249,13 @@ export async function inputProcessing( * @returns the enriched rows will be returned. */ export async function outputProcessing( - table: Table, + source: Table | ViewV2, rows: T, opts: { squash?: boolean preserveLinks?: boolean fromRow?: Row skipBBReferences?: boolean - fromViewId?: string aggregations?: Aggregation[] } = { squash: true, From 0eb90cfbea9e8dc16ecc917c1994382fdb901000 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 24 Sep 2024 16:35:53 +0100 Subject: [PATCH 06/28] Type checks pass, now to find out how much stuff I've broken. --- packages/backend-core/src/db/lucene.ts | 16 +++---- .../backend-core/src/db/tests/lucene.spec.ts | 4 +- .../src/api/controllers/row/external.ts | 2 +- .../src/api/controllers/row/internal.ts | 31 +++++------- .../src/api/controllers/row/staticFormula.ts | 32 +++++-------- .../src/api/controllers/row/utils/utils.ts | 18 +------ .../src/api/controllers/table/external.ts | 5 +- .../server/src/api/controllers/table/utils.ts | 3 +- .../src/api/routes/tests/search.spec.ts | 12 ++--- .../src/api/routes/tests/templates.spec.ts | 2 +- packages/server/src/db/linkedRows/index.ts | 26 +++++----- .../integrations/tests/googlesheets.spec.ts | 22 ++++----- packages/server/src/sdk/app/rows/external.ts | 11 +---- packages/server/src/sdk/app/rows/internal.ts | 47 ++++++++++--------- .../sdk/app/rows/search/internal/internal.ts | 2 +- .../sdk/app/rows/search/internal/lucene.ts | 20 +++++--- .../src/sdk/app/rows/search/internal/sqs.ts | 11 ++++- .../sdk/app/rows/search/tests/search.spec.ts | 10 ++-- .../sdk/app/rows/search/tests/utils.spec.ts | 12 ++--- .../src/sdk/app/rows/tests/utils.spec.ts | 42 ++++++++--------- packages/server/src/sdk/app/rows/utils.ts | 7 +++ packages/server/src/sdk/app/views/index.ts | 3 +- .../src/utilities/rowProcessor/index.ts | 23 +++++++-- .../tests/inputProcessing.spec.ts | 10 ++-- 24 files changed, 183 insertions(+), 188 deletions(-) diff --git a/packages/backend-core/src/db/lucene.ts b/packages/backend-core/src/db/lucene.ts index f5ad7e6433..b17c3ddf0d 100644 --- a/packages/backend-core/src/db/lucene.ts +++ b/packages/backend-core/src/db/lucene.ts @@ -79,8 +79,8 @@ export class QueryBuilder { return this } - setTable(tableId: string) { - this.#query.equal!.tableId = tableId + setSource(sourceId: string) { + this.#query.equal!.tableId = sourceId return this } @@ -638,8 +638,8 @@ async function recursiveSearch( .setSortOrder(params.sortOrder) .setSortType(params.sortType) - if (params.tableId) { - queryBuilder.setTable(params.tableId) + if (params.sourceId) { + queryBuilder.setSource(params.sourceId) } const page = await queryBuilder.run() @@ -672,8 +672,8 @@ export async function paginatedSearch( if (params.version) { search.setVersion(params.version) } - if (params.tableId) { - search.setTable(params.tableId) + if (params.sourceId) { + search.setSource(params.sourceId) } if (params.sort) { search @@ -695,8 +695,8 @@ export async function paginatedSearch( // Try fetching 1 row in the next page to see if another page of results // exists or not search.setBookmark(searchResults.bookmark).setLimit(1) - if (params.tableId) { - search.setTable(params.tableId) + if (params.sourceId) { + search.setSource(params.sourceId) } const nextResults = await search.run() diff --git a/packages/backend-core/src/db/tests/lucene.spec.ts b/packages/backend-core/src/db/tests/lucene.spec.ts index c41bdf88d1..8747f56a4b 100644 --- a/packages/backend-core/src/db/tests/lucene.spec.ts +++ b/packages/backend-core/src/db/tests/lucene.spec.ts @@ -366,7 +366,7 @@ describe("lucene", () => { }, }, { - tableId: TABLE_ID, + sourceId: TABLE_ID, limit: 1, sort: "property", sortType: SortType.STRING, @@ -390,7 +390,7 @@ describe("lucene", () => { }, }, { - tableId: TABLE_ID, + sourceId: TABLE_ID, query: {}, } ) diff --git a/packages/server/src/api/controllers/row/external.ts b/packages/server/src/api/controllers/row/external.ts index 2d4012dfcf..11b6559896 100644 --- a/packages/server/src/api/controllers/row/external.ts +++ b/packages/server/src/api/controllers/row/external.ts @@ -45,7 +45,7 @@ export async function patch(ctx: UserCtx) { const table = await utils.getTableFromSource(source) const { _id, ...rowData } = ctx.request.body - const { row: dataToUpdate } = await inputProcessing( + const dataToUpdate = await inputProcessing( ctx.user?._id, cloneDeep(source), rowData diff --git a/packages/server/src/api/controllers/row/internal.ts b/packages/server/src/api/controllers/row/internal.ts index 33e3c7707b..f5cb42f81d 100644 --- a/packages/server/src/api/controllers/row/internal.ts +++ b/packages/server/src/api/controllers/row/internal.ts @@ -21,18 +21,19 @@ import { import sdk from "../../../sdk" import { getLinkedTableIDs } from "../../../db/linkedRows/linkUtils" import { flatten } from "lodash" +import { findRow } from "../../../sdk/app/rows/internal" export async function patch(ctx: UserCtx) { - const { tableId, viewId } = utils.getSourceId(ctx) + const { tableId } = utils.getSourceId(ctx) + const source = await utils.getSource(ctx) + const table = sdk.views.isView(source) + ? await sdk.views.getTable(source.id) + : source const inputs = ctx.request.body const isUserTable = tableId === InternalTables.USER_METADATA let oldRow - const dbTable = await sdk.tables.getTable(tableId) try { - oldRow = await outputProcessing( - dbTable, - await utils.findRow(tableId, inputs._id!) - ) + oldRow = await outputProcessing(source, await findRow(tableId, inputs._id!)) } catch (err) { if (isUserTable) { // don't include the rev, it'll be the global rev @@ -48,22 +49,18 @@ export async function patch(ctx: UserCtx) { // need to build up full patch fields before coerce let combinedRow: any = cloneDeep(oldRow) for (let key of Object.keys(inputs)) { - if (!dbTable.schema[key]) continue + if (!table.schema[key]) continue combinedRow[key] = inputs[key] } // need to copy the table so it can be differenced on way out - const tableClone = cloneDeep(dbTable) + const tableClone = cloneDeep(table) // this returns the table and row incase they have been updated - let { table, row } = await inputProcessing( - ctx.user?._id, - tableClone, - combinedRow - ) + let row = await inputProcessing(ctx.user?._id, tableClone, combinedRow) const validateResult = await sdk.rows.utils.validate({ row, - table, + source, }) if (!validateResult.valid) { @@ -87,10 +84,8 @@ export async function patch(ctx: UserCtx) { return { row: ctx.body as Row, table, oldRow } } - const result = await finaliseRow(table, row, { - oldTable: dbTable, + const result = await finaliseRow(source, row, { updateFormula: true, - fromViewId: viewId, }) return { ...result, oldRow } @@ -186,7 +181,7 @@ export async function fetchEnrichedRow(ctx: UserCtx) { sdk.tables.getTable(tableId), linkRows.getLinkDocuments({ tableId, rowId, fieldName }), ]) - let row = await utils.findRow(tableId, rowId) + let row = await findRow(tableId, rowId) row = await outputProcessing(table, row) const linkVals = links as LinkDocumentValue[] diff --git a/packages/server/src/api/controllers/row/staticFormula.ts b/packages/server/src/api/controllers/row/staticFormula.ts index 777379db14..386dee7b4a 100644 --- a/packages/server/src/api/controllers/row/staticFormula.ts +++ b/packages/server/src/api/controllers/row/staticFormula.ts @@ -4,10 +4,11 @@ import { processFormulas, } from "../../../utilities/rowProcessor" import { context } from "@budibase/backend-core" -import { Table, Row, FormulaType, FieldType } from "@budibase/types" +import { Table, Row, FormulaType, FieldType, ViewV2 } from "@budibase/types" import * as linkRows from "../../../db/linkedRows" import isEqual from "lodash/isEqual" import { cloneDeep } from "lodash/fp" +import sdk from "../../../sdk" /** * This function runs through a list of enriched rows, looks at the rows which @@ -121,33 +122,26 @@ export async function updateAllFormulasInTable(table: Table) { * expects the row to be totally enriched/contain all relationships. */ export async function finaliseRow( - table: Table, + source: Table | ViewV2, row: Row, - { - oldTable, - updateFormula, - fromViewId, - }: { oldTable?: Table; updateFormula: boolean; fromViewId?: string } = { - updateFormula: true, - } + opts?: { updateFormula: boolean } ) { const db = context.getAppDB() + const { updateFormula = true } = opts || {} + const table = sdk.views.isView(source) + ? await sdk.views.getTable(source.id) + : source + row.type = "row" // process the row before return, to include relationships - let enrichedRow = (await outputProcessing(table, cloneDeep(row), { + let enrichedRow = await outputProcessing(source, cloneDeep(row), { squash: false, - })) as Row + }) // use enriched row to generate formulas for saving, specifically only use as context row = await processFormulas(table, row, { dynamic: false, contextRows: [enrichedRow], }) - // don't worry about rev, tables handle rev/lastID updates - // if another row has been written since processing this will - // handle the auto ID clash - if (oldTable && !isEqual(oldTable, table)) { - await db.put(table) - } const response = await db.put(row) // for response, calculate the formulas for the enriched row enrichedRow._rev = response.rev @@ -158,8 +152,6 @@ export async function finaliseRow( if (updateFormula) { await updateRelatedFormula(table, enrichedRow) } - const squashed = await linkRows.squashLinks(table, enrichedRow, { - fromViewId, - }) + const squashed = await linkRows.squashLinks(source, enrichedRow) return { row: enrichedRow, squashed, table } } diff --git a/packages/server/src/api/controllers/row/utils/utils.ts b/packages/server/src/api/controllers/row/utils/utils.ts index e5397ed4a5..0f565b6951 100644 --- a/packages/server/src/api/controllers/row/utils/utils.ts +++ b/packages/server/src/api/controllers/row/utils/utils.ts @@ -1,6 +1,6 @@ import * as utils from "../../../../db/utils" -import { context, docIds } from "@budibase/backend-core" +import { docIds } from "@budibase/backend-core" import { Aggregation, Ctx, @@ -20,7 +20,6 @@ import { basicProcessing, generateIdForRow, getInternalRowId } from "./basic" import sdk from "../../../../sdk" import { processStringSync } from "@budibase/string-templates" import validateJs from "validate.js" -import { getFullUser } from "../../../../utilities/users" validateJs.extend(validateJs.validators.datetime, { parse: function (value: string) { @@ -60,21 +59,6 @@ export async function processRelationshipFields( return row } -export async function findRow(tableId: string, rowId: string) { - const db = context.getAppDB() - let row: Row - // TODO remove special user case in future - if (tableId === utils.InternalTables.USER_METADATA) { - row = await getFullUser(rowId) - } else { - row = await db.get(rowId) - } - if (row.tableId !== tableId) { - throw "Supplied tableId does not match the rows tableId" - } - return row -} - export function getSourceId(ctx: Ctx): { tableId: string; viewId?: string } { // top priority, use the URL first if (ctx.params?.sourceId) { diff --git a/packages/server/src/api/controllers/table/external.ts b/packages/server/src/api/controllers/table/external.ts index c3356919c8..5b15d3d9c7 100644 --- a/packages/server/src/api/controllers/table/external.ts +++ b/packages/server/src/api/controllers/table/external.ts @@ -113,11 +113,10 @@ export async function bulkImport( const processed = await inputProcessing(ctx.user?._id, table, row, { noAutoRelationships: true, }) - parsedRows.push(processed.row) - table = processed.table + parsedRows.push(processed) } - await handleRequest(Operation.BULK_UPSERT, table._id!, { + await handleRequest(Operation.BULK_UPSERT, table, { rows: parsedRows, }) await events.rows.imported(table, parsedRows.length) diff --git a/packages/server/src/api/controllers/table/utils.ts b/packages/server/src/api/controllers/table/utils.ts index 269f079ae8..d568e5f33e 100644 --- a/packages/server/src/api/controllers/table/utils.ts +++ b/packages/server/src/api/controllers/table/utils.ts @@ -139,8 +139,7 @@ export async function importToRows( const processed = await inputProcessing(user?._id, table, row, { noAutoRelationships: true, }) - row = processed.row - table = processed.table + row = processed // However here we must reference the original table, as we want to mutate // the real schema of the table passed in, not the clone used for diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index c770c4e460..5788d9195a 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -157,7 +157,7 @@ describe.each([ if (isInMemory) { return dataFilters.search(_.cloneDeep(rows), this.query) } else { - return config.api.row.search(this.query.tableId, this.query) + return config.api.row.search(this.query.sourceId, this.query) } } @@ -327,8 +327,8 @@ describe.each([ } } - function expectSearch(query: Omit) { - return new SearchAssertion({ ...query, tableId: table._id! }) + function expectSearch(query: Omit) { + return new SearchAssertion({ ...query, sourceId: table._id! }) } function expectQuery(query: SearchFilters) { @@ -1898,7 +1898,7 @@ describe.each([ let { rows: fullRowList } = await config.api.row.search( table._id!, { - tableId: table._id!, + sourceId: table._id!, query: {}, } ) @@ -1909,7 +1909,7 @@ describe.each([ rowCount: number = 0 do { const response = await config.api.row.search(table._id!, { - tableId: table._id!, + sourceId: table._id!, limit: 1, paginate: true, query: {}, @@ -1933,7 +1933,7 @@ describe.each([ // eslint-disable-next-line no-constant-condition while (true) { const response = await config.api.row.search(table._id!, { - tableId: table._id!, + sourceId: table._id!, limit: 3, query: {}, bookmark, diff --git a/packages/server/src/api/routes/tests/templates.spec.ts b/packages/server/src/api/routes/tests/templates.spec.ts index 6f4d468a68..4290b4386f 100644 --- a/packages/server/src/api/routes/tests/templates.spec.ts +++ b/packages/server/src/api/routes/tests/templates.spec.ts @@ -113,7 +113,7 @@ describe("/templates", () => { expect(users.name).toBe("Users") const { rows } = await config.api.row.search(agencyProjects._id!, { - tableId: agencyProjects._id!, + sourceId: agencyProjects._id!, query: {}, }) diff --git a/packages/server/src/db/linkedRows/index.ts b/packages/server/src/db/linkedRows/index.ts index 4222f9b5e4..6e65ab36d1 100644 --- a/packages/server/src/db/linkedRows/index.ts +++ b/packages/server/src/db/linkedRows/index.ts @@ -255,31 +255,31 @@ export type SquashTableFields = Record * @returns The rows after having their links squashed to only contain the ID and primary display. */ export async function squashLinks( - table: Table, - enriched: T, - options?: { - fromViewId?: string - } + source: Table | ViewV2, + enriched: T ): Promise { const allowRelationshipSchemas = await features.flags.isEnabled( FeatureFlag.ENRICHED_RELATIONSHIPS ) let viewSchema: Record = {} - if (options?.fromViewId) { - const view = Object.values(table.views || {}).find( - (v): v is ViewV2 => sdk.views.isV2(v) && v.id === options?.fromViewId - ) - - if (view && helpers.views.isCalculationView(view)) { + if (sdk.views.isView(source)) { + if (helpers.views.isCalculationView(source)) { return enriched } - if (allowRelationshipSchemas && view) { - viewSchema = view.schema || {} + if (allowRelationshipSchemas) { + viewSchema = source.schema || {} } } + let table: Table + if (sdk.views.isView(source)) { + table = await sdk.views.getTable(source.id) + } else { + table = source + } + // will populate this as we find them const linkedTables = [table] const isArray = Array.isArray(enriched) diff --git a/packages/server/src/integrations/tests/googlesheets.spec.ts b/packages/server/src/integrations/tests/googlesheets.spec.ts index 34be1c0c6c..91addf8a50 100644 --- a/packages/server/src/integrations/tests/googlesheets.spec.ts +++ b/packages/server/src/integrations/tests/googlesheets.spec.ts @@ -219,7 +219,7 @@ describe("Google Sheets Integration", () => { }) let resp = await config.api.row.search(table._id!, { - tableId: table._id!, + sourceId: table._id!, query: {}, paginate: true, limit: 10, @@ -228,7 +228,7 @@ describe("Google Sheets Integration", () => { while (resp.hasNextPage) { resp = await config.api.row.search(table._id!, { - tableId: table._id!, + sourceId: table._id!, query: {}, paginate: true, limit: 10, @@ -637,7 +637,7 @@ describe("Google Sheets Integration", () => { it("should be able to find rows with equals filter", async () => { const response = await config.api.row.search(table._id!, { - tableId: table._id!, + sourceId: table._id!, query: { equal: { name: "Foo", @@ -651,7 +651,7 @@ describe("Google Sheets Integration", () => { it("should be able to find rows with not equals filter", async () => { const response = await config.api.row.search(table._id!, { - tableId: table._id!, + sourceId: table._id!, query: { notEqual: { name: "Foo", @@ -666,7 +666,7 @@ describe("Google Sheets Integration", () => { it("should be able to find rows with empty filter", async () => { const response = await config.api.row.search(table._id!, { - tableId: table._id!, + sourceId: table._id!, query: { empty: { name: null, @@ -679,7 +679,7 @@ describe("Google Sheets Integration", () => { it("should be able to find rows with not empty filter", async () => { const response = await config.api.row.search(table._id!, { - tableId: table._id!, + sourceId: table._id!, query: { notEmpty: { name: null, @@ -692,7 +692,7 @@ describe("Google Sheets Integration", () => { it("should be able to find rows with one of filter", async () => { const response = await config.api.row.search(table._id!, { - tableId: table._id!, + sourceId: table._id!, query: { oneOf: { name: ["Foo", "Bar"], @@ -707,7 +707,7 @@ describe("Google Sheets Integration", () => { it("should be able to find rows with fuzzy filter", async () => { const response = await config.api.row.search(table._id!, { - tableId: table._id!, + sourceId: table._id!, query: { fuzzy: { name: "oo", @@ -721,7 +721,7 @@ describe("Google Sheets Integration", () => { it("should be able to find rows with range filter", async () => { const response = await config.api.row.search(table._id!, { - tableId: table._id!, + sourceId: table._id!, query: { range: { name: { @@ -750,7 +750,7 @@ describe("Google Sheets Integration", () => { }) let response = await config.api.row.search(table._id!, { - tableId: table._id!, + sourceId: table._id!, query: { equal: { name: "Unique value!" } }, paginate: true, limit: 10, @@ -759,7 +759,7 @@ describe("Google Sheets Integration", () => { while (response.hasNextPage) { response = await config.api.row.search(table._id!, { - tableId: table._id!, + sourceId: table._id!, query: { equal: { name: "Unique value!" } }, paginate: true, limit: 10, diff --git a/packages/server/src/sdk/app/rows/external.ts b/packages/server/src/sdk/app/rows/external.ts index 24ec2302e7..060ef3738a 100644 --- a/packages/server/src/sdk/app/rows/external.ts +++ b/packages/server/src/sdk/app/rows/external.ts @@ -14,7 +14,6 @@ import { outputProcessing, } from "../../../utilities/rowProcessor" import cloneDeep from "lodash/fp/cloneDeep" -import isEqual from "lodash/fp/isEqual" import { tryExtractingTableAndViewId } from "./utils" export async function getRow( @@ -55,11 +54,7 @@ export async function save( source = await sdk.tables.getTable(tableId) } - const { table: updatedTable, row } = await inputProcessing( - userId, - cloneDeep(source), - inputs - ) + const row = await inputProcessing(userId, cloneDeep(source), inputs) const validateResult = await sdk.rows.utils.validate({ row, @@ -73,10 +68,6 @@ export async function save( row, }) - if (sdk.tables.isTable(source) && !isEqual(source, updatedTable)) { - await sdk.tables.saveTable(updatedTable) - } - const rowId = response.row._id if (rowId) { const row = await getRow(source, rowId, { diff --git a/packages/server/src/sdk/app/rows/internal.ts b/packages/server/src/sdk/app/rows/internal.ts index b5b3437e5e..f51bcc37a4 100644 --- a/packages/server/src/sdk/app/rows/internal.ts +++ b/packages/server/src/sdk/app/rows/internal.ts @@ -1,5 +1,5 @@ import { context, db } from "@budibase/backend-core" -import { Row } from "@budibase/types" +import { Row, Table, ViewV2 } from "@budibase/types" import sdk from "../../../sdk" import cloneDeep from "lodash/fp/cloneDeep" import { finaliseRow } from "../../../api/controllers/row/staticFormula" @@ -10,7 +10,7 @@ import { import * as linkRows from "../../../db/linkedRows" import { InternalTables } from "../../../db/utils" import { getFullUser } from "../../../utilities/users" -import { tryExtractingTableAndViewId } from "./utils" +import { getSource, tryExtractingTableAndViewId } from "./utils" export async function save( tableOrViewId: string, @@ -20,21 +20,28 @@ export async function save( const { tableId, viewId } = tryExtractingTableAndViewId(tableOrViewId) inputs.tableId = tableId + let source: Table | ViewV2 + let table: Table + if (viewId) { + source = await sdk.views.get(viewId) + table = await sdk.views.getTable(viewId) + } else { + source = await sdk.tables.getTable(tableId) + table = source + } + if (!inputs._rev && !inputs._id) { inputs._id = db.generateRowID(inputs.tableId) } - // this returns the table and row incase they have been updated - const dbTable = await sdk.tables.getTable(inputs.tableId) - // need to copy the table so it can be differenced on way out - const tableClone = cloneDeep(dbTable) + const sourceClone = cloneDeep(source) - let { table, row } = await inputProcessing(userId, tableClone, inputs) + let row = await inputProcessing(userId, sourceClone, inputs) const validateResult = await sdk.rows.utils.validate({ row, - table, + source, }) if (!validateResult.valid) { @@ -49,24 +56,18 @@ export async function save( table, })) as Row - return finaliseRow(table, row, { - oldTable: dbTable, - updateFormula: true, - fromViewId: viewId, + return finaliseRow(table, row, { updateFormula: true }) +} + +export async function find(sourceId: string, rowId: string): Promise { + const source = await getSource(sourceId) + return await outputProcessing(source, await findRow(sourceId, rowId), { + squash: true, }) } -export async function find(tableOrViewId: string, rowId: string): Promise { - const { tableId, viewId } = tryExtractingTableAndViewId(tableOrViewId) - - const table = await sdk.tables.getTable(tableId) - let row = await findRow(tableId, rowId) - - row = await outputProcessing(table, row, { squash: true, fromViewId: viewId }) - return row -} - -async function findRow(tableId: string, rowId: string) { +export async function findRow(sourceId: string, rowId: string) { + const { tableId } = tryExtractingTableAndViewId(sourceId) const db = context.getAppDB() let row: Row // TODO remove special user case in future diff --git a/packages/server/src/sdk/app/rows/search/internal/internal.ts b/packages/server/src/sdk/app/rows/search/internal/internal.ts index 6617fc376c..c9e2aba237 100644 --- a/packages/server/src/sdk/app/rows/search/internal/internal.ts +++ b/packages/server/src/sdk/app/rows/search/internal/internal.ts @@ -64,7 +64,7 @@ export async function exportRows( result = await outputProcessing(table, response) } else if (query) { let searchResponse = await sdk.rows.search({ - tableId, + sourceId: tableId, query, sort, sortOrder, diff --git a/packages/server/src/sdk/app/rows/search/internal/lucene.ts b/packages/server/src/sdk/app/rows/search/internal/lucene.ts index 2c149e5b21..24a7de1307 100644 --- a/packages/server/src/sdk/app/rows/search/internal/lucene.ts +++ b/packages/server/src/sdk/app/rows/search/internal/lucene.ts @@ -8,21 +8,30 @@ import { SortType, Table, User, + ViewV2, } from "@budibase/types" import { getGlobalUsersFromMetadata } from "../../../../../utilities/global" import { outputProcessing } from "../../../../../utilities/rowProcessor" import pick from "lodash/pick" +import sdk from "../../../../" export async function search( options: RowSearchParams, - table: Table + source: Table | ViewV2 ): Promise> { - const { tableId } = options + const { sourceId } = options + + let table: Table + if (sdk.views.isView(source)) { + table = await sdk.views.getTable(source.id) + } else { + table = source + } const { paginate, query } = options const params: RowSearchParams = { - tableId: options.tableId, + sourceId: options.sourceId, sort: options.sort, sortOrder: options.sortOrder, sortType: options.sortType, @@ -50,7 +59,7 @@ export async function search( // Enrich search results with relationships if (response.rows && response.rows.length) { // enrich with global users if from users table - if (tableId === InternalTables.USER_METADATA) { + if (sourceId === InternalTables.USER_METADATA) { response.rows = await getGlobalUsersFromMetadata(response.rows as User[]) } @@ -59,9 +68,8 @@ export async function search( response.rows = response.rows.map((r: any) => pick(r, fields)) } - response.rows = await outputProcessing(table, response.rows, { + response.rows = await outputProcessing(source, response.rows, { squash: true, - fromViewId: options.viewId, }) } diff --git a/packages/server/src/sdk/app/rows/search/internal/sqs.ts b/packages/server/src/sdk/app/rows/search/internal/sqs.ts index b5bf8e752f..90fb082214 100644 --- a/packages/server/src/sdk/app/rows/search/internal/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/internal/sqs.ts @@ -15,6 +15,7 @@ import { SortType, SqlClient, Table, + ViewV2, } from "@budibase/types" import { buildInternalRelationships, @@ -292,11 +293,18 @@ function resyncDefinitionsRequired(status: number, message: string) { export async function search( options: RowSearchParams, - table: Table, + source: Table | ViewV2, opts?: { retrying?: boolean } ): Promise> { let { paginate, query, ...params } = cloneDeep(options) + let table: Table + if (sdk.views.isView(source)) { + table = await sdk.views.getTable(source.id) + } else { + table = source + } + const allTables = await sdk.tables.getAllInternalTables() const allTablesMap = buildTableMap(allTables) // make sure we have the mapped/latest table @@ -406,7 +414,6 @@ export async function search( let finalRows = await outputProcessing(table, processed, { preserveLinks: true, squash: true, - fromViewId: options.viewId, aggregations: options.aggregations, }) diff --git a/packages/server/src/sdk/app/rows/search/tests/search.spec.ts b/packages/server/src/sdk/app/rows/search/tests/search.spec.ts index e7fd095865..194f2dd4e3 100644 --- a/packages/server/src/sdk/app/rows/search/tests/search.spec.ts +++ b/packages/server/src/sdk/app/rows/search/tests/search.spec.ts @@ -122,7 +122,7 @@ describe.each([ it("querying by fields will always return data attribute columns", async () => { await config.doInContext(config.appId, async () => { const { rows } = await search({ - tableId: table._id!, + sourceId: table._id!, query: {}, fields: ["name", "age"], }) @@ -142,7 +142,7 @@ describe.each([ it("will decode _id in oneOf query", async () => { await config.doInContext(config.appId, async () => { const result = await search({ - tableId: table._id!, + sourceId: table._id!, query: { oneOf: { _id: ["%5B1%5D", "%5B4%5D", "%5B8%5D"], @@ -174,7 +174,7 @@ describe.each([ }, }) const result = await search({ - tableId: table._id!, + sourceId: table._id!, query: {}, }) expect(result.rows).toHaveLength(10) @@ -205,7 +205,7 @@ describe.each([ }, }) const result = await search({ - tableId: table._id!, + sourceId: table._id!, query: {}, fields: ["name", "age"], }) @@ -229,7 +229,7 @@ describe.each([ async (queryFields, expectedRows) => { await config.doInContext(config.appId, async () => { const { rows } = await search({ - tableId: table._id!, + sourceId: table._id!, query: { $or: { conditions: [ diff --git a/packages/server/src/sdk/app/rows/search/tests/utils.spec.ts b/packages/server/src/sdk/app/rows/search/tests/utils.spec.ts index 0698f727df..e3f241f15a 100644 --- a/packages/server/src/sdk/app/rows/search/tests/utils.spec.ts +++ b/packages/server/src/sdk/app/rows/search/tests/utils.spec.ts @@ -48,7 +48,7 @@ describe.each([tableWithUserCol, tableWithUsersCol])( it("should be able to map ro_ to global user IDs", () => { const params: RowSearchParams = { - tableId, + sourceId: tableId, query: { equal: { "1:user": userMedataId, @@ -61,7 +61,7 @@ describe.each([tableWithUserCol, tableWithUsersCol])( it("should handle array of user IDs", () => { const params: RowSearchParams = { - tableId, + sourceId: tableId, query: { oneOf: { "1:user": [userMedataId, globalUserId], @@ -78,7 +78,7 @@ describe.each([tableWithUserCol, tableWithUsersCol])( it("shouldn't change any other input", () => { const email = "test@example.com" const params: RowSearchParams = { - tableId, + sourceId: tableId, query: { equal: { "1:user": email, @@ -90,10 +90,8 @@ describe.each([tableWithUserCol, tableWithUsersCol])( }) it("shouldn't error if no query supplied", () => { - const params: any = { - tableId, - } - const output = searchInputMapping(col, params) + // @ts-expect-error - intentionally passing in a bad type + const output = searchInputMapping(col, { sourceId: tableId }) expect(output.query).toBeUndefined() }) } diff --git a/packages/server/src/sdk/app/rows/tests/utils.spec.ts b/packages/server/src/sdk/app/rows/tests/utils.spec.ts index 55cdf9ea20..548b2b6bc9 100644 --- a/packages/server/src/sdk/app/rows/tests/utils.spec.ts +++ b/packages/server/src/sdk/app/rows/tests/utils.spec.ts @@ -33,7 +33,7 @@ describe("validate", () => { it("should accept empty values", async () => { const row = {} const table = getTable() - const output = await validate({ table, tableId: table._id!, row }) + const output = await validate({ source: table, row }) expect(output.valid).toBe(true) expect(output.errors).toEqual({}) }) @@ -43,7 +43,7 @@ describe("validate", () => { time: `${hour()}:${minute()}`, } const table = getTable() - const output = await validate({ table, tableId: table._id!, row }) + const output = await validate({ source: table, row }) expect(output.valid).toBe(true) }) @@ -52,7 +52,7 @@ describe("validate", () => { time: `${hour()}:${minute()}:${second()}`, } const table = getTable() - const output = await validate({ table, tableId: table._id!, row }) + const output = await validate({ source: table, row }) expect(output.valid).toBe(true) }) @@ -67,7 +67,7 @@ describe("validate", () => { table.schema.time.constraints = { presence: true, } - const output = await validate({ table, tableId: table._id!, row }) + const output = await validate({ source: table, row }) expect(output.valid).toBe(false) expect(output.errors).toEqual({ time: ['"time" is not a valid time'] }) }) @@ -91,7 +91,7 @@ describe("validate", () => { `${generator.integer({ min: 11, max: 23 })}:${minute()}`, ])("should accept values after config value (%s)", async time => { const row = { time } - const output = await validate({ table, tableId: table._id!, row }) + const output = await validate({ source: table, row }) expect(output.valid).toBe(true) }) @@ -100,7 +100,7 @@ describe("validate", () => { `${generator.integer({ min: 0, max: 9 })}:${minute()}`, ])("should reject values before config value (%s)", async time => { const row = { time } - const output = await validate({ table, tableId: table._id!, row }) + const output = await validate({ source: table, row }) expect(output.valid).toBe(false) expect(output.errors).toEqual({ time: ["must be no earlier than 10:00"], @@ -125,7 +125,7 @@ describe("validate", () => { `${generator.integer({ min: 0, max: 12 })}:${minute()}`, ])("should accept values before config value (%s)", async time => { const row = { time } - const output = await validate({ table, tableId: table._id!, row }) + const output = await validate({ source: table, row }) expect(output.valid).toBe(true) }) @@ -134,7 +134,7 @@ describe("validate", () => { `${generator.integer({ min: 16, max: 23 })}:${minute()}`, ])("should reject values after config value (%s)", async time => { const row = { time } - const output = await validate({ table, tableId: table._id!, row }) + const output = await validate({ source: table, row }) expect(output.valid).toBe(false) expect(output.errors).toEqual({ time: ["must be no later than 15:16:17"], @@ -156,7 +156,7 @@ describe("validate", () => { "should accept values in range (%s)", async time => { const row = { time } - const output = await validate({ table, tableId: table._id!, row }) + const output = await validate({ source: table, row }) expect(output.valid).toBe(true) } ) @@ -166,7 +166,7 @@ describe("validate", () => { `${generator.integer({ min: 0, max: 9 })}:${minute()}`, ])("should reject values before range (%s)", async time => { const row = { time } - const output = await validate({ table, tableId: table._id!, row }) + const output = await validate({ source: table, row }) expect(output.valid).toBe(false) expect(output.errors).toEqual({ time: ["must be no earlier than 10:00"], @@ -178,7 +178,7 @@ describe("validate", () => { `${generator.integer({ min: 16, max: 23 })}:${minute()}`, ])("should reject values after range (%s)", async time => { const row = { time } - const output = await validate({ table, tableId: table._id!, row }) + const output = await validate({ source: table, row }) expect(output.valid).toBe(false) expect(output.errors).toEqual({ time: ["must be no later than 15:00"], @@ -199,7 +199,7 @@ describe("validate", () => { "should accept values in range (%s)", async time => { const row = { time } - const output = await validate({ table, tableId: table._id!, row }) + const output = await validate({ source: table, row }) expect(output.valid).toBe(true) } ) @@ -208,7 +208,7 @@ describe("validate", () => { "should reject values out range (%s)", async time => { const row = { time } - const output = await validate({ table, tableId: table._id!, row }) + const output = await validate({ source: table, row }) expect(output.valid).toBe(false) expect(output.errors).toEqual({ time: ["must be no later than 10:00"], @@ -226,7 +226,7 @@ describe("validate", () => { table.schema.time.constraints = { presence: true, } - const output = await validate({ table, tableId: table._id!, row }) + const output = await validate({ source: table, row }) expect(output.valid).toBe(false) expect(output.errors).toEqual({ time: ["can't be blank"] }) }) @@ -237,7 +237,7 @@ describe("validate", () => { table.schema.time.constraints = { presence: true, } - const output = await validate({ table, tableId: table._id!, row }) + const output = await validate({ source: table, row }) expect(output.valid).toBe(false) expect(output.errors).toEqual({ time: ["can't be blank"] }) }) @@ -257,7 +257,7 @@ describe("validate", () => { "should accept values in range (%s)", async time => { const row = { time } - const output = await validate({ table, tableId: table._id!, row }) + const output = await validate({ source: table, row }) expect(output.valid).toBe(true) } ) @@ -267,7 +267,7 @@ describe("validate", () => { `${generator.integer({ min: 0, max: 9 })}:${minute()}`, ])("should reject values before range (%s)", async time => { const row = { time } - const output = await validate({ table, tableId: table._id!, row }) + const output = await validate({ source: table, row }) expect(output.valid).toBe(false) expect(output.errors).toEqual({ time: ["must be no earlier than 10:00"], @@ -279,7 +279,7 @@ describe("validate", () => { `${generator.integer({ min: 16, max: 23 })}:${minute()}`, ])("should reject values after range (%s)", async time => { const row = { time } - const output = await validate({ table, tableId: table._id!, row }) + const output = await validate({ source: table, row }) expect(output.valid).toBe(false) expect(output.errors).toEqual({ time: ["must be no later than 15:00"], @@ -301,7 +301,7 @@ describe("validate", () => { "should accept values in range (%s)", async time => { const row = { time } - const output = await validate({ table, tableId: table._id!, row }) + const output = await validate({ source: table, row }) expect(output.valid).toBe(true) } ) @@ -311,7 +311,7 @@ describe("validate", () => { `${generator.integer({ min: 0, max: 9 })}:${minute()}`, ])("should reject values before range (%s)", async time => { const row = { time } - const output = await validate({ table, tableId: table._id!, row }) + const output = await validate({ source: table, row }) expect(output.valid).toBe(false) expect(output.errors).toEqual({ time: ["must be no earlier than 10:00"], @@ -323,7 +323,7 @@ describe("validate", () => { `${generator.integer({ min: 16, max: 23 })}:${minute()}`, ])("should reject values after range (%s)", async time => { const row = { time } - const output = await validate({ table, tableId: table._id!, row }) + const output = await validate({ source: table, row }) expect(output.valid).toBe(false) expect(output.errors).toEqual({ time: ["must be no later than 15:00"], diff --git a/packages/server/src/sdk/app/rows/utils.ts b/packages/server/src/sdk/app/rows/utils.ts index f17d3e4a03..d5c0560d9b 100644 --- a/packages/server/src/sdk/app/rows/utils.ts +++ b/packages/server/src/sdk/app/rows/utils.ts @@ -321,3 +321,10 @@ export function tryExtractingTableAndViewId(tableOrViewId: string) { return { tableId: tableOrViewId } } + +export function getSource(tableOrViewId: string) { + if (docIds.isViewId(tableOrViewId)) { + return sdk.views.get(tableOrViewId) + } + return sdk.tables.getTable(tableOrViewId) +} diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index f911385dc1..4251383712 100644 --- a/packages/server/src/sdk/app/views/index.ts +++ b/packages/server/src/sdk/app/views/index.ts @@ -40,7 +40,8 @@ export async function getEnriched(viewId: string): Promise { return pickApi(tableId).getEnriched(viewId) } -export async function getTable(viewId: string): Promise
{ +export async function getTable(view: string | ViewV2): Promise
{ + const viewId = typeof view === "string" ? view : view.id const cached = context.getTableForView(viewId) if (cached) { return cached diff --git a/packages/server/src/utilities/rowProcessor/index.ts b/packages/server/src/utilities/rowProcessor/index.ts index 9aa53e18c1..24c8d11bd1 100644 --- a/packages/server/src/utilities/rowProcessor/index.ts +++ b/packages/server/src/utilities/rowProcessor/index.ts @@ -73,6 +73,7 @@ export async function processAutoColumn( // check its not user table, or whether any of the processing options have been disabled const shouldUpdateUserFields = !isUserTable && !opts?.reprocessing && !opts?.noAutoRelationships && !noUser + let tableMutated = false for (let [key, schema] of Object.entries(table.schema)) { if (!schema.autocolumn) { continue @@ -105,10 +106,17 @@ export async function processAutoColumn( row[key] = schema.lastID + 1 schema.lastID++ table.schema[key] = schema + tableMutated = true } break } } + + if (tableMutated) { + const db = context.getAppDB() + const resp = await db.put(table) + table._rev = resp.rev + } } async function processDefaultValues(table: Table, row: Row) { @@ -235,8 +243,7 @@ export async function inputProcessing( await processAutoColumn(userId, table, clonedRow, opts) await processDefaultValues(table, clonedRow) - - return { table, row: clonedRow } + return clonedRow } /** @@ -271,6 +278,14 @@ export async function outputProcessing( } else { safeRows = rows } + + let table: Table + if (sdk.views.isView(source)) { + table = await sdk.views.getTable(source.id) + } else { + table = source + } + // attach any linked row information let enriched = !opts.preserveLinks ? await linkRows.attachFullLinkedDocs(table.schema, safeRows, { @@ -360,9 +375,7 @@ export async function outputProcessing( enriched = await processFormulas(table, enriched, { dynamic: true }) if (opts.squash) { - enriched = await linkRows.squashLinks(table, enriched, { - fromViewId: opts?.fromViewId, - }) + enriched = await linkRows.squashLinks(source, enriched) } // remove null properties to match internal API diff --git a/packages/server/src/utilities/rowProcessor/tests/inputProcessing.spec.ts b/packages/server/src/utilities/rowProcessor/tests/inputProcessing.spec.ts index 244ea3794c..1a75cd6830 100644 --- a/packages/server/src/utilities/rowProcessor/tests/inputProcessing.spec.ts +++ b/packages/server/src/utilities/rowProcessor/tests/inputProcessing.spec.ts @@ -65,7 +65,7 @@ describe("rowProcessor - inputProcessing", () => { processInputBBReferenceMock.mockResolvedValue(user) - const { row } = await inputProcessing(userId, table, newRow) + const row = await inputProcessing(userId, table, newRow) expect(bbReferenceProcessor.processInputBBReference).toHaveBeenCalledTimes( 1 @@ -117,7 +117,7 @@ describe("rowProcessor - inputProcessing", () => { processInputBBReferencesMock.mockResolvedValue(user) - const { row } = await inputProcessing(userId, table, newRow) + const row = await inputProcessing(userId, table, newRow) expect(bbReferenceProcessor.processInputBBReferences).toHaveBeenCalledTimes( 1 @@ -164,7 +164,7 @@ describe("rowProcessor - inputProcessing", () => { name: "Jack", } - const { row } = await inputProcessing(userId, table, newRow) + const row = await inputProcessing(userId, table, newRow) expect(bbReferenceProcessor.processInputBBReferences).not.toHaveBeenCalled() expect(row).toEqual({ ...newRow, user: undefined }) @@ -207,7 +207,7 @@ describe("rowProcessor - inputProcessing", () => { user: userValue, } - const { row } = await inputProcessing(userId, table, newRow) + const row = await inputProcessing(userId, table, newRow) if (userValue === undefined) { // The 'user' field is omitted @@ -262,7 +262,7 @@ describe("rowProcessor - inputProcessing", () => { user: "123", } - const { row } = await inputProcessing(userId, table, newRow) + const row = await inputProcessing(userId, table, newRow) expect(bbReferenceProcessor.processInputBBReferences).not.toHaveBeenCalled() expect(row).toEqual({ From e3256cb005ec6f8517e5db995f8c555ba00f7b38 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 24 Sep 2024 17:46:38 +0100 Subject: [PATCH 07/28] Fix row.spec.ts. --- packages/backend-core/src/db/lucene.ts | 1 - .../src/api/controllers/row/external.ts | 4 ++-- .../server/src/api/controllers/row/index.ts | 4 ++-- .../src/api/controllers/row/utils/utils.ts | 8 +++++++- .../server/src/api/controllers/table/index.ts | 9 ++------- .../server/src/api/routes/tests/row.spec.ts | 12 ++++++++--- packages/server/src/db/linkedRows/index.ts | 7 ++++--- packages/server/src/sdk/app/rows/internal.ts | 2 +- .../sdk/app/rows/search/internal/lucene.ts | 2 +- .../src/sdk/app/rows/search/internal/sqs.ts | 20 ++++++++----------- 10 files changed, 36 insertions(+), 33 deletions(-) diff --git a/packages/backend-core/src/db/lucene.ts b/packages/backend-core/src/db/lucene.ts index b17c3ddf0d..7f58c7068e 100644 --- a/packages/backend-core/src/db/lucene.ts +++ b/packages/backend-core/src/db/lucene.ts @@ -612,7 +612,6 @@ async function runQuery( * limit {number} The number of results to fetch * bookmark {string|null} Current bookmark in the recursive search * rows {array|null} Current results in the recursive search - * @returns {Promise<*[]|*>} */ async function recursiveSearch( dbName: string, diff --git a/packages/server/src/api/controllers/row/external.ts b/packages/server/src/api/controllers/row/external.ts index 11b6559896..18a9be5087 100644 --- a/packages/server/src/api/controllers/row/external.ts +++ b/packages/server/src/api/controllers/row/external.ts @@ -76,11 +76,11 @@ export async function patch(ctx: UserCtx) { }) const [enrichedRow, oldRow] = await Promise.all([ - outputProcessing(table, row, { + outputProcessing(source, row, { squash: true, preserveLinks: true, }), - outputProcessing(table, beforeRow, { + outputProcessing(source, beforeRow, { squash: true, preserveLinks: true, }), diff --git a/packages/server/src/api/controllers/row/index.ts b/packages/server/src/api/controllers/row/index.ts index ac2c09e7a6..84b5edcc12 100644 --- a/packages/server/src/api/controllers/row/index.ts +++ b/packages/server/src/api/controllers/row/index.ts @@ -207,7 +207,7 @@ export async function destroy(ctx: UserCtx) { } export async function search(ctx: Ctx) { - const { tableId } = utils.getSourceId(ctx) + const { tableId, viewId } = utils.getSourceId(ctx) await context.ensureSnippetContext(true) @@ -221,7 +221,7 @@ export async function search(ctx: Ctx) { const searchParams: RowSearchParams = { ...ctx.request.body, query: enrichedQuery, - sourceId: tableId, + sourceId: viewId || tableId, } ctx.status = 200 diff --git a/packages/server/src/api/controllers/row/utils/utils.ts b/packages/server/src/api/controllers/row/utils/utils.ts index 0f565b6951..45abd93930 100644 --- a/packages/server/src/api/controllers/row/utils/utils.ts +++ b/packages/server/src/api/controllers/row/utils/utils.ts @@ -112,7 +112,7 @@ function fixBooleanFields({ row, table }: { row: Row; table: Table }) { export async function sqlOutputProcessing( rows: DatasourcePlusQueryResponse, - table: Table, + source: Table | ViewV2, tables: Record, relationships: RelationshipsJson[], opts?: { sqs?: boolean; aggregations?: Aggregation[] } @@ -120,6 +120,12 @@ export async function sqlOutputProcessing( if (isKnexEmptyReadResponse(rows)) { return [] } + let table: Table + if (sdk.views.isView(source)) { + table = await sdk.views.getTable(source.id) + } else { + table = source + } let finalRows: { [key: string]: Row } = {} for (let row of rows as Row[]) { let rowId = row._id diff --git a/packages/server/src/api/controllers/table/index.ts b/packages/server/src/api/controllers/table/index.ts index 0e16077092..7bec1581b4 100644 --- a/packages/server/src/api/controllers/table/index.ts +++ b/packages/server/src/api/controllers/table/index.ts @@ -33,7 +33,7 @@ import { import sdk from "../../../sdk" import { jsonFromCsvString } from "../../../utilities/csv" import { builderSocket } from "../../../websockets" -import { cloneDeep, isEqual } from "lodash" +import { cloneDeep } from "lodash" import { helpers, PROTECTED_EXTERNAL_COLUMNS, @@ -149,12 +149,7 @@ export async function bulkImport( ctx: UserCtx ) { const tableId = ctx.params.tableId - let tableBefore = await sdk.tables.getTable(tableId) - let tableAfter = await pickApi({ tableId }).bulkImport(ctx) - - if (!isEqual(tableBefore, tableAfter)) { - await sdk.tables.saveTable(tableAfter) - } + await pickApi({ tableId }).bulkImport(ctx) // right now we don't trigger anything for bulk import because it // can only be done in the builder, but in the future we may need to diff --git a/packages/server/src/api/routes/tests/row.spec.ts b/packages/server/src/api/routes/tests/row.spec.ts index dc03a21d6d..467faa8e06 100644 --- a/packages/server/src/api/routes/tests/row.spec.ts +++ b/packages/server/src/api/routes/tests/row.spec.ts @@ -76,7 +76,7 @@ async function waitForEvent( } describe.each([ - ["internal", undefined], + ["lucene", undefined], ["sqs", undefined], [DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)], [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)], @@ -2453,9 +2453,15 @@ describe.each([ let flagCleanup: (() => void) | undefined beforeAll(async () => { - flagCleanup = setCoreEnv({ + const env = { TENANT_FEATURE_FLAGS: `*:${FeatureFlag.ENRICHED_RELATIONSHIPS}`, - }) + } + if (isSqs) { + env.TENANT_FEATURE_FLAGS = `${env.TENANT_FEATURE_FLAGS},*:SQS` + } else { + env.TENANT_FEATURE_FLAGS = `${env.TENANT_FEATURE_FLAGS},*:!SQS` + } + flagCleanup = setCoreEnv(env) const aux2Table = await config.api.table.save(saveTableRequest()) const aux2Data = await config.api.row.save(aux2Table._id!, {}) diff --git a/packages/server/src/db/linkedRows/index.ts b/packages/server/src/db/linkedRows/index.ts index 6e65ab36d1..bc37fbef42 100644 --- a/packages/server/src/db/linkedRows/index.ts +++ b/packages/server/src/db/linkedRows/index.ts @@ -248,10 +248,11 @@ function getPrimaryDisplayValue(row: Row, table?: Table) { export type SquashTableFields = Record /** - * This function will take the given enriched rows and squash the links to only contain the primary display field. - * @param table The table from which the rows originated. + * This function will take the given enriched rows and squash the links to only + * contain the primary display field. + * + * @param source The table or view from which the rows originated. * @param enriched The pre-enriched rows (full docs) which are to be squashed. - * @param squashFields Per link column (key) define which columns are allowed while squashing. * @returns The rows after having their links squashed to only contain the ID and primary display. */ export async function squashLinks( diff --git a/packages/server/src/sdk/app/rows/internal.ts b/packages/server/src/sdk/app/rows/internal.ts index f51bcc37a4..13054f43f4 100644 --- a/packages/server/src/sdk/app/rows/internal.ts +++ b/packages/server/src/sdk/app/rows/internal.ts @@ -56,7 +56,7 @@ export async function save( table, })) as Row - return finaliseRow(table, row, { updateFormula: true }) + return finaliseRow(source, row, { updateFormula: true }) } export async function find(sourceId: string, rowId: string): Promise { diff --git a/packages/server/src/sdk/app/rows/search/internal/lucene.ts b/packages/server/src/sdk/app/rows/search/internal/lucene.ts index 24a7de1307..65cf4053d7 100644 --- a/packages/server/src/sdk/app/rows/search/internal/lucene.ts +++ b/packages/server/src/sdk/app/rows/search/internal/lucene.ts @@ -31,7 +31,7 @@ export async function search( const { paginate, query } = options const params: RowSearchParams = { - sourceId: options.sourceId, + sourceId: table._id!, sort: options.sort, sortOrder: options.sortOrder, sortType: options.sortType, diff --git a/packages/server/src/sdk/app/rows/search/internal/sqs.ts b/packages/server/src/sdk/app/rows/search/internal/sqs.ts index 90fb082214..5a9e1ddf24 100644 --- a/packages/server/src/sdk/app/rows/search/internal/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/internal/sqs.ts @@ -308,8 +308,8 @@ export async function search( const allTables = await sdk.tables.getAllInternalTables() const allTablesMap = buildTableMap(allTables) // make sure we have the mapped/latest table - if (table?._id) { - table = allTablesMap[table?._id] + if (table._id) { + table = allTablesMap[table._id] } if (!table) { throw new Error("Unable to find table") @@ -322,13 +322,6 @@ export async function search( documentType: DocumentType.ROW, } - let fields = options.fields - if (fields === undefined) { - fields = buildInternalFieldList(table, allTables, { relationships }) - } else { - fields = fields.map(f => mapToUserColumn(f)) - } - if (options.aggregations) { options.aggregations = options.aggregations.map(a => { a.field = mapToUserColumn(a.field) @@ -350,7 +343,10 @@ export async function search( tables: allTablesMap, columnPrefix: USER_COLUMN_PREFIX, }, - resource: { fields, aggregations: options.aggregations }, + resource: { + fields: buildInternalFieldList(table, allTables, { relationships }), + aggregations: options.aggregations, + }, relationships, } @@ -394,7 +390,7 @@ export async function search( // make sure JSON columns corrected const processed = builder.convertJsonStringColumns( table, - await sqlOutputProcessing(rows, table, allTablesMap, relationships, { + await sqlOutputProcessing(rows, source, allTablesMap, relationships, { sqs: true, aggregations: options.aggregations, }) @@ -411,7 +407,7 @@ export async function search( } // get the rows - let finalRows = await outputProcessing(table, processed, { + let finalRows = await outputProcessing(source, processed, { preserveLinks: true, squash: true, aggregations: options.aggregations, From f475454bce2a5daa3c745716c853be0a371239c1 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 24 Sep 2024 18:07:31 +0100 Subject: [PATCH 08/28] Fix search API break. --- packages/backend-core/src/db/lucene.ts | 16 ++++++------ .../backend-core/src/db/tests/lucene.spec.ts | 4 +-- .../server/src/api/controllers/row/index.ts | 3 ++- .../server/src/api/controllers/row/views.ts | 11 +++----- .../src/api/routes/tests/search.spec.ts | 16 +++++++----- .../src/api/routes/tests/templates.spec.ts | 2 +- .../integrations/tests/googlesheets.spec.ts | 22 ++++++++-------- packages/server/src/sdk/app/rows/search.ts | 25 ++++++++----------- .../src/sdk/app/rows/search/external.ts | 2 +- .../sdk/app/rows/search/internal/internal.ts | 2 +- .../sdk/app/rows/search/internal/lucene.ts | 7 +++--- .../sdk/app/rows/search/tests/search.spec.ts | 10 ++++---- .../sdk/app/rows/search/tests/utils.spec.ts | 8 +++--- packages/types/src/sdk/row.ts | 5 ++-- 14 files changed, 66 insertions(+), 67 deletions(-) diff --git a/packages/backend-core/src/db/lucene.ts b/packages/backend-core/src/db/lucene.ts index 7f58c7068e..0206bb2140 100644 --- a/packages/backend-core/src/db/lucene.ts +++ b/packages/backend-core/src/db/lucene.ts @@ -79,8 +79,8 @@ export class QueryBuilder { return this } - setSource(sourceId: string) { - this.#query.equal!.tableId = sourceId + setTable(tableId: string) { + this.#query.equal!.tableId = tableId return this } @@ -637,8 +637,8 @@ async function recursiveSearch( .setSortOrder(params.sortOrder) .setSortType(params.sortType) - if (params.sourceId) { - queryBuilder.setSource(params.sourceId) + if (params.tableId) { + queryBuilder.setTable(params.tableId) } const page = await queryBuilder.run() @@ -671,8 +671,8 @@ export async function paginatedSearch( if (params.version) { search.setVersion(params.version) } - if (params.sourceId) { - search.setSource(params.sourceId) + if (params.tableId) { + search.setTable(params.tableId) } if (params.sort) { search @@ -694,8 +694,8 @@ export async function paginatedSearch( // Try fetching 1 row in the next page to see if another page of results // exists or not search.setBookmark(searchResults.bookmark).setLimit(1) - if (params.sourceId) { - search.setSource(params.sourceId) + if (params.tableId) { + search.setTable(params.tableId) } const nextResults = await search.run() diff --git a/packages/backend-core/src/db/tests/lucene.spec.ts b/packages/backend-core/src/db/tests/lucene.spec.ts index 8747f56a4b..c41bdf88d1 100644 --- a/packages/backend-core/src/db/tests/lucene.spec.ts +++ b/packages/backend-core/src/db/tests/lucene.spec.ts @@ -366,7 +366,7 @@ describe("lucene", () => { }, }, { - sourceId: TABLE_ID, + tableId: TABLE_ID, limit: 1, sort: "property", sortType: SortType.STRING, @@ -390,7 +390,7 @@ describe("lucene", () => { }, }, { - sourceId: TABLE_ID, + tableId: TABLE_ID, query: {}, } ) diff --git a/packages/server/src/api/controllers/row/index.ts b/packages/server/src/api/controllers/row/index.ts index 84b5edcc12..680a7671d5 100644 --- a/packages/server/src/api/controllers/row/index.ts +++ b/packages/server/src/api/controllers/row/index.ts @@ -221,7 +221,8 @@ export async function search(ctx: Ctx) { const searchParams: RowSearchParams = { ...ctx.request.body, query: enrichedQuery, - sourceId: viewId || tableId, + tableId, + viewId, } ctx.status = 200 diff --git a/packages/server/src/api/controllers/row/views.ts b/packages/server/src/api/controllers/row/views.ts index 7008c5e0be..fa75990136 100644 --- a/packages/server/src/api/controllers/row/views.ts +++ b/packages/server/src/api/controllers/row/views.ts @@ -3,8 +3,6 @@ import { ViewV2, SearchRowResponse, SearchViewRowRequest, - RequiredKeys, - RowSearchParams, SearchFilterKey, LogicalOperator, Aggregation, @@ -83,9 +81,9 @@ export async function searchView( field, })) - const searchOptions: RequiredKeys & - RequiredKeys> = { - sourceId: view.id, + const result = await sdk.rows.search({ + viewId: view.id, + tableId: view.tableId, query: enrichedQuery, fields: viewFields, ...getSortOptions(body, view), @@ -94,9 +92,8 @@ export async function searchView( paginate: body.paginate, countRows: body.countRows, aggregations, - } + }) - const result = await sdk.rows.search(searchOptions) result.rows.forEach(r => (r._viewId = view.id)) ctx.body = result } diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 5788d9195a..090514250d 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -157,7 +157,11 @@ describe.each([ if (isInMemory) { return dataFilters.search(_.cloneDeep(rows), this.query) } else { - return config.api.row.search(this.query.sourceId, this.query) + const sourceId = this.query.viewId || this.query.tableId + if (!sourceId) { + throw new Error("No source ID provided") + } + return config.api.row.search(sourceId, this.query) } } @@ -327,8 +331,8 @@ describe.each([ } } - function expectSearch(query: Omit) { - return new SearchAssertion({ ...query, sourceId: table._id! }) + function expectSearch(query: Omit) { + return new SearchAssertion({ ...query, tableId: table._id! }) } function expectQuery(query: SearchFilters) { @@ -1898,7 +1902,7 @@ describe.each([ let { rows: fullRowList } = await config.api.row.search( table._id!, { - sourceId: table._id!, + tableId: table._id!, query: {}, } ) @@ -1909,7 +1913,7 @@ describe.each([ rowCount: number = 0 do { const response = await config.api.row.search(table._id!, { - sourceId: table._id!, + tableId: table._id!, limit: 1, paginate: true, query: {}, @@ -1933,7 +1937,7 @@ describe.each([ // eslint-disable-next-line no-constant-condition while (true) { const response = await config.api.row.search(table._id!, { - sourceId: table._id!, + tableId: table._id!, limit: 3, query: {}, bookmark, diff --git a/packages/server/src/api/routes/tests/templates.spec.ts b/packages/server/src/api/routes/tests/templates.spec.ts index 4290b4386f..6f4d468a68 100644 --- a/packages/server/src/api/routes/tests/templates.spec.ts +++ b/packages/server/src/api/routes/tests/templates.spec.ts @@ -113,7 +113,7 @@ describe("/templates", () => { expect(users.name).toBe("Users") const { rows } = await config.api.row.search(agencyProjects._id!, { - sourceId: agencyProjects._id!, + tableId: agencyProjects._id!, query: {}, }) diff --git a/packages/server/src/integrations/tests/googlesheets.spec.ts b/packages/server/src/integrations/tests/googlesheets.spec.ts index 91addf8a50..34be1c0c6c 100644 --- a/packages/server/src/integrations/tests/googlesheets.spec.ts +++ b/packages/server/src/integrations/tests/googlesheets.spec.ts @@ -219,7 +219,7 @@ describe("Google Sheets Integration", () => { }) let resp = await config.api.row.search(table._id!, { - sourceId: table._id!, + tableId: table._id!, query: {}, paginate: true, limit: 10, @@ -228,7 +228,7 @@ describe("Google Sheets Integration", () => { while (resp.hasNextPage) { resp = await config.api.row.search(table._id!, { - sourceId: table._id!, + tableId: table._id!, query: {}, paginate: true, limit: 10, @@ -637,7 +637,7 @@ describe("Google Sheets Integration", () => { it("should be able to find rows with equals filter", async () => { const response = await config.api.row.search(table._id!, { - sourceId: table._id!, + tableId: table._id!, query: { equal: { name: "Foo", @@ -651,7 +651,7 @@ describe("Google Sheets Integration", () => { it("should be able to find rows with not equals filter", async () => { const response = await config.api.row.search(table._id!, { - sourceId: table._id!, + tableId: table._id!, query: { notEqual: { name: "Foo", @@ -666,7 +666,7 @@ describe("Google Sheets Integration", () => { it("should be able to find rows with empty filter", async () => { const response = await config.api.row.search(table._id!, { - sourceId: table._id!, + tableId: table._id!, query: { empty: { name: null, @@ -679,7 +679,7 @@ describe("Google Sheets Integration", () => { it("should be able to find rows with not empty filter", async () => { const response = await config.api.row.search(table._id!, { - sourceId: table._id!, + tableId: table._id!, query: { notEmpty: { name: null, @@ -692,7 +692,7 @@ describe("Google Sheets Integration", () => { it("should be able to find rows with one of filter", async () => { const response = await config.api.row.search(table._id!, { - sourceId: table._id!, + tableId: table._id!, query: { oneOf: { name: ["Foo", "Bar"], @@ -707,7 +707,7 @@ describe("Google Sheets Integration", () => { it("should be able to find rows with fuzzy filter", async () => { const response = await config.api.row.search(table._id!, { - sourceId: table._id!, + tableId: table._id!, query: { fuzzy: { name: "oo", @@ -721,7 +721,7 @@ describe("Google Sheets Integration", () => { it("should be able to find rows with range filter", async () => { const response = await config.api.row.search(table._id!, { - sourceId: table._id!, + tableId: table._id!, query: { range: { name: { @@ -750,7 +750,7 @@ describe("Google Sheets Integration", () => { }) let response = await config.api.row.search(table._id!, { - sourceId: table._id!, + tableId: table._id!, query: { equal: { name: "Unique value!" } }, paginate: true, limit: 10, @@ -759,7 +759,7 @@ describe("Google Sheets Integration", () => { while (response.hasNextPage) { response = await config.api.row.search(table._id!, { - sourceId: table._id!, + tableId: table._id!, query: { equal: { name: "Unique value!" } }, paginate: true, limit: 10, diff --git a/packages/server/src/sdk/app/rows/search.ts b/packages/server/src/sdk/app/rows/search.ts index fa01a6cf13..c1685d8024 100644 --- a/packages/server/src/sdk/app/rows/search.ts +++ b/packages/server/src/sdk/app/rows/search.ts @@ -14,7 +14,7 @@ import { ExportRowsParams, ExportRowsResult } from "./search/types" import { dataFilters } from "@budibase/shared-core" import sdk from "../../index" import { searchInputMapping } from "./search/utils" -import { features, docIds } from "@budibase/backend-core" +import { features } from "@budibase/backend-core" import tracer from "dd-trace" import { getQueryableFields, removeInvalidFilters } from "./queryUtils" @@ -38,7 +38,8 @@ export async function search( ): Promise> { return await tracer.trace("search", async span => { span?.addTags({ - sourceId: options.sourceId, + tableId: options.tableId, + viewId: options.viewId, query: options.query, sort: options.sort, sortOrder: options.sortOrder, @@ -76,20 +77,16 @@ export async function search( let source: Table | ViewV2 let table: Table - if (docIds.isTableId(options.sourceId)) { - source = await sdk.tables.getTable(options.sourceId) - table = source - options = searchInputMapping(source, options) - } else if (docIds.isViewId(options.sourceId)) { - source = await sdk.views.get(options.sourceId) - table = await sdk.tables.getTable(source.tableId) + if (options.viewId) { + source = await sdk.views.get(options.viewId) + table = await sdk.views.getTable(source) + options = searchInputMapping(table, options) + } else if (options.tableId) { + source = await sdk.tables.getTable(options.tableId) + table = source options = searchInputMapping(table, options) - - span.addTags({ - tableId: table._id, - }) } else { - throw new Error(`Invalid source ID: ${options.sourceId}`) + throw new Error(`Invalid source ID: ${options.viewId || options.tableId}`) } if (options.query) { diff --git a/packages/server/src/sdk/app/rows/search/external.ts b/packages/server/src/sdk/app/rows/search/external.ts index a41ae8dcda..925b472a48 100644 --- a/packages/server/src/sdk/app/rows/search/external.ts +++ b/packages/server/src/sdk/app/rows/search/external.ts @@ -200,7 +200,7 @@ export async function exportRows( } let result = await search( - { sourceId: table._id!, query: requestQuery, sort, sortOrder }, + { tableId: table._id!, query: requestQuery, sort, sortOrder }, table ) let rows: Row[] = [] diff --git a/packages/server/src/sdk/app/rows/search/internal/internal.ts b/packages/server/src/sdk/app/rows/search/internal/internal.ts index c9e2aba237..6617fc376c 100644 --- a/packages/server/src/sdk/app/rows/search/internal/internal.ts +++ b/packages/server/src/sdk/app/rows/search/internal/internal.ts @@ -64,7 +64,7 @@ export async function exportRows( result = await outputProcessing(table, response) } else if (query) { let searchResponse = await sdk.rows.search({ - sourceId: tableId, + tableId, query, sort, sortOrder, diff --git a/packages/server/src/sdk/app/rows/search/internal/lucene.ts b/packages/server/src/sdk/app/rows/search/internal/lucene.ts index 65cf4053d7..694c660625 100644 --- a/packages/server/src/sdk/app/rows/search/internal/lucene.ts +++ b/packages/server/src/sdk/app/rows/search/internal/lucene.ts @@ -19,8 +19,6 @@ export async function search( options: RowSearchParams, source: Table | ViewV2 ): Promise> { - const { sourceId } = options - let table: Table if (sdk.views.isView(source)) { table = await sdk.views.getTable(source.id) @@ -31,7 +29,8 @@ export async function search( const { paginate, query } = options const params: RowSearchParams = { - sourceId: table._id!, + tableId: options.tableId, + viewId: options.viewId, sort: options.sort, sortOrder: options.sortOrder, sortType: options.sortType, @@ -59,7 +58,7 @@ export async function search( // Enrich search results with relationships if (response.rows && response.rows.length) { // enrich with global users if from users table - if (sourceId === InternalTables.USER_METADATA) { + if (table._id === InternalTables.USER_METADATA) { response.rows = await getGlobalUsersFromMetadata(response.rows as User[]) } diff --git a/packages/server/src/sdk/app/rows/search/tests/search.spec.ts b/packages/server/src/sdk/app/rows/search/tests/search.spec.ts index 194f2dd4e3..e7fd095865 100644 --- a/packages/server/src/sdk/app/rows/search/tests/search.spec.ts +++ b/packages/server/src/sdk/app/rows/search/tests/search.spec.ts @@ -122,7 +122,7 @@ describe.each([ it("querying by fields will always return data attribute columns", async () => { await config.doInContext(config.appId, async () => { const { rows } = await search({ - sourceId: table._id!, + tableId: table._id!, query: {}, fields: ["name", "age"], }) @@ -142,7 +142,7 @@ describe.each([ it("will decode _id in oneOf query", async () => { await config.doInContext(config.appId, async () => { const result = await search({ - sourceId: table._id!, + tableId: table._id!, query: { oneOf: { _id: ["%5B1%5D", "%5B4%5D", "%5B8%5D"], @@ -174,7 +174,7 @@ describe.each([ }, }) const result = await search({ - sourceId: table._id!, + tableId: table._id!, query: {}, }) expect(result.rows).toHaveLength(10) @@ -205,7 +205,7 @@ describe.each([ }, }) const result = await search({ - sourceId: table._id!, + tableId: table._id!, query: {}, fields: ["name", "age"], }) @@ -229,7 +229,7 @@ describe.each([ async (queryFields, expectedRows) => { await config.doInContext(config.appId, async () => { const { rows } = await search({ - sourceId: table._id!, + tableId: table._id!, query: { $or: { conditions: [ diff --git a/packages/server/src/sdk/app/rows/search/tests/utils.spec.ts b/packages/server/src/sdk/app/rows/search/tests/utils.spec.ts index e3f241f15a..daa658455b 100644 --- a/packages/server/src/sdk/app/rows/search/tests/utils.spec.ts +++ b/packages/server/src/sdk/app/rows/search/tests/utils.spec.ts @@ -48,7 +48,7 @@ describe.each([tableWithUserCol, tableWithUsersCol])( it("should be able to map ro_ to global user IDs", () => { const params: RowSearchParams = { - sourceId: tableId, + tableId: tableId, query: { equal: { "1:user": userMedataId, @@ -61,7 +61,7 @@ describe.each([tableWithUserCol, tableWithUsersCol])( it("should handle array of user IDs", () => { const params: RowSearchParams = { - sourceId: tableId, + tableId: tableId, query: { oneOf: { "1:user": [userMedataId, globalUserId], @@ -78,7 +78,7 @@ describe.each([tableWithUserCol, tableWithUsersCol])( it("shouldn't change any other input", () => { const email = "test@example.com" const params: RowSearchParams = { - sourceId: tableId, + tableId: tableId, query: { equal: { "1:user": email, @@ -91,7 +91,7 @@ describe.each([tableWithUserCol, tableWithUsersCol])( it("shouldn't error if no query supplied", () => { // @ts-expect-error - intentionally passing in a bad type - const output = searchInputMapping(col, { sourceId: tableId }) + const output = searchInputMapping(col, { tableId }) expect(output.query).toBeUndefined() }) } diff --git a/packages/types/src/sdk/row.ts b/packages/types/src/sdk/row.ts index 2b6ff3a6c6..f81d56c082 100644 --- a/packages/types/src/sdk/row.ts +++ b/packages/types/src/sdk/row.ts @@ -10,7 +10,8 @@ export interface Aggregation { } export interface SearchParams { - sourceId?: string + tableId?: string + viewId?: string query?: SearchFilters paginate?: boolean bookmark?: string | number @@ -29,7 +30,7 @@ export interface SearchParams { // when searching for rows we want a more extensive search type that requires certain properties export interface RowSearchParams - extends WithRequired {} + extends WithRequired {} export interface SearchResponse { rows: T[] From 76453bd500dc3ab65fee06739843654467227b1c Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 25 Sep 2024 14:44:11 +0100 Subject: [PATCH 09/28] Fix many more search tests. --- packages/server/src/sdk/app/rows/search.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/sdk/app/rows/search.ts b/packages/server/src/sdk/app/rows/search.ts index c1685d8024..25d381b636 100644 --- a/packages/server/src/sdk/app/rows/search.ts +++ b/packages/server/src/sdk/app/rows/search.ts @@ -92,7 +92,7 @@ export async function search( if (options.query) { const visibleFields = ( options.fields || Object.keys(table.schema) - ).filter(field => table.schema[field].visible) + ).filter(field => table.schema[field].visible !== false) const queryableFields = await getQueryableFields(table, visibleFields) options.query = removeInvalidFilters(options.query, queryableFields) From 564e16fd5c5a231d485c1864fc29d0b30b027af3 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 25 Sep 2024 15:41:18 +0100 Subject: [PATCH 10/28] wip --- .../src/api/routes/tests/search.spec.ts | 2 +- .../src/sdk/app/rows/search/internal/sqs.ts | 22 +++++++++++++------ 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index d69e93cfd3..9cfd633ad0 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2757,7 +2757,7 @@ describe.each([ }) }) - it.only("can filter by the row ID with limit 1", async () => { + it("can filter by the row ID with limit 1", async () => { await expectSearch({ query: { equal: { _id: row._id }, diff --git a/packages/server/src/sdk/app/rows/search/internal/sqs.ts b/packages/server/src/sdk/app/rows/search/internal/sqs.ts index 5a9e1ddf24..438cff154c 100644 --- a/packages/server/src/sdk/app/rows/search/internal/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/internal/sqs.ts @@ -45,6 +45,7 @@ import { import { dataFilters, helpers, + isInternalColumnName, PROTECTED_INTERNAL_COLUMNS, } from "@budibase/shared-core" import { isSearchingByRowID } from "../utils" @@ -130,15 +131,22 @@ function cleanupFilters( // generate a map of all possible column names (these can be duplicated across tables // the map of them will always be the same const userColumnMap: Record = {} - allTables.forEach(table => - Object.keys(table.schema).forEach( - key => (userColumnMap[key] = mapToUserColumn(key)) - ) - ) + for (const table of allTables) { + for (const key of Object.keys(table.schema)) { + if (isInternalColumnName(key)) { + continue + } + userColumnMap[key] = mapToUserColumn(key) + } + } // update the keys of filters to manage user columns - const keyInAnyTable = (key: string): boolean => - allTables.some(table => table.schema[key]) + const keyInAnyTable = (key: string): boolean => { + if (isInternalColumnName(key)) { + return false + } + return allTables.some(table => table.schema[key]) + } const splitter = new dataFilters.ColumnSplitter(allTables) From 566af9e454e1440d2fc9ecae9b3926f8bf85b7e8 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 25 Sep 2024 16:44:37 +0100 Subject: [PATCH 11/28] Fix bulk import to not modify the table schema. --- .../src/api/controllers/table/internal.ts | 22 +++++-------------- packages/server/src/utilities/schema.ts | 9 +++++++- 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/packages/server/src/api/controllers/table/internal.ts b/packages/server/src/api/controllers/table/internal.ts index 6d1c67e800..1562054ab4 100644 --- a/packages/server/src/api/controllers/table/internal.ts +++ b/packages/server/src/api/controllers/table/internal.ts @@ -70,22 +70,10 @@ export async function bulkImport( ) { const table = await sdk.tables.getTable(ctx.params.tableId) const { rows, identifierFields } = ctx.request.body - await handleDataImport( - { - ...table, - schema: { - _id: { - name: "_id", - type: FieldType.STRING, - }, - ...table.schema, - }, - }, - { - importRows: rows, - identifierFields, - user: ctx.user, - } - ) + await handleDataImport(table, { + importRows: rows, + identifierFields, + user: ctx.user, + }) return table } diff --git a/packages/server/src/utilities/schema.ts b/packages/server/src/utilities/schema.ts index b1fbd7577a..cfdd0d753a 100644 --- a/packages/server/src/utilities/schema.ts +++ b/packages/server/src/utilities/schema.ts @@ -148,9 +148,16 @@ export function parse(rows: Rows, table: Table): Rows { Object.keys(row).forEach(columnName => { const columnData = row[columnName] + + if (columnName === "_id") { + parsedRow[columnName] = columnData + return + } + const schema = table.schema if (!(columnName in schema)) { - // Objects can be present in the row data but not in the schema, so make sure we don't proceed in such a case + // Objects can be present in the row data but not in the schema, so make + // sure we don't proceed in such a case return } From efd677e16a59743f457a5ee7387373002a4839de Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 25 Sep 2024 16:50:06 +0100 Subject: [PATCH 12/28] Most tests passing. --- .../server/src/api/controllers/table/internal.ts | 1 - .../server/src/api/routes/tests/search.spec.ts | 14 +++++++------- .../server/src/api/routes/tests/viewV2.spec.ts | 14 +++++++------- 3 files changed, 14 insertions(+), 15 deletions(-) diff --git a/packages/server/src/api/controllers/table/internal.ts b/packages/server/src/api/controllers/table/internal.ts index 1562054ab4..4286d51d3e 100644 --- a/packages/server/src/api/controllers/table/internal.ts +++ b/packages/server/src/api/controllers/table/internal.ts @@ -3,7 +3,6 @@ import { handleDataImport } from "./utils" import { BulkImportRequest, BulkImportResponse, - FieldType, RenameColumn, SaveTableRequest, SaveTableResponse, diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 9cfd633ad0..090514250d 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -45,14 +45,14 @@ import { generateRowIdField } from "../../../integrations/utils" import { cloneDeep } from "lodash/fp" describe.each([ - // ["in-memory", undefined], - // ["lucene", undefined], + ["in-memory", undefined], + ["lucene", undefined], ["sqs", undefined], - // [DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)], - // [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)], - // [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)], - // [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)], - // [DatabaseName.ORACLE, getDatasource(DatabaseName.ORACLE)], + [DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)], + [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)], + [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)], + [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)], + [DatabaseName.ORACLE, getDatasource(DatabaseName.ORACLE)], ])("search (%s)", (name, dsProvider) => { const isSqs = name === "sqs" const isLucene = name === "lucene" diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index d673e6ee5e..3bde9770cd 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -40,13 +40,13 @@ import { import sdk from "../../../sdk" describe.each([ - // ["lucene", undefined], + ["lucene", undefined], ["sqs", undefined], - // [DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)], - // [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)], - // [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)], - // [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)], - // [DatabaseName.ORACLE, getDatasource(DatabaseName.ORACLE)], + [DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)], + [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)], + [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)], + [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)], + [DatabaseName.ORACLE, getDatasource(DatabaseName.ORACLE)], ])("/v2/views (%s)", (name, dsProvider) => { const config = setup.getConfig() const isSqs = name === "sqs" @@ -2384,7 +2384,7 @@ describe.each([ }) }) - describe("calculations", () => { + describe.skip("calculations", () => { let table: Table let rows: Row[] From 43265bf1ea9eb1c0886c780c85e8bd8a3a62e46b Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 26 Sep 2024 10:54:04 +0100 Subject: [PATCH 13/28] Rejig view calculation code to work with aggregates again. Broke some other tests in the process. --- .../src/api/controllers/row/utils/basic.ts | 71 ++++++++------- .../src/api/controllers/row/utils/sqlUtils.ts | 5 ++ .../src/api/controllers/row/utils/utils.ts | 50 ++++------- .../server/src/api/controllers/row/views.ts | 16 +--- .../src/api/routes/tests/viewV2.spec.ts | 16 ++-- packages/server/src/sdk/app/rows/search.ts | 3 - .../src/sdk/app/rows/search/internal/sqs.ts | 86 +++++++++++++------ packages/types/src/api/web/app/rows.ts | 1 - packages/types/src/sdk/row.ts | 1 - 9 files changed, 135 insertions(+), 114 deletions(-) diff --git a/packages/server/src/api/controllers/row/utils/basic.ts b/packages/server/src/api/controllers/row/utils/basic.ts index 20a1cd0742..9c86a165ab 100644 --- a/packages/server/src/api/controllers/row/utils/basic.ts +++ b/packages/server/src/api/controllers/row/utils/basic.ts @@ -5,7 +5,7 @@ import { Row, Table, JsonTypes, - Aggregation, + ViewV2, } from "@budibase/types" import { helpers, @@ -13,6 +13,7 @@ import { PROTECTED_INTERNAL_COLUMNS, } from "@budibase/shared-core" import { generateRowIdField } from "../../../../integrations/utils" +import sdk from "../../../../sdk" function extractFieldValue({ row, @@ -85,22 +86,28 @@ function fixJsonTypes(row: Row, table: Table) { return row } -export function basicProcessing({ +export async function basicProcessing({ row, - table, + source, tables, isLinked, sqs, - aggregations, }: { row: Row - table: Table + source: Table | ViewV2 tables: Table[] isLinked: boolean sqs?: boolean - aggregations?: Aggregation[] -}): Row { +}): Promise { + let table: Table + if (sdk.views.isView(source)) { + table = await sdk.views.getTable(source.id) + } else { + table = source + } + const thisRow: Row = {} + // filter the row down to what is actually the row (not joined) for (let fieldName of Object.keys(table.schema)) { let value = extractFieldValue({ @@ -118,8 +125,10 @@ export function basicProcessing({ } } - for (let aggregation of aggregations || []) { - thisRow[aggregation.name] = row[aggregation.name] + if (sdk.views.isView(source)) { + for (const key of Object.keys(helpers.views.calculationFields(source))) { + thisRow[key] = row[key] + } } let columns: string[] = Object.keys(table.schema) @@ -163,28 +172,30 @@ export function basicProcessing({ thisRow[col] = array // make sure all of them have an _id const sortField = relatedTable.primaryDisplay || relatedTable.primary![0]! - thisRow[col] = (thisRow[col] as Row[]) - .map(relatedRow => - basicProcessing({ - row: relatedRow, - table: relatedTable, - tables, - isLinked: false, - sqs, - }) + thisRow[col] = ( + await Promise.all( + (thisRow[col] as Row[]).map(relatedRow => + basicProcessing({ + row: relatedRow, + source: relatedTable, + tables, + isLinked: false, + sqs, + }) + ) ) - .sort((a, b) => { - const aField = a?.[sortField], - bField = b?.[sortField] - if (!aField) { - return 1 - } else if (!bField) { - return -1 - } - return aField.localeCompare - ? aField.localeCompare(bField) - : aField - bField - }) + ).sort((a, b) => { + const aField = a?.[sortField], + bField = b?.[sortField] + if (!aField) { + return 1 + } else if (!bField) { + return -1 + } + return aField.localeCompare + ? aField.localeCompare(bField) + : aField - bField + }) } } return fixJsonTypes(thisRow, table) diff --git a/packages/server/src/api/controllers/row/utils/sqlUtils.ts b/packages/server/src/api/controllers/row/utils/sqlUtils.ts index 249bb43bbc..d51972b2c9 100644 --- a/packages/server/src/api/controllers/row/utils/sqlUtils.ts +++ b/packages/server/src/api/controllers/row/utils/sqlUtils.ts @@ -7,6 +7,7 @@ import { ManyToManyRelationshipFieldMetadata, RelationshipFieldMetadata, RelationshipsJson, + Row, Table, } from "@budibase/types" import { breakExternalTableId } from "../../../../integrations/utils" @@ -149,3 +150,7 @@ export function isKnexEmptyReadResponse(resp: DatasourcePlusQueryResponse) { (DSPlusOperation.READ in resp[0] && resp[0].read === true) ) } + +export function isKnexRows(resp: DatasourcePlusQueryResponse): resp is Row[] { + return !isKnexEmptyReadResponse(resp) +} diff --git a/packages/server/src/api/controllers/row/utils/utils.ts b/packages/server/src/api/controllers/row/utils/utils.ts index 45abd93930..b673106d26 100644 --- a/packages/server/src/api/controllers/row/utils/utils.ts +++ b/packages/server/src/api/controllers/row/utils/utils.ts @@ -2,7 +2,6 @@ import * as utils from "../../../../db/utils" import { docIds } from "@budibase/backend-core" import { - Aggregation, Ctx, DatasourcePlusQueryResponse, FieldType, @@ -15,7 +14,7 @@ import { processDates, processFormulas, } from "../../../../utilities/rowProcessor" -import { isKnexEmptyReadResponse } from "./sqlUtils" +import { isKnexRows } from "./sqlUtils" import { basicProcessing, generateIdForRow, getInternalRowId } from "./basic" import sdk from "../../../../sdk" import { processStringSync } from "@budibase/string-templates" @@ -97,7 +96,7 @@ export async function getTableFromSource(source: Table | ViewV2) { return source } -function fixBooleanFields({ row, table }: { row: Row; table: Table }) { +function fixBooleanFields(row: Row, table: Table) { for (let col of Object.values(table.schema)) { if (col.type === FieldType.BOOLEAN) { if (row[col.name] === 1) { @@ -115,53 +114,40 @@ export async function sqlOutputProcessing( source: Table | ViewV2, tables: Record, relationships: RelationshipsJson[], - opts?: { sqs?: boolean; aggregations?: Aggregation[] } + opts?: { sqs?: boolean } ): Promise { - if (isKnexEmptyReadResponse(rows)) { + if (!isKnexRows(rows)) { return [] } + let table: Table if (sdk.views.isView(source)) { table = await sdk.views.getTable(source.id) } else { table = source } - let finalRows: { [key: string]: Row } = {} - for (let row of rows as Row[]) { - let rowId = row._id + + let processedRows: Row[] = [] + for (let row of rows) { if (opts?.sqs) { - rowId = getInternalRowId(row, table) - row._id = rowId - } else if (!rowId) { - rowId = generateIdForRow(row, table) - row._id = rowId + row._id = getInternalRowId(row, table) + } else if (row._id == null) { + row._id = generateIdForRow(row, table) } - const thisRow = basicProcessing({ + + row = await basicProcessing({ row, - table, + source, tables: Object.values(tables), isLinked: false, sqs: opts?.sqs, - aggregations: opts?.aggregations, }) - if (thisRow._id == null) { - throw new Error("Unable to generate row ID for SQL rows") - } - - finalRows[thisRow._id] = fixBooleanFields({ row: thisRow, table }) + row = fixBooleanFields(row, table) + row = await processRelationshipFields(table, tables, row, relationships) + processedRows.push(row) } - // make sure all related rows are correct - let finalRowArray = [] - for (let row of Object.values(finalRows)) { - finalRowArray.push( - await processRelationshipFields(table, tables, row, relationships) - ) - } - - // process some additional types - finalRowArray = processDates(table, finalRowArray) - return finalRowArray + return processDates(table, processedRows) } export function isUserMetadataTable(tableId: string) { diff --git a/packages/server/src/api/controllers/row/views.ts b/packages/server/src/api/controllers/row/views.ts index fa75990136..68958da8e7 100644 --- a/packages/server/src/api/controllers/row/views.ts +++ b/packages/server/src/api/controllers/row/views.ts @@ -5,9 +5,8 @@ import { SearchViewRowRequest, SearchFilterKey, LogicalOperator, - Aggregation, } from "@budibase/types" -import { dataFilters, helpers } from "@budibase/shared-core" +import { dataFilters } from "@budibase/shared-core" import sdk from "../../../sdk" import { db, context, features } from "@budibase/backend-core" import { enrichSearchContext } from "./utils" @@ -26,9 +25,6 @@ export async function searchView( ctx.throw(400, `This method only supports viewsV2`) } - const viewFields = Object.entries(helpers.views.basicFields(view)) - .filter(([_, value]) => value.visible) - .map(([key]) => key) const { body } = ctx.request // Enrich saved query with ephemeral query params. @@ -73,25 +69,15 @@ export async function searchView( user: sdk.users.getUserContextBindings(ctx.user), }) - const aggregations: Aggregation[] = Object.entries( - helpers.views.calculationFields(view) - ).map(([name, { field, calculationType }]) => ({ - name, - calculationType, - field, - })) - const result = await sdk.rows.search({ viewId: view.id, tableId: view.tableId, query: enrichedQuery, - fields: viewFields, ...getSortOptions(body, view), limit: body.limit, bookmark: body.bookmark, paginate: body.paginate, countRows: body.countRows, - aggregations, }) result.rows.forEach(r => (r._viewId = view.id)) diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 3bde9770cd..1d47769fb8 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -40,13 +40,13 @@ import { import sdk from "../../../sdk" describe.each([ - ["lucene", undefined], + // ["lucene", undefined], ["sqs", undefined], - [DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)], - [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)], - [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)], - [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)], - [DatabaseName.ORACLE, getDatasource(DatabaseName.ORACLE)], + // [DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)], + // [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)], + // [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)], + // [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)], + // [DatabaseName.ORACLE, getDatasource(DatabaseName.ORACLE)], ])("/v2/views (%s)", (name, dsProvider) => { const config = setup.getConfig() const isSqs = name === "sqs" @@ -1653,7 +1653,7 @@ describe.each([ }) describe("search", () => { - it("returns empty rows from view when no schema is passed", async () => { + it.only("returns empty rows from view when no schema is passed", async () => { const rows = await Promise.all( Array.from({ length: 10 }, () => config.api.row.save(table._id!, {})) ) @@ -2384,7 +2384,7 @@ describe.each([ }) }) - describe.skip("calculations", () => { + describe("calculations", () => { let table: Table let rows: Row[] diff --git a/packages/server/src/sdk/app/rows/search.ts b/packages/server/src/sdk/app/rows/search.ts index 25d381b636..cb7397689a 100644 --- a/packages/server/src/sdk/app/rows/search.ts +++ b/packages/server/src/sdk/app/rows/search.ts @@ -49,9 +49,6 @@ export async function search( paginate: options.paginate, fields: options.fields, countRows: options.countRows, - aggregations: options.aggregations - ?.map(a => `${a.field}:${a.calculationType}`) - .join(", "), }) options.query = dataFilters.cleanupQuery(options.query || {}) diff --git a/packages/server/src/sdk/app/rows/search/internal/sqs.ts b/packages/server/src/sdk/app/rows/search/internal/sqs.ts index 438cff154c..a01b1bc820 100644 --- a/packages/server/src/sdk/app/rows/search/internal/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/internal/sqs.ts @@ -1,4 +1,5 @@ import { + Aggregation, Datasource, DocumentType, FieldType, @@ -58,11 +59,34 @@ const MISSING_COLUMN_REGEX = new RegExp(`no such column: .+`) const MISSING_TABLE_REGX = new RegExp(`no such table: .+`) const DUPLICATE_COLUMN_REGEX = new RegExp(`duplicate column name: .+`) -function buildInternalFieldList( - table: Table, +async function buildInternalFieldList( + source: Table | ViewV2, tables: Table[], - opts?: { relationships?: RelationshipsJson[] } + opts?: { relationships?: RelationshipsJson[]; allowedFields?: string[] } ) { + const { relationships, allowedFields } = opts || {} + let schemaFields: string[] = [] + if (sdk.views.isView(source)) { + schemaFields = Object.keys(helpers.views.basicFields(source)).filter( + key => source.schema?.[key]?.visible !== false + ) + } else { + schemaFields = Object.keys(source.schema).filter( + key => source.schema[key].visible !== false + ) + } + + if (allowedFields) { + schemaFields = schemaFields.filter(field => allowedFields.includes(field)) + } + + let table: Table + if (sdk.views.isView(source)) { + table = await sdk.views.getTable(source.id) + } else { + table = source + } + let fieldList: string[] = [] const getJunctionFields = (relatedTable: Table, fields: string[]) => { const junctionFields: string[] = [] @@ -73,13 +97,18 @@ function buildInternalFieldList( }) return junctionFields } - fieldList = fieldList.concat( - PROTECTED_INTERNAL_COLUMNS.map(col => `${table._id}.${col}`) - ) - for (let key of Object.keys(table.schema)) { + if (sdk.tables.isTable(source)) { + for (const key of PROTECTED_INTERNAL_COLUMNS) { + if (allowedFields && !allowedFields.includes(key)) { + continue + } + fieldList.push(`${table._id}.${key}`) + } + } + for (let key of schemaFields) { const col = table.schema[key] const isRelationship = col.type === FieldType.LINK - if (!opts?.relationships && isRelationship) { + if (!relationships && isRelationship) { continue } if (!isRelationship) { @@ -90,7 +119,9 @@ function buildInternalFieldList( if (!relatedTable) { continue } - const relatedFields = buildInternalFieldList(relatedTable, tables).concat( + const relatedFields = ( + await buildInternalFieldList(relatedTable, tables) + ).concat( getJunctionFields(relatedTable, ["doc1.fieldName", "doc2.fieldName"]) ) // break out of the loop if we have reached the max number of columns @@ -330,11 +361,20 @@ export async function search( documentType: DocumentType.ROW, } - if (options.aggregations) { - options.aggregations = options.aggregations.map(a => { - a.field = mapToUserColumn(a.field) - return a - }) + let aggregations: Aggregation[] = [] + if (sdk.views.isView(source)) { + const calculationFields = helpers.views.calculationFields(source) + for (const [key, field] of Object.entries(calculationFields)) { + if (options.fields && !options.fields.includes(key)) { + continue + } + + aggregations.push({ + name: key, + field: mapToUserColumn(field.field), + calculationType: field.calculationType, + }) + } } const request: QueryJson = { @@ -352,8 +392,11 @@ export async function search( columnPrefix: USER_COLUMN_PREFIX, }, resource: { - fields: buildInternalFieldList(table, allTables, { relationships }), - aggregations: options.aggregations, + fields: await buildInternalFieldList(source, allTables, { + relationships, + allowedFields: options.fields, + }), + aggregations, }, relationships, } @@ -400,7 +443,6 @@ export async function search( table, await sqlOutputProcessing(rows, source, allTablesMap, relationships, { sqs: true, - aggregations: options.aggregations, }) ) @@ -418,16 +460,12 @@ export async function search( let finalRows = await outputProcessing(source, processed, { preserveLinks: true, squash: true, - aggregations: options.aggregations, + aggregations, }) // check if we need to pick specific rows out if (options.fields) { - const fields = [ - ...options.fields, - ...PROTECTED_INTERNAL_COLUMNS, - ...(options.aggregations || []).map(a => a.name), - ] + const fields = [...options.fields, ...PROTECTED_INTERNAL_COLUMNS] finalRows = finalRows.map((r: any) => pick(r, fields)) } @@ -450,7 +488,7 @@ export async function search( const msg = typeof err === "string" ? err : err.message if (!opts?.retrying && resyncDefinitionsRequired(err.status, msg)) { await sdk.tables.sqs.syncDefinition() - return search(options, table, { retrying: true }) + return search(options, source, { retrying: true }) } // previously the internal table didn't error when a column didn't exist in search if (err.status === 400 && msg?.match(MISSING_COLUMN_REGEX)) { diff --git a/packages/types/src/api/web/app/rows.ts b/packages/types/src/api/web/app/rows.ts index 5d49ac1812..ce6f6f672d 100644 --- a/packages/types/src/api/web/app/rows.ts +++ b/packages/types/src/api/web/app/rows.ts @@ -26,7 +26,6 @@ export interface SearchViewRowRequest | "paginate" | "query" | "countRows" - | "aggregations" > {} export interface SearchRowResponse { diff --git a/packages/types/src/sdk/row.ts b/packages/types/src/sdk/row.ts index f81d56c082..0b63e88229 100644 --- a/packages/types/src/sdk/row.ts +++ b/packages/types/src/sdk/row.ts @@ -25,7 +25,6 @@ export interface SearchParams { indexer?: () => Promise rows?: Row[] countRows?: boolean - aggregations?: Aggregation[] } // when searching for rows we want a more extensive search type that requires certain properties From 0ef633b87a9c3dfa9e08ef8e03471f39a0bb4e0b Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 26 Sep 2024 11:56:03 +0100 Subject: [PATCH 14/28] Fix viewV2.spec.ts for sqs --- .../src/api/routes/tests/viewV2.spec.ts | 24 +------------------ .../src/sdk/app/rows/search/internal/sqs.ts | 12 ++++++---- 2 files changed, 8 insertions(+), 28 deletions(-) diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 1d47769fb8..ba2e93afbf 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -1653,7 +1653,7 @@ describe.each([ }) describe("search", () => { - it.only("returns empty rows from view when no schema is passed", async () => { + it("returns empty rows from view when no schema is passed", async () => { const rows = await Promise.all( Array.from({ length: 10 }, () => config.api.row.save(table._id!, {})) ) @@ -2197,28 +2197,6 @@ describe.each([ expect(response.rows).toHaveLength(0) }) - it("queries the row api passing the view fields only", async () => { - const searchSpy = jest.spyOn(sdk.rows, "search") - - const view = await config.api.viewV2.create({ - tableId: table._id!, - name: generator.guid(), - schema: { - id: { visible: true }, - one: { visible: false }, - }, - }) - - await config.api.viewV2.search(view.id, { query: {} }) - expect(searchSpy).toHaveBeenCalledTimes(1) - - expect(searchSpy).toHaveBeenCalledWith( - expect.objectContaining({ - fields: ["id"], - }) - ) - }) - describe("foreign relationship columns", () => { let envCleanup: () => void beforeAll(() => { diff --git a/packages/server/src/sdk/app/rows/search/internal/sqs.ts b/packages/server/src/sdk/app/rows/search/internal/sqs.ts index a01b1bc820..26e8f59303 100644 --- a/packages/server/src/sdk/app/rows/search/internal/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/internal/sqs.ts @@ -463,11 +463,13 @@ export async function search( aggregations, }) - // check if we need to pick specific rows out - if (options.fields) { - const fields = [...options.fields, ...PROTECTED_INTERNAL_COLUMNS] - finalRows = finalRows.map((r: any) => pick(r, fields)) - } + const visibleFields = + options.fields || + Object.keys(source.schema || {}).filter( + key => source.schema?.[key].visible !== false + ) + const allowedFields = [...visibleFields, ...PROTECTED_INTERNAL_COLUMNS] + finalRows = finalRows.map((r: any) => pick(r, allowedFields)) const response: SearchResponse = { rows: finalRows, From c4c524c6ff35921f036b4ef9fdb051bf58ecc15f Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 26 Sep 2024 15:22:10 +0100 Subject: [PATCH 15/28] viewV2.spec.ts passsing in full --- packages/backend-core/src/sql/sql.ts | 5 +- .../api/controllers/row/ExternalRequest.ts | 22 +++- .../src/api/controllers/row/utils/basic.ts | 32 ++--- .../src/api/controllers/row/utils/sqlUtils.ts | 41 +++++-- .../src/api/controllers/row/utils/utils.ts | 5 +- .../src/api/routes/tests/viewV2.spec.ts | 114 +++++++++--------- .../src/sdk/app/rows/search/external.ts | 11 +- .../sdk/app/rows/search/internal/lucene.ts | 11 +- .../src/sdk/app/rows/search/internal/sqs.ts | 1 - .../src/utilities/rowProcessor/index.ts | 9 +- 10 files changed, 151 insertions(+), 100 deletions(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index bfb9d9bfbe..b7bf5bc102 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -859,7 +859,7 @@ class InternalBuilder { } addSorting(query: Knex.QueryBuilder): Knex.QueryBuilder { - let { sort } = this.query + let { sort, resource } = this.query const primaryKey = this.table.primary const tableName = getTableName(this.table) const aliases = this.query.tableAliases @@ -896,7 +896,8 @@ class InternalBuilder { // add sorting by the primary key if the result isn't already sorted by it, // to make sure result is deterministic - if (!sort || sort[primaryKey[0]] === undefined) { + const hasAggregations = (resource?.aggregations?.length ?? 0) > 0 + if (!hasAggregations && (!sort || sort[primaryKey[0]] === undefined)) { query = query.orderBy(`${aliased}.${primaryKey[0]}`) } return query diff --git a/packages/server/src/api/controllers/row/ExternalRequest.ts b/packages/server/src/api/controllers/row/ExternalRequest.ts index a52d7abcd1..4ce326c35f 100644 --- a/packages/server/src/api/controllers/row/ExternalRequest.ts +++ b/packages/server/src/api/controllers/row/ExternalRequest.ts @@ -1,5 +1,6 @@ import dayjs from "dayjs" import { + Aggregation, AutoFieldSubType, AutoReason, Datasource, @@ -47,7 +48,7 @@ import { db as dbCore } from "@budibase/backend-core" import sdk from "../../../sdk" import env from "../../../environment" import { makeExternalQuery } from "../../../integrations/base/query" -import { dataFilters } from "@budibase/shared-core" +import { dataFilters, helpers } from "@budibase/shared-core" export interface ManyRelationship { tableId?: string @@ -682,12 +683,26 @@ export class ExternalRequest { } } } + if ( operation === Operation.DELETE && (filters == null || Object.keys(filters).length === 0) ) { throw "Deletion must be filtered" } + + let aggregations: Aggregation[] = [] + if (sdk.views.isView(this.source)) { + const calculationFields = helpers.views.calculationFields(this.source) + for (const [key, field] of Object.entries(calculationFields)) { + aggregations.push({ + name: key, + field: field.field, + calculationType: field.calculationType, + }) + } + } + let json: QueryJson = { endpoint: { datasourceId: this.datasource._id!, @@ -697,10 +712,11 @@ export class ExternalRequest { resource: { // have to specify the fields to avoid column overlap (for SQL) fields: isSql - ? buildSqlFieldList(table, this.tables, { + ? await buildSqlFieldList(this.source, this.tables, { relationships: incRelationships, }) : [], + aggregations, }, filters, sort, @@ -748,7 +764,7 @@ export class ExternalRequest { } const output = await sqlOutputProcessing( response, - table, + this.source, this.tables, relationships ) diff --git a/packages/server/src/api/controllers/row/utils/basic.ts b/packages/server/src/api/controllers/row/utils/basic.ts index 9c86a165ab..23565670ba 100644 --- a/packages/server/src/api/controllers/row/utils/basic.ts +++ b/packages/server/src/api/controllers/row/utils/basic.ts @@ -100,8 +100,10 @@ export async function basicProcessing({ sqs?: boolean }): Promise { let table: Table + let isCalculationView = false if (sdk.views.isView(source)) { table = await sdk.views.getTable(source.id) + isCalculationView = helpers.views.isCalculationView(source) } else { table = source } @@ -132,20 +134,22 @@ export async function basicProcessing({ } let columns: string[] = Object.keys(table.schema) - if (!sqs) { - thisRow._id = generateIdForRow(row, table, isLinked) - thisRow.tableId = table._id - thisRow._rev = "rev" - columns = columns.concat(PROTECTED_EXTERNAL_COLUMNS) - } else { - columns = columns.concat(PROTECTED_EXTERNAL_COLUMNS) - for (let internalColumn of [...PROTECTED_INTERNAL_COLUMNS, ...columns]) { - thisRow[internalColumn] = extractFieldValue({ - row, - tableName: table._id!, - fieldName: internalColumn, - isLinked, - }) + if (!isCalculationView) { + if (!sqs) { + thisRow._id = generateIdForRow(row, table, isLinked) + thisRow.tableId = table._id + thisRow._rev = "rev" + columns = columns.concat(PROTECTED_EXTERNAL_COLUMNS) + } else { + columns = columns.concat(PROTECTED_EXTERNAL_COLUMNS) + for (let internalColumn of [...PROTECTED_INTERNAL_COLUMNS, ...columns]) { + thisRow[internalColumn] = extractFieldValue({ + row, + tableName: table._id!, + fieldName: internalColumn, + isLinked, + }) + } } } for (let col of columns) { diff --git a/packages/server/src/api/controllers/row/utils/sqlUtils.ts b/packages/server/src/api/controllers/row/utils/sqlUtils.ts index d51972b2c9..36521b10c6 100644 --- a/packages/server/src/api/controllers/row/utils/sqlUtils.ts +++ b/packages/server/src/api/controllers/row/utils/sqlUtils.ts @@ -9,9 +9,12 @@ import { RelationshipsJson, Row, Table, + ViewV2, } from "@budibase/types" import { breakExternalTableId } from "../../../../integrations/utils" import { generateJunctionTableID } from "../../../../db/utils" +import sdk from "../../../../sdk" +import { helpers } from "@budibase/shared-core" type TableMap = Record @@ -109,11 +112,12 @@ export function buildInternalRelationships( * Creating the specific list of fields that we desire, and excluding the ones that are no use to us * is more performant and has the added benefit of protecting against this scenario. */ -export function buildSqlFieldList( - table: Table, +export async function buildSqlFieldList( + source: Table | ViewV2, tables: TableMap, opts?: { relationships: boolean } ) { + const { relationships } = opts || {} function extractRealFields(table: Table, existing: string[] = []) { return Object.entries(table.schema) .filter( @@ -124,22 +128,33 @@ export function buildSqlFieldList( ) .map(column => `${table.name}.${column[0]}`) } - let fields = extractRealFields(table) + + let fields: string[] = [] + if (sdk.views.isView(source)) { + fields = Object.keys(helpers.views.basicFields(source)).filter( + key => source.schema?.[key]?.visible !== false + ) + } else { + fields = extractRealFields(source) + } + + let table: Table + if (sdk.views.isView(source)) { + table = await sdk.views.getTable(source.id) + } else { + table = source + } + for (let field of Object.values(table.schema)) { - if ( - field.type !== FieldType.LINK || - !opts?.relationships || - !field.tableId - ) { + if (field.type !== FieldType.LINK || !relationships || !field.tableId) { continue } - const { tableName: linkTableName } = breakExternalTableId(field.tableId) - const linkTable = tables[linkTableName] - if (linkTable) { - const linkedFields = extractRealFields(linkTable, fields) - fields = fields.concat(linkedFields) + const { tableName } = breakExternalTableId(field.tableId) + if (tables[tableName]) { + fields = fields.concat(extractRealFields(tables[tableName], fields)) } } + return fields } diff --git a/packages/server/src/api/controllers/row/utils/utils.ts b/packages/server/src/api/controllers/row/utils/utils.ts index b673106d26..4188fcced3 100644 --- a/packages/server/src/api/controllers/row/utils/utils.ts +++ b/packages/server/src/api/controllers/row/utils/utils.ts @@ -19,6 +19,7 @@ import { basicProcessing, generateIdForRow, getInternalRowId } from "./basic" import sdk from "../../../../sdk" import { processStringSync } from "@budibase/string-templates" import validateJs from "validate.js" +import { helpers } from "@budibase/shared-core" validateJs.extend(validateJs.validators.datetime, { parse: function (value: string) { @@ -121,8 +122,10 @@ export async function sqlOutputProcessing( } let table: Table + let isCalculationView = false if (sdk.views.isView(source)) { table = await sdk.views.getTable(source.id) + isCalculationView = helpers.views.isCalculationView(source) } else { table = source } @@ -131,7 +134,7 @@ export async function sqlOutputProcessing( for (let row of rows) { if (opts?.sqs) { row._id = getInternalRowId(row, table) - } else if (row._id == null) { + } else if (row._id == null && !isCalculationView) { row._id = generateIdForRow(row, table) } diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index ba2e93afbf..883b25aca1 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -37,16 +37,15 @@ import { setEnv as setCoreEnv, env, } from "@budibase/backend-core" -import sdk from "../../../sdk" describe.each([ - // ["lucene", undefined], + ["lucene", undefined], ["sqs", undefined], - // [DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)], - // [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)], - // [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)], - // [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)], - // [DatabaseName.ORACLE, getDatasource(DatabaseName.ORACLE)], + [DatabaseName.POSTGRES, getDatasource(DatabaseName.POSTGRES)], + [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)], + [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)], + [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)], + [DatabaseName.ORACLE, getDatasource(DatabaseName.ORACLE)], ])("/v2/views (%s)", (name, dsProvider) => { const config = setup.getConfig() const isSqs = name === "sqs" @@ -2362,63 +2361,70 @@ describe.each([ }) }) - describe("calculations", () => { - let table: Table - let rows: Row[] + !isLucene && + describe("calculations", () => { + let table: Table + let rows: Row[] - beforeAll(async () => { - table = await config.api.table.save( - saveTableRequest({ - schema: { - quantity: { - type: FieldType.NUMBER, - name: "quantity", + beforeAll(async () => { + table = await config.api.table.save( + saveTableRequest({ + schema: { + quantity: { + type: FieldType.NUMBER, + name: "quantity", + }, + price: { + type: FieldType.NUMBER, + name: "price", + }, }, - price: { - type: FieldType.NUMBER, - name: "price", + }) + ) + + rows = await Promise.all( + Array.from({ length: 10 }, () => + config.api.row.save(table._id!, { + quantity: generator.natural({ min: 1, max: 10 }), + price: generator.natural({ min: 1, max: 10 }), + }) + ) + ) + }) + + it("should be able to search by calculations", async () => { + const view = await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), + schema: { + "Quantity Sum": { + visible: true, + calculationType: CalculationType.SUM, + field: "quantity", }, }, }) - ) - rows = await Promise.all( - Array.from({ length: 10 }, () => - config.api.row.save(table._id!, { - quantity: generator.natural({ min: 1, max: 10 }), - price: generator.natural({ min: 1, max: 10 }), - }) + const response = await config.api.viewV2.search(view.id, { + query: {}, + }) + + expect(response.rows).toHaveLength(1) + expect(response.rows).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + "Quantity Sum": rows.reduce((acc, r) => acc + r.quantity, 0), + }), + ]) ) - ) - }) - it("should be able to search by calculations", async () => { - const view = await config.api.viewV2.create({ - tableId: table._id!, - name: generator.guid(), - schema: { - "Quantity Sum": { - visible: true, - calculationType: CalculationType.SUM, - field: "quantity", - }, - }, + // Calculation views do not return rows that can be linked back to + // the source table, and so should not have an _id field. + for (const row of response.rows) { + expect("_id" in row).toBe(false) + } }) - - const response = await config.api.viewV2.search(view.id, { - query: {}, - }) - - expect(response.rows).toHaveLength(1) - expect(response.rows).toEqual( - expect.arrayContaining([ - expect.objectContaining({ - "Quantity Sum": rows.reduce((acc, r) => acc + r.quantity, 0), - }), - ]) - ) }) - }) }) describe("permissions", () => { diff --git a/packages/server/src/sdk/app/rows/search/external.ts b/packages/server/src/sdk/app/rows/search/external.ts index 925b472a48..fa8961461b 100644 --- a/packages/server/src/sdk/app/rows/search/external.ts +++ b/packages/server/src/sdk/app/rows/search/external.ts @@ -127,10 +127,13 @@ export async function search( } } - if (options.fields) { - const fields = [...options.fields, ...PROTECTED_EXTERNAL_COLUMNS] - processed = processed.map((r: any) => pick(r, fields)) - } + const visibleFields = + options.fields || + Object.keys(source.schema || {}).filter( + key => source.schema?.[key].visible !== false + ) + const allowedFields = [...visibleFields, ...PROTECTED_EXTERNAL_COLUMNS] + processed = processed.map((r: any) => pick(r, allowedFields)) // need wrapper object for bookmarks etc when paginating const response: SearchResponse = { rows: processed, hasNextPage } diff --git a/packages/server/src/sdk/app/rows/search/internal/lucene.ts b/packages/server/src/sdk/app/rows/search/internal/lucene.ts index 694c660625..953fb90c1f 100644 --- a/packages/server/src/sdk/app/rows/search/internal/lucene.ts +++ b/packages/server/src/sdk/app/rows/search/internal/lucene.ts @@ -62,10 +62,13 @@ export async function search( response.rows = await getGlobalUsersFromMetadata(response.rows as User[]) } - if (options.fields) { - const fields = [...options.fields, ...PROTECTED_INTERNAL_COLUMNS] - response.rows = response.rows.map((r: any) => pick(r, fields)) - } + const visibleFields = + options.fields || + Object.keys(source.schema || {}).filter( + key => source.schema?.[key].visible !== false + ) + const allowedFields = [...visibleFields, ...PROTECTED_INTERNAL_COLUMNS] + response.rows = response.rows.map((r: any) => pick(r, allowedFields)) response.rows = await outputProcessing(source, response.rows, { squash: true, diff --git a/packages/server/src/sdk/app/rows/search/internal/sqs.ts b/packages/server/src/sdk/app/rows/search/internal/sqs.ts index 26e8f59303..c185ef18dd 100644 --- a/packages/server/src/sdk/app/rows/search/internal/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/internal/sqs.ts @@ -460,7 +460,6 @@ export async function search( let finalRows = await outputProcessing(source, processed, { preserveLinks: true, squash: true, - aggregations, }) const visibleFields = diff --git a/packages/server/src/utilities/rowProcessor/index.ts b/packages/server/src/utilities/rowProcessor/index.ts index 24c8d11bd1..818c98a84e 100644 --- a/packages/server/src/utilities/rowProcessor/index.ts +++ b/packages/server/src/utilities/rowProcessor/index.ts @@ -11,7 +11,6 @@ import { import { InternalTables } from "../../db/utils" import { TYPE_TRANSFORM_MAP } from "./map" import { - Aggregation, AutoFieldSubType, FieldType, IdentityType, @@ -263,7 +262,6 @@ export async function outputProcessing( preserveLinks?: boolean fromRow?: Row skipBBReferences?: boolean - aggregations?: Aggregation[] } = { squash: true, preserveLinks: false, @@ -411,8 +409,11 @@ export async function outputProcessing( f.toLowerCase() ) - for (const aggregation of opts.aggregations || []) { - fields.push(aggregation.name.toLowerCase()) + if (sdk.views.isView(source)) { + const aggregations = helpers.views.calculationFields(source) + for (const key of Object.keys(aggregations)) { + fields.push(key.toLowerCase()) + } } for (const row of enriched) { From 7c6c03c80b2ad15944b6c830f1abc6fd213d7c8e Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 26 Sep 2024 15:32:21 +0100 Subject: [PATCH 16/28] Rename ViewUIFieldMetadata -> ViewFieldMetadata to match master. --- packages/server/src/api/controllers/view/viewsV2.ts | 12 ++++++------ packages/server/src/api/routes/tests/viewV2.spec.ts | 4 ++-- packages/server/src/db/linkedRows/index.ts | 4 ++-- packages/shared-core/src/helpers/views.ts | 10 +++++----- packages/types/src/documents/app/view.ts | 10 +++++----- 5 files changed, 20 insertions(+), 20 deletions(-) diff --git a/packages/server/src/api/controllers/view/viewsV2.ts b/packages/server/src/api/controllers/view/viewsV2.ts index fddcef97a2..7f6f638541 100644 --- a/packages/server/src/api/controllers/view/viewsV2.ts +++ b/packages/server/src/api/controllers/view/viewsV2.ts @@ -7,17 +7,17 @@ import { ViewResponse, ViewResponseEnriched, ViewV2, - BasicViewUIFieldMetadata, + BasicViewFieldMetadata, ViewCalculationFieldMetadata, RelationSchemaField, - ViewUIFieldMetadata, + ViewFieldMetadata, } from "@budibase/types" import { builderSocket, gridSocket } from "../../../websockets" import { helpers } from "@budibase/shared-core" function stripUnknownFields( - field: BasicViewUIFieldMetadata -): RequiredKeys { + field: BasicViewFieldMetadata +): RequiredKeys { if (helpers.views.isCalculationField(field)) { const strippedField: RequiredKeys = { order: field.order, @@ -31,7 +31,7 @@ function stripUnknownFields( } return strippedField } else { - const strippedField: RequiredKeys = { + const strippedField: RequiredKeys = { order: field.order, width: field.width, visible: field.visible, @@ -83,7 +83,7 @@ async function parseSchema(view: CreateViewRequest) { p[fieldName] = fieldSchema return p - }, {} as Record>) + }, {} as Record>) return finalViewSchema } diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 883b25aca1..3712efa5a4 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -22,7 +22,7 @@ import { RelationshipType, TableSchema, RenameColumn, - ViewUIFieldMetadata, + ViewFieldMetadata, FeatureFlag, BBReferenceFieldSubType, } from "@budibase/types" @@ -1154,7 +1154,7 @@ describe.each([ const createView = async ( tableId: string, - schema: Record + schema: Record ) => await config.api.viewV2.create({ name: generator.guid(), diff --git a/packages/server/src/db/linkedRows/index.ts b/packages/server/src/db/linkedRows/index.ts index ed660288c8..41cbc5b9c1 100644 --- a/packages/server/src/db/linkedRows/index.ts +++ b/packages/server/src/db/linkedRows/index.ts @@ -20,7 +20,7 @@ import { Row, Table, TableSchema, - ViewUIFieldMetadata, + ViewFieldMetadata, ViewV2, } from "@budibase/types" import sdk from "../../sdk" @@ -263,7 +263,7 @@ export async function squashLinks( FeatureFlag.ENRICHED_RELATIONSHIPS ) - let viewSchema: Record = {} + let viewSchema: Record = {} if (sdk.views.isView(source)) { if (helpers.views.isCalculationView(source)) { return enriched diff --git a/packages/shared-core/src/helpers/views.ts b/packages/shared-core/src/helpers/views.ts index c65bc4882d..f41c66adc8 100644 --- a/packages/shared-core/src/helpers/views.ts +++ b/packages/shared-core/src/helpers/views.ts @@ -1,20 +1,20 @@ import { - BasicViewUIFieldMetadata, + BasicViewFieldMetadata, ViewCalculationFieldMetadata, - ViewUIFieldMetadata, + ViewFieldMetadata, ViewV2, } from "@budibase/types" import { pickBy } from "lodash" export function isCalculationField( - field: ViewUIFieldMetadata + field: ViewFieldMetadata ): field is ViewCalculationFieldMetadata { return "calculationType" in field } export function isBasicViewField( - field: ViewUIFieldMetadata -): field is BasicViewUIFieldMetadata { + field: ViewFieldMetadata +): field is BasicViewFieldMetadata { return !isCalculationField(field) } diff --git a/packages/types/src/documents/app/view.ts b/packages/types/src/documents/app/view.ts index 74b8f61f59..a957564039 100644 --- a/packages/types/src/documents/app/view.ts +++ b/packages/types/src/documents/app/view.ts @@ -33,7 +33,7 @@ export interface View { groupBy?: string } -export interface BasicViewUIFieldMetadata extends UIFieldMetadata { +export interface BasicViewFieldMetadata extends UIFieldMetadata { readonly?: boolean columns?: Record } @@ -42,13 +42,13 @@ export interface RelationSchemaField extends UIFieldMetadata { readonly?: boolean } -export interface ViewCalculationFieldMetadata extends BasicViewUIFieldMetadata { +export interface ViewCalculationFieldMetadata extends BasicViewFieldMetadata { calculationType: CalculationType field: string } -export type ViewUIFieldMetadata = - | BasicViewUIFieldMetadata +export type ViewFieldMetadata = + | BasicViewFieldMetadata | ViewCalculationFieldMetadata export enum CalculationType { @@ -71,7 +71,7 @@ export interface ViewV2 { order?: SortOrder type?: SortType } - schema?: Record + schema?: Record } export type ViewSchema = ViewCountOrSumSchema | ViewStatisticsSchema From 25a2e02a90acf6b07b71dce4cde62d4899ce7cdd Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 26 Sep 2024 15:40:25 +0100 Subject: [PATCH 17/28] Remove needless table copy. --- packages/server/src/api/controllers/row/internal.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/server/src/api/controllers/row/internal.ts b/packages/server/src/api/controllers/row/internal.ts index f5cb42f81d..bb9de6ce52 100644 --- a/packages/server/src/api/controllers/row/internal.ts +++ b/packages/server/src/api/controllers/row/internal.ts @@ -53,11 +53,8 @@ export async function patch(ctx: UserCtx) { combinedRow[key] = inputs[key] } - // need to copy the table so it can be differenced on way out - const tableClone = cloneDeep(table) - // this returns the table and row incase they have been updated - let row = await inputProcessing(ctx.user?._id, tableClone, combinedRow) + let row = await inputProcessing(ctx.user?._id, source, combinedRow) const validateResult = await sdk.rows.utils.validate({ row, source, From 26a27ff70f1a0e9a363ce9e9d19cae123f1e7776 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 26 Sep 2024 15:48:44 +0100 Subject: [PATCH 18/28] Remove needless table copy. --- packages/server/src/sdk/app/rows/internal.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/server/src/sdk/app/rows/internal.ts b/packages/server/src/sdk/app/rows/internal.ts index 13054f43f4..997af3c907 100644 --- a/packages/server/src/sdk/app/rows/internal.ts +++ b/packages/server/src/sdk/app/rows/internal.ts @@ -34,10 +34,7 @@ export async function save( inputs._id = db.generateRowID(inputs.tableId) } - // need to copy the table so it can be differenced on way out - const sourceClone = cloneDeep(source) - - let row = await inputProcessing(userId, sourceClone, inputs) + let row = await inputProcessing(userId, source, inputs) const validateResult = await sdk.rows.utils.validate({ row, From ec6fa5f79b2dda85dbb7043291fecf6bc59ec3f0 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 26 Sep 2024 15:50:49 +0100 Subject: [PATCH 19/28] Return SQS error to prevoius state. --- packages/server/src/sdk/app/rows/search/internal/sqs.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/sdk/app/rows/search/internal/sqs.ts b/packages/server/src/sdk/app/rows/search/internal/sqs.ts index c185ef18dd..f0e47ce2b7 100644 --- a/packages/server/src/sdk/app/rows/search/internal/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/internal/sqs.ts @@ -495,6 +495,6 @@ export async function search( if (err.status === 400 && msg?.match(MISSING_COLUMN_REGEX)) { return { rows: [] } } - throw err + throw new Error(`Unable to search by SQL - ${msg}`, { cause: err }) } } From ae8a8645660518404dba217013627c126c2783a5 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 26 Sep 2024 15:51:21 +0100 Subject: [PATCH 20/28] Collapse duplicated key names. --- packages/server/src/sdk/app/rows/search/tests/utils.spec.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/server/src/sdk/app/rows/search/tests/utils.spec.ts b/packages/server/src/sdk/app/rows/search/tests/utils.spec.ts index daa658455b..33594a621c 100644 --- a/packages/server/src/sdk/app/rows/search/tests/utils.spec.ts +++ b/packages/server/src/sdk/app/rows/search/tests/utils.spec.ts @@ -48,7 +48,7 @@ describe.each([tableWithUserCol, tableWithUsersCol])( it("should be able to map ro_ to global user IDs", () => { const params: RowSearchParams = { - tableId: tableId, + tableId, query: { equal: { "1:user": userMedataId, @@ -61,7 +61,7 @@ describe.each([tableWithUserCol, tableWithUsersCol])( it("should handle array of user IDs", () => { const params: RowSearchParams = { - tableId: tableId, + tableId, query: { oneOf: { "1:user": [userMedataId, globalUserId], @@ -78,7 +78,7 @@ describe.each([tableWithUserCol, tableWithUsersCol])( it("shouldn't change any other input", () => { const email = "test@example.com" const params: RowSearchParams = { - tableId: tableId, + tableId, query: { equal: { "1:user": email, From 743140eeaee5ea0d6b2269470e98d63411174d7f Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 26 Sep 2024 15:55:37 +0100 Subject: [PATCH 21/28] Update account-portal submodule to latest master. --- packages/account-portal | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/account-portal b/packages/account-portal index afe53b4be5..6c82625fdc 160000 --- a/packages/account-portal +++ b/packages/account-portal @@ -1 +1 @@ -Subproject commit afe53b4be55ac6f93dc9486596835412b2673f90 +Subproject commit 6c82625fdca3318e9fdbb61b8c2e9e78e5f8ace9 From aa738659aea8b8043a3dd834072448446b261f19 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 26 Sep 2024 16:21:34 +0100 Subject: [PATCH 22/28] Respond to PR feedback. --- .../src/api/controllers/row/utils/basic.ts | 30 +++++++++---------- packages/server/src/sdk/app/rows/internal.ts | 1 - 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/packages/server/src/api/controllers/row/utils/basic.ts b/packages/server/src/api/controllers/row/utils/basic.ts index 23565670ba..a7aa82e1f2 100644 --- a/packages/server/src/api/controllers/row/utils/basic.ts +++ b/packages/server/src/api/controllers/row/utils/basic.ts @@ -134,22 +134,20 @@ export async function basicProcessing({ } let columns: string[] = Object.keys(table.schema) - if (!isCalculationView) { - if (!sqs) { - thisRow._id = generateIdForRow(row, table, isLinked) - thisRow.tableId = table._id - thisRow._rev = "rev" - columns = columns.concat(PROTECTED_EXTERNAL_COLUMNS) - } else { - columns = columns.concat(PROTECTED_EXTERNAL_COLUMNS) - for (let internalColumn of [...PROTECTED_INTERNAL_COLUMNS, ...columns]) { - thisRow[internalColumn] = extractFieldValue({ - row, - tableName: table._id!, - fieldName: internalColumn, - isLinked, - }) - } + if (!sqs && !isCalculationView) { + thisRow._id = generateIdForRow(row, table, isLinked) + thisRow.tableId = table._id + thisRow._rev = "rev" + columns = columns.concat(PROTECTED_EXTERNAL_COLUMNS) + } else if (!isCalculationView) { + columns = columns.concat(PROTECTED_EXTERNAL_COLUMNS) + for (let internalColumn of [...PROTECTED_INTERNAL_COLUMNS, ...columns]) { + thisRow[internalColumn] = extractFieldValue({ + row, + tableName: table._id!, + fieldName: internalColumn, + isLinked, + }) } } for (let col of columns) { diff --git a/packages/server/src/sdk/app/rows/internal.ts b/packages/server/src/sdk/app/rows/internal.ts index 997af3c907..9306609132 100644 --- a/packages/server/src/sdk/app/rows/internal.ts +++ b/packages/server/src/sdk/app/rows/internal.ts @@ -1,7 +1,6 @@ import { context, db } from "@budibase/backend-core" import { Row, Table, ViewV2 } from "@budibase/types" import sdk from "../../../sdk" -import cloneDeep from "lodash/fp/cloneDeep" import { finaliseRow } from "../../../api/controllers/row/staticFormula" import { inputProcessing, From 8dd21e55929ec1968cf89ec9fcb36ac930513e02 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 26 Sep 2024 17:06:51 +0100 Subject: [PATCH 23/28] Wider check on fields == null. --- packages/server/src/sdk/app/rows/queryUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/sdk/app/rows/queryUtils.ts b/packages/server/src/sdk/app/rows/queryUtils.ts index a88763215e..7ef776a989 100644 --- a/packages/server/src/sdk/app/rows/queryUtils.ts +++ b/packages/server/src/sdk/app/rows/queryUtils.ts @@ -110,7 +110,7 @@ export const getQueryableFields = async ( "_id", // Querying by _id is always allowed, even if it's never part of the schema ] - if (fields === undefined) { + if (fields == null) { fields = Object.keys(table.schema) } result.push(...(await extractTableFields(table, fields, [table._id!]))) From d7ffdf02c27c9fcabc6f85c094471ee2e36834e4 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 26 Sep 2024 17:10:53 +0100 Subject: [PATCH 24/28] Update isTable and isView to depend on the ID format. --- packages/server/src/sdk/app/tables/utils.ts | 3 ++- packages/server/src/sdk/app/views/index.ts | 6 ++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/server/src/sdk/app/tables/utils.ts b/packages/server/src/sdk/app/tables/utils.ts index 7a8096fb0a..96499db00a 100644 --- a/packages/server/src/sdk/app/tables/utils.ts +++ b/packages/server/src/sdk/app/tables/utils.ts @@ -1,5 +1,6 @@ import { Table, TableSourceType } from "@budibase/types" import { isExternalTableID } from "../../../integrations/utils" +import { docIds } from "@budibase/backend-core" export function isExternal(opts: { table?: Table; tableId?: string }): boolean { if (opts.table && opts.table.sourceType === TableSourceType.EXTERNAL) { @@ -11,5 +12,5 @@ export function isExternal(opts: { table?: Table; tableId?: string }): boolean { } export function isTable(table: any): table is Table { - return table.type === "table" + return table._id && docIds.isTableId(table._id) } diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index 4251383712..4aea36fa2a 100644 --- a/packages/server/src/sdk/app/views/index.ts +++ b/packages/server/src/sdk/app/views/index.ts @@ -9,7 +9,7 @@ import { ViewV2ColumnEnriched, ViewV2Enriched, } from "@budibase/types" -import { context, HTTPError } from "@budibase/backend-core" +import { context, docIds, HTTPError } from "@budibase/backend-core" import { helpers, PROTECTED_EXTERNAL_COLUMNS, @@ -53,9 +53,7 @@ export async function getTable(view: string | ViewV2): Promise
{ } export function isView(view: any): view is ViewV2 { - return ( - view.version === 2 && "id" in view && "tableId" in view && "name" in view - ) + return view._id && docIds.isViewId(view._id) && view.version === 2 } async function guardCalculationViewSchema( From 559988e01134258e1954aeae5efa1326defea35f Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 26 Sep 2024 17:11:50 +0100 Subject: [PATCH 25/28] Correct error message. --- packages/server/src/sdk/app/rows/search.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/sdk/app/rows/search.ts b/packages/server/src/sdk/app/rows/search.ts index cb7397689a..809bd73d1f 100644 --- a/packages/server/src/sdk/app/rows/search.ts +++ b/packages/server/src/sdk/app/rows/search.ts @@ -83,7 +83,7 @@ export async function search( table = source options = searchInputMapping(table, options) } else { - throw new Error(`Invalid source ID: ${options.viewId || options.tableId}`) + throw new Error(`Must supply either a view ID or a table ID`) } if (options.query) { From 264b10f3f398a5e66189f5503f8802b0cca0f141 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 26 Sep 2024 17:22:11 +0100 Subject: [PATCH 26/28] Fix isView. --- packages/server/src/sdk/app/views/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/sdk/app/views/index.ts b/packages/server/src/sdk/app/views/index.ts index 4aea36fa2a..cd0fdf077e 100644 --- a/packages/server/src/sdk/app/views/index.ts +++ b/packages/server/src/sdk/app/views/index.ts @@ -53,7 +53,7 @@ export async function getTable(view: string | ViewV2): Promise
{ } export function isView(view: any): view is ViewV2 { - return view._id && docIds.isViewId(view._id) && view.version === 2 + return view.id && docIds.isViewId(view.id) && view.version === 2 } async function guardCalculationViewSchema( From 5919f1becdaf5745f5662eba202219bb0151e367 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Fri, 27 Sep 2024 09:57:56 +0100 Subject: [PATCH 27/28] Update account-portal submodule to latest master. --- packages/account-portal | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/account-portal b/packages/account-portal index 6c82625fdc..ef8690c955 160000 --- a/packages/account-portal +++ b/packages/account-portal @@ -1 +1 @@ -Subproject commit 6c82625fdca3318e9fdbb61b8c2e9e78e5f8ace9 +Subproject commit ef8690c955cb83768f21770ece72d68cccddc8a9 From b2545a30e1850a981394c627fd6520ef1d05a03a Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 30 Sep 2024 11:51:28 +0100 Subject: [PATCH 28/28] Update account-portal submodule to latest master. --- packages/account-portal | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/account-portal b/packages/account-portal index ef8690c955..3e24f6293f 160000 --- a/packages/account-portal +++ b/packages/account-portal @@ -1 +1 @@ -Subproject commit ef8690c955cb83768f21770ece72d68cccddc8a9 +Subproject commit 3e24f6293ff5ee5f9b42822e001504e3bbf19cc0