Fix Media Type Version Collation

src/AspNet/WebApi/src/Asp.Versioning.WebApi/Asp.Versioning.WebApi.csproj

@@ -1,7 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
  <PropertyGroup>
-  <VersionPrefix>10.2.0</VersionPrefix>
+  <VersionPrefix>10.2.1</VersionPrefix>
   <AssemblyVersion>10.2.0.0</AssemblyVersion>
   <TargetFrameworks>net45;net472</TargetFrameworks>
   <AssemblyTitle>ASP.NET Web API Versioning</AssemblyTitle>

src/AspNet/WebApi/src/Asp.Versioning.WebApi/MediaTypeApiVersionReader.cs

@@ -16,20 +16,19 @@ public virtual IReadOnlyList<string> Read( HttpRequestMessage request )
         var version = contentType is null ? default : ReadContentTypeHeader( contentType );
         var accept = request.Headers.Accept;
 
-        if ( accept is null || ReadAcceptHeader( accept ) is not string otherVersion )
+        if ( accept is null || accept.Count == 0 )
         {
             return version is null ? [] : [version];
         }
 
-        var comparer = StringComparer.OrdinalIgnoreCase;
-
-        if ( version is null || comparer.Equals( version, otherVersion ) )
+        // TODO: the ranked implementation is the correct way, but ReadAcceptHeader requires a breaking change that
+        // cannot ship until the next major version. internally do the right thing, but if ReadAcceptHeader is
+        // overridden, then make sure we honor the implementation. the onus is on the implementer.
+        if ( acceptHeaderOverridden )
         {
-            return [otherVersion];
+            return Collate( version, ReadAcceptHeader( accept ) );
         }
 
-        return comparer.Compare( version, otherVersion ) <= 0
-               ? [version, otherVersion]
-               : [otherVersion, version];
+        return Collate( version, ReadRankedAcceptHeader( version is null ? accept : MediaTypeQuality.MaxRanked( accept ) ) );
     }
 }

src/AspNet/WebApi/src/Asp.Versioning.WebApi/ReleaseNotes.txt

