Merge pull request #4682 from tgxworld/store_custom_emoji_as_uploads

FIX: Store custom emojis as uploads.

app/controllers/admin/emojis_controller.rb

@@ -14,25 +14,50 @@ class Admin::EmojisController < Admin::AdminController
                  .gsub(/_{2,}/, '_')
                  .downcase
 
-      data = if Emoji.exists?(name)
-        failed_json.merge(errors: [I18n.t("emoji.errors.name_already_exists", name: name)])
-      elsif emoji = Emoji.create_for(file, name)
-        emoji
-      else
-        failed_json.merge(errors: [I18n.t("emoji.errors.error_while_storing_emoji")])
-      end
+      upload = Upload.create_for(
+        current_user.id,
+        file.tempfile,
+        file.original_filename,
+        File.size(file.tempfile.path),
+        image_type: 'custom_emoji'
+      )
+
+      data =
+        if upload.persisted?
+          custom_emoji = CustomEmoji.new(name: name, upload: upload)
+
+          if custom_emoji.save
+            Emoji.clear_cache
+            { name: custom_emoji.name, url: custom_emoji.upload.url }
+          else
+            failed_json.merge(errors: custom_emoji.errors.full_messages)
+          end
+        else
+          failed_json.merge(errors: upload.errors.full_messages)
+        end
 
       MessageBus.publish("/uploads/emoji", data.as_json, user_ids: [current_user.id])
     end
 
-
     render json: success_json
   end
 
   def destroy
     name = params.require(:id)
-    Emoji[name].try(:remove)
-    render nothing: true
+
+    custom_emoji = CustomEmoji.find_by(name: name)
+    raise Discourse::InvalidParameters unless custom_emoji
+
+    CustomEmoji.transaction do
+      custom_emoji.upload.destroy!
+      custom_emoji.destroy!
+    end
+
+    Emoji.clear_cache
+
+    Jobs.enqueue(:rebake_custom_emoji_posts, name: name)
+
+    render json: success_json
   end
 
 end

app/jobs/onceoff/migrate_custom_emojis.rb

@@ -0,0 +1,35 @@
+module Jobs
+  class MigrateCustomEmojis < Jobs::Onceoff
+    def execute_onceoff(args)
+      return if Rails.env.test?
+
+      CustomEmoji.transaction do
+        Dir["#{Rails.root}/#{Emoji.base_directory}/*.{png,gif}"].each do |path|
+          name = File.basename(path, File.extname(path))
+
+          File.open(path) do |file|
+            upload = Upload.create_for(
+              Discourse.system_user.id,
+              file,
+              File.basename(path),
+              file.size,
+              image_type: 'custom_emoji'
+            )
+
+            if upload.persisted?
+              CustomEmoji.create!(name: name, upload: upload)
+            else
+              raise "Failed to create upload for '#{name}' custom emoji"
+            end
+          end
+        end
+
+        Emoji.clear_cache
+
+        Post.where("cooked LIKE '%#{Emoji.base_url}%'").find_each do |post|
+          post.rebake!
+        end
+      end
+    end
+  end
+end

app/jobs/regular/rebake_custom_emoji_posts.rb

@@ -0,0 +1,8 @@
+module Jobs
+  class RebakeCustomEmojiPosts < Jobs::Base
+    def execute(args)
+      name = args[:name]
+      Post.where("raw LIKE '%:#{name}:%'").find_each { |post| post.rebake! }
+    end
+  end
+end

app/jobs/regular/resize_emoji.rb

@@ -1,18 +0,0 @@
-module Jobs
-
-  class ResizeEmoji < Jobs::Base
-
-    def execute(args)
-      path = args[:path]
-      return unless File.exists?(path)
-
-      opts = {
-        allow_animation: true,
-        force_aspect_ratio: SiteSetting.enforce_square_emoji
-      }
-      # make sure emoji aren't too big
-      OptimizedImage.downsize(path, path, "100x100", opts)
-    end
-  end
-
-end

app/jobs/scheduled/clean_up_uploads.rb

@@ -22,11 +22,13 @@ module Jobs
         .joins("LEFT JOIN user_avatars ua ON (ua.gravatar_upload_id = uploads.id OR ua.custom_upload_id = uploads.id)")
         .joins("LEFT JOIN user_profiles up ON up.profile_background = uploads.url OR up.card_background = uploads.url")
         .joins("LEFT JOIN categories c ON c.uploaded_logo_id = uploads.id OR c.uploaded_background_id = uploads.id")
