Search query is especially slow if a user searches a generic string
which matches many records, in such case search can take tens of
seconds or time out. To speed up the search query, we search only for
first 1000 records, if there is >1000 matching records we just display
"1000+" instead of precise total count supposing that with such amount
the exact count is not so important for the user.
Because for issues even limited search was not fast enough, 2-phase
approach is used for issues: first we use simpler/faster query to get
all public issues, if this exceeds the limit, we just return the limit.
If the amount of matching results is lower than limit, we re-run more
complex search query (which includes also confidential issues).
Re-running the complex query should be fast enough in such case because the
amount of matching issues is lower than limit.
Because exact total_count is now limited, this patch also switches to
to "prev/next" pagination.
Related #40540
When searching for issues, an additional subquery
is added which filters only issues in a project. If global context is
used (no project is specified) this query filters all projects user has
access to.
In that case we can skip this filter because filtering only projects
user has access to is added anyway.
The filter is used only if a custom project context is specified
Related to #40540
When searching for merge requests, an additional subquery
is added which by default filters only merge requests which belong
to source or target project user has permission for.
This filter is not needed because more restrictive filter
which checks if user has permission for target project
is used in the query.
So unless a custom projects filter is used by user, it's possible
to skip the default projects filter and speed up the final query.
Related to #40540
Replace issue access checks with use of IssuableFinder
Split from !2024 to partially solve https://gitlab.com/gitlab-org/gitlab-ce/issues/23867
## Which fixes are in this MR?
⚠️ - Potentially untested
💣 - No test coverage
🚥 - Test coverage of some sort exists (a test failed when error raised)
🚦 - Test coverage of return value (a test failed when nil used)
✅ - Permissions check tested
### Issue lookup with access check
Using `visible_to_user` likely makes these security issues too. See [Code smells](#code-smells).
- [x] 🚦 app/finders/notes_finder.rb:15 [`visible_to_user`]
- [x] 🚥 app/views/layouts/nav/_project.html.haml:73 [`visible_to_user`] [`.count`]
- [x] ✅ app/services/merge_requests/build_service.rb:84 [`issue.try(:confidential?)`]
- [x] ✅ lib/api/issues.rb:112 [`visible_to_user`]
- CHANGELOG: Prevented API returning issues set to 'Only team members' to everyone
- [x] ✅ lib/api/helpers.rb:126 [`can?(current_user, :read_issue, issue)`] Maybe here too?
- [x] ✅ lib/gitlab/search_results.rb:53 [`visible_to_user`]
### Previous discussions
- [ ] https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/2024/diffs#b2ff264eddf9819d7693c14ae213d941494fe2b3_128_126
- [ ] https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/2024/diffs#7b6375270d22f880bdcb085e47b519b426a5c6c7_87_87
See merge request !2031
Instead of plucking IDs this class now uses ActiveRecord::Relation
objects. Plucking IDs is problematic as searching for projects can lead
to a huge amount of IDs being loaded into memory only to be used as an
argument for another query (instead of just using a sub-query).
Gitlab::SearchResults and Gitlab::ProjectSearchResults are libraries we
are going to use to get search results based on query, enitity type and
pagination.
It will allow us to get only issues from project #23 where title or
description includes 'foo'.
Ex:
search_results = Gitlab::ProjectSearchResults.new(project.id, 'foo', 'issues')
search_results.objects => # [<Issues #23>, <Issues #34>]
search_results.issues_count => 2
search_results.total_count => 12 (it includes results from comments and
merge requests too)
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
Sean McGivern