BUGFIX: we should not store absolute urls for locally uploaded avatar templates

Highly recommended to run: `RAILS_ENV=production bundle exec rake avatars:regenerate` to fix the avatar templates stored in the database.
This commit is contained in:
Régis Hanol 2014-01-07 17:45:06 +01:00
parent 0e7f2bc3a9
commit e732aa8a86
9 changed files with 29 additions and 24 deletions

View File

@ -57,7 +57,7 @@ module Jobs
external_copy.close! if Discourse.store.external?
# attach the avatar to the user
user.uploaded_avatar_template = Discourse.store.absolute_avatar_template(upload)
user.uploaded_avatar_template = Discourse.store.avatar_template(upload)
user.save!
end

View File

@ -325,7 +325,7 @@ class User < ActiveRecord::Base
def uploaded_avatar_path
return unless SiteSetting.allow_uploaded_avatars? && use_uploaded_avatar
avatar_template = uploaded_avatar_template.present? ? uploaded_avatar_template : uploaded_avatar.try(:url)
schemaless avatar_template
schemaless absolute avatar_template
end
def avatar_template

View File

@ -38,7 +38,7 @@ module FileStore
def download(upload)
end
def absolute_avatar_template(avatar)
def avatar_template(avatar)
end
def purge_tombstone(grace_period)

View File

@ -51,8 +51,8 @@ module FileStore
"#{public_dir}#{upload.url}"
end
def absolute_avatar_template(avatar)
avatar_template(avatar, absolute_base_url)
def avatar_template(avatar)
relative_avatar_template(avatar)
end
def purge_tombstone(grace_period)
@ -85,12 +85,8 @@ module FileStore
end
def relative_avatar_template(avatar)
avatar_template(avatar, relative_base_url)
end
def avatar_template(avatar, base_url)
File.join(
base_url,
relative_base_url,
"avatars",
avatar.sha1[0..2],
avatar.sha1[3..5],

View File

@ -58,8 +58,9 @@ module FileStore
temp_file
end
def absolute_avatar_template(avatar)
"#{absolute_base_url}/avatars/#{avatar.sha1}/{size}#{avatar.extension}"
def avatar_template(avatar)
template = relative_avatar_template(avatar)
"#{absolute_base_url}/#{template}"
end
def purge_tombstone(grace_period)
@ -77,7 +78,11 @@ module FileStore
end
def get_path_for_avatar(file, avatar, size)
"avatars/#{avatar.sha1}/#{size}#{avatar.extension}"
relative_avatar_template(avatar).gsub("{size}", size.to_s)
end
def relative_avatar_template(avatar)
"avatars/#{avatar.sha1}/{size}#{avatar.extension}"
end
def store_file(file, path, filename = nil, content_type = nil)

View File

@ -2,12 +2,11 @@ desc "re-generate avatars"
task "avatars:regenerate" => :environment do
RailsMultisite::ConnectionManagement.each_connection do |db|
puts "Generating avatars for: #{db}"
User.where("uploaded_avatar_id IS NOT NULL").all.each do |u|
next unless SiteSetting.allow_uploaded_avatars
User.where("uploaded_avatar_id IS NOT NULL").find_each do |u|
Jobs.enqueue(:generate_avatars, upload_id: u.uploaded_avatar_id, user_id: u.id)
putc "."
end
end
puts "\ndone."
end

View File

@ -122,10 +122,10 @@ describe FileStore::LocalStore do
store.external?.should == false
end
describe ".absolute_avatar_template" do
describe ".avatar_template" do
it "is present" do
store.absolute_avatar_template(avatar).should == "http://test.localhost/uploads/default/avatars/e9d/71f/5ee7c92d6d/{size}.jpg"
store.avatar_template(avatar).should == "/uploads/default/avatars/e9d/71f/5ee7c92d6d/{size}.jpg"
end
end

View File

@ -113,10 +113,10 @@ describe FileStore::S3Store do
store.internal?.should == false
end
describe ".absolute_avatar_template" do
describe ".avatar_template" do
it "is present" do
store.absolute_avatar_template(avatar).should == "//s3_upload_bucket.s3.amazonaws.com/avatars/e9d71f5ee7c92d6dc9e92ffdad17b8bd49418f98/{size}.jpg"
store.avatar_template(avatar).should == "//s3_upload_bucket.s3.amazonaws.com/avatars/e9d71f5ee7c92d6dc9e92ffdad17b8bd49418f98/{size}.jpg"
end
end

View File

@ -964,7 +964,7 @@ describe User do
describe ".small_avatar_url" do
let(:user) { build(:user, use_uploaded_avatar: true, uploaded_avatar_template: "http://test.localhost/uploaded/avatar/template/{size}.png") }
let(:user) { build(:user, use_uploaded_avatar: true, uploaded_avatar_template: "/uploaded/avatar/template/{size}.png") }
it "returns a 45-pixel-wide avatar" do
user.small_avatar_url.should == "//test.localhost/uploaded/avatar/template/45.png"
@ -974,7 +974,7 @@ describe User do
describe ".uploaded_avatar_path" do
let(:user) { build(:user, use_uploaded_avatar: true, uploaded_avatar_template: "http://test.localhost/uploaded/avatar/template/{size}.png") }
let(:user) { build(:user, use_uploaded_avatar: true, uploaded_avatar_template: "/uploaded/avatar/template/{size}.png") }
it "returns nothing when uploaded avatars are not allowed" do
SiteSetting.expects(:allow_uploaded_avatars).returns(false)
@ -985,6 +985,11 @@ describe User do
user.uploaded_avatar_path.should == "//test.localhost/uploaded/avatar/template/{size}.png"
end
it "returns a schemaless cdn-based avatar template" do
Rails.configuration.action_controller.stubs(:asset_host).returns("http://my.cdn.com")
user.uploaded_avatar_path.should == "//my.cdn.com/uploaded/avatar/template/{size}.png"
end
end
describe ".avatar_template" do
@ -992,8 +997,8 @@ describe User do
let(:user) { build(:user, email: "em@il.com") }
it "returns the uploaded_avatar_path by default" do
user.expects(:uploaded_avatar_path).returns("/uploaded/avatar.png")
user.avatar_template.should == "/uploaded/avatar.png"
user.expects(:uploaded_avatar_path).returns("//discourse.org/uploaded/avatar.png")
user.avatar_template.should == "//discourse.org/uploaded/avatar.png"
end
it "returns the gravatar when no avatar has been uploaded" do