Read about our upcoming Code of Conduct on this issue

  1. 12 Jan, 2018 1 commit
  2. 19 Jul, 2017 2 commits
    • Lin Jen-Shin's avatar
      Fix tests and fine tweak permission error message · fd63aaae8f8f
      Lin Jen-Shin authored
    • Lin Jen-Shin's avatar
      Eliminate N+1 queries on checking different protected refs · f759d31155b0
      Lin Jen-Shin authored
      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:
      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.
  3. 18 Jul, 2017 3 commits
  4. 17 Jul, 2017 1 commit
    • Lin Jen-Shin's avatar
      Add RequestStoreWrap to cache via RequestStore · b25e59b2d782
      Lin Jen-Shin authored
      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.
  5. 04 Jul, 2017 1 commit
    • Lin Jen-Shin's avatar
      Introduce Gitlab::Cache::RequestStoreWrap · dbffead222f2
      Lin Jen-Shin authored
      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.
  6. 03 Jul, 2017 1 commit
  7. 08 May, 2017 1 commit
  8. 28 Apr, 2017 1 commit
  9. 04 Apr, 2017 2 commits
  10. 03 Apr, 2017 2 commits
  11. 31 Mar, 2017 1 commit
  12. 09 Mar, 2017 2 commits
  13. 13 Jan, 2017 1 commit
  14. 16 Nov, 2016 1 commit
  15. 16 Aug, 2016 1 commit
  16. 04 Aug, 2016 1 commit
  17. 29 Jul, 2016 1 commit
    • Timothy Andrew's avatar
      Enforce "No One Can Push" during git operations. · 6c4eb21ce643
      Timothy Andrew authored
      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
      3. Fix and augment `user_access` and `git_access` specs.
  18. 18 Jul, 2016 1 commit
  19. 13 Jul, 2016 2 commits
    • Robert Speicher's avatar
      Revert "Merge branch '18193-developers-can-merge' into 'master' · df7fb9e8d12f
      Robert Speicher authored
      This reverts commit 72baf9a18669, reversing
      changes made to cf491007802b.
    • Timothy Andrew's avatar
      Refactor `Gitlab::GitAccess` · a0fe55c0e0b9
      Timothy Andrew authored
      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
          - 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
      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.
  20. 10 Mar, 2016 1 commit
  21. 09 Mar, 2016 1 commit
  22. 06 Aug, 2014 1 commit
  23. 15 May, 2014 1 commit