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

Remove GetByBean method because sometimes it's danger when query condition parameter is zero and also introduce new generic methods #28220

Merged
merged 19 commits into from
Dec 7, 2023
Merged
Show file tree
Hide file tree
Changes from 6 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
28 changes: 11 additions & 17 deletions models/asymkey/ssh_key_deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,15 +131,12 @@ func AddDeployKey(ctx context.Context, repoID int64, name, content string, readO
}
defer committer.Close()

pkey := &PublicKey{
Fingerprint: fingerprint,
}
has, err := db.GetByBean(ctx, pkey)
if err != nil {
pkey, err := db.Get[PublicKey](ctx, builder.Eq{"fingerprint": fingerprint})
if err != nil && !db.IsErrNotExist(err) {
return nil, err
}

if has {
if err == nil {
if pkey.Type != KeyTypeDeploy {
return nil, ErrKeyAlreadyExist{0, fingerprint, ""}
}
Expand All @@ -164,27 +161,24 @@ func AddDeployKey(ctx context.Context, repoID int64, name, content string, readO

// GetDeployKeyByID returns deploy key by given ID.
func GetDeployKeyByID(ctx context.Context, id int64) (*DeployKey, error) {
key := new(DeployKey)
has, err := db.GetEngine(ctx).ID(id).Get(key)
key, err := db.GetByID[DeployKey](ctx, id)
if err != nil {
if db.IsErrNotExist(err) {
return nil, ErrDeployKeyNotExist{0, 0, id}
}
return nil, err
} else if !has {
return nil, ErrDeployKeyNotExist{id, 0, 0}
}
return key, nil
}

// GetDeployKeyByRepo returns deploy key by given public key ID and repository ID.
func GetDeployKeyByRepo(ctx context.Context, keyID, repoID int64) (*DeployKey, error) {
key := &DeployKey{
KeyID: keyID,
RepoID: repoID,
}
has, err := db.GetByBean(ctx, key)
key, err := db.Get[DeployKey](ctx, builder.Eq{"key_id": keyID, "repo_id": repoID})
if err != nil {
if db.IsErrNotExist(err) {
return nil, ErrDeployKeyNotExist{0, keyID, repoID}
}
return nil, err
} else if !has {
return nil, ErrDeployKeyNotExist{0, keyID, repoID}
}
return key, nil
}
Expand Down
5 changes: 2 additions & 3 deletions models/asymkey/ssh_key_fingerprint.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"code.gitea.io/gitea/modules/util"

"golang.org/x/crypto/ssh"
"xorm.io/builder"
)

// ___________.__ .__ __
Expand All @@ -31,9 +32,7 @@ import (
// checkKeyFingerprint only checks if key fingerprint has been used as public key,
// it is OK to use same key as deploy key for multiple repositories/users.
func checkKeyFingerprint(ctx context.Context, fingerprint string) error {
has, err := db.GetByBean(ctx, &PublicKey{
Fingerprint: fingerprint,
})
has, err := db.Exist[PublicKey](ctx, builder.Eq{"fingerprint": fingerprint})
if err != nil {
return err
} else if has {
Expand Down
44 changes: 18 additions & 26 deletions models/auth/session.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (

"code.gitea.io/gitea/models/db"
"code.gitea.io/gitea/modules/timeutil"

"xorm.io/builder"
)

// Session represents a session compatible for go-chi session
Expand All @@ -33,34 +35,29 @@ func UpdateSession(ctx context.Context, key string, data []byte) error {

// ReadSession reads the data for the provided session
func ReadSession(ctx context.Context, key string) (*Session, error) {
session := Session{
Key: key,
}

ctx, committer, err := db.TxContext(ctx)
if err != nil {
return nil, err
}
defer committer.Close()

if has, err := db.GetByBean(ctx, &session); err != nil {
return nil, err
} else if !has {
session.Expiry = timeutil.TimeStampNow()
if err := db.Insert(ctx, &session); err != nil {
return nil, err
session, err := db.Get[Session](ctx, builder.Eq{"key": key})
if err != nil {
if db.IsErrNotExist(err) {
session.Expiry = timeutil.TimeStampNow()
if err := db.Insert(ctx, &session); err != nil {
return nil, err
}
}
return nil, err
}

return &session, committer.Commit()
return session, committer.Commit()
}

// ExistSession checks if a session exists
func ExistSession(ctx context.Context, key string) (bool, error) {
session := Session{
Key: key,
}
return db.GetEngine(ctx).Get(&session)
return db.Exist[Session](ctx, builder.Eq{"key": key})
}

// DestroySession destroys a session
Expand All @@ -79,17 +76,13 @@ func RegenerateSession(ctx context.Context, oldKey, newKey string) (*Session, er
}
defer committer.Close()

if has, err := db.GetByBean(ctx, &Session{
Key: newKey,
}); err != nil {
if has, err := db.Exist[Session](ctx, builder.Eq{"key": newKey}); err != nil {
return nil, err
} else if has {
return nil, fmt.Errorf("session Key: %s already exists", newKey)
}

if has, err := db.GetByBean(ctx, &Session{
Key: oldKey,
}); err != nil {
if has, err := db.Exist[Session](ctx, builder.Eq{"key": oldKey}); err != nil {
return nil, err
} else if !has {
if err := db.Insert(ctx, &Session{
Expand All @@ -104,14 +97,13 @@ func RegenerateSession(ctx context.Context, oldKey, newKey string) (*Session, er
return nil, err
}

s := Session{
Key: newKey,
}
if _, err := db.GetByBean(ctx, &s); err != nil {
s, err := db.Get[Session](ctx, builder.Eq{"key": newKey})
if err != nil {
// is not exist, it should be impossible
return nil, err
}

return &s, committer.Commit()
return s, committer.Commit()
}

// CountSessions returns the number of sessions
Expand Down
38 changes: 30 additions & 8 deletions models/db/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,9 +173,36 @@ func Exec(ctx context.Context, sqlAndArgs ...any) (sql.Result, error) {
return GetEngine(ctx).Exec(sqlAndArgs...)
}

// GetByBean filled empty fields of the bean according non-empty fields to query in database.
func GetByBean(ctx context.Context, bean any) (bool, error) {
return GetEngine(ctx).Get(bean)
func Get[T any](ctx context.Context, cond builder.Cond) (*T, error) {
var bean T
has, err := GetEngine(ctx).Where(cond).NoAutoCondition().Get(&bean)
lunny marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, err
} else if !has {
return nil, ErrNotExist{Resource: TableName(bean)}
}
return &bean, nil
}

func GetByID[T any](ctx context.Context, id int64) (*T, error) {
var bean T
has, err := GetEngine(ctx).ID(id).NoAutoCondition().Get(&bean)
if err != nil {
return nil, err
} else if !has {
return nil, ErrNotExist{Resource: TableName(bean), ID: id}
}
return &bean, nil
}

func Exist[T any](ctx context.Context, cond builder.Cond) (bool, error) {
var bean T
return GetEngine(ctx).Where(cond).NoAutoCondition().Exist(&bean)
}

func ExistByID[T any](ctx context.Context, id int64) (bool, error) {
var bean T
return GetEngine(ctx).ID(id).NoAutoCondition().Exist(&bean)
}

// DeleteByBean deletes all records according non-empty fields of the bean as conditions.
Expand Down Expand Up @@ -264,8 +291,3 @@ func inTransaction(ctx context.Context) (*xorm.Session, bool) {
return nil, false
}
}

func Exists[T any](ctx context.Context, opts FindOptions) (bool, error) {
var bean T
return GetEngine(ctx).Where(opts.ToConds()).Exist(&bean)
}
6 changes: 3 additions & 3 deletions models/db/iterate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ func TestIterate(t *testing.T) {
assert.EqualValues(t, cnt, repoUnitCnt)

err = db.Iterate(db.DefaultContext, nil, func(ctx context.Context, repoUnit *repo_model.RepoUnit) error {
reopUnit2 := repo_model.RepoUnit{ID: repoUnit.ID}
has, err := db.GetByBean(ctx, &reopUnit2)
has, err := db.ExistByID[repo_model.RepoUnit](ctx, repoUnit.ID)
if err != nil {
return err
} else if !has {
}
if !has {
return db.ErrNotExist{Resource: "repo_unit", ID: repoUnit.ID}
}
assert.EqualValues(t, repoUnit.RepoID, repoUnit.RepoID)
Expand Down
5 changes: 5 additions & 0 deletions models/db/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,3 +202,8 @@ func FindAndCount[T any](ctx context.Context, opts FindOptions) ([]*T, int64, er
}
return objects, cnt, nil
}

func Exists[T any](ctx context.Context, opts FindOptions) (bool, error) {
lunny marked this conversation as resolved.
Show resolved Hide resolved
var bean T
return GetEngine(ctx).Where(opts.ToConds()).Exist(&bean)
lunny marked this conversation as resolved.
Show resolved Hide resolved
}
9 changes: 5 additions & 4 deletions models/git/lfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ var ErrLFSObjectNotExist = db.ErrNotExist{Resource: "LFS Meta object"}

// NewLFSMetaObject stores a given populated LFSMetaObject structure in the database
// if it is not already present.
func NewLFSMetaObject(ctx context.Context, m *LFSMetaObject) (*LFSMetaObject, error) {
func NewLFSMetaObject(ctx context.Context, repoID int64, p lfs.Pointer) (*LFSMetaObject, error) {
var err error

ctx, committer, err := db.TxContext(ctx)
Expand All @@ -144,16 +144,17 @@ func NewLFSMetaObject(ctx context.Context, m *LFSMetaObject) (*LFSMetaObject, er
}
defer committer.Close()

has, err := db.GetByBean(ctx, m)
if err != nil {
m, err := db.Get[LFSMetaObject](ctx, builder.Eq{"repository_id": repoID, "oid": p.Oid})
if err != nil && !db.IsErrNotExist(err) {
return nil, err
}

if has {
if err == nil {
m.Existing = true
return m, committer.Commit()
}
lunny marked this conversation as resolved.
Show resolved Hide resolved

m = &LFSMetaObject{Pointer: p, RepositoryID: repoID}
if err = db.Insert(ctx, m); err != nil {
return nil, err
}
Expand Down
19 changes: 9 additions & 10 deletions models/git/protected_branch.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (

"github.com/gobwas/glob"
"github.com/gobwas/glob/syntax"
"xorm.io/builder"
)

var ErrBranchIsProtected = errors.New("branch is protected")
Expand Down Expand Up @@ -274,27 +275,25 @@ func (protectBranch *ProtectedBranch) IsUnprotectedFile(patterns []glob.Glob, pa

// GetProtectedBranchRuleByName getting protected branch rule by name
func GetProtectedBranchRuleByName(ctx context.Context, repoID int64, ruleName string) (*ProtectedBranch, error) {
rel := &ProtectedBranch{RepoID: repoID, RuleName: ruleName}
has, err := db.GetByBean(ctx, rel)
rel, err := db.Get[ProtectedBranch](ctx, builder.Eq{"repo_id": repoID, "rule_name": ruleName})
if err != nil {
if db.IsErrNotExist(err) {
return nil, nil
}
return nil, err
}
if !has {
return nil, nil
}
return rel, nil
}

// GetProtectedBranchRuleByID getting protected branch rule by rule ID
func GetProtectedBranchRuleByID(ctx context.Context, repoID, ruleID int64) (*ProtectedBranch, error) {
rel := &ProtectedBranch{ID: ruleID, RepoID: repoID}
has, err := db.GetByBean(ctx, rel)
rel, err := db.Get[ProtectedBranch](ctx, builder.Eq{"repo_id": repoID, "id": ruleID})
if err != nil {
if db.IsErrNotExist(err) {
return nil, nil
}
return nil, err
}
if !has {
return nil, nil
}
return rel, nil
}

Expand Down
4 changes: 3 additions & 1 deletion models/issues/assignees.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
"code.gitea.io/gitea/models/db"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/util"

"xorm.io/builder"
)

// IssueAssignees saves all issue assignees
Expand Down Expand Up @@ -59,7 +61,7 @@ func GetAssigneeIDsByIssue(ctx context.Context, issueID int64) ([]int64, error)

// IsUserAssignedToIssue returns true when the user is assigned to the issue
func IsUserAssignedToIssue(ctx context.Context, issue *Issue, user *user_model.User) (isAssigned bool, err error) {
return db.GetByBean(ctx, &IssueAssignees{IssueID: issue.ID, AssigneeID: user.ID})
return db.Exist[IssueAssignees](ctx, builder.Eq{"assignee_id": user.ID, "issue_id": issue.ID})
}

// ToggleIssueAssignee changes a user between assigned and not assigned for this issue, and make issue comment for it.
Expand Down
Loading
Loading