[rubocop] Reduce ABC metric to 20

.rubocop.yml

@@ -123,7 +123,7 @@ LineLength:
   Max: 120
 
 Metrics/AbcSize:
-  Max: 26
+  Max: 20
 
 MethodLength:
   Enabled: false

app/controllers/repository_git_config_keys_controller.rb

@@ -5,6 +5,7 @@ class RepositoryGitConfigKeysController < RevisionsGitControllerBase
   before_filter :can_view_config_keys,   only: [:index]
   before_filter :can_create_config_keys, only: [:new, :create]
   before_filter :can_edit_config_keys,   only: [:edit, :update, :destroy]
+  before_filter :create_config_key,      only: [:create]
 
   before_filter :find_repository_git_config_key, except: [:index, :new, :create]
 
@@ -22,39 +23,20 @@ def new
   end
 
   def create
-    @git_config_key = RepositoryGitConfigKey.new(params[:repository_git_config_keys])
-    @git_config_key.repository = @repository
-
-    respond_to do |format|
-      if @git_config_key.save
-        flash[:notice] = l(:notice_git_config_key_created)
-
-        format.html { redirect_to success_url }
-        format.js   { render js: "window.location = #{success_url.to_json};" }
-      else
-        format.html {
-          flash[:error] = l(:notice_git_config_key_create_failed)
-          render action: 'create'
-        }
-        format.js { render 'form_error', layout: false }
-      end
+    if @git_config_key.save
+      flash[:notice] = l(:notice_git_config_key_created)
+      key_change_success
+    else
+      fail_key_change(:notice_git_config_key_create_failed, 'create')
     end
   end
 
   def update
-    respond_to do |format|
-      if @git_config_key.update_attributes(params[:repository_git_config_keys])
-        flash[:notice] = l(:notice_git_config_key_updated)
-
-        format.html { redirect_to success_url }
-        format.js   { render js: "window.location = #{success_url.to_json};" }
-      else
-        format.html {
-          flash[:error] = l(:notice_git_config_key_update_failed)
-          render action: 'edit'
-        }
-        format.js { render 'form_error', layout: false }
-      end
+    if @git_config_key.update_attributes(params[:repository_git_config_keys])
+      flash[:notice] = l(:notice_git_config_key_updated)
+      key_change_success
+    else
+      fail_key_change(:notice_git_config_key_update_failed, 'edit')
     end
   end
 
@@ -71,16 +53,20 @@ def destroy
 
   private
 
+  def fail_unless_allowed_to(permission)
+    render_403 unless view_context.user_allowed_to(permission, @project)
+  end
+
   def can_view_config_keys
-    render_403 unless view_context.user_allowed_to(:view_repository_git_config_keys, @project)
+    fail_unless_allowed_to(:view_repository_git_config_keys)
   end
 
   def can_create_config_keys
-    render_403 unless view_context.user_allowed_to(:create_repository_git_config_keys, @project)
+    fail_unless_allowed_to(:create_repository_git_config_keys)
   end
 
   def can_edit_config_keys
-    render_403 unless view_context.user_allowed_to(:edit_repository_git_config_keys, @project)
+    fail_unless_allowed_to(:edit_repository_git_config_keys)
   end
 
   def find_repository_git_config_key
@@ -98,4 +84,26 @@ def find_repository_git_config_key
   def set_current_tab
     @tab = 'repository_git_config_keys'
   end
+
+  def create_config_key
+    @git_config_key = RepositoryGitConfigKey.new(params[:repository_git_config_keys])
+    @git_config_key.repository = @repository
+  end
+
+  def key_change_success
+    respond_to do |format|
+      format.html { redirect_to success_url }
+      format.js   { render js: "window.location = #{success_url.to_json};" }
+    end
+  end
+
+  def fail_key_change(action, label)
+    respond_to do |format|
+      format.html {
+        flash[:error] = l(label)
+        render action: action
+      }
+      format.js { render 'form_error', layout: false }
+    end
+  end
 end

