Commit 8648074489b14fc93049935c50affb180b18ab60
1 parent
1453287a
Exists in
master
Refs #1001 truncate long messages on cache
Long messages need to be truncated so that Errbit will not try to cache messages that are too long on Problem records because the mongo text index will complain if the text is too long. Also, it's a good idea to have these kinds of limits in general.
Showing
4 changed files
with
41 additions
and
6 deletions
Show diff stats
app/models/notice.rb
1 | require 'recurse' | 1 | require 'recurse' |
2 | 2 | ||
3 | class Notice | 3 | class Notice |
4 | + MESSAGE_LENGTH_LIMIT = 1000 | ||
5 | + | ||
4 | include Mongoid::Document | 6 | include Mongoid::Document |
5 | include Mongoid::Timestamps | 7 | include Mongoid::Timestamps |
6 | 8 | ||
@@ -32,6 +34,12 @@ class Notice | @@ -32,6 +34,12 @@ class Notice | ||
32 | where(:err_id.in => errs.all.map(&:id)) | 34 | where(:err_id.in => errs.all.map(&:id)) |
33 | } | 35 | } |
34 | 36 | ||
37 | + # Overwrite the default setter to make sure the message length is no longer | ||
38 | + # than the limit we impose | ||
39 | + def message=(m) | ||
40 | + super(m.is_a?(String) ? m[0, MESSAGE_LENGTH_LIMIT] : m) | ||
41 | + end | ||
42 | + | ||
35 | def user_agent | 43 | def user_agent |
36 | agent_string = env_vars['HTTP_USER_AGENT'] | 44 | agent_string = env_vars['HTTP_USER_AGENT'] |
37 | agent_string.blank? ? nil : UserAgent.parse(agent_string) | 45 | agent_string.blank? ? nil : UserAgent.parse(agent_string) |
app/models/problem.rb
@@ -55,7 +55,6 @@ class Problem | @@ -55,7 +55,6 @@ class Problem | ||
55 | validates :last_notice_at, :first_notice_at, presence: true | 55 | validates :last_notice_at, :first_notice_at, presence: true |
56 | 56 | ||
57 | before_create :cache_app_attributes | 57 | before_create :cache_app_attributes |
58 | - before_save :truncate_message | ||
59 | 58 | ||
60 | scope :resolved, -> { where(resolved: true) } | 59 | scope :resolved, -> { where(resolved: true) } |
61 | scope :unresolved, -> { where(resolved: false) } | 60 | scope :unresolved, -> { where(resolved: false) } |
@@ -223,10 +222,6 @@ class Problem | @@ -223,10 +222,6 @@ class Problem | ||
223 | self.app_name = app.name if app | 222 | self.app_name = app.name if app |
224 | end | 223 | end |
225 | 224 | ||
226 | - def truncate_message | ||
227 | - self.message = message[0, 1000] if message | ||
228 | - end | ||
229 | - | ||
230 | def issue_type | 225 | def issue_type |
231 | # Return issue_type if configured, but fall back to detecting app's issue tracker | 226 | # Return issue_type if configured, but fall back to detecting app's issue tracker |
232 | attributes['issue_type'] ||= | 227 | attributes['issue_type'] ||= |
spec/models/notice_spec.rb
@@ -19,6 +19,38 @@ describe Notice, type: 'model' do | @@ -19,6 +19,38 @@ describe Notice, type: 'model' do | ||
19 | end | 19 | end |
20 | end | 20 | end |
21 | 21 | ||
22 | + describe '#message=' do | ||
23 | + let(:long_message) do | ||
24 | + 'Presently I heard a slight groan, and I knew it was the groan of ' \ | ||
25 | + 'mortal terror. It was not a groan of pain or of grief --oh, no! ' \ | ||
26 | + '--it was the low stifled sound that arises from the bottom of the ' \ | ||
27 | + 'soul when overcharged with awe. I knew the sound well. Many a ' \ | ||
28 | + 'night, just at midnight, when all the world slept, it has welled ' \ | ||
29 | + 'up from my own bosom, deepening, with its dreadful echo, the ' \ | ||
30 | + 'terrors that distracted me. I say I knew it well. I knew what the ' \ | ||
31 | + 'old man felt, and pitied him, although I chuckled at heart. I ' \ | ||
32 | + 'knew that he had been lying awake ever since the first slight ' \ | ||
33 | + 'noise, when he had turned in the bed. His fears had been ever ' \ | ||
34 | + 'since growing upon him. He had been trying to fancy them ' \ | ||
35 | + 'causeless, but could not. He had been saying to himself --"It is ' \ | ||
36 | + 'nothing but the wind in the chimney --it is only a mouse crossing ' \ | ||
37 | + 'the floor," or "It is merely a cricket which has made a single ' \ | ||
38 | + 'chirp." Yes, he had been trying to comfort himself with these ' \ | ||
39 | + 'suppositions: but he had found all in vain. All in vain; because ' \ | ||
40 | + 'Death, in approaching him had stalked with his black shadow ' \ | ||
41 | + 'before him, and enveloped the victim. And it was the mournful ' \ | ||
42 | + 'influence of the unperceived shadow that caused him to feel ' \ | ||
43 | + '--although he neither saw nor heard --to feel the presence of my ' \ | ||
44 | + 'head within the room. ' | ||
45 | + end | ||
46 | + | ||
47 | + it 'truncates the message' do | ||
48 | + notice = Fabricate(:notice, message: long_message) | ||
49 | + expect(long_message.length).to be > 1000 | ||
50 | + expect(notice.message.length).to eq 1000 | ||
51 | + end | ||
52 | + end | ||
53 | + | ||
22 | describe "key sanitization" do | 54 | describe "key sanitization" do |
23 | before do | 55 | before do |
24 | @hash = { "some.key" => { "$nested.key" => { "$Path" => "/", "some$key" => "key" } } } | 56 | @hash = { "some.key" => { "$nested.key" => { "$Path" => "/", "some$key" => "key" } } } |
spec/models/user_spec.rb
@@ -42,7 +42,7 @@ describe User do | @@ -42,7 +42,7 @@ describe User do | ||
42 | expect(user.reset_password('Password123', 'Password123')).to be_truthy | 42 | expect(user.reset_password('Password123', 'Password123')).to be_truthy |
43 | end | 43 | end |
44 | 44 | ||
45 | - it 'should require a password with minimum of 6 characters' do | 45 | + it 'should require a password with minimum of 6 characters' do |
46 | user = Fabricate.build(:user) | 46 | user = Fabricate.build(:user) |
47 | user.reset_password('12345', '12345') | 47 | user.reset_password('12345', '12345') |
48 | expect(user.errors[:password]).to include("is too short (minimum is 6 characters)", "is too short (minimum is 6 characters)") | 48 | expect(user.errors[:password]).to include("is too short (minimum is 6 characters)", "is too short (minimum is 6 characters)") |