Only fire metadata savers when appropriate

3 files changed, 89 insertions, 99 deletions

diff --git a/MediaBrowser.Server.Implementations/Library/LibraryManager.cs b/MediaBrowser.Server.Implementations/Library/LibraryManager.cs
index d1b7634fb..0465cb5c3 100644
--- a/MediaBrowser.Server.Implementations/Library/LibraryManager.cs
+++ b/MediaBrowser.Server.Implementations/Library/LibraryManager.cs
@@ -128,6 +128,10 @@ namespace MediaBrowser.Server.Implementations.Library
         /// <value>The by reference items.</value>
         private ConcurrentDictionary<Guid, BaseItem> ByReferenceItems { get; set; }
 
+        private IEnumerable<IMetadataSaver> _savers;
+
+        private readonly Func<IDirectoryWatchers> _directoryWatchersFactory;
+
         /// <summary>
         /// The _library items cache
         /// </summary>
@@ -167,13 +171,14 @@ namespace MediaBrowser.Server.Implementations.Library
         /// <param name="userManager">The user manager.</param>
         /// <param name="configurationManager">The configuration manager.</param>
         /// <param name="userDataRepository">The user data repository.</param>
-        public LibraryManager(ILogger logger, ITaskManager taskManager, IUserManager userManager, IServerConfigurationManager configurationManager, IUserDataRepository userDataRepository)
+        public LibraryManager(ILogger logger, ITaskManager taskManager, IUserManager userManager, IServerConfigurationManager configurationManager, IUserDataRepository userDataRepository, Func<IDirectoryWatchers> directoryWatchersFactory)
         {
             _logger = logger;
             _taskManager = taskManager;
             _userManager = userManager;
             ConfigurationManager = configurationManager;
             _userDataRepository = userDataRepository;
+            _directoryWatchersFactory = directoryWatchersFactory;
             ByReferenceItems = new ConcurrentDictionary<Guid, BaseItem>();
 
             ConfigurationManager.ConfigurationUpdated += ConfigurationUpdated;
@@ -191,13 +196,15 @@ namespace MediaBrowser.Server.Implementations.Library
         /// <param name="itemComparers">The item comparers.</param>
         /// <param name="prescanTasks">The prescan tasks.</param>
         /// <param name="postscanTasks">The postscan tasks.</param>
+        /// <param name="savers">The savers.</param>
         public void AddParts(IEnumerable<IResolverIgnoreRule> rules,
             IEnumerable<IVirtualFolderCreator> pluginFolders,
             IEnumerable<IItemResolver> resolvers,
             IEnumerable<IIntroProvider> introProviders,
             IEnumerable<IBaseItemComparer> itemComparers,
             IEnumerable<ILibraryPrescanTask> prescanTasks,
-            IEnumerable<ILibraryPostScanTask> postscanTasks)
+            IEnumerable<ILibraryPostScanTask> postscanTasks,
+            IEnumerable<IMetadataSaver> savers)
         {
             EntityResolutionIgnoreRules = rules;
             PluginFolderCreators = pluginFolders;
@@ -206,6 +213,7 @@ namespace MediaBrowser.Server.Implementations.Library
             Comparers = itemComparers;
             PrescanTasks = prescanTasks;
             PostscanTasks = postscanTasks;
+            _savers = savers;
         }
 
         /// <summary>
@@ -326,7 +334,7 @@ namespace MediaBrowser.Server.Implementations.Library
         /// <param name="newName">The new name.</param>
         /// <param name="cancellationToken">The cancellation token.</param>
         /// <returns>Task.</returns>
-        private Task UpdateSeasonZeroNames(string newName, CancellationToken cancellationToken)
+        private async Task UpdateSeasonZeroNames(string newName, CancellationToken cancellationToken)
         {
             var seasons = RootFolder.RecursiveChildren
                 .OfType<Season>()
@@ -336,9 +344,16 @@ namespace MediaBrowser.Server.Implementations.Library
             foreach (var season in seasons)
             {
                 season.Name = newName;
-            }
 
-            return UpdateItems(seasons, cancellationToken);
+                try
+                {
+                    await UpdateItem(season, ItemUpdateType.MetadataEdit, cancellationToken).ConfigureAwait(false);
+                }
+                catch (Exception ex)
+                {
+                    _logger.ErrorException("Error saving {0}", ex, season.Path);
+                }
+            }
         }
 
         /// <summary>
