speed up startup (avoid loading some gems on startup)

correct group permission leaks
add Discourse.cache for richer caching support
This commit is contained in:
Sam 2013-05-13 18:04:03 +10:00
parent 9b33e826f2
commit b6bf95e741
21 changed files with 250 additions and 66 deletions

29
Gemfile
View File

@ -63,7 +63,7 @@ gem 'sinatra', require: nil
gem 'slim' # required for sidekiq-web
gem 'therubyracer', require: 'v8'
gem 'thin'
gem 'diffy'
gem 'diffy', require: false
# Gem that enables support for plugins. It is required.
gem 'discourse_plugin', path: 'vendor/gems/discourse_plugin'
@ -86,26 +86,27 @@ group :assets do
end
group :test do
gem 'fakeweb', '~> 1.3.0'
gem 'minitest'
gem 'fakeweb', '~> 1.3.0', require: false
gem 'minitest', require: false
end
group :test, :development do
gem 'jshint_on_rails'
gem 'guard-jshint-on-rails'
gem 'certified'
gem 'fabrication'
gem 'guard-jasmine'
gem 'guard-rspec'
gem 'guard-spork'
gem 'listen', require: false
gem 'guard-jshint-on-rails', require: false
gem 'certified', require: false
gem 'fabrication', require: false
gem 'guard-jasmine', require: false
gem 'guard-rspec', require: false
gem 'guard-spork', require: false
gem 'jasminerice'
gem 'mocha', require: false
gem 'rb-fsevent'
gem 'rb-inotify', '~> 0.9', require: RUBY_PLATFORM.include?('linux') && 'rb-inotify'
gem 'rspec-rails'
gem 'shoulda'
gem 'rb-fsevent', require: false
gem 'rb-inotify', '~> 0.9', require: false
gem 'rspec-rails', require: false
gem 'shoulda', require: false
gem 'simplecov', require: false
gem 'terminal-notifier-guard', require: RUBY_PLATFORM.include?('darwin') && 'terminal-notifier-guard'
gem 'terminal-notifier-guard', require: false
end
group :development do

View File

@ -482,6 +482,7 @@ DEPENDENCIES
jquery-rails
jshint_on_rails
librarian (>= 0.0.25)
listen
lru_redux
message_bus!
minitest

View File

@ -1,3 +1,6 @@
require 'rb-inotify' if RUBY_PLATFORM.include?('linux')
require 'terminal-notifier-guard' if RUBY_PLATFORM.include?('darwin')
phantom_path = File.expand_path('~/phantomjs/bin/phantomjs')
phantom_path = nil unless File.exists?(phantom_path)
@ -27,6 +30,7 @@ guard 'jshint-on-rails', config_path: 'config/jshint.yml' do
end
unless ENV["USING_AUTOSPEC"]
puts "Sam strongly recommends you Run: `bundle exec rake autospec` in favor of guard for specs, set USING_AUTOSPEC in .rvmrc to disable from Guard"
guard :spork, wait: 120 do
watch('config/application.rb')

View File

@ -99,7 +99,7 @@ class ApplicationController < ActionController::Base
guardian.current_user.sync_notification_channel_position
end
store_preloaded("site", Site.cached_json)
store_preloaded("site", Site.cached_json(current_user))
if current_user.present?
store_preloaded("currentUser", MultiJson.dump(CurrentUserSerializer.new(current_user, root: false)))

View File

@ -3,7 +3,7 @@ require_dependency 'search'
class SearchController < ApplicationController
def query
search_result = Search.query(params[:term], current_user, params[:type_filter], SiteSetting.min_search_term_length)
search_result = Search.query(params[:term], guardian, params[:type_filter], SiteSetting.min_search_term_length)
render_json_dump(search_result.as_json)
end

View File

@ -29,6 +29,16 @@ class Category < ActiveRecord::Base
scope :latest, ->{ order('topic_count desc') }
scope :secured, lambda { |guardian|
ids = nil
ids = guardian.secure_category_ids if guardian
if ids.present?
where("categories.secure ='f' or categories.id = :cats ", cats: ids)
else
where("categories.secure ='f'")
end
}
delegate :post_template, to: 'self.class'
# Internal: Update category stats: # of topics in past year, month, week for

View File

@ -3,19 +3,14 @@ class CategoryList
attr_accessor :categories, :topic_users, :uncategorized
def initialize(current_user)
def initialize(guardian)
@categories = Category
.includes(featured_topics: [:category])
.includes(:featured_users)
.where('topics.visible' => true)
.secured(guardian)
.order('categories.topics_week desc, categories.topics_month desc, categories.topics_year desc')
allowed_ids = current_user ? current_user.secure_category_ids : nil
if allowed_ids.present?
@categories = @categories.where("categories.secure = 'f' OR categories.id in (?)", allowed_ids)
else
@categories = @categories.where("categories.secure = 'f'")
end
@categories = @categories.to_a
@ -56,7 +51,7 @@ class CategoryList
@categories.insert(insert_at || @categories.size, uncategorized)
end
unless Guardian.new(current_user).can_create?(Category)
unless guardian.can_create?(Category)
# Remove categories with no featured topics unless we have the ability to edit one
@categories.delete_if { |c| c.featured_topics.blank? }
else
@ -69,7 +64,7 @@ class CategoryList
end
# Get forum topic user records if appropriate
if current_user.present?
if guardian.current_user
topics = []
@categories.each { |c| topics << c.featured_topics }
topics << @uncategorized
@ -77,7 +72,7 @@ class CategoryList
topics.flatten! if topics.present?
topics.compact! if topics.present?
topic_lookup = TopicUser.lookup_for(current_user, topics)
topic_lookup = TopicUser.lookup_for(guardian.current_user, topics)
# Attach some data for serialization to each topic
topics.each { |ft| ft.user_data = topic_lookup[ft.id] }

View File

@ -5,6 +5,10 @@ require_dependency 'trust_level'
class Site
include ActiveModel::Serialization
def initialize(guardian)
@guardian = guardian
end
def site_setting
SiteSetting
end
@ -22,27 +26,32 @@ class Site
end
def categories
Category.latest.includes(:topic_only_relative_url)
Category
.secured(@guardian)
.latest
.includes(:topic_only_relative_url)
end
def archetypes
Archetype.list.reject { |t| t.id == Archetype.private_message }
end
def self.cache_key
"site_json"
def cache_key
k ="site_json_cats_"
k << @guardian.secure_category_ids.join("_") if @guardian
end
def self.cached_json
def self.cached_json(guardian)
# Sam: bumping this way down, SiteSerializer will serialize post actions as well,
# On my local this was not being flushed as post actions types changed, it turn this
# broke local.
Rails.cache.fetch(Site.cache_key, expires_in: 1.minute) do
MultiJson.dump(SiteSerializer.new(Site.new, root: false))
site = Site.new(guardian)
Discourse.cache.fetch(site.cache_key, family: "site", expires_in: 1.minute) do
MultiJson.dump(SiteSerializer.new(site, root: false))
end
end
def self.invalidate_cache
Rails.cache.delete(Site.cache_key)
Discourse.cache.delete_by_family("site")
end
end

View File

@ -565,9 +565,10 @@ class User < ActiveRecord::Base
def secure_category_ids
cats = self.staff? ? Category.select(:id).where(secure: true) : secure_categories.select('categories.id')
cats.map{|c| c.id}
cats.map{|c| c.id}.sort
end
protected
def cook

View File

@ -4,7 +4,7 @@ class SiteSerializer < ApplicationSerializer
:notification_types,
:post_types
has_many :categories, embed: :objects
has_many :categories, serializer: BasicCategorySerializer, embed: :objects
has_many :post_action_types, embed: :objects
has_many :trust_levels, embed: :objects
has_many :archetypes, embed: :objects, serializer: ArchetypeSerializer

55
lib/cache.rb Normal file
View File

@ -0,0 +1,55 @@
# Standard Rails.cache is lacking support for this interface, possibly yank all in from redis:cache and start using this instead
#
class Cache
def initialize(redis=nil)
@redis = redis
end
def redis
@redis || $redis
end
def fetch(key, options={})
result = redis.get key
if result.nil?
if expiry = options[:expires_in]
if block_given?
result = yield
redis.setex(key, expiry, result)
end
else
if block_given?
result = yield
redis.set(key, result)
end
end
end
if family = family_key(options[:family])
redis.sadd(family, key)
end
result
end
def delete(key)
redis.del(key)
end
def delete_by_family(key)
k = family_key(key)
redis.smembers(k).each do |member|
delete(member)
end
redis.del(k)
end
private
def family_key(name)
if name
"FAMILY_#{name}"
end
end
end

View File

@ -1,3 +1,4 @@
require 'diffy'
# This class is used to generate diffs, it will be consumed by the UI on
# on the client the displays diffs.
#

