diff --git a/settings.py b/settings.py index 7ab618c3..79277052 100644 --- a/settings.py +++ b/settings.py @@ -3,6 +3,7 @@ Application settings / host.yaml interface using type hints. This is different from player options. """ +import os import os.path import shutil import sys @@ -11,7 +12,6 @@ import warnings from enum import IntEnum from threading import Lock from typing import cast, Any, BinaryIO, ClassVar, Dict, Iterator, List, Optional, TextIO, Tuple, Union, TypeVar -import os __all__ = [ "get_settings", "fmt_doc", "no_gui", @@ -798,6 +798,7 @@ class Settings(Group): atexit.register(autosave) def save(self, location: Optional[str] = None) -> None: # as above + from Utils import parse_yaml location = location or self._filename assert location, "No file specified" temp_location = location + ".tmp" # not using tempfile to test expected file access @@ -807,10 +808,18 @@ class Settings(Group): # can't use utf-8-sig because it breaks backward compat: pyyaml on Windows with bytes does not strip the BOM with open(temp_location, "w", encoding="utf-8") as f: self.dump(f) - # replace old with new - if os.path.exists(location): + f.flush() + if hasattr(os, "fsync"): + os.fsync(f.fileno()) + # validate new file is valid yaml + with open(temp_location, encoding="utf-8") as f: + parse_yaml(f.read()) + # replace old with new, try atomic operation first + try: + os.rename(temp_location, location) + except (OSError, FileExistsError): os.unlink(location) - os.rename(temp_location, location) + os.rename(temp_location, location) self._filename = location def dump(self, f: TextIO, level: int = 0) -> None: @@ -832,7 +841,6 @@ def get_settings() -> Settings: with _lock: # make sure we only have one instance res = getattr(get_settings, "_cache", None) if not res: - import os from Utils import user_path, local_path filenames = ("options.yaml", "host.yaml") locations: List[str] = [] diff --git a/test/general/test_host_yaml.py b/test/general/test_host_yaml.py index 7174befc..3edbd34a 100644 --- a/test/general/test_host_yaml.py +++ b/test/general/test_host_yaml.py @@ -1,11 +1,12 @@ import os +import os.path import unittest from io import StringIO -from tempfile import TemporaryFile +from tempfile import TemporaryDirectory, TemporaryFile from typing import Any, Dict, List, cast import Utils -from settings import Settings, Group +from settings import Group, Settings, ServerOptions class TestIDs(unittest.TestCase): @@ -80,3 +81,27 @@ class TestSettingsDumper(unittest.TestCase): self.assertEqual(value_spaces[2], value_spaces[0]) # start of sub-list self.assertGreater(value_spaces[3], value_spaces[0], f"{value_lines[3]} should have more indentation than {value_lines[0]} in {lines}") + + +class TestSettingsSave(unittest.TestCase): + def test_save(self) -> None: + """Test that saving and updating works""" + with TemporaryDirectory() as d: + filename = os.path.join(d, "host.yaml") + new_release_mode = ServerOptions.ReleaseMode("enabled") + # create default host.yaml + settings = Settings(None) + settings.save(filename) + self.assertTrue(os.path.exists(filename), + "Default settings could not be saved") + self.assertNotEqual(settings.server_options.release_mode, new_release_mode, + "Unexpected default release mode") + # update host.yaml + settings.server_options.release_mode = new_release_mode + settings.save(filename) + self.assertFalse(os.path.exists(filename + ".tmp"), + "Temp file was not removed during save") + # read back host.yaml + settings = Settings(filename) + self.assertEqual(settings.server_options.release_mode, new_release_mode, + "Settings were not overwritten")