Merge branch 'ce-to-ee' into 'master'

CE Upstream - Monday

Closes gitaly#524

See merge request gitlab-org/gitlab-ee!3007

GITALY_SERVER_VERSION

-0.38.0
+0.40.0

app/assets/javascripts/awards_handler.js

@@ -24,6 +24,9 @@ const categoryLabelMap = {
   flags: 'Flags',
 };
 
+const IS_VISIBLE = 'is-visible';
+const IS_RENDERED = 'is-rendered';
+
 class AwardsHandler {
   constructor(emoji) {
     this.emoji = emoji;
@@ -51,7 +54,7 @@ class AwardsHandler {
       if (!$target.closest('.emoji-menu').length) {
         if ($('.emoji-menu').is(':visible')) {
           $('.js-add-award.is-active').removeClass('is-active');
-          $('.emoji-menu').removeClass('is-visible');
+          this.hideMenuElement($('.emoji-menu'));
         }
       }
     });
@@ -88,12 +91,12 @@ class AwardsHandler {
     if ($menu.length) {
       if ($menu.is('.is-visible')) {
         $addBtn.removeClass('is-active');
-        $menu.removeClass('is-visible');
+        this.hideMenuElement($menu);
         $('.js-emoji-menu-search').blur();
       } else {
         $addBtn.addClass('is-active');
         this.positionMenu($menu, $addBtn);
-        $menu.addClass('is-visible');
+        this.showMenuElement($menu);
         $('.js-emoji-menu-search').focus();
       }
     } else {
@@ -103,7 +106,7 @@ class AwardsHandler {
         $addBtn.removeClass('is-loading');
         this.positionMenu($createdMenu, $addBtn);
         return setTimeout(() => {
-          $createdMenu.addClass('is-visible');
+          this.showMenuElement($createdMenu);
           $('.js-emoji-menu-search').focus();
         }, 200);
       });
@@ -241,7 +244,8 @@ class AwardsHandler {
     if (isInIssuePage() && !isMainAwardsBlock) {
       const id = votesBlock.attr('id').replace('note_', '');
 
-      $('.emoji-menu').removeClass('is-visible');
+      this.hideMenuElement($('.emoji-menu'));
+
       $('.js-add-award.is-active').removeClass('is-active');
       const toggleAwardEvent = new CustomEvent('toggleAward', {
         detail: {
@@ -261,7 +265,8 @@ class AwardsHandler {
       return typeof callback === 'function' ? callback() : undefined;
     });
 
-    $('.emoji-menu').removeClass('is-visible');
+    this.hideMenuElement($('.emoji-menu'));
+
     return $('.js-add-award.is-active').removeClass('is-active');
   }
 
@@ -529,6 +534,33 @@ class AwardsHandler {
     return $matchingElements.closest('li').clone();
   }
 
+  /* showMenuElement and hideMenuElement are performance optimizations. We use
+   * opacity to show/hide the emoji menu, because we can animate it. But opacity
+   * leaves hidden elements in the render tree, which is unacceptable given the number
+   * of emoji elements in the emoji menu (5k+). To get the best of both worlds, we separately
+   * apply IS_RENDERED to add/remove the menu from the render tree and IS_VISIBLE to animate
+   * the menu being opened and closed. */
+
+  showMenuElement($emojiMenu) {
+    $emojiMenu.addClass(IS_RENDERED);
+
+    // enqueues animation as a microtask, so it begins ASAP once IS_RENDERED added
+    return Promise.resolve()
+      .then(() => $emojiMenu.addClass(IS_VISIBLE));
+  }
+
+  hideMenuElement($emojiMenu) {
+    $emojiMenu.on(transitionEndEventString, (e) => {
+      if (e.currentTarget === e.target) {
+        $emojiMenu
+          .removeClass(IS_RENDERED)
+          .off(transitionEndEventString);
+      }
+    });
+
+    $emojiMenu.removeClass(IS_VISIBLE);
+  }
+
   destroy() {
     this.eventListeners.forEach((entry) => {
       entry.element.off.call(entry.element, ...entry.args);

app/assets/javascripts/pipelines/components/pipelines_actions.vue

@@ -69,8 +69,7 @@
           @click="onClickAction(action.path)"
           :class="{ disabled: isActionDisabled(action) }"
           :disabled="isActionDisabled(action)">
-          <span v-html="playIconSvg"></span>
-          <span>{{action.name}}</span>
+          {{action.name}}
         </button>
       </li>
     </ul>

app/assets/javascripts/pipelines/components/pipelines_artifacts.vue

@@ -39,11 +39,7 @@
           rel="nofollow"
           download
           :href="artifact.path">
-          <i
-            class="fa fa-download"
-            aria-hidden="true">
-          </i>
-          <span>Download {{artifact.name}} artifacts</span>
+          Download {{artifact.name}} artifacts
         </a>
       </li>
     </ul>

app/assets/javascripts/vue_shared/components/table_pagination.vue

 <script>
+import { s__ } from '../../locale';
+
 const PAGINATION_UI_BUTTON_LIMIT = 4;
 const UI_LIMIT = 6;
 const SPREAD = '...';
-const PREV = 'Prev';
-const NEXT = 'Next';
-const FIRST = '« First';
-const LAST = 'Last »';
+const PREV = s__('Pagination|Prev');
+const NEXT = s__('Pagination|Next');
+const FIRST = s__('Pagination|« First');
+const LAST = s__('Pagination|Last »');
 
 export default {
   props: {

app/assets/stylesheets/framework/awards.scss

@@ -9,6 +9,7 @@
 }
 
 .emoji-menu {
+  display: none;
   position: absolute;
   top: 0;
   margin-top: 3px;
@@ -27,6 +28,10 @@
   transition: .3s cubic-bezier(.67, .06, .19, 1.44);
   transition-property: transform, opacity;
 
+  &.is-rendered {
+    display: block;
+  }
+
   &.is-aligned-right {
     transform-origin: 100% -45px;
   }

app/controllers/projects/tree_controller.rb

@@ -35,7 +35,10 @@ class Projects::TreeController < Projects::ApplicationController
       end
 
       format.json do
-        render json: TreeSerializer.new(project: @project, repository: @repository, ref: @ref).represent(@tree)
+        # n+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/38261
+        Gitlab::GitalyClient.allow_n_plus_1_calls do
+          render json: TreeSerializer.new(project: @project, repository: @repository, ref: @ref).represent(@tree)
+        end
       end
     end
   end

app/helpers/groups_helper.rb

@@ -131,7 +131,7 @@ module GroupsHelper
   end
 
   def default_help
-    s_("GroupSettings|This setting will be applied to all subgroups unless overridden by a group owner.")
+    s_("GroupSettings|This setting will be applied to all subgroups unless overridden by a group owner. Groups that already have access to the project will continue to have access unless removed manually.")
   end
 
   def ancestor_locked_but_you_can_override(group)

app/models/repository.rb

@@ -10,6 +10,7 @@ class Repository
   RESERVED_REFS_NAMES = %W[
     heads
     tags
+    replace
     #{REF_ENVIRONMENTS}
     #{REF_KEEP_AROUND}
     #{REF_ENVIRONMENTS}

app/views/projects/branches/index.html.haml

@@ -36,7 +36,9 @@
           = s_('Branches|Delete merged branches')
         = link_to new_project_branch_path(@project), class: 'btn btn-create' do
           = s_('Branches|New branch')
+
   = render 'projects/commits/mirror_status'
+
   - if @branches.any?
     %ul.content-list.all-branches
       - @branches.each do |branch|

app/views/projects/buttons/_download.html.haml

@@ -11,19 +11,15 @@
         #{ _('Source code') }
       %li
         = link_to archive_project_repository_path(project, ref: ref, format: 'zip'), rel: 'nofollow', download: '' do
-          %i.fa.fa-download
           %span=  _('Download zip')
       %li
         = link_to archive_project_repository_path(project, ref: ref, format: 'tar.gz'), rel: 'nofollow', download: '' do
-          %i.fa.fa-download
           %span= _('Download tar.gz')
       %li
         = link_to archive_project_repository_path(project, ref: ref, format: 'tar.bz2'), rel: 'nofollow', download: '' do
-          %i.fa.fa-download
           %span= _('Download tar.bz2')
       %li
         = link_to archive_project_repository_path(project, ref: ref, format: 'tar'), rel: 'nofollow', download: '' do
-          %i.fa.fa-download
           %span= _('Download tar')
 
       - if pipeline && pipeline.latest_builds_with_artifacts.any?
@@ -36,6 +32,5 @@
         - pipeline.latest_builds_with_artifacts.each do |job|
           %li
             = link_to latest_succeeded_project_artifacts_path(project, "#{ref}/download", job: job.name), rel: 'nofollow', download: '' do
-              %i.fa.fa-download
               %span
                 #{s_('DownloadArtifacts|Download')} '#{job.name}'

app/views/projects/buttons/_dropdown.html.haml

@@ -11,19 +11,16 @@
       - if can_create_issue
         %li
           = link_to new_project_issue_path(@project) do
-            = icon('exclamation-circle fw')
             #{ _('New issue') }
 
       - if merge_project
         %li
           = link_to project_new_merge_request_path(merge_project) do
-            = icon('tasks fw')
             #{ _('New merge request') }
 
       - if can_create_snippet
         %li
           = link_to new_project_snippet_path(@project) do
-            = icon('file-text-o fw')
             #{ _('New snippet') }
 
       - if can_create_issue || merge_project || can_create_snippet
@@ -32,20 +29,16 @@
       - if can?(current_user, :push_code, @project)
         %li
           = link_to project_new_blob_path(@project, @project.default_branch || 'master') do
-            = icon('file fw')
             #{ _('New file') }
         %li
           = link_to new_project_branch_path(@project) do
-            = icon('code-fork fw')
             #{ _('New branch') }
         %li
           = link_to new_project_tag_path(@project) do
-            = icon('tags fw')
             #{ _('New tag') }
       - elsif current_user && current_user.already_forked?(@project)
         %li
           = link_to project_new_blob_path(@project, @project.default_branch || 'master') do
-            = icon('file fw')
             #{ _('New file') }
       - elsif can?(current_user, :fork_project, @project)
         %li
@@ -55,5 +48,4 @@
           - fork_path = project_forks_path(@project, namespace_key:  current_user.namespace.id,
                                                                                   continue:       continue_params)
           = link_to fork_path, method: :post do
-            = icon('file fw')
             #{ _('New file') }

app/views/projects/tree/_old_tree_header.html.haml

@@ -20,15 +20,12 @@
             - if can_edit_tree?
               %li
                 = link_to project_new_blob_path(@project, @id) do
-                  = icon('pencil fw')
                   #{ _('New file') }
               %li
                 = link_to '#modal-upload-blob', { 'data-target' => '#modal-upload-blob', 'data-toggle' => 'modal' } do
-                  = icon('file fw')
                   #{ _('Upload file') }
               %li
                 = link_to '#modal-create-new-dir', { 'data-target' => '#modal-create-new-dir', 'data-toggle' => 'modal' } do
-                  = icon('folder fw')
                   #{ _('New directory') }
             - elsif can?(current_user, :fork_project, @project)
               %li
@@ -38,7 +35,6 @@
                 - fork_path = project_forks_path(@project, namespace_key:  current_user.namespace.id,
                                                                                         continue:       continue_params)
                 = link_to fork_path, method: :post do
-                  = icon('pencil fw')
                   #{ _('New file') }
               %li
                 - continue_params = { to:         request.fullpath,
@@ -47,7 +43,6 @@
                 - fork_path = project_forks_path(@project, namespace_key:  current_user.namespace.id,
                                                                                         continue:       continue_params)
                 = link_to fork_path, method: :post do
-                  = icon('file fw')
                   #{ _('Upload file') }
               %li
                 - continue_params = { to:         request.fullpath,
@@ -56,15 +51,12 @@
                 - fork_path = project_forks_path(@project, namespace_key:  current_user.namespace.id,
                                                                                         continue:       continue_params)
                 = link_to fork_path, method: :post do
-                  = icon('folder fw')
                   #{ _('New directory') }
 
             %li.divider
             %li
               = link_to new_project_branch_path(@project) do
-                = icon('code-fork fw')
                 #{ _('New branch') }
             %li
               = link_to new_project_tag_path(@project) do
-                = icon('tags fw')
                 #{ _('New tag') }

changelogs/unreleased/36549-circuit-breaker-handles-missing-storages.yml

+---
+title: Allow the git circuit breaker to correctly handle missing repository storages
+merge_request: 14417
+author:
+type: fixed

changelogs/unreleased/38197-fix-ImapAuthenticationCheck.yml

+---
+title: Fix `rake gitlab:incoming_email:check` and make it report the actual error
+merge_request: 14423
+author:
+type: fixed

changelogs/unreleased/38234-reserve-refs-replace.yml

+---
+title: Also reserve refs/replace after importing a project
+merge_request: 14436
+author:
+type: fixed

changelogs/unreleased/ee-add-project-repository-storages-index.yml

+---
+title: Index projects on repository storage
+merge_request: 14414
+author:
+type: other

changelogs/unreleased/replace_project_shortcuts-feature.yml

+---
+title: Replace the 'project/shortcuts.feature' spinach test with an rspec analog
+merge_request: 14431
+author: Vitaliy @blackst0ne Klachkov
+type: other

config/initializers/1_settings.rb

@@ -578,9 +578,7 @@ Settings.backup['upload']['storage_class'] ||= nil
 # Git
 #
 Settings['git'] ||= Settingslogic.new({})
-Settings.git['max_size']  ||= 20971520 # 20.megabytes
-Settings.git['bin_path']  ||= '/usr/bin/git'
-Settings.git['timeout']   ||= 10
+Settings.git['bin_path'] ||= '/usr/bin/git'
 
 # Important: keep the satellites.path setting until GitLab 9.0 at
 # least. This setting is fed to 'rm -rf' in

db/migrate/20170828135939_migrate_user_external_mail_data.rb

@@ -33,7 +33,7 @@ class MigrateUserExternalMailData < ActiveRecord::Migration
           SELECT true
           FROM user_synced_attributes_metadata
           WHERE user_id = users.id
-          AND provider = users.email_provider
+          AND provider = users.email_provider OR (provider IS NULL AND users.email_provider IS NULL)
         )
         AND id BETWEEN #{start_id} AND #{end_id}
       EOF

db/migrate/20170921115009_add_project_repository_storage_index.rb

+class AddProjectRepositoryStorageIndex < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index(*index_spec) unless index_exists?(*index_spec)
+  end
+
+  def down
+    remove_concurrent_index(*index_spec) if index_exists?(*index_spec)
+  end
+
+  def index_spec
+    [:projects, :repository_storage]
+  end
+end

db/post_migrate/20170828170502_post_deploy_migrate_user_external_mail_data.rb

@@ -33,7 +33,7 @@ class PostDeployMigrateUserExternalMailData < ActiveRecord::Migration
           SELECT true
           FROM user_synced_attributes_metadata
           WHERE user_id = users.id
-          AND provider = users.email_provider
+          AND provider = users.email_provider OR (provider IS NULL AND users.email_provider IS NULL)
         )
         AND id BETWEEN #{start_id} AND #{end_id}
       EOF

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20170918223303) do
+ActiveRecord::Schema.define(version: 20170921115009) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -1528,6 +1528,7 @@ ActiveRecord::Schema.define(version: 20170918223303) do
   add_index "projects", ["path"], name: "index_projects_on_path", using: :btree
   add_index "projects", ["path"], name: "index_projects_on_path_trigram", using: :gin, opclasses: {"path"=>"gin_trgm_ops"}
   add_index "projects", ["pending_delete"], name: "index_projects_on_pending_delete", using: :btree
+  add_index "projects", ["repository_storage"], name: "index_projects_on_repository_storage", using: :btree
   add_index "projects", ["runners_token"], name: "index_projects_on_runners_token", using: :btree
   add_index "projects", ["star_count"], name: "index_projects_on_star_count", using: :btree
   add_index "projects", ["visibility_level"], name: "index_projects_on_visibility_level", using: :btree

doc/development/ux_guide/basics.md

@@ -33,7 +33,7 @@ This is the typeface used for code blocks and references to commits, branches, a
 
 ## Icons
 
-GitLab has a strong, unique personality. When you look at any screen, you should know immediately know that it is GitLab. 
+GitLab has a strong, unique personality. When you look at any screen, you should know immediately that it is GitLab. 
 Iconography is a powerful visual cue to the user and is a great way for us to reflect our particular sense of style.
 
 - **Standard size:** 16px * 16px

doc/user/markdown.md

@@ -596,6 +596,30 @@ See the documentation for HTML::Pipeline's [SanitizationFilter](http://www.rubyd
   <dd>Does *not* work **very** well. Use HTML <em>tags</em>.</dd>
 </dl>
 
+#### Details and Summary
+
+Content can be collapsed using HTML's [`<details>`](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/details) and [`<summary>`](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/summary) tags. This is especially useful for collapsing long logs so they take up less screen space.
+
+<p>
+<details>
+<summary>Click me to collapse/fold.</summary>
+These details will remain hidden until expanded.
+
+<pre><code>PASTE LOGS HERE</code></pre>
+</details>
+</p>
+
+**Note:** Unfortunately Markdown is not supported inside these tags, as described by the [markdown specification](https://daringfireball.net/projects/markdown/syntax#html). You can work around this by using HTML, for example you can use `<pre><code>` tags instead of [code fences](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/user/markdown.md#code-and-syntax-highlighting).
+
+```html
+<details>
+<summary>Click me to collapse/fold.</summary>
+These details will remain hidden until expanded.
+
+<pre><code>PASTE LOGS HERE</code></pre>
+</details>
+```
+
 ### Horizontal Rule
 
 ```

features/project/shortcuts.feature

-@dashboard
-Feature: Project Shortcuts
-  Background:
-    Given I sign in as a user
-    And I own a project
-    And I visit my project's commits page
-
-  @javascript
-  Scenario: Navigate to files tab
-    Given I press "g" and "f"
-    Then the active main tab should be Repository
-    Then the active sub tab should be Files
-
-  @javascript
-  Scenario: Navigate to commits tab
-    Given I visit my project's files page
-    Given I press "g" and "c"
-    Then the active main tab should be Repository
-    Then the active sub tab should be Commits
-
-  @javascript
-  Scenario: Navigate to graph tab
-    Given I press "g" and "n"
-    Then the active sub tab should be Graph
-    And the active main tab should be Repository
-
-  @javascript
-  Scenario: Navigate to repository charts tab
-    Given I press "g" and "d"
-    Then the active sub tab should be Charts
-    And the active main tab should be Repository
-
-  @javascript
-  Scenario: Navigate to issues tab
-    Given I press "g" and "i"
-    Then the active main tab should be Issues
-
-  @javascript
-  Scenario: Navigate to merge requests tab
-    Given I press "g" and "m"
-    Then the active main tab should be Merge Requests
-
-  @javascript
-  Scenario: Navigate to snippets tab
-    Given I press "g" and "s"
-    Then the active main tab should be Snippets
-
-  @javascript
-  Scenario: Navigate to wiki tab
-    Given I press "g" and "w"
-    Then the active main tab should be Wiki
-
-  @javascript
-  Scenario: Navigate to project home
-    Given I press "g" and "p"
-    Then the active sub tab should be Home
-    And the active main tab should be Project
-
-  @javascript
-  Scenario: Navigate to project feed
-    Given I press "g" and "e"
-    Then the active sub tab should be Activity
-    And the active main tab should be Project

features/steps/project/project_shortcuts.rb

-class Spinach::Features::ProjectShortcuts < Spinach::FeatureSteps
-  include SharedAuthentication
-  include SharedPaths
-  include SharedProject
-  include SharedProjectTab
-  include SharedShortcuts
-
-  step 'I press "g" and "f"' do
-    find('body').native.send_key('g')
-    find('body').native.send_key('f')
-  end
-
-  step 'I press "g" and "c"' do
-    find('body').native.send_key('g')
-    find('body').native.send_key('c')
-  end
-
-  step 'I press "g" and "n"' do
-    find('body').native.send_key('g')
-    find('body').native.send_key('n')
-  end
-
-  step 'I press "g" and "d"' do
-    find('body').native.send_key('g')
-    find('body').native.send_key('d')
-  end
-
-  step 'I press "g" and "s"' do
-    find('body').native.send_key('g')
-    find('body').native.send_key('s')
-  end
-
-  step 'I press "g" and "w"' do
-    find('body').native.send_key('g')
-    find('body').native.send_key('w')
-  end
-
-  step 'I press "g" and "e"' do
-    find('body').native.send_key('g')
-    find('body').native.send_key('e')
-  end
-end

lib/gitlab/data_builder/push.rb

@@ -64,8 +64,11 @@ module Gitlab
 
         # For performance purposes maximum 20 latest commits
         # will be passed as post receive hook data.
-        commit_attrs = commits_limited.map do |commit|
-          commit.hook_attrs(with_changed_files: true)
+        # n+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/38259
+        commit_attrs = Gitlab::GitalyClient.allow_n_plus_1_calls do
+          commits_limited.map do |commit|
+            commit.hook_attrs(with_changed_files: true)
+          end
         end
 
         type = Gitlab::Git.tag_ref?(ref) ? 'tag_push' : 'push'

lib/gitlab/diff/file.rb

@@ -5,7 +5,7 @@ module Gitlab
 
       delegate :new_file?, :deleted_file?, :renamed_file?,
         :old_path, :new_path, :a_mode, :b_mode, :mode_changed?,
-        :submodule?, :expanded?, :too_large?, :collapsed?, :line_count, to: :diff, prefix: false
+        :submodule?, :expanded?, :too_large?, :collapsed?, :line_count, :has_binary_notice?, to: :diff, prefix: false
 
       # Finding a viewer for a diff file happens based only on extension and whether the
       # diff file blobs are binary or text, which means 1 diff file should only be matched by 1 viewer,
@@ -166,7 +166,7 @@ module Gitlab
       end
 
       def binary?
-        old_blob&.binary? || new_blob&.binary?
+        has_binary_notice? || old_blob&.binary? || new_blob&.binary?
       end
 
       def text?

lib/gitlab/git/diff.rb

@@ -206,6 +206,10 @@ module Gitlab
         Diff.binary_message(@old_path, @new_path)
       end
 
+      def has_binary_notice?
+        @diff.start_with?('Binary')
+      end
+
       private
 
       def init_from_rugged(rugged)

lib/gitlab/git/repository.rb

@@ -475,7 +475,15 @@ module Gitlab
       # diff options.  The +options+ hash can also include :break_rewrites to
       # split larger rewrites into delete/add pairs.
       def diff(from, to, options = {}, *paths)
-        Gitlab::Git::DiffCollection.new(diff_patches(from, to, options, *paths), options)
+        iterator = gitaly_migrate(:diff_between) do |is_enabled|
+          if is_enabled
+            gitaly_commit_client.diff(from, to, options.merge(paths: paths))
+          else
+            diff_patches(from, to, options, *paths)
+          end
+        end
+
+        Gitlab::Git::DiffCollection.new(iterator, options)
       end
 
       # Returns a RefName for a given SHA

lib/gitlab/git/storage.rb

@@ -11,6 +11,7 @@ module Gitlab
       end
 
       CircuitOpen = Class.new(Inaccessible)
+      Misconfiguration = Class.new(Inaccessible)
 
       REDIS_KEY_PREFIX = 'storage_accessible:'.freeze
 

lib/gitlab/git/storage/circuit_breaker.rb

@@ -28,14 +28,26 @@ module Gitlab
         def self.for_storage(storage)
           cached_circuitbreakers = RequestStore.fetch(:circuitbreaker_cache) do
             Hash.new do |hash, storage_name|
-              hash[storage_name] = new(storage_name)
+              hash[storage_name] = build(storage_name)
             end
           end
 
           cached_circuitbreakers[storage]
         end
 
-        def initialize(storage, hostname = Gitlab::Environment.hostname)
+        def self.build(storage, hostname = Gitlab::Environment.hostname)
+          config = Gitlab.config.repositories.storages[storage]
+
+          if !config.present?
+            NullCircuitBreaker.new(storage, hostname, error: Misconfiguration.new("Storage '#{storage}' is not configured"))
+          elsif !config['path'].present?
+            NullCircuitBreaker.new(storage, hostname, error: Misconfiguration.new("Path for storage '#{storage}' is not configured"))
+          else
+            new(storage, hostname)
+          end
+        end
+
+        def initialize(storage, hostname)
           @storage = storage
           @hostname = hostname
 
@@ -64,6 +76,10 @@ module Gitlab
           recent_failure || too_many_failures
         end
 
+        def failure_info
+          @failure_info ||= get_failure_info
+        end
+
         # Memoizing the `storage_available` call means we only do it once per
         # request when the storage is available.
         #
@@ -121,10 +137,12 @@ module Gitlab
           end
         end
 
-        def failure_info
-          @failure_info ||= get_failure_info
+        def cache_key
+          @cache_key ||= "#{Gitlab::Git::Storage::REDIS_KEY_PREFIX}#{storage}:#{hostname}"
         end
 
+        private
+
         def get_failure_info
           last_failure, failure_count = Gitlab::Git::Storage.redis.with do |redis|
             redis.hmget(cache_key, :last_failure, :failure_count)
@@ -134,10 +152,6 @@ module Gitlab
 
           FailureInfo.new(last_failure, failure_count.to_i)
         end
-
-        def cache_key
-          @cache_key ||= "#{Gitlab::Git::Storage::REDIS_KEY_PREFIX}#{storage}:#{hostname}"
-        end
       end
     end
   end

lib/gitlab/git/storage/health.rb

@@ -78,7 +78,7 @@ module Gitlab
 
         def failing_circuit_breakers
           @failing_circuit_breakers ||= failing_on_hosts.map do |hostname|
-            CircuitBreaker.new(storage_name, hostname)
+            CircuitBreaker.build(storage_name, hostname)
           end
         end
 

lib/gitlab/git/storage/null_circuit_breaker.rb

+module Gitlab
+  module Git
+    module Storage
+      class NullCircuitBreaker
+        # These will have actual values
+        attr_reader :storage,
+                    :hostname
+
+        # These will always have nil values
+        attr_reader :storage_path,
+                    :failure_wait_time,
+                    :failure_reset_time,
+                    :storage_timeout
+
+        def initialize(storage, hostname, error: nil)
+          @storage = storage
+          @hostname = hostname
+          @error = error
+        end
+
+        def perform
+          @error ? raise(@error) : yield
+        end
+
+        def circuit_broken?
+          !!@error
+        end
+
+        def failure_count_threshold
+          1
+        end
+
+        def last_failure
+          circuit_broken? ? Time.now : nil
+        end
+
+        def failure_count
+          circuit_broken? ? 1 : 0
+        end
+
+        def failure_info
+          Gitlab::Git::Storage::CircuitBreaker::FailureInfo.new(last_failure, failure_count)
+        end
+      end
+    end
+  end
+end

lib/gitlab/gitaly_client/commit_service.rb

@@ -32,20 +32,38 @@ module Gitlab
         GitalyClient.call(@repository.storage, :commit_service, :commit_is_ancestor, request).value
       end
 
+      def diff(from, to, options = {})
+        from_id = case from
+                  when NilClass
+                    EMPTY_TREE_ID
+                  when Rugged::Commit
+                    from.oid
+                  else
+                    from
+                  end
+
+        to_id = case to
+                when NilClass
+                  EMPTY_TREE_ID
+                when Rugged::Commit
+                  to.oid
+                else
+                  to
+                end
+
+        request_params = diff_between_commits_request_params(from_id, to_id, options)
+
+        call_commit_diff(request_params, options)
+      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_params[:enforce_limits] = options.fetch(:limits, true)
-        request_params[:collapse_diffs] = request_params[:enforce_limits] || !options.fetch(:expanded, true)
-        request_params.merge!(Gitlab::Git::DiffCollection.collection_limits(options).to_h)
+        request_params = diff_from_parent_request_params(commit, options)
 
-        request = Gitaly::CommitDiffRequest.new(request_params)
-        response = GitalyClient.call(@repository.storage, :diff_service, :commit_diff, request)
-        GitalyClient::DiffStitcher.new(response)
+        call_commit_diff(request_params, options)
       end
 
       def commit_deltas(commit)
-        request = Gitaly::CommitDeltaRequest.new(commit_diff_request_params(commit))
+        request = Gitaly::CommitDeltaRequest.new(diff_from_parent_request_params(commit))
         response = GitalyClient.call(@repository.storage, :diff_service, :commit_delta, request)
 
         response.flat_map { |msg| msg.deltas }
@@ -214,13 +232,28 @@ module Gitlab
 
       private
 
-      def commit_diff_request_params(commit, options = {})
+      def call_commit_diff(request_params, options = {})
+        request_params[:ignore_whitespace_change] = options.fetch(:ignore_whitespace_change, false)
+        request_params[:enforce_limits] = options.fetch(:limits, true)
+        request_params[:collapse_diffs] = request_params[:enforce_limits] || !options.fetch(:expanded, true)
+        request_params.merge!(Gitlab::Git::DiffCollection.collection_limits(options).to_h)
+
+        request = Gitaly::CommitDiffRequest.new(request_params)
+        response = GitalyClient.call(@repository.storage, :diff_service, :commit_diff, request)
+        GitalyClient::DiffStitcher.new(response)
+      end
+
+      def diff_from_parent_request_params(commit, options = {})
         parent_id = commit.parent_ids.first || EMPTY_TREE_ID
 
+        diff_between_commits_request_params(parent_id, commit.id, options)
+      end
+
+      def diff_between_commits_request_params(from_id, to_id, options)
         {
           repository: @gitaly_repo,
-          left_commit_id: parent_id,
-          right_commit_id: commit.id,
+          left_commit_id: from_id,
+          right_commit_id: to_id,
           paths: options.fetch(:paths, []).map { |path| GitalyClient.encode(path) }
         }
       end

lib/gitlab/health_checks/fs_shards_check.rb

@@ -125,7 +125,7 @@ module Gitlab
         end
 
         def storage_circuitbreaker_test(storage_name)
-          Gitlab::Git::Storage::CircuitBreaker.new(storage_name).perform { "OK" }
+          Gitlab::Git::Storage::CircuitBreaker.build(storage_name).perform { "OK" }
         rescue Gitlab::Git::Storage::Inaccessible
           nil
         end

lib/system_check/incoming_email/imap_authentication_check.rb

@@ -4,22 +4,17 @@ module SystemCheck
       set_name 'IMAP server credentials are correct?'
 
       def check?
-        if mailbox_config
-          begin
-            imap = Net::IMAP.new(config[:host], port: config[:port], ssl: config[:ssl])
-            imap.starttls if config[:start_tls]
-            imap.login(config[:email], config[:password])
-            connected = true
-          rescue
-            connected = false
-          end
+        if config
+          try_connect_imap
+        else
+          @error = "#{mail_room_config_path} does not have mailboxes setup"
+          false
         end
-
-        connected
       end
 
       def show_error
         try_fixing_it(
+          "An error occurred: #{@error.class}: #{@error.message}",
           'Check that the information in config/gitlab.yml is correct'
         )
         for_more_information(
@@ -30,15 +25,31 @@ module SystemCheck
 
       private
 
-      def mailbox_config
-        return @config if @config
+      def try_connect_imap
+        imap = Net::IMAP.new(config[:host], port: config[:port], ssl: config[:ssl])
+        imap.starttls if config[:start_tls]
+        imap.login(config[:email], config[:password])
+        true
+      rescue => error
+        @error = error
+        false
+      end
+
+      def config
+        @config ||= load_config
+      end
+
+      def mail_room_config_path
+        @mail_room_config_path ||=
+          Rails.root.join('config', 'mail_room.yml').to_s
+      end
 
-        config_path = Rails.root.join('config', 'mail_room.yml').to_s
-        erb = ERB.new(File.read(config_path))
-        erb.filename = config_path
+      def load_config
+        erb = ERB.new(File.read(mail_room_config_path))
+        erb.filename = mail_room_config_path
         config_file = YAML.load(erb.result)
 
-        @config = config_file[:mailboxes]&.first
+        config_file.dig(:mailboxes, 0)
       end
     end
   end

locale/gitlab.pot

@@ -3,12 +3,13 @@
 # This file is distributed under the same license as the gitlab package.
 # FIRST AUTHOR <EMAIL@ADDRESS>, YEAR.
 #
+#, fuzzy
 msgid ""
 msgstr ""
 "Project-Id-Version: gitlab 1.0.0\n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2017-09-06 08:32+0200\n"
-"PO-Revision-Date: 2017-09-06 08:32+0200\n"
+"POT-Creation-Date: 2017-09-21 14:20+0530\n"
+"PO-Revision-Date: 2017-09-21 14:20+0530\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <LL@li.org>\n"
 "Language: \n"
@@ -52,6 +53,9 @@ msgid_plural "%d pipelines"
 msgstr[0] ""
 msgstr[1] ""
 
+msgid "1st contribution!"
+msgstr ""
+
 msgid "A collection of graphs regarding Continuous Integration"
 msgstr ""
 
@@ -94,7 +98,7 @@ msgstr ""
 msgid "All"
 msgstr ""
 
-msgid "Appearances"
+msgid "Appearance"
 msgstr ""
 
 msgid "Applications"
@@ -118,64 +122,37 @@ msgstr ""
 msgid "Are you sure?"
 msgstr ""
 
-msgid "Attach a file by drag &amp; drop or %{upload_link}"
-msgstr ""
-
-msgid "Authentication log"
-msgstr ""
-
-msgid "Billing"
-msgstr ""
-
-msgid "BillingPlans|%{group_name} is currently on the %{plan_link} plan."
+msgid "Artifacts"
 msgstr ""
 
-msgid "BillingPlans|Automatic downgrade and upgrade to some plans is currently not available."
-msgstr ""
-
-msgid "BillingPlans|Current plan"
-msgstr ""
-
-msgid "BillingPlans|Customer Support"
-msgstr ""
-
-msgid "BillingPlans|Learn more about each plan by reading our %{faq_link}."
-msgstr ""
-
-msgid "BillingPlans|Manage plan"
-msgstr ""
-
-msgid "BillingPlans|Please contact %{customer_support_link} in that case."
-msgstr ""
-
-msgid "BillingPlans|See all %{plan_name} features"
+msgid "Attach a file by drag &amp; drop or %{upload_link}"
 msgstr ""
 
-msgid "BillingPlans|This group uses the plan associated with its parent group."
+msgid "Authentication Log"
 msgstr ""
 
-msgid "BillingPlans|To manage the plan for this group, visit the billing section of %{parent_billing_page_link}."
+msgid "Auto DevOps (Beta)"
 msgstr ""
 
-msgid "BillingPlans|Upgrade"
+msgid "Auto DevOps can be activated for this project. It will automatically build, test, and deploy your application based on a predefined CI/CD configuration."
 msgstr ""
 
-msgid "BillingPlans|You are currently on the %{plan_link} plan."
+msgid "Auto DevOps documentation"
 msgstr ""
 
-msgid "BillingPlans|frequently asked questions"
+msgid "Auto Review Apps and Auto Deploy need a domain name and the %{kubernetes} to work correctly."
 msgstr ""
 
-msgid "BillingPlans|monthly"
+msgid "Auto Review Apps and Auto Deploy need a domain name to work correctly."
 msgstr ""
 
-msgid "BillingPlans|paid annually at %{price_per_year}"
+msgid "Auto Review Apps and Auto Deploy need the %{kubernetes} to work correctly."
 msgstr ""
 
-msgid "BillingPlans|per user"
+msgid "AutoDevOps|Learn more in the"
 msgstr ""
 
-msgid "Billinglans|Downgrade"
+msgid "Board"
 msgstr ""
 
 msgid "Branch"
@@ -195,6 +172,12 @@ msgstr ""
 msgid "Branches"
 msgstr ""
 
+msgid "Branches|This branch hasn’t been merged into %{default_branch}."
+msgstr ""
+
+msgid "Branches|To avoid data loss, consider merging this branch before deleting it."
+msgstr ""
+
 msgid "Browse Directory"
 msgstr ""
 
@@ -338,18 +321,12 @@ msgstr ""
 msgid "Compare"
 msgstr ""
 
-msgid "Container Registry"
-msgstr ""
-
 msgid "Contribution guide"
 msgstr ""
 
 msgid "Contributors"
 msgstr ""
 
-msgid "Copy SSH public key to clipboard"
-msgstr ""
-
 msgid "Copy URL to clipboard"
 msgstr ""
 
@@ -490,6 +467,9 @@ msgstr ""
 msgid "Emails"
 msgstr ""
 
+msgid "Enable in settings"
+msgstr ""
+
 msgid "EventFilterBy|Filter by all"
 msgstr ""
 
@@ -517,6 +497,9 @@ msgstr ""
 msgid "Every week (Sundays at 4:00am)"
 msgstr ""
 
+msgid "Explore projects"
+msgstr ""
+
 msgid "Failed to change the owner"
 msgstr ""
 
@@ -558,9 +541,6 @@ msgstr ""
 msgid "GPG Keys"
 msgstr ""
 
-msgid "Geo Nodes"
-msgstr ""
-
 msgid "Git storage health information has been reset"
 msgstr ""
 
@@ -573,7 +553,28 @@ msgstr ""
 msgid "GoToYourFork|Fork"
 msgstr ""
 
-msgid "Group overview"
+msgid "GroupSettings|Prevent sharing a project within %{group} with other groups"
+msgstr ""
+
+msgid "GroupSettings|Share with group lock"
+msgstr ""
+
+msgid "GroupSettings|This setting is applied on %{ancestor_group} and has been overridden on this subgroup."
+msgstr ""
+
+msgid "GroupSettings|This setting is applied on %{ancestor_group}. To share projects in this group with another group, ask the owner to override the setting or %{remove_ancestor_share_with_group_lock}."
+msgstr ""
+
+msgid "GroupSettings|This setting is applied on %{ancestor_group}. You can override the setting or %{remove_ancestor_share_with_group_lock}."
+msgstr ""
+
+msgid "GroupSettings|This setting will be applied to all subgroups unless overridden by a group owner."
+msgstr ""
+
+msgid "GroupSettings|cannot be disabled when the parent group \"Share with group lock\" is enabled, except by the owner of the parent group"
+msgstr ""
+
+msgid "GroupSettings|remove the share with group lock from %{ancestor_group_name}"
 msgstr ""
 
 msgid "Health Check"
@@ -597,9 +598,6 @@ msgstr ""
 msgid "Home"
 msgstr ""
 
-msgid "Hooks"
-msgstr ""
-
 msgid "Housekeeping successfully started"
 msgstr ""
 
@@ -621,6 +619,9 @@ msgstr ""
 msgid "Issues"
 msgstr ""
 
+msgid "Jobs"
+msgstr ""
+
 msgid "LFSStatus|Disabled"
 msgstr ""
 
@@ -662,17 +663,11 @@ msgstr ""
 msgid "Leave project"
 msgstr ""
 
-msgid "License"
-msgstr ""
-
 msgid "Limited to showing %d event at most"
 msgid_plural "Limited to showing %d events at most"
 msgstr[0] ""
 msgstr[1] ""
 
-msgid "Locked Files"
-msgstr ""
-
 msgid "Median"
 msgstr ""
 
@@ -813,6 +808,18 @@ msgstr ""
 msgid "Owner"
 msgstr ""
 
+msgid "Pagination|Last »"
+msgstr ""
+
+msgid "Pagination|Next"
+msgstr ""
+
+msgid "Pagination|Prev"
+msgstr ""
+
+msgid "Pagination|« First"
+msgstr ""
+
 msgid "Password"
 msgstr ""
 
@@ -828,9 +835,6 @@ msgstr ""
 msgid "Pipeline Schedules"
 msgstr ""
 
-msgid "Pipeline quota"
-msgstr ""
-
 msgid "PipelineCharts|Failed:"
 msgstr ""
 
@@ -918,10 +922,7 @@ msgstr ""
 msgid "Preferences"
 msgstr ""
 
-msgid "Profile Settings"
-msgstr ""
-
-msgid "Project"
+msgid "Profile"
 msgstr ""
 
 msgid "Project '%{project_name}' queued for deletion."
@@ -957,9 +958,6 @@ msgstr ""
 msgid "Project home"
 msgstr ""
 
-msgid "Project overview"
-msgstr ""
-
 msgid "ProjectActivityRSS|Subscribe"
 msgstr ""
 
@@ -984,22 +982,22 @@ msgstr ""
 msgid "ProjectNetworkGraph|Graph"
 msgstr ""
 
-msgid "Push Rules"
+msgid "ProjectsDropdown|Frequently visited"
 msgstr ""
 
 msgid "ProjectsDropdown|Loading projects"
 msgstr ""
 
-msgid "ProjectsDropdown|Sorry, no projects matched your search"
-msgstr ""
-
 msgid "ProjectsDropdown|Projects you visit often will appear here"
 msgstr ""
 
 msgid "ProjectsDropdown|Search your projects"
 msgstr ""
 
-msgid "ProjectsDropdown|Something went wrong on our end"
+msgid "ProjectsDropdown|Something went wrong on our end."
+msgstr ""
+
+msgid "ProjectsDropdown|Sorry, no projects matched your search"
 msgstr ""
 
 msgid "ProjectsDropdown|This feature requires browser localStorage support"
@@ -1074,6 +1072,9 @@ msgstr ""
 msgid "Schedule a new pipeline"
 msgstr ""
 
+msgid "Schedules"
+msgstr ""
+
 msgid "Scheduling Pipelines"
 msgstr ""
 
@@ -1113,6 +1114,12 @@ msgstr ""
 msgid "Settings"
 msgstr ""
 
+msgid "Show parent pages"
+msgstr ""
+
+msgid "Show parent subgroups"
+msgstr ""
+
 msgid "Showing %d event"
 msgid_plural "Showing %d events"
 msgstr[0] ""
@@ -1133,6 +1140,9 @@ msgstr ""
 msgid "StarProject|Star"
 msgstr ""
 
+msgid "Starred projects"
+msgstr ""
+
 msgid "Start a %{new_merge_request} with these changes"
 msgstr ""
 
@@ -1142,6 +1152,9 @@ msgstr ""
 msgid "Switch branch/tag"
 msgstr ""
 
+msgid "System Hooks"
+msgstr ""
+
 msgid "Tag"
 msgid_plural "Tags"
 msgstr[0] ""
@@ -1207,6 +1220,9 @@ msgstr ""
 msgid "There are problems accessing Git storage: "
 msgstr ""
 
+msgid "This is the author's first Merge Request to this project. Handle with care."
+msgstr ""
+
 msgid "This means you can not push code until you create an empty repository or import existing one."
 msgstr ""
 
@@ -1457,6 +1473,9 @@ msgstr ""
 msgid "Your name"
 msgstr ""
 
+msgid "Your projects"
+msgstr ""
+
 msgid "day"
 msgid_plural "days"
 msgstr[0] ""

spec/features/projects/shortcuts_spec.rb

-require 'spec_helper'
-
-feature 'Project shortcuts' do
-  let(:project) { create(:project, name: 'Victorialand') }
-  let(:user) { create(:user) }
-
-  describe 'On a project', js: true do
-    before do
-      project.team << [user, :master]
-      sign_in user
-      visit project_path(project)
-    end
-
-    describe 'pressing "i"' do
-      it 'redirects to new issue page' do
-        find('body').native.send_key('i')
-        expect(page).to have_content('Victorialand')
-      end
-    end
-  end
-end

spec/features/projects/user_uses_shortcuts_spec.rb

+require 'spec_helper'
+
+describe 'User uses shortcuts', :js do
+  let(:project) { create(:project, :repository) }
+  let(:user) { create(:user) }
+
+  before do
+    project.add_master(user)
+    sign_in(user)
+
+    visit(project_path(project))
+  end
+
+  context 'when navigating to the Overview pages' do
+    it 'redirects to the details page' do
+      find('body').native.send_key('g')
+      find('body').native.send_key('p')
+
+      expect(page).to have_active_navigation('Overview')
+      expect(page).to have_active_sub_navigation('Details')
+    end
+
+    it 'redirects to the activity page' do
+      find('body').native.send_key('g')
+      find('body').native.send_key('e')
+
+      expect(page).to have_active_navigation('Overview')
+      expect(page).to have_active_sub_navigation('Activity')
+    end
+  end
+
+  context 'when navigating to the Repository pages' do
+    it 'redirects to the repository files page' do
+      find('body').native.send_key('g')
+      find('body').native.send_key('f')
+
+      expect(page).to have_active_navigation('Repository')
+      expect(page).to have_active_sub_navigation('Files')
+    end
+
+    it 'redirects to the repository commits page' do
+      find('body').native.send_key('g')
+      find('body').native.send_key('c')
+
+      expect(page).to have_active_navigation('Repository')
+      expect(page).to have_active_sub_navigation('Commits')
+    end
+
+    it 'redirects to the repository graph page' do
+      find('body').native.send_key('g')
+      find('body').native.send_key('n')
+
+      expect(page).to have_active_navigation('Repository')
+      expect(page).to have_active_sub_navigation('Graph')
+    end
+
+    it 'redirects to the repository charts page' do
+      find('body').native.send_key('g')
+      find('body').native.send_key('d')
+
+      expect(page).to have_active_navigation('Repository')
+      expect(page).to have_active_sub_navigation('Charts')
+    end
+  end
+
+  context 'when navigating to the Issues pages' do
+    it 'redirects to the issues list page' do
+      find('body').native.send_key('g')
+      find('body').native.send_key('i')
+
+      expect(page).to have_active_navigation('Issues')
+      expect(page).to have_active_sub_navigation('List')
+    end
+
+    it 'redirects to the new issue page' do
+      find('body').native.send_key('i')
+
+      expect(page).to have_content(project.title)
+    end
+  end
+
+  context 'when navigating to the Merge Requests pages' do
+    it 'redirects to the merge requests page' do
+      find('body').native.send_key('g')
+      find('body').native.send_key('m')
+
+      expect(page).to have_active_navigation('Merge Requests')
+    end
+  end
+
+  context 'when navigating to the Snippets pages' do
+    it 'redirects to the snippets page' do
+      find('body').native.send_key('g')
+      find('body').native.send_key('s')
+
+      expect(page).to have_active_navigation('Snippets')
+    end
+  end
+
+  context 'when navigating to the Wiki pages' do
+    it 'redirects to the wiki page' do
+      find('body').native.send_key('g')
+      find('body').native.send_key('w')
+
+      expect(page).to have_active_navigation('Wiki')
+    end
+  end
+end

spec/javascripts/pipelines/pipelines_artifacts_spec.js

@@ -34,7 +34,7 @@ describe('Pipelines Artifacts dropdown', () => {
     ).toEqual(artifacts[0].path);
 
     expect(
-      component.$el.querySelector('.dropdown-menu li a span').textContent,
+      component.$el.querySelector('.dropdown-menu li a').textContent,
     ).toContain(artifacts[0].name);
   });
 });

spec/lib/gitlab/git/storage/circuit_breaker_spec.rb

@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe Gitlab::Git::Storage::CircuitBreaker, clean_gitlab_redis_shared_state: true, broken_storage: true do
   let(:storage_name) { 'default' }
-  let(:circuit_breaker) { described_class.new(storage_name) }
+  let(:circuit_breaker) { described_class.new(storage_name, hostname) }
   let(:hostname) { Gitlab::Environment.hostname }
   let(:cache_key) { "storage_accessible:#{storage_name}:#{hostname}" }
 
@@ -22,7 +22,8 @@ describe Gitlab::Git::Storage::CircuitBreaker, clean_gitlab_redis_shared_state:
                             'failure_wait_time' => 30,
                             'failure_reset_time' => 1800,
                             'storage_timeout' => 5
-                          }
+                          },
+                          'nopath' => { 'path' => nil }
                          )
   end
 
@@ -59,6 +60,14 @@ describe Gitlab::Git::Storage::CircuitBreaker, clean_gitlab_redis_shared_state:
       expect(breaker).to be_a(described_class)
       expect(described_class.for_storage('default')).to eq(breaker)
     end
+
+    it 'returns a broken circuit breaker for an unknown storage' do
+      expect(described_class.for_storage('unknown').circuit_broken?).to be_truthy
+    end
+
+    it 'returns a broken circuit breaker when the path is not set' do
+      expect(described_class.for_storage('nopath').circuit_broken?).to be_truthy
+    end
   end
 
   describe '#initialize' do

spec/lib/gitlab/git/storage/null_circuit_breaker_spec.rb

+require 'spec_helper'
+
+describe Gitlab::Git::Storage::NullCircuitBreaker do
+  let(:storage) { 'default' }
+  let(:hostname) { 'localhost' }
+  let(:error) { nil }
+
+  subject(:breaker) { described_class.new(storage, hostname, error: error) }
+
+  context 'with an error' do
+    let(:error) { Gitlab::Git::Storage::Misconfiguration.new('error') }
+
+    describe '#perform' do
+      it { expect { breaker.perform { 'ok' } }.to raise_error(error) }
+    end
+
+    describe '#circuit_broken?' do
+      it { expect(breaker.circuit_broken?).to be_truthy }
+    end
+
+    describe '#last_failure' do
+      it { Timecop.freeze { expect(breaker.last_failure).to eq(Time.now) } }
+    end
+
+    describe '#failure_count' do
+      it { expect(breaker.failure_count).to eq(breaker.failure_count_threshold) }
+    end
+
+    describe '#failure_info' do
+      it { Timecop.freeze { expect(breaker.failure_info).to eq(Gitlab::Git::Storage::CircuitBreaker::FailureInfo.new(Time.now, breaker.failure_count_threshold)) } }
+    end
+  end
+
+  context 'not broken' do
+    describe '#perform' do
+      it { expect(breaker.perform { 'ok' }).to eq('ok') }
+    end
+
+    describe '#circuit_broken?' do
+      it { expect(breaker.circuit_broken?).to be_falsy }
+    end
+
+    describe '#last_failure' do
+      it { expect(breaker.last_failure).to be_nil }
+    end
+
+    describe '#failure_count' do
+      it { expect(breaker.failure_count).to eq(0) }
+    end
+
+    describe '#failure_info' do
+      it { expect(breaker.failure_info).to eq(Gitlab::Git::Storage::CircuitBreaker::FailureInfo.new(nil, 0)) }
+    end
+  end
+
+  describe '#failure_count_threshold' do
+    it { expect(breaker.failure_count_threshold).to eq(1) }
+  end
+
+  it 'implements the CircuitBreaker interface' do
+    ours = described_class.public_instance_methods
+    theirs = Gitlab::Git::Storage::CircuitBreaker.public_instance_methods
+
+    # These methods are not part of the public API, but are public to allow the
+    # CircuitBreaker specs to operate. They should be made private over time.
+    exceptions = %i[
+      cache_key
+      check_storage_accessible!
+      no_failures?
+      storage_available?
+      track_storage_accessible
+      track_storage_inaccessible
+    ]
+
+    expect(theirs - ours).to contain_exactly(*exceptions)
+  end
+end

spec/lib/gitlab/health_checks/fs_shards_check_spec.rb

@@ -21,7 +21,7 @@ describe Gitlab::HealthChecks::FsShardsCheck do
 
   let(:metric_class) { Gitlab::HealthChecks::Metric }
   let(:result_class) { Gitlab::HealthChecks::Result }
-  let(:repository_storages) { [:default] }
+  let(:repository_storages) { ['default'] }
   let(:tmp_dir) { Dir.mktmpdir }
 
   let(:storages_paths) do
@@ -64,7 +64,7 @@ describe Gitlab::HealthChecks::FsShardsCheck do
           allow(described_class).to receive(:storage_circuitbreaker_test) { true }
         end
 
-        it { is_expected.to include(result_class.new(false, 'cannot stat storage', shard: :default)) }
+        it { is_expected.to include(result_class.new(false, 'cannot stat storage', shard: 'default')) }
       end
 
       context 'storage points to directory that has both read and write rights' do
@@ -72,7 +72,7 @@ describe Gitlab::HealthChecks::FsShardsCheck do
           FileUtils.chmod_R(0755, tmp_dir)
         end
 
-        it { is_expected.to include(result_class.new(true, nil, shard: :default)) }
+        it { is_expected.to include(result_class.new(true, nil, shard: 'default')) }
 
         it 'cleans up files used for testing' do
           expect(described_class).to receive(:storage_write_test).with(any_args).and_call_original
@@ -85,7 +85,7 @@ describe Gitlab::HealthChecks::FsShardsCheck do
             allow(described_class).to receive(:storage_read_test).with(any_args).and_return(false)
           end
 
-          it { is_expected.to include(result_class.new(false, 'cannot read from storage', shard: :default)) }
+          it { is_expected.to include(result_class.new(false, 'cannot read from storage', shard: 'default')) }
         end
 
         context 'write test fails' do
@@ -93,7 +93,7 @@ describe Gitlab::HealthChecks::FsShardsCheck do
             allow(described_class).to receive(:storage_write_test).with(any_args).and_return(false)
           end
 
-          it { is_expected.to include(result_class.new(false, 'cannot write to storage', shard: :default)) }
+          it { is_expected.to include(result_class.new(false, 'cannot write to storage', shard: 'default')) }
         end
       end
     end
@@ -109,7 +109,7 @@ describe Gitlab::HealthChecks::FsShardsCheck do
         it 'provides metrics' do
           metrics = described_class.metrics
 
-          expect(metrics).to all(have_attributes(labels: { shard: :default }))
+          expect(metrics).to all(have_attributes(labels: { shard: 'default' }))
           expect(metrics).to include(an_object_having_attributes(name: :filesystem_accessible, value: 0))
           expect(metrics).to include(an_object_having_attributes(name: :filesystem_readable, value: 0))
           expect(metrics).to include(an_object_having_attributes(name: :filesystem_writable, value: 0))
@@ -128,7 +128,7 @@ describe Gitlab::HealthChecks::FsShardsCheck do
         it 'provides metrics' do
           metrics = described_class.metrics
 
-          expect(metrics).to all(have_attributes(labels: { shard: :default }))
+          expect(metrics).to all(have_attributes(labels: { shard: 'default' }))
           expect(metrics).to include(an_object_having_attributes(name: :filesystem_accessible, value: 1))
           expect(metrics).to include(an_object_having_attributes(name: :filesystem_readable, value: 1))
           expect(metrics).to include(an_object_having_attributes(name: :filesystem_writable, value: 1))
@@ -156,14 +156,14 @@ describe Gitlab::HealthChecks::FsShardsCheck do
     describe '#readiness' do
       subject { described_class.readiness }
 
-      it { is_expected.to include(result_class.new(false, 'cannot stat storage', shard: :default)) }
+      it { is_expected.to include(result_class.new(false, 'cannot stat storage', shard: 'default')) }
     end
 
     describe '#metrics' do
       it 'provides metrics' do
         metrics = described_class.metrics
 
-        expect(metrics).to all(have_attributes(labels: { shard: :default }))
+        expect(metrics).to all(have_attributes(labels: { shard: 'default' }))
         expect(metrics).to include(an_object_having_attributes(name: :filesystem_accessible, value: 0))
         expect(metrics).to include(an_object_having_attributes(name: :filesystem_readable, value: 0))
         expect(metrics).to include(an_object_having_attributes(name: :filesystem_writable, value: 0))

spec/support/matchers/navigation_matcher.rb

@@ -4,3 +4,9 @@ RSpec::Matchers.define :have_active_navigation do |expected|
     expect(page.find('.sidebar-top-level-items > li.active')).to have_content(expected)
   end
 end
+
+RSpec::Matchers.define :have_active_sub_navigation do |expected|
+  match do |page|
+    expect(page.find('.sidebar-sub-level-items > li.active:not(.fly-out-top-item)')).to have_content(expected)
+  end
+end

spec/support/stub_configuration.rb

@@ -52,7 +52,7 @@ module StubConfiguration
     # Default storage is always required
     messages['default'] ||= Gitlab.config.repositories.storages.default
     messages.each do |storage_name, storage_settings|
-      storage_settings['path'] ||= TestEnv.repos_path
+      storage_settings['path'] = TestEnv.repos_path unless storage_settings.key?('path')
       storage_settings['failure_count_threshold'] ||= 10
       storage_settings['failure_wait_time'] ||= 30
       storage_settings['failure_reset_time'] ||= 1800