| Commit message (Collapse) | Author | Age | Files | Lines |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This completely rewrites the SnippetsFinder class from the ground up in
order to improve its performance. The old code was beyond salvaging. It
was complex, included various Rails 5 workarounds, comments that
shouldn't be necessary, and most important of all: it produced a really
poorly performing database query.
As a result, I opted for rewriting the finder from scratch, instead of
trying to patch the existing code. Instead of trying to reuse as many
existing methods as possible, I opted for defining new methods
specifically meant for the SnippetsFinder. This requires some extra code
here and there, but allows us to have much more control over the
resulting SQL queries. It is these changes that then allow us to produce
a _much_ more efficient query.
To illustrate how bad the old query was, we will use my own snippets as
an example. Currently I have 52 snippets, most of which are global ones.
To retrieve these, you would run the following Ruby code:
user = User.find_by(username: 'yorickpeterse')
SnippetsFinder.new(user, author: user).execute
On GitLab.com the resulting query will take between 10 and 15 seconds to
run, producing the query plan found at
https://explain.depesz.com/s/Y5IX. Apart from the long execution time,
the total number of buffers (the sum of all shared hits) is around 185
GB, though the real number is probably (hopefully) much lower as I doubt
simply summing these numbers produces the true total number of buffers
used.
The new query's plan can be found at https://explain.depesz.com/s/wHdN,
and this query takes between 10 and 100-ish milliseconds to run. The
total number of buffers used is only about 30 MB.
Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/52639
|
|
|
|
|
|
|
| |
It's possible that user pastes accidentally also unsubscribe link
which is included in footer of notification emails. This unsubscribe
link contains personal token which attacker then use to act as the
original user (e.g. for sending comments under his/her identity).
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
This commit adds the module `FromUnion`, which provides the class method
`from_union`. This simplifies the process of selecting data from the
result of a UNION, and reduces the likelihood of making mistakes. As a
result, instead of this:
union = Gitlab::SQL::Union.new([foo, bar])
Foo.from("(#{union.to_sql}) #{Foo.table_name}")
We can now write this instead:
Foo.from_union([foo, bar])
This commit also includes some changes to make this new setup work
properly. For example, a bug in Rails 4
(https://github.com/rails/rails/issues/24193) would break the use of
`from("sub-query-here").includes(:relation)` in certain cases. There was
also a CI query which appeared to repeat a lot of conditions from an
outer query on an inner query, which isn't necessary.
Finally, we include a RuboCop cop to ensure developers use this new
module, instead of using Gitlab::SQL::Union directly.
Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/51307
|
|\
| |
| |
| |
| | |
Enable frozen string in app/models/*.rb
See merge request gitlab-org/gitlab-ce!20851
|
| |
| |
| |
| | |
Partially addresses #47424.
|
|/
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The status is shown for
- The author of a commit when viewing a commit
- Notes on a commit (regular/diff)
- The user that triggered a pipeline when viewing a pipeline
- The author of a merge request when viewing a merge request
- The author of notes on a merge request (regular/diff)
- The author of an issue when viewing an issue
- The author of notes on an issue
- The author of a snippet when viewing a snippet
- The author of notes on a snippet
- A user's profile page
- The list of members of a group/user
|
|
|
|
|
| |
* adds basic discussions API for issues and snippets
* reorganizes notes specs (so same tests can be used for all noteable types - issues, MRs, snippets)
|
|
|
|
|
|
| |
'security-10-4-25223-snippets-finder-doesnt-obey-feature-visibility' into 'security-10-4'
[Port for security-10-4]: Makes SnippetFinder ensure feature visibility
|
|
|
|
| |
including/extending it
|
| |
|
| |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
The initializers including this were doing so at the top level, so every object
loaded after them had a `current_application_settings` method. However, if
someone had rack-attack enabled (which was loaded before these initializers), it
would try to load the API, and fail, because `Gitlab::CurrentSettings` didn't
have that method.
To fix this:
1. Don't include `Gitlab::CurrentSettings` at the top level. We do not need
`Object.new.current_application_settings` to work.
2. Make `Gitlab::CurrentSettings` explicitly `extend self`, as we already use it
like that in several places.
3. Change the initializers to use that new form.
|
|
|
|
|
|
|
|
| |
This is allowed for existing instances so we don't end up 76 offenses
right away, but for new code one should _only_ use this if they _have_
to remove non database data. Even then it's usually better to do this in
a service class as this gives you more control over how to remove the
data (e.g. in bulk).
|
| |
|
|\
| |
| |
| |
| |
| |
| | |
Support descriptions for snippets
Closes #31894
See merge request !11071
|
| | |
|
|/ |
|
|
|
|
|
|
| |
Refactor snippets finder & dont return internal snippets for external users
See merge request !2094
|
| |
|
| |
|
| |
|
| |
|
|
|
|
|
|
| |
Add `visibility` & `visibility=` methods to the
`Gitlab::VisibilityLevel` module so the `visibility_level` can be
get/set with a string value.
|
|
|
|
|
|
| |
Apply the same spam checks to public snippets (either personal snippets
that are public, or public snippets on public projects) as to issues on
public projects.
|
|
|
|
|
|
|
| |
When a snippet is submitted, but there's an error, we didn't keep the
visibility level. As the default is private, this means that submitting
a public snippet that failed would then fall back to being a private
snippet.
|
|
|
|
| |
Also added relevant specs and refactored to_references in a bunch of places to be more consistent.
|
|\
| |
| |
| |
| |
| |
| | |
Use :maximum instead of :within for length validators with a 0..N range
Closes #25209
See merge request !7894
|
| |
| |
| |
| | |
Signed-off-by: Rémy Coutable <remy@rymai.me>
|
|/ |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
This commit adds a number of _html columns and, with the exception of Note,
starts updating them whenever the content of their partner fields changes.
Note has a collision with the note_html attr_accessor; that will be fixed later
A background worker for clearing these cache columns is also introduced - use
`rake cache:clear` to set it off. You can clear the database or Redis caches
separately by running `rake cache:clear:db` or `rake cache:clear:redis`,
respectively.
|
| |
|
|\
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
Add custom highlighting via .gitattributes
## What does this MR do?
Allows user control of language selection via a `gitlab-language` entry in `.gitattributes`
## Are there points in the code the reviewer needs to double check?
(paired with @stanhu)
## Why was this MR needed?
Guessing languages by filename is fraught and often wrong. In one project, `foo.pl` may be perl, and in another it may be prolog. Users might have a Thingfile that needs ruby highlighting, or depend on things that can't work in general, like `*.C` (capitalized) mapping to C++ instead of C.
This allows the user to override language choice so they never have to look at a mis-highlighted file.
## What are the relevant issue numbers?
https://github.com/jneen/rouge/issues/494
https://gitlab.com/gitlab-org/gitlab-ce/issues/13818 (*.tpl can't in general map to Smarty)
https://gitlab.com/gitlab-org/gitlab-ce/issues/13615 (in cases we don't have a language and mis-identify it, users could map to 'text' to turn off highlighting)
## Screenshots (if relevant)
## Does this MR meet the acceptance criteria?
- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [x] API support added (N/A)
- [x] Tests
- [x] Added for this feature/bug
- [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
See merge request !4606
|
| | |
|
| | |
|
| | |
|
|/ |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
unnecessary queries
`notes_with_associations` are used for `participant` declarations, but `Participable`
only really cares about the target entity project, and not the participants
projects.
`notes_with_associations` are also used in `Commit::has_been_reverted?` which
employs the reference extractor of the commit, so no references to the notes
projects are made there (`Mentionable::all_references` cares only about the
`author` and other `attr_mentionable`). A paralel situation occurs on
`Issue::referenced_merge_requests`.
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
There are several changes to this module:
1. The use of an explicit stack in Participable#participants
2. Proc behaviour has been changed
3. Batch permissions checking
== Explicit Stack
Participable#participants no longer uses recursion to process "self" and
all child objects, instead it uses an Array and processes objects in
breadth-first order. This allows us to for example create a single
Gitlab::ReferenceExtractor instance and pass this to any Procs. Re-using
a ReferenceExtractor removes the need for running potentially many SQL
queries every time a Proc is called on a new object.
== Proc Behaviour Changed
Previously a Proc in Participable was expected to return an Array of
User instances. This has been changed and instead it's now expected that
a Proc modifies the Gitlab::ReferenceExtractor passed to it. The return
value of the Proc is ignored.
== Permissions Checking
The method Participable#participants uses
Ability.users_that_can_read_project to check if the returned users have
access to the project of "self" _without_ running multiple SQL queries
for every user.
|
|
|
|
|
|
|
|
|
| |
In 8278b763d96ef10c6494409b18b7eb541463af29 the default behaviour of annotation
has changes, which was causing a lot of noise in diffs. We decided in #17382
that it is better to get rid of the whole annotate gem, and instead let people
look at schema.rb for the columns in a table.
Fixes: #17382
|
| |
|
| |
|
|
|
|
|
| |
These methods are called quite often in loops so by memoizing their
output we can reduce timings a bit.
|
|
|
|
|
| |
We can just use "arel_table" in these cases instead of
"SomeClass.arel_table".
|
|
|
|
|
|
| |
Previously this used a regular LIKE which is case-sensitive on
PostgreSQL. This ensures that for both PostgreSQL and MySQL the
searching is case-insensitive similar to searching for projects.
|
|
|
|
|
|
| |
This was removed from the interface in
https://github.com/gitlabhq/gitlabhq/pull/6027 but its implementation
lingered around for two years.
|
| |
|
| |
|
|
|
|
| |
Fixes #2384.
|