From f9475e299c6f6b363d5ea302f1295a3ea0bf9adb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Francisco=20Javier=20L=C3=B3pez?= Date: Tue, 4 Sep 2018 10:39:08 +0000 Subject: Uploads to wiki stored inside the wiki git repository --- .../projects/wiki/user_creates_wiki_page_spec.rb | 2 + .../projects/wiki/user_updates_wiki_page_spec.rb | 26 ++- .../projects/wiki/user_views_wiki_page_spec.rb | 2 +- spec/lib/banzai/filter/wiki_link_filter_spec.rb | 40 ++++ spec/lib/gitlab/file_markdown_link_builder_spec.rb | 80 ++++++++ spec/lib/gitlab/file_type_detection_spec.rb | 82 +++++++++ spec/requests/api/wikis_spec.rb | 124 +++++++++++++ .../wikis/create_attachment_service_spec.rb | 202 +++++++++++++++++++++ .../wiki_file_attachments_examples.rb | 88 +++++++++ spec/uploaders/uploader_helper_spec.rb | 25 +-- 10 files changed, 635 insertions(+), 36 deletions(-) create mode 100644 spec/lib/gitlab/file_markdown_link_builder_spec.rb create mode 100644 spec/lib/gitlab/file_type_detection_spec.rb create mode 100644 spec/services/wikis/create_attachment_service_spec.rb create mode 100644 spec/support/shared_examples/wiki_file_attachments_examples.rb (limited to 'spec') diff --git a/spec/features/projects/wiki/user_creates_wiki_page_spec.rb b/spec/features/projects/wiki/user_creates_wiki_page_spec.rb index 149eeb4f9ba..b30286e4446 100644 --- a/spec/features/projects/wiki/user_creates_wiki_page_spec.rb +++ b/spec/features/projects/wiki/user_creates_wiki_page_spec.rb @@ -146,6 +146,8 @@ describe "User creates wiki page" do expect(page).to have_selector(".katex", count: 3).and have_content("2+2 is 4") end end + + it_behaves_like 'wiki file attachments' end context "in a group namespace", :js do diff --git a/spec/features/projects/wiki/user_updates_wiki_page_spec.rb b/spec/features/projects/wiki/user_updates_wiki_page_spec.rb index 2840d28cf30..2ce5ee0e87d 100644 --- a/spec/features/projects/wiki/user_updates_wiki_page_spec.rb +++ b/spec/features/projects/wiki/user_updates_wiki_page_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' describe 'User updates wiki page' do shared_examples 'wiki page user update' do let(:user) { create(:user) } + before do project.add_maintainer(user) sign_in(user) @@ -55,6 +56,8 @@ describe 'User updates wiki page' do expect(page).to have_content('Updated Wiki Content') end + + it_behaves_like 'wiki file attachments' end end @@ -64,14 +67,14 @@ describe 'User updates wiki page' do before do visit(project_wikis_path(project)) + + click_link('Edit') end context 'in a user namespace' do let(:project) { create(:project, :wiki_repo, namespace: user.namespace) } it 'updates a page' do - click_link('Edit') - # Commit message field should have correct value. expect(page).to have_field('wiki[message]', with: 'Update home') @@ -84,8 +87,6 @@ describe 'User updates wiki page' do end it 'shows a validation error message' do - click_link('Edit') - fill_in(:wiki_content, with: '') click_button('Save changes') @@ -97,8 +98,6 @@ describe 'User updates wiki page' do end it 'shows the emoji autocompletion dropdown', :js do - click_link('Edit') - find('#wiki_content').native.send_keys('') fill_in(:wiki_content, with: ':') @@ -106,8 +105,6 @@ describe 'User updates wiki page' do end it 'shows the error message' do - click_link('Edit') - wiki_page.update(content: 'Update') click_button('Save changes') @@ -116,30 +113,27 @@ describe 'User updates wiki page' do end it 'updates a page' do - click_on('Edit') fill_in('Content', with: 'Updated Wiki Content') click_on('Save changes') expect(page).to have_content('Updated Wiki Content') end - it 'cancels edititng of a page' do - click_on('Edit') - + it 'cancels editing of a page' do page.within(:css, '.wiki-form .form-actions') do click_on('Cancel') end expect(current_path).to eq(project_wiki_path(project, wiki_page)) end + + it_behaves_like 'wiki file attachments' end context 'in a group namespace' do let(:project) { create(:project, :wiki_repo, namespace: create(:group, :public)) } it 'updates a page' do - click_link('Edit') - # Commit message field should have correct value. expect(page).to have_field('wiki[message]', with: 'Update home') @@ -151,6 +145,8 @@ describe 'User updates wiki page' do expect(page).to have_content("Last edited by #{user.name}") expect(page).to have_content('My awesome wiki!') end + + it_behaves_like 'wiki file attachments' end end @@ -222,6 +218,8 @@ describe 'User updates wiki page' do expect(current_path).to eq(project_wiki_path(project, "foo1/bar1/#{page_name}")) end + + it_behaves_like 'wiki file attachments' end end diff --git a/spec/features/projects/wiki/user_views_wiki_page_spec.rb b/spec/features/projects/wiki/user_views_wiki_page_spec.rb index 760324adacc..747406efc8b 100644 --- a/spec/features/projects/wiki/user_views_wiki_page_spec.rb +++ b/spec/features/projects/wiki/user_views_wiki_page_spec.rb @@ -93,7 +93,7 @@ describe 'User views a wiki page' do allow(wiki_file).to receive(:mime_type).and_return('image/jpeg') allow_any_instance_of(ProjectWiki).to receive(:find_file).with('image.jpg', nil).and_return(wiki_file) - expect(page).to have_xpath('//img[@data-src="image.jpg"]') + expect(page).to have_xpath("//img[@data-src='#{project.wiki.wiki_base_path}/image.jpg']") expect(page).to have_link('image', href: "#{project.wiki.wiki_base_path}/image.jpg") click_on('image') diff --git a/spec/lib/banzai/filter/wiki_link_filter_spec.rb b/spec/lib/banzai/filter/wiki_link_filter_spec.rb index 50d053011b3..b9059b85fdc 100644 --- a/spec/lib/banzai/filter/wiki_link_filter_spec.rb +++ b/spec/lib/banzai/filter/wiki_link_filter_spec.rb @@ -7,6 +7,7 @@ describe Banzai::Filter::WikiLinkFilter do let(:project) { build_stubbed(:project, :public, name: "wiki_link_project", namespace: namespace) } let(:user) { double } let(:wiki) { ProjectWiki.new(project, user) } + let(:repository_upload_folder) { Wikis::CreateAttachmentService::ATTACHMENT_PATH } it "doesn't rewrite absolute links" do filtered_link = filter("Link", project_wiki: wiki).children[0] @@ -20,6 +21,45 @@ describe Banzai::Filter::WikiLinkFilter do expect(filtered_link.attribute('href').value).to eq('/uploads/a.test') end + describe "when links point to the #{Wikis::CreateAttachmentService::ATTACHMENT_PATH} folder" do + context 'with an "a" html tag' do + it 'rewrites links' do + filtered_link = filter("Link", project_wiki: wiki).children[0] + + expect(filtered_link.attribute('href').value).to eq("#{wiki.wiki_base_path}/#{repository_upload_folder}/a.test") + end + end + + context 'with "img" html tag' do + let(:path) { "#{wiki.wiki_base_path}/#{repository_upload_folder}/a.jpg" } + + context 'inside an "a" html tag' do + it 'rewrites links' do + filtered_elements = filter("example", project_wiki: wiki) + + expect(filtered_elements.search('img').first.attribute('src').value).to eq(path) + expect(filtered_elements.search('a').first.attribute('href').value).to eq(path) + end + end + + context 'outside an "a" html tag' do + it 'rewrites links' do + filtered_link = filter("example", project_wiki: wiki).children[0] + + expect(filtered_link.attribute('src').value).to eq(path) + end + end + end + + context 'with "video" html tag' do + it 'rewrites links' do + filtered_link = filter("", project_wiki: wiki).children[0] + + expect(filtered_link.attribute('src').value).to eq("#{wiki.wiki_base_path}/#{repository_upload_folder}/a.mp4") + end + end + end + describe "invalid links" do invalid_links = ["http://:8080", "http://", "http://:8080/path"] diff --git a/spec/lib/gitlab/file_markdown_link_builder_spec.rb b/spec/lib/gitlab/file_markdown_link_builder_spec.rb new file mode 100644 index 00000000000..feb2776c5d0 --- /dev/null +++ b/spec/lib/gitlab/file_markdown_link_builder_spec.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true +require 'rails_helper' + +describe Gitlab::FileMarkdownLinkBuilder do + let(:custom_class) do + Class.new do + include Gitlab::FileMarkdownLinkBuilder + end.new + end + + before do + allow(custom_class).to receive(:filename).and_return(filename) + end + + describe 'markdown_link' do + let(:url) { "/uploads/#{filename}"} + + before do + allow(custom_class).to receive(:secure_url).and_return(url) + end + + context 'when file name has the character ]' do + let(:filename) { 'd]k.png' } + + it 'escapes the character' do + expect(custom_class.markdown_link).to eq '![d\\]k](/uploads/d]k.png)' + end + end + + context 'when file is an image or video' do + let(:filename) { 'dk.png' } + + it 'returns preview markdown link' do + expect(custom_class.markdown_link).to eq '![dk](/uploads/dk.png)' + end + end + + context 'when file is not an image or video' do + let(:filename) { 'dk.zip' } + + it 'returns markdown link' do + expect(custom_class.markdown_link).to eq '[dk.zip](/uploads/dk.zip)' + end + end + + context 'when file name is blank' do + let(:filename) { nil } + + it 'returns nil' do + expect(custom_class.markdown_link).to eq nil + end + end + end + + describe 'mardown_name' do + context 'when file is an image or video' do + let(:filename) { 'dk.png' } + + it 'retrieves the name without the extension' do + expect(custom_class.markdown_name).to eq 'dk' + end + end + + context 'when file is not an image or video' do + let(:filename) { 'dk.zip' } + + it 'retrieves the name with the extesion' do + expect(custom_class.markdown_name).to eq 'dk.zip' + end + end + + context 'when file name is blank' do + let(:filename) { nil } + + it 'returns nil' do + expect(custom_class.markdown_name).to eq nil + end + end + end +end diff --git a/spec/lib/gitlab/file_type_detection_spec.rb b/spec/lib/gitlab/file_type_detection_spec.rb new file mode 100644 index 00000000000..5e9b8988cc8 --- /dev/null +++ b/spec/lib/gitlab/file_type_detection_spec.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true +require 'rails_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 + end + + it 'returns true for an image file' do + uploader.store!(upload_fixture('dk.png')) + + expect(uploader).to be_image_or_video + end + + it 'returns true for a video file' do + uploader.store!(upload_fixture('video_sample.mp4')) + + expect(uploader).to be_image_or_video + end + + it 'returns false for other extensions' do + uploader.store!(upload_fixture('doc_sample.txt')) + + expect(uploader).not_to be_image_or_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_image_or_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 + end + + 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 + end + + 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 + 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 + 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 + end + end + end +end diff --git a/spec/requests/api/wikis_spec.rb b/spec/requests/api/wikis_spec.rb index 489cb001b82..c40d01e1a14 100644 --- a/spec/requests/api/wikis_spec.rb +++ b/spec/requests/api/wikis_spec.rb @@ -139,6 +139,27 @@ describe API::Wikis do end end + shared_examples_for 'uploads wiki attachment' do + it 'pushes attachment to the wiki repository' do + allow(SecureRandom).to receive(:hex).and_return('fixed_hex') + + post(api(url, user), payload) + + expect(response).to have_gitlab_http_status(201) + expect(json_response).to eq result_hash.deep_stringify_keys + end + + it 'responds with validation error on empty file' do + payload.delete(:file) + + post(api(url, user), payload) + + expect(response).to have_gitlab_http_status(400) + expect(json_response.size).to eq(1) + expect(json_response['error']).to eq('file is missing') + end + end + describe 'GET /projects/:id/wikis' do let(:url) { "/projects/#{project.id}/wikis" } @@ -698,4 +719,107 @@ describe API::Wikis do include_examples '204 No Content' end end + + describe 'POST /projects/:id/wikis/attachments' do + let(:payload) { { file: fixture_file_upload('spec/fixtures/dk.png') } } + let(:url) { "/projects/#{project.id}/wikis/attachments" } + let(:file_path) { "#{Wikis::CreateAttachmentService::ATTACHMENT_PATH}/fixed_hex/dk.png" } + let(:result_hash) do + { + file_name: 'dk.png', + file_path: file_path, + branch: 'master', + link: { + url: file_path, + markdown: "![dk](#{file_path})" + } + } + end + + context 'when wiki is disabled' do + let(:project) { create(:project, :wiki_disabled, :wiki_repo) } + + context 'when user is guest' do + before do + post(api(url), payload) + end + + include_examples '404 Project Not Found' + end + + context 'when user is developer' do + before do + project.add_developer(user) + post(api(url, user), payload) + end + + include_examples '403 Forbidden' + end + + context 'when user is maintainer' do + before do + project.add_maintainer(user) + post(api(url, user), payload) + end + + include_examples '403 Forbidden' + end + end + + context 'when wiki is available only for team members' do + let(:project) { create(:project, :wiki_private, :wiki_repo) } + + context 'when user is guest' do + before do + post(api(url), payload) + end + + include_examples '404 Project Not Found' + end + + context 'when user is developer' do + before do + project.add_developer(user) + end + + include_examples 'uploads wiki attachment' + end + + context 'when user is maintainer' do + before do + project.add_maintainer(user) + end + + include_examples 'uploads wiki attachment' + end + end + + context 'when wiki is available for everyone with access' do + let(:project) { create(:project, :wiki_repo) } + + context 'when user is guest' do + before do + post(api(url), payload) + end + + include_examples '404 Project Not Found' + end + + context 'when user is developer' do + before do + project.add_developer(user) + end + + include_examples 'uploads wiki attachment' + end + + context 'when user is maintainer' do + before do + project.add_maintainer(user) + end + + include_examples 'uploads wiki attachment' + end + end + end end diff --git a/spec/services/wikis/create_attachment_service_spec.rb b/spec/services/wikis/create_attachment_service_spec.rb new file mode 100644 index 00000000000..3f4da873ce4 --- /dev/null +++ b/spec/services/wikis/create_attachment_service_spec.rb @@ -0,0 +1,202 @@ +# frozen_string_literal: true +require 'spec_helper' + +describe Wikis::CreateAttachmentService do + let(:project) { create(:project, :wiki_repo) } + let(:user) { create(:user) } + let(:file_name) { 'filename.txt' } + let(:file_path_regex) { %r{#{described_class::ATTACHMENT_PATH}/\h{32}/#{file_name}} } + + let(:file_opts) do + { + file_name: file_name, + file_content: 'Content of attachment' + } + end + let(:opts) { file_opts } + + subject(:service) { described_class.new(project, user, opts) } + + before do + project.add_developer(user) + end + + describe 'initialization' do + context 'author commit info' do + it 'does not raise error if user is nil' do + service = described_class.new(project, nil, opts) + + expect(service.instance_variable_get(:@author_email)).to be_nil + expect(service.instance_variable_get(:@author_name)).to be_nil + end + + it 'fills file_path from the repository uploads folder' do + expect(service.instance_variable_get(:@file_path)).to match(file_path_regex) + end + + context 'when no author info provided' do + it 'fills author_email and author_name from current_user info' do + expect(service.instance_variable_get(:@author_email)).to eq user.email + expect(service.instance_variable_get(:@author_name)).to eq user.name + end + end + + context 'when author info provided' do + let(:author_email) { 'author_email' } + let(:author_name) { 'author_name' } + let(:opts) { file_opts.merge(author_email: author_email, author_name: author_name) } + + it 'fills author_email and author_name from params' do + expect(service.instance_variable_get(:@author_email)).to eq author_email + expect(service.instance_variable_get(:@author_name)).to eq author_name + end + end + end + + context 'commit message' do + context 'when no commit message provided' do + it 'sets a default commit message' do + expect(service.instance_variable_get(:@commit_message)).to eq "Upload attachment #{opts[:file_name]}" + end + end + + context 'when commit message provided' do + let(:commit_message) { 'whatever' } + let(:opts) { file_opts.merge(commit_message: commit_message) } + + it 'use the commit message from params' do + expect(service.instance_variable_get(:@commit_message)).to eq commit_message + end + end + end + + context 'branch name' do + context 'when no branch provided' do + it 'sets the branch from the wiki default_branch' do + expect(service.instance_variable_get(:@branch_name)).to eq project.wiki.default_branch + end + end + + context 'when branch provided' do + let(:branch_name) { 'whatever' } + let(:opts) { file_opts.merge(branch_name: branch_name) } + + it 'use the commit message from params' do + expect(service.instance_variable_get(:@branch_name)).to eq branch_name + end + end + end + end + + describe 'validations' do + context 'when file_name' do + context 'is not present' do + let(:file_name) { nil } + + it 'returns error' do + result = service.execute + + expect(result[:status]).to eq :error + expect(result[:message]).to eq 'The file name cannot be empty' + end + end + + context 'length' do + context 'is bigger than 255' do + let(:file_name) { "#{'0' * 256}.jpg" } + + it 'truncates file name' do + result = service.execute + + expect(result[:status]).to eq :success + expect(result[:result][:file_name].length).to eq 255 + expect(result[:result][:file_name]).to match(/0{251}\.jpg/) + end + end + + context 'is less or equal to 255 does not return error' do + let(:file_name) { '0' * 255 } + + it 'does not return error' do + result = service.execute + + expect(result[:status]).to eq :success + end + end + end + end + + context 'when user' do + shared_examples 'wiki attachment user validations' do + it 'returns error' do + result = described_class.new(project, user2, opts).execute + + expect(result[:status]).to eq :error + expect(result[:message]).to eq 'You are not allowed to push to the wiki' + end + end + + context 'does not have permission' do + let(:user2) { create(:user) } + + it_behaves_like 'wiki attachment user validations' + end + + context 'is nil' do + let(:user2) { nil } + + it_behaves_like 'wiki attachment user validations' + end + end + end + + describe '#execute' do + let(:wiki) { project.wiki } + subject(:service_execute) { service.execute[:result] } + + context 'creates branch if it does not exists' do + let(:branch_name) { 'new_branch' } + let(:opts) { file_opts.merge(branch_name: branch_name) } + + it do + expect(wiki.repository.branches).to be_empty + expect { service.execute }.to change { wiki.repository.branches.count }.by(1) + expect(wiki.repository.branches.first.name).to eq branch_name + end + end + + it 'adds file to the repository' do + expect(wiki.repository.ls_files('HEAD')).to be_empty + + service.execute + + files = wiki.repository.ls_files('HEAD') + expect(files.count).to eq 1 + expect(files.first).to match(file_path_regex) + end + + context 'returns' do + before do + allow(SecureRandom).to receive(:hex).and_return('fixed_hex') + + service_execute + end + + it 'returns the file name' do + expect(service_execute[:file_name]).to eq file_name + end + + it 'returns the path where file was stored' do + expect(service_execute[:file_path]).to eq 'uploads/fixed_hex/filename.txt' + end + + it 'returns the branch where the file was pushed' do + expect(service_execute[:branch]).to eq wiki.default_branch + end + + it 'returns the commit id' do + expect(service_execute[:commit]).not_to be_empty + end + end + end +end diff --git a/spec/support/shared_examples/wiki_file_attachments_examples.rb b/spec/support/shared_examples/wiki_file_attachments_examples.rb new file mode 100644 index 00000000000..b6fb2a66b0e --- /dev/null +++ b/spec/support/shared_examples/wiki_file_attachments_examples.rb @@ -0,0 +1,88 @@ +# frozen_string_literal: true + +# Requires a context containing: +# project + +shared_examples 'wiki file attachments' do + include DropzoneHelper + + context 'uploading attachments', :js do + let(:wiki) { project.wiki } + + def attach_with_dropzone(wait = false) + dropzone_file([Rails.root.join('spec', 'fixtures', 'dk.png')], 0, wait) + end + + context 'before uploading' do + it 'shows "Attach a file" button' do + expect(page).to have_button('Attach a file') + expect(page).not_to have_selector('.uploading-progress-container', visible: true) + end + end + + context 'uploading is in progress' do + it 'cancels uploading on clicking to "Cancel" button' do + slow_requests do + attach_with_dropzone + + click_button 'Cancel' + end + + expect(page).to have_button('Attach a file') + expect(page).not_to have_button('Cancel') + expect(page).not_to have_selector('.uploading-progress-container', visible: true) + end + + it 'shows "Attaching a file" message on uploading 1 file' do + slow_requests do + attach_with_dropzone + + expect(page).to have_selector('.attaching-file-message', visible: true, text: 'Attaching a file -') + end + end + end + + context 'uploading is complete' do + it 'shows "Attach a file" button on uploading complete' do + attach_with_dropzone + wait_for_requests + + expect(page).to have_button('Attach a file') + expect(page).not_to have_selector('.uploading-progress-container', visible: true) + end + + it 'the markdown link is added to the page' do + fill_in(:wiki_content, with: '') + attach_with_dropzone(true) + wait_for_requests + + expect(page.find('#wiki_content').value) + .to match(%r{\!\[dk\]\(uploads/\h{32}/dk\.png\)$}) + end + + it 'the links point to the wiki root url' do + attach_with_dropzone(true) + wait_for_requests + + find('.js-md-preview-button').click + file_path = page.find('input[name="files[]"]', visible: :hidden).value + link = page.find('a.no-attachment-icon')['href'] + img_link = page.find('a.no-attachment-icon img')['src'] + + expect(link).to eq img_link + expect(URI.parse(link).path).to eq File.join(wiki.wiki_base_path, file_path) + end + + it 'the file has been added to the wiki repository' do + expect do + attach_with_dropzone(true) + wait_for_requests + end.to change { wiki.repository.ls_files('HEAD').count }.by(1) + + file_path = page.find('input[name="files[]"]', visible: :hidden).value + + expect(wiki.find_file(file_path, 'HEAD').path).not_to be_nil + end + end + end +end diff --git a/spec/uploaders/uploader_helper_spec.rb b/spec/uploaders/uploader_helper_spec.rb index 33da93cc9d0..fd6712d4645 100644 --- a/spec/uploaders/uploader_helper_spec.rb +++ b/spec/uploaders/uploader_helper_spec.rb @@ -11,27 +11,10 @@ describe UploaderHelper do example_uploader.new end - def upload_fixture(filename) - fixture_file_upload(File.join('spec', 'fixtures', filename)) - end - - describe '#image_or_video?' do - it 'returns true for an image file' do - uploader.store!(upload_fixture('dk.png')) - - expect(uploader).to be_image_or_video - end - - it 'it returns true for a video file' do - uploader.store!(upload_fixture('video_sample.mp4')) - - expect(uploader).to be_image_or_video - end - - it 'returns false for other extensions' do - uploader.store!(upload_fixture('doc_sample.txt')) - - expect(uploader).not_to be_image_or_video + describe '#extension_match?' do + it 'returns false if file does not exists' do + expect(uploader.file).to be_nil + expect(uploader.send(:extension_match?, 'jpg')).to eq false end end end -- cgit v1.2.1