Merge remote-tracking branch 'origin/master' into...

Merge remote-tracking branch 'origin/master' into 41416-making-instance-wide-data-tools-more-accessible

CHANGELOG.md

@@ -2,6 +2,24 @@
 documentation](doc/development/changelog.md) for instructions on adding your own
 entry.
 
+## 11.1.3 (2018-07-27)
+
+### Fixed (8 changes, 1 of them is from the community)
+
+- Rework some projects table indexes around repository_storage field. !20377
+- Fix navigation to First and Next discussion on MR Changes tab. !20434
+- Fix showing outdated discussions on Changes tab. !20445
+- Fix autosave and ESC confirmation issues for MR discussions. !20569
+- Fix rendering of the context lines in MR diffs page. !20642
+- Don't overflow project/group dropdown results. !20704 (gfyoung)
+- Fixed IDE not opening JSON files. !20798
+- Disable Gitaly timeouts when creating or restoring backups. !20810
+
+### Performance (1 change)
+
+- Reduces the client side memory footprint on merge requests. !20744
+
+
 ## 11.1.2 (2018-07-26)
 
 ### Security (4 changes)

GITLAB_SHELL_VERSION

-7.1.5
+7.2.0

app/assets/javascripts/pipelines/components/graph/dropdown_job_component.vue

