From b6ed2a8ab7401c78c39c9610456621099db6730c Mon Sep 17 00:00:00 2001
From: Leonel Galan <leonelgalan@gmail.com>
Date: Thu, 3 Aug 2017 16:18:44 -0400
Subject: [PATCH] Replaces thread storage with request_store
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fixes issue #197. Under load it was possible for a thread to handle
a second request without clearing the variables in the thread. This
caused log lines to have the incorrect values for `: lograge_location`.

@abrisse suggested we used `request_store`, after a brief examination
of this gem it seems that it’s well maintained and it solves our
problem.
---
 lib/lograge/log_subscriber.rb      | 15 ++++++++-------
 lograge.gemspec                    |  1 +
 spec/lograge_logsubscriber_spec.rb | 12 ++++++------
 3 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/lib/lograge/log_subscriber.rb b/lib/lograge/log_subscriber.rb
index ead52428..af17fd8a 100644
--- a/lib/lograge/log_subscriber.rb
+++ b/lib/lograge/log_subscriber.rb
@@ -2,6 +2,7 @@
 require 'action_pack'
 require 'active_support/core_ext/class/attribute'
 require 'active_support/log_subscriber'
+require 'request_store'
 
 module Lograge
   class RequestLogSubscriber < ActiveSupport::LogSubscriber
@@ -16,12 +17,12 @@ def process_action(event)
     end
 
     def redirect_to(event)
-      Thread.current[:lograge_location] = event.payload[:location]
+      RequestStore.store[:lograge_location] = event.payload[:location]
     end
 
     def unpermitted_parameters(event)
-      Thread.current[:lograge_unpermitted_params] ||= []
-      Thread.current[:lograge_unpermitted_params].concat(event.payload[:keys])
+      RequestStore.store[:lograge_unpermitted_params] ||= []
+      RequestStore.store[:lograge_unpermitted_params].concat(event.payload[:keys])
     end
 
     def logger
@@ -98,18 +99,18 @@ def extract_runtimes(event, payload)
     end
 
     def extract_location
-      location = Thread.current[:lograge_location]
+      location = RequestStore.store[:lograge_location]
       return {} unless location
 
-      Thread.current[:lograge_location] = nil
+      RequestStore.store[:lograge_location] = nil
       { location: location }
     end
 
     def extract_unpermitted_params
-      unpermitted_params = Thread.current[:lograge_unpermitted_params]
+      unpermitted_params = RequestStore.store[:lograge_unpermitted_params]
       return {} unless unpermitted_params
 
-      Thread.current[:lograge_unpermitted_params] = nil
+      RequestStore.store[:lograge_unpermitted_params] = nil
       { unpermitted_params: unpermitted_params }
     end
   end
diff --git a/lograge.gemspec b/lograge.gemspec
index 2e485321..4b6cd871 100644
--- a/lograge.gemspec
+++ b/lograge.gemspec
@@ -18,4 +18,5 @@ Gem::Specification.new do |s|
   s.add_runtime_dependency 'activesupport', '>= 4', '< 5.2'
   s.add_runtime_dependency 'actionpack',    '>= 4', '< 5.2'
   s.add_runtime_dependency 'railties',      '>= 4', '< 5.2'
+  s.add_runtime_dependency 'request_store', '~> 1.0'
 end
diff --git a/spec/lograge_logsubscriber_spec.rb b/spec/lograge_logsubscriber_spec.rb
index c7f939f3..11d4c8b7 100644
--- a/spec/lograge_logsubscriber_spec.rb
+++ b/spec/lograge_logsubscriber_spec.rb
@@ -74,7 +74,7 @@
 
     it 'stores the location in a thread local variable' do
       subscriber.redirect_to(redirect_event)
-      expect(Thread.current[:lograge_location]).to eq('http://example.com')
+      expect(RequestStore.store[:lograge_location]).to eq('http://example.com')
     end
   end
 
@@ -91,7 +91,7 @@
 
     it 'stores the parameters in a thread local variable' do
       subscriber.unpermitted_parameters(unpermitted_parameters_event)
-      expect(Thread.current[:lograge_unpermitted_params]).to eq(%w(foo bar))
+      expect(RequestStore.store[:lograge_unpermitted_params]).to eq(%w(foo bar))
     end
   end
 
@@ -167,7 +167,7 @@
 
     context 'with a redirect' do
       before do
-        Thread.current[:lograge_location] = 'http://www.example.com'
+        RequestStore.store[:lograge_location] = 'http://www.example.com'
       end
 
       it 'adds the location to the log line' do
@@ -177,7 +177,7 @@
 
       it 'removes the thread local variable' do
         subscriber.process_action(event)
-        expect(Thread.current[:lograge_location]).to be_nil
+        expect(RequestStore.store[:lograge_location]).to be_nil
       end
     end
 
@@ -188,7 +188,7 @@
 
     context 'with unpermitted_parameters' do
       before do
-        Thread.current[:lograge_unpermitted_params] = %w(florb blarf)
+        RequestStore.store[:lograge_unpermitted_params] = %w(florb blarf)
       end
 
       it 'adds the unpermitted_params to the log line' do
@@ -198,7 +198,7 @@
 
       it 'removes the thread local variable' do
         subscriber.process_action(event)
-        expect(Thread.current[:lograge_unpermitted_params]).to be_nil
+        expect(RequestStore.store[:lograge_unpermitted_params]).to be_nil
       end
     end