Extracted ResourceCollection from methods on ActiveAdmin::Namespace

lib/active_admin/namespace.rb

@@ -1,4 +1,5 @@
 require 'active_admin/helpers/settings'
+require 'active_admin/resource_collection'
 
 module ActiveAdmin
 
@@ -36,7 +37,7 @@ class Namespace
     def initialize(application, name)
       @application = application
       @name = name.to_s.underscore.to_sym
-      @resources = {}
+      @resources = ResourceCollection.new
       @menu = Menu.new
       register_module unless root?
       generate_dashboard_controller
@@ -106,23 +107,14 @@ def unload!
     # loaded. This method gets called to register each resource with the menu system.
     def load_menu!
       register_dashboard
-      resources.values.each do |config|
-        register_with_menu(config) if config.include_in_menu?
+      resources.each do |resource|
+        register_with_menu(resource) if resource.include_in_menu?
       end
     end
 
     # Returns the first registered ActiveAdmin::Resource instance for a given class
     def resource_for(klass)
-      klass_name = klass.to_s
-      actual = resources.values.find{|config| config.resource.to_s == klass_name }
-      return actual if actual
-
-      if klass.respond_to?(:base_class)
-        base_class_name = klass.base_class.to_s
-        resources.values.find{|config| config.resource.to_s == base_class_name }
-      else
-        nil
-      end
+      resources.find_by_resource_class(klass)
     end
 
     # Override from ActiveAdmin::Settings to inherit default attributes
@@ -136,30 +128,11 @@ def read_default_setting(name)
     # Either returns an existing Resource instance or builds a new
     # one for the resource and options
     def find_or_build_resource(resource_class, options)
-      resource = Resource.new(self, resource_class, options)
-
-      # If we've already registered this resource, use the existing
-      if @resources.has_key? resource.camelized_resource_name
-        existing_resource = @resources[resource.camelized_resource_name]
-
-        if existing_resource.resource != resource_class
-          raise ActiveAdmin::ResourceMismatchError, 
-            "Tried to register #{resource_class} as #{resource.camelized_resource_name} but already registered to #{resource.resource}"
-        end
-
-        resource = existing_resource
-      else
-        @resources[resource.camelized_resource_name] = resource
-      end
-
-      resource
+      resources.add Resource.new(self, resource_class, options)
     end
 
     def build_page(name, options)
-      page = Page.new(self, name, options)
-      @resources[page.camelized_resource_name] = page
-
-      page
+      resources.add Page.new(self, name, options)
     end
 
 
@@ -169,13 +142,13 @@ def register_page_controller(config)
     end
 
     def unload_resources!
-      resources.each do |name, config|
+      resources.each do |resource|
         parent = (module_name || 'Object').constantize
-        const_name = config.controller_name.split('::').last
+        const_name = resource.controller_name.split('::').last
         # Remove the const if its been defined
         parent.send(:remove_const, const_name) if parent.const_defined?(const_name)
       end
-      @resources = {}
+      @resources = ResourceCollection.new
     end
 
     def unload_dashboard!

lib/active_admin/resource/belongs_to.rb

@@ -16,8 +16,8 @@ def initialize(owner_resource, target_name, options = {})
 
       # Returns the target resource class or raises an exception if it doesn't exist
       def target
-        namespace.resources[@target_name.to_s.camelize] or 
-          raise TargetNotFound, "Could not find registered resource #{@target_name} in #{namespace.name} with #{namespace.resources.keys.inspect}"
+        namespace.resources.find_by_key(@target_name.to_s.camelize) or 
+          raise TargetNotFound, "Could not find registered resource #{@target_name} in #{namespace.name} with #{namespace.resources.inspect}"
       end
 
       def namespace

lib/active_admin/resource/naming.rb

@@ -15,6 +15,11 @@ def plural_resource_name
         @plural_resource_name ||= resource_name.pluralize
       end
 
+      # A name used internally to uniquely identify this resource
+      def resource_key
+        camelized_resource_name
+      end
+
       # A camelized safe representation for this resource
       def camelized_resource_name
         resource_name.titleize.gsub(' ', '')

lib/active_admin/resource_collection.rb

