Object mapper abstraction support. #835

Open
wants to merge 3 commits into from

3 participants

@eric1234

Initial support for ActiveRecord and MongoMapper. Provides obvious place to
add support for addition object mappers and to enhance additional support
needed on ActiveRecord and MongoMapper.

Browing through the MANY tickets regarding adding an object mapper abstraction
it seems the preferred method is to try to let ActiveModel handle most of the
abstraction and just extend the various object matters where ActiveModel is
not abstracting enough. This way most of the SimpleForm code does not care
what object mapper is being used.

Is seems to me there are two classes of methods that we need to extend the
object mappers with.

  1. Methods that return simple data. A great example of this is the "readonly_attributes" method. SimpleForm uses this method that ActiveRecord provides but an object mapper like MongoMapper doesn't have the concept of read only attributes. So in this case we take the simple approach and simply mimic the ActiveRecord method by returning an array (just like ActiveRecord) which is empty (since the concept is not supported).
  2. Methods that return complex data. A good example of this is "reflect_on_association". This returns an ActiveRecord-specific data structure. So in this case I made all data mappers (AR and MM) return a object mapper independent data structure (SimpleForm::Association). Because I am returning an SimpleForm specific data structure I have prefixed these methods with "simple_form_" so we reduce poluting the object mapper's namespace.

This second style method is basically what the testing is already doing. We
are just moving those object mapper independent data structures into the code
instead of having them only used in testing.

I have no doubt that this patch is missing stuff we will need down the road.
I am only using pieces of SimpleForm for my project so there may be areas where
we need more abstraction methods added to the object mappers. Obviously others
are also interested in other object mappers (DataMapper, Mongoid, etc). I feel
like that can come with time by additional patches. Right now we just need a
place to start. An obvious place to put these abstraction methods. Otherwise
we will continue to have dozens of issues where everybody want it but it cannot
get off the ground.

Regarding testing, I did add testing for the two object mapper independent
objects I added (Column and Association). But I was unsure the approach to take
regarding testing the abstraction methods that we add to the object mappers.
I was unsure if we wanted to required the tests to have all those database
types available. Or maybe have tests but skip ones not available. In general
the abstraction methods are simple. Just copying data from the object mapper
specific data structures to the object mapper independent data structures.

If you have suggestions on how to improve this patch I am open and ready to
follow suggestions. But I want to ensure we don't let perfection get in the
way of progress. For me it is ok if we have to refactor our approach as we add
more object mappers in the future. Sorry to be long winded. :)

@eric1234 eric1234 Object mapper abstraction support.
Initial support for ActiveRecord and MongoMapper. Provides obvious place to
add support for addition object mappers and to enhance additional support
needed on ActiveRecord and MongoMapper.

Browing through the MANY tickets regarding adding an object mapper abstraction
it seems the preferred method is to try to let ActiveModel handle most of the
abstraction and just extend the various object matters where ActiveModel is
not abstracting enough. This way most of the SimpleForm code does not care
what object mapper is being used.

Is seems to me there are two classes of methods that we need to extend the
object mappers with.

1. Methods that return simple data. A great example of this is the
   "readonly_attributes" method. SimpleForm uses this method that ActiveRecord
   provides but an object mapper like MongoMapper doesn't have the concept of
   read only attributes. So in this case we take the simple approach and simply
   mimic the ActiveRecord method by returning an array (just like ActiveRecord)
   which is empty (since the concept is not supported).
2. Methods that return complex data. A good example of this is
   "reflect_on_association". This returns an ActiveRecord-specific data
   structure. So in this case I made all data mappers (AR and MM) return a
   object mapper independent data structure (SimpleForm::Association). Because
   I am returning an SimpleForm specific data structure I have prefixed these
   methods with "simple_form_" so we reduce poluting the object mapper's
   namespace.

This second style method is basically what the testing is already doing. We
are just moving those object mapper independent data structures into the code
instead of having them only used in testing.

I have no doubt that this patch is missing stuff we will need down the road.
I am only using pieces of SimpleForm for my project so there may be areas where
we need more abstraction methods added to the object mappers. Obviously others
are also interested in other object mappers (DataMapper, Mongoid, etc). I feel
like that can come with time by additional patches. Right now we just need a
place to start. An obvious place to put these abstraction methods. Otherwise
we will continue to have dozens of issues where everybody want it but it cannot
get off the ground.

Regarding testing, I did add testing for the two object mapper independent
objects I added (Column and Association). But I was unsure the approach to take
regarding testing the abstraction methods that we add to the object mappers.
I was unsure if we wanted to required the tests to have all those database
types available. Or maybe have tests but skip ones not available. In general
the abstraction methods are simple. Just copying data from the object mapper
specific data structures to the object mapper independent data structures.

If you have suggestions on how to improve this patch I am open and ready to
follow suggestions. But I want to ensure we don't let perfection get in the
way of progress. For me it is ok if we have to refactor our approach as we add
more object mappers in the future. Sorry to be long winded. :)
a3615f6
@rafaelfranca

Thank you so muck. I really like this!

PS. I copied the commit message to the pull request description.

@rafaelfranca rafaelfranca commented on an outdated diff Jun 29, 2013
lib/simple_form/oms/mongo_mapper.rb
+ mm_assoc = associations[attribute]
+ return unless mm_assoc
+ SimpleForm::Association.new do |sf_assoc|
+ sf_assoc.klass = mm_assoc.klass
+ sf_assoc.name = mm_assoc.name
+ sf_assoc.macro = case mm_assoc.class.name
+ when /BelongsTo/ then :belongs_to
+ when /One/ then :has_one
+ when /Many/ then :has_many
+ end
+ sf_assoc.options = mm_assoc.options
+ end
+ end
+
+ # MongoMapper doesn't support readonly attributes
+ def readonly_attributes; [] end

To make this API explicit I wold define a simple_form_readonly_attributes and delegate it to readonly_attributes on the AR extention

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@rafaelfranca rafaelfranca commented on an outdated diff Jun 29, 2013
lib/simple_form/oms/active_record.rb
@@ -0,0 +1,30 @@
+module SimpleForm
+ module Oms

We need to document this module to tell which are the API that new object mappers need to implement. I think the better place in on the file above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@carlosantoniodasilva carlosantoniodasilva commented on an outdated diff Jun 29, 2013
lib/simple_form/column.rb
@@ -0,0 +1,24 @@
+# A Column object respresents the specs of an attribute as defined in

typo: represents.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@carlosantoniodasilva carlosantoniodasilva commented on an outdated diff Jun 29, 2013
lib/simple_form/column.rb
@@ -0,0 +1,24 @@
+# A Column object respresents the specs of an attribute as defined in
+# the database in an Object Mapper independent manor. A Object Mapper

manor doesn't seem to fit here? (or maybe I don't know the word enough 😄). Also, An Object Mapper.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
eric1234 added some commits Jun 29, 2013
@eric1234 eric1234 Fix typos b5e4971
@eric1234 eric1234 Small refactors based on feedback from Pull Request #835
* Put readonly_attributes under simple_form_ prefix. This will further reduce
  our polluting of the object mapper's namespace. This will also help ensure
  we don't conflict with future object mappers that maybe already implement
  readonly_attributes but in a different way than ActiveRecord.
* Updated the documentation on the object mapper modules so it is clear what
  someone needs to do to add support for a new object mapper.
a3eb222
@eric1234

I have pushed two more commits that incorporate the feedback. My abhorrent grammar is fixed, readonly_attributes is now under the simple_form_ prefix and additional documentation was provided. Let me know what else you need on this patch. Thanks for the feedback!

@thiagofm

Very cool, I think mongoid is of more importance than mongomapper as it's the right choice as mongo orm atm.

Soon I will have sometime to devote for this and I can help you with it. I like the direction of where this is going.

@eric1234

Yes, Mongoid does have more traction than MongoMapper so support for that would be great. I just don't use it (like the design and syntax of MongoMapper better) so didn't implement it. But I welcome others to implement other object mappers (DataMapper, Sequel, etc.). It's my hope that we don't wait for all the object mappers before getting this into a release. New ones can be added as people need them.

@rafaelfranca rafaelfranca added this to the 4.0 milestone Apr 3, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment