From 9917f2ac3aa24e2e690ffb237c5a50434c164793 Mon Sep 17 00:00:00 2001 From: alicezou Date: Sun, 27 Mar 2022 09:56:47 -0400 Subject: [PATCH 01/12] create db without prompt --- beets/dbcore/db.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index acd131be26..0e484d8ce4 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -900,9 +900,15 @@ class Database: """The current revision of the database. To be increased whenever data is written in a transaction. """ + def _path_checker(self, path): + newpath = os.path.dirname(path) + if not os.path.isdir(newpath): + os.makedirs(newpath) + def __init__(self, path, timeout=5.0): self.path = path + self._path_checker(path) self.timeout = timeout self._connections = {} From b07b0e2f7e377d466ef4e36901396a4bb7dcfdbf Mon Sep 17 00:00:00 2001 From: alicezou Date: Sun, 27 Mar 2022 10:33:54 -0400 Subject: [PATCH 02/12] add prompt --- beets/dbcore/db.py | 4 +++- beets/ui/commands.py | 6 ++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index 0e484d8ce4..bafd32ded2 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -903,7 +903,9 @@ class Database: def _path_checker(self, path): newpath = os.path.dirname(path) if not os.path.isdir(newpath): - os.makedirs(newpath) + from beets.ui.commands import database_dir_creation + if database_dir_creation(newpath): + os.makedirs(newpath) def __init__(self, path, timeout=5.0): diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 3a33740133..bf2b184d27 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -1398,6 +1398,12 @@ def show_version(lib, opts, args): version_cmd.func = show_version default_commands.append(version_cmd) +# database_location: return true if user wants to create the parent directories. +def database_dir_creation(path): + # Ask the user for a choice. + return ui.input_yn("{} does not exist, create it (Y/n)?" + .format(displayable_path(path))) + # modify: Declaratively change metadata. From c67245ed65a0c7f707ef9e755e32b782b276f69a Mon Sep 17 00:00:00 2001 From: alicezou Date: Sun, 27 Mar 2022 10:42:16 -0400 Subject: [PATCH 03/12] style checker, change log --- beets/dbcore/db.py | 3 ++- beets/ui/commands.py | 7 +++++-- docs/changelog.rst | 1 + 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index bafd32ded2..979c973a95 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -900,6 +900,8 @@ class Database: """The current revision of the database. To be increased whenever data is written in a transaction. """ + + # Check whether parental directories exist. def _path_checker(self, path): newpath = os.path.dirname(path) if not os.path.isdir(newpath): @@ -907,7 +909,6 @@ def _path_checker(self, path): if database_dir_creation(newpath): os.makedirs(newpath) - def __init__(self, path, timeout=5.0): self.path = path self._path_checker(path) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index bf2b184d27..da78d682a9 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -1398,11 +1398,14 @@ def show_version(lib, opts, args): version_cmd.func = show_version default_commands.append(version_cmd) -# database_location: return true if user wants to create the parent directories. +# database_location: return true if user +# wants to create the parent directories. + + def database_dir_creation(path): # Ask the user for a choice. return ui.input_yn("{} does not exist, create it (Y/n)?" - .format(displayable_path(path))) + .format(displayable_path(path))) # modify: Declaratively change metadata. diff --git a/docs/changelog.rst b/docs/changelog.rst index 002b96c5dc..fe4afa8100 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -7,6 +7,7 @@ Changelog Changelog goes here! New features: +* Create the parental directories for database if they do not exist. * :ref:`musicbrainz-config`: a new :ref:`musicbrainz.enabled` option allows disabling the MusicBrainz metadata source during the autotagging process From d3d9318c18ca5b40ad8e41f20b5ebe0ea49856d7 Mon Sep 17 00:00:00 2001 From: alicezou Date: Sun, 27 Mar 2022 10:58:09 -0400 Subject: [PATCH 04/12] change doc --- docs/guides/main.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/guides/main.rst b/docs/guides/main.rst index 8dbb113c4d..feb9185707 100644 --- a/docs/guides/main.rst +++ b/docs/guides/main.rst @@ -142,7 +142,8 @@ place to start:: Change that first path to a directory where you'd like to keep your music. Then, for ``library``, choose a good place to keep a database file that keeps an index -of your music. (The config's format is `YAML`_. You'll want to configure your +of your music. Beets will prompt you if the parental directories for database do +not exist. (The config's format is `YAML`_. You'll want to configure your text editor to use spaces, not real tabs, for indentation. Also, ``~`` means your home directory in these paths, even on Windows.) From 879ed7f5c2de73b44aa849b45b6915fecfca1380 Mon Sep 17 00:00:00 2001 From: alicezou Date: Sun, 27 Mar 2022 15:06:00 -0400 Subject: [PATCH 05/12] fix test cases, support in memory db --- beets/dbcore/db.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index 979c973a95..84fa1173dd 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -903,6 +903,8 @@ class Database: # Check whether parental directories exist. def _path_checker(self, path): + if path == ":memory:": # For testing + return newpath = os.path.dirname(path) if not os.path.isdir(newpath): from beets.ui.commands import database_dir_creation From c0d05f854508bd452e0c0b2927bca4d639b4577b Mon Sep 17 00:00:00 2001 From: alicezou Date: Sun, 27 Mar 2022 17:09:30 -0400 Subject: [PATCH 06/12] add test cases --- beets/dbcore/db.py | 2 +- test/test_dbcore.py | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index 84fa1173dd..243308c0d2 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -903,7 +903,7 @@ class Database: # Check whether parental directories exist. def _path_checker(self, path): - if path == ":memory:": # For testing + if path == ":memory:": # For testing return newpath = os.path.dirname(path) if not os.path.isdir(newpath): diff --git a/test/test_dbcore.py b/test/test_dbcore.py index 603d85bad5..5dbff52234 100644 --- a/test/test_dbcore.py +++ b/test/test_dbcore.py @@ -19,6 +19,8 @@ import shutil import sqlite3 import unittest +from random import random +from unittest import mock from test import _common from beets import dbcore @@ -760,8 +762,31 @@ def test_no_results(self): ModelFixture1, dbcore.query.FalseQuery()).get()) +class ParentalDirCreation(unittest.TestCase): + @mock.patch('builtins.input', side_effect=['y', ]) + def test_create_yes(self, _): + non_exist_path = "ParentalDirCreationTest/nonexist/" + str(random()) + try: + dbcore.Database(non_exist_path) + except OSError as e: + raise e + shutil.rmtree("ParentalDirCreationTest") + + @mock.patch('builtins.input', side_effect=['n', ]) + def test_create_no(self, _): + non_exist_path = "ParentalDirCreationTest/nonexist/" + str(random()) + try: + dbcore.Database(non_exist_path) + except OSError as e: + raise e + if os.path.exists("ParentalDirCreationTest/nonexist/"): + shutil.rmtree("ParentalDirCreationTest") + raise OSError("Should not create dir") + + def suite(): return unittest.TestLoader().loadTestsFromName(__name__) + if __name__ == '__main__': unittest.main(defaultTest='suite') From 9029a8a6b39cc5ad6746cd8840ddaefe9bffd020 Mon Sep 17 00:00:00 2001 From: alicezou Date: Sun, 27 Mar 2022 23:05:23 -0400 Subject: [PATCH 07/12] check bytes --- beets/dbcore/db.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index 243308c0d2..397dbcedf4 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -903,7 +903,7 @@ class Database: # Check whether parental directories exist. def _path_checker(self, path): - if path == ":memory:": # For testing + if not isinstance(path, bytes) and path == ':memory:': # in memory db return newpath = os.path.dirname(path) if not os.path.isdir(newpath): From a0b0028874989b0bc1cbbed00be4b05a55b1db52 Mon Sep 17 00:00:00 2001 From: alicezou Date: Tue, 29 Mar 2022 19:58:15 -0400 Subject: [PATCH 08/12] working db --- beets/ui/commands.py | 2 +- test/test_dbcore.py | 25 +++++++++++-------------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index da78d682a9..394a2831a8 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -1404,7 +1404,7 @@ def show_version(lib, opts, args): def database_dir_creation(path): # Ask the user for a choice. - return ui.input_yn("{} does not exist, create it (Y/n)?" + return ui.input_yn("The database directory {} does not exists, create it (Y/n)?" .format(displayable_path(path))) diff --git a/test/test_dbcore.py b/test/test_dbcore.py index 5dbff52234..c92f31c1f5 100644 --- a/test/test_dbcore.py +++ b/test/test_dbcore.py @@ -762,25 +762,22 @@ def test_no_results(self): ModelFixture1, dbcore.query.FalseQuery()).get()) -class ParentalDirCreation(unittest.TestCase): +class ParentalDirCreation(_common.TestCase): @mock.patch('builtins.input', side_effect=['y', ]) def test_create_yes(self, _): - non_exist_path = "ParentalDirCreationTest/nonexist/" + str(random()) - try: - dbcore.Database(non_exist_path) - except OSError as e: - raise e - shutil.rmtree("ParentalDirCreationTest") + non_exist_path = _common.util.py3_path(os.path.join( + self.temp_dir, b'nonexist', str(random()).encode())) + dbcore.Database(non_exist_path) @mock.patch('builtins.input', side_effect=['n', ]) def test_create_no(self, _): - non_exist_path = "ParentalDirCreationTest/nonexist/" + str(random()) - try: - dbcore.Database(non_exist_path) - except OSError as e: - raise e - if os.path.exists("ParentalDirCreationTest/nonexist/"): - shutil.rmtree("ParentalDirCreationTest") + non_exist_path_parent = _common.util.py3_path( + os.path.join(self.temp_dir, b'nonexist')) + non_exist_path = _common.util.py3_path(os.path.join( + non_exist_path_parent.encode(), str(random()).encode())) + dbcore.Database(non_exist_path) + if os.path.exists(non_exist_path_parent): + shutil.rmtree(non_exist_path_parent) raise OSError("Should not create dir") From 67e778fec6a4bd130ef0fc61714dcdaf95f874c4 Mon Sep 17 00:00:00 2001 From: alicezou Date: Tue, 29 Mar 2022 20:04:56 -0400 Subject: [PATCH 09/12] switch to control_stdin --- test/test_dbcore.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/test/test_dbcore.py b/test/test_dbcore.py index c92f31c1f5..95c52196f0 100644 --- a/test/test_dbcore.py +++ b/test/test_dbcore.py @@ -23,6 +23,7 @@ from unittest import mock from test import _common +from test.helper import control_stdin from beets import dbcore from tempfile import mkstemp @@ -763,19 +764,19 @@ def test_no_results(self): class ParentalDirCreation(_common.TestCase): - @mock.patch('builtins.input', side_effect=['y', ]) - def test_create_yes(self, _): + def test_create_yes(self): non_exist_path = _common.util.py3_path(os.path.join( self.temp_dir, b'nonexist', str(random()).encode())) - dbcore.Database(non_exist_path) + with control_stdin('y'): + dbcore.Database(non_exist_path) - @mock.patch('builtins.input', side_effect=['n', ]) - def test_create_no(self, _): + def test_create_no(self): non_exist_path_parent = _common.util.py3_path( os.path.join(self.temp_dir, b'nonexist')) non_exist_path = _common.util.py3_path(os.path.join( non_exist_path_parent.encode(), str(random()).encode())) - dbcore.Database(non_exist_path) + with control_stdin('n'): + dbcore.Database(non_exist_path) if os.path.exists(non_exist_path_parent): shutil.rmtree(non_exist_path_parent) raise OSError("Should not create dir") From 2886296c86eba136effff50a5149e76ed94f1d0f Mon Sep 17 00:00:00 2001 From: alicezou Date: Tue, 29 Mar 2022 21:24:13 -0400 Subject: [PATCH 10/12] fix code review comments --- beets/dbcore/db.py | 11 ----------- beets/ui/__init__.py | 13 +++++++++++++ beets/ui/commands.py | 3 ++- test/test_dbcore.py | 22 ---------------------- test/test_ui_init.py | 40 ++++++++++++++++++++++++++++++++++++++-- 5 files changed, 53 insertions(+), 36 deletions(-) diff --git a/beets/dbcore/db.py b/beets/dbcore/db.py index 397dbcedf4..acd131be26 100755 --- a/beets/dbcore/db.py +++ b/beets/dbcore/db.py @@ -901,19 +901,8 @@ class Database: data is written in a transaction. """ - # Check whether parental directories exist. - def _path_checker(self, path): - if not isinstance(path, bytes) and path == ':memory:': # in memory db - return - newpath = os.path.dirname(path) - if not os.path.isdir(newpath): - from beets.ui.commands import database_dir_creation - if database_dir_creation(newpath): - os.makedirs(newpath) - def __init__(self, path, timeout=5.0): self.path = path - self._path_checker(path) self.timeout = timeout self._connections = {} diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index 121cb5dc0e..b724a963a5 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -1206,11 +1206,24 @@ def _configure(options): util.displayable_path(config.config_dir())) return config +# Check whether parental directories exist. + + +def _check_db_directory_exists(path): + if path == b':memory:': # in memory db + return + newpath = os.path.dirname(path) + if not os.path.isdir(newpath): + from beets.ui.commands import database_dir_creation + if database_dir_creation(newpath): + os.makedirs(newpath) + def _open_library(config): """Create a new library instance from the configuration. """ dbpath = util.bytestring_path(config['library'].as_filename()) + _check_db_directory_exists(dbpath) try: lib = library.Library( dbpath, diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 394a2831a8..1261b17767 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -1404,7 +1404,8 @@ def show_version(lib, opts, args): def database_dir_creation(path): # Ask the user for a choice. - return ui.input_yn("The database directory {} does not exists, create it (Y/n)?" + return ui.input_yn("The database directory {} does not \ + exists, create it (Y/n)?" .format(displayable_path(path))) diff --git a/test/test_dbcore.py b/test/test_dbcore.py index 95c52196f0..80d85c3bb4 100644 --- a/test/test_dbcore.py +++ b/test/test_dbcore.py @@ -19,11 +19,8 @@ import shutil import sqlite3 import unittest -from random import random -from unittest import mock from test import _common -from test.helper import control_stdin from beets import dbcore from tempfile import mkstemp @@ -763,25 +760,6 @@ def test_no_results(self): ModelFixture1, dbcore.query.FalseQuery()).get()) -class ParentalDirCreation(_common.TestCase): - def test_create_yes(self): - non_exist_path = _common.util.py3_path(os.path.join( - self.temp_dir, b'nonexist', str(random()).encode())) - with control_stdin('y'): - dbcore.Database(non_exist_path) - - def test_create_no(self): - non_exist_path_parent = _common.util.py3_path( - os.path.join(self.temp_dir, b'nonexist')) - non_exist_path = _common.util.py3_path(os.path.join( - non_exist_path_parent.encode(), str(random()).encode())) - with control_stdin('n'): - dbcore.Database(non_exist_path) - if os.path.exists(non_exist_path_parent): - shutil.rmtree(non_exist_path_parent) - raise OSError("Should not create dir") - - def suite(): return unittest.TestLoader().loadTestsFromName(__name__) diff --git a/test/test_ui_init.py b/test/test_ui_init.py index bb9a922a5d..9f9487a6a7 100644 --- a/test/test_ui_init.py +++ b/test/test_ui_init.py @@ -15,11 +15,16 @@ """Test module for file ui/__init__.py """ - +import os +import shutil import unittest -from test import _common +from random import random +from copy import deepcopy from beets import ui +from test import _common +from test.helper import control_stdin +from beets import config class InputMethodsTest(_common.TestCase): @@ -121,8 +126,39 @@ def test_human_seconds(self): self.assertEqual(h, ui.human_seconds(i)) +class ParentalDirCreation(_common.TestCase): + def test_create_yes(self): + non_exist_path = _common.util.py3_path(os.path.join( + self.temp_dir, b'nonexist', str(random()).encode())) + # Deepcopy instead of recovering because exceptions might + # occcur; wish I can use a golang defer here. + test_config = deepcopy(config) + test_config['library'] = non_exist_path + with control_stdin('y'): + ui._open_library(test_config) + + def test_create_no(self): + non_exist_path_parent = _common.util.py3_path( + os.path.join(self.temp_dir, b'nonexist')) + non_exist_path = _common.util.py3_path(os.path.join( + non_exist_path_parent.encode(), str(random()).encode())) + test_config = deepcopy(config) + test_config['library'] = non_exist_path + + with control_stdin('n'): + try: + ui._open_library(test_config) + except ui.UserError: + if os.path.exists(non_exist_path_parent): + shutil.rmtree(non_exist_path_parent) + raise OSError("Parent directories should not be created.") + else: + raise OSError("Parent directories should not be created.") + + def suite(): return unittest.TestLoader().loadTestsFromName(__name__) + if __name__ == '__main__': unittest.main(defaultTest='suite') From b609cae1116e4bc9c73ac3748bc75f32eed7f682 Mon Sep 17 00:00:00 2001 From: alicezou Date: Wed, 30 Mar 2022 12:56:38 -0400 Subject: [PATCH 11/12] change location for database_dir_creation, change docs --- beets/ui/__init__.py | 8 +++++++- beets/ui/commands.py | 7 ------- docs/changelog.rst | 3 ++- docs/guides/main.rst | 3 +-- 4 files changed, 10 insertions(+), 11 deletions(-) diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index b724a963a5..a50c3464f8 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -1209,12 +1209,18 @@ def _configure(options): # Check whether parental directories exist. +def database_dir_creation(path): + # Ask the user for a choice. + return input_yn("The database directory {} does not \ + exists. Create it (Y/n)?" + .format(util.displayable_path(path))) + + def _check_db_directory_exists(path): if path == b':memory:': # in memory db return newpath = os.path.dirname(path) if not os.path.isdir(newpath): - from beets.ui.commands import database_dir_creation if database_dir_creation(newpath): os.makedirs(newpath) diff --git a/beets/ui/commands.py b/beets/ui/commands.py index 1261b17767..a18b09f7d6 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -1402,13 +1402,6 @@ def show_version(lib, opts, args): # wants to create the parent directories. -def database_dir_creation(path): - # Ask the user for a choice. - return ui.input_yn("The database directory {} does not \ - exists, create it (Y/n)?" - .format(displayable_path(path))) - - # modify: Declaratively change metadata. def modify_items(lib, mods, dels, query, write, move, album, confirm): diff --git a/docs/changelog.rst b/docs/changelog.rst index fe4afa8100..4126a8cce3 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -7,8 +7,9 @@ Changelog Changelog goes here! New features: -* Create the parental directories for database if they do not exist. +* Create the parental directories for database if they do not exist. + :bug:`3808` :bug:`4327` * :ref:`musicbrainz-config`: a new :ref:`musicbrainz.enabled` option allows disabling the MusicBrainz metadata source during the autotagging process * :doc:`/plugins/kodiupdate`: Now supports multiple kodi instances diff --git a/docs/guides/main.rst b/docs/guides/main.rst index feb9185707..8dbb113c4d 100644 --- a/docs/guides/main.rst +++ b/docs/guides/main.rst @@ -142,8 +142,7 @@ place to start:: Change that first path to a directory where you'd like to keep your music. Then, for ``library``, choose a good place to keep a database file that keeps an index -of your music. Beets will prompt you if the parental directories for database do -not exist. (The config's format is `YAML`_. You'll want to configure your +of your music. (The config's format is `YAML`_. You'll want to configure your text editor to use spaces, not real tabs, for indentation. Also, ``~`` means your home directory in these paths, even on Windows.) From fa5862d7d55c387accfbdc5b29e2d2dea6c7dbfc Mon Sep 17 00:00:00 2001 From: alicezou Date: Thu, 7 Apr 2022 15:21:34 -0400 Subject: [PATCH 12/12] change func name, inline function, fix typo --- beets/ui/__init__.py | 17 +++++------------ beets/ui/commands.py | 3 --- 2 files changed, 5 insertions(+), 15 deletions(-) diff --git a/beets/ui/__init__.py b/beets/ui/__init__.py index a50c3464f8..365a0d5293 100644 --- a/beets/ui/__init__.py +++ b/beets/ui/__init__.py @@ -1206,22 +1206,15 @@ def _configure(options): util.displayable_path(config.config_dir())) return config -# Check whether parental directories exist. - -def database_dir_creation(path): - # Ask the user for a choice. - return input_yn("The database directory {} does not \ - exists. Create it (Y/n)?" - .format(util.displayable_path(path))) - - -def _check_db_directory_exists(path): +def _ensure_db_directory_exists(path): if path == b':memory:': # in memory db return newpath = os.path.dirname(path) if not os.path.isdir(newpath): - if database_dir_creation(newpath): + if input_yn("The database directory {} does not \ + exist. Create it (Y/n)?" + .format(util.displayable_path(newpath))): os.makedirs(newpath) @@ -1229,7 +1222,7 @@ def _open_library(config): """Create a new library instance from the configuration. """ dbpath = util.bytestring_path(config['library'].as_filename()) - _check_db_directory_exists(dbpath) + _ensure_db_directory_exists(dbpath) try: lib = library.Library( dbpath, diff --git a/beets/ui/commands.py b/beets/ui/commands.py index a18b09f7d6..3a33740133 100755 --- a/beets/ui/commands.py +++ b/beets/ui/commands.py @@ -1398,9 +1398,6 @@ def show_version(lib, opts, args): version_cmd.func = show_version default_commands.append(version_cmd) -# database_location: return true if user -# wants to create the parent directories. - # modify: Declaratively change metadata.