distributed memoizer added to ensure absolute duplicate posts don't get through

in case of an absolute dupe just return the memoized post

This works around issues with wordpress being crazy
This commit is contained in:
Sam 2013-07-29 12:25:19 +10:00
parent 1e107fd68a
commit 4a20d09523
4 changed files with 162 additions and 11 deletions

View File

@ -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)
params = create_params
key = params_key(params)
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?
render_json_error(post_creator)
"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)
render_json_dump(post_serializer)
"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,

View File

@ -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

View File

@ -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

View File

@ -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)}