Reorganize delegated methods on CookbookVersion, cleanup comments

2 files changed, 80 insertions, 88 deletions

diff --git a/lib/chef/cookbook_manifest.rb b/lib/chef/cookbook_manifest.rb
index ef4adfbeb9..30f8596b39 100644
--- a/lib/chef/cookbook_manifest.rb
+++ b/lib/chef/cookbook_manifest.rb
@@ -24,7 +24,9 @@ class Chef
   # to a Chef Server.
   class CookbookManifest
 
-    # TODO: duplicates the same constant in CookbookVersion
+    # Duplicates the same constant in CookbookVersion. We cannot remove it
+    # there because it is treated by other code as part of CookbookVersion's
+    # public API (also used in some deprecated methods).
     COOKBOOK_SEGMENTS = [ :resources, :providers, :recipes, :definitions, :libraries, :attributes, :files, :templates, :root_files ].freeze
 
     extend Forwardable
@@ -144,14 +146,14 @@ class Chef
       "#{cookbook_url_path}/#{name}/#{version}?force=true"
     end
 
-    # TODO: This is kind of terrible. investigate removing it
+    # Update this CookbookManifest from the contents of another manifest, and
+    # make the corresponding changes to the cookbook_version object. Required
+    # to provide backward compatibility with CookbookVersion#manifest= method.
     def update_from(new_manifest)
       @manifest = Mash.new new_manifest
       @checksums = extract_checksums_from_manifest(@manifest)
       @manifest_records_by_path = extract_manifest_records_by_path(@manifest)
 
-      # TODO: this part of this method is "feature envious" it only deals with
-      # mutating the CookbookVersion object.
       COOKBOOK_SEGMENTS.each do |segment|
         next unless @manifest.has_key?(segment)
         filenames = @manifest[segment].map{|manifest_record| manifest_record['name']}
@@ -250,9 +252,6 @@ class Chef
       checksums
     end
 
-    # TODO: delegating to a class method like this is ugly. We should be able
-    # to fix this by moving logic into a class in a way that will make it easy
-    # to add support for SHA-2
     def checksum_cookbook_file(filepath)
       CookbookVersion.checksum_cookbook_file(filepath)
     end
diff --git a/lib/chef/cookbook_version.rb b/lib/chef/cookbook_version.rb
index c030e31a07..a637cac4e1 100644
--- a/lib/chef/cookbook_version.rb
+++ b/lib/chef/cookbook_version.rb
@@ -33,89 +33,8 @@ class Chef
   # cookbook. Chef supports maintaining multiple versions of a cookbook on a
   # single server; each version is represented by a distinct instance of this
   # class.
-  #--
-  # TODO: timh/cw: 5-24-2010: mutators for files (e.g., recipe_filenames=,
-  # recipe_filenames.insert) should dirty the manifest so it gets regenerated.
   class CookbookVersion
 
-    ## METHODS DELEGATED TO CookbookResourceAdapter
-    # TODO: reorganize these
-
-    private def cookbook_manifest
-      @cookbook_manifest ||= CookbookManifest.new(self)
-    end
-
-    def manifest
-      cookbook_manifest.manifest
-    end
-
-    # Returns a hash of checksums to either nil or the on disk path (which is
-    # done by generate_manifest).
-    def checksums
-      cookbook_manifest.checksums
-    end
-
-    def manifest_records_by_path
-      cookbook_manifest.manifest_records_by_path
-    end
-
-    def manifest=(new_manifest)
-      cookbook_manifest.update_from(new_manifest)
-    end
-
-    private def deprecated!(message)
-      if Chef::Config[:treat_deprecation_warnings_as_errors]
-        raise Exceptions::DeprecatedFeatureError, message
-      else
-        Chef::Log.warn(message)
-      end
-    end
-
-    def save_url
-      # TODO: double check this code suggestion
-      deprecated!(<<-DEPRECATED)
-Cookbooks now have multiple save URLs based on the capabilities of the Chef Server.
-To get the default save URL, use code like `Chef::CookbookManifest.new(cookbook_version).save_url`
-
-Called from #{caller(1).first}
-DEPRECATED
-
-      cookbook_manifest.save_url
-    end
-
-    def force_save_url
-      # TODO: double check this code suggestion
-      deprecated!(<<-DEPRECATED)
-Cookbooks now have multiple save URLs based on the capabilities of the Chef Server.
-To get the default save URL, use code like `Chef::CookbookManifest.new(cookbook_version).force_save_url`
-
-Called from #{caller(1).first}
-DEPRECATED
-      cookbook_manifest.force_save_url
-    end
-
-    def to_hash
-      # TODO: double check this code suggestion
-      deprecated!(<<-DEPRECATED)
-Cookbooks now have multiple JSON representations based on the capabilities of the Chef Server.
-To get the Hash representation, use code like `Chef::CookbookManifest.new(cookbook_version).to_hash`
-
-Called from #{caller(1).first}
-DEPRECATED
-      cookbook_manifest.to_hash
-    end
-
-    def to_json(*a)
-      # TODO: double check this code suggestion
-      deprecated!(<<-DEPRECATED)
-Cookbooks now have multiple JSON representations based on the capabilities of the Chef Server.
-To get the JSON representation, use code like `Chef::CookbookManifest.new(cookbook_version).to_json`
-
-Called from #{caller(1).first}
-DEPRECATED
-      cookbook_manifest.to_json
-    end
-
     include Comparable
 
     COOKBOOK_SEGMENTS = [ :resources, :providers, :recipes, :definitions, :libraries, :attributes, :files, :templates, :root_files ]
