Skip to content

Conversation

@spaasis
Copy link
Contributor

@spaasis spaasis commented Oct 6, 2025

No description provided.

@spaasis
Copy link
Contributor Author

spaasis commented Oct 6, 2025

@sergey-tihon Thanks for the help so far! I got up to running tests on the new controllers, but now another test fails that I haven't touched...

    FSC : error FS2014: A problem occurred writing the binary 'D:\Code\SwaggerProvider\tests\SwaggerProvider.ProviderTests\obj\Release\net9.0\SwaggerProvider.ProviderTests.dll': Error in pass3 for type Swashbuckle.v3.FileControllersTests, error: Error in pass3 for type Send file and get it back@40, error: Error in GetMethodRefAsMethodDefIdx for mref = ("PostApiReturnFileSingle", "Client"), error: MethodDefNotFound

I'm very new to the F# ecosystem - sorry I'm probably missing something obvious :)

@sergey-tihon
Copy link
Member

it was not an obvious issue... i believe you found another but with caching of provided types.

the next step would be to handle response correctly, current implementation tries to deserialize response as json here https://github.com/fsprojects/SwaggerProvider/blob/master/src/SwaggerProvider.DesignTime/v3/OperationCompiler.fs#L361
we probably need to add one more case here and treat text response differently
https://github.com/fsprojects/SwaggerProvider/blob/master/src/SwaggerProvider.DesignTime/v3/OperationCompiler.fs#L388-L400


Some(MediaTypes.ApplicationOctetStream, ty)
| content when content.Keys |> Seq.exists MediaTypes.isTextMediaType ->
let textKey = content.Keys |> Seq.find MediaTypes.isTextMediaType
Copy link
Member

Choose a reason for hiding this comment

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

here you can avoid code duplication and double enumeration.

This is perfect place to use F# Active Patterns, you can refactor isTextMediaType to active pattern that match and return matched value in the same time

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the tips ❤️! As you can guess, I'm not quite F#luent yet...

I got this to a point where the text/plain test passes but text/csv does not and I can't figure out why... I'm so lost as I can't just run the test in debug mode and use breakpoints in Rider 😅

Copy link
Member

Choose a reason for hiding this comment

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

i think that this log line is the key (aspnetcore does not know how to format csv)

warn: Microsoft.AspNetCore.Mvc.Infrastructure.ObjectResultExecutor[2]
      No output formatter was found for content types 'text/csv, text/csv' to write the response.

@sergey-tihon sergey-tihon marked this pull request as ready for review October 11, 2025 21:42
Copilot AI review requested due to automatic review settings October 11, 2025 21:42
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements support for text-based media types (text/plain, text/csv) in the Swagger provider by adding new controllers, test coverage, and compilation logic for handling text responses.

  • Adds controllers that return text/plain and text/csv content types
  • Implements custom CSV output formatter for ASP.NET Core
  • Updates the operation compiler to handle text/* media types as string responses

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/Swashbuckle.WebApi.Server/Swashbuckle.WebApi.Server.fsproj Adds new ReturnTextControllers.fs to project compilation
tests/Swashbuckle.WebApi.Server/Startup.fs Registers CSV output formatter with MVC options
tests/Swashbuckle.WebApi.Server/Controllers/ReturnTextControllers.fs New controllers for text/plain and text/csv endpoints with custom formatter
tests/SwaggerProvider.ProviderTests/v3/Swashbuckle.ReturnTextControllers.Tests.fs Test cases for the new text-based endpoints
tests/SwaggerProvider.ProviderTests/SwaggerProvider.ProviderTests.fsproj Adds new test file to project compilation
src/SwaggerProvider.Runtime/RuntimeHelpers.fs Removes extra blank line
src/SwaggerProvider.DesignTime/v3/OperationCompiler.fs Adds text media type detection and string response handling

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines 11 to 18
[<HttpGet; Consumes(MediaTypes.ApplicationJson); Produces("text/plain")>]
member this.Get() =
"Hello world" |> ActionResult<string>

[<Route("api/[controller]")>]
[<ApiController>]
type ReturnCsvController() =
[<HttpGet; Consumes(MediaTypes.ApplicationJson); Produces("text/csv")>]
Copy link

Copilot AI Oct 11, 2025

Choose a reason for hiding this comment

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

Using Consumes attribute for a GET request that doesn't accept a request body is unnecessary and potentially confusing. GET requests typically don't consume request bodies.

Suggested change
[<HttpGet; Consumes(MediaTypes.ApplicationJson); Produces("text/plain")>]
member this.Get() =
"Hello world" |> ActionResult<string>
[<Route("api/[controller]")>]
[<ApiController>]
type ReturnCsvController() =
[<HttpGet; Consumes(MediaTypes.ApplicationJson); Produces("text/csv")>]
[<HttpGet; Produces("text/plain")>]
member this.Get() =
"Hello world" |> ActionResult<string>
[<Route("api/[controller]")>]
[<ApiController>]
type ReturnCsvController() =
[<HttpGet; Produces("text/csv")>]

Copilot uses AI. Check for mistakes.
@sergey-tihon sergey-tihon merged commit a1a4eba into fsprojects:master Oct 11, 2025
1 check passed
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.

2 participants