diff --git a/src/AspNet/WebApi/src/Asp.Versioning.WebApi/Asp.Versioning.WebApi.csproj b/src/AspNet/WebApi/src/Asp.Versioning.WebApi/Asp.Versioning.WebApi.csproj index d9b9dba7..8e93499e 100644 --- a/src/AspNet/WebApi/src/Asp.Versioning.WebApi/Asp.Versioning.WebApi.csproj +++ b/src/AspNet/WebApi/src/Asp.Versioning.WebApi/Asp.Versioning.WebApi.csproj @@ -1,7 +1,7 @@  - 10.2.0 + 10.2.1 10.2.0.0 net45;net472 ASP.NET Web API Versioning diff --git a/src/AspNet/WebApi/src/Asp.Versioning.WebApi/MediaTypeApiVersionReader.cs b/src/AspNet/WebApi/src/Asp.Versioning.WebApi/MediaTypeApiVersionReader.cs index d3a96174..fd3aa4db 100644 --- a/src/AspNet/WebApi/src/Asp.Versioning.WebApi/MediaTypeApiVersionReader.cs +++ b/src/AspNet/WebApi/src/Asp.Versioning.WebApi/MediaTypeApiVersionReader.cs @@ -16,20 +16,19 @@ public virtual IReadOnlyList 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 ) ) ); } } \ No newline at end of file diff --git a/src/AspNet/WebApi/src/Asp.Versioning.WebApi/ReleaseNotes.txt b/src/AspNet/WebApi/src/Asp.Versioning.WebApi/ReleaseNotes.txt index 5f282702..a8dba7f3 100644 --- a/src/AspNet/WebApi/src/Asp.Versioning.WebApi/ReleaseNotes.txt +++ b/src/AspNet/WebApi/src/Asp.Versioning.WebApi/ReleaseNotes.txt @@ -1 +1 @@ - \ No newline at end of file +Fixed media type quality (q) ranking and collation [Issue #1221](https://github.com/dotnet/aspnet-api-versioning/issues/1221) \ No newline at end of file diff --git a/src/AspNet/WebApi/test/Asp.Versioning.WebApi.Tests/MediaTypeApiVersionReaderBuilderTest.cs b/src/AspNet/WebApi/test/Asp.Versioning.WebApi.Tests/MediaTypeApiVersionReaderBuilderTest.cs index a590d839..b943190e 100644 --- a/src/AspNet/WebApi/test/Asp.Versioning.WebApi.Tests/MediaTypeApiVersionReaderBuilderTest.cs +++ b/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,7 +212,7 @@ 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 = { @@ -188,14 +220,7 @@ public void read_should_ignore_excluded_media_types() { 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,7 +240,7 @@ 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 = { @@ -223,14 +248,7 @@ public void read_should_only_retrieve_included_media_types() { 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,7 +353,7 @@ 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 = { @@ -343,14 +361,7 @@ public void read_should_select_first_version() { 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,7 +381,7 @@ 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 = { @@ -378,14 +389,7 @@ public void read_should_select_last_version() { 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" ), }, }, }; diff --git a/src/AspNet/WebApi/test/Asp.Versioning.WebApi.Tests/MediaTypeApiVersionReaderTest.cs b/src/AspNet/WebApi/test/Asp.Versioning.WebApi.Tests/MediaTypeApiVersionReaderTest.cs index ea9ee138..37d3b032 100644 --- a/src/AspNet/WebApi/test/Asp.Versioning.WebApi.Tests/MediaTypeApiVersionReaderTest.cs +++ b/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 accept ) => "42.0"; + } } \ No newline at end of file diff --git a/src/AspNetCore/WebApi/src/Asp.Versioning.Http/Asp.Versioning.Http.csproj b/src/AspNetCore/WebApi/src/Asp.Versioning.Http/Asp.Versioning.Http.csproj index 2ffee07e..4e32da2e 100644 --- a/src/AspNetCore/WebApi/src/Asp.Versioning.Http/Asp.Versioning.Http.csproj +++ b/src/AspNetCore/WebApi/src/Asp.Versioning.Http/Asp.Versioning.Http.csproj @@ -1,7 +1,7 @@  - 10.2.2 + 10.2.3 10.2.0.0 $(DefaultTargetFramework) Asp.Versioning diff --git a/src/AspNetCore/WebApi/src/Asp.Versioning.Http/MediaTypeApiVersionReader.cs b/src/AspNetCore/WebApi/src/Asp.Versioning.Http/MediaTypeApiVersionReader.cs index 9a040c6f..420b37df 100644 --- a/src/AspNetCore/WebApi/src/Asp.Versioning.Http/MediaTypeApiVersionReader.cs +++ b/src/AspNetCore/WebApi/src/Asp.Versioning.Http/MediaTypeApiVersionReader.cs @@ -20,20 +20,19 @@ public virtual IReadOnlyList 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 ) ) ); } } \ No newline at end of file diff --git a/src/AspNetCore/WebApi/src/Asp.Versioning.Http/ReleaseNotes.txt b/src/AspNetCore/WebApi/src/Asp.Versioning.Http/ReleaseNotes.txt index 0ff9e487..a8dba7f3 100644 --- a/src/AspNetCore/WebApi/src/Asp.Versioning.Http/ReleaseNotes.txt +++ b/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) \ No newline at end of file +Fixed media type quality (q) ranking and collation [Issue #1221](https://github.com/dotnet/aspnet-api-versioning/issues/1221) \ No newline at end of file diff --git a/src/AspNetCore/WebApi/test/Asp.Versioning.Http.Tests/MediaTypeApiVersionBuilderTest.cs b/src/AspNetCore/WebApi/test/Asp.Versioning.Http.Tests/MediaTypeApiVersionBuilderTest.cs index e5372d37..37f94b54 100644 --- a/src/AspNetCore/WebApi/test/Asp.Versioning.Http.Tests/MediaTypeApiVersionBuilderTest.cs +++ b/src/AspNetCore/WebApi/test/Asp.Versioning.Http.Tests/MediaTypeApiVersionBuilderTest.cs @@ -75,6 +75,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 @@ -99,7 +101,33 @@ 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(); + var request = new Mock(); + var headers = new HeaderDictionary() + { + // 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 + ["Accept"] = new StringValues( "application/json;v=2.0" ), + ["Content-Type"] = new StringValues( "application/json;v=1.0" ), + }; + + request.SetupGet( r => r.Headers ).Returns( headers ); + request.SetupProperty( r => r.Body, Null ); + request.SetupProperty( r => r.ContentLength, 0L ); + request.SetupProperty( r => r.ContentType, "application/json;v=1.0" ); + + // act + var versions = reader.Read( request.Object ); + + // 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 +153,7 @@ public void read_should_retrieve_version_from_content_type_and_accept() var versions = reader.Read( request.Object ); // assert - versions.Should().BeEquivalentTo( "1.5", "2.0" ); + versions.Single().Should().Be( "2.0" ); } [Fact] @@ -183,18 +211,14 @@ public void read_should_ignore_excluded_media_types() { "application/xml", "application/xml+atom;q=0.8;v=1.5", - "application/json;q=0.2;v=2.0", + "application/json;q=0.8;v=2.0", }; var headers = new HeaderDictionary() { ["Accept"] = new StringValues( mediaTypes ), - ["Content-Type"] = new StringValues( "application/json;v=2.0" ), }; request.SetupGet( r => r.Headers ).Returns( headers ); - request.SetupProperty( r => r.Body, Null ); - request.SetupProperty( r => r.ContentLength, 0L ); - request.SetupProperty( r => r.ContentType, "application/json;v=2.0" ); // act var versions = reader.Read( request.Object ); @@ -216,18 +240,14 @@ public void read_should_only_retrieve_included_media_types() { "application/xml", "application/xml+atom;q=0.8;v=1.5", - "application/json;q=0.2;v=2.0", + "application/json;q=0.8;v=2.0", }; var headers = new HeaderDictionary() { ["Accept"] = new StringValues( mediaTypes ), - ["Content-Type"] = new StringValues( "application/json;v=2.0" ), }; request.SetupGet( r => r.Headers ).Returns( headers ); - request.SetupProperty( r => r.Body, Null ); - request.SetupProperty( r => r.ContentLength, 0L ); - request.SetupProperty( r => r.ContentType, "application/json;v=2.0" ); // act var versions = reader.Read( request.Object ); @@ -334,18 +354,14 @@ public void read_should_select_first_version() { "application/xml", "application/xml+atom;q=0.8;v=1.5", - "application/json;q=0.2;v=2.0", + "application/json;q=0.8;v=2.0", }; var headers = new HeaderDictionary() { ["Accept"] = new StringValues( mediaTypes ), - ["Content-Type"] = new StringValues( "application/json;v=2.0" ), }; request.SetupGet( r => r.Headers ).Returns( headers ); - request.SetupProperty( r => r.Body, Null ); - request.SetupProperty( r => r.ContentLength, 0L ); - request.SetupProperty( r => r.ContentType, "application/json;v=2.0" ); // act var versions = reader.Read( request.Object ); @@ -367,18 +383,14 @@ public void read_should_select_last_version() { "application/xml", "application/xml+atom;q=0.8;v=1.5", - "application/json;q=0.2;v=2.0", + "application/json;q=0.8;v=2.0", }; var headers = new HeaderDictionary() { ["Accept"] = new StringValues( mediaTypes ), - ["Content-Type"] = new StringValues( "application/json;v=2.0" ), }; request.SetupGet( r => r.Headers ).Returns( headers ); - request.SetupProperty( r => r.Body, Null ); - request.SetupProperty( r => r.ContentLength, 0L ); - request.SetupProperty( r => r.ContentType, "application/json;v=2.0" ); // act var versions = reader.Read( request.Object ); diff --git a/src/AspNetCore/WebApi/test/Asp.Versioning.Http.Tests/MediaTypeApiVersionReaderTest.cs b/src/AspNetCore/WebApi/test/Asp.Versioning.Http.Tests/MediaTypeApiVersionReaderTest.cs index 16671d3e..919aec7d 100644 --- a/src/AspNetCore/WebApi/test/Asp.Versioning.Http.Tests/MediaTypeApiVersionReaderTest.cs +++ b/src/AspNetCore/WebApi/test/Asp.Versioning.Http.Tests/MediaTypeApiVersionReaderTest.cs @@ -75,6 +75,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 @@ -95,7 +100,33 @@ 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 Mock(); + var headers = new HeaderDictionary() + { + // 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 + ["Accept"] = new StringValues( "application/json;v=2.0" ), + ["Content-Type"] = new StringValues( "application/json;v=1.0" ), + }; + + request.SetupGet( r => r.Headers ).Returns( headers ); + request.SetupProperty( r => r.Body, Null ); + request.SetupProperty( r => r.ContentLength, 0L ); + request.SetupProperty( r => r.ContentType, "application/json;v=1.0" ); + + // act + var versions = reader.Read( request.Object ); + + // 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(); @@ -121,7 +152,7 @@ public void read_should_retrieve_version_from_content_type_and_accept() var versions = reader.Read( request.Object ); // assert - versions.Should().BeEquivalentTo( "1.5", "2.0" ); + versions.Single().Should().Be( "2.0" ); } [Fact] @@ -182,4 +213,65 @@ 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 Mock(); + var mediaTypes = new[] { "application/json;v=1.0", "application/xml;v=2.0" }; + var headers = new HeaderDictionary() + { + // neither media type has a quality parameter, so they are ranked equally and + // both are collated, which is ambiguous + ["Accept"] = new StringValues( mediaTypes ), + }; + + request.SetupGet( r => r.Headers ).Returns( headers ); + + // act + var versions = reader.Read( request.Object ); + + // 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 Mock(); + var mediaTypes = new[] { "application/json;v=1.0", "application/xml;v=2.0" }; + var headers = new HeaderDictionary() + { + ["Accept"] = new StringValues( mediaTypes ), + }; + + request.SetupGet( r => r.Headers ).Returns( headers ); + + // act + var versions = reader.Read( request.Object ); + + // assert + versions.Single().Should().Be( "42.0" ); + } + + private sealed class DerivedReader : MediaTypeApiVersionReader + { + } + + private sealed class CustomAcceptHeaderReader : MediaTypeApiVersionReader + { + protected override string ReadAcceptHeader( + ICollection accept ) => "42.0"; + } } \ No newline at end of file diff --git a/src/Common/src/Common/MediaTypeApiVersionReader.cs b/src/Common/src/Common/MediaTypeApiVersionReader.cs index 4f01f4f0..6a4ecf2f 100644 --- a/src/Common/src/Common/MediaTypeApiVersionReader.cs +++ b/src/Common/src/Common/MediaTypeApiVersionReader.cs @@ -16,11 +16,17 @@ namespace Asp.Versioning; /// public partial class MediaTypeApiVersionReader : IApiVersionReader { + private readonly bool acceptHeaderOverridden; + /// /// Initializes a new instance of the class. /// /// This constructor always uses the "v" media type parameter. - public MediaTypeApiVersionReader() => ParameterName = "v"; + public MediaTypeApiVersionReader() + { + ParameterName = "v"; + acceptHeaderOverridden = IsAcceptHeaderOverridden(); + } /// /// Initializes a new instance of the class. @@ -30,6 +36,7 @@ public MediaTypeApiVersionReader( string parameterName ) { ArgumentException.ThrowIfNullOrEmpty( parameterName ); ParameterName = parameterName; + acceptHeaderOverridden = IsAcceptHeaderOverridden(); } /// @@ -45,12 +52,30 @@ public MediaTypeApiVersionReader( string parameterName ) /// The collection of Accept /// headers to read from. /// The API version read or null. - /// The default implementation will return the first defined API version ranked by the media type - /// quality parameter. + /// + /// This method returns the first defined API version ranked by the media type quality parameter. A media + /// type without a quality parameter has the highest weight of 1.0, and a media type with a quality of zero is + /// never considered because it means 'not acceptable'. + /// An API version reader discovers and collates API versions rather than selecting one, and equally + /// ranked media types can yield more than one. This method cannot express that, so it is only used when a + /// derived class overrides it; an overriding implementation is responsible for ranking and resolving the + /// Accept header itself. When it is not overridden, an internal implementation that collates equally ranked + /// media types is used instead. + /// This method will be refactored in a future major version. + /// protected virtual string? ReadAcceptHeader( ICollection accept ) { + // TODO: refactor breaking change at next major version ArgumentNullException.ThrowIfNull( accept ); + return ReadRankedAcceptHeader( accept ) is { } versions ? versions[0] : default; + } + // this is the correct, expected behavior because equally ranked media types can collate more than one API version. + // it cannot replace ReadAcceptHeader before the next major version because widening the return type is a breaking + // change for any derived class that overrides it. this becomes the permanent implementation when a breaking change + // is allowed, at which point ReadAcceptHeader and the override detection it requires can both be removed + private List? ReadRankedAcceptHeader( ICollection accept ) + { var count = accept.Count; if ( count == 0 ) @@ -60,48 +85,111 @@ public MediaTypeApiVersionReader( string parameterName ) var mediaTypes = accept.ToArray(); - System.Array.Sort( mediaTypes, ByQualityDescending ); + MediaTypeQuality.SortDescending( mediaTypes ); - for ( var i = 0; i < count; i++ ) + var versions = default( List ); + var start = 0; + + while ( start < count ) { -#if NETFRAMEWORK - var parameters = mediaTypes[i].Parameters.ToArray(); - var paramCount = parameters.Length; -#else - var parameters = mediaTypes[i].Parameters; - var paramCount = parameters.Count; -#endif - for ( var j = 0; j < paramCount; j++ ) + // neither this media type nor any that follow can be considered + if ( !MediaTypeQuality.IsAcceptable( mediaTypes[start] ) ) { - var parameter = parameters[j]; + break; + } + + var end = start + 1; + + while ( end < count && MediaTypeQuality.SameRank( mediaTypes[end], mediaTypes[start] ) ) + { + end++; + } - if ( parameter.Name.Equals( ParameterName, OrdinalIgnoreCase ) ) + // media types collated at the same rank are equally preferred + for ( var i = start; i < end; i++ ) + { + if ( ReadParameter( mediaTypes[i] ) is not string value ) { -#if NETFRAMEWORK - return parameter.Value; -#else - return parameter.Value.Value; -#endif + continue; + } + + versions ??= new( capacity: end - start ); + + if ( !versions.Contains( value, StringComparer.OrdinalIgnoreCase ) ) + { + versions.Add( value ); } } + + // a higher rank breaks precedence over every media type ranked below it + if ( versions is not null ) + { + break; + } + + start = end; } - return default; + return versions; } - /// - /// Reads the requested API version from the HTTP Content-Type header. - /// - /// The Content-Type header to read from. - /// The API version read or null. - protected virtual string? ReadContentTypeHeader( MediaTypeHeaderValue contentType ) + private bool IsAcceptHeaderOverridden() + { + if ( GetType() == typeof( MediaTypeApiVersionReader ) ) + { + return false; + } + + var readAcceptHeader = ReadAcceptHeader; + + return readAcceptHeader.Method.DeclaringType != typeof( MediaTypeApiVersionReader ); + } + + private static IReadOnlyList Collate( string? version, string? otherVersion ) + { + if ( otherVersion is null ) + { + return version is null ? [] : [version]; + } + + return version is null || StringComparer.OrdinalIgnoreCase.Equals( version, otherVersion ) + ? [otherVersion] + : [version, otherVersion]; + } + + private static List Collate( string? version, List? versions ) + { + if ( versions is null || versions.Count == 0 ) + { + return version is null ? [] : [version]; + } + + if ( version is null ) + { + return versions; + } + + // the content-type version is ranked first among the equally ranked versions it is collated with + var collated = new List( capacity: versions.Count + 1 ) { version }; + + for ( var i = 0; i < versions.Count; i++ ) + { + if ( !StringComparer.OrdinalIgnoreCase.Equals( version, versions[i] ) ) + { + collated.Add( versions[i] ); + } + } + + return collated; + } + + private string? ReadParameter( MediaTypeHeaderValue mediaType ) { - ArgumentNullException.ThrowIfNull( contentType ); #if NETFRAMEWORK - var parameters = contentType.Parameters.ToArray(); + var parameters = mediaType.Parameters.ToArray(); var count = parameters.Length; #else - var parameters = contentType.Parameters; + var parameters = mediaType.Parameters; var count = parameters.Count; #endif for ( var i = 0; i < count; i++ ) @@ -121,6 +209,17 @@ public MediaTypeApiVersionReader( string parameterName ) return default; } + /// + /// Reads the requested API version from the HTTP Content-Type header. + /// + /// The Content-Type header to read from. + /// The API version read or null. + protected virtual string? ReadContentTypeHeader( MediaTypeHeaderValue contentType ) + { + ArgumentNullException.ThrowIfNull( contentType ); + return ReadParameter( contentType ); + } + /// /// Provides API version parameter descriptions supported by the current reader using the supplied provider. /// @@ -130,7 +229,4 @@ public virtual void AddParameters( IApiVersionParameterDescriptionContext contex ArgumentNullException.ThrowIfNull( context ); context.AddParameter( ParameterName, MediaTypeParameter ); } - - private static int ByQualityDescending( MediaTypeWithQualityHeaderValue? left, MediaTypeWithQualityHeaderValue? right ) => - -Nullable.Compare( left?.Quality, right?.Quality ); } \ No newline at end of file diff --git a/src/Common/src/Common/MediaTypeApiVersionReaderBuilder.cs b/src/Common/src/Common/MediaTypeApiVersionReaderBuilder.cs index 7a344ba5..c18f5494 100644 --- a/src/Common/src/Common/MediaTypeApiVersionReaderBuilder.cs +++ b/src/Common/src/Common/MediaTypeApiVersionReaderBuilder.cs @@ -346,10 +346,12 @@ public IReadOnlyList Read( HttpRequest request ) var contentType = headers.ContentType; #endif var accept = headers.Accept; - var version = default( string ); - var versions = default( SortedSet ); + var versions = default( List ); var mediaTypes = default( List ); + // the content-type header has no quality parameter, so it always ranks at the maximum weight; adding it + // first keeps it ahead of the equally ranked Accept media types it is collated with, while any lower + // ranked media type is outranked by it if ( contentType != null ) { #if NETFRAMEWORK @@ -372,29 +374,64 @@ public IReadOnlyList Read( HttpRequest request ) Filter( mediaTypes ); - switch ( mediaTypes.Count ) + if ( mediaTypes.Count > 1 ) { - case 0: - return []; - case 1: - break; - default: - mediaTypes.Sort( static ( l, r ) => -Nullable.Compare( l.Quality, r.Quality ) ); - break; + MediaTypeQuality.SortDescending( mediaTypes ); } - Read( mediaTypes, ref version, ref versions ); + ReadHighestRanked( mediaTypes, ref versions ); if ( versions == null ) { - return version == null ? Array.Empty() : [version]; + return []; } - return selector( request, [.. versions] ); + return versions.Count == 1 ? versions : selector( request, versions ); + } + + // the accept header is a list of preferences; the highest-ranked media types that yield an api version break + // precedence over all lower-ranked media types, while media types collated at that same rank are equally + // preferred and all contribute + private void ReadHighestRanked( List mediaTypes, ref List? versions ) + { + var count = mediaTypes.Count; + var start = 0; + + while ( start < count ) + { + var end = start + 1; + + while ( end < count && MediaTypeQuality.SameRank( mediaTypes[end], mediaTypes[start] ) ) + { + end++; + } + + var ranked = start == 0 && end == count + ? mediaTypes + : mediaTypes.GetRange( start, end - start ); + var before = versions == null ? 0 : versions.Count; + + Read( ranked, ref versions ); + + if ( versions != null && versions.Count > before ) + { + return; + } + + start = end; + } } private void Filter( List mediaTypes ) { + for ( var i = mediaTypes.Count - 1; i >= 0; i-- ) + { + if ( !MediaTypeQuality.IsAcceptable( mediaTypes[i] ) ) + { + mediaTypes.RemoveAt( i ); + } + } + if ( excluded.Count > 0 ) { for ( var i = mediaTypes.Count - 1; i >= 0; i-- ) @@ -422,10 +459,9 @@ private void Filter( List mediaTypes ) } } - private void Read( - IReadOnlyList mediaTypes, - ref string? version, - ref SortedSet? versions ) + // the order results are discovered in is meaningful, so duplicates are removed in place rather than by + // collecting into a set, which would sort them + private void Read( IReadOnlyList mediaTypes, ref List? versions ) { for ( var i = 0; i < readers.Length; i++ ) { @@ -433,21 +469,13 @@ private void Read( for ( var j = 0; j < result.Count; j++ ) { - if ( version == null ) - { - version = result[j]; - } - else if ( versions == null ) - { - versions = new( StringComparer.OrdinalIgnoreCase ) - { - version, - result[j], - }; - } - else + var value = result[j]; + + versions ??= []; + + if ( !versions.Contains( value, StringComparer.OrdinalIgnoreCase ) ) { - versions.Add( result[j] ); + versions.Add( value ); } } } diff --git a/src/Common/src/Common/MediaTypeQuality.cs b/src/Common/src/Common/MediaTypeQuality.cs new file mode 100644 index 00000000..ac8b8004 --- /dev/null +++ b/src/Common/src/Common/MediaTypeQuality.cs @@ -0,0 +1,60 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. + +namespace Asp.Versioning; + +#if NETFRAMEWORK +using MediaType = System.Net.Http.Headers.MediaTypeWithQualityHeaderValue; +#else +using MediaType = Microsoft.Net.Http.Headers.MediaTypeHeaderValue; +#endif + +internal static class MediaTypeQuality +{ + // The weight is normalized to a real number in the range 0 through 1, where 0.001 is the least preferred and 1 is + // the most preferred; a value of 0 means 'not acceptable'. If no 'q' parameter is present, the default weight is 1 + // REF: https://www.rfc-editor.org/rfc/rfc9110#section-12.4.2 + private const double DefaultWeight = 1.0; + + internal static bool IsAcceptable( MediaType mediaType ) => WeightOf( mediaType ) > 0d; + + // weights parsed from the same textual form yield the same value and an omitted parameter always yields the same + // constant, so an exact comparison is appropriate here + internal static bool SameRank( MediaType left, MediaType right ) => WeightOf( left ) == WeightOf( right ); + + // the content-type header has no quality parameter, so it always has the maximum weight. only media types of the + // same rank can be collated with it; anything lower is outranked + internal static ICollection MaxRanked( ICollection mediaTypes ) + { + var maxRanked = default( List ); + + foreach ( var mediaType in mediaTypes.Where( mt => WeightOf( mt ) == DefaultWeight ) ) + { + ( maxRanked ??= new( capacity: mediaTypes.Count ) ).Add( mediaType ); + } + + return maxRanked ?? []; + } + + // an insertion sort is used because it is stable, which retains the order specified by the client for media types + // of equal weight. the number of media types in a header is expected to be small, which makes the cost negligible + internal static void SortDescending( IList mediaTypes ) + { + var count = mediaTypes.Count; + + for ( var i = 1; i < count; i++ ) + { + var mediaType = mediaTypes[i]; + var weight = WeightOf( mediaType ); + var j = i - 1; + + for ( ; j >= 0 && WeightOf( mediaTypes[j] ) < weight; j-- ) + { + mediaTypes[j + 1] = mediaTypes[j]; + } + + mediaTypes[j + 1] = mediaType; + } + } + + private static double WeightOf( MediaType mediaType ) => mediaType.Quality ?? DefaultWeight; +} \ No newline at end of file