From acda6ebd605d36aa5a265fe36f3cb023bf438db0 Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Tue, 10 Feb 2015 17:03:33 +1100
Subject: [PATCH] FIX: view tracking needs to release data earlier

retaining data during queuing was causing huge memory spikes
---
 lib/middleware/request_tracker.rb             | 52 +++++++++++++------
 .../middleware/request_tracker_spec.rb        | 13 +++--
 2 files changed, 44 insertions(+), 21 deletions(-)

diff --git a/lib/middleware/request_tracker.rb b/lib/middleware/request_tracker.rb
index b6e9b3a4e9e..f1558596dad 100644
--- a/lib/middleware/request_tracker.rb
+++ b/lib/middleware/request_tracker.rb
@@ -6,29 +6,25 @@ class Middleware::RequestTracker
     @app = app
   end
 
-  def self.log_request_on_site(result, env, helper=nil)
+  def self.log_request_on_site(data)
     host = RailsMultisite::ConnectionManagement.host(env)
     RailsMultisite::ConnectionManagement.with_hostname(host) do
-      log_request(result,env,helper)
+      log_request(data)
     end
   end
 
-  PATH_PARAMS = "action_dispatch.request.path_parameters".freeze
-  TRACK_VIEW = "HTTP_DISCOURSE_TRACK_VIEW".freeze
 
 
-  def self.log_request(result,env,helper=nil)
+  def self.log_request(data)
 
-    helper ||= Middleware::AnonymousCache::Helper.new(env)
-    request = Rack::Request.new(env)
 
-    status,headers = result
-    status = status.to_i
+    status = data[:status]
+    track_view = data[:track_view]
 
-    if (env[TRACK_VIEW] || (request.get? && !request.xhr? && headers["Content-Type"] =~ /text\/html/)) && status == 200
-      if helper.is_crawler?
+    if track_view
+      if data[:is_crawler]
         ApplicationRequest.increment!(:page_view_crawler)
-      elsif helper.has_auth_cookie?
+      elsif data[:has_auth_cookie]
         ApplicationRequest.increment!(:page_view_logged_in)
       else
         ApplicationRequest.increment!(:page_view_anon)
@@ -44,24 +40,46 @@ class Middleware::RequestTracker
     elsif status >= 300
       ApplicationRequest.increment!(:http_3xx)
     else
-      if request.path =~ /^\/message-bus\// || request.path == /\/topics\/timings/
+      if data[:is_background]
         ApplicationRequest.increment!(:http_background)
       elsif status >= 200 && status < 300
         ApplicationRequest.increment!(:http_2xx)
       end
     end
 
-  # rescue => ex
-  #   Discourse.handle_exception(ex, {message: "Failed to log request"})
   end
 
+  TRACK_VIEW = "HTTP_DISCOURSE_TRACK_VIEW".freeze
+  CONTENT_TYPE = "Content-Type".freeze
+  def self.get_data(env,result)
+
+    status,headers = result
+    status = status.to_i
+
+    helper = Middleware::AnonymousCache::Helper.new(env)
+    request = Rack::Request.new(env)
+    {
+      status: status,
+      is_crawler: helper.is_crawler?,
+      has_auth_cookie: helper.has_auth_cookie?,
+      is_background: request.path =~ /^\/message-bus\// || request.path == /\/topics\/timings/,
+      track_view: (env[TRACK_VIEW] || (request.get? && !request.xhr? && headers[CONTENT_TYPE] =~ /text\/html/)) && status == 200
+    }
+  end
 
   def call(env)
     result = @app.call(env)
   ensure
-    Scheduler::Defer.later("Track view", _db=nil) do
-      self.class.log_request_on_site(result,env)
+
+    # we got to skip this on error ... its just logging
+    data = self.class.get_data(env,result) rescue nil
+
+    if data
+      Scheduler::Defer.later("Track view", _db=nil) do
+        self.class.log_request_on_site(data)
+      end
     end
+
   end
 
 end
diff --git a/spec/components/middleware/request_tracker_spec.rb b/spec/components/middleware/request_tracker_spec.rb
index 4de9af7ebe6..c8064be052f 100644
--- a/spec/components/middleware/request_tracker_spec.rb
+++ b/spec/components/middleware/request_tracker_spec.rb
@@ -18,12 +18,17 @@ describe Middleware::RequestTracker do
 
       ApplicationRequest.clear_cache!
 
-      Middleware::RequestTracker.log_request(["200",{"Content-Type" => 'text/html'}], env(
+      data = Middleware::RequestTracker.get_data(env(
         "HTTP_USER_AGENT" => "AdsBot-Google (+http://www.google.com/adsbot.html)"
-      ))
-      Middleware::RequestTracker.log_request(["200",{}], env(
+      ), ["200",{"Content-Type" => 'text/html'}])
+
+      Middleware::RequestTracker.log_request(data)
+
+      data = Middleware::RequestTracker.get_data(env(
         "HTTP_DISCOURSE_TRACK_VIEW" => "1"
-      ))
+      ), ["200",{}])
+
+      Middleware::RequestTracker.log_request(data)
 
       ApplicationRequest.write_cache!