Activerecord find_or_create_by doesn't interpret symbols #12570

Closed
comjf opened this Issue Oct 17, 2013 · 9 comments

Comments

Projects
None yet
5 participants

comjf commented Oct 17, 2013

I have a simple model that has integers defined as symbols.

class Site < ActiveRecord::Base
  BUILD = { manual: 0, cap: 1, recap: 2 }

  def build
    BUILD.key(read_attribute(:build))
  end

  def build=(b)
    write_attribute(:build, BUILD[b])
  end
end

When I run db:seed, I get duplicate results for any record that isn't using the default value (:manual or 0)

Site.find_or_create_by(repo: 'bob', env: 'staging', build: :recap)
Site.find_or_create_by(repo: 'joe', env: 'staging', build: :manual)
Site.find_or_create_by(repo: 'jim', env: 'staging', build: :manual)

To me this indicates that find_or_create_by isn't properly interpreting the build symbol... or I am doing something completely wrong and should feel bad.

Please see: https://gist.github.com/comjf/7035218

Contributor

josephzidell commented Oct 17, 2013

How about Site.find_or_create_by(repo: 'bob', env: 'staging', build: BUILD[:recap])?

Contributor

pftg commented Oct 17, 2013

I am not clear, what is your problem. May you fix my test for this issue, because I have not reproduced your bug for 4.0.0 rails:

gemfile_path          = "#{File.basename(__FILE__, '.rb')}.gemfile"
ENV['BUNDLE_GEMFILE'] = File.expand_path(gemfile_path)

unless File.exists?(gemfile_path)
  File.write(gemfile_path, <<-GEMFILE)
    source 'https://rubygems.org'
    #gem 'rails', github: 'rails/rails'
    #gem 'rails', path: '../rails'
    gem 'rails', '4.0.1.rc1'
    #gem 'pg'
    gem 'sqlite3'
  GEMFILE

  system "bundle --gemfile=#{gemfile_path} --clean"
end

require 'bundler'
Bundler.setup(:default)

require 'active_record'
require 'minitest/autorun'
require 'logger'

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')

#database_configuration = { adapter: 'postgresql', database: 'rails_issues' }
#
#adapter_tasker = ActiveRecord::Tasks::PostgreSQLDatabaseTasks.new(database_configuration.stringify_keys)
#adapter_tasker.drop
#adapter_tasker.create


ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :sites, force: true do |t|
    t.string :repo
    t.string :env
    t.string :build
  end
end

class Site < ActiveRecord::Base
  BUILD = { manual: 0, cap: 1, recap: 2 }

  def build
    BUILD.key(read_attribute(:build))
  end

  def build=(b)
    write_attribute(:build, BUILD[b])
  end
end

class TestBuildSymbol < MiniTest::Unit::TestCase
  def test_find_or_create_by_with_build
    assert_equal :recap, Site.find_or_create_by(repo: 'bob', env: 'staging', build: :recap).build
    assert_equal :manual, Site.find_or_create_by(repo: 'joe', env: 'staging', build: :manual).build
    assert_equal :manual, Site.find_or_create_by(repo: 'jim', env: 'staging', build: :manual).build
    assert_equal 3, Site.count
  end
end

comjf commented Oct 18, 2013

Thank you for the reply, I've modified your code, please see: https://gist.github.com/comjf/7035218

Contributor

pftg commented Oct 20, 2013

@comjf this is expected behaviour, because where build query without pre-processing through wrappers for attributes: Site.where(repo: 'jim', env: 'staging', build: :manual).take || Site.create(repo: 'jim', env: 'staging', build: :manual).

So your problem, is to do pre-processing/type-casting Symbols => Integer through BUILD = { manual: 0, cap: 1, recap: 2 } for building where conditions.

As for me, best solution is using build: BUILD[:manual] as mentioned above.

comjf commented Oct 21, 2013

I think I understand the solution, however, I'm not sure why this the find part of the program doesn't work, yet the create does work. It's obviously not being created with 'manual'... the create part processes through the wrappers. If you think this isn't a bug, I'll go ahead and close it out... just seems odd to me..

I'd expect both parts to fail, or neither to. Having them work in different ways just seems inconsistent.

Thank you for all your help!

Contributor

pftg commented Oct 21, 2013

You may ask about such problem on Ruby on Rails Core Mailing List. Also my proposal to do it readable

Site.new(repo: 'jim', env: 'staging', build: :manual).find_by_self_or_save # will use internal attributes to search for similar records
Contributor

sheerun commented Oct 23, 2013

@pftg It would need to accept block for extra attributes... little limited imho. You don't want to create model with only attributes you want to search for. They're almost always invalid. Moreover how to know which attributes of Site include in query? repo, env and build were passed, but most likely model has other attributes modified in hooks.

Contributor

pftg commented Oct 23, 2013

@sheerun I tried to show simple semantics for using of Search by Example like it is in Hibernate, you may review: https://docs.jboss.org/hibernate/orm/3.3/reference/en/html/querycriteria.html#querycriteria-examples, elsewhere I do not think that this should be added to rails in any way, but just brainstorming.

Contributor

laurocaetano commented Apr 11, 2014

Hi @comjf!

It is expected to work for create_by but not for find_by. I created a gist based on yours and I explained in the tests why it's expected to behave this way.

Closing since it's the expected behavior.
Thanks for reporting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment