all: replace all uses of golang/protobuf/proto #1358

johanbrandhorst · 2020-05-17T19:19:51Z

We have to keep using the old ptypes, descriptor
and plugin packages since rules_go forces us to use them.
Eventually rules_go should move to the new API and we can
purge the use of the golang/protobuf package altogether.

codecov-io · 2020-05-17T19:22:24Z

Codecov Report

Merging #1358 into v2 will increase coverage by 0.19%.
The diff coverage is 61.11%.

@@            Coverage Diff             @@
##               v2    #1358      +/-   ##
==========================================
+ Coverage   53.95%   54.14%   +0.19%     
==========================================
  Files          40       40              
  Lines        4302     4316      +14     
==========================================
+ Hits         2321     2337      +16     
+ Misses       1725     1724       -1     
+ Partials      256      255       -1

Impacted Files	Coverage Δ
examples/internal/server/a_bit_of_everything.go	`0.00% <0.00%> (ø)`
internal/codegenerator/parse_req.go	`100.00% <ø> (ø)`
internal/descriptor/registry.go	`58.71% <ø> (ø)`
...-gen-grpc-gateway/internal/gengateway/generator.go	`37.86% <0.00%> (ø)`
protoc-gen-swagger/main.go	`26.12% <ø> (ø)`
runtime/marshal_httpbodyproto.go	`76.92% <0.00%> (ø)`
runtime/marshal_proto.go	`55.55% <ø> (ø)`
runtime/marshaler_registry.go	`85.71% <ø> (ø)`
runtime/mux.go	`61.06% <ø> (ø)`
runtime/proto2_convert.go	`0.00% <ø> (ø)`
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6cd4e05...2f16422. Read the comment docs.

johanbrandhorst · 2020-05-17T20:45:16Z

protoc-gen-swagger/internal/genswagger/generator.go

+	// TODO(johanbrandhorst): Use new conversion later when possible
+	// any := protodesc.ToFileDescriptorProto((&anypb.Any{}).ProtoReflect().Descriptor().ParentFile())
+	// status := protodesc.ToFileDescriptorProto((&statuspb.Status{}).ProtoReflect().Descriptor().ParentFile())


This is not possible yet because rules_go locks us into the older ptypes

johanbrandhorst · 2020-05-17T21:00:24Z

examples/internal/integration/integration_test.go

