diff --git a/CHANGELOG.md b/CHANGELOG.md index cb55143ac..1d08ed6e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Fix proxy policy doesn't send headers set by APIcast to the API Backend. [PR #1588](https://github.com/3scale/APIcast/pull/1588) [THREESCALE-10151](https://redhat.atlassian.net/browse/THREESCALE-10151) - Ensure the synchronization key is released correctly. [PR #1591](https://github.com/3scale/APIcast/pull/1590) - Use new cpu.requests formula from Kubernetes. [PR #1595](https://github.com/3scale/APIcast/pull/1595) [THREESCALE-15465](https://redhat.atlassian.net/browse/THREESCALE-15465) +- Fix batcher policy fails silently when configured with string values instead of integers. [PR #1597](https://github.com/3scale/APIcast/pull/1597) [THREESCALE-15547](https://redhat.atlassian.net/browse/THREESCALE-15547) ### Added - Update APIcast schema manifest [PR #1550](https://github.com/3scale/APIcast/pull/1550) diff --git a/gateway/src/apicast/policy/3scale_batcher/3scale_batcher.lua b/gateway/src/apicast/policy/3scale_batcher/3scale_batcher.lua index 7b726c576..5d0b38d51 100644 --- a/gateway/src/apicast/policy/3scale_batcher/3scale_batcher.lua +++ b/gateway/src/apicast/policy/3scale_batcher/3scale_batcher.lua @@ -9,9 +9,10 @@ local Transaction = require('transaction') local http_ng_resty = require('resty.http_ng.backend.resty') local semaphore = require('ngx.semaphore') local TimerTask = require('resty.concurrent.timer_task') - local metrics = require('apicast.policy.3scale_batcher.metrics') +local tonumber = tonumber + local default_auths_ttl = 10 local default_batch_reports_seconds = 10 @@ -32,13 +33,13 @@ end function _M.new(config) local self = new(config) - local auths_ttl = config.auths_ttl or default_auths_ttl + local auths_ttl = tonumber(config.auths_ttl) or default_auths_ttl self.auths_cache = AuthsCache.new(ngx.shared.cached_auths, auths_ttl) self.reports_batcher = ReportsBatcher.new( ngx.shared.batched_reports, 'batched_reports_locks') - self.batch_reports_seconds = config.batch_report_seconds or + self.batch_reports_seconds = tonumber(config.batch_report_seconds) or default_batch_reports_seconds -- Semaphore used to ensure that only one TimerTask is started per worker. diff --git a/spec/policy/3scale_batcher/3scale_batcher_spec.lua b/spec/policy/3scale_batcher/3scale_batcher_spec.lua index c73dc192f..f43842d07 100644 --- a/spec/policy/3scale_batcher/3scale_batcher_spec.lua +++ b/spec/policy/3scale_batcher/3scale_batcher_spec.lua @@ -1,4 +1,5 @@ local ThreescaleBatcher = require('apicast.policy.3scale_batcher') +local policy_config_validator = require('apicast.policy_config_validator') local AuthsCache = require('apicast.policy.3scale_batcher.auths_cache') local Transaction = require('apicast.policy.3scale_batcher.transaction') local Usage = require('apicast.usage') @@ -26,6 +27,55 @@ describe('3scale batcher policy', function() assert.equals(10, batcher_policy.batch_reports_seconds) end) + + it('converts string batch_report_seconds to number', function() + stub(policy_config_validator, 'validate_config').returns(true) + local config = { batch_report_seconds = '5' } + local batcher_policy = ThreescaleBatcher.new(config) + + assert.equals(5, batcher_policy.batch_reports_seconds) + policy_config_validator.validate_config:revert() + end) + + it('uses default when batch_report_seconds is a non-numeric string', function() + stub(policy_config_validator, 'validate_config').returns(true) + local config = { batch_report_seconds = 'invalid' } + local batcher_policy = ThreescaleBatcher.new(config) + + assert.equals(10, batcher_policy.batch_reports_seconds) + policy_config_validator.validate_config:revert() + end) + + it('allows to configure the auths TTL', function() + local config = { auths_ttl = 30 } + local batcher_policy = ThreescaleBatcher.new(config) + + assert.equals(30, batcher_policy.auths_cache.ttl) + end) + + it('assigns a default of 10s for the auths TTL', function() + local batcher_policy = ThreescaleBatcher.new({}) + + assert.equals(10, batcher_policy.auths_cache.ttl) + end) + + it('converts string auths_ttl to number', function() + stub(policy_config_validator, 'validate_config').returns(true) + local config = { auths_ttl = '20' } + local batcher_policy = ThreescaleBatcher.new(config) + + assert.equals(20, batcher_policy.auths_cache.ttl) + policy_config_validator.validate_config:revert() + end) + + it('uses default when auths_ttl is a non-numeric string', function() + stub(policy_config_validator, 'validate_config').returns(true) + local config = { auths_ttl = 'invalid' } + local batcher_policy = ThreescaleBatcher.new(config) + + assert.equals(10, batcher_policy.auths_cache.ttl) + policy_config_validator.validate_config:revert() + end) end) describe('.rewrite', function()