Merge branch 'ce-to-ee-2018-03-16' into 'master'

CE upstream - 2018-03-16 15:27 UTC

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

Gemfile

@@ -371,7 +371,7 @@ group :development, :test do
   gem 'benchmark-ips', '~> 2.3.0', require: false
 
   gem 'license_finder', '~> 3.1', require: false
-  gem 'knapsack', '~> 1.11.0'
+  gem 'knapsack', '~> 1.16'
 
   gem 'activerecord_sane_schema_dumper', '0.2'
 
@@ -435,9 +435,9 @@ gem 'google-protobuf', '= 3.5.1'
 gem 'toml-rb', '~> 1.0.0', require: false
 
 # Feature toggles
-gem 'flipper', '~> 0.11.0'
-gem 'flipper-active_record', '~> 0.11.0'
-gem 'flipper-active_support_cache_store', '~> 0.11.0'
+gem 'flipper', '~> 0.13.0'
+gem 'flipper-active_record', '~> 0.13.0'
+gem 'flipper-active_support_cache_store', '~> 0.13.0'
 
 # Structured logging
 gem 'lograge', '~> 0.5'

Gemfile.lock

@@ -242,13 +242,13 @@ GEM
       path_expander (~> 1.0)
       ruby_parser (~> 3.0)
       sexp_processor (~> 4.0)
-    flipper (0.11.0)
-    flipper-active_record (0.11.0)
+    flipper (0.13.0)
+    flipper-active_record (0.13.0)
       activerecord (>= 3.2, < 6)
-      flipper (~> 0.11.0)
-    flipper-active_support_cache_store (0.11.0)
+      flipper (~> 0.13.0)
+    flipper-active_support_cache_store (0.13.0)
       activesupport (>= 3.2, < 6)
-      flipper (~> 0.11.0)
+      flipper (~> 0.13.0)
     flowdock (0.7.1)
       httparty (~> 0.7)
       multi_json
@@ -479,9 +479,8 @@ GEM
       kaminari-core (= 1.0.1)
     kaminari-core (1.0.1)
     kgio (2.10.0)
-    knapsack (1.11.0)
+    knapsack (1.16.0)
       rake
-      timecop (>= 0.1.0)
     kubeclient (2.2.0)
       http (= 0.9.8)
       recursive-open-struct (= 1.0.0)
@@ -1075,9 +1074,9 @@ DEPENDENCIES
   fast_blank
   ffaker (~> 2.4)
   flay (~> 2.10.0)
-  flipper (~> 0.11.0)
-  flipper-active_record (~> 0.11.0)
-  flipper-active_support_cache_store (~> 0.11.0)
+  flipper (~> 0.13.0)
+  flipper-active_record (~> 0.13.0)
+  flipper-active_support_cache_store (~> 0.13.0)
   fog-aliyun (~> 0.2.0)
   fog-aws (~> 2.0)
   fog-core (~> 1.44)
@@ -1126,7 +1125,7 @@ DEPENDENCIES
   json-schema (~> 2.8.0)
   jwt (~> 1.5.6)
   kaminari (~> 1.0)
-  knapsack (~> 1.11.0)
+  knapsack (~> 1.16)
   kubeclient (~> 2.2.0)
   letter_opener_web (~> 1.3.0)
   license_finder (~> 3.1)

app/models/application_setting.rb

@@ -348,15 +348,15 @@ class ApplicationSetting < ActiveRecord::Base
   end
 
   def home_page_url_column_exists?
-    ActiveRecord::Base.connection.column_exists?(:application_settings, :home_page_url)
+    ::Gitlab::Database.cached_column_exists?(:application_settings, :home_page_url)
   end
 
   def help_page_support_url_column_exists?
-    ActiveRecord::Base.connection.column_exists?(:application_settings, :help_page_support_url)
+    ::Gitlab::Database.cached_column_exists?(:application_settings, :help_page_support_url)
   end
 
   def sidekiq_throttling_column_exists?
-    ActiveRecord::Base.connection.column_exists?(:application_settings, :sidekiq_throttling_enabled)
+    ::Gitlab::Database.cached_column_exists?(:application_settings, :sidekiq_throttling_enabled)
   end
 
   def domain_whitelist_raw

app/models/project_services/jira_service.rb

@@ -161,11 +161,6 @@ class JiraService < IssueTrackerService
     add_comment(data, jira_issue)
   end
 
-  # reason why service cannot be tested
-  def disabled_title
-    "Please fill in Password and Username."
-  end
-
   def test(_)
     result = test_settings
     success = result.present?

app/models/project_services/pipelines_email_service.rb

@@ -39,10 +39,6 @@ class PipelinesEmailService < Service
     project.pipelines.any?
   end
 
-  def disabled_title
-    'Please setup a pipeline on your repository.'
-  end
-
   def test_data(project, user)
     data = Gitlab::DataBuilder::Pipeline.build(project.pipelines.last)
     data[:user] = user.hook_attrs

app/models/service.rb

@@ -163,11 +163,6 @@ class Service < ActiveRecord::Base
     true
   end
 
-  # reason why service cannot be tested
-  def disabled_title
-    "Please setup a project repository."
-  end
-
   # Provide convenient accessor methods
   # for each serialized property.
   # Also keep track of updated properties in a similar way as ActiveModel::Dirty

app/services/files/create_service.rb

 module Files
   class CreateService < Files::BaseService
     def create_commit!
-      handler = Lfs::FileModificationHandler.new(project, @branch_name)
+      transformer = Lfs::FileTransformer.new(project, @branch_name)
 
-      handler.new_file(@file_path, @file_content) do |content_or_lfs_pointer|
-        create_transformed_commit(content_or_lfs_pointer)
-      end
+      result = transformer.new_file(@file_path, @file_content)
+
+      create_transformed_commit(result.content)
     end
 
     private

app/services/files/multi_service.rb

@@ -3,11 +3,33 @@ module Files
     UPDATE_FILE_ACTIONS = %w(update move delete).freeze
 
     def create_commit!
