mirror of
https://github.com/discourse/discourse.git
synced 2024-11-23 09:26:54 -06:00
Cache oneboxes in Redis now instead of postgres.
This commit is contained in:
parent
9d4ecd7ef8
commit
babcfe6234
@ -3,9 +3,7 @@ require_dependency 'oneboxer'
|
||||
class OneboxController < ApplicationController
|
||||
|
||||
def show
|
||||
Oneboxer.invalidate(params[:url]) if params[:refresh].present?
|
||||
|
||||
result = Oneboxer.preview(params[:url])
|
||||
result = Oneboxer.onebox(params[:url], invalidate_oneboxes: params[:refresh] == 'true')
|
||||
result.strip! if result.present?
|
||||
|
||||
# If there is no result, return a 404
|
||||
|
@ -1,8 +0,0 @@
|
||||
class OneboxRender < ActiveRecord::Base
|
||||
validates_presence_of :url
|
||||
validates_presence_of :cooked
|
||||
validates_presence_of :expires_at
|
||||
|
||||
has_many :post_onebox_renders, dependent: :delete_all
|
||||
has_many :posts, through: :post_onebox_renders
|
||||
end
|
@ -219,7 +219,7 @@ class Post < ActiveRecord::Base
|
||||
doc = Oneboxer.each_onebox_link(cooked) do |url, elem|
|
||||
cached = Oneboxer.render_from_cache(url)
|
||||
if cached.present?
|
||||
elem.swap(cached.cooked)
|
||||
elem.swap(cached)
|
||||
dirty = true
|
||||
end
|
||||
end
|
||||
@ -268,7 +268,7 @@ class Post < ActiveRecord::Base
|
||||
# Various callbacks
|
||||
before_create do
|
||||
if reply_to_post_number.present?
|
||||
self.reply_to_user_id ||= Post.select(:user_id).where(topic_id: topic_id, post_number: reply_to_post_number).first.try(:user_id)
|
||||
self.reply_to_user_id ||= Post.select(:user_id).where(topic_id: topic_id, post_number: reply_to_post_number).first.try(:user_id)
|
||||
end
|
||||
|
||||
self.post_number ||= Topic.next_post_number(topic_id, reply_to_post_number.present?)
|
||||
|
@ -1,6 +0,0 @@
|
||||
class PostOneboxRender < ActiveRecord::Base
|
||||
belongs_to :post
|
||||
belongs_to :onebox_render
|
||||
|
||||
validates_uniqueness_of :post_id, scope: :onebox_render_id
|
||||
end
|
9
db/migrate/20130321154905_remove_oneboxes_from_db.rb
Normal file
9
db/migrate/20130321154905_remove_oneboxes_from_db.rb
Normal file
@ -0,0 +1,9 @@
|
||||
class RemoveOneboxesFromDb < ActiveRecord::Migration
|
||||
def up
|
||||
drop_table :post_onebox_renders
|
||||
drop_table :onebox_renders
|
||||
end
|
||||
|
||||
def down
|
||||
end
|
||||
end
|
@ -1,4 +1,5 @@
|
||||
require 'open-uri'
|
||||
require 'digest/sha1'
|
||||
|
||||
require_dependency 'oneboxer/base'
|
||||
require_dependency 'oneboxer/whitelist'
|
||||
@ -14,7 +15,7 @@ module Oneboxer
|
||||
end
|
||||
|
||||
def self.default_expiry
|
||||
1.month
|
||||
1.day
|
||||
end
|
||||
|
||||
# Return a oneboxer for a given URL
|
||||
@ -79,72 +80,39 @@ module Oneboxer
|
||||
doc
|
||||
end
|
||||
|
||||
def self.create_post_reference(result, args={})
|
||||
result.post_onebox_renders.create(post_id: args[:post_id]) if args[:post_id].present?
|
||||
rescue ActiveRecord::RecordNotUnique
|
||||
def self.cache_key_for(url)
|
||||
"onebox:#{Digest::SHA1.hexdigest(url)}"
|
||||
end
|
||||
|
||||
def self.render_from_cache(url, args={})
|
||||
result = OneboxRender.where(url: url).first
|
||||
|
||||
# Return the result but also create a reference to it
|
||||
if result.present?
|
||||
create_post_reference(result, args)
|
||||
return result
|
||||
end
|
||||
nil
|
||||
def self.render_from_cache(url)
|
||||
Rails.cache.read(cache_key_for(url))
|
||||
end
|
||||
|
||||
# Cache results from a onebox call
|
||||
def self.fetch_and_cache(url, args)
|
||||
cooked, preview = onebox_nocache(url)
|
||||
return nil if cooked.blank?
|
||||
contents = onebox_nocache(url)
|
||||
return nil if contents.blank?
|
||||
|
||||
# Store a cooked version in the database
|
||||
OneboxRender.transaction do
|
||||
begin
|
||||
render = OneboxRender.create(url: url, preview: preview, cooked: cooked, expires_at: Oneboxer.default_expiry.from_now)
|
||||
create_post_reference(render, args)
|
||||
rescue ActiveRecord::RecordNotUnique
|
||||
end
|
||||
end
|
||||
|
||||
[cooked, preview]
|
||||
end
|
||||
|
||||
# Retrieve a preview of a onebox, caching the result for performance
|
||||
def self.preview(url, args={})
|
||||
cached = render_from_cache(url, args) unless args[:no_cache].present?
|
||||
|
||||
# If we have a preview stored, return that. Otherwise return cooked content.
|
||||
if cached.present?
|
||||
return cached.preview if cached.preview.present?
|
||||
return cached.cooked
|
||||
end
|
||||
cooked, preview = fetch_and_cache(url, args)
|
||||
|
||||
return preview if preview.present?
|
||||
cooked
|
||||
Rails.cache.write(cache_key_for(url), contents, expires_in: default_expiry)
|
||||
contents
|
||||
end
|
||||
|
||||
def self.invalidate(url)
|
||||
OneboxRender.destroy_all(url: url)
|
||||
Rails.cache.delete(cache_key_for(url))
|
||||
end
|
||||
|
||||
# Return the cooked content for a url, caching the result for performance
|
||||
def self.onebox(url, args={})
|
||||
|
||||
if args[:invalidate_oneboxes].present?
|
||||
if args[:invalidate_oneboxes]
|
||||
# Remove the onebox from the cache
|
||||
Oneboxer.invalidate(url)
|
||||
else
|
||||
cached = render_from_cache(url, args) unless args[:no_cache].present?
|
||||
return cached.cooked if cached.present?
|
||||
contents = render_from_cache(url)
|
||||
return contents if contents.present?
|
||||
end
|
||||
|
||||
|
||||
cooked, preview = fetch_and_cache(url, args)
|
||||
cooked
|
||||
fetch_and_cache(url, args)
|
||||
end
|
||||
|
||||
end
|
||||
|
@ -46,16 +46,68 @@ describe Oneboxer do
|
||||
end
|
||||
end
|
||||
|
||||
let(:dummy_onebox_url) { "http://dummy.localhost/dummy-object" }
|
||||
|
||||
before do
|
||||
Oneboxer.add_onebox DummyOnebox
|
||||
@dummy_onebox_url = "http://dummy.localhost/dummy-object"
|
||||
end
|
||||
|
||||
it 'should have matchers set up by default' do
|
||||
Oneboxer.matchers.should be_present
|
||||
end
|
||||
|
||||
context 'caching' do
|
||||
|
||||
let(:result) { "onebox result string" }
|
||||
|
||||
context "with invalidate_oneboxes true" do
|
||||
|
||||
it "invalidates the url" do
|
||||
Oneboxer.expects(:invalidate).with(dummy_onebox_url)
|
||||
Oneboxer.onebox(dummy_onebox_url, invalidate_oneboxes: true)
|
||||
end
|
||||
|
||||
it "doesn't render from cache" do
|
||||
Oneboxer.expects(:render_from_cache).never
|
||||
Oneboxer.onebox(dummy_onebox_url, invalidate_oneboxes: true)
|
||||
end
|
||||
|
||||
it "calls fetch and cache" do
|
||||
Oneboxer.expects(:fetch_and_cache).returns(result)
|
||||
Oneboxer.onebox(dummy_onebox_url, invalidate_oneboxes: true).should == result
|
||||
end
|
||||
|
||||
end
|
||||
|
||||
context 'with invalidate_oneboxes false' do
|
||||
|
||||
it "doesn't invalidate the url" do
|
||||
Oneboxer.expects(:invalidate).with(dummy_onebox_url).never
|
||||
Oneboxer.onebox(dummy_onebox_url, invalidate_oneboxes: false)
|
||||
end
|
||||
|
||||
it "returns render_from_cache if present" do
|
||||
Oneboxer.expects(:render_from_cache).with(dummy_onebox_url).returns(result)
|
||||
Oneboxer.onebox(dummy_onebox_url, invalidate_oneboxes: false).should == result
|
||||
end
|
||||
|
||||
it "doesn't call fetch_and_cache" do
|
||||
Oneboxer.expects(:render_from_cache).with(dummy_onebox_url).returns(result)
|
||||
Oneboxer.expects(:fetch_and_cache).never
|
||||
Oneboxer.onebox(dummy_onebox_url, invalidate_oneboxes: false)
|
||||
end
|
||||
|
||||
|
||||
it "calls fetch_and_cache if render from cache is blank" do
|
||||
Oneboxer.stubs(:render_from_cache)
|
||||
Oneboxer.expects(:fetch_and_cache).returns(result)
|
||||
Oneboxer.onebox(dummy_onebox_url, invalidate_oneboxes: false).should == result
|
||||
end
|
||||
|
||||
end
|
||||
|
||||
end
|
||||
|
||||
context 'find onebox for url' do
|
||||
|
||||
it 'returns blank with an unknown url' do
|
||||
@ -63,11 +115,11 @@ describe Oneboxer do
|
||||
end
|
||||
|
||||
it 'returns something when matched' do
|
||||
Oneboxer.onebox_for_url(@dummy_onebox_url).should be_present
|
||||
Oneboxer.onebox_for_url(dummy_onebox_url).should be_present
|
||||
end
|
||||
|
||||
it 'returns an instance of our class when matched' do
|
||||
Oneboxer.onebox_for_url(@dummy_onebox_url).kind_of?(DummyOnebox).should be_true
|
||||
Oneboxer.onebox_for_url(dummy_onebox_url).kind_of?(DummyOnebox).should be_true
|
||||
end
|
||||
|
||||
end
|
||||
@ -92,69 +144,10 @@ describe Oneboxer do
|
||||
|
||||
context 'without caching' do
|
||||
it 'calls the onebox method of our matched class' do
|
||||
Oneboxer.onebox_nocache(@dummy_onebox_url).should == 'dummy!'
|
||||
Oneboxer.onebox_nocache(dummy_onebox_url).should == 'dummy!'
|
||||
end
|
||||
end
|
||||
|
||||
context 'with caching' do
|
||||
|
||||
context 'initial cache is empty' do
|
||||
|
||||
it 'has no OneboxRender records' do
|
||||
OneboxRender.count.should == 0
|
||||
end
|
||||
|
||||
it 'calls the onebox_nocache method if there is no cache record yet' do
|
||||
Oneboxer.expects(:onebox_nocache).with(@dummy_onebox_url).once
|
||||
Oneboxer.onebox(@dummy_onebox_url)
|
||||
end
|
||||
end
|
||||
|
||||
context 'caching result' do
|
||||
before do
|
||||
@post = Fabricate(:post)
|
||||
@result = Oneboxer.onebox(@dummy_onebox_url, post_id: @post.id)
|
||||
@onebox_render = OneboxRender.where(url: @dummy_onebox_url).first
|
||||
end
|
||||
|
||||
it "returns the correct result" do
|
||||
@result.should == 'dummy!'
|
||||
end
|
||||
|
||||
it "created a OneboxRender record with the url" do
|
||||
@onebox_render.should be_present
|
||||
end
|
||||
|
||||
it "created a OneboxRender record with the url" do
|
||||
@onebox_render.url.should == @dummy_onebox_url
|
||||
end
|
||||
|
||||
it "associated the render with a post" do
|
||||
@onebox_render.posts.should == [@post]
|
||||
end
|
||||
|
||||
it "has an expires_at value" do
|
||||
@onebox_render.expires_at.should be_present
|
||||
end
|
||||
|
||||
it "doesn't call onebox_nocache on a cache hit" do
|
||||
Oneboxer.expects(:onebox_nocache).never
|
||||
Oneboxer.onebox(@dummy_onebox_url).should == 'dummy!'
|
||||
end
|
||||
|
||||
context 'invalidating cache' do
|
||||
|
||||
it "deletes the onebox render" do
|
||||
Oneboxer.expects(:onebox_nocache).once.returns('new cache value!')
|
||||
Oneboxer.onebox(@dummy_onebox_url, invalidate_oneboxes: true).should == 'new cache value!'
|
||||
end
|
||||
|
||||
end
|
||||
|
||||
end
|
||||
|
||||
end
|
||||
|
||||
context 'each_onebox_link' do
|
||||
|
||||
before do
|
||||
|
@ -5,8 +5,8 @@ describe OneboxController do
|
||||
let(:url) { "http://google.com" }
|
||||
|
||||
it 'invalidates the cache if refresh is passed' do
|
||||
Oneboxer.expects(:invalidate).with(url)
|
||||
xhr :get, :show, url: url, refresh: true
|
||||
Oneboxer.expects(:onebox).with(url, invalidate_oneboxes: true)
|
||||
xhr :get, :show, url: url, refresh: 'true'
|
||||
end
|
||||
|
||||
describe "found onebox" do
|
||||
@ -14,7 +14,7 @@ describe OneboxController do
|
||||
let(:body) { "this is the onebox body"}
|
||||
|
||||
before do
|
||||
Oneboxer.expects(:preview).with(url).returns(body)
|
||||
Oneboxer.expects(:onebox).with(url, invalidate_oneboxes: false).returns(body)
|
||||
xhr :get, :show, url: url
|
||||
end
|
||||
|
||||
@ -31,13 +31,13 @@ describe OneboxController do
|
||||
describe "missing onebox" do
|
||||
|
||||
it "returns 404 if the onebox is nil" do
|
||||
Oneboxer.expects(:preview).with(url).returns(nil)
|
||||
Oneboxer.expects(:onebox).with(url, invalidate_oneboxes: false).returns(nil)
|
||||
xhr :get, :show, url: url
|
||||
response.response_code.should == 404
|
||||
end
|
||||
|
||||
it "returns 404 if the onebox is an empty string" do
|
||||
Oneboxer.expects(:preview).with(url).returns(" \t ")
|
||||
Oneboxer.expects(:onebox).with(url, invalidate_oneboxes: false).returns(" \t ")
|
||||
xhr :get, :show, url: url
|
||||
response.response_code.should == 404
|
||||
end
|
||||
|
@ -1,11 +0,0 @@
|
||||
require 'spec_helper'
|
||||
|
||||
describe OneboxRender do
|
||||
|
||||
it { should validate_presence_of :url }
|
||||
it { should validate_presence_of :cooked }
|
||||
it { should validate_presence_of :expires_at }
|
||||
it { should have_many :post_onebox_renders }
|
||||
it { should have_many :posts }
|
||||
|
||||
end
|
@ -1,8 +0,0 @@
|
||||
require 'spec_helper'
|
||||
|
||||
describe PostOneboxRender do
|
||||
|
||||
it { should belong_to :onebox_render }
|
||||
it { should belong_to :post }
|
||||
|
||||
end
|
Loading…
Reference in New Issue
Block a user