Merge pull request #1474 from Shopify/master-vs-stable

Cherry-pick 1-x-stable changes into master

lib/resque/failure/base.rb

@@ -53,15 +53,15 @@ def self.each(*args)
       # A URL where someone can go to view failures.
       def self.url
       end
-      
+
       # Clear all failure objects
       def self.clear(*args)
       end
-      
-      def self.requeue(index)
+
+      def self.requeue(*args)
       end
 
-      def self.remove(index)
+      def self.remove(*args)
       end
 
       # Logging!

lib/resque/failure/redis.rb

@@ -48,14 +48,18 @@ def self.all(offset = 0, limit = 1, queue = nil)
       end
 
       def self.each(offset = 0, limit = self.count, queue = :failed, class_name = nil, order = 'desc')
+        if class_name
+          original_limit = limit
+          limit = count
+        end
         all_items = limit == 1 ? [all(offset,limit,queue)] : Array(all(offset, limit, queue))
         reversed = false
         if order.eql? 'desc'
           all_items.reverse!
           reversed = true
         end
         all_items.each_with_index do |item, i|
-          if !class_name || (item['payload'] && item['payload']['class'] == class_name)
+          if !class_name || (item['payload'] && item['payload']['class'] == class_name && (original_limit -= 1) >= 0)
             if reversed
               id = (all_items.length - 1) + (offset - i)
             else

lib/resque/server/helpers.rb

@@ -18,7 +18,7 @@ def failed_size
 
   def failed_per_page
     @failed_per_page = if params[:class]
-      failed_size  
+      failed_size
     else
       20
     end
@@ -49,4 +49,16 @@ def failed_class_counts(queue = params[:queue])
     end
     classes
   end
+
+  def page_entries_info(start, stop, size, name = nil)
+    if size == 0
+      name ? "No #{name}s" : '<b>0</b>'
+    elsif size == 1
+      'Showing <b>1</b>' + (name ? " #{name}" : '')
+    elsif size > failed_per_page
+      "Showing #{start}-#{stop} of <b>#{size}</b>" + (name ? " #{name}s" : '')
+    else
+      "Showing #{start} to <b>#{size - 1}</b>" + (name ? " #{name}s" : '')
+    end
+  end
 end

lib/resque/server/views/failed.erb

@@ -16,7 +16,7 @@
 <% if failed_multiple_queues? && !params[:queue] %>
 <%= partial :failed_queues_overview %>
 <% else %>
-<p class='sub'>Showing <%= failed_start_at %> to <%= failed_end_at %> of <b><%= failed_size %></b> jobs</p>
+<p class='sub'><%= page_entries_info failed_start_at, failed_end_at, failed_size, 'job' %></p>
 
 
 <ul class='failed'>
@@ -25,5 +25,5 @@
   <% end %>
 </ul>
 
-<%= partial :next_more, :start => failed_start_at, :size => failed_size, :per_page => failed_per_page %>
+<%= partial :next_more, :start => failed_start_at, :size => failed_size, :per_page => failed_per_page if failed_size > 0 %>
 <% end %>

lib/resque/server/views/key_sets.erb

@@ -1,8 +1,6 @@
 <% if key = params[:key] %>
 
-  <p class='sub'>
-    Showing <%= start = params[:start].to_i %> to <%= start + 20 %> of <b><%=size = redis_get_size(key) %></b>
-  </p>
+  <p class='sub'><%= page_entries_info start = params[:start].to_i, start + 20, size = redis_get_size(key) %></p>
 
   <h1>Key "<%= key %>" is a <%= resque.redis.type key %></h1>
   <table>

lib/resque/server/views/queues.erb

@@ -6,7 +6,7 @@
   <form method="POST" action="<%=u "/queues/#{queue}/remove" %>" class='remove-queue'>
     <input type='submit' name='' value='Remove Queue' onclick='return confirm("Are you absolutely sure? This cannot be undone.");' />
   </form>
-  <p class='sub'>Showing <%= start = params[:start].to_i %> to <%= start + 20 %> of <b><%=size = resque.size(queue)%></b> jobs</p>
+  <p class='sub'><%= page_entries_info start = params[:start].to_i, start + 19, size = resque.size(queue), 'job' %></p>
   <table class='jobs'>
     <tr>
       <th>Class</th>

test/resque_failure_redis_test.rb

@@ -2,13 +2,14 @@
 require 'resque/failure/redis'
 
 describe "Resque::Failure::Redis" do
+  let(:bad_string) { [39, 52, 127, 86, 93, 95, 39].map { |c| c.chr }.join }
+  let(:exception)  { StandardError.exception(bad_string) }
+  let(:worker)     { Resque::Worker.new(:test) }
+  let(:queue)      { "queue" }
+  let(:payload)    { { "class" => Object, "args" => 3 } }
+
   before do
     Resque::Failure::Redis.clear
-    @bad_string    = [39, 52, 127, 86, 93, 95, 39].map { |c| c.chr }.join
-    exception      = StandardError.exception(@bad_string)
-    worker         = Resque::Worker.new(:test)
-    queue          = "queue"
-    payload        = { "class" => Object, "args" => 3 }
     @redis_backend = Resque::Failure::Redis.new(exception, worker, queue, payload)
   end
 
@@ -48,4 +49,53 @@
     assert_equal 2, num_iterations
   end
 
+  it '.each should limit based on the class_name when class_name is specified' do
+    num_iterations = 0
+    class_one = 'Foo'
+    class_two = 'Bar'
+    [ class_one,
+      class_two,
+      class_one,
+      class_two,
+      class_one,
+      class_two
+    ].each do |class_name|
+      Resque::Failure::Redis.new(exception, worker, queue, payload.merge({ "class" => class_name })).save
+    end
+    # ensure that there are 6 failed jobs in total as configured
+    assert_equal 6, Resque::Failure::Redis.count
+    Resque::Failure::Redis.each 0, 2, nil, class_one do |id, item|
+      num_iterations += 1
+      # ensure it iterates only jobs with the specified class name (it was not
+      # which cause we only got 1 job with class=Foo since it iterates all the
+      # jobs and limit already reached)
+      assert_equal class_one, item['payload']['class']
+    end
+    # ensure only iterates max up to the limit specified
+    assert_equal 2, num_iterations
+  end
+
+  it '.each should limit normally when class_name is not specified' do
+    num_iterations = 0
+    class_one = 'Foo'
+    class_two = 'Bar'
+    [ class_one,
+      class_two,
+      class_one,
+      class_two,
+      class_one,
+      class_two
+    ].each do |class_name|
+      Resque::Failure::Redis.new(exception, worker, queue, payload.merge({ "class" => class_name })).save
+    end
+    # ensure that there are 6 failed jobs in total as configured
+    assert_equal 6, Resque::Failure::Redis.count
+    Resque::Failure::Redis.each 0, 5 do |id, item|
+      num_iterations += 1
+      assert_equal Hash, item.class
+    end
+    # ensure only iterates max up to the limit specified
+    assert_equal 5, num_iterations
+  end
+
 end