[PATCH] Fixes BZ 770618, enforcing unique pool names.

Kenneth Keiter kenkeiter at redhat.com
Mon Feb 20 22:27:21 UTC 2012


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 at redhat.com wrote:
>> From: Kenneth Keiter <ken at 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_uniqueness_of-slowness/

--
Kenneth Keiter
Sr. Software Engineer, Cloud BU
kenkeiter at redhat.com
http://twitter.com/kenkeiter
IRC: tigger or tiggerer






More information about the aeolus-devel mailing list