View File

@ -1,3 +1,5 @@
require 'cache'
module Discourse
# When they try to do something they should be logged in for
@ -12,6 +14,9 @@ module Discourse
# When something they want is not found
class NotFound < Exception; end
def self.cache
@cache ||= Cache.new
end
# Get the current base URL for the current site
def self.current_hostname
@ -20,7 +25,7 @@ module Discourse
def self.base_uri default_value=""
if !ActionController::Base.config.relative_url_root.blank?
return ActionController::Base.config.relative_url_root
return ActionController::Base.config.relative_url_root
else
return default_value
end

View File

@ -364,7 +364,7 @@ class Guardian
return true unless category.secure
return false unless @user
@user.secure_category_ids.include?(category.id)
secure_category_ids.include?(category.id)
end
def can_vote?(post, opts={})
@ -405,6 +405,6 @@ class Guardian
end
def secure_category_ids
@user ? @user.secure_category_ids : []
@secure_category_ids ||= @user ? @user.secure_category_ids : []
end
end

View File

@ -24,7 +24,7 @@ module Search
"
end
def self.post_query(current_user, type, args)
def self.post_query(guardian, type, args)
builder = SqlBuilder.new <<SQL
/*select*/
FROM topics AS ft
@ -64,20 +64,25 @@ s.search_data @@ TO_TSQUERY(:locale, :query)
AND ft.archetype <> '#{Archetype.private_message}'
SQL
add_allowed_categories(builder, guardian)
builder.exec(args)
end
def self.add_allowed_categories(builder, guardian)
allowed_categories = nil
allowed_categories = current_user.secure_category_ids if current_user
allowed_categories = guardian.secure_category_ids
if allowed_categories.present?
builder.where("(c.id IS NULL OR c.secure = 'f' OR c.id in (:category_ids))", category_ids: allowed_categories)
else
builder.where("(c.id IS NULL OR c.secure = 'f')")
end
builder.exec(args)
end
def self.category_query_sql
"SELECT 'category' AS type,
def self.category_query(guardian, args)
builder = SqlBuilder.new <<SQL
SELECT 'category' AS type,
c.name AS id,
'/category/' || c.slug AS url,
c.name AS title,
@ -86,10 +91,16 @@ SQL
c.text_color
FROM categories AS c
JOIN categories_search s on s.id = c.id
WHERE s.search_data @@ TO_TSQUERY(:locale, :query)
/*where*/
ORDER BY topics_month desc
LIMIT :limit
"
SQL
builder.where "s.search_data @@ TO_TSQUERY(:locale, :query)"
add_allowed_categories(builder,guardian)
builder.exec(args)
end
def self.current_locale_long
@ -108,7 +119,9 @@ SQL
end
# needs current user for secure categories
def self.query(term, current_user, type_filter=nil, min_search_term_length=3)
def self.query(term, guardian, type_filter=nil, min_search_term_length=3)
guardian ||= Guardian.new(nil)
return nil if term.blank?
@ -125,15 +138,17 @@ SQL
if type_filter.present?
raise Discourse::InvalidAccess.new("invalid type filter") unless Search.facets.include?(type_filter)
sql = Search.send("#{type_filter}_query_sql")
a = args.merge(limit: Search.per_facet * Search.facets.size)
if type_filter.to_s == "post"
db_result = post_query(current_user, :post, a)
db_result = post_query(guardian, :post, a)
elsif type_filter.to_s == "topic"
db_result = post_query(current_user, :topic, a)
db_result = post_query(guardian, :topic, a)
elsif type_filter.to_s == "category"
db_result = category_query(guardian, a)
else
sql = Search.send("#{type_filter}_query_sql")
db_result = ActiveRecord::Base.exec_sql(sql , a)
end
@ -141,11 +156,10 @@ SQL
db_result = []
[user_query_sql, category_query_sql].each do |sql|
db_result += ActiveRecord::Base.exec_sql(sql , args.merge(limit: (Search.per_facet + 1))).to_a
end
db_result += post_query(current_user, :topic, args.merge(limit: (Search.per_facet + 1))).to_a
a = args.merge(limit: (Search.per_facet + 1))
db_result += ActiveRecord::Base.exec_sql(user_query_sql, a).to_a
db_result += category_query(guardian, a).to_a
db_result += post_query(guardian, :topic, a).to_a
end
db_result = db_result.to_a
@ -161,7 +175,7 @@ SQL
end
if expected_topics > 0
tmp = post_query(current_user, :post, args.merge(limit: expected_topics * 3))
tmp = post_query(guardian, :post, args.merge(limit: expected_topics * 3))
topic_ids = Set.new db_result.map{|r| r["id"]}

