Skip to content

ApiConvention Analyzer behaviour for Non-200 Success messages #4950

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

Closed
Tornhoof opened this issue Dec 5, 2018 · 8 comments
Closed

ApiConvention Analyzer behaviour for Non-200 Success messages #4950

Tornhoof opened this issue Dec 5, 2018 · 8 comments
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates bug This issue describes a behavior which is not expected - a bug.
Milestone

Comments

@Tornhoof
Copy link
Contributor

Tornhoof commented Dec 5, 2018

Is this a Bug or Feature request?:

Bug

Steps to reproduce (preferably a link to a GitHub repo with a repro project):

  1. Create WebApp with 2.2 and paste the repro controller from below
  2. Add the analyzer package Microsoft.AspNetCore.Mvc.Api.Analyzers
  3. Look at the suggestions/warnings

screenshot

Description of the problem:

The analyzer apparently does not accept 204 NoContent as a valid success status code for controller actions which do not directly create the ActionResult object in the action method.
Instead it shows: API1001:
API1001 Action method returns a success result without a corresponding ProducesResponseType.
See Action Delete and Convention Delete

It apparently works if you create the ActionResult directly (Convention AnotherDelete) or if you add StatusCode.200OK to the API conventions (Convention MoreDelete).

Whatever you do to handle 200OK, you should do the same for 204 and probably 201 too.

Please ignore the async hints, they are not related.

Version of Microsoft.AspNetCore.Mvc or Microsoft.AspNetCore.App or Microsoft.AspNetCore.All:

2.2

Repro:

using System.Threading.Tasks;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.ApiExplorer;

namespace ApiConventionsDeletebug.Controllers
{
    [Route("api/[controller]")]
    [ApiController]
    [ApiConventionType(typeof(MyConventions))]
    public class ValuesController : ControllerBase
    {
        [HttpDelete("{id}")]
        public Task<ActionResult> Delete(int id)
        {
            return DeleteInternal(id);
        }

        private async Task<ActionResult> DeleteInternal(int id)
        {
            return NoContent();
        }


        [HttpDelete("/another/{id}")]
        public async Task<ActionResult> AnotherDelete(int id)
        {
            return NoContent();
        }

        [HttpDelete("/More/{id}")]
        public Task<ActionResult> MoreDelete(int id)
        {
            return MoreDeleteInternal(id);
        }

        private async Task<ActionResult> MoreDeleteInternal(int id)
        {
            return Ok();
        }
    }

    public static class MyConventions
    {
        [ProducesResponseType(StatusCodes.Status204NoContent)]
        [ProducesResponseType(StatusCodes.Status404NotFound)]
        [ProducesResponseType(StatusCodes.Status400BadRequest)]
        [ProducesDefaultResponseType]
        public static void Delete(
            [ApiConventionTypeMatch(ApiConventionTypeMatchBehavior.Any)]
            int id)
        {
        }

        [ProducesResponseType(StatusCodes.Status204NoContent)]
        [ProducesResponseType(StatusCodes.Status404NotFound)]
        [ProducesResponseType(StatusCodes.Status400BadRequest)]
        [ProducesDefaultResponseType]
        public static void AnotherDelete(
            [ApiConventionTypeMatch(ApiConventionTypeMatchBehavior.Any)]
            int id)
        {
        }

        [ProducesResponseType(StatusCodes.Status200OK)]
        [ProducesResponseType(StatusCodes.Status404NotFound)]
        [ProducesResponseType(StatusCodes.Status400BadRequest)]
        [ProducesDefaultResponseType]
        public static void MoreDelete(
            [ApiConventionTypeMatch(ApiConventionTypeMatchBehavior.Any)]
            int id)
        {
        }
    }
}
@mkArtakMSFT
Copy link
Member

Thanks for contacting us, @Tornhoof.
@pranavkm, can you please look into this when you'll get time? Thanks!

@mkArtakMSFT
Copy link
Member

@Tornhoof, can you also share the MyConventions type?

@Tornhoof
Copy link
Contributor Author

Tornhoof commented Dec 6, 2018

It's at the end of the repro in Details above. It's just in the same codesnippet

@mkArtakMSFT
Copy link
Member

Thanks. Found it!

@pranavkm
Copy link
Contributor

pranavkm commented Dec 7, 2018

@Tornhoof I can't reproduce the issue. Given

[Route("api/[controller]")]
[ApiController]
[ApiConventionType(typeof(DefaultApiConventions))]
public class ValuesController : ControllerBase
{
    // GET api/values
    [HttpDelete("{id}")]
    public Task<ActionResult> Delete(int id)
    {
        return DeleteInternal(id);
    }

    [HttpDelete("{id}")]
    public async Task<ActionResult> AnotherDelete(int id)
    {
        return NoContent();
    }

    private async Task<ActionResult> DeleteInternal(int id)
    {
        return NoContent();
    }
}

I get a single warning for the undocumented 204 in AnotherDelete:

Severity	Code	Description	Project	File	Line	Suppression State
Warning	API1000	Action method returns undeclared status code '204'.	WebApplication1	WebApplication1\Controllers\ValuesController.cs	21	Active

Do you have a repro app I can try out?

@Tornhoof
Copy link
Contributor Author

Tornhoof commented Dec 7, 2018

I have to apologize for hiding the Repro in a details tag in the original post, I hope you didn't assume that my Repro would be a screenshot.
Here are all required files without a details tag.