app/models/gitolite_public_key.rb

@@ -167,18 +167,24 @@ def key_uniqueness
 
     existing = GitolitePublicKey.find_by_fingerprint(fingerprint)
     if existing
-      # Hm.... have a duplicate key!
-      if existing.user == User.current
-        errors.add(:key, l(:error_key_in_use_by_you, name: existing.title))
-        return false
-      elsif User.current.admin?
-        errors.add(:key, l(:error_key_in_use_by_other, login: existing.user.login, name: existing.title))
-        return false
-      else
-        errors.add(:key, l(:error_key_in_use_by_someone))
-        return false
-      end
+      determine_duplicate_error(existing)
+      false
+    else
+      true
+    end
+  end
+
+  # Determine the reason of a duplicate error
+  # and print it to the user.
+  # Avoids display the username of the existing fingerprint
+  # unless current user is admin
+  def determine_duplicate_error(existing)
+    if existing.user == User.current
+      errors.add(:key, l(:error_key_in_use_by_you, name: existing.title))
+    elsif User.current.admin?
+      errors.add(:key, l(:error_key_in_use_by_other, login: existing.user.login, name: existing.title))
+    else
+      errors.add(:key, l(:error_key_in_use_by_someone))
     end
-    true
   end
 end

db/migrate/20140530181302_aggregated_git_hosting_migrations.rb

@@ -39,36 +39,46 @@ def up
       MIGRATION_FILES, OLD_PLUGIN_NAME
     )
     Migration::MigrationSquasher.squash(migration_names) do
-      create_table :gitolite_public_keys do |t|
-        t.column :title, :string, null: false
-        t.column :identifier, :string, null: false
-        t.column :key, :text, null: false
-        t.column :key_type, :integer, null: false, default: GitolitePublicKey::KEY_TYPE_USER
-        t.column :delete_when_unused, :boolean, default: true
-        t.references :user, null: false
-        t.timestamps
-      end
+      create_public_keys_schema
+      create_repository_git_extras
+      create_repository_config_keys
 
-      add_index :gitolite_public_keys, :user_id
-      add_index :gitolite_public_keys, :identifier
+      Migration::SettingRenamer.rename(OLD_PLUGIN_NAME, 'plugin_openproject_revisions_git')
+    end
+  end
 
-      create_table :repository_git_extras do |t|
-        t.references :repository, null: false
-        t.column :git_daemon, :boolean, default: true
-        t.column :git_http,   :boolean, default: true
-        t.column :git_notify, :boolean, default: false
-        t.column :default_branch, :string, null: false
-        t.column :key, :string, null: false
-      end
+  def create_repository_config_keys
+    create_table :repository_git_config_keys do |t|
+      t.references :repository, null: false
+      t.column :key,   :string, null: false
+      t.column :value, :string, null: false
+    end
+  end
 
-      create_table :repository_git_config_keys do |t|
-        t.references :repository, null: false
-        t.column :key,   :string, null: false
-        t.column :value, :string, null: false
-      end
+  def create_repository_git_extras
+    create_table :repository_git_extras do |t|
+      t.references :repository, null: false
+      t.column :git_daemon, :boolean, default: true
+      t.column :git_http,   :boolean, default: true
+      t.column :git_notify, :boolean, default: false
+      t.column :default_branch, :string, null: false
+      t.column :key, :string, null: false
+    end
+  end
 
-      Migration::SettingRenamer.rename(OLD_PLUGIN_NAME, 'plugin_openproject_revisions_git')
+  def create_public_keys_schema
+    create_table :gitolite_public_keys do |t|
+      t.column :title, :string, null: false
+      t.column :identifier, :string, null: false
+      t.column :key, :text, null: false
+      t.column :key_type, :integer, null: false, default: GitolitePublicKey::KEY_TYPE_USER
+      t.column :delete_when_unused, :boolean, default: true
+      t.references :user, null: false
+      t.timestamps
     end
+
+    add_index :gitolite_public_keys, :user_id
+    add_index :gitolite_public_keys, :identifier
   end
 
   def down

lib/open_project/revisions/git/gitolite_wrapper/users.rb

@@ -33,14 +33,18 @@ def update_all_ssh_keys_forced
     private
 
     def add_gitolite_key(key)
-      parts = key.key.split
       repo_keys = @admin.ssh_keys[key.identifier]
       repo_key = repo_keys.select { |k| k.location == key.title && k.ownidentifierer == key.identifier }.first
       if repo_key
         logger.info("#{@action} : SSH key '#{key.identifier}@#{key.location}' exists, removing first ...")
         @admin.rm_key(repo_key)
       end
 
+      save_key(key)
+    end
+
+    def save_key(key)
+      parts = key.key.split
       repo_key = Gitolite::SSHKey.new(parts[0], parts[1], parts[2], key.identifier, key.title)
       @admin.add_key(repo_key)
     end

lib/open_project/revisions/git/patches/git_adapter_patch.rb

@@ -3,7 +3,6 @@
 module OpenProject::Revisions::Git
   module Patches
     module GitAdapterPatch
-
       def self.included(base)
         base.class_eval do
           unloadable
@@ -14,9 +13,7 @@ def self.included(base)
         end
       end
 
-
       module InstanceMethods
-
         private
 
         def scm_cmd_with_revisions_git(*args, &block)
@@ -28,6 +25,10 @@ def scm_cmd_with_revisions_git(*args, &block)
           end
           full_args += args
           ret = run_scm_cmd(full_args.map { |e| shell_quote e.to_s }.join(' '), &block)
+          if $? && $?.exitstatus != 0
+            raise ScmCommandAborted, "git exited with non-zero status: #{$?.exitstatus}"
+          end
+          ret
         end
 
         def scm_popen_mode
@@ -39,11 +40,7 @@ def scm_popen_mode
         end
 
         def run_scm_cmd(cmd, &block)
-          if Rails.env == 'development'
-            # Capture stderr when running in dev environment
-            cmd = "#{cmd} 2>>#{Rails.root}/log/scm.stderr.log"
-            Rails.logger.debug "Shelling out: #{strip_credential(cmd)}"
-          end
+          cmd = stderr_if_development(cmd)
           begin
             root = Setting.plugin_openproject_revisions_git[:gitolite_global_storage_path]
             IO.popen(cmd, scm_popen_mode, chdir: root) do |io|
@@ -53,12 +50,23 @@ def run_scm_cmd(cmd, &block)
           rescue Errno::ENOENT => e
             msg = strip_credential(e.message)
             # The command failed, log it and re-raise
-            logger.error("SCM command failed, make sure that your SCM binary (eg. svn) is in PATH (#{ENV['PATH']}): #{strip_credential(cmd)}\n  with: #{msg}")
+            logger.error("SCM command failed, make sure that your SCM binary (eg. svn)
+              is in PATH (#{ENV['PATH']}): #{strip_credential(cmd)}\n  with: #{msg}")
             raise CommandFailed.new(msg)
           end
         end
+
+        # Capture stderr when running in dev environment
+        def stderr_if_development(cmd)
+          if Rails.env == 'development'
+            Rails.logger.debug "Shelling out: #{strip_credential(cmd)}"
+            "#{cmd} 2>>#{Rails.root}/log/scm.stderr.log"
+          else
+            cmd
+          end
+        end
       end
     end
   end
 end
-Redmine::Scm::Adapters::GitAdapter.send(:include, OpenProject::Revisions::Git::Patches::GitAdapterPatch)
+Redmine::Scm::Adapters::GitAdapter.send(:include, OpenProject::Revisions::Git::Patches::GitAdapterPatch)