Merge pull request #11297 from Budibase/fix/budi-7304

Fix duplicate dynamic variables when creating new REST queries

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,

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}`)
   }
 
   /**

packages/server/src/api/controllers/datasource.ts

@@ -1,35 +1,34 @@
 import {
+  DocumentType,
   generateDatasourceID,
-  getDatasourceParams,
   getQueryParams,
-  DocumentType,
-  BudibaseInternalDB,
   getTableParams,
 } from "../../db/utils"
 import { destroy as tableDestroy } from "./table/internal"
 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<any, UpdateDatasourceResponse>) {
     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)
   )

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

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]) {

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<string, any>,

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<string, any>
   plus?: boolean
   entities?: {
     [key: string]: Table