From 6d3e6d800fd8c6d5cb22271e2177946ab286855f Mon Sep 17 00:00:00 2001 From: PloughPuff Date: Wed, 13 Mar 2019 20:31:21 +0000 Subject: [PATCH 1/3] Only delay making request if necessary When requesting data from MusicBrainz, only delay the request if previous request was less than rate limit ago, instead of always delaying for one second at start. --- .../Music/MusicBrainzAlbumProvider.cs | 62 +++++++------------ .../Music/MusicBrainzArtistProvider.cs | 6 +- 2 files changed, 27 insertions(+), 41 deletions(-) diff --git a/MediaBrowser.Providers/Music/MusicBrainzAlbumProvider.cs b/MediaBrowser.Providers/Music/MusicBrainzAlbumProvider.cs index f86cdeab9..8a3860d74 100644 --- a/MediaBrowser.Providers/Music/MusicBrainzAlbumProvider.cs +++ b/MediaBrowser.Providers/Music/MusicBrainzAlbumProvider.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Globalization; using System.IO; using System.Linq; @@ -28,6 +29,7 @@ namespace MediaBrowser.Providers.Music private readonly IApplicationHost _appHost; private readonly ILogger _logger; private readonly IJsonSerializer _json; + private Stopwatch _stopWatchMusicBrainz = new Stopwatch(); public readonly string MusicBrainzBaseUrl; @@ -45,6 +47,9 @@ namespace MediaBrowser.Providers.Music MusicBrainzBaseUrl = configuration["MusicBrainz:BaseUrl"]; + // Use a stopwatch to ensure we don't exceed the MusicBrainz rate limit + _stopWatchMusicBrainz.Start(); + Current = this; } @@ -54,8 +59,6 @@ namespace MediaBrowser.Providers.Music var releaseGroupId = searchInfo.GetReleaseGroupId(); string url; - var isNameSearch = false; - bool forceMusicBrainzProper = false; if (!string.IsNullOrEmpty(releaseId)) { @@ -64,7 +67,6 @@ namespace MediaBrowser.Providers.Music else if (!string.IsNullOrEmpty(releaseGroupId)) { url = string.Format("/ws/2/release?release-group={0}", releaseGroupId); - forceMusicBrainzProper = true; } else { @@ -78,8 +80,6 @@ namespace MediaBrowser.Providers.Music } else { - isNameSearch = true; - // I'm sure there is a better way but for now it resolves search for 12" Mixes var queryName = searchInfo.Name.Replace("\"", string.Empty); @@ -91,7 +91,7 @@ namespace MediaBrowser.Providers.Music if (!string.IsNullOrWhiteSpace(url)) { - using (var response = await GetMusicBrainzResponse(url, isNameSearch, forceMusicBrainzProper, cancellationToken).ConfigureAwait(false)) + using (var response = await GetMusicBrainzResponse(url, cancellationToken).ConfigureAwait(false)) { using (var stream = response.Content) { @@ -247,7 +247,7 @@ namespace MediaBrowser.Providers.Music WebUtility.UrlEncode(albumName), artistId); - using (var response = await GetMusicBrainzResponse(url, true, cancellationToken).ConfigureAwait(false)) + using (var response = await GetMusicBrainzResponse(url, cancellationToken).ConfigureAwait(false)) using (var stream = response.Content) using (var oReader = new StreamReader(stream, Encoding.UTF8)) { @@ -272,7 +272,7 @@ namespace MediaBrowser.Providers.Music WebUtility.UrlEncode(albumName), WebUtility.UrlEncode(artistName)); - using (var response = await GetMusicBrainzResponse(url, true, cancellationToken).ConfigureAwait(false)) + using (var response = await GetMusicBrainzResponse(url, cancellationToken).ConfigureAwait(false)) using (var stream = response.Content) using (var oReader = new StreamReader(stream, Encoding.UTF8)) { @@ -589,7 +589,7 @@ namespace MediaBrowser.Providers.Music { var url = string.Format("/ws/2/release?release-group={0}", releaseGroupId); - using (var response = await GetMusicBrainzResponse(url, true, true, cancellationToken).ConfigureAwait(false)) + using (var response = await GetMusicBrainzResponse(url, cancellationToken).ConfigureAwait(false)) using (var stream = response.Content) using (var oReader = new StreamReader(stream, Encoding.UTF8)) { @@ -625,7 +625,7 @@ namespace MediaBrowser.Providers.Music { var url = string.Format("/ws/2/release-group/?query=reid:{0}", releaseEntryId); - using (var response = await GetMusicBrainzResponse(url, false, cancellationToken).ConfigureAwait(false)) + using (var response = await GetMusicBrainzResponse(url, cancellationToken).ConfigureAwait(false)) using (var stream = response.Content) using (var oReader = new StreamReader(stream, Encoding.UTF8)) { @@ -710,34 +710,32 @@ namespace MediaBrowser.Providers.Music return null; } - internal Task GetMusicBrainzResponse(string url, bool isSearch, CancellationToken cancellationToken) - { - return GetMusicBrainzResponse(url, isSearch, false, cancellationToken); - } - /// - /// Gets the music brainz response. + /// Makes request to MusicBrainz server and awaits a response. /// - internal async Task GetMusicBrainzResponse(string url, bool isSearch, bool forceMusicBrainzProper, CancellationToken cancellationToken) + internal async Task GetMusicBrainzResponse(string url, CancellationToken cancellationToken) { - var urlInfo = new MbzUrl(MusicBrainzBaseUrl, 1000); - var throttleMs = urlInfo.throttleMs; + // The Jellyfin user-agent is unrestricted but source IP must not exceed + // one request per second, therefore we rate limit to avoid throttling + // https://musicbrainz.org/doc/XML_Web_Service/Rate_Limiting + const long QueryIntervalMs = 1000u; - if (throttleMs > 0) + // Only delay if necessary + if (_stopWatchMusicBrainz.ElapsedMilliseconds < QueryIntervalMs) { - // MusicBrainz is extremely adamant about limiting to one request per second - _logger.LogDebug("Throttling MusicBrainz by {0}ms", throttleMs.ToString(CultureInfo.InvariantCulture)); - await Task.Delay(throttleMs, cancellationToken).ConfigureAwait(false); + var delayMs = QueryIntervalMs - _stopWatchMusicBrainz.ElapsedMilliseconds; + await Task.Delay((int)delayMs, cancellationToken).ConfigureAwait(false); } - url = urlInfo.url.TrimEnd('/') + url; + _logger.LogDebug("MusicBrainz time since previous request: {0}ms", _stopWatchMusicBrainz.ElapsedMilliseconds); + _stopWatchMusicBrainz.Restart(); var options = new HttpRequestOptions { - Url = url, + Url = MusicBrainzBaseUrl.TrimEnd('/') + url, CancellationToken = cancellationToken, UserAgent = _appHost.ApplicationUserAgent, - BufferContent = throttleMs > 0 + BufferContent = false }; return await _httpClient.SendAsync(options, "GET").ConfigureAwait(false); @@ -749,17 +747,5 @@ namespace MediaBrowser.Providers.Music { throw new NotImplementedException(); } - - internal class MbzUrl - { - internal MbzUrl(string url, int throttleMs) - { - this.url = url; - this.throttleMs = throttleMs; - } - - public string url { get; set; } - public int throttleMs { get; set; } - } } } diff --git a/MediaBrowser.Providers/Music/MusicBrainzArtistProvider.cs b/MediaBrowser.Providers/Music/MusicBrainzArtistProvider.cs index 55aab7778..59280df89 100644 --- a/MediaBrowser.Providers/Music/MusicBrainzArtistProvider.cs +++ b/MediaBrowser.Providers/Music/MusicBrainzArtistProvider.cs @@ -31,7 +31,7 @@ namespace MediaBrowser.Providers.Music { var url = string.Format("/ws/2/artist/?query=arid:{0}", musicBrainzId); - using (var response = await MusicBrainzAlbumProvider.Current.GetMusicBrainzResponse(url, false, cancellationToken).ConfigureAwait(false)) + using (var response = await MusicBrainzAlbumProvider.Current.GetMusicBrainzResponse(url, cancellationToken).ConfigureAwait(false)) { using (var stream = response.Content) { @@ -46,7 +46,7 @@ namespace MediaBrowser.Providers.Music var url = string.Format("/ws/2/artist/?query=\"{0}\"&dismax=true", UrlEncode(nameToSearch)); - using (var response = await MusicBrainzAlbumProvider.Current.GetMusicBrainzResponse(url, true, cancellationToken).ConfigureAwait(false)) + using (var response = await MusicBrainzAlbumProvider.Current.GetMusicBrainzResponse(url, cancellationToken).ConfigureAwait(false)) { using (var stream = response.Content) { @@ -64,7 +64,7 @@ namespace MediaBrowser.Providers.Music // Try again using the search with accent characters url url = string.Format("/ws/2/artist/?query=artistaccent:\"{0}\"", UrlEncode(nameToSearch)); - using (var response = await MusicBrainzAlbumProvider.Current.GetMusicBrainzResponse(url, true, cancellationToken).ConfigureAwait(false)) + using (var response = await MusicBrainzAlbumProvider.Current.GetMusicBrainzResponse(url, cancellationToken).ConfigureAwait(false)) { using (var stream = response.Content) { From f8bb7a7ff4d87d19a43f97fed557aa92a657a50f Mon Sep 17 00:00:00 2001 From: PloughPuff Date: Thu, 14 Mar 2019 00:43:41 +0000 Subject: [PATCH 2/3] Increased interval to 1050ms and moved to class scope Review comments from JustAMan. --- .../Music/MusicBrainzAlbumProvider.cs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/MediaBrowser.Providers/Music/MusicBrainzAlbumProvider.cs b/MediaBrowser.Providers/Music/MusicBrainzAlbumProvider.cs index 8a3860d74..a1366e0fd 100644 --- a/MediaBrowser.Providers/Music/MusicBrainzAlbumProvider.cs +++ b/MediaBrowser.Providers/Music/MusicBrainzAlbumProvider.cs @@ -33,6 +33,12 @@ namespace MediaBrowser.Providers.Music public readonly string MusicBrainzBaseUrl; + // The Jellyfin user-agent is unrestricted but source IP must not exceed + // one request per second, therefore we rate limit to avoid throttling. + // Be prudent, use a value slightly above the minimun required. + // https://musicbrainz.org/doc/XML_Web_Service/Rate_Limiting + private const long MusicBrainzQueryIntervalMs = 1050u; + public MusicBrainzAlbumProvider( IHttpClient httpClient, IApplicationHost appHost, @@ -715,15 +721,10 @@ namespace MediaBrowser.Providers.Music /// internal async Task GetMusicBrainzResponse(string url, CancellationToken cancellationToken) { - // The Jellyfin user-agent is unrestricted but source IP must not exceed - // one request per second, therefore we rate limit to avoid throttling - // https://musicbrainz.org/doc/XML_Web_Service/Rate_Limiting - const long QueryIntervalMs = 1000u; - - // Only delay if necessary - if (_stopWatchMusicBrainz.ElapsedMilliseconds < QueryIntervalMs) + if (_stopWatchMusicBrainz.ElapsedMilliseconds < MusicBrainzQueryIntervalMs) { - var delayMs = QueryIntervalMs - _stopWatchMusicBrainz.ElapsedMilliseconds; + // MusicBrainz is extremely adamant about limiting to one request per second + var delayMs = MusicBrainzQueryIntervalMs - _stopWatchMusicBrainz.ElapsedMilliseconds; await Task.Delay((int)delayMs, cancellationToken).ConfigureAwait(false); } From d125fbc43d453be7de08375dede1748d4c19a8cf Mon Sep 17 00:00:00 2001 From: PloughPuff Date: Thu, 14 Mar 2019 21:32:27 +0000 Subject: [PATCH 3/3] Added contact email to user agent MusicBrainz request a contact email address is supplied in comment section of user agent field. --- Emby.Server.Implementations/ApplicationHost.cs | 6 ++++++ MediaBrowser.Common/IApplicationHost.cs | 6 ++++++ MediaBrowser.Providers/Music/MusicBrainzAlbumProvider.cs | 5 +++-- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/Emby.Server.Implementations/ApplicationHost.cs b/Emby.Server.Implementations/ApplicationHost.cs index 94eaffa47..eaea8844d 100644 --- a/Emby.Server.Implementations/ApplicationHost.cs +++ b/Emby.Server.Implementations/ApplicationHost.cs @@ -428,6 +428,12 @@ namespace Emby.Server.Implementations /// The application user agent. public string ApplicationUserAgent => Name.Replace(' ','-') + "/" + ApplicationVersion; + /// + /// Gets the email address for use within a comment section of a user agent field. + /// Presently used to provide contact information to MusicBrainz service. + /// + public string ApplicationUserAgentAddress { get; } = "team@jellyfin.org"; + private string _productName; /// diff --git a/MediaBrowser.Common/IApplicationHost.cs b/MediaBrowser.Common/IApplicationHost.cs index 966448d63..2925a3efd 100644 --- a/MediaBrowser.Common/IApplicationHost.cs +++ b/MediaBrowser.Common/IApplicationHost.cs @@ -71,6 +71,12 @@ namespace MediaBrowser.Common /// The application user agent. string ApplicationUserAgent { get; } + /// + /// Gets the email address for use within a comment section of a user agent field. + /// Presently used to provide contact information to MusicBrainz service. + /// + string ApplicationUserAgentAddress { get; } + /// /// Gets the exports. /// diff --git a/MediaBrowser.Providers/Music/MusicBrainzAlbumProvider.cs b/MediaBrowser.Providers/Music/MusicBrainzAlbumProvider.cs index a1366e0fd..b716f40f0 100644 --- a/MediaBrowser.Providers/Music/MusicBrainzAlbumProvider.cs +++ b/MediaBrowser.Providers/Music/MusicBrainzAlbumProvider.cs @@ -1,7 +1,6 @@ using System; using System.Collections.Generic; using System.Diagnostics; -using System.Globalization; using System.IO; using System.Linq; using System.Net; @@ -735,7 +734,9 @@ namespace MediaBrowser.Providers.Music { Url = MusicBrainzBaseUrl.TrimEnd('/') + url, CancellationToken = cancellationToken, - UserAgent = _appHost.ApplicationUserAgent, + // MusicBrainz request a contact email address is supplied, as comment, in user agent field: + // https://musicbrainz.org/doc/XML_Web_Service/Rate_Limiting#User-Agent + UserAgent = string.Format("{0} ( {1} )", _appHost.ApplicationUserAgent, _appHost.ApplicationUserAgentAddress), BufferContent = false };