From 7dcccc3bb19655a6f83dd495ffc332708d0c8678 Mon Sep 17 00:00:00 2001 From: Rowan Bohde Date: Fri, 1 Nov 2024 22:29:37 -0500 Subject: [PATCH 001/156] improve performance of diffs (#32393) This has two major changes that significantly reduce the amount of work done for large diffs: * Kill a running git process when reaching the maximum number of files in a diff, preventing it from processing the entire diff. * When loading a diff with the URL param `file-only=true`, skip loading stats. This speeds up loading both hidden files of a diff and sections of a diff when clicking the "Show More" button. A couple of minor things from profiling are also included: * Reuse existing repo in `PrepareViewPullInfo` if head and base are the same. The performance impact is going to depend heavily on the individual diff and the hardware it runs on, but when testing locally on a diff changing 100k+ lines over hundreds of files, I'm seeing a roughly 75% reduction in time to load the result of "Show More" --------- Co-authored-by: wxiaoguang --- routers/web/repo/commit.go | 1 + routers/web/repo/compare.go | 3 ++ routers/web/repo/pull.go | 7 +-- services/gitdiff/gitdiff.go | 62 +++++++++++--------------- services/repository/files/temp_repo.go | 13 ++++-- 5 files changed, 44 insertions(+), 42 deletions(-) diff --git a/routers/web/repo/commit.go b/routers/web/repo/commit.go index 0e4e10bf50..d7865e18d6 100644 --- a/routers/web/repo/commit.go +++ b/routers/web/repo/commit.go @@ -328,6 +328,7 @@ func Diff(ctx *context.Context) { MaxLineCharacters: setting.Git.MaxGitDiffLineCharacters, MaxFiles: maxFiles, WhitespaceBehavior: gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)), + FileOnly: fileOnly, }, files...) if err != nil { ctx.NotFound("GetDiff", err) diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index e6a04782e5..3927972837 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -611,6 +611,8 @@ func PrepareCompareDiff( maxLines, maxFiles = -1, -1 } + fileOnly := ctx.FormBool("file-only") + diff, err := gitdiff.GetDiff(ctx, ci.HeadGitRepo, &gitdiff.DiffOptions{ BeforeCommitID: beforeCommitID, @@ -621,6 +623,7 @@ func PrepareCompareDiff( MaxFiles: maxFiles, WhitespaceBehavior: whitespaceBehavior, DirectComparison: ci.DirectComparison, + FileOnly: fileOnly, }, ctx.FormStrings("files")...) if err != nil { ctx.ServerError("GetDiffRangeWithWhitespaceBehavior", err) diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go index 0efe1be76a..cc554a71ff 100644 --- a/routers/web/repo/pull.go +++ b/routers/web/repo/pull.go @@ -395,12 +395,12 @@ func PrepareViewPullInfo(ctx *context.Context, issue *issues_model.Issue) *git.C var headBranchSha string // HeadRepo may be missing if pull.HeadRepo != nil { - headGitRepo, err := gitrepo.OpenRepository(ctx, pull.HeadRepo) + headGitRepo, closer, err := gitrepo.RepositoryFromContextOrOpen(ctx, pull.HeadRepo) if err != nil { - ctx.ServerError("OpenRepository", err) + ctx.ServerError("RepositoryFromContextOrOpen", err) return nil } - defer headGitRepo.Close() + defer closer.Close() if pull.Flow == issues_model.PullRequestFlowGithub { headBranchExist = headGitRepo.IsBranchExist(pull.HeadBranch) @@ -747,6 +747,7 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi MaxLineCharacters: setting.Git.MaxGitDiffLineCharacters, MaxFiles: maxFiles, WhitespaceBehavior: gitdiff.GetWhitespaceFlag(ctx.Data["WhitespaceBehavior"].(string)), + FileOnly: fileOnly, } if !willShowSpecifiedCommit { diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index 0ddd5a48e2..cf7da0707b 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -380,18 +380,11 @@ func (diffFile *DiffFile) GetType() int { } // GetTailSection creates a fake DiffLineSection if the last section is not the end of the file -func (diffFile *DiffFile) GetTailSection(gitRepo *git.Repository, leftCommitID, rightCommitID string) *DiffSection { +func (diffFile *DiffFile) GetTailSection(gitRepo *git.Repository, leftCommit, rightCommit *git.Commit) *DiffSection { if len(diffFile.Sections) == 0 || diffFile.Type != DiffFileChange || diffFile.IsBin || diffFile.IsLFSFile { return nil } - leftCommit, err := gitRepo.GetCommit(leftCommitID) - if err != nil { - return nil - } - rightCommit, err := gitRepo.GetCommit(rightCommitID) - if err != nil { - return nil - } + lastSection := diffFile.Sections[len(diffFile.Sections)-1] lastLine := lastSection.Lines[len(lastSection.Lines)-1] leftLineCount := getCommitFileLineCount(leftCommit, diffFile.Name) @@ -536,11 +529,6 @@ parsingLoop: lastFile := createDiffFile(diff, line) diff.End = lastFile.Name diff.IsIncomplete = true - _, err := io.Copy(io.Discard, reader) - if err != nil { - // By the definition of io.Copy this never returns io.EOF - return diff, fmt.Errorf("error during io.Copy: %w", err) - } break parsingLoop } @@ -1101,6 +1089,7 @@ type DiffOptions struct { MaxFiles int WhitespaceBehavior git.TrustedCmdArgs DirectComparison bool + FileOnly bool } // GetDiff builds a Diff between two commits of a repository. @@ -1109,12 +1098,16 @@ type DiffOptions struct { func GetDiff(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, files ...string) (*Diff, error) { repoPath := gitRepo.Path + var beforeCommit *git.Commit commit, err := gitRepo.GetCommit(opts.AfterCommitID) if err != nil { return nil, err } - cmdDiff := git.NewCommand(gitRepo.Ctx) + cmdCtx, cmdCancel := context.WithCancel(ctx) + defer cmdCancel() + + cmdDiff := git.NewCommand(cmdCtx) objectFormat, err := gitRepo.GetObjectFormat() if err != nil { return nil, err @@ -1136,6 +1129,12 @@ func GetDiff(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, fi AddArguments(opts.WhitespaceBehavior...). AddDynamicArguments(actualBeforeCommitID, opts.AfterCommitID) opts.BeforeCommitID = actualBeforeCommitID + + var err error + beforeCommit, err = gitRepo.GetCommit(opts.BeforeCommitID) + if err != nil { + return nil, err + } } // In git 2.31, git diff learned --skip-to which we can use to shortcut skip to file @@ -1163,14 +1162,16 @@ func GetDiff(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, fi Dir: repoPath, Stdout: writer, Stderr: stderr, - }); err != nil { + }); err != nil && err.Error() != "signal: killed" { log.Error("error during GetDiff(git diff dir: %s): %v, stderr: %s", repoPath, err, stderr.String()) } _ = writer.Close() }() - diff, err := ParsePatch(ctx, opts.MaxLines, opts.MaxLineCharacters, opts.MaxFiles, reader, parsePatchSkipToFile) + diff, err := ParsePatch(cmdCtx, opts.MaxLines, opts.MaxLineCharacters, opts.MaxFiles, reader, parsePatchSkipToFile) + // Ensure the git process is killed if it didn't exit already + cmdCancel() if err != nil { return nil, fmt.Errorf("unable to ParsePatch: %w", err) } @@ -1205,37 +1206,28 @@ func GetDiff(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, fi } diffFile.IsGenerated = isGenerated.Value() - tailSection := diffFile.GetTailSection(gitRepo, opts.BeforeCommitID, opts.AfterCommitID) + tailSection := diffFile.GetTailSection(gitRepo, beforeCommit, commit) if tailSection != nil { diffFile.Sections = append(diffFile.Sections, tailSection) } } - separator := "..." - if opts.DirectComparison { - separator = ".." + if opts.FileOnly { + return diff, nil } - diffPaths := []string{opts.BeforeCommitID + separator + opts.AfterCommitID} - if len(opts.BeforeCommitID) == 0 || opts.BeforeCommitID == objectFormat.EmptyObjectID().String() { - diffPaths = []string{objectFormat.EmptyTree().String(), opts.AfterCommitID} - } - diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(gitRepo.Ctx, repoPath, nil, diffPaths...) - if err != nil && strings.Contains(err.Error(), "no merge base") { - // git >= 2.28 now returns an error if base and head have become unrelated. - // previously it would return the results of git diff --shortstat base head so let's try that... - diffPaths = []string{opts.BeforeCommitID, opts.AfterCommitID} - diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(gitRepo.Ctx, repoPath, nil, diffPaths...) - } + stats, err := GetPullDiffStats(gitRepo, opts) if err != nil { return nil, err } + diff.NumFiles, diff.TotalAddition, diff.TotalDeletion = stats.NumFiles, stats.TotalAddition, stats.TotalDeletion + return diff, nil } type PullDiffStats struct { - TotalAddition, TotalDeletion int + NumFiles, TotalAddition, TotalDeletion int } // GetPullDiffStats @@ -1259,12 +1251,12 @@ func GetPullDiffStats(gitRepo *git.Repository, opts *DiffOptions) (*PullDiffStat diffPaths = []string{objectFormat.EmptyTree().String(), opts.AfterCommitID} } - _, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(gitRepo.Ctx, repoPath, nil, diffPaths...) + diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(gitRepo.Ctx, repoPath, nil, diffPaths...) if err != nil && strings.Contains(err.Error(), "no merge base") { // git >= 2.28 now returns an error if base and head have become unrelated. // previously it would return the results of git diff --shortstat base head so let's try that... diffPaths = []string{opts.BeforeCommitID, opts.AfterCommitID} - _, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(gitRepo.Ctx, repoPath, nil, diffPaths...) + diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(gitRepo.Ctx, repoPath, nil, diffPaths...) } if err != nil { return nil, err diff --git a/services/repository/files/temp_repo.go b/services/repository/files/temp_repo.go index d70b1e8d54..ce98967e79 100644 --- a/services/repository/files/temp_repo.go +++ b/services/repository/files/temp_repo.go @@ -358,6 +358,7 @@ func (t *TemporaryUploadRepository) DiffIndex() (*gitdiff.Diff, error) { Stderr: stderr, PipelineFunc: func(ctx context.Context, cancel context.CancelFunc) error { _ = stdoutWriter.Close() + defer cancel() diff, finalErr = gitdiff.ParsePatch(t.ctx, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, stdoutReader, "") if finalErr != nil { log.Error("ParsePatch: %v", finalErr) @@ -371,10 +372,14 @@ func (t *TemporaryUploadRepository) DiffIndex() (*gitdiff.Diff, error) { log.Error("Unable to ParsePatch in temporary repo %s (%s). Error: %v", t.repo.FullName(), t.basePath, finalErr) return nil, finalErr } - log.Error("Unable to run diff-index pipeline in temporary repo %s (%s). Error: %v\nStderr: %s", - t.repo.FullName(), t.basePath, err, stderr) - return nil, fmt.Errorf("Unable to run diff-index pipeline in temporary repo %s. Error: %w\nStderr: %s", - t.repo.FullName(), err, stderr) + + // If the process exited early, don't error + if err != context.Canceled { + log.Error("Unable to run diff-index pipeline in temporary repo %s (%s). Error: %v\nStderr: %s", + t.repo.FullName(), t.basePath, err, stderr) + return nil, fmt.Errorf("Unable to run diff-index pipeline in temporary repo %s. Error: %w\nStderr: %s", + t.repo.FullName(), err, stderr) + } } diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(t.ctx, t.basePath, git.TrustedCmdArgs{"--cached"}, "HEAD") From fec6b3d50072e48bb51c18c5c4ea682dc6319573 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 2 Nov 2024 12:08:28 +0800 Subject: [PATCH 002/156] Replace DateTime with DateUtils (#32383) --- modules/templates/helper.go | 1 + modules/templates/util_date.go | 34 +++++++++++++++++++ routers/web/repo/activity.go | 4 +-- services/context/context.go | 1 + templates/admin/auth/list.tmpl | 4 +-- templates/admin/cron.tmpl | 4 +-- templates/admin/notice.tmpl | 2 +- templates/admin/org/list.tmpl | 2 +- templates/admin/packages/list.tmpl | 2 +- templates/admin/repo/list.tmpl | 4 +-- templates/admin/user/list.tmpl | 4 +-- templates/explore/user_list.tmpl | 2 +- .../package/shared/cleanup_rules/preview.tmpl | 2 +- templates/package/view.tmpl | 2 +- templates/repo/diff/compare.tmpl | 2 +- templates/repo/empty.tmpl | 2 +- templates/repo/graph/commits.tmpl | 2 +- templates/repo/home.tmpl | 2 +- .../repo/issue/view_content/sidebar.tmpl | 2 +- templates/repo/pulse.tmpl | 2 +- templates/repo/settings/deploy_keys.tmpl | 2 +- templates/repo/settings/options.tmpl | 4 +-- templates/repo/user_cards.tmpl | 2 +- templates/shared/issuelist.tmpl | 2 +- templates/shared/secrets/add_list.tmpl | 2 +- templates/shared/user/profile_big_avatar.tmpl | 2 +- templates/shared/variables/variable_list.tmpl | 2 +- templates/user/settings/applications.tmpl | 2 +- templates/user/settings/grants_oauth2.tmpl | 2 +- templates/user/settings/keys_gpg.tmpl | 4 +-- templates/user/settings/keys_principal.tmpl | 2 +- templates/user/settings/keys_ssh.tmpl | 2 +- .../user/settings/security/webauthn.tmpl | 2 +- 33 files changed, 73 insertions(+), 37 deletions(-) create mode 100644 modules/templates/util_date.go diff --git a/modules/templates/helper.go b/modules/templates/helper.go index 5f73e6b278..5038e8a132 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -54,6 +54,7 @@ func NewFuncMap() template.FuncMap { "StringUtils": NewStringUtils, "SliceUtils": NewSliceUtils, "JsonUtils": NewJsonUtils, + "DateUtils": NewDateUtils, // TODO: to be replaced by DateUtils // ----------------------------------------------------------------- // svg / avatar / icon / color diff --git a/modules/templates/util_date.go b/modules/templates/util_date.go new file mode 100644 index 0000000000..ec48a7e4be --- /dev/null +++ b/modules/templates/util_date.go @@ -0,0 +1,34 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package templates + +import ( + "context" + "html/template" + + "code.gitea.io/gitea/modules/timeutil" +) + +type DateUtils struct { + ctx context.Context +} + +func NewDateUtils(ctx context.Context) *DateUtils { + return &DateUtils{ctx} +} + +// AbsoluteShort renders in "Jan 01, 2006" format +func (du *DateUtils) AbsoluteShort(time any) template.HTML { + return timeutil.DateTime("short", time) +} + +// AbsoluteLong renders in "January 01, 2006" format +func (du *DateUtils) AbsoluteLong(time any) template.HTML { + return timeutil.DateTime("short", time) +} + +// FullTime renders in "Jan 01, 2006 20:33:44" format +func (du *DateUtils) FullTime(time any) template.HTML { + return timeutil.DateTime("full", time) +} diff --git a/routers/web/repo/activity.go b/routers/web/repo/activity.go index 4b14c28b3e..65dd9e392f 100644 --- a/routers/web/repo/activity.go +++ b/routers/web/repo/activity.go @@ -48,8 +48,8 @@ func Activity(ctx *context.Context) { ctx.Data["Period"] = "weekly" timeFrom = timeUntil.Add(-time.Hour * 168) } - ctx.Data["DateFrom"] = timeFrom.UTC().Format(time.RFC3339) - ctx.Data["DateUntil"] = timeUntil.UTC().Format(time.RFC3339) + ctx.Data["DateFrom"] = timeFrom + ctx.Data["DateUntil"] = timeUntil ctx.Data["PeriodText"] = ctx.Tr("repo.activity.period." + ctx.Data["Period"].(string)) var err error diff --git a/services/context/context.go b/services/context/context.go index 42f7c3d9d1..322f228a4d 100644 --- a/services/context/context.go +++ b/services/context/context.go @@ -100,6 +100,7 @@ func NewTemplateContextForWeb(ctx *Context) TemplateContext { tmplCtx := NewTemplateContext(ctx) tmplCtx["Locale"] = ctx.Base.Locale tmplCtx["AvatarUtils"] = templates.NewAvatarUtils(ctx) + tmplCtx["DateUtils"] = templates.NewDateUtils(ctx) tmplCtx["RootData"] = ctx.Data tmplCtx["Consts"] = map[string]any{ "RepoUnitTypeCode": unit.TypeCode, diff --git a/templates/admin/auth/list.tmpl b/templates/admin/auth/list.tmpl index 7057169895..4d4f022f41 100644 --- a/templates/admin/auth/list.tmpl +++ b/templates/admin/auth/list.tmpl @@ -26,8 +26,8 @@ {{.Name}} {{.TypeName}} {{svg (Iif .IsActive "octicon-check" "octicon-x")}} - {{DateTime "short" .UpdatedUnix}} - {{DateTime "short" .CreatedUnix}} + {{ctx.DateUtils.AbsoluteShort .UpdatedUnix}} + {{ctx.DateUtils.AbsoluteShort .CreatedUnix}} {{svg "octicon-pencil"}} {{end}} diff --git a/templates/admin/cron.tmpl b/templates/admin/cron.tmpl index 1c16ed00ae..1174813f83 100644 --- a/templates/admin/cron.tmpl +++ b/templates/admin/cron.tmpl @@ -23,8 +23,8 @@ {{ctx.Locale.Tr (printf "admin.dashboard.%s" .Name)}} {{.Spec}} - {{DateTime "full" .Next}} - {{if gt .Prev.Year 1}}{{DateTime "full" .Prev}}{{else}}-{{end}} + {{ctx.DateUtils.FullTime .Next}} + {{if gt .Prev.Year 1}}{{ctx.DateUtils.FullTime .Prev}}{{else}}-{{end}} {{.ExecTimes}} {{if eq .Status ""}}—{{else}}{{svg (Iif (eq .Status "finished") "octicon-check" "octicon-x") 16}}{{end}} diff --git a/templates/admin/notice.tmpl b/templates/admin/notice.tmpl index 6e7eed7678..395e1dcd46 100644 --- a/templates/admin/notice.tmpl +++ b/templates/admin/notice.tmpl @@ -21,7 +21,7 @@ {{.ID}} {{ctx.Locale.Tr .TrStr}} {{.Description}} - {{DateTime "short" .CreatedUnix}} + {{ctx.DateUtils.AbsoluteShort .CreatedUnix}} {{svg "octicon-note" 16}} {{end}} diff --git a/templates/admin/org/list.tmpl b/templates/admin/org/list.tmpl index 987ceab1e0..6a6dc14609 100644 --- a/templates/admin/org/list.tmpl +++ b/templates/admin/org/list.tmpl @@ -63,7 +63,7 @@ {{.NumTeams}} {{.NumMembers}} {{.NumRepos}} - {{DateTime "short" .CreatedUnix}} + {{ctx.DateUtils.AbsoluteShort .CreatedUnix}} {{svg "octicon-pencil"}} {{end}} diff --git a/templates/admin/packages/list.tmpl b/templates/admin/packages/list.tmpl index a5ad93b89c..6f5cef7a7b 100644 --- a/templates/admin/packages/list.tmpl +++ b/templates/admin/packages/list.tmpl @@ -71,7 +71,7 @@ {{end}} {{FileSize .CalculateBlobSize}} - {{DateTime "short" .Version.CreatedUnix}} + {{ctx.DateUtils.AbsoluteShort .Version.CreatedUnix}} {{svg "octicon-trash"}} {{end}} diff --git a/templates/admin/repo/list.tmpl b/templates/admin/repo/list.tmpl index 77a275427a..1fd2f25fcf 100644 --- a/templates/admin/repo/list.tmpl +++ b/templates/admin/repo/list.tmpl @@ -82,8 +82,8 @@ {{.NumIssues}} {{FileSize .GitSize}} {{FileSize .LFSSize}} - {{DateTime "short" .UpdatedUnix}} - {{DateTime "short" .CreatedUnix}} + {{ctx.DateUtils.AbsoluteShort .UpdatedUnix}} + {{ctx.DateUtils.AbsoluteShort .CreatedUnix}} {{svg "octicon-trash"}} {{end}} diff --git a/templates/admin/user/list.tmpl b/templates/admin/user/list.tmpl index bc3d83fc5c..f7218ac2fa 100644 --- a/templates/admin/user/list.tmpl +++ b/templates/admin/user/list.tmpl @@ -96,9 +96,9 @@ {{svg (Iif .IsActive "octicon-check" "octicon-x")}} {{svg (Iif .IsRestricted "octicon-check" "octicon-x")}} {{svg (Iif (index $.UsersTwoFaStatus .ID) "octicon-check" "octicon-x")}} - {{DateTime "short" .CreatedUnix}} + {{ctx.DateUtils.AbsoluteShort .CreatedUnix}} {{if .LastLoginUnix}} - {{DateTime "short" .LastLoginUnix}} + {{ctx.DateUtils.AbsoluteShort .LastLoginUnix}} {{else}} {{ctx.Locale.Tr "admin.users.never_login"}} {{end}} diff --git a/templates/explore/user_list.tmpl b/templates/explore/user_list.tmpl index f2cf939ffb..ff46f13c17 100644 --- a/templates/explore/user_list.tmpl +++ b/templates/explore/user_list.tmpl @@ -21,7 +21,7 @@ {{.Email}} {{end}} - {{svg "octicon-calendar"}}{{ctx.Locale.Tr "user.joined_on" (DateTime "short" .CreatedUnix)}} + {{svg "octicon-calendar"}}{{ctx.Locale.Tr "user.joined_on" (ctx.DateUtils.AbsoluteShort .CreatedUnix)}} diff --git a/templates/package/shared/cleanup_rules/preview.tmpl b/templates/package/shared/cleanup_rules/preview.tmpl index cff8e8249f..f34112e026 100644 --- a/templates/package/shared/cleanup_rules/preview.tmpl +++ b/templates/package/shared/cleanup_rules/preview.tmpl @@ -22,7 +22,7 @@ {{.Version.Version}} {{.Creator.Name}} {{FileSize .CalculateBlobSize}} - {{DateTime "short" .Version.CreatedUnix}} + {{ctx.DateUtils.AbsoluteShort .Version.CreatedUnix}} {{else}} diff --git a/templates/package/view.tmpl b/templates/package/view.tmpl index 6beb249a7f..d104788483 100644 --- a/templates/package/view.tmpl +++ b/templates/package/view.tmpl @@ -92,7 +92,7 @@ {{range .LatestVersions}}
{{.Version}} - {{DateTime "short" .CreatedUnix}} + {{ctx.DateUtils.AbsoluteShort .CreatedUnix}}
{{end}} diff --git a/templates/repo/diff/compare.tmpl b/templates/repo/diff/compare.tmpl index f927501197..3e00700eb0 100644 --- a/templates/repo/diff/compare.tmpl +++ b/templates/repo/diff/compare.tmpl @@ -204,7 +204,7 @@ {{if .Repository.ArchivedUnix.IsZero}} {{ctx.Locale.Tr "repo.archive.title"}} {{else}} - {{ctx.Locale.Tr "repo.archive.title_date" (DateTime "long" .Repository.ArchivedUnix)}} + {{ctx.Locale.Tr "repo.archive.title_date" (ctx.DateUtils.AbsoluteLong .Repository.ArchivedUnix)}} {{end}} {{end}} diff --git a/templates/repo/empty.tmpl b/templates/repo/empty.tmpl index 7613643351..d7f05223af 100644 --- a/templates/repo/empty.tmpl +++ b/templates/repo/empty.tmpl @@ -10,7 +10,7 @@ {{if .Repository.ArchivedUnix.IsZero}} {{ctx.Locale.Tr "repo.archive.title"}} {{else}} - {{ctx.Locale.Tr "repo.archive.title_date" (DateTime "long" .Repository.ArchivedUnix)}} + {{ctx.Locale.Tr "repo.archive.title_date" (ctx.DateUtils.AbsoluteLong .Repository.ArchivedUnix)}} {{end}} {{end}} diff --git a/templates/repo/graph/commits.tmpl b/templates/repo/graph/commits.tmpl index 9b179552df..15443dec06 100644 --- a/templates/repo/graph/commits.tmpl +++ b/templates/repo/graph/commits.tmpl @@ -69,7 +69,7 @@ {{$userName}} {{end}} - {{DateTime "full" $commit.Date}} + {{ctx.DateUtils.FullTime $commit.Date}} {{end}} {{end}} diff --git a/templates/repo/home.tmpl b/templates/repo/home.tmpl index ff82f2ca80..f33a230c4b 100644 --- a/templates/repo/home.tmpl +++ b/templates/repo/home.tmpl @@ -37,7 +37,7 @@ {{if .Repository.ArchivedUnix.IsZero}} {{ctx.Locale.Tr "repo.archive.title"}} {{else}} - {{ctx.Locale.Tr "repo.archive.title_date" (DateTime "long" .Repository.ArchivedUnix)}} + {{ctx.Locale.Tr "repo.archive.title_date" (ctx.DateUtils.AbsoluteLong .Repository.ArchivedUnix)}} {{end}} {{end}} diff --git a/templates/repo/issue/view_content/sidebar.tmpl b/templates/repo/issue/view_content/sidebar.tmpl index e49e90df56..69e4725fa1 100644 --- a/templates/repo/issue/view_content/sidebar.tmpl +++ b/templates/repo/issue/view_content/sidebar.tmpl @@ -368,7 +368,7 @@
{{svg "octicon-calendar" 16 "tw-mr-2"}} - {{DateTime "long" .Issue.DeadlineUnix.FormatDate}} + {{ctx.DateUtils.AbsoluteLong .Issue.DeadlineUnix}}
{{if and .HasIssuesOrPullsWritePermission (not .Repository.IsArchived)}} diff --git a/templates/repo/pulse.tmpl b/templates/repo/pulse.tmpl index e68109b755..0fe3da1884 100644 --- a/templates/repo/pulse.tmpl +++ b/templates/repo/pulse.tmpl @@ -1,5 +1,5 @@

- {{DateTime "long" .DateFrom}} - {{DateTime "long" .DateUntil}} + {{ctx.DateUtils.AbsoluteLong .DateFrom}} - {{ctx.DateUtils.AbsoluteLong .DateUntil}}
diff --git a/templates/repo/settings/options.tmpl b/templates/repo/settings/options.tmpl index 8f71f0020f..1537a90043 100644 --- a/templates/repo/settings/options.tmpl +++ b/templates/repo/settings/options.tmpl @@ -117,7 +117,7 @@ {{.PullMirror.RemoteAddress}} {{ctx.Locale.Tr "repo.settings.mirror_settings.direction.pull"}} - {{DateTime "full" .PullMirror.UpdatedUnix}} + {{ctx.DateUtils.FullTime .PullMirror.UpdatedUnix}}
{{.CsrfTokenHtml}} @@ -205,7 +205,7 @@ {{.RemoteAddress}} {{ctx.Locale.Tr "repo.settings.mirror_settings.direction.push"}} - {{if .LastUpdateUnix}}{{DateTime "full" .LastUpdateUnix}}{{else}}{{ctx.Locale.Tr "never"}}{{end}} {{if .LastError}}
{{ctx.Locale.Tr "error"}}
{{end}} + {{if .LastUpdateUnix}}{{ctx.DateUtils.FullTime .LastUpdateUnix}}{{else}}{{ctx.Locale.Tr "never"}}{{end}} {{if .LastError}}
{{ctx.Locale.Tr "error"}}
{{end}}
- {{ctx.Locale.Tr "settings.added_on" (DateTime "short" .CreatedUnix)}} + {{ctx.Locale.Tr "settings.added_on" (ctx.DateUtils.AbsoluteShort .CreatedUnix)}}
- {{ctx.Locale.Tr "settings.added_on" (DateTime "short" .CreatedUnix)}} + {{ctx.Locale.Tr "settings.added_on" (ctx.DateUtils.AbsoluteShort .CreatedUnix)}}
diff --git a/templates/user/settings/grants_oauth2.tmpl b/templates/user/settings/grants_oauth2.tmpl index b5ae3e0337..6ccab492aa 100644 --- a/templates/user/settings/grants_oauth2.tmpl +++ b/templates/user/settings/grants_oauth2.tmpl @@ -14,7 +14,7 @@
{{.Application.Name}}
- {{ctx.Locale.Tr "settings.added_on" (DateTime "short" .CreatedUnix)}} + {{ctx.Locale.Tr "settings.added_on" (ctx.DateUtils.AbsoluteShort .CreatedUnix)}}
diff --git a/templates/user/settings/keys_gpg.tmpl b/templates/user/settings/keys_gpg.tmpl index d86d838f18..a806f87df8 100644 --- a/templates/user/settings/keys_gpg.tmpl +++ b/templates/user/settings/keys_gpg.tmpl @@ -63,9 +63,9 @@ {{ctx.Locale.Tr "settings.subkeys"}}: {{range .SubsKey}} {{.PaddedKeyID}} {{end}}
- {{ctx.Locale.Tr "settings.added_on" (DateTime "short" .AddedUnix)}} + {{ctx.Locale.Tr "settings.added_on" (ctx.DateUtils.AbsoluteShort .AddedUnix)}} - - {{if not .ExpiredUnix.IsZero}}{{ctx.Locale.Tr "settings.valid_until_date" (DateTime "short" .ExpiredUnix)}}{{else}}{{ctx.Locale.Tr "settings.valid_forever"}}{{end}} + {{if not .ExpiredUnix.IsZero}}{{ctx.Locale.Tr "settings.valid_until_date" (ctx.DateUtils.AbsoluteShort .ExpiredUnix)}}{{else}}{{ctx.Locale.Tr "settings.valid_forever"}}{{end}}
diff --git a/templates/user/settings/keys_principal.tmpl b/templates/user/settings/keys_principal.tmpl index 37d8fb0e95..484a29e22a 100644 --- a/templates/user/settings/keys_principal.tmpl +++ b/templates/user/settings/keys_principal.tmpl @@ -22,7 +22,7 @@
{{.Name}}
- {{ctx.Locale.Tr "settings.added_on" (DateTime "short" .CreatedUnix)}} — {{svg "octicon-info" 16}} {{if .HasUsed}}{{ctx.Locale.Tr "settings.last_used"}} {{DateTime "short" .UpdatedUnix}}{{else}}{{ctx.Locale.Tr "settings.no_activity"}}{{end}} + {{ctx.Locale.Tr "settings.added_on" (ctx.DateUtils.AbsoluteShort .CreatedUnix)}} — {{svg "octicon-info" 16}} {{if .HasUsed}}{{ctx.Locale.Tr "settings.last_used"}} {{ctx.DateUtils.AbsoluteShort .UpdatedUnix}}{{else}}{{ctx.Locale.Tr "settings.no_activity"}}{{end}}
diff --git a/templates/user/settings/keys_ssh.tmpl b/templates/user/settings/keys_ssh.tmpl index a2af1b7f82..0f72affa78 100644 --- a/templates/user/settings/keys_ssh.tmpl +++ b/templates/user/settings/keys_ssh.tmpl @@ -53,7 +53,7 @@ {{.Fingerprint}}
- {{ctx.Locale.Tr "settings.added_on" (DateTime "short" .CreatedUnix)}} — {{svg "octicon-info"}} {{if .HasUsed}}{{ctx.Locale.Tr "settings.last_used"}} {{DateTime "short" .UpdatedUnix}}{{else}}{{ctx.Locale.Tr "settings.no_activity"}}{{end}} + {{ctx.Locale.Tr "settings.added_on" (ctx.DateUtils.AbsoluteShort .CreatedUnix)}} — {{svg "octicon-info"}} {{if .HasUsed}}{{ctx.Locale.Tr "settings.last_used"}} {{ctx.DateUtils.AbsoluteShort .UpdatedUnix}}{{else}}{{ctx.Locale.Tr "settings.no_activity"}}{{end}}
diff --git a/templates/user/settings/security/webauthn.tmpl b/templates/user/settings/security/webauthn.tmpl index 280ece9175..7a04a2681f 100644 --- a/templates/user/settings/security/webauthn.tmpl +++ b/templates/user/settings/security/webauthn.tmpl @@ -12,7 +12,7 @@
{{.Name}}
- {{ctx.Locale.Tr "settings.added_on" (DateTime "short" .CreatedUnix)}} + {{ctx.Locale.Tr "settings.added_on" (ctx.DateUtils.AbsoluteShort .CreatedUnix)}}
From 13a203828c40f9ad1005b16b4ae26256a7df8263 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 1 Nov 2024 23:11:38 -0700 Subject: [PATCH 003/156] Fix created_unix for mirroring (#32342) Fix #32233 --- modules/repository/repo.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/modules/repository/repo.go b/modules/repository/repo.go index 3d1899b2fe..def2220b17 100644 --- a/modules/repository/repo.go +++ b/modules/repository/repo.go @@ -340,9 +340,10 @@ func pullMirrorReleaseSync(ctx context.Context, repo *repo_model.Repository, git for _, tag := range updates { if _, err := db.GetEngine(ctx).Where("repo_id = ? AND lower_tag_name = ?", repo.ID, strings.ToLower(tag.Name)). - Cols("sha1"). + Cols("sha1", "created_unix"). Update(&repo_model.Release{ - Sha1: tag.Object.String(), + Sha1: tag.Object.String(), + CreatedUnix: timeutil.TimeStamp(tag.Tagger.When.Unix()), }); err != nil { return fmt.Errorf("unable to update tag %s for pull-mirror Repo[%d:%s/%s]: %w", tag.Name, repo.ID, repo.OwnerName, repo.Name, err) } From e524f63d58900557d7d57fc3bcd19d9facc8b8ee Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sat, 2 Nov 2024 19:20:22 +0800 Subject: [PATCH 004/156] Fix git error handling (#32401) --- modules/git/error.go | 40 ++++++-------------------- modules/git/repo_attribute.go | 4 +-- routers/api/v1/repo/repo.go | 7 ++--- routers/private/default_branch.go | 11 +++---- routers/web/repo/githttp.go | 2 +- services/gitdiff/gitdiff.go | 2 +- services/mirror/mirror_pull.go | 10 ++----- services/repository/branch.go | 7 +---- services/repository/files/temp_repo.go | 37 +++++++++--------------- services/repository/push.go | 4 +-- 10 files changed, 35 insertions(+), 89 deletions(-) diff --git a/modules/git/error.go b/modules/git/error.go index 91d25eca69..10fb37be07 100644 --- a/modules/git/error.go +++ b/modules/git/error.go @@ -4,28 +4,14 @@ package git import ( + "context" + "errors" "fmt" "strings" - "time" "code.gitea.io/gitea/modules/util" ) -// ErrExecTimeout error when exec timed out -type ErrExecTimeout struct { - Duration time.Duration -} - -// IsErrExecTimeout if some error is ErrExecTimeout -func IsErrExecTimeout(err error) bool { - _, ok := err.(ErrExecTimeout) - return ok -} - -func (err ErrExecTimeout) Error() string { - return fmt.Sprintf("execution is timeout [duration: %v]", err.Duration) -} - // ErrNotExist commit not exist error type ErrNotExist struct { ID string @@ -62,21 +48,6 @@ func IsErrBadLink(err error) bool { return ok } -// ErrUnsupportedVersion error when required git version not matched -type ErrUnsupportedVersion struct { - Required string -} - -// IsErrUnsupportedVersion if some error is ErrUnsupportedVersion -func IsErrUnsupportedVersion(err error) bool { - _, ok := err.(ErrUnsupportedVersion) - return ok -} - -func (err ErrUnsupportedVersion) Error() string { - return fmt.Sprintf("Operation requires higher version [required: %s]", err.Required) -} - // ErrBranchNotExist represents a "BranchNotExist" kind of error. type ErrBranchNotExist struct { Name string @@ -185,3 +156,10 @@ func IsErrMoreThanOne(err error) bool { func (err *ErrMoreThanOne) Error() string { return fmt.Sprintf("ErrMoreThanOne Error: %v: %s\n%s", err.Err, err.StdErr, err.StdOut) } + +func IsErrCanceledOrKilled(err error) bool { + // When "cancel()" a git command's context, the returned error of "Run()" could be one of them: + // - context.Canceled + // - *exec.ExitError: "signal: killed" + return err != nil && (errors.Is(err, context.Canceled) || err.Error() == "signal: killed") +} diff --git a/modules/git/repo_attribute.go b/modules/git/repo_attribute.go index 84f85d1b1a..90eb783fe8 100644 --- a/modules/git/repo_attribute.go +++ b/modules/git/repo_attribute.go @@ -166,9 +166,7 @@ func (c *CheckAttributeReader) Run() error { Stdout: c.stdOut, Stderr: stdErr, }) - if err != nil && // If there is an error we need to return but: - c.ctx.Err() != err && // 1. Ignore the context error if the context is cancelled or exceeds the deadline (RunWithContext could return c.ctx.Err() which is Canceled or DeadlineExceeded) - err.Error() != "signal: killed" { // 2. We should not pass up errors due to the program being killed + if err != nil && !IsErrCanceledOrKilled(err) { return fmt.Errorf("failed to run attr-check. Error: %w\nStderr: %s", err, stdErr.String()) } return nil diff --git a/routers/api/v1/repo/repo.go b/routers/api/v1/repo/repo.go index 698ba3cc94..69a95dd5a5 100644 --- a/routers/api/v1/repo/repo.go +++ b/routers/api/v1/repo/repo.go @@ -21,7 +21,6 @@ import ( repo_model "code.gitea.io/gitea/models/repo" unit_model "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" - "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/label" "code.gitea.io/gitea/modules/log" @@ -739,10 +738,8 @@ func updateBasicProperties(ctx *context.APIContext, opts api.EditRepoOption) err if opts.DefaultBranch != nil && repo.DefaultBranch != *opts.DefaultBranch && (repo.IsEmpty || ctx.Repo.GitRepo.IsBranchExist(*opts.DefaultBranch)) { if !repo.IsEmpty { if err := gitrepo.SetDefaultBranch(ctx, ctx.Repo.Repository, *opts.DefaultBranch); err != nil { - if !git.IsErrUnsupportedVersion(err) { - ctx.Error(http.StatusInternalServerError, "SetDefaultBranch", err) - return err - } + ctx.Error(http.StatusInternalServerError, "SetDefaultBranch", err) + return err } updateRepoLicense = true } diff --git a/routers/private/default_branch.go b/routers/private/default_branch.go index 03c19c8ff4..8f6e9084df 100644 --- a/routers/private/default_branch.go +++ b/routers/private/default_branch.go @@ -8,7 +8,6 @@ import ( "net/http" repo_model "code.gitea.io/gitea/models/repo" - "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/private" gitea_context "code.gitea.io/gitea/services/context" @@ -23,12 +22,10 @@ func SetDefaultBranch(ctx *gitea_context.PrivateContext) { ctx.Repo.Repository.DefaultBranch = branch if err := gitrepo.SetDefaultBranch(ctx, ctx.Repo.Repository, ctx.Repo.Repository.DefaultBranch); err != nil { - if !git.IsErrUnsupportedVersion(err) { - ctx.JSON(http.StatusInternalServerError, private.Response{ - Err: fmt.Sprintf("Unable to set default branch on repository: %s/%s Error: %v", ownerName, repoName, err), - }) - return - } + ctx.JSON(http.StatusInternalServerError, private.Response{ + Err: fmt.Sprintf("Unable to set default branch on repository: %s/%s Error: %v", ownerName, repoName, err), + }) + return } if err := repo_model.UpdateDefaultBranch(ctx, ctx.Repo.Repository); err != nil { diff --git a/routers/web/repo/githttp.go b/routers/web/repo/githttp.go index ee1ec1fd0c..58a2bdbab1 100644 --- a/routers/web/repo/githttp.go +++ b/routers/web/repo/githttp.go @@ -467,7 +467,7 @@ func serviceRPC(ctx *context.Context, h *serviceHandler, service string) { Stderr: &stderr, UseContextTimeout: true, }); err != nil { - if err.Error() != "signal: killed" { + if !git.IsErrCanceledOrKilled(err) { log.Error("Fail to serve RPC(%s) in %s: %v - %s", service, h.getRepoDir(), err, stderr.String()) } return diff --git a/services/gitdiff/gitdiff.go b/services/gitdiff/gitdiff.go index cf7da0707b..bb1722039e 100644 --- a/services/gitdiff/gitdiff.go +++ b/services/gitdiff/gitdiff.go @@ -1162,7 +1162,7 @@ func GetDiff(ctx context.Context, gitRepo *git.Repository, opts *DiffOptions, fi Dir: repoPath, Stdout: writer, Stderr: stderr, - }); err != nil && err.Error() != "signal: killed" { + }); err != nil && !git.IsErrCanceledOrKilled(err) { log.Error("error during GetDiff(git diff dir: %s): %v, stderr: %s", repoPath, err, stderr.String()) } diff --git a/services/mirror/mirror_pull.go b/services/mirror/mirror_pull.go index 654a50d11e..a81fe6e9bd 100644 --- a/services/mirror/mirror_pull.go +++ b/services/mirror/mirror_pull.go @@ -613,14 +613,8 @@ func checkAndUpdateEmptyRepository(ctx context.Context, m *repo_model.Mirror, re } // Update the git repository default branch if err := gitrepo.SetDefaultBranch(ctx, m.Repo, m.Repo.DefaultBranch); err != nil { - if !git.IsErrUnsupportedVersion(err) { - log.Error("Failed to update default branch of underlying git repository %-v. Error: %v", m.Repo, err) - desc := fmt.Sprintf("Failed to update default branch of underlying git repository '%s': %v", m.Repo.RepoPath(), err) - if err = system_model.CreateRepositoryNotice(desc); err != nil { - log.Error("CreateRepositoryNotice: %v", err) - } - return false - } + log.Error("Failed to update default branch of underlying git repository %-v. Error: %v", m.Repo, err) + return false } m.Repo.IsEmpty = false // Update the is empty and default_branch columns diff --git a/services/repository/branch.go b/services/repository/branch.go index 67df4363e4..600ba96e92 100644 --- a/services/repository/branch.go +++ b/services/repository/branch.go @@ -602,12 +602,7 @@ func SetRepoDefaultBranch(ctx context.Context, repo *repo_model.Repository, gitR log.Error("CancelPreviousJobs: %v", err) } - if err := gitrepo.SetDefaultBranch(ctx, repo, newBranchName); err != nil { - if !git.IsErrUnsupportedVersion(err) { - return err - } - } - return nil + return gitrepo.SetDefaultBranch(ctx, repo, newBranchName) }); err != nil { return err } diff --git a/services/repository/files/temp_repo.go b/services/repository/files/temp_repo.go index ce98967e79..30ab22db1e 100644 --- a/services/repository/files/temp_repo.go +++ b/services/repository/files/temp_repo.go @@ -339,8 +339,7 @@ func (t *TemporaryUploadRepository) Push(doer *user_model.User, commitHash, bran func (t *TemporaryUploadRepository) DiffIndex() (*gitdiff.Diff, error) { stdoutReader, stdoutWriter, err := os.Pipe() if err != nil { - log.Error("Unable to open stdout pipe: %v", err) - return nil, fmt.Errorf("Unable to open stdout pipe: %w", err) + return nil, fmt.Errorf("unable to open stdout pipe: %w", err) } defer func() { _ = stdoutReader.Close() @@ -348,9 +347,7 @@ func (t *TemporaryUploadRepository) DiffIndex() (*gitdiff.Diff, error) { }() stderr := new(bytes.Buffer) var diff *gitdiff.Diff - var finalErr error - - if err := git.NewCommand(t.ctx, "diff-index", "--src-prefix=\\a/", "--dst-prefix=\\b/", "--cached", "-p", "HEAD"). + err = git.NewCommand(t.ctx, "diff-index", "--src-prefix=\\a/", "--dst-prefix=\\b/", "--cached", "-p", "HEAD"). Run(&git.RunOpts{ Timeout: 30 * time.Second, Dir: t.basePath, @@ -359,27 +356,19 @@ func (t *TemporaryUploadRepository) DiffIndex() (*gitdiff.Diff, error) { PipelineFunc: func(ctx context.Context, cancel context.CancelFunc) error { _ = stdoutWriter.Close() defer cancel() - diff, finalErr = gitdiff.ParsePatch(t.ctx, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, stdoutReader, "") - if finalErr != nil { - log.Error("ParsePatch: %v", finalErr) - cancel() - } + var diffErr error + diff, diffErr = gitdiff.ParsePatch(t.ctx, setting.Git.MaxGitDiffLines, setting.Git.MaxGitDiffLineCharacters, setting.Git.MaxGitDiffFiles, stdoutReader, "") _ = stdoutReader.Close() - return finalErr + if diffErr != nil { + // if the diffErr is not nil, it will be returned as the error of "Run()" + return fmt.Errorf("ParsePatch: %w", diffErr) + } + return nil }, - }); err != nil { - if finalErr != nil { - log.Error("Unable to ParsePatch in temporary repo %s (%s). Error: %v", t.repo.FullName(), t.basePath, finalErr) - return nil, finalErr - } - - // If the process exited early, don't error - if err != context.Canceled { - log.Error("Unable to run diff-index pipeline in temporary repo %s (%s). Error: %v\nStderr: %s", - t.repo.FullName(), t.basePath, err, stderr) - return nil, fmt.Errorf("Unable to run diff-index pipeline in temporary repo %s. Error: %w\nStderr: %s", - t.repo.FullName(), err, stderr) - } + }) + if err != nil && !git.IsErrCanceledOrKilled(err) { + log.Error("Unable to diff-index in temporary repo %s (%s). Error: %v\nStderr: %s", t.repo.FullName(), t.basePath, err, stderr) + return nil, fmt.Errorf("unable to run diff-index pipeline in temporary repo: %w", err) } diff.NumFiles, diff.TotalAddition, diff.TotalDeletion, err = git.GetDiffShortStat(t.ctx, t.basePath, git.TrustedCmdArgs{"--cached"}, "HEAD") diff --git a/services/repository/push.go b/services/repository/push.go index 8b81588c07..36c7927ab6 100644 --- a/services/repository/push.go +++ b/services/repository/push.go @@ -183,9 +183,7 @@ func pushUpdates(optsList []*repo_module.PushUpdateOptions) error { repo.IsEmpty = false if repo.DefaultBranch != setting.Repository.DefaultBranch { if err := gitrepo.SetDefaultBranch(ctx, repo, repo.DefaultBranch); err != nil { - if !git.IsErrUnsupportedVersion(err) { - return err - } + return err } } // Update the is empty and default_branch columns From 259811617ba15c77ddd89360178a59251d611af2 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 3 Nov 2024 05:04:53 +0800 Subject: [PATCH 005/156] Replace DateTime with proper functions (#32402) Follow #32383 This PR cleans up the "Deadline" usages in templates, make them call `ParseLegacy` first to get a `Time` struct then display by `DateUtils`. Now it should be pretty clear how "deadline string" works, it makes it possible to do further refactoring and correcting. --- modules/templates/helper.go | 4 +-- modules/templates/util_date.go | 26 ++++++++++++++ .../util_date_test.go} | 36 ++++++++++--------- modules/timeutil/datetime.go | 12 ++----- templates/repo/issue/milestone_issues.tmpl | 2 +- templates/repo/issue/milestones.tmpl | 2 +- .../repo/issue/view_content/comments.tmpl | 10 +++--- templates/user/dashboard/milestones.tmpl | 2 +- 8 files changed, 59 insertions(+), 35 deletions(-) rename modules/{timeutil/datetime_test.go => templates/util_date_test.go} (61%) diff --git a/modules/templates/helper.go b/modules/templates/helper.go index 5038e8a132..a5168541d8 100644 --- a/modules/templates/helper.go +++ b/modules/templates/helper.go @@ -54,7 +54,7 @@ func NewFuncMap() template.FuncMap { "StringUtils": NewStringUtils, "SliceUtils": NewSliceUtils, "JsonUtils": NewJsonUtils, - "DateUtils": NewDateUtils, // TODO: to be replaced by DateUtils + "DateUtils": NewDateUtils, // ----------------------------------------------------------------- // svg / avatar / icon / color @@ -71,7 +71,7 @@ func NewFuncMap() template.FuncMap { "CountFmt": base.FormatNumberSI, "TimeSince": timeutil.TimeSince, "TimeSinceUnix": timeutil.TimeSinceUnix, - "DateTime": timeutil.DateTime, + "DateTime": dateTimeLegacy, // for backward compatibility only, do not use it anymore "Sec2Time": util.SecToTime, "LoadTimes": func(startTime time.Time) string { return fmt.Sprint(time.Since(startTime).Nanoseconds()/1e6) + "ms" diff --git a/modules/templates/util_date.go b/modules/templates/util_date.go index ec48a7e4be..45dd8da02f 100644 --- a/modules/templates/util_date.go +++ b/modules/templates/util_date.go @@ -6,7 +6,9 @@ package templates import ( "context" "html/template" + "time" + "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" ) @@ -32,3 +34,27 @@ func (du *DateUtils) AbsoluteLong(time any) template.HTML { func (du *DateUtils) FullTime(time any) template.HTML { return timeutil.DateTime("full", time) } + +// ParseLegacy parses the datetime in legacy format, eg: "2016-01-02" in server's timezone. +// It shouldn't be used in new code. New code should use Time or TimeStamp as much as possible. +func (du *DateUtils) ParseLegacy(datetime string) time.Time { + return parseLegacy(datetime) +} + +func parseLegacy(datetime string) time.Time { + t, err := time.Parse(time.RFC3339, datetime) + if err != nil { + t, _ = time.ParseInLocation(time.DateOnly, datetime, setting.DefaultUILocation) + } + return t +} + +func dateTimeLegacy(format string, datetime any, _ ...string) template.HTML { + if !setting.IsProd || setting.IsInTesting { + panic("dateTimeLegacy is for backward compatibility only, do not use it in new code") + } + if s, ok := datetime.(string); ok { + datetime = parseLegacy(s) + } + return timeutil.DateTime(format, datetime) +} diff --git a/modules/timeutil/datetime_test.go b/modules/templates/util_date_test.go similarity index 61% rename from modules/timeutil/datetime_test.go rename to modules/templates/util_date_test.go index ac2ce35ba2..96c3776d39 100644 --- a/modules/timeutil/datetime_test.go +++ b/modules/templates/util_date_test.go @@ -1,7 +1,7 @@ // Copyright 2023 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT -package timeutil +package templates import ( "testing" @@ -9,6 +9,7 @@ import ( "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/test" + "code.gitea.io/gitea/modules/timeutil" "github.com/stretchr/testify/assert" ) @@ -16,32 +17,35 @@ import ( func TestDateTime(t *testing.T) { testTz, _ := time.LoadLocation("America/New_York") defer test.MockVariableValue(&setting.DefaultUILocation, testTz)() + defer test.MockVariableValue(&setting.IsInTesting, false)() + + du := NewDateUtils(nil) refTimeStr := "2018-01-01T00:00:00Z" refDateStr := "2018-01-01" refTime, _ := time.Parse(time.RFC3339, refTimeStr) - refTimeStamp := TimeStamp(refTime.Unix()) + refTimeStamp := timeutil.TimeStamp(refTime.Unix()) - assert.EqualValues(t, "-", DateTime("short", nil)) - assert.EqualValues(t, "-", DateTime("short", 0)) - assert.EqualValues(t, "-", DateTime("short", time.Time{})) - assert.EqualValues(t, "-", DateTime("short", TimeStamp(0))) + assert.EqualValues(t, "-", du.AbsoluteShort(nil)) + assert.EqualValues(t, "-", du.AbsoluteShort(0)) + assert.EqualValues(t, "-", du.AbsoluteShort(time.Time{})) + assert.EqualValues(t, "-", du.AbsoluteShort(timeutil.TimeStamp(0))) - actual := DateTime("short", "invalid") - assert.EqualValues(t, `invalid`, actual) + actual := dateTimeLegacy("short", "invalid") + assert.EqualValues(t, `-`, actual) - actual = DateTime("short", refTimeStr) - assert.EqualValues(t, `2018-01-01T00:00:00Z`, actual) - - actual = DateTime("short", refTime) + actual = dateTimeLegacy("short", refTimeStr) assert.EqualValues(t, `2018-01-01`, actual) - actual = DateTime("short", refDateStr) - assert.EqualValues(t, `2018-01-01`, actual) + actual = du.AbsoluteShort(refTime) + assert.EqualValues(t, `2018-01-01`, actual) - actual = DateTime("short", refTimeStamp) + actual = dateTimeLegacy("short", refDateStr) + assert.EqualValues(t, `2018-01-01`, actual) + + actual = du.AbsoluteShort(refTimeStamp) assert.EqualValues(t, `2017-12-31`, actual) - actual = DateTime("full", refTimeStamp) + actual = du.FullTime(refTimeStamp) assert.EqualValues(t, `2017-12-31 19:00:00 -05:00`, actual) } diff --git a/modules/timeutil/datetime.go b/modules/timeutil/datetime.go index c089173560..664e0320b0 100644 --- a/modules/timeutil/datetime.go +++ b/modules/timeutil/datetime.go @@ -12,9 +12,7 @@ import ( ) // DateTime renders an absolute time HTML element by datetime. -func DateTime(format string, datetime any, extraAttrs ...string) template.HTML { - // TODO: remove the extraAttrs argument, it's not used in any call to DateTime - +func DateTime(format string, datetime any) template.HTML { if p, ok := datetime.(*time.Time); ok { datetime = *p } @@ -34,9 +32,6 @@ func DateTime(format string, datetime any, extraAttrs ...string) template.HTML { switch v := datetime.(type) { case nil: return "-" - case string: - datetimeEscaped = html.EscapeString(v) - textEscaped = datetimeEscaped case time.Time: if v.IsZero() || v.Unix() == 0 { return "-" @@ -51,10 +46,7 @@ func DateTime(format string, datetime any, extraAttrs ...string) template.HTML { panic(fmt.Sprintf("Unsupported time type %T", datetime)) } - attrs := make([]string, 0, 10+len(extraAttrs)) - attrs = append(attrs, extraAttrs...) - attrs = append(attrs, `weekday=""`, `year="numeric"`) - + attrs := []string{`weekday=""`, `year="numeric"`} switch format { case "short", "long": // date only attrs = append(attrs, `month="`+format+`"`, `day="numeric"`) diff --git a/templates/repo/issue/milestone_issues.tmpl b/templates/repo/issue/milestone_issues.tmpl index 5bae6fc6d5..01bd944f46 100644 --- a/templates/repo/issue/milestone_issues.tmpl +++ b/templates/repo/issue/milestone_issues.tmpl @@ -38,7 +38,7 @@ {{if .Milestone.DeadlineString}} {{svg "octicon-calendar"}} - {{DateTime "short" .Milestone.DeadlineString}} + {{ctx.DateUtils.AbsoluteShort (.Milestone.DeadlineString|ctx.DateUtils.ParseLegacy)}} {{else}} {{svg "octicon-calendar"}} diff --git a/templates/repo/issue/milestones.tmpl b/templates/repo/issue/milestones.tmpl index bce7ad8717..8b84659d10 100644 --- a/templates/repo/issue/milestones.tmpl +++ b/templates/repo/issue/milestones.tmpl @@ -59,7 +59,7 @@ {{if .DeadlineString}} {{svg "octicon-calendar" 14}} - {{DateTime "short" .DeadlineString}} + {{ctx.DateUtils.AbsoluteShort (.DeadlineString|ctx.DateUtils.ParseLegacy)}} {{else}} {{svg "octicon-calendar" 14}} diff --git a/templates/repo/issue/view_content/comments.tmpl b/templates/repo/issue/view_content/comments.tmpl index 57abbeb8f7..9324959bed 100644 --- a/templates/repo/issue/view_content/comments.tmpl +++ b/templates/repo/issue/view_content/comments.tmpl @@ -296,7 +296,8 @@ {{template "shared/user/avatarlink" dict "user" .Poster}} {{template "shared/user/authorlink" .Poster}} - {{ctx.Locale.Tr "repo.issues.due_date_added" (DateTime "long" .Content) $createdStr}} + {{$dueDate := ctx.DateUtils.AbsoluteLong (.Content|ctx.DateUtils.ParseLegacy)}} + {{ctx.Locale.Tr "repo.issues.due_date_added" $dueDate $createdStr}}
{{else if eq .Type 17}} @@ -307,8 +308,8 @@ {{template "shared/user/authorlink" .Poster}} {{$parsedDeadline := StringUtils.Split .Content "|"}} {{if eq (len $parsedDeadline) 2}} - {{$from := DateTime "long" (index $parsedDeadline 1)}} - {{$to := DateTime "long" (index $parsedDeadline 0)}} + {{$to := ctx.DateUtils.AbsoluteLong ((index $parsedDeadline 0)|ctx.DateUtils.ParseLegacy)}} + {{$from := ctx.DateUtils.AbsoluteLong ((index $parsedDeadline 1)|ctx.DateUtils.ParseLegacy)}} {{ctx.Locale.Tr "repo.issues.due_date_modified" $to $from $createdStr}} {{end}} @@ -319,7 +320,8 @@ {{template "shared/user/avatarlink" dict "user" .Poster}} {{template "shared/user/authorlink" .Poster}} - {{ctx.Locale.Tr "repo.issues.due_date_remove" (DateTime "long" .Content) $createdStr}} + {{$dueDate := ctx.DateUtils.AbsoluteLong (.Content|ctx.DateUtils.ParseLegacy)}} + {{ctx.Locale.Tr "repo.issues.due_date_remove" $dueDate $createdStr}}
{{else if eq .Type 19}} diff --git a/templates/user/dashboard/milestones.tmpl b/templates/user/dashboard/milestones.tmpl index 71ff8dba3f..e01eca7c74 100644 --- a/templates/user/dashboard/milestones.tmpl +++ b/templates/user/dashboard/milestones.tmpl @@ -116,7 +116,7 @@ {{if .DeadlineString}} {{svg "octicon-calendar" 14}} - {{DateTime "short" .DeadlineString}} + {{ctx.DateUtils.AbsoluteShort (.DeadlineString|ctx.DateUtils.ParseLegacy)}} {{else}} {{svg "octicon-calendar" 14}} From f2a6df03d953d608a5cac19cb9fa2c6d62dbe0e3 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 3 Nov 2024 19:00:12 +0800 Subject: [PATCH 006/156] Refactor repo legacy (#32404) Only move code, no unnecessary logic change. (There are many problems in old code, but changing them is not in this PR's scope) Co-authored-by: Giteabot --- web_src/js/features/repo-common.ts | 11 + web_src/js/features/repo-issue-list.ts | 2 +- web_src/js/features/repo-issue-sidebar.ts | 274 +++++++++++++++++ web_src/js/features/repo-issue.ts | 32 +- web_src/js/features/repo-legacy.ts | 342 +--------------------- web_src/js/features/repo-milestone.ts | 11 + web_src/js/features/repo-new.ts | 14 + web_src/js/features/repo-settings.ts | 48 ++- web_src/js/index.ts | 11 +- 9 files changed, 383 insertions(+), 362 deletions(-) create mode 100644 web_src/js/features/repo-issue-sidebar.ts create mode 100644 web_src/js/features/repo-milestone.ts create mode 100644 web_src/js/features/repo-new.ts diff --git a/web_src/js/features/repo-common.ts b/web_src/js/features/repo-common.ts index de967ffba0..c7d84de9f0 100644 --- a/web_src/js/features/repo-common.ts +++ b/web_src/js/features/repo-common.ts @@ -90,3 +90,14 @@ export function initRepoCommonFilterSearchDropdown(selector) { message: {noResults: $dropdown[0].getAttribute('data-no-results')}, }); } + +export async function updateIssuesMeta(url, action, issue_ids, id) { + try { + const response = await POST(url, {data: new URLSearchParams({action, issue_ids, id})}); + if (!response.ok) { + throw new Error('Failed to update issues meta'); + } + } catch (error) { + console.error(error); + } +} diff --git a/web_src/js/features/repo-issue-list.ts b/web_src/js/features/repo-issue-list.ts index 134304617b..caf517c5e0 100644 --- a/web_src/js/features/repo-issue-list.ts +++ b/web_src/js/features/repo-issue-list.ts @@ -1,5 +1,5 @@ import $ from 'jquery'; -import {updateIssuesMeta} from './repo-issue.ts'; +import {updateIssuesMeta} from './repo-common.ts'; import {toggleElem, hideElem, isElemHidden} from '../utils/dom.ts'; import {htmlEscape} from 'escape-goat'; import {confirmModal} from './comp/ConfirmModal.ts'; diff --git a/web_src/js/features/repo-issue-sidebar.ts b/web_src/js/features/repo-issue-sidebar.ts new file mode 100644 index 0000000000..f33e192f29 --- /dev/null +++ b/web_src/js/features/repo-issue-sidebar.ts @@ -0,0 +1,274 @@ +import $ from 'jquery'; +import {POST} from '../modules/fetch.ts'; +import {updateIssuesMeta} from './repo-common.ts'; +import {svg} from '../svg.ts'; +import {htmlEscape} from 'escape-goat'; + +// if there are draft comments, confirm before reloading, to avoid losing comments +function reloadConfirmDraftComment() { + const commentTextareas = [ + document.querySelector('.edit-content-zone:not(.tw-hidden) textarea'), + document.querySelector('#comment-form textarea'), + ]; + for (const textarea of commentTextareas) { + // Most users won't feel too sad if they lose a comment with 10 chars, they can re-type these in seconds. + // But if they have typed more (like 50) chars and the comment is lost, they will be very unhappy. + if (textarea && textarea.value.trim().length > 10) { + textarea.parentElement.scrollIntoView(); + if (!window.confirm('Page will be reloaded, but there are draft comments. Continuing to reload will discard the comments. Continue?')) { + return; + } + break; + } + } + window.location.reload(); +} + +function initBranchSelector() { + const elSelectBranch = document.querySelector('.ui.dropdown.select-branch'); + if (!elSelectBranch) return; + + const urlUpdateIssueRef = elSelectBranch.getAttribute('data-url-update-issueref'); + const $selectBranch = $(elSelectBranch); + const $branchMenu = $selectBranch.find('.reference-list-menu'); + $branchMenu.find('.item:not(.no-select)').on('click', async function (e) { + e.preventDefault(); + const selectedValue = this.getAttribute('data-id'); // eg: "refs/heads/my-branch" + const selectedText = this.getAttribute('data-name'); // eg: "my-branch" + if (urlUpdateIssueRef) { + // for existing issue, send request to update issue ref, and reload page + try { + await POST(urlUpdateIssueRef, {data: new URLSearchParams({ref: selectedValue})}); + window.location.reload(); + } catch (error) { + console.error(error); + } + } else { + // for new issue, only update UI&form, do not send request/reload + const selectedHiddenSelector = this.getAttribute('data-id-selector'); + document.querySelector(selectedHiddenSelector).value = selectedValue; + elSelectBranch.querySelector('.text-branch-name').textContent = selectedText; + } + }); +} + +// List submits +function initListSubmits(selector, outerSelector) { + const $list = $(`.ui.${outerSelector}.list`); + const $noSelect = $list.find('.no-select'); + const $listMenu = $(`.${selector} .menu`); + let hasUpdateAction = $listMenu.data('action') === 'update'; + const items = {}; + + $(`.${selector}`).dropdown({ + 'action': 'nothing', // do not hide the menu if user presses Enter + fullTextSearch: 'exact', + async onHide() { + hasUpdateAction = $listMenu.data('action') === 'update'; // Update the var + if (hasUpdateAction) { + // TODO: Add batch functionality and make this 1 network request. + const itemEntries = Object.entries(items); + for (const [elementId, item] of itemEntries) { + await updateIssuesMeta( + item['update-url'], + item.action, + item['issue-id'], + elementId, + ); + } + if (itemEntries.length) { + reloadConfirmDraftComment(); + } + } + }, + }); + + $listMenu.find('.item:not(.no-select)').on('click', function (e) { + e.preventDefault(); + if (this.classList.contains('ban-change')) { + return false; + } + + hasUpdateAction = $listMenu.data('action') === 'update'; // Update the var + + const clickedItem = this; // eslint-disable-line unicorn/no-this-assignment + const scope = this.getAttribute('data-scope'); + + $(this).parent().find('.item').each(function () { + if (scope) { + // Enable only clicked item for scoped labels + if (this.getAttribute('data-scope') !== scope) { + return true; + } + if (this !== clickedItem && !this.classList.contains('checked')) { + return true; + } + } else if (this !== clickedItem) { + // Toggle for other labels + return true; + } + + if (this.classList.contains('checked')) { + $(this).removeClass('checked'); + $(this).find('.octicon-check').addClass('tw-invisible'); + if (hasUpdateAction) { + if (!($(this).data('id') in items)) { + items[$(this).data('id')] = { + 'update-url': $listMenu.data('update-url'), + action: 'detach', + 'issue-id': $listMenu.data('issue-id'), + }; + } else { + delete items[$(this).data('id')]; + } + } + } else { + $(this).addClass('checked'); + $(this).find('.octicon-check').removeClass('tw-invisible'); + if (hasUpdateAction) { + if (!($(this).data('id') in items)) { + items[$(this).data('id')] = { + 'update-url': $listMenu.data('update-url'), + action: 'attach', + 'issue-id': $listMenu.data('issue-id'), + }; + } else { + delete items[$(this).data('id')]; + } + } + } + }); + + // TODO: Which thing should be done for choosing review requests + // to make chosen items be shown on time here? + if (selector === 'select-reviewers-modify' || selector === 'select-assignees-modify') { + return false; + } + + const listIds = []; + $(this).parent().find('.item').each(function () { + if (this.classList.contains('checked')) { + listIds.push($(this).data('id')); + $($(this).data('id-selector')).removeClass('tw-hidden'); + } else { + $($(this).data('id-selector')).addClass('tw-hidden'); + } + }); + if (!listIds.length) { + $noSelect.removeClass('tw-hidden'); + } else { + $noSelect.addClass('tw-hidden'); + } + $($(this).parent().data('id')).val(listIds.join(',')); + return false; + }); + $listMenu.find('.no-select.item').on('click', function (e) { + e.preventDefault(); + if (hasUpdateAction) { + (async () => { + await updateIssuesMeta( + $listMenu.data('update-url'), + 'clear', + $listMenu.data('issue-id'), + '', + ); + reloadConfirmDraftComment(); + })(); + } + + $(this).parent().find('.item').each(function () { + $(this).removeClass('checked'); + $(this).find('.octicon-check').addClass('tw-invisible'); + }); + + if (selector === 'select-reviewers-modify' || selector === 'select-assignees-modify') { + return false; + } + + $list.find('.item').each(function () { + $(this).addClass('tw-hidden'); + }); + $noSelect.removeClass('tw-hidden'); + $($(this).parent().data('id')).val(''); + }); +} + +function selectItem(select_id, input_id) { + const $menu = $(`${select_id} .menu`); + const $list = $(`.ui${select_id}.list`); + const hasUpdateAction = $menu.data('action') === 'update'; + + $menu.find('.item:not(.no-select)').on('click', function () { + $(this).parent().find('.item').each(function () { + $(this).removeClass('selected active'); + }); + + $(this).addClass('selected active'); + if (hasUpdateAction) { + (async () => { + await updateIssuesMeta( + $menu.data('update-url'), + '', + $menu.data('issue-id'), + $(this).data('id'), + ); + reloadConfirmDraftComment(); + })(); + } + + let icon = ''; + if (input_id === '#milestone_id') { + icon = svg('octicon-milestone', 18, 'tw-mr-2'); + } else if (input_id === '#project_id') { + icon = svg('octicon-project', 18, 'tw-mr-2'); + } else if (input_id === '#assignee_id') { + icon = `avatar`; + } + + $list.find('.selected').html(` + + ${icon} + ${htmlEscape(this.textContent)} + + `); + + $(`.ui${select_id}.list .no-select`).addClass('tw-hidden'); + $(input_id).val($(this).data('id')); + }); + $menu.find('.no-select.item').on('click', function () { + $(this).parent().find('.item:not(.no-select)').each(function () { + $(this).removeClass('selected active'); + }); + + if (hasUpdateAction) { + (async () => { + await updateIssuesMeta( + $menu.data('update-url'), + '', + $menu.data('issue-id'), + $(this).data('id'), + ); + reloadConfirmDraftComment(); + })(); + } + + $list.find('.selected').html(''); + $list.find('.no-select').removeClass('tw-hidden'); + $(input_id).val(''); + }); +} + +export function initRepoIssueSidebar() { + initBranchSelector(); + + // Init labels and assignees + initListSubmits('select-label', 'labels'); + initListSubmits('select-assignees', 'assignees'); + initListSubmits('select-assignees-modify', 'assignees'); + initListSubmits('select-reviewers-modify', 'assignees'); + + // Milestone, Assignee, Project + selectItem('.select-project', '#project_id'); + selectItem('.select-milestone', '#milestone_id'); + selectItem('.select-assignee', '#assignee_id'); +} diff --git a/web_src/js/features/repo-issue.ts b/web_src/js/features/repo-issue.ts index 520f2081b0..a9a65cdc81 100644 --- a/web_src/js/features/repo-issue.ts +++ b/web_src/js/features/repo-issue.ts @@ -7,6 +7,8 @@ import {ComboMarkdownEditor, getComboMarkdownEditor, initComboMarkdownEditor} fr import {toAbsoluteUrl} from '../utils.ts'; import {GET, POST} from '../modules/fetch.ts'; import {showErrorToast} from '../modules/toast.ts'; +import {initRepoIssueSidebar} from './repo-issue-sidebar.ts'; +import {updateIssuesMeta} from './repo-common.ts'; const {appSubUrl} = window.config; @@ -369,17 +371,6 @@ export function initRepoIssueWipTitle() { }); } -export async function updateIssuesMeta(url, action, issue_ids, id) { - try { - const response = await POST(url, {data: new URLSearchParams({action, issue_ids, id})}); - if (!response.ok) { - throw new Error('Failed to update issues meta'); - } - } catch (error) { - console.error(error); - } -} - export function initRepoIssueComments() { if (!$('.repository.view.issue .timeline').length) return; @@ -665,7 +656,7 @@ export function initRepoIssueBranchSelect() { }); } -export async function initSingleCommentEditor($commentForm) { +async function initSingleCommentEditor($commentForm) { // pages: // * normal new issue/pr page: no status-button, no comment-button (there is only a normal submit button which can submit empty content) // * issue/pr view page: with comment form, has status-button and comment-button @@ -687,7 +678,7 @@ export async function initSingleCommentEditor($commentForm) { syncUiState(); } -export function initIssueTemplateCommentEditors($commentForm) { +function initIssueTemplateCommentEditors($commentForm) { // pages: // * new issue with issue template const $comboFields = $commentForm.find('.combo-editor-dropzone'); @@ -733,3 +724,18 @@ export function initArchivedLabelHandler() { toggleElem(label, label.classList.contains('checked')); } } + +export function initRepoCommentFormAndSidebar() { + const $commentForm = $('.comment.form'); + if (!$commentForm.length) return; + + if ($commentForm.find('.field.combo-editor-dropzone').length) { + // at the moment, if a form has multiple combo-markdown-editors, it must be an issue template form + initIssueTemplateCommentEditors($commentForm); + } else if ($commentForm.find('.combo-markdown-editor').length) { + // it's quite unclear about the "comment form" elements, sometimes it's for issue comment, sometimes it's for file editor/uploader message + initSingleCommentEditor($commentForm); + } + + initRepoIssueSidebar(); +} diff --git a/web_src/js/features/repo-legacy.ts b/web_src/js/features/repo-legacy.ts index 5844037770..6be3252a2b 100644 --- a/web_src/js/features/repo-legacy.ts +++ b/web_src/js/features/repo-legacy.ts @@ -1,13 +1,12 @@ import $ from 'jquery'; import { + initRepoCommentFormAndSidebar, initRepoIssueBranchSelect, initRepoIssueCodeCommentCancel, initRepoIssueCommentDelete, initRepoIssueComments, initRepoIssueDependencyDelete, initRepoIssueReferenceIssue, initRepoIssueTitleEdit, initRepoIssueWipToggle, - initRepoPullRequestUpdate, updateIssuesMeta, initIssueTemplateCommentEditors, initSingleCommentEditor, + initRepoPullRequestUpdate, } from './repo-issue.ts'; import {initUnicodeEscapeButton} from './repo-unicode-escape.ts'; -import {svg} from '../svg.ts'; -import {htmlEscape} from 'escape-goat'; import {initRepoBranchTagSelector} from '../components/RepoBranchTagSelector.vue'; import { initRepoCloneLink, initRepoCommonBranchOrTagDropdown, initRepoCommonFilterSearchDropdown, @@ -16,32 +15,13 @@ import {initCitationFileCopyContent} from './citation.ts'; import {initCompLabelEdit} from './comp/LabelEdit.ts'; import {initRepoDiffConversationNav} from './repo-diff.ts'; import {initCompReactionSelector} from './comp/ReactionSelector.ts'; -import {initRepoSettingBranches} from './repo-settings.ts'; +import {initRepoSettings} from './repo-settings.ts'; import {initRepoPullRequestMergeForm} from './repo-issue-pr-form.ts'; import {initRepoPullRequestCommitStatus} from './repo-issue-pr-status.ts'; import {hideElem, queryElemChildren, showElem} from '../utils/dom.ts'; -import {POST} from '../modules/fetch.ts'; import {initRepoIssueCommentEdit} from './repo-issue-edit.ts'; - -// if there are draft comments, confirm before reloading, to avoid losing comments -function reloadConfirmDraftComment() { - const commentTextareas = [ - document.querySelector('.edit-content-zone:not(.tw-hidden) textarea'), - document.querySelector('#comment-form textarea'), - ]; - for (const textarea of commentTextareas) { - // Most users won't feel too sad if they lose a comment with 10 chars, they can re-type these in seconds. - // But if they have typed more (like 50) chars and the comment is lost, they will be very unhappy. - if (textarea && textarea.value.trim().length > 10) { - textarea.parentElement.scrollIntoView(); - if (!window.confirm('Page will be reloaded, but there are draft comments. Continuing to reload will discard the comments. Continue?')) { - return; - } - break; - } - } - window.location.reload(); -} +import {initRepoMilestone} from './repo-milestone.ts'; +import {initRepoNew} from './repo-new.ts'; export function initBranchSelectorTabs() { const elSelectBranch = document.querySelector('.ui.dropdown.select-branch'); @@ -56,320 +36,16 @@ export function initBranchSelectorTabs() { }); } -export function initRepoCommentForm() { - const $commentForm = $('.comment.form'); - if (!$commentForm.length) return; - - if ($commentForm.find('.field.combo-editor-dropzone').length) { - // at the moment, if a form has multiple combo-markdown-editors, it must be an issue template form - initIssueTemplateCommentEditors($commentForm); - } else if ($commentForm.find('.combo-markdown-editor').length) { - // it's quite unclear about the "comment form" elements, sometimes it's for issue comment, sometimes it's for file editor/uploader message - initSingleCommentEditor($commentForm); - } - - function initBranchSelector() { - const elSelectBranch = document.querySelector('.ui.dropdown.select-branch'); - if (!elSelectBranch) return; - - const urlUpdateIssueRef = elSelectBranch.getAttribute('data-url-update-issueref'); - const $selectBranch = $(elSelectBranch); - const $branchMenu = $selectBranch.find('.reference-list-menu'); - $branchMenu.find('.item:not(.no-select)').on('click', async function (e) { - e.preventDefault(); - const selectedValue = this.getAttribute('data-id'); // eg: "refs/heads/my-branch" - const selectedText = this.getAttribute('data-name'); // eg: "my-branch" - if (urlUpdateIssueRef) { - // for existing issue, send request to update issue ref, and reload page - try { - await POST(urlUpdateIssueRef, {data: new URLSearchParams({ref: selectedValue})}); - window.location.reload(); - } catch (error) { - console.error(error); - } - } else { - // for new issue, only update UI&form, do not send request/reload - const selectedHiddenSelector = this.getAttribute('data-id-selector'); - document.querySelector(selectedHiddenSelector).value = selectedValue; - elSelectBranch.querySelector('.text-branch-name').textContent = selectedText; - } - }); - } - - initBranchSelector(); - - // List submits - function initListSubmits(selector, outerSelector) { - const $list = $(`.ui.${outerSelector}.list`); - const $noSelect = $list.find('.no-select'); - const $listMenu = $(`.${selector} .menu`); - let hasUpdateAction = $listMenu.data('action') === 'update'; - const items = {}; - - $(`.${selector}`).dropdown({ - 'action': 'nothing', // do not hide the menu if user presses Enter - fullTextSearch: 'exact', - async onHide() { - hasUpdateAction = $listMenu.data('action') === 'update'; // Update the var - if (hasUpdateAction) { - // TODO: Add batch functionality and make this 1 network request. - const itemEntries = Object.entries(items); - for (const [elementId, item] of itemEntries) { - await updateIssuesMeta( - item['update-url'], - item.action, - item['issue-id'], - elementId, - ); - } - if (itemEntries.length) { - reloadConfirmDraftComment(); - } - } - }, - }); - - $listMenu.find('.item:not(.no-select)').on('click', function (e) { - e.preventDefault(); - if (this.classList.contains('ban-change')) { - return false; - } - - hasUpdateAction = $listMenu.data('action') === 'update'; // Update the var - - const clickedItem = this; // eslint-disable-line unicorn/no-this-assignment - const scope = this.getAttribute('data-scope'); - - $(this).parent().find('.item').each(function () { - if (scope) { - // Enable only clicked item for scoped labels - if (this.getAttribute('data-scope') !== scope) { - return true; - } - if (this !== clickedItem && !this.classList.contains('checked')) { - return true; - } - } else if (this !== clickedItem) { - // Toggle for other labels - return true; - } - - if (this.classList.contains('checked')) { - $(this).removeClass('checked'); - $(this).find('.octicon-check').addClass('tw-invisible'); - if (hasUpdateAction) { - if (!($(this).data('id') in items)) { - items[$(this).data('id')] = { - 'update-url': $listMenu.data('update-url'), - action: 'detach', - 'issue-id': $listMenu.data('issue-id'), - }; - } else { - delete items[$(this).data('id')]; - } - } - } else { - $(this).addClass('checked'); - $(this).find('.octicon-check').removeClass('tw-invisible'); - if (hasUpdateAction) { - if (!($(this).data('id') in items)) { - items[$(this).data('id')] = { - 'update-url': $listMenu.data('update-url'), - action: 'attach', - 'issue-id': $listMenu.data('issue-id'), - }; - } else { - delete items[$(this).data('id')]; - } - } - } - }); - - // TODO: Which thing should be done for choosing review requests - // to make chosen items be shown on time here? - if (selector === 'select-reviewers-modify' || selector === 'select-assignees-modify') { - return false; - } - - const listIds = []; - $(this).parent().find('.item').each(function () { - if (this.classList.contains('checked')) { - listIds.push($(this).data('id')); - $($(this).data('id-selector')).removeClass('tw-hidden'); - } else { - $($(this).data('id-selector')).addClass('tw-hidden'); - } - }); - if (!listIds.length) { - $noSelect.removeClass('tw-hidden'); - } else { - $noSelect.addClass('tw-hidden'); - } - $($(this).parent().data('id')).val(listIds.join(',')); - return false; - }); - $listMenu.find('.no-select.item').on('click', function (e) { - e.preventDefault(); - if (hasUpdateAction) { - (async () => { - await updateIssuesMeta( - $listMenu.data('update-url'), - 'clear', - $listMenu.data('issue-id'), - '', - ); - reloadConfirmDraftComment(); - })(); - } - - $(this).parent().find('.item').each(function () { - $(this).removeClass('checked'); - $(this).find('.octicon-check').addClass('tw-invisible'); - }); - - if (selector === 'select-reviewers-modify' || selector === 'select-assignees-modify') { - return false; - } - - $list.find('.item').each(function () { - $(this).addClass('tw-hidden'); - }); - $noSelect.removeClass('tw-hidden'); - $($(this).parent().data('id')).val(''); - }); - } - - // Init labels and assignees - initListSubmits('select-label', 'labels'); - initListSubmits('select-assignees', 'assignees'); - initListSubmits('select-assignees-modify', 'assignees'); - initListSubmits('select-reviewers-modify', 'assignees'); - - function selectItem(select_id, input_id) { - const $menu = $(`${select_id} .menu`); - const $list = $(`.ui${select_id}.list`); - const hasUpdateAction = $menu.data('action') === 'update'; - - $menu.find('.item:not(.no-select)').on('click', function () { - $(this).parent().find('.item').each(function () { - $(this).removeClass('selected active'); - }); - - $(this).addClass('selected active'); - if (hasUpdateAction) { - (async () => { - await updateIssuesMeta( - $menu.data('update-url'), - '', - $menu.data('issue-id'), - $(this).data('id'), - ); - reloadConfirmDraftComment(); - })(); - } - - let icon = ''; - if (input_id === '#milestone_id') { - icon = svg('octicon-milestone', 18, 'tw-mr-2'); - } else if (input_id === '#project_id') { - icon = svg('octicon-project', 18, 'tw-mr-2'); - } else if (input_id === '#assignee_id') { - icon = `avatar`; - } - - $list.find('.selected').html(` - - ${icon} - ${htmlEscape(this.textContent)} - - `); - - $(`.ui${select_id}.list .no-select`).addClass('tw-hidden'); - $(input_id).val($(this).data('id')); - }); - $menu.find('.no-select.item').on('click', function () { - $(this).parent().find('.item:not(.no-select)').each(function () { - $(this).removeClass('selected active'); - }); - - if (hasUpdateAction) { - (async () => { - await updateIssuesMeta( - $menu.data('update-url'), - '', - $menu.data('issue-id'), - $(this).data('id'), - ); - reloadConfirmDraftComment(); - })(); - } - - $list.find('.selected').html(''); - $list.find('.no-select').removeClass('tw-hidden'); - $(input_id).val(''); - }); - } - - // Milestone, Assignee, Project - selectItem('.select-project', '#project_id'); - selectItem('.select-milestone', '#milestone_id'); - selectItem('.select-assignee', '#assignee_id'); -} - export function initRepository() { if (!$('.page-content.repository').length) return; initRepoBranchTagSelector('.js-branch-tag-selector'); - - // Options - if ($('.repository.settings.options').length > 0) { - // Enable or select internal/external wiki system and issue tracker. - $('.enable-system').on('change', function () { - if (this.checked) { - $($(this).data('target')).removeClass('disabled'); - if (!$(this).data('context')) $($(this).data('context')).addClass('disabled'); - } else { - $($(this).data('target')).addClass('disabled'); - if (!$(this).data('context')) $($(this).data('context')).removeClass('disabled'); - } - }); - $('.enable-system-radio').on('change', function () { - if (this.value === 'false') { - $($(this).data('target')).addClass('disabled'); - if ($(this).data('context') !== undefined) $($(this).data('context')).removeClass('disabled'); - } else if (this.value === 'true') { - $($(this).data('target')).removeClass('disabled'); - if ($(this).data('context') !== undefined) $($(this).data('context')).addClass('disabled'); - } - }); - const $trackerIssueStyleRadios = $('.js-tracker-issue-style'); - $trackerIssueStyleRadios.on('change input', () => { - const checkedVal = $trackerIssueStyleRadios.filter(':checked').val(); - $('#tracker-issue-style-regex-box').toggleClass('disabled', checkedVal !== 'regexp'); - }); - } + initRepoCommentFormAndSidebar(); // Labels initCompLabelEdit('.repository.labels'); - - // Milestones - if ($('.repository.new.milestone').length > 0) { - $('#clear-date').on('click', () => { - $('#deadline').val(''); - return false; - }); - } - - // Repo Creation - if ($('.repository.new.repo').length > 0) { - $('input[name="gitignores"], input[name="license"]').on('change', () => { - const gitignores = $('input[name="gitignores"]').val(); - const license = $('input[name="license"]').val(); - if (gitignores || license) { - document.querySelector('input[name="auto_init"]').checked = true; - } - }); - } + initRepoMilestone(); + initRepoNew(); // Compare or pull request const $repoDiff = $('.repository.diff'); @@ -380,7 +56,7 @@ export function initRepository() { initRepoCloneLink(); initCitationFileCopyContent(); - initRepoSettingBranches(); + initRepoSettings(); // Issues if ($('.repository.view.issue').length > 0) { diff --git a/web_src/js/features/repo-milestone.ts b/web_src/js/features/repo-milestone.ts new file mode 100644 index 0000000000..ddef723b48 --- /dev/null +++ b/web_src/js/features/repo-milestone.ts @@ -0,0 +1,11 @@ +import $ from 'jquery'; + +export function initRepoMilestone() { + // Milestones + if ($('.repository.new.milestone').length > 0) { + $('#clear-date').on('click', () => { + $('#deadline').val(''); + return false; + }); + } +} diff --git a/web_src/js/features/repo-new.ts b/web_src/js/features/repo-new.ts new file mode 100644 index 0000000000..22d8c8a47b --- /dev/null +++ b/web_src/js/features/repo-new.ts @@ -0,0 +1,14 @@ +import $ from 'jquery'; + +export function initRepoNew() { + // Repo Creation + if ($('.repository.new.repo').length > 0) { + $('input[name="gitignores"], input[name="license"]').on('change', () => { + const gitignores = $('input[name="gitignores"]').val(); + const license = $('input[name="license"]').val(); + if (gitignores || license) { + document.querySelector('input[name="auto_init"]').checked = true; + } + }); + } +} diff --git a/web_src/js/features/repo-settings.ts b/web_src/js/features/repo-settings.ts index 97211d035e..34a3b635b2 100644 --- a/web_src/js/features/repo-settings.ts +++ b/web_src/js/features/repo-settings.ts @@ -6,7 +6,7 @@ import {POST} from '../modules/fetch.ts'; const {appSubUrl, csrfToken} = window.config; -export function initRepoSettingsCollaboration() { +function initRepoSettingsCollaboration() { // Change collaborator access mode for (const dropdownEl of queryElems('.page-content.repository .ui.dropdown.access-mode')) { const textEl = dropdownEl.querySelector(':scope > .text'); @@ -43,7 +43,7 @@ export function initRepoSettingsCollaboration() { } } -export function initRepoSettingSearchTeamBox() { +function initRepoSettingsSearchTeamBox() { const searchTeamBox = document.querySelector('#search-team-box'); if (!searchTeamBox) return; @@ -69,13 +69,13 @@ export function initRepoSettingSearchTeamBox() { }); } -export function initRepoSettingGitHook() { +function initRepoSettingsGitHook() { if (!$('.edit.githook').length) return; const filename = document.querySelector('.hook-filename').textContent; - const _promise = createMonaco($('#content')[0], filename, {language: 'shell'}); + createMonaco($('#content')[0], filename, {language: 'shell'}); } -export function initRepoSettingBranches() { +function initRepoSettingsBranches() { if (!document.querySelector('.repository.settings.branches')) return; for (const el of document.querySelectorAll('.toggle-target-enabled')) { @@ -117,3 +117,41 @@ export function initRepoSettingBranches() { markMatchedStatusChecks(); document.querySelector('#status_check_contexts').addEventListener('input', onInputDebounce(markMatchedStatusChecks)); } + +function initRepoSettingsOptions() { + if ($('.repository.settings.options').length > 0) { + // Enable or select internal/external wiki system and issue tracker. + $('.enable-system').on('change', function () { + if (this.checked) { + $($(this).data('target')).removeClass('disabled'); + if (!$(this).data('context')) $($(this).data('context')).addClass('disabled'); + } else { + $($(this).data('target')).addClass('disabled'); + if (!$(this).data('context')) $($(this).data('context')).removeClass('disabled'); + } + }); + $('.enable-system-radio').on('change', function () { + if (this.value === 'false') { + $($(this).data('target')).addClass('disabled'); + if ($(this).data('context') !== undefined) $($(this).data('context')).removeClass('disabled'); + } else if (this.value === 'true') { + $($(this).data('target')).removeClass('disabled'); + if ($(this).data('context') !== undefined) $($(this).data('context')).addClass('disabled'); + } + }); + const $trackerIssueStyleRadios = $('.js-tracker-issue-style'); + $trackerIssueStyleRadios.on('change input', () => { + const checkedVal = $trackerIssueStyleRadios.filter(':checked').val(); + $('#tracker-issue-style-regex-box').toggleClass('disabled', checkedVal !== 'regexp'); + }); + } +} + +export function initRepoSettings() { + if (!document.querySelector('.page-content.repository.settings')) return; + initRepoSettingsOptions(); + initRepoSettingsBranches(); + initRepoSettingsCollaboration(); + initRepoSettingsSearchTeamBox(); + initRepoSettingsGitHook(); +} diff --git a/web_src/js/index.ts b/web_src/js/index.ts index f63d199488..08d8997fd1 100644 --- a/web_src/js/index.ts +++ b/web_src/js/index.ts @@ -43,11 +43,6 @@ import {initSshKeyFormParser} from './features/sshkey-helper.ts'; import {initUserSettings} from './features/user-settings.ts'; import {initRepoActivityTopAuthorsChart, initRepoArchiveLinks} from './features/repo-common.ts'; import {initRepoMigrationStatusChecker} from './features/repo-migrate.ts'; -import { - initRepoSettingGitHook, - initRepoSettingsCollaboration, - initRepoSettingSearchTeamBox, -} from './features/repo-settings.ts'; import {initRepoDiffView} from './features/repo-diff.ts'; import {initOrgTeamSearchRepoBox, initOrgTeamSettings} from './features/org-team.ts'; import {initUserAuthWebAuthn, initUserAuthWebAuthnRegister} from './features/user-auth-webauthn.ts'; @@ -59,7 +54,7 @@ import {initCompWebHookEditor} from './features/comp/WebHookEditor.ts'; import {initRepoBranchButton} from './features/repo-branch.ts'; import {initCommonOrganization} from './features/common-organization.ts'; import {initRepoWikiForm} from './features/repo-wiki.ts'; -import {initRepoCommentForm, initRepository, initBranchSelectorTabs} from './features/repo-legacy.ts'; +import {initRepository, initBranchSelectorTabs} from './features/repo-legacy.ts'; import {initCopyContent} from './features/copycontent.ts'; import {initCaptcha} from './features/captcha.ts'; import {initRepositoryActionView} from './components/RepoActionView.vue'; @@ -180,7 +175,6 @@ onDomReady(() => { initRepoArchiveLinks, initRepoBranchButton, initRepoCodeView, - initRepoCommentForm, initBranchSelectorTabs, initRepoEllipsisButton, initRepoDiffCommitBranchesAndTags, @@ -202,9 +196,6 @@ onDomReady(() => { initRepoPullRequestReview, initRepoRelease, initRepoReleaseNew, - initRepoSettingGitHook, - initRepoSettingSearchTeamBox, - initRepoSettingsCollaboration, initRepoTemplateSearch, initRepoTopicBar, initRepoWikiForm, From 54146e62c0b65a941017983f88f7715e6f35c7b1 Mon Sep 17 00:00:00 2001 From: Royce Remer Date: Sun, 3 Nov 2024 20:49:08 -0800 Subject: [PATCH 007/156] Make LFS http_client parallel within a batch. (#32369) Signed-off-by: Royce Remer Co-authored-by: wxiaoguang --- custom/conf/app.example.ini | 8 +- go.mod | 2 +- modules/lfs/http_client.go | 143 ++++++++++++++++------------- modules/lfs/http_client_test.go | 153 ++++++++++++++------------------ modules/repository/repo.go | 9 +- modules/setting/lfs.go | 8 +- modules/setting/lfs_test.go | 13 +++ 7 files changed, 183 insertions(+), 153 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 69b57a8c01..e080b0be72 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -2642,9 +2642,15 @@ LEVEL = Info ;; override the azure blob base path if storage type is azureblob ;AZURE_BLOB_BASE_PATH = lfs/ +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; +;; settings for Gitea's LFS client (eg: mirroring an upstream lfs endpoint) +;; ;[lfs_client] -;; When mirroring an upstream lfs endpoint, limit the number of pointers in each batch request to this number +;; Limit the number of pointers in each batch request to this number ;BATCH_SIZE = 20 +;; Limit the number of concurrent upload/download operations within a batch +;BATCH_OPERATION_CONCURRENCY = 3 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/go.mod b/go.mod index c98ef9a61b..ff0d612133 100644 --- a/go.mod +++ b/go.mod @@ -124,6 +124,7 @@ require ( golang.org/x/image v0.21.0 golang.org/x/net v0.30.0 golang.org/x/oauth2 v0.23.0 + golang.org/x/sync v0.8.0 golang.org/x/sys v0.26.0 golang.org/x/text v0.19.0 golang.org/x/tools v0.26.0 @@ -316,7 +317,6 @@ require ( go.uber.org/zap v1.27.0 // indirect golang.org/x/exp v0.0.0-20241009180824-f66d83c29e7c // indirect golang.org/x/mod v0.21.0 // indirect - golang.org/x/sync v0.8.0 // indirect golang.org/x/time v0.7.0 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20241021214115-324edc3d5d38 // indirect gopkg.in/alexcesaro/quotedprintable.v3 v3.0.0-20150716171945-2caba252f4dc // indirect diff --git a/modules/lfs/http_client.go b/modules/lfs/http_client.go index aa9e744d72..411c4248c4 100644 --- a/modules/lfs/http_client.go +++ b/modules/lfs/http_client.go @@ -17,6 +17,8 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/proxy" "code.gitea.io/gitea/modules/setting" + + "golang.org/x/sync/errgroup" ) // HTTPClient is used to communicate with the LFS server @@ -113,6 +115,7 @@ func (c *HTTPClient) Upload(ctx context.Context, objects []Pointer, callback Upl return c.performOperation(ctx, objects, nil, callback) } +// performOperation takes a slice of LFS object pointers, batches them, and performs the upload/download operations concurrently in each batch func (c *HTTPClient) performOperation(ctx context.Context, objects []Pointer, dc DownloadCallback, uc UploadCallback) error { if len(objects) == 0 { return nil @@ -133,71 +136,87 @@ func (c *HTTPClient) performOperation(ctx context.Context, objects []Pointer, dc return fmt.Errorf("TransferAdapter not found: %s", result.Transfer) } + errGroup, groupCtx := errgroup.WithContext(ctx) + errGroup.SetLimit(setting.LFSClient.BatchOperationConcurrency) for _, object := range result.Objects { - if object.Error != nil { - log.Trace("Error on object %v: %v", object.Pointer, object.Error) - if uc != nil { - if _, err := uc(object.Pointer, object.Error); err != nil { - return err - } - } else { - if err := dc(object.Pointer, nil, object.Error); err != nil { - return err - } - } - continue - } - - if uc != nil { - if len(object.Actions) == 0 { - log.Trace("%v already present on server", object.Pointer) - continue - } - - link, ok := object.Actions["upload"] - if !ok { - log.Debug("%+v", object) - return errors.New("missing action 'upload'") - } - - content, err := uc(object.Pointer, nil) - if err != nil { - return err - } - - err = transferAdapter.Upload(ctx, link, object.Pointer, content) - if err != nil { - return err - } - - link, ok = object.Actions["verify"] - if ok { - if err := transferAdapter.Verify(ctx, link, object.Pointer); err != nil { - return err - } - } - } else { - link, ok := object.Actions["download"] - if !ok { - // no actions block in response, try legacy response schema - link, ok = object.Links["download"] - } - if !ok { - log.Debug("%+v", object) - return errors.New("missing action 'download'") - } - - content, err := transferAdapter.Download(ctx, link) - if err != nil { - return err - } - - if err := dc(object.Pointer, content, nil); err != nil { - return err - } - } + errGroup.Go(func() error { + return performSingleOperation(groupCtx, object, dc, uc, transferAdapter) + }) } + // only the first error is returned, preserving legacy behavior before concurrency + return errGroup.Wait() +} + +// performSingleOperation performs an LFS upload or download operation on a single object +func performSingleOperation(ctx context.Context, object *ObjectResponse, dc DownloadCallback, uc UploadCallback, transferAdapter TransferAdapter) error { + // the response from a lfs batch api request for this specific object id contained an error + if object.Error != nil { + log.Trace("Error on object %v: %v", object.Pointer, object.Error) + + // this was an 'upload' request inside the batch request + if uc != nil { + if _, err := uc(object.Pointer, object.Error); err != nil { + return err + } + } else { + // this was NOT an 'upload' request inside the batch request, meaning it must be a 'download' request + if err := dc(object.Pointer, nil, object.Error); err != nil { + return err + } + } + // if the callback returns no err, then the error could be ignored, and the operations should continue + return nil + } + + // the response from an lfs batch api request contained necessary upload/download fields to act upon + if uc != nil { + if len(object.Actions) == 0 { + log.Trace("%v already present on server", object.Pointer) + return nil + } + + link, ok := object.Actions["upload"] + if !ok { + return errors.New("missing action 'upload'") + } + + content, err := uc(object.Pointer, nil) + if err != nil { + return err + } + + err = transferAdapter.Upload(ctx, link, object.Pointer, content) + if err != nil { + return err + } + + link, ok = object.Actions["verify"] + if ok { + if err := transferAdapter.Verify(ctx, link, object.Pointer); err != nil { + return err + } + } + } else { + link, ok := object.Actions["download"] + if !ok { + // no actions block in response, try legacy response schema + link, ok = object.Links["download"] + } + if !ok { + log.Debug("%+v", object) + return errors.New("missing action 'download'") + } + + content, err := transferAdapter.Download(ctx, link) + if err != nil { + return err + } + + if err := dc(object.Pointer, content, nil); err != nil { + return err + } + } return nil } diff --git a/modules/lfs/http_client_test.go b/modules/lfs/http_client_test.go index ec90f5375d..d22735147a 100644 --- a/modules/lfs/http_client_test.go +++ b/modules/lfs/http_client_test.go @@ -12,6 +12,8 @@ import ( "testing" "code.gitea.io/gitea/modules/json" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/test" "github.com/stretchr/testify/assert" ) @@ -183,93 +185,84 @@ func TestHTTPClientDownload(t *testing.T) { cases := []struct { endpoint string - expectederror string + expectedError string }{ - // case 0 { endpoint: "https://status-not-ok.io", - expectederror: io.ErrUnexpectedEOF.Error(), + expectedError: io.ErrUnexpectedEOF.Error(), }, - // case 1 { endpoint: "https://invalid-json-response.io", - expectederror: "invalid json", + expectedError: "invalid json", }, - // case 2 { endpoint: "https://valid-batch-request-download.io", - expectederror: "", + expectedError: "", }, - // case 3 { endpoint: "https://response-no-objects.io", - expectederror: "", + expectedError: "", }, - // case 4 { endpoint: "https://unknown-transfer-adapter.io", - expectederror: "TransferAdapter not found: ", + expectedError: "TransferAdapter not found: ", }, - // case 5 { endpoint: "https://error-in-response-objects.io", - expectederror: "Object not found", + expectedError: "Object not found", }, - // case 6 { endpoint: "https://empty-actions-map.io", - expectederror: "missing action 'download'", + expectedError: "missing action 'download'", }, - // case 7 { endpoint: "https://download-actions-map.io", - expectederror: "", + expectedError: "", }, - // case 8 { endpoint: "https://upload-actions-map.io", - expectederror: "missing action 'download'", + expectedError: "missing action 'download'", }, - // case 9 { endpoint: "https://verify-actions-map.io", - expectederror: "missing action 'download'", + expectedError: "missing action 'download'", }, - // case 10 { endpoint: "https://unknown-actions-map.io", - expectederror: "missing action 'download'", + expectedError: "missing action 'download'", }, - // case 11 { endpoint: "https://legacy-batch-request-download.io", - expectederror: "", + expectedError: "", }, } - for n, c := range cases { - client := &HTTPClient{ - client: hc, - endpoint: c.endpoint, - transfers: map[string]TransferAdapter{ - "dummy": dummy, - }, - } - - err := client.Download(context.Background(), []Pointer{p}, func(p Pointer, content io.ReadCloser, objectError error) error { - if objectError != nil { - return objectError + defer test.MockVariableValue(&setting.LFSClient.BatchOperationConcurrency, 3)() + for _, c := range cases { + t.Run(c.endpoint, func(t *testing.T) { + client := &HTTPClient{ + client: hc, + endpoint: c.endpoint, + transfers: map[string]TransferAdapter{ + "dummy": dummy, + }, + } + + err := client.Download(context.Background(), []Pointer{p}, func(p Pointer, content io.ReadCloser, objectError error) error { + if objectError != nil { + return objectError + } + b, err := io.ReadAll(content) + assert.NoError(t, err) + assert.Equal(t, []byte("dummy"), b) + return nil + }) + if c.expectedError != "" { + assert.ErrorContains(t, err, c.expectedError) + } else { + assert.NoError(t, err) } - b, err := io.ReadAll(content) - assert.NoError(t, err) - assert.Equal(t, []byte("dummy"), b) - return nil }) - if len(c.expectederror) > 0 { - assert.True(t, strings.Contains(err.Error(), c.expectederror), "case %d: '%s' should contain '%s'", n, err.Error(), c.expectederror) - } else { - assert.NoError(t, err, "case %d", n) - } } } @@ -296,81 +289,73 @@ func TestHTTPClientUpload(t *testing.T) { cases := []struct { endpoint string - expectederror string + expectedError string }{ - // case 0 { endpoint: "https://status-not-ok.io", - expectederror: io.ErrUnexpectedEOF.Error(), + expectedError: io.ErrUnexpectedEOF.Error(), }, - // case 1 { endpoint: "https://invalid-json-response.io", - expectederror: "invalid json", + expectedError: "invalid json", }, - // case 2 { endpoint: "https://valid-batch-request-upload.io", - expectederror: "", + expectedError: "", }, - // case 3 { endpoint: "https://response-no-objects.io", - expectederror: "", + expectedError: "", }, - // case 4 { endpoint: "https://unknown-transfer-adapter.io", - expectederror: "TransferAdapter not found: ", + expectedError: "TransferAdapter not found: ", }, - // case 5 { endpoint: "https://error-in-response-objects.io", - expectederror: "Object not found", + expectedError: "Object not found", }, - // case 6 { endpoint: "https://empty-actions-map.io", - expectederror: "", + expectedError: "", }, - // case 7 { endpoint: "https://download-actions-map.io", - expectederror: "missing action 'upload'", + expectedError: "missing action 'upload'", }, - // case 8 { endpoint: "https://upload-actions-map.io", - expectederror: "", + expectedError: "", }, - // case 9 { endpoint: "https://verify-actions-map.io", - expectederror: "missing action 'upload'", + expectedError: "missing action 'upload'", }, - // case 10 { endpoint: "https://unknown-actions-map.io", - expectederror: "missing action 'upload'", + expectedError: "missing action 'upload'", }, } - for n, c := range cases { - client := &HTTPClient{ - client: hc, - endpoint: c.endpoint, - transfers: map[string]TransferAdapter{ - "dummy": dummy, - }, - } + defer test.MockVariableValue(&setting.LFSClient.BatchOperationConcurrency, 3)() + for _, c := range cases { + t.Run(c.endpoint, func(t *testing.T) { + client := &HTTPClient{ + client: hc, + endpoint: c.endpoint, + transfers: map[string]TransferAdapter{ + "dummy": dummy, + }, + } - err := client.Upload(context.Background(), []Pointer{p}, func(p Pointer, objectError error) (io.ReadCloser, error) { - return io.NopCloser(new(bytes.Buffer)), objectError + err := client.Upload(context.Background(), []Pointer{p}, func(p Pointer, objectError error) (io.ReadCloser, error) { + return io.NopCloser(new(bytes.Buffer)), objectError + }) + if c.expectedError != "" { + assert.ErrorContains(t, err, c.expectedError) + } else { + assert.NoError(t, err) + } }) - if len(c.expectederror) > 0 { - assert.True(t, strings.Contains(err.Error(), c.expectederror), "case %d: '%s' should contain '%s'", n, err.Error(), c.expectederror) - } else { - assert.NoError(t, err, "case %d", n) - } } } diff --git a/modules/repository/repo.go b/modules/repository/repo.go index def2220b17..97b0343381 100644 --- a/modules/repository/repo.go +++ b/modules/repository/repo.go @@ -181,11 +181,12 @@ func StoreMissingLfsObjectsInRepository(ctx context.Context, repo *repo_model.Re downloadObjects := func(pointers []lfs.Pointer) error { err := lfsClient.Download(ctx, pointers, func(p lfs.Pointer, content io.ReadCloser, objectError error) error { + if errors.Is(objectError, lfs.ErrObjectNotExist) { + log.Warn("Ignoring missing upstream LFS object %-v: %v", p, objectError) + return nil + } + if objectError != nil { - if errors.Is(objectError, lfs.ErrObjectNotExist) { - log.Warn("Repo[%-v]: Ignore missing LFS object %-v: %v", repo, p, objectError) - return nil - } return objectError } diff --git a/modules/setting/lfs.go b/modules/setting/lfs.go index 24c49cabee..6b54ac0a60 100644 --- a/modules/setting/lfs.go +++ b/modules/setting/lfs.go @@ -28,7 +28,8 @@ var LFS = struct { // LFSClient represents configuration for Gitea's LFS clients, for example: mirroring upstream Git LFS var LFSClient = struct { - BatchSize int `ini:"BATCH_SIZE"` + BatchSize int `ini:"BATCH_SIZE"` + BatchOperationConcurrency int `ini:"BATCH_OPERATION_CONCURRENCY"` }{} func loadLFSFrom(rootCfg ConfigProvider) error { @@ -66,6 +67,11 @@ func loadLFSFrom(rootCfg ConfigProvider) error { LFSClient.BatchSize = 20 } + if LFSClient.BatchOperationConcurrency < 1 { + // match the default git-lfs's `lfs.concurrenttransfers` + LFSClient.BatchOperationConcurrency = 3 + } + LFS.HTTPAuthExpiry = sec.Key("LFS_HTTP_AUTH_EXPIRY").MustDuration(24 * time.Hour) if !LFS.StartServer || !InstallLock { diff --git a/modules/setting/lfs_test.go b/modules/setting/lfs_test.go index f7beaaa9c7..471fa8bff3 100644 --- a/modules/setting/lfs_test.go +++ b/modules/setting/lfs_test.go @@ -114,4 +114,17 @@ BATCH_SIZE = 0 assert.NoError(t, loadLFSFrom(cfg)) assert.EqualValues(t, 100, LFS.MaxBatchSize) assert.EqualValues(t, 20, LFSClient.BatchSize) + assert.EqualValues(t, 3, LFSClient.BatchOperationConcurrency) + + iniStr = ` +[lfs_client] +BATCH_SIZE = 50 +BATCH_OPERATION_CONCURRENCY = 10 +` + cfg, err = NewConfigProviderFromData(iniStr) + assert.NoError(t, err) + + assert.NoError(t, loadLFSFrom(cfg)) + assert.EqualValues(t, 50, LFSClient.BatchSize) + assert.EqualValues(t, 10, LFSClient.BatchOperationConcurrency) } From af28ce59b8695a8412632c50cf96fdd420215719 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 4 Nov 2024 18:14:36 +0800 Subject: [PATCH 008/156] Add some handy markdown editor features (#32400) There were some missing features from EasyMDE: 1. H1 - H3 style 2. Auto add task list 3. Insert a table And added some tests --- options/locale/locale_en-US.ini | 4 ++ templates/shared/combomarkdowneditor.tmpl | 15 +++++- web_src/css/editor/combomarkdowneditor.css | 28 +++++++++- web_src/css/modules/comment.css | 1 - .../js/features/comp/ComboMarkdownEditor.ts | 51 +++++++++++++++++-- .../js/features/comp/EditorMarkdown.test.ts | 27 ++++++++++ web_src/js/features/comp/EditorMarkdown.ts | 21 ++++++-- web_src/js/features/comp/EditorUpload.ts | 11 +--- web_src/js/modules/tippy.ts | 2 +- 9 files changed, 138 insertions(+), 22 deletions(-) create mode 100644 web_src/js/features/comp/EditorMarkdown.test.ts diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 06bf57fc62..679e64b424 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -209,6 +209,10 @@ buttons.link.tooltip = Add a link buttons.list.unordered.tooltip = Add a bullet list buttons.list.ordered.tooltip = Add a numbered list buttons.list.task.tooltip = Add a list of tasks +buttons.table.add.tooltip = Add a table +buttons.table.add.insert = Add +buttons.table.rows = Rows +buttons.table.cols = Columns buttons.mention.tooltip = Mention a user or team buttons.ref.tooltip = Reference an issue or pull request buttons.switch_to_legacy.tooltip = Use the legacy editor instead diff --git a/templates/shared/combomarkdowneditor.tmpl b/templates/shared/combomarkdowneditor.tmpl index 0a01dd9b1d..6ee989d1d6 100644 --- a/templates/shared/combomarkdowneditor.tmpl +++ b/templates/shared/combomarkdowneditor.tmpl @@ -21,7 +21,11 @@ Template Attributes:
- {{svg "octicon-heading"}} + {{svg "octicon-heading"}} + {{svg "octicon-heading"}} + {{svg "octicon-heading"}} +
+
{{svg "octicon-bold"}} {{svg "octicon-italic"}}
@@ -34,6 +38,7 @@ Template Attributes: {{svg "octicon-list-unordered"}} {{svg "octicon-list-ordered"}} {{svg "octicon-tasklist"}} +
{{svg "octicon-mention"}} @@ -56,4 +61,12 @@ Template Attributes:
{{ctx.Locale.Tr "loading"}}
+
+
+ + x + + +
+
diff --git a/web_src/css/editor/combomarkdowneditor.css b/web_src/css/editor/combomarkdowneditor.css index 8a2f4ea416..97a8b70227 100644 --- a/web_src/css/editor/combomarkdowneditor.css +++ b/web_src/css/editor/combomarkdowneditor.css @@ -7,17 +7,25 @@ display: flex; align-items: center; padding-bottom: 10px; - gap: .5rem; flex-wrap: wrap; } .combo-markdown-editor .markdown-toolbar-group { display: flex; + border-left: 1px solid var(--color-secondary); + padding: 0 0.5em; } +.combo-markdown-editor .markdown-toolbar-group:first-child { + border-left: 0; + padding-left: 0; +} .combo-markdown-editor .markdown-toolbar-group:last-child { flex: 1; justify-content: flex-end; + border-right: none; + border-left: 0; + padding-right: 0; } .combo-markdown-editor .markdown-toolbar-button { @@ -33,6 +41,24 @@ color: var(--color-primary); } +.combo-markdown-editor md-header { + position: relative; +} +.combo-markdown-editor md-header::after { + font-size: 10px; + position: absolute; + top: 7px; +} +.combo-markdown-editor md-header[level="1"]::after { + content: "1"; +} +.combo-markdown-editor md-header[level="2"]::after { + content: "2"; +} +.combo-markdown-editor md-header[level="3"]::after { + content: "3"; +} + .ui.form .combo-markdown-editor textarea.markdown-text-editor, .combo-markdown-editor textarea.markdown-text-editor { display: block; diff --git a/web_src/css/modules/comment.css b/web_src/css/modules/comment.css index cda16fdddc..68306686ef 100644 --- a/web_src/css/modules/comment.css +++ b/web_src/css/modules/comment.css @@ -21,7 +21,6 @@ padding: 0.5em 0 0; border: none; border-top: none; - line-height: 1.2; } .edit-content-zone .comment { diff --git a/web_src/js/features/comp/ComboMarkdownEditor.ts b/web_src/js/features/comp/ComboMarkdownEditor.ts index d0e122c54a..576c1bccd6 100644 --- a/web_src/js/features/comp/ComboMarkdownEditor.ts +++ b/web_src/js/features/comp/ComboMarkdownEditor.ts @@ -15,8 +15,14 @@ import {easyMDEToolbarActions} from './EasyMDEToolbarActions.ts'; import {initTextExpander} from './TextExpander.ts'; import {showErrorToast} from '../../modules/toast.ts'; import {POST} from '../../modules/fetch.ts'; -import {EventEditorContentChanged, initTextareaMarkdown, triggerEditorContentChanged} from './EditorMarkdown.ts'; +import { + EventEditorContentChanged, + initTextareaMarkdown, + textareaInsertText, + triggerEditorContentChanged, +} from './EditorMarkdown.ts'; import {DropzoneCustomEventReloadFiles, initDropzone} from '../dropzone.ts'; +import {createTippy} from '../../modules/tippy.ts'; let elementIdCounter = 0; @@ -122,8 +128,7 @@ export class ComboMarkdownEditor { const monospaceText = monospaceButton.getAttribute(monospaceEnabled ? 'data-disable-text' : 'data-enable-text'); monospaceButton.setAttribute('data-tooltip-content', monospaceText); monospaceButton.setAttribute('aria-checked', String(monospaceEnabled)); - - monospaceButton?.addEventListener('click', (e) => { + monospaceButton.addEventListener('click', (e) => { e.preventDefault(); const enabled = localStorage?.getItem('markdown-editor-monospace') !== 'true'; localStorage.setItem('markdown-editor-monospace', String(enabled)); @@ -134,12 +139,14 @@ export class ComboMarkdownEditor { }); const easymdeButton = this.container.querySelector('.markdown-switch-easymde'); - easymdeButton?.addEventListener('click', async (e) => { + easymdeButton.addEventListener('click', async (e) => { e.preventDefault(); this.userPreferredEditor = 'easymde'; await this.switchToEasyMDE(); }); + this.initMarkdownButtonTableAdd(); + initTextareaMarkdown(this.textarea); initTextareaEvents(this.textarea, this.dropzone); } @@ -219,6 +226,42 @@ export class ComboMarkdownEditor { }); } + generateMarkdownTable(rows: number, cols: number): string { + const tableLines = []; + tableLines.push( + `| ${'Header '.repeat(cols).trim().split(' ').join(' | ')} |`, + `| ${'--- '.repeat(cols).trim().split(' ').join(' | ')} |`, + ); + for (let i = 0; i < rows; i++) { + tableLines.push(`| ${'Cell '.repeat(cols).trim().split(' ').join(' | ')} |`); + } + return tableLines.join('\n'); + } + + initMarkdownButtonTableAdd() { + const addTableButton = this.container.querySelector('.markdown-button-table-add'); + const addTablePanel = this.container.querySelector('.markdown-add-table-panel'); + // here the tippy can't attach to the button because the button already owns a tippy for tooltip + const addTablePanelTippy = createTippy(addTablePanel, { + content: addTablePanel, + trigger: 'manual', + placement: 'bottom', + hideOnClick: true, + interactive: true, + getReferenceClientRect: () => addTableButton.getBoundingClientRect(), + }); + addTableButton.addEventListener('click', () => addTablePanelTippy.show()); + + addTablePanel.querySelector('.ui.button.primary').addEventListener('click', () => { + let rows = parseInt(addTablePanel.querySelector('[name=rows]').value); + let cols = parseInt(addTablePanel.querySelector('[name=cols]').value); + rows = Math.max(1, Math.min(100, rows)); + cols = Math.max(1, Math.min(100, cols)); + textareaInsertText(this.textarea, `\n${this.generateMarkdownTable(rows, cols)}\n\n`); + addTablePanelTippy.hide(); + }); + } + switchTabToEditor() { this.tabEditor.click(); } diff --git a/web_src/js/features/comp/EditorMarkdown.test.ts b/web_src/js/features/comp/EditorMarkdown.test.ts new file mode 100644 index 0000000000..acd496bed6 --- /dev/null +++ b/web_src/js/features/comp/EditorMarkdown.test.ts @@ -0,0 +1,27 @@ +import {initTextareaMarkdown} from './EditorMarkdown.ts'; + +test('EditorMarkdown', () => { + const textarea = document.createElement('textarea'); + initTextareaMarkdown(textarea); + + const testInput = (value, expected) => { + textarea.value = value; + textarea.setSelectionRange(value.length, value.length); + const e = new KeyboardEvent('keydown', {key: 'Enter', cancelable: true}); + textarea.dispatchEvent(e); + if (!e.defaultPrevented) textarea.value += '\n'; + expect(textarea.value).toEqual(expected); + }; + + testInput('-', '-\n'); + testInput('1.', '1.\n'); + + testInput('- ', ''); + testInput('1. ', ''); + + testInput('- x', '- x\n- '); + testInput('- [ ]', '- [ ]\n- '); + testInput('- [ ] foo', '- [ ] foo\n- [ ] '); + testInput('* [x] foo', '* [x] foo\n* [ ] '); + testInput('1. [x] foo', '1. [x] foo\n1. [ ] '); +}); diff --git a/web_src/js/features/comp/EditorMarkdown.ts b/web_src/js/features/comp/EditorMarkdown.ts index deee561dab..2af003ccb0 100644 --- a/web_src/js/features/comp/EditorMarkdown.ts +++ b/web_src/js/features/comp/EditorMarkdown.ts @@ -4,6 +4,16 @@ export function triggerEditorContentChanged(target) { target.dispatchEvent(new CustomEvent(EventEditorContentChanged, {bubbles: true})); } +export function textareaInsertText(textarea, value) { + const startPos = textarea.selectionStart; + const endPos = textarea.selectionEnd; + textarea.value = textarea.value.substring(0, startPos) + value + textarea.value.substring(endPos); + textarea.selectionStart = startPos; + textarea.selectionEnd = startPos + value.length; + textarea.focus(); + triggerEditorContentChanged(textarea); +} + function handleIndentSelection(textarea, e) { const selStart = textarea.selectionStart; const selEnd = textarea.selectionEnd; @@ -46,7 +56,7 @@ function handleIndentSelection(textarea, e) { triggerEditorContentChanged(textarea); } -function handleNewline(textarea, e) { +function handleNewline(textarea: HTMLTextAreaElement, e: Event) { const selStart = textarea.selectionStart; const selEnd = textarea.selectionEnd; if (selEnd !== selStart) return; // do not process when there is a selection @@ -66,9 +76,9 @@ function handleNewline(textarea, e) { const indention = /^\s*/.exec(line)[0]; line = line.slice(indention.length); - // parse the prefixes: "1. ", "- ", "* ", "[ ] ", "[x] " + // parse the prefixes: "1. ", "- ", "* ", there could also be " [ ] " or " [x] " for task lists // there must be a space after the prefix because none of "1.foo" / "-foo" is a list item - const prefixMatch = /^([0-9]+\.|[-*]|\[ \]|\[x\])\s/.exec(line); + const prefixMatch = /^([0-9]+\.|[-*])(\s\[([ x])\])?\s/.exec(line); let prefix = ''; if (prefixMatch) { prefix = prefixMatch[0]; @@ -85,8 +95,9 @@ function handleNewline(textarea, e) { } else { // start a new line with the same indention and prefix let newPrefix = prefix; - if (newPrefix === '[x]') newPrefix = '[ ]'; - if (/^\d+\./.test(newPrefix)) newPrefix = `1. `; // a simple approach, otherwise it needs to parse the lines after the current line + // a simple approach, otherwise it needs to parse the lines after the current line + if (/^\d+\./.test(prefix)) newPrefix = `1. ${newPrefix.slice(newPrefix.indexOf('.') + 2)}`; + newPrefix = newPrefix.replace('[x]', '[ ]'); const newLine = `\n${indention}${newPrefix}`; textarea.value = value.slice(0, selStart) + newLine + value.slice(selEnd); textarea.setSelectionRange(selStart + newLine.length, selStart + newLine.length); diff --git a/web_src/js/features/comp/EditorUpload.ts b/web_src/js/features/comp/EditorUpload.ts index 582639a817..b1f49cbe92 100644 --- a/web_src/js/features/comp/EditorUpload.ts +++ b/web_src/js/features/comp/EditorUpload.ts @@ -1,7 +1,7 @@ import {imageInfo} from '../../utils/image.ts'; import {replaceTextareaSelection} from '../../utils/dom.ts'; import {isUrl} from '../../utils/url.ts'; -import {triggerEditorContentChanged} from './EditorMarkdown.ts'; +import {textareaInsertText, triggerEditorContentChanged} from './EditorMarkdown.ts'; import { DropzoneCustomEventRemovedFile, DropzoneCustomEventUploadDone, @@ -41,14 +41,7 @@ class TextareaEditor { } insertPlaceholder(value) { - const editor = this.editor; - const startPos = editor.selectionStart; - const endPos = editor.selectionEnd; - editor.value = editor.value.substring(0, startPos) + value + editor.value.substring(endPos); - editor.selectionStart = startPos; - editor.selectionEnd = startPos + value.length; - editor.focus(); - triggerEditorContentChanged(editor); + textareaInsertText(this.editor, value); } replacePlaceholder(oldVal, newVal) { diff --git a/web_src/js/modules/tippy.ts b/web_src/js/modules/tippy.ts index 375d816c6b..d75015f69e 100644 --- a/web_src/js/modules/tippy.ts +++ b/web_src/js/modules/tippy.ts @@ -11,7 +11,7 @@ type TippyOpts = { const visibleInstances = new Set(); const arrowSvg = ``; -export function createTippy(target: Element, opts: TippyOpts = {}) { +export function createTippy(target: Element, opts: TippyOpts = {}): Instance { // the callback functions should be destructured from opts, // because we should use our own wrapper functions to handle them, do not let the user override them const {onHide, onShow, onDestroy, role, theme, arrow, ...other} = opts; From 61be51e56baf037aa7902e7cd066b895a10da244 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Mon, 4 Nov 2024 18:59:50 +0800 Subject: [PATCH 009/156] Refactor markup package (#32399) To make the markup package easier to maintain: 1. Split some go files into small files 2. Use a shared util.NopCloser, remove duplicate code 3. Remove unused functions --- modules/gitrepo/gitrepo.go | 9 +- modules/log/event_writer_console.go | 13 +- modules/log/event_writer_file.go | 3 +- modules/markup/html.go | 757 ------------------- modules/markup/html_commit.go | 225 ++++++ modules/markup/html_email.go | 21 + modules/markup/html_emoji.go | 115 +++ modules/markup/html_issue.go | 180 +++++ modules/markup/html_link.go | 227 ++++++ modules/markup/html_mention.go | 54 ++ modules/markup/markdown/goldmark.go | 2 +- modules/markup/markdown/toc.go | 10 +- modules/markup/markdown/transform_heading.go | 4 +- modules/markup/render.go | 226 ++++++ modules/markup/render_helper.go | 21 + modules/markup/render_links.go | 56 ++ modules/markup/renderer.go | 301 +------- modules/packages/debian/metadata_test.go | 11 +- modules/util/io.go | 6 + 19 files changed, 1154 insertions(+), 1087 deletions(-) create mode 100644 modules/markup/html_commit.go create mode 100644 modules/markup/html_email.go create mode 100644 modules/markup/html_emoji.go create mode 100644 modules/markup/html_issue.go create mode 100644 modules/markup/html_mention.go create mode 100644 modules/markup/render.go create mode 100644 modules/markup/render_helper.go create mode 100644 modules/markup/render_links.go diff --git a/modules/gitrepo/gitrepo.go b/modules/gitrepo/gitrepo.go index d89f8f9c0c..14d809aedb 100644 --- a/modules/gitrepo/gitrepo.go +++ b/modules/gitrepo/gitrepo.go @@ -11,6 +11,7 @@ import ( "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" ) type Repository interface { @@ -59,15 +60,11 @@ func repositoryFromContext(ctx context.Context, repo Repository) *git.Repository return nil } -type nopCloser func() - -func (nopCloser) Close() error { return nil } - // RepositoryFromContextOrOpen attempts to get the repository from the context or just opens it func RepositoryFromContextOrOpen(ctx context.Context, repo Repository) (*git.Repository, io.Closer, error) { gitRepo := repositoryFromContext(ctx, repo) if gitRepo != nil { - return gitRepo, nopCloser(nil), nil + return gitRepo, util.NopCloser{}, nil } gitRepo, err := OpenRepository(ctx, repo) @@ -95,7 +92,7 @@ func repositoryFromContextPath(ctx context.Context, path string) *git.Repository func RepositoryFromContextOrOpenPath(ctx context.Context, path string) (*git.Repository, io.Closer, error) { gitRepo := repositoryFromContextPath(ctx, path) if gitRepo != nil { - return gitRepo, nopCloser(nil), nil + return gitRepo, util.NopCloser{}, nil } gitRepo, err := git.OpenRepository(ctx, path) diff --git a/modules/log/event_writer_console.go b/modules/log/event_writer_console.go index 78183de644..e4c409d83e 100644 --- a/modules/log/event_writer_console.go +++ b/modules/log/event_writer_console.go @@ -4,8 +4,9 @@ package log import ( - "io" "os" + + "code.gitea.io/gitea/modules/util" ) type WriterConsoleOption struct { @@ -18,19 +19,13 @@ type eventWriterConsole struct { var _ EventWriter = (*eventWriterConsole)(nil) -type nopCloser struct { - io.Writer -} - -func (nopCloser) Close() error { return nil } - func NewEventWriterConsole(name string, mode WriterMode) EventWriter { w := &eventWriterConsole{EventWriterBaseImpl: NewEventWriterBase(name, "console", mode)} opt := mode.WriterOption.(WriterConsoleOption) if opt.Stderr { - w.OutputWriteCloser = nopCloser{os.Stderr} + w.OutputWriteCloser = util.NopCloser{Writer: os.Stderr} } else { - w.OutputWriteCloser = nopCloser{os.Stdout} + w.OutputWriteCloser = util.NopCloser{Writer: os.Stdout} } return w } diff --git a/modules/log/event_writer_file.go b/modules/log/event_writer_file.go index fd73d7d30a..f26286498a 100644 --- a/modules/log/event_writer_file.go +++ b/modules/log/event_writer_file.go @@ -6,6 +6,7 @@ package log import ( "io" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/util/rotatingfilewriter" ) @@ -42,7 +43,7 @@ func NewEventWriterFile(name string, mode WriterMode) EventWriter { // if the log file can't be opened, what should it do? panic/exit? ignore logs? fallback to stderr? // it seems that "fallback to stderr" is slightly better than others .... FallbackErrorf("unable to open log file %q: %v", opt.FileName, err) - w.fileWriter = nopCloser{Writer: LoggerToWriter(FallbackErrorf)} + w.fileWriter = util.NopCloser{Writer: LoggerToWriter(FallbackErrorf)} } w.OutputWriteCloser = w.fileWriter return w diff --git a/modules/markup/html.go b/modules/markup/html.go index 8d3327c49e..a9c3dc9ba2 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -6,25 +6,12 @@ package markup import ( "bytes" "io" - "net/url" - "path" - "path/filepath" "regexp" - "slices" "strings" "sync" - "code.gitea.io/gitea/modules/base" - "code.gitea.io/gitea/modules/emoji" - "code.gitea.io/gitea/modules/gitrepo" - "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/markup/common" - "code.gitea.io/gitea/modules/references" - "code.gitea.io/gitea/modules/regexplru" "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/modules/templates/vars" - "code.gitea.io/gitea/modules/translation" - "code.gitea.io/gitea/modules/util" "golang.org/x/net/html" "golang.org/x/net/html/atom" @@ -451,50 +438,6 @@ func createKeyword(content string) *html.Node { return span } -func createEmoji(content, class, name string) *html.Node { - span := &html.Node{ - Type: html.ElementNode, - Data: atom.Span.String(), - Attr: []html.Attribute{}, - } - if class != "" { - span.Attr = append(span.Attr, html.Attribute{Key: "class", Val: class}) - } - if name != "" { - span.Attr = append(span.Attr, html.Attribute{Key: "aria-label", Val: name}) - } - - text := &html.Node{ - Type: html.TextNode, - Data: content, - } - - span.AppendChild(text) - return span -} - -func createCustomEmoji(alias string) *html.Node { - span := &html.Node{ - Type: html.ElementNode, - Data: atom.Span.String(), - Attr: []html.Attribute{}, - } - span.Attr = append(span.Attr, html.Attribute{Key: "class", Val: "emoji"}) - span.Attr = append(span.Attr, html.Attribute{Key: "aria-label", Val: alias}) - - img := &html.Node{ - Type: html.ElementNode, - DataAtom: atom.Img, - Data: "img", - Attr: []html.Attribute{}, - } - img.Attr = append(img.Attr, html.Attribute{Key: "alt", Val: ":" + alias + ":"}) - img.Attr = append(img.Attr, html.Attribute{Key: "src", Val: setting.StaticURLPrefix + "/assets/img/emoji/" + alias + ".png"}) - - span.AppendChild(img) - return span -} - func createLink(href, content, class string) *html.Node { a := &html.Node{ Type: html.ElementNode, @@ -515,33 +458,6 @@ func createLink(href, content, class string) *html.Node { return a } -func createCodeLink(href, content, class string) *html.Node { - a := &html.Node{ - Type: html.ElementNode, - Data: atom.A.String(), - Attr: []html.Attribute{{Key: "href", Val: href}}, - } - - if class != "" { - a.Attr = append(a.Attr, html.Attribute{Key: "class", Val: class}) - } - - text := &html.Node{ - Type: html.TextNode, - Data: content, - } - - code := &html.Node{ - Type: html.ElementNode, - Data: atom.Code.String(), - Attr: []html.Attribute{{Key: "class", Val: "nohighlight"}}, - } - - code.AppendChild(text) - a.AppendChild(code) - return a -} - // replaceContent takes text node, and in its content it replaces a section of // it with the specified newNode. func replaceContent(node *html.Node, i, j int, newNode *html.Node) { @@ -573,676 +489,3 @@ func replaceContentList(node *html.Node, i, j int, newNodes []*html.Node) { }, nextSibling) } } - -func mentionProcessor(ctx *RenderContext, node *html.Node) { - start := 0 - nodeStop := node.NextSibling - for node != nodeStop { - found, loc := references.FindFirstMentionBytes(util.UnsafeStringToBytes(node.Data[start:])) - if !found { - node = node.NextSibling - start = 0 - continue - } - loc.Start += start - loc.End += start - mention := node.Data[loc.Start:loc.End] - teams, ok := ctx.Metas["teams"] - // FIXME: util.URLJoin may not be necessary here: - // - setting.AppURL is defined to have a terminal '/' so unless mention[1:] - // is an AppSubURL link we can probably fallback to concatenation. - // team mention should follow @orgName/teamName style - if ok && strings.Contains(mention, "/") { - mentionOrgAndTeam := strings.Split(mention, "/") - if mentionOrgAndTeam[0][1:] == ctx.Metas["org"] && strings.Contains(teams, ","+strings.ToLower(mentionOrgAndTeam[1])+",") { - replaceContent(node, loc.Start, loc.End, createLink(util.URLJoin(ctx.Links.Prefix(), "org", ctx.Metas["org"], "teams", mentionOrgAndTeam[1]), mention, "mention")) - node = node.NextSibling.NextSibling - start = 0 - continue - } - start = loc.End - continue - } - mentionedUsername := mention[1:] - - if DefaultProcessorHelper.IsUsernameMentionable != nil && DefaultProcessorHelper.IsUsernameMentionable(ctx.Ctx, mentionedUsername) { - replaceContent(node, loc.Start, loc.End, createLink(util.URLJoin(ctx.Links.Prefix(), mentionedUsername), mention, "mention")) - node = node.NextSibling.NextSibling - start = 0 - } else { - start = loc.End - } - } -} - -func shortLinkProcessor(ctx *RenderContext, node *html.Node) { - next := node.NextSibling - for node != nil && node != next { - m := shortLinkPattern.FindStringSubmatchIndex(node.Data) - if m == nil { - return - } - - content := node.Data[m[2]:m[3]] - tail := node.Data[m[4]:m[5]] - props := make(map[string]string) - - // MediaWiki uses [[link|text]], while GitHub uses [[text|link]] - // It makes page handling terrible, but we prefer GitHub syntax - // And fall back to MediaWiki only when it is obvious from the look - // Of text and link contents - sl := strings.Split(content, "|") - for _, v := range sl { - if equalPos := strings.IndexByte(v, '='); equalPos == -1 { - // There is no equal in this argument; this is a mandatory arg - if props["name"] == "" { - if IsFullURLString(v) { - // If we clearly see it is a link, we save it so - - // But first we need to ensure, that if both mandatory args provided - // look like links, we stick to GitHub syntax - if props["link"] != "" { - props["name"] = props["link"] - } - - props["link"] = strings.TrimSpace(v) - } else { - props["name"] = v - } - } else { - props["link"] = strings.TrimSpace(v) - } - } else { - // There is an equal; optional argument. - - sep := strings.IndexByte(v, '=') - key, val := v[:sep], html.UnescapeString(v[sep+1:]) - - // When parsing HTML, x/net/html will change all quotes which are - // not used for syntax into UTF-8 quotes. So checking val[0] won't - // be enough, since that only checks a single byte. - if len(val) > 1 { - if (strings.HasPrefix(val, "“") && strings.HasSuffix(val, "”")) || - (strings.HasPrefix(val, "‘") && strings.HasSuffix(val, "’")) { - const lenQuote = len("‘") - val = val[lenQuote : len(val)-lenQuote] - } else if (strings.HasPrefix(val, "\"") && strings.HasSuffix(val, "\"")) || - (strings.HasPrefix(val, "'") && strings.HasSuffix(val, "'")) { - val = val[1 : len(val)-1] - } else if strings.HasPrefix(val, "'") && strings.HasSuffix(val, "’") { - const lenQuote = len("‘") - val = val[1 : len(val)-lenQuote] - } - } - props[key] = val - } - } - - var name, link string - if props["link"] != "" { - link = props["link"] - } else if props["name"] != "" { - link = props["name"] - } - if props["title"] != "" { - name = props["title"] - } else if props["name"] != "" { - name = props["name"] - } else { - name = link - } - - name += tail - image := false - ext := filepath.Ext(link) - switch ext { - // fast path: empty string, ignore - case "": - // leave image as false - case ".jpg", ".jpeg", ".png", ".tif", ".tiff", ".webp", ".gif", ".bmp", ".ico", ".svg": - image = true - } - - childNode := &html.Node{} - linkNode := &html.Node{ - FirstChild: childNode, - LastChild: childNode, - Type: html.ElementNode, - Data: "a", - DataAtom: atom.A, - } - childNode.Parent = linkNode - absoluteLink := IsFullURLString(link) - if !absoluteLink { - if image { - link = strings.ReplaceAll(link, " ", "+") - } else { - link = strings.ReplaceAll(link, " ", "-") // FIXME: it should support dashes in the link, eg: "the-dash-support.-" - } - if !strings.Contains(link, "/") { - link = url.PathEscape(link) // FIXME: it doesn't seem right and it might cause double-escaping - } - } - if image { - if !absoluteLink { - link = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.IsWiki), link) - } - title := props["title"] - if title == "" { - title = props["alt"] - } - if title == "" { - title = path.Base(name) - } - alt := props["alt"] - if alt == "" { - alt = name - } - - // make the childNode an image - if we can, we also place the alt - childNode.Type = html.ElementNode - childNode.Data = "img" - childNode.DataAtom = atom.Img - childNode.Attr = []html.Attribute{ - {Key: "src", Val: link}, - {Key: "title", Val: title}, - {Key: "alt", Val: alt}, - } - if alt == "" { - childNode.Attr = childNode.Attr[:2] - } - } else { - link, _ = ResolveLink(ctx, link, "") - childNode.Type = html.TextNode - childNode.Data = name - } - linkNode.Attr = []html.Attribute{{Key: "href", Val: link}} - replaceContent(node, m[0], m[1], linkNode) - node = node.NextSibling.NextSibling - } -} - -func fullIssuePatternProcessor(ctx *RenderContext, node *html.Node) { - if ctx.Metas == nil { - return - } - next := node.NextSibling - for node != nil && node != next { - m := getIssueFullPattern().FindStringSubmatchIndex(node.Data) - if m == nil { - return - } - - mDiffView := getFilesChangedFullPattern().FindStringSubmatchIndex(node.Data) - // leave it as it is if the link is from "Files Changed" tab in PR Diff View https://domain/org/repo/pulls/27/files - if mDiffView != nil { - return - } - - link := node.Data[m[0]:m[1]] - text := "#" + node.Data[m[2]:m[3]] - // if m[4] and m[5] is not -1, then link is to a comment - // indicate that in the text by appending (comment) - if m[4] != -1 && m[5] != -1 { - if locale, ok := ctx.Ctx.Value(translation.ContextKey).(translation.Locale); ok { - text += " " + locale.TrString("repo.from_comment") - } else { - text += " (comment)" - } - } - - // extract repo and org name from matched link like - // http://localhost:3000/gituser/myrepo/issues/1 - linkParts := strings.Split(link, "/") - matchOrg := linkParts[len(linkParts)-4] - matchRepo := linkParts[len(linkParts)-3] - - if matchOrg == ctx.Metas["user"] && matchRepo == ctx.Metas["repo"] { - replaceContent(node, m[0], m[1], createLink(link, text, "ref-issue")) - } else { - text = matchOrg + "/" + matchRepo + text - replaceContent(node, m[0], m[1], createLink(link, text, "ref-issue")) - } - node = node.NextSibling.NextSibling - } -} - -func issueIndexPatternProcessor(ctx *RenderContext, node *html.Node) { - if ctx.Metas == nil { - return - } - - // FIXME: the use of "mode" is quite dirty and hacky, for example: what is a "document"? how should it be rendered? - // The "mode" approach should be refactored to some other more clear&reliable way. - crossLinkOnly := ctx.Metas["mode"] == "document" && !ctx.IsWiki - - var ( - found bool - ref *references.RenderizableReference - ) - - next := node.NextSibling - - for node != nil && node != next { - _, hasExtTrackFormat := ctx.Metas["format"] - - // Repos with external issue trackers might still need to reference local PRs - // We need to concern with the first one that shows up in the text, whichever it is - isNumericStyle := ctx.Metas["style"] == "" || ctx.Metas["style"] == IssueNameStyleNumeric - foundNumeric, refNumeric := references.FindRenderizableReferenceNumeric(node.Data, hasExtTrackFormat && !isNumericStyle, crossLinkOnly) - - switch ctx.Metas["style"] { - case "", IssueNameStyleNumeric: - found, ref = foundNumeric, refNumeric - case IssueNameStyleAlphanumeric: - found, ref = references.FindRenderizableReferenceAlphanumeric(node.Data) - case IssueNameStyleRegexp: - pattern, err := regexplru.GetCompiled(ctx.Metas["regexp"]) - if err != nil { - return - } - found, ref = references.FindRenderizableReferenceRegexp(node.Data, pattern) - } - - // Repos with external issue trackers might still need to reference local PRs - // We need to concern with the first one that shows up in the text, whichever it is - if hasExtTrackFormat && !isNumericStyle && refNumeric != nil { - // If numeric (PR) was found, and it was BEFORE the non-numeric pattern, use that - // Allow a free-pass when non-numeric pattern wasn't found. - if found && (ref == nil || refNumeric.RefLocation.Start < ref.RefLocation.Start) { - found = foundNumeric - ref = refNumeric - } - } - if !found { - return - } - - var link *html.Node - reftext := node.Data[ref.RefLocation.Start:ref.RefLocation.End] - if hasExtTrackFormat && !ref.IsPull { - ctx.Metas["index"] = ref.Issue - - res, err := vars.Expand(ctx.Metas["format"], ctx.Metas) - if err != nil { - // here we could just log the error and continue the rendering - log.Error("unable to expand template vars for ref %s, err: %v", ref.Issue, err) - } - - link = createLink(res, reftext, "ref-issue ref-external-issue") - } else { - // Path determines the type of link that will be rendered. It's unknown at this point whether - // the linked item is actually a PR or an issue. Luckily it's of no real consequence because - // Gitea will redirect on click as appropriate. - issuePath := util.Iif(ref.IsPull, "pulls", "issues") - if ref.Owner == "" { - link = createLink(util.URLJoin(ctx.Links.Prefix(), ctx.Metas["user"], ctx.Metas["repo"], issuePath, ref.Issue), reftext, "ref-issue") - } else { - link = createLink(util.URLJoin(ctx.Links.Prefix(), ref.Owner, ref.Name, issuePath, ref.Issue), reftext, "ref-issue") - } - } - - if ref.Action == references.XRefActionNone { - replaceContent(node, ref.RefLocation.Start, ref.RefLocation.End, link) - node = node.NextSibling.NextSibling - continue - } - - // Decorate action keywords if actionable - var keyword *html.Node - if references.IsXrefActionable(ref, hasExtTrackFormat) { - keyword = createKeyword(node.Data[ref.ActionLocation.Start:ref.ActionLocation.End]) - } else { - keyword = &html.Node{ - Type: html.TextNode, - Data: node.Data[ref.ActionLocation.Start:ref.ActionLocation.End], - } - } - spaces := &html.Node{ - Type: html.TextNode, - Data: node.Data[ref.ActionLocation.End:ref.RefLocation.Start], - } - replaceContentList(node, ref.ActionLocation.Start, ref.RefLocation.End, []*html.Node{keyword, spaces, link}) - node = node.NextSibling.NextSibling.NextSibling.NextSibling - } -} - -func commitCrossReferencePatternProcessor(ctx *RenderContext, node *html.Node) { - next := node.NextSibling - - for node != nil && node != next { - found, ref := references.FindRenderizableCommitCrossReference(node.Data) - if !found { - return - } - - reftext := ref.Owner + "/" + ref.Name + "@" + base.ShortSha(ref.CommitSha) - link := createLink(util.URLJoin(ctx.Links.Prefix(), ref.Owner, ref.Name, "commit", ref.CommitSha), reftext, "commit") - - replaceContent(node, ref.RefLocation.Start, ref.RefLocation.End, link) - node = node.NextSibling.NextSibling - } -} - -type anyHashPatternResult struct { - PosStart int - PosEnd int - FullURL string - CommitID string - SubPath string - QueryHash string -} - -func anyHashPatternExtract(s string) (ret anyHashPatternResult, ok bool) { - m := anyHashPattern.FindStringSubmatchIndex(s) - if m == nil { - return ret, false - } - - ret.PosStart, ret.PosEnd = m[0], m[1] - ret.FullURL = s[ret.PosStart:ret.PosEnd] - if strings.HasSuffix(ret.FullURL, ".") { - // if url ends in '.', it's very likely that it is not part of the actual url but used to finish a sentence. - ret.PosEnd-- - ret.FullURL = ret.FullURL[:len(ret.FullURL)-1] - for i := 0; i < len(m); i++ { - m[i] = min(m[i], ret.PosEnd) - } - } - - ret.CommitID = s[m[2]:m[3]] - if m[5] > 0 { - ret.SubPath = s[m[4]:m[5]] - } - - lastStart, lastEnd := m[len(m)-2], m[len(m)-1] - if lastEnd > 0 { - ret.QueryHash = s[lastStart:lastEnd][1:] - } - return ret, true -} - -// fullHashPatternProcessor renders SHA containing URLs -func fullHashPatternProcessor(ctx *RenderContext, node *html.Node) { - if ctx.Metas == nil { - return - } - nodeStop := node.NextSibling - for node != nodeStop { - if node.Type != html.TextNode { - node = node.NextSibling - continue - } - ret, ok := anyHashPatternExtract(node.Data) - if !ok { - node = node.NextSibling - continue - } - text := base.ShortSha(ret.CommitID) - if ret.SubPath != "" { - text += ret.SubPath - } - if ret.QueryHash != "" { - text += " (" + ret.QueryHash + ")" - } - replaceContent(node, ret.PosStart, ret.PosEnd, createCodeLink(ret.FullURL, text, "commit")) - node = node.NextSibling.NextSibling - } -} - -func comparePatternProcessor(ctx *RenderContext, node *html.Node) { - if ctx.Metas == nil { - return - } - nodeStop := node.NextSibling - for node != nodeStop { - if node.Type != html.TextNode { - node = node.NextSibling - continue - } - m := comparePattern.FindStringSubmatchIndex(node.Data) - if m == nil || slices.Contains(m[:8], -1) { // ensure that every group (m[0]...m[7]) has a match - node = node.NextSibling - continue - } - - urlFull := node.Data[m[0]:m[1]] - text1 := base.ShortSha(node.Data[m[2]:m[3]]) - textDots := base.ShortSha(node.Data[m[4]:m[5]]) - text2 := base.ShortSha(node.Data[m[6]:m[7]]) - - hash := "" - if m[9] > 0 { - hash = node.Data[m[8]:m[9]][1:] - } - - start := m[0] - end := m[1] - - // If url ends in '.', it's very likely that it is not part of the - // actual url but used to finish a sentence. - if strings.HasSuffix(urlFull, ".") { - end-- - urlFull = urlFull[:len(urlFull)-1] - if hash != "" { - hash = hash[:len(hash)-1] - } else if text2 != "" { - text2 = text2[:len(text2)-1] - } - } - - text := text1 + textDots + text2 - if hash != "" { - text += " (" + hash + ")" - } - replaceContent(node, start, end, createCodeLink(urlFull, text, "compare")) - node = node.NextSibling.NextSibling - } -} - -// emojiShortCodeProcessor for rendering text like :smile: into emoji -func emojiShortCodeProcessor(ctx *RenderContext, node *html.Node) { - start := 0 - next := node.NextSibling - for node != nil && node != next && start < len(node.Data) { - m := emojiShortCodeRegex.FindStringSubmatchIndex(node.Data[start:]) - if m == nil { - return - } - m[0] += start - m[1] += start - - start = m[1] - - alias := node.Data[m[0]:m[1]] - alias = strings.ReplaceAll(alias, ":", "") - converted := emoji.FromAlias(alias) - if converted == nil { - // check if this is a custom reaction - if _, exist := setting.UI.CustomEmojisMap[alias]; exist { - replaceContent(node, m[0], m[1], createCustomEmoji(alias)) - node = node.NextSibling.NextSibling - start = 0 - continue - } - continue - } - - replaceContent(node, m[0], m[1], createEmoji(converted.Emoji, "emoji", converted.Description)) - node = node.NextSibling.NextSibling - start = 0 - } -} - -// emoji processor to match emoji and add emoji class -func emojiProcessor(ctx *RenderContext, node *html.Node) { - start := 0 - next := node.NextSibling - for node != nil && node != next && start < len(node.Data) { - m := emoji.FindEmojiSubmatchIndex(node.Data[start:]) - if m == nil { - return - } - m[0] += start - m[1] += start - - codepoint := node.Data[m[0]:m[1]] - start = m[1] - val := emoji.FromCode(codepoint) - if val != nil { - replaceContent(node, m[0], m[1], createEmoji(codepoint, "emoji", val.Description)) - node = node.NextSibling.NextSibling - start = 0 - } - } -} - -// hashCurrentPatternProcessor renders SHA1 strings to corresponding links that -// are assumed to be in the same repository. -func hashCurrentPatternProcessor(ctx *RenderContext, node *html.Node) { - if ctx.Metas == nil || ctx.Metas["user"] == "" || ctx.Metas["repo"] == "" || (ctx.Repo == nil && ctx.GitRepo == nil) { - return - } - - start := 0 - next := node.NextSibling - if ctx.ShaExistCache == nil { - ctx.ShaExistCache = make(map[string]bool) - } - for node != nil && node != next && start < len(node.Data) { - m := hashCurrentPattern.FindStringSubmatchIndex(node.Data[start:]) - if m == nil { - return - } - m[2] += start - m[3] += start - - hash := node.Data[m[2]:m[3]] - // The regex does not lie, it matches the hash pattern. - // However, a regex cannot know if a hash actually exists or not. - // We could assume that a SHA1 hash should probably contain alphas AND numerics - // but that is not always the case. - // Although unlikely, deadbeef and 1234567 are valid short forms of SHA1 hash - // as used by git and github for linking and thus we have to do similar. - // Because of this, we check to make sure that a matched hash is actually - // a commit in the repository before making it a link. - - // check cache first - exist, inCache := ctx.ShaExistCache[hash] - if !inCache { - if ctx.GitRepo == nil { - var err error - var closer io.Closer - ctx.GitRepo, closer, err = gitrepo.RepositoryFromContextOrOpen(ctx.Ctx, ctx.Repo) - if err != nil { - log.Error("unable to open repository: %s Error: %v", gitrepo.RepoGitURL(ctx.Repo), err) - return - } - ctx.AddCancel(func() { - _ = closer.Close() - ctx.GitRepo = nil - }) - } - - // Don't use IsObjectExist since it doesn't support short hashs with gogit edition. - exist = ctx.GitRepo.IsReferenceExist(hash) - ctx.ShaExistCache[hash] = exist - } - - if !exist { - start = m[3] - continue - } - - link := util.URLJoin(ctx.Links.Prefix(), ctx.Metas["user"], ctx.Metas["repo"], "commit", hash) - replaceContent(node, m[2], m[3], createCodeLink(link, base.ShortSha(hash), "commit")) - start = 0 - node = node.NextSibling.NextSibling - } -} - -// emailAddressProcessor replaces raw email addresses with a mailto: link. -func emailAddressProcessor(ctx *RenderContext, node *html.Node) { - next := node.NextSibling - for node != nil && node != next { - m := emailRegex.FindStringSubmatchIndex(node.Data) - if m == nil { - return - } - - mail := node.Data[m[2]:m[3]] - replaceContent(node, m[2], m[3], createLink("mailto:"+mail, mail, "mailto")) - node = node.NextSibling.NextSibling - } -} - -// linkProcessor creates links for any HTTP or HTTPS URL not captured by -// markdown. -func linkProcessor(ctx *RenderContext, node *html.Node) { - next := node.NextSibling - for node != nil && node != next { - m := common.LinkRegex.FindStringIndex(node.Data) - if m == nil { - return - } - - uri := node.Data[m[0]:m[1]] - replaceContent(node, m[0], m[1], createLink(uri, uri, "link")) - node = node.NextSibling.NextSibling - } -} - -func genDefaultLinkProcessor(defaultLink string) processor { - return func(ctx *RenderContext, node *html.Node) { - ch := &html.Node{ - Parent: node, - Type: html.TextNode, - Data: node.Data, - } - - node.Type = html.ElementNode - node.Data = "a" - node.DataAtom = atom.A - node.Attr = []html.Attribute{ - {Key: "href", Val: defaultLink}, - {Key: "class", Val: "default-link muted"}, - } - node.FirstChild, node.LastChild = ch, ch - } -} - -// descriptionLinkProcessor creates links for DescriptionHTML -func descriptionLinkProcessor(ctx *RenderContext, node *html.Node) { - next := node.NextSibling - for node != nil && node != next { - m := common.LinkRegex.FindStringIndex(node.Data) - if m == nil { - return - } - - uri := node.Data[m[0]:m[1]] - replaceContent(node, m[0], m[1], createDescriptionLink(uri, uri)) - node = node.NextSibling.NextSibling - } -} - -func createDescriptionLink(href, content string) *html.Node { - textNode := &html.Node{ - Type: html.TextNode, - Data: content, - } - linkNode := &html.Node{ - FirstChild: textNode, - LastChild: textNode, - Type: html.ElementNode, - Data: "a", - DataAtom: atom.A, - Attr: []html.Attribute{ - {Key: "href", Val: href}, - {Key: "target", Val: "_blank"}, - {Key: "rel", Val: "noopener noreferrer"}, - }, - } - textNode.Parent = linkNode - return linkNode -} diff --git a/modules/markup/html_commit.go b/modules/markup/html_commit.go new file mode 100644 index 0000000000..86d70746d4 --- /dev/null +++ b/modules/markup/html_commit.go @@ -0,0 +1,225 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package markup + +import ( + "io" + "slices" + "strings" + + "code.gitea.io/gitea/modules/base" + "code.gitea.io/gitea/modules/gitrepo" + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/util" + + "golang.org/x/net/html" + "golang.org/x/net/html/atom" +) + +type anyHashPatternResult struct { + PosStart int + PosEnd int + FullURL string + CommitID string + SubPath string + QueryHash string +} + +func createCodeLink(href, content, class string) *html.Node { + a := &html.Node{ + Type: html.ElementNode, + Data: atom.A.String(), + Attr: []html.Attribute{{Key: "href", Val: href}}, + } + + if class != "" { + a.Attr = append(a.Attr, html.Attribute{Key: "class", Val: class}) + } + + text := &html.Node{ + Type: html.TextNode, + Data: content, + } + + code := &html.Node{ + Type: html.ElementNode, + Data: atom.Code.String(), + Attr: []html.Attribute{{Key: "class", Val: "nohighlight"}}, + } + + code.AppendChild(text) + a.AppendChild(code) + return a +} + +func anyHashPatternExtract(s string) (ret anyHashPatternResult, ok bool) { + m := anyHashPattern.FindStringSubmatchIndex(s) + if m == nil { + return ret, false + } + + ret.PosStart, ret.PosEnd = m[0], m[1] + ret.FullURL = s[ret.PosStart:ret.PosEnd] + if strings.HasSuffix(ret.FullURL, ".") { + // if url ends in '.', it's very likely that it is not part of the actual url but used to finish a sentence. + ret.PosEnd-- + ret.FullURL = ret.FullURL[:len(ret.FullURL)-1] + for i := 0; i < len(m); i++ { + m[i] = min(m[i], ret.PosEnd) + } + } + + ret.CommitID = s[m[2]:m[3]] + if m[5] > 0 { + ret.SubPath = s[m[4]:m[5]] + } + + lastStart, lastEnd := m[len(m)-2], m[len(m)-1] + if lastEnd > 0 { + ret.QueryHash = s[lastStart:lastEnd][1:] + } + return ret, true +} + +// fullHashPatternProcessor renders SHA containing URLs +func fullHashPatternProcessor(ctx *RenderContext, node *html.Node) { + if ctx.Metas == nil { + return + } + nodeStop := node.NextSibling + for node != nodeStop { + if node.Type != html.TextNode { + node = node.NextSibling + continue + } + ret, ok := anyHashPatternExtract(node.Data) + if !ok { + node = node.NextSibling + continue + } + text := base.ShortSha(ret.CommitID) + if ret.SubPath != "" { + text += ret.SubPath + } + if ret.QueryHash != "" { + text += " (" + ret.QueryHash + ")" + } + replaceContent(node, ret.PosStart, ret.PosEnd, createCodeLink(ret.FullURL, text, "commit")) + node = node.NextSibling.NextSibling + } +} + +func comparePatternProcessor(ctx *RenderContext, node *html.Node) { + if ctx.Metas == nil { + return + } + nodeStop := node.NextSibling + for node != nodeStop { + if node.Type != html.TextNode { + node = node.NextSibling + continue + } + m := comparePattern.FindStringSubmatchIndex(node.Data) + if m == nil || slices.Contains(m[:8], -1) { // ensure that every group (m[0]...m[7]) has a match + node = node.NextSibling + continue + } + + urlFull := node.Data[m[0]:m[1]] + text1 := base.ShortSha(node.Data[m[2]:m[3]]) + textDots := base.ShortSha(node.Data[m[4]:m[5]]) + text2 := base.ShortSha(node.Data[m[6]:m[7]]) + + hash := "" + if m[9] > 0 { + hash = node.Data[m[8]:m[9]][1:] + } + + start := m[0] + end := m[1] + + // If url ends in '.', it's very likely that it is not part of the + // actual url but used to finish a sentence. + if strings.HasSuffix(urlFull, ".") { + end-- + urlFull = urlFull[:len(urlFull)-1] + if hash != "" { + hash = hash[:len(hash)-1] + } else if text2 != "" { + text2 = text2[:len(text2)-1] + } + } + + text := text1 + textDots + text2 + if hash != "" { + text += " (" + hash + ")" + } + replaceContent(node, start, end, createCodeLink(urlFull, text, "compare")) + node = node.NextSibling.NextSibling + } +} + +// hashCurrentPatternProcessor renders SHA1 strings to corresponding links that +// are assumed to be in the same repository. +func hashCurrentPatternProcessor(ctx *RenderContext, node *html.Node) { + if ctx.Metas == nil || ctx.Metas["user"] == "" || ctx.Metas["repo"] == "" || (ctx.Repo == nil && ctx.GitRepo == nil) { + return + } + + start := 0 + next := node.NextSibling + if ctx.ShaExistCache == nil { + ctx.ShaExistCache = make(map[string]bool) + } + for node != nil && node != next && start < len(node.Data) { + m := hashCurrentPattern.FindStringSubmatchIndex(node.Data[start:]) + if m == nil { + return + } + m[2] += start + m[3] += start + + hash := node.Data[m[2]:m[3]] + // The regex does not lie, it matches the hash pattern. + // However, a regex cannot know if a hash actually exists or not. + // We could assume that a SHA1 hash should probably contain alphas AND numerics + // but that is not always the case. + // Although unlikely, deadbeef and 1234567 are valid short forms of SHA1 hash + // as used by git and github for linking and thus we have to do similar. + // Because of this, we check to make sure that a matched hash is actually + // a commit in the repository before making it a link. + + // check cache first + exist, inCache := ctx.ShaExistCache[hash] + if !inCache { + if ctx.GitRepo == nil { + var err error + var closer io.Closer + ctx.GitRepo, closer, err = gitrepo.RepositoryFromContextOrOpen(ctx.Ctx, ctx.Repo) + if err != nil { + log.Error("unable to open repository: %s Error: %v", gitrepo.RepoGitURL(ctx.Repo), err) + return + } + ctx.AddCancel(func() { + _ = closer.Close() + ctx.GitRepo = nil + }) + } + + // Don't use IsObjectExist since it doesn't support short hashs with gogit edition. + exist = ctx.GitRepo.IsReferenceExist(hash) + ctx.ShaExistCache[hash] = exist + } + + if !exist { + start = m[3] + continue + } + + link := util.URLJoin(ctx.Links.Prefix(), ctx.Metas["user"], ctx.Metas["repo"], "commit", hash) + replaceContent(node, m[2], m[3], createCodeLink(link, base.ShortSha(hash), "commit")) + start = 0 + node = node.NextSibling.NextSibling + } +} diff --git a/modules/markup/html_email.go b/modules/markup/html_email.go new file mode 100644 index 0000000000..a062789b35 --- /dev/null +++ b/modules/markup/html_email.go @@ -0,0 +1,21 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package markup + +import "golang.org/x/net/html" + +// emailAddressProcessor replaces raw email addresses with a mailto: link. +func emailAddressProcessor(ctx *RenderContext, node *html.Node) { + next := node.NextSibling + for node != nil && node != next { + m := emailRegex.FindStringSubmatchIndex(node.Data) + if m == nil { + return + } + + mail := node.Data[m[2]:m[3]] + replaceContent(node, m[2], m[3], createLink("mailto:"+mail, mail, "mailto")) + node = node.NextSibling.NextSibling + } +} diff --git a/modules/markup/html_emoji.go b/modules/markup/html_emoji.go new file mode 100644 index 0000000000..c60d06b823 --- /dev/null +++ b/modules/markup/html_emoji.go @@ -0,0 +1,115 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package markup + +import ( + "strings" + + "code.gitea.io/gitea/modules/emoji" + "code.gitea.io/gitea/modules/setting" + + "golang.org/x/net/html" + "golang.org/x/net/html/atom" +) + +func createEmoji(content, class, name string) *html.Node { + span := &html.Node{ + Type: html.ElementNode, + Data: atom.Span.String(), + Attr: []html.Attribute{}, + } + if class != "" { + span.Attr = append(span.Attr, html.Attribute{Key: "class", Val: class}) + } + if name != "" { + span.Attr = append(span.Attr, html.Attribute{Key: "aria-label", Val: name}) + } + + text := &html.Node{ + Type: html.TextNode, + Data: content, + } + + span.AppendChild(text) + return span +} + +func createCustomEmoji(alias string) *html.Node { + span := &html.Node{ + Type: html.ElementNode, + Data: atom.Span.String(), + Attr: []html.Attribute{}, + } + span.Attr = append(span.Attr, html.Attribute{Key: "class", Val: "emoji"}) + span.Attr = append(span.Attr, html.Attribute{Key: "aria-label", Val: alias}) + + img := &html.Node{ + Type: html.ElementNode, + DataAtom: atom.Img, + Data: "img", + Attr: []html.Attribute{}, + } + img.Attr = append(img.Attr, html.Attribute{Key: "alt", Val: ":" + alias + ":"}) + img.Attr = append(img.Attr, html.Attribute{Key: "src", Val: setting.StaticURLPrefix + "/assets/img/emoji/" + alias + ".png"}) + + span.AppendChild(img) + return span +} + +// emojiShortCodeProcessor for rendering text like :smile: into emoji +func emojiShortCodeProcessor(ctx *RenderContext, node *html.Node) { + start := 0 + next := node.NextSibling + for node != nil && node != next && start < len(node.Data) { + m := emojiShortCodeRegex.FindStringSubmatchIndex(node.Data[start:]) + if m == nil { + return + } + m[0] += start + m[1] += start + + start = m[1] + + alias := node.Data[m[0]:m[1]] + alias = strings.ReplaceAll(alias, ":", "") + converted := emoji.FromAlias(alias) + if converted == nil { + // check if this is a custom reaction + if _, exist := setting.UI.CustomEmojisMap[alias]; exist { + replaceContent(node, m[0], m[1], createCustomEmoji(alias)) + node = node.NextSibling.NextSibling + start = 0 + continue + } + continue + } + + replaceContent(node, m[0], m[1], createEmoji(converted.Emoji, "emoji", converted.Description)) + node = node.NextSibling.NextSibling + start = 0 + } +} + +// emoji processor to match emoji and add emoji class +func emojiProcessor(ctx *RenderContext, node *html.Node) { + start := 0 + next := node.NextSibling + for node != nil && node != next && start < len(node.Data) { + m := emoji.FindEmojiSubmatchIndex(node.Data[start:]) + if m == nil { + return + } + m[0] += start + m[1] += start + + codepoint := node.Data[m[0]:m[1]] + start = m[1] + val := emoji.FromCode(codepoint) + if val != nil { + replaceContent(node, m[0], m[1], createEmoji(codepoint, "emoji", val.Description)) + node = node.NextSibling.NextSibling + start = 0 + } + } +} diff --git a/modules/markup/html_issue.go b/modules/markup/html_issue.go new file mode 100644 index 0000000000..b6d4ed6a8e --- /dev/null +++ b/modules/markup/html_issue.go @@ -0,0 +1,180 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package markup + +import ( + "strings" + + "code.gitea.io/gitea/modules/base" + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/references" + "code.gitea.io/gitea/modules/regexplru" + "code.gitea.io/gitea/modules/templates/vars" + "code.gitea.io/gitea/modules/translation" + "code.gitea.io/gitea/modules/util" + + "golang.org/x/net/html" +) + +func fullIssuePatternProcessor(ctx *RenderContext, node *html.Node) { + if ctx.Metas == nil { + return + } + next := node.NextSibling + for node != nil && node != next { + m := getIssueFullPattern().FindStringSubmatchIndex(node.Data) + if m == nil { + return + } + + mDiffView := getFilesChangedFullPattern().FindStringSubmatchIndex(node.Data) + // leave it as it is if the link is from "Files Changed" tab in PR Diff View https://domain/org/repo/pulls/27/files + if mDiffView != nil { + return + } + + link := node.Data[m[0]:m[1]] + text := "#" + node.Data[m[2]:m[3]] + // if m[4] and m[5] is not -1, then link is to a comment + // indicate that in the text by appending (comment) + if m[4] != -1 && m[5] != -1 { + if locale, ok := ctx.Ctx.Value(translation.ContextKey).(translation.Locale); ok { + text += " " + locale.TrString("repo.from_comment") + } else { + text += " (comment)" + } + } + + // extract repo and org name from matched link like + // http://localhost:3000/gituser/myrepo/issues/1 + linkParts := strings.Split(link, "/") + matchOrg := linkParts[len(linkParts)-4] + matchRepo := linkParts[len(linkParts)-3] + + if matchOrg == ctx.Metas["user"] && matchRepo == ctx.Metas["repo"] { + replaceContent(node, m[0], m[1], createLink(link, text, "ref-issue")) + } else { + text = matchOrg + "/" + matchRepo + text + replaceContent(node, m[0], m[1], createLink(link, text, "ref-issue")) + } + node = node.NextSibling.NextSibling + } +} + +func issueIndexPatternProcessor(ctx *RenderContext, node *html.Node) { + if ctx.Metas == nil { + return + } + + // FIXME: the use of "mode" is quite dirty and hacky, for example: what is a "document"? how should it be rendered? + // The "mode" approach should be refactored to some other more clear&reliable way. + crossLinkOnly := ctx.Metas["mode"] == "document" && !ctx.IsWiki + + var ( + found bool + ref *references.RenderizableReference + ) + + next := node.NextSibling + + for node != nil && node != next { + _, hasExtTrackFormat := ctx.Metas["format"] + + // Repos with external issue trackers might still need to reference local PRs + // We need to concern with the first one that shows up in the text, whichever it is + isNumericStyle := ctx.Metas["style"] == "" || ctx.Metas["style"] == IssueNameStyleNumeric + foundNumeric, refNumeric := references.FindRenderizableReferenceNumeric(node.Data, hasExtTrackFormat && !isNumericStyle, crossLinkOnly) + + switch ctx.Metas["style"] { + case "", IssueNameStyleNumeric: + found, ref = foundNumeric, refNumeric + case IssueNameStyleAlphanumeric: + found, ref = references.FindRenderizableReferenceAlphanumeric(node.Data) + case IssueNameStyleRegexp: + pattern, err := regexplru.GetCompiled(ctx.Metas["regexp"]) + if err != nil { + return + } + found, ref = references.FindRenderizableReferenceRegexp(node.Data, pattern) + } + + // Repos with external issue trackers might still need to reference local PRs + // We need to concern with the first one that shows up in the text, whichever it is + if hasExtTrackFormat && !isNumericStyle && refNumeric != nil { + // If numeric (PR) was found, and it was BEFORE the non-numeric pattern, use that + // Allow a free-pass when non-numeric pattern wasn't found. + if found && (ref == nil || refNumeric.RefLocation.Start < ref.RefLocation.Start) { + found = foundNumeric + ref = refNumeric + } + } + if !found { + return + } + + var link *html.Node + reftext := node.Data[ref.RefLocation.Start:ref.RefLocation.End] + if hasExtTrackFormat && !ref.IsPull { + ctx.Metas["index"] = ref.Issue + + res, err := vars.Expand(ctx.Metas["format"], ctx.Metas) + if err != nil { + // here we could just log the error and continue the rendering + log.Error("unable to expand template vars for ref %s, err: %v", ref.Issue, err) + } + + link = createLink(res, reftext, "ref-issue ref-external-issue") + } else { + // Path determines the type of link that will be rendered. It's unknown at this point whether + // the linked item is actually a PR or an issue. Luckily it's of no real consequence because + // Gitea will redirect on click as appropriate. + issuePath := util.Iif(ref.IsPull, "pulls", "issues") + if ref.Owner == "" { + link = createLink(util.URLJoin(ctx.Links.Prefix(), ctx.Metas["user"], ctx.Metas["repo"], issuePath, ref.Issue), reftext, "ref-issue") + } else { + link = createLink(util.URLJoin(ctx.Links.Prefix(), ref.Owner, ref.Name, issuePath, ref.Issue), reftext, "ref-issue") + } + } + + if ref.Action == references.XRefActionNone { + replaceContent(node, ref.RefLocation.Start, ref.RefLocation.End, link) + node = node.NextSibling.NextSibling + continue + } + + // Decorate action keywords if actionable + var keyword *html.Node + if references.IsXrefActionable(ref, hasExtTrackFormat) { + keyword = createKeyword(node.Data[ref.ActionLocation.Start:ref.ActionLocation.End]) + } else { + keyword = &html.Node{ + Type: html.TextNode, + Data: node.Data[ref.ActionLocation.Start:ref.ActionLocation.End], + } + } + spaces := &html.Node{ + Type: html.TextNode, + Data: node.Data[ref.ActionLocation.End:ref.RefLocation.Start], + } + replaceContentList(node, ref.ActionLocation.Start, ref.RefLocation.End, []*html.Node{keyword, spaces, link}) + node = node.NextSibling.NextSibling.NextSibling.NextSibling + } +} + +func commitCrossReferencePatternProcessor(ctx *RenderContext, node *html.Node) { + next := node.NextSibling + + for node != nil && node != next { + found, ref := references.FindRenderizableCommitCrossReference(node.Data) + if !found { + return + } + + reftext := ref.Owner + "/" + ref.Name + "@" + base.ShortSha(ref.CommitSha) + link := createLink(util.URLJoin(ctx.Links.Prefix(), ref.Owner, ref.Name, "commit", ref.CommitSha), reftext, "commit") + + replaceContent(node, ref.RefLocation.Start, ref.RefLocation.End, link) + node = node.NextSibling.NextSibling + } +} diff --git a/modules/markup/html_link.go b/modules/markup/html_link.go index b086135348..9350634568 100644 --- a/modules/markup/html_link.go +++ b/modules/markup/html_link.go @@ -4,7 +4,16 @@ package markup import ( + "net/url" + "path" + "path/filepath" + "strings" + + "code.gitea.io/gitea/modules/markup/common" "code.gitea.io/gitea/modules/util" + + "golang.org/x/net/html" + "golang.org/x/net/html/atom" ) func ResolveLink(ctx *RenderContext, link, userContentAnchorPrefix string) (result string, resolved bool) { @@ -27,3 +36,221 @@ func ResolveLink(ctx *RenderContext, link, userContentAnchorPrefix string) (resu } return link, resolved } + +func shortLinkProcessor(ctx *RenderContext, node *html.Node) { + next := node.NextSibling + for node != nil && node != next { + m := shortLinkPattern.FindStringSubmatchIndex(node.Data) + if m == nil { + return + } + + content := node.Data[m[2]:m[3]] + tail := node.Data[m[4]:m[5]] + props := make(map[string]string) + + // MediaWiki uses [[link|text]], while GitHub uses [[text|link]] + // It makes page handling terrible, but we prefer GitHub syntax + // And fall back to MediaWiki only when it is obvious from the look + // Of text and link contents + sl := strings.Split(content, "|") + for _, v := range sl { + if equalPos := strings.IndexByte(v, '='); equalPos == -1 { + // There is no equal in this argument; this is a mandatory arg + if props["name"] == "" { + if IsFullURLString(v) { + // If we clearly see it is a link, we save it so + + // But first we need to ensure, that if both mandatory args provided + // look like links, we stick to GitHub syntax + if props["link"] != "" { + props["name"] = props["link"] + } + + props["link"] = strings.TrimSpace(v) + } else { + props["name"] = v + } + } else { + props["link"] = strings.TrimSpace(v) + } + } else { + // There is an equal; optional argument. + + sep := strings.IndexByte(v, '=') + key, val := v[:sep], html.UnescapeString(v[sep+1:]) + + // When parsing HTML, x/net/html will change all quotes which are + // not used for syntax into UTF-8 quotes. So checking val[0] won't + // be enough, since that only checks a single byte. + if len(val) > 1 { + if (strings.HasPrefix(val, "“") && strings.HasSuffix(val, "”")) || + (strings.HasPrefix(val, "‘") && strings.HasSuffix(val, "’")) { + const lenQuote = len("‘") + val = val[lenQuote : len(val)-lenQuote] + } else if (strings.HasPrefix(val, "\"") && strings.HasSuffix(val, "\"")) || + (strings.HasPrefix(val, "'") && strings.HasSuffix(val, "'")) { + val = val[1 : len(val)-1] + } else if strings.HasPrefix(val, "'") && strings.HasSuffix(val, "’") { + const lenQuote = len("‘") + val = val[1 : len(val)-lenQuote] + } + } + props[key] = val + } + } + + var name, link string + if props["link"] != "" { + link = props["link"] + } else if props["name"] != "" { + link = props["name"] + } + if props["title"] != "" { + name = props["title"] + } else if props["name"] != "" { + name = props["name"] + } else { + name = link + } + + name += tail + image := false + ext := filepath.Ext(link) + switch ext { + // fast path: empty string, ignore + case "": + // leave image as false + case ".jpg", ".jpeg", ".png", ".tif", ".tiff", ".webp", ".gif", ".bmp", ".ico", ".svg": + image = true + } + + childNode := &html.Node{} + linkNode := &html.Node{ + FirstChild: childNode, + LastChild: childNode, + Type: html.ElementNode, + Data: "a", + DataAtom: atom.A, + } + childNode.Parent = linkNode + absoluteLink := IsFullURLString(link) + if !absoluteLink { + if image { + link = strings.ReplaceAll(link, " ", "+") + } else { + link = strings.ReplaceAll(link, " ", "-") // FIXME: it should support dashes in the link, eg: "the-dash-support.-" + } + if !strings.Contains(link, "/") { + link = url.PathEscape(link) // FIXME: it doesn't seem right and it might cause double-escaping + } + } + if image { + if !absoluteLink { + link = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.IsWiki), link) + } + title := props["title"] + if title == "" { + title = props["alt"] + } + if title == "" { + title = path.Base(name) + } + alt := props["alt"] + if alt == "" { + alt = name + } + + // make the childNode an image - if we can, we also place the alt + childNode.Type = html.ElementNode + childNode.Data = "img" + childNode.DataAtom = atom.Img + childNode.Attr = []html.Attribute{ + {Key: "src", Val: link}, + {Key: "title", Val: title}, + {Key: "alt", Val: alt}, + } + if alt == "" { + childNode.Attr = childNode.Attr[:2] + } + } else { + link, _ = ResolveLink(ctx, link, "") + childNode.Type = html.TextNode + childNode.Data = name + } + linkNode.Attr = []html.Attribute{{Key: "href", Val: link}} + replaceContent(node, m[0], m[1], linkNode) + node = node.NextSibling.NextSibling + } +} + +// linkProcessor creates links for any HTTP or HTTPS URL not captured by +// markdown. +func linkProcessor(ctx *RenderContext, node *html.Node) { + next := node.NextSibling + for node != nil && node != next { + m := common.LinkRegex.FindStringIndex(node.Data) + if m == nil { + return + } + + uri := node.Data[m[0]:m[1]] + replaceContent(node, m[0], m[1], createLink(uri, uri, "link")) + node = node.NextSibling.NextSibling + } +} + +func genDefaultLinkProcessor(defaultLink string) processor { + return func(ctx *RenderContext, node *html.Node) { + ch := &html.Node{ + Parent: node, + Type: html.TextNode, + Data: node.Data, + } + + node.Type = html.ElementNode + node.Data = "a" + node.DataAtom = atom.A + node.Attr = []html.Attribute{ + {Key: "href", Val: defaultLink}, + {Key: "class", Val: "default-link muted"}, + } + node.FirstChild, node.LastChild = ch, ch + } +} + +// descriptionLinkProcessor creates links for DescriptionHTML +func descriptionLinkProcessor(ctx *RenderContext, node *html.Node) { + next := node.NextSibling + for node != nil && node != next { + m := common.LinkRegex.FindStringIndex(node.Data) + if m == nil { + return + } + + uri := node.Data[m[0]:m[1]] + replaceContent(node, m[0], m[1], createDescriptionLink(uri, uri)) + node = node.NextSibling.NextSibling + } +} + +func createDescriptionLink(href, content string) *html.Node { + textNode := &html.Node{ + Type: html.TextNode, + Data: content, + } + linkNode := &html.Node{ + FirstChild: textNode, + LastChild: textNode, + Type: html.ElementNode, + Data: "a", + DataAtom: atom.A, + Attr: []html.Attribute{ + {Key: "href", Val: href}, + {Key: "target", Val: "_blank"}, + {Key: "rel", Val: "noopener noreferrer"}, + }, + } + textNode.Parent = linkNode + return linkNode +} diff --git a/modules/markup/html_mention.go b/modules/markup/html_mention.go new file mode 100644 index 0000000000..3f0692e05f --- /dev/null +++ b/modules/markup/html_mention.go @@ -0,0 +1,54 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package markup + +import ( + "strings" + + "code.gitea.io/gitea/modules/references" + "code.gitea.io/gitea/modules/util" + + "golang.org/x/net/html" +) + +func mentionProcessor(ctx *RenderContext, node *html.Node) { + start := 0 + nodeStop := node.NextSibling + for node != nodeStop { + found, loc := references.FindFirstMentionBytes(util.UnsafeStringToBytes(node.Data[start:])) + if !found { + node = node.NextSibling + start = 0 + continue + } + loc.Start += start + loc.End += start + mention := node.Data[loc.Start:loc.End] + teams, ok := ctx.Metas["teams"] + // FIXME: util.URLJoin may not be necessary here: + // - setting.AppURL is defined to have a terminal '/' so unless mention[1:] + // is an AppSubURL link we can probably fallback to concatenation. + // team mention should follow @orgName/teamName style + if ok && strings.Contains(mention, "/") { + mentionOrgAndTeam := strings.Split(mention, "/") + if mentionOrgAndTeam[0][1:] == ctx.Metas["org"] && strings.Contains(teams, ","+strings.ToLower(mentionOrgAndTeam[1])+",") { + replaceContent(node, loc.Start, loc.End, createLink(util.URLJoin(ctx.Links.Prefix(), "org", ctx.Metas["org"], "teams", mentionOrgAndTeam[1]), mention, "mention")) + node = node.NextSibling.NextSibling + start = 0 + continue + } + start = loc.End + continue + } + mentionedUsername := mention[1:] + + if DefaultProcessorHelper.IsUsernameMentionable != nil && DefaultProcessorHelper.IsUsernameMentionable(ctx.Ctx, mentionedUsername) { + replaceContent(node, loc.Start, loc.End, createLink(util.URLJoin(ctx.Links.Prefix(), mentionedUsername), mention, "mention")) + node = node.NextSibling.NextSibling + start = 0 + } else { + start = loc.End + } + } +} diff --git a/modules/markup/markdown/goldmark.go b/modules/markup/markdown/goldmark.go index 515a79578d..0cd9dc5f30 100644 --- a/modules/markup/markdown/goldmark.go +++ b/modules/markup/markdown/goldmark.go @@ -45,7 +45,7 @@ func (g *ASTTransformer) Transform(node *ast.Document, reader text.Reader, pc pa ctx := pc.Get(renderContextKey).(*markup.RenderContext) rc := pc.Get(renderConfigKey).(*RenderConfig) - tocList := make([]markup.Header, 0, 20) + tocList := make([]Header, 0, 20) if rc.yamlNode != nil { metaNode := rc.toMetaNode() if metaNode != nil { diff --git a/modules/markup/markdown/toc.go b/modules/markup/markdown/toc.go index 38f744a25f..ea1af83a3e 100644 --- a/modules/markup/markdown/toc.go +++ b/modules/markup/markdown/toc.go @@ -7,13 +7,19 @@ import ( "fmt" "net/url" - "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/translation" "github.com/yuin/goldmark/ast" ) -func createTOCNode(toc []markup.Header, lang string, detailsAttrs map[string]string) ast.Node { +// Header holds the data about a header. +type Header struct { + Level int + Text string + ID string +} + +func createTOCNode(toc []Header, lang string, detailsAttrs map[string]string) ast.Node { details := NewDetails() summary := NewSummary() diff --git a/modules/markup/markdown/transform_heading.go b/modules/markup/markdown/transform_heading.go index b78720e16d..5f8a12794d 100644 --- a/modules/markup/markdown/transform_heading.go +++ b/modules/markup/markdown/transform_heading.go @@ -13,14 +13,14 @@ import ( "github.com/yuin/goldmark/text" ) -func (g *ASTTransformer) transformHeading(_ *markup.RenderContext, v *ast.Heading, reader text.Reader, tocList *[]markup.Header) { +func (g *ASTTransformer) transformHeading(_ *markup.RenderContext, v *ast.Heading, reader text.Reader, tocList *[]Header) { for _, attr := range v.Attributes() { if _, ok := attr.Value.([]byte); !ok { v.SetAttribute(attr.Name, []byte(fmt.Sprintf("%v", attr.Value))) } } txt := v.Text(reader.Source()) //nolint:staticcheck - header := markup.Header{ + header := Header{ Text: util.UnsafeBytesToString(txt), Level: v.Level, } diff --git a/modules/markup/render.go b/modules/markup/render.go new file mode 100644 index 0000000000..f2ce9229af --- /dev/null +++ b/modules/markup/render.go @@ -0,0 +1,226 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package markup + +import ( + "context" + "errors" + "fmt" + "io" + "net/url" + "path/filepath" + "strings" + "sync" + + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/gitrepo" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" + + "github.com/yuin/goldmark/ast" +) + +type RenderMetaMode string + +const ( + RenderMetaAsDetails RenderMetaMode = "details" // default + RenderMetaAsNone RenderMetaMode = "none" + RenderMetaAsTable RenderMetaMode = "table" +) + +// RenderContext represents a render context +type RenderContext struct { + Ctx context.Context + RelativePath string // relative path from tree root of the branch + Type string + IsWiki bool + Links Links + Metas map[string]string // user, repo, mode(comment/document) + DefaultLink string + GitRepo *git.Repository + Repo gitrepo.Repository + ShaExistCache map[string]bool + cancelFn func() + SidebarTocNode ast.Node + RenderMetaAs RenderMetaMode + InStandalonePage bool // used by external render. the router "/org/repo/render/..." will output the rendered content in a standalone page +} + +// Cancel runs any cleanup functions that have been registered for this Ctx +func (ctx *RenderContext) Cancel() { + if ctx == nil { + return + } + ctx.ShaExistCache = map[string]bool{} + if ctx.cancelFn == nil { + return + } + ctx.cancelFn() +} + +// AddCancel adds the provided fn as a Cleanup for this Ctx +func (ctx *RenderContext) AddCancel(fn func()) { + if ctx == nil { + return + } + oldCancelFn := ctx.cancelFn + if oldCancelFn == nil { + ctx.cancelFn = fn + return + } + ctx.cancelFn = func() { + defer oldCancelFn() + fn() + } +} + +// Render renders markup file to HTML with all specific handling stuff. +func Render(ctx *RenderContext, input io.Reader, output io.Writer) error { + if ctx.Type != "" { + return renderByType(ctx, input, output) + } else if ctx.RelativePath != "" { + return renderFile(ctx, input, output) + } + return errors.New("render options both filename and type missing") +} + +// RenderString renders Markup string to HTML with all specific handling stuff and return string +func RenderString(ctx *RenderContext, content string) (string, error) { + var buf strings.Builder + if err := Render(ctx, strings.NewReader(content), &buf); err != nil { + return "", err + } + return buf.String(), nil +} + +func renderIFrame(ctx *RenderContext, output io.Writer) error { + // set height="0" ahead, otherwise the scrollHeight would be max(150, realHeight) + // at the moment, only "allow-scripts" is allowed for sandbox mode. + // "allow-same-origin" should never be used, it leads to XSS attack, and it makes the JS in iframe can access parent window's config and CSRF token + // TODO: when using dark theme, if the rendered content doesn't have proper style, the default text color is black, which is not easy to read + _, err := io.WriteString(output, fmt.Sprintf(` +`, + setting.AppSubURL, + url.PathEscape(ctx.Metas["user"]), + url.PathEscape(ctx.Metas["repo"]), + ctx.Metas["BranchNameSubURL"], + url.PathEscape(ctx.RelativePath), + )) + return err +} + +func render(ctx *RenderContext, renderer Renderer, input io.Reader, output io.Writer) error { + var wg sync.WaitGroup + var err error + pr, pw := io.Pipe() + defer func() { + _ = pr.Close() + _ = pw.Close() + }() + + var pr2 io.ReadCloser + var pw2 io.WriteCloser + + var sanitizerDisabled bool + if r, ok := renderer.(ExternalRenderer); ok { + sanitizerDisabled = r.SanitizerDisabled() + } + + if !sanitizerDisabled { + pr2, pw2 = io.Pipe() + defer func() { + _ = pr2.Close() + _ = pw2.Close() + }() + + wg.Add(1) + go func() { + err = SanitizeReader(pr2, renderer.Name(), output) + _ = pr2.Close() + wg.Done() + }() + } else { + pw2 = util.NopCloser{Writer: output} + } + + wg.Add(1) + go func() { + if r, ok := renderer.(PostProcessRenderer); ok && r.NeedPostProcess() { + err = PostProcess(ctx, pr, pw2) + } else { + _, err = io.Copy(pw2, pr) + } + _ = pr.Close() + _ = pw2.Close() + wg.Done() + }() + + if err1 := renderer.Render(ctx, input, pw); err1 != nil { + return err1 + } + _ = pw.Close() + + wg.Wait() + return err +} + +func renderByType(ctx *RenderContext, input io.Reader, output io.Writer) error { + if renderer, ok := renderers[ctx.Type]; ok { + return render(ctx, renderer, input, output) + } + return fmt.Errorf("unsupported render type: %s", ctx.Type) +} + +// ErrUnsupportedRenderExtension represents the error when extension doesn't supported to render +type ErrUnsupportedRenderExtension struct { + Extension string +} + +func IsErrUnsupportedRenderExtension(err error) bool { + _, ok := err.(ErrUnsupportedRenderExtension) + return ok +} + +func (err ErrUnsupportedRenderExtension) Error() string { + return fmt.Sprintf("Unsupported render extension: %s", err.Extension) +} + +func renderFile(ctx *RenderContext, input io.Reader, output io.Writer) error { + extension := strings.ToLower(filepath.Ext(ctx.RelativePath)) + if renderer, ok := extRenderers[extension]; ok { + if r, ok := renderer.(ExternalRenderer); ok && r.DisplayInIFrame() { + if !ctx.InStandalonePage { + // for an external render, it could only output its content in a standalone page + // otherwise, a `, - setting.AppSubURL, - url.PathEscape(ctx.Metas["user"]), - url.PathEscape(ctx.Metas["repo"]), - ctx.Metas["BranchNameSubURL"], - url.PathEscape(ctx.RelativePath), - )) - return err -} - -func render(ctx *RenderContext, renderer Renderer, input io.Reader, output io.Writer) error { - var wg sync.WaitGroup - var err error - pr, pw := io.Pipe() - defer func() { - _ = pr.Close() - _ = pw.Close() - }() - - var pr2 io.ReadCloser - var pw2 io.WriteCloser - - var sanitizerDisabled bool - if r, ok := renderer.(ExternalRenderer); ok { - sanitizerDisabled = r.SanitizerDisabled() - } - - if !sanitizerDisabled { - pr2, pw2 = io.Pipe() - defer func() { - _ = pr2.Close() - _ = pw2.Close() - }() - - wg.Add(1) - go func() { - err = SanitizeReader(pr2, renderer.Name(), output) - _ = pr2.Close() - wg.Done() - }() - } else { - pw2 = nopCloser{output} - } - - wg.Add(1) - go func() { - if r, ok := renderer.(PostProcessRenderer); ok && r.NeedPostProcess() { - err = PostProcess(ctx, pr, pw2) - } else { - _, err = io.Copy(pw2, pr) - } - _ = pr.Close() - _ = pw2.Close() - wg.Done() - }() - - if err1 := renderer.Render(ctx, input, pw); err1 != nil { - return err1 - } - _ = pw.Close() - - wg.Wait() - return err -} - -// ErrUnsupportedRenderType represents -type ErrUnsupportedRenderType struct { - Type string -} - -func (err ErrUnsupportedRenderType) Error() string { - return fmt.Sprintf("Unsupported render type: %s", err.Type) -} - -func renderByType(ctx *RenderContext, input io.Reader, output io.Writer) error { - if renderer, ok := renderers[ctx.Type]; ok { - return render(ctx, renderer, input, output) - } - return ErrUnsupportedRenderType{ctx.Type} -} - -// ErrUnsupportedRenderExtension represents the error when extension doesn't supported to render -type ErrUnsupportedRenderExtension struct { - Extension string -} - -func IsErrUnsupportedRenderExtension(err error) bool { - _, ok := err.(ErrUnsupportedRenderExtension) - return ok -} - -func (err ErrUnsupportedRenderExtension) Error() string { - return fmt.Sprintf("Unsupported render extension: %s", err.Extension) -} - -func renderFile(ctx *RenderContext, input io.Reader, output io.Writer) error { - extension := strings.ToLower(filepath.Ext(ctx.RelativePath)) - if renderer, ok := extRenderers[extension]; ok { - if r, ok := renderer.(ExternalRenderer); ok && r.DisplayInIFrame() { - if !ctx.InStandalonePage { - // for an external render, it could only output its content in a standalone page - // otherwise, a `, setting.AppSubURL, - url.PathEscape(ctx.Metas["user"]), - url.PathEscape(ctx.Metas["repo"]), - ctx.Metas["BranchNameSubURL"], - url.PathEscape(ctx.RelativePath), + url.PathEscape(ctx.RenderOptions.Metas["user"]), + url.PathEscape(ctx.RenderOptions.Metas["repo"]), + ctx.RenderOptions.Metas["BranchNameSubURL"], + url.PathEscape(ctx.RenderOptions.RelativePath), )) return err } @@ -176,7 +245,7 @@ func render(ctx *RenderContext, renderer Renderer, input io.Reader, output io.Wr pr1, pw1, close1 := pipes() defer close1() - eg, _ := errgroup.WithContext(ctx.Ctx) + eg, _ := errgroup.WithContext(ctx) var pw2 io.WriteCloser = util.NopCloser{Writer: finalProcessor} if r, ok := renderer.(ExternalRenderer); !ok || !r.SanitizerDisabled() { @@ -230,3 +299,27 @@ func Init(ph *ProcessorHelper) { func ComposeSimpleDocumentMetas() map[string]string { return map[string]string{"markdownLineBreakStyle": "document"} } + +// NewTestRenderContext is a helper function to create a RenderContext for testing purpose +// It accepts string (RelativePath), Links, map[string]string (Metas), gitrepo.Repository +func NewTestRenderContext(a ...any) *RenderContext { + if !setting.IsInTesting { + panic("NewTestRenderContext should only be used in testing") + } + ctx := NewRenderContext(context.Background()) + for _, v := range a { + switch v := v.(type) { + case string: + ctx = ctx.WithRelativePath(v) + case Links: + ctx = ctx.WithLinks(v) + case map[string]string: + ctx = ctx.WithMetas(v) + case gitrepo.Repository: + ctx = ctx.WithRepoFacade(v) + default: + panic(fmt.Sprintf("unknown type %T", v)) + } + } + return ctx +} diff --git a/modules/templates/util_render.go b/modules/templates/util_render.go index 5776eefced..3237de5ecb 100644 --- a/modules/templates/util_render.go +++ b/modules/templates/util_render.go @@ -38,10 +38,7 @@ func (ut *RenderUtils) RenderCommitMessage(msg string, metas map[string]string) cleanMsg := template.HTMLEscapeString(msg) // we can safely assume that it will not return any error, since there // shouldn't be any special HTML. - fullMessage, err := markup.RenderCommitMessage(&markup.RenderContext{ - Ctx: ut.ctx, - Metas: metas, - }, cleanMsg) + fullMessage, err := markup.RenderCommitMessage(markup.NewRenderContext(ut.ctx).WithMetas(metas), cleanMsg) if err != nil { log.Error("RenderCommitMessage: %v", err) return "" @@ -68,10 +65,7 @@ func (ut *RenderUtils) RenderCommitMessageLinkSubject(msg, urlDefault string, me // we can safely assume that it will not return any error, since there // shouldn't be any special HTML. - renderedMessage, err := markup.RenderCommitMessageSubject(&markup.RenderContext{ - Ctx: ut.ctx, - Metas: metas, - }, urlDefault, template.HTMLEscapeString(msgLine)) + renderedMessage, err := markup.RenderCommitMessageSubject(markup.NewRenderContext(ut.ctx).WithMetas(metas), urlDefault, template.HTMLEscapeString(msgLine)) if err != nil { log.Error("RenderCommitMessageSubject: %v", err) return "" @@ -93,10 +87,7 @@ func (ut *RenderUtils) RenderCommitBody(msg string, metas map[string]string) tem return "" } - renderedMessage, err := markup.RenderCommitMessage(&markup.RenderContext{ - Ctx: ut.ctx, - Metas: metas, - }, template.HTMLEscapeString(msgLine)) + renderedMessage, err := markup.RenderCommitMessage(markup.NewRenderContext(ut.ctx).WithMetas(metas), template.HTMLEscapeString(msgLine)) if err != nil { log.Error("RenderCommitMessage: %v", err) return "" @@ -115,10 +106,7 @@ func renderCodeBlock(htmlEscapedTextToRender template.HTML) template.HTML { // RenderIssueTitle renders issue/pull title with defined post processors func (ut *RenderUtils) RenderIssueTitle(text string, metas map[string]string) template.HTML { - renderedText, err := markup.RenderIssueTitle(&markup.RenderContext{ - Ctx: ut.ctx, - Metas: metas, - }, template.HTMLEscapeString(text)) + renderedText, err := markup.RenderIssueTitle(markup.NewRenderContext(ut.ctx).WithMetas(metas), template.HTMLEscapeString(text)) if err != nil { log.Error("RenderIssueTitle: %v", err) return "" @@ -186,7 +174,7 @@ func (ut *RenderUtils) RenderLabel(label *issues_model.Label) template.HTML { // RenderEmoji renders html text with emoji post processors func (ut *RenderUtils) RenderEmoji(text string) template.HTML { - renderedText, err := markup.RenderEmoji(&markup.RenderContext{Ctx: ut.ctx}, template.HTMLEscapeString(text)) + renderedText, err := markup.RenderEmoji(markup.NewRenderContext(ut.ctx), template.HTMLEscapeString(text)) if err != nil { log.Error("RenderEmoji: %v", err) return "" @@ -208,10 +196,7 @@ func reactionToEmoji(reaction string) template.HTML { } func (ut *RenderUtils) MarkdownToHtml(input string) template.HTML { //nolint:revive - output, err := markdown.RenderString(&markup.RenderContext{ - Ctx: ut.ctx, - Metas: markup.ComposeSimpleDocumentMetas(), - }, input) + output, err := markdown.RenderString(markup.NewRenderContext(ut.ctx).WithMetas(markup.ComposeSimpleDocumentMetas()), input) if err != nil { log.Error("RenderString: %v", err) } diff --git a/routers/api/v1/misc/markup.go b/routers/api/v1/misc/markup.go index 868ed92519..7b3633552f 100644 --- a/routers/api/v1/misc/markup.go +++ b/routers/api/v1/misc/markup.go @@ -99,9 +99,7 @@ func MarkdownRaw(ctx *context.APIContext) { // "422": // "$ref": "#/responses/validationError" defer ctx.Req.Body.Close() - if err := markdown.RenderRaw(&markup.RenderContext{ - Ctx: ctx, - }, ctx.Req.Body, ctx.Resp); err != nil { + if err := markdown.RenderRaw(markup.NewRenderContext(ctx), ctx.Req.Body, ctx.Resp); err != nil { ctx.InternalServerError(err) return } diff --git a/routers/common/markup.go b/routers/common/markup.go index dd6b286109..59f338c2bc 100644 --- a/routers/common/markup.go +++ b/routers/common/markup.go @@ -28,13 +28,12 @@ func RenderMarkup(ctx *context.Base, repo *context.Repository, mode, text, urlPa // for example, when previewing file "/gitea/owner/repo/src/branch/features/feat-123/doc/CHANGE.md", then filePath is "doc/CHANGE.md" // and the urlPathContext is "/gitea/owner/repo/src/branch/features/feat-123/doc" - renderCtx := &markup.RenderContext{ - Ctx: ctx, - Links: markup.Links{AbsolutePrefix: true}, - MarkupType: markdown.MarkupName, - } + renderCtx := markup.NewRenderContext(ctx). + WithLinks(markup.Links{AbsolutePrefix: true}). + WithMarkupType(markdown.MarkupName) + if urlPathContext != "" { - renderCtx.Links.Base = fmt.Sprintf("%s%s", httplib.GuessCurrentHostURL(ctx), urlPathContext) + renderCtx.RenderOptions.Links.Base = fmt.Sprintf("%s%s", httplib.GuessCurrentHostURL(ctx), urlPathContext) } if mode == "" || mode == "markdown" { @@ -47,15 +46,14 @@ func RenderMarkup(ctx *context.Base, repo *context.Repository, mode, text, urlPa switch mode { case "gfm": // legacy mode, do nothing case "comment": - renderCtx.Metas = map[string]string{"markdownLineBreakStyle": "comment"} + renderCtx = renderCtx.WithMetas(map[string]string{"markdownLineBreakStyle": "comment"}) case "wiki": - renderCtx.Metas = map[string]string{"markdownLineBreakStyle": "document", "markupContentMode": "wiki"} + renderCtx = renderCtx.WithMetas(map[string]string{"markdownLineBreakStyle": "document", "markupContentMode": "wiki"}) case "file": // render the repo file content by its extension - renderCtx.Metas = map[string]string{"markdownLineBreakStyle": "document"} - renderCtx.MarkupType = "" - renderCtx.RelativePath = filePath - renderCtx.InStandalonePage = true + renderCtx = renderCtx.WithMetas(map[string]string{"markdownLineBreakStyle": "document"}). + WithMarkupType(""). + WithRelativePath(filePath) default: ctx.Error(http.StatusUnprocessableEntity, fmt.Sprintf("Unknown mode: %s", mode)) return @@ -70,17 +68,17 @@ func RenderMarkup(ctx *context.Base, repo *context.Repository, mode, text, urlPa refPath := strings.Join(fields[3:], "/") // it is "branch/features/feat-12/doc" refPath = strings.TrimSuffix(refPath, "/"+fileDir) // now we get the correct branch path: "branch/features/feat-12" - renderCtx.Links = markup.Links{AbsolutePrefix: true, Base: absoluteBasePrefix, BranchPath: refPath, TreePath: fileDir} + renderCtx = renderCtx.WithLinks(markup.Links{AbsolutePrefix: true, Base: absoluteBasePrefix, BranchPath: refPath, TreePath: fileDir}) } if repo != nil && repo.Repository != nil { - renderCtx.Repo = repo.Repository + renderCtx = renderCtx.WithRepoFacade(repo.Repository) if mode == "file" { - renderCtx.Metas = repo.Repository.ComposeDocumentMetas(ctx) + renderCtx = renderCtx.WithMetas(repo.Repository.ComposeDocumentMetas(ctx)) } else if mode == "wiki" { - renderCtx.Metas = repo.Repository.ComposeWikiMetas(ctx) + renderCtx = renderCtx.WithMetas(repo.Repository.ComposeWikiMetas(ctx)) } else if mode == "comment" { - renderCtx.Metas = repo.Repository.ComposeMetas(ctx) + renderCtx = renderCtx.WithMetas(repo.Repository.ComposeMetas(ctx)) } } if err := markup.Render(renderCtx, strings.NewReader(text), ctx.Resp); err != nil { diff --git a/routers/web/feed/convert.go b/routers/web/feed/convert.go index afc2c343a6..fad7dfdf5e 100644 --- a/routers/web/feed/convert.go +++ b/routers/web/feed/convert.go @@ -51,16 +51,14 @@ func toReleaseLink(ctx *context.Context, act *activities_model.Action) string { // renderMarkdown creates a minimal markdown render context from an action. // If rendering fails, the original markdown text is returned func renderMarkdown(ctx *context.Context, act *activities_model.Action, content string) template.HTML { - markdownCtx := &markup.RenderContext{ - Ctx: ctx, - Links: markup.Links{ + markdownCtx := markup.NewRenderContext(ctx). + WithLinks(markup.Links{ Base: act.GetRepoLink(ctx), - }, - Metas: map[string]string{ // FIXME: not right here, it should use issue to compose the metas + }). + WithMetas(map[string]string{ // FIXME: not right here, it should use issue to compose the metas "user": act.GetRepoUserName(ctx), "repo": act.GetRepoName(ctx), - }, - } + }) markdown, err := markdown.RenderString(markdownCtx, content) if err != nil { return templates.SanitizeHTML(content) // old code did so: use SanitizeHTML to render in tmpl @@ -296,14 +294,13 @@ func releasesToFeedItems(ctx *context.Context, releases []*repo_model.Release) ( } link := &feeds.Link{Href: rel.HTMLURL()} - content, err = markdown.RenderString(&markup.RenderContext{ - Ctx: ctx, - Repo: rel.Repo, - Links: markup.Links{ + content, err = markdown.RenderString(markup.NewRenderContext(ctx). + WithRepoFacade(rel.Repo). + WithLinks(markup.Links{ Base: rel.Repo.Link(), - }, - Metas: rel.Repo.ComposeMetas(ctx), - }, rel.Note) + }). + WithMetas(rel.Repo.ComposeMetas(ctx)), + rel.Note) if err != nil { return nil, err } diff --git a/routers/web/feed/profile.go b/routers/web/feed/profile.go index 6dd2d14cc6..7c4864b45e 100644 --- a/routers/web/feed/profile.go +++ b/routers/web/feed/profile.go @@ -41,13 +41,10 @@ func showUserFeed(ctx *context.Context, formatType string) { return } - ctxUserDescription, err := markdown.RenderString(&markup.RenderContext{ - Ctx: ctx, - Links: markup.Links{ - Base: ctx.ContextUser.HTMLURL(), - }, - Metas: markup.ComposeSimpleDocumentMetas(), - }, ctx.ContextUser.Description) + ctxUserDescription, err := markdown.RenderString(markup.NewRenderContext(ctx). + WithLinks(markup.Links{Base: ctx.ContextUser.HTMLURL()}). + WithMetas(markup.ComposeSimpleDocumentMetas()), + ctx.ContextUser.Description) if err != nil { ctx.ServerError("RenderString", err) return diff --git a/routers/web/org/home.go b/routers/web/org/home.go index 18648d33cd..d0ac82b1b0 100644 --- a/routers/web/org/home.go +++ b/routers/web/org/home.go @@ -180,17 +180,16 @@ func prepareOrgProfileReadme(ctx *context.Context, viewRepositories bool) bool { if bytes, err := profileReadme.GetBlobContent(setting.UI.MaxDisplayFileSize); err != nil { log.Error("failed to GetBlobContent: %v", err) } else { - if profileContent, err := markdown.RenderString(&markup.RenderContext{ - Ctx: ctx, - GitRepo: profileGitRepo, - Links: markup.Links{ + if profileContent, err := markdown.RenderString(markup.NewRenderContext(ctx). + WithGitRepo(profileGitRepo). + WithLinks(markup.Links{ // Pass repo link to markdown render for the full link of media elements. // The profile of default branch would be shown. Base: profileDbRepo.Link(), BranchPath: path.Join("branch", util.PathEscapeSegments(profileDbRepo.DefaultBranch)), - }, - Metas: markup.ComposeSimpleDocumentMetas(), - }, bytes); err != nil { + }). + WithMetas(markup.ComposeSimpleDocumentMetas()), + bytes); err != nil { log.Error("failed to RenderString: %v", err) } else { ctx.Data["ProfileReadme"] = profileContent diff --git a/routers/web/repo/commit.go b/routers/web/repo/commit.go index d7865e18d6..87b1f9019a 100644 --- a/routers/web/repo/commit.go +++ b/routers/web/repo/commit.go @@ -392,16 +392,15 @@ func Diff(ctx *context.Context) { if err == nil { ctx.Data["NoteCommit"] = note.Commit ctx.Data["NoteAuthor"] = user_model.ValidateCommitWithEmail(ctx, note.Commit) - ctx.Data["NoteRendered"], err = markup.RenderCommitMessage(&markup.RenderContext{ - Links: markup.Links{ + ctx.Data["NoteRendered"], err = markup.RenderCommitMessage(markup.NewRenderContext(ctx). + WithLinks(markup.Links{ Base: ctx.Repo.RepoLink, BranchPath: path.Join("commit", util.PathEscapeSegments(commitID)), - }, - Metas: ctx.Repo.Repository.ComposeMetas(ctx), - GitRepo: ctx.Repo.GitRepo, - Repo: ctx.Repo.Repository, - Ctx: ctx, - }, template.HTMLEscapeString(string(charset.ToUTF8WithFallback(note.Message, charset.ConvertOpts{})))) + }). + WithMetas(ctx.Repo.Repository.ComposeMetas(ctx)). + WithGitRepo(ctx.Repo.GitRepo). + WithRepoFacade(ctx.Repo.Repository), + template.HTMLEscapeString(string(charset.ToUTF8WithFallback(note.Message, charset.ConvertOpts{})))) if err != nil { ctx.ServerError("RenderCommitMessage", err) return diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index a5fdba3fde..278974bec3 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -149,7 +149,7 @@ func setCsvCompareContext(ctx *context.Context) { return csvReader, reader, err } - baseReader, baseBlobCloser, err := csvReaderFromCommit(&markup.RenderContext{Ctx: ctx, RelativePath: diffFile.OldName}, baseBlob) + baseReader, baseBlobCloser, err := csvReaderFromCommit(markup.NewRenderContext(ctx).WithRelativePath(diffFile.OldName), baseBlob) if baseBlobCloser != nil { defer baseBlobCloser.Close() } @@ -161,7 +161,7 @@ func setCsvCompareContext(ctx *context.Context) { return CsvDiffResult{nil, "unable to load file"} } - headReader, headBlobCloser, err := csvReaderFromCommit(&markup.RenderContext{Ctx: ctx, RelativePath: diffFile.Name}, headBlob) + headReader, headBlobCloser, err := csvReaderFromCommit(markup.NewRenderContext(ctx).WithRelativePath(diffFile.Name), headBlob) if headBlobCloser != nil { defer headBlobCloser.Close() } diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index d1dbdd6bff..d52dbf3939 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -366,15 +366,12 @@ func UpdateIssueContent(ctx *context.Context) { } } - content, err := markdown.RenderString(&markup.RenderContext{ - Links: markup.Links{ - Base: ctx.FormString("context"), // FIXME: <- IS THIS SAFE ? - }, - Metas: ctx.Repo.Repository.ComposeMetas(ctx), - GitRepo: ctx.Repo.GitRepo, - Repo: ctx.Repo.Repository, - Ctx: ctx, - }, issue.Content) + content, err := markdown.RenderString(markup.NewRenderContext(ctx). + WithLinks(markup.Links{Base: ctx.FormString("context")}). + WithMetas(ctx.Repo.Repository.ComposeMetas(ctx)). + WithGitRepo(ctx.Repo.GitRepo). + WithRepoFacade(ctx.Repo.Repository), + issue.Content) if err != nil { ctx.ServerError("RenderString", err) return diff --git a/routers/web/repo/issue_comment.go b/routers/web/repo/issue_comment.go index 6f0fa938ce..33105d67ca 100644 --- a/routers/web/repo/issue_comment.go +++ b/routers/web/repo/issue_comment.go @@ -267,15 +267,12 @@ func UpdateCommentContent(ctx *context.Context) { var renderedContent template.HTML if comment.Content != "" { - renderedContent, err = markdown.RenderString(&markup.RenderContext{ - Links: markup.Links{ - Base: ctx.FormString("context"), // FIXME: <- IS THIS SAFE ? - }, - Metas: ctx.Repo.Repository.ComposeMetas(ctx), - GitRepo: ctx.Repo.GitRepo, - Repo: ctx.Repo.Repository, - Ctx: ctx, - }, comment.Content) + renderedContent, err = markdown.RenderString(markup.NewRenderContext(ctx). + WithLinks(markup.Links{Base: ctx.FormString("context")}). + WithMetas(ctx.Repo.Repository.ComposeMetas(ctx)). + WithGitRepo(ctx.Repo.GitRepo). + WithRepoFacade(ctx.Repo.Repository), + comment.Content) if err != nil { ctx.ServerError("RenderString", err) return diff --git a/routers/web/repo/issue_view.go b/routers/web/repo/issue_view.go index 284928856f..55d36cfefa 100644 --- a/routers/web/repo/issue_view.go +++ b/routers/web/repo/issue_view.go @@ -359,15 +359,12 @@ func ViewIssue(ctx *context.Context) { } } ctx.Data["IssueWatch"] = iw - issue.RenderedContent, err = markdown.RenderString(&markup.RenderContext{ - Links: markup.Links{ - Base: ctx.Repo.RepoLink, - }, - Metas: ctx.Repo.Repository.ComposeMetas(ctx), - GitRepo: ctx.Repo.GitRepo, - Repo: ctx.Repo.Repository, - Ctx: ctx, - }, issue.Content) + issue.RenderedContent, err = markdown.RenderString(markup.NewRenderContext(ctx). + WithLinks(markup.Links{Base: ctx.Repo.RepoLink}). + WithMetas(ctx.Repo.Repository.ComposeMetas(ctx)). + WithGitRepo(ctx.Repo.GitRepo). + WithRepoFacade(ctx.Repo.Repository), + issue.Content) if err != nil { ctx.ServerError("RenderString", err) return @@ -467,15 +464,14 @@ func ViewIssue(ctx *context.Context) { comment.Issue = issue if comment.Type == issues_model.CommentTypeComment || comment.Type == issues_model.CommentTypeReview { - comment.RenderedContent, err = markdown.RenderString(&markup.RenderContext{ - Links: markup.Links{ + comment.RenderedContent, err = markdown.RenderString(markup.NewRenderContext(ctx). + WithLinks(markup.Links{ Base: ctx.Repo.RepoLink, - }, - Metas: ctx.Repo.Repository.ComposeMetas(ctx), - GitRepo: ctx.Repo.GitRepo, - Repo: ctx.Repo.Repository, - Ctx: ctx, - }, comment.Content) + }). + WithMetas(ctx.Repo.Repository.ComposeMetas(ctx)). + WithGitRepo(ctx.Repo.GitRepo). + WithRepoFacade(ctx.Repo.Repository), + comment.Content) if err != nil { ctx.ServerError("RenderString", err) return @@ -550,15 +546,12 @@ func ViewIssue(ctx *context.Context) { } } } else if comment.Type.HasContentSupport() { - comment.RenderedContent, err = markdown.RenderString(&markup.RenderContext{ - Links: markup.Links{ - Base: ctx.Repo.RepoLink, - }, - Metas: ctx.Repo.Repository.ComposeMetas(ctx), - GitRepo: ctx.Repo.GitRepo, - Repo: ctx.Repo.Repository, - Ctx: ctx, - }, comment.Content) + comment.RenderedContent, err = markdown.RenderString(markup.NewRenderContext(ctx). + WithLinks(markup.Links{Base: ctx.Repo.RepoLink}). + WithMetas(ctx.Repo.Repository.ComposeMetas(ctx)). + WithGitRepo(ctx.Repo.GitRepo). + WithRepoFacade(ctx.Repo.Repository), + comment.Content) if err != nil { ctx.ServerError("RenderString", err) return diff --git a/routers/web/repo/milestone.go b/routers/web/repo/milestone.go index 5c0972188c..7361fe66bc 100644 --- a/routers/web/repo/milestone.go +++ b/routers/web/repo/milestone.go @@ -79,15 +79,12 @@ func Milestones(ctx *context.Context) { } } for _, m := range miles { - m.RenderedContent, err = markdown.RenderString(&markup.RenderContext{ - Links: markup.Links{ - Base: ctx.Repo.RepoLink, - }, - Metas: ctx.Repo.Repository.ComposeMetas(ctx), - GitRepo: ctx.Repo.GitRepo, - Repo: ctx.Repo.Repository, - Ctx: ctx, - }, m.Content) + m.RenderedContent, err = markdown.RenderString(markup.NewRenderContext(ctx). + WithLinks(markup.Links{Base: ctx.Repo.RepoLink}). + WithMetas(ctx.Repo.Repository.ComposeMetas(ctx)). + WithGitRepo(ctx.Repo.GitRepo). + WithRepoFacade(ctx.Repo.Repository), + m.Content) if err != nil { ctx.ServerError("RenderString", err) return @@ -268,15 +265,12 @@ func MilestoneIssuesAndPulls(ctx *context.Context) { return } - milestone.RenderedContent, err = markdown.RenderString(&markup.RenderContext{ - Links: markup.Links{ - Base: ctx.Repo.RepoLink, - }, - Metas: ctx.Repo.Repository.ComposeMetas(ctx), - GitRepo: ctx.Repo.GitRepo, - Repo: ctx.Repo.Repository, - Ctx: ctx, - }, milestone.Content) + milestone.RenderedContent, err = markdown.RenderString(markup.NewRenderContext(ctx). + WithLinks(markup.Links{Base: ctx.Repo.RepoLink}). + WithMetas(ctx.Repo.Repository.ComposeMetas(ctx)). + WithGitRepo(ctx.Repo.GitRepo). + WithRepoFacade(ctx.Repo.Repository), + milestone.Content) if err != nil { ctx.ServerError("RenderString", err) return diff --git a/routers/web/repo/projects.go b/routers/web/repo/projects.go index 664ea7eb76..cce13df3be 100644 --- a/routers/web/repo/projects.go +++ b/routers/web/repo/projects.go @@ -92,15 +92,12 @@ func Projects(ctx *context.Context) { } for i := range projects { - projects[i].RenderedContent, err = markdown.RenderString(&markup.RenderContext{ - Links: markup.Links{ - Base: ctx.Repo.RepoLink, - }, - Metas: ctx.Repo.Repository.ComposeMetas(ctx), - GitRepo: ctx.Repo.GitRepo, - Repo: ctx.Repo.Repository, - Ctx: ctx, - }, projects[i].Description) + projects[i].RenderedContent, err = markdown.RenderString(markup.NewRenderContext(ctx). + WithLinks(markup.Links{Base: ctx.Repo.RepoLink}). + WithMetas(ctx.Repo.Repository.ComposeMetas(ctx)). + WithGitRepo(ctx.Repo.GitRepo). + WithRepoFacade(ctx.Repo.Repository), + projects[i].Description) if err != nil { ctx.ServerError("RenderString", err) return @@ -425,15 +422,12 @@ func ViewProject(ctx *context.Context) { ctx.Data["SelectLabels"] = selectLabels ctx.Data["AssigneeID"] = assigneeID - project.RenderedContent, err = markdown.RenderString(&markup.RenderContext{ - Links: markup.Links{ - Base: ctx.Repo.RepoLink, - }, - Metas: ctx.Repo.Repository.ComposeMetas(ctx), - GitRepo: ctx.Repo.GitRepo, - Repo: ctx.Repo.Repository, - Ctx: ctx, - }, project.Description) + project.RenderedContent, err = markdown.RenderString(markup.NewRenderContext(ctx). + WithLinks(markup.Links{Base: ctx.Repo.RepoLink}). + WithMetas(ctx.Repo.Repository.ComposeMetas(ctx)). + WithGitRepo(ctx.Repo.GitRepo). + WithRepoFacade(ctx.Repo.Repository), + project.Description) if err != nil { ctx.ServerError("RenderString", err) return diff --git a/routers/web/repo/release.go b/routers/web/repo/release.go index 566a82316f..1b5305a90d 100644 --- a/routers/web/repo/release.go +++ b/routers/web/repo/release.go @@ -114,15 +114,12 @@ func getReleaseInfos(ctx *context.Context, opts *repo_model.FindReleasesOptions) cacheUsers[r.PublisherID] = r.Publisher } - r.RenderedNote, err = markdown.RenderString(&markup.RenderContext{ - Links: markup.Links{ - Base: ctx.Repo.RepoLink, - }, - Metas: ctx.Repo.Repository.ComposeMetas(ctx), - GitRepo: ctx.Repo.GitRepo, - Repo: ctx.Repo.Repository, - Ctx: ctx, - }, r.Note) + r.RenderedNote, err = markdown.RenderString(markup.NewRenderContext(ctx). + WithLinks(markup.Links{Base: ctx.Repo.RepoLink}). + WithMetas(ctx.Repo.Repository.ComposeMetas(ctx)). + WithGitRepo(ctx.Repo.GitRepo). + WithRepoFacade(ctx.Repo.Repository), + r.Note) if err != nil { return nil, err } diff --git a/routers/web/repo/render.go b/routers/web/repo/render.go index 6aba9e0ac1..c551e44f46 100644 --- a/routers/web/repo/render.go +++ b/routers/web/repo/render.go @@ -56,18 +56,17 @@ func RenderFile(ctx *context.Context) { return } - err = markup.Render(&markup.RenderContext{ - Ctx: ctx, - RelativePath: ctx.Repo.TreePath, - Links: markup.Links{ + err = markup.Render(markup.NewRenderContext(ctx). + WithRelativePath(ctx.Repo.TreePath). + WithLinks(markup.Links{ Base: ctx.Repo.RepoLink, BranchPath: ctx.Repo.BranchNameSubURL(), TreePath: path.Dir(ctx.Repo.TreePath), - }, - Metas: ctx.Repo.Repository.ComposeDocumentMetas(ctx), - GitRepo: ctx.Repo.GitRepo, - InStandalonePage: true, - }, rd, ctx.Resp) + }). + WithMetas(ctx.Repo.Repository.ComposeDocumentMetas(ctx)). + WithGitRepo(ctx.Repo.GitRepo). + WithInStandalonePage(true), + rd, ctx.Resp) if err != nil { log.Error("Failed to render file %q: %v", ctx.Repo.TreePath, err) http.Error(ctx.Resp, "Failed to render file", http.StatusInternalServerError) diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index 5d68ace29b..aacd7de6b1 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -310,18 +310,17 @@ func renderReadmeFile(ctx *context.Context, subfolder string, readmeFile *git.Tr ctx.Data["IsMarkup"] = true ctx.Data["MarkupType"] = markupType - ctx.Data["EscapeStatus"], ctx.Data["FileContent"], err = markupRender(ctx, &markup.RenderContext{ - Ctx: ctx, - MarkupType: markupType, - RelativePath: path.Join(ctx.Repo.TreePath, readmeFile.Name()), // ctx.Repo.TreePath is the directory not the Readme so we must append the Readme filename (and path). - Links: markup.Links{ + ctx.Data["EscapeStatus"], ctx.Data["FileContent"], err = markupRender(ctx, markup.NewRenderContext(ctx). + WithMarkupType(markupType). + WithRelativePath(path.Join(ctx.Repo.TreePath, readmeFile.Name())). // ctx.Repo.TreePath is the directory not the Readme so we must append the Readme filename (and path). + WithLinks(markup.Links{ Base: ctx.Repo.RepoLink, BranchPath: ctx.Repo.BranchNameSubURL(), TreePath: path.Join(ctx.Repo.TreePath, subfolder), - }, - Metas: ctx.Repo.Repository.ComposeDocumentMetas(ctx), - GitRepo: ctx.Repo.GitRepo, - }, rd) + }). + WithMetas(ctx.Repo.Repository.ComposeDocumentMetas(ctx)). + WithGitRepo(ctx.Repo.GitRepo), + rd) if err != nil { log.Error("Render failed for %s in %-v: %v Falling back to rendering source", readmeFile.Name(), ctx.Repo.Repository, err) delete(ctx.Data, "IsMarkup") @@ -514,18 +513,17 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry) { ctx.Data["MarkupType"] = markupType metas := ctx.Repo.Repository.ComposeDocumentMetas(ctx) metas["BranchNameSubURL"] = ctx.Repo.BranchNameSubURL() - ctx.Data["EscapeStatus"], ctx.Data["FileContent"], err = markupRender(ctx, &markup.RenderContext{ - Ctx: ctx, - MarkupType: markupType, - RelativePath: ctx.Repo.TreePath, - Links: markup.Links{ + ctx.Data["EscapeStatus"], ctx.Data["FileContent"], err = markupRender(ctx, markup.NewRenderContext(ctx). + WithMarkupType(markupType). + WithRelativePath(ctx.Repo.TreePath). + WithLinks(markup.Links{ Base: ctx.Repo.RepoLink, BranchPath: ctx.Repo.BranchNameSubURL(), TreePath: path.Dir(ctx.Repo.TreePath), - }, - Metas: metas, - GitRepo: ctx.Repo.GitRepo, - }, rd) + }). + WithMetas(metas). + WithGitRepo(ctx.Repo.GitRepo), + rd) if err != nil { ctx.ServerError("Render", err) return @@ -606,18 +604,17 @@ func renderFile(ctx *context.Context, entry *git.TreeEntry) { rd := io.MultiReader(bytes.NewReader(buf), dataRc) ctx.Data["IsMarkup"] = true ctx.Data["MarkupType"] = markupType - ctx.Data["EscapeStatus"], ctx.Data["FileContent"], err = markupRender(ctx, &markup.RenderContext{ - Ctx: ctx, - MarkupType: markupType, - RelativePath: ctx.Repo.TreePath, - Links: markup.Links{ + ctx.Data["EscapeStatus"], ctx.Data["FileContent"], err = markupRender(ctx, markup.NewRenderContext(ctx). + WithMarkupType(markupType). + WithRelativePath(ctx.Repo.TreePath). + WithLinks(markup.Links{ Base: ctx.Repo.RepoLink, BranchPath: ctx.Repo.BranchNameSubURL(), TreePath: path.Dir(ctx.Repo.TreePath), - }, - Metas: ctx.Repo.Repository.ComposeDocumentMetas(ctx), - GitRepo: ctx.Repo.GitRepo, - }, rd) + }). + WithMetas(ctx.Repo.Repository.ComposeDocumentMetas(ctx)). + WithGitRepo(ctx.Repo.GitRepo), + rd) if err != nil { ctx.ServerError("Render", err) return diff --git a/routers/web/repo/wiki.go b/routers/web/repo/wiki.go index 2732a67e71..eda3320ff0 100644 --- a/routers/web/repo/wiki.go +++ b/routers/web/repo/wiki.go @@ -288,13 +288,9 @@ func renderViewPage(ctx *context.Context) (*git.Repository, *git.TreeEntry) { footerContent = data } - rctx := &markup.RenderContext{ - Ctx: ctx, - Metas: ctx.Repo.Repository.ComposeWikiMetas(ctx), - Links: markup.Links{ - Base: ctx.Repo.RepoLink, - }, - } + rctx := markup.NewRenderContext(ctx). + WithMetas(ctx.Repo.Repository.ComposeWikiMetas(ctx)). + WithLinks(markup.Links{Base: ctx.Repo.RepoLink}) buf := &strings.Builder{} renderFn := func(data []byte) (escaped *charset.EscapeStatus, output string, err error) { diff --git a/routers/web/shared/user/header.go b/routers/web/shared/user/header.go index 9467b0986b..4cb0592b4b 100644 --- a/routers/web/shared/user/header.go +++ b/routers/web/shared/user/header.go @@ -49,10 +49,7 @@ func PrepareContextForProfileBigAvatar(ctx *context.Context) { } ctx.Data["OpenIDs"] = openIDs if len(ctx.ContextUser.Description) != 0 { - content, err := markdown.RenderString(&markup.RenderContext{ - Metas: markup.ComposeSimpleDocumentMetas(), - Ctx: ctx, - }, ctx.ContextUser.Description) + content, err := markdown.RenderString(markup.NewRenderContext(ctx).WithMetas(markup.ComposeSimpleDocumentMetas()), ctx.ContextUser.Description) if err != nil { ctx.ServerError("RenderString", err) return diff --git a/routers/web/user/home.go b/routers/web/user/home.go index 2b16142f6d..0bd0371f14 100644 --- a/routers/web/user/home.go +++ b/routers/web/user/home.go @@ -257,14 +257,11 @@ func Milestones(ctx *context.Context) { continue } - milestones[i].RenderedContent, err = markdown.RenderString(&markup.RenderContext{ - Links: markup.Links{ - Base: milestones[i].Repo.Link(), - }, - Metas: milestones[i].Repo.ComposeMetas(ctx), - Ctx: ctx, - Repo: milestones[i].Repo, - }, milestones[i].Content) + milestones[i].RenderedContent, err = markdown.RenderString(markup.NewRenderContext(ctx). + WithLinks(markup.Links{Base: milestones[i].Repo.Link()}). + WithMetas(milestones[i].Repo.ComposeMetas(ctx)). + WithRepoFacade(milestones[i].Repo), + milestones[i].Content) if err != nil { ctx.ServerError("RenderString", err) return diff --git a/routers/web/user/profile.go b/routers/web/user/profile.go index 4fbfc2bd17..2c9487bbc0 100644 --- a/routers/web/user/profile.go +++ b/routers/web/user/profile.go @@ -246,10 +246,9 @@ func prepareUserProfileTabData(ctx *context.Context, showPrivate bool, profileDb if bytes, err := profileReadme.GetBlobContent(setting.UI.MaxDisplayFileSize); err != nil { log.Error("failed to GetBlobContent: %v", err) } else { - if profileContent, err := markdown.RenderString(&markup.RenderContext{ - Ctx: ctx, - GitRepo: profileGitRepo, - Links: markup.Links{ + if profileContent, err := markdown.RenderString(markup.NewRenderContext(ctx). + WithGitRepo(profileGitRepo). + WithLinks(markup.Links{ // Give the repo link to the markdown render for the full link of media element. // the media link usually be like /[user]/[repoName]/media/branch/[branchName], // Eg. /Tom/.profile/media/branch/main @@ -257,8 +256,8 @@ func prepareUserProfileTabData(ctx *context.Context, showPrivate bool, profileDb // https://docs.gitea.com/usage/profile-readme Base: profileDbRepo.Link(), BranchPath: path.Join("branch", util.PathEscapeSegments(profileDbRepo.DefaultBranch)), - }, - }, bytes); err != nil { + }), + bytes); err != nil { log.Error("failed to RenderString: %v", err) } else { ctx.Data["ProfileReadme"] = profileContent diff --git a/services/context/org.go b/services/context/org.go index 132ce19a31..bf482fa754 100644 --- a/services/context/org.go +++ b/services/context/org.go @@ -259,9 +259,7 @@ func HandleOrgAssignment(ctx *Context, args ...bool) { ctx.Data["IsFollowing"] = ctx.Doer != nil && user_model.IsFollowing(ctx, ctx.Doer.ID, ctx.ContextUser.ID) if len(ctx.ContextUser.Description) != 0 { - content, err := markdown.RenderString(&markup.RenderContext{ - Ctx: ctx, - }, ctx.ContextUser.Description) + content, err := markdown.RenderString(markup.NewRenderContext(ctx), ctx.ContextUser.Description) if err != nil { ctx.ServerError("RenderString", err) return diff --git a/services/mailer/mail.go b/services/mailer/mail.go index 23c91595b7..162e497dc0 100644 --- a/services/mailer/mail.go +++ b/services/mailer/mail.go @@ -219,15 +219,11 @@ func composeIssueCommentMessages(ctx *mailCommentContext, lang string, recipient } // This is the body of the new issue or comment, not the mail body - body, err := markdown.RenderString(&markup.RenderContext{ - Ctx: ctx, - Repo: ctx.Issue.Repo, - Links: markup.Links{ - AbsolutePrefix: true, - Base: ctx.Issue.Repo.HTMLURL(), - }, - Metas: ctx.Issue.Repo.ComposeMetas(ctx), - }, ctx.Content) + body, err := markdown.RenderString(markup.NewRenderContext(ctx). + WithRepoFacade(ctx.Issue.Repo). + WithLinks(markup.Links{AbsolutePrefix: true, Base: ctx.Issue.Repo.HTMLURL()}). + WithMetas(ctx.Issue.Repo.ComposeMetas(ctx)), + ctx.Content) if err != nil { return nil, err } diff --git a/services/mailer/mail_release.go b/services/mailer/mail_release.go index 01a8929e2d..3298c2273a 100644 --- a/services/mailer/mail_release.go +++ b/services/mailer/mail_release.go @@ -56,14 +56,11 @@ func mailNewRelease(ctx context.Context, lang string, tos []*user_model.User, re locale := translation.NewLocale(lang) var err error - rel.RenderedNote, err = markdown.RenderString(&markup.RenderContext{ - Ctx: ctx, - Repo: rel.Repo, - Links: markup.Links{ - Base: rel.Repo.HTMLURL(), - }, - Metas: rel.Repo.ComposeMetas(ctx), - }, rel.Note) + rel.RenderedNote, err = markdown.RenderString(markup.NewRenderContext(ctx). + WithRepoFacade(rel.Repo). + WithLinks(markup.Links{Base: rel.Repo.HTMLURL()}). + WithMetas(rel.Repo.ComposeMetas(ctx)), + rel.Note) if err != nil { log.Error("markdown.RenderString(%d): %v", rel.RepoID, err) return diff --git a/tests/fuzz/fuzz_test.go b/tests/fuzz/fuzz_test.go index 25a6ed8213..78d3027547 100644 --- a/tests/fuzz/fuzz_test.go +++ b/tests/fuzz/fuzz_test.go @@ -14,27 +14,22 @@ import ( "code.gitea.io/gitea/modules/setting" ) -var renderContext = markup.RenderContext{ - Ctx: context.Background(), - Links: markup.Links{ - Base: "https://example.com/go-gitea/gitea", - }, - Metas: map[string]string{ - "user": "go-gitea", - "repo": "gitea", - }, +func newFuzzRenderContext() *markup.RenderContext { + return markup.NewRenderContext(context.Background()). + WithLinks(markup.Links{Base: "https://example.com/go-gitea/gitea"}). + WithMetas(map[string]string{"user": "go-gitea", "repo": "gitea"}) } func FuzzMarkdownRenderRaw(f *testing.F) { f.Fuzz(func(t *testing.T, data []byte) { setting.AppURL = "http://localhost:3000/" - markdown.RenderRaw(&renderContext, bytes.NewReader(data), io.Discard) + markdown.RenderRaw(newFuzzRenderContext(), bytes.NewReader(data), io.Discard) }) } func FuzzMarkupPostProcess(f *testing.F) { f.Fuzz(func(t *testing.T, data []byte) { setting.AppURL = "http://localhost:3000/" - markup.PostProcess(&renderContext, bytes.NewReader(data), io.Discard) + markup.PostProcess(newFuzzRenderContext(), bytes.NewReader(data), io.Discard) }) } From bc7d599030cad2e69139b79ad5a878504fbf8ed9 Mon Sep 17 00:00:00 2001 From: Kerwin Bryant Date: Fri, 22 Nov 2024 14:12:50 +0800 Subject: [PATCH 090/156] Fix issues with inconsistent spacing in areas (#32607) Fix issues with inconsistent spacing in areas where the branch_dropdown component is used. before: ![1732238359257](https://github.com/user-attachments/assets/38edda1f-ec4e-419e-9264-68009375d177) ![1732238334410](https://github.com/user-attachments/assets/c4770aea-bc83-477c-9b6a-632f984c0d7d) after: ![1732238273317](https://github.com/user-attachments/assets/4d05068e-db97-45af-86c4-29442dff1bdf) ![1732238723881](https://github.com/user-attachments/assets/69acd286-f79b-44fe-ad73-2d5fc6dfc98c) --------- Co-authored-by: wxiaoguang --- templates/repo/commits.tmpl | 4 ++-- templates/repo/home.tmpl | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/templates/repo/commits.tmpl b/templates/repo/commits.tmpl index e6efe1ff54..6bce585774 100644 --- a/templates/repo/commits.tmpl +++ b/templates/repo/commits.tmpl @@ -4,8 +4,8 @@
{{template "repo/sub_menu" .}}
-
- {{template "repo/branch_dropdown" dict "root" . "ContainerClasses" "tw-mr-1"}} +
+ {{template "repo/branch_dropdown" dict "root" .}} {{svg "octicon-git-branch"}} {{ctx.Locale.Tr "repo.commit_graph"}} diff --git a/templates/repo/home.tmpl b/templates/repo/home.tmpl index 0a8391b553..12c4a17234 100644 --- a/templates/repo/home.tmpl +++ b/templates/repo/home.tmpl @@ -47,7 +47,7 @@ {{$isHomepage := (eq $n 0)}}
- {{template "repo/branch_dropdown" dict "root" . "ContainerClasses" "tw-mr-1"}} + {{template "repo/branch_dropdown" dict "root" .}} {{if and .CanCompareOrPull .IsViewBranch (not .Repository.IsArchived)}} {{$cmpBranch := ""}} {{if ne .Repository.ID .BaseRepo.ID}} From fe49cb0243bed03f565269d84836bf21a0597665 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 22 Nov 2024 07:44:48 -0800 Subject: [PATCH 091/156] Fix get reviewers' bug (#32415) This PR rewrites `GetReviewer` function and move it to service layer. Reviewers should not be watchers, so that this PR removed all watchers from reviewers. When the repository is under an organization, the pull request unit read permission will be checked to resolve the bug of #32394 Fix #32394 --- models/organization/team_repo.go | 14 +++++ models/organization/team_repo_test.go | 31 ++++++++++ models/repo/user_repo.go | 52 ---------------- models/repo/user_repo_test.go | 43 ------------- routers/api/v1/repo/collaborators.go | 10 ++- routers/web/repo/issue_page_meta.go | 8 +-- services/issue/assignee.go | 8 +-- services/pull/reviewer.go | 89 +++++++++++++++++++++++++++ services/pull/reviewer_test.go | 72 ++++++++++++++++++++++ services/repository/review.go | 24 -------- services/repository/review_test.go | 28 --------- tests/integration/api_repo_test.go | 4 +- 12 files changed, 225 insertions(+), 158 deletions(-) create mode 100644 models/organization/team_repo_test.go create mode 100644 services/pull/reviewer.go create mode 100644 services/pull/reviewer_test.go delete mode 100644 services/repository/review.go delete mode 100644 services/repository/review_test.go diff --git a/models/organization/team_repo.go b/models/organization/team_repo.go index 1184e39263..c90dfdeda0 100644 --- a/models/organization/team_repo.go +++ b/models/organization/team_repo.go @@ -9,6 +9,7 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/perm" repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unit" "xorm.io/builder" ) @@ -83,3 +84,16 @@ func GetTeamsWithAccessToRepo(ctx context.Context, orgID, repoID int64, mode per OrderBy("name"). Find(&teams) } + +// GetTeamsWithAccessToRepoUnit returns all teams in an organization that have given access level to the repository special unit. +func GetTeamsWithAccessToRepoUnit(ctx context.Context, orgID, repoID int64, mode perm.AccessMode, unitType unit.Type) ([]*Team, error) { + teams := make([]*Team, 0, 5) + return teams, db.GetEngine(ctx).Where("team_unit.access_mode >= ?", mode). + Join("INNER", "team_repo", "team_repo.team_id = team.id"). + Join("INNER", "team_unit", "team_unit.team_id = team.id"). + And("team_repo.org_id = ?", orgID). + And("team_repo.repo_id = ?", repoID). + And("team_unit.type = ?", unitType). + OrderBy("name"). + Find(&teams) +} diff --git a/models/organization/team_repo_test.go b/models/organization/team_repo_test.go new file mode 100644 index 0000000000..c0d6750df9 --- /dev/null +++ b/models/organization/team_repo_test.go @@ -0,0 +1,31 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package organization_test + +import ( + "testing" + + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/organization" + "code.gitea.io/gitea/models/perm" + "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unit" + "code.gitea.io/gitea/models/unittest" + + "github.com/stretchr/testify/assert" +) + +func TestGetTeamsWithAccessToRepoUnit(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + org41 := unittest.AssertExistsAndLoadBean(t, &organization.Organization{ID: 41}) + repo61 := unittest.AssertExistsAndLoadBean(t, &repo.Repository{ID: 61}) + + teams, err := organization.GetTeamsWithAccessToRepoUnit(db.DefaultContext, org41.ID, repo61.ID, perm.AccessModeRead, unit.TypePullRequests) + assert.NoError(t, err) + if assert.Len(t, teams, 2) { + assert.EqualValues(t, 21, teams[0].ID) + assert.EqualValues(t, 22, teams[1].ID) + } +} diff --git a/models/repo/user_repo.go b/models/repo/user_repo.go index ecc9216950..a9b1360df1 100644 --- a/models/repo/user_repo.go +++ b/models/repo/user_repo.go @@ -11,7 +11,6 @@ import ( "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/container" - api "code.gitea.io/gitea/modules/structs" "xorm.io/builder" ) @@ -146,57 +145,6 @@ func GetRepoAssignees(ctx context.Context, repo *Repository) (_ []*user_model.Us return users, nil } -// GetReviewers get all users can be requested to review: -// * for private repositories this returns all users that have read access or higher to the repository. -// * for public repositories this returns all users that have read access or higher to the repository, -// all repo watchers and all organization members. -// TODO: may be we should have a busy choice for users to block review request to them. -func GetReviewers(ctx context.Context, repo *Repository, doerID, posterID int64) ([]*user_model.User, error) { - // Get the owner of the repository - this often already pre-cached and if so saves complexity for the following queries - if err := repo.LoadOwner(ctx); err != nil { - return nil, err - } - - cond := builder.And(builder.Neq{"`user`.id": posterID}). - And(builder.Eq{"`user`.is_active": true}) - - if repo.IsPrivate || repo.Owner.Visibility == api.VisibleTypePrivate { - // This a private repository: - // Anyone who can read the repository is a requestable reviewer - - cond = cond.And(builder.In("`user`.id", - builder.Select("user_id").From("access").Where( - builder.Eq{"repo_id": repo.ID}. - And(builder.Gte{"mode": perm.AccessModeRead}), - ), - )) - - if repo.Owner.Type == user_model.UserTypeIndividual && repo.Owner.ID != posterID { - // as private *user* repos don't generate an entry in the `access` table, - // the owner of a private repo needs to be explicitly added. - cond = cond.Or(builder.Eq{"`user`.id": repo.Owner.ID}) - } - } else { - // This is a "public" repository: - // Any user that has read access, is a watcher or organization member can be requested to review - cond = cond.And(builder.And(builder.In("`user`.id", - builder.Select("user_id").From("access"). - Where(builder.Eq{"repo_id": repo.ID}. - And(builder.Gte{"mode": perm.AccessModeRead})), - ).Or(builder.In("`user`.id", - builder.Select("user_id").From("watch"). - Where(builder.Eq{"repo_id": repo.ID}. - And(builder.In("mode", WatchModeNormal, WatchModeAuto))), - ).Or(builder.In("`user`.id", - builder.Select("uid").From("org_user"). - Where(builder.Eq{"org_id": repo.OwnerID}), - ))))) - } - - users := make([]*user_model.User, 0, 8) - return users, db.GetEngine(ctx).Where(cond).OrderBy(user_model.GetOrderByName()).Find(&users) -} - // GetIssuePostersWithSearch returns users with limit of 30 whose username started with prefix that have authored an issue/pull request for the given repository // If isShowFullName is set to true, also include full name prefix search func GetIssuePostersWithSearch(ctx context.Context, repo *Repository, isPull bool, search string, isShowFullName bool) ([]*user_model.User, error) { diff --git a/models/repo/user_repo_test.go b/models/repo/user_repo_test.go index d2bf6dc912..f2abc2ffa0 100644 --- a/models/repo/user_repo_test.go +++ b/models/repo/user_repo_test.go @@ -38,46 +38,3 @@ func TestRepoAssignees(t *testing.T) { assert.NotContains(t, []int64{users[0].ID, users[1].ID, users[2].ID}, 15) } } - -func TestRepoGetReviewers(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - - // test public repo - repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) - - ctx := db.DefaultContext - reviewers, err := repo_model.GetReviewers(ctx, repo1, 2, 2) - assert.NoError(t, err) - if assert.Len(t, reviewers, 3) { - assert.ElementsMatch(t, []int64{1, 4, 11}, []int64{reviewers[0].ID, reviewers[1].ID, reviewers[2].ID}) - } - - // should include doer if doer is not PR poster. - reviewers, err = repo_model.GetReviewers(ctx, repo1, 11, 2) - assert.NoError(t, err) - assert.Len(t, reviewers, 3) - - // should not include PR poster, if PR poster would be otherwise eligible - reviewers, err = repo_model.GetReviewers(ctx, repo1, 11, 4) - assert.NoError(t, err) - assert.Len(t, reviewers, 2) - - // test private user repo - repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) - - reviewers, err = repo_model.GetReviewers(ctx, repo2, 2, 4) - assert.NoError(t, err) - assert.Len(t, reviewers, 1) - assert.EqualValues(t, reviewers[0].ID, 2) - - // test private org repo - repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) - - reviewers, err = repo_model.GetReviewers(ctx, repo3, 2, 1) - assert.NoError(t, err) - assert.Len(t, reviewers, 2) - - reviewers, err = repo_model.GetReviewers(ctx, repo3, 2, 2) - assert.NoError(t, err) - assert.Len(t, reviewers, 1) -} diff --git a/routers/api/v1/repo/collaborators.go b/routers/api/v1/repo/collaborators.go index ea9d8b0f37..0bbf5a1ea4 100644 --- a/routers/api/v1/repo/collaborators.go +++ b/routers/api/v1/repo/collaborators.go @@ -17,6 +17,8 @@ import ( "code.gitea.io/gitea/routers/api/v1/utils" "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/convert" + issue_service "code.gitea.io/gitea/services/issue" + pull_service "code.gitea.io/gitea/services/pull" repo_service "code.gitea.io/gitea/services/repository" ) @@ -320,7 +322,13 @@ func GetReviewers(ctx *context.APIContext) { // "404": // "$ref": "#/responses/notFound" - reviewers, err := repo_model.GetReviewers(ctx, ctx.Repo.Repository, ctx.Doer.ID, 0) + canChooseReviewer := issue_service.CanDoerChangeReviewRequests(ctx, ctx.Doer, ctx.Repo.Repository, 0) + if !canChooseReviewer { + ctx.Error(http.StatusForbidden, "GetReviewers", errors.New("doer has no permission to get reviewers")) + return + } + + reviewers, err := pull_service.GetReviewers(ctx, ctx.Repo.Repository, ctx.Doer.ID, 0) if err != nil { ctx.Error(http.StatusInternalServerError, "ListCollaborators", err) return diff --git a/routers/web/repo/issue_page_meta.go b/routers/web/repo/issue_page_meta.go index ac0b1c6425..e04d76b287 100644 --- a/routers/web/repo/issue_page_meta.go +++ b/routers/web/repo/issue_page_meta.go @@ -19,7 +19,7 @@ import ( shared_user "code.gitea.io/gitea/routers/web/shared/user" "code.gitea.io/gitea/services/context" issue_service "code.gitea.io/gitea/services/issue" - repo_service "code.gitea.io/gitea/services/repository" + pull_service "code.gitea.io/gitea/services/pull" ) type issueSidebarMilestoneData struct { @@ -186,7 +186,7 @@ func (d *IssuePageMetaData) retrieveReviewersData(ctx *context.Context) { if d.Issue == nil { data.CanChooseReviewer = true } else { - data.CanChooseReviewer = issue_service.CanDoerChangeReviewRequests(ctx, ctx.Doer, repo, d.Issue) + data.CanChooseReviewer = issue_service.CanDoerChangeReviewRequests(ctx, ctx.Doer, repo, d.Issue.PosterID) } } @@ -231,13 +231,13 @@ func (d *IssuePageMetaData) retrieveReviewersData(ctx *context.Context) { if data.CanChooseReviewer { var err error - reviewers, err = repo_model.GetReviewers(ctx, repo, ctx.Doer.ID, posterID) + reviewers, err = pull_service.GetReviewers(ctx, repo, ctx.Doer.ID, posterID) if err != nil { ctx.ServerError("GetReviewers", err) return } - teamReviewers, err = repo_service.GetReviewerTeams(ctx, repo) + teamReviewers, err = pull_service.GetReviewerTeams(ctx, repo) if err != nil { ctx.ServerError("GetReviewerTeams", err) return diff --git a/services/issue/assignee.go b/services/issue/assignee.go index 52ee9f2b22..c7e2495568 100644 --- a/services/issue/assignee.go +++ b/services/issue/assignee.go @@ -119,7 +119,7 @@ func isValidReviewRequest(ctx context.Context, reviewer, doer *user_model.User, return err } - canDoerChangeReviewRequests := CanDoerChangeReviewRequests(ctx, doer, issue.Repo, issue) + canDoerChangeReviewRequests := CanDoerChangeReviewRequests(ctx, doer, issue.Repo, issue.PosterID) if isAdd { if !permReviewer.CanAccessAny(perm.AccessModeRead, unit.TypePullRequests) { @@ -178,7 +178,7 @@ func isValidTeamReviewRequest(ctx context.Context, reviewer *organization.Team, } } - canDoerChangeReviewRequests := CanDoerChangeReviewRequests(ctx, doer, issue.Repo, issue) + canDoerChangeReviewRequests := CanDoerChangeReviewRequests(ctx, doer, issue.Repo, issue.PosterID) if isAdd { if issue.Repo.IsPrivate { @@ -276,12 +276,12 @@ func teamReviewRequestNotify(ctx context.Context, issue *issues_model.Issue, doe } // CanDoerChangeReviewRequests returns if the doer can add/remove review requests of a PR -func CanDoerChangeReviewRequests(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, issue *issues_model.Issue) bool { +func CanDoerChangeReviewRequests(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, posterID int64) bool { if repo.IsArchived { return false } // The poster of the PR can change the reviewers - if doer.ID == issue.PosterID { + if doer.ID == posterID { return true } diff --git a/services/pull/reviewer.go b/services/pull/reviewer.go new file mode 100644 index 0000000000..bf0d8cb298 --- /dev/null +++ b/services/pull/reviewer.go @@ -0,0 +1,89 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package pull + +import ( + "context" + + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/organization" + "code.gitea.io/gitea/models/perm" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unit" + user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/container" + + "xorm.io/builder" +) + +// GetReviewers get all users can be requested to review: +// - Poster should not be listed +// - For collaborator, all users that have read access or higher to the repository. +// - For repository under organization, users under the teams which have read permission or higher of pull request unit +// - Owner will be listed if it's not an organization, not the poster and not in the list of reviewers +func GetReviewers(ctx context.Context, repo *repo_model.Repository, doerID, posterID int64) ([]*user_model.User, error) { + if err := repo.LoadOwner(ctx); err != nil { + return nil, err + } + + e := db.GetEngine(ctx) + uniqueUserIDs := make(container.Set[int64]) + + collaboratorIDs := make([]int64, 0, 10) + if err := e.Table("collaboration").Where("repo_id=?", repo.ID). + And("mode >= ?", perm.AccessModeRead). + Select("user_id"). + Find(&collaboratorIDs); err != nil { + return nil, err + } + uniqueUserIDs.AddMultiple(collaboratorIDs...) + + if repo.Owner.IsOrganization() { + additionalUserIDs := make([]int64, 0, 10) + if err := e.Table("team_user"). + Join("INNER", "team_repo", "`team_repo`.team_id = `team_user`.team_id"). + Join("INNER", "team_unit", "`team_unit`.team_id = `team_user`.team_id"). + Where("`team_repo`.repo_id = ? AND (`team_unit`.access_mode >= ? AND `team_unit`.`type` = ?)", + repo.ID, perm.AccessModeRead, unit.TypePullRequests). + Distinct("`team_user`.uid"). + Select("`team_user`.uid"). + Find(&additionalUserIDs); err != nil { + return nil, err + } + uniqueUserIDs.AddMultiple(additionalUserIDs...) + } + + uniqueUserIDs.Remove(posterID) // posterID should not be in the list of reviewers + + // Leave a seat for owner itself to append later, but if owner is an organization + // and just waste 1 unit is cheaper than re-allocate memory once. + users := make([]*user_model.User, 0, len(uniqueUserIDs)+1) + if len(uniqueUserIDs) > 0 { + if err := e.In("id", uniqueUserIDs.Values()). + Where(builder.Eq{"`user`.is_active": true}). + OrderBy(user_model.GetOrderByName()). + Find(&users); err != nil { + return nil, err + } + } + + // add owner after all users are loaded because we can avoid load owner twice + if repo.OwnerID != posterID && !repo.Owner.IsOrganization() && !uniqueUserIDs.Contains(repo.OwnerID) { + users = append(users, repo.Owner) + } + + return users, nil +} + +// GetReviewerTeams get all teams can be requested to review +func GetReviewerTeams(ctx context.Context, repo *repo_model.Repository) ([]*organization.Team, error) { + if err := repo.LoadOwner(ctx); err != nil { + return nil, err + } + if !repo.Owner.IsOrganization() { + return nil, nil + } + + return organization.GetTeamsWithAccessToRepoUnit(ctx, repo.OwnerID, repo.ID, perm.AccessModeRead, unit.TypePullRequests) +} diff --git a/services/pull/reviewer_test.go b/services/pull/reviewer_test.go new file mode 100644 index 0000000000..1ff373bafb --- /dev/null +++ b/services/pull/reviewer_test.go @@ -0,0 +1,72 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package pull_test + +import ( + "testing" + + "code.gitea.io/gitea/models/db" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unittest" + pull_service "code.gitea.io/gitea/services/pull" + + "github.com/stretchr/testify/assert" +) + +func TestRepoGetReviewers(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + // test public repo + repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + + ctx := db.DefaultContext + reviewers, err := pull_service.GetReviewers(ctx, repo1, 2, 0) + assert.NoError(t, err) + if assert.Len(t, reviewers, 1) { + assert.ElementsMatch(t, []int64{2}, []int64{reviewers[0].ID}) + } + + // should not include doer and remove the poster + reviewers, err = pull_service.GetReviewers(ctx, repo1, 11, 2) + assert.NoError(t, err) + assert.Len(t, reviewers, 0) + + // should not include PR poster, if PR poster would be otherwise eligible + reviewers, err = pull_service.GetReviewers(ctx, repo1, 11, 4) + assert.NoError(t, err) + assert.Len(t, reviewers, 1) + + // test private user repo + repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) + + reviewers, err = pull_service.GetReviewers(ctx, repo2, 2, 4) + assert.NoError(t, err) + assert.Len(t, reviewers, 1) + assert.EqualValues(t, reviewers[0].ID, 2) + + // test private org repo + repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) + + reviewers, err = pull_service.GetReviewers(ctx, repo3, 2, 1) + assert.NoError(t, err) + assert.Len(t, reviewers, 2) + + reviewers, err = pull_service.GetReviewers(ctx, repo3, 2, 2) + assert.NoError(t, err) + assert.Len(t, reviewers, 1) +} + +func TestRepoGetReviewerTeams(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) + teams, err := pull_service.GetReviewerTeams(db.DefaultContext, repo2) + assert.NoError(t, err) + assert.Empty(t, teams) + + repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) + teams, err = pull_service.GetReviewerTeams(db.DefaultContext, repo3) + assert.NoError(t, err) + assert.Len(t, teams, 2) +} diff --git a/services/repository/review.go b/services/repository/review.go deleted file mode 100644 index 40513e6bc6..0000000000 --- a/services/repository/review.go +++ /dev/null @@ -1,24 +0,0 @@ -// Copyright 2022 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package repository - -import ( - "context" - - "code.gitea.io/gitea/models/organization" - "code.gitea.io/gitea/models/perm" - repo_model "code.gitea.io/gitea/models/repo" -) - -// GetReviewerTeams get all teams can be requested to review -func GetReviewerTeams(ctx context.Context, repo *repo_model.Repository) ([]*organization.Team, error) { - if err := repo.LoadOwner(ctx); err != nil { - return nil, err - } - if !repo.Owner.IsOrganization() { - return nil, nil - } - - return organization.GetTeamsWithAccessToRepo(ctx, repo.OwnerID, repo.ID, perm.AccessModeRead) -} diff --git a/services/repository/review_test.go b/services/repository/review_test.go deleted file mode 100644 index 2db56d4e8a..0000000000 --- a/services/repository/review_test.go +++ /dev/null @@ -1,28 +0,0 @@ -// Copyright 2022 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package repository - -import ( - "testing" - - "code.gitea.io/gitea/models/db" - repo_model "code.gitea.io/gitea/models/repo" - "code.gitea.io/gitea/models/unittest" - - "github.com/stretchr/testify/assert" -) - -func TestRepoGetReviewerTeams(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - - repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2}) - teams, err := GetReviewerTeams(db.DefaultContext, repo2) - assert.NoError(t, err) - assert.Empty(t, teams) - - repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) - teams, err = GetReviewerTeams(db.DefaultContext, repo3) - assert.NoError(t, err) - assert.Len(t, teams, 2) -} diff --git a/tests/integration/api_repo_test.go b/tests/integration/api_repo_test.go index 93c9ca0920..122afbfa08 100644 --- a/tests/integration/api_repo_test.go +++ b/tests/integration/api_repo_test.go @@ -718,8 +718,8 @@ func TestAPIRepoGetReviewers(t *testing.T) { resp := MakeRequest(t, req, http.StatusOK) var reviewers []*api.User DecodeJSON(t, resp, &reviewers) - if assert.Len(t, reviewers, 3) { - assert.ElementsMatch(t, []int64{1, 4, 11}, []int64{reviewers[0].ID, reviewers[1].ID, reviewers[2].ID}) + if assert.Len(t, reviewers, 1) { + assert.ElementsMatch(t, []int64{2}, []int64{reviewers[0].ID}) } } From ae90b21db036507b8df9ed22e7ba416139037547 Mon Sep 17 00:00:00 2001 From: hiifong Date: Sat, 23 Nov 2024 02:26:05 +0800 Subject: [PATCH 092/156] Apply to became a maintainer (#32614) [PRs list](https://github.com/go-gitea/gitea/pulls?q=is%3Apr+author%3Ahiifong+is%3Aclosed+is%3Amerged) --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 160bffcdb7..426181cbcf 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -63,3 +63,4 @@ Tim-Niclas Oelschläger (@zokkis) Yu Liu <1240335630@qq.com> (@HEREYUA) Kemal Zebari (@kemzeb) Rowan Bohde (@bohde) +hiifong (@hiifong) From f2a995174101b9fa9409acb2b47b065262098b28 Mon Sep 17 00:00:00 2001 From: Yarden Shoham Date: Fri, 22 Nov 2024 20:51:51 +0200 Subject: [PATCH 093/156] Update the list of watchers and stargazers when clicking watch/unwatch or star/unstar (#32570) We make sure the user cards are updated - Fixes https://github.com/go-gitea/gitea/issues/32561 I also removed `ctx.Data["PageIsWatchers"] = true` and `ctx.Data["PageIsStargazers"] = true` as they are not used anywhere. # Before ![before](https://github.com/user-attachments/assets/e3bc3235-35eb-4eda-862d-bdf2510282ea) # After ![after](https://github.com/user-attachments/assets/bc0488a5-8399-4cf6-95c9-17328a9702eb) --------- Signed-off-by: Yarden Shoham Co-authored-by: wxiaoguang Co-authored-by: silverwind --- routers/web/repo/repo.go | 3 +++ routers/web/repo/view.go | 3 --- templates/repo/user_cards.tmpl | 12 +++++++++++- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/routers/web/repo/repo.go b/routers/web/repo/repo.go index be6f2d483f..b62fd21585 100644 --- a/routers/web/repo/repo.go +++ b/routers/web/repo/repo.go @@ -352,6 +352,9 @@ func Action(ctx *context.Context) { ctx.Data["IsStaringRepo"] = repo_model.IsStaring(ctx, ctx.Doer.ID, ctx.Repo.Repository.ID) } + // see the `hx-trigger="refreshUserCards ..."` comments in tmpl + ctx.RespHeader().Add("hx-trigger", "refreshUserCards") + switch ctx.PathParam(":action") { case "watch", "unwatch", "star", "unstar": // we have to reload the repository because NumStars or NumWatching (used in the templates) has just changed diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go index aacd7de6b1..ec2ddfd79f 100644 --- a/routers/web/repo/view.go +++ b/routers/web/repo/view.go @@ -1123,8 +1123,6 @@ func RenderUserCards(ctx *context.Context, total int, getter func(opts db.ListOp func Watchers(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("repo.watchers") ctx.Data["CardsTitle"] = ctx.Tr("repo.watchers") - ctx.Data["PageIsWatchers"] = true - RenderUserCards(ctx, ctx.Repo.Repository.NumWatches, func(opts db.ListOptions) ([]*user_model.User, error) { return repo_model.GetRepoWatchers(ctx, ctx.Repo.Repository.ID, opts) }, tplWatchers) @@ -1134,7 +1132,6 @@ func Watchers(ctx *context.Context) { func Stars(ctx *context.Context) { ctx.Data["Title"] = ctx.Tr("repo.stargazers") ctx.Data["CardsTitle"] = ctx.Tr("repo.stargazers") - ctx.Data["PageIsStargazers"] = true RenderUserCards(ctx, ctx.Repo.Repository.NumStars, func(opts db.ListOptions) ([]*user_model.User, error) { return repo_model.GetStargazers(ctx, ctx.Repo.Repository, opts) }, tplWatchers) diff --git a/templates/repo/user_cards.tmpl b/templates/repo/user_cards.tmpl index 7cd3d4517a..360aeaf619 100644 --- a/templates/repo/user_cards.tmpl +++ b/templates/repo/user_cards.tmpl @@ -1,4 +1,14 @@ -
+ +
+
{{if .CardsTitle}}

{{.CardsTitle}} From 713364fc718d1d53840bd83ba6f6c307bd213fa8 Mon Sep 17 00:00:00 2001 From: Michael Owoc <130198442+mowoc-ocp@users.noreply.github.com> Date: Fri, 22 Nov 2024 15:12:06 -0500 Subject: [PATCH 094/156] Support optional/configurable IAMEndpoint for Minio Client (#32581) (#32581) Targeting issue #32271 This modification allows native Kubernetes + AWS (EKS) authentication with the Minio client, to Amazon S3 using the IRSA role assigned to a Service account by replacing the hard coded reference to the `DefaultIAMRoleEndpoint` with an optional configurable endpoint. Internally, Minio's `credentials.IAM` provider implements a discovery flow for IAM Endpoints if it is not set. For backwards compatibility: - We have added a configuration mechanism for an `IamEndpoint` to retain the unit test safety in `minio_test.go`. - We believe existing clients will continue to function the same without needing to provide a new config property since the internals of Minio client also often resolve to the `http://169.254.169.254` default endpoint that was being hard coded before To test, we were able to build a docker image from source and, observe it choosing the expected IAM endpoint, and see files uploaded via the client. --- custom/conf/app.example.ini | 14 ++++++++++++++ modules/setting/storage.go | 1 + modules/setting/storage_test.go | 13 +++++++++++++ modules/storage/minio.go | 8 +++++--- modules/storage/minio_test.go | 21 +++++++++++++-------- 5 files changed, 46 insertions(+), 11 deletions(-) diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index ef5684237d..c3b78e60bb 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -1944,6 +1944,13 @@ LEVEL = Info ;; Minio secretAccessKey to connect only available when STORAGE_TYPE is `minio` ;MINIO_SECRET_ACCESS_KEY = ;; +;; Preferred IAM Endpoint to override Minio's default IAM Endpoint resolution only available when STORAGE_TYPE is `minio`. +;; If not provided and STORAGE_TYPE is `minio`, will search for and derive endpoint from known environment variables +;; (AWS_CONTAINER_AUTHORIZATION_TOKEN, AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE, AWS_CONTAINER_CREDENTIALS_RELATIVE_URI, +;; AWS_CONTAINER_CREDENTIALS_FULL_URI, AWS_WEB_IDENTITY_TOKEN_FILE, AWS_ROLE_ARN, AWS_ROLE_SESSION_NAME, AWS_REGION), +;; or the DefaultIAMRoleEndpoint if not provided otherwise. +;MINIO_IAM_ENDPOINT = +;; ;; Minio bucket to store the attachments only available when STORAGE_TYPE is `minio` ;MINIO_BUCKET = gitea ;; @@ -2688,6 +2695,13 @@ LEVEL = Info ;; Minio secretAccessKey to connect only available when STORAGE_TYPE is `minio` ;MINIO_SECRET_ACCESS_KEY = ;; +;; Preferred IAM Endpoint to override Minio's default IAM Endpoint resolution only available when STORAGE_TYPE is `minio`. +;; If not provided and STORAGE_TYPE is `minio`, will search for and derive endpoint from known environment variables +;; (AWS_CONTAINER_AUTHORIZATION_TOKEN, AWS_CONTAINER_AUTHORIZATION_TOKEN_FILE, AWS_CONTAINER_CREDENTIALS_RELATIVE_URI, +;; AWS_CONTAINER_CREDENTIALS_FULL_URI, AWS_WEB_IDENTITY_TOKEN_FILE, AWS_ROLE_ARN, AWS_ROLE_SESSION_NAME, AWS_REGION), +;; or the DefaultIAMRoleEndpoint if not provided otherwise. +;MINIO_IAM_ENDPOINT = +;; ;; Minio bucket to store the attachments only available when STORAGE_TYPE is `minio` ;MINIO_BUCKET = gitea ;; diff --git a/modules/setting/storage.go b/modules/setting/storage.go index d6f7672b61..d3d1fb9f30 100644 --- a/modules/setting/storage.go +++ b/modules/setting/storage.go @@ -43,6 +43,7 @@ type MinioStorageConfig struct { Endpoint string `ini:"MINIO_ENDPOINT" json:",omitempty"` AccessKeyID string `ini:"MINIO_ACCESS_KEY_ID" json:",omitempty"` SecretAccessKey string `ini:"MINIO_SECRET_ACCESS_KEY" json:",omitempty"` + IamEndpoint string `ini:"MINIO_IAM_ENDPOINT" json:",omitempty"` Bucket string `ini:"MINIO_BUCKET" json:",omitempty"` Location string `ini:"MINIO_LOCATION" json:",omitempty"` BasePath string `ini:"MINIO_BASE_PATH" json:",omitempty"` diff --git a/modules/setting/storage_test.go b/modules/setting/storage_test.go index 44a5de6826..8ee37fd2b6 100644 --- a/modules/setting/storage_test.go +++ b/modules/setting/storage_test.go @@ -470,6 +470,19 @@ MINIO_BASE_PATH = /prefix cfg, err = NewConfigProviderFromData(` [storage] STORAGE_TYPE = minio +MINIO_IAM_ENDPOINT = 127.0.0.1 +MINIO_USE_SSL = true +MINIO_BASE_PATH = /prefix +`) + assert.NoError(t, err) + assert.NoError(t, loadRepoArchiveFrom(cfg)) + assert.EqualValues(t, "127.0.0.1", RepoArchive.Storage.MinioConfig.IamEndpoint) + assert.EqualValues(t, true, RepoArchive.Storage.MinioConfig.UseSSL) + assert.EqualValues(t, "/prefix/repo-archive/", RepoArchive.Storage.MinioConfig.BasePath) + + cfg, err = NewConfigProviderFromData(` +[storage] +STORAGE_TYPE = minio MINIO_ACCESS_KEY_ID = my_access_key MINIO_SECRET_ACCESS_KEY = my_secret_key MINIO_USE_SSL = true diff --git a/modules/storage/minio.go b/modules/storage/minio.go index 8acb7b0354..6b92be61fb 100644 --- a/modules/storage/minio.go +++ b/modules/storage/minio.go @@ -97,7 +97,7 @@ func NewMinioStorage(ctx context.Context, cfg *setting.Storage) (ObjectStorage, } minioClient, err := minio.New(config.Endpoint, &minio.Options{ - Creds: buildMinioCredentials(config, credentials.DefaultIAMRoleEndpoint), + Creds: buildMinioCredentials(config), Secure: config.UseSSL, Transport: &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: config.InsecureSkipVerify}}, Region: config.Location, @@ -164,7 +164,7 @@ func (m *MinioStorage) buildMinioDirPrefix(p string) string { return p } -func buildMinioCredentials(config setting.MinioStorageConfig, iamEndpoint string) *credentials.Credentials { +func buildMinioCredentials(config setting.MinioStorageConfig) *credentials.Credentials { // If static credentials are provided, use those if config.AccessKeyID != "" { return credentials.NewStaticV4(config.AccessKeyID, config.SecretAccessKey, "") @@ -184,7 +184,9 @@ func buildMinioCredentials(config setting.MinioStorageConfig, iamEndpoint string &credentials.FileAWSCredentials{}, // read IAM role from EC2 metadata endpoint if available &credentials.IAM{ - Endpoint: iamEndpoint, + // passing in an empty Endpoint lets the IAM Provider + // decide which endpoint to resolve internally + Endpoint: config.IamEndpoint, Client: &http.Client{ Transport: http.DefaultTransport, }, diff --git a/modules/storage/minio_test.go b/modules/storage/minio_test.go index 6eb03c4a45..395da051e8 100644 --- a/modules/storage/minio_test.go +++ b/modules/storage/minio_test.go @@ -107,8 +107,9 @@ func TestMinioCredentials(t *testing.T) { cfg := setting.MinioStorageConfig{ AccessKeyID: ExpectedAccessKey, SecretAccessKey: ExpectedSecretAccessKey, + IamEndpoint: FakeEndpoint, } - creds := buildMinioCredentials(cfg, FakeEndpoint) + creds := buildMinioCredentials(cfg) v, err := creds.Get() assert.NoError(t, err) @@ -117,13 +118,15 @@ func TestMinioCredentials(t *testing.T) { }) t.Run("Chain", func(t *testing.T) { - cfg := setting.MinioStorageConfig{} + cfg := setting.MinioStorageConfig{ + IamEndpoint: FakeEndpoint, + } t.Run("EnvMinio", func(t *testing.T) { t.Setenv("MINIO_ACCESS_KEY", ExpectedAccessKey+"Minio") t.Setenv("MINIO_SECRET_KEY", ExpectedSecretAccessKey+"Minio") - creds := buildMinioCredentials(cfg, FakeEndpoint) + creds := buildMinioCredentials(cfg) v, err := creds.Get() assert.NoError(t, err) @@ -135,7 +138,7 @@ func TestMinioCredentials(t *testing.T) { t.Setenv("AWS_ACCESS_KEY", ExpectedAccessKey+"AWS") t.Setenv("AWS_SECRET_KEY", ExpectedSecretAccessKey+"AWS") - creds := buildMinioCredentials(cfg, FakeEndpoint) + creds := buildMinioCredentials(cfg) v, err := creds.Get() assert.NoError(t, err) @@ -144,11 +147,11 @@ func TestMinioCredentials(t *testing.T) { }) t.Run("FileMinio", func(t *testing.T) { - t.Setenv("MINIO_SHARED_CREDENTIALS_FILE", "testdata/minio.json") // prevent loading any actual credentials files from the user + t.Setenv("MINIO_SHARED_CREDENTIALS_FILE", "testdata/minio.json") t.Setenv("AWS_SHARED_CREDENTIALS_FILE", "testdata/fake") - creds := buildMinioCredentials(cfg, FakeEndpoint) + creds := buildMinioCredentials(cfg) v, err := creds.Get() assert.NoError(t, err) @@ -161,7 +164,7 @@ func TestMinioCredentials(t *testing.T) { t.Setenv("MINIO_SHARED_CREDENTIALS_FILE", "testdata/fake.json") t.Setenv("AWS_SHARED_CREDENTIALS_FILE", "testdata/aws_credentials") - creds := buildMinioCredentials(cfg, FakeEndpoint) + creds := buildMinioCredentials(cfg) v, err := creds.Get() assert.NoError(t, err) @@ -187,7 +190,9 @@ func TestMinioCredentials(t *testing.T) { defer server.Close() // Use the provided EC2 Instance Metadata server - creds := buildMinioCredentials(cfg, server.URL) + creds := buildMinioCredentials(setting.MinioStorageConfig{ + IamEndpoint: server.URL, + }) v, err := creds.Get() assert.NoError(t, err) From fa175c16949f09757ae85db6697cec327c44cba9 Mon Sep 17 00:00:00 2001 From: silverwind Date: Sun, 24 Nov 2024 00:02:37 +0100 Subject: [PATCH 095/156] Add vue-tsc (#32601) As per https://vuejs.org/guide/typescript/overview#overview, typescript's `tsc` does not support importing `.vue` files from `.ts` files, so we need to use `vue-tsc` which patches in that support. Added a convenience alias `make tsc` to run it. --- Makefile | 8 ++- package-lock.json | 140 +++++++++++++++++++++++++++++++++++++++++++++- package.json | 3 +- 3 files changed, 147 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index b9e940b248..0cd6936b3b 100644 --- a/Makefile +++ b/Makefile @@ -377,12 +377,12 @@ lint-backend-fix: lint-go-fix lint-go-vet lint-editorconfig .PHONY: lint-js lint-js: node_modules npx eslint --color --max-warnings=0 --ext js,ts,vue $(ESLINT_FILES) -# npx tsc +# npx vue-tsc .PHONY: lint-js-fix lint-js-fix: node_modules npx eslint --color --max-warnings=0 --ext js,ts,vue $(ESLINT_FILES) --fix -# npx tsc +# npx vue-tsc .PHONY: lint-css lint-css: node_modules @@ -451,6 +451,10 @@ lint-templates: .venv node_modules lint-yaml: .venv @poetry run yamllint . +.PHONY: tsc +tsc: + npx vue-tsc + .PHONY: watch watch: @bash tools/watch.sh diff --git a/package-lock.json b/package-lock.json index 005c2e5fb5..989c2bd77f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -110,7 +110,8 @@ "type-fest": "4.26.1", "updates": "16.4.0", "vite-string-plugin": "1.3.4", - "vitest": "2.1.4" + "vitest": "2.1.4", + "vue-tsc": "2.1.10" }, "engines": { "node": ">= 18.0.0" @@ -5390,6 +5391,35 @@ "url": "https://opencollective.com/vitest" } }, + "node_modules/@volar/language-core": { + "version": "2.4.10", + "resolved": "https://registry.npmjs.org/@volar/language-core/-/language-core-2.4.10.tgz", + "integrity": "sha512-hG3Z13+nJmGaT+fnQzAkS0hjJRa2FCeqZt6Bd+oGNhUkQ+mTFsDETg5rqUTxyzIh5pSOGY7FHCWUS8G82AzLCA==", + "dev": true, + "license": "MIT", + "dependencies": { + "@volar/source-map": "2.4.10" + } + }, + "node_modules/@volar/source-map": { + "version": "2.4.10", + "resolved": "https://registry.npmjs.org/@volar/source-map/-/source-map-2.4.10.tgz", + "integrity": "sha512-OCV+b5ihV0RF3A7vEvNyHPi4G4kFa6ukPmyVocmqm5QzOd8r5yAtiNvaPEjl8dNvgC/lj4JPryeeHLdXd62rWA==", + "dev": true, + "license": "MIT" + }, + "node_modules/@volar/typescript": { + "version": "2.4.10", + "resolved": "https://registry.npmjs.org/@volar/typescript/-/typescript-2.4.10.tgz", + "integrity": "sha512-F8ZtBMhSXyYKuBfGpYwqA5rsONnOwAVvjyE7KPYJ7wgZqo2roASqNWUnianOomJX5u1cxeRooHV59N0PhvEOgw==", + "dev": true, + "license": "MIT", + "dependencies": { + "@volar/language-core": "2.4.10", + "path-browserify": "^1.0.1", + "vscode-uri": "^3.0.8" + } + }, "node_modules/@vue/compiler-core": { "version": "3.5.12", "resolved": "https://registry.npmjs.org/@vue/compiler-core/-/compiler-core-3.5.12.tgz", @@ -5449,6 +5479,58 @@ "@vue/shared": "3.5.12" } }, + "node_modules/@vue/compiler-vue2": { + "version": "2.7.16", + "resolved": "https://registry.npmjs.org/@vue/compiler-vue2/-/compiler-vue2-2.7.16.tgz", + "integrity": "sha512-qYC3Psj9S/mfu9uVi5WvNZIzq+xnXMhOwbTFKKDD7b1lhpnn71jXSFdTQ+WsIEk0ONCd7VV2IMm7ONl6tbQ86A==", + "dev": true, + "license": "MIT", + "dependencies": { + "de-indent": "^1.0.2", + "he": "^1.2.0" + } + }, + "node_modules/@vue/language-core": { + "version": "2.1.10", + "resolved": "https://registry.npmjs.org/@vue/language-core/-/language-core-2.1.10.tgz", + "integrity": "sha512-DAI289d0K3AB5TUG3xDp9OuQ71CnrujQwJrQnfuZDwo6eGNf0UoRlPuaVNO+Zrn65PC3j0oB2i7mNmVPggeGeQ==", + "dev": true, + "license": "MIT", + "dependencies": { + "@volar/language-core": "~2.4.8", + "@vue/compiler-dom": "^3.5.0", + "@vue/compiler-vue2": "^2.7.16", + "@vue/shared": "^3.5.0", + "alien-signals": "^0.2.0", + "minimatch": "^9.0.3", + "muggle-string": "^0.4.1", + "path-browserify": "^1.0.1" + }, + "peerDependencies": { + "typescript": "*" + }, + "peerDependenciesMeta": { + "typescript": { + "optional": true + } + } + }, + "node_modules/@vue/language-core/node_modules/minimatch": { + "version": "9.0.5", + "resolved": "https://registry.npmjs.org/minimatch/-/minimatch-9.0.5.tgz", + "integrity": "sha512-G6T0ZX48xgozx7587koeX9Ys2NYy6Gmv//P89sEte9V9whIapMNF4idKxnW2QtCcLiTWlb/wfCabAtAFWhhBow==", + "dev": true, + "license": "ISC", + "dependencies": { + "brace-expansion": "^2.0.1" + }, + "engines": { + "node": ">=16 || 14 >=14.17" + }, + "funding": { + "url": "https://github.com/sponsors/isaacs" + } + }, "node_modules/@vue/reactivity": { "version": "3.5.12", "resolved": "https://registry.npmjs.org/@vue/reactivity/-/reactivity-3.5.12.tgz", @@ -5821,6 +5903,13 @@ "ajv": "^8.8.2" } }, + "node_modules/alien-signals": { + "version": "0.2.2", + "resolved": "https://registry.npmjs.org/alien-signals/-/alien-signals-0.2.2.tgz", + "integrity": "sha512-cZIRkbERILsBOXTQmMrxc9hgpxglstn69zm+F1ARf4aPAzdAFYd6sBq87ErO0Fj3DV94tglcyHG5kQz9nDC/8A==", + "dev": true, + "license": "MIT" + }, "node_modules/ansi_up": { "version": "6.0.2", "resolved": "https://registry.npmjs.org/ansi_up/-/ansi_up-6.0.2.tgz", @@ -7484,6 +7573,13 @@ "integrity": "sha512-oaMBel6gjolK862uaPQOVTA7q3TZhuSvuMQAAglQDOWYO9A91IrAOUJEyKVlqJlHE0vq5p5UXxzdPfMH/x6xNg==", "license": "MIT" }, + "node_modules/de-indent": { + "version": "1.0.2", + "resolved": "https://registry.npmjs.org/de-indent/-/de-indent-1.0.2.tgz", + "integrity": "sha512-e/1zu3xH5MQryN2zdVaF0OrdNLUbvWxzMbi+iNA6Bky7l1RoP8a2fIbRocyHclXt/arDrrR6lL3TqFD9pMQTsg==", + "dev": true, + "license": "MIT" + }, "node_modules/debug": { "version": "4.3.7", "resolved": "https://registry.npmjs.org/debug/-/debug-4.3.7.tgz", @@ -10337,6 +10433,16 @@ "node": ">=12.4.0" } }, + "node_modules/he": { + "version": "1.2.0", + "resolved": "https://registry.npmjs.org/he/-/he-1.2.0.tgz", + "integrity": "sha512-F/1DnUGPopORZi0ni+CvrCgHQ5FyEAHRLSApuYWMmrbSwoN2Mn/7k+Gl38gJnR7yyDZk6WLXwiGod1JOWNDKGw==", + "dev": true, + "license": "MIT", + "bin": { + "he": "bin/he" + } + }, "node_modules/hosted-git-info": { "version": "2.8.9", "resolved": "https://registry.npmjs.org/hosted-git-info/-/hosted-git-info-2.8.9.tgz", @@ -11793,6 +11899,13 @@ "integrity": "sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA==", "license": "MIT" }, + "node_modules/muggle-string": { + "version": "0.4.1", + "resolved": "https://registry.npmjs.org/muggle-string/-/muggle-string-0.4.1.tgz", + "integrity": "sha512-VNTrAak/KhO2i8dqqnqnAHOa3cYBwXEZe9h+D5h/1ZqFSTEFHdM65lR7RoIqq3tBBYavsOXV84NoHXZ0AkPyqQ==", + "dev": true, + "license": "MIT" + }, "node_modules/mz": { "version": "2.7.0", "resolved": "https://registry.npmjs.org/mz/-/mz-2.7.0.tgz", @@ -12168,6 +12281,13 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/path-browserify": { + "version": "1.0.1", + "resolved": "https://registry.npmjs.org/path-browserify/-/path-browserify-1.0.1.tgz", + "integrity": "sha512-b7uo2UCUOYZcnF/3ID0lulOJi/bafxa1xPe7ZPsammBSpjSWQkjNxlt635YGS2MiR9GjvuXCtz2emr3jbsz98g==", + "dev": true, + "license": "MIT" + }, "node_modules/path-data-parser": { "version": "0.1.0", "resolved": "https://registry.npmjs.org/path-data-parser/-/path-data-parser-0.1.0.tgz", @@ -15521,6 +15641,24 @@ } } }, + "node_modules/vue-tsc": { + "version": "2.1.10", + "resolved": "https://registry.npmjs.org/vue-tsc/-/vue-tsc-2.1.10.tgz", + "integrity": "sha512-RBNSfaaRHcN5uqVqJSZh++Gy/YUzryuv9u1aFWhsammDJXNtUiJMNoJ747lZcQ68wUQFx6E73y4FY3D8E7FGMA==", + "dev": true, + "license": "MIT", + "dependencies": { + "@volar/typescript": "~2.4.8", + "@vue/language-core": "2.1.10", + "semver": "^7.5.4" + }, + "bin": { + "vue-tsc": "bin/vue-tsc.js" + }, + "peerDependencies": { + "typescript": ">=5.0.0" + } + }, "node_modules/watchpack": { "version": "2.4.2", "resolved": "https://registry.npmjs.org/watchpack/-/watchpack-2.4.2.tgz", diff --git a/package.json b/package.json index c65f0617d8..03c3b79990 100644 --- a/package.json +++ b/package.json @@ -109,7 +109,8 @@ "type-fest": "4.26.1", "updates": "16.4.0", "vite-string-plugin": "1.3.4", - "vitest": "2.1.4" + "vitest": "2.1.4", + "vue-tsc": "2.1.10" }, "browserslist": [ "defaults" From 633785a5f3fe00789a6cba7cc0db1333de1e9c52 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Sun, 24 Nov 2024 16:18:57 +0800 Subject: [PATCH 096/156] Refactor markup render system (#32612) This PR removes (almost) all path tricks, and introduces "renderhelper" package. Now we can clearly see the rendering behaviors for comment/file/wiki, more details are in "renderhelper" tests. Fix #31411 , fix #18592, fix #25632 and maybe more problems. (ps: fix #32608 by the way) --- assets/go-licenses.json | 4 +- models/activities/action.go | 8 +- models/issues/comment_code.go | 10 +- models/renderhelper/commit_checker.go | 53 +++ models/renderhelper/main_test.go | 27 ++ models/renderhelper/repo_comment.go | 73 +++ models/renderhelper/repo_comment_test.go | 76 ++++ models/renderhelper/repo_file.go | 77 ++++ models/renderhelper/repo_file_test.go | 83 ++++ models/renderhelper/repo_wiki.go | 80 ++++ models/renderhelper/repo_wiki_test.go | 65 +++ models/renderhelper/simple_document.go | 29 ++ models/renderhelper/simple_document_test.go | 40 ++ models/unittest/unit_tests.go | 7 +- modules/markup/csv/csv.go | 2 +- modules/markup/external/external.go | 10 +- modules/markup/html.go | 3 +- modules/markup/html_codepreview.go | 2 +- modules/markup/html_codepreview_test.go | 8 +- modules/markup/html_commit.go | 35 +- modules/markup/html_internal_test.go | 94 +--- modules/markup/html_issue.go | 9 +- modules/markup/html_link.go | 32 +- modules/markup/html_mention.go | 8 +- modules/markup/html_node.go | 6 +- modules/markup/html_test.go | 185 ++------ modules/markup/main_test.go | 8 +- modules/markup/markdown/goldmark.go | 4 +- modules/markup/markdown/main_test.go | 13 +- modules/markup/markdown/markdown_test.go | 468 ++------------------ modules/markup/markdown/transform_image.go | 8 +- modules/markup/markdown/transform_link.go | 13 +- modules/markup/orgmode/orgmode.go | 15 +- modules/markup/orgmode/orgmode_test.go | 26 +- modules/markup/render.go | 111 ++--- modules/markup/render_helper.go | 48 +- modules/markup/render_link.go | 42 ++ modules/markup/render_link_test.go | 27 ++ modules/markup/render_links.go | 56 --- modules/markup/renderer.go | 4 +- modules/markup/sanitizer_default.go | 3 + modules/markup/sanitizer_default_test.go | 1 + modules/templates/util_render_test.go | 11 +- routers/api/v1/misc/markup_test.go | 8 +- routers/common/markup.go | 91 ++-- routers/web/feed/convert.go | 40 +- routers/web/feed/profile.go | 7 +- routers/web/org/home.go | 16 +- routers/web/repo/commit.go | 12 +- routers/web/repo/issue.go | 10 +- routers/web/repo/issue_comment.go | 10 +- routers/web/repo/issue_view.go | 27 +- routers/web/repo/milestone.go | 18 +- routers/web/repo/projects.go | 18 +- routers/web/repo/release.go | 10 +- routers/web/repo/render.go | 18 +- routers/web/repo/view.go | 52 +-- routers/web/repo/wiki.go | 7 +- routers/web/user/home.go | 9 +- routers/web/user/profile.go | 24 +- services/mailer/mail.go | 8 +- services/mailer/mail_release.go | 8 +- services/mailer/mail_test.go | 2 +- services/markup/processorhelper.go | 6 +- tests/fuzz/fuzz_test.go | 5 +- 65 files changed, 1096 insertions(+), 1194 deletions(-) create mode 100644 models/renderhelper/commit_checker.go create mode 100644 models/renderhelper/main_test.go create mode 100644 models/renderhelper/repo_comment.go create mode 100644 models/renderhelper/repo_comment_test.go create mode 100644 models/renderhelper/repo_file.go create mode 100644 models/renderhelper/repo_file_test.go create mode 100644 models/renderhelper/repo_wiki.go create mode 100644 models/renderhelper/repo_wiki_test.go create mode 100644 models/renderhelper/simple_document.go create mode 100644 models/renderhelper/simple_document_test.go create mode 100644 modules/markup/render_link.go create mode 100644 modules/markup/render_link_test.go delete mode 100644 modules/markup/render_links.go diff --git a/assets/go-licenses.json b/assets/go-licenses.json index 796c2d6765..fcfde08800 100644 --- a/assets/go-licenses.json +++ b/assets/go-licenses.json @@ -1090,8 +1090,8 @@ "licenseText": "MIT License\n\nCopyright (c) 2017 Asher\n\nPermission is hereby granted, free of charge, to any person obtaining a copy\nof this software and associated documentation files (the \"Software\"), to deal\nin the Software without restriction, including without limitation the rights\nto use, copy, modify, merge, publish, distribute, sublicense, and/or sell\ncopies of the Software, and to permit persons to whom the Software is\nfurnished to do so, subject to the following conditions:\n\nThe above copyright notice and this permission notice shall be included in all\ncopies or substantial portions of the Software.\n\nTHE SOFTWARE IS PROVIDED \"AS IS\", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR\nIMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,\nFITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE\nAUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER\nLIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,\nOUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE\nSOFTWARE.\n" }, { - "name": "github.com/stretchr/testify/assert", - "path": "github.com/stretchr/testify/assert/LICENSE", + "name": "github.com/stretchr/testify", + "path": "github.com/stretchr/testify/LICENSE", "licenseText": "MIT License\n\nCopyright (c) 2012-2020 Mat Ryer, Tyler Bunnell and contributors.\n\nPermission is hereby granted, free of charge, to any person obtaining a copy\nof this software and associated documentation files (the \"Software\"), to deal\nin the Software without restriction, including without limitation the rights\nto use, copy, modify, merge, publish, distribute, sublicense, and/or sell\ncopies of the Software, and to permit persons to whom the Software is\nfurnished to do so, subject to the following conditions:\n\nThe above copyright notice and this permission notice shall be included in all\ncopies or substantial portions of the Software.\n\nTHE SOFTWARE IS PROVIDED \"AS IS\", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR\nIMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,\nFITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE\nAUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER\nLIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,\nOUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE\nSOFTWARE.\n" }, { diff --git a/models/activities/action.go b/models/activities/action.go index 43da557fff..e74deef1df 100644 --- a/models/activities/action.go +++ b/models/activities/action.go @@ -200,7 +200,7 @@ func (a *Action) LoadActUser(ctx context.Context) { } } -func (a *Action) loadRepo(ctx context.Context) { +func (a *Action) LoadRepo(ctx context.Context) { if a.Repo != nil { return } @@ -250,7 +250,7 @@ func (a *Action) GetActDisplayNameTitle(ctx context.Context) string { // GetRepoUserName returns the name of the action repository owner. func (a *Action) GetRepoUserName(ctx context.Context) string { - a.loadRepo(ctx) + a.LoadRepo(ctx) if a.Repo == nil { return "(non-existing-repo)" } @@ -265,7 +265,7 @@ func (a *Action) ShortRepoUserName(ctx context.Context) string { // GetRepoName returns the name of the action repository. func (a *Action) GetRepoName(ctx context.Context) string { - a.loadRepo(ctx) + a.LoadRepo(ctx) if a.Repo == nil { return "(non-existing-repo)" } @@ -644,7 +644,7 @@ func NotifyWatchers(ctx context.Context, actions ...*Action) error { } if repoChanged { - act.loadRepo(ctx) + act.LoadRepo(ctx) repo = act.Repo // check repo owner exist. diff --git a/models/issues/comment_code.go b/models/issues/comment_code.go index 751550f37a..67a77ceb13 100644 --- a/models/issues/comment_code.go +++ b/models/issues/comment_code.go @@ -7,8 +7,8 @@ import ( "context" "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/renderhelper" user_model "code.gitea.io/gitea/models/user" - "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/markup/markdown" "xorm.io/builder" @@ -112,12 +112,8 @@ func findCodeComments(ctx context.Context, opts FindCommentsOptions, issue *Issu } var err error - rctx := markup.NewRenderContext(ctx). - WithRepoFacade(issue.Repo). - WithLinks(markup.Links{Base: issue.Repo.Link()}). - WithMetas(issue.Repo.ComposeMetas(ctx)) - if comment.RenderedContent, err = markdown.RenderString(rctx, - comment.Content); err != nil { + rctx := renderhelper.NewRenderContextRepoComment(ctx, issue.Repo) + if comment.RenderedContent, err = markdown.RenderString(rctx, comment.Content); err != nil { return nil, err } } diff --git a/models/renderhelper/commit_checker.go b/models/renderhelper/commit_checker.go new file mode 100644 index 0000000000..4815643e67 --- /dev/null +++ b/models/renderhelper/commit_checker.go @@ -0,0 +1,53 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package renderhelper + +import ( + "context" + "io" + + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/gitrepo" + "code.gitea.io/gitea/modules/log" +) + +type commitChecker struct { + ctx context.Context + commitCache map[string]bool + gitRepoFacade gitrepo.Repository + + gitRepo *git.Repository + gitRepoCloser io.Closer +} + +func newCommitChecker(ctx context.Context, gitRepo gitrepo.Repository) *commitChecker { + return &commitChecker{ctx: ctx, commitCache: make(map[string]bool), gitRepoFacade: gitRepo} +} + +func (c *commitChecker) Close() error { + if c != nil && c.gitRepoCloser != nil { + return c.gitRepoCloser.Close() + } + return nil +} + +func (c *commitChecker) IsCommitIDExisting(commitID string) bool { + exist, inCache := c.commitCache[commitID] + if inCache { + return exist + } + + if c.gitRepo == nil { + r, closer, err := gitrepo.RepositoryFromContextOrOpen(c.ctx, c.gitRepoFacade) + if err != nil { + log.Error("unable to open repository: %s Error: %v", gitrepo.RepoGitURL(c.gitRepoFacade), err) + return false + } + c.gitRepo, c.gitRepoCloser = r, closer + } + + exist = c.gitRepo.IsReferenceExist(commitID) // Don't use IsObjectExist since it doesn't support short hashs with gogit edition. + c.commitCache[commitID] = exist + return exist +} diff --git a/models/renderhelper/main_test.go b/models/renderhelper/main_test.go new file mode 100644 index 0000000000..331450172a --- /dev/null +++ b/models/renderhelper/main_test.go @@ -0,0 +1,27 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package renderhelper + +import ( + "context" + "testing" + + "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/modules/markup" +) + +func TestMain(m *testing.M) { + unittest.MainTest(m, &unittest.TestOptions{ + FixtureFiles: []string{"repository.yml", "user.yml"}, + SetUp: func() error { + markup.RenderBehaviorForTesting.DisableAdditionalAttributes = true + markup.Init(&markup.RenderHelperFuncs{ + IsUsernameMentionable: func(ctx context.Context, username string) bool { + return username == "user2" + }, + }) + return nil + }, + }) +} diff --git a/models/renderhelper/repo_comment.go b/models/renderhelper/repo_comment.go new file mode 100644 index 0000000000..6bd5e91ad1 --- /dev/null +++ b/models/renderhelper/repo_comment.go @@ -0,0 +1,73 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package renderhelper + +import ( + "context" + "fmt" + + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/modules/markup" + "code.gitea.io/gitea/modules/util" +) + +type RepoComment struct { + ctx *markup.RenderContext + opts RepoCommentOptions + + commitChecker *commitChecker + repoLink string +} + +func (r *RepoComment) CleanUp() { + _ = r.commitChecker.Close() +} + +func (r *RepoComment) IsCommitIDExisting(commitID string) bool { + return r.commitChecker.IsCommitIDExisting(commitID) +} + +func (r *RepoComment) ResolveLink(link string, likeType markup.LinkType) (finalLink string) { + switch likeType { + case markup.LinkTypeApp: + finalLink = r.ctx.ResolveLinkApp(link) + default: + finalLink = r.ctx.ResolveLinkRelative(r.repoLink, r.opts.CurrentRefPath, link) + } + return finalLink +} + +var _ markup.RenderHelper = (*RepoComment)(nil) + +type RepoCommentOptions struct { + DeprecatedRepoName string // it is only a patch for the non-standard "markup" api + DeprecatedOwnerName string // it is only a patch for the non-standard "markup" api + CurrentRefPath string // eg: "branch/main" or "commit/11223344" +} + +func NewRenderContextRepoComment(ctx context.Context, repo *repo_model.Repository, opts ...RepoCommentOptions) *markup.RenderContext { + helper := &RepoComment{ + repoLink: repo.Link(), + opts: util.OptionalArg(opts), + } + rctx := markup.NewRenderContext(ctx) + helper.ctx = rctx + if repo != nil { + helper.repoLink = repo.Link() + helper.commitChecker = newCommitChecker(ctx, repo) + rctx = rctx.WithMetas(repo.ComposeMetas(ctx)) + } else { + // this is almost dead code, only to pass the incorrect tests + helper.repoLink = fmt.Sprintf("%s/%s", helper.opts.DeprecatedOwnerName, helper.opts.DeprecatedRepoName) + rctx = rctx.WithMetas(map[string]string{ + "user": helper.opts.DeprecatedOwnerName, + "repo": helper.opts.DeprecatedRepoName, + + "markdownLineBreakStyle": "comment", + "markupAllowShortIssuePattern": "true", + }) + } + rctx = rctx.WithHelper(helper) + return rctx +} diff --git a/models/renderhelper/repo_comment_test.go b/models/renderhelper/repo_comment_test.go new file mode 100644 index 0000000000..01e20b9e02 --- /dev/null +++ b/models/renderhelper/repo_comment_test.go @@ -0,0 +1,76 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package renderhelper + +import ( + "context" + "testing" + + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/modules/markup" + "code.gitea.io/gitea/modules/markup/markdown" + + "github.com/stretchr/testify/assert" +) + +func TestRepoComment(t *testing.T) { + unittest.PrepareTestEnv(t) + + repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + + t.Run("AutoLink", func(t *testing.T) { + rctx := NewRenderContextRepoComment(context.Background(), repo1).WithMarkupType(markdown.MarkupName) + rendered, err := markup.RenderString(rctx, ` +65f1bf27bc3bf70f64657658635e66094edbcb4d +#1 +@user2 +`) + assert.NoError(t, err) + assert.Equal(t, + `

65f1bf27bc
+#1
+@user2

+`, rendered) + }) + + t.Run("AbsoluteAndRelative", func(t *testing.T) { + rctx := NewRenderContextRepoComment(context.Background(), repo1).WithMarkupType(markdown.MarkupName) + + // It is Gitea's old behavior, the relative path is resolved to the repo path + // It is different from GitHub, GitHub resolves relative links to current page's path + rendered, err := markup.RenderString(rctx, ` +[/test](/test) +[./test](./test) +![/image](/image) +![./image](./image) +`) + assert.NoError(t, err) + assert.Equal(t, + `

/test
+./test
+/image
+./image

+`, rendered) + }) + + t.Run("WithCurrentRefPath", func(t *testing.T) { + rctx := NewRenderContextRepoComment(context.Background(), repo1, RepoCommentOptions{CurrentRefPath: "/commit/1234"}). + WithMarkupType(markdown.MarkupName) + + // the ref path is only used to render commit message: a commit message is rendered at the commit page with its commit ID path + rendered, err := markup.RenderString(rctx, ` +[/test](/test) +[./test](./test) +![/image](/image) +![./image](./image) +`) + assert.NoError(t, err) + assert.Equal(t, `

/test
+./test
+/image
+./image

+`, rendered) + }) +} diff --git a/models/renderhelper/repo_file.go b/models/renderhelper/repo_file.go new file mode 100644 index 0000000000..794828c617 --- /dev/null +++ b/models/renderhelper/repo_file.go @@ -0,0 +1,77 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package renderhelper + +import ( + "context" + "fmt" + "path" + + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/modules/markup" + "code.gitea.io/gitea/modules/util" +) + +type RepoFile struct { + ctx *markup.RenderContext + opts RepoFileOptions + + commitChecker *commitChecker + repoLink string +} + +func (r *RepoFile) CleanUp() { + _ = r.commitChecker.Close() +} + +func (r *RepoFile) IsCommitIDExisting(commitID string) bool { + return r.commitChecker.IsCommitIDExisting(commitID) +} + +func (r *RepoFile) ResolveLink(link string, likeType markup.LinkType) string { + finalLink := link + switch likeType { + case markup.LinkTypeApp: + finalLink = r.ctx.ResolveLinkApp(link) + case markup.LinkTypeDefault: + finalLink = r.ctx.ResolveLinkRelative(path.Join(r.repoLink, "src", r.opts.CurrentRefPath), r.opts.CurrentTreePath, link) + case markup.LinkTypeRaw: + finalLink = r.ctx.ResolveLinkRelative(path.Join(r.repoLink, "raw", r.opts.CurrentRefPath), r.opts.CurrentTreePath, link) + case markup.LinkTypeMedia: + finalLink = r.ctx.ResolveLinkRelative(path.Join(r.repoLink, "media", r.opts.CurrentRefPath), r.opts.CurrentTreePath, link) + } + return finalLink +} + +var _ markup.RenderHelper = (*RepoFile)(nil) + +type RepoFileOptions struct { + DeprecatedRepoName string // it is only a patch for the non-standard "markup" api + DeprecatedOwnerName string // it is only a patch for the non-standard "markup" api + + CurrentRefPath string // eg: "branch/main" + CurrentTreePath string // eg: "path/to/file" in the repo +} + +func NewRenderContextRepoFile(ctx context.Context, repo *repo_model.Repository, opts ...RepoFileOptions) *markup.RenderContext { + helper := &RepoFile{opts: util.OptionalArg(opts)} + rctx := markup.NewRenderContext(ctx) + helper.ctx = rctx + if repo != nil { + helper.repoLink = repo.Link() + helper.commitChecker = newCommitChecker(ctx, repo) + rctx = rctx.WithMetas(repo.ComposeDocumentMetas(ctx)) + } else { + // this is almost dead code, only to pass the incorrect tests + helper.repoLink = fmt.Sprintf("%s/%s", helper.opts.DeprecatedOwnerName, helper.opts.DeprecatedRepoName) + rctx = rctx.WithMetas(map[string]string{ + "user": helper.opts.DeprecatedOwnerName, + "repo": helper.opts.DeprecatedRepoName, + + "markdownLineBreakStyle": "document", + }) + } + rctx = rctx.WithHelper(helper) + return rctx +} diff --git a/models/renderhelper/repo_file_test.go b/models/renderhelper/repo_file_test.go new file mode 100644 index 0000000000..40027ec76f --- /dev/null +++ b/models/renderhelper/repo_file_test.go @@ -0,0 +1,83 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package renderhelper + +import ( + "context" + "testing" + + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/modules/markup" + "code.gitea.io/gitea/modules/markup/markdown" + + "github.com/stretchr/testify/assert" +) + +func TestRepoFile(t *testing.T) { + unittest.PrepareTestEnv(t) + repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1}) + + t.Run("AutoLink", func(t *testing.T) { + rctx := NewRenderContextRepoFile(context.Background(), repo1).WithMarkupType(markdown.MarkupName) + rendered, err := markup.RenderString(rctx, ` +65f1bf27bc3bf70f64657658635e66094edbcb4d +#1 +@user2 +`) + assert.NoError(t, err) + assert.Equal(t, + `

65f1bf27bc +#1 +@user2

+`, rendered) + }) + + t.Run("AbsoluteAndRelative", func(t *testing.T) { + rctx := NewRenderContextRepoFile(context.Background(), repo1, RepoFileOptions{CurrentRefPath: "branch/main"}). + WithMarkupType(markdown.MarkupName) + rendered, err := markup.RenderString(rctx, ` +[/test](/test) +[./test](./test) +![/image](/image) +![./image](./image) +`) + assert.NoError(t, err) + assert.Equal(t, + `

/test +./test +/image +./image

+`, rendered) + }) + + t.Run("WithCurrentRefPath", func(t *testing.T) { + rctx := NewRenderContextRepoFile(context.Background(), repo1, RepoFileOptions{CurrentRefPath: "/commit/1234"}). + WithMarkupType(markdown.MarkupName) + rendered, err := markup.RenderString(rctx, ` +[/test](/test) +![/image](/image) +`) + assert.NoError(t, err) + assert.Equal(t, `

/test +/image

+`, rendered) + }) + + t.Run("WithCurrentRefPathByTag", func(t *testing.T) { + rctx := NewRenderContextRepoFile(context.Background(), repo1, RepoFileOptions{ + CurrentRefPath: "/commit/1234", + CurrentTreePath: "my-dir", + }). + WithMarkupType(markdown.MarkupName) + rendered, err := markup.RenderString(rctx, ` + +