Merge pull request #19440 from peppy/bypass-local-metadata-cache

Fix beatmap updater potentially using outdated local metadata
This commit is contained in:
Dan Balasescu 2022-07-29 17:33:53 +09:00 committed by GitHub
commit 70420e6238
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 54 additions and 50 deletions

View File

@ -35,7 +35,7 @@ namespace osu.Game.Beatmaps
protected override string[] HashableFileTypes => new[] { ".osu" };
public Action<BeatmapSetInfo>? ProcessBeatmap { private get; set; }
public Action<(BeatmapSetInfo beatmapSet, bool isBatch)>? ProcessBeatmap { private get; set; }
public BeatmapImporter(Storage storage, RealmAccess realm)
: base(storage, realm)
@ -205,11 +205,10 @@ namespace osu.Game.Beatmaps
}
}
protected override void PostImport(BeatmapSetInfo model, Realm realm)
protected override void PostImport(BeatmapSetInfo model, Realm realm, bool batchImport)
{
base.PostImport(model, realm);
ProcessBeatmap?.Invoke(model);
base.PostImport(model, realm, batchImport);
ProcessBeatmap?.Invoke((model, batchImport));
}
private void validateOnlineIds(BeatmapSetInfo beatmapSet, Realm realm)

View File

@ -42,7 +42,7 @@ namespace osu.Game.Beatmaps
private readonly WorkingBeatmapCache workingBeatmapCache;
public Action<BeatmapSetInfo>? ProcessBeatmap { private get; set; }
public Action<(BeatmapSetInfo beatmapSet, bool isBatch)>? ProcessBeatmap { private get; set; }
public BeatmapManager(Storage storage, RealmAccess realm, IAPIProvider? api, AudioManager audioManager, IResourceStore<byte[]> gameResources, GameHost? host = null,
WorkingBeatmap? defaultBeatmap = null, BeatmapDifficultyCache? difficultyCache = null, bool performOnlineLookups = false)
@ -62,7 +62,7 @@ namespace osu.Game.Beatmaps
BeatmapTrackStore = audioManager.GetTrackStore(userResources);
beatmapImporter = CreateBeatmapImporter(storage, realm);
beatmapImporter.ProcessBeatmap = obj => ProcessBeatmap?.Invoke(obj);
beatmapImporter.ProcessBeatmap = args => ProcessBeatmap?.Invoke(args);
beatmapImporter.PostNotification = obj => PostNotification?.Invoke(obj);
workingBeatmapCache = CreateWorkingBeatmapCache(audioManager, gameResources, userResources, defaultBeatmap, host);
@ -323,7 +323,7 @@ namespace osu.Game.Beatmaps
setInfo.CopyChangesToRealm(liveBeatmapSet);
ProcessBeatmap?.Invoke(liveBeatmapSet);
ProcessBeatmap?.Invoke((liveBeatmapSet, false));
});
}

View File

@ -36,7 +36,7 @@ namespace osu.Game.Beatmaps
var matchingSet = r.All<BeatmapSetInfo>().FirstOrDefault(s => s.OnlineID == id);
if (matchingSet != null)
beatmapUpdater.Queue(matchingSet.ToLive(realm));
beatmapUpdater.Queue(matchingSet.ToLive(realm), true);
}
});
}

View File

