Skip to content

Commit

Permalink
[executor] call .init and .init_worker on policy modules
Browse files Browse the repository at this point in the history
Because it has to be executed for policies without configuration, it
needs to be executed on the module level, not instance.
  • Loading branch information
mikz committed Jun 18, 2018
1 parent 9d13e7d commit ebc62e5
Show file tree
Hide file tree
Showing 6 changed files with 84 additions and 30 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion examples/custom-module/blacklist.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
49 changes: 32 additions & 17 deletions gateway/src/apicast/executor.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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, ...)
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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)
Expand Down
3 changes: 3 additions & 0 deletions gateway/src/apicast/policy_chain.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
44 changes: 37 additions & 7 deletions spec/executor_spec.lua
Original file line number Diff line number Diff line change
@@ -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'

Expand All @@ -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
Expand All @@ -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)

Expand All @@ -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)

Expand All @@ -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)
Expand All @@ -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)
Expand Down

0 comments on commit ebc62e5

Please sign in to comment.