From 40b4943766f040edce2e87763a72d6be9cf34b05 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Thu, 20 Jul 2023 13:07:19 +0100 Subject: [PATCH] Fix for dynamic variables being duplicated when creating new REST queries - also moved some stuff to backend SDK to make a bit re-usable, allowing backend to detect duplicate dynamic variables and error. --- .../integration/RestQueryViewer.svelte | 26 ++++--- .../Variables/ViewDynamicVariables.svelte | 2 +- .../server/src/api/controllers/datasource.ts | 74 ++++++------------ .../server/src/api/controllers/query/index.ts | 22 +----- .../src/sdk/app/datasources/datasources.ts | 75 ++++++++++++++++++- .../server/src/sdk/app/queries/queries.ts | 44 +++++++++++ .../types/src/documents/app/datasource.ts | 4 +- 7 files changed, 161 insertions(+), 86 deletions(-) diff --git a/packages/builder/src/components/integration/RestQueryViewer.svelte b/packages/builder/src/components/integration/RestQueryViewer.svelte index 88c10422de..254f65fcaf 100644 --- a/packages/builder/src/components/integration/RestQueryViewer.svelte +++ b/packages/builder/src/components/integration/RestQueryViewer.svelte @@ -419,16 +419,22 @@ if (query && !query.fields.pagination) { query.fields.pagination = {} } - dynamicVariables = getDynamicVariables( - datasource, - query._id, - (variable, queryId) => variable.queryId === queryId - ) - globalDynamicBindings = getDynamicVariables( - datasource, - query._id, - (variable, queryId) => variable.queryId !== queryId - ) + // if query doesn't have ID then its new - don't try to copy existing dynamic variables + if (!queryId) { + dynamicVariables = [] + globalDynamicBindings = getDynamicVariables(datasource) + } else { + dynamicVariables = getDynamicVariables( + datasource, + query._id, + (variable, queryId) => variable.queryId === queryId + ) + globalDynamicBindings = getDynamicVariables( + datasource, + query._id, + (variable, queryId) => variable.queryId !== queryId + ) + } prettifyQueryRequestBody( query, diff --git a/packages/builder/src/pages/builder/app/[application]/data/datasource/[datasourceId]/_components/panels/Variables/ViewDynamicVariables.svelte b/packages/builder/src/pages/builder/app/[application]/data/datasource/[datasourceId]/_components/panels/Variables/ViewDynamicVariables.svelte index c5e3666cf8..dd5668d603 100644 --- a/packages/builder/src/pages/builder/app/[application]/data/datasource/[datasourceId]/_components/panels/Variables/ViewDynamicVariables.svelte +++ b/packages/builder/src/pages/builder/app/[application]/data/datasource/[datasourceId]/_components/panels/Variables/ViewDynamicVariables.svelte @@ -18,7 +18,7 @@ const onClick = dynamicVariable => { const queryId = dynamicVariable.queryId queries.select({ _id: queryId }) - $goto(`./${queryId}`) + $goto(`../../query/${queryId}`) } /** diff --git a/packages/server/src/api/controllers/datasource.ts b/packages/server/src/api/controllers/datasource.ts index 069842a020..bbea14a0c4 100644 --- a/packages/server/src/api/controllers/datasource.ts +++ b/packages/server/src/api/controllers/datasource.ts @@ -1,9 +1,7 @@ import { - generateDatasourceID, - getDatasourceParams, - getQueryParams, DocumentType, - BudibaseInternalDB, + generateDatasourceID, + getQueryParams, getTableParams, } from "../../db/utils" import { destroy as tableDestroy } from "./table/internal" @@ -11,25 +9,26 @@ import { BuildSchemaErrors, InvalidColumns } from "../../constants" import { getIntegration } from "../../integrations" import { getDatasourceAndQuery } from "./row/utils" import { invalidateDynamicVariables } from "../../threads/utils" -import { db as dbCore, context, events } from "@budibase/backend-core" +import { context, db as dbCore, events } from "@budibase/backend-core" import { - UserCtx, - Datasource, - Row, - CreateDatasourceResponse, - UpdateDatasourceResponse, CreateDatasourceRequest, - VerifyDatasourceRequest, - VerifyDatasourceResponse, + CreateDatasourceResponse, + Datasource, + DatasourcePlus, FetchDatasourceInfoRequest, FetchDatasourceInfoResponse, IntegrationBase, - DatasourcePlus, + RestConfig, SourceName, + UpdateDatasourceResponse, + UserCtx, + VerifyDatasourceRequest, + VerifyDatasourceResponse, } from "@budibase/types" import sdk from "../../sdk" import { builderSocket } from "../../websockets" import { setupCreationAuth as googleSetupCreationAuth } from "../../integrations/googlesheets" +import { areRESTVariablesValid } from "../../sdk/app/datasources/datasources" function getErrorTables(errors: any, errorType: string) { return Object.entries(errors) @@ -120,46 +119,7 @@ async function buildFilteredSchema(datasource: Datasource, filter?: string[]) { } export async function fetch(ctx: UserCtx) { - // Get internal tables - const db = context.getAppDB() - const internalTables = await db.allDocs( - getTableParams(null, { - include_docs: true, - }) - ) - - const internal = internalTables.rows.reduce((acc: any, row: Row) => { - const sourceId = row.doc.sourceId || "bb_internal" - acc[sourceId] = acc[sourceId] || [] - acc[sourceId].push(row.doc) - return acc - }, {}) - - const bbInternalDb = { - ...BudibaseInternalDB, - } - - // Get external datasources - const datasources = ( - await db.allDocs( - getDatasourceParams(null, { - include_docs: true, - }) - ) - ).rows.map(row => row.doc) - - const allDatasources: Datasource[] = await sdk.datasources.removeSecrets([ - bbInternalDb, - ...datasources, - ]) - - for (let datasource of allDatasources) { - if (datasource.type === dbCore.BUDIBASE_DATASOURCE_TYPE) { - datasource.entities = internal[datasource._id!] - } - } - - ctx.body = [bbInternalDb, ...datasources] + ctx.body = await sdk.datasources.fetch() } export async function verify( @@ -291,6 +251,14 @@ export async function update(ctx: UserCtx) { datasource.config!.auth = auth } + // check all variables are unique + if ( + datasource.source === SourceName.REST && + !sdk.datasources.areRESTVariablesValid(datasource) + ) { + ctx.throw(400, "Duplicate dynamic/static variable names are invalid.") + } + const response = await db.put( sdk.tables.populateExternalTableSchemas(datasource) ) diff --git a/packages/server/src/api/controllers/query/index.ts b/packages/server/src/api/controllers/query/index.ts index 356a2eb5c0..c32e17bd92 100644 --- a/packages/server/src/api/controllers/query/index.ts +++ b/packages/server/src/api/controllers/query/index.ts @@ -1,4 +1,4 @@ -import { generateQueryID, getQueryParams, isProdAppID } from "../../../db/utils" +import { generateQueryID } from "../../../db/utils" import { BaseQueryVerbs, FieldTypes } from "../../../constants" import { Thread, ThreadType } from "../../../threads" import { save as saveDatasource } from "../datasource" @@ -27,15 +27,7 @@ function enrichQueries(input: any) { } export async function fetch(ctx: any) { - const db = context.getAppDB() - - const body = await db.allDocs( - getQueryParams(null, { - include_docs: true, - }) - ) - - ctx.body = enrichQueries(body.rows.map((row: any) => row.doc)) + ctx.body = await sdk.queries.fetch() } const _import = async (ctx: any) => { @@ -102,14 +94,8 @@ export async function save(ctx: any) { } export async function find(ctx: any) { - const db = context.getAppDB() - const query = enrichQueries(await db.get(ctx.params.queryId)) - // remove properties that could be dangerous in real app - if (isProdAppID(ctx.appId)) { - delete query.fields - delete query.parameters - } - ctx.body = query + const queryId = ctx.params.queryId + ctx.body = await sdk.queries.find(queryId) } //Required to discern between OIDC OAuth config entries diff --git a/packages/server/src/sdk/app/datasources/datasources.ts b/packages/server/src/sdk/app/datasources/datasources.ts index 430f90c159..6f370d1c98 100644 --- a/packages/server/src/sdk/app/datasources/datasources.ts +++ b/packages/server/src/sdk/app/datasources/datasources.ts @@ -1,4 +1,4 @@ -import { context } from "@budibase/backend-core" +import { context, db as dbCore } from "@budibase/backend-core" import { findHBSBlocks, processObjectSync } from "@budibase/string-templates" import { Datasource, @@ -8,15 +8,88 @@ import { RestAuthConfig, RestAuthType, RestBasicAuthConfig, + Row, + RestConfig, SourceName, } from "@budibase/types" import { cloneDeep } from "lodash/fp" import { getEnvironmentVariables } from "../../utils" import { getDefinitions, getDefinition } from "../../../integrations" import _ from "lodash" +import { + BudibaseInternalDB, + getDatasourceParams, + getTableParams, +} from "../../../db/utils" +import sdk from "../../index" const ENV_VAR_PREFIX = "env." +export async function fetch() { + // Get internal tables + const db = context.getAppDB() + const internalTables = await db.allDocs( + getTableParams(null, { + include_docs: true, + }) + ) + + const internal = internalTables.rows.reduce((acc: any, row: Row) => { + const sourceId = row.doc.sourceId || "bb_internal" + acc[sourceId] = acc[sourceId] || [] + acc[sourceId].push(row.doc) + return acc + }, {}) + + const bbInternalDb = { + ...BudibaseInternalDB, + } + + // Get external datasources + const datasources = ( + await db.allDocs( + getDatasourceParams(null, { + include_docs: true, + }) + ) + ).rows.map(row => row.doc) + + const allDatasources: Datasource[] = await sdk.datasources.removeSecrets([ + bbInternalDb, + ...datasources, + ]) + + for (let datasource of allDatasources) { + if (datasource.type === dbCore.BUDIBASE_DATASOURCE_TYPE) { + datasource.entities = internal[datasource._id!] + } + } + + return [bbInternalDb, ...datasources] +} + +export function areRESTVariablesValid(datasource: Datasource) { + const restConfig = datasource.config as RestConfig + const varNames: string[] = [] + if (restConfig.dynamicVariables) { + for (let variable of restConfig.dynamicVariables) { + if (varNames.includes(variable.name)) { + return false + } + varNames.push(variable.name) + } + } + if (restConfig.staticVariables) { + for (let name of Object.keys(restConfig.staticVariables)) { + if (varNames.includes(name)) { + return false + } + varNames.push(name) + } + } + return true +} + export function checkDatasourceTypes(schema: Integration, config: any) { for (let key of Object.keys(config)) { if (!schema.datasource[key]) { diff --git a/packages/server/src/sdk/app/queries/queries.ts b/packages/server/src/sdk/app/queries/queries.ts index ca74eb44b5..408e393714 100644 --- a/packages/server/src/sdk/app/queries/queries.ts +++ b/packages/server/src/sdk/app/queries/queries.ts @@ -1,5 +1,49 @@ import { getEnvironmentVariables } from "../../utils" import { processStringSync } from "@budibase/string-templates" +import { context } from "@budibase/backend-core" +import { getQueryParams, isProdAppID } from "../../../db/utils" +import { BaseQueryVerbs } from "../../../constants" + +// simple function to append "readable" to all read queries +function enrichQueries(input: any) { + const wasArray = Array.isArray(input) + const queries = wasArray ? input : [input] + for (let query of queries) { + if (query.queryVerb === BaseQueryVerbs.READ) { + query.readable = true + } + } + return wasArray ? queries : queries[0] +} + +export async function find(queryId: string) { + const db = context.getAppDB() + const appId = context.getAppId() + const query = enrichQueries(await db.get(queryId)) + // remove properties that could be dangerous in real app + if (isProdAppID(appId)) { + delete query.fields + delete query.parameters + } + return query +} + +export async function fetch(opts: { enrich: boolean } = { enrich: true }) { + const db = context.getAppDB() + + const body = await db.allDocs( + getQueryParams(null, { + include_docs: true, + }) + ) + + const queries = body.rows.map((row: any) => row.doc) + if (opts.enrich) { + return enrichQueries(queries) + } else { + return queries + } +} export async function enrichContext( fields: Record, diff --git a/packages/types/src/documents/app/datasource.ts b/packages/types/src/documents/app/datasource.ts index 8dfdfe6d0f..855006ea4c 100644 --- a/packages/types/src/documents/app/datasource.ts +++ b/packages/types/src/documents/app/datasource.ts @@ -7,9 +7,7 @@ export interface Datasource extends Document { name?: string source: SourceName // the config is defined by the schema - config?: { - [key: string]: string | number | boolean | any[] - } + config?: Record plus?: boolean entities?: { [key: string]: Table