Skip to content

Commit 94746ec

Browse files
appleboyappleboy
committed
refactor: simplify logic and improve variable clarity
- Remove unused imports and functions - Refactor the `ParseCompareInfo` function to simplify the logic - Update variable names for clarity - Adjust the comparison of commits in the `CompareDiff` function Signed-off-by: appleboy <[email protected]>
1 parent dc7078d commit 94746ec

File tree

1 file changed

+24
-302
lines changed

1 file changed

+24
-302
lines changed

‎routers/api/v1/repo/compare.go

Lines changed: 24 additions & 302 deletions
Original file line numberDiff line numberDiff line change
@@ -7,304 +7,13 @@ import (
77
"net/http"
88
"strings"
99

10-
access_model "code.gitea.io/gitea/models/perm/access"
11-
repo_model "code.gitea.io/gitea/models/repo"
12-
"code.gitea.io/gitea/models/unit"
1310
user_model "code.gitea.io/gitea/models/user"
14-
"code.gitea.io/gitea/modules/git"
1511
"code.gitea.io/gitea/modules/gitrepo"
16-
"code.gitea.io/gitea/modules/log"
1712
api "code.gitea.io/gitea/modules/structs"
18-
"code.gitea.io/gitea/modules/util"
19-
"code.gitea.io/gitea/routers/common"
2013
"code.gitea.io/gitea/services/context"
2114
"code.gitea.io/gitea/services/convert"
2215
)
2316

