Add limited caching of rules for expressions

Expression rules were doing DB lookups every time they were used. This caches rules
just before the initial audit or a commit audit. This is weak caching and shouldn't
affect the analysts ability to change rules on the fly. Highly scientific emperical
studies show this change saves about eleventy billion DB lookups and keeps my laptop
from getting so hot I can cook with it.

Author: CalebFenton

app/models/rules.rb

@@ -37,7 +37,7 @@ def validate
       begin
         Boolean.parse(dummy)
 
-        exp = ExpressionRule.new(value)
+        exp = ExpressionRule.new(value, [])
         exp.rule_names.each do |exp_rule_name|
           next if Rules[name: exp_rule_name]
           errors.add(:value, "referenced rule does not exist: #{exp_rule_name}")

app/workers/commit_auditor.rb

@@ -30,13 +30,13 @@ def perform(project_id, commit, rules, github_token)
     diff = get_commit_diff(commit, rules, github_token)
 
     builder = AuditResultsBuilder.new
-    record = builder.build(project_id, commit, rules, diff)
+    results = builder.build(project_id, commit, diff, rules)
     return unless record
 
     begin
-      Commits.insert(record)
+      Commits.insert(results)
     rescue Sequel::UniqueConstraintViolation
-      Rails.logger.debug "Dropping duplicate commit: #{record}"
+      Rails.logger.debug "Dropping duplicate commit: #{results}"
     end
   end
 

app/workers/initial_auditor.rb

@@ -31,8 +31,8 @@ def perform(project_id, project_name, rules)
     tmpdir = Dir.mktmpdir(['cwatcher', project_name.sub('/', '-')])
     commits = []
     begin
-      git = clone(project_name, tmpdir)
       builder = AuditResultsBuilder.new
+      git = clone(project_name, tmpdir)
       # Log helpfully forces a limit which defaults to 30.
       git_commits = git.log(100000000)
       Rails.logger.debug "Collected #{git_commits.size} commits from #{project_name}"
@@ -53,13 +53,13 @@ def perform(project_id, project_name, rules)
 
         commit = build_commit_hash(c)
         commits << commit
-        record = builder.build(project_id, commit, rules, diff)
-        next unless record
+        results = builder.build(project_id, commit, diff, rules)
+        next unless results
 
         begin
-          Commits.create(record)
+          Commits.create(results)
         rescue Sequel::UniqueConstraintViolation
-          Rails.logger.debug "Dropping duplicate commit: #{record}"
+          Rails.logger.debug "Dropping duplicate commit: #{results}"
         end
       end
     ensure

app/workers/project_enqueuer.rb

@@ -41,7 +41,7 @@ def perform
 
   def enqueue_projects(projects, audit_frequency, github_token)
     projects.each do |project|
-      rule_sets = get_rules(JSON.parse(project[:rule_sets]))
+      rules = get_rules(JSON.parse(project[:rule_sets]))
 
       # Update next_audit immediately to avoid re-enqueueing.
       next_audit = Time.now.to_i + audit_frequency
@@ -57,14 +57,14 @@ def enqueue_projects(projects, audit_frequency, github_token)
         InitialAuditor.perform_async(
           project[:id],
           project[:name],
-          rule_sets.to_json
+          rules.to_json
         )
       else
         CommitCollector.perform_async(
           project[:id],
           project[:name],
           last_commit_time,
-          rule_sets.to_json,
+          rules.to_json,
           github_token
         )
       end
@@ -74,6 +74,7 @@ def enqueue_projects(projects, audit_frequency, github_token)
   def get_rules(rule_set_names)
     rule_sets = RuleSets.where(name: rule_set_names).to_hash
     rule_names = rule_sets.values.collect { |s| JSON.parse(s[:rules]) }.flatten.sort.uniq
+
     Rules.where(name: rule_names).select(:id, :name, :rule_type_id, :value, :notification_id)
   end
 end

lib/audit_results_builder.rb

@@ -3,33 +3,38 @@
 require_relative "#{Rails.root}/lib/rules/rule_auditor"
 
 class AuditResultsBuilder
-    def build(project_id, commit, rules, diff)
-        audit_results = []
-        rules.each do |r|
-            audit_result = RuleAuditor.audit(
-                commit,
-                r[:rule_type_id],
-                r[:value],
-                diff
-            )
-            next unless audit_result
+  def initialize
+    @all_rules = Rules.collect { |r| r.to_hash }
+  end
 
-            audit_results << {
-                rule_id: r[:id],
-                rule_name: r[:name],
-                rule_type_id: r[:rule_type_id],
-                rule_value: r[:value],
-                notification_id: r[:notification_id],
-                audit_result: audit_result,
-            }
-        end
-        return if audit_results.empty?
+  def build(project_id, commit, diff, rules)
+    audit_results = []
+    auditor = RuleAuditor.new(@all_rules)
+    rules.each do |r|
+      audit_result = auditor.audit(
+        commit,
+        r[:rule_type_id],
+        r[:value],
+        diff
+      )
+      next unless audit_result
 
