diff --git a/db/manifest b/db/manifest index 6342f4c83..c8652bebf 100755 --- a/db/manifest +++ b/db/manifest @@ -224,3 +224,4 @@ emails_from_update2.sql add_youtube_flag_to_claimed_recordings.sql add_session_create_type.sql user_syncs_and_quick_mix.sql +user_syncs_fix_dup_tracks_2408.sql diff --git a/db/up/user_syncs_fix_dup_tracks_2408.sql b/db/up/user_syncs_fix_dup_tracks_2408.sql new file mode 100644 index 000000000..2d98e8940 --- /dev/null +++ b/db/up/user_syncs_fix_dup_tracks_2408.sql @@ -0,0 +1,32 @@ +DROP VIEW user_syncs; + +CREATE VIEW user_syncs AS + SELECT DISTINCT b.id AS recorded_track_id, + CAST(NULL as BIGINT) AS mix_id, + CAST(NULL as BIGINT) as quick_mix_id, + b.id AS unified_id, + a.user_id AS user_id, + b.fully_uploaded, + recordings.created_at AS created_at, + recordings.id AS recording_id + FROM recorded_tracks a INNER JOIN recordings ON a.recording_id = recordings.id AND duration IS NOT NULL AND all_discarded = FALSE INNER JOIN recorded_tracks b ON a.recording_id = b.recording_id + UNION ALL + SELECT CAST(NULL as BIGINT) AS recorded_track_id, + mixes.id AS mix_id, + CAST(NULL as BIGINT) AS quick_mix_id, + mixes.id AS unified_id, + claimed_recordings.user_id AS user_id, + NULL as fully_uploaded, + recordings.created_at AS created_at, + recordings.id AS recording_id + FROM mixes INNER JOIN recordings ON mixes.recording_id = recordings.id INNER JOIN claimed_recordings ON recordings.id = claimed_recordings.recording_id WHERE claimed_recordings.discarded = FALSE + UNION ALL + SELECT CAST(NULL as BIGINT) AS recorded_track_id, + CAST(NULL as BIGINT) AS mix_id, + quick_mixes.id AS quick_mix_id, + quick_mixes.id AS unified_id, + quick_mixes.user_id, + quick_mixes.fully_uploaded, + recordings.created_at AS created_at, + recordings.id AS recording_id + FROM quick_mixes INNER JOIN recordings ON quick_mixes.recording_id = recordings.id AND duration IS NOT NULL AND all_discarded = FALSE; diff --git a/ruby/lib/jam_ruby/resque/audiomixer.rb b/ruby/lib/jam_ruby/resque/audiomixer.rb index 1b9065d3d..66973b12a 100644 --- a/ruby/lib/jam_ruby/resque/audiomixer.rb +++ b/ruby/lib/jam_ruby/resque/audiomixer.rb @@ -206,6 +206,18 @@ module JamRuby end + def cleanup_files() + File.delete(@output_ogg_filename) if File.exists(@output_ogg_filename) + File.delete(@output_mp3_filename) if File.exists(@output_mp3_filename) + File.delete(@manifest_file) if File.exists(@manifest_file) + File.delete(@error_out_filename) if File.exists(@error_out_filename) + + @manifest[:files].each do |file| + filename = file[:filename] + File.delete(filename) if File.exists(filename) + end + end + def post_success(mix) ogg_length = File.size(@output_ogg_filename) @@ -220,6 +232,7 @@ module JamRuby mix.finish(ogg_length, ogg_md5.to_s, mp3_length, mp3_md5.to_s) end + def post_error(mix, e) begin @@ -265,6 +278,9 @@ module JamRuby post_success(mix) + # only cleanup files if we manage to get this far + cleanup_files + @@log.info("audiomixer job successful. mix_id #{mix_id}") rescue Exception => e diff --git a/ruby/lib/jam_ruby/resque/quick_mixer.rb b/ruby/lib/jam_ruby/resque/quick_mixer.rb index 4f949c6d3..f9b9d80bb 100644 --- a/ruby/lib/jam_ruby/resque/quick_mixer.rb +++ b/ruby/lib/jam_ruby/resque/quick_mixer.rb @@ -63,6 +63,8 @@ module JamRuby post_success(@quick_mix) + cleanup_files + @@log.info("audiomixer job successful. mix_id #{quick_mix_id}") rescue Exception => e @@ -121,6 +123,11 @@ module JamRuby end end + def cleanup_files() + File.delete(@input_ogg_filename) if File.exists(@input_ogg_filename) + File.delete(@output_mp3_filename) if File.exists(@output_mp3_filename) + end + def fetch_audio_files @input_ogg_filename = Dir::Tmpname.make_tmpname( ["#{Dir.tmpdir}/quick_mixer_#{@quick_mix.id}}", '.ogg'], nil) diff --git a/ruby/spec/jam_ruby/models/user_sync_spec.rb b/ruby/spec/jam_ruby/models/user_sync_spec.rb index 4781200b5..349cf706e 100644 --- a/ruby/spec/jam_ruby/models/user_sync_spec.rb +++ b/ruby/spec/jam_ruby/models/user_sync_spec.rb @@ -167,6 +167,54 @@ describe UserSync do user_syncs.length.should == 0 end end + + describe "one recording with multi-track users" do + let!(:recording1) { + recording = FactoryGirl.create(:recording, owner: user1, band: nil, duration:1) + recording.recorded_tracks << FactoryGirl.create(:recorded_track, recording: recording, user: recording.owner) + recording.recorded_tracks << FactoryGirl.create(:recorded_track, recording: recording, user: recording.owner) + recording.recorded_tracks << FactoryGirl.create(:recorded_track, recording: recording, user: user2) + recording.recorded_tracks << FactoryGirl.create(:recorded_track, recording: recording, user: user2) + recording.save! + recording.reload + recording + } + + let(:sorted_tracks) { + Array.new(recording1.recorded_tracks).sort! {|a, b| + if a.created_at == b.created_at + a.id <=> b.id + else + a.created_at <=> b.created_at + end + } + } + + + it "one user decides to keep the recording" do + claimed_recording = FactoryGirl.create(:claimed_recording, user: user1, recording: recording1, discarded:false) + claimed_recording.recording.should == recording1 + + data = UserSync.index({user_id: user1.id}) + data[:next].should be_nil + user_syncs = data[:query] + user_syncs.length.should == 4 + user_syncs[0].recorded_track.should == sorted_tracks[0] + user_syncs[1].recorded_track.should == sorted_tracks[1] + user_syncs[2].recorded_track.should == sorted_tracks[2] + user_syncs[3].recorded_track.should == sorted_tracks[3] + + data = UserSync.index({user_id: user2.id}) + data[:next].should be_nil + user_syncs = data[:query] + user_syncs.length.should == 4 + user_syncs[0].recorded_track.should == sorted_tracks[0] + user_syncs[1].recorded_track.should == sorted_tracks[1] + user_syncs[2].recorded_track.should == sorted_tracks[2] + user_syncs[3].recorded_track.should == sorted_tracks[3] + end + end + end describe "pagination" do diff --git a/web/app/assets/javascripts/recording_utils.js.coffee b/web/app/assets/javascripts/recording_utils.js.coffee index 89c556a7c..3a6dbd002 100644 --- a/web/app/assets/javascripts/recording_utils.js.coffee +++ b/web/app/assets/javascripts/recording_utils.js.coffee @@ -34,7 +34,7 @@ class RecordingUtils mixStateDefinition: (mixState) => definition = switch mixState - when 'still-uploading' then "STILL UPLOADING means the you or others in the recording have not uploaded enough information yet to make a mix." + when 'still-uploading' then "STILL UPLOADING means you or others in the recording have not uploaded enough information yet to make a mix." when 'stream-mix' then "STREAM MIX means a user's real-time mix is available to listen to." when 'discarded' then "DISCARDED means you chose to not keep this recording when the recording was over." when 'mixed' then "MIXED means all tracks have been uploaded, and a final mix has been made." diff --git a/web/app/assets/javascripts/sessionModel.js b/web/app/assets/javascripts/sessionModel.js index 82a8724b0..9bfb3409d 100644 --- a/web/app/assets/javascripts/sessionModel.js +++ b/web/app/assets/javascripts/sessionModel.js @@ -31,6 +31,7 @@ var sessionPageEnterDeferred = null; var sessionPageEnterTimeout = null; var startTime = null; + var joinDeferred = null; server.registerOnSocketClosed(onWebsocketDisconnected); @@ -123,10 +124,18 @@ */ function joinSession(sessionId) { logger.debug("SessionModel.joinSession(" + sessionId + ")"); - var deferred = joinSessionRest(sessionId); + joinDeferred = joinSessionRest(sessionId); + + joinDeferred + .done(function(response){ + + if(!inSession()) { + // the user has left the session before they got joined. We need to issue a leave again to the server to make sure they are out + logger.debug("user left before fully joined to session. telling server again that they have left") + leaveSessionRest(response.id); + return; + } - deferred - .done(function(response){ logger.debug("calling jamClient.JoinSession"); // on temporary disconnect scenarios, a user may already be in a session when they enter this path // so we avoid double counting @@ -153,7 +162,7 @@ updateCurrentSession(null); }); - return deferred; + return joinDeferred; } function performLeaveSession(deferred) { @@ -251,13 +260,15 @@ } // universal place to clean up, reset items - function sessionEnded() { + // fullyJoined means the user stayed in the screen until they had a successful rest.joinSession + // you might not get a fully joined if you join/leave really quickly before the REST API completes + function sessionEnded(fullyJoined) { //cleanup server.unregisterMessageCallback(context.JK.MessageType.SESSION_JOIN, trackChanges); server.unregisterMessageCallback(context.JK.MessageType.SESSION_DEPART, trackChanges); server.unregisterMessageCallback(context.JK.MessageType.TRACKS_CHANGED, trackChanges); - server.registerMessageCallback(context.JK.MessageType.HEARTBEAT_ACK, trackChanges); + server.unregisterMessageCallback(context.JK.MessageType.HEARTBEAT_ACK, trackChanges); if(sessionPageEnterDeferred != null) { sessionPageEnterDeferred.reject('session_over'); @@ -265,7 +276,10 @@ } userTracks = null; startTime = null; - $(document).trigger(EVENTS.SESSION_ENDED, {session: {id: currentSessionId}}); + joinDeferred = null; + if(fullyJoined) { + $(document).trigger(EVENTS.SESSION_ENDED, {session: {id: currentSessionId}}); + } currentSessionId = null; } @@ -280,8 +294,8 @@ currentSession = sessionData; // the 'beforeUpdate != null' makes sure we only do a clean up one time internally - if(sessionData == null && beforeUpdate != null) { - sessionEnded(); + if(sessionData == null) { + sessionEnded(beforeUpdate != null); } } @@ -538,51 +552,65 @@ if(inSession() && text == "RebuildAudioIoControl") { - if(sessionPageEnterDeferred) { - // this means we are still waiting for the BACKEND_MIXER_CHANGE that indicates we have user tracks built-out/ready - - // we will get at least one BACKEND_MIXER_CHANGE that corresponds to the backend doing a 'audio pause', which won't matter much - // so we need to check that we actaully have userTracks before considering ourselves done - var inputTracks = context.JK.TrackHelpers.getUserTracks(context.jamClient); - if(inputTracks.length > 0) { - logger.debug("obtained tracks at start of session") - sessionPageEnterDeferred.resolve(inputTracks); - sessionPageEnterDeferred = null; - } - - return; - } - // the backend will send these events rapid-fire back to back. // the server can still perform correctly, but it is nicer to wait 100 ms to let them all fall through if(backendMixerAlertThrottleTimer) {clearTimeout(backendMixerAlertThrottleTimer);} backendMixerAlertThrottleTimer = setTimeout(function() { - // this is a local change to our tracks. we need to tell the server about our updated track information - var inputTracks = context.JK.TrackHelpers.getUserTracks(context.jamClient); - // create a trackSync request based on backend data - var syncTrackRequest = {}; - syncTrackRequest.client_id = app.clientId; - syncTrackRequest.tracks = inputTracks; - syncTrackRequest.id = id(); + if(sessionPageEnterDeferred) { + // this means we are still waiting for the BACKEND_MIXER_CHANGE that indicates we have user tracks built-out/ready - rest.putTrackSyncChange(syncTrackRequest) - .done(function() { - }) - .fail(function(jqXHR) { - if(jqXHR.status != 404) { - app.notify({ - "title": "Can't Sync Local Tracks", - "text": "The client is unable to sync local track information with the server. You should rejoin the session to ensure a good experience.", - "icon_url": "/assets/content/icon_alert_big.png" - }); - } - else { - logger.debug("Unable to sync local tracks because session is gone.") + // we will get at least one BACKEND_MIXER_CHANGE that corresponds to the backend doing a 'audio pause', which won't matter much + // so we need to check that we actaully have userTracks before considering ourselves done + var inputTracks = context.JK.TrackHelpers.getUserTracks(context.jamClient); + if(inputTracks.length > 0) { + logger.debug("obtained tracks at start of session") + sessionPageEnterDeferred.resolve(inputTracks); + sessionPageEnterDeferred = null; + } + + return; + } + + // wait until we are fully in session before trying to sync tracks to server + if(joinDeferred) { + joinDeferred.done(function() { + + // double check that we are in session, since a bunch could have happened since then + if(!inSession()) { + logger.debug("dropping queued up sync tracks because no longer in session"); + return; } + // this is a local change to our tracks. we need to tell the server about our updated track information + var inputTracks = context.JK.TrackHelpers.getUserTracks(context.jamClient); + + // create a trackSync request based on backend data + var syncTrackRequest = {}; + syncTrackRequest.client_id = app.clientId; + syncTrackRequest.tracks = inputTracks; + syncTrackRequest.id = id(); + + rest.putTrackSyncChange(syncTrackRequest) + .done(function() { + }) + .fail(function(jqXHR) { + if(jqXHR.status != 404) { + app.notify({ + "title": "Can't Sync Local Tracks", + "text": "The client is unable to sync local track information with the server. You should rejoin the session to ensure a good experience.", + "icon_url": "/assets/content/icon_alert_big.png" + }); + } + else { + logger.debug("Unable to sync local tracks because session is gone.") + } + + }) }) + } + }, 100); } else if(inSession() && (text == 'RebuildMediaControl' || text == 'RebuildRemoteUserControl')) { diff --git a/web/app/assets/javascripts/sync_viewer.js.coffee b/web/app/assets/javascripts/sync_viewer.js.coffee index 31f371a01..02fd55c1d 100644 --- a/web/app/assets/javascripts/sync_viewer.js.coffee +++ b/web/app/assets/javascripts/sync_viewer.js.coffee @@ -401,7 +401,7 @@ context.JK.SyncViewer = class SyncViewer summary = "Both you and the JamKazam server have the high-quality version of this track. Once all the other tracks for this recording are also synchronized, then the final mix can be made." clientStateDefinition = switch clientState - when @clientStates.too_many_downloads then "This track has been downloaded a unusually large number of times. No more downloads are allowed." + when @clientStates.too_many_downloads then "This track has been downloaded an unusually large number of times. No more downloads are allowed." when @clientStates.hq then "HIGHEST QUALITY means you have the original version of this track, as recorded by the user that made it." when @clientStates.sq then "STREAM QUALITY means you have the version of the track that you received over the internet in real-time." when @clientStates.missing then "MISSING means you do not have this track anymore." @@ -529,6 +529,10 @@ context.JK.SyncViewer = class SyncViewer exportRecording: (e) => $export = $(e.target) + if context.JK.CurrentSessionModel and context.JK.CurrentSessionModel.inSession() + context.JK.confirmBubble($export, 'sync-viewer-paused', {}, {offsetParent: $export.closest('.dialog')}) + return + recordingId = $export.closest('.details').attr('data-recording-id') if !recordingId? or recordingId == "" throw "exportRecording can't find data-recording-id" diff --git a/web/app/views/clients/_help.html.erb b/web/app/views/clients/_help.html.erb index 767a09794..405384d99 100644 --- a/web/app/views/clients/_help.html.erb +++ b/web/app/views/clients/_help.html.erb @@ -129,7 +129,7 @@