1
0
mirror of https://github.com/ppy/osu.git synced 2024-11-11 12:17:26 +08:00

Merge pull request #13633 from peppy/fix-beatmap-import-fk-failure

Fix import flow potentially hitting foreign key constraint
This commit is contained in:
Dan Balasescu 2021-06-29 15:03:13 +09:00 committed by GitHub
commit f547afe617
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 106 additions and 19 deletions

View File

@ -19,7 +19,9 @@ using osu.Game.Database;
using osu.Game.IO;
using osu.Game.Rulesets.Osu;
using osu.Game.Rulesets.Osu.Objects;
using osu.Game.Scoring;
using osu.Game.Tests.Resources;
using osu.Game.Tests.Scores.IO;
using osu.Game.Users;
using SharpCompress.Archives;
using SharpCompress.Archives.Zip;
@ -185,10 +187,58 @@ namespace osu.Game.Tests.Beatmaps.IO
}
}
private string hashFile(string filename)
[Test]
public async Task TestImportThenImportWithChangedHashedFile()
{
using (var s = File.OpenRead(filename))
return s.ComputeMD5Hash();
using (HeadlessGameHost host = new CleanRunHeadlessGameHost(nameof(ImportBeatmapTest)))
{
try
{
var osu = LoadOsuIntoHost(host);
var temp = TestResources.GetTestBeatmapForImport();
string extractedFolder = $"{temp}_extracted";
Directory.CreateDirectory(extractedFolder);
try
{
var imported = await LoadOszIntoOsu(osu);
await createScoreForBeatmap(osu, imported.Beatmaps.First());
using (var zip = ZipArchive.Open(temp))
zip.WriteToDirectory(extractedFolder);
// arbitrary write to hashed file
// this triggers the special BeatmapManager.PreImport deletion/replacement flow.
using (var sw = new FileInfo(Directory.GetFiles(extractedFolder, "*.osu").First()).AppendText())
await sw.WriteLineAsync("// changed");
using (var zip = ZipArchive.Create())
{
zip.AddAllFromDirectory(extractedFolder);
zip.SaveTo(temp, new ZipWriterOptions(CompressionType.Deflate));
}
var importedSecondTime = await osu.Dependencies.Get<BeatmapManager>().Import(new ImportTask(temp));
ensureLoaded(osu);
// check the newly "imported" beatmap is not the original.
Assert.IsTrue(imported.ID != importedSecondTime.ID);
Assert.IsTrue(imported.Beatmaps.First().ID != importedSecondTime.Beatmaps.First().ID);
}
finally
{
Directory.Delete(extractedFolder, true);
}
}
finally
{
host.Exit();
}
}
}
[Test]
@ -894,7 +944,17 @@ namespace osu.Game.Tests.Beatmaps.IO
Assert.IsTrue(manager.QueryBeatmapSets(_ => true).First().DeletePending);
}
private void checkBeatmapSetCount(OsuGameBase osu, int expected, bool includeDeletePending = false)
private static Task createScoreForBeatmap(OsuGameBase osu, BeatmapInfo beatmap)
{
return ImportScoreTest.LoadScoreIntoOsu(osu, new ScoreInfo
{
OnlineScoreID = 2,
Beatmap = beatmap,
BeatmapInfoID = beatmap.ID
}, new ImportScoreTest.TestArchiveReader());
}
private static void checkBeatmapSetCount(OsuGameBase osu, int expected, bool includeDeletePending = false)
{
var manager = osu.Dependencies.Get<BeatmapManager>();
@ -903,12 +963,18 @@ namespace osu.Game.Tests.Beatmaps.IO
: manager.GetAllUsableBeatmapSets().Count);
}
private void checkBeatmapCount(OsuGameBase osu, int expected)
private static string hashFile(string filename)
{
using (var s = File.OpenRead(filename))
return s.ComputeMD5Hash();
}
private static void checkBeatmapCount(OsuGameBase osu, int expected)
{
Assert.AreEqual(expected, osu.Dependencies.Get<BeatmapManager>().QueryBeatmaps(_ => true).ToList().Count);
}
private void checkSingleReferencedFileCount(OsuGameBase osu, int expected)
private static void checkSingleReferencedFileCount(OsuGameBase osu, int expected)
{
Assert.AreEqual(expected, osu.Dependencies.Get<FileStore>().QueryFiles(f => f.ReferenceCount == 1).Count());
}

View File

@ -43,7 +43,7 @@ namespace osu.Game.Tests.Scores.IO
OnlineScoreID = 12345,
};
var imported = await loadScoreIntoOsu(osu, toImport);
var imported = await LoadScoreIntoOsu(osu, toImport);
Assert.AreEqual(toImport.Rank, imported.Rank);
Assert.AreEqual(toImport.TotalScore, imported.TotalScore);
@ -75,7 +75,7 @@ namespace osu.Game.Tests.Scores.IO
Mods = new Mod[] { new OsuModHardRock(), new OsuModDoubleTime() },
};
var imported = await loadScoreIntoOsu(osu, toImport);
var imported = await LoadScoreIntoOsu(osu, toImport);
Assert.IsTrue(imported.Mods.Any(m => m is OsuModHardRock));
Assert.IsTrue(imported.Mods.Any(m => m is OsuModDoubleTime));
@ -105,7 +105,7 @@ namespace osu.Game.Tests.Scores.IO
}
};
var imported = await loadScoreIntoOsu(osu, toImport);
var imported = await LoadScoreIntoOsu(osu, toImport);
Assert.AreEqual(toImport.Statistics[HitResult.Perfect], imported.Statistics[HitResult.Perfect]);
Assert.AreEqual(toImport.Statistics[HitResult.Miss], imported.Statistics[HitResult.Miss]);
@ -136,7 +136,7 @@ namespace osu.Game.Tests.Scores.IO
}
};
var imported = await loadScoreIntoOsu(osu, toImport);
var imported = await LoadScoreIntoOsu(osu, toImport);
var beatmapManager = osu.Dependencies.Get<BeatmapManager>();
var scoreManager = osu.Dependencies.Get<ScoreManager>();
@ -144,7 +144,7 @@ namespace osu.Game.Tests.Scores.IO
beatmapManager.Delete(beatmapManager.QueryBeatmapSet(s => s.Beatmaps.Any(b => b.ID == imported.Beatmap.ID)));
Assert.That(scoreManager.Query(s => s.ID == imported.ID).DeletePending, Is.EqualTo(true));
var secondImport = await loadScoreIntoOsu(osu, imported);
var secondImport = await LoadScoreIntoOsu(osu, imported);
Assert.That(secondImport, Is.Null);
}
finally
@ -163,7 +163,7 @@ namespace osu.Game.Tests.Scores.IO
{
var osu = LoadOsuIntoHost(host, true);
await loadScoreIntoOsu(osu, new ScoreInfo { OnlineScoreID = 2 }, new TestArchiveReader());
await LoadScoreIntoOsu(osu, new ScoreInfo { OnlineScoreID = 2 }, new TestArchiveReader());
var scoreManager = osu.Dependencies.Get<ScoreManager>();
@ -177,7 +177,7 @@ namespace osu.Game.Tests.Scores.IO
}
}
private async Task<ScoreInfo> loadScoreIntoOsu(OsuGameBase osu, ScoreInfo score, ArchiveReader archive = null)
public static async Task<ScoreInfo> LoadScoreIntoOsu(OsuGameBase osu, ScoreInfo score, ArchiveReader archive = null)
{
var beatmapManager = osu.Dependencies.Get<BeatmapManager>();
@ -190,7 +190,7 @@ namespace osu.Game.Tests.Scores.IO
return scoreManager.GetAllUsableScores().FirstOrDefault();
}
private class TestArchiveReader : ArchiveReader
internal class TestArchiveReader : ArchiveReader
{
public TestArchiveReader()
: base("test_archive")

View File

@ -40,8 +40,6 @@ namespace osu.Game.Tests.Visual.Playlists
Dependencies.Cache(rulesets = new RulesetStore(ContextFactory));
Dependencies.Cache(manager = new BeatmapManager(LocalStorage, ContextFactory, rulesets, null, audio, Resources, host, Beatmap.Default));
manager.Import(new TestBeatmap(new OsuRuleset().RulesetInfo).BeatmapInfo.BeatmapSet).Wait();
((DummyAPIAccess)API).HandleRequest = req =>
{
switch (req)
@ -58,6 +56,7 @@ namespace osu.Game.Tests.Visual.Playlists
[SetUpSteps]
public void SetupSteps()
{
AddStep("ensure has beatmap", () => manager.Import(new TestBeatmap(new OsuRuleset().RulesetInfo).BeatmapInfo.BeatmapSet).Wait());
AddStep("load match", () => LoadScreen(match = new TestPlaylistsRoomSubScreen(Room)));
AddUntilStep("wait for load", () => match.IsCurrentScreen());
}
@ -111,10 +110,27 @@ namespace osu.Game.Tests.Visual.Playlists
public void TestBeatmapUpdatedOnReImport()
{
BeatmapSetInfo importedSet = null;
TestBeatmap beatmap = null;
// this step is required to make sure the further imports actually get online IDs.
// all the playlist logic relies on online ID matching.
AddStep("remove all matching online IDs", () =>
{
beatmap = new TestBeatmap(new OsuRuleset().RulesetInfo);
var existing = manager.QueryBeatmapSets(s => s.OnlineBeatmapSetID == beatmap.BeatmapInfo.BeatmapSet.OnlineBeatmapSetID).ToList();
foreach (var s in existing)
{
s.OnlineBeatmapSetID = null;
foreach (var b in s.Beatmaps)
b.OnlineBeatmapID = null;
manager.Update(s);
}
});
AddStep("import altered beatmap", () =>
{
var beatmap = new TestBeatmap(new OsuRuleset().RulesetInfo);
beatmap.BeatmapInfo.BaseDifficulty.CircleSize = 1;
importedSet = manager.Import(beatmap.BeatmapInfo.BeatmapSet).Result;

View File

@ -181,8 +181,13 @@ namespace osu.Game.Beatmaps
if (existingOnlineId != null)
{
Delete(existingOnlineId);
beatmaps.PurgeDeletable(s => s.ID == existingOnlineId.ID);
LogForModel(beatmapSet, $"Found existing beatmap set with same OnlineBeatmapSetID ({beatmapSet.OnlineBeatmapSetID}). It has been purged.");
// in order to avoid a unique key constraint, immediately remove the online ID from the previous set.
existingOnlineId.OnlineBeatmapSetID = null;
foreach (var b in existingOnlineId.Beatmaps)
b.OnlineBeatmapID = null;
LogForModel(beatmapSet, $"Found existing beatmap set with same OnlineBeatmapSetID ({beatmapSet.OnlineBeatmapSetID}). It has been deleted.");
}
}
}