Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjust transaction handling via db.Context #20031

Merged
merged 4 commits into from
Jun 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 19 additions & 26 deletions models/db/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,39 +23,46 @@ type contextKey struct {
name string
}

// EnginedContextKey is a context key. It is used with context.Value() to get the current Engined for the context
var EnginedContextKey = &contextKey{"engined"}
// enginedContextKey is a context key. It is used with context.Value() to get the current Engined for the context
var enginedContextKey = &contextKey{"engined"}
var _ Engined = &Context{}

// Context represents a db context
type Context struct {
context.Context
e Engine
e Engine
transaction bool
}

// WithEngine returns a Context from a context.Context and Engine
func WithEngine(ctx context.Context, e Engine) *Context {
func newContext(ctx context.Context, e Engine, transaction bool) *Context {
return &Context{
Context: ctx,
e: e.Context(ctx),
Context: ctx,
e: e,
transaction: transaction,
}
}

// InTransaction if context is in a transaction
func (ctx *Context) InTransaction() bool {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What exactly is the purpose of this method?
It seems currently unused.
And also, when would that be good to know?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a requirement from #19513 , I think there are some requirements in some place but previously there is no suitable function to use.

return ctx.transaction
}

// Engine returns db engine
func (ctx *Context) Engine() Engine {
return ctx.e
}

// Value shadows Value for context.Context but allows us to get ourselves and an Engined object
func (ctx *Context) Value(key interface{}) interface{} {
if key == EnginedContextKey {
if key == enginedContextKey {
return ctx
}
return ctx.Context.Value(key)
}

// WithContext returns this engine tied to this context
func (ctx *Context) WithContext(other context.Context) *Context {
return WithEngine(other, ctx.e)
return newContext(ctx, ctx.e.Context(other), ctx.transaction)
}

// Engined structs provide an Engine
Expand All @@ -68,7 +75,7 @@ func GetEngine(ctx context.Context) Engine {
if engined, ok := ctx.(Engined); ok {
return engined.Engine()
}
enginedInterface := ctx.Value(EnginedContextKey)
enginedInterface := ctx.Value(enginedContextKey)
if enginedInterface != nil {
return enginedInterface.(Engined).Engine()
}
Expand All @@ -89,18 +96,7 @@ func TxContext() (*Context, Committer, error) {
return nil, nil, err
}

return &Context{
Context: DefaultContext,
e: sess,
}, sess, nil
}

// WithContext represents executing database operations
func WithContext(f func(ctx *Context) error) error {
return f(&Context{
Context: DefaultContext,
e: x,
})
return newContext(DefaultContext, sess, true), sess, nil
}

// WithTx represents executing database operations on a transaction
Expand All @@ -118,10 +114,7 @@ func WithTx(f func(ctx context.Context) error, stdCtx ...context.Context) error
return err
}

if err := f(&Context{
Context: parentCtx,
e: sess,
}); err != nil {
if err := f(newContext(parentCtx, sess, true)); err != nil {
return err
}

Expand Down
16 changes: 8 additions & 8 deletions models/organization/org.go
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,7 @@ type accessibleReposEnv struct {
user *user_model.User
team *Team
teamIDs []int64
e db.Engine
ctx context.Context
keyword string
orderBy db.SearchOrderBy
}
Expand All @@ -706,7 +706,7 @@ func AccessibleReposEnv(ctx context.Context, org *Organization, userID int64) (A
org: org,
user: user,
teamIDs: teamIDs,
e: db.GetEngine(ctx),
ctx: ctx,
orderBy: db.SearchOrderByRecentUpdated,
}, nil
}
Expand All @@ -717,7 +717,7 @@ func (org *Organization) AccessibleTeamReposEnv(team *Team) AccessibleReposEnvir
return &accessibleReposEnv{
org: org,
team: team,
e: db.GetEngine(db.DefaultContext),
ctx: db.DefaultContext,
orderBy: db.SearchOrderByRecentUpdated,
}
}
Expand All @@ -744,7 +744,7 @@ func (env *accessibleReposEnv) cond() builder.Cond {
}

func (env *accessibleReposEnv) CountRepos() (int64, error) {
repoCount, err := env.e.
repoCount, err := db.GetEngine(env.ctx).
Join("INNER", "team_repo", "`team_repo`.repo_id=`repository`.id").
Where(env.cond()).
Distinct("`repository`.id").
Expand All @@ -761,7 +761,7 @@ func (env *accessibleReposEnv) RepoIDs(page, pageSize int) ([]int64, error) {
}

repoIDs := make([]int64, 0, pageSize)
return repoIDs, env.e.
return repoIDs, db.GetEngine(env.ctx).
Table("repository").
Join("INNER", "team_repo", "`team_repo`.repo_id=`repository`.id").
Where(env.cond()).
Expand All @@ -783,15 +783,15 @@ func (env *accessibleReposEnv) Repos(page, pageSize int) ([]*repo_model.Reposito
return repos, nil
}

return repos, env.e.
return repos, db.GetEngine(env.ctx).
In("`repository`.id", repoIDs).
OrderBy(string(env.orderBy)).
Find(&repos)
}

func (env *accessibleReposEnv) MirrorRepoIDs() ([]int64, error) {
repoIDs := make([]int64, 0, 10)
return repoIDs, env.e.
return repoIDs, db.GetEngine(env.ctx).
Table("repository").
Join("INNER", "team_repo", "`team_repo`.repo_id=`repository`.id AND `repository`.is_mirror=?", true).
Where(env.cond()).
Expand All @@ -812,7 +812,7 @@ func (env *accessibleReposEnv) MirrorRepos() ([]*repo_model.Repository, error) {
return repos, nil
}

return repos, env.e.
return repos, db.GetEngine(env.ctx).
In("`repository`.id", repoIDs).
Find(&repos)
}
Expand Down
8 changes: 1 addition & 7 deletions models/repo/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -319,13 +319,7 @@ func (repo *Repository) LoadUnits(ctx context.Context) (err error) {

// UnitEnabled if this repository has the given unit enabled
func (repo *Repository) UnitEnabled(tp unit.Type) (result bool) {
if err := db.WithContext(func(ctx *db.Context) error {
result = repo.UnitEnabledCtx(ctx, tp)
return nil
}); err != nil {
log.Error("repo.UnitEnabled: %v", err)
}
return result
return repo.UnitEnabledCtx(db.DefaultContext, tp)
}

// UnitEnabled if this repository has the given unit enabled
Expand Down
2 changes: 1 addition & 1 deletion modules/repository/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ func UpdateRepository(ctx context.Context, repo *repo_model.Repository, visibili
}

// Create/Remove git-daemon-export-ok for git-daemon...
if err := CheckDaemonExportOK(db.WithEngine(ctx, e), repo); err != nil {
if err := CheckDaemonExportOK(ctx, repo); err != nil {
return err
}

Expand Down