From 2e478d8537de717595193b5534476c131fc83e19 Mon Sep 17 00:00:00 2001
From: Dan Johnson <ComputerDruid@gmail.com>
Date: Mon, 24 Jun 2013 18:30:32 -0400
Subject: [PATCH 1/3] TopicLinkClick: convert 'ip' (bigint) -> 'ip_address'
(inet)
When accessed over IPv6, the ip address of the user is a 128-bit number,
too big for PostgreSQL's bigint data type. Since PostgresSQL has the
built-in inet type, which handles both IPv4 and IPv6 addresses, we
should use that instead. Where this is done elsewhere in the codebase,
the column is called ip_address, so we should follow that convention as
well.
This migration uses a SQL command to populate the new field from the old
one, so as not to rely on the TopicLinkClick model class, which should
keep the migration from failing if that class is modified in the future.
---
app/models/topic_link_click.rb | 8 +++----
..._change_ip_to_inet_in_topic_link_clicks.rb | 21 +++++++++++++++++++
spec/models/topic_link_click_spec.rb | 4 ++--
spec/models/topic_link_spec.rb | 2 +-
4 files changed, 27 insertions(+), 8 deletions(-)
create mode 100644 db/migrate/20130624203206_change_ip_to_inet_in_topic_link_clicks.rb
diff --git a/app/models/topic_link_click.rb b/app/models/topic_link_click.rb
index 560450525..174122b6c 100644
--- a/app/models/topic_link_click.rb
+++ b/app/models/topic_link_click.rb
@@ -5,10 +5,8 @@ class TopicLinkClick < ActiveRecord::Base
belongs_to :topic_link, counter_cache: :clicks
belongs_to :user
- has_ip_address :ip
-
validates_presence_of :topic_link_id
- validates_presence_of :ip
+ validates_presence_of :ip_address
# Create a click from a URL and post_id
def self.create_from(args={})
@@ -28,7 +26,7 @@ class TopicLinkClick < ActiveRecord::Base
rate_key = "link-clicks:#{link.id}:#{args[:user_id] || args[:ip]}"
if $redis.setnx(rate_key, "1")
$redis.expire(rate_key, 1.day.to_i)
- create!(topic_link_id: link.id, user_id: args[:user_id], ip: args[:ip])
+ create!(topic_link_id: link.id, user_id: args[:user_id], ip_address: args[:ip])
end
args[:url]
@@ -43,9 +41,9 @@ end
# id :integer not null, primary key
# topic_link_id :integer not null
# user_id :integer
-# ip :integer not null
# created_at :datetime not null
# updated_at :datetime not null
+# ip_address :string not null
#
# Indexes
#
diff --git a/db/migrate/20130624203206_change_ip_to_inet_in_topic_link_clicks.rb b/db/migrate/20130624203206_change_ip_to_inet_in_topic_link_clicks.rb
new file mode 100644
index 000000000..a0c3924bf
--- /dev/null
+++ b/db/migrate/20130624203206_change_ip_to_inet_in_topic_link_clicks.rb
@@ -0,0 +1,21 @@
+require 'ipaddr'
+
+class ChangeIpToInetInTopicLinkClicks < ActiveRecord::Migration
+ def up
+ add_column :topic_link_clicks, :ip_address, :inet
+
+ execute "UPDATE topic_link_clicks SET ip_address = inet(
+ (ip >> 24 & 255) || '.' ||
+ (ip >> 16 & 255) || '.' ||
+ (ip >> 8 & 255) || '.' ||
+ (ip >> 0 & 255)
+ );"
+
+ change_column :topic_link_clicks, :ip_address, :inet, { :null => false }
+ remove_column :topic_link_clicks, :ip
+ end
+
+ def down
+ raise ActiveRecord::IrreversibleMigration
+ end
+end
diff --git a/spec/models/topic_link_click_spec.rb b/spec/models/topic_link_click_spec.rb
index fce47cb59..1580de1ac 100644
--- a/spec/models/topic_link_click_spec.rb
+++ b/spec/models/topic_link_click_spec.rb
@@ -26,7 +26,7 @@ describe TopicLinkClick do
context 'create' do
before do
- TopicLinkClick.create(topic_link: @topic_link, ip: '192.168.1.1')
+ TopicLinkClick.create(topic_link: @topic_link, ip_address: '192.168.1.1')
end
it 'creates the forum topic link click' do
@@ -39,7 +39,7 @@ describe TopicLinkClick do
end
it 'serializes and deserializes the IP' do
- TopicLinkClick.first.ip.to_s.should == '192.168.1.1'
+ TopicLinkClick.first.ip_address.to_s.should == '192.168.1.1'
end
end
diff --git a/spec/models/topic_link_spec.rb b/spec/models/topic_link_spec.rb
index 820d45b59..29d7422a2 100644
--- a/spec/models/topic_link_spec.rb
+++ b/spec/models/topic_link_spec.rb
@@ -236,7 +236,7 @@ describe TopicLink do
it 'has the correct results' do
TopicLink.extract_from(post)
topic_link = post.topic.topic_links.first
- TopicLinkClick.create(topic_link: topic_link, ip: '192.168.1.1')
+ TopicLinkClick.create(topic_link: topic_link, ip_address: '192.168.1.1')
counts_for[post.id].should be_present
counts_for[post.id].find {|l| l[:url] == 'http://google.com'}[:clicks].should == 0
From 9f6b7889a8b516a9720e26496ae8235538166dce Mon Sep 17 00:00:00 2001
From: Dan Johnson <ComputerDruid@gmail.com>
Date: Tue, 25 Jun 2013 00:18:54 -0400
Subject: [PATCH 2/3] views: convert 'ip' (bigint) -> 'ip_address' (inet)
This fixes all known issues when connecting to discourse over IPv6.
This table has no primary key, so the migration is done with update_all,
for each ip address in the views table. Since this table can potentially
grow quite large, this process might take a long time. I don't know any
way around this, though.
This migration uses a SQL command to populate the new field from the old
one, so as not to rely on the View model class, which should keep the
migration from failing if that class is modified in the future.
---
app/models/view.rb | 6 ++---
...130625022454_change_ip_to_inet_in_views.rb | 22 +++++++++++++++++++
spec/models/view_spec.rb | 2 +-
3 files changed, 26 insertions(+), 4 deletions(-)
create mode 100644 db/migrate/20130625022454_change_ip_to_inet_in_views.rb
diff --git a/app/models/view.rb b/app/models/view.rb
index d7e29c2f2..efe6423e5 100644
--- a/app/models/view.rb
+++ b/app/models/view.rb
@@ -3,7 +3,7 @@ require 'ipaddr'
class View < ActiveRecord::Base
belongs_to :parent, polymorphic: true
belongs_to :user
- validates_presence_of :parent_type, :parent_id, :ip, :viewed_at
+ validates_presence_of :parent_type, :parent_id, :ip_address, :viewed_at
# TODO: This could happen asyncronously
def self.create_for(parent, ip, user=nil)
@@ -20,7 +20,7 @@ class View < ActiveRecord::Base
$redis.expire(redis_key, 1.day.to_i)
View.transaction do
- View.create(parent: parent, ip: IPAddr.new(ip).to_i, viewed_at: Date.today, user: user)
+ View.create(parent: parent, ip_address: ip, viewed_at: Date.today, user: user)
# Update the views count in the parent, if it exists.
if parent.respond_to?(:views)
@@ -37,9 +37,9 @@ end
#
# parent_id :integer not null
# parent_type :string(50) not null
-# ip :integer not null
# viewed_at :date not null
# user_id :integer
+# ip_address :string not null
#
# Indexes
#
diff --git a/db/migrate/20130625022454_change_ip_to_inet_in_views.rb b/db/migrate/20130625022454_change_ip_to_inet_in_views.rb
new file mode 100644
index 000000000..30a0e8390
--- /dev/null
+++ b/db/migrate/20130625022454_change_ip_to_inet_in_views.rb
@@ -0,0 +1,22 @@
+require 'ipaddr'
+
+class ChangeIpToInetInViews < ActiveRecord::Migration
+ def up
+ table = :views
+ add_column table, :ip_address, :inet
+
+ execute "UPDATE views SET ip_address = inet(
+ (ip >> 24 & 255) || '.' ||
+ (ip >> 16 & 255) || '.' ||
+ (ip >> 8 & 255) || '.' ||
+ (ip >> 0 & 255)
+ );"
+
+ change_column table, :ip_address, :inet, { :null => false }
+ remove_column table, :ip
+ end
+
+ def down
+ raise ActiveRecord::IrreversibleMigration
+ end
+end
diff --git a/spec/models/view_spec.rb b/spec/models/view_spec.rb
index 6a2ac8d9f..b4e0052db 100644
--- a/spec/models/view_spec.rb
+++ b/spec/models/view_spec.rb
@@ -6,7 +6,7 @@ describe View do
it { should belong_to :user }
it { should validate_presence_of :parent_type }
it { should validate_presence_of :parent_id }
- it { should validate_presence_of :ip }
+ it { should validate_presence_of :ip_address }
it { should validate_presence_of :viewed_at }
From 98f926f193a75137b327630922f33193da51db4a Mon Sep 17 00:00:00 2001
From: Dan Johnson <ComputerDruid@gmail.com>
Date: Tue, 25 Jun 2013 00:31:11 -0400
Subject: [PATCH 3/3] remove unneeded has_ip_address gem
---
Gemfile | 1 -
Gemfile.lock | 2 --
docs/SOFTWARE.md | 1 -
3 files changed, 4 deletions(-)
diff --git a/Gemfile b/Gemfile
index 454b161c2..6fd9de1aa 100644
--- a/Gemfile
+++ b/Gemfile
@@ -26,7 +26,6 @@ gem 'fast_xs'
gem 'fast_xor', git: 'https://github.com/CodeMonkeySteve/fast_xor.git'
gem 'fastimage'
gem 'fog', require: false
-gem 'has_ip_address'
gem 'hiredis'
gem 'email_reply_parser'
diff --git a/Gemfile.lock b/Gemfile.lock
index a5a7f415d..ccb1b7428 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -207,7 +207,6 @@ GEM
guard (>= 1.1)
spork (>= 0.8.4)
handlebars-source (1.0.0.rc4)
- has_ip_address (0.0.1)
hashie (2.0.4)
highline (1.6.18)
hike (1.2.2)
@@ -484,7 +483,6 @@ DEPENDENCIES
guard-rspec
guard-spork
handlebars-source (= 1.0.0.rc4)
- has_ip_address
highline
hiredis
image_optim
diff --git a/docs/SOFTWARE.md b/docs/SOFTWARE.md
index ba1a139ac..54f26c0c6 100644
--- a/docs/SOFTWARE.md
+++ b/docs/SOFTWARE.md
@@ -27,7 +27,6 @@ The following Ruby Gems are used in Discourse:
* [omniauth-facebook](https://github.com/mkdynamic/omniauth-facebook)
* [omniauth-twitter](https://github.com/arunagw/omniauth-twitter)
* [omniauth-github](https://github.com/intridea/omniauth-github)
-* [has_ip_address](https://rubygems.org/gems/has_ip_address)
* [vestal_versions](https://rubygems.org/gems/vestal_versions)
* [uglifier](https://rubygems.org/gems/uglifier)
* [nokogiri](https://rubygems.org/gems/nokogiri)