@@ -14,7 +14,7 @@ import tooltip from '../../../vue_shared/directives/tooltip';
  *   "id": 4256,
  *   "name": "test",
  *   "status": {
- *     "icon": "icon_status_success",
+ *     "icon": "status_success",
  *     "text": "passed",
  *     "label": "passed",
  *     "group": "success",

app/assets/javascripts/pipelines/components/graph/job_component.vue

@@ -13,7 +13,7 @@ import tooltip from '../../../vue_shared/directives/tooltip';
  *   "id": 4256,
  *   "name": "test",
  *   "status": {
- *     "icon": "icon_status_success",
+ *     "icon": "status_success",
  *     "text": "passed",
  *     "label": "passed",
  *     "group": "success",

app/finders/admin/projects_finder.rb

@@ -7,7 +7,7 @@ class Admin::ProjectsFinder
   end
 
   def execute
-    items = Project.without_deleted.with_statistics
+    items = Project.without_deleted.with_statistics.with_route
     items = by_namespace_id(items)
     items = by_visibilty_level(items)
     items = by_with_push(items)
@@ -16,7 +16,7 @@ class Admin::ProjectsFinder
     items = by_archived(items)
     items = by_personal(items)
     items = by_name(items)
-    items = items.includes(namespace: [:owner])
+    items = items.includes(namespace: [:owner, :route])
     sort(items).page(params[:page])
   end
 

app/mailers/notify.rb

@@ -124,7 +124,7 @@ class Notify < BaseMailer
 
       fallback_reply_message_id = "<reply-#{reply_key}@#{Gitlab.config.gitlab.host}>".freeze
       headers['References'] ||= []
-      headers['References'] << fallback_reply_message_id
+      headers['References'].unshift(fallback_reply_message_id)
 
       @reply_by_email = true
     end
@@ -158,7 +158,7 @@ class Notify < BaseMailer
   def mail_answer_thread(model, headers = {})
     headers['Message-ID'] = "<#{SecureRandom.hex}@#{Gitlab.config.gitlab.host}>"
     headers['In-Reply-To'] = message_id(model)
-    headers['References'] = message_id(model)
+    headers['References'] = [message_id(model)]
 
     headers[:subject]&.prepend('Re: ')
 

app/models/concerns/routable.rb

@@ -90,34 +90,17 @@ module Routable
   end
 
   def full_name
-    if route && route.name.present?
-      @full_name ||= route.name # rubocop:disable Gitlab/ModuleWithInstanceVariables
-    else
-      update_route if persisted?
-
-      build_full_name
-    end
+    route&.name || build_full_name
   end
 
-  # Every time `project.namespace.becomes(Namespace)` is called for polymorphic_path,
-  # a new instance is instantiated, and we end up duplicating the same query to retrieve
-  # the route. Caching this per request ensures that even if we have multiple instances,
-  # we will not have to duplicate work, avoiding N+1 queries in some cases.
   def full_path
-    return uncached_full_path unless RequestStore.active? && persisted?
-
-    RequestStore[full_path_key] ||= uncached_full_path
+    route&.path || build_full_path
   end
 
   def full_path_components
     full_path.split('/')
   end
 
-  def expires_full_path_cache
-    RequestStore.delete(full_path_key) if RequestStore.active?
-    @full_path = nil # rubocop:disable Gitlab/ModuleWithInstanceVariables
-  end
-
   def build_full_path
     if parent && path
       parent.full_path + '/' + path
@@ -138,16 +121,6 @@ module Routable
     self.errors[:path].concat(route_path_errors) if route_path_errors
   end
 
-  def uncached_full_path
-    if route && route.path.present?
-      @full_path ||= route.path # rubocop:disable Gitlab/ModuleWithInstanceVariables
-    else
-      update_route if persisted?
-
-      build_full_path
-    end
-  end
-
   def full_name_changed?
     name_changed? || parent_changed?
   end
@@ -156,10 +129,6 @@ module Routable
     path_changed? || parent_changed?
   end
 
-  def full_path_key
-    @full_path_key ||= "routable/full_path/#{self.class.name}/#{self.id}"
-  end
-
   def build_full_name
     if parent && name
       parent.human_name + ' / ' + name
@@ -168,18 +137,9 @@ module Routable
     end
   end
 
-  def update_route
-    return if Gitlab::Database.read_only?
-
-    prepare_route
-    route.save
-  end
-
   def prepare_route
     route || build_route(source: self)
     route.path = build_full_path
     route.name = build_full_name
-    @full_path = nil # rubocop:disable Gitlab/ModuleWithInstanceVariables
-    @full_name = nil # rubocop:disable Gitlab/ModuleWithInstanceVariables
   end
 end

app/models/concerns/storage/legacy_namespace.rb

@@ -11,8 +11,6 @@ module Storage
                      Namespace.find(parent_id_was) # raise NotFound early if needed
                    end
 
-      expires_full_path_cache
-
       move_repositories
 
       if parent_changed?

app/models/namespace.rb

@@ -304,7 +304,6 @@ class Namespace < ActiveRecord::Base
 
   def write_projects_repository_config
     all_projects.find_each do |project|
-      project.expires_full_path_cache # we need to clear cache to validate renames correctly
       project.write_repository_config
     end
   end

app/models/project.rb

@@ -1238,8 +1238,6 @@ class Project < ActiveRecord::Base
     return true if skip_disk_validation
     return false unless repository_storage
 
-    expires_full_path_cache # we need to clear cache to validate renames correctly
-
     # Check if repository with same path already exists on disk we can
     # skip this for the hashed storage because the path does not change
     if legacy_storage? && repository_with_same_path_already_exists?
@@ -1618,7 +1616,6 @@ class Project < ActiveRecord::Base
     # When we import a project overwriting the original project, there
     # is a move operation. In that case we don't want to send the instructions.
     send_move_instructions(full_path_was) unless import_started?
-    expires_full_path_cache
 
     self.old_path_with_namespace = full_path_was
     SystemHooksService.new.execute_hooks_for(self, :rename)

app/serializers/pipeline_serializer.rb

@@ -11,10 +11,15 @@ class PipelineSerializer < BaseSerializer
         :retryable_builds,
         :cancelable_statuses,
         :trigger_requests,
-        :project,
         :manual_actions,
         :artifacts,
-        { pending_builds: :project }
+        {
+          pending_builds: :project,
+          project: [:route, { namespace: :route }],
+          artifacts: {
+            project: [:route, { namespace: :route }]
+          }
+        }
       ])
     end
 

app/services/projects/transfer_service.rb

