From e5888cf09091b74edb7009fa41cff72b29065574 Mon Sep 17 00:00:00 2001 From: Sam Date: Wed, 20 May 2015 17:12:16 +1000 Subject: [PATCH] PERF: avoid preloading json in cases where it is not needed (uploads / avatars / non GET requests) --- app/controllers/application_controller.rb | 4 ++++ app/controllers/clicks_controller.rb | 2 +- app/controllers/draft_controller.rb | 3 ++- app/controllers/email_controller.rb | 2 +- app/controllers/embed_controller.rb | 4 +--- app/controllers/export_csv_controller.rb | 2 +- app/controllers/forums_controller.rb | 2 +- app/controllers/highlight_js_controller.rb | 2 +- app/controllers/invites_controller.rb | 3 ++- app/controllers/posts_controller.rb | 2 +- app/controllers/robots_txt_controller.rb | 2 +- app/controllers/session_controller.rb | 2 +- app/controllers/site_controller.rb | 2 ++ app/controllers/site_customizations_controller.rb | 2 +- app/controllers/uploads_controller.rb | 2 +- app/controllers/user_avatars_controller.rb | 2 +- 16 files changed, 22 insertions(+), 16 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 6775cdf53..c492f80f0 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -168,6 +168,10 @@ class ApplicationController < ActionController::Base # We don't preload JSON on xhr or JSON request return if request.xhr? || request.format.json? + # if we are posting in makes no sense to preload + return if request.method != "GET" + + # TODO should not be invoked on redirection so this should be further deferred preload_anonymous_data if current_user diff --git a/app/controllers/clicks_controller.rb b/app/controllers/clicks_controller.rb index d0ee53d14..4d7e186b0 100644 --- a/app/controllers/clicks_controller.rb +++ b/app/controllers/clicks_controller.rb @@ -1,6 +1,6 @@ class ClicksController < ApplicationController - skip_before_filter :check_xhr + skip_before_filter :check_xhr, :preload_json def track raise Discourse::NotFound unless params[:url] diff --git a/app/controllers/draft_controller.rb b/app/controllers/draft_controller.rb index 3dd165b44..7af211db5 100644 --- a/app/controllers/draft_controller.rb +++ b/app/controllers/draft_controller.rb @@ -1,6 +1,7 @@ class DraftController < ApplicationController before_filter :ensure_logged_in - skip_before_filter :check_xhr + # TODO really do we need to skip this? + skip_before_filter :check_xhr, :preload_json def show seq = params[:sequence] || DraftSequence.current(current_user, params[:draft_key]) diff --git a/app/controllers/email_controller.rb b/app/controllers/email_controller.rb index 6c47bc406..ca8255334 100644 --- a/app/controllers/email_controller.rb +++ b/app/controllers/email_controller.rb @@ -1,5 +1,5 @@ class EmailController < ApplicationController - skip_before_filter :check_xhr + skip_before_filter :check_xhr, :preload_json layout 'no_ember' before_filter :ensure_logged_in, only: :preferences_redirect diff --git a/app/controllers/embed_controller.rb b/app/controllers/embed_controller.rb index 033d47a18..249a18f7a 100644 --- a/app/controllers/embed_controller.rb +++ b/app/controllers/embed_controller.rb @@ -1,7 +1,5 @@ class EmbedController < ApplicationController - skip_before_filter :check_xhr - skip_before_filter :preload_json - skip_before_filter :verify_authenticity_token + skip_before_filter :check_xhr, :preload_json, :verify_authenticity_token before_filter :ensure_embeddable diff --git a/app/controllers/export_csv_controller.rb b/app/controllers/export_csv_controller.rb index 353ddcd9d..af53d963b 100644 --- a/app/controllers/export_csv_controller.rb +++ b/app/controllers/export_csv_controller.rb @@ -1,6 +1,6 @@ class ExportCsvController < ApplicationController - skip_before_filter :check_xhr, only: [:show] + skip_before_filter :preload_json, :check_xhr, only: [:show] def export_entity params.require(:entity) diff --git a/app/controllers/forums_controller.rb b/app/controllers/forums_controller.rb index ea80a017f..1f41e94dd 100644 --- a/app/controllers/forums_controller.rb +++ b/app/controllers/forums_controller.rb @@ -1,6 +1,6 @@ class ForumsController < ApplicationController - skip_before_filter :check_xhr + skip_before_filter :preload_json, :check_xhr skip_before_filter :authorize_mini_profiler, only: [:status] skip_before_filter :redirect_to_login_if_required, only: [:status] diff --git a/app/controllers/highlight_js_controller.rb b/app/controllers/highlight_js_controller.rb index 74ed98b3c..d9d9c4547 100644 --- a/app/controllers/highlight_js_controller.rb +++ b/app/controllers/highlight_js_controller.rb @@ -1,5 +1,5 @@ class HighlightJsController < ApplicationController - skip_before_filter :redirect_to_login_if_required, :check_xhr, :verify_authenticity_token, only: [:show] + skip_before_filter :preload_json, :redirect_to_login_if_required, :check_xhr, :verify_authenticity_token, only: [:show] def show RailsMultisite::ConnectionManagement.with_hostname(params[:hostname]) do diff --git a/app/controllers/invites_controller.rb b/app/controllers/invites_controller.rb index 6756e4a2f..3a7f6437f 100644 --- a/app/controllers/invites_controller.rb +++ b/app/controllers/invites_controller.rb @@ -1,6 +1,7 @@ class InvitesController < ApplicationController - skip_before_filter :check_xhr + # TODO tighten this, why skip check on everything? + skip_before_filter :check_xhr, :preload_json skip_before_filter :redirect_to_login_if_required before_filter :ensure_logged_in, only: [:destroy, :create, :resend_invite, :check_csv_chunk, :upload_csv_chunk] diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index 46748cf12..f595ada86 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -9,7 +9,7 @@ class PostsController < ApplicationController # Need to be logged in for all actions here before_filter :ensure_logged_in, except: [:show, :replies, :by_number, :short_link, :reply_history, :revisions, :latest_revision, :expand_embed, :markdown_id, :markdown_num, :cooked, :latest] - skip_before_filter :check_xhr, only: [:markdown_id, :markdown_num, :short_link] + skip_before_filter :preload_json, :check_xhr, only: [:markdown_id, :markdown_num, :short_link] def markdown_id markdown Post.find(params[:id].to_i) diff --git a/app/controllers/robots_txt_controller.rb b/app/controllers/robots_txt_controller.rb index bb6cc9631..0853c3efa 100644 --- a/app/controllers/robots_txt_controller.rb +++ b/app/controllers/robots_txt_controller.rb @@ -1,6 +1,6 @@ class RobotsTxtController < ApplicationController layout false - skip_before_filter :check_xhr + skip_before_filter :preload_json, :check_xhr def index path = if SiteSetting.allow_index_in_robots_txt diff --git a/app/controllers/session_controller.rb b/app/controllers/session_controller.rb index c1410ce73..d6a6432e4 100644 --- a/app/controllers/session_controller.rb +++ b/app/controllers/session_controller.rb @@ -4,7 +4,7 @@ require_dependency 'single_sign_on' class SessionController < ApplicationController skip_before_filter :redirect_to_login_if_required - skip_before_filter :check_xhr, only: ['sso', 'sso_login', 'become', 'sso_provider'] + skip_before_filter :preload_json, :check_xhr, only: ['sso', 'sso_login', 'become', 'sso_provider'] def csrf render json: {csrf: form_authenticity_token } diff --git a/app/controllers/site_controller.rb b/app/controllers/site_controller.rb index 5c20e87b1..6597e7e76 100644 --- a/app/controllers/site_controller.rb +++ b/app/controllers/site_controller.rb @@ -2,6 +2,8 @@ require_dependency 'site_serializer' class SiteController < ApplicationController + skip_before_filter :preload_json + def site render json: Site.json_for(guardian) end diff --git a/app/controllers/site_customizations_controller.rb b/app/controllers/site_customizations_controller.rb index 248a4bacc..78f2177b3 100644 --- a/app/controllers/site_customizations_controller.rb +++ b/app/controllers/site_customizations_controller.rb @@ -1,5 +1,5 @@ class SiteCustomizationsController < ApplicationController - skip_before_filter :check_xhr, :redirect_to_login_if_required + skip_before_filter :preload_json, :check_xhr, :redirect_to_login_if_required def show expires_in 1.year, public: true diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 40da4f273..212c923c4 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -1,6 +1,6 @@ class UploadsController < ApplicationController before_filter :ensure_logged_in, except: [:show] - skip_before_filter :check_xhr, only: [:show] + skip_before_filter :preload_json, :check_xhr, only: [:show] def create file = params[:file] || params[:files].first diff --git a/app/controllers/user_avatars_controller.rb b/app/controllers/user_avatars_controller.rb index 80093b600..f3a03cf00 100644 --- a/app/controllers/user_avatars_controller.rb +++ b/app/controllers/user_avatars_controller.rb @@ -3,7 +3,7 @@ require_dependency 'letter_avatar' class UserAvatarsController < ApplicationController DOT = Base64.decode64("R0lGODlhAQABALMAAAAAAIAAAACAAICAAAAAgIAAgACAgMDAwICAgP8AAAD/AP//AAAA//8A/wD//wBiZCH5BAEAAA8ALAAAAAABAAEAAAQC8EUAOw==") - skip_before_filter :redirect_to_login_if_required, :check_xhr, :verify_authenticity_token, only: [:show, :show_letter] + skip_before_filter :preload_json, :redirect_to_login_if_required, :check_xhr, :verify_authenticity_token, only: [:show, :show_letter] def refresh_gravatar user = User.find_by(username_lower: params[:username].downcase)