From dd30d9d5c0f577cb6e084aae6de2752ad43474d8 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 7 Dec 2023 15:27:36 +0800 Subject: [PATCH] Remove GetByBean method because sometimes it's danger when query condition parameter is zero and also introduce new generic methods (#28220) The function `GetByBean` has an obvious defect that when the fields are empty values, it will be ignored. Then users will get a wrong result which is possibly used to make a security problem. To avoid the possibility, this PR removed function `GetByBean` and all references. And some new generic functions have been introduced to be used. The recommand usage like below. ```go // if query an object according id obj, err := db.GetByID[Object](ctx, id) // query with other conditions obj, err := db.Get[Object](ctx, builder.Eq{"a": a, "b":b}) ``` --- models/asymkey/ssh_key_deploy.go | 33 +++++++----------- models/asymkey/ssh_key_fingerprint.go | 5 ++- models/auth/session.go | 35 +++++++------------ models/auth/source.go | 4 +-- models/db/context.go | 46 ++++++++++++++++++++----- models/db/error.go | 18 ++++++++++ models/db/iterate_test.go | 6 ++-- models/git/lfs.go | 9 +++-- models/git/protected_branch.go | 14 ++++---- models/issues/assignees.go | 4 ++- models/issues/label.go | 40 +++++++-------------- models/issues/pull.go | 7 ++-- models/org_team.go | 22 ++++++------ models/organization/team.go | 10 +++--- models/perm/access/access.go | 8 +++-- models/system/setting.go | 11 +++--- models/system/setting_test.go | 11 +++--- models/user/email_address.go | 18 +++++----- models/user/external_login_user.go | 14 ++++---- models/webhook/hooktask.go | 9 ++--- modules/repository/collaborator.go | 19 +++++----- modules/repository/repo.go | 4 +-- services/lfs/server.go | 4 +-- services/pull/lfs.go | 4 +-- services/repository/files/update.go | 2 +- services/repository/files/upload.go | 2 +- services/repository/lfs_test.go | 2 +- tests/integration/lfs_getobject_test.go | 2 +- 28 files changed, 189 insertions(+), 174 deletions(-) diff --git a/models/asymkey/ssh_key_deploy.go b/models/asymkey/ssh_key_deploy.go index 8f9f454051..923c5020ed 100644 --- a/models/asymkey/ssh_key_deploy.go +++ b/models/asymkey/ssh_key_deploy.go @@ -131,24 +131,22 @@ func AddDeployKey(ctx context.Context, repoID int64, name, content string, readO } defer committer.Close() - pkey := &PublicKey{ - Fingerprint: fingerprint, - } - has, err := db.GetByBean(ctx, pkey) + pkey, exist, err := db.Get[PublicKey](ctx, builder.Eq{"fingerprint": fingerprint}) if err != nil { return nil, err - } - - if has { + } else if exist { if pkey.Type != KeyTypeDeploy { return nil, ErrKeyAlreadyExist{0, fingerprint, ""} } } else { // First time use this deploy key. - pkey.Mode = accessMode - pkey.Type = KeyTypeDeploy - pkey.Content = content - pkey.Name = name + pkey = &PublicKey{ + Fingerprint: fingerprint, + Mode: accessMode, + Type: KeyTypeDeploy, + Content: content, + Name: name, + } if err = addKey(ctx, pkey); err != nil { return nil, fmt.Errorf("addKey: %w", err) } @@ -164,11 +162,10 @@ 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, exist, err := db.GetByID[DeployKey](ctx, id) if err != nil { return nil, err - } else if !has { + } else if !exist { return nil, ErrDeployKeyNotExist{id, 0, 0} } return key, nil @@ -176,14 +173,10 @@ func GetDeployKeyByID(ctx context.Context, id int64) (*DeployKey, error) { // 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, exist, err := db.Get[DeployKey](ctx, builder.Eq{"key_id": keyID, "repo_id": repoID}) if err != nil { return nil, err - } else if !has { + } else if !exist { return nil, ErrDeployKeyNotExist{0, keyID, repoID} } return key, nil diff --git a/models/asymkey/ssh_key_fingerprint.go b/models/asymkey/ssh_key_fingerprint.go index 2d6af0e3d6..b9cfb1b251 100644 --- a/models/asymkey/ssh_key_fingerprint.go +++ b/models/asymkey/ssh_key_fingerprint.go @@ -15,6 +15,7 @@ import ( "code.gitea.io/gitea/modules/util" "golang.org/x/crypto/ssh" + "xorm.io/builder" ) // ___________.__ .__ __ @@ -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 { diff --git a/models/auth/session.go b/models/auth/session.go index 28f25170ee..60fdeaba7c 100644 --- a/models/auth/session.go +++ b/models/auth/session.go @@ -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 @@ -33,34 +35,28 @@ 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 { + session, exist, err := db.Get[Session](ctx, builder.Eq{"key": key}) + if err != nil { return nil, err - } else if !has { + } else if !exist { session.Expiry = timeutil.TimeStampNow() if err := db.Insert(ctx, &session); err != nil { 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 @@ -79,17 +75,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{ @@ -104,14 +96,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 diff --git a/models/auth/source.go b/models/auth/source.go index 5e77afddc3..8a372c1429 100644 --- a/models/auth/source.go +++ b/models/auth/source.go @@ -265,10 +265,10 @@ func IsSSPIEnabled(ctx context.Context) bool { return false } - exist, err := db.Exists[Source](ctx, FindSourcesOptions{ + exist, err := db.Exist[Source](ctx, FindSourcesOptions{ IsActive: util.OptionalBoolTrue, LoginType: SSPI, - }) + }.ToConds()) if err != nil { log.Error("Active SSPI Sources: %v", err) return false diff --git a/models/db/context.go b/models/db/context.go index 45765ef7d3..7b739f7e9f 100644 --- a/models/db/context.go +++ b/models/db/context.go @@ -173,9 +173,44 @@ 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) (object *T, exist bool, err error) { + if !cond.IsValid() { + return nil, false, ErrConditionRequired{} + } + + var bean T + has, err := GetEngine(ctx).Where(cond).NoAutoCondition().Get(&bean) + if err != nil { + return nil, false, err + } else if !has { + return nil, false, nil + } + return &bean, true, nil +} + +func GetByID[T any](ctx context.Context, id int64) (object *T, exist bool, err error) { + var bean T + has, err := GetEngine(ctx).ID(id).NoAutoCondition().Get(&bean) + if err != nil { + return nil, false, err + } else if !has { + return nil, false, nil + } + return &bean, true, nil +} + +func Exist[T any](ctx context.Context, cond builder.Cond) (bool, error) { + if !cond.IsValid() { + return false, ErrConditionRequired{} + } + + 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. @@ -264,8 +299,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) -} diff --git a/models/db/error.go b/models/db/error.go index 665e970e17..f601a15c01 100644 --- a/models/db/error.go +++ b/models/db/error.go @@ -72,3 +72,21 @@ func (err ErrNotExist) Error() string { func (err ErrNotExist) Unwrap() error { return util.ErrNotExist } + +// ErrConditionRequired represents an error which require condition. +type ErrConditionRequired struct{} + +// IsErrConditionRequired checks if an error is an ErrConditionRequired +func IsErrConditionRequired(err error) bool { + _, ok := err.(ErrConditionRequired) + return ok +} + +func (err ErrConditionRequired) Error() string { + return "condition is required" +} + +// Unwrap unwraps this as a ErrNotExist err +func (err ErrConditionRequired) Unwrap() error { + return util.ErrInvalidArgument +} diff --git a/models/db/iterate_test.go b/models/db/iterate_test.go index 5362f34075..0f6ba2cc94 100644 --- a/models/db/iterate_test.go +++ b/models/db/iterate_test.go @@ -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) diff --git a/models/git/lfs.go b/models/git/lfs.go index e8192f92c5..837dc9fd31 100644 --- a/models/git/lfs.go +++ b/models/git/lfs.go @@ -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) @@ -144,16 +144,15 @@ func NewLFSMetaObject(ctx context.Context, m *LFSMetaObject) (*LFSMetaObject, er } defer committer.Close() - has, err := db.GetByBean(ctx, m) + m, exist, err := db.Get[LFSMetaObject](ctx, builder.Eq{"repository_id": repoID, "oid": p.Oid}) if err != nil { return nil, err - } - - if has { + } else if exist { m.Existing = true return m, committer.Commit() } + m = &LFSMetaObject{Pointer: p, RepositoryID: repoID} if err = db.Insert(ctx, m); err != nil { return nil, err } diff --git a/models/git/protected_branch.go b/models/git/protected_branch.go index 5be35f4b11..528acc175a 100644 --- a/models/git/protected_branch.go +++ b/models/git/protected_branch.go @@ -24,6 +24,7 @@ import ( "github.com/gobwas/glob" "github.com/gobwas/glob/syntax" + "xorm.io/builder" ) var ErrBranchIsProtected = errors.New("branch is protected") @@ -274,12 +275,11 @@ 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) + // branch_name is legacy name, it actually is rule name + rel, exist, err := db.Get[ProtectedBranch](ctx, builder.Eq{"repo_id": repoID, "branch_name": ruleName}) if err != nil { return nil, err - } - if !has { + } else if !exist { return nil, nil } return rel, nil @@ -287,12 +287,10 @@ func GetProtectedBranchRuleByName(ctx context.Context, repoID int64, ruleName st // 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, exist, err := db.Get[ProtectedBranch](ctx, builder.Eq{"repo_id": repoID, "id": ruleID}) if err != nil { return nil, err - } - if !has { + } else if !exist { return nil, nil } return rel, nil diff --git a/models/issues/assignees.go b/models/issues/assignees.go index fdd0d6f227..60f32d9557 100644 --- a/models/issues/assignees.go +++ b/models/issues/assignees.go @@ -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 @@ -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. diff --git a/models/issues/label.go b/models/issues/label.go index f8dbb9e39c..5c6b8e08d7 100644 --- a/models/issues/label.go +++ b/models/issues/label.go @@ -304,15 +304,11 @@ func GetLabelInRepoByName(ctx context.Context, repoID int64, labelName string) ( return nil, ErrRepoLabelNotExist{0, repoID} } - l := &Label{ - Name: labelName, - RepoID: repoID, - } - has, err := db.GetByBean(ctx, l) + l, exist, err := db.Get[Label](ctx, builder.Eq{"name": labelName, "repo_id": repoID}) if err != nil { return nil, err - } else if !has { - return nil, ErrRepoLabelNotExist{0, l.RepoID} + } else if !exist { + return nil, ErrRepoLabelNotExist{0, repoID} } return l, nil } @@ -323,15 +319,11 @@ func GetLabelInRepoByID(ctx context.Context, repoID, labelID int64) (*Label, err return nil, ErrRepoLabelNotExist{labelID, repoID} } - l := &Label{ - ID: labelID, - RepoID: repoID, - } - has, err := db.GetByBean(ctx, l) + l, exist, err := db.Get[Label](ctx, builder.Eq{"id": labelID, "repo_id": repoID}) if err != nil { return nil, err - } else if !has { - return nil, ErrRepoLabelNotExist{l.ID, l.RepoID} + } else if !exist { + return nil, ErrRepoLabelNotExist{labelID, repoID} } return l, nil } @@ -408,15 +400,11 @@ func GetLabelInOrgByName(ctx context.Context, orgID int64, labelName string) (*L return nil, ErrOrgLabelNotExist{0, orgID} } - l := &Label{ - Name: labelName, - OrgID: orgID, - } - has, err := db.GetByBean(ctx, l) + l, exist, err := db.Get[Label](ctx, builder.Eq{"name": labelName, "org_id": orgID}) if err != nil { return nil, err - } else if !has { - return nil, ErrOrgLabelNotExist{0, l.OrgID} + } else if !exist { + return nil, ErrOrgLabelNotExist{0, orgID} } return l, nil } @@ -427,15 +415,11 @@ func GetLabelInOrgByID(ctx context.Context, orgID, labelID int64) (*Label, error return nil, ErrOrgLabelNotExist{labelID, orgID} } - l := &Label{ - ID: labelID, - OrgID: orgID, - } - has, err := db.GetByBean(ctx, l) + l, exist, err := db.Get[Label](ctx, builder.Eq{"id": labelID, "org_id": orgID}) if err != nil { return nil, err - } else if !has { - return nil, ErrOrgLabelNotExist{l.ID, l.OrgID} + } else if !exist { + return nil, ErrOrgLabelNotExist{labelID, orgID} } return l, nil } diff --git a/models/issues/pull.go b/models/issues/pull.go index 2379c61976..c51a7daf4e 100644 --- a/models/issues/pull.go +++ b/models/issues/pull.go @@ -660,13 +660,10 @@ func GetPullRequestByIssueIDWithNoAttributes(ctx context.Context, issueID int64) // GetPullRequestByIssueID returns pull request by given issue ID. func GetPullRequestByIssueID(ctx context.Context, issueID int64) (*PullRequest, error) { - pr := &PullRequest{ - IssueID: issueID, - } - has, err := db.GetByBean(ctx, pr) + pr, exist, err := db.Get[PullRequest](ctx, builder.Eq{"issue_id": issueID}) if err != nil { return nil, err - } else if !has { + } else if !exist { return nil, ErrPullRequestNotExist{0, issueID, 0, 0, "", ""} } return pr, pr.LoadAttributes(ctx) diff --git a/models/org_team.go b/models/org_team.go index 03a4f6e98d..1a452436c3 100644 --- a/models/org_team.go +++ b/models/org_team.go @@ -162,7 +162,7 @@ func NewTeam(ctx context.Context, t *organization.Team) (err error) { return err } - has, err := db.GetEngine(ctx).ID(t.OrgID).Get(new(user_model.User)) + has, err := db.ExistByID[user_model.User](ctx, t.OrgID) if err != nil { return err } @@ -171,10 +171,10 @@ func NewTeam(ctx context.Context, t *organization.Team) (err error) { } t.LowerName = strings.ToLower(t.Name) - has, err = db.GetEngine(ctx). - Where("org_id=?", t.OrgID). - And("lower_name=?", t.LowerName). - Get(new(organization.Team)) + has, err = db.Exist[organization.Team](ctx, builder.Eq{ + "org_id": t.OrgID, + "lower_name": t.LowerName, + }) if err != nil { return err } @@ -232,20 +232,20 @@ func UpdateTeam(ctx context.Context, t *organization.Team, authChanged, includeA return err } defer committer.Close() - sess := db.GetEngine(ctx) t.LowerName = strings.ToLower(t.Name) - has, err := sess. - Where("org_id=?", t.OrgID). - And("lower_name=?", t.LowerName). - And("id!=?", t.ID). - Get(new(organization.Team)) + has, err := db.Exist[organization.Team](ctx, builder.Eq{ + "org_id": t.OrgID, + "lower_name": t.LowerName, + }.And(builder.Neq{"id": t.ID}), + ) if err != nil { return err } else if has { return organization.ErrTeamAlreadyExist{OrgID: t.OrgID, Name: t.LowerName} } + sess := db.GetEngine(ctx) if _, err = sess.ID(t.ID).Cols("name", "lower_name", "description", "can_create_org_repo", "authorize", "includes_all_repositories").Update(t); err != nil { return fmt.Errorf("update: %w", err) diff --git a/models/organization/team.go b/models/organization/team.go index 72afc67369..17db11c42d 100644 --- a/models/organization/team.go +++ b/models/organization/team.go @@ -16,6 +16,8 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/util" + + "xorm.io/builder" ) // ___________ @@ -203,14 +205,10 @@ func IsUsableTeamName(name string) error { // GetTeam returns team by given team name and organization. func GetTeam(ctx context.Context, orgID int64, name string) (*Team, error) { - t := &Team{ - OrgID: orgID, - LowerName: strings.ToLower(name), - } - has, err := db.GetByBean(ctx, t) + t, exist, err := db.Get[Team](ctx, builder.Eq{"org_id": orgID, "lower_name": strings.ToLower(name)}) if err != nil { return nil, err - } else if !has { + } else if !exist { return nil, ErrTeamNotExist{orgID, 0, name} } return t, nil diff --git a/models/perm/access/access.go b/models/perm/access/access.go index 2d1b2daa62..3e2568b4b4 100644 --- a/models/perm/access/access.go +++ b/models/perm/access/access.go @@ -13,6 +13,8 @@ import ( "code.gitea.io/gitea/models/perm" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" + + "xorm.io/builder" ) // Access represents the highest access level of a user to the repository. The only access type @@ -51,9 +53,11 @@ func accessLevel(ctx context.Context, user *user_model.User, repo *repo_model.Re return perm.AccessModeOwner, nil } - a := &Access{UserID: userID, RepoID: repo.ID} - if has, err := db.GetByBean(ctx, a); !has || err != nil { + a, exist, err := db.Get[Access](ctx, builder.Eq{"user_id": userID, "repo_id": repo.ID}) + if err != nil { return mode, err + } else if !exist { + return mode, nil } return a.Mode, nil } diff --git a/models/system/setting.go b/models/system/setting.go index cd1d373b98..4472b4c228 100644 --- a/models/system/setting.go +++ b/models/system/setting.go @@ -13,6 +13,8 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting/config" "code.gitea.io/gitea/modules/timeutil" + + "xorm.io/builder" ) type Setting struct { @@ -36,16 +38,17 @@ func init() { const keyRevision = "revision" func GetRevision(ctx context.Context) int { - revision := &Setting{SettingKey: keyRevision} - if has, err := db.GetByBean(ctx, revision); err != nil { + revision, exist, err := db.Get[Setting](ctx, builder.Eq{"setting_key": keyRevision}) + if err != nil { return 0 - } else if !has { + } else if !exist { err = db.Insert(ctx, &Setting{SettingKey: keyRevision, Version: 1}) if err != nil { return 0 } return 1 - } else if revision.Version <= 0 || revision.Version >= math.MaxInt-1 { + } + if revision.Version <= 0 || revision.Version >= math.MaxInt-1 { _, err = db.Exec(ctx, "UPDATE system_setting SET version=1 WHERE setting_key=?", keyRevision) if err != nil { return 0 diff --git a/models/system/setting_test.go b/models/system/setting_test.go index 0316ea144d..8f04412fb4 100644 --- a/models/system/setting_test.go +++ b/models/system/setting_test.go @@ -41,14 +41,11 @@ func TestSettings(t *testing.T) { assert.EqualValues(t, "false", settings[keyName]) // setting the same value should not trigger DuplicateKey error, and the "version" should be increased - setting := &system.Setting{SettingKey: keyName} - _, err = db.GetByBean(db.DefaultContext, setting) - assert.NoError(t, err) - assert.EqualValues(t, 2, setting.Version) err = system.SetSettings(db.DefaultContext, map[string]string{keyName: "false"}) assert.NoError(t, err) - setting = &system.Setting{SettingKey: keyName} - _, err = db.GetByBean(db.DefaultContext, setting) + + rev, settings, err = system.GetAllSettings(db.DefaultContext) assert.NoError(t, err) - assert.EqualValues(t, 3, setting.Version) + assert.Len(t, settings, 2) + assert.EqualValues(t, 4, rev) } diff --git a/models/user/email_address.go b/models/user/email_address.go index f1ed6692cf..2af2621f5f 100644 --- a/models/user/email_address.go +++ b/models/user/email_address.go @@ -527,12 +527,13 @@ func ActivateUserEmail(ctx context.Context, userID int64, email string, activate // Activate/deactivate a user's secondary email address // First check if there's another user active with the same address - addr := EmailAddress{UID: userID, LowerEmail: strings.ToLower(email)} - if has, err := db.GetByBean(ctx, &addr); err != nil { + addr, exist, err := db.Get[EmailAddress](ctx, builder.Eq{"uid": userID, "lower_email": strings.ToLower(email)}) + if err != nil { return err - } else if !has { + } else if !exist { return fmt.Errorf("no such email: %d (%s)", userID, email) } + if addr.IsActivated == activate { // Already in the desired state; no action return nil @@ -544,25 +545,26 @@ func ActivateUserEmail(ctx context.Context, userID int64, email string, activate return ErrEmailAlreadyUsed{Email: email} } } - if err = updateActivation(ctx, &addr, activate); err != nil { + if err = updateActivation(ctx, addr, activate); err != nil { return fmt.Errorf("unable to updateActivation() for %d:%s: %w", addr.ID, addr.Email, err) } // Activate/deactivate a user's primary email address and account if addr.IsPrimary { - user := User{ID: userID, Email: email} - if has, err := db.GetByBean(ctx, &user); err != nil { + user, exist, err := db.Get[User](ctx, builder.Eq{"id": userID, "email": email}) + if err != nil { return err - } else if !has { + } else if !exist { return fmt.Errorf("no user with ID: %d and Email: %s", userID, email) } + // The user's activation state should be synchronized with the primary email if user.IsActive != activate { user.IsActive = activate if user.Rands, err = GetUserSalt(); err != nil { return fmt.Errorf("unable to generate salt: %w", err) } - if err = UpdateUserCols(ctx, &user, "is_active", "rands"); err != nil { + if err = UpdateUserCols(ctx, user, "is_active", "rands"); err != nil { return fmt.Errorf("unable to updateUserCols() for user ID: %d: %w", userID, err) } } diff --git a/models/user/external_login_user.go b/models/user/external_login_user.go index 0db702f225..965b7a5ed1 100644 --- a/models/user/external_login_user.go +++ b/models/user/external_login_user.go @@ -98,9 +98,10 @@ func GetExternalLogin(ctx context.Context, externalLoginUser *ExternalLoginUser) // LinkExternalToUser link the external user to the user func LinkExternalToUser(ctx context.Context, user *User, externalLoginUser *ExternalLoginUser) error { - has, err := db.GetEngine(ctx).Where("external_id=? AND login_source_id=?", externalLoginUser.ExternalID, externalLoginUser.LoginSourceID). - NoAutoCondition(). - Exist(externalLoginUser) + has, err := db.Exist[ExternalLoginUser](ctx, builder.Eq{ + "external_id": externalLoginUser.ExternalID, + "login_source_id": externalLoginUser.LoginSourceID, + }) if err != nil { return err } else if has { @@ -145,9 +146,10 @@ func GetUserIDByExternalUserID(ctx context.Context, provider, userID string) (in // UpdateExternalUserByExternalID updates an external user's information func UpdateExternalUserByExternalID(ctx context.Context, external *ExternalLoginUser) error { - has, err := db.GetEngine(ctx).Where("external_id=? AND login_source_id=?", external.ExternalID, external.LoginSourceID). - NoAutoCondition(). - Exist(external) + has, err := db.Exist[ExternalLoginUser](ctx, builder.Eq{ + "external_id": external.ExternalID, + "login_source_id": external.LoginSourceID, + }) if err != nil { return err } else if !has { diff --git a/models/webhook/hooktask.go b/models/webhook/hooktask.go index 3ff27b93a7..2fb655ebca 100644 --- a/models/webhook/hooktask.go +++ b/models/webhook/hooktask.go @@ -16,6 +16,7 @@ import ( webhook_module "code.gitea.io/gitea/modules/webhook" gouuid "github.com/google/uuid" + "xorm.io/builder" ) // ___ ___ __ ___________ __ @@ -150,14 +151,10 @@ func UpdateHookTask(ctx context.Context, t *HookTask) error { // ReplayHookTask copies a hook task to get re-delivered func ReplayHookTask(ctx context.Context, hookID int64, uuid string) (*HookTask, error) { - task := &HookTask{ - HookID: hookID, - UUID: uuid, - } - has, err := db.GetByBean(ctx, task) + task, exist, err := db.Get[HookTask](ctx, builder.Eq{"hook_id": hookID, "uuid": uuid}) if err != nil { return nil, err - } else if !has { + } else if !exist { return nil, ErrHookTaskNotExist{ HookID: hookID, UUID: uuid, diff --git a/modules/repository/collaborator.go b/modules/repository/collaborator.go index f5cdc35045..ebe14e3a4c 100644 --- a/modules/repository/collaborator.go +++ b/modules/repository/collaborator.go @@ -11,24 +11,27 @@ import ( access_model "code.gitea.io/gitea/models/perm/access" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" + + "xorm.io/builder" ) func AddCollaborator(ctx context.Context, repo *repo_model.Repository, u *user_model.User) error { return db.WithTx(ctx, func(ctx context.Context) error { - collaboration := &repo_model.Collaboration{ - RepoID: repo.ID, - UserID: u.ID, - } - - has, err := db.GetByBean(ctx, collaboration) + has, err := db.Exist[repo_model.Collaboration](ctx, builder.Eq{ + "repo_id": repo.ID, + "user_id": u.ID, + }) if err != nil { return err } else if has { return nil } - collaboration.Mode = perm.AccessModeWrite - if err = db.Insert(ctx, collaboration); err != nil { + if err = db.Insert(ctx, &repo_model.Collaboration{ + RepoID: repo.ID, + UserID: u.ID, + Mode: perm.AccessModeWrite, + }); err != nil { return err } diff --git a/modules/repository/repo.go b/modules/repository/repo.go index 974449112f..a9a2773501 100644 --- a/modules/repository/repo.go +++ b/modules/repository/repo.go @@ -409,7 +409,7 @@ func StoreMissingLfsObjectsInRepository(ctx context.Context, repo *repo_model.Re defer content.Close() - _, err := git_model.NewLFSMetaObject(ctx, &git_model.LFSMetaObject{Pointer: p, RepositoryID: repo.ID}) + _, err := git_model.NewLFSMetaObject(ctx, repo.ID, p) if err != nil { log.Error("Repo[%-v]: Error creating LFS meta object %-v: %v", repo, p, err) return err @@ -456,7 +456,7 @@ func StoreMissingLfsObjectsInRepository(ctx context.Context, repo *repo_model.Re if exist { log.Trace("Repo[%-v]: LFS object %-v already present; creating meta object", repo, pointerBlob.Pointer) - _, err := git_model.NewLFSMetaObject(ctx, &git_model.LFSMetaObject{Pointer: pointerBlob.Pointer, RepositoryID: repo.ID}) + _, err := git_model.NewLFSMetaObject(ctx, repo.ID, pointerBlob.Pointer) if err != nil { log.Error("Repo[%-v]: Error creating LFS meta object %-v: %v", repo, pointerBlob.Pointer, err) return err diff --git a/services/lfs/server.go b/services/lfs/server.go index 58b4663345..62134b7d60 100644 --- a/services/lfs/server.go +++ b/services/lfs/server.go @@ -232,7 +232,7 @@ func BatchHandler(ctx *context.Context) { return } if accessible { - _, err := git_model.NewLFSMetaObject(ctx, &git_model.LFSMetaObject{Pointer: p, RepositoryID: repository.ID}) + _, err := git_model.NewLFSMetaObject(ctx, repository.ID, p) if err != nil { log.Error("Unable to create LFS MetaObject [%s] for %s/%s. Error: %v", p.Oid, rc.User, rc.Repo, err) writeStatus(ctx, http.StatusInternalServerError) @@ -325,7 +325,7 @@ func UploadHandler(ctx *context.Context) { log.Error("Error putting LFS MetaObject [%s] into content store. Error: %v", p.Oid, err) return err } - _, err := git_model.NewLFSMetaObject(ctx, &git_model.LFSMetaObject{Pointer: p, RepositoryID: repository.ID}) + _, err := git_model.NewLFSMetaObject(ctx, repository.ID, p) return err } diff --git a/services/pull/lfs.go b/services/pull/lfs.go index 2ca82b5633..ed03583d4f 100644 --- a/services/pull/lfs.go +++ b/services/pull/lfs.go @@ -127,9 +127,7 @@ func createLFSMetaObjectsFromCatFileBatch(ctx context.Context, catFileBatchReade // OK we have a pointer that is associated with the head repo // and is actually a file in the LFS // Therefore it should be associated with the base repo - meta := &git_model.LFSMetaObject{Pointer: pointer} - meta.RepositoryID = pr.BaseRepoID - if _, err := git_model.NewLFSMetaObject(ctx, meta); err != nil { + if _, err := git_model.NewLFSMetaObject(ctx, pr.BaseRepoID, pointer); err != nil { _ = catFileBatchReader.CloseWithError(err) break } diff --git a/services/repository/files/update.go b/services/repository/files/update.go index 2a08bcbace..42b98a2739 100644 --- a/services/repository/files/update.go +++ b/services/repository/files/update.go @@ -438,7 +438,7 @@ func CreateOrUpdateFile(ctx context.Context, t *TemporaryUploadRepository, file if lfsMetaObject != nil { // We have an LFS object - create it - lfsMetaObject, err = git_model.NewLFSMetaObject(ctx, lfsMetaObject) + lfsMetaObject, err = git_model.NewLFSMetaObject(ctx, lfsMetaObject.RepositoryID, lfsMetaObject.Pointer) if err != nil { return err } diff --git a/services/repository/files/upload.go b/services/repository/files/upload.go index f4e1da7bb1..6a1f2ccd16 100644 --- a/services/repository/files/upload.go +++ b/services/repository/files/upload.go @@ -143,7 +143,7 @@ func UploadRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use if infos[i].lfsMetaObject == nil { continue } - infos[i].lfsMetaObject, err = git_model.NewLFSMetaObject(ctx, infos[i].lfsMetaObject) + infos[i].lfsMetaObject, err = git_model.NewLFSMetaObject(ctx, infos[i].lfsMetaObject.RepositoryID, infos[i].lfsMetaObject.Pointer) if err != nil { // OK Now we need to cleanup return cleanUpAfterFailure(ctx, &infos, t, err) diff --git a/services/repository/lfs_test.go b/services/repository/lfs_test.go index 61348143d0..ee0b8f6b89 100644 --- a/services/repository/lfs_test.go +++ b/services/repository/lfs_test.go @@ -52,7 +52,7 @@ func storeObjectInRepo(t *testing.T, repositoryID int64, content *[]byte) string pointer, err := lfs.GeneratePointer(bytes.NewReader(*content)) assert.NoError(t, err) - _, err = git_model.NewLFSMetaObject(db.DefaultContext, &git_model.LFSMetaObject{Pointer: pointer, RepositoryID: repositoryID}) + _, err = git_model.NewLFSMetaObject(db.DefaultContext, repositoryID, pointer) assert.NoError(t, err) contentStore := lfs.NewContentStore() exist, err := contentStore.Exists(pointer) diff --git a/tests/integration/lfs_getobject_test.go b/tests/integration/lfs_getobject_test.go index fe070c62d5..a87f38be8a 100644 --- a/tests/integration/lfs_getobject_test.go +++ b/tests/integration/lfs_getobject_test.go @@ -29,7 +29,7 @@ func storeObjectInRepo(t *testing.T, repositoryID int64, content *[]byte) string pointer, err := lfs.GeneratePointer(bytes.NewReader(*content)) assert.NoError(t, err) - _, err = git_model.NewLFSMetaObject(db.DefaultContext, &git_model.LFSMetaObject{Pointer: pointer, RepositoryID: repositoryID}) + _, err = git_model.NewLFSMetaObject(db.DefaultContext, repositoryID, pointer) assert.NoError(t, err) contentStore := lfs.NewContentStore() exist, err := contentStore.Exists(pointer)