diff --git a/README.md b/README.md index 5969d25..ca74858 100644 --- a/README.md +++ b/README.md @@ -873,6 +873,8 @@ IReadOnlyList indexes = orders.ListIndexes(); orders.DropIndex("idx_status"); ``` +A unique index rejects a second document with the same value (`InvalidOperationException`); `DynamicCollection` indexes an explicit BSON `null` and rejects a second one, whereas typed collections do not index `null` keys, so any number of documents may hold `null` in a unique-indexed property. + ### Reading BsonDocument fields ```csharp diff --git a/src/BLite.Core/Collections/DocumentCollection.cs b/src/BLite.Core/Collections/DocumentCollection.cs index 19d77c2..a94b8da 100644 --- a/src/BLite.Core/Collections/DocumentCollection.cs +++ b/src/BLite.Core/Collections/DocumentCollection.cs @@ -2194,6 +2194,9 @@ private async Task InsertDataCore(TId id, T entity, byte[] docData, ITransaction : default; // ──────────────────────────────────────────────────────────────────── + // Unique secondary indexes are checked before any page is written. + _indexManager.EnsureUniqueInAll(entity, null, transaction); + DocumentLocation location; if (_isTimeSeries) { @@ -2782,6 +2785,10 @@ private async Task UpdateAtLocationCore(TId id, T entity, byte[] docData, var oldEntity = await FindByLocation(oldLocation, transaction); if (oldEntity == null) return false; + // Unique secondary indexes are checked before any page is written; only those whose + // key changed, so documents that already share a value can still be updated. + _indexManager.EnsureUniqueInAll(entity, oldEntity, transaction); + // Read old page var pageBuffer = ArrayPool.Shared.Rent(_storage.PageSize); try diff --git a/src/BLite.Core/Indexing/CollectionIndexManager.cs b/src/BLite.Core/Indexing/CollectionIndexManager.cs index d465365..863388a 100644 --- a/src/BLite.Core/Indexing/CollectionIndexManager.cs +++ b/src/BLite.Core/Indexing/CollectionIndexManager.cs @@ -25,6 +25,9 @@ public sealed class CollectionIndexManager : IDisposable where T : class private readonly string _collectionName; private CollectionMetadata _metadata; private volatile IReadOnlyList? _cachedIndexInfo; + // Whether any index is a unique BTree index, so EnsureUniqueInAll can skip the loop. + // Refreshed under _lock whenever _indexes changes. + private volatile bool _hasUniqueBTreeIndex; [RequiresDynamicCode("Index creation compiles key selector expressions using Expression.Compile() which requires dynamic code generation.")] [RequiresUnreferencedCode("Index creation uses reflection (Expression.PropertyOrField) to access type members. Ensure all entity types and their members are preserved.")] @@ -54,10 +57,20 @@ public CollectionIndexManager(StorageEngine storage, IDocumentMapper map rootAllocated |= index.RootPageId != idxMeta.RootPageId; } + RefreshHasUniqueBTreeIndex(); + if (rootAllocated) SaveMetadata(); } + private void RefreshHasUniqueBTreeIndex() + { + var any = false; + foreach (var index in _indexes.Values) + any |= index.IsUniqueBTree; + _hasUniqueBTreeIndex = any; + } + private void UpdateMetadata() { _metadata.Indexes.Clear(); @@ -101,6 +114,7 @@ public CollectionSecondaryIndex CreateIndex(CollectionIndexDefinition var secondaryIndex = new CollectionSecondaryIndex(definition, _storage, _mapper); _indexes[definition.Name] = secondaryIndex; _cachedIndexInfo = null; // invalidate cache + RefreshHasUniqueBTreeIndex(); // Persist metadata UpdateMetadata(); @@ -279,6 +293,7 @@ public bool DropIndex(string name) index.Dispose(); _indexes.Remove(name); _cachedIndexInfo = null; // invalidate cache + RefreshHasUniqueBTreeIndex(); SaveMetadata(); // Save metadata after dropping index return true; @@ -307,6 +322,7 @@ internal CollectionSecondaryIndex RecreateIndex(string name) }; _indexes[name] = fresh; _cachedIndexInfo = null; // invalidate cache + RefreshHasUniqueBTreeIndex(); // Persist the new root before freeing the old tree: a crash in between leaks the old // pages, whereas the reverse order would let the next open free them a second time. @@ -453,6 +469,32 @@ public void InsertIntoAll(T document, DocumentLocation location, ITransaction tr } } + /// + /// Throws if would violate any unique index. + /// Call before any page is written, so a rejected write leaves nothing behind. + /// On update, pass the stored version as : only indexes whose + /// key changed are checked, so documents that already share a value can still be updated. + /// + /// Document about to be inserted or written by an update + /// Stored version of the document on update; null on insert + /// Transaction context + public void EnsureUniqueInAll(T document, T? oldDocument, ITransaction transaction) + { + if (document == null) + throw new ArgumentNullException(nameof(document)); + + if (!_hasUniqueBTreeIndex) + return; + + lock (_lock) + { + foreach (var index in _indexes.Values) + { + index.EnsureUnique(document, oldDocument, transaction); + } + } + } + /// /// Updates a document in all indexes /// @@ -599,6 +641,7 @@ public void Dispose() } _indexes.Clear(); + _hasUniqueBTreeIndex = false; _disposed = true; } diff --git a/src/BLite.Core/Indexing/CollectionSecondaryIndex.cs b/src/BLite.Core/Indexing/CollectionSecondaryIndex.cs index 0afe4b9..e6c4ce1 100644 --- a/src/BLite.Core/Indexing/CollectionSecondaryIndex.cs +++ b/src/BLite.Core/Indexing/CollectionSecondaryIndex.cs @@ -26,6 +26,11 @@ public sealed class CollectionSecondaryIndex : IDisposable, ICollectionI private static readonly IndexKey s_maxBoundaryKey; // 16 bytes of 0xFF covers any GUID or ObjectId suffix in composite keys. private static readonly IndexKey s_maxIdBoundaryKey; + // Id types whose index key always has the same length (used by EnsureUnique). + private static readonly bool s_fixedSizeId = + typeof(TId) == typeof(ObjectId) || typeof(TId) == typeof(Guid) || + typeof(TId) == typeof(int) || typeof(TId) == typeof(long) || + typeof(TId) == typeof(double) || typeof(TId) == typeof(decimal); static CollectionSecondaryIndex() { @@ -403,6 +408,68 @@ public CollectionIndexInfo GetInfo() }; } + /// + /// True for a unique BTree index, the only kind checks. + /// + internal bool IsUniqueBTree => _definition.IsUnique && _btreeIndex != null; + + /// + /// Throws if this is a unique BTree index and another document already holds + /// 's key. The document's own entry (same id) is ignored. + /// On update, is the stored version: if its key equals the + /// new one, keeps the existing entry and nothing is checked, so a + /// document that already shares its value with another can still be updated. + /// Entries are stored as [user key][document id], so the BTree's own unique + /// check never sees two equal keys; the collection calls this before writing any page. + /// + internal void EnsureUnique(T document, T? oldDocument, ITransaction? transaction) + { + if (!IsUniqueBTree) + return; + + if (!_definition.TryGetKey(document, out var keyValue) || keyValue == null) + return; // null keys are not indexed, so they never conflict + + var userKey = ConvertToIndexKey(keyValue); + + if (oldDocument != null + && _definition.TryGetKey(oldDocument, out var oldKeyValue) && oldKeyValue != null + && (Equals(oldKeyValue, keyValue) || ConvertToIndexKey(oldKeyValue).Data.SequenceEqual(userKey.Data))) + return; // key unchanged + + // Every id key starts with its type's prefix byte, so the entries of this value lie in + // [user key][prefix] .. [user key][prefix + 1]. Bounding the scan there skips the entries + // of longer values starting with this one (e.g. "10", "11", ... when checking "1"). + var ownIdKey = _mapper.ToIndexKey(_mapper.GetId(document)); + var idPrefix = ownIdKey.Data[0]; + var minComposite = CreateIdPrefixBoundary(userKey, idPrefix); + var maxComposite = CreateIdPrefixBoundary(userKey, (byte)(idPrefix + 1)); + + foreach (var entry in _btreeIndex!.Range(minComposite, maxComposite, IndexDirection.Forward, transaction?.TransactionId)) + { + var idPart = entry.Key.Data.Slice(userKey.Data.Length); + if (idPart.SequenceEqual(ownIdKey.Data)) + continue; // this document's own entry + + // A longer value whose next byte is the prefix byte itself still falls in the range + // (e.g. "ab\u0002c" when checking "ab"); for fixed-size ids its id part has the wrong length. + if (s_fixedSizeId && idPart.Length != ownIdKey.Data.Length) + continue; + + throw new InvalidOperationException( + $"Duplicate key violation for unique index '{_definition.Name}'"); + } + } + + // [user key][one byte]: a range bound between the entries of user key and those of longer values. + private static IndexKey CreateIdPrefixBoundary(IndexKey userKey, byte idPrefixBound) + { + var bytes = new byte[userKey.Data.Length + 1]; + userKey.Data.CopyTo(bytes); + bytes[^1] = idPrefixBound; + return IndexKey.FromOwnedArray(bytes); + } + #region Composite Key Support (SQLite-style for Duplicate Keys) /// diff --git a/tests/BLite.Tests/UniqueIndexTests.cs b/tests/BLite.Tests/UniqueIndexTests.cs new file mode 100644 index 0000000..c468c36 --- /dev/null +++ b/tests/BLite.Tests/UniqueIndexTests.cs @@ -0,0 +1,226 @@ +using BLite.Core; +using BLite.Core.Query; +using BLite.Shared; + +namespace BLite.Tests; + +/// +/// Regression tests for issue #176: unique secondary indexes on typed collections accepted +/// duplicate values. Typed indexes store [user key][document id], so the BTree's own +/// unique check never fired; the collection now checks the user key before writing. +/// +public class UniqueIndexTests : IDisposable +{ + private readonly string _dbPath; + private readonly string _dynamicDbPath; + private readonly TestDbContext _db; + + public UniqueIndexTests() + { + var id = Guid.NewGuid(); + _dbPath = Path.Combine(Path.GetTempPath(), $"unique_index_{id}.db"); + _dynamicDbPath = Path.Combine(Path.GetTempPath(), $"unique_index_dyn_{id}.db"); + _db = new TestDbContext(_dbPath); + } + + public void Dispose() + { + _db.Dispose(); + foreach (var path in new[] { _dbPath, _dynamicDbPath }) + { + if (File.Exists(path)) File.Delete(path); + var wal = Path.ChangeExtension(path, ".wal"); + if (File.Exists(wal)) File.Delete(wal); + } + } + + [Fact] + public async Task Typed_Unique_Index_Rejects_Duplicates() + { + await _db.PeopleV2.CreateIndexAsync(p => p.Name, "idx_name", unique: true); + await _db.PeopleV2.InsertAsync(new PersonV2 { Name = "dup", Age = 1 }); + + await Assert.ThrowsAsync( + () => _db.PeopleV2.InsertAsync(new PersonV2 { Name = "dup", Age = 2 }).AsTask()); + + // The rejected insert leaves nothing behind. + Assert.Equal(1, await _db.PeopleV2.CountAsync()); + var matches = await _db.PeopleV2.AsQueryable().Where(p => p.Name == "dup").ToListAsync(); + Assert.Single(matches); + Assert.Equal(1, matches[0].Age); + } + + [Fact] + public async Task Dynamic_Unique_Index_Rejects_Duplicates() + { + using var engine = new BLiteEngine(_dynamicDbPath); + var col = engine.GetOrCreateCollection("people"); + await col.CreateIndexAsync("name", unique: true); + await col.InsertAsync(col.CreateDocument(["name"], b => b.AddString("name", "dup"))); + + await Assert.ThrowsAsync( + () => col.InsertAsync(col.CreateDocument(["name"], b => b.AddString("name", "dup"))).AsTask()); + } + + [Fact] + public async Task Dynamic_Unique_Index_Rejects_A_Second_Explicit_Null() + { + // Documented difference: DynamicCollection indexes an explicit BSON null, typed collections + // do not index null keys (see Typed_Unique_Index_Ignores_Null_Values). + using var engine = new BLiteEngine(_dynamicDbPath); + var col = engine.GetOrCreateCollection("people"); + await col.CreateIndexAsync("name", unique: true); + await col.InsertAsync(col.CreateDocument(["name"], b => b.AddNull("name"))); + + await Assert.ThrowsAsync( + () => col.InsertAsync(col.CreateDocument(["name"], b => b.AddNull("name"))).AsTask()); + } + + [Fact] + public async Task Typed_Unique_Index_Rejects_Update_Onto_Existing_Value() + { + await _db.PeopleV2.CreateIndexAsync(p => p.Name, "idx_name", unique: true); + await _db.PeopleV2.InsertAsync(new PersonV2 { Name = "alice", Age = 1 }); + var bobId = await _db.PeopleV2.InsertAsync(new PersonV2 { Name = "bob", Age = 2 }); + + var bob = (await _db.PeopleV2.FindByIdAsync(bobId))!; + bob.Name = "alice"; + + await Assert.ThrowsAsync( + () => _db.PeopleV2.UpdateAsync(bob).AsTask()); + + // The rejected update leaves the document and its index entry unchanged. + var stored = await _db.PeopleV2.FindByIdAsync(bobId); + Assert.Equal("bob", stored!.Name); + Assert.Single(await _db.PeopleV2.AsQueryable().Where(p => p.Name == "bob").ToListAsync()); + Assert.Single(await _db.PeopleV2.AsQueryable().Where(p => p.Name == "alice").ToListAsync()); + } + + [Fact] + public async Task Typed_Unique_Index_Allows_Update_That_Keeps_Or_Frees_A_Value() + { + await _db.PeopleV2.CreateIndexAsync(p => p.Name, "idx_name", unique: true); + var aliceId = await _db.PeopleV2.InsertAsync(new PersonV2 { Name = "alice", Age = 1 }); + + // Same key, other fields changed: the document's own entry is not a conflict. + var alice = (await _db.PeopleV2.FindByIdAsync(aliceId))!; + alice.Age = 42; + Assert.True(await _db.PeopleV2.UpdateAsync(alice)); + + // Moving the value away frees it for another document. + alice.Name = "alicia"; + Assert.True(await _db.PeopleV2.UpdateAsync(alice)); + await _db.PeopleV2.InsertAsync(new PersonV2 { Name = "alice", Age = 2 }); + + Assert.Equal(2, await _db.PeopleV2.CountAsync()); + } + + [Fact] + public async Task Typed_Unique_Index_Allows_Updates_Of_Documents_That_Already_Share_A_Value() + { + // Duplicates written before the unique index existed: the backfill does not reject them. + var firstId = await _db.PeopleV2.InsertAsync(new PersonV2 { Name = "dup", Age = 1 }); + var secondId = await _db.PeopleV2.InsertAsync(new PersonV2 { Name = "dup", Age = 2 }); + await _db.PeopleV2.InsertAsync(new PersonV2 { Name = "carol", Age = 3 }); + await _db.PeopleV2.CreateIndexAsync(p => p.Name, "idx_name", unique: true); + + // No-op update and upsert. + var first = (await _db.PeopleV2.FindByIdAsync(firstId))!; + Assert.True(await _db.PeopleV2.UpdateAsync(first)); + await _db.PeopleV2.UpsertAsync(first); + + // Update of another field. + var second = (await _db.PeopleV2.FindByIdAsync(secondId))!; + second.Age = 42; + Assert.True(await _db.PeopleV2.UpdateAsync(second)); + Assert.Equal(42, (await _db.PeopleV2.FindByIdAsync(secondId))!.Age); + + // Changing the key onto a value another document holds is still rejected. + second.Name = "carol"; + await Assert.ThrowsAsync( + () => _db.PeopleV2.UpdateAsync(second).AsTask()); + Assert.Equal("dup", (await _db.PeopleV2.FindByIdAsync(secondId))!.Name); + + Assert.Equal(2, (await _db.PeopleV2.AsQueryable().Where(p => p.Name == "dup").ToListAsync()).Count); + } + + [Fact] + public async Task Typed_Unique_Index_Separates_Values_Followed_By_Low_Control_Characters() + { + // The check scans [value][id prefix byte] .. [value][id prefix byte + 1]; longer values + // whose next byte is just below, at or above the prefix (0x02) sit right at those bounds. + await _db.PeopleV2.CreateIndexAsync(p => p.Name, "idx_name", unique: true); + string[] names = ["ab\u0001", "ab\u0002", "ab\u0002x", "ab\u0003", "abc", "a", "ab"]; + foreach (var name in names) + await _db.PeopleV2.InsertAsync(new PersonV2 { Name = name, Age = 1 }); + + foreach (var name in names) + { + await Assert.ThrowsAsync( + () => _db.PeopleV2.InsertAsync(new PersonV2 { Name = name, Age = 2 }).AsTask()); + } + + Assert.Equal(names.Length, await _db.PeopleV2.CountAsync()); + } + + [Fact] + public async Task Typed_Unique_Index_Allows_Value_Again_After_Delete() + { + await _db.PeopleV2.CreateIndexAsync(p => p.Name, "idx_name", unique: true); + var id = await _db.PeopleV2.InsertAsync(new PersonV2 { Name = "dup", Age = 1 }); + await _db.PeopleV2.DeleteAsync(id); + + await _db.PeopleV2.InsertAsync(new PersonV2 { Name = "dup", Age = 2 }); + + Assert.Equal(1, await _db.PeopleV2.CountAsync()); + } + + [Fact] + public async Task Typed_Unique_Index_Does_Not_Confuse_Values_Sharing_A_Prefix() + { + await _db.PeopleV2.CreateIndexAsync(p => p.Name, "idx_name", unique: true); + await _db.PeopleV2.InsertAsync(new PersonV2 { Name = "abc", Age = 1 }); + await _db.PeopleV2.InsertAsync(new PersonV2 { Name = "ab", Age = 2 }); + await _db.PeopleV2.InsertAsync(new PersonV2 { Name = "a", Age = 3 }); + + Assert.Equal(3, await _db.PeopleV2.CountAsync()); + } + + [Fact] + public async Task Typed_Unique_Index_With_String_Ids_Rejects_Duplicates_But_Not_Prefixes() + { + await _db.StringEntities.CreateIndexAsync(e => e.Value, "idx_value", unique: true); + await _db.StringEntities.InsertAsync(new StringEntity { Id = "1", Value = "abc" }); + await _db.StringEntities.InsertAsync(new StringEntity { Id = "2", Value = "ab" }); + + await Assert.ThrowsAsync( + () => _db.StringEntities.InsertAsync(new StringEntity { Id = "3", Value = "ab" }).AsTask()); + + Assert.Equal(2, await _db.StringEntities.CountAsync()); + } + + [Fact] + public async Task Typed_Unique_Index_Ignores_Null_Values() + { + await _db.StringEntities.CreateIndexAsync(e => e.Value, "idx_value", unique: true); + await _db.StringEntities.InsertAsync(new StringEntity { Id = "1", Value = null }); + await _db.StringEntities.InsertAsync(new StringEntity { Id = "2", Value = null }); + + Assert.Equal(2, await _db.StringEntities.CountAsync()); + } + + [Fact] + public async Task Typed_Unique_Index_Rejects_Duplicates_Within_A_Bulk_Insert() + { + await _db.PeopleV2.CreateIndexAsync(p => p.Name, "idx_name", unique: true); + + await Assert.ThrowsAsync(() => _db.PeopleV2.InsertBulkAsync( + [ + new PersonV2 { Name = "x", Age = 1 }, + new PersonV2 { Name = "dup", Age = 2 }, + new PersonV2 { Name = "dup", Age = 3 }, + ]).AsTask()); + + Assert.Equal(0, await _db.PeopleV2.CountAsync()); + } +}