New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Respect ActiveRecord's table_name_prefix option on schema_migrations #214

Merged
merged 1 commit into from May 24, 2013

Conversation

Projects
None yet
2 participants
@kstevens715
Contributor

kstevens715 commented May 24, 2013

This makes ActiveRecord responsible for determining the table name to
handle subtle nuisances like table_name_prefix and table_name_suffix. See issue #213.

Get schema_migrations table name from ActiveRecord
This makes ActiveRecord responsible for determining the table name to
handle subtle nuisances like table_name_prefix and table_name_suffix.
@bmabey

This comment has been minimized.

Show comment
Hide comment
@bmabey

bmabey May 24, 2013

Contributor

One potential issue I see with this approach is uses the global AR default connection. DatabaseCleaner supports multiple AR connections so it would make sense to get the prefix settings on a connection basis. Given an AR connection object can you call #schema_migrations_table_name or similar on it? Or do yo have any other ideas on how to accommodate the multiple connection use case? Or does AR only allow you to have one schema_migrations_table_name?

Contributor

bmabey commented May 24, 2013

One potential issue I see with this approach is uses the global AR default connection. DatabaseCleaner supports multiple AR connections so it would make sense to get the prefix settings on a connection basis. Given an AR connection object can you call #schema_migrations_table_name or similar on it? Or do yo have any other ideas on how to accommodate the multiple connection use case? Or does AR only allow you to have one schema_migrations_table_name?

@kstevens715

This comment has been minimized.

Show comment
Hide comment
@kstevens715

kstevens715 May 24, 2013

Contributor

Those are good questions, that I don't know the answers to right now. I was looking for something to do this Friday evening and it seems that something found me ;) I'll look into this further and get back to you. Thanks for your quick responses.

Contributor

kstevens715 commented May 24, 2013

Those are good questions, that I don't know the answers to right now. I was looking for something to do this Friday evening and it seems that something found me ;) I'll look into this further and get back to you. Thanks for your quick responses.

@kstevens715

This comment has been minimized.

Show comment
Hide comment
@kstevens715

kstevens715 May 24, 2013

Contributor

So, I did a little bit of research and it looks like Rails allows different table_name_prefix and table_name_suffix options if you separate your models into different modules. But without some hacks, Rails is always going to perform schema migrations against the base connection (ActiveRecord::Base.connection), and it will always use the base table_name_prefix and suffix for building the schema_migration table name.

Ultimately, ActiveRecord::Migrator.schema_migrations_table_name calls the following method to generate the schema_migrations name. It's only ever going to get the prefix and suffix from ActiveRecord::Base which is what I'm using in the unit test.

def self.table_name
  "#{Base.table_name_prefix}schema_migrations#{Base.table_name_suffix}"
end

So from everything I can determine, the changes in this PR should work universally for ActiveRecord unless someone has monkey patched the heck out of AR.

Some helpful links:
http://apidock.com/rails/ActiveRecord/Base/table_name_prefix/class
rails/rails#3497

Contributor

kstevens715 commented May 24, 2013

So, I did a little bit of research and it looks like Rails allows different table_name_prefix and table_name_suffix options if you separate your models into different modules. But without some hacks, Rails is always going to perform schema migrations against the base connection (ActiveRecord::Base.connection), and it will always use the base table_name_prefix and suffix for building the schema_migration table name.

Ultimately, ActiveRecord::Migrator.schema_migrations_table_name calls the following method to generate the schema_migrations name. It's only ever going to get the prefix and suffix from ActiveRecord::Base which is what I'm using in the unit test.

def self.table_name
  "#{Base.table_name_prefix}schema_migrations#{Base.table_name_suffix}"
end

So from everything I can determine, the changes in this PR should work universally for ActiveRecord unless someone has monkey patched the heck out of AR.

Some helpful links:
http://apidock.com/rails/ActiveRecord/Base/table_name_prefix/class
rails/rails#3497

@bmabey

This comment has been minimized.

Show comment
Hide comment
@bmabey

bmabey May 24, 2013

Contributor

Thanks for doing the extra research. I've also verified that the API remained consistent with the move to AR 4.0. This seems like a better default so I'm merging your PR in now. Thanks!

Contributor

bmabey commented May 24, 2013

Thanks for doing the extra research. I've also verified that the API remained consistent with the move to AR 4.0. This seems like a better default so I'm merging your PR in now. Thanks!

bmabey added a commit that referenced this pull request May 24, 2013

Merge pull request #214 from kstevens715/master
Respect ActiveRecord's table_name_prefix option on schema_migrations

@bmabey bmabey merged commit 7effdb7 into DatabaseCleaner:master May 24, 2013

1 check passed

default The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment