-
Notifications
You must be signed in to change notification settings - Fork 76
Greg #1
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
base: attributerouting
Are you sure you want to change the base?
Greg #1
Changes from all commits
a595793
ae552f8
aadd5df
9dbd972
3f22ce6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,18 +14,17 @@ public static class SwaggerNet | |
| { | ||
| public static void PreStart() | ||
| { | ||
| RouteTable.Routes.MapHttpRoute( | ||
| name: "SwaggerResourceList", | ||
| routeTemplate: "apidocs/swagger", | ||
| defaults: new { swagger = true, controller = "Swagger", action = "GetResourceList" } | ||
| ); | ||
| RouteTable.Routes.MapHttpRoute( | ||
| name: "SwaggerApiDeclaration", | ||
| routeTemplate: "apidocs/{controllerName}", | ||
| defaults: new { swagger = true, controller = "Swagger", action = "GetApiDeclaration" } | ||
| ); | ||
| //RouteTable.Routes.MapHttpRoute( | ||
| // name: "SwaggerResourceList", | ||
| // routeTemplate: "api/swagger", | ||
| // defaults: new { swagger = true, controller = "Swagger", action = "GetResourceList" } | ||
| //); | ||
| //RouteTable.Routes.MapHttpRoute( | ||
| // name: "SwaggerApiDeclaration", | ||
| // routeTemplate: "apidocs/{controllerName}", | ||
| // defaults: new { swagger = true, controller = "Swagger", action = "GetApiDeclaration" } | ||
| //); | ||
|
|
||
| SwaggerConfiguration.DefaultConfiguration = SwaggerConfiguration.CreateDefaultConfig(GlobalConfiguration.Configuration); | ||
| } | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line is essential for configuration -- this is where the developer user of the library would configure it. |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,10 @@ | ||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Linq; | ||
| using System.Reflection; | ||
| using System.Web; | ||
| using System.Web.Http; | ||
| using System.Web.Http.Description; | ||
| using System.Web.Mvc; | ||
| using System.Web.Optimization; | ||
| using System.Web.Routing; | ||
|
|
@@ -22,6 +24,11 @@ protected void Application_Start() | |
| FilterConfig.RegisterGlobalFilters(GlobalFilters.Filters); | ||
| RouteConfig.RegisterRoutes(RouteTable.Routes); | ||
| BundleConfig.RegisterBundles(BundleTable.Bundles); | ||
|
|
||
| var baseType = HttpContext.Current.ApplicationInstance.GetType().BaseType; | ||
| var assemblyname = Assembly.GetAssembly(baseType).GetName().Name; | ||
| var path = HttpContext.Current.Server.MapPath("~/bin/" + assemblyname + ".xml"); | ||
| GlobalConfiguration.Configuration.Services.Replace(typeof(IDocumentationProvider),new XmlCommentDocumentationProvider(path)); | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be in the library, or at least in the App_start\swaggerNet.cs file, not left up to App global.asax. Eg there should never have to be instructions like "install via nuget, then go add these lines to global.asax.."
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that's fine. I just tossed it in the first place I thought of. |
||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| <h1>Welcome to ASP.NET Web API!</h1> | ||
| Navigate to <a href="/docs">/docs</a> for the Swagger UI docs. These are not included in the NuGet package.<br /> | ||
| Navigate to <a href="/apidocs/swagger">/api/swagger</a> to see the JSON specification emitted.<br /> | ||
| Navigate to <a href="/api/swagger">/api/swagger</a> to see the JSON specification emitted.<br /> | ||
| You may then navigate to each api path to see the JSON documentation for each ApiController individually. |
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,41 +1,60 @@ | ||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Collections.ObjectModel; | ||
| using System.Configuration; | ||
| using System.Dynamic; | ||
| using System.Linq; | ||
| using System.Text; | ||
| using System.Web; | ||
| using System.Web.Http; | ||
| using System.Web.Http.Description; | ||
|
|
||
| namespace Swagger.Net | ||
| { | ||
|
|
||
| public class SwaggerConfiguration | ||
| { | ||
| /// <summary> | ||
| /// Static instance used by default | ||
| /// </summary> | ||
| public static SwaggerConfiguration DefaultConfiguration { get; set; } | ||
|
|
||
| /// <summary> | ||
| /// The API version of your application | ||
| /// </summary> | ||
| public virtual string ApiVersion { get; set; } | ||
| private static SwaggerConfiguration _instance; | ||
|
|
||
| /// <summary> | ||
| /// Swagger-spec version | ||
| /// </summary> | ||
| private HttpContext _context; | ||
| private Collection<ApiDescription> _apiDescriptions; | ||
|
|
||
| public virtual string ApiVersion { get; set; } | ||
| public virtual string SwaggerVersion { get; set; } | ||
| public virtual string DocumentControllerAlias { get; set; } | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is this for?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is to configure a route if you don't want to use default /api/docs |
||
|
|
||
|
|
||
| public virtual System.Web.Http.Description.IApiExplorer ApiExplorer { get; set; } | ||
| public Collection<ApiDescription> ApiDescriptions | ||
| { | ||
| get { return _apiDescriptions ?? GlobalConfiguration.Configuration.Services.GetApiExplorer().ApiDescriptions; } | ||
| set { _apiDescriptions = value; } | ||
| } | ||
|
|
||
| public static SwaggerConfiguration CreateDefaultConfig(HttpConfiguration httpConfig) { | ||
| public HttpContext HttpContext | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think HttpContext makes sense in Configuration. Context is created per-request. Configuration is meant to be initialized once at app start time. Perhaps our use of context this will not make a difference, but it would be easy to make a mistake in the future and end up using the wrong HttpContext object (eg: using the HttpContext.Current.Request object from app_start in a future request would be wrong).
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm... hadn't thought of configuration having implied lifecycle. My intent here was to consolidate nonspecific inputs for use by swagger. |
||
| { | ||
| get { return _context ?? HttpContext.Current; } | ||
| set { _context = value; } | ||
| } | ||
|
|
||
| // Singleton | ||
| public static SwaggerConfiguration Instance | ||
| { | ||
| get { return _instance ?? (_instance = CreateDefaultInstance()); } | ||
| set { _instance = value; } | ||
| } | ||
|
|
||
|
|
||
| public static SwaggerConfiguration CreateDefaultInstance() | ||
| { | ||
| // todo: fetch from web.config | ||
| var alias = ConfigurationManager.AppSettings["document_controller_alias"] ?? "api"; | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My strong personal preference is not to use web.config variables for this. Web.config is good for when the end-user of an application is configuring things, but not a development user who is consuming a library. I'd rather have config via the WebActivator App_Start\SwaggerNet.cs file, and if the developer implementing the library wants to use web.config values, it's up to them to wire that up (and more importantly, make sure that their web application uses it throughout).
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. either way works. I prefer settings in code as well |
||
| return new SwaggerConfiguration() | ||
| { | ||
| SwaggerVersion = "1.1", | ||
| ApiVersion = System.Reflection.Assembly.GetCallingAssembly().GetType().Assembly.GetName().Version.ToString(), | ||
| ApiExplorer = httpConfig.Services.GetApiExplorer() | ||
| ApiVersion = System.Reflection.Assembly.GetCallingAssembly().GetName().Version.ToString(), | ||
| DocumentControllerAlias = alias, | ||
| }; | ||
| } | ||
|
|
||
|
|
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |
| using System.Web.Http; | ||
| using System.Linq; | ||
| using System.Text.RegularExpressions; | ||
| using Swagger.Net.Factories; | ||
|
|
||
| namespace Swagger.Net | ||
| { | ||
|
|
@@ -22,44 +23,35 @@ public class SwaggerController : ApiController | |
| /// <returns>JSON document representing structure of API</returns> | ||
| public HttpResponseMessage GetResourceList() | ||
| { | ||
| var config = SwaggerConfig ?? SwaggerConfiguration.DefaultConfiguration; | ||
| var httpContext = new HttpContextWrapper(HttpContext.Current); // TODO: testable way to pass context | ||
|
|
||
| var resourceListing = new Models.ResourceListing() | ||
| { | ||
| apiVersion = config.ApiVersion, | ||
| basePath = ResolveServerUrl("~", httpContext), | ||
| swaggerVersion = config.SwaggerVersion, | ||
| apis = from d in config.ApiExplorer.ApiDescriptions | ||
| where d.ActionDescriptor.ControllerDescriptor.ControllerType != this.GetType() | ||
| group d by d.ActionDescriptor.ControllerDescriptor.ControllerName into g | ||
| select new Models.ResourceListing.ResourceListingApi() | ||
| { | ||
| path = "/apidocs/" + g.Key | ||
| } | ||
| }; | ||
| // Arrange | ||
| var config = SwaggerConfig ?? SwaggerConfiguration.Instance; | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The reason I used DefaultConfiguration instead of a singleton is because SwaggerConfig can be passed to the controller, and in fact, if you use an IoS container like autofac or unity etc, you can have it automatically inject a SwaggerConfig. Also makes unit tests easier; singletons are hard to test.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thats why I'm not using the singleton unless the this.SwaggerConfig property in null. Controller property can be marked as dependency property for IoC. (as can swaggerConfiguration.Instance) |
||
| var factory = new ResourceListingFactory(); | ||
|
|
||
| // Act | ||
| var resourceListing = factory.Create(config); | ||
|
|
||
| // Answer | ||
| return new HttpResponseMessage() | ||
| { | ||
| Content = new ObjectContent<Models.ResourceListing>(resourceListing, ControllerContext.Configuration.Formatters.JsonFormatter) | ||
| }; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Get the API Declaration for a particular controller | ||
| /// </summary> | ||
| public Models.ApiDeclaration GetApiDeclaration(string controllerName) | ||
| /// Get the API Declaration for a particular controller | ||
| /// </summary> | ||
| public Models.ApiDeclaration GetApiDeclaration(string id) | ||
| { | ||
| var config = SwaggerConfig ?? SwaggerConfiguration.DefaultConfiguration; | ||
| var config = SwaggerConfig ?? SwaggerConfiguration.Instance; | ||
| var httpContext = new HttpContextWrapper(HttpContext.Current); // TODO: testable way to pass context | ||
|
|
||
| return new Models.ApiDeclaration() | ||
| { | ||
| apiVersion = config.ApiVersion, | ||
| swaggerVersion = config.SwaggerVersion, | ||
| basePath = ResolveServerUrl("~", httpContext), | ||
| apis = from d in config.ApiExplorer.ApiDescriptions | ||
| where d.ActionDescriptor.ControllerDescriptor.ControllerName == controllerName | ||
| apis = from d in config.ApiDescriptions | ||
| where d.ActionDescriptor.ControllerDescriptor.ControllerName == id | ||
| group d by d.RelativePath into paths | ||
| select new Models.Api() | ||
| { | ||
|
|
@@ -76,8 +68,8 @@ group d by d.RelativePath into paths | |
| } | ||
|
|
||
| private static string ResolveServerUrl(string relativeUrl, HttpContextWrapper httpContext) | ||
| { | ||
| { | ||
| return httpContext.Request.Url.GetLeftPart(UriPartial.Authority) + VirtualPathUtility.ToAbsolute(relativeUrl); | ||
| } | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think value is lost by doing this only by convention.
Here's my main use case now: My root (/) app is a legacy webforms app. On top of that, I have a single-page javascript application (just html+js) in a sub-directory, and in /restapi I have a MVC (eventually WebAPI) app that is purely my API, called by the single-page javascript app. Using the default forced convention of ~/api means my urls become /restapi/api/ .. which is a bit redundant.
I could live with the default being /api/swagger but it has to be configurable. I think it also slightly confuses things - the developer's actual application API is /api, swagger is a separate concern and therefore it makes sense -- to me, anyway -- that it lives in a different place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see documentControllerAlias for configuration