Consolidate script parsing from object#59507
Conversation
The update by query action parses a script from an object (map or string). We will need to do the same for runtime fields as they are parsed as part of mappings (elastic#59391). This commit moves the existing parsing of a script from an object from RestUpdateByQueryAction to the Script class. It also adds tests and adjusts some error messages that are incorrect. Also, options were not parsed before and they are now. And unsupported fields are no longer silently ignored.
|
Pinging @elastic/es-core-infra (:Core/Infra/Scripting) |
This should have been added with elastic#59507
|
I think in a follow up PR we should make the builder in Script used for XContent generic enough to either XContent or a Map or even a String. This way the code follows the same path for both parsing and error checking instead of having separate parsers here. |
|
@jdconrad I agree that it's odd to have two separate code paths. On the other hand, it's two different sources that are parsed differently. I don't see how we can share the parsing code without converting the source format e.g. writing the map as xcontent and parsing it back. Can you elaborate? |
|
@javanna I see what you mean, I do think there's an opportunity here to use the same validation, though for both. I will give this some thought. |
This should have been added with #59507
The update by query action parses a script from an object (map or string). We will need to do the same for runtime fields as they are parsed as part of mappings (#59391).
This commit moves the existing parsing of a script from an object from RestUpdateByQueryAction to the Script class. It also adds tests and adjusts some error messages that are incorrect. Also, options were not parsed before and they are now. And unsupported fields are no longer silently ignored.
I plan on backporting this change to 7.x without the handling for unsupported fields, as that is a breaking change.