Merge pull request #2834 from evazion/fix-ip-search

IP search fixes

app/helpers/application_helper.rb

@@ -89,6 +89,10 @@ module ApplicationHelper
     time_tag(time.strftime("%Y-%m-%d %H:%M"), time)
   end
 
+  def link_to_ip(ip)
+    link_to ip, moderator_ip_addrs_path(:search => {:ip_addr => ip})
+  end
+
   def link_to_user(user, options = {})
     user_class = user.level_class
     user_class = user_class + " user-post-approver" if user.can_approve_posts?

app/logical/moderator/ip_addr_search.rb

@@ -1,10 +1,9 @@
 module Moderator
   class IpAddrSearch
-    attr_reader :params, :errors
+    attr_reader :params
 
     def initialize(params)
       @params = params
-      @errors = []
     end
 
     def execute
@@ -20,49 +19,52 @@ module Moderator
     end
 
   private
-    def select_all_sql(sql, source, *params)
-      source.select_all_sql(sql, *params)
-    end
-
     def search_by_ip_addr(ip_addrs)
       sums = Hash.new {|h, k| h[k] = 0}
 
-      add_row(sums, "select id as k, 1 as count from users where last_ip_addr in (?)", ip_addrs)
-      add_row(sums, "select creator_id as k, count(*) from comments where ip_addr in (?) group by k", ip_addrs)
-      add_row(sums, "select updater_id as k, count(*) from post_versions where updater_ip_addr in (?) group by k", ip_addrs)
-      add_row(sums, "select updater_id as k, count(*) from note_versions where updater_ip_addr in (?) group by k", ip_addrs)
-      add_row(sums, "select updater_id as k, count(*) from pool_versions where updater_ip_addr in (?) group by k", ip_addrs, PoolArchive)
-      add_row(sums, "select updater_id as k, count(*) from wiki_page_versions where updater_ip_addr in (?) group by k", ip_addrs)
-      add_row(sums, "select from_id as k, count(*) from dmails where creator_ip_addr in (?) group by k", ip_addrs)
+      add_row(sums, ArtistCommentaryVersion.where(updater_ip_addr: ip_addrs).group(:updater).count)
+      add_row(sums, ArtistVersion.where(updater_ip_addr: ip_addrs).group(:updater).count)
+      add_row(sums, NoteVersion.where(updater_ip_addr: ip_addrs).group(:updater).count)
+      add_row(sums, PoolArchive.where(updater_ip_addr: ip_addrs).group(:updater).count) if PoolArchive.enabled?
+      add_row(sums, PostVersion.where(updater_ip_addr: ip_addrs).group(:updater).count)
+      add_row(sums, WikiPageVersion.where(updater_ip_addr: ip_addrs).group(:updater).count)
+      add_row(sums, Comment.where(ip_addr: ip_addrs).group(:creator).count)
+      add_row(sums, Dmail.where(creator_ip_addr: ip_addrs).group(:from).count)
+      add_row(sums, PostAppeal.where(creator_ip_addr: ip_addrs).group(:creator).count)
+      add_row(sums, PostFlag.where(creator_ip_addr: ip_addrs).group(:creator).count)
+      add_row(sums, Upload.where(uploader_ip_addr: ip_addrs).group(:uploader).count)
+      add_row(sums, Hash[User.where(last_ip_addr: ip_addrs).collect { |user| [user, 1] }])
 
       sums
     end
 
     def search_by_user_name(user_names)
-      user_names = user_names.map do |username|
-        username.downcase.strip.tr(" ", "_")
-      end
-      users = User.where("lower(name) in (?)", user_names)
-      search_by_user_id(users.map(&:id))
+      user_ids = user_names.map { |name| User.name_to_id(name) }
+      search_by_user_id(user_ids)
     end
 
     def search_by_user_id(user_ids)
       sums = Hash.new {|h, k| h[k] = 0}
+      users = User.find(user_ids)
 
