db: migrate `admin.go` to `notices.go` with GORM (#7536)

.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/

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
+```
+

internal/db/admin.go

@@ -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(&notices)
-}
-
-// 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
-}

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

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)

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)
 			}
 		}

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 {

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 &notices{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(&notices).
+		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)
+		}
+	}
+}

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 := &notices{
+		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)
+}

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)
 				}
 			}

internal/db/testdata/backup/Notice.golden.json

@@ -0,0 +1 @@
+{"ID":1,"Type":1,"Description":"This is a notice","CreatedUnix":1588568886}

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
 	}

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>