diff options
author | Micaƫl Bergeron <mbergeron@gitlab.com> | 2018-04-23 19:59:53 +0300 |
---|---|---|
committer | John Northrup <john@gitlab.com> | 2018-04-23 19:59:53 +0300 |
commit | 741f333d23cd09de328c9f4035c16210cb97aa10 (patch) | |
tree | 7d7d064ad4b16848bd82a1c26876e5a4292151a3 | |
parent | c0eabb84f57aed587589839136dadff01836e888 (diff) |
Resolve "Avatar URLs are wrong when using a CDN path and Object Storage"
-rw-r--r-- | app/models/concerns/avatarable.rb | 5 | ||||
-rw-r--r-- | app/uploaders/gitlab_uploader.rb | 4 | ||||
-rw-r--r-- | changelogs/unreleased/44775-avatar-on-os-fails-with-cdn.yml | 5 | ||||
-rw-r--r-- | spec/models/concerns/avatarable_spec.rb | 16 |
4 files changed, 26 insertions, 4 deletions
diff --git a/app/models/concerns/avatarable.rb b/app/models/concerns/avatarable.rb index 7677891b9ce..13246a774e3 100644 --- a/app/models/concerns/avatarable.rb +++ b/app/models/concerns/avatarable.rb @@ -31,12 +31,13 @@ module Avatarable asset_host = ActionController::Base.asset_host use_asset_host = asset_host.present? + use_authentication = respond_to?(:public?) && !public? # Avatars for private and internal groups and projects require authentication to be viewed, # which means they can only be served by Rails, on the regular GitLab host. # If an asset host is configured, we need to return the fully qualified URL # instead of only the avatar path, so that Rails doesn't prefix it with the asset host. - if use_asset_host && respond_to?(:public?) && !public? + if use_asset_host && use_authentication use_asset_host = false only_path = false end @@ -49,6 +50,6 @@ module Avatarable url_base << gitlab_config.relative_url_root end - url_base + avatar.url + url_base + avatar.local_url end end diff --git a/app/uploaders/gitlab_uploader.rb b/app/uploaders/gitlab_uploader.rb index f12f0466a1d..f8a237178d9 100644 --- a/app/uploaders/gitlab_uploader.rb +++ b/app/uploaders/gitlab_uploader.rb @@ -65,6 +65,10 @@ class GitlabUploader < CarrierWave::Uploader::Base !!model end + def local_url + File.join('/', self.class.base_dir, dynamic_segment, filename) + end + private # Designed to be overridden by child uploaders that have a dynamic path diff --git a/changelogs/unreleased/44775-avatar-on-os-fails-with-cdn.yml b/changelogs/unreleased/44775-avatar-on-os-fails-with-cdn.yml new file mode 100644 index 00000000000..80b5b4a8abe --- /dev/null +++ b/changelogs/unreleased/44775-avatar-on-os-fails-with-cdn.yml @@ -0,0 +1,5 @@ +--- +title: Fixed wrong avatar URL when the avatar is on object storage. +merge_request: 18092 +author: +type: fixed diff --git a/spec/models/concerns/avatarable_spec.rb b/spec/models/concerns/avatarable_spec.rb index 3696e6f62fd..9faf21bfbbd 100644 --- a/spec/models/concerns/avatarable_spec.rb +++ b/spec/models/concerns/avatarable_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Avatarable do - set(:project) { create(:project, avatar: fixture_file_upload(File.join(Rails.root, 'spec/fixtures/dk.png'))) } + let(:project) { create(:project, :with_avatar) } let(:gitlab_host) { "https://gitlab.example.com" } let(:relative_url_root) { "/gitlab" } @@ -37,11 +37,23 @@ describe Avatarable do project.visibility_level = visibility_level end - let(:avatar_path) { (avatar_path_prefix + [project.avatar.url]).join } + let(:avatar_path) { (avatar_path_prefix + [project.avatar.local_url]).join } it 'returns the expected avatar path' do expect(project.avatar_path(only_path: only_path)).to eq(avatar_path) end + + context "when avatar is stored remotely" do + before do + stub_uploads_object_storage(AvatarUploader) + + project.avatar.migrate!(ObjectStorage::Store::REMOTE) + end + + it 'returns the expected avatar path' do + expect(project.avatar_url(only_path: only_path)).to eq(avatar_path) + end + end end end end |