| Commit message (Collapse) | Author | Age | Files | Lines |
|---|
| |
|
|
| |
prevent confusion with destroy_protected_branch
|
| | |
|
| |
|
|
|
| |
The query becomes a lot simpler if we can check the branch name as
well instead of having to load all branch names.
|
| | |
|
| | |
|
| | |
|
| | |
|
| |
|
|
| |
with StrongMemoize
|
| | |
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
I realized where the N+1 queries were actually coming from
project.protected_branches, but how come we cannot preload this,
or cache this at all?
Then I found that this is somehow a Rails limitation. What we're
doing before, eventually come to:
project.protected_branches.matching
But why it's not cached? (project.protected_branches.loaded? is always
false) It's because matching is a class method, which is called on
the proxy. In this case, Rails cannot cache the result. I don't know
if this is possible to implement or not, because clearly this would
require some tricks to implement class methods on associations.
So instead, we could just pass project.protected_branches to
ProtectedRef.matching, then it would work regularly.
With this change, there's no more N+1 queries.
|
| |
|
|
| |
Also make sure pipeline would also check against tag as well
|
| |\
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
* upstream/master: (25 commits)
Remove unneeded asserts and add tests for inactive RequestStore
Rename the methods to make it fit with current name
Follow feedback on the merge request
Make sure it checks against the tag only when it's a tag
Renamed Gitaly services
fix transient rspec failure due to Poll.js race condition
Refactor variables initialization in dropzone_input.js
cache the cache key...
avoid #respond_to? in Cache.id_for
cache DeclarativePolicy.class_for at the class level
Update 9.3-to-9.4.md
fix padding on filtered search dropdown. Styles should only apply to li in list
Cache Note#notable for commits and fix tests
Add changelog entry
Update the comments for the new functionality
Use RequestStoreWrap for Commit#author
Skip dead jobs queue for web hooks and project services
Add RequestStoreWrap to cache via RequestStore
Don't track cached queries in Gitlab::PerformanceBar::PeekQueryTracker
Add changelog entry
...
|
| | | |
|
| | | |
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
I don't like the idea of `RequestStore` at all, because it's just a
global state which shouldn't be used at all. But we have a number of
places calling `ProtectedBranch.protected?` and `ProtectedTag.protected?`
in a loop for the same user, project, and ref whenever we're checking
against if the jobs for a given pipeline is accessible for a given user.
This means we're effectively making N queries for the same thing over
and over.
To properly fix this, we need to change how we check the permission,
and that could be a huge work. To solve this quickly, adding a cache
layer for the given request would be quite simple to do.
We're already doing this in Commit#author, and this is extending that
idea and make it generalized.
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
So that we cache the result of UserAccess#can_push_or_merge_to_branch?
in RequestStore, avoiding querying ProtectedBranch over and over for
the list of pipelines (i.e. in PipelineSerializer)
I don't think this is ideal because I don't like the idea of
RequestStore in general, but this is the easiest way to cache it
without changing the architecture. In the future we should cache
more explicitly rather than this kind of global store.
|
| |/
|
|
|
| |
updating builds and updating pipelines. We check against
being able to merge or push if the ref is protected.
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| | |
|
| |
|
|
|
|
| |
if user has either push or merge permissions
+ Change log entry for fix to creating a branch matching a wildcard fails
|
| |
|
|
| |
checking user permission.
|
| |
|
|
|
|
|
|
|
| |
1. Change a few incorrect `access_level` to `access_levels.first` that
were missed in e805a64.
2. `API::Entities` can iterate over all access levels instead of just
the first one. This makes no difference to CE, and makes it more compatible
with EE.
|
| |
|
|
| |
aswell
|
| |
|
|
|
|
|
|
|
|
|
| |
1. The crux of this change is in `UserAccess`, which looks through all
the access levels, asking each if the user has access to push/merge
for the current project.
2. Update the `protected_branches` factory to create access levels as
necessary.
3. Fix and augment `user_access` and `git_access` specs.
|
| |
|
|
|
|
|
|
| |
This reverts commit 530f5158e297f3cde27f3566cfe13bad74ba3b50.
See !4892.
Signed-off-by: Rémy Coutable <remy@rymai.me>
|
| |
|
|
|
| |
This reverts commit 9ca633eb4c62231e4ddff5466c723cf8e2bdb25d, reversing
changes made to fb229bbf7970ba908962b837b270adf56f14098f.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
1. Don't use case statements for dispatch anymore. This leads to a lot
of duplication, and makes the logic harder to follow.
2. Remove duplicated logic.
- For example, the `can_push_to_branch?` exists, but we also have a
different way of checking the same condition within `change_access_check`.
- This kind of duplication is removed, and the `can_push_to_branch?`
method is used in both places.
3. Move checks returning true/false to `UserAccess`.
- All public methods in `GitAccess` now return an instance of
`GitAccessStatus`. Previously, some methods would return
true/false as well, which was confusing.
- It makes sense for these kinds of checks to be at the level of a
user, so the `UserAccess` class was repurposed for this. The prior
`UserAccess.allowed?` classmethod is converted into an instance
method.
- All external uses of these checks have been migrated to use the
`UserAccess` class
4. Move the "change_access_check" into a separate class.
- Create the `GitAccess::ChangeAccessCheck` class to run these
checks, which are quite substantial.
- `ChangeAccessCheck` returns an instance of `GitAccessStatus` as
well.
5. Break out the boolean logic in `ChangeAccessCheck` into `if/else`
chains - this seems more readable.
6. I can understand that this might look like overkill for !4892, but I
think this is a good opportunity to clean it up.
- http://martinfowler.com/bliki/OpportunisticRefactoring.html
|
| | |
|
| | |
|
| |
|
|
|
| |
This changes the number of LDAP calls when users access GitLab via
Git-over-SSH or the API. LDAP check results are cached for 1 hour.
|
| |
|
