diff --git a/src/BloomExe/Book/BookStarter.cs b/src/BloomExe/Book/BookStarter.cs index d83dbc044208..c01f2118489f 100644 --- a/src/BloomExe/Book/BookStarter.cs +++ b/src/BloomExe/Book/BookStarter.cs @@ -61,13 +61,17 @@ string parentCollectionPath parentCollectionPath ); + // Generate a new instance ID for the book before creating the folder + // so we can use it to create a unique folder name + var newBookInstanceId = Guid.NewGuid().ToString(); + // We use the "initial name" to make the initial copy, and it gives us something //to name the folder and file until such time as the user enters a title in for the book. - string initialBookName = GetInitialName(parentCollectionPath); + string initialBookName = GetInitialName(parentCollectionPath, newBookInstanceId); var newBookFolder = Path.Combine(parentCollectionPath, initialBookName); CopyFolder(sourceBookFolder, newBookFolder); BookStorage.RemoveLocalOnlyFiles(newBookFolder); - //if something bad happens from here on out, we need to delete that folder we just made + //if something bad happens from here on out, we need to delete that folder we just made try { var oldNamedFile = Path.Combine( @@ -89,7 +93,7 @@ string parentCollectionPath RobustFile.Move(oldNamedFile, newNamedFile); //the destination may change here... - newBookFolder = SetupNewDocumentContents(sourceBookFolder, newBookFolder); + newBookFolder = SetupNewDocumentContents(sourceBookFolder, newBookFolder, newBookInstanceId); if (OnNextRunSimulateFailureMakingBook) throw new ApplicationException("Simulated failure for unit test"); @@ -162,7 +166,7 @@ private string GetMetaValue(SafeXmlDocument Dom, string name, string defaultValu return defaultValue; } - private string SetupNewDocumentContents(string sourceFolderPath, string initialPath) + private string SetupNewDocumentContents(string sourceFolderPath, string initialPath, string newBookInstanceId) { // This bookInfo is temporary, just used to make the (also temporary) BookStorage we // use here in this method. I don't think it actually matters what its save context is. @@ -269,7 +273,7 @@ private string SetupNewDocumentContents(string sourceFolderPath, string initialP InjectXMatter(initialPath, storage, sizeAndOrientation); - SetLineageAndId(storage, sourceFolderPath); + SetLineageAndId(storage, sourceFolderPath, newBookInstanceId); if (makingTranslation) { @@ -412,7 +416,7 @@ private static void ProcessXMatterMetaTags(BookStorage storage) storage.Dom.RemoveMetaElement("xmatter-for-children"); } - private void SetLineageAndId(BookStorage storage, string sourceFolderPath) + private void SetLineageAndId(BookStorage storage, string sourceFolderPath, string newBookInstanceId) { string parentId = null; string lineage = null; @@ -439,7 +443,8 @@ private void SetLineageAndId(BookStorage storage, string sourceFolderPath) { storage.BookInfo.BookLineage = lineage + parentId; } - storage.BookInfo.Id = Guid.NewGuid().ToString(); + // Use the pre-generated instance ID that was used to create the unique folder name + storage.BookInfo.Id = newBookInstanceId; storage.Dom.RemoveMetaElement("bloomBookLineage"); //old metadata storage.Dom.RemoveMetaElement("bookLineage"); // even older name } @@ -680,10 +685,10 @@ SafeXmlElement childPageDiv } } - private string GetInitialName(string parentCollectionPath) + private string GetInitialName(string parentCollectionPath, string instanceId) { var name = BookStorage.SanitizeNameForFileSystem(UntitledBookName); - return BookStorage.GetUniqueFolderName(parentCollectionPath, name); + return BookStorage.GetUniqueBookFolderName(parentCollectionPath, name, instanceId); } public static string UntitledBookName diff --git a/src/BloomExe/Book/BookStorage.cs b/src/BloomExe/Book/BookStorage.cs index 97e8ffbe684b..a899ae9c8f11 100644 --- a/src/BloomExe/Book/BookStorage.cs +++ b/src/BloomExe/Book/BookStorage.cs @@ -1734,7 +1734,8 @@ public HtmlDom MakeDomRelocatable(HtmlDom dom) private static string GetActualPathToSave( string idealFolderName, string currentFolderName, - string idealFolderPath + string idealFolderPath, + string instanceId = null ) { // As of 16 Dec 2019 we changed our definition of "sanitized" to include some more characters that can @@ -1766,8 +1767,9 @@ string idealFolderPath ) return Path.Combine(Path.GetDirectoryName(idealFolderPath), currentFolderName); // 4. ideal name is in use, and currentFolderName is not a variant of it, - // so find a new variant that is not in use. - return GetUniqueFolderPath(idealFolderPath); + // so find a new variant that is not in use. Override the default separator since + // this not typically a copy. + return GetAvailableDirectoryPath(idealFolderPath, instanceId, " - "); } public void SetBookName(string name) @@ -1794,7 +1796,12 @@ public void SetBookName(string name) var currentFilePath = PathToExistingHtml; var currentFolderName = Path.GetFileNameWithoutExtension(currentFilePath); var idealFolderPath = Path.Combine(Directory.GetParent(FolderPath).FullName, name); - var actualSavePath = GetActualPathToSave(name, currentFolderName, idealFolderPath); + var actualSavePath = GetActualPathToSave( + name, + currentFolderName, + idealFolderPath, + _metaData?.Id + ); if (actualSavePath == Path.GetDirectoryName(currentFilePath)) return; // for this path they must be exactly the same, even by case. @@ -1851,7 +1858,7 @@ string newPath return; if (oldPath.ToLowerInvariant() == newPath.ToLowerInvariant()) { - var tempName = new Guid().ToString(); + var tempName = Guid.NewGuid().ToString(); var tempPath = Path.Combine(Path.GetDirectoryName(oldPath), tempName); // This is a case-only change. We can't just rename the file, because that throws. // So we move the file to a temporary name, and then rename again to what we want. @@ -1879,7 +1886,7 @@ public static void MoveDirectoryPossiblyOnlyChangingCase(string oldPath, string return; if (oldPath.ToLowerInvariant() == newPath.ToLowerInvariant()) { - var tempName = new Guid().ToString(); + var tempName = Guid.NewGuid().ToString(); var tempPath = Path.Combine(Path.GetDirectoryName(oldPath), tempName); // This is a case-only change. We can't just rename the directory, because that throws. // So we move the directory to the new name, and then move again to the one we want. @@ -3307,12 +3314,22 @@ private static string RemoveDangerousCharacters(string name) } /// - /// if necessary, append a number to make the folder path unique + /// as necessary, shorten and/or append a guid to make a path to a new folder + /// with a name that does not exceed our limit. /// - private static string GetUniqueFolderPath(string folderPath) + private static string GetAvailableDirectoryPath( + string folderPath, + string instanceId = null, + string separator = " - Copy" + ) { var parent = Directory.GetParent(folderPath).FullName; - var name = GetUniqueFolderName(parent, Path.GetFileName(folderPath)); + var name = GetAvailableDirectory( + parent, + Path.GetFileName(folderPath), + instanceId, + separator + ); return Path.Combine(parent, name); } @@ -3346,24 +3363,6 @@ string numberedNameTemplate return Path.Combine(parentFolderPath, newName); } - /// - /// if necessary, append a number to make the subfolder name unique within the given folder - /// - internal static string GetUniqueFolderName(string parentPath, string name) - { - // Don't be tempted to give this parentheses. That isn't compatible with - // SanitizeNameForFileSystem which removes parentheses. See BL-11663. - - int i = 1; // First non-blank suffix should be " 2" - string suffix = ""; - while (Directory.Exists(Path.Combine(parentPath, name + suffix))) - { - ++i; - suffix = " " + i.ToString(CultureInfo.InvariantCulture); - } - return name + suffix; - } - /// /// if necessary, append a number to make the file name unique within the given folder. /// name must NOT include an extension. The supplied extension will be added rather than @@ -3445,25 +3444,26 @@ public static void CopyDirectory( /// a path to the directory containing the duplicate public string Duplicate() { - // get the book name and copy number of the current directory + // get the book name of the current directory var baseName = Path.GetFileName(FolderPath); - // see if this already has a name like "foo Copy 3" - // If it does, we will use that number plus 1 as the starting point for looking for a new unique folder name - var regexToGetCopyNumber = new Regex(@"^(.+)(\s-\sCopy)(\s[0-9]+)?$"); - var match = regexToGetCopyNumber.Match(baseName); - var copyNum = 1; + // see if this already has a name like "foo - Copy-abc12345" or "foo - Copy 3" + // (The second is obsolete, but might still be encountered.) + // If it does, remove that suffix so we get back to the base name + var regexToRemoveCopySuffix = new Regex(@"^(.+)(\s-\sCopy)(-[0-9a-f]+|\s[0-9]+)?$"); + var match = regexToRemoveCopySuffix.Match(baseName); if (match.Success) { baseName = match.Groups[1].Value; - if (match.Groups[3].Success) - copyNum = 1 + Int32.Parse(match.Groups[3].Value.Trim()); } + // Generate a new instance ID for the duplicate + var newInstanceId = Guid.NewGuid().ToString(); + // directory for the new book var collectionDir = Path.GetDirectoryName(FolderPath); - var newBookName = GetAvailableDirectory(collectionDir, baseName, copyNum); + var newBookName = GetAvailableDirectory(collectionDir, baseName, newInstanceId); var newBookDir = Path.Combine(collectionDir, newBookName); Directory.CreateDirectory(newBookDir); @@ -3476,7 +3476,7 @@ public string Duplicate() ); var metaPath = Path.Combine(newBookDir, "meta.json"); - ChangeInstanceId(metaPath); + UpdateInstanceId(metaPath, newInstanceId); // rename the book htm file var oldName = Path.Combine(newBookDir, Path.GetFileName(PathToExistingHtml)); @@ -3485,7 +3485,7 @@ public string Duplicate() return newBookDir; } - private void ChangeInstanceId(string metaDataPath) + private void UpdateInstanceId(string metaDataPath, string newInstanceId) { // Update the InstanceId. This was not done prior to Bloom 4.2.104 // If the meta.json file is missing, ok that's weird but that means we @@ -3493,37 +3493,91 @@ private void ChangeInstanceId(string metaDataPath) if (RobustFile.Exists(metaDataPath)) { var meta = DynamicJson.Parse(RobustFile.ReadAllText(metaDataPath)); - meta.bookInstanceId = Guid.NewGuid().ToString(); + meta.bookInstanceId = newInstanceId; RobustFile.WriteAllText(metaDataPath, meta.ToString()); } } /// - /// Get an available directory name for a new copy of a book + /// Get an available directory name for a new copy of a book using part of the instance ID /// /// /// - /// + /// The instance ID to use for generating a unique suffix. If null, a new GUID will be generated. /// private static string GetAvailableDirectory( string collectionDir, string baseName, - int copyNum + string instanceId = null, + string separator = " - Copy-" ) { - string newName; - if (copyNum == 1) - newName = baseName + " - Copy"; - else - newName = baseName + " - Copy " + copyNum; + return GetUniqueBookFolderName(collectionDir, baseName, instanceId, separator); + } - while (Directory.Exists(Path.Combine(collectionDir, newName))) + /// + /// Get a unique book folder name using part of the instance ID or a new guid. + /// Observes our standard constraint on book folder name length. + /// + /// Parent directory path + /// Base name for the book + /// The instance ID to use for generating a unique suffix. + /// If null, a new GUID will be generated. + /// When making a new book (derivative, duplicate, etc) that already involves + /// making a new guid as the book's instanceId, we try to use that ID to make + /// a unique name. This is mainly helpful when the rest of the name is something + /// fixed like "Book"...it means the folder name has at least some relationship + /// to the book it contains. On the other hand, when it's an existing book, + /// part of the purpose of adding these guids rather than just a number is + /// that we want it to be unlikely that, in a Team Collection, someone elsewhere + /// will do a similar operation on the book (or another one) and come up with + /// the same name. + /// So, if the name is for a book that has a newly created instanceId, + /// it's good to pass it. If it's an instance Id that might already have been + /// shared somehow, don't pass it. (If it's just difficult for any reason + /// to get the instanceId, it's also harmless not to pass it.) + /// (There's about one chance in 4 billion that the name-plus-instanceId + /// produces the name of a folder that exists. In those very rare cases, + /// we will generate a new GUID even though one was passed.) + /// Separator string before the ID suffix (e.g., " - Copy-" or "-") + /// A unique folder name + internal static string GetUniqueBookFolderName( + string parentPath, + string baseName, + string instanceId = null, + string separator = "-" + ) + { + // Generate an instanceId if not provided + if (string.IsNullOrEmpty(instanceId)) { - copyNum++; - newName = baseName + " - Copy " + copyNum; + instanceId = Guid.NewGuid().ToString(); } - return newName; + // Calculate the maximum length for the base name + // We need room for: separator (variable length) + 8 characters from instanceId + var suffixLength = separator.Length + 8; // 8 hex chars from GUID + var maxBaseNameLength = kMaxFilenameLength - suffixLength; + + // Truncate and clean the base name if necessary + if (baseName.Length > maxBaseNameLength) + { + baseName = MiscUtils.TruncateSafely(baseName, maxBaseNameLength); + baseName = Regex.Replace(baseName, "[\\s.]+$", "", RegexOptions.Compiled); + } + + string proposedName; + do + { + // Use first 8 characters of the instance ID (without hyphens) as the unique suffix + var instanceSuffix = instanceId.Replace("-", "").Substring(0, 8).ToLowerInvariant(); + proposedName = baseName + separator + instanceSuffix; + + if (!Directory.Exists(Path.Combine(parentPath, proposedName))) + return proposedName; + // If there's a collision, generate a new GUID and try again + instanceId = Guid.NewGuid().ToString(); + } while (true); } public void EnsureOriginalTitle() @@ -3793,7 +3847,17 @@ public static string MoveBookToAvailableName( { if (desiredPath == null) desiredPath = oldBookFolder; - var newPathForExtraBook = BookStorage.GetUniqueFolderPath(desiredPath); + // not passing an instanceId here. Although we usually like to use the book's + // own instanceId when necessary to make a unique folder name if possible, the + // uses of this method mostly involve moving an existing book. That makes it + // at least somewhat likely that someone somewhere might already be using that + // instanceId in a name. So a brand new one makes for more uniqueness. + // Since we're not making a copy we will use a shorter separator + var newPathForExtraBook = BookStorage.GetAvailableDirectoryPath( + desiredPath, + null, + " - " + ); SIL.IO.RobustIO.MoveDirectory(oldBookFolder, newPathForExtraBook); var extraBookPath = Path.Combine( newPathForExtraBook, diff --git a/src/BloomTests/Book/BookStarterTests.cs b/src/BloomTests/Book/BookStarterTests.cs index c211cb2eedc8..b0ee79368440 100644 --- a/src/BloomTests/Book/BookStarterTests.cs +++ b/src/BloomTests/Book/BookStarterTests.cs @@ -260,7 +260,8 @@ public void CreateBookOnDiskFromTemplate_FromFactoryMoonAndCap_InitialFolderName ); var path = _starter.CreateBookOnDiskFromTemplate(source, _projectFolder.Path); - Assert.AreEqual("Book", Path.GetFileName(path)); + // With unique book names, the folder name should be "Book-" followed by 8 hex characters + Assert.That(Path.GetFileName(path), Does.Match("^Book-[a-f0-9]{8}$")); } [Test] @@ -1002,7 +1003,8 @@ public void CreateBookOnDiskFromTemplate_ShellHasNoNameDirective_FileNameJust_Bo GetShellBookFolder(), _projectFolder.Path ); - Assert.AreEqual("Book", Path.GetFileName(folderPath)); + // With unique book names, the folder name should be "Book-" followed by 8 hex characters + Assert.That(Path.GetFileName(folderPath), Does.Match("^Book-[a-f0-9]{8}$")); } [Test] @@ -1017,9 +1019,19 @@ public void CreateBookOnDiskFromTemplate_BookWithDefaultNameAlreadyExists_NameGe _projectFolder.Path ); - Assert.IsTrue(File.Exists(firstPath.CombineForPath("Book.htm"))); - Assert.IsTrue(File.Exists(secondPath.CombineForPath("Book 2.htm"))); + // With unique book names, each book gets a unique ID suffix, so the HTML file names will be different + var firstName = Path.GetFileName(firstPath); + var secondName = Path.GetFileName(secondPath); + Assert.That(firstName, Does.Match("^Book-[a-f0-9]{8}$")); + Assert.That(secondName, Does.Match("^Book-[a-f0-9]{8}$")); + Assert.IsTrue(File.Exists(Path.Combine(firstPath, firstName + ".htm"))); + Assert.IsTrue(File.Exists(Path.Combine(secondPath, secondName + ".htm"))); Assert.IsTrue(Directory.Exists(secondPath), "it clobbered the first one!"); + Assert.AreNotEqual( + firstName, + secondName, + "Both books should have different unique names" + ); } [Test] @@ -1033,7 +1045,8 @@ public void CreateBookOnDiskFromTemplate_FromBasicBook_GetsExpectedFileName() ); var path = GetPathToHtml(bookFolderPath); - Assert.AreEqual("Book.htm", Path.GetFileName(path)); + // With unique book names, the HTML file name should be "Book-<8 hex chars>.htm" + Assert.That(Path.GetFileName(path), Does.Match("^Book-[a-f0-9]{8}\\.htm$")); Assert.IsTrue(Directory.Exists(bookFolderPath)); Assert.IsTrue(File.Exists(path)); } @@ -1295,6 +1308,7 @@ public void CreateBookOnDiskFromTemplate_ShellHasBookshelf_DerivativeDoesNot() [Test] public void CreateBookOnDiskFromTemplate_FromFactoryTemplate_SameNameAlreadyUsed_FindsUsableNumberSuffix() { + // Create some directories with old-style naming to ensure the new unique naming doesn't conflict Directory.CreateDirectory(_projectFolder.Combine("Book")); Directory.CreateDirectory(_projectFolder.Combine("Book 2")); Directory.CreateDirectory(_projectFolder.Combine("Book 4")); @@ -1303,28 +1317,35 @@ public void CreateBookOnDiskFromTemplate_FromFactoryTemplate_SameNameAlreadyUsed var path = _starter.CreateBookOnDiskFromTemplate(source, _projectFolder.Path); - Assert.AreEqual("Book 3", Path.GetFileName(path)); + // With unique book names, the new book should get a unique ID suffix and not conflict + var folderName = Path.GetFileName(path); + Assert.That(folderName, Does.Match("^Book-[a-f0-9]{8}$")); Assert.IsTrue(Directory.Exists(path)); - Assert.IsTrue(File.Exists(Path.Combine(path, "Book 3.htm"))); + Assert.IsTrue(File.Exists(Path.Combine(path, folderName + ".htm"))); + // Verify it didn't use the old numbering scheme + Assert.AreNotEqual("Book 3", folderName); } [Test] public void CreateBookOnDiskFromTemplate_CreationFailsForSomeReason_DoesNotLeaveIncompleteFolderAround() { var source = BloomFileLocator.GetFactoryBookTemplateDirectory("Basic Book"); - var goodPath = _starter.CreateBookOnDiskFromTemplate(source, _projectFolder.Path); - SIL.IO.RobustIO.DeleteDirectoryAndContents(goodPath); //remove that good one. We just did it to get an idea of what the path is - //now fail while making a book + // Get the count of folders before attempting the failed creation + var folderCountBeforeFailure = Directory.GetDirectories(_projectFolder.Path).Length; + //now fail while making a book _starter.OnNextRunSimulateFailureMakingBook = true; Assert.Throws(() => _starter.CreateBookOnDiskFromTemplate(source, _projectFolder.Path) ); - Assert.IsFalse( - Directory.Exists(goodPath), - "Should not have left the folder there, after a failed book creation" + // Verify no new folders were created + var folderCountAfterFailure = Directory.GetDirectories(_projectFolder.Path).Length; + Assert.AreEqual( + folderCountBeforeFailure, + folderCountAfterFailure, + "Should not have left any folders after a failed book creation" ); } diff --git a/src/BloomTests/Book/BookStorageTests.cs b/src/BloomTests/Book/BookStorageTests.cs index 8148b8317e94..67e3ebcad096 100644 --- a/src/BloomTests/Book/BookStorageTests.cs +++ b/src/BloomTests/Book/BookStorageTests.cs @@ -200,31 +200,11 @@ public void MoveBookToSafeName_NameNotNFC_Moves() var expectedFolder = bookFolder.Replace("e\x0301", "\x00e9"); - Assert.That(BookStorage.MoveBookToSafeName(bookFolder), Is.EqualTo(expectedFolder)); - Assert.That(Directory.Exists(expectedFolder)); - var expectedBookPath = bookPath.Replace("e\x0301", "\x00e9"); - Assert.That(File.ReadAllText(expectedBookPath), Is.EqualTo("this is a test")); - } - - [Test] - public void MoveBookToSafeName_NameNotNFC_NewNameConflicts_Moves() - { - var oldName = "Some nice\x0301 book"; - var bookFolder = Path.Combine(_fixtureFolder.Path, oldName); - Directory.CreateDirectory(bookFolder); - var bookPath = Path.Combine(bookFolder, oldName + ".htm"); - File.WriteAllText(bookPath, "this is a test"); - - var desiredFolder = bookFolder.Replace("e\x0301", "\x00e9"); - Directory.CreateDirectory(desiredFolder); - var expectedFolder = desiredFolder + " 2"; - - Assert.That(BookStorage.MoveBookToSafeName(bookFolder), Is.EqualTo(expectedFolder)); - Assert.That(Directory.Exists(expectedFolder)); - var expectedBookPath = Path.Combine( - expectedFolder, - Path.GetFileName(expectedFolder) + ".htm" - ); + // Typically we added hyphen and a guid. Not sure if that's good. + var result = BookStorage.MoveBookToSafeName(bookFolder); + Assert.That(result, Does.StartWith(expectedFolder)); + Assert.That(Directory.Exists(result)); + var expectedBookPath = Path.Combine(result, Path.GetFileName(result) + ".htm"); Assert.That(File.ReadAllText(expectedBookPath), Is.EqualTo("this is a test")); } @@ -1098,12 +1078,12 @@ public void SetBookName_EasyCase_ChangesFolderAndFileName() } [Test] - public void SetBookName_FolderWithNameAlreadyExists_AddsANumberToName() + public void SetBookName_IdealNameAlreadyExists_AddsGuidToName() { + // Simulate renaming a book called 'original' to a name that, after sanitizing, + // reduces to 'foo'. But there's already a book called 'foo'. using (var original = new TemporaryFolder(_folder, "original")) - using (var x = new TemporaryFolder(_folder, "foo")) - using (new TemporaryFolder(_folder, "foo 2")) - using (var z = new TemporaryFolder(_folder, "foo 3")) + using (new TemporaryFolder(_folder, "foo")) // causes a conflict, so we can't call the new book 'foo' using (var projectFolder = new TemporaryFolder("BookStorage_ProjectCollection")) { File.WriteAllText( @@ -1122,49 +1102,45 @@ public void SetBookName_FolderWithNameAlreadyExists_AddsANumberToName() ); storage.Save(); - Directory.Delete(z.Path); - //so, we ask for "foo", but should get "foo 3", because there is already a "foo" and "foo 2" - var newBookName = Path.GetFileName(x.Path); + // Get folders before rename + var foldersBeforeRename = Directory.GetDirectories(_folder.Path); + + //so, we ask for "foo", but should get something like "foo - 12345678", + // because we want a unique name each time + // BL-7816 We added some new characters to the sanitization routine + const string newBookName = "foo?:&<>\'\"{}"; storage.SetBookName(newBookName); - var newPath = z.Combine("foo 3.htm"); - Assert.IsTrue(Directory.Exists(z.Path), "Expected folder:" + z.Path); - Assert.IsTrue(File.Exists(newPath), "Expected file:" + newPath); - } - } - [Test] - public void SetBookName_FolderWithSanitizedNameAlreadyExists_AddsANumberToName() - { - using (var original = new TemporaryFolder(_folder, "original")) - using (new TemporaryFolder(_folder, "foo")) - using (new TemporaryFolder(_folder, "foo 2")) - using (var z = new TemporaryFolder(_folder, "foo 3")) - using (var projectFolder = new TemporaryFolder("BookStorage_ProjectCollection")) - { - File.WriteAllText( - Path.Combine(original.Path, "original.htm"), - " href='file://blahblah\\editMode.css' type='text/css' />
" + // Get folders after rename - should be just one that was not there before. + var foldersAfterRename = Directory.GetDirectories(_folder.Path); + Assert.That( + foldersAfterRename.Length, + Is.EqualTo(foldersBeforeRename.Length), + "Should have exactly one new folder" ); - var collectionSettings = new CollectionSettings( - Path.Combine(projectFolder.Path, "test.bloomCollection") - ); - var storage = new BookStorage( - original.Path, - _fileLocator, - new BookRenamedEvent(), - collectionSettings + // Find the new folder + var newFolders = foldersAfterRename.Except(foldersBeforeRename).ToArray(); + + Assert.That(newFolders.Length, Is.EqualTo(1)); + + var newFolderPath = newFolders[0]; + var newFolderName = Path.GetFileName(newFolderPath); + + // Verify folder name starts with "foo - " followed by 8 hex digits + Assert.That( + newFolderName, + Does.Match("^foo - [a-f0-9]{8}$"), + "Folder name should be 'foo - ' followed by 8 hex characters" ); - storage.Save(); - Directory.Delete(z.Path); - //so, we ask for "foo", but should get "foo 3", because there is already a "foo" and "foo 2" - // BL-7816 We added some new characters to the sanitization routine - const string newBookName = "foo?:&<>\'\"{}"; - storage.SetBookName(newBookName); - var newPath = z.Combine("foo 3.htm"); - Assert.IsTrue(Directory.Exists(z.Path), "Expected folder:" + z.Path); - Assert.IsTrue(File.Exists(newPath), "Expected file:" + newPath); + // Verify the htm file exists with the same name as the folder + var expectedHtmlFile = Path.Combine(newFolderPath, newFolderName + ".htm"); + Assert.That( + File.Exists(expectedHtmlFile), + Is.True, + "Expected file: " + expectedHtmlFile + ); } } @@ -1650,6 +1626,15 @@ public void Duplicate_CopyGetsNewGuid() Path.GetDirectoryName(storage.FolderPath), "Should be in same collection folder" ); + + // Verify the duplicate folder name follows the pattern: "theBook - Copy-<8 hex chars>" + var duplicateFolderName = Path.GetFileName(folderForDuplicate); + Assert.That( + duplicateFolderName, + Does.Match("^theBook - Copy-[a-f0-9]{8}$"), + "Duplicate folder name should be 'theBook - Copy-' followed by 8 hex characters from instance ID" + ); + var metaPath = Path.Combine(folderForDuplicate, "meta.json"); var meta = DynamicJson.Parse(File.ReadAllText(metaPath)); Assert.AreNotEqual( @@ -1662,6 +1647,47 @@ public void Duplicate_CopyGetsNewGuid() Guid.Parse(meta.bookInstanceId); // will throw if we didn't actually get a guid } + [Test] + public void Duplicate_OfAPreviousDuplicate_RemovesOldCopySuffix() + { + var storage = GetInitialStorage(); + + // First duplication + var firstDuplicate = storage.Duplicate(); + var firstDuplicateName = Path.GetFileName(firstDuplicate); + + // Verify first duplicate has the expected name pattern + Assert.That( + firstDuplicateName, + Does.Match("^theBook - Copy-[a-f0-9]{8}$"), + "First duplicate should follow naming pattern" + ); + + // Now duplicate the duplicate + var duplicateStorage = new BookStorage( + firstDuplicate, + _fileLocator, + new BookRenamedEvent(), + new CollectionSettings() + ); + var secondDuplicate = duplicateStorage.Duplicate(); + var secondDuplicateName = Path.GetFileName(secondDuplicate); + + // Verify second duplicate also uses base name "theBook", not "theBook - Copy-abc12345" + Assert.That( + secondDuplicateName, + Does.Match("^theBook - Copy-[a-f0-9]{8}$"), + "Second duplicate should also use base name 'theBook' with new unique suffix" + ); + + // And they should be different from each other + Assert.AreNotEqual( + firstDuplicateName, + secondDuplicateName, + "Each duplicate should have a unique suffix" + ); + } + [Test] public void Duplicate_UnwantedFilesDropped() {