From 99a0b70cfaff94cf4bebb2637e69d3c530e3fd27 Mon Sep 17 00:00:00 2001 From: Andrew Camilleri Date: Fri, 13 Oct 2023 03:08:16 +0200 Subject: [PATCH] Fix form value setter (#5387) * Fix form value setter * Fix test parallelization --------- Co-authored-by: nicolas.dorier --- BTCPayServer.Abstractions/Form/Form.cs | 26 +-------------- BTCPayServer.Tests/FormTests.cs | 26 ++++++++++----- .../UICustodianAccountsController.cs | 8 ++--- BTCPayServer/Forms/FieldValueMirror.cs | 5 +++ BTCPayServer/Forms/FormDataService.cs | 32 +++++++++++++++++++ .../Forms/HtmlFieldsetFormProvider.cs | 6 ++++ BTCPayServer/Forms/IFormComponentProvider.cs | 7 ++++ .../Controllers/UIPointOfSaleController.cs | 2 +- 8 files changed, 74 insertions(+), 38 deletions(-) diff --git a/BTCPayServer.Abstractions/Form/Form.cs b/BTCPayServer.Abstractions/Form/Form.cs index 909594b66..0f91954e2 100644 --- a/BTCPayServer.Abstractions/Form/Form.cs +++ b/BTCPayServer.Abstractions/Form/Form.cs @@ -105,31 +105,7 @@ public class Form } } - public void SetValues(JObject values) - { - var fields = GetAllFields().ToDictionary(k => k.FullName, k => k.Field); - SetValues(fields, new List(), values); - } - - private void SetValues(Dictionary fields, List path, JObject values) - { - foreach (var prop in values.Properties()) - { - List propPath = new List(path.Count + 1); - propPath.AddRange(path); - propPath.Add(prop.Name); - if (prop.Value.Type == JTokenType.Object) - { - SetValues(fields, propPath, (JObject)prop.Value); - } - else if (prop.Value.Type == JTokenType.String) - { - var fullName = string.Join('_', propPath.Where(s => !string.IsNullOrEmpty(s))); - if (fields.TryGetValue(fullName, out var f) && !f.Constant) - f.Value = prop.Value.Value(); - } - } - } + } diff --git a/BTCPayServer.Tests/FormTests.cs b/BTCPayServer.Tests/FormTests.cs index 59669ed84..1d245f75d 100644 --- a/BTCPayServer.Tests/FormTests.cs +++ b/BTCPayServer.Tests/FormTests.cs @@ -1,5 +1,6 @@ using System.Collections.Generic; using System.Linq; +using System.Threading.Tasks; using BTCPayServer.Abstractions.Form; using BTCPayServer.Forms; using Microsoft.AspNetCore.Http; @@ -10,16 +11,25 @@ using Xunit.Abstractions; namespace BTCPayServer.Tests; -[Trait("Fast", "Fast")] +[Collection(nameof(NonParallelizableCollectionDefinition))] +[Trait("Integration", "Integration")] public class FormTests : UnitTestBase { public FormTests(ITestOutputHelper helper) : base(helper) { } - [Fact] - public void CanParseForm() + + [Fact(Timeout = TestUtils.TestTimeout)] + [Trait("Integration", "Integration")] + public async Task CanParseForm() { + using var tester = CreateServerTester(); + await tester.StartAsync(); + var user = tester.NewAccount(); + user.GrantAccess(); + var service = tester.PayTester.GetService(); + var form = new Form() { Fields = new List @@ -40,8 +50,6 @@ public class FormTests : UnitTestBase } } }; - var providers = new FormComponentProviders(new List()); - var service = new FormDataService(null, providers); Assert.False(service.IsFormSchemaValid(form.ToString(), out _, out _)); form = new Form { @@ -164,7 +172,7 @@ public class FormTests : UnitTestBase Assert.Equal("original", obj["invoice"]["test"].Value()); Assert.Equal("updated", obj["invoice_item3"].Value()); Clear(form); - form.SetValues(obj); + service.SetValues(form, obj); obj = service.GetValues(form); Assert.Equal("original", obj["invoice"]["test"].Value()); Assert.Equal("updated", obj["invoice_item3"].Value()); @@ -182,10 +190,12 @@ public class FormTests : UnitTestBase } } }; - form.SetValues(obj); + + service.SetValues(form, obj); obj = service.GetValues(form); Assert.Null(obj["test"].Value()); - form.SetValues(new JObject { ["test"] = "hello" }); + + service.SetValues(form, new JObject { ["test"] = "hello" }); obj = service.GetValues(form); Assert.Equal("hello", obj["test"].Value()); } diff --git a/BTCPayServer/Controllers/UICustodianAccountsController.cs b/BTCPayServer/Controllers/UICustodianAccountsController.cs index e3d2ce390..d143252bb 100644 --- a/BTCPayServer/Controllers/UICustodianAccountsController.cs +++ b/BTCPayServer/Controllers/UICustodianAccountsController.cs @@ -223,7 +223,7 @@ namespace BTCPayServer.Controllers var blob = custodianAccount.GetBlob(); var configForm = await custodian.GetConfigForm(blob, HttpContext.RequestAborted); - configForm.SetValues(blob); + _formDataService.SetValues(configForm, blob); var vm = new EditCustodianAccountViewModel(); vm.CustodianAccount = custodianAccount; @@ -280,14 +280,14 @@ namespace BTCPayServer.Controllers // First, we restore the previous form based on the previous blob that was // stored in config var form = await custodian.GetConfigForm(b, HttpContext.RequestAborted); - form.SetValues(b); + _formDataService.SetValues(form, b); // Then we apply new values overriding the previous blob from the Form params form.ApplyValuesFromForm(Request.Form); // We extract the new resulting blob, and request what is the next form based on it b = _formDataService.GetValues(form); form = await custodian.GetConfigForm(_formDataService.GetValues(form), HttpContext.RequestAborted); // We set all the values to this blob, and validate the form - form.SetValues(b); + _formDataService.SetValues(form, b); _formDataService.Validate(form, ModelState); return form; } @@ -600,7 +600,7 @@ namespace BTCPayServer.Controllers catch (BadConfigException e) { Form configForm = await custodian.GetConfigForm(config); - configForm.SetValues(config); + _formDataService.SetValues(configForm, config); string[] badConfigFields = new string[e.BadConfigKeys.Length]; int i = 0; foreach (var oneField in configForm.GetAllFields()) diff --git a/BTCPayServer/Forms/FieldValueMirror.cs b/BTCPayServer/Forms/FieldValueMirror.cs index 8421c6377..e809eba9f 100644 --- a/BTCPayServer/Forms/FieldValueMirror.cs +++ b/BTCPayServer/Forms/FieldValueMirror.cs @@ -31,4 +31,9 @@ public class FieldValueMirror : IFormComponentProvider return rawValue; } + + public void SetValue(Field field, JToken value) + { + //ignored + } } diff --git a/BTCPayServer/Forms/FormDataService.cs b/BTCPayServer/Forms/FormDataService.cs index a07c3c445..f3509a60f 100644 --- a/BTCPayServer/Forms/FormDataService.cs +++ b/BTCPayServer/Forms/FormDataService.cs @@ -218,4 +218,36 @@ public class FormDataService } return r; } + + public void SetValues(Form form, JObject values) + { + + var fields = form.GetAllFields().ToDictionary(k => k.FullName, k => k.Field); + SetValues(fields, new List(), values); + } + + private void SetValues(Dictionary fields, List path, JObject values) + { + foreach (var prop in values.Properties()) + { + List propPath = new List(path.Count + 1); + propPath.AddRange(path); + propPath.Add(prop.Name); + if (prop.Value.Type == JTokenType.Object) + { + SetValues(fields, propPath, (JObject)prop.Value); + } + else if (prop.Value.Type == JTokenType.String) + { + var fullName = string.Join('_', propPath.Where(s => !string.IsNullOrEmpty(s))); + if (fields.TryGetValue(fullName, out var f) && !f.Constant) + { + if (_formProviders.TypeToComponentProvider.TryGetValue(f.Type, out var formComponentProvider)) + { + formComponentProvider.SetValue(f, prop.Value); + } + } + } + } + } } diff --git a/BTCPayServer/Forms/HtmlFieldsetFormProvider.cs b/BTCPayServer/Forms/HtmlFieldsetFormProvider.cs index ef5fc5c3a..b49d7d965 100644 --- a/BTCPayServer/Forms/HtmlFieldsetFormProvider.cs +++ b/BTCPayServer/Forms/HtmlFieldsetFormProvider.cs @@ -1,5 +1,6 @@ using System.Collections.Generic; using BTCPayServer.Abstractions.Form; +using Newtonsoft.Json.Linq; namespace BTCPayServer.Forms; @@ -17,6 +18,11 @@ public class HtmlFieldsetFormProvider : IFormComponentProvider return null; } + public void SetValue(Field field, JToken value) + { + //ignored + } + public void Validate(Form form, Field field) { } diff --git a/BTCPayServer/Forms/IFormComponentProvider.cs b/BTCPayServer/Forms/IFormComponentProvider.cs index db492cfc3..d59a46d9e 100644 --- a/BTCPayServer/Forms/IFormComponentProvider.cs +++ b/BTCPayServer/Forms/IFormComponentProvider.cs @@ -1,6 +1,7 @@ using System.Collections.Generic; using System.ComponentModel.DataAnnotations; using BTCPayServer.Abstractions.Form; +using Newtonsoft.Json.Linq; namespace BTCPayServer.Forms; @@ -10,6 +11,7 @@ public interface IFormComponentProvider void Validate(Form form, Field field); void Register(Dictionary typeToComponentProvider); string GetValue(Form form, Field field); + void SetValue(Field field, JToken value); } public abstract class FormComponentProviderBase : IFormComponentProvider @@ -21,6 +23,11 @@ public abstract class FormComponentProviderBase : IFormComponentProvider return field.Value; } + public void SetValue(Field field, JToken value) + { + field.Value = value.ToString(); + } + public abstract void Validate(Form form, Field field); public void ValidateField(Field field) where T : ValidationAttribute, new() diff --git a/BTCPayServer/Plugins/PointOfSale/Controllers/UIPointOfSaleController.cs b/BTCPayServer/Plugins/PointOfSale/Controllers/UIPointOfSaleController.cs index 6e0621888..265a8f622 100644 --- a/BTCPayServer/Plugins/PointOfSale/Controllers/UIPointOfSaleController.cs +++ b/BTCPayServer/Plugins/PointOfSale/Controllers/UIPointOfSaleController.cs @@ -277,7 +277,7 @@ namespace BTCPayServer.Plugins.PointOfSale.Controllers formResponseJObject = TryParseJObject(formResponse) ?? new JObject(); var form = Form.Parse(formData.Config); - form.SetValues(formResponseJObject); + FormDataService.SetValues(form, formResponseJObject); if (!FormDataService.Validate(form, ModelState)) { //someone tried to bypass validation