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

fix: Add custom attribute for OptionValue and OptionLabel and allow empty string #14035

Merged
Show file tree
Hide file tree
Changes from 2 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,25 @@
using System;

namespace Altinn.Studio.Designer.Exceptions.Options;

/// <summary>
/// Indicates that an error occurred during json serialization of options.
/// </summary>
[Serializable]
public class InvalidOptionsFormatException : Exception
{
/// <inheritdoc/>
public InvalidOptionsFormatException()
{
}

/// <inheritdoc/>
public InvalidOptionsFormatException(string message) : base(message)
{
}

/// <inheritdoc/>
public InvalidOptionsFormatException(string message, Exception innerException) : base(message, innerException)
{
}
}
6 changes: 6 additions & 0 deletions backend/src/Designer/Filters/Options/OptionsErrorCodes.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
namespace Altinn.Studio.Designer.Filters.Options;

public class OptionsErrorCodes
{
public const string InvalidOptionsFormat = nameof(InvalidOptionsFormat);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
using System.Net;
using Altinn.Studio.Designer.Exceptions.Options;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.Controllers;
using Microsoft.AspNetCore.Mvc.Filters;

namespace Altinn.Studio.Designer.Filters.Options;

public class OptionsExceptionFilterAttribute : ExceptionFilterAttribute
{
public override void OnException(ExceptionContext context)
{
base.OnException(context);

if (context.ActionDescriptor is not ControllerActionDescriptor)
{
return;
}

if (context.Exception is InvalidOptionsFormatException)
{
context.Result = new ObjectResult(ProblemDetailsUtils.GenerateProblemDetails(context.Exception, OptionsErrorCodes.InvalidOptionsFormat, HttpStatusCode.BadRequest)) { StatusCode = (int)HttpStatusCode.BadRequest };
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@ namespace Altinn.Studio.Designer.Filters;
public static class ProblemDetailsExtensionsCodes
{
public const string ErrorCode = "errorCode";

public const string Detail = "detail";
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
using System.ComponentModel.DataAnnotations;

namespace Altinn.Studio.Designer.Helpers.JsonConverterHelpers;

public class NotNullableAttribute : ValidationAttribute
{
protected override ValidationResult IsValid(object value, ValidationContext validationContext)
{
if (value == null)
{
return new ValidationResult("The field is required.");
}
return ValidationResult.Success;
}
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
using System;
using System.Text.Json;
using System.Text.Json.Serialization;
using Altinn.Studio.Designer.Exceptions.Options;

namespace Altinn.Studio.Designer.Helpers.JsonConverterHelpers;

public class OptionConverter : JsonConverter<object>
public class OptionValueConverter : JsonConverter<object>
{
public override object Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
Expand All @@ -14,7 +15,7 @@ public override object Read(ref Utf8JsonReader reader, Type typeToConvert, JsonS
JsonTokenType.Number when reader.TryGetDouble(out double d) => d,
JsonTokenType.True => true,
JsonTokenType.False => false,
_ => throw new JsonException($"Unsupported JSON token for Option.Value: {reader.TokenType}")
_ => throw new InvalidOptionsFormatException($"Unsupported JSON token for Value field, {typeToConvert}: {reader.TokenType}.")
};
}

Expand All @@ -32,7 +33,7 @@ public override void Write(Utf8JsonWriter writer, object value, JsonSerializerOp
writer.WriteBooleanValue(b);
break;
default:
throw new JsonException("Unsupported type for Option.Value.");
throw new InvalidOptionsFormatException($"{value} is an unsupported type for Value field.");
ErlingHauan marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
2 changes: 2 additions & 0 deletions backend/src/Designer/Infrastructure/MvcConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using Altinn.Studio.Designer.Filters.DataModeling;
using Altinn.Studio.Designer.Filters.Git;
using Altinn.Studio.Designer.Filters.IO;
using Altinn.Studio.Designer.Filters.Options;
using Altinn.Studio.Designer.ModelBinding;
using Microsoft.AspNetCore.Mvc;
using Microsoft.Extensions.DependencyInjection;
Expand All @@ -26,6 +27,7 @@ public static IServiceCollection ConfigureMvc(this IServiceCollection services)
options.Filters.Add(typeof(DataModelingExceptionFilterAttribute));
options.Filters.Add(typeof(GitExceptionFilterAttribute));
options.Filters.Add(typeof(IoExceptionFilterAttribute));
options.Filters.Add(typeof(OptionsExceptionFilterAttribute));
})
.AddNewtonsoftJson(options => options.SerializerSettings.Converters.Add(new Newtonsoft.Json.Converters.StringEnumConverter()));

Expand Down
11 changes: 5 additions & 6 deletions backend/src/Designer/Models/Option.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
using System.ComponentModel.DataAnnotations;
using System.Text.Json.Serialization;
using Altinn.Studio.Designer.Helpers.JsonConverterHelpers;

Expand All @@ -12,17 +11,17 @@ public class Option
/// <summary>
/// Value that connects the option to the data model.
/// </summary>
[Required]
[NotNullable]
[JsonPropertyName("value")]
[JsonConverter(typeof(OptionConverter))]
public object Value { get; set; }
[JsonConverter(typeof(OptionValueConverter))]
public required object Value { get; set; }

/// <summary>
/// Label to present to the user.
/// </summary>
[Required]
[NotNullable]
[JsonPropertyName("label")]
public string Label { get; set; }
public required string Label { get; set; }

/// <summary>
/// Description, typically displayed below the label.
Expand Down
36 changes: 19 additions & 17 deletions backend/src/Designer/Services/Implementation/OptionsService.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
using System.Collections.Generic;
using System.Linq;
using System.ComponentModel.DataAnnotations;
using System.Text.Json;
using System.Threading;
using System.Threading.Tasks;
using Altinn.Studio.Designer.Exceptions.Options;
using Altinn.Studio.Designer.Models;
using Altinn.Studio.Designer.Services.Interfaces;
using LibGit2Sharp;
Expand Down Expand Up @@ -50,9 +51,26 @@ public async Task<List<Option>> GetOptionsList(string org, string repo, string d

string optionsListString = await altinnAppGitRepository.GetOptionsList(optionsListId, cancellationToken);
var optionsList = JsonSerializer.Deserialize<List<Option>>(optionsListString);

try
{
optionsList.ForEach(ValidateOption);
}
catch (ValidationException)
{
throw new InvalidOptionsFormatException($"One or more of the options have an invalid format in file: {optionsListId}.");
ErlingHauan marked this conversation as resolved.
Show resolved Hide resolved
}


return optionsList;
}

private void ValidateOption(Option option)
{
var validationContext = new ValidationContext(option);
Validator.ValidateObject(option, validationContext, validateAllProperties: true);
}

/// <inheritdoc />
public async Task<List<Option>> CreateOrOverwriteOptionsList(string org, string repo, string developer, string optionsListId, List<Option> payload, CancellationToken cancellationToken = default)
{
Expand All @@ -73,28 +91,12 @@ public async Task<List<Option>> UploadNewOption(string org, string repo, string
List<Option> deserializedOptions = JsonSerializer.Deserialize<List<Option>>(payload.OpenReadStream(),
new JsonSerializerOptions { WriteIndented = true, AllowTrailingCommas = true });

IEnumerable<Option> result = deserializedOptions.Where(option => IsNullOrEmptyOptionValue(option) || string.IsNullOrEmpty(option.Label));
if (result.Any())
{
throw new JsonException("Uploaded file is missing one of the following attributes for an option: value or label.");
}

var altinnAppGitRepository = _altinnGitRepositoryFactory.GetAltinnAppGitRepository(org, repo, developer);
await altinnAppGitRepository.CreateOrOverwriteOptionsList(optionsListId, deserializedOptions, cancellationToken);

return deserializedOptions;
}

bool IsNullOrEmptyOptionValue(object value)
{
if (value == null)
{
return true;
}

return value is string stringOption && string.IsNullOrEmpty(stringOption);
}

/// <inheritdoc />
public void DeleteOptionsList(string org, string repo, string developer, string optionsListId)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ public async Task GetOptionsListIds_ShouldReturnOk(string org, string app, strin
{
{ "other-options" },
{ "test-options" },
{ "options-with-null-fields" },
};

string url = $"{VersionPrefix(org, targetRepository)}/option-list-ids";
Expand All @@ -39,7 +40,7 @@ public async Task GetOptionsListIds_ShouldReturnOk(string org, string app, strin

string responseContent = await response.Content.ReadAsStringAsync();
List<string> responseList = JsonSerializer.Deserialize<List<string>>(responseContent);
responseList.Count.Should().Be(2);
responseList.Count.Should().Be(3);
foreach (string id in expectedOptionsListIds)
{
responseList.Should().Contain(id);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
using System.Collections.Generic;
using System.Linq;
using System.Net.Http;
using System.Text.Json;
using System.Threading.Tasks;
using Altinn.Studio.Designer.Filters;
using Altinn.Studio.Designer.Models;
using Designer.Tests.Controllers.ApiTests;
using FluentAssertions;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.Testing;
using Xunit;

Expand Down Expand Up @@ -96,4 +98,26 @@ public async Task GetSingleOptionsList_Returns404NotFound_WhenOptionsListDoesNot
// Assert
Assert.Equal(StatusCodes.Status404NotFound, (int)response.StatusCode);
}

[Fact]
public async Task GetSingleOptionsList_Returns400BadRequest_WhenOptionsListIsInvalid()
{
// Arrange
const string repo = "app-with-options";
const string optionsListId = "options-with-null-fields";

string apiUrl = $"/designer/api/ttd/{repo}/options/{optionsListId}";
using HttpRequestMessage httpRequestMessage = new(HttpMethod.Get, apiUrl);

// Act
using HttpResponseMessage response = await HttpClient.SendAsync(httpRequestMessage);

// Assert
Assert.Equal(StatusCodes.Status400BadRequest, (int)response.StatusCode);

var problemDetails = JsonSerializer.Deserialize<ProblemDetails>(await response.Content.ReadAsStringAsync());
problemDetails.Should().NotBeNull();
JsonElement errorCode = (JsonElement)problemDetails.Extensions[ProblemDetailsExtensionsCodes.ErrorCode];
errorCode.ToString().Should().Be("InvalidOptionsFormat");
}
}
Loading
Loading