Add OutFile property in WebResponseObject by jshigetomi · Pull Request #24047 · PowerShell/PowerShell

jshigetomi · Jul 12, 2024

PR Summary

Added OutFile as a property in WebResponseObject. OutFile will save whenever writing to pipe line.
Refactored one line to reduce redundancy.

PR Context

PR extends WebObjectResponse with a property that stores the installed file's path. It makes sense to have this as a built in property since we have access to it.

PR Checklist

iSazonov

Please add tests for the scenario in WebCmdlets.Tests.ps1.

…ll now be saved to the WebResponseObject when -OutFile flag is used. Added Tests for directory, file, and verbose. Co-authored-by: Ilya <darpa@yandex.ru>

iSazonov · Jul 16, 2024

@mklement0 Please review.

SteveL-MSFT · Jul 16, 2024

+        /// <summary>
+        /// Gets or sets the output file path.
+        /// </summary>
+        public string? OutFile { get; set; }


should this be an internal set?

I agree (public string? OutFile { get; internal set; })

@SteveL-MSFT, @iSazonov, just to confirm: Is it generally fine to extend public classes with new members?

Other than that, LGTM - thanks for taking this on, @jshigetomi.

@mklement0 Removing public element is a breaking change, adding is ok.

@jshigetomi Please address the request.

@iSazonov Thank you for bringing this to my attention. I misread this chain and thought there were no actionable items.

iSazonov · Jul 17, 2024

@jshigetomi Please resolve merge conflicts.

…Shell into issue-21082-fix

daxian-dbw

I guess the Invoke-ResetMethod -OutFile -Passthru needs to be updated accordingly to have the similar behavior, doesn't it?

PowerShell/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/InvokeRestMethodCommand.Common.cs

Line 143 in 8016daa

    
           string outFilePath = WebResponseHelper.GetOutFilePath(response, _qualifiedOutFile);

iSazonov · Jul 19, 2024

I guess the Invoke-ResetMethod -OutFile -Passthru needs to be updated accordingly to have the similar behavior, doesn't it?

I didn't find WebResponseObject there. It is seem used only in Invoke-WebRequest.

But I found WebResponseObject and BasicHtmlWebResponseObject in PowerShellCore_format_ps1xml.cs. I believe it would be useful to add new Outfile property in the formatting.

daxian-dbw · Jul 22, 2024

I guess the Invoke-ResetMethod -OutFile -Passthru needs to be updated accordingly to have the similar behavior, doesn't it?

The Invoke-RestMethod does support -OutFile and -Passthru to be specified together, so the file information should be kept in the returned object just like Invoke-WebRequest, isn't it?

mklement0 · Jul 23, 2024

@daxian-dbw: Invoke-RestMethod -PassThru -OutFile is currently broken:

Invoke-RestMethod parameter combination of OutFile/PassThru does not write to file as intended #15409

If it did / once it does work as intended, what would be passed through is data (possibly parsed into a [pscustomobject] graph / an[xml] DOM), not a WebResponseObject instance.

…Cmdlet/CoreCLR/InvokeWebRequestCommand.CoreClr.cs

daxian-dbw · Jul 23, 2024

@mklement0 Thanks for pointing to that issue. I didn't know it's currently broken. I re-opened it.

microsoft-github-policy-service · Jul 23, 2024

📣 Hey @jshigetomi, how did we do? We would love to hear your feedback with the link below! 🗣️

🔗 https://aka.ms/PSRepoFeedback

Microsoft Forms

jshigetomi force-pushed the issue-21082-fix branch from b60059c to 9dd6213 Compare July 12, 2024 19:40

jshigetomi changed the title ~~Issue 21082 fix~~ Fix Invoke-WeRequest -PassThru -OutFile WebRequestObject Property .OutFile Stores Out File Path Jul 12, 2024

iSazonov reviewed Jul 13, 2024

View reviewed changes

Comment thread ...ft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/Common/WebResponseObject.Common.cs Outdated

Added OutFile as a property of WebResponseObject. Output file path wi…

40781e0

…ll now be saved to the WebResponseObject when -OutFile flag is used. Added Tests for directory, file, and verbose. Co-authored-by: Ilya <darpa@yandex.ru>

