Merge pull request #955 from novemberkilo/master

Reduce complexity of PostCreator

lib/post_creator.rb

@@ -1,6 +1,7 @@
 # Responsible for creating posts and topics
 #
 require_dependency 'rate_limiter'
+require_dependency 'topic_creator'
 
 class PostCreator
 
@@ -51,108 +52,27 @@ class PostCreator
   end
 
   def create
-    topic = nil
+    @topic = nil
-    post = nil
+    @post = nil
-    new_topic = false
+    @new_topic = false
 
     Post.transaction do
-      if @opts[:topic_id].blank?
+      setup_topic
-        topic = create_topic
+      setup_post
-        new_topic = true
+      rollback_if_host_spam_detected
-      else
+      save_post
-        topic = Topic.where(id: @opts[:topic_id]).first
+      extract_links
-        guardian.ensure_can_create!(Post, topic)
+      store_unique_post_key
+      send_notifications_for_private_message
+      track_topic
+      update_user_counts
+      publish
+      @post.advance_draft_sequence
+      @post.save_reply_relationships
     end
 
-      post = topic.posts.new(raw: @opts[:raw],
+    enqueue_jobs
-                             user: @user,
+    @post
-                             reply_to_post_number: @opts[:reply_to_post_number])
-
-      post.post_type = @opts[:post_type] if @opts[:post_type].present?
-      post.no_bump = @opts[:no_bump] if @opts[:no_bump].present?
-      post.extract_quoted_post_numbers
-      post.acting_user = @opts[:acting_user] if @opts[:acting_user].present?
-      post.created_at = Time.zone.parse(@opts[:created_at].to_s) if @opts[:created_at].present?
-
-      post.image_sizes = @opts[:image_sizes] if @opts[:image_sizes].present?
-      post.invalidate_oneboxes = @opts[:invalidate_oneboxes] if @opts[:invalidate_oneboxes].present?
-
-
-      # If the post has host spam, roll it back.
-      if post.has_host_spam?
-        post.errors.add(:base, I18n.t(:spamming_host))
-        @errors = post.errors
-        @spam = true
-        raise ActiveRecord::Rollback.new
-      end
-
-      unless post.save
-        @errors = post.errors
-        raise ActiveRecord::Rollback.new
-      end
-
-      # Extract links
-      TopicLink.extract_from(post)
-
-      # Store unique post key
-      if SiteSetting.unique_posts_mins > 0
-        $redis.setex(post.unique_post_key, SiteSetting.unique_posts_mins.minutes.to_i, "1")
-      end
-
-      # 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
-                         )
-        end
-
-        clear_possible_flags(topic) if post.post_number > 1 && topic.user_id != post.user_id
-      end
-
-      # Track the topic
-      TopicUser.auto_track(@user.id, topic.id, TopicUser.notification_reasons[:created_post])
-
-      # We don't count replies to your own topics
-      if @user.id != topic.user_id
-        @user.update_topic_reply_count
-      end
-
-      @user.last_posted_at = post.created_at
-      @user.save!
-      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
-
-      # Save the quote relationships
-      post.save_reply_relationships
-    end
-
-    if post && !post.errors.present?
-
-      # We need to enqueue jobs after the transaction. Otherwise they might begin before the data has
-      # 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
-      after_post_create(post)
-      after_topic_create(topic) if new_topic
-    end
-
-    post
   end
 
 
@@ -169,66 +89,25 @@ class PostCreator
     end
   end
 
-  def after_post_create(post)
+  def after_post_create
-    if post.post_number > 1
+    if @post.post_number > 1
-      TopicTrackingState.publish_unread(post)
+      TopicTrackingState.publish_unread(@post)
     end
   end
 
-  def after_topic_create(topic)
+  def after_topic_create
 
     # Don't publish invisible topics
-    return unless topic.visible?
+    return unless @topic.visible?
 
-    return if topic.private_message?
+    return if @topic.private_message?
 
-    topic.posters = topic.posters_summary
+    @topic.posters = @topic.posters_summary
-    topic.posts_count = 1
+    @topic.posts_count = 1
 
-    TopicTrackingState.publish_new(topic)
+    TopicTrackingState.publish_new(@topic)
   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_params[:created_at] = Time.zone.parse(@opts[:created_at].to_s) if @opts[:created_at].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
@@ -249,27 +128,117 @@ class PostCreator
     end
   end
 
-  def add_users(topic, usernames)
+  private
-    return unless usernames
+
-    User.where(username: usernames.split(',')).each do |user|
+  def setup_topic
-      check_can_send_permission!(topic,user)
+    if @opts[:topic_id].blank?
-      topic.topic_allowed_users.build(user_id: user.id)
+      topic_creator = TopicCreator.new(@user, guardian, @opts)
+      topic = topic_creator.create
+      @errors = topic_creator.errors
+
+      @new_topic = true
+    else
+      topic = Topic.where(id: @opts[:topic_id]).first
+      guardian.ensure_can_create!(Post, topic)
     end
