Add custom attribute for OptionValue and OptionLabel and allow empty …

…string
Altinn · Nov 12, 2024 · 93399f2 · 93399f2
1 parent 0e4b916
commit 93399f2
Show file tree

Hide file tree

Showing 9 changed files with 160 additions and 52 deletions.
diff --git a/backend/src/Designer/Exceptions/Options/InvalidOptionsFormatException.cs b/backend/src/Designer/Exceptions/Options/InvalidOptionsFormatException.cs
@@ -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)
+    {
+    }
+}
diff --git a/backend/src/Designer/Filters/Options/OptionsErrorCodes.cs b/backend/src/Designer/Filters/Options/OptionsErrorCodes.cs
@@ -0,0 +1,6 @@
+namespace Altinn.Studio.Designer.Filters.Options;
+
+public class OptionsErrorCodes
+{
+    public const string InvalidOptionsFormat = nameof(InvalidOptionsFormat);
+}
diff --git a/backend/src/Designer/Filters/Options/OptionsExceptionFilterAttribute.cs b/backend/src/Designer/Filters/Options/OptionsExceptionFilterAttribute.cs
@@ -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 };
+        }
+    }
+
+}
diff --git a/backend/src/Designer/Helpers/JsonConverterHelpers/NotNullableAttribute.cs b/backend/src/Designer/Helpers/JsonConverterHelpers/NotNullableAttribute.cs
@@ -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;
+    }
+}
diff --git a/backend/src/Designer/Helpers/JsonConverterHelpers/OptionConverterHelper.cs b/backend/src/Designer/Helpers/JsonConverterHelpers/OptionConverterHelper.cs
@@ -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)
     {
@@ -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 Option property, {typeToConvert}: {reader.TokenType}.")
         };
     }
 
@@ -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 Option fields.");
         }
     }
 }
diff --git a/backend/src/Designer/Infrastructure/MvcConfiguration.cs b/backend/src/Designer/Infrastructure/MvcConfiguration.cs
@@ -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;
@@ -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()));
 

diff --git a/backend/src/Designer/Models/Option.cs b/backend/src/Designer/Models/Option.cs
@@ -1,4 +1,3 @@
-using System.ComponentModel.DataAnnotations;
 using System.Text.Json.Serialization;
 using Altinn.Studio.Designer.Helpers.JsonConverterHelpers;
 
@@ -12,15 +11,15 @@ public class Option
     /// <summary>
     /// Value that connects the option to the data model.
     /// </summary>
-    [Required]
+    [NotNullable]
     [JsonPropertyName("value")]
-    [JsonConverter(typeof(OptionConverter))]
+    [JsonConverter(typeof(OptionValueConverter))]
     public object Value { get; set; }
 
     /// <summary>
     /// Label to present to the user.
     /// </summary>
-    [Required]
+    [NotNullable]
     [JsonPropertyName("label")]
     public string Label { get; set; }
 

diff --git a/backend/src/Designer/Services/Implementation/OptionsService.cs b/backend/src/Designer/Services/Implementation/OptionsService.cs
@@ -3,6 +3,7 @@
 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;
@@ -73,26 +74,36 @@ 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.");
-        }
+        ValidateOptions(deserializedOptions);
 
         var altinnAppGitRepository = _altinnGitRepositoryFactory.GetAltinnAppGitRepository(org, repo, developer);
         await altinnAppGitRepository.CreateOrOverwriteOptionsList(optionsListId, deserializedOptions, cancellationToken);
 
         return deserializedOptions;
     }
 
-    bool IsNullOrEmptyOptionValue(object value)
+    private void ValidateOptions(List<Option> deserializedOptions)
     {
-        if (value == null)
+        IEnumerable<Option> optionsWithInvalidNullValues = deserializedOptions.Where(option => option.Value == null || option.Label == null);
+
+        if (optionsWithInvalidNullValues.Any())
         {
-            return true;
+            bool hasNullValue = optionsWithInvalidNullValues.Any(option => option.Value == null);
+            bool hasNullLabel = optionsWithInvalidNullValues.Any(option => option.Label == null);
+
+            if (hasNullValue && hasNullLabel)
+            {
+                throw new InvalidOptionsFormatException("Uploaded file is missing one or more 'Value' and 'Label' properties.");
+            }
+            if (hasNullValue)
+            {
+                throw new InvalidOptionsFormatException("Uploaded file is missing one or more 'Value' properties.");
+            }
+            if (hasNullLabel)
+            {
+                throw new InvalidOptionsFormatException("Uploaded file is missing one or more 'Label' properties.");
+            }
         }
-
-        return value is string stringOption && string.IsNullOrEmpty(stringOption);
     }
 
     /// <inheritdoc />

diff --git a/backend/tests/Designer.Tests/Controllers/OptionsController/UpdateOptionsTests.cs b/backend/tests/Designer.Tests/Controllers/OptionsController/UpdateOptionsTests.cs
@@ -106,6 +106,35 @@ public async Task Put_Returns_200OK_When_Option_Values_Are_Bool_String_Double()
         Assert.Equal(StatusCodes.Status200OK, (int)response.StatusCode);
     }
 
