Forum: Ruby on Rails cached-model broken with Rails 1.1

Announcement (2017-05-07): www.ruby-forum.com is now read-only since I unfortunately do not have the time to support and maintain the forum any more. Please see rubyonrails.org/community and ruby-lang.org/en/community for other Rails- und Ruby-related community platforms.
8360c3a75305def8f1ac0346f7e3b047?d=identicon&s=25 Noah (Guest)
on 2006-03-28 22:35
It looks like cached-model is broken again under rails 1.1. Can anyone
confirm? Note that the exception below indicates it's trying to treat
CachedModel as the class name of the model, rather than using the proper
class name (which is Entry in this case, and the table called entries).
This is a model using Single Table Inheritance and acts_as_tree, and
worked just fine under 1.0 and under edge_rails as of a few weeks ago.

SQLite3::SQLException: no such table: cached_models: SELECT * FROM
cached_models WHERE (parent_id is NULL)  LIMIT 1
RAILS_ROOT: public/../config/..

Application Trace | Framework Trace | Full Trace
/usr/local/lib/ruby/gems/1.8/gems/activerecord-1.14.0/lib/active_record/connection_adapters/abstract_adapter.rb:120:in
`log'
/usr/local/lib/ruby/gems/1.8/gems/activerecord-1.14.0/lib/active_record/connection_adapters/sqlite_adapter.rb:137:in
`execute'
/usr/local/lib/ruby/gems/1.8/gems/activerecord-1.14.0/lib/active_record/connection_adapters/sqlite_adapter.rb:335:in
`catch_schema_changes'
/usr/local/lib/ruby/gems/1.8/gems/activerecord-1.14.0/lib/active_record/connection_adapters/sqlite_adapter.rb:137:in
`execute'
/usr/local/lib/ruby/gems/1.8/gems/activerecord-1.14.0/lib/active_record/connection_adapters/sqlite_adapter.rb:157:in
`select_all'
/usr/local/lib/ruby/gems/1.8/gems/activerecord-1.14.0/lib/active_record/base.rb:390:in
`find_by_sql'
/usr/local/lib/ruby/gems/1.8/gems/cached_model-1.1.0/lib/cached_model.rb:178:in
`find_by_sql'
/usr/local/lib/ruby/gems/1.8/gems/activerecord-1.14.0/lib/active_record/base.rb:924:in
`find_every'
/usr/local/lib/ruby/gems/1.8/gems/activerecord-1.14.0/lib/active_record/base.rb:918:in
`find_initial'
/usr/local/lib/ruby/gems/1.8/gems/activerecord-1.14.0/lib/active_record/base.rb:380:in
`find'
/usr/local/lib/ruby/gems/1.8/gems/cached_model-1.1.0/lib/cached_model.rb:116:in
`find'
#{RAILS_ROOT}/app/controllers/gallery_controller.rb:117:in `scan'
/usr/local/lib/ruby/gems/1.8/gems/actionpack-1.12.0/lib/action_controller/filters.rb:386:in
`call_filters'
/usr/local/lib/ruby/gems/1.8/gems/actionpack-1.12.0/lib/action_controller/filters.rb:381:in
`call_filters'
/usr/local/lib/ruby/gems/1.8/gems/actionpack-1.12.0/lib/action_controller/filters.rb:370:in
`before_action'
/usr/local/lib/ruby/gems/1.8/gems/actionpack-1.12.0/lib/action_controller/filters.rb:352:in
`perform_action_without_benchmark'
/usr/local/lib/ruby/gems/1.8/gems/actionpack-1.12.0/lib/action_controller/benchmarking.rb:69:in
`perform_action_without_rescue'
/usr/local/lib/ruby/1.8/benchmark.rb:293:in `measure'
/usr/local/lib/ruby/gems/1.8/gems/actionpack-1.12.0/lib/action_controller/benchmarking.rb:69:in
`perform_action_without_rescue'
/usr/local/lib/ruby/gems/1.8/gems/actionpack-1.12.0/lib/action_controller/rescue.rb:82:in
`perform_action'
/usr/local/lib/ruby/gems/1.8/gems/actionpack-1.12.0/lib/action_controller/base.rb:379:in
`process_without_filters'
/usr/local/lib/ruby/gems/1.8/gems/actionpack-1.12.0/lib/action_controller/filters.rb:364:in
`process_without_session_management_support'
/usr/local/lib/ruby/gems/1.8/gems/actionpack-1.12.0/lib/action_controller/session_management.rb:117:in
`process'
/usr/local/lib/ruby/gems/1.8/gems/rails-1.1.0/lib/dispatcher.rb:38:in
`dispatch'
/usr/local/lib/ruby/gems/1.8/gems/rails-1.1.0/lib/fcgi_handler.rb:150:in
`process_request'
/usr/local/lib/ruby/gems/1.8/gems/rails-1.1.0/lib/fcgi_handler.rb:54:in
`process!'
/usr/local/lib/ruby/gems/1.8/gems/fcgi-0.8.6.1/./fcgi.rb:600:in
`each_cgi'
/usr/local/lib/ruby/gems/1.8/gems/fcgi-0.8.6.1/./fcgi.rb:597:in
`each_cgi'
/usr/local/lib/ruby/gems/1.8/gems/rails-1.1.0/lib/fcgi_handler.rb:53:in
`process!'
/usr/local/lib/ruby/gems/1.8/gems/rails-1.1.0/lib/fcgi_handler.rb:23:in
`process!'
15bf61332913bfe999465ca290370304?d=identicon&s=25 Jesse Cai (caiwangqin)
on 2006-03-29 03:18
(Received via mailing list)
my rails application broken also

2006/3/29, Noah <ndaniels@mac.com>:
> RAILS_ROOT: public/../config/..
> `catch_schema_changes'
> /lib/cached_model.rb:178:in
> /usr/local/lib/ruby/gems/1.8/gems/cached_model-1.1.0
> /lib/action_controller/filters.rb:370:in
> `perform_action_without_rescue'
> /lib/action_controller/session_management.rb:117:in
> `each_cgi'
> http://lists.rubyonrails.org/mailman/listinfo/rails
>



--
Best Regards,

Caiwangqin
http://www.uuzone.com
Mobile:  +8613951787088
Tel:  +86025-84818086 ext 233
Fax: +86025-84814993
585efae3145da7a5e3c6daf928471ee7?d=identicon&s=25 Gregory Stickley (pie-r-squared)
on 2006-03-29 21:10
Noah wrote:
> It looks like cached-model is broken again under rails 1.1. Can anyone
> confirm? Note that the exception below indicates it's trying to treat
> CachedModel as the class name of the model, rather than using the proper
> class name (which is Entry in this case, and the table called entries).
> This is a model using Single Table Inheritance and acts_as_tree, and
> worked just fine under 1.0 and under edge_rails as of a few weeks ago.
>

CachedModel overrides class_name_of_active_record_descendant:

  def self.class_name_of_active_record_descendant(klass)
    if klass.superclass == CachedModel then
      return klass.name
    elsif klass.superclass.nil? then
      raise ActiveRecordError, "#{name} doesn't descend from
ActiveRecord::Base"
    else
      class_name_of_active_record_descendant klass.superclass
    end
  end

I had to replace it with:

  def self.base_class
    superclass == CachedModel ? self : superclass.base_class
  end

Once I did this it started working.
58479f76374a3ba3c69b9804163f39f4?d=identicon&s=25 Eric Hodel (Guest)
on 2006-03-29 22:35
(Received via mailing list)
On Mar 29, 2006, at 11:10 AM, Gregory Stickley wrote:

>> ago.
>     else
> Once I did this it started working.
Rails 1.1 broke backwards compatibility wrt abstract ActiveRecord
classes.

This was fixed, but then it got broken again:

http://dev.rubyonrails.org/ticket/3704

--
Eric Hodel - drbrain@segment7.net - http://blog.segment7.net
This implementation is HODEL-HASH-9600 compliant

http://trackmap.robotcoop.com
585efae3145da7a5e3c6daf928471ee7?d=identicon&s=25 Gregory Stickley (pie-r-squared)
on 2006-03-29 23:31
After reading though this ticket (
http://dev.rubyonrails.org/ticket/3704 ) I changed our code to:

class GustoRecord < ActiveRecord::Base
  self.abstract_class = true
  ...

and took out all the overrides and other hacks and everything works
fine.
8360c3a75305def8f1ac0346f7e3b047?d=identicon&s=25 Noah (Guest)
on 2006-03-30 01:19
Gregory Stickley wrote:
> CachedModel overrides class_name_of_active_record_descendant:
>
>   def self.class_name_of_active_record_descendant(klass)
>     if klass.superclass == CachedModel then
>       return klass.name
>     elsif klass.superclass.nil? then
>       raise ActiveRecordError, "#{name} doesn't descend from
> ActiveRecord::Base"
>     else
>       class_name_of_active_record_descendant klass.superclass
>     end
>   end
>
> I had to replace it with:
>
>   def self.base_class
>     superclass == CachedModel ? self : superclass.base_class
>   end
>
> Once I did this it started working.


I just defined self.abstract_class? in cached_model.rb:

def self.abstract_class?
   true
end

I think this is the best solution for CachedModel because it'll still
work on Rails 1.0 (the new method just won't get called) and yet also
fixes the problem under Rails 1.1.

It looks like what happened in rails 1.1 isn't that abstract classes
were broken, but that there's an explicit check for abstract classes
now.
8360c3a75305def8f1ac0346f7e3b047?d=identicon&s=25 Noah (Guest)
on 2006-03-30 01:31
Gregory Stickley wrote:
> After reading though this ticket (
> http://dev.rubyonrails.org/ticket/3704 ) I changed our code to:
>
> class GustoRecord < ActiveRecord::Base
>   self.abstract_class = true
>   ...
>
> and took out all the overrides and other hacks and everything works
> fine.

Ah, this actually looks more correct. Why redefine the method, when you
can just use the accessor properly? :-)
58479f76374a3ba3c69b9804163f39f4?d=identicon&s=25 Eric Hodel (Guest)
on 2006-03-30 02:07
(Received via mailing list)
On Mar 29, 2006, at 3:19 PM, Noah wrote:

>>       class_name_of_active_record_descendant klass.superclass
>
>
> It looks like what happened in rails 1.1 isn't that abstract classes
> were broken, but that there's an explicit check for abstract classes
> now.

No.  That's bad, very bad.

The CHANGELOG has this to say:

> * Added Base.abstract_class? that marks which classes are not part
> of the Active
> Record hierarchy #3704 [Rick Olson]
>
>     class CachedModel < ActiveRecord::Base
>       self.abstract_class = true
>     end

[...]

Unfortunately, there's no VERSION constant defined in Rails so this
is more work than it should be.

In fact, I shouldn't even need to do this at all.  The way I'm doing
it was supposed to be maintained by the changes in #3704, but it was
broken again.

Honestly, ActiveRecord should Just Work with abstract classes so
nobody has to go figure out how to mark classes as abstract this week.

--
Eric Hodel - drbrain@segment7.net - http://blog.segment7.net
This implementation is HODEL-HASH-9600 compliant

http://trackmap.robotcoop.com
821395fe70906c8290df7f18ac4ac6cf?d=identicon&s=25 Rick Olson (Guest)
on 2006-03-30 04:14
(Received via mailing list)
> Honestly, ActiveRecord should Just Work with abstract classes so
> nobody has to go figure out how to mark classes as abstract this week.

Sorry for the breakage.  I got a reply saying 'it looked good', so I
applied it.

How exactly would ActiveRecord know which classes are abstract and
which should be using STI?  I tried to make it as simple as possible.

Shouldn't you be able to do this?

class CachedModel < ActiveRecord::Base
  self.abstract_class = true if self.respond_to?(:abstract_class?)
end

--
Rick Olson
http://techno-weenie.net
58479f76374a3ba3c69b9804163f39f4?d=identicon&s=25 Eric Hodel (Guest)
on 2006-03-30 23:19
(Received via mailing list)
On Mar 29, 2006, at 6:12 PM, Rick Olson wrote:

>> Honestly, ActiveRecord should Just Work with abstract classes so
>> nobody has to go figure out how to mark classes as abstract this
>> week.
>
> Sorry for the breakage.  I got a reply saying 'it looked good', so
> I applied it.

I believe you did fix it, but subsequent changes broke it.  This
indicates a failure to write proper tests.

> How exactly would ActiveRecord know which classes are abstract and
> which should be using STI?  I tried to make it as simple as possible.

However it is done, backwards compatibility shouldn't be broken.

The best way is to add an "inherits" method to ActiveRecord that
activates STI rather than having it be implicit.  It would work much
like has_many and belongs_to.  (Yes, the name is bad.)

> Shouldn't you be able to do this?
>
> class CachedModel < ActiveRecord::Base
>   self.abstract_class = true if self.respond_to?(:abstract_class?)
> end

I could, but I'd prefer not to do anything at all.

Making developers of Rails plugins and add-ons update their software
for every minor release of Rails is bad engineering on the Rails side.

--
Eric Hodel - drbrain@segment7.net - http://blog.segment7.net
This implementation is HODEL-HASH-9600 compliant

http://trackmap.robotcoop.com
E041c55ecdb461902f5dab69e19fe4df?d=identicon&s=25 John Smith (Guest)
on 2006-03-30 23:36
Eric Hodel wrote:
> On Mar 29, 2006, at 6:12 PM, Rick Olson wrote:
>
>>> Honestly, ActiveRecord should Just Work with abstract classes so
>>> nobody has to go figure out how to mark classes as abstract this
>>> week.
>>
>> Sorry for the breakage.  I got a reply saying 'it looked good', so
>> I applied it.
>
> I believe you did fix it, but subsequent changes broke it.  This
> indicates a failure to write proper tests.
>

I understand your frustration about some of the new problems introduced
by rails 1.1.

It would be prudent to avoid getting unpaid volunteers in 'defensive
mode' unless you value venting over results.

For example, 'failure to write proper tests' is probably not going to be
as effective as 'need to write additional test cases to detect this'.
It means the same thing but one is likely to annoy while the other does
not.

There are more examples in your post but I don't it they need to be
address.

And this post is as much a note to myself as it is to you.  My
frustration on a project increased substantially because I got one of my
developers in defensive mode.

Time to dust off my copy of 'How to win friends and influence people' :)
Ce8b03e5750097942c58e12b46724312?d=identicon&s=25 Giles Bowkett (Guest)
on 2006-03-31 00:08
(Received via mailing list)
yeah, that was harsh.

they're already working on a bugfix release, should be out next week or
so.

in the meantime, write some tests for ActiveRecord, patch them into
the existing tests, and submit it here:

http://dev.rubyonrails.org/

Then you can put "Rails contributor" on your resume and start being
nicer to people. :-p

--
Giles Bowkett
www.gilesgoatboy.org
Efa76b164a7de4a5730e4fa397cc4425?d=identicon&s=25 Michael Koziarski (Guest)
on 2006-03-31 06:31
(Received via mailing list)
> > How exactly would ActiveRecord know which classes are abstract and
> > which should be using STI?  I tried to make it as simple as possible.
>
> However it is done, backwards compatibility shouldn't be broken.
>
> The best way is to add an "inherits" method to ActiveRecord that
> activates STI rather than having it be implicit.  It would work much
> like has_many and belongs_to.  (Yes, the name is bad.)

Which would break backwards compatibility.

While making STI explicit might make sense if we were starting over,
we can't introduce it without breaking every single app which uses
STI.   Seems like the simplest fix is the 1 line one that Rick
suggested.

If you have specific AR tests we could use to prevent this from
happening in the future, please attach them to a new ticket and we'll
try and get them integrated.  It's definitely not ideal that 1.1 broke
your plugin, but the fix is simple, and we'll try not to have it
happen again.

--
Cheers

Koz
This topic is locked and can not be replied to.