From 93f018f2f15355ce9dc11946386df85913bb24e4 Mon Sep 17 00:00:00 2001 From: guscarreon Date: Wed, 13 Apr 2022 13:18:31 -0400 Subject: [PATCH] Zero TTL fix (#107) * Defined TTL constants and wrote decorator test case * Decorator logic modified and asserted in test case * Added test coverage to backends/config/config.go * Brian's review * Corrected Aerospike error message and assert.Contains Co-authored-by: Gus Carreon --- backends/aerospike.go | 2 +- backends/aerospike_test.go | 7 +- backends/config/config.go | 9 +- backends/config/config_test.go | 329 +++++++++++++++++++++++++ backends/constants.go | 7 - backends/decorators/limit_ttls.go | 27 +- backends/decorators/limit_ttls_test.go | 110 ++++++++- backends/decorators/metrics.go | 4 +- backends/redis.go | 1 + config/backends.go | 16 +- config/config.go | 14 +- config/config_test.go | 16 +- endpoints/get.go | 8 +- endpoints/put.go | 4 +- utils/constants.go | 17 ++ 15 files changed, 509 insertions(+), 62 deletions(-) create mode 100644 backends/config/config_test.go delete mode 100644 backends/constants.go create mode 100644 utils/constants.go diff --git a/backends/aerospike.go b/backends/aerospike.go index 02cb43ed..8be235aa 100644 --- a/backends/aerospike.go +++ b/backends/aerospike.go @@ -71,7 +71,7 @@ func NewAerospikeBackend(cfg config.Aerospike, metrics *metrics.Metrics) *Aerosp client, err := as.NewClientWithPolicyAndHost(clientPolicy, hosts...) if err != nil { - log.Fatalf("%v", classifyAerospikeError(err).Error()) + log.Fatalf("Error creating Aerospike backend: %s", classifyAerospikeError(err).Error()) panic("AerospikeBackend failure. This shouldn't happen.") } log.Infof("Connected to Aerospike host(s) %v on port %d", append(cfg.Hosts, cfg.Host), cfg.Port) diff --git a/backends/aerospike_test.go b/backends/aerospike_test.go index 0b7d8e96..76fc1ad7 100644 --- a/backends/aerospike_test.go +++ b/backends/aerospike_test.go @@ -36,9 +36,8 @@ func TestNewAerospikeBackend(t *testing.T) { }, expectPanic: true, expectedLogEntries: []logEntry{ - { - msg: "Failed to connect to host(s): [foo.com:8888 bat.com:8888]; error: Connecting to the cluster timed out.", + msg: "Error creating Aerospike backend: Failed to connect to host(s): [foo.com:8888 bat.com:8888]; error: Connecting to the cluster timed out.", lvl: logrus.FatalLevel, }, }, @@ -57,7 +56,7 @@ func TestNewAerospikeBackend(t *testing.T) { lvl: logrus.InfoLevel, }, { - msg: "Failed to connect to host(s): [fakeTestUrl.foo:8888 foo.com:8888 bat.com:8888]; error: Connecting to the cluster timed out.", + msg: "Error creating Aerospike backend: Failed to connect to host(s): [fakeTestUrl.foo:8888 foo.com:8888 bat.com:8888]; error: Connecting to the cluster timed out.", lvl: logrus.FatalLevel, }, }, @@ -75,7 +74,7 @@ func TestNewAerospikeBackend(t *testing.T) { lvl: logrus.InfoLevel, }, { - msg: "Failed to connect to host(s): [fakeTestUrl.foo:8888]; error: Connecting to the cluster timed out.", + msg: "Error creating Aerospike backend: Failed to connect to host(s): [fakeTestUrl.foo:8888]; error: Connecting to the cluster timed out.", lvl: logrus.FatalLevel, }, }, diff --git a/backends/config/config.go b/backends/config/config.go index fe102b0b..77a60dac 100644 --- a/backends/config/config.go +++ b/backends/config/config.go @@ -8,6 +8,7 @@ import ( "github.com/prebid/prebid-cache/compression" "github.com/prebid/prebid-cache/config" "github.com/prebid/prebid-cache/metrics" + "github.com/prebid/prebid-cache/utils" ) func NewBackend(cfg config.Configuration, appMetrics *metrics.Metrics) backends.Backend { @@ -71,8 +72,8 @@ func getMaxTTLSeconds(cfg config.Configuration) int { case config.BackendCassandra: // If config.request_limits.max_ttl_seconds was defined to be less than 2400 seconds, go // with 2400 as it has been the TTL limit hardcoded in the Cassandra backend so far. - if maxTTLSeconds > 2400 { - maxTTLSeconds = 2400 + if maxTTLSeconds > utils.CASSANDRA_DEFAULT_TTL_SECONDS { + maxTTLSeconds = utils.CASSANDRA_DEFAULT_TTL_SECONDS } case config.BackendAerospike: // If both config.request_limits.max_ttl_seconds and config.backend.aerospike.default_ttl_seconds @@ -83,8 +84,8 @@ func getMaxTTLSeconds(cfg config.Configuration) int { case config.BackendRedis: // If both config.request_limits.max_ttl_seconds and backend.redis.expiration // were defined, the smallest value takes preference - if cfg.Backend.Redis.Expiration > 0 && maxTTLSeconds > cfg.Backend.Redis.Expiration*60 { - maxTTLSeconds = cfg.Backend.Redis.Expiration * 60 + if cfg.Backend.Redis.ExpirationMinutes > 0 && maxTTLSeconds > cfg.Backend.Redis.ExpirationMinutes*60 { + maxTTLSeconds = cfg.Backend.Redis.ExpirationMinutes * 60 } } return maxTTLSeconds diff --git a/backends/config/config_test.go b/backends/config/config_test.go new file mode 100644 index 00000000..2c404b30 --- /dev/null +++ b/backends/config/config_test.go @@ -0,0 +1,329 @@ +package config + +import ( + "context" + "testing" + + "github.com/prebid/prebid-cache/backends" + "github.com/prebid/prebid-cache/compression" + "github.com/prebid/prebid-cache/config" + "github.com/prebid/prebid-cache/metrics/metricstest" + "github.com/prebid/prebid-cache/utils" + + "github.com/sirupsen/logrus" + logrusTest "github.com/sirupsen/logrus/hooks/test" + "github.com/stretchr/testify/assert" +) + +func TestApplyCompression(t *testing.T) { + testCases := []struct { + desc string + inConfig config.Compression + expectedBackendType backends.Backend + }{ + { + desc: "Compression type none, expect the default fakeBackend", + inConfig: config.Compression{ + Type: config.CompressionNone, + }, + expectedBackendType: &fakeBackend{}, + }, + { + desc: "Compression type snappy, expect the the backend to be a snappyCompressor backend", + inConfig: config.Compression{ + Type: config.CompressionSnappy, + }, + expectedBackendType: compression.SnappyCompress(&fakeBackend{}), + }, + } + + for _, tc := range testCases { + // set test + sampleBackend := &fakeBackend{} + + // run + actualBackend := applyCompression(tc.inConfig, sampleBackend) + + // assertions + assert.IsType(t, tc.expectedBackendType, actualBackend, tc.desc) + } +} + +func TestApplyUnknownCompression(t *testing.T) { + // logrus entries will be recorded to this `hook` object so we can compare and assert them + hook := logrusTest.NewGlobal() + defer func() { logrus.StandardLogger().ExitFunc = nil }() + logrus.StandardLogger().ExitFunc = func(int) {} + + // Input and expected values + inConfig := config.Compression{Type: "unknown"} + expectedLogMessage := "Unknown compression type: unknown" + expectedLogLevel := logrus.FatalLevel + + // run and assert it panics + panicTestFunction := func() { + applyCompression(inConfig, &fakeBackend{}) + } + assert.Panics(t, panicTestFunction, "Unknown compression type should have made applyCompression to panic") + + // assertions + assert.Equal(t, expectedLogMessage, hook.Entries[0].Message, "Expected log message not found") + assert.Equal(t, expectedLogLevel, hook.Entries[0].Level, "Unexpected log level") +} + +func TestNewMemoryOrMemcacheBackend(t *testing.T) { + testCases := []struct { + desc string + inConfig config.Backend + expectedBackend backends.Backend + }{ + { + desc: "Memory", + inConfig: config.Backend{Type: config.BackendMemory}, + expectedBackend: backends.NewMemoryBackend(), + }, + { + desc: "Memcache", + inConfig: config.Backend{Type: config.BackendMemcache}, + expectedBackend: &backends.MemcacheBackend{}, + }, + } + + for _, tc := range testCases { + // run + actualBackend := newBaseBackend(tc.inConfig, metricstest.CreateMockMetrics()) + + // assertions + assert.IsType(t, tc.expectedBackend, actualBackend, tc.desc) + } + +} + +func TestNewBaseBackend(t *testing.T) { + // logrus entries will be recorded to this `hook` object so we can compare and assert them + hook := logrusTest.NewGlobal() + defer func() { logrus.StandardLogger().ExitFunc = nil }() + logrus.StandardLogger().ExitFunc = func(int) {} + + type logEntry struct { + msg string + lvl logrus.Level + } + + testCases := []struct { + desc string + inConfig config.Backend + expectedBackendType backends.Backend + expectedLogEntries []logEntry + }{ + { + desc: "unknown", + inConfig: config.Backend{Type: "unknown"}, + expectedLogEntries: []logEntry{ + {msg: "Unknown backend type: unknown", lvl: logrus.FatalLevel}, + }, + }, + { + desc: "Cassandra", + inConfig: config.Backend{Type: config.BackendCassandra}, + expectedLogEntries: []logEntry{ + { + msg: "Error creating Cassandra backend: ", + lvl: logrus.FatalLevel, + }, + }, + }, + { + desc: "Aerospike", + inConfig: config.Backend{Type: config.BackendAerospike}, + expectedLogEntries: []logEntry{ + {msg: "Error creating Aerospike backend: ", lvl: logrus.FatalLevel}, + }, + }, + { + desc: "Redis", + inConfig: config.Backend{Type: config.BackendRedis}, + expectedLogEntries: []logEntry{ + {msg: "Error creating Redis backend: ", lvl: logrus.FatalLevel}, + }, + }, + } + + for _, tc := range testCases { + // run and assert it panics + panicTestFunction := func() { + newBaseBackend(tc.inConfig, metricstest.CreateMockMetrics()) + } + assert.Panics(t, panicTestFunction, "%s backend initialized in this test should error and panic.", tc.desc) + + // assertions + assert.Len(t, hook.Entries, len(tc.expectedLogEntries), tc.desc) + if len(tc.expectedLogEntries) > 0 { + for i := 0; i < len(tc.expectedLogEntries); i++ { + assert.Contains(t, hook.Entries[i].Message, tc.expectedLogEntries[i].msg, tc.desc) + assert.Equal(t, tc.expectedLogEntries[i].lvl, hook.Entries[i].Level, tc.desc) + } + } + hook.Reset() + assert.Nil(t, hook.LastEntry()) + } +} + +func TestGetMaxTTLSeconds(t *testing.T) { + const SIXTY_SECONDS = 60 + type testCases struct { + desc string + inConfig config.Configuration + expectedMaxTTLSeconds int + } + tests := []struct { + groupDesc string + unitTests []testCases + }{ + { + groupDesc: "Cassandra backend", + unitTests: []testCases{ + { + desc: "cfg.RequestLimits.MaxTTLSeconds > utils.CASSANDRA_DEFAULT_TTL_SECONDS", + inConfig: config.Configuration{ + Backend: config.Backend{ + Type: config.BackendCassandra, + }, + RequestLimits: config.RequestLimits{ + MaxTTLSeconds: utils.REQUEST_MAX_TTL_SECONDS, + }, + }, + expectedMaxTTLSeconds: utils.CASSANDRA_DEFAULT_TTL_SECONDS, + }, + { + desc: "cfg.RequestLimits.MaxTTLSeconds <= utils.CASSANDRA_DEFAULT_TTL_SECONDS", + inConfig: config.Configuration{ + Backend: config.Backend{ + Type: config.BackendCassandra, + }, + RequestLimits: config.RequestLimits{ + MaxTTLSeconds: 10, + }, + }, + expectedMaxTTLSeconds: 10, + }, + }, + }, + { + groupDesc: "Aerospike backend", + unitTests: []testCases{ + { + desc: "cfg.Backend.Aerospike.DefaultTTL <= 0", + inConfig: config.Configuration{ + Backend: config.Backend{ + Type: config.BackendAerospike, + Aerospike: config.Aerospike{ + DefaultTTL: 0, + }, + }, + RequestLimits: config.RequestLimits{ + MaxTTLSeconds: 10, + }, + }, + expectedMaxTTLSeconds: 10, + }, + { + desc: "cfg.Backend.Aerospike.DefaultTTL > 0 and maxTTLSeconds < cfg.Backend.Aerospike.DefaultTTL ", + inConfig: config.Configuration{ + Backend: config.Backend{ + Type: config.BackendAerospike, + Aerospike: config.Aerospike{ + DefaultTTL: 100, + }, + }, + RequestLimits: config.RequestLimits{ + MaxTTLSeconds: 10, + }, + }, + expectedMaxTTLSeconds: 10, + }, + { + desc: "cfg.Backend.Aerospike.DefaultTTL > 0 and maxTTLSeconds > cfg.Backend.Aerospike.DefaultTTL ", + inConfig: config.Configuration{ + Backend: config.Backend{ + Type: config.BackendAerospike, + Aerospike: config.Aerospike{ + DefaultTTL: 1, + }, + }, + RequestLimits: config.RequestLimits{ + MaxTTLSeconds: 10, + }, + }, + expectedMaxTTLSeconds: 1, + }, + }, + }, + { + groupDesc: "Redis backend", + unitTests: []testCases{ + { + desc: "cfg.Backend.Redis.Expiration <= 0", + inConfig: config.Configuration{ + Backend: config.Backend{ + Type: config.BackendRedis, + Redis: config.Redis{ + ExpirationMinutes: 0, + }, + }, + RequestLimits: config.RequestLimits{ + MaxTTLSeconds: 10, + }, + }, + expectedMaxTTLSeconds: 10, + }, + { + desc: "cfg.Backend.Redis.Expiration > 0 and maxTTLSeconds < cfg.Backend.Redis.Expiration*60", + inConfig: config.Configuration{ + Backend: config.Backend{ + Type: config.BackendRedis, + Redis: config.Redis{ + ExpirationMinutes: 1, + }, + }, + RequestLimits: config.RequestLimits{ + MaxTTLSeconds: 10, + }, + }, + expectedMaxTTLSeconds: 10, + }, + { + desc: "cfg.Backend.Redis.Expiration > 0 and maxTTLSeconds > cfg.Backend.Redis.Expiration", + inConfig: config.Configuration{ + Backend: config.Backend{ + Type: config.BackendRedis, + Redis: config.Redis{ + ExpirationMinutes: 1, + }, + }, + RequestLimits: config.RequestLimits{ + MaxTTLSeconds: utils.REQUEST_MAX_TTL_SECONDS, + }, + }, + expectedMaxTTLSeconds: SIXTY_SECONDS, + }, + }, + }, + } + + for _, tgroup := range tests { + for _, tc := range tgroup.unitTests { + assert.Equal(t, tc.expectedMaxTTLSeconds, getMaxTTLSeconds(tc.inConfig), tc.desc) + } + } +} + +type fakeBackend struct{} + +func (c *fakeBackend) Put(ctx context.Context, key string, value string, ttlSeconds int) error { + return nil +} + +func (c *fakeBackend) Get(ctx context.Context, key string) (string, error) { + return "", nil +} diff --git a/backends/constants.go b/backends/constants.go deleted file mode 100644 index a8b902ae..00000000 --- a/backends/constants.go +++ /dev/null @@ -1,7 +0,0 @@ -package backends - -// These strings are prefixed onto data put in the backend, to designate its type. -const ( - XML_PREFIX = "xml" - JSON_PREFIX = "json" -) diff --git a/backends/decorators/limit_ttls.go b/backends/decorators/limit_ttls.go index 2698ca60..9db3a75c 100644 --- a/backends/decorators/limit_ttls.go +++ b/backends/decorators/limit_ttls.go @@ -4,24 +4,41 @@ import ( "context" "github.com/prebid/prebid-cache/backends" + "github.com/prebid/prebid-cache/utils" ) // LimitTTLs wraps the delegate and makes sure that it never gets TTLs which exceed the max. +// or are less than zero. func LimitTTLs(delegate backends.Backend, maxTTLSeconds int) backends.Backend { + maxTTL := maxTTLSeconds + if maxTTLSeconds <= 0 { + maxTTL = utils.REQUEST_MAX_TTL_SECONDS + } return ttlLimited{ Backend: delegate, - maxTTLSeconds: maxTTLSeconds, + maxTTLSeconds: maxTTL, } } +// ttlLimited implements the backends.Backend interface to serve as a decorator that enforces +// a time-to-live limit on incoming PUT requests. type ttlLimited struct { backends.Backend maxTTLSeconds int } -func (l ttlLimited) Put(ctx context.Context, key string, value string, ttlSeconds int) error { - if l.maxTTLSeconds > ttlSeconds { - return l.Backend.Put(ctx, key, value, ttlSeconds) +// Put will make the delegate.Put() call with the default l.maxTTLSeconds whenever the +// request-defined ttl value is out of bounds +func (l ttlLimited) Put(ctx context.Context, key string, value string, requestTTLSeconds int) error { + ttl := l.maxTTLSeconds + + if l.maxTTLSeconds > requestTTLSeconds && requestTTLSeconds > 0 { + ttl = requestTTLSeconds } - return l.Backend.Put(ctx, key, value, l.maxTTLSeconds) + return l.Backend.Put(ctx, key, value, ttl) +} + +// Get will somply make the delegate.Get() call given that no TTL check is needed on the GET side +func (l ttlLimited) Get(ctx context.Context, key string) (string, error) { + return l.Backend.Get(ctx, key) } diff --git a/backends/decorators/limit_ttls_test.go b/backends/decorators/limit_ttls_test.go index 5f3a44c0..917836a9 100644 --- a/backends/decorators/limit_ttls_test.go +++ b/backends/decorators/limit_ttls_test.go @@ -5,23 +5,107 @@ import ( "testing" "github.com/prebid/prebid-cache/backends/decorators" + "github.com/prebid/prebid-cache/utils" + "github.com/stretchr/testify/assert" ) -func TestExcessiveTTL(t *testing.T) { - delegate := &ttlCapturer{} - wrapped := decorators.LimitTTLs(delegate, 100) - wrapped.Put(context.Background(), "foo", "bar", 200) - if delegate.lastTTL != 100 { - t.Errorf("lastTTL should be %d. Got %d", 100, delegate.lastTTL) +func TestLimitTTLDecorator(t *testing.T) { + type testCase struct { + desc string + inRequestTTL int + expectedTTL int } -} + testGroups := []struct { + groupDesc string + maxTTL int + testCases []testCase + }{ + { + groupDesc: "maxTTL is negative. Set to REQUEST_MAX_TTL_SECONDS constant in every scenario", + maxTTL: -1, + testCases: []testCase{ + { + desc: "reqTTL < maxTTL. Given that both hold negative values, set to REQUEST_MAX_TTL_SECONDS constant", + inRequestTTL: -2, + expectedTTL: utils.REQUEST_MAX_TTL_SECONDS, + }, + { + desc: "reqTTL = maxTTL. Given that both hold negative values, set to REQUEST_MAX_TTL_SECONDS constant", + inRequestTTL: -1, + expectedTTL: utils.REQUEST_MAX_TTL_SECONDS, + }, + { + desc: "maxTTL < reqTTL. Go with the non-negative, non-zero request ttl", + inRequestTTL: 10, + expectedTTL: 10, + }, + }, + }, + { + groupDesc: "maxTTL is zero", + maxTTL: 0, + testCases: []testCase{ + { + desc: "reqTTL < maxTTL. In the absence of a positive ttl value, set to REQUEST_MAX_TTL_SECONDS constant", + inRequestTTL: -1, + expectedTTL: utils.REQUEST_MAX_TTL_SECONDS, + }, + { + desc: "reqTTL = maxTTL. In the absence of a positive ttl value, set to REQUEST_MAX_TTL_SECONDS constant", + inRequestTTL: 0, + expectedTTL: utils.REQUEST_MAX_TTL_SECONDS, + }, + { + desc: "maxTTL < reqTTL. Go with the non-negative, non-zero request ttl", + inRequestTTL: 10, + expectedTTL: 10, + }, + }, + }, + { + groupDesc: "maxTTL is non-negative nor zero", + maxTTL: 10, + testCases: []testCase{ + { + desc: "reqTTL < 0 < maxTTL. Given that the request ttl is negative, set to maxTTL", + inRequestTTL: -1, + expectedTTL: 10, + }, + { + desc: "reqTTL equals zero. Given that the request ttl equals zero, set to maxTTL", + inRequestTTL: 0, + expectedTTL: 10, + }, + { + desc: "0 < reqTTL < maxTTL. Set to request ttl because its value is between zero and the maxTTL", + inRequestTTL: 5, + expectedTTL: 5, + }, + { + desc: "reqTTL equals maxTTL; set to request ttl because its value does not surpases maxTTL", + inRequestTTL: 10, + expectedTTL: 10, + }, + { + desc: "0 < maxTTL < reqTTL; set to maxTTL because request ttl goes past maxTTL", + inRequestTTL: 50, + expectedTTL: 10, + }, + }, + }, + } + for _, group := range testGroups { + for _, tc := range group.testCases { + // set test + delegate := &ttlCapturer{} + wrapped := decorators.LimitTTLs(delegate, group.maxTTL) + + // run + wrapped.Put(context.Background(), "key", "value", tc.inRequestTTL) -func TestSafeTTL(t *testing.T) { - delegate := &ttlCapturer{} - wrapped := decorators.LimitTTLs(delegate, 100) - wrapped.Put(context.Background(), "foo", "bar", 50) - if delegate.lastTTL != 50 { - t.Errorf("lastTTL should be %d. Got %d", 50, delegate.lastTTL) + // assertions + assert.Equal(t, tc.expectedTTL, delegate.lastTTL, "%s - %s", group.groupDesc, tc.desc) + } } } diff --git a/backends/decorators/metrics.go b/backends/decorators/metrics.go index f171fb5c..9021bce5 100644 --- a/backends/decorators/metrics.go +++ b/backends/decorators/metrics.go @@ -40,9 +40,9 @@ func (b *backendWithMetrics) Get(ctx context.Context, key string) (string, error func (b *backendWithMetrics) Put(ctx context.Context, key string, value string, ttlSeconds int) error { - if strings.HasPrefix(value, backends.XML_PREFIX) { + if strings.HasPrefix(value, utils.XML_PREFIX) { b.metrics.RecordPutBackendXml() - } else if strings.HasPrefix(value, backends.JSON_PREFIX) { + } else if strings.HasPrefix(value, utils.JSON_PREFIX) { b.metrics.RecordPutBackendJson() } else { b.metrics.RecordPutBackendInvalid() diff --git a/backends/redis.go b/backends/redis.go index 8fc3c983..e0f11403 100644 --- a/backends/redis.go +++ b/backends/redis.go @@ -72,6 +72,7 @@ func NewRedisBackend(cfg config.Redis) *RedisBackend { if err != nil { log.Fatalf("Error creating Redis backend: %v", err) + panic("RedisBackend failure. This shouldn't happen.") } log.Infof("Connected to Redis at %s:%d", cfg.Host, cfg.Port) diff --git a/config/backends.go b/config/backends.go index 8d3a785b..baa98f76 100644 --- a/config/backends.go +++ b/config/backends.go @@ -110,12 +110,12 @@ func (cfg *Memcache) validateAndLog() error { } type Redis struct { - Host string `mapstructure:"host"` - Port int `mapstructure:"port"` - Password string `mapstructure:"password"` - Db int `mapstructure:"db"` - Expiration int `mapstructure:"expiration"` - TLS RedisTLS `mapstructure:"tls"` + Host string `mapstructure:"host"` + Port int `mapstructure:"port"` + Password string `mapstructure:"password"` + Db int `mapstructure:"db"` + ExpirationMinutes int `mapstructure:"expiration"` + TLS RedisTLS `mapstructure:"tls"` } type RedisTLS struct { @@ -127,8 +127,8 @@ func (cfg *Redis) validateAndLog() error { log.Infof("config.backend.redis.host: %s", cfg.Host) log.Infof("config.backend.redis.port: %d", cfg.Port) log.Infof("config.backend.redis.db: %d", cfg.Db) - if cfg.Expiration > 0 { - log.Infof("config.backend.redis.expiration: %d. Note that this configuration option is being deprecated in favor of config.request_limits.max_ttl_seconds", cfg.Expiration) + if cfg.ExpirationMinutes > 0 { + log.Infof("config.backend.redis.expiration: %d. Note that this configuration option is being deprecated in favor of config.request_limits.max_ttl_seconds", cfg.ExpirationMinutes) } log.Infof("config.backend.redis.tls.enabled: %t", cfg.TLS.Enabled) log.Infof("config.backend.redis.tls.insecure_skip_verify: %t", cfg.TLS.InsecureSkipVerify) diff --git a/config/config.go b/config/config.go index 978d12e0..30daa17f 100644 --- a/config/config.go +++ b/config/config.go @@ -6,6 +6,8 @@ import ( log "github.com/sirupsen/logrus" "github.com/spf13/viper" + + "github.com/prebid/prebid-cache/utils" ) func NewConfig(filename string) Configuration { @@ -53,13 +55,13 @@ func setConfigDefaults(v *viper.Viper) { v.SetDefault("backend.aerospike.default_ttl_seconds", 0) v.SetDefault("backend.cassandra.hosts", "") v.SetDefault("backend.cassandra.keyspace", "") - v.SetDefault("backend.cassandra.default_ttl_seconds", 2400) + v.SetDefault("backend.cassandra.default_ttl_seconds", utils.CASSANDRA_DEFAULT_TTL_SECONDS) v.SetDefault("backend.memcache.hosts", []string{}) v.SetDefault("backend.redis.host", "") v.SetDefault("backend.redis.port", 0) v.SetDefault("backend.redis.password", "") v.SetDefault("backend.redis.db", 0) - v.SetDefault("backend.redis.expiration", 0) + v.SetDefault("backend.redis.expiration", utils.REDIS_DEFAULT_EXPIRATION_MINUTES) v.SetDefault("backend.redis.tls.enabled", false) v.SetDefault("backend.redis.tls.insecure_skip_verify", false) v.SetDefault("compression.type", "snappy") @@ -76,11 +78,11 @@ func setConfigDefaults(v *viper.Viper) { v.SetDefault("metrics.prometheus.timeout_ms", 0) v.SetDefault("metrics.prometheus.enabled", false) v.SetDefault("rate_limiter.enabled", true) - v.SetDefault("rate_limiter.num_requests", 100) + v.SetDefault("rate_limiter.num_requests", utils.RATE_LIMITER_NUM_REQUESTS) v.SetDefault("request_limits.allow_setting_keys", false) - v.SetDefault("request_limits.max_size_bytes", 10*1024) - v.SetDefault("request_limits.max_num_values", 10) - v.SetDefault("request_limits.max_ttl_seconds", 3600) + v.SetDefault("request_limits.max_size_bytes", utils.REQUEST_MAX_SIZE_BYTES) + v.SetDefault("request_limits.max_num_values", utils.REQUEST_MAX_NUM_VALUES) + v.SetDefault("request_limits.max_ttl_seconds", utils.REQUEST_MAX_TTL_SECONDS) v.SetDefault("routes.allow_public_write", true) } diff --git a/config/config_test.go b/config/config_test.go index a013bbb4..de14a8c8 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -8,6 +8,7 @@ import ( "testing" "time" + "github.com/prebid/prebid-cache/utils" "github.com/sirupsen/logrus" "github.com/sirupsen/logrus/hooks/test" "github.com/spf13/viper" @@ -1186,7 +1187,10 @@ func getExpectedDefaultConfig() Configuration { Hosts: []string{}, }, Cassandra: Cassandra{ - DefaultTTL: 2400, + DefaultTTL: utils.CASSANDRA_DEFAULT_TTL_SECONDS, + }, + Redis: Redis{ + ExpirationMinutes: utils.REDIS_DEFAULT_EXPIRATION_MINUTES, }, }, Compression: Compression{ @@ -1246,11 +1250,11 @@ func getExpectedFullConfigForTestFile() Configuration { Hosts: []string{"10.0.0.1:11211", "127.0.0.1"}, }, Redis: Redis{ - Host: "127.0.0.1", - Port: 6379, - Password: "redis-password", - Db: 1, - Expiration: 1, + Host: "127.0.0.1", + Port: 6379, + Password: "redis-password", + Db: 1, + ExpirationMinutes: 1, TLS: RedisTLS{ Enabled: false, InsecureSkipVerify: false, diff --git a/endpoints/get.go b/endpoints/get.go index 50e8b8ca..ebd00e54 100644 --- a/endpoints/get.go +++ b/endpoints/get.go @@ -85,12 +85,12 @@ func parseUUID(r *http.Request, allowCustomKeys bool) (string, error) { // writeGetResponse writes the "Content-Type" header and sends back the stored data as a response if // the sotred data is prefixed by either the "xml" or "json" func writeGetResponse(w http.ResponseWriter, storedData string) error { - if strings.HasPrefix(storedData, backends.XML_PREFIX) { + if strings.HasPrefix(storedData, utils.XML_PREFIX) { w.Header().Set("Content-Type", "application/xml") - w.Write([]byte(storedData)[len(backends.XML_PREFIX):]) - } else if strings.HasPrefix(storedData, backends.JSON_PREFIX) { + w.Write([]byte(storedData)[len(utils.XML_PREFIX):]) + } else if strings.HasPrefix(storedData, utils.JSON_PREFIX) { w.Header().Set("Content-Type", "application/json") - w.Write([]byte(storedData)[len(backends.JSON_PREFIX):]) + w.Write([]byte(storedData)[len(utils.JSON_PREFIX):]) } else { return utils.NewPBCError(utils.UNKNOWN_STORED_DATA_TYPE) } diff --git a/endpoints/put.go b/endpoints/put.go index b2f2eec6..0a9a7934 100644 --- a/endpoints/put.go +++ b/endpoints/put.go @@ -122,7 +122,7 @@ func parsePutObject(p putObject) (string, error) { } // Limit the type of data to XML or JSON - if p.Type == backends.XML_PREFIX { + if p.Type == utils.XML_PREFIX { if p.Value[0] != byte('"') || p.Value[len(p.Value)-1] != byte('"') { return "", utils.NewPBCError(utils.MALFORMED_XML, fmt.Sprintf("XML messages must have a String value. Found %v", p.Value)) } @@ -135,7 +135,7 @@ func parsePutObject(p putObject) (string, error) { } toCache = p.Type + interpreted - } else if p.Type == backends.JSON_PREFIX { + } else if p.Type == utils.JSON_PREFIX { toCache = p.Type + string(p.Value) } else { return "", utils.NewPBCError(utils.UNSUPPORTED_DATA_TO_STORE, fmt.Sprintf("Type must be one of [\"json\", \"xml\"]. Found %v", p.Type)) diff --git a/utils/constants.go b/utils/constants.go new file mode 100644 index 00000000..79bf6b64 --- /dev/null +++ b/utils/constants.go @@ -0,0 +1,17 @@ +package utils + +// These strings are prefixed onto data put in the backend, to designate its type. +const ( + XML_PREFIX = "xml" + JSON_PREFIX = "json" +) + +// The following numeric constants serve as configuration defaults +const ( + CASSANDRA_DEFAULT_TTL_SECONDS = 2400 + REDIS_DEFAULT_EXPIRATION_MINUTES = 60 + RATE_LIMITER_NUM_REQUESTS = 100 + REQUEST_MAX_SIZE_BYTES = 10 * 1024 + REQUEST_MAX_NUM_VALUES = 10 + REQUEST_MAX_TTL_SECONDS = 3600 +)