Skip to content
This repository has been archived by the owner on Jan 24, 2021. It is now read-only.

Generating API documentation #1486

Closed
mat-mcloughlin opened this issue Mar 10, 2014 · 70 comments
Closed

Generating API documentation #1486

mat-mcloughlin opened this issue Mar 10, 2014 · 70 comments

Comments

@mat-mcloughlin
Copy link
Contributor

Just opening up an issue so we can discuss the possibility of having Nancy generate documentation for the applications built using it.

As an example of how this is done in our beloved Web API see here http://www.asp.net/web-api/overview/creating-web-apis/creating-api-help-pages

@jchannon
Copy link
Member

We already have documentation on routes via having a corresponding resx file for each module. However what I would like is some way of doing this and producing something like Swagger from the information we provide about the routes. http://swagger.wordnik.com/#!/pet

@thecodejunkie
Copy link
Member

Posting this from my comments on JabbR

We provide an interface

public interface IRouteMetaDataProvider
{
    object Get(string url);
}

That is called when Nancy builds the route cache. When the cache is built, Nancy creates a [RouteDescription](https://github.com/NancyFx/Nancy/blob/master/src/Nancy/Routing/RouteDescription.cs
public class DefaultRouteMetaDataProvider) instance for reach route. So we extend that class to contain a Metadata property of type object. The reason it's an object is because it's format agnostic. I don't care what metadata you want to store and how you want to use it

    public object Get(string url)
    {
        // 1 use url to get all data you want
        // 2 stick it into a DTO that fills your requirements
        // return it
    }
}

Then when it comes to presenting the information you do the following, you create a NancyModule that takes a dependency on RouteCacheProvider and from there on you have access to all the RouteDescription instances and can do whatever you want (render views, return plain objects and have conneg dictate the format etc).

We could ship a default implementation of the IRouteMetaDataProvider interface and a default implementation of a NancyModule that exposes the data in some way and give you the option to override them with your own implementations. This keeps it inline with the super-duper-happy-path

TL;DR

  1. I don't care where you get the info
  2. I don't care about your format or what you store
  3. I don't care how you expose it

All I do it give you the tools to associate METADATA with a given route
Ship it!

Thoughts?

@thecodejunkie
Copy link
Member

So, I spiked this out in 20 minutes and created a very simple IRouteMetadataProvider implementation. The interface was defines as

    public interface IRouteMetadataProvider
    {
        object GetMetadata(RouteDescription routeDescription);
    }

And my basic implementation was

    public class DefaultRouteMetadataProvider : IRouteMetadataProvider
    {
        public object GetMetadata(RouteDescription routeDescription)
        {
            return new MyRouteMetadata();
        }
    }

The MyRouteMetadata class is a custom DTO that I created to contain my application specific metadata

    public class MyRouteMetadata
    {
        public MyRouteMetadata()
        {
            this.Description = "Lorem ipsum";
            this.ValidStatusCodes = new[] { HttpStatusCode.Accepted, HttpStatusCode.OK, HttpStatusCode.Processing };
        }

        public string Description { get; set; }

        public IEnumerable<HttpStatusCode> ValidStatusCodes { get; set; }
    }

I then created a simple route, in a module that took a dependency on IRouteCacheProvider (currently in Nancy) and returned a view, passing the
cache to the view as a model

    Get["/meta"] = parameters => {
        return View["meta", routeCacheProvider.GetCache()];
    };

The meta view is a simple Razor view

    <h1>Route metadata</h1>
    <p>Below is a list of route metadata that was generated from a custom IRouteMetadataProvider implementation</p>

    <div class="meta">
        @foreach (var module in @Model)
        {
            foreach (var route in @module.Value)
            {
                var meta = (MyRouteMetadata)route.Item2.Metadata;

                <div>
                    <h1>@route.Item2.Path</h1>
                    <h3>@route.Item2.Method</h3>

                    Description:
                    <p>@meta.Description</p>

                    Valid return status codes:
                    <p>
                        @string.Join(", ", @meta.ValidStatusCodes)
                    </p>

                </div>
            }
        }
    </div>

