PR "feedback": Adding a key to the dumps metadata JSON for clarity and expandability (#1136).

borgmatic/hooks/data_source/dump.py

@@ -49,7 +49,13 @@ def write_data_source_dumps_metadata(borgmatic_runtime_directory, hook_name, dum
 
     try:
         with open(dumps_metadata_path, 'w', encoding='utf-8') as metadata_file:
-            json.dump([dump._asdict() for dump in dumps_metadata], metadata_file, sort_keys=True)
+            json.dump(
+                {
+                    'dumps': [dump._asdict() for dump in dumps_metadata],
+                },
+                metadata_file,
+                sort_keys=True,
+            )
     except OSError as error:
         raise ValueError(f'Error writing to dumps metadata at {dumps_metadata_path}: {error}')
 
@@ -63,8 +69,10 @@ def parse_data_source_dumps_metadata(dumps_json, dumps_metadata_path):
     the expected keys.
     '''
     try:
-        return tuple(borgmatic.actions.restore.Dump(**dump) for dump in json.loads(dumps_json))
-    except (json.JSONDecodeError, TypeError) as error:
+        return tuple(
+            borgmatic.actions.restore.Dump(**dump) for dump in json.loads(dumps_json)['dumps']
+        )
+    except (json.JSONDecodeError, TypeError, KeyError) as error:
         raise ValueError(
             f'Cannot read archive data source dumps metadata at {dumps_metadata_path} due to invalid JSON: {error}',
         )

tests/unit/hooks/data_source/test_dump.py

@@ -51,7 +51,7 @@ def test_write_data_source_dumps_metadata_writes_json_to_file():
 
     assert (
         dumps_stream.getvalue()
-        == '[{"data_source_name": "foo", "hook_name": "databases", "hostname": "localhost", "port": null}, {"data_source_name": "bar", "hook_name": "databases", "hostname": "localhost", "port": null}]'
+        == '{"dumps": [{"data_source_name": "foo", "hook_name": "databases", "hostname": "localhost", "port": null}, {"data_source_name": "bar", "hook_name": "databases", "hostname": "localhost", "port": null}]}'
     )
 
 
@@ -72,7 +72,7 @@ def test_write_data_source_dumps_metadata_with_operating_system_error_raises():
 
 
 def test_parse_data_source_dumps_metadata_converts_json_to_dump_instances():
-    dumps_json = '[{"data_source_name": "foo", "hook_name": "databases", "hostname": "localhost", "port": null}, {"data_source_name": "bar", "hook_name": "databases", "hostname": "example.org", "port": 1234}]'
+    dumps_json = '{"dumps": [{"data_source_name": "foo", "hook_name": "databases", "hostname": "localhost", "port": null}, {"data_source_name": "bar", "hook_name": "databases", "hostname": "example.org", "port": 1234}]}'
 
     assert module.parse_data_source_dumps_metadata(
         dumps_json, 'borgmatic/databases/dumps.json'
@@ -88,7 +88,16 @@ def test_parse_data_source_dumps_metadata_with_invalid_json_raises():
 
 
 def test_parse_data_source_dumps_metadata_with_unknown_keys_raises():
-    dumps_json = '[{"data_source_name": "foo", "hook_name": "databases", "wtf": "is this"}]'
+    dumps_json = (
+        '{"dumps": [{"data_source_name": "foo", "hook_name": "databases", "wtf": "is this"}]}'
+    )
+
+    with pytest.raises(ValueError):
+        module.parse_data_source_dumps_metadata(dumps_json, 'borgmatic/databases/dumps.json')
+
+
+def test_parse_data_source_dumps_metadata_with_missing_dumps_key_raises():
+    dumps_json = '{"not": "what we are looking for"}'
 
     with pytest.raises(ValueError):
         module.parse_data_source_dumps_metadata(dumps_json, 'borgmatic/databases/dumps.json')