railsadminteam / rails_admin

RailsAdmin is a Rails engine that provides an easy-to-use interface for managing your data
MIT License
7.88k stars 2.25k forks source link

crash on querying models with json columns #2502

Open wlipa opened 8 years ago

wlipa commented 8 years ago

If you search / query a model with a jsonb field, and your search query is plain text (like "aa"), then the server will throw a JSON parser exception like the following:

 JSON::ParserError (757: unexpected token at '11'):
   json (1.8.3) lib/json/common.rb:155:in `parse'
   json (1.8.3) lib/json/common.rb:155:in `parse'
   rails_admin (0.8.1) lib/rails_admin/config/fields/types/json.rb:17:in `parse_value'
   rails_admin (0.8.1) lib/rails_admin/adapters/active_record.rb:121:in `block in query_scope'
   rails_admin (0.8.1) lib/rails_admin/adapters/active_record.rb:120:in `each'
   rails_admin (0.8.1) lib/rails_admin/adapters/active_record.rb:120:in `query_scope'
   rails_admin (0.8.1) lib/rails_admin/adapters/active_record.rb:33:in `all'
   rails_admin (0.8.1) app/controllers/rails_admin/main_controller.rb:131:in `get_collection'
   rails_admin (0.8.1) app/controllers/rails_admin/main_controller.rb:37:in `list_entries'
   rails_admin (0.8.1) lib/rails_admin/config/actions/index.rb:30:in `block (2 levels) in <class:Index>'
   rails_admin (0.8.1) app/controllers/rails_admin/main_controller.rb:22:in `instance_eval'

A workaround is to configure the json field as non-queryable.

kevinryantao commented 8 years ago

I also reproduced this exact error.

kevinryantao commented 8 years ago

I believe this bug was introduced from this commit: https://github.com/sferik/rails_admin/commit/fe2899fdeb48f4cdf33c0616f0024df13fd32f14

The problem is that it tries to run JSON.parse(value) even when it's not valid JSON.

zegomesjf commented 8 years ago

:+1: for @kevinryantao solution

bbenezech commented 8 years ago

I guess JSONB should be non-queryable. Can someone provide a PR?

krainboltgreene commented 8 years ago

It's not that it should be non-queryable, but rather it should be coerced into text.

jtoy commented 8 years ago

is this fixed? this seems like something a lot of people would run into

Saicheg commented 8 years ago

👍 for this issue

mapreal19 commented 8 years ago

Until this is fixed you could use this workaround in your configuration:

ActiveRecord::Base.descendants.each do |model|
  config.model "#{model.name}" do
    exclude_fields_if { type == :json }
  end
end
jtoy commented 8 years ago

That seems to exclude it always. Anyway to just exclude that in searches?

-- Sent from my mobile

On Sep 5, 2016, at 10:21 AM, Mario Pérez notifications@github.com wrote:

Until this is fixed you could use this workaround in your configuration:

ActiveRecord::Base.descendants.each do |model| config.model "#{model.name}" do exclude_fields_if { type == :json } end end — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

mapreal19 commented 8 years ago

@jtoy Yes. This will exclude it for all the models that have json columns

jtoy commented 8 years ago

@mapreal19 i couldn't get this working, are you using this?

mapreal19 commented 8 years ago

@jtoy I ended up making those json attrs non-queryable. Give this a try:

 ActiveRecord::Base.descendants.each do |model|
    config.model(model.name.to_s) do
      fields_of_type(:json) { queryable false }
    end
  end
jtoy commented 8 years ago

this works, thank you!

On Sep 22, 2016, at 3:11 AM, Mario Pérez notifications@github.com wrote:

@jtoy https://github.com/jtoy I ended up making those json attrs non-queryable. Give this a try:

ActiveRecord::Base.descendants.each do |model| config.model(model.name.to_s) do fields_of_type(:json) { queryable false } end end — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sferik/rails_admin/issues/2502#issuecomment-248829117, or mute the thread https://github.com/notifications/unsubscribe-auth/AAA5v55JCy-bEDljou-OqThN7uqNXNudks5qsioXgaJpZM4GyFQ_.

fny commented 7 years ago

Note for Rails 5 (or any situation where you have an abstract model defined like ApplicationRecord), you'll need to skip abstract classes:

  ActiveRecord::Base.descendants.each do |model|
    next if model.abstract_class?
    config.model(model.name.to_s) do
      fields_of_type(:json) { queryable false }
    end
  end

Also don't forget you'll probably need to eager load your models beforehand (Rails.application.eager_load!), otherwise they won't be found as descentendants of ActiveRecord::Base when that code runs

MatthiasRMS commented 7 years ago

@jtoy thanks for the workaround, should this go in an initializer or in dev/prod/test.rb? I've tried to add it to a rails_admin.rb initializer and to development.rb file but none worked.

serg-kovalev commented 7 years ago

@MatthiasRMS Did you eager load your models beforehand as @fny suggested?

Rails.application.eager_load!
ActiveRecord::Base.descendants.each do |model|
  next if model.abstract_class? # comment out this line if you are on Rails < 5
  config.model(model.name.to_s) do
    fields_of_type(:json) { queryable false }
  end
end
sbull commented 7 years ago

Here is my workaround, in an initializer:

RailsAdmin::Config::Fields::Types::Json.inspect # Load before override.
class RailsAdmin::Config::Fields::Types::Json
  def queryable?
    false
  end
end
MatthiasRMS commented 7 years ago

None of the above worked for me. This fixed it:

RailsAdmin.config do |config|
  ApplicationRecord.descendants.each do |model|
    next if model.abstract_class?
    config.model(model.name.to_s) do
      exclude_fields_if do
        type == :json
      end

      exclude_fields_if do
        type == :jsonb
      end
    end
  end
end
drewbaumann commented 6 years ago

@MatthiasRMS solution worked for us with small tweaks:

Rails.application.eager_load!
ActiveRecord::Base.descendants.each do |model|
  next if model.abstract_class?
  config.model(model.name.to_s) do
    exclude_fields_if do
      [:json, :jsonb].include?(type)
    end
  end
end
jeromedalbert commented 5 years ago

I improved on @sbull's solution by making it work with the current rails_admin version (1.4.2) at the time of writing:

# Prevent json fields from being queried, since RailsAdmin cannot handle them
RailsAdmin::Config::Fields::Types::Json.inspect
class RailsAdmin::Config::Fields::Types::Json
  register_instance_option :queryable? do
    false
  end
end

The advantage of this technique is that it does not force you to eager load your app.

jeromedalbert commented 5 years ago

Alternatively the following code works too, and does not crash with a 500 error if you enter bad JSON data in a RailsAdmin form:

# Don't blow up RailsAdmin on JSON searches or invalid JSON formats
class RailsAdmin::Config::Fields::Types::Json
  register_instance_option :formatted_value do
    if value.is_a?(Hash) || value.is_a?(Array)
      JSON.pretty_generate(value)
    else
      value
    end
  end

  def parse_value(value)
    value.present? ? JSON.parse(value) : nil
  rescue JSON::ParserError
    value
  end
end
mltsy commented 4 years ago

This seems to prevent an error, but even when I put in a valid JSON value to query by, the query by doesn't actually search the JSON field (it just returns no records), or if I filter specifically by the JSON field (an array in my case), it returns all records, regardless of a match.

AhmadIbrahiim commented 4 years ago

This above solution seems to prevent rails_admin from render any JSON, Any recommendation on how to return it as plain text?

jacobjlevine commented 2 years ago

I think I found a way of hacking the search function to work with JSONB fields. It casts the entire JSONB field as text, which means your search will also include key names.

list do
  field :field_name, :string do
    queryable true
    searchable_columns [{ column: "field_name::TEXT", type: :string }]
  end
end

Hope this helps! Maybe someone else can figure out how to eliminate the key names? And maybe this could also serve as the foundation for a more general solution that can be integrated into the core project?

Edit: I don't mean to pass the buck on this. I've just spent half a day trying to figure this out and I'm zonked right now.