From d127a1449f4c5c0b8ec92f429d0c616000fc9ea9 Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Sun, 25 Oct 2026 23:05:46 -0400 Subject: [PATCH 1/2] fix(generate): --services narrows the whole library, not just the facade `generate client --services` restricted only the facade: the persisted descriptor set, the catalog, the manifest's sources[].services and every language's bindings still carried every service of the contract (#1173). The library is now narrowed before anything is generated. The contract keeps the named services and the declarations they reach (their methods' messages and, transitively, every message and enum those name); a file defining a custom option they set is kept whole, as is a shared upstream file; no other service is declared anywhere. Catalog, manifest, bindings and facade all come from the narrowed contract, and its digest is the one recorded. A name the contract does not declare is refused by name. An aggregated library (sync solution-sdk) is narrowed as one, so a file two entries carry comes out the same for both. The docs' "--services does not work over a descriptor set" limitation is gone: the facade plugins run with strategy: all, and the qualified tests generate a narrowed descriptor set in Go, TypeScript and Python. Refs #1173 (the narrowing; the issue's --check mode is not part of this change) Co-Authored-By: Claude Opus 5.5 --- cmd/generate/client.go | 16 +- cmd/generate/client_services_test.go | 278 +++++++++++++ cmd/generate/client_test.go | 8 +- docs/cli-reference.md | 2 +- docs/commands.md | 27 +- pkg/generators/client.go | 55 ++- pkg/generators/narrow.go | 560 +++++++++++++++++++++++++++ pkg/generators/narrow_test.go | 340 ++++++++++++++++ 8 files changed, 1243 insertions(+), 43 deletions(-) create mode 100644 cmd/generate/client_services_test.go create mode 100644 pkg/generators/narrow.go create mode 100644 pkg/generators/narrow_test.go diff --git a/cmd/generate/client.go b/cmd/generate/client.go index a4fa1fb7..be8168d3 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 facade to these protobuf services (protobuf contracts only)") + 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().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") @@ -243,16 +243,10 @@ type resolvedContractSource struct { facadeModuleDefault string // protoSources, when set (the local module/service source only), carries - // the service's own .proto files. proto.GenerateClient's DescriptorSet - // path mishandles a --services subset once more than one file is present - // (core bug: a facade plugin invoked over a multi-file descriptor set — - // unavoidable once the module's proto imports anything, e.g. a REST - // gateway's google/api/annotations.proto — reports the target service as - // undeclared, even though the same content generates fine as Sources). - // The local source has the original files on hand, so it is generated - // from Sources instead, sidestepping the bug; contracts:/package: - // sources only ever have the persisted descriptor bytes and remain - // exposed to it (see docs/commands.md). + // the service's own .proto files, which a library binding the whole + // contract is generated from. A --services library is generated from its + // narrowed descriptor set instead, since the sources declare every + // service (see generators.ContractEntry.ProtoSources). protoSources []coreproto.Source } diff --git a/cmd/generate/client_services_test.go b/cmd/generate/client_services_test.go new file mode 100644 index 00000000..5d2f24d7 --- /dev/null +++ b/cmd/generate/client_services_test.go @@ -0,0 +1,278 @@ +package generate + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "slices" + "sort" + "strings" + "testing" + + "github.com/codefly-dev/core/composition" + "github.com/codefly-dev/core/resources" + googleproto "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/types/descriptorpb" + "gopkg.in/yaml.v3" +) + +// writeTwoServicesContractsFixture compiles a contract of two services over a +// shared file into a module's committed contracts/api directory: WantedService +// (one file, setting a google.api.http option) and OtherService (another file, +// the only one importing empty.proto), both naming declarations of +// acme/v1/common.proto, where OtherOnly and Spare serve neither. +func writeTwoServicesContractsFixture(t *testing.T) string { + t.Helper() + moduleDir := t.TempDir() + protoDir := filepath.Join(moduleDir, "proto") + writeTestFile(t, filepath.Join(protoDir, "buf.yaml"), "version: v1\ndeps:\n - buf.build/googleapis/googleapis\n") + writeTestFile(t, filepath.Join(protoDir, "acme", "v1", "common.proto"), `syntax = "proto3"; + +package acme.v1; + +import "google/protobuf/timestamp.proto"; + +// Shared is what both services name. +message Shared { + string id = 1; + google.protobuf.Timestamp at = 2; +} + +message OtherOnly { + string note = 1; +} + +message Spare { + string unused = 1; +} + +enum Color { + COLOR_UNSPECIFIED = 0; + COLOR_RED = 1; +} +`) + writeTestFile(t, filepath.Join(protoDir, "acme", "v1", "wanted.proto"), `syntax = "proto3"; + +package acme.v1; + +import "acme/v1/common.proto"; +import "google/api/annotations.proto"; + +message GetWantedRequest { + Shared shared = 1; + Color color = 2; +} + +message GetWantedResponse { + Shared shared = 1; +} + +// WantedService is the one service the library binds. +service WantedService { + rpc GetWanted(GetWantedRequest) returns (GetWantedResponse) { + option (google.api.http) = {get: "/v1/wanted"}; + } +} +`) + writeTestFile(t, filepath.Join(protoDir, "acme", "v1", "other.proto"), `syntax = "proto3"; + +package acme.v1; + +import "acme/v1/common.proto"; +import "google/protobuf/empty.proto"; + +message DoOtherRequest { + OtherOnly other = 1; + Shared shared = 2; +} + +service OtherService { + rpc DoOther(DoOtherRequest) returns (google.protobuf.Empty); +} +`) + descriptorSet, err := buildDescriptorSet(context.Background(), protoDir) + if err != nil { + t.Fatalf("buildDescriptorSet: %v", err) + } + var set descriptorpb.FileDescriptorSet + if err := googleproto.Unmarshal(descriptorSet, &set); err != nil { + t.Fatal(err) + } + return writeContractsCatalog(t, moduleDir, "codefly/acme", descriptorSet, composition.APIContractEndpoint{ + Service: "api", + Endpoint: "connect", + API: "connect", + Kind: composition.APIContractKindProtobuf, + Package: "acme.v1", + Services: composition.ProtobufServices(&set, "acme.v1"), + }) +} + +// generateWantedClient runs `generate client --services WantedService` over +// the two-service contract into output, in every client language. +func generateWantedClient(t *testing.T, contractsDir, output string) { + t.Helper() + resetClientFlags(t) + clientFrom = "contracts:" + contractsDir + clientLanguages = []string{"go", "typescript", "python"} + clientServices = []string{"WantedService"} + clientName = "wanted-client" + clientGoModule = "github.com/codefly-dev/wanted-client-go" + clientOutput = output + if err := ClientCmd.RunE(ClientCmd, nil); err != nil { + t.Fatalf("RunE: %v", err) + } +} + +func qualifyWorkspace(t *testing.T) { + t.Helper() + workspace := &resources.Workspace{Name: "qualify-ws", Layout: resources.LayoutKindModules} + wsDir := filepath.Join(t.TempDir(), "ws") + if err := workspace.SaveToDirUnsafe(context.Background(), wsDir); err != nil { + t.Fatal(err) + } + t.Chdir(wsDir) +} + +// TestGenerateClientServicesNarrowsTheWholeLibrary: a library generated for +// one service of a contract carries that service alone (#1173). Its persisted +// descriptor set declares that service and the declarations it reaches, its +// catalog and manifest list only it, and no language's bindings or facade +// mention the other service or what only the other service needs — and each +// still builds. +func TestGenerateClientServicesNarrowsTheWholeLibrary(t *testing.T) { + requireQualify(t) + contractsDir := writeTwoServicesContractsFixture(t) + qualifyWorkspace(t) + output := filepath.Join(t.TempDir(), "lib") + generateWantedClient(t, contractsDir, output) + + contract, err := os.ReadFile(filepath.Join(output, "contract", "contract.binpb")) + if err != nil { + t.Fatal(err) + } + var set descriptorpb.FileDescriptorSet + if err := googleproto.Unmarshal(contract, &set); err != nil { + t.Fatal(err) + } + var files, services, ownDeclarations []string + for _, file := range set.GetFile() { + files = append(files, file.GetName()) + for _, service := range file.GetService() { + services = append(services, file.GetPackage()+"."+service.GetName()) + } + if file.GetPackage() != "acme.v1" { + continue + } + for _, message := range file.GetMessageType() { + ownDeclarations = append(ownDeclarations, message.GetName()) + } + for _, enum := range file.GetEnumType() { + ownDeclarations = append(ownDeclarations, enum.GetName()) + } + } + if !slices.Equal(services, []string{"acme.v1.WantedService"}) { + t.Errorf("contract services = %v, want only acme.v1.WantedService", services) + } + sort.Strings(ownDeclarations) + if want := []string{"Color", "GetWantedRequest", "GetWantedResponse", "Shared"}; !slices.Equal(ownDeclarations, want) { + t.Errorf("contract declares %v, want exactly what WantedService reaches: %v", ownDeclarations, want) + } + for _, dropped := range []string{"acme/v1/other.proto", "google/protobuf/empty.proto"} { + if slices.Contains(files, dropped) { + t.Errorf("contract still carries %s: %v", dropped, files) + } + } + if !slices.Contains(files, "google/api/annotations.proto") { + t.Errorf("contract lost the file defining the option WantedService sets: %v", files) + } + + var catalog composition.APIContractCatalog + catalogData, err := os.ReadFile(filepath.Join(output, "contract", "catalog.codefly.json")) + if err != nil { + t.Fatal(err) + } + if err := json.Unmarshal(catalogData, &catalog); err != nil { + t.Fatal(err) + } + endpoint := catalog.Endpoints[0] + if len(endpoint.Services) != 1 || endpoint.Services[0].FullName != "acme.v1.WantedService" { + t.Errorf("catalog services = %+v, want only acme.v1.WantedService", endpoint.Services) + } + if endpoint.Digest != composition.APIContractDigest(contract) { + t.Errorf("catalog digest %s is not the persisted contract's %s", endpoint.Digest, composition.APIContractDigest(contract)) + } + + var manifest struct { + Sources []struct { + ContractDigest string `yaml:"contract-digest"` + Services []string `yaml:"services"` + } `yaml:"sources"` + } + manifestData, err := os.ReadFile(filepath.Join(output, resources.LibraryConfigurationName)) + if err != nil { + t.Fatal(err) + } + if err := yaml.Unmarshal(manifestData, &manifest); err != nil { + t.Fatal(err) + } + if source := manifest.Sources[0]; !slices.Equal(source.Services, []string{"WantedService"}) || source.ContractDigest != endpoint.Digest { + t.Errorf("manifest source = %+v, want services [WantedService] at the persisted contract's digest %s", source, endpoint.Digest) + } + + for _, lang := range []string{"go", "typescript", "python"} { + assertTreeLacks(t, filepath.Join(output, lang), "OtherService", "DoOther", "OtherOnly", "Spare") + } + assertFileContains(t, findFacadeFile(t, filepath.Join(output, "go"), "_facade.pb.go"), "WantedService") + assertFileContains(t, findFacadeFile(t, filepath.Join(output, "typescript"), "_facade.ts"), "WantedService") + assertFileContains(t, findFacadeFile(t, filepath.Join(output, "python", "wanted_client"), "acme.py"), "class WantedServiceClient") + goVetLibrary(t, filepath.Join(output, "go")) + assertPythonImports(t, filepath.Join(output, "python"), "wanted_client.acme") +} + +// TestGenerateClientServicesIsStable is the drift check a committed library is +// gated with: regenerating in place, and generating afresh into another tree, +// both reproduce the library byte for byte. +func TestGenerateClientServicesIsStable(t *testing.T) { + requireQualify(t) + contractsDir := writeTwoServicesContractsFixture(t) + qualifyWorkspace(t) + output := filepath.Join(t.TempDir(), "lib") + generateWantedClient(t, contractsDir, output) + first := hashTree(t, output) + + generateWantedClient(t, contractsDir, output) + if again := hashTree(t, output); again != first { + t.Fatalf("regenerating in place drifted: %s != %s", again, first) + } + + fresh := filepath.Join(t.TempDir(), "lib") + generateWantedClient(t, contractsDir, fresh) + if afresh := hashTree(t, fresh); afresh != first { + t.Fatalf("generating afresh drifted from the library: %s != %s", afresh, first) + } +} + +// assertTreeLacks fails for every file under root mentioning any of words. +func assertTreeLacks(t *testing.T, root string, words ...string) { + t.Helper() + err := filepath.WalkDir(root, func(path string, entry os.DirEntry, err error) error { + if err != nil || entry.IsDir() { + return err + } + data, err := os.ReadFile(path) + if err != nil { + return err + } + for _, word := range words { + if strings.Contains(string(data), word) { + t.Errorf("%s mentions %s", path, word) + } + } + return nil + }) + if err != nil { + t.Fatal(err) + } +} diff --git a/cmd/generate/client_test.go b/cmd/generate/client_test.go index 71eef605..0001d0dc 100644 --- a/cmd/generate/client_test.go +++ b/cmd/generate/client_test.go @@ -107,12 +107,8 @@ func TestGenerateClientFromModuleService(t *testing.T) { resetClientFlags(t) clientFrom = "billing/api/grpc" clientLanguages = []string{"go"} - // The local source generates from the service's own proto sources (see - // resolvedContractSource.protoSources), so --services works here; a - // contracts:/package: source only has the persisted descriptor bytes and - // hits a core bug filtering --services over a multi-file descriptor set - // (tracked, not something this repo can fix) — see - // TestGenerateClientFromContractsDir, which does not set --services. + // --services narrows the local contract too: the library is generated + // from the narrowed descriptor set rather than the service's sources. clientServices = []string{"ApiService"} clientOutput = filepath.Join(t.TempDir(), "lib") diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 78a2e5eb..8983dde8 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 facade to these protobuf services (protobuf contracts only) + --services strings restrict the library to these protobuf services and the declarations they need: contract, catalog, bindings and facade (protobuf contracts only) ``` ## `codefly generate contracts` diff --git a/docs/commands.md b/docs/commands.md index b1863972..2f88b5be 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -3407,7 +3407,7 @@ with no caller and no way to detect drift. |------|-------------| | `--from` | Required. Contract source: `module/service[/endpoint]` (a local workspace service), `package:@` (a composed module package), or `contracts:` (a `generate contracts` export) | | `--language` | Required, repeatable or comma-separated: `go`, `typescript`, `python` | -| `--services` | Restrict the facade to these protobuf services (protobuf contracts only; not supported when `--from` resolves through a descriptor set — see below) | +| `--services` | Restrict the library to these protobuf services and the declarations they need (protobuf contracts only; see below) | | `--endpoint` | Select an endpoint when `--from package:`/`contracts:` resolves to more than one: `service/endpoint` | | `--name` | Library name (default `--client`) | | `--module-name` | Facade entry-point name (default: derived from the contract's proto package) | @@ -3440,10 +3440,25 @@ that generates but does not build is left in place with the build error reported one-entry contract in memory (the same way `generate contracts` does) and generates from the service's own proto sources. `--from package:`/`contracts:` read an already-persisted contract (a `contract.binpb`/`openapi.json` plus -catalog entry) and generate from its descriptor bytes — a known upstream -limitation in the facade plugin currently means `--services` filtering does not -work over a multi-file descriptor set from these two sources; use the local -source, or omit `--services` to generate the full contract's facade. +catalog entry) and generate from its descriptor bytes. + +`--services` narrows the whole library, from any source, before anything is +generated. The contract it persists under `contract/` keeps the named services +and the declarations they reach: their methods' messages and, transitively, +every message and enum those name (a nested type keeps the top-level message +holding it). A file defining a custom option those declarations set is kept +whole, as is a shared upstream file (`google/protobuf`, `google/api`, +`google/rpc`, `google/type`, `buf/validate`). No other service is declared +anywhere, so the catalog, the manifest's `sources[].services`, the bindings and +the facade of every language carry the named services alone; the catalog and +`sources[].contract-digest` record the narrowed contract's digest. A name the +contract does not declare is refused, naming the services it does declare. +Narrowing is deterministic, so regenerating a library reproduces it byte for +byte. A library generated from a local source with `--services` is generated +from the narrowed descriptor set rather than the proto sources. When `sync +solution-sdk` aggregates several contracts into one library, the declarations +kept are computed across all of them, so a file two entries carry comes out the +same for both; an entry naming no services keeps its whole contract. TypeScript's generated entry point (`src/index.ts`) has three parts. Each facade is re-exported flat under the name its plugin gives it (`accounts`, @@ -3469,7 +3484,7 @@ are reachable only from inside the library. libraries/saas-starter-accounts-client/ library.codefly.yaml contract/ - catalog.codefly.json # the endpoint's catalog entry (subset if --services) + catalog.codefly.json # the endpoint's catalog entry (narrowed with --services) contract.binpb | openapi.json go/ go.mod, gen/…, accounts_facade.pb.go typescript/ package.json, tsconfig.json, src/index.ts, src/packages/…, src/gen/…, src/accounts_facade.ts diff --git a/pkg/generators/client.go b/pkg/generators/client.go index 840f3f44..deb808d8 100644 --- a/pkg/generators/client.go +++ b/pkg/generators/client.go @@ -49,15 +49,15 @@ type ContractEntry struct { // for this contract (the last non-version segment of its proto package). FacadeModuleDefault string - // ProtoSources, when set, carries the endpoint's original .proto files - // instead of relying on the persisted descriptor set: proto.GenerateClient's - // DescriptorSet path mishandles a Services subset once more than one file is - // present (core bug: a facade plugin invoked over a multi-file descriptor - // set reports the target service as undeclared, even though the same - // content generates fine as Sources). + // ProtoSources, when set, carries the endpoint's original .proto files, + // which an entry binding its whole contract is generated from instead of + // the persisted descriptor set. An entry naming Services is generated from + // its narrowed descriptor set: the sources declare every service. ProtoSources []coreproto.Source - // Services restricts this entry's facade to these protobuf service names. + // Services restricts this entry to these protobuf service names: the + // library's contract, catalog, manifest, bindings and facade carry them + // and the declarations they need, nothing else (see narrowEntries). Services []string // As overrides this entry's facade entry-point name (default: // FacadeModuleDefault, else defaultFacadeModule). @@ -113,13 +113,20 @@ func GenerateClientLibrary(ctx context.Context, req *ClientLibraryRequest) error if err := checkContractDiamond(req.Entries); err != nil { return err } + // From here on the library is what its entries select: the contract it + // persists, its catalog, its manifest and its bindings all come from the + // narrowed entries. + entries, err := narrowEntries(req.Entries) + if err != nil { + return err + } openAPICount := 0 - for i := range req.Entries { - if req.Entries[i].Endpoint.Kind == composition.APIContractKindOpenAPI { + for i := range entries { + if entries[i].Endpoint.Kind == composition.APIContractKindOpenAPI { openAPICount++ } } - if openAPICount > 0 && len(req.Entries) > 1 { + if openAPICount > 0 && len(entries) > 1 { return fmt.Errorf("cannot aggregate an OpenAPI contract with other entries in one library") } @@ -133,8 +140,8 @@ func GenerateClientLibrary(ctx context.Context, req *ClientLibraryRequest) error existingByKey[[3]string{s.Module, s.Service, s.Endpoint}] = *s } anyDigestChanged := false - for i := range req.Entries { - entry := &req.Entries[i] + for i := range entries { + entry := &entries[i] key := [3]string{entry.ModuleName, entry.Endpoint.Service, entry.Endpoint.Endpoint} prior, ok := existingByKey[key] if !ok || prior.ContractDigest == entry.Endpoint.Digest { @@ -161,7 +168,7 @@ func GenerateClientLibrary(ctx context.Context, req *ClientLibraryRequest) error } pyPackage := strings.ReplaceAll(req.Name, "-", "_") - version := libraryVersion(req.Output, req.Entries) + version := libraryVersion(req.Output, entries) if strings.TrimSpace(version) == "" { // Refused before anything is generated, so the library is untouched. return fmt.Errorf("library %s declares no version: its producer's version is empty", req.Name) @@ -190,14 +197,14 @@ func GenerateClientLibrary(ctx context.Context, req *ClientLibraryRequest) error if err := ctx.Err(); err != nil { return err } - s, genErr := stageLanguage(ctx, lang, filepath.Join(req.Output, string(lang)), req.Entries, !req.NoFacade, req.GoModule) + s, genErr := stageLanguage(ctx, lang, filepath.Join(req.Output, string(lang)), entries, !req.NoFacade, req.GoModule) if genErr != nil { return fmt.Errorf("cannot generate %s client: %w", lang, genErr) } staged = append(staged, s) } - if err := writeContractFiles(ctx, req.Output, req.Entries); err != nil { + if err := writeContractFiles(ctx, req.Output, entries); err != nil { return fmt.Errorf("cannot write contract: %w", err) } @@ -216,7 +223,7 @@ func GenerateClientLibrary(ctx context.Context, req *ClientLibraryRequest) error langExports = mergeLanguageExports(existingLanguages, langExports, req.Languages) - if err := writeLibraryManifest(ctx, req.Output, req.Name, version, req.Entries, langExports, !req.NoFacade); err != nil { + if err := writeLibraryManifest(ctx, req.Output, req.Name, version, entries, langExports, !req.NoFacade); err != nil { return fmt.Errorf("cannot write %s: %w", resources.LibraryConfigurationName, err) } @@ -629,11 +636,21 @@ func upstreamModulesIn(set *descriptorpb.FileDescriptorSet, lang languages.Langu return modules } +// upstreamModuleFor is the upstream module file belongs to when lang resolves +// that module from its distribution (TypeScript keeps local copies of every +// module but the well-known types). func upstreamModuleFor(file *descriptorpb.FileDescriptorProto, lang languages.Language) (upstreamProtoModule, bool) { + module, found := matchUpstreamModule(file) + if !found || (!module.bufModule.isZero() && lang == languages.TYPESCRIPT) { + return upstreamProtoModule{}, false + } + return module, true +} + +// matchUpstreamModule is the upstream module file belongs to, by its path and +// its package, whatever the language. +func matchUpstreamModule(file *descriptorpb.FileDescriptorProto) (upstreamProtoModule, bool) { for _, module := range upstreamProtoModules { - if !module.bufModule.isZero() && lang == languages.TYPESCRIPT { - continue - } if !strings.HasPrefix(file.GetName(), module.pathPrefix) { continue } diff --git a/pkg/generators/narrow.go b/pkg/generators/narrow.go new file mode 100644 index 00000000..19a148f8 --- /dev/null +++ b/pkg/generators/narrow.go @@ -0,0 +1,560 @@ +package generators + +import ( + "fmt" + "slices" + "sort" + "strings" + + "github.com/codefly-dev/core/composition" + "google.golang.org/protobuf/encoding/protowire" + googleproto "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/reflect/protodesc" + "google.golang.org/protobuf/reflect/protoreflect" + "google.golang.org/protobuf/reflect/protoregistry" + "google.golang.org/protobuf/types/descriptorpb" +) + +// narrowEntries narrows a library to the services its entries select. +// +// A protobuf entry naming Services binds those services and nothing else, so +// its contract is rewritten to declare only them before anything is generated: +// the persisted descriptor set, the catalog and the manifest describe the +// narrowed contract, and the bindings and facade are generated from it. The +// narrowed set keeps, from the files declaring the selected services and the +// files those reach, the top-level declarations the services need: their +// methods' messages and, transitively, every message and enum a kept message +// names (a nested type keeps the top-level message holding it). A file +// defining a custom option a kept declaration sets is kept whole, with its +// imports, and so is a file of a shared upstream module: neither is the +// library's own, and the upstream ones keep the shape their distributions +// publish. No file declares a service that is not selected. +// +// The narrowing is the library's, not one entry's: an aggregated library +// generates every entry into one tree, so a file two entries carry must come +// out the same for both. The kept declarations are computed across every +// entry at once, and an entry naming no Services keeps its whole contract +// (and is returned untouched). A library in which no entry names Services is +// returned as is. +func narrowEntries(entries []ContractEntry) ([]ContractEntry, error) { + if !slices.ContainsFunc(entries, func(entry ContractEntry) bool { return selectsServices(&entry) }) { + return entries, nil + } + n := &narrowing{ + files: map[string]*descriptorpb.FileDescriptorProto{}, + upstream: map[string]bool{}, + types: map[string]declaration{}, + services: map[string]string{}, + extensions: map[extensionKey]string{}, + selected: map[string]bool{}, + kept: map[string]*keptFile{}, + } + sets := make([]*descriptorpb.FileDescriptorSet, len(entries)) + for i := range entries { + if entries[i].Endpoint.Kind != composition.APIContractKindProtobuf { + continue + } + set := &descriptorpb.FileDescriptorSet{} + // No resolver: custom options stay unknown fields, so the narrowed + // bytes depend on the contract alone, never on what this binary links. + if err := (googleproto.UnmarshalOptions{Resolver: new(protoregistry.Types)}).Unmarshal(entries[i].ContractBytes, set); err != nil { + return nil, fmt.Errorf("entry %s: cannot parse contract descriptor set: %w", entryLabel(&entries[i]), err) + } + sets[i] = set + n.index(set) + } + + selections := make([][]composition.APIContractService, len(entries)) + for i := range entries { + if sets[i] == nil { + continue + } + if !selectsServices(&entries[i]) { + for _, file := range sets[i].GetFile() { + n.keepWhole(file.GetName()) + for _, service := range file.GetService() { + n.selected[qualify(file.GetPackage(), service.GetName())] = true + } + } + continue + } + selected, err := selectedServices(&entries[i]) + if err != nil { + return nil, err + } + selections[i] = selected + for _, service := range selected { + n.selected[service.FullName] = true + } + } + roots := make([]string, 0, len(n.selected)) + for fullName := range n.selected { + roots = append(roots, fullName) + } + sort.Strings(roots) + for _, fullName := range roots { + n.keepService(fullName) + } + if n.err != nil { + return nil, fmt.Errorf("cannot narrow the contract to its selected services: %w", n.err) + } + + narrowed := slices.Clone(entries) + for i := range narrowed { + if selections[i] == nil { + continue + } + entry := &narrowed[i] + data, err := n.emit(sets[i]) + if err != nil { + return nil, fmt.Errorf("entry %s: %w", entryLabel(entry), err) + } + entry.ContractBytes = data + entry.Endpoint.Services = selections[i] + entry.Endpoint.Digest = composition.APIContractDigest(data) + // The proto sources declare the whole contract; the narrowed + // descriptor set is what is generated from. + entry.ProtoSources = nil + } + return narrowed, nil +} + +func selectsServices(entry *ContractEntry) bool { + return entry.Endpoint.Kind == composition.APIContractKindProtobuf && len(entry.Services) > 0 +} + +func entryLabel(entry *ContractEntry) string { + return entry.ModuleName + "/" + entry.Endpoint.Service + "/" + entry.Endpoint.Endpoint +} + +// 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 + } + wanted := map[string]bool{} + var unknown []string + for _, name := range entry.Services { + name = strings.TrimSpace(name) + if !declared[name] { + unknown = append(unknown, fmt.Sprintf("%q", name)) + } + wanted[name] = true + } + if len(unknown) > 0 { + names := make([]string, 0, len(declared)) + for name := range declared { + names = append(names, name) + } + 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] { + continue + } + if service.FullName == "" { + service.FullName = qualify(entry.Endpoint.Package, service.Name) + } + selected = append(selected, service) + } + return selected, nil +} + +// narrowing is the closure of what a library's selected services need, over +// every file its entries' contracts carry (by name, first seen). +type narrowing struct { + files map[string]*descriptorpb.FileDescriptorProto + upstream map[string]bool + types map[string]declaration // message or enum full name -> its top-level declaration + services map[string]string // service full name -> file + extensions map[extensionKey]string + selected map[string]bool // service full names + kept map[string]*keptFile + err error +} + +// declaration is a top-level message or enum of a file. +type declaration struct { + file string + message *descriptorpb.DescriptorProto + enum *descriptorpb.EnumDescriptorProto +} + +type extensionKey struct { + extendee string + number int32 +} + +// keptFile is what a file keeps: everything when whole, otherwise the named +// top-level declarations, and the imports they need. +type keptFile struct { + whole bool + messages map[string]bool + enums map[string]bool + imports map[string]bool +} + +func qualify(pkg, name string) string { + if pkg == "" { + return name + } + return pkg + "." + name +} + +func (n *narrowing) fail(format string, args ...any) { + if n.err == nil { + n.err = fmt.Errorf(format, args...) + } +} + +func (n *narrowing) index(set *descriptorpb.FileDescriptorSet) { + for _, file := range set.GetFile() { + name := file.GetName() + if _, seen := n.files[name]; seen { + continue + } + n.files[name] = file + if _, upstream := matchUpstreamModule(file); upstream { + n.upstream[name] = true + } + pkg := file.GetPackage() + for _, message := range file.GetMessageType() { + n.indexMessage(qualify(pkg, message.GetName()), declaration{file: name, message: message}, message) + } + for _, enum := range file.GetEnumType() { + n.types[qualify(pkg, enum.GetName())] = declaration{file: name, enum: enum} + } + for _, service := range file.GetService() { + n.services[qualify(pkg, service.GetName())] = name + } + n.indexExtensions(name, file.GetExtension()) + } +} + +func (n *narrowing) indexMessage(fullName string, top declaration, message *descriptorpb.DescriptorProto) { + n.types[fullName] = top + for _, nested := range message.GetNestedType() { + n.indexMessage(fullName+"."+nested.GetName(), top, nested) + } + for _, enum := range message.GetEnumType() { + n.types[fullName+"."+enum.GetName()] = top + } + n.indexExtensions(top.file, message.GetExtension()) +} + +func (n *narrowing) indexExtensions(file string, extensions []*descriptorpb.FieldDescriptorProto) { + for _, extension := range extensions { + n.extensions[extensionKey{strings.TrimPrefix(extension.GetExtendee(), "."), extension.GetNumber()}] = file + } +} + +// file is name's kept state, created (and its file options visited) the first +// time anything of it is kept. +func (n *narrowing) file(name string) *keptFile { + if kept, ok := n.kept[name]; ok { + return kept + } + kept := &keptFile{messages: map[string]bool{}, enums: map[string]bool{}, imports: map[string]bool{}} + n.kept[name] = kept + if file := n.files[name]; file != nil { + n.visitOptions(name, file.GetOptions()) + } else { + n.fail("the contract imports %s but does not carry it", name) + } + return kept +} + +func (n *narrowing) keepWhole(name string) { + kept := n.file(name) + if kept.whole { + return + } + kept.whole = true + for _, dependency := range n.files[name].GetDependency() { + n.keepWhole(dependency) + } +} + +func (n *narrowing) keepService(fullName string) { + name, ok := n.services[fullName] + if !ok { + n.fail("the contract declares no service %s", fullName) + return + } + n.file(name) + file := n.files[name] + for _, service := range file.GetService() { + if qualify(file.GetPackage(), service.GetName()) != fullName { + continue + } + n.visitOptions(name, service.GetOptions()) + for _, method := range service.GetMethod() { + n.visitOptions(name, method.GetOptions()) + n.useType(name, method.GetInputType()) + n.useType(name, method.GetOutputType()) + } + } +} + +// useType keeps the top-level declaration typeName resolves to, and the +// import from reaches it through. +func (n *narrowing) useType(from, typeName string) { + fullName := strings.TrimPrefix(typeName, ".") + decl, ok := n.types[fullName] + if !ok { + n.fail("%s names %s, which no file of the contract declares", from, fullName) + return + } + n.require(from, decl.file) + kept := n.file(decl.file) + switch { + case kept.whole: + case n.upstream[decl.file]: + n.keepWhole(decl.file) + case decl.message != nil && !kept.messages[decl.message.GetName()]: + kept.messages[decl.message.GetName()] = true + n.visitMessage(decl.file, decl.message) + case decl.enum != nil && !kept.enums[decl.enum.GetName()]: + kept.enums[decl.enum.GetName()] = true + n.visitEnum(decl.file, decl.enum) + } +} + +func (n *narrowing) visitMessage(file string, message *descriptorpb.DescriptorProto) { + n.visitOptions(file, message.GetOptions()) + for _, field := range message.GetField() { + n.visitField(file, field) + } + for _, extension := range message.GetExtension() { + n.visitField(file, extension) + n.useType(file, extension.GetExtendee()) + } + for _, oneof := range message.GetOneofDecl() { + n.visitOptions(file, oneof.GetOptions()) + } + for _, nested := range message.GetNestedType() { + n.visitMessage(file, nested) + } + for _, enum := range message.GetEnumType() { + n.visitEnum(file, enum) + } +} + +func (n *narrowing) visitField(file string, field *descriptorpb.FieldDescriptorProto) { + n.visitOptions(file, field.GetOptions()) + if field.GetTypeName() != "" { + n.useType(file, field.GetTypeName()) + } +} + +func (n *narrowing) visitEnum(file string, enum *descriptorpb.EnumDescriptorProto) { + n.visitOptions(file, enum.GetOptions()) + for _, value := range enum.GetValue() { + n.visitOptions(file, value.GetOptions()) + } +} + +// visitOptions keeps, whole, every file defining a custom option set in +// options: a field the options message does not declare is an extension. +func (n *narrowing) visitOptions(from string, options googleproto.Message) { + if options == nil || !options.ProtoReflect().IsValid() { + return + } + descriptor := options.ProtoReflect().Descriptor() + data, err := googleproto.MarshalOptions{Deterministic: true}.Marshal(options) + if err != nil { + n.fail("%s: cannot read %s: %w", from, descriptor.FullName(), err) + return + } + for len(data) > 0 { + number, kind, length := protowire.ConsumeTag(data) + if length < 0 { + n.fail("%s: malformed %s", from, descriptor.FullName()) + return + } + data = data[length:] + if length = protowire.ConsumeFieldValue(number, kind, data); length < 0 { + n.fail("%s: malformed %s", from, descriptor.FullName()) + return + } + data = data[length:] + if descriptor.Fields().ByNumber(number) != nil { + continue + } + defining, ok := n.extensions[extensionKey{string(descriptor.FullName()), int32(number)}] + if !ok { + n.fail("%s sets option %d of %s, which no file of the contract defines", from, number, descriptor.FullName()) + continue + } + n.require(from, defining) + n.keepWhole(defining) + } +} + +// require records that from needs target: the import of from that reaches it, +// directly or through a chain of public imports, each of which is kept. +func (n *narrowing) require(from, target string) { + if from == target { + return + } + chain := n.importChain(from, target) + if chain == nil { + n.fail("%s needs %s but does not import it", from, target) + return + } + for _, hop := range chain { + n.file(from).imports[hop] = true + from = hop + } +} + +func (n *narrowing) importChain(from, target string) []string { + dependencies := n.files[from].GetDependency() + if slices.Contains(dependencies, target) { + return []string{target} + } + for _, dependency := range dependencies { + if chain := n.publicChain(dependency, target, map[string]bool{}); chain != nil { + return chain + } + } + return nil +} + +func (n *narrowing) publicChain(file, target string, seen map[string]bool) []string { + if file == target { + return []string{file} + } + if seen[file] || n.files[file] == nil { + return nil + } + seen[file] = true + descriptor := n.files[file] + for _, index := range descriptor.GetPublicDependency() { + if chain := n.publicChain(descriptor.GetDependency()[index], target, seen); chain != nil { + return append([]string{file}, chain...) + } + } + return nil +} + +// emit is set narrowed to what was kept, in set's file order, refused unless +// it resolves on its own. +func (n *narrowing) emit(set *descriptorpb.FileDescriptorSet) ([]byte, error) { + narrowed := &descriptorpb.FileDescriptorSet{} + for _, file := range set.GetFile() { + if kept, ok := n.kept[file.GetName()]; ok { + narrowed.File = append(narrowed.File, n.narrowFile(file, kept)) + } + } + if _, err := protodesc.NewFiles(narrowed); err != nil { + return nil, fmt.Errorf("the narrowed contract does not resolve: %w", err) + } + return googleproto.MarshalOptions{Deterministic: true}.Marshal(narrowed) +} + +func (n *narrowing) narrowFile(file *descriptorpb.FileDescriptorProto, kept *keptFile) *descriptorpb.FileDescriptorProto { + out, _ := googleproto.Clone(file).(*descriptorpb.FileDescriptorProto) + messages := keptIndexes(len(file.GetMessageType()), func(i int) bool { + return kept.whole || kept.messages[file.GetMessageType()[i].GetName()] + }) + enums := keptIndexes(len(file.GetEnumType()), func(i int) bool { + return kept.whole || kept.enums[file.GetEnumType()[i].GetName()] + }) + services := keptIndexes(len(file.GetService()), func(i int) bool { + return n.selected[qualify(file.GetPackage(), file.GetService()[i].GetName())] + }) + extensions := keptIndexes(len(file.GetExtension()), func(int) bool { return kept.whole }) + dependencies := keptIndexes(len(file.GetDependency()), func(i int) bool { + return kept.whole || kept.imports[file.GetDependency()[i]] + }) + public := keptIndexes(len(file.GetPublicDependency()), func(i int) bool { return dependencies[file.GetPublicDependency()[i]] >= 0 }) + weak := keptIndexes(len(file.GetWeakDependency()), func(i int) bool { return dependencies[file.GetWeakDependency()[i]] >= 0 }) + + out.MessageType = pick(out.GetMessageType(), messages) + out.EnumType = pick(out.GetEnumType(), enums) + out.Service = pick(out.GetService(), services) + out.Extension = pick(out.GetExtension(), extensions) + out.Dependency = pick(out.GetDependency(), dependencies) + out.PublicDependency = reindex(pick(out.GetPublicDependency(), public), dependencies) + out.WeakDependency = reindex(pick(out.GetWeakDependency(), weak), dependencies) + + // Source info locates a comment by index into these lists, so a comment + // follows its declaration and a dropped declaration's comments go with it. + if info := out.GetSourceCodeInfo(); info != nil { + moved := map[int32][]int32{ + fileField("message_type"): messages, + fileField("enum_type"): enums, + fileField("service"): services, + fileField("extension"): extensions, + fileField("dependency"): dependencies, + fileField("public_dependency"): public, + fileField("weak_dependency"): weak, + } + locations := info.GetLocation()[:0] + for _, location := range info.GetLocation() { + path := location.GetPath() + if mapping, ok := moved[firstOrNegative(path)]; ok && len(path) > 1 { + if int(path[1]) >= len(mapping) || mapping[path[1]] < 0 { + continue + } + path[1] = mapping[path[1]] + } + locations = append(locations, location) + } + info.Location = locations + } + return out +} + +func firstOrNegative(path []int32) int32 { + if len(path) == 0 { + return -1 + } + return path[0] +} + +// fileField is FileDescriptorProto's field number for name, read from its +// descriptor rather than retyped. +func fileField(name protoreflect.Name) int32 { + return int32((&descriptorpb.FileDescriptorProto{}).ProtoReflect().Descriptor().Fields().ByName(name).Number()) +} + +// keptIndexes maps each of count indexes to its index once the ones keep +// refuses are dropped, or -1 for a dropped one. +func keptIndexes(count int, keep func(int) bool) []int32 { + mapping := make([]int32, count) + var next int32 + for i := range mapping { + if keep(i) { + mapping[i] = next + next++ + } else { + mapping[i] = -1 + } + } + return mapping +} + +func pick[T any](items []T, mapping []int32) []T { + var kept []T + for i, item := range items { + if mapping[i] >= 0 { + kept = append(kept, item) + } + } + return kept +} + +func reindex(indexes []int32, mapping []int32) []int32 { + for i, index := range indexes { + indexes[i] = mapping[index] + } + return indexes +} diff --git a/pkg/generators/narrow_test.go b/pkg/generators/narrow_test.go new file mode 100644 index 00000000..9e569efe --- /dev/null +++ b/pkg/generators/narrow_test.go @@ -0,0 +1,340 @@ +package generators + +import ( + "bytes" + "slices" + "sort" + "strings" + "testing" + + coreproto "github.com/codefly-dev/core/companions/proto" + "github.com/codefly-dev/core/composition" + "google.golang.org/protobuf/encoding/protowire" + googleproto "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/reflect/protodesc" + "google.golang.org/protobuf/types/descriptorpb" + "google.golang.org/protobuf/types/known/emptypb" + "google.golang.org/protobuf/types/known/timestamppb" +) + +const scopeOption = 50001 + +func messageField(name string, number int32, typeName string) *descriptorpb.FieldDescriptorProto { + return &descriptorpb.FieldDescriptorProto{ + Name: googleproto.String(name), + Number: googleproto.Int32(number), + Label: descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(), + Type: descriptorpb.FieldDescriptorProto_TYPE_MESSAGE.Enum(), + TypeName: googleproto.String(typeName), + } +} + +func enumField(name string, number int32, typeName string) *descriptorpb.FieldDescriptorProto { + field := messageField(name, number, typeName) + field.Type = descriptorpb.FieldDescriptorProto_TYPE_ENUM.Enum() + return field +} + +func message(name string, fields ...*descriptorpb.FieldDescriptorProto) *descriptorpb.DescriptorProto { + return &descriptorpb.DescriptorProto{Name: googleproto.String(name), Field: fields} +} + +func enum(name string) *descriptorpb.EnumDescriptorProto { + return &descriptorpb.EnumDescriptorProto{ + Name: googleproto.String(name), + Value: []*descriptorpb.EnumValueDescriptorProto{{Name: googleproto.String(strings.ToUpper(name) + "_UNSPECIFIED"), Number: googleproto.Int32(0)}}, + } +} + +func service(name string, methods ...*descriptorpb.MethodDescriptorProto) *descriptorpb.ServiceDescriptorProto { + return &descriptorpb.ServiceDescriptorProto{Name: googleproto.String(name), Method: methods} +} + +func method(name, input, output string) *descriptorpb.MethodDescriptorProto { + return &descriptorpb.MethodDescriptorProto{Name: googleproto.String(name), InputType: googleproto.String(input), OutputType: googleproto.String(output)} +} + +func protoFile(name string, deps []string, messages []*descriptorpb.DescriptorProto, enums []*descriptorpb.EnumDescriptorProto, services ...*descriptorpb.ServiceDescriptorProto) *descriptorpb.FileDescriptorProto { + return &descriptorpb.FileDescriptorProto{ + Name: googleproto.String(name), + Package: googleproto.String("acme.v1"), + Syntax: googleproto.String("proto3"), + Dependency: deps, + MessageType: messages, + EnumType: enums, + Service: services, + } +} + +func leadingComment(path []int32, text string) *descriptorpb.SourceCodeInfo_Location { + return &descriptorpb.SourceCodeInfo_Location{Path: path, Span: []int32{0, 0, 1}, LeadingComments: googleproto.String(text)} +} + +// narrowFixture is a contract of two services, Wanted and Other, over a +// shared file that also declares a service and declarations neither needs. +// Wanted sets a custom method option defined in its own file, names a +// well-known type, an enum, a nested type of a shared message, and a nested +// type of its own request; Other is the only one importing empty.proto. +func narrowFixture(t *testing.T) []byte { + t.Helper() + holder := message("Holder") + holder.NestedType = []*descriptorpb.DescriptorProto{message("Nested")} + common := protoFile("acme/v1/common.proto", nil, + []*descriptorpb.DescriptorProto{ + message("Unused"), + message("Shared", messageField("leaf", 1, ".acme.v1.Leaf")), + message("Leaf"), + holder, + }, + []*descriptorpb.EnumDescriptorProto{enum("UnusedEnum"), enum("Color")}, + service("CommonService", method("Ping", ".acme.v1.Unused", ".acme.v1.Unused")), + ) + common.SourceCodeInfo = &descriptorpb.SourceCodeInfo{Location: []*descriptorpb.SourceCodeInfo_Location{ + leadingComment([]int32{4, 0}, " Unused doc\n"), + leadingComment([]int32{4, 2}, " Leaf doc\n"), + leadingComment([]int32{5, 1}, " Color doc\n"), + leadingComment([]int32{6, 0}, " CommonService doc\n"), + }} + + options := protoFile("acme/v1/options.proto", []string{"google/protobuf/descriptor.proto"}, nil, nil) + options.Extension = []*descriptorpb.FieldDescriptorProto{{ + Name: googleproto.String("scope"), + Number: googleproto.Int32(scopeOption), + Label: descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(), + Type: descriptorpb.FieldDescriptorProto_TYPE_STRING.Enum(), + Extendee: googleproto.String(".google.protobuf.MethodOptions"), + }} + + request := message("WantedRequest", + messageField("shared", 1, ".acme.v1.Shared"), + enumField("color", 2, ".acme.v1.Color"), + messageField("at", 3, ".google.protobuf.Timestamp"), + messageField("inner", 4, ".acme.v1.WantedRequest.Inner"), + ) + request.NestedType = []*descriptorpb.DescriptorProto{message("Inner")} + get := method("Get", ".acme.v1.WantedRequest", ".acme.v1.WantedResponse") + get.Options = &descriptorpb.MethodOptions{} + get.Options.ProtoReflect().SetUnknown(protowire.AppendString(protowire.AppendTag(nil, scopeOption, protowire.BytesType), "read")) + wanted := protoFile("acme/v1/wanted.proto", + []string{"acme/v1/common.proto", "acme/v1/options.proto", "google/protobuf/timestamp.proto"}, + []*descriptorpb.DescriptorProto{request, message("WantedResponse", messageField("nested", 1, ".acme.v1.Holder.Nested"))}, + nil, + service("Wanted", get), + ) + + other := protoFile("acme/v1/other.proto", + []string{"acme/v1/common.proto", "google/protobuf/empty.proto"}, + []*descriptorpb.DescriptorProto{message("OtherRequest", messageField("unused", 1, ".acme.v1.Unused"))}, + nil, + service("Other", method("Do", ".acme.v1.OtherRequest", ".google.protobuf.Empty")), + ) + + set := &descriptorpb.FileDescriptorSet{File: []*descriptorpb.FileDescriptorProto{ + protodesc.ToFileDescriptorProto(descriptorpb.File_google_protobuf_descriptor_proto), + protodesc.ToFileDescriptorProto(timestamppb.File_google_protobuf_timestamp_proto), + protodesc.ToFileDescriptorProto(emptypb.File_google_protobuf_empty_proto), + common, options, wanted, other, + }} + if _, err := protodesc.NewFiles(set); err != nil { + t.Fatalf("fixture does not resolve: %v", err) + } + data, err := googleproto.Marshal(set) + if err != nil { + t.Fatal(err) + } + return data +} + +func narrowEntry(contract []byte, services ...string) ContractEntry { + var set descriptorpb.FileDescriptorSet + _ = googleproto.Unmarshal(contract, &set) + return ContractEntry{ + Endpoint: composition.APIContractEndpoint{ + Service: "api", + Endpoint: "connect", + Kind: composition.APIContractKindProtobuf, + Package: "acme.v1", + Digest: composition.APIContractDigest(contract), + Services: composition.ProtobufServices(&set, "acme.v1"), + }, + ContractBytes: contract, + ModuleName: "acme", + ProtoSources: []coreproto.Source{{Path: "acme/v1/wanted.proto"}}, + Services: services, + } +} + +func parseNarrowed(t *testing.T, data []byte) map[string]*descriptorpb.FileDescriptorProto { + t.Helper() + var set descriptorpb.FileDescriptorSet + if err := googleproto.Unmarshal(data, &set); err != nil { + t.Fatal(err) + } + if _, err := protodesc.NewFiles(&set); err != nil { + t.Fatalf("narrowed contract does not resolve: %v", err) + } + files := map[string]*descriptorpb.FileDescriptorProto{} + for _, file := range set.GetFile() { + files[file.GetName()] = file + } + return files +} + +func declared(file *descriptorpb.FileDescriptorProto) (messages, enums, services []string) { + for _, m := range file.GetMessageType() { + messages = append(messages, m.GetName()) + } + for _, e := range file.GetEnumType() { + enums = append(enums, e.GetName()) + } + for _, s := range file.GetService() { + services = append(services, s.GetName()) + } + return messages, enums, services +} + +func keys(files map[string]*descriptorpb.FileDescriptorProto) []string { + names := make([]string, 0, len(files)) + for name := range files { + names = append(names, name) + } + sort.Strings(names) + return names +} + +// One selected service keeps exactly its methods' messages, what they name, +// the file defining the option it sets and the well-known types it uses; the +// other service, its file, its import and every declaration only it reaches +// are gone. +func TestNarrowEntriesKeepsOnlyTheSelectedServiceAndWhatItNeeds(t *testing.T) { + contract := narrowFixture(t) + narrowed, err := narrowEntries([]ContractEntry{narrowEntry(contract, "Wanted")}) + if err != nil { + t.Fatal(err) + } + entry := narrowed[0] + files := parseNarrowed(t, entry.ContractBytes) + + wantFiles := []string{"acme/v1/common.proto", "acme/v1/options.proto", "acme/v1/wanted.proto", "google/protobuf/descriptor.proto", "google/protobuf/timestamp.proto"} + if got := keys(files); !slices.Equal(got, wantFiles) { + t.Fatalf("files = %v, want %v", got, wantFiles) + } + var services []string + for _, file := range files { + _, _, s := declared(file) + services = append(services, s...) + } + if !slices.Equal(services, []string{"Wanted"}) { + t.Fatalf("services = %v, want only Wanted", services) + } + messages, enums, _ := declared(files["acme/v1/common.proto"]) + if want := []string{"Shared", "Leaf", "Holder"}; !slices.Equal(messages, want) { + t.Errorf("common.proto messages = %v, want %v", messages, want) + } + if want := []string{"Color"}; !slices.Equal(enums, want) { + t.Errorf("common.proto enums = %v, want %v", enums, want) + } + if got := files["acme/v1/common.proto"].GetMessageType()[2].GetNestedType(); len(got) != 1 { + t.Errorf("Holder lost its nested types: %v", got) + } + if got, want := files["acme/v1/wanted.proto"].GetDependency(), []string{"acme/v1/common.proto", "acme/v1/options.proto", "google/protobuf/timestamp.proto"}; !slices.Equal(got, want) { + t.Errorf("wanted.proto imports = %v, want %v", got, want) + } + if len(files["acme/v1/options.proto"].GetExtension()) != 1 { + t.Error("the file defining the option Wanted sets lost its extension") + } + if got := len(files["google/protobuf/descriptor.proto"].GetMessageType()); got != len(protodesc.ToFileDescriptorProto(descriptorpb.File_google_protobuf_descriptor_proto).GetMessageType()) { + t.Errorf("descriptor.proto was narrowed to %d messages; an upstream file is kept whole", got) + } + + comments := map[string]string{} + common := files["acme/v1/common.proto"] + for _, location := range common.GetSourceCodeInfo().GetLocation() { + path := location.GetPath() + switch path[0] { + case 4: + comments[common.GetMessageType()[path[1]].GetName()] = location.GetLeadingComments() + case 5: + comments[common.GetEnumType()[path[1]].GetName()] = location.GetLeadingComments() + default: + t.Errorf("a comment of a dropped declaration survived: %v %q", path, location.GetLeadingComments()) + } + } + if want := map[string]string{"Leaf": " Leaf doc\n", "Color": " Color doc\n"}; len(comments) != len(want) || comments["Leaf"] != want["Leaf"] || comments["Color"] != want["Color"] { + t.Errorf("comments = %v, want %v", comments, want) + } + + if len(entry.Endpoint.Services) != 1 || entry.Endpoint.Services[0].FullName != "acme.v1.Wanted" { + t.Errorf("catalog services = %+v, want only acme.v1.Wanted", entry.Endpoint.Services) + } + if entry.Endpoint.Digest != composition.APIContractDigest(entry.ContractBytes) { + t.Error("the catalog digest is not the narrowed contract's") + } + if entry.ProtoSources != nil { + t.Error("a narrowed entry still generates from the whole contract's sources") + } +} + +// Narrowing is a function of the contract: the same input gives the same +// bytes, and narrowing a narrowed contract changes nothing. +func TestNarrowEntriesIsStable(t *testing.T) { + contract := narrowFixture(t) + first, err := narrowEntries([]ContractEntry{narrowEntry(contract, "Wanted")}) + if err != nil { + t.Fatal(err) + } + second, err := narrowEntries([]ContractEntry{narrowEntry(contract, "Wanted")}) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(first[0].ContractBytes, second[0].ContractBytes) { + t.Fatal("two narrowings of one contract differ") + } + again, err := narrowEntries([]ContractEntry{narrowEntry(first[0].ContractBytes, "Wanted")}) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(first[0].ContractBytes, again[0].ContractBytes) { + t.Fatal("narrowing a narrowed contract changed it") + } +} + +func TestNarrowEntriesRefusesAServiceTheContractDoesNotDeclare(t *testing.T) { + _, err := narrowEntries([]ContractEntry{narrowEntry(narrowFixture(t), "Wanted", "Missing")}) + if err == nil || !strings.Contains(err.Error(), `"Missing"`) || !strings.Contains(err.Error(), "Other, Wanted") { + t.Fatalf("err = %v, want it to name Missing and the services declared", err) + } +} + +func TestNarrowEntriesLeavesALibrarySelectingNothingAsIs(t *testing.T) { + contract := narrowFixture(t) + entries := []ContractEntry{narrowEntry(contract)} + narrowed, err := narrowEntries(entries) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(narrowed[0].ContractBytes, contract) || len(narrowed[0].Endpoint.Services) != len(entries[0].Endpoint.Services) || narrowed[0].ProtoSources == nil { + t.Fatal("an entry naming no services was changed") + } +} + +// Entries of one library are generated into one tree, so a file two entries +// carry comes out the same for both, holding what either needs. +func TestNarrowEntriesNarrowsAnAggregatedLibraryAsOne(t *testing.T) { + contract := narrowFixture(t) + narrowed, err := narrowEntries([]ContractEntry{narrowEntry(contract, "Wanted"), narrowEntry(contract, "Other")}) + if err != nil { + t.Fatal(err) + } + wanted, other := parseNarrowed(t, narrowed[0].ContractBytes), parseNarrowed(t, narrowed[1].ContractBytes) + if !googleproto.Equal(wanted["acme/v1/common.proto"], other["acme/v1/common.proto"]) { + t.Fatal("the shared file differs between the two entries") + } + messages, _, services := declared(wanted["acme/v1/common.proto"]) + if !slices.Equal(messages, []string{"Unused", "Shared", "Leaf", "Holder"}) || len(services) != 0 { + t.Fatalf("common.proto = %v %v, want what Wanted and Other need and no service", messages, services) + } + if narrowed[0].Endpoint.Services[0].Name != "Wanted" || narrowed[1].Endpoint.Services[0].Name != "Other" { + t.Fatal("each entry's catalog lists its own selection") + } +} From 52d752313df913f65f1e1dcb2afa4e8019932ad5 Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Sun, 25 Oct 2026 23:59:04 -0400 Subject: [PATCH 2/2] fix(generate): a --services library records its producer's digest; narrowing follows extensions (r140) Review r140 of #1179: - F1/F7: the entry keeps the producer's contract digest. It is what sources[].contract-digest and clients.codefly.json both record, so editing an entry's services or adding an entry regenerates without --force; only a new producer contract needs it. The catalog under contract/ carries the digest of the contract persisted beside it. sync solution-sdk gains the --force its refusal names. - F2: each entry persists its own closure; bindings are generated from the library-wide closure, so a shared file is still generated once. - F3: options on extension ranges are visited. - F4: every extension of a kept message is kept, wherever it is declared. - F5: two entries carrying different versions of one file are refused, naming the file and both entries. - F6: extensions set inside message-valued options are followed, through the contract's descriptors or the protobuf runtime's. Refs #1173 Co-Authored-By: Claude Opus 5.5 --- cmd/generate/client_services_test.go | 100 +++++++- cmd/sync/solution_sdk.go | 3 + cmd/sync/solution_sdk_test.go | 6 +- docs/cli-reference.md | 1 + docs/commands.md | 34 +-- pkg/generators/client.go | 15 +- pkg/generators/narrow.go | 336 ++++++++++++++++++++------- pkg/generators/narrow_test.go | 154 +++++++++++- 8 files changed, 536 insertions(+), 113 deletions(-) diff --git a/cmd/generate/client_services_test.go b/cmd/generate/client_services_test.go index 5d2f24d7..3e1b6843 100644 --- a/cmd/generate/client_services_test.go +++ b/cmd/generate/client_services_test.go @@ -10,7 +10,9 @@ import ( "strings" "testing" + "github.com/codefly-dev/cli/pkg/generators" "github.com/codefly-dev/core/composition" + "github.com/codefly-dev/core/languages" "github.com/codefly-dev/core/resources" googleproto "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/descriptorpb" @@ -138,7 +140,8 @@ func qualifyWorkspace(t *testing.T) { // TestGenerateClientServicesNarrowsTheWholeLibrary: a library generated for // one service of a contract carries that service alone (#1173). Its persisted // descriptor set declares that service and the declarations it reaches, its -// catalog and manifest list only it, and no language's bindings or facade +// catalog and manifest list only it (the manifest at the producer's digest, +// which identifies the source), and no language's bindings or facade // mention the other service or what only the other service needs — and each // still builds. func TestGenerateClientServicesNarrowsTheWholeLibrary(t *testing.T) { @@ -217,8 +220,12 @@ func TestGenerateClientServicesNarrowsTheWholeLibrary(t *testing.T) { if err := yaml.Unmarshal(manifestData, &manifest); err != nil { t.Fatal(err) } - if source := manifest.Sources[0]; !slices.Equal(source.Services, []string{"WantedService"}) || source.ContractDigest != endpoint.Digest { - t.Errorf("manifest source = %+v, want services [WantedService] at the persisted contract's digest %s", source, endpoint.Digest) + producer, err := composition.LoadAPIContractCatalog(filepath.Dir(filepath.Dir(contractsDir))) + if err != nil { + t.Fatal(err) + } + if source := manifest.Sources[0]; !slices.Equal(source.Services, []string{"WantedService"}) || source.ContractDigest != producer.Endpoints[0].Digest { + t.Errorf("manifest source = %+v, want services [WantedService] at the producer's digest %s", source, producer.Endpoints[0].Digest) } for _, lang := range []string{"go", "typescript", "python"} { @@ -276,3 +283,90 @@ func assertTreeLacks(t *testing.T, root string, words ...string) { t.Fatal(err) } } + +// TestGenerateClientServicesSelectionChangesRegenerateWithoutForce: what a +// library records as its source is the producer's contract, so editing an +// entry's services and adding an entry regenerate in place without --force +// (sync solution-sdk does both on every api.consumes edit). Each entry's +// persisted contract declares its own selection alone, both record the +// producer digest `publish clients` records, and regenerating is stable. +// Python, because one Go package cannot hold two facades of different names. +func TestGenerateClientServicesSelectionChangesRegenerateWithoutForce(t *testing.T) { + requireQualify(t) + contractsDir := writeTwoServicesContractsFixture(t) + catalog, err := composition.LoadAPIContractCatalog(filepath.Dir(filepath.Dir(contractsDir))) + if err != nil { + t.Fatal(err) + } + producer := catalog.Endpoints[0] + contract, err := generators.ReadCatalogContractBytes(filepath.Dir(filepath.Dir(contractsDir)), &producer) + if err != nil { + t.Fatal(err) + } + entry := func(endpoint, as string, services ...string) generators.ContractEntry { + e := producer + e.Endpoint = endpoint + return generators.ContractEntry{ + Endpoint: e, ContractBytes: contract, PackageID: catalog.Package, PackageVersion: catalog.Version, + ModuleName: "acme", FacadeModuleDefault: generators.DefaultFacadeModuleName(&e), Services: services, As: as, + } + } + output := filepath.Join(t.TempDir(), "lib") + generate := func(entries ...generators.ContractEntry) { + t.Helper() + if err := generators.GenerateClientLibrary(context.Background(), &generators.ClientLibraryRequest{ + Name: "acme-sdk", Output: output, Languages: []languages.Language{languages.PYTHON}, Entries: entries, + }); err != nil { + t.Fatalf("regenerate without --force: %v", err) + } + } + + generate(entry("connect", "", "WantedService")) + generate(entry("connect", "", "WantedService", "OtherService")) + both := []generators.ContractEntry{entry("connect", "", "WantedService"), entry("connect-other", "other", "OtherService")} + generate(both...) + first := hashTree(t, output) + generate(both...) + if again := hashTree(t, output); again != first { + t.Fatalf("regenerating the aggregated library drifted: %s != %s", again, first) + } + + for dir, want := range map[string]string{"acme-api-connect": "acme.v1.WantedService", "acme-api-connect-other": "acme.v1.OtherService"} { + data, err := os.ReadFile(filepath.Join(output, "contract", dir, "contract.binpb")) + if err != nil { + t.Fatal(err) + } + var set descriptorpb.FileDescriptorSet + if err := googleproto.Unmarshal(data, &set); err != nil { + t.Fatal(err) + } + var services []string + for _, file := range set.GetFile() { + for _, service := range file.GetService() { + services = append(services, file.GetPackage()+"."+service.GetName()) + } + } + if !slices.Equal(services, []string{want}) { + t.Errorf("%s persists services %v, want only %s", dir, services, want) + } + } + var manifest struct { + Sources []struct { + ContractDigest string `yaml:"contract-digest"` + } `yaml:"sources"` + } + manifestData, err := os.ReadFile(filepath.Join(output, resources.LibraryConfigurationName)) + if err != nil { + t.Fatal(err) + } + if err := yaml.Unmarshal(manifestData, &manifest); err != nil { + t.Fatal(err) + } + for _, source := range manifest.Sources { + if source.ContractDigest != producer.Digest { + t.Errorf("manifest records contract-digest %s, want the producer's %s", source.ContractDigest, producer.Digest) + } + } + assertPythonImports(t, filepath.Join(output, "python"), "acme_sdk.acme") + assertPythonImports(t, filepath.Join(output, "python"), "acme_sdk.other") +} diff --git a/cmd/sync/solution_sdk.go b/cmd/sync/solution_sdk.go index 87bde894..f623088e 100644 --- a/cmd/sync/solution_sdk.go +++ b/cmd/sync/solution_sdk.go @@ -27,6 +27,7 @@ var ( solutionSDKLanguages []string solutionSDKCheck bool solutionSDKApply bool + solutionSDKForce bool // skipGenerateForTests short-circuits the actual codegen call // (generators.GenerateClientLibrary), so a test can exercise resolution, @@ -60,6 +61,7 @@ func init() { SolutionSDKCmd.Flags().StringSliceVar(&solutionSDKLanguages, "language", nil, "languages to generate: go,typescript,python") SolutionSDKCmd.Flags().BoolVar(&solutionSDKCheck, "check", false, "do not write; exit 1 if the library or service-dependencies are out of date") SolutionSDKCmd.Flags().BoolVar(&solutionSDKApply, "apply-dependencies", false, "add missing service-dependencies entries for every bound consume entry") + SolutionSDKCmd.Flags().BoolVar(&solutionSDKForce, "force", false, "overwrite a library generated from a different version of a consumed contract") } func runSyncSolutionSDK(ctx context.Context) error { @@ -401,6 +403,7 @@ func generateSolutionSDKInto(ctx context.Context, name, outputDir string, entrie Output: outputDir, Languages: langs, Entries: entries, + Force: solutionSDKForce, GoModule: "github.com/codefly-dev/" + name + "-go", NpmPackage: "@codefly-dev/" + name, }) diff --git a/cmd/sync/solution_sdk_test.go b/cmd/sync/solution_sdk_test.go index 6cb825a3..9ee14c12 100644 --- a/cmd/sync/solution_sdk_test.go +++ b/cmd/sync/solution_sdk_test.go @@ -224,11 +224,11 @@ func loadFixtureWorkspace(t *testing.T, wsDir string) *resources.Workspace { func resetSolutionSDKFlags(t *testing.T) { t.Helper() - prevLangs, prevCheck, prevApply, prevSkip := solutionSDKLanguages, solutionSDKCheck, solutionSDKApply, skipGenerateForTests - solutionSDKLanguages, solutionSDKCheck, solutionSDKApply = nil, false, false + prevLangs, prevCheck, prevApply, prevForce, prevSkip := solutionSDKLanguages, solutionSDKCheck, solutionSDKApply, solutionSDKForce, skipGenerateForTests + solutionSDKLanguages, solutionSDKCheck, solutionSDKApply, solutionSDKForce = nil, false, false, false skipGenerateForTests = true t.Cleanup(func() { - solutionSDKLanguages, solutionSDKCheck, solutionSDKApply, skipGenerateForTests = prevLangs, prevCheck, prevApply, prevSkip + solutionSDKLanguages, solutionSDKCheck, solutionSDKApply, solutionSDKForce, skipGenerateForTests = prevLangs, prevCheck, prevApply, prevForce, prevSkip }) } diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 8723b396..cc0fe0cd 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -4932,6 +4932,7 @@ Flags: ``` --apply-dependencies add missing service-dependencies entries for every bound consume entry --check do not write; exit 1 if the library or service-dependencies are out of date + --force overwrite a library generated from a different version of a consumed contract --language strings languages to generate: go,typescript,python ``` diff --git a/docs/commands.md b/docs/commands.md index ff0208b0..175843f1 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -2434,6 +2434,7 @@ lodestar derives from the manifest. | `--language` | Required, repeatable or comma-separated: `go`, `typescript`, `python` | | `--check` | Do not write; exit 1 if the library or `service-dependencies` are out of date | | `--apply-dependencies` | Add any `service-dependencies` entry missing for a bound consume entry to the entry service's `service.codefly.yaml`; without it, missing entries are printed as a warning. Existing entries are never removed. | +| `--force` | Overwrite a library generated from a different version of a consumed contract (its recorded `sources[].contract-digest` differs) | Each bound entry is resolved against the workspace's own composition: the composed module named by `module` (workspace `add module` first), its @@ -3446,19 +3447,26 @@ catalog entry) and generate from its descriptor bytes. generated. The contract it persists under `contract/` keeps the named services and the declarations they reach: their methods' messages and, transitively, every message and enum those name (a nested type keeps the top-level message -holding it). A file defining a custom option those declarations set is kept -whole, as is a shared upstream file (`google/protobuf`, `google/api`, -`google/rpc`, `google/type`, `buf/validate`). No other service is declared -anywhere, so the catalog, the manifest's `sources[].services`, the bindings and -the facade of every language carry the named services alone; the catalog and -`sources[].contract-digest` record the narrowed contract's digest. A name the -contract does not declare is refused, naming the services it does declare. -Narrowing is deterministic, so regenerating a library reproduces it byte for -byte. A library generated from a local source with `--services` is generated -from the narrowed descriptor set rather than the proto sources. When `sync -solution-sdk` aggregates several contracts into one library, the declarations -kept are computed across all of them, so a file two entries carry comes out the -same for both; an entry naming no services keeps its whole contract. +holding it), and every extension of a kept message. A file defining a custom +option those declarations set — on any element, extension ranges included, or +nested inside another option's value — is kept whole, as is a shared upstream +file (`google/protobuf`, `google/api`, `google/rpc`, `google/type`, +`buf/validate`). No other service is declared anywhere, so the catalog, the +manifest's `sources[].services`, the bindings and the facade of every language +carry the named services alone. The catalog's digest is the persisted +(narrowed) contract's; `sources[].contract-digest` stays the producer's, the +same digest `publish clients` records, so it keeps identifying the contract the +library was generated from: changing `--services` regenerates in place, and only +a new producer contract needs `--force`. A name the contract does not declare is +refused, naming the services it does declare. Narrowing is deterministic, so +regenerating a library reproduces it byte for byte. A library generated from a +local source with `--services` is generated from the narrowed descriptor set +rather than the proto sources. When `sync solution-sdk` aggregates several +contracts into one library, each entry persists its own narrowed contract, while +the bindings are generated from the closure of every entry at once, so a file +two entries carry is generated the same for both; an entry naming no services +keeps its whole contract, and two entries carrying different versions of one +file are refused, naming the file and both entries. TypeScript's generated entry point (`src/index.ts`) has three parts. Each facade is re-exported flat under the name its plugin gives it (`accounts`, diff --git a/pkg/generators/client.go b/pkg/generators/client.go index deb808d8..9ab8424b 100644 --- a/pkg/generators/client.go +++ b/pkg/generators/client.go @@ -62,6 +62,12 @@ type ContractEntry struct { // As overrides this entry's facade entry-point name (default: // FacadeModuleDefault, else defaultFacadeModule). As string + + // generateFrom, when set, is the descriptor set this entry's bindings are + // generated from instead of ContractBytes: the library's narrowing, which + // in an aggregated library also holds what its other entries need from a + // file they share (see narrowEntries). + generateFrom []byte } // ClientLibraryRequest is one library-generation run: N contract entries @@ -386,8 +392,12 @@ func (s *stagedLanguage) apply(goModule, npmPackage, pyPackage string) (generate // stageProtobufEntry generates one contract entry into a scratch directory of // its own, which the caller owns once this returns without error. func stageProtobufEntry(ctx context.Context, lang languages.Language, entry *ContractEntry, facade bool) (_ stagedEntry, err error) { + contract := entry.ContractBytes + if entry.generateFrom != nil { + contract = entry.generateFrom + } var set descriptorpb.FileDescriptorSet - if err = googleproto.Unmarshal(entry.ContractBytes, &set); err != nil { + if err = googleproto.Unmarshal(contract, &set); err != nil { return stagedEntry{}, fmt.Errorf("cannot parse contract descriptor set: %w", err) } raw, err := os.MkdirTemp("", "codefly-client-raw") @@ -840,8 +850,11 @@ func writeOneContractDir(ctx context.Context, contractDir string, entry *Contrac return err } + // The catalog describes the contract persisted beside it, which --services + // narrows; the manifest's contract-digest stays the producer's. catalogEntry := entry.Endpoint catalogEntry.Path = filename + catalogEntry.Digest = composition.APIContractDigest(entry.ContractBytes) packageID := entry.PackageID if packageID == "" { packageID = localPackageID diff --git a/pkg/generators/narrow.go b/pkg/generators/narrow.go index 19a148f8..a1cb88a4 100644 --- a/pkg/generators/narrow.go +++ b/pkg/generators/narrow.go @@ -17,38 +17,32 @@ import ( // narrowEntries narrows a library to the services its entries select. // -// A protobuf entry naming Services binds those services and nothing else, so -// its contract is rewritten to declare only them before anything is generated: -// the persisted descriptor set, the catalog and the manifest describe the -// narrowed contract, and the bindings and facade are generated from it. The -// narrowed set keeps, from the files declaring the selected services and the -// files those reach, the top-level declarations the services need: their -// methods' messages and, transitively, every message and enum a kept message -// names (a nested type keeps the top-level message holding it). A file -// defining a custom option a kept declaration sets is kept whole, with its -// imports, and so is a file of a shared upstream module: neither is the -// library's own, and the upstream ones keep the shape their distributions -// publish. No file declares a service that is not selected. +// A protobuf entry naming Services binds those services and nothing else. Its +// persisted contract (contract.binpb and its catalog) declares only them: the +// files declaring them and, of the files those reach, the top-level +// declarations they need — their methods' messages and, transitively, every +// message and enum a kept message names (a nested type keeps the top-level +// message holding it), and every extension of a kept message. A file defining +// a custom option a kept declaration sets is kept whole, with its imports, and +// so is a file of a shared upstream module: neither is the library's own, and +// the upstream ones keep the shape their distributions publish. No file +// declares a service that is not selected. // -// The narrowing is the library's, not one entry's: an aggregated library +// Bindings are the library's, not one entry's: an aggregated library // generates every entry into one tree, so a file two entries carry must come -// out the same for both. The kept declarations are computed across every -// entry at once, and an entry naming no Services keeps its whole contract -// (and is returned untouched). A library in which no entry names Services is -// returned as is. +// out the same for both. Each entry is therefore generated from the library's +// narrowing — the closure of every entry's selection at once, where an entry +// naming no Services keeps its whole contract — while its persisted contract +// is its own closure. For a library of one entry the two are the same set. +// +// The entry's Endpoint keeps the producer's digest: it identifies the contract +// the library was generated from, which a selection change does not move. A +// library in which no entry names Services is returned as is. func narrowEntries(entries []ContractEntry) ([]ContractEntry, error) { if !slices.ContainsFunc(entries, func(entry ContractEntry) bool { return selectsServices(&entry) }) { return entries, nil } - n := &narrowing{ - files: map[string]*descriptorpb.FileDescriptorProto{}, - upstream: map[string]bool{}, - types: map[string]declaration{}, - services: map[string]string{}, - extensions: map[extensionKey]string{}, - selected: map[string]bool{}, - kept: map[string]*keptFile{}, - } + library := newNarrowing() sets := make([]*descriptorpb.FileDescriptorSet, len(entries)) for i := range entries { if entries[i].Endpoint.Kind != composition.APIContractKindProtobuf { @@ -61,7 +55,9 @@ func narrowEntries(entries []ContractEntry) ([]ContractEntry, error) { return nil, fmt.Errorf("entry %s: cannot parse contract descriptor set: %w", entryLabel(&entries[i]), err) } sets[i] = set - n.index(set) + if err := library.index(set, entryLabel(&entries[i])); err != nil { + return nil, err + } } selections := make([][]composition.APIContractService, len(entries)) @@ -70,12 +66,7 @@ func narrowEntries(entries []ContractEntry) ([]ContractEntry, error) { continue } if !selectsServices(&entries[i]) { - for _, file := range sets[i].GetFile() { - n.keepWhole(file.GetName()) - for _, service := range file.GetService() { - n.selected[qualify(file.GetPackage(), service.GetName())] = true - } - } + library.selectWhole(sets[i]) continue } selected, err := selectedServices(&entries[i]) @@ -83,20 +74,10 @@ func narrowEntries(entries []ContractEntry) ([]ContractEntry, error) { return nil, err } selections[i] = selected - for _, service := range selected { - n.selected[service.FullName] = true - } - } - roots := make([]string, 0, len(n.selected)) - for fullName := range n.selected { - roots = append(roots, fullName) + library.selectServices(selected) } - sort.Strings(roots) - for _, fullName := range roots { - n.keepService(fullName) - } - if n.err != nil { - return nil, fmt.Errorf("cannot narrow the contract to its selected services: %w", n.err) + if err := library.close(); err != nil { + return nil, err } narrowed := slices.Clone(entries) @@ -105,13 +86,25 @@ func narrowEntries(entries []ContractEntry) ([]ContractEntry, error) { continue } entry := &narrowed[i] - data, err := n.emit(sets[i]) + own := newNarrowing() + if err := own.index(sets[i], entryLabel(entry)); err != nil { + return nil, err + } + own.selectServices(selections[i]) + if err := own.close(); err != nil { + return nil, err + } + contract, err := own.emit(sets[i]) + if err != nil { + return nil, fmt.Errorf("entry %s: %w", entryLabel(entry), err) + } + generated, err := library.emit(sets[i]) if err != nil { return nil, fmt.Errorf("entry %s: %w", entryLabel(entry), err) } - entry.ContractBytes = data + entry.ContractBytes = contract + entry.generateFrom = generated entry.Endpoint.Services = selections[i] - entry.Endpoint.Digest = composition.APIContractDigest(data) // The proto sources declare the whole contract; the narrowed // descriptor set is what is generated from. entry.ProtoSources = nil @@ -166,22 +159,41 @@ func selectedServices(entry *ContractEntry) ([]composition.APIContractService, e return selected, nil } -// narrowing is the closure of what a library's selected services need, over -// every file its entries' contracts carry (by name, first seen). +// narrowing is the closure of what a set of selected services needs, over +// every file the contracts it indexes carry. type narrowing struct { - files map[string]*descriptorpb.FileDescriptorProto - upstream map[string]bool - types map[string]declaration // message or enum full name -> its top-level declaration - services map[string]string // service full name -> file - extensions map[extensionKey]string - selected map[string]bool // service full names - kept map[string]*keptFile - err error + files map[string]*descriptorpb.FileDescriptorProto + origin map[string]string // file -> the entry it was first indexed from + upstream map[string]bool + types map[string]declaration // message or enum full name -> its top-level declaration + messages map[string]*descriptorpb.DescriptorProto + services map[string]string // service full name -> file + extensions map[extensionKey]extensionDefinition + extensionsOf map[string][]extensionDeclaration // extendee full name -> its extensions + selected map[string]bool // service full names + kept map[string]*keptFile + err error +} + +func newNarrowing() *narrowing { + return &narrowing{ + files: map[string]*descriptorpb.FileDescriptorProto{}, + origin: map[string]string{}, + upstream: map[string]bool{}, + types: map[string]declaration{}, + messages: map[string]*descriptorpb.DescriptorProto{}, + services: map[string]string{}, + extensions: map[extensionKey]extensionDefinition{}, + extensionsOf: map[string][]extensionDeclaration{}, + selected: map[string]bool{}, + kept: map[string]*keptFile{}, + } } // declaration is a top-level message or enum of a file. type declaration struct { file string + name string // its full name message *descriptorpb.DescriptorProto enum *descriptorpb.EnumDescriptorProto } @@ -191,13 +203,29 @@ type extensionKey struct { number int32 } +// extensionDefinition is where an extension is declared and, when its value +// is a message, that message's full name. +type extensionDefinition struct { + file string + messageType string +} + +// extensionDeclaration is one extension of a message: a file's top-level +// extension, or one nested in a message (container), which keeps it. +type extensionDeclaration struct { + file string + field *descriptorpb.FieldDescriptorProto + container *declaration +} + // keptFile is what a file keeps: everything when whole, otherwise the named // top-level declarations, and the imports they need. type keptFile struct { - whole bool - messages map[string]bool - enums map[string]bool - imports map[string]bool + whole bool + messages map[string]bool + enums map[string]bool + extensions map[string]bool + imports map[string]bool } func qualify(pkg, name string) string { @@ -213,45 +241,93 @@ func (n *narrowing) fail(format string, args ...any) { } } -func (n *narrowing) index(set *descriptorpb.FileDescriptorSet) { +// index adds set's files. A file another entry already carries must be the +// same file: one library generates one copy of it. +func (n *narrowing) index(set *descriptorpb.FileDescriptorSet, entry string) error { for _, file := range set.GetFile() { name := file.GetName() - if _, seen := n.files[name]; seen { + if seen, ok := n.files[name]; ok { + if !googleproto.Equal(seen, file) { + return fmt.Errorf("entries %s and %s carry different versions of %s; one library generates one copy of a file", n.origin[name], entry, name) + } continue } n.files[name] = file + n.origin[name] = entry if _, upstream := matchUpstreamModule(file); upstream { n.upstream[name] = true } pkg := file.GetPackage() for _, message := range file.GetMessageType() { - n.indexMessage(qualify(pkg, message.GetName()), declaration{file: name, message: message}, message) + fullName := qualify(pkg, message.GetName()) + n.indexMessage(fullName, declaration{file: name, name: fullName, message: message}, message) } for _, enum := range file.GetEnumType() { - n.types[qualify(pkg, enum.GetName())] = declaration{file: name, enum: enum} + fullName := qualify(pkg, enum.GetName()) + n.types[fullName] = declaration{file: name, name: fullName, enum: enum} } for _, service := range file.GetService() { n.services[qualify(pkg, service.GetName())] = name } - n.indexExtensions(name, file.GetExtension()) + n.indexExtensions(name, nil, file.GetExtension()) } + return nil } func (n *narrowing) indexMessage(fullName string, top declaration, message *descriptorpb.DescriptorProto) { n.types[fullName] = top + n.messages[fullName] = message for _, nested := range message.GetNestedType() { n.indexMessage(fullName+"."+nested.GetName(), top, nested) } for _, enum := range message.GetEnumType() { n.types[fullName+"."+enum.GetName()] = top } - n.indexExtensions(top.file, message.GetExtension()) + n.indexExtensions(top.file, &top, message.GetExtension()) } -func (n *narrowing) indexExtensions(file string, extensions []*descriptorpb.FieldDescriptorProto) { +func (n *narrowing) indexExtensions(file string, container *declaration, extensions []*descriptorpb.FieldDescriptorProto) { for _, extension := range extensions { - n.extensions[extensionKey{strings.TrimPrefix(extension.GetExtendee(), "."), extension.GetNumber()}] = file + extendee := strings.TrimPrefix(extension.GetExtendee(), ".") + definition := extensionDefinition{file: file} + if extension.GetType() == descriptorpb.FieldDescriptorProto_TYPE_MESSAGE { + definition.messageType = strings.TrimPrefix(extension.GetTypeName(), ".") + } + n.extensions[extensionKey{extendee, extension.GetNumber()}] = definition + n.extensionsOf[extendee] = append(n.extensionsOf[extendee], extensionDeclaration{file: file, field: extension, container: container}) + } +} + +// selectWhole keeps every file of set whole, with every service it declares. +func (n *narrowing) selectWhole(set *descriptorpb.FileDescriptorSet) { + for _, file := range set.GetFile() { + n.keepWhole(file.GetName()) + for _, service := range file.GetService() { + n.selected[qualify(file.GetPackage(), service.GetName())] = true + } + } +} + +func (n *narrowing) selectServices(services []composition.APIContractService) { + for _, service := range services { + n.selected[service.FullName] = true + } +} + +// close keeps what every selected service needs. +func (n *narrowing) close() error { + roots := make([]string, 0, len(n.selected)) + for fullName := range n.selected { + roots = append(roots, fullName) } + sort.Strings(roots) + for _, fullName := range roots { + n.keepService(fullName) + } + if n.err != nil { + return fmt.Errorf("cannot narrow the contract to its selected services: %w", n.err) + } + return nil } // file is name's kept state, created (and its file options visited) the first @@ -260,7 +336,7 @@ func (n *narrowing) file(name string) *keptFile { if kept, ok := n.kept[name]; ok { return kept } - kept := &keptFile{messages: map[string]bool{}, enums: map[string]bool{}, imports: map[string]bool{}} + kept := &keptFile{messages: map[string]bool{}, enums: map[string]bool{}, extensions: map[string]bool{}, imports: map[string]bool{}} n.kept[name] = kept if file := n.files[name]; file != nil { n.visitOptions(name, file.GetOptions()) @@ -276,9 +352,16 @@ func (n *narrowing) keepWhole(name string) { return } kept.whole = true - for _, dependency := range n.files[name].GetDependency() { + file := n.files[name] + for _, dependency := range file.GetDependency() { n.keepWhole(dependency) } + if n.upstream[name] { + return + } + for _, message := range file.GetMessageType() { + n.followExtensions(qualify(file.GetPackage(), message.GetName()), message) + } } func (n *narrowing) keepService(fullName string) { @@ -312,6 +395,10 @@ func (n *narrowing) useType(from, typeName string) { return } n.require(from, decl.file) + n.keep(decl) +} + +func (n *narrowing) keep(decl declaration) { kept := n.file(decl.file) switch { case kept.whole: @@ -320,12 +407,34 @@ func (n *narrowing) useType(from, typeName string) { case decl.message != nil && !kept.messages[decl.message.GetName()]: kept.messages[decl.message.GetName()] = true n.visitMessage(decl.file, decl.message) + n.followExtensions(decl.name, decl.message) case decl.enum != nil && !kept.enums[decl.enum.GetName()]: kept.enums[decl.enum.GetName()] = true n.visitEnum(decl.file, decl.enum) } } +// followExtensions keeps every extension of a kept message (and of the types +// nested in it), wherever it is declared: the message's readers rely on them. +func (n *narrowing) followExtensions(fullName string, message *descriptorpb.DescriptorProto) { + for _, extension := range n.extensionsOf[fullName] { + if extension.container != nil { + n.keep(*extension.container) + continue + } + kept := n.file(extension.file) + if kept.whole || kept.extensions[extension.field.GetName()] { + continue + } + kept.extensions[extension.field.GetName()] = true + n.visitField(extension.file, extension.field) + n.useType(extension.file, extension.field.GetExtendee()) + } + for _, nested := range message.GetNestedType() { + n.followExtensions(fullName+"."+nested.GetName(), nested) + } +} + func (n *narrowing) visitMessage(file string, message *descriptorpb.DescriptorProto) { n.visitOptions(file, message.GetOptions()) for _, field := range message.GetField() { @@ -335,6 +444,9 @@ func (n *narrowing) visitMessage(file string, message *descriptorpb.DescriptorPr n.visitField(file, extension) n.useType(file, extension.GetExtendee()) } + for _, extensionRange := range message.GetExtensionRange() { + n.visitOptions(file, extensionRange.GetOptions()) + } for _, oneof := range message.GetOneofDecl() { n.visitOptions(file, oneof.GetOptions()) } @@ -361,42 +473,92 @@ func (n *narrowing) visitEnum(file string, enum *descriptorpb.EnumDescriptorProt } // visitOptions keeps, whole, every file defining a custom option set in -// options: a field the options message does not declare is an extension. +// options, however deep in a message-valued option it is set. func (n *narrowing) visitOptions(from string, options googleproto.Message) { if options == nil || !options.ProtoReflect().IsValid() { return } - descriptor := options.ProtoReflect().Descriptor() + name := string(options.ProtoReflect().Descriptor().FullName()) data, err := googleproto.MarshalOptions{Deterministic: true}.Marshal(options) if err != nil { - n.fail("%s: cannot read %s: %w", from, descriptor.FullName(), err) + n.fail("%s: cannot read %s: %w", from, name, err) return } + n.visitValue(from, name, data) +} + +// visitValue walks the encoded value of message: a field the message does not +// declare is an extension, whose defining file is kept whole, and a +// message-valued field (declared or extension) is walked in turn. +func (n *narrowing) visitValue(from, message string, data []byte) { for len(data) > 0 { number, kind, length := protowire.ConsumeTag(data) if length < 0 { - n.fail("%s: malformed %s", from, descriptor.FullName()) + n.fail("%s: malformed %s", from, message) return } data = data[length:] if length = protowire.ConsumeFieldValue(number, kind, data); length < 0 { - n.fail("%s: malformed %s", from, descriptor.FullName()) + n.fail("%s: malformed %s", from, message) return } + value := data[:length] data = data[length:] - if descriptor.Fields().ByNumber(number) != nil { - continue + declared, messageType, known := n.fieldOf(message, number) + if !known { + n.fail("%s sets an option of type %s, which neither the contract nor the protobuf runtime describes", from, message) + return + } + if !declared { + definition, ok := n.extensions[extensionKey{message, int32(number)}] + if !ok { + n.fail("%s sets option %d of %s, which no file of the contract defines", from, number, message) + continue + } + n.require(from, definition.file) + n.keepWhole(definition.file) + messageType = definition.messageType } - defining, ok := n.extensions[extensionKey{string(descriptor.FullName()), int32(number)}] - if !ok { - n.fail("%s sets option %d of %s, which no file of the contract defines", from, number, descriptor.FullName()) + if messageType == "" || kind != protowire.BytesType { continue } - n.require(from, defining) - n.keepWhole(defining) + if inner, length := protowire.ConsumeBytes(value); length >= 0 { + n.visitValue(from, messageType, inner) + } } } +// fieldOf is whether message declares field number and, when it is +// message-valued, its message type; known is false when neither the contract +// nor the protobuf runtime describes message. +func (n *narrowing) fieldOf(message string, number protowire.Number) (declared bool, messageType string, known bool) { + if descriptor, ok := n.messages[message]; ok { + for _, field := range descriptor.GetField() { + if field.GetNumber() != int32(number) { + continue + } + if field.GetType() == descriptorpb.FieldDescriptorProto_TYPE_MESSAGE { + messageType = strings.TrimPrefix(field.GetTypeName(), ".") + } + return true, messageType, true + } + return false, "", true + } + found, err := protoregistry.GlobalFiles.FindDescriptorByName(protoreflect.FullName(message)) + descriptor, ok := found.(protoreflect.MessageDescriptor) + if err != nil || !ok { + return false, "", false + } + field := descriptor.Fields().ByNumber(number) + if field == nil { + return false, "", true + } + if field.Message() != nil { + messageType = string(field.Message().FullName()) + } + return true, messageType, true +} + // require records that from needs target: the import of from that reaches it, // directly or through a chain of public imports, each of which is kept. func (n *narrowing) require(from, target string) { @@ -470,7 +632,9 @@ func (n *narrowing) narrowFile(file *descriptorpb.FileDescriptorProto, kept *kep services := keptIndexes(len(file.GetService()), func(i int) bool { return n.selected[qualify(file.GetPackage(), file.GetService()[i].GetName())] }) - extensions := keptIndexes(len(file.GetExtension()), func(int) bool { return kept.whole }) + extensions := keptIndexes(len(file.GetExtension()), func(i int) bool { + return kept.whole || kept.extensions[file.GetExtension()[i].GetName()] + }) dependencies := keptIndexes(len(file.GetDependency()), func(i int) bool { return kept.whole || kept.imports[file.GetDependency()[i]] }) diff --git a/pkg/generators/narrow_test.go b/pkg/generators/narrow_test.go index 9e569efe..ec83cd49 100644 --- a/pkg/generators/narrow_test.go +++ b/pkg/generators/narrow_test.go @@ -267,8 +267,11 @@ func TestNarrowEntriesKeepsOnlyTheSelectedServiceAndWhatItNeeds(t *testing.T) { if len(entry.Endpoint.Services) != 1 || entry.Endpoint.Services[0].FullName != "acme.v1.Wanted" { t.Errorf("catalog services = %+v, want only acme.v1.Wanted", entry.Endpoint.Services) } - if entry.Endpoint.Digest != composition.APIContractDigest(entry.ContractBytes) { - t.Error("the catalog digest is not the narrowed contract's") + if entry.Endpoint.Digest != composition.APIContractDigest(contract) { + t.Error("the entry no longer records the producer's contract digest") + } + if !bytes.Equal(entry.generateFrom, entry.ContractBytes) { + t.Error("a library of one entry is not generated from the contract it persists") } if entry.ProtoSources != nil { t.Error("a narrowed entry still generates from the whole contract's sources") @@ -319,22 +322,159 @@ func TestNarrowEntriesLeavesALibrarySelectingNothingAsIs(t *testing.T) { } // Entries of one library are generated into one tree, so a file two entries -// carry comes out the same for both, holding what either needs. +// carry is generated the same for both, holding what either needs; each +// entry's persisted contract still declares its own selection alone, and +// records the producer's digest, which neither selection moves. func TestNarrowEntriesNarrowsAnAggregatedLibraryAsOne(t *testing.T) { contract := narrowFixture(t) narrowed, err := narrowEntries([]ContractEntry{narrowEntry(contract, "Wanted"), narrowEntry(contract, "Other")}) if err != nil { t.Fatal(err) } - wanted, other := parseNarrowed(t, narrowed[0].ContractBytes), parseNarrowed(t, narrowed[1].ContractBytes) + wanted, other := parseNarrowed(t, narrowed[0].generateFrom), parseNarrowed(t, narrowed[1].generateFrom) if !googleproto.Equal(wanted["acme/v1/common.proto"], other["acme/v1/common.proto"]) { - t.Fatal("the shared file differs between the two entries") + t.Fatal("the shared file is generated differently for the two entries") } messages, _, services := declared(wanted["acme/v1/common.proto"]) if !slices.Equal(messages, []string{"Unused", "Shared", "Leaf", "Holder"}) || len(services) != 0 { t.Fatalf("common.proto = %v %v, want what Wanted and Other need and no service", messages, services) } - if narrowed[0].Endpoint.Services[0].Name != "Wanted" || narrowed[1].Endpoint.Services[0].Name != "Other" { - t.Fatal("each entry's catalog lists its own selection") + + alone, err := narrowEntries([]ContractEntry{narrowEntry(contract, "Wanted")}) + if err != nil { + t.Fatal(err) + } + for i, want := range []string{"Wanted", "Other"} { + entry := narrowed[i] + var services []string + for _, file := range parseNarrowed(t, entry.ContractBytes) { + _, _, s := declared(file) + services = append(services, s...) + } + if !slices.Equal(services, []string{want}) || entry.Endpoint.Services[0].Name != want { + t.Errorf("entry %d persists services %v with catalog %v, want only %s", i, services, entry.Endpoint.Services, want) + } + if entry.Endpoint.Digest != composition.APIContractDigest(contract) { + t.Errorf("entry %d does not record the producer's digest", i) + } + } + if !bytes.Equal(narrowed[0].ContractBytes, alone[0].ContractBytes) { + t.Error("adding an entry changed another entry's persisted contract") + } +} + +func TestNarrowEntriesRefusesTwoVersionsOfOneFile(t *testing.T) { + contract := narrowFixture(t) + var set descriptorpb.FileDescriptorSet + if err := googleproto.Unmarshal(contract, &set); err != nil { + t.Fatal(err) + } + for _, file := range set.GetFile() { + if file.GetName() == "acme/v1/common.proto" { + leaf := file.GetMessageType()[2] + leaf.Field = append(leaf.Field, messageField("extra", 9, ".acme.v1.Shared")) + } + } + changed, err := googleproto.Marshal(&set) + if err != nil { + t.Fatal(err) + } + other := narrowEntry(changed, "Other") + other.Endpoint.Endpoint = "connect-other" + _, err = narrowEntries([]ContractEntry{narrowEntry(contract, "Wanted"), other}) + if err == nil || !strings.Contains(err.Error(), "acme/v1/common.proto") || !strings.Contains(err.Error(), "acme/api/connect-other") { + t.Fatalf("err = %v, want a refusal naming the file and both entries", err) + } +} + +func proto2File(name string, deps []string, messages ...*descriptorpb.DescriptorProto) *descriptorpb.FileDescriptorProto { + file := protoFile(name, deps, messages, nil) + file.Syntax = googleproto.String("proto2") + return file +} + +func stringExtension(name string, number int32, extendee string) *descriptorpb.FieldDescriptorProto { + return &descriptorpb.FieldDescriptorProto{ + Name: googleproto.String(name), + Number: googleproto.Int32(number), + Label: descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum(), + Type: descriptorpb.FieldDescriptorProto_TYPE_STRING.Enum(), + Extendee: googleproto.String(extendee), + } +} + +func extensionRange(start, end int32) *descriptorpb.DescriptorProto_ExtensionRange { + return &descriptorpb.DescriptorProto_ExtensionRange{Start: googleproto.Int32(start), End: googleproto.Int32(end)} +} + +// TestNarrowEntriesFollowsExtensionsAndNestedOptions covers what a proto2 +// contract adds: an option set on an extension range, an extension of a kept +// message declared in another file, and an extension set inside a +// message-valued option. Each defining file is kept; a declaration of those +// files nothing reaches is not. +func TestNarrowEntriesFollowsExtensionsAndNestedOptions(t *testing.T) { + rangeOption := proto2File("acme/v1/rangeopt.proto", []string{"google/protobuf/descriptor.proto"}) + rangeOption.Extension = []*descriptorpb.FieldDescriptorProto{stringExtension("range_note", 50002, ".google.protobuf.ExtensionRangeOptions")} + + carrier := message("P2", messageField("self", 1, ".acme.v1.P2")) + carrier.Field[0].Label = descriptorpb.FieldDescriptorProto_LABEL_OPTIONAL.Enum() + carrier.ExtensionRange = []*descriptorpb.DescriptorProto_ExtensionRange{extensionRange(100, 201)} + carrier.ExtensionRange[0].Options = &descriptorpb.ExtensionRangeOptions{} + carrier.ExtensionRange[0].Options.ProtoReflect().SetUnknown(protowire.AppendString(protowire.AppendTag(nil, 50002, protowire.BytesType), "note")) + p2 := proto2File("acme/v1/p2.proto", []string{"acme/v1/rangeopt.proto"}, carrier) + + more := messageField("more", 100, ".acme.v1.Extra") + more.Extendee = googleproto.String(".acme.v1.P2") + extending := proto2File("acme/v1/ext.proto", []string{"acme/v1/p2.proto"}, message("Extra"), message("Unrelated")) + extending.Extension = []*descriptorpb.FieldDescriptorProto{more} + + rule := message("Rule") + rule.Field = []*descriptorpb.FieldDescriptorProto{stringExtension("name", 1, "")} + rule.Field[0].Extendee = nil + rule.ExtensionRange = []*descriptorpb.DescriptorProto_ExtensionRange{extensionRange(10, 21)} + ruleOption := messageField("rule", 50003, ".acme.v1.Rule") + ruleOption.Extendee = googleproto.String(".google.protobuf.MethodOptions") + optionMessage := proto2File("acme/v1/optmsg.proto", []string{"google/protobuf/descriptor.proto"}, rule) + optionMessage.Extension = []*descriptorpb.FieldDescriptorProto{ruleOption} + ruleExtension := proto2File("acme/v1/ruleext.proto", []string{"acme/v1/optmsg.proto"}) + ruleExtension.Extension = []*descriptorpb.FieldDescriptorProto{stringExtension("tag", 10, ".acme.v1.Rule")} + + value := protowire.AppendString(protowire.AppendTag(nil, 1, protowire.BytesType), "n") + value = protowire.AppendString(protowire.AppendTag(value, 10, protowire.BytesType), "t") + do := method("Do", ".acme.v1.Req", ".acme.v1.Req") + do.Options = &descriptorpb.MethodOptions{} + do.Options.ProtoReflect().SetUnknown(protowire.AppendBytes(protowire.AppendTag(nil, 50003, protowire.BytesType), value)) + service := proto2File("acme/v1/svc.proto", []string{"acme/v1/p2.proto", "acme/v1/optmsg.proto", "acme/v1/ruleext.proto"}, message("Req", messageField("p", 1, ".acme.v1.P2"))) + service.Service = []*descriptorpb.ServiceDescriptorProto{{Name: googleproto.String("S"), Method: []*descriptorpb.MethodDescriptorProto{do}}} + + set := &descriptorpb.FileDescriptorSet{File: []*descriptorpb.FileDescriptorProto{ + protodesc.ToFileDescriptorProto(descriptorpb.File_google_protobuf_descriptor_proto), + rangeOption, p2, extending, optionMessage, ruleExtension, service, + }} + if _, err := protodesc.NewFiles(set); err != nil { + t.Fatalf("fixture does not resolve: %v", err) + } + contract, err := googleproto.Marshal(set) + if err != nil { + t.Fatal(err) + } + narrowed, err := narrowEntries([]ContractEntry{narrowEntry(contract, "S")}) + if err != nil { + t.Fatal(err) + } + files := parseNarrowed(t, narrowed[0].ContractBytes) + want := []string{"acme/v1/ext.proto", "acme/v1/optmsg.proto", "acme/v1/p2.proto", "acme/v1/rangeopt.proto", "acme/v1/ruleext.proto", "acme/v1/svc.proto", "google/protobuf/descriptor.proto"} + if got := keys(files); !slices.Equal(got, want) { + t.Fatalf("files = %v, want %v", got, want) + } + if got := files["acme/v1/p2.proto"].GetDependency(); !slices.Equal(got, []string{"acme/v1/rangeopt.proto"}) { + t.Errorf("p2.proto imports %v, want the file defining its extension-range option", got) + } + if got := files["acme/v1/svc.proto"].GetDependency(); !slices.Contains(got, "acme/v1/ruleext.proto") { + t.Errorf("svc.proto imports %v, want the file defining the extension set inside its rule option", got) + } + ext := files["acme/v1/ext.proto"] + if messages, _, _ := declared(ext); !slices.Equal(messages, []string{"Extra"}) || len(ext.GetExtension()) != 1 { + t.Errorf("ext.proto keeps %v and %d extensions, want Extra and the extension of P2", messages, len(ext.GetExtension())) } }