Switched all value management in props/params to internal classes.

This is a significant refactor of some very murky code, and it's
all much cleaner and more readable now.  All of the 'newvalue' methods
and any value-related code is in a ValueCollection class.

This puts us in a good position to refactor the Property and Parameter
classes more completely.

Signed-off-by: Luke Kanies <luke@madstop.com>

8 files changed, 928 insertions, 889 deletions

diff --git a/lib/puppet/parameter.rb b/lib/puppet/parameter.rb
index 06dfe5b91..40bb32ff6 100644
--- a/lib/puppet/parameter.rb
+++ b/lib/puppet/parameter.rb
@@ -9,10 +9,222 @@ class Puppet::Parameter
     include Puppet::Util::LogPaths
     include Puppet::Util::Logging
     include Puppet::Util::MethodHelper
+
+    # A collection of values and regexes, used for specifying
+    # what values are allowed in a given parameter.
+    class ValueCollection
+        class Value
+            attr_reader :name, :options, :event
+            attr_accessor :block, :call, :method, :required_features
+
+            # Add an alias for this value.
+            def alias(name)
+                @aliases << convert(name)
+            end
+
+            # Return all aliases.
+            def aliases
+                @aliases.dup
+            end
+
+            # Store the event that our value generates, if it does so.
+            def event=(value)
+                @event = convert(value)
+            end
+
+            def initialize(name)
+                if name.is_a?(Regexp)
+                    @name = name
+                else
+                    # Convert to a string and then a symbol, so things like true/false
+                    # still show up as symbols.
+                    @name = convert(name)
+                end
+
+                @aliases = []
+
+                @call = :instead
+            end
+
+            # Does a provided value match our value?
+            def match?(value)
+                if regex?
+                    return true if name =~ value.to_s
+                else
+                    return true if name == convert(value)
+                    return @aliases.include?(convert(value))
+                end
+            end
+
+            # Is our value a regex?
+            def regex?
+                @name.is_a?(Regexp)
+            end
+
+            private
+            
+            # A standard way of converting all of our values, so we're always
+            # comparing apples to apples.
+            def convert(value)
+                if value == ''
+                    # We can't intern an empty string, yay.
+                    value
+                else
+                    value.to_s.to_sym
+                end
+            end
+        end
+
+        def aliasvalue(name, other)
+            other = other.to_sym
+            unless value = match?(other)
+                raise Puppet::DevError, "Cannot alias nonexistent value %s" % other
+            end
+
+            value.alias(name)
+        end
+
+        # Return a doc string for all of the values in this parameter/property.
+        def doc
+            unless defined?(@doc)
+                @doc = ""
+                unless values.empty?
+                    @doc += "  Valid values are "
+                    @doc += @strings.collect do |value|
+                        if aliases = value.aliases and ! aliases.empty?
+                            "``%s`` (also called ``%s``)" % [value.name, aliases.join(", ")]
+                        else
+                            "``%s``" % value.name
+                        end
+                    end.join(", ") + "."
+                end
+
+                unless regexes.empty?
+                    @doc += "  Values can match ``" + regexes.join("``, ``") + "``."
+                end
+            end
+
+            @doc
+        end
+
+        # Does this collection contain any value definitions?
+        def empty?
+            @values.empty?
+        end
+
+        def initialize
+            # We often look values up by name, so a hash makes more sense.
+            @values = {}
+
+            # However, we want to retain the ability to match values in order,
+            # but we always prefer directly equality (i.e., strings) over regex matches.
+            @regexes = []
+            @strings = []
+        end
+
+        # Can we match a given value?
+        def match?(test_value)
+            # First look for normal values
+            if value = @strings.find { |v| v.match?(test_value) }
+                return value
+            end
+            
+            # Then look for a regex match
+            @regexes.find { |v| v.match?(test_value) }
+        end
+
+        # If the specified value is allowed, then munge appropriately.
+        def munge(value)
+            return value if empty?
+
+            if instance = match?(value)
+                if instance.regex?
+                    return value
+                else
+                    return instance.name
+                end
+            else
+                return value
+            end
+        end
+
+        # Define a new valid value for a property.  You must provide the value itself,
+        # usually as a symbol, or a regex to match the value.
+        #
+        # The first argument to the method is either the value itself or a regex.
+        # The second argument is an option hash; valid options are:
+        # * <tt>:event</tt>: The event that should be returned when this value is set.
+        # * <tt>:call</tt>: When to call any associated block.  The default value
+        #   is ``instead``, which means to call the value instead of calling the
+        #   provider.  You can also specify ``before`` or ``after``, which will 
+        #   call both the block and the provider, according to the order you specify
+        #   (the ``first`` refers to when the block is called, not the provider).
+        def newvalue(name, options = {}, &block)
+            value = Value.new(name)
+            @values[value.name] = value
+            if value.regex?
+                @regexes << value
+            else
+                @strings << value
+            end
+
+            options.each { |opt, arg| value.send(opt.to_s + "=", arg) }
+            if block_given?
+                value.block = block
+            else
+                value.call = options[:call] || :none
+            end
+
+            if block_given? and ! value.regex?
+                value.method ||= "set_" + value.name.to_s
+            end
+
+            value
+        end
+
+        # Define one or more new values for our parameter.
+        def newvalues(*names)
+            names.each { |name| newvalue(name) }
+        end
+
+        def regexes
+            @regexes.collect { |r| r.name.inspect }
+        end
+
+        # Verify that the passed value is valid.
+        def validate(value)
+            return if empty?
+
+            unless @values.detect { |name, v| v.match?(value) }
+                str = "Invalid value %s. " % [value.inspect]
+
+                unless values.empty?
+                    str += "Valid values are %s. " % values.join(", ")
+                end
+
+                unless regexes.empty?
+                    str += "Valid values match %s." % regexes.join(", ")
+                end
+
+                raise ArgumentError, str
+            end
+        end
+
+        # Return a single value instance.
+        def value(name)
+            @values[name]
+        end
+
+        # Return the list of valid values.
+        def values
+            @strings.collect { |s| s.name }
+        end
+    end
+
     class << self
         include Puppet::Util
         include Puppet::Util::Docs
-        attr_reader :validater, :munger, :name, :default, :required_features
+        attr_reader :validater, :munger, :name, :default, :required_features, :value_collection
         attr_accessor :metaparam
 
         # Define the default value for a given parameter or parameter.  This
@@ -36,36 +248,7 @@ class Puppet::Parameter
             @doc ||= ""
 
             unless defined? @addeddocvals
-                unless values.empty?
-                    if @aliasvalues.empty?
-                        @doc += "  Valid values are ``" +
-                            values.join("``, ``") + "``."
-                    else
-                        @doc += "  Valid values are "
-
-                        @doc += values.collect do |value|
-                            ary = @aliasvalues.find do |name, val|
-                                val == value
-                            end
-                            if ary
-                                "``%s`` (also called ``%s``)" % [value, ary[0]]
-                            else
-                                "``#{value}``"
-                            end
-                        end.join(", ") + "."
-                    end
-                end
-
-                if defined? @parameterregexes and ! @parameterregexes.empty?
-                    regs = @parameterregexes
-                    if @parameterregexes.is_a? Hash
-                        regs = @parameterregexes.keys
-                    end
-                    unless regs.empty?
-                        @doc += "  Values can also match ``" +
-                            regs.collect { |r| r.inspect }.join("``, ``") + "``."
-                    end
-                end
+                @doc += value_collection.doc
 
                 if f = self.required_features
                     @doc += "  Requires features %s." % f.flatten.collect { |f| f.to_s }.join(" ")
@@ -88,9 +271,7 @@ class Puppet::Parameter
         end
 
         def initvars
-            @parametervalues = []
-            @aliasvalues = {}
-            @parameterregexes = []
+            @value_collection = ValueCollection.new
         end
 
         # This is how we munge the value.  Basically, this is our
