(#6770) Change versioning; adopt :current over :latest.

As per discussion with Luke, versions of an interface are first looked up by
requiring 'puppet/interface/{name}', and secondarily looked up by requiring
'{name}@{version}/puppet/interface/{name}' if the first failed.

A version of `:current` can be used to represent the version living in
'puppet/interface/{name}'.

Paired-With: Nick Lewis

22 files changed, 114 insertions, 197 deletions

diff --git a/README.markdown b/README.markdown
index 29ff41430..28289ee10 100644
--- a/README.markdown
+++ b/README.markdown
@@ -1,11 +1,11 @@
-Puppet Interfaces
+Puppet Strings
 =================
 A set of executables that provide complete CLI access to Puppet's
-core data types.  They also provide Interface classes for
+core data types.  They also provide String classes for
 each of the core data types, which are extensible via plugins.
 
 For instance, you can create a new action for catalogs at
-lib/puppet/interface/catalog/$action.rb.
+lib/puppet/string/catalog/$action.rb.
 
 This is a Puppet module and should work fine if you install it
 in Puppet's module path.
@@ -22,7 +22,7 @@ Usage
 -----
 The general usage is:
 
-    $ puppet <interface> <verb> <name>
+    $ puppet <string> <verb> <name>
 
 So, e.g.:
 
@@ -31,7 +31,7 @@ So, e.g.:
 
 You can use it to list all known data types and the available terminus classes:
 
-    $ puppet interface list
+    $ puppet string list
     catalog                       : active_record, compiler, queue, rest, yaml
     certificate                   : ca, file, rest
     certificate_request           : ca, file, rest
@@ -82,21 +82,21 @@ This compiles a test catalog (assuming that ~/bin/test.pp exists) and returns it
 Or use IRB to do the same thing:
 
     $ irb
-    >> require 'puppet/interface'
+    >> require 'puppet/string'
     => true
-    >> interface = Puppet::Interface[:facts, '1.0.0']
-    => #<Puppet::Interface::Facts:0x1024a1390 @format=:yaml>
-    >> facts = interface.find("myhost")
+    >> string = Puppet::String[:facts, '1.0.0']
+    => #<Puppet::String::Facts:0x1024a1390 @format=:yaml>
+    >> facts = string.find("myhost")
 
 Like I said, a prototype, but I'd love it if people would play it with some and make some recommendations.
 
 Extending
 ---------
-Like most parts of Puppet, these are easy to extend.  Just drop a new action into a given interface's directory.  E.g.:
+Like most parts of Puppet, these are easy to extend.  Just drop a new action into a given string's directory.  E.g.:
 
-    $ cat lib/puppet/interface/catalog/select.rb 
+    $ cat lib/puppet/string/catalog/select.rb 
     # Select and show a list of resources of a given type.
-    Puppet::Interface.define(:catalog, '1.0.0') do
+    Puppet::String.define(:catalog, '1.0.0') do
       action :select do
         invoke do |host,type|
           catalog = Puppet::Resource::Catalog.indirection.find(host)
@@ -112,4 +112,4 @@ Like most parts of Puppet, these are easy to extend.  Just drop a new action int
 
 Notice that this gets loaded automatically when you try to use it.  So, if you have a simple command you've written, such as for cleaning up nodes or diffing catalogs, you an port it to this framework and it should fit cleanly.
 
-Also note that interfaces are versioned.  These version numbers are interpreted according to Semantic Versioning (http://semver.org).
+Also note that strings are versioned.  These version numbers are interpreted according to Semantic Versioning (http://semver.org).
diff --git a/lib/puppet/application/string_base.rb b/lib/puppet/application/string_base.rb
index 5b701597d..bc627adde 100644
--- a/lib/puppet/application/string_base.rb
+++ b/lib/puppet/application/string_base.rb
@@ -72,10 +72,10 @@ class Puppet::Application::StringBase < Puppet::Application
     @type = self.class.name.to_s.sub(/.+:/, '').downcase.to_sym
 
     # TODO: These should be configurable versions.
-    unless Puppet::String.string?(@type, :latest)
+    unless Puppet::String.string?(@type, :current)
       raise "Could not find any version of string '#{@type}'"
     end
-    @string = Puppet::String[@type, :latest]
+    @string = Puppet::String[@type, :current]
     @format ||= @string.default_format
 
     # We copy all of the app options to the string.
diff --git a/lib/puppet/string.rb b/lib/puppet/string.rb
index b5f7b9048..9a223a40c 100644
--- a/lib/puppet/string.rb
+++ b/lib/puppet/string.rb
@@ -70,23 +70,25 @@ class Puppet::String
 
   # Try to find actions defined in other files.
   def load_actions
-    path = "puppet/string/v#{version}/#{name}"
+    path = "puppet/string/#{name}"
 
     loaded = []
-    Puppet::String.autoloader.search_directories.each do |dir|
-      fdir = ::File.join(dir, path)
-      next unless FileTest.directory?(fdir)
-
-      Dir.chdir(fdir) do
-        Dir.glob("*.rb").each do |file|
-          aname = file.sub(/\.rb/, '')
-          if loaded.include?(aname)
-            Puppet.debug "Not loading duplicate action '#{aname}' for '#{name}' from '#{fdir}/#{file}'"
-            next
+    [path, "#{name}@#{version}/#{path}"].each do |path|
+      Puppet::String.autoloader.search_directories.each do |dir|
+        fdir = ::File.join(dir, path)
+        next unless FileTest.directory?(fdir)
+
+        Dir.chdir(fdir) do
+          Dir.glob("*.rb").each do |file|
+            aname = file.sub(/\.rb/, '')
+            if loaded.include?(aname)
+              Puppet.debug "Not loading duplicate action '#{aname}' for '#{name}' from '#{fdir}/#{file}'"
+              next
+            end
+            loaded << aname
+            Puppet.debug "Loading action '#{aname}' for '#{name}' from '#{fdir}/#{file}'"
+            require "#{path}/#{aname}"
           end
-          loaded << aname
-          Puppet.debug "Loading action '#{aname}' for '#{name}' from '#{fdir}/#{file}'"
-          require "#{path}/#{aname}"
         end
       end
     end
diff --git a/lib/puppet/string/v0.0.1/catalog.rb b/lib/puppet/string/catalog.rb
index 0ddd83176..0ddd83176 100644
--- a/lib/puppet/string/v0.0.1/catalog.rb
+++ b/lib/puppet/string/catalog.rb
diff --git a/lib/puppet/string/v0.0.1/catalog/select.rb b/lib/puppet/string/catalog/select.rb
index 52c77d3ce..52c77d3ce 100644
--- a/lib/puppet/string/v0.0.1/catalog/select.rb
+++ b/lib/puppet/string/catalog/select.rb
diff --git a/lib/puppet/string/v0.0.1/certificate.rb b/lib/puppet/string/certificate.rb
index 7b2e5f397..7b2e5f397 100644
--- a/lib/puppet/string/v0.0.1/certificate.rb
+++ b/lib/puppet/string/certificate.rb
diff --git a/lib/puppet/string/v0.0.1/certificate_request.rb b/lib/puppet/string/certificate_request.rb
index 218b40b98..218b40b98 100644
--- a/lib/puppet/string/v0.0.1/certificate_request.rb
+++ b/lib/puppet/string/certificate_request.rb
diff --git a/lib/puppet/string/v0.0.1/certificate_revocation_list.rb b/lib/puppet/string/certificate_revocation_list.rb
index 9731b4f2d..9731b4f2d 100644
--- a/lib/puppet/string/v0.0.1/certificate_revocation_list.rb
+++ b/lib/puppet/string/certificate_revocation_list.rb
diff --git a/lib/puppet/string/v0.0.1/config.rb b/lib/puppet/string/config.rb
index ae1a408cf..ae1a408cf 100644
--- a/lib/puppet/string/v0.0.1/config.rb
+++ b/lib/puppet/string/config.rb
diff --git a/lib/puppet/string/v0.0.1/configurer.rb b/lib/puppet/string/configurer.rb
index a6ea74b6a..a6ea74b6a 100644
--- a/lib/puppet/string/v0.0.1/configurer.rb
+++ b/lib/puppet/string/configurer.rb
diff --git a/lib/puppet/string/v0.0.1/facts.rb b/lib/puppet/string/facts.rb
index 73acb0df6..73acb0df6 100644
--- a/lib/puppet/string/v0.0.1/facts.rb
+++ b/lib/puppet/string/facts.rb
diff --git a/lib/puppet/string/v0.0.1/file.rb b/lib/puppet/string/file.rb
index cc5737f28..cc5737f28 100644
--- a/lib/puppet/string/v0.0.1/file.rb
+++ b/lib/puppet/string/file.rb
diff --git a/lib/puppet/string/v0.0.1/key.rb b/lib/puppet/string/key.rb
index 95aceade5..95aceade5 100644
--- a/lib/puppet/string/v0.0.1/key.rb
+++ b/lib/puppet/string/key.rb
diff --git a/lib/puppet/string/v0.0.1/node.rb b/lib/puppet/string/node.rb
index bc31a2cf3..bc31a2cf3 100644
--- a/lib/puppet/string/v0.0.1/node.rb
+++ b/lib/puppet/string/node.rb
diff --git a/lib/puppet/string/v0.0.1/report.rb b/lib/puppet/string/report.rb
index 55a008533..55a008533 100644
--- a/lib/puppet/string/v0.0.1/report.rb
+++ b/lib/puppet/string/report.rb
diff --git a/lib/puppet/string/v0.0.1/resource.rb b/lib/puppet/string/resource.rb
index 9838be0fa..9838be0fa 100644
--- a/lib/puppet/string/v0.0.1/resource.rb
+++ b/lib/puppet/string/resource.rb
diff --git a/lib/puppet/string/v0.0.1/resource_type.rb b/lib/puppet/string/resource_type.rb
index 8ca31ea6c..8ca31ea6c 100644
--- a/lib/puppet/string/v0.0.1/resource_type.rb
+++ b/lib/puppet/string/resource_type.rb
diff --git a/lib/puppet/string/v0.0.1/status.rb b/lib/puppet/string/status.rb
index 41de2bb99..41de2bb99 100644
--- a/lib/puppet/string/v0.0.1/status.rb
+++ b/lib/puppet/string/status.rb
diff --git a/lib/puppet/string/string_collection.rb b/lib/puppet/string/string_collection.rb
index e9cba7f55..45a192703 100644
--- a/lib/puppet/string/string_collection.rb
+++ b/lib/puppet/string/string_collection.rb
@@ -26,61 +26,39 @@ module Puppet::String::StringCollection
     return @strings.keys
   end
 
-  def self.versions(name)
-    versions = []
-    $LOAD_PATH.each do |dir|
-      next unless FileTest.directory?(dir)
-      v_dir = File.join dir, %w[puppet string v*]
-      Dir.glob(File.join v_dir, "#{name}{.rb,/*.rb}").each do |f|
-        v = f.sub(%r[.*/v([^/]+?)/#{name}(?:(?:/[^/]+)?.rb)$], '\1')
-        if validate_version(v)
-          versions << v
-        else
-          warn "'#{v}' (#{f}) is not a valid version string; skipping"
-        end
-      end
-    end
-    return versions.uniq.sort { |a, b| compare_versions(a, b)  }
-  end
-
   def self.validate_version(version)
     !!(SEMVER_VERSION =~ version.to_s)
   end
 
-  def self.compare_versions(a, b)
-    a, b = [a, b].map do |x|
-      parts = SEMVER_VERSION.match(x).to_a[1..4]
-      parts[0..2] = parts[0..2].map { |e| e.to_i }
-      parts
-    end
-
-    cmp = a[0..2] <=> b[0..2]
-    if cmp == 0
-      cmp = a[3] <=> b[3]
-      cmp = +1 if a[3].empty? && !b[3].empty?
-      cmp = -1 if b[3].empty? && !a[3].empty?
-    end
-    cmp
-  end
-
   def self.[](name, version)
-    version = versions(name).last if version == :latest
-    unless version.nil?
-      @strings[underscorize(name)][version] if string?(name, version)
-    end
+    @strings[underscorize(name)][version] if string?(name, version)
   end
 
   def self.string?(name, version)
-    version = versions(name).last if version == :latest
-    return false if version.nil?
-
     name = underscorize(name)
+    cache = @strings[name]
+    return true if cache.has_key?(version)
 
-    unless @strings.has_key?(name) && @strings[name].has_key?(version)
-      require "puppet/string/v#{version}/#{name}"
+    loaded = cache.keys
+
+    files = ["puppet/string/#{name}"]
+    unless version == :current
+      files << "#{name}@#{version}/puppet/string/#{name}"
     end
-    return @strings.has_key?(name) && @strings[name].has_key?(version)
-  rescue LoadError
+
+    files.each do |file|
+      begin
+        require file
+        if version == :current || !file.include?('@')
+          loaded = (cache.keys - loaded).first
+          cache[:current] = cache[loaded] unless loaded.nil?
+        end
+        return true if cache.has_key?(version)
+      rescue LoadError
+        # pass
+      end
+    end
+
     return false
   end
 
diff --git a/spec/unit/application/string_base_spec.rb b/spec/unit/application/string_base_spec.rb
index bc563e11d..86f9c09aa 100755
--- a/spec/unit/application/string_base_spec.rb
+++ b/spec/unit/application/string_base_spec.rb
@@ -2,13 +2,19 @@
 
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/application/string_base'
+require 'tmpdir'
+
+class Puppet::Application::StringBase::Basetest < Puppet::Application::StringBase
+end
 
 describe Puppet::Application::StringBase do
   before :all do
     @dir = Dir.mktmpdir
     $LOAD_PATH.push(@dir)
-    FileUtils.mkdir_p(File.join @dir, 'puppet', 'string', 'v0.0.1')
-    FileUtils.touch(File.join @dir, 'puppet', 'string', 'v0.0.1', 'basetest.rb')
+    FileUtils.mkdir_p(File.join @dir, 'puppet', 'string')
+    File.open(File.join(@dir, 'puppet', 'string', 'basetest.rb'), 'w') do |f|
+      f.puts "Puppet::String.define(:basetest, '0.0.1')"
+    end
   end
 
   after :all do
@@ -16,13 +22,8 @@ describe Puppet::Application::StringBase do
     $LOAD_PATH.pop
   end
 
-  base_string = Puppet::String.define(:basetest, '0.0.1')
-  class Puppet::Application::StringBase::Basetest < Puppet::Application::StringBase
-  end
-
   before do
     @app = Puppet::Application::StringBase::Basetest.new
-    @app.stubs(:string).returns base_string
     @app.stubs(:exit)
     @app.stubs(:puts)
     Puppet::Util::Log.stubs(:newdestination)
diff --git a/spec/unit/string/string_collection_spec.rb b/spec/unit/string/string_collection_spec.rb
index 46c431f75..63ddf7c5e 100755
--- a/spec/unit/string/string_collection_spec.rb
+++ b/spec/unit/string/string_collection_spec.rb
@@ -5,64 +5,19 @@ require 'tmpdir'
 
 describe Puppet::String::StringCollection do
   before :all do
-    @strings = subject.instance_variable_get("@strings").dup
+    @strings = subject.instance_variable_get("@strings")
+    @strings_backup = @strings.dup
   end
 
-  before :each do
-    subject.instance_variable_get("@strings").clear
-  end
+  before { @strings.clear }
 
   after :all do
-    subject.instance_variable_set("@strings", @strings)
+    subject.instance_variable_set("@strings", @strings_backup)
   end
 
   describe "::strings" do
   end
 
-  describe "::versions" do
-    before :each do
-      @dir = Dir.mktmpdir
-      @lib = FileUtils.mkdir_p(File.join @dir, 'puppet', 'string')
-      $LOAD_PATH.push(@dir)
-    end
-
-    after :each do
-      FileUtils.remove_entry_secure @dir
-      $LOAD_PATH.pop
-    end
-
-    it "should return an empty array when no versions are loadable" do
-      subject.versions(:fozzie).should == []
-    end
-
-    it "should return versions loadable as puppet/string/v{version}/{name}" do
-      FileUtils.mkdir_p(File.join @lib, 'v1.0.0')
-      FileUtils.touch(File.join @lib, 'v1.0.0', 'fozzie.rb')
-      subject.versions(:fozzie).should == ['1.0.0']
-    end
-
-    it "should an ordered list of all versions loadable as puppet/string/v{version}/{name}" do
-      %w[ 1.2.1rc2 1.2.1beta1 1.2.1rc1 1.2.1 1.2.2 ].each do |version|
-        FileUtils.mkdir_p(File.join @lib, "v#{version}")
-        FileUtils.touch(File.join @lib, "v#{version}", 'fozzie.rb')
-      end
-      subject.versions(:fozzie).should == %w[ 1.2.1beta1 1.2.1rc1 1.2.1rc2 1.2.1 1.2.2 ]
-    end
-
-    it "should not return a version for an empty puppet/string/v{version}/{name}" do
-      FileUtils.mkdir_p(File.join @lib, 'v1.0.0', 'fozzie')
-      subject.versions(:fozzie).should == []
-    end
-
-    it "should an ordered list of all versions loadable as puppet/string/v{version}/{name}/*.rb" do
-      %w[ 1.2.1rc2 1.2.1beta1 1.2.1rc1 1.2.1 1.2.2 ].each do |version|
-        FileUtils.mkdir_p(File.join @lib, "v#{version}", "fozzie")
-        FileUtils.touch(File.join @lib, "v#{version}", 'fozzie', 'action.rb')
-      end
-      subject.versions(:fozzie).should == %w[ 1.2.1beta1 1.2.1rc1 1.2.1rc2 1.2.1 1.2.2 ]
-    end
-  end
-
   describe "::validate_version" do
     it 'should permit three number versions' do
       subject.validate_version('10.10.10').should == true
@@ -89,63 +44,6 @@ describe Puppet::String::StringCollection do
     end
   end
 
-  describe "::compare_versions" do
-    # (a <=> b) should be:
-    #   -1 if a < b
-    #   0  if a == b
-    #   1  if a > b
-    it 'should sort major version numbers numerically' do
-      subject.compare_versions('1.0.0', '2.0.0').should == -1
-      subject.compare_versions('2.0.0', '1.1.1').should == 1
-      subject.compare_versions('2.0.0', '10.0.0').should == -1
-    end
-
-    it 'should sort minor version numbers numerically' do
-      subject.compare_versions('0.1.0', '0.2.0').should == -1
-      subject.compare_versions('0.2.0', '0.1.1').should == 1
-      subject.compare_versions('0.2.0', '0.10.0').should == -1
-    end
-
-    it 'should sort tiny version numbers numerically' do
-      subject.compare_versions('0.0.1', '0.0.2').should == -1
-      subject.compare_versions('0.0.2', '0.0.1').should == 1
-      subject.compare_versions('0.0.2', '0.0.10').should == -1
-    end
-
-    it 'should sort major version before minor version' do
-      subject.compare_versions('1.1.0', '1.2.0').should == -1
-      subject.compare_versions('1.2.0', '1.1.1').should == 1
-      subject.compare_versions('1.2.0', '1.10.0').should == -1
-
-      subject.compare_versions('1.1.0', '2.2.0').should == -1
-      subject.compare_versions('2.2.0', '1.1.1').should == 1
-      subject.compare_versions('2.2.0', '1.10.0').should == 1
-    end
-
-    it 'should sort minor version before tiny version' do
-      subject.compare_versions('0.1.1', '0.1.2').should == -1
-      subject.compare_versions('0.1.2', '0.1.1').should == 1
-      subject.compare_versions('0.1.2', '0.1.10').should == -1
-
-      subject.compare_versions('0.1.1', '0.2.2').should == -1
-      subject.compare_versions('0.2.2', '0.1.1').should == 1
-      subject.compare_versions('0.2.2', '0.1.10').should == 1
-    end
-
-    it 'should sort appended strings asciibetically' do
-      subject.compare_versions('0.0.0a', '0.0.0b').should == -1
-      subject.compare_versions('0.0.0beta1', '0.0.0beta2').should == -1
-      subject.compare_versions('0.0.0beta1', '0.0.0rc1').should == -1
-      subject.compare_versions('0.0.0beta1', '0.0.0alpha1').should == 1
-      subject.compare_versions('0.0.0beta1', '0.0.0beta1').should == 0
-    end
-
-    it "should sort appended strings before 'whole' versions" do
-      subject.compare_versions('0.0.1a', '0.0.1').should == -1
-      subject.compare_versions('0.0.1', '0.0.1beta').should == 1
-    end
-  end
-
   describe "::[]" do
     before :each do
       subject.instance_variable_get("@strings")[:foo]['0.0.1'] = 10
@@ -167,17 +65,15 @@ describe Puppet::String::StringCollection do
     end
 
     it "should attempt to load the string if it isn't found" do
-      subject.expects(:require).with('puppet/string/v0.0.1/bar')
+      subject.expects(:require).with('puppet/string/bar')
+      subject.expects(:require).with('bar@0.0.1/puppet/string/bar')
       subject["bar", '0.0.1']
     end
 
-    it "should attempt to load the string with the greatest version for specified version :latest" do
-      %w[ 1.2.1 1.2.2 ].each do |version|
-        FileUtils.mkdir_p(File.join @lib, "v#{version}")
-        FileUtils.touch(File.join @lib, "v#{version}", 'fozzie.rb')
-      end
-      subject.expects(:require).with('puppet/string/v1.2.2/fozzie')
-      subject['fozzie', :latest]
+    it "should attempt to load the default string for the specified version :current" do
+      subject.expects(:require).never # except...
+      subject.expects(:require).with('puppet/string/fozzie')
+      subject['fozzie', :current]
     end
   end
 
@@ -191,14 +87,25 @@ describe Puppet::String::StringCollection do
     end
 
     it "should attempt to require the string if it is not registered" do
-      subject.expects(:require).with('puppet/string/v0.0.1/bar')
-      subject.string?("bar", '0.0.1')
+      subject.expects(:require).with do |file|
+        @strings[:bar]['0.0.1'] = true
+        file == 'puppet/string/bar'
+      end
+      subject.string?("bar", '0.0.1').should == true
     end
 
     it "should return true if requiring the string registered it" do
       subject.stubs(:require).with do
         subject.instance_variable_get("@strings")[:bar]['0.0.1'] = 20
       end
+    end
+
+    it "should require the string by version if the 'current' version isn't it" do
+      subject.expects(:require).with('puppet/string/bar').raises(LoadError)
+      subject.expects(:require).with do |file|
+        @strings[:bar]['0.0.1'] = true
+        file == 'bar@0.0.1/puppet/string/bar'
+      end
       subject.string?("bar", '0.0.1').should == true
     end
 
@@ -211,6 +118,34 @@ describe Puppet::String::StringCollection do
       subject.stubs(:require).raises(LoadError)
       subject.string?("bar", '0.0.1').should == false
     end
+
+    it "should register the version loaded by `:current` as `:current`" do
+      subject.expects(:require).with do |file|
+        @strings[:huzzah]['2.0.1'] = :huzzah_string
+        file == 'puppet/string/huzzah'
+      end
+      subject.string?("huzzah", :current)
+      @strings[:huzzah][:current].should == :huzzah_string
+    end
+
+    it "should register the version loaded from `puppet/string/{name}` as `:current`" do
+      subject.expects(:require).with do |file|
+        @strings[:huzzah]['2.0.1'] = :huzzah_string
+        file == 'puppet/string/huzzah'
+      end
+      subject.string?("huzzah", '2.0.1')
+      @strings[:huzzah][:current].should == :huzzah_string
+    end
+
+    it "should not register the version loaded from `{name}@{version}` as `:current`" do
+      subject.expects(:require).with('puppet/string/huzzah').raises(LoadError)
+      subject.expects(:require).with do |file|
+        @strings[:huzzah]['0.0.1'] = true
+        file == 'huzzah@0.0.1/puppet/string/huzzah'
+      end
+      subject.string?("huzzah", '0.0.1')
+      @strings[:huzzah].should_not have_key(:current)
+    end
   end
 
   describe "::register" do
diff --git a/spec/unit/string_spec.rb b/spec/unit/string_spec.rb
index 73d1f2177..64d4f12f8 100755
--- a/spec/unit/string_spec.rb
+++ b/spec/unit/string_spec.rb
@@ -75,7 +75,8 @@ describe Puppet::String do
   end
 
   it "should try to require strings that are not known" do
-    Puppet::String::StringCollection.expects(:require).with "puppet/string/v0.0.1/foo"
+    Puppet::String::StringCollection.expects(:require).with "puppet/string/foo"
+    Puppet::String::StringCollection.expects(:require).with "foo@0.0.1/puppet/string/foo"
     Puppet::String[:foo, '0.0.1']
   end