+    [Fact]
+    public async Task Put_Returns_200OK_When_Option_Value_And_Label_Are_Empty_Strings()
+    {
+        string repo = "app-with-options";
+        string optionsListId = "test-options";
+        // Arrange
+        string targetRepository = TestDataHelper.GenerateTestRepoName();
+        await CopyRepositoryForTest(Org, repo, Developer, targetRepository);
+
+        string apiUrl = $"/designer/api/{Org}/{targetRepository}/options/{optionsListId}";
+        using HttpRequestMessage httpRequestMessage = new(HttpMethod.Put, apiUrl);
+
+        var emptyValueAndLabelOptionsList = new List<Option>
+        {
+            new ()
+            {
+                Label = "",
+                Value = "",
+            }
+        };
+        httpRequestMessage.Content = JsonContent.Create(emptyValueAndLabelOptionsList);
+
+        // Act
+        using HttpResponseMessage response = await HttpClient.SendAsync(httpRequestMessage);
+
+        // Assert
+        Assert.Equal(StatusCodes.Status200OK, (int)response.StatusCode);
+    }
+
     [Fact]
     public async Task Put_Returns_200OK_And_Overwrites_Existing_OptionsList()
     {
@@ -152,73 +181,67 @@ public async Task Put_Returns_200OK_And_Overwrites_Existing_OptionsList()
         }
     }
 
-    [Theory]
-    [InlineData("options-missing-label")]
-    [InlineData("options-empty-json")]
-    public async Task Put_Returns_400BadRequest_When_OptionsList_Format_Is_Invalid(string optionsListId)
+    [Fact]
+    public async Task Put_Returns_400BadRequest_When_Options_Is_Empty()
     {
         // Arrange
         string targetRepository = TestDataHelper.GenerateTestRepoName();
         await CopyRepositoryForTest(Org, "empty-app", Developer, targetRepository);
 
-        string apiUrl = $"/designer/api/{Org}/{targetRepository}/options/{optionsListId}";
+        string apiUrl = $"/designer/api/{Org}/{targetRepository}/options/empty-options";
         using HttpRequestMessage httpRequestMessage = new(HttpMethod.Put, apiUrl);
 
-        if (optionsListId == "options-missing-label")
-        {
-            var invalidOptionsList = new List<Option>
-            {
-                new ()
-                {
-                    // Missing Label
-                    Value = "value1",
-                },
-                new ()
-                {
-                    Label = "label2",
-                    Value = "value2",
-                }
-            };
-            httpRequestMessage.Content = JsonContent.Create(invalidOptionsList);
-        }
-        else if (optionsListId == "options-empty-json")
-        {
-            httpRequestMessage.Content = JsonContent.Create<List<Option>>(null);
-        }
+        httpRequestMessage.Content = JsonContent.Create<List<Option>>(null);
 
         // Act
         using HttpResponseMessage response = await HttpClient.SendAsync(httpRequestMessage);
+        var responseContent = await response.Content.ReadAsStringAsync();
+        var responseObject = JsonSerializer.Deserialize<JsonElement>(responseContent);
 
         // Assert
         Assert.Equal(StatusCodes.Status400BadRequest, (int)response.StatusCode);
+        Assert.True(responseObject.TryGetProperty("errors", out JsonElement errors));
+        Assert.True(errors.TryGetProperty("", out JsonElement labelErrors));
+        Assert.Contains("A non-empty request body is required.", labelErrors[0].GetString());
     }
 
     [Fact]
-    public async Task Put_Returns_400BadRequest_When_Option_Value_Is_Invalid()
+    public async Task Put_Returns_400BadRequest_When_Options_Has_Missing_Required_Fields()
     {
-        string repo = "app-with-options";
-        string optionsListId = "test-options";
         // Arrange
         string targetRepository = TestDataHelper.GenerateTestRepoName();
-        await CopyRepositoryForTest(Org, repo, Developer, targetRepository);
+        await CopyRepositoryForTest(Org, "empty-app", Developer, targetRepository);
 
-        string apiUrl = $"/designer/api/{Org}/{targetRepository}/options/{optionsListId}";
+        string apiUrl = $"/designer/api/{Org}/{targetRepository}/options/options-missing-fields";
         using HttpRequestMessage httpRequestMessage = new(HttpMethod.Put, apiUrl);
 
-        var invalidOptionsList = new List<Option>
+        var optionsWithMissingFields = new List<Option>
         {
             new ()
             {
-                Label = "ObjectValue",
-                Value = { },
+                // Missing Label
+                Value = "value1",
             },
+            new ()
+            {
+                Label = "label2",
+                // Missing Value
+            }
         };
-        httpRequestMessage.Content = JsonContent.Create(invalidOptionsList);
+        httpRequestMessage.Content = JsonContent.Create(optionsWithMissingFields);
 
         // Act
         using HttpResponseMessage response = await HttpClient.SendAsync(httpRequestMessage);
+        var responseContent = await response.Content.ReadAsStringAsync();
+        var responseObject = JsonSerializer.Deserialize<JsonElement>(responseContent);
 
         // Assert
         Assert.Equal(StatusCodes.Status400BadRequest, (int)response.StatusCode);
+        Assert.True(responseObject.TryGetProperty("errors", out JsonElement errors));
+        Assert.True(errors.TryGetProperty("[0].Label", out JsonElement labelErrors));
+        Assert.Contains("The field is required.", labelErrors[0].GetString());
+
+        Assert.True(errors.TryGetProperty("[1].Value", out JsonElement valueErrors));
+        Assert.Contains("The field is required.", valueErrors[0].GetString());
     }
 }