@@ -101,23 +282,6 @@ class Puppet::Parameter
             # class's context, not the instance's, thus the two methods,
             # instead of just one.
             define_method(:unsafe_munge, &block)
-
-            define_method(:munge) do |*args|
-                begin
-                    ret = unsafe_munge(*args)
-                rescue Puppet::Error => detail
-                    Puppet.debug "Reraising %s" % detail
-                    raise
-                rescue => detail
-                    raise Puppet::DevError, "Munging failed for value %s in class %s: %s" %
-                        [args.inspect, self.name, detail], detail.backtrace
-                end
-
-                if self.shadow
-                    self.shadow.munge(*args)
-                end
-                ret
-            end
         end
 
         # Mark whether we're the namevar.
@@ -140,6 +304,11 @@ class Puppet::Parameter
             @required = true
         end
 
+        # Specify features that are required for this parameter to work.
+        def required_features=(*args)
+            @required_features = args.flatten.collect { |a| a.to_s.downcase.intern }
+        end
+
         # Is this parameter required?  Defaults to false.
         def required?
             if defined? @required
@@ -151,88 +320,16 @@ class Puppet::Parameter
 
         # Verify that we got a good value
         def validate(&block)
-            #@validater = block
             define_method(:unsafe_validate, &block)
-
-            define_method(:validate) do |*args|
-                begin
-                    unsafe_validate(*args)
-                rescue ArgumentError, Puppet::Error, TypeError
-                    raise
-                rescue => detail
-                    raise Puppet::DevError,
-                        "Validate method failed for class %s: %s" %
-                        [self.name, detail], detail.backtrace
-                end
-            end
-        end
-
-        # Does the value match any of our regexes?
-        def match?(value)
-            value = value.to_s unless value.is_a? String
-            @parameterregexes.find { |r|
-                r = r[0] if r.is_a? Array # Properties use a hash here
-                r =~ value
-            }
         end
 
         # Define a new value for our parameter.
         def newvalues(*names)
-            names.each { |name|
-                name = name.intern if name.is_a? String
-
-                case name
-                when Symbol
-                    if @parametervalues.include?(name)
-                        Puppet.warning "%s already has a value for %s" %
-                            [name, name]
-                    end
-                    @parametervalues << name
-                when Regexp
-                    if @parameterregexes.include?(name)
-                        Puppet.warning "%s already has a value for %s" %
-                            [name, name]
-                    end
-                    @parameterregexes << name
-                else
-                    raise ArgumentError, "Invalid value %s of type %s" %
-                        [name, name.class]
-                end
-            }
+            @value_collection.newvalues(*names)
         end
 
         def aliasvalue(name, other)
-            other = symbolize(other)
-            unless @parametervalues.include?(other)
-                raise Puppet::DevError,
-                    "Cannot alias nonexistent value %s" % other
-            end
-
-            @aliasvalues[name] = other
-        end
-
-        def alias(name)
-            @aliasvalues[name]
-        end
-
-        def regexes
-            return @parameterregexes.dup
-        end
-
-        def required_features=(*args)
-            @required_features = args.flatten.collect { |a| a.to_s.downcase.intern }
-        end
-
-        # Return the list of valid values.
-        def values
-            #[@aliasvalues.keys, @parametervalues.keys].flatten
-            if @parametervalues.is_a? Array
-                return @parametervalues.dup
-            elsif @parametervalues.is_a? Hash
-                return @parametervalues.keys
-            else
-                return []
-            end
+            @value_collection.aliasvalue(name, other)
         end
     end
 
@@ -252,7 +349,6 @@ class Puppet::Parameter
     attr_accessor :resource
     # LAK 2007-05-09: Keep the @parent around for backward compatibility.
     attr_accessor :parent
-    attr_reader :shadow
 
     def devfail(msg)
         self.fail(Puppet::DevError, msg)
@@ -289,17 +385,12 @@ class Puppet::Parameter
             raise Puppet::DevError, "No resource set for %s" % self.class.name
         end
 
-        if ! self.metaparam? and klass = Puppet::Type.metaparamclass(self.class.name)
-            setup_shadow(klass)
-        end
-
         set_options(options)
     end
 
     # Log a message using the resource's log level.
     def log(msg)
         unless @resource[:loglevel]
-            p @resource
             self.devfail "Parent %s has no loglevel" %
                 @resource.name
         end
@@ -344,73 +435,45 @@ class Puppet::Parameter
     end
 
     # If the specified value is allowed, then munge appropriately.
-    munge do |value|
-        if self.class.values.empty? and self.class.regexes.empty?
-            # This parameter isn't using defined values to do its work.
-            return value
-        end
-
-        # We convert to a string and then a symbol so that things like
-        # booleans work as we expect.
-        intern = value.to_s.intern
-
-        # If it's a valid value, always return it as a symbol.
-        if self.class.values.include?(intern)
-            retval = intern
-        elsif other = self.class.alias(intern)
-            retval = other
-        elsif ary = self.class.match?(value)
-            retval = value
-        else
-            # If it passed the validation but is not a registered value,
-            # we just return it as is.
-            retval = value
-        end
+    # If the developer uses a 'munge' hook, this method will get overridden.
+    def unsafe_munge(value)
+        self.class.value_collection.munge(value)
+    end
 
-        retval
+    # A wrapper around our munging that makes sure we raise useful exceptions.
+    def munge(value)
+        begin
+            ret = unsafe_munge(value)
+        rescue Puppet::Error => detail
+            Puppet.debug "Reraising %s" % detail
+            raise
+        rescue => detail
+            raise Puppet::DevError, "Munging failed for value %s in class %s: %s" % [value.inspect, self.name, detail], detail.backtrace
+        end
+        ret
     end
 
     # Verify that the passed value is valid.
-    validate do |value|
-        vals = self.class.values
-        regs = self.class.regexes
-
-        # this is true on properties
-        regs = regs.keys if regs.is_a?(Hash)
-
-        # This parameter isn't using defined values to do its work.
-        return if vals.empty? and regs.empty?
-
-        newval = value
-        newval = value.to_s.intern unless value.is_a?(Symbol)
-
-        name = newval
-
-        unless vals.include?(newval) or name = self.class.alias(newval) or name = self.class.match?(value) # We match the string, not the symbol
-            str = "Invalid '%s' value %s. " %
-                [self.class.name, value.inspect]
-
-            unless vals.empty?
-                str += "Valid values are %s. " % vals.join(", ")
-            end
-
-            unless regs.empty?
-                str += "Valid values match %s." % regs.collect { |r|
-                    r.to_s
-                }.join(", ")
-            end
+    # If the developer uses a 'validate' hook, this method will get overridden.
+    def unsafe_validate(value)
+        self.class.value_collection.validate(value)
+    end
 
-            raise ArgumentError, str
+    # A protected validation method that only ever raises useful exceptions.
+    def validate(value)
+        begin
+            unsafe_validate(value)
+        rescue ArgumentError => detail
+            fail detail.to_s
+        rescue Puppet::Error, TypeError
+            raise
+        rescue => detail
+            raise Puppet::DevError, "Validate method failed for class %s: %s" % [self.name, detail], detail.backtrace
         end
-
-        # Now check for features.
-        name = name[0] if name.is_a?(Array) # This is true for regexes.
-        validate_features_per_value(name) if is_a?(Puppet::Property)
     end
 
     def remove
         @resource = nil
-        @shadow = nil
     end
 
     attr_reader :value
@@ -419,14 +482,9 @@ class Puppet::Parameter
     # late-binding (e.g., users might not exist when the value is assigned
     # but might when it is asked for).
     def value=(value)
-        if respond_to?(:validate)
-            validate(value)
-        end
+        validate(value)
 
-        if respond_to?(:munge)
-            value = munge(value)
-        end
-        @value = value
+        @value = munge(value)
     end
 
     def inspect
@@ -444,23 +502,7 @@ class Puppet::Parameter
         @resource.provider || @resource
     end
 
