h1or4dme8/osu-lazer
1
0
Fork 0
mirror of https://github.com/ppy/osu.git synced 2025-01-12 18:23:04 +08:00
Code Issues Releases Wiki Activity

Refactor again to fix test failures

Browse Source
This commit is contained in:
Bartłomiej Dach 2023-05-07 17:49:31 +02:00
parent b895d4a42f
commit 29ce27098d
No known key found for this signature in database
5 changed files with 94 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

86
osu.Game.Tests/Beatmaps/BeatmapUpdaterMetadataLookupTest.cs
View File

@ -28,9 +28,11 @@ namespace osu.Game.Tests.Beatmaps
[Test]
public void TestLocalCacheQueriedFirst()
{
var localLookupResult = new OnlineBeatmapMetadata { BeatmapID = 123456, BeatmapStatus = BeatmapOnlineStatus.Ranked };
localCachedMetadataSourceMock.Setup(src => src.Available).Returns(true);
localCachedMetadataSourceMock.Setup(src => src.Lookup(It.IsAny<BeatmapInfo>()))
.Returns(new OnlineBeatmapMetadata { BeatmapID = 123456, BeatmapStatus = BeatmapOnlineStatus.Ranked });
localCachedMetadataSourceMock.Setup(src => src.TryLookup(It.IsAny<BeatmapInfo>(), out localLookupResult))
.Returns(true);
apiMetadataSourceMock.Setup(src => src.Available).Returns(true);
var beatmap = new BeatmapInfo { OnlineID = 123456 };
@ -40,19 +42,22 @@ namespace osu.Game.Tests.Beatmaps
metadataLookup.Update(beatmapSet, preferOnlineFetch: false);
Assert.That(beatmap.Status, Is.EqualTo(BeatmapOnlineStatus.Ranked));
localCachedMetadataSourceMock.Verify(src => src.Lookup(beatmap), Times.Once);
apiMetadataSourceMock.Verify(src => src.Lookup(It.IsAny<BeatmapInfo>()), Times.Never);
localCachedMetadataSourceMock.Verify(src => src.TryLookup(beatmap, out It.Ref<OnlineBeatmapMetadata>.IsAny!), Times.Once);
apiMetadataSourceMock.Verify(src => src.TryLookup(It.IsAny<BeatmapInfo>(), out It.Ref<OnlineBeatmapMetadata>.IsAny!), Times.Never);
}
[Test]
public void TestAPIQueriedSecond()
{
OnlineBeatmapMetadata? localLookupResult = null;
localCachedMetadataSourceMock.Setup(src => src.Available).Returns(true);
localCachedMetadataSourceMock.Setup(src => src.Lookup(It.IsAny<BeatmapInfo>()))
.Returns((OnlineBeatmapMetadata?)null);
localCachedMetadataSourceMock.Setup(src => src.TryLookup(It.IsAny<BeatmapInfo>(), out localLookupResult))
.Returns(false);
var onlineLookupResult = new OnlineBeatmapMetadata { BeatmapID = 123456, BeatmapStatus = BeatmapOnlineStatus.Ranked };
apiMetadataSourceMock.Setup(src => src.Available).Returns(true);
apiMetadataSourceMock.Setup(src => src.Lookup(It.IsAny<BeatmapInfo>()))
.Returns(new OnlineBeatmapMetadata { BeatmapID = 123456, BeatmapStatus = BeatmapOnlineStatus.Ranked });
apiMetadataSourceMock.Setup(src => src.TryLookup(It.IsAny<BeatmapInfo>(), out onlineLookupResult))
.Returns(true);
var beatmap = new BeatmapInfo { OnlineID = 123456 };
var beatmapSet = new BeatmapSetInfo(beatmap.Yield());
@ -61,19 +66,22 @@ namespace osu.Game.Tests.Beatmaps
metadataLookup.Update(beatmapSet, preferOnlineFetch: false);
Assert.That(beatmap.Status, Is.EqualTo(BeatmapOnlineStatus.Ranked));
localCachedMetadataSourceMock.Verify(src => src.Lookup(beatmap), Times.Once);
apiMetadataSourceMock.Verify(src => src.Lookup(beatmap), Times.Once);
localCachedMetadataSourceMock.Verify(src => src.TryLookup(beatmap, out It.Ref<OnlineBeatmapMetadata?>.IsAny!), Times.Once);
apiMetadataSourceMock.Verify(src => src.TryLookup(beatmap, out It.Ref<OnlineBeatmapMetadata?>.IsAny!), Times.Once);
}
[Test]
public void TestPreferOnlineFetch()
{
var localLookupResult = new OnlineBeatmapMetadata { BeatmapID = 123456, BeatmapStatus = BeatmapOnlineStatus.Ranked };
localCachedMetadataSourceMock.Setup(src => src.Available).Returns(true);
localCachedMetadataSourceMock.Setup(src => src.Lookup(It.IsAny<BeatmapInfo>()))
.Returns(new OnlineBeatmapMetadata { BeatmapID = 123456, BeatmapStatus = BeatmapOnlineStatus.Ranked });
localCachedMetadataSourceMock.Setup(src => src.TryLookup(It.IsAny<BeatmapInfo>(), out localLookupResult))
.Returns(true);
var onlineLookupResult = new OnlineBeatmapMetadata { BeatmapID = 123456, BeatmapStatus = BeatmapOnlineStatus.Graveyard };
apiMetadataSourceMock.Setup(src => src.Available).Returns(true);
apiMetadataSourceMock.Setup(src => src.Lookup(It.IsAny<BeatmapInfo>()))
.Returns(new OnlineBeatmapMetadata { BeatmapID = 123456, BeatmapStatus = BeatmapOnlineStatus.Graveyard });
apiMetadataSourceMock.Setup(src => src.TryLookup(It.IsAny<BeatmapInfo>(), out onlineLookupResult))
.Returns(true);
var beatmap = new BeatmapInfo { OnlineID = 123456 };
var beatmapSet = new BeatmapSetInfo(beatmap.Yield());
@ -82,16 +90,18 @@ namespace osu.Game.Tests.Beatmaps
metadataLookup.Update(beatmapSet, preferOnlineFetch: true);
Assert.That(beatmap.Status, Is.EqualTo(BeatmapOnlineStatus.Graveyard));
localCachedMetadataSourceMock.Verify(src => src.Lookup(beatmap), Times.Never);
apiMetadataSourceMock.Verify(src => src.Lookup(beatmap), Times.Once);
localCachedMetadataSourceMock.Verify(src => src.TryLookup(beatmap, out It.Ref<OnlineBeatmapMetadata?>.IsAny!), Times.Never);
apiMetadataSourceMock.Verify(src => src.TryLookup(beatmap, out It.Ref<OnlineBeatmapMetadata?>.IsAny!), Times.Once);
}
[Test]
public void TestPreferOnlineFetchFallsBackToLocalCacheIfOnlineSourceUnavailable()
{
var localLookupResult = new OnlineBeatmapMetadata { BeatmapID = 123456, BeatmapStatus = BeatmapOnlineStatus.Ranked };
localCachedMetadataSourceMock.Setup(src => src.Available).Returns(true);
localCachedMetadataSourceMock.Setup(src => src.Lookup(It.IsAny<BeatmapInfo>()))
.Returns(new OnlineBeatmapMetadata { BeatmapID = 123456, BeatmapStatus = BeatmapOnlineStatus.Ranked });
localCachedMetadataSourceMock.Setup(src => src.TryLookup(It.IsAny<BeatmapInfo>(), out localLookupResult))
.Returns(true);
apiMetadataSourceMock.Setup(src => src.Available).Returns(false);
var beatmap = new BeatmapInfo { OnlineID = 123456 };
@ -101,19 +111,22 @@ namespace osu.Game.Tests.Beatmaps
metadataLookup.Update(beatmapSet, preferOnlineFetch: true);
Assert.That(beatmap.Status, Is.EqualTo(BeatmapOnlineStatus.Ranked));
localCachedMetadataSourceMock.Verify(src => src.Lookup(beatmap), Times.Once);
apiMetadataSourceMock.Verify(src => src.Lookup(beatmap), Times.Never);
localCachedMetadataSourceMock.Verify(src => src.TryLookup(beatmap, out It.Ref<OnlineBeatmapMetadata?>.IsAny!), Times.Once);
apiMetadataSourceMock.Verify(src => src.TryLookup(beatmap, out It.Ref<OnlineBeatmapMetadata?>.IsAny!), Times.Never);
}
[Test]
public void TestMetadataLookupFailed()
{
OnlineBeatmapMetadata? lookupResult = null;
localCachedMetadataSourceMock.Setup(src => src.Available).Returns(true);
localCachedMetadataSourceMock.Setup(src => src.Lookup(It.IsAny<BeatmapInfo>()))
.Returns((OnlineBeatmapMetadata?)null);
localCachedMetadataSourceMock.Setup(src => src.TryLookup(It.IsAny<BeatmapInfo>(), out lookupResult))
.Returns(false);
apiMetadataSourceMock.Setup(src => src.Available).Returns(true);
apiMetadataSourceMock.Setup(src => src.Lookup(It.IsAny<BeatmapInfo>()))
.Returns((OnlineBeatmapMetadata?)null);
apiMetadataSourceMock.Setup(src => src.TryLookup(It.IsAny<BeatmapInfo>(), out lookupResult))
.Returns(true);
var beatmap = new BeatmapInfo { OnlineID = 123456 };
var beatmapSet = new BeatmapSetInfo(beatmap.Yield());
@ -123,8 +136,8 @@ namespace osu.Game.Tests.Beatmaps
Assert.That(beatmap.Status, Is.EqualTo(BeatmapOnlineStatus.None));
Assert.That(beatmap.OnlineID, Is.EqualTo(-1));
localCachedMetadataSourceMock.Verify(src => src.Lookup(beatmap), Times.Once);
apiMetadataSourceMock.Verify(src => src.Lookup(beatmap), Times.Once);
localCachedMetadataSourceMock.Verify(src => src.TryLookup(beatmap, out It.Ref<OnlineBeatmapMetadata?>.IsAny!), Times.Once);
apiMetadataSourceMock.Verify(src => src.TryLookup(beatmap, out It.Ref<OnlineBeatmapMetadata?>.IsAny!), Times.Once);
}
/// <remarks>
@ -134,11 +147,13 @@ namespace osu.Game.Tests.Beatmaps
/// TODO: revisit if/when we have a better flow of queueing metadata retrieval.
/// </remarks>
[Test]
public void TestLocalMetadataLookupFailedAndOnlineLookupIsUnavailable([Values] bool preferOnlineFetch)
public void TestLocalMetadataLookupReturnedNoMatchAndOnlineLookupIsUnavailable([Values] bool preferOnlineFetch)
{
OnlineBeatmapMetadata? localLookupResult = null;
localCachedMetadataSourceMock.Setup(src => src.Available).Returns(true);
localCachedMetadataSourceMock.Setup(src => src.Lookup(It.IsAny<BeatmapInfo>()))
.Returns((OnlineBeatmapMetadata?)null);
localCachedMetadataSourceMock.Setup(src => src.TryLookup(It.IsAny<BeatmapInfo>(), out localLookupResult))
.Returns(false);
apiMetadataSourceMock.Setup(src => src.Available).Returns(false);
var beatmap = new BeatmapInfo { OnlineID = 123456 };
@ -158,20 +173,25 @@ namespace osu.Game.Tests.Beatmaps
/// TODO: revisit if/when we have a better flow of queueing metadata retrieval.
/// </remarks>
[Test]
public void TestNoAvailableSources()
public void TestNoAvailableSources([Values] bool preferOnlineFetch)
{
OnlineBeatmapMetadata? lookupResult = null;
localCachedMetadataSourceMock.Setup(src => src.Available).Returns(false);
localCachedMetadataSourceMock.Setup(src => src.TryLookup(It.IsAny<BeatmapInfo>(), out lookupResult))
.Returns(false);
apiMetadataSourceMock.Setup(src => src.Available).Returns(false);
apiMetadataSourceMock.Setup(src => src.TryLookup(It.IsAny<BeatmapInfo>(), out lookupResult))
.Returns(false);
var beatmap = new BeatmapInfo { OnlineID = 123456 };
var beatmapSet = new BeatmapSetInfo(beatmap.Yield());
beatmap.BeatmapSet = beatmapSet;
metadataLookup.Update(beatmapSet, preferOnlineFetch: false);
metadataLookup.Update(beatmapSet, preferOnlineFetch);
Assert.That(beatmap.OnlineID, Is.EqualTo(123456));
localCachedMetadataSourceMock.Verify(src => src.Lookup(beatmap), Times.Never);
apiMetadataSourceMock.Verify(src => src.Lookup(beatmap), Times.Never);
}
}
}

