CLI bug fixes

components/tools/OmeroPy/src/omero/config.py

@@ -82,34 +82,15 @@ class ConfigXml(object):
     DEFAULT = "omero.config.profile"
     IGNORE = (KEY, DEFAULT)
 
-    def __init__(self, filename, env_config = None, exclusive = True):
+    def __init__(self, filename, env_config=None, exclusive=True, read_only=False):
         self.logger = logging.getLogger(self.__class__.__name__)    #: Logs to the class name
         self.XML = None                                             #: Parsed XML Element
         self.filename = filename                                    #: Path to the file to be read and written
         self.env_config = Environment(env_config)                   #: Environment override
         self.exclusive = exclusive                                  #: Whether or not an exclusive lock should be acquired
+        self.read_only = read_only                                  #: Further, if saving should even be allowed.
         self.save_on_close = True
-
-        try:
-            # Try to open the file for modification
-            # If this fails, then the file is readonly
-            self.source = open(filename, "a+")                      #: Open file handle
-            self.lock = self._open_lock()                           #: Open file handle for lock
-        except IOError:
-            self.logger.debug("open('%s', 'a+') failed" % filename)
-            self.lock = None
-            self.exclusive = False
-            self.save_on_close = False
-
-            # Before we open the file read-only we need to check
-            # that no other configuration has been requested because
-            # it will not be possible to modify the __ACTIVE__ setting
-            # once it's read-only
-            val = self.env_config.is_non_default()
-            if val is not None:
-                raise Exception("Non-default OMERO_CONFIG on read-only: %s" % val)
-
-            self.source = open(filename, "r")                       #: Open file handle read-only
+        self.open_source()
 
         if self.exclusive:  # must be "a+"
             try:
@@ -140,6 +121,31 @@ def __init__(self, filename, env_config = None, exclusive = True):
             properties = SubElement(self.XML, "properties", id=default)
             _ = SubElement(properties, "property", name=self.KEY, value=self.VERSION)
 
+    def open_source(self):
+        self.source = None
+        if not self.read_only:
+            try:
+                # Try to open the file for modification
+                # If this fails, then the file is readonly
+                self.source = open(self.filename, "a+")                 #: Open file handle
+                self.lock = self._open_lock()                           #: Open file handle for lock
+            except IOError:
+                self.logger.debug("open('%s', 'a+') failed" % self.filename)
+
+                # Before we're forced to open read-only, we need to check
+                # that no other configuration has been requested because
+                # it will not be possible to modify the __ACTIVE__ setting
+                # once it's read-only
+                val = self.env_config.is_non_default()
+                if val is not None:
+                    raise Exception("Non-default OMERO_CONFIG on read-only: %s" % val)
+
+        if self.source is None:
+            self.lock = None
+            self.exclusive = False
+            self.save_on_close = False
+            self.source = open(self.filename, "r")                       #: Open file handle read-only
+
     def _open_lock(self):
         return open("%s.lock" % self.filename, "a+")
 
@@ -267,10 +273,22 @@ def save(self):
         for k, p in prop_list:
             self.clear_text(p)
             icegrid.append(p)
-        self.source.seek(0)
-        self.source.truncate()
-        self.source.write(self.element_to_xml(icegrid))
-        self.source.flush()
+
+        temp_file = path.path(self.filename + ".temp")
+        try:
+            temp_file.write_text(self.element_to_xml(icegrid))
+            temp_file.rename(self.filename)
+            try:
+                self._close_lock()
+            except:
+                self.logger.error("Failed to close lock", exc_info=1)
+            self.open_source()
+        except Exception, e:
+            try:
+                temp_file.remove()
+            except:
+                self.logger.error("Failed to remove temp file")
+            raise e
 
     def close(self):
         try:

components/tools/OmeroPy/src/omero/plugins/user.py

@@ -91,6 +91,9 @@ def _configure(self, parser):
         email.add_argument(
             "-i", "--ignore", action="store_true", default=False,
             help="Ignore users without email addresses")
+        email.add_argument(
+            "--all", action="store_true", default=False,
+            help="Include all users, including deactivated accounts")
 
         joingroup = parser.add(sub, self.joingroup, "Join one or more groups")
         self.add_user_arguments(joingroup)
@@ -143,6 +146,7 @@ def format_name(self, exp):
     def email(self, args):
         c = self.ctx.conn(args)
         a = c.sf.getAdminService()
+        r = a.getSecurityRoles()
 
         skipped = []
         records = []
@@ -154,6 +158,13 @@ def email(self, args):
                     skipped.append(exp)
                 continue
 
+            # Handle deactivated users
+            if not args.all:
+                groups = exp.linkedExperimenterGroupList()
+                group_ids = [x.id.val for x in groups]
+                if r.userGroupId not in group_ids:
+                    continue
+
             record = ""
             if args.names:
                 record += '"%s"' % self.format_name(exp)

components/tools/OmeroPy/test/unit/clitest/test_prefs.py

@@ -23,7 +23,7 @@
 @pytest.fixture
 def configxml(monkeypatch):
     class MockConfigXml(object):
-        def __init__(self, path):
+        def __init__(self, path, **kwargs):
             pass
 
         def as_map(self):

components/tools/OmeroPy/test/unit/test_config.py

@@ -228,6 +228,13 @@ def testReadOnlyConfigSimple(self):
         config = ConfigXml(filename=str(p), env_config=None)  # Must be None
         config.close()  # Shouldn't save
 
+    def testReadOnlyConfigPassesOnExplicitReadOnly(self):
+        p = create_path()
+        p.chmod(0444)  # r--r--r--
+        ConfigXml(filename=str(p),
+                  env_config="default",
+                  read_only=True).close()
+
     def testReadOnlyConfigFailsOnEnv1(self):
         p = create_path()
         p.chmod(0444)  # r--r--r--

components/tools/OmeroWeb/omeroweb/settings.py

@@ -135,7 +135,7 @@
 
 while True:
     try:
-        CONFIG_XML = omero.config.ConfigXml(CONFIG_XML)
+        CONFIG_XML = omero.config.ConfigXml(CONFIG_XML, read_only=True)
         CUSTOM_SETTINGS = CONFIG_XML.as_map()
         CONFIG_XML.close()
         break