[#1621] Composite keys for resources

This patch implements the fundamental pieces of the move to composite
keys:
    * Instead of having a single namevar, we have a non-empty collection
      of them, and two resources are the same if and only if all of them
      match.  Note that the present situation is a special case of this,
      where the collection always has exactly one member.
    * As currently, namevar is determined by the type.
    * Instead just of inferring the single namevar from the title we let
      types decompose the title into values for several (perhaps all) of
      the namevar components; note that the present situation is again a
      special case of this.

Signed-off-by: Jesse Wolfe <jes5199@gmail.com>

9 files changed, 116 insertions, 184 deletions

diff --git a/lib/puppet/parameter.rb b/lib/puppet/parameter.rb
index b709ebd07..5642a75eb 100644
--- a/lib/puppet/parameter.rb
+++ b/lib/puppet/parameter.rb
@@ -84,13 +84,6 @@ class Puppet::Parameter
             define_method(:unmunge, &block)
         end
 
-        # Optionaly convert the value to a canonical form so that it will
-        # be found in hashes, etc.  Mostly useful for namevars.
-        def to_canonicalize(&block)
-            singleton_class = (class << self; self; end)
-            singleton_class.send(:define_method,:canonicalize,&block)
-        end
-
         # Mark whether we're the namevar.
         def isnamevar
             @isnamevar = true
@@ -253,19 +246,10 @@ class Puppet::Parameter
         value
     end
 
-    # Assume the value is already in canonical form by default
-    def self.canonicalize(value)
-        value
-    end
-
-    def canonicalize(value)
-        self.class.canonicalize(value)
-    end
-
     # A wrapper around our munging that makes sure we raise useful exceptions.
     def munge(value)
         begin
-            ret = unsafe_munge(canonicalize(value))
+            ret = unsafe_munge(value)
         rescue Puppet::Error => detail
             Puppet.debug "Reraising %s" % detail
             raise
diff --git a/lib/puppet/parser/compiler.rb b/lib/puppet/parser/compiler.rb
index beba438a9..f869383dc 100644
--- a/lib/puppet/parser/compiler.rb
+++ b/lib/puppet/parser/compiler.rb
@@ -446,7 +446,7 @@ class Puppet::Parser::Compiler
         @catalog.version = known_resource_types.version
 
         # Create our initial scope and a resource that will evaluate main.
-        @topscope = Puppet::Parser::Scope.new(:compiler => self)
+        @topscope = Puppet::Parser::Scope.new(:compiler => self, :source => 'implicit')
 
         @main_stage_resource = Puppet::Parser::Resource.new("stage", :main, :scope => @topscope)
         @catalog.add_resource(@main_stage_resource)
diff --git a/lib/puppet/parser/resource.rb b/lib/puppet/parser/resource.rb
index f221979df..fd2f49264 100644
--- a/lib/puppet/parser/resource.rb
+++ b/lib/puppet/parser/resource.rb
@@ -144,13 +144,8 @@ class Puppet::Parser::Resource < Puppet::Resource
         ! (catalog and version = catalog.client_version and version = version.split(".") and version[0] == "0" and version[1].to_i >= 25)
     end
 
-    # Return the resource name, or the title if no name
-    # was specified.
     def name
-        unless defined? @name
-            @name = self[:name] || self.title
-        end
-        @name
+        self[:name] || self.title
     end
 
     def namespaces
diff --git a/lib/puppet/resource.rb b/lib/puppet/resource.rb
index 34d609703..6ffaa2f3b 100644
--- a/lib/puppet/resource.rb
+++ b/lib/puppet/resource.rb
@@ -194,28 +194,6 @@ class Puppet::Resource
         @title = nil
     end
 
-    def old_title
-        if type == "Class" and value == ""
-            @title = :main
-            return
-        end
-
-        if klass = resource_type
-            p klass
-            if type == "Class"
-                value = munge_type_name(resource_type.name)
-            end
-
-            if klass.respond_to?(:canonicalize_ref)
-                value = klass.canonicalize_ref(value)
-            end
-        elsif type == "Class"
-            value = munge_type_name(value)
-        end
-
-        @title = value
-    end
-
     def resource_type
         case type
         when "Class"; find_hostclass(title)
@@ -227,17 +205,26 @@ class Puppet::Resource
 
     # Produce a simple hash of our parameters.
     def to_hash
-        result = @parameters.dup
-        unless result.include?(namevar)
-            result[namevar] = title
-        end
-        result
+        parse_title.merge @parameters
     end
 
     def to_s
         "#{type}[#{title}]"
     end
 
+    def uniqueness_key
+        h = {}
+        key_attributes.each do |attribute|
+            h[attribute] = self.to_hash[attribute]
+        end
+        return h
+    end
+
+    def key_attributes
+        return resource_type.key_attributes if resource_type.respond_to? :key_attributes
+        return [:name]
+    end
+
     # Convert our resource to Puppet code.
     def to_manifest
         "%s { '%s':\n%s\n}" % [self.type.to_s.downcase, self.title,
@@ -395,8 +382,8 @@ class Puppet::Resource
     # The namevar for our resource type. If the type doesn't exist,
     # always use :name.
     def namevar
-        if builtin_type? and t = resource_type
-            t.namevar
+        if builtin_type? and t = resource_type and t.key_attributes.length == 1
+            t.key_attributes.first
         else
             :name
         end
@@ -471,7 +458,7 @@ class Puppet::Resource
         when "Class";
             resolve_title_for_class(@unresolved_title)
         else
-            resolve_title_for_resource(@unresolved_title)
+            @unresolved_title
         end
     end
 
@@ -482,19 +469,26 @@ class Puppet::Resource
 
         if klass = find_hostclass(title)
             result = klass.name
-
-            if klass.respond_to?(:canonicalize_ref)
-                result = klass.canonicalize_ref(result)
-            end
         end
         return munge_type_name(result || title)
     end
 
-    def resolve_title_for_resource(title)
-        if type = find_resource_type(@type) and type.respond_to?(:canonicalize_ref)
-            return type.canonicalize_ref(title)
+    def parse_title
+        h = {}
+        type = find_resource_type(@type)
+        if type.respond_to? :title_patterns
+            type.title_patterns.each { |regexp, symbols_and_lambdas|
+                if captures = regexp.match(title.to_s)
+                    symbols_and_lambdas.zip(captures[1..-1]).each { |symbol_and_lambda,capture|
+                        sym, lam = symbol_and_lambda
+                        #self[sym] = lam.call(capture)
+                        h[sym] = lam.call(capture)
+                    }
+                    return h
+                end
+            }
         else
-            return title
+            return { :name => title.to_s }
         end
     end
 end
diff --git a/lib/puppet/resource/catalog.rb b/lib/puppet/resource/catalog.rb
index 519f31909..b00b6f498 100644
--- a/lib/puppet/resource/catalog.rb
+++ b/lib/puppet/resource/catalog.rb
@@ -56,6 +56,11 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph
         tag(*classes)
     end
 
+    def title_key_for_ref( ref )
+        ref =~ /^(.+)\[(.*)\]/
+        [$1, $2]
+    end
+
     # Add one or more resources to our graph and to our resource table.
     # This is actually a relatively complicated method, because it handles multiple
     # aspects of Catalog behaviour:
@@ -68,16 +73,16 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph
             unless resource.respond_to?(:ref)
                 raise ArgumentError, "Can only add objects that respond to :ref, not instances of %s" % resource.class
             end
-        end.each { |resource| fail_unless_unique(resource) }.each do |resource|
-            ref = resource.ref
+        end.each { |resource| fail_on_duplicate_type_and_title(resource) }.each do |resource|
+            title_key = title_key_for_ref(resource.ref)
 
             @transient_resources << resource if applying?
-            @resource_table[ref] = resource
+            @resource_table[title_key] = resource
 
             # If the name and title differ, set up an alias
 
             if resource.respond_to?(:name) and resource.respond_to?(:title) and resource.respond_to?(:isomorphic?) and resource.name != resource.title
-                self.alias(resource, resource.name) if resource.isomorphic?
+                self.alias(resource, resource.uniqueness_key) if resource.isomorphic?
             end
 
             resource.catalog = self if resource.respond_to?(:catalog=)
@@ -93,20 +98,24 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph
     end
 
     # Create an alias for a resource.
-    def alias(resource, name)
-        #set $1
+    def alias(resource, key)
         resource.ref =~ /^(.+)\[/
+        class_name = $1 || resource.class.name
 
-        newref = "%s[%s]" % [$1 || resource.class.name, name]
+        newref = [class_name, key]
+
+        if key.is_a? String
+            ref_string = "%s[%s]" % [class_name, key]
+            return if ref_string == resource.ref
+        end
 
         # LAK:NOTE It's important that we directly compare the references,
         # because sometimes an alias is created before the resource is
         # added to the catalog, so comparing inside the below if block
         # isn't sufficient.
-        return if newref == resource.ref
         if existing = @resource_table[newref]
             return if existing == resource
-            raise(ArgumentError, "Cannot alias %s to %s; resource %s already exists" % [resource.ref, name, newref])
+            raise(ArgumentError, "Cannot alias %s to %s; resource %s already exists" % [resource.ref, key.inspect, newref.inspect])
         end
         @resource_table[newref] = resource
         @aliases[resource.ref] ||= []
@@ -360,18 +369,23 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph
         # Always create a resource reference, so that it always canonizes how we
         # are referring to them.
         if title
-            ref = Puppet::Resource.new(type, title).to_s
+            res = Puppet::Resource.new(type, title)
         else
             # If they didn't provide a title, then we expect the first
             # argument to be of the form 'Class[name]', which our
             # Reference class canonizes for us.
-            ref = Puppet::Resource.new(nil, type).to_s
+            res = Puppet::Resource.new(nil, type)
         end
-        @resource_table[ref]
+        title_key      = [res.type, res.title.to_s]
+        uniqueness_key = [res.type, res.uniqueness_key]
+        @resource_table[title_key] || @resource_table[uniqueness_key]
     end
 
-    # Return an array of all resources.
-    def resources
+    def resource_refs
+        resource_keys.collect{ |type, name| name.is_a?( String ) ? "#{type}[#{name}]" : nil}.compact
+    end
+
+    def resource_keys
         @resource_table.keys
     end
 
@@ -495,15 +509,11 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph
     end
 
     # Verify that the given resource isn't defined elsewhere.
-    def fail_unless_unique(resource)
+    def fail_on_duplicate_type_and_title(resource)
         # Short-curcuit the common case,
-        return unless existing_resource = @resource_table[resource.ref]
+        return unless existing_resource = @resource_table[title_key_for_ref(resource.ref)]
 
         # If we've gotten this far, it's a real conflict
-
-        # Either it's a defined type, which are never
-        # isomorphic, or it's a non-isomorphic type, so
-        # we should throw an exception.
         msg = "Duplicate definition: %s is already defined" % resource.ref
 
         if existing_resource.file and existing_resource.line
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 340512a6d..dde342880 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -38,27 +38,11 @@ class Type
         properties()
     end
 
-    # All parameters, in the appropriate order.  The namevar comes first, then
+    # All parameters, in the appropriate order.  The key_attributes come first, then
     # the provider, then the properties, and finally the params and metaparams
     # in the order they were specified in the files.
     def self.allattrs
-        # Cache this, since it gets called multiple times
-        namevar = self.namevar
-
-        order = [namevar]
-        if self.parameters.include?(:provider)
-            order << :provider
-        end
-        order << [self.properties.collect { |property| property.name },
-            self.parameters - [:provider],
-            self.metaparams].flatten.reject { |param|
-                # we don't want our namevar in there multiple times
-                param == namevar
-            }
-
-        order.flatten!
-
-        return order
+        key_attributes | (parameters & [:provider]) | properties.collect { |property| property.name } | parameters | metaparams
     end
 
     # Retrieve an attribute alias, if there is one.
@@ -105,23 +89,6 @@ class Type
         @attrtypes[attr]
     end
 
-    # Copy an existing class parameter.  This allows other types to avoid
-    # duplicating a parameter definition, and is mostly used by subclasses
-    # of the File class.
-    def self.copyparam(klass, name)
-        param = klass.attrclass(name)
-
-        unless param
-            raise Puppet::DevError, "Class %s has no param %s" % [klass, name]
-        end
-        @parameters << param
-        @parameters.each { |p| @paramhash[name] = p }
-
-        if param.isnamevar?
-            @namevar = param.name
-        end
-    end
-
     def self.eachmetaparam
         @@metaparams.each { |p| yield p.name }
     end
@@ -208,29 +175,31 @@ class Type
         return param
     end
 
-    # Find the namevar
-    def self.namevar_parameter
-        @namevar_parameter ||= (
+    def self.key_attribute_parameters
+        @key_attribute_parameters ||= (
             params = @parameters.find_all { |param|
                 param.isnamevar? or param.name == :name
             }
-
-            if params.length > 1
-                raise Puppet::DevError, "Found multiple namevars for %s" % self.name
-            elsif params.length == 1
-                params.first
-            else
-                raise Puppet::DevError, "No namevar for %s" % self.name
-            end
         )
     end
 
-    def self.namevar
-        @namevar ||= namevar_parameter.name
+    def self.key_attributes
+        key_attribute_parameters.collect { |p| p.name }
     end
 
-    def self.canonicalize_ref(s)
-        namevar_parameter.canonicalize(s)
+    def self.title_patterns
+        case key_attributes.length
+        when 0; []
+        when 1;
+            identity = lambda {|x| x}
+            [ [ /(.*)/, [ [key_attributes.first, identity ] ] ] ]
+        else
+            raise Puppet::DevError,"you must specify title patterns when there are two or more key attributes"
+        end
+    end
+
+    def uniqueness_key
+        to_resource.uniqueness_key
     end
 
     # Create a new parameter.  Requires a block and a name, stores it in the
@@ -255,10 +224,6 @@ class Type
 
         param.isnamevar if options[:namevar]
 
-        if param.isnamevar?
-            @namevar = param.name
-        end
-
         return param
     end
 
@@ -417,6 +382,14 @@ class Type
         return false
     end
 
+    #
+    # The name_var is the key_attribute in the case that there is only one.
+    #
+    def name_var
+        key_attributes = self.class.key_attributes
+        (key_attributes.length == 1) && key_attributes.first
+    end
+
     # abstract accessing parameters and properties, and normalize
     # access to always be symbols, not strings
     # This returns a value, not an object.  It returns the 'is'
@@ -430,7 +403,7 @@ class Type
         end
 
         if name == :name
-            name = self.class.namevar
+            name = name_var
         end
 
         if obj = @parameters[name]
@@ -453,7 +426,7 @@ class Type
         end
 
         if name == :name
-            name = self.class.namevar
+            name = name_var
         end
         if value.nil?
             raise Puppet::Error.new("Got nil value for %s" % name)
@@ -980,31 +953,15 @@ class Type
         end.compact
     end
 
-    # Convert a simple hash into a Resource instance.  This is a convenience method,
-    # so people can create RAL resources with a hash and get the same behaviour
-    # as we get internally when we use Resource instances.
-    #   This should only be used directly from Ruby -- it's not used when going through
-    # normal Puppet usage.
+    # Convert a simple hash into a Resource instance.
     def self.hash2resource(hash)
         hash = hash.inject({}) { |result, ary| result[ary[0].to_sym] = ary[1]; result }
 
-        if title = hash[:title]
-            hash.delete(:title)
-        else
-            if self.namevar != :name
-                if hash.include?(:name) and hash.include?(self.namevar)
-                    raise Puppet::Error, "Cannot provide both name and %s to resources of type %s" % [self.namevar, self.name]
-                end
-                if title = hash[self.namevar]
-                    hash.delete(self.namevar)
-                end
-            end
-
-            unless title ||= hash[:name]
-                raise Puppet::Error, "You must specify a name or title for resources"
-            end
-        end
+        title = hash.delete(:title) 
+        title ||= hash[:name]
+        title ||= hash[key_attributes.first] if key_attributes.length == 1
 
+        raise Puppet::Error, "Title or name must be provided" unless title
 
         # Now create our resource.
         resource = Puppet::Resource.new(self.name, title)
@@ -1902,9 +1859,7 @@ class Type
 
     # Set our resource's name.
     def set_name(hash)
-        n = self.class.namevar
-        self[n] = hash[n]
-        hash.delete(n)
+        self[name_var] = hash.delete(name_var) if name_var
     end
 
     # Set all of the parameters from a hash, in the appropriate order.
@@ -2015,14 +1970,13 @@ class Type
     # then use the object's name.
     def title
         unless defined? @title and @title
-            namevar = self.class.namevar
-            if self.class.validparameter?(namevar)
+            if self.class.validparameter?(name_var)
                 @title = self[:name]
-            elsif self.class.validproperty?(namevar)
-                @title = self.should(namevar)
+            elsif self.class.validproperty?(name_var)
+                @title = self.should(name_var)
             else
                 self.devfail "Could not find namevar %s for %s" %
-                    [namevar, self.class.name]
+                    [name_var, self.class.name]
             end
         end
 
@@ -2040,11 +1994,14 @@ class Type
 
         values = retrieve()
         values.each do |name, value|
-            trans[name.name] = value
+            # sometimes we get symbols and sometimes we get Properties
+            # I think it's a bug, but I can't find it. ~JW
+            name = name.name if name.respond_to? :name
+            trans[name] = value
         end
 
         @parameters.each do |name, param|
-            # Avoid adding each instance name as both the name and the namevar
+            # Avoid adding each instance name twice
             next if param.class.isnamevar? and param.value == self.title
 
             # We've already got property values
diff --git a/lib/puppet/type/file.rb b/lib/puppet/type/file.rb
index 04a676589..ec7a3faaf 100644
--- a/lib/puppet/type/file.rb
+++ b/lib/puppet/type/file.rb
@@ -24,6 +24,10 @@ Puppet::Type.newtype(:file) do
         Puppet Labs and we can hopefully work with you to develop a
         native resource to support what you are doing."
 
+    def self.title_patterns
+        [ [ /^(.*?)\/?$/, [ [ :path, lambda{|x| x} ] ] ] ]
+    end
+
     newparam(:path) do
         desc "The path to the file to manage.  Must be fully qualified."
         isnamevar
@@ -38,7 +42,7 @@ Puppet::Type.newtype(:file) do
         # convert the current path in an index into the collection and the last
         # path name. The aim is to use less storage for all common paths in a hierarchy
         munge do |value|
-            path, name = File.split(value)
+            path, name = File.split(value.gsub(/\/+/,'/'))
             { :index => Puppet::FileCollection.collection.index(path), :name => name }
         end
 
@@ -52,16 +56,6 @@ Puppet::Type.newtype(:file) do
                 File.join( basedir, value[:name] )
             end
         end
-
-        to_canonicalize do |s|
-            # * if it looks like a windows path, replace all backslashes with forward slashes
-            # * get rid of any duplicate slashes
-            # * remove any trailing slashes unless the title is just a slash, or a
-            #   drive letter in which case leave it
-            # * UNCs in the form //server//share/... keep their initial double slash.
-            s = s.gsub(/\\/, '/') if s =~ /^.:\/\\/ or s =~ /^\/\/[^\/]+\/[^\/]+/
-            s.gsub(/(.)\/+/, '\1/').sub(/([^:])\/$/,'\1')
-        end
     end
 
     newparam(:backup) do
@@ -425,7 +419,6 @@ Puppet::Type.newtype(:file) do
             end
         end
 
-        @title = self.class.canonicalize_ref(@title)
         @stat = nil
     end
 
diff --git a/lib/puppet/type/resources.rb b/lib/puppet/type/resources.rb
index e1de55711..f5cf5a41c 100644
--- a/lib/puppet/type/resources.rb
+++ b/lib/puppet/type/resources.rb
@@ -99,7 +99,7 @@ Puppet::Type.newtype(:resources) do
     def generate
         return [] unless self.purge?
         resource_type.instances.
-            reject { |r| catalog.resources.include? r.ref }.
+            reject { |r| catalog.resource_refs.include? r.ref }.
             select { |r| check(r) }.
             select { |r| r.class.validproperty?(:ensure) }.
             select { |r| able_to_ensure_absent?(r) }.
diff --git a/lib/puppet/util/settings.rb b/lib/puppet/util/settings.rb
index b2d1f4a1f..11d760f1a 100644
--- a/lib/puppet/util/settings.rb
+++ b/lib/puppet/util/settings.rb
@@ -617,8 +617,7 @@ Generated on #{Time.now}.
     # Convert to a parseable manifest
     def to_manifest
         catalog = to_catalog
-        # The resource list is a list of references, not actual instances.
-        catalog.resources.collect do |ref|
+        catalog.resource_refs.collect do |ref|
             catalog.resource(ref).to_manifest
         end.join("\n\n")
     end