Merge pull request #796 from flippercloud/standard-headers

Standardize Headers

Guardfile

@@ -21,6 +21,7 @@ guard 'rspec', rspec_options do
   watch('lib/flipper/api/middleware.rb') { 'spec/flipper/api_spec.rb' }
   watch(/shared_adapter_specs\.rb$/) { 'spec' }
   watch('spec/helper.rb') { 'spec' }
+  watch('lib/flipper/adapters/http/client.rb') { 'spec/flipper/adapters/http_spec.rb' }
 
   # To run all specs on every change... (useful with focus and fit)
   # watch(%r{.*}) { 'spec' }

lib/flipper/adapters/http/client.rb

@@ -7,36 +7,45 @@ module Adapters
     class Http
       class Client
         DEFAULT_HEADERS = {
-          'Content-Type' => 'application/json',
-          'Accept' => 'application/json',
-          'User-Agent' => "Flipper HTTP Adapter v#{VERSION}",
+          'content-type' => 'application/json',
+          'accept' => 'application/json',
+          'user-agent' => "Flipper HTTP Adapter v#{VERSION}",
         }.freeze
 
         HTTPS_SCHEME = "https".freeze
 
         CLIENT_FRAMEWORKS = {
-          rails: -> { Rails.version if defined?(Rails) },
-          sinatra: -> { Sinatra::VERSION if defined?(Sinatra) },
-          hanami: -> { Hanami::VERSION if defined?(Hanami) },
+          rails:    -> { Rails.version if defined?(Rails) },
+          sinatra:  -> { Sinatra::VERSION if defined?(Sinatra) },
+          hanami:   -> { Hanami::VERSION if defined?(Hanami) },
+          sidekiq:  -> { Sidekiq::VERSION if defined?(Sidekiq) },
+          good_job: -> { GoodJob::VERSION if defined?(GoodJob) },
         }
 
         attr_reader :uri, :headers
         attr_reader :basic_auth_username, :basic_auth_password
-        attr_reader :read_timeout, :open_timeout, :write_timeout, :max_retries, :debug_output
+        attr_reader :read_timeout, :open_timeout, :write_timeout
+        attr_reader :max_retries, :debug_output
 
         def initialize(options = {})
           @uri = URI(options.fetch(:url))
-          @headers = DEFAULT_HEADERS.merge(options[:headers] || {})
           @basic_auth_username = options[:basic_auth_username]
           @basic_auth_password = options[:basic_auth_password]
           @read_timeout = options[:read_timeout]
           @open_timeout = options[:open_timeout]
           @write_timeout = options[:write_timeout]
           @max_retries = options.key?(:max_retries) ? options[:max_retries] : 0
           @debug_output = options[:debug_output]
+
+          @headers = {}
+          DEFAULT_HEADERS.each { |key, value| add_header key, value }
+          if options[:headers]
+            options[:headers].each { |key, value| add_header key, value }
+          end
         end
 
         def add_header(key, value)
+          key = key.to_s.downcase.gsub('_'.freeze, '-'.freeze).freeze
           @headers[key] = value
         end
 
@@ -87,21 +96,21 @@ def build_http(uri)
 
         def build_request(http_method, uri, headers, options)
           request_headers = {
-            client_language: "ruby",
-            client_language_version: "#{RUBY_VERSION} p#{RUBY_PATCHLEVEL} (#{RUBY_RELEASE_DATE})",
-            client_platform: RUBY_PLATFORM,
-            client_engine: defined?(RUBY_ENGINE) ? RUBY_ENGINE : "",
-            client_pid: Process.pid.to_s,
-            client_thread: Thread.current.object_id.to_s,
-            client_hostname: Socket.gethostname,
+            'client-language' => "ruby",
+            'client-language-version' => "#{RUBY_VERSION} p#{RUBY_PATCHLEVEL} (#{RUBY_RELEASE_DATE})",
+            'client-platform' => RUBY_PLATFORM,
+            'client-engine' => defined?(RUBY_ENGINE) ? RUBY_ENGINE : "",
+            'client-pid' => Process.pid.to_s,
+            'client-thread' => Thread.current.object_id.to_s,
+            'client-hostname' => Socket.gethostname,
           }.merge(headers)
 
           body = options[:body]
           request = http_method.new(uri.request_uri)
           request.initialize_http_header(request_headers)
 
           client_frameworks.each do |framework, version|
-            request.add_field("Client-Framework", [framework, version].join("="))
+            request.add_field("client-framework", [framework, version].join("="))
           end
 
           request.body = body if body

lib/flipper/cloud/configuration.rb

@@ -163,8 +163,8 @@ def http_adapter
           max_retries: 0, # we'll handle retries ourselves
           debug_output: @debug_output,
           headers: {
-            flipper_cloud_token: @token,
-            accept_encoding: "gzip",
+            "flipper-cloud-token" => @token,
+            "accept-encoding" => "gzip",
           },
         })
       end
@@ -187,6 +187,11 @@ def setup_log(options)
       def setup_http(options)
         set_option :url, options, default: DEFAULT_URL
         set_option :debug_output, options, from_env: false
+
+        if @debug_output.nil? && Flipper::Typecast.to_boolean(ENV["FLIPPER_CLOUD_DEBUG_OUTPUT_STDOUT"])
+          @debug_output = STDOUT
+        end
+
         set_option :read_timeout, options, default: 5, typecast: :float, minimum: 0.1
         set_option :open_timeout, options, default: 2, typecast: :float, minimum: 0.1
         set_option :write_timeout, options, default: 5, typecast: :float, minimum: 0.1

lib/flipper/cloud/middleware.rb

@@ -41,12 +41,12 @@ def call!(env)
                 })
               rescue Flipper::Adapters::Http::Error => error
                 status = error.response.code.to_i == 402 ? 402 : 500
-                headers["Flipper-Cloud-Response-Error-Class"] = error.class.name
-                headers["Flipper-Cloud-Response-Error-Message"] = error.message
+                headers["flipper-cloud-response-error-class"] = error.class.name
+                headers["flipper-cloud-response-error-message"] = error.message
               rescue => error
                 status = 500
-                headers["Flipper-Cloud-Response-Error-Class"] = error.class.name
-                headers["Flipper-Cloud-Response-Error-Message"] = error.message
+                headers["flipper-cloud-response-error-class"] = error.class.name
+                headers["flipper-cloud-response-error-message"] = error.message
               end
             end
           rescue MessageVerifier::InvalidSignature

lib/flipper/cloud/telemetry/submitter.rb

@@ -78,8 +78,8 @@ def retry_with_backoff(attempts, &block)
 
         def submit(body)
           client = @cloud_configuration.http_client
-          client.add_header :schema_version, SCHEMA_VERSION
-          client.add_header :content_encoding, GZIP_ENCODING
+          client.add_header "schema-version", SCHEMA_VERSION
+          client.add_header "content-encoding", GZIP_ENCODING
 
           response = client.post PATH, body
           code = response.code.to_i

spec/flipper/adapters/http/client_spec.rb

@@ -0,0 +1,61 @@
+require "flipper/adapters/http/client"
+
+RSpec.describe Flipper::Adapters::Http::Client do
+  describe "#initialize" do
+    it "requires url" do
+      expect { described_class.new }.to raise_error(KeyError, "key not found: :url")
+    end
+
+    it "sets default headers" do
+      client = described_class.new(url: "http://example.com")
+      expect(client.headers).to eq({
+        'content-type' => 'application/json',
+        'accept' => 'application/json',
+        'user-agent' => "Flipper HTTP Adapter v#{Flipper::VERSION}",
+      })
+    end
+
+    it "adds custom headers" do
+      client = described_class.new(url: "http://example.com", headers: {'custom-header' => 'value'})
+      expect(client.headers).to include('custom-header' => 'value')
+    end
+
+    it "overrides default headers with custom headers" do
+      client = described_class.new(url: "http://example.com", headers: {'content-type' => 'text/plain'})
+      expect(client.headers['content-type']).to eq('text/plain')
+    end
+  end
+
+  describe "#add_header" do
+    it "can add string header" do
+      client = described_class.new(url: "http://example.com")
+      client.add_header("key", "value")
+      expect(client.headers.fetch("key")).to eq("value")
+    end
+
+    it "standardizes key to lowercase" do
+      client = described_class.new(url: "http://example.com")
+      client.add_header("Content-Type", "value")
+      expect(client.headers.fetch("content-type")).to eq("value")
+    end
+
+    it "standardizes key to dashes" do
+      client = described_class.new(url: "http://example.com")
+      client.add_header(:content_type, "value")
+      expect(client.headers.fetch("content-type")).to eq("value")
+    end
+
+    it "can add symbol header" do
+      client = described_class.new(url: "http://example.com")
+      client.add_header(:key, "value")
+      expect(client.headers.fetch("key")).to eq("value")
+    end
+
+    it "overrides existing header" do
+      client = described_class.new(url: "http://example.com")
+      client.add_header("key", "value 1")
+      client.add_header("key", "value 2")
+      expect(client.headers.fetch("key")).to eq("value 2")
+    end
+  end
+end

spec/flipper/adapters/http_spec.rb

@@ -11,7 +11,7 @@
 
   {
     basic: default_options.dup,
-    gzip: default_options.dup.merge(headers: { accept_encoding: 'gzip' }),
+    gzip: default_options.dup.merge(headers: { 'accept-encoding': 'gzip' }),
   }.each do |name, options|
     context "adapter (#{name} #{options.inspect})" do
       subject do
@@ -87,9 +87,9 @@
 
   it "sends default headers" do
     headers = {
-      'Accept' => 'application/json',
-      'Content-Type' => 'application/json',
-      'User-Agent' => "Flipper HTTP Adapter v#{Flipper::VERSION}",
+      'accept' => 'application/json',
+      'content-type' => 'application/json',
+      'user-agent' => "Flipper HTTP Adapter v#{Flipper::VERSION}",
     }
     stub_request(:get, "http://app.com/flipper/features/feature_panel")
       .with(headers: headers)
@@ -103,9 +103,17 @@
     stub_const("Rails", double(version: "7.1.0"))
     stub_const("Sinatra::VERSION", "3.1.0")
     stub_const("Hanami::VERSION", "0.7.2")
+    stub_const("GoodJob::VERSION", "3.21.5")
+    stub_const("Sidekiq::VERSION", "7.2.0")
 
     headers = {
-      "Client-Framework" => ["rails=7.1.0", "sinatra=3.1.0", "hanami=0.7.2"]
+      "client-framework" => [
+        "rails=7.1.0",
+        "sinatra=3.1.0",
+        "hanami=0.7.2",
+        "good_job=3.21.5",
+        "sidekiq=7.2.0",
+      ]
     }
 
     stub_request(:get, "http://app.com/flipper/features/feature_panel")
@@ -121,7 +129,7 @@
     stub_const("Sinatra::VERSION", "3.1.0")
 
     headers = {
-      "Client-Framework" => ["rails=7.1.0", "sinatra=3.1.0"]
+      "client-framework" => ["rails=7.1.0", "sinatra=3.1.0"]
     }
 
     stub_request(:get, "http://app.com/flipper/features/feature_panel")
@@ -289,7 +297,7 @@
     let(:options) do
       {
         url: 'http://app.com/mount-point',
-        headers: { 'X-Custom-Header' => 'foo' },
+        headers: { 'x-custom-header' => 'foo' },
         basic_auth_username: 'username',
         basic_auth_password: 'password',
         read_timeout: 100,
@@ -310,7 +318,7 @@
       subject.get(feature)
       expect(
         a_request(:get, 'http://app.com/mount-point/features/feature_panel')
-        .with(headers: { 'X-Custom-Header' => 'foo' })
+        .with(headers: { 'x-custom-header' => 'foo' })
       ).to have_been_made.once
     end
 

spec/flipper/cloud/configuration_spec.rb

@@ -86,6 +86,13 @@
     expect(instance.debug_output).to eq(STDOUT)
   end
 
+  it "defaults debug_output to STDOUT if FLIPPER_CLOUD_DEBUG_OUTPUT_STDOUT set to true" do
+    with_env "FLIPPER_CLOUD_DEBUG_OUTPUT_STDOUT" => "true" do
+      instance = described_class.new(required_options)
+    expect(instance.debug_output).to eq(STDOUT)
+    end
+  end
+
   it "defaults adapter block" do
     # The initial sync of http to local invokes this web request.
     stub_request(:get, /flippercloud\.io/).to_return(status: 200, body: "{}")
@@ -233,9 +240,9 @@
     stub = stub_request(:get, "https://www.flippercloud.io/adapter/features?exclude_gate_names=true").
       with({
         headers: {
-          'Flipper-Cloud-Token'=>'asdf',
+          'flipper-cloud-token'=>'asdf',
         },
-      }).to_return(status: 200, body: body, headers: {})
+      }).to_return(status: 200, body: body)
     instance = described_class.new(required_options)
     instance.sync
 

spec/flipper/cloud/dsl_spec.rb

@@ -18,7 +18,7 @@
     stub = stub_request(:get, "https://www.flippercloud.io/adapter/features?exclude_gate_names=true").
       with({
         headers: {
-          'Flipper-Cloud-Token'=>'asdf',
+          'flipper-cloud-token'=>'asdf',
         },
       }).to_return(status: 200, body: '{"features": {}}', headers: {})
     cloud_configuration = Flipper::Cloud::Configuration.new({
@@ -65,11 +65,11 @@
 
     it "sends writes to cloud and local" do
       add_stub = stub_request(:post, "https://www.flippercloud.io/adapter/features").
-        with({headers: {'Flipper-Cloud-Token'=>'asdf'}}).
-        to_return(status: 200, body: '{}', headers: {})
+        with({headers: {'flipper-cloud-token'=>'asdf'}}).
+        to_return(status: 200, body: '{}')
       enable_stub = stub_request(:post, "https://www.flippercloud.io/adapter/features/foo/boolean").
-        with(headers: {'Flipper-Cloud-Token'=>'asdf'}).
-        to_return(status: 200, body: '{}', headers: {})
+        with(headers: {'flipper-cloud-token'=>'asdf'}).
+        to_return(status: 200, body: '{}')
 
       subject.enable(:foo)
 

spec/flipper/cloud/middleware_spec.rb

@@ -101,8 +101,8 @@
       post '/', request_body, env
 
       expect(last_response.status).to eq(402)
-      expect(last_response.headers["Flipper-Cloud-Response-Error-Class"]).to eq("Flipper::Adapters::Http::Error")
-      expect(last_response.headers["Flipper-Cloud-Response-Error-Message"]).to include("Failed with status: 402")
+      expect(last_response.headers["flipper-cloud-response-error-class"]).to eq("Flipper::Adapters::Http::Error")
+      expect(last_response.headers["flipper-cloud-response-error-message"]).to include("Failed with status: 402")
       expect(stub).to have_been_requested
     end
   end
@@ -124,8 +124,8 @@
       post '/', request_body, env
 
       expect(last_response.status).to eq(500)
-      expect(last_response.headers["Flipper-Cloud-Response-Error-Class"]).to eq("Flipper::Adapters::Http::Error")
-      expect(last_response.headers["Flipper-Cloud-Response-Error-Message"]).to include("Failed with status: 503")
+      expect(last_response.headers["flipper-cloud-response-error-class"]).to eq("Flipper::Adapters::Http::Error")
+      expect(last_response.headers["flipper-cloud-response-error-message"]).to include("Failed with status: 503")
       expect(stub).to have_been_requested
     end
   end
@@ -147,8 +147,8 @@
       post '/', request_body, env
 
       expect(last_response.status).to eq(500)
-      expect(last_response.headers["Flipper-Cloud-Response-Error-Class"]).to eq("Net::OpenTimeout")
-      expect(last_response.headers["Flipper-Cloud-Response-Error-Message"]).to eq("execution expired")
+      expect(last_response.headers["flipper-cloud-response-error-class"]).to eq("Net::OpenTimeout")
+      expect(last_response.headers["flipper-cloud-response-error-message"]).to eq("execution expired")
       expect(stub).to have_been_requested
     end
   end
@@ -277,13 +277,13 @@ def stub_request_for_token(token, status: 200)
     stub = stub_request(:get, "https://www.flippercloud.io/adapter/features?exclude_gate_names=true").
       with({
         headers: {
-          'Flipper-Cloud-Token' => token,
+          'flipper-cloud-token' => token,
         },
       })
     if status == :timeout
       stub.to_timeout
     else
-      stub.to_return(status: status, body: response_body, headers: {})
+      stub.to_return(status: status, body: response_body)
     end
   end
 end