Merge p1-w1-ta-design-criticals: Critical IndexSystem lock races + TrackRepository.Update silent-create

DeepDrftContent.Services/FileDatabase/Services/IndexSystem.cs

@@ -57,7 +57,7 @@ public abstract class IndexDirectory : AbstractIndexContainer
 
     public int GetIndexSize() => Index.GetEntriesSize();
 
-    public bool HasIndexEntry(string entryId) => Index.HasEntry(entryId);
+    public virtual Task<bool> HasIndexEntry(string entryId) => Task.FromResult(Index.HasEntry(entryId));
 }
 
 /// <summary>
@@ -66,17 +66,26 @@ public abstract class IndexDirectory : AbstractIndexContainer
 public class DirectoryIndexDirectory : IndexDirectory
 {
     private readonly IDirectoryIndex _directoryIndex;
+    private readonly SemaphoreSlim _indexLock = new(1, 1);
 
-    public DirectoryIndexDirectory(string rootPath, IDirectoryIndex index, IIndexDataFactory? indexDataFactory = null) 
-        : base(rootPath, IndexType.Directory, index, indexDataFactory) 
+    public DirectoryIndexDirectory(string rootPath, IDirectoryIndex index, IIndexDataFactory? indexDataFactory = null)
+        : base(rootPath, IndexType.Directory, index, indexDataFactory)
     {
         _directoryIndex = index;
     }
 
     protected async Task AddToIndexAsync(string entryId)
     {
-        _directoryIndex.PutEntry(entryId);
-        await SaveIndexAsync(_directoryIndex);
+        await _indexLock.WaitAsync();
+        try
+        {
+            _directoryIndex.PutEntry(entryId);
+            await SaveIndexAsync(_directoryIndex);
+        }
+        finally
+        {
+            _indexLock.Release();
+        }
     }
 }
 
@@ -86,7 +95,7 @@ public class DirectoryIndexDirectory : IndexDirectory
 public class VaultIndexDirectory : IndexDirectory
 {
     private IVaultIndex _vaultIndex;
-    private readonly object _indexLock = new();
+    private readonly SemaphoreSlim _indexLock = new(1, 1);
     private readonly IndexFactoryService _factoryService = new();
 
     public VaultIndexDirectory(string rootPath, IVaultIndex index, IIndexDataFactory? indexDataFactory = null)
@@ -97,11 +106,16 @@ public class VaultIndexDirectory : IndexDirectory
 
     protected async Task AddToIndexAsync(string entryId, MetaData metaData)
     {
-        lock (_indexLock)
+        await _indexLock.WaitAsync();
+        try
         {
             _vaultIndex.PutEntry(entryId, metaData);
+            await SaveIndexAsync(_vaultIndex);
+        }
+        finally
+        {
+            _indexLock.Release();
         }
-        await SaveIndexAsync(_vaultIndex);
     }
 
     /// <summary>
@@ -109,16 +123,14 @@ public class VaultIndexDirectory : IndexDirectory
     /// </summary>
     public async Task ReloadIndexAsync()
     {
+        await _indexLock.WaitAsync();
         try
         {
             var newIndex = await _factoryService.LoadIndexAsync(IndexType.Vault, RootPath);
             if (newIndex is IVaultIndex vaultIndex)
             {
-                lock (_indexLock)
-                {
-                    _vaultIndex = vaultIndex;
-                    Index = vaultIndex;
-                }
+                _vaultIndex = vaultIndex;
+                Index = vaultIndex;
                 Console.WriteLine($"VaultIndexDirectory: Reloaded index for {RootPath}, {vaultIndex.GetEntriesSize()} entries");
             }
         }
@@ -126,27 +138,41 @@ public class VaultIndexDirectory : IndexDirectory
         {
             Console.WriteLine($"VaultIndexDirectory: Failed to reload index for {RootPath}: {ex.Message}");
         }
+        finally
+        {
+            _indexLock.Release();
+        }
     }
 
     /// <summary>
     /// Thread-safe check for index entry
     /// </summary>
-    public new bool HasIndexEntry(string entryId)
+    public override async Task<bool> HasIndexEntry(string entryId)
     {
-        lock (_indexLock)
+        await _indexLock.WaitAsync();
+        try
         {
             return _vaultIndex.HasEntry(entryId);
         }
+        finally
+        {
+            _indexLock.Release();
+        }
     }
 
     /// <summary>
     /// Thread-safe get entry metadata
     /// </summary>
-    public MetaData? GetEntryMetadata(string entryId)
+    public async Task<MetaData?> GetEntryMetadata(string entryId)
     {
-        lock (_indexLock)
+        await _indexLock.WaitAsync();
+        try
         {
             return _vaultIndex.GetEntry(entryId);
         }
+        finally
+        {
+            _indexLock.Release();
+        }
     }
 }

