diff --git a/picard/script/serializer.py b/picard/script/serializer.py index ef6530a86..3edc81125 100644 --- a/picard/script/serializer.py +++ b/picard/script/serializer.py @@ -25,7 +25,6 @@ from enum import ( IntEnum, unique, ) -import json import uuid import yaml @@ -146,9 +145,9 @@ class PicardScript(): Args: **kwargs: Properties to update in the form `property=value`. """ - self._update_from_dict(kwargs) + self.update_from_dict(kwargs) - def _update_from_dict(self, settings): + def update_from_dict(self, settings): """Updates the values of the properties based on the contents of the dictionary provided. Properties in the `settings` dictionary that are not found in the script object are ignored. @@ -190,19 +189,6 @@ class PicardScript(): items = {key: getattr(self, key) for key in self.OUTPUT_FIELDS} return yaml.dump(items, sort_keys=False) - def to_json(self, indent=None): - """Converts the properties of the script object to a JSON formatted string. Note that only property - names listed in `OUTPUT_FIELDS` will be included in the output. - - Args: - indent (int): Amount to indent the output. Defaults to None. - - Returns: - str: The properties of the script object formatted as a JSON string. - """ - items = {key: getattr(self, key) for key in self.OUTPUT_FIELDS} - return json.dumps(items, indent=indent, sort_keys=True) - @classmethod def create_from_yaml(cls, yaml_string, create_new_id=True): """Creates an instance based on the contents of the YAML string provided. @@ -220,45 +206,11 @@ class PicardScript(): raise ScriptImportError(N_("File content not a dictionary")) if 'title' not in yaml_dict or 'script' not in yaml_dict: raise ScriptImportError(N_('Invalid script package')) - new_object._update_from_dict(yaml_dict) + new_object.update_from_dict(yaml_dict) if create_new_id or not new_object['id']: new_object._set_new_id() return new_object - @classmethod - def create_from_json(cls, json_string, create_new_id=True): - """Creates an instance based on the contents of the JSON string provided. - Properties in the JSON string that are not found in the script object are ignored. - - Args: - json_string (str): JSON string containing the property settings. - create_new_id (bool): Do not use the existing id. Defaults to True. - - Returns: - object: An instance of the class, populated from the property settings in the JSON string. - """ - new_object = cls() - new_object.title = '' - new_object.update_from_json(json_string) - if not (new_object['title'] and new_object['script']): - raise ScriptImportError(N_('Invalid script package')) - if create_new_id or not new_object['id']: - new_object._set_new_id() - return new_object - - def update_from_json(self, json_string): - """Updates the values of the properties based on the contents of the JSON string provided. - Properties in the JSON string that are not found in the script object are ignored. - - Args: - json_string (str): JSON string containing the property settings. - """ - try: - decoded_string = json.loads(json_string) - except json.decoder.JSONDecodeError: - raise ScriptImportError(N_("Unable to decode JSON string")) - self._update_from_dict(decoded_string) - class FileNamingScript(PicardScript): """Picard file naming script class diff --git a/picard/ui/scripteditor.py b/picard/ui/scripteditor.py index 29840200f..2ae975d5a 100644 --- a/picard/ui/scripteditor.py +++ b/picard/ui/scripteditor.py @@ -524,7 +524,7 @@ class ScriptEditorDialog(PicardDialog): readonly=False, deletable=True, ) - self.naming_scripts.insert(0, script_item.to_json()) + self.naming_scripts.insert(0, script_item.to_yaml()) self.selected_script_id = script_item['id'] self.ui.preset_naming_scripts.blockSignals(True) @@ -541,11 +541,11 @@ class ScriptEditorDialog(PicardDialog): count = 0 # Use separate counter rather than `i` in case ScriptImportError triggers, resulting in an incorrect index count. for i in range(len(self.naming_scripts)): try: - script_item = FileNamingScript().create_from_json(self.naming_scripts[i], create_new_id=False) + script_item = FileNamingScript().create_from_yaml(self.naming_scripts[i], create_new_id=False) except ScriptImportError: pass else: - self.naming_scripts[i] = script_item.to_json() # Ensure scripts are stored with id codes + self.naming_scripts[i] = script_item.to_yaml() # Ensure scripts are stored with id codes idx, count = _add_and_check(idx, count, self.SCRIPT_TITLE_USER % script_item["title"], script_item) for script_item in get_file_naming_script_presets(): @@ -633,7 +633,7 @@ class ScriptEditorDialog(PicardDialog): script_item = self.ui.preset_naming_scripts.itemData(idx) # Only add items that can be removed -- no presets if script_item.deletable: - self.naming_scripts.append(script_item.to_json()) + self.naming_scripts.append(script_item.to_yaml()) def get_selected_item(self): """Get the selected item from the script selection combo box. diff --git a/test/test_script_serializer.py b/test/test_script_serializer.py index 2999aeeca..9e5c28652 100644 --- a/test/test_script_serializer.py +++ b/test/test_script_serializer.py @@ -22,6 +22,8 @@ import datetime +import yaml + from test.picardtestcase import PicardTestCase from picard.script.serializer import ( @@ -51,6 +53,9 @@ class PicardScriptTest(PicardTestCase): # Restore original datetime object datetime.datetime = self.original_datetime + def assertYamlEquals(self, yaml_str, obj, msg=None): + self.assertEqual(obj, yaml.safe_load(yaml_str), msg) + def test_script_object_1(self): # Check initial loaded values. test_script = PicardScript(title='Script 1', script='Script text', id='12345', last_updated='2021-04-26', script_language_version='1.0') @@ -58,7 +63,7 @@ class PicardScriptTest(PicardTestCase): self.assertEqual(test_script['id'], '12345') self.assertEqual(test_script.last_updated, '2021-04-26') self.assertEqual(test_script['last_updated'], '2021-04-26') - self.assertEqual(test_script.to_json(), '{"id": "12345", "script": "Script text", "script_language_version": "1.0", "title": "Script 1"}') + self.assertYamlEquals(test_script.to_yaml(), {"id": "12345", "script": "Script text\n", "script_language_version": "1.0", "title": "Script 1"}) def test_script_object_2(self): # Check updating values directly so as not to modify `last_updated`. @@ -93,9 +98,9 @@ class PicardScriptTest(PicardTestCase): self.assertEqual(test_script['last_updated'], '2020-01-02 12:34:56 UTC') def test_script_object_5(self): - # Check updating values from JSON that modify `last_updated`. + # Check updating values from dict that modify `last_updated`. test_script = PicardScript(title='Script 1', script='Script text', id='12345', last_updated='2021-04-26') - test_script.update_from_json('{"script": "Updated script"}') + test_script.update_from_dict({"script": "Updated script"}) self.assertEqual(test_script.script, 'Updated script') self.assertEqual(test_script['script'], 'Updated script') self.assertEqual(test_script.last_updated, '2020-01-02 12:34:56 UTC') @@ -106,16 +111,16 @@ class PicardScriptTest(PicardTestCase): test_script = PicardScript(title='Script 1', script='Script text', id='12345', last_updated='2021-04-26', script_language_version='1.0') test_script.update_script_setting(description='Updated description') self.assertEqual(test_script['last_updated'], '2021-04-26') - self.assertEqual(test_script.to_json(), '{"id": "12345", "script": "Script text", "script_language_version": "1.0", "title": "Script 1"}') + self.assertYamlEquals(test_script.to_yaml(), {"id": "12345", "script": "Script text\n", "script_language_version": "1.0", "title": "Script 1"}) with self.assertRaises(AttributeError): print(test_script.description) def test_script_object_7(self): - # Test that extra (unknown) settings are ignored during updating from JSON string + # Test that extra (unknown) settings are ignored during updating from dict test_script = PicardScript(title='Script 1', script='Script text', id='12345', last_updated='2021-04-26', script_language_version='1.0') - test_script.update_from_json('{"description": "Updated description"}') + test_script.update_from_dict({"description": "Updated description"}) self.assertEqual(test_script['last_updated'], '2021-04-26') - self.assertEqual(test_script.to_json(), '{"id": "12345", "script": "Script text", "script_language_version": "1.0", "title": "Script 1"}') + self.assertYamlEquals(test_script.to_yaml(), {"id": "12345", "script": "Script text\n", "script_language_version": "1.0", "title": "Script 1"}) with self.assertRaises(AttributeError): print(test_script.description) @@ -125,12 +130,10 @@ class PicardScriptTest(PicardTestCase): self.assertEqual(test_script['unknown_setting'], None) def test_script_object_9(self): - # Test that an exception is raised when creating or updating using an invalid JSON string + # Test that an exception is raised when creating or updating using an invalid YAML string with self.assertRaises(ScriptImportError): - test_script = PicardScript().create_from_json('Not a JSON string') - test_script = PicardScript(title='Script 1', script='Script text', id='12345', last_updated='2021-04-26', script_language_version='1.0') - with self.assertRaises(ScriptImportError): - test_script.update_from_json('Not a JSON string') + PicardScript().create_from_yaml('Not a YAML string') + PicardScript(title='Script 1', script='Script text', id='12345', last_updated='2021-04-26', script_language_version='1.0') def test_naming_script_object_1(self): # Check initial loaded values. @@ -160,31 +163,3 @@ class PicardScriptTest(PicardTestCase): " Script text\n" "id: '12345'\n" ) - self.assertEqual( - test_script.to_json(), - '{' - '"author": "Script author", ' - '"description": "Script description", ' - '"id": "12345", ' - '"last_updated": "2021-04-26", ' - '"license": "", ' - '"script": "Script text", ' - '"script_language_version": "1.0", ' - '"title": "Script 1", ' - '"version": ""' - '}' - ) - self.assertEqual( - test_script.to_json(indent=4), - '{\n' - ' "author": "Script author",\n' - ' "description": "Script description",\n' - ' "id": "12345",\n' - ' "last_updated": "2021-04-26",\n' - ' "license": "",\n' - ' "script": "Script text",\n' - ' "script_language_version": "1.0",\n' - ' "title": "Script 1",\n' - ' "version": ""\n' - '}' - )