From 6840b252f905a2995e3c1f1bc51dc53cd2c96520 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 26 Nov 2023 09:29:03 +0800 Subject: [PATCH 01/15] Remove GetByBean method because sometimes it's danger when query condition parameter is zero --- models/asymkey/ssh_key_deploy.go | 17 +++++------- models/asymkey/ssh_key_fingerprint.go | 4 +-- models/auth/session.go | 26 ++++++++---------- models/db/context.go | 5 ---- models/db/iterate_test.go | 4 +-- models/git/lfs.go | 11 +++++--- models/git/protected_branch.go | 12 ++++++--- models/issues/assignees.go | 4 ++- models/issues/label.go | 36 +++++++++++-------------- models/issues/pull.go | 8 +++--- models/org_team.go | 6 ++++- models/organization/team.go | 9 +++---- models/perm/access/access.go | 6 +++-- models/system/setting.go | 6 +++-- models/user/email_address.go | 12 ++++++--- models/webhook/hooktask.go | 9 +++---- modules/repository/collaborator.go | 9 +++---- 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 +- 24 files changed, 101 insertions(+), 103 deletions(-) diff --git a/models/asymkey/ssh_key_deploy.go b/models/asymkey/ssh_key_deploy.go index 8f9f454051b73..7cc1062e3a4ef 100644 --- a/models/asymkey/ssh_key_deploy.go +++ b/models/asymkey/ssh_key_deploy.go @@ -131,10 +131,10 @@ 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 := &PublicKey{} + has, err := db.GetEngine(ctx).Where("fingerprint=?", fingerprint). + NoAutoCondition(). + Get(ctx, pkey) if err != nil { return nil, err } @@ -176,17 +176,14 @@ 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) + var key DeployKey + has, err := db.GetEngine(ctx).Where("key_id=? AND repo_id=?", keyID, repoID).Get(&key) if err != nil { return nil, err } else if !has { return nil, ErrDeployKeyNotExist{0, keyID, repoID} } - return key, nil + return &key, nil } // IsDeployKeyExistByKeyID return true if there is at least one deploykey with the key id diff --git a/models/asymkey/ssh_key_fingerprint.go b/models/asymkey/ssh_key_fingerprint.go index 2d6af0e3d6cdf..d7564376b13f1 100644 --- a/models/asymkey/ssh_key_fingerprint.go +++ b/models/asymkey/ssh_key_fingerprint.go @@ -31,9 +31,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.GetEngine(ctx).Where("fingerprint=?", fingerprint).Exist(&PublicKey{}) if err != nil { return err } else if has { diff --git a/models/auth/session.go b/models/auth/session.go index 28f25170eec93..ed5e7c40dde3e 100644 --- a/models/auth/session.go +++ b/models/auth/session.go @@ -33,17 +33,15 @@ 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 { + var session Session + + if has, err := db.GetEngine(ctx).Where("key=?", key).Get(ctx, &session); err != nil { return nil, err } else if !has { session.Expiry = timeutil.TimeStampNow() @@ -79,17 +77,17 @@ 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.GetEngine(ctx).Where("key=?", newKey). + NoAutoCondition(). + Exist(new(Session)); 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.GetEngine(ctx).Where("key=?", oldKey). + NoAutoCondition(). + Exist(new(Session)); err != nil { return nil, err } else if !has { if err := db.Insert(ctx, &Session{ @@ -104,10 +102,8 @@ 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 { + var s Session + if _, err := db.GetEngine(ctx).Where("key=?", newKey).Get(&s); err != nil { return nil, err } diff --git a/models/db/context.go b/models/db/context.go index 45765ef7d38c7..c70474816dc25 100644 --- a/models/db/context.go +++ b/models/db/context.go @@ -173,11 +173,6 @@ 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) -} - // DeleteByBean deletes all records according non-empty fields of the bean as conditions. func DeleteByBean(ctx context.Context, bean any) (int64, error) { return GetEngine(ctx).Delete(bean) diff --git a/models/db/iterate_test.go b/models/db/iterate_test.go index 5362f34075793..f9010f991d015 100644 --- a/models/db/iterate_test.go +++ b/models/db/iterate_test.go @@ -31,8 +31,8 @@ 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) + reopUnit2 := repo_model.RepoUnit{} + has, err := db.GetEngine(ctx).ID(repoUnit.ID).NoAutoCondition().Get(&reopUnit2) if err != nil { return err } else if !has { diff --git a/models/git/lfs.go b/models/git/lfs.go index e8192f92c5c68..cbd608c9b212e 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,21 +144,24 @@ func NewLFSMetaObject(ctx context.Context, m *LFSMetaObject) (*LFSMetaObject, er } defer committer.Close() - has, err := db.GetByBean(ctx, m) + m := LFSMetaObject{Pointer: p, RepositoryID: repoID} + has, err := db.GetEngine(ctx).Where("repo_id=? AND oid=?", repoID, p.Oid). + NoAutoCondition(). + Get(&m) if err != nil { return nil, err } if has { m.Existing = true - return m, committer.Commit() + return &m, committer.Commit() } if err = db.Insert(ctx, m); err != nil { return nil, err } - return m, committer.Commit() + return &m, committer.Commit() } // GetLFSMetaObjectByOid selects a LFSMetaObject entry from database by its OID. diff --git a/models/git/protected_branch.go b/models/git/protected_branch.go index 5be35f4b113c7..68b8b12247859 100644 --- a/models/git/protected_branch.go +++ b/models/git/protected_branch.go @@ -274,8 +274,10 @@ 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 := &ProtectedBranch{} + has, err := db.GetEngine(ctx).Where("repo_id=? AND rule_name=?", repoID, ruleName). + NoAutoCondition(). + Get(rel) if err != nil { return nil, err } @@ -287,8 +289,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 := &ProtectedBranch{} + has, err := db.GetEngine(ctx).Where("repo_id=? AND id=?", repoID, ruleID). + NoAutoCondition(). + Get(rel) if err != nil { return nil, err } diff --git a/models/issues/assignees.go b/models/issues/assignees.go index fdd0d6f2274b6..50d8bd0ce1836 100644 --- a/models/issues/assignees.go +++ b/models/issues/assignees.go @@ -59,7 +59,9 @@ 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.GetEngine(ctx).Where("assignee_id=? AND issue_id=?", user.ID, issue.ID). + NoAutoCondition(). + Exist(new(IssueAssignees)) } // 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 f8dbb9e39cd46..d4f29913fa3b3 100644 --- a/models/issues/label.go +++ b/models/issues/label.go @@ -304,11 +304,10 @@ 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 := &Label{} + has, err := db.GetEngine(ctx).Where("name=? AND repo_id=?", labelName, repoID). + NoAutoCondition(). + Get(l) if err != nil { return nil, err } else if !has { @@ -323,11 +322,10 @@ 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 := &Label{} + has, err := db.GetEngine(ctx).Where("id=? AND repo_id=?", labelID, repoID). + NoAutoCondition(). + Get(l) if err != nil { return nil, err } else if !has { @@ -408,11 +406,10 @@ 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 := &Label{} + has, err := db.GetEngine(ctx).Where("name = ? AND org_id=?", labelName, orgID). + NoAutoCondition(). + Get(l) if err != nil { return nil, err } else if !has { @@ -427,11 +424,10 @@ 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 := &Label{} + has, err := db.GetEngine(ctx).Where("id = ? AND org_id=?", labelID, orgID). + NoAutoCondition(). + Get(l) if err != nil { return nil, err } else if !has { diff --git a/models/issues/pull.go b/models/issues/pull.go index 2379c61976635..051d7d336b9f0 100644 --- a/models/issues/pull.go +++ b/models/issues/pull.go @@ -660,10 +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 := &PullRequest{} + has, err := db.GetEngine(ctx).Where("issue_id = ?", issueID). + NoAutoCondition(). + Get(pr) if err != nil { return nil, err } else if !has { diff --git a/models/org_team.go b/models/org_team.go index 03a4f6e98d8fa..c23afd2c027e6 100644 --- a/models/org_team.go +++ b/models/org_team.go @@ -162,7 +162,9 @@ 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.GetEngine(ctx).ID(t.OrgID). + NoAutoCondition(). + Get(new(user_model.User)) if err != nil { return err } @@ -174,6 +176,7 @@ func NewTeam(ctx context.Context, t *organization.Team) (err error) { has, err = db.GetEngine(ctx). Where("org_id=?", t.OrgID). And("lower_name=?", t.LowerName). + NoAutoCondition(). Get(new(organization.Team)) if err != nil { return err @@ -239,6 +242,7 @@ func UpdateTeam(ctx context.Context, t *organization.Team, authChanged, includeA Where("org_id=?", t.OrgID). And("lower_name=?", t.LowerName). And("id!=?", t.ID). + NoAutoCondition(). Get(new(organization.Team)) if err != nil { return err diff --git a/models/organization/team.go b/models/organization/team.go index 72afc673693c5..f8a2a8cbe49d1 100644 --- a/models/organization/team.go +++ b/models/organization/team.go @@ -203,11 +203,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 := &Team{} + has, err := db.GetEngine(ctx).Where("org_id=? AND lower_name=?", orgID, strings.ToLower(name)). + NoAutoCondition(). + Get(t) if err != nil { return nil, err } else if !has { diff --git a/models/perm/access/access.go b/models/perm/access/access.go index 2d1b2daa62011..795460506178f 100644 --- a/models/perm/access/access.go +++ b/models/perm/access/access.go @@ -51,8 +51,10 @@ 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 := &Access{} + if has, err := db.GetEngine(ctx).Where("user_id=? AND repo_id=?", userID, repo.ID). + NoAutoCondition(). + Get(a); !has || err != nil { return mode, err } return a.Mode, nil diff --git a/models/system/setting.go b/models/system/setting.go index 507d23cff6bbf..da8e9488bd027 100644 --- a/models/system/setting.go +++ b/models/system/setting.go @@ -36,8 +36,10 @@ 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 := &Setting{} + if has, err := db.GetEngine(ctx).Where("setting_key=?", keyRevision). + NoAutoCondition(). + Get(revision); err != nil { return 0 } else if !has { err = db.Insert(ctx, &Setting{SettingKey: keyRevision, Version: 1}) diff --git a/models/user/email_address.go b/models/user/email_address.go index f1ed6692cfefc..5244d9c691f4b 100644 --- a/models/user/email_address.go +++ b/models/user/email_address.go @@ -527,8 +527,10 @@ 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 := EmailAddress{} + if has, err := db.GetEngine(ctx).Where("uid=? AND lower_email =?", userID, strings.ToLower(email)). + NoAutoCondition(). + Get(&addr); err != nil { return err } else if !has { return fmt.Errorf("no such email: %d (%s)", userID, email) @@ -550,8 +552,10 @@ func ActivateUserEmail(ctx context.Context, userID int64, email string, activate // 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 := User{} + if has, err := db.GetEngine(ctx).Where("id=? AND email=?", userID, email). + NoAutoCondition(). + Get(&user); err != nil { return err } else if !has { return fmt.Errorf("no user with ID: %d and Email: %s", userID, email) diff --git a/models/webhook/hooktask.go b/models/webhook/hooktask.go index 3ff27b93a777c..c183da705b2ca 100644 --- a/models/webhook/hooktask.go +++ b/models/webhook/hooktask.go @@ -150,11 +150,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 := &HookTask{} + has, err := db.GetEngine(ctx).Where("hook_id=? AND uuid=?", hookID, uuid). + NoAutoCondition(). + Get(ctx, task) if err != nil { return nil, err } else if !has { diff --git a/modules/repository/collaborator.go b/modules/repository/collaborator.go index f5cdc35045353..3d0a30d77a4b8 100644 --- a/modules/repository/collaborator.go +++ b/modules/repository/collaborator.go @@ -15,12 +15,11 @@ import ( 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, - } + collaboration := &repo_model.Collaboration{} - has, err := db.GetByBean(ctx, collaboration) + has, err := db.GetEngine(ctx).Where("repo_id=? AND user_id=?", repo.ID, u.ID). + NoAutoCondition(). + Get(ctx, collaboration) if err != nil { return err } else if has { diff --git a/modules/repository/repo.go b/modules/repository/repo.go index 974449112f30a..a9a2773501090 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 58b4663345ca6..62134b7d6063f 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 2ca82b5633a07..ed03583d4f2e8 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 2a08bcbaceac2..42b98a273948b 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 f4e1da7bb1a35..6a1f2ccd16c96 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 61348143d00bf..ee0b8f6b89527 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 fe070c62d5cea..a87f38be8ac9d 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) From 3407c4429662d37ed196a78c51fde091ec7506ed Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 26 Nov 2023 10:13:46 +0800 Subject: [PATCH 02/15] Use generic to replace use internal functions to simple the usage --- models/asymkey/ssh_key_deploy.go | 29 +++++++++++----------- models/auth/session.go | 9 +++---- models/db/context.go | 37 +++++++++++++++++++++++---- models/db/list.go | 5 ++++ models/git/lfs.go | 14 +++++------ models/git/protected_branch.go | 23 +++++++---------- models/issues/assignees.go | 5 ++-- models/issues/label.go | 40 ++++++++++++------------------ models/issues/pull.go | 10 +++----- models/org_team.go | 26 ++++++++----------- models/organization/team.go | 11 ++++---- models/perm/access/access.go | 7 +++--- models/system/setting.go | 23 +++++++++-------- models/user/email_address.go | 26 +++++++++---------- models/user/external_login_user.go | 14 ++++++----- models/webhook/hooktask.go | 17 ++++++------- modules/repository/collaborator.go | 18 ++++++++------ 17 files changed, 160 insertions(+), 154 deletions(-) diff --git a/models/asymkey/ssh_key_deploy.go b/models/asymkey/ssh_key_deploy.go index 7cc1062e3a4ef..65320a37c8ef8 100644 --- a/models/asymkey/ssh_key_deploy.go +++ b/models/asymkey/ssh_key_deploy.go @@ -131,15 +131,14 @@ func AddDeployKey(ctx context.Context, repoID int64, name, content string, readO } defer committer.Close() - pkey := &PublicKey{} - has, err := db.GetEngine(ctx).Where("fingerprint=?", fingerprint). - NoAutoCondition(). - Get(ctx, pkey) + pkey, err := db.Get[PublicKey](ctx, builder.Eq{"fingerprint": fingerprint}) if err != nil { - return nil, err + if !db.IsErrNotExist(err) { + return nil, err + } } - if has { + if err == nil { if pkey.Type != KeyTypeDeploy { return nil, ErrKeyAlreadyExist{0, fingerprint, ""} } @@ -164,26 +163,26 @@ 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) { - var key DeployKey - has, err := db.GetEngine(ctx).Where("key_id=? AND repo_id=?", keyID, repoID).Get(&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 + return key, nil } // IsDeployKeyExistByKeyID return true if there is at least one deploykey with the key id diff --git a/models/auth/session.go b/models/auth/session.go index ed5e7c40dde3e..f98fab953a2ef 100644 --- a/models/auth/session.go +++ b/models/auth/session.go @@ -9,6 +9,7 @@ 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 @@ -77,17 +78,13 @@ func RegenerateSession(ctx context.Context, oldKey, newKey string) (*Session, er } defer committer.Close() - if has, err := db.GetEngine(ctx).Where("key=?", newKey). - NoAutoCondition(). - Exist(new(Session)); 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.GetEngine(ctx).Where("key=?", oldKey). - NoAutoCondition(). - Exist(new(Session)); 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{ diff --git a/models/db/context.go b/models/db/context.go index c70474816dc25..3287f7b0549d8 100644 --- a/models/db/context.go +++ b/models/db/context.go @@ -173,6 +173,38 @@ func Exec(ctx context.Context, sqlAndArgs ...any) (sql.Result, error) { return GetEngine(ctx).Exec(sqlAndArgs...) } +func Get[T any](ctx context.Context, cond builder.Cond) (*T, error) { + var bean T + has, err := GetEngine(ctx).Where(cond).NoAutoCondition().Get(&bean) + 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. func DeleteByBean(ctx context.Context, bean any) (int64, error) { return GetEngine(ctx).Delete(bean) @@ -259,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) -} diff --git a/models/db/list.go b/models/db/list.go index b2f932e89bf8e..8a5174b52f886 100644 --- a/models/db/list.go +++ b/models/db/list.go @@ -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) { + var bean T + return GetEngine(ctx).Where(opts.ToConds()).Exist(&bean) +} diff --git a/models/git/lfs.go b/models/git/lfs.go index cbd608c9b212e..f0f6e6fc39ae4 100644 --- a/models/git/lfs.go +++ b/models/git/lfs.go @@ -144,24 +144,22 @@ func NewLFSMetaObject(ctx context.Context, repoID int64, p lfs.Pointer) (*LFSMet } defer committer.Close() - m := LFSMetaObject{Pointer: p, RepositoryID: repoID} - has, err := db.GetEngine(ctx).Where("repo_id=? AND oid=?", repoID, p.Oid). - NoAutoCondition(). - Get(&m) - if err != nil { + m, err := db.Get[LFSMetaObject](ctx, builder.Eq{"repo_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() + return m, committer.Commit() } + m = &LFSMetaObject{Pointer: p, RepositoryID: repoID} if err = db.Insert(ctx, m); err != nil { return nil, err } - return &m, committer.Commit() + return m, committer.Commit() } // GetLFSMetaObjectByOid selects a LFSMetaObject entry from database by its OID. diff --git a/models/git/protected_branch.go b/models/git/protected_branch.go index 68b8b12247859..a3e2119dcc1e4 100644 --- a/models/git/protected_branch.go +++ b/models/git/protected_branch.go @@ -21,6 +21,7 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" + "xorm.io/builder" "github.com/gobwas/glob" "github.com/gobwas/glob/syntax" @@ -274,31 +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{} - has, err := db.GetEngine(ctx).Where("repo_id=? AND rule_name=?", repoID, ruleName). - NoAutoCondition(). - Get(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{} - has, err := db.GetEngine(ctx).Where("repo_id=? AND id=?", repoID, ruleID). - NoAutoCondition(). - Get(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 } diff --git a/models/issues/assignees.go b/models/issues/assignees.go index 50d8bd0ce1836..187e6259d7044 100644 --- a/models/issues/assignees.go +++ b/models/issues/assignees.go @@ -10,6 +10,7 @@ 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,9 +60,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.GetEngine(ctx).Where("assignee_id=? AND issue_id=?", user.ID, issue.ID). - NoAutoCondition(). - Exist(new(IssueAssignees)) + 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 d4f29913fa3b3..1e2463812bf3f 100644 --- a/models/issues/label.go +++ b/models/issues/label.go @@ -304,14 +304,12 @@ func GetLabelInRepoByName(ctx context.Context, repoID int64, labelName string) ( return nil, ErrRepoLabelNotExist{0, repoID} } - l := &Label{} - has, err := db.GetEngine(ctx).Where("name=? AND repo_id=?", labelName, repoID). - NoAutoCondition(). - Get(l) + l, err := db.Get[Label](ctx, builder.Eq{"name": labelName, "repo_id": repoID}) if err != nil { + if db.IsErrNotExist(err) { + return nil, ErrRepoLabelNotExist{0, repoID} + } return nil, err - } else if !has { - return nil, ErrRepoLabelNotExist{0, l.RepoID} } return l, nil } @@ -322,14 +320,12 @@ func GetLabelInRepoByID(ctx context.Context, repoID, labelID int64) (*Label, err return nil, ErrRepoLabelNotExist{labelID, repoID} } - l := &Label{} - has, err := db.GetEngine(ctx).Where("id=? AND repo_id=?", labelID, repoID). - NoAutoCondition(). - Get(l) + l, err := db.Get[Label](ctx, builder.Eq{"id": labelID, "repo_id": repoID}) if err != nil { + if db.IsErrNotExist(err) { + return nil, ErrRepoLabelNotExist{l.ID, l.RepoID} + } return nil, err - } else if !has { - return nil, ErrRepoLabelNotExist{l.ID, l.RepoID} } return l, nil } @@ -406,14 +402,12 @@ func GetLabelInOrgByName(ctx context.Context, orgID int64, labelName string) (*L return nil, ErrOrgLabelNotExist{0, orgID} } - l := &Label{} - has, err := db.GetEngine(ctx).Where("name = ? AND org_id=?", labelName, orgID). - NoAutoCondition(). - Get(l) + l, err := db.Get[Label](ctx, builder.Eq{"name": labelName, "org_id": orgID}) if err != nil { + if db.IsErrNotExist(err) { + return nil, ErrOrgLabelNotExist{0, orgID} + } return nil, err - } else if !has { - return nil, ErrOrgLabelNotExist{0, l.OrgID} } return l, nil } @@ -424,14 +418,12 @@ func GetLabelInOrgByID(ctx context.Context, orgID, labelID int64) (*Label, error return nil, ErrOrgLabelNotExist{labelID, orgID} } - l := &Label{} - has, err := db.GetEngine(ctx).Where("id = ? AND org_id=?", labelID, orgID). - NoAutoCondition(). - Get(l) + l, err := db.Get[Label](ctx, builder.Eq{"id": labelID, "org_id": orgID}) if err != nil { + if db.IsErrNotExist(err) { + return nil, ErrOrgLabelNotExist{l.ID, l.OrgID} + } return nil, err - } else if !has { - return nil, ErrOrgLabelNotExist{l.ID, l.OrgID} } return l, nil } diff --git a/models/issues/pull.go b/models/issues/pull.go index 051d7d336b9f0..24466d4b9a727 100644 --- a/models/issues/pull.go +++ b/models/issues/pull.go @@ -660,14 +660,12 @@ 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{} - has, err := db.GetEngine(ctx).Where("issue_id = ?", issueID). - NoAutoCondition(). - Get(pr) + pr, err := db.Get[PullRequest](ctx, builder.Eq{"issue_id": issueID}) if err != nil { + if db.IsErrNotExist(err) { + return nil, ErrPullRequestNotExist{0, issueID, 0, 0, "", ""} + } return nil, err - } else if !has { - 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 c23afd2c027e6..1a452436c3e32 100644 --- a/models/org_team.go +++ b/models/org_team.go @@ -162,9 +162,7 @@ func NewTeam(ctx context.Context, t *organization.Team) (err error) { return err } - has, err := db.GetEngine(ctx).ID(t.OrgID). - NoAutoCondition(). - Get(new(user_model.User)) + has, err := db.ExistByID[user_model.User](ctx, t.OrgID) if err != nil { return err } @@ -173,11 +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). - NoAutoCondition(). - 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 } @@ -235,21 +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). - NoAutoCondition(). - 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 f8a2a8cbe49d1..defa5f66c0faa 100644 --- a/models/organization/team.go +++ b/models/organization/team.go @@ -16,6 +16,7 @@ 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 +204,12 @@ 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{} - has, err := db.GetEngine(ctx).Where("org_id=? AND lower_name=?", orgID, strings.ToLower(name)). - NoAutoCondition(). - Get(t) + t, err := db.Get[Team](ctx, builder.Eq{"org_id": orgID, "lower_name": strings.ToLower(name)}) if err != nil { + if db.IsErrNotExist(err) { + return nil, ErrTeamNotExist{orgID, 0, name} + } return nil, err - } else if !has { - return nil, ErrTeamNotExist{orgID, 0, name} } return t, nil } diff --git a/models/perm/access/access.go b/models/perm/access/access.go index 795460506178f..1f8b9b7e5383e 100644 --- a/models/perm/access/access.go +++ b/models/perm/access/access.go @@ -13,6 +13,7 @@ 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,10 +52,8 @@ func accessLevel(ctx context.Context, user *user_model.User, repo *repo_model.Re return perm.AccessModeOwner, nil } - a := &Access{} - if has, err := db.GetEngine(ctx).Where("user_id=? AND repo_id=?", userID, repo.ID). - NoAutoCondition(). - Get(a); !has || err != nil { + a, err := db.Get[Access](ctx, builder.Eq{"user_id": userID, "repo_id": repo.ID}) + if err != nil { return mode, err } return a.Mode, nil diff --git a/models/system/setting.go b/models/system/setting.go index da8e9488bd027..874ad89374f85 100644 --- a/models/system/setting.go +++ b/models/system/setting.go @@ -13,6 +13,7 @@ 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,18 +37,18 @@ func init() { const keyRevision = "revision" func GetRevision(ctx context.Context) int { - revision := &Setting{} - if has, err := db.GetEngine(ctx).Where("setting_key=?", keyRevision). - NoAutoCondition(). - Get(revision); err != nil { - return 0 - } else if !has { - err = db.Insert(ctx, &Setting{SettingKey: keyRevision, Version: 1}) - if err != nil { - return 0 + revision, err := db.Get[Setting](ctx, builder.Eq{"setting_key": keyRevision}) + if err != nil { + if db.IsErrNotExist(err) { + err = db.Insert(ctx, &Setting{SettingKey: keyRevision, Version: 1}) + if err != nil { + return 0 + } + return 1 } - return 1 - } else if revision.Version <= 0 || revision.Version >= math.MaxInt-1 { + return 0 + } + 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/user/email_address.go b/models/user/email_address.go index 5244d9c691f4b..8a3d7b1544b9e 100644 --- a/models/user/email_address.go +++ b/models/user/email_address.go @@ -527,13 +527,12 @@ 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{} - if has, err := db.GetEngine(ctx).Where("uid=? AND lower_email =?", userID, strings.ToLower(email)). - NoAutoCondition(). - Get(&addr); err != nil { + addr, err := db.Get[EmailAddress](ctx, builder.Eq{"uid": userID, "lower_email": strings.ToLower(email)}) + if err != nil { + if db.IsErrNotExist(err) { + return fmt.Errorf("no such email: %d (%s)", userID, email) + } return err - } else if !has { - return fmt.Errorf("no such email: %d (%s)", userID, email) } if addr.IsActivated == activate { // Already in the desired state; no action @@ -546,19 +545,18 @@ 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{} - if has, err := db.GetEngine(ctx).Where("id=? AND email=?", userID, email). - NoAutoCondition(). - Get(&user); err != nil { + user, err := db.Get[User](ctx, builder.Eq{"id": userID, "email": email}) + if err != nil { + if db.IsErrNotExist(err) { + return fmt.Errorf("no user with ID: %d and Email: %s", userID, email) + } return err - } else if !has { - 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 { @@ -566,7 +564,7 @@ func ActivateUserEmail(ctx context.Context, userID int64, email string, 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 0db702f22520f..965b7a5ed1dfd 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 c183da705b2ca..846fe1e0cfb55 100644 --- a/models/webhook/hooktask.go +++ b/models/webhook/hooktask.go @@ -14,6 +14,7 @@ import ( api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" webhook_module "code.gitea.io/gitea/modules/webhook" + "xorm.io/builder" gouuid "github.com/google/uuid" ) @@ -150,17 +151,15 @@ 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{} - has, err := db.GetEngine(ctx).Where("hook_id=? AND uuid=?", hookID, uuid). - NoAutoCondition(). - Get(ctx, task) + task, err := db.Get[HookTask](ctx, builder.Eq{"hook_id": hookID, "uuid": uuid}) if err != nil { - return nil, err - } else if !has { - return nil, ErrHookTaskNotExist{ - HookID: hookID, - UUID: uuid, + if db.IsErrNotExist(err) { + return nil, ErrHookTaskNotExist{ + HookID: hookID, + UUID: uuid, + } } + return nil, err } return CreateHookTask(ctx, &HookTask{ diff --git a/modules/repository/collaborator.go b/modules/repository/collaborator.go index 3d0a30d77a4b8..201986d2d89be 100644 --- a/modules/repository/collaborator.go +++ b/modules/repository/collaborator.go @@ -11,22 +11,24 @@ 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{} - - has, err := db.GetEngine(ctx).Where("repo_id=? AND user_id=?", repo.ID, u.ID). - NoAutoCondition(). - Get(ctx, collaboration) + collaboration, err := db.Get[repo_model.Collaboration](ctx, builder.Eq{ + "repo_id": repo.ID, + "user_id": u.ID, + }) if err != nil { + if db.IsErrNotExist(err) { + return nil + } return err - } else if has { - return nil } - collaboration.Mode = perm.AccessModeWrite + collaboration.Mode = perm.AccessModeWrite if err = db.Insert(ctx, collaboration); err != nil { return err } From f8b81fbd690502f5b607b3b44c31f87416b09753 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 26 Nov 2023 10:42:13 +0800 Subject: [PATCH 03/15] More changes --- models/asymkey/ssh_key_deploy.go | 6 ++---- models/asymkey/ssh_key_fingerprint.go | 3 ++- models/auth/session.go | 31 +++++++++++++-------------- models/db/iterate_test.go | 6 +++--- models/git/protected_branch.go | 2 +- models/issues/assignees.go | 1 + models/organization/team.go | 1 + models/perm/access/access.go | 1 + models/system/setting.go | 1 + models/webhook/hooktask.go | 2 +- modules/repository/collaborator.go | 8 +++---- 11 files changed, 32 insertions(+), 30 deletions(-) diff --git a/models/asymkey/ssh_key_deploy.go b/models/asymkey/ssh_key_deploy.go index 65320a37c8ef8..b942aea631141 100644 --- a/models/asymkey/ssh_key_deploy.go +++ b/models/asymkey/ssh_key_deploy.go @@ -132,10 +132,8 @@ func AddDeployKey(ctx context.Context, repoID int64, name, content string, readO defer committer.Close() pkey, err := db.Get[PublicKey](ctx, builder.Eq{"fingerprint": fingerprint}) - if err != nil { - if !db.IsErrNotExist(err) { - return nil, err - } + if err != nil && !db.IsErrNotExist(err) { + return nil, err } if err == nil { diff --git a/models/asymkey/ssh_key_fingerprint.go b/models/asymkey/ssh_key_fingerprint.go index d7564376b13f1..b9cfb1b2513c8 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,7 +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.GetEngine(ctx).Where("fingerprint=?", fingerprint).Exist(&PublicKey{}) + 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 f98fab953a2ef..97b2db3bf4253 100644 --- a/models/auth/session.go +++ b/models/auth/session.go @@ -9,6 +9,7 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/timeutil" + "xorm.io/builder" ) @@ -40,26 +41,23 @@ func ReadSession(ctx context.Context, key string) (*Session, error) { } defer committer.Close() - var session Session - - if has, err := db.GetEngine(ctx).Where("key=?", key).Get(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 @@ -99,12 +97,13 @@ func RegenerateSession(ctx context.Context, oldKey, newKey string) (*Session, er return nil, err } - var s Session - if _, err := db.GetEngine(ctx).Where("key=?", newKey).Get(&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/db/iterate_test.go b/models/db/iterate_test.go index f9010f991d015..0f6ba2cc94a3f 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{} - has, err := db.GetEngine(ctx).ID(repoUnit.ID).NoAutoCondition().Get(&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/protected_branch.go b/models/git/protected_branch.go index a3e2119dcc1e4..69407248659d6 100644 --- a/models/git/protected_branch.go +++ b/models/git/protected_branch.go @@ -21,10 +21,10 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" - "xorm.io/builder" "github.com/gobwas/glob" "github.com/gobwas/glob/syntax" + "xorm.io/builder" ) var ErrBranchIsProtected = errors.New("branch is protected") diff --git a/models/issues/assignees.go b/models/issues/assignees.go index 187e6259d7044..60f32d9557874 100644 --- a/models/issues/assignees.go +++ b/models/issues/assignees.go @@ -10,6 +10,7 @@ import ( "code.gitea.io/gitea/models/db" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/util" + "xorm.io/builder" ) diff --git a/models/organization/team.go b/models/organization/team.go index defa5f66c0faa..6a10c1294cc44 100644 --- a/models/organization/team.go +++ b/models/organization/team.go @@ -16,6 +16,7 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/util" + "xorm.io/builder" ) diff --git a/models/perm/access/access.go b/models/perm/access/access.go index 1f8b9b7e5383e..5f8ffde15bc52 100644 --- a/models/perm/access/access.go +++ b/models/perm/access/access.go @@ -13,6 +13,7 @@ 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" ) diff --git a/models/system/setting.go b/models/system/setting.go index 874ad89374f85..23968460445f2 100644 --- a/models/system/setting.go +++ b/models/system/setting.go @@ -13,6 +13,7 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting/config" "code.gitea.io/gitea/modules/timeutil" + "xorm.io/builder" ) diff --git a/models/webhook/hooktask.go b/models/webhook/hooktask.go index 846fe1e0cfb55..ccafb31415ea2 100644 --- a/models/webhook/hooktask.go +++ b/models/webhook/hooktask.go @@ -14,9 +14,9 @@ import ( api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" webhook_module "code.gitea.io/gitea/modules/webhook" - "xorm.io/builder" gouuid "github.com/google/uuid" + "xorm.io/builder" ) // ___ ___ __ ___________ __ diff --git a/modules/repository/collaborator.go b/modules/repository/collaborator.go index 201986d2d89be..ed81d2f650702 100644 --- a/modules/repository/collaborator.go +++ b/modules/repository/collaborator.go @@ -21,10 +21,10 @@ func AddCollaborator(ctx context.Context, repo *repo_model.Repository, u *user_m "repo_id": repo.ID, "user_id": u.ID, }) - if err != nil { - if db.IsErrNotExist(err) { - return nil - } + if err == nil { + return nil + } + if !db.IsErrNotExist(err) { return err } From 6f413998fac3c3efaefae3651a8ad394743b0162 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 26 Nov 2023 12:10:31 +0800 Subject: [PATCH 04/15] Fix logic --- models/perm/access/access.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/models/perm/access/access.go b/models/perm/access/access.go index 5f8ffde15bc52..e320936c68318 100644 --- a/models/perm/access/access.go +++ b/models/perm/access/access.go @@ -55,6 +55,9 @@ func accessLevel(ctx context.Context, user *user_model.User, repo *repo_model.Re a, err := db.Get[Access](ctx, builder.Eq{"user_id": userID, "repo_id": repo.ID}) if err != nil { + if db.IsErrNotExist(err) { + return mode, nil + } return mode, err } return a.Mode, nil From d16e1d8258aa13866932fb5fa489fd084c89a71a Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 26 Nov 2023 15:00:41 +0800 Subject: [PATCH 05/15] Fix bug --- modules/repository/collaborator.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/modules/repository/collaborator.go b/modules/repository/collaborator.go index ed81d2f650702..ebe14e3a4c2e5 100644 --- a/modules/repository/collaborator.go +++ b/modules/repository/collaborator.go @@ -17,19 +17,21 @@ import ( func AddCollaborator(ctx context.Context, repo *repo_model.Repository, u *user_model.User) error { return db.WithTx(ctx, func(ctx context.Context) error { - collaboration, err := db.Get[repo_model.Collaboration](ctx, builder.Eq{ + has, err := db.Exist[repo_model.Collaboration](ctx, builder.Eq{ "repo_id": repo.ID, "user_id": u.ID, }) - if err == nil { - return nil - } - if !db.IsErrNotExist(err) { + 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 } From 5d4deb3a71339defb5952a6e670658131911a444 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 26 Nov 2023 17:34:48 +0800 Subject: [PATCH 06/15] Fix bug --- models/git/lfs.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/git/lfs.go b/models/git/lfs.go index f0f6e6fc39ae4..ed42c0be7bca7 100644 --- a/models/git/lfs.go +++ b/models/git/lfs.go @@ -144,7 +144,7 @@ func NewLFSMetaObject(ctx context.Context, repoID int64, p lfs.Pointer) (*LFSMet } defer committer.Close() - m, err := db.Get[LFSMetaObject](ctx, builder.Eq{"repo_id": repoID, "oid": p.Oid}) + m, err := db.Get[LFSMetaObject](ctx, builder.Eq{"repository_id": repoID, "oid": p.Oid}) if err != nil && !db.IsErrNotExist(err) { return nil, err } From 4949cdc73b241de699afc424f9773a7a2472a2da Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 26 Nov 2023 19:01:04 +0800 Subject: [PATCH 07/15] prevent Get and Exist with no condition --- models/db/context.go | 8 ++++++++ models/db/error.go | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/models/db/context.go b/models/db/context.go index 3287f7b0549d8..b2a6102ca0960 100644 --- a/models/db/context.go +++ b/models/db/context.go @@ -174,6 +174,10 @@ func Exec(ctx context.Context, sqlAndArgs ...any) (sql.Result, error) { } func Get[T any](ctx context.Context, cond builder.Cond) (*T, error) { + if !cond.IsValid() { + return nil, ErrConditionRequired{} + } + var bean T has, err := GetEngine(ctx).Where(cond).NoAutoCondition().Get(&bean) if err != nil { @@ -196,6 +200,10 @@ func GetByID[T any](ctx context.Context, id int64) (*T, error) { } 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) } diff --git a/models/db/error.go b/models/db/error.go index 665e970e17c1e..c13a488468708 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 fmt.Sprintf("condition is required") +} + +// Unwrap unwraps this as a ErrNotExist err +func (err ErrConditionRequired) Unwrap() error { + return util.ErrInvalidArgument +} From 05a0f47a0df064862f17dcf70434dc790f83ccb7 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 26 Nov 2023 20:24:28 +0800 Subject: [PATCH 08/15] Fix lint error --- models/db/error.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/db/error.go b/models/db/error.go index c13a488468708..f601a15c01ef0 100644 --- a/models/db/error.go +++ b/models/db/error.go @@ -83,7 +83,7 @@ func IsErrConditionRequired(err error) bool { } func (err ErrConditionRequired) Error() string { - return fmt.Sprintf("condition is required") + return "condition is required" } // Unwrap unwraps this as a ErrNotExist err From 4c05b59250989c5d542d3e1822126d29cabe7563 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 26 Nov 2023 22:25:13 +0800 Subject: [PATCH 09/15] Fix column name --- models/git/protected_branch.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/models/git/protected_branch.go b/models/git/protected_branch.go index 69407248659d6..8ab9e86584ca1 100644 --- a/models/git/protected_branch.go +++ b/models/git/protected_branch.go @@ -275,7 +275,8 @@ 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, err := db.Get[ProtectedBranch](ctx, builder.Eq{"repo_id": repoID, "rule_name": ruleName}) + // branch_name is legacy name, it actually is rule name + rel, err := db.Get[ProtectedBranch](ctx, builder.Eq{"repo_id": repoID, "branch_name": ruleName}) if err != nil { if db.IsErrNotExist(err) { return nil, nil From 912df7f2f561257add6138df6ffb457b5402d907 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 26 Nov 2023 23:25:06 +0800 Subject: [PATCH 10/15] follow @delvh suggestion --- models/asymkey/ssh_key_deploy.go | 33 ++++++++++++++++---------------- models/auth/session.go | 15 +++++++-------- models/db/context.go | 18 ++++++++--------- models/git/lfs.go | 8 +++----- models/git/protected_branch.go | 14 ++++++-------- models/issues/label.go | 28 ++++++++++++--------------- models/issues/pull.go | 7 +++---- models/organization/team.go | 7 +++---- models/perm/access/access.go | 7 +++---- models/system/setting.go | 15 +++++++-------- models/user/email_address.go | 16 ++++++++-------- models/webhook/hooktask.go | 13 ++++++------- 12 files changed, 83 insertions(+), 98 deletions(-) diff --git a/models/asymkey/ssh_key_deploy.go b/models/asymkey/ssh_key_deploy.go index b942aea631141..f853c7e9035c3 100644 --- a/models/asymkey/ssh_key_deploy.go +++ b/models/asymkey/ssh_key_deploy.go @@ -131,21 +131,22 @@ func AddDeployKey(ctx context.Context, repoID int64, name, content string, readO } defer committer.Close() - pkey, err := db.Get[PublicKey](ctx, builder.Eq{"fingerprint": fingerprint}) - if err != nil && !db.IsErrNotExist(err) { + pkey, exist, err := db.Get[PublicKey](ctx, builder.Eq{"fingerprint": fingerprint}) + if err != nil { return nil, err - } - - if err == nil { + } 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) } @@ -161,24 +162,22 @@ 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, err := db.GetByID[DeployKey](ctx, id) + key, exist, err := db.GetByID[DeployKey](ctx, id) if err != nil { - if db.IsErrNotExist(err) { - return nil, ErrDeployKeyNotExist{0, 0, id} - } return nil, err + } else if !exist { + return nil, ErrDeployKeyNotExist{0, 0, id} } 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, err := db.Get[DeployKey](ctx, builder.Eq{"key_id": keyID, "repo_id": repoID}) + key, exist, 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 !exist { + return nil, ErrDeployKeyNotExist{0, keyID, repoID} } return key, nil } diff --git a/models/auth/session.go b/models/auth/session.go index 97b2db3bf4253..60fdeaba7c203 100644 --- a/models/auth/session.go +++ b/models/auth/session.go @@ -41,15 +41,14 @@ func ReadSession(ctx context.Context, key string) (*Session, error) { } defer committer.Close() - session, err := db.Get[Session](ctx, builder.Eq{"key": key}) + session, exist, 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 + } else if !exist { + session.Expiry = timeutil.TimeStampNow() + if err := db.Insert(ctx, &session); err != nil { + return nil, err + } } return session, committer.Commit() @@ -97,7 +96,7 @@ func RegenerateSession(ctx context.Context, oldKey, newKey string) (*Session, er return nil, err } - s, err := db.Get[Session](ctx, builder.Eq{"key": newKey}) + s, _, err := db.Get[Session](ctx, builder.Eq{"key": newKey}) if err != nil { // is not exist, it should be impossible return nil, err diff --git a/models/db/context.go b/models/db/context.go index b2a6102ca0960..a409094dabcda 100644 --- a/models/db/context.go +++ b/models/db/context.go @@ -173,30 +173,30 @@ func Exec(ctx context.Context, sqlAndArgs ...any) (sql.Result, error) { return GetEngine(ctx).Exec(sqlAndArgs...) } -func Get[T any](ctx context.Context, cond builder.Cond) (*T, error) { +func Get[T any](ctx context.Context, cond builder.Cond) (*T, bool, error) { if !cond.IsValid() { - return nil, ErrConditionRequired{} + return nil, false, ErrConditionRequired{} } var bean T has, err := GetEngine(ctx).Where(cond).NoAutoCondition().Get(&bean) if err != nil { - return nil, err + return nil, false, err } else if !has { - return nil, ErrNotExist{Resource: TableName(bean)} + return nil, false, nil } - return &bean, nil + return &bean, true, nil } -func GetByID[T any](ctx context.Context, id int64) (*T, error) { +func GetByID[T any](ctx context.Context, id int64) (*T, bool, error) { var bean T has, err := GetEngine(ctx).ID(id).NoAutoCondition().Get(&bean) if err != nil { - return nil, err + return nil, false, err } else if !has { - return nil, ErrNotExist{Resource: TableName(bean), ID: id} + return nil, false, nil } - return &bean, nil + return &bean, true, nil } func Exist[T any](ctx context.Context, cond builder.Cond) (bool, error) { diff --git a/models/git/lfs.go b/models/git/lfs.go index ed42c0be7bca7..837dc9fd312f5 100644 --- a/models/git/lfs.go +++ b/models/git/lfs.go @@ -144,12 +144,10 @@ func NewLFSMetaObject(ctx context.Context, repoID int64, p lfs.Pointer) (*LFSMet } defer committer.Close() - m, err := db.Get[LFSMetaObject](ctx, builder.Eq{"repository_id": repoID, "oid": p.Oid}) - if err != nil && !db.IsErrNotExist(err) { + m, exist, err := db.Get[LFSMetaObject](ctx, builder.Eq{"repository_id": repoID, "oid": p.Oid}) + if err != nil { return nil, err - } - - if err == nil { + } else if exist { m.Existing = true return m, committer.Commit() } diff --git a/models/git/protected_branch.go b/models/git/protected_branch.go index 8ab9e86584ca1..528acc175a27a 100644 --- a/models/git/protected_branch.go +++ b/models/git/protected_branch.go @@ -276,24 +276,22 @@ 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) { // branch_name is legacy name, it actually is rule name - rel, err := db.Get[ProtectedBranch](ctx, builder.Eq{"repo_id": repoID, "branch_name": ruleName}) + rel, exist, err := db.Get[ProtectedBranch](ctx, builder.Eq{"repo_id": repoID, "branch_name": ruleName}) if err != nil { - if db.IsErrNotExist(err) { - return nil, nil - } return nil, err + } else if !exist { + 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, err := db.Get[ProtectedBranch](ctx, builder.Eq{"repo_id": repoID, "id": ruleID}) + rel, exist, 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 + } else if !exist { + return nil, nil } return rel, nil } diff --git a/models/issues/label.go b/models/issues/label.go index 1e2463812bf3f..e6a25d2d0457a 100644 --- a/models/issues/label.go +++ b/models/issues/label.go @@ -304,12 +304,11 @@ func GetLabelInRepoByName(ctx context.Context, repoID int64, labelName string) ( return nil, ErrRepoLabelNotExist{0, repoID} } - l, err := db.Get[Label](ctx, builder.Eq{"name": labelName, "repo_id": repoID}) + l, exist, err := db.Get[Label](ctx, builder.Eq{"name": labelName, "repo_id": repoID}) if err != nil { - if db.IsErrNotExist(err) { - return nil, ErrRepoLabelNotExist{0, repoID} - } return nil, err + } else if !exist { + return nil, ErrRepoLabelNotExist{0, repoID} } return l, nil } @@ -320,12 +319,11 @@ func GetLabelInRepoByID(ctx context.Context, repoID, labelID int64) (*Label, err return nil, ErrRepoLabelNotExist{labelID, repoID} } - l, err := db.Get[Label](ctx, builder.Eq{"id": labelID, "repo_id": repoID}) + l, exist, err := db.Get[Label](ctx, builder.Eq{"id": labelID, "repo_id": repoID}) if err != nil { - if db.IsErrNotExist(err) { - return nil, ErrRepoLabelNotExist{l.ID, l.RepoID} - } return nil, err + } else if !exist { + return nil, ErrRepoLabelNotExist{l.ID, l.RepoID} } return l, nil } @@ -402,12 +400,11 @@ func GetLabelInOrgByName(ctx context.Context, orgID int64, labelName string) (*L return nil, ErrOrgLabelNotExist{0, orgID} } - l, err := db.Get[Label](ctx, builder.Eq{"name": labelName, "org_id": orgID}) + l, exist, err := db.Get[Label](ctx, builder.Eq{"name": labelName, "org_id": orgID}) if err != nil { - if db.IsErrNotExist(err) { - return nil, ErrOrgLabelNotExist{0, orgID} - } return nil, err + } else if !exist { + return nil, ErrOrgLabelNotExist{0, orgID} } return l, nil } @@ -418,12 +415,11 @@ func GetLabelInOrgByID(ctx context.Context, orgID, labelID int64) (*Label, error return nil, ErrOrgLabelNotExist{labelID, orgID} } - l, err := db.Get[Label](ctx, builder.Eq{"id": labelID, "org_id": orgID}) + l, exist, err := db.Get[Label](ctx, builder.Eq{"id": labelID, "org_id": orgID}) if err != nil { - if db.IsErrNotExist(err) { - return nil, ErrOrgLabelNotExist{l.ID, l.OrgID} - } return nil, err + } else if !exist { + return nil, ErrOrgLabelNotExist{l.ID, l.OrgID} } return l, nil } diff --git a/models/issues/pull.go b/models/issues/pull.go index 24466d4b9a727..c51a7daf4eca1 100644 --- a/models/issues/pull.go +++ b/models/issues/pull.go @@ -660,12 +660,11 @@ 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, err := db.Get[PullRequest](ctx, builder.Eq{"issue_id": issueID}) + pr, exist, err := db.Get[PullRequest](ctx, builder.Eq{"issue_id": issueID}) if err != nil { - if db.IsErrNotExist(err) { - return nil, ErrPullRequestNotExist{0, issueID, 0, 0, "", ""} - } return nil, err + } else if !exist { + return nil, ErrPullRequestNotExist{0, issueID, 0, 0, "", ""} } return pr, pr.LoadAttributes(ctx) } diff --git a/models/organization/team.go b/models/organization/team.go index 6a10c1294cc44..17db11c42d1a1 100644 --- a/models/organization/team.go +++ b/models/organization/team.go @@ -205,12 +205,11 @@ 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, err := db.Get[Team](ctx, builder.Eq{"org_id": orgID, "lower_name": strings.ToLower(name)}) + t, exist, err := db.Get[Team](ctx, builder.Eq{"org_id": orgID, "lower_name": strings.ToLower(name)}) if err != nil { - if db.IsErrNotExist(err) { - return nil, ErrTeamNotExist{orgID, 0, name} - } return nil, err + } 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 e320936c68318..3e2568b4b429c 100644 --- a/models/perm/access/access.go +++ b/models/perm/access/access.go @@ -53,12 +53,11 @@ func accessLevel(ctx context.Context, user *user_model.User, repo *repo_model.Re return perm.AccessModeOwner, nil } - a, err := db.Get[Access](ctx, builder.Eq{"user_id": userID, "repo_id": repo.ID}) + a, exist, err := db.Get[Access](ctx, builder.Eq{"user_id": userID, "repo_id": repo.ID}) if err != nil { - if db.IsErrNotExist(err) { - return mode, 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 23968460445f2..343fb6711551f 100644 --- a/models/system/setting.go +++ b/models/system/setting.go @@ -38,16 +38,15 @@ func init() { const keyRevision = "revision" func GetRevision(ctx context.Context) int { - revision, err := db.Get[Setting](ctx, builder.Eq{"setting_key": keyRevision}) + revision, exist, err := db.Get[Setting](ctx, builder.Eq{"setting_key": keyRevision}) if err != nil { - if db.IsErrNotExist(err) { - err = db.Insert(ctx, &Setting{SettingKey: keyRevision, Version: 1}) - if err != nil { - return 0 - } - return 1 - } return 0 + } else if !exist { + err = db.Insert(ctx, &Setting{SettingKey: keyRevision, Version: 1}) + if err != nil { + return 0 + } + return 1 } if revision.Version <= 0 || revision.Version >= math.MaxInt-1 { _, err = db.Exec(ctx, "UPDATE system_setting SET version=1 WHERE setting_key=?", keyRevision) diff --git a/models/user/email_address.go b/models/user/email_address.go index 8a3d7b1544b9e..2af2621f5feab 100644 --- a/models/user/email_address.go +++ b/models/user/email_address.go @@ -527,13 +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, err := db.Get[EmailAddress](ctx, builder.Eq{"uid": userID, "lower_email": strings.ToLower(email)}) + addr, exist, err := db.Get[EmailAddress](ctx, builder.Eq{"uid": userID, "lower_email": strings.ToLower(email)}) if err != nil { - if db.IsErrNotExist(err) { - return fmt.Errorf("no such email: %d (%s)", userID, email) - } return err + } 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 @@ -551,13 +551,13 @@ func ActivateUserEmail(ctx context.Context, userID int64, email string, activate // Activate/deactivate a user's primary email address and account if addr.IsPrimary { - user, err := db.Get[User](ctx, builder.Eq{"id": userID, "email": email}) + user, exist, err := db.Get[User](ctx, builder.Eq{"id": userID, "email": email}) if err != nil { - if db.IsErrNotExist(err) { - return fmt.Errorf("no user with ID: %d and Email: %s", userID, email) - } return err + } 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 diff --git a/models/webhook/hooktask.go b/models/webhook/hooktask.go index ccafb31415ea2..2fb655ebca102 100644 --- a/models/webhook/hooktask.go +++ b/models/webhook/hooktask.go @@ -151,15 +151,14 @@ 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, err := db.Get[HookTask](ctx, builder.Eq{"hook_id": hookID, "uuid": uuid}) + task, exist, err := db.Get[HookTask](ctx, builder.Eq{"hook_id": hookID, "uuid": uuid}) if err != nil { - if db.IsErrNotExist(err) { - return nil, ErrHookTaskNotExist{ - HookID: hookID, - UUID: uuid, - } - } return nil, err + } else if !exist { + return nil, ErrHookTaskNotExist{ + HookID: hookID, + UUID: uuid, + } } return CreateHookTask(ctx, &HookTask{ From 4aebf61a6518dfb9b35fd6c01c03e1923561764e Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Mon, 27 Nov 2023 12:13:00 +0800 Subject: [PATCH 11/15] fix bug --- models/issues/label.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/issues/label.go b/models/issues/label.go index e6a25d2d0457a..80c45c84a019e 100644 --- a/models/issues/label.go +++ b/models/issues/label.go @@ -323,7 +323,7 @@ func GetLabelInRepoByID(ctx context.Context, repoID, labelID int64) (*Label, err if err != nil { return nil, err } else if !exist { - return nil, ErrRepoLabelNotExist{l.ID, l.RepoID} + return nil, ErrRepoLabelNotExist{labelID, repoID} } return l, nil } From b149898cf1c81db3257d9b891c33d8dcb4cb2da9 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 28 Nov 2023 20:01:36 +0800 Subject: [PATCH 12/15] Fix bug --- models/asymkey/ssh_key_deploy.go | 2 +- models/issues/label.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/models/asymkey/ssh_key_deploy.go b/models/asymkey/ssh_key_deploy.go index f853c7e9035c3..923c5020edce0 100644 --- a/models/asymkey/ssh_key_deploy.go +++ b/models/asymkey/ssh_key_deploy.go @@ -166,7 +166,7 @@ func GetDeployKeyByID(ctx context.Context, id int64) (*DeployKey, error) { if err != nil { return nil, err } else if !exist { - return nil, ErrDeployKeyNotExist{0, 0, id} + return nil, ErrDeployKeyNotExist{id, 0, 0} } return key, nil } diff --git a/models/issues/label.go b/models/issues/label.go index 80c45c84a019e..5c6b8e08d72fc 100644 --- a/models/issues/label.go +++ b/models/issues/label.go @@ -419,7 +419,7 @@ func GetLabelInOrgByID(ctx context.Context, orgID, labelID int64) (*Label, error if err != nil { return nil, err } else if !exist { - return nil, ErrOrgLabelNotExist{l.ID, l.OrgID} + return nil, ErrOrgLabelNotExist{labelID, orgID} } return l, nil } From 4728885a72d942878477f2781a8134b9b60bae23 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 28 Nov 2023 21:46:33 +0800 Subject: [PATCH 13/15] Fix test --- models/system/setting_test.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/models/system/setting_test.go b/models/system/setting_test.go index 0316ea144df51..8f04412fb450b 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) } From 577133abad3ceb97e087d156d416f78b5e7eb9e6 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 3 Dec 2023 15:48:26 -0800 Subject: [PATCH 14/15] Add return parameter names --- models/db/context.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/models/db/context.go b/models/db/context.go index a409094dabcda..7b739f7e9f353 100644 --- a/models/db/context.go +++ b/models/db/context.go @@ -173,7 +173,7 @@ func Exec(ctx context.Context, sqlAndArgs ...any) (sql.Result, error) { return GetEngine(ctx).Exec(sqlAndArgs...) } -func Get[T any](ctx context.Context, cond builder.Cond) (*T, bool, error) { +func Get[T any](ctx context.Context, cond builder.Cond) (object *T, exist bool, err error) { if !cond.IsValid() { return nil, false, ErrConditionRequired{} } @@ -188,7 +188,7 @@ func Get[T any](ctx context.Context, cond builder.Cond) (*T, bool, error) { return &bean, true, nil } -func GetByID[T any](ctx context.Context, id int64) (*T, bool, error) { +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 { From 7a528b4a39064872bf0f218b73826516f312595c Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 6 Dec 2023 21:27:04 -0800 Subject: [PATCH 15/15] Remove unnecessary function --- models/auth/source.go | 4 ++-- models/db/list.go | 5 ----- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/models/auth/source.go b/models/auth/source.go index 5e77afddc369e..8a372c1429b30 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/list.go b/models/db/list.go index 8a5174b52f886..b2f932e89bf8e 100644 --- a/models/db/list.go +++ b/models/db/list.go @@ -202,8 +202,3 @@ 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) { - var bean T - return GetEngine(ctx).Where(opts.ToConds()).Exist(&bean) -}