Skip to content

Commit a7882d9

Browse files
committed
wrap beatmap deletion in transactions
map and score deletions were separate operations with no transaction wrapping. if the process crashed between deleting maps and deleting their scores, you'd end up with orphaned score rows pointing at maps that no longer exist. both deletion paths (map update and full set removal) now use transactions.
1 parent 205d641 commit a7882d9

File tree

1 file changed

+28
-26
lines changed

1 file changed

+28
-26
lines changed

app/objects/beatmap.py

Lines changed: 28 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -728,17 +728,18 @@ async def _update_if_available(self) -> None:
728728
# save changes to sql
729729

730730
if map_md5s_to_delete:
731-
# delete maps
732-
await app.state.services.database.execute(
733-
"DELETE FROM maps WHERE md5 IN :map_md5s",
734-
{"map_md5s": map_md5s_to_delete},
735-
)
731+
async with app.state.services.database.transaction():
732+
# delete maps
733+
await app.state.services.database.execute(
734+
"DELETE FROM maps WHERE md5 IN :map_md5s",
735+
{"map_md5s": map_md5s_to_delete},
736+
)
736737

737-
# delete scores on the maps
738-
await app.state.services.database.execute(
739-
"DELETE FROM scores WHERE map_md5 IN :map_md5s",
740-
{"map_md5s": map_md5s_to_delete},
741-
)
738+
# delete scores on the maps
739+
await app.state.services.database.execute(
740+
"DELETE FROM scores WHERE map_md5 IN :map_md5s",
741+
{"map_md5s": map_md5s_to_delete},
742+
)
742743

743744
# update last_osuapi_check
744745
await app.state.services.database.execute(
@@ -761,27 +762,28 @@ async def _update_if_available(self) -> None:
761762
# TODO: a couple of open questions here:
762763
# - should we delete the beatmap from the database if it's not in the osu!api?
763764
# - are 404 and 200 the only cases where we should delete the beatmap?
764-
if self.maps:
765-
map_md5s_to_delete = {bmap.md5 for bmap in self.maps}
765+
async with app.state.services.database.transaction():
766+
if self.maps:
767+
map_md5s_to_delete = {bmap.md5 for bmap in self.maps}
768+
769+
# delete maps
770+
await app.state.services.database.execute(
771+
"DELETE FROM maps WHERE md5 IN :map_md5s",
772+
{"map_md5s": map_md5s_to_delete},
773+
)
766774

767-
# delete maps
768-
await app.state.services.database.execute(
769-
"DELETE FROM maps WHERE md5 IN :map_md5s",
770-
{"map_md5s": map_md5s_to_delete},
771-
)
775+
# delete scores on the maps
776+
await app.state.services.database.execute(
777+
"DELETE FROM scores WHERE map_md5 IN :map_md5s",
778+
{"map_md5s": map_md5s_to_delete},
779+
)
772780

773-
# delete scores on the maps
781+
# delete set
774782
await app.state.services.database.execute(
775-
"DELETE FROM scores WHERE map_md5 IN :map_md5s",
776-
{"map_md5s": map_md5s_to_delete},
783+
"DELETE FROM mapsets WHERE id = :set_id",
784+
{"set_id": self.id},
777785
)
778786

779-
# delete set
780-
await app.state.services.database.execute(
781-
"DELETE FROM mapsets WHERE id = :set_id",
782-
{"set_id": self.id},
783-
)
784-
785787
async def _save_to_sql(self) -> None:
786788
"""Save the object's attributes into the database."""
787789
await app.state.services.database.execute_many(

0 commit comments

Comments
 (0)