lib/flapjack/data/notification_rule.rb in flapjack-0.7.6 vs lib/flapjack/data/notification_rule.rb in flapjack-0.7.7

@@ -35,17 +35,17 @@
       # replacing save! etc
       def self.add(rule_data, options = {})
         raise "Redis connection not set" unless redis = options[:redis]
 
         rule_id = SecureRandom.uuid
-        self.add_or_update(rule_data.merge(:id => rule_id), :redis => redis)
+        self.add_or_update(rule_data.merge(:id => rule_id), options)
         self.find_by_id(rule_id, :redis => redis)
       end
 
-      def update(rule_data)
-        return false unless self.class.add_or_update(rule_data.merge(:id => @id),
-          :redis => @redis)
+      def update(rule_data, opts = {})
+        return false unless self.class.add_or_update({:contact_id => @contact_id}.merge(rule_data.merge(:id => @id)),
+          :redis => @redis, :logger => opts[:logger])
         refresh
         true
       end
 
       # NB: ice_cube doesn't have much rule data validation, and has
@@ -106,11 +106,16 @@
       end
 
       def self.add_or_update(rule_data, options = {})
         redis = options[:redis]
         raise "a redis connection must be supplied" unless redis
+        logger = options[:logger]
 
+        # make some assumptions about the incoming data
+        rule_data[:warning_blackhole]  = rule_data[:warning_blackhole] || false
+        rule_data[:critical_blackhole] = rule_data[:critical_blackhole] || false
+
         return unless self.validate_data(rule_data, options)
 
         # whitelisting fields, rather than passing through submitted data directly
         json_rule_data = {
           :id                 => rule_data[:id].to_s,
@@ -121,10 +126,11 @@
           :warning_media      => Yajl::Encoder.encode(rule_data[:warning_media]),
           :critical_media     => Yajl::Encoder.encode(rule_data[:critical_media]),
           :warning_blackhole  => rule_data[:warning_blackhole],
           :critical_blackhole => rule_data[:critical_blackhole],
         }
+        logger.debug("NotificationRule#add_or_update json_rule_data: #{json_rule_data.inspect}") if logger
 
         redis.sadd("contact_notification_rules:#{json_rule_data[:contact_id]}",
                    json_rule_data[:id])
         redis.hmset("notification_rule:#{json_rule_data[:id]}",
                     *json_rule_data.flatten)
@@ -198,51 +204,56 @@
       def self.validate_data(d, options = {})
         # hash with validation => error_message
         validations = {proc { d.has_key?(:id) } =>
                        "id not set",
 
-                       proc { d.has_key?(:entities) &&
-                              d[:entities].is_a?(Array) &&
-                              d[:entities].all? {|e| e.is_a?(String)} } =>
+                       proc { !d.has_key?(:entities) ||
+                              ( d[:entities].nil? ||
+                                d[:entities].is_a?(Array) &&
+                                d[:entities].all? {|e| e.is_a?(String)} ) } =>
                        "entities must be a list of strings",
 
-                       proc { d.has_key?(:entity_tags) &&
-                              d[:entity_tags].is_a?(Array) &&
-                              d[:entity_tags].all? {|et| et.is_a?(String)}} =>
+                       proc { !d.has_key?(:entity_tags) ||
+                              ( d[:entity_tags].nil? ||
+                                d[:entity_tags].is_a?(Array) &&
+                                d[:entity_tags].all? {|et| et.is_a?(String)} ) } =>
                        "entity_tags must be a list of strings",
 
                        proc { (d.has_key?(:entities) &&
                                d[:entities].is_a?(Array) &&
                                (d[:entities].size > 0)) ||
                               (d.has_key?(:entity_tags) &&
                                d[:entity_tags].is_a?(Array) &&
                                (d[:entity_tags].size > 0)) } =>
                        "entities or entity tags must have at least one value",
 
-                       proc { d.has_key?(:time_restrictions) &&
-                              d[:time_restrictions].all? {|tr|
-                                !!prepare_time_restriction(symbolize(tr))
-                              }
+                       proc { !d.has_key?(:time_restrictions) ||
+                              ( d[:time_restrictions].nil? ||
+                                d[:time_restrictions].all? {|tr|
+                                  !!prepare_time_restriction(symbolize(tr))
+                                } )
                             } =>
                        "time restrictions are invalid",
 
                        # TODO should the media types be checked against a whitelist?
-                       proc { d.has_key?(:warning_media) &&
-                              d[:warning_media].is_a?(Array) &&
-                              d[:warning_media].all? {|et| et.is_a?(String)}} =>
+                       proc { !d.has_key?(:warning_media) ||
+                              ( d[:warning_media].nil? ||
+                                d[:warning_media].is_a?(Array) &&
+                                d[:warning_media].all? {|et| et.is_a?(String)} ) } =>
                        "warning_media must be a list of strings",
 
-                       proc { d.has_key?(:critical_media) &&
-                              d[:critical_media].is_a?(Array) &&
-                              d[:critical_media].all? {|et| et.is_a?(String)}} =>
+                       proc { !d.has_key?(:critical_media) ||
+                              ( d[:critical_media].nil? ||
+                                d[:critical_media].is_a?(Array) &&
+                                d[:critical_media].all? {|et| et.is_a?(String)} ) } =>
                        "critical_media must be a list of strings",
 
-                       proc { d.has_key?(:warning_blackhole) &&
+                       proc { !d.has_key?(:warning_blackhole) ||
                               [TrueClass, FalseClass].include?(d[:warning_blackhole].class) } =>
                        "warning_blackhole must be true or false",
 
-                       proc { d.has_key?(:critical_blackhole) &&
+                       proc { !d.has_key?(:critical_blackhole) ||
                               [TrueClass, FalseClass].include?(d[:critical_blackhole].class) } =>
                        "critical_blackhole must be true or false",
                       }
 
         errors = validations.keys.inject([]) {|ret,vk|
@@ -253,9 +264,10 @@
         return true if errors.empty?
 
         if logger = options[:logger]
           error_str = errors.join(", ")
           logger.info "validation error: #{error_str}"
+          logger.debug "rule failing validations: #{d.inspect}"
         end
         false
       end
 
       def refresh