Merge pull request #222 from claudiofullscreen/fix-infinite-loop-for-…

…eager-loading

Fix infinite loop for eager loading

CHANGELOG.md

@@ -6,6 +6,10 @@ For more information about changelogs, check
 [Keep a Changelog](http://keepachangelog.com) and
 [Vandamme](http://tech-angels.github.io/vandamme).
 
+## 0.25.3 - 2015-07-23
+
+* [BUGFIX] Don’t run an infinite loop when calling `.playlist_items.includes(:video)` on a playlist with only private or deleted videos
+
 ## 0.25.2 - 2015-07-22
 
 * [FEATURE] Add .includes(:video) to .playlist_items to eager-load video data of a list of playlist items.

README.md

@@ -41,7 +41,7 @@ To install on your system, run
 
 To use inside a bundled Ruby project, add this line to the Gemfile:
 
-    gem 'yt', '~> 0.25.2'
+    gem 'yt', '~> 0.25.3'
 
 Since the gem follows [Semantic Versioning](http://semver.org),
 indicating the full version in your Gemfile (~> *major*.*minor*.*patch*)

lib/yt/collections/videos.rb

@@ -76,6 +76,7 @@ def list_params
 
       def next_page
         super.tap do |items|
+          halt_list if use_list_endpoint? && items.empty? && @page_token.nil?
           add_offset_to(items) if !use_list_endpoint? && @page_token.nil? && videos_params[:order] == 'date'
         end
       end
@@ -91,6 +92,18 @@ def add_offset_to(items)
         end
       end
 
+      # If we ask for a list of videos matching specific IDs and no video is
+      # returned (e.g. they are all private/deleted), then we don’t want to
+      # switch from /videos to /search and keep on looking for videos, but
+      # simply return an empty array of items
+      def halt_list
+        @halt_list = true
+      end
+
+      def more_pages?
+        (@last_index.zero? && !@halt_list) || !@page_token.nil?
+      end
+
       def videos_params
         {}.tap do |params|
           params[:type] = :video

lib/yt/version.rb

@@ -1,3 +1,3 @@
 module Yt
-  VERSION = '0.25.2'
+  VERSION = '0.25.3'
 end

spec/requests/as_account/channel_spec.rb

@@ -72,7 +72,7 @@
       end
 
       describe 'when the channel has more than 500 videos' do
-        let(:id) { 'UCsmvakQZlvGsyjyOhmhvOsw' }
+        let(:id) { 'UC0v-tlzsn0QZwJnkiaUSJVQ' }
 
         specify 'the estimated and actual number of videos can be retrieved' do
           # @note: in principle, the following three counters should match, but

spec/requests/as_account/playlist_spec.rb

@@ -47,6 +47,18 @@
     end
   end
 
+  context 'given a playlist that only includes other people’s private or deleted videos' do
+    let(:id) { 'PLsnYEvcCzABOsJdehqkIDhwz8CPGWzX59' }
+
+    describe '.playlist_items.includes(:video)' do
+      let(:items) { playlist.playlist_items.includes(:video).map{|i| i} }
+
+      specify 'returns nil (without running an infinite loop)' do
+        expect(items.size).to be 2
+      end
+    end
+  end
+
   context 'given an unknown playlist' do
     let(:id) { 'not-a-playlist-id' }
 

spec/requests/as_content_owner/channel_spec.rb

@@ -711,7 +711,7 @@
       describe 'shares can be retrieved for a single country' do
         let(:country_code) { 'US' }
         let(:shares) { channel.shares since: date, by: by, in: location }
-        let(:date) { 4.days.ago }
+        let(:date) { ENV['YT_TEST_PARTNER_VIDEO_DATE'] }
 
         context 'and grouped by day' do
           let(:by) { :day }
@@ -753,7 +753,7 @@
       end
 
       describe 'shares can be grouped by country' do
-        let(:range) { {since: 4.days.ago, until: 3.days.ago} }
+        let(:range) { {since: ENV['YT_TEST_PARTNER_VIDEO_DATE']} }
 
         specify 'with the :by option set to :country' do
           shares = channel.shares range.merge by: :country
@@ -876,7 +876,7 @@
       describe 'favorites added can be retrieved for a single country' do
         let(:country_code) { 'US' }
         let(:favorites_added) { channel.favorites_added since: date, by: by, in: location }
-        let(:date) { 4.days.ago }
+        let(:date) { ENV['YT_TEST_PARTNER_VIDEO_DATE'] }
 
         context 'and grouped by day' do
           let(:by) { :day }

spec/requests/as_content_owner/playlist_spec.rb

@@ -614,7 +614,7 @@
       end
 
       describe 'playlist starts can be grouped by country' do
-        let(:range) { {since: 4.days.ago, until: 3.days.ago} }
+        let(:range) { {since: ENV['YT_TEST_PARTNER_PLAYLIST_DATE']} }
 
         specify 'with the :by option set to :country' do
           starts = playlist.playlist_starts range.merge by: :country
@@ -680,7 +680,7 @@
       end
 
       describe 'average time in playlist can be grouped by country' do
-        let(:range) { {since: 4.days.ago, until: 3.days.ago} }
+        let(:range) { {since: ENV['YT_TEST_PARTNER_PLAYLIST_DATE']} }
 
         specify 'with the :by option set to :country' do
           time = playlist.average_time_in_playlist range.merge by: :country
@@ -746,7 +746,7 @@
       end
 
       describe 'views per playlist start can be grouped by country' do
-        let(:range) { {since: 4.days.ago, until: 3.days.ago} }
+        let(:range) { {since: ENV['YT_TEST_PARTNER_PLAYLIST_DATE']} }
 
         specify 'with the :by option set to :country' do
           views = playlist.views_per_playlist_start range.merge by: :country