[executor] call .init and .init_worker on policy modules

Because it has to be executed for policies without configuration, it
needs to be executed on the module level, not instance.

Author: mikz

CHANGELOG.md

@@ -31,6 +31,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Use forked `resty.limit.count` that uses increments instead of decrements [PR #758](https://github.com/3scale/apicast/pull/758)
 - Rate Limit policy to take into account changes in the config [PR #703](https://github.com/3scale/apicast/pull/703)
 - The regular expression for mapping rules has been changed, so that special characters are accepted in the wildcard values for path [PR #717](https://github.com/3scale/apicast/pull/714)
+- Call `init` and `init_worker` on all available policies regardless they are used or not [PR #770](https://github.com/3scale/apicast/pull/770)
+- Cache loaded policies. Loading one policy several times will use the same instance [PR #770](https://github.com/3scale/apicast/pull/770)
+- Load all policies into cache when starting APIcast master process. [PR #770](https://github.com/3scale/apicast/pull/770)
+- `init` and `init_worker` phases are executed on the policy module, not the instance of a policy with a configuration [PR #770](https://github.com/3scale/apicast/pull/770)
 
 ### Fixed
 

examples/custom-module/blacklist.lua

@@ -35,7 +35,7 @@ end
 
 function _M.init()
   iputils.enable_lrucache()
-  apicast:init()
+  apicast.init()
 end
 
 local balancer_with_blacklist = resty_balancer.new(function(peers)

gateway/src/apicast/executor.lua

@@ -10,7 +10,7 @@ local PolicyChain = require('apicast.policy_chain')
 local policy = require('apicast.policy')
 local linked_list = require('apicast.linked_list')
 local prometheus = require('apicast.prometheus')
-local policy_loader = require('apicast.policy_loader')
+
 
 local setmetatable = setmetatable
 local ipairs = ipairs
@@ -19,8 +19,6 @@ local _M = { }
 
 local mt = { __index = _M }
 
-local policy_modules = policy_loader:get_all()
-
 -- forward all policy methods to the policy chain
 for _,phase in policy.phases() do
     _M[phase] = function(self, ...)
@@ -63,24 +61,41 @@ function _M:context(phase)
     return shared_build_context(self)
 end
 
-local init = _M.init
-function _M:init()
-    init(self)
+do
+    local policy_loader = require('apicast.policy_loader')
+    local policies
+
+    local init = _M.init
+    function _M:init()
+        local executed = {}
+
+        for _,policy in init(self) do
+            executed[policy.init] = true
+        end
+
+        policies = policy_loader:get_all()
 
-    for _, policy_mod in ipairs(policy_modules) do
-       if policy_mod.init then
-           policy_mod.init()
-       end
+        for _, policy in ipairs(policies) do
+            if not executed[policy.init] then
+                policy.init()
+                executed[policy.init] = true
+            end
+        end
     end
-end
 
-local init_worker = _M.init_worker
-function _M:init_worker()
-    init_worker(self)
+    local init_worker = _M.init_worker
+    function _M:init_worker()
+        local executed = {}
+
+        for _,policy in init_worker(self) do
+            executed[policy.init_worker] = true
+        end
 
-    for _, policy_mod in ipairs(policy_modules) do
-        if policy_mod.init_worker then
-            policy_mod.init_worker()
+        for _, policy in ipairs(policies or policy_loader:get_all()) do
+            if not executed[policy.init_worker] then
+                policy.init_worker()
+                executed[policy.init_worker] = true
+            end
         end
     end
 end

gateway/src/apicast/policy/load_configuration/load_configuration.lua

@@ -6,9 +6,11 @@ local configuration_store = require('apicast.configuration_store')
 
 local new = _M.new
 
+_M.configuration = configuration_store.new()
+
 function _M.new(...)
     local policy = new(...)
-    policy.configuration = configuration_store.new()
+    policy.configuration = _M.configuration
     return policy
 end
 
@@ -18,12 +20,12 @@ function _M:export()
     }
 end
 
-function _M:init()
-    configuration_loader.init(self.configuration)
+function _M.init()
+    configuration_loader.init(_M.configuration)
 end
 
-function _M:init_worker()
-    configuration_loader.init_worker(self.configuration)
+function _M.init_worker()
+    configuration_loader.init_worker(_M.configuration)
 end
 
 function _M:rewrite(context)

gateway/src/apicast/policy_chain.lua

@@ -9,6 +9,7 @@ local setmetatable = setmetatable
 local error = error
 local rawset = rawset
 local type = type
+local ipairs = ipairs
 local require = require
 local insert = table.insert
 local sub = string.sub
@@ -153,6 +154,8 @@ local function call_chain(phase_name)
             ngx.log(ngx.DEBUG, 'policy chain execute phase: ', phase_name, ', policy: ', self[i]._NAME, ', i: ', i)
             self[i][phase_name](self[i], ...)
         end
+
+        return ipairs(self)
     end
 end
 

spec/executor_spec.lua

@@ -1,4 +1,4 @@
-local executor = require 'apicast.executor'
+local Executor = require 'apicast.executor'
 local PolicyChain = require 'apicast.policy_chain'
 local Policy = require 'apicast.policy'
 
@@ -9,7 +9,7 @@ describe('executor', function()
 
   it('forwards all the policy methods to the policy chain', function()
     local chain = PolicyChain.default()
-    local exec = executor.new(chain)
+    local exec = Executor.new(chain)
     -- Stub all the nginx phases methods for each of the policies
     for _, phase in Policy.phases() do
       for _, policy in ipairs(chain) do
@@ -22,14 +22,44 @@ describe('executor', function()
     for _, phase in Policy.phases() do
       exec[phase](exec)
       for _, policy in ipairs(chain) do
-        assert.stub(policy[phase]).was_called()
+        assert.stub(policy[phase]).was_called(1)
       end
     end
   end)
 
+
+  context('policy is in the chain', function()
+
+    local executor
+    local policy
+
+    before_each(function()
+      local chain = PolicyChain.build({'apicast.policy.apicast'})
+      policy = getmetatable(chain[1]).__index
+      executor = Executor.new(chain)
+    end)
+
+    it('calls .init just once', function()
+      local init = stub(policy, 'init')
+
+      executor:init()
+
+      assert.stub(init).was_called(1)
+    end)
+
+    it('calls .init_worker just once', function()
+      local init = stub(policy, 'init_worker')
+
+      executor:init_worker()
+
+      assert.stub(init).was_called(1)
+    end)
+  end)
+
+
   it('is initialized with default chain', function()
     local default = PolicyChain.default()
-    local policy_chain = executor.policy_chain
+    local policy_chain = Executor.policy_chain
 
     assert.same(#default, #policy_chain)
 
@@ -45,7 +75,7 @@ describe('executor', function()
     local chain = PolicyChain.new({})
     assert.falsy(chain.frozen)
 
-    executor.new(chain)
+    Executor.new(chain)
     assert.truthy(chain.frozen)
   end)
 
@@ -58,7 +88,7 @@ describe('executor', function()
       policy_2.export = function() return { p2 = '2' } end
 
       local chain = PolicyChain.new({ policy_1, policy_2 })
-      local context = executor.new(chain):context('rewrite')
+      local context = Executor.new(chain):context('rewrite')
 
       assert.equal('1', context.p1)
       assert.equal('2', context.p2)
@@ -77,7 +107,7 @@ describe('executor', function()
       local inner_chain = PolicyChain.new({ policy_2, policy_3 })
       local outer_chain = PolicyChain.new({ policy_1, inner_chain })
 
-      local context = executor.new(outer_chain):context('rewrite')
+      local context = Executor.new(outer_chain):context('rewrite')
 
       assert.equal('1', context.p1)
       assert.equal('2', context.p2)