Skip to content

Commit

Permalink
Update gems to a Rails 4 compatible version
Browse files Browse the repository at this point in the history
Counter cache columns fix

Remove conditions,order,limit from has_many relations

Remove test runner

On minitest 5, the runner API was deprecated, so our custom test runner
is no longer working.

Remove useless add_index on lookup_values match

An index is added previously on lookup_values :priority, and on Rails 4
rename_column changes 'priority' to 'match' already changes the name of
the index.

Alias assert_include to assert_includes

Expire topbar cache

Subclass ApplicationMailer in test to avoid 'missing template' error

Fixes #12021 - Use .to_param to find parent object in LookupKeys controller

On Rails 4, .find will not default to .friendly.find so find_hostgroup,
find_environment and find_host will fail for non numeric IDs. However, we can
use from_param to find these objects. This is something we can do both on
Rails 3 and Rails 4, whereas using .friendly isn't an option until friendly_id
 5.0, which depends on Rails 4.

Use explicit friendly ID search

Remove RecordNotFound errors from parameterizable in rails 4

find_common finder uses from_param -> friendly -> find

The finder needed to be refactored because with the new Friendly ID we
have to use .friendly explicitely. It currently follows the strategy of
searching like this:
  - from_param -> .friendly -> regular find

Protected attributes automatically added through AccessibleAttributes

AccessibleAttributes pulls in all possible associations and models and
sets them as attr_accessible for the model. Where needed, additional
attr_accessible attributes have been added.

refs theforeman#3157 - adding protected_attributes gem

Conflicts:
	app/models/auth_source.rb
	app/models/domain.rb
	app/models/image.rb
	app/models/lookup_value.rb
	config/routes/test.rb

Conflicts:
	app/models/domain.rb
	app/models/lookup_value.rb

url_for doesn't append port when protocol is specified

Some of our tests are checking if port 80 or 443 is included in the URL
generated by lib/foreman/renderer.rb. However url_for has changed in
Rails 4 and now it does not append the port even if explicitly added.

If the protocol is specified, as it is in this case, then the url
generated will just have the protocol (http or https) but not the port
(80 or 443)

Validate object instead of _id column in join tables

On Rails 4 such validations will fail when we try to create objects
without explicitely assigning the id.
Puppetclass.new(:config_group => config_group) would fail even for a
valid config group, :config_group_id => config_group.id should be used
instead.

To avoid that, we validate the object, not the ID column

Rails 4 non backwards compatible syntax changes

Changes that have to deal with how some of the internal Rails objects
need a new syntax, like generating routes, exceptions, form builders.
None of these are Rails 3 compatible.

Fixes #12199 - rails 4 migration errors

There are actually two parts to the issue of not being able to migrate
with this branch. The first is
https://gist.github.com/eLobato/0f5db50b5c93cc6c277c and can be
temporarily fixed with rails/sass-rails#136 (comment) I am not sure of a
long term fix for that atm.

Once that is patched, this error
https://gist.github.com/johnpmitsch/96e5ba3629890931193a happens on a
migration. This will fix that error by changing the Migrator class
initializer arguments which have changed from rails 3 to rails 4

Refactor fact value test to not modify user_roles directly

Return external usergroupsas an array

Use eager_load instead of references

Other rails 4 particularities
  • Loading branch information
dLobatog committed Oct 28, 2015
1 parent 4e08a71 commit f4222bc
Show file tree
Hide file tree
Showing 118 changed files with 430 additions and 293 deletions.
15 changes: 9 additions & 6 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -4,31 +4,34 @@ require File.expand_path('../lib/regexp_extensions', FOREMAN_GEMFILE)

source 'https://rubygems.org'

