Skip to content

Commit f4980c3

Browse files
bkoelmanbkoelman
authored
OpenAPI tweaks and fixes (#1748)
* Improve test coverage * Remove the need for a workaround for "dotnet swagger" (swashbuckle.aspnetcore.cli global tool) * Fix thread safety when OpenAPI document is downloaded in parallel
1 parent 58cc1cb commit f4980c3

13 files changed

+263
-80
lines changed

‎src/JsonApiDotNetCore.OpenApi.Swashbuckle/ConfigureMvcOptions.cs

Lines changed: 4 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,45 +1,29 @@
11
using JsonApiDotNetCore.Configuration;
2-
using JsonApiDotNetCore.Middleware;
32
using Microsoft.AspNetCore.Mvc;
43
using Microsoft.Extensions.Options;
54

65
namespace JsonApiDotNetCore.OpenApi.Swashbuckle;
76

87
internal sealed class ConfigureMvcOptions : IConfigureOptions<MvcOptions>
98
{
10-
private readonly IJsonApiRoutingConvention _jsonApiRoutingConvention;
119
private readonly JsonApiRequestFormatMetadataProvider _jsonApiRequestFormatMetadataProvider;
12-
private readonly IJsonApiOptions _jsonApiOptions;
10+
private readonly JsonApiOptions _jsonApiOptions;
1311

14-
public ConfigureMvcOptions(IJsonApiRoutingConvention jsonApiRoutingConvention, JsonApiRequestFormatMetadataProvider jsonApiRequestFormatMetadataProvider,
15-
IJsonApiOptions jsonApiOptions)
12+
public ConfigureMvcOptions(JsonApiRequestFormatMetadataProvider jsonApiRequestFormatMetadataProvider, IJsonApiOptions jsonApiOptions)
1613
{
17-
ArgumentNullException.ThrowIfNull(jsonApiRoutingConvention);
1814
ArgumentNullException.ThrowIfNull(jsonApiRequestFormatMetadataProvider);
1915
ArgumentNullException.ThrowIfNull(jsonApiOptions);
2016

21-
_jsonApiRoutingConvention = jsonApiRoutingConvention;
2217
_jsonApiRequestFormatMetadataProvider = jsonApiRequestFormatMetadataProvider;
23-
_jsonApiOptions = jsonApiOptions;
18+
_jsonApiOptions = (JsonApiOptions)jsonApiOptions;
2419
}
2520

2621
public void Configure(MvcOptions options)
2722
{
2823
ArgumentNullException.ThrowIfNull(options);
2924

30-
AddSwashbuckleCliCompatibility(options);
31-
3225
options.InputFormatters.Add(_jsonApiRequestFormatMetadataProvider);
3326

34-
((JsonApiOptions)_jsonApiOptions).IncludeExtensions(OpenApiMediaTypeExtension.OpenApi, OpenApiMediaTypeExtension.RelaxedOpenApi);
35-
}
36-
37-
private void AddSwashbuckleCliCompatibility(MvcOptions options)
38-
{
39-
if (!options.Conventions.Any(convention => convention is IJsonApiRoutingConvention))
40-
{
41-
// See https://github.com/domaindrivendev/Swashbuckle.AspNetCore/issues/1957 for why this is needed.
42-
options.Conventions.Insert(0, _jsonApiRoutingConvention);
43-
}
27+
_jsonApiOptions.IncludeExtensions(OpenApiMediaTypeExtension.OpenApi, OpenApiMediaTypeExtension.RelaxedOpenApi);
4428
}
4529
}

‎src/JsonApiDotNetCore.OpenApi.Swashbuckle/JsonApiActionDescriptorCollectionProvider.cs

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System.Collections.Concurrent;
12
using System.Net;
23
using System.Reflection;
34
using JsonApiDotNetCore.Configuration;
@@ -36,8 +37,10 @@ internal sealed partial class JsonApiActionDescriptorCollectionProvider : IActio
3637
private readonly JsonApiEndpointMetadataProvider _jsonApiEndpointMetadataProvider;
3738
private readonly IJsonApiOptions _options;
3839
private readonly ILogger<JsonApiActionDescriptorCollectionProvider> _logger;
40+
private readonly ConcurrentDictionary<int, Lazy<ActionDescriptorCollection>> _versionedActionDescriptorCache = new();
3941

40-
public ActionDescriptorCollection ActionDescriptors => GetActionDescriptors();
42+
public ActionDescriptorCollection ActionDescriptors =>
43+
_versionedActionDescriptorCache.GetOrAdd(_defaultProvider.ActionDescriptors.Version, LazyGetActionDescriptors).Value;
4144

4245
public JsonApiActionDescriptorCollectionProvider(IActionDescriptorCollectionProvider defaultProvider, IControllerResourceMapping controllerResourceMapping,
4346
JsonApiEndpointMetadataProvider jsonApiEndpointMetadataProvider, IJsonApiOptions options, ILogger<JsonApiActionDescriptorCollectionProvider> logger)
@@ -55,7 +58,13 @@ public JsonApiActionDescriptorCollectionProvider(IActionDescriptorCollectionProv
5558
_logger = logger;
5659
}
5760

58-
private ActionDescriptorCollection GetActionDescriptors()
61+
private Lazy<ActionDescriptorCollection> LazyGetActionDescriptors(int version)
62+
{
63+
// https://andrewlock.net/making-getoradd-on-concurrentdictionary-thread-safe-using-lazy/
64+
return new Lazy<ActionDescriptorCollection>(() => GetActionDescriptors(version), LazyThreadSafetyMode.ExecutionAndPublication);
65+
}
66+
67+
private ActionDescriptorCollection GetActionDescriptors(int version)
5968
{
6069
List<ActionDescriptor> descriptors = [];
6170

@@ -106,8 +115,7 @@ private ActionDescriptorCollection GetActionDescriptors()
106115
descriptors.Add(descriptor);
107116
}
108117

109-
int descriptorVersion = _defaultProvider.ActionDescriptors.Version;
110-
return new ActionDescriptorCollection(descriptors.AsReadOnly(), descriptorVersion);
118+
return new ActionDescriptorCollection(descriptors.AsReadOnly(), version);
111119
}
112120

