chore(services): store observed counts in permissionsServer, add LR/LS flags by barakmich · Pull Request #2929 · authzed/spicedb

barakmich · 2026-02-28T00:36:12Z

Description

Add the "lr" and "ls" strings as inputs for --experimental-query-plan. Adds testing for the gRPC routes along with the existing consistency tests. And in so doing, found another bug: we want to combine the caveat both from the wildcard, and the relationship itself, which may or may not have one

Testing

References

codecov · 2026-02-28T00:41:48Z

Codecov Report

❌ Patch coverage is 59.93976% with 133 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.82%. Comparing base (0298415) to head (6518157).

Files with missing lines	Patch %	Lines
internal/services/v1/permissions_queryplan.go	51.77%	97 Missing and 26 partials ⚠️
pkg/query/datastore.go	85.11%	7 Missing ⚠️
internal/services/v1/permissions.go	57.15%	2 Missing and 1 partial ⚠️

❌ Your project check has failed because the head coverage (74.82%) is below the target coverage (75.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2929      +/-   ##
==========================================
- Coverage   74.88%   74.82%   -0.05%     
==========================================
  Files         498      498              
  Lines       60736    61041     +305     
==========================================
+ Hits        45474    45669     +195     
- Misses      12108    12182      +74     
- Partials     3154     3190      +36

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

internal/services/v1/permissions_queryplan.go

pkg/query/intersection_test.go

miparnisari · 2026-03-17T18:37:49Z

internal/services/integrationtesting/consistency_test.go

+// validateQueryPlanLookupSubjects checks that for each resource × subject-type
+// pair the query plan handler returns at least the subjects the accessibility
+// set defines as directly accessible. Wildcard exclusion checking is omitted
+// because the query plan handler does not yet populate ExcludedSubjects.


TIL. is there a ticket tracking this work?

There is now

miparnisari · 2026-03-17T19:06:23Z

pkg/query/intersection_arrow_test.go

+		ds, err := memdb.NewMemdbDatastore(0, 0, memdb.DisableGC)
+		require.NoError(err)
+		revision, err := ds.ReadWriteTx(context.Background(), func(ctx context.Context, tx datastore.ReadWriteTransaction) error {
+			return nil
+		})
+		require.NoError(err)
+		ctx := NewLocalContext(context.Background(),
+			WithRevisionedReader(datalayer.NewDataLayer(ds).SnapshotReader(revision)))


Please, here and below, use a mock instead:

ctrl := gomock.NewController(t) defer ctrl.Finish() ds := mock_datalayer.NewMockRevisionedReader(ctrl) ctx := NewLocalContext(context.Background(), WithRevisionedReader(ds))

also, i'm surprised that the mock is never invoked. And i'm not just talking about this test, i'm saying that in every test here where i did this change, the mock is never invoked. Is this right?

Why make a mock when we can just build a real one?

lots of reasons:

mocks are way faster

mocks are way easier to control and inspect (see my observation above)

if the signature of NewMemdbDatastore or of ReadWriteTx changes, you don't have to change anything here

(1) In the grand scheme of things, it's a couple extra milliseconds

(3) If the signature changes, I feel I actively want to change something here, to make sure I'm using it right

As for (2), a matter of taste.

But, I wonder if I can't obviate it entirely

But, I wonder if I can't obviate it entirely

yes, this is what i pointed out

pkg/query/datastore.go

miparnisari · 2026-03-17T22:16:47Z

internal/services/integrationtesting/query_plan_consistency_test.go

 	t.Run("lookup_resources", func(t *testing.T) {
 		if os.Getenv("TEST_QUERY_PLAN_RESOURCES") == "" {
-			t.Skip("Skipping IterResources tests: set TEST_QUERY_PLAN_RESOURCES=true to enable")
+			t.Skip("Skipping IterResources tests due to deprectation: Set TEST_QUERY_PLAN_RESOURCES=true to enable")


i'm confused; what deprecation?

…nd LS

github-actions bot added area/api v1 Affects the v1 API area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools) labels Feb 28, 2026

barakmich force-pushed the barakmich/ps_counts branch from e95f9b9 to 0bd1e09 Compare March 2, 2026 18:05

miparnisari reviewed Mar 3, 2026

View reviewed changes

internal/services/v1/permissions_queryplan.go Show resolved Hide resolved

barakmich force-pushed the barakmich/ps_counts branch from 0bd1e09 to 01dfa21 Compare March 16, 2026 17:12

barakmich marked this pull request as ready for review March 16, 2026 17:13

barakmich requested a review from a team as a code owner March 16, 2026 17:13

barakmich changed the title ~~chore(services): store observed counts in permissionsServer~~ chore(services): store observed counts in permissionsServer, add LR/LS flags Mar 16, 2026

github-actions bot added the area/cli Affects the command line label Mar 16, 2026

miparnisari reviewed Mar 17, 2026

View reviewed changes

pkg/query/intersection_test.go Outdated Show resolved Hide resolved

miparnisari reviewed Mar 17, 2026

View reviewed changes

barakmich force-pushed the barakmich/ps_counts branch 2 times, most recently from 5914507 to 4fc7de4 Compare March 17, 2026 18:59

miparnisari reviewed Mar 17, 2026

View reviewed changes

pkg/query/datastore.go Outdated Show resolved Hide resolved

miparnisari reviewed Mar 17, 2026

View reviewed changes

barakmich force-pushed the barakmich/ps_counts branch from 4fc7de4 to dc23bb5 Compare March 17, 2026 22:23

barakmich added 9 commits March 18, 2026 09:23

chore(services): store observed counts in permissionsServer

252c083

chore: update the --experimental-query-plan flag to also support LR a…

9bd04e1

…nd LS

fix: changelog

5398e84

test: cover some of the new entry points

40e857c

test: stop sharing contexts between tests

133dd71

test: add gRPC-based testing for query planner

7b803ef

fix: wildcard caveat non-propagation

5f72ab0

fix: longer timeout and race condition fix

62cc56c

test: keep removing unneccessary datastores

c2f94b1

barakmich force-pushed the barakmich/ps_counts branch from dc23bb5 to c2f94b1 Compare March 18, 2026 16:24

fix: fix build of optimizers on rebase

6518157

miparnisari approved these changes Mar 18, 2026

View reviewed changes

miparnisari merged commit ea6c5f2 into main Mar 18, 2026
43 of 45 checks passed

miparnisari deleted the barakmich/ps_counts branch March 18, 2026 17:23

github-actions bot locked and limited conversation to collaborators Mar 18, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(services): store observed counts in permissionsServer, add LR/LS flags#2929

chore(services): store observed counts in permissionsServer, add LR/LS flags#2929
miparnisari merged 10 commits intomainfrom
barakmich/ps_counts

barakmich commented Feb 28, 2026 •

edited

Loading

Uh oh!

codecov bot commented Feb 28, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

miparnisari Mar 17, 2026

Uh oh!

barakmich Mar 17, 2026

Uh oh!

miparnisari Mar 17, 2026 •

edited

Loading

Uh oh!

barakmich Mar 17, 2026

Uh oh!

miparnisari Mar 17, 2026

Uh oh!

barakmich Mar 17, 2026 •

edited

Loading

Uh oh!

barakmich Mar 17, 2026

Uh oh!

miparnisari Mar 17, 2026

Uh oh!

Uh oh!

miparnisari Mar 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

barakmich commented Feb 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing

References

Uh oh!

codecov bot commented Feb 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

miparnisari Mar 17, 2026

Choose a reason for hiding this comment

Uh oh!

barakmich Mar 17, 2026

Choose a reason for hiding this comment

Uh oh!

miparnisari Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

barakmich Mar 17, 2026

Choose a reason for hiding this comment

Uh oh!

miparnisari Mar 17, 2026

Choose a reason for hiding this comment

Uh oh!

barakmich Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

barakmich Mar 17, 2026

Choose a reason for hiding this comment

Uh oh!

miparnisari Mar 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

miparnisari Mar 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

barakmich commented Feb 28, 2026 •

edited

Loading

codecov bot commented Feb 28, 2026 •

edited

Loading

miparnisari Mar 17, 2026 •

edited

Loading

barakmich Mar 17, 2026 •

edited

Loading