Commit 8f584d5f authored by Valery Sizov's avatar Valery Sizov

Fix: Images cannot show when projects' path was changed

parent a2f0a365
...@@ -53,6 +53,7 @@ v 8.1.0 (unreleased) ...@@ -53,6 +53,7 @@ v 8.1.0 (unreleased)
- Add "New Page" button to Wiki Pages tab (Stan Hu) - Add "New Page" button to Wiki Pages tab (Stan Hu)
- Only render 404 page from /public - Only render 404 page from /public
- Hide passwords from services API (Alex Lossent) - Hide passwords from services API (Alex Lossent)
- Fix: Images cannot show when projects' path was changed
v 8.0.4 v 8.0.4
- Fix Message-ID header to be RFC 2111-compliant to prevent e-mails being dropped (Stan Hu) - Fix Message-ID header to be RFC 2111-compliant to prevent e-mails being dropped (Stan Hu)
......
...@@ -118,6 +118,8 @@ class Namespace < ActiveRecord::Base ...@@ -118,6 +118,8 @@ class Namespace < ActiveRecord::Base
gitlab_shell.add_namespace(path_was) gitlab_shell.add_namespace(path_was)
if gitlab_shell.mv_namespace(path_was, path) if gitlab_shell.mv_namespace(path_was, path)
Gitlab::UploadsTransfer.new.rename_namespace(path_was, path)
# If repositories moved successfully we need to # If repositories moved successfully we need to
# send update instructions to users. # send update instructions to users.
# However we cannot allow rollback since we moved namespace dir # However we cannot allow rollback since we moved namespace dir
......
...@@ -656,6 +656,8 @@ class Project < ActiveRecord::Base ...@@ -656,6 +656,8 @@ class Project < ActiveRecord::Base
# db changes in order to prevent out of sync between db and fs # db changes in order to prevent out of sync between db and fs
raise Exception.new('repository cannot be renamed') raise Exception.new('repository cannot be renamed')
end end
Gitlab::UploadsTransfer.new.rename_project(path_was, path, namespace.path)
end end
def hook_attrs def hook_attrs
......
...@@ -27,6 +27,7 @@ module Projects ...@@ -27,6 +27,7 @@ module Projects
def transfer(project, new_namespace) def transfer(project, new_namespace)
Project.transaction do Project.transaction do
old_path = project.path_with_namespace old_path = project.path_with_namespace
old_namespace = project.namespace
new_path = File.join(new_namespace.try(:path) || '', project.path) new_path = File.join(new_namespace.try(:path) || '', project.path)
if Project.where(path: project.path, namespace_id: new_namespace.try(:id)).present? if Project.where(path: project.path, namespace_id: new_namespace.try(:id)).present?
...@@ -51,6 +52,9 @@ module Projects ...@@ -51,6 +52,9 @@ module Projects
# clear project cached events # clear project cached events
project.reset_events_cache project.reset_events_cache
# Move uploads
Gitlab::UploadsTransfer.new.move_project(project.path, old_namespace.path, new_namespace.path)
true true
end end
end end
......
...@@ -26,7 +26,7 @@ class FileUploader < CarrierWave::Uploader::Base ...@@ -26,7 +26,7 @@ class FileUploader < CarrierWave::Uploader::Base
end end
def secure_url def secure_url
File.join(Gitlab.config.gitlab.url, @project.path_with_namespace, "uploads", @secret, file.filename) File.join("/uploads", @secret, file.filename)
end end
def file_storage? def file_storage?
......
...@@ -41,6 +41,8 @@ class Spinach::Features::AdminProjects < Spinach::FeatureSteps ...@@ -41,6 +41,8 @@ class Spinach::Features::AdminProjects < Spinach::FeatureSteps
end end
step 'I transfer project to group \'Web\'' do step 'I transfer project to group \'Web\'' do
allow_any_instance_of(Projects::TransferService).
to receive(:move_uploads_to_new_namespace).and_return(true)
find(:xpath, "//input[@id='new_namespace_id']").set group.id find(:xpath, "//input[@id='new_namespace_id']").set group.id
click_button 'Transfer' click_button 'Transfer'
end end
......
...@@ -48,6 +48,7 @@ module Gitlab ...@@ -48,6 +48,7 @@ module Gitlab
autoload :TableOfContentsFilter, 'gitlab/markdown/table_of_contents_filter' autoload :TableOfContentsFilter, 'gitlab/markdown/table_of_contents_filter'
autoload :TaskListFilter, 'gitlab/markdown/task_list_filter' autoload :TaskListFilter, 'gitlab/markdown/task_list_filter'
autoload :UserReferenceFilter, 'gitlab/markdown/user_reference_filter' autoload :UserReferenceFilter, 'gitlab/markdown/user_reference_filter'
autoload :UploadLinkFilter, 'gitlab/markdown/upload_link_filter'
# Public: Parse the provided text with GitLab-Flavored Markdown # Public: Parse the provided text with GitLab-Flavored Markdown
# #
...@@ -140,6 +141,7 @@ module Gitlab ...@@ -140,6 +141,7 @@ module Gitlab
Gitlab::Markdown::SyntaxHighlightFilter, Gitlab::Markdown::SyntaxHighlightFilter,
Gitlab::Markdown::SanitizationFilter, Gitlab::Markdown::SanitizationFilter,
Gitlab::Markdown::UploadLinkFilter,
Gitlab::Markdown::RelativeLinkFilter, Gitlab::Markdown::RelativeLinkFilter,
Gitlab::Markdown::EmojiFilter, Gitlab::Markdown::EmojiFilter,
Gitlab::Markdown::TableOfContentsFilter, Gitlab::Markdown::TableOfContentsFilter,
......
require 'gitlab/markdown'
require 'html/pipeline/filter'
require 'uri'
module Gitlab
module Markdown
# HTML filter that "fixes" relative upload links to files.
# Context options:
# :project (required) - Current project
#
class UploadLinkFilter < HTML::Pipeline::Filter
def call
doc.search('a').each do |el|
process_link_attr el.attribute('href')
end
doc.search('img').each do |el|
process_link_attr el.attribute('src')
end
doc
end
protected
def process_link_attr(html_attr)
return if html_attr.blank?
uri = html_attr.value
if uri.starts_with?("/uploads/")
html_attr.value = build_url(uri).to_s
end
end
def build_url(uri)
File.join(Gitlab.config.gitlab.url, context[:project].path_with_namespace, uri)
end
# Ensure that a :project key exists in context
#
# Note that while the key might exist, its value could be nil!
def validate
needs :project
end
end
end
end
module Gitlab
class UploadsTransfer
def move_project(project_path, namespace_path_was, namespace_path)
new_namespace_folder = File.join(root_dir, namespace_path)
FileUtils.mkdir_p(new_namespace_folder) unless Dir.exist?(new_namespace_folder)
from = File.join(root_dir, namespace_path_was, project_path)
to = File.join(root_dir, namespace_path, project_path)
move(from, to, "")
end
def rename_project(path_was, path, namespace_path)
base_dir = File.join(root_dir, namespace_path)
move(path_was, path, base_dir)
end
def rename_namespace(path_was, path)
move(path_was, path)
end
private
def move(path_was, path, base_dir = nil)
base_dir = root_dir unless base_dir
from = File.join(base_dir, path_was)
to = File.join(base_dir, path)
FileUtils.mv(from, to)
rescue Errno::ENOENT
false
end
def root_dir
File.join(Rails.root, "public", "uploads")
end
end
end
...@@ -33,7 +33,7 @@ describe Projects::UploadsController do ...@@ -33,7 +33,7 @@ describe Projects::UploadsController do
it 'returns a content with original filename, new link, and correct type.' do it 'returns a content with original filename, new link, and correct type.' do
expect(response.body).to match '\"alt\":\"rails_sample\"' expect(response.body).to match '\"alt\":\"rails_sample\"'
expect(response.body).to match "\"url\":\"http://localhost/#{project.path_with_namespace}/uploads" expect(response.body).to match "\"url\":\"/uploads"
expect(response.body).to match '\"is_image\":true' expect(response.body).to match '\"is_image\":true'
end end
end end
...@@ -49,7 +49,7 @@ describe Projects::UploadsController do ...@@ -49,7 +49,7 @@ describe Projects::UploadsController do
it 'returns a content with original filename, new link, and correct type.' do it 'returns a content with original filename, new link, and correct type.' do
expect(response.body).to match '\"alt\":\"doc_sample.txt\"' expect(response.body).to match '\"alt\":\"doc_sample.txt\"'
expect(response.body).to match "\"url\":\"http://localhost/#{project.path_with_namespace}/uploads" expect(response.body).to match "\"url\":\"/uploads"
expect(response.body).to match '\"is_image\":false' expect(response.body).to match '\"is_image\":false'
end end
end end
......
...@@ -13,7 +13,6 @@ describe Gitlab::Email::AttachmentUploader do ...@@ -13,7 +13,6 @@ describe Gitlab::Email::AttachmentUploader do
expect(link).not_to be_nil expect(link).not_to be_nil
expect(link[:is_image]).to be_truthy expect(link[:is_image]).to be_truthy
expect(link[:alt]).to eq("bricks") expect(link[:alt]).to eq("bricks")
expect(link[:url]).to include("/#{project.path_with_namespace}")
expect(link[:url]).to include("bricks.png") expect(link[:url]).to include("bricks.png")
end end
end end
......
# encoding: UTF-8
require 'spec_helper'
module Gitlab::Markdown
describe UploadLinkFilter do
def filter(doc, contexts = {})
contexts.reverse_merge!({
project: project
})
described_class.call(doc, contexts)
end
def image(path)
%(<img src="#{path}" />)
end
def link(path)
%(<a href="#{path}">#{path}</a>)
end
let(:project) { create(:project) }
shared_examples :preserve_unchanged do
it 'does not modify any relative URL in anchor' do
doc = filter(link('README.md'))
expect(doc.at_css('a')['href']).to eq 'README.md'
end
it 'does not modify any relative URL in image' do
doc = filter(image('files/images/logo-black.png'))
expect(doc.at_css('img')['src']).to eq 'files/images/logo-black.png'
end
end
it 'does not raise an exception on invalid URIs' do
act = link("://foo")
expect { filter(act) }.not_to raise_error
end
context 'with a valid repository' do
it 'rebuilds relative URL for a link' do
doc = filter(link('/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg'))
expect(doc.at_css('a')['href']).
to eq "#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg"
end
it 'rebuilds relative URL for an image' do
doc = filter(link('/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg'))
expect(doc.at_css('a')['href']).
to eq "#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg"
end
it 'does not modify absolute URL' do
doc = filter(link('http://example.com'))
expect(doc.at_css('a')['href']).to eq 'http://example.com'
end
it 'supports Unicode filenames' do
path = '/uploads/한글.png'
escaped = Addressable::URI.escape(path)
# Stub these methods so the file doesn't actually need to be in the repo
allow_any_instance_of(described_class).
to receive(:file_exists?).and_return(true)
allow_any_instance_of(described_class).
to receive(:image?).with(path).and_return(true)
doc = filter(image(escaped))
expect(doc.at_css('img')['src']).to match "#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}/uploads/%ED%95%9C%EA%B8%80.png"
end
end
end
end
require 'spec_helper'
describe Gitlab::UploadsTransfer do
before do
@root_dir = File.join(Rails.root, "public", "uploads")
@upload_transfer = Gitlab::UploadsTransfer.new
@project_path_was = "test_project_was"
@project_path = "test_project"
@namespace_path_was = "test_namespace_was"
@namespace_path = "test_namespace"
end
after do
FileUtils.rm_rf([
File.join(@root_dir, @namespace_path),
File.join(@root_dir, @namespace_path_was)
])
end
describe '#move_project' do
it "moves project upload to another namespace" do
FileUtils.mkdir_p(File.join(@root_dir, @namespace_path_was, @project_path))
@upload_transfer.move_project(@project_path, @namespace_path_was, @namespace_path)
expected_path = File.join(@root_dir, @namespace_path, @project_path)
expect(Dir.exist?(expected_path)).to be_truthy
end
end
describe '#rename_project' do
it "renames project" do
FileUtils.mkdir_p(File.join(@root_dir, @namespace_path, @project_path_was))
@upload_transfer.rename_project(@project_path_was, @project_path, @namespace_path)
expected_path = File.join(@root_dir, @namespace_path, @project_path)
expect(Dir.exist?(expected_path)).to be_truthy
end
end
describe '#rename_namespace' do
it "renames namespace" do
FileUtils.mkdir_p(File.join(@root_dir, @namespace_path_was, @project_path))
@upload_transfer.rename_namespace(@namespace_path_was, @namespace_path)
expected_path = File.join(@root_dir, @namespace_path, @project_path)
expect(Dir.exist?(expected_path)).to be_truthy
end
end
end
...@@ -37,7 +37,6 @@ describe Projects::DownloadService do ...@@ -37,7 +37,6 @@ describe Projects::DownloadService do
it { expect(@link_to_file).to have_key('url') } it { expect(@link_to_file).to have_key('url') }
it { expect(@link_to_file).to have_key('is_image') } it { expect(@link_to_file).to have_key('is_image') }
it { expect(@link_to_file['is_image']).to be true } it { expect(@link_to_file['is_image']).to be true }
it { expect(@link_to_file['url']).to match("/#{@project.path_with_namespace}") }
it { expect(@link_to_file['url']).to match('rails_sample.jpg') } it { expect(@link_to_file['url']).to match('rails_sample.jpg') }
it { expect(@link_to_file['alt']).to eq('rails_sample') } it { expect(@link_to_file['alt']).to eq('rails_sample') }
end end
...@@ -52,7 +51,6 @@ describe Projects::DownloadService do ...@@ -52,7 +51,6 @@ describe Projects::DownloadService do
it { expect(@link_to_file).to have_key('url') } it { expect(@link_to_file).to have_key('url') }
it { expect(@link_to_file).to have_key('is_image') } it { expect(@link_to_file).to have_key('is_image') }
it { expect(@link_to_file['is_image']).to be false } it { expect(@link_to_file['is_image']).to be false }
it { expect(@link_to_file['url']).to match("/#{@project.path_with_namespace}") }
it { expect(@link_to_file['url']).to match('doc_sample.txt') } it { expect(@link_to_file['url']).to match('doc_sample.txt') }
it { expect(@link_to_file['alt']).to eq('doc_sample.txt') } it { expect(@link_to_file['alt']).to eq('doc_sample.txt') }
end end
......
...@@ -7,6 +7,8 @@ describe Projects::TransferService do ...@@ -7,6 +7,8 @@ describe Projects::TransferService do
context 'namespace -> namespace' do context 'namespace -> namespace' do
before do before do
allow_any_instance_of(Gitlab::UploadsTransfer).
to receive(:move_project).and_return(true)
group.add_owner(user) group.add_owner(user)
@result = transfer_project(project, user, group) @result = transfer_project(project, user, group)
end end
......
...@@ -18,7 +18,6 @@ describe Projects::UploadService do ...@@ -18,7 +18,6 @@ describe Projects::UploadService do
it { expect(@link_to_file).to have_key(:is_image) } it { expect(@link_to_file).to have_key(:is_image) }
it { expect(@link_to_file).to have_value('banana_sample') } it { expect(@link_to_file).to have_value('banana_sample') }
it { expect(@link_to_file[:is_image]).to equal(true) } it { expect(@link_to_file[:is_image]).to equal(true) }
it { expect(@link_to_file[:url]).to match("/#{@project.path_with_namespace}") }
it { expect(@link_to_file[:url]).to match('banana_sample.gif') } it { expect(@link_to_file[:url]).to match('banana_sample.gif') }
end end
...@@ -34,7 +33,6 @@ describe Projects::UploadService do ...@@ -34,7 +33,6 @@ describe Projects::UploadService do
it { expect(@link_to_file).to have_value('dk') } it { expect(@link_to_file).to have_value('dk') }
it { expect(@link_to_file).to have_key(:is_image) } it { expect(@link_to_file).to have_key(:is_image) }
it { expect(@link_to_file[:is_image]).to equal(true) } it { expect(@link_to_file[:is_image]).to equal(true) }
it { expect(@link_to_file[:url]).to match("/#{@project.path_with_namespace}") }
it { expect(@link_to_file[:url]).to match('dk.png') } it { expect(@link_to_file[:url]).to match('dk.png') }
end end
...@@ -49,7 +47,6 @@ describe Projects::UploadService do ...@@ -49,7 +47,6 @@ describe Projects::UploadService do
it { expect(@link_to_file).to have_key(:is_image) } it { expect(@link_to_file).to have_key(:is_image) }
it { expect(@link_to_file).to have_value('rails_sample') } it { expect(@link_to_file).to have_value('rails_sample') }
it { expect(@link_to_file[:is_image]).to equal(true) } it { expect(@link_to_file[:is_image]).to equal(true) }
it { expect(@link_to_file[:url]).to match("/#{@project.path_with_namespace}") }
it { expect(@link_to_file[:url]).to match('rails_sample.jpg') } it { expect(@link_to_file[:url]).to match('rails_sample.jpg') }
end end
...@@ -64,7 +61,6 @@ describe Projects::UploadService do ...@@ -64,7 +61,6 @@ describe Projects::UploadService do
it { expect(@link_to_file).to have_key(:is_image) } it { expect(@link_to_file).to have_key(:is_image) }
it { expect(@link_to_file).to have_value('doc_sample.txt') } it { expect(@link_to_file).to have_value('doc_sample.txt') }
it { expect(@link_to_file[:is_image]).to equal(false) } it { expect(@link_to_file[:is_image]).to equal(false) }
it { expect(@link_to_file[:url]).to match("/#{@project.path_with_namespace}") }
it { expect(@link_to_file[:url]).to match('doc_sample.txt') } it { expect(@link_to_file[:url]).to match('doc_sample.txt') }
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment