diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index 5a214a11d..31f671021 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -396,7 +396,7 @@ class TopicsController < ApplicationController end unless request.xhr? Scheduler::Defer.later "Track Visit" do - View.create_for_parent(Topic, topic_id, ip, user_id) + TopicViewItem.add(topic_id, ip, user_id) if track_visit TopicUser.track_visit! topic_id, user_id end diff --git a/app/models/incoming_link.rb b/app/models/incoming_link.rb index 7fed7c746..232bbdb1e 100644 --- a/app/models/incoming_link.rb +++ b/app/models/incoming_link.rb @@ -115,7 +115,6 @@ end # Table name: incoming_links # # id :integer not null, primary key -# topic_id :integer # created_at :datetime # user_id :integer # ip_address :inet diff --git a/app/models/incoming_referer.rb b/app/models/incoming_referer.rb index 8d714b7ff..450e54f8c 100644 --- a/app/models/incoming_referer.rb +++ b/app/models/incoming_referer.rb @@ -24,7 +24,6 @@ end # Table name: incoming_referers # # id :integer not null, primary key -# url :string(1000) not null # path :string(1000) not null # incoming_domain_id :integer not null # diff --git a/app/models/leader_requirements.rb b/app/models/leader_requirements.rb index 5c71a5a02..6fe689b99 100644 --- a/app/models/leader_requirements.rb +++ b/app/models/leader_requirements.rb @@ -59,7 +59,7 @@ class LeaderRequirements end def topics_viewed_query - View.where(user_id: @user.id, parent_type: 'Topic').select('distinct(parent_id)') + TopicViewItem.where(user_id: @user.id).select('topic_id') end def topics_viewed diff --git a/app/models/top_topic.rb b/app/models/top_topic.rb index 93c10e2d8..39bbe91b1 100644 --- a/app/models/top_topic.rb +++ b/app/models/top_topic.rb @@ -92,8 +92,8 @@ class TopTopic < ActiveRecord::Base end def self.update_views_count_for(period) - sql = "SELECT parent_id as topic_id, COUNT(*) AS count - FROM views + sql = "SELECT topic_id, COUNT(*) AS count + FROM topic_views WHERE viewed_at >= :from GROUP BY topic_id" diff --git a/app/models/topic_view_item.rb b/app/models/topic_view_item.rb new file mode 100644 index 000000000..a3bbef6d0 --- /dev/null +++ b/app/models/topic_view_item.rb @@ -0,0 +1,50 @@ +require 'ipaddr' + +# awkward TopicView is taken +class TopicViewItem < ActiveRecord::Base + self.table_name = 'topic_views' + belongs_to :user + validates_presence_of :topic_id, :ip_address, :viewed_at + + def self.add(topic_id, ip, user_id, at=nil) + # Only store a view once per day per thing per user per ip + redis_key = "view:#{topic_id}:#{Date.today.to_s}" + if user_id + redis_key << ":user-#{user_id}" + else + redis_key << ":ip-#{ip}" + end + + if $redis.setnx(redis_key, "1") + $redis.expire(redis_key, 1.day.to_i) + + TopicViewItem.transaction do + at ||= Date.today + TopicViewItem.create!(topic_id: topic_id, ip_address: ip, viewed_at: at, user_id: user_id) + + # Update the views count in the parent, if it exists. + Topic.where(id: topic_id).update_all 'views = views + 1' + end + end + rescue ActiveRecord::RecordNotUnique + # don't care, skip + end + +end + +# == Schema Information +# +# Table name: topic_views +# +# topic_id :integer not null +# viewed_at :date not null +# user_id :integer +# ip_address :inet not null +# +# Indexes +# +# index_topic_views_on_topic_id_and_viewed_at (topic_id,viewed_at) +# index_topic_views_on_viewed_at_and_topic_id (viewed_at,topic_id) +# ip_address_topic_id_topic_views (ip_address,topic_id) UNIQUE +# user_id_topic_id_topic_views (user_id,topic_id) UNIQUE +# diff --git a/app/models/user.rb b/app/models/user.rb index 988e3ea02..3e0d634d2 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -86,7 +86,7 @@ class User < ActiveRecord::Base before_destroy do # These tables don't have primary keys, so destroying them with activerecord is tricky: PostTiming.delete_all(user_id: self.id) - View.delete_all(user_id: self.id) + TopicViewItem.delete_all(user_id: self.id) end # Whether we need to be sending a system message after creation diff --git a/app/models/user_stat.rb b/app/models/user_stat.rb index 5453db910..df032ab90 100644 --- a/app/models/user_stat.rb +++ b/app/models/user_stat.rb @@ -13,10 +13,9 @@ class UserStat < ActiveRecord::Base # Update denormalized topics_entered exec_sql "UPDATE user_stats SET topics_entered = X.c FROM - (SELECT v.user_id, - COUNT(DISTINCT parent_id) AS c - FROM views AS v - WHERE parent_type = 'Topic' AND v.user_id IN ( + (SELECT v.user_id, COUNT(topic_id) AS c + FROM topic_views AS v + WHERE v.user_id IN ( SELECT u1.id FROM users u1 where u1.last_seen_at > :seen_at ) GROUP BY v.user_id) AS X diff --git a/app/models/view.rb b/app/models/view.rb deleted file mode 100644 index 8033d03a2..000000000 --- a/app/models/view.rb +++ /dev/null @@ -1,51 +0,0 @@ -require 'ipaddr' - -class View < ActiveRecord::Base - belongs_to :parent, polymorphic: true - belongs_to :user - validates_presence_of :parent_type, :parent_id, :ip_address, :viewed_at - - def self.create_for_parent(parent_class, parent_id, ip, user_id) - # Only store a view once per day per thing per user per ip - redis_key = "view:#{parent_class.name}:#{parent_id}:#{Date.today.to_s}" - if user_id - redis_key << ":user-#{user_id}" - else - redis_key << ":ip-#{ip}" - end - - if $redis.setnx(redis_key, "1") - $redis.expire(redis_key, 1.day.to_i) - - View.transaction do - View.create!(parent_id: parent_id, parent_type: parent_class.to_s, ip_address: ip, viewed_at: Date.today, user_id: user_id) - - # Update the views count in the parent, if it exists. - if parent_class.columns_hash["views"] - parent_class.where(id: parent_id).update_all 'views = views + 1' - end - end - end - end - - def self.create_for(parent, ip, user=nil) - user_id = user.id if user - create_for_parent(parent.class, parent.id, ip, user_id) - end -end - -# == Schema Information -# -# Table name: views -# -# parent_id :integer not null -# parent_type :string(50) not null -# viewed_at :date not null -# user_id :integer -# ip_address :inet not null -# -# Indexes -# -# index_views_on_parent_id_and_parent_type (parent_id,parent_type) -# index_views_on_user_id_and_parent_type_and_parent_id (user_id,parent_type,parent_id) -# diff --git a/db/migrate/20140804072504_views_to_topic_views.rb b/db/migrate/20140804072504_views_to_topic_views.rb new file mode 100644 index 000000000..16e78bc94 --- /dev/null +++ b/db/migrate/20140804072504_views_to_topic_views.rb @@ -0,0 +1,11 @@ +class ViewsToTopicViews < ActiveRecord::Migration + def change + remove_column :views, :parent_type + rename_column :views, :parent_id, :topic_id + + rename_table :views, :topic_views + + add_index :topic_views, [:topic_id] + add_index :topic_views, [:user_id, :topic_id] + end +end diff --git a/db/migrate/20140804075613_normalize_topic_view_data_and_index.rb b/db/migrate/20140804075613_normalize_topic_view_data_and_index.rb new file mode 100644 index 000000000..4f6e6f1b2 --- /dev/null +++ b/db/migrate/20140804075613_normalize_topic_view_data_and_index.rb @@ -0,0 +1,39 @@ +class NormalizeTopicViewDataAndIndex < ActiveRecord::Migration + def change + remove_index :topic_views, [:topic_id] + remove_index :topic_views, [:user_id, :topic_id] + + execute 'CREATE TEMPORARY TABLE tmp_views_user(user_id int, topic_id int, viewed_at date, ip_address inet)' + + execute 'INSERT INTO tmp_views_user(user_id, topic_id, ip_address, viewed_at) + SELECT user_id, topic_id, min(ip_address::varchar)::inet, min(viewed_at) + FROM topic_views + WHERE user_id IS NOT NULL + GROUP BY user_id, topic_id + ' + + execute 'CREATE TEMPORARY TABLE tmp_views_ip(topic_id int, viewed_at date, ip_address inet)' + + execute 'INSERT INTO tmp_views_ip(topic_id, ip_address, viewed_at) + SELECT topic_id, ip_address, min(viewed_at) + FROM topic_views + WHERE user_id IS NULL + GROUP BY user_id, topic_id, ip_address + ' + + execute 'truncate table topic_views' + + execute 'INSERT INTO topic_views(user_id, topic_id, ip_address, viewed_at) + SELECT user_id, topic_id, ip_address, viewed_at FROM tmp_views_user + UNION ALL + SELECT NULL, topic_id, ip_address, viewed_at FROM tmp_views_ip + ' + + + execute 'CREATE UNIQUE INDEX user_id_topic_id_topic_views ON topic_views(user_id, topic_id) WHERE user_id IS NOT NULL' + execute 'CREATE UNIQUE INDEX ip_address_topic_id_topic_views ON topic_views(ip_address, topic_id) WHERE user_id IS NULL' + + add_index :topic_views, [:topic_id, :viewed_at] + add_index :topic_views, [:viewed_at, :topic_id] + end +end diff --git a/spec/controllers/topics_controller_spec.rb b/spec/controllers/topics_controller_spec.rb index 3b20a8adf..88b46af2d 100644 --- a/spec/controllers/topics_controller_spec.rb +++ b/spec/controllers/topics_controller_spec.rb @@ -587,7 +587,7 @@ describe TopicsController do end it 'records a view' do - lambda { xhr :get, :show, topic_id: topic.id, slug: topic.slug }.should change(View, :count).by(1) + lambda { xhr :get, :show, topic_id: topic.id, slug: topic.slug }.should change(TopicViewItem, :count).by(1) end it 'records incoming links' do diff --git a/spec/models/leader_requirements_spec.rb b/spec/models/leader_requirements_spec.rb index 2dfe679d5..3f24ea4e0 100644 --- a/spec/models/leader_requirements_spec.rb +++ b/spec/models/leader_requirements_spec.rb @@ -10,7 +10,7 @@ describe LeaderRequirements do end def make_view(id, at, user_id) - View.create!(parent_id: id, parent_type: 'Topic', ip_address: '11.22.33.44', viewed_at: at, user_id: user_id) + TopicViewItem.add(id, '11.22.33.44', user_id, at) end describe "requirements" do diff --git a/spec/models/topic_view_item_spec.rb b/spec/models/topic_view_item_spec.rb new file mode 100644 index 000000000..d6ae9531f --- /dev/null +++ b/spec/models/topic_view_item_spec.rb @@ -0,0 +1,5 @@ +require 'spec_helper' + +describe TopicView do + +end diff --git a/spec/models/user_stat_spec.rb b/spec/models/user_stat_spec.rb index a0fdf465d..bf402857d 100644 --- a/spec/models/user_stat_spec.rb +++ b/spec/models/user_stat_spec.rb @@ -26,7 +26,7 @@ describe UserStat do context 'with a view' do let(:topic) { Fabricate(:topic) } - let!(:view) { View.create_for(topic, '127.0.0.1', user) } + let!(:view) { TopicViewItem.add(topic.id, '127.0.0.1', user.id) } before do user.update_column :last_seen_at, 1.second.ago @@ -39,7 +39,7 @@ describe UserStat do end it "won't record a second view as a different topic" do - View.create_for(topic, '127.0.0.1', user) + TopicViewItem.add(topic.id, '127.0.0.1', user.id) UserStat.update_view_counts stat.reload stat.topics_entered.should == 1 diff --git a/spec/models/view_spec.rb b/spec/models/view_spec.rb deleted file mode 100644 index b4e0052db..000000000 --- a/spec/models/view_spec.rb +++ /dev/null @@ -1,13 +0,0 @@ -require 'spec_helper' - -describe View do - - it { should belong_to :parent } - 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_address } - it { should validate_presence_of :viewed_at } - - -end