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(a)redhat.com wrote:
> From: Kenneth Keiter <ken(a)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_un...
--
Kenneth Keiter
Sr. Software Engineer, Cloud BU
kenkeiter(a)redhat.com
http://twitter.com/kenkeiter
IRC: tigger or tiggerer