Merge pull request ome#242 from manics/appdirs

Use OS specific application directories instead of ~/omero

setup.py

@@ -214,6 +214,7 @@ def read(fname):
     },
     python_requires='>=3',
     install_requires=[
+        'appdirs',
         'future',
         'numpy',
         'Pillow',

src/omero/plugins/import.py

@@ -579,8 +579,7 @@ def download_omero_java(self, version):
         omero_java_zip = OMERO_JAVA_ZIP.format(version=version)
         self.ctx.err("Downloading %s" % omero_java_zip)
         jars_dir, omero_java_txt = self._userdir_jars(parentonly=True)
-        if not jars_dir.exists():
-            jars_dir.mkdir()
+        jars_dir.makedirs_p()
         with requests.get(omero_java_zip) as resp:
             with ZipFile(BytesIO(resp.content)) as zipfile:
                 topdirs = set(f.filename.split(

src/omero/util/__init__.py

@@ -12,6 +12,7 @@
 from future.utils import native_str
 from past.utils import old_div
 from builtins import object
+from appdirs import user_data_dir, user_cache_dir
 import os
 import sys
 import Ice
@@ -29,6 +30,8 @@
 import omero.ObjectFactoryRegistrar as ofr
 
 from omero.util.decorators import locked
+from omero_version import omero_version
+
 import omero_ext.path as path
 
 LOGDIR = os.path.join("var", "log")
@@ -42,6 +45,9 @@
 orig_stdout = sys.stdout
 orig_stderr = sys.stderr
 
+# Application name, Author (Windows only), Major version
+APPDIR_DEFAULTS = ('OMERO.py', 'OME', omero_version.split('.')[0])
+
 
 def make_logname(self):
     """
@@ -833,17 +839,35 @@ def get_user(default=None):
 
 
 def get_omero_userdir():
-    """Returns the OMERO user directory"""
+    """
+    Returns the OMERO user directory
+
+    In 6.0.0 the default will change to use appdirs.user_data_dir.
+    You can enable this behaviour now by setting OMERO_USERDIR="" (empty
+    string) instead of unset.
+
+    Note that files in the old user directory will not be migrated.
+    """
     omero_userdir = os.environ.get('OMERO_USERDIR', None)
     if omero_userdir:
         return path.path(omero_userdir)
+    elif omero_userdir == "":
+        return path.path(user_data_dir(*APPDIR_DEFAULTS))
     else:
         return old_div(path.path(get_user_dir()), "omero")
 
 
 def get_omero_user_cache_dir():
     """Returns the OMERO user cache directory"""
-    return get_omero_userdir()
+    omero_userdir = os.environ.get('OMERO_USERDIR', None)
+    if omero_userdir:
+        return path.path(omero_userdir) / "cache"
+    else:
+        return path.path(user_cache_dir(*APPDIR_DEFAULTS))
+
+
+# Other application directories may be added in future, see
+# https://pypi.org/project/appdirs/
 
 
 def get_user_dir():

test/unit/clitest/test_export.py

@@ -65,7 +65,7 @@ def invoke(self, string):
         self.cli.invoke(string, strict=True)
 
     def testSimpleExport(self):
-        self.invoke("x -f %s Image:3" % self.p)
+        self.invoke(["x", "-f", self.p, "Image:3"])
 
     def testStdOutExport(self):
         """

test/unit/clitest/test_prefs.py

@@ -188,23 +188,23 @@ def testPath(self, capsys):
     def testLoad(self, capsys):
         to_load = create_path()
         to_load.write_text("A=B")
-        self.invoke("load %s" % to_load)
+        self.invoke(["load", to_load])
         self.assertStdoutStderr(capsys)
         self.invoke("get")
         self.assertStdoutStderr(capsys, out="A=B")
 
         # Same property/value pairs should pass
-        self.invoke("load %s" % to_load)
+        self.invoke(["load", to_load])
 
         to_load.write_text("A=C")
         with pytest.raises(NonZeroReturnCode):
             # Different property/value pair should fail
-            self.invoke("load %s" % to_load)
+            self.invoke(["load", to_load])
         self.assertStdoutStderr(
             capsys, err="Duplicate property: A ('B' => 'C')")
 
         # Quiet load
-        self.invoke("load -q %s" % to_load)
+        self.invoke(["load", "-q", to_load])
         self.assertStdoutStderr(capsys)
         self.invoke("get")
         self.assertStdoutStderr(capsys, out="A=C")
@@ -217,7 +217,7 @@ def testLoadDoesNotExist(self):
     def testLoadMultiLine(self, capsys):
         to_load = create_path()
         to_load.write_text("A=B\\\nC")
-        self.invoke("load %s" % to_load)
+        self.invoke(["load", to_load])
         self.invoke("get")
         self.assertStdoutStderr(capsys, out="A=BC")
 
@@ -228,7 +228,7 @@ def testLoadWhitelist(self, capsys, validkeyvalue):
         valid_key, valid_value = validkeyvalue
         to_load = create_path()
         to_load.write_text("%s=%s\n" % (valid_key, valid_value))
-        self.invoke("load %s" % to_load)
+        self.invoke(["load", to_load])
         self.invoke("get %s" % valid_key)
         self.assertStdoutStderr(capsys, out=valid_value)
 
@@ -244,7 +244,7 @@ def testLoadInvalidKey(self, capsys, invalidline, invalidkey):
         to_load = create_path()
         to_load.write_text("C=D\n%s\nH=I\n" % invalidline)
         with pytest.raises(NonZeroReturnCode):
-            self.invoke("load %s" % to_load)
+            self.invoke(["load", to_load])
         self.assertStdoutStderr(
             capsys, err="Illegal property name: %s" % invalidkey)
         self.invoke("get")
@@ -269,7 +269,7 @@ def testSetInvalidKey(self, capsys, invalid_key):
     def testSetFromFile(self, capsys):
         to_load = create_path()
         to_load.write_text("Test")
-        self.invoke("set -f %s A" % to_load)
+        self.invoke(["set", "-f", to_load, "A"])
         self.invoke("get")
         self.assertStdoutStderr(capsys, out="A=Test")
 

test/unit/test_util.py

@@ -39,7 +39,8 @@
 from omero.util.text import CSVStyle, JSONStyle, PlainStyle, TableBuilder
 from omero.util.upgrade_check import UpgradeCheck
 from omero.util.temp_files import manager
-from omero.util import get_user_dir
+from omero.util import (
+    get_omero_userdir, get_omero_user_cache_dir, get_user_dir)
 from omero_version import omero_version
 import omero.util.image_utils as image_utils
 try:
@@ -314,3 +315,33 @@ def test_paste_image(self):
         data_canvas[256, 256] = [255, 255, 0]
         canvas = Image.fromarray(data_canvas, 'RGB')
         image_utils.paste_image(img, canvas, 0, 0)
+
+
+class TestUserdirs(object):
+
+    def testUserdirEnvironmentDefault(self, monkeypatch):
+        monkeypatch.delenv('OMERO_USERDIR', raising=False)
+
+        assert get_omero_userdir().basename() == 'omero'
+
+        c = get_omero_user_cache_dir()
+        assert c.basename() == omero_version.split('.')[0]
+        assert 'OMERO.py' in str(c)
+
+    def testUserdirEnvironmentAppdir(self, monkeypatch):
+        monkeypatch.setenv('OMERO_USERDIR', '')
+
+        d = get_omero_userdir()
+        assert d.basename() == omero_version.split('.')[0]
+        assert 'OMERO.py' in str(d)
+
+        c = get_omero_user_cache_dir()
+        assert c.basename() == omero_version.split('.')[0]
+        assert 'OMERO.py' in str(c)
+
+    def testUserdirEnvironmentSet(self, monkeypatch, tmpdir):
+        monkeypatch.setenv('OMERO_USERDIR', str(tmpdir))
+
+        assert get_omero_userdir() == tmpdir
+
+        assert get_omero_user_cache_dir() == tmpdir / "cache"