From 64af5493415144b14d0accf58d818cd58d9e5481 Mon Sep 17 00:00:00 2001 From: Rory Powell Date: Fri, 1 Sep 2023 15:12:23 +0100 Subject: [PATCH] Revert "Merge pull request #11489 from Budibase/fix/automation-disable" This reverts commit e6319cce675e7674c11df5dadbdc769d0f49bc0e, reversing changes made to 7c12280fe398bce4ef0b3e2c22ef852d344c0f69. --- .../backend-core/src/db/couch/DatabaseImpl.ts | 14 ---- .../server/src/api/controllers/application.ts | 4 +- .../server/src/api/controllers/automation.ts | 29 ++++---- packages/server/src/automations/triggers.ts | 4 +- packages/server/src/automations/utils.ts | 67 +++++++------------ .../src/sdk/app/automations/automations.ts | 38 ----------- .../server/src/sdk/app/automations/index.ts | 2 - packages/server/src/threads/automation.ts | 42 +++--------- .../types/src/documents/app/automation.ts | 5 -- packages/types/src/sdk/db.ts | 6 -- 10 files changed, 55 insertions(+), 156 deletions(-) delete mode 100644 packages/server/src/sdk/app/automations/automations.ts diff --git a/packages/backend-core/src/db/couch/DatabaseImpl.ts b/packages/backend-core/src/db/couch/DatabaseImpl.ts index 89f76769b3..29ca4123f5 100644 --- a/packages/backend-core/src/db/couch/DatabaseImpl.ts +++ b/packages/backend-core/src/db/couch/DatabaseImpl.ts @@ -8,7 +8,6 @@ import { DatabasePutOpts, DatabaseCreateIndexOpts, DatabaseDeleteIndexOpts, - DocExistsResponse, Document, isDocument, } from "@budibase/types" @@ -121,19 +120,6 @@ export class DatabaseImpl implements Database { return this.updateOutput(() => db.get(id)) } - async docExists(docId: string): Promise { - const db = await this.checkSetup() - let _rev, exists - try { - const { etag } = await db.head(docId) - _rev = etag - exists = true - } catch (err) { - exists = false - } - return { _rev, exists } - } - async remove(idOrDoc: string | Document, rev?: string) { const db = await this.checkSetup() let _id: string diff --git a/packages/server/src/api/controllers/application.ts b/packages/server/src/api/controllers/application.ts index f8b6bd6d16..c5556479fe 100644 --- a/packages/server/src/api/controllers/application.ts +++ b/packages/server/src/api/controllers/application.ts @@ -39,8 +39,9 @@ import { } from "../../db/defaultData/datasource_bb_default" import { removeAppFromUserRoles } from "../../utilities/workerRequests" import { stringToReadStream } from "../../utilities" -import { doesUserHaveLock } from "../../utilities/redis" +import { doesUserHaveLock, getLocksById } from "../../utilities/redis" import { cleanupAutomations } from "../../automations/utils" +import { checkAppMetadata } from "../../automations/logging" import { getUniqueRows } from "../../utilities/usageQuota/rows" import { groups, licensing, quotas } from "@budibase/pro" import { @@ -50,6 +51,7 @@ import { PlanType, Screen, UserCtx, + ContextUser, } from "@budibase/types" import { BASE_LAYOUT_PROP_IDS } from "../../constants/layouts" import sdk from "../../sdk" diff --git a/packages/server/src/api/controllers/automation.ts b/packages/server/src/api/controllers/automation.ts index c861782a02..a4697f99f2 100644 --- a/packages/server/src/api/controllers/automation.ts +++ b/packages/server/src/api/controllers/automation.ts @@ -20,7 +20,7 @@ import { Automation, AutomationActionStepId, AutomationResults, - Ctx, + BBContext, } from "@budibase/types" import { getActionDefinitions as actionDefs } from "../../automations/actions" import sdk from "../../sdk" @@ -73,7 +73,7 @@ function cleanAutomationInputs(automation: Automation) { return automation } -export async function create(ctx: Ctx) { +export async function create(ctx: BBContext) { const db = context.getAppDB() let automation = ctx.request.body automation.appId = ctx.appId @@ -142,7 +142,7 @@ export async function handleStepEvents( } } -export async function update(ctx: Ctx) { +export async function update(ctx: BBContext) { const db = context.getAppDB() let automation = ctx.request.body automation.appId = ctx.appId @@ -193,7 +193,7 @@ export async function update(ctx: Ctx) { builderSocket?.emitAutomationUpdate(ctx, automation) } -export async function fetch(ctx: Ctx) { +export async function fetch(ctx: BBContext) { const db = context.getAppDB() const response = await db.allDocs( getAutomationParams(null, { @@ -203,11 +203,12 @@ export async function fetch(ctx: Ctx) { ctx.body = response.rows.map(row => row.doc) } -export async function find(ctx: Ctx) { - ctx.body = await sdk.automations.get(ctx.params.id) +export async function find(ctx: BBContext) { + const db = context.getAppDB() + ctx.body = await db.get(ctx.params.id) } -export async function destroy(ctx: Ctx) { +export async function destroy(ctx: BBContext) { const db = context.getAppDB() const automationId = ctx.params.id const oldAutomation = await db.get(automationId) @@ -221,11 +222,11 @@ export async function destroy(ctx: Ctx) { builderSocket?.emitAutomationDeletion(ctx, automationId) } -export async function logSearch(ctx: Ctx) { +export async function logSearch(ctx: BBContext) { ctx.body = await automations.logs.logSearch(ctx.request.body) } -export async function clearLogError(ctx: Ctx) { +export async function clearLogError(ctx: BBContext) { const { automationId, appId } = ctx.request.body await context.doInAppContext(appId, async () => { const db = context.getProdAppDB() @@ -244,15 +245,15 @@ export async function clearLogError(ctx: Ctx) { }) } -export async function getActionList(ctx: Ctx) { +export async function getActionList(ctx: BBContext) { ctx.body = await getActionDefinitions() } -export async function getTriggerList(ctx: Ctx) { +export async function getTriggerList(ctx: BBContext) { ctx.body = getTriggerDefinitions() } -export async function getDefinitionList(ctx: Ctx) { +export async function getDefinitionList(ctx: BBContext) { ctx.body = { trigger: getTriggerDefinitions(), action: await getActionDefinitions(), @@ -265,7 +266,7 @@ export async function getDefinitionList(ctx: Ctx) { * * *********************/ -export async function trigger(ctx: Ctx) { +export async function trigger(ctx: BBContext) { const db = context.getAppDB() let automation = await db.get(ctx.params.id) @@ -310,7 +311,7 @@ function prepareTestInput(input: any) { return input } -export async function test(ctx: Ctx) { +export async function test(ctx: BBContext) { const db = context.getAppDB() let automation = await db.get(ctx.params.id) await setTestFlag(automation._id!) diff --git a/packages/server/src/automations/triggers.ts b/packages/server/src/automations/triggers.ts index 9cc8c6b077..922bc10343 100644 --- a/packages/server/src/automations/triggers.ts +++ b/packages/server/src/automations/triggers.ts @@ -6,11 +6,11 @@ import { isDevAppID } from "../db/utils" // need this to call directly, so we can get a response import { automationQueue } from "./bullboard" import { checkTestFlag } from "../utilities/redis" +import * as utils from "./utils" import env from "../environment" import { context, db as dbCore } from "@budibase/backend-core" import { Automation, Row, AutomationData, AutomationJob } from "@budibase/types" import { executeSynchronously } from "../threads/automation" -import sdk from "../sdk" export const TRIGGER_DEFINITIONS = definitions const JOB_OPTS = { @@ -142,7 +142,7 @@ export async function rebootTrigger() { let automations = await getAllAutomations() let rebootEvents = [] for (let automation of automations) { - if (sdk.automations.isReboot(automation)) { + if (utils.isRebootTrigger(automation)) { const job = { automation, event: { diff --git a/packages/server/src/automations/utils.ts b/packages/server/src/automations/utils.ts index 7b0ab4f2fd..194eff5b32 100644 --- a/packages/server/src/automations/utils.ts +++ b/packages/server/src/automations/utils.ts @@ -16,14 +16,13 @@ import { } from "@budibase/types" import sdk from "../sdk" +const REBOOT_CRON = "@reboot" const WH_STEP_ID = definitions.WEBHOOK.stepId +const CRON_STEP_ID = definitions.CRON.stepId const Runner = new Thread(ThreadType.AUTOMATION) -function loggingArgs( - job: AutomationJob, - timing?: { start: number; complete?: boolean } -) { - const logs: any[] = [ +function loggingArgs(job: AutomationJob) { + return [ { _logKey: "automation", trigger: job.data.automation.definition.trigger.event, @@ -33,53 +32,24 @@ function loggingArgs( jobId: job.id, }, ] - if (timing?.start) { - logs.push({ - _logKey: "startTime", - start: timing.start, - }) - } - if (timing?.start && timing?.complete) { - const end = new Date().getTime() - const duration = end - timing.start - logs.push({ - _logKey: "endTime", - end, - }) - logs.push({ - _logKey: "duration", - duration, - }) - } - return logs } export async function processEvent(job: AutomationJob) { const appId = job.data.event.appId! const automationId = job.data.automation._id! - const start = new Date().getTime() const task = async () => { try { // need to actually await these so that an error can be captured properly - console.log("automation running", ...loggingArgs(job, { start })) + console.log("automation running", ...loggingArgs(job)) const runFn = () => Runner.run(job) const result = await quotas.addAutomation(runFn, { automationId, }) - const end = new Date().getTime() - const duration = end - start - console.log( - "automation completed", - ...loggingArgs(job, { start, complete: true }) - ) + console.log("automation completed", ...loggingArgs(job)) return result } catch (err) { - console.error( - `automation was unable to run`, - err, - ...loggingArgs(job, { start, complete: true }) - ) + console.error(`automation was unable to run`, err, ...loggingArgs(job)) return { err } } } @@ -158,6 +128,19 @@ export async function clearMetadata() { await db.bulkDocs(automationMetadata) } +export function isCronTrigger(auto: Automation) { + return ( + auto && + auto.definition.trigger && + auto.definition.trigger.stepId === CRON_STEP_ID + ) +} + +export function isRebootTrigger(auto: Automation) { + const trigger = auto ? auto.definition.trigger : null + return isCronTrigger(auto) && trigger?.inputs.cron === REBOOT_CRON +} + /** * This function handles checking of any cron jobs that need to be enabled/updated. * @param {string} appId The ID of the app in which we are checking for webhooks @@ -165,13 +148,13 @@ export async function clearMetadata() { */ export async function enableCronTrigger(appId: any, automation: Automation) { const trigger = automation ? automation.definition.trigger : null - const validCron = sdk.automations.isCron(automation) && trigger?.inputs.cron - const needsCreated = - !sdk.automations.isReboot(automation) && - !sdk.automations.disabled(automation) // need to create cron job - if (validCron && needsCreated) { + if ( + isCronTrigger(automation) && + !isRebootTrigger(automation) && + trigger?.inputs.cron + ) { // make a job id rather than letting Bull decide, makes it easier to handle on way out const jobId = `${appId}_cron_${newid()}` const job: any = await automationQueue.add( diff --git a/packages/server/src/sdk/app/automations/automations.ts b/packages/server/src/sdk/app/automations/automations.ts deleted file mode 100644 index 80f84c90e2..0000000000 --- a/packages/server/src/sdk/app/automations/automations.ts +++ /dev/null @@ -1,38 +0,0 @@ -import { context } from "@budibase/backend-core" -import { Automation, AutomationState, DocumentType } from "@budibase/types" -import { definitions } from "../../../automations/triggerInfo" - -const REBOOT_CRON = "@reboot" - -export async function exists(automationId: string) { - if (!automationId?.startsWith(DocumentType.AUTOMATION)) { - throw new Error("Invalid automation ID.") - } - const db = context.getAppDB() - return db.docExists(automationId) -} - -export async function get(automationId: string) { - const db = context.getAppDB() - return (await db.get(automationId)) as Automation -} - -export function disabled(automation: Automation) { - return automation.state === AutomationState.DISABLED || !hasSteps(automation) -} - -export function isCron(automation: Automation) { - return ( - automation?.definition.trigger && - automation?.definition.trigger.stepId === definitions.CRON.stepId - ) -} - -export function isReboot(automation: Automation) { - const trigger = automation?.definition.trigger - return isCron(automation) && trigger?.inputs.cron === REBOOT_CRON -} - -export function hasSteps(automation: Automation) { - return automation?.definition?.steps?.length > 0 -} diff --git a/packages/server/src/sdk/app/automations/index.ts b/packages/server/src/sdk/app/automations/index.ts index 540d5545fc..16530cf085 100644 --- a/packages/server/src/sdk/app/automations/index.ts +++ b/packages/server/src/sdk/app/automations/index.ts @@ -1,9 +1,7 @@ import * as webhook from "./webhook" import * as utils from "./utils" -import * as automations from "./automations" export default { webhook, utils, - ...automations, } diff --git a/packages/server/src/threads/automation.ts b/packages/server/src/threads/automation.ts index 8bf0b506fe..bc0629a939 100644 --- a/packages/server/src/threads/automation.ts +++ b/packages/server/src/threads/automation.ts @@ -2,9 +2,9 @@ import { default as threadUtils } from "./utils" import { Job } from "bull" threadUtils.threadSetup() import { + isRecurring, disableCronById, isErrorInOutput, - isRecurring, } from "../automations/utils" import * as actions from "../automations/actions" import * as automationUtils from "../automations/automationUtils" @@ -15,17 +15,17 @@ import { AutomationErrors, MAX_AUTOMATION_RECURRING_ERRORS } from "../constants" import { storeLog } from "../automations/logging" import { Automation, - AutomationData, - AutomationJob, - AutomationMetadata, - AutomationStatus, AutomationStep, + AutomationStatus, + AutomationMetadata, + AutomationJob, + AutomationData, } from "@budibase/types" import { - AutomationContext, - LoopInput, LoopStep, + LoopInput, TriggerOutput, + AutomationContext, } from "../definitions/automations" import { WorkerCallback } from "./definitions" import { context, logging } from "@budibase/backend-core" @@ -34,8 +34,6 @@ import { cloneDeep } from "lodash/fp" import { performance } from "perf_hooks" import * as sdkUtils from "../sdk/utils" import env from "../environment" -import sdk from "../sdk" - const FILTER_STEP_ID = actions.BUILTIN_ACTION_DEFINITIONS.FILTER.stepId const LOOP_STEP_ID = actions.BUILTIN_ACTION_DEFINITIONS.LOOP.stepId const CRON_STEP_ID = triggerDefs.CRON.stepId @@ -516,8 +514,7 @@ class Orchestrator { export function execute(job: Job, callback: WorkerCallback) { const appId = job.data.event.appId - const automation = job.data.automation - const automationId = automation._id + const automationId = job.data.automation._id if (!appId) { throw new Error("Unable to execute, event doesn't contain app ID.") } @@ -528,30 +525,10 @@ export function execute(job: Job, callback: WorkerCallback) { appId, automationId, task: async () => { - let automation = job.data.automation, - isCron = sdk.automations.isCron(job.data.automation), - notFound = false - try { - automation = await sdk.automations.get(automationId) - } catch (err: any) { - // automation no longer exists - notFound = err - } - const disabled = sdk.automations.disabled(automation) - const stopAutomation = disabled || notFound const envVars = await sdkUtils.getEnvironmentVariables() // put into automation thread for whole context await context.doInEnvironmentContext(envVars, async () => { const automationOrchestrator = new Orchestrator(job) - // hard stop on automations - if (isCron && stopAutomation) { - await automationOrchestrator.stopCron( - disabled ? "disabled" : "not_found" - ) - } - if (stopAutomation) { - return - } try { const response = await automationOrchestrator.execute() callback(null, response) @@ -580,10 +557,11 @@ export function executeSynchronously(job: Job) { // put into automation thread for whole context return context.doInEnvironmentContext(envVars, async () => { const automationOrchestrator = new Orchestrator(job) - return await Promise.race([ + const response = await Promise.race([ automationOrchestrator.execute(), timeoutPromise, ]) + return response }) }) } diff --git a/packages/types/src/documents/app/automation.ts b/packages/types/src/documents/app/automation.ts index 05ac76d5e9..d1dbbec21b 100644 --- a/packages/types/src/documents/app/automation.ts +++ b/packages/types/src/documents/app/automation.ts @@ -100,10 +100,6 @@ export const AutomationStepIdArray = [ ...Object.values(AutomationTriggerStepId), ] -export enum AutomationState { - DISABLED = "disabled", -} - export interface Automation extends Document { definition: { steps: AutomationStep[] @@ -116,7 +112,6 @@ export interface Automation extends Document { name: string internal?: boolean type?: string - state?: AutomationState } interface BaseIOStructure { diff --git a/packages/types/src/sdk/db.ts b/packages/types/src/sdk/db.ts index 9ac8ff26d4..e07cc82fd2 100644 --- a/packages/types/src/sdk/db.ts +++ b/packages/types/src/sdk/db.ts @@ -40,11 +40,6 @@ export type DatabasePutOpts = { force?: boolean } -export type DocExistsResponse = { - _rev?: string - exists: boolean -} - export type DatabaseCreateIndexOpts = { index: { fields: string[] @@ -95,7 +90,6 @@ export interface Database { exists(): Promise checkSetup(): Promise> get(id?: string): Promise - docExists(id: string): Promise remove( id: string | Document, rev?: string