+        .joins("LEFT JOIN custom_emojis ce ON ce.upload_id = uploads.id")
         .where("pu.upload_id IS NULL")
         .where("u.uploaded_avatar_id IS NULL")
         .where("ua.gravatar_upload_id IS NULL AND ua.custom_upload_id IS NULL")
         .where("up.profile_background IS NULL AND up.card_background IS NULL")
         .where("c.uploaded_logo_id IS NULL AND c.uploaded_background_id IS NULL")
+        .where("ce.upload_id IS NULL")
         .where("uploads.url NOT IN (?)", ignore_urls)
 
       result.find_each do |upload|

app/models/custom_emoji.rb

@@ -0,0 +1,6 @@
+class CustomEmoji < ActiveRecord::Base
+  belongs_to :upload
+
+  validates :name, presence: true, uniqueness: true
+  validates :upload_id, presence: true
+end

app/models/emoji.rb

@@ -14,14 +14,6 @@ class Emoji
     @path = path
   end
 
-  def remove
-    return if path.blank?
-    if File.exists?(path)
-      File.delete(path) rescue nil
-      Emoji.clear_cache
-    end
-  end
-
   def self.all
     Discourse.cache.fetch(cache_key("all_emojis")) { standard | custom }
   end
@@ -46,14 +38,6 @@ class Emoji
     Emoji.custom.detect { |e| e.name == name }
   end
 
-  def self.create_from_path(path)
-    extension = File.extname(path)
-    Emoji.new(path).tap do |e|
-      e.name = File.basename(path, ".*")
-      e.url = "#{base_url}/#{e.name}#{extension}"
-    end
-  end
-
   def self.create_from_db_item(emoji)
     name = emoji["name"]
     filename = "#{emoji['filename'] || name}.png"
@@ -63,22 +47,6 @@ class Emoji
     end
   end
 
-  def self.create_for(file, name)
-    extension = File.extname(file.original_filename)
-    path = "#{Emoji.base_directory}/#{name}#{extension}"
-    full_path = "#{Rails.root}/#{path}"
-
-    # store the emoji
-    FileUtils.mkdir_p(Pathname.new(path).dirname)
-    File.open(path, "wb") { |f| f << file.tempfile.read }
-    # clear the cache
-    Emoji.clear_cache
-    # launch resize job
-    Jobs.enqueue(:resize_emoji, path: full_path)
-    # return created emoji
-    Emoji[name]
-  end
-
   def self.cache_key(name)
     "#{name}:#{EMOJI_VERSION}:#{Plugin::CustomEmoji.cache_key}"
   end
@@ -124,9 +92,12 @@ class Emoji
   def self.load_custom
     result = []
 
-    Dir.glob(File.join(Emoji.base_directory, "*.{png,gif}"))
-       .sort
-       .each { |emoji| result << Emoji.create_from_path(emoji) }
+    CustomEmoji.all.each do |emoji|
+      result << Emoji.new.tap do |e|
+        e.name = emoji.name
+        e.url = emoji.upload.url
+      end
+    end
 
     Plugin::CustomEmoji.emojis.each do |name, url|
       result << Emoji.new.tap do |e|

app/models/upload.rb

@@ -57,7 +57,12 @@ class Upload < ActiveRecord::Base
   end
 
   # list of image types that will be cropped
