Merge remote-tracking branch 'thedarkone/no-double-inclusion'

authentication/lib/refinery/authentication/engine.rb

@@ -23,8 +23,8 @@ class Engine < ::Rails::Engine
       end
 
       before_inclusion do
-        [Refinery::ApplicationController, Refinery::AdminController, ::ApplicationHelper].each do |c|
-          c.send :include, Refinery::AuthenticatedSystem
+        [Refinery::AdminController, ::ApplicationController].each do |c|
+          Refinery.include_unless_included(c, Refinery::AuthenticatedSystem)
         end
       end
 

core/lib/refinery/core.rb

@@ -159,7 +159,21 @@ def route_for_model(klass, plural = false)
       end
     end
 
+    def include_unless_included(base, extension_module)
+      base.send :include, extension_module unless included_extension_module?(base, extension_module)
+    end
+
   private
+    # plain Module#included? or Module#included_modules doesn't cut it here
+    def included_extension_module?(base, extension_module)
+      if base.kind_of?(Class)
+        direct_superclass = base.superclass
+        base.ancestors.take_while {|ancestor| ancestor != direct_superclass}.include?(extension_module)
+      else
+        base < extension_module # can't do better than that for modules
+      end
+    end
+
     def validate_extension!(const)
       unless const.respond_to?(:root) && const.root.is_a?(Pathname)
         raise InvalidEngineError, "Engine must define a root accessor that returns a pathname to its root"

core/lib/refinery/core/engine.rb

@@ -24,7 +24,7 @@ def load_decorators
         def refinery_inclusion!
           before_inclusion_procs.each(&:call)
 
-          ::ApplicationController.send :include, Refinery::ApplicationController
+          Refinery.include_unless_included(::ApplicationController, Refinery::ApplicationController)
           ::ApplicationController.send :helper, Refinery::Core::Engine.helpers
 
           after_inclusion_procs.each(&:call)

core/spec/lib/refinery/core_spec.rb

@@ -1,6 +1,24 @@
 require 'spec_helper'
 
 describe Refinery do
+  describe "#include_unless_included" do
+    it "shouldn't double include a module" do
+      mod = Module.new do
+        def self.included(base)
+          base::INCLUSIONS << self
+          super
+        end
+      end
+
+      [Module.new, Class.new].each do |target|
+        target::INCLUSIONS = []
+        subject.include_unless_included(target, mod)
+        subject.include_unless_included(target, mod)
+        target::INCLUSIONS.should have(1).item
+      end
+    end
+  end
+
   describe "#extensions" do
     it "should return an array of modules representing registered extensions" do
       subject.extensions.should be_a(Array)

core/spec/requests/refinery/core_spec.rb

@@ -0,0 +1,14 @@
+module Refinery
+  module Core
+    describe Engine do
+      describe "#refinery_inclusion!" do
+        it "should be idempotent" do
+          expect { visit(refinery.root_path) }.to_not raise_error(Exception)
+          Engine.refinery_inclusion!
+          Engine.refinery_inclusion!
+          expect { visit(refinery.root_path) }.to_not raise_error(Exception)
+        end
+      end
+    end
+  end
+end

pages/lib/refinery/pages/engine.rb

@@ -14,8 +14,8 @@ class Engine < ::Rails::Engine
       end
 
       after_inclusion do
-        ::ApplicationController.send :include, Refinery::Pages::InstanceMethods
-        Refinery::AdminController.send :include, Refinery::Pages::Admin::InstanceMethods
+        Refinery.include_unless_included(::ApplicationController, Refinery::Pages::InstanceMethods)
+        Refinery.include_unless_included(Refinery::AdminController, Refinery::Pages::Admin::InstanceMethods)
       end
 
       initializer "refinery.pages register plugin" do