Skip to content

Commit d52c666

Browse files
JusterZhuclaude
andauthored
fix: backup recursive nesting and timestamp-based naming (#501) (#502)
Root cause: Backup directory was created INSIDE InstallPath. If the user configured Directories as an empty list, the ?? null-coalescing didn't trigger, and CopyDirectory recursively copied the backup into itself, causing PathTooLongException. Changes: - Move backups to .backups/ subdirectory isolated from install path - Change naming from app-{version} to backup-{yyyyMMddHHmmss} - Merge BlackDefaults.DefaultDirectories into skip list at Backup() entry point so backups are always excluded, regardless of user config - Centralize all magic strings as constants (DirectoryName, LegacyDirectoryPrefix, BackupRootDirectory) referenced by BlackDefaults - TryRollback now falls back to GetLatestBackup() when specific backup absent - Auto-clean old backups after each backup (keep latest 3) - 6 new/updated tests covering no-recurse, skip-backup, GetLatestBackup Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 2921b1f commit d52c666

6 files changed

Lines changed: 367 additions & 49 deletions

File tree

src/c#/GeneralUpdate.Core/FileSystem/BlackDefaults.cs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,10 @@ public static class BlackDefaults
2222

2323
/// <summary>Default skipped directory prefixes.</summary>
2424
public static readonly List<string> DefaultDirectories = new()
25-
{ "app-", "fail" };
25+
{
26+
StorageManager.BackupRootDirectory,
27+
StorageManager.DirectoryName,
28+
StorageManager.LegacyDirectoryPrefix,
29+
"fail"
30+
};
2631
}

src/c#/GeneralUpdate.Core/FileSystem/StorageManager.cs

Lines changed: 148 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,26 @@ public sealed class StorageManager
3838
private long _fileCount = 0;
3939

4040
/// <summary>
41-
/// The default prefix name for backup directories.
41+
/// The default prefix for backup directory names (e.g. "backup-20260606235200").
4242
/// </summary>
43-
/// <remarks>
44-
/// Backup directories are named in the format "app-{version}". This constant defines the prefix portion.
45-
/// </remarks>
46-
public const string DirectoryName = "app-";
43+
public const string DirectoryName = "backup-";
44+
45+
/// <summary>
46+
/// Legacy backup directory prefix used by older versions (e.g. "app-1.0.0").
47+
/// Retained for backward compatibility in discovery and cleanup.
48+
/// </summary>
49+
public const string LegacyDirectoryPrefix = "app-";
50+
51+
/// <summary>
52+
/// The subdirectory under the install path where new-format backups are stored.
53+
/// </summary>
54+
public const string BackupRootDirectory = ".backups";
55+
56+
/// <summary>
57+
/// Backup directory name prefixes used for enumeration (both new and legacy formats).
58+
/// Derived from <see cref="DirectoryName"/> and <see cref="LegacyDirectoryPrefix"/>.
59+
/// </summary>
60+
private static readonly string[] BackupNamePrefixes = { DirectoryName, LegacyDirectoryPrefix };
4761

4862
/// <summary>
4963
/// Gets or sets the optional path/file blacklist matcher.
@@ -194,6 +208,64 @@ public static string GetTempDirectory(string name)
194208
return tempDir;
195209
}
196210