18
osu.Game/Beatmaps/APIBeatmapMetadataSource.cs
View File

@ -20,10 +20,13 @@ namespace osu.Game.Beatmaps
public bool Available => api.State.Value == APIState.Online;
public OnlineBeatmapMetadata? Lookup(BeatmapInfo beatmapInfo)
public bool TryLookup(BeatmapInfo beatmapInfo, out OnlineBeatmapMetadata? onlineMetadata)
{
if (!Available)
return null;
{
onlineMetadata = null;
return false;
}
Debug.Assert(beatmapInfo.BeatmapSet != null);
@ -37,7 +40,8 @@ namespace osu.Game.Beatmaps
if (req.CompletionState == APIRequestCompletionState.Failed)
{
logForModel(beatmapInfo.BeatmapSet, $@"Online retrieval failed for {beatmapInfo}");
return null;
onlineMetadata = null;
return true;
}
var res = req.Response;
@ -46,7 +50,7 @@ namespace osu.Game.Beatmaps
{
logForModel(beatmapInfo.BeatmapSet, $@"Online retrieval mapped {beatmapInfo} to {res.OnlineBeatmapSetID} / {res.OnlineID}.");
return new OnlineBeatmapMetadata
onlineMetadata = new OnlineBeatmapMetadata
{
BeatmapID = res.OnlineID,
BeatmapSetID = res.OnlineBeatmapSetID,
@ -58,14 +62,18 @@ namespace osu.Game.Beatmaps
MD5Hash = res.MD5Hash,
LastUpdated = res.LastUpdated
};
return true;
}
}
catch (Exception e)
{
logForModel(beatmapInfo.BeatmapSet, $@"Online retrieval failed for {beatmapInfo} ({e.Message})");
onlineMetadata = null;
return false;
}
return null;
onlineMetadata = null;
return false;
}
private void logForModel(BeatmapSetInfo set, string message) =>

