From 6a725b6f9cde28862869befb9d2b101d9e342427 Mon Sep 17 00:00:00 2001 From: Nanguan Lin <70063547+lng2020@users.noreply.github.com> Date: Wed, 20 Dec 2023 03:12:02 +0800 Subject: [PATCH] Remove deadcode under models/issues (#28536) Using the Go Official tool `golang.org/x/tools/cmd/deadcode@latest` mentioned by [go blog](https://go.dev/blog/deadcode). Just use `deadcode .` in the project root folder and it gives a list of unused functions. Though it has some false alarms. This PR removes dead code detected in `models/issues`. --- models/issues/assignees_test.go | 5 ++++- models/issues/issue.go | 9 --------- models/issues/issue_search.go | 20 -------------------- models/issues/issue_test.go | 30 ------------------------------ models/issues/label.go | 16 ---------------- models/issues/label_test.go | 24 ------------------------ models/issues/milestone_list.go | 26 -------------------------- models/issues/milestone_test.go | 29 ----------------------------- models/issues/pull.go | 30 ------------------------------ models/issues/stopwatch.go | 14 -------------- services/issue/assignee_test.go | 6 +++++- 11 files changed, 9 insertions(+), 200 deletions(-) diff --git a/models/issues/assignees_test.go b/models/issues/assignees_test.go index 3898e814c3..2c33efd99e 100644 --- a/models/issues/assignees_test.go +++ b/models/issues/assignees_test.go @@ -18,7 +18,10 @@ func TestUpdateAssignee(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) // Fake issue with assignees - issue, err := issues_model.GetIssueWithAttrsByID(db.DefaultContext, 1) + issue, err := issues_model.GetIssueByID(db.DefaultContext, 1) + assert.NoError(t, err) + + err = issue.LoadAttributes(db.DefaultContext) assert.NoError(t, err) // Assign multiple users diff --git a/models/issues/issue.go b/models/issues/issue.go index b0ff0adddd..90aad10bb9 100644 --- a/models/issues/issue.go +++ b/models/issues/issue.go @@ -534,15 +534,6 @@ func GetIssueByID(ctx context.Context, id int64) (*Issue, error) { return issue, nil } -// GetIssueWithAttrsByID returns an issue with attributes by given ID. -func GetIssueWithAttrsByID(ctx context.Context, id int64) (*Issue, error) { - issue, err := GetIssueByID(ctx, id) - if err != nil { - return nil, err - } - return issue, issue.LoadAttributes(ctx) -} - // GetIssuesByIDs return issues with the given IDs. // If keepOrder is true, the order of the returned issues will be the same as the given IDs. func GetIssuesByIDs(ctx context.Context, issueIDs []int64, keepOrder ...bool) (IssueList, error) { diff --git a/models/issues/issue_search.go b/models/issues/issue_search.go index 65ad3c8135..7dc277327a 100644 --- a/models/issues/issue_search.go +++ b/models/issues/issue_search.go @@ -455,26 +455,6 @@ func applySubscribedCondition(sess *xorm.Session, subscriberID int64) *xorm.Sess ) } -// GetRepoIDsForIssuesOptions find all repo ids for the given options -func GetRepoIDsForIssuesOptions(ctx context.Context, opts *IssuesOptions, user *user_model.User) ([]int64, error) { - repoIDs := make([]int64, 0, 5) - e := db.GetEngine(ctx) - - sess := e.Join("INNER", "repository", "`issue`.repo_id = `repository`.id") - - applyConditions(sess, opts) - - accessCond := repo_model.AccessibleRepositoryCondition(user, unit.TypeInvalid) - if err := sess.Where(accessCond). - Distinct("issue.repo_id"). - Table("issue"). - Find(&repoIDs); err != nil { - return nil, fmt.Errorf("unable to GetRepoIDsForIssuesOptions: %w", err) - } - - return repoIDs, nil -} - // Issues returns a list of issues by given conditions. func Issues(ctx context.Context, opts *IssuesOptions) (IssueList, error) { sess := db.GetEngine(ctx). diff --git a/models/issues/issue_test.go b/models/issues/issue_test.go index 4393d18bcf..723fa27b1b 100644 --- a/models/issues/issue_test.go +++ b/models/issues/issue_test.go @@ -216,36 +216,6 @@ func TestIssue_loadTotalTimes(t *testing.T) { assert.Equal(t, int64(3682), ms.TotalTrackedTime) } -func TestGetRepoIDsForIssuesOptions(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) - for _, test := range []struct { - Opts issues_model.IssuesOptions - ExpectedRepoIDs []int64 - }{ - { - issues_model.IssuesOptions{ - AssigneeID: 2, - }, - []int64{3, 32}, - }, - { - issues_model.IssuesOptions{ - RepoCond: builder.In("repo_id", 1, 2), - }, - []int64{1, 2}, - }, - } { - repoIDs, err := issues_model.GetRepoIDsForIssuesOptions(db.DefaultContext, &test.Opts, user) - assert.NoError(t, err) - if assert.Len(t, repoIDs, len(test.ExpectedRepoIDs)) { - for i, repoID := range repoIDs { - assert.EqualValues(t, test.ExpectedRepoIDs[i], repoID) - } - } - } -} - func testInsertIssue(t *testing.T, title, content string, expectIndex int64) *issues_model.Issue { var newIssue issues_model.Issue t.Run(title, func(t *testing.T) { diff --git a/models/issues/label.go b/models/issues/label.go index 5c6b8e08d7..3b811c1529 100644 --- a/models/issues/label.go +++ b/models/issues/label.go @@ -424,22 +424,6 @@ func GetLabelInOrgByID(ctx context.Context, orgID, labelID int64) (*Label, error return l, nil } -// GetLabelIDsInOrgByNames returns a list of labelIDs by names in a given -// organization. -func GetLabelIDsInOrgByNames(ctx context.Context, orgID int64, labelNames []string) ([]int64, error) { - if orgID <= 0 { - return nil, ErrOrgLabelNotExist{0, orgID} - } - labelIDs := make([]int64, 0, len(labelNames)) - - return labelIDs, db.GetEngine(ctx).Table("label"). - Where("org_id = ?", orgID). - In("name", labelNames). - Asc("name"). - Cols("id"). - Find(&labelIDs) -} - // GetLabelsInOrgByIDs returns a list of labels by IDs in given organization, // it silently ignores label IDs that do not belong to the organization. func GetLabelsInOrgByIDs(ctx context.Context, orgID int64, labelIDs []int64) ([]*Label, error) { diff --git a/models/issues/label_test.go b/models/issues/label_test.go index 3a8db6ceec..517a3cf1ab 100644 --- a/models/issues/label_test.go +++ b/models/issues/label_test.go @@ -164,30 +164,6 @@ func TestGetLabelInOrgByName(t *testing.T) { assert.True(t, issues_model.IsErrOrgLabelNotExist(err)) } -func TestGetLabelInOrgByNames(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - labelIDs, err := issues_model.GetLabelIDsInOrgByNames(db.DefaultContext, 3, []string{"orglabel3", "orglabel4"}) - assert.NoError(t, err) - - assert.Len(t, labelIDs, 2) - - assert.Equal(t, int64(3), labelIDs[0]) - assert.Equal(t, int64(4), labelIDs[1]) -} - -func TestGetLabelInOrgByNamesDiscardsNonExistentLabels(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - // orglabel99 doesn't exists.. See labels.yml - labelIDs, err := issues_model.GetLabelIDsInOrgByNames(db.DefaultContext, 3, []string{"orglabel3", "orglabel4", "orglabel99"}) - assert.NoError(t, err) - - assert.Len(t, labelIDs, 2) - - assert.Equal(t, int64(3), labelIDs[0]) - assert.Equal(t, int64(4), labelIDs[1]) - assert.NoError(t, err) -} - func TestGetLabelInOrgByID(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) label, err := issues_model.GetLabelInOrgByID(db.DefaultContext, 3, 3) diff --git a/models/issues/milestone_list.go b/models/issues/milestone_list.go index f331b2590f..a73bf73c17 100644 --- a/models/issues/milestone_list.go +++ b/models/issues/milestone_list.go @@ -160,32 +160,6 @@ func (m MilestonesStats) Total() int64 { return m.OpenCount + m.ClosedCount } -// GetMilestonesStatsByRepoCond returns milestone statistic information for dashboard by given conditions. -func GetMilestonesStatsByRepoCond(ctx context.Context, repoCond builder.Cond) (*MilestonesStats, error) { - var err error - stats := &MilestonesStats{} - - sess := db.GetEngine(ctx).Where("is_closed = ?", false) - if repoCond.IsValid() { - sess.And(builder.In("repo_id", builder.Select("id").From("repository").Where(repoCond))) - } - stats.OpenCount, err = sess.Count(new(Milestone)) - if err != nil { - return nil, err - } - - sess = db.GetEngine(ctx).Where("is_closed = ?", true) - if repoCond.IsValid() { - sess.And(builder.In("repo_id", builder.Select("id").From("repository").Where(repoCond))) - } - stats.ClosedCount, err = sess.Count(new(Milestone)) - if err != nil { - return nil, err - } - - return stats, nil -} - // GetMilestonesStatsByRepoCondAndKw returns milestone statistic information for dashboard by given repo conditions and name keyword. func GetMilestonesStatsByRepoCondAndKw(ctx context.Context, repoCond builder.Cond, keyword string) (*MilestonesStats, error) { var err error diff --git a/models/issues/milestone_test.go b/models/issues/milestone_test.go index 0581d3d148..7477af92c8 100644 --- a/models/issues/milestone_test.go +++ b/models/issues/milestone_test.go @@ -17,7 +17,6 @@ import ( "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" - "xorm.io/builder" ) func TestMilestone_State(t *testing.T) { @@ -285,34 +284,6 @@ func TestGetMilestonesByRepoIDs(t *testing.T) { }) } -func TestGetMilestonesStats(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - - test := func(repoID int64) { - repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: repoID}) - stats, err := issues_model.GetMilestonesStatsByRepoCond(db.DefaultContext, builder.And(builder.Eq{"repo_id": repoID})) - assert.NoError(t, err) - assert.EqualValues(t, repo.NumMilestones-repo.NumClosedMilestones, stats.OpenCount) - assert.EqualValues(t, repo.NumClosedMilestones, stats.ClosedCount) - } - test(1) - test(2) - test(3) - - stats, err := issues_model.GetMilestonesStatsByRepoCond(db.DefaultContext, builder.And(builder.Eq{"repo_id": unittest.NonexistentID})) - assert.NoError(t, err) - assert.EqualValues(t, 0, stats.OpenCount) - assert.EqualValues(t, 0, stats.ClosedCount) - - repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) - repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) - - milestoneStats, err := issues_model.GetMilestonesStatsByRepoCond(db.DefaultContext, builder.In("repo_id", []int64{repo1.ID, repo2.ID})) - assert.NoError(t, err) - assert.EqualValues(t, repo1.NumOpenMilestones+repo2.NumOpenMilestones, milestoneStats.OpenCount) - assert.EqualValues(t, repo1.NumClosedMilestones+repo2.NumClosedMilestones, milestoneStats.ClosedCount) -} - func TestNewMilestone(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) milestone := &issues_model.Milestone{ diff --git a/models/issues/pull.go b/models/issues/pull.go index c51a7daf4e..34bea921a0 100644 --- a/models/issues/pull.go +++ b/models/issues/pull.go @@ -78,24 +78,6 @@ func (err ErrPullRequestAlreadyExists) Unwrap() error { return util.ErrAlreadyExist } -// ErrPullRequestHeadRepoMissing represents a "ErrPullRequestHeadRepoMissing" error -type ErrPullRequestHeadRepoMissing struct { - ID int64 - HeadRepoID int64 -} - -// IsErrErrPullRequestHeadRepoMissing checks if an error is a ErrPullRequestHeadRepoMissing. -func IsErrErrPullRequestHeadRepoMissing(err error) bool { - _, ok := err.(ErrPullRequestHeadRepoMissing) - return ok -} - -// Error does pretty-printing :D -func (err ErrPullRequestHeadRepoMissing) Error() string { - return fmt.Sprintf("pull request head repo missing [id: %d, head_repo_id: %d]", - err.ID, err.HeadRepoID) -} - // ErrPullWasClosed is used close a closed pull request type ErrPullWasClosed struct { ID int64 @@ -758,18 +740,6 @@ func (pr *PullRequest) IsSameRepo() bool { return pr.BaseRepoID == pr.HeadRepoID } -// GetPullRequestsByHeadBranch returns all prs by head branch -// Since there could be multiple prs with the same head branch, this function returns a slice of prs -func GetPullRequestsByHeadBranch(ctx context.Context, headBranch string, headRepoID int64) ([]*PullRequest, error) { - log.Trace("GetPullRequestsByHeadBranch: headBranch: '%s', headRepoID: '%d'", headBranch, headRepoID) - prs := make([]*PullRequest, 0, 2) - if err := db.GetEngine(ctx).Where(builder.Eq{"head_branch": headBranch, "head_repo_id": headRepoID}). - Find(&prs); err != nil { - return nil, err - } - return prs, nil -} - // GetBaseBranchLink returns the relative URL of the base branch func (pr *PullRequest) GetBaseBranchLink(ctx context.Context) string { if err := pr.LoadBaseRepo(ctx); err != nil { diff --git a/models/issues/stopwatch.go b/models/issues/stopwatch.go index 2c662bdb06..fd9c7d7875 100644 --- a/models/issues/stopwatch.go +++ b/models/issues/stopwatch.go @@ -29,20 +29,6 @@ func (err ErrIssueStopwatchNotExist) Unwrap() error { return util.ErrNotExist } -// ErrIssueStopwatchAlreadyExist represents an error that stopwatch is already exist -type ErrIssueStopwatchAlreadyExist struct { - UserID int64 - IssueID int64 -} - -func (err ErrIssueStopwatchAlreadyExist) Error() string { - return fmt.Sprintf("issue stopwatch already exists[uid: %d, issue_id: %d", err.UserID, err.IssueID) -} - -func (err ErrIssueStopwatchAlreadyExist) Unwrap() error { - return util.ErrAlreadyExist -} - // Stopwatch represents a stopwatch for time tracking. type Stopwatch struct { ID int64 `xorm:"pk autoincr"` diff --git a/services/issue/assignee_test.go b/services/issue/assignee_test.go index e16b012a17..da25da60ee 100644 --- a/services/issue/assignee_test.go +++ b/services/issue/assignee_test.go @@ -18,8 +18,12 @@ func TestDeleteNotPassedAssignee(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) // Fake issue with assignees - issue, err := issues_model.GetIssueWithAttrsByID(db.DefaultContext, 1) + issue, err := issues_model.GetIssueByID(db.DefaultContext, 1) assert.NoError(t, err) + + err = issue.LoadAttributes(db.DefaultContext) + assert.NoError(t, err) + assert.Len(t, issue.Assignees, 1) user1, err := user_model.GetUserByID(db.DefaultContext, 1) // This user is already assigned (see the definition in fixtures), so running UpdateAssignee should unassign him