Skip to content

Commit

Permalink
Zero TTL fix (#107)
Browse files Browse the repository at this point in the history
* 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 <[email protected]>
  • Loading branch information
guscarreon and Gus Carreon authored Apr 13, 2022
1 parent f67d98a commit 93f018f
Show file tree
Hide file tree
Showing 15 changed files with 509 additions and 62 deletions.
2 changes: 1 addition & 1 deletion backends/aerospike.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 3 additions & 4 deletions backends/aerospike_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
},
Expand All @@ -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,
},
},
Expand All @@ -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,
},
},
Expand Down
9 changes: 5 additions & 4 deletions backends/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
329 changes: 329 additions & 0 deletions backends/config/config_test.go
Original file line number Diff line number Diff line change
@@ -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
}
Loading

0 comments on commit 93f018f

Please sign in to comment.