[banshee] Audioscrobbler: avoid 414 Request-URI Too Large (bgo#689018)



commit 3baa467511ad6661dad1924b88d9a805fc2bb1b2
Author: Phil Trimble <PhilTrimble gmail com>
Date:   Fri Feb 22 09:09:33 2013 +0000

    Audioscrobbler: avoid 414 Request-URI Too Large (bgo#689018)
    
    Lastfm has the policy of not accepting more than 50 songs per
    audioscrobbling submission; but it also has a limit on how long
    an HTTP request should be, so there could be occassions when
    the metadata of the 30 songs (our MAX) could surpass this limit.
    
    Now, LastfmRequest can receive parameters in batches (all
    parameters for a song, in a NameValueCollection). If that
    addition makes the request too large, a MaxSizeExceeded
    exception is thrown which means that no more parameters
    can be added and the request is ready to be sent.
    
    (All LastFmRequest tests still pass after this refactoring.)
    
    Co-authored-by: Andres G. Aragoneses <knocte gmail com>
     Signed-off-by: Andres G. Aragoneses <knocte gmail com>

 .../Banshee.Lastfm.Audioscrobbler/Queue.cs         |   12 ++-
 .../Lastfm/Lastfm/AudioscrobblerConnection.cs      |   61 ++++++++----
 src/Libraries/Lastfm/Lastfm/IQueue.cs              |    2 +-
 src/Libraries/Lastfm/Lastfm/LastfmRequest.cs       |  109 ++++++++++++++------
 4 files changed, 126 insertions(+), 58 deletions(-)
---
diff --git a/src/Extensions/Banshee.Lastfm/Banshee.Lastfm.Audioscrobbler/Queue.cs 
b/src/Extensions/Banshee.Lastfm/Banshee.Lastfm.Audioscrobbler/Queue.cs
index 72bc159..fe1972d 100644
--- a/src/Extensions/Banshee.Lastfm/Banshee.Lastfm.Audioscrobbler/Queue.cs
+++ b/src/Extensions/Banshee.Lastfm/Banshee.Lastfm.Audioscrobbler/Queue.cs
@@ -145,6 +145,7 @@ namespace Banshee.Lastfm.Audioscrobbler
         List<IQueuedTrack> queue;
         string xml_path;
         bool dirty;
+        int next = 0;
 
         public event EventHandler TrackAdded;
 
@@ -257,11 +258,12 @@ namespace Banshee.Lastfm.Audioscrobbler
             }
         }
 
-        public List<IQueuedTrack> GetTracks ()
+        public IQueuedTrack GetNextTrack ()
         {
-            // Last.fm can technically handle up to 50 songs in one request
-            // but seems to throw errors if our submission is too long.
-            return queue.GetRange (0, Math.Min (queue.Count, 30));
+            if (next >= queue.Count) {
+                return null;
+            }
+            return queue [next++];
         }
 
         public void Add (object track, DateTime started_at)
@@ -289,6 +291,7 @@ namespace Banshee.Lastfm.Audioscrobbler
         {
             queue.RemoveRange (first, count);
             dirty = true;
+            next = 0;
         }
 
         public int Count {
@@ -311,6 +314,7 @@ namespace Banshee.Lastfm.Audioscrobbler
                     removed_track_count
                 );
                 dirty = true;
+                next = 0;
                 Save ();
             }
         }
diff --git a/src/Libraries/Lastfm/Lastfm/AudioscrobblerConnection.cs 
b/src/Libraries/Lastfm/Lastfm/AudioscrobblerConnection.cs
index 4329b95..dbc91cb 100644
--- a/src/Libraries/Lastfm/Lastfm/AudioscrobblerConnection.cs
+++ b/src/Libraries/Lastfm/Lastfm/AudioscrobblerConnection.cs
@@ -5,8 +5,11 @@
 //   Chris Toshok <toshok ximian com>
 //   Alexander Hixon <hixon alexander mediati org>
 //   Phil Trimble <philtrimble gmail com>
