models: stop saving IP addresses in version tables.

Mark various `creator_ip_addr` and `updater_ip_addr` columns as ignored
and stop updating them in preparation for dropping them.
This commit is contained in:
evazion
2022-09-18 03:02:30 -05:00
parent 44d4452068
commit d4da8499ce
28 changed files with 60 additions and 50 deletions

View File

@@ -38,7 +38,7 @@ class ForumPostsController < ApplicationController
end
def create
@forum_post = authorize ForumPost.new(creator: CurrentUser.user, topic_id: params.dig(:forum_post, :topic_id))
@forum_post = authorize ForumPost.new(creator: CurrentUser.user, creator_ip_addr: CurrentUser.ip_addr, topic_id: params.dig(:forum_post, :topic_id))
@forum_post.update(permitted_attributes(@forum_post))
page = @forum_post.topic.last_page if @forum_post.topic.last_page > 1

View File

@@ -56,9 +56,9 @@ class ForumTopicsController < ApplicationController
end
def create
@forum_topic = authorize ForumTopic.new(permitted_attributes(ForumTopic))
@forum_topic.creator = CurrentUser.user
@forum_topic = authorize ForumTopic.new(creator: CurrentUser.user, **permitted_attributes(ForumTopic))
@forum_topic.original_post.creator = CurrentUser.user
@forum_topic.original_post.creator_ip_addr = CurrentUser.ip_addr
@forum_topic.save
respond_with(@forum_topic)

View File

@@ -5,12 +5,12 @@ class UploadsController < ApplicationController
skip_before_action :verify_authenticity_token, only: [:create], if: -> { request.xhr? }
def new
@upload = authorize Upload.new(uploader: CurrentUser.user, uploader_ip_addr: CurrentUser.ip_addr, source: params[:url], referer_url: params[:ref], **permitted_attributes(Upload))
@upload = authorize Upload.new(uploader: CurrentUser.user, source: params[:url], referer_url: params[:ref], **permitted_attributes(Upload))
respond_with(@upload)
end
def create
@upload = authorize Upload.new(uploader: CurrentUser.user, uploader_ip_addr: CurrentUser.ip_addr, **permitted_attributes(Upload))
@upload = authorize Upload.new(uploader: CurrentUser.user, **permitted_attributes(Upload))
@upload.save
respond_with(@upload)
end

View File

@@ -65,14 +65,14 @@ class SpamDetector
# Initialize a spam check for a message.
# @param record [Dmail, ForumPost, Comment] the message to spam check
# @param user_ip [String] the IP address of the user who posted the message
def initialize(record, user_ip: nil)
def initialize(record, user_ip:)
case record
when Dmail
@record = record
@user = record.from
@content = record.body
@comment_type = "message"
@user_ip = user_ip || record.creator_ip_addr.to_s
@user_ip = user_ip
when ForumPost
@record = record
@user = record.creator
@@ -84,7 +84,7 @@ class SpamDetector
@user = record.creator
@content = record.body
@comment_type = "comment"
@user_ip = user_ip || record.creator_ip_addr.to_s
@user_ip = user_ip
else
raise ArgumentError
end

View File

@@ -191,7 +191,6 @@ class ApplicationRecord < ActiveRecord::Base
belongs_to :updater, class_name: "User", **options
before_validation do |rec|
rec.updater_id = CurrentUser.id
rec.updater_ip_addr = CurrentUser.ip_addr if rec.respond_to?(:updater_ip_addr=)
end
end
end

View File

@@ -111,7 +111,6 @@ class Artist < ApplicationRecord
:artist_id => id,
:name => name,
:updater_id => CurrentUser.id,
:updater_ip_addr => CurrentUser.ip_addr,
:urls => url_array,
:is_deleted => is_deleted,
:is_banned => is_banned,

View File

@@ -1,6 +1,8 @@
# frozen_string_literal: true
class ArtistCommentaryVersion < ApplicationRecord
self.ignored_columns = [:updater_ip_addr]
belongs_to :post
belongs_to_updater

