Merge pull request #175 from launchdarkly/eb/sc-132086/cache-nil-big-seg

always cache big segment query result even if it's nil

Author: eli-darkly

lib/ldclient-rb/impl/big_segments.rb

@@ -11,6 +11,10 @@ module Impl
     BigSegmentMembershipResult = Struct.new(:membership, :status)
 
     class BigSegmentStoreManager
+      # use this as a singleton whenever a membership query returns nil; it's safe to reuse it because
+      # we will never modify the membership properties after they're queried
+      EMPTY_MEMBERSHIP = {}
+
       def initialize(big_segments_config, logger)
         @store = big_segments_config.store
         @stale_after_millis = big_segments_config.stale_after * 1000
@@ -38,6 +42,7 @@ def get_user_membership(user_key)
         if !membership
           begin
             membership = @store.get_membership(BigSegmentStoreManager.hash_for_user_key(user_key))
+            membership = EMPTY_MEMBERSHIP if membership.nil?
             @cache[user_key] = membership
           rescue => e
             LaunchDarkly::Util.log_exception(@logger, "Big Segment store membership query returned error", e)

spec/impl/big_segments_spec.rb

@@ -3,6 +3,7 @@
 
 require "concurrent/atomics"
 
+require "mock_components"
 require "spec_helper"
 
 module LaunchDarkly
@@ -50,6 +51,7 @@ def with_manager(config)
           store = double
           expect(store).to receive(:get_metadata).at_least(:once).and_return(always_up_to_date)
           expect(store).to receive(:get_membership).with(user_hash).once.and_return(expected_membership)
+          # the ".once" on this mock expectation is what verifies that the cache is working; there should only be one query
           allow(store).to receive(:stop)
 
           with_manager(BigSegmentsConfig.new(store: store)) do |m|
@@ -59,6 +61,36 @@ def with_manager(config)
           end
         end
 
+        it "can cache a nil result" do
+          store = double
+          expect(store).to receive(:get_metadata).at_least(:once).and_return(always_up_to_date)
+          expect(store).to receive(:get_membership).with(user_hash).once.and_return(nil)
+          # the ".once" on this mock expectation is what verifies that the cache is working; there should only be one query
+          allow(store).to receive(:stop)
+
+          with_manager(BigSegmentsConfig.new(store: store)) do |m|
+            expected_result = BigSegmentMembershipResult.new({}, BigSegmentsStatus::HEALTHY)
+            expect(m.get_user_membership(user_key)).to eq(expected_result)
+            expect(m.get_user_membership(user_key)).to eq(expected_result)
+          end
+        end
+
+        it "cache can expire" do
+          expected_membership = { 'key1' => true, 'key2' => true }
+          store = double
+          expect(store).to receive(:get_metadata).at_least(:once).and_return(always_up_to_date)
+          expect(store).to receive(:get_membership).with(user_hash).twice.and_return(expected_membership)
+          # the ".twice" on this mock expectation is what verifies that the cached result expired
+          allow(store).to receive(:stop)
+
+          with_manager(BigSegmentsConfig.new(store: store, user_cache_time: 0.01)) do |m|
+            expected_result = BigSegmentMembershipResult.new(expected_membership, BigSegmentsStatus::HEALTHY)
+            expect(m.get_user_membership(user_key)).to eq(expected_result)
+            sleep(0.1)
+            expect(m.get_user_membership(user_key)).to eq(expected_result)
+          end
+        end
+
         it "with stale status" do
           expected_membership = { 'key1' => true, 'key2' => true }
           store = double