@ -8,6 +8,7 @@ using System.Threading.Tasks;
using osu.Framework.Extensions.ObjectExtensions;
using osu.Framework.Logging;
using osu.Framework.Platform;
using osu.Framework.Threading;
using osu.Game.Database;
using osu.Game.Online.API;
using osu.Game.Rulesets.Objects;
@ -20,37 +21,45 @@ namespace osu.Game.Beatmaps
public class BeatmapUpdater : IDisposable
{
private readonly IWorkingBeatmapCache workingBeatmapCache;
private readonly BeatmapOnlineLookupQueue onlineLookupQueue;
private readonly BeatmapDifficultyCache difficultyCache;
private readonly BeatmapUpdaterMetadataLookup metadataLookup;
private const int update_queue_request_concurrency = 4;
private readonly ThreadedTaskScheduler updateScheduler = new ThreadedTaskScheduler(update_queue_request_concurrency, nameof(BeatmapUpdaterMetadataLookup));
public BeatmapUpdater(IWorkingBeatmapCache workingBeatmapCache, BeatmapDifficultyCache difficultyCache, IAPIProvider api, Storage storage)
{
this.workingBeatmapCache = workingBeatmapCache;
this.difficultyCache = difficultyCache;
onlineLookupQueue = new BeatmapOnlineLookupQueue(api, storage);
metadataLookup = new BeatmapUpdaterMetadataLookup(api, storage);
}
/// <summary>
/// Queue a beatmap for background processing.
/// </summary>
public void Queue(Live<BeatmapSetInfo> beatmap)
/// <param name="beatmapSet">The managed beatmap set to update. A transaction will be opened to apply changes.</param>
/// <param name="preferOnlineFetch">Whether metadata from an online source should be preferred. If <c>true</c>, the local cache will be skipped to ensure the freshest data state possible.</param>
public void Queue(Live<BeatmapSetInfo> beatmapSet, bool preferOnlineFetch = false)
{
Logger.Log($"Queueing change for local beatmap {beatmap}");
Task.Factory.StartNew(() => beatmap.PerformRead(Process));
Logger.Log($"Queueing change for local beatmap {beatmapSet}");
Task.Factory.StartNew(() => beatmapSet.PerformRead(b => Process(b, preferOnlineFetch)), default, TaskCreationOptions.HideScheduler | TaskCreationOptions.RunContinuationsAsynchronously, updateScheduler);
}
/// <summary>
/// Run all processing on a beatmap immediately.
/// </summary>
public void Process(BeatmapSetInfo beatmapSet) => beatmapSet.Realm.Write(r =>
/// <param name="beatmapSet">The managed beatmap set to update. A transaction will be opened to apply changes.</param>
/// <param name="preferOnlineFetch">Whether metadata from an online source should be preferred. If <c>true</c>, the local cache will be skipped to ensure the freshest data state possible.</param>
public void Process(BeatmapSetInfo beatmapSet, bool preferOnlineFetch = false) => beatmapSet.Realm.Write(r =>
{
// Before we use below, we want to invalidate.
workingBeatmapCache.Invalidate(beatmapSet);
// TODO: this call currently uses the local `online.db` lookup.
// We probably don't want this to happen after initial import (as the data may be stale).
onlineLookupQueue.Update(beatmapSet);
metadataLookup.Update(beatmapSet, preferOnlineFetch);
foreach (var beatmap in beatmapSet.Beatmaps)
{
@ -90,8 +99,11 @@ namespace osu.Game.Beatmaps
public void Dispose()
{
if (onlineLookupQueue.IsNotNull())
onlineLookupQueue.Dispose();
if (metadataLookup.IsNotNull())
metadataLookup.Dispose();
if (updateScheduler.IsNotNull())
updateScheduler.Dispose();
}
#endregion

View File

@ -6,8 +6,6 @@
using System;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Data.Sqlite;
using osu.Framework.Development;
@ -15,7 +13,6 @@ using osu.Framework.IO.Network;
using osu.Framework.Logging;
using osu.Framework.Platform;
using osu.Framework.Testing;
using osu.Framework.Threading;
using osu.Game.Database;
using osu.Game.Online.API;
using osu.Game.Online.API.Requests;
@ -32,20 +29,16 @@ namespace osu.Game.Beatmaps
/// This will always be checked before doing a second online query to get required metadata.
/// </remarks>
[ExcludeFromDynamicCompile]
public class BeatmapOnlineLookupQueue : IDisposable
public class BeatmapUpdaterMetadataLookup : IDisposable
{
private readonly IAPIProvider api;
private readonly Storage storage;
private const int update_queue_request_concurrency = 4;
private readonly ThreadedTaskScheduler updateScheduler = new ThreadedTaskScheduler(update_queue_request_concurrency, nameof(BeatmapOnlineLookupQueue));
private FileWebRequest cacheDownloadRequest;
private const string cache_database_name = "online.db";
public BeatmapOnlineLookupQueue(IAPIProvider api, Storage storage)
public BeatmapUpdaterMetadataLookup(IAPIProvider api, Storage storage)
{
this.api = api;
this.storage = storage;
@ -55,27 +48,27 @@ namespace osu.Game.Beatmaps
prepareLocalCache();
}
public void Update(BeatmapSetInfo beatmapSet)
/// <summary>
/// Queue an update for a beatmap set.
/// </summary>
/// <param name="beatmapSet">The beatmap set to update. Updates will be applied directly (so a transaction should be started if this instance is managed).</param>
/// <param name="preferOnlineFetch">Whether metadata from an online source should be preferred. If <c>true</c>, the local cache will be skipped to ensure the freshest data state possible.</param>
public void Update(BeatmapSetInfo beatmapSet, bool preferOnlineFetch)
{
foreach (var b in beatmapSet.Beatmaps)
lookup(beatmapSet, b);
lookup(beatmapSet, b, preferOnlineFetch);
}
public Task UpdateAsync(BeatmapSetInfo beatmapSet, CancellationToken cancellationToken)
private void lookup(BeatmapSetInfo set, BeatmapInfo beatmapInfo, bool preferOnlineFetch)
{
return Task.WhenAll(beatmapSet.Beatmaps.Select(b => UpdateAsync(beatmapSet, b, cancellationToken)).ToArray());
}
bool apiAvailable = api?.State.Value == APIState.Online;
// todo: expose this when we need to do individual difficulty lookups.
protected Task UpdateAsync(BeatmapSetInfo beatmapSet, BeatmapInfo beatmapInfo, CancellationToken cancellationToken)
=> Task.Factory.StartNew(() => lookup(beatmapSet, beatmapInfo), cancellationToken, TaskCreationOptions.HideScheduler | TaskCreationOptions.RunContinuationsAsynchronously, updateScheduler);
bool useLocalCache = !apiAvailable || !preferOnlineFetch;
private void lookup(BeatmapSetInfo set, BeatmapInfo beatmapInfo)
{
if (checkLocalCache(set, beatmapInfo))
if (useLocalCache && checkLocalCache(set, beatmapInfo))
return;
if (api?.State.Value != APIState.Online)
if (!apiAvailable)
return;
var req = new GetBeatmapRequest(beatmapInfo);
@ -134,7 +127,7 @@ namespace osu.Game.Beatmaps
File.Delete(compressedCacheFilePath);
File.Delete(cacheFilePath);
Logger.Log($"{nameof(BeatmapOnlineLookupQueue)}'s online cache download failed: {ex}", LoggingTarget.Database);
Logger.Log($"{nameof(BeatmapUpdaterMetadataLookup)}'s online cache download failed: {ex}", LoggingTarget.Database);
};
cacheDownloadRequest.Finished += () =>
@ -151,7 +144,7 @@ namespace osu.Game.Beatmaps
}
catch (Exception ex)
{
Logger.Log($"{nameof(BeatmapOnlineLookupQueue)}'s online cache extraction failed: {ex}", LoggingTarget.Database);
Logger.Log($"{nameof(BeatmapUpdaterMetadataLookup)}'s online cache extraction failed: {ex}", LoggingTarget.Database);
File.Delete(cacheFilePath);
}
finally
@ -238,12 +231,11 @@ namespace osu.Game.Beatmaps
}
private void logForModel(BeatmapSetInfo set, string message) =>
RealmArchiveModelImporter<BeatmapSetInfo>.LogForModel(set, $"[{nameof(BeatmapOnlineLookupQueue)}] {message}");
RealmArchiveModelImporter<BeatmapSetInfo>.LogForModel(set, $"[{nameof(BeatmapUpdaterMetadataLookup)}] {message}");
public void Dispose()
{
cacheDownloadRequest?.Dispose();
updateScheduler?.Dispose();
}
}
}

View File

@ -340,7 +340,7 @@ namespace osu.Game.Database
// import to store
realm.Add(item);
PostImport(item, realm);
PostImport(item, realm, batchImport);
transaction.Commit();
}
@ -485,7 +485,8 @@ namespace osu.Game.Database
/// </summary>
/// <param name="model">The model prepared for import.</param>
/// <param name="realm">The current realm context.</param>
protected virtual void PostImport(TModel model, Realm realm)
/// <param name="batchImport">Whether the import was part of a batch.</param>
protected virtual void PostImport(TModel model, Realm realm, bool batchImport)
{
}

View File

@ -287,7 +287,7 @@ namespace osu.Game
AddInternal(new BeatmapOnlineChangeIngest(beatmapUpdater, realm, metadataClient));
BeatmapManager.ProcessBeatmap = set => beatmapUpdater.Process(set);
BeatmapManager.ProcessBeatmap = args => beatmapUpdater.Process(args.beatmapSet, !args.isBatch);
dependencies.Cache(userCache = new UserLookupCache());
AddInternal(userCache);

View File

@ -75,9 +75,9 @@ namespace osu.Game.Scoring
model.StatisticsJson = JsonConvert.SerializeObject(model.Statistics);
}
protected override void PostImport(ScoreInfo model, Realm realm)
protected override void PostImport(ScoreInfo model, Realm realm, bool batchImport)
{
base.PostImport(model, realm);
base.PostImport(model, realm, batchImport);
var userRequest = new GetUserRequest(model.RealmUser.Username);