+      transformer = Lfs::FileTransformer.new(project, @branch_name)
+
+      actions = actions_after_lfs_transformation(transformer, params[:actions])
+
+      commit_actions!(actions)
+    end
+
+    private
+
+    def actions_after_lfs_transformation(transformer, actions)
+      actions.map do |action|
+        if action[:action] == 'create'
+          result = transformer.new_file(action[:file_path], action[:content], encoding: action[:encoding])
+          action[:content] = result.content
+          action[:encoding] = result.encoding
+        end
+
+        action
+      end
+    end
+
+    def commit_actions!(actions)
       repository.multi_action(
         current_user,
         message: @commit_message,
         branch_name: @branch_name,
-        actions: params[:actions],
+        actions: actions,
         author_email: @author_email,
         author_name: @author_name,
         start_project: @start_project,
@@ -17,8 +39,6 @@ module Files
       raise_error(e)
     end
 
-    private
-
     def validate!
       super
 

app/services/lfs/file_modification_handler.rb → app/services/lfs/file_transformer.rb

 module Lfs
-  class FileModificationHandler
+  # Usage: Calling `new_file` check to see if a file should be in LFS and
+  #        return a transformed result with `content` and `encoding` to commit.
+  #
+  #        For LFS an LfsObject linked to the project is stored and an LFS
+  #        pointer returned. If the file isn't in LFS the untransformed content
+  #        is returned to save in the commit.
+  #
+  # transformer = Lfs::FileTransformer.new(project, @branch_name)
+  # content_or_lfs_pointer = transformer.new_file(file_path, content).content
+  # create_transformed_commit(content_or_lfs_pointer)
+  #
+  class FileTransformer
     attr_reader :project, :branch_name
 
     delegate :repository, to: :project
@@ -9,24 +20,37 @@ module Lfs
       @branch_name = branch_name
     end
 
-    def new_file(file_path, file_content)
+    def new_file(file_path, file_content, encoding: nil)
       if project.lfs_enabled? && lfs_file?(file_path)
+        file_content = Base64.decode64(file_content) if encoding == 'base64'
         lfs_pointer_file = Gitlab::Git::LfsPointerFile.new(file_content)
         lfs_object = create_lfs_object!(lfs_pointer_file, file_content)
-        content = lfs_pointer_file.pointer
 
-        success = yield(content)
+        link_lfs_object!(lfs_object)
 
-        link_lfs_object!(lfs_object) if success
+        Result.new(content: lfs_pointer_file.pointer, encoding: 'text')
       else
-        yield(file_content)
+        Result.new(content: file_content, encoding: encoding)
+      end
+    end
+
+    class Result
+      attr_reader :content, :encoding
+
+      def initialize(content:, encoding:)
+        @content = content
+        @encoding = encoding
       end
     end
 
     private
 
     def lfs_file?(file_path)
-      repository.attributes_at(branch_name, file_path)['filter'] == 'lfs'
+      cached_attributes.attributes(file_path)['filter'] == 'lfs'
+    end
+
+    def cached_attributes
+      @cached_attributes ||= Gitlab::Git::AttributesAtRefParser.new(repository, branch_name)
     end
 
     def create_lfs_object!(lfs_pointer_file, file_content)

app/views/projects/services/_form.html.haml

@@ -15,11 +15,6 @@
         .footer-block.row-content-block
           = service_save_button(@service)
            
-          - if @service.valid? && @service.activated?
-            - unless @service.can_test?
-              - disabled_class = 'disabled'
-              - disabled_title = @service.disabled_title
-
           = link_to 'Cancel', project_settings_integrations_path(@project), class: 'btn btn-cancel'
 
 - if lookup_context.template_exists?('show', "projects/services/#{@service.to_param}", true)

changelogs/unreleased/44235-update-knapsack-to-1-16-0.yml

+---
+title: Update knapsack to 1.16.0
+merge_request: 17735
+author: Takuya Noguchi
+type: other

changelogs/unreleased/jej-commit-api-tracks-lfs.yml

+---
+title: Create commit API and Web IDE obey LFS filters
+merge_request: 16718
+author:
+type: fixed

changelogs/unreleased/sh-cache-column-exists.yml

+---
+title: Cache column_exists? for application settings
+merge_request:
+author:
+type: performance

changelogs/unreleased/sh-cache-table-exists.yml

+---
+title: Cache table_exists?('application_settings') to reduce repeated schema reloads
+merge_request:
+author:
+type: performance

doc/administration/raketasks/check.md

@@ -84,9 +84,9 @@ checks using those checksums can be run. These checks also detect missing files.
 
 Currently, integrity checks are supported for the following types of file:
 
-* CI artifacts
-* LFS objects
-* User uploads
+* CI artifacts (Available from version 10.7.0)
+* LFS objects (Available from version 10.6.0)
+* User uploads (Available from version 10.6.0)
 
 **Omnibus Installation**
 

doc/ci/examples/code_climate.md

@@ -35,7 +35,7 @@ Starting with [GitLab Starter][ee] 9.3, this information will
 be automatically extracted and shown right in the merge request widget. To do
 so, the CI/CD job must be named `codequality` and the artifact path must be
 `codeclimate.json`.
-[Learn more on code quality diffs in merge requests](../../user/project/merge_requests/code_quality_diff.md).
+[Learn more on code quality diffs in merge requests](https://docs.gitlab.com/ee/user/project/merge_requests/code_quality_diff.html).
 
 [cli]: https://github.com/codeclimate/codeclimate
 [dind]: ../docker/using_docker_build.md#use-docker-in-docker-executor

doc/development/writing_documentation.md

@@ -303,7 +303,10 @@ a couple more commits to the EE branch, but ask the reviewer to review the EE me
 additionally to the CE MR. If there are many EE-only changes though, start a new MR
 to EE only.
 
-### Previewing the changes live
+## Previewing the changes live
+
+To preview your changes to documentation locally, please follow
+this [development guide](https://gitlab.com/gitlab-com/gitlab-docs/blob/master/README.md#development).
 
 To preview your changes to documentation locally, please follow
 this [development guide](https://gitlab.com/gitlab-com/gitlab-docs/blob/master/README.md#development).

doc/downgrade_ee_to_ce/README.md

@@ -70,7 +70,7 @@ To downgrade an Omnibus installation, it is sufficient to install the Community
 Edition package on top of the currently installed one. You can do this manually,
 by directly [downloading the package](https://packages.gitlab.com/gitlab/gitlab-ce)
 you need, or by adding our CE package repository and following the
-[CE installation instructions](https://about.gitlab.com/downloads/?version=ce).
+[CE installation instructions](https://about.gitlab.com/installation/?version=ce).
 
 **Source Installation**
 

lib/gitlab/current_settings.rb

@@ -70,7 +70,7 @@ module Gitlab
         active_db_connection = ActiveRecord::Base.connection.active? rescue false
 
         active_db_connection &&
-          ActiveRecord::Base.connection.table_exists?('application_settings')
+          Gitlab::Database.cached_table_exists?('application_settings')
       rescue ActiveRecord::NoDatabaseError
         false
       end

lib/gitlab/database.rb

@@ -203,6 +203,11 @@ module Gitlab
       connection.schema_cache.columns_hash(table_name).has_key?(column_name.to_s)
     end
 
+    def self.cached_table_exists?(table_name)
+      # Rails 5 uses data_source_exists? instead of table_exists?
+      connection.schema_cache.table_exists?(table_name)
+    end
+
     private_class_method :connection
 
     def self.database_version

lib/gitlab/git/lfs_pointer_file.rb

 module Gitlab
   module Git
     class LfsPointerFile
+      VERSION = "https://git-lfs.github.com/spec/v1".freeze
+      VERSION_LINE = "version #{VERSION}".freeze
+
       def initialize(data)
         @data = data
       end
 
       def pointer
         @pointer ||= <<~FILE
-          version https://git-lfs.github.com/spec/v1
+          #{VERSION_LINE}
           oid sha256:#{sha256}
           size #{size}
         FILE
@@ -20,6 +23,10 @@ module Gitlab
       def sha256
         @sha256 ||= Digest::SHA256.hexdigest(@data)
       end
+
+      def inspect
+        "#<#{self.class}:#{object_id} @size=#{size}, @sha256=#{sha256.inspect}>"
+      end
     end
   end
 end

lib/gitlab/git/repository.rb

@@ -1006,8 +1006,9 @@ module Gitlab
       # This only checks the root .gitattributes file,
       # it does not traverse subfolders to find additional .gitattributes files
       #
-      # This method is around 30 times slower than `attributes`,
-      # which uses `$GIT_DIR/info/attributes`
+      # This method is around 30 times slower than `attributes`, which uses
+      # `$GIT_DIR/info/attributes`. Consider caching AttributesAtRefParser
+      # and reusing that for multiple calls instead of this method.
       def attributes_at(ref, file_path)
         parser = AttributesAtRefParser.new(self, ref)
         parser.attributes(file_path)

spec/lib/gitlab/database_spec.rb

@@ -298,6 +298,18 @@ describe Gitlab::Database do
     end
   end
 
+  describe '.cached_table_exists?' do
+    it 'only retrieves data once per table' do
+      expect(ActiveRecord::Base.connection).to receive(:table_exists?).with(:projects).once.and_call_original
+      expect(ActiveRecord::Base.connection).to receive(:table_exists?).with(:bogus_table_name).once.and_call_original
+
+      2.times do
+        expect(described_class.cached_table_exists?(:projects)).to be_truthy
+        expect(described_class.cached_table_exists?(:bogus_table_name)).to be_falsey
+      end
+    end
+  end
+
   describe '#true_value' do
     it 'returns correct value for PostgreSQL' do
       expect(described_class).to receive(:postgresql?).and_return(true)

spec/services/files/create_service_spec.rb

@@ -43,7 +43,7 @@ describe Files::CreateService do
 
           blob = repository.blob_at('lfs', file_path)
 
-          expect(blob.data).not_to start_with('version https://git-lfs.github.com/spec/v1')
+          expect(blob.data).not_to start_with(Gitlab::Git::LfsPointerFile::VERSION_LINE)
           expect(blob.data).to eq(file_content)
         end
       end
@@ -58,7 +58,7 @@ describe Files::CreateService do
 
           blob = repository.blob_at('lfs', file_path)
 
-          expect(blob.data).to start_with('version https://git-lfs.github.com/spec/v1')
+          expect(blob.data).to start_with(Gitlab::Git::LfsPointerFile::VERSION_LINE)
         end
 
         it "creates an LfsObject with the file's content" do

spec/services/files/multi_service_spec.rb

@@ -4,28 +4,30 @@ describe Files::MultiService do
   subject { described_class.new(project, user, commit_params) }
 
   let(:project) { create(:project, :repository) }
+  let(:repository) { project.repository }
   let(:user) { create(:user) }
   let(:branch_name) { project.default_branch }
   let(:original_file_path) { 'files/ruby/popen.rb' }
   let(:new_file_path) { 'files/ruby/popen.rb' }
+  let(:file_content) { 'New content' }
   let(:action) { 'update' }
 
   let!(:original_commit_id) do
     Gitlab::Git::Commit.last_for_path(project.repository, branch_name, original_file_path).sha
   end
 
-  let(:actions) do
-    [
-      {
-        action: action,
-        file_path: new_file_path,
-        previous_path: original_file_path,
-        content: 'New content',
-        last_commit_id: original_commit_id
-      }
-    ]
+  let(:default_action) do
+    {
+      action: action,
+      file_path: new_file_path,
+      previous_path: original_file_path,
+      content: file_content,
+      last_commit_id: original_commit_id
+    }
   end
 
+  let(:actions) { [default_action] }
+
   let(:commit_params) do
     {
       commit_message: "Update File",
@@ -110,6 +112,56 @@ describe Files::MultiService do
       end
     end
 
+    context 'when creating a file matching an LFS filter' do
+      let(:action) { 'create' }
+      let(:branch_name) { 'lfs' }
+      let(:new_file_path) { 'test_file.lfs' }
+
+      before do
+        allow(project).to receive(:lfs_enabled?).and_return(true)
+      end
+
+      it 'creates an LFS pointer' do
+        subject.execute
+
+        blob = repository.blob_at('lfs', new_file_path)
+
+        expect(blob.data).to start_with(Gitlab::Git::LfsPointerFile::VERSION_LINE)
+      end
+
+      it "creates an LfsObject with the file's content" do
+        subject.execute
+
+        expect(LfsObject.last.file.read).to eq file_content
+      end
+
+      context 'with base64 encoded content' do
+        let(:raw_file_content) { 'Raw content' }
+        let(:file_content) { Base64.encode64(raw_file_content) }
+        let(:actions) { [default_action.merge(encoding: 'base64')] }
+
+        it 'creates an LFS pointer' do
+          subject.execute
+
+          blob = repository.blob_at('lfs', new_file_path)
+
+          expect(blob.data).to start_with(Gitlab::Git::LfsPointerFile::VERSION_LINE)
+        end
+
+        it "creates an LfsObject with the file's content" do
+          subject.execute
+
+          expect(LfsObject.last.file.read).to eq raw_file_content
+        end
+      end
+
+      it 'links the LfsObject to the project' do
+        expect do
+          subject.execute
+        end.to change { project.lfs_objects.count }.by(1)
+      end
+    end
+
     context 'when file status validation is skipped' do
       let(:action) { 'create' }
       let(:new_file_path) { 'files/ruby/new_file.rb' }

spec/services/lfs/file_transformer_spec.rb

+require "spec_helper"
+
+describe Lfs::FileTransformer do
+  let(:project) { create(:project, :repository) }
+  let(:repository) { project.repository }
+  let(:file_content) { 'Test file content' }
+  let(:branch_name) { 'lfs' }
+  let(:file_path) { 'test_file.lfs' }
+
+  subject { described_class.new(project, branch_name) }
+
+  describe '#new_file' do
+    context 'with lfs disabled' do
+      it 'skips gitattributes check' do
+        expect(repository.raw).not_to receive(:blob_at)
+
+        subject.new_file(file_path, file_content)
+      end
+
+      it 'returns untransformed content' do
+        result = subject.new_file(file_path, file_content)
+
+        expect(result.content).to eq(file_content)
+      end
+
+      it 'returns untransformed encoding' do
+        result = subject.new_file(file_path, file_content, encoding: 'base64')
+
+        expect(result.encoding).to eq('base64')
+      end
+    end
+
+    context 'with lfs enabled' do
+      before do
+        allow(project).to receive(:lfs_enabled?).and_return(true)
+      end
+
+      it 'reuses cached gitattributes' do
+        second_file = 'another_file.lfs'
+
+        expect(repository.raw).to receive(:blob_at).with(branch_name, '.gitattributes').once
+
+        subject.new_file(file_path, file_content)
+        subject.new_file(second_file, file_content)
+      end
+
+      it "creates an LfsObject with the file's content" do
+        subject.new_file(file_path, file_content)
+
+        expect(LfsObject.last.file.read).to eq file_content
+      end
+
+      it 'returns an LFS pointer' do
+        result = subject.new_file(file_path, file_content)
+
+        expect(result.content).to start_with(Gitlab::Git::LfsPointerFile::VERSION_LINE)
+      end
+
+      it 'returns LFS pointer encoding as text' do
+        result = subject.new_file(file_path, file_content, encoding: 'base64')
+
+        expect(result.encoding).to eq('text')
+      end
+
+      context "when doesn't use LFS" do
+        let(:file_path) { 'other.filetype' }
+
+        it "doesn't create LFS pointers" do
+          new_content = subject.new_file(file_path, file_content).content
+
+          expect(new_content).not_to start_with(Gitlab::Git::LfsPointerFile::VERSION_LINE)
+          expect(new_content).to eq(file_content)
+        end
+      end
+
+      it 'links LfsObjects to project' do
+        expect do
+          subject.new_file(file_path, file_content)
+        end.to change { project.lfs_objects.count }.by(1)
+      end
+
+      context 'when LfsObject already exists' do
+        let(:lfs_pointer) { Gitlab::Git::LfsPointerFile.new(file_content) }
+
+        before do
+          create(:lfs_object, oid: lfs_pointer.sha256, size: lfs_pointer.size)
+        end
+
+        it 'links LfsObjects to project' do
+          expect do
+            subject.new_file(file_path, file_content)
+          end.to change { project.lfs_objects.count }.by(1)
+        end
+      end
+    end
+  end
+end