-    # If there's a shadowing metaparam, instantiate it now.
-    # This allows us to create a property or parameter with the
-    # same name as a metaparameter, and the metaparam will only be
-    # stored as a shadow.
-    def setup_shadow(klass)
-        @shadow = klass.new(:resource => self.resource)
-    end
-
     def to_s
         s = "Parameter(%s)" % self.name
     end
-
-    # Make sure that we've got all of the required features for a given value.
-    def validate_features_per_value(value)
-        if features = self.class.value_option(value, :required_features)
-            raise ArgumentError, "Provider must have features '%s' to set '%s' to '%s'" % [features, self.class.name, value] unless provider.satisfies?(features)
-        end
-    end
 end
-
diff --git a/lib/puppet/property.rb b/lib/puppet/property.rb
index 50a1b71de..96f6ff1bf 100644
--- a/lib/puppet/property.rb
+++ b/lib/puppet/property.rb
@@ -4,8 +4,7 @@
 require 'puppet'
 require 'puppet/parameter'
 
-module Puppet
-class Property < Puppet::Parameter
+class Puppet::Property < Puppet::Parameter
 
     # Because 'should' uses an array, we have a special method for handling
     # it.  We also want to keep copies of the original values, so that
@@ -54,39 +53,26 @@ class Property < Puppet::Parameter
     end
 
     # Look up a value's name, so we can find options and such.
-    def self.value_name(value)
-        if value != '' and name = symbolize(value) and @parametervalues.include?(name)
-            return name
-        elsif ary = self.match?(value)
-            return ary[0]
-        else
-            return nil
+    def self.value_name(name)
+        if value = value_collection.match?(name)
+            value.name
         end
     end
 
     # Retrieve an option set when a value was defined.
     def self.value_option(name, option)
-        option = option.to_sym
-        if hash = @parameteroptions[name]
-            hash[option]
-        else
-            nil
+        if value = value_collection.value(name)
+            value.send(option)
         end
     end
 
-    # Create the value management variables.
-    def self.initvars
-        @parametervalues = {}
-        @aliasvalues = {}
-        @parameterregexes = {}
-        @parameteroptions = {}
-    end
-
     # Define a new valid value for a property.  You must provide the value itself,
     # usually as a symbol, or a regex to match the value.
     #
     # The first argument to the method is either the value itself or a regex.
     # The second argument is an option hash; valid options are:
+    # * <tt>:method</tt>: The name of the method to define.  Defaults to 'set_<value>'.
+    # * <tt>:required_features</tt>: A list of features this value requires.
     # * <tt>:event</tt>: The event that should be returned when this value is set.
     # * <tt>:call</tt>: When to call any associated block.  The default value
     #   is ``instead``, which means to call the value instead of calling the
@@ -94,52 +80,12 @@ class Property < Puppet::Parameter
     #   call both the block and the provider, according to the order you specify
     #   (the ``first`` refers to when the block is called, not the provider).
     def self.newvalue(name, options = {}, &block)
-        name = name.intern if name.is_a? String
-
-        @parameteroptions[name] = {}
-        paramopts = @parameteroptions[name]
-
-        # Symbolize everything
-        options.each do |opt, val|
-            paramopts[symbolize(opt)] = symbolize(val)
-        end
-
-        # By default, call the block instead of the provider.
-        if block_given?
-            paramopts[:call] ||= :instead
-        else
-            paramopts[:call] ||= :none
-        end
-        # If there was no block given, we still want to store the information
-        # for validation, but we won't be defining a method
-        block ||= true
+        value = value_collection.newvalue(name, options, &block)
 
-        case name
-        when Symbol
-            if @parametervalues.include?(name)
-                Puppet.warning "%s reassigning value %s" % [self.name, name]
-            end
-            @parametervalues[name] = block
-
-            if block_given?
-                method = "set_" + name.to_s
-                settor = paramopts[:settor] || (self.name.to_s + "=")
-                define_method(method, &block)
-                paramopts[:method] = method
-            end
-        when Regexp
-            # The regexes are handled in parameter.rb.  This value is used
-            # for validation.
-            @parameterregexes[name] = block
-
-            # This is used for looking up the block for execution.
-            if block_given?
-                paramopts[:block] = block
-            end
-        else
-            raise ArgumentError, "Invalid value %s of type %s" %
-                [name, name.class]
+        if value.method and value.block
+            define_method(value.method, &value.block)
         end
+        value
     end
 
     # Call the provider method.
@@ -175,9 +121,9 @@ class Property < Puppet::Parameter
         elsif block = self.class.value_option(name, :block)
             # FIXME It'd be better here to define a method, so that
             # the blocks could return values.
-            # If the regex was defined with no associated block, then just pass
-            # through and the correct event will be passed back.
             event = self.instance_eval(&block)
+        else
+            devfail "Could not find method for value '%s'" % name
         end
         return event, name
     end
@@ -231,9 +177,15 @@ class Property < Puppet::Parameter
         return event
     end
     
+    attr_reader :shadow
+
     # initialize our property
     def initialize(hash = {})
         super
+
+        if ! self.metaparam? and klass = Puppet::Type.metaparamclass(self.class.name)
+            setup_shadow(klass)
+        end
     end
 
     def inspect
@@ -308,6 +260,13 @@ class Property < Puppet::Parameter
         self.class.array_matching == :all
     end
 
+    # Execute our shadow's munge code, too, if we have one.
+    def munge(value)
+        self.shadow.munge(value) if self.shadow
+
+        super
+    end
+
     # each property class must define the name() method, and property instances
     # do not change that name
     # this implicitly means that a given object can only have one property
@@ -344,27 +303,37 @@ class Property < Puppet::Parameter
 
         call = self.class.value_option(name, :call)
 
-        # If we're supposed to call the block first or instead, call it now
-        if call == :before or call == :instead
+        if call == :instead
             event, tmp = call_valuemethod(name, value) 
-        end
-        unless call == :instead
+        elsif call == :none
             if @resource.provider
                 call_provider(value)
             else
                 # They haven't provided a block, and our parent does not have
                 # a provider, so we have no idea how to handle this.
-                self.fail "%s cannot handle values of type %s" %
-                    [self.class.name, value.inspect]
+                self.fail "%s cannot handle values of type %s" % [self.class.name, value.inspect]
             end
-        end
-        if call == :after
-            event, tmp = call_valuemethod(name, value) 
+        else
+            # LAK:NOTE 20081031 This is a change in behaviour -- you could
+            # previously specify :call => [;before|:after], which would call
+            # the setter *in addition to* the block.  I'm convinced this
+            # was never used, and it makes things unecessarily complicated.
+            # If you want to specify a block and still call the setter, then
+            # do so in the block.
+            devfail "Cannot use obsolete :call value %s" % call
         end
 
         return event(name, event)
     end
 
+    # If there's a shadowing metaparam, instantiate it now.
+    # This allows us to create a property or parameter with the
+    # same name as a metaparameter, and the metaparam will only be
+    # stored as a shadow.
+    def setup_shadow(klass)
+        @shadow = klass.new(:resource => self.resource)
+    end
+
     # Only return the first value
     def should
         if defined? @should
@@ -390,18 +359,8 @@ class Property < Puppet::Parameter
 
         @shouldorig = values
 
-        if self.respond_to?(:validate)
-            values.each { |val|
-                validate(val)
-            }
-        end
-        if self.respond_to?(:munge)
-            @should = values.collect { |val|
-                self.munge(val)
-            }
-        else
-            @should = values
-        end
+        values.each { |val| validate(val) }
+        @should = values.collect { |val| self.munge(val) }
     end
 
     def should_to_s(newvalue)
@@ -415,7 +374,7 @@ class Property < Puppet::Parameter
 
     # The default 'sync' method only selects among a list of registered # values.
     def sync
-        self.devfail("No values defined for %s" % self.class.name) unless self.class.values
+        self.devfail("No values defined for %s" % self.class.name) if self.class.value_collection.empty?
 
         if value = self.should
             set(value)