24-
// ParseCompareInfo parse compare info between two commit for preparing comparing references
25-
func ParseCompareInfo(ctx *context.APIContext) *common.CompareInfo {
26-
baseRepo := ctx.Repo.Repository
27-
ci := &common.CompareInfo{}
28-
29-
fileOnly := ctx.FormBool("file-only")
30-
31-
// Get compared branches information
32-
// A full compare url is of the form:
33-
//
34-
// 1. /{:baseOwner}/{:baseRepoName}/compare/{:baseBranch}...{:headBranch}
35-
// 2. /{:baseOwner}/{:baseRepoName}/compare/{:baseBranch}...{:headOwner}:{:headBranch}
36-
// 3. /{:baseOwner}/{:baseRepoName}/compare/{:baseBranch}...{:headOwner}/{:headRepoName}:{:headBranch}
37-
// 4. /{:baseOwner}/{:baseRepoName}/compare/{:headBranch}
38-
// 5. /{:baseOwner}/{:baseRepoName}/compare/{:headOwner}:{:headBranch}
39-
// 6. /{:baseOwner}/{:baseRepoName}/compare/{:headOwner}/{:headRepoName}:{:headBranch}
40-
//
41-
// Here we obtain the infoPath "{:baseBranch}...[{:headOwner}/{:headRepoName}:]{:headBranch}" as ctx.Params("*")
42-
// with the :baseRepo in ctx.Repo.
43-
//
44-
// Note: Generally :headRepoName is not provided here - we are only passed :headOwner.
45-
//
46-
// How do we determine the :headRepo?
47-
//
48-
// 1. If :headOwner is not set then the :headRepo = :baseRepo
49-
// 2. If :headOwner is set - then look for the fork of :baseRepo owned by :headOwner
50-
// 3. But... :baseRepo could be a fork of :headOwner's repo - so check that
51-
// 4. Now, :baseRepo and :headRepos could be forks of the same repo - so check that
52-
//
53-
// format: <base branch>...[<head repo>:]<head branch>
54-
// base<-head: master...head:feature
55-
// same repo: master...feature
56-
57-
var (
58-
isSameRepo bool
59-
infoPath string
60-
err error
61-
)
62-
63-
infoPath = ctx.Params("*")
64-
var infos []string
65-
if infoPath == "" {
66-
infos = []string{baseRepo.DefaultBranch, baseRepo.DefaultBranch}
67-
} else {
68-
infos = strings.SplitN(infoPath, "...", 2)
69-
if len(infos) != 2 {
70-
if infos = strings.SplitN(infoPath, "..", 2); len(infos) == 2 {
71-
ci.DirectComparison = true
72-
} else {
73-
infos = []string{baseRepo.DefaultBranch, infoPath}
74-
}
75-
}
76-
}
77-
78-
ci.BaseBranch = infos[0]
79-
80-
// If there is no head repository, it means compare between same repository.
81-
headInfos := strings.Split(infos[1], ":")
82-
if len(headInfos) == 1 {
83-
isSameRepo = true
84-
ci.HeadUser = ctx.Repo.Owner
85-
ci.HeadBranch = headInfos[0]
86-
} else if len(headInfos) == 2 {
87-
headInfosSplit := strings.Split(headInfos[0], "/")
88-
if len(headInfosSplit) == 1 {
89-
ci.HeadUser, err = user_model.GetUserByName(ctx, headInfos[0])
90-
if err != nil {
91-
if user_model.IsErrUserNotExist(err) {
92-
ctx.NotFound("GetUserByName", nil)
93-
} else {
94-
ctx.ServerError("GetUserByName", err)
95-
}
96-
return nil
97-
}
98-
ci.HeadBranch = headInfos[1]
99-
isSameRepo = ci.HeadUser.ID == ctx.Repo.Owner.ID
100-
if isSameRepo {
101-
ci.HeadRepo = baseRepo
102-
}
103-
} else {
104-
ci.HeadRepo, err = repo_model.GetRepositoryByOwnerAndName(ctx, headInfosSplit[0], headInfosSplit[1])
105-
if err != nil {
106-
if repo_model.IsErrRepoNotExist(err) {
107-
ctx.NotFound("GetRepositoryByOwnerAndName", nil)
108-
} else {
109-
ctx.ServerError("GetRepositoryByOwnerAndName", err)
110-
}
111-
return nil
112-
}
113-
if err := ci.HeadRepo.LoadOwner(ctx); err != nil {
114-
if user_model.IsErrUserNotExist(err) {
115-
ctx.NotFound("GetUserByName", nil)
116-
} else {
117-
ctx.ServerError("GetUserByName", err)
118-
}
119-
return nil
120-
}
121-
ci.HeadBranch = headInfos[1]
122-
ci.HeadUser = ci.HeadRepo.Owner
123-
isSameRepo = ci.HeadRepo.ID == ctx.Repo.Repository.ID
124-
}
125-
} else {
126-
ctx.NotFound("CompareAndPullRequest", nil)
127-
return nil
128-
}
129-
ctx.Repo.PullRequest.SameRepo = isSameRepo
130-
131-
// Check if base branch is valid.
132-
baseIsCommit := ctx.Repo.GitRepo.IsCommitExist(ci.BaseBranch)
133-
baseIsBranch := ctx.Repo.GitRepo.IsBranchExist(ci.BaseBranch)
134-
baseIsTag := ctx.Repo.GitRepo.IsTagExist(ci.BaseBranch)
135-
136-
if !baseIsCommit && !baseIsBranch && !baseIsTag {
137-
// Check if baseBranch is short sha commit hash
138-
if baseCommit, _ := ctx.Repo.GitRepo.GetCommit(ci.BaseBranch); baseCommit != nil {
139-
ci.BaseBranch = baseCommit.ID.String()
140-
} else if ci.BaseBranch == ctx.Repo.GetObjectFormat().EmptyObjectID().String() {
141-
if isSameRepo {
142-
ctx.Redirect(ctx.Repo.RepoLink + "/compare/" + util.PathEscapeSegments(ci.HeadBranch))
143-
} else {
144-
ctx.Redirect(ctx.Repo.RepoLink + "/compare/" + util.PathEscapeSegments(ci.HeadRepo.FullName()) + ":" + util.PathEscapeSegments(ci.HeadBranch))
145-
}
146-
return nil
147-
} else {
148-
ctx.NotFound("IsRefExist", nil)
149-
return nil
150-
}
151-
}
152-
153-
// Now we have the repository that represents the base
154-
155-
// The current base and head repositories and branches may not
156-
// actually be the intended branches that the user wants to
157-
// create a pull-request from - but also determining the head
158-
// repo is difficult.
159-
160-
// We will want therefore to offer a few repositories to set as
161-
// our base and head
162-
163-
// 1. First if the baseRepo is a fork get the "RootRepo" it was
164-
// forked from
165-
var rootRepo *repo_model.Repository
166-
if baseRepo.IsFork {
167-
err = baseRepo.GetBaseRepo(ctx)
168-
if err != nil {
169-
if !repo_model.IsErrRepoNotExist(err) {
170-
ctx.ServerError("Unable to find root repo", err)
171-
return nil
172-
}
173-
} else {
174-
rootRepo = baseRepo.BaseRepo
175-
}
176-
}
177-
178-
// 2. Now if the current user is not the owner of the baseRepo,
179-
// check if they have a fork of the base repo and offer that as
180-
// "OwnForkRepo"
181-
var ownForkRepo *repo_model.Repository
182-
if ctx.Doer != nil && baseRepo.OwnerID != ctx.Doer.ID {
183-
repo := repo_model.GetForkedRepo(ctx, ctx.Doer.ID, baseRepo.ID)
184-
if repo != nil {
185-
ownForkRepo = repo
186-
}
187-
}
188-
189-
has := ci.HeadRepo != nil
190-
// 3. If the base is a forked from "RootRepo" and the owner of
191-
// the "RootRepo" is the :headUser - set headRepo to that
192-
if !has && rootRepo != nil && rootRepo.OwnerID == ci.HeadUser.ID {
193-
ci.HeadRepo = rootRepo
194-
has = true
195-
}
196-
197-
// 4. If the ctx.Doer has their own fork of the baseRepo and the headUser is the ctx.Doer
198-
// set the headRepo to the ownFork
199-
if !has && ownForkRepo != nil && ownForkRepo.OwnerID == ci.HeadUser.ID {
200-
ci.HeadRepo = ownForkRepo
201-
has = true
202-
}
203-
204-
// 5. If the headOwner has a fork of the baseRepo - use that
205-
if !has {
206-
ci.HeadRepo = repo_model.GetForkedRepo(ctx, ci.HeadUser.ID, baseRepo.ID)
207-
has = ci.HeadRepo != nil
208-
}
209-
210-
// 6. If the baseRepo is a fork and the headUser has a fork of that use that
211-
if !has && baseRepo.IsFork {
212-
ci.HeadRepo = repo_model.GetForkedRepo(ctx, ci.HeadUser.ID, baseRepo.ForkID)
213-
has = ci.HeadRepo != nil
214-
}
215-
216-
// 7. Finally open the git repo
217-
if isSameRepo {
218-
ci.HeadRepo = ctx.Repo.Repository
219-
ci.HeadGitRepo = ctx.Repo.GitRepo
220-
} else if has {
221-
ci.HeadGitRepo, err = gitrepo.OpenRepository(ctx, ci.HeadRepo)
222-
if err != nil {
223-
ctx.ServerError("OpenRepository", err)
224-
return nil
225-
}
226-
defer ci.HeadGitRepo.Close()
227-
} else {
228-
ctx.NotFound("ParseCompareInfo", nil)
229-
return nil
230-
}
231-
232-
// Now we need to assert that the ctx.Doer has permission to read
233-
// the baseRepo's code and pulls
234-
// (NOT headRepo's)
235-
permBase, err := access_model.GetUserRepoPermission(ctx, baseRepo, ctx.Doer)
236-
if err != nil {
237-
ctx.ServerError("GetUserRepoPermission", err)
238-
return nil
239-
}
240-
if !permBase.CanRead(unit.TypeCode) {
241-
if log.IsTrace() {
242-
log.Trace("Permission Denied: User: %-v cannot read code in Repo: %-v\nUser in baseRepo has Permissions: %-+v",
243-
ctx.Doer,
244-
baseRepo,
245-
permBase)
246-
}
247-
ctx.NotFound("ParseCompareInfo", nil)
248-
return nil
249-
}
250-
251-
// If we're not merging from the same repo:
252-
if !isSameRepo {
253-
// Assert ctx.Doer has permission to read headRepo's codes
254-
permHead, err := access_model.GetUserRepoPermission(ctx, ci.HeadRepo, ctx.Doer)
255-
if err != nil {
256-
ctx.ServerError("GetUserRepoPermission", err)
257-
return nil
258-
}
259-
if !permHead.CanRead(unit.TypeCode) {
260-
if log.IsTrace() {
261-
log.Trace("Permission Denied: User: %-v cannot read code in Repo: %-v\nUser in headRepo has Permissions: %-+v",
262-
ctx.Doer,
263-
ci.HeadRepo,
264-
permHead)
265-
}
266-
ctx.NotFound("ParseCompareInfo", nil)
267-
return nil
268-
}
269-
}
270-
271-
// Check if head branch is valid.
272-
headIsCommit := ci.HeadGitRepo.IsCommitExist(ci.HeadBranch)
273-
headIsBranch := ci.HeadGitRepo.IsBranchExist(ci.HeadBranch)
274-
headIsTag := ci.HeadGitRepo.IsTagExist(ci.HeadBranch)
275-
if !headIsCommit && !headIsBranch && !headIsTag {
276-
// Check if headBranch is short sha commit hash
277-
if headCommit, _ := ci.HeadGitRepo.GetCommit(ci.HeadBranch); headCommit != nil {
278-
ci.HeadBranch = headCommit.ID.String()
279-
ctx.Data["HeadBranch"] = ci.HeadBranch
280-
} else {
281-
ctx.NotFound("IsRefExist", nil)
282-
return nil
283-
}
284-
}
285-
286-
baseBranchRef := ci.BaseBranch
287-
if baseIsBranch {
288-
baseBranchRef = git.BranchPrefix + ci.BaseBranch
289-
} else if baseIsTag {
290-
baseBranchRef = git.TagPrefix + ci.BaseBranch
291-
}
292-
headBranchRef := ci.HeadBranch
293-
if headIsBranch {
294-
headBranchRef = git.BranchPrefix + ci.HeadBranch
295-
} else if headIsTag {
296-
headBranchRef = git.TagPrefix + ci.HeadBranch
297-
}
298-
299-
ci.CompareInfo, err = ci.HeadGitRepo.GetCompareInfo(baseRepo.RepoPath(), baseBranchRef, headBranchRef, ci.DirectComparison, fileOnly)
300-
if err != nil {
301-
ctx.ServerError("GetCompareInfo", err)
302-
return nil
303-
}
304-
305-
return ci
306-
}
307-
30817
// CompareDiff compare two branches or commits
30918
func CompareDiff(ctx *context.APIContext) {
31019
// swagger:operation GET /repos/{owner}/{repo}/compare/{basehead} Get commit comparison information
@@ -344,24 +53,37 @@ func CompareDiff(ctx *context.APIContext) {
34453
defer gitRepo.Close()
34554
}
34655

347-
ci := ParseCompareInfo(ctx)
348-
defer func() {
349-
if ci != nil && ci.HeadGitRepo != nil {
350-
ci.HeadGitRepo.Close()
56+
infoPath := ctx.Params("*")
57+
infos := []string{ctx.Repo.Repository.DefaultBranch, ctx.Repo.Repository.DefaultBranch}
58+
if infoPath != "" {
59+
infos = strings.SplitN(infoPath, "...", 2)
60+
if len(infos) != 2 {
61+
if infos = strings.SplitN(infoPath, "..", 2); len(infos) != 2 {
62+
infos = []string{ctx.Repo.Repository.DefaultBranch, infoPath}
63+
}
35164
}
352-
}()
65+
}
66+
67+
_, _, headGitRepo, ci, _, _ := parseCompareInfo(ctx, api.CreatePullRequestOption{
68+
Base: infos[0],
69+
Head: infos[1],
70+
})
35371
if ctx.Written() {
35472
return
35573
}
74+
defer headGitRepo.Close()
75+
76+
verification := ctx.FormString("verification") == "" || ctx.FormBool("verification")
77+
files := ctx.FormString("files") == "" || ctx.FormBool("files")
35678

357-
apiCommits := make([]*api.Commit, 0, len(ci.CompareInfo.Commits))
79+
apiCommits := make([]*api.Commit, 0, len(ci.Commits))
35880
userCache := make(map[string]*user_model.User)
359-
for i := 0; i < len(ci.CompareInfo.Commits); i++ {
360-
apiCommit, err := convert.ToCommit(ctx, ctx.Repo.Repository, ctx.Repo.GitRepo, ci.CompareInfo.Commits[i], userCache,
81+
for i := 0; i < len(ci.Commits); i++ {
82+
apiCommit, err := convert.ToCommit(ctx, ctx.Repo.Repository, ctx.Repo.GitRepo, ci.Commits[i], userCache,
36183
convert.ToCommitOptions{
36284
Stat: true,
363-
Verification: ctx.FormBool("verification"),
364-
Files: ctx.FormBool("files"),
85+
Verification: verification,
86+
Files: files,
36587
})
36688
if err != nil {
36789
ctx.ServerError("toCommit", err)
@@ -371,7 +93,7 @@ func CompareDiff(ctx *context.APIContext) {
37193
}
37294

37395
ctx.JSON(http.StatusOK, &api.Compare{
374-
TotalCommits: len(ci.CompareInfo.Commits),
96+
TotalCommits: len(ci.Commits),
37597
Commits: apiCommits,
37698
})
37799
}

0 commit comments

Comments
 (0)