fix: intersect OneOf and Equal choices in field2choices - #1069
fix: intersect OneOf and Equal choices in field2choices#1069alexchen-sys wants to merge 2 commits into
Conversation
…llow-code#198) Calculate intersection across all choice validators (both validate.OneOf and validate.Equal) present on field.validators. Signed-off-by: Alex Chen <l46983284@gmail.com>
lafrech
left a comment
lafrech
left a comment
There was a problem hiding this comment.
100+ PRs a month is a bit of a reg flag, but this PR makes sense, so...
| ) | ||
| res = spec_fixture.openapi.field2property(field) | ||
| assert set(res["enum"]) == {"brian", "john"} | ||
| assert res["enum"] == ["brian", "john"] |
There was a problem hiding this comment.
I guess the use of set here was intentional as we don't want to ensure the order.
There was a problem hiding this comment.
Done — reverted to set() comparison.
| assert res["enum"] == ["brian", "john"] | ||
|
|
||
|
|
||
| def test_field_with_choices_multiple_non_intersecting(spec_fixture): |
There was a problem hiding this comment.
I'd write oneof instead of choices in the test name.
| assert res["enum"] == [] | ||
|
|
||
|
|
||
| def test_field_with_multiple_equal_matching(spec_fixture): |
There was a problem hiding this comment.
I think the whole thing could be factorized in three tests:
- multiple oneof
- multiple equal
- oneof and equal
using parametrization to pass inputs and expected output.
There was a problem hiding this comment.
Factorized into three parametrized tests: multiple_oneof, multiple_equal, oneof_and_equal.
| ] | ||
| if choices: | ||
| attributes["enum"] = list(functools.reduce(operator.and_, choices)) | ||
| choices = [] |
There was a problem hiding this comment.
I've been wondering why we don't use a set here and update it in the loop.
This is probably due to a slight performance advantage doing it this way.
|
Thanks for the review. Comments addressed in e485237 — set comparison restored, tests renamed to oneof, factorized into three parametrized tests. |
2 participants
Fixes #198
When multiple choice validators are defined on a field (such as multiple
validate.OneOf, multiplevalidate.Equal, or a mix ofOneOfandEqual), the allowed choices should be the intersection of the values allowed by each validator.Previously:
validate.Equaloverrode any precedingvalidate.OneOfchoices.validate.Equalvalidators concatenated their values rather than intersecting them.Changes
validate.Equalcomparable values as single-element choice sets and intersect all choice sets viaOrderedSet.OneOf,OneOf+Equal, and multipleEqualvalidators (both intersecting and non-intersecting).