Skip to content

Why we cannot we assign a particular action method to a specific version of version neutral controller? #323

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

Closed
sajibcefalo opened this issue Jul 27, 2018 · 7 comments
Assignees
Milestone

Comments

@sajibcefalo
Copy link

sajibcefalo commented Jul 27, 2018

Hello,

I have the following requirement:

  • A controller has several action methods

  • Except for one particular action method, all the action methods should be version neutral.

I mark the controller version neutral and also that particular action method particular version. Now, it's not that obvious all the action methods should be work as version neutral and the particular action method should work for particular version as version level action method get preference should get preference than version level at the controller level.

@commonsensesoftware
Copy link
Collaborator

Interesting, so you want to be version neutral, but have specific mappings for some actions. Something like this:

[ApiVersionNeutral]
[Route("api/[controller]")]
public class MyContoller : ControllerBase
{
    [MapToApiVersion("1.0")]
    [HttpGet]
    public IActionResult Get() => Ok("Hello v1");

    [MapToApiVersion("2.0")]
    [HttpGet]
    public IActionResult GetV2() => Ok("Hello v2");

    [MapToApiVersion("3.0")]
    [HttpGet]
    public IActionResult GetV3() => Ok("Hello v3");

   public IActionResult GetAnythingElse(ApiVersion version) => Ok("Hello v" + version.ToString());

    [HttpDelete("{id}")]
    public IActionResult Delete(int id) => NoContent();
}

I've never tried this before. It's reasonable for this to work as long as there is at least one catch all action that can satisfy the request. By claiming to be API version-neutral, you have signed up to accept any and all API versions, including none at all. Off the top of my head, I don't see why this can't work as long as these rules are followed. It would be very weird to a client to get an error message about an unsupported API version from a version-neutral service.

An alternate way that I know will work is to do all the branching logic yourself. The ApiVersion class is IEquatable and IComparable, which makes writing the logic pretty straight foward. For example, instead of relying on MapToApiVersion to do the mapping, you can do this:

[ApiVersionNeutral]
[Route("api/[controller]")]
public class MyContoller : ControllerBase
{
    readonly static ApiVersion v1 = new ApiVersion(1, 0);
    readonly static ApiVersion v2 = new ApiVersion(1, 0);
    readonly static ApiVersion v3 = new ApiVersion(1, 0);

    [HttpGet]
    public IActionResult Get(ApiVersion version)
    {
         // NOTE: this is the forthcoming v3 way, but you can also use
         // var version = HttpContext.GetRequestedApiVersion();

         if ( version < V1 )
         {
             return Ok($"Hello beta (v{version})");
         }
         else if ( version < V2 )
         {
             return Ok("Hello v1");
         }
         else if ( version < V3 )
         {
             return Ok("Hello v2");
         }

          return Ok("Hello v" + version.ToString());
    }

    [HttpDelete("{id}")]
    public IActionResult Delete(int id) => NoContent();
}

Version neutrality was always meant to be just that. With the exception of just knowing what the client asked for, it was never really the intent to be able to have per-version branching logic in a version-neutral controller, even though it's possible.

I'll investigate and re-evaluate whether this should be a supported scenario as you've described. My concern is purely on how this appears to a client. In terms of the mapping on the server side, that's an implementation detail and at your discretion.

@sajibcefalo
Copy link
Author

sajibcefalo commented Jul 29, 2018

Yes, I tried the first approach. It's not possible to have per-version branching logic in a version-neutral controller. The second approach is not viable because we don't want to write a lot of api versioning code at controller level instead we do versioning using the convention outside controller.

You can think about the following scenario. If a controller has ten action methods and a certain change in one action method forces you to go for versioning. Instead of including every action method in versioning, is not it reasonable to exclude them from versioning and mark version only the action method needed to version?

@sajibcefalo
Copy link
Author

@commonsensesoftware Can you please tell me the latest update of the investigation?

@commonsensesoftware
Copy link
Collaborator

