Skip to content

Commit cfbaaa1

Browse files
authored
refactor: migrate from deprecated jsonpb to modern protojson APIs (#1890)
* refactor: migrate from deprecated jsonpb to modern protojson APIs Replace github.com/golang/protobuf/jsonpb with google.golang.org/protobuf/encoding/protojson and google.golang.org/protobuf/types/dynamicpb for better performance and future compatibility. - Remove deprecated jsonpb.Marshaler/Unmarshaler usage - Migrate to dynamicpb.NewMessage for dynamic message creation - Update anyResolver to implement modern protoreflect interfaces - Preserve cross-package Any field resolution functionality (PR #425) - Update tests to use modern resolver methods - Maintain backward compatibility for all public APIs All tests pass, ensuring no functional regressions. * fix: resolve linting errors in protobuf migration - Replace unused method receivers with underscore - Use errors.New instead of fmt.Errorf for static messages - Replace interface{} with any for Go 1.18+ compatibility - Remove commented code and unnecessary nolint directives - Use t.Log instead of t.Logf for simple messages * refactor: clean up protobuf deserialization comments * test: update protobuf error assertion message * test: fix protobuf field ordering in message comparison
1 parent d7ec450 commit cfbaaa1

File tree

5 files changed

+271
-54
lines changed

5 files changed

+271
-54
lines changed

backend/go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ require (
2525
github.com/go-git/go-billy/v5 v5.6.2
2626
github.com/go-git/go-git/v5 v5.16.2
2727
github.com/go-viper/mapstructure/v2 v2.3.0
28-
github.com/golang/protobuf v1.5.4
2928
github.com/google/go-cmp v0.7.0
3029
github.com/google/uuid v1.6.0
3130
github.com/gorilla/schema v1.4.1
@@ -134,6 +133,7 @@ require (
134133
github.com/gogo/protobuf v1.3.2 // indirect
135134
github.com/golang-jwt/jwt/v5 v5.2.2 // indirect
136135
github.com/golang/groupcache v0.0.0-20241129210726-2c02b8208cf8 // indirect
136+
github.com/golang/protobuf v1.5.4 // indirect
137137
github.com/golang/snappy v1.0.0 // indirect
138138
github.com/google/cel-go v0.25.0 // indirect
139139
github.com/google/pprof v0.0.0-20250607225305-033d6d78b36a // indirect

backend/pkg/api/handle_topic_messages_integration_test.go

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -865,23 +865,21 @@ func (s *APIIntegrationTestSuite) TestPublishMessages() {
865865

866866
objTime := time.Date(2023, time.September, 12, 10, 0, 0, 0, time.UTC)
867867

868-
expectedData, err := proto.Marshal(&things.Item{
868+
expectedObj := &things.Item{
869869
Id: "321",
870870
Name: "item_0",
871871
Version: 1,
872872
CreatedAt: timestamppb.New(objTime),
873-
})
874-
require.NoError(err)
875-
876-
assert.Equal(expectedData, record.Value)
873+
}
877874

878875
obj2 := things.Item{}
879876
err = proto.Unmarshal(record.Value, &obj2)
880877

881878
require.NoError(err)
882-
assert.Equal("321", obj2.Id)
883-
assert.Equal("item_0", obj2.Name)
884-
assert.Equal(timestamppb.New(objTime), obj2.CreatedAt)
879+
assert.Equal(expectedObj.Id, obj2.Id)
880+
assert.Equal(expectedObj.Name, obj2.Name)
881+
assert.Equal(expectedObj.Version, obj2.Version)
882+
assert.True(expectedObj.CreatedAt.AsTime().Equal(obj2.CreatedAt.AsTime()))
885883
})
886884

887885
t.Run("protobuf message - fail", func(t *testing.T) {
@@ -907,7 +905,7 @@ func (s *APIIntegrationTestSuite) TestPublishMessages() {
907905
assert.Nil(res)
908906

909907
require.Error(err)
910-
assert.Contains(err.Error(), "invalid_argument: failed to serialize json protobuf payload: failed to unmarshal protobuf message from JSON: bad Timestamp: parsing time")
908+
assert.Contains(err.Error(), "failed to serialize json protobuf payload: failed to unmarshal JSON into protobuf message: proto:")
911909
var connectErr *connect.Error
912910
require.True(errors.As(err, &connectErr))
913911
details := connectErr.Details()

backend/pkg/proto/service.go

Lines changed: 82 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,16 @@ import (
1919
"sync"
2020
"time"
2121

22-
//nolint:staticcheck // Switching to the google golang protojson comes with a few breaking changes.
23-
"github.com/golang/protobuf/jsonpb"
2422
"github.com/jhump/protoreflect/desc"
2523
"github.com/jhump/protoreflect/desc/protoparse"
2624
"github.com/jhump/protoreflect/dynamic"
2725
"github.com/jhump/protoreflect/dynamic/msgregistry"
2826
"github.com/twmb/franz-go/pkg/sr"
2927
"golang.org/x/sync/singleflight"
30-
"google.golang.org/protobuf/runtime/protoiface"
28+
"google.golang.org/protobuf/encoding/protojson"
29+
"google.golang.org/protobuf/proto"
30+
"google.golang.org/protobuf/reflect/protoreflect"
31+
"google.golang.org/protobuf/types/dynamicpb"
3132

3233
"github.com/redpanda-data/console/backend/pkg/config"
3334
"github.com/redpanda-data/console/backend/pkg/filesystem"
@@ -165,10 +166,18 @@ func (s *Service) DeserializeProtobufMessageToJSON(payload []byte, md *desc.Mess
165166
return nil, fmt.Errorf("failed to unmarshal payload into protobuf message: %w", err)
166167
}
167168

168-
jsonBytes, err := msg.MarshalJSONPB(&jsonpb.Marshaler{
169-
AnyResolver: &anyResolver{s.registry},
170-
EmitDefaults: true,
171-
})
169+
modernDesc := convertDescToProtoReflect(md)
170+
modernMsg := dynamicpb.NewMessage(modernDesc)
171+
172+
err = proto.Unmarshal(payload, modernMsg)
173+
if err != nil {
174+
return nil, fmt.Errorf("failed to unmarshal payload into modern protobuf message: %w", err)
175+
}
176+
177+
jsonBytes, err := protojson.MarshalOptions{
178+
EmitDefaultValues: true,
179+
Resolver: &anyResolver{mr: s.registry},
180+
}.Marshal(modernMsg)
172181
if err != nil {
173182
return nil, fmt.Errorf("failed to marshal protobuf message to JSON: %w", err)
174183
}
@@ -178,16 +187,21 @@ func (s *Service) DeserializeProtobufMessageToJSON(payload []byte, md *desc.Mess
178187

179188
// SerializeJSONToProtobufMessage serializes the JSON data to Protobuf message.
180189
func (s *Service) SerializeJSONToProtobufMessage(json []byte, md *desc.MessageDescriptor) ([]byte, error) {
181-
msg := dynamic.NewMessage(md)
182-
err := msg.UnmarshalJSONPB(&jsonpb.Unmarshaler{
183-
AnyResolver: &anyResolver{s.registry},
184-
AllowUnknownFields: true,
185-
}, json)
190+
// Convert to modern dynamicpb message
191+
modernDesc := convertDescToProtoReflect(md)
192+
modernMsg := dynamicpb.NewMessage(modernDesc)
193+
194+
// Use modern protojson to unmarshal
195+
err := protojson.UnmarshalOptions{
196+
Resolver: &anyResolver{mr: s.registry},
197+
DiscardUnknown: true,
198+
}.Unmarshal(json, modernMsg)
186199
if err != nil {
187-
return nil, fmt.Errorf("failed to unmarshal protobuf message from JSON: %w", err)
200+
return nil, fmt.Errorf("failed to unmarshal JSON into protobuf message: %w", err)
188201
}
189202

190-
return msg.Marshal()
203+
// Marshal back to binary format
204+
return proto.Marshal(modernMsg)
191205
}
192206

193207
// GetMessageDescriptorForSchema gets the Protobuf message descriptor for the schema ID and message index.
@@ -227,26 +241,30 @@ func (s *Service) SerializeJSONToConfluentProtobufMessage(json []byte, schemaID
227241
return nil, err
228242
}
229243

230-
msg := dynamic.NewMessage(messageDescriptor)
231-
err = msg.UnmarshalJSONPB(&jsonpb.Unmarshaler{
232-
AnyResolver: &anyResolver{s.registry},
233-
AllowUnknownFields: true,
234-
}, json)
244+
// Convert to modern dynamicpb message
245+
modernDesc := convertDescToProtoReflect(messageDescriptor)
246+
modernMsg := dynamicpb.NewMessage(modernDesc)
247+
248+
// Use modern protojson to unmarshal
249+
err = protojson.UnmarshalOptions{
250+
Resolver: &anyResolver{mr: s.registry},
251+
DiscardUnknown: true,
252+
}.Unmarshal(json, modernMsg)
235253
if err != nil {
236-
return nil, fmt.Errorf("failed to unmarshal protobuf message from JSON: %w", err)
254+
return nil, fmt.Errorf("failed to unmarshal JSON into protobuf message: %w", err)
237255
}
238256

239257
var srSerde sr.Serde
240258
srSerde.Register(
241259
schemaID,
242-
&dynamic.Message{},
260+
modernMsg,
243261
sr.EncodeFn(func(v any) ([]byte, error) {
244-
return v.(*dynamic.Message).Marshal()
262+
return proto.Marshal(v.(proto.Message))
245263
}),
246264
sr.Index(index...),
247265
)
248266

249-
return srSerde.Encode(msg)
267+
return srSerde.Encode(modernMsg)
250268
}
251269

252270
// UnmarshalPayload tries to deserialize a protobuf encoded payload to a JSON message,
@@ -550,11 +568,11 @@ func (s *Service) GetFileDescriptorBySchemaID(schemaID int) (*desc.FileDescripto
550568
s.fileDescriptorsBySchemaIDMutex.Lock()
551569
defer s.fileDescriptorsBySchemaIDMutex.Unlock()
552570

553-
desc, exists := s.fileDescriptorsBySchemaID[schemaID]
554-
return desc, exists
571+
fd, exists := s.fileDescriptorsBySchemaID[schemaID]
572+
return fd, exists
555573
}
556574

557-
// AnyResolver is used to resolve the google.protobuf.Any type.
575+
// anyResolver is used to resolve the google.protobuf.Any type.
558576
// It takes a type URL, present in an Any message, and resolves
559577
// it into an instance of the associated message.
560578
//
@@ -569,14 +587,46 @@ type anyResolver struct {
569587
mr *msgregistry.MessageRegistry
570588
}
571589

572-
func (r *anyResolver) Resolve(typeURL string) (protoiface.MessageV1, error) {
573-
// Protoreflect registers the type by stripping the contents before the last
574-
// slash. Therefore we need to mimic this behaviour in order to resolve
575-
// the type by it's given type url.
576-
mname := typeURL
590+
// FindMessageByName implements protoreflect.MessageTypeResolver
591+
func (r *anyResolver) FindMessageByName(message protoreflect.FullName) (protoreflect.MessageType, error) {
592+
// Convert to string and resolve using the existing registry
593+
msg, err := r.mr.Resolve(string(message))
594+
if err != nil {
595+
return nil, err
596+
}
597+
// Convert dynamic message to modern API
598+
if dynMsg, ok := msg.(*dynamic.Message); ok {
599+
// Get the descriptor and convert to modern format
600+
msgDesc := dynMsg.GetMessageDescriptor()
601+
// Convert jhump descriptor to modern protoreflect descriptor
602+
modernDesc := msgDesc.UnwrapMessage()
603+
return dynamicpb.NewMessageType(modernDesc), nil
604+
}
605+
return nil, fmt.Errorf("message type %s not found", message)
606+
}
607+
608+
// FindMessageByURL implements protoreflect.MessageTypeResolver
609+
func (r *anyResolver) FindMessageByURL(url string) (protoreflect.MessageType, error) {
610+
// Strip the URL prefix to get the message name
611+
mname := url
577612
if slash := strings.LastIndex(mname, "/"); slash >= 0 {
578613
mname = mname[slash+1:]
579614
}
615+
return r.FindMessageByName(protoreflect.FullName(mname))
616+
}
617+
618+
// FindExtensionByName implements protoreflect.ExtensionTypeResolver
619+
func (*anyResolver) FindExtensionByName(_ protoreflect.FullName) (protoreflect.ExtensionType, error) {
620+
return nil, errors.New("extension resolution not supported")
621+
}
622+
623+
// FindExtensionByNumber implements protoreflect.ExtensionTypeResolver
624+
func (*anyResolver) FindExtensionByNumber(_ protoreflect.FullName, _ protoreflect.FieldNumber) (protoreflect.ExtensionType, error) {
625+
return nil, errors.New("extension resolution not supported")
626+
}
580627

581-
return r.mr.Resolve(mname)
628+
// convertDescToProtoReflect converts a jhump MessageDescriptor to a protoreflect.MessageDescriptor
629+
func convertDescToProtoReflect(md *desc.MessageDescriptor) protoreflect.MessageDescriptor {
630+
// Use the Unwrap method to get the modern descriptor
631+
return md.UnwrapMessage()
582632
}

0 commit comments

Comments
 (0)