Add latest changes from gitlab-org/gitlab@master

spec/lib/gitlab/file_type_detection_spec.rb

@@ -2,38 +2,103 @@
 require 'spec_helper'
 describe Gitlab::FileTypeDetection do
-  def upload_fixture(filename)
-    fixture_file_upload(File.join('spec', 'fixtures', filename))
-  end
-  describe '#image_or_video?' do
-    context 'when class is an uploader' do
-      let(:uploader) do
-        example_uploader = Class.new(CarrierWave::Uploader::Base) do
-          include Gitlab::FileTypeDetection
-          storage :file
-        end
-        example_uploader.new
+  context 'when class is an uploader' do
+    shared_examples '#image? for an uploader' do
+      it 'returns true for an image file' do
+        uploader.store!(upload_fixture('dk.png'))
+        expect(uploader).to be_image
+      end
+      it 'returns false if filename has a dangerous image extension' do
+        uploader.store!(upload_fixture('unsanitized.svg'))
+        expect(uploader).to be_dangerous_image
+        expect(uploader).not_to be_image
       end
-      it 'returns true for an image file' do
+      it 'returns false for a video file' do
+        uploader.store!(upload_fixture('video_sample.mp4'))
+
+        expect(uploader).not_to be_image
+      end
+
+      it 'returns false if filename is blank' do
         uploader.store!(upload_fixture('dk.png'))
-        expect(uploader).to be_image_or_video
+        allow(uploader).to receive(:filename).and_return(nil)
+
+        expect(uploader).not_to be_image
       end
+    end
+    shared_examples '#video? for an uploader' do
       it 'returns true for a video file' do
         uploader.store!(upload_fixture('video_sample.mp4'))
-        expect(uploader).to be_image_or_video
+        expect(uploader).to be_video
+      end
+
+      it 'returns false for an image file' do
+        uploader.store!(upload_fixture('dk.png'))
+
+        expect(uploader).not_to be_video
+      end
+
+      it 'returns false if filename is blank' do
+        uploader.store!(upload_fixture('dk.png'))
+
+        allow(uploader).to receive(:filename).and_return(nil)
+
+        expect(uploader).not_to be_video
+      end
+    end
+
+    shared_examples '#dangerous_image? for an uploader' do
+      it 'returns true if filename has a dangerous extension' do
+        uploader.store!(upload_fixture('unsanitized.svg'))
+
+        expect(uploader).to be_dangerous_image
+      end
+
+      it 'returns false for an image file' do
+        uploader.store!(upload_fixture('dk.png'))
+
+        expect(uploader).not_to be_dangerous_image
+      end
+
+      it 'returns false for a video file' do
+        uploader.store!(upload_fixture('video_sample.mp4'))
+
+        expect(uploader).not_to be_dangerous_image
+      end
+
+      it 'returns false if filename is blank' do
+        uploader.store!(upload_fixture('dk.png'))
+
+        allow(uploader).to receive(:filename).and_return(nil)
+
+        expect(uploader).not_to be_dangerous_image
+      end
+    end
+
+    shared_examples '#dangerous_video? for an uploader' do
+      it 'returns false for a safe video file' do
+        uploader.store!(upload_fixture('video_sample.mp4'))
+
+        expect(uploader).not_to be_dangerous_video
+      end
+
+      it 'returns false if filename is a dangerous image extension' do
+        uploader.store!(upload_fixture('unsanitized.svg'))
+
+        expect(uploader).not_to be_dangerous_video
       end
-      it 'returns false for other extensions' do
-        uploader.store!(upload_fixture('doc_sample.txt'))
-        expect(uploader).not_to be_image_or_video
+      it 'returns false for an image file' do
+        uploader.store!(upload_fixture('dk.png'))
+        expect(uploader).not_to be_dangerous_video
       end
       it 'returns false if filename is blank' do
@@ -41,42 +106,190 @@ describe Gitlab::FileTypeDetection do
         allow(uploader).to receive(:filename).and_return(nil)
-        expect(uploader).not_to be_image_or_video
+        expect(uploader).not_to be_dangerous_video
       end
     end
-    context 'when class is a regular class' do
-      let(:custom_class) do
-        custom_class = Class.new do
-          include Gitlab::FileTypeDetection
-        end
-        custom_class.new
+    let(:uploader) do
+      example_uploader = Class.new(CarrierWave::Uploader::Base) do
+        include Gitlab::FileTypeDetection
+        storage :file
       end
+      example_uploader.new
+    end
+
+    def upload_fixture(filename)
+      fixture_file_upload(File.join('spec', 'fixtures', filename))
+    end
+
+    describe '#image?' do
+      include_examples '#image? for an uploader'
+    end
+
+    describe '#video?' do
+      include_examples '#video? for an uploader'
+    end
+
+    describe '#image_or_video?' do
+      include_examples '#image? for an uploader'
+      include_examples '#video? for an uploader'
+    end
+
+    describe '#dangerous_image?' do
+      include_examples '#dangerous_image? for an uploader'
+    end
+
+    describe '#dangerous_video?' do
+      include_examples '#dangerous_video? for an uploader'
+    end
+
+    describe '#dangerous_image_or_video?' do
+      include_examples '#dangerous_image? for an uploader'
+      include_examples '#dangerous_video? for an uploader'
+    end
+  end
+
+  context 'when class is a regular class' do
+    shared_examples '#image? for a regular class' do
       it 'returns true for an image file' do
         allow(custom_class).to receive(:filename).and_return('dk.png')
-        expect(custom_class).to be_image_or_video
+        expect(custom_class).to be_image
       end
+      it 'returns false if file has a dangerous image extension' do
+        allow(custom_class).to receive(:filename).and_return('unsanitized.svg')
+
+        expect(custom_class).to be_dangerous_image
+        expect(custom_class).not_to be_image
+      end
+
+      it 'returns false for any non image file' do
+        allow(custom_class).to receive(:filename).and_return('video_sample.mp4')
+
+        expect(custom_class).not_to be_image
+      end
+
+      it 'returns false if filename is blank' do
+        allow(custom_class).to receive(:filename).and_return(nil)
+
+        expect(custom_class).not_to be_image
+      end
+    end
+
+    shared_examples '#video? for a regular class' do
       it 'returns true for a video file' do
         allow(custom_class).to receive(:filename).and_return('video_sample.mp4')
-        expect(custom_class).to be_image_or_video
+        expect(custom_class).to be_video
+      end
+
+      it 'returns false for any non-video file' do
+        allow(custom_class).to receive(:filename).and_return('dk.png')
+
+        expect(custom_class).not_to be_video
+      end
+
+      it 'returns false if file has a dangerous image extension' do
+        allow(custom_class).to receive(:filename).and_return('unsanitized.svg')
+
+        expect(custom_class).to be_dangerous_image
+        expect(custom_class).not_to be_video
+      end
+
+      it 'returns false if filename is blank' do
+        allow(custom_class).to receive(:filename).and_return(nil)
+
+        expect(custom_class).not_to be_video
+      end
+    end
+
+    shared_examples '#dangerous_image? for a regular class' do
+      it 'returns true if file has a dangerous image extension' do
+        allow(custom_class).to receive(:filename).and_return('unsanitized.svg')
+
+        expect(custom_class).to be_dangerous_image
+      end
+
+      it 'returns false for an image file' do
+        allow(custom_class).to receive(:filename).and_return('dk.png')
+
+        expect(custom_class).not_to be_dangerous_image
+      end
+
+      it 'returns false for any non image file' do
+        allow(custom_class).to receive(:filename).and_return('video_sample.mp4')
+
+        expect(custom_class).not_to be_dangerous_image
+      end
+
+      it 'returns false if filename is blank' do
+        allow(custom_class).to receive(:filename).and_return(nil)
+
+        expect(custom_class).not_to be_dangerous_image
+      end
+    end
+
+    shared_examples '#dangerous_video? for a regular class' do
+      it 'returns false for a safe video file' do
+        allow(custom_class).to receive(:filename).and_return('video_sample.mp4')
+
+        expect(custom_class).not_to be_dangerous_video
+      end
+
+      it 'returns false for an image file' do
+        allow(custom_class).to receive(:filename).and_return('dk.png')
+
+        expect(custom_class).not_to be_dangerous_video
       end
-      it 'returns false for other extensions' do
-        allow(custom_class).to receive(:filename).and_return('doc_sample.txt')
-        expect(custom_class).not_to be_image_or_video
+      it 'returns false if file has a dangerous image extension' do
+        allow(custom_class).to receive(:filename).and_return('unsanitized.svg')
+        expect(custom_class).not_to be_dangerous_video
       end
       it 'returns false if filename is blank' do
         allow(custom_class).to receive(:filename).and_return(nil)
-        expect(custom_class).not_to be_image_or_video
+        expect(custom_class).not_to be_dangerous_video
       end
     end