+//   Andres G. Aragoneses <knocte gmail com>
 //
 // Copyright (C) 2005-2008 Novell, Inc.
+// Copyright (C) 2013 Phil Trimble
+// Copyright (C) 2013 Andres G. Aragoneses
 //
 // Permission is hereby granted, free of charge, to any person obtaining
 // a copy of this software and associated documentation files (the
@@ -40,6 +43,7 @@ using System.Web;
 using Hyena;
 using Hyena.Json;
 using Mono.Unix;
+using System.Collections.Specialized;
 
 namespace Lastfm
 {
@@ -199,33 +203,30 @@ namespace Lastfm
                 return;
             }
 
+            state = State.Transmitting;
             current_scrobble_request = new LastfmRequest ("track.scrobble", RequestType.Write, 
ResponseFormat.Json);
-            IList<IQueuedTrack> tracks = queue.GetTracks ();
 
-            for (int i = 0; i < tracks.Count; i++) {
-                IQueuedTrack track = tracks[i];
+            int trackCount = 0;
+            while (true) {
+                IQueuedTrack track = queue.GetNextTrack ();
+                if (track == null ||
+                    // Last.fm can technically handle up to 50 songs in one request
+                    // but let's not use the top limit
+                    trackCount == 40) {
+                    break;
+                 }
 
-                string str_track_number = String.Empty;
-                if (track.TrackNumber != 0) {
-                    str_track_number = track.TrackNumber.ToString();
+                try {
+                    current_scrobble_request.AddParameters (GetTrackParameters (track, trackCount));
+                    trackCount++;
+                } catch (MaxSizeExceededException) {
+                    break;
                 }
-
-                bool chosen_by_user = (track.TrackAuth.Length == 0);
-
-                current_scrobble_request.AddParameter (String.Format ("timestamp[{0}]", i), 
track.StartTime.ToString ());
-                current_scrobble_request.AddParameter (String.Format ("track[{0}]", i), track.Title);
-                current_scrobble_request.AddParameter (String.Format ("artist[{0}]", i), track.Artist);
-                current_scrobble_request.AddParameter (String.Format ("album[{0}]", i), track.Album);
-                current_scrobble_request.AddParameter (String.Format ("trackNumber[{0}]", i), 
str_track_number);
-                current_scrobble_request.AddParameter (String.Format ("duration[{0}]", i), 
track.Duration.ToString ());
-                current_scrobble_request.AddParameter (String.Format ("mbid[{0}]", i), track.MusicBrainzId);
-                current_scrobble_request.AddParameter (String.Format ("chosenByUser[{0}]", i), 
chosen_by_user ? "1" : "0");
             }
 
             Log.DebugFormat ("Last.fm scrobbler sending '{0}'", current_scrobble_request.ToString ());
 
-            state = State.Transmitting;
-            current_async_result = current_scrobble_request.BeginSend (OnScrobbleResponse, tracks.Count);
+            current_async_result = current_scrobble_request.BeginSend (OnScrobbleResponse, trackCount);
             state = State.WaitingForResponse;
             if (!(current_async_result.AsyncWaitHandle.WaitOne (TIME_OUT, false))) {
                 Log.Warning ("Audioscrobbler upload failed", "The request timed out and was aborted", false);
@@ -235,6 +236,26 @@ namespace Lastfm
             }
         }
 
+        private static NameValueCollection GetTrackParameters (IQueuedTrack track, int trackCount)
+        {
+            string str_track_number = String.Empty;
+            if (track.TrackNumber != 0) {
+                str_track_number = track.TrackNumber.ToString();
+            }
+            bool chosen_by_user = (track.TrackAuth.Length == 0);
+
+            return new NameValueCollection () {
+                { String.Format ("timestamp[{0}]", trackCount), track.StartTime.ToString () },
+                { String.Format ("track[{0}]", trackCount), track.Title },
+                { String.Format ("artist[{0}]", trackCount), track.Artist },
+                { String.Format ("album[{0}]", trackCount), track.Album },
+                { String.Format ("trackNumber[{0}]", trackCount), str_track_number },
+                { String.Format ("duration[{0}]", trackCount), track.Duration.ToString () },
+                { String.Format ("mbid[{0}]", trackCount), track.MusicBrainzId },
+                { String.Format ("chosenByUser[{0}]", trackCount), chosen_by_user ? "1" : "0" }
+            };
+        }
+
         private void OnScrobbleResponse (IAsyncResult ar)
         {
             int nb_tracks_scrobbled = 0;
diff --git a/src/Libraries/Lastfm/Lastfm/IQueue.cs b/src/Libraries/Lastfm/Lastfm/IQueue.cs
index 7f7b23a..6eb4015 100644
--- a/src/Libraries/Lastfm/Lastfm/IQueue.cs
+++ b/src/Libraries/Lastfm/Lastfm/IQueue.cs
@@ -56,7 +56,7 @@ namespace Lastfm
         void Save ();
         void Load ();
 
-        List<IQueuedTrack> GetTracks ();
+        IQueuedTrack GetNextTrack ();
 
         void Add (object track, DateTime started);
         void RemoveRange (int first, int count);
diff --git a/src/Libraries/Lastfm/Lastfm/LastfmRequest.cs b/src/Libraries/Lastfm/Lastfm/LastfmRequest.cs
index b59cfcf..5f04464 100644
--- a/src/Libraries/Lastfm/Lastfm/LastfmRequest.cs
+++ b/src/Libraries/Lastfm/Lastfm/LastfmRequest.cs
@@ -4,8 +4,11 @@
 // Authors:
 //   Bertrand Lorentz <bertrand lorentz gmail com>
 //   Phil Trimble <philtrimble gmail com>
+//   Andres G. Aragoneses <knocte gmail com>
 //
 // Copyright (C) 2009 Bertrand Lorentz
+// Copyright (C) 2013 Phil Trimble
+// Copyright (C) 2013 Andres G. Aragoneses
 //
 // Permission is hereby granted, free of charge, to any person obtaining
 // a copy of this software and associated documentation files (the
@@ -29,6 +32,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Collections.Specialized;
 using System.IO;
 using System.Net;
 using System.Text;
@@ -54,6 +58,10 @@ namespace Lastfm
 
     public delegate void SendRequestHandler ();
 
+    public class MaxSizeExceededException : ApplicationException
+    {
+    }
+
     internal class WebRequestCreator : IWebRequestCreate
     {
         public WebRequest Create (Uri uri)
@@ -91,6 +99,29 @@ namespace Lastfm
             if (this.web_request_creator == null) {
                 this.web_request_creator = new WebRequestCreator ();
             }
+
+            Init ();
+        }
+
+        private void Init ()
+        {
+            this.incremental_data = new StringBuilder ();
+
+            if (request_type != RequestType.Write) {
+                incremental_data.Append (API_ROOT);
+            }
+            incremental_data.AppendFormat ("?method={0}", method);
+            incremental_data.AppendFormat ("&api_key={0}", LastfmCore.ApiKey);
+
+            if (request_type == RequestType.AuthenticatedRead || request_type == RequestType.Write) {
+                AddParameter ("sk", LastfmCore.Account.SessionKey);
+            }
+
+            if (response_format == ResponseFormat.Json) {
+                AddParameter ("format", "json");
+            } else if (response_format == ResponseFormat.Raw) {
+                AddParameter ("raw", "true");
+            }
         }
 
         private string method;
@@ -99,12 +130,52 @@ namespace Lastfm
 
         private ResponseFormat response_format;
 
+        private StringBuilder incremental_data;
+
+        // This is close to the max based on testing.
+        private const int MAX_POST_LENGTH = 7000;
 
         public void AddParameter (string param_name, string param_value)
         {
+            var chunk = String.Format ("&{0}={1}",
+                                       param_name, param_value != null ? Uri.EscapeDataString (param_value) 
: null);
+
+            CheckSize (chunk.Length);
+
+            incremental_data.Append (chunk);
             parameters.Add (param_name, param_value);
         }
 
+        public void AddParameters (NameValueCollection parms)
+        {
+            var chunks = new StringBuilder ();
+            foreach (string key in parms) {
+                var value = parms [key];
+                chunks.AppendFormat ("&{0}={1}",
+                                     key, value != null ? Uri.EscapeDataString (value) : null);
+            }
+
+            CheckSize (chunks.Length);
+
+            incremental_data.Append (chunks.ToString ());
+            foreach (string key in parms) {
+                var value = parms [key];
+                parameters.Add (key, value);
+            }
+        }
+
+        private void CheckSize (int length)
+        {
+            var target_length = incremental_data.Length + length;
+            if (request_type != RequestType.Read) {
+                target_length += 9 + 32; // length of &api_sig={GetSignature()}
+            }
+
+            if (target_length > MAX_POST_LENGTH) {
+                throw new MaxSizeExceededException ();
+            }
+        }
+
         public Stream GetResponseStream ()
         {
             return response_stream;
@@ -116,12 +187,6 @@ namespace Lastfm
                 throw new InvalidOperationException ("The method name should be set");
             }
 
-            if (response_format == ResponseFormat.Json) {
-                AddParameter ("format", "json");
-            } else if (response_format == ResponseFormat.Raw) {
-                AddParameter ("raw", "true");
-            }
-
             if (request_type == RequestType.Write) {
                 response_stream = Post (API_ROOT, BuildPostData ());
             } else {
@@ -201,39 +266,17 @@ namespace Lastfm
 
         private string BuildGetUrl ()
         {
-            if (request_type == RequestType.AuthenticatedRead) {
-                parameters.Add ("sk", LastfmCore.Account.SessionKey);
-            }
-
-            StringBuilder url = new StringBuilder (API_ROOT);
-            url.AppendFormat ("?method={0}", method);
-            url.AppendFormat ("&api_key={0}", LastfmCore.ApiKey);
-            foreach (KeyValuePair<string, string> param in parameters) {
-                url.AppendFormat ("&{0}={1}", param.Key, Uri.EscapeDataString (param.Value));
-            }
             if (request_type == RequestType.AuthenticatedRead || request_type == RequestType.SessionRequest) 
{
-                url.AppendFormat ("&api_sig={0}", GetSignature ());
+                incremental_data.AppendFormat ("&api_sig={0}", GetSignature ());
             }
 
-            return url.ToString ();
+            return incremental_data.ToString ();
         }
 
         private string BuildPostData ()
         {
-            parameters.Add ("sk", LastfmCore.Account.SessionKey);
-
-            StringBuilder data = new StringBuilder ();
-            data.AppendFormat ("method={0}", method);
-            data.AppendFormat ("&api_key={0}", LastfmCore.ApiKey);
-
-            foreach (KeyValuePair<string, string> param in parameters) {
-                data.AppendFormat ("&{0}={1}",
-                                   param.Key, param.Value != null ? Uri.EscapeDataString (param.Value) : 
null);
-            }
-
-            data.AppendFormat ("&api_sig={0}", GetSignature ());
-
-            return data.ToString ();
+            incremental_data.AppendFormat ("&api_sig={0}", GetSignature ());
+            return incremental_data.ToString ();
         }
 
         private string GetSignature ()
@@ -311,7 +354,7 @@ namespace Lastfm
         private Stream Post (string uri, string data)
         {
             // Do not trust docs : it doesn't work if parameters are in the request body
-            var request = (HttpWebRequest)web_request_creator.Create (new Uri (String.Concat (uri, "?", 
data)));
+            var request = (HttpWebRequest)web_request_creator.Create (new Uri (String.Concat (uri, data)));
             request.UserAgent = LastfmCore.UserAgent;
             request.Timeout = 10000;
             request.Method = "POST";


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]