Prefer not deleted models when picking model instances for reuse when importing

This fell out while investigating why the issue with online IDs
mismatching in the `.osu` could be worked around by importing the map
three times in total when starting from it not being available locally.

Here follows an explanation of why that "helped".

Import 1:
- The beatmap set is imported normally.
- Online metadata population sees the online ID mismatch and resets it
  on the problematic beatmap.

Import 2:
- The existing beatmap set is found, but deemed not reusable
  because of the single beatmap having its ID reset to -1.
- The existing beatmap set is marked deleted, and all the IDs of
  its beatmaps are reset to -1.
- The beatmap set is reimported afresh.
- Online metadata population still sees the online ID mismatch
  and resets it on the problematic beatmap.

Note that at this point the first import *is still physically present
in the database* but marked deleted.

Import 3:
- When trying to find the existing beatmap set to see if it can be
  reused, *the one pending deletion and with its IDs reset -
  - the remnant from import 1 - is returned*.
- Because of this, `validateOnlineIds()` resets online IDs
  *on the model representing the current reimport*.
- The beatmap set is reimported yet again.
- With the online ID reset, the online metadata population check for
  online ID mismatch does not run because *the IDs were reset to -1*
  earlier.

Preferring undeleted models when picking the model instance for reuse
prevents this scenario.
This commit is contained in:
Bartłomiej Dach 2024-10-30 08:07:19 +01:00
parent 2c2f307a63
commit 0e52797f29
No known key found for this signature in database

View File

@ -528,7 +528,7 @@ namespace osu.Game.Database
/// <param name="model">The new model proposed for import.</param>
/// <param name="realm">The current realm context.</param>
/// <returns>An existing model which matches the criteria to skip importing, else null.</returns>
protected TModel? CheckForExisting(TModel model, Realm realm) => string.IsNullOrEmpty(model.Hash) ? null : realm.All<TModel>().FirstOrDefault(b => b.Hash == model.Hash);
protected TModel? CheckForExisting(TModel model, Realm realm) => string.IsNullOrEmpty(model.Hash) ? null : realm.All<TModel>().OrderBy(b => b.DeletePending).FirstOrDefault(b => b.Hash == model.Hash);
/// <summary>
/// Whether import can be skipped after finding an existing import early in the process.