From 8cc1cba017297a6724109b036afe4b0680e2d388 Mon Sep 17 00:00:00 2001 From: Andrea Dal Ponte Date: Thu, 3 Dec 2015 23:43:58 +0100 Subject: [PATCH 1/3] Fix: parse BSON object value --- lib/rails_admin/adapters/mongoid.rb | 13 ++++++++++++ .../adapters/mongoid/abstract_object.rb | 1 + .../config/fields/types/bson_object_id.rb | 16 +------------- spec/dummy_app/app/mongoid/field_test.rb | 2 +- .../fields/types/bson_object_id_spec.rb | 21 +++++++++++++++++++ 5 files changed, 37 insertions(+), 16 deletions(-) diff --git a/lib/rails_admin/adapters/mongoid.rb b/lib/rails_admin/adapters/mongoid.rb index be5adc16e2..3bd4857be6 100644 --- a/lib/rails_admin/adapters/mongoid.rb +++ b/lib/rails_admin/adapters/mongoid.rb @@ -8,6 +8,19 @@ module RailsAdmin module Adapters module Mongoid DISABLED_COLUMN_TYPES = ['Range', 'Moped::BSON::Binary', 'BSON::Binary', 'Mongoid::Geospatial::Point'] + OBJECT_ID ||= begin + if defined?(Moped::BSON) + Moped::BSON::ObjectId + elsif defined?(BSON::ObjectId) + BSON::ObjectId + end + end + + def parse_object_id(value) + OBJECT_ID.from_string(value) + rescue BSON::ObjectId::Invalid, BSON::InvalidObjectId, Moped::Errors::InvalidObjectId + nil + end def new(params = {}) AbstractObject.new(model.new(params)) diff --git a/lib/rails_admin/adapters/mongoid/abstract_object.rb b/lib/rails_admin/adapters/mongoid/abstract_object.rb index 886a91fddf..b1dd1dbe36 100644 --- a/lib/rails_admin/adapters/mongoid/abstract_object.rb +++ b/lib/rails_admin/adapters/mongoid/abstract_object.rb @@ -1,4 +1,5 @@ require 'rails_admin/adapters/active_record/abstract_object' + module RailsAdmin module Adapters module Mongoid diff --git a/lib/rails_admin/config/fields/types/bson_object_id.rb b/lib/rails_admin/config/fields/types/bson_object_id.rb index 97e54b6ac9..59f3077ee0 100644 --- a/lib/rails_admin/config/fields/types/bson_object_id.rb +++ b/lib/rails_admin/config/fields/types/bson_object_id.rb @@ -8,14 +8,6 @@ class BsonObjectId < RailsAdmin::Config::Fields::Types::String # Register field type for the type loader RailsAdmin::Config::Fields::Types.register(self) - OBJECT_ID ||= begin - if defined?(Moped::BSON) - Moped::BSON::ObjectId - elsif defined?(BSON::ObjectId) - BSON::ObjectId - end - end - register_instance_option :label do label = ((@label ||= {})[::I18n.locale] ||= abstract_model.model.human_attribute_name name) label = 'Id' if label == '' @@ -35,13 +27,7 @@ def generic_help end def parse_value(value) - value.present? ? OBJECT_ID.from_string(value) : nil - rescue BSON::ObjectId::Invalid - nil - rescue => e - unless ['BSON::InvalidObjectId', 'Moped::Errors::InvalidObjectId'].include? e.class.to_s - raise e - end + value.present? ? abstract_model.parse_object_id(value) : nil end def parse_input(params) diff --git a/spec/dummy_app/app/mongoid/field_test.rb b/spec/dummy_app/app/mongoid/field_test.rb index 6b22d39809..ec23cf6764 100644 --- a/spec/dummy_app/app/mongoid/field_test.rb +++ b/spec/dummy_app/app/mongoid/field_test.rb @@ -12,7 +12,7 @@ class FieldTest field :array_field, type: Array field :big_decimal_field, type: BigDecimal field :boolean_field, type: Boolean - field :bson_object_id_field, type: RailsAdmin::Config::Fields::Types::BsonObjectId::OBJECT_ID + field :bson_object_id_field, type: RailsAdmin::Adapters::Mongoid::OBJECT_ID field :bson_binary_field, type: BSON::Binary field :date_field, type: Date field :datetime_field, type: DateTime diff --git a/spec/rails_admin/config/fields/types/bson_object_id_spec.rb b/spec/rails_admin/config/fields/types/bson_object_id_spec.rb index 24ed287ce5..1d895e1931 100644 --- a/spec/rails_admin/config/fields/types/bson_object_id_spec.rb +++ b/spec/rails_admin/config/fields/types/bson_object_id_spec.rb @@ -2,4 +2,25 @@ describe RailsAdmin::Config::Fields::Types::BsonObjectId do it_behaves_like 'a generic field type', :string_field, :bson_object_id + + describe '#parse_value' do + let(:bson) { RailsAdmin::Adapters::Mongoid::OBJECT_ID.new } + let(:field) do + RailsAdmin.config(FieldTest).fields.detect do |f| + f.name == :bson_object_id_field + end + end + + before :each do + RailsAdmin.config do |config| + config.model FieldTest do + field :bson_object_id_field, :bson_object_id + end + end + end + + it 'parse valid bson_object_id', mongoid: true do + expect(field.parse_value(bson.to_s)).to eq bson + end + end end From 00e0263c686b6762c8433d7d883c4de6cfae57ba Mon Sep 17 00:00:00 2001 From: Andrea Dal Ponte Date: Fri, 4 Dec 2015 00:56:25 +0100 Subject: [PATCH 2/3] Update changelog --- CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5400fbce9a..1a78ffbca9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,10 @@ ## [Unreleased](https://github.com/sferik/rails_admin/tree/HEAD) -[Full Changelog](https://github.com/sferik/rails_admin/compare/v0.8.0...HEAD) +[Full Changelog](https://github.com/sferik/rails_admin/compare/v0.8.1...HEAD) + +### Fixed +- Fixed Mongoid BSON object field ([#2495](https://github.com/sferik/rails_admin/issues/2495)) ## [0.8.1](https://github.com/sferik/rails_admin/tree/v0.8.1) - 2015-11-24 From fb9d14b5f214b299cc85130033ee3cab847072d6 Mon Sep 17 00:00:00 2001 From: Andrea Dal Ponte Date: Fri, 4 Dec 2015 11:52:30 +0100 Subject: [PATCH 3/3] Refactor: split Bson parsing logic --- lib/rails_admin/adapters/mongoid.rb | 28 ++++-------------- .../adapters/mongoid/association.rb | 1 + lib/rails_admin/adapters/mongoid/bson.rb | 29 +++++++++++++++++++ spec/dummy_app/app/mongoid/field_test.rb | 2 +- .../fields/types/bson_object_id_spec.rb | 2 +- 5 files changed, 38 insertions(+), 24 deletions(-) create mode 100644 lib/rails_admin/adapters/mongoid/bson.rb diff --git a/lib/rails_admin/adapters/mongoid.rb b/lib/rails_admin/adapters/mongoid.rb index 3bd4857be6..485c80b53f 100644 --- a/lib/rails_admin/adapters/mongoid.rb +++ b/lib/rails_admin/adapters/mongoid.rb @@ -3,23 +3,15 @@ require 'rails_admin/adapters/mongoid/abstract_object' require 'rails_admin/adapters/mongoid/association' require 'rails_admin/adapters/mongoid/property' +require 'rails_admin/adapters/mongoid/bson' module RailsAdmin module Adapters module Mongoid - DISABLED_COLUMN_TYPES = ['Range', 'Moped::BSON::Binary', 'BSON::Binary', 'Mongoid::Geospatial::Point'] - OBJECT_ID ||= begin - if defined?(Moped::BSON) - Moped::BSON::ObjectId - elsif defined?(BSON::ObjectId) - BSON::ObjectId - end - end + DISABLED_COLUMN_TYPES = %w(Range Moped::BSON::Binary BSON::Binary Mongoid::Geospatial::Point) def parse_object_id(value) - OBJECT_ID.from_string(value) - rescue BSON::ObjectId::Invalid, BSON::InvalidObjectId, Moped::Errors::InvalidObjectId - nil + Bson.parse_object_id(value) end def new(params = {}) @@ -30,10 +22,10 @@ def get(id) AbstractObject.new(model.find(id)) rescue => e raise e if %w( - BSON::InvalidObjectId Mongoid::Errors::DocumentNotFound Mongoid::Errors::InvalidFind Moped::Errors::InvalidObjectId + BSON::InvalidObjectId ).exclude?(e.class.to_s) end @@ -130,11 +122,7 @@ def query_conditions(query, fields = config.list.fields.select(&:queryable?)) statements.concat make_condition_for_current_collection(field, conditions_per_collection) end - if statements.any? - {'$or' => statements} - else - {} - end + statements.any? ? {'$or' => statements} : {} end # filters example => {"string_field"=>{"0055"=>{"o"=>"like", "v"=>"test_value"}}, ...} @@ -157,11 +145,7 @@ def filter_conditions(filters, fields = config.list.fields.select(&:filterable?) end end - if statements.any? - {'$and' => statements} - else - {} - end + statements.any? ? {'$and' => statements} : {} end def parse_collection_name(column) diff --git a/lib/rails_admin/adapters/mongoid/association.rb b/lib/rails_admin/adapters/mongoid/association.rb index ea288e6371..8ae7532d9a 100644 --- a/lib/rails_admin/adapters/mongoid/association.rb +++ b/lib/rails_admin/adapters/mongoid/association.rb @@ -3,6 +3,7 @@ module Adapters module Mongoid class Association attr_reader :association, :model + def initialize(association, model) @association = association @model = model diff --git a/lib/rails_admin/adapters/mongoid/bson.rb b/lib/rails_admin/adapters/mongoid/bson.rb new file mode 100644 index 0000000000..d0750992f5 --- /dev/null +++ b/lib/rails_admin/adapters/mongoid/bson.rb @@ -0,0 +1,29 @@ +require 'mongoid' + +module RailsAdmin + module Adapters + module Mongoid + class Bson + OBJECT_ID = begin + if defined?(Moped::BSON) + Moped::BSON::ObjectId + elsif defined?(BSON::ObjectId) + BSON::ObjectId + end + end + + class << self + def parse_object_id(value) + OBJECT_ID.from_string(value) + rescue => e + raise e if %w( + Moped::Errors::InvalidObjectId + BSON::ObjectId::Invalid + BSON::InvalidObjectId + ).exclude?(e.class.to_s) + end + end + end + end + end +end diff --git a/spec/dummy_app/app/mongoid/field_test.rb b/spec/dummy_app/app/mongoid/field_test.rb index ec23cf6764..3c9488f750 100644 --- a/spec/dummy_app/app/mongoid/field_test.rb +++ b/spec/dummy_app/app/mongoid/field_test.rb @@ -12,7 +12,7 @@ class FieldTest field :array_field, type: Array field :big_decimal_field, type: BigDecimal field :boolean_field, type: Boolean - field :bson_object_id_field, type: RailsAdmin::Adapters::Mongoid::OBJECT_ID + field :bson_object_id_field, type: RailsAdmin::Adapters::Mongoid::Bson::OBJECT_ID field :bson_binary_field, type: BSON::Binary field :date_field, type: Date field :datetime_field, type: DateTime diff --git a/spec/rails_admin/config/fields/types/bson_object_id_spec.rb b/spec/rails_admin/config/fields/types/bson_object_id_spec.rb index 1d895e1931..d11ec37690 100644 --- a/spec/rails_admin/config/fields/types/bson_object_id_spec.rb +++ b/spec/rails_admin/config/fields/types/bson_object_id_spec.rb @@ -4,7 +4,7 @@ it_behaves_like 'a generic field type', :string_field, :bson_object_id describe '#parse_value' do - let(:bson) { RailsAdmin::Adapters::Mongoid::OBJECT_ID.new } + let(:bson) { RailsAdmin::Adapters::Mongoid::Bson::OBJECT_ID.new } let(:field) do RailsAdmin.config(FieldTest).fields.detect do |f| f.name == :bson_object_id_field