Interesting.. I wonder why it didn't fail testing for me?
Are we (canonically?) supporting unique names as case-sensitive, or insensitive? It might just be best to leave this issue alone and allow the user to deal with it, rather than creating too much trouble for ourselves. Thanks for testing it.
On Feb 20, 2012, at 2:23 PM, Matt Wagner wrote:
On Fri, Feb 17, 2012 at 09:00:15AM -0800, kkeiter@redhat.com wrote:
From: Kenneth Keiter ken@kenkeiter.com
src/app/models/pool.rb | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/app/models/pool.rb b/src/app/models/pool.rb index 8529a9d..5d79cf3 100644 --- a/src/app/models/pool.rb +++ b/src/app/models/pool.rb @@ -51,7 +51,7 @@ class Pool < ActiveRecord::Base validates_presence_of :quota validates_presence_of :pool_family validates_inclusion_of :enabled, :in => [true, false]
- validates_uniqueness_of :name
- validates_uniqueness_of :name, :case_sensitive => false validates_uniqueness_of :exported_as, :if => :exported_as validates_length_of :name, :maximum => 255
-- 1.7.4.1
This causes a test failure: cucumber features/catalogs.feature:10 # Scenario: Create a new catalog
It's actually a problem with pool_steps.rb, where we look up a pool named 'default' and create it if it's not failed. find_by_name is case-sensitive (on Postgres), so we find no pool with that name and thus create one -- but then the validation fails because "Default" exists.
I think the right thing to do is to fix the test to just use 'Default', as opposed to doing a case-insensitive lookup.
The problem I realized with case-insensitivity, by the way, is that the LOWER() operation ruins our ability to use an index -- so calling it forces a table scan[1]. That's (arguably) not a big deal when creating a new pool, which should happen relatively infrequently and which shouldn't be searching through tens of thousands of rows... But I'd hate to try to hack the find to be case-insensitive. So changing the test seems to be the least-bad option.
I'm going to change the test (and search for any other happenings of this) and send that as a follow-up.
-- Matt
[1] http://techblog.floorplanner.com/2009/11/17/case-insensitive-validates_uniqu...
-- Kenneth Keiter Sr. Software Engineer, Cloud BU kenkeiter@redhat.com http://twitter.com/kenkeiter IRC: tigger or tiggerer