Skip to content

Commit 02f7298

Browse files
zjjhotzjjhot
committed
Merge remote-tracking branch 'giteaoffical/main'
* 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)
2 parents 514311a + a8c4f8c commit 02f7298

37 files changed

+276
-86
lines changed

‎MAINTAINERS

+2-1
Original file line numberDiff line numberDiff line change
@@ -49,4 +49,5 @@ Jason Song <[email protected]> (@wolfogre)
4949
Yarden Shoham <[email protected]> (@yardenshoham)
5050
Yu Tian <[email protected]> (@Zettat123)
5151
Eddie Yang <[email protected]> (@yp05327)
52-
Dong Ge <[email protected]> (@sillyguodong)
52+
Dong Ge <[email protected]> (@sillyguodong)
53+
Xinyi Gong <[email protected]> (@HesterG)

‎models/actions/run.go

+5-7
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,13 @@ type ActionRun struct {
3232
OwnerID int64 `xorm:"index"`
3333
WorkflowID string `xorm:"index"` // the name of workflow file
3434
Index int64 `xorm:"index unique(repo_index)"` // a unique number for each run of a repository
35-
TriggerUserID int64
36-
TriggerUser *user_model.User `xorm:"-"`
35+
TriggerUserID int64 `xorm:"index"`
36+
TriggerUser *user_model.User `xorm:"-"`
3737
Ref string
3838
CommitSHA string
3939
IsForkPullRequest bool
40+
NeedApproval bool // may need approval if it's a fork pull request
41+
ApprovedBy int64 `xorm:"index"` // who approved
4042
Event webhook_module.HookEventType
4143
EventPayload string `xorm:"LONGTEXT"`
4244
Status Status `xorm:"index"`
@@ -164,10 +166,6 @@ func InsertRun(ctx context.Context, run *ActionRun, jobs []*jobparser.SingleWork
164166
}
165167
run.Index = index
166168

167-
if run.Status.IsUnknown() {
168-
run.Status = StatusWaiting
169-
}
170-
171169
if err := db.Insert(ctx, run); err != nil {
172170
return err
173171
}
@@ -191,7 +189,7 @@ func InsertRun(ctx context.Context, run *ActionRun, jobs []*jobparser.SingleWork
191189
job.EraseNeeds()
192190
payload, _ := v.Marshal()
193191
status := StatusWaiting
194-
if len(needs) > 0 {
192+
if len(needs) > 0 || run.NeedApproval {
195193
status = StatusBlocked
196194
}
197195
runJobs = append(runJobs, &ActionRunJob{

‎models/actions/run_list.go

+8
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,8 @@ type FindRunOptions struct {
6868
OwnerID int64
6969
IsClosed util.OptionalBool
7070
WorkflowFileName string
71+
TriggerUserID int64
72+
Approved bool // not util.OptionalBool, it works only when it's true
7173
}
7274

7375
func (opts FindRunOptions) toConds() builder.Cond {
@@ -89,6 +91,12 @@ func (opts FindRunOptions) toConds() builder.Cond {
8991
if opts.WorkflowFileName != "" {
9092
cond = cond.And(builder.Eq{"workflow_id": opts.WorkflowFileName})
9193
}
94+
if opts.TriggerUserID > 0 {
95+
cond = cond.And(builder.Eq{"trigger_user_id": opts.TriggerUserID})
96+
}
97+
if opts.Approved {
98+
cond = cond.And(builder.Gt{"approved_by": 0})
99+
}
92100
return cond
93101
}
94102

‎models/actions/status.go

+4
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,10 @@ func (s Status) IsRunning() bool {
8282
return s == StatusRunning
8383
}
8484

85+
func (s Status) IsBlocked() bool {
86+
return s == StatusBlocked
87+
}
88+
8589
// In returns whether s is one of the given statuses
8690
func (s Status) In(statuses ...Status) bool {
8791
for _, v := range statuses {

‎models/avatars/avatar.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ func generateEmailAvatarLink(ctx context.Context, email string, size int, final
153153
return DefaultAvatarLink()
154154
}
155155

156-
enableFederatedAvatar := system_model.GetSettingBool(ctx, system_model.KeyPictureEnableFederatedAvatar)
156+
enableFederatedAvatar := system_model.GetSettingWithCacheBool(ctx, system_model.KeyPictureEnableFederatedAvatar)
157157

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

177-
disableGravatar := system_model.GetSettingBool(ctx, system_model.KeyPictureDisableGravatar)
177+
disableGravatar := system_model.GetSettingWithCacheBool(ctx, system_model.KeyPictureDisableGravatar)
178178
if !disableGravatar {
179179
// copy GravatarSourceURL, because we will modify its Path.
180180
avatarURLCopy := *system_model.GravatarSourceURL

‎models/avatars/avatar_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ func enableGravatar(t *testing.T) {
2828
err := system_model.SetSettingNoVersion(db.DefaultContext, system_model.KeyPictureDisableGravatar, "false")
2929
assert.NoError(t, err)
3030
setting.GravatarSource = gravatarSource
31-
err = system_model.Init()
31+
err = system_model.Init(db.DefaultContext)
3232
assert.NoError(t, err)
3333
}
3434

‎models/db/list_options.go renamed to ‎models/db/list.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ func Find[T any](ctx context.Context, opts FindOptions, objects *[]T) error {
134134
if !opts.IsListAll() {
135135
sess.Limit(opts.GetSkipTake())
136136
}
137-
return sess.Find(&objects)
137+
return sess.Find(objects)
138138
}
139139

140140
// Count represents a common count function which accept an options interface
@@ -148,5 +148,5 @@ func FindAndCount[T any](ctx context.Context, opts FindOptions, objects *[]T) (i
148148
if !opts.IsListAll() {
149149
sess.Limit(opts.GetSkipTake())
150150
}
151-
return sess.FindAndCount(&objects)
151+
return sess.FindAndCount(objects)
152152
}

‎models/db/list_test.go

+48
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// Copyright 2023 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package db_test
5+
6+
import (
7+
"testing"
8+
9+
"code.gitea.io/gitea/models/db"
10+
repo_model "code.gitea.io/gitea/models/repo"
11+
"code.gitea.io/gitea/models/unittest"
12+
13+
"github.com/stretchr/testify/assert"
14+
"xorm.io/builder"
15+
)
16+
17+
type mockListOptions struct {
18+
db.ListOptions
19+
}
20+
21+
func (opts *mockListOptions) IsListAll() bool {
22+
return true
23+
}
24+
25+
func (opts *mockListOptions) ToConds() builder.Cond {
26+
return builder.NewCond()
27+
}
28+
29+
func TestFind(t *testing.T) {
30+
assert.NoError(t, unittest.PrepareTestDatabase())
31+
xe := unittest.GetXORMEngine()
32+
assert.NoError(t, xe.Sync(&repo_model.RepoUnit{}))
33+
34+
opts := mockListOptions{}
35+
var repoUnits []repo_model.RepoUnit
36+
err := db.Find(db.DefaultContext, &opts, &repoUnits)
37+
assert.NoError(t, err)
38+
assert.EqualValues(t, 83, len(repoUnits))
39+
40+
cnt, err := db.Count(db.DefaultContext, &opts, new(repo_model.RepoUnit))
41+
assert.NoError(t, err)
42+
assert.EqualValues(t, 83, cnt)
43+
44+
repoUnits = make([]repo_model.RepoUnit, 0, 10)
45+
newCnt, err := db.FindAndCount(db.DefaultContext, &opts, &repoUnits)
46+
assert.NoError(t, err)
47+
assert.EqualValues(t, cnt, newCnt)
48+
}

‎models/migrations/migrations.go

+4
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"code.gitea.io/gitea/models/migrations/v1_17"
2020
"code.gitea.io/gitea/models/migrations/v1_18"
2121
"code.gitea.io/gitea/models/migrations/v1_19"
22+
"code.gitea.io/gitea/models/migrations/v1_20"
2223
"code.gitea.io/gitea/models/migrations/v1_6"
2324
"code.gitea.io/gitea/models/migrations/v1_7"
2425
"code.gitea.io/gitea/models/migrations/v1_8"
@@ -463,6 +464,9 @@ var migrations = []Migration{
463464
NewMigration("Add exclusive label", v1_19.AddExclusiveLabel),
464465

465466
// Gitea 1.19.0 ends at v244
467+
468+
// v244 -> v245
469+
NewMigration("Add NeedApproval to actions tables", v1_20.AddNeedApprovalToActionRun),
466470
}
467471

468472
// GetCurrentDBVersion returns the current db version

‎models/migrations/v1_20/v244.go

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// Copyright 2023 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package v1_20 //nolint
5+
6+
import (
7+
"xorm.io/xorm"
8+
)
9+
10+
func AddNeedApprovalToActionRun(x *xorm.Engine) error {
11+
/*
12+
New index: TriggerUserID
13+
New fields: NeedApproval, ApprovedBy
14+
*/
15+
type ActionRun struct {
16+
TriggerUserID int64 `xorm:"index"`
17+
NeedApproval bool // may need approval if it's a fork pull request
18+
ApprovedBy int64 `xorm:"index"` // who approved
19+
}
20+
21+
return x.Sync(new(ActionRun))
22+
}

‎models/project/project.go

+3-10
Original file line numberDiff line numberDiff line change
@@ -217,16 +217,8 @@ func CountProjects(ctx context.Context, opts SearchOptions) (int64, error) {
217217

218218
// FindProjects returns a list of all projects that have been created in the repository
219219
func FindProjects(ctx context.Context, opts SearchOptions) ([]*Project, int64, error) {
220-
e := db.GetEngine(ctx)
220+
e := db.GetEngine(ctx).Where(opts.toConds())
221221
projects := make([]*Project, 0, setting.UI.IssuePagingNum)
222-
cond := opts.toConds()
223-
224-
count, err := e.Where(cond).Count(new(Project))
225-
if err != nil {
226-
return nil, 0, fmt.Errorf("Count: %w", err)
227-
}
228-
229-
e = e.Where(cond)
230222

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

246-
return projects, count, e.Find(&projects)
238+
count, err := e.FindAndCount(&projects)
239+
return projects, count, err
247240
}
248241

249242
// NewProject creates a new Project

‎models/repo.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,9 @@ import (
3939
var ItemsPerPage = 40
4040

4141
// Init initialize model
42-
func Init() error {
42+
func Init(ctx context.Context) error {
4343
unit.LoadUnitConfig()
44-
return system_model.Init()
44+
return system_model.Init(ctx)
4545
}
4646

4747
// DeleteRepository deletes a repository for a user or organization.

‎models/system/setting.go

+29-20
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,8 @@ func IsErrDataExpired(err error) bool {
7979
return ok
8080
}
8181

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

9494
const contextCacheKey = "system_setting"
9595

96-
// GetSetting returns the setting value via the key
97-
func GetSetting(ctx context.Context, key string) (string, error) {
96+
// GetSettingWithCache returns the setting value via the key
97+
func GetSettingWithCache(ctx context.Context, key string) (string, error) {
9898
return cache.GetWithContextCache(ctx, contextCacheKey, key, func() (string, error) {
9999
return cache.GetString(genSettingCacheKey(key), func() (string, error) {
100-
res, err := GetSettingNoCache(ctx, key)
100+
res, err := GetSetting(ctx, key)
101101
if err != nil {
102102
return "", err
103103
}
@@ -110,6 +110,15 @@ func GetSetting(ctx context.Context, key string) (string, error) {
110110
// none existing keys and errors are ignored and result in false
111111
func GetSettingBool(ctx context.Context, key string) bool {
112112
s, _ := GetSetting(ctx, key)
113+
if s == nil {
114+
return false
115+
}
116+
v, _ := strconv.ParseBool(s.SettingValue)
117+
return v
118+
}
119+
120+
func GetSettingWithCacheBool(ctx context.Context, key string) bool {
121+
s, _ := GetSettingWithCache(ctx, key)
113122
v, _ := strconv.ParseBool(s)
114123
return v
115124
}
@@ -120,7 +129,7 @@ func GetSettings(ctx context.Context, keys []string) (map[string]*Setting, error
120129
keys[i] = strings.ToLower(keys[i])
121130
}
122131
settings := make([]*Setting, 0, len(keys))
123-
if err := db.GetEngine(db.DefaultContext).
132+
if err := db.GetEngine(ctx).
124133
Where(builder.In("setting_key", keys)).
125134
Find(&settings); err != nil {
126135
return nil, err
@@ -151,9 +160,9 @@ func (settings AllSettings) GetVersion(key string) int {
151160
}
152161

153162
// GetAllSettings returns all settings from user
154-
func GetAllSettings() (AllSettings, error) {
163+
func GetAllSettings(ctx context.Context) (AllSettings, error) {
155164
settings := make([]*Setting, 0, 5)
156-
if err := db.GetEngine(db.DefaultContext).
165+
if err := db.GetEngine(ctx).
157166
Find(&settings); err != nil {
158167
return nil, err
159168
}
@@ -168,12 +177,12 @@ func GetAllSettings() (AllSettings, error) {
168177
func DeleteSetting(ctx context.Context, setting *Setting) error {
169178
cache.RemoveContextData(ctx, contextCacheKey, setting.SettingKey)
170179
cache.Remove(genSettingCacheKey(setting.SettingKey))
171-
_, err := db.GetEngine(db.DefaultContext).Delete(setting)
180+
_, err := db.GetEngine(ctx).Delete(setting)
172181
return err
173182
}
174183

175184
func SetSettingNoVersion(ctx context.Context, key, value string) error {
176-
s, err := GetSettingNoCache(ctx, key)
185+
s, err := GetSetting(ctx, key)
177186
if IsErrSettingIsNotExist(err) {
178187
return SetSetting(ctx, &Setting{
179188
SettingKey: key,
@@ -189,7 +198,7 @@ func SetSettingNoVersion(ctx context.Context, key, value string) error {
189198

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

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

208-
func upsertSettingValue(key, value string, version int) error {
209-
return db.WithTx(db.DefaultContext, func(ctx context.Context) error {
217+
func upsertSettingValue(parentCtx context.Context, key, value string, version int) error {
218+
return db.WithTx(parentCtx, func(ctx context.Context) error {
210219
e := db.GetEngine(ctx)
211220

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

252-
func Init() error {
261+
func Init(ctx context.Context) error {
253262
var disableGravatar bool
254-
disableGravatarSetting, err := GetSettingNoCache(db.DefaultContext, KeyPictureDisableGravatar)
263+
disableGravatarSetting, err := GetSetting(ctx, KeyPictureDisableGravatar)
255264
if IsErrSettingIsNotExist(err) {
256265
disableGravatar = setting_module.GetDefaultDisableGravatar()
257266
disableGravatarSetting = &Setting{SettingValue: strconv.FormatBool(disableGravatar)}
@@ -262,7 +271,7 @@ func Init() error {
262271
}
263272

264273
var enableFederatedAvatar bool
265-
enableFederatedAvatarSetting, err := GetSettingNoCache(db.DefaultContext, KeyPictureEnableFederatedAvatar)
274+
enableFederatedAvatarSetting, err := GetSetting(ctx, KeyPictureEnableFederatedAvatar)
266275
if IsErrSettingIsNotExist(err) {
267276
enableFederatedAvatar = setting_module.GetDefaultEnableFederatedAvatar(disableGravatar)
268277
enableFederatedAvatarSetting = &Setting{SettingValue: strconv.FormatBool(enableFederatedAvatar)}
@@ -275,13 +284,13 @@ func Init() error {
275284
if setting_module.OfflineMode {
276285
disableGravatar = true
277286
enableFederatedAvatar = false
278-
if !GetSettingBool(db.DefaultContext, KeyPictureDisableGravatar) {
279-
if err := SetSettingNoVersion(db.DefaultContext, KeyPictureDisableGravatar, "true"); err != nil {
287+
if !GetSettingBool(ctx, KeyPictureDisableGravatar) {
288+
if err := SetSettingNoVersion(ctx, KeyPictureDisableGravatar, "true"); err != nil {
280289
return fmt.Errorf("Failed to set setting %q: %w", KeyPictureDisableGravatar, err)
281290
}
282291
}
283-
if GetSettingBool(db.DefaultContext, KeyPictureEnableFederatedAvatar) {
284-
if err := SetSettingNoVersion(db.DefaultContext, KeyPictureEnableFederatedAvatar, "false"); err != nil {
292+
if GetSettingBool(ctx, KeyPictureEnableFederatedAvatar) {
293+
if err := SetSettingNoVersion(ctx, KeyPictureEnableFederatedAvatar, "false"); err != nil {
285294
return fmt.Errorf("Failed to set setting %q: %w", KeyPictureEnableFederatedAvatar, err)
286295
}
287296
}

0 commit comments

Comments
 (0)