From a8a1a303e09a17d196825386cfa476868339ec71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Sat, 17 Jan 2015 14:17:01 +0100 Subject: [PATCH] [rubocop] Reduce ABC metric to 20 --- .rubocop.yml | 2 +- .../repository_git_config_keys_controller.rb | 72 ++++++++++--------- app/models/gitolite_public_key.rb | 30 ++++---- ...81302_aggregated_git_hosting_migrations.rb | 60 +++++++++------- .../revisions/git/gitolite_wrapper/users.rb | 6 +- .../git/patches/git_adapter_patch.rb | 28 +++++--- 6 files changed, 117 insertions(+), 81 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 079a5dba5ce..995db89601e 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -123,7 +123,7 @@ LineLength: Max: 120 Metrics/AbcSize: - Max: 26 + Max: 20 MethodLength: Enabled: false diff --git a/app/controllers/repository_git_config_keys_controller.rb b/app/controllers/repository_git_config_keys_controller.rb index cc564b64540..22cf9181912 100644 --- a/app/controllers/repository_git_config_keys_controller.rb +++ b/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 diff --git a/app/models/gitolite_public_key.rb b/app/models/gitolite_public_key.rb index 6e40ac21f22..8d091930ec9 100644 --- a/app/models/gitolite_public_key.rb +++ b/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 diff --git a/db/migrate/20140530181302_aggregated_git_hosting_migrations.rb b/db/migrate/20140530181302_aggregated_git_hosting_migrations.rb index 8f06bbf9560..c87b7e973ef 100644 --- a/db/migrate/20140530181302_aggregated_git_hosting_migrations.rb +++ b/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 diff --git a/lib/open_project/revisions/git/gitolite_wrapper/users.rb b/lib/open_project/revisions/git/gitolite_wrapper/users.rb index fd597a697e6..f2283e0ad21 100644 --- a/lib/open_project/revisions/git/gitolite_wrapper/users.rb +++ b/lib/open_project/revisions/git/gitolite_wrapper/users.rb @@ -33,7 +33,6 @@ 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 @@ -41,6 +40,11 @@ def add_gitolite_key(key) @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 diff --git a/lib/open_project/revisions/git/patches/git_adapter_patch.rb b/lib/open_project/revisions/git/patches/git_adapter_patch.rb index f8ec75250ce..64d822442db 100644 --- a/lib/open_project/revisions/git/patches/git_adapter_patch.rb +++ b/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) \ No newline at end of file +Redmine::Scm::Adapters::GitAdapter.send(:include, OpenProject::Revisions::Git::Patches::GitAdapterPatch)