Commit 7352a1ce773b04a326c841531fe4b6b338a2cac6
1 parent
f13bc748
Exists in
master
and in
1 other branch
add specs for refingerprint/recache classes
Showing
8 changed files
with
98 additions
and
34 deletions
Show diff stats
app/interactors/notice_refingerprinter.rb
| @@ -15,19 +15,21 @@ class NoticeRefingerprinter | @@ -15,19 +15,21 @@ class NoticeRefingerprinter | ||
| 15 | end | 15 | end |
| 16 | 16 | ||
| 17 | puts 'Finished generating notice fingerprints' | 17 | puts 'Finished generating notice fingerprints' |
| 18 | - end | 18 | + puts 'Destroying orphaned err records' |
| 19 | + | ||
| 20 | + Err.each { |e| e.destroy if e.notices.size == 0 } | ||
| 19 | 21 | ||
| 20 | - def self.apps | ||
| 21 | - @apps ||= App.all.index_by(&:id) | 22 | + puts 'Finished destroying orphaned err records' |
| 22 | end | 23 | end |
| 23 | 24 | ||
| 24 | def self.refingerprint(notice) | 25 | def self.refingerprint(notice) |
| 25 | - app = apps[notice.try(:err).try(:problem).try(:app_id)] | ||
| 26 | - app.find_or_create_err!( | 26 | + app = notice.app |
| 27 | + notice.err = app.find_or_create_err!( | ||
| 27 | error_class: notice.error_class, | 28 | error_class: notice.error_class, |
| 28 | environment: notice.environment_name, | 29 | environment: notice.environment_name, |
| 29 | fingerprint: app.notice_fingerprinter.generate(app.api_key, notice, notice.backtrace) | 30 | fingerprint: app.notice_fingerprinter.generate(app.api_key, notice, notice.backtrace) |
| 30 | ) | 31 | ) |
| 32 | + notice.save! | ||
| 31 | end | 33 | end |
| 32 | 34 | ||
| 33 | def self.puts(*args) | 35 | def self.puts(*args) |
app/models/app.rb
| @@ -65,16 +65,15 @@ class App | @@ -65,16 +65,15 @@ class App | ||
| 65 | # * <tt>:fingerprint</tt> - a unique value identifying the notice | 65 | # * <tt>:fingerprint</tt> - a unique value identifying the notice |
| 66 | # | 66 | # |
| 67 | def find_or_create_err!(attrs) | 67 | def find_or_create_err!(attrs) |
| 68 | - Err.where( | ||
| 69 | - :fingerprint => attrs[:fingerprint] | ||
| 70 | - ).first || ( | ||
| 71 | - problem = problems.create!( | ||
| 72 | - error_class: attrs[:error_class], | ||
| 73 | - environment: attrs[:environment], | ||
| 74 | - app_name: name | ||
| 75 | - ) | ||
| 76 | - problem.errs.create!(attrs.slice(:fingerprint, :problem_id)) | 68 | + err = Err.where(fingerprint: attrs[:fingerprint]).first |
| 69 | + return err if err | ||
| 70 | + | ||
| 71 | + problem = problems.create!( | ||
| 72 | + error_class: attrs[:error_class], | ||
| 73 | + environment: attrs[:environment], | ||
| 74 | + app_name: name | ||
| 77 | ) | 75 | ) |
| 76 | + problem.errs.create!(attrs.slice(:fingerprint, :problem_id)) | ||
| 78 | end | 77 | end |
| 79 | 78 | ||
| 80 | # Mongoid Bug: find(id) on association proxies returns an Enumerator | 79 | # Mongoid Bug: find(id) on association proxies returns an Enumerator |
app/models/notice.rb
| @@ -29,9 +29,7 @@ class Notice | @@ -29,9 +29,7 @@ class Notice | ||
| 29 | scope :ordered, ->{ order_by(:created_at.asc) } | 29 | scope :ordered, ->{ order_by(:created_at.asc) } |
| 30 | scope :reverse_ordered, ->{ order_by(:created_at.desc) } | 30 | scope :reverse_ordered, ->{ order_by(:created_at.desc) } |
| 31 | scope :for_errs, Proc.new { |errs| | 31 | scope :for_errs, Proc.new { |errs| |
| 32 | - if (ids = errs.all.map(&:id)) && ids.present? | ||
| 33 | - where(:err_id.in => ids) | ||
| 34 | - end | 32 | + where(:err_id.in => errs.all.map(&:id)) |
| 35 | } | 33 | } |
| 36 | 34 | ||
| 37 | def user_agent | 35 | def user_agent |
app/models/notice_fingerprinter.rb
| @@ -14,7 +14,7 @@ class NoticeFingerprinter | @@ -14,7 +14,7 @@ class NoticeFingerprinter | ||
| 14 | embedded_in :site_config | 14 | embedded_in :site_config |
| 15 | 15 | ||
| 16 | def generate(api_key, notice, backtrace) | 16 | def generate(api_key, notice, backtrace) |
| 17 | - material = [] | 17 | + material = [ api_key ] |
| 18 | material << notice.error_class if error_class | 18 | material << notice.error_class if error_class |
| 19 | material << notice.filtered_message if message | 19 | material << notice.filtered_message if message |
| 20 | material << notice.component if component | 20 | material << notice.component if component |
spec/fabricators/backtrace_fabricator.rb
| @@ -7,13 +7,3 @@ Fabricator :backtrace do | @@ -7,13 +7,3 @@ Fabricator :backtrace do | ||
| 7 | } | 7 | } |
| 8 | end | 8 | end |
| 9 | end | 9 | end |
| 10 | - | ||
| 11 | -Fabricator :short_backtrace do | ||
| 12 | - lines(:count => 5) do | ||
| 13 | - { | ||
| 14 | - number: rand(999), | ||
| 15 | - file: "/path/to/file/#{SecureRandom.hex(4)}.rb", | ||
| 16 | - method: ActiveSupport.methods.shuffle.first | ||
| 17 | - } | ||
| 18 | - end | ||
| 19 | -end |
spec/fabricators/notice_fabricator.rb
spec/interactors/notice_refingerprinter_spec.rb
| 1 | describe NoticeRefingerprinter do | 1 | describe NoticeRefingerprinter do |
| 2 | + let(:app) { Fabricate(:app) } | ||
| 2 | let(:backtrace) do | 3 | let(:backtrace) do |
| 3 | Fabricate(:backtrace) | 4 | Fabricate(:backtrace) |
| 4 | end | 5 | end |
| 5 | 6 | ||
| 6 | - let(:notices) do | ||
| 7 | - 5.times.map do | ||
| 8 | - Fabricate(:notice, backtrace: backtrace) | 7 | + before do |
| 8 | + notices | ||
| 9 | + end | ||
| 10 | + | ||
| 11 | + context 'identical backtraces' do | ||
| 12 | + let(:notices) do | ||
| 13 | + 5.times.map do | ||
| 14 | + notice = Fabricate(:notice, backtrace: backtrace, app: app) | ||
| 15 | + notice.save! | ||
| 16 | + notice | ||
| 17 | + end | ||
| 18 | + end | ||
| 19 | + | ||
| 20 | + it 'has only one err' do | ||
| 21 | + described_class.run | ||
| 22 | + expect(Err.count).to eq(1) | ||
| 9 | end | 23 | end |
| 10 | end | 24 | end |
| 11 | 25 | ||
| 12 | - it 'shits' do | ||
| 13 | - binding.pry | ||
| 14 | - 1 | 26 | + context 'minor backtrace differences' do |
| 27 | + let(:notices) do | ||
| 28 | + line_numbers = [1, 1, 2, 2, 3] | ||
| 29 | + 5.times.map do | ||
| 30 | + b = backtrace.clone | ||
| 31 | + b.lines[5][:number] = line_numbers.shift | ||
| 32 | + b.save! | ||
| 33 | + notice = Fabricate(:notice, backtrace: b, app: app) | ||
| 34 | + notice.save! | ||
| 35 | + end | ||
| 36 | + end | ||
| 37 | + | ||
| 38 | + it 'has three errs with default fingerprinter' do | ||
| 39 | + described_class.run | ||
| 40 | + expect(Err.count).to eq(3) | ||
| 41 | + end | ||
| 42 | + | ||
| 43 | + it 'has one err when limiting backtrace line count' do | ||
| 44 | + fingerprinter = app.notice_fingerprinter | ||
| 45 | + fingerprinter.backtrace_lines = 4 | ||
| 46 | + fingerprinter.save! | ||
| 47 | + | ||
| 48 | + described_class.run | ||
| 49 | + expect(Err.count).to eq(1) | ||
| 50 | + end | ||
| 15 | end | 51 | end |
| 16 | end | 52 | end |
| @@ -0,0 +1,38 @@ | @@ -0,0 +1,38 @@ | ||
| 1 | +describe ProblemRecacher do | ||
| 2 | + let(:app) { Fabricate(:app) } | ||
| 3 | + let(:backtrace) do | ||
| 4 | + Fabricate(:backtrace) | ||
| 5 | + end | ||
| 6 | + | ||
| 7 | + before do | ||
| 8 | + notices | ||
| 9 | + | ||
| 10 | + NoticeRefingerprinter.run | ||
| 11 | + described_class.run | ||
| 12 | + end | ||
| 13 | + | ||
| 14 | + context 'minor backtrace differences' do | ||
| 15 | + let(:notices) do | ||
| 16 | + line_numbers = [1, 1, 2, 2, 3] | ||
| 17 | + 5.times.map do | ||
| 18 | + b = backtrace.clone | ||
| 19 | + b.lines[5][:number] = line_numbers.shift | ||
| 20 | + b.save! | ||
| 21 | + notice = Fabricate(:notice, backtrace: b, app: app) | ||
| 22 | + notice.save! | ||
| 23 | + notice | ||
| 24 | + end | ||
| 25 | + end | ||
| 26 | + | ||
| 27 | + it 'has three problems for the five notices' do | ||
| 28 | + expect(Notice.count).to eq(5) | ||
| 29 | + expect(Problem.count).to eq(3) | ||
| 30 | + end | ||
| 31 | + | ||
| 32 | + it 'the problems have the right cached attributes' do | ||
| 33 | + problem = notices.first.reload.problem | ||
| 34 | + | ||
| 35 | + expect(problem.notices_count).to eq(2) | ||
| 36 | + end | ||
| 37 | + end | ||
| 38 | +end |