Merge pull request #505 from 3scale/policy-chain-manipulation

methods for policy chain manipulation

CHANGELOG.md

@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - New policy chains system. This allows users to write custom policies to configure what Apicast can do on each of the Nginx phases [PR #450](https://github.com/3scale/apicast/pull/450)
 - Resolver can resolve nginx upstreams [PR #478](https://github.com/3scale/apicast/pull/478)
 - Calls 3scale backend with the 'no_body' option enabled. This reduces network traffic in cases where APIcast does not need to parse the response body [PR #483](https://github.com/3scale/apicast/pull/483)
+- Methods to modify policy chains [PR #505](https://github.com/3scale/apicast/pull/505)
 
 ## Changed
 

gateway/src/apicast/executor.lua

@@ -6,20 +6,14 @@
 
 require('apicast.loader') -- to load code from deprecated paths
 
-local policy_chain = require('apicast.policy_chain')
+local PolicyChain = require('apicast.policy_chain')
 local policy = require('apicast.policy')
 local linked_list = require('apicast.linked_list')
 
 local setmetatable = setmetatable
 
 local _M = { }
 
-local DEFAULT_POLICIES = {
-    'apicast.policy.load_configuration',
-    'apicast.policy.find_service',
-    'apicast.policy.local_chain'
-}
-
 local mt = { __index = _M }
 
 -- forward all policy methods to the policy chain
@@ -29,12 +23,8 @@ for _,phase in policy.phases() do
     end
 end
 
-local function global_chain()
-    return policy_chain.build(DEFAULT_POLICIES)
-end
-
-function _M.new()
-    return setmetatable({ policy_chain = global_chain() }, mt)
+function _M.new(policy_chain)
+    return setmetatable({ policy_chain = policy_chain:freeze() }, mt)
 end
 
 local function build_context(executor)
@@ -67,4 +57,4 @@ function _M:context(phase)
     return shared_build_context(self)
 end
 
-return _M.new()
+return _M.new(PolicyChain.default())

gateway/src/apicast/policy_chain.lua

@@ -10,10 +10,11 @@ local error = error
 local rawset = rawset
 local type = type
 local require = require
+local insert = table.insert
 local noop = function() end
 
 local linked_list = require('apicast.linked_list')
-local policy = require('apicast.policy')
+local policy_phases = require('apicast.policy').phases
 
 local _M = {
 
@@ -49,12 +50,25 @@ function _M.build(modules)
     return _M.new(chain)
 end
 
+
+local DEFAULT_POLICIES = {
+    'apicast.policy.load_configuration',
+    'apicast.policy.find_service',
+    'apicast.policy.local_chain'
+}
+
+--- Return new policy chain with default policies.
+-- @treturn PolicyChain
+function _M.default()
+    return _M.build(DEFAULT_POLICIES)
+end
+
 --- Load a module
 -- If the module is a string, returns the result of initializing it with the
 -- given arguments. Otherwise, this function simply returns the module
 -- received.
--- @tparam string/object The module
--- @tparam[opt] params needed to initialize the module
+-- @tparam string|table module the module or its name
+-- @tparam ?table ... params needed to initialize the module
 -- @treturn object The module instantiated
 function _M.load(module, ...)
     if type(module) == 'string' then
@@ -71,15 +85,20 @@ function _M.load(module, ...)
     end
 end
 
+--- Initialize new @{PolicyChain}.
+-- @treturn PolicyChain
 function _M.new(list)
     local chain = list or {}
 
     local self = setmetatable(chain, mt)
     chain.config = self:export()
-    self.frozen = true
     return self
 end
 
+---------------------
+--- @type PolicyChain
+-- An instance of @{policy_chain}.
+
 --- Export the shared context of the chain
 -- @treturn linked_list The context of the chain. Note: the list returned is
 --   read-only.
@@ -96,6 +115,29 @@ function _M:export()
     return chain
 end
 
+--- Freeze the policy chain to prevent modifications.
+-- After calling this method it won't be possible to insert more policies.
+-- @treturn PolicyChain returns self
+function _M:freeze()
+    self.frozen = true
+    return self
+end
+
+--- Insert a policy into the chain
+-- @tparam Policy policy the policy to be added to the chain
+-- @tparam[opt] int position the position to add the policy to, defaults to last one
+-- @treturn int lenght of the chain
+-- @error frozen | returned when chain is not modifiable
+-- @see freeze
+function _M:insert(policy, position)
+    if self.frozen then
+        return nil, 'frozen'
+    else
+        insert(self, position or #self+1, policy)
+        return #self
+    end
+end
+
 local function call_chain(phase_name)
     return function(self, ...)
         for i=1, #self do
@@ -104,8 +146,8 @@ local function call_chain(phase_name)
     end
 end
 
-for _,phase in policy.phases() do
+for _,phase in policy_phases() do
     _M[phase] = call_chain(phase)
 end
 
-return _M.build()
+return _M.build():freeze()

spec/executor_spec.lua

@@ -1,3 +1,6 @@
+local executor = require 'apicast.executor'
+local policy_chain = require 'apicast.policy_chain'
+
 describe('executor', function()
   local phases = {
     'init', 'init_worker',
@@ -7,8 +10,6 @@ describe('executor', function()
   }
 
   it('forwards all the policy methods to the policy chain', function()
-    local executor = require 'apicast.executor'
-
     -- Policies included by default in the executor
     local default_executor_chain = {
       require 'apicast.policy.load_configuration',
@@ -32,4 +33,12 @@ describe('executor', function()
       end
     end
   end)
+
+  it('freezes the policy chain', function()
+    local chain = policy_chain.new({})
+    assert.falsy(chain.frozen)
+
+    executor.new(chain)
+    assert.truthy(chain.frozen)
+  end)
 end)

spec/policy_chain_spec.lua

@@ -1,4 +1,5 @@
 local policy = require 'apicast.policy'
+local _M = require 'apicast.policy_chain'
 
 describe('policy_chain', function()
   local phases = {
@@ -9,10 +10,8 @@ describe('policy_chain', function()
   }
 
   it('defines a method for each of the nginx phases supported', function()
-    local policy_chain = require 'apicast.policy_chain'
-
     for _, phase in ipairs(phases) do
-      assert.equals('function', type(policy_chain[phase]))
+      assert.equals('function', type(_M[phase]))
     end
   end)
 
@@ -32,8 +31,7 @@ describe('policy_chain', function()
     stub(custom_policy_2, 'access')
 
     -- Build the policy chain
-    local policy_chain = require 'apicast.policy_chain'
-    local chain = policy_chain.build({ custom_policy_1, custom_policy_2 })
+    local chain = _M.build({ custom_policy_1, custom_policy_2 })
 
     chain:init()
     assert.stub(custom_policy_1.init).was_called()
@@ -48,7 +46,6 @@ describe('policy_chain', function()
 
   it('uses APIcast as default when no policies are specified', function()
     local apicast = require 'apicast.policy.apicast'
-    local policy_chain = require 'apicast.policy_chain'
 
     -- Stub apicast methods to avoid calling them. We are just interested in
     -- knowing whether they were called.
@@ -57,7 +54,7 @@ describe('policy_chain', function()
     end
 
     for _, phase in ipairs(phases) do
-      policy_chain[phase](policy_chain)
+      _M[phase](_M)
       assert.stub(apicast[phase]).was_called()
     end
   end)
@@ -74,32 +71,59 @@ describe('policy_chain', function()
       end
     end
 
-    local policy_chain = require 'apicast.policy_chain'
     local sorted_policies = { policies[2], policies[3], policies[1] }
-    local chain = policy_chain.build(sorted_policies)
+    local chain = _M.build(sorted_policies)
 
     chain:init()
     assert.same({'2', '3', '1'}, execution_order)
   end)
 
   it('does not allow to modify phase methods after the chain has been built', function()
-    local policy_chain = require 'apicast.policy_chain'
-
     for _, phase in ipairs(phases) do
       assert.has_error(function()
-        policy_chain[phase] = function() end
+        _M[phase] = function() end
       end, 'readonly table')
     end
   end)
 
   it('does not allow to add new methods to the chain after it has been built', function()
-    local policy_chain = require 'apicast.policy_chain'
-
     assert.has_error(function()
-      policy_chain['new_phase'] = function() end
+      _M['new_phase'] = function() end
     end, 'readonly table')
   end)
 
+  describe('.insert', function()
+
+    it('adds policy to the end of the chain', function()
+      local chain = _M.new({ 'one', 'two' })
+
+      chain:insert(policy)
+
+      assert.equal(policy, chain[3])
+      assert.equal(3, #chain)
+    end)
+
+    it('adds a policy to specific position', function()
+      local chain = _M.new({ 'one', 'two'})
+
+      chain:insert(policy, 2)
+      assert.equal(policy, chain[2])
+      assert.equal('one', chain[1])
+      assert.equal('two', chain[3])
+      assert.equal(3, #chain)
+    end)
+
+    it('errors when inserting to frozen chain', function()
+      local chain = _M.new({}):freeze()
+
+      local ok, err = chain:insert(policy, 1)
+
+      assert.is_nil(ok)
+      assert.equal(err, 'frozen')
+      assert.equal(0, #chain)
+    end)
+  end)
+
   describe('.export', function()
     it('returns the data exposed by each of its policies', function()
       local policy_1 = policy.new('1')
@@ -108,8 +132,7 @@ describe('policy_chain', function()
       local policy_2 = policy.new('2')
       policy_2.export = function() return { shared_data_2 = '2' } end
 
-      local policy_chain = require 'apicast.policy_chain'
-      local chain = policy_chain.build({ policy_1, policy_2 })
+      local chain = _M.build({ policy_1, policy_2 })
 
       local shared_data = chain:export()
       assert.equal('1', shared_data['shared_data_1'])
@@ -120,8 +143,7 @@ describe('policy_chain', function()
       local policy_1 = policy.new('1')
       policy_1.export = function() return { shared_data = '1' } end
 
-      local policy_chain = require 'apicast.policy_chain'
-      local chain = policy_chain.build({ policy_1 })
+      local chain = _M.build({ policy_1 })
 
       local shared_data = chain:export()
 
@@ -139,12 +161,23 @@ describe('policy_chain', function()
         local policy_2 = policy.new('custom_authorizer')
         policy_2.export = function() return { shared_data_1 = '2' } end
 
-        local policy_chain = require 'apicast.policy_chain'
-        local chain = policy_chain.build({ policy_1, policy_2 })
+        local chain = _M.build({ policy_1, policy_2 })
 
         local shared_data = chain:export()
         assert.equal('1', shared_data['shared_data_1'])
       end)
     end)
   end)
+
+  describe('.default', function()
+    it('returns a default policy chain', function()
+      local default = _M.default()
+
+      assert(#default > 1, 'has <= 1 policy')
+    end)
+
+    it('returns not frozen chain', function()
+      assert.falsy(_M.default().frozen)
+    end)
+  end)
 end)