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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/swagger #218

Closed
wants to merge 73 commits into from
Closed

Conversation

@rflechner
Copy link

@rflechner rflechner commented Jan 31, 2018

Hello,

In this PR I propose a solution to generate a swagger for Giraffe.
Issue #79 has label help wanted 馃槂 .

My solution for Suave was effectively not really easy to use.
Documentation and service implementation were too strongly coupled and the DSL was really verbose.

The good news is that we still have to declare our API routes the same way as before but to enable the route analysis we have to surround the app declaration with quotation marks.

With that in place we can decouple the app declaration from the analysis required to generate the swagger documentation. In other words this solution has the avantage to avoid corrupting your service implementation.

How does it work ?

I introduced the documents function that takes two arguments:

  1. the quotation expression containing webservice implementation.
  2. a DocumentationConfig argument.

This function does the analysis of your quotation to generate Swagger documentation.

DocumentationConfig contains the following properties:

  • MethodCallRules: allow you to provide custom functions to enrich DSL and / or quotation analysis.
  • DocumentationAddendums: allow you to add more informations to the documentation without introducing service implementation modification.

I introduced ==> operator that gives the possibility to add decorations in routes implementations.

Examples

There are 2 solutions to add documentation for a route.

See example

...
operationId "send_a_car" ==>
	consumes tcar ==>
		produces typeof<Car> ==>
			route "/car2" >=> submitCar
...

using DocumentationAddendums

...
route "/car" >=> submitCar
...

let docAddendums =
    fun (route:Analyzer.RouteInfos) (path:string,verb:HttpVerb,pathDef:PathDefinition) ->
        match path,verb,pathDef with
        | "/car", HttpVerb.Post,def ->
            let ndef = 
                (def.AddConsume "model" "application/json" Body typeof<Car>)
                    .AddResponse 200 "application/json" "A car" typeof<Car>
            path, verb, ndef
...

PS: I added a SLN file to improve experience with Jetbrains Rider IDE.
I propose to create a separated NuGet for Giraffe.Swagger.
In futur, SwaggerUi could be a submodule of the repository (if you like and accept this PR 馃槃 ).

Some features could be missing and some quotations could be difficult to parse.
For the moment, analyzer works with most basics default httphandlers.

I only implemented:

  • GET
  • POST
  • PUT
  • PATCH
  • DELETE
  • route
  • routeCi
  • routef
  • setStatusCode
  • text
  • json
  • choose
  • subRouteCi
  • subRoute

You can build and run SwaggerSample/Program.fs and
go to http://localhost:5000/swaggerui/

screen_giraffe_swagger1

Regards.

@rflechner

@cotyar
Copy link

@cotyar cotyar commented Feb 15, 2018

@rflechner "I just pushed fixes." - merged your changes to the temporal https://github.com/cotyar/Giraffe/tree/SwaggerAndWebsockets

@zaaack
Copy link

@zaaack zaaack commented Mar 20, 2018

Would this support TokenRouter?

@rflechner
Copy link
Author

@rflechner rflechner commented Mar 20, 2018

I don't think but I could try to implement this in a next PR or commit.

@NatElkins
Copy link

@NatElkins NatElkins commented Jun 14, 2018

So, I just tried to get this up and running on my machine (Ubuntu 18.04) and it seemed to work. A few things though:

  • The version of the dotnet SDK specified in the global.json is not available from the 18.04 repos.
    image
    Installing the dotnet-sdk-2.1 package maps to a version number of 2.1.300. So I had to change that in the global.json to get this to build.

  • The other thing is that it looks like the dotnet-xunit CLI tool needs to be installed at least once at some point before it can work. I added a line like
    <DotNetCliToolReference Include="dotnet-xunit" Version="2.3.1" /> (based on https://www.nuget.org/packages/dotnet-xunit/) to Giraffe.Tests.fsproj and it started working. Then when I switched back to master and tried to build again, it still worked. So I think resolving the build issue is as simple as adding that line to the tests project.

Hope this helps. Super excited to see this feature get in!!

@NatElkins
Copy link

@NatElkins NatElkins commented Jun 14, 2018

And of course, please let me know if there's anything I can do to help it along @dustinmoris @rflechner

<ItemGroup>
<DotNetCliToolReference Include="dotnet-xunit" Version="2.3.*" />

This comment has been minimized.

@NatElkins

NatElkins Jun 15, 2018

I think this is why the tests are failing.

@ElijahReva
Copy link

@ElijahReva ElijahReva commented Jul 3, 2018

@rflechner Could you give me access to this branch so I could fix it?

@ElijahReva
Copy link

@ElijahReva ElijahReva commented Jul 3, 2018

@rflechner Thank you!

@rflechner
Copy link
Author

@rflechner rflechner commented Jul 3, 2018

@ElijahReva @NatElkins, is it right for you now ?

@NatElkins
Copy link

@NatElkins NatElkins commented Jul 3, 2018

@rflechner Not able to check at the moment. But while I've got you here, can I suggest making this feature an entirely separate repo/Nuget package? That would allow it to have a separate release cadence from Giraffe and be more flexible about making updates to it.

When I last tested this feature with my project, an exception was thrown at startup (didn't have a chance to look too deeply into it). I'll try to take another look and see what went wrong.

@NatElkins
Copy link

@NatElkins NatElkins commented Jul 3, 2018

Also then @dustinmoris won't have to worry about it affecting the larger Giraffe code base.

@rflechner
Copy link
Author

@rflechner rflechner commented Jul 3, 2018

This PR is a technical proposition.
I think we should create another repository / nuget (in alpha) to be more agile.
Do you want to create a repo in your organization (https://github.com/giraffe-fsharp) ?

@cotyar
Copy link

@cotyar cotyar commented Jul 3, 2018

Guys, @dustinmoris hasn't updated Giraffe since Feb.
Does anyone know if he's alright and if he still would like to continue supporting the project please?

@NatElkins
Copy link

@NatElkins NatElkins commented Jul 4, 2018

@cotyar The develop branch was updated in May.

@czifro
Copy link

@czifro czifro commented Aug 3, 2018

What's the best way to try this out while this PR is pending?

@rflechner
Copy link
Author

@rflechner rflechner commented Aug 6, 2018

I am closing this PR.
You can try the solution here: https://github.com/rflechner/SwaggerForFsharp

@rflechner rflechner closed this Aug 6, 2018
@rflechner rflechner deleted the rflechner:feature/swagger branch Aug 6, 2018
@dustinmoris
Copy link
Member

@dustinmoris dustinmoris commented Sep 30, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

9 participants
You can鈥檛 perform that action at this time.