Skip to content
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

fix(*): gRPC and cJSON's behavior to empty array #10790

Merged
merged 3 commits into from
May 8, 2023
Merged

Conversation

StarlightIbuki
Copy link
Contributor

@StarlightIbuki StarlightIbuki commented May 5, 2023

Summary

Lua does not distinguish empty arrays and tables, which makes empty array decoded from gRPC gateway be encoded as {} by cJSON.
The fix makes use of cjson.empty_array_mt to hint the cJSON.
Special thanks to @starwing, who spent their public holidays adapting new features for this issue.

Checklist

Full changelog

  • Bumped lua-protobuf
  • Use cjson.empty_array_t for arrays
  • Tests

Issue reference

starwing/lua-protobuf#240
Fix FTI-5002
Fix #10801

Review suggestion

  1. Please go through the discussion add metatable to decoded array and message to tell array from map starwing/lua-protobuf#240 before reviewing;
  2. *.pb.go are auto-generated

@StarlightIbuki StarlightIbuki force-pushed the fix/grpc-empty-array branch from 866ef3c to dc4300f Compare May 5, 2023 06:38
@StarlightIbuki StarlightIbuki requested a review from chronolaw May 5, 2023 06:39
@StarlightIbuki StarlightIbuki force-pushed the fix/grpc-empty-array branch 2 times, most recently from d4ae6cb to 18cc5a6 Compare May 5, 2023 06:42
@StarlightIbuki StarlightIbuki force-pushed the fix/grpc-empty-array branch from cc5bb9a to 9d271ad Compare May 5, 2023 08:16
@chronolaw chronolaw added this to the 3.3.0 milestone May 6, 2023
@StarlightIbuki StarlightIbuki force-pushed the fix/grpc-empty-array branch from 9d271ad to dc719cb Compare May 6, 2023 08:02
@chronolaw
Copy link
Contributor

chronolaw commented May 6, 2023

Need a person familiar with Go to double-check.

Copy link
Contributor

@fffonion fffonion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two non-technical minor issues, otherwise lgtm

@hbagdi hbagdi requested review from dndx, gszr and flrgh May 8, 2023 17:09
StarlightIbuki and others added 3 commits May 8, 2023 10:47
Lua does not distinguish empty arrays and tables, which makes empty array decoded from gRPC gateway be encoded as `{}` by cJSON.
The fix makes use of cjson.empty_array_mt to hint the cJSON.
Special thanks to starwing<[email protected]>, who spent their public holidays adapting new features for this issue.

Fix FTI-5002
@flrgh flrgh force-pushed the fix/grpc-empty-array branch from e416cc8 to 2d5d2f2 Compare May 8, 2023 17:48
@flrgh
Copy link
Contributor

flrgh commented May 8, 2023

rebased onto master to fix conflicts; good to merge whenever CI is 🟢

@flrgh flrgh merged commit b18252b into master May 8, 2023
@flrgh flrgh deleted the fix/grpc-empty-array branch May 8, 2023 18:22
@kikito
Copy link
Member

kikito commented May 8, 2023

This fix was added very late so it was missing a the "cherrypick into 3.3" label (notice that this PR targets master). I just added it

team-gateway-bot pushed a commit that referenced this pull request May 8, 2023
* fix(*): gRPC and cJSON's behavior to empty array

Lua does not distinguish empty arrays and tables, which makes empty array decoded from gRPC gateway be encoded as `{}` by cJSON.
The fix makes use of cjson.empty_array_mt to hint the cJSON.
Special thanks to starwing<[email protected]>, who spent their public holidays adapting new features for this issue.

Fix FTI-5002

* Apply suggestions from code review

Co-authored-by: Wangchong Zhou <[email protected]>

* Apply suggestions from code review

---------

Co-authored-by: Wangchong Zhou <[email protected]>
(cherry picked from commit b18252b)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[gRPC-gateway] Empty arrays are decoded as {} (which should be [])
5 participants