Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(cron): validate arguments in RunAt and improve timestamp handling #1498

Closed
wants to merge 4 commits into from
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 22 additions & 9 deletions [core]/cron/server/main.lua
Original file line number Diff line number Diff line change
@@ -1,27 +1,41 @@
local Jobs = {}
local LastTime = nil

---@param h number Hour (0-23)
---@param m number Minute (0-59)
---@param cb function Callback to execute
function RunAt(h, m, cb)
if type(h) ~= "number" or type(m) ~= "number" or type(cb) ~= "function" then
print("[cron] Invalid arguments to RunAt. Expected (number, number, function).")
return
end

Jobs[#Jobs + 1] = {
h = h,
m = m,
cb = cb,
}
end

---@return number Current timestamp
function GetUnixTimestamp()
return os.time()
end

---@param time number The current Unix timestamp
function OnTime(time)
local currentDay = os.date("*t", time).day
local currentMonth = os.date("*t", time).month
local currentYear = os.date("*t", time).year

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactor to call os.date("*t", timestamp) once, store the result in a local table, and extract the required fields (day, month, year) from that table. This improves efficiency by avoiding repeated parsing and enhances code readability and maintainability.

for i = 1, #Jobs, 1 do
local scheduledTimestamp = os.time({
hour = Jobs[i].h,
min = Jobs[i].m,
sec = 0, -- Assuming tasks run at the start of the minute
day = os.date("%d", time),
month = os.date("%m", time),
year = os.date("%Y", time),
year = currentYear,
month = currentMonth,
day = currentDay
})

if time >= scheduledTimestamp and (not LastTime or LastTime < scheduledTimestamp) then
Expand All @@ -32,20 +46,19 @@ function OnTime(time)
end

function Tick()
local time = GetUnixTimestamp()
local currentTime = GetUnixTimestamp()

if not LastTime or os.date("%M", time) ~= os.date("%M", LastTime) then
OnTime(time)
LastTime = time
if not LastTime or os.date("%M", currentTime) ~= os.date("%M", LastTime) then
OnTime(currentTime)
LastTime = currentTime
end

SetTimeout(60000, Tick)
end

LastTime = GetUnixTimestamp()

Tick()

AddEventHandler("cron:runAt", function(h, m, cb)
RunAt(h, m, cb)
end)
end)
Loading