@@ -1278,33 +1293,35 @@ namespace MediaBrowser.Server.Implementations.Library
         }
 
         /// <summary>
-        /// Updates the items.
+        /// Updates the item.
         /// </summary>
-        /// <param name="items">The items.</param>
+        /// <param name="item">The item.</param>
+        /// <param name="updateReason">The update reason.</param>
         /// <param name="cancellationToken">The cancellation token.</param>
         /// <returns>Task.</returns>
-        private async Task UpdateItems(IEnumerable<BaseItem> items, CancellationToken cancellationToken)
+        public async Task UpdateItem(BaseItem item, ItemUpdateType updateReason, CancellationToken cancellationToken)
         {
-            var list = items.ToList();
+            await ItemRepository.SaveItem(item, cancellationToken).ConfigureAwait(false);
 
-            await ItemRepository.SaveItems(list, cancellationToken).ConfigureAwait(false);
+            UpdateItemInLibraryCache(item);
 
-            foreach (var item in list)
+            // If metadata was downloaded or edited, save external metadata
+            if ((updateReason & ItemUpdateType.MetadataEdit) == ItemUpdateType.MetadataEdit)
             {
-                UpdateItemInLibraryCache(item);
-                OnItemUpdated(item);
+                await SaveMetadata(item).ConfigureAwait(false);
             }
-        }
 
-        /// <summary>
-        /// Updates the item.
-        /// </summary>
-        /// <param name="item">The item.</param>
-        /// <param name="cancellationToken">The cancellation token.</param>
-        /// <returns>Task.</returns>
-        public Task UpdateItem(BaseItem item, CancellationToken cancellationToken)
-        {
-            return UpdateItems(new[] { item }, cancellationToken);
+            if (ItemUpdated != null)
+            {
+                try
+                {
+                    ItemUpdated(this, new ItemChangeEventArgs { Item = item });
+                }
+                catch (Exception ex)
+                {
+                    _logger.ErrorException("Error in ItemUpdated event handler", ex);
+                }
+            }
         }
 
         /// <summary>
@@ -1337,22 +1354,38 @@ namespace MediaBrowser.Server.Implementations.Library
             return ItemRepository.RetrieveItem(id, type);
         }
 
+        private readonly ConcurrentDictionary<string, SemaphoreSlim> _fileLocks = new ConcurrentDictionary<string, SemaphoreSlim>();
+
         /// <summary>
-        /// Called when [item updated].
+        /// Saves the metadata.
         /// </summary>
         /// <param name="item">The item.</param>
         /// <returns>Task.</returns>
-        private void OnItemUpdated(BaseItem item)
+        private async Task SaveMetadata(BaseItem item)
         {
-            if (ItemUpdated != null)
+            foreach (var saver in _savers.Where(i => i.Supports(item)))
             {
+                var path = saver.GetSavePath(item);
+
+                var semaphore = _fileLocks.GetOrAdd(path, key => new SemaphoreSlim(1, 1));
+
+                var directoryWatchers = _directoryWatchersFactory();
+
+                await semaphore.WaitAsync().ConfigureAwait(false);
+
                 try
                 {
-                    ItemUpdated(this, new ItemChangeEventArgs { Item = item });
+                    directoryWatchers.TemporarilyIgnore(path);
+                    saver.Save(item, CancellationToken.None);
                 }
                 catch (Exception ex)
                 {
-                    _logger.ErrorException("Error in ItemUpdated event handler", ex);
+                    _logger.ErrorException("Error in metadata saver", ex);
+                }
+                finally
+                {
+                    directoryWatchers.RemoveTempIgnore(path);
+                    semaphore.Release();
                 }
             }
         }
diff --git a/MediaBrowser.Server.Implementations/Providers/ProviderManager.cs b/MediaBrowser.Server.Implementations/Providers/ProviderManager.cs
index 01c0659b6..ee8bb4c09 100644
--- a/MediaBrowser.Server.Implementations/Providers/ProviderManager.cs
+++ b/MediaBrowser.Server.Implementations/Providers/ProviderManager.cs
@@ -60,8 +60,6 @@ namespace MediaBrowser.Server.Implementations.Providers
         /// <value>The metadata providers enumerable.</value>
         private BaseMetadataProvider[] MetadataProviders { get; set; }
 
-        private IEnumerable<IMetadataSaver> _savers;
-
         /// <summary>
         /// Initializes a new instance of the <see cref="ProviderManager" /> class.
         /// </summary>
@@ -79,44 +77,6 @@ namespace MediaBrowser.Server.Implementations.Providers
             _remoteImageCache = new FileSystemRepository(configurationManager.ApplicationPaths.DownloadedImagesDataPath);
 
             configurationManager.ConfigurationUpdated += configurationManager_ConfigurationUpdated;
-
-            libraryManager.ItemUpdated += libraryManager_ItemUpdated;
-        }
-
-        private readonly ConcurrentDictionary<string, SemaphoreSlim> _fileLocks = new ConcurrentDictionary<string, SemaphoreSlim>();
-        
-        /// <summary>
-        /// Handles the ItemUpdated event of the libraryManager control.
-        /// </summary>
-        /// <param name="sender">The source of the event.</param>
-        /// <param name="e">The <see cref="ItemChangeEventArgs"/> instance containing the event data.</param>
-        async void libraryManager_ItemUpdated(object sender, ItemChangeEventArgs e)
-        {
-            var item = e.Item;
-
-            foreach (var saver in _savers.Where(i => i.Supports(item)))
-            {
-                var path = saver.GetSavePath(item);
-
-                var semaphore = _fileLocks.GetOrAdd(path, key => new SemaphoreSlim(1, 1));
-
-                await semaphore.WaitAsync().ConfigureAwait(false);
-
-                try
-                {
-                    _directoryWatchers.TemporarilyIgnore(path);
-                    saver.Save(item, CancellationToken.None);
-                }
-                catch (Exception ex)
-                {
-                    _logger.ErrorException("Error in metadata saver", ex);
-                }
-                finally
-                {
-                    _directoryWatchers.RemoveTempIgnore(path);
-                    semaphore.Release();
-                }
-            }
         }
 
         /// <summary>
@@ -134,12 +94,9 @@ namespace MediaBrowser.Server.Implementations.Providers
         /// Adds the metadata providers.
         /// </summary>
         /// <param name="providers">The providers.</param>
-        /// <param name="savers">The savers.</param>
-        public void AddParts(IEnumerable<BaseMetadataProvider> providers,
-            IEnumerable<IMetadataSaver> savers)
+        public void AddParts(IEnumerable<BaseMetadataProvider> providers)
         {
             MetadataProviders = providers.OrderBy(e => e.Priority).ToArray();
-            _savers = savers;
         }
 
         /// <summary>
@@ -150,18 +107,14 @@ namespace MediaBrowser.Server.Implementations.Providers
         /// <param name="force">if set to <c>true</c> [force].</param>
         /// <param name="allowSlowProviders">if set to <c>true</c> [allow slow providers].</param>
         /// <returns>Task{System.Boolean}.</returns>
-        public async Task<bool> ExecuteMetadataProviders(BaseItem item, CancellationToken cancellationToken, bool force = false, bool allowSlowProviders = true)
+        public async Task<ItemUpdateType?> ExecuteMetadataProviders(BaseItem item, CancellationToken cancellationToken, bool force = false, bool allowSlowProviders = true)
         {
             if (item == null)
             {
                 throw new ArgumentNullException("item");
             }
 
-            // Allow providers of the same priority to execute in parallel
-            MetadataProviderPriority? currentPriority = null;
-            var currentTasks = new List<Task<bool>>();
-
-            var result = false;
+            ItemUpdateType? result = null;
 
             cancellationToken.ThrowIfCancellationRequested();
 
@@ -188,15 +141,6 @@ namespace MediaBrowser.Server.Implementations.Providers
                     continue;
                 }
 
