Refactor SCM config to lowercase / symbols

ENV-based configuration for various configs can only provide
case-insensitive values for SCM configuration and thus is
unable to properly configure, e.g., managed repositories.

This refactoring relies on lowercase configuration and symbols
to define SCM vendors, and camelizes them to the old notation
(e.g., :git => 'Git') when using `Setting.enabled_scms` to remain
compatible with previous installations.
pull/3379/head
Oliver Günther 9 years ago
parent e7e7d426a7
commit a71ce6f1a5
  1. 30
      app/assets/javascripts/settings.js.erb
  2. 23
      app/helpers/repositories_helper.rb
  3. 18
      app/models/repository.rb
  4. 2
      app/services/scm/repository_factory_service.rb
  5. 2
      app/views/repositories/_settings.html.erb
  6. 4
      app/views/repositories/settings/_vendor_form.html.erb
  7. 14
      app/views/settings/_repositories.html.erb
  8. 4
      config/configuration.yml.example
  9. 4
      config/initializers/scm.rb
  10. 4
      config/settings.yml
  11. 36
      db/migrate/20150819143300_underscore_scm_settings.rb
  12. 2
      lib/open_project/scm/adapters/base.rb
  13. 7
      lib/open_project/scm/adapters/local_client.rb
  14. 16
      lib/open_project/scm/manager.rb
  15. 4
      spec/app/services/scm/create_managed_repository_service_spec.rb
  16. 5
      spec/app/services/scm/delete_managed_repository_service_spec.rb
  17. 19
      spec/app/services/scm/repository_factory_service_spec.rb
  18. 4
      spec/controllers/repositories_controller_spec.rb
  19. 16
      spec/features/repositories/create_repository_spec.rb
  20. 16
      spec/features/repositories/repository_settings_spec.rb
  21. 4
      spec/legacy/functional/sys_controller_spec.rb
  22. 4
      spec/lib/acts_as_journalized/journaled_spec.rb
  23. 8
      spec/lib/open_project/scm/manager_spec.rb
  24. 8
      spec/models/enabled_module_spec.rb
  25. 2
      spec/models/repository/git_spec.rb
  26. 2
      spec/models/repository/subversion_spec.rb
  27. 2
      spec/support/repository_helpers.rb

@ -37,29 +37,21 @@ See doc/COPYRIGHT.rdoc for more details.
/** Sync SCM vendor select when enabled SCMs are changed */
$('[name="settings[enabled_scm][]"]').change(function () {
var checked = this.checked,
value = this.value,
var disabled = !this.checked,
vendor = this.value,
select = $('#settings_repositories_automatic_managed_vendor'),
option = select.find('option[value="' + value + '"]');
option = select.find('option[value="' + vendor + '"]');
if (checked && !option.length) {
select.append('<option value="' + value +'">' + value + '</option>')
.prop('disabled', false);
} else {
// Avoid selecting another vendor
if (option.prop('selected')) {
select.val('');
}
option.remove();
// Skip non-manageable SCMs
if (option.length === 0) {
return;
}
// Disable select when no option remains
if (select.find('option').length <= 1) {
select.prop('disabled', true);
}
option.prop('disabled', disabled);
if (disabled && option.prop('selected')) {
select.val('');
}
});
});
});
}(jQuery));

