diff --git a/internal/librarian/add.go b/internal/librarian/add.go index b09f7aa372a..02dd0edd544 100644 --- a/internal/librarian/add.go +++ b/internal/librarian/add.go @@ -243,7 +243,11 @@ func addNewLibrary(cfg *config.Config, api *config.API) (string, *config.Config, case config.LanguageGo: lib = golang.Add(lib) case config.LanguageJava: - lib = java.Add(lib) + var err error + lib, err = java.Add(lib, nil) + if err != nil { + return "", nil, err + } case config.LanguagePython: var err error lib, err = python.Add(cfg, lib) @@ -275,8 +279,15 @@ func updateExistingLibrary(cfg *config.Config, existingLib *config.Library, api case config.LanguageGo: existingLib.APIs = append(existingLib.APIs, api) existingLib = golang.Add(existingLib) - case config.LanguageJava, config.LanguageNodejs: + case config.LanguageNodejs: existingLib.APIs = append(existingLib.APIs, api) + case config.LanguageJava: + existingLib.APIs = append(existingLib.APIs, api) + var err error + existingLib, err = java.Add(existingLib, api) + if err != nil { + return "", nil, err + } default: return "", nil, fmt.Errorf("%w: %s", errLibraryAlreadyExists, existingLib.Name) } diff --git a/internal/librarian/add_test.go b/internal/librarian/add_test.go index 39c9edbdd90..16f160013b3 100644 --- a/internal/librarian/add_test.go +++ b/internal/librarian/add_test.go @@ -16,6 +16,7 @@ package librarian import ( "errors" + "os" "path/filepath" "sort" "strconv" @@ -165,6 +166,9 @@ func TestAddCommand(t *testing.T) { t.Run(test.name, func(t *testing.T) { tmpDir := t.TempDir() t.Chdir(tmpDir) + if err := os.WriteFile(filepath.Join(tmpDir, "versions.txt"), nil, 0644); err != nil { + t.Fatal(err) + } cfg := sample.Config() cfg.Default.Output = "output" @@ -220,6 +224,9 @@ func TestAddLibrary(t *testing.T) { t.Run(test.name, func(t *testing.T) { tmpDir := t.TempDir() t.Chdir(tmpDir) + if err := os.WriteFile(filepath.Join(tmpDir, "versions.txt"), nil, 0644); err != nil { + t.Fatal(err) + } cfg := sample.Config() cfg.Libraries = []*config.Library{ @@ -395,6 +402,7 @@ func TestAddLibrary_ExistingLibrary(t *testing.T) { {Path: "google/cloud/secretmanager/v1"}, {Path: "google/cloud/secretmanager/v1beta2"}, }, + Java: &config.JavaModule{ReleasedVersion: "1.2.3"}, }, }, }, @@ -403,6 +411,9 @@ func TestAddLibrary_ExistingLibrary(t *testing.T) { t.Run(test.name, func(t *testing.T) { tmpDir := t.TempDir() t.Chdir(tmpDir) + if err := os.WriteFile(filepath.Join(tmpDir, "versions.txt"), nil, 0644); err != nil { + t.Fatal(err) + } if err := yaml.Write(config.LibrarianYAML, test.cfg); err != nil { t.Fatal(err) } @@ -449,6 +460,9 @@ func TestAddLibrary_ExistingLibrary_Error(t *testing.T) { t.Run(test.name, func(t *testing.T) { tmpDir := t.TempDir() t.Chdir(tmpDir) + if err := os.WriteFile(filepath.Join(tmpDir, "versions.txt"), nil, 0644); err != nil { + t.Fatal(err) + } if err := yaml.Write(config.LibrarianYAML, test.cfg); err != nil { t.Fatal(err) } @@ -593,6 +607,9 @@ func TestAddLibraryCommand_Java(t *testing.T) { } tmpDir := t.TempDir() t.Chdir(tmpDir) + if err := os.WriteFile(filepath.Join(tmpDir, "versions.txt"), nil, 0644); err != nil { + t.Fatal(err) + } cfg := sample.Config() cfg.Language = config.LanguageJava diff --git a/internal/librarian/generate.go b/internal/librarian/generate.go index 2243bb53d2a..f654c3015fe 100644 --- a/internal/librarian/generate.go +++ b/internal/librarian/generate.go @@ -237,16 +237,7 @@ func generateLibraries(ctx context.Context, cfg *config.Config, libraries []*con } return g.Wait() case config.LanguageJava: - var allMissingArtifacts []java.MissingArtifact for _, library := range libraries { - missingArtifactIDs, err := java.IdentifyMissingModules(library, library.Output) - if err != nil { - return fmt.Errorf("failed to identify missing modules for %q: %w", library.Name, err) - } - for _, id := range missingArtifactIDs { - allMissingArtifacts = append(allMissingArtifacts, java.MissingArtifact{ID: id, Library: library}) - } - if err := java.Generate(ctx, cfg, library, src); err != nil { return fmt.Errorf("generate library %q (%s): %w", library.Name, cfg.Language, err) } @@ -254,7 +245,7 @@ func generateLibraries(ctx context.Context, cfg *config.Config, libraries []*con return fmt.Errorf("format library %q (%s): %w", library.Name, cfg.Language, err) } } - return java.PostGenerate(ctx, ".", cfg, allMissingArtifacts) + return java.PostGenerate(ctx, ".", cfg) case config.LanguageNodejs: g, gctx := errgroup.WithContext(ctx) for _, library := range libraries { diff --git a/internal/librarian/java/add.go b/internal/librarian/java/add.go index c38f1bcd22c..46c3b31ddd9 100644 --- a/internal/librarian/java/add.go +++ b/internal/librarian/java/add.go @@ -15,10 +15,15 @@ package java import ( + "bytes" + "fmt" "log" + "os" + "slices" "strings" "github.com/googleapis/librarian/internal/config" + "github.com/googleapis/librarian/internal/serviceconfig" ) // knownPrefixes contains API path prefixes to be stripped when deriving a @@ -35,11 +40,16 @@ const ( defaultVersion = "0.1.0-SNAPSHOT" defaultReleasedVersion = "0.0.0" fakeGroupID = "please-configure-java-group-id" + versionsFileName = "versions.txt" ) -// Add initializes a new Java library with default values. -func Add(lib *config.Library) *config.Library { - lib.Version = defaultVersion +// Add initializes a new Java library with default values, or extends an +// existing library with a new API path, and registers the appropriate +// modules in versions.txt. +func Add(lib *config.Library, addedAPI *config.API) (*config.Library, error) { + if lib.Version == "" { + lib.Version = defaultVersion + } // Java generation defaults to the system year for license headers, // so we reset it here to avoid redundancy in librarian.yaml. lib.CopyrightYear = "" @@ -47,28 +57,201 @@ func Add(lib *config.Library) *config.Library { if lib.Java == nil { lib.Java = &config.JavaModule{} } - lib.Java.ReleasedVersion = defaultReleasedVersion + if lib.Java.ReleasedVersion == "" && addedAPI == nil { + lib.Java.ReleasedVersion = defaultReleasedVersion + } // We use the first API to infer the group ID. // It is unrealistic for a single library to mix cloud and non-cloud APIs. apiPath := lib.APIs[0].Path switch { case strings.HasPrefix(apiPath, "google/shopping/"): - return setNonCloudMavenDefaults(lib, "com.google.shopping") + lib = setNonCloudMavenDefaults(lib, "com.google.shopping") case strings.HasPrefix(apiPath, "google/maps/"): - return setNonCloudMavenDefaults(lib, "com.google.maps") + lib = setNonCloudMavenDefaults(lib, "com.google.maps") case strings.HasPrefix(apiPath, "google/ads/"): - return setNonCloudMavenDefaults(lib, "com.google.api-ads") + lib = setNonCloudMavenDefaults(lib, "com.google.api-ads") + default: + if !strings.HasPrefix(apiPath, "google/cloud/") { + log.Printf( + "WARNING: unrecognized non-cloud API path %q. Setting fake GroupID %q. "+ + "Please manually configure java.group_id and java.distribution_name_override in librarian.yaml.", + apiPath, fakeGroupID, + ) + lib = setNonCloudMavenDefaults(lib, fakeGroupID) + } } - if !strings.HasPrefix(apiPath, "google/cloud/") { - log.Printf( - "WARNING: unrecognized non-cloud API path %q. Setting fake GroupID %q. "+ - "Please manually configure java.group_id and java.distribution_name_override in librarian.yaml.", - apiPath, fakeGroupID, - ) - setNonCloudMavenDefaults(lib, fakeGroupID) + + // Fill the library entry in order to derive the appropriate artifacts to add. + lib, err := Fill(lib) + if err != nil { + return nil, err } - return lib + + newArtifactIDs, err := deriveAddedArtifactIDs(lib, addedAPI) + if err != nil { + return nil, err + } + var versions []string + for _, id := range newArtifactIDs { + versions = append(versions, fmt.Sprintf("%s:%s:%s", id, lib.Java.ReleasedVersion, lib.Version)) + } + if err := appendVersions(versions); err != nil { + return nil, err + } + + lib, err = Tidy(lib) + if err != nil { + return nil, err + } + + return lib, nil +} + +func deriveAddedArtifactIDs(lib *config.Library, addedAPI *config.API) ([]string, error) { + libCoord := deriveLibraryCoordinates(lib) + var modules []expectedModule + + if addedAPI != nil { + transport, err := serviceconfig.FindTransport(addedAPI.Path, config.LanguageJava) + if err != nil { + return nil, err + } + modules = expectedAPIModules(lib, addedAPI, libCoord, transport) + } else { + var err error + modules, err = expectedNewLibraryModules(lib) + if err != nil { + return nil, err + } + } + + var artifacts []string + for _, m := range modules { + artifacts = append(artifacts, m.ArtifactID) + } + + if lib.Java != nil && len(lib.Java.ExcludedPOMs) > 0 { + var filtered []string + for _, art := range artifacts { + if !slices.Contains(lib.Java.ExcludedPOMs, art) { + filtered = append(filtered, art) + } + } + artifacts = filtered + } + + return artifacts, nil +} + +// expectedNewLibraryModules returns all expected modules for a new library, +// ordered to match the versions.txt expectation: Parent, BOM, APIs, Client. +func expectedNewLibraryModules(lib *config.Library) ([]expectedModule, error) { + transports, err := loadTransports(lib) + if err != nil { + return nil, err + } + modules := expectedModules(lib, transports) + + // Reorder modules to match versions.txt expectation: Parent, BOM, APIs, Client. + var parent *expectedModule + var bom *expectedModule + var client *expectedModule + var apis []expectedModule + + for _, m := range modules { + switch m.Kind { + case kindParent: + parent = &m + case kindBOM: + bom = &m + case kindClient: + client = &m + default: // kindProto, kindGRPC + apis = append(apis, m) + } + } + + var ordered []expectedModule + if parent != nil { + ordered = append(ordered, *parent) + } + if bom != nil { + ordered = append(ordered, *bom) + } + ordered = append(ordered, apis...) + if client != nil { + ordered = append(ordered, *client) + } + return ordered, nil +} + +// readExistingModules reads and parses the versions file at the given path, +// returning a map of existing module artifact IDs to true. +func readExistingModules(path string) (map[string]bool, error) { + content, err := os.ReadFile(path) + if err != nil { + if os.IsNotExist(err) { + return nil, nil + } + return nil, err + } + modules := make(map[string]bool) + lines := bytes.Split(content, []byte("\n")) + for _, line := range lines { + line = bytes.TrimSpace(line) + if len(line) == 0 || bytes.HasPrefix(line, []byte("#")) { + continue + } + parts := bytes.Split(line, []byte(":")) + if len(parts) > 0 && len(parts[0]) > 0 { + modules[string(parts[0])] = true + } + } + return modules, nil +} + +func appendVersions(versions []string) error { + existing, err := readExistingModules(versionsFileName) + if err != nil { + return fmt.Errorf("failed to read %s: %w", versionsFileName, err) + } + var newVersions []string + for _, line := range versions { + parts := strings.Split(line, ":") + if len(parts) > 0 && !existing[parts[0]] { + newVersions = append(newVersions, line) + } + } + if err := appendLines(versionsFileName, newVersions); err != nil { + return fmt.Errorf("failed to update %s: %w", versionsFileName, err) + } + return nil +} + +// appendLines appends the given lines to an existing file, ensuring that it +// ends with a newline character before appending. It returns an error if the +// file does not exist. +func appendLines(path string, lines []string) error { + if len(lines) == 0 { + return nil + } + existing, err := os.ReadFile(path) + if err != nil { + return err + } + var buf bytes.Buffer + buf.Write(existing) + // Ensure the file ends with a newline before appending so that we + // do not concatenate lines instead of appending them. + if len(existing) > 0 && existing[len(existing)-1] != '\n' { + buf.WriteByte('\n') + } + for _, line := range lines { + buf.WriteString(line) + buf.WriteByte('\n') + } + return os.WriteFile(path, buf.Bytes(), 0644) } func setNonCloudMavenDefaults(lib *config.Library, groupID string) *config.Library { diff --git a/internal/librarian/java/add_test.go b/internal/librarian/java/add_test.go index b9f8ef27896..d4f139e0c58 100644 --- a/internal/librarian/java/add_test.go +++ b/internal/librarian/java/add_test.go @@ -15,6 +15,8 @@ package java import ( + "os" + "strings" "testing" "github.com/google/go-cmp/cmp" @@ -42,9 +44,6 @@ func TestAdd(t *testing.T) { }, Version: defaultVersion, CopyrightYear: "", - Java: &config.JavaModule{ - ReleasedVersion: defaultReleasedVersion, - }, }, }, { @@ -63,9 +62,8 @@ func TestAdd(t *testing.T) { Version: defaultVersion, CopyrightYear: "", Java: &config.JavaModule{ - ArtifactID: "google-shopping-css", - GroupID: "com.google.shopping", - ReleasedVersion: defaultReleasedVersion, + ArtifactID: "google-shopping-css", + GroupID: "com.google.shopping", }, }, }, @@ -85,9 +83,8 @@ func TestAdd(t *testing.T) { Version: defaultVersion, CopyrightYear: "", Java: &config.JavaModule{ - ArtifactID: "google-maps-routing", - GroupID: "com.google.maps", - ReleasedVersion: defaultReleasedVersion, + ArtifactID: "google-maps-routing", + GroupID: "com.google.maps", }, }, }, @@ -107,9 +104,8 @@ func TestAdd(t *testing.T) { Version: defaultVersion, CopyrightYear: "", Java: &config.JavaModule{ - ArtifactID: "google-foo-bar", - GroupID: "please-configure-java-group-id", - ReleasedVersion: defaultReleasedVersion, + ArtifactID: "google-foo-bar", + GroupID: "please-configure-java-group-id", }, }, }, @@ -129,15 +125,22 @@ func TestAdd(t *testing.T) { Version: defaultVersion, CopyrightYear: "", Java: &config.JavaModule{ - ArtifactID: "google-ads-admanager", - GroupID: "com.google.api-ads", - ReleasedVersion: defaultReleasedVersion, + ArtifactID: "google-ads-admanager", + GroupID: "com.google.api-ads", }, }, }, } { t.Run(test.name, func(t *testing.T) { - got := Add(test.lib) + tmpDir := t.TempDir() + t.Chdir(tmpDir) + if err := os.WriteFile(versionsFileName, nil, 0644); err != nil { + t.Fatal(err) + } + got, err := Add(test.lib, nil) + if err != nil { + t.Fatalf("Add() error = %v", err) + } if diff := cmp.Diff(test.want, got); diff != "" { t.Errorf("mismatch (-want +got):\n%s", diff) } @@ -145,6 +148,171 @@ func TestAdd(t *testing.T) { } } +func TestAdd_VersionsTxt(t *testing.T) { + for _, test := range []struct { + name string + lib *config.Library + wantVersions []string + }{ + { + name: "standard cloud API", + lib: &config.Library{ + Name: "secretmanager", + APIs: []*config.API{ + {Path: "google/cloud/secretmanager/v1"}, + }, + }, + wantVersions: []string{ + "google-cloud-secretmanager-parent:0.0.0:0.1.0-SNAPSHOT", + "google-cloud-secretmanager-bom:0.0.0:0.1.0-SNAPSHOT", + "proto-google-cloud-secretmanager-v1:0.0.0:0.1.0-SNAPSHOT", + "grpc-google-cloud-secretmanager-v1:0.0.0:0.1.0-SNAPSHOT", + "google-cloud-secretmanager:0.0.0:0.1.0-SNAPSHOT", + }, + }, + { + name: "shopping API", + lib: &config.Library{ + Name: "shopping-css", + APIs: []*config.API{ + {Path: "google/shopping/css/v1"}, + }, + }, + wantVersions: []string{ + "google-shopping-css-parent:0.0.0:0.1.0-SNAPSHOT", + "google-shopping-css-bom:0.0.0:0.1.0-SNAPSHOT", + "proto-google-shopping-css-v1:0.0.0:0.1.0-SNAPSHOT", + "grpc-google-shopping-css-v1:0.0.0:0.1.0-SNAPSHOT", + "google-shopping-css:0.0.0:0.1.0-SNAPSHOT", + }, + }, + { + name: "maps API", + lib: &config.Library{ + Name: "maps-routing", + APIs: []*config.API{ + {Path: "google/maps/routing/v1"}, + }, + }, + wantVersions: []string{ + "google-maps-routing-parent:0.0.0:0.1.0-SNAPSHOT", + "google-maps-routing-bom:0.0.0:0.1.0-SNAPSHOT", + "proto-google-maps-routing-v1:0.0.0:0.1.0-SNAPSHOT", + "grpc-google-maps-routing-v1:0.0.0:0.1.0-SNAPSHOT", + "google-maps-routing:0.0.0:0.1.0-SNAPSHOT", + }, + }, + { + name: "unrecognized non-cloud API", + lib: &config.Library{ + Name: "foo-bar", + APIs: []*config.API{ + {Path: "google/foo/bar/v1"}, + }, + }, + wantVersions: []string{ + "google-foo-bar-parent:0.0.0:0.1.0-SNAPSHOT", + "google-foo-bar-bom:0.0.0:0.1.0-SNAPSHOT", + "proto-google-foo-bar-v1:0.0.0:0.1.0-SNAPSHOT", + "grpc-google-foo-bar-v1:0.0.0:0.1.0-SNAPSHOT", + "google-foo-bar:0.0.0:0.1.0-SNAPSHOT", + }, + }, + { + name: "ads API", + lib: &config.Library{ + Name: "ads-admanager", + APIs: []*config.API{ + {Path: "google/ads/admanager/v1"}, + }, + }, + wantVersions: []string{ + "google-ads-admanager-parent:0.0.0:0.1.0-SNAPSHOT", + "google-ads-admanager-bom:0.0.0:0.1.0-SNAPSHOT", + "proto-google-ads-admanager-v1:0.0.0:0.1.0-SNAPSHOT", + "google-ads-admanager:0.0.0:0.1.0-SNAPSHOT", + }, + }, + } { + t.Run(test.name, func(t *testing.T) { + tmpDir := t.TempDir() + t.Chdir(tmpDir) + if err := os.WriteFile(versionsFileName, nil, 0644); err != nil { + t.Fatal(err) + } + _, err := Add(test.lib, nil) + if err != nil { + t.Fatalf("Add() error = %v", err) + } + content, err := os.ReadFile(versionsFileName) + if err != nil { + t.Fatal(err) + } + var gotVersions []string + for _, line := range strings.Split(string(content), "\n") { + line = strings.TrimSpace(line) + if line != "" { + gotVersions = append(gotVersions, line) + } + } + if diff := cmp.Diff(test.wantVersions, gotVersions); diff != "" { + t.Errorf("versions mismatch (-want +got):\n%s", diff) + } + }) + } +} + +func TestAdd_ExistingLibrary(t *testing.T) { + tmpDir := t.TempDir() + t.Chdir(tmpDir) + // Write initial versions + initial := []string{ + "google-cloud-secretmanager-parent:1.0.0:1.1.0-SNAPSHOT", + "google-cloud-secretmanager-bom:1.0.0:1.1.0-SNAPSHOT", + "proto-google-cloud-secretmanager-v1:1.0.0:1.1.0-SNAPSHOT", + "grpc-google-cloud-secretmanager-v1:1.0.0:1.1.0-SNAPSHOT", + "google-cloud-secretmanager:1.0.0:1.1.0-SNAPSHOT", + } + if err := os.WriteFile(versionsFileName, []byte(strings.Join(initial, "\n")+"\n"), 0644); err != nil { + t.Fatal(err) + } + + lib := &config.Library{ + Name: "secretmanager", + Version: "1.1.0-SNAPSHOT", + APIs: []*config.API{ + {Path: "google/cloud/secretmanager/v1"}, + {Path: "google/cloud/secretmanager/v1beta1"}, + }, + } + addedAPI := &config.API{Path: "google/cloud/secretmanager/v1beta1"} + + got, err := Add(lib, addedAPI) + if err != nil { + t.Fatalf("Add() error = %v", err) + } + if got.Version != "1.1.0-SNAPSHOT" { + t.Errorf("version = %q, want %q", got.Version, "1.1.0-SNAPSHOT") + } + + content, err := os.ReadFile(versionsFileName) + if err != nil { + t.Fatal(err) + } + var gotVersions []string + for _, line := range strings.Split(string(content), "\n") { + line = strings.TrimSpace(line) + if line != "" { + gotVersions = append(gotVersions, line) + } + } + + wantVersions := append(initial, "proto-google-cloud-secretmanager-v1beta1:1.0.0:1.1.0-SNAPSHOT", "grpc-google-cloud-secretmanager-v1beta1:1.0.0:1.1.0-SNAPSHOT") + if diff := cmp.Diff(wantVersions, gotVersions); diff != "" { + t.Errorf("versions mismatch (-want +got):\n%s", diff) + } +} + func TestDefaultLibraryName(t *testing.T) { for _, test := range []struct { api string @@ -167,3 +335,72 @@ func TestDefaultLibraryName(t *testing.T) { }) } } + +func TestAppendVersions(t *testing.T) { + for _, test := range []struct { + name string + initial string + lines []string + want string + }{ + { + name: "empty file", + initial: "", + lines: []string{"a:1.0.0"}, + want: "a:1.0.0\n", + }, + { + name: "already has newline", + initial: "a:1.0.0\n", + lines: []string{"b:2.0.0"}, + want: "a:1.0.0\nb:2.0.0\n", + }, + { + name: "missing newline", + initial: "a:1.0.0", + lines: []string{"b:2.0.0"}, + want: "a:1.0.0\nb:2.0.0\n", + }, + { + name: "multiple lines missing newline", + initial: "a:1.0.0", + lines: []string{"b:2.0.0", "c:3.0.0"}, + want: "a:1.0.0\nb:2.0.0\nc:3.0.0\n", + }, + { + name: "no lines does nothing", + initial: "a:1.0.0\n", + lines: nil, + want: "a:1.0.0\n", + }, + } { + t.Run(test.name, func(t *testing.T) { + tmpDir := t.TempDir() + t.Chdir(tmpDir) + if err := os.WriteFile(versionsFileName, []byte(test.initial), 0644); err != nil { + t.Fatal(err) + } + if err := appendVersions(test.lines); err != nil { + t.Fatal(err) + } + got, err := os.ReadFile(versionsFileName) + if err != nil { + t.Fatal(err) + } + if diff := cmp.Diff(test.want, string(got)); diff != "" { + t.Errorf("mismatch (-want +got):\n%s", diff) + } + }) + } +} + +func TestAppendVersions_Error(t *testing.T) { + tmpDir := t.TempDir() + t.Chdir(tmpDir) + if err := os.Mkdir(versionsFileName, 0755); err != nil { + t.Fatal(err) + } + if err := appendVersions([]string{"line"}); err == nil { + t.Error("appendVersions() expected error, got nil") + } +} diff --git a/internal/librarian/java/pom.go b/internal/librarian/java/pom.go index e86d4de34f8..baac7227e6f 100644 --- a/internal/librarian/java/pom.go +++ b/internal/librarian/java/pom.go @@ -124,101 +124,96 @@ func loadTransports(library *config.Library) (map[string]serviceconfig.Transport return transports, nil } -func discoverModules(library *config.Library, libraryDir string, transports map[string]serviceconfig.Transport) ([]expectedModule, error) { - if library.Java != nil && library.Java.SkipPOMUpdates { - return nil, nil +func expectedAPIModules(library *config.Library, api *config.API, libCoord libraryCoordinate, transport serviceconfig.Transport) []expectedModule { + javaAPI := api.Java + if javaAPI == nil { + javaAPI = &config.JavaAPI{} } + apiBase := deriveAPIBase(library, api.Path) + apiCoord := deriveAPICoordinates(libCoord, apiBase, javaAPI) + + var modules []expectedModule + if shouldGenerateProto(javaAPI) { + modules = append(modules, expectedModule{ + ArtifactID: apiCoord.Proto.ArtifactID, + Kind: kindProto, + Coordinate: apiCoord.Proto, + APICoords: &apiCoord, + }) + } + if shouldGenerateGRPC(javaAPI) && transport != serviceconfig.Rest { + modules = append(modules, expectedModule{ + ArtifactID: apiCoord.GRPC.ArtifactID, + Kind: kindGRPC, + Coordinate: apiCoord.GRPC, + APICoords: &apiCoord, + }) + } + return modules +} + +func expectedModules(library *config.Library, transports map[string]serviceconfig.Transport) []expectedModule { var modules []expectedModule libCoord := deriveLibraryCoordinates(library) var shouldGenerateClient bool + for _, api := range library.APIs { javaAPI := api.Java if shouldGenerateGAPIC(javaAPI) || shouldGenerateResourceNames(javaAPI) { shouldGenerateClient = true } - apiBase := deriveAPIBase(library, api.Path) - apiCoord := deriveAPICoordinates(libCoord, apiBase, javaAPI) transport := transports[api.Path] - // Proto module - if shouldGenerateProto(javaAPI) { - protoDir := filepath.Join(libraryDir, apiCoord.Proto.ArtifactID) - isProtoMissing, err := isPOMMissing(protoDir) - if err != nil { - return nil, err - } - modules = append(modules, expectedModule{ - ArtifactID: apiCoord.Proto.ArtifactID, - Dir: protoDir, - Kind: kindProto, - IsMissing: isProtoMissing, - Coordinate: apiCoord.Proto, - APICoords: &apiCoord, - }) - } - // gRPC module - if shouldGenerateGRPC(javaAPI) && transport != serviceconfig.Rest { - gRPCDir := filepath.Join(libraryDir, apiCoord.GRPC.ArtifactID) - isGRPCMissing, err := isPOMMissing(gRPCDir) - if err != nil { - return nil, err - } - modules = append(modules, expectedModule{ - ArtifactID: apiCoord.GRPC.ArtifactID, - Dir: gRPCDir, - Kind: kindGRPC, - IsMissing: isGRPCMissing, - Coordinate: apiCoord.GRPC, - APICoords: &apiCoord, - }) - } + modules = append(modules, expectedAPIModules(library, api, libCoord, transport)...) } + // Client module if shouldGenerateClient { - clientDir := filepath.Join(libraryDir, libCoord.GAPIC.ArtifactID) - isClientMissing, err := isPOMMissing(clientDir) - if err != nil { - return nil, err - } modules = append(modules, expectedModule{ ArtifactID: libCoord.GAPIC.ArtifactID, - Dir: clientDir, Kind: kindClient, - IsMissing: isClientMissing, Coordinate: libCoord.GAPIC, }) } // BOM module - bomDir := filepath.Join(libraryDir, libCoord.BOM.ArtifactID) - isBOMMissing, err := isPOMMissing(bomDir) - if err != nil { - return nil, err - } modules = append(modules, expectedModule{ ArtifactID: libCoord.BOM.ArtifactID, - Dir: bomDir, Kind: kindBOM, - IsMissing: isBOMMissing, Coordinate: libCoord.BOM, }) // Parent module - parentDir := libraryDir - isParentMissing, err := isPOMMissing(parentDir) - if err != nil { - return nil, err - } modules = append(modules, expectedModule{ ArtifactID: libCoord.Parent.ArtifactID, - Dir: parentDir, Kind: kindParent, - IsMissing: isParentMissing, Coordinate: libCoord.Parent, }) + if library.Java == nil || len(library.Java.ExcludedPOMs) == 0 { - return modules, nil + return modules } return slices.DeleteFunc(modules, func(m expectedModule) bool { return slices.Contains(library.Java.ExcludedPOMs, m.ArtifactID) - }), nil + }) +} + +func discoverModules(library *config.Library, libraryDir string, transports map[string]serviceconfig.Transport) ([]expectedModule, error) { + if library.Java != nil && library.Java.SkipPOMUpdates { + return nil, nil + } + modules := expectedModules(library, transports) + for i := range modules { + m := &modules[i] + if m.Kind == kindParent { + m.Dir = libraryDir + } else { + m.Dir = filepath.Join(libraryDir, m.ArtifactID) + } + isMissing, err := isPOMMissing(m.Dir) + if err != nil { + return nil, err + } + m.IsMissing = isMissing + } + return modules, nil } // syncPOMs generates missing POMs and surgically updates existing client, BOM, diff --git a/internal/librarian/java/postgenerate.go b/internal/librarian/java/postgenerate.go index 974c80c9b40..c1cbf69ba8d 100644 --- a/internal/librarian/java/postgenerate.go +++ b/internal/librarian/java/postgenerate.go @@ -15,7 +15,6 @@ package java import ( - "bytes" "context" "encoding/xml" "errors" @@ -38,9 +37,6 @@ const ( // generated Bill of Materials (BOM) for all GAPIC libraries. gapicBOM = "gapic-libraries-bom" bomSuffix = "-bom" - // versionsFileName is the name of the manifest file that keeps track of - // artifact versions for release-please. - versionsFileName = "versions.txt" ) var ( @@ -76,12 +72,6 @@ type legacyBOM struct { artifactID string } -// MissingArtifact pairs an artifact ID with the library it was generated from. -type MissingArtifact struct { - ID string - Library *config.Library -} - type bomConfig struct { GroupID string ArtifactID string @@ -99,7 +89,7 @@ type mavenProject struct { } // PostGenerate performs repository-level actions after all individual Java libraries have been generated. -func PostGenerate(ctx context.Context, repoPath string, cfg *config.Config, missingArtifacts []MissingArtifact) error { +func PostGenerate(ctx context.Context, repoPath string, cfg *config.Config) error { monorepoVersion, err := findMonorepoVersion(cfg) if err != nil { return err @@ -115,12 +105,6 @@ func PostGenerate(ctx context.Context, repoPath string, cfg *config.Config, miss return fmt.Errorf("%s library not found in librarian.yaml", parentPOM) } - // TODO(https://github.com/googleapis/librarian/issues/5529): remove appending to versions.txt. - versions := constructVersionLines(missingArtifacts) - if err := appendVersions(repoPath, versions); err != nil { - return err - } - modules, err := searchForJavaModules(repoPath) if err != nil { return fmt.Errorf("%w: %w", errModuleDiscovery, err) @@ -138,47 +122,6 @@ func PostGenerate(ctx context.Context, repoPath string, cfg *config.Config, miss return nil } -func constructVersionLines(missingArtifacts []MissingArtifact) []string { - var lines []string - for _, ma := range missingArtifacts { - releasedVersion := ma.Library.Java.ReleasedVersion - lines = append(lines, fmt.Sprintf("%s:%s:%s", ma.ID, releasedVersion, ma.Library.Version)) - } - return lines -} - -func appendVersions(repoPath string, versions []string) error { - versionsPath := filepath.Join(repoPath, versionsFileName) - if err := appendLines(versionsPath, versions); err != nil { - return fmt.Errorf("failed to update %s: %w", versionsFileName, err) - } - return nil -} - -// appendLines appends the given lines to an existing file, ensuring that it -// ends with a newline character before appending. It returns an error if the -// file does not exist. -func appendLines(path string, lines []string) error { - if len(lines) == 0 { - return nil - } - existing, err := os.ReadFile(path) - if err != nil { - return err - } - var buf bytes.Buffer - buf.Write(existing) - // Ensure the file ends with a newline before appending. - if len(existing) > 0 && existing[len(existing)-1] != '\n' { - buf.WriteByte('\n') - } - for _, line := range lines { - buf.WriteString(line) - buf.WriteByte('\n') - } - return os.WriteFile(path, buf.Bytes(), 0644) -} - // searchForJavaModules scans top-level subdirectories in the repoPath for those that // contain a pom.xml file, excluding known non-library directories. Returns a sorted list of // subdirectory names as module names. diff --git a/internal/librarian/java/postgenerate_test.go b/internal/librarian/java/postgenerate_test.go index c2e21f3b3fd..c5faaeba2f9 100644 --- a/internal/librarian/java/postgenerate_test.go +++ b/internal/librarian/java/postgenerate_test.go @@ -47,7 +47,7 @@ func TestPostGenerate(t *testing.T) { {Name: "aiplatform", Version: "3.89.0"}, }, } - if err := PostGenerate(t.Context(), tmpDir, cfg, nil); err != nil { + if err := PostGenerate(t.Context(), tmpDir, cfg); err != nil { t.Fatal(err) } // Verify root pom.xml @@ -200,7 +200,7 @@ func TestPostGenerate_SearchError(t *testing.T) { {Name: parentPOM, Version: "1.2.3"}, }, } - err := PostGenerate(t.Context(), tmpDir, cfg, nil) + err := PostGenerate(t.Context(), tmpDir, cfg) if !errors.Is(err, errModuleDiscovery) { t.Errorf("got error %v, want %v", err, errModuleDiscovery) } @@ -220,7 +220,7 @@ func TestPostGenerate_Error(t *testing.T) { {Name: parentPOM, Version: "1.2.3"}, }, } - err := PostGenerate(t.Context(), tmpDir, cfg, nil) + err := PostGenerate(t.Context(), tmpDir, cfg) if !errors.Is(err, errRootPOMGeneration) { t.Errorf("got error %v, want %v", err, errRootPOMGeneration) } @@ -291,107 +291,3 @@ func copyDir(src, dest string) error { return filesystem.CopyFile(path, target) }) } - -func TestAppendVersions(t *testing.T) { - for _, test := range []struct { - name string - initial string - lines []string - want string - }{ - { - name: "empty file", - initial: "", - lines: []string{"a:1.0.0"}, - want: "a:1.0.0\n", - }, - { - name: "already has newline", - initial: "a:1.0.0\n", - lines: []string{"b:2.0.0"}, - want: "a:1.0.0\nb:2.0.0\n", - }, - { - name: "missing newline", - initial: "a:1.0.0", - lines: []string{"b:2.0.0"}, - want: "a:1.0.0\nb:2.0.0\n", - }, - { - name: "multiple lines missing newline", - initial: "a:1.0.0", - lines: []string{"b:2.0.0", "c:3.0.0"}, - want: "a:1.0.0\nb:2.0.0\nc:3.0.0\n", - }, - { - name: "no lines does nothing", - initial: "a:1.0.0\n", - lines: nil, - want: "a:1.0.0\n", - }, - } { - t.Run(test.name, func(t *testing.T) { - tmpDir := t.TempDir() - versionsPath := filepath.Join(tmpDir, versionsFileName) - if err := os.WriteFile(versionsPath, []byte(test.initial), 0644); err != nil { - t.Fatal(err) - } - if err := appendVersions(tmpDir, test.lines); err != nil { - t.Fatal(err) - } - got, err := os.ReadFile(versionsPath) - if err != nil { - t.Fatal(err) - } - if diff := cmp.Diff(test.want, string(got)); diff != "" { - t.Errorf("mismatch (-want +got):\n%s", diff) - } - }) - } -} - -func TestAppendVersions_Error(t *testing.T) { - t.Parallel() - if err := appendVersions("/non/existent/path", []string{"line"}); err == nil { - t.Error("appendVersions() expected error for non-existent file, got nil") - } -} - -func TestDeriveVersionLines(t *testing.T) { - library := &config.Library{ - Name: "secretmanager", - Version: "1.2.3", - Java: &config.JavaModule{ - ReleasedVersion: "1.2.3", - }, - } - for _, test := range []struct { - name string - missingArtifacts []MissingArtifact - want []string - }{ - { - name: "empty input", - missingArtifacts: nil, - want: nil, - }, - { - name: "valid artifact IDs", - missingArtifacts: []MissingArtifact{ - {ID: "proto-google-cloud-secretmanager-v1", Library: library}, - {ID: "google-cloud-secretmanager", Library: library}, - }, - want: []string{ - "proto-google-cloud-secretmanager-v1:1.2.3:1.2.3", - "google-cloud-secretmanager:1.2.3:1.2.3", - }, - }, - } { - t.Run(test.name, func(t *testing.T) { - got := constructVersionLines(test.missingArtifacts) - if diff := cmp.Diff(test.want, got); diff != "" { - t.Errorf("mismatch (-want +got):\n%s", diff) - } - }) - } -}