113121
internal static bool IsVisibleEndpoint(ActionDescriptor descriptor)
@@ -221,9 +229,9 @@ private ActionDescriptor[] SetEndpointMetadata(ActionDescriptor descriptor, Buil
221229
{
222230
Dictionary<RelationshipAttribute, ActionDescriptor> descriptorsByRelationship = [];
223231

224-
JsonApiEndpointMetadata? endpointMetadata = _jsonApiEndpointMetadataProvider.Get(descriptor);
232+
JsonApiEndpointMetadata endpointMetadata = _jsonApiEndpointMetadataProvider.Get(descriptor);
225233

226-
switch (endpointMetadata?.RequestMetadata)
234+
switch (endpointMetadata.RequestMetadata)
227235
{
228236
case AtomicOperationsRequestMetadata atomicOperationsRequestMetadata:
229237
{
@@ -259,7 +267,7 @@ private ActionDescriptor[] SetEndpointMetadata(ActionDescriptor descriptor, Buil
259267
}
260268
}
261269

262-
switch (endpointMetadata?.ResponseMetadata)
270+
switch (endpointMetadata.ResponseMetadata)
263271
{
264272
case AtomicOperationsResponseMetadata atomicOperationsResponseMetadata:
265273
{

‎src/JsonApiDotNetCore.OpenApi.Swashbuckle/JsonApiMetadata/JsonApiEndpointMetadataProvider.cs

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,17 +26,19 @@ public JsonApiEndpointMetadataProvider(IControllerResourceMapping controllerReso
2626
_nonPrimaryDocumentTypeFactory = nonPrimaryDocumentTypeFactory;
2727
}
2828

29-
public JsonApiEndpointMetadata? Get(ActionDescriptor descriptor)
29+
public JsonApiEndpointMetadata Get(ActionDescriptor descriptor)
3030
{
3131
ArgumentNullException.ThrowIfNull(descriptor);
3232

3333
var actionMethod = OpenApiActionMethod.Create(descriptor);
34+
JsonApiEndpointMetadata? metadata = null;
3435

3536
switch (actionMethod)
3637
{
3738
case AtomicOperationsActionMethod:
3839
{
39-
return new JsonApiEndpointMetadata(AtomicOperationsRequestMetadata.Instance, AtomicOperationsResponseMetadata.Instance);
40+
metadata = new JsonApiEndpointMetadata(AtomicOperationsRequestMetadata.Instance, AtomicOperationsResponseMetadata.Instance);
41+
break;
4042
}
4143
case JsonApiActionMethod jsonApiActionMethod:
4244
{
@@ -45,13 +47,13 @@ public JsonApiEndpointMetadataProvider(IControllerResourceMapping controllerReso
4547

4648
IJsonApiRequestMetadata? requestMetadata = GetRequestMetadata(jsonApiActionMethod.Endpoint, primaryResourceType);
4749
IJsonApiResponseMetadata? responseMetadata = GetResponseMetadata(jsonApiActionMethod.Endpoint, primaryResourceType);
48-
return new JsonApiEndpointMetadata(requestMetadata, responseMetadata);
49-
}
50-
default:
51-
{
52-
return null;
50+
metadata = new JsonApiEndpointMetadata(requestMetadata, responseMetadata);
51+
break;
5352
}
5453
}
54+
55+
ConsistencyGuard.ThrowIf(metadata == null);
56+
return metadata;
5557
}
5658

5759
private IJsonApiRequestMetadata? GetRequestMetadata(JsonApiEndpoints endpoint, ResourceType primaryResourceType)

‎src/JsonApiDotNetCore.OpenApi.Swashbuckle/JsonApiRequestFormatMetadataProvider.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System.Diagnostics;
2+
using System.Diagnostics.CodeAnalysis;
23
using Microsoft.AspNetCore.Mvc.ApiExplorer;
34
using Microsoft.AspNetCore.Mvc.Formatters;
45

@@ -10,12 +11,14 @@ namespace JsonApiDotNetCore.OpenApi.Swashbuckle;
1011
internal sealed class JsonApiRequestFormatMetadataProvider : IInputFormatter, IApiRequestFormatMetadataProvider
1112
{
1213
/// <inheritdoc />
14+
[ExcludeFromCodeCoverage]
1315
public bool CanRead(InputFormatterContext context)
1416
{
1517
return false;
1618
}
1719

1820
/// <inheritdoc />
21+
[ExcludeFromCodeCoverage]
1922
public Task<InputFormatterResult> ReadAsync(InputFormatterContext context)
2023
{
2124
throw new UnreachableException();

‎src/JsonApiDotNetCore.OpenApi.Swashbuckle/SchemaGenerationTracer.cs

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System.Runtime.CompilerServices;
12
using JsonApiDotNetCore.Resources.Annotations;
23
using JsonApiDotNetCore.Serialization.Objects;
34
using Microsoft.Extensions.Logging;
@@ -87,7 +88,7 @@ private static string GetSchemaTypeName(Type type)
8788

8889
private sealed partial class SchemaGenerationTraceScope : ISchemaGenerationTraceScope
8990
{
90-
private static readonly AsyncLocal<int> RecursionDepthAsyncLocal = new();
91+
private static readonly AsyncLocal<StrongBox<int>> RecursionDepthAsyncLocal = new();
9192

9293
private readonly ILogger _logger;
9394
private readonly string _schemaTypeName;
@@ -101,8 +102,10 @@ public SchemaGenerationTraceScope(ILogger logger, string schemaTypeName)
101102
_logger = logger;
102103
_schemaTypeName = schemaTypeName;
103104

104-
RecursionDepthAsyncLocal.Value++;
105-
LogStarted(RecursionDepthAsyncLocal.Value, _schemaTypeName);
105+
RecursionDepthAsyncLocal.Value ??= new StrongBox<int>(0);
106+
int depth = Interlocked.Increment(ref RecursionDepthAsyncLocal.Value.Value);
107+
108+
LogStarted(depth, _schemaTypeName);
106109
}
107110

108111
public void TraceSucceeded(string schemaId)
@@ -112,16 +115,18 @@ public void TraceSucceeded(string schemaId)
112115

113116
public void Dispose()
114117
{
118+
int depth = RecursionDepthAsyncLocal.Value!.Value;
119+
115120
if (_schemaId != null)
116121
{
117-
LogSucceeded(RecursionDepthAsyncLocal.Value, _schemaTypeName, _schemaId);
122+
LogSucceeded(depth, _schemaTypeName, _schemaId);
118123
}
119124
else
120125
{
121-
LogFailed(RecursionDepthAsyncLocal.Value, _schemaTypeName);
126+
LogFailed(depth, _schemaTypeName);
122127
}
123128

124-
RecursionDepthAsyncLocal.Value--;
129+
Interlocked.Decrement(ref RecursionDepthAsyncLocal.Value.Value);
125130
}
126131

127132
[LoggerMessage(Level = LogLevel.Trace, SkipEnabledCheck = true, Message = "({Depth:D2}) Started for {SchemaTypeName}.")]

‎src/JsonApiDotNetCore.OpenApi.Swashbuckle/ServiceCollectionExtensions.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ private static void AddCustomApiExplorer(IServiceCollection services)
6262

6363
AddApiExplorer(services);
6464

65-
services.AddSingleton<IConfigureOptions<MvcOptions>, ConfigureMvcOptions>();
65+
services.TryAddEnumerable(ServiceDescriptor.Singleton<IConfigureOptions<MvcOptions>, ConfigureMvcOptions>());
6666
}
6767

6868
private static void AddApiExplorer(IServiceCollection services)

‎src/JsonApiDotNetCore/Configuration/ApplicationBuilderExtensions.cs

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -33,20 +33,6 @@ public static void UseJsonApi(this IApplicationBuilder builder)
3333
inverseNavigationResolver.Resolve();
3434
}
3535

36-
var jsonApiApplicationBuilder = builder.ApplicationServices.GetRequiredService<IJsonApiApplicationBuilder>();
37-
38-
jsonApiApplicationBuilder.ConfigureMvcOptions = options =>
39-
{
40-
var inputFormatter = builder.ApplicationServices.GetRequiredService<IJsonApiInputFormatter>();
41-
options.InputFormatters.Insert(0, inputFormatter);
42-
43-
var outputFormatter = builder.ApplicationServices.GetRequiredService<IJsonApiOutputFormatter>();
44-
options.OutputFormatters.Insert(0, outputFormatter);
45-
46-
var routingConvention = builder.ApplicationServices.GetRequiredService<IJsonApiRoutingConvention>();
47-
options.Conventions.Insert(0, routingConvention);
48-
};
49-
5036
builder.UseMiddleware<JsonApiMiddleware>();
5137
}
5238

‎src/JsonApiDotNetCore/Configuration/ConfigureMvcOptions.cs

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
using JsonApiDotNetCore.Middleware;
2+
using Microsoft.AspNetCore.Mvc;
3+
using Microsoft.Extensions.Options;
4+
5+
namespace JsonApiDotNetCore.Configuration;
6+
7+
internal sealed class ConfigureMvcOptions : IConfigureOptions<MvcOptions>
8+
{
9+
private readonly IJsonApiInputFormatter _inputFormatter;
10+
private readonly IJsonApiOutputFormatter _outputFormatter;
11+
private readonly IJsonApiRoutingConvention _routingConvention;
12+
13+
public ConfigureMvcOptions(IJsonApiInputFormatter inputFormatter, IJsonApiOutputFormatter outputFormatter, IJsonApiRoutingConvention routingConvention)
14+
{
15+
ArgumentNullException.ThrowIfNull(inputFormatter);
16+
ArgumentNullException.ThrowIfNull(outputFormatter);
17+
ArgumentNullException.ThrowIfNull(routingConvention);
18+
19+
_inputFormatter = inputFormatter;
20+
_outputFormatter = outputFormatter;
21+
_routingConvention = routingConvention;
22+
}
23+
24+
public void Configure(MvcOptions options)
25+
{
26+
ArgumentNullException.ThrowIfNull(options);
27+
28+
options.EnableEndpointRouting = true;
29+
30+
options.InputFormatters.Insert(0, _inputFormatter);
31+
options.OutputFormatters.Insert(0, _outputFormatter);
32+
options.Conventions.Insert(0, _routingConvention);
33+
34+
options.Filters.AddService<IAsyncJsonApiExceptionFilter>();
35+
options.Filters.AddService<IAsyncQueryStringActionFilter>();
36+
options.Filters.AddService<IAsyncConvertEmptyActionResultFilter>();
37+
}
38+
}

‎src/JsonApiDotNetCore/Configuration/IJsonApiApplicationBuilder.cs

Lines changed: 0 additions & 8 deletions
This file was deleted.

‎src/JsonApiDotNetCore/Configuration/JsonApiApplicationBuilder.cs

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,22 +18,21 @@
1818
using Microsoft.Extensions.DependencyInjection;
1919
using Microsoft.Extensions.DependencyInjection.Extensions;
2020
using Microsoft.Extensions.Logging;
21+
using Microsoft.Extensions.Options;
2122

2223
namespace JsonApiDotNetCore.Configuration;
2324

2425
/// <summary>
2526
/// A utility class that builds a JSON:API application. It registers all required services and allows the user to override parts of the startup
2627
/// configuration.
2728
/// </summary>
28-
internal sealed class JsonApiApplicationBuilder : IJsonApiApplicationBuilder
29+
internal sealed class JsonApiApplicationBuilder
2930
{
3031
private readonly IServiceCollection _services;
3132
private readonly IMvcCoreBuilder _mvcBuilder;
3233
private readonly JsonApiOptions _options = new();
3334
private readonly ResourceDescriptorAssemblyCache _assemblyCache = new();
3435

35-
public Action<MvcOptions>? ConfigureMvcOptions { get; set; }
36-
3736
public JsonApiApplicationBuilder(IServiceCollection services, IMvcCoreBuilder mvcBuilder)
3837
{
3938
ArgumentNullException.ThrowIfNull(services);
@@ -105,15 +104,6 @@ public void ConfigureResourceGraph(ICollection<Type> dbContextTypes, Action<Reso
105104
/// </summary>
106105
public void ConfigureMvc()
107106
{
108-
_mvcBuilder.AddMvcOptions(options =>
109-
{
110-
options.EnableEndpointRouting = true;
111-
options.Filters.AddService<IAsyncJsonApiExceptionFilter>();
112-
options.Filters.AddService<IAsyncQueryStringActionFilter>();
113-
options.Filters.AddService<IAsyncConvertEmptyActionResultFilter>();
114-
ConfigureMvcOptions?.Invoke(options);
115-
});
116-
117107
if (_options.ValidateModelState)
118108
{
119109
_mvcBuilder.AddDataAnnotations();
@@ -175,14 +165,14 @@ public void ConfigureServiceContainer(ICollection<Type> dbContextTypes)
175165
private void AddMiddlewareLayer()
176166
{
177167
_services.TryAddSingleton<IJsonApiOptions>(_options);
178-
_services.TryAddSingleton<IJsonApiApplicationBuilder>(this);
179168
_services.TryAddSingleton<IExceptionHandler, ExceptionHandler>();
180169
_services.TryAddScoped<IAsyncJsonApiExceptionFilter, AsyncJsonApiExceptionFilter>();
181170
_services.TryAddScoped<IAsyncQueryStringActionFilter, AsyncQueryStringActionFilter>();
182171
_services.TryAddScoped<IAsyncConvertEmptyActionResultFilter, AsyncConvertEmptyActionResultFilter>();
183172
_services.TryAddSingleton<IJsonApiInputFormatter, JsonApiInputFormatter>();
184173
_services.TryAddSingleton<IJsonApiOutputFormatter, JsonApiOutputFormatter>();
185174
_services.TryAddSingleton<IJsonApiRoutingConvention, JsonApiRoutingConvention>();
175+
_services.TryAddEnumerable(ServiceDescriptor.Singleton<IConfigureOptions<MvcOptions>, ConfigureMvcOptions>());
186176
_services.TryAddSingleton<IControllerResourceMapping>(provider => provider.GetRequiredService<IJsonApiRoutingConvention>());
187177
_services.TryAddSingleton<IJsonApiEndpointFilter, AlwaysEnabledJsonApiEndpointFilter>();
188178
_services.TryAddSingleton<IHttpContextAccessor, HttpContextAccessor>();

0 commit comments

Comments
 (0)