-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Rename AtLeastOneOf new Constraint to OneOf #36722
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
I think |
@fancyweb 👍 : I didn't see it that way. But AtLeastOne suggest that many can succeed. But by the look of the code it stops as soon as a constraint has no violations. For me XOR equivalent would be |
@Neirda24 there is nothing in the current implementation forbidding multiple ones to succeed. We stop the loop once we know the final result (the validation succeeds), because it is useless to run all constraint validators if we already know the answer. The fact that it stops is an internal implementation detail to optimize performance, but it does not change the result. |
@stof : The fact that it stops once a constraint has no violations means we don't check for others. If we don't we can't know if more would have succeeded. Meaning that at this time the validator can only assure us that One and only one constraint did succeed. And regarding the fact that it is exactly what we want I thought that AtLeastOneOf was a bit redundant. |
@Neirda24 even if we would be running them all, we would only report a violation if all of them failed, as that's the only case where the The current implementation does not assure that one and only one constraint succeeded. To assure that |
@Neirda24 I changed the initial name to |
@przemyslaw-bogusz we don't care if others succeed or not. |
I'm personally fine with |
If we forget about the technical things for a moment, compare these two phrases:
To me, "one of" and "at least one of" have completely different meaning. Returning to technical discussions ... "one of" to me is equivalent to |
This is exactly my point. To me, the current validation does a In conclusion for me as long as we keep these optimization we change the behavior from At Least One Of to only One Of. I don't know how it works. If there is a vote or something. So please just let me know if I can help in any way. (documentation as @przemyslaw-bogusz suggested). |
IMO |
I'm closing because this is not going to be accepted, let's move on. Thanks for proposing! |
@Neirda24 the currently implementation does not do |
@przemyslaw-bogusz Originally created
AtLeastOne
constraint. Which it seems was merged in 5.1 asAtLeastOneOf
. (#35744).I propose to rename it simply
OneOf
because it stops as soon as one of the constraints has no violations. So At least and One of seems a duplicate of information. Also calling it justOneOf
is to have the same wording as OpenApi spec.