Extracted process guards into a separate class

lib/copycopter_client.rb

@@ -29,13 +29,11 @@ def self.deploy
   end
 
   # Starts the polling process.
-  # This is called from Unicorn worker processes.
   def self.start_sync
     sync.start
   end
 
   # Flush queued changed synchronously
-  # This is called from the Resque after perform "hook"
   def self.flush
     sync.flush
   end

lib/copycopter_client/configuration.rb

@@ -2,6 +2,7 @@
 require 'copycopter_client/i18n_backend'
 require 'copycopter_client/client'
 require 'copycopter_client/sync'
+require 'copycopter_client/process_guard'
 require 'copycopter_client/prefixed_logger'
 require 'copycopter_client/request_sync'
 
@@ -162,14 +163,15 @@ def applied?
     def apply
       client = Client.new(to_hash)
       sync = Sync.new(client, to_hash)
+      process_guard = ProcessGuard.new(sync, to_hash)
       I18n.backend = I18nBackend.new(sync)
       CopycopterClient.client = client
       CopycopterClient.sync = sync
       middleware.use(RequestSync, :sync => sync) if middleware && development?
       @applied = true
       logger.info("Client #{VERSION} ready")
       logger.info("Environment Info: #{environment_info}")
-      sync.start unless test?
+      process_guard.start unless test?
     end
 
     def port

lib/copycopter_client/process_guard.rb

@@ -0,0 +1,91 @@
+module CopycopterClient
+  # Starts the sync from a worker process, or register hooks for a spawner
+  # process (such as in Unicorn or Passenger). Also registers hooks for exiting
+  # processes and completing background jobs. Applications using the client
+  # will not need to interact with this class directly.
+  class ProcessGuard
+    # @param options [Hash]
+    # @option options [Logger] :logger where errors should be logged
+    def initialize(sync, options)
+      @sync   = sync
+      @logger = options[:logger]
+    end
+
+    # Starts the sync or registers hooks
+    def start
+      if spawner?
+        register_spawn_hooks
+      else
+        register_exit_hooks
+        register_job_hooks
+        start_sync
+      end
+    end
+
+    private
+
+    def start_sync
+      @sync.start
+    end
+
+    def spawner?
+      passenger_spawner? || unicorn_spawner?
+    end
+
+    def passenger_spawner?
+      $0.include?("ApplicationSpawner")
+    end
+
+    def unicorn_spawner?
+      $0.include?("unicorn") && !caller.any? { |line| line.include?("worker_loop") }
+    end
+
+    def register_spawn_hooks
+      if defined?(PhusionPassenger)
+        register_passenger_hook
+      elsif defined?(Unicorn::HttpServer)
+        register_unicorn_hook
+      end
+    end
+
+    def register_passenger_hook
+      @logger.info("Registered Phusion Passenger fork hook")
+      PhusionPassenger.on_event(:starting_worker_process) do |forked|
+        start_sync
+      end
+    end
+
+    def register_unicorn_hook
+      @logger.info("Registered Unicorn fork hook")
+      sync = @sync
+      Unicorn::HttpServer.class_eval do
+        alias_method :worker_loop_without_copycopter, :worker_loop
+        define_method :worker_loop do |worker|
+          sync.start
+          worker_loop_without_copycopter(worker)
+        end
+      end
+    end
+
+    def register_exit_hooks
+      at_exit do
+        @sync.flush
+      end
+    end
+
+    def register_job_hooks
+      if defined?(Resque)
+        @logger.info("Registered Resque after_perform hook")
+        sync = @sync
+        Resque::Job.class_eval do
+          alias_method :perform_without_copycopter, :perform
+          define_method :perform do
+            job_was_performed = perform_without_copycopter
+            sync.flush
+            job_was_performed
+          end
+        end
+      end
+    end
+  end
+end

lib/copycopter_client/sync.rb

@@ -27,21 +27,11 @@ def initialize(client, options)
     end
 
     # Starts the polling thread. The polling thread doesn't run in test environments.
-    #
-    # If this sync was created from a master spawner (as in the case for
-    # phusion passenger), it will instead register after fork hooks so that the
-    # poller starts in each spawned process.
     def start
-      if spawner?
-        register_spawn_hooks
-      else
-        register_job_hooks
-        logger.info("Starting poller")
-        @pending = true
-        at_exit { sync }
-        unless Thread.new { poll }
-          logger.error("Couldn't start poller thread")
-        end
+      logger.info("Starting poller")
+      @pending = true
+      unless Thread.new { poll }
+        logger.error("Couldn't start poller thread")
       end
     end
 
@@ -134,49 +124,5 @@ def with_queued_changes
     def lock(&block)
       @mutex.synchronize(&block)
     end
-
-    def spawner?
-      passenger_spawner? || unicorn_spawner?
-    end
-
-    def passenger_spawner?
-      $0.include?("ApplicationSpawner")
-    end
-
-    def unicorn_spawner?
-      $0.include?("unicorn") && !caller.any? { |line| line.include?("worker_loop") }
-    end
-
-    def register_job_hooks
-      if defined?(Resque)
-        logger.info("Registered Resque after_perform hook")
-        Resque::Job.class_eval do
-          alias_method :perform_without_copycopter, :perform
-          def perform
-            job_was_performed = perform_without_copycopter
-            CopycopterClient.flush
-            job_was_performed
-          end
-        end
-      end
-    end
-
-    def register_spawn_hooks
-      if defined?(PhusionPassenger)
-        logger.info("Registered Phusion Passenger fork hook")
-        PhusionPassenger.on_event(:starting_worker_process) do |forked|
-          start
-        end
-      elsif defined?(Unicorn::HttpServer)
-        logger.info("Registered Unicorn fork hook")
-        Unicorn::HttpServer.class_eval do
-          alias_method :worker_loop_without_copycopter, :worker_loop
-          def worker_loop(worker)
-            CopycopterClient.start_sync
-            worker_loop_without_copycopter(worker)
-          end
-        end
-      end
-    end
   end
 end

spec/copycopter_client/configuration_spec.rb

@@ -194,15 +194,17 @@
 
 share_examples_for "applied configuration" do
   let(:backend) { stub('i18n-backend') }
-  let(:sync)    { stub('sync', :start => nil) }
+  let(:sync)    { stub('sync') }
   let(:client)  { stub('client') }
+  let(:process_guard) { stub('process_guard', :start => nil) }
   let(:logger)  { FakeLogger.new }
   subject { CopycopterClient::Configuration.new }
 
   before do
     CopycopterClient::I18nBackend.stubs(:new => backend)
     CopycopterClient::Client.stubs(:new => client)
     CopycopterClient::Sync.stubs(:new => sync)
+    CopycopterClient::ProcessGuard.stubs(:new => process_guard)
     subject.logger = logger
     apply
   end
@@ -216,6 +218,10 @@
     I18n.backend.should == backend
   end
 
+  it "builds a process guard" do
+    CopycopterClient::ProcessGuard.should have_received(:new).with(sync, subject.to_hash)
+  end
+
   it "logs that it's ready" do
     logger.should have_entry(:info, "Client #{CopycopterClient::VERSION} ready")
   end
@@ -235,8 +241,8 @@
 
 describe CopycopterClient::Configuration, "applied when testing" do
   it_should_behave_like "applied configuration" do
-    it "doesn't start sync" do
-      sync.should have_received(:start).never
+    it "doesn't start the process guard" do
+      process_guard.should have_received(:start).never
     end
   end
 
@@ -248,8 +254,8 @@ def apply
 
 describe CopycopterClient::Configuration, "applied when not testing" do
   it_should_behave_like "applied configuration" do
-    it "starts sync" do
-      sync.should have_received(:start)
+    it "starts the process guard" do
+      process_guard.should have_received(:start)
     end
   end
 

spec/copycopter_client/process_guard_spec.rb

