Merge branch 'ce-to-ee-2018-06-18' into 'master'

CE upstream - 2018-06-18 09:22 UTC

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

Gemfile.rails5.lock

@@ -323,7 +323,7 @@ GEM
       flowdock (~> 0.7)
       gitlab-grit (>= 2.4.1)
       multi_json
-    gitlab-gollum-lib (4.2.7.2)
+    gitlab-gollum-lib (4.2.7.4)
       gemojione (~> 3.2)
       github-markup (~> 1.6)
       gollum-grit_adapter (~> 1.0)
@@ -331,7 +331,7 @@ GEM
       rouge (~> 3.1)
       sanitize (~> 2.1)
       stringex (~> 2.6)
-    gitlab-gollum-rugged_adapter (0.4.4)
+    gitlab-gollum-rugged_adapter (0.4.4.1)
       mime-types (>= 1.15)
       rugged (~> 0.25)
     gitlab-grit (2.8.2)

app/assets/javascripts/milestone_select.js

@@ -16,10 +16,10 @@ export default class MilestoneSelect {
         typeof currentProject === 'string' ? JSON.parse(currentProject) : currentProject;
     }
 
-    this.init(els, options);
+    MilestoneSelect.init(els, options);
   }
 
-  init(els, options) {
+  static init(els, options) {
     let $els = $(els);
 
     if (!els) {
@@ -224,7 +224,6 @@ export default class MilestoneSelect {
                 $selectBox.hide();
                 $value.css('display', '');
                 if (data.milestone != null) {
-                  data.milestone.full_path = this.currentProject.full_path;
                   data.milestone.remaining = timeFor(data.milestone.due_date);
                   data.milestone.name = data.milestone.title;
                   $value.html(milestoneLinkTemplate(data.milestone));

app/controllers/projects/blob_controller.rb

@@ -7,6 +7,7 @@ class Projects::BlobController < Projects::ApplicationController
 
   prepend_before_action :authenticate_user!, only: [:edit]
 
+  before_action :set_request_format, only: [:edit, :show, :update]
   before_action :require_non_empty_project, except: [:new, :create]
   before_action :authorize_download_code!
 
@@ -188,6 +189,18 @@ class Projects::BlobController < Projects::ApplicationController
       .last_for_path(@repository, @ref, @path).sha
   end
 
+  # In Rails 4.2 if params[:format] is empty, Rails set it to :html
+  # But since Rails 5.0 the framework now looks for an extension.
+  # E.g. for `blob/master/CHANGELOG.md` in Rails 4 the format would be `:html`, but in Rails 5 on it'd be `:md`
+  # This before_action explicitly sets the `:html` format for all requests unless `:format` is set by a client e.g. by JS for XHR requests.
+  def set_request_format
+    request.format = :html if set_request_format?
+  end
+
+  def set_request_format?
+    params[:id].present? && params[:format].blank? && request.format != "json"
+  end
+
   def show_html
     environment_params = @repository.branch_exists?(@ref) ? { ref: @ref } : { commit: @commit }
     @environment = EnvironmentsFinder.new(@project, current_user, environment_params).execute.last

app/controllers/projects/merge_requests_controller.rb

@@ -118,7 +118,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
       end
 
       format.json do
-        render json: @merge_request.to_json(include: { milestone: {}, assignee: { only: [:name, :username], methods: [:avatar_url] }, labels: { methods: :text_color } }, methods: [:task_status, :task_status_short])
+        render json: serializer.represent(@merge_request, serializer: 'basic')
       end
     end
   rescue ActiveRecord::StaleObjectError

app/serializers/merge_request_basic_entity.rb

@@ -5,4 +5,8 @@ class MergeRequestBasicEntity < IssuableSidebarEntity
   expose :state
   expose :source_branch_exists?, as: :source_branch_exists
   expose :rebase_in_progress?, as: :rebase_in_progress
+  expose :milestone, using: API::Entities::Milestone
+  expose :labels, using: LabelEntity
+  expose :assignee, using: API::Entities::UserBasic
+  expose :task_status, :task_status_short
 end

changelogs/unreleased/blackst0ne-rails5-expected-search-search-seed_project-got-nil.yml

+---
+title: "[Rails5] Fix sessions_controller_spec"
+merge_request: 19936
+author: "@blackst0ne"
+type: fixed

changelogs/unreleased/blackst0ne-rails5-fix-blob-requests-format.yml

+---
+title: "[Rails5] Explicitly set request.format for blob_controller"
+merge_request: 19876
+author: "@blackst0ne"
+type: fixed

changelogs/unreleased/blackst0ne-rails5-fix-pipeline-schedules-controller-spec.yml

+---
+title: "[Rails5] Fix pipeline_schedules_controller_spec"
+merge_request: 19919
+author: "@blackst0ne"
+type: fixed

changelogs/unreleased/rails5-fix-47960.yml

+---
+title: Rails5 fix update_attribute usage not causing a save
+merge_request: 19881
+author: Jasper Maes
+type: fixed

changelogs/unreleased/rails5-fix-48009.yml

+---
+title: Rails5 update Gemfile.rails5.lock
+merge_request: 19921
+author: Jasper Maes
+type: fixed

lib/gitlab/git/wiki.rb

@@ -27,63 +27,38 @@ module Gitlab
       end
 
       def write_page(name, format, content, commit_details)
-        @repository.gitaly_migrate(:wiki_write_page) do |is_enabled|
-          if is_enabled
+        @repository.wrapped_gitaly_errors do
           gitaly_write_page(name, format, content, commit_details)
-          else
-            gollum_write_page(name, format, content, commit_details)
-          end
         end
       end
 
       def delete_page(page_path, commit_details)
-        @repository.gitaly_migrate(:wiki_delete_page) do |is_enabled|
-          if is_enabled
+        @repository.wrapped_gitaly_errors do
           gitaly_delete_page(page_path, commit_details)
-          else
-            gollum_delete_page(page_path, commit_details)
-          end
         end
       end
 
       def update_page(page_path, title, format, content, commit_details)
-        @repository.gitaly_migrate(:wiki_update_page) do |is_enabled|
-          if is_enabled
+        @repository.wrapped_gitaly_errors do
           gitaly_update_page(page_path, title, format, content, commit_details)
-          else
-            gollum_update_page(page_path, title, format, content, commit_details)
-          end
         end
       end
 
       def pages(limit: nil)
-        @repository.gitaly_migrate(:wiki_get_all_pages) do |is_enabled|
-          if is_enabled
+        @repository.wrapped_gitaly_errors do
           gitaly_get_all_pages
-          else
-            gollum_get_all_pages(limit: limit)
-          end
         end
       end
 
       def page(title:, version: nil, dir: nil)
-        @repository.gitaly_migrate(:wiki_find_page,
-                      status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled|
-          if is_enabled
+        @repository.wrapped_gitaly_errors do
           gitaly_find_page(title: title, version: version, dir: dir)
-          else
-            gollum_find_page(title: title, version: version, dir: dir)
-          end
         end
       end
 
       def file(name, version)
-        @repository.gitaly_migrate(:wiki_find_file) do |is_enabled|
-          if is_enabled
+        @repository.wrapped_gitaly_errors do
           gitaly_find_file(name, version)
-          else
-            gollum_find_file(name, version)
-          end
         end
       end
 
@@ -92,24 +67,15 @@ module Gitlab
       #  :per_page - The number of items per page.
       #  :limit    - Total number of items to return.
       def page_versions(page_path, options = {})
-        @repository.gitaly_migrate(:wiki_page_versions) do |is_enabled|
-          if is_enabled
-            versions = gitaly_wiki_client.page_versions(page_path, options)
+        versions = @repository.wrapped_gitaly_errors do
+          gitaly_wiki_client.page_versions(page_path, options)
+        end
 
         # Gitaly uses gollum-lib to get the versions. Gollum defaults to 20
         # per page, but also fetches 20 if `limit` or `per_page` < 20.
         # Slicing returns an array with the expected number of items.
         slice_bound = options[:limit] || options[:per_page] || Gollum::Page.per_page
         versions[0..slice_bound]
-          else
-            current_page = gollum_page_by_path(page_path)
-
-            commits_from_page(current_page, options).map do |gitlab_git_commit|
-              gollum_page = gollum_wiki.page(current_page.title, gitlab_git_commit.id)
-              Gitlab::Git::WikiPageVersion.new(gitlab_git_commit, gollum_page&.format)
-            end
-          end
-        end
       end
 
       def count_page_versions(page_path)
@@ -131,46 +97,13 @@ module Gitlab
       def page_formatted_data(title:, dir: nil, version: nil)
         version = version&.id
 
-        @repository.gitaly_migrate(:wiki_page_formatted_data, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled|
-          if is_enabled
+        @repository.wrapped_gitaly_errors do
           gitaly_wiki_client.get_formatted_data(title: title, dir: dir, version: version)
-          else
-            # We don't use #page because if wiki_find_page feature is enabled, we would
-            # get a page without formatted_data.
-            gollum_find_page(title: title, dir: dir, version: version)&.formatted_data
-          end
-        end
         end
-
-      def gollum_wiki
-        @gollum_wiki ||= Gollum::Wiki.new(@repository.path)
       end
 
       private
 
-      # options:
-      #  :page     - The Integer page number.
-      #  :per_page - The number of items per page.
-      #  :limit    - Total number of items to return.
-      def commits_from_page(gollum_page, options = {})
-        unless options[:limit]
-          options[:offset] = ([1, options.delete(:page).to_i].max - 1) * Gollum::Page.per_page
-          options[:limit] = (options.delete(:per_page) || Gollum::Page.per_page).to_i
-        end
-
-        @repository.log(ref: gollum_page.last_version.id,
-                        path: gollum_page.path,
-                        limit: options[:limit],
-                        offset: options[:offset])
-      end
-
-      def gollum_page_by_path(page_path)
-        page_name = Gollum::Page.canonicalize_filename(page_path)
-        page_dir = File.split(page_path).first
-
-        gollum_wiki.paged(page_name, page_dir)
-      end
-
       def new_page(gollum_page)
         Gitlab::Git::WikiPage.new(gollum_page, new_version(gollum_page, gollum_page.version.id))
       end
@@ -199,65 +132,6 @@ module Gitlab
         @gitaly_wiki_client ||= Gitlab::GitalyClient::WikiService.new(@repository)
       end
 
-      def gollum_write_page(name, format, content, commit_details)
-        assert_type!(format, Symbol)
-        assert_type!(commit_details, CommitDetails)
-
-        with_committer_with_hooks(commit_details) do |committer|
-          filename = File.basename(name)
-          dir = (tmp_dir = File.dirname(name)) == '.' ? '' : tmp_dir
-
-          gollum_wiki.write_page(filename, format, content, { committer: committer }, dir)
-        end
-      rescue Gollum::DuplicatePageError => e
-        raise Gitlab::Git::Wiki::DuplicatePageError, e.message
-      end
-
-      def gollum_delete_page(page_path, commit_details)
-        assert_type!(commit_details, CommitDetails)
-
-        with_committer_with_hooks(commit_details) do |committer|
-          gollum_wiki.delete_page(gollum_page_by_path(page_path), committer: committer)
-        end
-      end
-
-      def gollum_update_page(page_path, title, format, content, commit_details)
-        assert_type!(format, Symbol)
-        assert_type!(commit_details, CommitDetails)
-
-        with_committer_with_hooks(commit_details) do |committer|
-          page = gollum_page_by_path(page_path)
-          # Instead of performing two renames if the title has changed,
-          # the update_page will only update the format and content and
-          # the rename_page will do anything related to moving/renaming
-          gollum_wiki.update_page(page, page.name, format, content, committer: committer)
-          gollum_wiki.rename_page(page, title, committer: committer)
-        end
-      end
-
-      def gollum_find_page(title:, version: nil, dir: nil)
-        if version
-          version = Gitlab::Git::Commit.find(@repository, version).id
-        end
-
-        gollum_page = gollum_wiki.page(title, version, dir)
-        return unless gollum_page
-
-        new_page(gollum_page)
-      end
-
-      def gollum_find_file(name, version)
-        version ||= self.class.default_ref
-        gollum_file = gollum_wiki.file(name, version)
-        return unless gollum_file
-
-        Gitlab::Git::WikiFile.new(gollum_file)
-      end
-
-      def gollum_get_all_pages(limit: nil)
-        gollum_wiki.pages(limit: limit).map { |gollum_page| new_page(gollum_page) }
-      end
-
       def gitaly_write_page(name, format, content, commit_details)
         gitaly_wiki_client.write_page(name, format, content, commit_details)
       end

lib/gitlab/profiler.rb

@@ -11,6 +11,7 @@ module Gitlab
       lib/gitlab/etag_caching/
       lib/gitlab/metrics/
       lib/gitlab/middleware/
+      ee/lib/gitlab/middleware/
       lib/gitlab/performance_bar/
       lib/gitlab/request_profiler/
       lib/gitlab/profiler.rb
@@ -98,11 +99,7 @@ module Gitlab
 
             super
 
-            backtrace = Rails.backtrace_cleaner.clean(caller)
-
-            backtrace.each do |caller_line|
-              next if caller_line.match(Regexp.union(IGNORE_BACKTRACES))
-
+            Gitlab::Profiler.clean_backtrace(caller).each do |caller_line|
               stripped_caller_line = caller_line.sub("#{Rails.root}/", '')
 
               super("  ↳ #{stripped_caller_line}")
@@ -112,6 +109,12 @@ module Gitlab
       end
     end
 
+    def self.clean_backtrace(backtrace)
+      Array(Rails.backtrace_cleaner.clean(backtrace)).reject do |line|
+        line.match(Regexp.union(IGNORE_BACKTRACES))
+      end
+    end
+
     def self.with_custom_logger(logger)
       original_colorize_logging = ActiveSupport::LogSubscriber.colorize_logging
       original_activerecord_logger = ActiveRecord::Base.logger

spec/controllers/projects/imports_controller_spec.rb

@@ -29,7 +29,7 @@ describe Projects::ImportsController do
 
       context 'when import is in progress' do
         before do
-          project.update_attribute(:import_status, :started)
+          project.update_attributes(import_status: :started)
         end
 
         it 'renders template' do
@@ -47,7 +47,7 @@ describe Projects::ImportsController do
 
       context 'when import failed' do
         before do
-          project.update_attribute(:import_status, :failed)
+          project.update_attributes(import_status: :failed)
         end
 
         it 'redirects to new_namespace_project_import_path' do
@@ -59,7 +59,7 @@ describe Projects::ImportsController do
 
       context 'when import finished' do
         before do
-          project.update_attribute(:import_status, :finished)
+          project.update_attributes(import_status: :finished)
         end
 
         context 'when project is a fork' do
@@ -108,7 +108,7 @@ describe Projects::ImportsController do
 
       context 'when import never happened' do
         before do
-          project.update_attribute(:import_status, :none)
+          project.update_attributes(import_status: :none)
         end
 
         it 'redirects to namespace_project_path' do

spec/controllers/projects/merge_requests_controller_spec.rb

@@ -234,7 +234,7 @@ describe Projects::MergeRequestsController do
         body = JSON.parse(response.body)
 
         expect(body['assignee'].keys)
-          .to match_array(%w(name username avatar_url))
+          .to match_array(%w(name username avatar_url id state web_url))
       end
     end
 

spec/controllers/projects/pipeline_schedules_controller_spec.rb

@@ -310,11 +310,21 @@ describe Projects::PipelineSchedulesController do
     end
 
     def go
+      if Gitlab.rails5?
+        put :update, params: { namespace_id: project.namespace.to_param,
+                               project_id: project,
+                               id: pipeline_schedule,
+                               schedule: schedule },
+                     as: :html
+
+      else
         put :update, namespace_id: project.namespace.to_param,
-                   project_id: project, id: pipeline_schedule,
+                     project_id: project,
+                     id: pipeline_schedule,
                      schedule: schedule
       end
     end
+  end
 
   describe 'GET #edit' do
     describe 'functionality' do

spec/controllers/sessions_controller_spec.rb

@@ -257,15 +257,15 @@ describe SessionsController do
     end
   end
 
-  describe '#new' do
+  describe "#new" do
     before do
       set_devise_mapping(context: @request)
     end
 
-    it 'redirects correctly for referer on same host with params' do
-      search_path = '/search?search=seed_project'
-      allow(controller.request).to receive(:referer)
-        .and_return('http://%{host}%{path}' % { host: 'test.host', path: search_path })
+    it "redirects correctly for referer on same host with params" do
+      host = "test.host"
+      search_path = "/search?search=seed_project"
+      request.headers[:HTTP_REFERER] = "http://#{host}#{search_path}"
 
       get(:new, redirect_to_referer: :yes)
 

spec/fixtures/api/schemas/entities/merge_request_basic.json

@@ -14,7 +14,21 @@
     "subscribed": { "type": ["boolean", "null"] },
     "participants": { "type": "array" },
     "allow_collaboration": { "type": "boolean"},
-    "allow_maintainer_to_push": { "type": "boolean"}
+    "allow_maintainer_to_push": { "type": "boolean"},
+    "assignee": {
+      "oneOf": [
+        { "type": "null" },
+        { "$ref": "user.json" }
+      ]
+    },
+    "milestone": {
+      "type": [ "object", "null" ]
+    },
+    "labels": {
+      "type": [ "array", "null" ]
+    },
+    "task_status": { "type": "string" },
+    "task_status_short": { "type": "string" }
   },
   "additionalProperties": false
 }

spec/lib/gitlab/git/committer_with_hooks_spec.rb

 require 'spec_helper'
 
 describe Gitlab::Git::CommitterWithHooks, seed_helper: true do
+  # TODO https://gitlab.com/gitlab-org/gitaly/issues/1234
+  skip 'needs to be moved to gitaly-ruby test suite' do
     shared_examples 'calling wiki hooks' do
       let(:project) { create(:project) }
       let(:user) { project.owner }
@@ -143,12 +145,12 @@ describe Gitlab::Git::CommitterWithHooks, seed_helper: true do
       end
     end
 
-  # TODO: Uncomment once Gitaly updates the ruby vendor code
-  # context 'when Gitaly is enabled' do
-  #   it_behaves_like 'calling wiki hooks'
-  # end
+    context 'when Gitaly is enabled' do
+      it_behaves_like 'calling wiki hooks'
+    end
 
-  context 'when Gitaly is disabled', :skip_gitaly_mock do
+    context 'when Gitaly is disabled', :disable_gitaly do
       it_behaves_like 'calling wiki hooks'
     end
+  end
 end

spec/lib/gitlab/git/wiki_spec.rb

@@ -6,9 +6,7 @@ describe Gitlab::Git::Wiki do
   let(:project_wiki) { ProjectWiki.new(project, user) }
   subject { project_wiki.wiki }
 
-  # Remove skip_gitaly_mock flag when gitaly_find_page when
-  # https://gitlab.com/gitlab-org/gitlab-ce/issues/42039 is solved
-  describe '#page', :skip_gitaly_mock do
+  describe '#page' do
     before do
       create_page('page1', 'content')
       create_page('foo/page1', 'content foo/page1')
@@ -25,7 +23,7 @@ describe Gitlab::Git::Wiki do
     end
   end
 
-  describe '#delete_page', :skip_gitaly_mock do
+  describe '#delete_page' do
     after do
       destroy_page('page1')
     end

spec/lib/gitlab/profiler_spec.rb

@@ -135,6 +135,51 @@ describe Gitlab::Profiler do
     end
   end
 
+  describe '.clean_backtrace' do
+    it 'uses the Rails backtrace cleaner' do
+      backtrace = []
+
+      expect(Rails.backtrace_cleaner).to receive(:clean).with(backtrace)
+
+      described_class.clean_backtrace(backtrace)
+    end
+
+    it 'removes lines from IGNORE_BACKTRACES' do
+      backtrace = [
+        "lib/gitlab/gitaly_client.rb:294:in `block (2 levels) in migrate'",
+        "lib/gitlab/gitaly_client.rb:331:in `allow_n_plus_1_calls'",
+        "lib/gitlab/gitaly_client.rb:280:in `block in migrate'",
+        "lib/gitlab/metrics/influx_db.rb:103:in `measure'",
+        "lib/gitlab/gitaly_client.rb:278:in `migrate'",
+        "lib/gitlab/git/repository.rb:1451:in `gitaly_migrate'",
+        "lib/gitlab/git/commit.rb:66:in `find'",
+        "app/models/repository.rb:1047:in `find_commit'",
+        "lib/gitlab/metrics/instrumentation.rb:159:in `block in find_commit'",
+        "lib/gitlab/metrics/method_call.rb:36:in `measure'",
+        "lib/gitlab/metrics/instrumentation.rb:159:in `find_commit'",
+        "app/models/repository.rb:113:in `commit'",
+        "lib/gitlab/i18n.rb:50:in `with_locale'",
+        "lib/gitlab/middleware/multipart.rb:95:in `call'",
+        "lib/gitlab/request_profiler/middleware.rb:14:in `call'",
+        "ee/lib/gitlab/database/load_balancing/rack_middleware.rb:37:in `call'",
+        "ee/lib/gitlab/jira/middleware.rb:15:in `call'"
+      ]
+
+      expect(described_class.clean_backtrace(backtrace))
+        .to eq([
+                 "lib/gitlab/gitaly_client.rb:294:in `block (2 levels) in migrate'",
+                 "lib/gitlab/gitaly_client.rb:331:in `allow_n_plus_1_calls'",
+                 "lib/gitlab/gitaly_client.rb:280:in `block in migrate'",
+                 "lib/gitlab/gitaly_client.rb:278:in `migrate'",
+                 "lib/gitlab/git/repository.rb:1451:in `gitaly_migrate'",
+                 "lib/gitlab/git/commit.rb:66:in `find'",
+                 "app/models/repository.rb:1047:in `find_commit'",
+                 "app/models/repository.rb:113:in `commit'",
+                 "ee/lib/gitlab/jira/middleware.rb:15:in `call'"
+               ])
+    end
+  end
+
   describe '.with_custom_logger' do
     context 'when the logger is set' do
       it 'uses the replacement logger for the duration of the block' do

spec/workers/every_sidekiq_worker_spec.rb

@@ -15,7 +15,7 @@ describe 'Every Sidekiq worker' do
     file_worker_queues = Gitlab::SidekiqConfig.worker_queues.to_set
 
     worker_queues = Gitlab::SidekiqConfig.workers.map(&:queue).to_set
-    worker_queues << ActionMailer::DeliveryJob.queue_name
+    worker_queues << ActionMailer::DeliveryJob.new.queue_name
     worker_queues << 'default'
 
     missing_from_file = worker_queues - file_worker_queues - DEPRECATED_QUEUES