From 28e0af25b4bdbdae0fa99470b986acfa005eb12a Mon Sep 17 00:00:00 2001 From: Mathieu Fenniak Date: Wed, 18 Mar 2026 17:01:44 +0100 Subject: [PATCH] perf: remove redundant & incorrect filters on 'SearchRepoOptions.OwnerID' (#11729) Improves the performance of the `/repo/search?uid=N` API call, which is used on the user's dashboard to load a repo list. More detailed notes are in https://codeberg.org/forgejo/forgejo/issues/11702. Removes a redundant query condition (that a user was part of a team in an organization which could see a repo), and a condition that seems incorrect (that a repo could be seen just by being public within a private org, which is incorrect because that doesn't mean the user is a collaborator on the repo). Covered by over 30 test cases in `repo_list_test.go` which did not fail from these changes. Mutation testing (removing the remaining "2." condition) verified that the codepath is covered as tests did fail. ## Checklist The [contributor guide](https://forgejo.org/docs/next/contributor/) contains information that will be helpful to first time contributors. There also are a few [conditions for merging Pull Requests in Forgejo repositories](https://codeberg.org/forgejo/governance/src/branch/main/PullRequestsAgreement.md). You are also welcome to join the [Forgejo development chatroom](https://matrix.to/#/#forgejo-development:matrix.org). ### Tests for Go changes - I added test coverage for Go changes... - [ ] in their respective `*_test.go` for unit tests. - [ ] in the `tests/integration` directory if it involves interactions with a live Forgejo server. - I ran... - [x] `make pr-go` before pushing ### Documentation - [ ] I created a pull request [to the documentation](https://codeberg.org/forgejo/docs) to explain to Forgejo users how to use this change. - [x] I did not document these changes and I do not expect someone else to do it. ### Release notes - [ ] This change will be noticed by a Forgejo user or admin (feature, bug fix, performance, etc.). I suggest to include a release note for this change. - [x] This change is not visible to a Forgejo user or admin (refactor, dependency upgrade, etc.). I think there is no need to add a release note for this change. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11729 Reviewed-by: Gusted Co-authored-by: Mathieu Fenniak Co-committed-by: Mathieu Fenniak --- models/repo/repo_list.go | 39 +++++---------------------------------- 1 file changed, 5 insertions(+), 34 deletions(-) diff --git a/models/repo/repo_list.go b/models/repo/repo_list.go index 5f56e43270..71ec8362f2 100644 --- a/models/repo/repo_list.go +++ b/models/repo/repo_list.go @@ -313,23 +313,6 @@ func userOrgPublicRepoCond(userID int64) builder.Cond { ) } -// userOrgPublicRepoCondPrivate returns the condition that one user could access all public repositories in private organizations -func userOrgPublicRepoCondPrivate(userID int64) builder.Cond { - return builder.And( - builder.Eq{"`repository`.is_private": false}, - builder.In("`repository`.owner_id", - builder.Select("`org_user`.org_id"). - From("org_user"). - Join("INNER", "`user`", "`user`.id = `org_user`.org_id"). - Where(builder.Eq{ - "`org_user`.uid": userID, - "`user`.`type`": user_model.UserTypeOrganization, - "`user`.visibility": structs.VisibleTypePrivate, - }), - ), - ) -} - // UserOrgPublicUnitRepoCond returns the condition that one user could access all public repositories in the special organization func UserOrgPublicUnitRepoCond(userID, orgID int64) builder.Cond { return userOrgPublicRepoCond(userID). @@ -382,26 +365,14 @@ func SearchRepositoryCondition(opts *SearchRepoOptions) builder.Cond { } if opts.Collaborate.ValueOrDefault(true) { - // A Collaboration is: - collaborateCond := builder.NewCond() + + // A Collaboration is: // 1. Repository we don't own collaborateCond = collaborateCond.And(builder.Neq{"owner_id": opts.OwnerID}) - // 2. But we can see because of: - { - userAccessCond := builder.NewCond() - // A. We have unit independent access - userAccessCond = userAccessCond.Or(UserAccessRepoCond("`repository`.id", opts.OwnerID)) - // B. We are in a team for - if opts.UnitType == unit.TypeInvalid { - userAccessCond = userAccessCond.Or(UserOrgTeamRepoCond("`repository`.id", opts.OwnerID)) - } else { - userAccessCond = userAccessCond.Or(userOrgTeamUnitRepoCond("`repository`.id", opts.OwnerID, opts.UnitType)) - } - // C. Public repositories in organizations that we are member of - userAccessCond = userAccessCond.Or(userOrgPublicRepoCondPrivate(opts.OwnerID)) - collaborateCond = collaborateCond.And(userAccessCond) - } + // 2. But we can have access to unit on the repo > AccessModeNone + collaborateCond = collaborateCond.And(UserAccessRepoCond("`repository`.id", opts.OwnerID)) + if !opts.Private { collaborateCond = collaborateCond.And(builder.Expr("owner_id NOT IN (SELECT org_id FROM org_user WHERE org_user.uid = ? AND org_user.is_public = ?)", opts.OwnerID, false)) }