Created
August 17, 2023 05:26
-
-
Save miparnisari/d833b97ba94e8d57ae50e6e8404ca4d6 to your computer and use it in GitHub Desktop.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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