Skip to content

Commit

Permalink
Address most review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@rmunn
rmunn committed Jan 8, 2025
1 parent 922f0a0 commit c3f0908
Show file tree
Hide file tree
Showing 8 changed files with 51 additions and 94 deletions.
42 changes: 21 additions & 21 deletions backend/FwLite/FwDataMiniLcmBridge/Api/FwDataMiniLcmApi.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,9 @@ internal void CompleteExemplars(WritingSystems writingSystems)
}
}

public Task<WritingSystem> CreateWritingSystem(WritingSystemType type, WritingSystem writingSystem)
public async Task<WritingSystem> CreateWritingSystem(WritingSystemType type, WritingSystem writingSystem)
{
validators.ValidateAndThrow(writingSystem);
await validators.ValidateAndThrow(writingSystem);
var exitingWs = type == WritingSystemType.Analysis ? Cache.ServiceLocator.WritingSystems.AnalysisWritingSystems : Cache.ServiceLocator.WritingSystems.VernacularWritingSystems;
if (exitingWs.Any(ws => ws.Id == writingSystem.WsId))
{
Expand Down Expand Up @@ -195,7 +195,7 @@ public Task<WritingSystem> CreateWritingSystem(WritingSystemType type, WritingSy
WritingSystemType.Vernacular => WritingSystemContainer.CurrentVernacularWritingSystems.Count,
_ => throw new ArgumentOutOfRangeException(nameof(type), type, null)
} - 1;
return Task.FromResult(FromLcmWritingSystem(ws, index, type));
return FromLcmWritingSystem(ws, index, type);
}

public async Task<WritingSystem> UpdateWritingSystem(WritingSystemId id, WritingSystemType type, UpdateObjectInput<WritingSystem> update)
Expand All @@ -221,7 +221,7 @@ await Cache.DoUsingNewOrCurrentUOW("Update WritingSystem",

public async Task<WritingSystem> UpdateWritingSystem(WritingSystem before, WritingSystem after)
{
await validators.ValidateAndThrowAsync(after);
await validators.ValidateAndThrow(after);
await Cache.DoUsingNewOrCurrentUOW("Update WritingSystem",
"Revert WritingSystem",
async () =>
Expand All @@ -248,9 +248,9 @@ public IAsyncEnumerable<PartOfSpeech> GetPartsOfSpeech()
? FromLcmPartOfSpeech(partOfSpeech) : null);
}

public Task<PartOfSpeech> CreatePartOfSpeech(PartOfSpeech partOfSpeech)
public async Task<PartOfSpeech> CreatePartOfSpeech(PartOfSpeech partOfSpeech)
{
validators.ValidateAndThrow(partOfSpeech);
await validators.ValidateAndThrow(partOfSpeech);
IPartOfSpeech? lcmPartOfSpeech = null;
if (partOfSpeech.Id == default) partOfSpeech.Id = Guid.NewGuid();
UndoableUnitOfWorkHelper.DoUsingNewOrCurrentUOW("Create Part of Speech",
Expand All @@ -262,7 +262,7 @@ public Task<PartOfSpeech> CreatePartOfSpeech(PartOfSpeech partOfSpeech)
.Create(partOfSpeech.Id, Cache.LangProject.PartsOfSpeechOA);
UpdateLcmMultiString(lcmPartOfSpeech.Name, partOfSpeech.Name);
});
return Task.FromResult(FromLcmPartOfSpeech(lcmPartOfSpeech ?? throw new InvalidOperationException("Part of speech was not created")));
return FromLcmPartOfSpeech(lcmPartOfSpeech ?? throw new InvalidOperationException("Part of speech was not created"));
}

public Task<PartOfSpeech> UpdatePartOfSpeech(Guid id, UpdateObjectInput<PartOfSpeech> update)
Expand All @@ -281,7 +281,7 @@ public Task<PartOfSpeech> UpdatePartOfSpeech(Guid id, UpdateObjectInput<PartOfSp

public async Task<PartOfSpeech> UpdatePartOfSpeech(PartOfSpeech before, PartOfSpeech after)
{
await validators.ValidateAndThrowAsync(after);
await validators.ValidateAndThrow(after);
await PartOfSpeechSync.Sync(before, after, this);
return await GetPartOfSpeech(after.Id) ?? throw new NullReferenceException($"unable to find part of speech with id {after.Id}");
}
Expand Down Expand Up @@ -402,7 +402,7 @@ private ComplexFormType ToComplexFormType(ILexEntryType t)

public async Task<ComplexFormType> CreateComplexFormType(ComplexFormType complexFormType)
{
await validators.ValidateAndThrowAsync(complexFormType);
await validators.ValidateAndThrow(complexFormType);
if (complexFormType.Id == default) complexFormType.Id = Guid.NewGuid();
UndoableUnitOfWorkHelper.DoUsingNewOrCurrentUOW("Create complex form type",
"Remove complex form type",
Expand Down Expand Up @@ -435,7 +435,7 @@ public Task<ComplexFormType> UpdateComplexFormType(Guid id, UpdateObjectInput<Co

public async Task<ComplexFormType> UpdateComplexFormType(ComplexFormType before, ComplexFormType after)
{
await validators.ValidateAndThrowAsync(after);
await validators.ValidateAndThrow(after);
await ComplexFormTypeSync.Sync(before, after, this);
return ToComplexFormType(ComplexFormTypesFlattened.Single(c => c.Guid == after.Id));
}
Expand Down Expand Up @@ -467,7 +467,7 @@ private PartOfSpeech FromLcmPartOfSpeech(IPartOfSpeech lcmPos)
Id = lcmPos.Guid,
Name = FromLcmMultiString(lcmPos.Name),
// TODO: Abreviation = FromLcmMultiString(partOfSpeech.Abreviation),
Predefined = true, // NOTE: the !string.IsNullOrEmpty(lcmPos.CatalogSourceId) check doesn't work if the PoS originated in CRDT
Predefined = CanonicalGuidsPartOfSpeech.CanonicalPosGuids.Contains(lcmPos.Guid),
};
}

Expand Down Expand Up @@ -667,7 +667,7 @@ public IAsyncEnumerable<Entry> SearchEntries(string query, QueryOptions? options
public async Task<Entry> CreateEntry(Entry entry)
{
entry.Id = entry.Id == default ? Guid.NewGuid() : entry.Id;
await validators.ValidateAndThrowAsync(entry);
await validators.ValidateAndThrow(entry);
try
{
UndoableUnitOfWorkHelper.DoUsingNewOrCurrentUOW("Create Entry",
Expand Down Expand Up @@ -860,7 +860,7 @@ public Task<Entry> UpdateEntry(Guid id, UpdateObjectInput<Entry> update)

public async Task<Entry> UpdateEntry(Entry before, Entry after)
{
await validators.ValidateAndThrowAsync(after);
await validators.ValidateAndThrow(after);
await Cache.DoUsingNewOrCurrentUOW("Update Entry",
"Revert entry",
async () =>
Expand Down Expand Up @@ -966,17 +966,17 @@ private void ApplySenseToLexSense(Sense sense, ILexSense lexSense)
return Task.FromResult(lcmSense is null ? null : FromLexSense(lcmSense));
}

public Task<Sense> CreateSense(Guid entryId, Sense sense, BetweenPosition? between = null)
public async Task<Sense> CreateSense(Guid entryId, Sense sense, BetweenPosition? between = null)
{
if (sense.Id == default) sense.Id = Guid.NewGuid();
if (!EntriesRepository.TryGetObject(entryId, out var lexEntry))
throw new InvalidOperationException("Entry not found");
validators.ValidateAndThrow(sense);
await validators.ValidateAndThrow(sense);
UndoableUnitOfWorkHelper.DoUsingNewOrCurrentUOW("Create Sense",
"Remove sense",
Cache.ServiceLocator.ActionHandler,
() => CreateSense(lexEntry, sense, between));
return Task.FromResult(FromLexSense(SenseRepository.GetObject(sense.Id)));
return FromLexSense(SenseRepository.GetObject(sense.Id));
}

public Task<Sense> UpdateSense(Guid entryId, Guid senseId, UpdateObjectInput<Sense> update)
Expand All @@ -996,7 +996,7 @@ public Task<Sense> UpdateSense(Guid entryId, Guid senseId, UpdateObjectInput<Sen

public async Task<Sense> UpdateSense(Guid entryId, Sense before, Sense after)
{
await validators.ValidateAndThrowAsync(after);
await validators.ValidateAndThrow(after);
await Cache.DoUsingNewOrCurrentUOW("Update Sense",
"Revert Sense",
async () =>
Expand Down Expand Up @@ -1078,17 +1078,17 @@ internal void CreateExampleSentence(ILexSense lexSense, ExampleSentence exampleS
lexExampleSentence.Reference.get_WritingSystem(0));
}

public Task<ExampleSentence> CreateExampleSentence(Guid entryId, Guid senseId, ExampleSentence exampleSentence)
public async Task<ExampleSentence> CreateExampleSentence(Guid entryId, Guid senseId, ExampleSentence exampleSentence)
{
if (exampleSentence.Id == default) exampleSentence.Id = Guid.NewGuid();
if (!SenseRepository.TryGetObject(senseId, out var lexSense))
throw new InvalidOperationException("Sense not found");
validators.ValidateAndThrow(exampleSentence);
await validators.ValidateAndThrow(exampleSentence);
UndoableUnitOfWorkHelper.DoUsingNewOrCurrentUOW("Create Example Sentence",
"Remove example sentence",
Cache.ServiceLocator.ActionHandler,
() => CreateExampleSentence(lexSense, exampleSentence));
return Task.FromResult(FromLexExampleSentence(senseId, ExampleSentenceRepository.GetObject(exampleSentence.Id)));
return FromLexExampleSentence(senseId, ExampleSentenceRepository.GetObject(exampleSentence.Id));
}

public Task<ExampleSentence> UpdateExampleSentence(Guid entryId,
Expand All @@ -1114,7 +1114,7 @@ public async Task<ExampleSentence> UpdateExampleSentence(Guid entryId,
ExampleSentence before,
ExampleSentence after)
{
await validators.ValidateAndThrowAsync(after);
await validators.ValidateAndThrow(after);
await Cache.DoUsingNewOrCurrentUOW("Update Example Sentence",
"Revert Example Sentence",
async () =>
Expand Down
8 changes: 1 addition & 7 deletions backend/FwLite/FwLiteProjectSync.Tests/Sena3SyncTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace FwLiteProjectSync.Tests;

[Trait("Category", "Integration")]
public class Sena3SyncTests : IClassFixture<Sena3Fixture>, IAsyncLifetime
{
private readonly Sena3Fixture _fixture;
Expand Down Expand Up @@ -75,7 +76,6 @@ private async Task WorkaroundMissingWritingSystems()
}

[Fact]
[Trait("Category", "Integration")]
public async Task DryRunImport_MakesNoChanges()
{
await WorkaroundMissingWritingSystems();
Expand All @@ -86,7 +86,6 @@ public async Task DryRunImport_MakesNoChanges()
}

[Fact]
[Trait("Category", "Integration")]
public async Task DryRunImport_MakesTheSameChangesAsImport()
{
var dryRunSyncResult = await _syncService.SyncDryRun(_crdtApi, _fwDataApi);
Expand All @@ -95,7 +94,6 @@ public async Task DryRunImport_MakesTheSameChangesAsImport()
}

[Fact]
[Trait("Category", "Integration")]
public async Task DryRunSync_MakesNoChanges()
{
await BypassImport();
Expand All @@ -108,7 +106,6 @@ public async Task DryRunSync_MakesNoChanges()

[Fact]
[Trait("Category", "Slow")]
[Trait("Category", "Integration")]
public async Task DryRunSync_MakesTheSameChangesAsSync()
{
//syncing requires querying entries, which fails if there are no writing systems, so we import those first
Expand All @@ -123,7 +120,6 @@ public async Task DryRunSync_MakesTheSameChangesAsSync()
}

[Fact]
[Trait("Category", "Integration")]
public async Task FirstSena3SyncJustDoesAnSync()
{
_fwDataApi.EntryCount.Should().BeGreaterThan(1000,
Expand All @@ -141,7 +137,6 @@ public async Task FirstSena3SyncJustDoesAnSync()

[Fact]
[Trait("Category", "Slow")]
[Trait("Category", "Integration")]
public async Task SyncWithoutImport_CrdtShouldMatchFwdata()
{
await BypassImport();
Expand All @@ -157,7 +152,6 @@ public async Task SyncWithoutImport_CrdtShouldMatchFwdata()
}

[Fact]
[Trait("Category", "Integration")]
public async Task SecondSena3SyncDoesNothing()
{
await _syncService.Sync(_crdtApi, _fwDataApi);
Expand Down
24 changes: 12 additions & 12 deletions backend/FwLite/LcmCrdt/CrdtMiniLcmApi.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public async Task<WritingSystems> GetWritingSystems()

public async Task<WritingSystem> CreateWritingSystem(WritingSystemType type, WritingSystem writingSystem)
{
await validators.ValidateAndThrowAsync(writingSystem);
await validators.ValidateAndThrow(writingSystem);
var entityId = Guid.NewGuid();
var wsCount = await WritingSystems.CountAsync(ws => ws.Type == type);
try
Expand All @@ -70,7 +70,7 @@ public async Task<WritingSystem> UpdateWritingSystem(WritingSystemId id, Writing

public async Task<WritingSystem> UpdateWritingSystem(WritingSystem before, WritingSystem after)
{
await validators.ValidateAndThrowAsync(after);
await validators.ValidateAndThrow(after);
await WritingSystemSync.Sync(before, after, this);
return await GetWritingSystem(after.WsId, after.Type) ?? throw new NullReferenceException("unable to find writing system with id " + after.WsId);
}
Expand Down Expand Up @@ -103,7 +103,7 @@ public IAsyncEnumerable<PartOfSpeech> GetPartsOfSpeech()

public async Task<PartOfSpeech> CreatePartOfSpeech(PartOfSpeech partOfSpeech)
{
await validators.ValidateAndThrowAsync(partOfSpeech);
await validators.ValidateAndThrow(partOfSpeech);
await dataModel.AddChange(ClientId, new CreatePartOfSpeechChange(partOfSpeech.Id, partOfSpeech.Name, partOfSpeech.Predefined));
return await GetPartOfSpeech(partOfSpeech.Id) ?? throw new NullReferenceException();
}
Expand All @@ -119,7 +119,7 @@ public async Task<PartOfSpeech> UpdatePartOfSpeech(Guid id, UpdateObjectInput<Pa

public async Task<PartOfSpeech> UpdatePartOfSpeech(PartOfSpeech before, PartOfSpeech after)
{
await validators.ValidateAndThrowAsync(after);
await validators.ValidateAndThrow(after);
await PartOfSpeechSync.Sync(before, after, this);
return await GetPartOfSpeech(after.Id) ?? throw new NullReferenceException($"unable to find part of speech with id {after.Id}");
}
Expand Down Expand Up @@ -184,7 +184,7 @@ public IAsyncEnumerable<ComplexFormType> GetComplexFormTypes()

public async Task<ComplexFormType> CreateComplexFormType(ComplexFormType complexFormType)
{
await validators.ValidateAndThrowAsync(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);
Expand All @@ -198,7 +198,7 @@ public async Task<ComplexFormType> UpdateComplexFormType(Guid id, UpdateObjectIn

public async Task<ComplexFormType> UpdateComplexFormType(ComplexFormType before, ComplexFormType after)
{
await validators.ValidateAndThrowAsync(after);
await validators.ValidateAndThrow(after);
await ComplexFormTypeSync.Sync(before, after, this);
return await GetComplexFormType(after.Id) ?? throw new NullReferenceException($"unable to find complex form type with id {after.Id}");
}
Expand Down Expand Up @@ -368,7 +368,7 @@ private IEnumerable<IChange> CreateEntryChanges(Entry entry, Dictionary<Guid, Se

public async Task<Entry> CreateEntry(Entry entry)
{
await validators.ValidateAndThrowAsync(entry);
await validators.ValidateAndThrow(entry);
await dataModel.AddChanges(ClientId,
[
new CreateEntryChange(entry),
Expand Down Expand Up @@ -456,7 +456,7 @@ public async Task<Entry> UpdateEntry(Guid id,

public async Task<Entry> UpdateEntry(Entry before, Entry after)
{
await validators.ValidateAndThrowAsync(after);
await validators.ValidateAndThrow(after);
await EntrySync.Sync(before, after, this);
return await GetEntry(after.Id) ?? throw new NullReferenceException("unable to find entry with id " + after.Id);
}
Expand Down Expand Up @@ -502,7 +502,7 @@ public async Task<Sense> CreateSense(Guid entryId, Sense sense, BetweenPosition?
throw new InvalidOperationException("Order should not be provided when creating a sense");

sense.Order = await OrderPicker.PickOrder(Senses.Where(s => s.EntryId == entryId), between);
await validators.ValidateAndThrowAsync(sense);
await validators.ValidateAndThrow(sense);
await dataModel.AddChanges(ClientId, await CreateSenseChanges(entryId, sense).ToArrayAsync());
return await dataModel.GetLatest<Sense>(sense.Id) ?? throw new NullReferenceException();
}
Expand All @@ -519,7 +519,7 @@ public async Task<Sense> UpdateSense(Guid entryId,

public async Task<Sense> UpdateSense(Guid entryId, Sense before, Sense after)
{
await validators.ValidateAndThrowAsync(after);
await validators.ValidateAndThrow(after);
await SenseSync.Sync(entryId, before, after, this);
return await GetSense(entryId, after.Id) ?? throw new NullReferenceException("unable to find sense with id " + after.Id);
}
Expand Down Expand Up @@ -549,7 +549,7 @@ public async Task<ExampleSentence> CreateExampleSentence(Guid entryId,
Guid senseId,
ExampleSentence exampleSentence)
{
await validators.ValidateAndThrowAsync(exampleSentence);
await validators.ValidateAndThrow(exampleSentence);
await dataModel.AddChange(ClientId, new CreateExampleSentenceChange(exampleSentence, senseId));
return await dataModel.GetLatest<ExampleSentence>(exampleSentence.Id) ?? throw new NullReferenceException();
}
Expand Down Expand Up @@ -578,7 +578,7 @@ public async Task<ExampleSentence> UpdateExampleSentence(Guid entryId,
ExampleSentence before,
ExampleSentence after)
{
await validators.ValidateAndThrowAsync(after);
await validators.ValidateAndThrow(after);
await ExampleSentenceSync.Sync(entryId, senseId, before, after, this);
return await GetExampleSentence(entryId, senseId, after.Id) ?? throw new NullReferenceException();
}
Expand Down
2 changes: 0 additions & 2 deletions backend/FwLite/MiniLcm.Tests/Validators/EntryValidatorTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ public void Fails_WhenComplexFormsContainCircularReference()
var entryId = Guid.NewGuid();
var entry = new Entry() { Id = entryId, LexemeForm = new MultiString(){{"en", "lexeme"}}, ComplexForms = [new ComplexFormComponent(){ ComplexFormEntryId = entryId, ComponentEntryId = Guid.Empty }] };
_validator.TestValidate(entry).ShouldHaveValidationErrorFor("ComplexForms[0]");
// _validator.TestValidate(entry).ShouldNotHaveAnyValidationErrors();
}

[Fact]
Expand All @@ -117,7 +116,6 @@ public void Fails_WhenComponentsContainCircularReference()
var entryId = Guid.NewGuid();
var entry = new Entry() { Id = entryId, LexemeForm = new MultiString(){{"en", "lexeme"}}, Components = [new ComplexFormComponent(){ ComplexFormEntryId = Guid.Empty, ComponentEntryId = entryId }] };
_validator.TestValidate(entry).ShouldHaveValidationErrorFor("Components[0]");
// _validator.TestValidate(entry).ShouldNotHaveAnyValidationErrors();
}


Expand Down
12 changes: 6 additions & 6 deletions backend/FwLite/MiniLcm.Tests/Validators/ExampleSentenceValidatorTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ public void Fails_WhenDeletedAtIsNotNull()
}

[Theory]
[InlineData("Sentence")]
[InlineData("Translation")]
[InlineData(nameof(ExampleSentence.Sentence))]
[InlineData(nameof(ExampleSentence.Translation))]
public void Succeeds_WhenNonEmptyFieldIsPresent(string fieldName)
{
var example = new ExampleSentence() { Id = Guid.NewGuid() };
Expand All @@ -32,8 +32,8 @@ public void Succeeds_WhenNonEmptyFieldIsPresent(string fieldName)
}

[Theory]
[InlineData("Sentence")]
[InlineData("Translation")]
[InlineData(nameof(ExampleSentence.Sentence))]
[InlineData(nameof(ExampleSentence.Translation))]
public void Succeeds_WhenNonEmptyFieldHasNoContent(string fieldName)
{
var example = new ExampleSentence() { Id = Guid.NewGuid() };
Expand All @@ -42,8 +42,8 @@ public void Succeeds_WhenNonEmptyFieldHasNoContent(string fieldName)
}

[Theory]
[InlineData("Sentence")]
[InlineData("Translation")]
[InlineData(nameof(ExampleSentence.Sentence))]
[InlineData(nameof(ExampleSentence.Translation))]
public void Fails_WhenNonEmptyFieldHasWsWithEmptyContent(string fieldName)
{
var example = new ExampleSentence() { Id = Guid.NewGuid() };
Expand Down
Loading

0 comments on commit c3f0908

Please sign in to comment.