Rails Ruby on Rails → 2.x

Comments and changes to this ticket

• You flagged this item as spam.
  

  acechase June 19th, 2008 @ 02:22 AM

  Hallelujah, how have we gone this long without model namespacing?!?!

  The big problem I see with your current approach is that it only works in one direction, from class name to table name.

  "Animal::Lion".tableize # --> animal_lions

  "animal_lions".classify # --> AnimalLion

  Until both directions work, I don't think module namespacing will be all that useful. It seems like we need some type of indicator to allow for translation in the other direction. Like you, I considered using a double underscore, but because that would require changes to Inflector#classify I opted instead for a double colon in my actual table name. Colon is a legal character in MySQL but I'm not sure about other dbs.

• You flagged this item as spam.
  

  Harri Kauhanen August 27th, 2008 @ 10:31 AM

  • Tag set to “activerecord, enhancement, fixtures, generators, patch, railties, routing”

  Some blog post related to this issue:

  http://www.strictlyuntyped.com/2...

  (mine) http://blog.humaneq.com/2008/6/2...

• 

  Pratik August 30th, 2008 @ 06:08 PM

  • Assigned user set to “Michael Koziarski”
• 

  Michael Koziarski August 30th, 2008 @ 07:16 PM

  I'd like to focus on these as two seperate issues, and only talk about the AR table naming stuff to start with.

  The reason we had differing behaviour for nested classes and modules was for backwards compatibility, so the table name doesn't change when you upgrade rails. However that doesn't need to hold us back from making the change.

  I can't follow your rationale of nesting in the table names:

  
  my_application_business_company_contacts
  MyApplication::Business::Client::Contact.table_name
  

  That seems far too verbose ;) Why not:

  
  my_application_businesses_companies_contacts
  

  or

  
  my_application_business_company_contacts
  

• 

  Michael Koziarski August 30th, 2008 @ 07:17 PM

  Obviously I messed up the cut-n-paste. But the verbose version is:

  
  my_application_business_company_my_application_business_client_contacts
  

• 

  Jeremy Kemper August 30th, 2008 @ 08:42 PM

  • Milestone set to “2.x”
  • State changed from “new” to “open”

• You flagged this item as spam.
  

  Harri Kauhanen September 2nd, 2008 @ 08:58 PM

  my_application_business_company_my_application_business_client_contacts is too verbose, indeed. I did not like this either, but did not fix it. This verbose naming seems to occur (when using inner nested classes!) because I just changed the method undecorated_table_name in activerecord/lib/active_record/base.rb, and perhaps for inner classes the table name is "joined" by calling undecorated_table_name multiple times.

  By inner class nesting I mean how MyApplication::Business::Client::Contact is actually defined in rails/activerecord/test/models/company_in_module.rb

  
  module MyApplication
    module Business
      class Company < ActiveRecord::Base
        attr_protected :rating
      end
  
      # ...
  
      class Client < Company
        belongs_to :firm, :foreign_key => "client_of"
        belongs_to :firm_with_other_name, :class_name => "Firm", :foreign_key => "client_of"
  
        class Contact < ActiveRecord::Base; end
      end
  
      # ...
    end
  end
  

  In my app, I did not have such complex nesting, just additional namespaces, therefore the table naming convention was nice and not too verbose.

• 

  Michael Koziarski September 3rd, 2008 @ 09:00 AM

  But what do you think the 'correct' table name should be?

  
  Account::Payment::Invoice
  

  Seems to me the choices are:

  
  account_payment_invoices or
  accounts_payments_invoices
  

• You flagged this item as spam.
  

  Harri Kauhanen September 3rd, 2008 @ 12:44 PM

  I see no reasons why namespaces should always be pluralized. Therefore I would go with the convention account_payment_invoices. If, however, namespace is plural (e.g. module Animals), then it should be kept pluralized. For instance: animals_cats.

• 

  Michael Koziarski September 3rd, 2008 @ 02:14 PM

  OK, We're about to push out 2.2 betas, so we won't be able to get this in before then, but we'll look at it after that.

  In the meantime maybe you should talk with a few people who use or want heavily nested models, and see what they think the table names should be.

  acechase, do you have any thoughts?

• You flagged this item as spam.
  

  acechase September 3rd, 2008 @ 06:50 PM

  I agree with Harri, I can't think of any good reason to add pluralization to the module names (especially since it would confuse things if you were starting with a module name that was already plural, like 'Animals').

  The one sticking point I would like to come back to is the inability to work backwards from table name to class name without some distinct differentiator to make clear the boundary between one module/class name and the next. It seems that the easiest way to get around this is to use a double underscore to separate distinct names from one another and to update Inflector.classify to support this.

  It seems that the one argument against double underscore is they lead to ugly table names. While not having double underscores (or some other differentiator) in the table name means there's absolutely no way to translate from table name to class name, thus taking away a very useful metaprogramming technique. In my mind, the answer is quite simple. Aesthetics are great, but when given the choice, I'll opt for correct functionality every time ;-)

