From 9e29b6d53cb2f519808be45bb9e3454cf32969c8 Mon Sep 17 00:00:00 2001 From: MartinGC94 Date: Sun, 6 Nov 2022 03:20:45 +0100 Subject: [PATCH 1/6] Do not require activity when creating a completed progress record --- .../commands/utility/WriteProgressCmdlet.cs | 49 ++++++++++--------- .../Write-Progress.Tests.ps1 | 4 ++ 2 files changed, 31 insertions(+), 22 deletions(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WriteProgressCmdlet.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WriteProgressCmdlet.cs index fd5476751ad..45f2dbd6f7a 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WriteProgressCmdlet.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WriteProgressCmdlet.cs @@ -9,59 +9,54 @@ namespace Microsoft.PowerShell.Commands /// /// Implements the write-progress cmdlet. /// - [Cmdlet(VerbsCommunications.Write, "Progress", HelpUri = "https://go.microsoft.com/fwlink/?LinkID=2097036", RemotingCapability = RemotingCapability.None)] + [Cmdlet(VerbsCommunications.Write, "Progress", DefaultParameterSetName = "Processing", HelpUri = "https://go.microsoft.com/fwlink/?LinkID=2097036", RemotingCapability = RemotingCapability.None)] public sealed class WriteProgressCommand : PSCmdlet { /// /// Describes the activity for which progress is being reported. /// - [Parameter( - Position = 0, - Mandatory = true, - HelpMessageBaseName = HelpMessageBaseName, - HelpMessageResourceId = "ActivityParameterHelpMessage")] + [Parameter(Position = 0, Mandatory = true, ParameterSetName = "Processing", HelpMessageBaseName = HelpMessageBaseName, HelpMessageResourceId = "ActivityParameterHelpMessage")] + [Parameter(Position = 0, ParameterSetName = "Completed", HelpMessageBaseName = HelpMessageBaseName, HelpMessageResourceId = "ActivityParameterHelpMessage")] public string Activity { get; set; } /// /// Describes the current state of the activity. /// - [Parameter( - Position = 1, - HelpMessageBaseName = HelpMessageBaseName, - HelpMessageResourceId = "StatusParameterHelpMessage")] + [Parameter(Position = 1, ParameterSetName = "Processing", HelpMessageBaseName = HelpMessageBaseName, HelpMessageResourceId = "StatusParameterHelpMessage")] [ValidateNotNullOrEmpty] public string Status { get; set; } = WriteProgressResourceStrings.Processing; /// /// Uniquely identifies this activity for purposes of chaining subordinate activities. /// - [Parameter(Position = 2)] + [Parameter(Position = 2, ParameterSetName = "Processing")] + [Parameter(Position = 1, ParameterSetName = "Completed")] [ValidateRange(0, int.MaxValue)] public int Id { get; set; } /// /// Percentage completion of the activity, or -1 if n/a. /// - [Parameter] + [Parameter(ParameterSetName = "Processing")] [ValidateRange(-1, 100)] public int PercentComplete { get; set; } = -1; /// /// Seconds remaining to complete the operation, or -1 if n/a. /// - [Parameter] + [Parameter(ParameterSetName = "Processing")] public int SecondsRemaining { get; set; } = -1; /// /// Description of current operation in activity, empty if n/a. /// - [Parameter] + [Parameter(ParameterSetName = "Processing")] public string CurrentOperation { get; set; } /// /// Identifies the parent Id of this activity, or -1 if none. /// - [Parameter] + [Parameter(ParameterSetName = "Processing")] [ValidateRange(-1, int.MaxValue)] public int ParentId { get; set; } = -1; @@ -69,7 +64,8 @@ public sealed class WriteProgressCommand : PSCmdlet /// Identifies whether the activity has completed (and the display for it should be removed), /// or if it is proceeding (and the display for it should be shown). /// - [Parameter] + [Parameter(ParameterSetName = "Processing")] + [Parameter(Mandatory = true, ParameterSetName = "Completed")] public SwitchParameter Completed { get @@ -96,12 +92,21 @@ protected override void ProcessRecord() { - ProgressRecord pr = new(Id, Activity, Status); - pr.ParentActivityId = ParentId; - pr.PercentComplete = PercentComplete; - pr.SecondsRemaining = SecondsRemaining; - pr.CurrentOperation = CurrentOperation; - pr.RecordType = this.Completed ? ProgressRecordType.Completed : ProgressRecordType.Processing; + ProgressRecord pr; + if (Completed || ParameterSetName == "Completed") + { + pr = new(Id, "null", Status); + pr.RecordType = ProgressRecordType.Completed; + } + else + { + pr = new(Id, Activity, Status); + pr.ParentActivityId = ParentId; + pr.PercentComplete = PercentComplete; + pr.SecondsRemaining = SecondsRemaining; + pr.CurrentOperation = CurrentOperation; + pr.RecordType = ProgressRecordType.Processing; + } WriteProgress(SourceId, pr); } diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/Write-Progress.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/Write-Progress.Tests.ps1 index 0f62cbcb6a6..1993adf662e 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/Write-Progress.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/Write-Progress.Tests.ps1 @@ -28,4 +28,8 @@ Describe "Write-Progress DRT Unit Tests" -Tags "CI" { { Write-Progress -Activity $activity -Status ('b' * ([console]::WindowWidth + 1)) -Id 1 } | Should -Not -Throw Write-Progress -Activity $activity -Id 1 -Completed } + + It 'Should be able to complete a progress record with no activity specified' { + { Write-Progress -Completed } | Should -Not -Throw + } } From 981a9085ab828c1ebe509cfaf3a0787b6060114c Mon Sep 17 00:00:00 2001 From: MartinGC94 Date: Wed, 1 Mar 2023 22:16:41 +0100 Subject: [PATCH 2/6] Revert parameterset changes. Remove mandatory property from Activity and set a default value instead. --- .../commands/utility/WriteProgressCmdlet.cs | 50 ++++++++----------- 1 file changed, 22 insertions(+), 28 deletions(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WriteProgressCmdlet.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WriteProgressCmdlet.cs index 45f2dbd6f7a..05532ea7765 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WriteProgressCmdlet.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WriteProgressCmdlet.cs @@ -9,54 +9,58 @@ namespace Microsoft.PowerShell.Commands /// /// Implements the write-progress cmdlet. /// - [Cmdlet(VerbsCommunications.Write, "Progress", DefaultParameterSetName = "Processing", HelpUri = "https://go.microsoft.com/fwlink/?LinkID=2097036", RemotingCapability = RemotingCapability.None)] + [Cmdlet(VerbsCommunications.Write, "Progress", HelpUri = "https://go.microsoft.com/fwlink/?LinkID=2097036", RemotingCapability = RemotingCapability.None)] public sealed class WriteProgressCommand : PSCmdlet { /// /// Describes the activity for which progress is being reported. /// - [Parameter(Position = 0, Mandatory = true, ParameterSetName = "Processing", HelpMessageBaseName = HelpMessageBaseName, HelpMessageResourceId = "ActivityParameterHelpMessage")] - [Parameter(Position = 0, ParameterSetName = "Completed", HelpMessageBaseName = HelpMessageBaseName, HelpMessageResourceId = "ActivityParameterHelpMessage")] - public string Activity { get; set; } + [Parameter( + Position = 0, + HelpMessageBaseName = HelpMessageBaseName, + HelpMessageResourceId = "ActivityParameterHelpMessage")] + public string Activity { get; set; } = " "; /// /// Describes the current state of the activity. /// - [Parameter(Position = 1, ParameterSetName = "Processing", HelpMessageBaseName = HelpMessageBaseName, HelpMessageResourceId = "StatusParameterHelpMessage")] + [Parameter( + Position = 1, + HelpMessageBaseName = HelpMessageBaseName, + HelpMessageResourceId = "StatusParameterHelpMessage")] [ValidateNotNullOrEmpty] public string Status { get; set; } = WriteProgressResourceStrings.Processing; /// /// Uniquely identifies this activity for purposes of chaining subordinate activities. /// - [Parameter(Position = 2, ParameterSetName = "Processing")] - [Parameter(Position = 1, ParameterSetName = "Completed")] + [Parameter(Position = 2)] [ValidateRange(0, int.MaxValue)] public int Id { get; set; } /// /// Percentage completion of the activity, or -1 if n/a. /// - [Parameter(ParameterSetName = "Processing")] + [Parameter] [ValidateRange(-1, 100)] public int PercentComplete { get; set; } = -1; /// /// Seconds remaining to complete the operation, or -1 if n/a. /// - [Parameter(ParameterSetName = "Processing")] + [Parameter] public int SecondsRemaining { get; set; } = -1; /// /// Description of current operation in activity, empty if n/a. /// - [Parameter(ParameterSetName = "Processing")] + [Parameter] public string CurrentOperation { get; set; } /// /// Identifies the parent Id of this activity, or -1 if none. /// - [Parameter(ParameterSetName = "Processing")] + [Parameter] [ValidateRange(-1, int.MaxValue)] public int ParentId { get; set; } = -1; @@ -64,8 +68,7 @@ public sealed class WriteProgressCommand : PSCmdlet /// Identifies whether the activity has completed (and the display for it should be removed), /// or if it is proceeding (and the display for it should be shown). /// - [Parameter(ParameterSetName = "Processing")] - [Parameter(Mandatory = true, ParameterSetName = "Completed")] + [Parameter] public SwitchParameter Completed { get @@ -92,21 +95,12 @@ protected override void ProcessRecord() { - ProgressRecord pr; - if (Completed || ParameterSetName == "Completed") - { - pr = new(Id, "null", Status); - pr.RecordType = ProgressRecordType.Completed; - } - else - { - pr = new(Id, Activity, Status); - pr.ParentActivityId = ParentId; - pr.PercentComplete = PercentComplete; - pr.SecondsRemaining = SecondsRemaining; - pr.CurrentOperation = CurrentOperation; - pr.RecordType = ProgressRecordType.Processing; - } + ProgressRecord pr = new(Id, Activity, Status); + pr.ParentActivityId = ParentId; + pr.PercentComplete = PercentComplete; + pr.SecondsRemaining = SecondsRemaining; + pr.CurrentOperation = CurrentOperation; + pr.RecordType = this.Completed ? ProgressRecordType.Completed : ProgressRecordType.Processing; WriteProgress(SourceId, pr); } From 491dfe0f057b190aa78e9d8be4718faa36c61af6 Mon Sep 17 00:00:00 2001 From: MartinGC94 Date: Thu, 2 Mar 2023 12:03:08 +0100 Subject: [PATCH 3/6] Allow null or empty strings for activity and remove test that is no longer valid. --- .../commands/utility/WriteProgressCmdlet.cs | 15 +++++++++++++-- .../Write-Progress.Tests.ps1 | 4 ---- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WriteProgressCmdlet.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WriteProgressCmdlet.cs index 05532ea7765..e772babc69d 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WriteProgressCmdlet.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WriteProgressCmdlet.cs @@ -1,7 +1,6 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -using System; using System.Management.Automation; namespace Microsoft.PowerShell.Commands @@ -19,7 +18,18 @@ public sealed class WriteProgressCommand : PSCmdlet Position = 0, HelpMessageBaseName = HelpMessageBaseName, HelpMessageResourceId = "ActivityParameterHelpMessage")] - public string Activity { get; set; } = " "; + public string Activity + { + get + { + return _activity; + } + + set + { + _activity = string.IsNullOrEmpty(value) ? " " : value; + } + } /// /// Describes the current state of the activity. @@ -106,6 +116,7 @@ protected override } private bool _completed; + private string _activity = " "; private const string HelpMessageBaseName = "WriteProgressResourceStrings"; } diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/Write-Progress.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/Write-Progress.Tests.ps1 index 8a4a022242d..25770eb9a67 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/Write-Progress.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/Write-Progress.Tests.ps1 @@ -1,10 +1,6 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. Describe "Write-Progress DRT Unit Tests" -Tags "CI" { - It "Should be able to throw exception when missing mandatory parameters" { - { Write-Progress $null } | Should -Throw -ErrorId 'ParameterArgumentValidationErrorNullNotAllowed,Microsoft.PowerShell.Commands.WriteProgressCommand' - } - It "Should be able to throw exception when running Write-Progress with bad percentage" { { Write-Progress -Activity 'myactivity' -Status 'mystatus' -percent 101 } | Should -Throw -ErrorId 'ParameterArgumentValidationError,Microsoft.PowerShell.Commands.WriteProgressCommand' From cec91525aaec81f81b9c33ccb67f84ab0d675a4d Mon Sep 17 00:00:00 2001 From: MartinGC94 Date: Mon, 27 Mar 2023 20:27:16 +0200 Subject: [PATCH 4/6] Add comment explaining the default value --- .../commands/utility/WriteProgressCmdlet.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WriteProgressCmdlet.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WriteProgressCmdlet.cs index e772babc69d..173818c48a2 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WriteProgressCmdlet.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WriteProgressCmdlet.cs @@ -27,6 +27,8 @@ public string Activity set { + // Activity used to be mandatory and the underlying API expects a value + // so we use a whitespace value as a default value. _activity = string.IsNullOrEmpty(value) ? " " : value; } } From bcfbfc76ae10395511c76ce18d2f360be4a962a0 Mon Sep 17 00:00:00 2001 From: MartinGC94 Date: Wed, 7 Jun 2023 22:34:40 +0200 Subject: [PATCH 5/6] Apply WG recommendations --- .../commands/utility/WriteProgressCmdlet.cs | 40 +++++++++++-------- .../engine/ProgressRecord.cs | 27 ++++++++++++- 2 files changed, 49 insertions(+), 18 deletions(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WriteProgressCmdlet.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WriteProgressCmdlet.cs index 173818c48a2..f0776d4a2ac 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WriteProgressCmdlet.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WriteProgressCmdlet.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System; using System.Management.Automation; namespace Microsoft.PowerShell.Commands @@ -18,20 +19,7 @@ public sealed class WriteProgressCommand : PSCmdlet Position = 0, HelpMessageBaseName = HelpMessageBaseName, HelpMessageResourceId = "ActivityParameterHelpMessage")] - public string Activity - { - get - { - return _activity; - } - - set - { - // Activity used to be mandatory and the underlying API expects a value - // so we use a whitespace value as a default value. - _activity = string.IsNullOrEmpty(value) ? " " : value; - } - } + public string Activity { get; set; } /// /// Describes the current state of the activity. @@ -107,7 +95,28 @@ protected override void ProcessRecord() { - ProgressRecord pr = new(Id, Activity, Status); + ProgressRecord pr; + if (string.IsNullOrEmpty(Activity)) + { + if (!Completed) + { + ThrowTerminatingError(new ErrorRecord( + new ArgumentException("Missing value for mandatory parameter.", nameof(Activity)), + "MissingActivity", + ErrorCategory.InvalidArgument, + Activity)); + return; + } + else + { + pr = new(Id); + } + } + else + { + pr = new(Id, Activity, Status); + } + pr.ParentActivityId = ParentId; pr.PercentComplete = PercentComplete; pr.SecondsRemaining = SecondsRemaining; @@ -118,7 +127,6 @@ protected override } private bool _completed; - private string _activity = " "; private const string HelpMessageBaseName = "WriteProgressResourceStrings"; } diff --git a/src/System.Management.Automation/engine/ProgressRecord.cs b/src/System.Management.Automation/engine/ProgressRecord.cs index cd5b33107aa..c9e0008a53d 100644 --- a/src/System.Management.Automation/engine/ProgressRecord.cs +++ b/src/System.Management.Automation/engine/ProgressRecord.cs @@ -59,6 +59,25 @@ class ProgressRecord this.status = statusDescription; } + /// + /// Initializes a new instance of the ProgressRecord class and defines the activity Id. + /// + /// + /// A unique numeric key that identifies the activity to which this record applies. + /// + public + ProgressRecord(int activityId) + { + if (activityId < 0) + { + // negative Ids are reserved to indicate "no id" for parent Ids. + + throw PSTraceSource.NewArgumentOutOfRangeException(nameof(activityId), activityId, ProgressRecordStrings.ArgMayNotBeNegative, "activityId"); + } + + this.id = activityId; + } + /// /// Cloning constructor (all fields are value types - can treat our implementation of cloning as "deep" copy) /// @@ -488,9 +507,13 @@ internal static ProgressRecord FromPSObjectForRemoting(PSObject progressAsPSObje /// This object as a PSObject property bag. internal PSObject ToPSObjectForRemoting() { - PSObject progressAsPSObject = RemotingEncoder.CreateEmptyPSObject(); + // Activity used to be mandatory but that's no longer the case. + // We ensure the string has a value to maintain compatibility with older versions. + string activity = string.IsNullOrEmpty(Activity) ? " " : Activity; - progressAsPSObject.Properties.Add(new PSNoteProperty(RemoteDataNameStrings.ProgressRecord_Activity, this.Activity)); + PSObject progressAsPSObject = RemotingEncoder.CreateEmptyPSObject(); + + progressAsPSObject.Properties.Add(new PSNoteProperty(RemoteDataNameStrings.ProgressRecord_Activity, activity)); progressAsPSObject.Properties.Add(new PSNoteProperty(RemoteDataNameStrings.ProgressRecord_ActivityId, this.ActivityId)); progressAsPSObject.Properties.Add(new PSNoteProperty(RemoteDataNameStrings.ProgressRecord_StatusDescription, this.StatusDescription)); From a7b9f2e0a502e0ba099a6a9a14171c617914ccfe Mon Sep 17 00:00:00 2001 From: MartinGC94 Date: Tue, 20 Jun 2023 00:22:40 +0200 Subject: [PATCH 6/6] Set status for completed progress records. --- .../commands/utility/WriteProgressCmdlet.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WriteProgressCmdlet.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WriteProgressCmdlet.cs index f0776d4a2ac..0751954c54e 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WriteProgressCmdlet.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WriteProgressCmdlet.cs @@ -110,6 +110,7 @@ protected override else { pr = new(Id); + pr.StatusDescription = Status; } } else