Merge branch 'with_cache_rework'

Author: igal

CHANGES.txt

@@ -5,6 +5,10 @@ OpenConferenceWare stable releases and changes included, with latest at top:
 
 * Next
 - FIXED Event, added uniqueness validations to slug and title fields.
+- FIXED `rake clear` to expire the compiled javascripts.
+- FIXED cache consistency, rollbacks now cause expirations so that cache doesn't store stale data.
+- Improved performance of shared fragments, they're now only regenerated as needed and use fast preloaded data.
+- Improved performance of user favorites, they now only load the proposal ids for more efficient transfers.
 - Improved models, added cascade deletes to avoid leaving orphaned records after parents are deleted.
 - Improved "README", described the various custom environmental variables that affect the application's behavior.
 - Improved performance of event loading code by checking identifiers rather than loading entire objects.

app/controllers/user_favorites_controller.rb

@@ -14,9 +14,10 @@ def index
     @user_favorites = Defer {
       view_cache_key = "favorites,user_#{@user.id}.#{request.format},join_#{params[:join]}"
       Rails.cache.fetch_object(view_cache_key) {
-        # The :join argument, used by the AJAX, causes action to return UserFavorite records, rather than Proposal records.
+        # The :join argument is sent by the AJAX UI to fetch a terse list of
+        # proposal_ids so it can display stars next to favorited proposals.
         if params[:join] == "1"
-          UserFavorite.find_all_by_user_id(@user.id)
+          UserFavorite.proposal_ids_for(@user)
         else
           @user.favorites.populated
         end

app/helpers/shared_fragment_helper.rb

@@ -9,9 +9,13 @@
 # You can render the fragments from the command-line by running:
 #   ./script/runner "SharedFragmentHelper.render_shared_fragments"
 module SharedFragmentHelper
-  # Provides the TestRequest and TestRequest objects for ::render_theme_header_to_string.
+  # Provides the TestRequest and TestRequest objects for ::render_theme_header_to_file.
   require 'action_controller/test_process'
 
+  # Should the shared fragments be rendered? Defaults to true.
+  mattr_accessor :enabled
+  @@enabled = true
+
   # Render all shared fragments to files.
   def self.render_shared_fragments
     Rails.logger.info("SharedFragmentHelper: rendering all shared fragments to files at #{self.shared_fragments_dir}")
@@ -22,7 +26,7 @@ def self.render_shared_fragments
   # Render the theme headers for all events in the database to files.
   def self.render_theme_headers_to_files
     self.render_theme_header_to_file # current
-    for event in Event.all
+    for event in Event.lookup
       self.render_theme_header_to_file(event)
     end
     return true
@@ -31,6 +35,11 @@ def self.render_theme_headers_to_files
   # Render the theme's header to a file for the given +event+. If no event was
   # specified, will render the current event.
   def self.render_theme_header_to_file(event=nil)
+    unless self.enabled
+      Rails.logger.info("SharedFragmentHelper: not rendering because disabled")
+      return false
+    end
+
     File.open(self.shared_fragment_path_for_header(event), 'w+') do |handle|
       handle.write(self.render_theme_header_to_string(event))
     end
@@ -81,7 +90,7 @@ def self.render_theme_header_to_string(event=nil)
       # Accept given object
     when String, Symbol, Fixnum
       # Lookup by slug
-      event = Event.find_by_slug(event.to_s)
+      event = Event.lookup(event.to_s)
     else
       # Use current event
       event = Event.current

app/mixins/cache_lookups_mixin.rb

@@ -35,6 +35,21 @@ def self.included(mixee)
   end
 
   module ClassMethods
+    # Should lookups be cached? If the "perform_caching" Rails environment
+    # configuration setting is enabled, default to using cache.
+    #
+    # You can force caching on or off using the CACHELOOKUPS environmental
+    # variable, e.g. activate with:
+    #
+    #   CACHELOOKUPS=1 ./script/server
+    def cache_lookups?
+      if Rails.configuration.action_controller.perform_caching
+        return ENV['CACHELOOKUPS'] != '0'
+      else
+        return ENV['CACHELOOKUPS'] == '1'
+      end
+    end
+
     # Setup the lookup caching system.
     #
     # Arguments:
@@ -50,32 +65,40 @@ def lookup_silo_name
       return "#{self.name.gsub('::', '__')}_dict"
     end
 
+    def query_one(key)
+      return self.find(:first, :conditions => {self.lookup_key => key})
+    end
+
+    def query_all
+      return self.all(self.lookup_opts)
+    end
+
     # Return instance from cache matching +key+. If +key+ is undefined, returns
     # array of all instances.
     def lookup(key=nil)
-      ### Bypass lookup caching if the following global is true:
-      if $cache_lookup_mixin_disable
-        if key
-          return self.find(:first, :conditions => {self.lookup_key => key})
-        else
-          return self.find(:all, self.lookup_opts)
-        end
+      unless self.cache_lookups?
+        return key ?
+          self.query_one(key) :
+          self.query_all
       end
 
       silo = self.lookup_silo_name
-      dict = nil
       ActiveRecord::Base.benchmark("Lookup: #{silo}#{key.ergo{'#'+to_s}}") do
         dict = self.fetch_object(silo){
           # FIXME Exceptions within this block are silently swallowed by something. This is bad.
-          self.find(:all, self.lookup_opts).inject(Dictionary.new){|s,v| s[v.send(self.lookup_key)] = v; s}
+          dict = Dictionary.new
+          for record in self.query_all
+            dict[record.send(self.lookup_key)] = record
+          end
+          dict
         }
+        return key ? dict[key.kind_of?(Symbol) ? key.to_s : key] : dict.values
       end
-      return key ? dict[key.with{kind_of?(Symbol) ? to_s : self}] : (dict.values || [])
     end
 
     def expire_cache
       Rails.logger.info("Lookup, expiring: #{self.name}")
-      Observist.expire(/#{lookup_silo_name}_.+/)
+      CacheWatcher.expire(/#{lookup_silo_name}_.+/)
     end
 
     def fetch_object(silo, &block)

app/models/event.rb

@@ -78,19 +78,14 @@ def current?
     return self == Event.current
   end
 
-  EVENT_CURRENT_CACHE_KEY = "event_current"
-
   # Return the current Event. Determines which event to return by checking to
   # see if a snippet says which is current, else tries to return the event
   # with the latest deadline, else returns a nil.
   def self.current
-    return self.fetch_object(EVENT_CURRENT_CACHE_KEY) do
-      if record = (self.current_by_settings || self.current_by_deadline)
-        self.lookup(record.slug)
-      else
-        nil
-      end
-    end
+    query = lambda { self.current_by_settings || self.current_by_deadline }
+    return self.cache_lookups? ?
+      self.fetch_object('event_current', &query) :
+      query.call
   end
 
   # Return current event by finding it by deadline.
@@ -112,24 +107,6 @@ def self.current_by_settings
     end
   end
 
-  # Delete the current cached event if it's present
-  def self.expire_current
-    RAILS_CACHE.delete(EVENT_CURRENT_CACHE_KEY)
-  end
-
-  # Override CacheLookupsMixin to expire more
-  def self.expire_cache
-    self.expire_current
-    super
-  end
-
-  # Returns cached array of proposals for this event.
-  def lookup_proposals
-    self.fetch_object("proposals_for_event_#{self.id}") do
-      self.proposals
-    end
-  end
-
   # Return an array of this Event's Proposals with their Tracks for use by proposals#stats.
   def proposals_for_stats
     return self.proposals.find(

app/models/proposal.rb

@@ -288,7 +288,7 @@ def add_user(user)
     if self.users.include?(user)
       return nil
     else
-      Observist.expire
+      CacheWatcher.expire
       self.users << user
       return user
     end
@@ -299,7 +299,7 @@ def remove_user(user)
     user = User.get(user)
 
     if self.users.include?(user)
-      Observist.expire
+      CacheWatcher.expire
       self.users.delete(user)
       return user
     else

app/models/user_favorite.rb

@@ -33,4 +33,8 @@ def self.remove(user_id, proposal_id)
     end
   end
 
+  # Return the ids of this +user+'s favorite proposals.
+  def self.proposal_ids_for(user)
+    return self.find_all_by_user_id(user.id, :select => 'proposal_id').map(&:proposal_id)
+  end
 end

app/observers/observist.rb app/observers/cache_watcher.rbapp/observers/observist.rb renamed to app/observers/cache_watcher.rb

@@ -1,4 +1,8 @@
-class Observist < ActiveRecord::Observer
+# = CacheWatcher
+#
+# Watches for changes in models and expires the cache.
+class CacheWatcher < ActiveRecord::Observer
+  # Watch for changes in these classes:
   observe \
     Comment,
     Event,
@@ -12,25 +16,25 @@ class Observist < ActiveRecord::Observer
     User,
     UserFavorite
 
+  # Expire the cache
   def self.expire(*args)
-    pattern = args.first || //
-    Rails.logger.info("Observist: expiring cache")
+    Rails.logger.info("CacheWatcher: expiring cache")
     case Rails.cache
     when ActiveSupport::Cache::MemCacheStore
       Rails.cache.instance_variable_get(:@data).flush_all
     when ActiveSupport::Cache::FileStore
-      Rails.cache.delete_matched(pattern) rescue nil
+      Rails.cache.delete_matched(//) rescue nil
     else
       raise NotImplementedError, "Don't know how to expire cache: #{Rails.cache.class.name}"
     end
-
-    SharedFragmentHelper.render_shared_fragments
   end
 
   def expire(*args)
-    self.class.expire
+    self.class.expire(*args)
   end
 
-  alias_method :after_save,    :expire
-  alias_method :after_destroy, :expire
+  # Expire the cache when these triggers are called on a record
+  alias_method :after_save,     :expire
+  alias_method :after_destroy,  :expire
+  alias_method :after_rollback, :expire
 end

app/observers/shared_fragment_watcher.rb

@@ -0,0 +1,23 @@
+# = SharedFragmentWatcher
+#
+# Watches for changes that will affect the shared fragments and renders
+# them as needed.
+class SharedFragmentWatcher < ActiveRecord::Observer
+  # Watch for changes in these classes:
+  observe :event
+
+  # Create or update the shared fragments
+  def self.render(*args)
+    Rails.logger.info("SharedFragmentWatcher: rendering shared fragments")
+    SharedFragmentHelper.render_shared_fragments
+  end
+
+  def render(*args)
+    self.class.render(*args)
+  end
+
+  # Render shared fragments when these triggers are called on a record
+  alias_method :after_save,     :render
+  alias_method :after_destroy,  :render
+  alias_method :after_rollback, :render
+end

config/environment.rb

@@ -75,7 +75,7 @@ def load_with_announcer(*args)
 
   # Activate observers that should always be running
   # config.active_record.observers = :cacher, :garbage_collector
-  config.active_record.observers = :observist unless defined?(Rake::Task) && ARGV.find{|t| /^db:(create|drop)/}
+  config.active_record.observers = [:cache_watcher, :shared_fragment_watcher] unless defined?(Rake::Task) && ARGV.find{|t| /^db:(create|drop)/}
   # TODO Remove the above "unless" after migrating to Rails 2.3 or above. It's needed by older versions to avoid a dependency loop where creating the database requires loading the observers which load the models which have plugins that try to talk to the database.
 
   # Make Active Record use UTC-base instead of local time