diff --git a/gateway/src/apicast/configuration/service.lua b/gateway/src/apicast/configuration/service.lua index 35b4d61c5..e5ef8d90c 100644 --- a/gateway/src/apicast/configuration/service.lua +++ b/gateway/src/apicast/configuration/service.lua @@ -7,12 +7,10 @@ local tostring = tostring local rawget = rawget local lower = string.lower local gsub = string.gsub -local format = string.format local select = select local concat = table.concat local insert = table.insert local re = require 'ngx.re' -local re_match = ngx.re.match local http_authorization = require 'resty.http_authorization' @@ -170,27 +168,13 @@ local function set_or_inc(t, name, delta) end local function check_rule(req, rule, usage_t, matched_rules, params) - local pattern = rule.regexpified_pattern - local match = re_match(req.path, format("^%s", pattern), 'oj') - - if match and req.method == rule.method then - local args = req.args - - if rule.querystring_params(args) then -- may return an empty table - local system_name = rule.system_name - -- FIXME: this had no effect, what is it supposed to do? - -- when no querystringparams - -- in the rule. it's fine - -- for i,p in ipairs(rule.parameters or {}) do - -- param[p] = match[i] - -- end - - local value = set_or_inc(usage_t, system_name, rule.delta) - - usage_t[system_name] = value - params['usage[' .. system_name .. ']'] = value - insert(matched_rules, rule.pattern) - end + if rule:matches(req.method, req.path, req.args) then + local system_name = rule.system_name + local value = set_or_inc(usage_t, system_name, rule.delta) + + usage_t[system_name] = value + params['usage[' .. system_name .. ']'] = value + insert(matched_rules, rule.pattern) end end diff --git a/gateway/src/apicast/mapping_rule.lua b/gateway/src/apicast/mapping_rule.lua index 3401d015a..9a175dec3 100644 --- a/gateway/src/apicast/mapping_rule.lua +++ b/gateway/src/apicast/mapping_rule.lua @@ -8,6 +8,7 @@ local setmetatable = setmetatable local pairs = pairs local error = error local type = type +local format = string.format local re_match = ngx.re.match local insert = table.insert @@ -25,13 +26,13 @@ local function hash_to_array(hash) return array end -local function regexpify(path) - return path:gsub('?.*', ''):gsub("{.-}", '([\\w_.-]+)'):gsub("%.", "\\.") +local function regexpify(pattern) + return pattern:gsub('?.*', ''):gsub("{.-}", '([\\w_.-]+)'):gsub("%.", "\\.") end local regex_variable = '\\{[-\\w_]+\\}' -local function check_querystring_params(params, args) +local function matches_querystring_params(params, args) local match = true for i=1, #params do @@ -67,6 +68,10 @@ local function check_querystring_params(params, args) return match end +local function matches_uri(rule_pattern, uri) + return re_match(uri, format("^%s", rule_pattern), 'oj') +end + local function new(http_method, pattern, params, querystring_params, metric, delta) local self = setmetatable({}, mt) @@ -80,7 +85,7 @@ local function new(http_method, pattern, params, querystring_params, metric, del self.delta = delta self.querystring_params = function(args) - return check_querystring_params(querystring_parameters, args) + return matches_querystring_params(querystring_parameters, args) end return self @@ -109,4 +114,19 @@ function _M.from_proxy_rule(proxy_rule) ) end +--- Checks if the mapping rule matches a given request method, URI, and args +-- +-- @tparam string method HTTP method (GET, POST, etc.). +-- @tparam string uri URI of an HTTP request. +-- @tparam table args Table with the args and values of an HTTP request. +-- @treturn boolean Whether the mapping rule matches the given request. +function _M:matches(method, uri, args) + local match = self.method == method and + matches_uri(self.regexpified_pattern, uri) and + self.querystring_params(args) + + -- match can be nil. Convert to boolean. + return match == true +end + return _M diff --git a/spec/mapping_rule_spec.lua b/spec/mapping_rule_spec.lua new file mode 100644 index 000000000..f95aba75e --- /dev/null +++ b/spec/mapping_rule_spec.lua @@ -0,0 +1,83 @@ +local MappingRule = require('apicast.mapping_rule') + +describe('mapping_rule', function() + describe('.matches', function() + it('returns true when method, URI, and args match', function() + local mapping_rule = MappingRule.from_proxy_rule({ + http_method = 'GET', + pattern = '/abc', + querystring_parameters = { a_param = '1' }, + metric_system_name = 'hits', + delta = 1 + }) + + local match = mapping_rule:matches('GET', '/abc', { a_param = '1' }) + assert.is_true(match) + end) + + it('returns true when method and URI match, and no args are required', function() + local mapping_rule = MappingRule.from_proxy_rule({ + http_method = 'GET', + pattern = '/abc', + querystring_parameters = { }, + metric_system_name = 'hits', + delta = 1 + }) + + local match = mapping_rule:matches('GET', '/abc', { a_param = '1' }) + assert.is_true(match) + end) + + it('returns false when the method does not match', function() + local mapping_rule = MappingRule.from_proxy_rule({ + http_method = 'GET', + pattern = '/abc', + querystring_parameters = { a_param = '1' }, + metric_system_name = 'hits', + delta = 1 + }) + + local match = mapping_rule:matches('POST', '/abc', { a_param = '1' }) + assert.is_false(match) + end) + + it('returns false when the URI does not match', function() + local mapping_rule = MappingRule.from_proxy_rule({ + http_method = 'GET', + pattern = '/abc', + querystring_parameters = { a_param = '1' }, + metric_system_name = 'hits', + delta = 1 + }) + + local match = mapping_rule:matches('GET', '/aaa', { a_param = '1' }) + assert.is_false(match) + end) + + it('returns false when the args do not match', function() + local mapping_rule = MappingRule.from_proxy_rule({ + http_method = 'GET', + pattern = '/abc', + querystring_parameters = { a_param = '1' }, + metric_system_name = 'hits', + delta = 1 + }) + + local match = mapping_rule:matches('GET', '/abc', { a_param = '2' }) + assert.is_false(match) + end) + + it('returns false when method, URI, and args do not match', function() + local mapping_rule = MappingRule.from_proxy_rule({ + http_method = 'GET', + pattern = '/abc', + querystring_parameters = { a_param = '1' }, + metric_system_name = 'hits', + delta = 1 + }) + + local match = mapping_rule:matches('POST', '/def', { x = 'y' }) + assert.is_false(match) + end) + end) +end)