gem 'rails', '3.2.22'
gem 'rails', '4.1.5'
gem 'rack-cache', '< 1.3.0'
gem 'json', '~> 1.5'
gem 'rest-client', '~> 1.6.0', :require => 'rest_client'
gem 'audited-activerecord', '3.0.0'
gem 'audited-activerecord', '~> 4.0'
gem 'will_paginate', '~> 3.0'
gem 'ancestry', '~> 2.0'
gem 'scoped_search', '~> 3.0'
gem 'scoped_search', '>= 3.2.2'
gem 'ldap_fluff', '>= 0.3.5', '< 1.0'
gem 'net-ldap', '>= 0.8.0'
gem 'apipie-rails', '~> 0.2.5'
gem 'apipie-rails', '~> 0.3.4'
gem 'rabl', '~> 0.11'
gem 'oauth', '~> 0.4'
gem 'deep_cloneable', '~> 2.0'
gem 'foreigner', '~> 1.4'
gem 'validates_lengths_from_database', '~> 0.2'
gem 'friendly_id', '~> 4.0'
gem 'friendly_id', '~> 5.0'
gem 'secure_headers', '~> 1.3'
gem 'safemode', '~> 1.2'
gem 'fast_gettext', '~> 0.8'
gem 'gettext_i18n_rails', '~> 1.0'
gem 'i18n', '~> 0.6.4'
gem 'rails-i18n', '~> 3.0.0'
gem 'rails-i18n', '~> 4.0.0'
gem 'turbolinks', '~> 2.5'
gem 'logging', '>= 1.8.0', '< 3.0.0'
gem 'activerecord-session_store'
gem 'rails-observers'
gem 'protected_attributes'

Dir["#{File.dirname(FOREMAN_GEMFILE)}/bundler.d/*.rb"].each do |bundle|
self.instance_eval(Bundler.read_file(bundle))
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/v1/reports_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def destroy
param :id, :identifier, :required => true

def last
conditions = { :host_id => Host.authorized(:view_hosts).find(params[:host_id]).try(:id) } if params[:host_id].present?
conditions = { :host_id => Host.authorized(:view_hosts).friendly.find(params[:host_id]).try(:id) } if params[:host_id].present?
max_id = resource_scope.where(conditions).maximum(:id)
@report = resource_scope.includes(:logs => [:message, :source]).find(max_id)
render :show
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/v2/host_classes_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def destroy
# overwrite resource_name so it's host and and not host_class, since we want to return @host
def find_host
not_found and return false if params[:host_id].blank?
@host = Host.find(params[:host_id]) if Host::Managed.respond_to?(:authorized) &&
@host = Host.friendly.find(params[:host_id]) if Host::Managed.respond_to?(:authorized) &&
Host::Managed.authorized("view_host", Host::Managed)
end
end
Expand Down
6 changes: 6 additions & 0 deletions app/controllers/api/v2/models_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@ def update
def destroy
process_response @model.destroy
end

private

def find_resource
@model = Model.friendly.find(params[:id]) || super
end
end
end
end
5 changes: 3 additions & 2 deletions app/controllers/api/v2/parameters_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,9 @@ def allowed_nested_id
def find_parameter
# nested_obj is required, so no need to check here
@parameters = nested_obj.send(parameters_method)
@parameter = @parameters.find(params[:id])
return @parameter if @parameter
@parameter = @parameters.from_param(params[:id])
@parameter ||= @parameters.friendly.find(params[:id])
return @parameter if @parameter.present?
not_found
end
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/v2/reports_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def destroy
param :id, :identifier, :required => true

def last
conditions = { :host_id => Host.authorized(:view_hosts).find(params[:host_id]).try(:id) } if params[:host_id].present?
conditions = { :host_id => Host.authorized(:view_hosts).friendly.find(params[:host_id]).try(:id) } if params[:host_id].present?
max_id = resource_scope.where(conditions).maximum(:id)
@report = resource_scope.includes(:logs => [:message, :source]).find(max_id)
render :show
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/v2/smart_variables_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def show
param_group :smart_variable, :as => :create

def create
@smart_variable = LookupKey.new(params[:smart_variable]) unless @puppetclass
@smart_variable = VariableLookupKey.new(params[:smart_variable]) unless @puppetclass
@smart_variable ||= @puppetclass.lookup_keys.build(params[:smart_variable])
process_response @smart_variable.save
end
Expand Down
57 changes: 21 additions & 36 deletions app/controllers/concerns/api/v2/lookup_keys_common_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,6 @@ module Api::V2::LookupKeysCommonController
before_filter :return_if_smart_mismatch, :only => [:show, :update, :destroy]
end

def puppetclass_id?
params.keys.include?('puppetclass_id')
end

def environment_id?
params.keys.include?('environment_id')
end

def host_id?
params.keys.include?('host_id')
end

def hostgroup_id?
params.keys.include?('hostgroup_id')
end

def smart_variable_id?
params.keys.include?('smart_variable_id') || controller_name.match(/smart_variables/)
end
Expand All @@ -43,28 +27,22 @@ def smart_class_parameter_id?
params.keys.include?('smart_class_parameter_id') || controller_name.match(/smart_class_parameters/)
end

def find_puppetclass
@puppetclass = Puppetclass.authorized(:view_puppetclasses).find(params['puppetclass_id'])
rescue ActiveRecord::RecordNotFound
not_found({ :error => { :message => (_("Puppet class with id '%{id}' was not found") % { :id => params['puppetclass_id'] }) } })
end
[Puppetclass, Environment, Host::Base, Hostgroup].each do |model|
model_string = model.to_s.split('::').first.downcase

def find_environment
@environment = Environment.authorized(:view_environments).find(params['environment_id'])
rescue ActiveRecord::RecordNotFound
not_found({ :error => { :message => (_("Environment with id '%{id}' was not found") % { :id => params['environment_id'] }) } })
end

def find_host
@host = Host::Base.authorized(:view_hosts).find(params['host_id'])
rescue ActiveRecord::RecordNotFound
not_found({ :error => { :message => (_("Host with id '%{id}' was not found") % { :id => params['host_id'] }) } })
end
define_method("#{model_string}_id?") do
params.keys.include?("#{model_string}_id")
end

def find_hostgroup
@hostgroup = Hostgroup.authorized(:view_hostgroups).find(params['hostgroup_id'])
rescue ActiveRecord::RecordNotFound
not_found({ :error => { :message => (_("Hostgroup with id '%{id}' was not found") % { :id => params['hostgroup_id'] }) } })
define_method("find_#{model_string}") do
scope = model.authorized(:"view_#{model_string.pluralize}")
begin
instance_variable_set("@#{model_string}",
resource_finder(scope, params["#{model_string}_id"]))
rescue ActiveRecord::RecordNotFound
model_not_found(model_string)
end
end
end

def find_smart_variable
Expand Down Expand Up @@ -144,4 +122,11 @@ def return_if_smart_mismatch
not_found "#{obj} not found by id '#{id}'"
end
end

private

def model_not_found(model_name)
not_found(:error => { :message => (_("#{model_name.capitalize} with id '%{id}' was not found") %
{ :id => params["#{model_name}_id"] } ) } )
end
end
16 changes: 11 additions & 5 deletions app/controllers/concerns/find_common.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,18 @@
# this mixin is used by both ApplicationController and Api::BaseController
# searches for an object based on its id, name, label, etc and assign it to an instance variable
# This mixin is used by both ApplicationController and Api::BaseController
# Searches for an object based on its id, name, label, etc and assign it to an instance variable
# friendly_id performs the logic if params[:id] is 'id' or 'id-name' or 'name'

module FindCommon
# example: @host = Host.find(params[:id])
def find_resource
not_found and return if params[:id].blank?
instance_variable_set("@#{resource_name}", resource_scope.find(params[:id]))
instance_variable_set("@#{resource_name}",
resource_finder(resource_scope, params[:id]))
end

def resource_finder(scope, id)
raise ActiveRecord::RecordNotFound if scope.empty?
result = scope.from_param(id) if scope.respond_to?(:from_param)
result ||= scope.friendly.find(id) if scope.respond_to?(:friendly)
result ||= scope.find(id)
end

def resource_name(resource = controller_name)
Expand Down
6 changes: 3 additions & 3 deletions app/controllers/hosts_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ def puppetclass_parameters
def externalNodes
certname = params[:name]
@host ||= resource_base.find_by_certname certname
@host ||= resource_base.find certname
@host ||= resource_base.friendly.find certname
not_found and return unless @host

begin
Expand Down Expand Up @@ -352,7 +352,7 @@ def update_multiple_parameters
skipped = []
params[:name].each do |name, value|
next if value.empty?
if (host_param = host.host_parameters.find(name))
if (host_param = host.host_parameters.friendly.find(name))
counter += 1 if host_param.update_attribute(:value, value)
else
skipped << name
Expand Down Expand Up @@ -676,7 +676,7 @@ def taxonomy_scope
# overwrite application_controller
def find_resource
not_found and return false if (id = params[:id]).blank?
@host = resource_base.find(id)
@host = resource_base.friendly.find(id)
@host ||= resource_base.find_by_mac params[:host][:mac] if params[:host] && params[:host][:mac]

not_found and return(false) unless @host
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/reports_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ def index
def show
# are we searching for the last report?
if params[:id] == "last"
conditions = { :host_id => Host.authorized(:view_hosts).find(params[:host_id]).try(:id) } if params[:host_id].present?
conditions = { :host_id => Host.authorized(:view_hosts).friendly.find(params[:host_id]).try(:id) } if params[:host_id].present?
params[:id] = resource_base.where(conditions).maximum(:id)
end
@report = resource_base.includes(:logs => [:message, :source]).find(params[:id])
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/settings_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ class SettingsController < ApplicationController
end

def index
@settings = Setting.live_descendants.search_for(params[:search])
@settings = Setting.live_descendants.search_for(params[:search]).where(nil)
end

def update
@setting = Setting.find(params[:id])
@setting = Setting.friendly.find(params[:id])
if @setting.parse_string_value(params[:setting][:value]) && @setting.save
render :json => @setting
else
Expand Down
6 changes: 3 additions & 3 deletions app/controllers/unattended_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ def built
def template
return head(:not_found) unless (params.has_key?("id") and params.has_key?(:hostgroup))

template = ProvisioningTemplate.find(params['id'])
@host = Hostgroup.find(params['hostgroup'])
template = ProvisioningTemplate.friendly.find(params['id'])
@host = Hostgroup.friendly.find(params['hostgroup'])

return head(:not_found) unless template and @host

Expand Down Expand Up @@ -108,7 +108,7 @@ def get_host_details

def find_host_by_spoof
host = Nic::Base.primary.find_by_ip(params.delete('spoof')).try(:host) if params['spoof'].present?
host ||= Host.find(params.delete('hostname')) if params['hostname'].present?
host ||= Host.friendly.find(params.delete('hostname')) if params['hostname'].present?
@spoof = host.present?
host
end
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/usergroups_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def find_by_id(permission = :view_usergroups)

def get_external_usergroups_to_refresh
# we need to load current status, so we call all explicitly
@external_usergroups = @usergroup.external_usergroups.all
@external_usergroups = @usergroup.external_usergroups.to_a
end

def external_usergroups
Expand Down
7 changes: 7 additions & 0 deletions app/helpers/application_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@ def link_to(*args, &block)
end
end

def link_to_function(name, function, html_options = {})
onclick = "#{"#{html_options[:onclick]}; " if html_options[:onclick]}#{function}; return false;"
href = html_options[:href] || '#'

content_tag(:a, name, html_options.merge(:href => href, :onclick => onclick))
end

protected

def contract(model)
Expand Down
3 changes: 3 additions & 0 deletions app/models/architecture.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,16 @@ class Architecture < ActiveRecord::Base
friendly_id :name
include Parameterizable::ByIdName

