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/…
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.
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
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.
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.
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’
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.
