Skip to content

VersionByNamespaceConvention for major version with 2 or more digits #805

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
Doomic opened this issue Mar 3, 2022 · 2 comments
Closed
Assignees
Labels

Comments

@Doomic
Copy link

Doomic commented Mar 3, 2022

We want to make a V10.0 API version and we are using VersionByNamespaceConvention configuration. But it looks like that this is not supported (if you skip the date part).
I checked the Pattern used in the VersionByNamespaceConvention and it looks like "10" is matching the second group (month). And therefore it is not seen..
It will work if we use "V___10_0" as namespace; but that will be ugly.
I would prefer an option to configure that you only use major/minor convention..
or maybe a more easy way of fixing would be; first check if it has at least 8digits before date matching will be applied.

@commonsensesoftware
Copy link
Collaborator

Thanks for reporting this. You are correct. The issue is less about the pattern and more about how the results are used. Regardless, this is not the expected behavior and is a bug. The next major release has this entire implementation rewritten to not use regular expressions. It's not only faster, but avoids issues like this. I've added this specific test case (and it passes 😉)

I'll add this to the list to put in the next patch version. In the meantime, my recommendation would be to use your own custom convention to workaround the issue. That will prevent you from having to resort to ugly namespace components. Since the implementation is localized to your application, you don't have to add generic validation.

It would look something like:

public class CustomNamespaceConvention : IControllerConvention
{
    public bool Apply( IControllerConventionBuilder builder, ControllerModel controller )
    {
    	var name = controller.ControllerType.Namespace; // extract from here
    	var version = ApiVersion.Parse( name );    
        var deprecated = controller.Attributes.OfType<ObsoleteAttribute>().Any();

        if ( deprecated )
        {
            builder.HasDeprecatedApiVersion( version );
        }
        else
        {
            builder.HasApiVersion( version );
        }

        return true;
    }
}

Honestly, you don't even have to parse it. You could have a dictionary that maps ApiVersion to namespace string. The choice is yours, but that should suffice as a bridge until the issue is fixed.

@Doomic
Copy link
Author

Doomic commented Mar 4, 2022

Thank you for the fast response and for the suggestion to temporary fix this. I will close this issue, because the fix is coming.

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

No branches or pull requests

2 participants