Merge pull request nesquena#131 from acant/bug/restart

Fixing crash on configuration restarts through the web UI

CHANGELOG

@@ -2,6 +2,7 @@
 
  * fix gem dependencies to preserve Ruby v2.0.0 compatibility (@acant)
  * add --host flag for setting the address for the web interface (@acant)
+ * fix unexpected exit when changing settings through web UI (@acant)
 
 0.6.2 (July 5th 2016)
 

lib/gitdocs.rb

@@ -18,6 +18,7 @@
 require 'gitdocs/configuration'
 require 'gitdocs/cli'
 require 'gitdocs/manager'
+require 'gitdocs/celluloid_facade'
 require 'gitdocs/synchronizer'
 require 'gitdocs/notifier'
 require 'gitdocs/git_notifier'
@@ -64,8 +65,6 @@ def self.log_error(message)
 
   ##############################################################################
 
-  private_class_method
-
   # @return [void]
   def self.init_log
     return if @initialized
@@ -76,4 +75,5 @@ def self.init_log
     Celluloid.logger.level = Initializer.verbose ? Logger::DEBUG : Logger::INFO
     @initialized = true
   end
+  private_class_method :init_log
 end

lib/gitdocs/browser_app.rb

@@ -1,7 +1,7 @@
 # -*- encoding : utf-8 -*-
 
 require 'sinatra/base'
-require 'uri'
+require 'cgi'
 require 'haml'
 require 'mimetype_fu'
 require 'gitdocs/rendering_helper'
@@ -27,7 +27,7 @@ def repository
       def path
         halt(404) unless repository
         @path ||= Repository::Path.new(
-          repository, URI.decode(params[:splat].first)
+          repository, CGI.unescape(params[:splat].first)
         )
       end
     end

lib/gitdocs/celluloid_facade.rb

@@ -0,0 +1,73 @@
+# -*- encoding : utf-8 -*-
+
+module Gitdocs
+  class CelluloidFascade
+    # @param [String] host
+    # @param [Integer] port
+    def initialize(host, port)
+      @host = host
+      @port = port
+    end
+
+    # @return [void]
+    def start
+      Celluloid.boot unless Celluloid.running?
+      @supervisor = Celluloid::SupervisionGroup.run!
+
+      # Start the web server ###################################################
+      app =
+        Rack::Builder.new do
+          use Rack::Static,
+              urls: %w(/css /js /img /doc),
+              root: File.expand_path('../public', __FILE__)
+          use Rack::MethodOverride
+          map('/settings') { run SettingsApp }
+          map('/')         { run BrowserApp }
+        end
+      @supervisor.add(
+        Reel::Rack::Server,
+        as: :reel_rack_server,
+        args: [
+          app,
+          {
+            Host:  @host,
+            Port:  @port,
+            quiet: true
+          }
+        ]
+      )
+
+      # Start the synchronizers ################################################
+      Share.all.each do |share|
+        @supervisor.add(
+          Synchronizer, as: share.id.to_s, args: [share]
+        )
+      end
+
+      # Start the repository listeners #########################################
+      @listener =
+        Listen.to(
+          *Share.paths,
+          ignore: /#{File::SEPARATOR}\.git#{File::SEPARATOR}/
+        ) do |modified, added, removed|
+          all_changes = modified + added + removed
+          changed_repository_paths =
+            Share.paths.select do |directory|
+              all_changes.any? { |x| x.start_with?(directory) }
+            end
+
+          changed_repository_paths.each do |directory|
+            actor_id = Share.find_by_path(directory).id.to_s
+            Celluloid::Actor[actor_id].async.synchronize
+          end
+        end
+      @listener.start
+    end
+
+    # @return [void]
+    def terminate
+      @listener.stop        if @listener
+      @supervisor.terminate if @supervisor
+    end
+  end
+end

lib/gitdocs/cli.rb

@@ -67,7 +67,7 @@ def restart
     method_option :pid,          type: :string,  aliases: '-P'
     method_option :interval,     type: :numeric, aliases: '-i', default: 15
     method_option :notification, type: :boolean, aliases: '-n', default: true
-    method_option :sync,         type: :boolean, aliases: '-s', default: 'full'
+    method_option :sync,         type: :string,  aliases: '-s', default: 'full', enum: %w(full fetch)
     def add(path)
       Share.create_by_path!(
         normalize_path(path),
@@ -83,7 +83,7 @@ def add(path)
     method_option :pid,          type: :string,  aliases: '-P'
     method_option :interval,     type: :numeric, aliases: '-i', default: 15
     method_option :notification, type: :boolean, aliases: '-n', default: true
-    method_option :sync,         type: :boolean, aliases: '-s', default: 'full'
+    method_option :sync,         type: :string,  aliases: '-s', default: 'full', enum: %w(full fetch)
     def create(path, remote)
       Repository.clone(path, remote)
       Share.create_by_path!(

lib/gitdocs/manager.rb

@@ -30,88 +30,33 @@ def start(host, port)
         "Using configuration root: '#{Initializer.root_dirname}'"
       )
 
-      Celluloid.boot unless Celluloid.running?
-      @supervisor = Celluloid::SupervisionGroup.run!
-
-      # Start the web server ###################################################
-      app =
-        Rack::Builder.new do
-          use Rack::Static,
-              urls: %w(/css /js /img /doc),
-              root: File.expand_path('../public', __FILE__)
-          use Rack::MethodOverride
-          map('/settings') { run SettingsApp }
-          map('/')         { run BrowserApp }
-        end
-      @supervisor.add(
-        Reel::Rack::Server,
-        as: :reel_rack_server,
-        args: [
-          app,
-          {
-            Host:  host,
-            Port:  port,
-            quiet: true
-          }
-        ]
-      )
-
-      # Start the synchronizers ################################################
-      @synchronization_supervisor = Celluloid::SupervisionGroup.run!
-      Share.all.each do |share|
-        @synchronization_supervisor.add(
-          Synchronizer, as: share.id.to_s, args: [share]
+      @celluloid = Gitdocs::CelluloidFascade.new(host, port)
+
+      begin
+        @celluloid.start
+        # ... and wait ###########################################################
+        sleep
+      rescue Restart
+        Gitdocs.log_info('Restarting actors...')
+        @celluloid.terminate
+        retry
+      rescue Interrupt, SystemExit
+        Gitdocs.log_info('Interrupt received...')
+        @celluloid.terminate
+      rescue Exception => e # rubocop:disable RescueException
+        Gitdocs.log_error("#{e.inspect} - #{e.message}")
+        Gitdocs.log_error(e.backtrace.join("\n"))
+        Notifier.error(
+          'Unexpected exit',
+          'Something went wrong. Please see the log for details.'
         )
-      end
-
-      # Start the repository listeners #########################################
-      @listener =
-        Listen.to(
-          *Share.paths,
-          ignore: /#{File::SEPARATOR}\.git#{File::SEPARATOR}/
-        ) do |modified, added, removed|
-          all_changes = modified + added + removed
-          changed_repository_paths =
-            Share.paths.select do |directory|
-              all_changes.any? { |x| x.start_with?(directory) }
-            end
-
-          changed_repository_paths.each do |directory|
-            actor_id = Share.find_by_path(directory).id.to_s
-            Celluloid::Actor[actor_id].async.synchronize
-          end
-        end
-      @listener.start
-
-      # ... and wait ###########################################################
-      sleep
+        @celluloid.terminate
 
-    rescue Interrupt, SystemExit
-      Gitdocs.log_info('Interrupt received...')
-    rescue Exception => e # rubocop:disable RescueException
-      Gitdocs.log_error(
-        "#{e.class.inspect} - #{e.inspect} - #{e.message.inspect}"
-      )
-      Gitdocs.log_error(e.backtrace.join("\n"))
-      Notifier.error(
-        'Unexpected exit',
-        'Something went wrong. Please see the log for details.'
-      )
-      raise
-    ensure
-      Gitdocs.log_info('stopping listeners...')
-      @listener.stop if @listener
-
-      Gitdocs.log_info('stopping synchronizers...')
-      @synchronization_supervisor.terminate if @synchronization_supervisor
-
-      Gitdocs.log_info('terminate supervisor...')
-      @supervisor.terminate if @supervisor
-
-      Gitdocs.log_info('disconnect notifier...')
-      Notifier.disconnect
-
-      Gitdocs.log_info("Gitdocs is terminating...goodbye\n\n")
+        raise
+      ensure
+        Notifier.disconnect
+        Gitdocs.log_info("Gitdocs is terminating...goodbye\n\n")
+      end
     end
   end
 end

test/integration/test_helper.rb

@@ -89,6 +89,12 @@ def before_setup
   end
 
   def teardown
+    # Verify that there are not errors in the log file.
+    log_filename = File.join(abs_current_dir, '.gitdocs', 'log')
+    refute(
+      File.read(log_filename).include?('ERROR'), 'Unexpected ERROR in log'
+    ) if File.exist?(log_filename)
+
     Capybara.reset_sessions!
     Capybara.use_default_driver
   end

test/unit/celluloid_facade_test.rb

@@ -0,0 +1,11 @@
+# -*- encoding : utf-8 -*-
+
+require File.expand_path('../test_helper', __FILE__)
+
+describe 'Gitdocs::CelluloidFacade' do
+  let(:celluloid_facade) { Gitdocs::CelluloidFacade.new(:host, :port) }
+
+  # TODO: describe '#start' do
+
+  # TODO: describe '#terminate' do
+end

test/unit/manager_test.rb

@@ -14,7 +14,16 @@
     it { subject }
   end
 
-  # TODO: describe '.restart_synchronization' do
+  describe '.restart_synchronization' do
+    subject { Gitdocs::Manager.restart_synchronization }
+
+    before do
+      Thread.expects(:main).returns(thread = mock)
+      thread.expects(:raise).with(Gitdocs::Restart, 'restarting ... ')
+    end
+
+    it { subject }
+  end
 
   describe '.listen_method' do
     subject { Gitdocs::Manager.listen_method }
@@ -32,5 +41,90 @@
     end
   end
 
-  # TODO: describe '.start'
+  let(:manager) { Gitdocs::Manager.new }
+
+  describe '.start' do
+    subject { manager.start(:host, :port) }
+
+    let(:celluloid_fascade) { mock }
+    before do
+      Gitdocs::Initializer.stubs(:root_dirname).returns(:root_dirname)
+      Gitdocs.expects(:log_info).with("Starting Gitdocs v#{Gitdocs::VERSION}...")
+      Gitdocs.expects(:log_info).with("Using configuration root: 'root_dirname'")
+
+      Gitdocs::CelluloidFascade.expects(:new)
+        .with(:host, :port)
+        .returns(celluloid_fascade)
+#      celluloid_fascade.expects(:start)
+#      manager.expects(:sleep).raises(expected_exception)
+    end
+
+    describe 'restart' do
+      before do
+        celluloid_fascade.expects(:start).twice
+        celluloid_fascade.expects(:terminate)#.twice
+        manager.stubs(:sleep).raises(Gitdocs::Restart).then.returns(:result)
+
+        Gitdocs.expects(:log_info).with('Restarting actors...')
+        Gitdocs::Notifier.expects(:disconnect)
+        Gitdocs.expects(:log_info).with("Gitdocs is terminating...goodbye\n\n")
+      end
+
+      it { subject.must_equal(:result) }
+    end
+
+    describe 'exit' do
+      before do
+        celluloid_fascade.expects(:start)
+        manager.expects(:sleep).raises(expected_exception)
+      end
+
+      describe 'Interrupt' do
+        let(:expected_exception) { Interrupt }
+
+        before do
+          Gitdocs.expects(:log_info).with('Interrupt received...')
+          celluloid_fascade.expects(:terminate)
+          Gitdocs::Notifier.expects(:disconnect)
+          Gitdocs.expects(:log_info).with("Gitdocs is terminating...goodbye\n\n")
+        end
+
+        it { subject }
+      end
+
+      describe 'SystemExit' do
+        let(:expected_exception) { SystemExit }
+
+        before do
+          Gitdocs.expects(:log_info).with('Interrupt received...')
+          celluloid_fascade.expects(:terminate)
+          Gitdocs::Notifier.expects(:disconnect)
+          Gitdocs.expects(:log_info).with("Gitdocs is terminating...goodbye\n\n")
+        end
+
+        it { subject }
+      end
+
+      describe 'unexpected Exception' do
+        let(:expected_exception) { Exception.new }
+
+        before do
+          expected_exception.stubs(:backtrace).returns(%w(foo bar))
+          expected_exception.stubs(:message).returns(:message)
+
+          Gitdocs.expects(:log_error).with("#{expected_exception.inspect} - message")
+          Gitdocs.expects(:log_error).with("foo\nbar")
+          Gitdocs::Notifier.expects(:error).with(
+            'Unexpected exit',
+            'Something went wrong. Please see the log for details.'
+          )
+          celluloid_fascade.expects(:terminate)
+          Gitdocs::Notifier.expects(:disconnect)
+          Gitdocs.expects(:log_info).with("Gitdocs is terminating...goodbye\n\n")
+        end
+
+        it { proc { subject }.must_raise(Exception) }
+      end
+    end
+  end
 end