• 

  Jeremy Kemper September 3rd, 2008 @ 11:05 PM

  Agreed, acechase. As long as we introduce a convention for this, it should actually work both ways.

  How about a different delimiter character entirely?

• You flagged this item as spam.
  

  acechase September 4th, 2008 @ 01:27 AM

  Jeremy, do you have any other delimiters in mind? I had previously suggested we use '::' -- I know that's legal in MySQL, but I don't know how that flies with other dbs. Also, it may be nice to use a delimiter that doesn't force the table name to be quoted (not that that's an issue with AR). In MySQL I think that narrows it down to either underscore or question mark. Trying to be db agnostic, I took a look around the SLQ92 spec and reading that it looked like latin characters and underscore are the characters the spec specifically defines as legal, but my BNF reading skills are at about a first grade level so I may have misunderstood.

  Personally, I'm somewhat indifferent, so long as we can agree on some identifier and stick with it.

• 

  Michael Koziarski September 4th, 2008 @ 12:57 PM

  I think underscores are pretty much the only option. That won't allow for perfect roundtripping from class -> table, but I don't think that's going to be a real problem in actual applications. I don't really think it's likely that someone will have

  
  AnimalFeed
  

  and

  
  Animal::Feed
  

  In the same app.

• You flagged this item as spam.
  

  Harri Kauhanen September 4th, 2008 @ 03:41 PM

  Don't we have a problem already to resolve the class name from table name? I mean...

  
  # table name animals_cats could be...
  class AnimalsCat < ActiveRecord::Base
    # ...
  end
  
  # or table animals_cats could as well be...
  class Animal < ActiveRecord::Base
    class Cat <  ActiveRecord::Base
      # ...
    end
  end
  

  Using colons '::' might not be working solution. MySQL table names must be allowed filenames, and colon is not allowed in Windows filesystems (I did not test it).

  http://dev.mysql.com/doc/refman/... http://en.wikipedia.org/wiki/Fil...

  Using colons '::' or slash '/' would be the prettiest solution, but I'm also afraid we are stuck using underscores. I don't have very strong opinions should the namespace separator be '_' or '__'. I ended up using a single underscore mainly because I could not get double underscore (easily) working (I just don't understand the Rails internals enough). Single underscore looks better, but double underscore perhaps tells you more.

• 

  Jeremy Kemper September 4th, 2008 @ 04:06 PM

  Harri, the second case is animal_cats.

  I guess I'd go for double underscore, then -- it seems worth being able to predictably do table_name.classify.constantize (fixtures, for example)

• 

  Michael Koziarski September 4th, 2008 @ 04:07 PM

  Yeah, the roundtripping is already broken, but acechase mentioned it as an important feature.

  I'm personally not worried about it, so perhaps it's a non-issue.

  I like the idea of having modules not be pluralized but keeping the nested classes behaving like they do at present.

  After 2.2 we'll pick this discussion up again, but in the meantime you can probably get the patch split up into two chunks. AR fixes, and the rest. Then get the AR fixes working correctly

• You flagged this item as spam.
  

  CJV September 4th, 2008 @ 08:50 PM

  Michael says "Yeah, the roundtripping is already broken, but acechase mentioned it as an important feature. I'm personally not worried about it, so perhaps it's a non-issue."

  This is a little like saying "the brakes on all Ford vehicles are defective, but I don't drive a Ford so perhaps it's a non-issue". Simply because you or some subset of the population aren't affected doesn't mean it doesn't affect the rest of us, with potentially hazardous results.

  Yes, I'm being a bit sarcastic, but the point is that if something is broken, don't use that as justification to break it further, without a really good reason.

  To be fair, you're not always going to make everyone happy, and some things will change over time and break old apps. But these types of changes should not be done lightly, or without proper advertising of that fact. If the current system has shortcomings or known use cases where it does not work as expected, then this should be documented as well.

• You flagged this item as spam.
  

  acechase September 4th, 2008 @ 10:14 PM

  I went ahead and implemented the double underscore differentiator in Inflector to see what kind of ramifications it would have. By changing the Inflector#underscore method to gsub '::' with '' (instead of '/') and changing Inflector#camelize to gsub '' rather than '/' we get the desired roundtrip behavior:

  
  >> module TestModule ; class Test ; class NestedTest ; end ; end ; end
  >> TestModule::Test::NestedTest.name.tableize
  => "test_module__test__nested_tests"
  >> TestModule::Test::NestedTest.name.tableize.classify
  => "TestModule::Test::NestedTest"
  

  Of course, changing the #camelize and #underscore methods will break code relying on the old behavior. IMO, camelize and underscore should just be camelizing and underscoring, the fact they currently deal with slashes as well is a bad idea since that is beyond the scope of those methods responsibilities. Ideally, that logic that adds in slashes would live in a #pathify method. But for the purposes of the here and now, we can keep all of the necessary changes consolidated to just the #classify and #tableize methods, like so:

  
  class Inflector
  ...
  def tableize(class_name)
    pluralize(underscore(class_name)).gsub(/\//, '__')
  end
  def classify(table_name)
    camelize(singularize(table_name.to_s.sub(/.*\./, '')).gsub(/__/, '/'))
  end
  

  Sorry for belaboring this point... I hope I'm not frustrating anyone too much with my excessive anal-ness ;)

• You flagged this item as spam.
  

  Harri Kauhanen September 4th, 2008 @ 10:26 PM

  I understand the concern of CJV. If this change is made so that the table name convention is configurable, and the initial configuration keeps the naming "the old way", there should not be a big hassle.

  I still believe this "new convention" should be default in some point. It could, for instance, ease the existing problems of many plugins and namespace conflicts.

• You flagged this item as spam.
  

  matthuhiggins September 8th, 2008 @ 05:33 AM

  I don't see reason for using double underscores to disambiguate modules from class names. There is no need to go from table name -> class.

  Currently, Inflector.tableize does not consider the 'ActiveRecord::Base.pluralize_table_names' option, so ActiveRecord cannot even use it. Trying to modify or fix tableize might be out of the scope of this patch. (On a grander scale, I think adding tableize to Inflectors was silly in the first place)

  ActiveRecord::Base.demodulize_table_names seems most consistent naming with the existing 'pluralize_table_names' option.

• You flagged this item as spam.
  

  Harri Kauhanen September 10th, 2008 @ 03:51 PM

  Ok, I did some rework with the patch and sliced it into pieces.

  The first patch just updates the active_record so that table names for namespaced models are (can be) in format namespace1module2model_objects. Namespaces (modules) are used in database table name, and separated with two underscore characters.

  This is configurable using parameter ActiveRecord::Base.namespace_modules_in_table_names. For initial backward compatibility, this is by default set to false.

  Note! class_name method is broken with nested classes, and probably impossible to fix. However, it should now work fine with the namespaced model and new tablename convention.

  Note! I did not touch the Inflector class. The active_record code was not using it either, and there was quite strong statement written by Rails core team in the file header of inflector.rb not to patch it.

  • rails_namespaced_models_activerecord.diff 4 KB

• You flagged this item as spam.
  

  Harri Kauhanen September 10th, 2008 @ 03:52 PM

  The next patch has nothing to do with the database naming convention. It should, however, fix some issues with namespaced models, and relations such as "has_many" and "belongs_to". Without this patch, your namespaced models might seem to work fine at first, but you will run into errors in certain cases. For instance:

  
  class Animal::Dog < ActiveRecord::Base
    has_many :bugs
  end
  
  class Animal::Bug < ActiveRecord::Base
    belongs_to :dog
  end
  

  Now you may e.g. execute Animal::Dog.first.bugs and it works. If you reload the console and call Animal::Bug.first.dog will also work. If, however, you execute these sequentially WITHOUT reloading the console, you will get an error such as "Bug is not missing constant Dog!"

  I have no idea, whether my fix is 'correct', and why it works. However, it fixed the problem and seemed not break Rails tests.

  • rails_namespaced_models_fix_load_missing_constant.diff 1.6 KB

• You flagged this item as spam.
  

  Harri Kauhanen September 10th, 2008 @ 03:53 PM

  This patch just ads a simple test case for the improved naming convention. I admit, we should perhaps more tests (e.g. testing the "has_many" and other relations with namespaced models).

  • rails_namespaced_models_simple_testcase.diff 2.1 KB

• You flagged this item as spam.
  

  Harri Kauhanen September 10th, 2008 @ 03:53 PM

  I believe this new convention should be default in some point of time. I found this "new_rails_defaults" initializer, and thought it probably should be here. If I was right, here's the patch.

  • rails_namespaced_models_new_rails_defaults.diff 991 Bytes

• You flagged this item as spam.
  

  Harri Kauhanen September 10th, 2008 @ 03:54 PM

  This patch makes the fixtures for namespaced models work. If you have e.g. class Animal::Monkey, the fixture file should be either animal__monkey.yml or monkey.yml UNDER THE FOLDER called animal. The first option resembles the database names used and second keeps you more organized (probably the better choice).

  I had to write couple of additional methods to magic happen. Hopefully someone has time to go through the code. All the active_record tests seemed to work fine with mysql.

  I also fixed the rake task db:fixtures:load to support namespaced models.

  • rails_namespaced_models_fixtures_fixed.diff 7.3 KB

• You flagged this item as spam.
  

  Harri Kauhanen September 10th, 2008 @ 03:55 PM

  This is a rather bigger patch. It makes Rails generator generate working code with namespaced model. For instance, if you run script/generate scaffold Animal::Monkey::Ape name:string age:integer, you should get

  • migration file that uses new naming convention (tablename --> animalmonkeyapes)
  • fixtures: animal/monkey/apes.yml
  • working functional (and unit) tests
  • namespaced routes into route.rb
  • working web interface you can access at: http://localhost:3000/animal/mon...

  Note! This patch is not checking whether ActiveRecord::Base.namespace_modules_in_table_names is turned on or off

  Again, there are some code changes that might be questionable... or not :) Just check out the comments in the code, before committing.

  How model associations work with namespaced models? See the code below:

  
  class Animal::Bug < ActiveRecord::Base
    # This one uses Animal::Dog
    belongs_to :dog
  end
  
  class Animal::Dog < ActiveRecord::Base
    # This one uses Animal::Bug
    has_many :bugs
  
    # If not found in the same module, will look it from "root" namespace automatically
    belongs_to :person
  end
  
  class Person < ActiveRecord::Base
    # Dog class not found if class name (wtih namespace) not defined
    has_many :dogs, :class_name => "Animal::Dog"
  end
  

  • rails_namespaced_models_generator_stuff.diff 19 KB

• You flagged this item as spam.
  

  Harri Kauhanen September 10th, 2008 @ 03:55 PM

  This patch contains all the previous patches (except the improve_model_namespace_support.diff which is now obsolete).

  While doing this patch, I noticed that Rails is not very DRY about finding out the correct name for database table/class/etc. Perhaps there should be a centralized piece of logic handling all this stuff (just my 5 cents here). It was quite fun to have a peek into Rails codebase. Hopefully, improved namespace support will end up there as well :-)

  • rails_namespaced_models_ALL_PATCHES.diff 35 KB

