From 3e6edd73ee28b5e52ee8fce1a34909fc0614752b Mon Sep 17 00:00:00 2001 From: SkyratBot <59378654+SkyratBot@users.noreply.github.com> Date: Thu, 28 Dec 2023 17:08:13 +0100 Subject: [PATCH] [MIRROR] Add system for safely manipulating JSON databases and apply it to photo albums and photo frames [MDB IGNORE] (#25887) * Add system for safely manipulating JSON databases and apply it to photo albums and photo frames (#80519) We frequently have issues with data loss in our long storage .json files for various reasons, such as the file being completely blanked out on write etc. This introduces a system that tries to safely handle that by saving the known working json file into a backup that will be loaded in the case a write fails. This system queues updates in order to send through to the next tick. This is an improvement over the existing implementation of photo albums and photo frames (I think all persistence, even) which do not save until the end of a properly rebooted round, but not during a server crash. Also saves the jsons in pretty prints, which make them easier to read but especially make them easier to diff in a git repository, which MSO wants to setup (and hopefully make public so I can make a dashboard on bus.moth.fans for looking at photo albums and their history, which is something I've wanted to do for a very long time). ## Changelog :cl: refactor: Photo albums and photo frames are now more resilient to data loss, especially when a server crashes. /:cl: * Add system for safely manipulating JSON databases and apply it to photo albums and photo frames --------- Co-authored-by: Mothblocks <35135081+Mothblocks@users.noreply.github.com> --- .../subsystem/persistence/_persistence.dm | 20 ++- .../subsystem/persistence/photo_albums.dm | 79 ++--------- code/datums/json_database.dm | 128 ++++++++++++++++++ code/modules/photography/photos/album.dm | 36 +++-- code/modules/photography/photos/frame.dm | 38 +++--- tgstation.dme | 1 + 6 files changed, 207 insertions(+), 95 deletions(-) create mode 100644 code/datums/json_database.dm diff --git a/code/controllers/subsystem/persistence/_persistence.dm b/code/controllers/subsystem/persistence/_persistence.dm index 565cf519d4b..e42247c71a9 100644 --- a/code/controllers/subsystem/persistence/_persistence.dm +++ b/code/controllers/subsystem/persistence/_persistence.dm @@ -20,8 +20,23 @@ SUBSYSTEM_DEF(persistence) var/list/blocked_maps = list() var/list/saved_trophies = list() var/list/picture_logging_information = list() - var/list/obj/structure/sign/picture_frame/photo_frames - var/list/obj/item/storage/photo_album/photo_albums + + /// A json_database linking to data/photo_frames.json. + /// Schema is persistence_id => array of photo names. + var/datum/json_database/photo_frames_database + + /// A lazy list of every picture frame that is going to be loaded with persistent photos. + /// Will be null'd once the persistence system initializes, and never read from again. + var/list/obj/structure/sign/picture_frame/queued_photo_frames + + /// A json_database linking to data/photo_albums.json. + /// Schema is persistence_id => array of photo names. + var/datum/json_database/photo_albums_database + + /// A lazy list of every photo album that is going to be loaded with persistent photos. + /// Will be null'd once the persistence system initializes, and never read from again. + var/list/obj/item/storage/photo_album/queued_photo_albums + var/rounds_since_engine_exploded = 0 var/delam_highscore = 0 var/tram_hits_this_round = 0 @@ -48,7 +63,6 @@ SUBSYSTEM_DEF(persistence) save_prisoner_tattoos() collect_trophies() collect_maps() - save_photo_persistence() //THIS IS PERSISTENCE, NOT THE LOGGING PORTION. save_randomized_recipes() save_scars() save_custom_outfits() diff --git a/code/controllers/subsystem/persistence/photo_albums.dm b/code/controllers/subsystem/persistence/photo_albums.dm index 3aee856b2c2..90c8ec31ac1 100644 --- a/code/controllers/subsystem/persistence/photo_albums.dm +++ b/code/controllers/subsystem/persistence/photo_albums.dm @@ -1,15 +1,3 @@ -///Loads up the photo album source file. -/datum/controller/subsystem/persistence/proc/get_photo_albums() - var/album_path = file("data/photo_albums.json") - if(fexists(album_path)) - return json_decode(file2text(album_path)) - -///Loads up the photo frames source file. -/datum/controller/subsystem/persistence/proc/get_photo_frames() - var/frame_path = file("data/photo_frames.json") - if(fexists(frame_path)) - return json_decode(file2text(frame_path)) - /// Removes the identifier of a persistent photo frame from the json. /datum/controller/subsystem/persistence/proc/remove_photo_frames(identifier) var/frame_path = file("data/photo_frames.json") @@ -25,62 +13,23 @@ ///Loads photo albums, and populates them; also loads and applies frames to picture frames. /datum/controller/subsystem/persistence/proc/load_photo_persistence() - var/album_path = file("data/photo_albums.json") - var/frame_path = file("data/photo_frames.json") - if(fexists(album_path)) - var/list/json = json_decode(file2text(album_path)) - if(json.len) - for(var/i in photo_albums) - var/obj/item/storage/photo_album/A = i - if(!A.persistence_id) - continue - if(json[A.persistence_id]) - A.populate_from_id_list(json[A.persistence_id]) - - if(fexists(frame_path)) - var/list/json = json_decode(file2text(frame_path)) - if(json.len) - for(var/i in photo_frames) - var/obj/structure/sign/picture_frame/PF = i - if(!PF.persistence_id) - continue - if(json[PF.persistence_id]) - PF.load_from_id(json[PF.persistence_id]) - -///Saves the contents of photo albums and the picture frames. -/datum/controller/subsystem/persistence/proc/save_photo_persistence() - var/album_path = file("data/photo_albums.json") - var/frame_path = file("data/photo_frames.json") - - var/list/frame_json = list() - var/list/album_json = list() - - if(fexists(album_path)) - album_json = json_decode(file2text(album_path)) - fdel(album_path) - - for(var/i in photo_albums) - var/obj/item/storage/photo_album/A = i - if(!istype(A) || !A.persistence_id) + photo_albums_database = new("data/photo_albums.json") + for (var/obj/item/storage/photo_album/album as anything in queued_photo_albums) + if (isnull(album.persistence_id)) continue - var/list/L = A.get_picture_id_list() - album_json[A.persistence_id] = L - album_json = json_encode(album_json) + var/album_data = photo_albums_database.get_key(album.persistence_id) + if (!isnull(album_data)) + album.populate_from_id_list(album_data) - WRITE_FILE(album_path, album_json) - - if(fexists(frame_path)) - frame_json = json_decode(file2text(frame_path)) - fdel(frame_path) - - for(var/i in photo_frames) - var/obj/structure/sign/picture_frame/F = i - if(!istype(F) || !F.persistence_id) + photo_frames_database = new("data/photo_frames.json") + for (var/obj/structure/sign/picture_frame/frame as anything in queued_photo_frames) + if (isnull(frame.persistence_id)) continue - frame_json[F.persistence_id] = F.get_photo_id() - frame_json = json_encode(frame_json) - - WRITE_FILE(frame_path, frame_json) + var/frame_data = photo_frames_database.get_key(frame.persistence_id) + if (!isnull(frame_data)) + frame.load_from_id(frame_data) + queued_photo_albums = null + queued_photo_frames = null diff --git a/code/datums/json_database.dm b/code/datums/json_database.dm new file mode 100644 index 00000000000..ea3ff354b48 --- /dev/null +++ b/code/datums/json_database.dm @@ -0,0 +1,128 @@ +/// Represents a json file being used as a database in the data/ folder. +/// Changes made here will save back to the associated file, with recovery. +/// Will defer writes until later if multiple happen in the same tick. +/// Do not add an extra cache on top of this. This IS your cache. +/datum/json_database + VAR_PRIVATE + filepath + backup_filepath + + cached_data + save_queued = FALSE + + static/existing_json_database = list() + +/datum/json_database/New(filepath) + if (IsAdminAdvancedProcCall()) + to_chat(usr, "json_database creation, linking to [html_encode(filepath)], was blocked.", confidential = TRUE) + return + + ASSERT(isnull(existing_json_database[filepath]), "[filepath] already has an associated json_database. You must expose it somehow and use that instead of making a new one.") + + existing_json_database[filepath] = TRUE + + src.filepath = filepath + backup_filepath = "[filepath].savebac" + + if (fexists(filepath)) + cached_data = safe_json_decode(file2text(filepath)) + if (isnull(cached_data)) + var/scenario = "[filepath] existed, but did not have valid JSON" + + if (fexists(backup_filepath)) + load_backup(scenario) + else + stack_trace("[scenario]. No backup could be found.") + cached_data = list() + else + if (fexists(backup_filepath)) + load_backup("[filepath] didn't exist") + else + cached_data = list() + +/datum/json_database/Destroy() + if (save_queued) + save() + + existing_json_database -= filepath + + return ..() + +/// Returns the cached data. +/// Be careful on holding onto this data for too long, as it can mutate when other stuff changes it. +/// Do not mutate it yourself. +/datum/json_database/proc/get() + return cached_data + +/// Returns the data with the given key. +/// For arrays, this is a number. +/// Be careful on holding onto this data for too long, as it can mutate when other stuff changes it. +/// Do not mutate it yourself. +/datum/json_database/proc/get_key(key) + return cached_data[key] + +/// Sets the data at the key to the value, and queues a save. +/datum/json_database/proc/set_key(key, value) + cached_data[key] = value + queue_save() + +/// Removes the data at the given item, and queues a save. +/// For dictionaries, this can be the key. +/// For arrays, this can be the value. +/datum/json_database/proc/remove(item) + UNTYPED_LIST_REMOVE(cached_data, item) + queue_save() + +/// Inserts the data at the end of what is assumed to be an array, and queues a save. +/datum/json_database/proc/insert(value) + UNTYPED_LIST_ADD(cached_data, value) + queue_save() + +/// Replaces the cache with the new data completely, and queues a save. +/// Do not touch the new data after passing it in. +/datum/json_database/proc/replace(list/new_data) + cached_data = new_data + queue_save() + +/datum/json_database/proc/queue_save() + PRIVATE_PROC(TRUE) + + if (save_queued) + return + + addtimer(CALLBACK(src, PROC_REF(save)), 0) + +/datum/json_database/proc/save() + PRIVATE_PROC(TRUE) + + save_queued = FALSE + + if (fexists(filepath)) + rustg_file_write(file2text(filepath), backup_filepath) + + rustg_file_write(json_encode(cached_data, JSON_PRETTY_PRINT), filepath) + + ASSERT(!isnull(safe_json_decode(file2text(filepath))), "JSON written to [filepath] was not valid. Backup will be preserved.") + + fdel(backup_filepath) + +/datum/json_database/proc/load_backup(scenario) + PRIVATE_PROC(TRUE) + + var/cached_contents = file2text(backup_filepath) + var/list/backed_up_data = safe_json_decode(cached_contents) + + if (isnull(backed_up_data)) + stack_trace("[scenario]. Backup existed, but also did not have valid JSON.") + cached_data = list() + else + stack_trace("[scenario]. Backup existed and was used instead. The JSON file has been updated.") + cached_data = backed_up_data + rustg_file_write(cached_contents, filepath) + +/datum/json_database/vv_edit_var(var_name, var_value) + switch (var_name) + if (nameof(filepath), nameof(backup_filepath)) + return FALSE + else + return ..() diff --git a/code/modules/photography/photos/album.dm b/code/modules/photography/photos/album.dm index 35d7f27017c..f8f8a8fb062 100644 --- a/code/modules/photography/photos/album.dm +++ b/code/modules/photography/photos/album.dm @@ -9,6 +9,7 @@ inhand_icon_state = "album" lefthand_file = 'icons/mob/inhands/items/books_lefthand.dmi' righthand_file = 'icons/mob/inhands/items/books_righthand.dmi' + storage_type = /datum/storage/photo_album resistance_flags = FLAMMABLE w_class = WEIGHT_CLASS_SMALL flags_1 = PREVENT_CONTENTS_EXPLOSION_1 @@ -16,13 +17,11 @@ /obj/item/storage/photo_album/Initialize(mapload) . = ..() - atom_storage.set_holdable(list(/obj/item/photo)) - atom_storage.max_total_storage = 42 - atom_storage.max_slots = 21 - LAZYADD(SSpersistence.photo_albums, src) + if (!SSpersistence.initialized) + LAZYADD(SSpersistence.queued_photo_albums, src) /obj/item/storage/photo_album/Destroy() - LAZYREMOVE(SSpersistence.photo_albums, src) + LAZYREMOVE(SSpersistence.queued_photo_albums, src) return ..() /obj/item/storage/photo_album/proc/get_picture_id_list() @@ -41,9 +40,9 @@ //Manual loading, DO NOT USE FOR HARDCODED/MAPPED IN ALBUMS. This is for if an album needs to be loaded mid-round from an ID. /obj/item/storage/photo_album/proc/persistence_load() - var/list/data = SSpersistence.get_photo_albums() - if(data[persistence_id]) - populate_from_id_list(data[persistence_id]) + var/list/data = SSpersistence.photo_albums_database.get_key(persistence_id) + if (!isnull(data)) + populate_from_id_list(data) /obj/item/storage/photo_album/proc/populate_from_id_list(list/ids) var/list/current_ids = get_picture_id_list() @@ -55,6 +54,27 @@ if(!atom_storage?.attempt_insert(P, override = TRUE)) qdel(P) +/datum/storage/photo_album + max_total_storage = 42 + max_slots = 21 + +/datum/storage/photo_album/New(atom/parent, max_slots, max_specific_storage, max_total_storage, numerical_stacking, allow_quick_gather, allow_quick_empty, collection_mode, attack_hand_interact) + . = ..() + set_holdable(list(/obj/item/photo)) + +/datum/storage/photo_album/proc/save_everything() + var/obj/item/storage/photo_album/album = parent.resolve() + ASSERT(istype(album)) + SSpersistence.photo_albums_database.set_key(album.persistence_id, album.get_picture_id_list()) + +/datum/storage/photo_album/handle_enter(datum/source, obj/item/arrived) + . = ..() + save_everything() + +/datum/storage/photo_album/handle_exit(datum/source, obj/item/gone) + . = ..() + save_everything() + /obj/item/storage/photo_album/hos name = "photo album (Head of Security)" icon_state = "album_blue" diff --git a/code/modules/photography/photos/frame.dm b/code/modules/photography/photos/frame.dm index f2d77553ea1..4fbe3e034d8 100644 --- a/code/modules/photography/photos/frame.dm +++ b/code/modules/photography/photos/frame.dm @@ -56,7 +56,7 @@ var/obj/structure/sign/picture_frame/PF = O PF.copy_overlays(src) if(displayed) - PF.framed = displayed + PF.set_and_save_framed(displayed) if(contents.len) var/obj/item/I = pick(contents) I.forceMove(PF) @@ -70,27 +70,19 @@ resistance_flags = FLAMMABLE var/obj/item/photo/framed var/persistence_id - var/del_id_on_destroy = FALSE var/art_value = OK_ART var/can_decon = TRUE -#define FRAME_DEFINE(id) /obj/structure/sign/picture_frame/##id/persistence_id = #id - -//Put default persistent frame defines here! - -#undef FRAME_DEFINE - /obj/structure/sign/picture_frame/Initialize(mapload, dir, building) . = ..() AddElement(/datum/element/art, art_value) - LAZYADD(SSpersistence.photo_frames, src) + if (!SSpersistence.initialized) + LAZYADD(SSpersistence.queued_photo_frames, src) if(dir) setDir(dir) /obj/structure/sign/picture_frame/Destroy() - LAZYREMOVE(SSpersistence.photo_frames, src) - if(persistence_id && del_id_on_destroy) - SSpersistence.remove_photo_frames(persistence_id) + LAZYREMOVE(SSpersistence.queued_photo_frames, src) return ..() /obj/structure/sign/picture_frame/proc/get_photo_id() @@ -99,9 +91,9 @@ //Manual loading, DO NOT USE FOR HARDCODED/MAPPED IN ALBUMS. This is for if an album needs to be loaded mid-round from an ID. /obj/structure/sign/picture_frame/proc/persistence_load() - var/list/data = SSpersistence.get_photo_frames() - if(data[persistence_id]) - load_from_id(data[persistence_id]) + var/list/data = SSpersistence.photo_frames_database.get_key(persistence_id) + if(!isnull(data)) + load_from_id(data) /obj/structure/sign/picture_frame/proc/load_from_id(id) var/obj/item/photo/old/P = load_photo_from_disk(id) @@ -113,6 +105,15 @@ framed = P update_appearance() +/// Given a photo (or null), will change the contained picture, and queue a persistent save. +/obj/structure/sign/picture_frame/proc/set_and_save_framed(obj/item/photo/photo) + framed = photo + + if (isnull(persistence_id)) + return + + SSpersistence.photo_frames_database.set_key(persistence_id, photo?.picture?.id) + /obj/structure/sign/picture_frame/examine(mob/user) . = ..() if(in_range(src, user)) @@ -141,7 +142,7 @@ tool.play_tool_sound(src) framed.forceMove(drop_location()) user.visible_message(span_warning("[user] cuts away [framed] from [src]!")) - framed = null + set_and_save_framed(null) update_appearance() return ITEM_INTERACT_SUCCESS @@ -155,7 +156,7 @@ var/obj/item/photo/P = I if(!user.transferItemToLoc(P, src)) return - framed = P + set_and_save_framed(P) update_appearance() return TRUE ..() @@ -177,7 +178,7 @@ var/obj/item/wallframe/picture/F = new /obj/item/wallframe/picture(loc) if(framed) F.displayed = framed - framed = null + set_and_save_framed(null) if(contents.len) var/obj/item/I = pick(contents) I.forceMove(F) @@ -277,7 +278,6 @@ /obj/structure/sign/picture_frame/portrait/bar persistence_id = "frame_bar" - del_id_on_destroy = TRUE ///Generates a persistence id unique to the current map. Every bar should feel a little bit different after all. /obj/structure/sign/picture_frame/portrait/bar/Initialize(mapload) diff --git a/tgstation.dme b/tgstation.dme index 07414c62a5f..a694eecd449 100644 --- a/tgstation.dme +++ b/tgstation.dme @@ -856,6 +856,7 @@ #include "code\datums\hotkeys_help.dm" #include "code\datums\http.dm" #include "code\datums\hud.dm" +#include "code\datums\json_database.dm" #include "code\datums\json_savefile.dm" #include "code\datums\lazy_template.dm" #include "code\datums\map_config.dm"