-                // When a new priority is reached, await the ones that are currently running and clear the list
-                if (currentPriority.HasValue && currentPriority.Value != provider.Priority && currentTasks.Count > 0)
-                {
-                    var results = await Task.WhenAll(currentTasks).ConfigureAwait(false);
-                    result |= results.Contains(true);
-
-                    currentTasks.Clear();
-                }
-
                 // Put this check below the await because the needs refresh of the next tier of providers may depend on the previous ones running
                 //  This is the case for the fan art provider which depends on the movie and tv providers having run before them
                 if (provider.RequiresInternet && item.DontFetchMeta)
@@ -216,14 +160,19 @@ namespace MediaBrowser.Server.Implementations.Providers
                     _logger.Error("Error determining NeedsRefresh for {0}", ex, item.Path);
                 }
 
-                currentTasks.Add(FetchAsync(provider, item, force, cancellationToken));
-                currentPriority = provider.Priority;
-            }
+                var updateType = await FetchAsync(provider, item, force, cancellationToken).ConfigureAwait(false);
 
-            if (currentTasks.Count > 0)
-            {
-                var results = await Task.WhenAll(currentTasks).ConfigureAwait(false);
-                result |= results.Contains(true);
+                if (updateType.HasValue)
+                {
+                    if (result.HasValue)
+                    {
+                        result = result.Value | updateType.Value;
+                    }
+                    else
+                    {
+                        result = updateType;
+                    }
+                }
             }
 
             return result;
@@ -238,7 +187,7 @@ namespace MediaBrowser.Server.Implementations.Providers
         /// <param name="cancellationToken">The cancellation token.</param>
         /// <returns>Task{System.Boolean}.</returns>
         /// <exception cref="System.ArgumentNullException"></exception>
-        private async Task<bool> FetchAsync(BaseMetadataProvider provider, BaseItem item, bool force, CancellationToken cancellationToken)
+        private async Task<ItemUpdateType?> FetchAsync(BaseMetadataProvider provider, BaseItem item, bool force, CancellationToken cancellationToken)
         {
             if (item == null)
             {
@@ -256,7 +205,14 @@ namespace MediaBrowser.Server.Implementations.Providers
 
             try
             {
-                return await provider.FetchAsync(item, force, CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, innerCancellationTokenSource.Token).Token).ConfigureAwait(false);
+                var changed = await provider.FetchAsync(item, force, CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, innerCancellationTokenSource.Token).Token).ConfigureAwait(false);
+            
+                if (changed)
+                {
+                    return provider.ItemUpdateType;
+                }
+
+                return null;
             }
             catch (OperationCanceledException ex)
             {
@@ -268,14 +224,15 @@ namespace MediaBrowser.Server.Implementations.Providers
                     throw;
                 }
 
-                return false;
+                return null;
             }
             catch (Exception ex)
             {
                 _logger.ErrorException("{0} failed refreshing {1}", ex, provider.GetType().Name, item.Name);
 
                 provider.SetLastRefreshed(item, DateTime.UtcNow, ProviderRefreshStatus.Failure);
-                return true;
+
+                return ItemUpdateType.Unspecified;
             }
             finally
             {
diff --git a/MediaBrowser.Server.Implementations/ScheduledTasks/VideoImagesTask.cs b/MediaBrowser.Server.Implementations/ScheduledTasks/VideoImagesTask.cs
index d63494c1e..dc8425ac8 100644
--- a/MediaBrowser.Server.Implementations/ScheduledTasks/VideoImagesTask.cs
+++ b/MediaBrowser.Server.Implementations/ScheduledTasks/VideoImagesTask.cs
@@ -294,7 +294,7 @@ namespace MediaBrowser.Server.Implementations.ScheduledTasks
                     // Image is already in the cache
                     item.PrimaryImagePath = path;
 
-                    await _libraryManager.UpdateItem(item, cancellationToken).ConfigureAwait(false);
+                    await _libraryManager.UpdateItem(item, ItemUpdateType.ImageUpdate, cancellationToken).ConfigureAwait(false);
                 }
                 else
                 {