From 8d18831491f69868e44cbc7c3bbb07d06af976c6 Mon Sep 17 00:00:00 2001
From: Cody Cutrer <cody@instructure.com>
Date: Mon, 6 Feb 2012 11:36:46 -0700
Subject: [PATCH] don't perma-cache special accounts

test plan:
 * set a custom css for site admin
 * other root accounts should pick up that custom css
   immediately without restarting the server

Change-Id: Ief1356f7a67b3ea461656bc8f6a9bf1938566b91
Reviewed-on: https://gerrit.instructure.com/8522
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Brian Palmer <brianp@instructure.com>
---
 app/middleware/load_account.rb            |  1 +
 app/models/account.rb                     | 40 +++++++++++------------
 spec/controllers/files_controller_spec.rb |  2 +-
 spec/spec_helper.rb                       |  6 ++++
 4 files changed, 27 insertions(+), 22 deletions(-)

diff --git a/app/middleware/load_account.rb b/app/middleware/load_account.rb
index 2a872b5812b..3e6393d2b8e 100644
--- a/app/middleware/load_account.rb
+++ b/app/middleware/load_account.rb
@@ -4,6 +4,7 @@ class LoadAccount
   end
 
   def call(env)
+    Account.clear_special_account_cache!
     domain_root_account = LoadAccount.default_domain_root_account
     configure_for_root_account(domain_root_account)
 
diff --git a/app/models/account.rb b/app/models/account.rb
index 2222efdfa79..467900f65b8 100644
--- a/app/models/account.rb
+++ b/app/models/account.rb
@@ -690,37 +690,35 @@ class Account < ActiveRecord::Base
   end
 
   def self.site_admin
-    get_special_account('site_admin', 'Site Admin')
+    get_special_account(:site_admin, 'Site Admin')
   end
 
   def self.default
-    get_special_account('default', 'Default Account')
+    get_special_account(:default, 'Default Account')
+  end
+
+  def self.clear_special_account_cache!
+    @special_accounts = {}
   end
 
   def self.get_special_account(special_account_type, default_account_name)
+    @special_account_ids ||= {}
     @special_accounts ||= {}
 
-    if Rails.env.test?
-      # TODO: we have to do this because tests run in transactions. maybe it'd
-      # be good to create some sort of of memoize_if_safe method, that only
-      # memoizes when we're caching classes and not in test mode? I dunno. But
-      # this stinks.
-      @special_accounts[special_account_type] = Account.find_by_parent_account_id_and_name(nil, default_account_name)
-      return @special_accounts[special_account_type] ||= Account.create(:parent_account => nil, :name => default_account_name)
-    end
-
     account = @special_accounts[special_account_type]
-    return account if account
-    if (account_id = Setting.get("#{special_account_type}_account_id", nil)) && account_id.present?
-      account = Account.find_by_id(account_id)
+    unless account
+      special_account_id = @special_account_ids[special_account_type] ||= Setting.get("#{special_account_type}_account_id", nil)
+      account = @special_accounts[special_account_type] = Account.find_by_id(special_account_id) if special_account_id
     end
-    return @special_accounts[special_account_type] = account if account
-    # TODO i18n
-    t '#account.default_site_administrator_account_name', 'Site Admin'
-    t '#account.default_account_name', 'Default Account'
-    account = Account.create!(:name => default_account_name)
-    Setting.set("#{special_account_type}_account_id", account.id)
-    return @special_accounts[special_account_type] = account
+    unless account
+      # TODO i18n
+      t '#account.default_site_administrator_account_name', 'Site Admin'
+      t '#account.default_account_name', 'Default Account'
+      account = @special_accounts[special_account_type] = Account.create!(:name => default_account_name)
+      Setting.set("#{special_account_type}_account_id", account.id)
+      @special_account_ids[special_account_type] = account.id
+    end
+    account
   end
 
   def site_admin?
diff --git a/spec/controllers/files_controller_spec.rb b/spec/controllers/files_controller_spec.rb
index 1ca2cb13899..d03de23a920 100644
--- a/spec/controllers/files_controller_spec.rb
+++ b/spec/controllers/files_controller_spec.rb
@@ -268,7 +268,7 @@ describe FilesController do
       @file.save!
       
       # turn off google docs previews for this acccount so we can isolate testing just scribd.
-      account = @module.context.account
+      account = Account.default
       account.disable_service(:google_docs_previews)
       account.save!
       
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 8729354de96..35b4bd7a0fc 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -94,8 +94,14 @@ Spec::Runner.configure do |config|
 
   config.include Webrat::Matchers, :type => :views
 
+  config.before :all do
+    # so before(:all)'s don't get confused
+    Account.clear_special_account_cache!
+  end
+
   config.before :each do
     Time.zone = 'UTC'
+    Account.clear_special_account_cache!
     Account.default.update_attribute(:default_time_zone, 'UTC')
     Setting.reset_cache!
     HostUrl.reset_cache!