nic-6443 commented on code in PR #12649:
URL: https://github.com/apache/apisix/pull/12649#discussion_r3393092528
##########
apisix/plugins/grpc-transcode/response.lua:
##########
@@ -23,6 +23,58 @@ local string = string
local ngx_decode_base64 = ngx.decode_base64
local ipairs = ipairs
local pcall = pcall
+local type = type
+local pairs = pairs
+local setmetatable = setmetatable
+
+local _M = {}
+
+-- Protobuf repeated field label value
+local PROTOBUF_REPEATED_LABEL = 3
+local FIELD_TYPE_MESSAGE = 11
+
+local function set_default_array(tab, descriptor, message_index)
Review Comment:
I think there is a much simpler way to achieve this that lets lua-protobuf
do all the heavy lifting. The version we pin (0.5.3) supports a special
`*array` default metatable
([docs](https://github.com/starwing/lua-protobuf#default-values)): if we call
`pb.defaults("*array", core.json.array_mt)` once in `compile_proto()` (inside
the `pb.state` window, right before `compiled.pb_state =
pb.state(old_pb_state)`), then `pb.decode` attaches the array metatable to
every repeated field table by itself.
I verified this locally against lua-protobuf 0.5.3 with the plugin's default
options: empty repeated fields encode as `[]`, map fields correctly stay `{}`
(lua-protobuf distinguishes maps from arrays natively), nested repeated fields
inside map values and repeated messages also get `[]`, and it works for both
the text proto path and the binary descriptor path. So it would replace
`build_message_index` + `set_default_array` entirely with one line, with zero
per-request traversal cost.
One caveat: `pb.defaults` is tied to the active pb state, so it must be set
per compiled proto rather than at module load time. What do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