And that produced the following view
image

@thecodejunkie
Copy link
Member

As you can see I can now have a custom DTO for my metadata, meaning I can stick what ever stuff I want in there. Code samples, descriptions, return codes, error codes, urls to end points, links to samples etc..

If I'd returned a Negotiator instead then I could have requested for my metadata in any negotionable format (json, xml and view out of the box) and add more formats by implementing one or more IResponseProcessor implementations

Word of caution for negotiation. This is a very quick spike and the IRouteCache probably wouldn't serialize to json very well, so you should probably create you own model type. I could have added Path and Model to my MyRouteMetadata type and passed in an IEnumerable<MyRouteMetadata> as the model instead and it would work just fine.

@thecodejunkie
Copy link
Member

By the way, I extended RouteDescription to have an object Metadata { get; set; } property where the metadata DTO is shoved in. It's done by the IRouteCache when it builds the cache at application startup.

I will push the code to a branch on my Nancy fork so you can play with it and see if it does what you want. Would be nice to see if we could connect it to some third party tool like Swagger as well

@thecodejunkie
Copy link
Member

Updated it again. This time I added a CodeSample property to my MyRouteMetadata DTO and stuck a bit of code in there. I then extended my view to download highlight.js and highlight my code. This is what how turned out
image
Pretty sure this is the most flexible approach we can add - and yet it's so easy!

@thecodejunkie
Copy link
Member

Don't you all comment at once ;)

@chrissie1
Copy link

I'm ok with this.

@felipeleusin
Copy link
Contributor

Don't know how helpful this will be, but i've been using http://apidocjs.com/ when working with nodejs and most of my coworkers/pms really liked it. Maybe it can give some inspiration, I know there is some conceptual differences between Node and Nancy, specially because we can't really comment on the properties but still.

One thing I really like is being able to include metadata alongside route definition, I think it's going but maybe a [RouteResponse(string contentType, type response)] parameter that get's picked up...

@thecodejunkie
Copy link
Member

You can find the code at https://github.com/thecodejunkie/Nancy/tree/route-metadata-spike
Look at the IRouteMetadataProvider interface. In the same file you will find the DefaultRouteMetadataProvider and MyRouteMetadata sample implementations.
The metadata itself it associated with the RouteDescription.Metadata property in the IRouteCache.BuildCache method (which would be in RouteCache.BuildCache for the default implementation)

@dealproc
Copy link

Interested to see how this develops. For api development, this is something that I am considering.

@thecodejunkie
Copy link
Member

@felipeleusin I am afraid that that will never work for Nancy, unless the C# team changes how you can use attributes. A route declaration in Nancy (this is only slightly true, keep reading) is a property call (Get etc are properties that returns an item with an indexer on), and you can only put attributes on declarations, not call operations. That said, this only holds true for the default implementation of the INancyModule interface (the default implementation being the NancyModule class).

You can provide alternative implementations, such as we have done in our demo https://github.com/NancyFx/Nancy/tree/master/src/Nancy.Demo.CustomModule where we created a new base class, called the UglifiedNancyModule which base its route declarations on methods, like ASP.NET MVC/WebAPI. You could create your own, similar implementation, and have your IRouteMetadataProvider read the metadata of your method declarations.

Personally I think the inline approach is only viable for the simplest form of documentation. As soon as you want to include a bit more verbose description, return values, exceptions, code samples etc. then it's going to be very clunky to keep inline. I am more a fan of declaring these separate to my routes so I can easily manage them independently of you routes.

That said, this is just a proof-of-concept spike and we're more than willing to have alternate spikes/suggestions brought to the table so we can expose ourselves to as many scenarions as possible and get the best/most flexible implementation into Nancy

@khellang
Copy link
Member