View File

@@ -1,6 +1,8 @@
# frozen_string_literal: true
class ArtistVersion < ApplicationRecord
self.ignored_columns = [:updater_ip_addr]
array_attribute :urls
array_attribute :other_names

View File

@@ -1,6 +1,10 @@
# frozen_string_literal: true
class Comment < ApplicationRecord
self.ignored_columns = [:creator_ip_addr, :updater_ip_addr]
attr_accessor :creator_ip_addr
belongs_to :post
belongs_to :creator, class_name: "User"
belongs_to_updater
@@ -52,7 +56,7 @@ class Comment < ApplicationRecord
extend SearchMethods
def autoreport_spam
if SpamDetector.new(self).spam?
if SpamDetector.new(self, user_ip: creator_ip_addr).spam?
moderation_reports << ModerationReport.new(creator: User.system, reason: "Spam.")
end
end

View File

@@ -1,6 +1,10 @@
# frozen_string_literal: true
class Dmail < ApplicationRecord
self.ignored_columns = [:creator_ip_addr]
attr_accessor :creator_ip_addr
validate :validate_sender_is_not_limited, on: :create
validates :title, presence: true, length: { maximum: 200 }, if: :title_changed?
validates :body, presence: true, length: { maximum: 50_000 }, if: :body_changed?
@@ -52,7 +56,7 @@ class Dmail < ApplicationRecord
end
def create_automated(params)
dmail = Dmail.new(from: User.system, creator_ip_addr: "127.0.0.1", **params)
dmail = Dmail.new(from: User.system, **params)
dmail.owner = dmail.to
dmail.save
dmail
@@ -171,7 +175,7 @@ class Dmail < ApplicationRecord
end
def autoreport_spam
if is_recipient? && !is_sender? && SpamDetector.new(self).spam?
if is_recipient? && !is_sender? && SpamDetector.new(self, user_ip: creator_ip_addr).spam?
self.is_deleted = true
moderation_reports << ModerationReport.new(creator: User.system, reason: "Spam.")
end

View File

@@ -2,6 +2,7 @@
class ForumPost < ApplicationRecord
attr_readonly :topic_id
attr_accessor :creator_ip_addr
belongs_to :creator, class_name: "User"
belongs_to_updater
@@ -97,7 +98,7 @@ class ForumPost < ApplicationRecord
end
def autoreport_spam
if SpamDetector.new(self, user_ip: CurrentUser.ip_addr).spam?
if SpamDetector.new(self, user_ip: creator_ip_addr).spam?
moderation_reports << ModerationReport.new(creator: User.system, reason: "Spam.")
end
end

View File