Controller and Convention

using System.Threading.Tasks;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.ApiExplorer;

namespace ApiConventionsDeletebug.Controllers
{
    [Route("api/[controller]")]
    [ApiController]
    [ApiConventionType(typeof(MyConventions))]
    public class ValuesController : ControllerBase
    {
        [HttpDelete("{id}")]
        public Task<ActionResult> Delete(int id)
        {
            return DeleteInternal(id);
        }

        private async Task<ActionResult> DeleteInternal(int id)
        {
            return NoContent();
        }


        [HttpDelete("/another/{id}")]
        public async Task<ActionResult> AnotherDelete(int id)
        {
            return NoContent();
        }

        [HttpDelete("/More/{id}")]
        public Task<ActionResult> MoreDelete(int id)
        {
            return MoreDeleteInternal(id);
        }

        private async Task<ActionResult> MoreDeleteInternal(int id)
        {
            return Ok();
        }
    }

    public static class MyConventions
    {
        [ProducesResponseType(StatusCodes.Status204NoContent)]
        [ProducesResponseType(StatusCodes.Status404NotFound)]
        [ProducesResponseType(StatusCodes.Status400BadRequest)]
        [ProducesDefaultResponseType]
        public static void Delete(
            [ApiConventionTypeMatch(ApiConventionTypeMatchBehavior.Any)]
            int id)
        {
        }

        [ProducesResponseType(StatusCodes.Status204NoContent)]
        [ProducesResponseType(StatusCodes.Status404NotFound)]
        [ProducesResponseType(StatusCodes.Status400BadRequest)]
        [ProducesDefaultResponseType]
        public static void AnotherDelete(
            [ApiConventionTypeMatch(ApiConventionTypeMatchBehavior.Any)]
            int id)
        {
        }

        [ProducesResponseType(StatusCodes.Status200OK)]
        [ProducesResponseType(StatusCodes.Status404NotFound)]
        [ProducesResponseType(StatusCodes.Status400BadRequest)]
        [ProducesDefaultResponseType]
        public static void MoreDelete(
            [ApiConventionTypeMatch(ApiConventionTypeMatchBehavior.Any)]
            int id)
        {
        }
    }
}

Project file

<Project Sdk="Microsoft.NET.Sdk.Web">

  <PropertyGroup>
    <TargetFramework>netcoreapp2.2</TargetFramework>
    <AspNetCoreHostingModel>InProcess</AspNetCoreHostingModel>
    <LangVersion>latest</LangVersion>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.AspNetCore.App" />
    <PackageReference Include="Microsoft.AspNetCore.Mvc.Api.Analyzers" Version="2.2.0" />
    <PackageReference Include="Microsoft.AspNetCore.Razor.Design" Version="2.2.0" PrivateAssets="All" />
  </ItemGroup>

</Project>

Startup

using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Mvc;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;

namespace ApiConventionsDeletebug
{
    public class Startup
    {
        public Startup(IConfiguration configuration)
        {
            Configuration = configuration;
        }

        public IConfiguration Configuration { get; }

        // This method gets called by the runtime. Use this method to add services to the container.
        public void ConfigureServices(IServiceCollection services)
        {
            services.AddMvc().SetCompatibilityVersion(CompatibilityVersion.Version_2_2);
        }

        // This method gets called by the runtime. Use this method to configure the HTTP request pipeline.
        public void Configure(IApplicationBuilder app, IHostingEnvironment env)
        {
            if (env.IsDevelopment())
            {
                app.UseDeveloperExceptionPage();
            }

            app.UseMvc();
        }
    }
}

Program

using Microsoft.AspNetCore;
using Microsoft.AspNetCore.Hosting;
namespace ApiConventionsDeletebug
{
    public class Program
    {
        public static void Main(string[] args)
        {
            CreateWebHostBuilder(args).Build().Run();
        }

        public static IWebHostBuilder CreateWebHostBuilder(string[] args) =>
            WebHost.CreateDefaultBuilder(args)
                .UseStartup<Startup>();
    }
}

@aspnet-hello aspnet-hello transferred this issue from aspnet/Mvc Dec 14, 2018
@aspnet-hello aspnet-hello assigned pranavkm and unassigned pranavkm Dec 14, 2018
@aspnet-hello aspnet-hello added this to the 3.0.0-preview3 milestone Dec 14, 2018
@aspnet-hello aspnet-hello added 1 - Ready area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates bug This issue describes a behavior which is not expected - a bug. labels Dec 14, 2018
@mkArtakMSFT mkArtakMSFT modified the milestones: 3.0.0-preview6, 3.1.0 Apr 19, 2019
@mkArtakMSFT mkArtakMSFT modified the milestones: 3.1.0, 5.0.0-preview1 Aug 27, 2019
@pranavkm pranavkm removed their assignment Jan 27, 2020
@mkArtakMSFT
Copy link
Member

Hi. Thanks for contacting us.
We're closing this issue as there was not much community interest in this ask for quite a while now.

@OldrichDlouhy
Copy link

Why is this bug closed when it is not fixed?

I would understand giving this issue low priority, but closing it just because people do not comment on it does not make the bug report invalid.

@ghost ghost locked as resolved and limited conversation to collaborators Jul 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates bug This issue describes a behavior which is not expected - a bug.
Projects
None yet
Development

No branches or pull requests

5 participants