From d21ce9fa0739a954e2ecbc5d2aa1e324b5781a4b Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 13 Jun 2025 02:12:45 +0800 Subject: [PATCH] Improve the performance when detecting the file editable (#34653) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Noticed the SQL will be executed 4 times when visit the file render view page. For a repository which have many pull requests, it maybe slow. ```SQL 2025/06/08 15:24:44 models/issues/pull_list.go:69:GetUnmergedPullRequestsByHeadInfo() [I] [SQL] SELECT * FROM `pull_request` INNER JOIN `issue` ON issue.id = pull_request.issue_id WHERE (head_repo_id = ? AND head_branch = ? AND has_merged = ? AND issue.is_closed = ? AND flow = ?) [393 main false false 0] - 2.004167ms 2025/06/08 15:24:44 models/issues/pull_list.go:69:GetUnmergedPullRequestsByHeadInfo() [I] [SQL] SELECT * FROM `pull_request` INNER JOIN `issue` ON issue.id = pull_request.issue_id WHERE (head_repo_id = ? AND head_branch = ? AND has_merged = ? AND issue.is_closed = ? AND flow = ?) [393 main false false 0] - 1.03975ms 2025/06/08 15:24:44 models/issues/pull_list.go:69:GetUnmergedPullRequestsByHeadInfo() [I] [SQL] SELECT * FROM `pull_request` INNER JOIN `issue` ON issue.id = pull_request.issue_id WHERE (head_repo_id = ? AND head_branch = ? AND has_merged = ? AND issue.is_closed = ? AND flow = ?) [393 main false false 0] - 881.583µs 2025/06/08 15:24:44 models/issues/pull_list.go:69:GetUnmergedPullRequestsByHeadInfo() [I] [SQL] SELECT * FROM `pull_request` INNER JOIN `issue` ON issue.id = pull_request.issue_id WHERE (head_repo_id = ? AND head_branch = ? AND has_merged = ? AND issue.is_closed = ? AND flow = ?) [393 main false false 0] - 935.084µs ``` This PR did a refactor to query it once only. --- routers/web/repo/view_file.go | 80 ++++++++++++++++++++--------------- services/context/repo.go | 5 ++- 2 files changed, 48 insertions(+), 37 deletions(-) diff --git a/routers/web/repo/view_file.go b/routers/web/repo/view_file.go index 3df6051975..f43433fb0d 100644 --- a/routers/web/repo/view_file.go +++ b/routers/web/repo/view_file.go @@ -140,13 +140,6 @@ func prepareToRenderFile(ctx *context.Context, entry *git.TreeEntry) { ctx.Data["LFSLockHint"] = ctx.Tr("repo.editor.this_file_locked") } - // Assume file is not editable first. - if fInfo.isLFSFile { - ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.cannot_edit_lfs_files") - } else if !isRepresentableAsText { - ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.cannot_edit_non_text_files") - } - // read all needed attributes which will be used later // there should be no performance different between reading 2 or 4 here attrsMap, err := attribute.CheckAttributes(ctx, ctx.Repo.GitRepo, ctx.Repo.CommitID, attribute.CheckAttributeOpts{ @@ -243,21 +236,6 @@ func prepareToRenderFile(ctx *context.Context, entry *git.TreeEntry) { ctx.Data["FileContent"] = fileContent ctx.Data["LineEscapeStatus"] = statuses } - if !fInfo.isLFSFile { - if ctx.Repo.CanEnableEditor(ctx, ctx.Doer) { - if lfsLock != nil && lfsLock.OwnerID != ctx.Doer.ID { - ctx.Data["CanEditFile"] = false - ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.this_file_locked") - } else { - ctx.Data["CanEditFile"] = true - ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.edit_this_file") - } - } else if !ctx.Repo.RefFullName.IsBranch() { - ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.must_be_on_a_branch") - } else if !ctx.Repo.CanWriteToBranch(ctx, ctx.Doer, ctx.Repo.BranchName) { - ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.fork_before_edit") - } - } case fInfo.st.IsPDF(): ctx.Data["IsPDFFile"] = true @@ -307,17 +285,49 @@ func prepareToRenderFile(ctx *context.Context, entry *git.TreeEntry) { } } - if ctx.Repo.CanEnableEditor(ctx, ctx.Doer) { - if lfsLock != nil && lfsLock.OwnerID != ctx.Doer.ID { - ctx.Data["CanDeleteFile"] = false - ctx.Data["DeleteFileTooltip"] = ctx.Tr("repo.editor.this_file_locked") - } else { - ctx.Data["CanDeleteFile"] = true - ctx.Data["DeleteFileTooltip"] = ctx.Tr("repo.editor.delete_this_file") - } - } else if !ctx.Repo.RefFullName.IsBranch() { - ctx.Data["DeleteFileTooltip"] = ctx.Tr("repo.editor.must_be_on_a_branch") - } else if !ctx.Repo.CanWriteToBranch(ctx, ctx.Doer, ctx.Repo.BranchName) { - ctx.Data["DeleteFileTooltip"] = ctx.Tr("repo.editor.must_have_write_access") - } + prepareToRenderButtons(ctx, fInfo.isLFSFile, isRepresentableAsText, lfsLock) +} + +func prepareToRenderButtons(ctx *context.Context, isLFSFile, isRepresentableAsText bool, lfsLock *git_model.LFSLock) { + // archived or mirror repository, the buttons should not be shown + if ctx.Repo.Repository.IsArchived || !ctx.Repo.Repository.CanEnableEditor() { + return + } + + // The buttons should not be shown if it's not a branch + if !ctx.Repo.RefFullName.IsBranch() { + ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.must_be_on_a_branch") + ctx.Data["DeleteFileTooltip"] = ctx.Tr("repo.editor.must_be_on_a_branch") + return + } + + if isLFSFile { + ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.cannot_edit_lfs_files") + } else if !isRepresentableAsText { + ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.cannot_edit_non_text_files") + } + + if !ctx.Repo.CanWriteToBranch(ctx, ctx.Doer, ctx.Repo.BranchName) { + if !isLFSFile { // lfs file cannot be edited after fork + ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.fork_before_edit") + } + ctx.Data["DeleteFileTooltip"] = ctx.Tr("repo.editor.must_have_write_access") + return + } + + // it's a lfs file and the user is not the owner of the lock + if lfsLock != nil && lfsLock.OwnerID != ctx.Doer.ID { + ctx.Data["CanEditFile"] = false + ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.this_file_locked") + ctx.Data["CanDeleteFile"] = false + ctx.Data["DeleteFileTooltip"] = ctx.Tr("repo.editor.this_file_locked") + return + } + + if !isLFSFile { // lfs file cannot be edited + ctx.Data["CanEditFile"] = true + ctx.Data["EditFileTooltip"] = ctx.Tr("repo.editor.edit_this_file") + } + ctx.Data["CanDeleteFile"] = true + ctx.Data["DeleteFileTooltip"] = ctx.Tr("repo.editor.delete_this_file") } diff --git a/services/context/repo.go b/services/context/repo.go index dafa398dc8..32d54c88ff 100644 --- a/services/context/repo.go +++ b/services/context/repo.go @@ -123,7 +123,8 @@ func (r *Repository) CanCommitToBranch(ctx context.Context, doer *user_model.Use sign, keyID, _, err := asymkey_service.SignCRUDAction(ctx, r.Repository.RepoPath(), doer, r.Repository.RepoPath(), git.BranchPrefix+r.BranchName) - canCommit := r.CanEnableEditor(ctx, doer) && userCanPush + canEnableEditor := r.CanEnableEditor(ctx, doer) + canCommit := canEnableEditor && userCanPush if requireSigned { canCommit = canCommit && sign } @@ -139,7 +140,7 @@ func (r *Repository) CanCommitToBranch(ctx context.Context, doer *user_model.Use return CanCommitToBranchResults{ CanCommitToBranch: canCommit, - EditorEnabled: r.CanEnableEditor(ctx, doer), + EditorEnabled: canEnableEditor, UserCanPush: userCanPush, RequireSigned: requireSigned, WillSign: sign,