Conversation
| } | ||
|
|
||
| if (File.Exists(filePath)) { | ||
| File.Replace(tempPath, filePath, null); |
There was a problem hiding this comment.
File.Replace can fail if the file is opened somewhere. A more conservative approach would be to move the existing file, if that fails choose another name and try again, move the new file to the intended location, then try to delete the existing file, ignore failures. Failed deletes can be retried on subsequent calls to this function.
If you choose to implement this then please consider writing a separate method for MoveOrReplaceFile.
There was a problem hiding this comment.
Hmm this is the same pattern we use currently and haven't had any reports of issues.
In the current implementation, failing to write leaves the last save there and the save services continues to consider the save as dirty (meaning it will try to save again in about 4 seconds. With this implementation, the most you'll lose is the delta between the actually saved version and the .new version (which can be manually restored).
Your approach has a few extra moving parts that I worry could result in losing a settings file because there isn't a settings file in the right spot at all times.
There was a problem hiding this comment.
Sounds reasonable to me, since it's a one time migration.
There was a problem hiding this comment.
This code is used during normal operations, on every save of any settings file.
I have no strong opinion on this either way.
entrhopi
left a comment
entrhopi
left a comment
There was a problem hiding this comment.
I've looked into the file saving mechanic again, and I think it is sufficiently safe. In case of it not working we still have the .new file for recovery.
3 participants
Problem
Module settings are stored along with all other settings within the
setting.jsonfile. To avoid deserializing module-specific types when a module isn't loaded, the settings are kept as raw JSON strings (viaSettingCollection's lazy loading). However, this means the full JSON blob for every module (including those that are disabled) is held in memory at all times. For users with many modules, this results in a largesettings.jsonand unnecessary memory usage.Solution
Module settings are migrated out of
settings.jsoninto individual files under a newsettings/directory, named by module namespace (e.g.settings/bh.community.pathing.json). Settings are only loaded into memory when a module is enabled, and flushed back to disk and freed when it's disabled.Settings migration
settings.jsonfile. When the migration occurs, a backup of the original settings file is made (if one doesn't already exist), just in case.Results