Merge pull request #1004 from stevecrozz/1001_truncate_long_notice_messages

Refs #1001 truncate long messages on cache

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