@@ -1 +1 @@
-
+Fixed media type quality (q) ranking and collation [Issue #1221](https://github.com/dotnet/aspnet-api-versioning/issues/1221)

src/AspNet/WebApi/test/Asp.Versioning.WebApi.Tests/MediaTypeApiVersionReaderBuilderTest.cs

@@ -74,6 +74,8 @@ public void read_should_retrieve_version_from_accept()
     [InlineData( new[] { "application/xml", "application/json;q=0.2;v=1.0" }, "1.0" )]
     [InlineData( new[] { "application/json", "application/xml" }, null )]
     [InlineData( new[] { "application/xml", "application/xml+atom;q=0.8;api.ver=2.5", "application/json;q=0.2;v=1.0" }, "2.5" )]
+    [InlineData( new[] { "application/xml;q=0;v=2.0" }, null )]
+    [InlineData( new[] { "application/json;q=0;v=1.0", "application/xml;q=0.2;v=2.0" }, "2.0" )]
     public void read_should_retrieve_version_from_accept_with_quality( string[] mediaTypes, string expected )
     {
         // arrange
@@ -97,7 +99,37 @@ public void read_should_retrieve_version_from_accept_with_quality( string[] medi
     }
 
     [Fact]
-    public void read_should_retrieve_version_from_content_type_and_accept()
+    public void read_should_collate_incongruent_versions_from_content_type_and_accept()
+    {
+        // arrange
+        var reader = new MediaTypeApiVersionReaderBuilder().Parameter( "v" ).Build();
+
+        // the Accept media type has no quality parameter, so it is ranked equally with
+        // the Content-Type media type and both are collated, which is ambiguous
+        var request = new HttpRequestMessage( Post, "http://tempuri.org" )
+        {
+            Headers =
+            {
+                Accept = { Parse( "application/json;v=2.0" ) },
+            },
+            Content = new StringContent( "{\"message\":\"test\"}", UTF8 )
+            {
+                Headers =
+                {
+                    ContentType = Parse( "application/json;v=1.0" ),
+                },
+            },
+        };
+
+        // act
+        var versions = reader.Read( request );
+
+        // assert
+        versions.Should().BeEquivalentTo( ["1.0", "2.0"] );
+    }
+
+    [Fact]
+    public void read_should_prefer_version_from_content_type_over_accept()
     {
         // arrange
         var reader = new MediaTypeApiVersionReaderBuilder().Parameter( "v" ).Build();
@@ -125,7 +157,7 @@ public void read_should_retrieve_version_from_content_type_and_accept()
         var versions = reader.Read( request );
 
         // assert
-        versions.Should().BeEquivalentTo( ["1.5", "2.0"] );
+        versions.Single().Should().Be( "2.0" );
     }
 
     [Fact]
@@ -180,22 +212,15 @@ public void read_should_ignore_excluded_media_types()
             .Exclude( "application/xml" )
             .Exclude( "application/xml+atom" )
             .Build();
-        var request = new HttpRequestMessage( Post, "http://tempuri.org" )
+        var request = new HttpRequestMessage( Get, "http://tempuri.org" )
         {
             Headers =
             {
                 Accept =
                 {
                     Parse( "application/xml" ),
                     Parse( "application/xml+atom;q=0.8;v=1.5" ),
-                    Parse( "application/json;q=0.2;v=2.0" ),
-                },
-            },
-            Content = new StringContent( "{\"message\":\"test\"}", UTF8 )
-            {
-                Headers =
-                {
-                    ContentType = Parse( "application/json;v=2.0" ),
+                    Parse( "application/json;q=0.8;v=2.0" ),
                 },
             },
         };
@@ -215,22 +240,15 @@ public void read_should_only_retrieve_included_media_types()
             .Parameter( "v" )
             .Include( "application/json" )
             .Build();
-        var request = new HttpRequestMessage( Post, "http://tempuri.org" )
+        var request = new HttpRequestMessage( Get, "http://tempuri.org" )
         {
             Headers =
             {
                 Accept =
                 {
                     Parse( "application/xml" ),
                     Parse( "application/xml+atom;q=0.8;v=1.5" ),
-                    Parse( "application/json;q=0.2;v=2.0" ),
-                },
-            },
-            Content = new StringContent( "{\"message\":\"test\"}", UTF8 )
-            {
-                Headers =
-                {
-                    ContentType = Parse( "application/json;v=2.0" ),
+                    Parse( "application/json;q=0.8;v=2.0" ),
                 },
             },
         };
@@ -335,22 +353,15 @@ public void read_should_select_first_version()
             .Parameter( "v" )
             .SelectFirstOrDefault()
             .Build();
-        var request = new HttpRequestMessage( Post, "http://tempuri.org" )
+        var request = new HttpRequestMessage( Get, "http://tempuri.org" )
         {
             Headers =
             {
                 Accept =
                 {
                     Parse( "application/xml" ),
                     Parse( "application/xml+atom;q=0.8;v=1.5" ),
-                    Parse( "application/json;q=0.2;v=2.0" ),
-                },
-            },
-            Content = new StringContent( "{\"message\":\"test\"}", UTF8 )
-            {
-                Headers =
-                {
-                    ContentType = Parse( "application/json;v=2.0" ),
+                    Parse( "application/json;q=0.8;v=2.0" ),
                 },
             },
         };
@@ -370,22 +381,15 @@ public void read_should_select_last_version()
             .Parameter( "v" )
             .SelectLastOrDefault()
             .Build();
-        var request = new HttpRequestMessage( Post, "http://tempuri.org" )
+        var request = new HttpRequestMessage( Get, "http://tempuri.org" )
         {
             Headers =
             {
                 Accept =
                 {
                     Parse( "application/xml" ),
                     Parse( "application/xml+atom;q=0.8;v=1.5" ),
-                    Parse( "application/json;q=0.2;v=2.0" ),
-                },
-            },
-            Content = new StringContent( "{\"message\":\"test\"}", UTF8 )
-            {
-                Headers =
-                {
-                    ContentType = Parse( "application/json;v=2.0" ),
+                    Parse( "application/json;q=0.8;v=2.0" ),
                 },
             },
         };

src/AspNet/WebApi/test/Asp.Versioning.WebApi.Tests/MediaTypeApiVersionReaderTest.cs

@@ -74,6 +74,11 @@ public void read_should_retrieve_version_from_accept()
     [InlineData( new[] { "application/xml", "application/json;q=0.2;v=1.0" }, "1.0" )]
     [InlineData( new[] { "application/json", "application/xml" }, null )]
     [InlineData( new[] { "application/xml", "application/xml+atom;q=0.8;v=2.5", "application/json;q=0.2;v=1.0" }, "2.5" )]
+    [InlineData( new[] { "application/json;v=1.0", "application/xml;q=0.2;v=2.0" }, "1.0" )]
+    [InlineData( new[] { "application/xml;q=0.2;v=2.0", "application/json;v=1.0" }, "1.0" )]
+    [InlineData( new[] { "application/json;v=1.0", "application/xml;q=0;v=2.0" }, "1.0" )]
+    [InlineData( new[] { "application/xml;q=0;v=2.0" }, null )]
+    [InlineData( new[] { "application/json;q=0;v=1.0", "application/xml;q=0.2;v=2.0" }, "2.0" )]
     public void read_should_retrieve_version_from_accept_with_quality( string[] mediaTypes, string expected )
     {
         // arrange
@@ -93,7 +98,29 @@ public void read_should_retrieve_version_from_accept_with_quality( string[] medi
     }
 
     [Fact]
-    public void read_should_retrieve_version_from_content_type_and_accept()
+    public void read_should_collate_incongruent_versions_from_content_type_and_accept()
+    {
+        // arrange
+        var reader = new MediaTypeApiVersionReader();
+        var request = new HttpRequestMessage( Post, "http://tempuri.org" )
+        {
+            Content = new StringContent( "{\"message\":\"test\"}", UTF8 ),
+        };
+
+        // the Accept media type has no quality parameter, so it is ranked equally with
+        // the Content-Type media type and both are collated, which is ambiguous
+        request.Content.Headers.ContentType = Parse( "application/json;v=1.0" );
+        request.Headers.Accept.Add( Parse( "application/json;v=2.0" ) );
+
+        // act
+        var versions = reader.Read( request );
+
+        // assert
+        versions.Should().BeEquivalentTo( ["1.0", "2.0"] );
+    }
+
+    [Fact]
+    public void read_should_prefer_version_from_content_type_over_accept()
     {
         // arrange
         var reader = new MediaTypeApiVersionReader();
@@ -111,7 +138,7 @@ public void read_should_retrieve_version_from_content_type_and_accept()
         var versions = reader.Read( request );
 
         // assert
-        versions.Should().BeEquivalentTo( ["1.5", "2.0"] );
+        versions.Single().Should().Be( "2.0" );
     }
 
     [Fact]
@@ -171,4 +198,57 @@ public void add_parameters_should_add_parameter_for_media_type()
         // assert
         context.Verify( c => c.AddParameter( "v", MediaTypeParameter ), Times.Once() );
     }
+
+    [Fact]
+    public void read_should_collate_equally_ranked_versions_from_accept() =>
+        AssertEquallyRankedVersionsAreCollated( new MediaTypeApiVersionReader() );
+
+    // a derived reader that does not override ReadAcceptHeader still gets the correct behavior
+    [Fact]
+    public void derived_read_should_collate_equally_ranked_versions_from_accept() =>
+        AssertEquallyRankedVersionsAreCollated( new DerivedReader() );
+
+    private static void AssertEquallyRankedVersionsAreCollated( MediaTypeApiVersionReader reader )
+    {
+        // arrange
+        var request = new HttpRequestMessage( Get, "http://tempuri.org" );
+
+        // neither media type has a quality parameter, so they are ranked equally and
+        // both are collated, which is ambiguous
+        request.Headers.Accept.Add( Parse( "application/json;v=1.0" ) );
+        request.Headers.Accept.Add( Parse( "application/xml;v=2.0" ) );
+
+        // act
+        var versions = reader.Read( request );
+
+        // assert
+        versions.Should().BeEquivalentTo( ["1.0", "2.0"] );
+    }
+
+    [Fact]
+    public void read_should_defer_to_overridden_accept_header()
+    {
+        // arrange
+        var reader = new CustomAcceptHeaderReader();
+        var request = new HttpRequestMessage( Get, "http://tempuri.org" );
+
+        request.Headers.Accept.Add( Parse( "application/json;v=1.0" ) );
+        request.Headers.Accept.Add( Parse( "application/xml;v=2.0" ) );
+
+        // act
+        var versions = reader.Read( request );
+
+        // assert
+        versions.Single().Should().Be( "42.0" );
+    }
+
+    private sealed class DerivedReader : MediaTypeApiVersionReader
+    {
+    }
+
+    private sealed class CustomAcceptHeaderReader : MediaTypeApiVersionReader
+    {
+        protected override string ReadAcceptHeader(
+            ICollection<MediaTypeWithQualityHeaderValue> accept ) => "42.0";
+    }
 }

src/AspNetCore/WebApi/src/Asp.Versioning.Http/Asp.Versioning.Http.csproj

@@ -1,7 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
  <PropertyGroup>
-  <VersionPrefix>10.2.2</VersionPrefix>
+  <VersionPrefix>10.2.3</VersionPrefix>
   <AssemblyVersion>10.2.0.0</AssemblyVersion>
   <TargetFramework>$(DefaultTargetFramework)</TargetFramework>
   <RootNamespace>Asp.Versioning</RootNamespace>

src/AspNetCore/WebApi/src/Asp.Versioning.Http/MediaTypeApiVersionReader.cs

@@ -20,20 +20,19 @@ public virtual IReadOnlyList<string> Read( HttpRequest request )
         var version = contentType is null ? default : ReadContentTypeHeader( contentType );
         var accept = headers.Accept;
 
-        if ( accept is null || ReadAcceptHeader( accept ) is not string otherVersion )
+        if ( accept is null || accept.Count == 0 )
         {
             return version is null ? [] : [version];
         }
 
-        var comparer = StringComparer.OrdinalIgnoreCase;
-
-        if ( version is null || comparer.Equals( version, otherVersion ) )
+        // TODO: the ranked implementation is the correct way, but ReadAcceptHeader requires a breaking change that
+        // cannot ship until the next major version. internally do the right thing, but if ReadAcceptHeader is
+        // overridden, then make sure we honor the implementation. the onus is on the implementer.
+        if ( acceptHeaderOverridden )
         {
-            return [otherVersion];
+            return Collate( version, ReadAcceptHeader( accept ) );
         }
 
-        return comparer.Compare( version, otherVersion ) <= 0
-               ? [version, otherVersion]
-               : [otherVersion, version];
+        return Collate( version, ReadRankedAcceptHeader( version is null ? accept : MediaTypeQuality.MaxRanked( accept ) ) );
     }
 }

src/AspNetCore/WebApi/src/Asp.Versioning.Http/ReleaseNotes.txt

@@ -1 +1 @@
-Versioned endpoint route builder should support keyed services [Issue #1219](https://github.com/dotnet/aspnet-api-versioning/issues/1219)
+Fixed media type quality (q) ranking and collation [Issue #1221](https://github.com/dotnet/aspnet-api-versioning/issues/1221)