Latest Release Breaks STI #80

Closed
aaronchi opened this Issue Aug 26, 2011 · 8 comments

2 participants

@aaronchi

I had some trouble tracking this down, but for some reason, in 0.2.0, if add tire to an ActiveRecord model that is inherited via STI, it breaks Rail's ability to correctly set the type column. Search still works, but for some reason, Rails can no longer correctly set the type column for the model when new records are created.

@karmi
Owner

Hmm, that is weird -- could you check if STI uses a _type method of the model? Previously, both type and _type methods were defined for the model instance in question.

@karmi
Owner

Any news on this?

@aaronchi

I don't believe STI uses _type but something happens when you include Tire::Model::Search that breaks the type handling. I'll take a deeper look sometime next week and see if I can figure out what's going on.

@aaronchi

Ah yes. I believe rails does use _type internally. there may be other stuff going on as well.

In my model:

class Image < Asset

end

without tire:

> Image.first._type
 ←[1m←[35mImage Load (1.0ms)←[0m  SELECT `assets`.* FROM `assets` WHERE `assets`.`type` IN ('Image') LIMIT 1
=> "Image"

with tire:

> Image.first._type
←[1m←[35mImage Load (1.0ms)←[0m  SELECT `assets`.* FROM `assets` WHERE `assets`.`type` IN ('Image') LIMIT 1
 NoMethodError: You have a nil object when you didn't expect it!
 You might have expected an instance of Array.
 The error occurred while evaluating nil.[]
    from C:/Ruby192/lib/ruby/gems/1.9.1/gems/tire-0.3.1/lib/tire/model/search.rb:199:in `block (2 levels) in <class:InstanceMethodsProxy>'
    from C:/Ruby192/lib/ruby/gems/1.9.1/gems/tire-0.3.1/lib/tire/model/search.rb:259:in `_type'
    from (irb):66
    from C:/Ruby192/lib/ruby/gems/1.9.1/gems/railties-3.1.0/lib/rails/commands/console.rb:45:in `start'
    from C:/Ruby192/lib/ruby/gems/1.9.1/gems/railties-3.1.0/lib/rails/commands/console.rb:8:in `start'
    from C:/Ruby192/lib/ruby/gems/1.9.1/gems/railties-3.1.0/lib/rails/commands.rb:40:in `<top (required)>'
    from script/rails:35:in `require'
    from script/rails:35:in `<main>'

also:

> Image.first
  ←[1m←[36mImage Load (0.0ms)←[0m  ←[1mSELECT `assets`.* FROM `assets` WHERE `assets`.`type` IN ('Image') LIMIT 1←[0m
(Object doesn't support #inspect)
=>
@karmi
Owner

Hmm, _type should not be added when the model has that method already defined. Could you create a Rails template which generates some STI scaffolds etc, so I have less work with recreating that?

@karmi
Owner

Any news? Neither #type nor #_type really should be added to your model on 0.3.

@aaronchi

I added a basic rails app that shows the problem here:
https://github.com/aaronchi/tire-sti

Tire is defined on the Image model which is a child of the Asset model. If you go into console and try to create an image

Image.create(:title => 'Something')

You get the error I was describing above:

 NoMethodError: You have a nil object when you didn't expect it!
 You might have expected an instance of Array.
 The error occurred while evaluating nil.[]
    from C:/Ruby192/lib/ruby/gems/1.9.1/gems/tire-0.3.2/lib/tire/model/search.rb:199:in `block (2 levels) in <class:InstanceMethodsProxy>'
    from C:/Ruby192/lib/ruby/gems/1.9.1/gems/tire-0.3.2/lib/tire/model/search.rb:259:in `_type'

If you move the tire index to the Asset model, creation works fine because it doesn't conflict with the STI type variable.

@karmi karmi added a commit that closed this issue Sep 9, 2011
@karmi [FIX] Moved unneccessary methods from Model::Search to Model::Persist…
…ance, leaving only `Model#matches`

Closes #80.
fde6746
@karmi karmi closed this in fde6746 Sep 9, 2011
@karmi
Owner

OK, thanks for being stubborn on this. I have found the offending code and moved the the unneccessary methods into Model::Persistence. The sample app works now.

@sarmiena sarmiena pushed a commit to sarmiena/tire that referenced this issue Mar 25, 2012
@karmi [#218] Cleaned up the codebase for loading instances of multiple clas…
…ses / single table inheritance

Previously, eagerly loading instances of multiple classes with the `load: true` option was not possible.

Now, you can use the `Tire.search` DSL for searching and loading multiple models from one or several indices:

    ActiveRecordModelOne.create :title => 'Title One', timestamp: Time.now.to_i

    s = Tire.search ['active_record_model_one', 'active_record_model_two'], :load => true do
                  query { string 'title' }
                  sort  { by :timestamp }
                end

    puts s.results[0].inspect
    # => <ActiveRecordModelOne id: 1, title: "Title One", timestamp: "1332437294">

Fixed indentation/formatting/whitespace throughout the suite :/

Fixes #80, fixes #178, fixes #218, fixes #277, closes #131.
d5c08fb
@karmi karmi added a commit that referenced this issue Jan 4, 2013
@karmi Refactored the `update_index` method for search and persistence integ…
…ration

Previously, both the ActiveModel search integration and the Persistence support
relied on shared `update_index` implementation.

This has been confusing, and also a source of bugs in Rails
applications using "single table inheritance" or
["single collection"](http://mongomapper.com/documentation/plugins/single-collection-inheritance.html)
in MongoMapper applications, which both rely on the `_type` property.

The `update_index` method has been extracted to the `Tire::Model::Persistence::Storage` module,
and the offending code has been removed from the ActiveModel integration.

Closes #477, fixes #80 and #394.
b5e9210
@MrRuru MrRuru added a commit that referenced this issue Apr 30, 2013
@karmi Refactored the `update_index` method for search and persistence integ…
…ration

Previously, both the ActiveModel search integration and the Persistence support
relied on shared `update_index` implementation.

This has been confusing, and also a source of bugs in Rails
applications using "single table inheritance" or
["single collection"](http://mongomapper.com/documentation/plugins/single-collection-inheritance.html)
in MongoMapper applications, which both rely on the `_type` property.

The `update_index` method has been extracted to the `Tire::Model::Persistence::Storage` module,
and the offending code has been removed from the ActiveModel integration.

Closes #477, fixes #80 and #394.
2fdc640
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment