[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
🆑
refactor: Photo albums and photo frames are now more resilient to data
loss, especially when a server crashes.
/🆑

* 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>

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()

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

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, "<span class='admin prefix'>json_database creation, linking to [html_encode(filepath)], was blocked.</span>", 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 ..()

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"

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)

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"