attr_accessible :name, :operatingsystem_ids

before_destroy EnsureNotUsedBy.new(:hosts, :hostgroups)
validates_lengths_from_database

has_many_hosts
has_many :hostgroups
has_many :images, :dependent => :destroy
has_and_belongs_to_many :operatingsystems

validates :name, :presence => true, :uniqueness => true, :no_whitespace => true
audited :allow_mass_assignment => true, :except => [:hosts_count, :hostgroups_count]

Expand Down
6 changes: 6 additions & 0 deletions app/models/auth_source.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,18 @@

class AuthSource < ActiveRecord::Base
include Authorizable

attr_accessible :name, :host, :tls, :port, :server_type, :account,
:base_dn, :groups_base, :ldap_filter, :onthefly_register,
:attr_login, :attr_firstname, :attr_lastname, :attr_mail, :attr_photo

audited :allow_mass_assignment => true

validates_lengths_from_database :except => [:name, :account_password, :host, :attr_login, :attr_firstname, :attr_lastname, :attr_mail]
before_destroy EnsureNotUsedBy.new(:users)
has_many :users
has_many :external_usergroups, :dependent => :destroy
attr_protected :type

validates :name, :presence => true, :uniqueness => true, :length => { :maximum => 60 }

Expand Down
4 changes: 3 additions & 1 deletion app/models/bookmark.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ class Bookmark < ActiveRecord::Base
friendly_id :name
include Parameterizable::ByIdName

attr_accessible :name, :query, :public, :controller

validates_lengths_from_database

belongs_to :owner, :polymorphic => true
attr_accessible :name, :controller, :query, :public
audited :allow_mass_assignment => true

validates :name, :uniqueness => {:scope => :controller}, :unless => Proc.new{|b| Bookmark.my_bookmarks.where(:name => b.name).empty?}
Expand All @@ -16,6 +17,7 @@ class Bookmark < ActiveRecord::Base
:inclusion => {
:in => ["dashboard"] + ActiveRecord::Base.connection.tables.map(&:to_s),
:message => _("%{value} is not a valid controller") }

default_scope -> { order(:name) }
before_validation :set_default_user

Expand Down
2 changes: 0 additions & 2 deletions app/models/cached_user_role.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
class CachedUserRole < ActiveRecord::Base
attr_accessible :role_id, :user_id, :user_role_id, :role, :user, :user_role

belongs_to :user
belongs_to :role

Expand Down
2 changes: 0 additions & 2 deletions app/models/cached_usergroup_member.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
class CachedUsergroupMember < ActiveRecord::Base
attr_accessible :user_id, :usergroup_id, :user, :usergroup

belongs_to :user
belongs_to :usergroup
end
1 change: 1 addition & 0 deletions app/models/compute_attribute.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
class ComputeAttribute < ActiveRecord::Base
attr_accessible :compute_profile_id, :compute_resource_id, :vm_attrs

audited :associated_with => :compute_profile

belongs_to :compute_resource
Expand Down
3 changes: 2 additions & 1 deletion app/models/compute_profile.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@ class ComputeProfile < ActiveRecord::Base
friendly_id :name
include Parameterizable::ByIdName

validates_lengths_from_database
attr_accessible :name

validates_lengths_from_database
audited
has_associated_audits

Expand Down
5 changes: 5 additions & 0 deletions app/models/compute_resource.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ class ComputeResource < ActiveRecord::Base
include Parameterizable::ByIdName
encrypts :password

attr_accessible :name, :provider, :description, :url, :display_type,
:set_console_password, :user, :password, :public_key,
:server, :tenant, :project, :email, :key_path,
:location_ids, :organization_ids

class_attribute :supported_providers
self.supported_providers = {
'Libvirt' => 'Foreman::Model::Libvirt',
Expand Down
Loading

0 comments on commit f4222bc

Please sign in to comment.