16
osu.Game/Beatmaps/BeatmapUpdaterMetadataLookup.cs
View File

@ -93,18 +93,12 @@ namespace osu.Game.Beatmaps
/// </remarks>
private bool tryLookup(BeatmapInfo beatmapInfo, bool preferOnlineFetch, out OnlineBeatmapMetadata? result)
{
if (localCachedMetadataSource.Available && (!apiMetadataSource.Available || !preferOnlineFetch))
{
result = localCachedMetadataSource.Lookup(beatmapInfo);
if (result != null)
return true;
}
if (apiMetadataSource.Available)
{
result = apiMetadataSource.Lookup(beatmapInfo);
bool useLocalCache = !apiMetadataSource.Available || !preferOnlineFetch;
if (useLocalCache && localCachedMetadataSource.TryLookup(beatmapInfo, out result))
return true;
if (apiMetadataSource.TryLookup(beatmapInfo, out result))
return true;
}
result = null;
return false;

10
osu.Game/Beatmaps/IOnlineBeatmapMetadataSource.cs
View File

@ -18,9 +18,15 @@ namespace osu.Game.Beatmaps
/// <summary>
/// Looks up the online metadata for the supplied <paramref name="beatmapInfo"/>.
/// </summary>
/// <param name="beatmapInfo">The <see cref="BeatmapInfo"/> to look up.</param>
/// <param name="onlineMetadata">
/// An <see cref="OnlineBeatmapMetadata"/> instance if the lookup is successful.
/// <see langword="null"/> if a mismatch between the local instance and the looked-up data was detected.
/// The returned value is only valid if the return value of the method is <see langword="true"/>.
/// </param>
/// <returns>
/// An <see cref="OnlineBeatmapMetadata"/> instance if the lookup is successful, or <see langword="null"/> if the lookup did not return a matching beatmap.
/// Whether the lookup was performed.
/// </returns>
OnlineBeatmapMetadata? Lookup(BeatmapInfo beatmapInfo);
bool TryLookup(BeatmapInfo beatmapInfo, out OnlineBeatmapMetadata? onlineMetadata);
}
}

