Allow specifying an exclusive set of fields on ObjectParser #52893
Conversation
romseygeek
added
>enhancement
:Core/Infra/Core
|
One tweak that we could make would be to combine |
|
Pinging @elastic/es-core-infra (:Core/Infra/Core) |
| @@ -302,18 +306,40 @@ public Value parse(XContentParser parser, Value value, Context context) throws I | |||
| } | |||
| } | |||
|
|
|||
| // Check if this field is in an exclusive set, if it is then mark | |||
| // it as seen. If the set is already marked, then we have a duplicate | |||
There was a problem hiding this comment.
This doesn't throw an exception though!
There was a problem hiding this comment.
Oh, I see. It does, but later. Got it. Maybe update the comment. Not sure.
There was a problem hiding this comment.
I rewrote the comments a bit to make things clearer
| * E.g. <code>declareExclusiveFieldSet("foo", "bar");</code> means that only one of 'foo' | ||
| * or 'bar' must be present, and if both appear then an exception will be thrown. | ||
| * | ||
| * Multiple exclusive sets may be declared |
There was a problem hiding this comment.
I think it might be worth mentioning that it doesn't make foo or bar required.
| } else if (indexedDocumentId != null) { | ||
| queryBuilder = new PercolateQueryBuilder(field, indexedDocumentIndex, indexedDocumentId, indexedDocumentRouting, | ||
| indexedDocumentPreference, indexedDocumentVersion); | ||
| @SuppressWarnings("unchecked") |
There was a problem hiding this comment.
I tend to put this on the variable that needs it for extra paranoia.
There was a problem hiding this comment.
TIL you can do this on variables - thanks!
| public static PercolateQueryBuilder fromXContent(XContentParser parser) throws IOException { | ||
| float boost = AbstractQueryBuilder.DEFAULT_BOOST; | ||
|
|
||
| String field = null; |
|
Thanks for the review @nik9000 , I pushed some changes to address your concerns. |
|
I'm totally swamped right now and Nik knows way more about parser stuff than me, so don't feel blocked on my missing review :) |
ObjectParser allows you to declare a set of required fields, such that at least one of the set must appear in an xcontent object for it to be valid. This commit adds the similar concept of a set of exclusive fields, such that at most one of the set must be present. It also enables required fields on ConstructingObjectParser, and re-implements PercolateQueryBuilder.fromXContent() to use object parsing as an example of how this works.
ObjectParser allows you to declare a set of required fields, such that at least one
of the set must appear in an xcontent object for it to be valid. This commit adds
the similar concept of a set of exclusive fields, such that at most one of the set
must be present. It also enables required fields on ConstructingObjectParser, and
re-implements
PercolateQueryBuilder.fromXContent()to use object parsing asan example of how this could work.