Merge remote-tracking branch 'upstream/master' into 30634-protected-pipeline

* 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
  ...

Gemfile

@@ -237,7 +237,6 @@ gem 'webpack-rails', '~> 0.9.10'
 gem 'rack-proxy', '~> 0.6.0'
 
 gem 'sass-rails', '~> 5.0.6'
-gem 'coffee-rails', '~> 4.1.0'
 gem 'uglifier', '~> 2.7.2'
 
 gem 'addressable', '~> 2.3.8'

Gemfile.lock

@@ -122,13 +122,6 @@ GEM
     coderay (1.1.1)
     coercible (1.0.0)
       descendants_tracker (~> 0.0.1)
-    coffee-rails (4.1.1)
-      coffee-script (>= 2.2.0)
-      railties (>= 4.0.0, < 5.1.x)
-    coffee-script (2.4.1)
-      coffee-script-source
-      execjs
-    coffee-script-source (1.10.0)
     colorize (0.7.7)
     concurrent-ruby (1.0.5)
     concurrent-ruby-ext (1.0.5)
@@ -938,7 +931,6 @@ DEPENDENCIES
   charlock_holmes (~> 0.7.3)
   chronic (~> 0.10.2)
   chronic_duration (~> 0.10.6)
-  coffee-rails (~> 4.1.0)
   concurrent-ruby (~> 1.0.5)
   connection_pool (~> 2.0)
   creole (~> 0.5.0)

app/assets/javascripts/dropzone_input.js