DeepDrftContent.Services/FileDatabase/Services/MediaVault.cs

@@ -64,11 +64,11 @@ public abstract class MediaVault : VaultIndexDirectory
         var vaultType = MediaVaultTypeMap.GetVaultType<T>();
 
         // Use thread-safe method from VaultIndexDirectory
-        if (!HasIndexEntry(entryId))
+        if (!await HasIndexEntry(entryId))
             return null;
 
         // Use thread-safe metadata retrieval
-        var metaData = GetEntryMetadata(entryId);
+        var metaData = await GetEntryMetadata(entryId);
         if (metaData == null)
             return null;
 

DeepDrftTests/FileDatabaseTests.cs

@@ -92,7 +92,7 @@ public class FileDatabaseTests
         // Assert
         var vault = _fileDatabase.GetVault(TestData.TestKeys.ImageVaultKey);
         Assert.That(vault, Is.Not.Null, "Vault should not be null");
-        Assert.That(vault!.HasIndexEntry(TestData.TestKeys.TestImageEntry), Is.True, 
+        Assert.That(await vault!.HasIndexEntry(TestData.TestKeys.TestImageEntry), Is.True,
             "Added image should be in the index");
     }
 
@@ -182,7 +182,7 @@ public class FileDatabaseTests
         Assert.That(reloadedDatabase.GetIndexSize(), Is.EqualTo(1), "Index count should be 1");
         
         // Verify vault exists
-        Assert.That(reloadedDatabase.HasIndexEntry(TestData.TestKeys.ImageVaultKey), Is.True, 
+        Assert.That(await reloadedDatabase.HasIndexEntry(TestData.TestKeys.ImageVaultKey), Is.True,
             "Vault should be present in index");
         Assert.That(reloadedDatabase.HasVault(TestData.TestKeys.ImageVaultKey), Is.True, 
             "Vault should be present in vault collection");

DeepDrftTests/IndexSystemTests.cs

@@ -492,13 +492,13 @@ public class IndexSystemTests
             await database!.CreateVaultAsync(testVaultKey, MediaVaultType.Image);
 
             // Assert
-            Assert.That(database.HasIndexEntry(testVaultKey), Is.True, "Should contain added entry");
+            Assert.That(await database.HasIndexEntry(testVaultKey), Is.True, "Should contain added entry");
             Assert.That(database.GetIndexSize(), Is.EqualTo(1), "Should have one entry");
             Assert.That(File.Exists(IndexPath), Is.True, "Should persist to file");
 
             // Verify persistence by reloading database
             var reloadedDatabase = await FileDatabase.FromAsync(TestDirectory);
-            Assert.That(reloadedDatabase!.HasIndexEntry(testVaultKey), Is.True, "Should persist entry across restarts");
+            Assert.That(await reloadedDatabase!.HasIndexEntry(testVaultKey), Is.True, "Should persist entry across restarts");
         }
 
         [Test]