-		{
-			endpoint:     fmt.Sprintf("http://localhost:%d/responsestrings/foo", port),
-			expectedCode: http.StatusOK,
-			expectedBody: `["hello","foo"]`,


Tests that relied on exact JSON formatting have had to be rewritten to marshal to types before comparing (see golang/protobuf#1121)

johanbrandhorst · 2020-05-17T21:01:54Z

examples/internal/integration/integration_test.go

 		requestContent io.Reader
 	}{
 		{
 			name:        "get url query values",
 			httpMethod:  "GET",
 			contentType: "application/json",
 			apiURL:      fmt.Sprintf("http://localhost:%d/v1/example/a_bit_of_everything/params/get/foo?double_value=%v&bool_value=%v", port, 1234.56, true),
-			wantContent: `{"single_nested":{"name":"foo"},"double_value":1234.56,"bool_value":true}`,


More converted tests

johanbrandhorst · 2020-05-17T21:16:04Z

protoc-gen-swagger/internal/genswagger/template_test.go

@@ -1051,11 +1053,36 @@ func TestApplyTemplateExtensions(t *testing.T) {
 	if want, is, name := "2.0", result.Swagger, "Swagger"; !reflect.DeepEqual(is, want) {
 		t.Errorf("applyTemplate(%#v).%s = %s want to be %s", file, name, is, want)
 	}
-	if want, is, name := []extension{
-		{key: "x-bar", value: json.RawMessage("[\n      \"baz\"\n    ]")},
-		{key: "x-foo", value: json.RawMessage("\"bar\"")},


More JSON tests replaced

johanbrandhorst · 2020-05-17T21:19:04Z

runtime/convert_test.go

+			switch {
+			case err != nil && !spec.wanterr:
+				t.Errorf("got unexpected error\n%#v", err)
+			case err == nil && spec.wanterr:
+				t.Errorf("did not error when expecte")
+			case !proto.Equal(ts, spec.output):


Replaced direct error comparison with boolean test (we don't care what the error looks like).

johanbrandhorst · 2020-05-17T21:19:57Z

runtime/errors_test.go

-			continue
-		}
-
-		if got, want := body["message"].(string), spec.msg; !strings.Contains(got, want) {


Replace this nonsense with explicit status unmarshalling

johanbrandhorst · 2020-05-17T21:34:33Z

runtime/marshal_jsonpb.go

+			rv.Set(reflect.Append(rv, bv.Elem()))
+		}
+		return nil
+	}


I had to add this to support marshalling slices of proto messages, not sure how this worked before.

johanbrandhorst · 2020-05-17T21:36:52Z

runtime/marshal_jsonpb_test.go

 	}{
 		{
 			input: &examplepb.ResponseBodyOut{
 				Response: &examplepb.ResponseBodyOut_Response{Data: "abcdef"},
 			},
-			verifier: func(json string) {
-				expected := `{"response":{"data":"abcdef"}}`


More JSON testing changes

johanbrandhorst · 2020-05-17T21:43:40Z

runtime/marshaler_registry.go

+			// TODO(johanbrandhorst): Change this to true before v2
+			EmitUnpopulated: false,
+			// TODO(johanbrandhorst): Change this to false before v2
+			UseProtoNames: true,


These changes will be the next PR

We have to keep using the old ptypes, descriptor and plugin packages since rules_go forces us to use them. Eventually rules_go should move to the new API and we can purge the use of the golang/protobuf package altogether.

johanbrandhorst · 2020-05-17T21:46:22Z

Alright, as this is a pretty substantial PR I've annotated the larger changes made. Please take a look if you have time @achew22. I'll merge in a day regardless.

googlebot added the cla: yes label May 17, 2020

johanbrandhorst force-pushed the jsonpb-default branch from 33c809e to eeda767 Compare May 17, 2020 19:52

johanbrandhorst mentioned this pull request May 17, 2020

v2 release planning #1223

Closed

13 tasks

johanbrandhorst force-pushed the jsonpb-default branch 3 times, most recently from 2f16422 to 7b189ae Compare May 17, 2020 20:31

johanbrandhorst commented May 17, 2020

View reviewed changes

johanbrandhorst force-pushed the jsonpb-default branch from 7b189ae to af0bb36 Compare May 17, 2020 21:27

johanbrandhorst commented May 17, 2020

View reviewed changes

johanbrandhorst force-pushed the jsonpb-default branch from af0bb36 to 39fa0ac Compare May 17, 2020 21:42

johanbrandhorst commented May 17, 2020

View reviewed changes

johanbrandhorst force-pushed the jsonpb-default branch from 39fa0ac to 1b9c7a9 Compare May 17, 2020 21:45

all: replace all uses of golang/protobuf/proto

183caa7

We have to keep using the old ptypes, descriptor and plugin packages since rules_go forces us to use them. Eventually rules_go should move to the new API and we can purge the use of the golang/protobuf package altogether.

johanbrandhorst force-pushed the jsonpb-default branch from 1b9c7a9 to 183caa7 Compare May 17, 2020 21:45

johanbrandhorst merged commit 899f1f0 into v2 May 19, 2020

johanbrandhorst deleted the jsonpb-default branch May 19, 2020 09:34

This was referenced Aug 18, 2023

replace all uses of golang/protobuf/protobuf #3511

Closed

fix(deps): replace all uses of golang/protobuf/protobuf #3516

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

all: replace all uses of golang/protobuf/proto #1358

all: replace all uses of golang/protobuf/proto #1358

johanbrandhorst commented May 17, 2020

codecov-io commented May 17, 2020 •

edited

Loading

johanbrandhorst May 17, 2020

johanbrandhorst May 17, 2020

johanbrandhorst May 17, 2020

johanbrandhorst May 17, 2020

johanbrandhorst May 17, 2020

johanbrandhorst May 17, 2020

johanbrandhorst May 17, 2020

johanbrandhorst May 17, 2020

johanbrandhorst May 17, 2020

johanbrandhorst commented May 17, 2020

all: replace all uses of golang/protobuf/proto #1358

all: replace all uses of golang/protobuf/proto #1358

Conversation

johanbrandhorst commented May 17, 2020

codecov-io commented May 17, 2020 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johanbrandhorst commented May 17, 2020

codecov-io commented May 17, 2020 •

edited

Loading