Merge pull request #1 from mercari/service-dependency-name-optional

Make ServiceDependency name optional
mercari · Aug 22, 2023 · 3c1d4d6 · 3c1d4d6
2 parents a596cf9 + c16dea1
commit 3c1d4d6
Show file tree

Hide file tree

Showing 11 changed files with 2,128 additions and 2,202 deletions.
diff --git a/_examples/04_timeout/grpc/federation/federation.pb.go b/_examples/04_timeout/grpc/federation/federation.pb.go
diff --git a/_examples/05_async/grpc/federation/federation.pb.go b/_examples/05_async/grpc/federation/federation.pb.go
diff --git a/_examples/06_alias/grpc/federation/federation.pb.go b/_examples/06_alias/grpc/federation/federation.pb.go
diff --git a/generator/testdata/expected_simple_aggregation.go b/generator/testdata/expected_simple_aggregation.go
diff --git a/grpc/federation/federation.pb.go b/grpc/federation/federation.pb.go
diff --git a/proto/grpc/federation/federation.proto b/proto/grpc/federation/federation.proto
@@ -58,7 +58,7 @@ message ServiceRule {
 // ServiceDependency assigns a uniquely determined name to the service.
 message ServiceDependency {
   // name to be used when initializing the gRPC client.
-  string name = 1;
+  optional string name = 1;
   // service is the name of the dependent service.
   string service = 2;
 }

diff --git a/resolver/resolver.go b/resolver/resolver.go
@@ -292,15 +292,6 @@ func (r *Resolver) resolveService(ctx *context, pkg *Package, name string) *Serv
 }
 
 func (r *Resolver) resolveServiceDependency(ctx *context, def *federation.ServiceDependency) *ServiceDependency {
-	name := def.GetName()
-	if name == "" {
-		ctx.addError(
-			ErrWithLocation(
-				`"name" must be specified`,
-				source.ServiceDependencyNameLocation(ctx.fileName(), ctx.serviceName(), ctx.depIndex()),
-			),
-		)
-	}
 	var service *Service
 	serviceWithPkgName := def.GetService()
 	if serviceWithPkgName == "" {
@@ -333,7 +324,7 @@ func (r *Resolver) resolveServiceDependency(ctx *context, def *federation.Servic
 			}
 		}
 	}
-	return &ServiceDependency{Name: name, Service: service}
+	return &ServiceDependency{Name: def.GetName(), Service: service}
 }
 
 func (r *Resolver) resolveMethod(ctx *context, service *Service, methodDef *descriptorpb.MethodDescriptorProto) *Method {

diff --git a/resolver/resolver_test.go b/resolver/resolver_test.go
@@ -357,8 +357,8 @@ func TestSimpleAggregation(t *testing.T) {
 				).
 				SetRule(
 					testutil.NewServiceRuleBuilder().
-						AddDependency("post_service", ref.Service(t, "org.post", "PostService")).
-						AddDependency("user_service", ref.Service(t, "org.user", "UserService")).
+						AddDependency("", ref.Service(t, "org.post", "PostService")).
+						AddDependency("", ref.Service(t, "org.user", "UserService")).
 						Build(t),
 				).
 				AddMessage(ref.Message(t, "org.federation", "GetPostResponse"), ref.Message(t, "org.federation", "GetPostResponseArgument")).

diff --git a/testdata/simple_aggregation.proto b/testdata/simple_aggregation.proto
@@ -12,8 +12,8 @@ option go_package = "example/federation;federation";
 service FederationService {
   option (grpc.federation.service) = {
     dependencies: [
-      { name: "post_service", service: "org.post.PostService" },
-      { name: "user_service", service: "org.user.UserService" }
+      { service: "org.post.PostService" },
+      { service: "org.user.UserService" }
     ]
   };
   rpc GetPost(GetPostRequest) returns (GetPostResponse) {

diff --git a/validator/testdata/missing_service_dependency_name.proto b/validator/testdata/missing_service_dependency_name.proto
diff --git a/validator/validator_test.go b/validator/validator_test.go
@@ -287,14 +287,6 @@ testdata/invalid_service_dependency_service.proto: "user.InvalidService" service
 testdata/invalid_service_dependency_service.proto:15:42: "user.InvalidService" does not exist
 15:          { name: "user_service", service: "user.InvalidService" }
                                               ^
-`},
-		{file: "missing_service_dependency_name.proto", expected: `
-testdata/missing_service_dependency_name.proto:14:9: "name" must be specified
-14:          { service: "post.PostService" },
-             ^
-testdata/missing_service_dependency_name.proto:15:9: "name" must be specified
-15:          { service: "user.UserService" }
-             ^
 `},
 		{file: "missing_service_dependency_service.proto", expected: `
 testdata/missing_service_dependency_service.proto:14:9: "service" must be specified