Encode default-ness in the registry path

Previously, there was a separate parameter for specifying whether a
registry value is the unnamed (aka default). However, the following
example resulted in non unique paths:

  registry_value { 'hklm\foo':
    isdefault => true
  }
  registry_value { 'hklm\foo':
    isdefault => false
  }

The first value is managing the default value for the `hklm\foo` key. The
second value is managing the value named `foo` for the `hklm` key.

This commit eliminates this issue and eliminates the need for an isdefault
parameter. The above examples become:

  registry_value { 'hklm\foo\\':
  }
  registry_value { 'hklm\foo':
  }

Note, the trailing backslash must be escaped, since it is immediately
followed by a single quote.

lib/puppet/type/registry_value.rb

@@ -7,23 +7,22 @@ def self.title_patterns
     [ [ /^(.*?)\Z/m, [ [ :path, lambda{|x| x} ] ] ] ]
   end
 
-  attr_reader :hkey, :subkey, :valuename
+  attr_accessor :hkey, :subkey, :valuename
 
   ensurable
 
   newparam(:path, :namevar => true) do
+    validate do |path|
+      # really we should have a RegistryPath parameter, with hkey, etc readers
+      resource.hkey, resource.subkey, resource.valuename = resource.value_split(path.to_s)
+    end
   end
 
   newparam(:redirect) do
     newvalues(:true, :false)
     defaultto :false
   end
 
-  newparam(:isdefault) do
-    newvalues(:true, :false)
-    defaultto :false
-  end
-
   newproperty(:type) do
     newvalues(:string, :array, :dword, :qword, :binary, :expand)
     defaultto :string
@@ -49,10 +48,6 @@ def self.title_patterns
     defaultto ''
   end
 
-  validate do
-    @hkey, @subkey, @valuename = value_split(self[:path], self[:isdefault])
-  end
-
   autorequire(:registry_key) do
     parents = []
     ascend(hkey, subkey) do |h, s|

lib/puppet/util/registry_base.rb

@@ -62,12 +62,19 @@ def key_split(path)
           raise ArgumentError, "Unsupported prefined key: #{path}"
         end
 
+    # leading backslash is not part of the subkey name
+    subkey = subkey[1..-1] unless subkey.empty?
+
     [hkey, subkey]
   end
 
-  def value_split(path, isdefault = nil)
-    if isdefault == :true
-      hkey, subkey = key_split(path)
+  def value_split(path)
+    unless path
+      raise ArgumentError, "Invalid registry value"
+    end
+
+    if path[-1, 1] == '\\' # trailing backslash implies default value
+      hkey, subkey = key_split(path.gsub(/\\*$/, ''))
       value = ''
     else
       idx = path.rindex('\\')

spec/unit/puppet/type/registry_value_spec.rb

@@ -3,9 +3,6 @@
 require 'puppet/util/registry_base'
 
 describe Puppet::Type.type(:registry_value) do
-  let (:path) { 'HKLM\Software\PuppetSpecTest\ValueName' }
-  let (:value) { Puppet::Type.type(:registry_value).new(:path => path) }
-
   [:ensure, :type, :data].each do |property|
     it "should have a #{property} property" do
       described_class.attrclass(property).ancestors.should be_include(Puppet::Property)
@@ -21,33 +18,46 @@
       Puppet::Type.type(:registry_key).attrtype(:path).should == :param
     end
 
-    it 'should accept a fully qualified path' do
-      value[:path].should == path
+    %w[hklm\propname hklm\software\propname].each do |path|
+      it "should accept #{path}" do
+        described_class.new(:path => path)
+      end
     end
 
-    Puppet::Util::RegistryBase::HKEYS.each do |hkey|
-      it "should accept #{hkey}\Subkey\value" do
-        Puppet::Type.type(:registry_key).new(:path => "#{hkey}\\Subkey\\value")
+    %w[hklm\\ hklm\software\\ hklm\software\vendor\\].each do |path|
+      it "should accept the unnamed (default) value: #{path}" do
+        described_class.new(:path => path)
       end
     end
 
-    it 'should reject a root key' do
-      pending("Should it allow default value of root key")
-      expect { value[:path] = 'HKLM' }.should raise_error(Puppet::Error)
+    it "should strip trailling slashes from unnamed values" do
+      described_class.new(:path => 'hklm\\software\\\\')
     end
 
-    it 'should reject unknown root keys' do
-      pending("This is not working")
-      expect { value[:path] = 'UNKNOWN\Bar\Baz' }.should raise_error(Puppet::Error)
+    %w[unknown\name unknown\subkey\name HKEY_PERFORMANCE_DATA\name].each do |path|
+      it "should reject #{path} as unsupported" do
+        expect { described_class.new(:path => path) }.to raise_error(Puppet::Error, /Unsupported/)
+      end
     end
 
+    %[hklm hkcr].each do |path|
+      it "should reject #{path} as invalid" do
+        pending 'wrong message'
+        expect { described_class.new(:path => path) }.should raise_error(Puppet::Error, /Invalid registry key/)
+      end
+    end
+
+    it 'should validate the length of the value name'
+    it 'should validate the length of the value data'
     it 'should canonicalize the root key'
     it 'should be case-preserving'
     it 'should be case-insensitive'
     it 'should autorequire ancestor keys'
   end
 
   describe "redirect parameter" do
+    let (:value) { described_class.new(:path => 'hklm\software\foo') }
+
     it 'should not redirect by default' do
       value[:redirect].should == :false
     end
@@ -58,18 +68,9 @@
     end
   end
 
-  describe "isdefault parameter" do
-    it 'should not refer to the "default" value by default' do
-      value[:isdefault].should == :false
-    end
-
-    it 'should allow default' do
-      value[:isdefault] = true
-      value[:isdefault].should be_true
-    end
-  end
-
   describe "type property" do
+    let (:value) { described_class.new(:path => 'hklm\software\foo') }
+
     [:string, :array, :dword, :qword, :binary, :expand].each do |type|
       it "should support a #{type.to_s} type" do
         value[:type] = type
@@ -83,6 +84,8 @@
   end
 
   describe "data property" do
+    let (:value) { described_class.new(:path => 'hklm\software\foo') }
+
     it "should support string data" do
       value[:type] = :string
       value[:data] = 'foobar'