Allow to place the batcher policy after APIcast policy in the chain

CHANGELOG.md

@@ -10,7 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 - OpenTracing support [PR #669](https://github.com/3scale/apicast/pull/669), [THREESCALE-1159](https://issues.jboss.org/browse/THREESCALE-1159)
 - Generate new policy scaffold from the CLI [PR #682](https://github.com/3scale/apicast/pull/682)
-- 3scale batcher policy [PR #685](https://github.com/3scale/apicast/pull/685), [PR #710](https://github.com/3scale/apicast/pull/710), [PR #757](https://github.com/3scale/apicast/pull/757), [PR #786](https://github.com/3scale/apicast/pull/786), [THREESCALE-1155](https://issues.jboss.org/browse/THREESCALE-1155)
+- 3scale batcher policy [PR #685](https://github.com/3scale/apicast/pull/685), [PR #710](https://github.com/3scale/apicast/pull/710), [PR #757](https://github.com/3scale/apicast/pull/757), [PR #786](https://github.com/3scale/apicast/pull/786), [PR #823](https://github.com/3scale/apicast/pull/823), [THREESCALE-1155](https://issues.jboss.org/browse/THREESCALE-1155)
 - Liquid templating support in the headers policy configuration [PR #716](https://github.com/3scale/apicast/pull/716), [THREESCALE-1140](https://issues.jboss.org/browse/THREESCALE-1140)
 - Ability to modify query parameters in the URL rewriting policy [PR #724](https://github.com/3scale/apicast/pull/724), [PR #818](https://github.com/3scale/apicast/pull/818), [THREESCALE-1139](https://issues.jboss.org/browse/THREESCALE-1139)
 - 3scale referrer policy [PR #728](https://github.com/3scale/apicast/pull/728), [PR #777](https://github.com/3scale/apicast/pull/777), [THREESCALE-329](https://issues.jboss.org/browse/THREESCALE-329)

gateway/src/apicast/policy/3scale_batcher/3scale_batcher.lua

@@ -173,6 +173,15 @@ local function handle_backend_error(self, service, transaction, cache_handler)
   end
 end
 
+function _M.rewrite(_, context)
+  -- The APIcast policy reads these flags in the access() and post_action()
+  -- phases. That's why we need to set them before those phases. If we set
+  -- them in access() and placed the APIcast policy before the batcher in the
+  -- chain, APIcast would read the flags before the batcher set them, and both
+  -- policies would report to the 3scale backend.
+  set_flags_to_avoid_auths_in_apicast(context)
+end
+
 -- Note: when an entry in the cache expires, there might be several requests
 -- with those credentials and all of them will call auth() on backend with the
 -- same parameters until the auth status is cached again. In the future, we
@@ -211,8 +220,6 @@ function _M:access(context)
       return error(service, cached_auth.rejection_reason)
     end
   end
-
-  set_flags_to_avoid_auths_in_apicast(context)
 end
 
 return _M

spec/policy/3scale_batcher/3scale_batcher_spec.lua

@@ -27,6 +27,18 @@ describe('3scale batcher policy', function()
     end)
   end)
 
+  describe('.rewrite', function()
+    it('sets flags to avoid calling backend in the APIcast policy', function()
+      local context = {}
+      local batcher_policy = ThreescaleBatcher.new({})
+
+      batcher_policy:rewrite(context)
+
+      assert.is_true(context.skip_apicast_access)
+      assert.is_true(context.skip_apicast_post_action)
+    end)
+  end)
+
   describe('.access', function()
     local service = configuration.parse_service({ id = 42 })
     local service_id = service.id

t/apicast-policy-3scale-batcher.t

@@ -324,7 +324,77 @@ $res
 --- no_error_log
 [error]
 
-=== TEST 4: with caching policy (resilient mode)
+=== TEST 4: after apicast policy in the chain
+We want to check that only the batcher policy is reporting to backend. We know
+that the APIcast policy calls "/transactions/authrep.xml" whereas the batcher
+calls "/transactions/authorize.xml" and "/transactions.xml", because it
+authorizes and reports separately. Therefore, raising an error in
+"/transactions/authrep.xml" is enough to detect that the APIcast policy is
+calling backend when it's not supposed to.
+--- http_config
+include $TEST_NGINX_UPSTREAM_CONFIG;
+lua_shared_dict cached_auths 1m;
+lua_shared_dict batched_reports 1m;
+lua_shared_dict batched_reports_locks 1m;
+lua_package_path "$TEST_NGINX_LUA_PATH";
+init_by_lua_block {
+  require('apicast.configuration_loader').mock({
+    services = {
+      {
+        id = 1,
+        backend_version = 1,
+        backend_authentication_type = 'service_token',
+        backend_authentication_value = 'token-value',
+        proxy = {
+          backend = { endpoint = "http://127.0.0.1:$TEST_NGINX_SERVER_PORT" },
+          api_backend = "http://127.0.0.1:$TEST_NGINX_SERVER_PORT/api-backend/",
+          proxy_rules = {
+            { pattern = '/', http_method = 'GET', metric_system_name = 'hits', delta = 2 }
+          },
+          policy_chain = {
+            { name = 'apicast.policy.apicast' },
+            {
+              name = 'apicast.policy.3scale_batcher',
+              configuration = { batch_report_seconds = 1 }
+            }
+          }
+        }
+      }
+    }
+  })
+}
+--- config
+  include $TEST_NGINX_APICAST_CONFIG;
+
+  location /transactions/authrep.xml {
+    content_by_lua_block {
+      ngx.log(ngx.ERR, 'APIcast policy called authrep and it was not supposed to!')
+    }
+  }
+
+  location /transactions/authorize.xml {
+    content_by_lua_block {
+      ngx.exit(200)
+    }
+  }
+
+  location /api-backend {
+     echo 'yay, api backend';
+  }
+
+  location /transactions.xml {
+    content_by_lua_block {
+      ngx.exit(200)
+    }
+  }
+
+--- request
+GET /test?user_key=uk
+--- error_code: 200
+--- no_error_log
+[error]
+
+=== TEST 5: with caching policy (resilient mode)
 The purpose of this test is to test that the 3scale batcher policy works
 correctly when combined with the caching one.
 In this case, the caching policy is configured as "resilient". We define a