Skip to content

Commit 846855a

Browse files
wxiaoguangwxiaoguang
committed
fix
1 parent 8135254 commit 846855a

File tree

13 files changed

+58
-60
lines changed

13 files changed

+58
-60
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/api/actions/runner/utils.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ func generateTaskContext(t *actions_model.ActionTask) *structpb.Struct {
120120
"ref": ref, // string, The fully-formed ref of the branch or tag that triggered the workflow run. For workflows triggered by push, this is the branch or tag ref that was pushed. For workflows triggered by pull_request, this is the pull request merge branch. For workflows triggered by release, this is the release tag created. For other triggers, this is the branch or tag ref that triggered the workflow run. This is only set if a branch or tag is available for the event type. The ref given is fully-formed, meaning that for branches the format is refs/heads/<branch_name>, for pull requests it is refs/pull/<pr_number>/merge, and for tags it is refs/tags/<tag_name>. For example, refs/heads/feature-branch-1.
121121
"ref_name": refName.ShortName(), // string, The short ref name of the branch or tag that triggered the workflow run. This value matches the branch or tag name shown on GitHub. For example, feature-branch-1.
122122
"ref_protected": false, // boolean, true if branch protections are configured for the ref that triggered the workflow run.
123-
"ref_type": refName.RefType(), // string, The type of ref that triggered the workflow run. Valid values are branch or tag.
123+
"ref_type": string(refName.RefType()), // string, The type of ref that triggered the workflow run. Valid values are branch or tag.
124124
"path": "", // string, Path on the runner to the file that sets system PATH variables from workflow commands. This file is unique to the current step and is a different file for each step in a job. For more information, see "Workflow commands for GitHub Actions."
125125
"repository": t.Job.Run.Repo.OwnerName + "/" + t.Job.Run.Repo.Name, // string, The owner and repository name. For example, Codertocat/Hello-World.
126126
"repository_owner": t.Job.Run.Repo.OwnerName, // string, The repository owner's name. For example, Codertocat.

‎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)