From bf431f81c42f3e424f8bf09ea04d05c51ca82756 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= <freben@gmail.com>
Date: Thu, 14 Sep 2023 09:23:19 +0200
Subject: [PATCH] fixes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Fredrik Adelöw <freben@gmail.com>
---
 .changeset/dry-yaks-type.md                   |  5 ++-
 .changeset/real-turkeys-report.md             |  5 ---
 .../src/tasks/TaskWorker.test.ts              | 11 ++++--
 .../backend-tasks/src/tasks/TaskWorker.ts     | 39 ++++++++++++-------
 4 files changed, 36 insertions(+), 24 deletions(-)
 delete mode 100644 .changeset/real-turkeys-report.md

diff --git a/.changeset/dry-yaks-type.md b/.changeset/dry-yaks-type.md
index 33d053a154..3e9cdef474 100644
--- a/.changeset/dry-yaks-type.md
+++ b/.changeset/dry-yaks-type.md
@@ -2,4 +2,7 @@
 '@backstage/backend-tasks': patch
 ---
 
-Fixed bug in backend TaskWorker, 'next_run_start_at' will be always the least between schedule changes.
+When starting a task that existed before, with a faster schedule than it
+previously had, the task will now correctly obey the faster schedule
+immediately. Before this fix, the new schedule was only obeyed after the next
+pending (according to the old schedule) run had completed.
diff --git a/.changeset/real-turkeys-report.md b/.changeset/real-turkeys-report.md
deleted file mode 100644
index 7e13f3a937..0000000000
--- a/.changeset/real-turkeys-report.md
+++ /dev/null
@@ -1,5 +0,0 @@
----
-'@backstage/backend-tasks': patch
----
-
-Fixed bug in backend TaskWorker, adding `next_run_start_at` as a merge conflict when scheduler changes
diff --git a/packages/backend-tasks/src/tasks/TaskWorker.test.ts b/packages/backend-tasks/src/tasks/TaskWorker.test.ts
index 38bb2f7e99..71d02274e2 100644
--- a/packages/backend-tasks/src/tasks/TaskWorker.test.ts
+++ b/packages/backend-tasks/src/tasks/TaskWorker.test.ts
@@ -330,6 +330,8 @@ describe('TaskWorker', () => {
       const before = fn1.mock.calls.length;
       await promise2;
       expect(fn1.mock.calls.length).toBeGreaterThan(before);
+
+      await knex.destroy();
     },
   );
 
@@ -351,6 +353,7 @@ describe('TaskWorker', () => {
 
       const worker = new TaskWorker('task99', fn, knex, logger);
       await worker.persistTask(settings);
+      const row1 = (await knex<DbTasksRow>(DB_TASKS_TABLE))[0];
 
       const settings2 = {
         ...settings,
@@ -358,15 +361,17 @@ describe('TaskWorker', () => {
         initialDelayDuration: 'PT1M',
       };
       await worker.persistTask(settings2);
-
       const row2 = (await knex<DbTasksRow>(DB_TASKS_TABLE))[0];
 
+      expect(row2.next_run_start_at).not.toStrictEqual(row1.next_run_start_at);
+
       const settings3 = { ...settings };
       await worker.persistTask(settings3);
-
       const row3 = (await knex<DbTasksRow>(DB_TASKS_TABLE))[0];
 
-      expect(row3.next_run_start_at).toBe(row2.next_run_start_at);
+      expect(row3.next_run_start_at).toStrictEqual(row2.next_run_start_at);
+
+      await knex.destroy();
     },
   );
 });
diff --git a/packages/backend-tasks/src/tasks/TaskWorker.ts b/packages/backend-tasks/src/tasks/TaskWorker.ts
index 6ede00c590..c1007b4964 100644
--- a/packages/backend-tasks/src/tasks/TaskWorker.ts
+++ b/packages/backend-tasks/src/tasks/TaskWorker.ts
@@ -190,29 +190,38 @@ export class TaskWorker {
 
     this.logger.debug(`task: ${this.taskId} configured to run at: ${startAt}`);
 
-    const start_at_previuos = this.knex(DB_TASKS_TABLE)
-      .where('id', '=', this.taskId)
-      .select('next_run_start_at')
-      .first();
-
     // It's OK if the task already exists; if it does, just replace its
     // settings with the new value and start the loop as usual.
+    const settingsJson = JSON.stringify(settings);
     await this.knex<DbTasksRow>(DB_TASKS_TABLE)
       .insert({
         id: this.taskId,
-        settings_json: JSON.stringify(settings),
+        settings_json: settingsJson,
         next_run_start_at: startAt,
       })
       .onConflict('id')
-      .merge({
-        settings_json: JSON.stringify(settings),
-        next_run_start_at: this.knex.raw('CASE WHEN ? < ? THEN ? ELSE ? END', [
-          start_at_previuos,
-          startAt,
-          start_at_previuos,
-          startAt,
-        ]),
-      });
+      .merge(
+        this.knex.client.config.client.includes('mysql')
+          ? {
+              settings_json: settingsJson,
+              next_run_start_at: this.knex.raw(
+                `CASE WHEN ?? < ?? THEN ?? ELSE ?? END`,
+                [startAt, 'next_run_start_at', startAt, 'next_run_start_at'],
+              ),
+            }
+          : {
+              settings_json: this.knex.ref('excluded.settings_json'),
+              next_run_start_at: this.knex.raw(
+                `CASE WHEN ?? < ?? THEN ?? ELSE ?? END`,
+                [
+                  'excluded.next_run_start_at',
+                  `${DB_TASKS_TABLE}.next_run_start_at`,
+                  'excluded.next_run_start_at',
+                  `${DB_TASKS_TABLE}.next_run_start_at`,
+                ],
+              ),
+            },
+      );
   }
 
   /**