-      add_row(sums, "select ip_addr as k, count(*) from comments where creator_id in (?) group by k", user_ids)
-      add_row(sums, "select updater_ip_addr as k, count(*) from post_versions where updater_id in (?) group by k", user_ids)
-      add_row(sums, "select updater_ip_addr as k, count(*) from note_versions where updater_id in (?) group by k", user_ids)
-      add_row(sums, "select updater_ip_addr as k, count(*) from pool_versions where updater_id in (?) group by k", user_ids, PoolArchive)
-      add_row(sums, "select updater_ip_addr as k, count(*) from wiki_page_versions where updater_id in (?) group by k", user_ids)
-      add_row(sums, "select creator_ip_addr as k, count(*) from dmails where from_id in (?) group by k", user_ids)
+      add_row(sums, ArtistCommentaryVersion.where(updater: users).group(:updater_ip_addr).count)
+      add_row(sums, ArtistVersion.where(updater: users).group(:updater_ip_addr).count)
+      add_row(sums, NoteVersion.where(updater: users).group(:updater_ip_addr).count)
+      add_row(sums, PoolArchive.where(updater: users).group(:updater_ip_addr).count) if PoolArchive.enabled?
+      add_row(sums, PostVersion.where(updater: users).group(:updater_ip_addr).count)
+      add_row(sums, WikiPageVersion.where(updater: users).group(:updater_ip_addr).count)
+      add_row(sums, Comment.where(creator: users).group(:ip_addr).count)
+      add_row(sums, Dmail.where(from: users).group(:creator_ip_addr).count)
+      add_row(sums, PostAppeal.where(creator: users).where.not(creator_ip_addr: nil).group(:creator_ip_addr).count)
+      add_row(sums, PostFlag.where(creator: users).group(:creator_ip_addr).count)
+      add_row(sums, Upload.where(uploader: users).group(:uploader_ip_addr).count)
+      add_row(sums, User.where(id: users).where.not(last_ip_addr: nil).group(:last_ip_addr).count)
 
       sums
     end
 
-    def add_row(sums, sql, ip_addrs, source = ActiveRecord::Base)
-      select_all_sql(sql, source, ip_addrs).each do |row|
-        sums[row["k"]] += row["count"].to_i
-      end
+    def add_row(sums, counts)
+      sums.merge!(counts) { |key, oldcount, newcount| oldcount + newcount }
     end
   end
 end

app/views/artist_commentary_versions/index.html.erb

@@ -32,7 +32,7 @@
             </td>
             <% if CurrentUser.is_moderator? %>
               <td>
-                <%= commentary_version.updater_ip_addr %>
+                <%= link_to_ip commentary_version.updater_ip_addr %>
               </td>
             <% end %>
             <td><%= link_to_user commentary_version.updater %></td>

app/views/artist_versions/index.html.erb

@@ -37,7 +37,7 @@
               <td><%= link_to_user artist_version.updater %></td>
               <% if CurrentUser.is_moderator? %>
                 <td>
-                  <%= artist_version.updater_ip_addr %>
+                  <%= link_to_ip artist_version.updater_ip_addr %>
                 </td>
               <% end %>
               <td><%= artist_version.is_active? %></td>

app/views/dmails/show.html.erb

@@ -9,7 +9,7 @@
         <li><strong>Recipient</strong>: <%= link_to_user @dmail.to %></li>
         <li><strong>Date</strong>: <%= compact_time(@dmail.created_at) %></li>
         <% if CurrentUser.user.is_moderator? %>
-          <li><strong>Sender IP</strong>: <%= @dmail.creator_ip_addr %></li>
+          <li><strong>Sender IP</strong>: <%= link_to_ip @dmail.creator_ip_addr %></li>
         <% end %>
       </ul>
 

app/views/ip_bans/index.html.erb

@@ -14,7 +14,7 @@
       <tbody>
         <% @ip_bans.each do |ip_ban| %>
           <tr>
-            <td><%= ip_ban.ip_addr %></td>
+            <td><%= link_to_ip ip_ban.ip_addr %></td>
             <td><%= ip_ban.creator.name %></td>
             <td><%= ip_ban.reason %></td>
             <td><%= link_to "Unban", ip_ban_path(ip_ban), :remote => true, :method => :delete, :data => {:confirm => "Do your really want to unban #{ip_ban.ip_addr}?"} %></td>

app/views/maintenance/user/dmail_filters/edit.html.erb

@@ -10,7 +10,7 @@
         <li><strong>Recipient</strong>: <%= link_to_user @dmail.to, :raw => true %></li>
         <li><strong>Date</strong>: <%= compact_time(@dmail.created_at) %></li>
         <% if CurrentUser.user.is_moderator? %>
-          <li><strong>Sender IP</strong>: <%= @dmail.creator_ip_addr %></li>
+          <li><strong>Sender IP</strong>: <%= link_to_ip @dmail.creator_ip_addr %></li>
         <% end %>
       </ul>
 

app/views/moderator/ip_addrs/_ip_listing.erb

