-
-
Notifications
You must be signed in to change notification settings - Fork 38.2k
Added working support for private storage #16903
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 9 commits
fe5ff38
e5eb061
7df69ed
e5cb184
86303e3
11d03d0
10646fa
83a1913
070ac97
52dfcc1
f80a92e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -187,7 +187,9 @@ def _write_data(self, path: str, data: Dict): | |
| os.makedirs(os.path.dirname(path)) | ||
|
|
||
| _LOGGER.debug('Writing data for %s', self.key) | ||
| json.save_json(path, data, self._private) | ||
| tmp_path = path + "__TEMP__" | ||
| json.save_json(tmp_path, data, self._private) | ||
| os.replace(tmp_path, path) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that we should move this functionality to I also think that we should consider cleaning up the temp path in case of an exception replacing the other file
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair points. I'll make it so. |
||
|
|
||
| async def _async_migrate_func(self, old_version, old_data): | ||
| """Migrate to the new version.""" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| """Test Home Assistant json utility functions.""" | ||
| import os | ||
| import unittest | ||
| import sys | ||
| from tempfile import mkdtemp | ||
|
|
||
| from homeassistant.util.json import (SerializationError, | ||
| load_json, save_json) | ||
| from homeassistant.exceptions import HomeAssistantError | ||
|
|
||
| # Test data that can be saved as JSON | ||
| TEST_JSON_A = {"a": 1, "B": "two"} | ||
| TEST_JSON_B = {"a": "one", "B": 2} | ||
| # Test data that can not be saved as JSON (keys must be strings) | ||
| TEST_BAD_OBJECT = {("A",): 1} | ||
| # Test data that can not be loaded as JSON | ||
| TEST_BAD_SERIALIED = "THIS IS NOT JSON\n" | ||
|
|
||
|
|
||
| class TestJSON(unittest.TestCase): | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. expected 2 blank lines, found 1 |
||
| """Test util.json save and load.""" | ||
|
|
||
| def setUp(self): | ||
| """Set up for tests.""" | ||
| self.tmp_dir = mkdtemp() | ||
|
|
||
| def tearDown(self): | ||
| """Clean up after tests.""" | ||
| for fname in os.listdir(self.tmp_dir): | ||
| os.remove(os.path.join(self.tmp_dir, fname)) | ||
| os.rmdir(self.tmp_dir) | ||
|
|
||
| def _path_for(self, leaf_name): | ||
| return os.path.join(self.tmp_dir, leaf_name+".json") | ||
|
|
||
| def test_save_and_load(self): | ||
| """Test saving and loading back.""" | ||
| fname = self._path_for("test1") | ||
| save_json(fname, TEST_JSON_A) | ||
| data = load_json(fname) | ||
| self.assertEqual(data, TEST_JSON_A) | ||
|
|
||
| # Skipped on Windows | ||
| @unittest.skipIf(sys.platform.startswith('win'), | ||
| "private permissions not supported on Windows") | ||
| def test_save_and_load_private(self): | ||
| """Test we can load private files and that they are protected.""" | ||
| fname = self._path_for("test2") | ||
| save_json(fname, TEST_JSON_A, private=True) | ||
| data = load_json(fname) | ||
| self.assertEqual(data, TEST_JSON_A) | ||
| stats = os.stat(fname) | ||
| self.assertEqual(stats.st_mode & 0o77, 0) | ||
|
|
||
| def test_overwrite_and_reload(self): | ||
| """Test that we can overwrite an existing file and read back.""" | ||
| fname = self._path_for("test3") | ||
| save_json(fname, TEST_JSON_A) | ||
| save_json(fname, TEST_JSON_B) | ||
| data = load_json(fname) | ||
| self.assertEqual(data, TEST_JSON_B) | ||
|
|
||
| def test_save_bad_data(self): | ||
| """Test error from trying to save unserialisable data.""" | ||
| fname = self._path_for("test4") | ||
| with self.assertRaises(SerializationError): | ||
| save_json(fname, TEST_BAD_OBJECT) | ||
|
|
||
| def test_load_bad_data(self): | ||
| """Test error from trying to load unserialisable data.""" | ||
| fname = self._path_for("test5") | ||
| with open(fname, "w") as fh: | ||
| fh.write(TEST_BAD_SERIALIED) | ||
| with self.assertRaises(HomeAssistantError): | ||
| load_json(fname) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mark file as not visible with
., and we should use a context manager to cleanup the temporary file after this.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to clean up since we're using os.replace