From b5f6c99389ed24d55fb1e3e43286a42cb25f93c1 Mon Sep 17 00:00:00 2001 From: Antoine Toussaint <312536060+antoinetoussaint-byte@users.noreply.github.com> Date: Wed, 21 Oct 2026 10:22:25 -0400 Subject: [PATCH] fix(sync): solution-sdk generates a working client for a solution consuming modules through the host gateway (#1094) Four gaps stopped `codefly sync solution-sdk` on any solution that names no executor and consumes modules over the transports the host gateway serves: - The manifest is read with core's LoadConsumption, the loader core keeps for a reader of api.consumes alone: a solution with no `agent` block is no longer refused for lacking a codefly:solution executor nothing runs. - A consume of a connect or rest endpoint resolves to its service's protobuf contract, which a catalog exports under grpc. - Only a consume of the solution's own module asks for (or, with --apply-dependencies, gets) a service-dependencies edge; another module is reached through the host gateway, and a direct edge is the mesh path a solution must not hold. - The aggregated library gives each facade a package of its own (Go go// with bindings under gen//; TypeScript src// exported as /, the root re-exporting the facades alone), and refuses two entries generating one facade name. Covered by a fixture in cmd/sync/testdata/solution-sdk and a test per gap; the Docker end-to-end builds the Go half, writes the TypeScript half and passes --check. Co-Authored-By: Claude Opus 5.5 (1M context) --- cmd/sync/solution_sdk.go | 134 ++++++++-- cmd/sync/solution_sdk_fixture_test.go | 238 ++++++++++++++++++ cmd/sync/solution_sdk_test.go | 50 +++- .../testdata/solution-sdk/module.codefly.yaml | 5 + .../modules/accounts/module.codefly.yaml | 4 + .../accounts/module.package.codefly.yaml | 10 + .../services/accounts/proto/accounts.proto | 11 + .../services/accounts/service.codefly.yaml | 16 ++ .../modules/records/module.codefly.yaml | 4 + .../records/module.package.codefly.yaml | 10 + .../services/records/proto/records.proto | 9 + .../services/records/service.codefly.yaml | 16 ++ .../services/backend/service.codefly.yaml | 13 + .../solution-sdk/solution.codefly.yaml | 18 ++ .../solution-sdk/workspace.codefly.yaml | 9 + docs/cli-reference.md | 11 +- docs/commands.md | 52 +++- docs/design/library-system.md | 13 +- pkg/generators/client.go | 52 +++- pkg/generators/client_library.go | 182 ++++++++++---- pkg/generators/client_library_test.go | 4 +- pkg/generators/facade_packages_test.go | 189 ++++++++++++++ 22 files changed, 938 insertions(+), 112 deletions(-) create mode 100644 cmd/sync/solution_sdk_fixture_test.go create mode 100644 cmd/sync/testdata/solution-sdk/module.codefly.yaml create mode 100644 cmd/sync/testdata/solution-sdk/modules/accounts/module.codefly.yaml create mode 100644 cmd/sync/testdata/solution-sdk/modules/accounts/module.package.codefly.yaml create mode 100644 cmd/sync/testdata/solution-sdk/modules/accounts/services/accounts/proto/accounts.proto create mode 100644 cmd/sync/testdata/solution-sdk/modules/accounts/services/accounts/service.codefly.yaml create mode 100644 cmd/sync/testdata/solution-sdk/modules/records/module.codefly.yaml create mode 100644 cmd/sync/testdata/solution-sdk/modules/records/module.package.codefly.yaml create mode 100644 cmd/sync/testdata/solution-sdk/modules/records/services/records/proto/records.proto create mode 100644 cmd/sync/testdata/solution-sdk/modules/records/services/records/service.codefly.yaml create mode 100644 cmd/sync/testdata/solution-sdk/services/backend/service.codefly.yaml create mode 100644 cmd/sync/testdata/solution-sdk/solution.codefly.yaml create mode 100644 cmd/sync/testdata/solution-sdk/workspace.codefly.yaml create mode 100644 pkg/generators/facade_packages_test.go diff --git a/cmd/sync/solution_sdk.go b/cmd/sync/solution_sdk.go index 87bde8942..989147e82 100644 --- a/cmd/sync/solution_sdk.go +++ b/cmd/sync/solution_sdk.go @@ -19,6 +19,7 @@ import ( resources "github.com/codefly-dev/core/resources" "github.com/codefly-dev/core/shared" "github.com/codefly-dev/core/solution/manifest" + "github.com/codefly-dev/core/standards" "github.com/spf13/cobra" "gopkg.in/yaml.v3" ) @@ -43,8 +44,13 @@ var SolutionSDKCmd = &cobra.Command{ Short: "Aggregate one client library per solution from api.consumes", Long: `Reads api.consumes in solution.codefly.yaml and produces one aggregated client library per solution, carrying only the declared services, generated -from the contracts of the module packages the workspace composes. Also keeps -the runtime service-dependencies in step with the declaration. +from the contracts of the module packages the workspace composes. A consume +of a module's rest or connect endpoint is generated from that service's +protobuf contract. Each consumed module's facade gets a package of its own. + +A consume of another module is reached through the host's gateway and needs +no service-dependencies entry; only a consume of the solution's own module is +a direct call, and that one is kept in step with the declaration. `, Args: cobra.NoArgs, RunE: func(_ *cobra.Command, _ []string) error { @@ -59,7 +65,7 @@ the runtime service-dependencies in step with the declaration. 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(&solutionSDKApply, "apply-dependencies", false, "add missing service-dependencies entries for consumes of the solution's own module (other modules are reached through the host gateway)") } func runSyncSolutionSDK(ctx context.Context) error { @@ -75,7 +81,11 @@ func runSyncSolutionSDK(ctx context.Context) error { if err != nil { return fmt.Errorf("cannot read %s: %w", manifest.FileName, err) } - solutionManifest, err := manifest.Load(data) + // This command reads api.consumes and nothing else, so it reads the + // manifest the way core says such a reader must: LoadConsumption holds the + // consumption strictly and leaves the executor sections (agent, lifecycle) + // unread, so a solution with no executor is not refused for lacking one. + solutionManifest, err := manifest.LoadConsumption(data) if err != nil { return fmt.Errorf("cannot load %s: %w", manifest.FileName, err) } @@ -94,7 +104,7 @@ func runSyncSolutionSDK(ctx context.Context) error { return err } - missing, err := missingServiceDependencies(entryService, entries) + missing, err := missingServiceDependencies(entryModule, entryService, solutionManifest.API.Consumes) if err != nil { return err } @@ -176,15 +186,9 @@ func resolveConsumedContracts(ctx context.Context, workspace *resources.Workspac } } - var endpointEntry *composition.APIContractEndpoint - for i := range catalog.Endpoints { - if catalog.Endpoints[i].Service == consume.Service && catalog.Endpoints[i].Endpoint == consume.Endpoint { - endpointEntry = &catalog.Endpoints[i] - break - } - } - if endpointEntry == nil { - return nil, fmt.Errorf("module %s catalog has no endpoint %s/%s", consume.Module, consume.Service, consume.Endpoint) + endpointEntry, err := consumedContract(ctx, dir, catalog, consume) + if err != nil { + return nil, err } if len(consume.Services) > 0 { @@ -222,6 +226,82 @@ func resolveConsumedContracts(ctx context.Context, workspace *resources.Workspac return entries, nil } +// consumedContract selects the catalog entry a consume is generated from. The +// endpoint it names wins when the catalog carries a protobuf contract for it. +// A rest or connect endpoint otherwise serves its service's protobuf contract +// over another transport — connect natively, rest transcoded — while the +// catalog carries that contract under the service's grpc endpoint, so the +// consume resolves to the service's one protobuf entry. A rest endpoint of a +// service with no protobuf contract keeps its own OpenAPI entry. +func consumedContract(ctx context.Context, moduleDir string, catalog *composition.APIContractCatalog, consume *manifest.APIDeclaration) (*composition.APIContractEndpoint, error) { + var named *composition.APIContractEndpoint + var protobuf []*composition.APIContractEndpoint + for i := range catalog.Endpoints { + entry := &catalog.Endpoints[i] + if entry.Service != consume.Service { + continue + } + if entry.Endpoint == consume.Endpoint { + named = entry + } + if entry.Kind == composition.APIContractKindProtobuf { + protobuf = append(protobuf, entry) + } + } + if named != nil && named.Kind == composition.APIContractKindProtobuf { + return named, nil + } + + api := "" + if named != nil { + api = named.API + } else { + declared, err := producerEndpointAPI(ctx, moduleDir, consume) + if err != nil { + return nil, err + } + api = declared + } + if api == standards.REST || api == standards.CONNECT { + switch len(protobuf) { + case 0: + case 1: + return protobuf[0], nil + default: + endpoints := make([]string, 0, len(protobuf)) + for _, entry := range protobuf { + endpoints = append(endpoints, entry.Endpoint) + } + return nil, fmt.Errorf("consume %s/%s/%s: service %s exports protobuf contracts on several endpoints (%s), so which one its %s endpoint serves has no single answer; consume one of them by name", + consume.Module, consume.Service, consume.Endpoint, consume.Service, strings.Join(endpoints, ", "), api) + } + } + if named != nil { + return named, nil + } + return nil, fmt.Errorf("module %s catalog has no contract for endpoint %s/%s (api %s)", consume.Module, consume.Service, consume.Endpoint, api) +} + +// producerEndpointAPI reads the API of the endpoint a consume names from the +// producing service's own declaration, for an endpoint the catalog carries no +// contract for. +func producerEndpointAPI(ctx context.Context, moduleDir string, consume *manifest.APIDeclaration) (string, error) { + module, err := resources.LoadModuleFromDir(ctx, moduleDir) + if err != nil { + return "", fmt.Errorf("cannot load module %s: %w", consume.Module, err) + } + service, err := module.LoadServiceFromName(ctx, consume.Service) + if err != nil { + return "", fmt.Errorf("consume %s/%s/%s: module %s has no service %s: %w", consume.Module, consume.Service, consume.Endpoint, consume.Module, consume.Service, err) + } + for _, endpoint := range service.Endpoints { + if endpoint != nil && endpoint.Name == consume.Endpoint { + return endpoint.API, nil + } + } + return "", fmt.Errorf("consume %s/%s/%s: service %s/%s declares no endpoint %s", consume.Module, consume.Service, consume.Endpoint, consume.Module, consume.Service, consume.Endpoint) +} + // resolveSolutionEntryService finds the solution root — the workspace's own // module (the `path: .` self module, or, failing an explicit path, the // module whose name matches the workspace) — and its service-entry. @@ -266,21 +346,28 @@ type missingDependency struct { Module, Service, Endpoint string } -// missingServiceDependencies reports which consumed entries have no matching -// service-dependencies item, matched by (name, module) — the same identity -// granularity core's own Application.HasServiceDependency uses. -func missingServiceDependencies(service *resources.Service, entries []generators.ContractEntry) ([]missingDependency, error) { +// missingServiceDependencies reports which consumed entries the entry service +// calls directly and declares no service-dependencies item for, matched by +// (name, module) — the same identity granularity core's own +// Application.HasServiceDependency uses. +// +// Only a consume of the solution's own module is a direct call. A consume of +// any other module is reached through the host's gateway (the routes +// orchestration.HostRouteTargets derives from the same api.consumes), and a +// service-dependency on it would be a mesh edge past the gateway — the path a +// solution must not hold — so none is asked for, or added. +func missingServiceDependencies(solution *resources.Module, service *resources.Service, consumes []manifest.APIDeclaration) ([]missingDependency, error) { existing := make(map[[2]string]bool, len(service.ServiceDependencies)) for _, dep := range service.ServiceDependencies { existing[[2]string{dep.Name, dep.Module}] = true } var missing []missingDependency - for i := range entries { - entry := &entries[i] - if existing[[2]string{entry.Endpoint.Service, entry.ModuleName}] { + for i := range consumes { + consume := &consumes[i] + if consume.Module != solution.Name || existing[[2]string{consume.Service, consume.Module}] { continue } - missing = append(missing, missingDependency{Module: entry.ModuleName, Service: entry.Endpoint.Service, Endpoint: entry.Endpoint.Endpoint}) + missing = append(missing, missingDependency{Module: consume.Module, Service: consume.Service, Endpoint: consume.Endpoint}) } return missing, nil } @@ -403,6 +490,9 @@ func generateSolutionSDKInto(ctx context.Context, name, outputDir string, entrie Entries: entries, GoModule: "github.com/codefly-dev/" + name + "-go", NpmPackage: "@codefly-dev/" + name, + // One library aggregates every consumed module, so each facade + // gets a package of its own. + FacadePackages: true, }) } diff --git a/cmd/sync/solution_sdk_fixture_test.go b/cmd/sync/solution_sdk_fixture_test.go new file mode 100644 index 000000000..65c1d65f5 --- /dev/null +++ b/cmd/sync/solution_sdk_fixture_test.go @@ -0,0 +1,238 @@ +package sync + +import ( + "bytes" + "context" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/codefly-dev/cli/pkg/conformance/conformancetest" + "github.com/codefly-dev/core/composition" + resources "github.com/codefly-dev/core/resources" + "github.com/codefly-dev/core/solution/manifest" + "github.com/codefly-dev/core/standards" +) + +// fixtureProducers are testdata/solution-sdk's two composed packages: the +// consumed service, the protobuf package its proto declares, and its service. +// The solution consumes accounts over connect and records over rest; each +// catalog carries its protobuf contract under grpc, as `generate contracts` +// exports a service whose transport endpoints are not in its interface. +var fixtureProducers = []struct{ module, pkg, service string }{ + {"accounts", "fixture.accounts.v1", "AccountsService"}, + {"records", "fixture.records.v1", "RecordsService"}, +} + +// newTwoModuleSolution copies testdata/solution-sdk into a temp workspace and +// writes each producer's contract catalog, with contract(t, protoDir) as the +// grpc endpoint's descriptor set. +func newTwoModuleSolution(t *testing.T, contract func(t *testing.T, protoDir string) []byte) string { + t.Helper() + dir := t.TempDir() + if err := os.CopyFS(dir, os.DirFS(filepath.Join("testdata", "solution-sdk"))); err != nil { + t.Fatal(err) + } + for _, producer := range fixtureProducers { + moduleDir := filepath.Join(dir, "modules", producer.module) + data := contract(t, filepath.Join(moduleDir, "services", producer.module, "proto")) + path := "contracts/api/" + producer.module + "/grpc/contract.binpb" + if err := os.MkdirAll(filepath.Join(moduleDir, filepath.Dir(filepath.FromSlash(path))), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(moduleDir, filepath.FromSlash(path)), data, 0o644); err != nil { + t.Fatal(err) + } + catalog := &composition.APIContractCatalog{ + Schema: composition.APIContractCatalogSchema, + Package: "fixture/" + producer.module, + Version: "0.1.0", + Endpoints: []composition.APIContractEndpoint{{ + Service: producer.module, + Endpoint: "grpc", + API: standards.GRPC, + Kind: composition.APIContractKindProtobuf, + Package: producer.pkg, + Path: path, + Digest: composition.APIContractDigest(data), + Services: []composition.APIContractService{{ + Name: producer.service, + FullName: producer.pkg + "." + producer.service, + Procedures: []string{"/" + producer.pkg + "." + producer.service + "/Get"}, + }}, + }}, + } + encoded, err := catalog.CanonicalBytes() + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(moduleDir, "contracts", "api", "catalog.codefly.json"), encoded, 0o644); err != nil { + t.Fatal(err) + } + } + return dir +} + +// syntheticContract stands in for a descriptor set where nothing reaches +// codegen. +func syntheticContract(_ *testing.T, protoDir string) []byte { + return []byte("synthetic-descriptor-set " + protoDir) +} + +func backendServiceFile(wsDir string) string { + return filepath.Join(wsDir, "services", "backend", resources.ServiceConfigurationName) +} + +// Gap 1: the manifest names no executor. core's strict Load refuses it for +// that; this command reads only api.consumes and must not. +func TestSyncSolutionSDKReadsAManifestWithoutAnAgent(t *testing.T) { + wsDir := newTwoModuleSolution(t, syntheticContract) + data, err := os.ReadFile(filepath.Join(wsDir, manifest.FileName)) + if err != nil { + t.Fatal(err) + } + if _, err := manifest.Load(data); err == nil || !strings.Contains(err.Error(), "agent.kind") { + t.Fatalf("precondition: the fixture must be a manifest core's executor loader refuses for its agent block; got %v", err) + } + + t.Chdir(wsDir) + resetSolutionSDKFlags(t) + if err := runSyncSolutionSDK(context.Background()); err != nil { + t.Fatalf("a solution manifest without an agent block must sync: %v", err) + } +} + +// Gap 2: a consume of a module's connect or rest endpoint resolves to that +// service's protobuf contract, which the catalog carries under grpc. +func TestSyncSolutionSDKResolvesTransportEndpointsToTheProtobufContract(t *testing.T) { + wsDir := newTwoModuleSolution(t, syntheticContract) + ctx := context.Background() + ws := loadFixtureWorkspace(t, wsDir) + data, err := os.ReadFile(filepath.Join(wsDir, manifest.FileName)) + if err != nil { + t.Fatal(err) + } + declared, err := manifest.LoadConsumption(data) + if err != nil { + t.Fatal(err) + } + + entries, err := resolveConsumedContracts(ctx, ws, declared) + if err != nil { + t.Fatalf("resolveConsumedContracts: %v", err) + } + if len(entries) != len(fixtureProducers) { + t.Fatalf("resolved %d entries, want %d", len(entries), len(fixtureProducers)) + } + for i, producer := range fixtureProducers { + got := entries[i].Endpoint + if got.Endpoint != "grpc" || got.Kind != composition.APIContractKindProtobuf || got.Package != producer.pkg { + t.Errorf("consume of %s resolved to %s/%s %s %s, want the grpc protobuf contract %s", producer.module, got.Service, got.Endpoint, got.Kind, got.Package, producer.pkg) + } + } + + // An endpoint the producer does not declare is still refused, by name. + declared.API.Consumes[1].Endpoint = "graphql" + if _, err := resolveConsumedContracts(ctx, ws, declared); err == nil || !strings.Contains(err.Error(), "declares no endpoint graphql") { + t.Fatalf("error = %v, want the undeclared endpoint named", err) + } +} + +// Gap 3: both consumes are reached through the host's gateway, so neither +// the check nor --apply-dependencies asks for a service-dependency: that edge +// is the mesh path past the gateway a solution must not hold. +func TestSyncSolutionSDKAsksNoMeshEdgeForAGatewayServedConsume(t *testing.T) { + wsDir := newTwoModuleSolution(t, syntheticContract) + before, err := os.ReadFile(backendServiceFile(wsDir)) + if err != nil { + t.Fatal(err) + } + + t.Chdir(wsDir) + resetSolutionSDKFlags(t) + solutionSDKApply = true + if err := runSyncSolutionSDK(context.Background()); err != nil { + t.Fatalf("runSyncSolutionSDK --apply-dependencies: %v", err) + } + after, err := os.ReadFile(backendServiceFile(wsDir)) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(before, after) { + t.Fatalf("--apply-dependencies added a mesh edge to a gateway-served module:\n%s", after) + } + + // --check counts the absent library, and nothing else. + resetSolutionSDKFlags(t) + solutionSDKCheck = true + if err := runSyncSolutionSDK(context.Background()); err == nil || !strings.Contains(err.Error(), "(1 problem(s))") { + t.Fatalf("--check = %v, want only the absent library reported", err) + } +} + +// buildFixtureContract compiles a fixture proto directory with buf, as +// `generate contracts` would export it. +func buildFixtureContract(t *testing.T, protoDir string) []byte { + t.Helper() + out := filepath.Join(t.TempDir(), "contract.binpb") + cmd := exec.Command("buf", "build", protoDir, "-o", out) + if output, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("buf build %s: %v\n%s", protoDir, err, output) + } + data, err := os.ReadFile(out) + if err != nil { + t.Fatal(err) + } + return data +} + +// The whole issue, through the real proto companion: the solution SDK of a +// solution with no agent block, consuming two modules over connect and rest +// with no service-dependencies, generates a Go half that builds (the command +// compiles it) and a TypeScript half, and --check then passes on it. +func TestSyncSolutionSDKTwoModulesEndToEnd(t *testing.T) { + conformancetest.Gate(t, "linux-amd64-docker-generate", "docker", "buf") + wsDir := newTwoModuleSolution(t, buildFixtureContract) + ctx := context.Background() + + t.Chdir(wsDir) + resetSolutionSDKFlags(t) + skipGenerateForTests = false + solutionSDKLanguages = []string{"go", "typescript"} + if err := runSyncSolutionSDK(ctx); err != nil { + t.Fatalf("runSyncSolutionSDK: %v", err) + } + + library := filepath.Join(wsDir, "libraries", "portal-sdk") + t.Cleanup(func() { + if t.Failed() { + _ = filepath.WalkDir(library, func(path string, entry os.DirEntry, err error) error { + if err == nil && !entry.IsDir() { + t.Log(strings.TrimPrefix(path, library+string(filepath.Separator))) + } + return nil + }) + } + }) + for _, path := range []string{ + "go/accounts/accounts_facade.pb.go", + "go/records/records_facade.pb.go", + "typescript/src/index.ts", + "typescript/src/accounts/index.ts", + "typescript/src/records/index.ts", + } { + if _, err := os.Stat(filepath.Join(library, filepath.FromSlash(path))); err != nil { + t.Errorf("generated library lacks %s: %v", path, err) + } + } + + resetSolutionSDKFlags(t) + skipGenerateForTests = false + solutionSDKLanguages = []string{"go", "typescript"} + solutionSDKCheck = true + if err := runSyncSolutionSDK(ctx); err != nil { + t.Fatalf("--check on the library just generated: %v", err) + } +} diff --git a/cmd/sync/solution_sdk_test.go b/cmd/sync/solution_sdk_test.go index 6cb825a35..94a280fcc 100644 --- a/cmd/sync/solution_sdk_test.go +++ b/cmd/sync/solution_sdk_test.go @@ -319,6 +319,10 @@ func TestSyncSolutionSDKRejectsUnknownService(t *testing.T) { } } +// TestSyncSolutionSDKApplyDependencies: the only consume the entry service +// calls directly is one of the solution's own module, and --apply-dependencies +// adds exactly that edge, node-preserving. A consume of another module (here +// host's accounts) is reached through the host's gateway and gets none. func TestSyncSolutionSDKApplyDependencies(t *testing.T) { root := t.TempDir() hostDir := filepath.Join(root, "host") @@ -348,28 +352,48 @@ func TestSyncSolutionSDKApplyDependencies(t *testing.T) { t.Fatalf("runSyncSolutionSDK: %v", err) } - servicePath := filepath.Join(backendDir, "services", "api", resources.ServiceConfigurationName) - data, err := os.ReadFile(servicePath) + serviceDir := filepath.Join(backendDir, "services", "api") + service, err := resources.LoadServiceFromDir(ctx, serviceDir) if err != nil { t.Fatal(err) } - content := string(data) - if !strings.Contains(content, "# hand-written note: do not remove") { - t.Fatalf("hand-written comment was dropped:\n%s", content) + if len(service.ServiceDependencies) != 0 { + t.Fatalf("--apply-dependencies added a mesh edge to a gateway-served module: %+v", service.ServiceDependencies) + } + + ws := loadFixtureWorkspace(t, wsDir) + solution, err := ws.LoadModuleFromName(ctx, "backend") + if err != nil { + t.Fatal(err) + } + consumes := []manifest.APIDeclaration{ + {ID: "accounts", Protocol: standards.CONNECT, Module: "host", Service: "accounts", Endpoint: "connect"}, + {ID: "worker", Protocol: standards.GRPC, Module: "backend", Service: "worker", Endpoint: "grpc"}, + } + missing, err := missingServiceDependencies(solution, service, consumes) + if err != nil { + t.Fatal(err) + } + if len(missing) != 1 || missing[0] != (missingDependency{Module: "backend", Service: "worker", Endpoint: "grpc"}) { + t.Fatalf("missing = %+v, want only the solution's own backend/worker", missing) + } + if err := applyServiceDependencies(service, missing); err != nil { + t.Fatal(err) } - service, err := resources.LoadServiceFromDir(ctx, filepath.Join(backendDir, "services", "api")) + data, err := os.ReadFile(filepath.Join(serviceDir, resources.ServiceConfigurationName)) if err != nil { t.Fatal(err) } - found := false - for _, dep := range service.ServiceDependencies { - if dep.Name == "accounts" && dep.Module == "host" { - found = true - } + if !strings.Contains(string(data), "# hand-written note: do not remove") { + t.Fatalf("hand-written comment was dropped:\n%s", data) + } + service, err = resources.LoadServiceFromDir(ctx, serviceDir) + if err != nil { + t.Fatal(err) } - if !found { - t.Fatalf("service-dependencies missing accounts/host: %+v", service.ServiceDependencies) + if len(service.ServiceDependencies) != 1 || service.ServiceDependencies[0].Name != "worker" || service.ServiceDependencies[0].Module != "backend" { + t.Fatalf("service-dependencies = %+v, want backend/worker alone", service.ServiceDependencies) } } diff --git a/cmd/sync/testdata/solution-sdk/module.codefly.yaml b/cmd/sync/testdata/solution-sdk/module.codefly.yaml new file mode 100644 index 000000000..8fb9a41e1 --- /dev/null +++ b/cmd/sync/testdata/solution-sdk/module.codefly.yaml @@ -0,0 +1,5 @@ +kind: module +name: portal +service-entry: backend +services: + - name: backend diff --git a/cmd/sync/testdata/solution-sdk/modules/accounts/module.codefly.yaml b/cmd/sync/testdata/solution-sdk/modules/accounts/module.codefly.yaml new file mode 100644 index 000000000..f447d2b7b --- /dev/null +++ b/cmd/sync/testdata/solution-sdk/modules/accounts/module.codefly.yaml @@ -0,0 +1,4 @@ +kind: module +name: accounts +services: + - name: accounts diff --git a/cmd/sync/testdata/solution-sdk/modules/accounts/module.package.codefly.yaml b/cmd/sync/testdata/solution-sdk/modules/accounts/module.package.codefly.yaml new file mode 100644 index 000000000..e8fe994be --- /dev/null +++ b/cmd/sync/testdata/solution-sdk/modules/accounts/module.package.codefly.yaml @@ -0,0 +1,10 @@ +kind: module-package +schema: codefly/module-package/v2 +id: fixture/accounts +version: 0.1.0 +minimum-codefly-version: ">=0.0.0" +artifact-roots: + - contracts +contracts: + composition: ">=2.0" +services: [] diff --git a/cmd/sync/testdata/solution-sdk/modules/accounts/services/accounts/proto/accounts.proto b/cmd/sync/testdata/solution-sdk/modules/accounts/services/accounts/proto/accounts.proto new file mode 100644 index 000000000..677d7761f --- /dev/null +++ b/cmd/sync/testdata/solution-sdk/modules/accounts/services/accounts/proto/accounts.proto @@ -0,0 +1,11 @@ +syntax = "proto3"; +package fixture.accounts.v1; + +// GetRequest and GetResponse share their names with the records contract's, +// so one TypeScript barrel re-exporting both contracts' bindings collides. +message GetRequest { string id = 1; } +message GetResponse { string name = 1; } + +service AccountsService { + rpc Get(GetRequest) returns (GetResponse); +} diff --git a/cmd/sync/testdata/solution-sdk/modules/accounts/services/accounts/service.codefly.yaml b/cmd/sync/testdata/solution-sdk/modules/accounts/services/accounts/service.codefly.yaml new file mode 100644 index 000000000..3a65d8ae6 --- /dev/null +++ b/cmd/sync/testdata/solution-sdk/modules/accounts/services/accounts/service.codefly.yaml @@ -0,0 +1,16 @@ +kind: service +name: accounts +version: 0.1.0 +module: accounts +agent: + kind: codefly:service + name: go-grpc + publisher: codefly.dev + version: 0.0.1 +endpoints: + - name: grpc + api: grpc + visibility: internal + - name: connect + api: connect + visibility: internal diff --git a/cmd/sync/testdata/solution-sdk/modules/records/module.codefly.yaml b/cmd/sync/testdata/solution-sdk/modules/records/module.codefly.yaml new file mode 100644 index 000000000..c364ab663 --- /dev/null +++ b/cmd/sync/testdata/solution-sdk/modules/records/module.codefly.yaml @@ -0,0 +1,4 @@ +kind: module +name: records +services: + - name: records diff --git a/cmd/sync/testdata/solution-sdk/modules/records/module.package.codefly.yaml b/cmd/sync/testdata/solution-sdk/modules/records/module.package.codefly.yaml new file mode 100644 index 000000000..d80215029 --- /dev/null +++ b/cmd/sync/testdata/solution-sdk/modules/records/module.package.codefly.yaml @@ -0,0 +1,10 @@ +kind: module-package +schema: codefly/module-package/v2 +id: fixture/records +version: 0.1.0 +minimum-codefly-version: ">=0.0.0" +artifact-roots: + - contracts +contracts: + composition: ">=2.0" +services: [] diff --git a/cmd/sync/testdata/solution-sdk/modules/records/services/records/proto/records.proto b/cmd/sync/testdata/solution-sdk/modules/records/services/records/proto/records.proto new file mode 100644 index 000000000..61a79f294 --- /dev/null +++ b/cmd/sync/testdata/solution-sdk/modules/records/services/records/proto/records.proto @@ -0,0 +1,9 @@ +syntax = "proto3"; +package fixture.records.v1; + +message GetRequest { string key = 1; } +message GetResponse { bytes value = 1; } + +service RecordsService { + rpc Get(GetRequest) returns (GetResponse); +} diff --git a/cmd/sync/testdata/solution-sdk/modules/records/services/records/service.codefly.yaml b/cmd/sync/testdata/solution-sdk/modules/records/services/records/service.codefly.yaml new file mode 100644 index 000000000..3bf17db5b --- /dev/null +++ b/cmd/sync/testdata/solution-sdk/modules/records/services/records/service.codefly.yaml @@ -0,0 +1,16 @@ +kind: service +name: records +version: 0.1.0 +module: records +agent: + kind: codefly:service + name: go-grpc + publisher: codefly.dev + version: 0.0.1 +endpoints: + - name: grpc + api: grpc + visibility: internal + - name: rest + api: rest + visibility: internal diff --git a/cmd/sync/testdata/solution-sdk/services/backend/service.codefly.yaml b/cmd/sync/testdata/solution-sdk/services/backend/service.codefly.yaml new file mode 100644 index 000000000..cb09c9f68 --- /dev/null +++ b/cmd/sync/testdata/solution-sdk/services/backend/service.codefly.yaml @@ -0,0 +1,13 @@ +kind: service +name: backend +version: 0.0.1 +module: portal +agent: + kind: codefly:service + name: go + publisher: codefly.dev + version: 0.0.1 +endpoints: + - name: http + api: http + visibility: private diff --git a/cmd/sync/testdata/solution-sdk/solution.codefly.yaml b/cmd/sync/testdata/solution-sdk/solution.codefly.yaml new file mode 100644 index 000000000..f7e10330e --- /dev/null +++ b/cmd/sync/testdata/solution-sdk/solution.codefly.yaml @@ -0,0 +1,18 @@ +# A solution that names no executor: nothing it runs reads an agent block, +# so it declares none. It reaches both modules through the host's gateway, +# so its backend declares no service-dependencies on them. +schema_version: codefly.solution-manifest/v0 +protocol_version: codefly.solution/v0 +api: + consumes: + - id: accounts + protocol: connect + module: accounts + service: accounts + endpoint: connect + services: [AccountsService] + - id: records + protocol: rest + module: records + service: records + endpoint: rest diff --git a/cmd/sync/testdata/solution-sdk/workspace.codefly.yaml b/cmd/sync/testdata/solution-sdk/workspace.codefly.yaml new file mode 100644 index 000000000..c2a4c2ce6 --- /dev/null +++ b/cmd/sync/testdata/solution-sdk/workspace.codefly.yaml @@ -0,0 +1,9 @@ +name: portal +layout: modules +modules: + - name: portal + path: . + - name: accounts + path: modules/accounts + - name: records + path: modules/records diff --git a/docs/cli-reference.md b/docs/cli-reference.md index d16c03f9c..6777d1a46 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -4773,8 +4773,13 @@ Aggregate one client library per solution from api.consumes ``` Reads api.consumes in solution.codefly.yaml and produces one aggregated client library per solution, carrying only the declared services, generated -from the contracts of the module packages the workspace composes. Also keeps -the runtime service-dependencies in step with the declaration. +from the contracts of the module packages the workspace composes. A consume +of a module's rest or connect endpoint is generated from that service's +protobuf contract. Each consumed module's facade gets a package of its own. + +A consume of another module is reached through the host's gateway and needs +no service-dependencies entry; only a consume of the solution's own module is +a direct call, and that one is kept in step with the declaration. ``` ``` @@ -4784,7 +4789,7 @@ codefly sync solution-sdk [flags] Flags: ``` - --apply-dependencies add missing service-dependencies entries for every bound consume entry + --apply-dependencies add missing service-dependencies entries for consumes of the solution's own module (other modules are reached through the host gateway) --check do not write; exit 1 if the library or service-dependencies are out of date --language strings languages to generate: go,typescript,python ``` diff --git a/docs/commands.md b/docs/commands.md index c7022e149..6484d25a8 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -2127,36 +2127,72 @@ codefly sync solution-sdk --language python --apply-dependencies # Aggregate ap ```bash codefly sync solution-sdk --language python # Generate/refresh the solution's library -codefly sync solution-sdk --language go,python --apply-dependencies # Also wire up service-dependencies +codefly sync solution-sdk --language go,typescript # Go and TypeScript halves of one library codefly sync solution-sdk --language python --check # CI drift gate ``` Run from a solution workspace (`workspace.codefly.yaml` plus a module whose `service-entry` is the solution backend, as in `core-solutions/solutions/*`). Reads the single declaration `api.consumes` in `solution.codefly.yaml` (the -solution manifest core's `solution/manifest` package defines) and aggregates +solution manifest core's `solution/manifest` package defines) — through core's +`LoadConsumption`, which holds `api.consumes` strictly and leaves the executor +sections (`agent`, `lifecycle`) unread, so a solution that names no +`codefly:solution` executor needs no `agent` block — and aggregates every module-bound entry — `{module, service, endpoint, version, services, as}` — into **one** library, `libraries/-sdk/`, generated from the composed module packages' API contract catalogs (the same generator `generate client` uses, aggregating N contract entries into one library instead of one). This is the **one declaration, three outputs** rule: the same -`api.consumes` entries drive the generated SDK, the runtime -`service-dependencies` (below), and the `SolutionBundle` `platform.consumes` -lodestar derives from the manifest. +`api.consumes` entries drive the generated SDK, the host gateway's routes to +the consumed modules, and the `SolutionBundle` `platform.consumes` lodestar +derives from the manifest. | Flag | Description | |------|-------------| | `--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. | +| `--apply-dependencies` | Add any `service-dependencies` entry missing for a consume of the solution's **own** module to the entry service's `service.codefly.yaml`; without it, missing entries are printed as a warning. Existing entries are never removed. | + +A consume of **another** module needs no `service-dependencies` entry, and +neither `--check` nor `--apply-dependencies` asks for one: the solution reaches +that module through the host's gateway (the same routes `run` and the render +derive from `api.consumes`), and a direct edge to the module's service would be +a mesh path past the gateway, which a solution must not hold. Only a consume of +the solution's own module is a direct call. Each bound entry is resolved against the workspace's own composition: the composed module named by `module` (workspace `add module` first), its `module.package.codefly.yaml` and `contracts/api/catalog.codefly.json` (from `generate contracts`), `version` checked as a semver constraint against the composed package version, and `services` validated against the endpoint's -declared protobuf services. Two entries whose contracts share a proto package -at different digests (a diamond) are rejected. +declared protobuf services. A consume of a `connect` or `rest` endpoint is +generated from that service's protobuf contract when the catalog carries none +for the endpoint itself: both transports serve the same proto services +(connect natively, rest transcoded), and a catalog exports the contract under +the service's `grpc` endpoint. A service exporting protobuf contracts on +several endpoints is refused as ambiguous; a `rest` endpoint of a service with +no protobuf contract keeps its own OpenAPI entry. Two entries whose contracts +share a proto package at different digests (a diamond), or that would generate +the same facade name (set `as` on one), are rejected. + +The library gives each consumed module's facade a package of its own, so any +number of modules aggregate into one library: + +``` +libraries/-sdk/ + go/ go.mod, accounts/accounts_facade.pb.go, gen/accounts/…, records/…, gen/records/… + typescript/ package.json, src/index.ts, src/accounts/{index.ts,accounts_facade.ts}, src/gen/accounts/…, … + python/ pyproject.toml, /accounts.py, /records.py, /_gen/… +``` + +Go imports a facade as `/`; its bindings live under +`gen//`, since two contracts each rooted at an `api.proto` would +otherwise write one file. TypeScript's root (``) re-exports every +facade and nothing else — two contracts' bindings can declare the same name, +which one barrel cannot re-export twice — and `/` exports that +facade with its own bindings. The `package.json` subpath export is part of the +write-once scaffold, so a library whose `package.json` predates it needs the +`"./*"` export added by hand. If a `solution-sdk.yaml` (the pre-`api.consumes` declaration this command replaces) sits next to the manifest, its dependencies must be migrated into diff --git a/docs/design/library-system.md b/docs/design/library-system.md index 2aa0b5390..f1f053e24 100644 --- a/docs/design/library-system.md +++ b/docs/design/library-system.md @@ -590,11 +590,14 @@ rejected as a diamond, the same rule `pkg/composition/coherence.go` enforces for a service's transitive library closure, applied here to the contracts one generation aggregates. -This single declaration drives three outputs, kept in step by the same -command: the generated SDK above; the entry service's runtime -`service-dependencies` (`--apply-dependencies` adds any entry missing there, -never removing an existing one); and the `SolutionBundle`'s -`platform.consumes`, which lodestar derives from the same manifest. Before +This single declaration drives three outputs: the generated SDK above; the +host gateway's routes to the consumed modules, which `run` and the render +derive from it; and the `SolutionBundle`'s `platform.consumes`, which lodestar +derives from the same manifest. It does not drive the entry service's +`service-dependencies`: a solution reaches another module only through the +host's gateway, and a direct edge would be a mesh path past it. Only a consume +of the solution's own module is a direct call, and `--apply-dependencies` adds +that edge when it is missing, never removing an existing one. Before `api.consumes`, a solution declared what it consumed in up to four disconnected places — `service-dependencies`, a vendored `solution-sdk.yaml`, a `go.mod` replace pin, and a hand-written `Manifest{}` struct for host diff --git a/pkg/generators/client.go b/pkg/generators/client.go index 50967c596..d51fed708 100644 --- a/pkg/generators/client.go +++ b/pkg/generators/client.go @@ -75,6 +75,17 @@ type ClientLibraryRequest struct { Force bool GoModule string NpmPackage string + + // FacadePackages gives every entry's facade a package of its own, named + // after its facade entry-point: go// (import /) + // and typescript/src// (import /, the facade + // and that entry's bindings), with the TypeScript root re-exporting the + // facades alone. An aggregated library needs it: each Go facade carries + // its own package clause, so two cannot share go/, and two contracts' + // bindings can declare the same message name, so one TypeScript barrel + // cannot re-export both. Without it the single facade sits at the root, + // the layout `generate client` has always produced. + FacadePackages bool } // generatedLanguageExport is one languages: entry of library.codefly.yaml. @@ -112,6 +123,9 @@ func GenerateClientLibrary(ctx context.Context, req *ClientLibraryRequest) error if err := checkContractDiamond(req.Entries); err != nil { return err } + if err := checkFacadeNames(req.Entries); err != nil { + return err + } openAPICount := 0 for i := range req.Entries { if req.Entries[i].Endpoint.Kind == composition.APIContractKindOpenAPI { @@ -170,7 +184,7 @@ func GenerateClientLibrary(ctx context.Context, req *ClientLibraryRequest) error 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) + export, genErr := generateLanguageForEntries(ctx, lang, filepath.Join(req.Output, string(lang)), req.Entries, !req.NoFacade, req.FacadePackages, req.GoModule, req.NpmPackage, pyPackage) if genErr != nil { return fmt.Errorf("cannot generate %s client: %w", lang, genErr) } @@ -228,24 +242,44 @@ func checkContractDiamond(entries []ContractEntry) error { return nil } +// checkFacadeNames refuses two entries generating the same facade entry-point: +// in every language the name is a file (and, with FacadePackages, a package +// directory), so the second would overwrite the first. +func checkFacadeNames(entries []ContractEntry) error { + seen := map[string]*ContractEntry{} + for i := range entries { + entry := &entries[i] + if entry.Endpoint.Kind != composition.APIContractKindProtobuf { + continue + } + name := entryModuleName(entry) + if previous, exists := seen[name]; exists { + return fmt.Errorf("%s/%s/%s and %s/%s/%s both generate the facade %q; give one of them its own name (`as` in api.consumes)", + previous.ModuleName, previous.Endpoint.Service, previous.Endpoint.Endpoint, entry.ModuleName, entry.Endpoint.Service, entry.Endpoint.Endpoint, name) + } + seen[name] = entry + } + 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) { +func generateLanguageForEntries(ctx context.Context, lang languages.Language, langDir string, entries []ContractEntry, facade, facadePackages bool, goModule, npmPackage, pyPackage string) (generatedLanguageExport, error) { if err := os.MkdirAll(langDir, 0o755); err != nil { return generatedLanguageExport{}, err } if len(entries) == 1 && entries[0].Endpoint.Kind == composition.APIContractKindOpenAPI { return generateOpenAPILanguageEntry(ctx, lang, langDir, &entries[0], goModule, npmPackage) } - return generateProtobufLanguageEntries(ctx, lang, langDir, entries, facade, goModule, npmPackage, pyPackage) + return generateProtobufLanguageEntries(ctx, lang, langDir, entries, facade, facadePackages, 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) { +func generateProtobufLanguageEntries(ctx context.Context, lang languages.Language, langDir string, entries []ContractEntry, facade, facadePackages bool, goModule, npmPackage, pyPackage string) (generatedLanguageExport, error) { switch lang { case languages.GO: if err := wipeGoLibrary(langDir); err != nil { @@ -266,7 +300,7 @@ func generateProtobufLanguageEntries(ctx context.Context, lang languages.Languag var mapped []upstreamProtoModule for i := range entries { entry := &entries[i] - marked, err := generateOneProtobufEntry(ctx, lang, langDir, entry, facade, goModule, pyPackage) + marked, err := generateOneProtobufEntry(ctx, lang, langDir, entry, facade, facadePackages, goModule, pyPackage) if err != nil { return generatedLanguageExport{}, fmt.Errorf("entry %s/%s/%s: %w", entry.ModuleName, entry.Endpoint.Service, entry.Endpoint.Endpoint, err) } @@ -277,7 +311,7 @@ func generateProtobufLanguageEntries(ctx context.Context, lang languages.Languag case languages.GO: return finalizeGoLibrary(langDir, goModule) case languages.TYPESCRIPT: - return finalizeTypeScriptLibrary(langDir, npmPackage) + return finalizeTypeScriptLibrary(langDir, npmPackage, facadePackages) case languages.PYTHON: return finalizePythonLibrary(langDir, pyPackage, pythonPackagesFor(mapped)) default: @@ -288,7 +322,7 @@ func generateProtobufLanguageEntries(ctx context.Context, lang languages.Languag // 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) { +func generateOneProtobufEntry(ctx context.Context, lang languages.Language, langDir string, entry *ContractEntry, facade, facadePackages bool, goModule, pyPackage string) ([]upstreamProtoModule, error) { raw, err := os.MkdirTemp("", "codefly-client-raw") if err != nil { return nil, err @@ -325,9 +359,9 @@ func generateOneProtobufEntry(ctx context.Context, lang languages.Language, lang switch lang { case languages.GO: - return mapped, mergeGoEntry(langDir, raw, moduleName, goModule) + return mapped, mergeGoEntry(langDir, raw, moduleName, goModule, facadePackages) case languages.TYPESCRIPT: - return mapped, mergeTypeScriptEntry(langDir, raw) + return mapped, mergeTypeScriptEntry(langDir, raw, moduleName, facadePackages) case languages.PYTHON: return mapped, mergePythonEntry(filepath.Join(langDir, pyPackage), raw, moduleName, pyPackage) default: diff --git a/pkg/generators/client_library.go b/pkg/generators/client_library.go index fda872877..f261bd789 100644 --- a/pkg/generators/client_library.go +++ b/pkg/generators/client_library.go @@ -51,23 +51,45 @@ func wipeGoLibrary(langDir string) error { if err := os.RemoveAll(filepath.Join(langDir, "gen")); err != nil { return err } - return removeMatching(langDir, "*_facade.pb.go") + if err := removeMatching(langDir, "*_facade.pb.go"); err != nil { + return err + } + // A facade package (FacadePackages) is go//: its facade file is + // this command's, the directory only while nothing else lives in it. + packages, err := filepath.Glob(filepath.Join(langDir, "*", "*_facade.pb.go")) + if err != nil { + return err + } + for _, facade := range packages { + if err := os.Remove(facade); err != nil && !os.IsNotExist(err) { + return err + } + _ = os.Remove(filepath.Dir(facade)) // kept when a hand-added file remains + } + return nil } // mergeGoEntry rewrites raw's hardcoded import prefix to goModule/gen (keyed // to this entry's own facade module name) and moves it into langDir: the -// *_facade.pb.go file(s) to the go/ root, everything else to go/gen/. -func mergeGoEntry(langDir, raw, moduleName, goModule string) error { +// *_facade.pb.go file(s) to the go/ root, everything else to go/gen/. With +// facadePackages the entry's facade moves to its own package go// and +// its bindings to go/gen//: buf writes a binding where its proto file +// sits in the contract, and two contracts each rooted at an api.proto would +// otherwise write one go/gen/api.pb.go. +func mergeGoEntry(langDir, raw, moduleName, goModule string, facadePackages bool) error { service := moduleName if service == "" { service = defaultFacadeModule } + genDir, genImport, facadeDir := filepath.Join(langDir, "gen"), goModule+"/gen", langDir + if facadePackages { + genDir, genImport, facadeDir = filepath.Join(genDir, service), genImport+"/"+service, filepath.Join(langDir, service) + } oldPrefix := "github.com/codefly-dev/cli/pkg/builder/clients/" + service - if err := rewriteGoImportPrefix(raw, oldPrefix, goModule+"/gen"); err != nil { + if err := rewriteGoImportPrefix(raw, oldPrefix, genImport); err != nil { return fmt.Errorf("cannot rewrite generated import paths: %w", err) } - genDir := filepath.Join(langDir, "gen") - return moveTreeSplitBySuffix(raw, genDir, langDir, "_facade.pb.go") + return moveTreeSplitBySuffix(raw, genDir, facadeDir, "_facade.pb.go") } // finalizeGoLibrary writes go.mod (if absent), resolves it and proves the @@ -92,78 +114,126 @@ func wipeTypeScriptLibrary(langDir string) error { } // mergeTypeScriptEntry moves raw's tree under src/gen/, hoists the -// *_facade.ts file(s) to src/, and rewrites the facade's same-directory -// relative import ("./x") to reach into its new location under gen/ instead. -func mergeTypeScriptEntry(langDir, raw string) error { +// *_facade.ts file(s) to src/ — or, with facadePackages, moves the bindings +// under src/gen// and the facade to the entry's own src//, +// beside an index.ts re-exporting that facade and the entry's bindings — and +// rewrites the facade's same-directory relative import ("./x") to reach into +// its new location under gen/ instead. +func mergeTypeScriptEntry(langDir, raw, moduleName string, facadePackages bool) error { srcDir := filepath.Join(langDir, "src") genDir := filepath.Join(srcDir, "gen") - facadeFiles, err := moveTreeSplitBySuffixWithRelDir(raw, genDir, srcDir, "_facade.ts") + facadeDir, toGen := srcDir, "./gen" + if facadePackages { + genDir, facadeDir, toGen = filepath.Join(genDir, moduleName), filepath.Join(srcDir, moduleName), "../gen/"+moduleName + } + bindings, err := typeScriptModules(raw, "_facade.ts") + if err != nil { + return err + } + facadeFiles, err := moveTreeSplitBySuffixWithRelDir(raw, genDir, facadeDir, "_facade.ts") if err != nil { return err } relImportRe := regexp.MustCompile(`(from\s+")\.\/`) for facadePath, relDir := range facadeFiles { - data, err := os.ReadFile(facadePath) //nolint:gosec // facadePath is a file this command just wrote - if err != nil { - return err + data, readErr := os.ReadFile(facadePath) //nolint:gosec // facadePath is a file this command just wrote + if readErr != nil { + return readErr } - rewritten := relImportRe.ReplaceAll(data, []byte(`${1}./gen/`+filepath.ToSlash(relDir)+`/`)) - if err := os.WriteFile(facadePath, rewritten, 0o644); err != nil { //nolint:gosec - return err + rewritten := relImportRe.ReplaceAll(data, []byte(`${1}`+toGen+`/`+filepath.ToSlash(relDir)+`/`)) + if writeErr := os.WriteFile(facadePath, rewritten, 0o644); writeErr != nil { //nolint:gosec + return writeErr } } - return nil -} - -func finalizeTypeScriptLibrary(langDir, npmPackage string) (generatedLanguageExport, error) { - if err := writeTypeScriptScaffold(langDir, npmPackage); err != nil { - return generatedLanguageExport{}, err + if !facadePackages { + return nil } - if err := writeTypeScriptIndex(langDir); err != nil { - return generatedLanguageExport{}, err + facades, err := typeScriptModules(facadeDir, "") + if err != nil { + return err } - return generatedLanguageExport{Name: "typescript", Path: "typescript/", Exports: []string{npmPackage}}, nil + modules := make([]string, 0, len(facades)+len(bindings)) + for _, facade := range facades { + modules = append(modules, "./"+facade) + } + for _, binding := range bindings { + modules = append(modules, toGen+"/"+binding) + } + return writeTypeScriptBarrel(filepath.Join(facadeDir, "index.ts"), modules) } -// writeTypeScriptIndex writes src/index.ts: the barrel re-exporting every -// hoisted facade and every generated binding, and so the one file -// package.json's "exports" names. It is generated output, not a scaffold — -// src/ is wholly owned by this command (wipeTypeScriptLibrary removes it) and -// which files exist depends on the contracts generated — so it is rewritten on -// every run instead of written once. Without it the library declares an entry -// point nothing produced and cannot be resolved at all. -func writeTypeScriptIndex(langDir string) error { - srcDir := filepath.Join(langDir, "src") - if err := os.MkdirAll(srcDir, 0o755); err != nil { - return err - } - index := filepath.Join(srcDir, "index.ts") +// typeScriptModules lists the .ts files under dir as the ".js" module +// specifiers the generated sources import each other by, relative to dir and +// sorted, leaving out index.ts and, when excludeSuffix is set, the files +// ending in it. +func typeScriptModules(dir, excludeSuffix string) ([]string, error) { var modules []string - if err := filepath.WalkDir(srcDir, func(path string, entry fs.DirEntry, err error) error { + err := filepath.WalkDir(dir, func(path string, entry fs.DirEntry, err error) error { if err != nil { return err } - if entry.IsDir() || path == index || !strings.HasSuffix(entry.Name(), ".ts") { + name := entry.Name() + if entry.IsDir() || !strings.HasSuffix(name, ".ts") || name == "index.ts" || (excludeSuffix != "" && strings.HasSuffix(name, excludeSuffix)) { return nil } - relative, relErr := filepath.Rel(srcDir, path) + relative, relErr := filepath.Rel(dir, path) if relErr != nil { return relErr } // The ".js" specifier the generated bindings and facades already use: // the sources are ESM and resolve by emitted path, not by source name. - modules = append(modules, "./"+strings.TrimSuffix(filepath.ToSlash(relative), ".ts")+".js") + modules = append(modules, strings.TrimSuffix(filepath.ToSlash(relative), ".ts")+".js") return nil - }); err != nil { + }) + sort.Strings(modules) + return modules, err +} + +func finalizeTypeScriptLibrary(langDir, npmPackage string, facadePackages bool) (generatedLanguageExport, error) { + if err := writeTypeScriptScaffold(langDir, npmPackage, facadePackages); err != nil { + return generatedLanguageExport{}, err + } + if err := writeTypeScriptIndex(langDir, facadePackages); err != nil { + return generatedLanguageExport{}, err + } + return generatedLanguageExport{Name: "typescript", Path: "typescript/", Exports: []string{npmPackage}}, nil +} + +// writeTypeScriptIndex writes src/index.ts: the barrel re-exporting every +// hoisted facade and every generated binding — or, with facadePackages, every +// facade alone, since two contracts' bindings may declare one name and each +// facade package (src//index.ts) re-exports its own — and so the one +// file package.json's "." export names. It is generated output, not a +// scaffold — src/ is wholly owned by this command (wipeTypeScriptLibrary +// removes it) and which files exist depends on the contracts generated — so it +// is rewritten on every run instead of written once. Without it the library +// declares an entry point nothing produced and cannot be resolved at all. +func writeTypeScriptIndex(langDir string, facadePackages bool) error { + srcDir := filepath.Join(langDir, "src") + if err := os.MkdirAll(srcDir, 0o755); err != nil { return err } - sort.Strings(modules) + modules, err := typeScriptModules(srcDir, "") + if err != nil { + return err + } + exported := make([]string, 0, len(modules)) + for _, module := range modules { + if facadePackages && !strings.HasSuffix(module, "_facade.js") { + continue + } + exported = append(exported, "./"+module) + } + return writeTypeScriptBarrel(filepath.Join(srcDir, "index.ts"), exported) +} + +func writeTypeScriptBarrel(path string, modules []string) error { var content strings.Builder content.WriteString("// generated by codefly generate client; do not edit\n") for _, module := range modules { fmt.Fprintf(&content, "export * from %q;\n", module) } - return os.WriteFile(index, []byte(content.String()), 0o644) //nolint:gosec + return os.WriteFile(path, []byte(content.String()), 0o644) //nolint:gosec } // wipePythonLibrary clears pkgDir's generated content once, before any entry @@ -300,10 +370,10 @@ func assembleOpenAPILibrary(ctx context.Context, lang languages.Language, langDi bestEffortGoModTidy(langDir) return generatedLanguageExport{Name: "go", Path: "go/", Exports: []string{goModule}}, nil } - if err := writeTypeScriptScaffold(langDir, npmPackage); err != nil { + if err := writeTypeScriptScaffold(langDir, npmPackage, false); err != nil { return generatedLanguageExport{}, err } - if err := writeTypeScriptIndex(langDir); err != nil { + if err := writeTypeScriptIndex(langDir, false); err != nil { return generatedLanguageExport{}, err } return generatedLanguageExport{Name: "typescript", Path: "typescript/", Exports: []string{npmPackage}}, nil @@ -640,7 +710,19 @@ func bestEffortGoModTidy(dir string) { } } -func writeTypeScriptScaffold(langDir, npmPackage string) error { +// writeTypeScriptScaffold writes package.json and tsconfig.json once. With +// facadePackages, package.json also exports each facade package by its +// directory ("/"), the import a Go consumer writes as +// "/". +func writeTypeScriptScaffold(langDir, npmPackage string, facadePackages bool) error { + subpaths := "" + if facadePackages { + subpaths = `, + "./*": { + "types": "./dist/*/index.d.ts", + "import": "./dist/*/index.js" + }` + } packageJSON := fmt.Sprintf(`{ "name": %q, "version": "0.0.1", @@ -649,7 +731,7 @@ func writeTypeScriptScaffold(langDir, npmPackage string) error { ".": { "types": "./dist/index.d.ts", "import": "./dist/index.js" - } + }%s }, "files": [ "dist", @@ -669,7 +751,7 @@ func writeTypeScriptScaffold(langDir, npmPackage string) error { "registry": "https://npm.pkg.github.com" } } -`, npmPackage, pinnedTypeScriptVersion) +`, npmPackage, subpaths, pinnedTypeScriptVersion) if err := writeFileIfMissing(filepath.Join(langDir, "package.json"), []byte(packageJSON)); err != nil { return err } diff --git a/pkg/generators/client_library_test.go b/pkg/generators/client_library_test.go index 19fa614a6..460401bf6 100644 --- a/pkg/generators/client_library_test.go +++ b/pkg/generators/client_library_test.go @@ -165,7 +165,7 @@ func TestTypeScriptLibraryDeclaresAnEntryPointItProduces(t *testing.T) { } } - export, err := finalizeTypeScriptLibrary(langDir, "@acme/pdf-client") + export, err := finalizeTypeScriptLibrary(langDir, "@acme/pdf-client", false) if err != nil { t.Fatalf("finalizeTypeScriptLibrary: %v", err) } @@ -235,7 +235,7 @@ func TestTypeScriptLibraryDeclaresAnEntryPointItProduces(t *testing.T) { if err = os.WriteFile(filepath.Join(genDir, "other_pb.ts"), []byte("export const Other = {};\n"), 0o644); err != nil { t.Fatal(err) } - if _, err = finalizeTypeScriptLibrary(langDir, "@acme/pdf-client"); err != nil { + if _, err = finalizeTypeScriptLibrary(langDir, "@acme/pdf-client", false); err != nil { t.Fatalf("second finalizeTypeScriptLibrary: %v", err) } index, err = os.ReadFile(filepath.Join(langDir, "src", "index.ts")) diff --git a/pkg/generators/facade_packages_test.go b/pkg/generators/facade_packages_test.go new file mode 100644 index 000000000..7c8779424 --- /dev/null +++ b/pkg/generators/facade_packages_test.go @@ -0,0 +1,189 @@ +package generators + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/codefly-dev/core/composition" + "github.com/codefly-dev/core/languages" +) + +// writeRaw writes files (slash-separated path → content) under a fresh +// directory: the shape proto.GenerateClient leaves for one entry. +func writeRaw(t *testing.T, files map[string]string) string { + t.Helper() + dir := t.TempDir() + for path, content := range files { + target := filepath.Join(dir, filepath.FromSlash(path)) + if err := os.MkdirAll(filepath.Dir(target), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(target, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + } + return dir +} + +// rawGoEntry is what the Go facade plugin emits for one contract whose proto +// sits at the contract's root (a service's proto/api.proto): the binding at +// raw's root, and a facade, in a package named after the facade entry-point, +// importing it through the plugin's hardcoded prefix. Two such entries write +// the same binding path. +func rawGoEntry(t *testing.T, module string) string { + t.Helper() + return writeRaw(t, map[string]string{ + "api.pb.go": "package " + module + "v1\n\ntype GetRequest struct{}\n", + module + "_facade.pb.go": "package " + module + "\n\nimport (\n\t" + module + "v1 \"github.com/codefly-dev/cli/pkg/builder/clients/" + module + "\"\n)\n\ntype Request = " + module + "v1.GetRequest\n", + }) +} + +func mergeGoFixture(t *testing.T, facadePackages bool) string { + t.Helper() + langDir := t.TempDir() + const goModule = "example.test/portal-sdk-go" + if err := os.WriteFile(filepath.Join(langDir, "go.mod"), []byte("module "+goModule+"\n\ngo "+goLanguageVersion+"\n"), 0o644); err != nil { + t.Fatal(err) + } + for _, module := range []string{"accounts", "records"} { + if err := mergeGoEntry(langDir, rawGoEntry(t, module), module, goModule, facadePackages); err != nil { + t.Fatalf("mergeGoEntry %s: %v", module, err) + } + } + return langDir +} + +// TestAggregatedGoLibraryGivesEachFacadeItsOwnPackage is the aggregated Go +// library with two consumed modules: each facade carries its own package +// clause, so at the go/ root they are two packages in one directory and the +// library does not build; each in its own directory, it does. +func TestAggregatedGoLibraryGivesEachFacadeItsOwnPackage(t *testing.T) { + t.Setenv("GOFLAGS", "") + t.Setenv("GOPROXY", "off") + + shared := mergeGoFixture(t, false) + err := resolveAndVerifyGoLibrary(shared) + if err == nil { + t.Fatal("two entries merged into one go/ root and one go/gen/ must not build") + } + + langDir := mergeGoFixture(t, true) + for _, module := range []string{"accounts", "records"} { + if _, err := os.Stat(filepath.Join(langDir, module, module+"_facade.pb.go")); err != nil { + t.Fatalf("facade of %s is not in its own package go/%s/: %v", module, module, err) + } + if _, err := os.Stat(filepath.Join(langDir, "gen", module, "api.pb.go")); err != nil { + t.Fatalf("bindings of %s are not under go/gen/%s/: %v", module, module, err) + } + } + if err := resolveAndVerifyGoLibrary(langDir); err != nil { + t.Fatalf("library with a package per facade does not build: %v", err) + } + + // A regenerate clears the facade packages it wrote, and only what it wrote. + note := filepath.Join(langDir, "records", "README.md") + if err := os.WriteFile(note, []byte("hand-written\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := wipeGoLibrary(langDir); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(filepath.Join(langDir, "accounts")); !os.IsNotExist(err) { + t.Fatalf("wipe left the generated facade package go/accounts/ behind: %v", err) + } + if _, err := os.Stat(filepath.Join(langDir, "records", "records_facade.pb.go")); !os.IsNotExist(err) { + t.Fatalf("wipe left a generated facade behind: %v", err) + } + if _, err := os.Stat(note); err != nil { + t.Fatalf("wipe removed a hand-added file: %v", err) + } +} + +// rawTypeScriptEntry is what protoc-gen-es and the TypeScript facade plugin +// emit for one contract rooted at api.proto. Both fixture contracts write +// api_pb.ts and declare GetRequestSchema: one gen/ cannot hold both files, and +// one barrel re-exporting both cannot resolve the name. +func rawTypeScriptEntry(t *testing.T, module string) string { + t.Helper() + return writeRaw(t, map[string]string{ + "api_pb.ts": "export const GetRequestSchema = {};\n", + module + "_facade.ts": "import { GetRequestSchema } from \"./api_pb.js\";\nexport const " + module + " = { GetRequestSchema };\n", + }) +} + +func TestAggregatedTypeScriptLibraryExportsEachFacadePackage(t *testing.T) { + langDir := t.TempDir() + for _, module := range []string{"accounts", "records"} { + if err := mergeTypeScriptEntry(langDir, rawTypeScriptEntry(t, module), module, true); err != nil { + t.Fatalf("mergeTypeScriptEntry %s: %v", module, err) + } + } + if _, err := finalizeTypeScriptLibrary(langDir, "@acme/portal-sdk", true); err != nil { + t.Fatalf("finalizeTypeScriptLibrary: %v", err) + } + read := func(path string) string { + t.Helper() + data, err := os.ReadFile(filepath.Join(langDir, filepath.FromSlash(path))) + if err != nil { + t.Fatal(err) + } + return string(data) + } + + facade := read("src/accounts/accounts_facade.ts") + if !strings.Contains(facade, `from "../gen/accounts/./api_pb.js"`) { + t.Errorf("the facade does not import its bindings from its package directory:\n%s", facade) + } + + // The root re-exports the facades alone: their names are the facade + // entry-points, unique per library; the bindings' are not. + want := "// generated by codefly generate client; do not edit\n" + + "export * from \"./accounts/accounts_facade.js\";\n" + + "export * from \"./records/records_facade.js\";\n" + if got := read("src/index.ts"); got != want { + t.Errorf("src/index.ts =\n%s\nwant\n%s", got, want) + } + + // Each facade package re-exports its facade and its own contract's + // bindings, never another contract's. + want = "// generated by codefly generate client; do not edit\n" + + "export * from \"./accounts_facade.js\";\n" + + "export * from \"../gen/accounts/api_pb.js\";\n" + if got := read("src/accounts/index.ts"); got != want { + t.Errorf("src/accounts/index.ts =\n%s\nwant\n%s", got, want) + } + + // And package.json reaches each facade package by its directory. + if manifest := read("package.json"); !strings.Contains(manifest, `"./*": {`) || !strings.Contains(manifest, `"import": "./dist/*/index.js"`) { + t.Errorf("package.json does not export the facade packages:\n%s", manifest) + } +} + +// TestAggregatedLibraryRefusesTwoEntriesWithOneFacadeName: the facade name is +// a file and a package directory in every language, so the second entry +// would overwrite the first. Refused before any codegen. +func TestAggregatedLibraryRefusesTwoEntriesWithOneFacadeName(t *testing.T) { + entry := func(module, pkg string) ContractEntry { + return ContractEntry{ + ModuleName: module, + FacadeModuleDefault: "accounts", + Endpoint: composition.APIContractEndpoint{ + Service: "accounts", Endpoint: "grpc", Kind: composition.APIContractKindProtobuf, + Package: pkg, Digest: "sha256:" + module, + }, + } + } + err := GenerateClientLibrary(context.Background(), &ClientLibraryRequest{ + Name: "portal-sdk", + Output: t.TempDir(), + Languages: []languages.Language{languages.GO}, + Entries: []ContractEntry{entry("host", "host.accounts.v1"), entry("other", "other.accounts.v1")}, + FacadePackages: true, + }) + if err == nil || !strings.Contains(err.Error(), `both generate the facade "accounts"`) { + t.Fatalf("error = %v, want the shared facade name refused", err) + } +}