@@ -1,4 +1,4 @@
-<p><%= link_to "Search for users with these IP addresses", moderator_ip_addrs_path(:search => {:ip_addr => @results.map {|k, count| k}.reject{|ip| ip == "127.0.0.1"}.join(",")}) %></p>
+<p><%= link_to "Search for users with these IP addresses", moderator_ip_addrs_path(:search => {:ip_addr => @results.keys.reject{|ip| ip == "127.0.0.1"}.join(",")}) %></p>
 
 <table class="striped">
   <thead>
@@ -10,7 +10,7 @@
   <tbody>
     <% @results.each do |ip_address, count| %>
       <tr>
-        <td><%= link_to ip_address, moderator_ip_addrs_path(:search => {:ip_addr => ip_address}) %></td>
+        <td><%= link_to_ip ip_address %></td>
         <td><%= count %></td>
       </tr>
     <% end %>

app/views/moderator/ip_addrs/_user_listing.erb

@@ -1,4 +1,4 @@
-<p><%= link_to "Search for IP addresses with these users", moderator_ip_addrs_path(:search => {:user_id => @results.map{|k, count| k}.join(",")}) %></p>
+<p><%= link_to "Search for IP addresses with these users", moderator_ip_addrs_path(:search => {:user_id => @results.map{|user, count| user.id}.join(",")}) %></p>
 
 <table class="striped">
   <thead>
@@ -9,12 +9,12 @@
     </tr>
   </thead>
   <tbody>
-    <% @results.each do |user_id, count| %>
+    <% @results.each do |user, count| %>
       <tr>
-        <td><%= link_to User.id_to_name(user_id), user_path(user_id) %></td>
+        <td><%= link_to_user user %></td>
         <td><%= count %></td>
-        <td><%= link_to "Show IP addresses", moderator_ip_addrs_path(:search => {:user_id => user_id}) %></td>
+        <td><%= link_to "Show IP addresses", moderator_ip_addrs_path(:search => {:user_id => user.id}) %></td>
       </tr>
     <% end %>
   </tbody>
-</table>
+</table>

app/views/note_versions/index.html.erb

@@ -38,7 +38,7 @@
             </td>
             <% if CurrentUser.is_moderator? %>
               <td>
-                <%= note_version.updater_ip_addr %>
+                <%= link_to_ip note_version.updater_ip_addr %>
               </td>
             <% end %>
             <td><%= link_to_user note_version.updater %></td>

app/views/pool_versions/index.html.erb

@@ -27,7 +27,7 @@
             <td><%= link_to_user pool_version.updater %></td>
             <% if CurrentUser.is_moderator? %>
               <td>
-                <%= pool_version.updater_ip_addr %>
+                <%= link_to_ip pool_version.updater_ip_addr %>
               </td>
             <% end %>
             <td><%= compact_time pool_version.updated_at %></td>

app/views/post_versions/_listing.html.erb

@@ -30,7 +30,7 @@
           <td><%= post_version.parent_id %></td>
           <% if CurrentUser.is_moderator? %>
             <td>
-              <%= post_version.updater_ip_addr %>
+              <%= link_to_ip post_version.updater_ip_addr %>
             </td>
           <% end %>
           <td><%= post_version_diff(post_version) %></td>

app/views/wiki_page_versions/index.html.erb

@@ -54,7 +54,7 @@
                 <td><%= link_to "wiki", wiki_page_path(wiki_page_version.wiki_page_id) %></td>
                 <% if CurrentUser.is_moderator? %>
                   <td>
-                    <%= wiki_page_version.updater_ip_addr %>
+                    <%= link_to_ip wiki_page_version.updater_ip_addr %>
                   </td>
                 <% end %>
                 <td>

db/migrate/20170112021922_add_ip_addr_indexes_to_tables.rb

@@ -0,0 +1,9 @@
+class AddIpAddrIndexesToTables < ActiveRecord::Migration
+  def change
+    reversible { execute "set statement_timeout = 0" }
+    add_index :wiki_page_versions, :updater_ip_addr
+    add_index :artist_commentary_versions, :updater_ip_addr
+    add_index :artist_versions, :updater_ip_addr
+    add_index :comments, :ip_addr
+  end
+end

db/migrate/20170112060921_change_ip_addr_to_inet_on_post_appeals.rb

