TrueCloudLab/frostfs-api-go

Fork 16

protogen: Always marshal empty fields #122

Merged

fyrchik merged 1 commit from fyrchik/frostfs-api-go:fix-enum-marshaling into master

2024-10-07 12:12:41 +00:00

fyrchik commented

2024-10-07 12:04:55 +00:00

Owner

This is how it was done previously:

return protojson.MarshalOptions{

The tricky part is []byte which is marshaled as null by easyjson
helper, but as "" by protojson.

Signed-off-by: Evgenii Stratonikov e.stratonikov@yadro.com

This is how it was done previously: https://git.frostfs.info/TrueCloudLab/frostfs-api-go/src/commit/a0a9b765f3a56423b1c6f1fef1d1b413da0e03ef/rpc/message/encoding.go#L31 The tricky part is `[]byte` which is marshaled as `null` by easyjson helper, but as `""` by protojson. Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>

fyrchik added 1 commit 2024-10-07 12:04:56 +00:00

protogen: Always marshal empty fields

DCO action / DCO (pull_request) Failing after 56s

Details

Tests and linters / Tests with -race (pull_request) Successful in 1m10s

Details

Tests and linters / Lint (pull_request) Successful in 1m15s

Details

Tests and linters / Tests (pull_request) Successful in 1m29s

Details

1ec1762d3a

This is how it was done previously:
a0a9b765f3/rpc/message/encoding.go (L31)

The tricky part is `[]byte` which is marshaled as `null` by easyjson
helper, but as `""` by protojson.

Signed-off-by: Evgenii Stratonikov <e.stratonikov@yadro.com>

fyrchik requested review from storage-core-committers 2024-10-07 12:05:23 +00:00

fyrchik requested review from storage-core-developers 2024-10-07 12:05:30 +00:00

fyrchik force-pushed fix-enum-marshaling from 1ec1762d3a to 29c522d5d8

2024-10-07 12:05:52 +00:00

Compare

dstepanov-yadro approved these changes 2024-10-07 12:10:55 +00:00

fyrchik merged commit 29c522d5d8 into master

2024-10-07 12:12:41 +00:00

fyrchik referenced this pull request from a commit

2024-10-07 12:12:45 +00:00

[#122] protogen: Always marshal empty fields

fyrchik deleted branch fix-enum-marshaling

2024-10-07 12:12:46 +00:00

alexvanin commented

2024-10-10 15:13:52 +00:00

Owner

The tricky part is []byte which is marshaled as null by easyjson helper, but as "" by protojson.

Seems like it breaks this test in SDK

Not equal: 

expected: &object.Object{objectID:(*refs.ObjectID)(0xc0001247e0), idSig:(*refs.Signature)(nil), header:(*object.Header)(0xc0001cc5b0), payload:[]uint8(nil), marshalData:[]uint8(nil)}

actual  : &object.Object{objectID:(*refs.ObjectID)(0xc0001248b8), idSig:(*refs.Signature)(nil), header:(*object.Header)(0xc0001ccc40), payload:[]uint8{}, marshalData:[]uint8(nil)}

/cc @fyrchik @pogpp

> The tricky part is []byte which is marshaled as null by easyjson helper, but as "" by protojson. Seems like it breaks [this](https://git.frostfs.info/TrueCloudLab/frostfs-sdk-go/src/commit/99c5c583650992659175bce9df256b4398c35ebe/object/raw_test.go#L299) test in SDK ``` Not equal: expected: &object.Object{objectID:(*refs.ObjectID)(0xc0001247e0), idSig:(*refs.Signature)(nil), header:(*object.Header)(0xc0001cc5b0), payload:[]uint8(nil), marshalData:[]uint8(nil)} actual : &object.Object{objectID:(*refs.ObjectID)(0xc0001248b8), idSig:(*refs.Signature)(nil), header:(*object.Header)(0xc0001ccc40), payload:[]uint8{}, marshalData:[]uint8(nil)} ``` /cc @fyrchik @pogpp

fyrchik commented

2024-10-10 15:27:36 +00:00

Author

Owner

I will look into it, the current behaviour should've been identical to the protojson lib.

fyrchik commented

2024-10-10 19:57:32 +00:00

Author

Owner

I have found the bug -- empty string seem indeed to unmarshal to nil, which is strange, considered that I have explicitly checked this exact behaviour in the PR, probably missed something.

fyrchik referenced this pull request

2024-10-11 07:21:45 +00:00

protogen: Treat bytes field as non-nullable #123

fyrchik referenced this pull request from a commit

2024-10-11 12:15:47 +00:00

[#123] protogen: Treat bytes field as non-nullable