diff --git a/Schema.Editor.Test/SchemaFileTests.cs b/Schema.Editor.Test/SchemaFileTests.cs index f7112a3..cdd5f72 100644 --- a/Schema.Editor.Test/SchemaFileTests.cs +++ b/Schema.Editor.Test/SchemaFileTests.cs @@ -166,6 +166,61 @@ public void SavingOverAnExistingFileReplacesIt() Assert.HasCount(1, Directory.GetFiles(scratchDirectory)); } + /// + /// Saving through a symbolic link writes the file the link names and leaves the link a link. The + /// atomic rename used to replace the link itself with a plain file, so a schema shared between + /// projects through a link silently stopped being shared. + /// + [TestMethod] + public void SavingThroughASymbolicLinkUpdatesTheFileItNames() + { + AbsoluteFilePath target = ScratchFile("shared.schema.json"); + AbsoluteFilePath link = ScratchFile("link.schema.json"); + Assert.IsTrue(SchemaFile.TrySave(new Schema(), target)); + + try + { + File.CreateSymbolicLink(link, target); + } + catch (Exception exception) when (exception is IOException or UnauthorizedAccessException) + { + Assert.Inconclusive($"Symbolic links cannot be made here: {exception.Message}"); + } + + Schema edited = new(); + edited.AddClass("Order".As()); + Assert.IsTrue(SchemaFile.TrySave(edited, link)); + + Assert.IsNotNull(new FileInfo(link).LinkTarget, "The link was replaced by a plain file."); + Assert.IsTrue(SchemaFile.TryLoad(target, out Schema? loaded)); + Assert.IsNotNull(loaded); + Assert.IsNotNull(loaded.GetClass("Order".As())); + Assert.HasCount(2, Directory.GetFileSystemEntries(scratchDirectory)); + } + + /// + /// Saving over a file keeps its permission bits. The rename used to give the saved file the + /// process's default mode, so a group-writable schema became writable by its owner alone. + /// + [TestMethod] + public void SavingOverAFileKeepsItsPermissions() + { + if (OperatingSystem.IsWindows()) + { + Assert.Inconclusive("Unix file modes do not apply on Windows."); + return; + } + + AbsoluteFilePath path = ScratchFile("shared-mode.schema.json"); + Assert.IsTrue(SchemaFile.TrySave(new Schema(), path)); + const UnixFileMode Mode = UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.GroupRead | UnixFileMode.GroupWrite | UnixFileMode.OtherRead; + File.SetUnixFileMode(path, Mode); + + Assert.IsTrue(SchemaFile.TrySave(new Schema(), path)); + + Assert.AreEqual(Mode, File.GetUnixFileMode(path)); + } + [TestMethod] public void SavingWithNoPathFails() => Assert.IsFalse(SchemaFile.TrySave(new Schema(), new AbsoluteFilePath())); diff --git a/Schema.Editor/SchemaFile.cs b/Schema.Editor/SchemaFile.cs index 58e1204..971c331 100644 --- a/Schema.Editor/SchemaFile.cs +++ b/Schema.Editor/SchemaFile.cs @@ -161,12 +161,23 @@ internal static bool TrySave(Schema schema, AbsoluteFilePath filePath, Action /// + /// /// The temporary file is in the same directory so the final step is a rename on one volume rather /// than a copy. It is deleted on any failure, which is what the finally is for: an /// exception the caller does not catch still must not leave it behind. + /// + /// + /// A rename replaces a directory entry, not a file, so two things the target's entry carried + /// would be lost to it. A symbolic link would become a plain file holding the new content while + /// the file it pointed at - a schema shared between projects - kept the old one, so the path is + /// followed to the file it finally names and that file is the one replaced. And the new file + /// would have the process's default mode, so on Unix the existing file's permission bits are + /// copied onto the temporary file before the swap. + /// /// private static void WriteAtomically(string targetPath, string content, Action writeFile) { + targetPath = ResolveFinalTarget(targetPath); string temporaryPath = $"{targetPath}.tmp-{Guid.NewGuid():N}"; try @@ -175,6 +186,11 @@ private static void WriteAtomically(string targetPath, string content, Action + /// Follows through any symbolic links to the path of the file it finally + /// names, or returns it unchanged when it is not a link. + /// + /// + /// A link whose target does not exist yet still resolves to that target, so the first save through + /// it creates the file the link names rather than replacing the link. + /// + private static string ResolveFinalTarget(string path) + { + FileInfo file = new(path); + + // Resolving a path that names nothing throws rather than answering null, and a first save + // always names nothing, so only a path that is a link is resolved. + return file.LinkTarget is null ? path : file.ResolveLinkTarget(returnFinalTarget: true)?.FullName ?? path; + } } diff --git a/Schema/Cli/SchemaCommandLine.cs b/Schema/Cli/SchemaCommandLine.cs index 1d9bb90..d240490 100644 --- a/Schema/Cli/SchemaCommandLine.cs +++ b/Schema/Cli/SchemaCommandLine.cs @@ -66,7 +66,7 @@ public static int Run(string[] args, TextWriter output, TextWriter error, Schema { "generate" => RejectUnknownOptions([.. args.Skip(1)], [GeneratorOption], output, error, host) ?? Generate([.. args.Skip(1)], output, error), - "validate" => RejectUnknownOptions([.. args.Skip(1)], [], output, error, host) + "validate" => RejectUnknownOptions([.. args.Skip(1)], [NoDataOption], output, error, host) ?? Validate([.. args.Skip(1)], output, error), _ => UnknownCommand(args[0], output, error, host), };