Merge branch 'ce-to-ee-2018-04-02' into 'master'

CE upstream - 2018-04-02 14:31 UTC

Closes #2262 and gitlab-ce#2262

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

app/helpers/application_helper.rb

@@ -291,6 +291,10 @@ module ApplicationHelper
     class_names
   end
 
+  # EE feature: System header and footer, unavailable in CE
+  def system_message_class
+  end
+
   # Returns active css class when condition returns true
   # otherwise returns nil.
   #

app/validators/certificate_fingerprint_validator.rb

+class CertificateFingerprintValidator < ActiveModel::EachValidator
+  FINGERPRINT_PATTERN = /\A([a-zA-Z0-9]{2}[\s\-:]?){16,}\z/.freeze
+
+  def validate_each(record, attribute, value)
+    unless value.try(:match, FINGERPRINT_PATTERN)
+      record.errors.add(attribute, "must be a hash containing only letters, numbers, spaces, : and -")
+    end
+  end
+end

app/validators/top_level_group_validator.rb

+class TopLevelGroupValidator < ActiveModel::EachValidator
+  def validate_each(record, attribute, value)
+    if value&.subgroup?
+      record.errors.add(attribute, "must be a top level Group")
+    end
+  end
+end

app/workers/object_storage/migrate_uploads_worker.rb

@@ -138,21 +138,18 @@ module ObjectStorage
 
     include Report
 
-    def self.enqueue!(uploads, mounted_as, to_store)
-      sanity_check!(uploads, mounted_as)
+    def self.enqueue!(uploads, model_class, mounted_as, to_store)
+      sanity_check!(uploads, model_class, mounted_as)
 
-      perform_async(uploads.ids, mounted_as, to_store)
+      perform_async(uploads.ids, model_class.to_s, mounted_as, to_store)
     end
 
     # We need to be sure all the uploads are for the same uploader and model type
     # and that the mount point exists if provided.
     #
-    def self.sanity_check!(uploads, mounted_as)
+    def self.sanity_check!(uploads, model_class, mounted_as)
       upload = uploads.first
-
       uploader_class = upload.uploader.constantize
-      model_class = uploads.first.model_type.constantize
-
       uploader_types = uploads.map(&:uploader).uniq
       model_types = uploads.map(&:model_type).uniq
       model_has_mount = mounted_as.nil? || model_class.uploaders[mounted_as] == uploader_class
@@ -162,7 +159,12 @@ module ObjectStorage
       raise(SanityCheckError, "Mount point #{mounted_as} not found in #{model_class}.") unless model_has_mount
     end
 
-    def perform(ids, mounted_as, to_store)
+    def perform(*args)
+      args_check!(args)
+
+      (ids, model_type, mounted_as, to_store) = args
+
+      @model_class = model_type.constantize
       @mounted_as = mounted_as&.to_sym
       @to_store = to_store
 
@@ -178,7 +180,17 @@ module ObjectStorage
     end
 
     def sanity_check!(uploads)
-      self.class.sanity_check!(uploads, @mounted_as)
+      self.class.sanity_check!(uploads, @model_class, @mounted_as)
+    end
+
+    def args_check!(args)
+      return if args.count == 4
+
+      case args.count
+      when 3 then raise SanityCheckError, "Job is missing the `model_type` argument."
+      else
+        raise SanityCheckError, "Job has wrong arguments format."
+      end
     end
 
     def build_uploaders(uploads)

changelogs/unreleased/44776-fix-upload-migrate-fails-for-group.yml

+---
+title: Fixed gitlab:uploads:migrate task failing for Groups' avatar.
+merge_request: 18088
+author:
+type: fixed

doc/api/commits.md

@@ -412,9 +412,10 @@ Example response:
 
 Since GitLab 8.1, this is the new commit status API.
 
-### Get the status of a commit
+### List the statuses of a commit
 
