From c6d233f7dd2ce77c92d9bed5538ca20017b181c4 Mon Sep 17 00:00:00 2001 From: Kevin Hahn Date: Fri, 29 Nov 2024 14:12:55 +0700 Subject: [PATCH] complex form type validation (#1282) * create ComplexFormTypeValidator and test it * wire up complex form type validation, fix some failing tests due to bad input * add basic entry validator as an example of validating dependent properties like EntryId on sense * set a custom message for Sense.EntryId validation * add a validation method to MiniLcmValidators * defined MultiString validators for required and no empty values --- .../Api/FwDataMiniLcmApi.cs | 9 ++-- .../FwDataMiniLcmBridge/FwDataBridgeKernel.cs | 2 + .../FwDataMiniLcmBridge/FwDataFactory.cs | 9 ++-- .../LcmCrdt.Tests/Changes/ComplexFormTests.cs | 4 +- backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs | 5 ++- backend/FwLite/LcmCrdt/LcmCrdtKernel.cs | 2 + .../ComplexFormTypeValidationTests.cs | 43 +++++++++++++++++++ .../Validators/EntryValidatorTests.cs | 34 +++++++++++++++ backend/FwLite/MiniLcm/MiniLcm.csproj | 5 +++ .../Validators/ComplexFormTypeValidator.cs | 14 ++++++ .../MiniLcm/Validators/EntryValidator.cs | 13 ++++++ .../MiniLcm/Validators/MiniLcmValidators.cs | 23 ++++++++++ .../Validators/MultiStringValidator.cs | 17 ++++++++ .../MiniLcm/Validators/SenseValidator.cs | 18 ++++++++ 14 files changed, 189 insertions(+), 9 deletions(-) create mode 100644 backend/FwLite/MiniLcm.Tests/Validators/ComplexFormTypeValidationTests.cs create mode 100644 backend/FwLite/MiniLcm.Tests/Validators/EntryValidatorTests.cs create mode 100644 backend/FwLite/MiniLcm/Validators/ComplexFormTypeValidator.cs create mode 100644 backend/FwLite/MiniLcm/Validators/EntryValidator.cs create mode 100644 backend/FwLite/MiniLcm/Validators/MiniLcmValidators.cs create mode 100644 backend/FwLite/MiniLcm/Validators/MultiStringValidator.cs create mode 100644 backend/FwLite/MiniLcm/Validators/SenseValidator.cs diff --git a/backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs b/backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs index bc143f845..2fc63ed95 100644 --- a/backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs +++ b/backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs @@ -2,6 +2,7 @@ using System.Globalization; using System.Reflection; using System.Text; +using FluentValidation; using FwDataMiniLcmBridge.Api.UpdateProxy; using FwDataMiniLcmBridge.LcmUtils; using Microsoft.Extensions.Logging; @@ -9,6 +10,7 @@ using MiniLcm.Exceptions; using MiniLcm.Models; using MiniLcm.SyncHelpers; +using MiniLcm.Validators; using SIL.LCModel; using SIL.LCModel.Core.KernelInterfaces; using SIL.LCModel.Core.Text; @@ -18,7 +20,7 @@ namespace FwDataMiniLcmBridge.Api; -public class FwDataMiniLcmApi(Lazy cacheLazy, bool onCloseSave, ILogger logger, FwDataProject project) : IMiniLcmApi, IDisposable +public class FwDataMiniLcmApi(Lazy cacheLazy, bool onCloseSave, ILogger logger, FwDataProject project, MiniLcmValidators validators) : IMiniLcmApi, IDisposable { internal LcmCache Cache => cacheLazy.Value; public FwDataProject Project { get; } = project; @@ -380,8 +382,9 @@ private ComplexFormType ToComplexFormType(ILexEntryType t) return new ComplexFormType() { Id = t.Guid, Name = FromLcmMultiString(t.Name) }; } - public Task CreateComplexFormType(ComplexFormType complexFormType) + public async Task CreateComplexFormType(ComplexFormType complexFormType) { + await validators.ValidateAndThrow(complexFormType); if (complexFormType.Id == default) complexFormType.Id = Guid.NewGuid(); UndoableUnitOfWorkHelper.DoUsingNewOrCurrentUOW("Create complex form type", "Remove complex form type", @@ -394,7 +397,7 @@ public Task CreateComplexFormType(ComplexFormType complexFormTy ComplexFormTypes.PossibilitiesOS.Add(lexComplexFormType); UpdateLcmMultiString(lexComplexFormType.Name, complexFormType.Name); }); - return Task.FromResult(ToComplexFormType(ComplexFormTypesFlattened.Single(c => c.Guid == complexFormType.Id))); + return ToComplexFormType(ComplexFormTypesFlattened.Single(c => c.Guid == complexFormType.Id)); } public IAsyncEnumerable GetVariantTypes() diff --git a/backend/FwLite/FwDataMiniLcmBridge/FwDataBridgeKernel.cs b/backend/FwLite/FwDataMiniLcmBridge/FwDataBridgeKernel.cs index 0e20c2cf4..395c5c78d 100644 --- a/backend/FwLite/FwDataMiniLcmBridge/FwDataBridgeKernel.cs +++ b/backend/FwLite/FwDataMiniLcmBridge/FwDataBridgeKernel.cs @@ -1,6 +1,7 @@ using FwDataMiniLcmBridge.LcmUtils; using Microsoft.Extensions.DependencyInjection; using MiniLcm; +using MiniLcm.Validators; namespace FwDataMiniLcmBridge; @@ -16,6 +17,7 @@ public static IServiceCollection AddFwDataBridge(this IServiceCollection service services.AddSingleton(); services.AddSingleton(); services.AddKeyedScoped(FwDataApiKey, (provider, o) => provider.GetRequiredService().GetCurrentFwDataMiniLcmApi(true)); + services.AddMiniLcmValidators(); services.AddSingleton(); return services; } diff --git a/backend/FwLite/FwDataMiniLcmBridge/FwDataFactory.cs b/backend/FwLite/FwDataMiniLcmBridge/FwDataFactory.cs index 78fc74092..f780b9c13 100644 --- a/backend/FwLite/FwDataMiniLcmBridge/FwDataFactory.cs +++ b/backend/FwLite/FwDataMiniLcmBridge/FwDataFactory.cs @@ -4,6 +4,7 @@ using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; +using MiniLcm.Validators; using SIL.LCModel; namespace FwDataMiniLcmBridge; @@ -14,7 +15,8 @@ public class FwDataFactory( IMemoryCache cache, ILogger logger, IProjectLoader projectLoader, - FieldWorksProjectList fieldWorksProjectList) : IDisposable + FieldWorksProjectList fieldWorksProjectList, + MiniLcmValidators validators) : IDisposable { private bool _shuttingDown = false; public FwDataFactory(FwDataProjectContext context, @@ -23,7 +25,8 @@ public FwDataFactory(FwDataProjectContext context, ILogger logger, IProjectLoader projectLoader, IHostApplicationLifetime lifetime, - FieldWorksProjectList fieldWorksProjectList) : this(context, fwdataLogger, cache, logger, projectLoader, fieldWorksProjectList) + FieldWorksProjectList fieldWorksProjectList, + MiniLcmValidators validators) : this(context, fwdataLogger, cache, logger, projectLoader, fieldWorksProjectList, validators) { lifetime.ApplicationStopping.Register(() => { @@ -43,7 +46,7 @@ public FwDataMiniLcmApi GetFwDataMiniLcmApi(string projectName, bool saveOnDispo public FwDataMiniLcmApi GetFwDataMiniLcmApi(FwDataProject project, bool saveOnDispose) { - return new FwDataMiniLcmApi(new (() => GetProjectServiceCached(project)), saveOnDispose, fwdataLogger, project); + return new FwDataMiniLcmApi(new (() => GetProjectServiceCached(project)), saveOnDispose, fwdataLogger, project, validators); } private HashSet _projects = []; diff --git a/backend/FwLite/LcmCrdt.Tests/Changes/ComplexFormTests.cs b/backend/FwLite/LcmCrdt.Tests/Changes/ComplexFormTests.cs index 804cacdad..e53c60b01 100644 --- a/backend/FwLite/LcmCrdt.Tests/Changes/ComplexFormTests.cs +++ b/backend/FwLite/LcmCrdt.Tests/Changes/ComplexFormTests.cs @@ -11,7 +11,7 @@ public class ComplexFormTests(MiniLcmApiFixture fixture) : IClassFixture projectService.ProjectData; @@ -163,6 +165,7 @@ public IAsyncEnumerable GetComplexFormTypes() public async Task CreateComplexFormType(ComplexFormType complexFormType) { + await validators.ValidateAndThrow(complexFormType); if (complexFormType.Id == default) complexFormType.Id = Guid.NewGuid(); await dataModel.AddChange(ClientId, new CreateComplexFormType(complexFormType.Id, complexFormType.Name)); return await ComplexFormTypes.SingleAsync(c => c.Id == complexFormType.Id); diff --git a/backend/FwLite/LcmCrdt/LcmCrdtKernel.cs b/backend/FwLite/LcmCrdt/LcmCrdtKernel.cs index 74562c440..a6a468f16 100644 --- a/backend/FwLite/LcmCrdt/LcmCrdtKernel.cs +++ b/backend/FwLite/LcmCrdt/LcmCrdtKernel.cs @@ -16,6 +16,7 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; +using MiniLcm.Validators; using Refit; using SIL.Harmony.Db; @@ -34,6 +35,7 @@ public static IServiceCollection AddLcmCrdtClient(this IServiceCollection servic ConfigureCrdt ); services.AddScoped(); + services.AddMiniLcmValidators(); services.AddScoped(); services.AddSingleton(); services.AddSingleton(); diff --git a/backend/FwLite/MiniLcm.Tests/Validators/ComplexFormTypeValidationTests.cs b/backend/FwLite/MiniLcm.Tests/Validators/ComplexFormTypeValidationTests.cs new file mode 100644 index 000000000..8b6005add --- /dev/null +++ b/backend/FwLite/MiniLcm.Tests/Validators/ComplexFormTypeValidationTests.cs @@ -0,0 +1,43 @@ +using FluentValidation.TestHelper; +using MiniLcm.Validators; + +namespace MiniLcm.Tests.Validators; + +public class ComplexFormTypeValidationTests +{ + private readonly ComplexFormTypeValidator _validator = new(); + + [Fact] + public void FailsForEmptyName() + { + var complexFormType = new ComplexFormType() { Name = new MultiString() }; + _validator.TestValidate(complexFormType).ShouldHaveValidationErrorFor(c => c.Name); + } + [Fact] + public void FailsForNameWithEmptyStringValue() + { + var complexFormType = new ComplexFormType() { Name = new(){ { "en", string.Empty } } }; + _validator.TestValidate(complexFormType).ShouldHaveValidationErrorFor(c => c.Name); + } + + [Fact] + public void FailsForNonNullDeletedAt() + { + var complexFormType = new ComplexFormType() + { + Name = new() { { "en", "test" } }, DeletedAt = DateTimeOffset.UtcNow + }; + _validator.TestValidate(complexFormType).ShouldHaveValidationErrorFor(c => c.DeletedAt); + } + + [Fact] + public void Succeeds() + { + var complexFormType = new ComplexFormType() + { + Name = new() { { "en", "test" } }, + DeletedAt = null + }; + _validator.TestValidate(complexFormType).ShouldNotHaveAnyValidationErrors(); + } +} diff --git a/backend/FwLite/MiniLcm.Tests/Validators/EntryValidatorTests.cs b/backend/FwLite/MiniLcm.Tests/Validators/EntryValidatorTests.cs new file mode 100644 index 000000000..3a92fc6b9 --- /dev/null +++ b/backend/FwLite/MiniLcm.Tests/Validators/EntryValidatorTests.cs @@ -0,0 +1,34 @@ +using FluentValidation.TestHelper; +using MiniLcm.Validators; + +namespace MiniLcm.Tests.Validators; + +public class EntryValidatorTests +{ + private readonly EntryValidator _validator = new(); + + [Fact] + public void Succeeds_WhenSenseEntryIdIsGuidEmpty() + { + var entryId = Guid.NewGuid(); + var entry = new Entry() { Id = entryId, Senses = [new Sense() { EntryId = Guid.Empty, }] }; + _validator.TestValidate(entry).ShouldNotHaveAnyValidationErrors(); + } + + [Fact] + public void Succeeds_WhenSenseEntryIdMatchesEntry() + { + + var entryId = Guid.NewGuid(); + var entry = new Entry() { Id = entryId, Senses = [new Sense() { EntryId = entryId, }] }; + _validator.TestValidate(entry).ShouldNotHaveAnyValidationErrors(); + } + + [Fact] + public void Fails_WhenSenseEntryIdDoesNotMatchEntry() + { + var entryId = Guid.NewGuid(); + var entry = new Entry() { Id = entryId, Senses = [new Sense() { EntryId = Guid.NewGuid(), }] }; + _validator.TestValidate(entry).ShouldHaveValidationErrorFor("Senses[0].EntryId"); + } +} diff --git a/backend/FwLite/MiniLcm/MiniLcm.csproj b/backend/FwLite/MiniLcm/MiniLcm.csproj index 7635880b1..e806ed4ff 100644 --- a/backend/FwLite/MiniLcm/MiniLcm.csproj +++ b/backend/FwLite/MiniLcm/MiniLcm.csproj @@ -6,9 +6,14 @@ + + + + + diff --git a/backend/FwLite/MiniLcm/Validators/ComplexFormTypeValidator.cs b/backend/FwLite/MiniLcm/Validators/ComplexFormTypeValidator.cs new file mode 100644 index 000000000..645d3c81a --- /dev/null +++ b/backend/FwLite/MiniLcm/Validators/ComplexFormTypeValidator.cs @@ -0,0 +1,14 @@ +using FluentValidation; +using FluentValidation.Validators; +using MiniLcm.Models; + +namespace MiniLcm.Validators; + +internal class ComplexFormTypeValidator : AbstractValidator +{ + public ComplexFormTypeValidator() + { + RuleFor(c => c.DeletedAt).Null(); + RuleFor(c => c.Name).Required(); + } +} diff --git a/backend/FwLite/MiniLcm/Validators/EntryValidator.cs b/backend/FwLite/MiniLcm/Validators/EntryValidator.cs new file mode 100644 index 000000000..a68076b26 --- /dev/null +++ b/backend/FwLite/MiniLcm/Validators/EntryValidator.cs @@ -0,0 +1,13 @@ +using FluentValidation; +using MiniLcm.Models; + +namespace MiniLcm.Validators; + +public class EntryValidator : AbstractValidator +{ + public EntryValidator() + { + RuleForEach(e => e.Senses).SetValidator(entry => new SenseValidator(entry)); + //todo just a stub as an example for senses + } +} diff --git a/backend/FwLite/MiniLcm/Validators/MiniLcmValidators.cs b/backend/FwLite/MiniLcm/Validators/MiniLcmValidators.cs new file mode 100644 index 000000000..2da5c773c --- /dev/null +++ b/backend/FwLite/MiniLcm/Validators/MiniLcmValidators.cs @@ -0,0 +1,23 @@ +using FluentValidation; +using Microsoft.Extensions.DependencyInjection; +using MiniLcm.Models; + +namespace MiniLcm.Validators; + +public record MiniLcmValidators(IValidator ComplexFormTypeValidator) +{ + public async Task ValidateAndThrow(ComplexFormType value) + { + await ComplexFormTypeValidator.ValidateAndThrowAsync(value); + } +} + +public static class MiniLcmValidatorsExtensions +{ + public static IServiceCollection AddMiniLcmValidators(this IServiceCollection services) + { + services.AddTransient(); + services.AddTransient, ComplexFormTypeValidator>(); + return services; + } +} diff --git a/backend/FwLite/MiniLcm/Validators/MultiStringValidator.cs b/backend/FwLite/MiniLcm/Validators/MultiStringValidator.cs new file mode 100644 index 000000000..0ee9959de --- /dev/null +++ b/backend/FwLite/MiniLcm/Validators/MultiStringValidator.cs @@ -0,0 +1,17 @@ +using FluentValidation; +using MiniLcm.Models; + +namespace MiniLcm.Validators; + +internal static class MultiStringValidator +{ + public static IRuleBuilderOptions Required(this IRuleBuilder ruleBuilder) + { + return ruleBuilder.NotEmpty().NoEmptyValues(); + } + public static IRuleBuilderOptions NoEmptyValues(this IRuleBuilder ruleBuilder) + { + return ruleBuilder.Must(ms => ms.Values.All(v => !string.IsNullOrEmpty(v.Value))).WithMessage((parent, ms) => + $"MultiString must not contain empty values, but [{string.Join(", ", ms.Values.Where(v => string.IsNullOrWhiteSpace(v.Value)).Select(v => v.Key))}] was empty"); + } +} diff --git a/backend/FwLite/MiniLcm/Validators/SenseValidator.cs b/backend/FwLite/MiniLcm/Validators/SenseValidator.cs new file mode 100644 index 000000000..43f72508d --- /dev/null +++ b/backend/FwLite/MiniLcm/Validators/SenseValidator.cs @@ -0,0 +1,18 @@ +using FluentValidation; +using MiniLcm.Models; + +namespace MiniLcm.Validators; + +public class SenseValidator : AbstractValidator +{ + public SenseValidator() + { + //todo add validation for the other properties + } + + public SenseValidator(Entry entry): this() + { + //it's ok if senses EntryId is an Empty guid + RuleFor(s => s.EntryId).Equal(entry.Id).When(s => s.EntryId != Guid.Empty).WithMessage(sense => $"Sense (Id: {sense.Id}) EntryId must match Entry {entry.Id}, but instead was {sense.EntryId}"); + } +}