-
Notifications
You must be signed in to change notification settings - Fork 2.7k
test: cover grpc-transcode empty array responses #12649
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
base: master
Are you sure you want to change the base?
Conversation
|
Hi @bytelazy, could you merge the latest main branch? |
sure~ |
|
pls merge the fix the ci case: |
| if type(v) == "table" then | ||
| local sub_names = fetch_proto_array_names(v) | ||
| for sub_name,_ in pairs(sub_names) do | ||
| names[sub_name] = 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| names[sub_name] = 1 | |
| names[sub_name] = true |
| local PROTOBUF_REPEATED_LABEL = 3 | ||
| local repeated_label = PROTOBUF_REPEATED_LABEL | ||
|
|
||
| local function fetch_proto_array_names(proto_obj) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function does not take into account the possibility of duplicate names in nested fields, for example:
message Company {
repeated string name = 1;
}
message HelloRequest {
string name = 1;
Company company = 2;
}
|
|
||
| local function fetch_proto_array_names(proto_obj) | ||
| local names = {} | ||
| if type(proto_obj) == "table" then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| if type(proto_obj) == "table" then | |
| if type(proto_obj) ~= "table" then | |
| return | |
| end |
| return err_msg | ||
| end | ||
|
|
||
| local array_names = fetch_proto_array_names(proto) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can cache it without having to reprocess it every time a request is made.
| local function create_proto_obj(proto_id) |
|
Hi @bytelazy, please check these comments. |
Description
In the grpc-transcode plugin, when a repeated field in the gRPC response is empty, it is incorrectly encoded as an empty JSON object ({}) instead of an empty JSON array ([]). This behavior does not comply with the gRPC-JSON transcoding specification and can cause type-related errors on the client-side, which expects an array.
Fixes #11440
Checklist
Changes or the new features added to this PR
The main changes are in the response.lua file to correctly handle the serialization of repeated fields.
Identify repeated fields: A new function fetch_proto_array_names has been added. It recursively traverses the loaded Protobuf schema definition to identify and collect the names of all fields marked as repeated.
Ensure correct JSON array serialization:
A new function set_default_array has been introduced. After the gRPC response is decoded into a Lua table, this function traverses the table.
For any field that was identified as repeated, it sets a specific metatable (core.json.array_mt).
This metatable forces the core.json.encode function to serialize the corresponding Lua table as a JSON array ([]), even when it is empty.
This logic is applied just before the decoded response is encoded into the final JSON output, ensuring the structure is correct.