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.

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

Updated FeatureGateAttribute to be able to be used on Razor Pages. #166

Merged
merged 9 commits into from
Mar 29, 2022

Conversation

jimmyca15
Copy link
Member

See #34 for discussion leading to this PR.

Currently the library supports controlling visibility of controllers with the FeatureGateAttribute. This works with MVC controllers and actions. It does not work with Razor pages. This is because the FeatureGateAttribute only runs as an MVC action filter. Code execution in the Razor Page pipeline is performed via page handlers.

By updating the FeatureGateAttribute to implement IAsyncPageFilter we are capable of supporting Razor Pages. MVC controllers only execute the action filter portion of FeatureGateAttribute, while Razor Pages only execute the page filter portion.

Usage

namespace RazorPages.Pages
{
    //
    // The content of this Razor page will only be served
    // if the "Home" feature is enabled.
    [FeatureGate("Home")]
    public class IndexModel : PageModel
    {
        public void OnGet()
        {
        }
    }
}

Comment on lines -9 to -18
<ItemGroup>
<None Remove="appsettings.json" />
</ItemGroup>

<ItemGroup>
<Content Include="appsettings.json">
<CopyToOutputDirectory>Always</CopyToOutputDirectory>
<CopyToPublishDirectory>PreserveNewest</CopyToPublishDirectory>
</Content>
</ItemGroup>

Choose a reason for hiding this comment

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

Curious to know why we don't need to override these options anymore.

Copy link
Member Author

Choose a reason for hiding this comment

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

In order to use Razor Pages in the test project I had to update the sdk type to Microsoft.NET.Sdk.Web. The default settings for that sdk type suffice.


<PropertyGroup>
<TargetFrameworks>netcoreapp2.1;netcoreapp3.1;net5.0</TargetFrameworks>
<TargetFrameworks>netcoreapp3.1;net5.0</TargetFrameworks>

Choose a reason for hiding this comment

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

While we are at it, should we also add .NET6 to the targets since it is the latest LTS release of .NET.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, it's a good idea. but that would be better in a separate PR to add .NET 6 targeting.

Copy link

@pratiksanglikar pratiksanglikar left a comment

Choose a reason for hiding this comment

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

Looks good to me!

}
else
{
context.Result = new NotFoundResult();
Copy link
Member

@zhenlan zhenlan Mar 29, 2022

Choose a reason for hiding this comment

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

Should we allow users to customize the behavior like for actions? #Closed

Copy link
Member Author

Choose a reason for hiding this comment

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

It's hard to say, I left it out to see if it is a requested feature. In the issue most proposals seemed to be fine with 404 responses on pages with disabled features. I have no problem with adding it.

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense. We can always add it in the future.

Copy link
Member

@zhenlan zhenlan left a comment

Choose a reason for hiding this comment

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

:shipit:

@jimmyca15 jimmyca15 merged commit 6cfe92d into microsoft:main Mar 29, 2022
jimmyca15 added a commit that referenced this pull request Jun 8, 2022
* Updated FeatureGateAttribute to be able to be used on Razor Pages. (#166)

* Add PageFeatureGate

* Fix tests.

* Remove test page.

* Added missing license headers.

* Bundle PageFeatureGateAttribute functionality into FeatureGate. Page filters and action filter execution is isolated.

* Use OkResult instead of StatusCode. Updated indenting.

* Remove unnecessary virtual for interface implementation.

* Add back netcoreapp2.1 in tests.

* Remove tabs from csproj

* Update package versions. (#167)

* Update type/method summaries in FeatureGateAttribute. (#170)

* Update type/method summaries in FeatureGateAttribute.

* Add back text.

* Add documentation for FeatureGateAttribute usage on Razor pages. (#169)

* Microsoft mandatory file (#177)

Co-authored-by: microsoft-github-policy-service[bot] <77245923+microsoft-github-policy-service[bot]@users.noreply.github.com>

Co-authored-by: microsoft-github-policy-service[bot] <77245923+microsoft-github-policy-service[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants