Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Safer and better testing of asset mp3 tags, names, and generation of …

…permalinks
  • Loading branch information...
commit f37856f18942c18bfef18ec136513f0c1f3c5602 1 parent 487b495
@sudara authored
View
6 app/models/asset.rb
@@ -140,15 +140,17 @@ def types_to_conditions(types)
def name
# make sure the title is there, and if not, the filename is used...
- title && !title.empty? ? title : clean_filename
+ (title && !title.strip.blank?) ? title.strip : clean_filename
end
def public_mp3
self.s3_url
end
+ # never allow this to be blank, as permalinks are generated from it
def clean_filename
- self.filename[0..-5].gsub(/-|_/,' ').titleize
+ clean = self.filename[0..-5].gsub(/-|_/,' ').strip.titleize
+ clean.blank? ? 'untitled' : clean
end
def clean_permalink
View
BIN  spec/fixtures/assets/_ .mp3
Binary file not shown
View
BIN  spec/fixtures/assets/emptytags.mp3
Binary file not shown
View
BIN  spec/fixtures/assets/中文測試.mp3
Binary file not shown
View
34 spec/models/asset_spec.rb
@@ -54,10 +54,42 @@
@asset.name.should == 'Old Muppet Men Booing'
end
+ it 'should still work even when tags are empty and the name is weird' do
+ @asset = new_track('_ .mp3')
+ @asset.save
+ @asset.permalink.should == 'untitled'
+ @asset.name.should == 'untitled'
+ end
+
it 'should handle strange charsets / characters in title tags' do
@asset = new_track('japanese-characters.mp3')
+ @asset.title = ''
@asset.save
- @asset.name.should == 'Old Muppet Men Booing'
+ @asset.permalink.should == 'untitled' # name is still 01-\266Ե??\313"
+ end
+
+ it 'should handle empty name' do
+ @asset = new_track('japanese-characters.mp3')
+ @asset.save
+ @asset.permalink.should == "untitled" # name is 01-\266Ե??\313"
+ @asset.title = 'bee'
+ @asset.save
+ @asset.permalink.should == 'bee'
+ end
+
+ it 'should handle non-english filenames and default to untitled' do
+ @asset = new_track('中文測試.mp3')
+ @asset.save.should == true
+ @asset.permalink.should == 'untitled'
+ end
+
+ it 'should handle titles with only ???? and default to untitled' do
+ @asset = new_track('中文測試.mp3')
+ @asset.save.should == true
+ @asset.permalink.should == 'untitled'
+ @asset.title = "中文測試"
+ @asset.save.should == true
+ @asset.permalink.should == 'untitled'
end
it 'should use the mp3 tag1 title as name if present' do
View
1  spec/spec_helper.rb
@@ -10,6 +10,7 @@
config.use_instantiated_fixtures = false
config.fixture_path = RAILS_ROOT + '/spec/fixtures/'
+
# == Fixtures
#
# You can declare fixtures for each example_group like this:
View
27 vendor/plugins/in_place_editor/README
@@ -1,27 +0,0 @@
-InPlaceEditor
-=============
-
-v0.9
-
-An improved in_place_editor method than that provided in rails.
-
-This plugin is drop-in compatible with the current/old in_place_editor method, but adds the ability to specify any option that the Ajax.InPlaceEditor control accepts.
-
-The current rails version is missing a lot of valid options for the control, but this plugin will accept any current and new parameters for the control.
-
-For example, the current rails version (< 2.0) doesn't allow the setting of onFailure for the control, but with this plugin, simply specify it as follows...
-:on_failure => "function(transport) {alert(\"Error: \" + transport.responseText.stripTags());}"
-
-The option translation simply converts underscored options to camelcase in the JS.
-eg. :on_failure -> onFailure, :rows -> rows, :highlight_color -> highlightColor
-
-* note
-highlightColor is the name of the option in the new rewritten version of the control, highlightcolor (no capital C) is used in the version 1.7 of scriptaculous and previous versions.
-
-Some options require quoting to be valid in the JS, such as :highlight_color. The 2 ways to specify the option are...
-
-1) Manually quote the option
-:highlight_color => "'#000000'"
-
-2) Use the :quoted option
-:quoted => {:highlight_color => '#000000'}
View
22 vendor/plugins/permalink_fu/Rakefile
@@ -0,0 +1,22 @@
+require 'rake'
+require 'rake/testtask'
+require 'rake/rdoctask'
+
+desc 'Default: run unit tests.'
+task :default => :test
+
+desc 'Test the permalink_fu plugin.'
+Rake::TestTask.new(:test) do |t|
+ t.libs << 'lib'
+ t.pattern = 'test/**/*_test.rb'
+ t.verbose = true
+end
+
+desc 'Generate documentation for the permalink_fu plugin.'
+Rake::RDocTask.new(:rdoc) do |rdoc|
+ rdoc.rdoc_dir = 'rdoc'
+ rdoc.title = 'InPlaceEditor'
+ rdoc.options << '--line-numbers' << '--inline-source'
+ rdoc.rdoc_files.include('README')
+ rdoc.rdoc_files.include('lib/**/*.rb')
+end
View
7 vendor/plugins/permalink_fu/lib/permalink_fu.rb
@@ -6,11 +6,16 @@ class << self
attr_accessor :translation_from
def escape(str)
- s = Iconv.iconv(translation_to, translation_from, str).to_s
+ begin # this can fail when its passed japanese characters
+ s = Iconv.iconv(translation_to, translation_from, str).to_s
+ rescue Iconv::InvalidCharacter
+ s = 'untitled'
+ end
s.gsub!(/\W+/, ' ') # all non-word chars to spaces
s.strip! # ohh la la
s.downcase! #
s.gsub!(/\ +/, '-') # spaces to dashes, preferred separator char everywhere
+ s = (s.blank? ? 'untitled' : s)
s
end
end
View
261 vendor/plugins/permalink_fu/test/permalink_fu_test.rb
@@ -0,0 +1,261 @@
+require 'test/unit'
+require File.join(File.dirname(__FILE__), '../lib/permalink_fu')
+
+class FauxColumn < Struct.new(:limit)
+end
+
+class BaseModel
+ def self.columns_hash
+ @columns_hash ||= {'permalink' => FauxColumn.new(100)}
+ end
+
+ include PermalinkFu
+ attr_accessor :id
+ attr_accessor :title
+ attr_accessor :extra
+ attr_reader :permalink
+ attr_accessor :foo
+
+ class << self
+ attr_accessor :validation
+ end
+
+ def self.generated_methods
+ @generated_methods ||= []
+ end
+
+ def self.primary_key
+ :id
+ end
+
+ def self.logger
+ nil
+ end
+
+ # ripped from AR
+ def self.evaluate_attribute_method(attr_name, method_definition, method_name=attr_name)
+
+ unless method_name.to_s == primary_key.to_s
+ generated_methods << method_name
+ end
+
+ begin
+ class_eval(method_definition, __FILE__, __LINE__)
+ rescue SyntaxError => err
+ generated_methods.delete(attr_name)
+ if logger
+ logger.warn "Exception occurred during reader method compilation."
+ logger.warn "Maybe #{attr_name} is not a valid Ruby identifier?"
+ logger.warn "#{err.message}"
+ end
+ end
+ end
+
+ def self.exists?(*args)
+ false
+ end
+
+ def self.before_validation(method)
+ self.validation = method
+ end
+
+ def validate
+ send self.class.validation
+ permalink
+ end
+
+ def new_record?
+ @id.nil?
+ end
+
+ def write_attribute(key, value)
+ instance_variable_set "@#{key}", value
+ end
+end
+
+class MockModel < BaseModel
+ def self.exists?(conditions)
+ if conditions[1] == 'foo' || conditions[1] == 'bar' ||
+ (conditions[1] == 'bar-2' && conditions[2] != 2)
+ true
+ else
+ false
+ end
+ end
+
+ has_permalink :title
+end
+
+class ScopedModel < BaseModel
+ def self.exists?(conditions)
+ if conditions[1] == 'foo' && conditions[2] != 5
+ true
+ else
+ false
+ end
+ end
+
+ has_permalink :title, :scope => :foo
+end
+
+class IfProcConditionModel < BaseModel
+ has_permalink :title, :if => Proc.new { |obj| false }
+end
+
+class IfMethodConditionModel < BaseModel
+ has_permalink :title, :if => :false_method
+
+ def false_method; false; end
+end
+
+class IfStringConditionModel < BaseModel
+ has_permalink :title, :if => 'false'
+end
+
+class UnlessProcConditionModel < BaseModel
+ has_permalink :title, :unless => Proc.new { |obj| false }
+end
+
+class UnlessMethodConditionModel < BaseModel
+ has_permalink :title, :unless => :false_method
+
+ def false_method; false; end
+end
+
+class UnlessStringConditionModel < BaseModel
+ has_permalink :title, :unless => 'false'
+end
+
+class MockModelExtra < BaseModel
+ has_permalink [:title, :extra]
+end
+
+class PermalinkFuTest < Test::Unit::TestCase
+ @@samples = {
+ 'This IS a Tripped out title!!.!1 (well/ not really)' => 'this-is-a-tripped-out-title-1-well-not-really',
+ '////// meph1sto r0x ! \\\\\\' => 'meph1sto-r0x',
+ 'āčēģīķļņū' => 'acegiklnu',
+ '中文測試 chinese text' => 'chinese-text',
+ '中文測試' => 'untitled' # the more common scenario with foreign char sets (no english)
+ }
+
+ @@extra = { 'some-)()()-ExtRa!/// .data==?> to \/\/test' => 'some-extra-data-to-test' }
+
+ def test_should_escape_permalinks
+ @@samples.each do |from, to|
+ assert_equal to, PermalinkFu.escape(from)
+ end
+ end
+
+ def test_should_escape_activerecord_model
+ @a = MockModel.new
+ @@samples.each do |from, to|
+
+ @a.title = from; @a.permalink = nil
+ assert_equal to, @a.validate
+ end
+ end
+
+ def test_multiple_attribute_permalink
+ @m = MockModelExtra.new
+ @@samples.each do |from, to|
+ @@extra.each do |from_extra, to_extra|
+ @m.title = from; @m.extra = from_extra; @m.permalink = nil
+ assert_equal "#{to}-#{to_extra}", @m.validate
+ end
+ end
+ end
+
+ def test_should_create_unique_permalink
+ @m = MockModel.new
+ @m.permalink = 'foo'
+ @m.validate
+ assert_equal 'foo-2', @m.permalink
+
+ @m.permalink = 'bar'
+ @m.validate
+ assert_equal 'bar-3', @m.permalink
+ end
+
+ def test_should_not_check_itself_for_unique_permalink
+ @m = MockModel.new
+ @m.id = 2
+ @m.permalink = 'bar-2'
+ @m.validate
+ assert_equal 'bar-2', @m.permalink
+ end
+
+ def test_should_create_unique_scoped_permalink
+ @m = ScopedModel.new
+ @m.permalink = 'foo'
+ @m.validate
+ assert_equal 'foo-2', @m.permalink
+
+ @m.foo = 5
+ @m.permalink = 'foo'
+ @m.validate
+ assert_equal 'foo', @m.permalink
+ end
+
+ def test_should_limit_permalink
+ @old = MockModel.columns_hash['permalink'].limit
+ MockModel.columns_hash['permalink'].limit = 2
+ @m = MockModel.new
+ @m.title = 'BOO'
+ assert_equal 'bo', @m.validate
+ ensure
+ MockModel.columns_hash['permalink'].limit = @old
+ end
+
+ def test_should_limit_unique_permalink
+ @old = MockModel.columns_hash['permalink'].limit
+ MockModel.columns_hash['permalink'].limit = 3
+ @m = MockModel.new
+ @m.title = 'foo'
+ assert_equal 'f-2', @m.validate
+ ensure
+ MockModel.columns_hash['permalink'].limit = @old
+ end
+
+ def test_should_abide_by_if_proc_condition
+ @m = IfProcConditionModel.new
+ @m.title = 'dont make me a permalink'
+ @m.validate
+ assert_nil @m.permalink
+ end
+
+ def test_should_abide_by_if_method_condition
+ @m = IfMethodConditionModel.new
+ @m.title = 'dont make me a permalink'
+ @m.validate
+ assert_nil @m.permalink
+ end
+
+ def test_should_abide_by_if_string_condition
+ @m = IfStringConditionModel.new
+ @m.title = 'dont make me a permalink'
+ @m.validate
+ assert_nil @m.permalink
+ end
+
+ def test_should_abide_by_unless_proc_condition
+ @m = UnlessProcConditionModel.new
+ @m.title = 'make me a permalink'
+ @m.validate
+ assert_not_nil @m.permalink
+ end
+
+ def test_should_abide_by_unless_method_condition
+ @m = UnlessMethodConditionModel.new
+ @m.title = 'make me a permalink'
+ @m.validate
+ assert_not_nil @m.permalink
+ end
+
+ def test_should_abide_by_unless_string_condition
+ @m = UnlessStringConditionModel.new
+ @m.title = 'make me a permalink'
+ @m.validate
+ assert_not_nil @m.permalink
+ end
+end
Please sign in to comment.
Something went wrong with that request. Please try again.