Skip to content

Instantly share code, notes, and snippets.

@miparnisari
Created August 17, 2023 05:26
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save miparnisari/d833b97ba94e8d57ae50e6e8404ca4d6 to your computer and use it in GitHub Desktop.
Save miparnisari/d833b97ba94e8d57ae50e6e8404ca4d6 to your computer and use it in GitHub Desktop.
diff --git a/internal/graph/check.go b/internal/graph/check.go
index 4449d6af..66dbe94c 100644
--- a/internal/graph/check.go
+++ b/internal/graph/check.go
@@ -143,7 +143,7 @@ func WithCachedResolver(opts ...CachedCheckResolverOpt) LocalCheckerOption {
d,
opts...,
)
- d.SetDelegate(cachedCheckResolver)
+ d.delegate = cachedCheckResolver
}
}
@@ -418,10 +418,6 @@ func exclusion(ctx context.Context, concurrencyLimit uint32, handlers ...CheckHa
func (c *LocalChecker) Close() {
}
-func (c *LocalChecker) SetDelegate(delegate CheckResolver) {
- c.delegate = delegate
-}
-
// dispatch dispatches the provided Check request to the CheckResolver this LocalChecker
// was constructed with.
func (c *LocalChecker) dispatch(ctx context.Context, req *ResolveCheckRequest) CheckHandlerFunc {
diff --git a/pkg/server/commands/list_objects.go b/pkg/server/commands/list_objects.go
index 32b8a261..014313de 100644
--- a/pkg/server/commands/list_objects.go
+++ b/pkg/server/commands/list_objects.go
@@ -9,7 +9,6 @@ import (
"sync/atomic"
"time"
- "github.com/karlseguin/ccache/v3"
openfgav1 "github.com/openfga/api/proto/openfga/v1"
"github.com/openfga/openfga/internal/graph"
"github.com/openfga/openfga/internal/validation"
@@ -34,8 +33,6 @@ const (
defaultListObjectsDeadline = 3 * time.Second
defaultListObjectsMaxResults = 1000
defaultMaxConcurrentReads = 30
-
- defaultCheckQueryCacheTTL = 10 * time.Second
)
var (
@@ -59,9 +56,8 @@ type ListObjectsQuery struct {
resolveNodeBreadthLimit uint32
maxConcurrentReads uint32
- // configurations for caching results
- checkQueryCacheTTL time.Duration
- checkCache *ccache.Cache[*graph.CachedResolveCheckResponse] // checkCache has to be shared across requests
+ // configurations for check
+ checkOptions []graph.LocalCheckerOption
}
type ListObjectsQueryOption func(d *ListObjectsQuery)
@@ -105,17 +101,9 @@ func WithLogger(l logger.Logger) ListObjectsQueryOption {
}
}
-// WithCheckQueryCacheTTL sets the check cache query TTL
-func WithCheckQueryCacheTTL(ttl time.Duration) ListObjectsQueryOption {
- return func(d *ListObjectsQuery) {
- d.checkQueryCacheTTL = ttl
- }
-}
-
-// WithCheckCache sets the cache used for resolve check results
-func WithCheckCache(checkCache *ccache.Cache[*graph.CachedResolveCheckResponse]) ListObjectsQueryOption {
+func WithCheckOptions(checkOptions []graph.LocalCheckerOption) ListObjectsQueryOption {
return func(d *ListObjectsQuery) {
- d.checkCache = checkCache
+ d.checkOptions = checkOptions
}
}
@@ -128,8 +116,6 @@ func NewListObjectsQuery(ds storage.RelationshipTupleReader, opts ...ListObjects
resolveNodeLimit: defaultResolveNodeLimit,
resolveNodeBreadthLimit: defaultResolveNodeBreadthLimit,
maxConcurrentReads: defaultMaxConcurrentReads,
- checkCache: nil,
- checkQueryCacheTTL: defaultCheckQueryCacheTTL,
}
for _, opt := range opts {
@@ -248,24 +234,7 @@ func (q *ListObjectsQuery) evaluate(
close(connectedObjectsResChan)
}()
- var checkResolver graph.CheckResolver
- if q.checkCache == nil {
- checkResolver = graph.NewLocalChecker(
- storagewrappers.NewCombinedTupleReader(q.datastore, req.GetContextualTuples().GetTupleKeys()),
- graph.WithResolveNodeBreadthLimit(q.resolveNodeBreadthLimit),
- graph.WithMaxConcurrentReads(q.maxConcurrentReads),
- )
- } else {
- checkResolver = graph.NewLocalChecker(
- storagewrappers.NewCombinedTupleReader(q.datastore, req.GetContextualTuples().GetTupleKeys()),
- graph.WithResolveNodeBreadthLimit(q.resolveNodeBreadthLimit),
- graph.WithMaxConcurrentReads(q.maxConcurrentReads),
- graph.WithCachedResolver(
- graph.WithExistingCache(q.checkCache),
- graph.WithCacheTTL(q.checkQueryCacheTTL),
- ),
- )
- }
+ checkResolver := graph.NewLocalChecker(storagewrappers.NewCombinedTupleReader(q.datastore, req.GetContextualTuples().GetTupleKeys()), q.checkOptions...)
defer checkResolver.Close()
concurrencyLimiterCh := make(chan struct{}, q.resolveNodeBreadthLimit)
diff --git a/pkg/server/server.go b/pkg/server/server.go
index 20c0831e..5d8d34f6 100644
--- a/pkg/server/server.go
+++ b/pkg/server/server.go
@@ -90,6 +90,7 @@ type Server struct {
typesystemResolver typesystem.TypesystemResolverFunc
+ checkOptions []graph.LocalCheckerOption
checkQueryCacheEnabled bool
checkQueryCacheLimit uint32
checkQueryCacheTTL time.Duration
@@ -248,6 +249,10 @@ func NewServerWithOpts(opts ...OpenFGAServiceV1Option) (*Server, error) {
for _, opt := range opts {
opt(s)
}
+ s.checkOptions = []graph.LocalCheckerOption{
+ graph.WithResolveNodeBreadthLimit(s.resolveNodeBreadthLimit),
+ graph.WithMaxConcurrentReads(s.maxConcurrentReadsForCheck),
+ }
if slices.Contains(s.experimentals, ExperimentalCheckQueryCache) && s.checkQueryCacheEnabled {
s.logger.Info("Check query cache is enabled and may lead to stale query results up to the configured query cache TTL",
@@ -256,6 +261,12 @@ func NewServerWithOpts(opts ...OpenFGAServiceV1Option) (*Server, error) {
s.checkCache = ccache.New(
ccache.Configure[*graph.CachedResolveCheckResponse]().MaxSize(int64(s.checkQueryCacheLimit)),
)
+ s.checkOptions = append(s.checkOptions,
+ graph.WithCachedResolver(
+ graph.WithExistingCache(s.checkCache),
+ graph.WithCacheTTL(s.checkQueryCacheTTL),
+ ),
+ )
}
if s.datastore == nil {
@@ -292,8 +303,7 @@ func (s *Server) ListObjects(ctx context.Context, req *openfgav1.ListObjectsRequ
commands.WithResolveNodeLimit(s.resolveNodeLimit),
commands.WithResolveNodeBreadthLimit(s.resolveNodeBreadthLimit),
commands.WithMaxConcurrentReads(s.maxConcurrentReadsForListObjects),
- commands.WithCheckCache(s.checkCache),
- commands.WithCheckQueryCacheTTL(s.checkQueryCacheTTL),
+ commands.WithCheckOptions(s.checkOptions),
)
return q.Execute(
@@ -332,8 +342,7 @@ func (s *Server) StreamedListObjects(req *openfgav1.StreamedListObjectsRequest,
commands.WithResolveNodeLimit(s.resolveNodeLimit),
commands.WithResolveNodeBreadthLimit(s.resolveNodeBreadthLimit),
commands.WithMaxConcurrentReads(s.maxConcurrentReadsForListObjects),
- commands.WithCheckCache(s.checkCache),
- commands.WithCheckQueryCacheTTL(s.checkQueryCacheTTL),
+ commands.WithCheckOptions(s.checkOptions),
)
req.AuthorizationModelId = typesys.GetAuthorizationModelID() // the resolved model id
@@ -414,24 +423,7 @@ func (s *Server) Check(ctx context.Context, req *openfgav1.CheckRequest) (*openf
ctx = typesystem.ContextWithTypesystem(ctx, typesys)
- var checkResolver graph.CheckResolver
- if s.checkCache == nil {
- checkResolver = graph.NewLocalChecker(
- storagewrappers.NewCombinedTupleReader(s.datastore, req.ContextualTuples.GetTupleKeys()),
- graph.WithResolveNodeBreadthLimit(s.resolveNodeBreadthLimit),
- graph.WithMaxConcurrentReads(s.maxConcurrentReadsForCheck),
- )
- } else {
- checkResolver = graph.NewLocalChecker(
- storagewrappers.NewCombinedTupleReader(s.datastore, req.ContextualTuples.GetTupleKeys()),
- graph.WithResolveNodeBreadthLimit(s.resolveNodeBreadthLimit),
- graph.WithMaxConcurrentReads(s.maxConcurrentReadsForCheck),
- graph.WithCachedResolver(
- graph.WithExistingCache(s.checkCache),
- graph.WithCacheTTL(s.checkQueryCacheTTL),
- ),
- )
- }
+ checkResolver := graph.NewLocalChecker(storagewrappers.NewCombinedTupleReader(s.datastore, req.ContextualTuples.GetTupleKeys()), s.checkOptions...)
defer checkResolver.Close()
resp, err := checkResolver.ResolveCheck(ctx, &graph.ResolveCheckRequest{
diff --git a/pkg/server/test/list_objects.go b/pkg/server/test/list_objects.go
index 5972cd62..25e603b5 100644
--- a/pkg/server/test/list_objects.go
+++ b/pkg/server/test/list_objects.go
@@ -243,8 +243,11 @@ func TestListObjectsRespectsMaxResults(t *testing.T, ds storage.OpenFGADatastore
defer checkCache.Stop()
opts = append(opts,
- commands.WithCheckCache(checkCache),
- commands.WithCheckQueryCacheTTL(10*time.Second))
+ commands.WithCheckOptions([]graph.LocalCheckerOption{
+ graph.WithCachedResolver(
+ graph.WithExistingCache(checkCache),
+ graph.WithCacheTTL(10*time.Second)),
+ }))
}
listObjectsQuery := commands.NewListObjectsQuery(datastore, opts...)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment