Skip to content

Commit bbbc508

Browse files
wxiaoguangwxiaoguang
committed
fix
1 parent 8135254 commit bbbc508

File tree

12 files changed

+57
-59
lines changed

12 files changed

+57
-59
lines changed

‎models/actions/run.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ func (run *ActionRun) RefLink() string {
8888
if refName.IsPull() {
8989
return run.Repo.Link() + "/pulls/" + refName.ShortName()
9090
}
91-
return git.RefURL(run.Repo.Link(), run.Ref)
91+
return run.Repo.Link() + "/src/" + refName.RefWebLinkPath()
9292
}
9393

9494
// PrettyRef return #id for pull ref or ShortName for others

‎models/activities/action.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ func (a *Action) GetBranch() string {
355355

356356
// GetRefLink returns the action's ref link.
357357
func (a *Action) GetRefLink(ctx context.Context) string {
358-
return git.RefURL(a.GetRepoLink(ctx), a.RefName)
358+
return a.GetRepoLink(ctx) + "/src/" + git.RefName(a.RefName).RefWebLinkPath()
359359
}
360360

361361
// GetTag returns the action's repository tag.

‎modules/git/commit.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -476,8 +476,12 @@ func (c *Commit) GetRepositoryDefaultPublicGPGKey(forceUpdate bool) (*GPGSetting
476476
}
477477

478478
func IsStringLikelyCommitID(objFmt ObjectFormat, s string, minLength ...int) bool {
479-
minLen := util.OptionalArg(minLength, objFmt.FullLength())
480-
if len(s) < minLen || len(s) > objFmt.FullLength() {
479+
maxLen := 64 // sha256
480+
if objFmt != nil {
481+
maxLen = objFmt.FullLength()
482+
}
483+
minLen := util.OptionalArg(minLength, maxLen)
484+
if len(s) < minLen || len(s) > maxLen {
481485
return false
482486
}
483487
for _, c := range s {

‎modules/git/ref.go

Lines changed: 28 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -185,32 +185,38 @@ func (ref RefName) RefGroup() string {
185185
return ""
186186
}
187187

188+
// RefType is a simple ref type of the reference, it is used for UI and webhooks
189+
type RefType string
190+
191+
const (
192+
RefTypeBranch RefType = "branch"
193+
RefTypeTag RefType = "tag"
194+
RefTypeCommit RefType = "commit"
195+
)
196+
188197
// RefType returns the simple ref type of the reference, e.g. branch, tag
189198
// It's different from RefGroup, which is using the name of the directory under .git/refs
190-
// Here we using branch but not heads, using tag but not tags
191-
func (ref RefName) RefType() string {
192-
var refType string
193-
if ref.IsBranch() {
194-
refType = "branch"
195-
} else if ref.IsTag() {
196-
refType = "tag"
199+
func (ref RefName) RefType() RefType {
200+
switch {
201+
case ref.IsBranch():
202+
return RefTypeBranch
203+
case ref.IsTag():
204+
return RefTypeTag
205+
case IsStringLikelyCommitID(nil, string(ref), 6):
206+
return RefTypeCommit
197207
}
198-
return refType
208+
return ""
199209
}
200210

201-
// RefURL returns the absolute URL for a ref in a repository
202-
func RefURL(repoURL, ref string) string {
203-
refFullName := RefName(ref)
204-
refName := util.PathEscapeSegments(refFullName.ShortName())
205-
switch {
206-
case refFullName.IsBranch():
207-
return repoURL + "/src/branch/" + refName
208-
case refFullName.IsTag():
209-
return repoURL + "/src/tag/" + refName
210-
case !Sha1ObjectFormat.IsValid(ref):
211-
// assume they mean a branch
212-
return repoURL + "/src/branch/" + refName
213-
default:
214-
return repoURL + "/src/commit/" + refName
211+
// RefWebLinkPath returns a path for the reference that can be used in a web link:
212+
// * "branch/<branch_name>"
213+
// * "tag/<tag_name>"
214+
// * "commit/<commit_id>"
215+
// It returns an empty string if the reference is not a branch, tag or commit.
216+
func (ref RefName) RefWebLinkPath() string {
217+
refType := ref.RefType()
218+
if refType == "" {
219+
return ""
215220
}
221+
return string(refType) + "/" + util.PathEscapeSegments(ref.ShortName())
216222
}

‎modules/git/ref_test.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,8 @@ func TestRefName(t *testing.T) {
3232
assert.Equal(t, "c0ffee", RefName("c0ffee").ShortName())
3333
}
3434

35-
func TestRefURL(t *testing.T) {
36-
repoURL := "/user/repo"
37-
assert.Equal(t, repoURL+"/src/branch/foo", RefURL(repoURL, "refs/heads/foo"))
38-
assert.Equal(t, repoURL+"/src/tag/foo", RefURL(repoURL, "refs/tags/foo"))
39-
assert.Equal(t, repoURL+"/src/commit/c0ffee", RefURL(repoURL, "c0ffee"))
35+
func TestRefWebLinkPath(t *testing.T) {
36+
assert.Equal(t, "branch/foo", RefName("refs/heads/foo").RefWebLinkPath())
37+
assert.Equal(t, "tag/foo", RefName("refs/tags/foo").RefWebLinkPath())
38+
assert.Equal(t, "commit/c0ffee", RefName("c0ffee").RefWebLinkPath())
4039
}

‎routers/web/web.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1572,6 +1572,7 @@ func registerRoutes(m *web.Router) {
15721572
m.Get("/atom/branch/*", context.RepoRefByType(context.RepoRefBranch), feedEnabled, feed.RenderBranchFeed)
15731573

15741574
m.Group("/src", func() {
1575+
m.Get("", func(ctx *context.Context) { ctx.Redirect(ctx.Repo.RepoLink) }) // there is no "{owner}/{repo}/src" page, so redirect to "{owner}/{repo}" to avoid 404
15751576
m.Get("/branch/*", context.RepoRefByType(context.RepoRefBranch), repo.Home)
15761577
m.Get("/tag/*", context.RepoRefByType(context.RepoRefTag), repo.Home)
15771578
m.Get("/commit/*", context.RepoRefByType(context.RepoRefCommit), repo.Home)

‎services/actions/notifier.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -563,9 +563,9 @@ func (n *actionsNotifier) CreateRef(ctx context.Context, pusher *user_model.User
563563
newNotifyInput(repo, pusher, webhook_module.HookEventCreate).
564564
WithRef(refFullName.String()).
565565
WithPayload(&api.CreatePayload{
566-
Ref: refFullName.String(),
566+
Ref: refFullName.String(), // HINT: here is inconsistent with the Webhook's payload: webhook uses ShortName
567567
Sha: refID,
568-
RefType: refFullName.RefType(),
568+
RefType: string(refFullName.RefType()),
569569
Repo: apiRepo,
570570
Sender: apiPusher,
571571
}).
@@ -580,8 +580,8 @@ func (n *actionsNotifier) DeleteRef(ctx context.Context, pusher *user_model.User
580580

581581
newNotifyInput(repo, pusher, webhook_module.HookEventDelete).
582582
WithPayload(&api.DeletePayload{
583-
Ref: refFullName.String(),
584-
RefType: refFullName.RefType(),
583+
Ref: refFullName.String(), // HINT: here is inconsistent with the Webhook's payload: webhook uses ShortName
584+
RefType: string(refFullName.RefType()),
585585
PusherType: api.PusherTypeUser,
586586
Repo: apiRepo,
587587
Sender: apiPusher,

‎services/context/repo.go

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -210,16 +210,7 @@ func (r *Repository) GetCommitGraphsCount(ctx context.Context, hidePRRefs bool,
210210
// * "commit/123456"
211211
// It is usually used to construct a link like ".../src/{{RefTypeNameSubURL}}/{{PathEscapeSegments TreePath}}"
212212
func (r *Repository) RefTypeNameSubURL() string {
213-
switch {
214-
case r.IsViewBranch:
215-
return "branch/" + util.PathEscapeSegments(r.BranchName)
216-
case r.IsViewTag:
217-
return "tag/" + util.PathEscapeSegments(r.TagName)
218-
case r.IsViewCommit:
219-
return "commit/" + util.PathEscapeSegments(r.CommitID)
220-
}
221-
log.Error("Unknown view type for repo: %v", r)
222-
return ""
213+
return r.RefFullName.RefWebLinkPath()
223214
}
224215

225216
// GetEditorconfig returns the .editorconfig definition if found in the

‎services/feed/notifier.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,7 @@ func (a *actionNotifier) NewRelease(ctx context.Context, rel *repo_model.Release
469469
Repo: rel.Repo,
470470
IsPrivate: rel.Repo.IsPrivate,
471471
Content: rel.Title,
472-
RefName: rel.TagName, // FIXME: use a full ref name?
472+
RefName: git.RefNameFromTag(rel.TagName).String(), // Other functions in this file all use "refFullName.String()"
473473
}); err != nil {
474474
log.Error("NotifyWatchers: %v", err)
475475
}

‎services/issue/issue.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -250,8 +250,9 @@ func GetRefEndNamesAndURLs(issues []*issues_model.Issue, repoLink string) (map[i
250250
issueRefURLs := make(map[int64]string, len(issues))
251251
for _, issue := range issues {
252252
if issue.Ref != "" {
253-
issueRefEndNames[issue.ID] = git.RefName(issue.Ref).ShortName()
254-
issueRefURLs[issue.ID] = git.RefURL(repoLink, issue.Ref)
253+
ref := git.RefName(issue.Ref)
254+
issueRefEndNames[issue.ID] = ref.ShortName()
255+
issueRefURLs[issue.ID] = repoLink + "/src/" + ref.RefWebLinkPath()
255256
}
256257
}
257258
return issueRefEndNames, issueRefURLs

‎services/webhook/notifier.go

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -763,12 +763,10 @@ func (m *webhookNotifier) PullRequestReviewRequest(ctx context.Context, doer *us
763763
func (m *webhookNotifier) CreateRef(ctx context.Context, pusher *user_model.User, repo *repo_model.Repository, refFullName git.RefName, refID string) {
764764
apiPusher := convert.ToUser(ctx, pusher, nil)
765765
apiRepo := convert.ToRepo(ctx, repo, access_model.Permission{AccessMode: perm.AccessModeNone})
766-
refName := refFullName.ShortName()
767-
768766
if err := PrepareWebhooks(ctx, EventSource{Repository: repo}, webhook_module.HookEventCreate, &api.CreatePayload{
769-
Ref: refName, // FIXME: should it be a full ref name?
767+
Ref: refFullName.ShortName(), // FIXME: should it be a full ref name? But it will break the existing webhooks?
770768
Sha: refID,
771-
RefType: refFullName.RefType(),
769+
RefType: string(refFullName.RefType()),
772770
Repo: apiRepo,
773771
Sender: apiPusher,
774772
}); err != nil {
@@ -800,11 +798,9 @@ func (m *webhookNotifier) PullRequestSynchronized(ctx context.Context, doer *use
800798
func (m *webhookNotifier) DeleteRef(ctx context.Context, pusher *user_model.User, repo *repo_model.Repository, refFullName git.RefName) {
801799
apiPusher := convert.ToUser(ctx, pusher, nil)
802800
apiRepo := convert.ToRepo(ctx, repo, access_model.Permission{AccessMode: perm.AccessModeOwner})
803-
refName := refFullName.ShortName()
804-
805801
if err := PrepareWebhooks(ctx, EventSource{Repository: repo}, webhook_module.HookEventDelete, &api.DeletePayload{
806-
Ref: refName, // FIXME: should it be a full ref name?
807-
RefType: refFullName.RefType(),
802+
Ref: refFullName.ShortName(), // FIXME: should it be a full ref name? But it will break the existing webhooks?
803+
RefType: string(refFullName.RefType()),
808804
PusherType: api.PusherTypeUser,
809805
Repo: apiRepo,
810806
Sender: apiPusher,

‎services/webhook/slack.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,9 @@ func SlackLinkFormatter(url, text string) string {
8484
// SlackLinkToRef slack-formatter link to a repo ref
8585
func SlackLinkToRef(repoURL, ref string) string {
8686
// FIXME: SHA1 hardcoded here
87-
url := git.RefURL(repoURL, ref)
88-
refName := git.RefName(ref).ShortName()
89-
return SlackLinkFormatter(url, refName)
87+
refName := git.RefName(ref)
88+
url := repoURL + "/src/" + refName.RefWebLinkPath()
89+
return SlackLinkFormatter(url, refName.ShortName())
9090
}
9191

9292
// Create implements payloadConvertor Create method

0 commit comments

Comments
 (0)