Use random commit id when migrating to add morph types (#2344)

* add a new debug helper to diagnose issue cloning a project

* tweak the download projects test to fail due to morph type migrations.

* only seed morphtypes when needed, Modify CreateMorphTypeChange to handle creating the same morph type multiple times due to migrations.

* Implement CreateMorphType in MiniLcm

* Add #2350 comments for follow up work

* Return existing morph type if found in `CreateMorphType` to avoid duplicates.

---------

Co-authored-by: Tim Haasdyk <tim_haasdyk@sil.org>

Author: hahn-kev

backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs

@@ -592,6 +592,11 @@ internal MorphType FromLcmMorphType(IMoMorphType morphType)
         };
     }
 
+    public Task<MorphType> CreateMorphType(MorphType morphType)
+    {
+        throw new NotSupportedException("Morph types cannot be created in fwdata; they are predefined");
+    }
+
     public Task<MorphType> UpdateMorphType(Guid id, UpdateObjectInput<MorphType> update)
     {
         var lcmMorphType = MorphTypeRepository.GetObject(id);

backend/FwLite/FwLiteProjectSync/DryRunMiniLcmApi.cs

@@ -126,6 +126,12 @@ public Task DeleteComplexFormType(Guid id)
         return Task.CompletedTask;
     }
 
