From ad223000d42782d1cd5a2ef9e8cb3a51f76739f9 Mon Sep 17 00:00:00 2001 From: BoYanZh Date: Wed, 13 Nov 2024 13:17:54 -0500 Subject: [PATCH 1/7] Perf: add extra index to notification table (#32395) Index SQL: `CREATE INDEX u_s_uu ON notification(user_id, status, updated_unix);` The naming follows `action.go` in the same dir. I am unsure which version I should add SQL to the migration folder, so I have not modified it. Fix #32390 --- models/activities/notification.go | 58 +++++++++++++++++++---- models/migrations/migrations.go | 1 + models/migrations/v1_23/v309.go | 77 +++++++++++++++++++++++++++++++ 3 files changed, 127 insertions(+), 9 deletions(-) create mode 100644 models/migrations/v1_23/v309.go diff --git a/models/activities/notification.go b/models/activities/notification.go index b888adeb60..6dde26fd53 100644 --- a/models/activities/notification.go +++ b/models/activities/notification.go @@ -18,6 +18,7 @@ import ( "code.gitea.io/gitea/modules/timeutil" "xorm.io/builder" + "xorm.io/xorm/schemas" ) type ( @@ -50,25 +51,64 @@ const ( // Notification represents a notification type Notification struct { ID int64 `xorm:"pk autoincr"` - UserID int64 `xorm:"INDEX NOT NULL"` - RepoID int64 `xorm:"INDEX NOT NULL"` + UserID int64 `xorm:"NOT NULL"` + RepoID int64 `xorm:"NOT NULL"` - Status NotificationStatus `xorm:"SMALLINT INDEX NOT NULL"` - Source NotificationSource `xorm:"SMALLINT INDEX NOT NULL"` + Status NotificationStatus `xorm:"SMALLINT NOT NULL"` + Source NotificationSource `xorm:"SMALLINT NOT NULL"` - IssueID int64 `xorm:"INDEX NOT NULL"` - CommitID string `xorm:"INDEX"` + IssueID int64 `xorm:"NOT NULL"` + CommitID string CommentID int64 - UpdatedBy int64 `xorm:"INDEX NOT NULL"` + UpdatedBy int64 `xorm:"NOT NULL"` Issue *issues_model.Issue `xorm:"-"` Repository *repo_model.Repository `xorm:"-"` Comment *issues_model.Comment `xorm:"-"` User *user_model.User `xorm:"-"` - CreatedUnix timeutil.TimeStamp `xorm:"created INDEX NOT NULL"` - UpdatedUnix timeutil.TimeStamp `xorm:"updated INDEX NOT NULL"` + CreatedUnix timeutil.TimeStamp `xorm:"created NOT NULL"` + UpdatedUnix timeutil.TimeStamp `xorm:"updated NOT NULL"` +} + +// TableIndices implements xorm's TableIndices interface +func (n *Notification) TableIndices() []*schemas.Index { + indices := make([]*schemas.Index, 0, 8) + usuuIndex := schemas.NewIndex("u_s_uu", schemas.IndexType) + usuuIndex.AddColumn("user_id", "status", "updated_unix") + indices = append(indices, usuuIndex) + + // Add the individual indices that were previously defined in struct tags + userIDIndex := schemas.NewIndex("idx_notification_user_id", schemas.IndexType) + userIDIndex.AddColumn("user_id") + indices = append(indices, userIDIndex) + + repoIDIndex := schemas.NewIndex("idx_notification_repo_id", schemas.IndexType) + repoIDIndex.AddColumn("repo_id") + indices = append(indices, repoIDIndex) + + statusIndex := schemas.NewIndex("idx_notification_status", schemas.IndexType) + statusIndex.AddColumn("status") + indices = append(indices, statusIndex) + + sourceIndex := schemas.NewIndex("idx_notification_source", schemas.IndexType) + sourceIndex.AddColumn("source") + indices = append(indices, sourceIndex) + + issueIDIndex := schemas.NewIndex("idx_notification_issue_id", schemas.IndexType) + issueIDIndex.AddColumn("issue_id") + indices = append(indices, issueIDIndex) + + commitIDIndex := schemas.NewIndex("idx_notification_commit_id", schemas.IndexType) + commitIDIndex.AddColumn("commit_id") + indices = append(indices, commitIDIndex) + + updatedByIndex := schemas.NewIndex("idx_notification_updated_by", schemas.IndexType) + updatedByIndex.AddColumn("updated_by") + indices = append(indices, updatedByIndex) + + return indices } func init() { diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 0333e7e204..e0361af86b 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -366,6 +366,7 @@ func prepareMigrationTasks() []*migration { newMigration(306, "Add BlockAdminMergeOverride to ProtectedBranch", v1_23.AddBlockAdminMergeOverrideBranchProtection), newMigration(307, "Fix milestone deadline_unix when there is no due date", v1_23.FixMilestoneNoDueDate), newMigration(308, "Add index(user_id, is_deleted) for action table", v1_23.AddNewIndexForUserDashboard), + newMigration(309, "Improve Notification table indices", v1_23.ImproveNotificationTableIndices), } return preparedMigrations } diff --git a/models/migrations/v1_23/v309.go b/models/migrations/v1_23/v309.go new file mode 100644 index 0000000000..5b39398443 --- /dev/null +++ b/models/migrations/v1_23/v309.go @@ -0,0 +1,77 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_23 //nolint + +import ( + "code.gitea.io/gitea/modules/timeutil" + + "xorm.io/xorm" + "xorm.io/xorm/schemas" +) + +type improveNotificationTableIndicesAction struct { + ID int64 `xorm:"pk autoincr"` + UserID int64 `xorm:"NOT NULL"` + RepoID int64 `xorm:"NOT NULL"` + + Status uint8 `xorm:"SMALLINT NOT NULL"` + Source uint8 `xorm:"SMALLINT NOT NULL"` + + IssueID int64 `xorm:"NOT NULL"` + CommitID string + CommentID int64 + + UpdatedBy int64 `xorm:"NOT NULL"` + + CreatedUnix timeutil.TimeStamp `xorm:"created NOT NULL"` + UpdatedUnix timeutil.TimeStamp `xorm:"updated NOT NULL"` +} + +// TableName sets the name of this table +func (*improveNotificationTableIndicesAction) TableName() string { + return "notification" +} + +// TableIndices implements xorm's TableIndices interface +func (*improveNotificationTableIndicesAction) TableIndices() []*schemas.Index { + indices := make([]*schemas.Index, 0, 8) + usuuIndex := schemas.NewIndex("u_s_uu", schemas.IndexType) + usuuIndex.AddColumn("user_id", "status", "updated_unix") + indices = append(indices, usuuIndex) + + // Add the individual indices that were previously defined in struct tags + userIDIndex := schemas.NewIndex("idx_notification_user_id", schemas.IndexType) + userIDIndex.AddColumn("user_id") + indices = append(indices, userIDIndex) + + repoIDIndex := schemas.NewIndex("idx_notification_repo_id", schemas.IndexType) + repoIDIndex.AddColumn("repo_id") + indices = append(indices, repoIDIndex) + + statusIndex := schemas.NewIndex("idx_notification_status", schemas.IndexType) + statusIndex.AddColumn("status") + indices = append(indices, statusIndex) + + sourceIndex := schemas.NewIndex("idx_notification_source", schemas.IndexType) + sourceIndex.AddColumn("source") + indices = append(indices, sourceIndex) + + issueIDIndex := schemas.NewIndex("idx_notification_issue_id", schemas.IndexType) + issueIDIndex.AddColumn("issue_id") + indices = append(indices, issueIDIndex) + + commitIDIndex := schemas.NewIndex("idx_notification_commit_id", schemas.IndexType) + commitIDIndex.AddColumn("commit_id") + indices = append(indices, commitIDIndex) + + updatedByIndex := schemas.NewIndex("idx_notification_updated_by", schemas.IndexType) + updatedByIndex.AddColumn("updated_by") + indices = append(indices, updatedByIndex) + + return indices +} + +func ImproveNotificationTableIndices(x *xorm.Engine) error { + return x.Sync(&improveNotificationTableIndicesAction{}) +} From 9880c1372e6db6dd9cf28ae240af6bd52164459d Mon Sep 17 00:00:00 2001 From: silverwind Date: Wed, 13 Nov 2024 22:39:55 +0100 Subject: [PATCH 2/7] Bump CI,Flake and Snap to Node 22 (#32487) Node 22 is LTS since 2024-10-29. Updated it everywhere. --------- Co-authored-by: techknowlogick --- .github/workflows/pull-compliance.yml | 8 ++++---- .github/workflows/pull-e2e-tests.yml | 2 +- .github/workflows/release-nightly.yml | 2 +- .github/workflows/release-tag-rc.yml | 2 +- .github/workflows/release-tag-version.yml | 2 +- flake.lock | 12 ++++++------ flake.nix | 2 +- snap/snapcraft.yaml | 2 +- 8 files changed, 16 insertions(+), 16 deletions(-) diff --git a/.github/workflows/pull-compliance.yml b/.github/workflows/pull-compliance.yml index 429829dfe0..7e988e0449 100644 --- a/.github/workflows/pull-compliance.yml +++ b/.github/workflows/pull-compliance.yml @@ -37,7 +37,7 @@ jobs: python-version: "3.12" - uses: actions/setup-node@v4 with: - node-version: 20 + node-version: 22 cache: npm cache-dependency-path: package-lock.json - run: pip install poetry @@ -66,7 +66,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-node@v4 with: - node-version: 20 + node-version: 22 cache: npm cache-dependency-path: package-lock.json - run: make deps-frontend @@ -137,7 +137,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-node@v4 with: - node-version: 20 + node-version: 22 cache: npm cache-dependency-path: package-lock.json - run: make deps-frontend @@ -186,7 +186,7 @@ jobs: - uses: actions/checkout@v4 - uses: actions/setup-node@v4 with: - node-version: 20 + node-version: 22 cache: npm cache-dependency-path: package-lock.json - run: make deps-frontend diff --git a/.github/workflows/pull-e2e-tests.yml b/.github/workflows/pull-e2e-tests.yml index 35ac7598f6..b84c69e4a0 100644 --- a/.github/workflows/pull-e2e-tests.yml +++ b/.github/workflows/pull-e2e-tests.yml @@ -23,7 +23,7 @@ jobs: check-latest: true - uses: actions/setup-node@v4 with: - node-version: 20 + node-version: 22 cache: npm cache-dependency-path: package-lock.json - run: make deps-frontend frontend deps-backend diff --git a/.github/workflows/release-nightly.yml b/.github/workflows/release-nightly.yml index 10fe94b296..6e1b6e0758 100644 --- a/.github/workflows/release-nightly.yml +++ b/.github/workflows/release-nightly.yml @@ -22,7 +22,7 @@ jobs: check-latest: true - uses: actions/setup-node@v4 with: - node-version: 20 + node-version: 22 cache: npm cache-dependency-path: package-lock.json - run: make deps-frontend deps-backend diff --git a/.github/workflows/release-tag-rc.yml b/.github/workflows/release-tag-rc.yml index 55908d3657..41037df29c 100644 --- a/.github/workflows/release-tag-rc.yml +++ b/.github/workflows/release-tag-rc.yml @@ -23,7 +23,7 @@ jobs: check-latest: true - uses: actions/setup-node@v4 with: - node-version: 20 + node-version: 22 cache: npm cache-dependency-path: package-lock.json - run: make deps-frontend deps-backend diff --git a/.github/workflows/release-tag-version.yml b/.github/workflows/release-tag-version.yml index edf7ea1270..a23e698200 100644 --- a/.github/workflows/release-tag-version.yml +++ b/.github/workflows/release-tag-version.yml @@ -25,7 +25,7 @@ jobs: check-latest: true - uses: actions/setup-node@v4 with: - node-version: 20 + node-version: 22 cache: npm cache-dependency-path: package-lock.json - run: make deps-frontend deps-backend diff --git a/flake.lock b/flake.lock index 9eadad2b94..1890b82dcf 100644 --- a/flake.lock +++ b/flake.lock @@ -5,11 +5,11 @@ "systems": "systems" }, "locked": { - "lastModified": 1710146030, - "narHash": "sha256-SZ5L6eA7HJ/nmkzGG7/ISclqe6oZdOZTNoesiInkXPQ=", + "lastModified": 1726560853, + "narHash": "sha256-X6rJYSESBVr3hBoH0WbKE5KvhPU5bloyZ2L4K60/fPQ=", "owner": "numtide", "repo": "flake-utils", - "rev": "b1d9ab70662946ef0850d488da1c9019f3a9752a", + "rev": "c1dfcf08411b08f6b8615f7d8971a2bfa81d5e8a", "type": "github" }, "original": { @@ -20,11 +20,11 @@ }, "nixpkgs": { "locked": { - "lastModified": 1720542800, - "narHash": "sha256-ZgnNHuKV6h2+fQ5LuqnUaqZey1Lqqt5dTUAiAnqH0QQ=", + "lastModified": 1731139594, + "narHash": "sha256-IigrKK3vYRpUu+HEjPL/phrfh7Ox881er1UEsZvw9Q4=", "owner": "nixos", "repo": "nixpkgs", - "rev": "feb2849fdeb70028c70d73b848214b00d324a497", + "rev": "76612b17c0ce71689921ca12d9ffdc9c23ce40b2", "type": "github" }, "original": { diff --git a/flake.nix b/flake.nix index e2f273e341..2c9d74c137 100644 --- a/flake.nix +++ b/flake.nix @@ -22,7 +22,7 @@ gzip # frontend - nodejs_20 + nodejs_22 # linting python312 diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml index a0a85d85af..c4c31968e0 100644 --- a/snap/snapcraft.yaml +++ b/snap/snapcraft.yaml @@ -44,7 +44,7 @@ parts: source: . stage-packages: [ git, sqlite3, openssh-client ] build-packages: [ git, libpam0g-dev, libsqlite3-dev, build-essential] - build-snaps: [ go/1.23/stable, node/20/stable ] + build-snaps: [ go/1.23/stable, node/22/stable ] build-environment: - LDFLAGS: "" override-pull: | From 985e2a8af3d6468bac3ab178148c38bdbd8414f5 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 14 Nov 2024 12:17:58 +0800 Subject: [PATCH 3/7] Fix nil panic if repo doesn't exist (#32501) fix #32496 --- models/activities/action.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/models/activities/action.go b/models/activities/action.go index c83dba9d46..43da557fff 100644 --- a/models/activities/action.go +++ b/models/activities/action.go @@ -251,6 +251,9 @@ 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) + if a.Repo == nil { + return "(non-existing-repo)" + } return a.Repo.OwnerName } @@ -263,6 +266,9 @@ 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) + if a.Repo == nil { + return "(non-existing-repo)" + } return a.Repo.Name } From 3f9c3e7bc394c115ccc4818d6505f1f68de350d2 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Thu, 14 Nov 2024 13:02:11 +0800 Subject: [PATCH 4/7] Refactor render system (#32492) There were too many patches to the Render system, it's really difficult to make further improvements. This PR clears the legacy problems and fix TODOs. 1. Rename `RenderContext.Type` to `RenderContext.MarkupType` to clarify its usage. 2. Use `ContentMode` to replace `meta["mode"]` and `IsWiki`, to clarify the rendering behaviors. 3. Use "wiki" mode instead of "mode=gfm + wiki=true" 4. Merge `renderByType` and `renderByFile` 5. Add more comments ---- The problem of "mode=document": in many cases it is not set, so many non-comment places use comment's hard line break incorrectly --- models/repo/repo.go | 2 - modules/markup/console/console.go | 23 +--- modules/markup/html.go | 9 +- modules/markup/html_codepreview_test.go | 5 +- modules/markup/html_internal_test.go | 22 ++-- modules/markup/html_issue.go | 5 +- modules/markup/html_link.go | 4 +- modules/markup/html_node.go | 4 +- modules/markup/html_test.go | 52 ++++----- modules/markup/markdown/goldmark.go | 7 +- modules/markup/markdown/markdown.go | 4 +- modules/markup/markdown/markdown_test.go | 40 ++++--- modules/markup/markdown/transform_image.go | 2 +- modules/markup/orgmode/orgmode.go | 5 +- modules/markup/orgmode/orgmode_test.go | 3 +- modules/markup/render.go | 108 ++++++++++-------- modules/structs/miscellaneous.go | 10 +- modules/templates/util_render.go | 10 +- modules/templates/util_render_test.go | 15 ++- routers/api/v1/misc/markup.go | 12 +- routers/api/v1/misc/markup_test.go | 13 ++- routers/common/markup.go | 70 +++++------- routers/web/feed/convert.go | 1 - routers/web/misc/markup.go | 4 +- routers/web/repo/view.go | 12 +- routers/web/repo/wiki.go | 6 +- routers/web/user/profile.go | 1 - services/context/org.go | 3 +- templates/swagger/v1_json.tmpl | 8 +- tests/integration/markup_external_test.go | 27 ++++- .../js/features/comp/ComboMarkdownEditor.ts | 3 - web_src/js/features/repo-wiki.ts | 4 +- 32 files changed, 237 insertions(+), 257 deletions(-) diff --git a/models/repo/repo.go b/models/repo/repo.go index 68f8e16a21..4776ff0b9c 100644 --- a/models/repo/repo.go +++ b/models/repo/repo.go @@ -479,7 +479,6 @@ func (repo *Repository) ComposeMetas(ctx context.Context) map[string]string { metas := map[string]string{ "user": repo.OwnerName, "repo": repo.Name, - "mode": "comment", } unit, err := repo.GetUnit(ctx, unit.TypeExternalTracker) @@ -521,7 +520,6 @@ func (repo *Repository) ComposeDocumentMetas(ctx context.Context) map[string]str for k, v := range repo.ComposeMetas(ctx) { metas[k] = v } - metas["mode"] = "document" repo.DocumentRenderingMetas = metas } return repo.DocumentRenderingMetas diff --git a/modules/markup/console/console.go b/modules/markup/console/console.go index 01653565fe..d991527b80 100644 --- a/modules/markup/console/console.go +++ b/modules/markup/console/console.go @@ -8,7 +8,6 @@ import ( "io" "path/filepath" "regexp" - "strings" "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/setting" @@ -17,9 +16,6 @@ import ( "github.com/go-enry/go-enry/v2" ) -// MarkupName describes markup's name -var MarkupName = "console" - func init() { markup.RegisterRenderer(Renderer{}) } @@ -29,7 +25,7 @@ type Renderer struct{} // Name implements markup.Renderer func (Renderer) Name() string { - return MarkupName + return "console" } // Extensions implements markup.Renderer @@ -67,20 +63,3 @@ func (Renderer) Render(ctx *markup.RenderContext, input io.Reader, output io.Wri _, err = output.Write(buf) return err } - -// Render renders terminal colors to HTML with all specific handling stuff. -func Render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error { - if ctx.Type == "" { - ctx.Type = MarkupName - } - return markup.Render(ctx, input, output) -} - -// RenderString renders terminal colors in string to HTML with all specific handling stuff and return string -func RenderString(ctx *markup.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 -} diff --git a/modules/markup/html.go b/modules/markup/html.go index e2eefefc4b..54c65c95d2 100644 --- a/modules/markup/html.go +++ b/modules/markup/html.go @@ -442,12 +442,11 @@ func createLink(href, content, class string) *html.Node { a := &html.Node{ Type: html.ElementNode, Data: atom.A.String(), - Attr: []html.Attribute{ - {Key: "href", Val: href}, - {Key: "data-markdown-generated-content"}, - }, + Attr: []html.Attribute{{Key: "href", Val: href}}, + } + if !RenderBehaviorForTesting.DisableInternalAttributes { + a.Attr = append(a.Attr, html.Attribute{Key: "data-markdown-generated-content"}) } - if class != "" { a.Attr = append(a.Attr, html.Attribute{Key: "class", Val: class}) } diff --git a/modules/markup/html_codepreview_test.go b/modules/markup/html_codepreview_test.go index a90de278f5..5054627dde 100644 --- a/modules/markup/html_codepreview_test.go +++ b/modules/markup/html_codepreview_test.go @@ -11,6 +11,7 @@ import ( "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/markup" + "code.gitea.io/gitea/modules/markup/markdown" "github.com/stretchr/testify/assert" ) @@ -23,8 +24,8 @@ func TestRenderCodePreview(t *testing.T) { }) test := func(input, expected string) { buffer, err := markup.RenderString(&markup.RenderContext{ - Ctx: git.DefaultContext, - Type: "markdown", + Ctx: git.DefaultContext, + MarkupType: markdown.MarkupName, }, input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer)) diff --git a/modules/markup/html_internal_test.go b/modules/markup/html_internal_test.go index 8f516751b0..2fb657f56b 100644 --- a/modules/markup/html_internal_test.go +++ b/modules/markup/html_internal_test.go @@ -11,6 +11,7 @@ import ( "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/setting" + testModule "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" @@ -123,8 +124,9 @@ func TestRender_IssueIndexPattern2(t *testing.T) { } expectedNil := fmt.Sprintf(expectedFmt, links...) testRenderIssueIndexPattern(t, s, expectedNil, &RenderContext{ - Ctx: git.DefaultContext, - Metas: localMetas, + Ctx: git.DefaultContext, + Metas: localMetas, + ContentMode: RenderContentAsComment, }) class := "ref-issue" @@ -137,8 +139,9 @@ func TestRender_IssueIndexPattern2(t *testing.T) { } expectedNum := fmt.Sprintf(expectedFmt, links...) testRenderIssueIndexPattern(t, s, expectedNum, &RenderContext{ - Ctx: git.DefaultContext, - Metas: numericMetas, + Ctx: git.DefaultContext, + Metas: numericMetas, + ContentMode: RenderContentAsComment, }) } @@ -266,7 +269,6 @@ func TestRender_IssueIndexPattern_Document(t *testing.T) { "user": "someUser", "repo": "someRepo", "style": IssueNameStyleNumeric, - "mode": "document", } testRenderIssueIndexPattern(t, "#1", "#1", &RenderContext{ @@ -316,8 +318,8 @@ func TestRender_AutoLink(t *testing.T) { Links: Links{ Base: TestRepoURL, }, - Metas: localMetas, - IsWiki: true, + Metas: localMetas, + ContentMode: RenderContentAsWiki, }, strings.NewReader(input), &buffer) assert.Equal(t, err, nil) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer.String())) @@ -340,7 +342,7 @@ func TestRender_AutoLink(t *testing.T) { func TestRender_FullIssueURLs(t *testing.T) { setting.AppURL = TestAppURL - + defer testModule.MockVariableValue(&RenderBehaviorForTesting.DisableInternalAttributes, true)() test := func(input, expected string) { var result strings.Builder err := postProcess(&RenderContext{ @@ -351,9 +353,7 @@ func TestRender_FullIssueURLs(t *testing.T) { Metas: localMetas, }, []processor{fullIssuePatternProcessor}, strings.NewReader(input), &result) assert.NoError(t, err) - actual := result.String() - actual = strings.ReplaceAll(actual, ` data-markdown-generated-content=""`, "") - assert.Equal(t, expected, actual) + assert.Equal(t, expected, result.String()) } test("Here is a link https://git.osgeo.org/gogs/postgis/postgis/pulls/6", "Here is a link https://git.osgeo.org/gogs/postgis/postgis/pulls/6") diff --git a/modules/markup/html_issue.go b/modules/markup/html_issue.go index b6d4ed6a8e..fa630656ce 100644 --- a/modules/markup/html_issue.go +++ b/modules/markup/html_issue.go @@ -67,9 +67,8 @@ func issueIndexPatternProcessor(ctx *RenderContext, node *html.Node) { 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 + // crossLinkOnly if not comment and not wiki + crossLinkOnly := ctx.ContentMode != RenderContentAsTitle && ctx.ContentMode != RenderContentAsComment && ctx.ContentMode != RenderContentAsWiki var ( found bool diff --git a/modules/markup/html_link.go b/modules/markup/html_link.go index 9350634568..30564da548 100644 --- a/modules/markup/html_link.go +++ b/modules/markup/html_link.go @@ -20,7 +20,7 @@ func ResolveLink(ctx *RenderContext, link, userContentAnchorPrefix string) (resu isAnchorFragment := link != "" && link[0] == '#' if !isAnchorFragment && !IsFullURLString(link) { linkBase := ctx.Links.Base - if ctx.IsWiki { + if ctx.ContentMode == RenderContentAsWiki { // no need to check if the link should be resolved as a wiki link or a wiki raw link // just use wiki link here and it will be redirected to a wiki raw link if necessary linkBase = ctx.Links.WikiLink() @@ -147,7 +147,7 @@ func shortLinkProcessor(ctx *RenderContext, node *html.Node) { } if image { if !absoluteLink { - link = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.IsWiki), link) + link = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.ContentMode == RenderContentAsWiki), link) } title := props["title"] if title == "" { diff --git a/modules/markup/html_node.go b/modules/markup/html_node.go index 6d784975b9..c499854053 100644 --- a/modules/markup/html_node.go +++ b/modules/markup/html_node.go @@ -17,7 +17,7 @@ func visitNodeImg(ctx *RenderContext, img *html.Node) (next *html.Node) { } if IsNonEmptyRelativePath(attr.Val) { - attr.Val = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.IsWiki), attr.Val) + attr.Val = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.ContentMode == RenderContentAsWiki), attr.Val) // By default, the "" tag should also be clickable, // because frontend use `` to paste the re-scaled image into the markdown, @@ -53,7 +53,7 @@ func visitNodeVideo(ctx *RenderContext, node *html.Node) (next *html.Node) { continue } if IsNonEmptyRelativePath(attr.Val) { - attr.Val = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.IsWiki), attr.Val) + attr.Val = util.URLJoin(ctx.Links.ResolveMediaLink(ctx.ContentMode == RenderContentAsWiki), attr.Val) } attr.Val = camoHandleLink(attr.Val) node.Attr[i] = attr diff --git a/modules/markup/html_test.go b/modules/markup/html_test.go index 82aded4407..262d0fc4dd 100644 --- a/modules/markup/html_test.go +++ b/modules/markup/html_test.go @@ -14,6 +14,7 @@ import ( "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/markup/markdown" "code.gitea.io/gitea/modules/setting" + testModule "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" @@ -104,7 +105,7 @@ func TestRender_Commits(t *testing.T) { func TestRender_CrossReferences(t *testing.T) { setting.AppURL = markup.TestAppURL - + defer testModule.MockVariableValue(&markup.RenderBehaviorForTesting.DisableInternalAttributes, true)() test := func(input, expected string) { buffer, err := markup.RenderString(&markup.RenderContext{ Ctx: git.DefaultContext, @@ -116,9 +117,7 @@ func TestRender_CrossReferences(t *testing.T) { Metas: localMetas, }, input) assert.NoError(t, err) - actual := strings.TrimSpace(buffer) - actual = strings.ReplaceAll(actual, ` data-markdown-generated-content=""`, "") - assert.Equal(t, strings.TrimSpace(expected), actual) + assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer)) } test( @@ -148,7 +147,7 @@ func TestRender_CrossReferences(t *testing.T) { func TestRender_links(t *testing.T) { setting.AppURL = markup.TestAppURL - + defer testModule.MockVariableValue(&markup.RenderBehaviorForTesting.DisableInternalAttributes, true)() test := func(input, expected string) { buffer, err := markup.RenderString(&markup.RenderContext{ Ctx: git.DefaultContext, @@ -158,9 +157,7 @@ func TestRender_links(t *testing.T) { }, }, input) assert.NoError(t, err) - actual := strings.TrimSpace(buffer) - actual = strings.ReplaceAll(actual, ` data-markdown-generated-content=""`, "") - assert.Equal(t, strings.TrimSpace(expected), actual) + assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer)) } oldCustomURLSchemes := setting.Markdown.CustomURLSchemes @@ -261,7 +258,7 @@ func TestRender_links(t *testing.T) { func TestRender_email(t *testing.T) { setting.AppURL = markup.TestAppURL - + defer testModule.MockVariableValue(&markup.RenderBehaviorForTesting.DisableInternalAttributes, true)() test := func(input, expected string) { res, err := markup.RenderString(&markup.RenderContext{ Ctx: git.DefaultContext, @@ -271,9 +268,7 @@ func TestRender_email(t *testing.T) { }, }, input) assert.NoError(t, err) - actual := strings.TrimSpace(res) - actual = strings.ReplaceAll(actual, ` data-markdown-generated-content=""`, "") - assert.Equal(t, strings.TrimSpace(expected), actual) + assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(res)) } // Text that should be turned into email link @@ -302,10 +297,10 @@ func TestRender_email(t *testing.T) { j.doe@example.com; j.doe@example.com? j.doe@example.com!`, - `

j.doe@example.com,
-j.doe@example.com.
-j.doe@example.com;
-j.doe@example.com?
+ `

j.doe@example.com, +j.doe@example.com. +j.doe@example.com; +j.doe@example.com? j.doe@example.com!

`) // Test that should *not* be turned into email links @@ -418,8 +413,8 @@ func TestRender_ShortLinks(t *testing.T) { Links: markup.Links{ Base: markup.TestRepoURL, }, - Metas: localMetas, - IsWiki: true, + Metas: localMetas, + ContentMode: markup.RenderContentAsWiki, }, input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expectedWiki), strings.TrimSpace(string(buffer))) @@ -531,10 +526,10 @@ func TestRender_ShortLinks(t *testing.T) { func TestRender_RelativeMedias(t *testing.T) { render := func(input string, isWiki bool, links markup.Links) string { buffer, err := markdown.RenderString(&markup.RenderContext{ - Ctx: git.DefaultContext, - Links: links, - Metas: localMetas, - IsWiki: isWiki, + Ctx: git.DefaultContext, + Links: links, + Metas: localMetas, + ContentMode: util.Iif(isWiki, markup.RenderContentAsWiki, markup.RenderContentAsComment), }, input) assert.NoError(t, err) return strings.TrimSpace(string(buffer)) @@ -604,12 +599,7 @@ func Test_ParseClusterFuzz(t *testing.T) { func TestPostProcess_RenderDocument(t *testing.T) { setting.AppURL = markup.TestAppURL setting.StaticURLPrefix = markup.TestAppURL // can't run standalone - - localMetas := map[string]string{ - "user": "go-gitea", - "repo": "gitea", - "mode": "document", - } + defer testModule.MockVariableValue(&markup.RenderBehaviorForTesting.DisableInternalAttributes, true)() test := func(input, expected string) { var res strings.Builder @@ -619,12 +609,10 @@ func TestPostProcess_RenderDocument(t *testing.T) { AbsolutePrefix: true, Base: "https://example.com", }, - Metas: localMetas, + Metas: map[string]string{"user": "go-gitea", "repo": "gitea"}, }, strings.NewReader(input), &res) assert.NoError(t, err) - actual := strings.TrimSpace(res.String()) - actual = strings.ReplaceAll(actual, ` data-markdown-generated-content=""`, "") - assert.Equal(t, strings.TrimSpace(expected), actual) + assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(res.String())) } // Issue index shouldn't be post processing in a document. diff --git a/modules/markup/markdown/goldmark.go b/modules/markup/markdown/goldmark.go index 0cd9dc5f30..c8488cfb50 100644 --- a/modules/markup/markdown/goldmark.go +++ b/modules/markup/markdown/goldmark.go @@ -72,7 +72,12 @@ func (g *ASTTransformer) Transform(node *ast.Document, reader text.Reader, pc pa g.transformList(ctx, v, rc) case *ast.Text: if v.SoftLineBreak() && !v.HardLineBreak() { - if ctx.Metas["mode"] != "document" { + // TODO: this was a quite unclear part, old code: `if metas["mode"] != "document" { use comment link break setting }` + // many places render non-comment contents with no mode=document, then these contents also use comment's hard line break setting + // especially in many tests. + if markup.RenderBehaviorForTesting.ForceHardLineBreak { + v.SetHardLineBreak(true) + } else if ctx.ContentMode == markup.RenderContentAsComment { v.SetHardLineBreak(setting.Markdown.EnableHardLineBreakInComments) } else { v.SetHardLineBreak(setting.Markdown.EnableHardLineBreakInDocuments) diff --git a/modules/markup/markdown/markdown.go b/modules/markup/markdown/markdown.go index db4e5706f6..6af0deb27b 100644 --- a/modules/markup/markdown/markdown.go +++ b/modules/markup/markdown/markdown.go @@ -257,9 +257,7 @@ func (Renderer) Render(ctx *markup.RenderContext, input io.Reader, output io.Wri // Render renders Markdown to HTML with all specific handling stuff. func Render(ctx *markup.RenderContext, input io.Reader, output io.Writer) error { - if ctx.Type == "" { - ctx.Type = MarkupName - } + ctx.MarkupType = MarkupName return markup.Render(ctx, input, output) } diff --git a/modules/markup/markdown/markdown_test.go b/modules/markup/markdown/markdown_test.go index ad38e7a088..315eed2e62 100644 --- a/modules/markup/markdown/markdown_test.go +++ b/modules/markup/markdown/markdown_test.go @@ -16,6 +16,7 @@ import ( "code.gitea.io/gitea/modules/markup/markdown" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/svg" + "code.gitea.io/gitea/modules/test" "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" @@ -74,7 +75,7 @@ func TestRender_StandardLinks(t *testing.T) { Links: markup.Links{ Base: FullURL, }, - IsWiki: true, + ContentMode: markup.RenderContentAsWiki, }, input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expectedWiki), strings.TrimSpace(string(buffer))) @@ -296,23 +297,22 @@ This PR has been generated by [Renovate Bot](https://github.com/renovatebot/reno } func TestTotal_RenderWiki(t *testing.T) { + defer test.MockVariableValue(&markup.RenderBehaviorForTesting.ForceHardLineBreak, true)() + defer test.MockVariableValue(&markup.RenderBehaviorForTesting.DisableInternalAttributes, true)() setting.AppURL = AppURL - answers := testAnswers(util.URLJoin(FullURL, "wiki"), util.URLJoin(FullURL, "wiki", "raw")) - for i := 0; i < len(sameCases); i++ { line, err := markdown.RenderString(&markup.RenderContext{ Ctx: git.DefaultContext, Links: markup.Links{ Base: FullURL, }, - Repo: newMockRepo(testRepoOwnerName, testRepoName), - Metas: localMetas, - IsWiki: true, + Repo: newMockRepo(testRepoOwnerName, testRepoName), + Metas: localMetas, + ContentMode: markup.RenderContentAsWiki, }, sameCases[i]) assert.NoError(t, err) - actual := strings.ReplaceAll(string(line), ` data-markdown-generated-content=""`, "") - assert.Equal(t, answers[i], actual) + assert.Equal(t, answers[i], string(line)) } testCases := []string{ @@ -334,19 +334,18 @@ func TestTotal_RenderWiki(t *testing.T) { Links: markup.Links{ Base: FullURL, }, - IsWiki: true, + ContentMode: markup.RenderContentAsWiki, }, testCases[i]) assert.NoError(t, err) - actual := strings.ReplaceAll(string(line), ` data-markdown-generated-content=""`, "") - assert.EqualValues(t, testCases[i+1], actual) + assert.EqualValues(t, testCases[i+1], string(line)) } } func TestTotal_RenderString(t *testing.T) { + defer test.MockVariableValue(&markup.RenderBehaviorForTesting.ForceHardLineBreak, true)() + defer test.MockVariableValue(&markup.RenderBehaviorForTesting.DisableInternalAttributes, true)() setting.AppURL = AppURL - answers := testAnswers(util.URLJoin(FullURL, "src", "master"), util.URLJoin(FullURL, "media", "master")) - for i := 0; i < len(sameCases); i++ { line, err := markdown.RenderString(&markup.RenderContext{ Ctx: git.DefaultContext, @@ -358,8 +357,7 @@ func TestTotal_RenderString(t *testing.T) { Metas: localMetas, }, sameCases[i]) assert.NoError(t, err) - actual := strings.ReplaceAll(string(line), ` data-markdown-generated-content=""`, "") - assert.Equal(t, answers[i], actual) + assert.Equal(t, answers[i], string(line)) } testCases := []string{} @@ -428,6 +426,7 @@ func TestRenderSiblingImages_Issue12925(t *testing.T) { expected := `