@@ -0,0 +1,130 @@
+require 'spec_helper'
+
+describe CopycopterClient::ProcessGuard do
+  include DefinesConstants
+
+  before do
+    @original_process_name = $0
+  end
+
+  after do
+    $0 = @original_process_name
+  end
+
+  let(:sync) { stub('sync', :start => nil, :flush => nil) }
+
+  def build_process_guard(options = {})
+    options[:logger] ||= FakeLogger.new
+    CopycopterClient::ProcessGuard.new(sync, options)
+  end
+
+  it "starts polling from a worker process" do
+    process_guard = build_process_guard
+
+    process_guard.start
+
+    sync.should have_received(:start)
+  end
+
+  it "registers passenger hooks from the passenger master" do
+    logger = FakeLogger.new
+    passenger = define_constant('PhusionPassenger', FakePassenger.new)
+    passenger.become_master
+
+    process_guard = build_process_guard(:logger => logger)
+    process_guard.start
+
+    logger.should have_entry(:info, "Registered Phusion Passenger fork hook")
+    sync.should have_received(:start).never
+  end
+
+  it "starts polling from a passenger worker" do
+    logger = FakeLogger.new
+    passenger = define_constant('PhusionPassenger', FakePassenger.new)
+    passenger.become_master
+    process_guard = build_process_guard(:logger => logger)
+
+    process_guard.start
+    passenger.spawn
+
+    sync.should have_received(:start)
+  end
+
+  it "registers unicorn hooks from the unicorn master" do
+    logger = FakeLogger.new
+    define_constant('Unicorn', Module.new)
+    http_server = Class.new(FakeUnicornServer)
+    unicorn = define_constant('Unicorn::HttpServer', http_server).new
+    unicorn.become_master
+
+    process_guard = build_process_guard(:logger => logger)
+    process_guard.start
+
+    logger.should have_entry(:info, "Registered Unicorn fork hook")
+    sync.should have_received(:start).never
+  end
+
+  it "starts polling from a unicorn worker" do
+    logger = FakeLogger.new
+    define_constant('Unicorn', Module.new)
+    http_server = Class.new(FakeUnicornServer)
+    unicorn = define_constant('Unicorn::HttpServer', http_server).new
+    unicorn.become_master
+    process_guard = build_process_guard(:logger => logger)
+
+    process_guard.start
+    unicorn.spawn
+
+    sync.should have_received(:start)
+  end
+
+  it "flushes when the process terminates" do
+    api_key = "12345"
+    FakeCopycopterApp.add_project api_key
+    pid = fork do
+      config = { :logger => FakeLogger.new, :polling_delay => 86400, :api_key => api_key }
+      default_config = CopycopterClient::Configuration.new.to_hash.update(config)
+      client = CopycopterClient::Client.new(default_config)
+      sync = CopycopterClient::Sync.new(client, default_config)
+      process_guard = CopycopterClient::ProcessGuard.new(sync, default_config)
+      process_guard.start
+      sync['test.key'] = 'value'
+      Signal.trap("INT") { exit }
+      sleep
+    end
+    sleep(0.5)
+    Process.kill("INT", pid)
+    Process.wait
+    project = FakeCopycopterApp.project(api_key)
+    project.draft['test.key'].should == 'value'
+  end
+
+  it "flushes after running a resque job" do
+    define_constant('Resque', Module.new)
+    job_class = define_constant('Resque::Job', FakeResqueJob)
+
+    api_key = "12345"
+    FakeCopycopterApp.add_project api_key
+    logger = FakeLogger.new
+
+    config = { :logger => logger, :polling_delay => 86400, :api_key => api_key }
+    default_config = CopycopterClient::Configuration.new.to_hash.update(config)
+    client = CopycopterClient::Client.new(default_config)
+    sync = CopycopterClient::Sync.new(client, default_config)
+    job = job_class.new { sync["test.key"] = "expected value" }
+    process_guard = CopycopterClient::ProcessGuard.new(sync, default_config)
+
+    process_guard.start
+
+    if fork
+      Process.wait
+    else
+      job.perform
+      exit!
+    end
+
+    project = FakeCopycopterApp.project(api_key)
+    project.draft['test.key'].should == 'expected value'
+    logger.should have_entry(:info, "Registered Resque after_perform hook")
+  end
+end