-
Notifications
You must be signed in to change notification settings - Fork 709
IControllerConfiguration: An item with the same key has already been added #313
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
Comments
Looks like this might be an issue with your routing configuration. I don't have the complete picture of your setup, but the exception is occurring where attribute-based routes (formally known as direct routes) are being added to the pipeline. The RoutePrefixAttribute and RouteAttribute are typically not honored in inheritance scenarios. That could explain why you see it for some scenarios. If you can share more of your setup, that will help. Better still, if you have complete, albeit stripped down, repro, that will go a long way at trying to troubleshoot the issue. |
Thanks for the quick reply. When you say "an issue with your routing configuration" do you mean specifically related to the VersionedApiExplorer or in general? Before integrating the VersionedApiExplorer I had no issues getting Swagger to document with my current setup. None of my BaseControllers are decorated with the RoutePrefixAttribute or the RouteAttribute and neither of them contain any actions. Those attributes are only present in the sub classes. The entire solution only uses attribute based routing (i.e there is no route table setup in the WebApiConfig). The versioning setup is as follows:
While trying to pinpoint the issue this morning I was able to find another piece to the puzzle. In one of my controllers that inherits from BaseController i had the following action:
When I removed the Name = "ConfirmEmailRoute", everything started working correctly. I can't seem to find any mention of "ConfirmEmailRoute" so maybe this is legacy code but that route name was not duplicated anywhere, nor the controller prefix so why would it have already been added to the route table? It's odd that it worked when removing the IControllerConfiguration and now works correctly after removing the Route's Name. Is there something in the VersionedApiExplorer that would cause it to be added twice because of that specific setup? |
This is exactly the type of information I was hoping to see. The issue is not with the VersionedApiExplorer, that just happens to be where the issue is ultimately manifesting. The issue you're facing appears to be related to the presence of the route name a la Unfortunately, route names are not API version-aware. It's novel ideal to support in some future release, but I'm not even sure if it's technically possible. The guidance and only feasible solution I know of is to either ditch using names or append a version to the name (ex: A lot of people don't like to hear it, but honestly feel that inheritance is the wrong way to think about the problem. I'm all for reducing duplication; however, most don't understand all the other complexities that come with inheritance:
If you're going to use inheritance, I suggest having the base class only include protected methods that are generally reusable by subclass. If you're really careful, it's possible to make public virtual methods without route information work, but it can get tricky. Most of the logic should be in the proverbial business layer or in reusable decorators (i.e. IHttpActionFilter) for things like validation. I've found this to reduce action code to 5-10 lines max. Duplicating that is a non-issue since it's literally a copy and paste. If there is a change between implementations, I know I didn't break the other. Having a defined versioning policy like N-2 versions also make things easier to reason about.
Hopefully that unblocks you and gets you back on your way. |
OK you lost me a little bit there. In my specific scenario the base controllers are abstract and don't have any Route specific attributes and contain no actions. They do contain the attributes I've shown above (i.e. AllowAnon, IControllerConfiguration, etc.) but nowhere do they define any routes configuration. Are you saying having any attributes (like what i have) is still the root cause of the issue? In short, I need to move all of those to the specific controllers to resolve the issue permanently if I wanted to use the "Name" and avoid future problems? |
Ah … sorry … misread. In short, if two routes defined by routes anywhere have the same name (e.g. key), you'll face this problem. It doesn't matter if you're using inheritance or not. The names used in the route table are unioned. |
Right but that is not the case. That route name was never duplicated... so that problem should not have manifested, correct? |
You stated:
This should mean the route was either duplicated somewhere in code or was duplicated via inheritance when the route is registered 2+ times. From the API Explorer's perspective, it doesn't care about such things. It's simply asking Web API for the all the routes and then explores them. This appears to happen when the routes are explored. Does the application even run? I would expect the same problem to arise when the route table is built in all cases, not just for the API Explorer. |
That's the thing -- it's been running without issue. It's in production, so the only net new addition to the code was the versioning support. There has never been an issue with the route table before. This problem has only arisen after using the VersionedApiExplorer. At the end of the day it's working without the Name="ConfirmEmailRoute" but that Name was never duplicated (or used) throughout the solution. Again, at first it seemed like an issue with the IControllerConfiguration so maybe when the VersionedApiExplorer is "FlattenVersionApi" it's doing something that is causing it to be registered twice. Nothing in my code is registering twice... |
Congratulations! You are the proud winner of a shiny, new bug! =P So the issue is actually in the VersionedApiExplorer, but it's so weird. The API Explorer creates a dummy HttpControllerDescriptor to collect the API version information because that's what all the extension methods as such are already based on. Apparently the constructor that I chose to use has side effects that I didn't know about. Specifically, if the associated controller type implements IControllerConfiguration, it tries to initialize it. This results in a recursive call that produces this issue because the configuration and route table has already been initialized. This is a very simple fix. I should have something out shortly. |
OK great. No rush on my side since I removed Name anyway. |
This also surfaced a related, but previously undetected issue. The formatters were always coming from the original configuration, but a IControllerConfiguration can modify the formatters, which yields an entirely new configuration. It's not particular common and know one has ever reported it before, but it could happen. Interestingly, my OData version does the right thing. I suspect this was an issue in the original API Explorer code I forked from Web API. Both issues will be fixed and published soon. |
The fix has been published as https://www.nuget.org/packages/Microsoft.AspNet.WebApi.Versioning.ApiExplorer/1.2.1 Thanks |
I'm getting an error about the same key being added when trying to loop through the ApiDescriptions. My setup is pretty simple in that i have:
I have two controllers that inherit from these base classes that have no duplicate names or actions.
Observations:
MyCustomIControllerConfiguration is as follows:
Removing the attribute isn't an option. It's odd that (1) is true with the attribute considering the error. Has anyone seen this type of error before?
Stack Trace:
[ArgumentException: An item with the same key has already been added.]
System.ThrowHelper.ThrowArgumentException(ExceptionResource resource) +56
System.Collections.Generic.Dictionary
2.Insert(TKey key, TValue value, Boolean add) +14763607 System.Web.Http.HttpRouteCollection.Add(String name, IHttpRoute route) +44 System.Web.Http.Routing.AttributeRoutingMapper.AddGenerationHooksForSubRoutes(HttpRouteCollection routeTable, IEnumerable
1 entries) +147System.Web.Http.Routing.<>c__DisplayClass2.b__0(HttpConfiguration config) +149
System.Web.Http.HttpConfiguration.ApplyControllerSettings(HttpControllerSettings settings, HttpConfiguration configuration) +94
System.Web.Http.Controllers.HttpControllerDescriptor.InvokeAttributesOnControllerType(HttpControllerDescriptor controllerDescriptor, Type type) +200
System.Web.Http.Controllers.HttpControllerDescriptor.InvokeAttributesOnControllerType(HttpControllerDescriptor controllerDescriptor, Type type) +63
System.Web.Http.Controllers.HttpControllerDescriptor..ctor(HttpConfiguration configuration, String controllerName, Type controllerType) +131
Microsoft.Web.Http.Description.VersionedApiExplorer.FlattenApiVersions() +474
Microsoft.Web.Http.Description.VersionedApiExplorer.InitializeApiDescriptions() +264
System.Lazy
1.CreateValue() +236 System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw() +31 System.Lazy
1.get_Value() +14620009[TargetInvocationException: Exception has been thrown by the target of an invocation.]
System.RuntimeMethodHandle.InvokeMethod(Object target, Object[] arguments, Signature sig, Boolean constructor) +0
System.Reflection.RuntimeMethodInfo.UnsafeInvokeInternal(Object obj, Object[] parameters, Object[] arguments) +128
System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture) +142
Owin.Loader.<>c__DisplayClass12.b__b(IAppBuilder builder) +93
Owin.Loader.<>c__DisplayClass1.b__0(IAppBuilder builder) +212
Microsoft.Owin.Host.SystemWeb.OwinAppContext.Initialize(Action
1 startup) +873 Microsoft.Owin.Host.SystemWeb.OwinBuilder.Build(Action
1 startup) +51Microsoft.Owin.Host.SystemWeb.OwinHttpModule.InitializeBlueprint() +101
System.Threading.LazyInitializer.EnsureInitializedCore(T& target, Boolean& initialized, Object& syncLock, Func`1 valueFactory) +135
Microsoft.Owin.Host.SystemWeb.OwinHttpModule.Init(HttpApplication context) +160
System.Web.HttpApplication.RegisterEventSubscriptionsWithIIS(IntPtr appContext, HttpContext context, MethodInfo[] handlers) +581
System.Web.HttpApplication.InitSpecial(HttpApplicationState state, MethodInfo[] handlers, IntPtr appContext, HttpContext context) +168
System.Web.HttpApplicationFactory.GetSpecialApplicationInstance(IntPtr appContext, HttpContext context) +414
System.Web.Hosting.PipelineRuntime.InitializeApplication(IntPtr appContext) +369
[HttpException (0x80004005): Exception has been thrown by the target of an invocation.]
System.Web.HttpRuntime.FirstRequestInit(HttpContext context) +532
System.Web.HttpRuntime.EnsureFirstRequestInit(HttpContext context) +111
System.Web.HttpRuntime.ProcessRequestNotificationPrivate(IIS7WorkerRequest wr, HttpContext context) +714
The text was updated successfully, but these errors were encountered: