From 1851e11bc08fd50a1bbc603f4bc01e416e984b7b Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 8 Jul 2024 13:28:48 +0100 Subject: [PATCH 01/25] wip --- packages/backend-core/src/db/couch/utils.ts | 10 ++++++++++ packages/backend-core/src/environment.ts | 2 ++ packages/server/src/api/controllers/table/utils.ts | 12 ++++++------ packages/server/src/sdk/app/rows/search.ts | 4 ++-- packages/server/src/sdk/app/tables/getters.ts | 4 ++-- 5 files changed, 22 insertions(+), 10 deletions(-) diff --git a/packages/backend-core/src/db/couch/utils.ts b/packages/backend-core/src/db/couch/utils.ts index 270d953320..8502bd9ab6 100644 --- a/packages/backend-core/src/db/couch/utils.ts +++ b/packages/backend-core/src/db/couch/utils.ts @@ -1,6 +1,8 @@ import { getCouchInfo } from "./connections" import fetch from "node-fetch" import { checkSlashesInUrl } from "../../helpers" +import * as context from "../../context" +import env from "../../environment" export async function directCouchCall( path: string, @@ -53,3 +55,11 @@ export async function directCouchQuery( throw "Cannot connect to CouchDB instance" } } + +export function isSqsEnabledForTenant(): boolean { + const tenantId = context.getTenantId() + return ( + env.SQS_SEARCH_ENABLE !== undefined && + env.SQS_SEARCH_ENABLE_TENANTS.includes(tenantId) + ) +} diff --git a/packages/backend-core/src/environment.ts b/packages/backend-core/src/environment.ts index e06d51f918..06256239f1 100644 --- a/packages/backend-core/src/environment.ts +++ b/packages/backend-core/src/environment.ts @@ -116,6 +116,8 @@ const environment = { COUCH_DB_URL: process.env.COUCH_DB_URL || "http://localhost:4005", COUCH_DB_SQL_URL: process.env.COUCH_DB_SQL_URL || "http://localhost:4006", SQS_SEARCH_ENABLE: process.env.SQS_SEARCH_ENABLE, + SQS_SEARCH_ENABLE_TENANTS: + process.env.SQS_SEARCH_ENABLE_TENANTS?.split(",") || [], COUCH_DB_USERNAME: process.env.COUCH_DB_USER, COUCH_DB_PASSWORD: process.env.COUCH_DB_PASSWORD, GOOGLE_CLIENT_ID: process.env.GOOGLE_CLIENT_ID, diff --git a/packages/server/src/api/controllers/table/utils.ts b/packages/server/src/api/controllers/table/utils.ts index 0e9a32b294..5e5b14e6f2 100644 --- a/packages/server/src/api/controllers/table/utils.ts +++ b/packages/server/src/api/controllers/table/utils.ts @@ -15,7 +15,7 @@ import { getViews, saveView } from "../view/utils" import viewTemplate from "../view/viewBuilder" import { cloneDeep } from "lodash/fp" import { quotas } from "@budibase/pro" -import { events, context } from "@budibase/backend-core" +import { events, context, db } from "@budibase/backend-core" import { AutoFieldSubType, ContextUser, @@ -324,7 +324,7 @@ class TableSaveFunctions { importRows: this.importRows, user: this.user, }) - if (env.SQS_SEARCH_ENABLE) { + if (db.isSqsEnabledForTenant()) { await sdk.tables.sqs.addTable(table) } return table @@ -498,16 +498,16 @@ export function setStaticSchemas(datasource: Datasource, table: Table) { } export async function internalTableCleanup(table: Table, rows?: Row[]) { - const db = context.getAppDB() + const appDb = context.getAppDB() const tableId = table._id! // remove table search index if (!env.isTest() || env.COUCH_DB_URL) { - const currentIndexes = await db.getIndexes() + const currentIndexes = await appDb.getIndexes() const existingIndex = currentIndexes.indexes.find( (existing: any) => existing.name === `search:${tableId}` ) if (existingIndex) { - await db.deleteIndex(existingIndex) + await appDb.deleteIndex(existingIndex) } } @@ -518,7 +518,7 @@ export async function internalTableCleanup(table: Table, rows?: Row[]) { if (rows) { await AttachmentCleanup.tableDelete(table, rows) } - if (env.SQS_SEARCH_ENABLE) { + if (db.isSqsEnabledForTenant()) { await sdk.tables.sqs.removeTable(table) } } diff --git a/packages/server/src/sdk/app/rows/search.ts b/packages/server/src/sdk/app/rows/search.ts index 286a88054c..6ead244601 100644 --- a/packages/server/src/sdk/app/rows/search.ts +++ b/packages/server/src/sdk/app/rows/search.ts @@ -12,11 +12,11 @@ import * as internal from "./search/internal" import * as external from "./search/external" import { NoEmptyFilterStrings } from "../../../constants" import * as sqs from "./search/sqs" -import env from "../../../environment" import { ExportRowsParams, ExportRowsResult } from "./search/types" import { dataFilters } from "@budibase/shared-core" import sdk from "../../index" import { searchInputMapping } from "./search/utils" +import { db } from "@budibase/backend-core" export { isValidFilter } from "../../../integrations/utils" @@ -115,7 +115,7 @@ export async function search( if (isExternalTable) { return external.search(options, table) - } else if (env.SQS_SEARCH_ENABLE) { + } else if (db.isSqsEnabledForTenant()) { return sqs.search(options, table) } else { return internal.search(options, table) diff --git a/packages/server/src/sdk/app/tables/getters.ts b/packages/server/src/sdk/app/tables/getters.ts index 738e57eff8..a12933c966 100644 --- a/packages/server/src/sdk/app/tables/getters.ts +++ b/packages/server/src/sdk/app/tables/getters.ts @@ -1,4 +1,4 @@ -import { context } from "@budibase/backend-core" +import { context, db } from "@budibase/backend-core" import { getTableParams } from "../../../db/utils" import { breakExternalTableId, @@ -34,7 +34,7 @@ export function processTable(table: Table): Table { sourceId: table.sourceId || INTERNAL_TABLE_SOURCE_ID, sourceType: TableSourceType.INTERNAL, } - if (env.SQS_SEARCH_ENABLE) { + if (db.isSqsEnabledForTenant()) { processed.sql = !!env.SQS_SEARCH_ENABLE } return processed From 69d54b523dd08a33c401e13282fc1f3f22616fdd Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 8 Jul 2024 14:21:07 +0100 Subject: [PATCH 02/25] Finish implementation, fix tests. --- packages/backend-core/src/db/couch/utils.ts | 10 ---------- packages/backend-core/src/db/utils.ts | 18 ++++++++++++++++++ packages/backend-core/src/environment.ts | 2 ++ .../server/src/api/routes/tests/search.spec.ts | 2 +- .../src/api/routes/tests/templates.spec.ts | 4 +++- .../server/src/api/routes/tests/viewV2.spec.ts | 10 ++++++++-- .../server/src/appMigrations/migrations.ts | 2 +- .../migrations/20240604153647_initial_sqs.ts | 3 +-- .../tests/20240604153647_initial_sqs.spec.ts | 4 ++-- packages/server/src/environment.ts | 2 -- .../sdk/app/rows/search/tests/search.spec.ts | 11 +++++++++-- packages/server/src/sdk/app/tables/getters.ts | 3 +-- .../src/tests/utilities/TestConfiguration.ts | 10 +++++----- .../src/api/controllers/system/environment.ts | 2 +- packages/worker/src/api/index.ts | 3 +-- packages/worker/src/environment.ts | 1 - 16 files changed, 53 insertions(+), 34 deletions(-) diff --git a/packages/backend-core/src/db/couch/utils.ts b/packages/backend-core/src/db/couch/utils.ts index 8502bd9ab6..270d953320 100644 --- a/packages/backend-core/src/db/couch/utils.ts +++ b/packages/backend-core/src/db/couch/utils.ts @@ -1,8 +1,6 @@ import { getCouchInfo } from "./connections" import fetch from "node-fetch" import { checkSlashesInUrl } from "../../helpers" -import * as context from "../../context" -import env from "../../environment" export async function directCouchCall( path: string, @@ -55,11 +53,3 @@ export async function directCouchQuery( throw "Cannot connect to CouchDB instance" } } - -export function isSqsEnabledForTenant(): boolean { - const tenantId = context.getTenantId() - return ( - env.SQS_SEARCH_ENABLE !== undefined && - env.SQS_SEARCH_ENABLE_TENANTS.includes(tenantId) - ) -} diff --git a/packages/backend-core/src/db/utils.ts b/packages/backend-core/src/db/utils.ts index 906a95e1db..4362bd2c94 100644 --- a/packages/backend-core/src/db/utils.ts +++ b/packages/backend-core/src/db/utils.ts @@ -206,3 +206,21 @@ export function pagination( nextPage, } } + +export function isSqsEnabledForTenant(): boolean { + const tenantId = getTenantId() + if (!env.SQS_SEARCH_ENABLE) { + return false + } + + // This is to guard against the situation in tests where tests pass because + // we're not actually using SQS, we're using Lucene and the tests pass due to + // parity. + if (env.isTest() && env.SQS_SEARCH_ENABLE_TENANTS.length === 0) { + throw new Error( + "to enable SQS you must specify a list of tenants in the SQS_SEARCH_ENABLE_TENANTS env var" + ) + } + + return env.SQS_SEARCH_ENABLE_TENANTS.includes(tenantId) +} diff --git a/packages/backend-core/src/environment.ts b/packages/backend-core/src/environment.ts index 06256239f1..d3d4ab96ff 100644 --- a/packages/backend-core/src/environment.ts +++ b/packages/backend-core/src/environment.ts @@ -1,5 +1,6 @@ import { existsSync, readFileSync } from "fs" import { ServiceType } from "@budibase/types" +import { SQS } from "aws-sdk" function isTest() { return isJest() @@ -118,6 +119,7 @@ const environment = { SQS_SEARCH_ENABLE: process.env.SQS_SEARCH_ENABLE, SQS_SEARCH_ENABLE_TENANTS: process.env.SQS_SEARCH_ENABLE_TENANTS?.split(",") || [], + SQS_MIGRATION_ENABLE: process.env.SQS_MIGRATION_ENABLE, COUCH_DB_USERNAME: process.env.COUCH_DB_USER, COUCH_DB_PASSWORD: process.env.COUCH_DB_PASSWORD, GOOGLE_CLIENT_ID: process.env.GOOGLE_CLIENT_ID, diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index d9036b22fb..a64685d87f 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -55,7 +55,7 @@ describe.each([ beforeAll(async () => { if (isSqs) { - envCleanup = config.setEnv({ SQS_SEARCH_ENABLE: "true" }) + envCleanup = config.setCoreEnv({ SQS_SEARCH_ENABLE: "true" }) } await config.init() diff --git a/packages/server/src/api/routes/tests/templates.spec.ts b/packages/server/src/api/routes/tests/templates.spec.ts index 680ddb39d7..03f13ba88e 100644 --- a/packages/server/src/api/routes/tests/templates.spec.ts +++ b/packages/server/src/api/routes/tests/templates.spec.ts @@ -2,6 +2,7 @@ import * as setup from "./utilities" import path from "path" import nock from "nock" import { generator } from "@budibase/backend-core/tests" +import { SQS } from "aws-sdk" interface App { background: string @@ -86,9 +87,10 @@ describe("/templates", () => { async source => { const env = { SQS_SEARCH_ENABLE: source === "sqs" ? "true" : "false", + SQS_SEARCH_ENABLE_TENANTS: [config.getTenantId()], } - await config.withEnv(env, async () => { + await config.withCoreEnv(env, async () => { const name = generator.guid().replaceAll("-", "") const url = `/${name}` diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 43a6d39172..fcfeb74236 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -88,10 +88,16 @@ describe.each([ } beforeAll(async () => { + await config.withCoreEnv( + { SQS_SEARCH_ENABLE: isSqs ? "true" : "false" }, + () => config.init() + ) if (isSqs) { - envCleanup = config.setEnv({ SQS_SEARCH_ENABLE: "true" }) + envCleanup = config.setCoreEnv({ + SQS_SEARCH_ENABLE: "true", + SQS_SEARCH_ENABLE_TENANTS: [config.getTenantId()], + }) } - await config.init() if (dsProvider) { datasource = await config.createDatasource({ diff --git a/packages/server/src/appMigrations/migrations.ts b/packages/server/src/appMigrations/migrations.ts index 7437753ada..247b74ebfc 100644 --- a/packages/server/src/appMigrations/migrations.ts +++ b/packages/server/src/appMigrations/migrations.ts @@ -1,6 +1,6 @@ // This file should never be manually modified, use `yarn add-app-migration` in order to add a new one -import env from "../environment" +import { env } from "@budibase/backend-core" import { AppMigration } from "." import m20240604153647_initial_sqs from "./migrations/20240604153647_initial_sqs" diff --git a/packages/server/src/appMigrations/migrations/20240604153647_initial_sqs.ts b/packages/server/src/appMigrations/migrations/20240604153647_initial_sqs.ts index da435705c1..ec7fba99e9 100644 --- a/packages/server/src/appMigrations/migrations/20240604153647_initial_sqs.ts +++ b/packages/server/src/appMigrations/migrations/20240604153647_initial_sqs.ts @@ -1,8 +1,7 @@ -import { context } from "@budibase/backend-core" +import { context, env } from "@budibase/backend-core" import { allLinkDocs } from "../../db/utils" import LinkDocumentImpl from "../../db/linkedRows/LinkDocument" import sdk from "../../sdk" -import env from "../../environment" const migration = async () => { const linkDocs = await allLinkDocs() diff --git a/packages/server/src/appMigrations/migrations/tests/20240604153647_initial_sqs.spec.ts b/packages/server/src/appMigrations/migrations/tests/20240604153647_initial_sqs.spec.ts index 0a34fb2bb4..4301cb791f 100644 --- a/packages/server/src/appMigrations/migrations/tests/20240604153647_initial_sqs.spec.ts +++ b/packages/server/src/appMigrations/migrations/tests/20240604153647_initial_sqs.spec.ts @@ -69,11 +69,11 @@ function oldLinkDocument(): Omit { type SQSEnvVar = "SQS_MIGRATION_ENABLE" | "SQS_SEARCH_ENABLE" async function sqsDisabled(envVar: SQSEnvVar, cb: () => Promise) { - await config.withEnv({ [envVar]: "" }, cb) + await config.withCoreEnv({ [envVar]: "" }, cb) } async function sqsEnabled(envVar: SQSEnvVar, cb: () => Promise) { - await config.withEnv({ [envVar]: "1" }, cb) + await config.withCoreEnv({ [envVar]: "1" }, cb) } describe.each(["SQS_MIGRATION_ENABLE", "SQS_SEARCH_ENABLE"] as SQSEnvVar[])( diff --git a/packages/server/src/environment.ts b/packages/server/src/environment.ts index b643bd50b0..28673dcaa8 100644 --- a/packages/server/src/environment.ts +++ b/packages/server/src/environment.ts @@ -87,8 +87,6 @@ const environment = { SQL_MAX_ROWS: process.env.SQL_MAX_ROWS, SQL_LOGGING_ENABLE: process.env.SQL_LOGGING_ENABLE, SQL_ALIASING_DISABLE: process.env.SQL_ALIASING_DISABLE, - SQS_SEARCH_ENABLE: process.env.SQS_SEARCH_ENABLE, - SQS_MIGRATION_ENABLE: process.env.SQS_MIGRATION_ENABLE, // flags ALLOW_DEV_AUTOMATIONS: process.env.ALLOW_DEV_AUTOMATIONS, DISABLE_THREADING: process.env.DISABLE_THREADING, 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 2f347475f4..4b80d5bcd3 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 @@ -31,10 +31,17 @@ describe.each([ let rows: Row[] beforeAll(async () => { + await config.withCoreEnv( + { SQS_SEARCH_ENABLE: isSqs ? "true" : "false" }, + () => config.init() + ) + if (isSqs) { - envCleanup = config.setEnv({ SQS_SEARCH_ENABLE: "true" }) + envCleanup = config.setCoreEnv({ + SQS_SEARCH_ENABLE: "true", + SQS_SEARCH_ENABLE_TENANTS: [config.getTenantId()], + }) } - await config.init() if (dsProvider) { datasource = await config.createDatasource({ diff --git a/packages/server/src/sdk/app/tables/getters.ts b/packages/server/src/sdk/app/tables/getters.ts index a12933c966..a775a6d37e 100644 --- a/packages/server/src/sdk/app/tables/getters.ts +++ b/packages/server/src/sdk/app/tables/getters.ts @@ -1,4 +1,4 @@ -import { context, db } from "@budibase/backend-core" +import { context, db, env } from "@budibase/backend-core" import { getTableParams } from "../../../db/utils" import { breakExternalTableId, @@ -15,7 +15,6 @@ import { } from "@budibase/types" import datasources from "../datasources" import sdk from "../../../sdk" -import env from "../../../environment" export function processTable(table: Table): Table { if (!table) { diff --git a/packages/server/src/tests/utilities/TestConfiguration.ts b/packages/server/src/tests/utilities/TestConfiguration.ts index 793bfa8c6a..828b389add 100644 --- a/packages/server/src/tests/utilities/TestConfiguration.ts +++ b/packages/server/src/tests/utilities/TestConfiguration.ts @@ -245,10 +245,10 @@ export default class TestConfiguration { } } - async withEnv(newEnvVars: Partial, f: () => Promise) { + async withEnv(newEnvVars: Partial, f: () => Promise) { let cleanup = this.setEnv(newEnvVars) try { - await f() + return await f() } finally { cleanup() } @@ -273,13 +273,13 @@ export default class TestConfiguration { } } - async withCoreEnv( + async withCoreEnv( newEnvVars: Partial, - f: () => Promise + f: () => Promise ) { let cleanup = this.setCoreEnv(newEnvVars) try { - await f() + return await f() } finally { cleanup() } diff --git a/packages/worker/src/api/controllers/system/environment.ts b/packages/worker/src/api/controllers/system/environment.ts index 4deca5df6e..788b45f25d 100644 --- a/packages/worker/src/api/controllers/system/environment.ts +++ b/packages/worker/src/api/controllers/system/environment.ts @@ -24,7 +24,7 @@ async function isSqsAvailable() { } async function isSqsMissing() { - return env.SQS_SEARCH_ENABLE && !(await isSqsAvailable()) + return coreEnv.SQS_SEARCH_ENABLE && !(await isSqsAvailable()) } export const fetch = async (ctx: Ctx) => { diff --git a/packages/worker/src/api/index.ts b/packages/worker/src/api/index.ts index 08c65b98d4..8ab093a359 100644 --- a/packages/worker/src/api/index.ts +++ b/packages/worker/src/api/index.ts @@ -5,8 +5,7 @@ const compress = require("koa-compress") import zlib from "zlib" import { routes } from "./routes" import { middleware as pro, sdk } from "@budibase/pro" -import { auth, middleware } from "@budibase/backend-core" -import env from "../environment" +import { auth, middleware, env } from "@budibase/backend-core" if (env.SQS_SEARCH_ENABLE) { sdk.auditLogs.useSQLSearch() diff --git a/packages/worker/src/environment.ts b/packages/worker/src/environment.ts index d642d50846..9f7baf9e9b 100644 --- a/packages/worker/src/environment.ts +++ b/packages/worker/src/environment.ts @@ -46,7 +46,6 @@ const environment = { DISABLE_ACCOUNT_PORTAL: process.env.DISABLE_ACCOUNT_PORTAL, SMTP_FALLBACK_ENABLED: process.env.SMTP_FALLBACK_ENABLED, DISABLE_DEVELOPER_LICENSE: process.env.DISABLE_DEVELOPER_LICENSE, - SQS_SEARCH_ENABLE: process.env.SQS_SEARCH_ENABLE, BUDIBASE_ENVIRONMENT: process.env.BUDIBASE_ENVIRONMENT, // smtp SMTP_USER: process.env.SMTP_USER, From 014ff81841a2cac60b31221dd7c7ae3678c1face Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 8 Jul 2024 14:25:05 +0100 Subject: [PATCH 03/25] Remove accidental imports of the aws-sdk SQS (fml) --- packages/backend-core/src/environment.ts | 1 - packages/server/src/api/routes/tests/templates.spec.ts | 1 - 2 files changed, 2 deletions(-) diff --git a/packages/backend-core/src/environment.ts b/packages/backend-core/src/environment.ts index d3d4ab96ff..8ed96b32c3 100644 --- a/packages/backend-core/src/environment.ts +++ b/packages/backend-core/src/environment.ts @@ -1,6 +1,5 @@ import { existsSync, readFileSync } from "fs" import { ServiceType } from "@budibase/types" -import { SQS } from "aws-sdk" function isTest() { return isJest() diff --git a/packages/server/src/api/routes/tests/templates.spec.ts b/packages/server/src/api/routes/tests/templates.spec.ts index 03f13ba88e..81c615487d 100644 --- a/packages/server/src/api/routes/tests/templates.spec.ts +++ b/packages/server/src/api/routes/tests/templates.spec.ts @@ -2,7 +2,6 @@ import * as setup from "./utilities" import path from "path" import nock from "nock" import { generator } from "@budibase/backend-core/tests" -import { SQS } from "aws-sdk" interface App { background: string From 72a0364ca0a5c92170bf128866b2ee5f53c3e8f9 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 8 Jul 2024 15:02:26 +0100 Subject: [PATCH 04/25] Fix search tests. --- packages/server/src/api/routes/tests/search.spec.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index a64685d87f..785ac1498d 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -54,10 +54,13 @@ describe.each([ let rows: Row[] beforeAll(async () => { + await config.withCoreEnv({ SQS_SEARCH_ENABLE: "true" }, () => config.init()) if (isSqs) { - envCleanup = config.setCoreEnv({ SQS_SEARCH_ENABLE: "true" }) + envCleanup = config.setCoreEnv({ + SQS_SEARCH_ENABLE: "true", + SQS_SEARCH_ENABLE_TENANTS: [config.getTenantId()], + }) } - await config.init() if (config.app?.appId) { config.app = await config.api.application.update(config.app?.appId, { From b9ac15296ab3fdcd87ca0a2890bf1f3c576c7e6c Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 8 Jul 2024 17:00:33 +0100 Subject: [PATCH 05/25] Fix migration test. --- .../migrations/tests/20240604153647_initial_sqs.spec.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/server/src/appMigrations/migrations/tests/20240604153647_initial_sqs.spec.ts b/packages/server/src/appMigrations/migrations/tests/20240604153647_initial_sqs.spec.ts index 4301cb791f..f337958a5b 100644 --- a/packages/server/src/appMigrations/migrations/tests/20240604153647_initial_sqs.spec.ts +++ b/packages/server/src/appMigrations/migrations/tests/20240604153647_initial_sqs.spec.ts @@ -69,11 +69,14 @@ function oldLinkDocument(): Omit { type SQSEnvVar = "SQS_MIGRATION_ENABLE" | "SQS_SEARCH_ENABLE" async function sqsDisabled(envVar: SQSEnvVar, cb: () => Promise) { - await config.withCoreEnv({ [envVar]: "" }, cb) + await config.withCoreEnv({ [envVar]: "", SQS_SEARCH_ENABLE_TENANTS: [] }, cb) } async function sqsEnabled(envVar: SQSEnvVar, cb: () => Promise) { - await config.withCoreEnv({ [envVar]: "1" }, cb) + await config.withCoreEnv( + { [envVar]: "1", SQS_SEARCH_ENABLE_TENANTS: [config.getTenantId()] }, + cb + ) } describe.each(["SQS_MIGRATION_ENABLE", "SQS_SEARCH_ENABLE"] as SQSEnvVar[])( From b5f0619c89f4412fe5167855ef2737333441888b Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 8 Jul 2024 17:43:05 +0100 Subject: [PATCH 06/25] Fix attachments test. --- packages/server/src/sdk/tests/attachments.spec.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/server/src/sdk/tests/attachments.spec.ts b/packages/server/src/sdk/tests/attachments.spec.ts index 12c808d3a5..5656a1b608 100644 --- a/packages/server/src/sdk/tests/attachments.spec.ts +++ b/packages/server/src/sdk/tests/attachments.spec.ts @@ -39,7 +39,9 @@ describe("should be able to re-write attachment URLs", () => { } const db = dbCore.getDB(config.getAppId()) - await sdk.backups.updateAttachmentColumns(db.name, db) + await config.doInContext(config.getAppId(), () => + sdk.backups.updateAttachmentColumns(db.name, db) + ) return { db, From c0b85c63797d7bacb59f75f458eef5f3db090871 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 8 Jul 2024 18:42:11 +0100 Subject: [PATCH 07/25] Initial implementation - needs testing. --- .../src/api/controllers/row/utils/sqlUtils.ts | 16 +++++- .../server/src/sdk/app/rows/search/sqs.ts | 55 ++++++++++++++----- packages/server/src/sdk/app/rows/sqlAlias.ts | 3 +- 3 files changed, 59 insertions(+), 15 deletions(-) diff --git a/packages/server/src/api/controllers/row/utils/sqlUtils.ts b/packages/server/src/api/controllers/row/utils/sqlUtils.ts index 767916616c..76cf4d01ff 100644 --- a/packages/server/src/api/controllers/row/utils/sqlUtils.ts +++ b/packages/server/src/api/controllers/row/utils/sqlUtils.ts @@ -22,6 +22,18 @@ export function isManyToMany( return !!(field as ManyToManyRelationshipFieldMetadata).through } +function isCorrectRelationship( + relationship: RelationshipsJson, + row: Row +): boolean { + const junctionTableId = relationship.through! + const possibleColumns = [ + `${junctionTableId}.doc1.fieldName`, + `${junctionTableId}.doc2.fieldName`, + ] + return !!possibleColumns.find(col => row[col] === relationship.column) +} + /** * This iterates through the returned rows and works out what elements of the rows * actually match up to another row (based on primary keys) - this is pretty specific @@ -64,7 +76,9 @@ export async function updateRelationshipColumns( if (!linked._id) { continue } - columns[relationship.column] = linked + if (opts?.sqs && isCorrectRelationship(relationship, row)) { + columns[relationship.column] = linked + } } for (let [column, related] of Object.entries(columns)) { if (!row._id) { diff --git a/packages/server/src/sdk/app/rows/search/sqs.ts b/packages/server/src/sdk/app/rows/search/sqs.ts index 72a1557cc9..b7af8705a2 100644 --- a/packages/server/src/sdk/app/rows/search/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/sqs.ts @@ -5,6 +5,7 @@ import { Operation, QueryJson, RelationshipFieldMetadata, + RelationshipsJson, Row, RowSearchParams, SearchFilters, @@ -30,7 +31,10 @@ import { SQLITE_DESIGN_DOC_ID, SQS_DATASOURCE_INTERNAL, } from "@budibase/backend-core" -import { CONSTANT_INTERNAL_ROW_COLS } from "../../../../db/utils" +import { + CONSTANT_INTERNAL_ROW_COLS, + generateJunctionTableID, +} from "../../../../db/utils" import AliasTables from "../sqlAlias" import { outputProcessing } from "../../../../utilities/rowProcessor" import pick from "lodash/pick" @@ -52,7 +56,7 @@ const USER_COLUMN_PREFIX_REGEX = new RegExp( function buildInternalFieldList( table: Table, tables: Table[], - opts: { relationships: boolean } = { relationships: true } + opts?: { relationships?: RelationshipsJson[] } ) { let fieldList: string[] = [] fieldList = fieldList.concat( @@ -60,20 +64,31 @@ function buildInternalFieldList( ) for (let col of Object.values(table.schema)) { const isRelationship = col.type === FieldType.LINK - if (!opts.relationships && isRelationship) { + if (!opts?.relationships && isRelationship) { continue } if (isRelationship) { const linkCol = col as RelationshipFieldMetadata const relatedTable = tables.find(table => table._id === linkCol.tableId)! - fieldList = fieldList.concat( - buildInternalFieldList(relatedTable, tables, { relationships: false }) + // no relationships provided, don't go more than a layer deep + fieldList = fieldList.concat(buildInternalFieldList(relatedTable, tables)) + fieldList.push( + `${generateJunctionTableID( + table._id!, + relatedTable._id! + )}.doc1.fieldName` + ) + fieldList.push( + `${generateJunctionTableID( + table._id!, + relatedTable._id! + )}.doc2.fieldName` ) } else { fieldList.push(`${table._id}.${mapToUserColumn(col.name)}`) } } - return fieldList + return [...new Set(fieldList)] } function cleanupFilters( @@ -165,18 +180,27 @@ function reverseUserColumnMapping(rows: Row[]) { }) } -function runSqlQuery(json: QueryJson, tables: Table[]): Promise function runSqlQuery( json: QueryJson, tables: Table[], + relationships: RelationshipsJson[] +): Promise +function runSqlQuery( + json: QueryJson, + tables: Table[], + relationships: RelationshipsJson[], opts: { countTotalRows: true } ): Promise async function runSqlQuery( json: QueryJson, tables: Table[], + relationships: RelationshipsJson[], opts?: { countTotalRows?: boolean } ) { - const alias = new AliasTables(tables.map(table => table.name)) + const relationshipJunctionTableIds = relationships.map(rel => rel.through!) + const alias = new AliasTables( + tables.map(table => table.name).concat(relationshipJunctionTableIds) + ) if (opts?.countTotalRows) { json.endpoint.operation = Operation.COUNT } @@ -193,8 +217,13 @@ async function runSqlQuery( let bindings = query.bindings // quick hack for docIds - sql = sql.replace(/`doc1`.`rowId`/g, "`doc1.rowId`") - sql = sql.replace(/`doc2`.`rowId`/g, "`doc2.rowId`") + + const fixJunctionDocs = (field: string) => + ["doc1", "doc2"].forEach(doc => { + sql = sql.replaceAll(`\`${doc}\`.\`${field}\``, `\`${doc}.${field}\``) + }) + fixJunctionDocs("rowId") + fixJunctionDocs("fieldName") if (Array.isArray(query)) { throw new Error("SQS cannot currently handle multiple queries") @@ -260,7 +289,7 @@ export async function search( columnPrefix: USER_COLUMN_PREFIX, }, resource: { - fields: buildInternalFieldList(table, allTables), + fields: buildInternalFieldList(table, allTables, { relationships }), }, relationships, } @@ -292,11 +321,11 @@ export async function search( try { const queries: Promise[] = [] - queries.push(runSqlQuery(request, allTables)) + queries.push(runSqlQuery(request, allTables, relationships)) if (options.countRows) { // get the total count of rows queries.push( - runSqlQuery(request, allTables, { + runSqlQuery(request, allTables, relationships, { countTotalRows: true, }) ) diff --git a/packages/server/src/sdk/app/rows/sqlAlias.ts b/packages/server/src/sdk/app/rows/sqlAlias.ts index bc8fc56d5e..664e64057b 100644 --- a/packages/server/src/sdk/app/rows/sqlAlias.ts +++ b/packages/server/src/sdk/app/rows/sqlAlias.ts @@ -111,7 +111,8 @@ export default class AliasTables { aliasField(field: string) { const tableNames = this.tableNames if (field.includes(".")) { - const [tableName, column] = field.split(".") + const [tableName, ...rest] = field.split(".") + const column = rest.join(".") const foundTableName = tableNames.find(name => { const idx = tableName.indexOf(name) if (idx === -1 || idx > 1) { From 4c6f7f25c27192564b9be2c606b554cdd9add581 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 9 Jul 2024 11:45:01 +0100 Subject: [PATCH 08/25] Fix bug in oneOf search. --- .../src/api/routes/tests/search.spec.ts | 52 +++++++++++++++++++ packages/server/src/sdk/app/rows/search.ts | 27 +--------- packages/shared-core/src/filters.ts | 37 +++++++++++-- 3 files changed, 87 insertions(+), 29 deletions(-) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index d9036b22fb..ab4c28a90d 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -780,6 +780,32 @@ describe.each([ it("fails to find nonexistent row", async () => { await expectQuery({ oneOf: { name: ["none"] } }).toFindNothing() }) + + it("can have multiple values for same column", async () => { + await expectQuery({ + oneOf: { + name: ["foo", "bar"], + }, + }).toContainExactly([{ name: "foo" }, { name: "bar" }]) + }) + + it("splits comma separated strings", async () => { + await expectQuery({ + oneOf: { + // @ts-ignore + name: "foo,bar", + }, + }).toContainExactly([{ name: "foo" }, { name: "bar" }]) + }) + + it("trims whitespace", async () => { + await expectQuery({ + oneOf: { + // @ts-ignore + name: "foo, bar", + }, + }).toContainExactly([{ name: "foo" }, { name: "bar" }]) + }) }) describe("fuzzy", () => { @@ -1002,6 +1028,32 @@ describe.each([ it("fails to find nonexistent row", async () => { await expectQuery({ oneOf: { age: [2] } }).toFindNothing() }) + + // I couldn't find a way to make this work in Lucene and given that + // we're getting rid of Lucene soon I wasn't inclined to spend time on + // it. + !isLucene && + it("can convert from a string", async () => { + await expectQuery({ + oneOf: { + // @ts-ignore + age: "1", + }, + }).toContainExactly([{ age: 1 }]) + }) + + // I couldn't find a way to make this work in Lucene and given that + // we're getting rid of Lucene soon I wasn't inclined to spend time on + // it. + !isLucene && + it("can find multiple values for same column", async () => { + await expectQuery({ + oneOf: { + // @ts-ignore + age: "1,10", + }, + }).toContainExactly([{ age: 1 }, { age: 10 }]) + }) }) describe("range", () => { diff --git a/packages/server/src/sdk/app/rows/search.ts b/packages/server/src/sdk/app/rows/search.ts index 286a88054c..e2ec743d0e 100644 --- a/packages/server/src/sdk/app/rows/search.ts +++ b/packages/server/src/sdk/app/rows/search.ts @@ -66,37 +66,12 @@ export function removeEmptyFilters(filters: SearchFilters) { return filters } -// The frontend can send single values for array fields sometimes, so to handle -// this we convert them to arrays at the controller level so that nothing below -// this has to worry about the non-array values. -function fixupFilterArrays(filters: SearchFilters) { - const arrayFields = [ - SearchFilterOperator.ONE_OF, - SearchFilterOperator.CONTAINS, - SearchFilterOperator.NOT_CONTAINS, - SearchFilterOperator.CONTAINS_ANY, - ] - for (const searchField of arrayFields) { - const field = filters[searchField] - if (field == null) { - continue - } - - for (const key of Object.keys(field)) { - if (!Array.isArray(field[key])) { - field[key] = [field[key]] - } - } - } - return filters -} - export async function search( options: RowSearchParams ): Promise> { const isExternalTable = isExternalTableID(options.tableId) options.query = removeEmptyFilters(options.query || {}) - options.query = fixupFilterArrays(options.query) + options.query = dataFilters.fixupFilterArrays(options.query) if ( !dataFilters.hasFilters(options.query) && options.query.onEmptyFilter === EmptyFilterOption.RETURN_NONE diff --git a/packages/shared-core/src/filters.ts b/packages/shared-core/src/filters.ts index 3c6901e195..d4d5918bbb 100644 --- a/packages/shared-core/src/filters.ts +++ b/packages/shared-core/src/filters.ts @@ -327,6 +327,35 @@ export const buildQuery = (filter: SearchFilter[]) => { return query } +// The frontend can send single values for array fields sometimes, so to handle +// this we convert them to arrays at the controller level so that nothing below +// this has to worry about the non-array values. +export function fixupFilterArrays(filters: SearchFilters) { + const arrayFields = [ + SearchFilterOperator.ONE_OF, + SearchFilterOperator.CONTAINS, + SearchFilterOperator.NOT_CONTAINS, + SearchFilterOperator.CONTAINS_ANY, + ] + for (const searchField of arrayFields) { + const field = filters[searchField] + if (field == null) { + continue + } + + for (const key of Object.keys(field)) { + if (!Array.isArray(field[key])) { + if (typeof field[key] !== "string") { + field[key] = [field[key]] + } else { + field[key] = field[key].split(",").map(x => x.trim()) + } + } + } + } + return filters +} + export const search = ( docs: Record[], query: RowSearchParams @@ -360,6 +389,7 @@ export const runQuery = (docs: Record[], query: SearchFilters) => { } query = cleanupQuery(query) + query = fixupFilterArrays(query) if ( !hasFilters(query) && @@ -528,9 +558,10 @@ export const runQuery = (docs: Record[], query: SearchFilters) => { (docValue: any, testValue: any) => { if (typeof testValue === "string") { testValue = testValue.split(",") - if (typeof docValue === "number") { - testValue = testValue.map((item: string) => parseFloat(item)) - } + } + + if (typeof docValue === "number") { + testValue = testValue.map((item: string) => parseFloat(item)) } if (!Array.isArray(testValue)) { From 102bd28980a266a5aa57a513609019cfd80a47f4 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 9 Jul 2024 11:52:20 +0100 Subject: [PATCH 09/25] Fix lint. --- packages/server/src/sdk/app/rows/search.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/server/src/sdk/app/rows/search.ts b/packages/server/src/sdk/app/rows/search.ts index e2ec743d0e..7bcd26806c 100644 --- a/packages/server/src/sdk/app/rows/search.ts +++ b/packages/server/src/sdk/app/rows/search.ts @@ -2,7 +2,6 @@ import { EmptyFilterOption, Row, RowSearchParams, - SearchFilterOperator, SearchFilters, SearchResponse, SortOrder, From cd192020429bda774d1d8e05b8a087c5f4569ea0 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 9 Jul 2024 13:39:49 +0100 Subject: [PATCH 10/25] Fix external relationships. --- packages/server/src/api/controllers/row/utils/sqlUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/api/controllers/row/utils/sqlUtils.ts b/packages/server/src/api/controllers/row/utils/sqlUtils.ts index 76cf4d01ff..0d64113788 100644 --- a/packages/server/src/api/controllers/row/utils/sqlUtils.ts +++ b/packages/server/src/api/controllers/row/utils/sqlUtils.ts @@ -76,7 +76,7 @@ export async function updateRelationshipColumns( if (!linked._id) { continue } - if (opts?.sqs && isCorrectRelationship(relationship, row)) { + if (!opts?.sqs || isCorrectRelationship(relationship, row)) { columns[relationship.column] = linked } } From 6e699a163d696ef251afc21b60730ae4a6388714 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 9 Jul 2024 16:32:35 +0100 Subject: [PATCH 11/25] Cleaning up how junction fields are added to query. --- .../server/src/sdk/app/rows/search/sqs.ts | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/packages/server/src/sdk/app/rows/search/sqs.ts b/packages/server/src/sdk/app/rows/search/sqs.ts index b7af8705a2..4745aee7fb 100644 --- a/packages/server/src/sdk/app/rows/search/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/sqs.ts @@ -59,6 +59,13 @@ function buildInternalFieldList( opts?: { relationships?: RelationshipsJson[] } ) { let fieldList: string[] = [] + const addJunctionFields = (relatedTable: Table, fields: string[]) => { + fields.forEach(field => { + fieldList.push( + `${generateJunctionTableID(table._id!, relatedTable._id!)}.${field}` + ) + }) + } fieldList = fieldList.concat( CONSTANT_INTERNAL_ROW_COLS.map(col => `${table._id}.${col}`) ) @@ -72,18 +79,7 @@ function buildInternalFieldList( const relatedTable = tables.find(table => table._id === linkCol.tableId)! // no relationships provided, don't go more than a layer deep fieldList = fieldList.concat(buildInternalFieldList(relatedTable, tables)) - fieldList.push( - `${generateJunctionTableID( - table._id!, - relatedTable._id! - )}.doc1.fieldName` - ) - fieldList.push( - `${generateJunctionTableID( - table._id!, - relatedTable._id! - )}.doc2.fieldName` - ) + addJunctionFields(relatedTable, ["doc1.fieldName", "doc2.fieldName"]) } else { fieldList.push(`${table._id}.${mapToUserColumn(col.name)}`) } From 4cb23759a3c3aeca2cc6a8a642c50c9948ea3356 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 9 Jul 2024 16:33:10 +0100 Subject: [PATCH 12/25] Removing tables and their related table definitions. --- .../server/src/sdk/app/tables/internal/sqs.ts | 21 ++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/packages/server/src/sdk/app/tables/internal/sqs.ts b/packages/server/src/sdk/app/tables/internal/sqs.ts index 2d49adf96e..706e0ca167 100644 --- a/packages/server/src/sdk/app/tables/internal/sqs.ts +++ b/packages/server/src/sdk/app/tables/internal/sqs.ts @@ -176,9 +176,24 @@ export async function addTable(table: Table) { export async function removeTable(table: Table) { const db = context.getAppDB() try { - const definition = await db.get(SQLITE_DESIGN_DOC_ID) - if (definition.sql?.tables?.[table._id!]) { - delete definition.sql.tables[table._id!] + let response = await Promise.all([ + tablesSdk.getAllInternalTables(), + db.get(SQLITE_DESIGN_DOC_ID), + ]) + const tables: Table[] = response[0], + definition: SQLiteDefinition = response[1] + const tableIds = tables + .map(tbl => tbl._id!) + .filter(id => !id.includes(table._id!)) + let cleanup = false + for (let tableKey of Object.keys(definition.sql?.tables || {})) { + // there are no tables matching anymore + if (!tableIds.find(id => tableKey.includes(id))) { + delete definition.sql.tables[tableKey] + cleanup = true + } + } + if (cleanup) { await db.put(definition) // make sure SQS is cleaned up, tables removed await db.sqlDiskCleanup() From 9e8a855d14616b6ba8fa20526f648ac4b1bad7b7 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Tue, 9 Jul 2024 19:09:01 +0100 Subject: [PATCH 13/25] Adding test case for separating columns to rows in same table. --- .../src/api/routes/tests/search.spec.ts | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index d9036b22fb..9c76d90e24 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2085,6 +2085,58 @@ describe.each([ }) }) + isInternal && + describe("relations to same table", () => { + let relatedTable: Table, relatedRows: Row[] + + beforeAll(async () => { + relatedTable = await createTable( + { + name: { name: "name", type: FieldType.STRING }, + }, + "productCategory" + ) + table = await createTable({ + name: { name: "name", type: FieldType.STRING }, + related1: { + type: FieldType.LINK, + name: "related1", + fieldName: "main1", + tableId: relatedTable._id!, + relationshipType: RelationshipType.MANY_TO_MANY, + }, + related2: { + type: FieldType.LINK, + name: "related2", + fieldName: "main2", + tableId: relatedTable._id!, + relationshipType: RelationshipType.MANY_TO_MANY, + }, + }) + relatedRows = await Promise.all([ + config.api.row.save(relatedTable._id!, { name: "foo" }), + config.api.row.save(relatedTable._id!, { name: "bar" }), + ]) + await config.api.row.save(table._id!, { + name: "test", + related1: [relatedRows[0]._id!], + related2: [relatedRows[1]._id!], + }) + }) + + it("should be able to relate to same table", async () => { + await expectSearch({ + query: {}, + }).toContainExactly([ + { + name: "test", + related1: [{ _id: relatedRows[0]._id }], + related2: [{ _id: relatedRows[1]._id }], + }, + ]) + }) + }) + isInternal && describe("no column error backwards compat", () => { beforeAll(async () => { From b9c32b7068d65fdac1aefdea05a63bc0032f5415 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 10 Jul 2024 09:14:15 +0100 Subject: [PATCH 14/25] Fix tests? --- packages/shared-core/src/filters.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/shared-core/src/filters.ts b/packages/shared-core/src/filters.ts index d4d5918bbb..8298f7929a 100644 --- a/packages/shared-core/src/filters.ts +++ b/packages/shared-core/src/filters.ts @@ -348,7 +348,7 @@ export function fixupFilterArrays(filters: SearchFilters) { if (typeof field[key] !== "string") { field[key] = [field[key]] } else { - field[key] = field[key].split(",").map(x => x.trim()) + field[key] = field[key].split(",").map((x: string) => x.trim()) } } } From 114edc895482ed865776e0e0c2494dbc30745426 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 10 Jul 2024 10:50:05 +0100 Subject: [PATCH 15/25] Respond to PR feedback. --- packages/shared-core/src/filters.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/shared-core/src/filters.ts b/packages/shared-core/src/filters.ts index de4ee1897a..f7c7aa0922 100644 --- a/packages/shared-core/src/filters.ts +++ b/packages/shared-core/src/filters.ts @@ -18,7 +18,7 @@ import { import dayjs from "dayjs" import { OperatorOptions, SqlNumberTypeRangeMap } from "./constants" import { deepGet, schema } from "./helpers" -import _ from "lodash" +import { isPlainObject, isEmpty, isArray } from "lodash" const HBS_REGEX = /{{([^{].*?)}}/g @@ -335,7 +335,7 @@ export function fixupFilterArrays(filters: SearchFilters) { ] for (const searchField of arrayFields) { const field = filters[searchField] - if (field == null) { + if (field == null || !isPlainObject(field)) { continue } @@ -444,11 +444,11 @@ export const runQuery = (docs: Record[], query: SearchFilters) => { return false } - if (_.isObject(testValue.low) && _.isEmpty(testValue.low)) { + if (isPlainObject(testValue.low) && isEmpty(testValue.low)) { testValue.low = undefined } - if (_.isObject(testValue.high) && _.isEmpty(testValue.high)) { + if (isPlainObject(testValue.high) && isEmpty(testValue.high)) { testValue.high = undefined } From 4ab3aef02058f63f2eff700cd6822f472959bf12 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 10 Jul 2024 11:05:06 +0100 Subject: [PATCH 16/25] PR comments. --- .../src/api/controllers/row/utils/sqlUtils.ts | 9 +- .../src/api/routes/tests/search.spec.ts | 119 ++++++++++++------ .../server/src/sdk/app/tables/internal/sqs.ts | 4 +- 3 files changed, 91 insertions(+), 41 deletions(-) diff --git a/packages/server/src/api/controllers/row/utils/sqlUtils.ts b/packages/server/src/api/controllers/row/utils/sqlUtils.ts index 0d64113788..32124fa79d 100644 --- a/packages/server/src/api/controllers/row/utils/sqlUtils.ts +++ b/packages/server/src/api/controllers/row/utils/sqlUtils.ts @@ -24,9 +24,11 @@ export function isManyToMany( function isCorrectRelationship( relationship: RelationshipsJson, + table1: Table, + table2: Table, row: Row ): boolean { - const junctionTableId = relationship.through! + const junctionTableId = generateJunctionTableID(table1._id!, table2._id!) const possibleColumns = [ `${junctionTableId}.doc1.fieldName`, `${junctionTableId}.doc2.fieldName`, @@ -76,7 +78,10 @@ export async function updateRelationshipColumns( if (!linked._id) { continue } - if (!opts?.sqs || isCorrectRelationship(relationship, row)) { + if ( + !opts?.sqs || + isCorrectRelationship(relationship, table, linkedTable, row) + ) { columns[relationship.column] = linked } } diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 9c76d90e24..d27d0e677c 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2085,57 +2085,104 @@ describe.each([ }) }) - isInternal && - describe("relations to same table", () => { - let relatedTable: Table, relatedRows: Row[] + describe("relations to same table", () => { + let relatedTable: Table, relatedRows: Row[] - beforeAll(async () => { - relatedTable = await createTable( - { - name: { name: "name", type: FieldType.STRING }, - }, - "productCategory" - ) - table = await createTable({ + beforeAll(async () => { + relatedTable = await createTable( + { name: { name: "name", type: FieldType.STRING }, - related1: { - type: FieldType.LINK, - name: "related1", - fieldName: "main1", - tableId: relatedTable._id!, - relationshipType: RelationshipType.MANY_TO_MANY, - }, - related2: { - type: FieldType.LINK, - name: "related2", - fieldName: "main2", - tableId: relatedTable._id!, - relationshipType: RelationshipType.MANY_TO_MANY, - }, - }) - relatedRows = await Promise.all([ - config.api.row.save(relatedTable._id!, { name: "foo" }), - config.api.row.save(relatedTable._id!, { name: "bar" }), - ]) - await config.api.row.save(table._id!, { + }, + "productCategory" + ) + table = await createTable({ + name: { name: "name", type: FieldType.STRING }, + related1: { + type: FieldType.LINK, + name: "related1", + fieldName: "main1", + tableId: relatedTable._id!, + relationshipType: RelationshipType.MANY_TO_MANY, + }, + related2: { + type: FieldType.LINK, + name: "related2", + fieldName: "main2", + tableId: relatedTable._id!, + relationshipType: RelationshipType.MANY_TO_MANY, + }, + }) + relatedRows = await Promise.all([ + config.api.row.save(relatedTable._id!, { name: "foo" }), + config.api.row.save(relatedTable._id!, { name: "bar" }), + config.api.row.save(relatedTable._id!, { name: "baz" }), + config.api.row.save(relatedTable._id!, { name: "boo" }), + ]) + await Promise.all([ + config.api.row.save(table._id!, { name: "test", related1: [relatedRows[0]._id!], related2: [relatedRows[1]._id!], - }) + }), + config.api.row.save(table._id!, { + name: "test2", + related1: [relatedRows[2]._id!], + related2: [relatedRows[3]._id!], + }), + ]) + }) + + it("should be able to relate to same table", async () => { + await expectSearch({ + query: {}, + }).toContainExactly([ + { + name: "test", + related1: [{ _id: relatedRows[0]._id }], + related2: [{ _id: relatedRows[1]._id }], + }, + { + name: "test2", + related1: [{ _id: relatedRows[2]._id }], + related2: [{ _id: relatedRows[3]._id }], + }, + ]) + }) + + isSqs && + it("should be able to filter down to second row with equal", async () => { + await expectSearch({ + query: { + equal: { + ["related1.name"]: "baz", + }, + }, + }).toContainExactly([ + { + name: "test2", + related1: [{ _id: relatedRows[2]._id }], + }, + ]) }) - it("should be able to relate to same table", async () => { + isSqs && + it("should be able to filter down to first row with not equal", async () => { await expectSearch({ - query: {}, + query: { + notEqual: { + ["1:related2.name"]: "bar", + ["2:related2.name"]: "baz", + ["3:related2.name"]: "boo", + }, + }, }).toContainExactly([ { name: "test", related1: [{ _id: relatedRows[0]._id }], - related2: [{ _id: relatedRows[1]._id }], }, ]) }) - }) + }) isInternal && describe("no column error backwards compat", () => { diff --git a/packages/server/src/sdk/app/tables/internal/sqs.ts b/packages/server/src/sdk/app/tables/internal/sqs.ts index 706e0ca167..fc0ee8fc0b 100644 --- a/packages/server/src/sdk/app/tables/internal/sqs.ts +++ b/packages/server/src/sdk/app/tables/internal/sqs.ts @@ -176,12 +176,10 @@ export async function addTable(table: Table) { export async function removeTable(table: Table) { const db = context.getAppDB() try { - let response = await Promise.all([ + const [tables, definition] = await Promise.all([ tablesSdk.getAllInternalTables(), db.get(SQLITE_DESIGN_DOC_ID), ]) - const tables: Table[] = response[0], - definition: SQLiteDefinition = response[1] const tableIds = tables .map(tbl => tbl._id!) .filter(id => !id.includes(table._id!)) From 093579a34189a8c4dd30ffcaaf0ee83350d894fc Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 10 Jul 2024 11:05:16 +0100 Subject: [PATCH 17/25] Respond to PR feedback. --- packages/shared-core/src/filters.ts | 78 +++++++++++++---------------- packages/types/src/sdk/search.ts | 40 +++++++++------ 2 files changed, 58 insertions(+), 60 deletions(-) diff --git a/packages/shared-core/src/filters.ts b/packages/shared-core/src/filters.ts index f7c7aa0922..34e107562a 100644 --- a/packages/shared-core/src/filters.ts +++ b/packages/shared-core/src/filters.ts @@ -6,6 +6,7 @@ import { SearchFilter, SearchFilters, SearchQueryFields, + ArrayOperator, SearchFilterOperator, SortType, FieldConstraints, @@ -14,11 +15,13 @@ import { EmptyFilterOption, SearchResponse, Table, + BasicOperator, + RangeOperator, } from "@budibase/types" import dayjs from "dayjs" import { OperatorOptions, SqlNumberTypeRangeMap } from "./constants" import { deepGet, schema } from "./helpers" -import { isPlainObject, isEmpty, isArray } from "lodash" +import { isPlainObject, isEmpty } from "lodash" const HBS_REGEX = /{{([^{].*?)}}/g @@ -327,13 +330,7 @@ export const buildQuery = (filter: SearchFilter[]) => { // this we convert them to arrays at the controller level so that nothing below // this has to worry about the non-array values. export function fixupFilterArrays(filters: SearchFilters) { - const arrayFields = [ - SearchFilterOperator.ONE_OF, - SearchFilterOperator.CONTAINS, - SearchFilterOperator.NOT_CONTAINS, - SearchFilterOperator.CONTAINS_ANY, - ] - for (const searchField of arrayFields) { + for (const searchField of Object.values(ArrayOperator)) { const field = filters[searchField] if (field == null || !isPlainObject(field)) { continue @@ -341,10 +338,12 @@ export function fixupFilterArrays(filters: SearchFilters) { for (const key of Object.keys(field)) { if (!Array.isArray(field[key])) { - if (typeof field[key] !== "string") { - field[key] = [field[key]] + if (typeof field[key] === "string") { + field[key] = (field[key] as string) + .split(",") + .map((x: string) => x.trim()) } else { - field[key] = field[key].split(",").map((x: string) => x.trim()) + field[key] = [field[key]] } } } @@ -412,7 +411,7 @@ export const runQuery = (docs: Record[], query: SearchFilters) => { } const stringMatch = match( - SearchFilterOperator.STRING, + BasicOperator.STRING, (docValue: any, testValue: any) => { if (!(typeof docValue === "string")) { return false @@ -425,7 +424,7 @@ export const runQuery = (docs: Record[], query: SearchFilters) => { ) const fuzzyMatch = match( - SearchFilterOperator.FUZZY, + BasicOperator.FUZZY, (docValue: any, testValue: any) => { if (!(typeof docValue === "string")) { return false @@ -438,7 +437,7 @@ export const runQuery = (docs: Record[], query: SearchFilters) => { ) const rangeMatch = match( - SearchFilterOperator.RANGE, + RangeOperator.RANGE, (docValue: any, testValue: any) => { if (docValue == null || docValue === "") { return false @@ -527,11 +526,8 @@ export const runQuery = (docs: Record[], query: SearchFilters) => { (...args: T): boolean => !f(...args) - const equalMatch = match(SearchFilterOperator.EQUAL, _valueMatches) - const notEqualMatch = match( - SearchFilterOperator.NOT_EQUAL, - not(_valueMatches) - ) + const equalMatch = match(BasicOperator.EQUAL, _valueMatches) + const notEqualMatch = match(BasicOperator.NOT_EQUAL, not(_valueMatches)) const _empty = (docValue: any) => { if (typeof docValue === "string") { @@ -546,27 +542,24 @@ export const runQuery = (docs: Record[], query: SearchFilters) => { return docValue == null } - const emptyMatch = match(SearchFilterOperator.EMPTY, _empty) - const notEmptyMatch = match(SearchFilterOperator.NOT_EMPTY, not(_empty)) + const emptyMatch = match(BasicOperator.EMPTY, _empty) + const notEmptyMatch = match(BasicOperator.NOT_EMPTY, not(_empty)) - const oneOf = match( - SearchFilterOperator.ONE_OF, - (docValue: any, testValue: any) => { - if (typeof testValue === "string") { - testValue = testValue.split(",") - } - - if (typeof docValue === "number") { - testValue = testValue.map((item: string) => parseFloat(item)) - } - - if (!Array.isArray(testValue)) { - return false - } - - return testValue.some(item => _valueMatches(docValue, item)) + const oneOf = match(ArrayOperator.ONE_OF, (docValue: any, testValue: any) => { + if (typeof testValue === "string") { + testValue = testValue.split(",") } - ) + + if (typeof docValue === "number") { + testValue = testValue.map((item: string) => parseFloat(item)) + } + + if (!Array.isArray(testValue)) { + return false + } + + return testValue.some(item => _valueMatches(docValue, item)) + }) const _contains = (f: "some" | "every") => (docValue: any, testValue: any) => { @@ -593,7 +586,7 @@ export const runQuery = (docs: Record[], query: SearchFilters) => { } const contains = match( - SearchFilterOperator.CONTAINS, + ArrayOperator.CONTAINS, (docValue: any, testValue: any) => { if (Array.isArray(testValue) && testValue.length === 0) { return true @@ -602,7 +595,7 @@ export const runQuery = (docs: Record[], query: SearchFilters) => { } ) const notContains = match( - SearchFilterOperator.NOT_CONTAINS, + ArrayOperator.NOT_CONTAINS, (docValue: any, testValue: any) => { // Not sure if this is logically correct, but at the time this code was // written the search endpoint behaved this way and we wanted to make this @@ -613,10 +606,7 @@ export const runQuery = (docs: Record[], query: SearchFilters) => { return not(_contains("every"))(docValue, testValue) } ) - const containsAny = match( - SearchFilterOperator.CONTAINS_ANY, - _contains("some") - ) + const containsAny = match(ArrayOperator.CONTAINS_ANY, _contains("some")) const docMatch = (doc: Record) => { const filterFunctions = { diff --git a/packages/types/src/sdk/search.ts b/packages/types/src/sdk/search.ts index ccb73a7fba..5607efece8 100644 --- a/packages/types/src/sdk/search.ts +++ b/packages/types/src/sdk/search.ts @@ -3,20 +3,28 @@ import { Row, Table, DocumentType } from "../documents" import { SortOrder, SortType } from "../api" import { Knex } from "knex" -export enum SearchFilterOperator { - STRING = "string", - FUZZY = "fuzzy", - RANGE = "range", +export enum BasicOperator { EQUAL = "equal", NOT_EQUAL = "notEqual", EMPTY = "empty", NOT_EMPTY = "notEmpty", - ONE_OF = "oneOf", + FUZZY = "fuzzy", + STRING = "string", +} + +export enum ArrayOperator { CONTAINS = "contains", NOT_CONTAINS = "notContains", CONTAINS_ANY = "containsAny", + ONE_OF = "oneOf", } +export enum RangeOperator { + RANGE = "range", +} + +export type SearchFilterOperator = BasicOperator | ArrayOperator | RangeOperator + export enum InternalSearchFilterOperator { COMPLEX_ID_OPERATOR = "_complexIdOperator", } @@ -52,17 +60,17 @@ export interface SearchFilters { // allows just fuzzy to be or - all the fuzzy/like parameters fuzzyOr?: boolean onEmptyFilter?: EmptyFilterOption - [SearchFilterOperator.STRING]?: BasicFilter - [SearchFilterOperator.FUZZY]?: BasicFilter - [SearchFilterOperator.RANGE]?: RangeFilter - [SearchFilterOperator.EQUAL]?: BasicFilter - [SearchFilterOperator.NOT_EQUAL]?: BasicFilter - [SearchFilterOperator.EMPTY]?: BasicFilter - [SearchFilterOperator.NOT_EMPTY]?: BasicFilter - [SearchFilterOperator.ONE_OF]?: ArrayFilter - [SearchFilterOperator.CONTAINS]?: ArrayFilter - [SearchFilterOperator.NOT_CONTAINS]?: ArrayFilter - [SearchFilterOperator.CONTAINS_ANY]?: ArrayFilter + [BasicOperator.STRING]?: BasicFilter + [BasicOperator.FUZZY]?: BasicFilter + [RangeOperator.RANGE]?: RangeFilter + [BasicOperator.EQUAL]?: BasicFilter + [BasicOperator.NOT_EQUAL]?: BasicFilter + [BasicOperator.EMPTY]?: BasicFilter + [BasicOperator.NOT_EMPTY]?: BasicFilter + [ArrayOperator.ONE_OF]?: ArrayFilter + [ArrayOperator.CONTAINS]?: ArrayFilter + [ArrayOperator.NOT_CONTAINS]?: ArrayFilter + [ArrayOperator.CONTAINS_ANY]?: ArrayFilter // specific to SQS/SQLite search on internal tables this can be used // to make sure the documents returned are always filtered down to a // specific document type (such as just rows) From 5356cfdce5f9f674aa0c81ad7913cab7b3e33f66 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 10 Jul 2024 11:08:11 +0100 Subject: [PATCH 18/25] Fix uses of SearchFilterOperator. --- packages/backend-core/src/users/users.ts | 9 +++++---- packages/server/src/api/routes/tests/viewV2.spec.ts | 12 ++++++------ 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/packages/backend-core/src/users/users.ts b/packages/backend-core/src/users/users.ts index 7d62a6ef39..0c994d8287 100644 --- a/packages/backend-core/src/users/users.ts +++ b/packages/backend-core/src/users/users.ts @@ -18,9 +18,10 @@ import { CouchFindOptions, DatabaseQueryOpts, SearchFilters, - SearchFilterOperator, SearchUsersRequest, User, + BasicOperator, + ArrayOperator, } from "@budibase/types" import * as context from "../context" import { getGlobalDB } from "../context" @@ -46,9 +47,9 @@ function removeUserPassword(users: User | User[]) { export function isSupportedUserSearch(query: SearchFilters) { const allowed = [ - { op: SearchFilterOperator.STRING, key: "email" }, - { op: SearchFilterOperator.EQUAL, key: "_id" }, - { op: SearchFilterOperator.ONE_OF, key: "_id" }, + { op: BasicOperator.STRING, key: "email" }, + { op: BasicOperator.EQUAL, key: "_id" }, + { op: ArrayOperator.ONE_OF, key: "_id" }, ] for (let [key, operation] of Object.entries(query)) { if (typeof operation !== "object") { diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 43a6d39172..93ce912472 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -9,7 +9,6 @@ import { QuotaUsageType, Row, SaveTableRequest, - SearchFilterOperator, SortOrder, SortType, StaticQuotaName, @@ -19,6 +18,7 @@ import { ViewUIFieldMetadata, ViewV2, SearchResponse, + BasicOperator, } from "@budibase/types" import { generator, mocks } from "@budibase/backend-core/tests" import { DatabaseName, getDatasource } from "../../../integrations/tests/utils" @@ -149,7 +149,7 @@ describe.each([ primaryDisplay: "id", query: [ { - operator: SearchFilterOperator.EQUAL, + operator: BasicOperator.EQUAL, field: "field", value: "value", }, @@ -561,7 +561,7 @@ describe.each([ ...view, query: [ { - operator: SearchFilterOperator.EQUAL, + operator: BasicOperator.EQUAL, field: "newField", value: "thatValue", }, @@ -589,7 +589,7 @@ describe.each([ primaryDisplay: "Price", query: [ { - operator: SearchFilterOperator.EQUAL, + operator: BasicOperator.EQUAL, field: generator.word(), value: generator.word(), }, @@ -673,7 +673,7 @@ describe.each([ tableId: generator.guid(), query: [ { - operator: SearchFilterOperator.EQUAL, + operator: BasicOperator.EQUAL, field: "newField", value: "thatValue", }, @@ -1194,7 +1194,7 @@ describe.each([ name: generator.guid(), query: [ { - operator: SearchFilterOperator.EQUAL, + operator: BasicOperator.EQUAL, field: "two", value: "bar2", }, From 79b4d260f11cef0545c3ded050d8f74d4adf738f Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 10 Jul 2024 11:17:59 +0100 Subject: [PATCH 19/25] Fix more fucky wucky typey wipey stuff. --- .../src/components/FilterBuilder.svelte | 4 ++-- packages/shared-core/src/filters.ts | 17 +++++++++-------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/packages/frontend-core/src/components/FilterBuilder.svelte b/packages/frontend-core/src/components/FilterBuilder.svelte index c0bc328a4e..5b6b6b4c86 100644 --- a/packages/frontend-core/src/components/FilterBuilder.svelte +++ b/packages/frontend-core/src/components/FilterBuilder.svelte @@ -11,7 +11,7 @@ Label, Multiselect, } from "@budibase/bbui" - import { FieldType, SearchFilterOperator } from "@budibase/types" + import { ArrayOperator, FieldType } from "@budibase/types" import { generate } from "shortid" import { QueryUtils, Constants } from "@budibase/frontend-core" import { getContext } from "svelte" @@ -268,7 +268,7 @@ {:else if [FieldType.STRING, FieldType.LONGFORM, FieldType.NUMBER, FieldType.BIGINT, FieldType.FORMULA].includes(filter.type)} - {:else if filter.type === FieldType.ARRAY || (filter.type === FieldType.OPTIONS && filter.operator === SearchFilterOperator.ONE_OF)} + {:else if filter.type === FieldType.ARRAY || (filter.type === FieldType.OPTIONS && filter.operator === ArrayOperator.ONE_OF)} x.trim()) - } else { - field[key] = [field[key]] - } + if (Array.isArray(field[key])) { + continue + } + + const value = field[key] as any + if (typeof value === "string") { + field[key] = value.split(",").map((x: string) => x.trim()) + } else { + field[key] = [value] } } } From d6ad6a46860a3d17223bb6351cea152a95a82f1d Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Wed, 10 Jul 2024 11:21:41 +0100 Subject: [PATCH 20/25] Missing internal check. --- .../src/api/routes/tests/search.spec.ts | 169 +++++++++--------- 1 file changed, 85 insertions(+), 84 deletions(-) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index d27d0e677c..ce9ef8034b 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2085,104 +2085,105 @@ describe.each([ }) }) - describe("relations to same table", () => { - let relatedTable: Table, relatedRows: Row[] + isInternal && + describe("relations to same table", () => { + let relatedTable: Table, relatedRows: Row[] - beforeAll(async () => { - relatedTable = await createTable( - { - name: { name: "name", type: FieldType.STRING }, - }, - "productCategory" - ) - table = await createTable({ - name: { name: "name", type: FieldType.STRING }, - related1: { - type: FieldType.LINK, - name: "related1", - fieldName: "main1", - tableId: relatedTable._id!, - relationshipType: RelationshipType.MANY_TO_MANY, - }, - related2: { - type: FieldType.LINK, - name: "related2", - fieldName: "main2", - tableId: relatedTable._id!, - relationshipType: RelationshipType.MANY_TO_MANY, - }, - }) - relatedRows = await Promise.all([ - config.api.row.save(relatedTable._id!, { name: "foo" }), - config.api.row.save(relatedTable._id!, { name: "bar" }), - config.api.row.save(relatedTable._id!, { name: "baz" }), - config.api.row.save(relatedTable._id!, { name: "boo" }), - ]) - await Promise.all([ - config.api.row.save(table._id!, { - name: "test", - related1: [relatedRows[0]._id!], - related2: [relatedRows[1]._id!], - }), - config.api.row.save(table._id!, { - name: "test2", - related1: [relatedRows[2]._id!], - related2: [relatedRows[3]._id!], - }), - ]) - }) - - it("should be able to relate to same table", async () => { - await expectSearch({ - query: {}, - }).toContainExactly([ - { - name: "test", - related1: [{ _id: relatedRows[0]._id }], - related2: [{ _id: relatedRows[1]._id }], - }, - { - name: "test2", - related1: [{ _id: relatedRows[2]._id }], - related2: [{ _id: relatedRows[3]._id }], - }, - ]) - }) - - isSqs && - it("should be able to filter down to second row with equal", async () => { - await expectSearch({ - query: { - equal: { - ["related1.name"]: "baz", - }, - }, - }).toContainExactly([ + beforeAll(async () => { + relatedTable = await createTable( { - name: "test2", - related1: [{ _id: relatedRows[2]._id }], + name: { name: "name", type: FieldType.STRING }, }, + "productCategory" + ) + table = await createTable({ + name: { name: "name", type: FieldType.STRING }, + related1: { + type: FieldType.LINK, + name: "related1", + fieldName: "main1", + tableId: relatedTable._id!, + relationshipType: RelationshipType.MANY_TO_MANY, + }, + related2: { + type: FieldType.LINK, + name: "related2", + fieldName: "main2", + tableId: relatedTable._id!, + relationshipType: RelationshipType.MANY_TO_MANY, + }, + }) + relatedRows = await Promise.all([ + config.api.row.save(relatedTable._id!, { name: "foo" }), + config.api.row.save(relatedTable._id!, { name: "bar" }), + config.api.row.save(relatedTable._id!, { name: "baz" }), + config.api.row.save(relatedTable._id!, { name: "boo" }), + ]) + await Promise.all([ + config.api.row.save(table._id!, { + name: "test", + related1: [relatedRows[0]._id!], + related2: [relatedRows[1]._id!], + }), + config.api.row.save(table._id!, { + name: "test2", + related1: [relatedRows[2]._id!], + related2: [relatedRows[3]._id!], + }), ]) }) - isSqs && - it("should be able to filter down to first row with not equal", async () => { + it("should be able to relate to same table", async () => { await expectSearch({ - query: { - notEqual: { - ["1:related2.name"]: "bar", - ["2:related2.name"]: "baz", - ["3:related2.name"]: "boo", - }, - }, + query: {}, }).toContainExactly([ { name: "test", related1: [{ _id: relatedRows[0]._id }], + related2: [{ _id: relatedRows[1]._id }], + }, + { + name: "test2", + related1: [{ _id: relatedRows[2]._id }], + related2: [{ _id: relatedRows[3]._id }], }, ]) }) - }) + + isSqs && + it("should be able to filter down to second row with equal", async () => { + await expectSearch({ + query: { + equal: { + ["related1.name"]: "baz", + }, + }, + }).toContainExactly([ + { + name: "test2", + related1: [{ _id: relatedRows[2]._id }], + }, + ]) + }) + + isSqs && + it("should be able to filter down to first row with not equal", async () => { + await expectSearch({ + query: { + notEqual: { + ["1:related2.name"]: "bar", + ["2:related2.name"]: "baz", + ["3:related2.name"]: "boo", + }, + }, + }).toContainExactly([ + { + name: "test", + related1: [{ _id: relatedRows[0]._id }], + }, + ]) + }) + }) isInternal && describe("no column error backwards compat", () => { From d0c3de63eba8fd42e0fc986e9bde7ba0a9593c52 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 10 Jul 2024 11:25:53 +0100 Subject: [PATCH 21/25] Fix sqs image reference. --- .github/workflows/budibase_ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/budibase_ci.yml b/.github/workflows/budibase_ci.yml index 288a0462e7..d63596f08f 100644 --- a/.github/workflows/budibase_ci.yml +++ b/.github/workflows/budibase_ci.yml @@ -108,7 +108,7 @@ jobs: - name: Pull testcontainers images run: | docker pull testcontainers/ryuk:0.5.1 & - docker pull budibase/couchdb:v3.2.1-sql & + docker pull budibase/couchdb:v3.2.1-sqs & docker pull redis & wait $(jobs -p) From 76d22dfffd1f012dd2528da9770b91bda8e31e32 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 10 Jul 2024 11:32:34 +0100 Subject: [PATCH 22/25] Respond to PR comments. --- packages/server/src/api/controllers/table/utils.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/server/src/api/controllers/table/utils.ts b/packages/server/src/api/controllers/table/utils.ts index 5e5b14e6f2..7e2d1060eb 100644 --- a/packages/server/src/api/controllers/table/utils.ts +++ b/packages/server/src/api/controllers/table/utils.ts @@ -15,7 +15,7 @@ import { getViews, saveView } from "../view/utils" import viewTemplate from "../view/viewBuilder" import { cloneDeep } from "lodash/fp" import { quotas } from "@budibase/pro" -import { events, context, db } from "@budibase/backend-core" +import { events, context, db as dbCore } from "@budibase/backend-core" import { AutoFieldSubType, ContextUser, @@ -498,16 +498,16 @@ export function setStaticSchemas(datasource: Datasource, table: Table) { } export async function internalTableCleanup(table: Table, rows?: Row[]) { - const appDb = context.getAppDB() + const db = context.getAppDB() const tableId = table._id! // remove table search index if (!env.isTest() || env.COUCH_DB_URL) { - const currentIndexes = await appDb.getIndexes() + const currentIndexes = await db.getIndexes() const existingIndex = currentIndexes.indexes.find( (existing: any) => existing.name === `search:${tableId}` ) if (existingIndex) { - await appDb.deleteIndex(existingIndex) + await db.deleteIndex(existingIndex) } } @@ -518,7 +518,7 @@ export async function internalTableCleanup(table: Table, rows?: Row[]) { if (rows) { await AttachmentCleanup.tableDelete(table, rows) } - if (db.isSqsEnabledForTenant()) { + if (dbCore.isSqsEnabledForTenant()) { await sdk.tables.sqs.removeTable(table) } } From 5069d3f95392fb002473a089589dd4dc595183fa Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 10 Jul 2024 11:37:37 +0100 Subject: [PATCH 23/25] Rename db to dbCore in more places. --- packages/server/src/api/controllers/table/utils.ts | 2 +- packages/server/src/sdk/app/rows/search.ts | 4 ++-- packages/server/src/sdk/app/tables/getters.ts | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/server/src/api/controllers/table/utils.ts b/packages/server/src/api/controllers/table/utils.ts index 7e2d1060eb..e2036c8115 100644 --- a/packages/server/src/api/controllers/table/utils.ts +++ b/packages/server/src/api/controllers/table/utils.ts @@ -324,7 +324,7 @@ class TableSaveFunctions { importRows: this.importRows, user: this.user, }) - if (db.isSqsEnabledForTenant()) { + if (dbCore.isSqsEnabledForTenant()) { await sdk.tables.sqs.addTable(table) } return table diff --git a/packages/server/src/sdk/app/rows/search.ts b/packages/server/src/sdk/app/rows/search.ts index 6ead244601..b7d7889c94 100644 --- a/packages/server/src/sdk/app/rows/search.ts +++ b/packages/server/src/sdk/app/rows/search.ts @@ -16,7 +16,7 @@ import { ExportRowsParams, ExportRowsResult } from "./search/types" import { dataFilters } from "@budibase/shared-core" import sdk from "../../index" import { searchInputMapping } from "./search/utils" -import { db } from "@budibase/backend-core" +import { db as dbCore } from "@budibase/backend-core" export { isValidFilter } from "../../../integrations/utils" @@ -115,7 +115,7 @@ export async function search( if (isExternalTable) { return external.search(options, table) - } else if (db.isSqsEnabledForTenant()) { + } else if (dbCore.isSqsEnabledForTenant()) { return sqs.search(options, table) } else { return internal.search(options, table) diff --git a/packages/server/src/sdk/app/tables/getters.ts b/packages/server/src/sdk/app/tables/getters.ts index a775a6d37e..725c4e5cd2 100644 --- a/packages/server/src/sdk/app/tables/getters.ts +++ b/packages/server/src/sdk/app/tables/getters.ts @@ -1,4 +1,4 @@ -import { context, db, env } from "@budibase/backend-core" +import { context, db as dbCore, env } from "@budibase/backend-core" import { getTableParams } from "../../../db/utils" import { breakExternalTableId, @@ -33,7 +33,7 @@ export function processTable(table: Table): Table { sourceId: table.sourceId || INTERNAL_TABLE_SOURCE_ID, sourceType: TableSourceType.INTERNAL, } - if (db.isSqsEnabledForTenant()) { + if (dbCore.isSqsEnabledForTenant()) { processed.sql = !!env.SQS_SEARCH_ENABLE } return processed From 56c6742a0e70fb0b22c1a9109142fdb969aaa85a Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 10 Jul 2024 15:22:18 +0100 Subject: [PATCH 24/25] Create a way to enable SQS for all tenants. --- packages/backend-core/src/db/utils.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/backend-core/src/db/utils.ts b/packages/backend-core/src/db/utils.ts index 4362bd2c94..5aad05d3e3 100644 --- a/packages/backend-core/src/db/utils.ts +++ b/packages/backend-core/src/db/utils.ts @@ -222,5 +222,13 @@ export function isSqsEnabledForTenant(): boolean { ) } + // Special case to enable all tenants, for testing in QA. + if ( + env.SQS_SEARCH_ENABLE_TENANTS.length === 1 && + env.SQS_SEARCH_ENABLE_TENANTS[0] === "*" + ) { + return true + } + return env.SQS_SEARCH_ENABLE_TENANTS.includes(tenantId) } From 00c8dffd3fa93318e55a8be86ff23d031cb5543c Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Wed, 10 Jul 2024 15:01:55 +0000 Subject: [PATCH 25/25] Bump version to 2.29.16 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index 4d2b5a6cf8..73d20398e5 100644 --- a/lerna.json +++ b/lerna.json @@ -1,6 +1,6 @@ { "$schema": "node_modules/lerna/schemas/lerna-schema.json", - "version": "2.29.15", + "version": "2.29.16", "npmClient": "yarn", "packages": [ "packages/*",