@@ -5,21 +5,28 @@ import './preview_markdown';
 
 window.DropzoneInput = (function() {
   function DropzoneInput(form) {
-    var updateAttachingMessage, $attachingFileMessage, $mdArea, $attachButton, $cancelButton, $retryLink, $uploadingErrorContainer, $uploadingErrorMessage, $uploadProgress, $uploadingProgressContainer, appendToTextArea, btnAlert, child, closeAlertMessage, closeSpinner, divHover, divSpinner, dropzone, $formDropzone, formTextarea, getFilename, handlePaste, iconPaperclip, iconSpinner, insertToTextArea, isImage, maxFileSize, pasteText, uploadsPath, showError, showSpinner, uploadFile, addFileToForm;
     Dropzone.autoDiscover = false;
-    divHover = '<div class="div-dropzone-hover"></div>';
-    iconPaperclip = '<i class="fa fa-paperclip div-dropzone-icon"></i>';
-    $attachButton = form.find('.button-attach-file');
-    $attachingFileMessage = form.find('.attaching-file-message');
-    $cancelButton = form.find('.button-cancel-uploading-files');
-    $retryLink = form.find('.retry-uploading-link');
-    $uploadProgress = form.find('.uploading-progress');
-    $uploadingErrorContainer = form.find('.uploading-error-container');
-    $uploadingErrorMessage = form.find('.uploading-error-message');
-    $uploadingProgressContainer = form.find('.uploading-progress-container');
-    uploadsPath = window.uploads_path || null;
-    maxFileSize = gon.max_file_size || 10;
-    formTextarea = form.find('.js-gfm-input');
+    const divHover = '<div class="div-dropzone-hover"></div>';
+    const iconPaperclip = '<i class="fa fa-paperclip div-dropzone-icon"></i>';
+    const $attachButton = form.find('.button-attach-file');
+    const $attachingFileMessage = form.find('.attaching-file-message');
+    const $cancelButton = form.find('.button-cancel-uploading-files');
+    const $retryLink = form.find('.retry-uploading-link');
+    const $uploadProgress = form.find('.uploading-progress');
+    const $uploadingErrorContainer = form.find('.uploading-error-container');
+    const $uploadingErrorMessage = form.find('.uploading-error-message');
+    const $uploadingProgressContainer = form.find('.uploading-progress-container');
+    const uploadsPath = window.uploads_path || null;
+    const maxFileSize = gon.max_file_size || 10;
+    const formTextarea = form.find('.js-gfm-input');
+    let handlePaste;
+    let pasteText;
+    let addFileToForm;
+    let updateAttachingMessage;
+    let isImage;
+    let getFilename;
+    let uploadFile;
+
     formTextarea.wrap('<div class="div-dropzone"></div>');
     formTextarea.on('paste', (function(_this) {
       return function(event) {
@@ -28,16 +35,16 @@ window.DropzoneInput = (function() {
     })(this));
 
     // Add dropzone area to the form.
-    $mdArea = formTextarea.closest('.md-area');
+    const $mdArea = formTextarea.closest('.md-area');
     form.setupMarkdownPreview();
-    $formDropzone = form.find('.div-dropzone');
+    const $formDropzone = form.find('.div-dropzone');
     $formDropzone.parent().addClass('div-dropzone-wrapper');
     $formDropzone.append(divHover);
     $formDropzone.find('.div-dropzone-hover').append(iconPaperclip);
 
     if (!uploadsPath) return;
 
-    dropzone = $formDropzone.dropzone({
+    const dropzone = $formDropzone.dropzone({
       url: uploadsPath,
       dictDefaultMessage: '',
       clickable: true,
@@ -117,7 +124,7 @@ window.DropzoneInput = (function() {
       }
     });
 
-    child = $(dropzone[0]).children('textarea');
+    const child = $(dropzone[0]).children('textarea');
 
     // removeAllFiles(true) stops uploading files (if any)
     // and remove them from dropzone files queue.
@@ -214,6 +221,35 @@ window.DropzoneInput = (function() {
       return value.first();
     };
 
+    const showSpinner = function(e) {
+      return $uploadingProgressContainer.removeClass('hide');
+    };
+
+    const closeSpinner = function() {
+      return $uploadingProgressContainer.addClass('hide');
+    };
+
+    const showError = function(message) {
+      $uploadingErrorContainer.removeClass('hide');
+      $uploadingErrorMessage.html(message);
+    };
+
+    const closeAlertMessage = function() {
+      return form.find('.div-dropzone-alert').alert('close');
+    };
+
+    const insertToTextArea = function(filename, url) {
+      return $(child).val(function(index, val) {
+        return val.replace(`{{${filename}}}`, url);
+      });
+    };
+
+    const appendToTextArea = function(url) {
+      return $(child).val(function(index, val) {
+        return val + url + "\n";
+      });
+    };
+
     uploadFile = function(item, filename) {
       var formData;
       formData = new FormData();
@@ -262,35 +298,6 @@ window.DropzoneInput = (function() {
       messageContainer.text(attachingMessage);
     };
 
-    insertToTextArea = function(filename, url) {
-      return $(child).val(function(index, val) {
-        return val.replace(`{{${filename}}}`, url);
-      });
-    };
-
-    appendToTextArea = function(url) {
-      return $(child).val(function(index, val) {
-        return val + url + "\n";
-      });
-    };
-
-    showSpinner = function(e) {
-      return $uploadingProgressContainer.removeClass('hide');
-    };
-
-    closeSpinner = function() {
-      return $uploadingProgressContainer.addClass('hide');
-    };
-
-    showError = function(message) {
-      $uploadingErrorContainer.removeClass('hide');
-      $uploadingErrorMessage.html(message);
-    };
-
-    closeAlertMessage = function() {
-      return form.find('.div-dropzone-alert').alert('close');
-    };
-
     form.find('.markdown-selector').click(function(e) {
       e.preventDefault();
       $(this).closest('.gfm-form').find('.div-dropzone').click();

app/assets/javascripts/lib/utils/http_status.js

@@ -3,6 +3,7 @@
  */
 
 export default {
+  ABORTED: 0,
   NO_CONTENT: 204,
   OK: 200,
 };

app/assets/javascripts/lib/utils/poll.js

@@ -81,6 +81,9 @@ export default class Poll {
       })
       .catch((error) => {
         notificationCallback(false);
+        if (error.status === httpStatusCodes.ABORTED) {
+          return;
+        }
         errorCallback(error);
       });
   }

app/assets/stylesheets/framework/dropdowns.scss

@@ -297,18 +297,12 @@
 }
 
 .droplab-dropdown {
-  .description {
-    display: inline-block;
-    white-space: normal;
-    margin-left: 5px;
-  }
-
   .dropdown-toggle > i {
     pointer-events: none;
   }
 
-  li {
-    padding: $gl-btn-padding $gl-btn-padding 2px;
+  .dropdown-menu li {
+    padding: $gl-btn-padding;
     cursor: pointer;
 
     > a,
@@ -344,9 +338,25 @@
       visibility: visible;
     }
 
+    &.divider {
+      margin: 0 8px;
+      padding: 0;
+      border-top: $gray-darkest;
+    }
+
     .icon {
       visibility: hidden;
     }
+
+    .description {
+      display: inline-block;
+      white-space: normal;
+      margin-left: 5px;
+
+      p {
+        margin-bottom: 0;
+      }
+    }
   }
 
   .icon {
@@ -354,12 +364,6 @@
     vertical-align: top;
     padding-top: 2px;
   }
-
-  .divider {
-    margin: 0 8px;
-    padding: 0;
-    border-top: $gray-darkest;
-  }
 }
 
 .droplab-dropdown .dropdown-menu,

app/assets/stylesheets/framework/filters.scss

@@ -275,7 +275,7 @@
 }
 
 .filtered-search-input-dropdown-menu {
-  max-height: 215px;
+  max-height: 225px;
   max-width: 280px;
   overflow: auto;
 
@@ -382,10 +382,6 @@
   }
 }
 
-.dropdown-menu .filter-dropdown-item {
-  padding: 0;
-}
-
 @media (max-width: $screen-xs-max) {
   .issues-details-filters {
     padding: 0 0 10px;
@@ -435,6 +431,7 @@
 
     .fa {
       width: 15px;
+      line-height: $line-height-base;
     }
 
     .dropdown-label-box {

app/assets/stylesheets/pages/issuable.scss

@@ -813,8 +813,6 @@
   }
 
   .description {
-    margin-bottom: 10px;
-
     .text {
       margin: 0;
     }

app/models/commit.rb

 class Commit
   extend ActiveModel::Naming
+  extend Gitlab::Cache::RequestCache
 
   include ActiveModel::Conversion
   include Noteable
@@ -169,19 +170,9 @@ class Commit
   end
 
   def author
-    if RequestStore.active?
-      key = "commit_author:#{author_email.downcase}"
-      # nil is a valid value since no author may exist in the system
-      if RequestStore.store.key?(key)
-        @author = RequestStore.store[key]
-      else
-        @author = find_author_by_any_email
-        RequestStore.store[key] = @author
-      end
-    else
-      @author ||= find_author_by_any_email
-    end
+    User.find_by_any_email(author_email.downcase)
   end
+  request_cache(:author) { author_email.downcase }
 
   def committer
     @committer ||= User.find_by_any_email(committer_email.downcase)
@@ -322,7 +313,7 @@ class Commit
 
   def raw_diffs(*args)
     if Gitlab::GitalyClient.feature_enabled?(:commit_raw_diffs)
-      Gitlab::GitalyClient::Commit.new(project.repository).diff_from_parent(self, *args)
+      Gitlab::GitalyClient::CommitService.new(project.repository).diff_from_parent(self, *args)
     else
       raw.diffs(*args)
     end
@@ -331,7 +322,7 @@ class Commit
   def raw_deltas
     @deltas ||= Gitlab::GitalyClient.migrate(:commit_deltas) do |is_enabled|
       if is_enabled
-        Gitlab::GitalyClient::Commit.new(project.repository).commit_deltas(self)
+        Gitlab::GitalyClient::CommitService.new(project.repository).commit_deltas(self)
       else
         raw.deltas
       end
@@ -368,10 +359,6 @@ class Commit
     end
   end
 
-  def find_author_by_any_email
-    User.find_by_any_email(author_email.downcase)
-  end
-
   def repo_changes
     changes = { added: [], modified: [], removed: [] }
 

app/models/note.rb

@@ -190,7 +190,7 @@ class Note < ActiveRecord::Base
   # override to return commits, which are not active record
   def noteable
     if for_commit?
-      project.commit(commit_id)
+      @commit ||= project.commit(commit_id)
     else
       super
     end

app/policies/ci/build_policy.rb

 module Ci
   class BuildPolicy < CommitStatusPolicy
     condition(:user_cannot_update) do
-      !::Gitlab::UserAccess
-        .new(@user, project: @subject.project)
-        .can_push_or_merge_to_branch?(@subject.ref)
+      access = ::Gitlab::UserAccess.new(@user, project: @subject.project)
+
+      if @subject.tag?
+        !access.can_create_tag?(@subject.ref)
+      else
+        !access.can_push_or_merge_to_branch?(@subject.ref)
+      end
     end
 
     rule { user_cannot_update }.prevent :update_build

app/views/layouts/nav/_new_group_sidebar.html.haml

@@ -55,7 +55,22 @@
         %span
           Members
     - if current_user && can?(current_user, :admin_group, @group)
-      = nav_link(path: %w[groups#projects groups#edit]) do
+      = nav_link(path: %w[groups#projects groups#edit ci_cd#show]) do
         = link_to edit_group_path(@group), title: 'Settings' do
           %span
             Settings
+        %ul.sidebar-sub-level-items
+          = nav_link(path: 'groups#edit') do
+            = link_to edit_group_path(@group), title: 'General' do
+              %span
+                General
+
+          = nav_link(path: 'groups#projects') do
+            = link_to projects_group_path(@group), title: 'Projects' do
+              %span
+                Projects
+
+          = nav_link(controller: :ci_cd) do
+            = link_to group_settings_ci_cd_path(@group), title: 'Pipelines' do
+              %span
+                Pipelines

app/views/layouts/nav/_new_project_sidebar.html.haml

@@ -165,7 +165,7 @@
             Snippets
 
     - if project_nav_tab? :settings
-      = nav_link(path: %w[projects#edit members#show integrations#show services#edit repository#show ci_cd#show pages#show]) do
+      = nav_link(path: %w[projects#edit project_members#index integrations#show services#edit repository#show ci_cd#show pages#show]) do
         = link_to edit_project_path(@project), title: 'Settings', class: 'shortcuts-tree' do
           %span
             Settings
@@ -177,8 +177,8 @@
               = link_to edit_project_path(@project), title: 'General' do
                 %span
                   General
-          = nav_link(controller: :members) do
-            = link_to project_settings_members_path(@project), title: 'Members' do
+          = nav_link(controller: :project_members) do
+            = link_to project_project_members_path(@project), title: 'Members' do
               %span
                 Members
           - if can_edit

app/workers/project_service_worker.rb

@@ -2,6 +2,8 @@ class ProjectServiceWorker
   include Sidekiq::Worker
   include DedicatedSidekiqQueue
 
+  sidekiq_options dead: false
+
   def perform(hook_id, data)
     data = data.with_indifferent_access
     Service.find(hook_id).execute(data)

app/workers/web_hook_worker.rb

@@ -2,7 +2,7 @@ class WebHookWorker
   include Sidekiq::Worker
   include DedicatedSidekiqQueue
 
-  sidekiq_options retry: 4
+  sidekiq_options retry: 4, dead: false
 
   def perform(hook_id, data, hook_name)
     hook = WebHook.find(hook_id)

changelogs/unreleased/29901-refactor-initialization-dropzone_input-js.yml

+---
+title: refactor initializations in dropzone_input.js
+merge_request: 12768
+author: Brandon Everett

changelogs/unreleased/31571-don-t-let-webhooks-jobs-go-to-the-dead-jobs-queue.yml

+---
+title: Prevent web hook and project service background jobs from going to the dead
+  jobs queue
+merge_request:
+author:

changelogs/unreleased/34831-remove-coffee-rails-gem.yml

+---
+title: Remove coffee-rails gem
+merge_request:
+author: Takuya Noguchi

changelogs/unreleased/34927-protect-manual-actions-on-tags.yml

+---
+title: Protect manual actions against protected tag too
+merge_request: 12908
+author:

changelogs/unreleased/35225-transient-poll.yml

+---
+title: fix transient js error in rspec tests
+merge_request:
+author:

changelogs/unreleased/request-store-wrap.yml

+---
+title: Add RequestCache which makes caching with RequestStore easier
+merge_request: 12920
+author:

config/initializers/gettext_rails_i18n_patch.rb

@@ -33,7 +33,6 @@ module GettextI18nRailsJs
         [
           ".js",
           ".jsx",
-          ".coffee",
           ".vue"
         ].include? ::File.extname(file)
       end

doc/update/9.3-to-9.4.md

@@ -157,8 +157,7 @@ configuration file may contain syntax errors. The block name
 file, should be `[[storage]]` instead.
 
 ```shell
-cd /home/git/gitaly
-sudo -u git -H editor config.toml
+sudo -u git -H sed -i.pre-9.4 's/\[\[storages\]\]/[[storage]]/' /home/git/gitaly/config.toml
 ```
 
 #### Compile Gitaly

lib/api/internal.rb

@@ -150,7 +150,7 @@ module API
         #
         # begin
         #   repository = wiki? ? project.wiki.repository : project.repository
-        #   Gitlab::GitalyClient::Notifications.new(repository.raw_repository).post_receive
+        #   Gitlab::GitalyClient::NotificationService.new(repository.raw_repository).post_receive
         # rescue GRPC::Unavailable => e
         #   render_api_error!(e, 500)
         # end

lib/declarative_policy.rb

@@ -8,7 +8,12 @@ require_dependency 'declarative_policy/step'
 
 require_dependency 'declarative_policy/base'
 
+require 'thread'
+
 module DeclarativePolicy
+  CLASS_CACHE_MUTEX = Mutex.new
+  CLASS_CACHE_IVAR = :@__DeclarativePolicy_CLASS_CACHE
+
   class << self
     def policy_for(user, subject, opts = {})
       cache = opts[:cache] || {}
@@ -23,7 +28,36 @@ module DeclarativePolicy
 
       subject = find_delegate(subject)
 
-      subject.class.ancestors.each do |klass|
+      class_for_class(subject.class)
+    end
+
+    private
+
+    # This method is heavily cached because there are a lot of anonymous
+    # modules in play in a typical rails app, and #name performs quite
+    # slowly for anonymous classes and modules.
+    #
+    # See https://bugs.ruby-lang.org/issues/11119
+    #
+    # if the above bug is resolved, this caching could likely be removed.
+    def class_for_class(subject_class)
+      unless subject_class.instance_variable_defined?(CLASS_CACHE_IVAR)
+        CLASS_CACHE_MUTEX.synchronize do
+          # re-check in case of a race
+          break if subject_class.instance_variable_defined?(CLASS_CACHE_IVAR)
+
+          policy_class = compute_class_for_class(subject_class)
+          subject_class.instance_variable_set(CLASS_CACHE_IVAR, policy_class)
+        end
+      end
+
+      policy_class = subject_class.instance_variable_get(CLASS_CACHE_IVAR)
+      raise "no policy for #{subject.class.name}" if policy_class.nil?
+      policy_class
+    end
+
+    def compute_class_for_class(subject_class)
+      subject_class.ancestors.each do |klass|
         next unless klass.name
 
         begin
@@ -37,12 +71,8 @@ module DeclarativePolicy
           nil
         end
       end
-
-      raise "no policy for #{subject.class.name}"
     end
 
-    private
-
     def find_delegate(subject)
       seen = Set.new
 

lib/declarative_policy/cache.rb

@@ -21,11 +21,14 @@ module DeclarativePolicy
       private
 
       def id_for(obj)
-        if obj.respond_to?(:id) && obj.id
-          obj.id.to_s
-        else
-          "##{obj.object_id}"
-        end
+        id =
+          begin
+            obj.id
+          rescue NoMethodError
+            nil
+          end
+
+        id || "##{obj.object_id}"
       end
     end
   end

lib/declarative_policy/condition.rb

@@ -82,13 +82,14 @@ module DeclarativePolicy
     # depending on the scope, we may cache only by the user or only by
     # the subject, resulting in sharing across different policy objects.
     def cache_key
-      case @condition.scope
-      when :normal  then "/dp/condition/#{@condition.key}/#{user_key},#{subject_key}"
-      when :user    then "/dp/condition/#{@condition.key}/#{user_key}"
-      when :subject then "/dp/condition/#{@condition.key}/#{subject_key}"
-      when :global  then "/dp/condition/#{@condition.key}"
-      else raise 'invalid scope'
-      end
+      @cache_key ||=
+        case @condition.scope
+        when :normal  then "/dp/condition/#{@condition.key}/#{user_key},#{subject_key}"
+        when :user    then "/dp/condition/#{@condition.key}/#{user_key}"
+        when :subject then "/dp/condition/#{@condition.key}/#{subject_key}"
+        when :global  then "/dp/condition/#{@condition.key}"
+        else raise 'invalid scope'
+        end
     end
 
     def user_key

lib/gitlab/cache/request_cache.rb

+module Gitlab
+  module Cache
+    # This module provides a simple way to cache values in RequestStore,
+    # and the cache key would be based on the class name, method name,
+    # optionally customized instance level values, optionally customized
+    # method level values, and optional method arguments.
+    #
+    # A simple example:
+    #
+    # class UserAccess
+    #   extend Gitlab::Cache::RequestCache
+    #
+    #   request_cache_key do
+    #     [user&.id, project&.id]
+    #   end
+    #
+    #   request_cache def can_push_to_branch?(ref)
+    #     # ...
+    #   end
+    # end
+    #
+    # This way, the result of `can_push_to_branch?` would be cached in
+    # `RequestStore.store` based on the cache key. If RequestStore is not
+    # currently active, then it would be stored in a hash saved in an
+    # instance variable, so the cache logic would be the same.
+    # Here's another example using customized method level values:
+    #
+    # class Commit
+    #   extend Gitlab::Cache::RequestCache
+    #
+    #   def author
+    #     User.find_by_any_email(author_email.downcase)
+    #   end
+    #   request_cache(:author) { author_email.downcase }
+    # end
+    #
+    # So that we could have different strategies for different methods
+    #
+    module RequestCache
+      def self.extended(klass)
+        return if klass < self
+
+        extension = Module.new
+        klass.const_set(:RequestCacheExtension, extension)
+        klass.prepend(extension)
+      end
+
+      def request_cache_key(&block)
+        if block_given?
+          @request_cache_key = block
+        else
+          @request_cache_key
+        end
+      end
+
+      def request_cache(method_name, &method_key_block)
+        const_get(:RequestCacheExtension).module_eval do
+          cache_key_method_name = "#{method_name}_cache_key"
+
+          define_method(method_name) do |*args|
+            store =
+              if RequestStore.active?
+                RequestStore.store
+              else
+                ivar_name = # ! and ? cannot be used as ivar name
+                  "@cache_#{method_name.to_s.tr('!?', "\u2605\u2606")}"
+
+                instance_variable_get(ivar_name) ||
+                  instance_variable_set(ivar_name, {})
+              end
+
+            key = __send__(cache_key_method_name, args)
+
+            store.fetch(key) { store[key] = super(*args) }
+          end
+
+          define_method(cache_key_method_name) do |args|
+            klass = self.class
+
+            instance_key = instance_exec(&klass.request_cache_key) if
+              klass.request_cache_key
+
+            method_key = instance_exec(&method_key_block) if method_key_block
+
+            [klass.name, method_name, *instance_key, *method_key, *args]
+              .join(':')
+          end
+
+          private cache_key_method_name
+        end
+      end
+    end
+  end
+end

lib/gitlab/git/blob.rb

@@ -29,7 +29,7 @@ module Gitlab
           path = path.sub(/\A\/*/, '')
           path = '/' if path.empty?
           name = File.basename(path)
-          entry = Gitlab::GitalyClient::Commit.new(repository).tree_entry(sha, path, MAX_DATA_DISPLAY_SIZE)
+          entry = Gitlab::GitalyClient::CommitService.new(repository).tree_entry(sha, path, MAX_DATA_DISPLAY_SIZE)
           return unless entry
 
           case entry.type
@@ -87,10 +87,10 @@ module Gitlab
         def raw(repository, sha)
           Gitlab::GitalyClient.migrate(:git_blob_raw) do |is_enabled|
             if is_enabled
-              Gitlab::GitalyClient::Blob.new(repository).get_blob(oid: sha, limit: MAX_DATA_DISPLAY_SIZE)
+              Gitlab::GitalyClient::BlobService.new(repository).get_blob(oid: sha, limit: MAX_DATA_DISPLAY_SIZE)
             else
               blob = repository.lookup(sha)
-    
+
               new(
                 id: blob.oid,
                 size: blob.size,
@@ -182,7 +182,7 @@ module Gitlab
         Gitlab::GitalyClient.migrate(:git_blob_load_all_data) do |is_enabled|
           @data = begin
             if is_enabled
-              Gitlab::GitalyClient::Blob.new(repository).get_blob(oid: id, limit: -1).data
+              Gitlab::GitalyClient::BlobService.new(repository).get_blob(oid: id, limit: -1).data
             else
               repository.lookup(id).content
             end

lib/gitlab/git/repository.rb

@@ -1106,11 +1106,11 @@ module Gitlab
       end
 
       def gitaly_ref_client
-        @gitaly_ref_client ||= Gitlab::GitalyClient::Ref.new(self)
+        @gitaly_ref_client ||= Gitlab::GitalyClient::RefService.new(self)
       end
 
       def gitaly_commit_client
-        @gitaly_commit_client ||= Gitlab::GitalyClient::Commit.new(self)
+        @gitaly_commit_client ||= Gitlab::GitalyClient::CommitService.new(self)
       end
 
       def gitaly_migrate(method, &block)

lib/gitlab/gitaly_client/blob.rb → lib/gitlab/gitaly_client/blob_service.rb

 module Gitlab
   module GitalyClient
-    class Blob
+    class BlobService
       def initialize(repository)
         @gitaly_repo = repository.gitaly_repository
       end
-      
+
       def get_blob(oid:, limit:)
         request = Gitaly::GetBlobRequest.new(
           repository: @gitaly_repo,

lib/gitlab/gitaly_client/commit.rb → lib/gitlab/gitaly_client/commit_service.rb

 module Gitlab
   module GitalyClient
-    class Commit
+    class CommitService
       # The ID of empty tree.
       # See http://stackoverflow.com/a/40884093/1856239 and https://github.com/git/git/blob/3ad8b5bf26362ac67c9020bf8c30eee54a84f56d/cache.h#L1011-L1012
       EMPTY_TREE_ID = '4b825dc642cb6eb9a060e54bf8d69288fbee4904'.freeze
@@ -17,20 +17,20 @@ module Gitlab
           child_id: child_id
         )
 
-        GitalyClient.call(@repository.storage, :commit, :commit_is_ancestor, request).value
+        GitalyClient.call(@repository.storage, :commit_service, :commit_is_ancestor, request).value
       end
 
       def diff_from_parent(commit, options = {})
         request_params = commit_diff_request_params(commit, options)
         request_params[:ignore_whitespace_change] = options.fetch(:ignore_whitespace_change, false)
         request = Gitaly::CommitDiffRequest.new(request_params)
-        response = GitalyClient.call(@repository.storage, :diff, :commit_diff, request)
+        response = GitalyClient.call(@repository.storage, :diff_service, :commit_diff, request)
         Gitlab::Git::DiffCollection.new(GitalyClient::DiffStitcher.new(response), options)
       end
 
       def commit_deltas(commit)
         request = Gitaly::CommitDeltaRequest.new(commit_diff_request_params(commit))
-        response = GitalyClient.call(@repository.storage, :diff, :commit_delta, request)
+        response = GitalyClient.call(@repository.storage, :diff_service, :commit_delta, request)
         response.flat_map do |msg|
           msg.deltas.map { |d| Gitlab::Git::Diff.new(d) }
         end
@@ -44,7 +44,7 @@ module Gitlab
           limit: limit.to_i
         )
 
-        response = GitalyClient.call(@repository.storage, :commit, :tree_entry, request)
+        response = GitalyClient.call(@repository.storage, :commit_service, :tree_entry, request)
         entry = response.first
         return unless entry.oid.present?
 

lib/gitlab/gitaly_client/notifications.rb → lib/gitlab/gitaly_client/notification_service.rb

 module Gitlab
   module GitalyClient
-    class Notifications
+    class NotificationService
       # 'repository' is a Gitlab::Git::Repository
       def initialize(repository)
         @gitaly_repo = repository.gitaly_repository
@@ -10,7 +10,7 @@ module Gitlab
       def post_receive
         GitalyClient.call(
           @storage,
-          :notifications,
+          :notification_service,
           :post_receive,
           Gitaly::PostReceiveRequest.new(repository: @gitaly_repo)
         )

lib/gitlab/gitaly_client/ref.rb → lib/gitlab/gitaly_client/ref_service.rb

 module Gitlab
   module GitalyClient
-    class Ref
+    class RefService
       include Gitlab::EncodingHelper
 
       # 'repository' is a Gitlab::Git::Repository
@@ -12,19 +12,19 @@ module Gitlab
 
       def default_branch_name
         request = Gitaly::FindDefaultBranchNameRequest.new(repository: @gitaly_repo)
-        response = GitalyClient.call(@storage, :ref, :find_default_branch_name, request)
+        response = GitalyClient.call(@storage, :ref_service, :find_default_branch_name, request)
         Gitlab::Git.branch_name(response.name)
       end
 
       def branch_names
         request = Gitaly::FindAllBranchNamesRequest.new(repository: @gitaly_repo)
-        response = GitalyClient.call(@storage, :ref, :find_all_branch_names, request)
+        response = GitalyClient.call(@storage, :ref_service, :find_all_branch_names, request)
         consume_refs_response(response) { |name| Gitlab::Git.branch_name(name) }
       end
 
       def tag_names
         request = Gitaly::FindAllTagNamesRequest.new(repository: @gitaly_repo)
-        response = GitalyClient.call(@storage, :ref, :find_all_tag_names, request)
+        response = GitalyClient.call(@storage, :ref_service, :find_all_tag_names, request)
         consume_refs_response(response) { |name| Gitlab::Git.tag_name(name) }
       end
 
@@ -34,7 +34,7 @@ module Gitlab
           commit_id: commit_id,
           prefix: ref_prefix
         )
-        encode!(GitalyClient.call(@storage, :ref, :find_ref_name, request).name.dup)
+        encode!(GitalyClient.call(@storage, :ref_service, :find_ref_name, request).name.dup)
       end
 
       def count_tag_names
@@ -48,7 +48,7 @@ module Gitlab
       def local_branches(sort_by: nil)
         request = Gitaly::FindLocalBranchesRequest.new(repository: @gitaly_repo)
         request.sort_by = sort_by_param(sort_by) if sort_by
-        response = GitalyClient.call(@storage, :ref, :find_local_branches, request)
+        response = GitalyClient.call(@storage, :ref_service, :find_local_branches, request)
         consume_branches_response(response)
       end
 

lib/gitlab/performance_bar/peek_query_tracker.rb

 # Inspired by https://github.com/peek/peek-pg/blob/master/lib/peek/views/pg.rb
+# PEEK_DB_CLIENT is a constant set in config/initializers/peek.rb
 module Gitlab
   module PerformanceBar
     module PeekQueryTracker
@@ -23,7 +24,13 @@ module Gitlab
 
         subscribe('sql.active_record') do |_, start, finish, _, data|
           if RequestStore.active? && RequestStore.store[:peek_enabled]
-            track_query(data[:sql].strip, data[:binds], start, finish)
+            # data[:cached] is only available starting from Rails 5.1.0
+            # https://github.com/rails/rails/blob/v5.1.0/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb#L113
+            # Before that, data[:name] was set to 'CACHE'
+            # https://github.com/rails/rails/blob/v4.2.9/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb#L80
+            unless data.fetch(:cached, data[:name] == 'CACHE')
+              track_query(data[:sql].strip, data[:binds], start, finish)
+            end
           end
         end
       end

lib/gitlab/user_access.rb

 module Gitlab
   class UserAccess
-    extend Gitlab::Cache::RequestStoreWrap
+    extend Gitlab::Cache::RequestCache
 
-    request_store_wrap_key do
+    request_cache_key do
       [user&.id, project&.id]
     end
 
@@ -34,7 +34,7 @@ module Gitlab
       true
     end
 
-    def can_create_tag?(ref)
+    request_cache def can_create_tag?(ref)
       return false unless can_access_git?
 
       if ProtectedTag.protected?(project, ref)
@@ -44,7 +44,7 @@ module Gitlab
       end
     end
 
-    def can_delete_branch?(ref)
+    request_cache def can_delete_branch?(ref)
       return false unless can_access_git?
 
       if ProtectedBranch.protected?(project, ref)
@@ -58,7 +58,7 @@ module Gitlab
       can_push_to_branch?(ref) || can_merge_to_branch?(ref)
     end
 
-    request_store_wrap def can_push_to_branch?(ref)
+    request_cache def can_push_to_branch?(ref)
       return false unless can_access_git?
 
       if ProtectedBranch.protected?(project, ref)
@@ -70,7 +70,7 @@ module Gitlab
       end
     end
 
-    request_store_wrap def can_merge_to_branch?(ref)
+    request_cache def can_merge_to_branch?(ref)
       return false unless can_access_git?
 
       if ProtectedBranch.protected?(project, ref)

lib/tasks/gettext.rake

@@ -5,7 +5,7 @@ namespace :gettext do
   # See: https://github.com/grosser/gettext_i18n_rails#customizing-list-of-translatable-files
   def files_to_translate
     folders = %W(app lib config #{locale_path}).join(',')
-    exts = %w(rb erb haml slim rhtml js jsx vue coffee handlebars hbs mustache).join(',')
+    exts = %w(rb erb haml slim rhtml js jsx vue handlebars hbs mustache).join(',')
 
     Dir.glob(
       "{#{folders}}/**/*.{#{exts}}"

spec/factories/commits.rb

@@ -4,14 +4,19 @@ FactoryGirl.define do
   factory :commit do
     git_commit RepoHelpers.sample_commit
     project factory: :empty_project
-    author { build(:author) }
 
     initialize_with do
       new(git_commit, project)
     end
 
+    after(:build) do |commit|
+      allow(commit).to receive(:author).and_return build(:author)
+    end
+
     trait :without_author do
-      author nil
+      after(:build) do |commit|
+        allow(commit).to receive(:author).and_return nil
+      end
     end
   end
 end

spec/features/participants_autocomplete_spec.rb

@@ -54,7 +54,8 @@ feature 'Member autocomplete', :js do
     let(:note) { create(:note_on_commit, project: project, commit_id: project.commit.id) }
 
     before do
-      allow_any_instance_of(Commit).to receive(:author).and_return(author)
+      allow(User).to receive(:find_by_any_email)
+        .with(noteable.author_email.downcase).and_return(author)
 
       visit project_commit_path(project, noteable)
     end

spec/javascripts/lib/utils/poll_spec.js

@@ -25,23 +25,28 @@ function mockServiceCall(service, response, shouldFail = false) {
 
 describe('Poll', () => {
   const service = jasmine.createSpyObj('service', ['fetch']);
-  const callbacks = jasmine.createSpyObj('callbacks', ['success', 'error']);
+  const callbacks = jasmine.createSpyObj('callbacks', ['success', 'error', 'notification']);
+
+  function setup() {
+    return new Poll({
+      resource: service,
+      method: 'fetch',
+      successCallback: callbacks.success,
+      errorCallback: callbacks.error,
+      notificationCallback: callbacks.notification,
+    }).makeRequest();
+  }
 
   afterEach(() => {
     callbacks.success.calls.reset();
     callbacks.error.calls.reset();
+    callbacks.notification.calls.reset();
     service.fetch.calls.reset();
   });
 
   it('calls the success callback when no header for interval is provided', (done) => {
     mockServiceCall(service, { status: 200 });
-
-    new Poll({
-      resource: service,
-      method: 'fetch',
-      successCallback: callbacks.success,
-      errorCallback: callbacks.error,
-    }).makeRequest();
+    setup();
 
     waitForAllCallsToFinish(service, 1, () => {
       expect(callbacks.success).toHaveBeenCalled();
@@ -51,15 +56,9 @@ describe('Poll', () => {
     });
   });
 
-  it('calls the error callback whe the http request returns an error', (done) => {
+  it('calls the error callback when the http request returns an error', (done) => {
     mockServiceCall(service, { status: 500 }, true);
-
-    new Poll({
-      resource: service,
-      method: 'fetch',
-      successCallback: callbacks.success,
-      errorCallback: callbacks.error,
-    }).makeRequest();
+    setup();
 
     waitForAllCallsToFinish(service, 1, () => {
       expect(callbacks.success).not.toHaveBeenCalled();
@@ -69,15 +68,22 @@ describe('Poll', () => {
     });
   });
 
+  it('skips the error callback when request is aborted', (done) => {
+    mockServiceCall(service, { status: 0 }, true);
+    setup();
+
+    waitForAllCallsToFinish(service, 1, () => {
+      expect(callbacks.success).not.toHaveBeenCalled();
+      expect(callbacks.error).not.toHaveBeenCalled();
+      expect(callbacks.notification).toHaveBeenCalled();
+
+      done();
+    });
+  });
+
   it('should call the success callback when the interval header is -1', (done) => {
     mockServiceCall(service, { status: 200, headers: { 'poll-interval': -1 } });
-
-    new Poll({
-      resource: service,
-      method: 'fetch',
-      successCallback: callbacks.success,
-      errorCallback: callbacks.error,
-    }).makeRequest().then(() => {
+    setup().then(() => {
       expect(callbacks.success).toHaveBeenCalled();
       expect(callbacks.error).not.toHaveBeenCalled();
 

spec/lib/gitlab/cache/request_cache_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Cache::RequestCache do
+  let(:klass) do
+    Class.new do
+      extend Gitlab::Cache::RequestCache
+
+      attr_accessor :id, :name, :result, :extra
+
+      def self.name
+        'ExpensiveAlgorithm'
+      end
+
+      def initialize(id, name, result, extra = nil)
+        self.id = id
+        self.name = name
+        self.result = result
+        self.extra = nil
+      end
+
+      request_cache def compute(arg)
+        result << arg
+      end
+
+      request_cache def repute(arg)
+        result << arg
+      end
+
+      def dispute(arg)
+        result << arg
+      end
+      request_cache(:dispute) { extra }
+    end
+  end
+
+  let(:algorithm) { klass.new('id', 'name', []) }
+
+  shared_examples 'cache for the same instance' do
+    it 'does not compute twice for the same argument' do
+      algorithm.compute(true)
+      result = algorithm.compute(true)
+
+      expect(result).to eq([true])
+    end
+
+    it 'computes twice for the different argument' do
+      algorithm.compute(true)
+      result = algorithm.compute(false)
+
+      expect(result).to eq([true, false])
+    end
+
+    it 'computes twice for the different class name' do
+      algorithm.compute(true)
+      allow(klass).to receive(:name).and_return('CheapAlgo')
+      result = algorithm.compute(true)
+
+      expect(result).to eq([true, true])
+    end
+
+    it 'computes twice for the different method' do
+      algorithm.compute(true)
+      result = algorithm.repute(true)
+
+      expect(result).to eq([true, true])
+    end
+
+    context 'when request_cache_key is provided' do
+      before do
+        klass.request_cache_key do
+          [id, name]
+        end
+      end
+
+      it 'computes twice for the different keys, id' do
+        algorithm.compute(true)
+        algorithm.id = 'ad'
+        result = algorithm.compute(true)
+
+        expect(result).to eq([true, true])
+      end
+
+      it 'computes twice for the different keys, name' do
+        algorithm.compute(true)
+        algorithm.name = 'same'
+        result = algorithm.compute(true)
+
+        expect(result).to eq([true, true])
+      end
+
+      it 'uses extra method cache key if provided' do
+        algorithm.dispute(true) # miss
+        algorithm.extra = true
+        algorithm.dispute(true) # miss
+        result = algorithm.dispute(true) # hit
+
+        expect(result).to eq([true, true])
+      end
+    end
+  end
+
+  context 'when RequestStore is active', :request_store do
+    it_behaves_like 'cache for the same instance'
+
+    it 'computes once for different instances when keys are the same' do
+      algorithm.compute(true)
+      result = klass.new('id', 'name', algorithm.result).compute(true)
+
+      expect(result).to eq([true])
+    end
+
+    it 'computes twice if RequestStore starts over' do
+      algorithm.compute(true)
+      RequestStore.end!
+      RequestStore.clear!
+      RequestStore.begin!
+      result = algorithm.compute(true)
+
+      expect(result).to eq([true, true])
+    end
+  end
+
+  context 'when RequestStore is inactive' do
+    it_behaves_like 'cache for the same instance'
+
+    it 'computes twice for different instances even if keys are the same' do
+      algorithm.compute(true)
+      result = klass.new('id', 'name', algorithm.result).compute(true)
+
+      expect(result).to eq([true, true])
+    end
+  end
+end

spec/lib/gitlab/git/repository_spec.rb

@@ -45,11 +45,11 @@ describe Gitlab::Git::Repository, seed_helper: true do
     end
 
     it 'gets the branch name from GitalyClient' do
-      expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:default_branch_name)
+      expect_any_instance_of(Gitlab::GitalyClient::RefService).to receive(:default_branch_name)
       repository.root_ref
     end
 
-    it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::Ref, :default_branch_name do
+    it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::RefService, :default_branch_name do
       subject { repository.root_ref }
     end
   end
@@ -132,11 +132,11 @@ describe Gitlab::Git::Repository, seed_helper: true do
     it { is_expected.not_to include("branch-from-space") }
 
     it 'gets the branch names from GitalyClient' do
-      expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:branch_names)
+      expect_any_instance_of(Gitlab::GitalyClient::RefService).to receive(:branch_names)
       subject
     end
 
-    it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::Ref, :branch_names
+    it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::RefService, :branch_names
   end
 
   describe '#tag_names' do
@@ -160,11 +160,11 @@ describe Gitlab::Git::Repository, seed_helper: true do
     it { is_expected.not_to include("v5.0.0") }
 
     it 'gets the tag names from GitalyClient' do
-      expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:tag_names)
+      expect_any_instance_of(Gitlab::GitalyClient::RefService).to receive(:tag_names)
       subject
     end
 
-    it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::Ref, :tag_names
+    it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::RefService, :tag_names
   end
 
   shared_examples 'archive check' do |extenstion|
@@ -368,7 +368,7 @@ describe Gitlab::Git::Repository, seed_helper: true do
 
     context 'when Gitaly commit_count feature is enabled' do
       it_behaves_like 'counting commits'
-      it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::Commit, :commit_count do
+      it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::CommitService, :commit_count do
         subject { repository.commit_count('master') }
       end
     end
@@ -1225,12 +1225,12 @@ describe Gitlab::Git::Repository, seed_helper: true do
     end
 
     it 'gets the branches from GitalyClient' do
-      expect_any_instance_of(Gitlab::GitalyClient::Ref).to receive(:local_branches)
+      expect_any_instance_of(Gitlab::GitalyClient::RefService).to receive(:local_branches)
         .and_return([])
       @repo.local_branches
     end
 
-    it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::Ref, :local_branches do
+    it_behaves_like 'wrapping gRPC errors', Gitlab::GitalyClient::RefService, :local_branches do
       subject { @repo.local_branches }
     end
   end

spec/lib/gitlab/gitaly_client/commit_spec.rb → spec/lib/gitlab/gitaly_client/commit_service_spec.rb

 require 'spec_helper'
 
-describe Gitlab::GitalyClient::Commit do
-  let(:diff_stub) { double('Gitaly::Diff::Stub') }
+describe Gitlab::GitalyClient::CommitService do
+  let(:diff_stub) { double('Gitaly::DiffService::Stub') }
   let(:project) { create(:project, :repository) }
   let(:repository) { project.repository }
   let(:repository_message) { repository.gitaly_repository }
@@ -16,7 +16,7 @@ describe Gitlab::GitalyClient::Commit do
           right_commit_id: commit.id
         )
 
-        expect_any_instance_of(Gitaly::Diff::Stub).to receive(:commit_diff).with(request, kind_of(Hash))
+        expect_any_instance_of(Gitaly::DiffService::Stub).to receive(:commit_diff).with(request, kind_of(Hash))
 
         described_class.new(repository).diff_from_parent(commit)
       end
@@ -31,7 +31,7 @@ describe Gitlab::GitalyClient::Commit do
           right_commit_id: initial_commit.id
         )
 
-        expect_any_instance_of(Gitaly::Diff::Stub).to receive(:commit_diff).with(request, kind_of(Hash))
+        expect_any_instance_of(Gitaly::DiffService::Stub).to receive(:commit_diff).with(request, kind_of(Hash))
 
         described_class.new(repository).diff_from_parent(initial_commit)
       end
@@ -61,7 +61,7 @@ describe Gitlab::GitalyClient::Commit do
           right_commit_id: commit.id
         )
 
-        expect_any_instance_of(Gitaly::Diff::Stub).to receive(:commit_delta).with(request, kind_of(Hash)).and_return([])
+        expect_any_instance_of(Gitaly::DiffService::Stub).to receive(:commit_delta).with(request, kind_of(Hash)).and_return([])
 
         described_class.new(repository).commit_deltas(commit)
       end
@@ -76,7 +76,7 @@ describe Gitlab::GitalyClient::Commit do
           right_commit_id: initial_commit.id
         )
 
-        expect_any_instance_of(Gitaly::Diff::Stub).to receive(:commit_delta).with(request, kind_of(Hash)).and_return([])
+        expect_any_instance_of(Gitaly::DiffService::Stub).to receive(:commit_delta).with(request, kind_of(Hash)).and_return([])
 
         described_class.new(repository).commit_deltas(initial_commit)
       end

spec/lib/gitlab/gitaly_client/notifications_spec.rb → spec/lib/gitlab/gitaly_client/notification_service_spec.rb

 require 'spec_helper'
 
-describe Gitlab::GitalyClient::Notifications do
+describe Gitlab::GitalyClient::NotificationService do
   describe '#post_receive' do
     let(:project) { create(:empty_project) }
     let(:storage_name) { project.repository_storage }
@@ -8,7 +8,7 @@ describe Gitlab::GitalyClient::Notifications do
     subject { described_class.new(project.repository) }
 
     it 'sends a post_receive message' do
-      expect_any_instance_of(Gitaly::Notifications::Stub)
+      expect_any_instance_of(Gitaly::NotificationService::Stub)
         .to receive(:post_receive).with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash))
 
       subject.post_receive

spec/lib/gitlab/gitaly_client/ref_spec.rb → spec/lib/gitlab/gitaly_client/ref_service_spec.rb

 require 'spec_helper'
 
-describe Gitlab::GitalyClient::Ref do
+describe Gitlab::GitalyClient::RefService do
   let(:project) { create(:empty_project) }
   let(:storage_name) { project.repository_storage }
   let(:relative_path) { project.path_with_namespace + '.git' }
@@ -8,7 +8,7 @@ describe Gitlab::GitalyClient::Ref do
 
   describe '#branch_names' do
     it 'sends a find_all_branch_names message' do
-      expect_any_instance_of(Gitaly::Ref::Stub)
+      expect_any_instance_of(Gitaly::RefService::Stub)
         .to receive(:find_all_branch_names)
         .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash))
         .and_return([])
@@ -19,7 +19,7 @@ describe Gitlab::GitalyClient::Ref do
 
   describe '#tag_names' do
     it 'sends a find_all_tag_names message' do
-      expect_any_instance_of(Gitaly::Ref::Stub)
+      expect_any_instance_of(Gitaly::RefService::Stub)
         .to receive(:find_all_tag_names)
         .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash))
         .and_return([])
@@ -30,7 +30,7 @@ describe Gitlab::GitalyClient::Ref do
 
   describe '#default_branch_name' do
     it 'sends a find_default_branch_name message' do
-      expect_any_instance_of(Gitaly::Ref::Stub)
+      expect_any_instance_of(Gitaly::RefService::Stub)
         .to receive(:find_default_branch_name)
         .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash))
         .and_return(double(name: 'foo'))
@@ -41,7 +41,7 @@ describe Gitlab::GitalyClient::Ref do
 
   describe '#local_branches' do
     it 'sends a find_local_branches message' do
-      expect_any_instance_of(Gitaly::Ref::Stub)
+      expect_any_instance_of(Gitaly::RefService::Stub)
         .to receive(:find_local_branches)
         .with(gitaly_request_with_path(storage_name, relative_path), kind_of(Hash))
         .and_return([])
@@ -50,7 +50,7 @@ describe Gitlab::GitalyClient::Ref do
     end
 
     it 'parses and sends the sort parameter' do
-      expect_any_instance_of(Gitaly::Ref::Stub)
+      expect_any_instance_of(Gitaly::RefService::Stub)
         .to receive(:find_local_branches)
         .with(gitaly_request_with_params(sort_by: :UPDATED_DESC), kind_of(Hash))
         .and_return([])
@@ -59,7 +59,7 @@ describe Gitlab::GitalyClient::Ref do
     end
 
     it 'translates known mismatches on sort param values' do
-      expect_any_instance_of(Gitaly::Ref::Stub)
+      expect_any_instance_of(Gitaly::RefService::Stub)
         .to receive(:find_local_branches)
         .with(gitaly_request_with_params(sort_by: :NAME), kind_of(Hash))
         .and_return([])

spec/lib/gitlab/gitaly_client_spec.rb

@@ -16,9 +16,9 @@ describe Gitlab::GitalyClient, lib: true, skip_gitaly_mock: true do
           'default' => { 'gitaly_address' => address }
         })
 
-        expect(Gitaly::Commit::Stub).to receive(:new).with(address, any_args)
+        expect(Gitaly::CommitService::Stub).to receive(:new).with(address, any_args)
 
-        described_class.stub(:commit, 'default')
+        described_class.stub(:commit_service, 'default')
       end
     end
 
@@ -31,9 +31,9 @@ describe Gitlab::GitalyClient, lib: true, skip_gitaly_mock: true do
           'default' => { 'gitaly_address' => prefixed_address }
         })
 
-        expect(Gitaly::Commit::Stub).to receive(:new).with(address, any_args)
+        expect(Gitaly::CommitService::Stub).to receive(:new).with(address, any_args)
 
-        described_class.stub(:commit, 'default')
+        described_class.stub(:commit_service, 'default')
       end
     end
   end

spec/models/commit_spec.rb

@@ -19,17 +19,15 @@ describe Commit, models: true do
       expect(commit.author).to eq(user)
     end
 
-    it 'caches the author' do
-      allow(RequestStore).to receive(:active?).and_return(true)
+    it 'caches the author', :request_store do
       user = create(:user, email: commit.author_email)
-      expect_any_instance_of(Commit).to receive(:find_author_by_any_email).and_call_original
+      expect(User).to receive(:find_by_any_email).and_call_original
 
       expect(commit.author).to eq(user)
-      key = "commit_author:#{commit.author_email}"
+      key = "Commit:author:#{commit.author_email.downcase}"
       expect(RequestStore.store[key]).to eq(user)
 
       expect(commit.author).to eq(user)
-      RequestStore.store.clear
     end
   end
 

spec/policies/ci/build_policy_spec.rb

@@ -114,14 +114,6 @@ describe Ci::BuildPolicy, :models do
         end
       end
 
-      context 'when developers can push to the branch' do
-        let(:branch_policy) { :developers_can_push }
-
-        it 'includes ability to update build' do
-          expect(policy).to be_allowed :update_build
-        end
-      end
-
       context 'when developers can push to the branch' do
         let(:branch_policy) { :developers_can_merge }
 

spec/requests/api/internal_spec.rb

@@ -594,10 +594,10 @@ describe API::Internal do
   #   end
   #
   #   it "calls the Gitaly client with the project's repository" do
-  #     expect(Gitlab::GitalyClient::Notifications).
+  #     expect(Gitlab::GitalyClient::NotificationService).
   #       to receive(:new).with(gitlab_git_repository_with(path: project.repository.path)).
   #       and_call_original
-  #     expect_any_instance_of(Gitlab::GitalyClient::Notifications).
+  #     expect_any_instance_of(Gitlab::GitalyClient::NotificationService).
   #       to receive(:post_receive)
   #
   #     post api("/internal/notify_post_receive"), valid_params
@@ -606,10 +606,10 @@ describe API::Internal do
   #   end
   #
   #   it "calls the Gitaly client with the wiki's repository if it's a wiki" do
-  #     expect(Gitlab::GitalyClient::Notifications).
+  #     expect(Gitlab::GitalyClient::NotificationService).
   #       to receive(:new).with(gitlab_git_repository_with(path: project.wiki.repository.path)).
   #       and_call_original
-  #     expect_any_instance_of(Gitlab::GitalyClient::Notifications).
+  #     expect_any_instance_of(Gitlab::GitalyClient::NotificationService).
   #       to receive(:post_receive)
   #
   #     post api("/internal/notify_post_receive"), valid_wiki_params
@@ -618,7 +618,7 @@ describe API::Internal do
   #   end
   #
   #   it "returns 500 if the gitaly call fails" do
-  #     expect_any_instance_of(Gitlab::GitalyClient::Notifications).
+  #     expect_any_instance_of(Gitlab::GitalyClient::NotificationService).
   #       to receive(:post_receive).and_raise(GRPC::Unavailable)
   #
   #     post api("/internal/notify_post_receive"), valid_params
@@ -636,10 +636,10 @@ describe API::Internal do
   #     end
   #
   #     it "calls the Gitaly client with the project's repository" do
-  #       expect(Gitlab::GitalyClient::Notifications).
+  #       expect(Gitlab::GitalyClient::NotificationService).
   #         to receive(:new).with(gitlab_git_repository_with(path: project.repository.path)).
   #         and_call_original
-  #       expect_any_instance_of(Gitlab::GitalyClient::Notifications).
+  #       expect_any_instance_of(Gitlab::GitalyClient::NotificationService).
   #         to receive(:post_receive)
   #
   #       post api("/internal/notify_post_receive"), valid_params
@@ -648,10 +648,10 @@ describe API::Internal do
   #     end
   #
   #     it "calls the Gitaly client with the wiki's repository if it's a wiki" do
-  #       expect(Gitlab::GitalyClient::Notifications).
+  #       expect(Gitlab::GitalyClient::NotificationService).
   #         to receive(:new).with(gitlab_git_repository_with(path: project.wiki.repository.path)).
   #         and_call_original
-  #       expect_any_instance_of(Gitlab::GitalyClient::Notifications).
+  #       expect_any_instance_of(Gitlab::GitalyClient::NotificationService).
   #         to receive(:post_receive)
   #
   #       post api("/internal/notify_post_receive"), valid_wiki_params

spec/services/notification_service_spec.rb

@@ -383,7 +383,7 @@ describe NotificationService, services: true do
       before do
         build_team(note.project)
         reset_delivered_emails!
-        allow_any_instance_of(Commit).to receive(:author).and_return(@u_committer)
+        allow(note.noteable).to receive(:author).and_return(@u_committer)
         update_custom_notification(:new_note, @u_guest_custom, resource: project)
         update_custom_notification(:new_note, @u_custom_global)
       end