Skip to content

AddVersionedApiExplorer duplicates APIs when inheriting from base controller #229

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
sabmah opened this issue Dec 28, 2017 · 6 comments
Closed

Comments

@sabmah
Copy link

sabmah commented Dec 28, 2017

I have a following code

public class BaseController : ApiController{}

 [ApiVersion("1")]
 [RoutePrefix("api/v{version:apiversion}/myresource")]
 public class MyController: BaseController{

    [Route("")]
    public async Task<IHttpActionResult> GetUsers()
    {
        var users = await _userRepository.GetAllUsersAsync();
        return Ok(users);
    }
}

In WebConfig.cs , I have

      config.AddApiVersioning(opt =>
            {
                opt.ReportApiVersions = true;
                opt.AssumeDefaultVersionWhenUnspecified = true;
                opt.ApiVersionSelector = new CurrentImplementationApiVersionSelector(opt);
            }
        );

config.AddVersionedApiExplorer()

When I go to my Help Page, I see duplicates:
GET api/v{version:apiversion}/myresource
GET api/v{version:apiversion}/myresource

But when I remove the Base Controller and just inherit directly ApiController to "MyController", I don't see the duplicates.

Is there a problem with inheriting ?

@commonsensesoftware
Copy link
Collaborator

Honestly, it's been a very long time since I've used the Help Pages since most have switched to Swagger. The API Explorer for API versioning uses the same discovery process as routing. If things route the way you expect, then there is likely not an issue with inheritance.

How are you building your help page? While it is possible, it's much harder to use the IApiExplorer interface directly because its design has no support for API versioning. The VersionedApiExplorer does implement this interface, but the ApiDescription items are reported in a flat list, which is all that IApiExplorer supports. I recommend using the VersionedApiExplorer directly instead. You can achieve this by capturing the reference when it's added or cast it from the registered instance.

// capture during registration
var apiExplorer = configuration.AddVersionedApiExplorer();

// cast from registered instance
apiExplorer = (VersionedApiExplorer) configuration.Services.GetApiExplorer();

Once you have an reference to the strongly-typed VersionedApiExplorer instance, the ApiDescriptions property will be grouped by API version for you. Each item in the collection is a group of groups, which should come out something like this when enumerated:

  • 1.0
    • GET api/v{version:apiversion}/myresource
  • 2.0
    • GET api/v{version:apiversion}/myresource

When you use IApiExplorer directly, the list is flat, which has the appearance of duplicates. The entries are not actually duplicates, but are identical route templates for different API versions. You should be able to confirm that through the debugger display. It's possible to cast the individual ApiDescription entries to their VersionedApiDescription equivalent, which will contain the API version and group information as well.

One final note. Remember that when you version by URL segment the API version is a route parameter value with a constraint. The API Explorer is not going to fill in or translate the URL for you. It will, however, include a parameter that defaults to the expected value. If you want the URLs to be pre-generated, you'll have to add your own code to perform the replacement. The behavior is no different than if the route was api/v1/myresource/{id}. You wouldn't expect the API Explorer to fill in the value of {id}. The same is true for the {version} parameter.

I hope that helps. Let me know if you have more questions.

@sabmah
Copy link
Author

sabmah commented Dec 28, 2017

It makes sense if I have multiple versions like you said (1.0 and 2.0). I only have 1 version.

@commonsensesoftware
Copy link
Collaborator

  1. What is the API version reported for the two routes?
    a. Are they the same?
    b. How many API versions are explored?
    c. Which APIs are explored per API version?
  2. What does your routing configuration look like? Are you also defining convention-based routes?

API descriptions are de-duplicated by ID and API version.

@commonsensesoftware
Copy link
Collaborator

Hmmm.... seems like there might be a problem. I was able to repro your scenario in the Web API Swagger sample. The issue seems to be evaluation of the API version itself.

I think there may be a bug here. I need to investigate further. To unblock you though, try changing your attribute from [ApiVersion("1")] to [ApiVersion("1.0")]. Things worked for me in Swagger when I made this change. This definitely doesn't matter during route evaluation and shouldn't matter here, but somehow it is.

Try that and hopefully you'll be unblocked. I'll continue investigating. Thanks.

@sabmah
Copy link
Author

sabmah commented Dec 28, 2017

Thanks. We removed the inheritance for now but your solution is an alternative that we can use too. Thanks for looking into it

@commonsensesoftware
Copy link
Collaborator

Uggg ... it's a bug. GetHashCode is implemented poorly in the ApiVersion (bad developer; no cookie). The current implementation relies on the hash code of ToString. Early on, the minor version component wasn't optional. It's now optional, but is treated as zero when unspecified. However, ToString intentionally does not modify the source input (e.g. the input to Parse should match ToString). As a result, this ends up causing 1 and 1.0 to be evaluated differently and, thus, not de-duplicated. In your scenario, this probably because ApiVersioningOptions.DefaultApiVersion equals new ApiVersion(1,0) and [ApiVersion("1")] parses to new ApiVersion(1).

The fix is pretty simple, but I'm amazed no one else has neither hit this error or reported it. Nevertheless, the behavior is wrong. I have several fixes I'm working on. I will roll this into it and push it out as soon as possible. I'm advertising the end of next week, but there is a good chance it will be much sooner than that.

I tried the inheritance combination and that worked just fine. If BaseController doesn't provide a public API surface, consider making it abstract. That should guarantee it's excluded from the results. As a workaround, you can use [ApiVersion("1.0")]. Both cases will support either api/v1/myresource or api/v1.0/myresource. You can use the for format codes to have your Help Pages show the version without the .0. Hopefully that gets you back on your way.

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