Merge branch 'main' into lunny/fix_migration_redirect

Makefile

@@ -15,7 +15,7 @@ XGO_VERSION := go-1.25.x
 AIR_PACKAGE ?= github.com/air-verse/air@v1
 EDITORCONFIG_CHECKER_PACKAGE ?= github.com/editorconfig-checker/editorconfig-checker/v3/cmd/editorconfig-checker@v3
 GOFUMPT_PACKAGE ?= mvdan.cc/gofumpt@v0.9.2
-GOLANGCI_LINT_PACKAGE ?= github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.10.1
+GOLANGCI_LINT_PACKAGE ?= github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.11.2
 GXZ_PACKAGE ?= github.com/ulikunitz/xz/cmd/gxz@v0.5.15
 MISSPELL_PACKAGE ?= github.com/golangci/misspell/cmd/misspell@v0.8.0
 SWAGGER_PACKAGE ?= github.com/go-swagger/go-swagger/cmd/swagger@v0.33.1
@@ -155,6 +155,7 @@ GO_SOURCES := $(wildcard *.go)
 GO_SOURCES += $(shell find $(GO_DIRS) -type f -name "*.go")
 GO_SOURCES += $(GENERATED_GO_DEST)
 
+ESLINT_CONCURRENCY ?= 2
 
 SWAGGER_SPEC := templates/swagger/v1_json.tmpl
 SWAGGER_SPEC_INPUT := templates/swagger/v1_input.json
@@ -292,12 +293,12 @@ lint-backend-fix: lint-go-fix lint-go-gitea-vet lint-editorconfig ## lint backen
 
 .PHONY: lint-js
 lint-js: node_modules ## lint js and ts files
-	$(NODE_VARS) pnpm exec eslint --color --max-warnings=0 $(ESLINT_FILES)
+	$(NODE_VARS) pnpm exec eslint --color --max-warnings=0 --concurrency $(ESLINT_CONCURRENCY) $(ESLINT_FILES)
 	$(NODE_VARS) pnpm exec vue-tsc
 
 .PHONY: lint-js-fix
 lint-js-fix: node_modules ## lint js and ts files and fix issues
-	$(NODE_VARS) pnpm exec eslint --color --max-warnings=0 $(ESLINT_FILES) --fix
+	$(NODE_VARS) pnpm exec eslint --color --max-warnings=0 --concurrency $(ESLINT_CONCURRENCY) $(ESLINT_FILES) --fix
 	$(NODE_VARS) pnpm exec vue-tsc
 
 .PHONY: lint-css
@@ -368,11 +369,11 @@ lint-yaml: .venv ## lint yaml files
 
 .PHONY: lint-json
 lint-json: node_modules ## lint json files
-	$(NODE_VARS) pnpm exec eslint -c eslint.json.config.ts --color --max-warnings=0
+	$(NODE_VARS) pnpm exec eslint -c eslint.json.config.ts --color --max-warnings=0 --concurrency $(ESLINT_CONCURRENCY)
 
 .PHONY: lint-json-fix
 lint-json-fix: node_modules ## lint and fix json files
-	$(NODE_VARS) pnpm exec eslint -c eslint.json.config.ts --color --max-warnings=0 --fix
+	$(NODE_VARS) pnpm exec eslint -c eslint.json.config.ts --color --max-warnings=0 --concurrency $(ESLINT_CONCURRENCY) --fix
 
 .PHONY: watch
 watch: ## watch everything and continuously rebuild

go.mod

@@ -1,6 +1,6 @@
 module code.gitea.io/gitea
 
-go 1.26.0
+go 1.26.1
 
 // rfc5280 said: "The serial number is an integer assigned by the CA to each certificate."
 // But some CAs use negative serial number, just relax the check. related:

modules/markup/html_internal_test.go

