sillsdev / sillsdev/languageforge-lexbox

MiniLcmApiValidationWrapper.CreateEntry skips entry validation

Open
#2,359 4 comments 0 reactions 0 assignees View on GitHub

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

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.