@@ -432,7 +391,7 @@ class Property < Puppet::Parameter
             if @resource.respond_to? :tags
                 @tags = @resource.tags
             end
-            @tags << self.name
+            @tags << self.name.to_s
         end
         @tags
     end
@@ -441,6 +400,20 @@ class Property < Puppet::Parameter
         return "%s(%s)" % [@resource.name,self.name]
     end
 
+    # Verify that the passed value is valid.
+    # If the developer uses a 'validate' hook, this method will get overridden.
+    def unsafe_validate(value)
+        super
+        validate_features_per_value(value)
+    end
+
+    # Make sure that we've got all of the required features for a given value.
+    def validate_features_per_value(value)
+        if features = self.class.value_option(self.class.value_name(value), :required_features)
+            raise ArgumentError, "Provider must have features '%s' to set '%s' to '%s'" % [features.collect { |f| f.to_s }.join(", "), self.class.name, value] unless provider.satisfies?(features)
+        end
+    end
+
     # Just return any should value we might have.
     def value
         self.should
@@ -545,5 +518,3 @@ class Property < Puppet::Parameter
         end
     end
 end
-end
-
diff --git a/lib/puppet/type/schedule.rb b/lib/puppet/type/schedule.rb
index ecbf7d49a..620472530 100755
--- a/lib/puppet/type/schedule.rb
+++ b/lib/puppet/type/schedule.rb
@@ -325,7 +325,7 @@ module Puppet
             )
 
             # And then one for every period
