More refactoring and add tests

lib/ldclient-rb/ldclient.rb

@@ -56,19 +56,23 @@ def flush
       end
 
       if !events.empty?
-        res = log_timings("Flush events") {
-          next @client.post (@config.base_uri + "/api/events/bulk") do |req|
-            req.headers['Authorization'] = 'api_key ' + @api_key
-            req.headers['User-Agent'] = 'RubyClient/' + LaunchDarkly::VERSION
-            req.headers['Content-Type'] = 'application/json'
-            req.body = events.to_json
-            req.options.timeout = @config.read_timeout
-            req.options.open_timeout = @config.connect_timeout
-          end
-        }
-        if res.status != 200
-          @config.logger.error("[LDClient] Unexpected status code while processing events: #{res.status}")
+        post_flushed_events(events)
+      end
+    end
+
+    def post_flushed_events(events)
+      res = log_timings("Flush events") {
+        next @client.post (@config.base_uri + "/api/events/bulk") do |req|
+          req.headers['Authorization'] = 'api_key ' + @api_key
+          req.headers['User-Agent'] = 'RubyClient/' + LaunchDarkly::VERSION
+          req.headers['Content-Type'] = 'application/json'
+          req.body = events.to_json
+          req.options.timeout = @config.read_timeout
+          req.options.open_timeout = @config.connect_timeout
         end
+      }
+      if res.status/100 != 2
+        @config.logger.error("[LDClient] Unexpected status code while processing events: #{res.status}")
       end
     end
 
@@ -122,9 +126,7 @@ def get_flag?(key, user, default=false)
     #
     # @return [Boolean] whether or not the flag should be enabled, or the default value if the flag is disabled on the LaunchDarkly control panel
     def toggle?(key, user, default=false)
-      if @offline
-        return default
-      end
+      return default if @offline
 
       unless user
         @config.logger.error("[LDClient] Must specify user")
@@ -136,14 +138,7 @@ def toggle?(key, user, default=false)
       end
 
       if @config.stream? and @stream_processor.initialized?
-        feature = get_flag_stream(key)
-        if @config.debug_stream?
-          polled = get_flag_int(key)
-          diff = HashDiff.diff(feature, polled)
-          if not diff.empty?
-            @config.logger.error("Streamed flag differs from polled flag " + diff.to_s)
-          end
-        end
+        feature = get_streamed_flag(key)
       else
         feature = get_flag_int(key)
       end
@@ -159,9 +154,8 @@ def toggle?(key, user, default=false)
     end
 
     def add_event(event)
-      if @offline
-        return
-      end
+      return if @offline
+
       if @queue.length < @config.capacity
         event[:creationDate] = (Time.now.to_f * 1000).to_i
         @queue.push(event)
@@ -220,13 +214,25 @@ def feature_keys
     def get_features
       res = make_request '/api/features'
 
-      if res.status == 200 then
+      if res.status/100 == 2
         return JSON.parse(res.body, symbolize_names: true)[:items]
       else
         @config.logger.error("[LDClient] Unexpected status code #{res.status}")
       end
     end
 
+    def get_streamed_flag(key)
+      feature = get_flag_stream(key)
+      if @config.debug_stream?
+        polled = get_flag_int(key)
+        diff = HashDiff.diff(feature, polled)
+        if not diff.empty?
+          @config.logger.error("Streamed flag differs from polled flag " + diff.to_s)
+        end
+      end
+      feature
+    end
+
     def get_flag_stream(key)
       @stream_processor.get_feature(key)
     end
@@ -246,7 +252,7 @@ def get_flag_int(key)
         return nil
       end
 
-      if res.status != 200
+      if res.status/100 != 2
         @config.logger.error("[LDClient] Unexpected status code #{res.status}")
         return nil
       end
@@ -265,13 +271,10 @@ def make_request(path)
     end
 
     def param_for_user(feature, user)
-      if !! user[:key]
-        id_hash = user[:key]
-      else
-        return nil
-      end
+      return nil unless user[:key]
 
-      if !! user[:secondary]
+      id_hash = user[:key]
+      if user[:secondary]
         id_hash += '.' + user[:secondary]
       end
 
@@ -285,19 +288,14 @@ def match_target?(target, user)
       attrib = target[:attribute].to_sym
 
       if BUILTINS.include?(attrib)
-        if user[attrib]
-          u_value = user[attrib]
-          return target[:values].include? u_value
-        else
-          return false
-        end
+        return false unless user[attrib]
+
+        u_value = user[attrib]
+        return target[:values].include? u_value
       else # custom attribute
-        unless !! user[:custom]
-          return false
-        end
-        unless user[:custom].include? attrib
-          return false
-        end
+        return false unless user[:custom]
+        return false unless user[:custom].include? attrib
+
         u_value = user[:custom][attrib]
         if u_value.is_a? Array
           return ! ((target[:values] & u_value).empty?)
@@ -311,12 +309,19 @@ def match_target?(target, user)
     end
 
     def match_user?(variation, user)
-      if !!variation[:userTarget]
+      if variation[:userTarget]
         return match_target?(variation[:userTarget], user)
       end
       return false
     end
 
+    def find_user_match(feature, user)
+      feature[:variations].each do |variation|
+        return variation[:value] if match_user?(variation, user)
+      end
+      return nil
+    end
+
     def match_variation?(variation, user)
       variation[:targets].each do |target|
         if !!variation[:userTarget] and target[:attribute].to_sym == :key
@@ -330,46 +335,42 @@ def match_variation?(variation, user)
       return false
     end
 
-    def evaluate(feature, user)
-      if feature.nil? || !feature[:on]
-        return nil
-      end
-
-      param = param_for_user(feature, user)
-
-      if param.nil?
-        return nil
-      end
-
-      feature[:variations].each do |variation|
-        if match_user?(variation, user)
-          return variation[:value]
-        end
-      end
-
+    def find_target_match(feature, user)
       feature[:variations].each do |variation|
-        if match_variation?(variation, user)
-          return variation[:value]
-        end
+        return variation[:value] if match_variation?(variation, user)
       end
+      return nil
+    end
 
+    def find_weight_match(feature, param)
       total = 0.0
       feature[:variations].each do |variation|
         total += variation[:weight].to_f / 100.0
 
-        if param < total
-          return variation[:value]
-        end
+        return variation[:value] if param < total
       end
 
       return nil
+    end
+
+    def evaluate(feature, user)
+      return nil if feature.nil?
+      return nil unless feature[:on]
+
+      param = param_for_user(feature, user)
+      return nil if param.nil?
+
+      value = find_user_match(feature, user)
+      return value if !value.nil?
 
+      value = find_target_match(feature, user)
+      return value if !value.nil?
+
+      find_weight_match(feature, param)
     end
 
     def log_timings(label, &block)
-      if !@config.log_timings? || !@config.logger.debug?
-        return block.call
-      end
+      return block.call unless @config.log_timings? && @config.logger.debug?
       res = nil
       exn = nil
       bench = Benchmark.measure {
@@ -380,13 +381,11 @@ def log_timings(label, &block)
         end
       }
       @config.logger.debug { "[LDClient] #{label} timing: #{bench}".chomp }
-      if exn
-        raise exn
-      end
+      raise exn if exn
       return res
     end
 
-    private :add_event, :get_flag_stream, :get_flag_int, :make_request, :param_for_user, :match_target?, :match_user?, :match_variation?, :evaluate, :create_worker, :log_timings
+    private :post_flushed_events, :add_event, :get_streamed_flag, :get_flag_stream, :get_flag_int, :make_request, :param_for_user, :match_target?, :match_user?, :match_variation?, :evaluate, :create_worker, :log_timings
 
   end
 end

lib/ldclient-rb/stream.rb

@@ -104,14 +104,10 @@ def start_reactor
     def start
       # Try to start the reactor. If it's not started, we shouldn't start
       # the stream processor
-      if not start_reactor
-        return
-      end
+      return if not start_reactor
 
       # If someone else booted the stream processor connection, just return
-      if not @started.make_true
-        return
-      end
+      return unless @started.make_true
 
       # If we're the first and only thread to set started, boot
       # the stream processor connection
@@ -126,9 +122,9 @@ def boot_event_manager
                                   {'Accept' => 'text/event-stream',
                                    'Authorization' => 'api_key ' + @api_key,
                                    'User-Agent' => 'RubyClient/' + LaunchDarkly::VERSION})
-      source.on PUT { |message| process_message(message, PUT) }
-      source.on PATCH { |message| process_message(message, PATCH) }
-      source.on DELETE { |message| process_message(message, DELETE) }
+      source.on(PUT) { |message| process_message(message, PUT) }
+      source.on(PATCH) { |message| process_message(message, PATCH) }
+      source.on(DELETE) { |message| process_message(message, DELETE) }
       source.error do |error|
         @config.logger.info("[LDClient] Error subscribing to stream API: #{error}")
         set_disconnected

spec/fixtures/feature.json

@@ -0,0 +1,67 @@
+{
+   "name":"New recommendations engine",
+   "key":"engine.enable",
+   "kind":"flag",
+   "salt":"ZW5naW5lLmVuYWJsZQ==",
+   "on":true,
+   "variations":[
+      {
+         "value":true,
+         "weight":31,
+         "targets":[
+            {
+               "attribute":"groups",
+               "op":"in",
+               "values":[
+                  "Microsoft"
+               ]
+            }
+         ],
+         "userTarget":{
+            "attribute":"key",
+            "op":"in",
+            "values":[
+               "foo@bar.com",
+               "Abbey.Orkwis@example.com",
+               "Abbie.Stolte@example.com",
+               "44d2781c-5985-4d89-b07a-0dffbd24126f",
+               "0ffe4f0c-7aa9-4621-a87c-abe1c051abd8",
+               "f52d99be-6a40-4cdd-a7b4-0548834fcbe5",
+               "Vernetta.Belden@example.com",
+               "c9d591bd-ea1f-465f-86d2-239ea41d0f0f",
+               "870745092"
+            ]
+         }
+      },
+      {
+         "value":false,
+         "weight":69,
+         "targets":[
+            {
+               "attribute":"key",
+               "op":"in",
+               "values":[
+                  "Alida.Caples@example.com"
+               ]
+            },
+            {
+               "attribute":"groups",
+               "op":"in",
+               "values":[
+
+               ]
+            }
+         ],
+         "userTarget":{
+            "attribute":"key",
+            "op":"in",
+            "values":[
+               "Alida.Caples@example.com"
+            ]
+         }
+      }
+   ],
+   "ttl":0,
+   "commitDate":"2015-05-10T06:06:45.381Z",
+   "creationDate":"2014-09-02T20:39:18.61Z"
+}

spec/fixtures/user.json

@@ -0,0 +1,9 @@
+{
+    "key":"user@test.com",
+    "custom":{
+        "groups":[
+            "microsoft",
+            "google"
+        ]
+    }
+}