From f58d85edea0f85a9da52798aa45bf6a202c10a61 Mon Sep 17 00:00:00 2001 From: Sam Date: Tue, 5 May 2015 15:50:13 +1000 Subject: [PATCH] FEATURE: move stylesheet cache out of the uploads directory --- app/controllers/stylesheets_controller.rb | 33 +++++++++++++++++++ app/models/stylesheet_cache.rb | 25 ++++++++++++++ config/initializers/06-mini_profiler.rb | 3 +- config/nginx.sample.conf | 2 +- config/routes.rb | 2 ++ .../20150505044154_add_stylesheet_cache.rb | 12 +++++++ lib/sass/discourse_stylesheets.rb | 10 +++--- spec/models/stylesheet_cache_spec.rb | 24 ++++++++++++++ 8 files changed, 105 insertions(+), 6 deletions(-) create mode 100644 app/controllers/stylesheets_controller.rb create mode 100644 app/models/stylesheet_cache.rb create mode 100644 db/migrate/20150505044154_add_stylesheet_cache.rb create mode 100644 spec/models/stylesheet_cache_spec.rb diff --git a/app/controllers/stylesheets_controller.rb b/app/controllers/stylesheets_controller.rb new file mode 100644 index 00000000000..ffdf7e02f5d --- /dev/null +++ b/app/controllers/stylesheets_controller.rb @@ -0,0 +1,33 @@ +class StylesheetsController < ApplicationController + skip_before_filter :preload_json, :redirect_to_login_if_required, :check_xhr, :verify_authenticity_token, only: [:show] + + def show + + target,digest = params[:name].split("_") + + digest = "_" + digest if digest + + # Security note, safe due to route constraint + location = "#{DiscourseStylesheets::CACHE_PATH}/#{target}#{digest}.css" + + unless File.exist?(location) + query = StylesheetCache.where(target: target) + if digest + query = query.where(digest: digest) + else + query = query.order('id desc') + end + + if current = query.first + File.write(location, current.content) + else + return render nothing: true, status: 404 + end + end + + expires_in 1.year, public: true unless Rails.env == "development" + send_file(location, disposition: :inline) + + end +end + diff --git a/app/models/stylesheet_cache.rb b/app/models/stylesheet_cache.rb new file mode 100644 index 00000000000..9b4f0c5b197 --- /dev/null +++ b/app/models/stylesheet_cache.rb @@ -0,0 +1,25 @@ +class StylesheetCache < ActiveRecord::Base + self.table_name = 'stylesheet_cache' + + MAX_TO_KEEP = 10 + + def self.add(target,digest,content) + success = create(target: target, digest: digest, content: content) + + count = StylesheetCache.count + if count > MAX_TO_KEEP + + remove_lower = StylesheetCache.limit(MAX_TO_KEEP) + .order('id desc') + .pluck(:id) + .last + + exec_sql("DELETE FROM stylesheet_cache where id < :id", id: remove_lower) + end + + success + rescue ActiveRecord::RecordNotUnique + false + end + +end diff --git a/config/initializers/06-mini_profiler.rb b/config/initializers/06-mini_profiler.rb index f8ae1b3335a..bd13624a609 100644 --- a/config/initializers/06-mini_profiler.rb +++ b/config/initializers/06-mini_profiler.rb @@ -33,7 +33,8 @@ if defined?(Rack::MiniProfiler) /^\/site_customizations/, /^\/uploads/, /^\/javascripts\//, - /^\/images\// + /^\/images\//, + /^\/stylesheets\// ] # For our app, let's just show mini profiler always, polling is chatty so nuke that diff --git a/config/nginx.sample.conf b/config/nginx.sample.conf index 0e6a6c5f549..ca7f2d1cbc5 100644 --- a/config/nginx.sample.conf +++ b/config/nginx.sample.conf @@ -158,7 +158,7 @@ server { # This big block is needed so we can selectively enable # acceleration for backups and avatars # see note about repetition above - location ~ ^/(letter_avatar|user_avatar|highlight-js) { + location ~ ^/(letter_avatar|user_avatar|highlight-js|stylesheets) { proxy_set_header Host $http_host; proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; diff --git a/config/routes.rb b/config/routes.rb index c697f4b20fd..28254886624 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -287,6 +287,8 @@ Discourse::Application.routes.draw do get "highlight-js/:hostname/:version.js" => "highlight_js#show", format: false, constraints: { hostname: /[\w\.-]+/ } + get "stylesheets/:name.css" => "stylesheets#show", constraints: {name: /[a-z0-9_]+/} + get "uploads/:site/:id/:sha.:extension" => "uploads#show", constraints: {site: /\w+/, id: /\d+/, sha: /[a-z0-9]{15,16}/i, extension: /\w{2,}/} get "uploads/:site/:sha" => "uploads#show", constraints: { site: /\w+/, sha: /[a-z0-9]{40}/} post "uploads" => "uploads#create" diff --git a/db/migrate/20150505044154_add_stylesheet_cache.rb b/db/migrate/20150505044154_add_stylesheet_cache.rb new file mode 100644 index 00000000000..a23757ef973 --- /dev/null +++ b/db/migrate/20150505044154_add_stylesheet_cache.rb @@ -0,0 +1,12 @@ +class AddStylesheetCache < ActiveRecord::Migration + def change + create_table :stylesheet_cache do |t| + t.string :target, null: false + t.string :digest, null: false + t.text :content, null: false + t.timestamps + end + + add_index :stylesheet_cache, [:target, :digest], unique: true + end +end diff --git a/lib/sass/discourse_stylesheets.rb b/lib/sass/discourse_stylesheets.rb index 2b8195fcc22..9d7dbbb8de3 100644 --- a/lib/sass/discourse_stylesheets.rb +++ b/lib/sass/discourse_stylesheets.rb @@ -3,7 +3,7 @@ require_dependency 'distributed_cache' class DiscourseStylesheets - CACHE_PATH ||= 'uploads/stylesheet-cache' + CACHE_PATH ||= 'tmp/stylesheet-cache' MANIFEST_DIR ||= "#{Rails.root}/tmp/cache/assets/#{Rails.env}" MANIFEST_FULL_PATH ||= "#{MANIFEST_DIR}/stylesheet-manifest" @@ -89,6 +89,7 @@ class DiscourseStylesheets File.open(stylesheet_fullpath, "w") do |f| f.puts css end + StylesheetCache.add(@target, digest, css) css end @@ -100,7 +101,7 @@ class DiscourseStylesheets end def cache_fullpath - "#{Rails.root}/public/#{CACHE_PATH}" + "#{Rails.root}/#{CACHE_PATH}" end def stylesheet_fullpath @@ -118,12 +119,13 @@ class DiscourseStylesheets "#{GlobalSetting.relative_url_root}/" end + # using uploads cause we already have all the routing in place def stylesheet_relpath - "#{root_path}#{CACHE_PATH}/#{stylesheet_filename}" + "#{root_path}stylesheets/#{stylesheet_filename}" end def stylesheet_relpath_no_digest - "#{root_path}#{CACHE_PATH}/#{stylesheet_filename_no_digest}" + "#{root_path}stylesheets/#{stylesheet_filename_no_digest}" end def stylesheet_filename diff --git a/spec/models/stylesheet_cache_spec.rb b/spec/models/stylesheet_cache_spec.rb new file mode 100644 index 00000000000..8438aba7b7c --- /dev/null +++ b/spec/models/stylesheet_cache_spec.rb @@ -0,0 +1,24 @@ +require 'spec_helper' + +describe StylesheetCache do + + describe "add" do + it "correctly cycles once MAX_TO_KEEP is hit" do + (StylesheetCache::MAX_TO_KEEP + 1).times do |i| + StylesheetCache.add(i.to_s, "d" + i.to_s, "c" + i.to_s) + end + + expect(StylesheetCache.count).to eq StylesheetCache::MAX_TO_KEEP + expect(StylesheetCache.order(:id).first.content).to eq "c1" + end + + it "does nothing if digest is set and already exists" do + StylesheetCache.add("a", "b", "c") + StylesheetCache.add("a", "b", "cc") + + expect(StylesheetCache.count).to eq 1 + expect(StylesheetCache.first.content).to eq "c" + end + + end +end