chore: remove redundant function (#2614)

- Remove `findLibraryByID` as `LibrarianState.LibraryByID` has provided
the same functionality.
- Adding unit tests to verify the nil cases.

Author: JoeWang1127

internal/librarian/command.go

@@ -101,7 +101,7 @@ type ContainerClient interface {
 type commitInfo struct {
 	// branch is the base branch of the created pull request.
 	branch string
-	// commit declares whether or not to create a commit.
+	// commit declares whether to create a commit.
 	commit bool
 	// commitMessage is used as the message on the actual git commit.
 	commitMessage string
@@ -111,7 +111,7 @@ type commitInfo struct {
 	prType pullRequestType
 	// pullRequestLabels is a list of labels to add to the created pull request.
 	pullRequestLabels []string
-	// push declares whether or not to push the commits to GitHub.
+	// push declares whether to push the commits to GitHub.
 	push bool
 	// languageRepo is the git repository containing the language-specific libraries.
 	languageRepo gitrepo.Repository
@@ -129,7 +129,7 @@ type commitInfo struct {
 	library string
 	// prBodyBuilder is a callback function for building the pull request body
 	prBodyBuilder func() (string, error)
-	// isDraft declares whether or not to create the pull request as a draft.
+	// isDraft declares whether to create the pull request as a draft.
 	isDraft bool
 }
 
@@ -269,18 +269,6 @@ func findLibraryIDByAPIPath(state *config.LibrarianState, apiPath string) string
 	return ""
 }
 
-func findLibraryByID(state *config.LibrarianState, libraryID string) *config.LibraryState {
-	if state == nil {
-		return nil
-	}
-	for _, lib := range state.Libraries {
-		if lib.ID == libraryID {
-			return lib
-		}
-	}
-	return nil
-}
-
 func formatTimestamp(t time.Time) string {
 	const yyyyMMddHHmmss = "20060102T150405Z" // Expected format by time library
 	return t.Format(yyyyMMddHHmmss)
@@ -289,7 +277,7 @@ func formatTimestamp(t time.Time) string {
 // cleanAndCopyLibrary cleans the files of the given library in repoDir and copies
 // the new files from outputDir.
 func cleanAndCopyLibrary(state *config.LibrarianState, repoDir, libraryID, outputDir string) error {
-	library := findLibraryByID(state, libraryID)
+	library := state.LibraryByID(libraryID)
 	if library == nil {
 		return fmt.Errorf("library %q not found during clean and copy, despite being found in earlier steps", libraryID)
 	}
@@ -320,7 +308,7 @@ func cleanAndCopyLibrary(state *config.LibrarianState, repoDir, libraryID, outpu
 // If a file is being copied to the library's SourceRoots in the dest folder but the folder does
 // not exist, the copy fails.
 func copyLibraryFiles(state *config.LibrarianState, dest, libraryID, src string) error {
-	library := findLibraryByID(state, libraryID)
+	library := state.LibraryByID(libraryID)
 	if library == nil {
 		return fmt.Errorf("library %q not found", libraryID)
 	}

internal/librarian/command_test.go

@@ -32,56 +32,6 @@ import (
 	"github.com/googleapis/librarian/internal/gitrepo"
 )
 
-func TestFindLibraryByID(t *testing.T) {
-	lib1 := &config.LibraryState{ID: "lib1"}
-	lib2 := &config.LibraryState{ID: "lib2"}
-	stateWithLibs := &config.LibrarianState{
-		Libraries: []*config.LibraryState{lib1, lib2},
-	}
-	stateNoLibs := &config.LibrarianState{
-		Libraries: []*config.LibraryState{},
-	}
-
-	for _, test := range []struct {
-		name      string
-		state     *config.LibrarianState
-		libraryID string
-		want      *config.LibraryState
-	}{
-		{
-			name:      "found first library",
-			state:     stateWithLibs,
-			libraryID: "lib1",
-			want:      lib1,
-		},
-		{
-			name:      "found second library",
-			state:     stateWithLibs,
-			libraryID: "lib2",
-			want:      lib2,
-		},
-		{
-			name:      "not found",
-			state:     stateWithLibs,
-			libraryID: "lib3",
-			want:      nil,
-		},
-		{
-			name:      "empty libraries slice",
-			state:     stateNoLibs,
-			libraryID: "lib1",
-			want:      nil,
-		},
-	} {
-		t.Run(test.name, func(t *testing.T) {
-			got := findLibraryByID(test.state, test.libraryID)
-			if diff := cmp.Diff(test.want, got); diff != "" {
-				t.Errorf("findLibraryByID() mismatch (-want +got):\n%s", diff)
-			}
-		})
-	}
-}
-
 func TestDeriveImage(t *testing.T) {
 	for _, test := range []struct {
 		name          string

internal/librarian/generate_command.go

@@ -226,7 +226,7 @@ func (r *generateRunner) generateSingleLibrary(ctx context.Context, libraryID, o
 	}
 
 	// At this point, we should have a library in the state.
-	libraryState := findLibraryByID(r.state, libraryID)
+	libraryState := r.state.LibraryByID(libraryID)
 	if libraryState == nil {
 		return nil, fmt.Errorf("library %q not configured yet, generation stopped", libraryID)
 	}
@@ -261,7 +261,7 @@ func (r *generateRunner) generateSingleLibrary(ctx context.Context, libraryID, o
 }
 
 func (r *generateRunner) needsConfigure() bool {
-	return r.api != "" && r.library != "" && findLibraryByID(r.state, r.library) == nil
+	return r.api != "" && r.library != "" && r.state.LibraryByID(r.library) == nil
 }
 
 func (r *generateRunner) updateLastGeneratedCommitState(libraryID string) error {
@@ -375,7 +375,11 @@ func (r *generateRunner) runConfigureCommand(ctx context.Context, outputDir stri
 
 // getExistingSrc returns source roots as-is of a given library ID, if the source roots exist in the language repo.
 func (r *generateRunner) getExistingSrc(libraryID string) []string {
-	library := findLibraryByID(r.state, libraryID)
+	library := r.state.LibraryByID(libraryID)
+	if library == nil {
+		return nil
+	}
+
 	var existingSrc []string
 	for _, src := range library.SourceRoots {
 		relPath := filepath.Join(r.repo.GetDir(), src)

internal/librarian/generate_command_test.go

@@ -1059,6 +1059,81 @@ func TestGenerateSingleLibraryCommand(t *testing.T) {
 	}
 }
 
+func TestGetExistingSrc(t *testing.T) {
+	t.Parallel()
+	for _, test := range []struct {
+		name      string
+		libraryID string
+		paths     []string
+		want      []string
+	}{
+		{
+			name:      "all_source_paths_existed",
+			libraryID: "some-library",
+			paths: []string{
+				"a/path",
+				"another/path",
+			},
+			want: []string{
+				"a/path",
+				"another/path",
+			},
+		},
+		{
+			name:      "one_source_paths_existed",
+			libraryID: "some-library",
+			paths: []string{
+				"a/path",
+			},
+			want: []string{
+				"a/path",
+			},
+		},
+		{
+			name:      "no_source_paths_existed",
+			libraryID: "some-library",
+			want:      nil,
+		},
+		{
+			name:      "no_library_existed",
+			libraryID: "another-library",
+			want:      nil,
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+			repo := newTestGitRepo(t)
+			state := &config.LibrarianState{
+				Libraries: []*config.LibraryState{
+					{
+						ID: "some-library",
+						SourceRoots: []string{
+							"a/path",
+							"another/path",
+						},
+					},
+				},
+			}
+			for _, path := range test.paths {
+				relPath := filepath.Join(repo.GetDir(), path)
+				if err := os.MkdirAll(relPath, 0755); err != nil {
+					t.Fatal(err)
+				}
+			}
+
+			r := &generateRunner{
+				repo:  repo,
+				state: state,
+			}
+
+			got := r.getExistingSrc(test.libraryID)
+			if diff := cmp.Diff(test.want, got); diff != "" {
+				t.Errorf("getExistingSrc() mismatch (-want +got):%s", diff)
+			}
+		})
+	}
+}
+
 func TestUpdateLastGeneratedCommitState(t *testing.T) {
 	t.Parallel()
 	sourceRepo := newTestGitRepo(t)

internal/librarian/generate_pull_request.go

@@ -164,7 +164,10 @@ func formatOnboardPRBody(request *onboardPRRequest) (string, error) {
 
 // getPiperID extracts the Piper ID from the commit message that onboarded the API.
 func getPiperID(state *config.LibrarianState, sourceRepo gitrepo.Repository, apiPath, library string) (string, error) {
-	libraryState := findLibraryByID(state, library)
+	libraryState := state.LibraryByID(library)
+	if libraryState == nil {
+		return "", fmt.Errorf("library %s not found", library)
+	}
 	serviceYaml := ""
 	for _, api := range libraryState.APIs {
 		if api.Path == apiPath {

internal/librarian/generate_pull_request_test.go

@@ -662,6 +662,20 @@ Language Image: %s`,
 			wantErr:       true,
 			wantErrPhrase: errPiperNotFound.Error(),
 		},
+		{
+			name: "library_not_found_in_state",
+			state: &config.LibrarianState{
+				Image: "go:1.21",
+				Libraries: []*config.LibraryState{
+					{
+						ID: "one-library",
+					},
+				},
+			},
+			library:       "another-library",
+			wantErr:       true,
+			wantErrPhrase: "library another-library not found",
+		},
 	} {
 		t.Run(test.name, func(t *testing.T) {
 			req := &onboardPRRequest{

internal/librarian/generate_test.go

@@ -15,8 +15,6 @@
 package librarian
 
 import (
-	"os"
-	"path/filepath"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -92,72 +90,6 @@ func TestGenerateSingleLibrary(t *testing.T) {
 	}
 }
 
-func TestGetExistingSrc(t *testing.T) {
-	t.Parallel()
-	for _, test := range []struct {
-		name  string
-		paths []string
-		want  []string
-	}{
-		{
-			name: "all_source_paths_existed",
-			paths: []string{
-				"a/path",
-				"another/path",
-			},
-			want: []string{
-				"a/path",
-				"another/path",
-			},
-		},
-		{
-			name: "one_source_paths_existed",
-			paths: []string{
-				"a/path",
-			},
-			want: []string{
-				"a/path",
-			},
-		},
-		{
-			name: "no_source_paths_existed",
-			want: nil,
-		},
-	} {
-		t.Run(test.name, func(t *testing.T) {
-			t.Parallel()
-			repo := newTestGitRepo(t)
-			state := &config.LibrarianState{
-				Libraries: []*config.LibraryState{
-					{
-						ID: "some-library",
-						SourceRoots: []string{
-							"a/path",
-							"another/path",
-						},
-					},
-				},
-			}
-			for _, path := range test.paths {
-				relPath := filepath.Join(repo.GetDir(), path)
-				if err := os.MkdirAll(relPath, 0755); err != nil {
-					t.Fatal(err)
-				}
-			}
-
-			r := &generateRunner{
-				repo:  repo,
-				state: state,
-			}
-
-			got := r.getExistingSrc("some-library")
-			if diff := cmp.Diff(test.want, got); diff != "" {
-				t.Errorf("getExistingSrc() mismatch (-want +got):%s", diff)
-			}
-		})
-	}
-}
-
 func TestGetSafeDirectoryName(t *testing.T) {
 	for _, test := range []struct {
 		name string

internal/librarian/release_init.go

@@ -133,7 +133,7 @@ func (r *initRunner) runInitCommand(ctx context.Context, outputDir string) error
 	src := r.repo.GetDir()
 	librariesToRelease := r.state.Libraries
 	if r.library != "" {
-		library := findLibraryByID(r.state, r.library)
+		library := r.state.LibraryByID(r.library)
 		if library == nil {
 			return fmt.Errorf("unable to find library for release: %s", r.library)
 		}