@@ -77,7 +77,6 @@ module Projects
         Gitlab::PagesTransfer.new.move_project(project.path, @old_namespace.full_path, @new_namespace.full_path)
 
         project.old_path_with_namespace = @old_path
-        project.expires_full_path_cache
 
         write_repository_config(@new_path)
 

app/views/admin/projects/_projects.html.haml

@@ -17,7 +17,7 @@
             - if project.archived
               %span.badge.badge-warning archived
           .title
-            = link_to [:admin, project.namespace.becomes(Namespace), project] do
+            = link_to(admin_namespace_project_path(project.namespace, project)) do
               .dash-project-avatar
                 .avatar-container.s40
                   = project_icon(project, alt: '', class: 'avatar project-avatar s40')

app/views/projects/jobs/_sidebar.html.haml

@@ -86,7 +86,7 @@
       - HasStatus::ORDERED_STATUSES.each do |build_status|
         - builds.select{|build| build.status == build_status}.each do |build|
           .build-job{ class: sidebar_build_class(build, @build), data: { stage: build.stage } }
-            - tooltip = sanitize(build.tooltip_message)
+            - tooltip = sanitize(build.tooltip_message.dup)
             = link_to(project_job_path(@project, build), data: { toggle: 'tooltip', html: 'true', title: tooltip, container: 'body' }) do
               = sprite_icon('arrow-right', size:16, css_class: 'icon-arrow-right')
               %span{ class: "ci-status-icon-#{build.status}" }

changelogs/unreleased/4525-fix-project-indexes.yml

----
-title: Rework some projects table indexes around repository_storage field
-merge_request: 20377
-author:
-type: fixed

changelogs/unreleased/47728-mr-api-documentation-changes.yml

+---
+title: Remove changes_count from MR API documentation where necessary
+merge_request: 19745
+author: Jan Beckmann
+type: fixed

changelogs/unreleased/48817-fix-mr-changes-discussion-navigation.yml

----
-title: Fix navigation to First and Next discussion on MR Changes tab
-merge_request: 20434
-author:
-type: fixed

changelogs/unreleased/_acet-fix-expanding-context-lines.yml

----
-title: Fix rendering of the context lines in MR diffs page
-merge_request: 20642
-author:
-type: fixed

changelogs/unreleased/_acet-fix-mr-autosave.yml

----
-title: Fix autosave and ESC confirmation issues for MR discussions
-merge_request: 20569
-author:
-type: fixed

changelogs/unreleased/_acet-fix-outdated-discussions.yml

----
-title: Fix showing outdated discussions on Changes tab
-merge_request: 20445
-author:
-type: fixed

changelogs/unreleased/ide-edit-json-files.yml

----
-title: Fixed IDE not opening JSON files
-merge_request: 20798
-author:
-type: fixed

changelogs/unreleased/project-dropdown-list-overflow.yml

----
-title: Don't overflow project/group dropdown results
-merge_request: 20704
-author: gfyoung
-type: fixed

changelogs/unreleased/stop-dynamic-routable-creation.yml

+---
+title: Stop dynamically creating project and namespace routes
+merge_request: 20313
+author:
+type: performance

changelogs/unreleased/tc-reorder-mail-notify-references.yml

+---
+title: Put fallback reply-key address first in the References header
+merge_request: 20871
+author:
+type: changed

changelogs/unreleased/tz-mr-refactor-memory-reduction.yml

----
-title: Reduces the client side memory footprint on merge requests
-merge_request: 20744
-author:
-type: performance

changelogs/unreleased/zj-backup-timeout.yml

----
-title: Disable Gitaly timeouts when creating or restoring backups
-merge_request: 20810
-author:
-type: fixed

