Skip to content

Commit 16b2ebe

Browse files
Chris Martinezcommonsensesoftware
Chris Martinez
authored and
commonsensesoftware
committed
Refactor API version model aggregation to be pre-computed
1 parent f17a892 commit 16b2ebe

File tree

9 files changed

+285
-32
lines changed

9 files changed

+285
-32
lines changed

Diff for: ‎.editorconfig

+2-2
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,11 @@ indent_size = 4
1313

1414
# xml project files
1515
[*.{csproj,vbproj,proj,projitems,shproj}]
16-
indent_size = 2
16+
indent_size = 1
1717

1818
# xml config files
1919
[*.{props,targets,ruleset,config,nuspec,resx,vsixmanifest,vsct}]
20-
indent_size = 2
20+
indent_size = 1
2121

2222
# json files
2323
[*.json]

Diff for: ‎build/common.props

+4
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@
77
<Copyright>© $(Company). All rights reserved.</Copyright>
88
<NeutralLanguage>en</NeutralLanguage>
99
<DefaultLanguage>en-US</DefaultLanguage>
10+
</PropertyGroup>
11+
12+
<PropertyGroup Label="C#">
13+
<LangVersion>latest</LangVersion>
1014
</PropertyGroup>
1115

1216
</Project>

Diff for: ‎src/Microsoft.AspNetCore.Mvc.Versioning/Abstractions/ActionDescriptorExtensions.cs

