Skip to content

Commit

Permalink
cache handlers: receive status code instead of full response
Browse files Browse the repository at this point in the history
  • Loading branch information
@davidor
davidor committed Jan 15, 2018
1 parent ca31f54 commit 37f2f06
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 37 deletions.
21 changes: 11 additions & 10 deletions gateway/src/apicast/backend/cache_handler.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,28 +34,29 @@ local function fetch_cached_key()
return ok and stored
end

function _M.handlers.strict(cache, cached_key, response, ttl)
if response.status == 200 then
function _M.handlers.strict(cache, cached_key, status_code, ttl)
if status_code == 200 then
-- cached_key is set in post_action and it is in in authorize
-- so to not write the cache twice lets write it just in authorize

if fetch_cached_key(cached_key) ~= cached_key then
ngx.log(ngx.INFO, 'apicast cache write key: ', cached_key, ', ttl: ', ttl, ' sub: ')
ngx.log(ngx.INFO, 'apicast cache write key: ', cached_key,
', ttl: ', ttl, ' sub: ')
cache:set(cached_key, 200, ttl or 0)
end
else
ngx.log(ngx.NOTICE, 'apicast cache delete key: ', cached_key, ' cause status ', response.status)
ngx.log(ngx.NOTICE, 'apicast cache delete key: ', cached_key,
' cause status ', status_code)
cache:delete(cached_key)
end
end

function _M.handlers.resilient(cache, cached_key, response, ttl)
local status = response.status
function _M.handlers.resilient(cache, cached_key, status_code, ttl)
if status_code and status_code < 500 then
ngx.log(ngx.INFO, 'apicast cache write key: ', cached_key,
' status: ', status_code, ', ttl: ', ttl )

if status and status < 500 then
ngx.log(ngx.INFO, 'apicast cache write key: ', cached_key, ' status: ', status, ', ttl: ', ttl )

cache:set(cached_key, status, ttl or 0)
cache:set(cached_key, status_code, ttl or 0)
end
end

Expand Down
16 changes: 7 additions & 9 deletions gateway/src/apicast/policy/caching/policy.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,26 +16,24 @@ local _M = policy.new('Caching policy')

local new = _M.new

local function strict_handler(cache, cached_key, response, ttl)
if response.status == 200 then
local function strict_handler(cache, cached_key, status_code, ttl)
if status_code == 200 then
ngx.log(ngx.INFO, 'apicast cache write key: ', cached_key,
', ttl: ', ttl, ' sub: ')
cache:set(cached_key, 200, ttl or 0)
else
ngx.log(ngx.NOTICE, 'apicast cache delete key: ', cached_key,
' cause status ', response.status)
' cause status ', status_code)
cache:delete(cached_key)
end
end

local function resilient_handler(cache, cached_key, response, ttl)
local status = response.status

if status and status < 500 then
local function resilient_handler(cache, cached_key, status_code, ttl)
if status_code and status_code < 500 then
ngx.log(ngx.INFO, 'apicast cache write key: ', cached_key,
' status: ', status, ', ttl: ', ttl)
' status: ', status_code, ', ttl: ', ttl)

cache:set(cached_key, status, ttl or 0)
cache:set(cached_key, status_code, ttl or 0)
end
end

Expand Down
2 changes: 1 addition & 1 deletion gateway/src/apicast/proxy.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ end
function _M:handle_backend_response(cached_key, response, ttl)
ngx.log(ngx.DEBUG, '[backend] response status: ', response.status, ' body: ', response.body)

self.cache_handler(self.cache, cached_key, response, ttl)
self.cache_handler(self.cache, cached_key, response.status, ttl)

