diff options
author | Joe Chen <jc@unknwon.io> | 2023-08-23 00:15:30 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-08-23 00:15:30 -0400 |
commit | 069f1ed9a4651dd2598a513d94278a400a04e5a7 (patch) | |
tree | 4e2455a2aab10daba29985bffe15183ab09bb5d8 | |
parent | 1112a71ea5279a29666d54f07ef101480519fd16 (diff) |
db: migrate `admin.go` to `notices.go` with GORM (#7536)
-rw-r--r-- | .github/workflows/scip.yml | 4 | ||||
-rw-r--r-- | docs/dev/database_schema.md | 13 | ||||
-rw-r--r-- | internal/db/admin.go | 118 | ||||
-rw-r--r-- | internal/db/backup_test.go | 9 | ||||
-rw-r--r-- | internal/db/db.go | 2 | ||||
-rw-r--r-- | internal/db/mirror.go | 6 | ||||
-rw-r--r-- | internal/db/models.go | 2 | ||||
-rw-r--r-- | internal/db/notices.go | 122 | ||||
-rw-r--r-- | internal/db/notices_test.go | 175 | ||||
-rw-r--r-- | internal/db/repo.go | 10 | ||||
-rw-r--r-- | internal/db/testdata/backup/Notice.golden.json | 1 | ||||
-rw-r--r-- | internal/route/admin/notice.go | 8 | ||||
-rw-r--r-- | templates/admin/notice.tmpl | 2 |
13 files changed, 335 insertions, 137 deletions
diff --git a/.github/workflows/scip.yml b/.github/workflows/scip.yml index 0310cfb7..41d4c37e 100644 --- a/.github/workflows/scip.yml +++ b/.github/workflows/scip.yml @@ -32,7 +32,3 @@ jobs: run: src code-intel upload -github-token='${{ secrets.GITHUB_TOKEN }}' -no-progress -repo=github.com/gogs/gogs env: SRC_ENDPOINT: https://sourcegraph.sourcegraph.com/ - - name: Upload SCIP data to cs.unknwon.dev - run: src code-intel upload -github-token='${{ secrets.GITHUB_TOKEN }}' -no-progress -repo=github.com/gogs/gogs - env: - SRC_ENDPOINT: https://cs.unknwon.dev/ diff --git a/docs/dev/database_schema.md b/docs/dev/database_schema.md index e33321e3..b49cca3e 100644 --- a/docs/dev/database_schema.md +++ b/docs/dev/database_schema.md @@ -116,3 +116,16 @@ Primary keys: repo_id, oid Primary keys: id ``` +# Table "notice" + +``` + FIELD | COLUMN | POSTGRESQL | MYSQL | SQLITE3 +--------------+--------------+------------+-----------------------+---------- + ID | id | BIGSERIAL | BIGINT AUTO_INCREMENT | INTEGER + Type | type | BIGINT | BIGINT | INTEGER + Description | description | TEXT | TEXT | TEXT + CreatedUnix | created_unix | BIGINT | BIGINT | INTEGER + +Primary keys: id +``` + diff --git a/internal/db/admin.go b/internal/db/admin.go deleted file mode 100644 index 22f9e1e6..00000000 --- a/internal/db/admin.go +++ /dev/null @@ -1,118 +0,0 @@ -// Copyright 2014 The Gogs Authors. All rights reserved. -// Use of this source code is governed by a MIT-style -// license that can be found in the LICENSE file. - -package db - -import ( - "fmt" - "os" - "strings" - "time" - - "github.com/unknwon/com" - log "unknwon.dev/clog/v2" - "xorm.io/xorm" - - "gogs.io/gogs/internal/tool" -) - -type NoticeType int - -const ( - NOTICE_REPOSITORY NoticeType = iota + 1 -) - -// Notice represents a system notice for admin. -type Notice struct { - ID int64 - Type NoticeType - Description string `xorm:"TEXT"` - Created time.Time `xorm:"-" json:"-"` - CreatedUnix int64 -} - -func (n *Notice) BeforeInsert() { - n.CreatedUnix = time.Now().Unix() -} - -func (n *Notice) AfterSet(colName string, _ xorm.Cell) { - switch colName { - case "created_unix": - n.Created = time.Unix(n.CreatedUnix, 0).Local() - } -} - -// TrStr returns a translation format string. -func (n *Notice) TrStr() string { - return "admin.notices.type_" + com.ToStr(n.Type) -} - -// CreateNotice creates new system notice. -func CreateNotice(tp NoticeType, desc string) error { - // Prevent panic if database connection is not available at this point - if x == nil { - return fmt.Errorf("could not save notice due database connection not being available: %d %s", tp, desc) - } - - n := &Notice{ - Type: tp, - Description: desc, - } - _, err := x.Insert(n) - return err -} - -// CreateRepositoryNotice creates new system notice with type NOTICE_REPOSITORY. -func CreateRepositoryNotice(desc string) error { - return CreateNotice(NOTICE_REPOSITORY, desc) -} - -// RemoveAllWithNotice removes all directories in given path and -// creates a system notice when error occurs. -func RemoveAllWithNotice(title, path string) { - if err := os.RemoveAll(path); err != nil { - desc := fmt.Sprintf("%s [%s]: %v", title, path, err) - log.Warn(desc) - if err = CreateRepositoryNotice(desc); err != nil { - log.Error("CreateRepositoryNotice: %v", err) - } - } -} - -// CountNotices returns number of notices. -func CountNotices() int64 { - count, _ := x.Count(new(Notice)) - return count -} - -// Notices returns number of notices in given page. -func Notices(page, pageSize int) ([]*Notice, error) { - notices := make([]*Notice, 0, pageSize) - return notices, x.Limit(pageSize, (page-1)*pageSize).Desc("id").Find(¬ices) -} - -// DeleteNotice deletes a system notice by given ID. -func DeleteNotice(id int64) error { - _, err := x.Id(id).Delete(new(Notice)) - return err -} - -// DeleteNotices deletes all notices with ID from start to end (inclusive). -func DeleteNotices(start, end int64) error { - sess := x.Where("id >= ?", start) - if end > 0 { - sess.And("id <= ?", end) - } - _, err := sess.Delete(new(Notice)) - return err -} - -// DeleteNoticesByIDs deletes notices by given IDs. -func DeleteNoticesByIDs(ids []int64) error { - if len(ids) == 0 { - return nil - } - _, err := x.Where("id IN (" + strings.Join(tool.Int64sToStrings(ids), ",") + ")").Delete(new(Notice)) - return err -} diff --git a/internal/db/backup_test.go b/internal/db/backup_test.go index 4c5e4752..9d2d9403 100644 --- a/internal/db/backup_test.go +++ b/internal/db/backup_test.go @@ -31,7 +31,7 @@ func TestDumpAndImport(t *testing.T) { } t.Parallel() - const wantTables = 7 + const wantTables = 8 if len(Tables) != wantTables { t.Fatalf("New table has added (want %d got %d), please add new tests for the table and update this check", wantTables, len(Tables)) } @@ -190,6 +190,13 @@ func setupDBToDump(t *testing.T, db *gorm.DB) { }), CreatedUnix: 1588568886, }, + + &Notice{ + ID: 1, + Type: NoticeTypeRepository, + Description: "This is a notice", + CreatedUnix: 1588568886, + }, } for _, val := range vals { err := db.Create(val).Error diff --git a/internal/db/db.go b/internal/db/db.go index d50b934f..9878032d 100644 --- a/internal/db/db.go +++ b/internal/db/db.go @@ -45,6 +45,7 @@ var Tables = []any{ new(EmailAddress), new(Follow), new(LFSObject), new(LoginSource), + new(Notice), } // Init initializes the database with given logger. @@ -124,6 +125,7 @@ func Init(w logger.Writer) (*gorm.DB, error) { Actions = NewActionsStore(db) LoginSources = &loginSources{DB: db, files: sourceFiles} LFS = &lfs{DB: db} + Notices = NewNoticesStore(db) Orgs = NewOrgsStore(db) Perms = NewPermsStore(db) Repos = NewReposStore(db) diff --git a/internal/db/mirror.go b/internal/db/mirror.go index 21d86f47..11ab1c6a 100644 --- a/internal/db/mirror.go +++ b/internal/db/mirror.go @@ -215,7 +215,7 @@ func (m *Mirror) runSync() ([]*mirrorSyncResult, bool) { // good condition to prevent long blocking on URL resolution without syncing anything. if !git.IsURLAccessible(time.Minute, m.RawAddress()) { desc := fmt.Sprintf("Source URL of mirror repository '%s' is not accessible: %s", m.Repo.FullName(), m.MosaicsAddress()) - if err := CreateRepositoryNotice(desc); err != nil { + if err := Notices.Create(context.TODO(), NoticeTypeRepository, desc); err != nil { log.Error("CreateRepositoryNotice: %v", err) } return nil, false @@ -231,7 +231,7 @@ func (m *Mirror) runSync() ([]*mirrorSyncResult, bool) { if err != nil { desc := fmt.Sprintf("Failed to update mirror repository '%s': %s", repoPath, stderr) log.Error(desc) - if err = CreateRepositoryNotice(desc); err != nil { + if err = Notices.Create(context.TODO(), NoticeTypeRepository, desc); err != nil { log.Error("CreateRepositoryNotice: %v", err) } return nil, false @@ -249,7 +249,7 @@ func (m *Mirror) runSync() ([]*mirrorSyncResult, bool) { "git", "remote", "update", "--prune"); err != nil { desc := fmt.Sprintf("Failed to update mirror wiki repository '%s': %s", wikiPath, stderr) log.Error(desc) - if err = CreateRepositoryNotice(desc); err != nil { + if err = Notices.Create(context.TODO(), NoticeTypeRepository, desc); err != nil { log.Error("CreateRepositoryNotice: %v", err) } } diff --git a/internal/db/models.go b/internal/db/models.go index 715df242..1e3b8d9b 100644 --- a/internal/db/models.go +++ b/internal/db/models.go @@ -58,7 +58,7 @@ func init() { new(Mirror), new(Release), new(Webhook), new(HookTask), new(ProtectBranch), new(ProtectBranchWhitelist), new(Team), new(OrgUser), new(TeamUser), new(TeamRepo), - new(Notice)) + ) gonicNames := []string{"SSL"} for _, name := range gonicNames { diff --git a/internal/db/notices.go b/internal/db/notices.go new file mode 100644 index 00000000..c1601735 --- /dev/null +++ b/internal/db/notices.go @@ -0,0 +1,122 @@ +// Copyright 2023 The Gogs Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package db + +import ( + "context" + "fmt" + "os" + "strconv" + "time" + + "gorm.io/gorm" + log "unknwon.dev/clog/v2" +) + +// NoticesStore is the persistent interface for system notices. +type NoticesStore interface { + // Create creates a system notice with the given type and description. + Create(ctx context.Context, typ NoticeType, desc string) error + // DeleteByIDs deletes system notices by given IDs. + DeleteByIDs(ctx context.Context, ids ...int64) error + // DeleteAll deletes all system notices. + DeleteAll(ctx context.Context) error + // List returns a list of system notices. Results are paginated by given page + // and page size, and sorted by primary key (id) in descending order. + List(ctx context.Context, page, pageSize int) ([]*Notice, error) + // Count returns the total number of system notices. + Count(ctx context.Context) int64 +} + +var Notices NoticesStore + +var _ NoticesStore = (*notices)(nil) + +type notices struct { + *gorm.DB +} + +// NewNoticesStore returns a persistent interface for system notices with given +// database connection. +func NewNoticesStore(db *gorm.DB) NoticesStore { + return ¬ices{DB: db} +} + +func (db *notices) Create(ctx context.Context, typ NoticeType, desc string) error { + return db.WithContext(ctx).Create( + &Notice{ + Type: typ, + Description: desc, + }, + ).Error +} + +func (db *notices) DeleteByIDs(ctx context.Context, ids ...int64) error { + return db.WithContext(ctx).Where("id IN (?)", ids).Delete(&Notice{}).Error +} + +func (db *notices) DeleteAll(ctx context.Context) error { + return db.WithContext(ctx).Where("TRUE").Delete(&Notice{}).Error +} + +func (db *notices) List(ctx context.Context, page, pageSize int) ([]*Notice, error) { + notices := make([]*Notice, 0, pageSize) + return notices, db.WithContext(ctx). + Limit(pageSize).Offset((page - 1) * pageSize). + Order("id DESC"). + Find(¬ices). + Error +} + +func (db *notices) Count(ctx context.Context) int64 { + var count int64 + db.WithContext(ctx).Model(&Notice{}).Count(&count) + return count +} + +type NoticeType int + +const ( + NoticeTypeRepository NoticeType = iota + 1 +) + +// TrStr returns a translation format string. +func (t NoticeType) TrStr() string { + return "admin.notices.type_" + strconv.Itoa(int(t)) +} + +// Notice represents a system notice for admin. +type Notice struct { + ID int64 `gorm:"primarykey"` + Type NoticeType + Description string `xorm:"TEXT" gorm:"type:TEXT"` + Created time.Time `xorm:"-" gorm:"-" json:"-"` + CreatedUnix int64 +} + +// BeforeCreate implements the GORM create hook. +func (n *Notice) BeforeCreate(tx *gorm.DB) error { + if n.CreatedUnix == 0 { + n.CreatedUnix = tx.NowFunc().Unix() + } + return nil +} + +// AfterFind implements the GORM query hook. +func (n *Notice) AfterFind(_ *gorm.DB) error { + n.Created = time.Unix(n.CreatedUnix, 0).Local() + return nil +} + +// RemoveAllWithNotice is a helper function to remove all directories in given +// path and creates a system notice in case of an error. +func RemoveAllWithNotice(title, path string) { + if err := os.RemoveAll(path); err != nil { + desc := fmt.Sprintf("%s [%s]: %v", title, path, err) + if err = Notices.Create(context.Background(), NoticeTypeRepository, desc); err != nil { + log.Error("Failed to create repository notice: %v", err) + } + } +} diff --git a/internal/db/notices_test.go b/internal/db/notices_test.go new file mode 100644 index 00000000..3ae934dd --- /dev/null +++ b/internal/db/notices_test.go @@ -0,0 +1,175 @@ +// Copyright 2023 The Gogs Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package db + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gorm.io/gorm" + + "gogs.io/gogs/internal/dbtest" +) + +func TestNotice_BeforeCreate(t *testing.T) { + now := time.Now() + db := &gorm.DB{ + Config: &gorm.Config{ + SkipDefaultTransaction: true, + NowFunc: func() time.Time { + return now + }, + }, + } + + t.Run("CreatedUnix has been set", func(t *testing.T) { + notice := &Notice{ + CreatedUnix: 1, + } + _ = notice.BeforeCreate(db) + assert.Equal(t, int64(1), notice.CreatedUnix) + }) + + t.Run("CreatedUnix has not been set", func(t *testing.T) { + notice := &Notice{} + _ = notice.BeforeCreate(db) + assert.Equal(t, db.NowFunc().Unix(), notice.CreatedUnix) + }) +} + +func TestNotice_AfterFind(t *testing.T) { + now := time.Now() + db := &gorm.DB{ + Config: &gorm.Config{ + SkipDefaultTransaction: true, + NowFunc: func() time.Time { + return now + }, + }, + } + + notice := &Notice{ + CreatedUnix: now.Unix(), + } + _ = notice.AfterFind(db) + assert.Equal(t, notice.CreatedUnix, notice.Created.Unix()) +} + +func TestNotices(t *testing.T) { + if testing.Short() { + t.Skip() + } + t.Parallel() + + tables := []any{new(Notice)} + db := ¬ices{ + DB: dbtest.NewDB(t, "notices", tables...), + } + + for _, tc := range []struct { + name string + test func(t *testing.T, db *notices) + }{ + {"Create", noticesCreate}, + {"DeleteByIDs", noticesDeleteByIDs}, + {"DeleteAll", noticesDeleteAll}, + {"List", noticesList}, + {"Count", noticesCount}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Cleanup(func() { + err := clearTables(t, db.DB, tables...) + require.NoError(t, err) + }) + tc.test(t, db) + }) + if t.Failed() { + break + } + } +} + +func noticesCreate(t *testing.T, db *notices) { + ctx := context.Background() + + err := db.Create(ctx, NoticeTypeRepository, "test") + require.NoError(t, err) + + count := db.Count(ctx) + assert.Equal(t, int64(1), count) +} + +func noticesDeleteByIDs(t *testing.T, db *notices) { + ctx := context.Background() + + err := db.Create(ctx, NoticeTypeRepository, "test") + require.NoError(t, err) + + notices, err := db.List(ctx, 1, 10) + require.NoError(t, err) + ids := make([]int64, 0, len(notices)) + for _, notice := range notices { + ids = append(ids, notice.ID) + } + + // Non-existing IDs should be ignored + ids = append(ids, 404) + err = db.DeleteByIDs(ctx, ids...) + require.NoError(t, err) + + count := db.Count(ctx) + assert.Equal(t, int64(0), count) +} + +func noticesDeleteAll(t *testing.T, db *notices) { + ctx := context.Background() + + err := db.Create(ctx, NoticeTypeRepository, "test") + require.NoError(t, err) + + err = db.DeleteAll(ctx) + require.NoError(t, err) + + count := db.Count(ctx) + assert.Equal(t, int64(0), count) +} + +func noticesList(t *testing.T, db *notices) { + ctx := context.Background() + + err := db.Create(ctx, NoticeTypeRepository, "test 1") + require.NoError(t, err) + err = db.Create(ctx, NoticeTypeRepository, "test 2") + require.NoError(t, err) + + got1, err := db.List(ctx, 1, 1) + require.NoError(t, err) + require.Len(t, got1, 1) + + got2, err := db.List(ctx, 2, 1) + require.NoError(t, err) + require.Len(t, got2, 1) + assert.True(t, got1[0].ID > got2[0].ID) + + got, err := db.List(ctx, 1, 3) + require.NoError(t, err) + require.Len(t, got, 2) +} + +func noticesCount(t *testing.T, db *notices) { + ctx := context.Background() + + count := db.Count(ctx) + assert.Equal(t, int64(0), count) + + err := db.Create(ctx, NoticeTypeRepository, "test") + require.NoError(t, err) + + count = db.Count(ctx) + assert.Equal(t, int64(1), count) +} diff --git a/internal/db/repo.go b/internal/db/repo.go index 28a211fc..8f15ea2b 100644 --- a/internal/db/repo.go +++ b/internal/db/repo.go @@ -1947,7 +1947,7 @@ func DeleteOldRepositoryArchives() { if err = os.Remove(archivePath); err != nil { desc := fmt.Sprintf("Failed to health delete archive '%s': %v", archivePath, err) log.Warn(desc) - if err = CreateRepositoryNotice(desc); err != nil { + if err = Notices.Create(context.TODO(), NoticeTypeRepository, desc); err != nil { log.Error("CreateRepositoryNotice: %v", err) } } @@ -1985,7 +1985,7 @@ func gatherMissingRepoRecords() ([]*Repository, error) { } return nil }); err != nil { - if err2 := CreateRepositoryNotice(fmt.Sprintf("gatherMissingRepoRecords: %v", err)); err2 != nil { + if err2 := Notices.Create(context.TODO(), NoticeTypeRepository, fmt.Sprintf("gatherMissingRepoRecords: %v", err)); err2 != nil { return nil, fmt.Errorf("CreateRepositoryNotice: %v", err) } } @@ -2006,7 +2006,7 @@ func DeleteMissingRepositories() error { for _, repo := range repos { log.Trace("Deleting %d/%d...", repo.OwnerID, repo.ID) if err := DeleteRepository(repo.OwnerID, repo.ID); err != nil { - if err2 := CreateRepositoryNotice(fmt.Sprintf("DeleteRepository [%d]: %v", repo.ID, err)); err2 != nil { + if err2 := Notices.Create(context.TODO(), NoticeTypeRepository, fmt.Sprintf("DeleteRepository [%d]: %v", repo.ID, err)); err2 != nil { return fmt.Errorf("CreateRepositoryNotice: %v", err) } } @@ -2028,7 +2028,7 @@ func ReinitMissingRepositories() error { for _, repo := range repos { log.Trace("Initializing %d/%d...", repo.OwnerID, repo.ID) if err := git.Init(repo.RepoPath(), git.InitOptions{Bare: true}); err != nil { - if err2 := CreateRepositoryNotice(fmt.Sprintf("init repository [repo_id: %d]: %v", repo.ID, err)); err2 != nil { + if err2 := Notices.Create(context.TODO(), NoticeTypeRepository, fmt.Sprintf("init repository [repo_id: %d]: %v", repo.ID, err)); err2 != nil { return fmt.Errorf("create repository notice: %v", err) } } @@ -2086,7 +2086,7 @@ func GitFsck() { if err != nil { desc := fmt.Sprintf("Failed to perform health check on repository '%s': %v", repoPath, err) log.Warn(desc) - if err = CreateRepositoryNotice(desc); err != nil { + if err = Notices.Create(context.TODO(), NoticeTypeRepository, desc); err != nil { log.Error("CreateRepositoryNotice: %v", err) } } diff --git a/internal/db/testdata/backup/Notice.golden.json b/internal/db/testdata/backup/Notice.golden.json new file mode 100644 index 00000000..4d472520 --- /dev/null +++ b/internal/db/testdata/backup/Notice.golden.json @@ -0,0 +1 @@ +{"ID":1,"Type":1,"Description":"This is a notice","CreatedUnix":1588568886} diff --git a/internal/route/admin/notice.go b/internal/route/admin/notice.go index 5e1f377f..87d9920c 100644 --- a/internal/route/admin/notice.go +++ b/internal/route/admin/notice.go @@ -25,14 +25,14 @@ func Notices(c *context.Context) { c.Data["PageIsAdmin"] = true c.Data["PageIsAdminNotices"] = true - total := db.CountNotices() + total := db.Notices.Count(c.Req.Context()) page := c.QueryInt("page") if page <= 1 { page = 1 } c.Data["Page"] = paginater.New(int(total), conf.UI.Admin.NoticePagingNum, page, 5) - notices, err := db.Notices(page, conf.UI.Admin.NoticePagingNum) + notices, err := db.Notices.List(c.Req.Context(), page, conf.UI.Admin.NoticePagingNum) if err != nil { c.Error(err, "list notices") return @@ -53,7 +53,7 @@ func DeleteNotices(c *context.Context) { } } - if err := db.DeleteNoticesByIDs(ids); err != nil { + if err := db.Notices.DeleteByIDs(c.Req.Context(), ids...); err != nil { c.Flash.Error("DeleteNoticesByIDs: " + err.Error()) c.Status(http.StatusInternalServerError) } else { @@ -63,7 +63,7 @@ func DeleteNotices(c *context.Context) { } func EmptyNotices(c *context.Context) { - if err := db.DeleteNotices(0, 0); err != nil { + if err := db.Notices.DeleteAll(c.Req.Context()); err != nil { c.Error(err, "delete notices") return } diff --git a/templates/admin/notice.tmpl b/templates/admin/notice.tmpl index fc2c703c..781b44eb 100644 --- a/templates/admin/notice.tmpl +++ b/templates/admin/notice.tmpl @@ -29,7 +29,7 @@ </div> </td> <td>{{.ID}}</td> - <td>{{$.i18n.Tr .TrStr}}</td> + <td>{{$.i18n.Tr .Type.TrStr}}</td> <td>{{SubStr .Description 0 120}}...</td> <td><span class="poping up" data-content="{{.Created}}" data-variation="inverted tiny">{{DateFmtShort .Created}}</span></td> <td><a href="#"><i class="browser icon view-detail" data-content="{{.Description}}"></i></a></td> |