From a992688be3eb34c0f3ca29e3c2a7e9eec8bcd678 Mon Sep 17 00:00:00 2001 From: Juster Zhu Date: Sat, 23 May 2026 16:16:15 +0800 Subject: [PATCH] Fix Copilot review: add OrdinalIgnoreCase to DefaultCleanMatcher/DefaultDirtyMatcher, fix .patch stripping, restore old DiffPipeline constructor for binary compat --- .../Matchers/DefaultCleanMatcher.cs | 84 +++++++++---------- .../Matchers/DefaultDirtyMatcher.cs | 67 ++++++++------- .../Pipeline/DiffPipeline.cs | 76 +++++++++-------- 3 files changed, 120 insertions(+), 107 deletions(-) diff --git a/src/c#/GeneralUpdate.Differential/Matchers/DefaultCleanMatcher.cs b/src/c#/GeneralUpdate.Differential/Matchers/DefaultCleanMatcher.cs index a9d45a18..ef52e1e5 100644 --- a/src/c#/GeneralUpdate.Differential/Matchers/DefaultCleanMatcher.cs +++ b/src/c#/GeneralUpdate.Differential/Matchers/DefaultCleanMatcher.cs @@ -1,42 +1,42 @@ -using System.Collections.Generic; -using System.IO; -using System.Linq; -using GeneralUpdate.Common.FileBasic; - -namespace GeneralUpdate.Differential.Matchers -{ - /// - /// Default implementation of that preserves the - /// original behaviour of DifferentialCore.Clean. - /// - /// delegates to . - /// delegates to . - /// considers a new file matched to an old file when both share the - /// same name, both exist on disk, and they reside at the same relative path. - /// - /// - public class DefaultCleanMatcher : ICleanMatcher - { - private readonly StorageManager _storageManager = new StorageManager(); - - /// - public ComparisonResult Compare(string sourcePath, string targetPath) - => _storageManager.Compare(sourcePath, targetPath); - - /// - public IEnumerable? Except(string sourcePath, string targetPath) - => _storageManager.Except(sourcePath, targetPath); - - /// - public FileNode? Match(FileNode newFile, IEnumerable leftNodes) - { - var oldFile = leftNodes.FirstOrDefault(i => - string.Equals(i.Name, newFile.Name) && - string.Equals(i.RelativePath, newFile.RelativePath)); - if (oldFile is null) return null; - if (!File.Exists(oldFile.FullName)) return null; - if (!File.Exists(newFile.FullName)) return null; - return oldFile; - } - } -} +using System.Collections.Generic; +using System.IO; +using System.Linq; +using GeneralUpdate.Common.FileBasic; + +namespace GeneralUpdate.Differential.Matchers +{ + /// + /// Default implementation of that preserves the + /// original behaviour of DifferentialCore.Clean. + /// + /// delegates to . + /// delegates to . + /// considers a new file matched to an old file when both share the + /// same name, both exist on disk, and they reside at the same relative path. + /// + /// + public class DefaultCleanMatcher : ICleanMatcher + { + private readonly StorageManager _storageManager = new StorageManager(); + + /// + public ComparisonResult Compare(string sourcePath, string targetPath) + => _storageManager.Compare(sourcePath, targetPath); + + /// + public IEnumerable? Except(string sourcePath, string targetPath) + => _storageManager.Except(sourcePath, targetPath); + + /// + public FileNode? Match(FileNode newFile, IEnumerable leftNodes) + { + var oldFile = leftNodes.FirstOrDefault(i => + string.Equals(i.Name, newFile.Name, System.StringComparison.OrdinalIgnoreCase) && + string.Equals(i.RelativePath, newFile.RelativePath, System.StringComparison.OrdinalIgnoreCase)); + if (oldFile is null) return null; + if (!File.Exists(oldFile.FullName)) return null; + if (!File.Exists(newFile.FullName)) return null; + return oldFile; + } + } +} diff --git a/src/c#/GeneralUpdate.Differential/Matchers/DefaultDirtyMatcher.cs b/src/c#/GeneralUpdate.Differential/Matchers/DefaultDirtyMatcher.cs index aa28d527..b6ca75d5 100644 --- a/src/c#/GeneralUpdate.Differential/Matchers/DefaultDirtyMatcher.cs +++ b/src/c#/GeneralUpdate.Differential/Matchers/DefaultDirtyMatcher.cs @@ -1,30 +1,37 @@ -using System.Collections.Generic; -using System.IO; -using System.Linq; - -namespace GeneralUpdate.Differential.Matchers -{ - /// - /// Default implementation of that preserves the - /// original matching behaviour of DifferentialCore.Dirty: - /// a patch file matches an application file when the patch file's name - /// (without the .patch extension) equals the application file's name, - /// and the patch file carries the .patch extension. - /// - public class DefaultDirtyMatcher : IDirtyMatcher - { - private const string PatchFormat = ".patch"; - - /// - public FileInfo? Match(FileInfo oldFile, IEnumerable patchFiles) - { - var findFile = patchFiles.FirstOrDefault(f => - Path.GetFileNameWithoutExtension(f.Name).Replace(PatchFormat, "").Equals(oldFile.Name)); - - if (findFile != null && string.Equals(Path.GetExtension(findFile.FullName), PatchFormat)) - return findFile; - - return null; - } - } -} +using System.Collections.Generic; +using System.IO; +using System.Linq; + +namespace GeneralUpdate.Differential.Matchers +{ + /// + /// Default implementation of that preserves the + /// original matching behaviour of DifferentialCore.Dirty: + /// a patch file matches an application file when the patch file's name + /// (without the .patch extension, case-insensitive) equals the application file's name, + /// and the patch file carries the .patch extension. + /// + public class DefaultDirtyMatcher : IDirtyMatcher + { + private const string PatchFormat = ".patch"; + + /// + public FileInfo? Match(FileInfo oldFile, IEnumerable patchFiles) + { + var findFile = patchFiles.FirstOrDefault(f => + { + var name = Path.GetFileNameWithoutExtension(f.Name); + // Strip only a trailing .patch extension (case-insensitive) + if (name.EndsWith(PatchFormat, System.StringComparison.OrdinalIgnoreCase)) + name = name.Substring(0, name.Length - PatchFormat.Length); + return name.Equals(oldFile.Name, System.StringComparison.OrdinalIgnoreCase); + }); + + if (findFile != null && + Path.GetExtension(findFile.FullName).Equals(PatchFormat, System.StringComparison.OrdinalIgnoreCase)) + return findFile; + + return null; + } + } +} diff --git a/src/c#/GeneralUpdate.Differential/Pipeline/DiffPipeline.cs b/src/c#/GeneralUpdate.Differential/Pipeline/DiffPipeline.cs index 7c5cca77..22e6e1c8 100644 --- a/src/c#/GeneralUpdate.Differential/Pipeline/DiffPipeline.cs +++ b/src/c#/GeneralUpdate.Differential/Pipeline/DiffPipeline.cs @@ -8,31 +8,35 @@ using GeneralUpdate.Common.HashAlgorithms; using GeneralUpdate.Common.Internal.JsonContext; using GeneralUpdate.Differential.Abstractions; +using GeneralUpdate.Differential.Matchers; using GeneralUpdate.Differential.Models; namespace GeneralUpdate.Differential.Pipeline { /// - /// Parallel differential pipeline with configurable parallelism, progress reporting, and cancellation. + /// Parallel differential pipeline with configurable parallelism, progress reporting, + /// pluggable matchers, and cancellation. /// /// - /// Wraps the existing strategy layer and parallelizes per-file diff operations - /// using a throttled producer-consumer pattern via . /// Use for fluent configuration. + /// Direct construction is also supported for scenarios where a builder is unnecessary. /// public class DiffPipeline { private readonly DiffPipelineOptions _options; private readonly IBinaryDiffer _binaryDiffer; + private readonly ICleanMatcher _cleanMatcher; + private readonly IDirtyMatcher _dirtyMatcher; private readonly IProgress? _progress; + private const string PatchExtension = ".patch"; private const string DeleteListFileName = "generalupdate_delete_files.json"; /// - /// Initialises a new pipeline with default options (). + /// Initialises a new pipeline with default options and matchers. /// public DiffPipeline() - : this(new DiffPipelineOptions(), new Differ.StreamingHdiffDiffer(), null) + : this(new DiffPipelineOptions(), new Differ.StreamingHdiffDiffer(), null, null, null) { } @@ -40,22 +44,43 @@ public DiffPipeline() /// Initialises a new pipeline with the specified options. /// public DiffPipeline(DiffPipelineOptions options) - : this(options, new Differ.StreamingHdiffDiffer(), null) + : this(options, new Differ.StreamingHdiffDiffer(), null, null, null) { } /// - /// Initialises a new pipeline with the specified options, differ, and optional progress reporter. + /// Initialises a new pipeline with full configuration. /// - public DiffPipeline(DiffPipelineOptions options, IBinaryDiffer binaryDiffer, IProgress? progress = null) + /// Pipeline options. Must not be null. + /// Binary differ. Must not be null. + /// Clean-phase file matcher. Defaults to . + /// Dirty-phase file matcher. Defaults to . + /// Optional progress reporter. + public DiffPipeline( + DiffPipelineOptions options, + IBinaryDiffer binaryDiffer, + ICleanMatcher? cleanMatcher = null, + IDirtyMatcher? dirtyMatcher = null, + IProgress? progress = null) { _options = options ?? throw new ArgumentNullException(nameof(options)); _binaryDiffer = binaryDiffer ?? throw new ArgumentNullException(nameof(binaryDiffer)); + _cleanMatcher = cleanMatcher ?? new DefaultCleanMatcher(); + _dirtyMatcher = dirtyMatcher ?? new DefaultDirtyMatcher(); _progress = progress; } /// - /// Compares source and target directories, generating patch files in parallel. + /// Initialises a new pipeline (backward-compatible constructor, preserved for binary compatibility). + /// + public DiffPipeline(DiffPipelineOptions options, IBinaryDiffer binaryDiffer, IProgress? progress = null) + : this(options, binaryDiffer, null, null, progress) + { + } + + /// + /// Compares source and target directories using the configured clean matcher, + /// generating patch files in parallel. /// public async Task CleanAsync( string sourcePath, @@ -67,8 +92,7 @@ public async Task CleanAsync( var reporter = progress ?? _progress; ValidateDirectories(sourcePath, targetPath, patchPath); - var storageManager = new StorageManager(); - var comparisonResult = storageManager.Compare(sourcePath, targetPath); + var comparisonResult = _cleanMatcher.Compare(sourcePath, targetPath); var differentFiles = comparisonResult.DifferentNodes.ToList(); var leftNodes = comparisonResult.LeftNodes.ToList(); @@ -90,7 +114,7 @@ public async Task CleanAsync( cancellationToken.ThrowIfCancellationRequested(); var tempDir = GetTempDirectory(file, targetPath, patchPath); - var oldFile = FindMatchingFile(file, leftNodes); + var oldFile = _cleanMatcher.Match(file, leftNodes); if (oldFile != null) { @@ -125,7 +149,7 @@ public async Task CleanAsync( await Task.WhenAll(tasks); - var exceptFiles = storageManager.Except(sourcePath, targetPath)?.ToList(); + var exceptFiles = _cleanMatcher.Except(sourcePath, targetPath)?.ToList(); if (exceptFiles is { Count: > 0 }) { var deletePath = Path.Combine(patchPath, DeleteListFileName); @@ -136,7 +160,8 @@ public async Task CleanAsync( } /// - /// Applies patches from patchPath to appPath in parallel. + /// Applies patches from patchPath to appPath in parallel, + /// using the configured dirty matcher. /// public async Task DirtyAsync( string appPath, @@ -165,17 +190,11 @@ public async Task DirtyAsync( int completed = 0; var semaphore = new SemaphoreSlim(_options.MaxDegreeOfParallelism); + // Match old files to patches using the pluggable dirty matcher var matchedPairs = new List<(FileInfo OldFile, FileInfo PatchFile)>(); foreach (var oldFile in oldFiles) { - var patchFile = patchFiles.FirstOrDefault(f => - { - var name = Path.GetFileNameWithoutExtension(f.Name); - if (name.EndsWith(".patch", StringComparison.OrdinalIgnoreCase)) - name = name.Substring(0, name.Length - 6); - return name.Equals(oldFile.Name, StringComparison.OrdinalIgnoreCase); - }); - + var patchFile = _dirtyMatcher.Match(oldFile, patchFiles); if (patchFile != null) matchedPairs.Add((oldFile, patchFile)); } @@ -221,7 +240,6 @@ private async Task ApplyPatch(string appFilePath, string patchFilePath, Cancella await _binaryDiffer.DirtyAsync(appFilePath, tempPath, patchFilePath, ct); - // Atomic replacement if (File.Exists(appFilePath)) { File.SetAttributes(appFilePath, FileAttributes.Normal); @@ -287,18 +305,6 @@ private static string GetTempDirectory(FileNode file, string targetPath, string return tempDir; } - private static FileNode? FindMatchingFile(FileNode newFile, IEnumerable leftNodes) - { - var match = leftNodes.FirstOrDefault(i => - string.Equals(i.Name, newFile.Name, StringComparison.OrdinalIgnoreCase) && - string.Equals(i.RelativePath, newFile.RelativePath, StringComparison.OrdinalIgnoreCase)); - - if (match == null) return null; - if (!File.Exists(match.FullName)) return null; - if (!File.Exists(newFile.FullName)) return null; - return match; - } - private static void ValidateDirectories(string sourcePath, string targetPath, string patchPath) { if (string.IsNullOrWhiteSpace(sourcePath)) throw new ArgumentNullException(nameof(sourcePath));