Consider to add options.Conventions.Controller(Type controllerType) method

[hikalkan]

Consider to add options.Conventions.Controller(Type controllerType) method for ApiVersionConventionBuilder. Thus, we can define conventions for controllers where we can only resolve their type on runtime.

Surely, we can use reflection (I will do it now), but such an option would be better.

[commonsensesoftware]

Reasonable request and something I considered in the original design. I'm willing to discuss how this would work. The biggest challenge I see is defining how to map API version versions to specific actions. This, more than anything, is the reason why just providing type information was not included as an option.

For example, the compile-time typed version looks like:

var controller = options.Conventions.Controller<ValuesController>();
controller.HasApiVersion( 1, 0 )
          .HasApiVersion( 2, 0 )
          .Action( c => c.GetV2() ).MapToApiVersion( 2, 0 );

Using the run-time types, how would this work?

var controller = options.Conventions.Controller( typeof( ValuesController ) );
controller.HasApiVersion( 1, 0 )
          .HasApiVersion( 2, 0 );
     // TODO: how can be define a lambda here since the type is not known at compile-time?
     //  .Action( c => c.GetV2() ).MapToApiVersion( 2, 0 );
     // Maybe this would work?
     //  .Action( "GetV2" ).MapToApiVersion( 2, 0 );

If you can articulate a use case about how you would use this, that might help drive the feature. While limiting, perhaps version interleaving cannot (and should not) be supported by this method of convention.

[hikalkan]

Actions will not be type safe in that case. We can just pass MethodInfo (I suppose it's already possible in the code).

Controller<T> convention currently requires to set versions of controllers one by one. However, if we have a Controller(Type) method we can loop throught an assembly, find controller types via reflection and configure all them by our custom convention. For my case, all controllers in a specific assembly implements same version, while different controllers in different assemblies may implement different versions.
I'm creating a framework, not an application, so I need such flexibilities.

[commonsensesoftware]

Sounds reasonable. I'll re-evaluate the design and see if it's not possible to make this work. The scenario you described is one of the reasons to support conventions in the first place (since you might not be able to apply attributes - say if it were 3rd party). I'll bounce some things off of you once I have something to look at.

[hikalkan]

Thanks a lot for your consideration. For now, I'm using reflection, something like that:

foreach (var controllerType in setting.ControllerTypes)
{
    var controllerBuilder = typeof(ApiVersionConventionBuilder)
        .GetMethod(nameof(ApiVersionConventionBuilder.Controller), BindingFlags.Instance | BindingFlags.Public)
        .MakeGenericMethod(controllerType)
        .Invoke(options.Conventions, null);

    typeof(ControllerApiVersionConventionBuilder<>)
        .MakeGenericType(controllerType)
        .GetMethod("HasApiVersion")
        .Invoke(controllerBuilder, new object[] { setting.ApiVersion });
}

[commonsensesoftware]

@hikalkan have a look at PR #199 whenever you have a minute. This should be what you're looking for.

Per our previous discussion, the expanded support will look like:

var controller = options.Conventions.Controller( typeof( ValuesController ) );
controller.HasApiVersion( 1, 0 )
          .HasApiVersion( 2, 0 );
          .Action( "GetV2" ).MapToApiVersion( 2, 0 );

Action method binding is really the only tricky and loosey-goosey part. The matching rules will be as follows:

• Must be a public method
• Must be a non-static method
• Methods with [NonAction] applied are ignored
• No matches yields MissingMethodException
• Unresolved, duplicate matches yield AmbiguousMatchException
• Method overloads can be resolved by providing the corresponding argument types
  • If there is only one method with the specified name, that's sufficient for matching

As you mentioned, there is already a way to map actions using MethodInfo as that is how the underlying mapping is performed. In the compile-time builder path, this method is intentionally hidden from Intellisense (but callable). In the new run-time builder path, this method will be visible as expected.

One final design note is the builders are not substitutable. This means that you cannot use both Conventions.Controller<ValuesController>() and Conventions.Controller( typeof( ValuesController ) ). Each controller type must be mapped using one style or the other. You can mix and match as long as they are not for the same controller type.

I'll probably commit and potentially publish by day's end tomorrow. If you don't get a chance to review, no big deal. A second pair of eyes is always nice though. If there's any other relevant feedback or suggestions, I'll consider integrating them.

[hikalkan]

Thank you very much @commonsensesoftware for your great work.
It was a huge PR to check with my eyes, but seems fine at a first quick look.
In addition, this library is really good, thanks again.