From f3c73fe4a82a6a4f379426a84e895e2af0cc552f Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 22 Jul 2024 17:43:53 +0100 Subject: [PATCH 01/46] Support primitives in feature flags, make flag types flow, remove some obsolete feature flag systems. --- packages/backend-core/src/features/index.ts | 151 +++++++++++------- .../backend-core/src/features/installation.ts | 17 -- .../src/features/tests/featureFlags.spec.ts | 85 ---------- .../src/features/tests/features.spec.ts | 86 ++++++++++ packages/backend-core/src/index.ts | 3 +- packages/builder/src/helpers/featureFlags.js | 3 +- packages/pro | 2 +- packages/server/src/features.ts | 21 ++- packages/types/src/sdk/featureFlag.ts | 9 -- packages/types/src/sdk/index.ts | 1 - packages/types/src/sdk/koa.ts | 3 +- .../worker/src/api/controllers/global/self.ts | 6 +- packages/worker/src/environment.ts | 2 - packages/worker/src/features.ts | 13 -- 14 files changed, 203 insertions(+), 199 deletions(-) delete mode 100644 packages/backend-core/src/features/installation.ts delete mode 100644 packages/backend-core/src/features/tests/featureFlags.spec.ts create mode 100644 packages/backend-core/src/features/tests/features.spec.ts delete mode 100644 packages/types/src/sdk/featureFlag.ts delete mode 100644 packages/worker/src/features.ts diff --git a/packages/backend-core/src/features/index.ts b/packages/backend-core/src/features/index.ts index ad517082de..d7f7c76436 100644 --- a/packages/backend-core/src/features/index.ts +++ b/packages/backend-core/src/features/index.ts @@ -1,79 +1,108 @@ import env from "../environment" import * as context from "../context" +import { cloneDeep } from "lodash" -export * from "./installation" - -/** - * Read the TENANT_FEATURE_FLAGS env var and return an array of features flags for each tenant. - * The env var is formatted as: - * tenant1:feature1:feature2,tenant2:feature1 - */ -export function buildFeatureFlags() { - if (!env.TENANT_FEATURE_FLAGS) { - return +class Flag { + static withDefault(value: T) { + return new Flag(value) } - const tenantFeatureFlags: Record = {} + private constructor(public defaultValue: T) {} +} - env.TENANT_FEATURE_FLAGS.split(",").forEach(tenantToFeatures => { - const [tenantId, ...features] = tenantToFeatures.split(":") +// This is the primary source of truth for feature flags. If you want to add a +// new flag, add it here and use the `fetch` and `get` functions to access it. +// All of the machinery in this file is to make sure that flags have their +// default values set correctly and their types flow through the system. +const FLAGS = { + LICENSING: Flag.withDefault(false), + GOOGLE_SHEETS: Flag.withDefault(false), + USER_GROUPS: Flag.withDefault(false), + ONBOARDING_TOUR: Flag.withDefault(false), +} - features.forEach(feature => { - if (!tenantFeatureFlags[tenantId]) { - tenantFeatureFlags[tenantId] = [] +const DEFAULTS = Object.keys(FLAGS).reduce((acc, key) => { + const typedKey = key as keyof typeof FLAGS + // @ts-ignore + acc[typedKey] = FLAGS[typedKey].defaultValue + return acc +}, {} as Flags) + +type UnwrapFlag = F extends Flag ? U : never +export type Flags = { + [K in keyof typeof FLAGS]: UnwrapFlag<(typeof FLAGS)[K]> +} + +// Exported for use in tests, should not be used outside of this file. +export function defaultFlags(): Flags { + return cloneDeep(DEFAULTS) +} + +function isFlagName(name: string): name is keyof Flags { + return FLAGS[name as keyof typeof FLAGS] !== undefined +} + +/** + * Reads the TENANT_FEATURE_FLAGS environment variable and returns a Flags object + * populated with the flags for the current tenant, filling in the default values + * if the flag is not set. + * + * Check the tests for examples of how TENANT_FEATURE_FLAGS should be formatted. + * + * In future we plan to add more ways of setting feature flags, e.g. PostHog, and + * they will be accessed through this function as well. + */ +export async function fetch(): Promise { + const currentTenantId = context.getTenantId() + const flags = defaultFlags() + + const split = (env.TENANT_FEATURE_FLAGS || "") + .split(",") + .map(x => x.split(":")) + for (const [tenantId, ...features] of split) { + if (!tenantId || (tenantId !== "*" && tenantId !== currentTenantId)) { + continue + } + + for (let feature of features) { + let value = true + if (feature.startsWith("!")) { + feature = feature.slice(1) + value = false } - tenantFeatureFlags[tenantId].push(feature) - }) - }) - return tenantFeatureFlags -} + if (!isFlagName(feature)) { + throw new Error(`Feature: ${feature} is not an allowed option`) + } -export function isEnabled(featureFlag: string) { - const tenantId = context.getTenantId() - const flags = getTenantFeatureFlags(tenantId) - return flags.includes(featureFlag) -} + if (typeof flags[feature] !== "boolean") { + throw new Error(`Feature: ${feature} is not a boolean`) + } -export function getTenantFeatureFlags(tenantId: string) { - let flags: string[] = [] - const envFlags = buildFeatureFlags() - if (envFlags) { - const globalFlags = envFlags["*"] - const tenantFlags = envFlags[tenantId] || [] - - // Explicitly exclude tenants from global features if required. - // Prefix the tenant flag with '!' - const tenantOverrides = tenantFlags.reduce( - (acc: string[], flag: string) => { - if (flag.startsWith("!")) { - let stripped = flag.substring(1) - acc.push(stripped) - } - return acc - }, - [] - ) - - if (globalFlags) { - flags.push(...globalFlags) + // @ts-ignore + flags[feature] = value } - if (tenantFlags.length) { - flags.push(...tenantFlags) - } - - // Purge any tenant specific overrides - flags = flags.filter(flag => { - return tenantOverrides.indexOf(flag) == -1 && !flag.startsWith("!") - }) } return flags } -export enum TenantFeatureFlag { - LICENSING = "LICENSING", - GOOGLE_SHEETS = "GOOGLE_SHEETS", - USER_GROUPS = "USER_GROUPS", - ONBOARDING_TOUR = "ONBOARDING_TOUR", +// Gets a single feature flag value. This is a convenience function for +// `fetch().then(flags => flags[name])`. +export async function get(name: K): Promise { + const flags = await fetch() + return flags[name] +} + +type BooleanFlags = { + [K in keyof typeof FLAGS]: (typeof FLAGS)[K] extends Flag ? K : never +}[keyof typeof FLAGS] + +// Convenience function for boolean flag values. This makes callsites more +// readable for boolean flags. +export async function isEnabled( + name: K +): Promise { + const flags = await fetch() + return flags[name] } diff --git a/packages/backend-core/src/features/installation.ts b/packages/backend-core/src/features/installation.ts deleted file mode 100644 index defc8bf987..0000000000 --- a/packages/backend-core/src/features/installation.ts +++ /dev/null @@ -1,17 +0,0 @@ -export function processFeatureEnvVar( - fullList: string[], - featureList?: string -) { - let list - if (!featureList) { - list = fullList - } else { - list = featureList.split(",") - } - for (let feature of list) { - if (!fullList.includes(feature)) { - throw new Error(`Feature: ${feature} is not an allowed option`) - } - } - return list as unknown as T[] -} diff --git a/packages/backend-core/src/features/tests/featureFlags.spec.ts b/packages/backend-core/src/features/tests/featureFlags.spec.ts deleted file mode 100644 index 1b68959329..0000000000 --- a/packages/backend-core/src/features/tests/featureFlags.spec.ts +++ /dev/null @@ -1,85 +0,0 @@ -import { - TenantFeatureFlag, - buildFeatureFlags, - getTenantFeatureFlags, -} from "../" -import env from "../../environment" - -const { ONBOARDING_TOUR, LICENSING, USER_GROUPS } = TenantFeatureFlag - -describe("featureFlags", () => { - beforeEach(() => { - env._set("TENANT_FEATURE_FLAGS", "") - }) - - it("Should return no flags when the TENANT_FEATURE_FLAG is empty", async () => { - let features = buildFeatureFlags() - expect(features).toBeUndefined() - }) - - it("Should generate a map of global and named tenant feature flags from the env value", async () => { - env._set( - "TENANT_FEATURE_FLAGS", - `*:${ONBOARDING_TOUR},tenant1:!${ONBOARDING_TOUR},tenant2:${USER_GROUPS},tenant1:${LICENSING}` - ) - - const parsedFlags: Record = { - "*": [ONBOARDING_TOUR], - tenant1: [`!${ONBOARDING_TOUR}`, LICENSING], - tenant2: [USER_GROUPS], - } - - let features = buildFeatureFlags() - - expect(features).toBeDefined() - expect(features).toEqual(parsedFlags) - }) - - it("Should add feature flag flag only to explicitly configured tenant", async () => { - env._set( - "TENANT_FEATURE_FLAGS", - `*:${LICENSING},*:${USER_GROUPS},tenant1:${ONBOARDING_TOUR}` - ) - - let tenant1Flags = getTenantFeatureFlags("tenant1") - let tenant2Flags = getTenantFeatureFlags("tenant2") - - expect(tenant1Flags).toBeDefined() - expect(tenant1Flags).toEqual([LICENSING, USER_GROUPS, ONBOARDING_TOUR]) - - expect(tenant2Flags).toBeDefined() - expect(tenant2Flags).toEqual([LICENSING, USER_GROUPS]) - }) -}) - -it("Should exclude tenant1 from global feature flag", async () => { - env._set( - "TENANT_FEATURE_FLAGS", - `*:${LICENSING},*:${ONBOARDING_TOUR},tenant1:!${ONBOARDING_TOUR}` - ) - - let tenant1Flags = getTenantFeatureFlags("tenant1") - let tenant2Flags = getTenantFeatureFlags("tenant2") - - expect(tenant1Flags).toBeDefined() - expect(tenant1Flags).toEqual([LICENSING]) - - expect(tenant2Flags).toBeDefined() - expect(tenant2Flags).toEqual([LICENSING, ONBOARDING_TOUR]) -}) - -it("Should explicitly add flags to configured tenants only", async () => { - env._set( - "TENANT_FEATURE_FLAGS", - `tenant1:${ONBOARDING_TOUR},tenant1:${LICENSING},tenant2:${LICENSING}` - ) - - let tenant1Flags = getTenantFeatureFlags("tenant1") - let tenant2Flags = getTenantFeatureFlags("tenant2") - - expect(tenant1Flags).toBeDefined() - expect(tenant1Flags).toEqual([ONBOARDING_TOUR, LICENSING]) - - expect(tenant2Flags).toBeDefined() - expect(tenant2Flags).toEqual([LICENSING]) -}) diff --git a/packages/backend-core/src/features/tests/features.spec.ts b/packages/backend-core/src/features/tests/features.spec.ts new file mode 100644 index 0000000000..83a89940b8 --- /dev/null +++ b/packages/backend-core/src/features/tests/features.spec.ts @@ -0,0 +1,86 @@ +import { defaultFlags, fetch, get, Flags } from "../" +import { context } from "../.." +import env from "../../environment" + +async function withFlags(flags: string, f: () => T): Promise { + const oldFlags = env.TENANT_FEATURE_FLAGS + env._set("TENANT_FEATURE_FLAGS", flags) + try { + return await f() + } finally { + env._set("TENANT_FEATURE_FLAGS", oldFlags) + } +} + +describe("feature flags", () => { + interface TestCase { + tenant: string + flags: string + expected: Partial + } + + it.each([ + { + tenant: "tenant1", + flags: "tenant1:ONBOARDING_TOUR", + expected: { ONBOARDING_TOUR: true }, + }, + { + tenant: "tenant1", + flags: "tenant1:!ONBOARDING_TOUR", + expected: { ONBOARDING_TOUR: false }, + }, + { + tenant: "tenant1", + flags: "*:ONBOARDING_TOUR", + expected: { ONBOARDING_TOUR: true }, + }, + { + tenant: "tenant1", + flags: "tenant2:ONBOARDING_TOUR", + expected: { ONBOARDING_TOUR: false }, + }, + { + tenant: "tenant1", + flags: "", + expected: defaultFlags(), + }, + ])( + 'should find flags $expected for $tenant with string "$flags"', + ({ tenant, flags, expected }) => + context.doInTenant(tenant, () => + withFlags(flags, async () => { + const flags = await fetch() + expect(flags).toMatchObject(expected) + + for (const [key, expectedValue] of Object.entries(expected)) { + const value = await get(key as keyof Flags) + expect(value).toBe(expectedValue) + } + }) + ) + ) + + interface FailedTestCase { + tenant: string + flags: string + expected: string | RegExp + } + + it.each([ + { + tenant: "tenant1", + flags: "tenant1:ONBOARDING_TOUR,tenant1:FOO", + expected: "Feature: FOO is not an allowed option", + }, + ])( + "should fail with message \"$expected\" for $tenant with string '$flags'", + async ({ tenant, flags, expected }) => { + context.doInTenant(tenant, () => + withFlags(flags, async () => { + await expect(fetch()).rejects.toThrow(expected) + }) + ) + } + ) +}) diff --git a/packages/backend-core/src/index.ts b/packages/backend-core/src/index.ts index 30c5fbdd7a..a14a344655 100644 --- a/packages/backend-core/src/index.ts +++ b/packages/backend-core/src/index.ts @@ -7,8 +7,7 @@ export * as roles from "./security/roles" export * as permissions from "./security/permissions" export * as accounts from "./accounts" export * as installation from "./installation" -export * as featureFlags from "./features" -export * as features from "./features/installation" +export * as features from "./features" export * as sessions from "./security/sessions" export * as platform from "./platform" export * as auth from "./auth" diff --git a/packages/builder/src/helpers/featureFlags.js b/packages/builder/src/helpers/featureFlags.js index 462dae8c54..fe30fb9980 100644 --- a/packages/builder/src/helpers/featureFlags.js +++ b/packages/builder/src/helpers/featureFlags.js @@ -5,9 +5,10 @@ export const TENANT_FEATURE_FLAGS = { LICENSING: "LICENSING", USER_GROUPS: "USER_GROUPS", ONBOARDING_TOUR: "ONBOARDING_TOUR", + GOOGLE_SHEETS: "GOOGLE_SHEETS", } export const isEnabled = featureFlag => { const user = get(auth).user - return !!user?.featureFlags?.includes(featureFlag) + return !!user?.flags?.[featureFlag] } diff --git a/packages/pro b/packages/pro index 7dbe323aec..62ef0e2d6e 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit 7dbe323aec724ae6336b13c06aaefa4a89837edf +Subproject commit 62ef0e2d6e83522b6732fb3c61338de303f06ff0 diff --git a/packages/server/src/features.ts b/packages/server/src/features.ts index f040cf82a2..bf92ede18e 100644 --- a/packages/server/src/features.ts +++ b/packages/server/src/features.ts @@ -1,4 +1,3 @@ -import { features } from "@budibase/backend-core" import env from "./environment" enum AppFeature { @@ -6,7 +5,25 @@ enum AppFeature { AUTOMATIONS = "automations", } -const featureList = features.processFeatureEnvVar( +export function processFeatureEnvVar( + fullList: string[], + featureList?: string +) { + let list + if (!featureList) { + list = fullList + } else { + list = featureList.split(",") + } + for (let feature of list) { + if (!fullList.includes(feature)) { + throw new Error(`Feature: ${feature} is not an allowed option`) + } + } + return list as unknown as T[] +} + +const featureList = processFeatureEnvVar( Object.values(AppFeature), env.APP_FEATURES ) diff --git a/packages/types/src/sdk/featureFlag.ts b/packages/types/src/sdk/featureFlag.ts deleted file mode 100644 index ca0046696a..0000000000 --- a/packages/types/src/sdk/featureFlag.ts +++ /dev/null @@ -1,9 +0,0 @@ -export enum FeatureFlag { - LICENSING = "LICENSING", - PER_CREATOR_PER_USER_PRICE = "PER_CREATOR_PER_USER_PRICE", - PER_CREATOR_PER_USER_PRICE_ALERT = "PER_CREATOR_PER_USER_PRICE_ALERT", -} - -export interface TenantFeatureFlags { - [key: string]: FeatureFlag[] -} diff --git a/packages/types/src/sdk/index.ts b/packages/types/src/sdk/index.ts index d87ec58b0c..c48cbaf65c 100644 --- a/packages/types/src/sdk/index.ts +++ b/packages/types/src/sdk/index.ts @@ -11,7 +11,6 @@ export * from "./auth" export * from "./locks" export * from "./db" export * from "./middleware" -export * from "./featureFlag" export * from "./environmentVariables" export * from "./auditLogs" export * from "./sso" diff --git a/packages/types/src/sdk/koa.ts b/packages/types/src/sdk/koa.ts index a7df701171..07ce2efb6e 100644 --- a/packages/types/src/sdk/koa.ts +++ b/packages/types/src/sdk/koa.ts @@ -1,6 +1,6 @@ import { Context, Request } from "koa" import { User, Role, UserRoles, Account, ConfigType } from "../documents" -import { FeatureFlag, License } from "../sdk" +import { License } from "../sdk" import { Files } from "formidable" export interface ContextUser extends Omit { @@ -11,7 +11,6 @@ export interface ContextUser extends Omit { role?: Role roles?: UserRoles csrfToken?: string - featureFlags?: FeatureFlag[] accountPortalAccess?: boolean providerType?: ConfigType account?: Account diff --git a/packages/worker/src/api/controllers/global/self.ts b/packages/worker/src/api/controllers/global/self.ts index d762f5168a..ec154adf7f 100644 --- a/packages/worker/src/api/controllers/global/self.ts +++ b/packages/worker/src/api/controllers/global/self.ts @@ -1,6 +1,6 @@ import * as userSdk from "../../../sdk/users" import { - featureFlags, + features, tenancy, db as dbCore, utils, @@ -104,8 +104,8 @@ export async function getSelf(ctx: any) { ctx.body = await groups.enrichUserRolesFromGroups(user) // add the feature flags for this tenant - const tenantId = tenancy.getTenantId() - ctx.body.featureFlags = featureFlags.getTenantFeatureFlags(tenantId) + const flags = await features.fetch() + ctx.body.flags = flags addSessionAttributesToUser(ctx) } diff --git a/packages/worker/src/environment.ts b/packages/worker/src/environment.ts index 9f7baf9e9b..6e36b45a3b 100644 --- a/packages/worker/src/environment.ts +++ b/packages/worker/src/environment.ts @@ -19,8 +19,6 @@ function parseIntSafe(number: any) { } const environment = { - // features - WORKER_FEATURES: process.env.WORKER_FEATURES, // auth MINIO_ACCESS_KEY: process.env.MINIO_ACCESS_KEY, MINIO_SECRET_KEY: process.env.MINIO_SECRET_KEY, diff --git a/packages/worker/src/features.ts b/packages/worker/src/features.ts deleted file mode 100644 index 075b3b81ca..0000000000 --- a/packages/worker/src/features.ts +++ /dev/null @@ -1,13 +0,0 @@ -import { features } from "@budibase/backend-core" -import env from "./environment" - -enum WorkerFeature {} - -const featureList: WorkerFeature[] = features.processFeatureEnvVar( - Object.values(WorkerFeature), - env.WORKER_FEATURES -) - -export function isFeatureEnabled(feature: WorkerFeature) { - return featureList.includes(feature) -} From f0d3f3fdc1815d995e4a9f305f707e14422aeff2 Mon Sep 17 00:00:00 2001 From: Guspan Tanadi <36249910+guspan-tanadi@users.noreply.github.com> Date: Mon, 5 Aug 2024 07:15:02 +0700 Subject: [PATCH 02/46] docs: reference link --- README.md | 2 +- i18n/README.es.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 4979f0ee8e..64492b97e4 100644 --- a/README.md +++ b/README.md @@ -116,7 +116,7 @@ As with anything that we build in Budibase, our new public API is simple to use, You can learn more about the Budibase API at the following places: - [General documentation](https://docs.budibase.com/docs/public-api): Learn how to get your API key, how to use spec, and how to use Postman -- [Interactive API documentation](https://docs.budibase.com/reference/post_applications) : Learn how to interact with the API +- [Interactive API documentation](https://docs.budibase.com/reference/appcreate) : Learn how to interact with the API

diff --git a/i18n/README.es.md b/i18n/README.es.md index a7d1112914..ee92ca24d5 100644 --- a/i18n/README.es.md +++ b/i18n/README.es.md @@ -144,7 +144,7 @@ del sistema. Budibase API ofrece: Puedes aprender mas acerca de Budibase API en los siguientes documentos: - [Documentacion general](https://docs.budibase.com/docs/public-api) : Como optener tu clave para la API, usar Insomnia y Postman -- [API Interactiva](https://docs.budibase.com/reference/post_applications) : Aprende como trabajar con la API +- [API Interactiva](https://docs.budibase.com/reference/appcreate) : Aprende como trabajar con la API #### Guias From 4799e0c2c4d10dd90d431c33a585f3a7fb704e95 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 5 Aug 2024 10:29:36 +0200 Subject: [PATCH 03/46] Add extra typings --- packages/types/src/sdk/search.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/types/src/sdk/search.ts b/packages/types/src/sdk/search.ts index 5607efece8..7f4c1ca977 100644 --- a/packages/types/src/sdk/search.ts +++ b/packages/types/src/sdk/search.ts @@ -75,6 +75,13 @@ export interface SearchFilters { // to make sure the documents returned are always filtered down to a // specific document type (such as just rows) documentType?: DocumentType + + $and?: { + conditions: SearchFilters[] + } + $or?: { + conditions: SearchFilters[] + } } export type SearchFilterKey = keyof Omit< From ff0bee59748c380bcda00addfc6ebf66ecfab75c Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 5 Aug 2024 10:55:56 +0200 Subject: [PATCH 04/46] Add tests --- .../src/api/routes/tests/search.spec.ts | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index bc3cdccf18..401715e198 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2692,4 +2692,49 @@ describe.each([ ) }) }) + + describe("$and", () => { + beforeAll(async () => { + table = await createTable({ + age: { name: "age", type: FieldType.NUMBER }, + name: { name: "name", type: FieldType.STRING }, + }) + await createRows([ + { age: 1, name: "Jane" }, + { age: 10, name: "Jack" }, + { age: 7, name: "Hanna" }, + { age: 8, name: "Jan" }, + ]) + }) + + it("successfully finds a row for one level condition", async () => { + await expectQuery({ + $and: { + conditions: [{ equal: { age: 10 } }, { equal: { name: "Jack" } }], + }, + }).toContainExactly([{ age: 10, name: "Jack" }]) + }) + + it("successfully finds a row for one level with multiple conditions", async () => { + await expectQuery({ + $and: { + conditions: [{ equal: { age: 10 } }, { equal: { name: "Jack" } }], + }, + }).toContainExactly([{ age: 10, name: "Jack" }]) + }) + + it("successfully finds multiple rows for one level with multiple conditions", async () => { + await expectQuery({ + $and: { + conditions: [ + { range: { age: { low: 1, high: 9 } } }, + { string: { name: "Ja" } }, + ], + }, + }).toContainExactly([ + { age: 1, name: "Jane" }, + { age: 8, name: "Jan" }, + ]) + }) + }) }) From 47de3f0c5348a4927b3d9d372bb445fe24157515 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 5 Aug 2024 11:07:37 +0200 Subject: [PATCH 05/46] Add or tests --- .../src/api/routes/tests/search.spec.ts | 112 ++++++++++++++++++ 1 file changed, 112 insertions(+) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 401715e198..3a54ec9035 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2736,5 +2736,117 @@ describe.each([ { age: 8, name: "Jan" }, ]) }) + + it("successfully finds rows for nested filters", async () => { + await expectQuery({ + $and: { + conditions: [ + { + $and: { + conditions: [ + { + range: { age: { low: 1, high: 10 } }, + }, + { string: { name: "Ja" } }, + ], + }, + equal: { name: "Jane" }, + }, + ], + }, + }).toContainExactly([{ age: 1, name: "Jane" }]) + }) + + it("returns nothing when filtering out all data", async () => { + await expectQuery({ + $and: { + conditions: [{ equal: { age: 7 } }, { equal: { name: "Jack" } }], + }, + }).toFindNothing() + }) + }) + + describe("$or", () => { + beforeAll(async () => { + table = await createTable({ + age: { name: "age", type: FieldType.NUMBER }, + name: { name: "name", type: FieldType.STRING }, + }) + await createRows([ + { age: 1, name: "Jane" }, + { age: 10, name: "Jack" }, + { age: 7, name: "Hanna" }, + { age: 8, name: "Jan" }, + ]) + }) + + it("successfully finds a row for one level condition", async () => { + await expectQuery({ + $or: { + conditions: [{ equal: { age: 7 } }, { equal: { name: "Jack" } }], + }, + }).toContainExactly([ + { age: 10, name: "Jack" }, + { age: 7, name: "Hanna" }, + ]) + }) + + it("successfully finds a row for one level with multiple conditions", async () => { + await expectQuery({ + $or: { + conditions: [{ equal: { age: 7 } }, { equal: { name: "Jack" } }], + }, + }).toContainExactly([ + { age: 10, name: "Jack" }, + { age: 7, name: "Hanna" }, + ]) + }) + + it("successfully finds multiple rows for one level with multiple conditions", async () => { + await expectQuery({ + $or: { + conditions: [ + { range: { age: { low: 1, high: 9 } } }, + { string: { name: "Ja" } }, + ], + }, + }).toContainExactly([ + { age: 1, name: "Jane" }, + { age: 7, name: "Hanna" }, + { age: 8, name: "Jan" }, + ]) + }) + + it("successfully finds rows for nested filters", async () => { + await expectQuery({ + $or: { + conditions: [ + { + $or: { + conditions: [ + { + range: { age: { low: 1, high: 7 } }, + }, + { string: { name: "Jan" } }, + ], + }, + equal: { name: "Jane" }, + }, + ], + }, + }).toContainExactly([ + { age: 1, name: "Jane" }, + { age: 7, name: "Hanna" }, + { age: 8, name: "Jan" }, + ]) + }) + + it("returns nothing when filtering out all data", async () => { + await expectQuery({ + $or: { + conditions: [{ equal: { age: 6 } }, { equal: { name: "John" } }], + }, + }).toFindNothing() + }) }) }) From 2e23a0e4ceb60de68bb9f054554211219299e1ba Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 5 Aug 2024 12:33:44 +0200 Subject: [PATCH 06/46] Implement SQL and/or --- packages/backend-core/src/sql/sql.ts | 19 +++++++++++++++++++ .../src/api/routes/tests/search.spec.ts | 2 +- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index 76c86b2b62..f58ddbac6a 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -369,6 +369,25 @@ class InternalBuilder { } } + if (filters.$and) { + iterate(filters.$and, (key: string, conditions: any[]) => { + query = query.where(x => { + for (const condition of conditions) { + x = this.addFilters(x, condition, table, opts) + } + }) + }) + } + + if (filters.$or) { + const { $or } = filters + query = query.where(x => { + for (const condition of $or.conditions) { + x = this.addFilters(x, { ...condition, allOr: true }, table, opts) + } + }) + } + if (filters.oneOf) { const fnc = allOr ? "orWhereIn" : "whereIn" iterate( diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 3a54ec9035..9243ec6b6e 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2807,7 +2807,7 @@ describe.each([ $or: { conditions: [ { range: { age: { low: 1, high: 9 } } }, - { string: { name: "Ja" } }, + { string: { name: "Jan" } }, ], }, }).toContainExactly([ From 940a080e1825667dced94be466e56f47839ffbc0 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 5 Aug 2024 12:35:09 +0200 Subject: [PATCH 07/46] Run only for external --- .../src/api/routes/tests/search.spec.ts | 290 +++++++++--------- 1 file changed, 146 insertions(+), 144 deletions(-) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 9243ec6b6e..bcffc8a868 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2693,160 +2693,162 @@ describe.each([ }) }) - describe("$and", () => { - beforeAll(async () => { - table = await createTable({ - age: { name: "age", type: FieldType.NUMBER }, - name: { name: "name", type: FieldType.STRING }, - }) - await createRows([ - { age: 1, name: "Jane" }, - { age: 10, name: "Jack" }, - { age: 7, name: "Hanna" }, - { age: 8, name: "Jan" }, - ]) - }) - - it("successfully finds a row for one level condition", async () => { - await expectQuery({ - $and: { - conditions: [{ equal: { age: 10 } }, { equal: { name: "Jack" } }], - }, - }).toContainExactly([{ age: 10, name: "Jack" }]) - }) - - it("successfully finds a row for one level with multiple conditions", async () => { - await expectQuery({ - $and: { - conditions: [{ equal: { age: 10 } }, { equal: { name: "Jack" } }], - }, - }).toContainExactly([{ age: 10, name: "Jack" }]) - }) - - it("successfully finds multiple rows for one level with multiple conditions", async () => { - await expectQuery({ - $and: { - conditions: [ - { range: { age: { low: 1, high: 9 } } }, - { string: { name: "Ja" } }, - ], - }, - }).toContainExactly([ - { age: 1, name: "Jane" }, - { age: 8, name: "Jan" }, - ]) + !isInternal && + describe("$and", () => { + beforeAll(async () => { + table = await createTable({ + age: { name: "age", type: FieldType.NUMBER }, + name: { name: "name", type: FieldType.STRING }, + }) + await createRows([ + { age: 1, name: "Jane" }, + { age: 10, name: "Jack" }, + { age: 7, name: "Hanna" }, + { age: 8, name: "Jan" }, + ]) }) - it("successfully finds rows for nested filters", async () => { - await expectQuery({ - $and: { - conditions: [ - { - $and: { - conditions: [ - { - range: { age: { low: 1, high: 10 } }, - }, - { string: { name: "Ja" } }, - ], + it("successfully finds a row for one level condition", async () => { + await expectQuery({ + $and: { + conditions: [{ equal: { age: 10 } }, { equal: { name: "Jack" } }], + }, + }).toContainExactly([{ age: 10, name: "Jack" }]) + }) + + it("successfully finds a row for one level with multiple conditions", async () => { + await expectQuery({ + $and: { + conditions: [{ equal: { age: 10 } }, { equal: { name: "Jack" } }], + }, + }).toContainExactly([{ age: 10, name: "Jack" }]) + }) + + it("successfully finds multiple rows for one level with multiple conditions", async () => { + await expectQuery({ + $and: { + conditions: [ + { range: { age: { low: 1, high: 9 } } }, + { string: { name: "Ja" } }, + ], + }, + }).toContainExactly([ + { age: 1, name: "Jane" }, + { age: 8, name: "Jan" }, + ]) + }) + + it("successfully finds rows for nested filters", async () => { + await expectQuery({ + $and: { + conditions: [ + { + $and: { + conditions: [ + { + range: { age: { low: 1, high: 10 } }, + }, + { string: { name: "Ja" } }, + ], + }, + equal: { name: "Jane" }, }, - equal: { name: "Jane" }, - }, - ], - }, - }).toContainExactly([{ age: 1, name: "Jane" }]) - }) - - it("returns nothing when filtering out all data", async () => { - await expectQuery({ - $and: { - conditions: [{ equal: { age: 7 } }, { equal: { name: "Jack" } }], - }, - }).toFindNothing() - }) - }) - - describe("$or", () => { - beforeAll(async () => { - table = await createTable({ - age: { name: "age", type: FieldType.NUMBER }, - name: { name: "name", type: FieldType.STRING }, + ], + }, + }).toContainExactly([{ age: 1, name: "Jane" }]) + }) + + it("returns nothing when filtering out all data", async () => { + await expectQuery({ + $and: { + conditions: [{ equal: { age: 7 } }, { equal: { name: "Jack" } }], + }, + }).toFindNothing() }) - await createRows([ - { age: 1, name: "Jane" }, - { age: 10, name: "Jack" }, - { age: 7, name: "Hanna" }, - { age: 8, name: "Jan" }, - ]) }) - it("successfully finds a row for one level condition", async () => { - await expectQuery({ - $or: { - conditions: [{ equal: { age: 7 } }, { equal: { name: "Jack" } }], - }, - }).toContainExactly([ - { age: 10, name: "Jack" }, - { age: 7, name: "Hanna" }, - ]) - }) + !isInternal && + describe("$or", () => { + beforeAll(async () => { + table = await createTable({ + age: { name: "age", type: FieldType.NUMBER }, + name: { name: "name", type: FieldType.STRING }, + }) + await createRows([ + { age: 1, name: "Jane" }, + { age: 10, name: "Jack" }, + { age: 7, name: "Hanna" }, + { age: 8, name: "Jan" }, + ]) + }) - it("successfully finds a row for one level with multiple conditions", async () => { - await expectQuery({ - $or: { - conditions: [{ equal: { age: 7 } }, { equal: { name: "Jack" } }], - }, - }).toContainExactly([ - { age: 10, name: "Jack" }, - { age: 7, name: "Hanna" }, - ]) - }) + it("successfully finds a row for one level condition", async () => { + await expectQuery({ + $or: { + conditions: [{ equal: { age: 7 } }, { equal: { name: "Jack" } }], + }, + }).toContainExactly([ + { age: 10, name: "Jack" }, + { age: 7, name: "Hanna" }, + ]) + }) - it("successfully finds multiple rows for one level with multiple conditions", async () => { - await expectQuery({ - $or: { - conditions: [ - { range: { age: { low: 1, high: 9 } } }, - { string: { name: "Jan" } }, - ], - }, - }).toContainExactly([ - { age: 1, name: "Jane" }, - { age: 7, name: "Hanna" }, - { age: 8, name: "Jan" }, - ]) - }) + it("successfully finds a row for one level with multiple conditions", async () => { + await expectQuery({ + $or: { + conditions: [{ equal: { age: 7 } }, { equal: { name: "Jack" } }], + }, + }).toContainExactly([ + { age: 10, name: "Jack" }, + { age: 7, name: "Hanna" }, + ]) + }) - it("successfully finds rows for nested filters", async () => { - await expectQuery({ - $or: { - conditions: [ - { - $or: { - conditions: [ - { - range: { age: { low: 1, high: 7 } }, - }, - { string: { name: "Jan" } }, - ], + it("successfully finds multiple rows for one level with multiple conditions", async () => { + await expectQuery({ + $or: { + conditions: [ + { range: { age: { low: 1, high: 9 } } }, + { string: { name: "Jan" } }, + ], + }, + }).toContainExactly([ + { age: 1, name: "Jane" }, + { age: 7, name: "Hanna" }, + { age: 8, name: "Jan" }, + ]) + }) + + it("successfully finds rows for nested filters", async () => { + await expectQuery({ + $or: { + conditions: [ + { + $or: { + conditions: [ + { + range: { age: { low: 1, high: 7 } }, + }, + { string: { name: "Jan" } }, + ], + }, + equal: { name: "Jane" }, }, - equal: { name: "Jane" }, - }, - ], - }, - }).toContainExactly([ - { age: 1, name: "Jane" }, - { age: 7, name: "Hanna" }, - { age: 8, name: "Jan" }, - ]) - }) + ], + }, + }).toContainExactly([ + { age: 1, name: "Jane" }, + { age: 7, name: "Hanna" }, + { age: 8, name: "Jan" }, + ]) + }) - it("returns nothing when filtering out all data", async () => { - await expectQuery({ - $or: { - conditions: [{ equal: { age: 6 } }, { equal: { name: "John" } }], - }, - }).toFindNothing() - }) + it("returns nothing when filtering out all data", async () => { + await expectQuery({ + $or: { + conditions: [{ equal: { age: 6 } }, { equal: { name: "John" } }], + }, + }).toFindNothing() + }) }) }) From ebca381e9bfe74ffa1c8eea413348f06ee25644d Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 5 Aug 2024 15:09:33 +0200 Subject: [PATCH 08/46] Nested $and's and $or's test --- packages/backend-core/src/sql/sql.ts | 11 +++-- .../src/api/routes/tests/search.spec.ts | 43 +++++++++++++++++++ 2 files changed, 48 insertions(+), 6 deletions(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index f58ddbac6a..714d68067d 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -370,12 +370,11 @@ class InternalBuilder { } if (filters.$and) { - iterate(filters.$and, (key: string, conditions: any[]) => { - query = query.where(x => { - for (const condition of conditions) { - x = this.addFilters(x, condition, table, opts) - } - }) + const { $and } = filters + query = query.where(x => { + for (const condition of $and.conditions) { + x = this.addFilters(x, condition, table, opts) + } }) } diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index bcffc8a868..517664c5ca 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2850,5 +2850,48 @@ describe.each([ }, }).toFindNothing() }) + + it("can nest $and under $or filters", async () => { + await expectQuery({ + $or: { + conditions: [ + { + $and: { + conditions: [ + { + range: { age: { low: 1, high: 8 } }, + }, + { equal: { name: "Jan" } }, + ], + }, + equal: { name: "Jane" }, + }, + ], + }, + }).toContainExactly([ + { age: 1, name: "Jane" }, + { age: 8, name: "Jan" }, + ]) + }) + + it("can nest $or under $and filters", async () => { + await expectQuery({ + $and: { + conditions: [ + { + $or: { + conditions: [ + { + range: { age: { low: 1, high: 8 } }, + }, + { equal: { name: "Jan" } }, + ], + }, + equal: { name: "Jane" }, + }, + ], + }, + }).toContainExactly([{ age: 1, name: "Jane" }]) + }) }) }) From f846507877344c0de714d3136a83adb20aee37b5 Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Mon, 5 Aug 2024 15:18:06 +0000 Subject: [PATCH 09/46] Bump version to 2.29.29 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index d7d2bdce39..19303c8580 100644 --- a/lerna.json +++ b/lerna.json @@ -1,6 +1,6 @@ { "$schema": "node_modules/lerna/schemas/lerna-schema.json", - "version": "2.29.28", + "version": "2.29.29", "npmClient": "yarn", "packages": [ "packages/*", From 0b5eb9f21cac4d0cc81156462e767b1fdcb406a0 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 5 Aug 2024 17:19:14 +0200 Subject: [PATCH 10/46] Run tests for all sql --- packages/server/src/api/routes/tests/search.spec.ts | 4 ++-- 1 file changed, 2 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 517664c5ca..6c114ecf5b 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2693,7 +2693,7 @@ describe.each([ }) }) - !isInternal && + isSql && describe("$and", () => { beforeAll(async () => { table = await createTable({ @@ -2767,7 +2767,7 @@ describe.each([ }) }) - !isInternal && + isSql && describe("$or", () => { beforeAll(async () => { table = await createTable({ From 1cd33472626cac98ee9c5f8d0a5b98431a4e8013 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 5 Aug 2024 17:21:10 +0200 Subject: [PATCH 11/46] Types --- .../src/sdk/app/rows/search/internal/sqs.ts | 1 + packages/types/src/sdk/search.ts | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 3 deletions(-) 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 bc46e8d4c1..8cc200545a 100644 --- a/packages/server/src/sdk/app/rows/search/internal/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/internal/sqs.ts @@ -2,6 +2,7 @@ import { Datasource, DocumentType, FieldType, + LogicalOperator, Operation, QueryJson, RelationshipFieldMetadata, diff --git a/packages/types/src/sdk/search.ts b/packages/types/src/sdk/search.ts index 7f4c1ca977..11bfb12b12 100644 --- a/packages/types/src/sdk/search.ts +++ b/packages/types/src/sdk/search.ts @@ -23,7 +23,16 @@ export enum RangeOperator { RANGE = "range", } -export type SearchFilterOperator = BasicOperator | ArrayOperator | RangeOperator +export enum LogicalOperator { + AND = "$and", + OR = "$or", +} + +export type SearchFilterOperator = + | BasicOperator + | ArrayOperator + | RangeOperator + | LogicalOperator export enum InternalSearchFilterOperator { COMPLEX_ID_OPERATOR = "_complexIdOperator", @@ -76,10 +85,10 @@ export interface SearchFilters { // specific document type (such as just rows) documentType?: DocumentType - $and?: { + [LogicalOperator.AND]?: { conditions: SearchFilters[] } - $or?: { + [LogicalOperator.OR]?: { conditions: SearchFilters[] } } From c845db966f98fbe2b2c9fc3bad273a34d2cbecb2 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 5 Aug 2024 17:25:17 +0200 Subject: [PATCH 12/46] Fix prefixes for SQS --- .../src/sdk/app/rows/search/internal/sqs.ts | 38 +++++++++++++------ 1 file changed, 27 insertions(+), 11 deletions(-) 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 8cc200545a..b72e854ef2 100644 --- a/packages/server/src/sdk/app/rows/search/internal/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/internal/sqs.ts @@ -135,20 +135,36 @@ function cleanupFilters( allTables.some(table => table.schema[key]) const splitter = new dataFilters.ColumnSplitter(allTables) - for (const filter of Object.values(filters)) { - for (const key of Object.keys(filter)) { - const { numberPrefix, relationshipPrefix, column } = splitter.run(key) - if (keyInAnyTable(column)) { - filter[ - `${numberPrefix || ""}${relationshipPrefix || ""}${mapToUserColumn( - column - )}` - ] = filter[key] - delete filter[key] + + const prefixFilters = (filters: SearchFilters) => { + for (const filterKey of Object.keys(filters) as (keyof SearchFilters)[]) { + if ( + filterKey === LogicalOperator.AND || + filterKey === LogicalOperator.OR + ) { + for (const condition of filters[filterKey]!.conditions) { + prefixFilters(condition) + } + } else { + const filter = filters[filterKey]! + if (typeof filter !== "object") { + continue + } + for (const key of Object.keys(filter)) { + const { numberPrefix, relationshipPrefix, column } = splitter.run(key) + if (keyInAnyTable(column)) { + filter[ + `${numberPrefix || ""}${ + relationshipPrefix || "" + }${mapToUserColumn(column)}` + ] = filter[key] + delete filter[key] + } + } } } } - + prefixFilters(filters) return filters } From 58162410eafeb0d460f15c39f258ba478faad64d Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Mon, 5 Aug 2024 17:32:40 +0200 Subject: [PATCH 13/46] Fix merge conflicts --- packages/backend-core/src/sql/sql.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/backend-core/src/sql/sql.ts b/packages/backend-core/src/sql/sql.ts index d9379773ff..bb3133f852 100644 --- a/packages/backend-core/src/sql/sql.ts +++ b/packages/backend-core/src/sql/sql.ts @@ -467,7 +467,7 @@ class InternalBuilder { const { $and } = filters query = query.where(x => { for (const condition of $and.conditions) { - x = this.addFilters(x, condition, table, opts) + x = this.addFilters(x, condition, opts) } }) } @@ -476,7 +476,7 @@ class InternalBuilder { const { $or } = filters query = query.where(x => { for (const condition of $or.conditions) { - x = this.addFilters(x, { ...condition, allOr: true }, table, opts) + x = this.addFilters(x, { ...condition, allOr: true }, opts) } }) } From 48ddc059af4adc6d0d7c6d27de4989204a964633 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 5 Aug 2024 16:38:21 +0100 Subject: [PATCH 14/46] Bumping google-spreadsheet version to latest version with fix added by Dean. --- packages/server/package.json | 2 +- yarn.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/server/package.json b/packages/server/package.json index 48ab0685d9..b835477489 100644 --- a/packages/server/package.json +++ b/packages/server/package.json @@ -80,7 +80,7 @@ "dotenv": "8.2.0", "form-data": "4.0.0", "global-agent": "3.0.0", - "google-spreadsheet": "npm:@budibase/google-spreadsheet@4.1.2", + "google-spreadsheet": "npm:@budibase/google-spreadsheet@4.1.3", "ioredis": "5.3.2", "isolated-vm": "^4.7.2", "jimp": "0.22.12", diff --git a/yarn.lock b/yarn.lock index 607db0b7bb..0195f19a2a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -12072,10 +12072,10 @@ google-p12-pem@^4.0.0: dependencies: node-forge "^1.3.1" -"google-spreadsheet@npm:@budibase/google-spreadsheet@4.1.2": - version "4.1.2" - resolved "https://registry.yarnpkg.com/@budibase/google-spreadsheet/-/google-spreadsheet-4.1.2.tgz#90548ccba2284b3042b08d2974ef3caeaf772ad9" - integrity sha512-dxoY3rQGGnuNeZiXhNc9oYPduzU8xnIjWujFwNvaRRv3zWeUV7mj6HE2o/OJOeekPGt7o44B+w6DfkiaoteZgg== +"google-spreadsheet@npm:@budibase/google-spreadsheet@4.1.3": + version "4.1.3" + resolved "https://registry.yarnpkg.com/@budibase/google-spreadsheet/-/google-spreadsheet-4.1.3.tgz#bcee7bd9d90f82c54b16a9aca963b87aceb050ad" + integrity sha512-03VX3/K5NXIh6+XAIDZgcHPmR76xwd8vIDL7RedMpvM2IcXK0Iq/KU7FmLY0t/mKqORAGC7+0rajd0jLFezC4w== dependencies: axios "^1.4.0" lodash "^4.17.21" From 879552d298bd753b5ac9189cfcf37ac8e8135603 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Mon, 5 Aug 2024 17:27:03 +0100 Subject: [PATCH 15/46] Fix account-portal server tests. --- packages/types/src/sdk/featureFlag.ts | 9 +++++++++ packages/types/src/sdk/index.ts | 1 + packages/types/src/sdk/koa.ts | 3 ++- 3 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 packages/types/src/sdk/featureFlag.ts diff --git a/packages/types/src/sdk/featureFlag.ts b/packages/types/src/sdk/featureFlag.ts new file mode 100644 index 0000000000..ca0046696a --- /dev/null +++ b/packages/types/src/sdk/featureFlag.ts @@ -0,0 +1,9 @@ +export enum FeatureFlag { + LICENSING = "LICENSING", + PER_CREATOR_PER_USER_PRICE = "PER_CREATOR_PER_USER_PRICE", + PER_CREATOR_PER_USER_PRICE_ALERT = "PER_CREATOR_PER_USER_PRICE_ALERT", +} + +export interface TenantFeatureFlags { + [key: string]: FeatureFlag[] +} diff --git a/packages/types/src/sdk/index.ts b/packages/types/src/sdk/index.ts index c48cbaf65c..d87ec58b0c 100644 --- a/packages/types/src/sdk/index.ts +++ b/packages/types/src/sdk/index.ts @@ -11,6 +11,7 @@ export * from "./auth" export * from "./locks" export * from "./db" export * from "./middleware" +export * from "./featureFlag" export * from "./environmentVariables" export * from "./auditLogs" export * from "./sso" diff --git a/packages/types/src/sdk/koa.ts b/packages/types/src/sdk/koa.ts index 07ce2efb6e..a7df701171 100644 --- a/packages/types/src/sdk/koa.ts +++ b/packages/types/src/sdk/koa.ts @@ -1,6 +1,6 @@ import { Context, Request } from "koa" import { User, Role, UserRoles, Account, ConfigType } from "../documents" -import { License } from "../sdk" +import { FeatureFlag, License } from "../sdk" import { Files } from "formidable" export interface ContextUser extends Omit { @@ -11,6 +11,7 @@ export interface ContextUser extends Omit { role?: Role roles?: UserRoles csrfToken?: string + featureFlags?: FeatureFlag[] accountPortalAccess?: boolean providerType?: ConfigType account?: Account From 98d9f52f66cdfc44f58d84342e24a12d701d5e33 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 6 Aug 2024 08:09:20 +0200 Subject: [PATCH 16/46] Fix shared-core build --- packages/shared-core/src/filters.ts | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/packages/shared-core/src/filters.ts b/packages/shared-core/src/filters.ts index d30f591abc..f4e40f50cb 100644 --- a/packages/shared-core/src/filters.ts +++ b/packages/shared-core/src/filters.ts @@ -17,6 +17,7 @@ import { Table, BasicOperator, RangeOperator, + LogicalOperator, } from "@budibase/types" import dayjs from "dayjs" import { OperatorOptions, SqlNumberTypeRangeMap } from "./constants" @@ -358,6 +359,11 @@ export const buildQuery = (filter: SearchFilter[]) => { high: value, } } + } else if ( + queryOperator === LogicalOperator.AND || + queryOperator === LogicalOperator.OR + ) { + // TODO } else if (query[queryOperator] && operator !== "onEmptyFilter") { if (type === "boolean") { // Transform boolean filters to cope with null. @@ -666,8 +672,26 @@ export const runQuery = (docs: Record[], query: SearchFilters) => { ) const containsAny = match(ArrayOperator.CONTAINS_ANY, _contains("some")) + const and = match( + LogicalOperator.AND, + (_docValue: Record, _testValue: any) => { + // TODO + return false + } + ) + const or = match( + LogicalOperator.AND, + (_docValue: Record, _testValue: any) => { + // TODO + return false + } + ) + const docMatch = (doc: Record) => { - const filterFunctions = { + const filterFunctions: Record< + SearchFilterOperator, + (doc: Record) => boolean + > = { string: stringMatch, fuzzy: fuzzyMatch, range: rangeMatch, @@ -679,6 +703,8 @@ export const runQuery = (docs: Record[], query: SearchFilters) => { contains: contains, containsAny: containsAny, notContains: notContains, + [LogicalOperator.AND]: and, + [LogicalOperator.OR]: or, } const results = Object.entries(query || {}) From 1e6bb7ebd760ee8e8a951392581cc2e79a4c4fd1 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 6 Aug 2024 08:15:30 +0200 Subject: [PATCH 17/46] Fix build on view search --- packages/server/src/api/controllers/row/views.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/server/src/api/controllers/row/views.ts b/packages/server/src/api/controllers/row/views.ts index 12e76155bc..ad35e86de8 100644 --- a/packages/server/src/api/controllers/row/views.ts +++ b/packages/server/src/api/controllers/row/views.ts @@ -6,6 +6,7 @@ import { RequiredKeys, RowSearchParams, SearchFilterKey, + LogicalOperator, } from "@budibase/types" import { dataFilters } from "@budibase/shared-core" import sdk from "../../../sdk" @@ -48,9 +49,17 @@ export async function searchView( // Carry over filters for unused fields Object.keys(body.query).forEach(key => { const operator = key as SearchFilterKey + Object.keys(body.query[operator] || {}).forEach(field => { if (!existingFields.includes(db.removeKeyNumbering(field))) { - query[operator]![field] = body.query[operator]![field] + if ( + operator === LogicalOperator.AND || + operator === LogicalOperator.OR + ) { + // TODO + } else { + query[operator]![field] = body.query[operator]![field] + } } }) }) From b9c1aa05b078367f9acfbfbfabfe2dedeaaa7f83 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 6 Aug 2024 09:10:41 +0200 Subject: [PATCH 18/46] Add todo for types --- packages/server/src/automations/steps/queryRows.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/server/src/automations/steps/queryRows.ts b/packages/server/src/automations/steps/queryRows.ts index 172bbf7f55..abcbd4e359 100644 --- a/packages/server/src/automations/steps/queryRows.ts +++ b/packages/server/src/automations/steps/queryRows.ts @@ -114,10 +114,12 @@ function typeCoercion(filters: SearchFilters, table: Table) { } if (column.type === FieldType.NUMBER) { if (key === "oneOf") { + // @ts-ignore TODO searchParam[property] = value .split(",") .map(item => parseFloat(item)) } else { + // @ts-ignore TODO searchParam[property] = parseFloat(value) } } From 185a3462b53c8c863caac749e03b1b01c5574edb Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 6 Aug 2024 09:11:34 +0200 Subject: [PATCH 19/46] Fix tsconfig paths --- packages/shared-core/tsconfig.build.json | 2 +- packages/shared-core/tsconfig.json | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/shared-core/tsconfig.build.json b/packages/shared-core/tsconfig.build.json index 13e298d71c..c1d5bc96e8 100644 --- a/packages/shared-core/tsconfig.build.json +++ b/packages/shared-core/tsconfig.build.json @@ -18,6 +18,6 @@ }, "tsBuildInfoFile": "dist/tsconfig.tsbuildinfo" }, - "include": ["src/**/*"], + "include": ["src/**/*.ts"], "exclude": ["**/*.spec.ts", "**/*.spec.js", "__mocks__", "src/tests"] } diff --git a/packages/shared-core/tsconfig.json b/packages/shared-core/tsconfig.json index d0c5134f1c..41dcee87c9 100644 --- a/packages/shared-core/tsconfig.json +++ b/packages/shared-core/tsconfig.json @@ -1,9 +1,6 @@ { "extends": "./tsconfig.build.json", "compilerOptions": { - "baseUrl": "..", - "rootDir": "src", - "composite": true, "types": ["node", "jest"] }, "exclude": ["node_modules", "dist"] From a451b6eb3c387a90822633cd9a798860e561ff3a Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 6 Aug 2024 09:58:02 +0100 Subject: [PATCH 20/46] Add Oracle to datasource.spec.ts. --- .../server/src/api/routes/tests/datasource.spec.ts | 10 ++-------- packages/server/src/integrations/oracle.ts | 4 +++- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/packages/server/src/api/routes/tests/datasource.spec.ts b/packages/server/src/api/routes/tests/datasource.spec.ts index 4ca766247b..127ebaa6ae 100644 --- a/packages/server/src/api/routes/tests/datasource.spec.ts +++ b/packages/server/src/api/routes/tests/datasource.spec.ts @@ -164,6 +164,7 @@ describe("/datasources", () => { [DatabaseName.MYSQL, getDatasource(DatabaseName.MYSQL)], [DatabaseName.SQL_SERVER, getDatasource(DatabaseName.SQL_SERVER)], [DatabaseName.MARIADB, getDatasource(DatabaseName.MARIADB)], + [DatabaseName.ORACLE, getDatasource(DatabaseName.ORACLE)], ])("%s", (_, dsProvider) => { let rawDatasource: Datasource beforeEach(async () => { @@ -285,9 +286,6 @@ describe("/datasources", () => { [FieldType.STRING]: { name: stringName, type: FieldType.STRING, - constraints: { - presence: true, - }, }, [FieldType.LONGFORM]: { name: "longform", @@ -381,10 +379,6 @@ describe("/datasources", () => { ), schema: Object.entries(table.schema).reduce( (acc, [fieldName, field]) => { - // the constraint will be unset - as the DB doesn't recognise it as not null - if (fieldName === stringName) { - field.constraints = {} - } acc[fieldName] = expect.objectContaining({ ...field, }) @@ -441,7 +435,7 @@ describe("/datasources", () => { }) describe("info", () => { - it("should fetch information about postgres datasource", async () => { + it("should fetch information about a datasource", async () => { const table = await config.api.table.save( tableForDatasource(datasource, { schema: { diff --git a/packages/server/src/integrations/oracle.ts b/packages/server/src/integrations/oracle.ts index d1c0978b89..545cbf40f6 100644 --- a/packages/server/src/integrations/oracle.ts +++ b/packages/server/src/integrations/oracle.ts @@ -400,7 +400,9 @@ class OracleIntegration extends Sql implements DatasourcePlus { if (oracleConstraint.type === OracleContraintTypes.PRIMARY) { table.primary!.push(columnName) } else if ( - oracleConstraint.type === OracleContraintTypes.NOT_NULL_OR_CHECK + oracleConstraint.type === + OracleContraintTypes.NOT_NULL_OR_CHECK && + oracleConstraint.searchCondition?.endsWith("IS NOT NULL") ) { table.schema[columnName].constraints = { presence: true, From 589909c3a29253d44c98684495ca764402d0f571 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 6 Aug 2024 14:35:23 +0100 Subject: [PATCH 21/46] Bump up the packages/server test runners. --- .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 a17ca352cc..1b236caab5 100644 --- a/.github/workflows/budibase_ci.yml +++ b/.github/workflows/budibase_ci.yml @@ -147,7 +147,7 @@ jobs: fi test-server: - runs-on: budi-tubby-tornado-quad-core-150gb + runs-on: budi-tubby-tornado-quad-core-300gb steps: - name: Checkout repo uses: actions/checkout@v4 From e5ae064d2c3366401d03c06346e66e6d0ddc6517 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 6 Aug 2024 14:14:42 +0100 Subject: [PATCH 22/46] Fix tables showing up multiple times for Oracle in the datasource info endpoint. --- .../src/api/routes/tests/datasource.spec.ts | 45 +++++++++++++------ packages/server/src/integrations/oracle.ts | 6 ++- 2 files changed, 36 insertions(+), 15 deletions(-) diff --git a/packages/server/src/api/routes/tests/datasource.spec.ts b/packages/server/src/api/routes/tests/datasource.spec.ts index 127ebaa6ae..83ce3f6fdd 100644 --- a/packages/server/src/api/routes/tests/datasource.spec.ts +++ b/packages/server/src/api/routes/tests/datasource.spec.ts @@ -17,9 +17,15 @@ import { SupportedSqlTypes, JsonFieldSubType, } from "@budibase/types" -import { DatabaseName, getDatasource } from "../../../integrations/tests/utils" +import { + DatabaseName, + getDatasource, + knexClient, +} from "../../../integrations/tests/utils" import { tableForDatasource } from "../../../tests/utilities/structures" import nock from "nock" +import { Knex } from "knex" +import { uuid } from "@budibase/backend-core/tests/core/utilities/structures" describe("/datasources", () => { const config = setup.getConfig() @@ -167,9 +173,12 @@ describe("/datasources", () => { [DatabaseName.ORACLE, getDatasource(DatabaseName.ORACLE)], ])("%s", (_, dsProvider) => { let rawDatasource: Datasource + let client: Knex + beforeEach(async () => { rawDatasource = await dsProvider datasource = await config.api.datasource.create(rawDatasource) + client = await knexClient(rawDatasource) }) describe("get", () => { @@ -434,21 +443,29 @@ describe("/datasources", () => { }) }) - describe("info", () => { - it("should fetch information about a datasource", async () => { - const table = await config.api.table.save( - tableForDatasource(datasource, { - schema: { - name: { - name: "name", - type: FieldType.STRING, - }, - }, - }) - ) + describe.only("info", () => { + it("should fetch information about a datasource with a single table", async () => { + const tableName = uuid() + await client.schema.createTable(tableName, table => { + table.increments("id").primary() + table.string("name") + }) const info = await config.api.datasource.info(datasource) - expect(info.tableNames).toContain(table.name) + expect(info.tableNames).toEqual([tableName]) + }) + + it("should fetch information about a datasource with multiple tables", async () => { + const tableNames = [uuid(), uuid(), uuid(), uuid()] + for (const tableName of tableNames) { + await client.schema.createTable(tableName, table => { + table.increments("id").primary() + table.string("name") + }) + } + + const info = await config.api.datasource.info(datasource) + expect(info.tableNames).toEqual(expect.arrayContaining(tableNames)) }) }) }) diff --git a/packages/server/src/integrations/oracle.ts b/packages/server/src/integrations/oracle.ts index 545cbf40f6..6139b18844 100644 --- a/packages/server/src/integrations/oracle.ts +++ b/packages/server/src/integrations/oracle.ts @@ -423,7 +423,11 @@ class OracleIntegration extends Sql implements DatasourcePlus { const columnsResponse = await this.internalQuery({ sql: OracleIntegration.COLUMNS_SQL, }) - return (columnsResponse.rows || []).map(row => row.TABLE_NAME) + const tableNames = new Set() + for (const row of columnsResponse.rows || []) { + tableNames.add(row.TABLE_NAME) + } + return Array.from(tableNames) } async testConnection() { From 00970d5db3426a7bb8d214b71dce3c5f51a145fa Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 6 Aug 2024 14:21:07 +0100 Subject: [PATCH 23/46] Fix lint. --- .../server/src/api/routes/tests/datasource.spec.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/server/src/api/routes/tests/datasource.spec.ts b/packages/server/src/api/routes/tests/datasource.spec.ts index 83ce3f6fdd..e79f7f4f48 100644 --- a/packages/server/src/api/routes/tests/datasource.spec.ts +++ b/packages/server/src/api/routes/tests/datasource.spec.ts @@ -25,7 +25,6 @@ import { import { tableForDatasource } from "../../../tests/utilities/structures" import nock from "nock" import { Knex } from "knex" -import { uuid } from "@budibase/backend-core/tests/core/utilities/structures" describe("/datasources", () => { const config = setup.getConfig() @@ -443,9 +442,9 @@ describe("/datasources", () => { }) }) - describe.only("info", () => { + describe("info", () => { it("should fetch information about a datasource with a single table", async () => { - const tableName = uuid() + const tableName = generator.guid() await client.schema.createTable(tableName, table => { table.increments("id").primary() table.string("name") @@ -456,7 +455,12 @@ describe("/datasources", () => { }) it("should fetch information about a datasource with multiple tables", async () => { - const tableNames = [uuid(), uuid(), uuid(), uuid()] + const tableNames = [ + generator.guid(), + generator.guid(), + generator.guid(), + generator.guid(), + ] for (const tableName of tableNames) { await client.schema.createTable(tableName, table => { table.increments("id").primary() From 3b603bdd35eb79d326eaab376728ec58c554ac7c Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 6 Aug 2024 14:29:31 +0100 Subject: [PATCH 24/46] Fix datasource.spec.ts tests. --- .../src/api/routes/tests/datasource.spec.ts | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/packages/server/src/api/routes/tests/datasource.spec.ts b/packages/server/src/api/routes/tests/datasource.spec.ts index e79f7f4f48..237133e639 100644 --- a/packages/server/src/api/routes/tests/datasource.spec.ts +++ b/packages/server/src/api/routes/tests/datasource.spec.ts @@ -444,6 +444,10 @@ describe("/datasources", () => { describe("info", () => { it("should fetch information about a datasource with a single table", async () => { + const existingTableNames = ( + await config.api.datasource.info(datasource) + ).tableNames + const tableName = generator.guid() await client.schema.createTable(tableName, table => { table.increments("id").primary() @@ -451,10 +455,17 @@ describe("/datasources", () => { }) const info = await config.api.datasource.info(datasource) - expect(info.tableNames).toEqual([tableName]) + expect(info.tableNames).toEqual( + expect.arrayContaining([tableName, ...existingTableNames]) + ) + expect(info.tableNames).toHaveLength(existingTableNames.length + 1) }) it("should fetch information about a datasource with multiple tables", async () => { + const existingTableNames = ( + await config.api.datasource.info(datasource) + ).tableNames + const tableNames = [ generator.guid(), generator.guid(), @@ -469,7 +480,12 @@ describe("/datasources", () => { } const info = await config.api.datasource.info(datasource) - expect(info.tableNames).toEqual(expect.arrayContaining(tableNames)) + expect(info.tableNames).toEqual( + expect.arrayContaining([...tableNames, ...existingTableNames]) + ) + expect(info.tableNames).toHaveLength( + existingTableNames.length + tableNames.length + ) }) }) }) From 2c104a470344749aeda0ff8b811676612c56a252 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Tue, 6 Aug 2024 14:36:24 +0100 Subject: [PATCH 25/46] Update packages/server to use new larger runners. --- .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 a17ca352cc..1b236caab5 100644 --- a/.github/workflows/budibase_ci.yml +++ b/.github/workflows/budibase_ci.yml @@ -147,7 +147,7 @@ jobs: fi test-server: - runs-on: budi-tubby-tornado-quad-core-150gb + runs-on: budi-tubby-tornado-quad-core-300gb steps: - name: Checkout repo uses: actions/checkout@v4 From 3b40db5db02852e3838a3f6618746e4a7e27a725 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 6 Aug 2024 10:30:19 +0200 Subject: [PATCH 26/46] Run tests for in-memory --- packages/server/src/api/routes/tests/search.spec.ts | 4 ++-- 1 file changed, 2 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 6ed9245ee6..e3f17c36cb 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2697,7 +2697,7 @@ describe.each([ }) }) - isSql && + !isLucene && describe("$and", () => { beforeAll(async () => { table = await createTable({ @@ -2771,7 +2771,7 @@ describe.each([ }) }) - isSql && + !isLucene && describe("$or", () => { beforeAll(async () => { table = await createTable({ From 5caf20635aaedc40cd8afae7c3eaaddcce9c951d Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 6 Aug 2024 17:34:40 +0200 Subject: [PATCH 27/46] Implement in memory filter --- packages/shared-core/src/filters.ts | 39 +++++++++++++++++++++++------ 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/packages/shared-core/src/filters.ts b/packages/shared-core/src/filters.ts index f4e40f50cb..bad6d36cba 100644 --- a/packages/shared-core/src/filters.ts +++ b/packages/shared-core/src/filters.ts @@ -464,14 +464,18 @@ export const runQuery = (docs: Record[], query: SearchFilters) => { ) => (doc: Record) => { for (const [key, testValue] of Object.entries(query[type] || {})) { - const result = test(deepGet(doc, removeKeyNumbering(key)), testValue) + const valueToCheck = + type === LogicalOperator.AND || type === LogicalOperator.OR + ? doc + : deepGet(doc, removeKeyNumbering(key)) + const result = test(valueToCheck, testValue) if (query.allOr && result) { return true } else if (!query.allOr && !result) { return false } } - return true + return !query.allOr } const stringMatch = match( @@ -674,15 +678,34 @@ export const runQuery = (docs: Record[], query: SearchFilters) => { const and = match( LogicalOperator.AND, - (_docValue: Record, _testValue: any) => { - // TODO - return false + (docValue: Record, conditions: any) => { + if (!conditions.length) { + return false + } + for (const condition of conditions) { + const matchesCondition = runQuery([docValue], condition) + if (!matchesCondition.length) { + return false + } + } + return true } ) const or = match( - LogicalOperator.AND, - (_docValue: Record, _testValue: any) => { - // TODO + LogicalOperator.OR, + (docValue: Record, conditions: any) => { + if (!conditions.length) { + return false + } + for (const condition of conditions) { + const matchesCondition = runQuery([docValue], { + ...condition, + allOr: true, + }) + if (matchesCondition.length) { + return true + } + } return false } ) From 0667449463161e37a81080d3a03596ffd40948ce Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Tue, 6 Aug 2024 18:25:35 +0200 Subject: [PATCH 28/46] Types --- packages/shared-core/src/filters.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/shared-core/src/filters.ts b/packages/shared-core/src/filters.ts index bad6d36cba..e16e27fd30 100644 --- a/packages/shared-core/src/filters.ts +++ b/packages/shared-core/src/filters.ts @@ -678,7 +678,7 @@ export const runQuery = (docs: Record[], query: SearchFilters) => { const and = match( LogicalOperator.AND, - (docValue: Record, conditions: any) => { + (docValue: Record, conditions: SearchFilters[]) => { if (!conditions.length) { return false } @@ -693,7 +693,7 @@ export const runQuery = (docs: Record[], query: SearchFilters) => { ) const or = match( LogicalOperator.OR, - (docValue: Record, conditions: any) => { + (docValue: Record, conditions: SearchFilters[]) => { if (!conditions.length) { return false } From 24463bd38775951122e6b31fc95f372b57264b8e Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 7 Aug 2024 10:51:40 +0200 Subject: [PATCH 29/46] Add helper --- packages/types/src/sdk/search.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/types/src/sdk/search.ts b/packages/types/src/sdk/search.ts index 11bfb12b12..6feea40766 100644 --- a/packages/types/src/sdk/search.ts +++ b/packages/types/src/sdk/search.ts @@ -28,6 +28,12 @@ export enum LogicalOperator { OR = "$or", } +export function isLogicalSearchOperator( + value: string +): value is LogicalOperator { + return value === LogicalOperator.AND || value === LogicalOperator.OR +} + export type SearchFilterOperator = | BasicOperator | ArrayOperator From c5f504d724d41c315e0ac52e1b41087d09564b3a Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 7 Aug 2024 10:54:19 +0200 Subject: [PATCH 30/46] Use helper --- .../server/src/sdk/app/rows/search/internal/sqs.ts | 7 ++----- packages/shared-core/src/filters.ts | 13 +++++-------- 2 files changed, 7 insertions(+), 13 deletions(-) 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 bbcbf2c7cd..66ec905c61 100644 --- a/packages/server/src/sdk/app/rows/search/internal/sqs.ts +++ b/packages/server/src/sdk/app/rows/search/internal/sqs.ts @@ -2,7 +2,7 @@ import { Datasource, DocumentType, FieldType, - LogicalOperator, + isLogicalSearchOperator, Operation, QueryJson, RelationshipFieldMetadata, @@ -141,10 +141,7 @@ function cleanupFilters( const prefixFilters = (filters: SearchFilters) => { for (const filterKey of Object.keys(filters) as (keyof SearchFilters)[]) { - if ( - filterKey === LogicalOperator.AND || - filterKey === LogicalOperator.OR - ) { + if (isLogicalSearchOperator(filterKey)) { for (const condition of filters[filterKey]!.conditions) { prefixFilters(condition) } diff --git a/packages/shared-core/src/filters.ts b/packages/shared-core/src/filters.ts index e16e27fd30..e1a783175d 100644 --- a/packages/shared-core/src/filters.ts +++ b/packages/shared-core/src/filters.ts @@ -18,6 +18,7 @@ import { BasicOperator, RangeOperator, LogicalOperator, + isLogicalSearchOperator, } from "@budibase/types" import dayjs from "dayjs" import { OperatorOptions, SqlNumberTypeRangeMap } from "./constants" @@ -359,10 +360,7 @@ export const buildQuery = (filter: SearchFilter[]) => { high: value, } } - } else if ( - queryOperator === LogicalOperator.AND || - queryOperator === LogicalOperator.OR - ) { + } else if (isLogicalSearchOperator(queryOperator)) { // TODO } else if (query[queryOperator] && operator !== "onEmptyFilter") { if (type === "boolean") { @@ -464,10 +462,9 @@ export const runQuery = (docs: Record[], query: SearchFilters) => { ) => (doc: Record) => { for (const [key, testValue] of Object.entries(query[type] || {})) { - const valueToCheck = - type === LogicalOperator.AND || type === LogicalOperator.OR - ? doc - : deepGet(doc, removeKeyNumbering(key)) + const valueToCheck = isLogicalSearchOperator(type) + ? doc + : deepGet(doc, removeKeyNumbering(key)) const result = test(valueToCheck, testValue) if (query.allOr && result) { return true From b5423d71a0daf3f949c2942573494ea6724998d2 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 7 Aug 2024 11:16:18 +0100 Subject: [PATCH 31/46] Add detailed tracing to searches. --- packages/server/src/sdk/app/rows/search.ts | 78 ++++++++++++++++------ 1 file changed, 56 insertions(+), 22 deletions(-) diff --git a/packages/server/src/sdk/app/rows/search.ts b/packages/server/src/sdk/app/rows/search.ts index c008d43548..1ccd89639b 100644 --- a/packages/server/src/sdk/app/rows/search.ts +++ b/packages/server/src/sdk/app/rows/search.ts @@ -13,6 +13,7 @@ import { dataFilters } from "@budibase/shared-core" import sdk from "../../index" import { searchInputMapping } from "./search/utils" import { db as dbCore } from "@budibase/backend-core" +import tracer from "dd-trace" export { isValidFilter } from "../../../integrations/utils" @@ -32,32 +33,65 @@ function pickApi(tableId: any) { export async function search( options: RowSearchParams ): Promise> { - const isExternalTable = isExternalTableID(options.tableId) - options.query = dataFilters.cleanupQuery(options.query || {}) - options.query = dataFilters.fixupFilterArrays(options.query) - if ( - !dataFilters.hasFilters(options.query) && - options.query.onEmptyFilter === EmptyFilterOption.RETURN_NONE - ) { - return { - rows: [], + return await tracer.trace("search", async span => { + span?.addTags({ + tableId: options.tableId, + query: options.query, + sort: options.sort, + sortOrder: options.sortOrder, + sortType: options.sortType, + limit: options.limit, + bookmark: options.bookmark, + paginate: options.paginate, + fields: options.fields, + countRows: options.countRows, + }) + + const isExternalTable = isExternalTableID(options.tableId) + options.query = dataFilters.cleanupQuery(options.query || {}) + options.query = dataFilters.fixupFilterArrays(options.query) + + span?.addTags({ + cleanedQuery: options.query, + isExternalTable, + }) + + if ( + !dataFilters.hasFilters(options.query) && + options.query.onEmptyFilter === EmptyFilterOption.RETURN_NONE + ) { + span?.addTags({ emptyQuery: true }) + return { + rows: [], + } } - } - if (options.sortOrder) { - options.sortOrder = options.sortOrder.toLowerCase() as SortOrder - } + if (options.sortOrder) { + options.sortOrder = options.sortOrder.toLowerCase() as SortOrder + } - const table = await sdk.tables.getTable(options.tableId) - options = searchInputMapping(table, options) + const table = await sdk.tables.getTable(options.tableId) + options = searchInputMapping(table, options) - if (isExternalTable) { - return external.search(options, table) - } else if (dbCore.isSqsEnabledForTenant()) { - return internal.sqs.search(options, table) - } else { - return internal.lucene.search(options, table) - } + let result: SearchResponse + if (isExternalTable) { + span?.addTags({ searchType: "external" }) + result = await external.search(options, table) + } else if (dbCore.isSqsEnabledForTenant()) { + span?.addTags({ searchType: "sqs" }) + result = await internal.sqs.search(options, table) + } else { + span?.addTags({ searchType: "lucene" }) + result = await internal.lucene.search(options, table) + } + + span?.addTags({ + foundRows: result.rows.length, + totalRows: result.totalRows, + }) + + return result + }) } export async function exportRows( From 8191552352875a020582f018da734eee5a55f53d Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 7 Aug 2024 12:36:51 +0200 Subject: [PATCH 32/46] Bypass view --- .../src/api/routes/tests/viewV2.spec.ts | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 8c0bc39234..b2ac580f6c 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -1485,6 +1485,43 @@ describe.each([ } ) }) + + it("cannot bypass a view filter", async () => { + await config.api.row.save(table._id!, { + one: "foo", + two: "bar", + }) + await config.api.row.save(table._id!, { + one: "foo2", + two: "bar2", + }) + + const view = await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), + query: [ + { + operator: BasicOperator.EQUAL, + field: "two", + value: "bar2", + }, + ], + schema: { + id: { visible: true }, + one: { visible: false }, + two: { visible: true }, + }, + }) + + const response = await config.api.viewV2.search(view.id, { + query: { + equal: { + two: "bar", + }, + }, + }) + expect(response.rows).toHaveLength(0) + }) }) describe("permissions", () => { From 42b6b6e9190c8bf601b94b59ecb165d1bf44afc8 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 7 Aug 2024 12:37:37 +0200 Subject: [PATCH 33/46] Fix --- .../server/src/api/controllers/row/views.ts | 32 ++++--------------- 1 file changed, 6 insertions(+), 26 deletions(-) diff --git a/packages/server/src/api/controllers/row/views.ts b/packages/server/src/api/controllers/row/views.ts index ad35e86de8..643b95df6f 100644 --- a/packages/server/src/api/controllers/row/views.ts +++ b/packages/server/src/api/controllers/row/views.ts @@ -5,12 +5,10 @@ import { SearchViewRowRequest, RequiredKeys, RowSearchParams, - SearchFilterKey, - LogicalOperator, } from "@budibase/types" import { dataFilters } from "@budibase/shared-core" import sdk from "../../../sdk" -import { db, context } from "@budibase/backend-core" +import { context } from "@budibase/backend-core" import { enrichSearchContext } from "./utils" export async function searchView( @@ -36,33 +34,15 @@ export async function searchView( // that could let users find rows they should not be allowed to access. let query = dataFilters.buildQuery(view.query || []) if (body.query) { - // Extract existing fields - const existingFields = - view.query - ?.filter(filter => filter.field) - .map(filter => db.removeKeyNumbering(filter.field)) || [] - // Delete extraneous search params that cannot be overridden delete body.query.allOr delete body.query.onEmptyFilter - // Carry over filters for unused fields - Object.keys(body.query).forEach(key => { - const operator = key as SearchFilterKey - - Object.keys(body.query[operator] || {}).forEach(field => { - if (!existingFields.includes(db.removeKeyNumbering(field))) { - if ( - operator === LogicalOperator.AND || - operator === LogicalOperator.OR - ) { - // TODO - } else { - query[operator]![field] = body.query[operator]![field] - } - } - }) - }) + query = { + $and: { + conditions: [query, body.query], + }, + } } await context.ensureSnippetContext(true) From 94b6737bdcbe4bb05a42629bd401e3c3d7caf7f8 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 7 Aug 2024 12:40:03 +0200 Subject: [PATCH 34/46] Add extra tests --- .../src/api/routes/tests/viewV2.spec.ts | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index b2ac580f6c..6edecde266 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -1486,6 +1486,39 @@ describe.each([ ) }) + it("can filter a view without a view filter", async () => { + const one = await config.api.row.save(table._id!, { + one: "foo", + two: "bar", + }) + await config.api.row.save(table._id!, { + one: "foo2", + two: "bar2", + }) + + const view = await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), + schema: { + id: { visible: true }, + one: { visible: false }, + two: { visible: true }, + }, + }) + + const response = await config.api.viewV2.search(view.id, { + query: { + equal: { + two: "bar", + }, + }, + }) + expect(response.rows).toHaveLength(1) + expect(response.rows).toEqual([ + expect.objectContaining({ _id: one._id }), + ]) + }) + it("cannot bypass a view filter", async () => { await config.api.row.save(table._id!, { one: "foo", From 28f11a5765a3173e92cf324ad4cc1fcbc6f61a1d Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 7 Aug 2024 13:15:30 +0200 Subject: [PATCH 35/46] Don't add breaking changes --- .../server/src/api/controllers/row/views.ts | 31 ++++++++++--- .../src/api/routes/tests/viewV2.spec.ts | 43 +++++++++++++++++++ 2 files changed, 69 insertions(+), 5 deletions(-) diff --git a/packages/server/src/api/controllers/row/views.ts b/packages/server/src/api/controllers/row/views.ts index 643b95df6f..c38a415aa2 100644 --- a/packages/server/src/api/controllers/row/views.ts +++ b/packages/server/src/api/controllers/row/views.ts @@ -5,11 +5,14 @@ import { SearchViewRowRequest, RequiredKeys, RowSearchParams, + SearchFilterKey, + LogicalOperator, } from "@budibase/types" import { dataFilters } from "@budibase/shared-core" import sdk from "../../../sdk" -import { context } from "@budibase/backend-core" +import { db, context } from "@budibase/backend-core" import { enrichSearchContext } from "./utils" +import { isExternalTableID } from "../../../integrations/utils" export async function searchView( ctx: UserCtx @@ -38,10 +41,28 @@ export async function searchView( delete body.query.allOr delete body.query.onEmptyFilter - query = { - $and: { - conditions: [query, body.query], - }, + if (!isExternalTableID(view.tableId) && !db.isSqsEnabledForTenant()) { + // Extract existing fields + const existingFields = + view.query + ?.filter(filter => filter.field) + .map(filter => db.removeKeyNumbering(filter.field)) || [] + + // Carry over filters for unused fields + Object.keys(body.query).forEach(key => { + const operator = key as Exclude + Object.keys(body.query[operator] || {}).forEach(field => { + if (!existingFields.includes(db.removeKeyNumbering(field))) { + query[operator]![field] = body.query[operator]![field] + } + }) + }) + } else { + query = { + $and: { + conditions: [query, body.query], + }, + } } } diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 6edecde266..02dbb70666 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -1486,6 +1486,48 @@ describe.each([ ) }) + isLucene && + it("in lucene, cannot override a view filter", async () => { + await config.api.row.save(table._id!, { + one: "foo", + two: "bar", + }) + const two = await config.api.row.save(table._id!, { + one: "foo2", + two: "bar2", + }) + + const view = await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), + query: [ + { + operator: BasicOperator.EQUAL, + field: "two", + value: "bar2", + }, + ], + schema: { + id: { visible: true }, + one: { visible: false }, + two: { visible: true }, + }, + }) + + const response = await config.api.viewV2.search(view.id, { + query: { + equal: { + two: "bar", + }, + }, + }) + expect(response.rows).toHaveLength(1) + expect(response.rows).toEqual([ + expect.objectContaining({ _id: two._id }), + ]) + }) + + !isLucene && it("can filter a view without a view filter", async () => { const one = await config.api.row.save(table._id!, { one: "foo", @@ -1519,6 +1561,7 @@ describe.each([ ]) }) + !isLucene && it("cannot bypass a view filter", async () => { await config.api.row.save(table._id!, { one: "foo", From 00cf4e48e3247cea8b68b73ca2e6ae7a49bcdc7e Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 7 Aug 2024 13:21:28 +0200 Subject: [PATCH 36/46] Lint --- .../src/api/routes/tests/viewV2.spec.ts | 120 +++++++++--------- 1 file changed, 60 insertions(+), 60 deletions(-) diff --git a/packages/server/src/api/routes/tests/viewV2.spec.ts b/packages/server/src/api/routes/tests/viewV2.spec.ts index 02dbb70666..ea6aedbe3c 100644 --- a/packages/server/src/api/routes/tests/viewV2.spec.ts +++ b/packages/server/src/api/routes/tests/viewV2.spec.ts @@ -1528,76 +1528,76 @@ describe.each([ }) !isLucene && - it("can filter a view without a view filter", async () => { - const one = await config.api.row.save(table._id!, { - one: "foo", - two: "bar", - }) - await config.api.row.save(table._id!, { - one: "foo2", - two: "bar2", - }) + it("can filter a view without a view filter", async () => { + const one = await config.api.row.save(table._id!, { + one: "foo", + two: "bar", + }) + await config.api.row.save(table._id!, { + one: "foo2", + two: "bar2", + }) - const view = await config.api.viewV2.create({ - tableId: table._id!, - name: generator.guid(), - schema: { - id: { visible: true }, - one: { visible: false }, - two: { visible: true }, - }, - }) - - const response = await config.api.viewV2.search(view.id, { - query: { - equal: { - two: "bar", + const view = await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), + schema: { + id: { visible: true }, + one: { visible: false }, + two: { visible: true }, }, - }, + }) + + const response = await config.api.viewV2.search(view.id, { + query: { + equal: { + two: "bar", + }, + }, + }) + expect(response.rows).toHaveLength(1) + expect(response.rows).toEqual([ + expect.objectContaining({ _id: one._id }), + ]) }) - expect(response.rows).toHaveLength(1) - expect(response.rows).toEqual([ - expect.objectContaining({ _id: one._id }), - ]) - }) !isLucene && - it("cannot bypass a view filter", async () => { - await config.api.row.save(table._id!, { - one: "foo", - two: "bar", - }) - await config.api.row.save(table._id!, { - one: "foo2", - two: "bar2", - }) + it("cannot bypass a view filter", async () => { + await config.api.row.save(table._id!, { + one: "foo", + two: "bar", + }) + await config.api.row.save(table._id!, { + one: "foo2", + two: "bar2", + }) - const view = await config.api.viewV2.create({ - tableId: table._id!, - name: generator.guid(), - query: [ - { - operator: BasicOperator.EQUAL, - field: "two", - value: "bar2", + const view = await config.api.viewV2.create({ + tableId: table._id!, + name: generator.guid(), + query: [ + { + operator: BasicOperator.EQUAL, + field: "two", + value: "bar2", + }, + ], + schema: { + id: { visible: true }, + one: { visible: false }, + two: { visible: true }, }, - ], - schema: { - id: { visible: true }, - one: { visible: false }, - two: { visible: true }, - }, - }) + }) - const response = await config.api.viewV2.search(view.id, { - query: { - equal: { - two: "bar", + const response = await config.api.viewV2.search(view.id, { + query: { + equal: { + two: "bar", + }, }, - }, + }) + expect(response.rows).toHaveLength(0) }) - expect(response.rows).toHaveLength(0) - }) }) describe("permissions", () => { From d6e1bcb3829fafba06a95b9f66230021524363f0 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 7 Aug 2024 13:56:25 +0200 Subject: [PATCH 37/46] Type search validators --- packages/server/src/api/routes/utils/validators.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/server/src/api/routes/utils/validators.ts b/packages/server/src/api/routes/utils/validators.ts index 671ce95038..0322120ecf 100644 --- a/packages/server/src/api/routes/utils/validators.ts +++ b/packages/server/src/api/routes/utils/validators.ts @@ -1,6 +1,6 @@ import { auth, permissions } from "@budibase/backend-core" import { DataSourceOperation } from "../../../constants" -import { Table, WebhookActionType } from "@budibase/types" +import { SearchFilters, Table, WebhookActionType } from "@budibase/types" import Joi, { CustomValidator } from "joi" import { ValidSnippetNameRegex, helpers } from "@budibase/shared-core" import sdk from "../../../sdk" @@ -84,7 +84,7 @@ export function datasourceValidator() { } function filterObject() { - return Joi.object({ + const filtersValidators: Record = { string: Joi.object().optional(), fuzzy: Joi.object().optional(), range: Joi.object().optional(), @@ -96,7 +96,8 @@ function filterObject() { contains: Joi.object().optional(), notContains: Joi.object().optional(), allOr: Joi.boolean().optional(), - }).unknown(true) + } + return Joi.object(filtersValidators).unknown(true) } export function internalSearchValidator() { From 8d1c658c7c744f0a3a7d984600b8830c91d6075d Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 7 Aug 2024 13:57:34 +0200 Subject: [PATCH 38/46] Add containsAny validator --- packages/server/src/api/routes/utils/validators.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/server/src/api/routes/utils/validators.ts b/packages/server/src/api/routes/utils/validators.ts index 0322120ecf..9b9a763e2c 100644 --- a/packages/server/src/api/routes/utils/validators.ts +++ b/packages/server/src/api/routes/utils/validators.ts @@ -95,6 +95,7 @@ function filterObject() { oneOf: Joi.object().optional(), contains: Joi.object().optional(), notContains: Joi.object().optional(), + containsAny: Joi.object().optional(), allOr: Joi.boolean().optional(), } return Joi.object(filtersValidators).unknown(true) From c6f7f0133dcb52aa77ee0e5c906a5ce2d0b19695 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 7 Aug 2024 13:57:54 +0200 Subject: [PATCH 39/46] Disallow fuzzyOr and documentType --- packages/server/src/api/routes/utils/validators.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/server/src/api/routes/utils/validators.ts b/packages/server/src/api/routes/utils/validators.ts index 9b9a763e2c..ddd98f3858 100644 --- a/packages/server/src/api/routes/utils/validators.ts +++ b/packages/server/src/api/routes/utils/validators.ts @@ -97,6 +97,8 @@ function filterObject() { notContains: Joi.object().optional(), containsAny: Joi.object().optional(), allOr: Joi.boolean().optional(), + fuzzyOr: Joi.disallow(), + documentType: Joi.disallow(), } return Joi.object(filtersValidators).unknown(true) } From 22d9b930fc280c7ca129f05aae1d5b6232ee1f8b Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 7 Aug 2024 14:02:09 +0200 Subject: [PATCH 40/46] Validate onEmptyFilter --- packages/server/src/api/routes/utils/validators.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/server/src/api/routes/utils/validators.ts b/packages/server/src/api/routes/utils/validators.ts index ddd98f3858..72c97396db 100644 --- a/packages/server/src/api/routes/utils/validators.ts +++ b/packages/server/src/api/routes/utils/validators.ts @@ -1,6 +1,11 @@ import { auth, permissions } from "@budibase/backend-core" import { DataSourceOperation } from "../../../constants" -import { SearchFilters, Table, WebhookActionType } from "@budibase/types" +import { + EmptyFilterOption, + SearchFilters, + Table, + WebhookActionType, +} from "@budibase/types" import Joi, { CustomValidator } from "joi" import { ValidSnippetNameRegex, helpers } from "@budibase/shared-core" import sdk from "../../../sdk" @@ -97,6 +102,9 @@ function filterObject() { notContains: Joi.object().optional(), containsAny: Joi.object().optional(), allOr: Joi.boolean().optional(), + onEmptyFilter: Joi.string() + .optional() + .valid(...Object.values(EmptyFilterOption)), fuzzyOr: Joi.disallow(), documentType: Joi.disallow(), } From 00c12b9686dd07d6a466d099ae6b90ccc49e62a8 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 7 Aug 2024 14:32:42 +0200 Subject: [PATCH 41/46] Add tests --- .../src/api/routes/tests/search.spec.ts | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index e3f17c36cb..05f9e1676e 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2769,6 +2769,37 @@ describe.each([ }, }).toFindNothing() }) + + it("validates conditions that are not objects", async () => { + await expect( + expectQuery({ + $and: { + conditions: [{ equal: { age: 10 } }, "invalidCondition" as any], + }, + }).toFindNothing() + ).rejects.toThrow( + 'Invalid body - "query.$and.conditions[1]" must be of type object' + ) + }) + + it("validates $and without conditions", async () => { + await expect( + expectQuery({ + $and: { + conditions: [ + { equal: { age: 10 } }, + { + $and: { + conditions: undefined as any, + }, + }, + ], + }, + }).toFindNothing() + ).rejects.toThrow( + 'Invalid body - "query.$and.conditions[1].$and.conditions" is required' + ) + }) }) !isLucene && From bc7ab264b0500410f18a93f2a9e0bddd0893f125 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 7 Aug 2024 14:32:59 +0200 Subject: [PATCH 42/46] Add validations --- packages/server/src/api/routes/utils/validators.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/server/src/api/routes/utils/validators.ts b/packages/server/src/api/routes/utils/validators.ts index 72c97396db..9aa112cf4d 100644 --- a/packages/server/src/api/routes/utils/validators.ts +++ b/packages/server/src/api/routes/utils/validators.ts @@ -89,6 +89,11 @@ export function datasourceValidator() { } function filterObject() { + const conditionalFilteringObject = () => + Joi.object({ + conditions: Joi.array().items(Joi.link("#schema")).required(), + }) + const filtersValidators: Record = { string: Joi.object().optional(), fuzzy: Joi.object().optional(), @@ -105,10 +110,12 @@ function filterObject() { onEmptyFilter: Joi.string() .optional() .valid(...Object.values(EmptyFilterOption)), - fuzzyOr: Joi.disallow(), - documentType: Joi.disallow(), + $and: conditionalFilteringObject(), + $or: conditionalFilteringObject(), + fuzzyOr: Joi.forbidden(), + documentType: Joi.forbidden(), } - return Joi.object(filtersValidators).unknown(true) + return Joi.object(filtersValidators).unknown(true).id("schema") } export function internalSearchValidator() { From 32702f2e9db4f7b53df67b9775a6733eee2a1353 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 7 Aug 2024 14:39:05 +0200 Subject: [PATCH 43/46] Don't validate for in-memory --- packages/server/src/api/routes/tests/search.spec.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 05f9e1676e..a92e2d3164 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2770,6 +2770,7 @@ describe.each([ }).toFindNothing() }) + !isInMemory && it("validates conditions that are not objects", async () => { await expect( expectQuery({ @@ -2782,6 +2783,7 @@ describe.each([ ) }) + !isInMemory && it("validates $and without conditions", async () => { await expect( expectQuery({ From 44a053ee0812e974d10aea4d7fd844aace9529bb Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 7 Aug 2024 14:40:23 +0200 Subject: [PATCH 44/46] Lint --- .../src/api/routes/tests/search.spec.ts | 56 +++++++++---------- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index a92e2d3164..9f0a9e6269 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -2771,37 +2771,37 @@ describe.each([ }) !isInMemory && - it("validates conditions that are not objects", async () => { - await expect( - expectQuery({ - $and: { - conditions: [{ equal: { age: 10 } }, "invalidCondition" as any], - }, - }).toFindNothing() - ).rejects.toThrow( - 'Invalid body - "query.$and.conditions[1]" must be of type object' - ) - }) + it("validates conditions that are not objects", async () => { + await expect( + expectQuery({ + $and: { + conditions: [{ equal: { age: 10 } }, "invalidCondition" as any], + }, + }).toFindNothing() + ).rejects.toThrow( + 'Invalid body - "query.$and.conditions[1]" must be of type object' + ) + }) !isInMemory && - it("validates $and without conditions", async () => { - await expect( - expectQuery({ - $and: { - conditions: [ - { equal: { age: 10 } }, - { - $and: { - conditions: undefined as any, + it("validates $and without conditions", async () => { + await expect( + expectQuery({ + $and: { + conditions: [ + { equal: { age: 10 } }, + { + $and: { + conditions: undefined as any, + }, }, - }, - ], - }, - }).toFindNothing() - ).rejects.toThrow( - 'Invalid body - "query.$and.conditions[1].$and.conditions" is required' - ) - }) + ], + }, + }).toFindNothing() + ).rejects.toThrow( + 'Invalid body - "query.$and.conditions[1].$and.conditions" is required' + ) + }) }) !isLucene && From 80ff96308241d330fb24e9a997a8823a5dd50c61 Mon Sep 17 00:00:00 2001 From: Adria Navarro Date: Wed, 7 Aug 2024 17:01:59 +0200 Subject: [PATCH 45/46] Remove unnecessary coercion --- .../server/src/automations/steps/queryRows.ts | 41 +------------------ 1 file changed, 2 insertions(+), 39 deletions(-) diff --git a/packages/server/src/automations/steps/queryRows.ts b/packages/server/src/automations/steps/queryRows.ts index abcbd4e359..526e994c1f 100644 --- a/packages/server/src/automations/steps/queryRows.ts +++ b/packages/server/src/automations/steps/queryRows.ts @@ -11,13 +11,10 @@ import { AutomationStepSchema, AutomationStepType, EmptyFilterOption, - SearchFilters, - Table, SortOrder, QueryRowsStepInputs, QueryRowsStepOutputs, } from "@budibase/types" -import { db as dbCore } from "@budibase/backend-core" const SortOrderPretty = { [SortOrder.ASCENDING]: "Ascending", @@ -95,40 +92,6 @@ async function getTable(appId: string, tableId: string) { return ctx.body } -function typeCoercion(filters: SearchFilters, table: Table) { - if (!filters || !table) { - return filters - } - for (let key of Object.keys(filters)) { - const searchParam = filters[key as keyof SearchFilters] - if (typeof searchParam === "object") { - for (let [property, value] of Object.entries(searchParam)) { - // We need to strip numerical prefixes here, so that we can look up - // the correct field name in the schema - const columnName = dbCore.removeKeyNumbering(property) - const column = table.schema[columnName] - - // convert string inputs - if (!column || typeof value !== "string") { - continue - } - if (column.type === FieldType.NUMBER) { - if (key === "oneOf") { - // @ts-ignore TODO - searchParam[property] = value - .split(",") - .map(item => parseFloat(item)) - } else { - // @ts-ignore TODO - searchParam[property] = parseFloat(value) - } - } - } - } - } - return filters -} - function hasNullFilters(filters: any[]) { return ( filters.length === 0 || @@ -159,7 +122,7 @@ export async function run({ sortType = fieldType === FieldType.NUMBER ? FieldType.NUMBER : FieldType.STRING } - const ctx: any = buildCtx(appId, null, { + const ctx = buildCtx(appId, null, { params: { tableId, }, @@ -167,7 +130,7 @@ export async function run({ sortType, limit, sort: sortColumn, - query: typeCoercion(filters || {}, table), + query: filters || {}, // default to ascending, like data tab sortOrder: sortOrder || SortOrder.ASCENDING, }, From 4b716ac6ea0d0b6df43e1e58da3bfd31508f8375 Mon Sep 17 00:00:00 2001 From: Budibase Staging Release Bot <> Date: Wed, 7 Aug 2024 15:03:18 +0000 Subject: [PATCH 46/46] Bump version to 2.29.30 --- lerna.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lerna.json b/lerna.json index 19303c8580..1b6574c7df 100644 --- a/lerna.json +++ b/lerna.json @@ -1,6 +1,6 @@ { "$schema": "node_modules/lerna/schemas/lerna-schema.json", - "version": "2.29.29", + "version": "2.29.30", "npmClient": "yarn", "packages": [ "packages/*",