Move mapping rule checking to MappingRule and add missing tests

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
 

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

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)