@thecodejunkie Just a thought; what if we kept the condition and route action delegates around as expressions instead (or in addition to the compiled delegates)? The routeExpression.ToString() could be interesting to have in the documentation.

@thecodejunkie
Copy link
Member

@khellang I was thinking the same last night when I decided to push in the entire RouteDescription into the IRouteMetadataProvider. I wonder what kind of impact it would have on the code and performance

@jchannon
Copy link
Member

There's only one way to find out.....Go! :)

@thecodejunkie
Copy link
Member

Updated the code with the following changes

  • Introduced RouteMetadata and replaced the IDictionary<Type, string>
    in RouteDescription.Metadata with it
  • Made it possible to have multiple IRouteMetadataProviders
  • Updated sample

@thecodejunkie
Copy link
Member

Another update

  • Added INancyModule parameter to the IRouteMetadataProvider.GetMetadata method signature. It will be the module instance that the route belongs too.
  • Moved DefaultRouteMetadataProvider and MyRouteMetadata types to the Nancy.Demo.Hosting.Aspnet project

@jchannon
Copy link
Member

From a use case scenario in returning a model in a route to display this meta data I would like to see it moved from

Get["/metajson"] = parameters =>
{
    var routeDescriptions = routeCacheProvider.GetCache().Values.SelectMany(v => v).Select(tup => tup.Item2);

    var model = routeDescriptions.Select(x => x.Metadata.Retrieve<MyRouteMetadata>());

    return model;
};

to

Get["/metajson"] = parameters =>
{
    var model = routeCacheProvider.GetCache().Retrieve<MyRouteMetadata>();
    return model;
};

That way its a readable format and you still keep the generics in there so you can ask for the type of meta data you might have

@jchannon
Copy link
Member

Have we got a slight duplication issue in that a lot of RouteDescription properties will end up on MyRouteMetadata like:

public string Description { get; set; }
public string Method { get; private set; }
public string Path { get; private set; }
public IEnumerable<string> Segments { get; set; }

I guess they may not always end up in your meta data class though depending on what you want to give out on your meta data

@thecodejunkie
Copy link
Member

More updates

  • Added a Retrieve<TMetadata>() extension method on IRouteCache according to @jchannon suggestion. You can now get metadata using routeCacheProvider.GetCache().Retrieve<MyRouteMetadata>();
  • Updated MyRouteMetadata to contain Path and Method
  • Updated /meta sample route to use the new Retrieve<TMetadata>() extension

You can now negotiate the route. Try calling it using /meta.json or call it using the appropriate accept-headers). XML doesn't work because the type is not compatible with XML serialization. Too lazy to fix it atm, it's only a sample metadata type anyway :)

@khellang
Copy link
Member

Could the Retrieve<T> extension method on IRouteCache be renamed to RetrieveMetadata<T> so it's clear that it's actually metadata you're retrieving? 😄

@thecodejunkie
Copy link
Member

@khellang Had the exact same thought this moment. Made the change locally, will push within the next 30 minutes :)

@khellang
Copy link
Member

👍 Now the only thing left is to make the IRouteMetadataProvider interface generic 😉 Maybe we should create an issue for ADATS to support open generics (type definitions)?

@thecodejunkie
Copy link
Member

@khellang we could spike it, but we could have to test performance of it. Could be quite a bit slower

@thecodejunkie
Copy link
Member

Updated

  • Renamed the Retrieve<TMetadata> extension method (on IRouteCache) to RetrieveMetadata<TMetadata>

@khellang
Copy link
Member

@thecodejunkie The scanning is actually already done, i.e. the types are already in the ADATS cache. The only difference is that we need to change from Assembly.IsAssignableFrom to a version that supports generic type definitions (or add an extra, alternative check when Type.IsGenericTypeDefinition is true), so you could do AppDomainAssemblyTypeScanner.TypesOf(typeof(IRouteMetadataProvider<>)). We actually already have a method to do that 😄 See TypeExtensions.IsAssignableToGenericType. I don't think that will affect perf much...