-        {
-            project_id: project_id,
-            commit_hash: commit[:sha],
-            commit_date: Time.iso8601(commit[:commit][:author][:date]),
-            audit_results: audit_results.to_json,
-        }
+      audit_results << {
+        rule_id: r[:id],
+        rule_name: r[:name],
+        rule_type_id: r[:rule_type_id],
+        rule_value: r[:value],
+        notification_id: r[:notification_id],
+        audit_result: audit_result,
+      }
     end
+    return if audit_results.empty?
+
+    {
+      project_id: project_id,
+      commit_hash: commit[:sha],
+      commit_date: Time.iso8601(commit[:commit][:author][:date]),
+      audit_results: audit_results.to_json,
+    }
+  end
 end

lib/rules/expression_rule.rb

@@ -1,15 +1,14 @@
-class ExpressionRule
-  attr_reader :rule_names
+require_relative 'rule_auditor'
 
-  def initialize(expression)
+class ExpressionRule
+  def initialize(expression, all_rules)
     @expression = expression
-    @rule_names = parse_rule_names
-    @rules = nil
+    @all_rules = all_rules
+    @auditor = RuleAuditor.new(@all_rules)
+    @rule_name_to_rule = Hash[@all_rules.map { |r| [r[:name], r] }]
   end
 
   def evaluate(commit, diff)
-    @rules ||= Rules.where(name: @rule_names)
-
     all_results = []
 
     if diff
@@ -25,13 +24,15 @@ def evaluate(commit, diff)
     all_results.empty? ? nil : all_results
   end
 
-private
+  def rule_names
+    @expression.tr('!&|()', '').split(/\s+/).uniq.sort
+  end
 
   def evaluate_patch(commit, patch)
-    # Todo change to collect
     results = []
-    resolved_expr = "#{@expression}"
-    @rules.each do |rule|
+    resolved_expr = @expression.to_s
+    rule_names.each do |rule_name|
+      rule = @rule_name_to_rule[rule_name]
       resolved_expr, result = resolve_rule(rule, resolved_expr, commit, patch)
       results << result
     end
@@ -48,18 +49,14 @@ def resolve_rule(rule, resolved_expr, commit, patch)
     bool_value = !result.nil?
 
     [
-      resolved_expr.gsub(rule[:name], "#{bool_value}"),
+      resolved_expr.gsub(rule[:name], bool_value.to_s),
       # Store result even if nil. Expression could be "!someRule"
       { rule_name: rule[:name], result: result }
     ]
   end
 
-  def parse_rule_names
-    @expression.tr('!&|()', '').split(/\s+/).uniq.sort
-  end
-
   def evaluate_rule(rule, commit, diff)
-    RuleAuditor.audit(
+    @auditor.audit(
       commit,
       rule[:rule_type_id],
       rule[:value],

lib/rules/rule_auditor.rb

@@ -1,10 +1,15 @@
 require_relative 'expression_rule'
 
 class RuleAuditor
-  # The actual content of the line is not exposed. *shrug*
+  # The actual content of the line is not publicly exposed.
   GitDiffParser::Line.class_eval { attr_reader :content }
 
-  def self.audit(commit, rule_type_id, rule_value, diff)
+  def initialize(all_rules)
+    # Used for expression rules so it doesn't have to look them up
+    @all_rules = all_rules
+  end
+
+  def audit(commit, rule_type_id, rule_value, diff)
     case rule_type_id
     when 1
       return unless diff
@@ -28,13 +33,13 @@ def self.audit(commit, rule_type_id, rule_value, diff)
 
 private
 
-  def self.audit_filename_pattern(pattern, diff)
+  def audit_filename_pattern(pattern, diff)
     filenames = diff.collect { |e| e.file }
     results = filenames.select { |e| e =~ pattern }
     results.empty? ? nil : results
   end
 
-  def self.audit_changed_code_pattern(pattern, diff)
+  def audit_changed_code_pattern(pattern, diff)
     results = []
     diff.each do |d|
       matches = d.body.scan(pattern)
@@ -81,7 +86,7 @@ def self.audit_changed_code_pattern(pattern, diff)
     results.empty? ? nil : results
   end
 
-  def self.audit_code_pattern(pattern, diff)
+  def audit_code_pattern(pattern, diff)
     results = []
     diff.each do |d|
       matches = d.body.scan(pattern)
@@ -111,24 +116,24 @@ def self.audit_code_pattern(pattern, diff)
     results.empty? ? nil : results
   end
 
-  def self.audit_message_pattern(commit, pattern)
+  def audit_message_pattern(commit, pattern)
     message = commit[:commit][:message]
     (message =~ pattern) ? message : nil
   end
 
-  def self.audit_author_pattern(commit, pattern)
+  def audit_author_pattern(commit, pattern)
     author_name = commit[:commit][:author][:name]
     author_email = commit[:commit][:author][:email]
     author = "#{author_name} <#{author_email}>"
     (author =~ pattern) ? author : nil
   end
 
-  def self.audit_expression(commit, expression, diff)
-    rule = ExpressionRule.new(expression)
+  def audit_expression(commit, expression, diff)
+    rule = ExpressionRule.new(expression, @all_rules)
     rule.evaluate(commit, diff)
   end
 
-  def self.audit_commit_pattern(commit, pattern, diff)
+  def audit_commit_pattern(commit, pattern, diff)
     results = []
     results << audit_message_pattern(commit, pattern)
     results << audit_code_pattern(pattern, diff) if diff