Skip to content
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

Jump to bottom

Expose the original status code in StatusCodeReExecuteFeature #46539

Merged
merged 5 commits into from
Mar 4, 2023
Merged

Expose the original status code in StatusCodeReExecuteFeature #46539

merged 5 commits into from
Mar 4, 2023

Conversation

amcasey
Copy link
Member

@amcasey amcasey commented Feb 9, 2023

Expose the original status code in StatusCodeReExecuteFeature

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Expose the original status code in StatusCodeReExecuteFeature

Description

Expose the original status code in StatusCodeReExecuteFeature so that it can be used in places like StaticFileOptions.OnPrepareResponse.

With this change it will be possible to write code like

app.UseStaticFiles(new StaticFileOptions
{
    OnPrepareResponse = responseContext =>
    {
        var context = responseContext.Context;
        if (context.Response.StatusCode == StatusCodes.Status200OK &&
            context.Features[typeof(IStatusCodeReExecuteFeature)] is IStatusCodeReExecuteFeature statusCodeFeature)
        {
            context.Response.StatusCode = statusCodeFeature.OriginalStatusCode;
        }
    }
});

Fixes #46538

@ghost ghost added the area-runtime label Feb 9, 2023
Tratcher
Tratcher reviewed Feb 9, 2023
View reviewed changes
Copy link
Member

@Tratcher Tratcher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests?

src/Middleware/Diagnostics.Abstractions/src/IStatusCodeReExecuteFeature.cs Outdated Show resolved Hide resolved
@ghost
Copy link

ghost commented Feb 18, 2023

Looks like this PR hasn't been active for some time and the codebase could have been changed in the meantime.
To make sure no breaking changes are introduced, please remove the pending ci rerun label to kick off a new CI run.

@ghost ghost added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 18, 2023
@amcasey
Expose the original status code in StatusCodeReExecuteFeature
fa0a306 
...so that it can be used in places like `StaticFileOptions.OnPrepareResponse`.

With this change it will be possible to write code like

```csharp
app.UseStaticFiles(new StaticFileOptions
{
    OnPrepareResponse = responseContext =>
    {
        var context = responseContext.Context;
        if (context.Response.StatusCode == StatusCodes.Status200OK &&
            context.Features[typeof(IStatusCodeReExecuteFeature)] is IStatusCodeReExecuteFeature statusCodeFeature)
        {
            context.Response.StatusCode = statusCodeFeature.OriginalStatusCode;
        }
    }
});
```

Fixes dotnet#46538
@amcasey
Copy link
Member Author

amcasey commented Mar 3, 2023

Force push is a trivial rebase. API Review changes to follow.

@amcasey amcasey removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Mar 3, 2023
@amcasey
Copy link
Member Author

amcasey commented Mar 3, 2023

Tests?

There aren't any for IStatusCodeReExecuteFeature, AFAICT.

@build-analysis build-analysis bot mentioned this pull request Mar 3, 2023
Closed
5 tasks
@amcasey
Make setter internal
8661a8f 
...the other setters in the type should also be internal, but it's too late now.
@amcasey
Copy link
Member Author

amcasey commented Mar 3, 2023

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@amcasey amcasey merged commit 7dfe715 into dotnet:main Mar 4, 2023
@amcasey amcasey deleted the gh46538 branch March 4, 2023 01:03
@ghost ghost added this to the 8.0-preview3 milestone Mar 4, 2023
@amcasey amcasey added area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlesware and removed area-runtime labels Jun 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Tratcher Tratcher Tratcher left review comments

@BrennanConroy BrennanConroy BrennanConroy approved these changes

Assignees
No one assigned
Labels
area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlesware
Projects
None yet
Milestone
8.0-preview3
Development

Successfully merging this pull request may close these issues.

Expose the original status code in StatusCodeReExecuteFeature
3 participants
@amcasey @Tratcher @BrennanConroy