Correctly fail to parse list-of-oneOf with an invalid item#4078
Correctly fail to parse list-of-oneOf with an invalid item#4078ddadamhooper wants to merge 1 commit into
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 80fbe0bc2c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| public void setGroupBy(FormulaAndFunctionEventQueryGroupByConfig groupBy) { | ||
| this.groupBy = groupBy; | ||
| if (groupBy != null) { | ||
| this.unparsed |= groupBy.unparsed; |
There was a problem hiding this comment.
Propagate unparsed through the oneOf group_by wrapper
In the event-query path, group_by is a FormulaAndFunctionEventQueryGroupByConfig oneOf wrapper; its deserializer stores the List<FormulaAndFunctionEventQueryGroupBy> variant and increments match without checking whether any list item has unparsed=true, so this added copy still sees groupBy.unparsed == false. A payload such as group_by: [{facet: "@x", sort: {aggregation: "NOT_A_VALID_AGGREGATION"}}] therefore leaves FormulaAndFunctionEventQueryDefinition parsed even though the child sort is invalid; the new test covers the UserJourney model's direct list, not this oneOf wrapper.
Useful? React with 👍 / 👎.
jack-edmonds-dd
left a comment
There was a problem hiding this comment.
approval for modelSimple.j2 (I'm assuming the rest of the changes are generated).
What does this PR do?
Correctly fails to parse a
FormulaAndFunctionEventQueryDefinitionwhen it contains an invalidgroup_bylist entry.The code generator was failing to propagate
unparsedfrom aoneOfto its parent in this case.(DataDog's APIs don't generally return invalid objects, so this shouldn't affect users. The value comes with an upcoming PR, where a
oneOfitem contains a list which contains anotheroneOfitem. In that case, we need to detect invalid objects from the inneroneOf.)Review checklist
Please check relevant items below: