Skip to content

Commit 2d125bf

Browse files
committed
Refs: #59 Implement cross-bucket snapshot candidate query fix and add related tests
1 parent 973ee89 commit 2d125bf

12 files changed

Lines changed: 173 additions & 4 deletions

File tree

Changelog.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@
1010
- added tests and CI support for Oracle Database (using Oracle XE in docker)
1111
- Research and enable Oracle integration test infrastructure [#58](https://github.com/NEventStore/NEventStore.Persistence.SQL/issues/58)
1212

13+
### BugFix
14+
15+
- Fix cross-bucket contamination in snapshot candidate query [#59](https://github.com/NEventStore/NEventStore.Persistence.SQL/issues/59)
16+
1317
### Breaking Changes
1418

1519
- Minimum NEventStore version is now 10.2.0.

docs/Project-Analysis-Issue-Drafts.md

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,3 +257,25 @@ Validation:
257257

258258
- `dotnet test .\src\NEventStore.Persistence.Oracle.Tests\NEventStore.Persistence.Oracle.Core.Tests.csproj -c Release --no-build -f net8.0`
259259
- Result: 137 passed, 0 failed, 0 skipped.
260+
261+
### Issue #59: Cross-Bucket Snapshot Candidate Query
262+
263+
Status: completed on 2026-06-11.
264+
265+
Snapshot candidate queries now stay bucket-scoped. `GetStreamsRequiringSnapshots` filters `Commits` by the requested bucket and joins `Snapshots` by both bucket and stream id, preventing a snapshot in bucket `b` from hiding or changing eligibility for the same stream id in bucket `a`.
266+
267+
Implementation notes:
268+
269+
- Added sync and async regression tests under `src/NEventStore.Persistence.Sql.Tests/` and linked them into all provider test projects.
270+
- The regression creates the same stream id in buckets `a` and `b`, adds a bucket `b` snapshot, and verifies bucket `a` still returns its snapshot candidate with `SnapshotRevision == 0`.
271+
- Applied the SQL fix to common provider SQL and Oracle SQL now that Oracle support is available.
272+
273+
Validation:
274+
275+
- Test-first check: SQLite focused regression failed before the SQL fix, then passed after it.
276+
- `dotnet test .\src\NEventStore.Persistence.Sqlite.Tests\NEventStore.Persistence.Sqlite.Core.Tests.csproj -c Release --no-build -f net8.0`
277+
- Result: 141 passed, 0 failed, 0 skipped.
278+
- `dotnet test .\src\NEventStore.Persistence.Oracle.Tests\NEventStore.Persistence.Oracle.Core.Tests.csproj -c Release -f net8.0 --no-build`
279+
- Result: 141 passed, 0 failed, 0 skipped.
280+
- `dotnet build .\src\NEventStore.Persistence.Sql.Core.sln -c Release --no-restore /p:ContinuousIntegrationBuild=true -m:1 -nr:false`
281+
- Result: passed.

src/NEventStore.Persistence.MsSql.Tests/NEventStore.Persistence.MsSql.Core.Tests.csproj

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@
4747
<Compile Include="..\..\dependencies\NEventStore\src\NEventStore.Persistence.AcceptanceTests\PersistenceTests.Async.cs" Link="PersistenceTests.Async.cs" />
4848
<Compile Include="..\NEventStore.Persistence.Sql.Tests\PersistenceTests.Transactions.cs" Link="PersistenceTests.Transactions.cs" />
4949
<Compile Include="..\NEventStore.Persistence.Sql.Tests\PersistenceTests.Transactions.Async.cs" Link="PersistenceTests.Transactions.Async.cs" />
50+
<Compile Include="..\NEventStore.Persistence.Sql.Tests\PersistenceTests.Snapshots.cs" Link="PersistenceTests.Snapshots.cs" />
51+
<Compile Include="..\NEventStore.Persistence.Sql.Tests\PersistenceTests.Snapshots.Async.cs" Link="PersistenceTests.Snapshots.Async.cs" />
5052
</ItemGroup>
5153

5254
<ItemGroup>

src/NEventStore.Persistence.MySql.Tests/NEventStore.Persistence.MySql.Core.Tests.csproj

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@
4646
<ItemGroup>
4747
<Compile Include="..\..\dependencies\NEventStore\src\NEventStore.Persistence.AcceptanceTests\PersistenceTests.cs" Link="PersistenceTests.cs" />
4848
<Compile Include="..\..\dependencies\NEventStore\src\NEventStore.Persistence.AcceptanceTests\PersistenceTests.Async.cs" Link="PersistenceTests.Async.cs" />
49+
<Compile Include="..\NEventStore.Persistence.Sql.Tests\PersistenceTests.Snapshots.cs" Link="PersistenceTests.Snapshots.cs" />
50+
<Compile Include="..\NEventStore.Persistence.Sql.Tests\PersistenceTests.Snapshots.Async.cs" Link="PersistenceTests.Snapshots.Async.cs" />
4951
</ItemGroup>
5052

5153
<ItemGroup>

src/NEventStore.Persistence.Oracle.Tests/NEventStore.Persistence.Oracle.Core.Tests.csproj

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@
4949
<ItemGroup>
5050
<Compile Include="..\..\dependencies\NEventStore\src\NEventStore.Persistence.AcceptanceTests\PersistenceTests.cs" Link="PersistenceTests.cs" />
5151
<Compile Include="..\..\dependencies\NEventStore\src\NEventStore.Persistence.AcceptanceTests\PersistenceTests.Async.cs" Link="PersistenceTests.Async.cs" />
52+
<Compile Include="..\NEventStore.Persistence.Sql.Tests\PersistenceTests.Snapshots.cs" Link="PersistenceTests.Snapshots.cs" />
53+
<Compile Include="..\NEventStore.Persistence.Sql.Tests\PersistenceTests.Snapshots.Async.cs" Link="PersistenceTests.Snapshots.Async.cs" />
5254
</ItemGroup>
5355

5456
<ItemGroup>

src/NEventStore.Persistence.PostgreSql.Tests/NEventStore.Persistence.PostgreSql.Core.Tests.csproj

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@
4343
<ItemGroup>
4444
<Compile Include="..\..\dependencies\NEventStore\src\NEventStore.Persistence.AcceptanceTests\PersistenceTests.cs" Link="PersistenceTests.cs" />
4545
<Compile Include="..\..\dependencies\NEventStore\src\NEventStore.Persistence.AcceptanceTests\PersistenceTests.Async.cs" Link="PersistenceTests.Async.cs" />
46+
<Compile Include="..\NEventStore.Persistence.Sql.Tests\PersistenceTests.Snapshots.cs" Link="PersistenceTests.Snapshots.cs" />
47+
<Compile Include="..\NEventStore.Persistence.Sql.Tests\PersistenceTests.Snapshots.Async.cs" Link="PersistenceTests.Snapshots.Async.cs" />
4648
</ItemGroup>
4749

4850
<ItemGroup>

src/NEventStore.Persistence.Sql.Tests/NEventStore.Persistence.Sql.Core.Tests.csproj

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
<ItemGroup>
1111
<Compile Remove="PersistenceTests.Transactions.Async.cs" />
1212
<Compile Remove="PersistenceTests.Transactions.cs" />
13+
<Compile Remove="PersistenceTests.Snapshots.Async.cs" />
14+
<Compile Remove="PersistenceTests.Snapshots.cs" />
1315
</ItemGroup>
1416

1517
<ItemGroup>
@@ -19,6 +21,8 @@
1921
<ItemGroup>
2022
<None Include="PersistenceTests.Transactions.Async.cs" />
2123
<None Include="PersistenceTests.Transactions.cs" />
24+
<None Include="PersistenceTests.Snapshots.Async.cs" />
25+
<None Include="PersistenceTests.Snapshots.cs" />
2226
</ItemGroup>
2327

2428
<ItemGroup>
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
#pragma warning disable IDE1006 // Naming Styles
2+
3+
using FluentAssertions;
4+
using NEventStore.Persistence.AcceptanceTests.BDD;
5+
6+
#if MSTEST
7+
using Microsoft.VisualStudio.TestTools.UnitTesting;
8+
#endif
9+
#if NUNIT
10+
using NUnit.Framework;
11+
#endif
12+
#if XUNIT
13+
using Xunit;
14+
#endif
15+
16+
namespace NEventStore.Persistence.AcceptanceTests.Async
17+
{
18+
#if MSTEST
19+
[TestClass]
20+
#endif
21+
public class when_getting_streams_to_snapshot_for_a_bucket_that_shares_stream_ids_with_other_buckets : PersistenceEngineConcernAsync
22+
{
23+
private const string BucketAId = "a";
24+
private const string BucketBId = "b";
25+
private const int Threshold = 2;
26+
private string? _streamId;
27+
private StreamHeadObserver? _observer;
28+
29+
protected override async Task ContextAsync()
30+
{
31+
_streamId = Guid.NewGuid().ToString();
32+
33+
var bucketAFirst = _streamId.BuildAttempt(bucketId: BucketAId);
34+
await Persistence.CommitAsync(bucketAFirst, CancellationToken.None);
35+
36+
var bucketASecond = bucketAFirst.BuildNextAttempt();
37+
await Persistence.CommitAsync(bucketASecond, CancellationToken.None);
38+
39+
await Persistence.CommitAsync(_streamId.BuildAttempt(bucketId: BucketBId), CancellationToken.None);
40+
await Persistence.AddSnapshotAsync(new Snapshot(BucketBId, _streamId, bucketASecond.StreamRevision, "SnapshotB"), CancellationToken.None);
41+
}
42+
43+
protected override async Task BecauseAsync()
44+
{
45+
_observer = new StreamHeadObserver();
46+
await Persistence.GetStreamsToSnapshotAsync(BucketAId, Threshold, _observer, CancellationToken.None);
47+
}
48+
49+
[Fact]
50+
public void should_return_only_bucket_a_stream_heads()
51+
{
52+
_observer!.StreamHeads.Should().ContainSingle(x => x.BucketId == BucketAId && x.StreamId == _streamId);
53+
_observer.StreamHeads.Should().NotContain(x => x.BucketId == BucketBId);
54+
}
55+
56+
[Fact]
57+
public void should_not_let_a_bucket_b_snapshot_hide_bucket_a_snapshot_eligibility()
58+
{
59+
_observer!.StreamHeads.Should().ContainSingle(x => x.StreamId == _streamId && x.SnapshotRevision == 0);
60+
}
61+
}
62+
}
63+
64+
#pragma warning restore IDE1006 // Naming Styles
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
#pragma warning disable IDE1006 // Naming Styles
2+
3+
using FluentAssertions;
4+
using NEventStore.Persistence.AcceptanceTests.BDD;
5+
6+
#if MSTEST
7+
using Microsoft.VisualStudio.TestTools.UnitTesting;
8+
#endif
9+
#if NUNIT
10+
using NUnit.Framework;
11+
#endif
12+
#if XUNIT
13+
using Xunit;
14+
#endif
15+
16+
namespace NEventStore.Persistence.AcceptanceTests
17+
{
18+
#if MSTEST
19+
[TestClass]
20+
#endif
21+
public class when_getting_streams_to_snapshot_for_a_bucket_that_shares_stream_ids_with_other_buckets : PersistenceEngineConcern
22+
{
23+
private const string BucketAId = "a";
24+
private const string BucketBId = "b";
25+
private const int Threshold = 2;
26+
private string? _streamId;
27+
private IStreamHead[]? _streamsToSnapshot;
28+
29+
protected override void Context()
30+
{
31+
_streamId = Guid.NewGuid().ToString();
32+
33+
var bucketAFirst = _streamId.BuildAttempt(bucketId: BucketAId);
34+
Persistence.Commit(bucketAFirst);
35+
36+
var bucketASecond = bucketAFirst.BuildNextAttempt();
37+
Persistence.Commit(bucketASecond);
38+
39+
Persistence.Commit(_streamId.BuildAttempt(bucketId: BucketBId));
40+
Persistence.AddSnapshot(new Snapshot(BucketBId, _streamId, bucketASecond.StreamRevision, "SnapshotB"));
41+
}
42+
43+
protected override void Because()
44+
{
45+
_streamsToSnapshot = Persistence.GetStreamsToSnapshot(BucketAId, Threshold).ToArray();
46+
}
47+
48+
[Fact]
49+
public void should_return_only_bucket_a_stream_heads()
50+
{
51+
_streamsToSnapshot.Should().ContainSingle(x => x.BucketId == BucketAId && x.StreamId == _streamId);
52+
_streamsToSnapshot.Should().NotContain(x => x.BucketId == BucketBId);
53+
}
54+
55+
[Fact]
56+
public void should_not_let_a_bucket_b_snapshot_hide_bucket_a_snapshot_eligibility()
57+
{
58+
_streamsToSnapshot.Should().ContainSingle(x => x.StreamId == _streamId && x.SnapshotRevision == 0);
59+
}
60+
}
61+
}
62+
63+
#pragma warning restore IDE1006 // Naming Styles

src/NEventStore.Persistence.Sql/SqlDialects/CommonSqlStatements.resx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -183,9 +183,10 @@ AND NOT EXISTS
183183
<value>SELECT C.BucketId, C.StreamId, C.StreamIdOriginal, MAX(C.StreamRevision) AS StreamRevision, MAX(COALESCE(S.StreamRevision, 0)) AS SnapshotRevision
184184
FROM Commits AS C
185185
LEFT OUTER JOIN Snapshots AS S
186-
ON C.BucketId = @BucketId
186+
ON S.BucketId = C.BucketId
187187
AND C.StreamId = S.StreamId
188188
AND C.StreamRevision &gt;= S.StreamRevision
189+
WHERE C.BucketId = @BucketId
189190
GROUP BY C.StreamId, C.BucketId, C.StreamIdOriginal
190191
HAVING MAX(C.StreamRevision) &gt;= MAX(COALESCE(S.StreamRevision, 0)) + @Threshold
191192
AND C.StreamId &gt; @StreamId
@@ -240,4 +241,4 @@ ORDER BY CheckpointNumber
240241
ORDER BY CheckpointNumber
241242
LIMIT @Limit OFFSET @Skip;</value>
242243
</data>
243-
</root>
244+
</root>

0 commit comments

Comments
 (0)