diff --git a/app/finders/message_finder.rb b/app/finders/message_finder.rb index bf82d61d5..915019f43 100644 --- a/app/finders/message_finder.rb +++ b/app/finders/message_finder.rb @@ -1,4 +1,6 @@ class MessageFinder + MESSAGE_ID_MAX = 2_147_483_647 + def initialize(conversation, params) @conversation = conversation @params = params @@ -21,12 +23,14 @@ class MessageFinder end def current_messages + return messages.none if oversized_message_id?(@params[:after]) + if @params[:after].present? && @params[:before].present? - messages_between(@params[:after].to_i, @params[:before].to_i) + messages_between(normalized_message_id(@params[:after]), @params[:before].to_i) elsif @params[:before].present? messages_before(@params[:before].to_i) elsif @params[:after].present? - messages_after(@params[:after].to_i) + messages_after(normalized_message_id(@params[:after])) else messages_latest end @@ -37,16 +41,29 @@ class MessageFinder end def messages_before(before_id) + return messages_latest if oversized_message_id?(before_id) + + before_id = normalized_message_id(before_id) messages.reorder('created_at desc').where('id < ?', before_id).limit(20).reverse end def messages_between(after_id, before_id) - messages.reorder('created_at asc').where('id >= ? AND id < ?', after_id, before_id).limit(1000) + message_scope = messages.reorder('created_at asc').where('id >= ?', after_id) + message_scope = message_scope.where('id < ?', normalized_message_id(before_id)) unless oversized_message_id?(before_id) + message_scope.limit(1000) end def messages_latest messages.reorder('created_at desc').limit(20).reverse end + + def normalized_message_id(value) + value.to_i.clamp(0, MESSAGE_ID_MAX) + end + + def oversized_message_id?(value) + value.to_i > MESSAGE_ID_MAX + end end MessageFinder.prepend_mod_with('MessageFinder') diff --git a/spec/finders/message_finder_spec.rb b/spec/finders/message_finder_spec.rb index 04d6e2df9..0c1e7a2cd 100644 --- a/spec/finders/message_finder_spec.rb +++ b/spec/finders/message_finder_spec.rb @@ -48,6 +48,17 @@ describe MessageFinder do end end + context 'with a before attribute above the message id range' do + let!(:max_id_message) do + create(:message, id: described_class::MESSAGE_ID_MAX, account: account, inbox: inbox, conversation: conversation) + end + let(:params) { { before: 4_611_686_018_427_387_903 } } + + it 'includes the maximum valid message id without overflowing the database column' do + expect(message_finder.perform).to include(max_id_message) + end + end + context 'with after attribute' do let(:params) { { after: conversation.messages.first.id } } @@ -59,6 +70,14 @@ describe MessageFinder do end end + context 'with an after attribute above the message id range' do + let(:params) { { after: 881_965_304_328 } } + + it 'returns no messages without overflowing the database column' do + expect(message_finder.perform).to be_empty + end + end + context 'with after and before attribute' do let(:params) do { @@ -73,5 +92,24 @@ describe MessageFinder do expect(result.last.id).to be conversation.messages[-2].id end end + + context 'with after and before attributes above the message id range' do + let!(:max_id_message) do + create(:message, id: described_class::MESSAGE_ID_MAX, account: account, inbox: inbox, conversation: conversation) + end + let(:params) do + { + after: 881_965_304_328, + before: 4_611_686_018_427_387_903 + } + end + + it 'returns no messages' do + result = message_finder.perform + + expect(result).to be_empty + expect(result).not_to include(max_id_message) + end + end end end