+
+    let(:custom_class) do
+      custom_class = Class.new do
+        include Gitlab::FileTypeDetection
+      end
+
+      custom_class.new
+    end
+
+    describe '#image?' do
+      include_examples '#image? for a regular class'
+    end
+
+    describe '#video?' do
+      include_examples '#video? for a regular class'
+    end
+
+    describe '#image_or_video?' do
+      include_examples '#image? for a regular class'
+      include_examples '#video? for a regular class'
+    end
+
+    describe '#dangerous_image?' do
+      include_examples '#dangerous_image? for a regular class'
+    end
+
+    describe '#dangerous_video?' do
+      include_examples '#dangerous_video? for a regular class'
+    end
+
+    describe '#dangerous_image_or_video?' do
+      include_examples '#dangerous_image? for a regular class'
+      include_examples '#dangerous_video? for a regular class'
+    end
   end
 end

spec/lib/gitlab/usage_data_spec.rb

@@ -64,31 +64,29 @@ describe Gitlab::UsageData do
         avg_cycle_analytics
         influxdb_metrics_enabled
         prometheus_metrics_enabled
-        cycle_analytics_views
-        productivity_analytics_views
       ))
-
-      expect(subject).to include(
-        snippet_create: a_kind_of(Integer),
-        snippet_update: a_kind_of(Integer),
-        snippet_comment: a_kind_of(Integer),
-        merge_request_comment: a_kind_of(Integer),
-        merge_request_create: a_kind_of(Integer),
-        commit_comment: a_kind_of(Integer),
-        wiki_pages_create: a_kind_of(Integer),
-        wiki_pages_update: a_kind_of(Integer),
-        wiki_pages_delete: a_kind_of(Integer),
-        web_ide_views: a_kind_of(Integer),
-        web_ide_commits: a_kind_of(Integer),
-        web_ide_merge_requests: a_kind_of(Integer),
-        navbar_searches: a_kind_of(Integer),
-        cycle_analytics_views: a_kind_of(Integer),
-        productivity_analytics_views: a_kind_of(Integer),
-        source_code_pushes: a_kind_of(Integer)
-      )
     end
     it 'gathers usage counts' do
+      smau_keys = %i(
+        snippet_create
+        snippet_update
+        snippet_comment
+        merge_request_comment
+        merge_request_create
+        commit_comment
+        wiki_pages_create
+        wiki_pages_update
+        wiki_pages_delete
+        web_ide_views
+        web_ide_commits
+        web_ide_merge_requests
+        navbar_searches
+        cycle_analytics_views
+        productivity_analytics_views
+        source_code_pushes
+      )
+
       expected_keys = %i(
         assignee_lists
         boards
@@ -154,12 +152,13 @@ describe Gitlab::UsageData do
         uploads
         web_hooks
         user_preferences
-      )
+      ).push(*smau_keys)
       count_data = subject[:counts]
       expect(count_data[:boards]).to eq(1)
       expect(count_data[:projects]).to eq(4)
+      expect(count_data.values_at(*smau_keys)).to all(be_an(Integer))
       expect(count_data.keys).to include(*expected_keys)
       expect(expected_keys - count_data.keys).to be_empty
     end

spec/models/repository_spec.rb

@@ -1223,36 +1223,66 @@ describe Repository do
   end
   describe '#branch_exists?' do
-    it 'uses branch_names' do
-      allow(repository).to receive(:branch_names).and_return(['foobar'])
-      expect(repository.branch_exists?('foobar')).to eq(true)
-      expect(repository.branch_exists?('master')).to eq(false)
+    let(:branch) { repository.root_ref }
+    subject { repository.branch_exists?(branch) }
+
+    it 'delegates to branch_names when the cache is empty' do
+      repository.expire_branches_cache
+
+      expect(repository).to receive(:branch_names).and_call_original
+      is_expected.to eq(true)
+    end
+
+    it 'uses redis set caching when the cache is filled' do
+      repository.branch_names # ensure the branch name cache is filled
+
+      expect(repository)
+        .to receive(:branch_names_include?)
+        .with(branch)
+        .and_call_original
+
+      is_expected.to eq(true)
     end
   end
   describe '#tag_exists?' do
-    it 'uses tag_names' do
-      allow(repository).to receive(:tag_names).and_return(['foobar'])
-      expect(repository.tag_exists?('foobar')).to eq(true)
-      expect(repository.tag_exists?('master')).to eq(false)
+    let(:tag) { repository.tags.first.name }
+
+    subject { repository.tag_exists?(tag) }
+
+    it 'delegates to tag_names when the cache is empty' do
+      repository.expire_tags_cache
+
+      expect(repository).to receive(:tag_names).and_call_original
+      is_expected.to eq(true)
+    end
+
+    it 'uses redis set caching when the cache is filled' do
+      repository.tag_names # ensure the tag name cache is filled
+
+      expect(repository)
+        .to receive(:tag_names_include?)
+        .with(tag)
+        .and_call_original
+      is_expected.to eq(true)
     end
   end
