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

Matt Wagner matt.wagner at redhat.com
Mon Feb 20 22:23:14 UTC 2012


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/



More information about the aeolus-devel mailing list