diff --git a/app/models/message_bus_observer.rb b/app/models/message_bus_observer.rb deleted file mode 100644 index cd851bfc7c4..00000000000 --- a/app/models/message_bus_observer.rb +++ /dev/null @@ -1,49 +0,0 @@ -require_dependency 'discourse_observer' - -# This class is responsible for notifying the message bus of various -# events. -class MessageBusObserver < DiscourseObserver - observe :notification, :user_action, :topic - - def after_create_notification(notification) - refresh_notification_count(notification) - end - - def after_destroy_notification(notification) - refresh_notification_count(notification) - end - - def after_create_user_action(user_action) - MessageBus.publish("/users/#{user_action.user.username.downcase}", user_action.id) - end - - def after_create_topic(topic) - - # Don't publish invisible topics - return unless topic.visible? - - return if topic.private_message? - - topic.posters = topic.posters_summary - topic.posts_count = 1 - topic_json = TopicListItemSerializer.new(topic).as_json - MessageBus.publish("/latest", topic_json) - - # If it has a category, add it to the category views too - if topic.category.present? - MessageBus.publish("/category/#{topic.category.slug}", topic_json) - end - - end - - protected - - def refresh_notification_count(notification) - user_id = notification.user.id - MessageBus.publish("/notification/#{user_id}", - {unread_notifications: notification.user.unread_notifications, - unread_private_messages: notification.user.unread_private_messages}, - user_ids: [user_id] # only publish the notification to this user - ) - end -end diff --git a/app/models/notification.rb b/app/models/notification.rb index a81bf397f5f..d2142c592a1 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -10,6 +10,9 @@ class Notification < ActiveRecord::Base scope :unread, lambda { where(read: false) } scope :recent, lambda { order('created_at desc').limit(10) } + after_save :refresh_notification_count + after_destroy :refresh_notification_count + def self.types @types ||= Enum.new( :mentioned, :replied, :quoted, :edited, :liked, :private_message, @@ -74,5 +77,18 @@ class Notification < ActiveRecord::Base Post.where(topic_id: topic_id, post_number: post_number).first end + + + protected + + def refresh_notification_count + user_id = user.id + MessageBus.publish("/notification/#{user_id}", + {unread_notifications: user.unread_notifications, + unread_private_messages: user.unread_private_messages}, + user_ids: [user_id] # only publish the notification to this user + ) + end + end diff --git a/app/models/user_action.rb b/app/models/user_action.rb index 20299bf79ab..dd12e57879c 100644 --- a/app/models/user_action.rb +++ b/app/models/user_action.rb @@ -150,6 +150,19 @@ LEFT JOIN categories c on c.id = t.category_id User.update_all('likes_received = likes_received + 1', id: user_id) end + topic = Topic.includes(:category).where(id: hash[:target_topic_id]).first + + # move into Topic perhaps + group_ids = nil + if topic && topic.category && topic.category.secure + group_ids = topic.category.groups.select("groups.id").map{|g| g.id} + end + + MessageBus.publish("/users/#{action.user.username.downcase}", + action.id, + user_ids: [user_id], + group_ids: group_ids ) + rescue ActiveRecord::RecordNotUnique # can happen, don't care already logged raise ActiveRecord::Rollback diff --git a/config/application.rb b/config/application.rb index a58589a6baf..bcc6f311ad4 100644 --- a/config/application.rb +++ b/config/application.rb @@ -55,7 +55,6 @@ module Discourse config.active_record.observers = [ :user_email_observer, :user_action_observer, - :message_bus_observer, :post_alert_observer, :search_observer ] diff --git a/config/initializers/04-message_bus.rb b/config/initializers/04-message_bus.rb index 67b0e27c2cd..efc333f2d62 100644 --- a/config/initializers/04-message_bus.rb +++ b/config/initializers/04-message_bus.rb @@ -7,6 +7,11 @@ MessageBus.user_id_lookup do |env| user.id if user end +MessageBus.group_ids_lookup do |env| + user = CurrentUser.lookup_from_env(env) + user.groups.select('groups.id').map{|g| g.id} if user +end + MessageBus.on_connect do |site_id| RailsMultisite::ConnectionManagement.establish_connection(db: site_id) end diff --git a/lib/autospec/runner.rb b/lib/autospec/runner.rb index f73e23db510..39dfb85df13 100644 --- a/lib/autospec/runner.rb +++ b/lib/autospec/runner.rb @@ -12,7 +12,6 @@ class Autospec::Runner watch(%r{^spec/.+_spec\.rb$}) watch(%r{^lib/(.+)\.rb$}) { |m| "spec/components/#{m[1]}_spec.rb" } - watch('spec/spec_helper.rb') { "spec" } # Rails example watch(%r{^app/(.+)\.rb$}) { |m| "spec/#{m[1]}_spec.rb" } @@ -24,6 +23,14 @@ class Autospec::Runner # Capybara request specs watch(%r{^app/views/(.+)/.*\.(erb|haml)$}) { |m| "spec/requests/#{m[1]}_spec.rb" } + RELOAD_MATCHERS = Set.new + def self.watch_reload(pattern) + RELOAD_MATCHERS << pattern + end + + watch_reload('spec/spec_helper.rb') + watch_reload('config/(.*).rb') + def self.run(opts={}) self.new.run(opts) @@ -156,6 +163,15 @@ class Autospec::Runner specs = [] hit = false files.each do |file| + RELOAD_MATCHERS.each do |k| + if k.match(file) + spork_service.abort + stop_spork + sleep 1 + start_spork + return + end + end MATCHERS.each do |k,v| if m = k.match(file) hit = true diff --git a/lib/post_creator.rb b/lib/post_creator.rb index a93a14b4941..d71cbbbb228 100644 --- a/lib/post_creator.rb +++ b/lib/post_creator.rb @@ -41,45 +41,12 @@ class PostCreator def create topic = nil post = nil + new_topic = false Post.transaction do if @opts[:topic_id].blank? - topic_params = {title: @opts[:title], user_id: @user.id, last_post_user_id: @user.id} - topic_params[:archetype] = @opts[:archetype] if @opts[:archetype].present? - topic_params[:subtype] = @opts[:subtype] if @opts[:subtype].present? - - guardian.ensure_can_create!(Topic) - - category = Category.where(name: @opts[:category]).first - topic_params[:category_id] = category.id if category.present? - topic_params[:meta_data] = @opts[:meta_data] if @opts[:meta_data].present? - - topic = Topic.new(topic_params) - - if @opts[:auto_close_days] - guardian.ensure_can_moderate!(topic) - topic.auto_close_days = @opts[:auto_close_days] - end - - if @opts[:archetype] == Archetype.private_message - - topic.subtype = TopicSubtype.user_to_user unless topic.subtype - - unless @opts[:target_usernames].present? || @opts[:target_group_names].present? - topic.errors.add(:archetype, :cant_send_pm) - @errors = topic.errors - raise ActiveRecord::Rollback.new - end - - add_users(topic,@opts[:target_usernames]) - add_groups(topic,@opts[:target_group_names]) - topic.topic_allowed_users.build(user_id: @user.id) - end - - unless topic.save - @errors = topic.errors - raise ActiveRecord::Rollback.new - end + topic = create_topic + new_topic = true else topic = Topic.where(id: @opts[:topic_id]).first guardian.ensure_can_create!(Post, topic) @@ -112,7 +79,12 @@ class PostCreator # send a mail to notify users in case of a private message if topic.private_message? topic.allowed_users.where(["users.email_private_messages = true and users.id != ?", @user.id]).each do |u| - Jobs.enqueue_in(SiteSetting.email_time_window_mins.minutes, :user_email, type: :private_message, user_id: u.id, post_id: post.id) + Jobs.enqueue_in(SiteSetting.email_time_window_mins.minutes, + :user_email, + type: :private_message, + user_id: u.id, + post_id: post.id + ) end clear_possible_flags(topic) if post.post_number > 1 && topic.user_id != post.user_id @@ -129,12 +101,16 @@ class PostCreator @user.last_posted_at = post.created_at @user.save! - # Publish the post in the message bus - MessageBus.publish("/topic/#{post.topic_id}", - id: post.id, - created_at: post.created_at, - user: BasicUserSerializer.new(post.user).as_json(root: false), - post_number: post.post_number) + if post.post_number > 1 + MessageBus.publish("/topic/#{post.topic_id}",{ + id: post.id, + created_at: post.created_at, + user: BasicUserSerializer.new(post.user).as_json(root: false), + post_number: post.post_number + }, + group_ids: secure_group_ids(topic) + ) + end # Advance the draft sequence post.advance_draft_sequence @@ -147,7 +123,10 @@ class PostCreator # been comitted. topic_id = @opts[:topic_id] || topic.try(:id) Jobs.enqueue(:feature_topic_users, topic_id: topic.id) if topic_id.present? - post.trigger_post_process if post.present? + if post + post.trigger_post_process + after_topic_create(topic) if new_topic + end post end @@ -160,6 +139,74 @@ class PostCreator protected + def secure_group_ids(topic) + @secure_group_ids ||= if topic.category && topic.category.secure? + topic.category.groups.select("groups.id").map{|g| g.id} + end + end + + def after_topic_create(topic) + + # Don't publish invisible topics + return unless topic.visible? + + return if topic.private_message? + + topic.posters = topic.posters_summary + topic.posts_count = 1 + topic_json = TopicListItemSerializer.new(topic).as_json + + group_ids = secure_group_ids(topic) + + MessageBus.publish("/latest", topic_json, group_ids: group_ids) + + # If it has a category, add it to the category views too + if topic.category + MessageBus.publish("/category/#{topic.category.slug}", topic_json, group_ids: group_ids) + end + end + + def create_topic + topic_params = {title: @opts[:title], user_id: @user.id, last_post_user_id: @user.id} + topic_params[:archetype] = @opts[:archetype] if @opts[:archetype].present? + topic_params[:subtype] = @opts[:subtype] if @opts[:subtype].present? + + guardian.ensure_can_create!(Topic) + + category = Category.where(name: @opts[:category]).first + topic_params[:category_id] = category.id if category.present? + topic_params[:meta_data] = @opts[:meta_data] if @opts[:meta_data].present? + + topic = Topic.new(topic_params) + + if @opts[:auto_close_days] + guardian.ensure_can_moderate!(topic) + topic.auto_close_days = @opts[:auto_close_days] + end + + if @opts[:archetype] == Archetype.private_message + + topic.subtype = TopicSubtype.user_to_user unless topic.subtype + + unless @opts[:target_usernames].present? || @opts[:target_group_names].present? + topic.errors.add(:archetype, :cant_send_pm) + @errors = topic.errors + raise ActiveRecord::Rollback.new + end + + add_users(topic,@opts[:target_usernames]) + add_groups(topic,@opts[:target_group_names]) + topic.topic_allowed_users.build(user_id: @user.id) + end + + unless topic.save + @errors = topic.errors + raise ActiveRecord::Rollback.new + end + + topic + end + def clear_possible_flags(topic) # at this point we know the topic is a PM and has been replied to ... check if we need to clear any flags # diff --git a/spec/components/post_creator_spec.rb b/spec/components/post_creator_spec.rb index 71e1be09c83..f7c9dba1c1c 100644 --- a/spec/components/post_creator_spec.rb +++ b/spec/components/post_creator_spec.rb @@ -31,12 +31,53 @@ describe PostCreator do end context 'success' do - it 'creates a topic' do - lambda { creator.create }.should change(Topic, :count).by(1) + + it 'generates the correct messages for a secure topic' do + + admin = Fabricate(:admin) + + cat = Fabricate(:category) + cat.deny(:all) + cat.allow(Group[:admins]) + cat.save + + created_post = nil + reply = nil + + messages = MessageBus.track_publish do + created_post = PostCreator.new(admin, basic_topic_params.merge(category: cat.name)).create + reply = PostCreator.new(admin, raw: 'this is my test reply 123 testing', topic_id: created_post.topic_id).create + end + + topic_id = created_post.topic_id + + + messages.map{|m| m.channel}.sort.should == [ "/latest", + "/users/#{admin.username}", + "/users/#{admin.username}", + "/topic/#{created_post.topic_id}", + "/category/#{cat.slug}" + ].sort + admin_ids = [Group[:admins].id] + messages.any?{|m| m.group_ids != admin_ids}.should be_false + end - it 'returns a post' do - creator.create.is_a?(Post).should be_true + it 'generates the correct messages for a normal topic' do + + p = nil + messages = MessageBus.track_publish do + p = creator.create + topic_id = p.topic_id + end + + latest = messages.find{|m| m.channel == "/latest"} + latest.should_not be_nil + + user_action = messages.find{|m| m.channel == "/users/#{p.user.username}"} + user_action.should_not be_nil + + messages.length.should == 2 end it 'extracts links from the post' do @@ -44,20 +85,8 @@ describe PostCreator do creator.create end - it 'enqueues the post on the message bus' do - MessageBus.stubs(:publish).with("/users/#{user.username}", anything) - MessageBus.expects(:publish).with("/topic/#{topic.id}", instance_of(Hash)) - PostCreator.new(user, raw: basic_topic_params[:raw], topic_id: topic.id) - end - - it 'features topic users' do - Jobs.stubs(:enqueue).with(:process_post, anything) - Jobs.expects(:enqueue).with(:feature_topic_users, has_key(:topic_id)) - creator.create - end - it 'queues up post processing job when saved' do - Jobs.stubs(:enqueue).with(:feature_topic_users, has_key(:topic_id)) + Jobs.expects(:enqueue).with(:feature_topic_users, has_key(:topic_id)) Jobs.expects(:enqueue).with(:process_post, has_key(:post_id)) creator.create end diff --git a/spec/models/message_bus_observer_spec.rb b/spec/models/message_bus_observer_spec.rb deleted file mode 100644 index 6af9b095f20..00000000000 --- a/spec/models/message_bus_observer_spec.rb +++ /dev/null @@ -1,18 +0,0 @@ -require 'spec_helper' - -describe MessageBusObserver do - - context 'after create topic' do - - after do - @topic = Fabricate(:topic) - end - - it 'publishes the topic to the list' do - - end - - end - - -end diff --git a/spec/models/notification_spec.rb b/spec/models/notification_spec.rb index 75031814055..42de7c5f776 100644 --- a/spec/models/notification_spec.rb +++ b/spec/models/notification_spec.rb @@ -90,7 +90,7 @@ describe Notification do describe 'message bus' do it 'updates the notification count on create' do - MessageBusObserver.any_instance.expects(:refresh_notification_count).with(instance_of(Notification)) + Notification.any_instance.expects(:refresh_notification_count).returns(nil) Fabricate(:notification) end @@ -99,7 +99,7 @@ describe Notification do let!(:notification) { Fabricate(:notification) } it 'updates the notification count on destroy' do - MessageBusObserver.any_instance.expects(:refresh_notification_count).with(instance_of(Notification)) + Notification.any_instance.expects(:refresh_notification_count).returns(nil) notification.destroy end diff --git a/spec/models/topic_spec.rb b/spec/models/topic_spec.rb index 31428241e59..992476f6f31 100644 --- a/spec/models/topic_spec.rb +++ b/spec/models/topic_spec.rb @@ -180,14 +180,6 @@ describe Topic do end - context 'message bus' do - it 'calls the message bus observer after create' do - ActiveRecord::Base.observers.enable :all - MessageBusObserver.any_instance.expects(:after_create_topic).with(instance_of(Topic)) - Fabricate(:topic) - end - end - context 'post_numbers' do let!(:topic) { Fabricate(:topic) } let!(:p1) { Fabricate(:post, topic: topic, user: topic.user) } diff --git a/spec/models/user_action_spec.rb b/spec/models/user_action_spec.rb index 5f280b2af95..802f2b11102 100644 --- a/spec/models/user_action_spec.rb +++ b/spec/models/user_action_spec.rb @@ -91,11 +91,6 @@ describe UserAction do end end - it 'calls the message bus observer' do - MessageBusObserver.any_instance.expects(:after_create_user_action).with(instance_of(UserAction)) - Fabricate(:user_action) - end - describe 'when user likes' do let!(:post) { Fabricate(:post) } diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index ce1a3d32fcd..195a2d5a641 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -119,6 +119,30 @@ def build(*args) Fabricate.build(*args) end +module MessageBus::DiagnosticsHelper + def publish(channel, data, opts = nil) + id = super(channel, data, opts) + if @tracking + m = MessageBus::Message.new(-1, id, channel, data) + m.user_ids = opts[:user_ids] if opts + m.group_ids = opts[:group_ids] if opts + @tracking << m + end + id + end + + def track_publish + @tracking = tracking = [] + yield + @tracking = nil + tracking + end +end + +module MessageBus + extend MessageBus::DiagnosticsHelper +end + # --- Instructions --- # Sort the contents of this file into a Spork.prefork and a Spork.each_run # block.