From 6ca2fcebfce6467398d12fc69911ffb3f1c03eff Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Mon, 10 Jun 2019 19:34:32 +0900 Subject: [PATCH] Centalise and prefix all ArchiveModelManager database logging --- osu.Game/Beatmaps/BeatmapManager.cs | 28 ++++++++++-------- osu.Game/Database/ArchiveModelManager.cs | 36 +++++++++++++++--------- 2 files changed, 39 insertions(+), 25 deletions(-) diff --git a/osu.Game/Beatmaps/BeatmapManager.cs b/osu.Game/Beatmaps/BeatmapManager.cs index 435edcf722..47411c69ec 100644 --- a/osu.Game/Beatmaps/BeatmapManager.cs +++ b/osu.Game/Beatmaps/BeatmapManager.cs @@ -110,7 +110,7 @@ namespace osu.Game.Beatmaps validateOnlineIds(beatmapSet); - await Task.WhenAll(beatmapSet.Beatmaps.Select(b => updateQueue.Perform(b, cancellationToken)).ToArray()); + await updateQueue.Perform(beatmapSet, cancellationToken); } protected override void PreImport(BeatmapSetInfo beatmapSet) @@ -127,7 +127,7 @@ namespace osu.Game.Beatmaps { Delete(existingOnlineId); beatmaps.PurgeDeletable(s => s.ID == existingOnlineId.ID); - Logger.Log($"Found existing beatmap set with same OnlineBeatmapSetID ({beatmapSet.OnlineBeatmapSetID}). It has been purged.", LoggingTarget.Database); + LogForModel(beatmapSet, $"Found existing beatmap set with same OnlineBeatmapSetID ({beatmapSet.OnlineBeatmapSetID}). It has been purged."); } } } @@ -433,23 +433,29 @@ namespace osu.Game.Beatmaps this.api = api; } - public Task Perform(BeatmapInfo beatmap, CancellationToken cancellationToken) - => Task.Factory.StartNew(() => perform(beatmap, cancellationToken), cancellationToken, TaskCreationOptions.HideScheduler, updateScheduler); - - private void perform(BeatmapInfo beatmap, CancellationToken cancellation) + public async Task Perform(BeatmapSetInfo beatmapSet, CancellationToken cancellationToken) { - cancellation.ThrowIfCancellationRequested(); - if (api?.State != APIState.Online) return; - Logger.Log("Attempting online lookup for the missing values...", LoggingTarget.Database); + LogForModel(beatmapSet, "Performing online lookups..."); + await Task.WhenAll(beatmapSet.Beatmaps.Select(b => Perform(beatmapSet, b, cancellationToken)).ToArray()); + } + + // todo: expose this when we need to do individual difficulty lookups. + protected Task Perform(BeatmapSetInfo beatmapSet, BeatmapInfo beatmap, CancellationToken cancellationToken) + => Task.Factory.StartNew(() => perform(beatmapSet, beatmap), cancellationToken, TaskCreationOptions.HideScheduler, updateScheduler); + + private void perform(BeatmapSetInfo set, BeatmapInfo beatmap) + { + if (api?.State != APIState.Online) + return; var req = new GetBeatmapRequest(beatmap); req.Success += res => { - Logger.Log($"Successfully mapped to {res.OnlineBeatmapSetID} / {res.OnlineBeatmapID}.", LoggingTarget.Database); + LogForModel(set, $"Online retrieval mapped {beatmap} to {res.OnlineBeatmapSetID} / {res.OnlineBeatmapID}."); beatmap.Status = res.Status; beatmap.BeatmapSet.Status = res.BeatmapSet.Status; @@ -457,7 +463,7 @@ namespace osu.Game.Beatmaps beatmap.OnlineBeatmapID = res.OnlineBeatmapID; }; - req.Failure += e => { Logger.Log($"Failed ({e})", LoggingTarget.Database); }; + req.Failure += e => { LogForModel(set, $"Online retrieval failed for {beatmap}", e); }; // intentionally blocking to limit web request concurrency req.Perform(api); diff --git a/osu.Game/Database/ArchiveModelManager.cs b/osu.Game/Database/ArchiveModelManager.cs index 4e9478dc10..844ae622a5 100644 --- a/osu.Game/Database/ArchiveModelManager.cs +++ b/osu.Game/Database/ArchiveModelManager.cs @@ -173,7 +173,7 @@ namespace osu.Game.Database } catch (Exception e) { - Logger.Error(e, $@"Could not import ({Path.GetFileName(path)})"); + Logger.Error(e, $@"Could not import ({Path.GetFileName(path)})", LoggingTarget.Database); } })); @@ -227,7 +227,7 @@ namespace osu.Game.Database } catch (Exception e) { - Logger.Error(e, $@"Could not delete original file after import ({Path.GetFileName(path)})"); + LogForModel(import, $@"Could not delete original file after import ({Path.GetFileName(path)})", e); } return import; @@ -247,7 +247,7 @@ namespace osu.Game.Database { cancellationToken.ThrowIfCancellationRequested(); - TModel model; + TModel model = null; try { @@ -263,7 +263,7 @@ namespace osu.Game.Database } catch (Exception e) { - Logger.Error(e, $"Model creation of {archive.Name} failed.", LoggingTarget.Database); + LogForModel(model, $"Model creation of {archive.Name} failed.", e); return null; } @@ -277,6 +277,16 @@ namespace osu.Game.Database /// protected abstract string[] HashableFileTypes { get; } + protected static void LogForModel(TModel model, string message, Exception e = null) + { + string prefix = $"[{(model?.Hash ?? "?????").Substring(0, 5)}]"; + + if (e != null) + Logger.Error(e, $"{prefix} {message}", LoggingTarget.Database); + else + Logger.Log($"{prefix} {message}", LoggingTarget.Database); + } + /// /// Create a SHA-2 hash from the provided archive based on file content of all files matching . /// @@ -308,14 +318,14 @@ namespace osu.Game.Database if (!Delete(item)) { // We may have not yet added the model to the underlying table, but should still clean up files. - Logger.Log($"Dereferencing files for incomplete import of {item}.", LoggingTarget.Database); + LogForModel(item, "Dereferencing files for incomplete import."); Files.Dereference(item.Files.Select(f => f.FileInfo).ToArray()); } } try { - Logger.Log($"Importing {item}...", LoggingTarget.Database); + LogForModel(item, "Beginning import..."); item.Files = archive != null ? createFileInfos(archive, Files) : new List(); @@ -334,7 +344,7 @@ namespace osu.Game.Database if (CanUndelete(existing, item)) { Undelete(existing); - Logger.Log($"Found existing {humanisedModelName} for {item} (ID {existing.ID}). Skipping import.", LoggingTarget.Database); + LogForModel(item, $"Found existing {humanisedModelName} for {item} (ID {existing.ID}) – skipping import."); handleEvent(() => ItemAdded?.Invoke(existing, true)); // existing item will be used; rollback new import and exit early. @@ -342,11 +352,9 @@ namespace osu.Game.Database flushEvents(true); return existing; } - else - { - Delete(existing); - ModelStore.PurgeDeletable(s => s.ID == existing.ID); - } + + Delete(existing); + ModelStore.PurgeDeletable(s => s.ID == existing.ID); } PreImport(item); @@ -361,12 +369,12 @@ namespace osu.Game.Database } } - Logger.Log($"Import of {item} successfully completed!", LoggingTarget.Database); + LogForModel(item, "Import successfully completed!"); } catch (Exception e) { if (!(e is TaskCanceledException)) - Logger.Error(e, $"Import of {item} failed and has been rolled back.", LoggingTarget.Database); + LogForModel(item, "Database import or population failed and has been rolled back.", e); rollback(); flushEvents(false);