Skip to content

Commit

Permalink
chore: fix cacher interceptor ordering (#1914)
Browse files Browse the repository at this point in the history 
* chore: fix cacher interceptor ordering

* chore: fix storage cacher tests (#1916)

* chore: why are you changing go.work.sum

* chore: fixup cacher tests into new eval-v2 branch

* chore: rm unknown file
  • Loading branch information
@markphelps
markphelps committed Jul 28, 2023
1 parent 9bfda68 commit 15a9858
Show file tree
Hide file tree
Showing 5 changed files with 353 additions and 258 deletions.
1 change: 1 addition & 0 deletions go.work.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,7 @@ github.com/prometheus/common v0.9.1/go.mod h1:yhUN8i9wzaXS3w1O07YhxHEBxD+W35wd8b
github.com/prometheus/common v0.42.0/go.mod h1:xBwqVerjNdUDjgODMpudtOMwlOwf2SaTr1yjz4b7Zbc=
github.com/prometheus/procfs v0.0.8/go.mod h1:7Qr8sr6344vo1JqZ6HhLceV9o3AJ1Ff+GxbHq6oeK9A=
github.com/rogpeppe/go-charset v0.0.0-20180617210344-2471d30d28b4/go.mod h1:qgYeAmZ5ZIpBWTGllZSQnw97Dj+woV0toclVaRGI8pc=
github.com/russross/blackfriday v1.6.0 h1:KqfZb0pUVN2lYqZUYRddxF4OR8ZMURnJIG5Y3VRLtww=
github.com/ryanuber/columnize v0.0.0-20160712163229-9b3edd62028f/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts=
github.com/sagikazarmark/crypt v0.10.0/go.mod h1:gwTNHQVoOS3xp9Xvz5LLR+1AauC5M6880z5NWzdhOyQ=
github.com/sean-/seed v0.0.0-20170313163322-e2103e2c3529/go.mod h1:DxrIzT+xaE7yg65j358z/aeFdxmN0P9QXhEzd20vsDc=
Expand Down
15 changes: 9 additions & 6 deletions internal/cmd/grpc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,9 +237,8 @@ func NewGRPCServer(
otelgrpc.UnaryServerInterceptor(),
}

Check warning on line 238 in internal/cmd/grpc.go

View check run for this annotation

Codecov / codecov/patch

internal/cmd/grpc.go#L238 

Added line #L238 was not covered by tests

var cacher cache.Cacher

Check warning on line 240 in internal/cmd/grpc.go

View check run for this annotation

Codecov / codecov/patch

internal/cmd/grpc.go#L240 

Added line #L240 was not covered by tests
if cfg.Cache.Enabled {
var cacher cache.Cacher

switch cfg.Cache.Backend {
case config.CacheMemory:
cacher = memory.NewCache(cfg.Cache)
Expand Down Expand Up @@ -268,7 +267,6 @@ func NewGRPCServer(
}))
}

interceptors = append(interceptors, middlewaregrpc.CacheUnaryInterceptor(cacher, logger))
store = storagecache.New(store, cacher, logger)

logger.Debug("cache enabled", zap.Stringer("backend", cacher))
Expand Down Expand Up @@ -325,7 +323,12 @@ func NewGRPCServer(
)...,
)

// audit sinks configuration.
// cache must come after auth interceptors
if cfg.Cache.Enabled && cacher != nil {
interceptors = append(interceptors, middlewaregrpc.CacheUnaryInterceptor(cacher, logger))
}

Check warning on line 329 in internal/cmd/grpc.go

View check run for this annotation

Codecov / codecov/patch

internal/cmd/grpc.go#L315-L329 

Added lines #L315 - L329 were not covered by tests

// audit sinks configuration
sinks := make([]audit.Sink, 0)

