Policy: rate-limit headers, address PR feedback

Signed-off-by: Eloy Coto <eloy.coto@acalustra.com>

gateway/src/apicast/policy/rate_limit_headers/cache_entry.lua

@@ -7,6 +7,8 @@ local stringx = require 'pl.stringx'
 local _M = {}
 local mt = { __index = _M }
 
+local export_separator = "#"
+
 function _M.new(usage, max, remaining, reset)
   local self = setmetatable({}, mt)
 
@@ -43,9 +45,9 @@ function _M:reset(max, remaining, reset)
 end
 
 function _M:export()
-  return string.format("%s#%s#%s",
-    self.limit:__tostring(),
-    self.remaining:__tostring(),
+  return string.format("%s%s%s%s%s",
+    self.limit:__tostring(), export_separator,
+    self.remaining:__tostring(), export_separator,
     self.reset:remaining_secs_positive(now()))
 end
 
@@ -58,7 +60,7 @@ function _M.import(usage, exported_data)
     return _M.Init_empty(usage)
   end
 
-  local data = stringx.split(exported_data, "#")
+  local data = stringx.split(exported_data, export_separator)
   return _M.new(usage, data[1] or 0, data[2] or 0, data[3] or 0)
 end
 

spec/policy/custom_metrics/custom_metrics_spec.lua

@@ -1,6 +1,5 @@
 local custom_metrics =  require("apicast.policy.custom_metrics")
 local ngx_variable = require 'apicast.policy.ngx_variable'
-local backend_client = require('apicast.backend_client')
 local Usage = require('apicast.usage')
 
 
@@ -12,7 +11,7 @@ describe('custom metrics policy', function()
     ngx.var = {}
     ngx.header = {}
 
-    stub(ngx_variable, 'available_context', function(context) return context end)
+    stub(ngx_variable, 'available_context', function(ctx) return ctx end)
     stub(ngx.req, 'get_headers', function() return {} end)
     stub(ngx.resp, 'get_headers', function() return {} end)
 
@@ -23,7 +22,8 @@ describe('custom metrics policy', function()
     usage:add("foo", 1)
   end)
 
-  it('if Auth cache is disabled, report the usage', function()
+  describe('if Auth cache is disabled', function()
+    it('reports the usage', function()
       local config = {
         rules = {
           {
@@ -41,39 +41,42 @@ describe('custom metrics policy', function()
 
       local policy = custom_metrics.new(config)
 
-      stub(policy, "report", function(context, usage) return end)
+      stub(policy, "report", function(_, _) return end)
 
       policy:post_action(context)
       assert.spy(policy.report).was_called()
       assert.spy(policy.report).was.called_with(context, usage)
+    end)
   end)
 
-  it('if Auth cache is enabled, usage is incremented', function()
-    ngx.var.cached_key = true
-
-    local config = {
-      rules = {
-        {
-          metric = "foo",
-          increment = "1",
-          condition = {
-            combine_op = "and",
-            operations = {
-              { left = "foo", op = "==", right = "foo" }
+  describe('if Auth cache is enabled', function()
+    it('usage is incremented', function()
+      ngx.var.cached_key = true
+
+      local config = {
+        rules = {
+          {
+            metric = "foo",
+            increment = "1",
+            condition = {
+              combine_op = "and",
+              operations = {
+                { left = "foo", op = "==", right = "foo" }
+              }
             }
           }
         }
       }
-    }
-
-    local policy = custom_metrics.new(config)
 
-    stub(policy, "report", function(context, usage) return end)
-    policy:post_action(context)
-    assert.spy(policy.report).was_not_called()
-    assert.spy(context.usage.merge).was_called()
+      local policy = custom_metrics.new(config)
 
-    assert.spy(context.usage.merge).was_called_with(context.usage, usage)
+      stub(policy, "report", function(_, _) return end)
+      policy:post_action(context)
+      assert.spy(policy.report).was_not_called()
+      assert.spy(context.usage.merge).was_called()
 
+      assert.spy(context.usage.merge).was_called_with(context.usage, usage)
+    end)
   end)
+
 end)

spec/policy/rate_limit_headers/cache_entry_spec.lua

@@ -1,12 +1,16 @@
 local Usage = require('apicast.usage')
-local cache_entry = require("apicast.policy.rate_limit_headers.cache_entry")
+
 
 describe("Cache key", function()
   local usage
+  local cache_entry
 
   before_each(function()
     usage = Usage.new()
     usage:add("a", 3)
+    stub(ngx, 'now', function() return 100 end)
+    -- Imported here to be able to use stub ngx.now()
+    cache_entry = require("apicast.policy.rate_limit_headers.cache_entry")
   end)
 
   it("New works as expected", function()
@@ -32,10 +36,10 @@ describe("Cache key", function()
 
       local entry = cache_entry.new(usage, 1, 10, 3)
       assert.same(entry:export(), "1#10#3")
-
       local data = cache_entry.import(usage, entry:export())
       assert.same(data.limit:__tostring(), "1")
       assert.same(data.remaining:__tostring(), "10")
+      assert.same(data.reset:__tostring(), "103")
     end)
 
 
@@ -47,12 +51,14 @@ describe("Cache key", function()
       local data = cache_entry.import(usage, "1#asd#123")
       assert.same(data.limit:__tostring(), "1")
       assert.same(data.remaining:__tostring(), "0")
+      assert.same(data.reset:__tostring(), "223")
     end)
 
     it("no raw_data return empty data", function()
       local data = cache_entry.import(usage, "")
       assert.same(data.limit:__tostring(), "0")
       assert.same(data.remaining:__tostring(), "0")
+      assert.same(data.reset:__tostring(), "100")
     end)
 
   end)