diff --git a/backends/aerospike.go b/backends/aerospike.go index e81e38e1..bb1c4200 100644 --- a/backends/aerospike.go +++ b/backends/aerospike.go @@ -3,6 +3,7 @@ package backends import ( "context" "errors" + "fmt" "time" as "github.com/aerospike/aerospike-client-go/v6" @@ -52,37 +53,22 @@ type AerospikeBackend struct { } // NewAerospikeBackend validates config.Aerospike and returns an AerospikeBackend -func NewAerospikeBackend(cfg config.Aerospike, metrics *metrics.Metrics) *AerospikeBackend { - var hosts []*as.Host - - clientPolicy := as.NewClientPolicy() - // cfg.User and cfg.Password are optional parameters - // if left blank in the config, they will default to the empty - // string and be ignored - clientPolicy.User = cfg.User - clientPolicy.Password = cfg.Password - // Aerospike's connection idle deadline default is 55 seconds. If greater than zero, this - // value will override - if cfg.ConnIdleTimeoutSecs > 0 { - clientPolicy.IdleTimeout = time.Duration(cfg.ConnIdleTimeoutSecs) * time.Second - } +type NewAerospikeClientFunc func(*as.ClientPolicy, ...*as.Host) (*as.Client, as.Error) - // Aerospike's default connection queue size per node is 256. - // If cfg.ConnQueueSize is greater than zero, it will override the default. - if cfg.ConnQueueSize > 0 { - clientPolicy.ConnectionQueueSize = cfg.ConnQueueSize - } +func NewAerospikeBackend(cfg config.Aerospike, metrics *metrics.Metrics) *AerospikeBackend { + return newAerospikeBackend(as.NewClientWithPolicyAndHost, cfg, metrics) +} - if len(cfg.Host) > 1 { - hosts = append(hosts, as.NewHost(cfg.Host, cfg.Port)) - log.Info("config.backend.aerospike.host is being deprecated in favor of config.backend.aerospike.hosts") - } - for _, host := range cfg.Hosts { - hosts = append(hosts, as.NewHost(host, cfg.Port)) +func newAerospikeBackend(newAerospikeClient NewAerospikeClientFunc, cfg config.Aerospike, metrics *metrics.Metrics) *AerospikeBackend { + clientPolicy := generateAerospikeClientPolicy(cfg) + hosts, err := generateHostsList(cfg) + if err != nil { + log.Fatalf("Error creating Aerospike backend: %s", err.Error()) + return nil } - client, err := as.NewClientWithPolicyAndHost(clientPolicy, hosts...) + client, err := newAerospikeClient(clientPolicy, hosts...) if err != nil { log.Fatalf("Error creating Aerospike backend: %s", classifyAerospikeError(err).Error()) panic("AerospikeBackend failure. This shouldn't happen.") @@ -110,6 +96,48 @@ func NewAerospikeBackend(cfg config.Aerospike, metrics *metrics.Metrics) *Aerosp } } +// generateAerospikeClientPolicy returns an Aerospike ClientPolicy object configured according to values +// in config.Aerospike fields +func generateAerospikeClientPolicy(cfg config.Aerospike) *as.ClientPolicy { + clientPolicy := as.NewClientPolicy() + // cfg.User and cfg.Password are optional parameters + // if left blank in the config, they will default to the empty + // string and be ignored + clientPolicy.User = cfg.User + clientPolicy.Password = cfg.Password + + // Connection idle timeout default is 55 seconds + if cfg.ConnIdleTimeoutSecs > 0 { + clientPolicy.IdleTimeout = time.Duration(cfg.ConnIdleTimeoutSecs) * time.Second + } + + // Default connection queue size per node is 256 + if cfg.ConnQueueSize > 0 { + clientPolicy.ConnectionQueueSize = cfg.ConnQueueSize + } + + return clientPolicy +} + +func generateHostsList(cfg config.Aerospike) ([]*as.Host, error) { + var hosts []*as.Host + + if cfg.Port <= 0 { + return nil, fmt.Errorf("Cannot connect to Aerospike host at port %d", cfg.Port) + } + if len(cfg.Host) > 1 { + hosts = append(hosts, as.NewHost(cfg.Host, cfg.Port)) + log.Info("config.backend.aerospike.host is being deprecated in favor of config.backend.aerospike.hosts") + } + for _, host := range cfg.Hosts { + hosts = append(hosts, as.NewHost(host, cfg.Port)) + } + if len(hosts) == 0 { + return nil, errors.New("Cannot connect to empty Aerospike host(s)") + } + return hosts, nil +} + // Get creates an aerospike key based on the UUID key parameter, perfomrs the client's Get call // and validates results. Can return a KEY_NOT_FOUND error or other Aerospike server errors func (a *AerospikeBackend) Get(ctx context.Context, key string) (string, error) { diff --git a/backends/aerospike_test.go b/backends/aerospike_test.go index 82d2bbfc..85500eff 100644 --- a/backends/aerospike_test.go +++ b/backends/aerospike_test.go @@ -2,8 +2,10 @@ package backends import ( "context" + "errors" "fmt" "testing" + "time" as "github.com/aerospike/aerospike-client-go/v6" as_types "github.com/aerospike/aerospike-client-go/v6/types" @@ -17,66 +19,281 @@ import ( "github.com/stretchr/testify/assert" ) -func TestNewAerospikeBackend(t *testing.T) { +func TestGenerateAerospikeClientPolicy(t *testing.T) { + testCases := []struct { + desc string + inCfg config.Aerospike + expected *as.ClientPolicy + }{ + { + desc: "Blank configuration", + inCfg: config.Aerospike{}, + expected: as.NewClientPolicy(), + }, + { + desc: "Config with credentials", + inCfg: config.Aerospike{ + User: "foobar", + Password: "password", + }, + expected: &as.ClientPolicy{ + User: "foobar", + Password: "password", + AuthMode: as.AuthModeInternal, + Timeout: 30 * time.Second, + IdleTimeout: 0 * time.Second, + LoginTimeout: 10 * time.Second, + ConnectionQueueSize: 100, + OpeningConnectionThreshold: 0, + FailIfNotConnected: true, + TendInterval: time.Second, + LimitConnectionsToQueueSize: true, + IgnoreOtherSubnetAliases: false, + MaxErrorRate: 100, + ErrorRateWindow: 1, + }, + }, + { + desc: "Config with ConnIdleTimeoutSecs", + inCfg: config.Aerospike{ + ConnIdleTimeoutSecs: 3600, + }, + expected: &as.ClientPolicy{ + AuthMode: as.AuthModeInternal, + Timeout: 30 * time.Second, + IdleTimeout: 3600 * time.Second, + LoginTimeout: 10 * time.Second, + ConnectionQueueSize: 100, + OpeningConnectionThreshold: 0, + FailIfNotConnected: true, + TendInterval: time.Second, + LimitConnectionsToQueueSize: true, + IgnoreOtherSubnetAliases: false, + MaxErrorRate: 100, + ErrorRateWindow: 1, + }, + }, + { + desc: "Config with ConnIdleTimeoutSecs", + inCfg: config.Aerospike{ + ConnQueueSize: 31416, + }, + expected: &as.ClientPolicy{ + AuthMode: as.AuthModeInternal, + Timeout: 30 * time.Second, + IdleTimeout: 0 * time.Second, + LoginTimeout: 10 * time.Second, + ConnectionQueueSize: 31416, + OpeningConnectionThreshold: 0, + FailIfNotConnected: true, + TendInterval: time.Second, + LimitConnectionsToQueueSize: true, + IgnoreOtherSubnetAliases: false, + MaxErrorRate: 100, + ErrorRateWindow: 1, + }, + }, + } + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + asPolicy := generateAerospikeClientPolicy(tc.inCfg) + assert.Equal(t, tc.expected, asPolicy) + }) + } +} + +func TestGenerateHostsList(t *testing.T) { + type testOutput struct { + hosts []*as.Host + err error + } type logEntry struct { msg string lvl logrus.Level } - testCases := []struct { desc string inCfg config.Aerospike - expectPanic bool + expectedOut testOutput expectedLogEntries []logEntry }{ { - desc: "Unable to connect hosts fakeTestUrl panic and log fatal error when passed additional hosts", + desc: "no_port", + inCfg: config.Aerospike{}, + expectedOut: testOutput{ + err: errors.New("Cannot connect to Aerospike host at port 0"), + }, + }, + { + desc: "port_no_host_nor_hosts", + inCfg: config.Aerospike{Port: 8888}, + expectedOut: testOutput{ + err: errors.New("Cannot connect to empty Aerospike host(s)"), + }, + }, + { + desc: "port_and_hosts_no_host", inCfg: config.Aerospike{ - Hosts: []string{"foo.com", "bat.com"}, Port: 8888, + Hosts: []string{"foo.com", "bar.com"}, + }, + expectedOut: testOutput{ + hosts: []*as.Host{ + as.NewHost("foo.com", 8888), + as.NewHost("bar.com", 8888), + }, + }, + }, + { + desc: "port_and_host", + inCfg: config.Aerospike{ + Host: "foo.com", + Port: 8888, + }, + expectedOut: testOutput{ + hosts: []*as.Host{as.NewHost("foo.com", 8888)}, }, - expectPanic: true, expectedLogEntries: []logEntry{ { - msg: "Error creating Aerospike backend: ResultCode: TIMEOUT, Iteration: 0, InDoubt: false, Node: : command execution timed out on client: See `Policy.Timeout`", - lvl: logrus.FatalLevel, + msg: "config.backend.aerospike.host is being deprecated in favor of config.backend.aerospike.hosts", + lvl: logrus.InfoLevel, }, }, }, { - desc: "Unable to connect host and hosts panic and log fatal error when passed additional hosts", + desc: "Port_host_and_hosts", inCfg: config.Aerospike{ - Host: "fakeTestUrl.foo", - Hosts: []string{"foo.com", "bat.com"}, Port: 8888, + Host: "foo.com", + Hosts: []string{"foo.com", "bar.com"}, + }, + expectedOut: testOutput{ + hosts: []*as.Host{ + as.NewHost("foo.com", 8888), + as.NewHost("foo.com", 8888), + as.NewHost("bar.com", 8888), + }, }, - expectPanic: true, expectedLogEntries: []logEntry{ { msg: "config.backend.aerospike.host is being deprecated in favor of config.backend.aerospike.hosts", lvl: logrus.InfoLevel, }, + }, + }, + } + + // logrus entries will be recorded to this `hook` object so we can compare and assert them + hook := test.NewGlobal() + + //substitute logger exit function so execution doesn't get interrupted when log.Fatalf() call comes + defer func() { logrus.StandardLogger().ExitFunc = nil }() + logrus.StandardLogger().ExitFunc = func(int) {} + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + asHosts, err := generateHostsList(tc.inCfg) + assert.Equal(t, tc.expectedOut.err, err) + if assert.Len(t, asHosts, len(tc.expectedOut.hosts)) { + assert.ElementsMatch(t, tc.expectedOut.hosts, asHosts) + } + if assert.Len(t, hook.Entries, len(tc.expectedLogEntries)) { + for i := 0; i < len(tc.expectedLogEntries); i++ { + assert.Equal(t, tc.expectedLogEntries[i].lvl, hook.Entries[i].Level) + assert.Equal(t, tc.expectedLogEntries[i].msg, hook.Entries[i].Message) + } + } + //Reset log after every test and assert successful reset + hook.Reset() + assert.Nil(t, hook.LastEntry()) + }) + } +} + +func TestNewAerospikeBackend(t *testing.T) { + type logEntry struct { + msg string + lvl logrus.Level + } + + errorProneNewClientFunc := func(*as.ClientPolicy, ...*as.Host) (*as.Client, as.Error) { + return nil, &as.AerospikeError{} + } + successfulNewClientFunc := func(*as.ClientPolicy, ...*as.Host) (*as.Client, as.Error) { + return nil, nil + } + + testCases := []struct { + desc string + inCfg config.Aerospike + newClientFunc NewAerospikeClientFunc + expectedLogEntries []logEntry + expectedPanic bool + }{ + { + desc: "no_port_error", + inCfg: config.Aerospike{}, + expectedLogEntries: []logEntry{ + { + msg: "Error creating Aerospike backend: Cannot connect to Aerospike host at port 0", + lvl: logrus.FatalLevel, + }, + }, + }, + { + desc: "no_host_nor_hosts_error", + inCfg: config.Aerospike{Port: 8888}, + expectedLogEntries: []logEntry{ { - msg: "Error creating Aerospike backend: ResultCode: TIMEOUT, Iteration: 0, InDoubt: false, Node: : command execution timed out on client: See `Policy.Timeout`", + msg: "Error creating Aerospike backend: Cannot connect to empty Aerospike host(s)", lvl: logrus.FatalLevel, }, }, }, { - desc: "Unable to connect host panic and log fatal error", + desc: "newAerospikeClient_error", inCfg: config.Aerospike{ - Host: "fakeTestUrl.foo", + Hosts: []string{"fakeUrl"}, + Port: 8888, + }, + newClientFunc: errorProneNewClientFunc, + expectedLogEntries: []logEntry{ + { + msg: "Error creating Aerospike backend: ResultCode: OK, Iteration: 0, InDoubt: false, Node: : ", + lvl: logrus.FatalLevel, + }, + }, + expectedPanic: true, + }, + { + desc: "success_with_deprecated_host", + inCfg: config.Aerospike{ + Host: "fakeUrl", Port: 8888, }, - expectPanic: true, + newClientFunc: successfulNewClientFunc, expectedLogEntries: []logEntry{ { msg: "config.backend.aerospike.host is being deprecated in favor of config.backend.aerospike.hosts", lvl: logrus.InfoLevel, }, { - msg: "Error creating Aerospike backend: ResultCode: TIMEOUT, Iteration: 0, InDoubt: false, Node: : command execution timed out on client: See `Policy.Timeout`", - lvl: logrus.FatalLevel, + msg: "Connected to Aerospike host(s) [fakeUrl] on port 8888", + lvl: logrus.InfoLevel, + }, + }, + }, + { + desc: "success_with_hosts_list", + inCfg: config.Aerospike{ + Hosts: []string{"fakeUrl"}, + Port: 8888, + }, + newClientFunc: successfulNewClientFunc, + expectedLogEntries: []logEntry{ + { + msg: "Connected to Aerospike host(s) [fakeUrl ] on port 8888", + lvl: logrus.InfoLevel, }, }, }, @@ -90,18 +307,29 @@ func TestNewAerospikeBackend(t *testing.T) { logrus.StandardLogger().ExitFunc = func(int) {} for _, test := range testCases { - // Run test - assert.Panics(t, func() { NewAerospikeBackend(test.inCfg, nil) }, "Aerospike library's NewClientWithPolicyAndHost() should have thrown an error and didn't, hence the panic didn't happen") - if assert.Len(t, hook.Entries, len(test.expectedLogEntries), test.desc) { - for i := 0; i < len(test.expectedLogEntries); i++ { - assert.Equal(t, test.expectedLogEntries[i].msg, hook.Entries[i].Message, test.desc) - assert.Equal(t, test.expectedLogEntries[i].lvl, hook.Entries[i].Level, test.desc) + t.Run(test.desc, func(t *testing.T) { + if test.expectedPanic { + if !assert.Panics(t, func() { newAerospikeBackend(test.newClientFunc, test.inCfg, nil) }, "Aerospike library's NewClientWithPolicyAndHost() should have thrown an error and didn't, hence the panic didn't happen") { + return + } + } else { + if !assert.NotPanics(t, func() { newAerospikeBackend(test.newClientFunc, test.inCfg, nil) }, "Aerospike library's NewClientWithPolicyAndHost() should have thrown an error and didn't, hence the panic didn't happen") { + return + } } - } - //Reset log after every test and assert successful reset - hook.Reset() - assert.Nil(t, hook.LastEntry()) + if assert.Len(t, hook.Entries, len(test.expectedLogEntries), test.desc) { + for i := 0; i < len(test.expectedLogEntries); i++ { + assert.Equal(t, test.expectedLogEntries[i].lvl, hook.Entries[i].Level, test.desc) + assert.Equal(t, test.expectedLogEntries[i].msg, hook.Entries[i].Message, test.desc) + } + } + + //Reset log after every test and assert successful reset + hook.Reset() + assert.Nil(t, hook.LastEntry()) + }) + } } diff --git a/backends/config/config_test.go b/backends/config/config_test.go index ea72d46a..220b934f 100644 --- a/backends/config/config_test.go +++ b/backends/config/config_test.go @@ -121,19 +121,22 @@ func TestNewBaseBackend(t *testing.T) { testCases := []struct { desc string inConfig config.Backend + inExpectPanic bool expectedBackendType backends.Backend expectedLogEntries []logEntry }{ { - desc: "unknown", - inConfig: config.Backend{Type: "unknown"}, + desc: "unknown", + inConfig: config.Backend{Type: "unknown"}, + inExpectPanic: true, expectedLogEntries: []logEntry{ {msg: "Unknown backend type: unknown", lvl: logrus.FatalLevel}, }, }, { - desc: "Cassandra", - inConfig: config.Backend{Type: config.BackendCassandra}, + desc: "Cassandra", + inConfig: config.Backend{Type: config.BackendCassandra}, + inExpectPanic: true, expectedLogEntries: []logEntry{ {msg: "Error creating Cassandra backend: ", lvl: logrus.FatalLevel}, }, @@ -146,15 +149,17 @@ func TestNewBaseBackend(t *testing.T) { }, }, { - desc: "Redis", - inConfig: config.Backend{Type: config.BackendRedis}, + desc: "Redis", + inConfig: config.Backend{Type: config.BackendRedis}, + inExpectPanic: true, expectedLogEntries: []logEntry{ {msg: "Error creating Redis backend: ", lvl: logrus.FatalLevel}, }, }, { - desc: "Ignite", - inConfig: config.Backend{Type: config.BackendIgnite}, + desc: "Ignite", + inConfig: config.Backend{Type: config.BackendIgnite}, + inExpectPanic: true, expectedLogEntries: []logEntry{ { msg: "Error creating Ignite backend: configuration is missing ignite.schema, ignite.host, ignite.port or ignite.cache.name", @@ -170,11 +175,20 @@ func TestNewBaseBackend(t *testing.T) { MetricEngines: []metrics.CacheMetrics{&mockMetrics}, } - // run and assert it panics + // run panicTestFunction := func() { newBaseBackend(tc.inConfig, m) } - assert.Panics(t, panicTestFunction, "%s backend initialized in this test should error and panic.", tc.desc) + + if tc.inExpectPanic { + if !assert.Panics(t, panicTestFunction, "%s backend initialized in this test should error and panic.", tc.desc) { + continue + } + } else { + if !assert.NotPanics(t, panicTestFunction, "%s backend initialized in this test should not panic.", tc.desc) { + continue + } + } // assertions assert.Len(t, hook.Entries, len(tc.expectedLogEntries), tc.desc) diff --git a/go.mod b/go.mod index c3e28311..04c26750 100644 --- a/go.mod +++ b/go.mod @@ -54,7 +54,7 @@ require ( golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f // indirect golang.org/x/text v0.3.8 // indirect golang.org/x/time v0.0.0-20200416051211-89c76fbcd5d1 // indirect - google.golang.org/protobuf v1.28.0 // indirect + google.golang.org/protobuf v1.33.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/ini.v1 v1.66.4 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect diff --git a/go.sum b/go.sum index f01caaca..5d792d68 100644 --- a/go.sum +++ b/go.sum @@ -629,8 +629,8 @@ google.golang.org/protobuf v1.24.0/go.mod h1:r/3tXBNzIEhYS9I1OUVjXDlt8tc493IdKGj google.golang.org/protobuf v1.25.0/go.mod h1:9JNX74DMeImyA3h4bdi1ymwjUzf21/xIlbajtzgsN7c= google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw= google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc= -google.golang.org/protobuf v1.28.0 h1:w43yiav+6bVFTBQFZX0r7ipe9JQ1QsbMgHwbBziscLw= -google.golang.org/protobuf v1.28.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I= +google.golang.org/protobuf v1.33.0 h1:uNO2rsAINq/JlFpSdYEKIZ0uKD/R9cpdv0T+yoGwGmI= +google.golang.org/protobuf v1.33.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos= gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=