fix(cron): prevent duplicate executions and enhance type safety (#690)

imports/cron/server.lua

@@ -7,28 +7,26 @@ local currentDate = {}
 setmetatable(currentDate, {
     __index = function(self, index)
         local newDate = os.date('*t') --[[@as Date]]
-
         for k, v in pairs(newDate) do
             self[k] = v
         end
-
         SetTimeout(1000, function() table.wipe(self) end)
-
         return self[index]
     end
 })
 
 ---@class OxTaskProperties
----@field minute? number | string
+---@field minute? number|string|function
----@field hour? number | string
+---@field hour? number|string|function
----@field day? number | string
+---@field day? number|string|function
----@field month? number | string
+---@field month? number|string|function
----@field year? number | string
+---@field year? number|string|function
----@field weekday? number | string
+---@field weekday? number|string|function
 ---@field job fun(task: OxTask, date: osdate)
 ---@field isActive boolean
 ---@field id number
 ---@field debug? boolean
+---@field lastRun? number
 
 ---@class OxTask : OxTaskProperties
 ---@field expression string
@@ -36,6 +34,14 @@ setmetatable(currentDate, {
 local OxTask = {}
 OxTask.__index = OxTask
 
+local validRanges = {
+    min = { min = 0, max = 59 },
+    hour = { min = 0, max = 23 },
+    day = { min = 1, max = 31 },
+    month = { min = 1, max = 12 },
+    wday = { min = 0, max = 7 },
+}
+
 local maxUnits = {
     min = 60,
     hour = 24,
@@ -44,7 +50,23 @@ local maxUnits = {
     month = 12,
 }
 
---- Gets the amount of days in certain month
+local weekdayMap = {
+    sun = 0,
+    mon = 1,
+    tue = 2,
+    wed = 3,
+    thu = 4,
+    fri = 5,
+    sat = 6,
+}
+
+local monthMap = {
+    jan = 1, feb = 2, mar = 3, apr = 4,
+    may = 5, jun = 6, jul = 7, aug = 8,
+    sep = 9, oct = 10, nov = 11, dec = 12
+}
+
+---Returns the last day of the specified month
 ---@param month number
 ---@param year? number
 ---@return number
@@ -52,9 +74,96 @@ local function getMaxDaysInMonth(month, year)
     return os.date('*t', os.time({ year = year or currentDate.year, month = month + 1, day = -1 })).day --[[@as number]]
 end
 
----@param value string | number | nil
+---@param value string|number
 ---@param unit string
----@return string | number | false | nil
+---@return boolean
+local function isValueInRange(value, unit)
+    local range = validRanges[unit]
+    if not range then return true end
+    return value >= range.min and value <= range.max
+end
+
+---@param value string
+---@param unit string
+---@return number|string|function|nil
+local function parseCron(value, unit)
+    if not value or value == '*' then return end
+
+    if unit == 'day' and value:lower() == 'l' then
+        return function()
+            return getMaxDaysInMonth(currentDate.month, currentDate.year)
+        end
+    end
+
+    local num = tonumber(value)
+    if num then
+        if not isValueInRange(num, unit) then
+            error(("^1invalid cron expression. '%s' is out of range for %s^0"):format(value, unit), 3)
+        end
+        return num
+    end
+
+    if unit == 'wday' then
+        local start, stop = value:match('(%a+)-(%a+)')
+        if start and stop then
+            start = weekdayMap[start:lower()]
+            stop = weekdayMap[stop:lower()]
+            if start and stop then
+                if stop < start then stop = stop + 7 end
+                return ('%d-%d'):format(start, stop)
+            end
+        end
+        local day = weekdayMap[value:lower()]
+        if day then return day end
+    end
+
+    if unit == 'month' then
+        local months = {}
+        for month in value:gmatch('[^,]+') do
+            local monthNum = monthMap[month:lower()]
+            if monthNum then
+                months[#months + 1] = tostring(monthNum)
+            end
+        end
+        if #months > 0 then
+            return table.concat(months, ',')
+        end
+    end
+
+    local stepMatch = value:match('^%*/(%d+)$')
+    if stepMatch then
+        local step = tonumber(stepMatch)
+        if not step or step == 0 then
+            error(("^1invalid cron expression. Step value cannot be %s^0"):format(step or 'nil'), 3)
+        end
+        return value
+    end
+
+    local start, stop = value:match('^(%d+)-(%d+)$')
+    if start and stop then
+        start, stop = tonumber(start), tonumber(stop)
+        if not start or not stop or not isValueInRange(start, unit) or not isValueInRange(stop, unit) then
+            error(("^1invalid cron expression. Range '%s' is invalid for %s^0"):format(value, unit), 3)
+        end
+        return value
+    end
+
+    local valid = true
+    for item in value:gmatch('[^,]+') do
+        local num = tonumber(item)
+        if not num or not isValueInRange(num, unit) then
+            valid = false
+            break
+        end
+    end
+    if valid then return value end
+
+    error(("^1invalid cron expression. '%s' is not supported for %s^0"):format(value, unit), 3)
+end
+
+---@param value string|number|function|nil
+---@param unit string
+---@return number|false|nil
 local function getTimeUnit(value, unit)
     local currentTime = currentDate[unit]
 
@@ -62,25 +171,24 @@ local function getTimeUnit(value, unit)
         return unit == 'min' and currentTime + 1 or currentTime
     end
 
+    if type(value) == 'function' then
+        return value()
+    end
+
     local unitMax = maxUnits[unit]
 
     if type(value) == 'string' then
         local stepValue = string.match(value, '*/(%d+)')
 
         if stepValue then
-            -- */10 * * * * is equal to a list of 0,10,20,30,40,50
+            local step = tonumber(stepValue)
-            -- best suited to factors of unitMax (excluding the highest and lowest numbers)
-            -- i.e. for minutes - 2, 3, 4, 5, 6, 10, 12, 15, 20, 30
             for i = currentTime + 1, unitMax do
-                -- if i is divisible by stepValue
+                if i % step == 0 then return i end
-                if i % stepValue == 0 then return i end
             end
-
+            return step + unitMax
-            return stepValue + unitMax
         end
 
         local range = string.match(value, '%d+-%d+')
-
         if range then
             local min, max = string.strsplit('-', range)
             min, max = tonumber(min, 10), tonumber(max, 10)
@@ -97,12 +205,15 @@ local function getTimeUnit(value, unit)
         end
 
         local list = string.match(value, '%d+,%d+')
-
         if list then
-            for listValue in string.gmatch(value, '%d+') --[[@as number]] do
+            local values = {}
-                listValue = tonumber(listValue)
+            for listValue in string.gmatch(value, '%d+') do
+                values[#values + 1] = tonumber(listValue)
+            end
+            table.sort(values)
 
-                -- e.g. if current time is less than in the expression 0,10,20,45 * * * *
+            for i = 1, #values do
+                local listValue = values[i]
                 if unit == 'min' then
                     if currentTime < listValue then
                         return listValue
@@ -112,56 +223,47 @@ local function getTimeUnit(value, unit)
                 end
             end
 
-            -- if iterator failed, return the first value in the list
+            return values[1] + unitMax
-            return tonumber(string.match(value, '%d+')) + unitMax
         end
 
         return false
     end
 
     if unit == 'min' then
-        return value <= currentTime and value + unitMax or value
+        return value <= currentTime and value + unitMax or value --[[@as number]]
     end
 
-    return value < currentTime and value + unitMax or value
+    return value < currentTime and value + unitMax or value --[[@as number]]
 end
 
----Get a timestamp for the next time to run the task today.
 ---@return number?
 function OxTask:getNextTime()
     if not self.isActive then return end
 
     local day = getTimeUnit(self.day, 'day')
 
-    -- If current day is the last day of the month, and the task is scheduled for the last day of the month, then the task should run.
     if day == 0 then
-        -- Should probably be used month from getTimeUnit, but don't want to reorder this code.
         day = getMaxDaysInMonth(currentDate.month)
     end
 
     if day ~= currentDate.day then return end
 
     local month = getTimeUnit(self.month, 'month')
-
     if month ~= currentDate.month then return end
 
     local weekday = getTimeUnit(self.weekday, 'wday')
-
+    if weekday and weekday ~= currentDate.wday then return end
-    if weekday ~= currentDate.wday then return end
 
     local minute = getTimeUnit(self.minute, 'min')
-
     if not minute then return end
 
     local hour = getTimeUnit(self.hour, 'hour')
-
     if not hour then return end
 
     if minute >= maxUnits.min then
         if not self.hour then
             hour += math.floor(minute / maxUnits.min)
         end
-
         minute = minute % maxUnits.min
     end
 
@@ -169,20 +271,31 @@ function OxTask:getNextTime()
         if not self.day then
             day += math.floor(hour / maxUnits.hour)
         end
-
         hour = hour % maxUnits.hour
     end
 
-    return os.time({
+    local nextTime = os.time({
         min = minute,
         hour = hour,
         day = day or currentDate.day,
         month = month or currentDate.month,
         year = currentDate.year,
     })
+
+    if self.lastRun and nextTime - self.lastRun < 60 then
+        if self.debug then
+            print(('Preventing duplicate execution of task %s - Last run: %s, Next scheduled: %s'):format(
+                self.id,
+                os.date('%c', self.lastRun),
+                os.date('%c', nextTime)
+            ))
+        end
+        return
+    end
+
+    return nextTime
 end
 
----Get timestamp for next time to run task at any day.
 ---@return number
 function OxTask:getAbsoluteNextTime()
     local minute = getTimeUnit(self.minute, 'min')
@@ -191,7 +304,6 @@ function OxTask:getAbsoluteNextTime()
     local month = getTimeUnit(self.month, 'month')
     local year = getTimeUnit(self.year, 'year')
 
-    -- To avoid modifying getTimeUnit function, the day is adjusted here if needed.
     if self.day then
         if currentDate.hour < hour or (currentDate.hour == hour and currentDate.min < minute) then
             day = day - 1
@@ -209,7 +321,6 @@ function OxTask:getAbsoluteNextTime()
         end
     end
 
-    -- Check if time will be in next year.
     ---@diagnostic disable-next-line: assign-type-mismatch
     if os.time({ year = year, month = month, day = day, hour = hour, min = minute }) < os.time() then
         year = year and year + 1 or currentDate.year + 1
@@ -255,7 +366,7 @@ function OxTask:scheduleTask()
 
     if sleep > 0 then
         Wait(sleep * 1000)
-    else -- will this even happen?
+    else
         Wait(1000)
         return true
     end
@@ -267,15 +378,13 @@ function OxTask:scheduleTask()
 
         Citizen.CreateThreadNow(function()
             self:job(currentDate)
+            self.lastRun = os.time()
         end)
 
-        -- Wait(30000)
-
         return true
     end
 end
 
----Start an inactive task.
 function OxTask:run()
     if self.isActive then return end
 
@@ -298,45 +407,6 @@ function OxTask:stop(msg)
     end
 end
 
----@param value string
----@return number | string | nil
-local function parseCron(value, unit)
-    if not value or value == '*' then return end
-
-    local num = tonumber(value)
-
-    if num then return num end
-
-    if unit == 'wday' then
-        if value == 'sun' then return 1 end
-        if value == 'mon' then return 2 end
-        if value == 'tue' then return 3 end
-        if value == 'wed' then return 4 end
-        if value == 'thu' then return 5 end
-        if value == 'fri' then return 6 end
-        if value == 'sat' then return 7 end
-    end
-
-    if unit == 'month' then
-        if value == 'jan' then return 1 end
-        if value == 'feb' then return 2 end
-        if value == 'mar' then return 3 end
-        if value == 'apr' then return 4 end
-        if value == 'may' then return 5 end
-        if value == 'jun' then return 6 end
-        if value == 'jul' then return 7 end
-        if value == 'aug' then return 8 end
-        if value == 'sep' then return 9 end
-        if value == 'oct' then return 10 end
-        if value == 'nov' then return 11 end
-        if value == 'dec' then return 12 end
-    end
-
-    if getTimeUnit(value, unit) then return value end
-
-    error(("^1invalid cron expression. '%s' is not supported for %s^0"):format(value, unit), 3)
-end
-
 ---@param expression string A cron expression such as `* * * * *` representing minute, hour, day, month, and day of the week.
 ---@param job fun(task: OxTask, date: osdate)
 ---@param options? { debug?: boolean }
@@ -360,6 +430,7 @@ function lib.cron.new(expression, job, options)
     task.weekday = parseCron(weekday, 'wday')
     task.id = #tasks + 1
     task.job = job
+    task.lastRun = nil
     tasks[task.id] = task
     task:run()
 
@@ -370,7 +441,6 @@ end
 lib.cron.new('0 0 * * *', function()
     for i = 1, #tasks do
         local task = tasks[i]
-
         if not task.isActive then
             task:run()
         end