From cbe1b703dcb7f2b8754406e4abbd5aa24a29a774 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 27 Jun 2026 10:24:02 -0700 Subject: [PATCH] refactor: Use db.Get[] instead of db.GetEngine(ctx).Get(bean) to avoid zero value fetching wrong database record (#37977) This PR replaces a set of struct-based `Get` lookups with explicit `db.Get` / `db.Exist` conditions in places where zero-value fields can lead to ambiguous matches or incorrect records being returned. The main goal is to make read paths deterministic and avoid accidentally matching the wrong row when only part of a struct is populated. ### What changed - replace many `db.GetEngine(ctx).Get(bean)` calls with explicit `builder.Eq` conditions across models such as actions, admin tasks, issues, pull requests, repositories, users, packages, redirects, watches, stars, and follows - use quoted column names where needed for reserved fields like `index`, `type`, and `name` - add dedicated user lookup helpers for: - primary email - OAuth login source / login name - update sign-in and OAuth-related flows to use explicit individual-user lookups instead of partially populated `User` structs - tighten package property and Terraform lock lookups to avoid ambiguous reads and updates - keep existing fallback behavior where needed, while removing reliance on zero-value struct matching ### User-facing impact These changes primarily affect authentication and account lookup paths: - email/username sign-in now re-fetches users through explicit keys - OAuth2 auto-linking now resolves users by name or primary email explicitly - OAuth2 login/sync now looks up users by login source, login type, and login name explicitly - non-individual accounts are no longer implicitly matched through partial user lookups in these flows This should reduce the risk of incorrect account matches and make query behavior more predictable across the codebase. --------- Co-authored-by: bircni --- models/actions/run.go | 12 ++++------ models/admin/task.go | 12 ++++------ models/avatars/avatar.go | 12 ++++++---- models/git/branch.go | 8 +------ models/git/lfs.go | 3 +-- models/git/protected_tag.go | 5 ++-- models/issues/issue.go | 10 +++----- models/issues/issue_update.go | 2 +- models/issues/issue_xref.go | 10 ++++---- models/issues/pull.go | 6 +---- models/organization/org.go | 7 ++---- models/packages/package_property.go | 7 +++--- models/packages/package_version.go | 4 +--- models/pull/review_state.go | 13 ++++++++--- models/repo/attachment.go | 5 ++-- models/repo/collaboration.go | 25 +++++--------------- models/repo/mirror.go | 5 ++-- models/repo/redirect.go | 6 +++-- models/repo/release.go | 3 +-- models/repo/repo.go | 6 ++--- models/repo/star.go | 4 +++- models/repo/watch.go | 12 ++++++---- models/system/appstate.go | 6 ++--- models/user/email_address.go | 4 ++-- models/user/follow.go | 4 +++- models/user/openid.go | 4 +++- models/user/redirect.go | 6 +++-- models/user/setting.go | 3 +-- models/user/user.go | 27 +++++++++++++++++----- modules/packages/terraform/lock.go | 5 ++-- routers/web/auth/auth.go | 21 ++++++++++------- routers/web/auth/oauth.go | 8 +------ services/auth/signin.go | 25 ++++++++++++++++---- services/auth/source/oauth2/source_sync.go | 14 +++-------- services/issue/comments.go | 10 ++++---- 35 files changed, 161 insertions(+), 153 deletions(-) diff --git a/models/actions/run.go b/models/actions/run.go index bba6b4c34e..8d6d804340 100644 --- a/models/actions/run.go +++ b/models/actions/run.go @@ -21,6 +21,8 @@ import ( "gitea.dev/modules/timeutil" "gitea.dev/modules/util" webhook_module "gitea.dev/modules/webhook" + + "xorm.io/builder" ) // ActionRun represents a run of a workflow file @@ -264,11 +266,7 @@ func GetRunByRepoAndID(ctx context.Context, repoID, runID int64) (*ActionRun, er } func GetRunByRepoAndIndex(ctx context.Context, repoID, runIndex int64) (*ActionRun, error) { - run := &ActionRun{ - RepoID: repoID, - Index: runIndex, - } - has, err := db.GetEngine(ctx).Get(run) + run, has, err := db.Get[ActionRun](ctx, builder.Eq{"repo_id": repoID, "`index`": runIndex}) if err != nil { return nil, err } else if !has { @@ -279,9 +277,7 @@ func GetRunByRepoAndIndex(ctx context.Context, repoID, runIndex int64) (*ActionR } func GetLatestRun(ctx context.Context, repoID int64) (*ActionRun, error) { - run := &ActionRun{ - RepoID: repoID, - } + run := &ActionRun{} has, err := db.GetEngine(ctx).Where("repo_id=?", repoID).Desc("index").Get(run) if err != nil { return nil, err diff --git a/models/admin/task.go b/models/admin/task.go index be02137c90..32c9ffef44 100644 --- a/models/admin/task.go +++ b/models/admin/task.go @@ -18,6 +18,8 @@ import ( "gitea.dev/modules/structs" "gitea.dev/modules/timeutil" "gitea.dev/modules/util" + + "xorm.io/builder" ) // Task represents a task @@ -172,17 +174,13 @@ func (err ErrTaskDoesNotExist) Unwrap() error { // GetMigratingTask returns the migrating task by repo's id func GetMigratingTask(ctx context.Context, repoID int64) (*Task, error) { - task := Task{ - RepoID: repoID, - Type: structs.TaskTypeMigrateRepo, - } - has, err := db.GetEngine(ctx).Get(&task) + task, has, err := db.Get[Task](ctx, builder.Eq{"repo_id": repoID, "`type`": structs.TaskTypeMigrateRepo}) if err != nil { return nil, err } else if !has { - return nil, ErrTaskDoesNotExist{0, repoID, task.Type} + return nil, ErrTaskDoesNotExist{0, repoID, structs.TaskTypeMigrateRepo} } - return &task, nil + return task, nil } // CreateTask creates a task on database diff --git a/models/avatars/avatar.go b/models/avatars/avatar.go index 7ecb55183c..59300103a7 100644 --- a/models/avatars/avatar.go +++ b/models/avatars/avatar.go @@ -20,6 +20,7 @@ import ( "gitea.dev/modules/setting" "strk.kbt.io/projects/go/libravatar" + "xorm.io/builder" ) const ( @@ -99,12 +100,13 @@ func HashEmail(email string) string { // GetEmailForHash converts a provided md5sum to the email func GetEmailForHash(ctx context.Context, md5Sum string) (string, error) { return cache.GetString("Avatar:"+md5Sum, func() (string, error) { - emailHash := EmailHash{ - Hash: strings.ToLower(strings.TrimSpace(md5Sum)), + emailHash, has, err := db.Get[EmailHash](ctx, builder.Eq{"`hash`": strings.ToLower(strings.TrimSpace(md5Sum))}) + if err != nil { + return "", err + } else if !has { + return "", nil } - - _, err := db.GetEngine(ctx).Get(&emailHash) - return emailHash.Email, err + return emailHash.Email, nil }) } diff --git a/models/git/branch.go b/models/git/branch.go index 17433fcff8..b74c7aa8b0 100644 --- a/models/git/branch.go +++ b/models/git/branch.go @@ -323,13 +323,7 @@ type RenamedBranch struct { // FindRenamedBranch check if a branch was renamed func FindRenamedBranch(ctx context.Context, repoID int64, from string) (branch *RenamedBranch, exist bool, err error) { - branch = &RenamedBranch{ - RepoID: repoID, - From: from, - } - exist, err = db.GetEngine(ctx).Get(branch) - - return branch, exist, err + return db.Get[RenamedBranch](ctx, builder.Eq{"repo_id": repoID, "`from`": from}) } // RenameBranch rename a branch diff --git a/models/git/lfs.go b/models/git/lfs.go index 75abb89f1f..c5018cb398 100644 --- a/models/git/lfs.go +++ b/models/git/lfs.go @@ -126,8 +126,7 @@ func GetLFSMetaObjectByOid(ctx context.Context, repoID int64, oid string) (*LFSM return nil, ErrLFSObjectNotExist } - m := &LFSMetaObject{Pointer: lfs.Pointer{Oid: oid}, RepositoryID: repoID} - has, err := db.GetEngine(ctx).Get(m) + m, has, err := db.Get[LFSMetaObject](ctx, builder.Eq{"repository_id": repoID, "oid": oid}) if err != nil { return nil, err } else if !has { diff --git a/models/git/protected_tag.go b/models/git/protected_tag.go index 35d406f45c..3af2c5c769 100644 --- a/models/git/protected_tag.go +++ b/models/git/protected_tag.go @@ -13,6 +13,8 @@ import ( "gitea.dev/models/organization" "gitea.dev/modules/glob" "gitea.dev/modules/timeutil" + + "xorm.io/builder" ) // ProtectedTag struct @@ -111,8 +113,7 @@ func GetProtectedTagByID(ctx context.Context, id int64) (*ProtectedTag, error) { // GetProtectedTagByNamePattern gets protected tag by name_pattern func GetProtectedTagByNamePattern(ctx context.Context, repoID int64, pattern string) (*ProtectedTag, error) { - tag := &ProtectedTag{NamePattern: pattern, RepoID: repoID} - has, err := db.GetEngine(ctx).Get(tag) + tag, has, err := db.Get[ProtectedTag](ctx, builder.Eq{"name_pattern": pattern, "repo_id": repoID}) if err != nil { return nil, err } diff --git a/models/issues/issue.go b/models/issues/issue.go index 1dba275ff0..a0af16db83 100644 --- a/models/issues/issue.go +++ b/models/issues/issue.go @@ -361,8 +361,8 @@ func (issue *Issue) ResetAttributesLoaded() { // GetIsRead load the `IsRead` field of the issue func (issue *Issue) GetIsRead(ctx context.Context, userID int64) error { - issueUser := &IssueUser{IssueID: issue.ID, UID: userID} - if has, err := db.GetEngine(ctx).Get(issueUser); err != nil { + issueUser, has, err := db.Get[IssueUser](ctx, builder.Eq{"issue_id": issue.ID, "uid": userID}) + if err != nil { return err } else if !has { issue.IsRead = false @@ -499,11 +499,7 @@ func GetIssueByIndex(ctx context.Context, repoID, index int64) (*Issue, error) { if index < 1 { return nil, ErrIssueNotExist{} } - issue := &Issue{ - RepoID: repoID, - Index: index, - } - has, err := db.GetEngine(ctx).Get(issue) + issue, has, err := db.Get[Issue](ctx, builder.Eq{"repo_id": repoID, "`index`": index}) if err != nil { return nil, err } else if !has { diff --git a/models/issues/issue_update.go b/models/issues/issue_update.go index 74a4b4aac1..f1f877f0d8 100644 --- a/models/issues/issue_update.go +++ b/models/issues/issue_update.go @@ -599,7 +599,7 @@ func ResolveIssueMentionsByVisibility(ctx context.Context, issue *Issue, doer *u resolved[issue.Repo.Owner.LowerName+"/"+team.LowerName] = true continue } - has, err := db.GetEngine(ctx).Get(&organization.TeamUnit{OrgID: issue.Repo.Owner.ID, TeamID: team.ID, Type: unittype}) + has, err := db.Exist[organization.TeamUnit](ctx, builder.Eq{"org_id": issue.Repo.Owner.ID, "team_id": team.ID, "`type`": unittype}) if err != nil { return nil, fmt.Errorf("get team units (%d): %w", team.ID, err) } diff --git a/models/issues/issue_xref.go b/models/issues/issue_xref.go index 0bdc48d715..74f97f1ad5 100644 --- a/models/issues/issue_xref.go +++ b/models/issues/issue_xref.go @@ -13,6 +13,8 @@ import ( user_model "gitea.dev/models/user" "gitea.dev/modules/log" "gitea.dev/modules/references" + + "xorm.io/builder" ) type crossReference struct { @@ -189,11 +191,11 @@ func (issue *Issue) updateCrossReferenceList(list []*crossReference, xref *cross func (issue *Issue) verifyReferencedIssue(stdCtx context.Context, ctx *crossReferencesContext, repo *repo_model.Repository, ref references.IssueReference, ) (*Issue, references.XRefAction, error) { - refIssue := &Issue{RepoID: repo.ID, Index: ref.Index} refAction := ref.Action - e := db.GetEngine(stdCtx) - - if has, _ := e.Get(refIssue); !has { + refIssue, has, err := db.Get[Issue](stdCtx, builder.Eq{"repo_id": repo.ID, "`index`": ref.Index}) + if err != nil { + return nil, references.XRefActionNone, err + } else if !has { return nil, references.XRefActionNone, nil } if err := refIssue.LoadRepo(stdCtx); err != nil { diff --git a/models/issues/pull.go b/models/issues/pull.go index 6de5338a53..7eb0a5b72d 100644 --- a/models/issues/pull.go +++ b/models/issues/pull.go @@ -535,12 +535,8 @@ func GetPullRequestByIndex(ctx context.Context, repoID, index int64) (*PullReque if index < 1 { return nil, ErrPullRequestNotExist{} } - pr := &PullRequest{ - BaseRepoID: repoID, - Index: index, - } - has, err := db.GetEngine(ctx).Get(pr) + pr, has, err := db.Get[PullRequest](ctx, builder.Eq{"base_repo_id": repoID, "`index`": index}) if err != nil { return nil, err } else if !has { diff --git a/models/organization/org.go b/models/organization/org.go index 9a28aa3918..deb7d54b79 100644 --- a/models/organization/org.go +++ b/models/organization/org.go @@ -411,11 +411,8 @@ func GetOrgByName(ctx context.Context, name string) (*Organization, error) { if len(name) == 0 { return nil, ErrOrgNotExist{0, name} } - u := &Organization{ - LowerName: strings.ToLower(name), - Type: user_model.UserTypeOrganization, - } - has, err := db.GetEngine(ctx).Get(u) + + u, has, err := db.Get[Organization](ctx, builder.Eq{"lower_name": strings.ToLower(name), "`type`": user_model.UserTypeOrganization}) if err != nil { return nil, err } else if !has { diff --git a/models/packages/package_property.go b/models/packages/package_property.go index 52815a9e43..6aa5c435a9 100644 --- a/models/packages/package_property.go +++ b/models/packages/package_property.go @@ -58,7 +58,7 @@ func GetProperties(ctx context.Context, refType PropertyType, refID int64) ([]*P // GetPropertiesByName gets all properties with a specific name func GetPropertiesByName(ctx context.Context, refType PropertyType, refID int64, name string) ([]*PackageProperty, error) { pps := make([]*PackageProperty, 0, 10) - return pps, db.GetEngine(ctx).Where("ref_type = ? AND ref_id = ? AND name = ?", refType, refID, name).OrderBy("id").Find(&pps) + return pps, db.GetEngine(ctx).Where("ref_type = ? AND ref_id = ? AND `name` = ?", refType, refID, name).OrderBy("id").Find(&pps) } // UpdateProperty updates a property @@ -68,13 +68,12 @@ func UpdateProperty(ctx context.Context, pp *PackageProperty) error { } func InsertOrUpdateProperty(ctx context.Context, refType PropertyType, refID int64, name, value string) error { - pp := PackageProperty{RefType: refType, RefID: refID, Name: name} - ok, err := db.GetEngine(ctx).Get(&pp) + pp, ok, err := db.Get[PackageProperty](ctx, builder.Eq{"ref_type": refType, "ref_id": refID, "`name`": name}) if err != nil { return err } if ok { - _, err = db.GetEngine(ctx).Where("ref_type=? AND ref_id=? AND name=?", refType, refID, name).Cols("value").Update(&PackageProperty{Value: value}) + _, err = db.GetEngine(ctx).ID(pp.ID).Cols("value").Update(&PackageProperty{Value: value}) return err } _, err = InsertProperty(ctx, refType, refID, name, value) diff --git a/models/packages/package_version.go b/models/packages/package_version.go index 77b1da43de..d854545d0e 100644 --- a/models/packages/package_version.go +++ b/models/packages/package_version.go @@ -164,9 +164,7 @@ func DeleteVersionsByPackageID(ctx context.Context, packageID int64) error { // HasVersionFileReferences checks if there are associated files func HasVersionFileReferences(ctx context.Context, versionID int64) (bool, error) { - return db.GetEngine(ctx).Get(&PackageFile{ - VersionID: versionID, - }) + return db.Exist[PackageFile](ctx, builder.Eq{"version_id": versionID}) } // SearchValue describes a value to search diff --git a/models/pull/review_state.go b/models/pull/review_state.go index 241a79d1df..13969d895a 100644 --- a/models/pull/review_state.go +++ b/models/pull/review_state.go @@ -11,6 +11,8 @@ import ( "gitea.dev/models/db" "gitea.dev/modules/log" "gitea.dev/modules/timeutil" + + "xorm.io/builder" ) // ViewedState stores for a file in which state it is currently viewed @@ -66,9 +68,14 @@ func (rs *ReviewState) GetViewedFileCount() int { // If the review didn't exist before in the database, it won't afterwards either. // The returned boolean shows whether the review exists in the database func GetReviewState(ctx context.Context, userID, pullID int64, commitSHA string) (*ReviewState, bool, error) { - review := &ReviewState{UserID: userID, PullID: pullID, CommitSHA: commitSHA} - has, err := db.GetEngine(ctx).Get(review) - return review, has, err + review, has, err := db.Get[ReviewState](ctx, builder.Eq{"user_id": userID, "pull_id": pullID, "commit_sha": commitSHA}) + if err != nil { + return nil, false, err + } + if review == nil { + review = &ReviewState{UserID: userID, PullID: pullID, CommitSHA: commitSHA} + } + return review, has, nil } // UpdateReviewState updates the given review inside the database, regardless of whether it existed before or not diff --git a/models/repo/attachment.go b/models/repo/attachment.go index b7136b657b..cb74f310d4 100644 --- a/models/repo/attachment.go +++ b/models/repo/attachment.go @@ -17,6 +17,8 @@ import ( "gitea.dev/modules/storage" "gitea.dev/modules/timeutil" "gitea.dev/modules/util" + + "xorm.io/builder" ) // Attachment represent a attachment of issue/comment/release. @@ -156,8 +158,7 @@ func GetAttachmentsByCommentID(ctx context.Context, commentID int64) ([]*Attachm // GetAttachmentByReleaseIDFileName returns attachment by given releaseId and fileName. func GetAttachmentByReleaseIDFileName(ctx context.Context, releaseID int64, fileName string) (*Attachment, error) { - attach := &Attachment{ReleaseID: releaseID, Name: fileName} - has, err := db.GetEngine(ctx).Get(attach) + attach, has, err := db.Get[Attachment](ctx, builder.Eq{"release_id": releaseID, "`name`": fileName}) if err != nil { return nil, err } else if !has { diff --git a/models/repo/collaboration.go b/models/repo/collaboration.go index 5748265f96..9173ec733e 100644 --- a/models/repo/collaboration.go +++ b/models/repo/collaboration.go @@ -102,20 +102,13 @@ func GetCollaborators(ctx context.Context, opts *FindCollaborationOptions) ([]*C // GetCollaboration get collaboration for a repository id with a user id func GetCollaboration(ctx context.Context, repoID, uid int64) (*Collaboration, error) { - collaboration := &Collaboration{ - RepoID: repoID, - UserID: uid, - } - has, err := db.GetEngine(ctx).Get(collaboration) - if !has { - collaboration = nil - } + collaboration, _, err := db.Get[Collaboration](ctx, builder.Eq{"repo_id": repoID, "user_id": uid}) return collaboration, err } // IsCollaborator check if a user is a collaborator of a repository func IsCollaborator(ctx context.Context, repoID, userID int64) (bool, error) { - return db.GetEngine(ctx).Get(&Collaboration{RepoID: repoID, UserID: userID}) + return db.Exist[Collaboration](ctx, builder.Eq{"repo_id": repoID, "user_id": userID}) } // ChangeCollaborationAccessMode sets new access mode for the collaboration. @@ -126,13 +119,7 @@ func ChangeCollaborationAccessMode(ctx context.Context, repo *Repository, uid in } return db.WithTx(ctx, func(ctx context.Context) error { - e := db.GetEngine(ctx) - - collaboration := &Collaboration{ - RepoID: repo.ID, - UserID: uid, - } - has, err := e.Get(collaboration) + collaboration, has, err := db.Get[Collaboration](ctx, builder.Eq{"repo_id": repo.ID, "user_id": uid}) if err != nil { return fmt.Errorf("get collaboration: %w", err) } else if !has { @@ -144,12 +131,12 @@ func ChangeCollaborationAccessMode(ctx context.Context, repo *Repository, uid in } collaboration.Mode = mode - if _, err = e. + if _, err = db.GetEngine(ctx). ID(collaboration.ID). Cols("mode"). Update(collaboration); err != nil { return fmt.Errorf("update collaboration: %w", err) - } else if _, err = e.Exec("UPDATE access SET mode = ? WHERE user_id = ? AND repo_id = ?", mode, uid, repo.ID); err != nil { + } else if _, err = db.Exec(ctx, "UPDATE access SET mode = ? WHERE user_id = ? AND repo_id = ?", mode, uid, repo.ID); err != nil { return fmt.Errorf("update access table: %w", err) } @@ -174,5 +161,5 @@ func IsOwnerMemberCollaborator(ctx context.Context, repo *Repository, userID int return true, nil } - return db.GetEngine(ctx).Get(&Collaboration{RepoID: repo.ID, UserID: userID}) + return db.Exist[Collaboration](ctx, builder.Eq{"repo_id": repo.ID, "user_id": userID}) } diff --git a/models/repo/mirror.go b/models/repo/mirror.go index 8bfff01a02..1985b49963 100644 --- a/models/repo/mirror.go +++ b/models/repo/mirror.go @@ -12,6 +12,8 @@ import ( "gitea.dev/modules/log" "gitea.dev/modules/timeutil" "gitea.dev/modules/util" + + "xorm.io/builder" ) // ErrMirrorNotExist mirror does not exist error @@ -76,8 +78,7 @@ func (m *Mirror) ScheduleNextUpdate() { // GetMirrorByRepoID returns mirror information of a repository. func GetMirrorByRepoID(ctx context.Context, repoID int64) (*Mirror, error) { - m := &Mirror{RepoID: repoID} - has, err := db.GetEngine(ctx).Get(m) + m, has, err := db.Get[Mirror](ctx, builder.Eq{"repo_id": repoID}) if err != nil { return nil, err } else if !has { diff --git a/models/repo/redirect.go b/models/repo/redirect.go index 405feb5782..c51c5d69b0 100644 --- a/models/repo/redirect.go +++ b/models/repo/redirect.go @@ -10,6 +10,8 @@ import ( "gitea.dev/models/db" "gitea.dev/modules/util" + + "xorm.io/builder" ) // ErrRedirectNotExist represents a "RedirectNotExist" kind of error. @@ -52,8 +54,8 @@ func init() { // LookupRedirect look up if a repository has a redirect name func LookupRedirect(ctx context.Context, ownerID int64, repoName string) (int64, error) { repoName = strings.ToLower(repoName) - redirect := &Redirect{OwnerID: ownerID, LowerName: repoName} - if has, err := db.GetEngine(ctx).Get(redirect); err != nil { + redirect, has, err := db.Get[Redirect](ctx, builder.Eq{"owner_id": ownerID, "lower_name": repoName}) + if err != nil { return 0, err } else if !has { return 0, ErrRedirectNotExist{OwnerID: ownerID, RepoName: repoName} diff --git a/models/repo/release.go b/models/repo/release.go index 2c3ee18757..b4f5fa902b 100644 --- a/models/repo/release.go +++ b/models/repo/release.go @@ -216,8 +216,7 @@ func AddReleaseAttachments(ctx context.Context, releaseID int64, attachmentUUIDs // GetRelease returns release by given ID. func GetRelease(ctx context.Context, repoID int64, tagName string) (*Release, error) { - rel := &Release{RepoID: repoID, LowerTagName: strings.ToLower(tagName)} - has, err := db.GetEngine(ctx).Get(rel) + rel, has, err := db.Get[Release](ctx, builder.Eq{"repo_id": repoID, "lower_tag_name": strings.ToLower(tagName)}) if err != nil { return nil, err } else if !has { diff --git a/models/repo/repo.go b/models/repo/repo.go index e56603fc81..ee7e62b119 100644 --- a/models/repo/repo.go +++ b/models/repo/repo.go @@ -849,9 +849,9 @@ func GetRepositoriesMapByIDs(ctx context.Context, ids []int64) (map[int64]*Repos } func IsRepositoryModelExist(ctx context.Context, u *user_model.User, repoName string) (bool, error) { - return db.GetEngine(ctx).Get(&Repository{ - OwnerID: u.ID, - LowerName: strings.ToLower(repoName), + return db.Exist[Repository](ctx, builder.Eq{ + "owner_id": u.ID, + "lower_name": strings.ToLower(repoName), }) } diff --git a/models/repo/star.go b/models/repo/star.go index c3f1203fec..cecaecfdea 100644 --- a/models/repo/star.go +++ b/models/repo/star.go @@ -9,6 +9,8 @@ import ( "gitea.dev/models/db" user_model "gitea.dev/models/user" "gitea.dev/modules/timeutil" + + "xorm.io/builder" ) // Star represents a starred repo by a user. @@ -68,7 +70,7 @@ func StarRepo(ctx context.Context, doer *user_model.User, repo *Repository, star // IsStaring checks if user has starred given repository. func IsStaring(ctx context.Context, userID, repoID int64) bool { - has, _ := db.GetEngine(ctx).Get(&Star{UID: userID, RepoID: repoID}) + has, _ := db.Exist[Star](ctx, builder.Eq{"uid": userID, "repo_id": repoID}) return has } diff --git a/models/repo/watch.go b/models/repo/watch.go index 3d349dde3e..7b7c331c2b 100644 --- a/models/repo/watch.go +++ b/models/repo/watch.go @@ -10,6 +10,8 @@ import ( user_model "gitea.dev/models/user" "gitea.dev/modules/setting" "gitea.dev/modules/timeutil" + + "xorm.io/builder" ) // WatchMode specifies what kind of watch the user has on a repository @@ -41,12 +43,14 @@ func init() { } // GetWatch gets what kind of subscription a user has on a given repository; returns dummy record if none found -func GetWatch(ctx context.Context, userID, repoID int64) (Watch, error) { - watch := Watch{UserID: userID, RepoID: repoID} - has, err := db.GetEngine(ctx).Get(&watch) +func GetWatch(ctx context.Context, userID, repoID int64) (*Watch, error) { + watch, has, err := db.Get[Watch](ctx, builder.Eq{"user_id": userID, "repo_id": repoID}) if err != nil { return watch, err } + if watch == nil { + watch = &Watch{UserID: userID, RepoID: repoID} + } if !has { watch.Mode = WatchModeNone } @@ -64,7 +68,7 @@ func IsWatching(ctx context.Context, userID, repoID int64) bool { return err == nil && IsWatchMode(watch.Mode) } -func watchRepoMode(ctx context.Context, watch Watch, mode WatchMode) (err error) { +func watchRepoMode(ctx context.Context, watch *Watch, mode WatchMode) (err error) { if watch.Mode == mode { return nil } diff --git a/models/system/appstate.go b/models/system/appstate.go index 34a136e3bd..c7203a52ec 100644 --- a/models/system/appstate.go +++ b/models/system/appstate.go @@ -7,6 +7,8 @@ import ( "context" "gitea.dev/models/db" + + "xorm.io/builder" ) // AppState represents a state record in database @@ -44,9 +46,7 @@ func SaveAppStateContent(ctx context.Context, key, content string) error { // GetAppStateContent gets an app state from database func GetAppStateContent(ctx context.Context, key string) (content string, err error) { - e := db.GetEngine(ctx) - appState := &AppState{ID: key} - has, err := e.Get(appState) + appState, has, err := db.Get[AppState](ctx, builder.Eq{"id": key}) if err != nil { return "", err } else if !has { diff --git a/models/user/email_address.go b/models/user/email_address.go index 5a259b8e2b..7365f5374e 100644 --- a/models/user/email_address.go +++ b/models/user/email_address.go @@ -348,8 +348,8 @@ func VerifyActiveEmailCode(ctx context.Context, code, email string) *EmailAddres opts := &TimeLimitCodeOptions{Purpose: TimeLimitCodeActivateEmail, NewEmail: email} data := makeTimeLimitCodeHashData(opts, user) if base.VerifyTimeLimitCode(time.Now(), data, setting.Service.ActiveCodeLives, prefix) { - emailAddress := &EmailAddress{UID: user.ID, Email: email} - if has, _ := db.GetEngine(ctx).Get(emailAddress); has { + emailAddress, has, _ := db.Get[EmailAddress](ctx, builder.Eq{"uid": user.ID, "email": email}) + if has { return emailAddress } } diff --git a/models/user/follow.go b/models/user/follow.go index 617211886e..11d293bc5e 100644 --- a/models/user/follow.go +++ b/models/user/follow.go @@ -8,6 +8,8 @@ import ( "gitea.dev/models/db" "gitea.dev/modules/timeutil" + + "xorm.io/builder" ) // Follow represents relations of user and their followers. @@ -24,7 +26,7 @@ func init() { // IsFollowing returns true if user is following followID. func IsFollowing(ctx context.Context, userID, followID int64) bool { - has, _ := db.GetEngine(ctx).Get(&Follow{UserID: userID, FollowID: followID}) + has, _ := db.Exist[Follow](ctx, builder.Eq{"user_id": userID, "follow_id": followID}) return has } diff --git a/models/user/openid.go b/models/user/openid.go index 84ec5a5cd8..dd4682b440 100644 --- a/models/user/openid.go +++ b/models/user/openid.go @@ -9,6 +9,8 @@ import ( "gitea.dev/models/db" "gitea.dev/modules/util" + + "xorm.io/builder" ) // UserOpenID is the list of all OpenID identities of a user. @@ -43,7 +45,7 @@ func isOpenIDUsed(ctx context.Context, uri string) (bool, error) { return true, nil } - return db.GetEngine(ctx).Get(&UserOpenID{URI: uri}) + return db.Exist[UserOpenID](ctx, builder.Eq{"uri": uri}) } // ErrOpenIDAlreadyUsed represents a "OpenIDAlreadyUsed" kind of error. diff --git a/models/user/redirect.go b/models/user/redirect.go index 325a643eb6..93a9c94c73 100644 --- a/models/user/redirect.go +++ b/models/user/redirect.go @@ -10,6 +10,8 @@ import ( "gitea.dev/models/db" "gitea.dev/modules/util" + + "xorm.io/builder" ) // ErrUserRedirectNotExist represents a "UserRedirectNotExist" kind of error. @@ -50,8 +52,8 @@ func init() { // LookupUserRedirect look up userID if a user has a redirect name func LookupUserRedirect(ctx context.Context, userName string) (int64, error) { userName = strings.ToLower(userName) - redirect := &Redirect{LowerName: userName} - if has, err := db.GetEngine(ctx).Get(redirect); err != nil { + redirect, has, err := db.Get[Redirect](ctx, builder.Eq{"lower_name": userName}) + if err != nil { return 0, err } else if !has { return 0, ErrUserRedirectNotExist{Name: userName} diff --git a/models/user/setting.go b/models/user/setting.go index a3903d5915..8888e3a2e0 100644 --- a/models/user/setting.go +++ b/models/user/setting.go @@ -125,8 +125,7 @@ func GetUserSetting(ctx context.Context, userID int64, key string, def ...string return "", err } - setting := &Setting{UserID: userID, SettingKey: key} - has, err := db.GetEngine(ctx).Get(setting) + setting, has, err := db.Get[Setting](ctx, builder.Eq{"user_id": userID, "setting_key": key}) if err != nil { return "", err } diff --git a/models/user/user.go b/models/user/user.go index 4e6227de9e..6d3d85c309 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -1276,8 +1276,7 @@ func GetUserByEmail(ctx context.Context, email string) (*User, error) { email = strings.ToLower(email) // Otherwise, check in alternative list for activated email addresses - emailAddress := &EmailAddress{LowerEmail: email, IsActivated: true} - has, err := db.GetEngine(ctx).Get(emailAddress) + emailAddress, has, err := db.Get[EmailAddress](ctx, builder.Eq{"lower_email": email, "is_activated": true}) if err != nil { return nil, err } @@ -1297,13 +1296,29 @@ func GetUserByEmail(ctx context.Context, email string) (*User, error) { return nil, ErrUserNotExist{Name: email} } -func GetIndividualUser(ctx context.Context, user *User) (bool, error) { - // FIXME: the design is wrong, empty User fields won't apply, this function should be removed in the future - has, err := db.GetEngine(ctx).Get(user) +func GetIndividualUserByPrimaryEmail(ctx context.Context, email string) (*User, error) { + email = strings.ToLower(strings.TrimSpace(email)) + if len(email) == 0 { + return nil, ErrUserNotExist{Name: email} + } + + user, has, err := db.Get[User](ctx, builder.Eq{"email": email, "type": UserTypeIndividual}) + if err != nil { + return nil, err + } + if !has { + return nil, ErrUserNotExist{Name: email} + } + return user, nil +} + +func GetIndividualUserByLoginSource(ctx context.Context, loginType auth.Type, loginSource int64, loginName string) (*User, bool, error) { + user, has, err := db.Get[User](ctx, builder.Eq{"login_type": loginType, "login_source": loginSource, "login_name": loginName}) if has && user.Type != UserTypeIndividual { has = false + user = nil } - return has, err + return user, has, err } // GetUserByOpenID returns the user object by given OpenID if exists. diff --git a/modules/packages/terraform/lock.go b/modules/packages/terraform/lock.go index ed9edad8e2..1bce4b109e 100644 --- a/modules/packages/terraform/lock.go +++ b/modules/packages/terraform/lock.go @@ -79,13 +79,12 @@ func RemoveLock(ctx context.Context, packageID int64) error { } func updateLock(ctx context.Context, refID int64, value string, cond builder.Cond) error { - pp := packages_model.PackageProperty{RefType: packages_model.PropertyTypePackage, RefID: refID, Name: LockFile} - ok, err := db.GetEngine(ctx).Get(&pp) + pp, ok, err := db.Get[packages_model.PackageProperty](ctx, builder.Eq{"ref_type": packages_model.PropertyTypePackage, "ref_id": refID, "`name`": LockFile}) if err != nil { return err } if ok { - n, err := db.GetEngine(ctx).Where("ref_type=? AND ref_id=? AND name=?", packages_model.PropertyTypePackage, refID, LockFile).And(cond).Cols("value").Update(&packages_model.PackageProperty{Value: value}) + n, err := db.GetEngine(ctx).ID(pp.ID).And(cond).Cols("value").Update(&packages_model.PackageProperty{Value: value}) if err != nil { return err } diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go index 63d069d31d..18218e92da 100644 --- a/routers/web/auth/auth.go +++ b/routers/web/auth/auth.go @@ -627,14 +627,19 @@ func createUserInContext(ctx *context.Context, tpl templates.TplName, form any, if possibleLinkAccountData != nil && (user_model.IsErrUserAlreadyExist(err) || user_model.IsErrEmailAlreadyUsed(err)) { switch setting.OAuth2Client.AccountLinking { case setting.OAuth2AccountLinkingAuto: - var user *user_model.User - user = &user_model.User{Name: u.Name} - hasUser, err := user_model.GetIndividualUser(ctx, user) - if !hasUser || err != nil { - user = &user_model.User{Email: u.Email} - hasUser, err = user_model.GetIndividualUser(ctx, user) - if !hasUser || err != nil { - ctx.ServerError("UserLinkAccount", err) + user, err := user_model.GetIndividualUserByName(ctx, u.Name) + if err != nil { + if !user_model.IsErrUserNotExist(err) { + ctx.ServerError("GetIndividualUserByName", err) + return false + } + user, err = user_model.GetIndividualUserByPrimaryEmail(ctx, u.Email) + if err != nil { + if !user_model.IsErrUserNotExist(err) { + ctx.ServerError("GetIndividualUserByPrimaryEmail", err) + } else { + ctx.NotFound(err) + } return false } } diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index 06c1b20d57..7099aa55dd 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -503,13 +503,7 @@ func oAuth2UserLoginCallback(ctx *context.Context, authSource *auth.Source, requ } } - user := &user_model.User{ - LoginName: gothUser.UserID, - LoginType: auth.OAuth2, - LoginSource: authSource.ID, - } - - hasUser, err := user_model.GetIndividualUser(ctx, user) + user, hasUser, err := user_model.GetIndividualUserByLoginSource(ctx, auth.OAuth2, authSource.ID, gothUser.UserID) if err != nil { return nil, goth.User{}, err } diff --git a/services/auth/signin.go b/services/auth/signin.go index b4ba50c250..516d5463bc 100644 --- a/services/auth/signin.go +++ b/services/auth/signin.go @@ -19,6 +19,8 @@ import ( _ "gitea.dev/services/auth/source/ldap" // register the ldap source _ "gitea.dev/services/auth/source/pam" // register the pam source _ "gitea.dev/services/auth/source/sspi" // register the sspi source + + "xorm.io/builder" ) // UserSignIn validates user name and password. @@ -27,9 +29,8 @@ func UserSignIn(ctx context.Context, username, password string) (*user_model.Use isEmail := false if strings.Contains(username, "@") { isEmail = true - emailAddress := user_model.EmailAddress{LowerEmail: strings.ToLower(strings.TrimSpace(username))} // check same email - has, err := db.GetEngine(ctx).Get(&emailAddress) + emailAddress, has, err := db.Get[user_model.EmailAddress](ctx, builder.Eq{"lower_email": strings.ToLower(strings.TrimSpace(username))}) if err != nil { return nil, nil, err } @@ -51,9 +52,23 @@ func UserSignIn(ctx context.Context, username, password string) (*user_model.Use } if user != nil { - hasUser, err := user_model.GetIndividualUser(ctx, user) - if err != nil { - return nil, nil, err + var hasUser bool + var err error + if user.ID > 0 { + user, err = user_model.GetUserByID(ctx, user.ID) + if err != nil && !user_model.IsErrUserNotExist(err) { + return nil, nil, err + } + if user != nil && user.Type != user_model.UserTypeIndividual { + return nil, nil, user_model.ErrUserNotExist{Name: username} + } + hasUser = user != nil + } else if user.LowerName != "" { + user, err = user_model.GetIndividualUserByName(ctx, user.LowerName) + if err != nil && !user_model.IsErrUserNotExist(err) { + return nil, nil, err + } + hasUser = user != nil } if hasUser { diff --git a/services/auth/source/oauth2/source_sync.go b/services/auth/source/oauth2/source_sync.go index 3123e54bbb..5b8511287e 100644 --- a/services/auth/source/oauth2/source_sync.go +++ b/services/auth/source/oauth2/source_sync.go @@ -61,13 +61,7 @@ func (source *Source) refresh(ctx context.Context, provider goth.Provider, u *us } } - user := &user_model.User{ - LoginName: u.ExternalID, - LoginType: auth.OAuth2, - LoginSource: u.LoginSourceID, - } - - hasUser, err := user_model.GetIndividualUser(ctx, user) + user, hasUser, err := user_model.GetIndividualUserByLoginSource(ctx, auth.OAuth2, u.LoginSourceID, u.ExternalID) if err != nil { return err } @@ -77,13 +71,11 @@ func (source *Source) refresh(ctx context.Context, provider goth.Provider, u *us // recognizes them as a valid user, they will be able to login // via their provider and reactivate their account. if shouldDisable { - log.Info("SyncExternalUsers[%s] disabling user %d", source.AuthSource.Name, user.ID) - return db.WithTx(ctx, func(ctx context.Context) error { if hasUser { + log.Info("SyncExternalUsers[%s] disabling user %d", source.AuthSource.Name, user.ID) user.IsActive = false - err := user_model.UpdateUserCols(ctx, user, "is_active") - if err != nil { + if err := user_model.UpdateUserCols(ctx, user, "is_active"); err != nil { return err } } diff --git a/services/issue/comments.go b/services/issue/comments.go index ff6c9fccf5..5b70dbf72e 100644 --- a/services/issue/comments.go +++ b/services/issue/comments.go @@ -19,6 +19,8 @@ import ( "gitea.dev/modules/timeutil" git_service "gitea.dev/services/git" notify_service "gitea.dev/services/notify" + + "xorm.io/builder" ) // CreateRefComment creates a commit reference comment to issue. @@ -34,10 +36,10 @@ func CreateRefComment(ctx context.Context, doer *user_model.User, repo *repo_mod } // Check if same reference from same commit has already existed. - has, err := db.GetEngine(ctx).Get(&issues_model.Comment{ - Type: issues_model.CommentTypeCommitRef, - IssueID: issue.ID, - CommitSHA: commitSHA, + has, err := db.Exist[issues_model.Comment](ctx, builder.Eq{ + "`type`": issues_model.CommentTypeCommitRef, + "issue_id": issue.ID, + "commit_sha": commitSHA, }) if err != nil { return fmt.Errorf("check reference comment: %w", err)