db/migrate/20180702124358_remove_orphaned_routes.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class RemoveOrphanedRoutes < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  class Route < ActiveRecord::Base
+    self.table_name = 'routes'
+    include EachBatch
+
+    def self.orphaned_namespace_routes
+      where(source_type: 'Namespace')
+        .where('NOT EXISTS ( SELECT 1 FROM namespaces WHERE namespaces.id = routes.source_id )')
+    end
+
+    def self.orphaned_project_routes
+      where(source_type: 'Project')
+        .where('NOT EXISTS ( SELECT 1 FROM projects WHERE projects.id = routes.source_id )')
+    end
+  end
+
+  def up
+    # Some of these queries can take up to 10 seconds to run on GitLab.com,
+    # which is pretty close to our 15 second statement timeout. To ensure a
+    # smooth deployment procedure we disable the statement timeouts for this
+    # migration, just in case.
+    disable_statement_timeout
+
+    # On GitLab.com there are around 4000 orphaned project routes, and around
+    # 150 orphaned namespace routes.
+    [
+      Route.orphaned_project_routes,
+      Route.orphaned_namespace_routes
+    ].each do |relation|
+      relation.each_batch(of: 1_000) do |batch|
+        batch.delete_all
+      end
+    end
+  end
+
+  def down
+    # There is no way to restore orphaned routes, and this doesn't make any
+    # sense anyway.
+  end
+end

db/migrate/20180702134423_generate_missing_routes.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+# This migration generates missing routes for any projects and namespaces that
+# don't already have a route.
+#
+# On GitLab.com this would insert 611 project routes, and 0 namespace routes.
+# The exact number could vary per instance, so we take care of both just in
+# case.
+class GenerateMissingRoutes < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  class User < ActiveRecord::Base
+    self.table_name = 'users'
+  end
+
+  class Route < ActiveRecord::Base
+    self.table_name = 'routes'
+  end
+
+  module Routable
+    def build_full_path
+      if parent && path
+        parent.build_full_path + '/' + path
+      else
+        path
+      end
+    end
+
+    def build_full_name
+      if parent && name
+        parent.human_name + ' / ' + name
+      else
+        name
+      end
+    end
+
+    def human_name
+      build_full_name
+    end
+
+    def attributes_for_insert
+      time = Time.zone.now
+
+      {
+        # We can't use "self.class.name" here as that would include the
+        # migration namespace.
+        source_type: source_type_for_route,
+        source_id: id,
+        created_at: time,
+        updated_at: time,
+        name: build_full_name,
+
+        # The route path might already be taken. Instead of trying to generate a
+        # new unique name on every conflict, we just append the row ID to the
+        # route path.
+        path: "#{build_full_path}-#{id}"
+      }
+    end
+  end
+
+  class Project < ActiveRecord::Base
+    self.table_name = 'projects'
+
+    include EachBatch
+    include GenerateMissingRoutes::Routable
+
+    belongs_to :namespace, class_name: 'GenerateMissingRoutes::Namespace'
+
+    has_one :route,
+      as: :source,
+      inverse_of: :source,
+      class_name: 'GenerateMissingRoutes::Route'
+
+    alias_method :parent, :namespace
+    alias_attribute :parent_id, :namespace_id
+
+    def self.without_routes
+      where(
+        'NOT EXISTS (
+          SELECT 1
+          FROM routes
+          WHERE source_type = ?
+          AND source_id = projects.id
+        )',
+        'Project'
+      )
+    end
+
+    def source_type_for_route
+      'Project'
+    end
+  end
+
+  class Namespace < ActiveRecord::Base
+    self.table_name = 'namespaces'
+
+    include EachBatch
+    include GenerateMissingRoutes::Routable
+
+    belongs_to :parent, class_name: 'GenerateMissingRoutes::Namespace'
+    belongs_to :owner, class_name: 'GenerateMissingRoutes::User'
+
+    has_one :route,
+      as: :source,
+      inverse_of: :source,
+      class_name: 'GenerateMissingRoutes::Route'
+
+    def self.without_routes
+      where(
+        'NOT EXISTS (
+          SELECT 1
+          FROM routes
+          WHERE source_type = ?
+          AND source_id = namespaces.id
+        )',
+        'Namespace'
+      )
+    end
+
+    def source_type_for_route
+      'Namespace'
+    end
+  end
+
+  def up
+    [Namespace, Project].each do |model|
+      model.without_routes.each_batch(of: 100) do |batch|
+        rows = batch.map(&:attributes_for_insert)
+
+        Gitlab::Database.bulk_insert(:routes, rows)
+      end
+    end
+  end
+
+  def down
+    # Removing routes we previously generated makes no sense.
+  end
+end

