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
Add Web API support for both WebHost and SelfHost scenarios #57
Conversation
Web API portion has not been tested yet
This contains the logic so it only needs to be in one place
It turns out the System.Web.Http.WebHost package already takes care of converting WebAPI attributes and crap into System.Web.Routing-compatible types. All the work I did was not lost, it will help support a Self hosted WebAPI solution too. Plus, the separation allows more flexibility. Basic test of an API controller shows the correct routes.
This will let people use the default matching Web API provides, {controller}/{id} based on method names.
Also reorganized LogWriter so it used AttributeRouteInfo
Use non-generic IRouteConvention to clean up API for WebAPI conventions
Follow the same convention MS does. Web, Web.Mvc, Http, etc. Moved all publicly consumable constraints, conventions, and attributes to the root project namespaces.
Added in some logic the runtime also uses to determine valid actions. Renamed to "DefaultHttpRouteConvention" so it's more clear what it does.
TController didn't need to actually be on it; that is only used in TranslationBuilder. Added localized API controller.
Forgot to save sln
This simplifies a lot of code, removing need for derived fluent translation providers. Slowly but surely.
This makes keeping version information consistent for Nuget and to help with maintenance.
AttributeRouting.Http -> AttributeRouting.Web.Http to match convention used by ASP.NET Web Stack
Also fixed an issue where the MVC-based config was accepting promoted controllers of the Api controller types
Talk about taking ownership! Thanks for the long description of the changes. This will definitely be v2. I'll take a look soon, but might have to wait until next week (what with Easter holiday and traveling). Again, this is awesome -- radical rethinking, and the impl to back it up. You rock. t On Apr 7, 2012, at 12:30 AM, Kamran Ayub wrote:
|
You're hired! I'll look it over tomorrow or Tues and merge. I'm starting to build an app with MVC 4 and having Web API support will be nice! Seriously much appreciated contribution as I've been slammed with other things recently. Also realized I was on your blog earlier this week checking out some of your writeup on cassette :). t On Apr 8, 2012, at 9:05 PM, Kamran Ayub wrote:
|
👏 |
Was just looking things over. Ran build and got an error "'nuget' is not recognized as an internal or external command, operable program or batch file." So would be good to fix that. Otherwise, I'm ready to merge and push out v2. Just want to make sure that you can update the wiki where appropriate? |
Whoops, I have nuget installed in my PATH but not everyone does. I will have it point to the exe included in And yes, I'll update the wiki. |
Kamran -- I just merged a bug fix that you'll want to merge into your feature branch. |
MVC accepts HttpVerbs overload and Web API accepts HttpMethod overload
Alright, should be good to go! |
Hey - so by good to go, you mean you're ready to have this merged in to master? I know you had said you wanted a couple more days mid-week to tidy some things. |
Yup! On Sun, Apr 15, 2012 at 11:52 AM, Tim McCall <
Kamran Ayub |
Just looking stuff over before merging. Noticed that the tests are failing. Also wondering why you have both interfaces and abstract base classes in AttributeRouting? For example: RouteConstraintAttributeBase, and IAttributeRouteConstraint. I would prefer fewer files if the Interface does not have more than one direct implementation. I'm going through stuff getting the tests running again and being a code and formatting Nazi. Might have to ping ya with a few more questions, which I'll add to this comment as they arise. Cheers! |
…(had single impl in abstract base classes - those are good enough for me). fixed broken tests.
I merged your branch, but into a new branch: 57-web-api. I'm still poking around. You should base any futher work off of this branch (until it goes into master, of course). |
…ull plug-and-chug (ie: no missing imports, etc).
IT'S ALIVE! v2 is live on nuget. Thanks for your hard work. Thanks for the new build tool, too. That was a backburner todo for a long time. |
Are the tests still failing? I will take a look if so. I'll get back to you on the interfaces; most of them are for generating framework-agnostic types... for example, constraints in MVC use |
I fixed the tests, massaged some inconsistent formatting, and released On Apr 16, 2012, at 6:59 AM, Kamran Ayub
|
Sweeeet, thanks! There is a minor issue with the default http convention, I've been using my v2 fork for the past week or so and it's been awesome, On Mon, Apr 16, 2012 at 9:20 AM, Tim McCall <
Kamran Ayub |
Hey Kamran -- jst getting word that v2 introduced a big (at least so I think) bug. The bottom line is routes on two actions with the same name cannot be dismbiguated since the route attributes do not derive from AttributeMethodSelectorAttribute anymore (which is in System.Web.Mvc, and not in System.Web.Http). I've spent a bit of time trying to find a way around this, and long story short -- it doesn't look good. Disambiguation in System.Web.Http is controlled by IActionMethodSelector and this is an internal interface, and the Http{verb} attributes in System.Web.Http are all sealed, so we cannot simply derive from those. I need to step away from the problem for a bit, but just wanted to let you know I have pulled all v2 stuff from nuget, leaving the v1.7 of AR listed instead. I put a note in issue #63 with a workaround, but I am not happy with the workaround as an actual fix. Frankly I'm not sure that we can support Web API stuff without relying on multiple attributes, which makes using AR on Web API routes kind of kludgy. I would love to find a solution, but ultimately, I think Web API stuff might need to stay outside the scope of AR. Please comment on this issue or email me and we can discuss in painful detail offline. Cheers! |
Fixed it. Was idiotic. But there's a breaking change alert: #63. |
This is a massive change, or at least it is in terms of organization. Core code really hasn't changed. There are specs for Web API with the feature files finished, but tests are missing for all the other standard unit tests (Translations, notably).
You don't need to pull this in right now, rather I want to keep this open to track changes and discuss them. I've tried to keep the structure mostly the same as it was before, and hopefully most of the changes make sense.
Major Changes
Namespacing
It looks like there's a ton of projects, and there are, but it's mostly to keep with the ASP.NET web stack convention and to implement derived classes that simply implement the proper framework interfaces.
Projects
Quick rundown, but each should be self-explanatory:
The main reason I decoupled everything was for ease of downstream consumption. If I'm doing a MVC project, I don't want Web API DLLs in my project. If I'm doing only ASP.NET Web API, I don't want MVC stuff in my project. If I'm writing a Web API console app, I don't need web stuff. This just keeps everything nice and separated, no un-needed dependencies.
Http Convention
For most of the Web API-specific stuff, I prefix everything with
Http
to keep in line with what MS is doing. So it isHttpAttributeRoutingConfiguration
andMapHttpAttributeRoutes
. This makes it easier to use MVC and Web API at the same time, which will probably be the most common.Minor Changes
AttributeRouting.Framework.Factories
and have implementations in the 3 projects.system.webServer
) to handle routes.axd