[WIP] Unclutter commit monitor

app/workers/commit_monitor.rb

@@ -1,5 +1,3 @@
-require 'yaml'
-
 class CommitMonitor
   include Sidekiq::Worker
   sidekiq_options :queue => :miq_bot_glacial, :retry => false
@@ -9,36 +7,20 @@ class CommitMonitor
 
   include SidekiqWorkerMixin
 
-  # commit handlers expect to handle a specific commit at a time.
-  #
-  # Example: A commit message checker that will check for URLs and act upon them.
-  def self.commit_handlers
-    @commit_handlers ||= handlers_for(:commit)
-  end
-
-  # commit_range handlers expect to handle a range of commits as a group.
-  #
-  # Example: A style/syntax/warning checker on a PR branch, where we only want
-  #   to check the new commits, but as a group, since newer commits may fix
-  #   issues in prior commits.
-  def self.commit_range_handlers
-    @commit_range_handlers ||= handlers_for(:commit_range).select { |h| !h.respond_to?(:perform_batch_async) }
-  end
-
-  # branch handlers expect to handle an entire branch at once.
-  #
-  # Example: A PR branch mergability tester to see if the entire branch can be
-  #   merged or not.
-  def self.branch_handlers
-    @branch_handlers ||= handlers_for(:branch)
+  def self.handlers
+    @handlers ||= begin
+      workers_path = Rails.root.join("app/workers")
+      Dir.glob(workers_path.join("commit_monitor_handlers/*.rb")).collect do |f|
+        path = Pathname.new(f).relative_path_from(workers_path).to_s
+        path.chomp(".rb").classify.constantize
+      end
+    end
   end
 
-  # batch handlers expect to handle a batch of workers at once and will need
-  #   a wider range of information
-  #
-  # Example: A general commenter to GitHub for a number of issues
-  def self.batch_handlers
-    @batch_handlers ||= handlers_for(:commit_range).select { |h| h.respond_to?(:perform_batch_async) }
+  def self.handlers_for(branch)
+    handlers.select do |h|
+      h.handled_branch_modes.include?(branch.mode) && h.enabled_for?(branch.repo)
+    end
   end
 
   def perform
@@ -55,19 +37,16 @@ def process_repos
 
   private
 
-  attr_reader :repo, :branch, :new_commits, :all_commits, :statistics
+  attr_reader :repo, :branch, :new_commits, :all_commits
 
   def process_repo(repo)
-    @statistics = {}
-
     @repo = repo
     repo.git_fetch
 
     # Sort PR branches after regular branches
     sorted_branches = repo.branches.sort_by { |b| b.pull_request? ? 1 : -1 }
 
     sorted_branches.each do |branch|
-      @new_commits_details = nil
       @branch = branch
       process_branch
     end
@@ -78,8 +57,6 @@ def process_branch
 
     @new_commits, @all_commits = detect_commits
 
-    statistics[branch.name] = {:new_commits => new_commits} unless branch.pull_request?
-
     logger.info "Detected new commits #{new_commits}" if new_commits.any?
 
     save_branch_record
@@ -112,17 +89,6 @@ def compare_commits_list(left, right)
     {:same => same, :left_only => left_only, :right_only => right_only}
   end
 
-  def new_commits_details
-    @new_commits_details ||=
-      new_commits.each_with_object({}) do |commit, h|
-        git_commit = branch.git_service.commit(commit)
-        h[commit] = {
-          "message" => git_commit.full_message,
-          "files"   => git_commit.diff.file_status.keys
-        }
-      end
-  end
-
   def save_branch_record
     attrs = {:last_checked_on => Time.now.utc}
     attrs[:last_commit] = new_commits.last if new_commits.any?
@@ -135,102 +101,10 @@ def save_branch_record
     branch.update_columns(attrs)
   end
 
