feat: implement OpenAPI discriminator support for oneOf/anyOf#4
Merged
feat: implement OpenAPI discriminator support for oneOf/anyOf#4
Conversation
- issue201: duplicate path template with different methods - issue639: request body decode edge cases (empty objects, optional body, additional properties, nested validation) - issue707: path parameter edge cases (type coercion, min/max constraints, multiple params) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add resolve_discriminator() in body.lua that selects the correct oneOf/anyOf branch based on the discriminator property value, avoiding false positives/negatives when branches aren't mutually exclusive. The ref resolver now annotates resolved nodes with _ref so the discriminator mapping can match branches by their original $ref path. Falls back to enum-based matching when mapping is absent. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
📝 WalkthroughWalkthroughThis pull request adds discriminator resolution support for JSON body validation in OpenAPI specifications. When a schema contains a discriminator, the validator now resolves the correct Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
t/conformance/test_issue707.lua (2)
56-57: Preserve compile error details in the assertion message.Line 57 currently hides the underlying compile failure reason, which makes debugging harder when this test fails in CI.
🛠️ Proposed improvement
-local v = ov.compile(spec) -assert(v, "compile failed") +local v, compile_err = ov.compile(spec) +assert(v, "compile failed: " .. tostring(compile_err))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@t/conformance/test_issue707.lua` around lines 56 - 57, The assertion hides the underlying compile error; change the call to capture the compile error returned by ov.compile and include it in the assertion message (e.g., call ov.compile as local v, err = ov.compile(spec) and update the assert to include tostring(err) so the failure message shows the real compile error), referring to the ov.compile(spec) call and the variable v used in the current assertion.
102-124: Add an explicit maximum-boundary success case (/bounded/100).Line 118 verifies “above maximum” failure, but there’s no direct assertion that the exact maximum endpoint passes. Adding it makes boundary coverage complete.
➕ Proposed test addition
T.describe("issue707: bounded path param at minimum (valid)", function() local ok, err = v:validate_request({ method = "GET", path = "/bounded/1", }) T.ok(ok, "should pass: " .. tostring(err)) end) +T.describe("issue707: bounded path param at maximum (valid)", function() + local ok, err = v:validate_request({ + method = "GET", + path = "/bounded/100", + }) + T.ok(ok, "should pass: " .. tostring(err)) +end) + T.describe("issue707: bounded path param below minimum (fail)", function() local ok, err = v:validate_request({ method = "GET", path = "/bounded/0", })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@t/conformance/test_issue707.lua` around lines 102 - 124, Add a new test case mirroring the existing "issue707: bounded path param at minimum (valid)" block but for the maximum boundary: call v:validate_request with method = "GET" and path = "/bounded/100" and assert T.ok(ok, "should pass: " .. tostring(err)) to ensure the exact maximum value passes; place this T.describe block alongside the other bounded tests so it covers the upper-bound success case for the same validation logic.lib/resty/openapi_validator/body.lua (1)
399-415: Consider returning early or skipping validation on discriminator error.When
disc_erris set, the error is recorded but validation continues against the original composite schema. This may produce confusing additional errors (e.g., "does not match any schema" from the discriminator, plus oneOf validation errors).Consider either returning early or skipping JSON schema validation when discriminator resolution fails:
♻️ Option: Return early on discriminator error
local effective_schema = schema local disc_schema, disc_err = resolve_discriminator(schema, body_data) if disc_err then tab_insert(errs, errors.new("body", nil, disc_err)) + return false, errs elseif disc_schema then effective_schema = disc_schema end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/resty/openapi_validator/body.lua` around lines 399 - 415, When resolve_discriminator(schema, body_data) sets disc_err, stop further JSON Schema validation to avoid confusing follow-up errors: after inserting the discriminator error into errs (where disc_err is handled), immediately return (or skip calling get_validator/validator) so no validation is run against the original composite schema; modify the logic around resolve_discriminator, effective_schema, get_validator and validator in this function to short-circuit on disc_err (with the existing errors.new("body", nil, disc_err) insertion) instead of proceeding to run validator(body_data).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@lib/resty/openapi_validator/body.lua`:
- Around line 399-415: When resolve_discriminator(schema, body_data) sets
disc_err, stop further JSON Schema validation to avoid confusing follow-up
errors: after inserting the discriminator error into errs (where disc_err is
handled), immediately return (or skip calling get_validator/validator) so no
validation is run against the original composite schema; modify the logic around
resolve_discriminator, effective_schema, get_validator and validator in this
function to short-circuit on disc_err (with the existing errors.new("body", nil,
disc_err) insertion) instead of proceeding to run validator(body_data).
In `@t/conformance/test_issue707.lua`:
- Around line 56-57: The assertion hides the underlying compile error; change
the call to capture the compile error returned by ov.compile and include it in
the assertion message (e.g., call ov.compile as local v, err = ov.compile(spec)
and update the assert to include tostring(err) so the failure message shows the
real compile error), referring to the ov.compile(spec) call and the variable v
used in the current assertion.
- Around line 102-124: Add a new test case mirroring the existing "issue707:
bounded path param at minimum (valid)" block but for the maximum boundary: call
v:validate_request with method = "GET" and path = "/bounded/100" and assert
T.ok(ok, "should pass: " .. tostring(err)) to ensure the exact maximum value
passes; place this T.describe block alongside the other bounded tests so it
covers the upper-bound success case for the same validation logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eae90cbc-713c-419d-916f-4cacf0e28c8d
📒 Files selected for processing (6)
lib/resty/openapi_validator/body.lualib/resty/openapi_validator/refs.luat/conformance/test_issue201.luat/conformance/test_issue639.luat/conformance/test_issue707.luat/conformance/test_validation_discriminator.lua
nic-6443
approved these changes
Apr 21, 2026
jarvis9443
added a commit
that referenced
this pull request
Apr 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds discriminator support to the JSON body validation path, fixing false positives/negatives when
oneOf/anyOfbranches aren't mutually exclusive (noadditionalProperties: false).Changes
lib/resty/openapi_validator/body.luaresolve_discriminator(schema, body_data)— reads the discriminator property from the request body, finds the matchingoneOf/anyOfbranch, and returns it for validation instead of the full composite schema_refannotations whendiscriminator.mappingexists, falls back to enum-based matchingallOfsub-schemas when searching for matching brancheslib/resty/openapi_validator/refs.lua$refnodes with_reffield preserving the original ref path, so discriminator mapping can match branches after ref resolutiont/conformance/test_validation_discriminator.luaSummary by CodeRabbit
New Features
oneOf/anyOfscenarios.Tests