Skip to content

Commit

Permalink
Merge remote-tracking branch 'giteaoffical/main'
Browse files Browse the repository at this point in the history
* giteaoffical/main:
  Fix db.Find bug (go-gitea#23115)
  Avoid warning for system setting when start up (go-gitea#23054)
  Require approval to run actions for fork pull request (go-gitea#22803)
  Fix nil context in RenderMarkdownToHtml (go-gitea#23092)
  Add HesterG to maintainers (go-gitea#23104)
  improve FindProjects (go-gitea#23085)
  • Loading branch information
zjjhot committed Feb 24, 2023
2 parents 514311a + a8c4f8c commit 02f7298
Show file tree
Hide file tree
Showing 37 changed files with 276 additions and 86 deletions.
3 changes: 2 additions & 1 deletion MAINTAINERS
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,5 @@ Jason Song <i@wolfogre.com> (@wolfogre)
Yarden Shoham <hrsi88@gmail.com> (@yardenshoham)
Yu Tian <zettat123@gmail.com> (@Zettat123)
Eddie Yang <576951401@qq.com> (@yp05327)
Dong Ge <gedong_1994@163.com> (@sillyguodong)
Dong Ge <gedong_1994@163.com> (@sillyguodong)
Xinyi Gong <hestergong@gmail.com> (@HesterG)
12 changes: 5 additions & 7 deletions models/actions/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,13 @@ type ActionRun struct {
OwnerID int64 `xorm:"index"`
WorkflowID string `xorm:"index"` // the name of workflow file
Index int64 `xorm:"index unique(repo_index)"` // a unique number for each run of a repository
TriggerUserID int64
TriggerUser *user_model.User `xorm:"-"`
TriggerUserID int64 `xorm:"index"`
TriggerUser *user_model.User `xorm:"-"`
Ref string
CommitSHA string
IsForkPullRequest bool
NeedApproval bool // may need approval if it's a fork pull request
ApprovedBy int64 `xorm:"index"` // who approved
Event webhook_module.HookEventType
EventPayload string `xorm:"LONGTEXT"`
Status Status `xorm:"index"`
Expand Down Expand Up @@ -164,10 +166,6 @@ func InsertRun(ctx context.Context, run *ActionRun, jobs []*jobparser.SingleWork
}
run.Index = index

if run.Status.IsUnknown() {
run.Status = StatusWaiting
}

if err := db.Insert(ctx, run); err != nil {
return err
}
Expand All @@ -191,7 +189,7 @@ func InsertRun(ctx context.Context, run *ActionRun, jobs []*jobparser.SingleWork
job.EraseNeeds()
payload, _ := v.Marshal()
status := StatusWaiting
if len(needs) > 0 {
if len(needs) > 0 || run.NeedApproval {
status = StatusBlocked
}
runJobs = append(runJobs, &ActionRunJob{
Expand Down
8 changes: 8 additions & 0 deletions models/actions/run_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ type FindRunOptions struct {
OwnerID int64
IsClosed util.OptionalBool
WorkflowFileName string
TriggerUserID int64
Approved bool // not util.OptionalBool, it works only when it's true
}

func (opts FindRunOptions) toConds() builder.Cond {
Expand All @@ -89,6 +91,12 @@ func (opts FindRunOptions) toConds() builder.Cond {
if opts.WorkflowFileName != "" {
cond = cond.And(builder.Eq{"workflow_id": opts.WorkflowFileName})
}
if opts.TriggerUserID > 0 {
cond = cond.And(builder.Eq{"trigger_user_id": opts.TriggerUserID})
}
if opts.Approved {
cond = cond.And(builder.Gt{"approved_by": 0})
}
return cond
}

Expand Down
4 changes: 4 additions & 0 deletions models/actions/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ func (s Status) IsRunning() bool {
return s == StatusRunning
}

func (s Status) IsBlocked() bool {
return s == StatusBlocked
}

// In returns whether s is one of the given statuses
func (s Status) In(statuses ...Status) bool {
for _, v := range statuses {
Expand Down
4 changes: 2 additions & 2 deletions models/avatars/avatar.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ func generateEmailAvatarLink(ctx context.Context, email string, size int, final
return DefaultAvatarLink()
}

enableFederatedAvatar := system_model.GetSettingBool(ctx, system_model.KeyPictureEnableFederatedAvatar)
enableFederatedAvatar := system_model.GetSettingWithCacheBool(ctx, system_model.KeyPictureEnableFederatedAvatar)

var err error
if enableFederatedAvatar && system_model.LibravatarService != nil {
Expand All @@ -174,7 +174,7 @@ func generateEmailAvatarLink(ctx context.Context, email string, size int, final
return urlStr
}

disableGravatar := system_model.GetSettingBool(ctx, system_model.KeyPictureDisableGravatar)
disableGravatar := system_model.GetSettingWithCacheBool(ctx, system_model.KeyPictureDisableGravatar)
if !disableGravatar {
// copy GravatarSourceURL, because we will modify its Path.
avatarURLCopy := *system_model.GravatarSourceURL
Expand Down
2 changes: 1 addition & 1 deletion models/avatars/avatar_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func enableGravatar(t *testing.T) {
err := system_model.SetSettingNoVersion(db.DefaultContext, system_model.KeyPictureDisableGravatar, "false")
assert.NoError(t, err)
setting.GravatarSource = gravatarSource
err = system_model.Init()
err = system_model.Init(db.DefaultContext)
assert.NoError(t, err)
}

Expand Down
4 changes: 2 additions & 2 deletions models/db/list_options.go → models/db/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func Find[T any](ctx context.Context, opts FindOptions, objects *[]T) error {
if !opts.IsListAll() {
sess.Limit(opts.GetSkipTake())
}
return sess.Find(&objects)
return sess.Find(objects)
}

// Count represents a common count function which accept an options interface
Expand All @@ -148,5 +148,5 @@ func FindAndCount[T any](ctx context.Context, opts FindOptions, objects *[]T) (i
if !opts.IsListAll() {
sess.Limit(opts.GetSkipTake())
}
return sess.FindAndCount(&objects)
return sess.FindAndCount(objects)
}
48 changes: 48 additions & 0 deletions models/db/list_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
// Copyright 2023 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package db_test

import (
"testing"

"code.gitea.io/gitea/models/db"
repo_model "code.gitea.io/gitea/models/repo"
"code.gitea.io/gitea/models/unittest"

"github.com/stretchr/testify/assert"
"xorm.io/builder"
)

type mockListOptions struct {
db.ListOptions
}

func (opts *mockListOptions) IsListAll() bool {
return true
}

func (opts *mockListOptions) ToConds() builder.Cond {
return builder.NewCond()
}

func TestFind(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
xe := unittest.GetXORMEngine()
assert.NoError(t, xe.Sync(&repo_model.RepoUnit{}))

opts := mockListOptions{}
var repoUnits []repo_model.RepoUnit
err := db.Find(db.DefaultContext, &opts, &repoUnits)
assert.NoError(t, err)
assert.EqualValues(t, 83, len(repoUnits))

cnt, err := db.Count(db.DefaultContext, &opts, new(repo_model.RepoUnit))
assert.NoError(t, err)
assert.EqualValues(t, 83, cnt)

repoUnits = make([]repo_model.RepoUnit, 0, 10)
newCnt, err := db.FindAndCount(db.DefaultContext, &opts, &repoUnits)
assert.NoError(t, err)
assert.EqualValues(t, cnt, newCnt)
}
4 changes: 4 additions & 0 deletions models/migrations/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"code.gitea.io/gitea/models/migrations/v1_17"
"code.gitea.io/gitea/models/migrations/v1_18"
"code.gitea.io/gitea/models/migrations/v1_19"
"code.gitea.io/gitea/models/migrations/v1_20"
"code.gitea.io/gitea/models/migrations/v1_6"
"code.gitea.io/gitea/models/migrations/v1_7"
"code.gitea.io/gitea/models/migrations/v1_8"
Expand Down Expand Up @@ -463,6 +464,9 @@ var migrations = []Migration{
NewMigration("Add exclusive label", v1_19.AddExclusiveLabel),

// Gitea 1.19.0 ends at v244

// v244 -> v245
NewMigration("Add NeedApproval to actions tables", v1_20.AddNeedApprovalToActionRun),
}

// GetCurrentDBVersion returns the current db version
Expand Down
22 changes: 22 additions & 0 deletions models/migrations/v1_20/v244.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Copyright 2023 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT

package v1_20 //nolint

import (
"xorm.io/xorm"
)

func AddNeedApprovalToActionRun(x *xorm.Engine) error {
/*
New index: TriggerUserID
New fields: NeedApproval, ApprovedBy
*/
type ActionRun struct {
TriggerUserID int64 `xorm:"index"`
NeedApproval bool // may need approval if it's a fork pull request
ApprovedBy int64 `xorm:"index"` // who approved
}

return x.Sync(new(ActionRun))
}
13 changes: 3 additions & 10 deletions models/project/project.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,16 +217,8 @@ func CountProjects(ctx context.Context, opts SearchOptions) (int64, error) {

// FindProjects returns a list of all projects that have been created in the repository
func FindProjects(ctx context.Context, opts SearchOptions) ([]*Project, int64, error) {
e := db.GetEngine(ctx)
e := db.GetEngine(ctx).Where(opts.toConds())
projects := make([]*Project, 0, setting.UI.IssuePagingNum)
cond := opts.toConds()

count, err := e.Where(cond).Count(new(Project))
if err != nil {
return nil, 0, fmt.Errorf("Count: %w", err)
}

e = e.Where(cond)

if opts.Page > 0 {
e = e.Limit(setting.UI.IssuePagingNum, (opts.Page-1)*setting.UI.IssuePagingNum)
Expand All @@ -243,7 +235,8 @@ func FindProjects(ctx context.Context, opts SearchOptions) ([]*Project, int64, e
e.Asc("created_unix")
}

return projects, count, e.Find(&projects)
count, err := e.FindAndCount(&projects)
return projects, count, err
}

// NewProject creates a new Project
Expand Down
4 changes: 2 additions & 2 deletions models/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,9 @@ import (
var ItemsPerPage = 40

// Init initialize model
func Init() error {
func Init(ctx context.Context) error {
unit.LoadUnitConfig()
return system_model.Init()
return system_model.Init(ctx)
}

// DeleteRepository deletes a repository for a user or organization.
Expand Down
49 changes: 29 additions & 20 deletions models/system/setting.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ func IsErrDataExpired(err error) bool {
return ok
}

// GetSettingNoCache returns specific setting without using the cache
func GetSettingNoCache(ctx context.Context, key string) (*Setting, error) {
// GetSetting returns specific setting without using the cache
func GetSetting(ctx context.Context, key string) (*Setting, error) {
v, err := GetSettings(ctx, []string{key})
if err != nil {
return nil, err
Expand All @@ -93,11 +93,11 @@ func GetSettingNoCache(ctx context.Context, key string) (*Setting, error) {

const contextCacheKey = "system_setting"

// GetSetting returns the setting value via the key
func GetSetting(ctx context.Context, key string) (string, error) {
// GetSettingWithCache returns the setting value via the key
func GetSettingWithCache(ctx context.Context, key string) (string, error) {
return cache.GetWithContextCache(ctx, contextCacheKey, key, func() (string, error) {
return cache.GetString(genSettingCacheKey(key), func() (string, error) {
res, err := GetSettingNoCache(ctx, key)
res, err := GetSetting(ctx, key)
if err != nil {
return "", err
}
Expand All @@ -110,6 +110,15 @@ func GetSetting(ctx context.Context, key string) (string, error) {
// none existing keys and errors are ignored and result in false
func GetSettingBool(ctx context.Context, key string) bool {
s, _ := GetSetting(ctx, key)
if s == nil {
return false
}
v, _ := strconv.ParseBool(s.SettingValue)
return v
}

func GetSettingWithCacheBool(ctx context.Context, key string) bool {
s, _ := GetSettingWithCache(ctx, key)
v, _ := strconv.ParseBool(s)
return v
}
Expand All @@ -120,7 +129,7 @@ func GetSettings(ctx context.Context, keys []string) (map[string]*Setting, error
keys[i] = strings.ToLower(keys[i])
}
settings := make([]*Setting, 0, len(keys))
if err := db.GetEngine(db.DefaultContext).
if err := db.GetEngine(ctx).
Where(builder.In("setting_key", keys)).
Find(&settings); err != nil {
return nil, err
Expand Down Expand Up @@ -151,9 +160,9 @@ func (settings AllSettings) GetVersion(key string) int {
}

// GetAllSettings returns all settings from user
func GetAllSettings() (AllSettings, error) {
func GetAllSettings(ctx context.Context) (AllSettings, error) {
settings := make([]*Setting, 0, 5)
if err := db.GetEngine(db.DefaultContext).
if err := db.GetEngine(ctx).
Find(&settings); err != nil {
return nil, err
}
Expand All @@ -168,12 +177,12 @@ func GetAllSettings() (AllSettings, error) {
func DeleteSetting(ctx context.Context, setting *Setting) error {
cache.RemoveContextData(ctx, contextCacheKey, setting.SettingKey)
cache.Remove(genSettingCacheKey(setting.SettingKey))
_, err := db.GetEngine(db.DefaultContext).Delete(setting)
_, err := db.GetEngine(ctx).Delete(setting)
return err
}

func SetSettingNoVersion(ctx context.Context, key, value string) error {
s, err := GetSettingNoCache(ctx, key)
s, err := GetSetting(ctx, key)
if IsErrSettingIsNotExist(err) {
return SetSetting(ctx, &Setting{
SettingKey: key,
Expand All @@ -189,7 +198,7 @@ func SetSettingNoVersion(ctx context.Context, key, value string) error {

// SetSetting updates a users' setting for a specific key
func SetSetting(ctx context.Context, setting *Setting) error {
if err := upsertSettingValue(strings.ToLower(setting.SettingKey), setting.SettingValue, setting.Version); err != nil {
if err := upsertSettingValue(ctx, strings.ToLower(setting.SettingKey), setting.SettingValue, setting.Version); err != nil {
return err
}

Expand All @@ -205,8 +214,8 @@ func SetSetting(ctx context.Context, setting *Setting) error {
return nil
}

func upsertSettingValue(key, value string, version int) error {
return db.WithTx(db.DefaultContext, func(ctx context.Context) error {
func upsertSettingValue(parentCtx context.Context, key, value string, version int) error {
return db.WithTx(parentCtx, func(ctx context.Context) error {
e := db.GetEngine(ctx)

// here we use a general method to do a safe upsert for different databases (and most transaction levels)
Expand Down Expand Up @@ -249,9 +258,9 @@ var (
LibravatarService *libravatar.Libravatar
)

func Init() error {
func Init(ctx context.Context) error {
var disableGravatar bool
disableGravatarSetting, err := GetSettingNoCache(db.DefaultContext, KeyPictureDisableGravatar)
disableGravatarSetting, err := GetSetting(ctx, KeyPictureDisableGravatar)
if IsErrSettingIsNotExist(err) {
disableGravatar = setting_module.GetDefaultDisableGravatar()
disableGravatarSetting = &Setting{SettingValue: strconv.FormatBool(disableGravatar)}
Expand All @@ -262,7 +271,7 @@ func Init() error {
}

var enableFederatedAvatar bool
enableFederatedAvatarSetting, err := GetSettingNoCache(db.DefaultContext, KeyPictureEnableFederatedAvatar)
enableFederatedAvatarSetting, err := GetSetting(ctx, KeyPictureEnableFederatedAvatar)
if IsErrSettingIsNotExist(err) {
enableFederatedAvatar = setting_module.GetDefaultEnableFederatedAvatar(disableGravatar)
enableFederatedAvatarSetting = &Setting{SettingValue: strconv.FormatBool(enableFederatedAvatar)}
Expand All @@ -275,13 +284,13 @@ func Init() error {
if setting_module.OfflineMode {
disableGravatar = true
enableFederatedAvatar = false
if !GetSettingBool(db.DefaultContext, KeyPictureDisableGravatar) {
if err := SetSettingNoVersion(db.DefaultContext, KeyPictureDisableGravatar, "true"); err != nil {
if !GetSettingBool(ctx, KeyPictureDisableGravatar) {
if err := SetSettingNoVersion(ctx, KeyPictureDisableGravatar, "true"); err != nil {
return fmt.Errorf("Failed to set setting %q: %w", KeyPictureDisableGravatar, err)
}
}
if GetSettingBool(db.DefaultContext, KeyPictureEnableFederatedAvatar) {
if err := SetSettingNoVersion(db.DefaultContext, KeyPictureEnableFederatedAvatar, "false"); err != nil {
if GetSettingBool(ctx, KeyPictureEnableFederatedAvatar) {
if err := SetSettingNoVersion(ctx, KeyPictureEnableFederatedAvatar, "false"); err != nil {
return fmt.Errorf("Failed to set setting %q: %w", KeyPictureEnableFederatedAvatar, err)
}
}
Expand Down
Loading

0 comments on commit 02f7298

Please sign in to comment.