Merge branch 'ce-to-ee-2018-01-30' into 'master'

CE upstream - 2018-01-30 21:25 UTC

Closes gitaly#958 et gitaly#784

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

Gemfile

@@ -142,7 +142,7 @@ gem 'asciidoctor-plantuml', '0.0.7'
 gem 'rouge', '~> 2.0'
 gem 'truncato', '~> 0.7.9'
 gem 'bootstrap_form', '~> 2.7.0'
-gem 'nokogiri', '~> 1.8.1'
+gem 'nokogiri', '~> 1.8.2'
 
 # Diffs
 gem 'diffy', '~> 3.1.0'

Gemfile.lock

@@ -542,7 +542,7 @@ GEM
     net-ntp (2.1.3)
     net-ssh (4.1.0)
     netrc (0.11.0)
-    nokogiri (1.8.1)
+    nokogiri (1.8.2)
       mini_portile2 (~> 2.3.0)
     numerizer (0.1.1)
     oauth (0.5.1)
@@ -1138,7 +1138,7 @@ DEPENDENCIES
   net-ldap
   net-ntp
   net-ssh (~> 4.1.0)
-  nokogiri (~> 1.8.1)
+  nokogiri (~> 1.8.2)
   oauth2 (~> 1.4)
   octokit (~> 4.6.2)
   oj (~> 2.17.4)

app/assets/javascripts/users/activity_calendar.js

@@ -98,7 +98,7 @@ export default class ActivityCalendar {
     const secondLastColMonth = this.timestampsTmp[group - 2][0].date.getMonth();
 
     if (lastColMonth !== secondLastColMonth) {
-      extraWidthPadding = 3;
+      extraWidthPadding = 6;
     }
 
     return extraWidthPadding;

app/models/concerns/discussion_on_diff.rb

@@ -37,6 +37,8 @@ module DiscussionOnDiff
 
   # Returns an array of at most 16 highlighted lines above a diff note
   def truncated_diff_lines(highlight: true)
+    return [] if diff_line.nil? && first_note.is_a?(LegacyDiffNote)
+
     lines = highlight ? highlighted_diff_lines : diff_lines
 
     initial_line_index = [diff_line.index - NUMBER_OF_TRUNCATED_DIFF_LINES + 1, 0].max

app/models/project_wiki.rb

@@ -139,6 +139,12 @@ class ProjectWiki
     update_project_activity
   end
 
+  def page_formatted_data(page)
+    page_title, page_dir = page_title_and_dir(page.title)
+
+    wiki.page_formatted_data(title: page_title, dir: page_dir, version: page.version)
+  end
+
   def page_title_and_dir(title)
     title_array = title.split("/")
     title = title_array.pop

app/models/repository.rb

@@ -262,6 +262,8 @@ class Repository
 
     # This will still fail if the file is corrupted (e.g. 0 bytes)
     raw_repository.write_ref(keep_around_ref_name(sha), sha, shell: false)
+  rescue Gitlab::Git::CommandError => ex
+    Rails.logger.error "Unable to create keep-around reference for repository #{path}: #{ex}"
   end
 
   def kept_around?(sha)

app/models/wiki_page.rb

@@ -107,7 +107,10 @@ class WikiPage
 
   # The processed/formatted content of this page.
   def formatted_content
-    @attributes[:formatted_content] ||= @page&.formatted_data
+    # Assuming @page exists, nil formatted_data means we didn't load it
+    # before hand (i.e. page was fetched by Gitaly), so we fetch it separately.
+    # If the page was fetched by Gollum, formatted_data would've been a String.
+    @attributes[:formatted_content] ||= @page&.formatted_data || @wiki.page_formatted_data(@page)
   end
 
   # The markup format for the page.

changelogs/unreleased/21554-mark-new-user-as-external.yml

+---
+title: Login via OAuth now only marks new users as external
+merge_request: 16672
+author:
+type: fixed

changelogs/unreleased/42160-error-500-loading-merge-request-undefined-method-index-for-nil-nilclass.yml

+---
+title: Fix 500 error when loading a merge request with an invalid comment
+merge_request: 16795
+author:
+type: fixed

changelogs/unreleased/42591-update-nokogiri.yml

+---
+title: Update nokogiri to 1.8.2
+merge_request: 16807
+author:
+type: security

changelogs/unreleased/contribution_calendar_label_cut_off.yml

+---
+title: Contribution calendar label was cut off
+merge_request:
+author: Branka Martinovic
+type: fixed

lib/gitlab/ee_compat_check.rb

@@ -5,7 +5,7 @@ module Gitlab
     DEFAULT_CE_PROJECT_URL = 'https://gitlab.com/gitlab-org/gitlab-ce'.freeze
     EE_REPO_URL = 'https://gitlab.com/gitlab-org/gitlab-ee.git'.freeze
     CHECK_DIR = Rails.root.join('ee_compat_check')
-    IGNORED_FILES_REGEX = /(VERSION|CHANGELOG\.md:\d+)/.freeze
+    IGNORED_FILES_REGEX = %r{VERSION|CHANGELOG\.md|db/schema\.rb}i.freeze
     PLEASE_READ_THIS_BANNER = %Q{
       ============================================================
       ===================== PLEASE READ THIS =====================

lib/gitlab/git/repository.rb

@@ -1106,10 +1106,14 @@ module Gitlab
       end
 
       def write_ref(ref_path, ref, old_ref: nil, shell: true)
-        if shell
-          shell_write_ref(ref_path, ref, old_ref)
-        else
-          rugged_write_ref(ref_path, ref)
+        ref_path = "#{Gitlab::Git::BRANCH_REF_PREFIX}#{ref_path}" unless ref_path.start_with?("refs/") || ref_path == "HEAD"
+
+        gitaly_migrate(:write_ref) do |is_enabled|
+          if is_enabled
+            gitaly_repository_client.write_ref(ref_path, ref, old_ref, shell)
+          else
+            local_write_ref(ref_path, ref, old_ref: old_ref, shell: shell)
+          end
         end
       end
 
@@ -1433,6 +1437,14 @@ module Gitlab
 
       private
 
+      def local_write_ref(ref_path, ref, old_ref: nil, shell: true)
+        if shell
+          shell_write_ref(ref_path, ref, old_ref)
+        else
+          rugged_write_ref(ref_path, ref)
+        end
+      end
+
       def shell_write_ref(ref_path, ref, old_ref)
         raise ArgumentError, "invalid ref_path #{ref_path.inspect}" if ref_path.include?(' ')
         raise ArgumentError, "invalid ref #{ref.inspect}" if ref.include?("\x00")

lib/gitlab/git/wiki.rb

@@ -117,6 +117,20 @@ module Gitlab
         page.url_path
       end
 
+      def page_formatted_data(title:, dir: nil, version: nil)
+        version = version&.id
+
+        @repository.gitaly_migrate(:wiki_page_formatted_data) do |is_enabled|
+          if is_enabled
+            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
+
       private
 
       # options:

lib/gitlab/gitaly_client/repository_service.rb

@@ -203,6 +203,22 @@ module Gitlab
           timeout: GitalyClient.default_timeout
         )
       end
+
+      def write_ref(ref_path, ref, old_ref, shell)
+        request = Gitaly::WriteRefRequest.new(
+          repository: @gitaly_repo,
+          ref: ref_path.b,
+          revision: ref.b,
+          shell: shell
+        )
+        request.old_revision = old_ref.b unless old_ref.nil?
+
+        response = GitalyClient.call(@storage, :repository_service, :write_ref, request)
+
+        raise Gitlab::Git::CommandError, encode!(response.error) if response.error.present?
+
+        true
+      end
     end
   end
 end

lib/gitlab/gitaly_client/wiki_service.rb

@@ -127,6 +127,18 @@ module Gitlab
         wiki_file
       end
 
+      def get_formatted_data(title:, dir: nil, version: nil)
+        request = Gitaly::WikiGetFormattedDataRequest.new(
+          repository: @gitaly_repo,
+          title: encode_binary(title),
+          revision: encode_binary(version),
+          directory: encode_binary(dir)
+        )
+
+        response = GitalyClient.call(@repository.storage, :wiki_service, :wiki_get_formatted_data, request)
+        response.reduce("") { |memo, msg| memo << msg.data }
+      end
+
       private
 
       # If a block is given and the yielded value is true, iteration will be

lib/gitlab/o_auth/user.rb

@@ -57,7 +57,7 @@ module Gitlab
         user ||= find_or_build_ldap_user if auto_link_ldap_user?
         user ||= build_new_user if signup_enabled?
 
-        user.external = true if external_provider? && user
+        user.external = true if external_provider? && user&.new_record?
 
         user
       end

spec/lib/gitlab/o_auth/user_spec.rb

@@ -44,6 +44,18 @@ describe Gitlab::OAuth::User do
 
     let(:provider) { 'twitter' }
 
+    describe 'when account exists on server' do
+      it 'does not mark the user as external' do
+        create(:omniauth_user, extern_uid: 'my-uid', provider: provider)
+        stub_omniauth_config(allow_single_sign_on: [provider], external_providers: [provider])
+
+        oauth_user.save
+
+        expect(gl_user).to be_valid
+        expect(gl_user.external).to be_falsey
+      end
+    end
+
     describe 'signup' do
       context 'when signup is disabled' do
         before do
@@ -51,7 +63,7 @@ describe Gitlab::OAuth::User do
         end
 
         it 'creates the user' do
-          stub_omniauth_config(allow_single_sign_on: ['twitter'])
+          stub_omniauth_config(allow_single_sign_on: [provider])
 
           oauth_user.save
 
@@ -65,7 +77,7 @@ describe Gitlab::OAuth::User do
         end
 
         it 'creates and confirms the user anyway' do
-          stub_omniauth_config(allow_single_sign_on: ['twitter'])
+          stub_omniauth_config(allow_single_sign_on: [provider])
 
           oauth_user.save
 
@@ -75,7 +87,7 @@ describe Gitlab::OAuth::User do
       end
 
       it 'marks user as having password_automatically_set' do
-        stub_omniauth_config(allow_single_sign_on: ['twitter'], external_providers: ['twitter'])
+        stub_omniauth_config(allow_single_sign_on: [provider], external_providers: [provider])
 
         oauth_user.save
 
@@ -86,7 +98,7 @@ describe Gitlab::OAuth::User do
       shared_examples 'to verify compliance with allow_single_sign_on' do
         context 'provider is marked as external' do
           it 'marks user as external' do
-            stub_omniauth_config(allow_single_sign_on: ['twitter'], external_providers: ['twitter'])
+            stub_omniauth_config(allow_single_sign_on: [provider], external_providers: [provider])
             oauth_user.save
             expect(gl_user).to be_valid
             expect(gl_user.external).to be_truthy
@@ -95,8 +107,8 @@ describe Gitlab::OAuth::User do
 
         context 'provider was external, now has been removed' do
           it 'does not mark external user as internal' do
-            create(:omniauth_user, extern_uid: 'my-uid', provider: 'twitter', external: true)
-            stub_omniauth_config(allow_single_sign_on: ['twitter'], external_providers: ['facebook'])
+            create(:omniauth_user, extern_uid: 'my-uid', provider: provider, external: true)
+            stub_omniauth_config(allow_single_sign_on: [provider], external_providers: ['facebook'])
             oauth_user.save
             expect(gl_user).to be_valid
             expect(gl_user.external).to be_truthy
@@ -118,7 +130,7 @@ describe Gitlab::OAuth::User do
 
         context 'with new allow_single_sign_on enabled syntax' do
           before do
-            stub_omniauth_config(allow_single_sign_on: ['twitter'])
+            stub_omniauth_config(allow_single_sign_on: [provider])
           end
 
           it "creates a user from Omniauth" do
@@ -127,7 +139,7 @@ describe Gitlab::OAuth::User do
             expect(gl_user).to be_valid
             identity = gl_user.identities.first
             expect(identity.extern_uid).to eql uid
-            expect(identity.provider).to eql 'twitter'
+            expect(identity.provider).to eql provider
           end
         end
 
@@ -142,7 +154,7 @@ describe Gitlab::OAuth::User do
             expect(gl_user).to be_valid
             identity = gl_user.identities.first
             expect(identity.extern_uid).to eql uid
-            expect(identity.provider).to eql 'twitter'
+            expect(identity.provider).to eql provider
           end
         end
 

spec/models/concerns/discussion_on_diff_spec.rb

@@ -20,6 +20,16 @@ describe DiscussionOnDiff do
         expect(truncated_lines).not_to include(be_meta)
       end
     end
+
+    context "when the diff line does not exist on a legacy diff note" do
+      it "returns an empty array" do
+        legacy_note = LegacyDiffNote.new
+
+        allow(subject).to receive(:first_note).and_return(legacy_note)
+
+        expect(truncated_lines).to eq([])
+      end
+    end
   end
 
   describe '#line_code_in_diffs' do

spec/models/wiki_page_spec.rb

@@ -387,13 +387,23 @@ describe WikiPage do
   end
 
   describe '#formatted_content' do
-    it 'returns processed content of the page', :disable_gitaly do
-      subject.create({ title: "RDoc", content: "*bold*", format: "rdoc" })
-      page = wiki.find_page('RDoc')
+    shared_examples 'fetching page formatted content' do
+      it 'returns processed content of the page' do
+        subject.create({ title: "RDoc", content: "*bold*", format: "rdoc" })
+        page = wiki.find_page('RDoc')
 
-      expect(page.formatted_content).to eq("\n<p><strong>bold</strong></p>\n")
+        expect(page.formatted_content).to eq("\n<p><strong>bold</strong></p>\n")
 
-      destroy_page('RDoc')
+        destroy_page('RDoc')
+      end
+    end
+
+    context 'when Gitaly wiki_page_formatted_data is enabled' do
+      it_behaves_like 'fetching page formatted content'
+    end
+
+    context 'when Gitaly wiki_page_formatted_data is disabled', :disable_gitaly do
+      it_behaves_like 'fetching page formatted content'
     end
   end