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

backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs

@@ -2,13 +2,15 @@
 using System.Globalization;
 using System.Reflection;
 using System.Text;
+using FluentValidation;
 using FwDataMiniLcmBridge.Api.UpdateProxy;
 using FwDataMiniLcmBridge.LcmUtils;
 using Microsoft.Extensions.Logging;
 using MiniLcm;
 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<LcmCache> cacheLazy, bool onCloseSave, ILogger<FwDataMiniLcmApi> logger, FwDataProject project) : IMiniLcmApi, IDisposable
+public class FwDataMiniLcmApi(Lazy<LcmCache> cacheLazy, bool onCloseSave, ILogger<FwDataMiniLcmApi> 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<ComplexFormType> CreateComplexFormType(ComplexFormType complexFormType)
+    public async Task<ComplexFormType> 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<ComplexFormType> 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<VariantType> GetVariantTypes()

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<FieldWorksProjectList>();
         services.AddSingleton<IProjectLoader, ProjectLoader>();
         services.AddKeyedScoped<IMiniLcmApi>(FwDataApiKey, (provider, o) => provider.GetRequiredService<FwDataFactory>().GetCurrentFwDataMiniLcmApi(true));
+        services.AddMiniLcmValidators();
         services.AddSingleton<FwDataProjectContext>();
         return services;
     }

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<FwDataFactory> 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<FwDataFactory> 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<string> _projects = [];

backend/FwLite/LcmCrdt.Tests/Changes/ComplexFormTests.cs

@@ -11,7 +11,7 @@ public class ComplexFormTests(MiniLcmApiFixture fixture) : IClassFixture<MiniLcm
     [Fact]
     public async Task AddComplexFormType()
     {
-        var complexFormType = new ComplexFormType() { Id = Guid.NewGuid(), Name = new MultiString() };
+        var complexFormType = new ComplexFormType() { Id = Guid.NewGuid(), Name = new() { { "en", "test" } } };
         await fixture.Api.CreateComplexFormType(complexFormType);
         var complexEntry = await fixture.Api.CreateEntry(new() { LexemeForm = { { "en", "Coat rack" } }, });
         var change = new AddComplexFormTypeChange(complexEntry.Id,complexFormType);
@@ -25,7 +25,7 @@ public async Task AddComplexFormType()
     public async Task RemoveComplexFormType()
     {
         var complexEntry = await fixture.Api.CreateEntry(new() { LexemeForm = { { "en", "Coat rack" } }, });
-        var complexFormType = new ComplexFormType() { Id = Guid.NewGuid(), Name = new MultiString() };
+        var complexFormType = new ComplexFormType() { Id = Guid.NewGuid(), Name = new(){ { "en", "test" } } };
         await fixture.Api.CreateComplexFormType(complexFormType);
         await fixture.DataModel.AddChange(
             Guid.NewGuid(),

backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs

@@ -1,4 +1,5 @@
 using System.Linq.Expressions;
+using FluentValidation;
 using SIL.Harmony;
 using SIL.Harmony.Changes;
 using LcmCrdt.Changes;
@@ -9,11 +10,12 @@
 using LinqToDB.EntityFrameworkCore;
 using MiniLcm.Exceptions;
 using MiniLcm.SyncHelpers;
+using MiniLcm.Validators;
 using SIL.Harmony.Db;
 
 namespace LcmCrdt;
 
-public class CrdtMiniLcmApi(DataModel dataModel, CurrentProjectService projectService, LcmCrdtDbContext dbContext) : IMiniLcmApi
+public class CrdtMiniLcmApi(DataModel dataModel, CurrentProjectService projectService, LcmCrdtDbContext dbContext, MiniLcmValidators validators) : IMiniLcmApi
 {
     private Guid ClientId { get; } = projectService.ProjectData.ClientId;
     public ProjectData ProjectData => projectService.ProjectData;
@@ -163,6 +165,7 @@ public IAsyncEnumerable<ComplexFormType> GetComplexFormTypes()
 
     public async Task<ComplexFormType> 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);

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<IMiniLcmApi, CrdtMiniLcmApi>();
+        services.AddMiniLcmValidators();
         services.AddScoped<CurrentProjectService>();
         services.AddSingleton<ProjectContext>();
         services.AddSingleton<ProjectsService>();

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();
+    }
+}

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");
+    }
+}

backend/FwLite/MiniLcm/MiniLcm.csproj

@@ -6,9 +6,14 @@
     </PropertyGroup>
 
     <ItemGroup>
+      <PackageReference Include="FluentValidation" Version="11.11.0" />
+      <PackageReference Include="Microsoft.Extensions.DependencyInjection.Abstractions" Version="8.0.2" />
       <PackageReference Include="SIL.WritingSystems" Version="14.2.0-beta*" />
       <PackageReference Include="System.Text.Json" Version="9.0.0" />
       <PackageReference Include="SystemTextJsonPatch" Version="3.2.1" />
     </ItemGroup>
 
+    <ItemGroup>
+        <InternalsVisibleTo Include="MiniLcm.Tests" />
+    </ItemGroup>
 </Project>

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<ComplexFormType>
+{
+    public ComplexFormTypeValidator()
+    {
+        RuleFor(c => c.DeletedAt).Null();
+        RuleFor(c => c.Name).Required();
+    }
+}

backend/FwLite/MiniLcm/Validators/EntryValidator.cs

@@ -0,0 +1,13 @@
+using FluentValidation;
+using MiniLcm.Models;
+
+namespace MiniLcm.Validators;
+
+public class EntryValidator : AbstractValidator<Entry>
+{
+    public EntryValidator()
+    {
+        RuleForEach(e => e.Senses).SetValidator(entry => new SenseValidator(entry));
+        //todo just a stub as an example for senses
+    }
+}

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<ComplexFormType> ComplexFormTypeValidator)
+{
+    public async Task ValidateAndThrow(ComplexFormType value)
+    {
+        await ComplexFormTypeValidator.ValidateAndThrowAsync(value);
+    }
+}
+
+public static class MiniLcmValidatorsExtensions
+{
+    public static IServiceCollection AddMiniLcmValidators(this IServiceCollection services)
+    {
+        services.AddTransient<MiniLcmValidators>();
+        services.AddTransient<IValidator<ComplexFormType>, ComplexFormTypeValidator>();
+        return services;
+    }
+}

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<T, MultiString> Required<T>(this IRuleBuilder<T, MultiString> ruleBuilder)
+    {
+        return ruleBuilder.NotEmpty().NoEmptyValues();
+    }
+    public static IRuleBuilderOptions<T, MultiString> NoEmptyValues<T>(this IRuleBuilder<T, MultiString> 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");
+    }
+}

backend/FwLite/MiniLcm/Validators/SenseValidator.cs

@@ -0,0 +1,18 @@
+using FluentValidation;
+using MiniLcm.Models;
+
+namespace MiniLcm.Validators;
+
+public class SenseValidator : AbstractValidator<Sense>
+{
+    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}");
+    }
+}