-  #
-  # Handler processing methods
-  #
-
-  def self.handlers_for(type)
-    workers_path = Rails.root.join("app/workers")
-    Dir.glob(workers_path.join("commit_monitor_handlers/#{type}/*.rb")).collect do |f|
-      path = Pathname.new(f).relative_path_from(workers_path).to_s
-      path.chomp(".rb").classify.constantize
-    end
-  end
-  private_class_method(:handlers_for)
-
-  def filter_handlers(handlers)
-    handlers.select do |h|
-      h.handled_branch_modes.include?(branch.mode) && h.enabled_for?(repo)
-    end
-  end
-
-  def commit_handlers
-    filter_handlers(self.class.commit_handlers)
-  end
-
-  def commit_range_handlers
-    filter_handlers(self.class.commit_range_handlers)
-  end
-
-  def branch_handlers
-    filter_handlers(self.class.branch_handlers)
-  end
-
-  def batch_handlers
-    filter_handlers(self.class.batch_handlers)
-  end
-
   def process_handlers
-    process_commit_handlers       if process_commit_handlers?
-    process_commit_range_handlers if process_commit_range_handlers?
-    process_branch_handlers       if process_branch_handlers?
-    process_batch_handlers        if process_batch_handlers?
-  end
-
-  def process_commit_handlers?
-    commit_handlers.any? && new_commits.any?
-  end
-
-  def process_commit_range_handlers?
-    commit_range_handlers.any? && new_commits.any?
-  end
-
-  def process_branch_handlers?
-    branch_handlers.any? && send("process_#{branch.mode}_branch_handlers?")
-  end
-
-  def process_batch_handlers?
-    batch_handlers.any? && new_commits.any?
-  end
-
-  def process_pr_branch_handlers?
-    parent_branch_new_commits = statistics.fetch_path("master", :new_commits)
-    new_commits.any? || parent_branch_new_commits.any?
-  end
-
-  def process_regular_branch_handlers?
-    new_commits.any?
-  end
-
-  def process_commit_handlers
-    new_commits_details.each do |commit, details|
-      commit_handlers.each do |h|
-        logger.info("Queueing #{h.name.split("::").last} for commit #{commit} on branch #{branch.name}")
-        h.perform_async(branch.id, commit, details)
-      end
-    end
-  end
-
-  def process_commit_range_handlers
-    commit_range = [new_commits.first, new_commits.last].uniq.join("..")
-
-    commit_range_handlers.each do |h|
-      logger.info("Queueing #{h.name.split("::").last} for commit range #{commit_range} on branch #{branch.name}")
-      h.perform_async(branch.id, new_commits)
-    end
-  end
-
-  def process_branch_handlers
-    branch_handlers.each do |h|
-      logger.info("Queueing #{h.name.split("::").last} for branch #{branch.name}")
-      h.perform_async(branch.id)
-    end
-  end
-
-  def process_batch_handlers
-    batch_handlers.each do |h|
-      logger.info("Queueing #{h.name} for branch #{branch.name}")
-      h.perform_batch_async(branch.id, new_commits_details)
+    self.class.handlers_for(branch).each do |handler|
+      method = handler.respond_to?(:perform_batch_async) ? :perform_batch_async : :perform_async
+      handler.public_send(method, branch.id, new_commits)
     end
   end
 end

app/workers/commit_monitor_handlers/commit_range/branch_mergeability_checker.rb → app/workers/commit_monitor_handlers/branch_mergeability_checker.rb

@@ -1,4 +1,4 @@
-class CommitMonitorHandlers::CommitRange::BranchMergeabilityChecker
+class CommitMonitorHandlers::BranchMergeabilityChecker
   include Sidekiq::Worker
   sidekiq_options :queue => :miq_bot
 

app/workers/commit_monitor_handlers/bugzilla_commenter.rb

