diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb
index 45081177ba5..64e1e7c25ca 100644
--- a/app/controllers/posts_controller.rb
+++ b/app/controllers/posts_controller.rb
@@ -1,5 +1,6 @@
require_dependency 'post_creator'
require_dependency 'post_destroyer'
+require_dependency 'distributed_memoizer'
class PostsController < ApplicationController
@@ -25,21 +26,36 @@ class PostsController < ApplicationController
end
def create
- post_creator = PostCreator.new(current_user, create_params)
- post = post_creator.create
- if post_creator.errors.present?
+ params = create_params
- # If the post was spam, flag all the user's posts as spam
- current_user.flag_linked_posts_as_spam if post_creator.spam?
+ key = params_key(params)
- render_json_error(post_creator)
- else
- post_serializer = PostSerializer.new(post, scope: guardian, root: false)
- post_serializer.topic_slug = post.topic.slug if post.topic.present?
- post_serializer.draft_sequence = DraftSequence.current(current_user, post.topic.draft_key)
- render_json_dump(post_serializer)
+ result = DistributedMemoizer.memoize(key, 120) do
+ post_creator = PostCreator.new(current_user, params)
+ post = post_creator.create
+ if post_creator.errors.present?
+
+ # If the post was spam, flag all the user's posts as spam
+ current_user.flag_linked_posts_as_spam if post_creator.spam?
+
+ "e" << MultiJson.dump(errors: post_creator.errors.full_messages)
+ else
+ post_serializer = PostSerializer.new(post, scope: guardian, root: false)
+ post_serializer.topic_slug = post.topic.slug if post.topic.present?
+ post_serializer.draft_sequence = DraftSequence.current(current_user, post.topic.draft_key)
+ "s" << MultiJson.dump(post_serializer)
+ end
end
+
+ payload = result[1..-1]
+ if result[0] == "e"
+ # don't memoize errors
+ $redis.del(DistributedMemoizer.redis_key(key))
+ render json: payload, status: 422
+ else
+ render json: payload
+ end
end
def update
@@ -193,6 +209,15 @@ class PostsController < ApplicationController
private
+ def params_key(params)
+ "post##" << Digest::SHA1.hexdigest(params
+ .to_a
+ .concat([["user", current_user.id]])
+ .sort{|x,y| x[0] <=> y[0]}.join do |x,y|
+ "#{x}:#{y}"
+ end)
+ end
+
def create_params
permitted = [
:raw,
diff --git a/lib/distributed_memoizer.rb b/lib/distributed_memoizer.rb
new file mode 100644
index 00000000000..a76403fb5bb
--- /dev/null
+++ b/lib/distributed_memoizer.rb
@@ -0,0 +1,59 @@
+class DistributedMemoizer
+
+ # never wait for longer that 1 second for a cross process lock
+ MAX_WAIT = 2
+ LOCK = Mutex.new
+
+ # memoize a key across processes and machines
+ def self.memoize(key, duration = 60 * 60 * 24, redis = nil)
+ redis ||= $redis
+
+ redis_key = self.redis_key(key)
+
+ unless result = redis.get(redis_key)
+ redis_lock_key = self.redis_lock_key(key)
+
+ start = Time.new
+ got_lock = false
+ while Time.new < start + MAX_WAIT && !got_lock
+ LOCK.synchronize do
+ got_lock = get_lock(redis,redis_lock_key)
+ end
+ sleep 0.001
+ end
+
+ unless result = redis.get(redis_key)
+ result = yield
+ redis.setex(redis_key, duration, result)
+ end
+ redis.del(redis_lock_key)
+ end
+
+ result
+ end
+
+
+ def self.redis_lock_key(key)
+ "memoize_lock_" << key
+ end
+
+ def self.redis_key(key)
+ "memoize_" << key
+ end
+
+ protected
+ def self.get_lock(redis, redis_lock_key)
+ redis.watch(redis_lock_key)
+ current = redis.get(redis_lock_key)
+ return false if current
+
+ unique = SecureRandom.hex
+
+ result = redis.multi do
+ redis.setex(redis_lock_key, MAX_WAIT, unique)
+ end
+
+ redis.unwatch
+ return result == ["OK"]
+ end
+end
diff --git a/spec/components/distributed_memoizer_spec.rb b/spec/components/distributed_memoizer_spec.rb
new file mode 100644
index 00000000000..16c364feb51
--- /dev/null
+++ b/spec/components/distributed_memoizer_spec.rb
@@ -0,0 +1,55 @@
+require 'spec_helper'
+require_dependency 'distributed_memoizer'
+
+describe DistributedMemoizer do
+
+ before do
+ $redis.del(DistributedMemoizer.redis_key("hello"))
+ $redis.del(DistributedMemoizer.redis_lock_key("hello"))
+ $redis.unwatch
+ end
+
+ # NOTE we could use a mock redis here, but I think it makes sense to test the real thing
+ # let(:mock_redis) { MockRedis.new }
+
+ def memoize(&block)
+ DistributedMemoizer.memoize("hello", duration = 120, &block)
+ end
+
+ it "returns the value of a block" do
+ memoize do
+ "abc"
+ end.should == "abc"
+ end
+
+ it "return the old value once memoized" do
+
+ memoize do
+ "abc"
+ end
+
+ memoize do
+ "world"
+ end.should == "abc"
+ end
+
+ it "memoizes correctly when used concurrently" do
+ results = []
+ threads = []
+
+ 5.times do
+ threads << Thread.new do
+ results << memoize do
+ sleep 0.001
+ SecureRandom.hex
+ end
+ end
+ end
+
+ threads.each(&:join)
+ results.uniq.length.should == 1
+ results.count.should == 5
+
+ end
+
+end
diff --git a/spec/controllers/posts_controller_spec.rb b/spec/controllers/posts_controller_spec.rb
index fa476d0c5e1..94d1f0505f4 100644
--- a/spec/controllers/posts_controller_spec.rb
+++ b/spec/controllers/posts_controller_spec.rb
@@ -282,6 +282,18 @@ describe PostsController do
::JSON.parse(response.body).should be_present
end
+ it 'protects against dupes' do
+ # TODO we really should be using a mock redis here
+ xhr :post, :create, {raw: 'this is a test post 123', title: 'this is a test title 123', wpid: 1}
+ response.should be_success
+ original = response.body
+
+ xhr :post, :create, {raw: 'this is a test post 123', title: 'this is a test title 123', wpid: 2}
+ response.should be_success
+
+ response.body.should == original
+ end
+
context "errors" do
let(:post_with_errors) { Fabricate.build(:post, user: user)}