couchbase / couchbase-ruby-model

The Active Model implementation for Couchbase Server built on couchbase-ruby-client
61 stars 23 forks source link

Provide proper attribute inheritance when subclassing Models #2

Closed mje113 closed 11 years ago

mje113 commented 11 years ago

Currently, Couchbase::Model stores attributes (and views) in a class variable set to a mutable hash. This breaks the expected subclass attribute inheritance behavior that ActiveModel provides.

Example:

class A < Couchbase::Model
  attribute :one
end

class B < A
  attribute :two
end

B.attributes #=> {:one => nil, :two => nil}
A.attributes #=> {:one => nil, :two => nil}
B.attributes.object_id == A.attributes.object_id #=> true

This patch enables proper inheritance when ::Rails is present by using Class#class_attribute and detecting subclass additions to attribute:

class A < Couchbase::Model
  attribute :one
end

class B < A
  attribute :two
end

A.attributes #=> {:one => nil}
B.attributes #=> {:one => nil, :two => nil}
B.attributes.object_id != A.attributes.object_id #=> true
mje113 commented 11 years ago

Ok, I managed to rewrite the patch to not rely on class_attribute. It was actually breaking some non-active model related functionality anyway.

The main change with @@attributes & @@views is instead of tracking each sub-classes' attributes in their own key on the one class variable, each subclass dupes it's parent's class or instance variable version into it's own class-level instance variable.

mje113 commented 11 years ago

Thanks, missed some cleanup.

avsej commented 11 years ago

Hi Mike, sorry for delay. I've uploaded your patch into our master repository, which managed by gerrit code review tool. Could you register there and accept our CLA? After that I can merge your change. Github is just a mirror of that repository. Your patch located here:

http://review.couchbase.org/23392

Thanks for your contribution

Sergey Avseyev