Pathrouting fix

CHANGELOG.md

@@ -11,7 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 - Fixed issues with liquid replaces [THREESCALE-4937](https://issues.jboss.org/browse/THREESCALE-4937) [PR #1185](https://github.com/3scale/APIcast/pull/1185)
 - Fixed issues with HTTPS_PROXY and large bodies [THREESCALE-3863](https://issues.jboss.org/browse/THREESCALE-3863) [PR #1191](https://github.com/3scale/APIcast/pull/1191)
-
+- Fixed issues with path routing and query args [THREESCALE-5149](https://issues.redhat.com/browse/THREESCALE-5149) [PR #1190](https://github.com/3scale/APIcast/pull/1190)
 
 ### Added
 

gateway/src/apicast/policy/find_service/path_based_finder.lua

@@ -8,6 +8,7 @@ function _M.find_service(config_store, host)
   local services = config_store:find_by_host(host)
   local method = ngx.req.get_method()
   local uri = escape.escape_uri(ngx.var.uri)
+  local args = ngx.req.get_uri_args()
 
   for s=1, #services do
     local service = services[s]
@@ -17,7 +18,7 @@ function _M.find_service(config_store, host)
       if hosts[h] == host then
         local name = service.system_name or service.id
         ngx.log(ngx.DEBUG, 'service ', name, ' matched host ', hosts[h])
-        local matches = mapping_rules_matcher.matches(method, uri, {}, service.rules)
+        local matches = mapping_rules_matcher.matches(method, uri, args, service.rules)
         -- matches() also returns the index of the first rule that matched.
         -- As a future optimization, in the part of the code that calculates
         -- the usage, we could use this to avoid trying to match again all the

spec/policy/find_service/path_based_finder_spec.lua

@@ -100,5 +100,77 @@ describe('PathBasedFinder', function()
 
       assert.is_nil(service_found)
     end)
+
+    it('returns the service if it matches the path, the host and the querystring params', function()
+      ngx.var = { uri = '/abc' }
+      stub(ngx.req, 'get_uri_args', function() return { foo = 'bar' } end)
+      stub(ngx.req, 'get_method', function() return 'GET' end)
+
+      local host = 'example.com'
+
+      local service_not_matching = Configuration.parse_service({
+        id = 1,
+        proxy = {
+          hosts = { host },
+          proxy_rules = { { pattern = '/dont_match', http_method = 'GET',
+                            metric_system_name = 'hits', delta = 2 } }
+        }
+      })
+
+      local service_matching = Configuration.parse_service({
+        id = 2,
+        proxy = {
+          hosts = { host },
+          proxy_rules = { { pattern = '/abc',
+                            querystring_parameters = { foo = '{bar}' },
+                            http_method = 'GET', metric_system_name = 'hits',
+                            delta = 2 } }
+        }
+      })
+
+      local services = { service_not_matching, service_matching }
+      local config_store = ConfigurationStore.new(nil, config_store_opts)
+      config_store:store({ services = services })
+
+      local service_found = PathBasedFinder.find_service(config_store, host)
+
+      assert.equal(service_matching, service_found)
+    end)
+
+    it('returns nil if it matches the path, the host but not the querystring params', function()
+      ngx.var = { uri = '/abc' }
+      stub(ngx.req, 'get_uri_args', function() return { } end)
+      stub(ngx.req, 'get_method', function() return 'GET' end)
+
+      local host = 'example.com'
+
+      local service_not_matching = Configuration.parse_service({
+        id = 1,
+        proxy = {
+          hosts = { host },
+          proxy_rules = { { pattern = '/dont_match', http_method = 'GET',
+                            metric_system_name = 'hits', delta = 2 } }
+        }
+      })
+
+      local service_matching = Configuration.parse_service({
+        id = 2,
+        proxy = {
+          hosts = { host },
+          proxy_rules = { { pattern = '/abc',
+                            querystring_parameters = { foo = 'bar' },
+                            http_method = 'GET', metric_system_name = 'hits',
+                            delta = 2 } }
+        }
+      })
+
+      local services = { service_not_matching, service_matching }
+      local config_store = ConfigurationStore.new(nil, config_store_opts)
+      config_store:store({ services = services })
+
+      local service_found = PathBasedFinder.find_service(config_store, host)
+
+      assert.is_nil(service_found)
+    end)
   end)
 end)

t/apicast-path-routing.t

@@ -294,3 +294,79 @@ Host: one
 [200, 200, 200]
 --- no_error_log
 [error]
+
+=== TEST 5: multi service configuration with path based routing and query args in mapping rules
+--- env eval
+('APICAST_PATH_ROUTING' => '1')
+--- configuration
+{
+  "services": [
+    {
+      "id": 42,
+      "backend_version": 2,
+      "proxy": {
+        "api_backend": "http://test:$TEST_NGINX_SERVER_PORT/api-backend/foo/",
+        "hosts": [
+          "same"
+        ],
+        "backend_authentication_type": "service_token",
+        "backend_authentication_value": "service-one",
+        "proxy_rules": [
+          {
+            "pattern": "/one/test",
+            "http_method": "GET",
+            "metric_system_name": "one",
+            "delta": 1
+          }
+        ]
+      }
+    },
+    {
+      "id": 21,
+      "backend_version": 2,
+      "proxy": {
+        "api_backend": "http://test:$TEST_NGINX_SERVER_PORT/api-backend/bar/",
+        "hosts": [
+          "same"
+        ],
+        "backend_authentication_type": "service_token",
+        "backend_authentication_value": "service-two",
+        "proxy_rules": [
+          {
+            "pattern": "/two/test?foo={bar}",
+            "http_method": "GET",
+            "metric_system_name": "two",
+            "delta": 2,
+            "querystring_parameters": {
+                "foo": "{bar}"
+            }
+          }
+        ]
+      }
+    }
+  ]
+}
+--- backend
+  location /transactions/authrep.xml {
+    content_by_lua_block { ngx.exit(200) }
+  }
+--- upstream
+  location ~ /api-backend(/.+) {
+     echo 'yay, api backend: $1';
+  }
+--- request eval
+[
+  "GET /two/test?app_id=two-id&app_key=two-key&foo=bar",
+  "GET /two/test?app_id=two-id&app_key=two-key"
+]
+--- more_headers eval
+["Host: same", "Host: same"]
+--- response_body eval
+[
+  "yay, api backend: /bar/two/test\x{0a}",
+  "No Mapping Rule matched"
+]
+--- error_code eval
+[200, 404]
+--- no_error_log
+[error]