-  CROPPED_IMAGE_TYPES ||= %w{avatar profile_background card_background}
+  CROPPED_IMAGE_TYPES ||= %w{
+    avatar
+    profile_background
+    card_background
+    custom_emoji
+  }
 
   WHITELISTED_SVG_ELEMENTS ||= %w{
     circle
@@ -92,7 +97,7 @@ class Upload < ActiveRecord::Base
   # options
   #   - content_type
   #   - origin (url)
-  #   - image_type ("avatar", "profile_background", "card_background")
+  #   - image_type ("avatar", "profile_background", "card_background", "custom_emoji")
   #   - is_attachment_for_group_message (boolean)
   def self.create_for(user_id, file, filename, filesize, options = {})
     upload = Upload.new
@@ -145,6 +150,8 @@ class Upload < ActiveRecord::Base
             max_width = 590 * max_pixel_ratio
             width, height = ImageSizer.resize(w, h, max_width: max_width, max_height: max_width)
             OptimizedImage.downsize(file.path, file.path, "#{width}x#{height}", filename: filename, allow_animation: allow_animation)
+          when "custom_emoji"
+            OptimizedImage.downsize(file.path, file.path, "100x100", filename: filename, allow_animation: allow_animation)
           end
         end
 

config/initializers/100-wrap_parameters.rb

@@ -12,4 +12,3 @@ end
 ActiveSupport.on_load(:active_record) do
   self.include_root_in_json = false
 end
-

config/locales/server.en.yml

@@ -391,6 +391,10 @@ en:
           attributes:
             payload_url:
               invalid: "URL is invalid. URL should includes http:// or https://. And no blank is allowed."
+        custom_emoji:
+          attributes:
+            name:
+              taken: is already in use by another emoji
 
   user_profile:
     no_info_me: "<div class='missing-profile'>the About Me field of your profile is currently blank, <a href='/users/%{username_lower}/preferences/about-me'>would you like to fill it out?</a></div>"
@@ -1574,11 +1578,6 @@ en:
     post_revision_text: "Ownership transferred from %{old_user} to %{new_user}"
     deleted_user: "a deleted user"
 
-  emoji:
-    errors:
-      name_already_exists: "Sorry, the name '%{name}' is already used by another emoji."
-      error_while_storing_emoji: "Sorry, there has been an error while storing the emoji."
-
   topic_statuses:
     archived_enabled: "This topic is now archived. It is frozen and cannot be changed in any way."
     archived_disabled: "This topic is now unarchived. It is no longer frozen, and can be changed."

db/migrate/20170201085745_create_custom_emojis.rb

@@ -0,0 +1,12 @@
+class CreateCustomEmojis < ActiveRecord::Migration
+  def change
+    create_table :custom_emojis do |t|
+      t.string :name, null: false
+      t.integer :upload_id, null: false
+
+      t.timestamps null: false
+    end
+
+    add_index :custom_emojis, :name, unique: true
+  end
+end

lib/tasks/db.rake

@@ -6,6 +6,7 @@ end
 # we need to run seed_fu every time we run rake db:migrate
 task 'db:migrate' => ['environment', 'set_locale'] do
   SeedFu.seed
+  Jobs::Onceoff.enqueue_all
 end
 
 task 'test:prepare' => 'environment' do

spec/components/pretty_text_spec.rb

@@ -431,7 +431,7 @@ HTML
 
   describe "custom emoji" do
     it "replaces the custom emoji" do
-      Emoji.stubs(:custom).returns([ Emoji.create_from_path('trout') ])
+      CustomEmoji.create!(name: 'trout', upload: Fabricate(:upload))
       expect(PrettyText.cook("hello :trout:")).to match(/<img src[^>]+trout[^>]+>/)
     end
   end

spec/controllers/admin/emojis_controller_spec.rb

@@ -16,10 +16,6 @@ describe Admin::EmojisController do
     end
   end
 
-  it "is a subclass of AdminController" do
-    expect(Admin::EmojisController < Admin::AdminController).to eq(true)
-  end
-
   context "when logged in" do
     let!(:user) { log_in(:admin) }
 
@@ -33,56 +29,6 @@ describe Admin::EmojisController do
         expect(json[0]["url"]).to eq(custom_emoji.url)
       end
     end
-
-    context ".create" do
-
-      before { Emoji.expects(:custom).returns([custom_emoji]) }
-
-      context "name already exist" do
-        it "throws an error" do
-          message = MessageBus.track_publish do
-            xhr :post, :create, { name: "hello", file: "" }
-          end.first
-
-          expect(response).to be_success
-          expect(message.data["errors"]).to be
-        end
-      end
-
-      context "error while saving emoji" do
-        it "throws an error" do
-          Emoji.expects(:create_for).returns(nil)
-          message = MessageBus.track_publish do
-            xhr :post, :create, { name: "garbage", file: "" }
-          end.first
-
-          expect(response).to be_success
-          expect(message.data["errors"]).to be
-        end
-      end
-
-      it "works" do
-        Emoji.expects(:create_for).returns(custom_emoji2)
-
-        message = MessageBus.track_publish do
-          xhr :post, :create, { name: "hello2", file: ""}
-        end.first
-
-        expect(response).to be_success
-
-        expect(message.data["name"]).to eq(custom_emoji2.name)
-        expect(message.data["url"]).to eq(custom_emoji2.url)
-      end
-    end
-
-    context ".destroy" do
-      it "deletes the custom emoji" do
-        custom_emoji.expects(:remove)
-        Emoji.expects(:custom).returns([custom_emoji])
-        xhr :delete, :destroy, id: "hello"
-        expect(response).to be_success
-      end
-    end
   end
 
 end

spec/integration/admin/emojis_spec.rb

@@ -0,0 +1,76 @@
+require 'rails_helper'
+
+RSpec.describe "Managing custom emojis" do
+  let(:admin) { Fabricate(:admin) }
+  let(:upload) { Fabricate(:upload) }
+
+  before do
+    sign_in(admin)
+  end
+
+  describe "creating a custom emoji" do
+    describe 'when upload is invalid' do
+      it 'should publish the right error' do
+        message = MessageBus.track_publish do
+          post("/admin/customize/emojis.json", {
+            name: 'test',
+            file: fixture_file_upload("#{Rails.root}/spec/fixtures/images/fake.jpg")
+          })
+        end.first
+
+        expect(message.channel).to eq("/uploads/emoji")
+        expect(message.data["errors"]).to eq([I18n.t('upload.images.size_not_found')])
+      end
+    end
+
+    describe 'when emoji name already exists' do
+      it 'should publish the right error' do
+        CustomEmoji.create!(name: 'test', upload: upload)
+
+        message = MessageBus.track_publish do
+          post("/admin/customize/emojis.json", {
+            name: 'test',
+            file: fixture_file_upload("#{Rails.root}/spec/fixtures/images/logo.png")
+          })
+        end.first
+
+        expect(message.channel).to eq("/uploads/emoji")
+
+        expect(message.data["errors"]).to eq([
+          "Name #{I18n.t('activerecord.errors.models.custom_emoji.attributes.name.taken')}"
+        ])
+      end
+    end
+
+    it 'should allow an admin to add a custom emoji' do
+        Emoji.expects(:clear_cache)
+
+        message = MessageBus.track_publish do
+          post("/admin/customize/emojis.json", {
+            name: 'test',
+            file: fixture_file_upload("#{Rails.root}/spec/fixtures/images/logo.png")
+          })
+        end.first
+
+        custom_emoji = CustomEmoji.last
+        upload = custom_emoji.upload
+
+        expect(upload.original_filename).to eq('logo.png')
+        expect(message.channel).to eq("/uploads/emoji")
+        expect(message.data["errors"]).to eq(nil)
+        expect(message.data["name"]).to eq(custom_emoji.name)
+        expect(message.data["url"]).to eq(upload.url)
+    end
+  end
+
+  describe 'deleting a custom emoji' do
+    it 'should allow an admin to delete a custom emoji' do
+      custom_emoji = CustomEmoji.create!(name: 'test', upload: upload)
+      Emoji.clear_cache
+
+      expect { delete "/admin/customize/emojis/#{custom_emoji.name}.json", name: 'test' }
+        .to change { Upload.count }.by(-1)
+        .and change { CustomEmoji.count }.by(-1)
+    end
+  end
+end

spec/jobs/clean_up_uploads_spec.rb

@@ -140,4 +140,14 @@ describe Jobs::CleanUpUploads do
     expect(Upload.find_by(id: upload.id)).to eq(upload)
   end
 
+  it "does not delete custom emojis" do
+    upload = fabricate_upload
+    CustomEmoji.create!(name: 'test', upload: upload)
+
+    Jobs::CleanUpUploads.new.execute(nil)
+
+    expect(Upload.find_by(id: @upload.id)).to eq(nil)
+    expect(Upload.find_by(id: upload.id)).to eq(upload)
+  end
+
 end

spec/jobs/rebake_custom_emoji_posts_spec.rb

@@ -0,0 +1,19 @@
+require 'rails_helper'
+
+RSpec.describe Jobs::RebakeCustomEmojiPosts do
+  it 'should rebake posts that are using a given custom emoji' do
+    custom_emoji = CustomEmoji.create!(name: 'test', upload: Fabricate(:upload))
+    Emoji.clear_cache
+    post = Fabricate(:post, raw: 'some post with :test: yay')
+
+    expect(post.reload.cooked).to eq(
+      "<p>some post with <img src=\"/uploads/default/0/1234567890123456.png?v=3\" title=\":test:\" class=\"emoji emoji-custom\" alt=\":test:\"> yay</p>"
+    )
+
+    custom_emoji.destroy!
+    Emoji.clear_cache
+    described_class.new.execute(name: 'test')
+
+    expect(post.reload.cooked).to eq('<p>some post with :test: yay</p>')
+  end
+end