+    public Task<MorphType> CreateMorphType(MorphType morphType)
+    {
+        DryRunRecords.Add(new DryRunRecord(nameof(CreateMorphType), $"Create morph type {morphType.Kind} ({morphType.Id})"));
+        return Task.FromResult(morphType);
+    }
+
     public async Task<MorphType> UpdateMorphType(Guid id, UpdateObjectInput<MorphType> update)
     {
         DryRunRecords.Add(new DryRunRecord(nameof(UpdateMorphType), $"Update morph type {id}"));

backend/FwLite/FwLiteProjectSync/Import/ResumableImportApi.cs

@@ -62,6 +62,10 @@ async Task<SemanticDomain> IMiniLcmWriteApi.CreateSemanticDomain(SemanticDomain
     {
         return await HasCreated(semanticDomain, _api.GetSemanticDomains(), () => _api.CreateSemanticDomain(semanticDomain));
     }
+    async Task<MorphType> IMiniLcmWriteApi.CreateMorphType(MorphType morphType)
+    {
+        return await HasCreated(morphType, _api.GetMorphTypes(), () => _api.CreateMorphType(morphType));
+    }
     async Task<Publication> IMiniLcmWriteApi.CreatePublication(Publication publication)
     {
         return await HasCreated(publication, _api.GetPublications(), () => _api.CreatePublication(publication));

backend/FwLite/FwLiteProjectSync/MiniLcmImport.cs

@@ -72,6 +72,7 @@ public async Task ImportProject(IMiniLcmApi importTo, IMiniLcmApi importFrom, in
         }
 
         // Morph types are created automatically for CRDT projects, so we update them instead of creating them
+        // Optimize this to a simple foreach like above in #2350
         var importFromMorphTypes = await importFrom.GetMorphTypes().ToArrayAsync();
         var existingMorphTypes = await importTo.GetMorphTypes().ToArrayAsync();
         await MorphTypeSync.Sync(existingMorphTypes, importFromMorphTypes, importTo);

backend/FwLite/LcmCrdt.Tests/Data/DownloadProjectTests.cs

@@ -1,16 +1,23 @@
+using Microsoft.EntityFrameworkCore;
 using Microsoft.Extensions.DependencyInjection;
-using Microsoft.Extensions.Logging;
+using SIL.Harmony.Core;
 
 namespace LcmCrdt.Tests.Data;
 
 public class DownloadProjectTests : IAsyncLifetime
 {
     private readonly RegressionTestHelper _helper = new("DownloadProject");
-    private readonly MiniLcmApiFixture _apiFixture = MiniLcmApiFixture.Create(false);
+    private static readonly Guid _projectId = new("B467051E-A492-4E5B-9C17-858D7797292C");//internal project Id of v2 project
+    private readonly MiniLcmApiFixture _apiFixture = MiniLcmApiFixture.Create(false, _projectId);
 
     public async Task InitializeAsync()
     {
-        await _helper.InitializeAsync();
+        await _helper.InitializeAsync(RegressionTestHelper.RegressionVersion.v2, withDataMigrations: true);
+        //add a change after migration which creates MorphTypes
+        await _helper.Services.GetRequiredService<IMiniLcmApi>().CreateEntry(new Entry()
+        {
+            LexemeForm = {{"en", "test"}}
+        });
         await _apiFixture.InitializeAsync();
     }
 
@@ -24,6 +31,17 @@ public async Task DisposeAsync()
     public async Task CanCreateANewProjectViaSync()
     {
         var remoteModel = _helper.Services.GetRequiredService<DataModel>();
+        var remoteDb = await _helper.Services.GetRequiredService<IDbContextFactory<LcmCrdtDbContext>>().CreateDbContextAsync();
+
         await _apiFixture.DataModel.SyncWith(remoteModel);
+        var localCommits = await _apiFixture.DbContext.Set<Commit>().DefaultOrder().ToListAsync();
+        var remoteCommits = await remoteDb.Set<Commit>().DefaultOrder().ToListAsync();
+        localCommits.Count.Should().Be(remoteCommits.Count);
+        for (var i = localCommits.Count - 1; i >= 0; i--)
+        {
+            var localCommit = localCommits[i];
+            var remoteCommit = remoteCommits[i];
+            localCommit.Should().BeEquivalentTo(remoteCommit, "commit index {0} should be the same", i);
+        }
     }
 }

backend/FwLite/LcmCrdt.Tests/Data/RegressionTestHelper.cs

@@ -6,24 +6,19 @@
 
 namespace LcmCrdt.Tests.Data;
 
-public class RegressionTestHelper(string dbName): IAsyncLifetime
+public class RegressionTestHelper(string projectName) : IAsyncLifetime
 {
     private IHost _host = null!;
     private AsyncServiceScope _asyncScope;
+    //unique db path per instance, so CurrentProjectService doesn't think a db has already run migrations
+    private readonly CrdtProject _crdtProject = new(projectName, $"{projectName}-{Guid.NewGuid():N}.sqlite");
     public IServiceProvider Services => _asyncScope.ServiceProvider;
 
     private async Task InitDbFromScripts(RegressionVersion version)
     {
         var initialSqlFile = GetFilePath($"Scripts/{version}.sql");
         var projectsService = _asyncScope.ServiceProvider.GetRequiredService<CurrentProjectService>();
-        var crdtProject = new CrdtProject(dbName, $"{dbName}.sqlite");
-        if (File.Exists(crdtProject.DbPath))
-        {
-            using var clearConn = new SqliteConnection($"Data Source={crdtProject.DbPath}");
-            SqliteConnection.ClearPool(clearConn);
-            File.Delete(crdtProject.DbPath);
-        }
-        projectsService.SetupProjectContextForNewDb(crdtProject);
+        projectsService.SetupProjectContextForNewDb(_crdtProject);
         await using var lcmCrdtDbContext = await _asyncScope.ServiceProvider.GetRequiredService<IDbContextFactory<LcmCrdtDbContext>>().CreateDbContextAsync();
         var sql = await File.ReadAllTextAsync(initialSqlFile);
         var dbConnection = lcmCrdtDbContext.Database.GetDbConnection();
@@ -48,14 +43,24 @@ public Task InitializeAsync()
         return InitializeAsync(RegressionVersion.v2);
     }
 
-    public async Task InitializeAsync(RegressionVersion version)
+    public async Task InitializeAsync(RegressionVersion version, bool withDataMigrations = false)
     {
         var builder = Host.CreateEmptyApplicationBuilder(null);
         builder.Services.AddTestLcmCrdtClient();
         _host = builder.Build();
         var services = _host.Services;
         _asyncScope = services.CreateAsyncScope();
         await InitDbFromScripts(version);
+
+        // Data migrations are already on their way out. It doesn't really make sense for all tests to run them,
+        // which would change a bunch of verified project snapshots.
+        // When data migrations are removed (#2350), we should run this code unconditionally
+        // at the end of InitDbFromScripts (instead of the current db-migration and refresh-project-data approach)
+        // Because that's closer to the prod code.
+        if (withDataMigrations)
+        {
+            await Services.GetRequiredService<CurrentProjectService>().SetupProjectContext(_crdtProject);
+        }
     }
 
     public async Task DisposeAsync()
@@ -66,6 +71,13 @@ public async Task DisposeAsync()
         {
             _host.Dispose();
         }
+
+        if (File.Exists(_crdtProject.DbPath))
+        {
+            using var connection = new SqliteConnection($"Data Source={_crdtProject.DbPath}");
+            SqliteConnection.ClearPool(connection);
+            File.Delete(_crdtProject.DbPath);
+        }
     }
 
     private static string GetFilePath(string name, [CallerFilePath] string sourceFile = "")

backend/FwLite/LcmCrdt.Tests/MiniLcmApiFixture.cs

@@ -14,6 +14,7 @@ namespace LcmCrdt.Tests;
 public class MiniLcmApiFixture : IAsyncLifetime, IAsyncDisposable
 {
     private readonly bool _seedWs = true;
+    private readonly Guid? _projectId;
     private AsyncServiceScope _services;
     private LcmCrdtDbContext? _crdtDbContext;
     public CrdtMiniLcmApi Api => (CrdtMiniLcmApi)_services.ServiceProvider.GetRequiredService<IMiniLcmApi>();
@@ -31,22 +32,23 @@ public MiniLcmApiFixture()
     {
     }
 
-    public static MiniLcmApiFixture Create(bool seedWs = true)
+    public static MiniLcmApiFixture Create(bool seedWs = true, Guid? projectId = null)
     {
-        return new MiniLcmApiFixture(seedWs);
+        return new MiniLcmApiFixture(seedWs, projectId);
     }
 
-    private MiniLcmApiFixture(bool seedWs = true)
+    private MiniLcmApiFixture(bool seedWs = true, Guid? projectId = null)
     {
         _seedWs = seedWs;
+        _projectId = projectId;
     }
 
     public async Task InitializeAsync()
     {
-        await InitializeAsync("sena-3");
+        await InitializeAsync("sena-3", _projectId);
     }
 
-    public async Task InitializeAsync(string projectName)
+    public async Task InitializeAsync(string projectName, Guid? projectId = null)
     {
         var db = $"file:{Guid.NewGuid():N}?mode=memory&cache=shared";
         if (Debugger.IsAttached)
@@ -72,12 +74,10 @@ public async Task InitializeAsync(string projectName)
         _crdtDbContext = await _services.ServiceProvider.GetRequiredService<IDbContextFactory<LcmCrdtDbContext>>().CreateDbContextAsync();
         await _crdtDbContext.Database.OpenConnectionAsync();
         //can't use ProjectsService.CreateProject because it opens and closes the db context, this would wipe out the in memory db.
-        var projectData = new ProjectData("Sena 3", projectName, Guid.NewGuid(), null, Guid.NewGuid());
-        await CrdtProjectsService.InitProjectDb(_crdtDbContext,
-            projectData);
-        await currentProjectService.RefreshProjectData();
-        // CreateProject would also seed morph types — so we need to do it manually here
-        await PreDefinedData.AddPredefinedMorphTypes(_services.ServiceProvider.GetRequiredService<DataModel>(), projectData);
+        var projectData = new ProjectData("Sena 3", projectName, projectId ?? Guid.NewGuid(), null, Guid.NewGuid());
+        await CrdtProjectsService.InitProjectDb(_crdtDbContext, projectData);
+        // Also trigger "data migrations" that CreateProject runs
+        await currentProjectService.SetupProjectContext(crdtProject);
         if (_seedWs)
         {
             await Api.CreateWritingSystem(new WritingSystem()

backend/FwLite/LcmCrdt.Tests/MiniLcmTests/MorphTypeTests.cs

@@ -16,4 +16,14 @@ public override async Task DisposeAsync()
         await base.DisposeAsync();
         await _fixture.DisposeAsync();
     }
+
+    // CRDT only, because fwdata does not support creating morph-types
+    [Fact]
+    public async Task CreateMorphType_Works()
+    {
+        var morphType = CanonicalMorphTypes.All.First().Value.Copy();
+        var createdMorphType = await Api.CreateMorphType(morphType);
+        createdMorphType.Should().NotBeNull();
+        createdMorphType.Should().BeEquivalentTo(morphType);
+    }
 }

backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs

@@ -373,6 +373,14 @@ public async IAsyncEnumerable<MorphType> GetMorphTypes()
         return await repo.MorphTypes.SingleOrDefaultAsync(m => m.Kind == kind);
     }
 
+    public async Task<MorphType> CreateMorphType(MorphType morphType)
+    {
+        //I don't like returning a different object than what the user requested, it feels very unexpected, however this is pretty much what happens in the change anyway and that can't be avoided
+        if (await GetMorphType(morphType.Kind) is {} actualMorphType) return actualMorphType;
+        await AddChange(new CreateMorphTypeChange(morphType));
+        return await GetMorphType(morphType.Id) ?? throw NotFoundException.ForType<MorphType>(morphType.Id);
+    }
+
     public async Task<MorphType> UpdateMorphType(Guid id, UpdateObjectInput<MorphType> update)
     {
         await AddChange(new JsonPatchChange<MorphType>(id, update.Patch));

backend/FwLite/LcmCrdt/CrdtProjectsService.cs

@@ -166,13 +166,15 @@ public virtual async Task<CrdtProject> CreateProject(CreateProjectRequest reques
             crdtProject.Data = projectData;
             await InitProjectDb(db, projectData);
             await currentProjectService.RefreshProjectData();
-            // Morph types are predefined system data that must always exist — seed them
-            // unconditionally so they're available before AfterCreate (e.g. import) runs.
             var dataModel = serviceScope.ServiceProvider.GetRequiredService<DataModel>();
-            await PreDefinedData.AddPredefinedMorphTypes(dataModel, projectData);
             if (request.SeedNewProjectData)
                 await SeedSystemData(dataModel, projectData);
             await (request.AfterCreate?.Invoke(serviceScope.ServiceProvider, crdtProject) ?? Task.CompletedTask);
+            // Ensure "data migrations" are executed on project creation (e.g. seeding morph types)
+            // These should happen AFTER the initial download, so they can be run conditionally based on
+            // the current state of the project.
+            // probably just remove this in #2350
+            await currentProjectService.SetupProjectContext(crdtProject);
         }
         catch (Exception e)
         {
@@ -245,7 +247,7 @@ internal static async Task InitProjectDb(LcmCrdtDbContext db, ProjectData data)
 
     internal static async Task SeedSystemData(DataModel dataModel, ProjectData projectData)
     {
-        // Note: AddPredefinedMorphTypes is seeded unconditionally in CreateProject, not here.
+        await PreDefinedData.AddPredefinedMorphTypes(dataModel, projectData);
         await PreDefinedData.AddPredefinedComplexFormTypes(dataModel, projectData);
         await PreDefinedData.AddPredefinedPartsOfSpeech(dataModel, projectData);
         await PreDefinedData.AddPredefinedSemanticDomains(dataModel, projectData);