diff --git a/Gemfile b/Gemfile index 24f6b7059..254945c6d 100644 --- a/Gemfile +++ b/Gemfile @@ -18,6 +18,6 @@ gem "haml" gem "simple_form" gem "mechanize" gem "nokogiri" -gem "meta_search" +gem "meta_search", :git => "git://github.com/ernie/meta_search.git" gem "will_paginate", :git => "git://github.com/mmack/will_paginate.git", :branch => "rails3.1" gem "silent-postgres" diff --git a/Gemfile.lock b/Gemfile.lock index 626d719ad..a81fee34a 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,3 +1,12 @@ +GIT + remote: git://github.com/ernie/meta_search.git + revision: 79806e6a9db5dc4eedbd45bccc5929cf6fbf3593 + specs: + meta_search (1.1.0.pre) + actionpack (~> 3.1.0.alpha) + activerecord (~> 3.1.0.alpha) + activesupport (~> 3.1.0.alpha) + GIT remote: git://github.com/mmack/will_paginate.git revision: 5816b4e8d4382b4b167621a9aea4b8fe72983c37 @@ -7,9 +16,9 @@ GIT GIT remote: http://github.com/EmmanuelOga/ffaker.git - revision: baf4891a5351ad01775f3e4bb77c78ceed349360 + revision: 52feff4ecddbe8834b63beb122b153b65833e309 specs: - ffaker (1.7.0) + ffaker (1.8.0) GEM remote: http://gemcutter.org/ @@ -64,11 +73,6 @@ GEM mechanize (1.0.0) nokogiri (>= 1.2.1) memcache-client (1.8.5) - meta_search (0.5.4) - actionpack (>= 3.0.0.beta4) - activerecord (>= 3.0.0.beta4) - activesupport (>= 3.0.0.beta4) - arel (>= 0.4.0) mime-types (1.16) mocha (0.9.12) multi_json (1.0.3) @@ -98,14 +102,14 @@ GEM rack-ssl (~> 1.3.2) rake (>= 0.8.7) thor (~> 0.14.6) - rake (0.9.0) + rake (0.9.2) shoulda (2.11.3) silent-postgres (0.0.8) simple_form (1.4.0) simplecov (0.4.2) simplecov-html (~> 0.4.4) simplecov-html (0.4.5) - sprockets (2.0.0.beta.8) + sprockets (2.0.0.beta.10) hike (~> 1.0) rack (~> 1.0) tilt (!= 1.3.0, ~> 1.1) @@ -113,7 +117,7 @@ GEM actionmailer rake thor (0.14.6) - tilt (1.3.1) + tilt (1.3.2) treetop (1.4.9) polyglot (>= 0.3.1) tzinfo (0.3.27) @@ -129,7 +133,7 @@ DEPENDENCIES imagesize mechanize memcache-client - meta_search + meta_search! mocha nokogiri pg diff --git a/app/models/favorite.rb b/app/models/favorite.rb index b0ef6b9dd..bb4a1c6f8 100644 --- a/app/models/favorite.rb +++ b/app/models/favorite.rb @@ -3,7 +3,7 @@ class Favorite < ActiveRecord::Base validates_uniqueness_of :post_id, :scope => :user_id def self.model_for(user_id) - mod = user_id % TABLE_COUNT + mod = user_id.to_i % TABLE_COUNT Object.const_get("Favorite#{mod}") end diff --git a/app/models/forum_post.rb b/app/models/forum_post.rb index cdca4a6fc..260239f1b 100644 --- a/app/models/forum_post.rb +++ b/app/models/forum_post.rb @@ -38,8 +38,10 @@ class ForumPost < ActiveRecord::Base end def update_topic_updated_at - topic.update_attribute(:updater_id, CurrentUser.id) - topic.touch + if topic + topic.update_attribute(:updater_id, CurrentUser.id) + topic.touch + end end def initialize_creator diff --git a/app/models/pool.rb b/app/models/pool.rb index 357aeff55..b85d233bf 100644 --- a/app/models/pool.rb +++ b/app/models/pool.rb @@ -6,9 +6,10 @@ class Pool < ActiveRecord::Base has_many :versions, :class_name => "PoolVersion", :dependent => :destroy, :order => "pool_versions.id ASC" before_validation :normalize_name before_validation :normalize_post_ids + before_validation :initialize_post_count before_validation :initialize_creator, :on => :create after_save :create_version - after_save :update_posts + after_save :balance_post_ids attr_accessible :name, :description, :post_ids, :is_active, :post_id_array def self.name_to_id(name) @@ -29,58 +30,90 @@ class Pool < ActiveRecord::Base end end + def self.normalize_name(name) + name.downcase.gsub(/\s+/, "_") + end + + def self.normalize_post_ids(post_ids) + post_ids.gsub(/\s{2,}/, " ").strip + end + def initialize_creator self.creator_id = CurrentUser.id end def normalize_name - self.name = name.downcase + self.name = Pool.normalize_name(name) end def normalize_post_ids - self.post_ids = post_ids.gsub(/\s\s+/, " ") - self.post_ids = post_ids.gsub(/^\s+/, "") - self.post_ids = post_ids.gsub(/\s+$/, "") + self.post_ids = Pool.normalize_post_ids(post_ids) end def revert_to!(version) self.post_ids = version.post_ids save end - - def update_posts - post_id_array.each do |post_id| - post = Post.find(post_id) - post.add_pool(self) - end + + def contains_post?(post_id) + post_ids =~ /(?:\A| )#{post_id}(?:\Z| )/ end def add_post!(post) - return if post_ids =~ /(?:\A| )#{post.id}(?:\Z| )/ - self.post_ids += " #{post.id}" - self.post_ids = post_ids.strip + return if contains_post?(post.id) + + increment!(:post_count) + update_attribute(:post_ids, "#{post_ids} #{post.id}".strip) + post.add_pool!(self) clear_post_id_array - save end def remove_post!(post) - self.post_ids = post_ids.gsub(/(?:\A| )#{post.id}(?:\Z| )/, " ") - self.post_ids = post_ids.strip + return unless contains_post?(post.id) + + decrement!(:post_count) + update_attribute(:post_ids, Pool.normalize_post_ids(post_ids.gsub(/(?:\A| )#{post.id}(?:\Z| )/, " "))) + post.remove_pool!(self) clear_post_id_array - save end def posts(options = {}) - offset = options[:offset] || 0 - limit = options[:limit] || Danbooru.config.posts_per_page - ids = post_id_array[offset, limit] - Post.where(["id IN (?)", ids]).order(Favorite.sql_order_clause(ids)) + if options[:offset] + limit = options[:limit] || Danbooru.config.posts_per_page + slice = post_id_array.slice(options[:offset], limit) + if slice && slice.any? + Post.where("id in (?)", slice).order(arbitrary_sql_order_clause(slice, "posts")) + else + Post.where("false") + end + else + Post.where("id IN (?)", post_id_array).order(arbitrary_sql_order_clause(post_id_array, "posts")) + end + end + + def balance_post_ids + added = post_id_array - post_id_array_was + removed = post_id_array_was - post_id_array + + added.each do |post_id| + post = Post.find(post_id) + post.add_pool!(self) + end + + removed.each do |post_id| + post = Post.find(post_id) + post.remove_pool!(self) + end end def post_id_array @post_id_array ||= post_ids.scan(/\d+/).map(&:to_i) end + def post_id_array_was + @post_id_array_was ||= post_ids_was.scan(/\d+/).map(&:to_i) + end + def post_id_array=(array) self.post_ids = array.join(" ") clear_post_id_array @@ -88,6 +121,11 @@ class Pool < ActiveRecord::Base def clear_post_id_array @post_id_array = nil + @post_id_array_was = nil + end + + def initialize_post_count + self.post_count = post_id_array.size end def neighbor_posts(post) @@ -95,13 +133,13 @@ class Pool < ActiveRecord::Base post_ids =~ /\A#{post.id} (\d+)|(\d+) #{post.id} (\d+)|(\d+) #{post.id}\Z/ if $2 && $3 - {:previous => $2.to_i, :next => $3.to_i} + OpenStruct.new(:previous => $2.to_i, :next => $3.to_i) elsif $1 - {:next => $1.to_i} + OpenStruct.new(:next => $1.to_i) elsif $4 - {:previous => $4.to_i} + OpenStruct.new(:previous => $4.to_i) else - {} + OpenStruct.new end end end diff --git a/app/models/post.rb b/app/models/post.rb index 9c0f883c3..01aba4ada 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -632,17 +632,20 @@ class Post < ActiveRecord::Base end end - def add_pool(pool) - return if pool_string =~ /(?:\A| )pool:#{pool.id}(?:\Z| )/ - self.pool_string += " pool:#{pool.id}" - self.pool_string.strip! - execute_sql("UPDATE posts SET pool_string = ? WHERE id = ?", pool_string, id) + def belongs_to_pool?(pool) + pool_string =~ /(?:\A| )pool:#{pool.id}(?:\Z| )/ end - def remove_pool(pool) - self.pool_string.gsub!(/(?:\A| )pool:#{pool.id}(?:\Z| )/, " ") - self.pool_string.strip! - execute_sql("UPDATE posts SET pool_string = ? WHERE id = ?", pool_string, id) + def add_pool!(pool) + return if belongs_to_pool?(pool) + update_attribute(:pool_string, "#{pool_string} pool:#{pool.id}".strip) + pool.add_post!(self) + end + + def remove_pool!(pool) + return unless belongs_to_pool?(pool) + update_attribute(:pool_string, pool_string.gsub(/(?:\A| )pool:#{pool.id}(?:\Z| )/, " ").strip) + pool.remove_post!(self) end end diff --git a/config/initializers/active_record_extensions.rb b/config/initializers/active_record_extensions.rb new file mode 100644 index 000000000..b83192c25 --- /dev/null +++ b/config/initializers/active_record_extensions.rb @@ -0,0 +1,39 @@ +module Danbooru + module Extensions + module ActiveRecord + %w(execute select_value select_values select_all).each do |method_name| + define_method("#{method_name}_sql") do |sql, *params| + connection.__send__(method_name, self.class.sanitize_sql_array([sql, *params])) + end + + self.class.__send__(:define_method, "#{method_name}_sql") do |sql, *params| + connection.__send__(method_name, sanitize_sql_array([sql, *params])) + end + end + + def arbitrary_sql_order_clause(ids, table_name = nil) + table_name = self.class.table_name if table_name.nil? + + if ids.empty? + return "#{table_name}.id desc" + end + + conditions = [] + + ids.each_with_index do |x, n| + conditions << "when #{x} then #{n}" + end + + "case #{table_name}.id " + conditions.join(" ") + " end" + end + end + end +end + +class ActiveRecord::Base + class << self + public :sanitize_sql_array + end + + include Danbooru::Extensions::ActiveRecord +end diff --git a/config/initializers/core_extensions.rb b/config/initializers/core_extensions.rb index c7f2482f3..88dfdbfdb 100644 --- a/config/initializers/core_extensions.rb +++ b/config/initializers/core_extensions.rb @@ -1,17 +1,5 @@ module Danbooru module Extensions - module ActiveRecord - %w(execute select_value select_values select_all).each do |method_name| - define_method("#{method_name}_sql") do |sql, *params| - connection.__send__(method_name, self.class.sanitize_sql_array([sql, *params])) - end - - self.class.__send__(:define_method, "#{method_name}_sql") do |sql, *params| - connection.__send__(method_name, sanitize_sql_array([sql, *params])) - end - end - end - module String def to_escaped_for_sql_like return self.gsub(/\\/, '\0\0').gsub(/%/, '\\%').gsub(/_/, '\\_').gsub(/\*/, '%') @@ -24,14 +12,6 @@ module Danbooru end end -class ActiveRecord::Base - class << self - public :sanitize_sql_array - end - - include Danbooru::Extensions::ActiveRecord -end - class String include Danbooru::Extensions::String end diff --git a/db/development_structure.sql b/db/development_structure.sql index 2e6fed053..14ee6d555 100644 --- a/db/development_structure.sql +++ b/db/development_structure.sql @@ -8385,4 +8385,6 @@ INSERT INTO schema_migrations (version) VALUES ('20100826232512'); INSERT INTO schema_migrations (version) VALUES ('20110328215652'); -INSERT INTO schema_migrations (version) VALUES ('20110328215701'); \ No newline at end of file +INSERT INTO schema_migrations (version) VALUES ('20110328215701'); + +INSERT INTO schema_migrations (version) VALUES ('20110607194023'); \ No newline at end of file diff --git a/db/migrate/20100211025616_create_pools.rb b/db/migrate/20100211025616_create_pools.rb index 53ca23905..e53ef9e0c 100644 --- a/db/migrate/20100211025616_create_pools.rb +++ b/db/migrate/20100211025616_create_pools.rb @@ -12,17 +12,6 @@ class CreatePools < ActiveRecord::Migration add_index :pools, :name add_index :pools, :creator_id - - - create_table :pool_versions do |t| - t.column :pool_id, :integer - t.column :post_ids, :text, :null => false, :default => "" - t.column :updater_id, :integer, :null => false - t.column :updater_ip_addr, "inet", :null => false - t.timestamps - end - - add_index :pool_versions, :pool_id end def self.down diff --git a/db/migrate/20110607194023_create_pool_versions.rb b/db/migrate/20110607194023_create_pool_versions.rb new file mode 100644 index 000000000..9717ec57a --- /dev/null +++ b/db/migrate/20110607194023_create_pool_versions.rb @@ -0,0 +1,17 @@ +class CreatePoolVersions < ActiveRecord::Migration + def up + create_table :pool_versions do |t| + t.column :pool_id, :integer + t.column :post_ids, :text, :null => false, :default => "" + t.column :updater_id, :integer, :null => false + t.column :updater_ip_addr, "inet", :null => false + t.timestamps + end + + add_index :pool_versions, :pool_id + end + + def down + drop_table :pool_versions + end +end diff --git a/test/unit/pool_test.rb b/test/unit/pool_test.rb index 72f5a7779..a8fc8997d 100644 --- a/test/unit/pool_test.rb +++ b/test/unit/pool_test.rb @@ -14,10 +14,6 @@ class PoolTest < ActiveSupport::TestCase end context "A pool" do - setup do - MEMCACHE.flush_all - end - should "create versions for each distinct user" do pool = Factory.create(:pool) user = Factory.create(:user) @@ -39,9 +35,6 @@ class PoolTest < ActiveSupport::TestCase p2 = Factory.create(:post) p3 = Factory.create(:post) p4 = Factory.create(:post) - p1.add_pool(pool) - p2.add_pool(pool) - p3.add_pool(pool) pool.add_post!(p1) pool.add_post!(p2) pool.add_post!(p3) @@ -52,7 +45,7 @@ class PoolTest < ActiveSupport::TestCase posts = pool.posts.all assert_equal(3, posts.size) assert_equal([p1.id, p2.id, p3.id], posts.map(&:id)) - posts = pool.posts(:limit => 1, :offset => 1).all + posts = pool.posts.limit(1).offset(1).all assert_equal(1, posts.size) assert_equal([p2.id], posts.map(&:id)) end @@ -62,27 +55,75 @@ class PoolTest < ActiveSupport::TestCase p1 = Factory.create(:post) p2 = Factory.create(:post) p3 = Factory.create(:post) - p1.add_pool(pool) - p2.add_pool(pool) - p3.add_pool(pool) pool.add_post!(p1) pool.add_post!(p2) pool.add_post!(p3) pool.reload neighbors = pool.neighbor_posts(p1) - assert_nil(neighbors[:previous]) - assert_equal(p2.id, neighbors[:next]) + assert_nil(neighbors.previous) + assert_equal(p2.id, neighbors.next) pool.reload neighbors = pool.neighbor_posts(p2) - assert_equal(p1.id, neighbors[:previous]) - assert_equal(p3.id, neighbors[:next]) + assert_equal(p1.id, neighbors.previous) + assert_equal(p3.id, neighbors.next) pool.reload neighbors = pool.neighbor_posts(p3) - assert_equal(p2.id, neighbors[:previous]) - assert_nil(neighbors[:next]) + assert_equal(p2.id, neighbors.previous) + assert_nil(neighbors.next) + end + + should "know what its post_ids were" do + p1 = Factory.create(:post) + p2 = Factory.create(:post) + pool = Factory.create(:pool, :post_ids => "#{p1.id}") + pool.post_id_array = [p1.id, p2.id] + assert_equal([p1.id], pool.post_id_array_was) + end + + should "update its posts if the post_ids is updated directly" do + p1 = Factory.create(:post) + p2 = Factory.create(:post) + pool = Factory.create(:pool, :post_ids => "#{p1.id}") + pool.post_id_array = [p1.id, p2.id] + pool.save + p1.reload + p2.reload + assert_equal("pool:#{pool.id}", p1.pool_string) + assert_equal("pool:#{pool.id}", p2.pool_string) + end + + should "set its post count even if post_ids is updated directly" do + p1 = Factory.create(:post) + p2 = Factory.create(:post) + pool = Factory.create(:pool, :post_ids => "#{p1.id}") + pool.post_id_array = [p1.id, p2.id] + pool.save + assert_equal(2, pool.post_count) + end + + should "increment the post count every time a post is added" do + p1 = Factory.create(:post) + pool = Factory.create(:pool) + pool.add_post!(p1) + assert_equal(1, pool.post_count) + end + + should "not double increment when the same post is readded" do + p1 = Factory.create(:post) + pool = Factory.create(:pool) + pool.add_post!(p1) + pool.add_post!(p1) + assert_equal(1, pool.post_count) + end + + should "not double decrement" do + p1 = Factory.create(:post) + pool = Factory.create(:pool) + pool.remove_post!(p1) + assert_equal(0, pool.post_count) end end diff --git a/test/unit/post_sets/pool_test.rb b/test/unit/post_sets/pool_test.rb index 9c4dd228c..95f4b047b 100644 --- a/test/unit/post_sets/pool_test.rb +++ b/test/unit/post_sets/pool_test.rb @@ -16,9 +16,6 @@ module PostSets @pool.add_post!(@post_2) @pool.add_post!(@post_1) @pool.add_post!(@post_3) - @post_2.add_pool(@pool) - @post_1.add_pool(@pool) - @post_3.add_pool(@pool) @set = PostSets::Pool.new(@pool, :page => 1) end diff --git a/test/unit/post_test.rb b/test/unit/post_test.rb index 56057b9cf..9987df120 100644 --- a/test/unit/post_test.rb +++ b/test/unit/post_test.rb @@ -356,20 +356,31 @@ class PostTest < ActiveSupport::TestCase end context "Pools:" do + context "Removing a post from a pool" do + should "update the post's pool string" do + post = Factory.create(:post) + pool = Factory.create(:pool) + post.add_pool!(pool) + post.remove_pool!(pool) + post.reload + assert_equal("", post.pool_string) + post.remove_pool!(pool) + post.reload + assert_equal("", post.pool_string) + end + end + context "Adding a post to a pool" do should "update the post's pool string" do post = Factory.create(:post) pool = Factory.create(:pool) - post.add_pool(pool) + post.add_pool!(pool) post.reload assert_equal("pool:#{pool.id}", post.pool_string) - post.add_pool(pool) + post.add_pool!(pool) post.reload assert_equal("pool:#{pool.id}", post.pool_string) - post.remove_pool(pool) - post.reload - assert_equal("", post.pool_string) - post.remove_pool(pool) + post.remove_pool!(pool) post.reload assert_equal("", post.pool_string) end @@ -474,7 +485,7 @@ class PostTest < ActiveSupport::TestCase post2 = Factory.create(:post) post3 = Factory.create(:post) pool = Factory.create(:pool) - post1.add_pool(pool) + post1.add_pool!(pool) relation = Post.tag_match("pool:#{pool.name}") assert_equal(1, relation.count) assert_equal(post1.id, relation.first.id) diff --git a/test/unit/upload_test.rb b/test/unit/upload_test.rb index a31f1693c..c0c52b66c 100644 --- a/test/unit/upload_test.rb +++ b/test/unit/upload_test.rb @@ -2,7 +2,7 @@ require_relative '../test_helper' class UploadTest < ActiveSupport::TestCase setup do - user = Factory.create(:user) + user = Factory.create(:contributor_user) CurrentUser.user = user CurrentUser.ip_addr = "127.0.0.1" MEMCACHE.flush_all diff --git a/test/unit/user_feedback_test.rb b/test/unit/user_feedback_test.rb index 1da4b337d..7267faed8 100644 --- a/test/unit/user_feedback_test.rb +++ b/test/unit/user_feedback_test.rb @@ -1,39 +1,28 @@ require_relative '../test_helper' class UserFeedbackTest < ActiveSupport::TestCase - setup do - user = Factory.create(:user) - CurrentUser.user = user - CurrentUser.ip_addr = "127.0.0.1" - MEMCACHE.flush_all - end - - teardown do - CurrentUser.user = nil - CurrentUser.ip_addr = nil - end - context "A user's feedback" do + setup do + CurrentUser.ip_addr = "127.0.0.1" + MEMCACHE.flush_all + end + + teardown do + CurrentUser.user = nil + CurrentUser.ip_addr = nil + end + should "should not validate if the creator is not privileged" do user = Factory.create(:user) - admin = Factory.create(:admin_user) - moderator = Factory.create(:moderator_user) - janitor = Factory.create(:janitor_user) - contributor = Factory.create(:contributor_user) privileged = Factory.create(:privileged_user) member = Factory.create(:user) - feedback = Factory.create(:user_feedback, :user => user, :creator => admin) + CurrentUser.user = privileged + feedback = Factory.create(:user_feedback, :user => user) assert(feedback.errors.empty?) - feedback = Factory.create(:user_feedback, :user => user, :creator => moderator) - assert(feedback.errors.empty?) - feedback = Factory.create(:user_feedback, :user => user, :creator => janitor) - assert(feedback.errors.empty?) - feedback = Factory.create(:user_feedback, :user => user, :creator => contributor) - assert(feedback.errors.empty?) - feedback = Factory.create(:user_feedback, :user => user, :creator => privileged) - assert(feedback.errors.empty?) - feedback = Factory.build(:user_feedback, :user => user, :creator => member) + + CurrentUser.user = member + feedback = Factory.build(:user_feedback, :user => user) feedback.save assert(feedback.errors.any?) end diff --git a/tmp/test.jpg b/tmp/test.jpg new file mode 100644 index 000000000..253c508d9 Binary files /dev/null and b/tmp/test.jpg differ