The investigation here is evaluating the feasibility of your ask. Your request should theoretically be possible, but I don't believe it's currently supported. The primary reason is that version-neutral means that you can support any and all versions. If your controller doesn't support a version because it's not mapped, that is an odd behavior. That would have to yield a 500 or something as a developer mistake; otherwise, I don't really see anything else immediately preventing this from being possible. I'll likely queue this up as a new feature for v3.0.

An API cannot be both versioned and version-neutral simultaneously. Mixing and matching that way is simply not supported. The current interleaving support allows roll forward behavior for newly declared API versions so that only the deltas need to be touched. If you introduce complex interleaving within a single controller, then you will naturally end up with a bunch of complex mapping as well. I recommend that interleaving be limited to small deltas.

Interleaving across types/classes is interesting and arguably useful. It sounds like this is what you are asking for. This is queued up for #230. This feature is much more complicated to implement than it is to consume. The best use case for this scenario is a DELETE action which should be effectively version-neutral despite any other action. The expected implementation will behave similar to the roll forward behavior.

Keep in mind that version-neutrality does not perform any validation on the requested API version beyond ensuring that it's well-formed. If any API is truly version-neutral, it should behave accordingly. It would be bizarre to perform DELETE on resource using an arbitrary, but well-formed API version. The desire to desire to be API version-neutral in this case is a developer convenience and not a reflection of the API itself. The enhancement is, therefore, more about simplifying the implementation details for a service author as opposed to defining an API version-neutral service or action, which is entirely different.

@sajibcefalo
Copy link
Author

Thanks for your response.

@commonsensesoftware
Copy link
Collaborator

As expected, there was a lot of refactoring and work to get toward supporting this scenario. This scenario will be a subset of #230.

I'm still testing out all variations, but I have a baseline refactoring that works for all the existing functionality (e.g. I didn't break anything). Now, I can move on to verifying the new scenarios. The type of setup you are asking for will look something like:

[Route("api/[controller]")]
public class MyContoller : ControllerBase
{
    [ApiVersion("1.0")]
    [HttpGet]
    public IActionResult Get() => Ok("Hello v1");

    [ApiVersion("2.0")]
    [HttpGet]
    public IActionResult GetV2() => Ok("Hello v2");

    [ApiVersion("3.0")]
    [HttpGet]
    public IActionResult GetV3() => Ok("Hello v3");

    [ApiVersionNeutral]
    [HttpDelete("{id}")]
    public IActionResult Delete(int id) => NoContent();
}

In short, you'll now be able to declare API versions at the action level. This behavior is different from mapping an API version, which explicitly declares which API version an action maps to that is implicitly inferred from its declaring controller. Mixing and matching these concepts can get confusing; however, API versions declared (a la ApiVersionAttribute or ApiVersionNeutralAttribute) will always take prescedence over implicitly infer API versions declared at the controller level. This means there are new conventions to express this as well.

I should also point out that matching can never be implicit or a fall-through. The closest concept is version-neutral, but this means any and all versions, including none at all. I think using MapToApiVersionAttribute as in the original example will work because mapping an API version is not a declaration of a version. From a mapping perspective, an action can explicitly or implicitly indicate it matches an API version declared by it's defining controller. In the case of version-neutral, this would be all API versions. I'm honestly not sure how much sense that actually makes though because that means a client can send you any well-formed API version and it will be accepted. I've seen that setup in practice and it turned out to be nightmare for the service team. In fact, the team never even realized clients were sending bogus, undefined API versions until they started enforcing strong API version semantics.

I expect 3.0 Beta 2 to contain this functionality. As nearly all the issue from Beta 1 have been addressed, I anticipate that release coming in the next week or so.

@commonsensesoftware commonsensesoftware added this to the 3.0 milestone Nov 6, 2018
@sajibcefalo
Copy link
Author

@commonsensesoftware I understand your point and it's reasonable. But, there are some cases when you compelled to introduce versioning for only specific one action method and you don't want to introduce versioning for that for the rest of the action methods. Also, there is the controller which will never be upgraded and hence does not need backward compatibility support.

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

No branches or pull requests

2 participants