Merge pull request #376 from Shopify/tag-class-reloading

Avoid holding on to stale tag classes

lib/liquid.rb

@@ -38,6 +38,9 @@ module Liquid
   PartialTemplateParser       = /#{TagStart}.*?#{TagEnd}|#{VariableStart}.*?#{VariableIncompleteEnd}/om
   TemplateParser              = /(#{PartialTemplateParser}|#{AnyStartingTag})/om
   VariableParser              = /\[[^\]]+\]|#{VariableSegment}+\??/o
+
+  singleton_class.send(:attr_accessor, :cache_classes)
+  self.cache_classes = true
 end
 
 require "liquid/version"

lib/liquid/template.rb

@@ -21,6 +21,36 @@ module Liquid
     attr_accessor :root, :resource_limits
     @@file_system = BlankFileSystem.new
 
+    class TagRegistry
+      def initialize
+        @tags  = {}
+        @cache = {}
+      end
+
+      def [](tag_name)
+        return nil unless @tags.has_key?(tag_name)
+        return @cache[tag_name] if Liquid.cache_classes
+
+        lookup_class(@tags[tag_name]).tap { |o| @cache[tag_name] = o }
+      end
+
+      def []=(tag_name, klass)
+        @tags[tag_name]  = klass.name
+        @cache[tag_name] = klass
+      end
+
+      def delete(tag_name)
+        @tags.delete(tag_name)
+        @cache.delete(tag_name)
+      end
+
+      private
+
+      def lookup_class(name)
+        name.split("::").reject(&:empty?).reduce(Object) { |scope, const| scope.const_get(const) }
+      end
+    end
+
     class << self
       # Sets how strict the parser should be.
       # :lax acts like liquid 2.5 and silently ignores malformed tags in most cases.
@@ -41,7 +71,7 @@ module Liquid
       end
 
       def tags
-        @tags ||= {}
+        @tags ||= TagRegistry.new
       end
 
       def error_mode

test/unit/template_unit_test.rb

@@ -16,4 +16,54 @@ class TemplateUnitTest < Test::Unit::TestCase
     assert_instance_of I18n, t.root.options[:locale]
     assert_equal fixture("en_locale.yml"), t.root.options[:locale].path
   end
+
+  def test_with_cache_classes_tags_returns_the_same_class
+    original_cache_setting = Liquid.cache_classes
+    Liquid.cache_classes = true
+
+    original_klass = Class.new
+    Object.send(:const_set, :CustomTag, original_klass)
+    Template.register_tag('custom', CustomTag)
+
+    Object.send(:remove_const, :CustomTag)
+
+    new_klass = Class.new
+    Object.send(:const_set, :CustomTag, new_klass)
+
+    assert Template.tags['custom'].equal?(original_klass)
+  ensure
+    Object.send(:remove_const, :CustomTag)
+    Template.tags.delete('custom')
+    Liquid.cache_classes = original_cache_setting
+  end
+
+  def test_without_cache_classes_tags_reloads_the_class
+    original_cache_setting = Liquid.cache_classes
+    Liquid.cache_classes = false
+
+    original_klass = Class.new
+    Object.send(:const_set, :CustomTag, original_klass)
+    Template.register_tag('custom', CustomTag)
+
+    Object.send(:remove_const, :CustomTag)
+
+    new_klass = Class.new
+    Object.send(:const_set, :CustomTag, new_klass)
+
+    assert Template.tags['custom'].equal?(new_klass)
+  ensure
+    Object.send(:remove_const, :CustomTag)
+    Template.tags.delete('custom')
+    Liquid.cache_classes = original_cache_setting
+  end
+
+  class FakeTag; end
+
+  def test_tags_delete
+    Template.register_tag('fake', FakeTag)
+    assert_equal FakeTag, Template.tags['fake']
+
+    Template.tags.delete('fake')
+    assert_nil Template.tags['fake']
+  end
 end