From d4ec07fa8a2100345b5684c6f16fa2d403bf7b94 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 28 Nov 2024 15:30:10 +0000 Subject: [PATCH 1/8] Removing the datasource query API for datasource plus, this isn't used and has no real purpose (internal tables aren't supported) removing it as it exposes very internal types to the API, making it harder to adjust these types. --- .../server/src/api/controllers/datasource.ts | 10 ----- packages/server/src/api/routes/datasource.ts | 9 ---- .../routes/tests/queries/generic-sql.spec.ts | 43 ------------------- .../src/tests/utilities/api/datasource.ts | 10 ----- 4 files changed, 72 deletions(-) diff --git a/packages/server/src/api/controllers/datasource.ts b/packages/server/src/api/controllers/datasource.ts index 794e2dfddd..97cf8db299 100644 --- a/packages/server/src/api/controllers/datasource.ts +++ b/packages/server/src/api/controllers/datasource.ts @@ -296,16 +296,6 @@ export async function find(ctx: UserCtx) { ctx.body = await sdk.datasources.removeSecretSingle(datasource) } -// dynamic query functionality -export async function query(ctx: UserCtx) { - const queryJson = ctx.request.body - try { - ctx.body = await sdk.rows.utils.getDatasourceAndQuery(queryJson) - } catch (err: any) { - ctx.throw(400, err) - } -} - export async function getExternalSchema(ctx: UserCtx) { const datasource = await sdk.datasources.get(ctx.params.datasourceId) const enrichedDatasource = await sdk.datasources.getAndMergeDatasource( diff --git a/packages/server/src/api/routes/datasource.ts b/packages/server/src/api/routes/datasource.ts index 755088c56c..22f9a77cc9 100644 --- a/packages/server/src/api/routes/datasource.ts +++ b/packages/server/src/api/routes/datasource.ts @@ -41,15 +41,6 @@ router ), datasourceController.update ) - .post( - "/api/datasources/query", - authorized( - permissions.PermissionType.TABLE, - permissions.PermissionLevel.READ - ), - datasourceQueryValidator(), - datasourceController.query - ) .post( "/api/datasources/:datasourceId/schema", authorized(permissions.BUILDER), diff --git a/packages/server/src/api/routes/tests/queries/generic-sql.spec.ts b/packages/server/src/api/routes/tests/queries/generic-sql.spec.ts index 44b21e0350..001f4890fd 100644 --- a/packages/server/src/api/routes/tests/queries/generic-sql.spec.ts +++ b/packages/server/src/api/routes/tests/queries/generic-sql.spec.ts @@ -817,49 +817,6 @@ if (descriptions.length) { }) describe("query through datasource", () => { - it("should be able to query the datasource", async () => { - const datasource = await config.api.datasource.create(rawDatasource) - - const entityId = tableName - await config.api.datasource.update({ - ...datasource, - entities: { - [entityId]: { - name: entityId, - schema: {}, - type: "table", - primary: ["id"], - sourceId: datasource._id!, - sourceType: TableSourceType.EXTERNAL, - }, - }, - }) - - const res = await config.api.datasource.query({ - endpoint: { - datasourceId: datasource._id!, - operation: Operation.READ, - entityId, - }, - resource: { - fields: ["id", "name"], - }, - filters: { - string: { - name: "two", - }, - }, - }) - expect(res).toHaveLength(1) - expect(res[0]).toEqual({ - id: 2, - name: "two", - // the use of table.* introduces the possibility of nulls being returned - birthday: null, - number: null, - }) - }) - // this parameter really only impacts SQL queries describe("confirm nullDefaultSupport", () => { let queryParams: Partial diff --git a/packages/server/src/tests/utilities/api/datasource.ts b/packages/server/src/tests/utilities/api/datasource.ts index 67484a688a..9be7be998d 100644 --- a/packages/server/src/tests/utilities/api/datasource.ts +++ b/packages/server/src/tests/utilities/api/datasource.ts @@ -66,16 +66,6 @@ export class DatasourceAPI extends TestAPI { return await this._get(`/api/datasources`, { expectations }) } - query = async ( - query: Omit & Partial>, - expectations?: Expectations - ) => { - return await this._post(`/api/datasources/query`, { - body: query, - expectations, - }) - } - fetchSchema = async ( { datasourceId, From a162d8d9f584ff5d255e2799c8e62bd95f947bd2 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 28 Nov 2024 16:43:21 +0000 Subject: [PATCH 2/8] Linting. --- packages/server/src/api/routes/datasource.ts | 5 +---- .../src/api/routes/tests/queries/generic-sql.spec.ts | 8 +------- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/packages/server/src/api/routes/datasource.ts b/packages/server/src/api/routes/datasource.ts index 22f9a77cc9..9ec792632a 100644 --- a/packages/server/src/api/routes/datasource.ts +++ b/packages/server/src/api/routes/datasource.ts @@ -2,10 +2,7 @@ import Router from "@koa/router" import * as datasourceController from "../controllers/datasource" import authorized from "../../middleware/authorized" import { permissions } from "@budibase/backend-core" -import { - datasourceValidator, - datasourceQueryValidator, -} from "./utils/validators" +import { datasourceValidator } from "./utils/validators" const router: Router = new Router() diff --git a/packages/server/src/api/routes/tests/queries/generic-sql.spec.ts b/packages/server/src/api/routes/tests/queries/generic-sql.spec.ts index 001f4890fd..e7ddc0df22 100644 --- a/packages/server/src/api/routes/tests/queries/generic-sql.spec.ts +++ b/packages/server/src/api/routes/tests/queries/generic-sql.spec.ts @@ -1,10 +1,4 @@ -import { - Datasource, - Operation, - Query, - QueryPreview, - TableSourceType, -} from "@budibase/types" +import { Datasource, Query, QueryPreview } from "@budibase/types" import { DatabaseName, datasourceDescribe, From 69106306c4f50de1aece5fc74383f4b9716d40c6 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 28 Nov 2024 16:43:47 +0000 Subject: [PATCH 3/8] Removing unused validator. --- .../server/src/api/routes/utils/validators.ts | 24 ------------------- 1 file changed, 24 deletions(-) diff --git a/packages/server/src/api/routes/utils/validators.ts b/packages/server/src/api/routes/utils/validators.ts index 3bee4f88ce..30862cfc68 100644 --- a/packages/server/src/api/routes/utils/validators.ts +++ b/packages/server/src/api/routes/utils/validators.ts @@ -231,30 +231,6 @@ export function externalSearchValidator() { ) } -export function datasourceQueryValidator() { - return auth.joiValidator.body( - Joi.object({ - endpoint: Joi.object({ - datasourceId: Joi.string().required(), - operation: Joi.string() - .required() - .valid(...Object.values(DataSourceOperation)), - entityId: Joi.string().required(), - }).required(), - resource: Joi.object({ - fields: Joi.array().items(Joi.string()).optional(), - }).optional(), - body: Joi.object().optional(), - sort: Joi.object().optional(), - filters: filterObject().optional(), - paginate: Joi.object({ - page: Joi.string().alphanum().optional(), - limit: Joi.number().optional(), - }).optional(), - }) - ) -} - export function webhookValidator() { return auth.joiValidator.body( Joi.object({ From 8b653d8dd9184a696633ae4be8844188ff06b451 Mon Sep 17 00:00:00 2001 From: melohagan <101575380+melohagan@users.noreply.github.com> Date: Thu, 28 Nov 2024 16:45:43 +0000 Subject: [PATCH 4/8] Update pro ref (#15088) --- packages/pro | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/pro b/packages/pro index 6f38253253..e2252498dd 160000 --- a/packages/pro +++ b/packages/pro @@ -1 +1 @@ -Subproject commit 6f38253253ee364aea636add990083ca5cda3bde +Subproject commit e2252498ddfade3c2592b1ec78f7bee4e3cf0d2f From f91d442ad46169b2f97856263dfc98f4a591eee3 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 28 Nov 2024 16:48:28 +0000 Subject: [PATCH 5/8] Remove exited containers when running tests. --- .../core/utilities/testContainerUtils.ts | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/packages/backend-core/tests/core/utilities/testContainerUtils.ts b/packages/backend-core/tests/core/utilities/testContainerUtils.ts index 71d7fa32db..727b290183 100644 --- a/packages/backend-core/tests/core/utilities/testContainerUtils.ts +++ b/packages/backend-core/tests/core/utilities/testContainerUtils.ts @@ -25,7 +25,7 @@ function getTestcontainers(): ContainerInfo[] { // We use --format json to make sure the output is nice and machine-readable, // and we use --no-trunc so that the command returns full container IDs so we // can filter on them correctly. - return execSync("docker ps --format json --no-trunc") + return execSync("docker ps --all --format json --no-trunc") .toString() .split("\n") .filter(x => x.length > 0) @@ -37,6 +37,10 @@ function getTestcontainers(): ContainerInfo[] { ) } +function removeContainer(container: ContainerInfo) { + execSync(`docker rm ${container.ID}`) +} + export function getContainerByImage(image: string) { const containers = getTestcontainers().filter(x => x.Image.startsWith(image)) if (containers.length > 1) { @@ -49,6 +53,10 @@ export function getContainerByImage(image: string) { return containers[0] } +function getContainerByName(name: string) { + return getTestcontainers().find(x => x.Names === name) +} + export function getContainerById(id: string) { return getTestcontainers().find(x => x.ID === id) } @@ -116,6 +124,16 @@ export async function startContainer(container: GenericContainer) { key = imageName.split("@")[0] } key = key.replace(/\//g, "-").replace(/:/g, "-") + const name = `${key}_testcontainer` + + // If a container has died it hangs around and future attempts to start a + // container with the same name will fail. What we do here is if we find a + // matching container and it has exited, we remove it before carrying on. This + // removes the need to do this removal manually. + const existingContainer = getContainerByName(name) + if (existingContainer?.State === "exited") { + removeContainer(existingContainer) + } container = container .withReuse() From 13918c5789fa6e1195cda7376e2c6bfda885fe45 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 28 Nov 2024 16:49:35 +0000 Subject: [PATCH 6/8] More linting. --- packages/server/src/api/routes/utils/validators.ts | 1 - packages/server/src/constants/index.ts | 11 ----------- 2 files changed, 12 deletions(-) diff --git a/packages/server/src/api/routes/utils/validators.ts b/packages/server/src/api/routes/utils/validators.ts index 30862cfc68..a8fa12ec3d 100644 --- a/packages/server/src/api/routes/utils/validators.ts +++ b/packages/server/src/api/routes/utils/validators.ts @@ -1,5 +1,4 @@ import { auth, permissions } from "@budibase/backend-core" -import { DataSourceOperation } from "../../../constants" import { AutomationActionStepId, AutomationStep, diff --git a/packages/server/src/constants/index.ts b/packages/server/src/constants/index.ts index bac838b53e..604a81cd9f 100644 --- a/packages/server/src/constants/index.ts +++ b/packages/server/src/constants/index.ts @@ -45,17 +45,6 @@ export enum AuthTypes { EXTERNAL = "external", } -export enum DataSourceOperation { - CREATE = "CREATE", - READ = "READ", - UPDATE = "UPDATE", - DELETE = "DELETE", - BULK_CREATE = "BULK_CREATE", - CREATE_TABLE = "CREATE_TABLE", - UPDATE_TABLE = "UPDATE_TABLE", - DELETE_TABLE = "DELETE_TABLE", -} - export enum DatasourceAuthTypes { GOOGLE = "google", } From 3cf2dbe37ce40c0cab4b9f3ffde523bbd6802db1 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 28 Nov 2024 17:16:23 +0000 Subject: [PATCH 7/8] Make sure we're always using the current docker context. --- globalSetup.ts | 28 +++++++++++++++++++ .../core/utilities/testContainerUtils.ts | 27 ++++++++++++++++++ .../src/api/routes/tests/search.spec.ts | 2 +- 3 files changed, 56 insertions(+), 1 deletion(-) diff --git a/globalSetup.ts b/globalSetup.ts index 07a0cec5e2..9ebda5626d 100644 --- a/globalSetup.ts +++ b/globalSetup.ts @@ -6,6 +6,26 @@ import { import { ContainerInfo } from "dockerode" import path from "path" import lockfile from "proper-lockfile" +import { execSync } from "child_process" + +interface DockerContext { + Name: string + Description: string + DockerEndpoint: string + ContextType: string + Error: string +} + +function getCurrentDockerContext(): DockerContext { + const out = execSync("docker context ls --format json") + for (const line of out.toString().split("\n")) { + const parsed = JSON.parse(line) + if (parsed.Current) { + return parsed as DockerContext + } + } + throw new Error("No current Docker context") +} async function getBudibaseContainers() { const client = await getContainerRuntimeClient() @@ -27,6 +47,14 @@ async function killContainers(containers: ContainerInfo[]) { } export default async function setup() { + // For whatever reason, testcontainers doesn't always use the correct current + // docker context. This bit of code forces the issue by finding the current + // context and setting it as the DOCKER_HOST environment + if (!process.env.DOCKER_HOST) { + const dockerContext = getCurrentDockerContext() + process.env.DOCKER_HOST = dockerContext.DockerEndpoint + } + const lockPath = path.resolve(__dirname, "globalSetup.ts") // If you run multiple tests at the same time, it's possible for the CouchDB // shared container to get started multiple times despite having an diff --git a/packages/backend-core/tests/core/utilities/testContainerUtils.ts b/packages/backend-core/tests/core/utilities/testContainerUtils.ts index 727b290183..339656cf09 100644 --- a/packages/backend-core/tests/core/utilities/testContainerUtils.ts +++ b/packages/backend-core/tests/core/utilities/testContainerUtils.ts @@ -78,7 +78,34 @@ export function getExposedV4Port(container: ContainerInfo, port: number) { return getExposedV4Ports(container).find(x => x.container === port)?.host } +interface DockerContext { + Name: string + Description: string + DockerEndpoint: string + ContextType: string + Error: string +} + +function getCurrentDockerContext(): DockerContext { + const out = execSync("docker context ls --format json") + for (const line of out.toString().split("\n")) { + const parsed = JSON.parse(line) + if (parsed.Current) { + return parsed as DockerContext + } + } + throw new Error("No current Docker context") +} + export function setupEnv(...envs: any[]) { + // For whatever reason, testcontainers doesn't always use the correct current + // docker context. This bit of code forces the issue by finding the current + // context and setting it as the DOCKER_HOST environment + if (!process.env.DOCKER_HOST) { + const dockerContext = getCurrentDockerContext() + process.env.DOCKER_HOST = dockerContext.DockerEndpoint + } + // We start couchdb in globalSetup.ts, in the root of the monorepo, so it // should be relatively safe to look for it by its image name. const couch = getContainerByImage("budibase/couchdb") diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 5384444067..6a56ec37c6 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -3329,7 +3329,7 @@ if (descriptions.length) { }) isSql && - describe("primaryDisplay", () => { + describe.only("primaryDisplay", () => { beforeAll(async () => { let toRelateTableId = await createTable({ name: { From 3bff9f2e551005a98a6448d4faeacde1509fa425 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 28 Nov 2024 17:23:16 +0000 Subject: [PATCH 8/8] Unfocus test. --- packages/server/src/api/routes/tests/search.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/server/src/api/routes/tests/search.spec.ts b/packages/server/src/api/routes/tests/search.spec.ts index 6a56ec37c6..5384444067 100644 --- a/packages/server/src/api/routes/tests/search.spec.ts +++ b/packages/server/src/api/routes/tests/search.spec.ts @@ -3329,7 +3329,7 @@ if (descriptions.length) { }) isSql && - describe.only("primaryDisplay", () => { + describe("primaryDisplay", () => { beforeAll(async () => { let toRelateTableId = await createTable({ name: {