Commit d4e46b3dcad3008861d889ed9a48b815ef757bb0
1 parent
7bbbeef2
Exists in
master
and in
1 other branch
Validate required hoptoad notifier fields
Showing
8 changed files
with
83 additions
and
3 deletions
Show diff stats
Gemfile
Gemfile.lock
@@ -37,6 +37,10 @@ GEM | @@ -37,6 +37,10 @@ GEM | ||
37 | diff-lcs (1.1.2) | 37 | diff-lcs (1.1.2) |
38 | erubis (2.6.6) | 38 | erubis (2.6.6) |
39 | abstract (>= 1.0.0) | 39 | abstract (>= 1.0.0) |
40 | + factory_girl (1.3.2) | ||
41 | + factory_girl_rails (1.0) | ||
42 | + factory_girl (~> 1.3) | ||
43 | + rails (>= 3.0.0.beta4) | ||
40 | haml (3.0.15) | 44 | haml (3.0.15) |
41 | i18n (0.4.1) | 45 | i18n (0.4.1) |
42 | libxml-ruby (1.1.4) | 46 | libxml-ruby (1.1.4) |
@@ -101,6 +105,7 @@ PLATFORMS | @@ -101,6 +105,7 @@ PLATFORMS | ||
101 | DEPENDENCIES | 105 | DEPENDENCIES |
102 | bson_ext | 106 | bson_ext |
103 | database_cleaner (= 0.5.2) | 107 | database_cleaner (= 0.5.2) |
108 | + factory_girl_rails | ||
104 | haml | 109 | haml |
105 | libxml-ruby | 110 | libxml-ruby |
106 | mongoid (= 2.0.0.beta.15) | 111 | mongoid (= 2.0.0.beta.15) |
app/models/error.rb
@@ -2,8 +2,16 @@ class Error | @@ -2,8 +2,16 @@ class Error | ||
2 | include Mongoid::Document | 2 | include Mongoid::Document |
3 | include Mongoid::Timestamps | 3 | include Mongoid::Timestamps |
4 | 4 | ||
5 | + field :klass | ||
6 | + field :message | ||
7 | + field :component | ||
8 | + field :action | ||
9 | + field :environment | ||
10 | + | ||
5 | embeds_many :notices | 11 | embeds_many :notices |
6 | 12 | ||
13 | + validates_presence_of :klass, :environment | ||
14 | + | ||
7 | def self.for(attrs) | 15 | def self.for(attrs) |
8 | self.where(attrs).first || create(attrs) | 16 | self.where(attrs).first || create(attrs) |
9 | end | 17 | end |
app/models/notice.rb
@@ -11,11 +11,13 @@ class Notice | @@ -11,11 +11,13 @@ class Notice | ||
11 | 11 | ||
12 | embedded_in :error, :inverse_of => :notices | 12 | embedded_in :error, :inverse_of => :notices |
13 | 13 | ||
14 | + validates_presence_of :backtrace, :server_environment, :notifier | ||
15 | + | ||
14 | def self.from_xml(hoptoad_xml) | 16 | def self.from_xml(hoptoad_xml) |
15 | hoptoad_notice = Hoptoad::V2.parse_xml(hoptoad_xml) | 17 | hoptoad_notice = Hoptoad::V2.parse_xml(hoptoad_xml) |
16 | 18 | ||
17 | error = Error.for({ | 19 | error = Error.for({ |
18 | - :class_name => hoptoad_notice['error']['class'], | 20 | + :klass => hoptoad_notice['error']['class'], |
19 | :message => hoptoad_notice['error']['message'], | 21 | :message => hoptoad_notice['error']['message'], |
20 | :component => hoptoad_notice['request']['component'], | 22 | :component => hoptoad_notice['request']['component'], |
21 | :action => hoptoad_notice['request']['action'], | 23 | :action => hoptoad_notice['request']['action'], |
lib/hoptoad.rb
1 | module Hoptoad | 1 | module Hoptoad |
2 | module V2 | 2 | module V2 |
3 | + class ApiVersionError < StandardError | ||
4 | + def initialize | ||
5 | + super "Wrong API Version: Expecting v2.0" | ||
6 | + end | ||
7 | + end | ||
3 | 8 | ||
4 | def self.parse_xml(xml) | 9 | def self.parse_xml(xml) |
5 | parsed = ActiveSupport::XmlMini.backend.parse(xml)['notice'] | 10 | parsed = ActiveSupport::XmlMini.backend.parse(xml)['notice'] |
11 | + raise ApiVersionError unless parsed && parsed['version'] == '2.0' | ||
6 | rekey(parsed) | 12 | rekey(parsed) |
7 | end | 13 | end |
8 | 14 |
@@ -0,0 +1,24 @@ | @@ -0,0 +1,24 @@ | ||
1 | +Factory.define :error do |e| | ||
2 | + e.klass 'FooError' | ||
3 | + e.message 'FooError: Too Much Bar' | ||
4 | + e.component 'foo' | ||
5 | + e.action 'bar' | ||
6 | + e.environment 'production' | ||
7 | +end | ||
8 | + | ||
9 | +Factory.define :notice do |n| | ||
10 | + n.error {|e| e.association :error} | ||
11 | + n.backtrace { random_backtrace } | ||
12 | + n.server_environment 'server-environment' => 'production' | ||
13 | + n.notifier 'name' => 'Notifier', 'version' => '1', 'url' => 'http://toad.com' | ||
14 | +end | ||
15 | + | ||
16 | +def random_backtrace | ||
17 | + backtrace = [] | ||
18 | + 99.times {|t| backtrace << { | ||
19 | + 'number' => rand(999), | ||
20 | + 'file' => "/path/to/file/#{ActiveSupport::SecureRandom.hex(4)}.rb", | ||
21 | + 'method' => ActiveSupport.methods.shuffle.first | ||
22 | + }} | ||
23 | + backtrace | ||
24 | +end | ||
0 | \ No newline at end of file | 25 | \ No newline at end of file |
spec/models/error_spec.rb
@@ -2,10 +2,24 @@ require 'spec_helper' | @@ -2,10 +2,24 @@ require 'spec_helper' | ||
2 | 2 | ||
3 | describe Error do | 3 | describe Error do |
4 | 4 | ||
5 | + context 'validations' do | ||
6 | + it 'requires a klass' do | ||
7 | + error = Factory.build(:error, :klass => nil) | ||
8 | + error.should_not be_valid | ||
9 | + error.errors[:klass].should include("can't be blank") | ||
10 | + end | ||
11 | + | ||
12 | + it 'requires an environment' do | ||
13 | + error = Factory.build(:error, :environment => nil) | ||
14 | + error.should_not be_valid | ||
15 | + error.errors[:environment].should include("can't be blank") | ||
16 | + end | ||
17 | + end | ||
18 | + | ||
5 | context '#for' do | 19 | context '#for' do |
6 | before do | 20 | before do |
7 | @conditions = { | 21 | @conditions = { |
8 | - :class_name => 'Whoops', | 22 | + :klass => 'Whoops', |
9 | :message => 'Whoops: Oopsy Daisy', | 23 | :message => 'Whoops: Oopsy Daisy', |
10 | :component => 'Foo', | 24 | :component => 'Foo', |
11 | :action => 'bar', | 25 | :action => 'bar', |
spec/models/notice_spec.rb
@@ -2,6 +2,26 @@ require 'spec_helper' | @@ -2,6 +2,26 @@ require 'spec_helper' | ||
2 | 2 | ||
3 | describe Notice do | 3 | describe Notice do |
4 | 4 | ||
5 | + context 'validations' do | ||
6 | + it 'requires a backtrace' do | ||
7 | + notice = Factory.build(:notice, :backtrace => nil) | ||
8 | + notice.should_not be_valid | ||
9 | + notice.errors[:backtrace].should include("can't be blank") | ||
10 | + end | ||
11 | + | ||
12 | + it 'requires the server_environment' do | ||
13 | + notice = Factory.build(:notice, :server_environment => nil) | ||
14 | + notice.should_not be_valid | ||
15 | + notice.errors[:server_environment].should include("can't be blank") | ||
16 | + end | ||
17 | + | ||
18 | + it 'requires the notifier' do | ||
19 | + notice = Factory.build(:notice, :notifier => nil) | ||
20 | + notice.should_not be_valid | ||
21 | + notice.errors[:notifier].should include("can't be blank") | ||
22 | + end | ||
23 | + end | ||
24 | + | ||
5 | context '#from_xml' do | 25 | context '#from_xml' do |
6 | before do | 26 | before do |
7 | @xml = Rails.root.join('spec','fixtures','hoptoad_test_notice.xml').read | 27 | @xml = Rails.root.join('spec','fixtures','hoptoad_test_notice.xml').read |
@@ -9,7 +29,7 @@ describe Notice do | @@ -9,7 +29,7 @@ describe Notice do | ||
9 | 29 | ||
10 | it 'finds the correct error for the notice' do | 30 | it 'finds the correct error for the notice' do |
11 | Error.should_receive(:for).with({ | 31 | Error.should_receive(:for).with({ |
12 | - :class_name => 'HoptoadTestingException', | 32 | + :klass => 'HoptoadTestingException', |
13 | :message => 'HoptoadTestingException: Testing hoptoad via "rake hoptoad:test". If you can see this, it works.', | 33 | :message => 'HoptoadTestingException: Testing hoptoad via "rake hoptoad:test". If you can see this, it works.', |
14 | :component => 'application', | 34 | :component => 'application', |
15 | :action => 'verify', | 35 | :action => 'verify', |