diff --git a/Gemfile b/Gemfile index a0b89c092..be35d7741 100644 --- a/Gemfile +++ b/Gemfile @@ -15,5 +15,7 @@ gem "delayed_job" gem "super_exception_notifier" gem "haml" gem "simple_form" +gem "mechanize" +gem "nokogiri" gem "will_paginate", :git => "http://github.com/mislav/will_paginate.git", :branch => "rails3" diff --git a/Gemfile.lock b/Gemfile.lock index 4774600bf..1da923683 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -58,10 +58,13 @@ GEM i18n (~> 0.4.1) mime-types (~> 1.16) treetop (~> 1.4.8) + mechanize (1.0.0) + nokogiri (>= 1.2.1) memcache-client (1.8.5) mime-types (1.16) mocha (0.9.9) rake + nokogiri (1.4.3.1) pg (0.9.0) polyglot (0.3.1) rack (1.2.1) @@ -102,8 +105,10 @@ DEPENDENCIES ffaker! haml imagesize + mechanize memcache-client mocha + nokogiri pg rails (= 3.0.0) shoulda diff --git a/app/logical/pixiv_proxy.rb b/app/logical/pixiv_proxy.rb index 25b2fa7dd..4dc192081 100644 --- a/app/logical/pixiv_proxy.rb +++ b/app/logical/pixiv_proxy.rb @@ -23,7 +23,7 @@ class PixivProxy mech = create_mechanize hash = {} mech.get(url) do |page| - hash[:artist] = page.search("div#profile/div/a/img").attr("alt") + hash[:artist] = page.search("a.avatar_m").attr("title").value hash[:listing_url] = "/member_illust.php?id=" + url[/id=(\d+)/, 1] end hash @@ -34,13 +34,13 @@ class PixivProxy mech = create_mechanize hash = {} mech.get(url) do |page| - if page.search("div#profile/div/a/img") - hash[:artist] = page.search("div#profile/div/a/img").attr("alt") - hash[:image_url] = page.search("div#profile/div/a/img").attr("src").sub("_m.", ".") - hash[:profile_url] = page.search("div#profile/div/a").attr("href") - hash[:jp_tags] = page.search("div#tag_area/span#tags/a").map do |node| + if page.search("a.avatar_m") + hash[:artist] = page.search("a.avatar_m").attr("title").value + hash[:image_url] = page.search("div.works_display/a/img").attr("src").value.sub("_m.", ".") + hash[:profile_url] = page.search("a.avatar_m").attr("href").value + hash[:jp_tags] = page.search("span#tags/a").map do |node| [node.inner_text, node.attribute("href").to_s] - end + end.reject {|x| x[0].empty?} else hash[:artist] = "?" hash[:image_url] = "?" @@ -80,10 +80,10 @@ class PixivProxy end def self.create_mechanize - mech = WWW::Mechanize.new + mech = Mechanize.new mech.get("http://www.pixiv.net") do |page| - page.form_with(:action => "login.php") do |form| + page.form_with(:action => "/login.php") do |form| form.pixiv_id = "uroobnad" form.pass = "uroobnad556" end.click_button diff --git a/app/models/tag_alias.rb b/app/models/tag_alias.rb index 7bdaf1ed3..b72cdfc7e 100644 --- a/app/models/tag_alias.rb +++ b/app/models/tag_alias.rb @@ -1,9 +1,13 @@ class TagAlias < ActiveRecord::Base attr_accessor :creator_ip_addr after_save :update_posts - after_commit :clear_cache - after_commit :clear_remote_cache - validates_presence_of :creator_id, :creator_ip_addr + after_save :clear_cache + after_save :clear_remote_cache + after_save :update_cache + after_destroy :clear_cache + after_destroy :clear_remote_cache + before_validation :initialize_creator, :on => :create + validates_presence_of :creator_id validates_uniqueness_of :antecedent_name validate :absence_of_transitive_relation belongs_to :creator, :class_name => "User" @@ -21,6 +25,10 @@ class TagAlias < ActiveRecord::Base alias_hash.values.flatten.uniq end + def initialize_creator + self.creator_id = CurrentUser.user.id + end + def absence_of_transitive_relation # We don't want a -> b && b -> c chains if self.class.exists?(["antecedent_name = ?", consequent_name]) || self.class.exists?(["consequent_name = ?", antecedent_name]) @@ -48,11 +56,9 @@ class TagAlias < ActiveRecord::Base escaped_antecedent_name = Regexp.escape(antecedent_name) fixed_tags = post.tag_string.sub(/(?:\A| )#{escaped_antecedent_name}(?:\Z| )/, " #{consequent_name} ").strip - CurrentUser.scoped(creator, creator_ip_addr) do - post.update_attributes( - :tag_string => fixed_tags - ) - end + post.update_attributes( + :tag_string => fixed_tags + ) end end end diff --git a/app/models/tag_implication.rb b/app/models/tag_implication.rb index 5d5fa2dff..f264dfd58 100644 --- a/app/models/tag_implication.rb +++ b/app/models/tag_implication.rb @@ -1,19 +1,104 @@ class TagImplication < ActiveRecord::Base - attr_accessor :creator_ip_addr before_save :clear_cache before_save :update_descendant_names after_save :update_descendant_names_for_parent - after_destroy :clear_cache + after_save :update_cache after_save :update_posts + after_destroy :clear_cache + after_destroy :clear_remote_cache belongs_to :creator, :class_name => "User" - validates_presence_of :creator_id, :creator_ip_addr + before_validation :initialize_creator, :on => :create + validates_presence_of :creator_id validates_uniqueness_of :antecedent_name, :scope => :consequent_name validate :absence_of_circular_relation - def self.with_descendants(names) - Cache.get_multi(names.flatten, "ti") do |name| - ([name] + where(["antecedent_name = ?", name]).all.map {|x| x.descendant_names_array}).flatten - end.values.flatten.uniq + module CacheMethods + def clear_cache + Cache.delete("ti:#{Cache.sanitize(antecedent_name)}") + @descendants = nil + end + + def clear_remote_cache + Danbooru.config.other_server_hosts.each do |server| + Net::HTTP.delete(URI.parse("http://#{server}/tag_implications/#{id}/cache")) + end + end + + def update_cache + descendant_names_array + true + end + end + + module DescendantMethods + extend ActiveSupport::Concern + + module ClassMethods + def with_descendants(names) + names + Cache.get_multi(names.flatten, "ti") do |name| + ([name] + where(["antecedent_name = ?", name]).all.map {|x| x.descendant_names_array}).flatten + end.values.flatten.uniq + end + end + + def descendants + @descendants ||= begin + [].tap do |all| + children = [consequent_name] + + until children.empty? + all.concat(children) + children = self.class.where(["antecedent_name IN (?)", children]).all.map(&:consequent_name) + end + end + end + end + + def descendant_names_array + Cache.get("ti:#{Cache.sanitize(antecedent_name)}") do + descendant_names.split(/ /) + end + end + + def update_descendant_names + self.descendant_names = descendants.join(" ") + end + + def update_descendant_names! + update_descendant_names + save! + end + + def update_descendant_names_for_parent + p = parent + + while p + p.update_descendant_names! + p = p.parent + end + end + + def clear_descendants_cache + @descendants = nil + end + end + + module ParentMethods + def parent + @parent ||= self.class.where(["consequent_name = ?", antecedent_name]).first + end + + def clear_parent_cache + @parent = nil + end + end + + include CacheMethods + include DescendantMethods + include ParentMethods + + def initialize_creator + self.creator_id = CurrentUser.user.id end def absence_of_circular_relation @@ -24,57 +109,13 @@ class TagImplication < ActiveRecord::Base end end - def parent - @parent ||= self.class.where(["consequent_name = ?", antecedent_name]).first - end - - def descendants - @descendants ||= begin - [].tap do |all| - children = [consequent_name] - - until children.empty? - all += children - children = self.class.where(["antecedent_name IN (?)", children]).all.map(&:consequent_name) - end - end - end - end - - def descendant_names_array - Cache.get("ti:#{Cache.sanitize(antecedent_name)}") do - descendant_names.split(/ /) - end - end - - def update_descendant_names - self.descendant_names = descendants.join(" ") - end - - def update_descendant_names! - update_descendant_names - save! - end - - def update_descendant_names_for_parent - if parent - parent.update_descendant_names! - end - end - - def clear_cache - Cache.delete("ti:#{Cache.sanitize(antecedent_name)}") - end - def update_posts Post.find_by_tags(antecedent_name).find_each do |post| escaped_antecedent_name = Regexp.escape(antecedent_name) fixed_tags = post.tag_string.sub(/(?:\A| )#{escaped_antecedent_name}(?:\Z| )/, " #{antecedent_name} #{descendant_names} ").strip - CurrentUser.scoped(creator, creator_ip_addr) do - post.update_attributes( - :tag_string => fixed_tags - ) - end + post.update_attributes( + :tag_string => fixed_tags + ) end end @@ -83,12 +124,4 @@ class TagImplication < ActiveRecord::Base clear_parent_cache clear_descendants_cache end - - def clear_descendants_cache - @descendants = nil - end - - def clear_parent_cache - @parent = nil - end end diff --git a/app/models/upload.rb b/app/models/upload.rb index 380d03ff8..03aba6a5b 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -10,6 +10,7 @@ class Upload < ActiveRecord::Base before_validation :initialize_uploader, :on => :create before_validation :initialize_status, :on => :create before_create :convert_cgi_file + after_destroy :delete_temp_file validate :uploader_is_not_limited module ValidationMethods @@ -70,6 +71,8 @@ class Upload < ActiveRecord::Base end rescue Exception => x update_attribute(:status, "error: #{x} - #{x.message}") + ensure + delete_temp_file end def convert_to_post @@ -100,6 +103,10 @@ class Upload < ActiveRecord::Base end module FileMethods + def delete_temp_file + FileUtils.rm_f(temp_file_path) + end + def move_file FileUtils.mv(file_path, md5_file_path) end @@ -223,7 +230,7 @@ class Upload < ActiveRecord::Base end def temp_file_path - File.join(Rails.root, "tmp", "#{Time.now.to_f}.#{$PROCESS_ID}") + @temp_file_path ||= File.join(Rails.root, "tmp", "#{Time.now.to_f}.#{$PROCESS_ID}") end end diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb index 1ef46f025..16b04ccd0 100644 --- a/app/models/wiki_page.rb +++ b/app/models/wiki_page.rb @@ -29,14 +29,14 @@ class WikiPage < ActiveRecord::Base titled(title).select("title, id").first end - def revert_to(version, reverter_id, reverter_ip_addr) + def revert_to(version) self.title = version.title self.body = version.body self.is_locked = version.is_locked end - def revert_to!(version, reverter_id, reverter_ip_addr) - revert_to(version, reverter_id, reverter_ip_addr) + def revert_to!(version) + revert_to(version) save! end diff --git a/test/factories/tag_alias.rb b/test/factories/tag_alias.rb index d6e752a36..f96bee565 100644 --- a/test/factories/tag_alias.rb +++ b/test/factories/tag_alias.rb @@ -1,4 +1,2 @@ Factory.define(:tag_alias) do |f| - f.creator {|x| x.association(:user)} - f.creator_ip_addr "127.0.0.1" end diff --git a/test/factories/tag_implication.rb b/test/factories/tag_implication.rb index c6ba0b719..5f96c137e 100644 --- a/test/factories/tag_implication.rb +++ b/test/factories/tag_implication.rb @@ -1,4 +1,2 @@ Factory.define(:tag_implication) do |f| - f.creator {|x| x.association(:user)} - f.creator_ip_addr "127.0.0.1" end diff --git a/test/factories/wiki_page.rb b/test/factories/wiki_page.rb index aa8308047..3a624e47c 100644 --- a/test/factories/wiki_page.rb +++ b/test/factories/wiki_page.rb @@ -2,6 +2,4 @@ Factory.define(:wiki_page) do |f| f.creator {|x| x.association(:user)} f.title {|x| Faker::Lorem.words} f.body {Faker::Lorem.sentences} - f.updater_id {|x| x.creator_id} - f.updater_ip_addr "127.0.0.1" end diff --git a/test/unit/artist_test.rb b/test/unit/artist_test.rb index e1a3370df..ba3037218 100644 --- a/test/unit/artist_test.rb +++ b/test/unit/artist_test.rb @@ -48,7 +48,7 @@ class ArtistTest < ActiveSupport::TestCase should "parse urls" do artist = Factory.create(:artist, :name => "rembrandt", :url_string => "http://rembrandt.com/test.jpg http://aaa.com") artist.reload - assert_equal(["http://aaa.com", "http://rembrandt.com/test.jpg"], artist.artist_urls.map(&:to_s).sort) + assert_equal(["http://aaa.com", "http://rembrandt.com/test.jpg"], artist.urls.map(&:to_s).sort) end should "make sure old urls are deleted" do @@ -56,7 +56,7 @@ class ArtistTest < ActiveSupport::TestCase artist.url_string = "http://not.rembrandt.com/test.jpg" artist.save artist.reload - assert_equal(["http://not.rembrandt.com/test.jpg"], artist.artist_urls.map(&:to_s).sort) + assert_equal(["http://not.rembrandt.com/test.jpg"], artist.urls.map(&:to_s).sort) end should "find matches by url" do diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index ad561fee3..952ffa4ee 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -521,11 +521,15 @@ class PostTest < ActiveSupport::TestCase end should "return posts for the metatag" do - user = Factory.create(:user) - post1 = Factory.create(:post, :uploader => user) - post2 = Factory.create(:post) - post3 = Factory.create(:post) - relation = Post.find_by_tags("uploader:#{user.name}") + second_user = Factory.create(:user) + post1 = Factory.create(:post) + + CurrentUser.scoped(second_user, "127.0.0.2") do + post2 = Factory.create(:post) + post3 = Factory.create(:post) + end + + relation = Post.find_by_tags("uploader:#{CurrentUser.user.name}") assert_equal(1, relation.count) assert_equal(post1.id, relation.first.id) end diff --git a/test/unit/tag_alias_test.rb b/test/unit/tag_alias_test.rb index edeab86ce..d4a148e29 100644 --- a/test/unit/tag_alias_test.rb +++ b/test/unit/tag_alias_test.rb @@ -14,6 +14,11 @@ class TagAliasTest < ActiveSupport::TestCase CurrentUser.ip_addr = nil end + should "populate the creator information" do + ta = Factory.create(:tag_alias, :antecedent_name => "aaa", :consequent_name => "bbb") + assert_equal(CurrentUser.user.id, ta.creator_id) + end + should "convert a tag to its normalized version" do tag1 = Factory.create(:tag, :name => "aaa") tag2 = Factory.create(:tag, :name => "bbb") @@ -53,5 +58,17 @@ class TagAliasTest < ActiveSupport::TestCase assert_equal("Tag alias can not create a transitive relation with another tag alias", ta3.errors.full_messages.join) end end + + should "record the alias's creator in the tag history" do + user = Factory.create(:user) + p1 = nil + CurrentUser.scoped(user, "127.0.0.1") do + p1 = Factory.create(:post, :tag_string => "aaa bbb ccc") + end + ta1 = Factory.create(:tag_alias, :antecedent_name => "aaa", :consequent_name => "xxx") + p1.reload + assert_not_equal("uploader:#{ta1.creator_id}", p1.uploader_string) + assert_equal(ta1.creator_id, p1.versions.last.updater_id) + end end end diff --git a/test/unit/tag_implication_test.rb b/test/unit/tag_implication_test.rb index bab4f7f21..37153fa22 100644 --- a/test/unit/tag_implication_test.rb +++ b/test/unit/tag_implication_test.rb @@ -15,6 +15,11 @@ class TagImplicationTest < ActiveSupport::TestCase CurrentUser.ip_addr = nil end + should "populate the creator information" do + ti = Factory.create(:tag_implication, :antecedent_name => "aaa", :consequent_name => "bbb") + assert_equal(CurrentUser.user.id, ti.creator_id) + end + should "not validate when a circular relation is created" do ti1 = Factory.create(:tag_implication, :antecedent_name => "aaa", :consequent_name => "bbb") ti2 = Factory.build(:tag_implication, :antecedent_name => "bbb", :consequent_name => "aaa") @@ -38,7 +43,7 @@ class TagImplicationTest < ActiveSupport::TestCase ti1.update_attributes( :consequent_name => "ccc" ) - assert_nil(MEMCACHE.get("ti:aaa")) + assert_equal(["ccc"], MEMCACHE.get("ti:aaa")) end should "clear the cache upon destruction" do @@ -99,5 +104,17 @@ class TagImplicationTest < ActiveSupport::TestCase p1.reload assert_equal("aaa yyy xxx bbb ccc", p1.tag_string) end + + should "record the implication's creator in the tag history" do + user = Factory.create(:user) + p1 = nil + CurrentUser.scoped(user, "127.0.0.1") do + p1 = Factory.create(:post, :tag_string => "aaa bbb ccc") + end + ti1 = Factory.create(:tag_implication, :antecedent_name => "aaa", :consequent_name => "xxx") + p1.reload + assert_not_equal("uploader:#{ti1.creator_id}", p1.uploader_string) + assert_equal(ti1.creator_id, p1.versions.last.updater_id) + end end end diff --git a/test/unit/upload_test.rb b/test/unit/upload_test.rb index dcb34c0b1..a31f1693c 100644 --- a/test/unit/upload_test.rb +++ b/test/unit/upload_test.rb @@ -11,6 +11,8 @@ class UploadTest < ActiveSupport::TestCase teardown do CurrentUser.user = nil CurrentUser.ip_addr = nil + + @upload.delete_temp_file if @upload end context "An upload" do @@ -165,4 +167,15 @@ class UploadTest < ActiveSupport::TestCase assert_equal(post.id, @upload.post_id) assert_equal("completed", @upload.status) end + + should "delete the temporary file upon completion" do + @upload = Factory.create(:source_upload, + :rating => "s", + :uploader_ip_addr => "127.0.0.1", + :tag_string => "hoge foo" + ) + + @upload.process! + assert(!File.exists?(@upload.temp_file_path)) + end end diff --git a/test/unit/wiki_page_test.rb b/test/unit/wiki_page_test.rb index 33aa11d64..487d00a76 100644 --- a/test/unit/wiki_page_test.rb +++ b/test/unit/wiki_page_test.rb @@ -28,8 +28,6 @@ class WikiPageTest < ActiveSupport::TestCase should "create versions" do wp = nil - user = Factory.create(:user) - reverter = Factory.create(:user) assert_difference("WikiPageVersion.count") do wp = Factory.create(:wiki_page, :title => "xxx") @@ -37,15 +35,29 @@ class WikiPageTest < ActiveSupport::TestCase assert_difference("WikiPageVersion.count") do wp.title = "yyy" - wp.updater_id = user.id - wp.updater_ip_addr = "127.0.0.1" wp.save end - + end + + should "revert to a prior version" do + wp = Factory.create(:wiki_page, :title => "xxx") + wp.title = "yyy" + wp.save version = WikiPageVersion.first - wp.revert_to!(version, reverter.id, "127.0.0.1") - + wp.revert_to!(version) + wp.reload assert_equal("xxx", wp.title) end + + should "differentiate between updater and creator" do + user = Factory.create(:user) + wp = Factory.create(:wiki_page, :title => "xxx") + CurrentUser.scoped(user, "127.0.0.1") do + wp.title = "yyy" + wp.save + end + version = WikiPageVersion.first + assert_not_equal(wp.creator_id, version.updater_id) + end end end