@ -185,20 +185,17 @@ module RepositoriesHelper
# and injects an already persisted repository for correctly
# displaying an existing repository.
def scm_options(repository = nil)
scms = OpenProject::Scm::Manager.enabled
vendor = repository.nil? ? nil : repository.vendor
options = []
OpenProject::Scm::Manager.enabled.each do |vendor, klass|
# Skip repositories that were configured to have no
# available types left.
next if klass.available_types.empty?
## Set selected vendor
if vendor && !repository.new_record?
scms[vendor] = vendor
options << [klass.vendor_name, vendor]
end
# Remove repositories that were configured to have no
# available types left.
scms.reject! { |_, klass| klass.available_types.empty? }
scms = [default_selected_option] + scms.keys
options_for_select(scms, vendor)
existing_vendor = repository.nil? ? nil : repository.vendor
options_for_select([default_selected_option] + options, existing_vendor)
end
def default_selected_option
@ -209,10 +206,6 @@ module RepositoriesHelper
]
end
def vendor_name(repository)
repository.vendor.underscore
end
def scm_vendor_tag(repository)
select_tag('scm_vendor',
scm_options(repository),

@ -57,7 +57,7 @@ class Repository < ActiveRecord::Base
# Checks if the SCM is enabled when creating a repository
def validate_enabled_scm
errors.add(:type, :invalid) unless Setting.enabled_scm.include?(self.class.name.demodulize)
errors.add(:type, :invalid) unless OpenProject::Scm::Manager.enabled?(vendor)
end
# Removes leading and trailing whitespace
@ -299,7 +299,7 @@ class Repository < ActiveRecord::Base
# Builds a model instance of type +Repository::#{vendor}+ with the given parameters.
#
# @param [Project] project The project this repository belongs to.
# @param [String] vendor The SCM vendor name (e.g., Git, Subversion)
# @param [Symbol] vendor The SCM vendor symbol (e.g., :git, :subversion)
# @param [Hash] params Custom parameters for this SCM as delivered from the repository
# field.
#
@ -367,7 +367,21 @@ class Repository < ActiveRecord::Base
nil
end
def self.enabled?
OpenProject::Scm::Manager.enabled?(vendor)
end
##
# Returns the SCM vendor symbol for this repository
# e.g., Repository::Git => :git
def self.vendor
vendor_name.underscore.to_sym
end
##
# Returns the SCM vendor name for this repository
# e.g., Repository::Git => Git
def self.vendor_name
name.demodulize
end

@ -76,7 +76,7 @@ Scm::RepositoryFactoryService = Struct.new :project, :params do
def build_with_type(scm_type)
Repository.build(
project,
params[:scm_vendor],
params.fetch(:scm_vendor).to_sym,
params.fetch(:repository, {}),
scm_type
)

@ -53,7 +53,7 @@ See doc/COPYRIGHT.rdoc for more details.
<%# Show (selected) type options %>
<% unless @repository.nil? %>
<%= render partial: "/repositories/settings/vendor_form",
locals: { form: f, repository: @repository, vendor: vendor_name(@repository) } %>
locals: { form: f, repository: @repository } %>
<% end %>
<%# Allow plugins to add additional information %>

@ -32,13 +32,13 @@ See doc/COPYRIGHT.rdoc for more details.
<% scm_types.each do |type|%>
<%= render partial: "/repositories/settings/vendor_attribute_groups",
locals: { existing: false, alone: scm_types.length == 1,
vendor: vendor, type: type,
vendor: @repository.vendor, type: type,
form: form, repository: repository } %>
<% end %>
</div>
<% else %>
<%= render partial: "/repositories/settings/vendor_attribute_groups",
locals: { existing: true, alone: true, vendor: repository.vendor.underscore,
locals: { existing: true, alone: true, vendor: @repository.vendor,
type: repository.scm_type,
form: form, repository: repository } %>
<% end %>

@ -56,18 +56,26 @@ See doc/COPYRIGHT.rdoc for more details.
<%= link_to_function l(:label_generate_key), "if ($('settings_sys_api_key').disabled == false) { $('settings_sys_api_key').value = randomKey(20) }" %>
</span>
</div>
<div class="form--field"><%= setting_multiselect(:enabled_scm, OpenProject::Scm::Manager.vendors) %></div>
<%
available_scms = OpenProject::Scm::Manager.registered
.map {|vendor, klass| [klass.vendor_name, vendor] }
manageable_scms = OpenProject::Scm::Manager.manageable
.map {|vendor, klass| [klass.vendor_name, vendor, { disabled: !klass.enabled? }]}
%>
<div class="form--field">
<%= setting_multiselect(:enabled_scm, available_scms) %>
</div>
<div class="form--field">
<%= styled_label_tag :repositories_automatic_managed_vendor, l(:setting_repositories_automatic_managed_vendor) %>
<div class="form--field-container">
<% manageable_scms = OpenProject::Scm::Manager.manageable %>
<%= setting_select :repositories_automatic_managed_vendor,
[[
"--- #{l('repositories.settings.automatic_managed_repos_disabled')} ---",
'',
{ selected: true }
]] + manageable_scms,
disabled: manageable_scms.empty?,
label: false %>
</div>
<div class="form--field-instructions"><%= l('repositories.settings.automatic_managed_repos_text') %></div>

@ -246,12 +246,12 @@ default:
# Examplary configuration
#
# scm:
# Git:
# git:
# client_command: /usr/local/bin/git
# disabled_types:
# - :local
# manages: /opt/repositories/git
# Subversion:
# subversion:
# client_command: /usr/local/bin/svn
# disabled_types:
# - :existing

@ -29,5 +29,5 @@
require 'open_project/scm/manager'
OpenProject::Scm::Manager.add 'Subversion'
OpenProject::Scm::Manager.add 'Git'
OpenProject::Scm::Manager.add :subversion
OpenProject::Scm::Manager.add :git

@ -127,8 +127,8 @@ diff_max_lines_displayed:
enabled_scm:
serialized: true
default:
- Subversion
- Git
- subversion
- git
autofetch_changesets:
default: 1
sys_api_enabled:

@ -0,0 +1,36 @@
#-- copyright
# OpenProject is a project management system.
# Copyright (C) 2012-2015 the OpenProject Foundation (OPF)
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License version 3.
#
# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
# Copyright (C) 2006-2013 Jean-Philippe Lang
# Copyright (C) 2010-2013 the ChiliProject Team
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License
# as published by the Free Software Foundation; either version 2
# of the License, or (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
#
# See doc/COPYRIGHT.rdoc for more details.
#++
class UnderscoreScmSettings < ActiveRecord::Migration
def up
Setting.enabled_scm = Setting.enabled_scm.map(&:underscore)
end
def down
Setting.enabled_scm = Setting.enabled_scm.map(&:camelize)
end
end

@ -36,7 +36,7 @@ module OpenProject
attr_accessor :url, :root_url
def self.vendor
name.demodulize
name.demodulize.underscore
end
def initialize(url, root_url = nil)

@ -82,13 +82,8 @@ module OpenProject
root_url
end
##
# Reads the configuration for this strategy from OpenProject's `configuration.yml`.
def config
scm_config = OpenProject::Configuration
['scm', vendor].inject(scm_config) do |acc, key|
HashWithIndifferentAccess.new acc[key]
end
self.class.config
end
##

@ -35,6 +35,9 @@ module OpenProject
@scms ||= {}
end
##
# Returns a list of registered SCM vendor symbols
# (e.g., :git, :subversion)
def vendors
@scms.keys
end
@ -43,12 +46,19 @@ module OpenProject
# Returns all enabled repositories as a Hash
# { vendor_name: repository class constant }
def enabled
registered.select { |scm| Setting.enabled_scm.include?(scm) }
registered.select { |sym, _| Setting.enabled_scm.include?(sym.to_s) }
end
##
# Returns whether the particular vendor symbol
# is available AND enabled through settings.
def enabled?(vendor)
enabled.include?(vendor)
end
# Return all manageable vendors
def manageable
enabled.select { |_, vendor| vendor.manageable? }.keys
registered.select { |_, vendor| vendor.manageable? }
end
##
@ -67,7 +77,7 @@ module OpenProject
def add(scm_name)
# Force model lookup to avoid
# const errors later on.
klass = Repository.const_get(scm_name)
klass = Repository.const_get(scm_name.to_s.camelize)
registered[scm_name] = klass
end

@ -73,8 +73,8 @@ describe Scm::CreateManagedRepositoryService do
include_context 'with tmpdir'
let(:config) {
{
Subversion: { manages: File.join(tmpdir, 'svn') },
Git: { manages: File.join(tmpdir, 'git') }
subversion: { manages: File.join(tmpdir, 'svn') },
git: { manages: File.join(tmpdir, 'git') }
}
}

@ -40,6 +40,7 @@ describe Scm::DeleteManagedRepositoryService do
before do
allow(OpenProject::Configuration).to receive(:[]).and_call_original
allow(OpenProject::Configuration).to receive(:[]).with('scm').and_return(config)
allow(Setting).to receive(:enabled_scm).and_return(['subversion', 'git'])
end
shared_examples 'does not delete the repository' do
@ -66,8 +67,8 @@ describe Scm::DeleteManagedRepositoryService do
include_context 'with tmpdir'
let(:config) {
{
Subversion: { manages: File.join(tmpdir, 'svn') },
Git: { manages: File.join(tmpdir, 'git') }
subversion: { manages: File.join(tmpdir, 'svn') },
git: { manages: File.join(tmpdir, 'git') }
}
}

@ -32,7 +32,7 @@ describe Scm::RepositoryFactoryService do
let(:user) { FactoryGirl.build(:user) }
let(:project) { FactoryGirl.build(:project) }
let(:enabled_scms) { ['Subversion', 'Git'] }
let(:enabled_scms) { ['subversion', 'git'] }
let(:params_hash) { {} }
let(:params) { ActionController::Parameters.new params_hash }
@ -45,14 +45,15 @@ describe Scm::RepositoryFactoryService do
context 'with empty hash' do
it 'should not build a repository' do
expect(service.build_temporary).not_to be true
expect { service.build_temporary }
.to raise_error KeyError
expect(service.repository).to be_nil
end
end
context 'with valid vendor' do
let(:params_hash) {
{ scm_vendor: 'Subversion' }
{ scm_vendor: 'subversion' }
}
it 'should allow temporary build repository' do
@ -70,27 +71,27 @@ describe Scm::RepositoryFactoryService do
context 'with invalid vendor' do
let(:params_hash) {
{ scm_vendor: 'NotSubversion', scm_type: 'foo' }
{ scm_vendor: 'not_subversion', scm_type: 'foo' }
}
it 'should not allow to temporary build repository' do
expect { service.build_temporary }.not_to raise_error
expect(service.repository).to be_nil
expect(service.build_error).to include('The SCM vendor NotSubversion is disabled')
expect(service.build_error).to include('The SCM vendor not_subversion is disabled')
end
it 'should not allow to persist a repository' do
expect { service.build_temporary }.not_to raise_error
expect(service.repository).to be_nil
expect(service.build_error).to include('The SCM vendor NotSubversion is disabled')
expect(service.build_error).to include('The SCM vendor not_subversion is disabled')
end
end
context 'with vendor and type' do
let(:params_hash) {
{ scm_vendor: 'Subversion', scm_type: 'existing' }
{ scm_vendor: 'subversion', scm_type: 'existing' }
}
it 'should not allow to persist a repository without URL' do
@ -104,7 +105,7 @@ describe Scm::RepositoryFactoryService do
context 'with invalid hash' do
let(:params_hash) {
{
scm_vendor: 'Subversion', scm_type: 'existing',
scm_vendor: 'subversion', scm_type: 'existing',
repository: { url: '/tmp/foo.svn' }
}
}
@ -120,7 +121,7 @@ describe Scm::RepositoryFactoryService do
context 'with valid hash' do
let(:params_hash) {
{
scm_vendor: 'Subversion', scm_type: 'existing',
scm_vendor: 'subversion', scm_type: 'existing',
repository: { url: 'file:///tmp/foo.svn' }
}
}

@ -41,7 +41,7 @@ describe RepositoriesController, type: :controller do
let (:url) { 'file:///tmp/something/does/not/exist.svn' }
let(:repository) do
allow(Setting).to receive(:enabled_scm).and_return(['Subversion'])
allow(Setting).to receive(:enabled_scm).and_return(['subversion'])
repo = FactoryGirl.build_stubbed(:repository_subversion,
scm_type: 'local',
url: url,
@ -107,7 +107,7 @@ describe RepositoriesController, type: :controller do
before do
xhr :post,
:create,
scm_vendor: 'Subversion',
scm_vendor: 'subversion',
scm_type: 'local',
url: 'file:///tmp/repo.svn/'
end

@ -36,7 +36,7 @@ describe 'Create repository', type: :feature, js: true do
# Allow to override configuration values to determine
# whether to activate managed repositories
let(:enabled_scms) { %w[Subversion Git] }
let(:enabled_scms) { %w[subversion git] }
let(:config) { nil }
let(:scm_vendor_input_css) { 'select[name="scm_vendor"]' }
@ -58,7 +58,7 @@ describe 'Create repository', type: :feature, js: true do
it 'displays the vendor selection' do
expect(scm_vendor_input).not_to be_nil
enabled_scms.each do |scm|
expect(scm_vendor_input).to have_selector('option', text: scm)
expect(scm_vendor_input).to have_selector('option', text: scm.camelize)
end
end
end
@ -68,7 +68,7 @@ describe 'Create repository', type: :feature, js: true do
end
context 'with only one enabled scm' do
let(:enabled_scms) { %w[Subversion] }
let(:enabled_scms) { %w[subversion] }
it_behaves_like 'shows enabled scms'
it 'does not show git' do
expect(scm_vendor_input).not_to have_selector('option', text: 'Git')
@ -163,14 +163,14 @@ describe 'Create repository', type: :feature, js: true do
end
context 'with Subversion selected' do
let(:vendor) { 'Subversion' }
let(:vendor) { 'subversion' }
it_behaves_like 'displays only the type', 'existing'
context 'and managed repositories' do
include_context 'with tmpdir'
let(:config) {
{ Subversion: { manages: tmpdir } }
{ subversion: { manages: tmpdir } }
}
it_behaves_like 'has managed and other type', 'existing'
it_behaves_like 'it can create the managed repository'
@ -181,12 +181,12 @@ describe 'Create repository', type: :feature, js: true do
end
context 'with Git selected' do
let(:vendor) { 'Git' }
let(:vendor) { 'git' }
it_behaves_like 'displays only the type', 'local'
context 'and managed repositories, but not ours' do
let(:config) {
{ Subversion: { manages: '/tmp/whatever' } }
{ subversion: { manages: '/tmp/whatever' } }
}
it_behaves_like 'displays only the type', 'local'
end
@ -194,7 +194,7 @@ describe 'Create repository', type: :feature, js: true do
context 'and managed repositories' do
include_context 'with tmpdir'
let(:config) {
{ Git: { manages: tmpdir } }
{ git: { manages: tmpdir } }
}
it_behaves_like 'has managed and other type', 'local'

@ -36,7 +36,7 @@ describe 'Repository Settings', type: :feature, js: true do
# Allow to override configuration values to determine
# whether to activate managed repositories
let(:enabled_scms) { %w[Subversion Git] }
let(:enabled_scms) { %w[subversion git] }
let(:config) { nil }
before do
@ -82,22 +82,22 @@ describe 'Repository Settings', type: :feature, js: true do
shared_examples 'manages the repository with' do |name, type|
let(:repository) {
FactoryGirl.create("repository_#{name.downcase}".to_sym,
FactoryGirl.create("repository_#{name}".to_sym,
scm_type: type,
project: project)
}
it_behaves_like 'manages the repository', type
end
it_behaves_like 'manages the repository with', 'Subversion', 'existing'
it_behaves_like 'manages the repository with', 'Git', 'local'
it_behaves_like 'manages the repository with', 'subversion', 'existing'
it_behaves_like 'manages the repository with', 'git', 'local'
context 'managed repositories' do
include_context 'with tmpdir'
let(:config) {
{
Subversion: { manages: File.join(tmpdir, 'svn') },
Git: { manages: File.join(tmpdir, 'git') }
subversion: { manages: File.join(tmpdir, 'svn') },
git: { manages: File.join(tmpdir, 'git') }
}
}
@ -115,12 +115,12 @@ describe 'Repository Settings', type: :feature, js: true do
}
context 'Subversion' do
let(:managed_vendor) { 'Subversion' }
let(:managed_vendor) { :subversion }
it_behaves_like 'manages the repository', 'managed'
end
context 'Git' do
let(:managed_vendor) { 'Git' }
let(:managed_vendor) { :git }
it_behaves_like 'manages the repository', 'managed'
end
end

@ -35,7 +35,7 @@ describe SysController, type: :controller do
before do
Setting.sys_api_enabled = '1'
Setting.enabled_scm = %w(Subversion Git)
Setting.enabled_scm = %w(subversion git)
end
it 'should projects with repository enabled' do
@ -51,7 +51,7 @@ describe SysController, type: :controller do
assert_nil Project.find(4).repository
post :create_project_repository, id: 4,
scm_vendor: 'Subversion',
scm_vendor: 'subversion',
scm_type: 'existing',
repository: { url: 'file:///create/project/repository/subproject2' }
assert_response :created

@ -96,9 +96,7 @@ describe 'Journalized Objects' do
it 'should work with changesets' do
Setting.enabled_scm = ['Subversion']
@repository ||= Repository.build_scm_class('Subversion').new
@repository.assign_attributes(scm_type: 'existing', url: 'http://svn.test.com')
@repository.save!
@repository ||= FactoryGirl.create(:repository_subversion, url: 'http://svn.test.com')
@changeset ||= FactoryGirl.create(:changeset, committer: @current.login, repository: @repository)
initial_journal = @changeset.journals.first

@ -35,21 +35,21 @@ describe OpenProject::Scm::Manager do
before do
Repository.const_set(vendor, scm_class)
OpenProject::Scm::Manager.add vendor
OpenProject::Scm::Manager.add :test_scm
end
after do
Repository.send(:remove_const, vendor)
OpenProject::Scm::Manager.delete vendor
OpenProject::Scm::Manager.delete :test_scm
end
it 'is a valid const' do
expect(OpenProject::Scm::Manager.registered[vendor]).to eq(Repository::TestScm)
expect(OpenProject::Scm::Manager.registered[:test_scm]).to eq(Repository::TestScm)
end
context 'scm is not known' do
it 'is not included' do
expect(OpenProject::Scm::Manager.registered).to_not have_key('SomeOtherScm')
expect(OpenProject::Scm::Manager.registered).to_not have_key(:some_scm)
end
end
end

@ -111,17 +111,17 @@ describe EnabledModule, type: :model do
end
context 'with enabled setting' do
let(:vendor) { 'Git' }
let(:vendor) { 'git' }
include_context 'with tmpdir'
let(:config) {
{
Git: { manages: File.join(tmpdir, 'git') }
git: { manages: File.join(tmpdir, 'git') }
}
}
before do
allow(Setting).to receive(:enabled_scm).and_return(['Git'])
allow(Setting).to receive(:enabled_scm).and_return(['git'])
allow(OpenProject::Configuration).to receive(:[]).and_call_original
allow(OpenProject::Configuration).to receive(:[]).with('scm').and_return(config)
end
@ -130,7 +130,7 @@ describe EnabledModule, type: :model do
project.reload
expect(project.repository).not_to be_nil
expect(project.repository.vendor).to eq('Git')
expect(project.repository.vendor).to eq('git')
expect(project.repository.managed?).to be true
end

@ -35,7 +35,7 @@ describe Repository::Git, type: :model do
let(:config) { {} }
before do
allow(Setting).to receive(:enabled_scm).and_return(['Git'])
allow(Setting).to receive(:enabled_scm).and_return(['git'])
allow(instance).to receive(:scm).and_return(adapter)
allow(adapter.class).to receive(:config).and_return(config)
end

@ -34,7 +34,7 @@ describe Repository::Subversion, type: :model do
let(:config) { {} }
before do
allow(Setting).to receive(:enabled_scm).and_return(['Subversion'])
allow(Setting).to receive(:enabled_scm).and_return(['subversion'])
allow(instance).to receive(:scm).and_return(adapter)
allow(instance.class).to receive(:scm_config).and_return(config)
end

@ -78,7 +78,7 @@ def with_virtual_subversion_repository(&block)
let(:repository) { FactoryGirl.create(:repository_subversion) }
before do
allow(Setting).to receive(:enabled_scm).and_return(['Subversion'])
allow(Setting).to receive(:enabled_scm).and_return(['subversion'])
end
block.call

Loading…
Cancel
Save