diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-05-20 17:34:42 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-05-20 17:34:42 +0300 |
commit | 9f46488805e86b1bc341ea1620b866016c2ce5ed (patch) | |
tree | f9748c7e287041e37d6da49e0a29c9511dc34768 /spec/services/snippets | |
parent | dfc92d081ea0332d69c8aca2f0e745cb48ae5e6d (diff) |
Add latest changes from gitlab-org/gitlab@13-0-stable-ee
Diffstat (limited to 'spec/services/snippets')
-rw-r--r-- | spec/services/snippets/create_service_spec.rb | 154 | ||||
-rw-r--r-- | spec/services/snippets/update_service_spec.rb | 90 |
2 files changed, 145 insertions, 99 deletions
diff --git a/spec/services/snippets/create_service_spec.rb b/spec/services/snippets/create_service_spec.rb index c1a8a026b90..786fc3ec8dd 100644 --- a/spec/services/snippets/create_service_spec.rb +++ b/spec/services/snippets/create_service_spec.rb @@ -74,47 +74,6 @@ describe Snippets::CreateService do end end - shared_examples 'spam check is performed' do - shared_examples 'marked as spam' do - it 'marks a snippet as spam' do - expect(snippet).to be_spam - end - - it 'invalidates the snippet' do - expect(snippet).to be_invalid - end - - it 'creates a new spam_log' do - expect { snippet } - .to have_spam_log(title: snippet.title, noteable_type: snippet.class.name) - end - - it 'assigns a spam_log to an issue' do - expect(snippet.spam_log).to eq(SpamLog.last) - end - end - - let(:extra_opts) do - { visibility_level: Gitlab::VisibilityLevel::PUBLIC, request: double(:request, env: {}) } - end - - before do - expect_next_instance_of(Spam::AkismetService) do |akismet_service| - expect(akismet_service).to receive_messages(spam?: true) - end - end - - [true, false, nil].each do |allow_possible_spam| - context "when recaptcha_disabled flag is #{allow_possible_spam.inspect}" do - before do - stub_feature_flags(allow_possible_spam: allow_possible_spam) unless allow_possible_spam.nil? - end - - it_behaves_like 'marked as spam' - end - end - end - shared_examples 'snippet create data is tracked' do let(:counter) { Gitlab::UsageDataCounters::SnippetCounter } @@ -169,8 +128,8 @@ describe Snippets::CreateService do expect { subject }.not_to change { Snippet.count } end - it 'returns the error' do - expect(snippet.errors.full_messages).to include('Repository could not be created') + it 'returns a generic creation error' do + expect(snippet.errors[:repository]).to eq ['Error creating the snippet - Repository could not be created'] end it 'does not return a snippet with an id' do @@ -178,6 +137,14 @@ describe Snippets::CreateService do end end + context 'when repository creation fails with invalid file name' do + let(:extra_opts) { { file_name: 'invalid://file/name/here' } } + + it 'returns an appropriate error' do + expect(snippet.errors[:repository]).to eq ['Error creating the snippet - Invalid file name'] + end + end + context 'when the commit action fails' do before do allow_next_instance_of(SnippetRepository) do |instance| @@ -209,11 +176,11 @@ describe Snippets::CreateService do subject end - it 'returns the error' do + it 'returns a generic error' do response = subject expect(response).to be_error - expect(response.payload[:snippet].errors.full_messages).to eq ['foobar'] + expect(response.payload[:snippet].errors[:repository]).to eq ['Error creating the snippet'] end end @@ -228,36 +195,14 @@ describe Snippets::CreateService do expect(snippet.repository_exists?).to be_falsey end end - - context 'when feature flag :version_snippets is disabled' do - before do - stub_feature_flags(version_snippets: false) - end - - it 'does not create snippet repository' do - expect do - subject - end.to change(Snippet, :count).by(1) - - expect(snippet.repository_exists?).to be_falsey - end - - it 'does not try to commit files' do - expect_next_instance_of(described_class) do |instance| - expect(instance).not_to receive(:create_commit) - end - - subject - end - end end - shared_examples 'after_save callback to store_mentions' do + shared_examples 'after_save callback to store_mentions' do |mentionable_class| context 'when mentionable attributes change' do let(:extra_opts) { { description: "Description with #{user.to_reference}" } } it 'saves mentions' do - expect_next_instance_of(Snippet) do |instance| + expect_next_instance_of(mentionable_class) do |instance| expect(instance).to receive(:store_mentions!).and_call_original end expect(snippet.user_mentions.count).to eq 1 @@ -266,7 +211,7 @@ describe Snippets::CreateService do context 'when mentionable attributes do not change' do it 'does not call store_mentions' do - expect_next_instance_of(Snippet) do |instance| + expect_next_instance_of(mentionable_class) do |instance| expect(instance).not_to receive(:store_mentions!) end expect(snippet.user_mentions.count).to eq 0 @@ -277,7 +222,7 @@ describe Snippets::CreateService do it 'does not call store_mentions' do base_opts.delete(:title) - expect_next_instance_of(Snippet) do |instance| + expect_next_instance_of(mentionable_class) do |instance| expect(instance).not_to receive(:store_mentions!) end expect(snippet.valid?).to be false @@ -294,11 +239,25 @@ describe Snippets::CreateService do it_behaves_like 'a service that creates a snippet' it_behaves_like 'public visibility level restrictions apply' - it_behaves_like 'spam check is performed' + it_behaves_like 'snippets spam check is performed' it_behaves_like 'snippet create data is tracked' it_behaves_like 'an error service response when save fails' it_behaves_like 'creates repository and files' - it_behaves_like 'after_save callback to store_mentions' + it_behaves_like 'after_save callback to store_mentions', ProjectSnippet + + context 'when uploaded files are passed to the service' do + let(:extra_opts) { { files: ['foo'] } } + + it 'does not move uploaded files to the snippet' do + expect_next_instance_of(described_class) do |instance| + expect(instance).to receive(:move_temporary_files).and_call_original + end + + expect_any_instance_of(FileMover).not_to receive(:execute) + + subject + end + end end context 'when PersonalSnippet' do @@ -306,12 +265,55 @@ describe Snippets::CreateService do it_behaves_like 'a service that creates a snippet' it_behaves_like 'public visibility level restrictions apply' - it_behaves_like 'spam check is performed' + it_behaves_like 'snippets spam check is performed' it_behaves_like 'snippet create data is tracked' it_behaves_like 'an error service response when save fails' it_behaves_like 'creates repository and files' - pending('See https://gitlab.com/gitlab-org/gitlab/issues/30742') do - it_behaves_like 'after_save callback to store_mentions' + it_behaves_like 'after_save callback to store_mentions', PersonalSnippet + + context 'when the snippet description contains files' do + include FileMoverHelpers + + let(:title) { 'Title' } + let(:picture_secret) { SecureRandom.hex } + let(:text_secret) { SecureRandom.hex } + let(:picture_file) { "/-/system/user/#{creator.id}/#{picture_secret}/picture.jpg" } + let(:text_file) { "/-/system/user/#{creator.id}/#{text_secret}/text.txt" } + let(:files) { [picture_file, text_file] } + let(:description) do + "Description with picture: ![picture](/uploads#{picture_file}) and "\ + "text: [text.txt](/uploads#{text_file})" + end + + before do + allow(FileUtils).to receive(:mkdir_p) + allow(FileUtils).to receive(:move) + end + + let(:extra_opts) { { description: description, title: title, files: files } } + + it 'stores the snippet description correctly' do + stub_file_mover(text_file) + stub_file_mover(picture_file) + + snippet = subject.payload[:snippet] + + expected_description = "Description with picture: "\ + "![picture](/uploads/-/system/personal_snippet/#{snippet.id}/#{picture_secret}/picture.jpg) and "\ + "text: [text.txt](/uploads/-/system/personal_snippet/#{snippet.id}/#{text_secret}/text.txt)" + + expect(snippet.description).to eq(expected_description) + end + + context 'when there is a validation error' do + let(:title) { nil } + + it 'does not move uploaded files to the snippet' do + expect_any_instance_of(described_class).not_to receive(:move_temporary_files) + + subject + end + end end end end diff --git a/spec/services/snippets/update_service_spec.rb b/spec/services/snippets/update_service_spec.rb index 05fb725c065..38747ae907f 100644 --- a/spec/services/snippets/update_service_spec.rb +++ b/spec/services/snippets/update_service_spec.rb @@ -7,7 +7,7 @@ describe Snippets::UpdateService do let_it_be(:user) { create(:user) } let_it_be(:admin) { create :user, admin: true } let(:visibility_level) { Gitlab::VisibilityLevel::PRIVATE } - let(:options) do + let(:base_opts) do { title: 'Test snippet', file_name: 'snippet.rb', @@ -15,6 +15,8 @@ describe Snippets::UpdateService do visibility_level: visibility_level } end + let(:extra_opts) { {} } + let(:options) { base_opts.merge(extra_opts) } let(:updater) { user } let(:service) { Snippets::UpdateService.new(project, updater, options) } @@ -85,7 +87,7 @@ describe Snippets::UpdateService do end context 'when update fails' do - let(:options) { { title: '' } } + let(:extra_opts) { { title: '' } } it 'does not increment count' do expect { subject }.not_to change { counter.read(:update) } @@ -112,25 +114,16 @@ describe Snippets::UpdateService do expect(blob.data).to eq options[:content] end - context 'when the repository does not exist' do - it 'does not try to commit file' do - allow(snippet).to receive(:repository_exists?).and_return(false) - - expect(service).not_to receive(:create_commit) - - subject - end - end - - context 'when feature flag is disabled' do + context 'when the repository creation fails' do before do - stub_feature_flags(version_snippets: false) + allow(snippet).to receive(:repository_exists?).and_return(false) end - it 'does not create repository' do - subject + it 'raise an error' do + response = subject - expect(snippet.repository).not_to exist + expect(response).to be_error + expect(response.payload[:snippet].errors[:repository].to_sentence).to eq 'Error updating the snippet - Repository could not be created' end it 'does not try to commit file' do @@ -205,14 +198,24 @@ describe Snippets::UpdateService do end end - it 'rolls back any snippet modifications' do - option_keys = options.stringify_keys.keys - orig_attrs = snippet.attributes.select { |k, v| k.in?(option_keys) } + context 'with snippet modifications' do + let(:option_keys) { options.stringify_keys.keys } - subject + it 'rolls back any snippet modifications' do + orig_attrs = snippet.attributes.select { |k, v| k.in?(option_keys) } + + subject + + persisted_attrs = snippet.reload.attributes.select { |k, v| k.in?(option_keys) } + expect(orig_attrs).to eq persisted_attrs + end + + it 'keeps any snippet modifications' do + subject - current_attrs = snippet.attributes.select { |k, v| k.in?(option_keys) } - expect(orig_attrs).to eq current_attrs + instance_attrs = snippet.attributes.select { |k, v| k.in?(option_keys) } + expect(options.stringify_keys).to eq instance_attrs + end end end @@ -270,6 +273,35 @@ describe Snippets::UpdateService do end end + shared_examples 'committable attributes' do + context 'when file_name is updated' do + let(:extra_opts) { { file_name: 'snippet.rb' } } + + it 'commits to repository' do + expect(service).to receive(:create_commit) + expect(subject).to be_success + end + end + + context 'when content is updated' do + let(:extra_opts) { { content: 'puts "hello world"' } } + + it 'commits to repository' do + expect(service).to receive(:create_commit) + expect(subject).to be_success + end + end + + context 'when content or file_name is not updated' do + let(:options) { { title: 'Test snippet' } } + + it 'does not perform any commit' do + expect(service).not_to receive(:create_commit) + expect(subject).to be_success + end + end + end + context 'when Project Snippet' do let_it_be(:project) { create(:project) } let!(:snippet) { create(:project_snippet, :repository, author: user, project: project) } @@ -283,6 +315,12 @@ describe Snippets::UpdateService do it_behaves_like 'snippet update data is tracked' it_behaves_like 'updates repository content' it_behaves_like 'commit operation fails' + it_behaves_like 'committable attributes' + it_behaves_like 'snippets spam check is performed' do + before do + subject + end + end context 'when snippet does not have a repository' do let!(:snippet) { create(:project_snippet, author: user, project: project) } @@ -301,6 +339,12 @@ describe Snippets::UpdateService do it_behaves_like 'snippet update data is tracked' it_behaves_like 'updates repository content' it_behaves_like 'commit operation fails' + it_behaves_like 'committable attributes' + it_behaves_like 'snippets spam check is performed' do + before do + subject + end + end context 'when snippet does not have a repository' do let!(:snippet) { create(:personal_snippet, author: user, project: project) } |