first attempt at refactoring ResourceCollection into two objects

1 files changed, 54 insertions, 154 deletions

diff --git a/lib/chef/resource_collection.rb b/lib/chef/resource_collection.rb
index cc14a03962..2cac42703e 100644
--- a/lib/chef/resource_collection.rb
+++ b/lib/chef/resource_collection.rb
@@ -17,209 +17,83 @@
 # limitations under the License.
 #
 
-require 'chef/resource'
-require 'chef/resource_collection/stepable_iterator'
+require 'chef/resource_set'
+require 'chef/resource_list'
 
 class Chef
   class ResourceCollection
-    include Enumerable
-
-    # Matches a multiple resource lookup specification,
-    # e.g., "service[nginx,unicorn]"
-    MULTIPLE_RESOURCE_MATCH = /^(.+)\[(.+?),(.+)\]$/
-
-    # Matches a single resource lookup specification,
-    # e.g., "service[nginx]"
-    SINGLE_RESOURCE_MATCH = /^(.+)\[(.+)\]$/
-
-    attr_reader :iterator
 
     def initialize
-      @resources = Array.new
-      @resources_by_name = Hash.new
-      @insert_after_idx = nil
+      @resource_set = ResourceSet.new
+      @resource_list = ResourceList.new
     end
 
+    # TODO proxy all calls with
+    # http://simonecarletti.com/blog/2010/05/understanding-ruby-and-rails-proxy-patter-delegation-and-basicobject/
+
+    # TODO fundamentally we want to write objects into 2 different data containers.  We can proxy reads, but it is
+    # much harder to proxy writes through 1 object.
+
     def all_resources
-      @resources
+      @resource_list.all_resources
     end
 
     def [](index)
-      @resources[index]
+      @resource_list[index]
     end
 
     def []=(index, arg)
-      is_chef_resource(arg)
-      @resources[index] = arg
-      @resources_by_name[arg.to_s] = index
+      @resource_list[index] = arg
     end
 
     def <<(*args)
-      args.flatten.each do |a|
-        is_chef_resource(a)
-        @resources << a
-        @resources_by_name[a.to_s] = @resources.length - 1
-      end
-      self
+      @resource_list.send(:<<, *args)
     end
 
     # 'push' is an alias method to <<
     alias_method :push, :<<
 
     def insert(resource)
-      if @insert_after_idx
-        # in the middle of executing a run, so any resources inserted now should
-        # be placed after the most recent addition done by the currently executing
-        # resource
-        insert_at(@insert_after_idx + 1, resource)
-        @insert_after_idx += 1
-      else
-        is_chef_resource(resource)
-        @resources << resource
-        @resources_by_name[resource.to_s] = @resources.length - 1
-      end
+      @resource_list.insert(resource)
     end
 
     def insert_at(insert_at_index, *resources)
-      resources.each do |resource|
-        is_chef_resource(resource)
-      end
-      @resources.insert(insert_at_index, *resources)
-      # update name -> location mappings and register new resource
-      @resources_by_name.each_key do |key|
-        @resources_by_name[key] += resources.size if @resources_by_name[key] >= insert_at_index
-      end
-      resources.each_with_index do |resource, i|
-        @resources_by_name[resource.to_s] = insert_at_index + i
-      end
+      @resource_list.insert_at(insert_at_index, *resources)
+    end
+
+    def insert_as(resource_type, instance_name, resource)
+      # TODO how does this compete with the above 2 methods?  How do I combine them?
+      @resource_set.insert_as(resource_type, instance_name, resource)
     end
 
     def each
-      @resources.each do |resource|
-        yield resource
-      end
+      @resource_list.each
     end
 
     def execute_each_resource(&resource_exec_block)
-      @iterator = StepableIterator.for_collection(@resources)
-      @iterator.each_with_index do |resource, idx|
-        @insert_after_idx = idx
-        yield resource
-      end
+      @resource_list.execute_each_resource(&resource_exec_block)
     end
 
     def each_index
-      @resources.each_index do |i|
-        yield i
-      end
+      @resource_list.each_index
     end
 
     def empty?
-      @resources.empty?
+      @resources_list.empty?
     end
 