image1
image2

` + defer test.MockVariableValue(&markup.RenderBehaviorForTesting.ForceHardLineBreak, true)() res, err := markdown.RenderRawString(&markup.RenderContext{Ctx: git.DefaultContext}, testcase) assert.NoError(t, err) assert.Equal(t, expected, res) @@ -996,11 +995,16 @@ space

}, } + defer test.MockVariableValue(&markup.RenderBehaviorForTesting.ForceHardLineBreak, true)() + defer test.MockVariableValue(&markup.RenderBehaviorForTesting.DisableInternalAttributes, true)() for i, c := range cases { - result, err := markdown.RenderString(&markup.RenderContext{Ctx: context.Background(), Links: c.Links, IsWiki: c.IsWiki}, input) + result, err := markdown.RenderString(&markup.RenderContext{ + Ctx: context.Background(), + Links: c.Links, + ContentMode: util.Iif(c.IsWiki, markup.RenderContentAsWiki, markup.RenderContentAsDefault), + }, input) assert.NoError(t, err, "Unexpected error in testcase: %v", i) - actual := strings.ReplaceAll(string(result), ` data-markdown-generated-content=""`, "") - assert.Equal(t, c.Expected, actual, "Unexpected result in testcase %v", i) + assert.Equal(t, c.Expected, string(result), "Unexpected result in testcase %v", i) } } diff --git a/modules/markup/markdown/transform_image.go b/modules/markup/markdown/transform_image.go index 812e24f0a2..4ed4118854 100644 --- a/modules/markup/markdown/transform_image.go +++ b/modules/markup/markdown/transform_image.go @@ -21,7 +21,7 @@ func (g *ASTTransformer) transformImage(ctx *markup.RenderContext, v *ast.Image) // Check if the destination is a real link if len(v.Destination) > 0 && !markup.IsFullURLBytes(v.Destination) { v.Destination = []byte(giteautil.URLJoin( - ctx.Links.ResolveMediaLink(ctx.IsWiki), + ctx.Links.ResolveMediaLink(ctx.ContentMode == markup.RenderContentAsWiki), strings.TrimLeft(string(v.Destination), "/"), )) } diff --git a/modules/markup/orgmode/orgmode.go b/modules/markup/orgmode/orgmode.go index 25f8d15ef4..6b9c963157 100644 --- a/modules/markup/orgmode/orgmode.go +++ b/modules/markup/orgmode/orgmode.go @@ -144,14 +144,15 @@ func (r *Writer) resolveLink(kind, link string) string { } base := r.Ctx.Links.Base - if r.Ctx.IsWiki { + isWiki := r.Ctx.ContentMode == markup.RenderContentAsWiki + if isWiki { base = r.Ctx.Links.WikiLink() } else if r.Ctx.Links.HasBranchInfo() { base = r.Ctx.Links.SrcLink() } if kind == "image" || kind == "video" { - base = r.Ctx.Links.ResolveMediaLink(r.Ctx.IsWiki) + base = r.Ctx.Links.ResolveMediaLink(isWiki) } link = util.URLJoin(base, link) diff --git a/modules/markup/orgmode/orgmode_test.go b/modules/markup/orgmode/orgmode_test.go index 75b60ed81f..b882678c7e 100644 --- a/modules/markup/orgmode/orgmode_test.go +++ b/modules/markup/orgmode/orgmode_test.go @@ -10,6 +10,7 @@ import ( "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" ) @@ -26,7 +27,7 @@ func TestRender_StandardLinks(t *testing.T) { Base: "/relative-path", BranchPath: "branch/main", }, - IsWiki: isWiki, + ContentMode: util.Iif(isWiki, markup.RenderContentAsWiki, markup.RenderContentAsDefault), }, input) assert.NoError(t, err) assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(buffer)) diff --git a/modules/markup/render.go b/modules/markup/render.go index f2ce9229af..add50f4382 100644 --- a/modules/markup/render.go +++ b/modules/markup/render.go @@ -5,11 +5,9 @@ package markup import ( "context" - "errors" "fmt" "io" "net/url" - "path/filepath" "strings" "sync" @@ -29,15 +27,44 @@ const ( RenderMetaAsTable RenderMetaMode = "table" ) +type RenderContentMode string + +const ( + RenderContentAsDefault RenderContentMode = "" // empty means "default", no special handling, maybe just a simple "document" + RenderContentAsComment RenderContentMode = "comment" + RenderContentAsTitle RenderContentMode = "title" + RenderContentAsWiki RenderContentMode = "wiki" +) + +var RenderBehaviorForTesting struct { + // Markdown line break rendering has 2 default behaviors: + // * Use hard: replace "\n" with "
" for comments, setting.Markdown.EnableHardLineBreakInComments=true + // * Keep soft: "\n" for non-comments (a.k.a. documents), setting.Markdown.EnableHardLineBreakInDocuments=false + // In history, there was a mess: + // * The behavior was controlled by `Metas["mode"] != "document", + // * However, many places render the content without setting "mode" in Metas, all these places used comment line break setting incorrectly + ForceHardLineBreak bool + + // Gitea will emit some internal attributes for various purposes, these attributes don't affect rendering. + // But there are too many hard-coded test cases, to avoid changing all of them again and again, we can disable emitting these internal attributes. + DisableInternalAttributes bool +} + // 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 + Ctx context.Context + RelativePath string // relative path from tree root of the branch + + // eg: "orgmode", "asciicast", "console" + // for file mode, it could be left as empty, and will be detected by file extension in RelativePath + MarkupType string + + // what the content will be used for: eg: for comment or for wiki? or just render a file? + ContentMode RenderContentMode + + Links Links // special link references for rendering, especially when there is a branch/tree path + Metas map[string]string // user&repo, format&style®exp (for external issue pattern), teams&org (for mention), BranchNameSubURL(for iframe&asciicast) + DefaultLink string // TODO: need to figure out GitRepo *git.Repository Repo gitrepo.Repository ShaExistCache map[string]bool @@ -77,12 +104,29 @@ func (ctx *RenderContext) AddCancel(fn func()) { // 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) + if ctx.MarkupType == "" && ctx.RelativePath != "" { + ctx.MarkupType = DetectMarkupTypeByFileName(ctx.RelativePath) + if ctx.MarkupType == "" { + return util.NewInvalidArgumentErrorf("unsupported file to render: %q", ctx.RelativePath) + } } - return errors.New("render options both filename and type missing") + + renderer := renderers[ctx.MarkupType] + if renderer == nil { + return util.NewInvalidArgumentErrorf("unsupported markup type: %q", ctx.MarkupType) + } + + if ctx.RelativePath != "" { + if externalRender, ok := renderer.(ExternalRenderer); ok && externalRender.DisplayInIFrame() { + if !ctx.InStandalonePage { + // for an external "DisplayInIFrame" render, it could only output its content in a standalone page + // otherwise, a