From 89fd0c29956296bbc065d48af9973f93b6da20d9 Mon Sep 17 00:00:00 2001 From: Seth Call Date: Wed, 8 Jan 2014 03:28:15 +0000 Subject: [PATCH] * VRFS-971- fixed duplicate recorded_tracks being sent as a result of list_downloads; query much more efficient too --- ruby/lib/jam_ruby/models/recorded_track.rb | 10 +-- ruby/lib/jam_ruby/models/recording.rb | 70 ++++++++----------- ruby/spec/jam_ruby/models/mix_spec.rb | 3 +- .../jam_ruby/models/recorded_track_spec.rb | 2 +- ruby/spec/jam_ruby/models/recording_spec.rb | 1 + 5 files changed, 40 insertions(+), 46 deletions(-) diff --git a/ruby/lib/jam_ruby/models/recorded_track.rb b/ruby/lib/jam_ruby/models/recorded_track.rb index f56bbd335..53302a141 100644 --- a/ruby/lib/jam_ruby/models/recorded_track.rb +++ b/ruby/lib/jam_ruby/models/recorded_track.rb @@ -86,7 +86,7 @@ module JamRuby recorded_track.next_part_to_upload = 0 recorded_track.file_offset = 0 recorded_track.save - recorded_track.url = construct_filename(recording.id, track.id) + recorded_track.url = construct_filename(recording.id, track.client_track_id) recorded_track.save recorded_track end @@ -160,12 +160,12 @@ module JamRuby def filename # construct a path for s3 - RecordedTrack.construct_filename(self.recording.id, self.track_id) + RecordedTrack.construct_filename(self.recording.id, self.client_track_id) end - def self.construct_filename(recording_id, track_id) - raise "unknown ID" unless track_id - "recordings/#{recording_id}/track-#{track_id}.ogg" + def self.construct_filename(recording_id, client_track_id) + raise "unknown ID" unless client_track_id + "recordings/#{recording_id}/track-#{client_track_id}.ogg" end end end diff --git a/ruby/lib/jam_ruby/models/recording.rb b/ruby/lib/jam_ruby/models/recording.rb index ac1fa2f29..f96d4e23d 100644 --- a/ruby/lib/jam_ruby/models/recording.rb +++ b/ruby/lib/jam_ruby/models/recording.rb @@ -163,53 +163,45 @@ module JamRuby # ":recordings =>" part, you'll just get the recorded_tracks that I played. Very different! # we also only allow you to be told about downloads if you have claimed the recording - User.joins(:recordings).joins(:recordings => :recorded_tracks).joins(:recordings => :claimed_recordings) - .order(%Q{ recorded_tracks.id }) - .where(%Q{ recorded_tracks.fully_uploaded = TRUE }) + #User.joins(:recordings).joins(:recordings => :recorded_tracks).joins(:recordings => :claimed_recordings) + RecordedTrack.joins(:recording).joins(:recording => :claimed_recordings) + .order('recorded_tracks.id') + .where('recorded_tracks.fully_uploaded = TRUE') .where('recorded_tracks.id > ?', since) - .where('claimed_recordings.user_id = ?', user) - .where(:id => user.id).limit(limit).each do |theuser| - theuser.recordings.each do |recording| - recording.recorded_tracks.each do |recorded_track| - downloads.push( - { - :type => "recorded_track", - :id => recorded_track.client_track_id, - :recording_id => recording.id, - :length => recorded_track.length, - :md5 => recorded_track.md5, - :url => recorded_track.url, - :next => recorded_track.id - } - ) - end - end + .where('claimed_recordings.user_id = ?', user).limit(limit).each do |recorded_track| + downloads.push( + { + :type => "recorded_track", + :id => recorded_track.client_track_id, + :recording_id => recorded_track.recording_id, + :length => recorded_track.length, + :md5 => recorded_track.md5, + :url => recorded_track.url, + :next => recorded_track.id + } + ) end latest_recorded_track = downloads[-1][:next] if downloads.length > 0 - User.joins(:recordings).joins(:recordings => :mixes) + Mix.joins(:recording).joins(:recording => :claimed_recordings) .order('mixes.id') .where('mixes.completed_at IS NOT NULL') .where('mixes.id > ?', since) - .where(:id => user.id) - .limit(limit).each do |theuser| - theuser.recordings.each do |recording| - recording.mixes.each do |mix| - downloads.push( - { - :type => "mix", - :id => mix.id, - :recording_id => recording.id, - :length => mix.length, - :md5 => mix.md5, - :url => mix.url, - :created_at => mix.created_at, - :next => mix.id - } - ) - end - end + .where('claimed_recordings.user_id = ?', user) + .limit(limit).each do |mix| + downloads.push( + { + :type => "mix", + :id => mix.id, + :recording_id => mix.recording_id, + :length => mix.length, + :md5 => mix.md5, + :url => mix.url, + :created_at => mix.created_at, + :next => mix.id + } + ) end latest_mix = downloads[-1][:next] if downloads.length > 0 diff --git a/ruby/spec/jam_ruby/models/mix_spec.rb b/ruby/spec/jam_ruby/models/mix_spec.rb index 3c902b492..2053bf1c8 100755 --- a/ruby/spec/jam_ruby/models/mix_spec.rb +++ b/ruby/spec/jam_ruby/models/mix_spec.rb @@ -11,7 +11,8 @@ describe Mix do @music_session.save @recording = Recording.start(@music_session, @user) @recording.stop - @recording.claim(@user, "name", "description", Genre.first, true, true) + @recording.claim(@user, "name", "description", Genre.first, true, true) + @recording.errors.any?.should be_false @mix = Mix.schedule(@recording, "{}") end diff --git a/ruby/spec/jam_ruby/models/recorded_track_spec.rb b/ruby/spec/jam_ruby/models/recorded_track_spec.rb index e77ae5e92..ed2b74701 100644 --- a/ruby/spec/jam_ruby/models/recorded_track_spec.rb +++ b/ruby/spec/jam_ruby/models/recorded_track_spec.rb @@ -42,7 +42,7 @@ describe RecordedTrack do it "gets a url for the track" do @recorded_track = RecordedTrack.create_from_track(@track, @recording) @recorded_track.save.should be_true - @recorded_track.url.should == "recordings/#{@recording.id}/track-#{@track.id}.ogg" + @recorded_track.url.should == "recordings/#{@recording.id}/track-#{@track.client_track_id}.ogg" end it "signs url" do diff --git a/ruby/spec/jam_ruby/models/recording_spec.rb b/ruby/spec/jam_ruby/models/recording_spec.rb index 6cbb0147c..7f34bb2c3 100644 --- a/ruby/spec/jam_ruby/models/recording_spec.rb +++ b/ruby/spec/jam_ruby/models/recording_spec.rb @@ -230,6 +230,7 @@ describe Recording do downloads["downloads"].length.should == 1 end + it "should return a file list for a user properly" do pending stub_const("APP_CONFIG", app_config)