Merge pull request #791 from 3scale/use-gc-module-in-timer-task

TimerTask improvements

CHANGELOG.md

@@ -21,7 +21,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - ENV variables to make APIcast listen on HTTPS port [PR #622](https://github.com/3scale/apicast/pull/622) 
 - New `ssl_certificate` phase allows policies to provide certificate to terminate HTTPS connection [PR #622](https://github.com/3scale/apicast/pull/622).
 - Configurable `auth_type` for the token introspection policy [PR #755](https://github.com/3scale/apicast/pull/755)
-- `TimerTask` module to execute recurrent tasks that can be cancelled [PR #782](https://github.com/3scale/apicast/pull/782), [#784](https://github.com/3scale/apicast/pull/784)
+- `TimerTask` module to execute recurrent tasks that can be cancelled [PR #782](https://github.com/3scale/apicast/pull/782), [PR #784](https://github.com/3scale/apicast/pull/784), [PR #791](https://github.com/3scale/apicast/pull/791)
 - `GC` module that implements a workaround to be able to define `__gc` on tables [PR #790](https://github.com/3scale/apicast/pull/790)
 
 ### Changed

gateway/src/resty/concurrent/timer_task.lua

@@ -1,7 +1,6 @@
+local GC = require 'apicast.gc'
+
 local uuid = require 'resty.jit-uuid'
-local setmetatable = setmetatable
-local getmetatable = getmetatable
-local newproxy = newproxy
 local unpack = unpack
 
 local _M = {}
@@ -26,17 +25,15 @@ local function generate_id()
   return uuid.generate_v4()
 end
 
-local function mt(id)
-  -- Using 'newproxy' is needed to be able to define __gc().
-  -- With __gc we can make sure that the task will stop scheduling more work
-  -- after it has been garbage collected.
-  local proxy = newproxy(true)
-  local res_mt = getmetatable(proxy)
-  res_mt.__gc = function() _M.unregister_task(id) end
-  res_mt.__index = _M
-  return res_mt
+local function gc(self)
+  _M.unregister_task(self.id)
 end
 
+local mt = {
+  __gc = gc,
+  __index = _M
+}
+
 --- Initialize a TimerTask.
 -- @tparam function task The function to be run periodically
 -- @tparam[opt] table opts
@@ -47,7 +44,7 @@ function _M.new(task, opts)
 
   local id = generate_id()
 
-  local self = setmetatable({}, mt(id))
+  local self = GC.set_metatable_gc({}, mt)
   self.task = task
   self.args = options.args
   self.interval = options.interval or default_interval_seconds
@@ -60,25 +57,25 @@ end
 
 local run_periodic, schedule_next, timer_execute
 
-run_periodic = function(self, run_now)
-  if not _M.task_is_active(self.id) then return end
+run_periodic = function(run_now, id, func, args, interval)
+  if not _M.task_is_active(id) then return end
 
   if run_now then
-    self.task(unpack(self.args))
+    func(unpack(args))
   end
 
-  schedule_next(self)
+  schedule_next(interval, id, func, args, interval)
 end
 
 -- Note: ngx.timer.at always sends "premature" as the first param.
 -- "premature" is boolean value indicating whether it is a premature timer
 -- expiration.
-timer_execute = function(_, self)
-  run_periodic(self, true)
+timer_execute = function(_, id, func, args, interval)
+  run_periodic(true, id, func, args, interval)
 end
 
-schedule_next = function(self)
-  local ok, err = ngx.timer.at(self.interval, timer_execute, self)
+schedule_next = function(id, func, args, interval)
+  local ok, err = ngx.timer.at(interval, timer_execute, id, func, args, interval)
 
   if not ok then
     ngx.log(ngx.ERR, "failed to schedule timer task: ", err)
@@ -89,7 +86,7 @@ end
 -- @tparam[opt] run_now boolean True to run the task immediately or False to
 --   wait 'interval' seconds. (Defaults to false)
 function _M:execute(run_now)
-  run_periodic(self, run_now or false)
+  run_periodic(run_now or false, self.id, self.task, self.args, self.interval)
 end
 
 function _M:cancel()

spec/resty/concurrent/timer_task_spec.lua

@@ -85,11 +85,11 @@ describe('TimerTask', function()
     describe('when the task is active', function()
       it('runs the task', function()
         local timer_task = TimerTask.new(func, { args = args, interval = interval })
-        local func_stub = stub(timer_task, 'task')
+        local func_spy = spy.on(timer_task, 'task')
 
         timer_task:execute(true)
 
-        assert.stub(func_stub).was_called_with(unpack(args))
+        assert.spy(func_spy).was_called_with(unpack(args))
       end)
 
       it('schedules the next one', function()
@@ -104,12 +104,12 @@ describe('TimerTask', function()
     describe('when the task is not active', function()
       it('does not run the task', function()
         local timer_task = TimerTask.new(func, { args = args, interval = interval })
-        local func_stub = stub(timer_task, 'task')
+        local func_spy = spy.on(timer_task, 'task')
         timer_task:cancel()
 
         timer_task:execute(true)
 
-        assert.stub(func_stub).was_not_called()
+        assert.spy(func_spy).was_not_called()
       end)
 
       it('does not schedule another task', function()
@@ -125,12 +125,12 @@ describe('TimerTask', function()
     describe('when the option to wait an interval instead of running now is passed', function()
       it('does not run the task inmediately', function()
         local timer_task = TimerTask.new(func, { args = args, interval = interval })
-        local func_stub = stub(timer_task, 'task')
+        local func_spy = spy.on(timer_task, 'task')
 
         timer_task:execute(false)
 
         -- It will be called in 'interval' seconds, but not now
-        assert.stub(func_stub).was_not_called()
+        assert.spy(func_spy).was_not_called()
       end)
 
       it('schedules the next one', function()