PICARD-2189: Add appdirs module to fix getting proper cache location

The cache location previously was wrong because it was created before app and org name had been set. The new module fixes this and allows more flexible replacement of the implementation.

picard/const/__init__.py

@@ -34,28 +34,20 @@
 
 import builtins
 from collections import OrderedDict
-import os
 
-from PyQt5.QtCore import QStandardPaths
-
-from picard import (
-    PICARD_APP_NAME,
-    PICARD_ORG_NAME,
-    PICARD_VERSION,
-)
+from picard import PICARD_VERSION
 from picard.const.attributes import MB_ATTRIBUTES
+from picard.const import appdirs
 
 # Install gettext "noop" function in case const.py gets imported directly.
 builtins.__dict__['N_'] = lambda a: a
 
 
 # Config directory
-_appconfiglocation = QStandardPaths.writableLocation(QStandardPaths.AppConfigLocation)
-USER_DIR = os.path.normpath(os.path.join(_appconfiglocation, PICARD_ORG_NAME, PICARD_APP_NAME))
-USER_PLUGIN_DIR = os.path.normpath(os.path.join(USER_DIR, "plugins"))
+USER_DIR = appdirs.config_folder()
+USER_PLUGIN_DIR = appdirs.plugin_folder()
 
 # Network Cache default settings
-CACHE_DIR = os.path.normpath(QStandardPaths.writableLocation(QStandardPaths.CacheLocation))
 CACHE_SIZE_IN_BYTES = 100*1000*1000
 
 # AcousticBrainz

picard/const/appdirs.py

@@ -0,0 +1,50 @@
+# -*- coding: utf-8 -*-
+#
+# Picard, the next-generation MusicBrainz tagger
+#
+# Copyright (C) 2021 Philipp Wolfer
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+
+import os.path
+
+from PyQt5.QtCore import (
+    QCoreApplication,
+    QStandardPaths,
+)
+
+from picard import (
+    PICARD_APP_NAME,
+    PICARD_ORG_NAME,
+)
+
+
+# Ensure the application is properly configured for the paths to work
+QCoreApplication.setApplicationName(PICARD_APP_NAME)
+QCoreApplication.setOrganizationName(PICARD_ORG_NAME)
+
+
+def config_folder():
+    return os.path.normpath(QStandardPaths.writableLocation(QStandardPaths.AppConfigLocation))
+
+
+def cache_folder():
+    return os.path.normpath(QStandardPaths.writableLocation(QStandardPaths.CacheLocation))
+
+
+def plugin_folder():
+    # FIXME: This really should be in QStandardPaths.AppDataLocation instead,
+    # but this is a breaking change that requires data migration
+    return os.path.normpath(os.path.join(config_folder(), 'plugins'))

picard/webservice/__init__.py

@@ -56,8 +56,8 @@ from picard import (
 )
 from picard.config import get_config
 from picard.const import (
-    CACHE_DIR,
     CACHE_SIZE_IN_BYTES,
+    appdirs,
 )
 from picard.oauth import OAuthManager
 from picard.util import (
@@ -324,7 +324,7 @@ class WebService(QtCore.QObject):
         if cache_size_in_bytes is None:
             cache_size_in_bytes = CACHE_SIZE_IN_BYTES
         cache = QtNetwork.QNetworkDiskCache()
-        cache.setCacheDirectory(os.path.join(CACHE_DIR, 'network'))
+        cache.setCacheDirectory(os.path.join(appdirs.cache_folder(), 'network'))
         cache.setMaximumCacheSize(cache_size_in_bytes)
         self.manager.setCache(cache)
         log.debug("NetworkDiskCache dir: %r current size: %s max size: %s",

scripts/pyinstaller/portable-hook.py

@@ -28,6 +28,7 @@ from picard import (
     PICARD_ORG_NAME,
 )
 import picard.const
+import picard.const.appdirs
 
 
 # The portable version stores all data in a folder beside the executable
@@ -41,9 +42,13 @@ if '--config-file' not in sys.argv and '-c' not in sys.argv:
     sys.argv.append(os.path.join(basedir, 'Config.ini'))
 
 # Setup plugin folder
-picard.const.USER_PLUGIN_DIR = os.path.normpath(os.path.join(basedir, 'Plugins'))
+plugindir = os.path.normpath(os.path.join(basedir, 'Plugins'))
+picard.const.USER_PLUGIN_DIR = plugindir
 
 # Set standard cache location
 cachedir = os.path.normpath(os.path.join(basedir, 'Cache'))
 os.makedirs(cachedir, exist_ok=True)
-picard.const.CACHE_DIR = cachedir
+
+picard.const.appdirs.config_folder = lambda: basedir
+picard.const.appdirs.cache_folder = lambda: cachedir
+picard.const.appdirs.plugin_folder = lambda: plugindir

test/test_const_appdirs.py

@@ -0,0 +1,76 @@
+# -*- coding: utf-8 -*-
+#
+# Picard, the next-generation MusicBrainz tagger
+#
+# Copyright (C) 2021 Philipp Wolfer
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+
+import os.path
+import unittest
+
+from test.picardtestcase import PicardTestCase
+
+from picard.const.appdirs import (
+    cache_folder,
+    config_folder,
+    plugin_folder,
+)
+from picard.const.sys import (
+    IS_LINUX,
+    IS_MACOS,
+    IS_WIN,
+)
+
+
+class AppPathsTest(PicardTestCase):
+    def assert_home_path_equals(self, expected, actual):
+        self.assertEqual(os.path.normpath(os.path.expanduser(expected)), actual)
+
+    @unittest.skipUnless(IS_WIN, "Windows test")
+    def test_config_folder_win(self):
+        self.assert_home_path_equals('~/AppData/Local/MusicBrainz/Picard', config_folder())
+
+    @unittest.skipUnless(IS_MACOS, "macOS test")
+    def test_config_folder_macos(self):
+        self.assert_home_path_equals('~/Library/Preferences/MusicBrainz/Picard', config_folder())
+
+    @unittest.skipUnless(IS_LINUX, "Linux test")
+    def test_config_folder_linux(self):
+        self.assert_home_path_equals('~/.config/MusicBrainz/Picard', config_folder())
+
+    @unittest.skipUnless(IS_WIN, "Windows test")
+    def test_cache_folder_win(self):
+        self.assert_home_path_equals('~/AppData/Local/MusicBrainz/Picard/cache', cache_folder())
+
+    @unittest.skipUnless(IS_MACOS, "macOS test")
+    def test_cache_folder_macos(self):
+        self.assert_home_path_equals('~/Library/Caches/MusicBrainz/Picard', cache_folder())
+
+    @unittest.skipUnless(IS_LINUX, "Linux test")
+    def test_cache_folder_linux(self):
+        self.assert_home_path_equals('~/.cache/MusicBrainz/Picard', cache_folder())
+
+    @unittest.skipUnless(IS_WIN, "Windows test")
+    def test_plugin_folder_win(self):
+        self.assert_home_path_equals('~/AppData/Local/MusicBrainz/Picard/plugins', plugin_folder())
+
+    @unittest.skipUnless(IS_MACOS, "macOS test")
+    def test_plugin_folder_macos(self):
+        self.assert_home_path_equals('~/Library/Preferences/MusicBrainz/Picard/plugins', plugin_folder())
+
+    @unittest.skipUnless(IS_LINUX, "Linux test")
+    def test_plugin_folder_linux(self):
+        self.assert_home_path_equals('~/.config/MusicBrainz/Picard/plugins', plugin_folder())