user avatar urls/templates refactor

app/controllers/user_avatars_controller.rb

@@ -42,7 +42,7 @@ class UserAvatarsController < ApplicationController
 
     # we need multisite support to keep a single origin pull for CDNs
     RailsMultisite::ConnectionManagement.with_hostname(params[:hostname]) do
-      show_in_site(RailsMultisite::ConnectionManagement.current_hostname)
+      show_in_site(params[:hostname])
     end
   end
 
@@ -60,14 +60,16 @@ class UserAvatarsController < ApplicationController
 
     if !Discourse.avatar_sizes.include?(size) && Discourse.store.external?
       closest = Discourse.avatar_sizes.to_a.min { |a,b| (size-a).abs <=> (size-b).abs }
-      return redirect_to cdn_path("/user_avatar/#{params[:hostname]}/#{user.username_lower}/#{closest}/#{version}.png")
+      avatar_url = UserAvatar.local_avatar_url(hostname, user.username_lower, version, closest)
+      return redirect_to cdn_path(avatar_url)
     end
 
     upload = Upload.find_by(id: version) if user_avatar.contains_upload?(version)
     upload ||= user.uploaded_avatar if user.uploaded_avatar_id == version
 
     if user.uploaded_avatar && !upload
-      return redirect_to cdn_path("/user_avatar/#{hostname}/#{user.username_lower}/#{size}/#{user.uploaded_avatar_id}.png")
+      avatar_url = UserAvatar.local_avatar_url(hostname, user.username_lower, user.uploaded_avatar_id, size)
+      return redirect_to cdn_path(avatar_url)
     elsif upload
       original = Discourse.store.path_for(upload)
       if Discourse.store.external? || File.exists?(original)

app/models/user.rb

@@ -420,9 +420,9 @@ class User < ActiveRecord::Base
 
   def self.avatar_template(username,uploaded_avatar_id)
     return letter_avatar_template(username) if !uploaded_avatar_id
-    id = uploaded_avatar_id
     username ||= ""
-    "#{Discourse.base_uri}/user_avatar/#{RailsMultisite::ConnectionManagement.current_hostname}/#{username.downcase}/{size}/#{id}.png"
+    hostname = RailsMultisite::ConnectionManagement.current_hostname
+    UserAvatar.local_avatar_template(hostname, username.downcase, uploaded_avatar_id)
   end
 
   def self.letter_avatar_template(username)

app/models/user_avatar.rb

@@ -38,6 +38,30 @@ class UserAvatar < ActiveRecord::Base
     end
   end
 
+  def self.local_avatar_url(hostname, username, upload_id, size)
+    version = self.version(upload_id)
+    "#{Discourse.base_uri}/user_avatar/#{hostname}/#{username}/#{size}/#{version}.png"
+  end
+
+  def self.local_avatar_template(hostname, username, upload_id)
+    version = self.version(upload_id)
+    "#{Discourse.base_uri}/user_avatar/#{hostname}/#{username}/{size}/#{version}.png"
+  end
+
+  def self.external_avatar_url(user_id, upload_id, size)
+    version = self.version(upload_id)
+    "#{Discourse.store.absolute_base_url}/avatars/#{user_id}/#{size}/#{version}.png"
+  end
+
+  def self.external_avatar_template(user_id, upload_id)
+    version = self.version(upload_id)
+    "#{Discourse.store.absolute_base_url}/avatars/#{user_id}/{size}/#{version}.png"
+  end
+
+  def self.version(upload_id)
+    "#{upload_id}_#{OptimizedImage::VERSION}"
+  end
+
 end
 
 # == Schema Information

spec/controllers/user_avatars_controller_spec.rb

@@ -27,7 +27,7 @@ describe UserAvatarsController do
       get :show, size: 97, username: user.username, version: upload.id, hostname: 'default'
 
       # 98 is closest which is 49 * 2 for retina
-      expect(response).to redirect_to("http://awesome.com/boom/user_avatar/default/#{user.username_lower}/98/#{upload.id}.png")
+      expect(response).to redirect_to("http://awesome.com/boom/user_avatar/default/#{user.username_lower}/98/#{upload.id}_#{OptimizedImage::VERSION}.png")
 
       get :show, size: 98, username: user.username, version: upload.id, hostname: 'default'
       expect(response).to redirect_to("http://cdn.com/something/else")

spec/models/user_spec.rb

@@ -884,12 +884,12 @@ describe User do
     let(:user) { build(:user, uploaded_avatar_id: 99, username: 'Sam') }
 
     it "returns a schemaless avatar template with correct id" do
-      expect(user.avatar_template_url).to eq("//test.localhost/user_avatar/test.localhost/sam/{size}/99.png")
+      expect(user.avatar_template_url).to eq("//test.localhost/user_avatar/test.localhost/sam/{size}/99_#{OptimizedImage::VERSION}.png")
     end
 
     it "returns a schemaless cdn-based avatar template" do
       Rails.configuration.action_controller.stubs(:asset_host).returns("http://my.cdn.com")
-      expect(user.avatar_template_url).to eq("//my.cdn.com/user_avatar/test.localhost/sam/{size}/99.png")
+      expect(user.avatar_template_url).to eq("//my.cdn.com/user_avatar/test.localhost/sam/{size}/99_#{OptimizedImage::VERSION}.png")
     end
 
   end