From 0db9043aa76db098bcb4b962674bc9ca92fc84c9 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 10 Sep 2020 16:27:49 +0800 Subject: [PATCH] return error when create gitlabdownloader (#12790) --- modules/migrations/gitlab.go | 28 ++++++---------------------- modules/migrations/gitlab_test.go | 7 ++++--- 2 files changed, 10 insertions(+), 25 deletions(-) diff --git a/modules/migrations/gitlab.go b/modules/migrations/gitlab.go index 7e7d88b702d..33ccc4dcadb 100644 --- a/modules/migrations/gitlab.go +++ b/modules/migrations/gitlab.go @@ -47,7 +47,7 @@ func (f *GitlabDownloaderFactory) New(ctx context.Context, opts base.MigrateOpti log.Trace("Create gitlab downloader. BaseURL: %s RepoName: %s", baseURL, repoNameSpace) - return NewGitlabDownloader(ctx, baseURL, repoNameSpace, opts.AuthUsername, opts.AuthPassword, opts.AuthToken), nil + return NewGitlabDownloader(ctx, baseURL, repoNameSpace, opts.AuthUsername, opts.AuthPassword, opts.AuthToken) } // GitServiceType returns the type of git service @@ -73,7 +73,7 @@ type GitlabDownloader struct { // NewGitlabDownloader creates a gitlab Downloader via gitlab API // Use either a username/password, personal token entered into the username field, or anonymous/public access // Note: Public access only allows very basic access -func NewGitlabDownloader(ctx context.Context, baseURL, repoPath, username, password, token string) *GitlabDownloader { +func NewGitlabDownloader(ctx context.Context, baseURL, repoPath, username, password, token string) (*GitlabDownloader, error) { var gitlabClient *gitlab.Client var err error if token != "" { @@ -84,19 +84,19 @@ func NewGitlabDownloader(ctx context.Context, baseURL, repoPath, username, passw if err != nil { log.Trace("Error logging into gitlab: %v", err) - return nil + return nil, err } // Grab and store project/repo ID here, due to issues using the URL escaped path gr, _, err := gitlabClient.Projects.GetProject(repoPath, nil, nil, gitlab.WithContext(ctx)) if err != nil { log.Trace("Error retrieving project: %v", err) - return nil + return nil, err } if gr == nil { log.Trace("Error getting project, project is nil") - return nil + return nil, errors.New("Error getting project, project is nil") } return &GitlabDownloader{ @@ -104,7 +104,7 @@ func NewGitlabDownloader(ctx context.Context, baseURL, repoPath, username, passw client: gitlabClient, repoID: gr.ID, repoName: gr.Name, - } + }, nil } // SetContext set context @@ -114,10 +114,6 @@ func (g *GitlabDownloader) SetContext(ctx context.Context) { // GetRepoInfo returns a repository information func (g *GitlabDownloader) GetRepoInfo() (*base.Repository, error) { - if g == nil { - return nil, errors.New("error: GitlabDownloader is nil") - } - gr, _, err := g.client.Projects.GetProject(g.repoID, nil, nil, gitlab.WithContext(g.ctx)) if err != nil { return nil, err @@ -154,10 +150,6 @@ func (g *GitlabDownloader) GetRepoInfo() (*base.Repository, error) { // GetTopics return gitlab topics func (g *GitlabDownloader) GetTopics() ([]string, error) { - if g == nil { - return nil, errors.New("error: GitlabDownloader is nil") - } - gr, _, err := g.client.Projects.GetProject(g.repoID, nil, nil, gitlab.WithContext(g.ctx)) if err != nil { return nil, err @@ -167,9 +159,6 @@ func (g *GitlabDownloader) GetTopics() ([]string, error) { // GetMilestones returns milestones func (g *GitlabDownloader) GetMilestones() ([]*base.Milestone, error) { - if g == nil { - return nil, errors.New("error: GitlabDownloader is nil") - } var perPage = 100 var state = "all" var milestones = make([]*base.Milestone, 0, perPage) @@ -228,9 +217,6 @@ func (g *GitlabDownloader) GetMilestones() ([]*base.Milestone, error) { // GetLabels returns labels func (g *GitlabDownloader) GetLabels() ([]*base.Label, error) { - if g == nil { - return nil, errors.New("error: GitlabDownloader is nil") - } var perPage = 100 var labels = make([]*base.Label, 0, perPage) for i := 1; ; i++ { @@ -466,7 +452,6 @@ func (g *GitlabDownloader) GetComments(issueNumber int64) ([]*base.Comment, erro // GetPullRequests returns pull requests according page and perPage func (g *GitlabDownloader) GetPullRequests(page, perPage int) ([]*base.PullRequest, error) { - opt := &gitlab.ListProjectMergeRequestsOptions{ ListOptions: gitlab.ListOptions{ PerPage: perPage, @@ -576,7 +561,6 @@ func (g *GitlabDownloader) GetPullRequests(page, perPage int) ([]*base.PullReque // GetReviews returns pull requests review func (g *GitlabDownloader) GetReviews(pullRequestNumber int64) ([]*base.Review, error) { - state, _, err := g.client.MergeRequestApprovals.GetApprovalState(g.repoID, int(pullRequestNumber), gitlab.WithContext(g.ctx)) if err != nil { return nil, err diff --git a/modules/migrations/gitlab_test.go b/modules/migrations/gitlab_test.go index 065a9a65903..936201b1f25 100644 --- a/modules/migrations/gitlab_test.go +++ b/modules/migrations/gitlab_test.go @@ -6,6 +6,7 @@ package migrations import ( "context" + "fmt" "net/http" "os" "testing" @@ -28,9 +29,9 @@ func TestGitlabDownloadRepo(t *testing.T) { t.Skipf("Can't access test repo, skipping %s", t.Name()) } - downloader := NewGitlabDownloader(context.Background(), "https://gitlab.com", "gitea/test_repo", "", "", gitlabPersonalAccessToken) - if downloader == nil { - t.Fatal("NewGitlabDownloader is nil") + downloader, err := NewGitlabDownloader(context.Background(), "https://gitlab.com", "gitea/test_repo", "", "", gitlabPersonalAccessToken) + if err != nil { + t.Fatal(fmt.Sprintf("NewGitlabDownloader is nil: %v", err)) } repo, err := downloader.GetRepoInfo() assert.NoError(t, err)