sillsdev / sillsdev/languageforge-lexbox
MiniLcmApiValidationWrapper.CreateEntry skips entry validation
Nobody has claimed this yet.
- Dominant language
- C#
- Stars
- 9
- Forks
- 8
- Avg merge
- 2d 13h
- Merged PRs (30d)
- 49
Description
Summary
Entry validation is silently not run for CreateEntry calls made through the user-facing MiniLcm wrapper stack. UpdateEntry, CreateSense, etc. are unaffected.
Verified behavior
Creating an invalid Entry (e.g. with DeletedAt set, which EntryValidator requires to be null) through a validation-wrapped api does not throw ValidationException:
var validatedApi = sp.GetRequiredService<MiniLcmApiValidationWrapperFactory>().Create(api);
var invalid = new Entry { Id = Guid.NewGuid(), LexemeForm = { { "en", "test" } }, DeletedAt = DateTimeOffset.UtcNow };
await validatedApi.CreateEntry(invalid, ...); // expected: throws; actual: succeeds
Likely cause (not fully confirmed)
MiniLcmApiValidationWrapper declares a hand-written one-arg override:
public async Task<Entry> CreateEntry(Entry entry) // validates, then _api.CreateEntry(entry)
but the interface member is two-arg CreateEntry(Entry, CreateEntryOptions? = null). The [BeaKona.AutoInterface(MemberMatch = Any)] generator appears to emit a two-arg forwarder (_api.CreateEntry(entry, options)) with no ValidateAndThrow call, and all interface dispatch hits that forwarder — so the validating one-arg method is never reached. The sibling wrappers (MiniLcmApiWriteNormalizationWrapper, MiniLcmApiNotifyWrapper) use the two-arg signature and forward correctly.
Suggested fix
Give the override the full signature so it both validates and forwards options (matching the siblings):
public async Task<Entry> CreateEntry(Entry entry, CreateEntryOptions? options = null)
{
await validators.ValidateAndThrow(entry);
return await _api.CreateEntry(entry, options);
}
Caution
Enabling validation may begin rejecting entries that were previously accepted via this path — the fix needs its own review and a test run across CreateEntry callers (interactive create, GraphQL/route, hub, JS-invokable).
Discovered during review of #2341 (the Publication.IsMain work); pre-existing and unrelated, so filed separately rather than fixed there.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with MiniLcmApiValidationWrapper.CreateEntry and compare its one-argument override with the two-argument signatures in MiniLcmApiWriteNormalizationWrapper and MiniLcmApiNotifyWrapper. Run the relevant CreateEntry callers and validation tests, including interactive create, GraphQL/route, hub, and JS-invokable paths. Done means invalid entries raise ValidationException while options are still forwarded correctly.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- api, backend, testing
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 68/100