Skip to content

Commit 6780594

Browse files
ethantkoenigethantkoenig
committed
Make URL scheme unambiguous
Redirect old routes to new routes
1 parent 6e98812 commit 6780594

File tree

18 files changed

+211
-88
lines changed

18 files changed

+211
-88
lines changed

‎integrations/editor_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ func testEditFile(t *testing.T, session *TestSession, user, repo, branch, filePa
111111
resp = session.MakeRequest(t, req, http.StatusFound)
112112

113113
// Verify the change
114-
req = NewRequest(t, "GET", path.Join(user, repo, "raw", branch, filePath))
114+
req = NewRequest(t, "GET", path.Join(user, repo, "raw/branch", branch, filePath))
115115
resp = session.MakeRequest(t, req, http.StatusOK)
116116
assert.EqualValues(t, newContent, string(resp.Body))
117117

@@ -142,7 +142,7 @@ func testEditFileToNewBranch(t *testing.T, session *TestSession, user, repo, bra
142142
resp = session.MakeRequest(t, req, http.StatusFound)
143143

144144
// Verify the change
145-
req = NewRequest(t, "GET", path.Join(user, repo, "raw", targetBranch, filePath))
145+
req = NewRequest(t, "GET", path.Join(user, repo, "raw/branch", targetBranch, filePath))
146146
resp = session.MakeRequest(t, req, http.StatusOK)
147147
assert.EqualValues(t, newContent, string(resp.Body))
148148

‎integrations/links_test.go

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import (
1010
"testing"
1111

1212
api "code.gitea.io/sdk/gitea"
13+
14+
"github.com/stretchr/testify/assert"
1315
)
1416

1517
func TestLinksNoLogin(t *testing.T) {
@@ -38,6 +40,20 @@ func TestLinksNoLogin(t *testing.T) {
3840
}
3941
}
4042

43+
func TestRedirectsNoLogin(t *testing.T) {
44+
prepareTestEnv(t)
45+
46+
var redirects = map[string]string{
47+
"/user2/repo1/commits/master": "/user2/repo1/commits/branch/master",
48+
"/user2/repo1/src/master": "/user2/repo1/src/branch/master",
49+
}
50+
for link, redirectLink := range redirects {
51+
req := NewRequest(t, "GET", link)
52+
resp := MakeRequest(t, req, http.StatusFound)
53+
assert.EqualValues(t, redirectLink, RedirectURL(t, resp))
54+
}
55+
}
56+
4157
func testLinksAsUser(userName string, t *testing.T) {
4258
var links = []string{
4359
"/explore/repos",
@@ -99,7 +115,7 @@ func testLinksAsUser(userName string, t *testing.T) {
99115
"",
100116
"/issues",
101117
"/pulls",
102-
"/commits/master",
118+
"/commits/branch/master",
103119
"/graph",
104120
"/settings",
105121
"/settings/collaboration",

‎integrations/repo_commits_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ func TestRepoCommits(t *testing.T) {
2020
session := loginUser(t, "user2")
2121

2222
// Request repository commits page
23-
req := NewRequest(t, "GET", "/user2/repo1/commits/master")
23+
req := NewRequest(t, "GET", "/user2/repo1/commits/branch/master")
2424
resp := session.MakeRequest(t, req, http.StatusOK)
2525

2626
doc := NewHTMLParser(t, resp.Body)
@@ -35,7 +35,7 @@ func doTestRepoCommitWithStatus(t *testing.T, state string, classes ...string) {
3535
session := loginUser(t, "user2")
3636

3737
// Request repository commits page
38-
req := NewRequest(t, "GET", "/user2/repo1/commits/master")
38+
req := NewRequest(t, "GET", "/user2/repo1/commits/branch/master")
3939
resp := session.MakeRequest(t, req, http.StatusOK)
4040

4141
doc := NewHTMLParser(t, resp.Body)
@@ -56,7 +56,7 @@ func doTestRepoCommitWithStatus(t *testing.T, state string, classes ...string) {
5656

5757
resp = session.MakeRequest(t, req, http.StatusCreated)
5858

59-
req = NewRequest(t, "GET", "/user2/repo1/commits/master")
59+
req = NewRequest(t, "GET", "/user2/repo1/commits/branch/master")
6060
resp = session.MakeRequest(t, req, http.StatusOK)
6161

6262
doc = NewHTMLParser(t, resp.Body)

‎models/webhook_slack.go

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -80,12 +80,22 @@ func SlackLinkFormatter(url string, text string) string {
8080
return fmt.Sprintf("<%s|%s>", url, SlackTextFormatter(text))
8181
}
8282

83-
func getSlackCreatePayload(p *api.CreatePayload, slack *SlackMeta) (*SlackPayload, error) {
84-
// created tag/branch
85-
refName := git.RefEndName(p.Ref)
83+
// SlackLinkToRef slack-formatter link to a repo ref
84+
func SlackLinkToRef(repoURL, ref string) string {
85+
refName := git.RefEndName(ref)
86+
switch {
87+
case strings.HasPrefix(ref, git.BranchPrefix):
88+
return SlackLinkFormatter(repoURL+"/src/branch/"+refName, refName)
89+
case strings.HasPrefix(ref, git.TagPrefix):
90+
return SlackLinkFormatter(repoURL+"/src/tag/"+refName, refName)
91+
default:
92+
return SlackLinkFormatter(repoURL+"/src/commit/"+refName, refName)
93+
}
94+
}
8695

96+
func getSlackCreatePayload(p *api.CreatePayload, slack *SlackMeta) (*SlackPayload, error) {
8797
repoLink := SlackLinkFormatter(p.Repo.HTMLURL, p.Repo.Name)
88-
refLink := SlackLinkFormatter(p.Repo.HTMLURL+"/src/"+refName, refName)
98+
refLink := SlackLinkToRef(p.Repo.HTMLURL, p.Ref)
8999
text := fmt.Sprintf("[%s:%s] %s created by %s", repoLink, refLink, p.RefType, p.Sender.UserName)
90100

91101
return &SlackPayload{
@@ -99,7 +109,6 @@ func getSlackCreatePayload(p *api.CreatePayload, slack *SlackMeta) (*SlackPayloa
99109
func getSlackPushPayload(p *api.PushPayload, slack *SlackMeta) (*SlackPayload, error) {
100110
// n new commits
101111
var (
102-
branchName = git.RefEndName(p.Ref)
103112
commitDesc string
104113
commitString string
105114
)
@@ -116,7 +125,7 @@ func getSlackPushPayload(p *api.PushPayload, slack *SlackMeta) (*SlackPayload, e
116125
}
117126

118127
repoLink := SlackLinkFormatter(p.Repo.HTMLURL, p.Repo.Name)
119-
branchLink := SlackLinkFormatter(p.Repo.HTMLURL+"/src/"+branchName, branchName)
128+
branchLink := SlackLinkToRef(p.Repo.HTMLURL, p.Ref)
120129
text := fmt.Sprintf("[%s:%s] %s pushed by %s", repoLink, branchLink, commitString, p.Pusher.UserName)
121130

122131
var attachmentText string

‎modules/context/repo.go

Lines changed: 90 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"code.gitea.io/git"
1515
"code.gitea.io/gitea/models"
1616
"code.gitea.io/gitea/modules/cache"
17+
"code.gitea.io/gitea/modules/log"
1718
"code.gitea.io/gitea/modules/setting"
1819

1920
"github.com/Unknwon/com"
@@ -117,6 +118,20 @@ func (r *Repository) GetCommitsCount() (int64, error) {
117118
})
118119
}
119120

121+
// BranchNameSubURL sub-URL for the BranchName field
122+
func (r *Repository) BranchNameSubURL() string {
123+
switch {
124+
case r.IsViewBranch:
125+
return "branch/" + r.BranchName
126+
case r.IsViewTag:
127+
return "tag/" + r.BranchName
128+
case r.IsViewCommit:
129+
return "commit/" + r.BranchName
130+
}
131+
log.Error(4, "Unknown view type for repo: %v", r)
132+
return ""
133+
}
134+
120135
// GetEditorconfig returns the .editorconfig definition if found in the
121136
// HEAD of the default repo branch.
122137
func (r *Repository) GetEditorconfig() (*editorconfig.Editorconfig, error) {
@@ -444,8 +459,71 @@ func RepoAssignment() macaron.Handler {
444459
}
445460
}
446461

447-
// RepoRef handles repository reference name including those contain `/`.
462+
// RepoRefType type of repo reference
463+
type RepoRefType int
464+
465+
const (
466+
// RepoRefUnknown unknown type, make educated guess and redirect.
467+
// for backward compatibility with previous URL scheme
468+
RepoRefUnknown RepoRefType = iota
469+
// RepoRefBranch branch
470+
RepoRefBranch
471+
// RepoRefTag tag
472+
RepoRefTag
473+
// RepoRefCommit commit
474+
RepoRefCommit
475+
)
476+
477+
// RepoRef handles repository reference names when the ref name is not
478+
// explicitly given
448479
func RepoRef() macaron.Handler {
480+
// since no ref name is explicitly specified, ok to just use branch
481+
return RepoRefByType(RepoRefBranch)
482+
}
483+
484+
func getRefNameFromPath(ctx *Context, path string, isExist func(string) bool) string {
485+
refName := ""
486+
parts := strings.Split(path, "/")
487+
for i, part := range parts {
488+
refName = strings.TrimPrefix(refName+"/"+part, "/")
489+
if isExist(refName) {
490+
ctx.Repo.TreePath = strings.Join(parts[i+1:], "/")
491+
return refName
492+
}
493+
}
494+
return ""
495+
}
496+
497+
func getRefName(ctx *Context, pathType RepoRefType) string {
498+
path := ctx.Params("*")
499+
switch pathType {
500+
case RepoRefUnknown:
501+
if refName := getRefName(ctx, RepoRefBranch); len(refName) > 0 {
502+
return refName
503+
}
504+
if refName := getRefName(ctx, RepoRefTag); len(refName) > 0 {
505+
return refName
506+
}
507+
return getRefName(ctx, RepoRefCommit)
508+
case RepoRefBranch:
509+
return getRefNameFromPath(ctx, path, ctx.Repo.GitRepo.IsBranchExist)
510+
case RepoRefTag:
511+
return getRefNameFromPath(ctx, path, ctx.Repo.GitRepo.IsTagExist)
512+
case RepoRefCommit:
513+
parts := strings.Split(path, "/")
514+
if len(parts) > 0 && len(parts[0]) == 40 {
515+
ctx.Repo.TreePath = strings.Join(parts[1:], "/")
516+
return parts[0]
517+
}
518+
default:
519+
log.Error(4, "Unrecognized path type: %v", path)
520+
}
521+
return ""
522+
}
523+
524+
// RepoRefByType handles repository reference name for a specific type
525+
// of repository reference
526+
func RepoRefByType(refType RepoRefType) macaron.Handler {
449527
return func(ctx *Context) {
450528
// Empty repository does not have reference information.
451529
if ctx.Repo.Repository.IsBare {
@@ -492,25 +570,7 @@ func RepoRef() macaron.Handler {
492570
ctx.Repo.IsViewBranch = true
493571

494572
} else {
495-
hasMatched := false
496-
parts := strings.Split(ctx.Params("*"), "/")
497-
for i, part := range parts {
498-
refName = strings.TrimPrefix(refName+"/"+part, "/")
499-
500-
if ctx.Repo.GitRepo.IsBranchExist(refName) ||
501-
ctx.Repo.GitRepo.IsTagExist(refName) {
502-
if i < len(parts)-1 {
503-
ctx.Repo.TreePath = strings.Join(parts[i+1:], "/")
504-
}
505-
hasMatched = true
506-
break
507-
}
508-
}
509-
if !hasMatched && len(parts[0]) == 40 {
510-
refName = parts[0]
511-
ctx.Repo.TreePath = strings.Join(parts[1:], "/")
512-
}
513-
573+
refName = getRefName(ctx, refType)
514574
if ctx.Repo.GitRepo.IsBranchExist(refName) {
515575
ctx.Repo.IsViewBranch = true
516576

@@ -542,10 +602,20 @@ func RepoRef() macaron.Handler {
542602
ctx.Handle(404, "RepoRef invalid repo", fmt.Errorf("branch or tag not exist: %s", refName))
543603
return
544604
}
605+
606+
if refType == RepoRefUnknown {
607+
// redirect from old URL scheme to new URL scheme
608+
ctx.Redirect(path.Join(
609+
ctx.Repo.BranchNameSubURL(),
610+
ctx.Repo.TreePath,
611+
))
612+
return
613+
}
545614
}
546615

547616
ctx.Repo.BranchName = refName
548617
ctx.Data["BranchName"] = ctx.Repo.BranchName
618+
ctx.Data["BranchNameSubURL"] = ctx.Repo.BranchNameSubURL()
549619
ctx.Data["CommitID"] = ctx.Repo.CommitID
550620
ctx.Data["TreePath"] = ctx.Repo.TreePath
551621
ctx.Data["IsViewBranch"] = ctx.Repo.IsViewBranch

‎routers/api/v1/api.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,7 @@ func RegisterRoutes(m *macaron.Macaron) {
391391
Post(reqToken(), bind(api.CreateForkOption{}), repo.CreateFork)
392392
m.Group("/branches", func() {
393393
m.Get("", repo.ListBranches)
394-
m.Get("/*", context.RepoRef(), repo.GetBranch)
394+
m.Get("/*", context.RepoRefByType(context.RepoRefBranch), repo.GetBranch)
395395
})
396396
m.Group("/keys", func() {
397397
m.Combo("").Get(repo.ListDeployKeys).

‎routers/repo/branch.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ func CreateBranch(ctx *context.Context, form auth.NewBranchForm) {
202202

203203
if ctx.HasError() {
204204
ctx.Flash.Error(ctx.GetErrMsg())
205-
ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchName)
205+
ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL())
206206
return
207207
}
208208

@@ -216,19 +216,19 @@ func CreateBranch(ctx *context.Context, form auth.NewBranchForm) {
216216
if models.IsErrTagAlreadyExists(err) {
217217
e := err.(models.ErrTagAlreadyExists)
218218
ctx.Flash.Error(ctx.Tr("repo.branch.tag_collision", e.TagName))
219-
ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchName)
219+
ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL())
220220
return
221221
}
222222
if models.IsErrBranchAlreadyExists(err) {
223223
e := err.(models.ErrBranchAlreadyExists)
224224
ctx.Flash.Error(ctx.Tr("repo.branch.branch_already_exists", e.BranchName))
225-
ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchName)
225+
ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL())
226226
return
227227
}
228228
if models.IsErrBranchNameConflict(err) {
229229
e := err.(models.ErrBranchNameConflict)
230230
ctx.Flash.Error(ctx.Tr("repo.branch.branch_name_conflict", form.NewBranchName, e.BranchName))
231-
ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchName)
231+
ctx.Redirect(ctx.Repo.RepoLink + "/src/" + ctx.Repo.BranchNameSubURL())
232232
return
233233
}
234234

@@ -237,5 +237,5 @@ func CreateBranch(ctx *context.Context, form auth.NewBranchForm) {
237237
}
238238

239239
ctx.Flash.Success(ctx.Tr("repo.branch.create_success", form.NewBranchName))
240-
ctx.Redirect(ctx.Repo.RepoLink + "/src/" + form.NewBranchName)
240+
ctx.Redirect(ctx.Repo.RepoLink + "/src/branch/" + form.NewBranchName)
241241
}

‎routers/repo/commit.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ func SearchCommits(ctx *context.Context) {
120120

121121
keyword := strings.Trim(ctx.Query("q"), " ")
122122
if len(keyword) == 0 {
123-
ctx.Redirect(ctx.Repo.RepoLink + "/commits/" + ctx.Repo.BranchName)
123+
ctx.Redirect(ctx.Repo.RepoLink + "/commits/" + ctx.Repo.BranchNameSubURL())
124124
return
125125
}
126126
all := ctx.QueryBool("all")

0 commit comments

Comments
 (0)