fieldmask-utils icon indicating copy to clipboard operation
fieldmask-utils copied to clipboard

Convert protobuf message to map after applying a field mask doesn't work

Open marcoshuck opened this issue 5 months ago • 5 comments

Context

I'm trying to convert a proto message to map using this library in order to perform a PATCH update in an endpoint. I'm getting an empty map instead of a single field map with the respective value.

Relevant code

mask := &fieldmaskpb.FieldMask{Paths: []string{"title"}}
mask.Normalize()
task := &tasksv1.Task{
  Title: "test",
}
if !mask.IsValid(task) {
  return nil, errors.New("invalid mask")
}
protoMask, err := fieldmask_utils.MaskFromProtoFieldMask(mask, func(s string) string { return s })
if err != nil {
  return nil, err
}
m := make(map[string]any)
if err = fieldmask_utils.StructToMap(protoMask, task, m); err != nil {
  return nil, err
}
fmt.Println("Map:", m)

Current behavior

m is empty

Expected behavior

m := map[string]any{
  "title": "test",
}

Reference

Protobuf messages: https://github.com/marcoshuck/todo/blob/main/api/tasks/v1/tasks.proto#L60

marcoshuck avatar Jan 22 '24 19:01 marcoshuck

@mennanov any chances you can take a look at this?

marcoshuck avatar Jan 26 '24 18:01 marcoshuck

I'll take a look within the next few days

mennanov avatar Jan 27 '24 12:01 mennanov

I wrote a similar test that passes:

func TestTitleBug(t *testing.T) {
	mask := &fieldmaskpb.FieldMask{Paths: []string{"title"}}
	mask.Normalize()
	task := &testproto.Task{
		Title: "test",
	}
	require.True(t, mask.IsValid(task))
	protoMask, err := fieldmask_utils.MaskFromProtoFieldMask(mask, func(s string) string { return generator.CamelCase(s) })
	require.Nil(t, err)
	m := make(map[string]any)
	err = fieldmask_utils.StructToMap(protoMask, task, m)
	require.Nil(t, err)
	assert.Equal(t, map[string]any{"Title": "test"}, m)
}

Try using generator.CamelCase(s) which converts a lower case field name (in the field mask) into a camel case (golang struct naming).

mennanov avatar Jan 29 '24 17:01 mennanov

Thanks for the information, will give it a try! We should probably add this test case as it would help future devs that find this issue.

I can take care of creating a PR for this test and adding some docs about the generator package (which I was completely unaware of).

marcoshuck avatar Jan 30 '24 21:01 marcoshuck

A PR is welcomed!

mennanov avatar Jan 30 '24 22:01 mennanov

Resolved in PR #42

mennanov avatar Feb 15 '24 06:02 mennanov