Save etag and send it for get_all

Author: jnunemaker

lib/flipper/adapters/http.rb

@@ -22,6 +22,8 @@ def initialize(options = {})
                              write_timeout: options[:write_timeout],
                              max_retries: options[:max_retries],
                              debug_output: options[:debug_output])
+        @last_get_all_etag = nil
+        @last_get_all_result = nil
       end
 
       def get(feature)
@@ -58,9 +60,27 @@ def get_multi(features)
       def get_all(cache_bust: false)
         path = "/features?exclude_gate_names=true"
         path += "&_cb=#{Time.now.to_i}" if cache_bust
-        response = @client.get(path)
+
+        # Pass If-None-Match header if we have an ETag
+        options = {}
+        if @last_get_all_etag
+          options[:headers] = { if_none_match: @last_get_all_etag }
+        end
+
+        response = @client.get(path, options)
+
+        # Handle 304 Not Modified - return cached result
+        if response.is_a?(Net::HTTPNotModified)
+          return @last_get_all_result if @last_get_all_result
+          # If we somehow got 304 without a cached result, treat as error
+          raise Error, response
+        end
+
         raise Error, response unless response.is_a?(Net::HTTPOK)
 
+        # Store ETag from response for future requests
+        @last_get_all_etag = response['etag'] if response['etag']
+
         parsed_response = response.body.empty? ? {} : Typecast.from_json(response.body)
         parsed_features = parsed_response['features'] || []
         gates_by_key = parsed_features.each_with_object({}) do |parsed_feature, hash|
@@ -73,6 +93,9 @@ def get_all(cache_bust: false)
           feature = Feature.new(key, self)
           result[feature.key] = result_for_feature(feature, gates_by_key[feature.key])
         end
+
+        # Cache the result for 304 responses
+        @last_get_all_result = result
         result
       end
 

lib/flipper/adapters/http/client.rb

@@ -45,12 +45,19 @@ def initialize(options = {})
         end
 
         def add_header(key, value)
-          key = key.to_s.downcase.gsub('_'.freeze, '-'.freeze).freeze
-          @headers[key] = value
+          @headers[normalize_header_key(key)] = value
         end
 
-        def get(path)
-          perform Net::HTTP::Get, path, @headers
+        def get(path, options = {})
+          headers = @headers.dup
+
+          if options[:headers]
+            options[:headers].each do |key, value|
+              headers[normalize_header_key(key)] = value
+            end
+          end
+
+          perform Net::HTTP::Get, path, headers
         end
 
         def post(path, body = nil)
@@ -125,6 +132,10 @@ def build_request(http_method, uri, headers, options)
         def client_frameworks
           CLIENT_FRAMEWORKS.transform_values { |detect| detect.call rescue nil }.compact
         end
+
+        def normalize_header_key(key)
+          key.to_s.downcase.gsub('_'.freeze, '-'.freeze)
+        end
       end
     end
   end

spec/flipper/adapters/http_spec.rb

@@ -185,6 +185,112 @@
         adapter.get_all
       }.to raise_error(Flipper::Adapters::Http::Error)
     end
+
+    it "stores ETag and sends If-None-Match on subsequent requests" do
+      features_response = {
+        "features" => [
+          {
+            "key" => "search",
+            "gates" => [
+              {"key" => "boolean", "value" => true}
+            ]
+          }
+        ]
+      }
+
+      # First request - server returns ETag
+      stub_request(:get, "http://app.com/flipper/features?exclude_gate_names=true")
+        .to_return(
+          status: 200,
+          body: JSON.generate(features_response),
+          headers: { 'ETag' => '"abc123"' }
+        )
+
+      adapter = described_class.new(url: 'http://app.com/flipper')
+      result = adapter.get_all
+
+      expect(result).to have_key("search")
+
+      # Second request - should send If-None-Match header
+      stub_request(:get, "http://app.com/flipper/features?exclude_gate_names=true")
+        .with(headers: { 'If-None-Match' => '"abc123"' })
+        .to_return(
+          status: 200,
+          body: JSON.generate(features_response),
+          headers: { 'ETag' => '"abc123"' }
+        )
+
+      etag_result = adapter.get_all
+
+      expect(etag_result).to eq(result)
+      expect(etag_result).to have_key("search")
+      expect(
+        a_request(:get, "http://app.com/flipper/features?exclude_gate_names=true")
+        .with(headers: { 'If-None-Match' => '"abc123"' })
+      ).to have_been_made.once
+    end
+
+    it "returns cached result when server returns 304 Not Modified" do
+      features_response = {
+        "features" => [
+          {
+            "key" => "search",
+            "gates" => [
+              {"key" => "boolean", "value" => true}
+            ]
+          }
+        ]
+      }
+
+      # First request - server returns ETag
+      stub_request(:get, "http://app.com/flipper/features?exclude_gate_names=true")
+        .to_return(
+          status: 200,
+          body: JSON.generate(features_response),
+          headers: { 'ETag' => '"abc123"' }
+        )
+
+      adapter = described_class.new(url: 'http://app.com/flipper')
+      first_result = adapter.get_all
+
+      expect(first_result).to have_key("search")
+
+      # Second request - server returns 304
+      stub_request(:get, "http://app.com/flipper/features?exclude_gate_names=true")
+        .with(headers: { 'If-None-Match' => '"abc123"' })
+        .to_return(status: 304, headers: { 'ETag' => '"abc123"' })
+
+      second_result = adapter.get_all
+
+      # Should return the cached result
+      expect(second_result).to eq(first_result)
+      expect(second_result).to have_key("search")
+    end
+
+    it "raises error when 304 received without cached result" do
+      # Server returns 304 without any prior request
+      stub_request(:get, "http://app.com/flipper/features?exclude_gate_names=true")
+        .to_return(status: 304)
+
+      adapter = described_class.new(url: 'http://app.com/flipper')
+      expect {
+        adapter.get_all
+      }.to raise_error(Flipper::Adapters::Http::Error)
+    end
+
+    it "does not send If-None-Match for other endpoints" do
+      stub_request(:get, "http://app.com/flipper/features/search")
+        .to_return(status: 404)
+
+      adapter = described_class.new(url: 'http://app.com/flipper')
+      adapter.get(flipper[:search])
+
+      # Verify no If-None-Match header was sent
+      expect(
+        a_request(:get, "http://app.com/flipper/features/search")
+        .with { |req| req.headers['If-None-Match'].nil? }
+      ).to have_been_made.once
+    end
   end
 
   describe "#features" do