@@ -0,0 +1,11 @@
+class ChangeIpAddrToInetOnPostAppeals < ActiveRecord::Migration
+  def up
+    execute "set statement_timeout = 0"
+    change_column_null :post_appeals, :creator_ip_addr, true
+    execute "ALTER TABLE post_appeals ALTER COLUMN creator_ip_addr TYPE inet USING NULL"
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration, "Can't recover the lost data"
+  end
+end

db/structure.sql

@@ -2557,7 +2557,7 @@ CREATE TABLE post_appeals (
     id integer NOT NULL,
     post_id integer NOT NULL,
     creator_id integer NOT NULL,
-    creator_ip_addr integer NOT NULL,
+    creator_ip_addr inet,
     reason text,
     created_at timestamp without time zone,
     updated_at timestamp without time zone
@@ -4881,6 +4881,13 @@ CREATE INDEX index_artist_commentary_versions_on_post_id ON artist_commentary_ve
 CREATE INDEX index_artist_commentary_versions_on_updater_id_and_post_id ON artist_commentary_versions USING btree (updater_id, post_id);
 
 
+--
+-- Name: index_artist_commentary_versions_on_updater_ip_addr; Type: INDEX; Schema: public; Owner: -
+--
+
+CREATE INDEX index_artist_commentary_versions_on_updater_ip_addr ON artist_commentary_versions USING btree (updater_ip_addr);
+
+
 --
 -- Name: index_artist_urls_on_artist_id; Type: INDEX; Schema: public; Owner: -
 --
@@ -4937,6 +4944,13 @@ CREATE INDEX index_artist_versions_on_name ON artist_versions USING btree (name)
 CREATE INDEX index_artist_versions_on_updater_id ON artist_versions USING btree (updater_id);
 
 
+--
+-- Name: index_artist_versions_on_updater_ip_addr; Type: INDEX; Schema: public; Owner: -
+--
+
+CREATE INDEX index_artist_versions_on_updater_ip_addr ON artist_versions USING btree (updater_ip_addr);
+
+
 --
 -- Name: index_artists_on_group_name; Type: INDEX; Schema: public; Owner: -
 --
@@ -5035,6 +5049,13 @@ CREATE INDEX index_comments_on_body_index ON comments USING gin (body_index);
 CREATE INDEX index_comments_on_creator_id_and_post_id ON comments USING btree (creator_id, post_id);
 
 
+--
+-- Name: index_comments_on_ip_addr; Type: INDEX; Schema: public; Owner: -
+--
+
+CREATE INDEX index_comments_on_ip_addr ON comments USING btree (ip_addr);
+
+
 --
 -- Name: index_comments_on_post_id; Type: INDEX; Schema: public; Owner: -
 --
@@ -7051,6 +7072,13 @@ CREATE INDEX index_users_on_last_ip_addr ON users USING btree (last_ip_addr) WHE
 CREATE UNIQUE INDEX index_users_on_name ON users USING btree (lower((name)::text));
 
 
+--
+-- Name: index_wiki_page_versions_on_updater_ip_addr; Type: INDEX; Schema: public; Owner: -
+--
+
+CREATE INDEX index_wiki_page_versions_on_updater_ip_addr ON wiki_page_versions USING btree (updater_ip_addr);
+
+
 --
 -- Name: index_wiki_page_versions_on_wiki_page_id; Type: INDEX; Schema: public; Owner: -
 --
@@ -7409,3 +7437,7 @@ INSERT INTO schema_migrations (version) VALUES ('20161229001201');
 
 INSERT INTO schema_migrations (version) VALUES ('20170106012138');
 
+INSERT INTO schema_migrations (version) VALUES ('20170112021922');
+
+INSERT INTO schema_migrations (version) VALUES ('20170112060921');
+

test/unit/moderator/ip_addr_search_test.rb

@@ -19,17 +19,17 @@ module Moderator
 
       should "find by ip addr" do
         @search = IpAddrSearch.new(:ip_addr => "127.0.0.1")
-        assert_equal({@user.id.to_s => 2}, @search.execute)
+        assert_equal({@user => 2}, @search.execute)
       end
 
       should "find by user id" do
         @search = IpAddrSearch.new(:user_id => @user.id.to_s)
-        assert_equal({"127.0.0.1" => 2}, @search.execute)
+        assert_equal({IPAddr.new("127.0.0.1") => 2}, @search.execute)
       end
 
       should "find by user name" do
         @search = IpAddrSearch.new(:user_name => @user.name)
-        assert_equal({"127.0.0.1" => 2}, @search.execute)
+        assert_equal({IPAddr.new("127.0.0.1") => 2}, @search.execute)
       end
     end
   end