211+
/// <summary>
212+
/// Generates a timestamp-based backup directory name in the format "backup-{yyyyMMddHHmmss}".
213+
/// </summary>
214+
/// <returns>A backup directory name string, e.g. "backup-20260606235200".</returns>
215+
/// <remarks>
216+
/// Timestamp naming ensures each backup is unique and naturally sortable by creation time.
217+
/// Used by <see cref="Backup"/> to create version-independent backup directory names.
218+
/// </remarks>
219+
public static string GetBackupDirectoryName() => $"{DirectoryName}{DateTime.Now:yyyyMMddHHmmss}";
220+
221+
/// <summary>
222+
/// Finds the most recent backup directory by scanning for backup directories
223+
/// matching patterns derived from <see cref="BlackDefaults.DefaultDirectories"/>.
224+
/// </summary>
225+
/// <param name="installPath">The application installation root directory.</param>
226+
/// <returns>The full path of the most recent backup directory, or <c>null</c> if none exists.</returns>
227+
/// <remarks>
228+
/// Directories are sorted by name in descending order. Since both "backup-{yyyyMMddHHmmss}"
229+
/// and "app-{version}" formats are lexicographically sortable, this works for both.
230+
/// Search patterns are derived from <see cref="BlackDefaults.DefaultDirectories"/> by
231+
/// appending "*" to each entry — e.g. "backup-" becomes "backup-*".
232+
/// </remarks>
233+
public static string? GetLatestBackup(string installPath)
234+
{
235+
if (!Directory.Exists(installPath)) return null;
236+
237+
var allBackups = new List<string>();
238+
239+
// Scan BackupRootDirectory subdirectory (new-format backup container)
240+
var backupRoot = Path.Combine(installPath, BackupRootDirectory);
241+
if (Directory.Exists(backupRoot))
242+
{
243+
allBackups.AddRange(Directory.GetDirectories(backupRoot, "*", SearchOption.TopDirectoryOnly));
244+
}
245+
246+
// Scan installPath directly for backup dirs matching patterns from defaults
247+
allBackups.AddRange(GetBackupDirectories(installPath));
248+
249+
return allBackups
250+
.OrderByDescending(d => d)
251+
.FirstOrDefault();
252+
}
253+
254+
/// <summary>
255+
/// Enumerates all backup directories in the given path using patterns derived
256+
/// from <see cref="BackupNamePrefixes"/> (both new and legacy formats).
257+
/// </summary>
258+
private static IEnumerable<string> GetBackupDirectories(string path)
259+
{
260+
foreach (var prefix in BackupNamePrefixes)
261+
{
262+
foreach (var dir in Directory.GetDirectories(path, prefix + "*", SearchOption.TopDirectoryOnly))
263+
{
264+
yield return dir;
265+
}
266+
}
267+
}
268+
197269
/// <summary>
198270
/// Recursively deletes the specified directory and all of its subdirectories and files.
199271
/// </summary>
@@ -328,19 +400,26 @@ public static bool HashEquals(string leftPath, string rightPath)
328400
/// </remarks>
329401
public static void Backup(string sourcePath, string backupPath, IReadOnlyList<string> directoryNames)
330402
{
403+
// Merge default backup-exclusion prefixes with user-configured directories.
404+
// This ensures backup/legacy directories are ALWAYS skipped, preventing
405+
// infinite recursion even when the user passes an empty skip list.
406+
var effectiveDirectories = new List<string>(directoryNames);
407+
effectiveDirectories.AddRange(BlackDefaults.DefaultDirectories);
408+
331409
if (Directory.Exists(backupPath))
332410
{
333411
DeleteDirectory(backupPath);
334412
}
335413
Directory.CreateDirectory(backupPath);
336-
CopyDirectory(sourcePath, backupPath, directoryNames);
414+
CopyDirectory(sourcePath, backupPath, effectiveDirectories);
337415
}
338416

