Merge pull request #1305 from dandi/enh-location-folder

`?location` parameter in URLs can only point to a folder

dandi/dandiarchive.py

@@ -571,7 +571,7 @@ class _dandi_url_parser:
             re.compile(
                 rf"{server_grp}(#/)?(?P<asset_type>dandiset)/{dandiset_id_grp}"
                 rf"(/(?P<version>{VERSION_REGEX}))?"
-                r"(/(files(\?location=(?P<location>.*)?)?)?)?"
+                r"(/(files(\?location=(?P<location_folder>.*)?)?)?)?"
             ),
             {},
             "https://<server>[/api]/[#/]dandiset/<dandiset id>[/<version>]"
@@ -694,12 +694,12 @@ def parse(
                 assert not handle_redirect
                 assert not settings.get("map_instance")
                 new_url = rewrite(url)
-                return cls.parse(new_url)
+                return cls.parse(new_url, map_instance=map_instance, glob=glob)
             elif handle_redirect:
                 assert handle_redirect in ("pass", "only")
                 new_url = cls.follow_redirect(url)
                 if new_url != url:
-                    return cls.parse(new_url)
+                    return cls.parse(new_url, map_instance=map_instance, glob=glob)
                 if handle_redirect == "pass":
                     # We used to issue warning in such cases, but may be it got implemented
                     # now via reverse proxy and we had added a new regex? let's just
@@ -712,7 +712,7 @@ def parse(
                     )
             elif settings.get("map_instance"):
                 if map_instance:
-                    parsed_url = cls.parse(url, map_instance=False)
+                    parsed_url = cls.parse(url, map_instance=False, glob=glob)
                     if settings["map_instance"] not in known_instances:
                         raise ValueError(
                             "Unknown instance {}. Known are: {}".format(
@@ -754,12 +754,18 @@ def parse(
         # asset_type = groups.get("asset_type")
         dandiset_id = groups.get("dandiset_id")
         version_id = groups.get("version")
-        location = groups.get("location")
+        location: str
+        if groups.get("location_folder") is not None:
+            assert "location" not in groups
+            location = urlunquote(groups["location_folder"])
+            if not location.endswith("/") and not glob:
+                location += "/"
+        else:
+            location = urlunquote(groups.get("location") or "")
         asset_id = groups.get("asset_id")
         path = groups.get("path")
         glob_param = groups.get("glob")
         if location:
-            location = urlunquote(location)
             # ATM carries leading '/' which IMHO is not needed/misguiding
             # somewhat, so I will just strip it
             location = location.lstrip("/")

dandi/tests/test_dandiarchive.py

@@ -193,22 +193,22 @@
         pytest.param(
             "https://gui.dandiarchive.org/#/dandiset/000001/files"
             "?location=%2Fsub-anm369962",
-            AssetItemURL(
+            AssetFolderURL(
                 instance=known_instances["dandi"],
                 dandiset_id="000001",
                 version_id=None,
-                path="sub-anm369962",
+                path="sub-anm369962/",
             ),
             marks=mark.skipif_no_network,
         ),
         pytest.param(
             "https://gui.dandiarchive.org/#/dandiset/000006/0.200714.1807/files"
             "?location=%2Fsub-anm369962",
-            AssetItemURL(
+            AssetFolderURL(
                 instance=known_instances["dandi"],
                 dandiset_id="000006",
                 version_id="0.200714.1807",
-                path="sub-anm369962",
+                path="sub-anm369962/",
             ),
             marks=mark.skipif_no_network,
         ),
@@ -325,11 +325,11 @@
         pytest.param(
             "https://gui.dandiarchive.org/#/dandiset/001001/draft/files"
             "?location=sub-RAT123/*.nwb",
-            AssetItemURL(
+            AssetFolderURL(
                 instance=known_instances["dandi"],
                 dandiset_id="001001",
                 version_id="draft",
-                path="sub-RAT123/*.nwb",
+                path="sub-RAT123/*.nwb/",
             ),
             marks=mark.skipif_no_network,
         ),
@@ -361,6 +361,16 @@ def test_parse_api_url(url: str, parsed_url: ParsedDandiURL) -> None:
 @pytest.mark.parametrize(
     "url,parsed_url",
     [
+        (
+            "https://dandiarchive.org/dandiset/001001/draft/files"
+            "?location=sub-RAT123/*.nwb",
+            AssetGlobURL(
+                instance=known_instances["dandi"],
+                dandiset_id="001001",
+                version_id="draft",
+                path="sub-RAT123/*.nwb",
+            ),
+        ),
         pytest.param(
             "https://gui.dandiarchive.org/#/dandiset/001001/draft/files"
             "?location=sub-RAT123/*.nwb",

docs/source/ref/urls.rst

@@ -35,13 +35,9 @@ has one, and its draft version will be used otherwise.
     a collection of assets whose paths match the glob pattern ``path``, and
     `parse_dandi_url()` will convert the URL to an `AssetGlobURL`.
 
-  - If the ``glob``/``--path-type glob`` option is not in effect and ``path``
-    ends with a trailing slash, the URL refers to an asset folder by path, and
-    `parse_dandi_url()` will convert the URL to an `AssetFolderURL`.
-
-  - If the ``glob``/``--path-type glob`` option is not in effect and ``path``
-    does not end with a trailing slash, the URL refers to a single asset by
-    path, and `parse_dandi_url()` will convert the URL to an `AssetItemURL`.
+  - If the ``glob``/``--path-type glob`` option is not in effect, the URL
+    refers to an asset folder by path, and `parse_dandi_url()` will convert the
+    URL to an `AssetFolderURL`.
 
 - :samp:`https://{server}[/api]/dandisets/{dandiset-id}[/versions[/{version}]]`
   — Refers to a Dandiset.  `parse_dandi_url()` converts this format to a