Skip to content

Commit 218d782

Browse files
author
Chris Martinez
committed
Fixes the cloning of supported media types in formatters. Relates to #164 
1 parent 37ea922 commit 218d782

File tree

4 files changed

+78
-21
lines changed

4 files changed

+78
-21
lines changed

‎src/Microsoft.AspNet.WebApi.Versioning.ApiExplorer/Description/ApiVersionParameterDescriptionContext.cs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,9 +158,13 @@ protected virtual void AddMediaTypeParameter( string name )
158158

159159
CloneFormattersAndAddMediaTypeParameter( parameter, ApiDescription.SupportedRequestBodyFormatters );
160160
CloneFormattersAndAddMediaTypeParameter( parameter, ApiDescription.SupportedResponseFormatters );
161+
parameters.Add( NewApiVersionParameter( name, Unknown, allowOptional: false ) );
161162
}
162163

163-
ApiParameterDescription NewApiVersionParameter( string name, ApiParameterSource source )
164+
ApiParameterDescription NewApiVersionParameter( string name, ApiParameterSource source ) =>
165+
NewApiVersionParameter( name, source, optional );
166+
167+
ApiParameterDescription NewApiVersionParameter( string name, ApiParameterSource source, bool allowOptional )
164168
{
165169
Contract.Requires( !string.IsNullOrEmpty( name ) );
166170
Contract.Ensures( Contract.Result<ApiParameterDescription>() != null );
@@ -170,7 +174,7 @@ ApiParameterDescription NewApiVersionParameter( string name, ApiParameterSource
170174
{
171175
Name = name,
172176
Documentation = Options.DefaultApiVersionParameterDescription,
173-
ParameterDescriptor = new ApiVersionParameterDescriptor( name, ApiVersion.ToString(), optional )
177+
ParameterDescriptor = new ApiVersionParameterDescriptor( name, ApiVersion.ToString(), allowOptional )
174178
{
175179
Configuration = action.Configuration,
176180
ActionDescriptor = action

‎src/Microsoft.AspNet.WebApi.Versioning.ApiExplorer/Microsoft.AspNet.WebApi.Versioning.ApiExplorer.csproj

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
<Project Sdk="Microsoft.NET.Sdk">
22

33
<PropertyGroup>
4-
<VersionPrefix>1.0.1</VersionPrefix>
4+
<VersionPrefix>1.0.2</VersionPrefix>
55
<AssemblyVersion>1.0.0.0</AssemblyVersion>
66
<TargetFramework>net45</TargetFramework>
77
<AssemblyName>Microsoft.AspNet.WebApi.Versioning.ApiExplorer</AssemblyName>
@@ -10,7 +10,7 @@
1010
<RootNamespace>Microsoft.Web.Http</RootNamespace>
1111
<DefineConstants>$(DefineConstants);WEBAPI</DefineConstants>
1212
<PackageTags>Microsoft;AspNet;AspNetWebAPI;Versioning;ApiExplorer</PackageTags>
13-
<PackageReleaseNotes>• Improve cloning of MediaTypeFormatter (Issue #156)</PackageReleaseNotes>
13+
<PackageReleaseNotes>• Fix cloning MediaTypeFormatter.SupportedMediaTypes (Issue #164)</PackageReleaseNotes>
1414
</PropertyGroup>
1515

1616
<ItemGroup>

‎src/Microsoft.AspNet.WebApi.Versioning.ApiExplorer/System.Web.Http/MediaTypeFormatterAdapterFactory.cs

Lines changed: 63 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,12 @@
44
using Microsoft.Web.Http;
55
using System;
66
using System.Collections.Concurrent;
7+
using System.Collections.Generic;
78
using System.Diagnostics.Contracts;
89
using System.Linq;
910
using System.Net.Http.Formatting;
11+
using System.Net.Http.Headers;
12+
using System.Reflection;
1013
using static System.Linq.Expressions.Expression;
1114
using static System.Net.Http.Headers.MediaTypeHeaderValue;
1215
using static System.Reflection.BindingFlags;
@@ -28,13 +31,7 @@ internal static Func<MediaTypeFormatter, MediaTypeFormatter> GetOrCreateCloneFun
2831
return cloneFunctions.GetOrAdd( type, NewCloneFunction );
2932
}
3033

31-
static MediaTypeFormatter UseICloneable( MediaTypeFormatter instance )
32-
{
33-
Contract.Requires( instance != null );
34-
Contract.Ensures( Contract.Result<MediaTypeFormatter>() != null );
35-
36-
return (MediaTypeFormatter) ( (ICloneable) instance ).Clone();
37-
}
34+
static MediaTypeFormatter UseICloneable( MediaTypeFormatter instance ) => (MediaTypeFormatter) ( (ICloneable) instance ).Clone();
3835

3936
static Func<MediaTypeFormatter, MediaTypeFormatter> NewCloneFunction( Type type )
4037
{
@@ -45,7 +42,7 @@ static Func<MediaTypeFormatter, MediaTypeFormatter> NewCloneFunction( Type type
4542
NewParameterlessConstructorActivator( type ) ??
4643
throw new InvalidOperationException( LocalSR.MediaTypeFormatterNotCloneable.FormatDefault( type.Name, typeof( ICloneable ).Name ) );
4744

48-
return instance => CloneMediaTypes( clone( instance ) );
45+
return instance => CloneMediaTypes( clone( instance ), instance );
4946
}
5047

5148
static Func<MediaTypeFormatter, MediaTypeFormatter> NewCopyConstructorActivator( Type type )
@@ -67,7 +64,20 @@ static Func<MediaTypeFormatter, MediaTypeFormatter> NewCopyConstructorActivator(
6764
var @new = New( constructor, Convert( formatter, type ) );
6865
var lambda = Lambda<Func<MediaTypeFormatter, MediaTypeFormatter>>( @new, formatter );
6966

70-
return lambda.Compile(); // formatter => new MediaTypeFormatter( formatter );
67+
return ReinitializeSupportedMediaTypes( lambda.Compile() ); // formatter => new MediaTypeFormatter( formatter );
68+
}
69+
70+
static Func<MediaTypeFormatter, MediaTypeFormatter> ReinitializeSupportedMediaTypes( Func<MediaTypeFormatter, MediaTypeFormatter> clone )
71+
{
72+
Contract.Requires( clone != null );
73+
Contract.Ensures( Contract.Result<Func<MediaTypeFormatter, MediaTypeFormatter>>() != null );
74+
75+
return formatter =>
76+
{
77+
var instance = clone( formatter );
78+
SupportedMediaTypesInitializer.Initialize( instance );
79+
return instance;
80+
};
7181
}
7282

7383
static Func<MediaTypeFormatter, MediaTypeFormatter> NewParameterlessConstructorActivator( Type type )
@@ -92,21 +102,58 @@ static Func<MediaTypeFormatter, MediaTypeFormatter> NewParameterlessConstructorA
92102
return lambda.Compile(); // formatter => new MediaTypeFormatter();
93103
}
94104

95-
static MediaTypeFormatter CloneMediaTypes( MediaTypeFormatter instance )
105+
static MediaTypeFormatter CloneMediaTypes( MediaTypeFormatter target, MediaTypeFormatter source )
96106
{
97-
Contract.Requires( instance != null );
107+
Contract.Requires( target != null );
108+
Contract.Requires( source != null );
98109
Contract.Ensures( Contract.Result<MediaTypeFormatter>() != null );
99110

100-
var mediaTypes = instance.SupportedMediaTypes.ToArray();
111+
target.SupportedMediaTypes.Clear();
101112

102-
instance.SupportedMediaTypes.Clear();
113+
foreach ( var mediaType in source.SupportedMediaTypes )
114+
{
115+
target.SupportedMediaTypes.Add( Parse( mediaType.ToString() ) );
116+
}
117+
118+
return target;
119+
}
120+
121+
/// <summary>
122+
/// Supports cloning with a copy constructor.
123+
/// </summary>
124+
/// <remarks>
125+
/// The <see cref="MediaTypeFormatter"/> copy constructor does not clone the SupportedMediaTypes property or backing field.
126+
/// <seealso cref="!:https://github.com/ASP-NET-MVC/aspnetwebstack/blob/4e40cdef9c8a8226685f95ef03b746bc8322aa92/src/System.Net.Http.Formatting/Formatting/MediaTypeFormatter.cs#L62"/>
127+
/// </remarks>
128+
static class SupportedMediaTypesInitializer
129+
{
130+
static FieldInfo field;
131+
static PropertyInfo property;
132+
static readonly ConstructorInfo newCollection;
103133

104-
foreach ( var mediaType in mediaTypes )
134+
static SupportedMediaTypesInitializer()
105135
{
106-
instance.SupportedMediaTypes.Add( Parse( mediaType.ToString() ) );
136+
var flags = Public | NonPublic | Instance;
137+
var mediaTypeFormatter = typeof( MediaTypeFormatter );
138+
139+
field = mediaTypeFormatter.GetField( "_supportedMediaTypes", flags );
140+
property = mediaTypeFormatter.GetProperty( nameof( MediaTypeFormatter.SupportedMediaTypes ), flags );
141+
newCollection = mediaTypeFormatter.GetNestedType( "MediaTypeHeaderValueCollection", flags ).GetConstructors( flags ).Single();
107142
}
108143

109-
return instance;
144+
internal static void Initialize( MediaTypeFormatter instance )
145+
{
146+
var list = new List<MediaTypeHeaderValue>();
147+
var collection = newCollection.Invoke( new object[] { list } );
148+
149+
// the _supportedMediaTypes field is "readonly", which is why we must use Reflection instead of compiling an expression;
150+
// interestingly, the Reflection API lets us break rules that expression compilation does not
151+
field.SetValue( instance, list );
152+
153+
// since the value for the SupportedMediaTypes property comes from the backing field, we must do this here, even
154+
// though it's possible to set this property with a compiled expression
155+
property.SetMethod.Invoke( instance, new object[] { collection } );
156+
}
110157
}
111158
}
112159
}

‎test/Microsoft.AspNet.WebApi.Versioning.ApiExplorer.Tests/Description/ApiVersionParameterDescriptionContextTest.cs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,14 +169,20 @@ public void add_parameter_should_add_descriptor_for_media_type_parameter()
169169
{
170170
// arrange
171171
var configuration = new HttpConfiguration();
172+
var action = new Mock<HttpActionDescriptor>() { CallBase = true }.Object;
172173
var json = new JsonMediaTypeFormatter();
173174
var formUrlEncoded = new FormUrlEncodedMediaTypeFormatter();
174175

175176
configuration.Formatters.Clear();
176177
configuration.Formatters.Add( json );
177178
configuration.Formatters.Add( formUrlEncoded );
179+
action.Configuration = configuration;
178180

179-
var description = new ApiDescription() { SupportedRequestBodyFormatters = { json, formUrlEncoded } };
181+
var description = new ApiDescription()
182+
{
183+
ActionDescriptor = action,
184+
SupportedRequestBodyFormatters = { json, formUrlEncoded }
185+
};
180186
var version = new ApiVersion( 1, 0 );
181187
var options = new ApiExplorerOptions( configuration );
182188
var context = new ApiVersionParameterDescriptionContext( description, version, options );

0 commit comments

Comments
 (0)