• You flagged this item as spam.
  

  Harri Kauhanen September 10th, 2008 @ 04:05 PM

  Uups.. wiki formatting ate my double underscores. animalmonkeyapes should be animal(double-underscore)monkey(double-underscore)apes .

• 

  Pratik December 20th, 2008 @ 07:31 PM

  Patch doesn't apply clean anymore.

• 

  Pratik December 20th, 2008 @ 07:55 PM

  • State changed from “open” to “incomplete”

• You flagged this item as spam.
  

  Harri Kauhanen December 21st, 2008 @ 09:41 AM

  Ok.

  My personal project no longer needs this. Therefore (and unfortunatelly), I do not have interest enough to go through the fixing process again.

  I'm really happy to see Rails is maturing and have gone such a great improvements lately directly benefitting my work (e.g. thread safety). Namespaced models would be quite natural improvement -- perhaps not a major feature for the most of us, but something that should IMHO work out of the box. Perhaps one day they do ;)

• You flagged this item as spam.
  

  wildchild January 13th, 2010 @ 11:25 AM

  Would be awesome to have such feature in rails 3 out of the box.

• 

  José Valim February 21st, 2010 @ 07:37 PM

  • State changed from “incomplete” to “stale”

  I'm marking this as stale. This would be a great feature, but we need someone to actually work on it. If someone has a patch, I'm glad to review and apply.

• 

  Andrew White February 22nd, 2010 @ 03:17 AM

  José, I still think having some degree of control over the table namespace would be a good idea, however I think blindly adding in the parent module names isn't the way to go. The main need for something like this is to avoid collisions with the host application when writing engines/plugins and a simple prefix is probably all that is needed.

  Therefore what I'm proposing is that we enhance the current table_name_prefix which is global to first check whether a parent module responds to table_name_prefix and then use that instead of the global setting. This has the benefit of not breaking backward compatibility and is a lot simpler than the proposed patch in this ticket. I could knock together a patch within 24/48 hours if the idea is acceptable.

• 

  José Valim February 22nd, 2010 @ 06:51 AM

  Sure, put up a patch and feel free to create a new ticket and assign it to me.

• 

  Andrew White February 22nd, 2010 @ 11:41 AM

  Okay - patch and new ticket here