jshigetomi force-pushed the issue-21082-fix branch from fdf53e3 to 40781e0 Compare July 15, 2024 20:34

mklement0 mentioned this pull request Jul 16, 2024

Show file name in Invoke-WebRequest -OutFile -Verbose #24041

Merged

21 tasks

SteveL-MSFT requested changes Jul 16, 2024

View reviewed changes

microsoft-github-policy-service Bot added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Jul 16, 2024

iSazonov changed the title ~~Fix Invoke-WeRequest -PassThru -OutFile WebRequestObject Property .OutFile Stores Out File Path~~ Add OutFile property in WebResponseObject Jul 17, 2024

Merge branch 'master' into issue-21082-fix

2f5adf1

microsoft-github-policy-service Bot removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Jul 17, 2024

iSazonov approved these changes Jul 18, 2024

View reviewed changes

Justin Chung added 2 commits July 18, 2024 10:52

internal set

063e741

Merge branch 'issue-21082-fix' of https://github.com/jshigetomi/Power…

c5438a5

…Shell into issue-21082-fix

daxian-dbw requested changes Jul 18, 2024

View reviewed changes

Comment thread ...Shell.Commands.Utility/commands/utility/WebCmdlet/CoreCLR/InvokeWebRequestCommand.CoreClr.cs

microsoft-github-policy-service Bot added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Jul 18, 2024

SteveL-MSFT approved these changes Jul 22, 2024

View reviewed changes

SteveL-MSFT assigned iSazonov Jul 22, 2024

daxian-dbw reviewed Jul 23, 2024

View reviewed changes

Comment thread ...Shell.Commands.Utility/commands/utility/WebCmdlet/CoreCLR/InvokeWebRequestCommand.CoreClr.cs Outdated

daxian-dbw approved these changes Jul 23, 2024

View reviewed changes

Update src/Microsoft.PowerShell.Commands.Utility/commands/utility/Web…

b7c05f6

…Cmdlet/CoreCLR/InvokeWebRequestCommand.CoreClr.cs

daxian-dbw enabled auto-merge (squash) July 23, 2024 20:30

daxian-dbw merged commit 9995125 into PowerShell:master Jul 23, 2024

iSazonov added CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log and removed Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept labels Jul 24, 2024

jshigetomi deleted the issue-21082-fix branch July 24, 2024 15:36

sdwheeler mentioned this pull request Aug 5, 2024

Document improved behavior of web cmdlets MicrosoftDocs/PowerShell-Docs#11317

Closed

2 tasks

chrisdent-de pushed a commit to chrisdent-de/PowerShell that referenced this pull request Sep 12, 2024

Add OutFile property in WebResponseObject (PowerShell#24047)

9bc6226

Search code, repositories, users, issues, pull requests...

Conversation

jshigetomi commented Jul 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary

PR Context

PR Checklist

Uh oh!

iSazonov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

iSazonov commented Jul 16, 2024

Uh oh!

SteveL-MSFT Jul 16, 2024

Choose a reason for hiding this comment

Uh oh!

mklement0 Jul 16, 2024

Choose a reason for hiding this comment

Uh oh!

iSazonov Jul 17, 2024

Choose a reason for hiding this comment

Uh oh!

iSazonov Jul 18, 2024

Choose a reason for hiding this comment

Uh oh!

jshigetomi Jul 18, 2024

Choose a reason for hiding this comment

Uh oh!

iSazonov commented Jul 17, 2024

Uh oh!

daxian-dbw left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

iSazonov commented Jul 19, 2024

Uh oh!

daxian-dbw commented Jul 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mklement0 commented Jul 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

daxian-dbw commented Jul 23, 2024

Uh oh!

microsoft-github-policy-service Bot commented Jul 23, 2024 • edited by unfurl-links Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

jshigetomi commented Jul 12, 2024 •

edited

Loading

daxian-dbw commented Jul 22, 2024 •

edited

Loading

mklement0 commented Jul 23, 2024 •

edited

Loading

microsoft-github-policy-service Bot commented Jul 23, 2024 •

edited by unfurl-links Bot

Loading