+    @topic = topic
   end
 
-  def add_groups(topic, groups)
+  def setup_post
-    return unless groups
+    post = @topic.posts.new(raw: @opts[:raw],
-    Group.where(name: groups.split(',')).each do |group|
+                           user: @user,
-      check_can_send_permission!(topic,group)
+                           reply_to_post_number: @opts[:reply_to_post_number])
-      topic.topic_allowed_groups.build(group_id: group.id)
+
-    end
+    post.post_type = @opts[:post_type] if @opts[:post_type].present?
+    post.no_bump = @opts[:no_bump] if @opts[:no_bump].present?
+    post.extract_quoted_post_numbers
+    post.acting_user = @opts[:acting_user] if @opts[:acting_user].present?
+    post.created_at = Time.zone.parse(@opts[:created_at].to_s) if @opts[:created_at].present?
+
+    post.image_sizes = @opts[:image_sizes] if @opts[:image_sizes].present?
+    post.invalidate_oneboxes = @opts[:invalidate_oneboxes] if @opts[:invalidate_oneboxes].present?
+    @post = post
   end
 
-  def check_can_send_permission!(topic,item)
+  def rollback_if_host_spam_detected
-    unless guardian.can_send_private_message?(item)
+    if @post.has_host_spam?
-      topic.errors.add(:archetype, :cant_send_pm)
+      @post.errors.add(:base, I18n.t(:spamming_host))
-      @errors = topic.errors
+      @errors = @post.errors
+      @spam = true
       raise ActiveRecord::Rollback.new
     end
   end
+
+  def save_post
+    unless @post.save
+      @errors = @post.errors
+      raise ActiveRecord::Rollback.new
+    end
+  end
+
+  def store_unique_post_key
+    if SiteSetting.unique_posts_mins > 0
+      $redis.setex(@post.unique_post_key, SiteSetting.unique_posts_mins.minutes.to_i, "1")
+    end
+  end
+
+  def send_notifications_for_private_message
+    # 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
+                       )
+      end
+
+      clear_possible_flags(@topic) if @post.post_number > 1 && @topic.user_id != @post.user_id
+    end
+  end
+
+  def update_user_counts
+    # We don't count replies to your own topics
+    if @user.id != @topic.user_id
+      @user.update_topic_reply_count
+    end
+
+    @user.last_posted_at = @post.created_at
+    @user.save!
+  end
+
+  def publish
+    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
+  end
+
+  def extract_links
+    TopicLink.extract_from(@post)
+  end
+
+  def track_topic
+    TopicUser.auto_track(@user.id, @topic.id, TopicUser.notification_reasons[:created_post])
+  end
+
+  def enqueue_jobs
+    if @post && !@post.errors.present?
+
+      # We need to enqueue jobs after the transaction. Otherwise they might begin before the data has
+      # 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
+      after_post_create
+      after_topic_create if @new_topic
+    end
+  end
 end

lib/topic_creator.rb

@@ -0,0 +1,88 @@
+class TopicCreator
+
+  attr_accessor :errors
+
+  def initialize(user, guardian, opts)
+    @user = user
+    @guardian = guardian
+    @opts = opts
+  end
+
+  def create
+    topic_params = setup
+    @topic = Topic.new(topic_params)
+
+    setup_auto_close_days if @opts[:auto_close_days]
+
+    process_private_message if @opts[:archetype] == Archetype.private_message
+    save_topic
+
+    @topic
+  end
+
+  private
+
+  def setup
+    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_params[:created_at] = Time.zone.parse(@opts[:created_at].to_s) if @opts[:created_at].present?
+    topic_params
+  end
+
+  def setup_auto_close_days
+    @guardian.ensure_can_moderate!(@topic)
+    @topic.auto_close_days = @opts[:auto_close_days]
+  end
+
+  def process_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
+
+  def save_topic
+    unless @topic.save
+      @errors = @topic.errors
+      raise ActiveRecord::Rollback.new
+    end
+  end
+
+  def add_users(topic, usernames)
+    return unless usernames
+    User.where(username: usernames.split(',')).each do |user|
+      check_can_send_permission!(topic,user)
+      topic.topic_allowed_users.build(user_id: user.id)
+    end
+  end
+
+  def add_groups(topic, groups)
+    return unless groups
+    Group.where(name: groups.split(',')).each do |group|
+      check_can_send_permission!(topic,group)
+      topic.topic_allowed_groups.build(group_id: group.id)
+    end
+  end
+
+  def check_can_send_permission!(topic,item)
+    unless @guardian.can_send_private_message?(item)
+      topic.errors.add(:archetype, :cant_send_pm)
+      @errors = topic.errors
+      raise ActiveRecord::Rollback.new
+    end
+  end
+end