-  describe '#branch_names', :use_clean_rails_memory_store_caching do
+  describe '#branch_names', :clean_gitlab_redis_cache do
     let(:fake_branch_names) { ['foobar'] }
     it 'gets cached across Repository instances' do
       allow(repository.raw_repository).to receive(:branch_names).once.and_return(fake_branch_names)
-      expect(repository.branch_names).to eq(fake_branch_names)
+      expect(repository.branch_names).to match_array(fake_branch_names)
       fresh_repository = Project.find(project.id).repository
       expect(fresh_repository.object_id).not_to eq(repository.object_id)
       expect(fresh_repository.raw_repository).not_to receive(:branch_names)
-      expect(fresh_repository.branch_names).to eq(fake_branch_names)
+      expect(fresh_repository.branch_names).to match_array(fake_branch_names)
     end
   end

spec/models/suggestion_spec.rb

@@ -38,16 +38,6 @@ describe Suggestion do
   end
   describe '#appliable?' do
-    context 'when note does not support suggestions' do
-      it 'returns false' do
-        expect_next_instance_of(DiffNote) do |note|
-          allow(note).to receive(:supports_suggestion?) { false }
-        end
-
-        expect(suggestion).not_to be_appliable
-      end
-    end
-
     context 'when patch is already applied' do
       let(:suggestion) { create(:suggestion, :applied) }

spec/requests/api/releases_spec.rb

@@ -54,6 +54,15 @@ describe API::Releases do
         expect(response).to match_response_schema('public_api/v4/releases')
       end
+
+      it 'returns rendered helper paths' do
+        get api("/projects/#{project.id}/releases", maintainer)
+
+        expect(json_response.first['commit_path']).to eq("/#{release_2.project.full_path}/commit/#{release_2.commit.id}")
+        expect(json_response.first['tag_path']).to eq("/#{release_2.project.full_path}/-/tags/#{release_2.tag}")
+        expect(json_response.second['commit_path']).to eq("/#{release_1.project.full_path}/commit/#{release_1.commit.id}")
+        expect(json_response.second['tag_path']).to eq("/#{release_1.project.full_path}/-/tags/#{release_1.tag}")
+      end
     end
     it 'returns an upcoming_release status for a future release' do
@@ -103,11 +112,13 @@ describe API::Releases do
         expect(response).to have_gitlab_http_status(:ok)
       end
-      it "does not expose tag, commit and source code" do
+      it "does not expose tag, commit, source code or helper paths" do
         get api("/projects/#{project.id}/releases", guest)
         expect(response).to match_response_schema('public_api/v4/release/releases_for_guest')
         expect(json_response[0]['assets']['count']).to eq(release.links.count)
+        expect(json_response[0]['commit_path']).to be_nil
+        expect(json_response[0]['tag_path']).to be_nil
       end
       context 'when project is public' do
@@ -119,11 +130,13 @@ describe API::Releases do
           expect(response).to have_gitlab_http_status(:ok)
         end
-        it "exposes tag, commit and source code" do
+        it "exposes tag, commit, source code and helper paths" do
           get api("/projects/#{project.id}/releases", guest)
           expect(response).to match_response_schema('public_api/v4/releases')
-          expect(json_response[0]['assets']['count']).to eq(release.links.count + release.sources.count)
+          expect(json_response.first['assets']['count']).to eq(release.links.count + release.sources.count)
+          expect(json_response.first['commit_path']).to eq("/#{release.project.full_path}/commit/#{release.commit.id}")
+          expect(json_response.first['tag_path']).to eq("/#{release.project.full_path}/-/tags/#{release.tag}")
         end
       end
     end
@@ -172,6 +185,8 @@ describe API::Releases do
         expect(json_response['author']['name']).to eq(maintainer.name)
         expect(json_response['commit']['id']).to eq(commit.id)
         expect(json_response['assets']['count']).to eq(4)
+        expect(json_response['commit_path']).to eq("/#{release.project.full_path}/commit/#{release.commit.id}")
+        expect(json_response['tag_path']).to eq("/#{release.project.full_path}/-/tags/#{release.tag}")
       end
       it 'matches response schema' do

spec/support/helpers/repo_helpers.rb

@@ -92,7 +92,7 @@ eos
     )
   end
-  def sample_compare
+  def sample_compare(extra_changes = [])
     changes = [
       {
         line_code: 'a5cc2925ca8258af241be7e5b0381edf30266302_20_20',
@@ -102,7 +102,7 @@ eos
         line_code: '7445606fbf8f3683cd42bdc54b05d7a0bc2dfc44_4_6',
         file_path: '.gitmodules'
       }
-    ]
+    ] + extra_changes
     commits = %w(
       5937ac0a7beb003549fc5fd26fc247adbce4a52e