Skip to content
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

VersionedApiDescriptionProvider does not set the correct SunsetPolicy to ApiDescription instances #1064

Closed
1 task done
reinaldoarrosi opened this issue Jan 10, 2024 · 1 comment · Fixed by #1065
Closed
1 task done
Assignees
Labels

Comments

@reinaldoarrosi
Copy link

reinaldoarrosi commented Jan 10, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

The wiki for versioning policies explicitly states that

Sunset policies do not have to have a date. The following scenarios are supported:
-Define a sunset policy by API name and version
-Define a sunset policy by API name for any version
-Define a sunset policy by API version for any API

Based on the last item in that list, I assumed that I should be able to create a sunset policy for a specific API version, regardless of the API name, so this is what I did:

services.AddApiVersioning(options =>
{
    options.ReportApiVersions = true;

    options.Policies.Sunset(new ApiVersion(3)).Effective(2025, 1, 1);
    options.Policies.Sunset(new ApiVersion(4)).Effective(2025, 12, 1);
})

I can confirm that the above works well because responses for API v3 now include Sunset Mon, 01 Jan 2025 00:00:00 GMT HTTP header.

However, when trying to access the sunset policy from the ApiExplorer APIs (i.e. bestMatch.Properties.TryGetValue(typeof(SunsetPolicy))) it returns null

Digging into the code for VersionedApiDescriptionProvider, I noticed that it uses ISunsetPolicyManagerExtensions.TryResolvePolicy to try to obtain the SunsetPolicy based on the API name and version and that method is returning null in my scenario. The reason it returns null is because it tries to find the policy using name AND version and, as shown above, my sunset policies do not have a name associated with them - they should apply to any API name as described on the wiki.

Notice that this behavior is different than what DefaultApiVersionReporter does. That class, who is responsible for writing the Sunset HTTP response header, uses a different strategy to obtain the SunsetPolicy for a given API name and version: first it tries to get a policy based on the name AND version provided, if it can't find one then it tries to find a policy based on the name only, if it still can't find a policy it then tries to find a policy based on the version only.

My assumption is that VersionedApiDescriptionProvider should use the same strategy as DefaultApiVersionReporter to find a suitable SunsetPolicy.

Expected Behavior

No response

Steps To Reproduce

No response

Exceptions (if any)

No response

.NET Version

No response

Anything else?

No response

@commonsensesoftware
Copy link
Collaborator

Strange... I guess I added the new function, but didn't refactor it or somehow forgot. It seems TryResolvePolicy still has the same logic, but after a closer inspection, I repro'd the issue. The cause appears to be here:

else if ( apiVersion != null && policyManager.TryGetPolicy( apiVersion, out sunsetPolicy ) )

It should be a new if statement rather than else if. This causes it to not fallback when it should. I can also see this wasn't caught because the test case is setup incorrectly:

This is a bug. I'll work on a fix as well as unify the usage so they are consistent. Thanks for reporting it.

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

Successfully merging a pull request may close this issue.

2 participants