Skip to content

Add minimal option to webapi template #36068

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

Merged
merged 6 commits into from
Sep 7, 2021

Conversation

DamianEdwards
Copy link
Member

  • Add "minimal" option to webapi project template
  • Factor Program.cs into multiple files and update template manifest to exclude/rename dependent on selected options
  • Updated controller and minimal versions to set endpoint/route name when EnableOpenAPI is true
  • Configure webapi template minimal option for VS display as "Use controllers"

Fixes #35996

- Add "minimal" option to webapi project template
- Factor Program.cs into multiple files and update template manifest to exclude/rename dependent on selected options
- Updated controller and minimal versions to set endpoint/route name when EnableOpenAPI is true
- Configure webapi template minimal option for VS display as "Use controllers"
@ghost ghost added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Sep 1, 2021
@DamianEdwards
Copy link
Member Author

Here's how the Program.cs looks if minimal is selected:

var builder = WebApplication.CreateBuilder(args);

// Add services to the container.
builder.Services.AddEndpointsApiExplorer();
builder.Services.AddSwaggerGen();

var app = builder.Build();

// Configure the HTTP request pipeline.
if (app.Environment.IsDevelopment())
{
    app.UseSwagger();
    app.UseSwaggerUI();
}

app.UseHttpsRedirection();

var summaries = new[]
{
    "Freezing", "Bracing", "Chilly", "Cool", "Mild", "Warm", "Balmy", "Hot", "Sweltering", "Scorching"
};

app.MapGet("/weatherforecast", () =>
{
    var forecast =  Enumerable.Range(1, 5).Select(index => new WeatherForecast
    {
        Date = DateTime.Now.AddDays(index),
        TemperatureC = Random.Shared.Next(-20, 55),
        Summary = summaries[Random.Shared.Next(summaries.Length)]
    })
    .ToArray();
    return forecast;
})
.WithName("GetWeatherForecast");

app.Run();

class WeatherForecast
{
    public DateTime Date { get; set; }

    public int TemperatureC { get; set; }

    public int TemperatureF => 32 + (int)(TemperatureC / 0.5556);

    public string? Summary { get; set; }
}

And here it is with --auth SingleOrg:

using Microsoft.AspNetCore.Authentication;
using Microsoft.AspNetCore.Authentication.JwtBearer;
using Microsoft.Identity.Web;
using Microsoft.Identity.Web.Resource;

var builder = WebApplication.CreateBuilder(args);

// Add services to the container.
builder.Services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme)
    .AddMicrosoftIdentityWebApi(builder.Configuration.GetSection("AzureAd"));
builder.Services.AddAuthorization();

builder.Services.AddEndpointsApiExplorer();
builder.Services.AddSwaggerGen();

var app = builder.Build();

// Configure the HTTP request pipeline.
if (app.Environment.IsDevelopment())
{
    app.UseSwagger();
    app.UseSwaggerUI();
}

app.UseHttpsRedirection();

app.UseAuthentication();
app.UseAuthorization();

var scopeRequiredByApi = app.Configuration["AzureAd:Scopes"];
var summaries = new[]
{
    "Freezing", "Bracing", "Chilly", "Cool", "Mild", "Warm", "Balmy", "Hot", "Sweltering", "Scorching"
};

app.MapGet("/weatherforecast", (HttpContext httpContext) =>
{
    httpContext.VerifyUserHasAnyAcceptedScope(scopeRequiredByApi);

    var forecast =  Enumerable.Range(1, 5).Select(index => new WeatherForecast
    {
        Date = DateTime.Now.AddDays(index),
        TemperatureC = Random.Shared.Next(-20, 55),
        Summary = summaries[Random.Shared.Next(summaries.Length)]
    })
    .ToArray();
    return forecast;
})
.WithName("GetWeatherForecast")
.RequireAuthorization();

app.Run();

class WeatherForecast
{
    public DateTime Date { get; set; }

    public int TemperatureC { get; set; }

    public int TemperatureF => 32 + (int)(TemperatureC / 0.5556);

    public string? Summary { get; set; }
}

@davidfowl
Copy link
Member

Should WeatherForecast be in another file?

@DamianEdwards
Copy link
Member Author

@davidfowl that's up to us. I moved it into Program.cs given what we've shown so far everywhere else with minimal APIs, plus it also means removing the example is just code in the one file. But I'm not super invested in either approach.

Made the template baseline test more resilient by ensuring that all template arg options without values are added to the project key rather than a specific few. Args that have a value are still not added to the key. Keys are all tracked now to ensure uniqueness & an exception is thrown if they aren't. Renamed a few things for better clarity and easy of debugging too.

text += AuthenticationOptionRegex.Match(arguments)
.Groups.TryGetValue("auth", out var auth) ? auth.Value : "";
private string CreateProjectKey(string arguments)
Copy link
Member

Choose a reason for hiding this comment

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

Only minor question I had was to confirm that we don't care about ordering variation... since "dotnet new blazorwasm -ho --auth Individual" won't be considered the same as "dotnet new blazorwasm --auth Individual -ho", probably not an issue since our tests hopefully are always consistent

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a good question as this routine would now produce a unique key. I think the "worst" that would happen in that case is we end up with a duplicate test, rather than the worst case now, being tests that should be unique, aren't, and the tests fail with somewhat obscure errors.

Copy link
Member Author

Choose a reason for hiding this comment

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

I can add a sort to make it a non-issue.

Copy link
Member Author

Choose a reason for hiding this comment

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

@DamianEdwards DamianEdwards requested a review from HaoK September 2, 2021 21:58
Comment on lines 114 to 117
var argumentsArray = arguments
.Split(" --", int.MaxValue, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries)
.SelectMany(e => e.Split(" -", StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries))
.ToArray();
Copy link
Member

Choose a reason for hiding this comment

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

Is this any different than the following?

Suggested change
var argumentsArray = arguments
.Split(" --", int.MaxValue, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries)
.SelectMany(e => e.Split(" -", StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries))
.ToArray();
var argumentsArray = arguments
.Split(new[] { " --", " -" }, StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries)
.ToArray();

Copy link
Member Author

Choose a reason for hiding this comment

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

So I tried that first and it didn't seem to produce what I wanted, but it may be that my intent changed during refactoring. I can try again although I found the meaning of passing a string array as slightly ambiguous WRT to pass ordering.

Copy link
Member

Choose a reason for hiding this comment

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

My basic testing shows it gives the same result "new templatename -minimal -au SingleOrg --another-option OptionValue". I nerd sniped @BrennanConroy who found this in the docs.

To avoid ambiguous results when strings in separator have characters in common, the Split method proceeds from the beginning to the end of the value of the instance, and matches the first element in separator that is equal to a delimiter in the instance. The order in which substrings are encountered in the instance takes precedence over the order of elements in separator.

https://docs.microsoft.com/en-us/dotnet/api/system.string.split?view=net-5.0#System_String_Split_System_String___System_StringSplitOptions_

So it seems it should work.

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed it locally and it does work. I'll roll it into the change to make WeatherForecast a record too.

- Change WeatherForecast to a record
- Simplify method in test
@davidfowl
Copy link
Member

This looks really good @DamianEdwards! Thanks for getting this done so quickly

@DamianEdwards
Copy link
Member Author

@dougbu can you merge this please?

@dougbu dougbu merged commit fb14e61 into release/6.0 Sep 7, 2021
@dougbu dougbu deleted the damianedwards/minimal-api-template-35996 branch September 7, 2021 17:42
DamianEdwards added a commit that referenced this pull request Sep 7, 2021
* Add minimal option to webapi template

- Add "minimal" option to webapi project template
- Factor Program.cs into multiple files and update template manifest to exclude/rename dependent on selected options
- Updated controller and minimal versions to set endpoint/route name when EnableOpenAPI is true
- Configure webapi template minimal option for VS display as "Use controllers"

* Update template baselines & fix casing of option description

* Fix template baseline tests issue

* Update template baseline test to be more resilient

Made the template baseline test more resilient by ensuring that all template arg options without values are added to the project key rather than a specific few. Args that have a value are still not added to the key. Keys are all tracked now to ensure uniqueness & an exception is thrown if they aren't. Renamed a few things for better clarity and easy of debugging too.

* Make template baseline test project key disregard ordering

* Update based on feedback

- Change WeatherForecast to a record
- Simplify method in test
DamianEdwards added a commit that referenced this pull request Sep 7, 2021
- Add "minimal" option to webapi project template
- Factor Program.cs into multiple files and update template manifest to exclude/rename dependent on selected options
- Updated controller and minimal versions to set endpoint/route name when EnableOpenAPI is true
- Configure webapi template minimal option for VS display as "Use controllers"
@halter73 halter73 added this to the 6.0-rc2 milestone Sep 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants