Refactor key and value paths

Previously, the component parts of the key and value types were stored as
instance variables of the resource, even though they were derived from the
path parameter.

This commit introduces a KeyPath parameter which knows how to validate
registry key paths. It also contains the instance data associated with the
root key and subkey. The ValuePath parameter extends KeyPath and knows
about validating registry value paths, including how to handle default
values.

lib/puppet/provider/registry_key/registry.rb

@@ -18,28 +18,25 @@ def self.instances
 
   def create
     Puppet.debug("create key #{resource[:path]}")
-    hkey, subkey = key
-    hkey.create(subkey, access(Win32::Registry::KEY_ALL_ACCESS)) {|reg| true }
+    keypath.hkey.create(keypath.subkey, access(Win32::Registry::KEY_ALL_ACCESS)) {|reg| true }
   end
 
   def exists?
     Puppet.debug("exists? key #{resource[:path]}")
-    hkey, subkey = key
-    !!hkey.open(subkey, access(Win32::Registry::KEY_READ)) {|reg| true } rescue false
+    !!keypath.hkey.open(keypath.subkey, access(Win32::Registry::KEY_READ)) {|reg| true } rescue false
   end
 
   def destroy
     Puppet.debug("destroy key #{resource[:path]}")
-    hkey, subkey = key
 
-    raise "Cannot delete root key: #{resource[:path]}" unless subkey
+    raise "Cannot delete root key: #{resource[:path]}" unless keypath.subkey
 
-    if RegDeleteKeyEx.call(hkey.hkey, subkey, access, 0) != 0
+    if RegDeleteKeyEx.call(keypath.hkey.hkey, keypath.subkey, access, 0) != 0
       raise "Failed to delete registry key: #{resource[:path]}"
     end
   end
 
-  def key
-    key_split(resource[:path])
+  def keypath
+    @keypath ||= resource.parameter(:path)
   end
 end

lib/puppet/provider/registry_value/registry.rb

@@ -14,38 +14,38 @@ def self.instances
   def create
     Puppet.info("creating: #{self}")
 
-    resource.hkey.open(resource.subkey, access(Win32::Registry::KEY_ALL_ACCESS)) do |reg|
-      reg.write(resource.valuename, name2type(resource[:type]), resource[:data])
+    valuepath.hkey.open(valuepath.subkey, access(Win32::Registry::KEY_ALL_ACCESS)) do |reg|
+      reg.write(valuepath.valuename, name2type(resource[:type]), resource[:data])
     end
   end
 
   def exists?
     Puppet.info("exists: #{self}")
 
     found = false
-    resource.hkey.open(resource.subkey, access(Win32::Registry::KEY_READ)) do |reg|
+    valuepath.hkey.open(valuepath.subkey, access(Win32::Registry::KEY_READ)) do |reg|
       type = [0].pack('L')
       size = [0].pack('L')
-      found = RegQueryValueExA.call(reg.hkey, resource.valuename, 0, type, 0, size) == 0
+      found = RegQueryValueExA.call(reg.hkey, valuepath.valuename, 0, type, 0, size) == 0
     end
     found
   end
 
   def flush
-    Puppet.info("flushing: #{self}")
+    return if resource[:ensure] == :absent
 
-    # REMIND: not during destroy
+    Puppet.info("flushing: #{self}")
 
-    resource.hkey.open(resource.subkey, access(Win32::Registry::KEY_ALL_ACCESS)) do |reg|
-      reg.write(resource.valuename, name2type(regvalue[:type]), regvalue[:data])
+    valuepath.hkey.open(valuepath.subkey, access(Win32::Registry::KEY_ALL_ACCESS)) do |reg|
+      reg.write(valuepath.valuename, name2type(regvalue[:type]), regvalue[:data])
     end
   end
 
   def destroy
     Puppet.info("destroying: #{self}")
 
-    resource.hkey.open(resource.subkey, access(Win32::Registry::KEY_ALL_ACCESS)) do |reg|
-      reg.delete_value(resource.valuename)
+    valuepath.hkey.open(valuepath.subkey, access(Win32::Registry::KEY_ALL_ACCESS)) do |reg|
+      reg.delete_value(valuepath.valuename)
     end
   end
 
@@ -68,20 +68,24 @@ def data=(value)
   def regvalue
     unless @regvalue
       @regvalue = {}
-      resource.hkey.open(resource.subkey, access(Win32::Registry::KEY_ALL_ACCESS)) do |reg|
+      valuepath.hkey.open(valuepath.subkey, access(Win32::Registry::KEY_ALL_ACCESS)) do |reg|
         type = [0].pack('L')
         size = [0].pack('L')
 
-        if RegQueryValueExA.call(reg.hkey, resource.valuename, 0, type, 0, size) == 0
-          is_type, is_data = reg.read(resource.valuename)
+        if RegQueryValueExA.call(reg.hkey, valuepath.valuename, 0, type, 0, size) == 0
+          is_type, is_data = reg.read(valuepath.valuename)
           @regvalue[:type], @regvalue[:data] = type2name(is_type), is_data
         end
       end
     end
     @regvalue
   end
 
+  def valuepath
+    @valuepath ||= resource.parameter(:path)
+  end
+
   def to_s
-    "#{resource.hkey.keyname}\\#{resource.subkey}\\#{resource.valuename}"
+    "#{valuepath.hkey.keyname}\\#{valuepath.subkey}\\#{valuepath.valuename}"
   end
 end

lib/puppet/type/registry_key.rb

@@ -1,4 +1,5 @@
 require 'puppet/util/registry_base'
+require 'puppet/util/key_path'
 
 Puppet::Type.newtype(:registry_key) do
   include Puppet::Util::RegistryBase
@@ -9,10 +10,7 @@ def self.title_patterns
 
   ensurable
 
-  newparam(:path, :namevar => true) do
-    validate do |value|
-      resource.key_split(value.to_s)
-    end
+  newparam(:path, :parent => Puppet::Util::KeyPath, :namevar => true) do
   end
 
   newparam(:redirect) do
@@ -21,14 +19,11 @@ def self.title_patterns
   end
 
   autorequire(:registry_key) do
-    hkey, subkey = key_split(self[:path])
-
     parents = []
-    ascend(hkey, subkey) do |h, s|
-      # skip ourselves
-      parents << "#{h.keyname}\\#{s}" unless s == subkey
+    path = parameter(:path)
+    path.ascend do |hkey, subkey|
+      parents << "#{hkey.keyname}\\#{subkey}" unless subkey == path.subkey # skip ourselves
     end
-
     parents
   end
 end

lib/puppet/type/registry_value.rb

@@ -1,4 +1,5 @@
 require 'puppet/util/registry_base'
+require 'puppet/util/value_path'
 
 Puppet::Type.newtype(:registry_value) do
   include Puppet::Util::RegistryBase
@@ -7,15 +8,9 @@ def self.title_patterns
     [ [ /^(.*?)\Z/m, [ [ :path, lambda{|x| x} ] ] ] ]
   end
 
-  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
+  newparam(:path, :parent => Puppet::Util::ValuePath, :namevar => true) do
   end
 
   newparam(:redirect) do
@@ -50,8 +45,8 @@ def self.title_patterns
 
   autorequire(:registry_key) do
     parents = []
-    ascend(hkey, subkey) do |h, s|
-      parents << "#{h}\\#{s}"
+    parameter(:path).ascend do |hkey, subkey|
+      parents << "#{hkey.keyname}\\#{subkey}"
     end
     parents
   end

lib/puppet/util/key_path.rb

@@ -0,0 +1,43 @@
+require 'puppet/parameter'
+require 'puppet/util/registry_base'
+
+class Puppet::Util::KeyPath < Puppet::Parameter
+  include Puppet::Util::RegistryBase
+
+  attr_reader :hkey, :subkey
+
+  def validate(path)
+    split(path.to_s)
+  end
+
+  def split(path)
+    unless match = /^([^\\]*)((?:\\[^\\]{1,255})*)$/.match(path)
+      raise ArgumentError, "Invalid registry key: #{path}"
+    end
+
+    @hkey =
+        case match[1].downcase
+        when /hkey_local_machine/, /hklm/
+          HKEYS[:hklm]
+        when /hkey_classes_root/, /hkcr/
+          HKEYS[:hkcr]
+        else
+          raise ArgumentError, "Unsupported prefined key: #{path}"
+        end
+
+    # leading backslash is not part of the subkey name
+    @subkey = match[2]
+    @subkey = @subkey[1..-1] unless @subkey.empty?
+  end
+
+  def ascend(&block)
+    s = subkey
+
+    yield hkey, s
+
+    while idx = s.rindex('\\')
+      s = s[0, idx]
+      yield hkey, s
+    end
+  end
+end

lib/puppet/util/registry_base.rb

@@ -37,55 +37,6 @@ def hkeys
     HKEYS
   end
 
-  def ascend(hkey, subkey, &block)
-    yield hkey, subkey
-
-    while idx = subkey.rindex('\\')
-      subkey = subkey[0, idx]
-      yield hkey, subkey
-    end
-  end
-
-  def key_split(path)
-    unless match = /^([^\\]*)((?:\\[^\\]{1,255})*)$/.match(path)
-      raise ArgumentError, "Invalid registry key: #{path}"
-    end
-
-    rootkey, subkey = match.captures
-    hkey =
-        case rootkey.downcase
-        when /hkey_local_machine/, /hklm/
-          HKEYS[:hklm]
-        when /hkey_classes_root/, /hkcr/
-          HKEYS[:hkcr]
-        else
-          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)
-    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('\\')
-      raise ArgumentError, "Registry value path must contain at least one backslash." unless idx
-
-      hkey, subkey = key_split(path[0, idx])
-      value = path[idx+1..-1] if idx > 0
-    end
-    [hkey, subkey, value]
-  end
-
   def access(mask = 0)
     # REMIND: skip this if 32-bit OS?
     #:redirect) == :true ? 0x200 : 0x100)

lib/puppet/util/value_path.rb

@@ -0,0 +1,23 @@
+require 'puppet/parameter'
+require 'puppet/util/registry_base'
+
+class Puppet::Util::ValuePath < Puppet::Parameter::KeyPath
+  attr_reader :valuename
+
+  def split(path)
+    unless path
+      raise ArgumentError, "Invalid registry value"
+    end
+
+    if path[-1, 1] == '\\' # trailing backslash implies default value
+      super(path.gsub(/\\*$/, ''))
+      @valuename = ''
+    else
+      idx = path.rindex('\\')
+      raise ArgumentError, "Registry value path must contain at least one backslash." unless idx
+
+      super(path[0, idx])
+      @valuename = path[idx+1..-1] if idx > 0
+    end
+  end
+end