Merge pull request '[gitea] week 2024-34 cherry pick (gitea/main -> forgejo)' (#4998) from earl-warren/wcp/2024-34 into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/4998
Reviewed-by: Michael Kriese <michael.kriese@gmx.de>

models/asymkey/ssh_key.go

@@ -229,35 +229,26 @@ func UpdatePublicKeyUpdated(ctx context.Context, id int64) error {
 
 // PublicKeysAreExternallyManaged returns whether the provided KeyID represents an externally managed Key
 func PublicKeysAreExternallyManaged(ctx context.Context, keys []*PublicKey) ([]bool, error) {
-	sources := make([]*auth.Source, 0, 5)
+	sourceCache := make(map[int64]*auth.Source, len(keys))
 	externals := make([]bool, len(keys))
-keyloop:
+
 	for i, key := range keys {
 		if key.LoginSourceID == 0 {
 			externals[i] = false
-			continue keyloop
+			continue
 		}
 
-		var source *auth.Source
+		source, ok := sourceCache[key.LoginSourceID]
-
+		if !ok {
-	sourceloop:
-		for _, s := range sources {
-			if s.ID == key.LoginSourceID {
-				source = s
-				break sourceloop
-			}
-		}
-
-		if source == nil {
 			var err error
 			source, err = auth.GetSourceByID(ctx, key.LoginSourceID)
 			if err != nil {
 				if auth.IsErrSourceNotExist(err) {
 					externals[i] = false
-					sources[i] = &auth.Source{
+					sourceCache[key.LoginSourceID] = &auth.Source{
 						ID: key.LoginSourceID,
 					}
-					continue keyloop
+					continue
 				}
 				return nil, err
 			}

models/asymkey/ssh_key_test.go

@@ -12,6 +12,8 @@ import (
 	"strings"
 	"testing"
 
+	"code.gitea.io/gitea/models/db"
+	"code.gitea.io/gitea/models/unittest"
 	"code.gitea.io/gitea/modules/setting"
 
 	"github.com/42wim/sshsig"
@@ -501,3 +503,11 @@ func runErr(t *testing.T, stdin []byte, args ...string) {
 		t.Fatal("expected error")
 	}
 }
+
+func Test_PublicKeysAreExternallyManaged(t *testing.T) {
+	key1 := unittest.AssertExistsAndLoadBean(t, &PublicKey{ID: 1})
+	externals, err := PublicKeysAreExternallyManaged(db.DefaultContext, []*PublicKey{key1})
+	require.NoError(t, err)
+	assert.Len(t, externals, 1)
+	assert.False(t, externals[0])
+}

models/git/lfs_lock.go

@@ -6,6 +6,7 @@ package git
 import (
 	"context"
 	"errors"
+	"fmt"
 	"strings"
 	"time"
 
@@ -24,6 +25,7 @@ type LFSLock struct {
 	ID      int64            `xorm:"pk autoincr"`
 	RepoID  int64            `xorm:"INDEX NOT NULL"`
 	OwnerID int64            `xorm:"INDEX NOT NULL"`
+	Owner   *user_model.User `xorm:"-"`
 	Path    string           `xorm:"TEXT"`
 	Created time.Time        `xorm:"created"`
 }
@@ -37,6 +39,35 @@ func (l *LFSLock) BeforeInsert() {
 	l.Path = util.PathJoinRel(l.Path)
 }
 
+// LoadAttributes loads attributes of the lock.
+func (l *LFSLock) LoadAttributes(ctx context.Context) error {
+	// Load owner
+	if err := l.LoadOwner(ctx); err != nil {
+		return fmt.Errorf("load owner: %w", err)
+	}
+
+	return nil
+}
+
+// LoadOwner loads owner of the lock.
+func (l *LFSLock) LoadOwner(ctx context.Context) error {
+	if l.Owner != nil {
+		return nil
+	}
+
+	owner, err := user_model.GetUserByID(ctx, l.OwnerID)
+	if err != nil {
+		if user_model.IsErrUserNotExist(err) {
+			l.Owner = user_model.NewGhostUser()
+			return nil
+		}
+		return err
+	}
+	l.Owner = owner
+
+	return nil
+}
+
 // CreateLFSLock creates a new lock.
 func CreateLFSLock(ctx context.Context, repo *repo_model.Repository, lock *LFSLock) (*LFSLock, error) {
 	dbCtx, committer, err := db.TxContext(ctx)
@@ -94,7 +125,7 @@ func GetLFSLockByID(ctx context.Context, id int64) (*LFSLock, error) {
 }
 
 // GetLFSLockByRepoID returns a list of locks of repository.
-func GetLFSLockByRepoID(ctx context.Context, repoID int64, page, pageSize int) ([]*LFSLock, error) {
+func GetLFSLockByRepoID(ctx context.Context, repoID int64, page, pageSize int) (LFSLockList, error) {
 	e := db.GetEngine(ctx)
 	if page >= 0 && pageSize > 0 {
 		start := 0
@@ -103,7 +134,7 @@ func GetLFSLockByRepoID(ctx context.Context, repoID int64, page, pageSize int) (
 		}
 		e.Limit(pageSize, start)
 	}
-	lfsLocks := make([]*LFSLock, 0, pageSize)
+	lfsLocks := make(LFSLockList, 0, pageSize)
 	return lfsLocks, e.Find(&lfsLocks, &LFSLock{RepoID: repoID})
 }
 

models/git/lfs_lock_list.go

@@ -0,0 +1,54 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package git
+
+import (
+	"context"
+	"fmt"
+
+	"code.gitea.io/gitea/models/db"
+	user_model "code.gitea.io/gitea/models/user"
+	"code.gitea.io/gitea/modules/container"
+)
+
+// LFSLockList is a list of LFSLock
+type LFSLockList []*LFSLock
+
+// LoadAttributes loads the attributes for the given locks
+func (locks LFSLockList) LoadAttributes(ctx context.Context) error {
+	if len(locks) == 0 {
+		return nil
+	}
+
+	if err := locks.LoadOwner(ctx); err != nil {
+		return fmt.Errorf("load owner: %w", err)
+	}
+
+	return nil
+}
+
+// LoadOwner loads the owner of the locks
+func (locks LFSLockList) LoadOwner(ctx context.Context) error {
+	if len(locks) == 0 {
+		return nil
+	}
+
+	usersIDs := container.FilterSlice(locks, func(lock *LFSLock) (int64, bool) {
+		return lock.OwnerID, true
+	})
+	users := make(map[int64]*user_model.User, len(usersIDs))
+	if err := db.GetEngine(ctx).
+		In("id", usersIDs).
+		Find(&users); err != nil {
+		return fmt.Errorf("find users: %w", err)
+	}
+	for _, v := range locks {
+		v.Owner = users[v.OwnerID]
+		if v.Owner == nil { // not exist
+			v.Owner = user_model.NewGhostUser()
+		}
+	}
+
+	return nil
+}

release-notes/4998.md

@@ -0,0 +1,4 @@
+fix: [commit](https://codeberg.org/forgejo/forgejo/commit/7f1db1df3ee8d620f997b8e70a40c2f48ae96c0f) Show lock owner instead of repo owner on LFS setting page.
+feat: [commit](https://codeberg.org/forgejo/forgejo/commit/ebfdc659d814561f8783094e2eb26738a5500e55) Render plain text file if the LFS object doesn't exist.
+fix: [commit](https://codeberg.org/forgejo/forgejo/commit/9e066c3cad7bb1b30e2def34bd0608aac825cf58) Fix panic of ssh public key page after deletion of auth source.
+fix: [commit](https://codeberg.org/forgejo/forgejo/commit/a8e25e907c66140961f28ba92403176c816dfb60) Add missing repository type filter parameters to pager.

routers/web/explore/repo.go

@@ -144,6 +144,21 @@ func RenderRepoSearch(ctx *context.Context, opts *RepoSearchOptions) {
 	pager.AddParam(ctx, "topic", "TopicOnly")
 	pager.AddParam(ctx, "language", "Language")
 	pager.AddParamString(relevantReposOnlyParam, fmt.Sprint(opts.OnlyShowRelevant))
+	if archived.Has() {
+		pager.AddParamString("archived", fmt.Sprint(archived.Value()))
+	}
+	if fork.Has() {
+		pager.AddParamString("fork", fmt.Sprint(fork.Value()))
+	}
+	if mirror.Has() {
+		pager.AddParamString("mirror", fmt.Sprint(mirror.Value()))
+	}
+	if template.Has() {
+		pager.AddParamString("template", fmt.Sprint(template.Value()))
+	}
+	if private.Has() {
+		pager.AddParamString("private", fmt.Sprint(private.Value()))
+	}
 	ctx.Data["Page"] = pager
 
 	ctx.HTML(http.StatusOK, opts.TplName)

routers/web/org/home.go

@@ -4,6 +4,7 @@
 package org
 
 import (
+	"fmt"
 	"net/http"
 	"path"
 	"strings"
@@ -154,7 +155,22 @@ func Home(ctx *context.Context) {
 
 	pager := context.NewPagination(int(count), setting.UI.User.RepoPagingNum, page, 5)
 	pager.SetDefaultParams(ctx)
-	pager.AddParam(ctx, "language", "Language")
+	pager.AddParamString("language", language)
+	if archived.Has() {
+		pager.AddParamString("archived", fmt.Sprint(archived.Value()))
+	}
+	if fork.Has() {
+		pager.AddParamString("fork", fmt.Sprint(fork.Value()))
+	}
+	if mirror.Has() {
+		pager.AddParamString("mirror", fmt.Sprint(mirror.Value()))
+	}
+	if template.Has() {
+		pager.AddParamString("template", fmt.Sprint(template.Value()))
+	}
+	if private.Has() {
+		pager.AddParamString("private", fmt.Sprint(private.Value()))
+	}
 	ctx.Data["Page"] = pager
 
 	ctx.Data["ShowMemberAndTeamTab"] = ctx.Org.IsMember || len(members) > 0

routers/web/repo/pull.go

@@ -1524,13 +1524,12 @@ func CompareAndPullRequestPost(ctx *context.Context) {
 	// instead of 500.
 
 	if err := pull_service.NewPullRequest(ctx, repo, pullIssue, labelIDs, attachments, pullRequest, assigneeIDs); err != nil {
-		if errors.Is(err, user_model.ErrBlockedByUser) {
+		switch {
+		case errors.Is(err, user_model.ErrBlockedByUser):
 			ctx.JSONError(ctx.Tr("repo.pulls.blocked_by_user"))
-			return
+		case repo_model.IsErrUserDoesNotHaveAccessToRepo(err):
-		} else if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) {
 			ctx.Error(http.StatusBadRequest, "UserDoesNotHaveAccessToRepo", err.Error())
-			return
+		case git.IsErrPushRejected(err):
-		} else if git.IsErrPushRejected(err) {
 			pushrejErr := err.(*git.ErrPushRejected)
 			message := pushrejErr.Message
 			if len(message) == 0 {
@@ -1547,7 +1546,11 @@ func CompareAndPullRequestPost(ctx *context.Context) {
 				return
 			}
 			ctx.JSONError(flashError)
-			return
+		default:
+			// It's an unexpected error.
+			// If it happens, we should add another case to handle it.
+			log.Error("Unexpected error of NewPullRequest: %T %s", err, err)
+			ctx.ServerError("CompareAndPullRequest", err)
 		}
 		ctx.ServerError("NewPullRequest", err)
 		return

routers/web/repo/setting/lfs.go

@@ -95,6 +95,11 @@ func LFSLocks(ctx *context.Context) {
 		ctx.ServerError("LFSLocks", err)
 		return
 	}
+	if err := lfsLocks.LoadAttributes(ctx); err != nil {
+		ctx.ServerError("LFSLocks", err)
+		return
+	}
+
 	ctx.Data["LFSLocks"] = lfsLocks
 
 	if len(lfsLocks) == 0 {

routers/web/repo/view.go

@@ -240,14 +240,12 @@ func getFileReader(ctx gocontext.Context, repoID int64, blob *git.Blob) ([]byte,
 	}
 
 	meta, err := git_model.GetLFSMetaObjectByOid(ctx, repoID, pointer.Oid)
-	if err != nil && err != git_model.ErrLFSObjectNotExist { // fallback to plain file
+	if err != nil { // fallback to plain file
+		log.Warn("Unable to access LFS pointer %s in repo %d: %v", pointer.Oid, repoID, err)
 		return buf, dataRc, &fileInfo{isTextFile, false, blob.Size(), nil, st}, nil
 	}
 
 	dataRc.Close()
-	if err != nil {
-		return nil, nil, nil, err
-	}
 
 	dataRc, err = lfs.ReadMetaObject(pointer)
 	if err != nil {

routers/web/user/notification.go

@@ -446,6 +446,21 @@ func NotificationWatching(ctx *context.Context) {
 	// redirect to last page if request page is more than total pages
 	pager := context.NewPagination(total, setting.UI.User.RepoPagingNum, page, 5)
 	pager.SetDefaultParams(ctx)
+	if archived.Has() {
+		pager.AddParamString("archived", fmt.Sprint(archived.Value()))
+	}
+	if fork.Has() {
+		pager.AddParamString("fork", fmt.Sprint(fork.Value()))
+	}
+	if mirror.Has() {
+		pager.AddParamString("mirror", fmt.Sprint(mirror.Value()))
+	}
+	if template.Has() {
+		pager.AddParamString("template", fmt.Sprint(template.Value()))
+	}
+	if private.Has() {
+		pager.AddParamString("private", fmt.Sprint(private.Value()))
+	}
 	ctx.Data["Page"] = pager
 
 	ctx.Data["Status"] = 2

routers/web/user/profile.go

@@ -335,6 +335,21 @@ func prepareUserProfileTabData(ctx *context.Context, showPrivate bool, profileDb
 	if tab == "activity" {
 		pager.AddParam(ctx, "date", "Date")
 	}
+	if archived.Has() {
+		pager.AddParamString("archived", fmt.Sprint(archived.Value()))
+	}
+	if fork.Has() {
+		pager.AddParamString("fork", fmt.Sprint(fork.Value()))
+	}
+	if mirror.Has() {
+		pager.AddParamString("mirror", fmt.Sprint(mirror.Value()))
+	}
+	if template.Has() {
+		pager.AddParamString("template", fmt.Sprint(template.Value()))
+	}
+	if private.Has() {
+		pager.AddParamString("private", fmt.Sprint(private.Value()))
+	}
 	ctx.Data["Page"] = pager
 }
 

templates/repo/settings/lfs_locks.tmpl

@@ -30,9 +30,9 @@
 								{{end}}
 							</td>
 							<td>
-								<a href="{{$.Owner.HomeLink}}">
+								<a href="{{$lock.Owner.HomeLink}}">
-									{{ctx.AvatarUtils.Avatar $.Owner}}
+									{{ctx.AvatarUtils.Avatar $lock.Owner}}
-									{{$.Owner.DisplayName}}
+									{{$lock.Owner.DisplayName}}
 								</a>
 							</td>
 							<td>{{TimeSince .Created ctx.Locale}}</td>

tests/gitea-repositories-meta/user2/lfs.git/objects/30/77e1c4c8964613df72c37d14275c1eda5228a9

@@ -0,0 +1,2 @@
+x
KÊÉOR0´0`pö÷	òt
+
ñôs×ËMQHËÌ)I-²ÍI+VHÉLK3rS‹ÒSÁ,Ý’ÔŠ.-½¬‚t"U&eæ¥23¯,1'“8û
ØæAÅ

tests/gitea-repositories-meta/user2/lfs.git/refs/heads/master

@@ -1 +1 @@
-73cf03db6ece34e12bf91e8853dc58f678f2f82d
+e9c32647bab825977942598c0efa415de300304b

tests/integration/lfs_view_test.go

@@ -4,12 +4,21 @@
 package integration
 
 import (
+	"context"
+	"fmt"
 	"net/http"
+	"strings"
 	"testing"
 
+	repo_model "code.gitea.io/gitea/models/repo"
+	"code.gitea.io/gitea/models/unittest"
+	user_model "code.gitea.io/gitea/models/user"
+	"code.gitea.io/gitea/modules/lfs"
+	api "code.gitea.io/gitea/modules/structs"
 	"code.gitea.io/gitea/tests"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 // check that files stored in LFS render properly in the web UI
@@ -102,4 +111,70 @@ func TestLFSRender(t *testing.T) {
 		doc := NewHTMLParser(t, resp.Body).doc
 		assert.Equal(t, 1, doc.Find(`.sha.label[href="/user2/lfs/commit/73cf03db6ece34e12bf91e8853dc58f678f2f82d"]`).Length(), "could not find link to commit")
 	})
+
+	// check that an invalid lfs entry defaults to plaintext
+	t.Run("Invalid", func(t *testing.T) {
+		defer tests.PrintCurrentTest(t)()
+
+		req := NewRequest(t, "GET", "/user2/lfs/src/branch/master/invalid")
+		resp := session.MakeRequest(t, req, http.StatusOK)
+
+		doc := NewHTMLParser(t, resp.Body).doc
+
+		content := doc.Find("div.file-view").Text()
+		assert.Contains(t, content, "oid sha256:9d178b5f15046343fd32f451df93acc2bdd9e6373be478b968e4cad6b6647351")
+	})
+}
+
+// TestLFSLockView tests the LFS lock view on settings page of repositories
+func TestLFSLockView(t *testing.T) {
+	defer tests.PrepareTestEnv(t)()
+
+	user2 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2})       // in org 3
+	repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) // own by org 3
+	session := loginUser(t, user2.Name)
+
+	// create a lock
+	lockPath := "test_lfs_lock_view.zip"
+	lockID := ""
+	{
+		req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/%s.git/info/lfs/locks", repo3.FullName()), map[string]string{"path": lockPath})
+		req.Header.Set("Accept", lfs.AcceptHeader)
+		req.Header.Set("Content-Type", lfs.MediaType)
+		resp := session.MakeRequest(t, req, http.StatusCreated)
+		lockResp := &api.LFSLockResponse{}
+		DecodeJSON(t, resp, lockResp)
+		lockID = lockResp.Lock.ID
+	}
+	defer func() {
+		// release the lock
+		req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/%s.git/info/lfs/locks/%s/unlock", repo3.FullName(), lockID), map[string]string{})
+		req.Header.Set("Accept", lfs.AcceptHeader)
+		req.Header.Set("Content-Type", lfs.MediaType)
+		session.MakeRequest(t, req, http.StatusOK)
+	}()
+
+	t.Run("owner name", func(t *testing.T) {
+		defer tests.PrintCurrentTest(t)()
+
+		// make sure the display names are different, or the test is meaningless
+		require.NoError(t, repo3.LoadOwner(context.Background()))
+		require.NotEqual(t, user2.DisplayName(), repo3.Owner.DisplayName())
+
+		req := NewRequest(t, "GET", fmt.Sprintf("/%s/settings/lfs/locks", repo3.FullName()))
+		resp := session.MakeRequest(t, req, http.StatusOK)
+
+		doc := NewHTMLParser(t, resp.Body).doc
+
+		tr := doc.Find("table#lfs-files-locks-table tbody tr")
+		require.Equal(t, 1, tr.Length())
+
+		td := tr.First().Find("td")
+		require.Equal(t, 4, td.Length())
+
+		// path
+		assert.Equal(t, lockPath, strings.TrimSpace(td.Eq(0).Text()))
+		// owner name
+		assert.Equal(t, user2.DisplayName(), strings.TrimSpace(td.Eq(1).Text()))
+	})
 }

web_src/js/components/RepoActionView.vue

@@ -878,6 +878,7 @@ export function initRepositoryActionView() {
   word-break: break-all;
   white-space: break-spaces;
   margin-left: 10px;
+  overflow-wrap: anywhere;
 }
 
 /* selectors here are intentionally exact to only match fullscreen */