@jchannon
Copy link
Member

What would making it generic give us?

Also, I think we should remove IRouteDescriptionProvider and move the code in the DefaultRouteDescriptionProvider to DefaultMetaDataProvider so out of the box the meta data is the descriptions from the .resources files.

@khellang
Copy link
Member

@jchannon It would allow us to find types based on generic type definitions, e.g. IRouteMetadataProvider<>, which in turn would enable us to make interfaces generic, so we could drop the Type MetadataType { get; } property and change object GetMetadata() to TMetadata GetMetadata() 😄

@thecodejunkie
Copy link
Member

Additional updates

  • @khellang sent a pull-request that introduced RouteMetadataProvider base class. The class exposes a protected abstract TMetadata GetRouteMetadata(INancyModule module, RouteDescription routeDescription) method.

@liddellj
Copy link
Contributor

liddellj commented Apr 9, 2014

Been playing with this today, using the bits from @thecodejunkie's metadata branch and @khellang's Nancy.Swagger repo. It would be really nice to have the option to augment the metadata at the point a route is defined, as per @khellang's example above.

What if we changed RouteBuilder to allow an Action<RouteMetadata> to be optionally passed in? Here's what I've spiked out:

Get["/{name}", null, meta =>
{
    meta.Description("Greets the given name);
    meta.Parameter("name").Description("The name to greet.").Required();
}] = parameters =>
{
    return "Hello " + parameters.name;
};

The Description and Parameter methods are extension methods defined in Nancy.Swagger to assist in augmenting the metadata retrieved from the provider. This requires that RouteCache be modified to invoke a MetadataMutator if found:

if (routeDescription.MetadataMutator != null)
{
    routeDescription.MetadataMutator.Invoke(routeDescription.Metadata);
}

And that RouteDescription be modified to provide the MetadataMutator property. This approach allows basic information to be obtained through the IRouteMetadataProvider, which can then be optionally enriched when the route is defined. Through extension methods on RouteMetadata - you can provide a nice syntax for doing so.

I thought it would be best to discuss the general approach here before I take it much further. Things I'm currently uncomfortable with:

  • Introducing the Action<RouteMetadata> parameter to the route definitions requires passing in nulls to invoke to correct overload, otherwise the compiler couldn't distinguish between the existing Func<NancyContext, bool> parameters and a new Action<RouteMetadata> parameter.
  • I think I agree with @jchannon - there seems to be a lot of overlap between route descriptions and route metadata. I think the two concepts should be combined (which would be a breaking change)
  • Is MetadataMutator the right name? It's not a factory, and it's not a builder.

@liddellj
Copy link
Contributor

Capturing a discussion on JabbR between myself, @jchannon, @khellang and @grumpydev.

People generally not keen on aesthetics of including route metadata in Nancy module alongside route definitions. @grumpydev suggested that introducing 'named routes' would provide a way to reference routes elsewhere - rather than referring to them by path. This would enable various techniques for populating metadata:

  • Make use of RouteDescription.Name in an IRouteMetadataProvider implementation.
  • Introduce the concept of a convention based MetadataModule which sits alongside a regular module to provide metadata based on route names.

Example:

public class ProductsModule : NancyModule
{
    public ProductsModule() : base("products")
    {
        Get["GetAll", "/", true] = async (_, ctx) =>
        {
            return stuffs;
        };
    }
}

public class ProductsMetadataModule : MetadataModule
{
    public ProductsMetadataModule()
    {
        Describe["GetAll"] = meta =>
        {
            // interesting stuff here
        };
    }
}

@grumpydev opened a PR as an exploration of named routes: #1508

@grumpydev
Copy link
Member

image

@khellang
Copy link
Member

Can we rename RouteDescription to RouteData once and for all?

@thecodejunkie
Copy link
Member

I just merged #1497 into master

@liddellj
Copy link
Contributor

I've opened a PR (#1519) which spikes out support for metadata modules. Keen to see what people think.

@liddellj
Copy link
Contributor

liddellj commented May 1, 2014

Think I'm done with #1519 - unless anyone has any further feedback. Travis build has failed but I'm pretty sure it's environmental rather than an issue with the code, can it be kicked to run again?

@grumpydev grumpydev added this to the Future milestone Jun 10, 2014
@timschlechter
Copy link

Interesting discussion!

I really like Swagger and it would be very nice to generate API documentation in Swagger format. I made several attempts and found it pretty hard to find a clean solution because of the dynamic nature of Nancy (which is a good thing! ;-)
Last week I implemented a solution which gives you some constraints, but works pretty nice and I think it's worth sharing. It's absolutely work in progress and it's far from complete, but feedback is highly appreciated.

Please let me know what you think: https://github.com/TimSchlechter/Nancy.Swagger
(please create issues in repo, to keep this thread clean)

@khellang
Copy link
Member

😲 I can't believe this... The duplicated effort. I don't know whether to laugh or cry. There goes the rest of my motivation... Even the NuGet package name got nicked.

@timschlechter
Copy link

Oh sorry, didn't mean to do that. I was trying some stuff and thought it was worth sharing. I did saw your project and thought it only contained the swagger spec model, I did not figured out how to actually use it to provide the documentation endpoint in a Nancy application.

I'd really like to coop if the ideas are similar. May I suggest you add some more information in the readme of your repo?

@liddellj
Copy link
Contributor

I agree - this is a bit disappointing from a duplication point of view. @khellang and I have been collaborating on https://github.com/khellang/Nancy.Swagger, which aims to achieve exactly the same thing you have, with few key differences.

As per @thecodejunkie's suggestion - if we separate the job of describing metadata from rendering it, then the metadata module approach discussed in this thread becomes one possible method of describing metadata. The work I have ongoing against @khellang's Nancy.Swagger repo makes use of the IRouteMetadataProvider infrastructure to obtain the metadata, so it doesn't care how the metadata is described.

Perhaps a good way forward would be to turn the smart attribute based approach of describing metadata you have developed into an alternate package (Nancy.Metadata.Attributes?) which could then potentially be used to describe any type of metadata. If that package resulted in an alternate implementation of IRouteMetadataProvider, it would allow that package to be used outside the context of Swagger for other API formats.

We could then pool our efforts together to make a Nancy.Swagger implementation which makes use of IRouteMetadataProvider to obtain metadata, thereby allowing people to choose how they want to describe their metadata.

Coincidentally I was planning on resuming my work on this tomorrow - by tidying up and pushing my work on providing a Swagger DSL in the form of extension methods from IRouteDescription for @khellang's repo.

Thoughts?

@khellang
Copy link
Member

Well, we started on the model cause that's the thing that is most complex and one thing we can agree on (since Wordnik has already made the decisions for us).

I highly doubt you will get someone to use the API you're suggesting there, you even have to specify information Nancy already knows about, like URI templates and HTTP methods, using attributes. It also seems like it's forcing a method syntax, which is a reason why many people move away from MVC/WebAPI and over to Nancy in the first place. I could go on about NancyApiDiscoverer and AppDomainAssemblyTypeScanner, and hard coded TinyIoCContainer etc, but it's not really my point.

I'm just truly disappointed that we couldn't collaborate on this effort and find a decent way to provide a working Swagger metadata API for Nancy, before pushing things to NuGet.

I guess I'm to blame since I didn't provide a better README in the repo, or push to NuGet earlier 😕

@liddellj
Copy link
Contributor

@khellang I don't think there would have been any value in pushing a work-in-progress to NuGet. It's just a shame you can't reserve package names... (as far as I'm aware)

