Skip to content

Commit c87a510

Browse files
authored
Merge pull request #843 from Hitachi/count-inc-decr
[count-inc] Add decrement logic to the incoming function
2 parents df259c6 + c4231f0 commit c87a510

File tree

4 files changed

+167
-4
lines changed

4 files changed

+167
-4
lines changed

CHANGELOG.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
6969
- The `scope` of the Rate Limit policy is `service` by default [PR #704](https://github.com/3scale/apicast/pull/704)
7070
- Decoded JWTs are now exposed in the policies context by the APIcast policy [PR #718](https://github.com/3scale/apicast/pull/718)
7171
- Upgraded OpenResty to 1.13.6.2, uses OpenSSL 1.1 [PR #733](https://github.com/3scale/apicast/pull/733)
72-
- Use forked `resty.limit.count` that uses increments instead of decrements [PR #758](https://github.com/3scale/apicast/pull/758)
72+
- Use forked `resty.limit.count` that uses increments instead of decrements [PR #758](https://github.com/3scale/apicast/pull/758), [PR 843](https://github.com/3scale/apicast/pull/843)
7373
- Rate Limit policy to take into account changes in the config [PR #703](https://github.com/3scale/apicast/pull/703)
7474
- The regular expression for mapping rules has been changed, so that special characters are accepted in the wildcard values for path [PR #714](https://github.com/3scale/apicast/pull/714)
7575
- Call `init` and `init_worker` on all available policies regardless they are used or not [PR #770](https://github.com/3scale/apicast/pull/770)

gateway/src/resty/limit/count-inc.lua

+11
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,17 @@ function _M.incoming(self, key, commit)
3333
if not count then
3434
return nil, err
3535
end
36+
37+
if count > limit then
38+
count, err = dict:incr(key, -1)
39+
40+
if not count then
41+
return nil, err
42+
end
43+
44+
return nil, "rejected"
45+
end
46+
3647
else
3748
count = (dict:get(key) or 0) + 1
3849
end

spec/policy/rate_limit/rate_limit_spec.lua

+3-3
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ describe('Rate limit policy', function()
194194
assert(rate_limit_policy:access(context))
195195
assert.returns_error('limits exceeded', rate_limit_policy:access(context))
196196

197-
assert.equal('2', redis:get('11110_fixed_window_test3'))
197+
assert.equal('1', redis:get('11110_fixed_window_test3'))
198198
assert.spy(ngx.exit).was_called_with(429)
199199
end)
200200

@@ -211,7 +211,7 @@ describe('Rate limit policy', function()
211211
assert(rate_limit_policy:access(ctx))
212212
assert.returns_error('limits exceeded', rate_limit_policy:access(ctx))
213213

214-
assert.equal('2', redis:get('11110_fixed_window_test3'))
214+
assert.equal('1', redis:get('11110_fixed_window_test3'))
215215
assert.spy(ngx.exit).was_called_with(429)
216216
end)
217217

@@ -232,7 +232,7 @@ describe('Rate limit policy', function()
232232
assert(rate_limit_policy:access(context))
233233
assert.returns_error('limits exceeded', rate_limit_policy:access(context))
234234

235-
assert.equal('2', redis:get('11110_fixed_window_' .. test_host))
235+
assert.equal('1', redis:get('11110_fixed_window_' .. test_host))
236236
assert.spy(ngx.exit).was_called_with(429)
237237
end)
238238

spec/resty/limit/count-inc_spec.lua

+152
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
local _M = require 'resty.limit.count-inc'
2+
3+
local shdict_mt = {
4+
__index = {
5+
get = function(t, k) return rawget(t, k) end,
6+
set = function(t, k, v) rawset(t, k , v); return true end,
7+
incr = function(t, key, inc, init, _)
8+
local value = t:get(key) or init
9+
if not value then return nil, 'not found' end
10+
11+
t:set(key, value + inc)
12+
return t:get(key)
13+
end,
14+
}
15+
}
16+
local function shdict()
17+
return setmetatable({ }, shdict_mt)
18+
end
19+
20+
describe('resty.limit.count-inc', function()
21+
22+
before_each(function()
23+
ngx.shared.limiter = mock(shdict(), true)
24+
end)
25+
26+
describe('.new', function()
27+
describe('using correct shdict', function()
28+
it('returns limiter', function()
29+
local lim = _M.new('limiter', 1, 1)
30+
assert.same(1, lim.limit)
31+
assert.same(1, lim.window)
32+
end)
33+
end)
34+
35+
describe('using incorrect shdict', function()
36+
it('returns error', function()
37+
local _, err = _M.new('incorrect', 1, 1)
38+
assert.same('shared dict not found', err)
39+
end)
40+
end)
41+
end)
42+
43+
describe('.incoming', function()
44+
local lim
45+
46+
before_each(function()
47+
lim = _M.new('limiter', 1, 1)
48+
end)
49+
50+
describe('when commit is true', function()
51+
describe('if count is less than limit', function()
52+
it('returns zero and remaining', function()
53+
local delay, err = lim:incoming('tmp', true)
54+
assert.same(0, delay)
55+
assert.same(0, err)
56+
end)
57+
58+
describe('and incr method fails', function()
59+
it('returns error', function()
60+
ngx.shared.limiter.incr = function()
61+
return nil, 'something'
62+
end
63+
local delay, err = lim:incoming('tmp', true)
64+
assert.is_nil(delay)
65+
assert.same('something', err)
66+
end)
67+
end)
68+
end)
69+
70+
describe('if count is greater than limit', function()
71+
it('return rejected', function()
72+
lim:incoming('tmp', true)
73+
local delay, err = lim:incoming('tmp', true)
74+
assert.is_nil(delay)
75+
assert.same('rejected', err)
76+
end)
77+
78+
describe('and incr method fails', function()
79+
it('returns error', function()
80+
ngx.shared.limiter.incr = function(t, key, inc, init, _)
81+
local value = t:get(key) or init
82+
if not value then return nil, 'not found' end
83+
if inc == -1 then return nil, 'something' end
84+
t:set(key, value + inc)
85+
return t:get(key)
86+
end
87+
lim:incoming('tmp', true)
88+
local delay, err = lim:incoming('tmp', true)
89+
assert.is_nil(delay)
90+
assert.same('something', err)
91+
end)
92+
end)
93+
end)
94+
end)
95+
96+
describe('when commit is false', function()
97+
describe('if count is less than limit', function()
98+
it('returns zero and remaining', function()
99+
local delay, err = lim:incoming('tmp', false)
100+
assert.same(0, delay)
101+
assert.same(0, err)
102+
end)
103+
end)
104+
105+
describe('if count is greater than limit', function()
106+
it('return rejected', function()
107+
lim:incoming('tmp', true)
108+
local delay, err = lim:incoming('tmp', false)
109+
assert.is_nil(delay)
110+
assert.same('rejected', err)
111+
end)
112+
end)
113+
end)
114+
115+
end)
116+
117+
describe('.uncommit', function()
118+
local lim
119+
120+
before_each(function()
121+
lim = _M.new('limiter', 1, 1)
122+
end)
123+
124+
describe('when incr method succeeds', function()
125+
it('returns remaining', function()
126+
lim:incoming('tmp', true)
127+
local delay = lim:uncommit('tmp')
128+
assert.same(1, delay)
129+
end)
130+
end)
131+
132+
describe('when incr method fails', function()
133+
describe('if key is not found', function()
134+
it('returns remaining', function()
135+
local delay = lim:uncommit('tmp')
136+
assert.same(1, delay)
137+
end)
138+
end)
139+
140+
it('returns error', function()
141+
lim:incoming('tmp', true)
142+
ngx.shared.limiter.incr = function()
143+
return nil, 'something'
144+
end
145+
local delay, err = lim:uncommit('tmp')
146+
assert.is_nil(delay)
147+
assert.same('something', err)
148+
end)
149+
end)
150+
end)
151+
152+
end)

0 commit comments

Comments
 (0)