20
osu.Game/Beatmaps/LocalCachedBeatmapMetadataSource.cs
View File

@ -108,15 +108,21 @@ namespace osu.Game.Beatmaps
// cached database exists on disk.
&& storage.Exists(cache_database_name);
public OnlineBeatmapMetadata? Lookup(BeatmapInfo beatmapInfo)
public bool TryLookup(BeatmapInfo beatmapInfo, out OnlineBeatmapMetadata? onlineMetadata)
{
if (!Available)
return null;
{
onlineMetadata = null;
return false;
}
if (string.IsNullOrEmpty(beatmapInfo.MD5Hash)
&& string.IsNullOrEmpty(beatmapInfo.Path)
&& beatmapInfo.OnlineID <= 0)
return null;
{
onlineMetadata = null;
return false;
}
Debug.Assert(beatmapInfo.BeatmapSet != null);
@ -141,7 +147,7 @@ namespace osu.Game.Beatmaps
{
logForModel(beatmapInfo.BeatmapSet, $@"Cached local retrieval for {beatmapInfo}.");
return new OnlineBeatmapMetadata
onlineMetadata = new OnlineBeatmapMetadata
{
BeatmapSetID = reader.GetInt32(0),
BeatmapID = reader.GetInt32(1),
@ -152,6 +158,7 @@ namespace osu.Game.Beatmaps
LastUpdated = reader.GetDateTimeOffset(5),
// TODO: DateSubmitted and DateRanked are not provided by local cache.
};
return true;
}
}
}
@ -160,9 +167,12 @@ namespace osu.Game.Beatmaps
catch (Exception ex)
{
logForModel(beatmapInfo.BeatmapSet, $@"Cached local retrieval for {beatmapInfo} failed with {ex}.");
onlineMetadata = null;
return false;
}
return null;
onlineMetadata = null;
return false;
}
private void logForModel(BeatmapSetInfo set, string message) =>