From 1bdfab769b8c1b28d4a7c06c1f3a4d7ae5611b48 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 7 Aug 2024 11:03:45 +0800 Subject: [PATCH 1/6] fix: show lock owner --- models/git/lfs_lock.go | 41 +++++++++++++++++---- models/git/lfs_lock_list.go | 51 ++++++++++++++++++++++++++ routers/web/repo/setting/lfs.go | 5 +++ templates/repo/settings/lfs_locks.tmpl | 6 +-- 4 files changed, 93 insertions(+), 10 deletions(-) create mode 100644 models/git/lfs_lock_list.go diff --git a/models/git/lfs_lock.go b/models/git/lfs_lock.go index 2f65833fe35d7..d6c0b11892c69 100644 --- a/models/git/lfs_lock.go +++ b/models/git/lfs_lock.go @@ -6,6 +6,7 @@ package git import ( "context" "errors" + "fmt" "strings" "time" @@ -21,11 +22,12 @@ import ( // LFSLock represents a git lfs lock of repository. type LFSLock struct { - ID int64 `xorm:"pk autoincr"` - RepoID int64 `xorm:"INDEX NOT NULL"` - OwnerID int64 `xorm:"INDEX NOT NULL"` - Path string `xorm:"TEXT"` - Created time.Time `xorm:"created"` + 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"` } func init() { @@ -37,6 +39,31 @@ 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 { + 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 +121,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 +130,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}) } diff --git a/models/git/lfs_lock_list.go b/models/git/lfs_lock_list.go new file mode 100644 index 0000000000000..368a94bb7a198 --- /dev/null +++ b/models/git/lfs_lock_list.go @@ -0,0 +1,51 @@ +// Copyright 2017 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] + } + + return nil +} diff --git a/routers/web/repo/setting/lfs.go b/routers/web/repo/setting/lfs.go index 3b96f93ff2f71..fad635966864c 100644 --- a/routers/web/repo/setting/lfs.go +++ b/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 { diff --git a/templates/repo/settings/lfs_locks.tmpl b/templates/repo/settings/lfs_locks.tmpl index 98f0e4909760b..778a200a52cc6 100644 --- a/templates/repo/settings/lfs_locks.tmpl +++ b/templates/repo/settings/lfs_locks.tmpl @@ -30,9 +30,9 @@ {{end}} - - {{ctx.AvatarUtils.Avatar $.Owner}} - {{$.Owner.DisplayName}} + + {{ctx.AvatarUtils.Avatar .Owner}} + {{.Owner.DisplayName}} {{TimeSince .Created ctx.Locale}} From 4773709bae5349e14001ef1089784dc3945e31ca Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 7 Aug 2024 11:05:14 +0800 Subject: [PATCH 2/6] chore: fix header --- models/git/lfs_lock_list.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/git/lfs_lock_list.go b/models/git/lfs_lock_list.go index 368a94bb7a198..4ff60ea3be70d 100644 --- a/models/git/lfs_lock_list.go +++ b/models/git/lfs_lock_list.go @@ -1,4 +1,4 @@ -// Copyright 2017 The Gitea Authors. All rights reserved. +// Copyright 2024 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT package git From cdf82afd2f16e5b2cadb7d3ad7a265197c95d911 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 7 Aug 2024 12:06:21 +0800 Subject: [PATCH 3/6] test: TestLFSLockView --- tests/integration/lfs_view_test.go | 63 ++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/tests/integration/lfs_view_test.go b/tests/integration/lfs_view_test.go index 1775fa629f8f9..c74368c533c94 100644 --- a/tests/integration/lfs_view_test.go +++ b/tests/integration/lfs_view_test.go @@ -4,12 +4,22 @@ 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/PuerkitoBio/goquery" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // check that files stored in LFS render properly in the web UI @@ -81,3 +91,56 @@ func TestLFSRender(t *testing.T) { assert.Contains(t, content, "Testing READMEs in LFS") }) } + +// 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) + + // 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()) + + // 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() { + // delete 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) + }() + + 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 + trs := doc.Find("table#lfs-files-locks-table tbody tr") + + var tr *goquery.Selection + for i := range trs.Length() { + v := trs.Eq(i) + if strings.TrimSpace(v.Find("td").Eq(0).Text()) == lockPath { + tr = v + break + } + } + require.NotNilf(t, tr, "lock %s not found", lockPath) + + td := tr.Find("td") + require.Equal(t, 4, td.Length()) + assert.Equal(t, user2.DisplayName(), strings.TrimSpace(td.Eq(1).Text())) +} From a48aedcabfc017edcc919387bb1a21c5ed58cdef Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 7 Aug 2024 12:13:01 +0800 Subject: [PATCH 4/6] test: improve --- tests/integration/lfs_view_test.go | 43 +++++++++++++++--------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/tests/integration/lfs_view_test.go b/tests/integration/lfs_view_test.go index c74368c533c94..c28ecf1d7a377 100644 --- a/tests/integration/lfs_view_test.go +++ b/tests/integration/lfs_view_test.go @@ -17,7 +17,6 @@ import ( api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/tests" - "github.com/PuerkitoBio/goquery" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -100,10 +99,6 @@ func TestLFSLockView(t *testing.T) { repo3 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 3}) // own by org 3 session := loginUser(t, user2.Name) - // 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()) - // create a lock lockPath := "test_lfs_lock_view.zip" lockID := "" @@ -117,30 +112,34 @@ func TestLFSLockView(t *testing.T) { lockID = lockResp.Lock.ID } defer func() { - // delete the lock + // 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) }() - req := NewRequest(t, "GET", fmt.Sprintf("/%s/settings/lfs/locks", repo3.FullName())) - resp := session.MakeRequest(t, req, http.StatusOK) + t.Run("owner name", func(t *testing.T) { + defer tests.PrintCurrentTest(t)() - doc := NewHTMLParser(t, resp.Body).doc - trs := doc.Find("table#lfs-files-locks-table tbody tr") + // 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()) - var tr *goquery.Selection - for i := range trs.Length() { - v := trs.Eq(i) - if strings.TrimSpace(v.Find("td").Eq(0).Text()) == lockPath { - tr = v - break - } - } - require.NotNilf(t, tr, "lock %s not found", lockPath) + req := NewRequest(t, "GET", fmt.Sprintf("/%s/settings/lfs/locks", repo3.FullName())) + resp := session.MakeRequest(t, req, http.StatusOK) - td := tr.Find("td") - require.Equal(t, 4, td.Length()) - assert.Equal(t, user2.DisplayName(), strings.TrimSpace(td.Eq(1).Text())) + 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())) + }) } From 448a8e42104869bcc907a802d5a7c14f38d0ccef Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 7 Aug 2024 14:54:09 +0800 Subject: [PATCH 5/6] chore: $lock --- templates/repo/settings/lfs_locks.tmpl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/templates/repo/settings/lfs_locks.tmpl b/templates/repo/settings/lfs_locks.tmpl index 778a200a52cc6..9a18f525e95d8 100644 --- a/templates/repo/settings/lfs_locks.tmpl +++ b/templates/repo/settings/lfs_locks.tmpl @@ -30,9 +30,9 @@ {{end}} - - {{ctx.AvatarUtils.Avatar .Owner}} - {{.Owner.DisplayName}} + + {{ctx.AvatarUtils.Avatar $lock.Owner}} + {{$lock.Owner.DisplayName}} {{TimeSince .Created ctx.Locale}} From 765ff4bb547fa4764bd75cb10844e84c5bb91794 Mon Sep 17 00:00:00 2001 From: Jason Song Date: Wed, 7 Aug 2024 16:29:25 +0800 Subject: [PATCH 6/6] fix: ghost user --- models/git/lfs_lock.go | 4 ++++ models/git/lfs_lock_list.go | 3 +++ 2 files changed, 7 insertions(+) diff --git a/models/git/lfs_lock.go b/models/git/lfs_lock.go index d6c0b11892c69..07ce7d4abf389 100644 --- a/models/git/lfs_lock.go +++ b/models/git/lfs_lock.go @@ -57,6 +57,10 @@ func (l *LFSLock) LoadOwner(ctx context.Context) error { 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 diff --git a/models/git/lfs_lock_list.go b/models/git/lfs_lock_list.go index 4ff60ea3be70d..cab1e61cabc6c 100644 --- a/models/git/lfs_lock_list.go +++ b/models/git/lfs_lock_list.go @@ -45,6 +45,9 @@ func (locks LFSLockList) LoadOwner(ctx context.Context) error { } for _, v := range locks { v.Owner = users[v.OwnerID] + if v.Owner == nil { // not exist + v.Owner = user_model.NewGhostUser() + } } return nil