Regex detection for Puppet configuration

Question

I've got a PR to add a feature to a Ruby application (Puppet) to allow Regex detection.

Right now, the method only does comparison on exact string matching:

defaultfor :operatingsystem => :fedora, :operatingsystemmajrelease => ['22', '23', '24']

This is annoying, as we have to add in new values everytime a new Fedora release comes out, for example.

I've extended it to match on a Regex:

defaultfor :operatingsystem => :fedora, :operatingsystemmajrelease => /^2[2-9]$/

The old method looks like this:

def self.fact_match(fact, values)
 values = [values] unless values.is_a? Array
 values.map! { |v| v.to_s.downcase.intern }
 if fval = Facter.value(fact).to_s and fval != ""
 fval = fval.to_s.downcase.intern
 values.include?(fval)
 else
 false
 end
end

This is my current proof-of-concept code, which will match both regex and strings, but it feels off.

def self.fact_match(fact, values)
 values = [values] unless values.is_a? Array
 values.map! do |v|
 if v.is_a? Regexp
 v
 else
 v.to_s.downcase.intern
 end
 end
 if fval = Facter.value(fact).to_s and fval != ""
 fval = fval.to_s.downcase.intern
 if values.any? {|v| v.is_a? Regexp }
 regex_match = Regexp.union(values)
 fval =~ regex_match
 else
 values.include?(fval)
 end
 else
 false
 end
end

I feel like there's a much easier way of detecting if it's a Regex or not.

Answer 1

You're working too hard, trying to distinguish strings from regular expressions. Rather, you should take advantage of the fact that the === operator for a RegExp and the === operator for a String both express the idea of a "match", which is what you want here.

I don't understand why you need to #intern all the strings.

I don't see why the fval = Facter.value(fact).to_s assignment appears inside an if condition. The result of the assignment would only be false if Facter.value(fact).to_s is false or nil, which can never happen. (Even nil.to_s results in an empty string.)

To treat values as an array regardless of whether a scalar or an array was passed in, you can use the [*values] idiom.

def self.fact_match(fact, values)
 fact_val = Facter.value(fact).to_s.downcase
 fact_val != "" and [*values].any? { |v| v === fact_val }
end

In addition to supporting literal case-insensitive matches and regular expression matches, also consider supporting Ranges.

• Peter Souter: That's a lot cleaner, thanks! Makes a lot more sense. Looking at the commit for that code, it looks like that extra intern logic was added when each defaultfor became a hash of multiple different values. Indeed, when I change the logic to the code given, everything works ok except for the unit tests for multiple defaultfor values... I'll have to investigate deeper!
• Wayne Conrad: I'm gladyou answered with the === idiom. It's one of my favorites. Also, the [*values] idiom can also be expressed as Array(values)