@@ -516,13 +516,13 @@ public class IndexSystemTests
             await vault!.AddEntryAsync(testKey, testImage);
 
             // Assert
-            Assert.That(vault.HasIndexEntry(testKey), Is.True, "Should contain added entry");
+            Assert.That(await vault.HasIndexEntry(testKey), Is.True, "Should contain added entry");
             Assert.That(vault.GetIndexSize(), Is.EqualTo(1), "Should have one entry");
             Assert.That(File.Exists(IndexPath), Is.True, "Should persist to file");
 
             // Verify persistence by reloading vault
             var reloadedVault = await ImageVault.FromAsync(TestDirectory);
-            Assert.That(reloadedVault!.HasIndexEntry(testKey), Is.True, "Should persist entry across restarts");
+            Assert.That(await reloadedVault!.HasIndexEntry(testKey), Is.True, "Should persist entry across restarts");
         }
     }
 }

DeepDrftTests/MediaVaultTests.cs

@@ -120,7 +120,7 @@ public class MediaVaultTests
             await _imageVault.AddEntryAsync(entryKey, imageBinary);
 
             // Assert
-            Assert.That(_imageVault.HasIndexEntry(entryKey), Is.True, "Should add to index");
+            Assert.That(await _imageVault.HasIndexEntry(entryKey), Is.True, "Should add to index");
             
             var expectedFilePath = Path.Combine(TestDirectory, "test-image.png");
             AssertMediaFileExists(expectedFilePath, imageBinary.Buffer);
@@ -148,7 +148,7 @@ public class MediaVaultTests
             
             foreach (var (key, binary) in entries)
             {
-                Assert.That(_imageVault.HasIndexEntry(key), Is.True, $"Should contain {key} in index");
+                Assert.That(await _imageVault.HasIndexEntry(key), Is.True, $"Should contain {key} in index");
                 var expectedFilePath = Path.Combine(TestDirectory, $"{key}.png");
                 AssertMediaFileExists(expectedFilePath, binary.Buffer);
             }
@@ -264,7 +264,7 @@ public class MediaVaultTests
             await _audioVault.AddEntryAsync(entryKey, audioBinary);
 
             // Assert
-            Assert.That(_audioVault.HasIndexEntry(entryKey), Is.True, "Should add to index");
+            Assert.That(await _audioVault.HasIndexEntry(entryKey), Is.True, "Should add to index");
             
             var expectedFilePath = Path.Combine(TestDirectory, "test-audio.mp3");
             AssertMediaFileExists(expectedFilePath, audioBinary.Buffer);
@@ -337,7 +337,7 @@ public class MediaVaultTests
                 return (string)method!.Invoke(_vault, new object[] { mediaKey })!;
             }
 
-            public bool HasIndexEntry(string entryId) => _vault.HasIndexEntry(entryId);
+            public Task<bool> HasIndexEntry(string entryId) => _vault.HasIndexEntry(entryId);
             public Task AddEntryAsync(string entryId, FileBinary media) => 
                 _vault.AddEntryAsync(entryId, media);
         }
@@ -423,7 +423,7 @@ public class MediaVaultTests
             await vault!.AddEntryAsync(entryKey, imageBinary);
 
             // Assert
-            Assert.That(vault.HasIndexEntry(entryKey), Is.True, "Should add entry to index");
+            Assert.That(await vault.HasIndexEntry(entryKey), Is.True, "Should add entry to index");
             
             var expectedFilePath = Path.Combine(TestDirectory, "test-media.png");
             AssertMediaFileExists(expectedFilePath, imageBinary.Buffer);

DeepDrftWeb.Services/Repositories/TrackRepository.cs

@@ -50,7 +50,7 @@ public class TrackRepository
 
         if (trackEntity == null)
         {
-            return await Create(track);
+            throw new InvalidOperationException($"Track not found: {track.Id}");
         }
         
         trackEntity.Album = track.Album;