From a1835ae5b9b2dcfb1c3bb2f115d89b2b1b274636 Mon Sep 17 00:00:00 2001 From: rhymes Date: Mon, 3 Feb 2020 16:35:08 +0100 Subject: [PATCH] User counters using PostgreSQL JSON (#5373) [deploy] Co-authored-by: Ben Halpern --- app/models/user.rb | 11 ++++++ app/models/user_counter.rb | 11 ++++++ app/serializers/hash_serializer.rb | 9 +++++ .../20200106074859_create_user_counters.rb | 14 +++++++ db/schema.rb | 11 +++++- lib/tasks/counters.rake | 19 +++++++++ spec/factories/user_counters.rb | 6 +++ spec/models/user_counter_spec.rb | 39 +++++++++++++++++++ spec/models/user_spec.rb | 1 + 9 files changed, 120 insertions(+), 1 deletion(-) create mode 100644 app/models/user_counter.rb create mode 100644 app/serializers/hash_serializer.rb create mode 100644 db/migrate/20200106074859_create_user_counters.rb create mode 100644 lib/tasks/counters.rake create mode 100644 spec/factories/user_counters.rb create mode 100644 spec/models/user_counter_spec.rb diff --git a/app/models/user.rb b/app/models/user.rb index 1fb0c97a9..53d0e98e3 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -55,6 +55,7 @@ class User < ApplicationRecord has_many :blocker_blocks, class_name: "UserBlock", foreign_key: :blocker_id, inverse_of: :blocker, dependent: :delete_all has_many :blocked_blocks, class_name: "UserBlock", foreign_key: :blocked_id, inverse_of: :blocked, dependent: :delete_all has_one :pro_membership, dependent: :destroy + has_one :counters, class_name: "UserCounter", dependent: :destroy has_many :created_podcasts, class_name: "Podcast", foreign_key: :creator_id, inverse_of: :creator, dependent: :nullify mount_uploader :profile_image, ProfileImageUploader @@ -155,6 +156,16 @@ class User < ApplicationRecord scope :dev_account, -> { find_by(id: SiteConfig.staff_user_id) } + scope :with_this_week_comments, lambda { |number| + includes(:counters).joins(:counters).where("(user_counters.data -> 'comments_these_7_days')::int >= ?", number) + } + scope :with_previous_week_comments, lambda { |number| + includes(:counters).joins(:counters).where("(user_counters.data -> 'comments_prior_7_days')::int >= ?", number) + } + scope :top_commenters, lambda { |number = 10| + includes(:counters).order(Arel.sql("user_counters.data -> 'comments_these_7_days' DESC")).limit(number) + } + after_create_commit :send_welcome_notification after_save :bust_cache after_save :subscribe_to_mailchimp_newsletter diff --git a/app/models/user_counter.rb b/app/models/user_counter.rb new file mode 100644 index 000000000..05008d0a7 --- /dev/null +++ b/app/models/user_counter.rb @@ -0,0 +1,11 @@ +class UserCounter < ApplicationRecord + belongs_to :user + + serialize :data, HashSerializer + store_accessor :data, :comments_these_7_days, :comments_prior_7_days + + validates :user, presence: true, uniqueness: true + + validates :comments_these_7_days, numericality: { only_integer: true } + validates :comments_prior_7_days, numericality: { only_integer: true } +end diff --git a/app/serializers/hash_serializer.rb b/app/serializers/hash_serializer.rb new file mode 100644 index 000000000..ee82af608 --- /dev/null +++ b/app/serializers/hash_serializer.rb @@ -0,0 +1,9 @@ +class HashSerializer + def self.dump(hash) + hash + end + + def self.load(hash) + (hash || {}).with_indifferent_access + end +end diff --git a/db/migrate/20200106074859_create_user_counters.rb b/db/migrate/20200106074859_create_user_counters.rb new file mode 100644 index 000000000..69a268a3d --- /dev/null +++ b/db/migrate/20200106074859_create_user_counters.rb @@ -0,0 +1,14 @@ +class CreateUserCounters < ActiveRecord::Migration[5.2] + def change + create_table :user_counters do |t| + t.references :user, index: { unique: true } + t.jsonb :data, null: false, default: {} + + t.timestamps null: false + end + + add_foreign_key :user_counters, :users, on_delete: :cascade + + add_index :user_counters, :data, using: :gin + end +end diff --git a/db/schema.rb b/db/schema.rb index 7cc8c8c8b..3dfdc8e43 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,6 @@ # It's strongly recommended that you check this file into your version control system. ActiveRecord::Schema.define(version: 2020_01_20_053525) do - # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -1018,6 +1017,15 @@ ActiveRecord::Schema.define(version: 2020_01_20_053525) do t.index ["blocked_id", "blocker_id"], name: "index_user_blocks_on_blocked_id_and_blocker_id", unique: true end + create_table "user_counters", force: :cascade do |t| + t.datetime "created_at", null: false + t.jsonb "data", default: {}, null: false + t.datetime "updated_at", null: false + t.bigint "user_id" + t.index ["data"], name: "index_user_counters_on_data", using: :gin + t.index ["user_id"], name: "index_user_counters_on_user_id", unique: true + end + create_table "users", id: :serial, force: :cascade do |t| t.integer "articles_count", default: 0, null: false t.string "available_for" @@ -1224,6 +1232,7 @@ ActiveRecord::Schema.define(version: 2020_01_20_053525) do add_foreign_key "tag_adjustments", "users", on_delete: :cascade add_foreign_key "user_blocks", "users", column: "blocked_id" add_foreign_key "user_blocks", "users", column: "blocker_id" + add_foreign_key "user_counters", "users", on_delete: :cascade add_foreign_key "users_roles", "users", on_delete: :cascade add_foreign_key "webhook_endpoints", "oauth_applications" add_foreign_key "webhook_endpoints", "users" diff --git a/lib/tasks/counters.rake b/lib/tasks/counters.rake new file mode 100644 index 000000000..5d085001c --- /dev/null +++ b/lib/tasks/counters.rake @@ -0,0 +1,19 @@ +namespace :counters do + task update_users: :environment do + ActiveRecord::Base.transaction do + # NOTE: we could bypass Rails by using SQL directly to compute the counts and update these columns + User.includes(:counters).find_each do |user| + user.build_counters unless user.counters + + relation = user.comments + + user.counters.comments_these_7_days = relation.where("created_at > ?", 7.days.ago).size + user.counters.comments_prior_7_days = relation. + where("created_at > ? AND created_at < ?", 14.days.ago, 7.days.ago). + size + + user.counters.save! + end + end + end +end diff --git a/spec/factories/user_counters.rb b/spec/factories/user_counters.rb new file mode 100644 index 000000000..ef940fa57 --- /dev/null +++ b/spec/factories/user_counters.rb @@ -0,0 +1,6 @@ +FactoryBot.define do + factory :user_counter do + user + data { { comments_these_7_days: 0, comments_prior_7_days: 0 } } + end +end diff --git a/spec/models/user_counter_spec.rb b/spec/models/user_counter_spec.rb new file mode 100644 index 000000000..6cc7c3dbb --- /dev/null +++ b/spec/models/user_counter_spec.rb @@ -0,0 +1,39 @@ +require "rails_helper" + +RSpec.describe UserCounter, type: :model do + let(:counters) { build(:user_counter) } + + describe "validations" do + describe "builtin validations" do + subject { build(:user_counter, user: create(:user)) } + + it { is_expected.to belong_to(:user) } + it { is_expected.to validate_presence_of(:user) } + it { is_expected.to validate_uniqueness_of(:user) } + end + + describe "#comments_these_7_days" do + it "is valid if comments_these_7_days is an integer" do + counters.comments_these_7_days = 1 + expect(counters).to be_valid + end + + it "is is not if comments_these_7_days is not an integer" do + counters.comments_these_7_days = 1.2 + expect(counters).not_to be_valid + end + end + + describe "#comments_prior_7_days" do + it "is valid if comments_prior_7_days is an integer" do + counters.comments_prior_7_days = 1 + expect(counters).to be_valid + end + + it "is is not if comments_prior_7_days is not an integer" do + counters.comments_prior_7_days = 1.2 + expect(counters).not_to be_valid + end + end + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 24ad7bcbf..643ea712f 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -33,6 +33,7 @@ RSpec.describe User, type: :model do it { is_expected.to have_many(:chat_channels).through(:chat_channel_memberships) } it { is_expected.to have_many(:notification_subscriptions).dependent(:destroy) } it { is_expected.to have_one(:pro_membership).dependent(:destroy) } + it { is_expected.to have_one(:counters).dependent(:destroy) } # rubocop:disable RSpec/NamedSubject it "has created_podcasts" do