[#681,#714] when opening data object results in error, do not leave a local file.

Author: d-w-moore

irods/helpers/__init__.py

@@ -1,11 +1,18 @@
 import contextlib
 import sys
 from irods import env_filename_from_keyword_args
+import irods.exception as ex
 from irods.message import ET, XML_Parser_Type, IRODS_VERSION
+from irods.path import iRODSPath
 from irods.session import iRODSSession
 
-
-__all__ = ["make_session", "home_collection", "xml_mode"]
+__all__ = [
+    "make_session",
+    "home_collection",
+    "xml_mode",
+    "get_collection",
+    "get_data_object",
+]
 
 
 class StopTestsException(Exception):
@@ -104,3 +111,33 @@ def temporarily_assign_attribute(
             setattr(target, attr, save)
         else:
             delattr(target, attr)
+
+
+def get_data_object(sess, logical_path):
+    """Get a reference to the data object (as an iRODSDataObject) at the given path, if one is found.
+    Else, return None.
+
+    Parameters:
+        sess: an authenticated session object.
+        logical_path: the full logical path where the data object is to be found.  Can be in unnormalized form.
+    """
+    try:
+        # Check for a data object at the normalized path.
+        return sess.data_objects.get(iRODSPath(logical_path))
+    except ex.DataObjectDoesNotExist:
+        return None
+
+
+def get_collection(sess, logical_path):
+    """Get a reference to the collection (as an iRODSCollection) at the given path, if one is found.
+    Else, return None.
+
+    Parameters:
+        sess: an authenticated session object.
+        logical_path: the full logical path where the collection is to be found.  Can be in unnormalized form.
+    """
+    try:
+        # Path normalization is internal to this call.
+        return sess.collections.get(logical_path)
+    except ex.CollectionDoesNotExist:
+        return None

irods/manager/data_object_manager.py

@@ -237,24 +237,23 @@ def _download(self, obj, local_path, num_threads, updatables=(), **options):
             raise ex.OVERWRITE_WITHOUT_FORCE_FLAG
 
         data_open_returned_values_ = {}
-        with open(local_file, "wb") as f:
-            with self.open(
-                obj, "r", returned_values=data_open_returned_values_, **options
-            ) as o:
-                if self.should_parallelize_transfer(
-                    num_threads, o, open_options=options.items()
+        with self.open(
+            obj, "r", returned_values=data_open_returned_values_, **options
+        ) as o:
+            if self.should_parallelize_transfer(
+                num_threads, o, open_options=options.items()
+            ):
+                if not self.parallel_get(
+                    (obj, o),
+                    local_file,
+                    num_threads=num_threads,
+                    target_resource_name=options.get(kw.RESC_NAME_KW, ""),
+                    data_open_returned_values=data_open_returned_values_,
+                    updatables=updatables,
                 ):
-                    f.close()
-                    if not self.parallel_get(
-                        (obj, o),
-                        local_file,
-                        num_threads=num_threads,
-                        target_resource_name=options.get(kw.RESC_NAME_KW, ""),
-                        data_open_returned_values=data_open_returned_values_,
-                        updatables=updatables,
-                    ):
-                        raise RuntimeError("parallel get failed")
-                else:
+                    raise RuntimeError("parallel get failed")
+            else:
+                with open(local_file, "wb") as f:
                     for chunk in chunks(o, self.READ_BUFFER_SIZE):
                         f.write(chunk)
                         do_progress_updates(updatables, len(chunk))

irods/test/data_obj_test.py

@@ -2955,6 +2955,60 @@ def test_replica_truncate__issue_534(self):
                     if data_objs.exists(data_path):
                         data_objs.unlink(data_path, force=True)
 
+    def test_get_collection_returns_a_reference_exclusively_to_an_existing_collection__issue_734(
+        self,
+    ):
+        from irods.helpers import get_collection, get_data_object
+
+        sess = self.sess
+        logical_path = "{}/{}".format(
+            self.coll_path, unique_name(my_function_name(), datetime.now())
+        )
+        sess.collections.create(logical_path)
+
+        self.assertIs(get_data_object(sess, logical_path), None)
+        self.assertTrue(get_collection(sess, logical_path))
+
+    def test_get_data_object_returns_a_reference_exclusively_to_an_existing_data_object__issue_734(
+        self,
+    ):
+        from irods.helpers import get_collection, get_data_object
+
+        sess = self.sess
+        logical_path = "{}/{}".format(
+            self.coll_path, unique_name(my_function_name(), datetime.now())
+        )
+        sess.data_objects.create(logical_path)
+
+        self.assertIs(get_collection(sess, logical_path), None)
+        self.assertTrue(get_data_object(sess, logical_path))
+
+    def test_data_object_download_error_leaves_no_file_relic__issue_681(self):
+        from irods.helpers import get_collection, get_data_object
+
+        sess = self.sess
+
+        # Generate a test path.
+        data_path = "{}/{}".format(
+            self.coll_path, unique_name(my_function_name(), datetime.now())
+        )
+
+        # Test that neither a data object nor a collection exists at the data_path.
+        self.assertIs(None, get_data_object(sess, data_path))
+        self.assertIs(None, get_collection(sess, data_path))
+
+        # Make sure that no directory or file exists at the download target path in the local filesystem.
+        with NamedTemporaryFile(delete=True) as f:
+            local_path = f.name
+        self.assertFalse(os.path.exists(local_path))
+
+        # Attempt downloading the nonexisting object, expecting an error.
+        with self.assertRaises((ex.DataObjectDoesNotExist, ex.OBJ_PATH_DOES_NOT_EXIST)):
+            sess.data_objects.get(data_path, local_path)
+
+        # Assert no relic left at local_path.
+        self.assertFalse(os.path.exists(local_path))
+
 
 if __name__ == "__main__":
     # let the tests find the parent irods lib