diff --git a/Backend.Tests/Controllers/UserEditControllerTests.cs b/Backend.Tests/Controllers/UserEditControllerTests.cs index 9d6d669d4b..37d8f3e281 100644 --- a/Backend.Tests/Controllers/UserEditControllerTests.cs +++ b/Backend.Tests/Controllers/UserEditControllerTests.cs @@ -147,6 +147,25 @@ public async Task TestAddGoalToUserEdit() Assert.That(repoUserEdit, Is.EqualTo(updatedUserEdit).UsingPropertiesComparer()); } + [Test] + public async Task TestUpdateGoalInUserEdit() + { + var userEdit = RandomUserEdit(); + await _userEditRepo.Create(userEdit); + var existingEdit = userEdit.Edits.First(); + + var updatedEdit = existingEdit.Clone(); + updatedEdit.GoalType = existingEdit.GoalType + 1; + updatedEdit.StepData = ["A revised step"]; + await _userEditController.UpdateUserEditGoal(ProjId, userEdit.Id, updatedEdit); + + var repoUserEdit = await _userEditRepo.GetUserEdit(ProjId, userEdit.Id); + Assert.That(repoUserEdit, Is.Not.Null); + Assert.That(repoUserEdit.Edits, Has.Count.EqualTo(1)); + Assert.That(repoUserEdit.Edits.First().GoalType, Is.EqualTo(updatedEdit.GoalType)); + Assert.That(repoUserEdit.Edits.First().StepData, Is.EqualTo(updatedEdit.StepData)); + } + [Test] public async Task TestAddGoalToUserEditNoPermissions() { diff --git a/Backend.Tests/Mocks/UserEditRepositoryMock.cs b/Backend.Tests/Mocks/UserEditRepositoryMock.cs index ef1a98202f..edf4484891 100644 --- a/Backend.Tests/Mocks/UserEditRepositoryMock.cs +++ b/Backend.Tests/Mocks/UserEditRepositoryMock.cs @@ -26,7 +26,7 @@ public Task> GetAllUserEdits(string projectId) { try { - var foundUserEdit = _userEdits.Single(ue => ue.Id == userEditId); + var foundUserEdit = _userEdits.Single(ue => ue.ProjectId == projectId && ue.Id == userEditId); return Task.FromResult(foundUserEdit.Clone()); } catch (InvalidOperationException) @@ -44,21 +44,66 @@ public Task Create(UserEdit userEdit) public Task DeleteAllUserEdits(string projectId) { - _userEdits.Clear(); - return Task.FromResult(true); + var rmCount = _userEdits.RemoveAll(userEdit => userEdit.ProjectId == projectId); + return Task.FromResult(rmCount > 0); } public Task Delete(string projectId, string userEditId) { - var rmCount = _userEdits.RemoveAll(userEdit => userEdit.Id == userEditId); + var rmCount = _userEdits.RemoveAll( + userEdit => userEdit.ProjectId == projectId && userEdit.Id == userEditId); return Task.FromResult(rmCount > 0); } - public Task Replace(string projectId, string userEditId, UserEdit userEdit) + public Task AddEdit(string projectId, string userEditId, Edit edit) { - var rmCount = _userEdits.RemoveAll(ue => ue.Id == userEditId); - _userEdits.Add(userEdit); - return Task.FromResult(rmCount > 0); + var userEdit = _userEdits.Find(ue => ue.ProjectId == projectId && ue.Id == userEditId); + if (userEdit is null) + { + return Task.FromResult(false); + } + userEdit.Edits.Add(edit.Clone()); + return Task.FromResult(true); + } + + public Task ReplaceEdit(string projectId, string userEditId, Edit edit) + { + var userEdit = _userEdits.Find(ue => ue.ProjectId == projectId && ue.Id == userEditId); + // Match the first entry by guid, as MongoDB's UpdateOne does in the real repository. + var editIndex = userEdit?.Edits.FindIndex(e => e.Guid == edit.Guid) ?? -1; + if (userEdit is null || editIndex == -1) + { + return Task.FromResult(false); + } + userEdit.Edits[editIndex] = edit.Clone(); + return Task.FromResult(true); + } + + public Task AddStepToEdit(string projectId, string userEditId, Guid editGuid, string stepData) + { + var userEdit = _userEdits.Find(ue => ue.ProjectId == projectId && ue.Id == userEditId); + var edit = userEdit?.Edits.Find(e => e.Guid == editGuid); + if (edit is null) + { + return Task.FromResult(false); + } + edit.StepData.Add(stepData); + edit.Modified = DateTime.UtcNow; + return Task.FromResult(true); + } + + public Task UpdateStepInEdit( + string projectId, string userEditId, Guid editGuid, int stepIndex, string stepData) + { + var userEdit = _userEdits.Find(ue => ue.ProjectId == projectId && ue.Id == userEditId); + var edit = userEdit?.Edits.Find(e => e.Guid == editGuid); + if (edit is null || stepIndex < 0 || stepIndex >= edit.StepData.Count) + { + return Task.FromResult(false); + } + edit.StepData[stepIndex] = stepData; + edit.Modified = DateTime.UtcNow; + return Task.FromResult(true); } } } diff --git a/Backend.Tests/Repositories/MongoDbSetUpFixture.cs b/Backend.Tests/Repositories/MongoDbSetUpFixture.cs new file mode 100644 index 0000000000..6ef083527d --- /dev/null +++ b/Backend.Tests/Repositories/MongoDbSetUpFixture.cs @@ -0,0 +1,26 @@ +using NUnit.Framework; + +namespace Backend.Tests.Repositories +{ + /// + /// Starts a single shared MongoDB instance for all repository integration tests. + /// Fixtures isolate themselves by using distinct database names. + /// + [SetUpFixture] + public sealed class MongoDbSetUpFixture + { + internal static MongoDbTestRunner Runner { get; private set; } = null!; + + [OneTimeSetUp] + public void StartMongo() + { + Runner = MongoDbTestRunner.Start(); + } + + [OneTimeTearDown] + public void StopMongo() + { + Runner?.Dispose(); + } + } +} diff --git a/Backend.Tests/Repositories/UserEditRepositoryTests.cs b/Backend.Tests/Repositories/UserEditRepositoryTests.cs new file mode 100644 index 0000000000..95417c4a81 --- /dev/null +++ b/Backend.Tests/Repositories/UserEditRepositoryTests.cs @@ -0,0 +1,355 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using BackendFramework.Contexts; +using BackendFramework.Models; +using BackendFramework.Repositories; +using Microsoft.Extensions.Options; +using MongoDB.Bson; +using MongoDB.Driver; +using NUnit.Framework; + +namespace Backend.Tests.Repositories +{ + /// + /// Integration tests for that spin up an actual MongoDB instance. + /// + [TestFixture] + [Category("IntegrationTest")] + public sealed class UserEditRepositoryTests + { + private MongoDbContext _dbContext = null!; + private UserEditRepository _repo = null!; + private IMongoCollection _editsCollection = null!; + private string _projectId = null!; + + [SetUp] + public void SetUp() + { + _projectId = Guid.NewGuid().ToString(); + var options = Options.Create(new BackendFramework.Startup.Settings + { + ConnectionString = MongoDbSetUpFixture.Runner.ConnectionString, + CombineDatabase = "UserEditRepositoryTests", + }); + _dbContext = new MongoDbContext(options); + _repo = new UserEditRepository(_dbContext); + _editsCollection = _dbContext.Db.GetCollection("EditsCollection"); + } + + private Task CreateUserEdit(params Edit[] edits) + { + return _repo.Create(new UserEdit { ProjectId = _projectId, Edits = [.. edits] }); + } + + /// Generates a valid MongoDB ObjectId string that does not exist in the database. + private static string NewObjectId() => ObjectId.GenerateNewId().ToString(); + + /// Gets all EditsCollection documents for a user edit in the test project. + private Task> GetStoredEdits(string userEditId) + { + return _editsCollection + .Find(e => e.ProjectId == _projectId && e.UserEditId == userEditId).ToListAsync(); + } + + /// Counts EditsCollection documents (in any project) for an edit guid. + private async Task CountStoredEditsWithGuid(Guid editGuid) + { + return await _editsCollection.CountDocumentsAsync(e => e.Guid == editGuid); + } + + [Test] + public async Task TestCreateEmptyUserEditSetsId() + { + var userEdit = await CreateUserEdit(); + + Assert.That(userEdit.Id, Is.Not.Empty); + var retrieved = await _repo.GetUserEdit(_projectId, userEdit.Id); + Assert.That(retrieved, Is.Not.Null); + Assert.That(retrieved.Edits, Is.Empty); + } + + [Test] + public async Task TestCreateWithEditsPersistsStoredEditDocuments() + { + var modified = new DateTime(2024, 1, 2, 3, 4, 5, DateTimeKind.Utc); + var editA = new Edit { GoalType = 1, StepData = ["a1", "a2"], Changes = "{\"a\":1}", Modified = modified }; + var editB = new Edit { GoalType = 2, StepData = ["b"], Changes = "{\"b\":2}" }; + + var userEdit = await CreateUserEdit(editA, editB); + + // The returned wire object keeps its edits and gains an id. + Assert.That(userEdit.Id, Is.Not.Empty); + Assert.That(userEdit.Edits, Has.Count.EqualTo(2)); + + // One StoredEdit document per edit, each with the correct projectId and userEditId. + var storedEdits = await GetStoredEdits(userEdit.Id); + Assert.That(storedEdits, Has.Count.EqualTo(2)); + var storedIdsByGuid = storedEdits.ToDictionary(e => e.Guid, e => e.Id); + + // The UserEdit document's edits array holds ObjectId refs to those documents, in order. + var rawUserEdits = _dbContext.Db.GetCollection("UserEditsCollection"); + var rawDoc = await rawUserEdits.Find(new BsonDocument("_id", ObjectId.Parse(userEdit.Id))).FirstAsync(); + var refs = rawDoc["edits"].AsBsonArray; + Assert.That(refs.Select(r => r.BsonType), Is.All.EqualTo(BsonType.ObjectId)); + Assert.That(refs.Select(r => r.AsObjectId.ToString()), + Is.EqualTo(new[] { storedIdsByGuid[editA.Guid], storedIdsByGuid[editB.Guid] })); + + // GetUserEdit round-trips all edit fields. + var retrieved = await _repo.GetUserEdit(_projectId, userEdit.Id); + Assert.That(retrieved, Is.Not.Null); + Assert.That(retrieved.ProjectId, Is.EqualTo(_projectId)); + Assert.That(retrieved.Edits, Is.EqualTo(new[] { editA, editB }).UsingPropertiesComparer()); + } + + [Test] + public async Task TestGetUserEditNonexistentReturnsNull() + { + var retrieved = await _repo.GetUserEdit(_projectId, NewObjectId()); + Assert.That(retrieved, Is.Null); + } + + [Test] + public async Task TestGetUserEditAssemblesEditsInInsertionOrder() + { + var userEdit = await CreateUserEdit(); + var edits = new List { new(), new(), new() }; + foreach (var edit in edits) + { + Assert.That(await _repo.AddEdit(_projectId, userEdit.Id, edit), Is.True); + } + + var retrieved = await _repo.GetUserEdit(_projectId, userEdit.Id); + Assert.That(retrieved, Is.Not.Null); + Assert.That(retrieved.Edits.Select(e => e.Guid), Is.EqualTo(edits.Select(e => e.Guid))); + } + + [Test] + public async Task TestGetAllUserEditsExcludesOtherProjects() + { + var userEditA = await CreateUserEdit(new Edit { StepData = ["a"] }); + var userEditB = await CreateUserEdit(new Edit { StepData = ["b1"] }, new Edit { StepData = ["b2"] }); + var otherUserEdit = await _repo.Create( + new UserEdit { ProjectId = Guid.NewGuid().ToString(), Edits = [new Edit()] }); + + var userEdits = await _repo.GetAllUserEdits(_projectId); + + Assert.That(userEdits, Has.Count.EqualTo(2)); + Assert.That(userEdits.Select(u => u.Id), Does.Not.Contain(otherUserEdit.Id)); + var retrievedA = userEdits.Find(u => u.Id == userEditA.Id); + Assert.That(retrievedA, Is.Not.Null); + Assert.That(retrievedA.Edits, Is.EqualTo(userEditA.Edits).UsingPropertiesComparer()); + var retrievedB = userEdits.Find(u => u.Id == userEditB.Id); + Assert.That(retrievedB, Is.Not.Null); + Assert.That(retrievedB.Edits, Is.EqualTo(userEditB.Edits).UsingPropertiesComparer()); + } + + [Test] + public async Task TestAddEditAppendsToExistingUserEdit() + { + var userEdit = await CreateUserEdit(new Edit { StepData = ["step"] }); + var newEdit = new Edit { GoalType = 2, Changes = "{\"a\":1}" }; + + var result = await _repo.AddEdit(_projectId, userEdit.Id, newEdit); + + Assert.That(result, Is.True); + var retrieved = await _repo.GetUserEdit(_projectId, userEdit.Id); + Assert.That(retrieved, Is.Not.Null); + Assert.That(retrieved.Edits, Has.Count.EqualTo(2)); + Assert.That(retrieved.Edits.Last().Guid, Is.EqualTo(newEdit.Guid)); + + // The EditsCollection gained a document with the correct projectId and userEditId. + var storedEdits = await GetStoredEdits(userEdit.Id); + Assert.That(storedEdits, Has.Count.EqualTo(2)); + Assert.That(storedEdits.Select(e => e.Guid), Does.Contain(newEdit.Guid)); + } + + [Test] + public async Task TestAddEditNonexistentUserEditReturnsFalse() + { + var edit = new Edit(); + var result = await _repo.AddEdit(_projectId, NewObjectId(), edit); + + Assert.That(result, Is.False); + // The transaction must abort without leaving an orphaned EditsCollection document. + Assert.That(await CountStoredEditsWithGuid(edit.Guid), Is.Zero); + } + + [Test] + public async Task TestAddEditWrongProjectIdReturnsFalse() + { + var userEdit = await CreateUserEdit(); + var edit = new Edit(); + var result = await _repo.AddEdit(Guid.NewGuid().ToString(), userEdit.Id, edit); + + Assert.That(result, Is.False); + // The transaction must abort without leaving an orphaned EditsCollection document. + Assert.That(await CountStoredEditsWithGuid(edit.Guid), Is.Zero); + } + + [Test] + public async Task TestReplaceEditReplacesMatchingEdit() + { + var edit = new Edit { GoalType = 1, StepData = ["old"], Changes = "{}" }; + var userEdit = await CreateUserEdit(new Edit(), edit); + var replacement = new Edit + { + Guid = edit.Guid, + GoalType = 3, + StepData = ["new"], + Changes = "{\"b\":2}", + Modified = new DateTime(2024, 5, 6, 7, 8, 9, DateTimeKind.Utc), + }; + + var result = await _repo.ReplaceEdit(_projectId, userEdit.Id, replacement); + + Assert.That(result, Is.True); + var retrieved = await _repo.GetUserEdit(_projectId, userEdit.Id); + Assert.That(retrieved, Is.Not.Null); + Assert.That(retrieved.Edits, Has.Count.EqualTo(2)); + var retrievedEdit = retrieved.Edits.Find(e => e.Guid == edit.Guid); + Assert.That(retrievedEdit, Is.EqualTo(replacement).UsingPropertiesComparer()); + } + + [Test] + public async Task TestReplaceEditIdenticalEditReturnsTrue() + { + var edit = new Edit { GoalType = 1, StepData = ["step"] }; + var userEdit = await CreateUserEdit(edit); + var result = await _repo.ReplaceEdit(_projectId, userEdit.Id, edit); + Assert.That(result, Is.True); + } + + [Test] + public async Task TestReplaceEditUnknownGuidReturnsFalse() + { + var userEdit = await CreateUserEdit(new Edit()); + var result = await _repo.ReplaceEdit(_projectId, userEdit.Id, new Edit()); + Assert.That(result, Is.False); + } + + [Test] + public async Task TestAddStepToEditAppendsToRightEdit() + { + var targetEdit = new Edit { StepData = ["first"] }; + var otherEdit = new Edit { StepData = ["other"] }; + var userEdit = await CreateUserEdit(otherEdit, targetEdit); + + var result = await _repo.AddStepToEdit(_projectId, userEdit.Id, targetEdit.Guid, "second"); + + Assert.That(result, Is.True); + var retrieved = await _repo.GetUserEdit(_projectId, userEdit.Id); + Assert.That(retrieved, Is.Not.Null); + var retrievedTarget = retrieved.Edits.Find(e => e.Guid == targetEdit.Guid); + Assert.That(retrievedTarget, Is.Not.Null); + Assert.That(retrievedTarget.StepData, Is.EqualTo(["first", "second"])); + Assert.That(retrievedTarget.Modified, Is.Not.Null); + var retrievedOther = retrieved.Edits.Find(e => e.Guid == otherEdit.Guid); + Assert.That(retrievedOther, Is.Not.Null); + Assert.That(retrievedOther.StepData, Is.EqualTo(["other"])); + } + + [Test] + public async Task TestAddStepToEditUnknownGuidReturnsFalse() + { + var userEdit = await CreateUserEdit(new Edit()); + var result = await _repo.AddStepToEdit(_projectId, userEdit.Id, Guid.NewGuid(), "step"); + Assert.That(result, Is.False); + } + + [Test] + public async Task TestUpdateStepInEditOverwritesRightIndex() + { + var edit = new Edit { StepData = ["a", "b", "c"] }; + var userEdit = await CreateUserEdit(edit); + + var result = await _repo.UpdateStepInEdit(_projectId, userEdit.Id, edit.Guid, 1, "B"); + + Assert.That(result, Is.True); + var retrieved = await _repo.GetUserEdit(_projectId, userEdit.Id); + Assert.That(retrieved, Is.Not.Null); + var retrievedEdit = retrieved.Edits.Find(e => e.Guid == edit.Guid); + Assert.That(retrievedEdit, Is.Not.Null); + Assert.That(retrievedEdit.StepData, Is.EqualTo(["a", "B", "c"])); + Assert.That(retrievedEdit.Modified, Is.Not.Null); + } + + [Test] + public async Task TestUpdateStepInEditUnknownGuidReturnsFalse() + { + var userEdit = await CreateUserEdit(new Edit { StepData = ["a"] }); + var result = await _repo.UpdateStepInEdit(_projectId, userEdit.Id, Guid.NewGuid(), 0, "A"); + Assert.That(result, Is.False); + } + + [Test] + public async Task TestUpdateStepInEditOutOfBoundsReturnsFalseWithoutPadding() + { + var edit = new Edit { StepData = ["a"] }; + var userEdit = await CreateUserEdit(edit); + + var result = await _repo.UpdateStepInEdit(_projectId, userEdit.Id, edit.Guid, 2, "C"); + + Assert.That(result, Is.False); + var retrieved = await _repo.GetUserEdit(_projectId, userEdit.Id); + Assert.That(retrieved, Is.Not.Null); + var retrievedEdit = retrieved.Edits.Find(e => e.Guid == edit.Guid); + Assert.That(retrievedEdit, Is.Not.Null); + Assert.That(retrievedEdit.StepData, Is.EqualTo(["a"])); + } + + [Test] + public async Task TestDeleteRemovesUserEditAndItsEdits() + { + var userEdit = await CreateUserEdit(new Edit(), new Edit()); + var otherUserEdit = await CreateUserEdit(new Edit()); + + var result = await _repo.Delete(_projectId, userEdit.Id); + + Assert.That(result, Is.True); + Assert.That(await _repo.GetUserEdit(_projectId, userEdit.Id), Is.Null); + Assert.That(await GetStoredEdits(userEdit.Id), Is.Empty); + + // Another user edit in the same project is untouched. + Assert.That(await GetStoredEdits(otherUserEdit.Id), Has.Count.EqualTo(1)); + var retrievedOther = await _repo.GetUserEdit(_projectId, otherUserEdit.Id); + Assert.That(retrievedOther, Is.Not.Null); + Assert.That(retrievedOther.Edits, Has.Count.EqualTo(1)); + } + + [Test] + public async Task TestDeleteNonexistentUserEditReturnsFalse() + { + var result = await _repo.Delete(_projectId, NewObjectId()); + Assert.That(result, Is.False); + } + + [Test] + public async Task TestDeleteAllUserEditsRemovesProjectDocuments() + { + await CreateUserEdit(new Edit()); + await CreateUserEdit(new Edit(), new Edit()); + var otherProjectId = Guid.NewGuid().ToString(); + var otherUserEdit = await _repo.Create(new UserEdit { ProjectId = otherProjectId, Edits = [new Edit()] }); + + var result = await _repo.DeleteAllUserEdits(_projectId); + + Assert.That(result, Is.True); + Assert.That(await _repo.GetAllUserEdits(_projectId), Is.Empty); + Assert.That(await _editsCollection.CountDocumentsAsync(e => e.ProjectId == _projectId), Is.Zero); + + // Another project's documents are untouched. + var retrievedOther = await _repo.GetUserEdit(otherProjectId, otherUserEdit.Id); + Assert.That(retrievedOther, Is.Not.Null); + Assert.That(retrievedOther.Edits, Has.Count.EqualTo(1)); + } + + [Test] + public async Task TestDeleteAllUserEditsEmptyProjectReturnsFalse() + { + var result = await _repo.DeleteAllUserEdits(_projectId); + Assert.That(result, Is.False); + } + } +} diff --git a/Backend.Tests/Repositories/WordRepositoryTests.cs b/Backend.Tests/Repositories/WordRepositoryTests.cs index e87e541360..d6607f4cf2 100644 --- a/Backend.Tests/Repositories/WordRepositoryTests.cs +++ b/Backend.Tests/Repositories/WordRepositoryTests.cs @@ -17,30 +17,16 @@ namespace Backend.Tests.Repositories [Category("IntegrationTest")] public sealed class WordRepositoryTests { - private static MongoDbTestRunner _runner = null!; private WordRepository _repo = null!; private string _projectId = null!; - [OneTimeSetUp] - public static void StartMongo() - { - _runner?.Dispose(); - _runner = MongoDbTestRunner.Start(); - } - - [OneTimeTearDown] - public static void StopMongo() - { - _runner?.Dispose(); - } - [SetUp] public void SetUp() { _projectId = Guid.NewGuid().ToString(); var options = Options.Create(new BackendFramework.Startup.Settings { - ConnectionString = _runner.ConnectionString, + ConnectionString = MongoDbSetUpFixture.Runner.ConnectionString, CombineDatabase = "WordRepositoryTests", }); _repo = new WordRepository(new MongoDbContext(options)); diff --git a/Backend/Interfaces/IUserEditRepository.cs b/Backend/Interfaces/IUserEditRepository.cs index 7742eee459..9e152b7253 100644 --- a/Backend/Interfaces/IUserEditRepository.cs +++ b/Backend/Interfaces/IUserEditRepository.cs @@ -1,4 +1,5 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using System.Threading.Tasks; using BackendFramework.Models; @@ -11,6 +12,9 @@ public interface IUserEditRepository Task Create(UserEdit userEdit); Task Delete(string projectId, string userEditId); Task DeleteAllUserEdits(string projectId); - Task Replace(string projectId, string userEditId, UserEdit userEdit); + Task AddEdit(string projectId, string userEditId, Edit edit); + Task ReplaceEdit(string projectId, string userEditId, Edit edit); + Task AddStepToEdit(string projectId, string userEditId, Guid editGuid, string stepData); + Task UpdateStepInEdit(string projectId, string userEditId, Guid editGuid, int stepIndex, string stepData); } } diff --git a/Backend/Models/UserEdit.cs b/Backend/Models/UserEdit.cs index 5e572be237..afd6166be0 100644 --- a/Backend/Models/UserEdit.cs +++ b/Backend/Models/UserEdit.cs @@ -87,4 +87,79 @@ public Edit Clone() return clone; } } + + /// + /// The persisted form of a in the UserEditsCollection, with each edit stored as a + /// reference to a document so the document cannot grow toward MongoDB's 16 MB limit. + /// + public class StoredUserEdit + { + [BsonId] + [BsonRepresentation(BsonType.ObjectId)] + public string Id { get; set; } = ""; + + /// Ids of this user edit's documents, in order. + [BsonElement("edits")] + [BsonRepresentation(BsonType.ObjectId)] + public List EditIds { get; set; } = []; + + [BsonElement("projectId")] + public string ProjectId { get; set; } = ""; + } + + /// The persisted form of an : one document in the EditsCollection. + public class StoredEdit + { + [BsonId] + [BsonRepresentation(BsonType.ObjectId)] + public string Id { get; set; } = ""; + + [BsonElement("projectId")] + public string ProjectId { get; set; } = ""; + + /// Id of the document this edit belongs to. + [BsonElement("userEditId")] + public string UserEditId { get; set; } = ""; + + [BsonElement("guid")] + [BsonGuidRepresentation(GuidRepresentation.Standard)] +#pragma warning disable CA1720 + public Guid Guid { get; set; } = Guid.NewGuid(); +#pragma warning restore CA1720 + + [BsonElement("goalType")] + public int GoalType { get; set; } + + [BsonElement("stepData")] + public List StepData { get; set; } = []; + + [BsonElement("changes")] + public string Changes { get; set; } = "{}"; + + [BsonElement("modified")] + public DateTime? Modified { get; set; } + + public StoredEdit() { } + + public StoredEdit(string projectId, string userEditId, Edit edit) + { + ProjectId = projectId; + UserEditId = userEditId; + Guid = edit.Guid; + GoalType = edit.GoalType; + StepData = [.. edit.StepData]; + Changes = edit.Changes; + Modified = edit.Modified; + } + + /// Convert to the API-facing shape. + public Edit ToEdit() => new() + { + Guid = Guid, + GoalType = GoalType, + StepData = [.. StepData], + Changes = Changes, + Modified = Modified, + }; + } } diff --git a/Backend/Repositories/UserEditRepository.cs b/Backend/Repositories/UserEditRepository.cs index d0570ceb65..617cfd0939 100644 --- a/Backend/Repositories/UserEditRepository.cs +++ b/Backend/Repositories/UserEditRepository.cs @@ -1,10 +1,12 @@ using System; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; +using System.Linq; using System.Threading.Tasks; using BackendFramework.Interfaces; using BackendFramework.Models; using BackendFramework.Otel; +using MongoDB.Bson; using MongoDB.Driver; namespace BackendFramework.Repositories @@ -13,17 +15,71 @@ namespace BackendFramework.Repositories [ExcludeFromCodeCoverage] public class UserEditRepository(IMongoDbContext dbContext) : IUserEditRepository { - private readonly IMongoCollection _userEdits = - dbContext.Db.GetCollection("UserEditsCollection"); + private readonly IMongoDbContext _dbContext = dbContext; + private readonly IMongoCollection _userEdits = + dbContext.Db.GetCollection("UserEditsCollection"); + private readonly IMongoCollection _edits = + dbContext.Db.GetCollection("EditsCollection"); private const string otelTagName = "otel.UserEditRepository"; + #region Private helper methods + + /// Creates a mongo filter for the in a specified project. + private static FilterDefinition GetUserEditFilter(string projectId, string userEditId) + { + var filterDef = new FilterDefinitionBuilder(); + return filterDef.And(filterDef.Eq(u => u.ProjectId, projectId), filterDef.Eq(u => u.Id, userEditId)); + } + + /// Creates a mongo filter for all of a user edit's s. + private static FilterDefinition GetEditsFilter(string projectId, string userEditId) + { + var filterDef = new FilterDefinitionBuilder(); + return filterDef.And( + filterDef.Eq(e => e.ProjectId, projectId), filterDef.Eq(e => e.UserEditId, userEditId)); + } + + /// Creates a mongo filter for a user edit's with specified guid. + private static FilterDefinition GetEditFilter(string projectId, string userEditId, Guid editGuid) + { + var filterDef = new FilterDefinitionBuilder(); + return filterDef.And( + filterDef.Eq(e => e.ProjectId, projectId), + filterDef.Eq(e => e.UserEditId, userEditId), + filterDef.Eq(e => e.Guid, editGuid)); + } + + /// + /// Assembles a with edits in the order of the stored references, + /// skipping any dangling references. + /// + private static UserEdit AssembleUserEdit(StoredUserEdit storedUserEdit, List storedEdits) + { + var editsById = storedEdits.ToDictionary(e => e.Id); + var edits = new List(); + foreach (var editId in storedUserEdit.EditIds) + { + if (editsById.TryGetValue(editId, out var storedEdit)) + { + edits.Add(storedEdit.ToEdit()); + } + } + return new UserEdit { Id = storedUserEdit.Id, ProjectId = storedUserEdit.ProjectId, Edits = edits }; + } + + #endregion + /// Finds all s with specified projectId public async Task> GetAllUserEdits(string projectId) { using var activity = OtelService.StartActivityWithTag(otelTagName, "getting all user edits"); - return await _userEdits.Find(u => u.ProjectId == projectId).ToListAsync(); + var storedUserEdits = await _userEdits.Find(u => u.ProjectId == projectId).ToListAsync(); + var storedEdits = await _edits.Find(e => e.ProjectId == projectId).ToListAsync(); + var editsByUserEditId = storedEdits.GroupBy(e => e.UserEditId).ToDictionary(g => g.Key, g => g.ToList()); + return storedUserEdits.ConvertAll( + u => AssembleUserEdit(u, editsByUserEditId.GetValueOrDefault(u.Id, []))); } /// Removes all s for specified @@ -32,29 +88,33 @@ public async Task DeleteAllUserEdits(string projectId) { using var activity = OtelService.StartActivityWithTag(otelTagName, "deleting all user edits"); - var deleted = await _userEdits.DeleteManyAsync(u => u.ProjectId == projectId); - return deleted.DeletedCount != 0; + return await _dbContext.ExecuteInTransaction(async s => + { + await _edits.DeleteManyAsync(s, e => e.ProjectId == projectId); + var deleted = await _userEdits.DeleteManyAsync(s, u => u.ProjectId == projectId); + return deleted.DeletedCount != 0; + }); } - /// Finds with specified userRoleId and projectId + /// Finds with specified userEditId and projectId public async Task GetUserEdit(string projectId, string userEditId) { using var activity = OtelService.StartActivityWithTag(otelTagName, "getting a user edit"); - var filterDef = new FilterDefinitionBuilder(); - var filter = filterDef.And(filterDef.Eq( - x => x.ProjectId, projectId), filterDef.Eq(x => x.Id, userEditId)); - - var userEditList = await _userEdits.FindAsync(filter); + var userEditList = await _userEdits.FindAsync(GetUserEditFilter(projectId, userEditId)); + StoredUserEdit storedUserEdit; try { - return await userEditList.FirstAsync(); + storedUserEdit = await userEditList.FirstAsync(); } catch (InvalidOperationException) { return null; } + + var storedEdits = await _edits.Find(GetEditsFilter(projectId, userEditId)).ToListAsync(); + return AssembleUserEdit(storedUserEdit, storedEdits); } /// Adds a @@ -63,36 +123,120 @@ public async Task Create(UserEdit userEdit) { using var activity = OtelService.StartActivityWithTag(otelTagName, "creating a user edit"); - await _userEdits.InsertOneAsync(userEdit); + var storedUserEdit = new StoredUserEdit { ProjectId = userEdit.ProjectId }; + if (userEdit.Edits.Count == 0) + { + await _userEdits.InsertOneAsync(storedUserEdit); + } + else + { + // Pre-generate the id so the StoredEdit documents can reference their parent. + storedUserEdit.Id = ObjectId.GenerateNewId().ToString(); + var storedEdits = userEdit.Edits.ConvertAll( + e => new StoredEdit(userEdit.ProjectId, storedUserEdit.Id, e)); + await _dbContext.ExecuteInTransaction(async s => + { + await _edits.InsertManyAsync(s, storedEdits); + storedUserEdit.EditIds = storedEdits.ConvertAll(e => e.Id); + await _userEdits.InsertOneAsync(s, storedUserEdit); + return true; + }); + } + + userEdit.Id = storedUserEdit.Id; return userEdit; } - /// Removes with specified userRoleId and projectId + /// Removes with specified userEditId and projectId /// A bool: success of operation public async Task Delete(string projectId, string userEditId) { using var activity = OtelService.StartActivityWithTag(otelTagName, "deleting a user edit"); - var filterDef = new FilterDefinitionBuilder(); - var filter = filterDef.And(filterDef.Eq( - x => x.ProjectId, projectId), filterDef.Eq(x => x.Id, userEditId)); + return await _dbContext.ExecuteInTransaction(async s => + { + await _edits.DeleteManyAsync(s, GetEditsFilter(projectId, userEditId)); + var deleted = await _userEdits.DeleteOneAsync(s, GetUserEditFilter(projectId, userEditId)); + return deleted.DeletedCount > 0; + }); + } + + /// + /// Appends an to the with specified userEditId and projectId. + /// + /// A bool: success of operation + public async Task AddEdit(string projectId, string userEditId, Edit edit) + { + using var activity = OtelService.StartActivityWithTag(otelTagName, "adding an edit to a user edit"); - var deleted = await _userEdits.DeleteOneAsync(filter); - return deleted.DeletedCount > 0; + var storedEdit = new StoredEdit(projectId, userEditId, edit); + // A null result aborts the transaction, so no orphaned edit document is left behind + // when the user edit doesn't exist. + return await _dbContext.ExecuteInTransactionAllowNull(async s => + { + await _edits.InsertOneAsync(s, storedEdit); + var update = Builders.Update.Push(u => u.EditIds, storedEdit.Id); + var result = await _userEdits.UpdateOneAsync(s, GetUserEditFilter(projectId, userEditId), update); + return result.IsAcknowledged && result.MatchedCount == 1 ? true : null; + }) ?? false; } - /// Replaces with specified userRoleId and projectId + /// + /// Replaces the contents of the with matching guid in the + /// with specified userEditId and projectId. + /// /// A bool: success of operation - public async Task Replace(string projectId, string userEditId, UserEdit userEdit) + public async Task ReplaceEdit(string projectId, string userEditId, Edit edit) { - using var activity = OtelService.StartActivityWithTag(otelTagName, "replacing a user edit"); + using var activity = OtelService.StartActivityWithTag(otelTagName, "replacing an edit in a user edit"); + + var update = Builders.Update + .Set(e => e.GoalType, edit.GoalType) + .Set(e => e.StepData, edit.StepData) + .Set(e => e.Changes, edit.Changes) + .Set(e => e.Modified, edit.Modified); + var result = await _edits.UpdateOneAsync(GetEditFilter(projectId, userEditId, edit.Guid), update); + // MatchedCount, not ModifiedCount: replacing an edit with an identical one is still a success. + return result.IsAcknowledged && result.MatchedCount == 1; + } - var filterDef = new FilterDefinitionBuilder(); - var filter = filterDef.And(filterDef.Eq( - x => x.ProjectId, projectId), filterDef.Eq(x => x.Id, userEditId)); + /// + /// Appends a step to the with matching guid in the + /// with specified userEditId and projectId. + /// + /// A bool: success of operation + public async Task AddStepToEdit(string projectId, string userEditId, Guid editGuid, string stepData) + { + using var activity = OtelService.StartActivityWithTag(otelTagName, "adding a step to an edit"); + + var update = Builders.Update + .Push(e => e.StepData, stepData) + .Set(e => e.Modified, DateTime.UtcNow); + var result = await _edits.UpdateOneAsync(GetEditFilter(projectId, userEditId, editGuid), update); + return result.IsAcknowledged && result.MatchedCount == 1; + } - var result = await _userEdits.ReplaceOneAsync(filter, userEdit); - return result.ModifiedCount == 1; + /// + /// Overwrites the step at the given index of the with matching guid + /// in the with specified userEditId and projectId. + /// + /// A bool: success of operation (false if the step doesn't exist) + public async Task UpdateStepInEdit( + string projectId, string userEditId, Guid editGuid, int stepIndex, string stepData) + { + using var activity = OtelService.StartActivityWithTag(otelTagName, "updating a step in an edit"); + + // Requiring the step to exist makes the bounds check atomic with the write; + // a $set beyond the array's end would pad the array with nulls instead of failing. + var filterDef = new FilterDefinitionBuilder(); + var filter = filterDef.And( + GetEditFilter(projectId, userEditId, editGuid), filterDef.Exists($"stepData.{stepIndex}")); + + var update = Builders.Update + .Set($"stepData.{stepIndex}", stepData) + .Set(e => e.Modified, DateTime.UtcNow); + var result = await _edits.UpdateOneAsync(filter, update); + return result.IsAcknowledged && result.MatchedCount == 1; } } } diff --git a/Backend/Services/UserEditService.cs b/Backend/Services/UserEditService.cs index 6418bb5606..821b359fcf 100644 --- a/Backend/Services/UserEditService.cs +++ b/Backend/Services/UserEditService.cs @@ -24,58 +24,32 @@ public UserEditService(IUserEditRepository userEditRepo) /// /// /// Tuple of - /// bool: true if Edit added/updated, false if nothing modified - /// Guid?: guid of added/updated Edit, or null if UserEdit not found + /// bool: true if the Edit was added or updated, false if the UserEdit was not found + /// Guid?: guid of the added/updated Edit, or null if the UserEdit was not found /// public async Task> AddGoalToUserEdit(string projectId, string userEditId, Edit edit) { using var activity = OtelService.StartActivityWithTag(otelTagName, "adding goal to user edit"); - // Get userEdit to change - var userEdit = await _userEditRepo.GetUserEdit(projectId, userEditId); - if (userEdit is null) - { - return new Tuple(false, null); - } - edit.Modified = DateTime.UtcNow; - // Update existing Edit if guid exists, otherwise add new one at end of List. - var editIndex = userEdit.Edits.FindLastIndex(e => e.Guid == edit.Guid); - if (editIndex > -1) - { - userEdit.Edits[editIndex] = edit; - } - else - { - userEdit.Edits.Add(edit); - } - - // Replace the old UserEdit object with the new one that contains the new/updated edit - var editReplaced = await _userEditRepo.Replace(projectId, userEditId, userEdit); + // Replace the Edit with this guid if present, otherwise add it. Each repo call is atomic on its + // own; taking the branch from ReplaceEdit's result rather than a prior read closes the stale-read + // race. Two concurrent first-time writes of the same new guid could still both add it, but guids + // are client-generated and unique per goal, so that window is tolerated rather than locked. + var isSuccess = await _userEditRepo.ReplaceEdit(projectId, userEditId, edit) + || await _userEditRepo.AddEdit(projectId, userEditId, edit); - return new Tuple(editReplaced, edit.Guid); + return new Tuple(isSuccess, isSuccess ? edit.Guid : null); } /// Adds a string representation of a step to a specified - /// A bool: success of operation + /// A bool: success of operation (false if userEdit or edit not found) public async Task AddStepToGoal(string projectId, string userEditId, Guid editGuid, string stepString) { using var activity = OtelService.StartActivityWithTag(otelTagName, "adding step to goal"); - var userEdit = await _userEditRepo.GetUserEdit(projectId, userEditId); - if (userEdit is null) - { - return false; - } - var edit = userEdit.Edits.FindLast(e => e.Guid == editGuid); - if (edit is null) - { - return false; - } - edit.Modified = DateTime.UtcNow; - edit.StepData.Add(stepString); - return await _userEditRepo.Replace(projectId, userEditId, userEdit); + return await _userEditRepo.AddStepToEdit(projectId, userEditId, editGuid, stepString); } /// Updates a specified step in a specified @@ -89,19 +63,9 @@ public async Task UpdateStepInGoal( { return false; } - var userEdit = await _userEditRepo.GetUserEdit(projectId, userEditId); - if (userEdit is null) - { - return false; - } - var edit = userEdit.Edits.FindLast(e => e.Guid == editGuid); - if (edit is null || stepIndex >= edit.StepData.Count) - { - return false; - } - edit.Modified = DateTime.UtcNow; - edit.StepData[stepIndex] = stepString; - return await _userEditRepo.Replace(projectId, userEditId, userEdit); + + // The repository bounds-checks stepIndex atomically with the write. + return await _userEditRepo.UpdateStepInEdit(projectId, userEditId, editGuid, stepIndex, stepString); } } } diff --git a/database/migrate-useredits-to-edits-collection.js b/database/migrate-useredits-to-edits-collection.js new file mode 100644 index 0000000000..5e823365dc --- /dev/null +++ b/database/migrate-useredits-to-edits-collection.js @@ -0,0 +1,120 @@ +// Migration script: move each embedded edit of UserEditsCollection into its own +// document in EditsCollection, leaving the UserEdit's `edits` array holding only +// ObjectId refs (in the original order). +// +// Usage (local): +// mongosh CombineDatabase database/migrate-useredits-to-edits-collection.js +// +// Usage (Kubernetes, e.g. production): +// kubectl -n thecombine cp database/migrate-useredits-to-edits-collection.js \ +// :/tmp/migrate-useredits-to-edits-collection.js +// kubectl -n thecombine exec -- \ +// mongosh CombineDatabase /tmp/migrate-useredits-to-edits-collection.js +// +// IMPORTANT: +// - Back up the database first (e.g., maintenance/scripts/combine_backup.py, or at +// minimum `mongodump --db=CombineDatabase --collection=UserEditsCollection`). +// - This is a BREAKING schema change: run it while the backend is stopped/scaled +// down, then deploy the backend version that reads the new schema. Old backends +// cannot read migrated documents, and the new backend cannot read unmigrated ones. +// - The script is idempotent: it only touches documents still in the old format, +// and clears partial output from any interrupted previous run before redoing it. +// - If an old (pre-migration) backup is ever restored, rerun this script. +// +// Background (https://github.com/sillsdev/TheCombine/issues/4320): +// UserEdit documents grow with every goal a user works on; documents approaching +// MongoDB's 16 MB limit reject all further writes (WriteError 17419), permanently +// blocking the user's goal/step progress in that project. With one document per +// edit, no document grows meaningfully with use. UserEdit documents are modified +// in place — never deleted or recreated, since each user's `workedProjects` map +// references the document `_id`. + +// New-format EditsCollection document (must match StoredEdit in Backend/Models/UserEdit.cs): +// { _id: ObjectId, projectId: string, userEditId: string, +// guid: UUID, goalType: int, stepData: [string], changes: string, modified: Date? } +// Migrated UserEditsCollection `edits` field (must match StoredUserEdit): [ObjectId, ...] + +var userEdits = db.getCollection("UserEditsCollection"); +var edits = db.getCollection("EditsCollection"); + +// Old-format documents have embedded edit objects (each with a `guid` field); +// migrated documents hold plain ObjectIds, which have no subfields. +var oldFormat = { "edits.0.guid": { $exists: true } }; + +// Gather ids up front so the update inside the loop can't disturb the cursor, +// and so multi-MB documents are held in memory only one at a time. +var idsToMigrate = userEdits.find(oldFormat, { _id: 1 }).toArray().map(function (d) { + return d._id; +}); +print("UserEdit documents to migrate: " + idsToMigrate.length); + +var totalEditsMoved = 0; +idsToMigrate.forEach(function (id) { + var doc = userEdits.findOne({ _id: id }); + var userEditId = id.toHexString(); + + // Clear any partial output from a previous interrupted run: while the document + // is still old-format, every EditsCollection row for it is a leftover. + edits.deleteMany({ userEditId: userEditId }); + + var refs = []; + var newDocs = doc.edits.map(function (e) { + var newDoc = { + _id: new ObjectId(), + projectId: doc.projectId, + userEditId: userEditId, + guid: e.guid, + goalType: e.goalType, + stepData: e.stepData, + changes: e.changes, + }; + if ("modified" in e) { + newDoc.modified = e.modified; + } + refs.push(newDoc._id); + return newDoc; + }); + + if (newDocs.length > 0) { + edits.insertMany(newDocs, { ordered: true }); + } + userEdits.updateOne({ _id: id }, { $set: { edits: refs } }); + + totalEditsMoved += newDocs.length; + print( + " migrated " + userEditId + " (projectId: " + doc.projectId + "): " + + newDocs.length + " edit(s), was " + + (Object.bsonsize(doc) / (1024 * 1024)).toFixed(2) + " MB" + ); +}); +print("Moved " + totalEditsMoved + " edit(s) from " + idsToMigrate.length + " document(s)."); + +// Backend queries filter by projectId + userEditId (list a user edit's edits) and by +// projectId + userEditId + guid (replace/update a single edit). One compound index serves +// both: the leading fields cover the former, the full key covers the latter. +edits.createIndex({ projectId: 1, userEditId: 1, guid: 1 }); +print("Ensured index { projectId: 1, userEditId: 1, guid: 1 } on EditsCollection."); + +// Verify: no old-format documents remain, and every ref resolves. +var failures = 0; +if (userEdits.countDocuments(oldFormat) > 0) { + failures++; + print("WARNING: old-format UserEdit documents remain."); +} +userEdits.find().forEach(function (doc) { + var refCount = doc.edits.length; + var docCount = edits.countDocuments({ userEditId: doc._id.toHexString() }); + if (refCount !== docCount) { + failures++; + print( + "WARNING: UserEdit " + doc._id.toHexString() + " has " + refCount + + " ref(s) but " + docCount + " EditsCollection document(s)." + ); + } +}); +if (failures === 0) { + print( + "Verification passed: " + userEdits.countDocuments({}) + " UserEdit document(s), " + + edits.countDocuments({}) + " EditsCollection document(s), all refs resolve." + ); +} diff --git a/maintenance/scripts/rm_project.py b/maintenance/scripts/rm_project.py index 6983918be4..95d83dea50 100755 --- a/maintenance/scripts/rm_project.py +++ b/maintenance/scripts/rm_project.py @@ -20,6 +20,7 @@ from combine_app import CombineApp collections_with_project_id = ( + "EditsCollection", "FrontierCollection", "MergeBlacklistCollection", "MergeGraylistCollection",