Replace ReplicationProgressHandler with IProgress<T> - #229
Merged
Conversation
DirectoryReplicationClient.GetAccounts, ReplicateAllObjects, and FetchFullSchema now accept IProgress<ReplicationProgress> instead of the custom delegate, and gain an optional CancellationToken so long-running replication can be cancelled cooperatively between cycles. Get-ADReplAccount uses a small SynchronousProgress<T> adapter to dispatch on the pipeline thread (the BCL Progress<T> marshals through the SynchronizationContext, which would move WriteProgress off the pipeline thread). The cmdlet now also forwards Ctrl+C as a CancellationToken and converts OperationCanceledException into a clean progress completion. This is a breaking change in the DSInternals.Replication library.
Contributor
There was a problem hiding this comment.
Pull request overview
Replaces the bespoke ReplicationProgressHandler delegate with the idiomatic IProgress<ReplicationProgress> interface across DirectoryReplicationClient's replication methods, and adds optional CancellationToken parameters for cooperative cancellation between DRS cycles. The Get-ADReplAccount cmdlet is wired up to translate Ctrl+C into cancellation and to convert OperationCanceledException into a clean progress completion.
Changes:
- Introduce
ReplicationProgressrecord struct and remove the legacy delegate. - Update
GetAccounts,ReplicateAllObjects, andFetchFullSchemasignatures to useIProgress<T>+CancellationToken, with cancellation checks between cycles. - Add internal
SynchronousProgress<T>helper and wireGetADReplAccountCommandto use it, withStopProcessing/Disposemanaging aCancellationTokenSource.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Src/DSInternals.Replication/ReplicationProgressHandler.cs | Removes the legacy delegate type. |
| Src/DSInternals.Replication/ReplicationProgress.cs | New record struct carrying cookie + counts for progress reports. |
| Src/DSInternals.Replication/DirectoryReplicationClient.cs | Migrates 3 public methods to IProgress<ReplicationProgress> + CancellationToken. |
| Src/DSInternals.PowerShell/Commands/Replication/GetADReplAccountCommand.cs | Uses SynchronousProgress<T>, adds StopProcessing/Dispose for cancellation, wraps work in try/catch/finally. |
| Src/DSInternals.PowerShell/Commands/Base/SynchronousProgress.cs | New internal pipeline-thread IProgress<T> helper, with rationale in remarks. |
| Documentation/CHANGELOG.md | Documents breaking API change under Unreleased → Changed. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ReplicationProgressHandlerdelegate onDirectoryReplicationClientwith the standardIProgress<ReplicationProgress>interface acrossGetAccounts,ReplicateAllObjects, andFetchFullSchema.CancellationTokenparameters so long-running replication can be cancelled cooperatively between DRS cycles.Get-ADReplAccountto surfaceCtrl+Cas cancellation and to convertOperationCanceledExceptioninto a clean progress completion.Why
The custom delegate predates wide adoption of
System.IProgress<T>. Switching aligns the publicDSInternals.ReplicationAPI with idiomatic .NET conventions, pairs naturally withCancellationToken, and makes the surface self-documenting for external consumers.Notes for reviewers
DSInternals.Replication: callers of the affected methods must migrate fromReplicationProgressHandlertoIProgress<ReplicationProgress>. The newReplicationProgresspayload keeps the same data the old delegate received (Cookie,ProcessedObjectCount,TotalObjectCount) so external consumers that need the cookie for resumable replication keep working after a one-line adapter. Documented under### ChangedinDocumentation/CHANGELOG.md.SynchronousProgress<T>helper is intentionally used instead of the BCLProgress<T>: PowerShell cmdlets must callWriteProgresson the pipeline thread, butProgress<T>marshals through the capturedSynchronizationContext(or the thread pool when none is present), which would move the callback off the pipeline thread.Ctrl+Cexperience and is intentional.CancellationTokenSourceis scoped toGetADReplAccountCommandrather than the sharedADReplCommandBase, since it is the only cmdlet that needs it today.Test plan
dotnet build DSInternals.DotNetSdk.slnf(Release, x64) — 0 errorsdotnet test --solution DSInternals.DotNetSdk.slnfGet-ADReplAccount -Allagainst a lab DC — progress advances;Ctrl+Ccompletes cleanlyGet-ADReplAccount -All -Properties LAPS— schema progress (activity 2) still rendersGet-ADReplAccount -SamAccountName Administrator— non-bulk path still workspwsh -File Scripts/Invoke-SmokeTests.ps1 -Configuration Debug