diff --git a/lib/stringex/acts_as_url.rb b/lib/stringex/acts_as_url.rb index b61fc7b4..bb13d981 100644 --- a/lib/stringex/acts_as_url.rb +++ b/lib/stringex/acts_as_url.rb @@ -30,7 +30,7 @@ module ActsAsUrlClassMethods # :doc: # unless you are using multiple ORMs in a single project. # :allow_slash:: If true, allow the generated url to contain slashes. Default is false[y]. # :allow_duplicates:: If true, allow duplicate urls instead of appending numbers to - # differentiate between urls. Default is false[y]. + # differentiate between urls. Default is false[y]. See note on :scope. # :duplicate_count_separator:: String to use when forcing unique urls from non-unique strings. # Default is "-". # :exclude_list:: List of complete strings that should not be transformed by acts_as_url. @@ -38,6 +38,8 @@ module ActsAsUrlClassMethods # :doc: # :only_when_blank:: If true, the url generation will only happen when :url_attribute is # blank. Default is false[y] (meaning url generation will happen always). # :scope:: The name of model attribute to scope unique urls to. There is no default here. + # Note: this will automatically act as if :allow_duplicates + # is set to true. # :sync_url:: If set to true, the url field will be updated when changes are made to the # attribute it is based on. Default is false[y]. # :url_attribute:: The name of the attribute to use for storing the generated url string. diff --git a/lib/stringex/acts_as_url/adapter/active_record.rb b/lib/stringex/acts_as_url/adapter/active_record.rb index 05b20599..818c0067 100644 --- a/lib/stringex/acts_as_url/adapter/active_record.rb +++ b/lib/stringex/acts_as_url/adapter/active_record.rb @@ -26,6 +26,14 @@ def add_scoped_url_owner_conditions @url_owner_conditions << instance.send(settings.scope_for_url) end + def create_callback + if settings.sync_url + klass.before_validation :ensure_unique_url + else + klass.before_validation :ensure_unique_url, :on => :create + end + end + # NOTE: The instance here is not the cached instance but a block variable # passed from klass_previous_instances, just to be clear def ensure_unique_url_for!(instance) diff --git a/lib/stringex/acts_as_url/adapter/base.rb b/lib/stringex/acts_as_url/adapter/base.rb index f1f57e70..1a95a4d6 100644 --- a/lib/stringex/acts_as_url/adapter/base.rb +++ b/lib/stringex/acts_as_url/adapter/base.rb @@ -11,21 +11,9 @@ def initialize(configuration) end def create_callbacks!(klass) - if settings.sync_url - klass.before_validation :ensure_unique_url - else - if defined?(ActiveModel::Callbacks) - klass.before_validation :ensure_unique_url, :on => :create - else - klass.before_validation_on_create :ensure_unique_url - end - end - - klass.class_eval <<-"END" - def #{settings.url_attribute} - acts_as_url_configuration.adapter.url_attribute self - end - END + self.klass = klass + create_method_to_callback + create_callback end def ensure_unique_url!(instance) @@ -44,6 +32,7 @@ def initialize_urls!(klass) end def url_attribute(instance) + # Retrieve from database record if there are errors on attribute_to_urlify if !instance.new_record? && instance.errors[settings.attribute_to_urlify].present? instance.class.find(instance.id).send settings.url_attribute else @@ -62,6 +51,14 @@ def self.loadable? private + def create_method_to_callback + klass.class_eval <<-"END" + def #{settings.url_attribute} + acts_as_url_configuration.adapter.url_attribute self + end + END + end + def duplicate_for_base_url(n) "#{base_url}#{settings.duplicate_count_separator}#{n}" end diff --git a/lib/stringex/acts_as_url/adapter/mongoid.rb b/lib/stringex/acts_as_url/adapter/mongoid.rb index 214918c8..4a6e911b 100644 --- a/lib/stringex/acts_as_url/adapter/mongoid.rb +++ b/lib/stringex/acts_as_url/adapter/mongoid.rb @@ -24,6 +24,14 @@ def add_scoped_url_owner_conditions @url_owner_conditions.merge! settings.scope_for_url => instance.send(settings.scope_for_url) end + def create_callback + if settings.sync_url + klass.before_validation :ensure_unique_url + else + klass.before_validation :ensure_unique_url, :on => :create + end + end + # NOTE: The instance here is not the cached instance but a block variable # passed from klass_previous_instances, just to be clear def ensure_unique_url_for!(instance) diff --git a/test/acts_as_url/adapter/active_record.rb b/test/acts_as_url/adapter/active_record.rb index 4dc6add7..d5471d34 100644 --- a/test/acts_as_url/adapter/active_record.rb +++ b/test/acts_as_url/adapter/active_record.rb @@ -14,48 +14,12 @@ ActiveRecord::Migration.verbose = false ActiveRecord::Schema.define do create_table :documents, :force => true do |t| - t.string :title, :url, :other + t.string :title, :other, :url end - create_table :updatuments, :force => true do |t| - t.string :title, :url, :other - end - - create_table :mocuments, :force => true do |t| - t.string :title, :url, :other - end - - create_table :permuments, :force => true do |t| - t.string :title, :permalink - end - - create_table :procuments, :force => true do |t| - t.string :title, :url - end - - create_table :blankuments, :force => true do |t| - t.string :title, :url - end - - create_table :duplicatuments, :force => true do |t| - t.string :title, :url - end - - create_table :validatuments, :force => true do |t| - t.string :title, :url - end - - create_table :ununiquments, :force => true do |t| - t.string :title, :url - end - - create_table :limituments, :force => true do |t| - t.string :title, :url - end - - create_table :skipuments, :force => true do |t| - t.string :title, :url - end + # create_table :permuments, :force => true do |t| + # t.string :title, :permalink + # end end ActiveRecord::Migration.verbose = true @@ -63,50 +27,50 @@ class Document < ActiveRecord::Base acts_as_url :title end -class Updatument < ActiveRecord::Base - acts_as_url :title, :sync_url => true -end +# class Updatument < ActiveRecord::Base +# acts_as_url :title, :sync_url => true +# end -class Mocument < ActiveRecord::Base - acts_as_url :title, :scope => :other, :sync_url => true -end +# class Mocument < ActiveRecord::Base +# acts_as_url :title, :scope => :other, :sync_url => true +# end -class Permument < ActiveRecord::Base - acts_as_url :title, :url_attribute => :permalink -end +# class Permument < ActiveRecord::Base +# acts_as_url :title, :url_attribute => :permalink +# end -class Procument < ActiveRecord::Base - acts_as_url :non_attribute_method +# class Procument < ActiveRecord::Base +# acts_as_url :non_attribute_method - def non_attribute_method - "#{title} got massaged" - end -end +# def non_attribute_method +# "#{title} got massaged" +# end +# end -class Blankument < ActiveRecord::Base - acts_as_url :title, :only_when_blank => true -end +# class Blankument < ActiveRecord::Base +# acts_as_url :title, :only_when_blank => true +# end -class Duplicatument < ActiveRecord::Base - acts_as_url :title, :duplicate_count_separator => "---" -end +# class Duplicatument < ActiveRecord::Base +# acts_as_url :title, :duplicate_count_separator => "---" +# end -class Validatument < ActiveRecord::Base - acts_as_url :title, :sync_url => true - validates_presence_of :title -end +# class Validatument < ActiveRecord::Base +# acts_as_url :title, :sync_url => true +# validates_presence_of :title +# end -class Ununiqument < ActiveRecord::Base - acts_as_url :title, :allow_duplicates => true -end +# class Ununiqument < ActiveRecord::Base +# acts_as_url :title, :allow_duplicates => true +# end -class Limitument < ActiveRecord::Base - acts_as_url :title, :limit => 13 -end +# class Limitument < ActiveRecord::Base +# acts_as_url :title, :limit => 13 +# end -class Skipument < ActiveRecord::Base - acts_as_url :title, :exclude => ["_So_Fucking_Special"] -end +# class Skipument < ActiveRecord::Base +# acts_as_url :title, :exclude => ["_So_Fucking_Special"] +# end module AdapterSpecificTestBehaviors def setup @@ -114,6 +78,22 @@ def setup end def teardown - # No teardown tasks at present + Document.delete_all + # Reset behavior to default + Document.class_eval do + acts_as_url :title + end + end + + def add_validation_on_document_title + Document.class_eval do + validates_presence_of :title + end + end + + def remove_validation_on_document_title + Document.class_eval do + _validators.delete :title + end end end diff --git a/test/acts_as_url/adapter/mongoid.rb b/test/acts_as_url/adapter/mongoid.rb index 70d17892..680c260b 100644 --- a/test/acts_as_url/adapter/mongoid.rb +++ b/test/acts_as_url/adapter/mongoid.rb @@ -22,101 +22,28 @@ class Document acts_as_url :title end -class Updatument - include Mongoid::Document - field :title, :type => String - field :other, :type => String - field :url, :type => String - - acts_as_url :title, :sync_url => true -end - -class Mocument - include Mongoid::Document - field :title, :type => String - field :other, :type => String - field :url, :type => String - - acts_as_url :title, :scope => :other, :sync_url => true -end - -class Permument - include Mongoid::Document - field :title, :type => String - field :url, :type => String - - acts_as_url :title, :url_attribute => :permalink -end - -class Procument - include Mongoid::Document - field :title, :type => String - field :url, :type => String - - acts_as_url :non_attribute_method - - def non_attribute_method - "#{title} got massaged" - end -end - -class Blankument - include Mongoid::Document - field :title, :type => String - field :url, :type => String - - acts_as_url :title, :only_when_blank => true -end - -class Duplicatument - include Mongoid::Document - field :title, :type => String - field :url, :type => String - - acts_as_url :title, :duplicate_count_separator => "---" -end - -class Validatument - include Mongoid::Document - field :title, :type => String - field :url, :type => String - - acts_as_url :title, :sync_url => true - validates_presence_of :title -end - -class Ununiqument - include Mongoid::Document - field :title, :type => String - field :url, :type => String - - acts_as_url :title, :allow_duplicates => true -end - -class Limitument - include Mongoid::Document - field :title, :type => String - field :url, :type => String - - acts_as_url :title, :limit => 13 -end - -class Skipument - include Mongoid::Document - field :title, :type => String - field :url, :type => String - - acts_as_url :title, :exclude => ["_So_Fucking_Special"] -end - module AdapterSpecificTestBehaviors def setup # No setup tasks at present end def teardown - Mongoid.default_session.collections.each do |collection| - collection.drop + Document.delete_all + # Reset behavior to default + Document.class_eval do + acts_as_url :title + end + end + + def add_validation_on_document_title + Document.class_eval do + validates_presence_of :title + end + end + + def remove_validation_on_document_title + Document.class_eval do + _validators.delete :title end end end diff --git a/test/acts_as_url_integration_test.rb b/test/acts_as_url_integration_test.rb index 9910c0f2..ee4634c5 100644 --- a/test/acts_as_url_integration_test.rb +++ b/test/acts_as_url_integration_test.rb @@ -15,16 +15,41 @@ def test_should_create_url def test_should_create_unique_url @doc = Document.create!(:title => "Unique") @other_doc = Document.create!(:title => "Unique") + assert_equal "unique", @doc.url assert_equal "unique-1", @other_doc.url end - def test_should_not_create_unique_url - @doc = Ununiqument.create!(:title => "I am not a clone") - @other_doc = Ununiqument.create!(:title => "I am not a clone") - assert_equal "i-am-not-a-clone", @other_doc.url + def test_should_create_unique_url_when_partial_url_already_exists + @doc = Document.create!(:title => "House Farms") + @other_doc = Document.create!(:title => "House Farm") + + assert_equal "house-farms", @doc.url + assert_equal "house-farm", @other_doc.url + end + + def test_should_not_sync_url_by_default + @doc = Document.create!(:title => "Stable as Stone") + @original_url = @doc.url + @doc.update_attributes! :title => "New Unstable Madness" + assert_equal @original_url, @doc.url end - def test_should_not_succ_on_repeated_saves + def test_should_allow_syncing_url + Document.class_eval do + acts_as_url :title, :sync_url => true + end + + @doc = Document.create!(:title => "Original") + @original_url = @doc.url + @doc.update_attributes! :title => "New and Improved" + assert_not_equal @original_url, @doc.url + end + + def test_should_not_increment_count_on_repeated_saves + Document.class_eval do + acts_as_url :title, :sync_url => true + end + @doc = Document.new(:title => "Continuous or Constant") 5.times do @doc.save! @@ -32,134 +57,184 @@ def test_should_not_succ_on_repeated_saves end end - def test_should_create_unique_url_and_not_clobber_if_another_exists - @doc = Updatument.create!(:title => "Unique") - @other_doc = Updatument.create!(:title => "Unique") - @doc.update_attributes :other => "foo" - - @doc2 = Document.create!(:title => "twonique") - @other_doc2 = Document.create!(:title => "twonique") - @doc2.update_attributes(:other => "foo") - - assert_equal "unique", @doc.url - assert_equal "foo", @doc.other - assert_equal "unique-1", @other_doc.url + def test_should_allow_allowing_duplicate_url + Document.class_eval do + acts_as_url :title, :allow_duplicates => true + end - assert_equal "twonique", @doc2.url - assert_equal "foo", @doc2.other - assert_equal "twonique-1", @other_doc2.url + @doc = Document.create!(:title => "I am not a clone") + @other_doc = Document.create!(:title => "I am not a clone") + assert_equal @doc.url, @other_doc.url end - def test_should_create_unique_url_when_partial_url_already_exists - @doc = Document.create!(:title => "House Farms") - @other_doc = Document.create!(:title => "House Farm") + def test_should_allow_scoping_url_uniqueness + Document.class_eval do + acts_as_url :title, :scope => :other + end - assert_equal "house-farms", @doc.url - assert_equal "house-farm", @other_doc.url + @doc = Document.create!(:title => "Mocumentary", :other => "I don't care if I'm unique for some reason") + @other_doc = Document.create!(:title => "Mocumentary", :other => "Me either") + assert_equal @doc.url, @other_doc.url end - def test_should_scope_uniqueness - @moc = Mocument.create!(:title => "Mocumentary", :other => "I dunno why but I don't care if I'm unique") - @other_moc = Mocument.create!(:title => "Mocumentary") - assert_equal @moc.url, @other_moc.url - end + def test_should_still_create_unique_urls_if_scoped_attribute_is_the_same + Document.class_eval do + acts_as_url :title, :scope => :other + end - def test_should_still_create_unique_if_in_same_scope - @moc = Mocument.create!(:title => "Mocumentary", :other => "Suddenly, I care if I'm unique") - @other_moc = Mocument.create!(:title => "Mocumentary", :other => "Suddenly, I care if I'm unique") - assert_not_equal @moc.url, @other_moc.url + @doc = Document.create!(:title => "Mocumentary", :other => "Suddenly, I care if I'm unique") + @other_doc = Document.create!(:title => "Mocumentary", :other => "Suddenly, I care if I'm unique") + assert_not_equal @doc.url, @other_doc.url end - def test_should_use_alternate_field_name - @perm = Permument.create!(:title => "Anything at This Point") - assert_equal "anything-at-this-point", @perm.permalink - end + def test_should_allow_setting_url_attribute + Document.class_eval do + # Manually undefining the url method on Document which, in a real class not reused for tests, + # would never have been defined to begin with. + remove_method :url + acts_as_url :title, :url_attribute => :other + end - def test_should_not_update_url_by_default - @doc = Document.create!(:title => "Stable as Stone") - @original_url = @doc.url - @doc.update_attributes :title => "New Unstable Madness" - assert_equal @original_url, @doc.url + @doc = Document.create!(:title => "Anything at This Point") + assert_equal "anything-at-this-point", @doc.other + assert_nil @doc.url + ensure + Document.class_eval do + # Manually undefining the other method on Document for the same reasons as before + remove_method :other + end end - def test_should_update_url_if_asked - @moc = Mocument.create!(:title => "Original") - @original_url = @moc.url - @moc.update_attributes :title => "New and Improved" - assert_not_equal @original_url, @moc.url - end + def test_should_allow_updating_url_only_when_blank + Document.class_eval do + acts_as_url :title, :only_when_blank => true + end - def test_should_update_url_only_when_blank_if_asked - @original_url = 'the-url-of-concrete' - @blank = Blankument.create!(:title => "Stable as Stone", :url => @original_url) - assert_equal @original_url, @blank.url - @blank = Blankument.create!(:title => "Stable as Stone") - assert_equal 'stable-as-stone', @blank.url + @string = 'the-url-of-concrete' + @doc = Document.create!(:title => "Stable as Stone", :url => @string) + assert_equal @string, @doc.url + @other_doc = Document.create!(:title => "Stable as Stone") + assert_equal 'stable-as-stone', @other_doc.url end def test_should_mass_initialize_urls - @doc_1 = Document.create!(:title => "Initial") - @doc_2 = Document.create!(:title => "Subsequent") - @doc_1.update_attribute :url, nil - @doc_2.update_attribute :url, nil - assert_nil @doc_1.url - assert_nil @doc_2.url + @doc = Document.create!(:title => "Initial") + @other_doc = Document.create!(:title => "Subsequent") + @doc.update_attributes! :url => nil + @other_doc.update_attributes! :url => nil + # Just making sure this got unset before the real test + assert_nil @doc.url + assert_nil @other_doc.url + Document.initialize_urls - @doc_1.reload - @doc_2.reload - assert_equal "initial", @doc_1.url - assert_equal "subsequent", @doc_2.url + + @doc.reload + @other_doc.reload + assert_equal "initial", @doc.url + assert_equal "subsequent", @other_doc.url end def test_should_mass_initialize_urls_with_custom_url_attribute - @doc_1 = Permument.create!(:title => "Initial") - @doc_2 = Permument.create!(:title => "Subsequent") - @doc_1.update_attribute :permalink, nil - @doc_2.update_attribute :permalink, nil - assert_nil @doc_1.permalink - assert_nil @doc_2.permalink - Permument.initialize_urls - @doc_1.reload - @doc_2.reload - assert_equal "initial", @doc_1.permalink - assert_equal "subsequent", @doc_2.permalink - end - - def test_should_utilize_block_if_given - @doc = Procument.create!(:title => "Title String") + Document.class_eval do + # Manually undefining the url method on Document which, in a real class not reused for tests, + # would never have been defined to begin with. + remove_method :url + acts_as_url :title, :url_attribute => :other + end + + @doc = Document.create!(:title => "Initial") + @other_doc = Document.create!(:title => "Subsequent") + @doc.update_attributes! :other => nil + @other_doc.update_attributes! :other => nil + # Just making sure this got unset before the real test + assert_nil @doc.other + assert_nil @other_doc.other + + Document.initialize_urls + + @doc.reload + @other_doc.reload + assert_equal "initial", @doc.other + assert_equal "subsequent", @other_doc.other + ensure + Document.class_eval do + # Manually undefining the other method on Document for the same reasons as before + remove_method :other + end + end + + def test_should_allow_using_custom_method_for_generating_url + Document.class_eval do + acts_as_url :non_attribute_method + + def non_attribute_method + "#{title} got massaged" + end + end + + @doc = Document.create!(:title => "Title String") assert_equal "title-string-got-massaged", @doc.url + ensure + Document.class_eval do + # Manually undefining method that isn't defined on Document by default + remove_method :non_attribute_method + end end - def test_should_create_unique_with_custom_duplicate_count_separator - @doc = Duplicatument.create!(:title => "Unique") - @other_doc = Duplicatument.create!(:title => "Unique") + def test_should_allow_customizing_duplicate_count_separator + Document.class_eval do + acts_as_url :title, :duplicate_count_separator => "---" + end + + @doc = Document.create!(:title => "Unique") + @other_doc = Document.create!(:title => "Unique") assert_equal "unique", @doc.url assert_equal "unique---1", @other_doc.url end - def test_should_only_update_url_if_model_is_valid - @doc = Validatument.create!(:title => "Initial") + def test_should_only_update_url_if_url_attribute_is_valid + Document.class_eval do + acts_as_url :title, :sync_url => true + end + add_validation_on_document_title + + @doc = Document.create!(:title => "Valid Record", :other => "Present") + assert_equal "valid-record", @doc.url @doc.title = nil - assert !@doc.valid? - assert_equal "initial", @doc.url + assert_equal false, @doc.valid? + assert_equal "valid-record", @doc.url + ensure + remove_validation_on_document_title end - def test_should_allow_url_limit - @doc = Limitument.create!(:title => "I am much too long") + def test_should_allow_customizing_url_limit + Document.class_eval do + acts_as_url :title, :limit => 13 + end + + @doc = Document.create!(:title => "I am much too long") assert_equal "i-am-much-too", @doc.url end - def test_should_handle_duplicate_urls_with_limits - @doc = Limitument.create!(:title => "I am long but not unique") - assert_equal "i-am-long-but", @doc.url - @doc_2 = Limitument.create!(:title => "I am long but not unique") - assert_equal "i-am-long-but-1", @doc_2.url + def test_handling_duplicate_urls_with_limits + Document.class_eval do + acts_as_url :title, :limit => 13 + end + + @doc = Document.create!(:title => "I am much too long and also duplicated") + assert_equal "i-am-much-too", @doc.url + @other_doc = Document.create!(:title => "I am much too long and also duplicated") + assert_equal "i-am-much-too-1", @other_doc.url end - def test_should_allow_exclusions - @doc = Skipument.create!(:title => "_So_Fucking_Special") + def test_should_allow_excluding_specific_values_from_being_run_through_to_url + Document.class_eval do + acts_as_url :title, :exclude => ["_So_Fucking_Special"] + end + + @doc = Document.create!(:title => "_So_Fucking_Special") assert_equal "_So_Fucking_Special", @doc.url - @doc_2 = Skipument.create!(:title => "But I'm a creep") + @doc_2 = Document.create!(:title => "But I'm a creep") assert_equal "but-im-a-creep", @doc_2.url end end