-            @parameters.find { |p| p.name == :period }.values.each { |value|
+            @parameters.find { |p| p.name == :period }.value_collection.values.each { |value|
                 result << self.create(
                     :name => value.to_s,
                     :period => value
diff --git a/lib/puppet/type/zone.rb b/lib/puppet/type/zone.rb
index 7601ec47b..1dee65b78 100644
--- a/lib/puppet/type/zone.rb
+++ b/lib/puppet/type/zone.rb
@@ -77,6 +77,7 @@ Puppet::Type.newtype(:zone) do
             used to stop zones."
 
         @states = {}
+        @parametervalues = []
 
         def self.alias_state(values)
             @state_aliases ||= {}
diff --git a/spec/unit/parameter.rb b/spec/unit/parameter.rb
index d6858c29d..53ea07648 100755
--- a/spec/unit/parameter.rb
+++ b/spec/unit/parameter.rb
@@ -5,20 +5,359 @@ require File.dirname(__FILE__) + '/../spec_helper'
 require 'puppet/parameter'
 
 describe Puppet::Parameter do
-    describe "when returning the value" do
-        before do
-            @class = Class.new(Puppet::Parameter)
-            @class.initvars
-            @parameter = @class.new :resource => mock('resource')
-        end
+    before do
+        @class = Class.new(Puppet::Parameter)
+        @class.initvars
+        @resource = mock 'resource'
+        @resource.stub_everything
+        @parameter = @class.new :resource => @resource
+    end
+
+    it "should create a value collection" do
+        @class = Class.new(Puppet::Parameter)
+        @class.value_collection.should be_nil
+        @class.initvars
+        @class.value_collection.should be_instance_of(Puppet::Parameter::ValueCollection)
+    end
 
+    describe "when returning the value" do
         it "should return nil if no value is set" do
             @parameter.value.should be_nil
         end
 
+        it "should validate the value" do
+            @parameter.expects(:validate).with("foo")
+            @parameter.value = "foo"
+        end
+
+        it "should munge the value and use any result as the actual value" do
+            @parameter.expects(:munge).with("foo").returns "bar"
+            @parameter.value = "foo"
+            @parameter.value.should == "bar"
+        end
+
         it "should return any set value" do
             @parameter.value = "foo"
             @parameter.value.should == "foo"
         end
     end
+
+    describe "when validating values" do
+        it "should do nothing if no values or regexes have been defined" do
+            @parameter.validate("foo")
+        end
+
+        it "should catch abnormal failures thrown during validation" do
+            @class.validate { |v| raise "This is broken" }
+            lambda { @parameter.validate("eh") }.should raise_error(Puppet::DevError)
+        end
+
+        it "should fail if the value is not a defined value or alias and does not match a regex" do
+            @class.newvalues :foo
+            lambda { @parameter.validate("bar") }.should raise_error(Puppet::Error)
+        end
+
+        it "should succeed if the value is one of the defined values" do
+            @class.newvalues :foo
+            lambda { @parameter.validate(:foo) }.should_not raise_error(ArgumentError)
+        end
+
+        it "should succeed if the value is one of the defined values even if the definition uses a symbol and the validation uses a string" do
+            @class.newvalues :foo
+            lambda { @parameter.validate("foo") }.should_not raise_error(ArgumentError)
+        end
+
+        it "should succeed if the value is one of the defined values even if the definition uses a string and the validation uses a symbol" do
+            @class.newvalues "foo"
+            lambda { @parameter.validate(:foo) }.should_not raise_error(ArgumentError)
+        end
+
+        it "should succeed if the value is one of the defined aliases" do
+            @class.newvalues :foo
+            @class.aliasvalue :bar, :foo
+            lambda { @parameter.validate("bar") }.should_not raise_error(ArgumentError)
+        end
+
+        it "should succeed if the value matches one of the regexes" do
+            @class.newvalues %r{\d}
+            lambda { @parameter.validate("10") }.should_not raise_error(ArgumentError)
+        end
+    end
+
+    describe "when munging values" do
+        it "should do nothing if no values or regexes have been defined" do
+            @parameter.munge("foo").should == "foo"
+        end
+
+        it "should catch abnormal failures thrown during munging" do
+            @class.munge { |v| raise "This is broken" }
+            lambda { @parameter.munge("eh") }.should raise_error(Puppet::DevError)
+        end
+
+        it "should return return any matching defined values" do
+            @class.newvalues :foo, :bar
+            @parameter.munge("foo").should == :foo
+        end
+
+        it "should return any matching aliases" do
+            @class.newvalues :foo
+            @class.aliasvalue :bar, :foo
+            @parameter.munge("bar").should == :foo
+        end
+
+        it "should return the value if it matches a regex" do
+            @class.newvalues %r{\w}
+            @parameter.munge("bar").should == "bar"
+        end
+
+        it "should return the value if no other option is matched" do
+            @class.newvalues :foo
+            @parameter.munge("bar").should == "bar"
+        end
+    end
+end
+
+describe Puppet::Parameter::ValueCollection do
+    before do
+        @collection = Puppet::Parameter::ValueCollection.new
+    end
+
+    it "should have a method for defining new values" do
+        @collection.should respond_to(:newvalues)
+    end
+
+    it "should have a method for adding individual values" do
+        @collection.should respond_to(:newvalue)
+    end
+
+    it "should be able to retrieve individual values" do
+        value = @collection.newvalue(:foo)
+        @collection.value(:foo).should equal(value)
+    end
+
+    it "should be able to add an individual value with a block" do
+        @collection.newvalue(:foo) { raise "testing" }
+        @collection.value(:foo).block.should be_instance_of(Proc)
+    end
+
+    it "should be able to add values that are empty strings" do
+        lambda { @collection.newvalue('') }.should_not raise_error
+    end
+
+    it "should be able to add values that are empty strings" do
+        value = @collection.newvalue('')
+        @collection.match?('').should equal(value)
+    end
+
+    it "should set :call to :none when adding a value with no block" do
+        value = @collection.newvalue(:foo)
+        value.call.should == :none
+    end
+
+    describe "when adding a value with a block" do
+        it "should set the method name to 'set_' plus the value name" do
+            value = @collection.newvalue(:myval) { raise "testing" }
+            value.method.should == "set_myval"
+        end
+    end
+
+    it "should be able to add an individual value with options" do
+        value = @collection.newvalue(:foo, :call => :bar)
+        value.call.should == :bar
+    end
+
+    it "should have a method for validating a value" do
+        @collection.should respond_to(:validate)
+    end
+
+    it "should have a method for munging a value" do
+        @collection.should respond_to(:munge)
+    end
+
+    it "should be able to generate documentation when it has both values and regexes" do
+        @collection.newvalues :foo, "bar", %r{test}
+        @collection.doc.should be_instance_of(String)
+    end
+
+    it "should correctly generate documentation for values" do
+        @collection.newvalues :foo
+        @collection.doc.should be_include("Valid values are ``foo``")
+    end
+
+    it "should correctly generate documentation for regexes" do
+        @collection.newvalues %r{\w+}
+        @collection.doc.should be_include("Values can match ``/\\w+/``")
+    end
+
+    it "should be able to find the first matching value" do
+        @collection.newvalues :foo, :bar
+        @collection.match?("foo").should be_instance_of(Puppet::Parameter::ValueCollection::Value)
+    end
+
+    it "should be able to match symbols" do
+        @collection.newvalues :foo, :bar
+        @collection.match?(:foo).should be_instance_of(Puppet::Parameter::ValueCollection::Value)
+    end
+
+    it "should be able to match symbols when a regex is provided" do
+        @collection.newvalues %r{.}
+        @collection.match?(:foo).should be_instance_of(Puppet::Parameter::ValueCollection::Value)
+    end
+
+    it "should be able to match values using regexes" do
+        @collection.newvalues %r{.}
+        @collection.match?("foo").should_not be_nil
+    end
+
+    it "should prefer value matches to regex matches" do
+        @collection.newvalues %r{.}, :foo
+        @collection.match?("foo").name.should == :foo
+    end
+
+    describe "when validating values" do
+        it "should do nothing if no values or regexes have been defined" do
+            @collection.validate("foo")
+        end
+
+        it "should fail if the value is not a defined value or alias and does not match a regex" do
+            @collection.newvalues :foo
+            lambda { @collection.validate("bar") }.should raise_error(ArgumentError)
+        end
+
+        it "should succeed if the value is one of the defined values" do
+            @collection.newvalues :foo
+            lambda { @collection.validate(:foo) }.should_not raise_error(ArgumentError)
+        end
+
+        it "should succeed if the value is one of the defined values even if the definition uses a symbol and the validation uses a string" do
+            @collection.newvalues :foo
+            lambda { @collection.validate("foo") }.should_not raise_error(ArgumentError)
+        end
+
+        it "should succeed if the value is one of the defined values even if the definition uses a string and the validation uses a symbol" do
+            @collection.newvalues "foo"
+            lambda { @collection.validate(:foo) }.should_not raise_error(ArgumentError)
+        end
+
+        it "should succeed if the value is one of the defined aliases" do
+            @collection.newvalues :foo
+            @collection.aliasvalue :bar, :foo
+            lambda { @collection.validate("bar") }.should_not raise_error(ArgumentError)
+        end
+
+        it "should succeed if the value matches one of the regexes" do
+            @collection.newvalues %r{\d}
+            lambda { @collection.validate("10") }.should_not raise_error(ArgumentError)
+        end
+    end
+
+    describe "when munging values" do
+        it "should do nothing if no values or regexes have been defined" do
+            @collection.munge("foo").should == "foo"
+        end
+
+        it "should return return any matching defined values" do
+            @collection.newvalues :foo, :bar
+            @collection.munge("foo").should == :foo
+        end
+
+        it "should return any matching aliases" do
+            @collection.newvalues :foo
+            @collection.aliasvalue :bar, :foo
+            @collection.munge("bar").should == :foo
+        end
+
+        it "should return the value if it matches a regex" do
+            @collection.newvalues %r{\w}
+            @collection.munge("bar").should == "bar"
+        end
+
+        it "should return the value if no other option is matched" do
+            @collection.newvalues :foo
+            @collection.munge("bar").should == "bar"
+        end
+    end
+end
+
+describe Puppet::Parameter::ValueCollection::Value do
+    it "should require a name" do
+        lambda { Puppet::Parameter::ValueCollection::Value.new }.should raise_error(ArgumentError)
+    end
+
+    it "should set its name" do
+        Puppet::Parameter::ValueCollection::Value.new(:foo).name.should == :foo
+    end
+
+    it "should support regexes as names" do
+        lambda { Puppet::Parameter::ValueCollection::Value.new(%r{foo}) }.should_not raise_error
+    end
+
+    it "should mark itself as a regex if its name is a regex" do
+        Puppet::Parameter::ValueCollection::Value.new(%r{foo}).should be_regex
+    end
+
+    it "should always convert its name to a symbol if it is not a regex" do
+        Puppet::Parameter::ValueCollection::Value.new("foo").name.should == :foo
+        Puppet::Parameter::ValueCollection::Value.new(true).name.should == :true
+    end
+
+    it "should support adding aliases" do
+        Puppet::Parameter::ValueCollection::Value.new("foo").should respond_to(:alias)
+    end
+
+    it "should be able to return its aliases" do
+        value = Puppet::Parameter::ValueCollection::Value.new("foo")
+        value.alias("bar")
+        value.alias("baz")
+        value.aliases.should == [:bar, :baz]
+    end
+
+    [:block, :call, :method, :event, :required_features].each do |attr|
+        it "should support a #{attr} attribute" do
+            value = Puppet::Parameter::ValueCollection::Value.new("foo")
+            value.should respond_to(attr.to_s + "=")
+            value.should respond_to(attr)
+        end
+    end
+
+    it "should default to :instead for :call if a block is provided" do
+        Puppet::Parameter::ValueCollection::Value.new("foo").call.should == :instead
+    end
+
+    it "should always return events as symbols" do
+        value = Puppet::Parameter::ValueCollection::Value.new("foo")
+        value.event = "foo_test"
+        value.event.should == :foo_test
+    end
+
+    describe "when matching" do
+        describe "a regex" do
+            it "should return true if the regex matches the value" do
+                Puppet::Parameter::ValueCollection::Value.new(/\w/).should be_match("foo")
+            end
+
+            it "should return false if the regex does not match the value" do
+                Puppet::Parameter::ValueCollection::Value.new(/\d/).should_not be_match("foo")
+            end
+        end
+
+        describe "a non-regex" do
+            it "should return true if the value, converted to a symbol, matches the name" do
+                Puppet::Parameter::ValueCollection::Value.new("foo").should be_match("foo")
+                Puppet::Parameter::ValueCollection::Value.new(:foo).should be_match(:foo)
+                Puppet::Parameter::ValueCollection::Value.new(:foo).should be_match("foo")
+                Puppet::Parameter::ValueCollection::Value.new("foo").should be_match(:foo)
+            end
+
+            it "should return false if the value, converted to a symbol, does not match the name" do
+                Puppet::Parameter::ValueCollection::Value.new(:foo).should_not be_match(:bar)
+            end
+
+            it "should return true if any of its aliases match" do
+                value = Puppet::Parameter::ValueCollection::Value.new("foo")
+                value.alias("bar")
+                value.should be_match("bar")
+            end
+        end
+    end
 end
diff --git a/spec/unit/property.rb b/spec/unit/property.rb
index e5b1e0013..89b736cf4 100755
--- a/spec/unit/property.rb
+++ b/spec/unit/property.rb
@@ -1,35 +1,283 @@
-#!/usr/bin/env ruby
+#!/usr/bin/env ruby"
 
 require File.dirname(__FILE__) + '/../spec_helper'
-
 require 'puppet/property'
 
 describe Puppet::Property do
-    describe "when setting the value" do
-        it "should just set the 'should' value" do
-            @class = Class.new(Puppet::Property)
-            @class.initvars
-            @property = @class.new :resource => mock('resource')
+    before do
+        @class = Class.new(Puppet::Property) do
+            @name = :foo
+        end
+        @class.initvars
+        @provider = mock 'provider'
+        @resource = stub 'resource', :provider => @provider
+        @resource.stub_everything
+        @property = @class.new :resource => @resource
+    end
+
+    it "should be able to look up the modified name for a given value" do
+        @class.newvalue(:foo)
+        @class.value_name("foo").should == :foo
+    end
+
+    it "should be able to look up the modified name for a given value matching a regex" do
+        @class.newvalue(%r{.})
+        @class.value_name("foo").should == %r{.}
+    end
+
+    it "should be able to look up a given value option" do
+        @class.newvalue(:foo, :event => :whatever)
+        @class.value_option(:foo, :event).should == :whatever
+    end
+
+    it "should return the resource's tags plus its name as its tags" do
+        @resource.expects(:tags).returns %w{one two}
+        @property.tags.should == %w{one two foo}
+    end
+
+    it "should be able to specify required features" do
+        @class.should respond_to(:required_features=)
+    end
 
-            @property.expects(:should=).with("foo")
+    it "should always convert required features into an array of symbols" do
+        @class.required_features = %w{one two}
+        @class.required_features.should == [:one, :two]
+    end
+
+    it "should be able to shadow metaparameters" do
+        @property.must respond_to(:shadow)
+    end
+
+    describe "when shadowing metaparameters" do
+        before do
+            @shadow_class = Class.new(Puppet::Property) do
+                @name = :alias
+            end
+            @shadow_class.initvars
+        end
+
+        it "should create an instance of the metaparameter at initialization" do
+            Puppet::Type.metaparamclass(:alias).expects(:new).with(:resource => @resource)
+
+            @shadow_class.new :resource => @resource
+        end
+
+        it "should munge values using the shadow's munge method" do
+            shadow = mock 'shadow'
+            Puppet::Type.metaparamclass(:alias).expects(:new).returns shadow
+
+            shadow.expects(:munge).with "foo"
+
+            property = @shadow_class.new :resource => @resource
+            property.munge("foo")
+        end
+    end
+
+    describe "when defining new values" do
+        it "should define a method for each value created with a block that's not a regex" do
+            @class.newvalue(:foo) { }
+            @property.must respond_to(:set_foo)
+        end
+    end
+
+    describe "when assigning the value" do
+        it "should just set the 'should' value" do
             @property.value = "foo"
+            @property.should.must == "foo"
+        end
+
+        it "should validate each value separately" do
+            @property.expects(:validate).with("one")
+            @property.expects(:validate).with("two")
+
+            @property.value = %w{one two}
+        end
+
+        it "should munge each value separately and use any result as the actual value" do
+            @property.expects(:munge).with("one").returns :one
+            @property.expects(:munge).with("two").returns :two
+
+            # Do this so we get the whole array back.
+            @class.array_matching = :all
+
+            @property.value = %w{one two}
+            @property.should.must == [:one, :two]
+        end
+
+        it "should return any set value" do
+            (@property.value = :one).should == :one
         end
     end
 
     describe "when returning the value" do
-        before do
-            @class = Class.new(Puppet::Property)
-            @class.initvars
-            @property = @class.new :resource => mock('resource')
+        it "should return nil if no value is set" do
+            @property.should.must be_nil
         end
 
-        it "should return nil if no value is set" do
-            @property.value.should be_nil
+        it "should return the first set 'should' value if :array_matching is set to :first" do
+            @class.array_matching = :first
+            @property.should = %w{one two}
+            @property.should.must == "one"
+        end
+
+        it "should return all set 'should' values as an array if :array_matching is set to :all" do
+            @class.array_matching = :all
+            @property.should = %w{one two}
+            @property.should.must == %w{one two}
+        end
+
+        it "should default to :first array_matching" do
+            @class.array_matching.should == :first
+        end
+    end
+
+    describe "when validating values" do
+        it "should do nothing if no values or regexes have been defined" do
+            lambda { @property.should = "foo" }.should_not raise_error
+        end
+
+        it "should fail if the value is not a defined value or alias and does not match a regex" do
+            @class.newvalue(:foo)
+
+            lambda { @property.should = "bar" }.should raise_error
+        end
+
+        it "should succeeed if the value is one of the defined values" do
+            @class.newvalue(:foo)
+
+            lambda { @property.should = :foo }.should_not raise_error
+        end
+
+        it "should succeeed if the value is one of the defined values even if the definition uses a symbol and the validation uses a string" do
+            @class.newvalue(:foo)
+
+            lambda { @property.should = "foo" }.should_not raise_error
+        end
+
+        it "should succeeed if the value is one of the defined values even if the definition uses a string and the validation uses a symbol" do
+            @class.newvalue("foo")
+
+            lambda { @property.should = :foo }.should_not raise_error
+        end
+
+        it "should succeed if the value is one of the defined aliases" do
+            @class.newvalue("foo")
+            @class.aliasvalue("bar", "foo")
+
+            lambda { @property.should = :bar }.should_not raise_error
         end
 
-        it "should return any set 'should' value" do
+        it "should succeed if the value matches one of the regexes" do
+            @class.newvalue(/./)
+
+            lambda { @property.should = "bar" }.should_not raise_error
+        end
+
+        it "should validate that all required features are present" do
+            @class.newvalue(:foo, :required_features => [:a, :b])
+
+            @provider.expects(:satisfies?).with([:a, :b]).returns true
+
+            @property.should = :foo
+        end
+
+        it "should fail if required features are missing" do
+            @class.newvalue(:foo, :required_features => [:a, :b])
+
+            @provider.expects(:satisfies?).with([:a, :b]).returns false
+
+            lambda { @property.should = :foo }.should raise_error(Puppet::Error)
+        end
+
+        it "should validate that all required features are present for regexes" do
+            value = @class.newvalue(/./, :required_features => [:a, :b])
+
+            @provider.expects(:satisfies?).with([:a, :b]).returns true
+
             @property.should = "foo"
-            @property.value.should == "foo"
+        end
+    end
+
+    describe "when munging values" do
+        it "should do nothing if no values or regexes have been defined" do
+            @property.munge("foo").should == "foo"
+        end
+
+        it "should return return any matching defined values" do
+            @class.newvalue(:foo)
+            @property.munge("foo").should == :foo
+        end
+
+        it "should return any matching aliases" do
+            @class.newvalue(:foo)
+            @class.aliasvalue(:bar, :foo)
+            @property.munge("bar").should == :foo
+        end
+
+        it "should return the value if it matches a regex" do
+            @class.newvalue(/./)
+            @property.munge("bar").should == "bar"
+        end
+
+        it "should return the value if no other option is matched" do
+            @class.newvalue(:foo)
+            @property.munge("bar").should == "bar"
+        end
+    end
+
+    describe "when syncing the 'should' value" do
+        it "should fail if no values have been defined and the 'sync' method has not been overridden" do
+            @property.should = "eh"
+            lambda { @property.sync }.should raise_error(Puppet::DevError)
+        end
+
+        it "should set the value" do
+            @class.newvalue(:foo)
+            @property.should = :foo
+            @property.expects(:set).with(:foo)
+            @property.sync
+        end
+    end
+
+    describe "when setting a value" do
+        it "should catch exceptions and raise Puppet::Error" do
+            @class.newvalue(:foo) { raise "eh" }
+            lambda { @property.set(:foo) }.should raise_error(Puppet::Error)
+        end
+
+        describe "that was defined without a block" do
+            it "should call the settor on the provider" do
+                @class.newvalue(:bar)
+                @provider.expects(:foo=).with :bar
+                @property.set(:bar)
+            end
+
+            it "should return any specified event" do
+                @class.newvalue(:bar, :event => :whatever)
+                @property.should = :bar
+                @provider.expects(:foo=).with :bar
+                @property.set(:bar).should == :whatever
+            end
+        end
+
+        describe "that was defined with a block" do
+            it "should call the method created for the value if the value is not a regex" do
+                @class.newvalue(:bar) {}
+                @property.expects(:set_bar)
+                @property.set(:bar)
+            end
+
+            it "should call the provided block if the value is a regex" do
+                @class.newvalue(/./) { self.test }
+                @property.expects(:test)
+                @property.set("foo")
+            end
+
+            it "should return any specified event" do
+                @class.newvalue(:bar, :event => :myevent) {}
+                @property.expects(:set_bar)
+                @property.set(:bar).should == :myevent
+            end
         end
     end
 end
diff --git a/test/ral/type/parameter.rb b/test/ral/type/parameter.rb
deleted file mode 100755
index 04c4b0ce1..000000000
--- a/test/ral/type/parameter.rb
+++ /dev/null
@@ -1,174 +0,0 @@
-#!/usr/bin/env ruby
-
-require File.dirname(__FILE__) + '/../../lib/puppettest'
-
-require 'puppettest'
-
-class TestParameter < Test::Unit::TestCase
-	include PuppetTest
-    
-    def newparam(name = :fakeparam)
-        assert_nothing_raised {
-            param = Class.new(Puppet::Parameter) do
-                @name = :fakeparam
-            end
-            param.initvars
-
-            return param
-        }
-    end
-
-    def newinst(param)
-        assert_nothing_raised {
-            return param.new(:resource => "yay")
-        }
-    end
-
-    # Test the basic newvalue stuff.
-    def test_newvalue
-        param = newparam()
-
-        # Try it with both symbols and strings.
-        assert_nothing_raised {
-            param.newvalues(:one, "two")
-        }
-
-        inst = newinst(param)
-
-        assert_nothing_raised {
-            inst.value = "one"
-        }
-
-        assert_equal(:one, inst.value)
-
-        assert_nothing_raised {
-            inst.value = :two
-        }
-        assert_equal(:two, inst.value)
-
-        assert_raise(ArgumentError) {
-            inst.value = :three
-        }
-        assert_equal(:two, inst.value)
-    end
-
-    # Test using regexes.
-    def test_regexvalues
-        param = newparam
-
-        assert_nothing_raised {
-            param.newvalues(/^\d+$/)
-        }
-        assert(param.match?("14"))
-        assert(param.match?(14))
-
-        inst = newinst(param)
-
-        assert_nothing_raised {
-            inst.value = 14
-        }
-
-        assert_nothing_raised {
-            inst.value = "14"
-        }
-
-        assert_raise(ArgumentError) {
-            inst.value = "a14"
-        }
-    end
-
-    # Test using both.  Equality should beat matching.
-    def test_regexesandnormals
-        param = newparam
-
-        assert_nothing_raised {
-            param.newvalues(:one, /^\w+$/)
-        }
-
-        inst = newinst(param)
-
-        assert_nothing_raised {
-            inst.value = "one"
-        }
-
-        assert_equal(:one, inst.value, "Value used regex instead of equality")
-
-        assert_nothing_raised {
-            inst.value = "two"
-        }
-        assert_equal("two", inst.value, "Matched value didn't take")
-    end
-
-    def test_shadowing
-        type = Puppet::Type.newtype(:faketype) { newparam(:name) {} }
-
-        cleanup { Puppet::Type.rmtype(:faketype) }
-
-        param = nil
-        assert_nothing_raised do
-            param = type.newproperty(:alias)
-        end
-
-        assert(param, "did not create param")
-
-        inst = type.create(:name => "test")
-
-        config = mk_catalog
-        inst.catalog = config
-
-        assert_nothing_raised("Could not create shadowed param") {
-            inst[:alias] = "foo"
-        }
-
-        # Get the parameter hash from the instance so we can check the shadow
-        params = inst.instance_variable_get("@parameters")
-        obj = params[:alias]
-        assert(obj, "did not get alias parameter")
-        assert(obj.shadow, "shadow was not created for alias param")
-
-        assert(obj.is_a?(Puppet::Property),
-            "alias instance is not a property")
-        assert_instance_of(param, obj, "alias is an instance of the wrong class")
-
-        # Make sure the alias got created
-        assert(config.resource(type.name, "foo"), "Did not retrieve object by its alias")
-        
-        # Now try it during initialization
-        other = nil
-        assert_nothing_raised("Could not create instance with shadow") do
-            other = type.create(:name => "rah", :alias => "one", :catalog => config)
-        end
-        params = other.instance_variable_get("@parameters")
-        obj = params[:alias]
-        assert(obj, "did not get alias parameter")
-        assert(obj.shadow, "shadow was not created for alias param")
-
-        assert_instance_of(param, obj, "alias is an instance of the wrong class")
-        assert(obj.is_a?(Puppet::Property),
-            "alias instance is not a property")
-
-        # Now change the alias and make sure it works out well
-        assert_nothing_raised("Could not modify shadowed alias") do
-            other[:alias] = "two"
-        end
-
-        obj = params[:alias]
-        assert(obj, "did not get alias parameter")
-        assert_instance_of(param, obj, "alias is now an instance of the wrong class")
-        assert(obj.is_a?(Puppet::Property),
-            "alias instance is now not a property")
-    end
-
-    # Make sure properties can correctly require features and behave appropriately when
-    # those features are missing.
-    def test_requires_features
-        param = newparam(:feature_tests)
-
-        assert_nothing_raised("could not add feature requirements to property") do
-            param.required_features = "testing"
-        end
-
-        assert_equal([:testing], param.required_features, "required features value was not arrayfied and interned")
-    end
-end
-
diff --git a/test/ral/type/property.rb b/test/ral/type/property.rb
deleted file mode 100755
index 6a3370caa..000000000
--- a/test/ral/type/property.rb
+++ /dev/null
@@ -1,388 +0,0 @@
-#!/usr/bin/env ruby
-
-require File.dirname(__FILE__) + '/../../lib/puppettest'
-
-require 'puppettest'
-
-class TestProperty < Test::Unit::TestCase
-	include PuppetTest
-
-    def newinst(property, resource = nil)
-        inst = nil
-        unless resource
-            resource = "fakeresource"
-            resource.meta_def(:pathbuilder) do [self.to_s] end
-            resource.meta_def(:provider) do nil end
-            resource.meta_def(:fakeproperty) do '' end
-        end
-        assert_nothing_raised {
-            newinst = property.new(:resource => resource)
-            def newinst.retrieve(); return @fakeprovidervalue; end;
-            return newinst
-        }
-    end
-    
-    def newproperty(name = :fakeproperty)
-        property = Class.new(Puppet::Property) do
-            @name = name
-        end
-        Object.const_set("FakeProperty", property)
-        property.initvars
-        cleanup do
-            Object.send(:remove_const, "FakeProperty")
-        end
-
-        return property
-    end
-
-    def newmodel(name)
-        # Create an object that responds to myproperty as an attr
-        provklass = Class.new { attr_accessor name
-            def pathbuilder
-                ["provklass"]
-            end
-        }
-        prov = provklass.new
-
-        klass = Class.new { attr_accessor :provider, :path
-            def pathbuilder
-                ["instklass"]
-            end
-        }
-        klassinst = klass.new
-        klassinst.path = "instpath"
-        klassinst.provider = prov
-
-        return prov, klassinst
-    end
-
-    # Make sure we correctly look up names.
-    def test_value_name
-        property = newproperty()
-
-        property.newvalue(:one)
-        property.newvalue(/\d+/)
-
-        name = nil
-        ["one", :one].each do |value|
-            assert_nothing_raised do
-                name = property.value_name(value)
-            end
-            assert_equal(:one, name)
-        end
-        ["42"].each do |value|
-            assert_nothing_raised do
-                name = property.value_name(value)
-            end
-            assert_equal(/\d+/, name)
-        end
-        # these values should not have a name
-        ["two", :three, ''].each do |value|
-            assert_nothing_raised do
-                name = property.value_name(value)
-            end
-            assert_nil(name)
-        end
-    end
-
-    # Test that we correctly look up options for values.
-    def test_value_option
-        property = newproperty()
-
-        options = {
-            :one => {:event => :yay, :call => :before},
-            /\d+/ => {:event => :fun, :call => :instead}
-        }
-        property.newvalue(:one, options[:one])
-        property.newvalue(/\d+/, options[/\d+/])
-
-        options.each do |name, opts|
-            opts.each do |param, value|
-                assert_equal(value, property.value_option(name, param))
-            end
-        end
-    end
-
-    def test_newvalue
-        property = newproperty()
-
-        # These are bogus because they don't define events. :/
-        assert_nothing_raised {
-            property.newvalue(:one) do
-                @fakeprovidervalue = 1
-            end
-        }
-
-        assert_nothing_raised {
-            property.newvalue("two") do
-                @fakeprovidervalue = 2
-            end
-        }
-
-        # Make sure we default to using the block instead
-        assert_equal(:instead, property.value_option(:one, :call),
-            ":call was not set to :instead when a block was provided")
-
-        inst = newinst(property)
-
-        assert_nothing_raised {
-            inst.should = "one"
-        }
-
-        assert_equal(:one, inst.should)
-        ret = nil
-        assert_nothing_raised { inst.set_one }
-        assert_equal(1, inst.retrieve)
-
-        assert_nothing_raised {
-            inst.should = :two
-        }
-
-        assert_equal(:two, inst.should)
-        assert_nothing_raised { inst.set_two }
-        assert_equal(2, inst.retrieve)
-    end
-
-    def test_newpropertyvaluewithregexes
-        property = newproperty()
-
-        assert_nothing_raised {
-            property.newvalue(/^\w+$/) do
-                return :regex_matched
-            end
-        }
-
-        inst = newinst(property)
-
-        assert_nothing_raised {
-            inst.should = "yayness"
-        }
-
-        assert_equal("yayness", inst.should)
-
-        assert_nothing_raised {
-            inst.sync
-        }
-
-        assert_equal("yayness".upcase, inst.retrieve)
-    end
-
-    def test_newvalue_event_option
-        property = newproperty()
-
-        assert_nothing_raised do
-            property.newvalue(:myvalue, :event => :fake_valued) do
-            end
-            property.newvalue(:other, :event => "fake_other") do
-            end
-        end
-        inst = newinst(property)
-
-        assert_nothing_raised {
-            inst.should = :myvalue
-        }
-
-        ret = nil
-        assert_nothing_raised {
-            ret = inst.sync
-        }
-
-        assert_equal(:fake_valued, ret,
-                     "Event did not get returned correctly")
-
-        assert_nothing_raised {
-            inst.should = :other
-        }
-
-        assert_nothing_raised {
-            ret = inst.sync
-        }
-
-        assert_equal(:fake_other, ret,
-                     "Event did not get returned correctly")
-    end
-
-    # We want to support values with no blocks, either regexes or strings.
-    # If there's no block provided, then we should call the provider mechanism
-    # like we would normally.
-    def test_newvalue_with_no_block
-        property = newproperty(:myproperty)
-
-        assert_nothing_raised {
-            property.newvalue(:value, :event => :matched_value)
-        }
-        assert_nothing_raised {
-            property.newvalue(/^\d+$/, :event => :matched_number)
-        }
-
-        assert_equal(:none, property.value_option(:value, :call),
-            ":call was not set to none when no block is provided")
-
-        prov, klassinst = newmodel(:myproperty)
-
-        inst = newinst(property, klassinst)
-
-        # Now make sure we can set the values, they get validated as normal,
-        # and they set the values on the resource rather than trying to call
-        # a method
-        {:value => :matched_value, "27" => :matched_number}.each do |value, event|
-            assert_nothing_raised do
-                inst.should = value
-            end
-            ret = nil
-            assert_nothing_raised do
-                ret = inst.sync
-            end
-            assert_equal(event, ret, "Did not return correct event for %s" % value)
-            assert_equal(value, prov.myproperty, "%s was not set right" % value)
-        end
-
-        # And make sure we still fail validations
-        assert_raise(ArgumentError) do
-            inst.should = "invalid"
-        end
-    end
-
-    def test_tags
-        obj = "yay"
-        metaobj = class << obj; self; end
-
-        metaobj.send(:attr_accessor, :tags)
-
-        tags = [:some, :tags, :for, :testing]
-        obj.tags = tags
-
-        propertyklass = newproperty
- 
-        inst = nil
-        assert_nothing_raised do
-            inst = propertyklass.new(:resource => obj)
-        end
-
-        assert_nothing_raised do
-            assert_equal(tags + [inst.name], inst.tags)
-        end
-    end
-
-    def test_failure
-        s = Struct.new(:line, :file, :path, :pathbuilder, :name)
-        p = s.new(1, "yay", "rah", "struct", "name")
-
-        myprovider = Class.new(Puppet::Provider)
-  
-        def p.provider; nil; end;
-        myproperty = Class.new(Puppet::Property) do 
-            @name = 'name'
-        end
-        myproperty.initvars
-
-        myproperty.newvalue :mkfailure do
-            raise "It's all broken"
-        end
-        property = myproperty.new(:resource => p)
-
-        assert_raise(Puppet::Error) do
-            property.set(:mkfailure)
-        end
-    end
-
-    # Make sure 'set' behaves correctly WRT to call order.  This tests that the
-    # :call value is handled correctly in all cases.
-    def test_set
-        property = newproperty(:myproperty)
-
-        $setting = []
-
-        newval = proc do |name, call|
-            options = {}
-            if call
-                options[:call] = name
-                block = proc { $setting << name }
-            end
-            assert_nothing_raised("Could not create %s value" % name) {
-                if block
-                    property.newvalue(name, options, &block)
-                else
-                    property.newvalue(name, options)
-                end
-            }
-        end
-
-        newval.call(:none, false)
-
-        # Create a value with no block; it should default to :none
-        newval.call(:before, true)
-
-        # One with a block but after
-        newval.call(:after, true)
-
-        # One with an explicit instead
-        newval.call(:instead, true)
-
-        # And one with an implicit instead
-        assert_nothing_raised do
-            property.newvalue(:implicit) do
-                $setting << :implicit
-            end
-        end
-
-        # Now create a provider
-        prov, model = newmodel(:myproperty)
-        inst = newinst(property, model)
-
-        # Mark when we're called
-        prov.meta_def(:myproperty=) do |value| $setting << :provider end
-
-        # Now run through the list and make sure everything is correct
-        {:before => [:before, :provider],
-            :after => [:provider, :after],
-            :instead => [:instead],
-            :none => [:provider],
-            :implicit => [:implicit]
-        }.each do |name, result|
-            inst.set(name)
-
-            assert_equal(result, $setting, "%s was not handled right" % name)
-            $setting.clear
-        end
-    end
-
-    # Make sure we can specify that we want to use the whole array, rather
-    # than just individual values.
-    def test_array_handling
-        property = newproperty(:arraytests)
-
-        prov, model = newmodel(:array_testing)
-        inst = newinst(property, model)
-
-        # Make sure it defaults to first
-        assert_equal(:first, property.array_matching, "Property did not default to matching first value in an array")
-        assert(! inst.match_all?, "match_all? returned true when array_matching is :first")
-
-        vals = %w{one two three}
-        inst.should = vals
-
-        # Make sure we only get the first value back
-        assert_equal("one", inst.should, "Returned wrong value when array_matching == first")
-
-        # And make sure any of these values is considered in sync
-        vals.each do |value|
-            assert(inst.insync?(value), "#{value} was not considered in sync when array_matching == first")
-        end
-
-        # Now change it to all
-        property.array_matching = :all
-        assert_equal(:all, property.array_matching, "Property did not change value of array_matching")
-        assert(inst.match_all?, "match_all? returned false when array_matching is :all")
-
-        # Make sure we only get the first value back
-        assert_equal(vals, inst.should, "Returned wrong value when array_matching == all")
-
-        # And make sure any of these values is considered in sync
-        %w{one two three}.each do |value|
-            assert(! inst.insync?(value), "individual value #{value} was considered in sync when array_matching == all")
-        end
-        assert(inst.insync?(vals), "value array was not considered in sync when array_matching == all")
-    end
-end
-