Skip to content

Commit

Permalink
Use ApiConventions to solve enum serialization on assembly scope (#784)
Browse files Browse the repository at this point in the history
* new concept

* remove confusing setup in test

* fix warnings

* implement canWriteResult on formatter

* use JsonNumberEnumConverter instead of custom converter

* use altinnapi as jsonsettings name

* remove unused logger

* use api conventions to add JsonSettingsNameAttribute attribute to controllers

* use default json converter

* Use UnsafeRelaxedJsonEscaping, change formatter ordering

* Clarify comments

* add constants for json setting names

* pascalcase for const

* use copy of deafult encoder

* add tests

* add the tests i forgot in the last commit

* use the const instead of magic strings

* cleanup unused usings

* use defaults for serializeroptions

* revert to add custom options

* add tests

* dispose all disposable, even in tests

* add the final using statement

---------

Co-authored-by: Martin Othamar <martin.othamar@digdir.no>
  • Loading branch information
HauklandJ and martinothamar authored Oct 7, 2024
1 parent dca53d4 commit fa0d791
Show file tree
Hide file tree
Showing 12 changed files with 628 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
using Microsoft.AspNetCore.Mvc.Filters;

namespace Altinn.App.Api.Controllers.Attributes;

[AttributeUsage(AttributeTargets.Class)]
internal class JsonSettingsNameAttribute : Attribute, IFilterMetadata
{
internal JsonSettingsNameAttribute(string name)
{
Name = name;
}

internal string Name { get; }
}

internal static class JsonSettingNames
{
internal const string AltinnApi = "AltinnApi";
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
using System.Text.Encodings.Web;
using System.Text.Json;
using Altinn.App.Api.Extensions;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.Formatters;

namespace Altinn.App.Api.Controllers.Conventions;

internal sealed class AltinnApiJsonFormatter : SystemTextJsonOutputFormatter
{
private AltinnApiJsonFormatter(string settingsName, JsonSerializerOptions options)
: base(options)
{
SettingsName = settingsName;
}

internal string SettingsName { get; }

public override bool CanWriteResult(OutputFormatterCanWriteContext context)
{
if (context.HttpContext.GetJsonSettingsName() != SettingsName)
{
return false;
}

return base.CanWriteResult(context);
}

internal static AltinnApiJsonFormatter CreateFormatter(string settingsName, JsonOptions jsonOptions)
{
var jsonSerializerOptions = jsonOptions.JsonSerializerOptions;

if (jsonSerializerOptions.Encoder is null)
{
// If the user hasn't explicitly configured the encoder, use the less strict encoder that does not encode all non-ASCII characters.
jsonSerializerOptions = new JsonSerializerOptions(jsonSerializerOptions)
{
Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping,
};
}

return new AltinnApiJsonFormatter(settingsName, jsonSerializerOptions);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
using Altinn.App.Api.Controllers.Attributes;
using Microsoft.AspNetCore.Mvc.ApplicationModels;

namespace Altinn.App.Api.Controllers.Conventions;

internal class AltinnControllerConventions : IControllerModelConvention
{
public void Apply(ControllerModel controller)
{
controller.Filters.Add(new JsonSettingsNameAttribute(JsonSettingNames.AltinnApi));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.Formatters;
using Microsoft.Extensions.Options;

namespace Altinn.App.Api.Controllers.Conventions;

/// <summary>
/// Configures MVC options to use a specific JSON serialization settings for enum-to-number conversion.
/// </summary>
public class ConfigureMvcJsonOptions : IConfigureOptions<MvcOptions>
{
private readonly string _jsonSettingsName;
private readonly IOptionsMonitor<JsonOptions> _jsonOptions;

/// <summary>
/// Initializes a new instance of the <see cref="ConfigureMvcJsonOptions"/> class.
/// </summary>
/// <param name="jsonSettingsName">The name of the JSON settings to be used for enum-to-number conversion.</param>
/// /// <param name="jsonOptions">An <see cref="IOptionsMonitor{TOptions}"/> to access the named JSON options.</param>
public ConfigureMvcJsonOptions(string jsonSettingsName, IOptionsMonitor<JsonOptions> jsonOptions)
{
_jsonSettingsName = jsonSettingsName;
_jsonOptions = jsonOptions;
}

/// <summary>
/// Configures the MVC options to use the <see cref="AltinnApiJsonFormatter"/> for the specified JSON settings.
/// Makes sure to add to the formatter before the default <see cref="SystemTextJsonOutputFormatter"/> .
/// </summary>
/// <param name="options">The <see cref="MvcOptions"/> to configure.</param>
public void Configure(MvcOptions options)
{
var defaultJsonFormatter =
options.OutputFormatters.OfType<SystemTextJsonOutputFormatter>().FirstOrDefault()
?? throw new InvalidOperationException("Could not find the default JSON output formatter");

var indexOfDefaultJsonFormatter = options.OutputFormatters.IndexOf(defaultJsonFormatter);

var jsonOptions = _jsonOptions.Get(_jsonSettingsName);
options.OutputFormatters.Insert(
indexOfDefaultJsonFormatter,
AltinnApiJsonFormatter.CreateFormatter(_jsonSettingsName, jsonOptions)
);
}
}
11 changes: 11 additions & 0 deletions src/Altinn.App.Api/Extensions/HttpContextExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
using Altinn.App.Api.Controllers.Attributes;

namespace Altinn.App.Api.Extensions;

internal static class HttpContextExtensions
{
internal static string? GetJsonSettingsName(this HttpContext context)
{
return context.GetEndpoint()?.Metadata.GetMetadata<JsonSettingsNameAttribute>()?.Name;
}
}
28 changes: 28 additions & 0 deletions src/Altinn.App.Api/Extensions/MvcBuilderExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
using Altinn.App.Api.Controllers.Conventions;
using Microsoft.AspNetCore.Mvc;
using Microsoft.Extensions.Options;

namespace Altinn.App.Api.Extensions;

internal static class MvcBuilderExtensions
{
internal static IMvcBuilder AddJsonOptions(
this IMvcBuilder builder,
string settingsName,
Action<JsonOptions> configure
)
{
ArgumentNullException.ThrowIfNull(builder);
ArgumentNullException.ThrowIfNull(configure);

builder.Services.Configure(settingsName, configure);

builder.Services.AddSingleton<IConfigureOptions<MvcOptions>>(sp =>
{
var options = sp.GetRequiredService<IOptionsMonitor<JsonOptions>>();
return new ConfigureMvcJsonOptions(settingsName, options);
});

return builder;
}
}
10 changes: 10 additions & 0 deletions src/Altinn.App.Api/Extensions/ServiceCollectionExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
using System.Diagnostics;
using Altinn.App.Api.Controllers;
using Altinn.App.Api.Controllers.Attributes;
using Altinn.App.Api.Controllers.Conventions;
using Altinn.App.Api.Helpers;
using Altinn.App.Api.Infrastructure.Filters;
using Altinn.App.Api.Infrastructure.Health;
Expand Down Expand Up @@ -43,10 +45,18 @@ public static void AddAltinnAppControllersWithViews(this IServiceCollection serv
IMvcBuilder mvcBuilder = services.AddControllersWithViews(options =>
{
options.Filters.Add<TelemetryEnrichingResultFilter>();
options.Conventions.Add(new AltinnControllerConventions());
});
mvcBuilder
.AddApplicationPart(typeof(InstancesController).Assembly)
.AddXmlSerializerFormatters()
.AddJsonOptions(
JsonSettingNames.AltinnApi,
options =>
{
options.JsonSerializerOptions.PropertyNamingPolicy = System.Text.Json.JsonNamingPolicy.CamelCase;
}
)
.AddJsonOptions(options =>
{
options.JsonSerializerOptions.PropertyNamingPolicy = System.Text.Json.JsonNamingPolicy.CamelCase;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
using System.Text.Encodings.Web;
using System.Text.Json.Serialization.Metadata;
using Altinn.App.Api.Controllers.Attributes;
using Altinn.App.Api.Controllers.Conventions;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.Formatters;

namespace Altinn.App.Api.Tests.Controllers.Conventions;

public class AltinnApiJsonFormatterTests
{
[Fact]
public void CreateFormatter_WhenEncoderIsNotNull_PreservesEncoder()
{
// Arrange
string settingsName = JsonSettingNames.AltinnApi;
var originalEncoder = JavaScriptEncoder.Default;

var jsonOptions = new JsonOptions();
jsonOptions.JsonSerializerOptions.Encoder = originalEncoder;
jsonOptions.JsonSerializerOptions.TypeInfoResolver = new DefaultJsonTypeInfoResolver();

// Act
var formatter = AltinnApiJsonFormatter.CreateFormatter(settingsName, jsonOptions);

// Assert
Assert.NotNull(formatter);
Assert.Equal(settingsName, formatter.SettingsName);
Assert.Equal(originalEncoder, formatter.SerializerOptions.Encoder);
}

[Fact]
public void CanWriteResult_SettingsNameMatches_ReturnsTrue()
{
// Arrange
string settingsName = JsonSettingNames.AltinnApi;

var jsonOptions = new JsonOptions();
jsonOptions.JsonSerializerOptions.TypeInfoResolver = new DefaultJsonTypeInfoResolver();

var formatter = AltinnApiJsonFormatter.CreateFormatter(settingsName, jsonOptions);

var httpContext = new DefaultHttpContext();

// Create an Endpoint with JsonSettingsNameAttribute
var endpoint = new Endpoint(
requestDelegate: null,
metadata: new EndpointMetadataCollection(new JsonSettingsNameAttribute(settingsName)),
displayName: null
);

httpContext.SetEndpoint(endpoint);

var context = new OutputFormatterWriteContext(
httpContext,
(stream, encoding) => new StreamWriter(stream, encoding),
typeof(object),
new object()
);

// Act
bool canWrite = formatter.CanWriteResult(context);

// Assert
Assert.True(canWrite);
}

[Fact]
public void CanWriteResult_SettingsNameMisMatch_ReturnsFalse()
{
// Arrange
string formatterSettingsName = "FormatterSettingName";
string endpointSettingsName = "EndpointSettingName";

var jsonOptions = new JsonOptions();
jsonOptions.JsonSerializerOptions.TypeInfoResolver = new DefaultJsonTypeInfoResolver();

var formatter = AltinnApiJsonFormatter.CreateFormatter(formatterSettingsName, jsonOptions);

var httpContext = new DefaultHttpContext();

// Create an Endpoint with JsonSettingsNameAttribute with a different name
var endpoint = new Endpoint(
requestDelegate: null,
metadata: new EndpointMetadataCollection(new JsonSettingsNameAttribute(endpointSettingsName)),
displayName: null
);

httpContext.SetEndpoint(endpoint);

var context = new OutputFormatterWriteContext(
httpContext,
(stream, encoding) => new StreamWriter(stream, encoding),
typeof(object),
new object()
);

// Act
bool canWrite = formatter.CanWriteResult(context);

// Assert
Assert.False(canWrite);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
using System.Reflection;
using Altinn.App.Api.Controllers.Attributes;
using Altinn.App.Api.Controllers.Conventions;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.ApplicationModels;

namespace Altinn.App.Api.Tests.Controllers.Conventions;

public class AltinnControllerConventionsTests
{
[Fact]
public void Apply_AddsJsonSettingsNameAttributeToControllerModel()
{
// Arrange
var convention = new AltinnControllerConventions();
var controllerType = typeof(TestController).GetTypeInfo();
var controllerModel = new ControllerModel(controllerType, []);

// Act
convention.Apply(controllerModel);

// Assert
var attribute = controllerModel.Filters.OfType<JsonSettingsNameAttribute>().FirstOrDefault();

Assert.NotNull(attribute);
Assert.Equal(JsonSettingNames.AltinnApi, attribute.Name);
}

// Dummy controller
private class TestController : ControllerBase { }
}
Loading

0 comments on commit fa0d791

Please sign in to comment.