-    def lookup(resource)
-      lookup_by = nil
-      if resource.kind_of?(Chef::Resource)
-        lookup_by = resource.to_s
-      elsif resource.kind_of?(String)
-        lookup_by = resource
-      else
-        raise ArgumentError, "Must pass a Chef::Resource or String to lookup"
-      end
-      res = @resources_by_name[lookup_by]
-      unless res
-        raise Chef::Exceptions::ResourceNotFound, "Cannot find a resource matching #{lookup_by} (did you define it first?)"
-      end
-      @resources[res]
+    def lookup(resource_type, instance_name)
+      @resource_set.lookup(resource_type, instance_name)
     end
 
-    # Find existing resources by searching the list of existing resources.  Possible
-    # forms are:
-    #
-    # find(:file => "foobar")
-    # find(:file => [ "foobar", "baz" ])
-    # find("file[foobar]", "file[baz]")
-    # find("file[foobar,baz]")
-    #
-    # Returns the matching resource, or an Array of matching resources.
-    #
-    # Raises an ArgumentError if you feed it bad lookup information
-    # Raises a Runtime Error if it can't find the resources you are looking for.
     def find(*args)
-      results = Array.new
-      args.each do |arg|
-        case arg
-        when Hash
-          results << find_resource_by_hash(arg)
-        when String
-          results << find_resource_by_string(arg)
-        else
-          msg = "arguments to #{self.class.name}#find should be of the form :resource => 'name' or resource[name]"
-          raise Chef::Exceptions::InvalidResourceSpecification, msg
-        end
-      end
-      flat_results = results.flatten
-      flat_results.length == 1 ? flat_results[0] : flat_results
+      @resource_list.find(*args)
     end
 
     # resources is a poorly named, but we have to maintain it for back
     # compat.
     alias_method :resources, :find
 
-    # Returns true if +query_object+ is a valid string for looking up a
-    # resource, or raises InvalidResourceSpecification if not.
-    # === Arguments
-    # * query_object should be a string of the form
-    # "resource_type[resource_name]", a single element Hash (e.g., :service =>
-    # "apache2"), or a Chef::Resource (this is the happy path). Other arguments
-    # will raise an exception.
-    # === Returns
-    # * true returns true for all valid input.
-    # === Raises
-    # * Chef::Exceptions::InvalidResourceSpecification for all invalid input.
-    def validate_lookup_spec!(query_object)
-      case query_object
-      when Chef::Resource
-        true
-      when SINGLE_RESOURCE_MATCH, MULTIPLE_RESOURCE_MATCH
-        true
-      when Hash
-        true
-      when String
-        raise Chef::Exceptions::InvalidResourceSpecification,
-          "The string `#{query_object}' is not valid for resource collection lookup. Correct syntax is `resource_type[resource_name]'"
-      else
-        raise Chef::Exceptions::InvalidResourceSpecification,
-          "The object `#{query_object.inspect}' is not valid for resource collection lookup. " +
-          "Use a String like `resource_type[resource_name]' or a Chef::Resource object"
-      end
-    end
-
-    # Serialize this object as a hash
-    def to_hash
-      instance_vars = Hash.new
-      self.instance_variables.each do |iv|
-        instance_vars[iv] = self.instance_variable_get(iv)
-      end
-      {
-        'json_class' => self.class.name,
-        'instance_vars' => instance_vars
-      }
-    end
-
-    def to_json(*a)
-      Chef::JSONCompat.to_json(to_hash, *a)
-    end
-
-    def self.json_create(o)
-      collection = self.new()
-      o["instance_vars"].each do |k,v|
-        collection.instance_variable_set(k.to_sym, v)
-      end
-      collection
-    end
-
     private
 
       def find_resource_by_hash(arg)
@@ -263,4 +137,30 @@ class Chef
         true
       end
   end
+
+  module ResourceCollectionSerialization
+    # Serialize this object as a hash
+    def to_hash
+      instance_vars = Hash.new
+      self.instance_variables.each do |iv|
+        instance_vars[iv] = self.instance_variable_get(iv)
+      end
+      {
+          'json_class' => self.class.name,
+          'instance_vars' => instance_vars
+      }
+    end
+
+    def to_json(*a)
+      Chef::JSONCompat.to_json(to_hash, *a)
+    end
+
+    def self.json_create(o)
+      collection = self.new()
+      o["instance_vars"].each do |k,v|
+        collection.instance_variable_set(k.to_sym, v)
+      end
+      collection
+    end
+  end
 end