feat(internal/gengapic): include API version in gapic metadata (#1653)

noahdietz · web-flow · commit a098fbadd24b · 2025-11-03T09:26:35.000-08:00
diff --git a/internal/gengapic/client_init.go b/internal/gengapic/client_init.go
@@ -19,6 +19,8 @@ import (
 	"strings"
 
 	"github.com/googleapis/gapic-generator-go/internal/pbinfo"
+	"google.golang.org/genproto/googleapis/api/annotations"
+	"google.golang.org/protobuf/proto"
 	"google.golang.org/protobuf/types/descriptorpb"
 )
 
@@ -380,6 +382,9 @@ func (g *generator) makeClients(serv *descriptorpb.ServiceDescriptorProto, servN
 		return err
 	}
 
+	apiVersion := proto.GetExtension(serv.Options, annotations.E_ApiVersion).(string)
+	g.addMetadataServiceEntry(serv.GetName(), apiVersion)
+
 	err = g.internalClientIntfInit(serv, servName)
 	if err != nil {
 		return err
diff --git a/internal/gengapic/client_init_test.go b/internal/gengapic/client_init_test.go
@@ -330,7 +330,8 @@ func TestClientInit(t *testing.T) {
 		},
 		Options: &descriptorpb.ServiceOptions{},
 	}
-	proto.SetExtension(servPlain.Options, annotations.E_ApiVersion, "v1_20240425")
+	servPlainAPIVersion := "v1_20240425"
+	proto.SetExtension(servPlain.Options, annotations.E_ApiVersion, servPlainAPIVersion)
 
 	servLRO := &descriptorpb.ServiceDescriptorProto{
 		Name: proto.String("Foo"),
@@ -592,6 +593,12 @@ func TestClientInit(t *testing.T) {
 			g.snippetMetadata = sm
 			g.makeClients(tst.serv, tst.servName)
 
+			if md, ok := g.metadata.GetServices()[tst.serv.GetName()]; !ok {
+				t.Errorf("ClientInit(%s) gapic metadata, expected %s to be present but found %+v", tst.tstName, tst.serv.GetName(), g.metadata.GetServices())
+			} else if tst.serv == servPlain && md.GetApiVersion() != servPlainAPIVersion {
+				t.Errorf("ClinitInit(%s) gapic metadata service entry api version, got %q, want %q", tst.tstName, md.GetApiVersion(), servPlainAPIVersion)
+			}
+
 			if diff := cmp.Diff(g.imports, tst.imports); diff != "" {
 				t.Errorf("ClientInit(%s) imports got(-),want(+):\n%s", tst.tstName, diff)
 			}
diff --git a/internal/gengapic/metadata.go b/internal/gengapic/metadata.go
@@ -35,33 +35,65 @@ func (g *generator) genGapicMetadataFile() error {
 	return nil
 }
 
+// Initializes the service metadata entry with the API version, if set.
+// Per-transport clients will be added as they are generated.
+func (g *generator) addMetadataServiceEntry(service, apiVersion string) {
+	if g.metadata.Services == nil {
+		g.metadata.Services = make(map[string]*metadata.GapicMetadata_ServiceForTransport)
+	}
+	_, ok := g.metadata.GetServices()[service]
+	if !ok {
+		s := &metadata.GapicMetadata_ServiceForTransport{
+			Clients:    make(map[string]*metadata.GapicMetadata_ServiceAsClient),
+			ApiVersion: apiVersion,
+		}
+		g.metadata.Services[service] = s
+	}
+}
+
 // Adds a metadata structure for the (service, transport) combination.
+// Will exit early if addMetadataServiceEntry is not called prior to this.
 // This method is idempotent.
 func (g *generator) addMetadataServiceForTransport(service, transport, lib string) {
+	if g.metadata.Services == nil {
+		return
+	}
+
 	s, ok := g.metadata.GetServices()[service]
 	if !ok {
-		s = &metadata.GapicMetadata_ServiceForTransport{
-			Clients: make(map[string]*metadata.GapicMetadata_ServiceAsClient),
-		}
-		g.metadata.Services[service] = s
+		return
 	}
 
 	if _, ok := s.Clients[transport]; !ok {
 		s.Clients[transport] = &metadata.GapicMetadata_ServiceAsClient{
 			// The "Client" part of the generated type's name is hard-coded in the
 			// generator so we need to append it to the lib name.
-			//
-			// TODO(noahdietz): when REGAPIC is added we may need to special-case based
-			// on transport.
 			LibraryClient: lib + "Client",
 			Rpcs:          make(map[string]*metadata.GapicMetadata_MethodList),
 		}
 	}
 }
 
+// Adds a metadata service transport client method entry for the given RPC.
+// Will exit early if addMetadataServiceEntry or addMetadaServiceForTransport
+// are not called prior to this.
 func (g *generator) addMetadataMethod(service, transport, rpc string) {
+	if g.metadata.Services == nil {
+		return
+	}
+	s, ok := g.metadata.GetServices()[service]
+	if !ok {
+		return
+	}
+	c, ok := s.GetClients()[transport]
+	if !ok {
+		return
+	}
+	if c.GetRpcs() == nil {
+		c.Rpcs = make(map[string]*metadata.GapicMetadata_MethodList)
+	}
 	// There is only one method per RPC on a generated Go client, with the same name as the RPC.
-	g.metadata.GetServices()[service].GetClients()[transport].GetRpcs()[rpc] = &metadata.GapicMetadata_MethodList{
+	c.GetRpcs()[rpc] = &metadata.GapicMetadata_MethodList{
 		Methods: []string{rpc},
 	}
 }
diff --git a/internal/gengapic/metadata_test.go b/internal/gengapic/metadata_test.go
@@ -23,6 +23,79 @@ import (
 	"google.golang.org/protobuf/proto"
 )
 
+func TestAddMetadataServiceEntry(t *testing.T) {
+	for _, tst := range []struct {
+		name, service, apiVersion string
+		init, want                *metadata.GapicMetadata
+	}{
+		{
+			name:       "empty_metadata",
+			service:    "Fooer",
+			apiVersion: "v1",
+			init:       &metadata.GapicMetadata{},
+			want: &metadata.GapicMetadata{
+				Services: map[string]*metadata.GapicMetadata_ServiceForTransport{
+					"Fooer": {
+						Clients:    make(map[string]*metadata.GapicMetadata_ServiceAsClient),
+						ApiVersion: "v1",
+					},
+				},
+			},
+		},
+		{
+			name:       "service_exists",
+			service:    "Fooer",
+			apiVersion: "v1",
+			init: &metadata.GapicMetadata{
+				Services: map[string]*metadata.GapicMetadata_ServiceForTransport{
+					"Fooer": {
+						Clients: map[string]*metadata.GapicMetadata_ServiceAsClient{
+							"grpc": {
+								LibraryClient: "FooerClient",
+							},
+						},
+					},
+				},
+			},
+			want: &metadata.GapicMetadata{
+				Services: map[string]*metadata.GapicMetadata_ServiceForTransport{
+					"Fooer": {
+						Clients: map[string]*metadata.GapicMetadata_ServiceAsClient{
+							"grpc": {
+								LibraryClient: "FooerClient",
+							},
+						},
+					},
+				},
+			},
+		},
+		{
+			name:    "no_api_version",
+			service: "Fooer",
+			init:    &metadata.GapicMetadata{},
+			want: &metadata.GapicMetadata{
+				Services: map[string]*metadata.GapicMetadata_ServiceForTransport{
+					"Fooer": {
+						Clients: make(map[string]*metadata.GapicMetadata_ServiceAsClient),
+					},
+				},
+			},
+		},
+	} {
+		t.Run(tst.name, func(t *testing.T) {
+			g := generator{
+				metadata: tst.init,
+			}
+			g.addMetadataServiceEntry(tst.service, tst.apiVersion)
+
+			if diff := cmp.Diff(g.metadata, tst.want, cmp.Comparer(proto.Equal)); diff != "" {
+				t.Errorf("addMetadataServiceEntry(%q, %q): got(-),want(+):\n%s", tst.service, tst.apiVersion, diff)
+			}
+		})
+	}
+
+}
+
 func TestAddMetadataServiceForTransport(t *testing.T) {
 	for _, tst := range []struct {
 		service, lib string
@@ -34,6 +107,20 @@ func TestAddMetadataServiceForTransport(t *testing.T) {
 			init: &metadata.GapicMetadata{
 				Services: make(map[string]*metadata.GapicMetadata_ServiceForTransport),
 			},
+			want: &metadata.GapicMetadata{
+				Services: make(map[string]*metadata.GapicMetadata_ServiceForTransport),
+			},
+		},
+		{
+			service: "LibraryService",
+			lib:     "LibraryService",
+			init: &metadata.GapicMetadata{
+				Services: map[string]*metadata.GapicMetadata_ServiceForTransport{
+					"LibraryService": {
+						Clients: make(map[string]*metadata.GapicMetadata_ServiceAsClient),
+					},
+				},
+			},
 			want: &metadata.GapicMetadata{
 				Services: map[string]*metadata.GapicMetadata_ServiceForTransport{
 					"LibraryService": {