@@ -242,6 +161,24 @@ DEPRECATED
     alias :attribute_files :attribute_filenames
     alias :attribute_files= :attribute_filenames=
 
+    def manifest
+      cookbook_manifest.manifest
+    end
+
+    # Returns a hash of checksums to either nil or the on disk path (which is
+    # done by generate_manifest).
+    def checksums
+      cookbook_manifest.checksums
+    end
+
+    def manifest_records_by_path
+      cookbook_manifest.manifest_records_by_path
+    end
+
+    def manifest=(new_manifest)
+      cookbook_manifest.update_from(new_manifest)
+    end
+
     # Return recipe names in the form of cookbook_name::recipe_name
     def fully_qualified_recipe_names
       results = Array.new
@@ -535,6 +472,28 @@ DEPRECATED
       rendered_manifest
     end
 
+
+    def to_hash
+      deprecated!(<<-DEPRECATED)
+Cookbooks now have multiple JSON representations based on the capabilities of the Chef Server.
+To get the Hash representation, use code like `Chef::CookbookManifest.new(cookbook_version).to_hash`
+
+Called from #{caller(1).first}
+DEPRECATED
+      cookbook_manifest.to_hash
+    end
+
+    def to_json(*a)
+      deprecated!(<<-DEPRECATED)
+Cookbooks now have multiple JSON representations based on the capabilities of the Chef Server.
+To get the JSON representation, use code like `Chef::CookbookManifest.new(cookbook_version).to_json`
+
+Called from #{caller(1).first}
+DEPRECATED
+      cookbook_manifest.to_json
+    end
+
+
     def metadata_json_file
       File.join(root_paths[0], "metadata.json")
     end
@@ -552,6 +511,28 @@ DEPRECATED
     ##
     # REST API
     ##
+
+    def save_url
+      deprecated!(<<-DEPRECATED)
+Cookbooks now have multiple save URLs based on the capabilities of the Chef Server.
+To get the default save URL, use code like `Chef::CookbookManifest.new(cookbook_version).save_url`
+
+Called from #{caller(1).first}
+DEPRECATED
+
+      cookbook_manifest.save_url
+    end
+
+    def force_save_url
+      deprecated!(<<-DEPRECATED)
+Cookbooks now have multiple save URLs based on the capabilities of the Chef Server.
+To get the default save URL, use code like `Chef::CookbookManifest.new(cookbook_version).force_save_url`
+
+Called from #{caller(1).first}
+DEPRECATED
+      cookbook_manifest.force_save_url
+    end
+
     def self.chef_server_rest
       Chef::REST.new(Chef::Config[:chef_server_url])
     end
@@ -613,6 +594,18 @@ DEPRECATED
 
     private
 
+    def deprecated!(message)
+      if Chef::Config[:treat_deprecation_warnings_as_errors]
+        raise Exceptions::DeprecatedFeatureError, message
+      else
+        Chef::Log.warn(message)
+      end
+    end
+
+    def cookbook_manifest
+      @cookbook_manifest ||= CookbookManifest.new(self)
+    end
+
     def find_preferred_manifest_record(node, segment, filename)
       preferences = preferences_for_path(node, segment, filename)