@@ -54,7 +54,7 @@ class Note < ApplicationRecord
end
end
def create_version(updater: CurrentUser.user, updater_ip_addr: CurrentUser.ip_addr)
def create_version(updater: CurrentUser.user)
return unless saved_change_to_versioned_attributes?
if merge_version?(updater.id)
@@ -62,7 +62,7 @@ class Note < ApplicationRecord
else
Note.where(:id => id).update_all("version = coalesce(version, 0) + 1")
reload
create_new_version(updater.id, updater_ip_addr)
create_new_version(updater.id)
end
end
@@ -70,10 +70,9 @@ class Note < ApplicationRecord
new_record? || saved_change_to_x? || saved_change_to_y? || saved_change_to_width? || saved_change_to_height? || saved_change_to_is_active? || saved_change_to_body?
end
def create_new_version(updater_id, updater_ip_addr)
def create_new_version(updater_id)
versions.create(
:updater_id => updater_id,
:updater_ip_addr => updater_ip_addr,
:post_id => post_id,
:x => x,
:y => y,

View File

@@ -1,6 +1,8 @@
# frozen_string_literal: true
class NoteVersion < ApplicationRecord
self.ignored_columns = [:updater_ip_addr]
belongs_to :post
belongs_to :note
belongs_to_updater :counter_cache => "note_update_count"

View File

@@ -220,9 +220,9 @@ class Pool < ApplicationRecord
post_count > 0 ? Post.find(post_ids.first) : nil
end
def create_version(updater: CurrentUser.user, updater_ip_addr: CurrentUser.ip_addr)
def create_version(updater: CurrentUser.user)
if PoolVersion.enabled?
PoolVersion.queue(self, updater, updater_ip_addr)
PoolVersion.queue(self, updater)
else
Rails.logger.warn("Archive service is not configured. Pool versions will not be saved.")
end

View File

@@ -1,6 +1,8 @@
# frozen_string_literal: true
class PoolVersion < ApplicationRecord
self.ignored_columns = [:updater_ip_addr]
belongs_to :updater, :class_name => "User"
belongs_to :pool
@@ -64,7 +66,7 @@ class PoolVersion < ApplicationRecord
SqsService.new(Danbooru.config.aws_sqs_archives_url)
end
def self.queue(pool, updater, updater_ip_addr)
def self.queue(pool, updater)
# queue updates to sqs so that if archives goes down for whatever reason it won't
# block pool updates
raise NotImplementedError, "Archive service is not configured." if !enabled?
@@ -73,7 +75,6 @@ class PoolVersion < ApplicationRecord
pool_id: pool.id,
post_ids: pool.post_ids,
updater_id: updater.id,
updater_ip_addr: updater_ip_addr.to_s,
created_at: pool.created_at.try(:iso8601),
updated_at: pool.updated_at.try(:iso8601),
description: pool.description,

View File

@@ -1,6 +1,8 @@
# frozen_string_literal: true
class Post < ApplicationRecord
self.ignored_columns = [:uploader_ip_addr]
class RevertError < StandardError; end
class DeletionError < StandardError; end
@@ -109,7 +111,6 @@ class Post < ApplicationRecord
post = Post.new(
uploader: upload.uploader,
uploader_ip_addr: upload.uploader_ip_addr,
md5: media_asset&.md5,
file_ext: media_asset&.file_ext,
file_size: media_asset&.file_size,
@@ -1420,7 +1421,7 @@ class Post < ApplicationRecord
:image_height, :tag_count, :has_children, :has_active_children,
:is_pending, :is_flagged, :is_deleted, :is_banned,
:last_comment_bumped_at, :last_commented_at, :last_noted_at,
:uploader_ip_addr, :uploader, :approver, :parent,
:uploader, :approver, :parent,
:artist_commentary, :flags, :appeals, :notes, :comments, :children,
:approvals, :replacements, :pixiv_ugoira_frame_data
)

View File

@@ -1,6 +1,8 @@
# frozen_string_literal: true
class PostVersion < ApplicationRecord
self.ignored_columns = [:updater_ip_addr]
class RevertError < StandardError; end
extend Memoist
@@ -91,7 +93,6 @@ class PostVersion < ApplicationRecord
"parent_id" => post.parent_id,
"source" => post.source,
"updater_id" => CurrentUser.id,
"updater_ip_addr" => CurrentUser.ip_addr.to_s,
"updated_at" => post.updated_at.try(:iso8601),
"created_at" => post.created_at.try(:iso8601),
"tags" => post.tag_string,

View File

@@ -1,6 +1,8 @@
# frozen_string_literal: true
class Upload < ApplicationRecord
self.ignored_columns = [:uploader_ip_addr]
extend Memoist
class Error < StandardError; end

View File

@@ -185,7 +185,6 @@ class WikiPage < ApplicationRecord
def create_new_version
versions.create(
:updater_id => CurrentUser.id,
:updater_ip_addr => CurrentUser.ip_addr,
:title => title,
:body => body,
:is_locked => is_locked,

View File

@@ -1,6 +1,8 @@
# frozen_string_literal: true
class WikiPageVersion < ApplicationRecord
self.ignored_columns = [:updater_ip_addr]
array_attribute :other_names
belongs_to :wiki_page
belongs_to_updater

View File

@@ -56,8 +56,7 @@ def populate_posts(n, search: "rating:s", batch_size: 200, timeout: 30.seconds)
posts.each do |danbooru_post|
Timeout.timeout(timeout) do
user = User.order("random()").first
ip_addr = FFaker::Internet.ip_v4_address
upload = Upload.create(uploader: user, uploader_ip_addr: ip_addr, source: danbooru_post["file_url"])
upload = Upload.create(uploader: user, source: danbooru_post["file_url"])
sleep 1 until upload.reload.is_finished? # wait for the job worker to process the upload in the background
post = Post.new_from_upload(upload.upload_media_assets.first, tag_string: danbooru_post["tag_string"], source: danbooru_post["source"], rating: danbooru_post["rating"])
@@ -77,8 +76,7 @@ def populate_comments(n)
n.times do |i|
user = User.order("random()").first
post = Post.order("random()").first
ip_addr = FFaker::Internet.ip_v4_address
comment = CurrentUser.scoped(user) { Comment.create(creator: user, creator_ip_addr: ip_addr, post: post, body: FFaker::Lorem.paragraph) }
comment = CurrentUser.scoped(user) { Comment.create(creator: user, post: post, body: FFaker::Lorem.paragraph) }
puts "Created comment ##{comment.id}"
end

View File

@@ -2,7 +2,6 @@ FactoryBot.define do
factory(:comment) do |f|
creator
post
creator_ip_addr { FFaker::Internet.ip_v4_address }
body {FFaker::Lorem.sentences.join(" ")}
end
end

View File

@@ -3,7 +3,6 @@ FactoryBot.define do
owner factory: :user
from factory: :user
to factory: :user
creator_ip_addr { FFaker::Internet.ip_v4_address }
title {FFaker::Lorem.words.join(" ")}
body {FFaker::Lorem.sentences.join(" ")}
end

View File

@@ -2,7 +2,6 @@ FactoryBot.define do
factory(:post) do
md5 { SecureRandom.hex(32) }
uploader
uploader_ip_addr {"127.0.0.1"}
tag_string {"tag1 tag2"}
tag_count {2}
tag_count_general {2}

View File

@@ -1,7 +1,6 @@
FactoryBot.define do
factory(:upload) do
uploader factory: :user
uploader_ip_addr { "127.0.0.1" }
status { "pending" }
source { "https://cdn.donmai.us/original/d3/4e/d34e4cf0a437a5d65f8e82b7bcd02606.jpg" }

View File

@@ -11,7 +11,7 @@ class SearchableTest < ActiveSupport::TestCase
subject { Post }
setup do
@p1 = create(:post, source: "a1", score: 1, is_deleted: true, uploader_ip_addr: "10.0.0.1")
@p1 = create(:post, source: "a1", score: 1, is_deleted: true)
@p2 = create(:post, source: "b2", score: 2, is_deleted: false)
@p3 = create(:post, source: "c3", score: 3, is_deleted: false)
end
@@ -108,11 +108,14 @@ class SearchableTest < ActiveSupport::TestCase
end
context "for an inet attribute" do
subject { UserSession }
should "work" do
assert_search_equals(@p1, uploader_ip_addr: "10.0.0.1")
assert_search_equals(@p1, uploader_ip_addr: "10.0.0.1/24")
assert_search_equals(@p1, uploader_ip_addr: "10.0.0.1,1.1.1.1")
assert_search_equals(@p1, uploader_ip_addr: "10.0.0.1 1.1.1.1")
@us = create(:user_session, ip_addr: "10.0.0.1")
assert_search_equals(@us, ip_addr: "10.0.0.1")
assert_search_equals(@us, ip_addr: "10.0.0.1/24")
assert_search_equals(@us, ip_addr: "10.0.0.1,1.1.1.1")
assert_search_equals(@us, ip_addr: "10.0.0.1 1.1.1.1")
end
end
@@ -258,8 +261,8 @@ class SearchableTest < ActiveSupport::TestCase
should "work" do
@media_asset = create(:media_asset)
@upload1 = create(:upload, media_assets: [@media_asset])
@upload2 = create(:upload, media_assets: [@media_asset])
@upload1 = create(:upload, upload_media_assets: [build(:upload_media_asset, media_asset: @media_asset)])
@upload2 = create(:upload, upload_media_assets: [build(:upload_media_asset, media_asset: @media_asset)])
assert_search_equals([@upload2, @upload1], media_asset: { md5: @media_asset.md5 })
end

View File

@@ -5,7 +5,6 @@ class DmailTest < ActiveSupport::TestCase
setup do
@user = FactoryBot.create(:user)
CurrentUser.user = @user
CurrentUser.ip_addr = "1.2.3.4"
end
teardown do
@@ -80,7 +79,7 @@ class DmailTest < ActiveSupport::TestCase
should "create a copy for each user" do
@new_user = FactoryBot.create(:user)
assert_difference("Dmail.count", 2) do
Dmail.create_split(from: CurrentUser.user, creator_ip_addr: "127.0.0.1", to: @new_user, title: "foo", body: "foo")
Dmail.create_split(from: CurrentUser.user, to: @new_user, title: "foo", body: "foo")
end
end
@@ -118,11 +117,11 @@ class DmailTest < ActiveSupport::TestCase
context "sending a dmail" do
should "fail if the user has sent too many dmails recently" do
10.times do
Dmail.create_split(from: @user, to: create(:user), title: "blah", body: "blah", creator_ip_addr: "127.0.0.1")
Dmail.create_split(from: @user, to: create(:user), title: "blah", body: "blah")
end
assert_no_difference("Dmail.count") do
@dmail = Dmail.create_split(from: @user, to: create(:user), title: "blah", body: "blah", creator_ip_addr: "127.0.0.1")
@dmail = Dmail.create_split(from: @user, to: create(:user), title: "blah", body: "blah")
assert_equal(false, @dmail.valid?)
assert_equal(["You can't send dmails to more than 10 users per hour"], @dmail.errors[:base])
@@ -133,7 +132,7 @@ class DmailTest < ActiveSupport::TestCase
context "destroying a dmail" do
setup do
@recipient = create(:user)
@dmail = Dmail.create_split(from: @user, to: @recipient, creator_ip_addr: "127.0.0.1", title: "foo", body: "foo")
@dmail = Dmail.create_split(from: @user, to: @recipient, title: "foo", body: "foo")
@modreport = create(:moderation_report, model: @dmail)
end

View File

@@ -5,12 +5,10 @@ class NoteTest < ActiveSupport::TestCase
setup do
@user = FactoryBot.create(:user)
CurrentUser.user = @user
CurrentUser.ip_addr = "127.0.0.1"
end
teardown do
CurrentUser.user = nil
CurrentUser.ip_addr = nil
end
context "#merge_version" do
@@ -77,7 +75,6 @@ class NoteTest < ActiveSupport::TestCase
assert_equal(1, @note.version)
assert_equal(1, @note.versions.first.version)
assert_equal(@user.id, @note.versions.first.updater_id)
assert_equal(CurrentUser.ip_addr, @note.versions.first.updater_ip_addr.to_s)
end
should "update the post's last_noted_at field" do
@@ -120,7 +117,6 @@ class NoteTest < ActiveSupport::TestCase
assert_equal("fafafa", @note.versions.last.body)
assert_equal(2, @note.version)
assert_equal(@user.id, @note.versions.last.updater_id)
assert_equal(CurrentUser.ip_addr, @note.versions.last.updater_ip_addr.to_s)
end
context "without making any changes" do