-Get the statuses of a commit in a project.
+List the statuses of a commit in a project.
+The pagination parameters `page` and `per_page` can be used to restrict the list of references.
 
 ```
 GET /projects/:id/repository/commits/:sha/statuses

lib/gitlab/git/gitmodules_parser.rb

@@ -46,6 +46,8 @@ module Gitlab
         iterator = State.new
 
         @content.split("\n").each_with_object(iterator) do |text, iterator|
+          text.chomp!
+
           next if text =~ /^\s*#/
 
           if text =~ /\A\[submodule "(?<name>[^"]+)"\]\z/
@@ -55,7 +57,7 @@ module Gitlab
 
             next unless text =~ /\A\s*(?<key>\w+)\s*=\s*(?<value>.*)\z/
 
-            value = $~[:value].chomp
+            value = $~[:value]
             iterator.set_attribute($~[:key], value)
           end
         end

lib/tasks/gitlab/uploads/migrate.rake

@@ -13,6 +13,7 @@ namespace :gitlab do
 
     def enqueue_batch(batch, index)
       job = ObjectStorage::MigrateUploadsWorker.enqueue!(batch,
+                                                         @model_class,
                                                          @mounted_as,
                                                          @to_store)
       puts "Enqueued job ##{index}: #{job}"

spec/lib/gitlab/git/gitmodules_parser_spec.rb

@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe Gitlab::Git::GitmodulesParser do
   it 'should parse a .gitmodules file correctly' do
-    parser = described_class.new(<<-'GITMODULES'.strip_heredoc)
+    data = <<~GITMODULES
       [submodule "vendor/libgit2"]
          path = vendor/libgit2
       [submodule "vendor/libgit2"]
@@ -16,6 +16,7 @@ describe Gitlab::Git::GitmodulesParser do
           url = https://example.com/another/project
     GITMODULES
 
+    parser = described_class.new(data.gsub("\n", "\r\n"))
     modules = parser.parse
 
     expect(modules).to eq({

spec/support/cookie_helper.rb

@@ -2,12 +2,25 @@
 #
 module CookieHelper
   def set_cookie(name, value, options = {})
+    case page.driver
+    when Capybara::RackTest::Driver
+      rack_set_cookie(name, value)
+    else
+      selenium_set_cookie(name, value, options)
+    end
+  end
+
+  def selenium_set_cookie(name, value, options = {})
     # Selenium driver will not set cookies for a given domain when the browser is at `about:blank`.
     # It also doesn't appear to allow overriding the cookie path. loading `/` is the most inclusive.
     visit options.fetch(:path, '/') unless on_a_page?
     page.driver.browser.manage.add_cookie(name: name, value: value, **options)
   end
 
+  def rack_set_cookie(name, value)
+    page.driver.browser.set_cookie("#{name}=#{value}")
+  end
+
   def get_cookie(name)
     page.driver.browser.manage.cookie_named(name)
   end

spec/support/login_helpers.rb

@@ -140,6 +140,10 @@ module LoginHelpers
     end
     allow(Gitlab::Auth::OAuth::Provider).to receive_messages(providers: [:saml], config_for: mock_saml_config)
     stub_omniauth_setting(messages)
+    stub_saml_authorize_path_helpers
+  end
+
+  def stub_saml_authorize_path_helpers
     allow_any_instance_of(Object).to receive(:user_saml_omniauth_authorize_path).and_return('/users/auth/saml')
     allow_any_instance_of(Object).to receive(:omniauth_authorize_path).with(:user, "saml").and_return('/users/auth/saml')
   end

spec/tasks/gitlab/uploads/migrate_rake_spec.rb

 require 'rake_helper'
 
 describe 'gitlab:uploads:migrate rake tasks' do
-  let!(:projects) { create_list(:project, 10, :with_avatar) }
-  let(:model_class) { Project }
-  let(:uploader_class) { AvatarUploader }
-  let(:mounted_as) { :avatar }
+  let(:model_class) { nil }
+  let(:uploader_class) { nil }
+  let(:mounted_as) { nil }
   let(:batch_size) { 3 }
 
   before do
@@ -30,6 +29,14 @@ describe 'gitlab:uploads:migrate rake tasks' do
     end
   end
 
+  context "for AvatarUploader" do
+    let(:uploader_class) { AvatarUploader }
+    let(:mounted_as) { :avatar }
+
+    context "for Project" do
+      let(:model_class) { Project }
+      let!(:projects) { create_list(:project, 10, :with_avatar) }
+
       it_behaves_like 'enqueue jobs in batch', batch: 4
 
       context 'Upload has store = nil' do
@@ -37,6 +44,100 @@ describe 'gitlab:uploads:migrate rake tasks' do
           Upload.where(model: projects).update_all(store: nil)
         end
 
+        it_behaves_like 'enqueue jobs in batch', batch: 4
+      end
+    end
+
+    context "for Group" do
+      let(:model_class) { Group }
+
+      before do
+        create_list(:group, 10, :with_avatar)
+      end
+
+      it_behaves_like 'enqueue jobs in batch', batch: 4
+    end
+
+    context "for User" do
+      let(:model_class) { User }
+
+      before do
+        create_list(:user, 10, :with_avatar)
+      end
+
+      it_behaves_like 'enqueue jobs in batch', batch: 4
+    end
+  end
+
+  context "for AttachmentUploader" do
+    let(:uploader_class) { AttachmentUploader }
+
+    context "for Note" do
+      let(:model_class) { Note }
+      let(:mounted_as) { :attachment }
+
+      before do
+        create_list(:note, 10, :with_attachment)
+      end
+
+      it_behaves_like 'enqueue jobs in batch', batch: 4
+    end
+
+    context "for Appearance" do
+      let(:model_class) { Appearance }
+      let(:mounted_as) { :logo }
+
+      before do
+        create(:appearance, :with_logos)
+      end
+
+      %i(logo header_logo).each do |mount|
+        it_behaves_like 'enqueue jobs in batch', batch: 1 do
+          let(:mounted_as) { mount }
+        end
+      end
+    end
+  end
+
+  context "for FileUploader" do
+    let(:uploader_class) { FileUploader }
+    let(:model_class) { Project }
+
+    before do
+      create_list(:project, 10) do |model|
+        uploader_class.new(model)
+          .store!(fixture_file_upload('spec/fixtures/doc_sample.txt'))
+      end
+    end
+
+    it_behaves_like 'enqueue jobs in batch', batch: 4
+  end
+
+  context "for PersonalFileUploader" do
+    let(:uploader_class) { PersonalFileUploader }
+    let(:model_class) { PersonalSnippet }
+
+    before do
+      create_list(:personal_snippet, 10) do |model|
+        uploader_class.new(model)
+          .store!(fixture_file_upload('spec/fixtures/doc_sample.txt'))
+      end
+    end
+
+    it_behaves_like 'enqueue jobs in batch', batch: 4
+  end
+
+  context "for NamespaceFileUploader" do
+    let(:uploader_class) { NamespaceFileUploader }
+    let(:model_class) { Snippet }
+
+    before do
+      create_list(:snippet, 10) do |model|
+        uploader_class.new(model)
+          .store!(fixture_file_upload('spec/fixtures/doc_sample.txt'))
+      end
+    end
+
     it_behaves_like 'enqueue jobs in batch', batch: 4
   end
 end

ee/spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb → spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb

@@ -9,6 +9,7 @@ describe ObjectStorage::MigrateUploadsWorker, :sidekiq do
 
   let!(:projects) { create_list(:project, 10, :with_avatar) }
   let(:uploads) { Upload.all }
+  let(:model_class) { Project }
   let(:mounted_as) { :avatar }
   let(:to_store) { ObjectStorage::Store::REMOTE }
 
@@ -18,7 +19,7 @@ describe ObjectStorage::MigrateUploadsWorker, :sidekiq do
 
   describe '.enqueue!' do
     def enqueue!
-      described_class.enqueue!(uploads, mounted_as, to_store)
+      described_class.enqueue!(uploads, Project, mounted_as, to_store)
     end
 
     it 'is guarded by .sanity_check!' do
@@ -44,7 +45,7 @@ describe ObjectStorage::MigrateUploadsWorker, :sidekiq do
       let(:mount_point) { nil }
 
       it do
-        expect { described_class.sanity_check!(uploads, mount_point) }
+        expect { described_class.sanity_check!(uploads, model_class, mount_point) }
           .to raise_error(described_class::SanityCheckError)
       end
     end
@@ -70,7 +71,7 @@ describe ObjectStorage::MigrateUploadsWorker, :sidekiq do
 
   describe '#perform' do
     def perform
-      described_class.new.perform(uploads.ids, mounted_as, to_store)
+      described_class.new.perform(uploads.ids, model_class.to_s, mounted_as, to_store)
     rescue ObjectStorage::MigrateUploadsWorker::Report::MigrationFailures
       # swallow
     end