discourse/spec/models/user_avatar_spec.rb
Sam c2332d7505
FEATURE: reduce avatar sizes to 6 from 20 (#21319)
* FEATURE: reduce avatar sizes to 6 from 20

This PR introduces 3 changes:

1. SiteSetting.avatar_sizes, now does what is says on the tin.
previously it would introduce a large number of extra sizes, to allow for
various DPIs. Instead we now trust the admin with the size list.

2. When `avatar_sizes` changes, we ensure consistency and remove resized
avatars that are not longer allowed per site setting. This happens on the
12 hourly job and limited out of the box to 20k cleanups per cycle, given
this may reach out to AWS 20k times to remove things.

3.Our default avatar sizes are now "24|48|72|96|144|288" these sizes were
very specifically picked to limit amount of bluriness introduced by webkit.
Our avatars are already blurry due to 1px border, so this corrects old blur.

This change heavily reduces storage required by forums which simplifies
site moves and more.

Co-authored-by: David Taylor <david@taylorhq.com>
2023-06-01 10:00:01 +10:00

239 lines
7.3 KiB
Ruby

# frozen_string_literal: true
RSpec.describe UserAvatar do
fab!(:user) { Fabricate(:user) }
let(:avatar) { user.create_user_avatar! }
describe "#update_gravatar!" do
let(:temp) { Tempfile.new("test") }
fab!(:upload) { Fabricate(:upload, user: user) }
describe "when working" do
before do
temp.binmode
# tiny valid png
temp.write(
Base64.decode64(
"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAACklEQVR4nGMAAQAABQABDQottAAAAABJRU5ErkJggg==",
),
)
temp.rewind
FileHelper.expects(:download).returns(temp)
end
after { temp.unlink }
it "can update gravatars" do
freeze_time Time.now
expect { avatar.update_gravatar! }.to change { Upload.count }.by(1)
expect(avatar.gravatar_upload).to eq(Upload.last)
expect(avatar.last_gravatar_download_attempt).to eq(Time.now)
expect(user.reload.uploaded_avatar).to eq(nil)
expect do avatar.destroy end.to_not change { Upload.count }
end
it "updates gravatars even if uploads have been disabled" do
SiteSetting.authorized_extensions = ""
expect { avatar.update_gravatar! }.to change { Upload.count }.by(1)
expect(avatar.gravatar_upload).to eq(Upload.last)
end
describe "when user has an existing custom upload" do
it "should not change the user's uploaded avatar" do
user.update!(uploaded_avatar: upload)
avatar.update!(custom_upload: upload, gravatar_upload: Fabricate(:upload, user: user))
avatar.update_gravatar!
expect(upload.reload).to eq(upload)
expect(user.reload.uploaded_avatar).to eq(upload)
expect(avatar.reload.custom_upload).to eq(upload)
expect(avatar.gravatar_upload).to eq(Upload.last)
end
end
describe "when user has an existing gravatar" do
it "should update the user's uploaded avatar correctly" do
user.update!(uploaded_avatar: upload)
avatar.update!(gravatar_upload: upload)
avatar.update_gravatar!
# old upload to be cleaned up via clean_up_uploads
expect(Upload.find_by(id: upload.id)).not_to eq(nil)
new_upload = Upload.last
expect(user.reload.uploaded_avatar).to eq(new_upload)
expect(avatar.reload.gravatar_upload).to eq(new_upload)
end
end
end
describe "when failing" do
it "always update 'last_gravatar_download_attempt'" do
freeze_time Time.now
FileHelper.expects(:download).raises(SocketError)
expect do expect { avatar.update_gravatar! }.to raise_error(SocketError) end.to_not change {
Upload.count
}
expect(avatar.last_gravatar_download_attempt).to eq(Time.now)
end
end
describe "404 should be silent, nothing to do really" do
it "does nothing when avatar is 404" do
SecureRandom.stubs(:urlsafe_base64).returns("5555")
freeze_time Time.now
stub_request(
:get,
"https://www.gravatar.com/avatar/#{avatar.user.email_hash}.png?d=404&reset_cache=5555&s=#{Discourse.avatar_sizes.max}",
).to_return(status: 404, body: "", headers: {})
expect do avatar.update_gravatar! end.to_not change { Upload.count }
expect(avatar.last_gravatar_download_attempt).to eq(Time.now)
end
end
it "should not raise an error when there's no primary_email" do
avatar.user.primary_email.destroy
avatar.user.reload
# If raises an error, test fails
avatar.update_gravatar!
end
end
describe ".import_url_for_user" do
it "creates user_avatar record if missing" do
user = Fabricate(:user)
user.user_avatar.destroy
user.reload
FileHelper.stubs(:download).returns(file_from_fixtures("logo.png"))
UserAvatar.import_url_for_user("logo.png", user)
user.reload
expect(user.uploaded_avatar_id).not_to eq(nil)
expect(user.user_avatar.custom_upload_id).to eq(user.uploaded_avatar_id)
end
it "can leave gravatar alone" do
upload = Fabricate(:upload)
user = Fabricate(:user, uploaded_avatar_id: upload.id)
user.user_avatar.update_columns(gravatar_upload_id: upload.id)
stub_request(:get, "http://thisfakesomething.something.com/").to_return(
status: 200,
body: file_from_fixtures("logo.png"),
headers: {
},
)
url = "http://thisfakesomething.something.com/"
expect do UserAvatar.import_url_for_user(url, user, override_gravatar: false) end.to change {
Upload.count
}.by(1)
user.reload
expect(user.uploaded_avatar_id).to eq(upload.id)
last_id = Upload.last.id
expect(last_id).not_to eq(upload.id)
expect(user.user_avatar.custom_upload_id).to eq(last_id)
end
describe "when avatar url returns an invalid status code" do
it "should not do anything" do
stub_request(:get, "http://thisfakesomething.something.com/").to_return(
status: 500,
body: "",
headers: {
},
)
url = "http://thisfakesomething.something.com/"
expect do UserAvatar.import_url_for_user(url, user) end.to_not change { Upload.count }
user.reload
expect(user.uploaded_avatar_id).to eq(nil)
expect(user.user_avatar.custom_upload_id).to eq(nil)
end
end
it "doesn't error out if the remote request fails" do
FileHelper.stubs(:download).raises(FinalDestination::SSRFDetector::LookupFailedError.new)
expect { UserAvatar.import_url_for_user(anything, user) }.not_to raise_error
end
end
describe "ensure_consistency!" do
it "cleans up incorrectly sized avatars" do
SiteSetting.avatar_sizes = "10|20|30"
upload = Fabricate(:upload)
user_avatar = Fabricate(:user).user_avatar
user_avatar.update_columns(custom_upload_id: upload.id)
Fabricate(:optimized_image, upload: upload, width: 10, height: 10)
Fabricate(:optimized_image, upload: upload, width: 15, height: 15)
Fabricate(:optimized_image, upload: upload, width: 20, height: 20)
UserAvatar.ensure_consistency!
expect(OptimizedImage.where(upload_id: upload.id).pluck(:width, :height).sort).to eq(
[[10, 10], [20, 20]],
)
# will not clean up if referenced
Fabricate(:optimized_image, upload: upload, width: 15, height: 15)
UploadReference.create!(upload: upload, target: Fabricate(:post))
UserAvatar.ensure_consistency!
expect(OptimizedImage.where(upload_id: upload.id).pluck(:width, :height).sort).to eq(
[[10, 10], [15, 15], [20, 20]],
)
end
it "will clean up dangling avatars" do
upload1 = Fabricate(:upload)
upload2 = Fabricate(:upload)
user_avatar = Fabricate(:user).user_avatar
user_avatar.update_columns(gravatar_upload_id: upload1.id, custom_upload_id: upload2.id)
upload1.destroy!
upload2.destroy!
user_avatar.reload
expect(user_avatar.gravatar_upload_id).to eq(nil)
expect(user_avatar.custom_upload_id).to eq(nil)
user_avatar.update_columns(gravatar_upload_id: upload1.id, custom_upload_id: upload2.id)
UserAvatar.ensure_consistency!
user_avatar.reload
expect(user_avatar.gravatar_upload_id).to eq(nil)
expect(user_avatar.custom_upload_id).to eq(nil)
end
end
end