From d5657999005d69d296f205353c7136a18cab3cd2 Mon Sep 17 00:00:00 2001 From: antoinetoussaint-byte <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Sun, 25 Oct 2026 20:08:49 -0400 Subject: [PATCH] fix(generate client): generate every language before writing the library (#1163) A failed run deleted the library's generated tree before the companion ran: GenerateClientLibrary rewrote contract/ first, then each language wiped its gen/ (_gen/ and the facades for Python) and only afterwards asked the companion to generate. When the companion failed, the library was left without bindings. Builds older than #1128 also wiped every module at the Python package root, including hand-written ones. Every language now runs the companion into scratch directories first (stageLanguage). contract/ and each language directory are written only once all of them have succeeded (stagedLanguage.apply), with the existing ownership rules unchanged: the wipe still spares hand-written files and scaffolds. This is how `generate proto` already stages. OpenAPI clients stage the same way, generating against a copy of the library's go.mod so the import paths match. The buf/validate half of #1163 is already fixed at its owner: core 15188366 gives Python's buf.yaml the dependencies every other language has, and main carries it since #1128 moved core to 88f4434c. The CLI that reported the issue predated that pin. A new companion-gated test holds the Python sources path to it: a proto importing buf/validate generates and its bindings import. Also fixes two pre-existing test problems: assertPythonImports ran "proto:" without the registry, and TestGenerateClientPythonMapsUpstreamProtoModules still expected option-only imports that companion 0.0.17's Python plugin drops. Co-Authored-By: Claude Opus 5.5 --- cmd/generate/client_test.go | 174 ++++++++++++++++++- docs/commands.md | 11 ++ pkg/generators/client.go | 217 +++++++++++++++--------- pkg/generators/client_library.go | 83 ++++++--- pkg/generators/python_ownership_test.go | 100 +++++++++-- 5 files changed, 470 insertions(+), 115 deletions(-) diff --git a/cmd/generate/client_test.go b/cmd/generate/client_test.go index 10847bbc..d45bf046 100644 --- a/cmd/generate/client_test.go +++ b/cmd/generate/client_test.go @@ -16,8 +16,10 @@ import ( "testing" "github.com/codefly-dev/cli/pkg/conformance/conformancetest" + "github.com/codefly-dev/cli/pkg/generators" coreproto "github.com/codefly-dev/core/companions/proto" "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" @@ -270,13 +272,166 @@ func TestGenerateClientPythonMapsUpstreamProtoModules(t *testing.T) { } } + // A shared module a message's fields use is imported from its + // distribution, which the library declares. One used only for an option + // (buf/validate, google/api) is dropped by the companion's Python plugin + // (companion 0.0.17), so the bindings neither import nor depend on it. bindings := filepath.Join(genDir, "accounts", "v1", "accounts_pb2.py") - assertFileContains(t, bindings, "from buf.validate import validate_pb2") - assertFileContains(t, bindings, "from google.api import annotations_pb2") + assertFileContains(t, bindings, "from google.rpc import status_pb2") + assertFileContains(t, bindings, "from google.type import money_pb2") + assertFileLacks(t, bindings, "buf.validate") + assertFileLacks(t, bindings, "from google.api import") pyproject := filepath.Join(clientOutput, "python", "pyproject.toml") - assertFileContains(t, pyproject, `"protovalidate"`) assertFileContains(t, pyproject, `"googleapis-common-protos"`) + assertFileLacks(t, pyproject, `"protovalidate"`) +} + +// TestGenerateClientPythonFromSourcesResolvesProtovalidate: a local service +// whose proto validates its fields with buf/validate generates a Python +// client from its own sources (#1163). Python's buf configuration used to +// resolve googleapis alone, so the import did not exist for Python while Go +// and TypeScript generated the same contract; the companion's configuration +// now gives every language the same dependencies, and this holds the CLI's +// Python path to it. +func TestGenerateClientPythonFromSourcesResolvesProtovalidate(t *testing.T) { + requireQualify(t) + output := filepath.Join(t.TempDir(), "lib") + entry := localProtoEntry(t, validatedAccountsProto) + if len(entry.ProtoSources) == 0 { + t.Fatal("the local entry carries no proto sources, so this would not exercise the sources path") + } + + if err := generatePythonLibrary(output, entry); err != nil { + t.Fatalf("generate python client: %v", err) + } + + // The import is used only for a field option, which the companion's + // Python plugin drops, so the bindings stand alone and import. + pythonDir := filepath.Join(output, "python") + bindings := filepath.Join(pythonDir, "accounts_client", "_gen", "accounts", "v1", "accounts_pb2.py") + assertFileContains(t, bindings, "source: accounts/v1/accounts.proto") + if _, err := os.Stat(filepath.Join(pythonDir, "accounts_client", "_gen", "buf")); !os.IsNotExist(err) { + t.Errorf("_gen/buf was generated: protovalidate is the upstream distribution's (stat error: %v)", err) + } + assertPythonImports(t, pythonDir, "accounts_client._gen.accounts.v1.accounts_pb2") +} + +// TestGenerateClientFailureLeavesLibraryUntouched: a generation the companion +// fails leaves the library byte for byte as it was (#1163) — its generated +// bindings and facade, the contract it records, and the developer's own +// modules and tests beside them. It used to wipe the generated tree, and +// before that every module at the package root, before the companion ran. +func TestGenerateClientFailureLeavesLibraryUntouched(t *testing.T) { + requireQualify(t) + output := filepath.Join(t.TempDir(), "lib") + if err := generatePythonLibrary(output, localProtoEntry(t, validatedAccountsProto)); err != nil { + t.Fatalf("first generation: %v", err) + } + pythonDir := filepath.Join(output, "python") + for path, content := range map[string]string{ + filepath.Join(pythonDir, "accounts_client", "work_context.py"): "def stamp(ctx):\n return ctx\n", + filepath.Join(pythonDir, "tests", "test_work_context.py"): "def test_stamp():\n pass\n", + } { + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + } + before := hashTree(t, output) + + // The descriptor compiles, and the sources the companion is handed import + // a file nothing provides: generation fails inside the companion, after + // the contract is known, which is where a wipe-first run lost the tree. + broken := localProtoEntry(t, validatedAccountsProto) + source := &broken.ProtoSources[0] + source.Content = []byte(strings.Replace(string(source.Content), + `import "buf/validate/validate.proto";`, + `import "buf/validate/validate.proto";`+"\nimport \"absent/nowhere.proto\";", 1)) + if err := generatePythonLibrary(output, broken); err == nil { + t.Fatal("generating from sources importing a missing file succeeded") + } + if after := hashTree(t, output); after != before { + t.Fatal("a failed generation changed the library") + } + assertFileContains(t, filepath.Join(pythonDir, "accounts_client", "work_context.py"), "def stamp") +} + +// validatedAccountsProto validates a field with buf/validate, as the +// authority interface's protos do. +const validatedAccountsProto = `syntax = "proto3"; + +package accounts.v1; + +import "buf/validate/validate.proto"; + +message Account { + string id = 1 [(buf.validate.field).string.min_len = 1]; +} + +service Accounts { + rpc Get(Account) returns (Account); +} +` + +// localProtoEntry builds the contract entry --from module/service builds for +// a live service, through the same buildLocalContractEndpoint, for a service +// whose proto/ holds accounts/v1/accounts.proto with content and a buf.yaml +// declaring the shared modules it imports. +func localProtoEntry(t *testing.T, content string) generators.ContractEntry { + t.Helper() + ctx := context.Background() + serviceDir := filepath.Join(t.TempDir(), "api") + protoDir := filepath.Join(serviceDir, "proto") + if err := os.MkdirAll(filepath.Join(protoDir, "accounts", "v1"), 0o755); err != nil { + t.Fatal(err) + } + bufYAML := "version: v1\ndeps:\n - buf.build/googleapis/googleapis\n - buf.build/bufbuild/protovalidate\n" + if err := os.WriteFile(filepath.Join(protoDir, "buf.yaml"), []byte(bufYAML), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(protoDir, "accounts", "v1", "accounts.proto"), []byte(content), 0o644); err != nil { + t.Fatal(err) + } + if err := (&resources.Service{Name: "api", Version: "0.0.1"}).SaveAtDir(ctx, serviceDir); err != nil { + t.Fatal(err) + } + service, err := resources.LoadServiceFromDir(ctx, serviceDir) + if err != nil { + t.Fatal(err) + } + endpoint := &basev0.Endpoint{ + Name: "grpc", Service: "api", Api: standards.GRPC, + ApiDetails: &basev0.API{Value: &basev0.API_Grpc{Grpc: &basev0.GrpcAPI{}}}, + } + descriptorSet, contract, sources, err := buildLocalContractEndpoint(ctx, service, endpoint) + if err != nil { + t.Fatalf("buildLocalContractEndpoint: %v", err) + } + return generators.ContractEntry{ + Endpoint: *contract, + ContractBytes: descriptorSet, + PackageID: "local", + PackageVersion: "0.0.1", + ModuleName: "accounts", + FacadeModuleDefault: generators.DefaultFacadeModuleName(contract), + ProtoSources: sources, + } +} + +// generatePythonLibrary generates entry's Python bindings, no facade, into +// output as the accounts-client library. +func generatePythonLibrary(output string, entry generators.ContractEntry) error { + return generators.GenerateClientLibrary(context.Background(), &generators.ClientLibraryRequest{ + Name: "accounts-client", + Output: output, + Languages: []languages.Language{languages.PYTHON}, + Entries: []generators.ContractEntry{entry}, + NoFacade: true, + Force: true, + }) } // writeUpstreamModulesContractsFixture compiles a proto importing the @@ -765,6 +920,17 @@ func assertFileContains(t *testing.T, path, substr string) { } } +func assertFileLacks(t *testing.T, path, substr string) { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read %s: %v", path, err) + } + if strings.Contains(string(data), substr) { + t.Errorf("%s contains %q", path, substr) + } +} + func goVetLibrary(t *testing.T, goDir string) { t.Helper() cmd := exec.Command("go", "vet", "./...") @@ -789,7 +955,7 @@ func assertPythonImports(t *testing.T, pythonDir, dottedModule string) { t.Fatalf("CompanionImage: %v", err) } cmd := exec.CommandContext(ctx, "docker", "run", "--rm", - "-v", pythonDir+":/out", "-w", "/out", image.Name+":"+image.Tag, + "-v", pythonDir+":/out", "-w", "/out", image.FullName(), "/venv/bin/python", "-c", "import "+dottedModule) if out, err := cmd.CombinedOutput(); err != nil { t.Fatalf("import %s failed: %v\n%s", dottedModule, err, out) diff --git a/docs/commands.md b/docs/commands.md index e6188cae..9234d87a 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -3425,6 +3425,17 @@ from. `library.codefly.yaml` always carries a `sources:` list (one element for `generate client`, one per aggregated contract for `sync solution-sdk` below) so both commands share one schema. +Every requested language is generated into scratch directories before anything +under the library is written. A run that fails in the proto companion (a +contract that does not compile, a buf dependency that does not resolve) leaves +the library exactly as it was: its contract, its generated tree and the files +written by hand beside them. Once every language has generated, the command +replaces only what it owns: `gen/`, `src/` and `_gen/`, and each facade (a Python +facade is recognised by its generated-code notice). Scaffold files are written +only when missing, except that a Python `pyproject.toml` gains any dependency its +bindings now need. A Go library is compiled after it is written, so a library +that generates but does not build is left in place with the build error reported. + `--from module/service[/endpoint]` loads the service's Builder to build a one-entry contract in memory (the same way `generate contracts` does) and generates from the service's own proto sources. `--from package:`/`contracts:` diff --git a/pkg/generators/client.go b/pkg/generators/client.go index f8dd56f9..49c246af 100644 --- a/pkg/generators/client.go +++ b/pkg/generators/client.go @@ -160,23 +160,44 @@ func GenerateClientLibrary(ctx context.Context, req *ClientLibraryRequest) error } } - if err := writeContractFiles(ctx, req.Output, req.Entries); err != nil { - return fmt.Errorf("cannot write contract: %w", err) - } - pyPackage := strings.ReplaceAll(req.Name, "-", "_") - var langExports []generatedLanguageExport + // Every language is generated before anything under req.Output is + // touched. The companion runs are what fail — a contract that does not + // compile, a buf dependency that does not resolve, an image that is not + // there — and they write only into scratch directories; the library is + // rewritten from those once all of them have succeeded. A failed run + // leaves the library exactly as it found it, generated files and + // hand-written ones alike, as `generate proto` does with its staging tree. + var staged []*stagedLanguage + defer func() { + for _, s := range staged { + s.remove() + } + }() for _, lang := range req.Languages { if err := ctx.Err(); err != nil { return err } - export, genErr := generateLanguageForEntries(ctx, lang, filepath.Join(req.Output, string(lang)), req.Entries, !req.NoFacade, req.GoModule, req.NpmPackage, pyPackage) + s, genErr := stageLanguage(ctx, lang, filepath.Join(req.Output, string(lang)), req.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 { + return fmt.Errorf("cannot write contract: %w", err) + } + + var langExports []generatedLanguageExport + for _, s := range staged { + export, applyErr := s.apply(req.GoModule, req.NpmPackage, pyPackage) + if applyErr != nil { + return fmt.Errorf("cannot generate %s client: %w", s.lang, applyErr) + } langExports = append(langExports, export) - cli.Info("generated %s client at %s", lang, filepath.Join(req.Output, string(lang))) + cli.Info("generated %s client at %s", s.lang, s.langDir) } langExports = mergeLanguageExports(existingLanguages, langExports, req.Languages) @@ -229,72 +250,134 @@ func checkContractDiamond(entries []ContractEntry) error { return nil } -// generateLanguageForEntries generates one language's export for the library. -// An OpenAPI contract has no facade to aggregate and is only ever generated -// alone (GenerateClientLibrary refuses to mix it with other entries). -func generateLanguageForEntries(ctx context.Context, lang languages.Language, langDir string, entries []ContractEntry, facade bool, goModule, npmPackage, pyPackage string) (generatedLanguageExport, error) { - if err := os.MkdirAll(langDir, 0o755); err != nil { - return generatedLanguageExport{}, err +// stagedLanguage is one language's generated output, held in scratch +// directories until every language of the run has generated, then applied +// to langDir. An OpenAPI contract has no facade to aggregate and is only ever +// generated alone (GenerateClientLibrary refuses to mix it with other +// entries), so a language holds either protobuf entries or one OpenAPI tree. +type stagedLanguage struct { + lang languages.Language + langDir string + entries []stagedEntry + openAPI string +} + +// stagedEntry is one protobuf contract entry generated into raw, with the +// upstream proto modules it dropped from the output, which the library's own +// dependency declaration must cover. +type stagedEntry struct { + raw string + moduleName string + mapped []upstreamProtoModule +} + +func (s *stagedLanguage) remove() { + for _, entry := range s.entries { + _ = os.RemoveAll(entry.raw) } - if len(entries) == 1 && entries[0].Endpoint.Kind == composition.APIContractKindOpenAPI { - return generateOpenAPILanguageEntry(ctx, lang, langDir, &entries[0], goModule, npmPackage) + if s.openAPI != "" { + _ = os.RemoveAll(s.openAPI) } - return generateProtobufLanguageEntries(ctx, lang, langDir, entries, facade, goModule, npmPackage, pyPackage) } -// generateProtobufLanguageEntries wipes langDir's generated content once, -// then runs proto.GenerateClient once per entry and merges each result in — -// so N entries land in one gen/ tree with one facade file per entry, sharing -// one scaffold. -func generateProtobufLanguageEntries(ctx context.Context, lang languages.Language, langDir string, entries []ContractEntry, facade bool, goModule, npmPackage, pyPackage string) (generatedLanguageExport, error) { +// stageLanguage runs the companion for every entry of one language, writing +// only into scratch directories. On error it leaves nothing behind. +func stageLanguage(ctx context.Context, lang languages.Language, langDir string, entries []ContractEntry, facade bool, goModule string) (_ *stagedLanguage, err error) { + staged := &stagedLanguage{lang: lang, langDir: langDir} + defer func() { + if err != nil { + staged.remove() + } + }() + if len(entries) == 1 && entries[0].Endpoint.Kind == composition.APIContractKindOpenAPI { + staged.openAPI, err = stageOpenAPIEntry(ctx, lang, langDir, &entries[0], goModule) + return staged, err + } switch lang { - case languages.GO: - if err := wipeGoLibrary(langDir); err != nil { - return generatedLanguageExport{}, err + case languages.GO, languages.TYPESCRIPT, languages.PYTHON: + default: + return nil, fmt.Errorf("language %s is not supported for client generation", lang) + } + for i := range entries { + entry := &entries[i] + generated, genErr := stageProtobufEntry(ctx, lang, entry, facade) + if genErr != nil { + return nil, fmt.Errorf("entry %s/%s/%s: %w", entry.ModuleName, entry.Endpoint.Service, entry.Endpoint.Endpoint, genErr) } + staged.entries = append(staged.entries, generated) + } + return staged, nil +} + +// apply rewrites langDir from the staged output: it wipes langDir's generated +// content once, merges each entry in — so N entries land in one gen/ tree +// with one facade file per entry, sharing one scaffold — and finalizes the +// scaffold. What the wipe spares (hand-written files, a hand-edited +// scaffold) is the wipe's own rule; staging only decides when it runs. +func (s *stagedLanguage) apply(goModule, npmPackage, pyPackage string) (generatedLanguageExport, error) { + if err := os.MkdirAll(s.langDir, 0o755); err != nil { + return generatedLanguageExport{}, err + } + if s.openAPI != "" { + return applyOpenAPILibrary(s.lang, s.langDir, s.openAPI, goModule, npmPackage) + } + pkgDir := filepath.Join(s.langDir, pyPackage) + var wipeErr error + switch s.lang { + case languages.GO: + wipeErr = wipeGoLibrary(s.langDir) case languages.TYPESCRIPT: - if err := wipeTypeScriptLibrary(langDir); err != nil { - return generatedLanguageExport{}, err - } + wipeErr = wipeTypeScriptLibrary(s.langDir) case languages.PYTHON: - if err := wipePythonLibrary(filepath.Join(langDir, pyPackage)); err != nil { - return generatedLanguageExport{}, err - } - default: - return generatedLanguageExport{}, fmt.Errorf("language %s is not supported for client generation", lang) + wipeErr = wipePythonLibrary(pkgDir) + } + if wipeErr != nil { + return generatedLanguageExport{}, wipeErr } var mapped []upstreamProtoModule - for i := range entries { - entry := &entries[i] - marked, err := generateOneProtobufEntry(ctx, lang, langDir, entry, facade, goModule, pyPackage) - if err != nil { - return generatedLanguageExport{}, fmt.Errorf("entry %s/%s/%s: %w", entry.ModuleName, entry.Endpoint.Service, entry.Endpoint.Endpoint, err) + for _, entry := range s.entries { + var mergeErr error + switch s.lang { + case languages.GO: + mergeErr = mergeGoEntry(s.langDir, entry.raw, entry.moduleName, goModule) + case languages.TYPESCRIPT: + mergeErr = mergeTypeScriptEntry(s.langDir, entry.raw) + case languages.PYTHON: + mergeErr = mergePythonEntry(pkgDir, entry.raw, entry.moduleName, pyPackage) } - mapped = append(mapped, marked...) + if mergeErr != nil { + return generatedLanguageExport{}, mergeErr + } + mapped = append(mapped, entry.mapped...) } - switch lang { + switch s.lang { case languages.GO: - return finalizeGoLibrary(langDir, goModule) + return finalizeGoLibrary(s.langDir, goModule) case languages.TYPESCRIPT: - return finalizeTypeScriptLibrary(langDir, npmPackage) - case languages.PYTHON: - return finalizePythonLibrary(langDir, pyPackage, pythonPackagesFor(mapped)) + return finalizeTypeScriptLibrary(s.langDir, npmPackage) default: - return generatedLanguageExport{}, fmt.Errorf("language %s is not supported for client generation", lang) + return finalizePythonLibrary(s.langDir, pyPackage, pythonPackagesFor(mapped)) } } -// generateOneProtobufEntry generates one contract entry into langDir and -// reports the upstream proto modules it dropped from the output, which the -// library's own dependency declaration must now cover. -func generateOneProtobufEntry(ctx context.Context, lang languages.Language, langDir string, entry *ContractEntry, facade bool, goModule, pyPackage string) ([]upstreamProtoModule, error) { +// 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) { + var set descriptorpb.FileDescriptorSet + if err = googleproto.Unmarshal(entry.ContractBytes, &set); err != nil { + return stagedEntry{}, fmt.Errorf("cannot parse contract descriptor set: %w", err) + } raw, err := os.MkdirTemp("", "codefly-client-raw") if err != nil { - return nil, err + return stagedEntry{}, err } - defer func() { _ = os.RemoveAll(raw) }() + defer func() { + if err != nil { + _ = os.RemoveAll(raw) + } + }() moduleName := entryModuleName(entry) req := coreproto.ClientRequest{ @@ -305,10 +388,6 @@ func generateOneProtobufEntry(ctx context.Context, lang languages.Language, lang Facade: facade, GoRequires: linkedGoBindings(), } - var set descriptorpb.FileDescriptorSet - if err := googleproto.Unmarshal(entry.ContractBytes, &set); err != nil { - return nil, fmt.Errorf("cannot parse contract descriptor set: %w", err) - } var mapped []upstreamProtoModule if len(entry.ProtoSources) > 0 && !vendorsUpstreamModules(entry.ProtoSources, &set, lang) { // Generated from sources, buf compiles only the entry's own files: the @@ -322,28 +401,18 @@ func generateOneProtobufEntry(ctx context.Context, lang languages.Language, lang req.Sources = entry.ProtoSources mapped = upstreamModulesIn(&set, lang) } else { - image, marked, err := markUpstreamModulesAsImports(&set, lang) - if err != nil { - return nil, fmt.Errorf("cannot prepare contract descriptor set: %w", err) + image, marked, markErr := markUpstreamModulesAsImports(&set, lang) + if markErr != nil { + return stagedEntry{}, fmt.Errorf("cannot prepare contract descriptor set: %w", markErr) } req.DescriptorSet = image req.TargetFiles = targetFilesForPackage(&set, entry.Endpoint.Package) mapped = marked } - if err := coreproto.GenerateClient(ctx, req); err != nil { - return nil, err - } - - switch lang { - case languages.GO: - return mapped, mergeGoEntry(langDir, raw, moduleName, goModule) - case languages.TYPESCRIPT: - return mapped, mergeTypeScriptEntry(langDir, raw) - case languages.PYTHON: - return mapped, mergePythonEntry(filepath.Join(langDir, pyPackage), raw, moduleName, pyPackage) - default: - return nil, fmt.Errorf("language %s is not supported for client generation", lang) + if err = coreproto.GenerateClient(ctx, req); err != nil { + return stagedEntry{}, err } + return stagedEntry{raw: raw, moduleName: moduleName, mapped: mapped}, nil } // entryModuleName is the facade entry-point name proto.GenerateClient uses @@ -359,14 +428,6 @@ func entryModuleName(entry *ContractEntry) string { return defaultFacadeModule } -// generateOpenAPILanguageEntry synthesizes a basev0.Endpoint carrying the raw -// OpenAPI document (the only thing proto.GenerateOpenAPI reads) and runs it. -// OpenAPI clients have no facade: proto.GenerateOpenAPI's contract-derived -// codegen is a plain typed HTTP client. -func generateOpenAPILanguageEntry(ctx context.Context, lang languages.Language, langDir string, entry *ContractEntry, goModule, npmPackage string) (generatedLanguageExport, error) { - return assembleOpenAPILibrary(ctx, lang, langDir, entry, goModule, npmPackage) -} - // upstreamProtoModule is a shared proto module a persisted contract carries // among its imports and that already ships canonical bindings of its own. // diff --git a/pkg/generators/client_library.go b/pkg/generators/client_library.go index e04d9db7..394a9b4c 100644 --- a/pkg/generators/client_library.go +++ b/pkg/generators/client_library.go @@ -354,21 +354,40 @@ func pythonDistribution(requirement string) string { return strings.ToLower(strings.ReplaceAll(strings.TrimSpace(name), "_", "-")) } -// assembleOpenAPILibrary synthesizes a basev0.Endpoint carrying the raw -// OpenAPI document (the only thing proto.GenerateOpenAPI reads) and runs it. -// OpenAPI clients have no facade: proto.GenerateOpenAPI's contract-derived -// codegen is a plain typed HTTP client. -func assembleOpenAPILibrary(ctx context.Context, lang languages.Language, langDir string, entry *ContractEntry, goModule, npmPackage string) (generatedLanguageExport, error) { +// stageOpenAPIEntry synthesizes a basev0.Endpoint carrying the raw OpenAPI +// document (the only thing proto.GenerateOpenAPI reads) and generates it into +// a scratch directory laid out as the library's language directory: gen/, and +// for Go the go.mod go-swagger derives its import paths from — the library's +// own when it has one, the scaffold's otherwise — so the staged tree imports +// exactly what generating in place would have. OpenAPI clients have no +// facade: proto.GenerateOpenAPI's contract-derived codegen is a plain typed +// HTTP client. +func stageOpenAPIEntry(ctx context.Context, lang languages.Language, langDir string, entry *ContractEntry, goModule string) (_ string, err error) { if lang != languages.GO && lang != languages.TYPESCRIPT { - return generatedLanguageExport{}, fmt.Errorf("openapi clients support go or typescript, not %s", lang) + return "", fmt.Errorf("openapi clients support go or typescript, not %s", lang) } - - genDir := filepath.Join(langDir, "gen") - if err := os.RemoveAll(genDir); err != nil { - return generatedLanguageExport{}, err + raw, err := os.MkdirTemp("", "codefly-client-openapi") + if err != nil { + return "", err } - if err := os.MkdirAll(genDir, 0o755); err != nil { - return generatedLanguageExport{}, err + defer func() { + if err != nil { + _ = os.RemoveAll(raw) + } + }() + + if lang == languages.GO { + existing := filepath.Join(langDir, "go.mod") + if _, statErr := os.Stat(existing); statErr == nil { + if err = copyFile(existing, filepath.Join(raw, "go.mod")); err != nil { + return "", err + } + } else if !os.IsNotExist(statErr) { + return "", statErr + } + if err = writeGoModule(raw, goModule); err != nil { + return "", err + } } endpoint := &basev0.Endpoint{ @@ -380,23 +399,31 @@ func assembleOpenAPILibrary(ctx context.Context, lang languages.Language, langDi Value: &basev0.API_Rest{Rest: &basev0.RestAPI{Openapi: entry.ContractBytes}}, }, } + if err = coreproto.GenerateOpenAPI(ctx, lang, filepath.Join(raw, "gen"), "", endpoint); err != nil { + return "", err + } + return raw, nil +} + +// applyOpenAPILibrary replaces langDir's gen/ with the staged one and writes +// the scaffold where it is missing. +func applyOpenAPILibrary(lang languages.Language, langDir, raw, goModule, npmPackage string) (generatedLanguageExport, error) { + genDir := filepath.Join(langDir, "gen") + if err := os.RemoveAll(genDir); err != nil { + return generatedLanguageExport{}, err + } + if err := copyTree(filepath.Join(raw, "gen"), genDir); err != nil { + return generatedLanguageExport{}, err + } if lang == languages.GO { // go.mod lives at langDir, not genDir: genDir is fully regenerated // every run (it is proto.GenerateOpenAPI's own output), but go.mod is // a scaffold file a developer may hand-edit (an added dependency, a - // replace directive) and findModRoot walks up from genDir to find it - // either way, so placing it one level up costs nothing and makes it - // write-once like the protobuf path's go.mod. + // replace directive), so it is write-once like the protobuf path's. if err := writeGoModule(langDir, goModule); err != nil { return generatedLanguageExport{}, err } - } - if err := coreproto.GenerateOpenAPI(ctx, lang, genDir, "", endpoint); err != nil { - return generatedLanguageExport{}, err - } - - if lang == languages.GO { bestEffortGoModTidy(langDir) return generatedLanguageExport{Name: "go", Path: "go/", Exports: []string{goModule}}, nil } @@ -637,6 +664,20 @@ func copyFile(src, dst string) error { return err } +// copyTree copies every file under src to the same relative path under dst. +func copyTree(src, dst string) error { + return filepath.WalkDir(src, func(p string, d os.DirEntry, err error) error { + if err != nil || d.IsDir() { + return err + } + rel, err := filepath.Rel(src, p) + if err != nil { + return err + } + return copyFile(p, filepath.Join(dst, rel)) + }) +} + // removeMatching removes every file directly in dir (non-recursive) whose // basename matches pattern, except any name listed in keep. Used to clear a // stale generated artifact (e.g. a facade file left over from a diff --git a/pkg/generators/python_ownership_test.go b/pkg/generators/python_ownership_test.go index 5641682d..e6a2e772 100644 --- a/pkg/generators/python_ownership_test.go +++ b/pkg/generators/python_ownership_test.go @@ -1,9 +1,15 @@ package generators import ( + "context" + "io/fs" "os" "path/filepath" + "strings" "testing" + + "github.com/codefly-dev/core/composition" + "github.com/codefly-dev/core/languages" ) // pythonFacade is a facade as the Python facade plugin writes it: its module @@ -25,9 +31,10 @@ def ` + module + `(gateway): ` } -// regeneratePython runs one Python library generation over pkgDir the way -// generateProtobufLanguageEntries does — wipe once, merge the entry, finalize -// — from a raw plugin output holding the facade and one binding. +// regeneratePython applies one staged Python generation to langDir — wipe +// once, merge the entry, finalize — from a raw plugin output holding the +// facade and one binding, as GenerateClientLibrary does once the companion +// has succeeded. func regeneratePython(t *testing.T, langDir, module string) { t.Helper() raw := t.TempDir() @@ -37,15 +44,9 @@ func regeneratePython(t *testing.T, langDir, module string) { } { writeTestFile(t, path, content) } - pkgDir := filepath.Join(langDir, "acme_pdf") - if err := wipePythonLibrary(pkgDir); err != nil { - t.Fatalf("wipePythonLibrary: %v", err) - } - if err := mergePythonEntry(pkgDir, raw, module, "acme_pdf"); err != nil { - t.Fatalf("mergePythonEntry: %v", err) - } - if _, err := finalizePythonLibrary(langDir, "acme_pdf", nil); err != nil { - t.Fatalf("finalizePythonLibrary: %v", err) + staged := &stagedLanguage{lang: languages.PYTHON, langDir: langDir, entries: []stagedEntry{{raw: raw, moduleName: module}}} + if _, err := staged.apply("", "", "acme_pdf"); err != nil { + t.Fatalf("apply: %v", err) } } @@ -129,3 +130,78 @@ func TestAPythonNoticeIsReadFromTheLeadingBlockOnly(t *testing.T) { } } } + +// TestAFailedGenerationLeavesTheLibraryUntouched: nothing under the library is +// written until every language has generated, so a run that fails leaves the +// library byte for byte as it was — the contract it records, the generated +// facade and bindings, and the developer's own files beside them. It used to +// rewrite the contract and wipe the generated tree before the companion ran, +// so a failed run left a library with no bindings (and, before the package +// root's ownership was read from notices, no hand-written modules either). +func TestAFailedGenerationLeavesTheLibraryUntouched(t *testing.T) { + output := t.TempDir() + langDir := filepath.Join(output, "python") + pkgDir := filepath.Join(langDir, "acme_pdf") + for path, content := range map[string]string{ + filepath.Join(output, "contract", "contract.binpb"): "the contract the library was generated from", + filepath.Join(pkgDir, "pdf.py"): pythonFacade("pdf"), + filepath.Join(pkgDir, "_gen", "pdf", "v1", "api_pb2.py"): "# Generated by the protocol buffer compiler. DO NOT EDIT!\n", + filepath.Join(pkgDir, "reader.py"): "from acme_pdf.pdf import pdf\n", + filepath.Join(pkgDir, "__init__.py"): "from acme_pdf.reader import read\n", + filepath.Join(langDir, "tests", "test_reader.py"): "def test_read():\n pass\n", + filepath.Join(langDir, "pyproject.toml"): "[project]\nname = \"acme-pdf\"\ndependencies = [\"protobuf\"]\n", + } { + writeTestFile(t, path, content) + } + before := snapshotTree(t, output) + + err := GenerateClientLibrary(context.Background(), &ClientLibraryRequest{ + Name: "acme-pdf", + Output: output, + Languages: []languages.Language{languages.PYTHON}, + Force: true, + Entries: []ContractEntry{{ + Endpoint: composition.APIContractEndpoint{ + Service: "api", Endpoint: "grpc", Kind: composition.APIContractKindProtobuf, Package: "pdf.v1", + }, + // Not a descriptor set: generation fails before any companion runs. + ContractBytes: []byte{0xff}, + }}, + }) + if err == nil { + t.Fatal("generating from a contract that does not parse succeeded") + } + if after := snapshotTree(t, output); after != before { + t.Fatalf("a failed generation changed the library:\nbefore:\n%s\nafter:\n%s", before, after) + } +} + +// snapshotTree renders every file under root as its path, mode and content. +func snapshotTree(t *testing.T, root string) string { + t.Helper() + var b strings.Builder + err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + info, err := d.Info() + if err != nil { + return err + } + rel, _ := filepath.Rel(root, path) + b.WriteString(rel + " " + info.Mode().String()) + if !d.IsDir() { + content, err := os.ReadFile(path) + if err != nil { + return err + } + b.WriteString(" " + string(content)) + } + b.WriteString("\n") + return nil + }) + if err != nil { + t.Fatal(err) + } + return b.String() +}