doc/api/merge_requests.md

@@ -15,11 +15,6 @@ given state (`opened`, `closed`, `locked`, or `merged`) or all of them (`all`).
 The pagination parameters `page` and `per_page` can be used to
 restrict the list of merge requests.
 
-**Note**: the `changes_count` value in the response is a string, not an
-integer. This is because when an MR has too many changes to display and store,
-it will be capped at 1,000. In that case, the API will return the string
-`"1000+"` for the changes count.
-
 ```
 GET /merge_requests
 GET /merge_requests?state=opened
@@ -104,7 +99,6 @@ Parameters:
     "sha": "8888888888888888888888888888888888888888",
     "merge_commit_sha": null,
     "user_notes_count": 1,
-    "changes_count": "1",
     "should_remove_source_branch": true,
     "force_remove_source_branch": false,
     "squash": false,
@@ -144,10 +138,6 @@ will be the same. In the case of a merge request from a fork,
 `target_project_id` and `project_id` will be the same and
 `source_project_id` will be the fork project's ID.
 
-**Note**: the `changes_count` value in the response is a string, not an
-integer. This is because when an MR has too many changes to display and store,
-it will be capped at 1,000. In that case, the API will return the string
-`"1000+"` for the changes count.
 
 Parameters:
 
@@ -224,7 +214,6 @@ Parameters:
     "sha": "8888888888888888888888888888888888888888",
     "merge_commit_sha": null,
     "user_notes_count": 1,
-    "changes_count": "1",
     "should_remove_source_branch": true,
     "force_remove_source_branch": false,
     "squash": false,
@@ -331,7 +320,6 @@ Parameters:
     "sha": "8888888888888888888888888888888888888888",
     "merge_commit_sha": null,
     "user_notes_count": 1,
-    "changes_count": "1",
     "should_remove_source_branch": true,
     "force_remove_source_branch": false,
     "web_url": "http://example.com/example/example/merge_requests/1",
@@ -350,6 +338,11 @@ Parameters:
 
 Shows information about a single merge request.
 
+**Note**: the `changes_count` value in the response is a string, not an
+integer. This is because when an MR has too many changes to display and store,
+it will be capped at 1,000. In that case, the API will return the string
+`"1000+"` for the changes count.
+
 ```
 GET /projects/:id/merge_requests/:merge_request_iid
 ```

spec/controllers/projects/pipelines_controller_spec.rb

@@ -55,10 +55,8 @@ describe Projects::PipelinesController do
         stub_feature_flags(ci_pipeline_persisted_stages: false)
       end
 
-      it 'returns JSON with serialized pipelines', :request_store do
-        queries = ActiveRecord::QueryRecorder.new do
-          get_pipelines_index_json
-        end
+      it 'returns JSON with serialized pipelines' do
+        get_pipelines_index_json
 
         expect(response).to have_gitlab_http_status(:ok)
         expect(response).to match_response_schema('pipeline')
@@ -73,8 +71,14 @@ describe Projects::PipelinesController do
         json_response.dig('pipelines', 0, 'details', 'stages').tap do |stages|
           expect(stages.count).to eq 3
         end
+      end
+
+      it 'does not execute N+1 queries' do
+        queries = ActiveRecord::QueryRecorder.new do
+          get_pipelines_index_json
+        end
 
-        expect(queries.count).to be_within(5).of(30)
+        expect(queries.count).to be <= 36
       end
     end
 

spec/fixtures/emails/commands_in_reply.eml

@@ -8,7 +8,7 @@ From: Jake the Dog <jake@adventuretime.ooo>
 To: reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo
 Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com>
 In-Reply-To: <issue_1@localhost>
-References: <issue_1@localhost> <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost>
+References: <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost> <issue_1@localhost>
 Subject: re: [Discourse Meta] eviltrout posted in 'Adventure Time Sux'
 Mime-Version: 1.0
 Content-Type: text/plain;

spec/fixtures/emails/commands_only_reply.eml

@@ -8,7 +8,7 @@ From: Jake the Dog <jake@adventuretime.ooo>
 To: reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo
 Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com>
 In-Reply-To: <issue_1@localhost>
-References: <issue_1@localhost> <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost>
+References: <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost> <issue_1@localhost>
 Subject: re: [Discourse Meta] eviltrout posted in 'Adventure Time Sux'
 Mime-Version: 1.0
 Content-Type: text/plain;

spec/fixtures/emails/reply_without_subaddressing_and_key_inside_references.eml

@@ -8,7 +8,7 @@ From: Jake the Dog <jake@adventuretime.ooo>
 To: reply@appmail.adventuretime.ooo
 Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com>
 In-Reply-To: <issue_1@localhost>
-References: <issue_1@localhost> <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost>
+References: <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost> <issue_1@localhost>
 Subject: re: [Discourse Meta] eviltrout posted in 'Adventure Time Sux'
 Mime-Version: 1.0
 Content-Type: text/plain;

spec/fixtures/emails/reply_without_subaddressing_and_key_inside_references_with_a_comma.eml

@@ -8,7 +8,7 @@ From: Jake the Dog <jake@adventuretime.ooo>
 To: reply@appmail.adventuretime.ooo
 Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com>
 In-Reply-To: <issue_1@localhost>
-References: <issue_1@localhost> <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost>,<exchange@microsoft.com>
+References: <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost> <issue_1@localhost>,<exchange@microsoft.com>
 Subject: re: [Discourse Meta] eviltrout posted in 'Adventure Time Sux'
 Mime-Version: 1.0
 Content-Type: text/plain;

spec/fixtures/emails/update_commands_only_reply.eml

@@ -8,7 +8,7 @@ From: Jake the Dog <jake@adventuretime.ooo>
 To: reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo
 Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com>
 In-Reply-To: <issue_1@localhost>
-References: <issue_1@localhost> <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost>
+References: <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost> <issue_1@localhost>
 Subject: re: [Discourse Meta] eviltrout posted in 'Adventure Time Sux'
 Mime-Version: 1.0
 Content-Type: text/plain;

spec/fixtures/emails/valid_reply.eml

@@ -8,7 +8,7 @@ From: Jake the Dog <jake@adventuretime.ooo>
 To: reply+59d8df8370b7e95c5a49fbf86aeb2c93@appmail.adventuretime.ooo
 Message-ID: <CADkmRc+rNGAGGbV2iE5p918UVy4UyJqVcXRO2=otppgzduJSg@mail.gmail.com>
 In-Reply-To: <issue_1@localhost>
-References: <issue_1@localhost> <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost>
+References: <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost> <issue_1@localhost>
 Subject: re: [Discourse Meta] eviltrout posted in 'Adventure Time Sux'
 Mime-Version: 1.0
 Content-Type: text/plain;

spec/javascripts/pipelines/graph/dropdown_job_component_spec.js

@@ -12,7 +12,7 @@ describe('dropdown job component', () => {
         id: 4256,
         name: '<img src=x onerror=alert(document.domain)>',
         status: {
-          icon: 'icon_status_success',
+          icon: 'status_success',
           text: 'passed',
           label: 'passed',
           tooltip: 'passed',
@@ -31,7 +31,7 @@ describe('dropdown job component', () => {
         id: 4299,
         name: 'test',
         status: {
-          icon: 'icon_status_success',
+          icon: 'status_success',
           text: 'passed',
           label: 'passed',
           tooltip: 'passed',
@@ -50,7 +50,7 @@ describe('dropdown job component', () => {
     name: 'rspec:linux',
     size: 2,
     status: {
-      icon: 'icon_status_success',
+      icon: 'status_success',
       text: 'passed',
       label: 'passed',
       tooltip: 'passed',

spec/javascripts/pipelines/graph/job_component_spec.js

@@ -169,7 +169,7 @@ describe('pipeline graph job component', () => {
           id: 4259,
           name: '<img src=x onerror=alert(document.domain)>',
           status: {
-            icon: 'icon_status_success',
+            icon: 'status_success',
             label: 'success',
             tooltip: 'failed',
           },

spec/migrations/generate_missing_routes_spec.rb

+require 'spec_helper'
+require Rails.root.join('db', 'migrate', '20180702134423_generate_missing_routes.rb')
+
+describe GenerateMissingRoutes, :migration do
+  describe '#up' do
+    let(:namespaces) { table(:namespaces) }
+    let(:projects) { table(:projects) }
+    let(:routes) { table(:routes) }
+
+    it 'creates routes for projects without a route' do
+      namespace = namespaces.create!(name: 'GitLab', path: 'gitlab')
+
+      routes.create!(
+        path: 'gitlab',
+        source_type: 'Namespace',
+        source_id: namespace.id
+      )
+
+      project = projects.create!(
+        name: 'GitLab CE',
+        path: 'gitlab-ce',
+        namespace_id: namespace.id
+      )
+
+      described_class.new.up
+
+      route = routes.where(source_type: 'Project').take
+
+      expect(route.source_id).to eq(project.id)
+      expect(route.path).to eq("gitlab/gitlab-ce-#{project.id}")
+    end
+
+    it 'creates routes for namespaces without a route' do
+      namespace = namespaces.create!(name: 'GitLab', path: 'gitlab')
+
+      described_class.new.up
+
+      route = routes.where(source_type: 'Namespace').take
+
+      expect(route.source_id).to eq(namespace.id)
+      expect(route.path).to eq("gitlab-#{namespace.id}")
+    end
+
+    it 'does not create routes for namespaces that already have a route' do
+      namespace = namespaces.create!(name: 'GitLab', path: 'gitlab')
+
+      routes.create!(
+        path: 'gitlab',
+        source_type: 'Namespace',
+        source_id: namespace.id
+      )
+
+      described_class.new.up
+
+      expect(routes.count).to eq(1)
+    end
+
+    it 'does not create routes for projects that already have a route' do
+      namespace = namespaces.create!(name: 'GitLab', path: 'gitlab')
+
+      routes.create!(
+        path: 'gitlab',
+        source_type: 'Namespace',
+        source_id: namespace.id
+      )
+
+      project = projects.create!(
+        name: 'GitLab CE',
+        path: 'gitlab-ce',
+        namespace_id: namespace.id
+      )
+
+      routes.create!(
+        path: 'gitlab/gitlab-ce',
+        source_type: 'Project',
+        source_id: project.id
+      )
+
+      described_class.new.up
+
+      expect(routes.count).to eq(2)
+    end
+  end
+end

spec/models/concerns/routable_spec.rb

@@ -12,16 +12,6 @@ describe Group, 'Routable' do
     it { is_expected.to have_many(:redirect_routes).dependent(:destroy) }
   end
 
-  describe 'GitLab read-only instance' do
-    it 'does not save route if route is not present' do
-      group.route.path = ''
-      allow(Gitlab::Database).to receive(:read_only?).and_return(true)
-      expect(group).to receive(:update_route).and_call_original
-
-      expect { group.full_path }.to change { Route.count }.by(0)
-    end
-  end
-
   describe 'Callbacks' do
     it 'creates route record on create' do
       expect(group.route.path).to eq(group.path)
@@ -131,29 +121,6 @@ describe Group, 'Routable' do
 
     it { expect(group.full_path).to eq(group.path) }
     it { expect(nested_group.full_path).to eq("#{group.full_path}/#{nested_group.path}") }
-
-    context 'with RequestStore active', :request_store do
-      it 'does not load the route table more than once' do
-        group.expires_full_path_cache
-        expect(group).to receive(:uncached_full_path).once.and_call_original
-
-        3.times { group.full_path }
-        expect(group.full_path).to eq(group.path)
-      end
-    end
-  end
-
-  describe '#expires_full_path_cache' do
-    context 'with RequestStore active', :request_store do
-      it 'expires the full_path cache' do
-        expect(group.full_path).to eq('foo')
-
-        group.route.update(path: 'bar', name: 'bar')
-        group.expires_full_path_cache
-
-        expect(group.full_path).to eq('bar')
-      end
-    end
   end
 
   describe '#full_name' do

spec/models/namespace_spec.rb

@@ -323,6 +323,16 @@ describe Namespace do
 
       parent.update(path: 'mygroup_new')
 
+      # Routes are loaded when creating the projects, so we need to manually
+      # reload them for the below code to be aware of the above UPDATE.
+      [
+        project_in_parent_group,
+        hashed_project_in_subgroup,
+        legacy_project_in_subgroup
+      ].each do |project|
+        project.route.reload
+      end
+
       expect(project_rugged(project_in_parent_group).config['gitlab.fullpath']).to eq "mygroup_new/#{project_in_parent_group.path}"
       expect(project_rugged(hashed_project_in_subgroup).config['gitlab.fullpath']).to eq "mygroup_new/mysubgroup/#{hashed_project_in_subgroup.path}"
       expect(project_rugged(legacy_project_in_subgroup).config['gitlab.fullpath']).to eq "mygroup_new/mysubgroup/#{legacy_project_in_subgroup.path}"

spec/models/project_spec.rb

@@ -2958,8 +2958,6 @@ describe Project do
 
         expect(project).to receive(:expire_caches_before_rename)
 
-        expect(project).to receive(:expires_full_path_cache)
-
         project.rename_repo
       end
 
@@ -3119,8 +3117,6 @@ describe Project do
 
         expect(project).to receive(:expire_caches_before_rename)
 
-        expect(project).to receive(:expires_full_path_cache)
-
         project.rename_repo
       end
 

spec/serializers/pipeline_serializer_spec.rb

@@ -125,7 +125,7 @@ describe PipelineSerializer do
         it 'verifies number of queries', :request_store do
           recorded = ActiveRecord::QueryRecorder.new { subject }
 
-          expect(recorded.count).to be_within(2).of(27)
+          expect(recorded.count).to be_within(2).of(31)
           expect(recorded.cached_count).to eq(0)
         end
       end
@@ -144,7 +144,7 @@ describe PipelineSerializer do
           # pipeline. With the same ref this check is cached but if refs are
           # different then there is an extra query per ref
           # https://gitlab.com/gitlab-org/gitlab-ce/issues/46368
-          expect(recorded.count).to be_within(2).of(30)
+          expect(recorded.count).to be_within(2).of(34)
           expect(recorded.cached_count).to eq(0)
         end
       end

spec/services/projects/transfer_service_spec.rb

@@ -31,12 +31,6 @@ describe Projects::TransferService do
       transfer_project(project, user, group)
     end
 
-    it 'expires full_path cache' do
-      expect(project).to receive(:expires_full_path_cache)
-
-      transfer_project(project, user, group)
-    end
-
     it 'invalidates the user\'s personal_project_count cache' do
       expect(user).to receive(:invalidate_personal_projects_count)
 

spec/support/shared_examples/notify_shared_examples.rb

@@ -77,7 +77,7 @@ shared_examples 'a thread answer email with reply-by-email enabled' do
     aggregate_failures do
       is_expected.to have_header('Message-ID', /\A<.*@#{host}>\Z/)
       is_expected.to have_header('In-Reply-To', "<#{route_key}@#{host}>")
-      is_expected.to have_header('References',  /\A<#{route_key}@#{host}> <reply\-.*@#{host}>\Z/ )
+      is_expected.to have_header('References', /\A<reply\-.*@#{host}> <#{route_key}@#{host}>\Z/ )
       is_expected.to have_subject(/^Re: /)
     end
   end