diff options
| -rw-r--r-- | lib/puppet/string/string_collection.rb | 86 | ||||
| -rwxr-xr-x | spec/unit/string/string_collection_spec.rb | 54 | ||||
| -rwxr-xr-x | spec/unit/string_spec.rb | 1 |
3 files changed, 89 insertions, 52 deletions
diff --git a/lib/puppet/string/string_collection.rb b/lib/puppet/string/string_collection.rb index f8fa38b9c..ecd99359d 100644 --- a/lib/puppet/string/string_collection.rb +++ b/lib/puppet/string/string_collection.rb @@ -1,3 +1,4 @@ +# -*- coding: utf-8 -*- require 'puppet/string' module Puppet::String::StringCollection @@ -30,37 +31,82 @@ module Puppet::String::StringCollection !!(SEMVER_VERSION =~ version.to_s) end + def self.cmp_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) @strings[underscorize(name)][version] if string?(name, version) end def self.string?(name, version) name = underscorize(name) - cache = @strings[name] - return true if cache.has_key?(version) - - loaded = cache.keys + return true if @strings[name].has_key?(version) - module_names = ["puppet/string/#{name}"] - unless version == :current - module_names << "#{name}@#{version}/puppet/string/#{name}" - end + # We always load the current version file; the common case is that we have + # the expected version and any compatibility versions in the same file, + # the default. Which means that this is almost always the case. + # + # We use require to avoid executing the code multiple times, like any + # other Ruby library that we might want to use. --daniel 2011-04-06 + begin + require "puppet/string/#{name}" - module_names.each do |module_name| - begin - require module_name - if version == :current || !module_name.include?('@') - loaded = (cache.keys - loaded).first - cache[:current] = cache[loaded] unless loaded.nil? - end - return true if cache.has_key?(version) - rescue LoadError => e - raise unless e.message =~ /-- #{module_name}$/ - # pass + # If we wanted :current, we need to index to find that; direct version + # requests just work™ as they go. --daniel 2011-04-06 + if version == :current then + # We need to find current out of this. This is the largest version + # number that doesn't have a dedicated on-disk file present; those + # represent "experimental" versions of strings, which we don't fully + # support yet. + # + # We walk the versions from highest to lowest and take the first version + # that is not defined in an explicitly versioned file on disk as the + # current version. + # + # This constrains us to only ship experimental versions with *one* + # version in the file, not multiple, but given you can't reliably load + # them except by side-effect when you ignore that rule this seems safe + # enough... + # + # Given those constraints, and that we are not going to ship a versioned + # interface that is not :current in this release, we are going to leave + # these thoughts in place, and just punt on the actual versioning. + # + # When we upgrade the core to support multiple versions we can solve the + # problems then; as lazy as possible. + # + # We do support multiple versions in the same file, though, so we sort + # versions here and return the last item in that set. + # + # --daniel 2011-04-06 + latest_ver = @strings[name].keys.sort {|a, b| cmp_versions(a, b) }.last + @strings[name][:current] = @strings[name][latest_ver] end + rescue LoadError => e + raise unless e.message =~ %r{-- puppet/string/#{name}$} + # ...guess we didn't find the file; return a much better problem. end - return false + # Now, either we have the version in our set of strings, or we didn't find + # the version they were looking for. In the future we will support + # loading versioned stuff from some look-aside part of the Ruby load path, + # but we don't need that right now. + # + # So, this comment is a place-holder for that. --daniel 2011-04-06 + return !! @strings[name].has_key?(version) end def self.register(string) diff --git a/spec/unit/string/string_collection_spec.rb b/spec/unit/string/string_collection_spec.rb index fa63f18e3..fab647da0 100755 --- a/spec/unit/string/string_collection_spec.rb +++ b/spec/unit/string/string_collection_spec.rb @@ -70,7 +70,6 @@ describe Puppet::String::StringCollection do it "should attempt to load the string if it isn't found" do subject.expects(:require).with('puppet/string/bar') - subject.expects(:require).with('bar@0.0.1/puppet/string/bar') subject["bar", '0.0.1'] end @@ -104,26 +103,15 @@ describe Puppet::String::StringCollection do 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, 'no such file to load -- puppet/string/bar') - subject.expects(:require).with do |file| - subject.instance_variable_get("@strings")[:bar]['0.0.1'] = true - file == 'bar@0.0.1/puppet/string/bar' - end - subject.string?("bar", '0.0.1').should == true - end - it "should return false if the string is not registered" do subject.stubs(:require).returns(true) - subject.string?("bar", '0.0.1').should == false + subject.string?("bar", '0.0.1').should be_false end - it "should return false if there is a LoadError requiring the string" do + it "should return false if the string file itself is missing" do subject.stubs(:require). - raises(LoadError, 'no such file to load -- puppet/string/bar').then. - raises(LoadError, 'no such file to load -- bar@0.0.1/puppet/string/bar') - subject.string?("bar", '0.0.1').should == false + raises(LoadError, 'no such file to load -- puppet/string/bar') + subject.string?("bar", '0.0.1').should be_false end it "should register the version loaded by `:current` as `:current`" do @@ -135,24 +123,28 @@ describe Puppet::String::StringCollection do subject.instance_variable_get("@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| - subject.instance_variable_get("@strings")[:huzzah]['2.0.1'] = :huzzah_string - file == 'puppet/string/huzzah' + context "with something on disk" do + before :each do + write_scratch_string :huzzah do |fh| + fh.puts <<EOF +Puppet::String.define(:huzzah, '2.0.1') do + action :bar do "is where beer comes from" end +end +EOF + end end - subject.string?("huzzah", '2.0.1') - subject.instance_variable_get("@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, 'no such file to load -- puppet/string/huzzah') - subject.expects(:require).with do |file| - subject.instance_variable_get("@strings")[:huzzah]['0.0.1'] = true - file == 'huzzah@0.0.1/puppet/string/huzzah' + it "should register the version loaded from `puppet/string/{name}` as `:current`" do + subject.should be_string "huzzah", '2.0.1' + subject.should be_string "huzzah", :current + Puppet::String[:huzzah, '2.0.1'].should == Puppet::String[:huzzah, :current] + end + + it "should index :current when the code was pre-required" do + subject.instance_variable_get("@strings")[:huzzah].should_not be_key :current + require 'puppet/string/huzzah' + subject.string?(:huzzah, :current).should be_true end - subject.string?("huzzah", '0.0.1') - subject.instance_variable_get("@strings")[:huzzah].should_not have_key(:current) end end diff --git a/spec/unit/string_spec.rb b/spec/unit/string_spec.rb index 358668f9b..9b7cd887c 100755 --- a/spec/unit/string_spec.rb +++ b/spec/unit/string_spec.rb @@ -76,7 +76,6 @@ describe Puppet::String do it "should try to require strings that are not known" do 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 |