View File

@ -4,6 +4,13 @@
desc "Run all specs automatically as needed"
task "autospec" => :environment do
if RUBY_PLATFORM.include?('linux')
require 'rb-inotify'
end
require 'listen'
puts "If file watching is not working you can force polling with: bundle exec rake autospec p l=3"
require 'autospec/runner'

View File

@ -0,0 +1,63 @@
require 'spec_helper'
require 'cache'
describe Cache do
let :cache do
Cache.new
end
it "can delete by family" do
cache.fetch("key2", family: "my_family") do
"test"
end
cache.fetch("key", expires_in: 1.minute, family: "my_family") do
"test"
end
cache.delete_by_family("my_family")
cache.fetch("key").should be_nil
cache.fetch("key2").should be_nil
end
it "can delete correctly" do
r = cache.fetch("key", expires_in: 1.minute) do
"test"
end
cache.delete("key")
cache.fetch("key").should be_nil
end
it "can store with expiry correctly" do
$redis.expects(:get).with("key").returns nil
$redis.expects(:setex).with("key", 60 , "bob")
r = cache.fetch("key", expires_in: 1.minute) do
"bob"
end
r.should == "bob"
end
it "can store and fetch correctly" do
$redis.expects(:get).with("key").returns nil
$redis.expects(:set).with("key", "bob")
r = cache.fetch "key" do
"bob"
end
r.should == "bob"
end
it "can fetch existing correctly" do
$redis.expects(:get).with("key").returns "bill"
r = cache.fetch "key" do
"bob"
end
r.should == "bill"
end
end

View File

@ -4,7 +4,7 @@ require 'category_list'
describe CategoryList do
let(:user) { Fabricate(:user) }
let(:category_list) { CategoryList.new(user) }
let(:category_list) { CategoryList.new(Guardian.new user) }
context "with no categories" do
@ -39,9 +39,9 @@ describe CategoryList do
cat.allow(Group[:admins])
cat.save
CategoryList.new(admin).categories.count.should == 1
CategoryList.new(user).categories.count.should == 0
CategoryList.new(nil).categories.count.should == 0
CategoryList.new(Guardian.new admin).categories.count.should == 1
CategoryList.new(Guardian.new user).categories.count.should == 0
CategoryList.new(Guardian.new nil).categories.count.should == 0
end
end

View File

@ -164,12 +164,20 @@ describe Search do
context 'categories' do
let!(:category) { Fabricate(:category) }
let(:result) { first_of_type(Search.query('amazing', nil), 'category') }
def result
first_of_type(Search.query('amazing', nil), 'category')
end
it 'returns the correct result' do
result.should be_present
result['title'].should == category.name
result['url'].should == "/category/#{category.slug}"
r = result
r.should be_present
r['title'].should == category.name
r['url'].should == "/category/#{category.slug}"
category.deny(:all)
category.save
result.should_not be_present
end
end

View File

@ -3,12 +3,16 @@ require 'spec_helper'
describe SearchController do
it 'performs the query' do
Search.expects(:query).with('test', nil, nil, 3)
m = Guardian.new(nil)
Guardian.stubs(:new).returns(m)
Search.expects(:query).with('test', m, nil, 3)
xhr :get, :query, term: 'test'
end
it 'performs the query with a filter' do
Search.expects(:query).with('test', nil, 'topic', 3)
m = Guardian.new(nil)
Guardian.stubs(:new).returns(m)
Search.expects(:query).with('test', m, 'topic', 3)
xhr :get, :query, term: 'test', type_filter: 'topic'
end

View File

@ -33,10 +33,16 @@ Spork.prefork do
# Loading more in this block will cause your tests to run faster. However,
# if you change any configuration or code from libraries loaded here, you'll
# need to restart spork for it take effect.
require 'fabrication'
require 'mocha/api'
require 'fakeweb'
require 'certified'
ENV["RAILS_ENV"] ||= 'test'
require File.expand_path("../../config/environment", __FILE__)
require 'rspec/rails'
require 'rspec/autorun'
require 'shoulda'
# Requires supporting ruby files with custom matchers and macros, etc,
# in spec/support/ and its subdirectories.