[CHEF-1469] load knife commands by inheritance not location

UI improvements will need to put non-subcommand files in the knife/
directory, so knife needs to have a different way to determine what the
subcommands are.

16 files changed, 164 insertions, 85 deletions

diff --git a/chef/lib/chef/knife.rb b/chef/lib/chef/knife.rb
index df6c7d7ded..2791a16068 100644
--- a/chef/lib/chef/knife.rb
+++ b/chef/lib/chef/knife.rb
@@ -17,6 +17,7 @@
 # limitations under the License.
 #
 
+require 'chef/version'
 require 'mixlib/cli'
 require 'chef/mixin/convert_to_class_name'
 
@@ -27,23 +28,56 @@ class Chef
     include Mixlib::CLI
     extend Chef::Mixin::ConvertToClassName
 
+    DEFAULT_SUBCOMMAND_FILES = Dir[File.expand_path(File.join(File.dirname(__FILE__), 'knife', '*.rb'))]
+    DEFAULT_SUBCOMMAND_FILES.map! { |knife_file| knife_file[/#{CHEF_ROOT}#{Regexp.escape(File::SEPARATOR)}(.*)\.rb/,1] }
+
     attr_accessor :name_args
 
     def self.msg(msg)
       puts msg
     end
 
+    def self.inherited(subclass)
+      unless subclass.nameless?
+        subcommands[subclass.snake_case_name] = subclass
+        #subcommands_by_category[subclass.subcommand_category] << subclass.snake_case_name
+      end
+    end
+
+    def self.category(new_category)
+      @category = new_category
+    end
+
+    def self.subcommand_category
+      @category || snake_case_name.split('_').first unless nameless?
+    end
+
+    def self.snake_case_name
+      convert_to_snake_case(name.split('::').last) unless nameless?
+    end
+
+    def self.nameless?
+      name.nil? || name.empty?
+    end
+
+    def self.subcommands
+      @@subcommands ||= {}
+    end
+
+    def self.subcommands_by_category
+      unless @subcommands_by_category
+        @subcommands_by_category = Hash.new { |hash, key| hash[key] = [] }
+        subcommands.each do |snake_cased, klass|
+          @subcommands_by_category[klass.subcommand_category] << snake_cased
+        end
+      end
+      @subcommands_by_category
+    end
+
     # Load all the sub-commands
     def self.load_commands
-      @sub_classes = Hash.new
-      Dir[
-        File.expand_path(File.join(File.dirname(__FILE__), 'knife', '*.rb'))
-      ].each do |knife_file|
-        require knife_file
-        snake_case_file_name = File.basename(knife_file).sub(/\.rb$/, '')
-        @sub_classes[snake_case_file_name] = convert_to_class_name(snake_case_file_name)
-      end
-      @sub_classes
+      DEFAULT_SUBCOMMAND_FILES.each { |subcommand| require subcommand }
+      subcommands
     end
 
     def self.list_commands(preferred_category=nil)
@@ -59,25 +93,14 @@ class Chef
       commands_to_show.sort.each do |category, commands|
         puts "** #{category.upcase} COMMANDS **"
         commands.each do |command|
-          puts Chef::Knife.const_get(@sub_classes[command]).banner
+          puts subcommands[command].banner
         end
         puts
       end
     end
 
-    def self.subcommands_by_category
-      unless @subcommands_by_category
-        @subcommands_by_category = Hash.new { |h, k| h[k] = [] }
-        @sub_classes.keys.sort.each do |command|
-          category = command.split('_').first
-          @subcommands_by_category[category] << command
-        end
-      end
-      @subcommands_by_category
-    end
-
     def self.build_sub_class(snake_case, merge_opts=nil)
-      klass = Chef::Knife.const_get(@sub_classes[snake_case])
+      klass = subcommands[snake_case]
       klass.options.merge!(merge_opts) if merge_opts 
       klass.new
     end
@@ -97,7 +120,7 @@ class Chef
         cli_bits = non_dash_args[0..to_try]
         snake_case_class_name = cli_bits.join("_")
 
-        if @sub_classes.has_key?(snake_case_class_name)
+        if subcommands.has_key?(snake_case_class_name)
           klass_instance = build_sub_class(snake_case_class_name, merge_opts)
           break
         end
diff --git a/chef/lib/chef/knife/cookbook_site_download.rb b/chef/lib/chef/knife/cookbook_site_download.rb
index 04f3667034..b5775ce3ac 100644
--- a/chef/lib/chef/knife/cookbook_site_download.rb
+++ b/chef/lib/chef/knife/cookbook_site_download.rb
@@ -24,6 +24,7 @@ class Chef
       attr_reader :version
 
       banner "knife cookbook site download COOKBOOK [VERSION] (options)"
+      category "cookbook site"
 
       option :file,
        :short => "-f FILE",
diff --git a/chef/lib/chef/knife/cookbook_site_list.rb b/chef/lib/chef/knife/cookbook_site_list.rb
index 28760a1f72..e55f20a911 100644
--- a/chef/lib/chef/knife/cookbook_site_list.rb
+++ b/chef/lib/chef/knife/cookbook_site_list.rb
@@ -23,6 +23,7 @@ class Chef
     class CookbookSiteList < Knife
 
       banner "knife cookbook site list (options)"
+      category "cookbook site"
 
       option :with_uri,
         :short => "-w",
diff --git a/chef/lib/chef/knife/cookbook_site_search.rb b/chef/lib/chef/knife/cookbook_site_search.rb
index d5dc0ff8c6..3faa3c583e 100644
--- a/chef/lib/chef/knife/cookbook_site_search.rb
+++ b/chef/lib/chef/knife/cookbook_site_search.rb
@@ -22,6 +22,7 @@ class Chef
     class CookbookSiteSearch < Knife
 
       banner "knife cookbook site search QUERY (options)"
+      category "cookbook site"
 
       def run
         output(search_cookbook(name_args[0]))
diff --git a/chef/lib/chef/knife/cookbook_site_show.rb b/chef/lib/chef/knife/cookbook_site_show.rb
index d426fdc6c3..816c0d685e 100644
--- a/chef/lib/chef/knife/cookbook_site_show.rb
+++ b/chef/lib/chef/knife/cookbook_site_show.rb
@@ -22,6 +22,7 @@ class Chef
     class CookbookSiteShow < Knife
 
       banner "knife cookbook site show COOKBOOK [VERSION] (options)"
+      category "cookbook site"
 
       def run
         case @name_args.length
diff --git a/chef/lib/chef/knife/cookbook_site_vendor.rb b/chef/lib/chef/knife/cookbook_site_vendor.rb
index 7cf3643385..de935093e5 100644
--- a/chef/lib/chef/knife/cookbook_site_vendor.rb
+++ b/chef/lib/chef/knife/cookbook_site_vendor.rb
@@ -24,6 +24,7 @@ class Chef
     class CookbookSiteVendor < Knife
 
       banner "knife cookbook site vendor COOKBOOK [VERSION] (options)"
+      category "cookbook site"
 
       option :deps,
        :short => "-d",
diff --git a/chef/lib/chef/knife/data_bag_create.rb b/chef/lib/chef/knife/data_bag_create.rb
index 20a5e035cd..22b0e0a1b1 100644
--- a/chef/lib/chef/knife/data_bag_create.rb
+++ b/chef/lib/chef/knife/data_bag_create.rb
@@ -24,6 +24,7 @@ class Chef
     class DataBagCreate < Knife
 
       banner "knife data bag create BAG [ITEM] (options)"
+      category "data bag"
 
       def run
         @data_bag_name, @data_bag_item_name = @name_args
diff --git a/chef/lib/chef/knife/data_bag_delete.rb b/chef/lib/chef/knife/data_bag_delete.rb
index 23a68737ac..6e64e6ec70 100644
--- a/chef/lib/chef/knife/data_bag_delete.rb
+++ b/chef/lib/chef/knife/data_bag_delete.rb
@@ -24,6 +24,7 @@ class Chef
     class DataBagDelete < Knife
 
       banner "knife data bag delete BAG [ITEM] (options)"
+      category "data bag"
 
       def run 
         if @name_args.length == 2
diff --git a/chef/lib/chef/knife/data_bag_edit.rb b/chef/lib/chef/knife/data_bag_edit.rb
index 9bf351d061..2f2168ef44 100644
--- a/chef/lib/chef/knife/data_bag_edit.rb
+++ b/chef/lib/chef/knife/data_bag_edit.rb
@@ -24,6 +24,7 @@ class Chef
     class DataBagEdit < Knife
 
       banner "knife data bag edit BAG ITEM (options)"
+      category "data bag"
 
       def run 
         if @name_args.length != 2
diff --git a/chef/lib/chef/knife/data_bag_from_file.rb b/chef/lib/chef/knife/data_bag_from_file.rb
index 6259e4c3c0..2c8417a4f9 100644
--- a/chef/lib/chef/knife/data_bag_from_file.rb
+++ b/chef/lib/chef/knife/data_bag_from_file.rb
@@ -25,6 +25,7 @@ class Chef
     class DataBagFromFile < Knife
 
       banner "knife data bag from file BAG FILE (options)"
+      category "data bag"
 
       def run 
         updated = load_from_file(Chef::DataBagItem, @name_args[1], @name_args[0])
diff --git a/chef/lib/chef/knife/data_bag_list.rb b/chef/lib/chef/knife/data_bag_list.rb
index 07ed6ad912..daa7624432 100644
--- a/chef/lib/chef/knife/data_bag_list.rb
+++ b/chef/lib/chef/knife/data_bag_list.rb
@@ -24,6 +24,7 @@ class Chef
     class DataBagList < Knife
 
       banner "knife data bag list (options)"
+      category "data bag"
 
       option :with_uri,
         :short => "-w",
diff --git a/chef/lib/chef/knife/data_bag_show.rb b/chef/lib/chef/knife/data_bag_show.rb
index 6838dd963a..3eae8a6632 100644
--- a/chef/lib/chef/knife/data_bag_show.rb
+++ b/chef/lib/chef/knife/data_bag_show.rb
@@ -24,6 +24,7 @@ class Chef
     class DataBagShow < Knife
 
       banner "knife data bag show BAG [ITEM] (options)"
+      category "data bag"
 
       def run
         display = case @name_args.length
diff --git a/chef/lib/chef/version.rb b/chef/lib/chef/version.rb
index 857eb3da20..c7bfdf6298 100644
--- a/chef/lib/chef/version.rb
+++ b/chef/lib/chef/version.rb
@@ -16,5 +16,6 @@
 # limitations under the License.
 
 class Chef
+  CHEF_ROOT = File.dirname(File.expand_path(File.dirname(__FILE__)))
   VERSION = '0.9.7'
 end
diff --git a/chef/spec/data/knife_subcommand/test_explicit_category.rb b/chef/spec/data/knife_subcommand/test_explicit_category.rb
new file mode 100644
index 0000000000..96d50691a1
--- /dev/null
+++ b/chef/spec/data/knife_subcommand/test_explicit_category.rb
@@ -0,0 +1,7 @@
+module KnifeSpecs
+  class TestExplicitCategory < Chef::Knife
+    # i.e., the cookbook site commands should be in the cookbook site
+    # category instead of cookbook (which is what would be assumed)
+    category "cookbook site"
+  end
+end
\ No newline at end of file
diff --git a/chef/spec/data/knife_subcommand/test_name_mapping.rb b/chef/spec/data/knife_subcommand/test_name_mapping.rb
new file mode 100644
index 0000000000..25c49b0df0
--- /dev/null
+++ b/chef/spec/data/knife_subcommand/test_name_mapping.rb
@@ -0,0 +1,4 @@
+module KnifeSpecs
+  class TestNameMapping < Chef::Knife
+  end  
+end
diff --git a/chef/spec/unit/knife_spec.rb b/chef/spec/unit/knife_spec.rb
index f620f1a85b..1d7d8e646e 100644
--- a/chef/spec/unit/knife_spec.rb
+++ b/chef/spec/unit/knife_spec.rb
@@ -16,6 +16,10 @@
 # limitations under the License.
 #
 
+module KnifeSpecs
+  
+end
+
 require File.expand_path(File.join(File.dirname(__FILE__), "..", "spec_helper"))
 
 describe Chef::Knife do
@@ -26,87 +30,116 @@ describe Chef::Knife do
     Chef::Knife.stub!(:puts)
   end
 
-  describe "class method" do
-    describe "load_commands" do
-      it "should require all the sub commands" do
-        sub_classes = Chef::Knife.load_commands
-        sub_classes.should have_key("node_show")
-        sub_classes["node_show"].should == "NodeShow"
-      end
+  it "builds a list of the core subcommand file require paths" do
+    Chef::Knife::DEFAULT_SUBCOMMAND_FILES.should_not be_empty
+    Chef::Knife::DEFAULT_SUBCOMMAND_FILES.each do |require_path|
+      require_path.should match(%w{chef knife .*}.join(Regexp.escape(File::SEPARATOR)))
     end
+  end
 
-    describe "list_commands" do
-      before(:each) do
-        @orig_argv ||= ARGV
-        redefine_argv([])
+  describe "after loading a subcommand" do
+    before do
+      if KnifeSpecs.const_defined?(:TestNameMapping)
+        KnifeSpecs.send(:remove_const, :TestNameMapping)
       end
+      Kernel.load(File.join(CHEF_SPEC_DATA, 'knife_subcommand', 'test_name_mapping.rb'))
+    end
 
-      after(:each) do
-        redefine_argv(@orig_argv)
-      end
+    it "has a category based on its name" do
+      KnifeSpecs::TestNameMapping.subcommand_category.should == 'test'
+    end
 
-      it "should load commands" do
-        Chef::Knife.should_receive(:load_commands)
-        Chef::Knife.list_commands
-      end
+    it "has an explictly defined category if set" do
+      Kernel.load(File.join(CHEF_SPEC_DATA, 'knife_subcommand', 'test_explicit_category.rb'))
+      KnifeSpecs::TestExplicitCategory.subcommand_category.should == 'cookbook site'
     end
 
-    describe "build_sub_class" do
-      before(:each) do
-        Chef::Knife.load_commands
-      end
+    it "can reference the class by its snake cased name" do
+      Chef::Knife.subcommands['test_name_mapping'].should equal(KnifeSpecs::TestNameMapping)
+    end
 
-      it "should build a sub class" do
-        Chef::Knife.build_sub_class("node_show").should be_a_kind_of(Chef::Knife::NodeShow)
-      end
+    it "can reference the class' snake cased name by its implicit category" do
+      Chef::Knife.subcommands_by_category['test'].should include('test_name_mapping')
+    end
 
-      it "should not merge options if none are passed" do
-        Chef::Knife::NodeShow.options.should_not_receive(:merge!)
-        Chef::Knife.build_sub_class("node_show")
-      end
+  end
 
-      it "should merge options if some are passed" do
-        Chef::Knife::NodeShow.options.should_receive(:merge!).with(Chef::Application::Knife.options)
-        Chef::Knife.build_sub_class("node_show", Chef::Application::Knife.options)
-      end
+  describe "after loading all subcommands" do
+    before do
+      Chef::Knife.load_commands
     end
 
-    describe "find_command" do
-      before(:each) do
-        @args = [ "node", "show", "computron" ]
-        @sub_class = Chef::Knife::NodeShow.new
-        @sub_class.stub!(:parse_options).and_return([@args[-1]])
-        @sub_class.stub!(:configure_chef).and_return(true)
-        Chef::Knife.stub!(:build_sub_class).and_return(@sub_class)
-      end
+    it "should require all the sub commands" do
+      Chef::Knife.subcommands.should have_key("node_show")
+      Chef::Knife.subcommands["node_show"].should == Chef::Knife::NodeShow
+    end
+  end
 
-      it "should find the most appropriate class" do
-        Chef::Knife.should_receive(:build_sub_class).with("node_show", {}).and_return(@sub_class)
-        Chef::Knife.find_command(@args).should be_a_kind_of(Chef::Knife::NodeShow)
-      end
+  describe "list_commands" do
+    before(:each) do
+      @orig_argv ||= ARGV
+      redefine_argv([])
+    end
 
-      it "should parse the configuration arguments" do
-        @sub_class.should_receive(:parse_options).with(@args)
-        Chef::Knife.find_command(@args)
-      end
+    after(:each) do
+      redefine_argv(@orig_argv)
+    end
 
-      it "should set the name args" do
-        @sub_class.should_receive(:name_args=).with([@args[-1]])
-        Chef::Knife.find_command(@args)
-      end
+    it "should load commands" do
+      Chef::Knife.should_receive(:load_commands)
+      Chef::Knife.list_commands
+    end
+  end
 
-      it "should exit 10 if the sub command is not found" do
-        Chef::Knife.stub!(:list_commands).and_return(true)
-        lambda {
-          Chef::Knife.find_command([ "monkey", "man" ])
-        }.should raise_error(SystemExit) { |e| e.status.should == 10 }
-      end
+  describe "build_sub_class" do
+    before(:each) do
+      Chef::Knife.load_commands
+    end
+
+    it "should build a sub class" do
+      Chef::Knife.build_sub_class("node_show").should be_a_kind_of(Chef::Knife::NodeShow)
+    end
+
+    it "should not merge options if none are passed" do
+      Chef::Knife::NodeShow.options.should_not_receive(:merge!)
+      Chef::Knife.build_sub_class("node_show")
+    end
+
+    it "should merge options if some are passed" do
+      Chef::Knife::NodeShow.options.should_receive(:merge!).with(Chef::Application::Knife.options)
+      Chef::Knife.build_sub_class("node_show", Chef::Application::Knife.options)
     end
   end
 
-  describe "initialize" do
-    it "should create a new Chef::Knife" do
-      @knife.should be_a_kind_of(Chef::Knife)
+  describe "find_command" do
+    before(:each) do
+      @args = [ "node", "show", "computron" ]
+      @sub_class = Chef::Knife::NodeShow.new
+      @sub_class.stub!(:parse_options).and_return([@args[-1]])
+      @sub_class.stub!(:configure_chef).and_return(true)
+      Chef::Knife.stub!(:build_sub_class).and_return(@sub_class)
+    end
+
+    it "should find the most appropriate class" do
+      Chef::Knife.should_receive(:build_sub_class).with("node_show", {}).and_return(@sub_class)
+      Chef::Knife.find_command(@args).should be_a_kind_of(Chef::Knife::NodeShow)
+    end
+
+    it "should parse the configuration arguments" do
+      @sub_class.should_receive(:parse_options).with(@args)
+      Chef::Knife.find_command(@args)
+    end
+
+    it "should set the name args" do
+      @sub_class.should_receive(:name_args=).with([@args[-1]])
+      Chef::Knife.find_command(@args)
+    end
+
+    it "should exit 10 if the sub command is not found" do
+      Chef::Knife.stub!(:list_commands).and_return(true)
+      lambda {
+        Chef::Knife.find_command([ "monkey", "man" ])
+      }.should raise_error(SystemExit) { |e| e.status.should == 10 }
     end
   end