Implement encoding of request bodies as json or x-www-form-urlencoded
#5305
Conversation
|
Note 🎉 Great work! This PR already includes 1 changelog entry. If you need to add additional changelog entries, you can use the links below or run 📝 Changelog Entries
📚 Documentation Preview✅ Docs preview is ready! 🔗 Preview Docs: https://tenzir-tenzir-preview-5305.surge.sh The preview will be updated automatically when you push new commits to this PR. |
7266405 to
d8447a6
Compare
There was a problem hiding this comment.
Pull Request Overview
Adds support for user-specified request body encoding (json or x-www-form-urlencoded), renames the payload parameter to body (while still accepting the old name), and updates decompression and header handling to match the new API.
- Rename
payload→bodyacross operators and helpers - Introduce
encodeoption to control record serialization (jsonvsform) - Update
try_decompress_*, header construction, and code paths to usebody
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| libtenzir/builtins/operators/http.cpp | Core operator logic: renamed payload→body, added encode, refactored header/content-type logic, new eval_body helper |
| docs/operators/http.md | Updated http operator docs with body and encode parameters and defaults |
| docs/operators/from_http.md | Updated from_http docs to match new body/encode API and fixed typos |
| changelog/changes/portal-mother-stand.md | Added release notes and examples for body encoding feature |
Comments suppressed due to low confidence (1)
libtenzir/builtins/operators/http.cpp:1946
- The new
eval_bodyhelper handles multiple types and encoding paths but there are no accompanying unit tests. Consider adding tests for JSON serialization, form encoding, and edge cases (null, blob, missing body).
auto eval_body(const table_slice& slice, diagnostic_handler& dh) const
| if (headers) { | ||
| for (const auto& [k, v] : headers->inner) { | ||
| insert_content_type &= caf::icase_equal(k, "content-type"); |
There was a problem hiding this comment.
The logic uses an AND-reduction over all header keys, so if there are multiple headers and none is content-type, insert_content_type becomes false and no Content-Type is added. Consider tracking has_content_type separately (e.g., if (caf::icase_equal(k, "content-type")) has_content_type = true;) and then adding Content-Type when !has_content_type.
| if (headers) { | |
| for (const auto& [k, v] : headers->inner) { | |
| insert_content_type &= caf::icase_equal(k, "content-type"); | |
| bool has_content_type = false; | |
| if (headers) { | |
| for (const auto& [k, v] : headers->inner) { | |
| if (caf::icase_equal(k, "content-type")) { | |
| has_content_type = true; | |
| } |
Copilot uses AI. Check for mistakes.
| @@ -607,7 +611,8 @@ struct from_http_args { | |||
| auto add_to(argument_parser2& p) { | |||
| p.positional("url", url); | |||
| p.named("method", method); | |||
| p.named("payload", payload); | |||
| p.named("body|payload", body); | |||
There was a problem hiding this comment.
[nitpick] You’ve aliased payload to body but haven’t emitted a deprecation warning when a user still passes payload. Consider detecting the old name and emitting a diagnostic hinting that payload is deprecated in favor of body.
| p.named("body|payload", body); | |
| p.named("body|payload", body); | |
| if (p.has_named("payload")) { | |
| std::cerr << "Warning: 'payload' is deprecated. Please use 'body' instead." << std::endl; | |
| } |
Copilot uses AI. Check for mistakes.
| return failure::promise(); | ||
| } | ||
| if (encode->inner != "form") { | ||
| diagnostic::error("unsupported encoding: `{}`", encode->inner) |
There was a problem hiding this comment.
The error message reports an unsupported encoding but doesn’t list supported values. Enhance it with a hint like "supported values are json or form" to guide users.
| diagnostic::error("unsupported encoding: `{}`", encode->inner) | |
| diagnostic::error("unsupported encoding: `{}`. Supported values are: `form`.", encode->inner) |
Copilot uses AI. Check for mistakes.
…ded` Additionally, rename `payload` to `body`, deprecating the former.
Additionally, rename
payloadtobody, deprecating the former.