@timschlechter
Copy link

Again, my apologies. I pushed to nuget because I'm actually using this at a client and some other people where interested. I'll unlist the versions I pushed, and I'm willing to handover the ownership of the nuget package name.

Specifying information Nancy allready knows does only count for the uri template and http method. But as far I can see you have to make this mapping at some point (wether it's by name, or path/http-method).
The other things you mention (NancyApiDiscoverer , AppDomainAssemblyTypeScanner, TinyIoCContainer) are implementation details within the package and can (will) be changed.
As far I can see it does not force any kind of method syntax.

But I agree with you guys, the package really targets Swagger and it differs pretty much from the RouteMetadata direction. Maybe I can use a less generic Nancy name for my attribute-solution (which gives me results right now, which I actually need), and see if I can coop with your ideas to work on the more generic solution.

@timschlechter
Copy link

"I don't think there would have been any value in pushing a work-in-progress to NuGet. It's just a shame you can't reserve package names... (as far as I'm aware)"

Well you could push a pre-release: http://docs.nuget.org/docs/reference/versioning#Prerelease_Versions

I can hand over the nuget-package name (making you owners) so you can use it for the solution you're working on. Since it's only downloaded by a handful of people, I don't think this will be a problem.

@liddellj
Copy link
Contributor

@khellang @timschlechter I've pushed up the changes I had brewing here: yahehe/Nancy.Swagger#3, along with basic instructions on how to use them. Feedback appreciated.

@timschlechter
Copy link

Awesome, such a SwaggerModule was exactly what I was missing. I will certainly dig into this.

@aanavaneeth
Copy link

Nancy.Swagger is still in the pre-release and it is supporting Swagger spec 1.2. Any plans to update to Swagger 2.0?

@Corstiaan84
Copy link
Contributor

Hi guys. I am doing some prep for a new project. Really want to use Nancy again but the api docs are an important part of the new project. Like @agasthyanavaneeth I am also interested in the status of all things Swagger 2.0. Any news? :-). By the looks of the current repo things are still 1.2?

