Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

new concept #784

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
namespace Altinn.App.Api.Controllers.Attributes;

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

internal string Name { get; }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
using Microsoft.AspNetCore.Mvc;
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;
private readonly ILoggerFactory _loggerFactory;

/// <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>
/// <param name="loggerFactory">A factory for creating loggers.</param>
public ConfigureMvcJsonOptions(
string jsonSettingsName,
IOptionsMonitor<JsonOptions> jsonOptions,
ILoggerFactory loggerFactory
)
{
_jsonSettingsName = jsonSettingsName;
_jsonOptions = jsonOptions;
_loggerFactory = loggerFactory;
}

/// <summary>
/// Configures the MVC options to use the EnumAsNumberFormatter for the specified JSON settings.
/// </summary>
/// <param name="options">The <see cref="MvcOptions"/> to configure.</param>
public void Configure(MvcOptions options)
{
var jsonOptions = _jsonOptions.Get(_jsonSettingsName);
var logger = _loggerFactory.CreateLogger<EnumAsNumberFormatter>();

Check warning

Code scanning / CodeQL

Useless assignment to local variable Warning

This assignment to
logger
is useless, since its value is never read.
options.OutputFormatters.Insert(0, new EnumAsNumberFormatter(_jsonSettingsName, jsonOptions));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
using System.Text.Json;
using System.Text.Json.Serialization;
using Altinn.App.Api.Extensions;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.Formatters;

namespace Altinn.App.Api.Controllers.Conventions;

internal class EnumAsNumberFormatter : SystemTextJsonOutputFormatter
martinothamar marked this conversation as resolved.
Show resolved Hide resolved
{
internal EnumAsNumberFormatter(string settingsName, JsonOptions options)
: base(CreateSerializerOptions(options))
{
SettingsName = settingsName;
}

internal string SettingsName { get; }

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

return base.CanWriteResult(context);
}

private static JsonSerializerOptions CreateSerializerOptions(JsonOptions options)
{
var newOptions = new JsonSerializerOptions(options.JsonSerializerOptions);
newOptions.Converters.Add(new EnumToNumberJsonConverterFactory());
return newOptions;
}
}

internal class EnumToNumberJsonConverterFactory : JsonConverterFactory
{
public override bool CanConvert(Type typeToConvert)
{
return typeToConvert.IsEnum;
}

public override JsonConverter CreateConverter(Type typeToConvert, JsonSerializerOptions options)
{
var instance =
Activator.CreateInstance(typeof(EnumToNumberJsonConverter<>).MakeGenericType(typeToConvert))
?? throw new InvalidOperationException($"Failed to create converter for type {typeToConvert.FullName}");
return (JsonConverter)instance;
}
}

internal class EnumToNumberJsonConverter<TEnum> : JsonConverter<TEnum>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need a custom converter? Isn't number encoding the default?

where TEnum : struct, Enum
{
public override TEnum Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
if (reader.TokenType == JsonTokenType.Number)
{
var value = reader.GetInt32();
return (TEnum)Enum.ToObject(typeToConvert, value);
}

throw new JsonException($"Unexpected token type: {reader.TokenType}");
}

public override void Write(Utf8JsonWriter writer, TEnum value, JsonSerializerOptions options)
{
writer.WriteNumberValue(Convert.ToInt32(value, System.Globalization.CultureInfo.InvariantCulture));
}
}
2 changes: 2 additions & 0 deletions src/Altinn.App.Api/Controllers/PartiesController.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Globalization;
using Altinn.App.Api.Controllers.Attributes;
using Altinn.App.Core.Configuration;
using Altinn.App.Core.Helpers;
using Altinn.App.Core.Internal.App;
Expand All @@ -21,6 +22,7 @@ namespace Altinn.App.Api.Controllers;
/// </summary>
[Authorize]
[ApiController]
[JsonSettingsName("EnumAsNumber")]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the setting name should just be AltinnApis or something? Denoting that whatever settings are injected are what is required to make our own APIs stable?

Now maybe we should consider API conventions as well so that we don't have to remember this attribute everywhere?

