Skip to content

Fix Copilot review: add OrdinalIgnoreCase to DefaultCleanMatcher/Defa… #273

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

Merged
JusterZhu merged 1 commit into from
May 23, 2026
Merged

Fix Copilot review: add OrdinalIgnoreCase to DefaultCleanMatcher/Defa… #273

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
84 changes: 42 additions & 42 deletions src/c#/GeneralUpdate.Differential/Matchers/DefaultCleanMatcher.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,42 +1,42 @@
using System.Collections.Generic;
using System.IO;
using System.Linq;
using GeneralUpdate.Common.FileBasic;

namespace GeneralUpdate.Differential.Matchers
{
/// <summary>
/// Default implementation of <see cref="ICleanMatcher"/> that preserves the
/// original behaviour of <c>DifferentialCore.Clean</c>.
/// <list type="bullet">
/// <item><description><see cref="Compare"/> delegates to <see cref="StorageManager.Compare"/>.</description></item>
/// <item><description><see cref="Except"/> delegates to <see cref="StorageManager.Except"/>.</description></item>
/// <item><description><see cref="Match"/> 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.</description></item>
/// </list>
/// </summary>
public class DefaultCleanMatcher : ICleanMatcher
{
private readonly StorageManager _storageManager = new StorageManager();

/// <inheritdoc/>
public ComparisonResult Compare(string sourcePath, string targetPath)
=> _storageManager.Compare(sourcePath, targetPath);

/// <inheritdoc/>
public IEnumerable<FileNode>? Except(string sourcePath, string targetPath)
=> _storageManager.Except(sourcePath, targetPath);

/// <inheritdoc/>
public FileNode? Match(FileNode newFile, IEnumerable<FileNode> 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
{
/// <summary>
/// Default implementation of <see cref="ICleanMatcher"/> that preserves the
/// original behaviour of <c>DifferentialCore.Clean</c>.
/// <list type="bullet">
/// <item><description><see cref="Compare"/> delegates to <see cref="StorageManager.Compare"/>.</description></item>
/// <item><description><see cref="Except"/> delegates to <see cref="StorageManager.Except"/>.</description></item>
/// <item><description><see cref="Match"/> 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.</description></item>
/// </list>
/// </summary>
public class DefaultCleanMatcher : ICleanMatcher
{
private readonly StorageManager _storageManager = new StorageManager();
/// <inheritdoc/>
public ComparisonResult Compare(string sourcePath, string targetPath)
=> _storageManager.Compare(sourcePath, targetPath);
/// <inheritdoc/>
public IEnumerable<FileNode>? Except(string sourcePath, string targetPath)
=> _storageManager.Except(sourcePath, targetPath);
/// <inheritdoc/>
public FileNode? Match(FileNode newFile, IEnumerable<FileNode> 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;
}
}
}
67 changes: 37 additions & 30 deletions src/c#/GeneralUpdate.Differential/Matchers/DefaultDirtyMatcher.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,30 +1,37 @@
using System.Collections.Generic;
using System.IO;
using System.Linq;

namespace GeneralUpdate.Differential.Matchers
{
/// <summary>
/// Default implementation of <see cref="IDirtyMatcher"/> that preserves the
/// original matching behaviour of <c>DifferentialCore.Dirty</c>:
/// a patch file matches an application file when the patch file's name
/// (without the <c>.patch</c> extension) equals the application file's name,
/// and the patch file carries the <c>.patch</c> extension.
/// </summary>
public class DefaultDirtyMatcher : IDirtyMatcher
{
private const string PatchFormat = ".patch";

/// <inheritdoc/>
public FileInfo? Match(FileInfo oldFile, IEnumerable<FileInfo> 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
{
/// <summary>
/// Default implementation of <see cref="IDirtyMatcher"/> that preserves the
/// original matching behaviour of <c>DifferentialCore.Dirty</c>:
/// a patch file matches an application file when the patch file's name
/// (without the <c>.patch</c> extension, case-insensitive) equals the application file's name,
/// and the patch file carries the <c>.patch</c> extension.
/// </summary>
public class DefaultDirtyMatcher : IDirtyMatcher
{
private const string PatchFormat = ".patch";

/// <inheritdoc/>
public FileInfo? Match(FileInfo oldFile, IEnumerable<FileInfo> 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;
}
}
}
76 changes: 41 additions & 35 deletions src/c#/GeneralUpdate.Differential/Pipeline/DiffPipeline.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,54 +8,79 @@
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
{
/// <summary>
/// Parallel differential pipeline with configurable parallelism, progress reporting, and cancellation.
/// Parallel differential pipeline with configurable parallelism, progress reporting,
/// pluggable matchers, and cancellation.
/// </summary>
/// <remarks>
/// Wraps the existing strategy layer and parallelizes per-file diff operations
/// using a throttled producer-consumer pattern via <see cref="SemaphoreSlim"/>.
/// Use <see cref="DiffPipelineBuilder"/> for fluent configuration.
/// Direct construction is also supported for scenarios where a builder is unnecessary.
/// </remarks>
public class DiffPipeline
{
private readonly DiffPipelineOptions _options;
private readonly IBinaryDiffer _binaryDiffer;
private readonly ICleanMatcher _cleanMatcher;
private readonly IDirtyMatcher _dirtyMatcher;
private readonly IProgress<DiffProgress>? _progress;

private const string PatchExtension = ".patch";
private const string DeleteListFileName = "generalupdate_delete_files.json";

/// <summary>
/// Initialises a new pipeline with default options (<see cref="Differ.StreamingHdiffDiffer"/>).
/// Initialises a new pipeline with default options and matchers.
/// </summary>
public DiffPipeline()
: this(new DiffPipelineOptions(), new Differ.StreamingHdiffDiffer(), null)
: this(new DiffPipelineOptions(), new Differ.StreamingHdiffDiffer(), null, null, null)
{
}

/// <summary>
/// Initialises a new pipeline with the specified options.
/// </summary>
public DiffPipeline(DiffPipelineOptions options)
: this(options, new Differ.StreamingHdiffDiffer(), null)
: this(options, new Differ.StreamingHdiffDiffer(), null, null, null)
{
}

/// <summary>
/// Initialises a new pipeline with the specified options, differ, and optional progress reporter.
/// Initialises a new pipeline with full configuration.
/// </summary>
public DiffPipeline(DiffPipelineOptions options, IBinaryDiffer binaryDiffer, IProgress<DiffProgress>? progress = null)
/// <param name="options">Pipeline options. Must not be null.</param>
/// <param name="binaryDiffer">Binary differ. Must not be null.</param>
/// <param name="cleanMatcher">Clean-phase file matcher. Defaults to <see cref="DefaultCleanMatcher"/>.</param>
/// <param name="dirtyMatcher">Dirty-phase file matcher. Defaults to <see cref="DefaultDirtyMatcher"/>.</param>
/// <param name="progress">Optional progress reporter.</param>
public DiffPipeline(
DiffPipelineOptions options,
IBinaryDiffer binaryDiffer,
ICleanMatcher? cleanMatcher = null,
IDirtyMatcher? dirtyMatcher = null,
IProgress<DiffProgress>? 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;
}

/// <summary>
/// Compares source and target directories, generating patch files in parallel.
/// Initialises a new pipeline (backward-compatible constructor, preserved for binary compatibility).
/// </summary>
public DiffPipeline(DiffPipelineOptions options, IBinaryDiffer binaryDiffer, IProgress<DiffProgress>? progress = null)
: this(options, binaryDiffer, null, null, progress)
{
}

/// <summary>
/// Compares source and target directories using the configured clean matcher,
/// generating patch files in parallel.
/// </summary>
public async Task CleanAsync(
string sourcePath,
Expand All @@ -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();

Expand All @@ -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)
{
Expand Down Expand Up @@ -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);
Expand All @@ -136,7 +160,8 @@ public async Task CleanAsync(
}

/// <summary>
/// Applies patches from patchPath to appPath in parallel.
/// Applies patches from patchPath to appPath in parallel,
/// using the configured dirty matcher.
/// </summary>
public async Task DirtyAsync(
string appPath,
Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -287,18 +305,6 @@ private static string GetTempDirectory(FileNode file, string targetPath, string
return tempDir;
}

private static FileNode? FindMatchingFile(FileNode newFile, IEnumerable<FileNode> 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));
Expand Down
Loading