From 1e6f886886ce469b114c737c9322bf7bc4acd50c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Mon, 9 Feb 2015 18:54:57 +0100
Subject: [PATCH] FIX: use distributed mutex to prevent errors when uploading
 emojis in batches

---
 app/controllers/admin/emojis_controller.rb |  5 +--
 app/models/emoji.rb                        | 43 ++++++++++++++++------
 2 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/app/controllers/admin/emojis_controller.rb b/app/controllers/admin/emojis_controller.rb
index ea230f1224d..7af02c211d1 100644
--- a/app/controllers/admin/emojis_controller.rb
+++ b/app/controllers/admin/emojis_controller.rb
@@ -13,8 +13,7 @@ class Admin::EmojisController < Admin::AdminController
                .gsub(/_{2,}/, '_')
                .downcase
 
-    # check the name doesn't already exist
-    if Emoji.custom.detect { |e| e.name == name }
+    if Emoji.exists?(name)
       render json: failed_json.merge(message: I18n.t("emoji.errors.name_already_exists", name: name)), status: 422
     else
       if emoji = Emoji.create_for(file, name)
@@ -28,7 +27,7 @@ class Admin::EmojisController < Admin::AdminController
 
   def destroy
     name = params.require(:id)
-    Emoji.custom.detect { |e| e.name == name }.try(:remove)
+    Emoji[name].try(:remove)
     render nothing: true
   end
 
diff --git a/app/models/emoji.rb b/app/models/emoji.rb
index b283e36a40f..cb79f0f5c37 100644
--- a/app/models/emoji.rb
+++ b/app/models/emoji.rb
@@ -1,6 +1,8 @@
 class Emoji
   include ActiveModel::SerializerSupport
 
+  EMOJIS_CUSTOM_LOCK ||= "_emojis_custom_lock_".freeze
+
   attr_reader :path
   attr_accessor :name, :url
 
@@ -13,9 +15,12 @@ class Emoji
 
   def remove
     return if path.blank?
-    if File.exists?(path)
-      File.delete(path) rescue nil
-      Emoji.clear_cache
+
+    DistributedMutex.new(EMOJIS_CUSTOM_LOCK).synchronize do
+      if File.exists?(path)
+        File.delete(path) rescue nil
+        Emoji.clear_cache
+      end
     end
   end
 
@@ -31,6 +36,14 @@ class Emoji
     Discourse.cache.fetch("custom", family: "emoji") { load_custom }
   end
 
+  def self.exists?(name)
+    Emoji[name].present?
+  end
+
+  def self.[](name)
+    Emoji.custom.detect { |e| e.name == name }
+  end
+
   def self.create_from_path(path)
     extension = File.extname(path)
     Emoji.new(path).tap do |e|
@@ -51,15 +64,19 @@ class Emoji
   def self.create_for(file, name)
     extension = File.extname(file.original_filename)
     path = "#{Emoji.base_directory}/#{name}#{extension}"
-    # 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
+
+    DistributedMutex.new(EMOJIS_CUSTOM_LOCK).synchronize do
+      # 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
+    end
+
     # launch resize job
     Jobs.enqueue(:resize_emoji, path: path)
     # return created emoji
-    Emoji.custom.detect { |e| e.name == name }
+    Emoji[name]
   end
 
   def self.clear_cache
@@ -76,9 +93,11 @@ class Emoji
   end
 
   def self.load_custom
-    Dir.glob(File.join(Emoji.base_directory, "*.{png,gif}"))
-       .sort
-       .map { |emoji| Emoji.create_from_path(emoji) }
+    DistributedMutex.new(EMOJIS_CUSTOM_LOCK).synchronize do
+      Dir.glob(File.join(Emoji.base_directory, "*.{png,gif}"))
+         .sort
+         .map { |emoji| Emoji.create_from_path(emoji) }
+    end
   end
 
   def self.base_directory