public class PartiesController : ControllerBase
{
private readonly IAuthorizationClient _authorizationClient;
Expand Down
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;
}
}
29 changes: 29 additions & 0 deletions src/Altinn.App.Api/Extensions/MvcBuilderExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
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>>();
var loggerFactory = sp.GetRequiredService<ILoggerFactory>();
return new ConfigureMvcJsonOptions(settingsName, options, loggerFactory);
});

return builder;
}
}
9 changes: 9 additions & 0 deletions src/Altinn.App.Api/Extensions/ServiceCollectionExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System.Diagnostics;
using Altinn.App.Api.Controllers;
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 @@ -47,6 +48,14 @@ public static void AddAltinnAppControllersWithViews(this IServiceCollection serv
mvcBuilder
.AddApplicationPart(typeof(InstancesController).Assembly)
.AddXmlSerializerFormatters()
.AddJsonOptions(
"EnumAsNumber",
options =>
{
options.JsonSerializerOptions.Converters.Add(new EnumToNumberJsonConverterFactory());
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,93 @@
using System.Net;
using System.Text.Json;
using System.Text.Json.Serialization;
using Altinn.App.Api.Controllers.Conventions;
using Altinn.App.Api.Extensions;
using Altinn.App.Core.Internal.App;
using Altinn.App.Core.Internal.Auth;
using Altinn.App.Core.Models;
using Altinn.Platform.Register.Enums;
using Altinn.Platform.Storage.Interface.Models;
using FluentAssertions;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.Testing;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Moq;
using Xunit.Abstractions;

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

public class EnumSerializationTests : ApiTestBase, IClassFixture<WebApplicationFactory<Program>>
{
private const string Org = "tdd";
private const string App = "contributer-restriction";
private const int PartyId = 500600;

private readonly Mock<IAuthorizationClient> _authorizationClientMock;
private readonly Mock<IAppMetadata> _appMetadataMock;

public EnumSerializationTests(WebApplicationFactory<Program> factory, ITestOutputHelper outputHelper)
: base(factory, outputHelper)
{
// Mock auth client to return the enum we want to test
_authorizationClientMock = new Mock<IAuthorizationClient>();
_authorizationClientMock
.Setup(a => a.GetPartyList(It.IsAny<int>()))
.ReturnsAsync([new() { PartyTypeName = PartyType.Person }]);

_appMetadataMock = new Mock<IAppMetadata>();
_appMetadataMock
.Setup(s => s.GetApplicationMetadata())
.ReturnsAsync(
new ApplicationMetadata(id: "ttd/test") { PartyTypesAllowed = new PartyTypesAllowed { Person = true } }
);

OverrideServicesForAllTests = (services) =>
{
services
.AddControllers()
.AddJsonOptions(options =>
{
options.JsonSerializerOptions.Converters.Add(new JsonStringEnumConverter());
});

services.AddSingleton<IConfigureOptions<MvcOptions>>(sp =>
{
var options = sp.GetRequiredService<IOptionsMonitor<JsonOptions>>();
var loggerFactory = sp.GetRequiredService<ILoggerFactory>();
return new ConfigureMvcJsonOptions("EnumAsNumber", options, loggerFactory);
});

services.AddScoped(_ => _authorizationClientMock.Object);
services.AddScoped(_ => _appMetadataMock.Object);
};
}

[Fact]
public async Task ValidateInstantiation_SerializesPartyTypesAllowedAsNumber()
{
// Arrange
using var client = GetRootedClient(Org, App, 1337, PartyId);

// Act
var response = await client.PostAsync(
$"{Org}/{App}/api/v1/parties/validateInstantiation?partyId={PartyId}",
null
);
response.Should().HaveStatusCode(HttpStatusCode.OK);
var content = await response.Content.ReadAsStringAsync();
var partyTypeEnumJson = JsonDocument
.Parse(content)
.RootElement.GetProperty("validParties")
.EnumerateArray()
.First()
.GetProperty("partyTypeName");

// Assert
partyTypeEnumJson.Should().NotBeNull();
partyTypeEnumJson.TryGetInt32(out var partyTypeJsonValue);
partyTypeJsonValue.Should().Be(1, "PartyTypesAllowed should be serialized as its numeric value");
}
}
Loading