-26
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22
{
33
using ApplicationModels;
44
using System;
5-
using System.Collections.Generic;
6-
using System.Diagnostics.Contracts;
75
using System.Linq;
86
using Versioning;
97

@@ -13,30 +11,6 @@
1311
[CLSCompliant( false )]
1412
public static class ActionDescriptorExtensions
1513
{
16-
const string VersionPolicyIsAppliedKey = "MS_" + nameof( VersionPolicyIsApplied );
17-
18-
static void VersionPolicyIsApplied( this ActionDescriptor action, bool value ) => action.Properties[VersionPolicyIsAppliedKey] = value;
19-
20-
internal static bool VersionPolicyIsApplied( this ActionDescriptor action ) => action.Properties.GetOrDefault( VersionPolicyIsAppliedKey, false );
21-
22-
internal static void AggregateAllVersions( this ActionDescriptor action, IEnumerable<ActionDescriptor> matchingActions )
23-
{
24-
Contract.Requires( action != null );
25-
Contract.Requires( matchingActions != null );
26-
27-
if ( action.VersionPolicyIsApplied() )
28-
{
29-
return;
30-
}
31-
32-
action.VersionPolicyIsApplied( true );
33-
34-
var model = action.GetProperty<ApiVersionModel>();
35-
Contract.Assume( model != null );
36-
37-
action.SetProperty( model.Aggregate( matchingActions.Select( a => a.GetProperty<ApiVersionModel>() ).Where( m => m != null ) ) );
38-
}
39-
4014
/// <summary>
4115
/// Returns a value indicating whether the provided action implicitly maps to the specified version.
4216
/// </summary>

Diff for: ‎src/Microsoft.AspNetCore.Mvc.Versioning/Microsoft.Extensions.DependencyInjection/IServiceCollectionExtensions.cs

+2
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using Extensions;
88
using Microsoft.AspNetCore.Builder;
99
using Microsoft.AspNetCore.Hosting;
10+
using Microsoft.AspNetCore.Mvc.Abstractions;
1011
using Options;
1112
using System;
1213
using System.Diagnostics.Contracts;
@@ -54,6 +55,7 @@ public static IServiceCollection AddApiVersioning( this IServiceCollection servi
5455
if ( options.ReportApiVersions )
5556
{
5657
services.TryAddSingleton<IReportApiVersions, DefaultApiVersionReporter>();
58+
services.AddTransient<IActionDescriptorProvider, ApiVersionCollator>();
5759
}
5860
else
5961
{

Diff for: ‎src/Microsoft.AspNetCore.Mvc.Versioning/Routing/DefaultApiVersionRoutePolicy.cs

-2
Original file line numberDiff line numberDiff line change
@@ -164,9 +164,7 @@ protected virtual void OnSingleMatch( RouteContext context, ActionSelectionResul
164164
Arg.NotNull( match, nameof( match ) );
165165

166166
var handler = new DefaultActionHandler( ActionInvokerFactory, ActionContextAccessor, selectionResult, match );
167-
var candidates = selectionResult.CandidateActions.SelectMany( kvp => kvp.Value );
168167

169-
match.Action.AggregateAllVersions( candidates );
170168
context.RouteData = match.RouteData;
171169
context.Handler = handler.Invoke;
172170
}

Diff for: ‎src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersionActionSelector.cs

+2-1
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,7 @@ public virtual ActionDescriptor SelectBestCandidate( RouteContext context, IRead
166166
}
167167
else
168168
{
169+
AppendPossibleMatches( new[] { selectedAction }, context, selectionResult );
169170
return selectedAction;
170171
}
171172
}
@@ -261,7 +262,7 @@ static ActionDescriptor SelectActionWithApiVersionPolicyApplied( IReadOnlyList<A
261262

262263
var match = matches[0];
263264

264-
if ( match.VersionPolicyIsApplied() && !result.HasMatchesInPreviousIterations )
265+
if ( !result.HasMatchesInPreviousIterations )
265266
{
266267
return match;
267268
}

Diff for: ‎src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersionCollator.cs

+134
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
namespace Microsoft.AspNetCore.Mvc.Versioning
2+
{
3+
using Microsoft.AspNetCore.Mvc.Abstractions;
4+
using Microsoft.AspNetCore.Mvc.Controllers;
5+
using System;
6+
using System.Collections.Generic;
7+
using System.Diagnostics.Contracts;
8+
using System.Linq;
9+
10+
/// <summary>
11+
/// Represents an object that collates <see cref="ApiVersion">API versions</see> per <see cref="ActionDescriptor">action</see>.
12+
/// </summary>
13+
[CLSCompliant( false )]
14+
public class ApiVersionCollator : IActionDescriptorProvider
15+
{
16+
/// <inheritdoc />
17+
public int Order { get; protected set; }
18+
19+
/// <inheritdoc />
20+
public virtual void OnProvidersExecuted( ActionDescriptorProviderContext context )
21+
{
22+
foreach ( var actions in GroupActionsByController( context.Results ) )
23+
{
24+
var collatedModel = CollateModel( actions );
25+
26+
foreach ( var action in actions )
27+
{
28+
var model = action.GetProperty<ApiVersionModel>();
29+
30+
if ( model != null )
31+
{
32+
action.SetProperty( model.Aggregate( collatedModel ) );
33+
}
34+
}
35+
}
36+
}
37+
38+
/// <inheritdoc />
39+
public virtual void OnProvidersExecuting( ActionDescriptorProviderContext context )
40+
{
41+
}
42+
43+
/// <summary>
44+
/// Resolves and returns the logical controller name for the specified action.
45+
/// </summary>
46+
/// <param name="action">The <see cref="ActionDescriptor">action</see> to get the controller name from.</param>
47+
/// <returns>The logical name of the associated controller.</returns>
48+
/// <remarks>
49+
/// <para>
50+
/// The logical controller name is used to collate actions together and aggregate API versions. The
51+
/// default implementation uses the "controller" route parameter and falls back to the
52+
/// <see cref="ControllerActionDescriptor.ControllerName"/> property when available.
53+
/// </para>
54+
/// <para>
55+
/// The default implementation will also trim trailing numbers in the controller name by convention. For example,
56+
/// the type "Values2Controller" will have the controller name "Values2", which will be trimmed to just "Values".
57+
/// This behavior can be changed by using the <see cref="ControllerNameAttribute"/> or overriding the default
58+
/// implementation.
59+
/// </para>
60+
/// </remarks>
61+
protected virtual string GetControllerName( ActionDescriptor action )
62+
{
63+
Arg.NotNull( action, nameof( action ) );
64+
65+
if ( !action.RouteValues.TryGetValue( "controller", out var key ) )
66+
{
67+
if ( action is ControllerActionDescriptor controllerAction )
68+
{
69+
key = controllerAction.ControllerName;
70+
}
71+
}
72+
73+
return TrimTrailingNumbers( key );
74+
}
75+
76+
IEnumerable<IEnumerable<ActionDescriptor>> GroupActionsByController( IEnumerable<ActionDescriptor> actions )
77+
{
78+
Contract.Requires( actions != null );
79+
Contract.Ensures( Contract.Result<IEnumerable<IEnumerable<ActionDescriptor>>>() != null );
80+
81+
var groups = new Dictionary<string, List<ActionDescriptor>>( StringComparer.OrdinalIgnoreCase );
82+
83+
foreach ( var action in actions )
84+
{
85+
var key = GetControllerName( action );
86+
87+
if ( string.IsNullOrEmpty( key ) )
88+
{
89+
continue;
90+
}
91+
92+
if ( !groups.TryGetValue( key, out var values ) )
93+
{
94+
groups.Add( key, values = new List<ActionDescriptor>() );
95+
}
96+
97+
values.Add( action );
98+
}
99+
100+
foreach ( var value in groups.Values )
101+
{
102+
yield return value;
103+
}
104+
}
105+
106+
static string TrimTrailingNumbers( string name )
107+
{
108+
if ( string.IsNullOrEmpty( name ) )
109+
{
110+
return name;
111+
}
112+
113+
var last = name.Length - 1;
114+
115+
for ( var i = last; i >= 0; i-- )
116+
{
117+
if ( !char.IsNumber( name[i] ) )
118+
{
119+
if ( i < last )
120+
{
121+
return name.Substring( 0, i + 1 );
122+
}
123+
124+
return name;
125+
}
126+
}
127+
128+
return name;
129+
}
130+
131+
static ApiVersionModel CollateModel( IEnumerable<ActionDescriptor> actions ) =>
132+
actions.Select( a => a.GetProperty<ApiVersionModel>() ).Where( m => m != null ).Aggregate();
133+
}
134+
}

Diff for: ‎test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Versioning/ApiVersionActionSelectorTest.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -452,7 +452,7 @@ public async Task select_best_candidate_should_return_correct_controller_for_ver
452452
var deprecated = new[] { new ApiVersion( 4, 0 ) };
453453
var implemented = supported.Union( deprecated ).OrderBy( v => v ).ToArray();
454454

455-
using ( var server = new WebServer() )
455+
using ( var server = new WebServer( o => o.ReportApiVersions = true ) )
456456
{
457457
await server.Client.GetAsync( $"api/{versionSegment}/attributed" );
458458

Diff for: ‎test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Versioning/ApiVersionCollatorTest.cs

+140
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,140 @@
1+
namespace Microsoft.AspNetCore.Mvc.Versioning
2+
{
3+
using FluentAssertions;
4+
using Microsoft.AspNetCore.Mvc.Abstractions;
5+
using Microsoft.AspNetCore.Mvc.Controllers;
6+
using System.Collections.Generic;
7+
using System.Linq;
8+
using Xunit;
9+
10+
public class ApiVersionCollatorTest
11+
{
12+
[Theory]
13+
[MemberData( nameof( ActionDescriptorProviderContexts ) )]
14+
public void on_providers_executed_should_aggregate_api_version_models_by_controller( ActionDescriptorProviderContext context )
15+
{
16+
// arrange
17+
var collator = new ApiVersionCollator();
18+
var expected = new[] { new ApiVersion( 1, 0 ), new ApiVersion( 2, 0 ), new ApiVersion( 3, 0 ) };
19+
20+
// act
21+
collator.OnProvidersExecuted( context );
22+
23+
// assert
24+
var actions = context.Results.Where( a => a.GetProperty<ApiVersionModel>() != null );
25+
26+
actions.All( a => a.GetProperty<ApiVersionModel>().SupportedApiVersions.SequenceEqual( expected ) ).Should().BeTrue();
27+
}
28+
29+
public static IEnumerable<object[]> ActionDescriptorProviderContexts
30+
{
31+
get
32+
{
33+
yield return new object[] { ActionsWithRouteValues };
34+
yield return new object[] { ActionsByControllerName };
35+
}
36+
}
37+
38+
private static ActionDescriptorProviderContext ActionsWithRouteValues =>
39+
new ActionDescriptorProviderContext()
40+
{
41+
Results =
42+
{
43+
new ActionDescriptor()
44+
{
45+
RouteValues = new Dictionary<string, string>()
46+
{
47+
["controller"] = "Values",
48+
["action"] = "Get",
49+
},
50+
Properties = new Dictionary<object, object>()
51+
{
52+
[typeof(ApiVersionModel)] = new ApiVersionModel( new ApiVersion( 1, 0 ) ),
53+
},
54+
},
55+
new ActionDescriptor()
56+
{
57+
RouteValues = new Dictionary<string, string>()
58+
{
59+
["page"] = "/Some/Page",
60+
},
61+
},
62+
new ActionDescriptor()
63+
{
64+
RouteValues = new Dictionary<string, string>()
65+
{
66+
["controller"] = "Values",
67+
["action"] = "Get",
68+
},
69+
Properties = new Dictionary<object, object>()
70+
{
71+
[typeof(ApiVersionModel)] = new ApiVersionModel( new ApiVersion( 2, 0 ) ),
72+
},
73+
},
74+
new ActionDescriptor()
75+
{
76+
RouteValues = new Dictionary<string, string>()
77+
{
78+
["controller"] = "Values",
79+
["action"] = "Get",
80+
},
81+
Properties = new Dictionary<object, object>()
82+
{
83+
[typeof(ApiVersionModel)] = new ApiVersionModel( new ApiVersion( 3, 0 ) ),
84+
},
85+
},
86+
},
87+
};
88+
89+
private static ActionDescriptorProviderContext ActionsByControllerName =>
90+
new ActionDescriptorProviderContext()
91+
{
92+
Results =
93+
{
94+
new ControllerActionDescriptor()
95+
{
96+
ControllerName = "Values",
97+
RouteValues = new Dictionary<string, string>()
98+
{
99+
["action"] = "Get",
100+
},
101+
Properties = new Dictionary<object, object>()
102+
{
103+
[typeof(ApiVersionModel)] = new ApiVersionModel( new ApiVersion( 1, 0 ) ),
104+
},
105+
},
106+
new ActionDescriptor()
107+
{
108+
RouteValues = new Dictionary<string, string>()
109+
{
110+
["page"] = "/Some/Page",
111+
},
112+
},
113+
new ControllerActionDescriptor()
114+
{
115+
ControllerName = "Values2",
116+
RouteValues = new Dictionary<string, string>()
117+
{
118+
["action"] = "Get",
119+
},
120+
Properties = new Dictionary<object, object>()
121+
{
122+
[typeof(ApiVersionModel)] = new ApiVersionModel( new ApiVersion( 2, 0 ) ),
123+
},
124+
},
125+
new ControllerActionDescriptor()
126+
{
127+
ControllerName = "Values3",
128+
RouteValues = new Dictionary<string, string>()
129+
{
130+
["action"] = "Get",
131+
},
132+
Properties = new Dictionary<object, object>()
133+
{
134+
[typeof(ApiVersionModel)] = new ApiVersionModel( new ApiVersion( 3, 0 ) ),
135+
},
136+
},
137+
},
138+
};
139+
}
140+
}

0 commit comments

Comments
 (0)