Repository: dandi/dandi-cli

Issue:
dandi-api: improve download functionality
TODOs
- [x] add support for downloading a specific folder (that was disabled)
- [x] replace hardcoded to consider "draft" to be the "latest version" . We should remove that and allow for version to be left unspecified, and then DandiAPIClient code handle it properly (get the latest version, download for it)

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/dandi/dandi-cli/pull/377?src=pr&el=h1) Report
> Merging [#377](https://codecov.io/gh/dandi/dandi-cli/pull/377?src=pr&el=desc) (f3824b0) into [master](https://codecov.io/gh/dandi/dandi-cli/commit/4b277aef9fa9cbf180a4207b0ef24262487dd822?el=desc) (4b277ae) will **decrease** coverage by `18.18%`.
> The diff coverage is `9.09%`.

[![Impacted file tree graph](https://codecov.io/gh/dandi/dandi-cli/pull/377/graphs/tree.svg?width=650&height=150&src=pr&token=cBhomxJ9Ua)](https://codecov.io/gh/dandi/dandi-cli/pull/377?src=pr&el=tree)

```diff
@@             Coverage Diff             @@
##           master     #377       +/-   ##
===========================================
- Coverage   83.05%   64.86%   -18.19%     
===========================================
  Files          55       55               
  Lines        5606     5547       -59     
===========================================
- Hits         4656     3598     -1058     
- Misses        950     1949      +999     
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests | `64.86% <9.09%> (-18.19%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/dandi/dandi-cli/pull/377?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [dandi/dandiarchive.py](https://codecov.io/gh/dandi/dandi-cli/pull/377/diff?src=pr&el=tree#diff-ZGFuZGkvZGFuZGlhcmNoaXZlLnB5) | `70.74% <9.09%> (-7.83%)` | :arrow_down: |
| [dandi/tests/test\_dandiapi.py](https://codecov.io/gh/dandi/dandi-cli/pull/377/diff?src=pr&el=tree#diff-ZGFuZGkvdGVzdHMvdGVzdF9kYW5kaWFwaS5weQ==) | `12.50% <0.00%> (-87.50%)` | :arrow_down: |
| [dandi/tests/test\_upload.py](https://codecov.io/gh/dandi/dandi-cli/pull/377/diff?src=pr&el=tree#diff-ZGFuZGkvdGVzdHMvdGVzdF91cGxvYWQucHk=) | `13.96% <0.00%> (-79.34%)` | :arrow_down: |
| [dandi/upload.py](https://codecov.io/gh/dandi/dandi-cli/pull/377/diff?src=pr&el=tree#diff-ZGFuZGkvdXBsb2FkLnB5) | `2.75% <0.00%> (-73.25%)` | :arrow_down: |
| [dandi/dandiapi.py](https://codecov.io/gh/dandi/dandi-cli/pull/377/diff?src=pr&el=tree#diff-ZGFuZGkvZGFuZGlhcGkucHk=) | `20.37% <0.00%> (-66.42%)` | :arrow_down: |
| [dandi/support/generatorify.py](https://codecov.io/gh/dandi/dandi-cli/pull/377/diff?src=pr&el=tree#diff-ZGFuZGkvc3VwcG9ydC9nZW5lcmF0b3JpZnkucHk=) | `0.00% <0.00%> (-64.37%)` | :arrow_down: |
| [dandi/tests/test\_register.py](https://codecov.io/gh/dandi/dandi-cli/pull/377/diff?src=pr&el=tree#diff-ZGFuZGkvdGVzdHMvdGVzdF9yZWdpc3Rlci5weQ==) | `37.50% <0.00%> (-62.50%)` | :arrow_down: |
| [dandi/register.py](https://codecov.io/gh/dandi/dandi-cli/pull/377/diff?src=pr&el=tree#diff-ZGFuZGkvcmVnaXN0ZXIucHk=) | `30.76% <0.00%> (-42.31%)` | :arrow_down: |
| [dandi/tests/fixtures.py](https://codecov.io/gh/dandi/dandi-cli/pull/377/diff?src=pr&el=tree#diff-ZGFuZGkvdGVzdHMvZml4dHVyZXMucHk=) | `55.14% <0.00%> (-40.78%)` | :arrow_down: |
| [dandi/girder.py](https://codecov.io/gh/dandi/dandi-cli/pull/377/diff?src=pr&el=tree#diff-ZGFuZGkvZ2lyZGVyLnB5) | `61.16% <0.00%> (-23.10%)` | :arrow_down: |
| ... and [25 more](https://codecov.io/gh/dandi/dandi-cli/pull/377/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/dandi/dandi-cli/pull/377?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/dandi/dandi-cli/pull/377?src=pr&el=footer). Last update [4b277ae...f3824b0](https://codecov.io/gh/dandi/dandi-cli/pull/377?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/dandi/dandiarchive.py b/dandi/dandiarchive.py
index 854a1547..8cb0c4eb 100644
--- a/dandi/dandiarchive.py
+++ b/dandi/dandiarchive.py
@@ -46,25 +46,34 @@ def navigate_url(url):
         args = asset_id, asset_type
     elif server_type == "api":
         client = DandiAPIClient(server_url)
+        if asset_id["version"] is None:
+            r = client.get(f"/dandisets/{asset_id['dandiset_id']}/")
+            asset_id["version"] = r["most_recent_version"]["version"]
         args = (asset_id["dandiset_id"], asset_id["version"])
         kwargs["include_metadata"] = True
         if asset_id.get("location"):
-            # API interface was RFed in 6ba45daf7c00d6cbffd33aed91a984ad28419f56
-            # and no longer takes "location" kwarg. There is `get_dandiset_assets`
-            # but it doesn't provide dandiset... review the use of navigate_url
-            # to see if we should keep its interface as is and provide dandiset...
-            raise NotImplementedError(
-                "No support for path specific handling via API yet"
-            )
+            with client.session():
+                dandiset = client.get_dandiset(*args)
+                if asset_type == "folder":
+                    assets = client.get_dandiset_assets(
+                        *args, path=asset_id["location"]
+                    )
+                elif asset_type == "item":
+                    asset = client.get_asset_bypath(*args, asset_id["location"])
+                    assets = [asset] if asset is not None else []
+                else:
+                    raise NotImplementedError(
+                        f"Do not know how to handle asset type {asset_type} with location"
+                    )
+                yield (client, dandiset, assets)
+            return
     else:
         raise NotImplementedError(
             f"Download from server of type {server_type} is not yet implemented"
         )
 
     with client.session():
-        dandiset, assets = client.get_dandiset_and_assets(
-            *args, **kwargs
-        )  # , recursive=recursive)
+        dandiset, assets = client.get_dandiset_and_assets(*args, **kwargs)
         yield client, dandiset, assets
 
 
@@ -128,7 +137,8 @@ class _dandi_url_parser:
         # https://gui.dandiarchive.org/#/dandiset/000001/0.201104.2302/files
         # TODO: upload something to any dandiset to see what happens when there are files
         # and adjust for how path is provided (if not ?location=)
-        f"(?P<server>(?P<protocol>https?)://(?P<hostname>gui-beta-dandiarchive-org.netlify.app)/)"
+        "(?P<server>(?P<protocol>https?)://"
+        "(?P<hostname>gui-beta-dandiarchive-org\\.netlify\\.app)/)"
         f"#/(?P<asset_type>dandiset)/{dandiset_id_grp}"
         "(/(?P<version>([.0-9]{5,}|draft)))?"
         f"(/files(\\?location=(?P<location>.*)?)?)?"
@@ -150,6 +160,14 @@ class _dandi_url_parser:
         "(/(?P<version>draft))?"
         f"(/files(\\?_id={id_grp}(&_modelType=folder)?)?)?"
         "$": {"server_type": "girder"},
+        # ad-hoc explicitly pointing within URL to the specific instance to use
+        # and otherwise really simple: dandi://INSTANCE/DANDISET_ID[@VERSION][/PATH]
+        # For now to not be promoted to the users, and primarily for internal use
+        f"dandi://(?P<instance_name>({'|'.join(known_instances)}))"
+        f"/{dandiset_id_grp}"
+        "(@(?P<version>([.0-9]{5,}|draft)))?"
+        f"(/(?P<location>.*)?)?"
+        "$": {"server_type": "api"},
         # https://deploy-preview-341--gui-dandiarchive-org.netlify.app/#/dandiset/000006/draft
         # (no API yet)
         "https?://.*": {"handle_redirect": "only"},
@@ -215,6 +233,8 @@ class _dandi_url_parser:
             match = re.match(regex, url)
             if not match:
                 continue
+            groups = match.groupdict()
+            lgr.log(5, "Matched %r into %s", url, groups)
             rewrite = settings.get("rewrite", False)
             handle_redirect = settings.get("handle_redirect", False)
             if rewrite:
@@ -253,21 +273,27 @@ class _dandi_url_parser:
                         server += "/"
                     return (server_type, server) + tuple(_)
                 continue  # in this run we ignore an match further
+            elif "instance_name" in groups:
+                known_instance = get_instance(groups["instance_name"])
+                server_type = "girder" if known_instance.girder else "api"
+                assert known_instance.api  # must be defined
+                groups["server"] = known_instance.api
+                # could be overloaded later depending if location is provided
+                groups["asset_type"] = "dandiset"
+                break
             else:
                 server_type = settings.get("server_type", "girder")
                 break
-
         if not match:
             known_regexes = "\n - ".join([""] + list(cls.known_urls))
             # TODO: may be make use of etelemetry and report if newer client
             # which might know is available?
             raise UnknownURLError(
-                f"We do not know how to map URL {url} to girder.\n"
+                f"We do not know how to map URL {url} to our servers.\n"
                 f"Regular expressions for known setups:"
                 f"{known_regexes}"
             )
 
-        groups = match.groupdict()
         url_server = groups["server"]
         server = cls.map_to[server_type].get(url_server.rstrip("/"), url_server)
 
@@ -285,12 +311,8 @@ class _dandi_url_parser:
             location = location.lstrip("/")
         if not (asset_type == "dandiset" and dandiset_id):
             raise ValueError(f"{url} does not point to a dandiset")
-        if not version:
+        if server_type == "girder" and not version:
             version = "draft"
-            # TODO: verify since web UI might have different opinion: it does show e.g.
-            # released version for 000001 now, but that one could be produced only from draft
-            # so we should be ok.  Otherwise we should always then query "dandiset_read" endpoint
-            # to figure out what is the "most recent one"
         # Let's just return a structured record for the requested asset
         asset_ids = {"dandiset_id": dandiset_id, "version": version}
         # if location is not degenerate -- it would be a folder or a file
diff --git a/dandi/download.py b/dandi/download.py
index 781655ef..4e74a909 100644
--- a/dandi/download.py
+++ b/dandi/download.py
@@ -13,7 +13,7 @@ import requests
 
 from .consts import dandiset_metadata_file, metadata_digests
 from .dandiapi import DandiAPIClient
-from .dandiarchive import navigate_url
+from .dandiarchive import navigate_url, parse_dandi_url
 from .dandiset import Dandiset
 from . import get_logger, girder
 from .support.pyout import naturalsize
@@ -119,6 +119,7 @@ def download_generator(
         # on which instance it exists!  Thus ATM we would do nothing but crash
         raise NotImplementedError("No URLs were provided.  Cannot download anything")
 
+    server_type, _, asset_type, asset_id = parse_dandi_url(urls[0])
     with navigate_url(urls[0]) as (client, dandiset, assets):
         if assets_it:
             assets_it.gen = assets
@@ -130,13 +131,17 @@ def download_generator(
             identifier = Dandiset._get_identifier(dandiset)
             if not identifier:
                 raise ValueError(f"Cannot deduce dandiset identifier from {dandiset}")
-            output_path = op.join(output_dir, identifier)
-            if get_metadata:
-                dandiset_metadata = dandiset.get("metadata", {})
-                for resp in _populate_dandiset_yaml(
-                    output_path, dandiset_metadata, existing == "overwrite"
-                ):
-                    yield dict(path=dandiset_metadata_file, **resp)
+            if server_type == "girder" or asset_type == "dandiset":
+                output_path = op.join(output_dir, identifier)
+                if get_metadata:
+                    dandiset_metadata = dandiset.get("metadata", {})
+                    for resp in _populate_dandiset_yaml(
+                        output_path, dandiset_metadata, existing == "overwrite"
+                    ):
+                        yield dict(path=dandiset_metadata_file, **resp)
+            else:
+                output_path = output_dir
+
         else:
             output_path = output_dir
 
@@ -185,6 +190,16 @@ def download_generator(
 
             path = asset["path"].lstrip("/")  # make into relative path
             path = op.normpath(path)
+            if server_type == "api" and asset_type != "dandiset":
+                if asset_type == "folder":
+                    folder_path = op.normpath(asset_id["location"])
+                    path = op.join(
+                        op.basename(folder_path), op.relpath(path, folder_path)
+                    )
+                elif asset_type == "item":
+                    path = op.basename(path)
+                else:
+                    raise NotImplementedError(f"Unexpected asset type {asset_type}")
             download_path = op.join(output_path, path)
 
             downloader = client.get_download_file_iter(*down_args)


Test patch:
diff --git a/dandi/tests/test_dandiapi.py b/dandi/tests/test_dandiapi.py
index ea6148de..1e410792 100644
--- a/dandi/tests/test_dandiapi.py
+++ b/dandi/tests/test_dandiapi.py
@@ -1,11 +1,12 @@
-from collections import deque
 import os.path
+from pathlib import Path
 from shutil import rmtree
 
 from ..consts import dandiset_metadata_file
 from ..dandiapi import DandiAPIClient
 from ..download import download
 from ..upload import upload
+from ..utils import find_files
 
 
 def test_upload(local_dandi_api, simple1_nwb, tmp_path):
@@ -50,14 +51,7 @@ def test_publish_and_manipulate(local_dandi_api, mocker, monkeypatch, tmp_path):
     download_dir.mkdir()
 
     def downloaded_files():
-        dirs = deque([download_dir])
-        while dirs:
-            d = dirs.popleft()
-            for p in d.iterdir():
-                if p.is_dir():
-                    dirs.append(p)
-                else:
-                    yield p
+        return list(map(Path, find_files(r".*", paths=[download_dir])))
 
     dandiset_yaml = download_dir / dandiset_id / dandiset_metadata_file
     file_in_version = download_dir / dandiset_id / "subdir" / "file.txt"
@@ -66,7 +60,7 @@ def test_publish_and_manipulate(local_dandi_api, mocker, monkeypatch, tmp_path):
         f"{local_dandi_api['instance'].api}/dandisets/{dandiset_id}/versions/{version_id}",
         download_dir,
     )
-    assert sorted(downloaded_files()) == [dandiset_yaml, file_in_version]
+    assert downloaded_files() == [dandiset_yaml, file_in_version]
     assert file_in_version.read_text() == "This is test text.\n"
 
     (upload_dir / "subdir" / "file.txt").write_text("This is different text.\n")
@@ -82,7 +76,7 @@ def test_publish_and_manipulate(local_dandi_api, mocker, monkeypatch, tmp_path):
         f"{local_dandi_api['instance'].api}/dandisets/{dandiset_id}/versions/{version_id}",
         download_dir,
     )
-    assert sorted(downloaded_files()) == [dandiset_yaml, file_in_version]
+    assert downloaded_files() == [dandiset_yaml, file_in_version]
     assert file_in_version.read_text() == "This is test text.\n"
 
     (upload_dir / "subdir" / "file2.txt").write_text("This is more text.\n")
@@ -112,7 +106,7 @@ def test_publish_and_manipulate(local_dandi_api, mocker, monkeypatch, tmp_path):
         f"{local_dandi_api['instance'].api}/dandisets/{dandiset_id}/versions/{version_id}",
         download_dir,
     )
-    assert sorted(downloaded_files()) == [dandiset_yaml, file_in_version]
+    assert downloaded_files() == [dandiset_yaml, file_in_version]
     assert file_in_version.read_text() == "This is test text.\n"
 
     client.delete_asset_bypath(dandiset_id, "draft", "subdir/file.txt")
@@ -122,10 +116,7 @@ def test_publish_and_manipulate(local_dandi_api, mocker, monkeypatch, tmp_path):
         f"{local_dandi_api['instance'].api}/dandisets/{dandiset_id}/versions/draft",
         download_dir,
     )
-    assert sorted(downloaded_files()) == [
-        dandiset_yaml,
-        file_in_version.with_name("file2.txt"),
-    ]
+    assert downloaded_files() == [dandiset_yaml, file_in_version.with_name("file2.txt")]
     assert file_in_version.with_name("file2.txt").read_text() == "This is more text.\n"
 
     rmtree(download_dir / dandiset_id)
@@ -133,7 +124,7 @@ def test_publish_and_manipulate(local_dandi_api, mocker, monkeypatch, tmp_path):
         f"{local_dandi_api['instance'].api}/dandisets/{dandiset_id}/versions/{version_id}",
         download_dir,
     )
-    assert sorted(downloaded_files()) == [dandiset_yaml, file_in_version]
+    assert downloaded_files() == [dandiset_yaml, file_in_version]
     assert file_in_version.read_text() == "This is test text.\n"
 
 
diff --git a/dandi/tests/test_dandiarchive.py b/dandi/tests/test_dandiarchive.py
index 17b99bf9..6c760e6d 100644
--- a/dandi/tests/test_dandiarchive.py
+++ b/dandi/tests/test_dandiarchive.py
@@ -6,98 +6,148 @@ from dandi.exceptions import NotFoundError
 from dandi.tests.skip import mark
 
 
-def _assert_parse_girder_url(url):
+@pytest.mark.parametrize(
+    "url,asset_type,asset_id",
+    [
+        # ATM we point to drafts, so girder
+        ("DANDI:000027", "dandiset", {"dandiset_id": "000027", "version": "draft"}),
+        # Example of current web ui (with girder backend) as of 20210119
+        (
+            "https://gui.dandiarchive.org/#/dandiset/000003/draft/"
+            "files?_id=5e74ee41368d3c79a8006d29&_modelType=folder",
+            "folder",
+            {
+                "dandiset_id": "000003",
+                "version": "draft",
+                "folder_id": "5e74ee41368d3c79a8006d29",
+            },
+        ),
+        pytest.param(
+            "https://dandiarchive.org/dandiset/000003",
+            "dandiset",
+            {"dandiset_id": "000003", "version": "draft"},
+            marks=mark.skipif_no_network,
+        ),
+    ],
+)
+def test_parse_girder_url(url, asset_type, asset_id):
     st, s, a, aid = parse_dandi_url(url)
     assert st == "girder"
     assert s == known_instances["dandi"].girder + "/"
-    return a, aid
+    assert a == asset_type
+    assert aid == asset_id
 
 
-def _assert_parse_api_url(url, instance="dandi-api"):
+@pytest.mark.parametrize(
+    "url,instance,asset_type,asset_id",
+    [
+        # New DANDI web UI driven by DANDI API.
+        (
+            "https://gui-beta-dandiarchive-org.netlify.app/#/dandiset/000001",
+            "dandi-api",
+            "dandiset",
+            {"dandiset_id": "000001", "version": None},
+        ),
+        (
+            "https://gui-beta-dandiarchive-org.netlify.app/#/dandiset/000001/0.201104.2302",
+            "dandi-api",
+            "dandiset",
+            {"dandiset_id": "000001", "version": "0.201104.2302"},
+        ),
+        (
+            "https://gui-beta-dandiarchive-org.netlify.app/#/dandiset/000001/0.201104.2302/files",
+            "dandi-api",
+            "dandiset",
+            {"dandiset_id": "000001", "version": "0.201104.2302"},
+        ),
+        (
+            "http://localhost:8000/api/dandisets/000002/versions/draft",
+            "dandi-api-local-docker-tests",
+            "dandiset",
+            {"dandiset_id": "000002", "version": "draft"},
+        ),
+        (
+            "https://gui-beta-dandiarchive-org.netlify.app/#/dandiset/000001/"
+            "files?location=%2Fsub-anm369962",
+            "dandi-api",
+            "item",
+            {"dandiset_id": "000001", "version": None, "location": "sub-anm369962"},
+        ),
+        (
+            "https://gui-beta-dandiarchive-org.netlify.app/#/dandiset/000006/"
+            "0.200714.1807/files?location=%2Fsub-anm369962",
+            "dandi-api",
+            "item",
+            {
+                "dandiset_id": "000006",
+                "version": "0.200714.1807",
+                "location": "sub-anm369962",
+            },
+        ),
+        (
+            "https://gui-beta-dandiarchive-org.netlify.app/#/dandiset/001001/"
+            "draft/files?location=sub-RAT123%2F",
+            "dandi-api",
+            "folder",
+            {"dandiset_id": "001001", "version": "draft", "location": "sub-RAT123/"},
+        ),
+        # by direct instance name ad-hoc URI instance:ID[@version][/path]
+        (
+            "dandi://dandi-api-local-docker-tests/000002@draft",
+            "dandi-api-local-docker-tests",
+            "dandiset",
+            {"dandiset_id": "000002", "version": "draft"},
+        ),
+        (
+            "dandi://dandi-api-local-docker-tests/000002@draft/path",
+            "dandi-api-local-docker-tests",
+            "item",
+            {"dandiset_id": "000002", "location": "path", "version": "draft"},
+        ),
+        (
+            "dandi://dandi-api-local-docker-tests/000002/path",
+            "dandi-api-local-docker-tests",
+            "item",
+            {"dandiset_id": "000002", "location": "path", "version": None},
+        ),
+        (  # test on "public" instance and have trailing / to signal the folder
+            "dandi://dandi-api/000002/path/",
+            "dandi-api",
+            "folder",
+            {"dandiset_id": "000002", "location": "path/", "version": None},
+        ),
+        # TODO: bring back a test on deploy-preview-
+        # # And the hybrid for "drafts" where it still goes by girder ID
+        # (
+        #     "https://deploy-preview-341--gui-dandiarchive-org.netlify.app/#/dandiset/000027"
+        #     "/draft/files?_id=5f176583f63d62e1dbd06943&_modelType=folder",
+        #     "dandi-api",
+        #     "folder",
+        #     {
+        #         "dandiset_id": "000027",
+        #         "version": "draft",
+        #         "folder_id": "5f176583f63d62e1dbd06943",
+        #     },
+        # )
+    ],
+)
+def test_parse_api_url(url, instance, asset_type, asset_id):
     st, s, a, aid = parse_dandi_url(url)
     assert st == "api"
     assert s == known_instances[instance].api + "/"
-    return a, aid
-
-
-def test_parse_dandi_url():
-    # ATM we point to drafts, so girder
-    assert _assert_parse_girder_url("DANDI:000027") == (
-        "dandiset",
-        {"dandiset_id": "000027", "version": "draft"},
-    )
-
-    # Example of current web ui (with girder backend) as of 20210119
-    assert _assert_parse_girder_url(
-        "https://gui.dandiarchive.org/#/dandiset/000003/draft/"
-        "files?_id=5e74ee41368d3c79a8006d29&_modelType=folder"
-    ) == (
-        "folder",
-        {
-            "dandiset_id": "000003",
-            "version": "draft",
-            "folder_id": "5e74ee41368d3c79a8006d29",
-        },
-    )
-
-    # New DANDI web UI driven by DANDI API.
-    url1 = "https://gui-beta-dandiarchive-org.netlify.app/#/dandiset/000001"
-    assert _assert_parse_api_url(url1) == (
-        "dandiset",
-        # TODO: in -cli we assume draft, but web ui might be taking the "latest"
-        {"dandiset_id": "000001", "version": "draft"},
-    )
-    assert _assert_parse_api_url(url1 + "/0.201104.2302") == (
-        "dandiset",
-        {"dandiset_id": "000001", "version": "0.201104.2302"},
-    )
-    assert _assert_parse_api_url(url1 + "/0.201104.2302/files") == (
-        "dandiset",
-        {"dandiset_id": "000001", "version": "0.201104.2302"},
-    )
-
-    assert _assert_parse_api_url(
-        "http://localhost:8000/api/dandisets/000002/versions/draft",
-        instance="dandi-api-local-docker-tests",
-    ) == ("dandiset", {"dandiset_id": "000002", "version": "draft"})
-    # TODO: bring it inline with how it would look whenever there is a folder
-    # ATM there is not a single dataset with a folder to see how it looks
-    # no trailing / - Yarik considers it to be an item (file)
-    # assert _assert_parse_api_url(url1 + "/files?location=%2Fsub-anm369962") == (
-    #     "item",
-    #     {
-    #         "dandiset_id": "000006",
-    #         "version": "0.200714.1807",
-    #         "location": "sub-anm369962",
-    #     },
-    # )
-
-    # TODO: bring back a test on deploy-preview-
-    # # And the hybrid for "drafts" where it still goes by girder ID
-    # assert _assert_parse_api_url(
-    #     "https://deploy-preview-341--gui-dandiarchive-org.netlify.app/#/dandiset/000027"
-    #     "/draft/files?_id=5f176583f63d62e1dbd06943&_modelType=folder"
-    # ) == (
-    #     "folder",
-    #     {
-    #         "dandiset_id": "000027",
-    #         "version": "draft",
-    #         "folder_id": "5f176583f63d62e1dbd06943",
-    #     },
-    # )
+    assert a == asset_type
+    assert aid == asset_id
 
 
 @mark.skipif_no_network
-def test_parse_dandi_url_redirect():
+def test_parse_dandi_url_not_found():
     # Unlikely this one would ever come to existence
     with pytest.raises(NotFoundError):
         parse_dandi_url("https://dandiarchive.org/dandiset/999999")
-    # Is there ATM
-    assert _assert_parse_girder_url("https://dandiarchive.org/dandiset/000003") == (
-        "dandiset",
-        {"dandiset_id": "000003", "version": "draft"},
-    )
-    # And this one would point to a folder
+
+
+@mark.skipif_no_network
+def test_follow_redirect():
     assert (
         follow_redirect("https://bit.ly/dandi12")
         == "https://gui.dandiarchive.org/#/file-browser/folder/5e72b6ac3da50caa9adb0498"
diff --git a/dandi/tests/test_download.py b/dandi/tests/test_download.py
index 5bd2fdbd..8e2f19ad 100644
--- a/dandi/tests/test_download.py
+++ b/dandi/tests/test_download.py
@@ -1,12 +1,18 @@
 import json
 import os
 import os.path as op
+from pathlib import Path
+from shutil import rmtree
 
 import pytest
 import tqdm
 
+from ..consts import dandiset_metadata_file
+from ..dandiapi import DandiAPIClient
 from ..download import download
 from ..girder import TQDMProgressReporter
+from ..upload import upload
+from ..utils import find_files
 
 
 # both urls point to 000027 (lean test dataset), and both draft and "released"
@@ -135,3 +141,111 @@ def test_download_000027_resume(tmp_path, resizer, version):
     assert sorted(contents) == ["sub-RAT123", op.join("sub-RAT123", "sub-RAT123.nwb")]
     assert nwb.stat().st_size == size
     assert digester(str(nwb)) == digests
+
+
+def test_download_newest_version(local_dandi_api, monkeypatch, tmp_path):
+    client = DandiAPIClient(
+        api_url=local_dandi_api["instance"].api, token=local_dandi_api["api_key"]
+    )
+    dandiset_id = client.create_dandiset("Test Dandiset", {})["identifier"]
+    upload_dir = tmp_path / "upload"
+    upload_dir.mkdir()
+    (upload_dir / dandiset_metadata_file).write_text(f"identifier: '{dandiset_id}'\n")
+    (upload_dir / "file.txt").write_text("This is test text.\n")
+    monkeypatch.chdir(upload_dir)
+    monkeypatch.setenv("DANDI_API_KEY", local_dandi_api["api_key"])
+    upload(
+        paths=[],
+        dandi_instance=local_dandi_api["instance_id"],
+        devel_debug=True,
+        allow_any_path=True,
+        validation="skip",
+    )
+    download_dir = tmp_path / "download"
+    download_dir.mkdir()
+    download(f"{local_dandi_api['instance'].api}/dandisets/{dandiset_id}", download_dir)
+    assert (
+        download_dir / dandiset_id / "file.txt"
+    ).read_text() == "This is test text.\n"
+    client.publish_version(dandiset_id, "draft")
+    (upload_dir / "file.txt").write_text("This is different text.\n")
+    upload(
+        paths=[],
+        dandi_instance=local_dandi_api["instance_id"],
+        devel_debug=True,
+        allow_any_path=True,
+        validation="skip",
+    )
+    rmtree(download_dir / dandiset_id)
+    download(f"{local_dandi_api['instance'].api}/dandisets/{dandiset_id}", download_dir)
+    assert (
+        download_dir / dandiset_id / "file.txt"
+    ).read_text() == "This is test text.\n"
+
+
+def test_download_folder(local_dandi_api, monkeypatch, tmp_path):
+    client = DandiAPIClient(
+        api_url=local_dandi_api["instance"].api, token=local_dandi_api["api_key"]
+    )
+    dandiset_id = client.create_dandiset("Test Dandiset", {})["identifier"]
+    upload_dir = tmp_path / "upload"
+    upload_dir.mkdir()
+    (upload_dir / dandiset_metadata_file).write_text(f"identifier: '{dandiset_id}'\n")
+    (upload_dir / "subdir1").mkdir()
+    (upload_dir / "subdir1" / "file.txt").write_text("This is test text.\n")
+    (upload_dir / "subdir2").mkdir()
+    (upload_dir / "subdir2" / "apple.txt").write_text("Apple\n")
+    (upload_dir / "subdir2" / "banana.txt").write_text("Banana\n")
+    monkeypatch.chdir(upload_dir)
+    monkeypatch.setenv("DANDI_API_KEY", local_dandi_api["api_key"])
+    upload(
+        paths=[],
+        dandi_instance=local_dandi_api["instance_id"],
+        devel_debug=True,
+        allow_any_path=True,
+        validation="skip",
+    )
+    download_dir = tmp_path / "download"
+    download_dir.mkdir()
+    download(
+        f"dandi://{local_dandi_api['instance_id']}/{dandiset_id}/subdir2/", download_dir
+    )
+    assert sorted(map(Path, find_files(r".*", paths=[download_dir], dirs=True))) == [
+        download_dir / "subdir2",
+        download_dir / "subdir2" / "apple.txt",
+        download_dir / "subdir2" / "banana.txt",
+    ]
+    assert (download_dir / "subdir2" / "apple.txt").read_text() == "Apple\n"
+    assert (download_dir / "subdir2" / "banana.txt").read_text() == "Banana\n"
+
+
+def test_download_item(local_dandi_api, monkeypatch, tmp_path):
+    client = DandiAPIClient(
+        api_url=local_dandi_api["instance"].api, token=local_dandi_api["api_key"]
+    )
+    dandiset_id = client.create_dandiset("Test Dandiset", {})["identifier"]
+    upload_dir = tmp_path / "upload"
+    upload_dir.mkdir()
+    (upload_dir / dandiset_metadata_file).write_text(f"identifier: '{dandiset_id}'\n")
+    (upload_dir / "subdir").mkdir()
+    (upload_dir / "subdir" / "apple.txt").write_text("Apple\n")
+    (upload_dir / "subdir" / "banana.txt").write_text("Banana\n")
+    monkeypatch.chdir(upload_dir)
+    monkeypatch.setenv("DANDI_API_KEY", local_dandi_api["api_key"])
+    upload(
+        paths=[],
+        dandi_instance=local_dandi_api["instance_id"],
+        devel_debug=True,
+        allow_any_path=True,
+        validation="skip",
+    )
+    download_dir = tmp_path / "download"
+    download_dir.mkdir()
+    download(
+        f"dandi://{local_dandi_api['instance_id']}/{dandiset_id}/subdir/banana.txt",
+        download_dir,
+    )
+    assert list(map(Path, find_files(r".*", paths=[download_dir], dirs=True))) == [
+        download_dir / "banana.txt"
+    ]
+    assert (download_dir / "banana.txt").read_text() == "Banana\n"