@@ -23,12 +23,12 @@ const (
 
 // externalIssueLink an HTML link to an alphanumeric-style issue
 func externalIssueLink(baseURL, class, name string) string {
-	return link(util.URLJoin(baseURL, name), class, name)
+	return link(strings.TrimSuffix(baseURL, "/")+"/"+name, class, name)
 }
 
 // numericLink an HTML to a numeric-style issue
 func numericIssueLink(baseURL, class string, index int, marker string) string {
-	return link(util.URLJoin(baseURL, strconv.Itoa(index)), class, fmt.Sprintf("%s%d", marker, index))
+	return link(strings.TrimSuffix(baseURL, "/")+"/"+strconv.Itoa(index), class, fmt.Sprintf("%s%d", marker, index))
 }
 
 // link an HTML link
@@ -116,7 +116,7 @@ func TestRender_IssueIndexPattern2(t *testing.T) {
 
 		links := make([]any, len(indices))
 		for i, index := range indices {
-			links[i] = numericIssueLink(util.URLJoin("/test-owner/test-repo", path), "ref-issue", index, marker)
+			links[i] = numericIssueLink("/test-owner/test-repo/"+path, "ref-issue", index, marker)
 		}
 		expectedNil := fmt.Sprintf(expectedFmt, links...)
 		testRenderIssueIndexPattern(t, s, expectedNil, NewTestRenderContext(TestAppURL, localMetas))
@@ -210,7 +210,7 @@ func TestRender_IssueIndexPattern5(t *testing.T) {
 		metas["regexp"] = pattern
 		links := make([]any, len(ids))
 		for i, id := range ids {
-			links[i] = link(util.URLJoin("https://someurl.com/someUser/someRepo/", id), "ref-issue ref-external-issue", names[i])
+			links[i] = link("https://someurl.com/someUser/someRepo/"+id, "ref-issue ref-external-issue", names[i])
 		}
 
 		expected := fmt.Sprintf(expectedFmt, links...)
@@ -288,11 +288,11 @@ func TestRender_AutoLink(t *testing.T) {
 	}
 
 	// render valid issue URLs
-	test(util.URLJoin(TestRepoURL, "issues", "3333"),
-		numericIssueLink(util.URLJoin(TestRepoURL, "issues"), "ref-issue", 3333, "#"))
+	test(TestRepoURL+"issues/3333",
+		numericIssueLink(TestRepoURL+"issues", "ref-issue", 3333, "#"))
 
 	// render valid commit URLs
-	tmp := util.URLJoin(TestRepoURL, "commit", "d8a994ef243349f321568f9e36d5c3f444b99cae")
+	tmp := TestRepoURL + "commit/d8a994ef243349f321568f9e36d5c3f444b99cae"
 	test(tmp, "<a href=\""+tmp+"\" class=\"commit\"><code>d8a994ef24</code></a>")
 	tmp += "#diff-2"
 	test(tmp, "<a href=\""+tmp+"\" class=\"commit\"><code>d8a994ef24 (diff-2)</code></a>")

modules/markup/html_test.go

@@ -34,15 +34,15 @@ func TestRender_Commits(t *testing.T) {
 
 	sha := "65f1bf27bc3bf70f64657658635e66094edbcb4d"
 	repo := markup.TestAppURL + testRepoOwnerName + "/" + testRepoName + "/"
-	commit := util.URLJoin(repo, "commit", sha)
+	commit := repo + "commit/" + sha
 	commitPath := "/user13/repo11/commit/" + sha
-	tree := util.URLJoin(repo, "tree", sha, "src")
+	tree := repo + "tree/" + sha + "/src"
 
-	file := util.URLJoin(repo, "commit", sha, "example.txt")
+	file := repo + "commit/" + sha + "/example.txt"
 	fileWithExtra := file + ":"
 	fileWithHash := file + "#L2"
 	fileWithHasExtra := file + "#L2:"
-	commitCompare := util.URLJoin(repo, "compare", sha+"..."+sha)
+	commitCompare := repo + "compare/" + sha + "..." + sha
 	commitCompareWithHash := commitCompare + "#L2"
 
 	test(sha, `<p><a href="`+commitPath+`" rel="nofollow"><code>65f1bf27bc</code></a></p>`)
@@ -90,14 +90,14 @@ func TestRender_CrossReferences(t *testing.T) {
 		"/home/gitea/go-gitea/gitea#12345",
 		`<p>/home/gitea/go-gitea/gitea#12345</p>`)
 	test(
-		util.URLJoin(markup.TestAppURL, "gogitea", "gitea", "issues", "12345"),
-		`<p><a href="`+util.URLJoin(markup.TestAppURL, "gogitea", "gitea", "issues", "12345")+`" class="ref-issue" rel="nofollow">gogitea/gitea#12345</a></p>`)
+		markup.TestAppURL+"gogitea/gitea/issues/12345",
+		`<p><a href="`+markup.TestAppURL+`gogitea/gitea/issues/12345" class="ref-issue" rel="nofollow">gogitea/gitea#12345</a></p>`)
 	test(
-		util.URLJoin(markup.TestAppURL, "go-gitea", "gitea", "issues", "12345"),
-		`<p><a href="`+util.URLJoin(markup.TestAppURL, "go-gitea", "gitea", "issues", "12345")+`" class="ref-issue" rel="nofollow">go-gitea/gitea#12345</a></p>`)
+		markup.TestAppURL+"go-gitea/gitea/issues/12345",
+		`<p><a href="`+markup.TestAppURL+`go-gitea/gitea/issues/12345" class="ref-issue" rel="nofollow">go-gitea/gitea#12345</a></p>`)
 	test(
-		util.URLJoin(markup.TestAppURL, "gogitea", "some-repo-name", "issues", "12345"),
-		`<p><a href="`+util.URLJoin(markup.TestAppURL, "gogitea", "some-repo-name", "issues", "12345")+`" class="ref-issue" rel="nofollow">gogitea/some-repo-name#12345</a></p>`)
+		markup.TestAppURL+"gogitea/some-repo-name/issues/12345",
+		`<p><a href="`+markup.TestAppURL+`gogitea/some-repo-name/issues/12345" class="ref-issue" rel="nofollow">gogitea/some-repo-name#12345</a></p>`)
 
 	inputURL := setting.AppURL + "a/b/commit/0123456789012345678901234567890123456789/foo.txt?a=b#L2-L3"
 	test(
@@ -375,7 +375,7 @@ func TestRender_emoji(t *testing.T) {
 
 func TestRender_ShortLinks(t *testing.T) {
 	setting.AppURL = markup.TestAppURL
-	tree := util.URLJoin(markup.TestRepoURL, "src", "master")
+	tree := markup.TestRepoURL + "src/master"
 
 	test := func(input, expected string) {
 		buffer, err := markdown.RenderString(markup.NewTestRenderContext(tree), input)
@@ -383,15 +383,15 @@ func TestRender_ShortLinks(t *testing.T) {
 		assert.Equal(t, strings.TrimSpace(expected), strings.TrimSpace(string(buffer)))
 	}
 
-	url := util.URLJoin(tree, "Link")
-	otherURL := util.URLJoin(tree, "Other-Link")
-	encodedURL := util.URLJoin(tree, "Link%3F")
-	imgurl := util.URLJoin(tree, "Link.jpg")
-	otherImgurl := util.URLJoin(tree, "Link+Other.jpg")
-	encodedImgurl := util.URLJoin(tree, "Link+%23.jpg")
-	notencodedImgurl := util.URLJoin(tree, "some", "path", "Link%20#.jpg")
-	renderableFileURL := util.URLJoin(tree, "markdown_file.md")
-	unrenderableFileURL := util.URLJoin(tree, "file.zip")
+	url := tree + "/Link"
+	otherURL := tree + "/Other-Link"
+	encodedURL := tree + "/Link%3F"
+	imgurl := tree + "/Link.jpg"
+	otherImgurl := tree + "/Link+Other.jpg"
+	encodedImgurl := tree + "/Link+%23.jpg"
+	notencodedImgurl := tree + "/some/path/Link%20#.jpg"
+	renderableFileURL := tree + "/markdown_file.md"
+	unrenderableFileURL := tree + "/file.zip"
 	favicon := "http://google.com/favicon.ico"
 
 	test(

modules/svg/svg.go

@@ -8,13 +8,32 @@ import (
 	"html/template"
 	"path"
 	"strings"
+	"sync"
 
 	gitea_html "code.gitea.io/gitea/modules/htmlutil"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/public"
 )
 
-var svgIcons map[string]string
+type svgIconItem struct {
+	html    string
+	mocking bool
+}
+
+type svgCacheKey struct {
+	icon  string
+	size  int
+	class string
+}
+
+var (
+	svgIcons map[string]svgIconItem
+
+	svgCacheMu    sync.Mutex
+	svgCache      sync.Map
+	svgCacheCount int
+	svgCacheLimit = 10000
+)
 
 const defaultSize = 16
 
@@ -26,7 +45,7 @@ func Init() error {
 		return err
 	}
 
-	svgIcons = make(map[string]string, len(files))
+	svgIcons = make(map[string]svgIconItem, len(files))
 	for _, file := range files {
 		if path.Ext(file) != ".svg" {
 			continue
@@ -35,7 +54,7 @@ func Init() error {
 		if err != nil {
 			log.Error("Failed to read SVG file %s: %v", file, err)
 		} else {
-			svgIcons[file[:len(file)-4]] = string(Normalize(bs, defaultSize))
+			svgIcons[file[:len(file)-4]] = svgIconItem{html: string(Normalize(bs, defaultSize))}
 		}
 	}
 	return nil
@@ -43,10 +62,13 @@ func Init() error {
 
 func MockIcon(icon string) func() {
 	if svgIcons == nil {
-		svgIcons = make(map[string]string)
+		svgIcons = make(map[string]svgIconItem)
 	}
 	orig, exist := svgIcons[icon]
-	svgIcons[icon] = fmt.Sprintf(`<svg class="svg %s" width="%d" height="%d"></svg>`, icon, defaultSize, defaultSize)
+	svgIcons[icon] = svgIconItem{
+		html:    fmt.Sprintf(`<svg class="svg %s" width="%d" height="%d"></svg>`, icon, defaultSize, defaultSize),
+		mocking: true,
+	}
 	return func() {
 		if exist {
 			svgIcons[icon] = orig
@@ -58,11 +80,28 @@ func MockIcon(icon string) func() {
 
 // RenderHTML renders icons - arguments icon name (string), size (int), class (string)
 func RenderHTML(icon string, others ...any) template.HTML {
+	result, _ := renderHTML(icon, others...)
+	return result
+}
+
+func renderHTML(icon string, others ...any) (_ template.HTML, usingCache bool) {
 	if icon == "" {
-		return ""
+		return "", false
 	}
 	size, class := gitea_html.ParseSizeAndClass(defaultSize, "", others...)
-	if svgStr, ok := svgIcons[icon]; ok {
+	if svgItem, ok := svgIcons[icon]; ok {
+		svgStr := svgItem.html
+		// fast path for default size and no classes
+		if size == defaultSize && class == "" {
+			return template.HTML(svgStr), false
+		}
+
+		cacheKey := svgCacheKey{icon, size, class}
+		cachedHTML, cached := svgCache.Load(cacheKey)
+		if cached && !svgItem.mocking {
+			return cachedHTML.(template.HTML), true
+		}
+
 		// the code is somewhat hacky, but it just works, because the SVG contents are all normalized
 		if size != defaultSize {
 			svgStr = strings.Replace(svgStr, fmt.Sprintf(`width="%d"`, defaultSize), fmt.Sprintf(`width="%d"`, size), 1)
@@ -71,8 +110,24 @@ func RenderHTML(icon string, others ...any) template.HTML {
 		if class != "" {
 			svgStr = strings.Replace(svgStr, `class="`, fmt.Sprintf(`class="%s `, class), 1)
 		}
-		return template.HTML(svgStr)
+		result := template.HTML(svgStr)
+
+		if !svgItem.mocking {
+			// no need to double-check, the rendering is fast enough and the cache is just an optimization
+			svgCacheMu.Lock()
+			if svgCacheCount >= svgCacheLimit {
+				svgCache.Clear()
+				svgCacheCount = 0
+			}
+			svgCacheCount++
+			svgCache.Store(cacheKey, result)
+			svgCacheMu.Unlock()
+		}
+
+		return result, false
 	}
+
 	// during test (or something wrong happens), there is no SVG loaded, so use a dummy span to tell that the icon is missing
-	return template.HTML(fmt.Sprintf("<span>%s(%d/%s)</span>", template.HTMLEscapeString(icon), size, template.HTMLEscapeString(class)))
+	dummy := template.HTML(fmt.Sprintf("<span>%s(%d/%s)</span>", template.HTMLEscapeString(icon), size, template.HTMLEscapeString(class)))
+	return dummy, false
 }

modules/svg/svg_test.go

@@ -0,0 +1,54 @@
+// Copyright 2026 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package svg
+
+import (
+	"testing"
+
+	"code.gitea.io/gitea/modules/test"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestRenderHTMLCache(t *testing.T) {
+	const svgRealContent = "RealContent"
+	svgIcons = map[string]svgIconItem{
+		"test": {html: `<svg class="svg test" width="16" height="16">` + svgRealContent + `</svg>`},
+	}
+
+	// default params: no cache entry
+	_, usingCache := renderHTML("test")
+	assert.False(t, usingCache)
+	_, usingCache = renderHTML("test")
+	assert.False(t, usingCache)
+
+	// non-default params: cached
+	_, usingCache = renderHTML("test", 24)
+	assert.False(t, usingCache)
+	_, usingCache = renderHTML("test", 24)
+	assert.True(t, usingCache)
+
+	// mocked svg shouldn't be cached
+	revertMock := MockIcon("test")
+	mockedHTML, usingCache := renderHTML("test", 24)
+	assert.False(t, usingCache)
+	assert.NotContains(t, mockedHTML, svgRealContent)
+	revertMock()
+	realHTML, usingCache := renderHTML("test", 24)
+	assert.True(t, usingCache)
+	assert.Contains(t, realHTML, svgRealContent)
+
+	t.Run("CacheWithLimit", func(t *testing.T) {
+		assert.NotZero(t, svgCacheCount)
+		const testLimit = 3
+		defer test.MockVariableValue(&svgCacheLimit, testLimit)()
+		for i := range 10 {
+			_, usingCache = renderHTML("test", 100+i)
+			assert.False(t, usingCache)
+			_, usingCache = renderHTML("test", 100+i)
+			assert.True(t, usingCache)
+			assert.LessOrEqual(t, svgCacheCount, testLimit)
+		}
+	})
+}

modules/util/url.go

@@ -5,7 +5,6 @@ package util
 
 import (
 	"net/url"
-	"path"
 	"strings"
 )
 
@@ -19,29 +18,6 @@ func PathEscapeSegments(path string) string {
 	return escapedPath
 }
 
-// URLJoin joins url components, like path.Join, but preserving contents
-// Deprecated: it has unclear behaviors, should not be used anymore. It is only used in some tests.
-// Need to be removed in the future.
-func URLJoin(base string, elems ...string) string {
-	if !strings.HasSuffix(base, "/") {
-		base += "/"
-	}
-	baseURL, err := url.Parse(base)
-	if err != nil {
-		return ""
-	}
-	joinedPath := path.Join(elems...)
-	argURL, err := url.Parse(joinedPath)
-	if err != nil {
-		return ""
-	}
-	joinedURL := baseURL.ResolveReference(argURL).String()
-	if !baseURL.IsAbs() && !strings.HasPrefix(base, "/") {
-		return joinedURL[1:] // Removing leading '/' if needed
-	}
-	return joinedURL
-}
-
 func SanitizeURL(s string) (string, error) {
 	u, err := url.Parse(s)
 	if err != nil {

modules/util/util_test.go

@@ -11,39 +11,6 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-func TestURLJoin(t *testing.T) {
-	type test struct {
-		Expected string
-		Base     string
-		Elements []string
-	}
-	newTest := func(expected, base string, elements ...string) test {
-		return test{Expected: expected, Base: base, Elements: elements}
-	}
-	for _, test := range []test{
-		newTest("https://try.gitea.io/a/b/c",
-			"https://try.gitea.io", "a/b", "c"),
-		newTest("https://try.gitea.io/a/b/c",
-			"https://try.gitea.io/", "/a/b/", "/c/"),
-		newTest("https://try.gitea.io/a/c",
-			"https://try.gitea.io/", "/a/./b/", "../c/"),
-		newTest("a/b/c",
-			"a", "b/c/"),
-		newTest("a/b/d",
-			"a/", "b/c/", "/../d/"),
-		newTest("https://try.gitea.io/a/b/c#d",
-			"https://try.gitea.io", "a/b", "c#d"),
-		newTest("/a/b/d",
-			"/a/", "b/c/", "/../d/"),
-		newTest("/a/b/c",
-			"/a", "b/c/"),
-		newTest("/a/b/c#hash",
-			"/a", "b/c#hash"),
-	} {
-		assert.Equal(t, test.Expected, URLJoin(test.Base, test.Elements...))
-	}
-}
-
 func TestIsEmptyString(t *testing.T) {
 	cases := []struct {
 		s        string

services/convert/git_commit_test.go

@@ -11,7 +11,6 @@ import (
 	"code.gitea.io/gitea/models/unittest"
 	"code.gitea.io/gitea/modules/git"
 	api "code.gitea.io/gitea/modules/structs"
-	"code.gitea.io/gitea/modules/util"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -35,7 +34,7 @@ func TestToCommitMeta(t *testing.T) {
 	assert.NotNil(t, commitMeta)
 	assert.Equal(t, &api.CommitMeta{
 		SHA:     sha1.EmptyObjectID().String(),
-		URL:     util.URLJoin(headRepo.APIURL(), "git/commits", sha1.EmptyObjectID().String()),
+		URL:     headRepo.APIURL() + "/git/commits/" + sha1.EmptyObjectID().String(),
 		Created: time.Unix(0, 0),
 	}, commitMeta)
 }

services/gitdiff/gitdiff.go

@@ -1477,38 +1477,40 @@ func SyncUserSpecificDiff(ctx context.Context, userID int64, pull *issues_model.
 	if errIgnored != nil {
 		log.Error("Could not get changed files between %s and %s for pull request %d in repo with path %s. Assuming no changes. Error: %w", review.CommitSHA, latestCommit, pull.Index, gitRepo.Path, err)
 	}
+	changedFilesSet := make(map[string]struct{}, len(changedFiles))
+	for _, changedFile := range changedFiles {
+		changedFilesSet[changedFile] = struct{}{}
+	}
 
 	filesChangedSinceLastDiff := make(map[string]pull_model.ViewedState)
-outer:
 	for _, diffFile := range diff.Files {
-		fileViewedState := review.UpdatedFiles[diffFile.GetDiffFileName()]
-
-		// Check whether it was previously detected that the file has changed since the last review
-		if fileViewedState == pull_model.HasChanged {
-			diffFile.HasChangedSinceLastReview = true
-			continue
-		}
-
 		filename := diffFile.GetDiffFileName()
+		fileViewedState := review.UpdatedFiles[filename]
 
-		// Check explicitly whether the file has changed since the last review
-		for _, changedFile := range changedFiles {
-			diffFile.HasChangedSinceLastReview = filename == changedFile
-			if diffFile.HasChangedSinceLastReview {
-				filesChangedSinceLastDiff[filename] = pull_model.HasChanged
-				continue outer // We don't want to check if the file is viewed here as that would fold the file, which is in this case unwanted
-			}
-		}
-		// Check whether the file has already been viewed
-		if fileViewedState == pull_model.Viewed {
+		if fileViewedState == pull_model.HasChanged { // Check whether it was previously detected that the file has changed since the last review
+			diffFile.HasChangedSinceLastReview = true
+			delete(changedFilesSet, filename)
+		} else if _, ok := changedFilesSet[filename]; ok { // Check explicitly whether the file has changed since the last review
+			diffFile.HasChangedSinceLastReview = true
+			filesChangedSinceLastDiff[filename] = pull_model.HasChanged
+			delete(changedFilesSet, filename)
+		} else if fileViewedState == pull_model.Viewed { // Check whether the file has already been viewed
 			diffFile.IsViewed = true
 		}
 	}
 
+	// All changed files still present at this point aren't part of the diff anymore, this occurs
+	// when a file was modified in a previous commit of the diff and the modification got reverted afterwards.
+	// Marking the files as unviewed to prevent errors where a non-existing file has a view state
+	for changedFile := range changedFilesSet {
+		if _, ok := review.UpdatedFiles[changedFile]; ok {
+			filesChangedSinceLastDiff[changedFile] = pull_model.Unviewed
+		}
+	}
+
 	if len(filesChangedSinceLastDiff) > 0 {
 		// Explicitly store files that have changed in the database, if any is present at all.
 		// This has the benefit that the "Has Changed" attribute will be present as long as the user does not explicitly mark this file as viewed, so it will even survive a page reload after marking another file as viewed.
-		// On the other hand, this means that even if a commit reverting an unseen change is committed, the file will still be seen as changed.
 		updatedReview, err := pull_model.UpdateReviewState(ctx, review.UserID, review.PullID, review.CommitSHA, filesChangedSinceLastDiff)
 		if err != nil {
 			log.Warn("Could not update review for user %d, pull %d, commit %s and the changed files %v: %v", review.UserID, review.PullID, review.CommitSHA, filesChangedSinceLastDiff, err)

services/gitdiff/gitdiff_test.go

@@ -11,6 +11,7 @@ import (
 	"testing"
 
 	issues_model "code.gitea.io/gitea/models/issues"
+	pull_model "code.gitea.io/gitea/models/pull"
 	"code.gitea.io/gitea/models/unittest"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/git"
@@ -1143,3 +1144,96 @@ func TestHighlightCodeLines(t *testing.T) {
 		}, ret)
 	})
 }
+
+func TestSyncUserSpecificDiff_UpdatedFiles(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+
+	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})
+	pull := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 7})
+	assert.NoError(t, pull.LoadBaseRepo(t.Context()))
+
+	stdin := `blob
+mark :1
+data 7
+change
+
+commit refs/heads/branch1
+mark :2
+committer test <test@example.com> 1772749114 +0000
+data 7
+change
+from 1978192d98bb1b65e11c2cf37da854fbf94bffd6
+M 100644 :1 test2.txt
+M 100644 :1 test3.txt
+
+commit refs/heads/branch1
+committer test <test@example.com> 1772749114 +0000
+data 7
+revert
+from :2
+D test2.txt
+D test10.txt`
+	require.NoError(t, gitcmd.NewCommand("fast-import").WithDir(pull.BaseRepo.RepoPath()).WithStdinBytes([]byte(stdin)).Run(t.Context()))
+
+	gitRepo, err := git.OpenRepository(t.Context(), pull.BaseRepo.RepoPath())
+	assert.NoError(t, err)
+	defer gitRepo.Close()
+
+	firstReviewCommit := "1978192d98bb1b65e11c2cf37da854fbf94bffd6"
+	firstReviewUpdatedFiles := map[string]pull_model.ViewedState{
+		"test1.txt":  pull_model.Viewed,
+		"test2.txt":  pull_model.Viewed,
+		"test10.txt": pull_model.Viewed,
+	}
+	_, err = pull_model.UpdateReviewState(t.Context(), user.ID, pull.ID, firstReviewCommit, firstReviewUpdatedFiles)
+	assert.NoError(t, err)
+	firstReview, err := pull_model.GetNewestReviewState(t.Context(), user.ID, pull.ID)
+	assert.NoError(t, err)
+	assert.NotNil(t, firstReview)
+	assert.Equal(t, firstReviewUpdatedFiles, firstReview.UpdatedFiles)
+	assert.Equal(t, 3, firstReview.GetViewedFileCount())
+
+	secondReviewCommit := "f80737c7dc9de0a9c1e051e83cb6897f950c6bb8"
+	secondReviewUpdatedFiles := map[string]pull_model.ViewedState{
+		"test1.txt":  pull_model.Viewed,
+		"test2.txt":  pull_model.HasChanged,
+		"test3.txt":  pull_model.HasChanged,
+		"test10.txt": pull_model.Viewed,
+	}
+	secondReviewDiffOpts := &DiffOptions{
+		AfterCommitID:     secondReviewCommit,
+		BeforeCommitID:    pull.MergeBase,
+		MaxLines:          setting.Git.MaxGitDiffLines,
+		MaxLineCharacters: setting.Git.MaxGitDiffLineCharacters,
+		MaxFiles:          setting.Git.MaxGitDiffFiles,
+	}
+	secondReviewDiff, err := GetDiffForAPI(t.Context(), gitRepo, secondReviewDiffOpts)
+	assert.NoError(t, err)
+	secondReview, err := SyncUserSpecificDiff(t.Context(), user.ID, pull, gitRepo, secondReviewDiff, secondReviewDiffOpts)
+	assert.NoError(t, err)
+	assert.NotNil(t, secondReview)
+	assert.Equal(t, secondReviewUpdatedFiles, secondReview.UpdatedFiles)
+	assert.Equal(t, 2, secondReview.GetViewedFileCount())
+
+	thirdReviewCommit := "73424f3a99e140f6399c73a1712654e122d2a74b"
+	thirdReviewUpdatedFiles := map[string]pull_model.ViewedState{
+		"test1.txt":  pull_model.Viewed,
+		"test2.txt":  pull_model.Unviewed,
+		"test3.txt":  pull_model.HasChanged,
+		"test10.txt": pull_model.Unviewed,
+	}
+	thirdReviewDiffOpts := &DiffOptions{
+		AfterCommitID:     thirdReviewCommit,
+		BeforeCommitID:    pull.MergeBase,
+		MaxLines:          setting.Git.MaxGitDiffLines,
+		MaxLineCharacters: setting.Git.MaxGitDiffLineCharacters,
+		MaxFiles:          setting.Git.MaxGitDiffFiles,
+	}
+	thirdReviewDiff, err := GetDiffForAPI(t.Context(), gitRepo, thirdReviewDiffOpts)
+	assert.NoError(t, err)
+	thirdReview, err := SyncUserSpecificDiff(t.Context(), user.ID, pull, gitRepo, thirdReviewDiff, thirdReviewDiffOpts)
+	assert.NoError(t, err)
+	assert.NotNil(t, thirdReview)
+	assert.Equal(t, thirdReviewUpdatedFiles, thirdReview.UpdatedFiles)
+	assert.Equal(t, 1, thirdReview.GetViewedFileCount())
+}

tests/integration/api_packages_conan_test.go

@@ -346,15 +346,16 @@ func TestPackageConan(t *testing.T) {
 					pb, err := packages.GetBlobByID(t.Context(), pf.BlobID)
 					assert.NoError(t, err)
 
-					if pf.Name == conanfileName {
+					switch pf.Name {
+					case conanfileName:
 						assert.True(t, pf.IsLead)
 
 						assert.Equal(t, int64(len(buildConanfileContent(name, version1))), pb.Size)
-					} else if pf.Name == conaninfoName {
+					case conaninfoName:
 						assert.False(t, pf.IsLead)
 
 						assert.Equal(t, int64(len(contentConaninfo)), pb.Size)
-					} else {
+					default:
 						assert.FailNow(t, "unknown file", "unknown file: %s", pf.Name)
 					}
 				}

tests/integration/api_packages_generic_test.go

@@ -140,11 +140,12 @@ func TestPackageGeneric(t *testing.T) {
 		t.Run("ServeDirect", func(t *testing.T) {
 			defer tests.PrintCurrentTest(t)()
 
-			if setting.Packages.Storage.Type == setting.MinioStorageType {
+			switch setting.Packages.Storage.Type {
+			case setting.MinioStorageType:
 				defer test.MockVariableValue(&setting.Packages.Storage.MinioConfig.ServeDirect, true)()
-			} else if setting.Packages.Storage.Type == setting.AzureBlobStorageType {
+			case setting.AzureBlobStorageType:
 				defer test.MockVariableValue(&setting.Packages.Storage.AzureBlobConfig.ServeDirect, true)()
-			} else {
+			default:
 				t.Skip("Test skipped for non-Minio-storage and non-AzureBlob-storage.")
 			}
 

tests/integration/api_packages_rpm_test.go

@@ -20,7 +20,6 @@ import (
 	user_model "code.gitea.io/gitea/models/user"
 	rpm_module "code.gitea.io/gitea/modules/packages/rpm"
 	"code.gitea.io/gitea/modules/setting"
-	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/tests"
 
 	"github.com/ProtonMail/go-crypto/openpgp"
@@ -99,7 +98,7 @@ gpgcheck=1
 gpgkey=%sapi/packages/%s/rpm/repository.key`,
 					strings.Join(append([]string{user.LowerName}, groupParts...), "-"),
 					strings.Join(append([]string{user.Name, setting.AppName}, groupParts...), " - "),
-					util.URLJoin(setting.AppURL, groupURL),
+					strings.TrimSuffix(setting.AppURL, "/")+groupURL,
 					setting.AppURL,
 					user.Name,
 				)

tests/integration/api_releases_test.go

@@ -422,7 +422,7 @@ func TestAPIUploadAssetRelease(t *testing.T) {
 		defer tests.PrintCurrentTest(t)()
 		const filename = "image.png"
 
-		performUpload := func(t *testing.T, uploadURL string, buf []byte, expectedStatus int) *httptest.ResponseRecorder {
+		performUpload := func(t *testing.T, uploadURL string, _ []byte, _ int) *httptest.ResponseRecorder {
 			body := &bytes.Buffer{}
 			writer := multipart.NewWriter(body)
 			part, err := writer.CreateFormFile("attachment", filename)

tests/integration/api_repo_get_contents_test.go

@@ -60,7 +60,7 @@ func TestAPIGetContents(t *testing.T) {
 	})
 }
 
-func testAPIGetContents(t *testing.T, u *url.URL) {
+func testAPIGetContents(t *testing.T, _ *url.URL) {
 	/*** SETUP ***/
 	user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})         // owner of the repo1 & repo16
 	org3 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 3})          // owner of the repo3, is an org

tests/integration/api_repo_git_tags_test.go

@@ -14,7 +14,6 @@ import (
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/gitrepo"
 	api "code.gitea.io/gitea/modules/structs"
-	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/tests"
 
 	"github.com/stretchr/testify/assert"
@@ -58,7 +57,7 @@ func TestAPIGitTags(t *testing.T) {
 	assert.Equal(t, aTagMessage+"\n", tag.Message)
 	assert.Equal(t, user.Name, tag.Tagger.Name)
 	assert.Equal(t, user.Email, tag.Tagger.Email)
-	assert.Equal(t, util.URLJoin(repo.APIURL(), "git/tags", aTag.ID.String()), tag.URL)
+	assert.Equal(t, repo.APIURL()+"/git/tags/"+aTag.ID.String(), tag.URL)
 
 	// Should NOT work for lightweight tags
 	badReq := NewRequestf(t, "GET", "/api/v1/repos/%s/%s/git/tags/%s", user.Name, repo.Name, commit.ID.String()).

tests/integration/api_token_test.go

@@ -502,11 +502,12 @@ func runTestCase(t *testing.T, testCase *requiredScopeTestCase, user *user_model
 			}
 			unauthorizedLevel := auth_model.Write
 			if categoryIsRequired {
-				if minRequiredLevel == auth_model.Read {
+				switch minRequiredLevel {
+				case auth_model.Read:
 					unauthorizedLevel = auth_model.NoAccess
-				} else if minRequiredLevel == auth_model.Write {
+				case auth_model.Write:
 					unauthorizedLevel = auth_model.Read
-				} else {
+				default:
 					assert.FailNow(t, "Invalid test case", "Unknown access token scope level: %v", minRequiredLevel)
 				}
 			}

tests/integration/pull_comment_test.go

@@ -74,7 +74,7 @@ func testPullCommentRebase(t *testing.T, u *url.URL, session *TestSession) {
 	assert.True(t, lastComment.IsForcePush)
 }
 
-func testPullCommentRetarget(t *testing.T, u *url.URL, session *TestSession) {
+func testPullCommentRetarget(t *testing.T, _ *url.URL, session *TestSession) {
 	testPRTitle := "Test PR for retarget comment"
 	// keep a non-conflict branch
 	testCreateBranch(t, session, "user2", "repo1", "branch/test-branch/retarget", "test-branch/retarget-no-conflict", http.StatusSeeOther)

tests/integration/repo_test.go

@@ -92,9 +92,10 @@ func testViewRepoWithCache(t *testing.T) {
 			tds := s.Find(".repo-file-cell")
 			var f file
 			tds.Each(func(i int, s *goquery.Selection) {
-				if i == 0 {
+				switch i {
+				case 0:
 					f.fileName = strings.TrimSpace(s.Text())
-				} else if i == 1 {
+				case 1:
 					a := s.Find("a")
 					f.commitMsg = strings.TrimSpace(a.Text())
 					l, _ := a.Attr("href")

web_src/js/features/pull-view-file.ts

@@ -9,13 +9,13 @@ const viewedCheckboxSelector = '.viewed-file-form'; // Selector under which all
 const expandFilesBtnSelector = '#expand-files-btn';
 const collapseFilesBtnSelector = '#collapse-files-btn';
 
-// Refreshes the summary of viewed files if present
+// Refreshes the summary of viewed files
 // The data used will be window.config.pageData.prReview.numberOf{Viewed}Files
 function refreshViewedFilesSummary() {
-  const viewedFilesProgress = document.querySelector('#viewed-files-summary');
-  viewedFilesProgress?.setAttribute('value', prReview.numberOfViewedFiles);
-  const summaryLabel = document.querySelector('#viewed-files-summary-label')!;
-  if (summaryLabel) summaryLabel.innerHTML = summaryLabel.getAttribute('data-text-changed-template')!
+  const viewedFilesProgress = document.querySelector('#viewed-files-summary')!;
+  viewedFilesProgress.setAttribute('value', prReview.numberOfViewedFiles);
+  const summaryLabel = document.querySelector<HTMLElement>('#viewed-files-summary-label')!;
+  summaryLabel.textContent = summaryLabel.getAttribute('data-text-changed-template')!
     .replace('%[1]d', prReview.numberOfViewedFiles)
     .replace('%[2]d', prReview.numberOfFiles);
 }