Skip to content

Commit 5914507

Browse files
barakmichbarakmich
committed
fix: longer timeout and race condition fix
1 parent 5ace6b9 commit 5914507

File tree

6 files changed

+101
-46
lines changed

6 files changed

+101
-46
lines changed

‎pkg/query/alias_test.go‎

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,11 @@ import (
99
func TestAliasIterator(t *testing.T) {
1010
t.Parallel()
1111

12-
// Create test context
13-
ctx := NewLocalContext(t.Context())
14-
1512
t.Run("Check_BasicRelationRewriting", func(t *testing.T) {
1613
t.Parallel()
1714
require := require.New(t)
1815

16+
ctx := NewLocalContext(t.Context())
1917
// Create a sub-iterator with document relations
2018
subIt := NewDocumentAccessFixedIterator()
2119

@@ -47,6 +45,7 @@ func TestAliasIterator(t *testing.T) {
4745
t.Parallel()
4846
require := require.New(t)
4947

48+
ctx := NewLocalContext(t.Context())
5049
// Create an empty sub-iterator since we only want to test self-edge detection
5150
subIt := NewEmptyFixedIterator()
5251

@@ -79,6 +78,7 @@ func TestAliasIterator(t *testing.T) {
7978
t.Parallel()
8079
require := require.New(t)
8180

81+
ctx := NewLocalContext(t.Context())
8282
// Create a sub-iterator
8383
subIt := NewSingleUserFixedIterator("bob")
8484

@@ -103,6 +103,7 @@ func TestAliasIterator(t *testing.T) {
103103
t.Parallel()
104104
require := require.New(t)
105105

106+
ctx := NewLocalContext(t.Context())
106107
subIt := NewDocumentAccessFixedIterator()
107108
aliasIt := NewAliasIterator("access", subIt)
108109

@@ -130,6 +131,7 @@ func TestAliasIterator(t *testing.T) {
130131
t.Parallel()
131132
require := require.New(t)
132133

134+
ctx := NewLocalContext(t.Context())
133135
subIt := NewDocumentAccessFixedIterator()
134136
aliasIt := NewAliasIterator("permission", subIt)
135137

@@ -151,6 +153,7 @@ func TestAliasIterator(t *testing.T) {
151153
t.Parallel()
152154
require := require.New(t)
153155

156+
ctx := NewLocalContext(t.Context())
154157
subIt := NewDocumentAccessFixedIterator()
155158
aliasIt := NewAliasIterator("can_view", subIt)
156159

@@ -170,6 +173,7 @@ func TestAliasIterator(t *testing.T) {
170173
t.Parallel()
171174
require := require.New(t)
172175

176+
ctx := NewLocalContext(t.Context())
173177
// Create an empty sub-iterator since we only want to test self-edge detection
174178
subIt := NewEmptyFixedIterator()
175179

@@ -202,6 +206,7 @@ func TestAliasIterator(t *testing.T) {
202206
t.Parallel()
203207
require := require.New(t)
204208

209+
ctx := NewLocalContext(t.Context())
205210
subIt := NewEmptyFixedIterator()
206211
aliasIt := NewAliasIterator("empty_alias", subIt)
207212

@@ -217,6 +222,7 @@ func TestAliasIterator(t *testing.T) {
217222
t.Parallel()
218223
require := require.New(t)
219224

225+
ctx := NewLocalContext(t.Context())
220226
// Create empty sub-iterator
221227
subIt := NewEmptyFixedIterator()
222228
aliasIt := NewAliasIterator("self", subIt)
@@ -238,6 +244,7 @@ func TestAliasIterator(t *testing.T) {
238244
t.Parallel()
239245
require := require.New(t)
240246

247+
ctx := NewLocalContext(t.Context())
241248
subIt := NewEmptyFixedIterator()
242249
aliasIt := NewAliasIterator("owner", subIt)
243250

@@ -320,13 +327,11 @@ func TestAliasIteratorExplain(t *testing.T) {
320327
func TestAliasIteratorErrorHandling(t *testing.T) {
321328
t.Parallel()
322329

323-
// Create test context
324-
ctx := NewLocalContext(t.Context())
325-
326330
t.Run("Check_SubIteratorError", func(t *testing.T) {
327331
t.Parallel()
328332
require := require.New(t)
329333

334+
ctx := NewLocalContext(t.Context())
330335
// Create a faulty sub-iterator
331336
faultyIt := NewFaultyIterator(true, false, ObjectType{}, []ObjectType{})
332337
aliasIt := NewAliasIterator("error_test", faultyIt)
@@ -339,6 +344,7 @@ func TestAliasIteratorErrorHandling(t *testing.T) {
339344
t.Parallel()
340345
require := require.New(t)
341346

347+
ctx := NewLocalContext(t.Context())
342348
// Create a faulty sub-iterator that fails during collection
343349
faultyIt := NewFaultyIterator(false, true, ObjectType{}, []ObjectType{})
344350
aliasIt := NewAliasIterator("collection_error_test", faultyIt)
@@ -355,6 +361,7 @@ func TestAliasIteratorErrorHandling(t *testing.T) {
355361
t.Parallel()
356362
require := require.New(t)
357363

364+
ctx := NewLocalContext(t.Context())
358365
// Create a faulty sub-iterator that fails on IterSubjectsImpl
359366
faultyIt := NewFaultyIterator(true, false, ObjectType{}, []ObjectType{})
360367
aliasIt := NewAliasIterator("error_test", faultyIt)
@@ -368,6 +375,7 @@ func TestAliasIteratorErrorHandling(t *testing.T) {
368375
t.Parallel()
369376
require := require.New(t)
370377

378+
ctx := NewLocalContext(t.Context())
371379
// Create a faulty sub-iterator that fails during collection
372380
faultyIt := NewFaultyIterator(false, true, ObjectType{}, []ObjectType{})
373381
aliasIt := NewAliasIterator("collection_error_test", faultyIt)
@@ -385,6 +393,7 @@ func TestAliasIteratorErrorHandling(t *testing.T) {
385393
t.Parallel()
386394
require := require.New(t)
387395

396+
ctx := NewLocalContext(t.Context())
388397
// Create a faulty sub-iterator that fails on IterResourcesImpl
389398
faultyIt := NewFaultyIterator(true, false, ObjectType{}, []ObjectType{})
390399
aliasIt := NewAliasIterator("error_test", faultyIt)
@@ -398,6 +407,7 @@ func TestAliasIteratorErrorHandling(t *testing.T) {
398407
t.Parallel()
399408
require := require.New(t)
400409

410+
ctx := NewLocalContext(t.Context())
401411
// Create a faulty sub-iterator that fails during collection
402412
faultyIt := NewFaultyIterator(false, true, ObjectType{}, []ObjectType{})
403413
aliasIt := NewAliasIterator("collection_error_test", faultyIt)
@@ -415,6 +425,7 @@ func TestAliasIteratorErrorHandling(t *testing.T) {
415425
t.Parallel()
416426
require := require.New(t)
417427

428+
ctx := NewLocalContext(t.Context())
418429
// Create a faulty sub-iterator that errors on CheckImpl
419430
faultyIt := NewFaultyIterator(true, false, ObjectType{}, []ObjectType{})
420431
aliasIt := NewAliasIterator("self", faultyIt)
@@ -431,6 +442,7 @@ func TestAliasIteratorErrorHandling(t *testing.T) {
431442
t.Parallel()
432443
require := require.New(t)
433444

445+
ctx := NewLocalContext(t.Context())
434446
// Create a faulty sub-iterator that fails during collection
435447
faultyIt := NewFaultyIterator(false, true, ObjectType{}, []ObjectType{})
436448
aliasIt := NewAliasIterator("self", faultyIt)
@@ -450,13 +462,11 @@ func TestAliasIteratorErrorHandling(t *testing.T) {
450462
func TestAliasIteratorAdvancedScenarios(t *testing.T) {
451463
t.Parallel()
452464

453-
// Create test context
454-
ctx := NewLocalContext(t.Context())
455-
456465
t.Run("Check_MultipleResourcesSelfEdgeWithSubResults", func(t *testing.T) {
457466
t.Parallel()
458467
require := require.New(t)
459468

469+
ctx := NewLocalContext(t.Context())
460470
// Create sub-iterator with real data
461471
subIt := NewDocumentAccessFixedIterator()
462472
aliasIt := NewAliasIterator("admin", subIt)
@@ -497,6 +507,7 @@ func TestAliasIteratorAdvancedScenarios(t *testing.T) {
497507
t.Parallel()
498508
require := require.New(t)
499509

510+
ctx := NewLocalContext(t.Context())
500511
// Create empty sub-iterator to isolate self-edge logic
501512
subIt := NewEmptyFixedIterator()
502513
aliasIt := NewAliasIterator("owner", subIt)
@@ -525,6 +536,7 @@ func TestAliasIteratorAdvancedScenarios(t *testing.T) {
525536
t.Parallel()
526537
require := require.New(t)
527538

539+
ctx := NewLocalContext(t.Context())
528540
subIt := NewEmptyFixedIterator()
529541
aliasIt := NewAliasIterator("empty_relation", subIt)
530542

@@ -540,6 +552,7 @@ func TestAliasIteratorAdvancedScenarios(t *testing.T) {
540552
t.Parallel()
541553
require := require.New(t)
542554

555+
ctx := NewLocalContext(t.Context())
543556
subIt := NewEmptyFixedIterator()
544557
aliasIt := NewAliasIterator("empty_relation", subIt)
545558

@@ -555,6 +568,7 @@ func TestAliasIteratorAdvancedScenarios(t *testing.T) {
555568
t.Parallel()
556569
require := require.New(t)
557570

571+
ctx := NewLocalContext(t.Context())
558572
// Use iterator with many results
559573
subIt := NewLargeFixedIterator()
560574
aliasIt := NewAliasIterator("massive", subIt)
@@ -579,6 +593,7 @@ func TestAliasIteratorAdvancedScenarios(t *testing.T) {
579593
t.Parallel()
580594
require := require.New(t)
581595

596+
ctx := NewLocalContext(t.Context())
582597
subIt := NewDocumentAccessFixedIterator()
583598
original := NewAliasIterator("original", subIt)
584599

@@ -615,6 +630,7 @@ func TestAliasIteratorAdvancedScenarios(t *testing.T) {
615630
t.Parallel()
616631
require := require.New(t)
617632

633+
ctx := NewLocalContext(t.Context())
618634
subIt := NewDocumentAccessFixedIterator()
619635
aliasIt := NewAliasIterator("early_test", subIt)
620636

@@ -638,6 +654,7 @@ func TestAliasIteratorAdvancedScenarios(t *testing.T) {
638654
t.Parallel()
639655
require := require.New(t)
640656

657+
ctx := NewLocalContext(t.Context())
641658
subIt := NewDocumentAccessFixedIterator()
642659
aliasIt := NewAliasIterator("admin", subIt)
643660

@@ -663,6 +680,7 @@ func TestAliasIteratorAdvancedScenarios(t *testing.T) {
663680
t.Parallel()
664681
require := require.New(t)
665682

683+
ctx := NewLocalContext(t.Context())
666684
subIt := NewDocumentAccessFixedIterator()
667685
aliasIt := NewAliasIterator("early_subjects", subIt)
668686

@@ -686,6 +704,7 @@ func TestAliasIteratorAdvancedScenarios(t *testing.T) {
686704
t.Parallel()
687705
require := require.New(t)
688706

707+
ctx := NewLocalContext(t.Context())
689708
subIt := NewDocumentAccessFixedIterator()
690709
aliasIt := NewAliasIterator("early_resources", subIt)
691710

@@ -709,6 +728,7 @@ func TestAliasIteratorAdvancedScenarios(t *testing.T) {
709728
t.Parallel()
710729
require := require.New(t)
711730

731+
ctx := NewLocalContext(t.Context())
712732
// Use sub-iterator that will return paths that need rewriting
713733
subIt := NewDocumentAccessFixedIterator()
714734
aliasIt := NewAliasIterator("admin", subIt)

‎pkg/query/context.go‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package query
22

33
import (
44
"context"
5+
"sync"
56

67
"github.com/authzed/spicedb/internal/caveats"
78
"github.com/authzed/spicedb/pkg/datalayer"
@@ -39,6 +40,7 @@ type Context struct {
3940
// IterSubjects call on this context. Nil means not yet set.
4041
// Used to wrap only the top-level call with DeduplicatePathSeq.
4142
topLevelIterator Iterator
43+
topLevelOnce sync.Once
4244
}
4345

4446
// NewLocalContext creates a new query execution context with a LocalExecutor.
@@ -243,10 +245,11 @@ func (ctx *Context) IterSubjects(it Iterator, resource Object, filterSubjectType
243245
return nil, spiceerrors.MustBugf("no executor has been set")
244246
}
245247

246-
isTopLevel := ctx.topLevelIterator == nil
247-
if isTopLevel {
248+
isTopLevel := false
249+
ctx.topLevelOnce.Do(func() {
248250
ctx.topLevelIterator = it
249-
}
251+
isTopLevel = true
252+
})
250253

251254
tracedIterator := ctx.traceEnterIfEnabled(it, iterSubjectsTraceString(resource, filterSubjectType))
252255
key := it.CanonicalKey()
@@ -275,10 +278,11 @@ func (ctx *Context) IterResources(it Iterator, subject ObjectAndRelation, filter
275278
return nil, spiceerrors.MustBugf("no executor has been set")
276279
}
277280

278-
isTopLevel := ctx.topLevelIterator == nil
279-
if isTopLevel {
281+
isTopLevel := false
282+
ctx.topLevelOnce.Do(func() {
280283
ctx.topLevelIterator = it
281-
}
284+
isTopLevel = true
285+
})
282286

283287
tracedIterator := ctx.traceEnterIfEnabled(it, iterResourcesTraceString(subject, filterResourceType))
284288
key := it.CanonicalKey()

‎pkg/query/intersection_arrow_test.go‎

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -276,21 +276,18 @@ func TestIntersectionArrowIteratorCaveatCombination(t *testing.T) {
276276

277277
require := require.New(t)
278278

279-
// Create test context
280-
ds, err := memdb.NewMemdbDatastore(0, 0, memdb.DisableGC)
281-
require.NoError(err)
282-
283-
revision, err := ds.ReadWriteTx(context.Background(), func(ctx context.Context, tx datastore.ReadWriteTransaction) error {
284-
return nil
285-
})
286-
require.NoError(err)
287-
288-
ctx := NewLocalContext(context.Background(),
289-
WithRevisionedReader(datalayer.NewDataLayer(ds).SnapshotReader(revision)))
290-
291279
t.Run("CombineTwoCaveats_AND_Logic", func(t *testing.T) {
292280
t.Parallel()
293281

282+
ds, err := memdb.NewMemdbDatastore(0, 0, memdb.DisableGC)
283+
require.NoError(err)
284+
revision, err := ds.ReadWriteTx(context.Background(), func(ctx context.Context, tx datastore.ReadWriteTransaction) error {
285+
return nil
286+
})
287+
require.NoError(err)
288+
ctx := NewLocalContext(context.Background(),
289+
WithRevisionedReader(datalayer.NewDataLayer(ds).SnapshotReader(revision)))
290+
294291
// Left side path with caveat
295292
leftPath := MustPathFromString("document:doc1#team@team:team1")
296293
leftPath.Caveat = &core.CaveatExpression{
@@ -345,6 +342,15 @@ func TestIntersectionArrowIteratorCaveatCombination(t *testing.T) {
345342
t.Run("LeftCaveat_Right_NoCaveat", func(t *testing.T) {
346343
t.Parallel()
347344

345+
ds, err := memdb.NewMemdbDatastore(0, 0, memdb.DisableGC)
346+
require.NoError(err)
347+
revision, err := ds.ReadWriteTx(context.Background(), func(ctx context.Context, tx datastore.ReadWriteTransaction) error {
348+
return nil
349+
})
350+
require.NoError(err)
351+
ctx := NewLocalContext(context.Background(),
352+
WithRevisionedReader(datalayer.NewDataLayer(ds).SnapshotReader(revision)))
353+
348354
// Left side path with caveat
349355
leftPath := MustPathFromString("document:doc1#team@team:team1")
350356
leftPath.Caveat = &core.CaveatExpression{
@@ -390,6 +396,15 @@ func TestIntersectionArrowIteratorCaveatCombination(t *testing.T) {
390396
t.Run("MultiplePaths_CombineCaveats_AND_Logic", func(t *testing.T) {
391397
t.Parallel()
392398

399+
ds, err := memdb.NewMemdbDatastore(0, 0, memdb.DisableGC)
400+
require.NoError(err)
401+
revision, err := ds.ReadWriteTx(context.Background(), func(ctx context.Context, tx datastore.ReadWriteTransaction) error {
402+
return nil
403+
})
404+
require.NoError(err)
405+
ctx := NewLocalContext(context.Background(),
406+
WithRevisionedReader(datalayer.NewDataLayer(ds).SnapshotReader(revision)))
407+
393408
// Left side: document has multiple teams, each with different caveats
394409
leftPath1 := MustPathFromString("document:doc1#team@team:team1")
395410
leftPath1.Caveat = &core.CaveatExpression{

0 commit comments

Comments
 (0)