Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove safe_yaml. #2397

Merged
merged 2 commits into from
Jun 7, 2016
Merged

Remove safe_yaml. #2397

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5481aa6
Remove safe_yaml.
bf4 Sep 4, 2015
5c14d48
Fix time-of-day-related query failures
Jun 6, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions lib/rails_admin.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
require 'rails_admin/extensions/history'
require 'rails_admin/support/csv_converter'
require 'rails_admin/support/hash_helper'
require 'yaml'

module RailsAdmin
# Setup RailsAdmin
Expand All @@ -33,6 +34,28 @@ def self.config(entity = nil, &block)
RailsAdmin::Config
end
end

# Backwards-compatible with safe_yaml/load when SafeYAML isn't available.
# Evaluates available YAML loaders at boot and creates appropriate method,
# so no conditionals are required at runtime.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two conditions for defining RailsAdmin.yaml_load depending on available gems. But the nicest part is, that this method can be customized by any end user :)

begin
require 'safe_yaml/load'
def self.yaml_load(yaml)
SafeYAML.load(yaml)
end
Copy link
Contributor Author

@bf4 bf4 Jun 7, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

define self.yaml_load to use SafeYAML.load in non-monkey-patching mode, when available

rescue LoadError
if YAML.respond_to?(:safe_load)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

define self.yaml_load to use Psych.safe_load when available

def self.yaml_load(yaml)
YAML.safe_load(yaml)
end
else
raise LoadError.new "Safe-loading of YAML is not available. Please install 'safe_yaml' or install Psych 2.0+"
end
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fail with useful warning if user does not have safe_yaml or Psych 2.0+

end

def self.yaml_dump(object)
YAML.dump(object)
end
end

require 'rails_admin/bootstrap-sass' unless defined? Bootstrap
4 changes: 2 additions & 2 deletions lib/rails_admin/config/fields/types/serialized.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ class Serialized < RailsAdmin::Config::Fields::Types::Text
RailsAdmin::Config::Fields::Types.register(self)

register_instance_option :formatted_value do
YAML.dump(value) unless value.nil?
RailsAdmin.yaml_dump(value) unless value.nil?
end

def parse_value(value)
value.present? ? (SafeYAML.load(value) || nil) : nil
value.present? ? (RailsAdmin.yaml_load(value) || nil) : nil
end

def parse_input(params)
Expand Down
1 change: 0 additions & 1 deletion lib/rails_admin/engine.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
require 'rails'
require 'rails_admin'
require 'remotipart'
require 'safe_yaml/load'

module RailsAdmin
class Engine < Rails::Engine
Expand Down
1 change: 0 additions & 1 deletion rails_admin.gemspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ Gem::Specification.new do |spec|
spec.add_dependency 'rack-pjax', '~> 0.7'
spec.add_dependency 'rails', ['>= 4.0', '< 6']
spec.add_dependency 'remotipart', '~> 1.0'
spec.add_dependency 'safe_yaml', '~> 1.0'
spec.add_dependency 'sass-rails', ['>= 4.0', '< 6']
spec.add_development_dependency 'bundler', '~> 1.0'
spec.authors = ['Erik Michaels-Ober', 'Bogdan Gaza', 'Petteri Kaapa', 'Benoit Benezech', 'Mitsuhiro Shibuya']
Expand Down
10 changes: 5 additions & 5 deletions spec/rails_admin/abstract_model_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,11 @@
end

it 'lists elements within outbound limits' do
expect(@abstract_model.all(filters: {'datetime_field' => {'1' => {v: ['', 'January 02, 2012 00:00', 'January 03, 2012 00:00'], o: 'between'}}}).count).to eq(2)
expect(@abstract_model.all(filters: {'datetime_field' => {'1' => {v: ['', 'January 02, 2012 00:00', 'January 02, 2012 00:00'], o: 'between'}}}).count).to eq(1)
expect(@abstract_model.all(filters: {'datetime_field' => {'1' => {v: ['', 'January 03, 2012 00:00', ''], o: 'between'}}}).count).to eq(2)
expect(@abstract_model.all(filters: {'datetime_field' => {'1' => {v: ['', '', 'January 02, 2012 00:00'], o: 'between'}}}).count).to eq(2)
expect(@abstract_model.all(filters: {'datetime_field' => {'1' => {v: ['January 02, 2012 00:00'], o: 'default'}}}).count).to eq(1)
expect(@abstract_model.all(filters: {'datetime_field' => {'1' => {v: ['', 'January 02, 2012 12:00', 'January 03, 2012 12:00'], o: 'between'}}}).count).to eq(2)
expect(@abstract_model.all(filters: {'datetime_field' => {'1' => {v: ['', 'January 02, 2012 12:00', 'January 02, 2012 12:00'], o: 'between'}}}).count).to eq(1)
expect(@abstract_model.all(filters: {'datetime_field' => {'1' => {v: ['', 'January 03, 2012 12:00', ''], o: 'between'}}}).count).to eq(2)
expect(@abstract_model.all(filters: {'datetime_field' => {'1' => {v: ['', '', 'January 02, 2012 12:00'], o: 'between'}}}).count).to eq(2)
expect(@abstract_model.all(filters: {'datetime_field' => {'1' => {v: ['January 02, 2012 12:00'], o: 'default'}}}).count).to eq(1)
end
end
end
Expand Down
8 changes: 4 additions & 4 deletions spec/rails_admin/adapters/active_record_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,10 +315,10 @@ class PlayerWithDefaultScope < Player

it 'supports datetime type query' do
scope = FieldTest.all
expect(abstract_model.send(:filter_scope, scope, 'datetime_field' => {'1' => {v: ['', 'February 01, 2012 00:00', 'March 01, 2012 00:00'], o: 'between'}}).where_values).to eq(scope.where(['(field_tests.datetime_field BETWEEN ? AND ?)', Time.local(2012, 2, 1), Time.local(2012, 3, 1).end_of_day]).where_values)
expect(abstract_model.send(:filter_scope, scope, 'datetime_field' => {'1' => {v: ['', 'March 01, 2012 00:00', ''], o: 'between'}}).where_values).to eq(scope.where(['(field_tests.datetime_field >= ?)', Time.local(2012, 3, 1)]).where_values)
expect(abstract_model.send(:filter_scope, scope, 'datetime_field' => {'1' => {v: ['', '', 'February 01, 2012 00:00'], o: 'between'}}).where_values).to eq(scope.where(['(field_tests.datetime_field <= ?)', Time.local(2012, 2, 1).end_of_day]).where_values)
expect(abstract_model.send(:filter_scope, scope, 'datetime_field' => {'1' => {v: ['February 01, 2012 00:00'], o: 'default'}}).where_values).to eq(scope.where(['(field_tests.datetime_field BETWEEN ? AND ?)', Time.local(2012, 2, 1), Time.local(2012, 2, 1).end_of_day]).where_values)
expect(abstract_model.send(:filter_scope, scope, 'datetime_field' => {'1' => {v: ['', 'February 01, 2012 12:00', 'March 01, 2012 12:00'], o: 'between'}}).where_values).to eq(scope.where(['(field_tests.datetime_field BETWEEN ? AND ?)', Time.local(2012, 2, 1), Time.local(2012, 3, 1).end_of_day]).where_values)
expect(abstract_model.send(:filter_scope, scope, 'datetime_field' => {'1' => {v: ['', 'March 01, 2012 12:00', ''], o: 'between'}}).where_values).to eq(scope.where(['(field_tests.datetime_field >= ?)', Time.local(2012, 3, 1)]).where_values)
expect(abstract_model.send(:filter_scope, scope, 'datetime_field' => {'1' => {v: ['', '', 'February 01, 2012 12:00'], o: 'between'}}).where_values).to eq(scope.where(['(field_tests.datetime_field <= ?)', Time.local(2012, 2, 1).end_of_day]).where_values)
expect(abstract_model.send(:filter_scope, scope, 'datetime_field' => {'1' => {v: ['February 01, 2012 12:00'], o: 'default'}}).where_values).to eq(scope.where(['(field_tests.datetime_field BETWEEN ? AND ?)', Time.local(2012, 2, 1), Time.local(2012, 2, 1).end_of_day]).where_values)
end

it 'supports enum type query' do
Expand Down