@@ -172,6 +172,7 @@ module FriendlyId raise ActiveRecord::RecordNotFound, "Couldn't find all #{ name.pluralize } with IDs (#{ ids_and_names * ', ' }) AND #{ sanitize_sql options[:conditions] } (found #{ results.size } results, but was looking for #{ expected_size })" if results.size != expected_size # assign finder slugs + # FIXME set the actual slugs, not the name to "lazy load" later, because that's not lazy! slugs.each do |slug| result = results.find { |r| r.id == slug.sluggable_id } and result.finder_slug_name = slug.name @@ -183,6 +184,8 @@ module FriendlyId module SluggableInstanceMethods + NUM_CHARS_RESERVED_FOR_FRIENDLY_ID_EXTENSION = 2 + attr :finder_slug attr_accessor :finder_slug_name @@ -192,7 +195,7 @@ module FriendlyId # Was the record found using one of its friendly ids? def found_using_friendly_id? - @finder_slug_name + finder_slug end # Was the record found using its numeric id? @@ -212,7 +215,7 @@ module FriendlyId # Returns the friendly id. def friendly_id - finder_slug_name or slug.name + slug(true).name end alias best_id friendly_id @@ -230,10 +233,15 @@ module FriendlyId # Generate the text for the friendly id, ensuring no duplication. def generate_friendly_id - slug_text = truncated_friendly_id_base - count = Slug.count_matches slug_text, self.class.name, :all, :conditions => "sluggable_id <> #{ id.to_i }" - count += 1 if self.class.friendly_id_options[:reserved].include?(slug_text) - count == 0 ? slug_text : generate_friendly_id_with_extension(slug_text, count) + base = friendly_id_base + opts = self.class.friendly_id_options + if base.length > opts[:max_length] + base = base[0...opts[:max_length] - NUM_CHARS_RESERVED_FOR_FRIENDLY_ID_EXTENSION] + end + if opts[:reserved].include?(base) + base = "#{base}-2" + end + Slug.get_best_name(base, self.class) end # Set the slug using the generated friendly id. @@ -241,12 +249,13 @@ module FriendlyId if self.class.friendly_id_options[:use_slug] @most_recent_slug = nil slug_text = generate_friendly_id - + # Avoids regenerating slug over and over again. + # FIXME This could perform pretty badly if a model has tons of similarly-named slugs + return if slug && slug.succ == slug_text if slugs.empty? || slugs.first.name != slug_text - previous_slug = slugs.find_by_name slug_text + previous_slug = slugs.find_by_name friendly_id_base previous_slug.destroy if previous_slug - - slugs.build :name => slug_text + slugs.build :name => generate_friendly_id end end end @@ -267,33 +276,13 @@ module FriendlyId private - NUM_CHARS_RESERVED_FOR_FRIENDLY_ID_EXTENSION = 2 - def init_finder_slug - raise RuntimeError, 'No slug name is set' if !@finder_slug_name + return false if !@finder_slug_name slug = Slug.find(:first, :conditions => {:sluggable_id => id, :name => @finder_slug_name}) slug.sluggable = self return slug end - def truncated_friendly_id_base - max_length = friendly_id_options[:max_length] - slug_text = friendly_id_base[0, max_length - NUM_CHARS_RESERVED_FOR_FRIENDLY_ID_EXTENSION] - end - - # Reserve a few spaces at the end of the slug for the counter extension. - # This is to avoid generating slugs longer than the maxlength when an - # extension is added. - POSSIBILITIES = 10 ** NUM_CHARS_RESERVED_FOR_FRIENDLY_ID_EXTENSION - 1 - def generate_friendly_id_with_extension(slug_text, count) - count <= POSSIBILITIES or - raise FriendlyId::SlugGenerationError.new("slug text #{slug_text} goes over limit for similarly named slugs") - - slug_text = "#{ truncated_friendly_id_base }-#{ count + 1 }" - - count = Slug.count_matches slug_text, self.class.name, :all, :conditions => "sluggable_id <> #{ id.to_i }" - count > 0 ? "#{ truncated_friendly_id_base }-#{ count + 1 }" : slug_text - end end -end +end \ No newline at end of file lib/friendly_id.rb @@ -5,50 +5,46 @@ class Slug < ActiveRecord::Base validates_uniqueness_of :name, :scope => :sluggable_type class << self + def with_name(name) "#{ quoted_table_name }.name = #{ quote_value name, columns_hash['name'] }" end + def with_names(names) name_column = columns_hash['name'] names = names.map { |n| "#{ quote_value n, name_column }" }.join ',' "#{ quoted_table_name }.name IN (#{ names })" end + def find_all_by_names_and_sluggable_type(names, type) names = with_names names type = "#{ quoted_table_name }.sluggable_type = #{ quote_value type, columns_hash['sluggable_type'] }" find :all, :conditions => "#{ names } AND #{ type }" end - # Count exact matches for a slug. Matches include slugs with the same name - # and an appended numeric suffix, i.e., "an-example-slug" and - # "an-example-slug-2" - # - # The first two arguments are required, after which you may pass in the - # same arguments as ActiveRecord::Base.find. - COND = 'name LIKE ? AND sluggable_type = ?'.freeze - def count_matches(name, type, *args) - name_esc = Regexp.escape name - - with_scope(:find => {:conditions => [COND, "#{name}%", type]}) { - find(*args) - }.inject(0) do |count, slug| - slug.name =~ /\A#{name_esc}(-[\d]+)*\Z/ ? count + 1 : count - end + # Checks a slug name for collisions + def get_best_name(name, type) + slugs = find :all, :conditions => ['name LIKE ? AND sluggable_type = ?', "#{name}%", type.to_s] + return name if slugs.size == 0 + slugs.map { |x| slugs.delete(x) unless x.base == name } + slugs.sort! { |x, y| x.extension <=> y.extension } + slugs.empty? ? name : slugs.last.succ end # Sanitizes and dasherizes string to make it safe for URL's. # # Example: + # # slug.normalize('This... is an example!') # => "this-is-an-example" # - # Note that Rails 2.2.x offers a parameterize method for stripping - # diacritics. This is not used here because at the time of writing, it - # handles several characters incorrectly, for instance replacing - # Icelandic's "thorn" character with "y" rather than "d." This might be - # pedantic, but I don't want to piss off the Vikings. The last time anyone - # pissed them off, they uleashed a wave of terror in Europe unlike - # anything ever seen before or after. I'm not taking any chances. + # Note that Rails 2.2.x offers a parameterize method for this. It's not + # used here because at the time of writing, it handles several characters + # incorrectly, for instance replacing Icelandic's "thorn" character with + # "y" rather than "d." This might be pedantic, but I don't want to piss + # off the Vikings. The last time anyone pissed them off, they uleashed a + # wave of terror in Europe unlike anything ever seen before or after. I'm + # not taking any chances. def normalize(slug_text) # Use this onces it starts working reliably # return slug_text.parameterize.to_s if slug_text.respond_to?(:parameterize) @@ -68,7 +64,19 @@ class Slug < ActiveRecord::Base end end - + + def succ + extension == 0 ? "#{base}-2" : name.succ + end + + def base + name.gsub(/-?\d*\z/, '') + end + + def extension + /\d*\z/.match(name).to_s.to_i + end + # Whether or not this slug is the most recent of its owner's slugs. def is_most_recent? debugger lib/slug.rb @@ -1,3 +1,3 @@ class Post < ActiveRecord::Base - has_friendly_id :name, :use_slug => true, :reserved => ['new', 'recent'] + has_friendly_id :name, :use_slug => true, :reserved => ['new', 'edit'] end test/fixtures/post.rb @@ -7,7 +7,7 @@ class SluggableTest < Test::Unit::TestCase def setup Post.friendly_id_options[:max_length] = FriendlyId::ClassMethods::DEFAULT_FRIENDLY_ID_OPTIONS[:max_length] end - + def test_finder_options_are_not_ignored assert_raises ActiveRecord::RecordNotFound do Post.find(slugs(:one).name, :conditions => "1 = 2") @@ -180,5 +180,26 @@ class SluggableTest < Test::Unit::TestCase post = Post.create!(:name => 'new') assert_equal 'new-2', post.friendly_id end + + def test_slug_sequence_is_based_on_highest_extension_rather_than_slug_count + assert_equal "test", Slug.get_best_name("test", Post) + @post = Post.create!(:name => "test", :content => "stuff") + assert_equal "test", @post.slug.name + + assert_equal "test-2", Slug.get_best_name("test", Post) + @post2 = Post.create!(:name => "test", :content => "stuff") # slug should be "test-2" + assert_equal "test-2", @post2.slug.name + + assert_equal "test-3", Slug.get_best_name("test", Post) + @post3 = Post.create!(:name => "test", :content => "stuff") + assert_equal "test-3", @post3.slug.name + + assert_equal "test-4", Slug.get_best_name("test", Post) + @post.destroy # will destroy slug named "test" along with it + # Make sure the next slug is still test-4 and not test-3 + assert_equal "test-4", Slug.get_best_name("test", Post) + @post4 = Post.create!(:name => "test", :content => "stuff") + assert_equal "test-4", @post4.slug.name + end end \ No newline at end of file test/sluggable_test.rb 61ab15579986f9240f3d74a6f8affcaa13ebfa77 Norman Clarke norman@randomba.org http://github.com/norman/friendly_id/commit/f2fac151e73c1649fe5c619a6506ab4aa2e95cbd f2fac151e73c1649fe5c619a6506ab4aa2e95cbd 2008-10-31T12:40:04-07:00 2008-10-31T12:38:23-07:00 Fixed logic error with slug generation. Resolves ticket #7. Thanks to Tim Kadom for reporting the bug that this commit fixes. 858beecf4218e0bdad544b30210769ae189380d0 Norman Clarke norman@randomba.org