@@ -0,0 +1,73 @@
+class CommitMonitorHandlers::BugzillaCommenter
+  include Sidekiq::Worker
+  sidekiq_options :queue => :miq_bot
+
+  include BranchWorkerMixin
+
+  def self.handled_branch_modes
+    [:regular]
+  end
+
+  attr_reader :commit, :message
+
+  def perform(branch_id, new_commits)
+    return unless find_branch(branch_id, :regular)
+
+    bugs = Hash.new { |h, k| h[k] = [] }
+
+    new_commits.each do |commit|
+      message = repo.git_service.commit(commit).full_message
+      BugzillaService.search_in_message(message).each do |bug|
+        bugs[bug[:bug_id]] << bug.merge(:commit => commit, :commit_message => message)
+      end
+    end
+
+    bugs.each do |bug_id, info|
+      resolved = info.any? { |i| i[:resolution] }
+      comment_parts = info.collect { |i| format_comment_part(i[:commit], i[:commit_message]) }
+      comments = build_comments(comment_parts)
+
+      update_bugzilla_status(bug_id, comments, resolved)
+    end
+  end
+
+  private
+
+  def update_bugzilla_status(bug_id, comments, resolution)
+    logger.info "Adding #{"comment".pluralize(comments.size)} to bug #{bug_id}."
+
+    BugzillaService.call do |service|
+      service.with_bug(bug_id) do |bug|
+        break if bug.nil?
+
+        comments.each { |comment| bug.add_comment(comment) }
+        update_bug_status(bug) if resolution
+        bug.save
+      end
+    end
+  end
+
+  def message_header(messages)
+    @message_header ||= "New #{"commit".pluralize(messages.size)} detected on #{fq_repo_name}/#{branch.name}:\n\n"
+  end
+
+  def build_comments(messages)
+    message_builder = BugzillaService::MessageBuilder.new(message_header(messages))
+    messages.each { |m| message_builder.write("#{m}\n\n\n") }
+    message_builder.comments
+  end
+
+  def format_comment_part(commit, message)
+    "#{branch.commit_uri_to(commit)}\n#{message}"
+  end
+
+  def update_bug_status(bug)
+    case bug.status
+    when "NEW", "ASSIGNED", "ON_DEV"
+      logger.info "Changing status of bug #{bug.id} to POST."
+      bug.status = "POST"
+    else
+      logger.info "Not changing status of bug #{bug.id} due to status of #{bug.status}"
+    end
+  end
+end

app/workers/commit_monitor_handlers/bugzilla_pr_checker.rb

@@ -0,0 +1,68 @@
+class CommitMonitorHandlers::BugzillaPrChecker
+  include Sidekiq::Worker
+  sidekiq_options :queue => :miq_bot
+
+  include BranchWorkerMixin
+
+  def self.handled_branch_modes
+    [:pr]
+  end
+
+  def perform(branch_id, new_commits)
+    return unless find_branch(branch_id, :pr)
+
+    bug_ids = new_commits.flat_map do |commit|
+      message = repo.git_service.commit(commit).full_message
+      BugzillaService.ids_in_git_commit_message(message)
+    end
+
+    bug_ids.uniq.each do |bug_id|
+      update_bugzilla_status(bug_id)
+    end
+  end
+
+  private
+
+  def update_bugzilla_status(bug_id)
+    BugzillaService.call do |service|
+      service.with_bug(bug_id) do |bug|
+        break if bug.nil?
+
+        add_pr_comment(bug)
+        update_bug_status(bug)
+        bug.save
+      end
+    end
+  end
+
+  def add_pr_comment(bug)
+    if bug_has_pr_uri_comment?(bug)
+      logger.info "Not commenting on bug #{bug.id} due to duplicate comment."
+      return
+    end
+
+    case bug.status
+    when "NEW", "ASSIGNED", "ON_DEV"
+      logger.info "Adding comment to bug #{bug.id}."
+      bug.add_comment(@branch.github_pr_uri)
+    else
+      logger.info "Not commenting on bug #{bug.id} due to status of #{bug.status}"
+    end
+  end
+
+  def bug_has_pr_uri_comment?(bug)
+    bug.comments.any? do |c|
+      c.text.include?(@branch.github_pr_uri)
+    end
+  end
+
+  def update_bug_status(bug)
+    case bug.status
+    when "NEW", "ASSIGNED"
+      logger.info "Changing status of bug #{bug.id} to ON_DEV."
+      bug.status = "ON_DEV"
+    else
+      logger.info "Not changing status of bug #{bug.id} due to status of #{bug.status}"
+    end
+  end
+end