Skip to content

Generate more unique names for new book folders (BL-15608) #7683

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
JohnThomson wants to merge 4 commits into
base: master
Choose a base branch
from
+256 −140
Open

Generate more unique names for new book folders (BL-15608) #7683

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c557e04
Generate more unique names for new book folders (BL-15608)
JohnThomson Feb 4, 2026
2c0c60a
Unit test fixes (by CoPilot)
JohnThomson Feb 11, 2026
d4d7229
More fixes suggested by Devin
JohnThomson Feb 12, 2026
4346384
Post-review improvements; add guids in more places
JohnThomson Feb 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 14 additions & 9 deletions src/BloomExe/Book/BookStarter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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");
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -269,7 +273,7 @@ private string SetupNewDocumentContents(string sourceFolderPath, string initialP

InjectXMatter(initialPath, storage, sizeAndOrientation);

SetLineageAndId(storage, sourceFolderPath);
SetLineageAndId(storage, sourceFolderPath, newBookInstanceId);

if (makingTranslation)
{
Expand Down Expand Up @@ -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;
Expand All @@ -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
}
Expand Down Expand Up @@ -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
Expand Down
168 changes: 116 additions & 52 deletions src/BloomExe/Book/BookStorage.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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.

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -3307,12 +3314,22 @@ private static string RemoveDangerousCharacters(string name)
}

/// <summary>
/// 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.
/// </summary>
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);
}

Expand Down Expand Up @@ -3346,24 +3363,6 @@ string numberedNameTemplate
return Path.Combine(parentFolderPath, newName);
}

/// <summary>
/// if necessary, append a number to make the subfolder name unique within the given folder
/// </summary>
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;
}

/// <summary>
/// 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
Expand Down Expand Up @@ -3445,25 +3444,26 @@ public static void CopyDirectory(
/// <returns>a path to the directory containing the duplicate</returns>
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);

Expand All @@ -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));
Expand All @@ -3485,45 +3485,99 @@ 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
// don't have a duplicate bookInstanceId to worry about.
if (RobustFile.Exists(metaDataPath))
{
var meta = DynamicJson.Parse(RobustFile.ReadAllText(metaDataPath));
meta.bookInstanceId = Guid.NewGuid().ToString();
meta.bookInstanceId = newInstanceId;
RobustFile.WriteAllText(metaDataPath, meta.ToString());
}
}

/// <summary>
/// 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
/// </summary>
/// <param name="collectionDir"></param>
/// <param name="baseName"></param>
/// <param name="copyNum"></param>
/// <param name="instanceId">The instance ID to use for generating a unique suffix. If null, a new GUID will be generated.</param>
/// <returns></returns>
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)))
/// <summary>
/// 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.
/// </summary>
/// <param name="parentPath">Parent directory path</param>
/// <param name="baseName">Base name for the book</param>
/// <param name="instanceId">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.)</param>
/// <param name="separator">Separator string before the ID suffix (e.g., " - Copy-" or "-")</param>
/// <returns>A unique folder name</returns>
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()
Expand Down Expand Up @@ -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,
Expand Down
Loading