diff --git a/src/GameCult.Caching/CultCache.cs b/src/GameCult.Caching/CultCache.cs index 48bad3a3..8934aec9 100644 --- a/src/GameCult.Caching/CultCache.cs +++ b/src/GameCult.Caching/CultCache.cs @@ -5132,7 +5132,7 @@ namespace GameCult.Caching var attributes = FileInfo.Attributes; if ((int)attributes == -1) return null; - if ((attributes & FileAttributes.Directory) != 0 && (attributes & FileAttributes.ReparsePoint) == 0) + if ((attributes & FileAttributes.Directory) != 0 && Directory.Exists(FileInfo.FullName)) throw new IOException("The store path is a directory, not a store file."); try { @@ -5162,9 +5162,19 @@ namespace GameCult.Caching return (int)attributes != -1 && (attributes & FileAttributes.ReparsePoint) != 0; } + // A write never goes through a link at the store path: replacing it would silently detach the store from its link. Every + // write takes the lock first, so this is the one place a link is refused, before the lock file or anything else is made. + private void RefuseLinkedStore() + { + if (PathIsALink()) + throw new CultStoreLinkedException( + $"The store path {FileInfo.FullName} is a link; a write is refused so it does not replace the link. Configure the link's target as the store path to write to it."); + } + // The lock is a sidecar opened exclusively, which excludes other handles in this process and in others alike. private FileStream? AcquireLock(bool wait) { + RefuseLinkedStore(); Directory.CreateDirectory(FileInfo.DirectoryName!); var lockPath = FileInfo.FullName + ".lock"; var started = Stopwatch.StartNew(); diff --git a/src/GameCult.Caching/CultStoreLinkedException.cs b/src/GameCult.Caching/CultStoreLinkedException.cs new file mode 100644 index 00000000..2c3c0ca2 --- /dev/null +++ b/src/GameCult.Caching/CultStoreLinkedException.cs @@ -0,0 +1,17 @@ +using System.IO; + +namespace GameCult.Caching +{ + /// + /// A write was attempted at a single-file store path that is a link (a symbolic link to a file or a directory, a Windows + /// junction or any other reparse-point link). The write is refused before the lock, the staging file or the replace, so the + /// link and what it points at are untouched. Configure the link's target as the store path to write to it; reads through a + /// live link are unchanged. names the path. + /// + public sealed class CultStoreLinkedException : IOException + { + public CultStoreLinkedException(string message) : base(message) + { + } + } +} diff --git a/tests/GameCult.Caching.Tests/StoreReadabilityTests.cs b/tests/GameCult.Caching.Tests/StoreReadabilityTests.cs index f923dbfb..b244dc5f 100644 --- a/tests/GameCult.Caching.Tests/StoreReadabilityTests.cs +++ b/tests/GameCult.Caching.Tests/StoreReadabilityTests.cs @@ -266,7 +266,7 @@ namespace GameCult.Caching.Tests using var cache = CultCacheMessagePack.Create(path, new CultCacheOpenOptions { Registry = Registry }); CreateDanglingDirectoryLink(path, Path.Combine(_directory, "unmounted")); - Assert.That(() => cache.Commit(batch => batch.Upsert(typeof(IdDeck), new IdDeck { Name = "e" }, new CultRecordKey("e"))), Throws.TypeOf()); + Assert.That(() => cache.Commit(batch => batch.Upsert(typeof(IdDeck), new IdDeck { Name = "e" }, new CultRecordKey("e"))), Throws.TypeOf()); Assert.That(IsALink(path), "the link was replaced"); Assert.That(Directory.Exists(Path.Combine(_directory, "unmounted")), Is.False, "a write went through the link"); } @@ -292,11 +292,66 @@ namespace GameCult.Caching.Tests using var cache = CultCacheMessagePack.Create(path, new CultCacheOpenOptions { Registry = Registry }); File.Delete(target); - Assert.That(() => cache.Commit(batch => batch.Upsert(typeof(IdDeck), new IdDeck { Name = "e" }, new CultRecordKey("e"))), Throws.TypeOf()); + Assert.That(() => cache.Commit(batch => batch.Upsert(typeof(IdDeck), new IdDeck { Name = "e" }, new CultRecordKey("e"))), Throws.TypeOf()); Assert.That(new FileInfo(path).LinkTarget, Is.Not.Null, "the link was replaced"); Assert.That(File.Exists(target), Is.False, "a write went through the link"); } + // A write at a store path that is a link is refused with a typed error before the lock or the replace: the link, its target + // and the directory beside them are as they were. + [Test] + public void ACacheCommitAtALiveLinkIsRefusedAndNothingChanges() + { + var target = Path.Combine(_directory, "target.cc"); + var link = Path.Combine(_directory, "link.cc"); + using (var seed = CultCacheMessagePack.Create(target, new CultCacheOpenOptions { Registry = Registry })) + seed.Commit(batch => batch.Upsert(typeof(IdDeck), new IdDeck { Name = "a" }, new CultRecordKey("a"))); + CreateFileLinkOrIgnore(link, target); + var before = File.ReadAllBytes(target); + using var cache = CultCacheMessagePack.Create(link, new CultCacheOpenOptions { Registry = Registry, FlushOnDispose = false, StoreFlushOnDispose = false }); + + var refusal = Assert.Throws(() => cache.Commit(batch => batch.Upsert(typeof(IdDeck), new IdDeck { Name = "b" }, new CultRecordKey("b"))))!; + + Assert.That(refusal.Message, Does.Contain(link)); + Assert.That(IsALink(link), "the link was replaced"); + Assert.That(File.ReadAllBytes(target), Is.EqualTo(before)); + Assert.That(File.Exists(link + ".lock"), Is.False, "the refused write took the lock"); + Assert.That(Directory.GetFileSystemEntries(_directory).Where(entry => entry.EndsWith(".tmp")), Is.Empty); + } + + [Test] + public void AFlushAtALiveLinkIsRefusedAndNothingChanges() + { + var target = Path.Combine(_directory, "target.cc"); + var link = Path.Combine(_directory, "link.cc"); + using (var seed = CultCacheMessagePack.Create(target, new CultCacheOpenOptions { Registry = Registry })) + seed.Commit(batch => batch.Upsert(typeof(IdDeck), new IdDeck { Name = "a" }, new CultRecordKey("a"))); + CreateFileLinkOrIgnore(link, target); + var before = File.ReadAllBytes(target); + using var cache = CultCacheMessagePack.Create(link, new CultCacheOpenOptions { Registry = Registry, FlushOnDispose = false, StoreFlushOnDispose = false }); + cache.UpsertAsync(typeof(IdDeck), new IdDeck { Name = "b" }, new CultRecordKey("b")).GetAwaiter().GetResult(); + + Assert.Throws(() => cache.FlushAllBackingStores()); + + Assert.That(IsALink(link), "the link was replaced"); + Assert.That(File.ReadAllBytes(target), Is.EqualTo(before)); + Assert.That(File.Exists(link + ".lock"), Is.False); + } + + [Test] + public void AnOpenAtALiveLinkReadsTheTargetsRecords() + { + var target = Path.Combine(_directory, "target.cc"); + var link = Path.Combine(_directory, "link.cc"); + using (var seed = CultCacheMessagePack.Create(target, new CultCacheOpenOptions { Registry = Registry })) + seed.Commit(batch => batch.Upsert(typeof(IdDeck), new IdDeck { Name = "a" }, new CultRecordKey("a"))); + CreateFileLinkOrIgnore(link, target); + + using var cache = CultCacheMessagePack.Create(link, new CultCacheOpenOptions { Registry = Registry, FlushOnDispose = false, StoreFlushOnDispose = false }); + + Assert.That(cache.Get(new CultRecordKey("a"))?.Name, Is.EqualTo("a")); + } + // A directory at the path is something at the path: it is an I/O error on open and on commit, never an empty store, and // it is left as it was. [Test] diff --git a/tests/GameCult.Mesh.Tests/CultMeshSingleFileStoreTests.cs b/tests/GameCult.Mesh.Tests/CultMeshSingleFileStoreTests.cs index 7ce94ed2..7e210081 100644 --- a/tests/GameCult.Mesh.Tests/CultMeshSingleFileStoreTests.cs +++ b/tests/GameCult.Mesh.Tests/CultMeshSingleFileStoreTests.cs @@ -434,32 +434,74 @@ public sealed class CultMeshSingleFileStoreTests TempFiles().Should().BeEmpty(); } - // The store's replace decides what a write leaves where a live link was, and a Mesh write leaves what a cache commit leaves: - // one replace, whatever the platform does with a link. - [Test] - public void AMeshWriteOverALiveLinkLeavesWhatACacheCommitLeaves() + // A write at a store path that is a link is refused before the lock, the staging file and the replace: the link and its + // target stay as they were, and nothing is left beside them. + private void AssertRefusedAndUntouched(string link, string target, byte[] targetBefore, Action write) { - string Outcome(string name, Action write) + var refusal = Assert.Throws(() => write())!; + refusal.Message.Should().Contain(link); + IsALink(link).Should().BeTrue("the link was replaced"); + File.ReadAllBytes(target).Should().Equal(targetBefore); + File.Exists(link + ".lock").Should().BeFalse("the refused write took the lock"); + TempFiles().Should().BeEmpty(); + } + + [Test] + public void AMeshWriteAtALiveLinkIsRefusedAndNothingChanges() + { + var target = Path.Combine(_root, "target.cc"); + var link = Path.Combine(_root, "link.cc"); + CultMesh.WriteSingleFileDocument(target, Key, Publication("first")); + CreateLiveLinkOrIgnore(link, target); + var before = File.ReadAllBytes(target); + + AssertRefusedAndUntouched(link, target, before, () => CultMesh.WriteSingleFileDocument(link, Key, Publication("second"))); + AssertRefusedAndUntouched(link, target, before, () => CultMesh.WriteSingleFileDocumentPayload(link, Key, Raw, null, EmptyArray)); + } + + [Test] + public void AReadThroughALiveLinkStillWorks() + { + var target = Path.Combine(_root, "target.cc"); + var link = Path.Combine(_root, "link.cc"); + CultMesh.WriteSingleFileDocument(target, Key, Publication("first")); + CreateLiveLinkOrIgnore(link, target); + + CultMesh.ReadSingleFileDocument(link, Key).ProducerId.Should().Be("first"); + RecordsSeenByACache(link).Should().Be(1); + } + + // A link to a directory that is there is a directory, not an unreachable store; a write at it is a link refusal. + [Test] + public void ALiveDirectoryLinkIsADirectoryErrorOnReadAndALinkRefusalOnWrite() + { + var directory = Path.Combine(_root, "real-directory"); + var link = Path.Combine(_root, "directory-link.cc"); + Directory.CreateDirectory(directory); + CreateDirectoryLink(link, directory); + + Assert.Catch(() => CultMesh.ReadSingleFileDocumentPayload(link, Key, "raw:schema"))!.Message.Should().Contain("directory"); + Assert.Catch(() => RecordsSeenByACache(link))!.Message.Should().Contain("directory"); + Assert.Throws(() => CultMesh.WriteSingleFileDocument(link, Key, Publication())); + Directory.GetFileSystemEntries(directory).Should().BeEmpty(); + IsALink(link).Should().BeTrue(); + } + + private static void CreateDirectoryLink(string path, string target) + { + if (!OperatingSystem.IsWindows()) { - var target = Path.Combine(_root, name + "-target.cc"); - var link = Path.Combine(_root, name + "-link.cc"); - CultMesh.WriteSingleFileDocument(target, Key, Publication("first")); - CreateLiveLinkOrIgnore(link, target); - var before = File.ReadAllBytes(target); - - write(link); - - CultMesh.ReadSingleFileDocument(link, Key).ProducerId.Should().Be("second", name); - return $"linkKept={IsALink(link)} targetChanged={!File.ReadAllBytes(target).SequenceEqual(before)}"; + Directory.CreateSymbolicLink(path, target); + return; } - var viaMesh = Outcome("mesh", link => CultMesh.WriteSingleFileDocument(link, Key, Publication("second"))); - var viaCache = Outcome("cache", link => + var mklink = Process.Start(new ProcessStartInfo("cmd.exe", $"/c mklink /J \"{path}\" \"{target}\"") { - using var cache = CultCacheMessagePack.Create(link, ReadOnlyOpen); - cache.Commit(batch => batch.Upsert(typeof(CultMeshBodyPublicationDocument), Publication("second"), Key)); - }); - - viaMesh.Should().Be(viaCache); + CreateNoWindow = true, + RedirectStandardOutput = true, + RedirectStandardError = true, + })!; + mklink.WaitForExit(); + Assert.That(mklink.ExitCode, Is.Zero, "mklink /J failed"); } }