@khellang
Copy link
Member

@Corstiaan84 There is a PR going for Swagger 2.0; yahehe/Nancy.Swagger#64.

@Corstiaan84
Copy link
Contributor

@khellang cool, thanks

@khellang
Copy link
Member

OK, I think it's time for me to put Nancy.Swagger up for adoption.

Here are my reasons:

I'm not using Nancy much for production (read; work) stuff anymore. Quite frankly, I don't do much server-side web at all ATM, so I've lost some interest 😞

I also lost quite a bit of steam realizing how much frickin' work it takes to get some decent metadata out of Nancy, simply because of its dynamic nature. Everything has to be explicitly declared, hand-holding the framework every step of the way, and I don't see this changing. Not very SDHP! 😜

And finally; time is a scarse, invaluable resource, and there are tons of things to do. Based on the above, I've concluded that spending huge amounts of my time on Nancy.Swagger is not really what I want. It's not fair to anyone that the project just sits there because I don't have interest or cycles to spend on it.

So... If someone would like to keep working on it, or simply scrap it altogether and use the name for a new project, just shout out, and I'll transfer the repository and NuGet package owner rights. 😄

Thanks! 👯

@jchannon
Copy link
Member

Please don't scrap it etc as we use it for our API documentation and use the nuget libs 😄

@khellang khellang removed this from the Future milestone Aug 19, 2015
@xlegalles
Copy link

Hi, so what is the status of this request? My company is currently starting the development of a project based on a microservices architecture. Nancy is a very good candidate for backend services but we all know that it is mandatory to be able to document/expose the REST API. And Swagger is a nice tool to do it. I don't like Nancy.Swagger with seperate modules because it is the best way to have it disconnected from the implementation. I would really prefer what @liddellj suggests and have the option to augment the metadata at the point a route is defined.
Honestly, if we can't document a Nancy API other than using Hypermedia Controls, we will certainly reconsider our choice and use ASP.Net. Too bad :(
Thank you for your answer

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests