From e38ed14138f81055cb228e53b0d958a4b4e72ca2 Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Mon, 26 Oct 2026 09:43:15 -0400 Subject: [PATCH] fix(generate): select protobuf services by qualified identity Reuse contract narrowing for fully qualified service names and refuse ambiguous short names. This lets accounts clients select their own services from a multi-package contract without generating unrelated facades. Co-Authored-By: Claude Opus 5.5 --- cmd/generate/client.go | 2 +- cmd/generate/client_services_test.go | 21 +++++++++++++++ docs/cli-reference.md | 2 +- pkg/generators/narrow.go | 40 ++++++++++++++++++---------- pkg/generators/narrow_test.go | 37 +++++++++++++++++++++++++ 5 files changed, 86 insertions(+), 16 deletions(-) diff --git a/cmd/generate/client.go b/cmd/generate/client.go index be8168d3..11f6625a 100644 --- a/cmd/generate/client.go +++ b/cmd/generate/client.go @@ -76,7 +76,7 @@ contracts directory on disk (--from contracts:). func init() { ClientCmd.Flags().StringVar(&clientFrom, "from", "", "contract source: module/service[/endpoint], package:@, or contracts:") ClientCmd.Flags().StringSliceVar(&clientLanguages, "language", nil, "languages to generate: go,typescript,python") - ClientCmd.Flags().StringSliceVar(&clientServices, "services", nil, "restrict the library to these protobuf services and the declarations they need: contract, catalog, bindings and facade (protobuf contracts only)") + ClientCmd.Flags().StringSliceVar(&clientServices, "services", nil, "restrict the library to these protobuf services (short or fully qualified names; ambiguous short names are refused) and the declarations they need: contract, catalog, bindings and facade") ClientCmd.Flags().StringVar(&clientName, "name", "", "library name (default: --client)") ClientCmd.Flags().StringVar(&clientModuleName, "module-name", "", "facade entry-point name (default: derived from the contract)") ClientCmd.Flags().BoolVar(&clientNoFacade, "no-facade", false, "generate bindings only, no facade") diff --git a/cmd/generate/client_services_test.go b/cmd/generate/client_services_test.go index 3e1b6843..093ff599 100644 --- a/cmd/generate/client_services_test.go +++ b/cmd/generate/client_services_test.go @@ -370,3 +370,24 @@ func TestGenerateClientServicesSelectionChangesRegenerateWithoutForce(t *testing assertPythonImports(t, filepath.Join(output, "python"), "acme_sdk.acme") assertPythonImports(t, filepath.Join(output, "python"), "acme_sdk.other") } + +func TestGenerateClientQualifiedServiceName(t *testing.T) { + requireQualify(t) + contractsDir := writeTwoServicesContractsFixture(t) + qualifyWorkspace(t) + resetClientFlags(t) + clientFrom = "contracts:" + contractsDir + clientLanguages = []string{"go"} + clientServices = []string{"acme.v1.WantedService"} + clientName = "wanted-client" + clientGoModule = "github.com/codefly-dev/wanted-client-go" + clientOutput = filepath.Join(t.TempDir(), "lib") + if err := ClientCmd.RunE(ClientCmd, nil); err != nil { + t.Fatal(err) + } + goDir := filepath.Join(clientOutput, "go") + facade := findFacadeFile(t, goDir, "_facade.pb.go") + assertFileContains(t, facade, "WantedServiceClient") + assertFileLacks(t, facade, "OtherService") + goVetLibrary(t, goDir) +} diff --git a/docs/cli-reference.md b/docs/cli-reference.md index cc0fe0cd..368ce663 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -2762,7 +2762,7 @@ Flags: --no-facade generate bindings only, no facade --npm-scope string npm package name for the generated typescript/ library (default: @codefly-dev/) --output string output directory (default: /libraries/) - --services strings restrict the library to these protobuf services and the declarations they need: contract, catalog, bindings and facade (protobuf contracts only) + --services strings restrict the library to these protobuf services (short or fully qualified names; ambiguous short names are refused) and the declarations they need: contract, catalog, bindings and facade ``` ## `codefly generate contracts` diff --git a/pkg/generators/narrow.go b/pkg/generators/narrow.go index a1cb88a4..95946c3d 100644 --- a/pkg/generators/narrow.go +++ b/pkg/generators/narrow.go @@ -105,6 +105,9 @@ func narrowEntries(entries []ContractEntry) ([]ContractEntry, error) { entry.ContractBytes = contract entry.generateFrom = generated entry.Endpoint.Services = selections[i] + // The companion filters by short name. The descriptor has already + // selected exact service identities, so this cannot widen the output. + entry.Services = protobufServiceNames(selections[i]) // The proto sources declare the whole contract; the narrowed // descriptor set is what is generated from. entry.ProtoSources = nil @@ -123,37 +126,46 @@ func entryLabel(entry *ContractEntry) string { // selectedServices is the catalog entries of the services entry names, in // catalog order. A name the contract does not declare is refused by name. func selectedServices(entry *ContractEntry) ([]composition.APIContractService, error) { - declared := map[string]bool{} - for _, service := range entry.Endpoint.Services { - declared[service.Name] = true + declared := map[string][]string{} + services := slices.Clone(entry.Endpoint.Services) + for i := range services { + service := &services[i] + if service.FullName == "" { + service.FullName = qualify(entry.Endpoint.Package, service.Name) + } + declared[service.FullName] = []string{service.FullName} + if service.Name != service.FullName { + declared[service.Name] = append(declared[service.Name], service.FullName) + } } wanted := map[string]bool{} var unknown []string for _, name := range entry.Services { name = strings.TrimSpace(name) - if !declared[name] { + matches := declared[name] + if len(matches) == 0 { unknown = append(unknown, fmt.Sprintf("%q", name)) + continue + } + if len(matches) > 1 { + sort.Strings(matches) + return nil, fmt.Errorf("entry %s: service %q is ambiguous; select a fully qualified name: %s", + entryLabel(entry), name, strings.Join(matches, ", ")) } - wanted[name] = true + wanted[matches[0]] = true } if len(unknown) > 0 { - names := make([]string, 0, len(declared)) - for name := range declared { - names = append(names, name) - } + names := protobufServiceNames(services) sort.Strings(names) sort.Strings(unknown) return nil, fmt.Errorf("entry %s: services %s are not declared by contract %s, which declares %s", entryLabel(entry), strings.Join(unknown, ", "), entry.Endpoint.Package, strings.Join(names, ", ")) } var selected []composition.APIContractService - for _, service := range entry.Endpoint.Services { - if !wanted[service.Name] { + for _, service := range services { + if !wanted[service.FullName] { continue } - if service.FullName == "" { - service.FullName = qualify(entry.Endpoint.Package, service.Name) - } selected = append(selected, service) } return selected, nil diff --git a/pkg/generators/narrow_test.go b/pkg/generators/narrow_test.go index ec83cd49..1b7713f0 100644 --- a/pkg/generators/narrow_test.go +++ b/pkg/generators/narrow_test.go @@ -478,3 +478,40 @@ func TestNarrowEntriesFollowsExtensionsAndNestedOptions(t *testing.T) { t.Errorf("ext.proto keeps %v and %d extensions, want Extra and the extension of P2", messages, len(ext.GetExtension())) } } + +// A multi-package contract can declare the same short service name twice. +// Selecting one must never include the other package's service or messages. +func TestNarrowEntriesSelectsQualifiedServiceIdentity(t *testing.T) { + var files []*descriptorpb.FileDescriptorProto + for _, pkg := range []string{"acme.v1", "other.v1"} { + file := protoFile(pkg+"/api.proto", nil, + []*descriptorpb.DescriptorProto{message("Request")}, nil, + service("DirectoryService", method("List", "."+pkg+".Request", "."+pkg+".Request"))) + file.Package = googleproto.String(pkg) + files = append(files, file) + } + set := &descriptorpb.FileDescriptorSet{File: files} + data, err := googleproto.Marshal(set) + if err != nil { + t.Fatal(err) + } + entry := narrowEntry(data, "acme.v1.DirectoryService") + entry.Endpoint.Package = "" + entry.Endpoint.Services = composition.ProtobufServices(set, "") + narrowed, err := narrowEntries([]ContractEntry{entry}) + if err != nil { + t.Fatal(err) + } + got := parseNarrowed(t, narrowed[0].ContractBytes) + if len(got) != 1 || got["acme.v1/api.proto"] == nil { + t.Fatalf("qualified selection includes wrong files: %v", got) + } + if !slices.Equal(narrowed[0].Services, []string{"DirectoryService"}) { + t.Fatalf("companion selection = %v", narrowed[0].Services) + } + entry.Services = []string{"DirectoryService"} + _, err = narrowEntries([]ContractEntry{entry}) + if err == nil || !strings.Contains(err.Error(), "ambiguous") || !strings.Contains(err.Error(), "acme.v1.DirectoryService") || !strings.Contains(err.Error(), "other.v1.DirectoryService") { + t.Fatalf("ambiguous selection error = %v", err) + } +}