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 bug in export() applied to policies in the local chain #673

Merged
merged 4 commits into from
Apr 23, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- Rate Limit policy [PR #648](https://github.com/3scale/apicast/pull/648)
- [THREESCALE-799](https://issues.jboss.org/browse/THREESCALE-799) Documented restrictions in the position in the chain for some policies [PR #675](https://github.com/3scale/apicast/pull/675)

### Fixed

- `export()` now works correctly in policies of the local chain [PR #673](https://github.com/3scale/apicast/pull/673)

### Changed

- descriptions in `oneOf`s in policy manifests have been replaced with titles [PR #663](https://github.com/3scale/apicast/pull/663)
Expand Down
12 changes: 10 additions & 2 deletions gateway/src/apicast/policy/local_chain/local_chain.lua
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ local resty_env = require('resty.env')

local policy = require('apicast.policy')
local Proxy = require('apicast.proxy')
local LinkedList = require('apicast.linked_list')
local _M = policy.new('Local Policy Chain')

local function build_default_chain()
Expand All @@ -27,7 +28,11 @@ local function build_chain(context)
local proxy = Proxy.new(context.configuration)

context.proxy = context.proxy or proxy
context.policy_chain = find_policy_chain(context)

local policy_chain = find_policy_chain(context)
context.policy_chain = policy_chain

return policy_chain
end

-- forward all policy methods to the policy chain
Expand All @@ -44,7 +49,10 @@ end
local rewrite = _M.rewrite

function _M:rewrite(context, ...)
build_chain(context)
local policy_chain = build_chain(context)

context = LinkedList.readwrite(context, policy_chain:export())

rewrite(self, context, ...)
end

Expand Down
83 changes: 83 additions & 0 deletions spec/policy/local_chain/local_chain_spec.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
local LocalChain = require 'apicast.policy.local_chain'
local policy = require 'apicast.policy'
local PolicyChain = require 'apicast.policy_chain'

describe('local chain', function()
describe('.rewrite', function()
-- In all these tests, we export some data in 2 policies of the local
-- chain, then call rewrite() on the local chain, and finally, check that
-- each policy has the correct context when it runs its rewrite() method.

it('forwards a context that includes the data exported by the policies', function()
local assert_correct_context = function(_, context)
assert.equals(context.shared_data_1, 'from_policy_1')
assert.equals(context.shared_data_2, 'from_policy_2' )
end

local policy_1 = policy.new()
policy_1.export = function() return { shared_data_1 = 'from_policy_1' } end
policy_1.rewrite = assert_correct_context

local policy_2 = policy.new()
policy_2.export = function() return { shared_data_2 = 'from_policy_2' } end
policy_2.rewrite = assert_correct_context

local policy_chain = PolicyChain.build({ policy_1, policy_2 })

-- Passing policy_chain in the context. That way the local_chain will use
-- it instead of building a new one.
-- Configuration is needed to avoid instantiating a proxy
local context = { configuration = {}, policy_chain = policy_chain }

LocalChain.new():rewrite(context)
end)

describe('when there are several policies exposing the same data', function()
it('gives precedence to the first policy in the chain', function()
local assert_correct_context = function(_, context)
assert.equals(context.shared_data, 'from_policy_1')
end

local policy_1 = policy.new()
policy_1.export = function() return { shared_data = 'from_policy_1' } end
policy_1.rewrite = assert_correct_context

local policy_2 = policy.new()
policy_2.export = function() return { shared_data = 'from_policy_2' } end
policy_2.rewrite = assert_correct_context

local policy_chain = PolicyChain.build({ policy_1, policy_2 })

local context = { configuration = {}, policy_chain = policy_chain }

LocalChain.new():rewrite(context)
end)
end)

describe('when the context already includes some data', function()
it('gives precedence to that data over the one in the local chain', function()
local assert_correct_context = function(_, context)
assert.equals(context.shared_data, 'from_context')
end

local policy_1 = policy.new()
policy_1.export = function() return { shared_data = 'from_policy_1' } end
policy_1.rewrite = assert_correct_context

local policy_2 = policy.new()
policy_2.export = function() return { shared_data = 'from_policy_2' } end
policy_2.rewrite = assert_correct_context

local policy_chain = PolicyChain.build({ policy_1, policy_2 })

local context = {
configuration = {},
policy_chain = policy_chain,
shared_data = 'from_context'
}

LocalChain.new():rewrite(context)
end)
end)
end)
end)