if cfg.Audit.Sinks.LogFile.Enabled {
Expand All @@ -337,8 +340,8 @@ func NewGRPCServer(
sinks = append(sinks, logFileSink)
}

// Based on audit sink configuration from the user, provision the audit sinks and add them to a slice,
// and if the slice has a non-zero length, add the audit sink interceptor.
// based on audit sink configuration from the user, provision the audit sinks and add them to a slice,
// and if the slice has a non-zero length, add the audit sink interceptor
if len(sinks) > 0 {
sse := audit.NewSinkSpanExporter(logger, sinks)
tracingProvider.RegisterSpanProcessor(tracesdk.NewBatchSpanProcessor(sse, tracesdk.WithBatchTimeout(cfg.Audit.Buffer.FlushPeriod), tracesdk.WithMaxExportBatchSize(cfg.Audit.Buffer.Capacity)))
Expand Down
108 changes: 64 additions & 44 deletions internal/storage/cache/cache_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,78 +5,98 @@ import (
"errors"
"testing"

"github.com/stretchr/testify/require"
"github.com/stretchr/testify/assert"
"go.flipt.io/flipt/internal/storage"
"go.uber.org/zap/zaptest"
)

func TestSetCacheHandleMarshalError(t *testing.T) {
store := &mockStore{}
cacher := &mockCacher{}
logger := zaptest.NewLogger(t)
cachedStore, _ := New(store, cacher, logger).(*Store)
var (
store = &storeMock{}
cacher = &cacheSpy{}
logger = zaptest.NewLogger(t)
cachedStore, _ = New(store, cacher, logger).(*Store)
)

cachedStore.setCache(context.TODO(), "key", make(chan int))
require.Equal(t, "", cacher.CacheKey)
assert.Empty(t, cacher.cacheKey)
}

func TestGetCacheHandleGetError(t *testing.T) {
store := &mockStore{}
cacher := &mockCacher{GetErr: errors.New("get error")}
logger := zaptest.NewLogger(t)
cachedStore, _ := New(store, cacher, logger).(*Store)
var (
store = &storeMock{}
cacher = &cacheSpy{getErr: errors.New("get error")}
logger = zaptest.NewLogger(t)
cachedStore, _ = New(store, cacher, logger).(*Store)
)

value := make(map[string]string)
cacheHit := cachedStore.getCache(context.TODO(), "key", &value)
require.False(t, cacheHit)
assert.False(t, cacheHit)
}

func TestGetCacheHandleUnmarshalError(t *testing.T) {
store := &mockStore{}
cacher := &mockCacher{
Cached: true,
CachedValue: []byte(`{"invalid":"123"`),
}
logger := zaptest.NewLogger(t)
cachedStore, _ := New(store, cacher, logger).(*Store)
var (
store = &storeMock{}
cacher = &cacheSpy{
cached: true,
cachedValue: []byte(`{"invalid":"123"`),
}
logger = zaptest.NewLogger(t)
cachedStore, _ = New(store, cacher, logger).(*Store)
)

value := make(map[string]string)
cacheHit := cachedStore.getCache(context.TODO(), "key", &value)
require.False(t, cacheHit)
assert.False(t, cacheHit)
}

func TestGetEvaluationRules(t *testing.T) {
expectedRules := []*storage.EvaluationRule{{ID: "123"}}
store := &mockStore{
EvaluationRules: expectedRules,
}
cacher := &mockCacher{}
logger := zaptest.NewLogger(t)
cachedStore := New(store, cacher, logger)
var (
expectedRules = []*storage.EvaluationRule{{ID: "123"}}
store = &storeMock{}
)

store.On("GetEvaluationRules", context.TODO(), "ns", "flag-1").Return(
expectedRules, nil,
)

var (
cacher = &cacheSpy{}
logger = zaptest.NewLogger(t)
cachedStore = New(store, cacher, logger)
)

rules, err := cachedStore.GetEvaluationRules(context.TODO(), "ns", "flag-1")
require.Nil(t, err)
require.Equal(t, expectedRules, rules)
assert.Nil(t, err)
assert.Equal(t, expectedRules, rules)

require.Equal(t, "s:er:ns:flag-1", cacher.CacheKey)
require.Equal(t, []byte(`[{"id":"123"}]`), cacher.CachedValue)
assert.Equal(t, "s:er:ns:flag-1", cacher.cacheKey)
assert.Equal(t, []byte(`[{"id":"123"}]`), cacher.cachedValue)
}

func TestGetEvaluationRulesCached(t *testing.T) {
expectedRules := []*storage.EvaluationRule{{ID: "123"}}
store := &mockStore{
EvaluationRules: []*storage.EvaluationRule{{ID: "543"}},
}
cacher := &mockCacher{
Cached: true,
CachedValue: []byte(`[{"id":"123"}]`),
}
logger := zaptest.NewLogger(t)
cachedStore := New(store, cacher, logger)
var (
expectedRules = []*storage.EvaluationRule{{ID: "123"}}
store = &storeMock{}
)

rules, err := cachedStore.GetEvaluationRules(context.TODO(), "ns", "flag-1")
require.Nil(t, err)
require.Equal(t, expectedRules, rules)
store.On("GetEvaluationRules", context.TODO(), "ns", "flag-1").Return(
expectedRules, nil,
)

require.Equal(t, "s:er:ns:flag-1", cacher.CacheKey)
var (
cacher = &cacheSpy{
cached: true,
cachedValue: []byte(`[{"id":"123"}]`),
}

logger = zaptest.NewLogger(t)
cachedStore = New(store, cacher, logger)
)

rules, err := cachedStore.GetEvaluationRules(context.TODO(), "ns", "flag-1")
assert.Nil(t, err)
assert.Equal(t, expectedRules, rules)
assert.Equal(t, "s:er:ns:flag-1", cacher.cacheKey)
}
Loading

0 comments on commit 15a9858

Please sign in to comment.