local authorized = (response.status == 200)
local unauthorized_reason = not authorized and rejection_reason(response.headers)
Expand Down
16 changes: 8 additions & 8 deletions spec/backend/cache_handler_spec.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ describe('Cache Handler', function()
local cache = lrucache.new(1)
ngx.var = { cached_key = nil }

handler(cache, 'foobar', { status = 200 })
handler(cache, 'foobar', 200)

assert.equal(200, cache:get('foobar'))
end)
Expand All @@ -41,7 +41,7 @@ describe('Cache Handler', function()
local cache = lrucache.new(1)
ngx.var = { cached_key = 'foobar' } -- means it is performed in post_action

handler(cache, 'foobar', { status = 200 })
handler(cache, 'foobar', 200)

assert.falsy(cache:get('foobar'))
end)
Expand All @@ -50,7 +50,7 @@ describe('Cache Handler', function()
local cache = lrucache.new(1)
cache:set('foobar', 200)

handler(cache, 'foobar', { status = 403 })
handler(cache, 'foobar', 403)

assert.falsy(cache:get('foobar'))
end)
Expand All @@ -59,7 +59,7 @@ describe('Cache Handler', function()
local cache = lrucache.new(1)
cache:set('foobar', 200)

handler(cache, 'foobar', { status = 503 })
handler(cache, 'foobar', 503)

assert.falsy(cache:get('foobar'))
end)
Expand All @@ -71,23 +71,23 @@ describe('Cache Handler', function()
it('caches successful response', function()
local cache = lrucache.new(1)

handler(cache, 'foobar', { status = 200 })
handler(cache, 'foobar', 200)

assert.equal(200, cache:get('foobar'))
end)

it('caches forbidden response', function()
local cache = lrucache.new(1)

handler(cache, 'foobar', { status = 403 })
handler(cache, 'foobar', 403)

assert.equal(403, cache:get('foobar'))
end)

it('not caches server errors', function()
local cache = lrucache.new(1)

handler(cache, 'foobar', { status = 503 })
handler(cache, 'foobar', 503)

assert.falsy(cache:get('foobar'))
end)
Expand All @@ -97,7 +97,7 @@ describe('Cache Handler', function()

cache:set('foobar', 200)

handler(cache, 'foobar', { status = 503 })
handler(cache, 'foobar', 503)

assert.equal(200, cache:get('foobar'))
end)
Expand Down
14 changes: 7 additions & 7 deletions spec/policy/caching/policy_spec.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,23 +37,23 @@ describe('policy', function()

it('caches authorized requests', function()
caching_policy:access(ctx)
ctx.proxy.cache_handler(cache, 'a_key', { status = 200 }, nil)
ctx.proxy.cache_handler(cache, 'a_key', 200, nil)
assert.equals(200, cache:get('a_key'))
end)

it('clears the cache entry for a request when it is denied', function()
cache:set('a_key', 200)

caching_policy:access(ctx)
ctx.proxy.cache_handler(cache, 'a_key', { status = 403 }, nil)
ctx.proxy.cache_handler(cache, 'a_key', 403, nil)
assert.is_nil(cache:get('a_key'))
end)

it('clears the cache entry for a request when it fails', function()
cache:set('a_key', 200)

caching_policy:access(ctx)
ctx.proxy.cache_handler(cache, 'a_key', { status = 500 }, nil)
ctx.proxy.cache_handler(cache, 'a_key', 500, nil)
assert.is_nil(cache:get('a_key'))
end)
end)
Expand All @@ -71,21 +71,21 @@ describe('policy', function()

it('caches authorized requests', function()
caching_policy:access(ctx)
ctx.proxy.cache_handler(cache, 'a_key', { status = 200 }, nil)
ctx.proxy.cache_handler(cache, 'a_key', 200, nil)
assert.equals(200, cache:get('a_key'))
end)

it('caches denied requests', function()
caching_policy:access(ctx)
ctx.proxy.cache_handler(cache, 'a_key', { status = 403 }, nil)
ctx.proxy.cache_handler(cache, 'a_key', 403, nil)
assert.equals(403, cache:get('a_key'))
end)

it('does not clear the cache entry for a request when it fails', function()
cache:set('a_key', 200)

caching_policy:access(ctx)
ctx.proxy.cache_handler(cache, 'a_key', { status = 500 }, nil)
ctx.proxy.cache_handler(cache, 'a_key', 500, nil)
assert.equals(200, cache:get('a_key'))
end)
end)
Expand All @@ -103,7 +103,7 @@ describe('policy', function()

it('does not cache anything', function()
caching_policy:access(ctx)
ctx.proxy.cache_handler(cache, 'a_key', { status = 500 }, nil)
ctx.proxy.cache_handler(cache, 'a_key', 500, nil)
assert.is_nil(cache:get('a_key'))
end)
end)
Expand Down
4 changes: 2 additions & 2 deletions spec/proxy_spec.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ describe('Proxy', function()

proxy:authorize(service, { foo = 0 }, { client_id = 'blah' }, ttl)

assert.spy(proxy.cache_handler).was.called_with(proxy.cache, 'client_id=blah:foo=0', response, ttl)
assert.spy(proxy.cache_handler).was.called_with(proxy.cache, 'client_id=blah:foo=0', response.status, ttl)
end)

it('works with no ttl', function()
Expand All @@ -109,7 +109,7 @@ describe('Proxy', function()

proxy:authorize(service, { foo = 0 }, { client_id = 'blah' })

assert.spy(proxy.cache_handler).was.called_with(proxy.cache, 'client_id=blah:foo=0', response, nil)
assert.spy(proxy.cache_handler).was.called_with(proxy.cache, 'client_id=blah:foo=0', response.status, nil)
end)
end)

Expand Down

0 comments on commit 37f2f06

Please sign in to comment.