Repository: ResearchObject/ro-crate-py
Pull request #45 (2021-02-10 16:57:35)

File fixes
Fixes some problems with the handling of `source` and `dest_path` in `File`.

#### Nonexistent path as source and `dest_path` not `None`:
* **before**: a File entity with `dest_path` as the id was added to the metadata, but no file was added to the output crate upon writing
* **now**: raise `ValueError(f"'{source}' is not a path to a local file or a valid remote URI")`

Note that the `ValueError` is also raised with a nonexistent path as source and `dest_path` set to `None`, but this case was already being handled correctly.

#### `dest_path` set to `None` and source set to a `StringIO` or to `None`:
* **before**: `TypeError` from `os.path.isfile`
* **now**: raise `ValueError("dest_path must be provided if source is not a path or URI")`



Patch:
diff --git a/rocrate/model/file.py b/rocrate/model/file.py
index f430480..8d88885 100644
--- a/rocrate/model/file.py
+++ b/rocrate/model/file.py
@@ -25,9 +25,9 @@ import urllib
 from io import IOBase
 from shutil import copy
 from urllib.error import HTTPError
-from urllib.parse import urlparse
 
 from .data_entity import DataEntity
+from ..utils import is_url
 
 
 class File(DataEntity):
@@ -36,25 +36,27 @@ class File(DataEntity):
                  validate_url=True, properties=None):
         if properties is None:
             properties = {}
-        # process source
         self.fetch_remote = fetch_remote
         self.source = source
+        is_local = is_remote = False
+        if isinstance(source, (str, pathlib.Path)):
+            is_local = os.path.isfile(str(source))
+            is_remote = is_url(str(source))
+            if not is_local and not is_remote:
+                raise ValueError(f"'{source}' is not a path to a local file or a valid remote URI")
+        elif dest_path is None:
+            raise ValueError("dest_path must be provided if source is not a path or URI")
         if dest_path:
             # the entity is refrencing a path relative to the ro-crate root
             identifier = dest_path  # relative path?
         else:
             # if there is no dest_path there must be a URI/local path as source
-            if os.path.isfile(source):
+            if is_local:
                 # local source -> becomes local reference = reference relative
                 # to ro-crate root
                 identifier = os.path.basename(source)
             else:
                 # entity is refering an external object (absolute URI)
-                # first chec that it is a valid remote URI
-                url = urlparse(source)
-                if not all([url.scheme, url.netloc, url.path]):
-                    raise RuntimeError("Source is not a path to a local file"
-                                       " or a valid remote URI")
                 if validate_url:
 
                     # specification says remote URI should always be


Test patch:
diff --git a/test/test_write.py b/test/test_write.py
index fcc6579..5308814 100644
--- a/test/test_write.py
+++ b/test/test_write.py
@@ -18,6 +18,7 @@
 import io
 import pytest
 import sys
+import uuid
 import zipfile
 from urllib.error import URLError
 
@@ -140,10 +141,10 @@ def test_remote_uri(tmpdir, helpers, fetch_remote):
 
 
 @pytest.mark.skipif(sys.platform.startswith("win"), reason="dir mode has no effect on Windows")
-def test_remote_uri_exceptions(tmpdir, helpers):
+def test_remote_uri_exceptions(tmpdir):
     crate = ROCrate()
     url = ('https://raw.githubusercontent.com/ResearchObject/ro-crate-py/'
-           'master/test/test-data/_do_not_create_this_.foo')
+           f'master/test/test-data/{uuid.uuid4().hex}.foo')
     crate.add_file(source=url, fetch_remote=True)
     out_path = tmpdir / 'ro_crate_out_1'
     out_path.mkdir()
@@ -159,3 +160,28 @@ def test_remote_uri_exceptions(tmpdir, helpers):
     (out_path / "a").mkdir(mode=0o444)
     with pytest.raises(PermissionError):
         crate.write_crate(out_path)
+
+
+@pytest.mark.parametrize("fetch_remote,validate_url", [(False, False), (False, True), (True, False), (True, True)])
+def test_missing_source(test_data_dir, fetch_remote, validate_url):
+    crate = ROCrate()
+    path = test_data_dir / uuid.uuid4().hex
+    with pytest.raises(ValueError):
+        crate.add_file(str(path), fetch_remote=fetch_remote, validate_url=validate_url)
+
+    with pytest.raises(ValueError):
+        crate.add_file(str(path), path.name, fetch_remote=fetch_remote, validate_url=validate_url)
+
+
+@pytest.mark.parametrize("fetch_remote,validate_url", [(False, False), (False, True), (True, False), (True, True)])
+def test_stringio_no_dest(test_data_dir, fetch_remote, validate_url):
+    crate = ROCrate()
+    with pytest.raises(ValueError):
+        crate.add_file(io.StringIO("foo"))
+
+
+@pytest.mark.parametrize("fetch_remote,validate_url", [(False, False), (False, True), (True, False), (True, True)])
+def test_no_source_no_dest(test_data_dir, fetch_remote, validate_url):
+    crate = ROCrate()
+    with pytest.raises(ValueError):
+        crate.add_file()