fix(job-scheduler): upsert template when same pattern options are pro…

…vided (#2943) ref #2940
taskforcesh · Nov 29, 2024 · b56c3b4 · b56c3b4
1 parent 36b9973
commit b56c3b4
Show file tree

Hide file tree

Showing 2 changed files with 74 additions and 21 deletions.
diff --git a/src/commands/addJobScheduler-2.lua b/src/commands/addJobScheduler-2.lua
@@ -63,10 +63,12 @@ end
 local prevMillis = rcall("ZSCORE", repeatKey, jobSchedulerId)
 if prevMillis ~= false then
   local delayedJobId =  "repeat:" .. jobSchedulerId .. ":" .. prevMillis
-  local nextDelayedJobId =  repeatKey .. ":" .. jobSchedulerId .. ":" .. nextMillis
+  local nextDelayedJobId =  "repeat:" .. jobSchedulerId .. ":" .. nextMillis
+  local nextDelayedJobKey =  repeatKey .. ":" .. jobSchedulerId .. ":" .. nextMillis
 
   if rcall("ZSCORE", delayedKey, delayedJobId) ~= false
-   and rcall("EXISTS", nextDelayedJobId) ~= 1 then
+    and (rcall("EXISTS", nextDelayedJobKey) ~= 1 
+    or delayedJobId == nextDelayedJobId) then
     removeJob(delayedJobId, true, prefixKey, true --[[remove debounce key]])
     rcall("ZREM", delayedKey, delayedJobId)
   end

diff --git a/tests/test_job_scheduler.ts b/tests/test_job_scheduler.ts
@@ -1403,35 +1403,86 @@ describe('Job Scheduler', function () {
     });
   });
 
-  it('should keep only one delayed job if adding a new repeatable job with the same id', async function () {
-    const date = new Date('2017-02-07 9:24:00');
-    const key = 'mykey';
+  describe('when every option is provided', function () {
+    it('should keep only one delayed job if adding a new repeatable job with the same id', async function () {
+      const date = new Date('2017-02-07 9:24:00');
+      const key = 'mykey';
 
-    this.clock.setSystemTime(date);
+      this.clock.setSystemTime(date);
 
-    const nextTick = 2 * ONE_SECOND;
+      const nextTick = 2 * ONE_SECOND;
 
-    await queue.upsertJobScheduler(key, {
-      every: 10_000,
-    });
+      await queue.upsertJobScheduler(key, {
+        every: 10_000,
+      });
 
-    this.clock.tick(nextTick);
+      this.clock.tick(nextTick);
 
-    let jobs = await queue.getJobSchedulers();
-    expect(jobs).to.have.length(1);
+      let jobs = await queue.getJobSchedulers();
+      expect(jobs).to.have.length(1);
 
-    let delayedJobs = await queue.getDelayed();
-    expect(delayedJobs).to.have.length(1);
+      let delayedJobs = await queue.getDelayed();
+      expect(delayedJobs).to.have.length(1);
 
-    await queue.upsertJobScheduler(key, {
-      every: 35_160,
+      await queue.upsertJobScheduler(key, {
+        every: 35_160,
+      });
+
+      jobs = await queue.getJobSchedulers();
+      expect(jobs).to.have.length(1);
+
+      delayedJobs = await queue.getDelayed();
+      expect(delayedJobs).to.have.length(1);
     });
+  });
+
+  describe('when pattern option is provided', function () {
+    it('should keep only one delayed job if adding a new repeatable job with the same id', async function () {
+      const date = new Date('2017-02-07 9:24:00');
+      const key = 'mykey';
+
+      this.clock.setSystemTime(date);
+
+      const nextTick = 2 * ONE_SECOND;
 
-    jobs = await queue.getJobSchedulers();
-    expect(jobs).to.have.length(1);
+      await queue.upsertJobScheduler(
+        key,
+        {
+          pattern: '0 * 1 * *',
+        },
+        { name: 'test1', data: { foo: 'bar' }, opts: { priority: 1 } },
+      );
+
+      this.clock.tick(nextTick);
+
+      let jobs = await queue.getJobSchedulers();
+      expect(jobs).to.have.length(1);
+
+      let delayedJobs = await queue.getDelayed();
+      expect(delayedJobs).to.have.length(1);
+
+      await queue.upsertJobScheduler(
+        key,
+        {
+          pattern: '0 * 1 * *',
+        },
+        { name: 'test2', data: { foo: 'baz' }, opts: { priority: 2 } },
+      );
+
+      jobs = await queue.getJobSchedulers();
+      expect(jobs).to.have.length(1);
 
-    delayedJobs = await queue.getDelayed();
-    expect(delayedJobs).to.have.length(1);
+      delayedJobs = await queue.getDelayed();
+      expect(delayedJobs).to.have.length(1);
+
+      expect(delayedJobs[0].name).to.be.equal('test2');
+      expect(delayedJobs[0].data).to.deep.equal({
+        foo: 'baz',
+      });
+      expect(delayedJobs[0].opts).to.deep.include({
+        priority: 2,
+      });
+    });
   });
 
   // This test is flaky and too complex we need something simpler that tests the same thing