339417
private static void CopyDirectory(string sourceDir, string targetDir, IReadOnlyList<string> directoryNames)
340418
{
341419
foreach (string dirPath in Directory.GetDirectories(sourceDir, "*", SearchOption.TopDirectoryOnly))
342420
{
343-
if (!directoryNames.Any(name => Path.GetFileName(dirPath).Contains(name)))
421+
var dirName = Path.GetFileName(dirPath);
422+
if (!directoryNames.Any(name => dirName.Contains(name)))
344423
{
345424
string newTargetDir = Path.Combine(targetDir, Path.GetFileName(dirPath));
346425
Directory.CreateDirectory(newTargetDir);
@@ -518,23 +597,49 @@ private IEnumerable<FileNode> ReadFileNode(string path, string rootPath = null)
518597
/// <param name="installPath">The application installation root directory path.</param>
519598
/// <param name="keepVersions">The number of most recent backup versions to retain. Default is 3.</param>
520599
/// <remarks>
521-
/// Backup directories are located under <c>{installPath}/__backups</c>, with each subdirectory named by version.
522-
/// This method sorts directories by version in descending order, retains the top N versions, and deletes all others.
523-
/// If a version cannot be parsed, it is treated as version <c>0.0</c> (and will be deleted first).
524-
/// If the <c>__backups</c> directory does not exist, no operation is performed.
600+
/// Scans for backup directories in two locations:
601+
/// 1. <c>{installPath}\{BackupRootDirectory}\</c> — new-format container for backup-{timestamp} dirs
602+
/// 2. <c>{installPath}\</c> directly — backup dirs matching patterns from <see cref="BlackDefaults.DefaultDirectories"/>
603+
/// Directories are sorted by name in descending order (both timestamp and version
604+
/// strings are lexicographically sortable), retaining the top N and deleting the rest.
525605
/// </remarks>
526606
public static void CleanBackup(string installPath, int keepVersions = 3)
527607
{
528-
var backupRoot = Path.Combine(installPath, "__backups");
529-
if (!Directory.Exists(backupRoot)) return;
608+
// Scan BackupRootDirectory subdirectory (new-format backup container)
609+
var backupRoot = Path.Combine(installPath, BackupRootDirectory);
610+
if (Directory.Exists(backupRoot))
611+
{
612+
CleanDirectories(backupRoot, keepVersions);
613+
}
614+
615+
// Scan installPath directly for backup dirs matching patterns from defaults
616+
foreach (var pattern in GetBackupSearchPatterns())
617+
{
618+
CleanDirectories(installPath, keepVersions, pattern);
619+
}
620+
}
621+
622+
/// <summary>
623+
/// Derives search patterns from <see cref="BackupNamePrefixes"/>
624+
/// by appending "*" to each entry (e.g. "backup-" → "backup-*").
625+
/// </summary>
626+
private static IEnumerable<string> GetBackupSearchPatterns()
627+
{
628+
foreach (var prefix in BackupNamePrefixes)
629+
{
630+
yield return prefix + "*";
631+
}
632+
}
530633

531-
var dirs = Directory.GetDirectories(backupRoot)
634+
/// <summary>
635+
/// Cleans directories matching the specified pattern in the given root path,
636+
/// retaining only the most recent N directories.
637+
/// </summary>
638+
private static void CleanDirectories(string rootPath, int keepVersions, string searchPattern = "*")
639+
{
640+
var dirs = Directory.GetDirectories(rootPath, searchPattern, SearchOption.TopDirectoryOnly)
532641
.Select(d => new DirectoryInfo(d))
533-
.OrderByDescending(d =>
534-
{
535-
var name = d.Name;
536-
return Version.TryParse(name, out var v) ? v : new Version(0, 0);
537-
})
642+
.OrderByDescending(d => d.Name)
538643
.Skip(keepVersions);
539644

540645
foreach (var dir in dirs)
@@ -547,20 +652,37 @@ public static void CleanBackup(string installPath, int keepVersions = 3)
547652
/// <param name="installPath">The application installation root directory path.</param>
548653
/// <returns>A read-only collection of <see cref="BackupInfo"/> for all backup versions.</returns>
549654
/// <remarks>
550-
/// Each backup entry contains the version name, full path, creation time, and total size in bytes.
551-
/// If the <c>__backups</c> directory does not exist, an empty collection is returned.
655+
/// Scans both <c>{installPath}\{BackupRootDirectory}</c> (new format) and <c>{installPath}</c> directly
656+
/// (backup dirs matching patterns from <see cref="BlackDefaults.DefaultDirectories"/>).
657+
/// Each backup entry contains the directory name, full path, creation time, and total size in bytes.
552658
/// </remarks>
553659
public static IReadOnlyList<BackupInfo> ListBackups(string installPath)
554660
{
555-
var backupRoot = Path.Combine(installPath, "__backups");
556-
if (!Directory.Exists(backupRoot)) return Array.Empty<BackupInfo>();
661+
var result = new List<BackupInfo>();
662+
663+
// Scan BackupRootDirectory subdirectory (new-format backup container)
664+
var backupRoot = Path.Combine(installPath, BackupRootDirectory);
665+
if (Directory.Exists(backupRoot))
666+
{
667+
result.AddRange(ToBackupInfos(backupRoot, "*"));
668+
}
669+
670+
// Scan installPath directly for backup dirs matching patterns from defaults
671+
foreach (var pattern in GetBackupSearchPatterns())
672+
{
673+
result.AddRange(ToBackupInfos(installPath, pattern));
674+
}
675+
676+
return result;
677+
}
557678

558-
return Directory.GetDirectories(backupRoot)
679+
private static IEnumerable<BackupInfo> ToBackupInfos(string rootPath, string searchPattern)
680+
{
681+
return Directory.GetDirectories(rootPath, searchPattern, SearchOption.TopDirectoryOnly)
559682
.Select(d => new DirectoryInfo(d))
560683
.Select(d => new BackupInfo(
561684
d.Name, d.FullName, d.CreationTime,
562-
d.GetFiles("*", SearchOption.AllDirectories).Sum(f => f.Length)))
563-
.ToList();
685+
d.GetFiles("*", SearchOption.AllDirectories).Sum(f => f.Length)));
564686
}
565687

566688
#endregion

src/c#/GeneralUpdate.Core/Strategy/AbstractStrategy.cs

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -387,19 +387,32 @@ private string ResolveUpdateDir()
387387

388388
/// <summary>
389389
/// Attempts to restore from backup when a pipeline execution fails.
390-
/// Only restores if a backup directory exists for the current version.
390+
/// Tries the specific backup directory for this update first;
391+
/// falls back to the most recent backup if the specific one is unavailable.
391392
/// </summary>
392393
private void TryRollback()
393394
{
394395
try
395396
{
396397
var backupDir = _configinfo.BackupDirectory;
398+
// If the specific backup for this update doesn't exist,
399+
// fall back to the most recent backup by timestamp.
400+
if (string.IsNullOrWhiteSpace(backupDir) || !Directory.Exists(backupDir))
401+
{
402+
GeneralTracer.Info($"AbstractStrategy.TryRollback: specific backup not found ({backupDir}), searching for latest.");
403+
backupDir = StorageManager.GetLatestBackup(_configinfo.InstallPath);
404+
}
405+
397406
if (!string.IsNullOrWhiteSpace(backupDir) && Directory.Exists(backupDir))
398407
{
399408
GeneralTracer.Warn($"AbstractStrategy.TryRollback: restoring from backup {backupDir} -> {_configinfo.InstallPath}");
400409
StorageManager.Restore(backupDir, _configinfo.InstallPath);
401410
GeneralTracer.Info("AbstractStrategy.TryRollback: restore completed.");
402411
}
412+
else
413+
{
414+
GeneralTracer.Warn("AbstractStrategy.TryRollback: no backup found, rollback skipped.");
415+
}
403416
}
404417
catch (Exception ex)
405418
{

src/c#/GeneralUpdate.Core/Strategy/ClientStrategy.cs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -544,8 +544,8 @@ private async Task ExecuteStandardWorkflowAsync()
544544

545545
InitBlackPolicy();
546546
_configInfo.TempPath = StorageManager.GetTempDirectory("main_temp");
547-
_configInfo.BackupDirectory = Path.Combine(_configInfo.InstallPath,
548-
$"{StorageManager.DirectoryName}{_configInfo.ClientVersion}");
547+
_configInfo.BackupDirectory = Path.Combine(_configInfo.InstallPath, StorageManager.BackupRootDirectory,
548+
StorageManager.GetBackupDirectoryName());
549549

550550
// Check failed version
551551
if (!string.IsNullOrEmpty(_configInfo.LastVersion) && CheckFail(_configInfo.LastVersion))
@@ -854,15 +854,18 @@ private void InitBlackPolicy()
854854
/// </summary>
855855
/// <remarks>
856856
/// The backup operation is performed via <c>StorageManager.Backup</c>, excluding directories configured in the blacklist.
857-
/// The backup directory path format is: {InstallPath}/backup_{ClientVersion}.
857+
/// The backup directory path format is: {InstallPath}/__backups/backup_{yyyyMMddHHmmss}.
858858
/// This step can be skipped by setting <c>UpdateContext.BackupEnabled</c> to <c>false</c>.
859+
/// After a successful backup, old backups are cleaned up, retaining only the most recent 3.
859860
/// </remarks>
860861
private void Backup()
861862
{
862863
GeneralTracer.Info(
863864
$"ClientStrategy: backing up {_configInfo!.InstallPath} -> {_configInfo.BackupDirectory}");
864865
StorageManager.Backup(_configInfo.InstallPath, _configInfo.BackupDirectory,
865866
_configInfo.Directories ?? BlackDefaults.DefaultDirectories);
867+
// Retain only the most recent 3 backups to prevent disk accumulation
868+
StorageManager.CleanBackup(_configInfo.InstallPath, keepVersions: 3);
866869
}
867870

868871
/// <summary>

0 commit comments

Comments
 (0)