@@ -0,0 +1,88 @@
+module ActiveAdmin
+
+  class ResourceMismatchError < StandardError; end
+
+  # Holds on to a collection of Resources. Is an Enumerable object
+  # so it has some Array like qualities.
+  #
+  # Adding a resource assumes that the object responds to #resource_key
+  class ResourceCollection
+    include Enumerable
+
+    def initialize
+      @resource_hash = {}
+    end
+
+    # Add a new resource to the collection. If the resource_key already
+    # exists, the exiting resource is returned.
+    #
+    # @param [Resource, Page] resource The resource to add to the collection
+    #
+    # @returns [Resource, Page] Either the existing resource or the new one
+    def add(resource)
+      if has_key?(resource.resource_key)
+        existing_resource = find_by_key(resource.resource_key)
+        ensure_resource_classes_match!(existing_resource, resource)
+        existing_resource
+      else
+        @resource_hash[resource.resource_key] = resource
+      end
+    end
+
+    # @returns [Array] An array of all the resources
+    def resources
+      @resource_hash.values
+    end
+
+    # For enumerable
+    def each(&block)
+      @resource_hash.values.each(&block)
+    end
+
+    # @returns [Array] An array of all the keys registered in the collection
+    def keys
+      @resource_hash.keys
+    end
+
+    # @returns [Boolean] If the key has been registered in the collection
+    def has_key?(resource_key)
+      @resource_hash.has_key?(resource_key)
+    end
+
+    # Finds a resource by a given key
+    def find_by_key(resource_key)
+      @resource_hash[resource_key]
+    end
+
+    # Finds a resource based on it's class. Looks up the class Heirarchy if its
+    # a subclass of an Active Record class (ie: implementes base_class)
+    def find_by_resource_class(resource_class)
+      resource_class_name = resource_class.to_s
+      match = resources_with_a_resource_class.find{|r| r.resource.to_s == resource_class_name }
+      return match if match
+
+      if resource_class.respond_to?(:base_class)
+        base_class_name = resource_class.base_class.to_s
+        resources_with_a_resource_class.find{|r| r.resource.to_s == base_class_name }
+      else
+        nil
+      end
+    end
+
+    private
+
+    def resources_with_a_resource_class
+      select{|resource| resource.respond_to?(:resource) }
+    end
+
+    def ensure_resource_classes_match!(existing_resource, resource)
+      return unless existing_resource.respond_to?(:resource) && resource.respond_to?(:resource)
+
+      if existing_resource.resource != resource.resource
+        raise ActiveAdmin::ResourceMismatchError, 
+          "Tried to register #{resource.resource} as #{resource.resource_key} but already registered to #{existing_resource.resource}"
+      end
+    end
+
+  end
+end

lib/active_admin/router.rb

@@ -28,7 +28,7 @@ def apply(router)
 
       # Now define the routes for each resource
       router.instance_exec(@application.namespaces) do |namespaces|
-        resources = namespaces.values.collect{|n| n.resources.values }.flatten
+        resources = namespaces.values.collect{|n| n.resources.resources }.flatten
         resources.each do |config|
 
           # Define the block the will get eval'd within the namespace

spec/unit/namespace_spec.rb

@@ -16,7 +16,7 @@
     end
 
     it "should have no resources" do
-      namespace.resources.should == {}
+      namespace.resources.resources.should be_empty
     end
 
     it "should have an empty menu" do

spec/unit/resource_collection_spec.rb

@@ -0,0 +1,101 @@
+require 'spec_helper'
+require 'active_admin/resource_collection'
+
+include ActiveAdmin
+
+describe ActiveAdmin::ResourceCollection do
+
+  let(:collection){ ResourceCollection.new }
+
+  it "should have no resources when new" do
+    collection.resources.should == []
+  end
+
+  it "should be enumerable" do
+    resource = mock(:resource_key => "MyResource")
+    collection.add(resource)
+    collection.each{|r| r.should == resource }
+  end
+
+  it "should return the available keys" do
+    resource = mock(:resource_key => "MyResource")
+    collection.add resource
+    collection.keys.should == [resource.resource_key]
+  end
+
+  describe "adding a new resource" do
+    let(:resource){ mock(:resource_key => "MyResource") }
+
+    it "should return the resource" do
+      collection.add(resource).should == resource
+    end
+
+    it "should add a new resource" do
+      collection.add(resource)
+      collection.resources.should == [resource]
+    end
+
+    it "should be available by name" do
+      collection.add(resource)
+      collection.find_by_key(resource.resource_key).should == resource
+    end
+  end
+
+  describe "adding a new resource when the key already exists" do
+    let(:stored_resource){ mock(:resource_key => "MyResource") }
+    let(:resource){ mock(:resource_key => "MyResource") }
+
+    before do
+      collection.add(stored_resource)
+    end
+
+    it "should return the original resource" do
+      collection.add(resource).should == stored_resource
+    end
+
+    it "should not add a new resource" do
+      collection.add(resource)
+      collection.resources.should == [stored_resource]
+    end
+  end
+
+  describe "adding an existing resource key with a different resource class" do
+    let(:stored_resource){ mock(:resource_key => "MyResource", :resource => mock) }
+    let(:resource){ mock(:resource_key => "MyResource", :resource => mock) }
+
+    it "should raise a ActiveAdmin::ResourceMismatchError" do
+      collection.add(stored_resource)
+      lambda {
+        collection.add(resource)
+      }.should raise_error(ActiveAdmin::ResourceMismatchError)
+    end
+
+  end
+
+  describe "#find_by_resource_class" do
+
+    let(:base_class){ mock(:to_s => "BaseClass")}
+    let(:resource_from_base_class){ mock(:resource_key => "MyBaseClassResource", :resource => base_class )}
+    let(:resource_class){ mock(:base_class => base_class, :to_s => "ResourceClass") }
+    let(:resource){ mock(:resource_key => "MyResource", :resource => resource_class) }
+
+    it "should find a resource when it's in the collection" do
+      collection.add resource
+      collection.find_by_resource_class(resource_class).should == resource
+    end
+
+    it "should return nil when the resource class is not in the collection" do
+      collection.find_by_resource_class(resource_class).should == nil
+    end
+
+    it "should return the resource when it and it's base class is in the collection" do
+      collection.add resource_from_base_class
+      collection.find_by_resource_class(resource_class).should == resource_from_base_class
+    end
+
+    it "should return nil the resource_class does not repond to base_class and it's not in the collection" do
+      collection.find_by_resource_class(mock).should == nil
+    end
+  end
+
+end