Merge pull request #1921 from minrk/trailing-slash-repoproviders

strip trailing slash on github/gist/gitlab URLs

Author: consideRatio

binderhub/repoproviders.py

@@ -711,7 +711,7 @@ class GitLabRepoProvider(RepoProvider):
         "displayName": "GitLab",
         "id": "gl",
         "spec": {"validateRegex": r"[^/]+/.+"},
-        "detect": {"regex": "^(https?://gitlab.com/)?(?<repo>.*)"},
+        "detect": {"regex": "^(https?://gitlab.com/)?(?<repo>.*[^/])/?"},
         "repo": {
             "label": "GitLab repository name or URL",
             "placeholder": "example: https://gitlab.com/mosaik/examples/mosaik-tutorials-on-binder or mosaik/examples/mosaik-tutorials-on-binder",
@@ -839,8 +839,8 @@ class GitHubRepoProvider(RepoProvider):
     display_config = {
         "displayName": "GitHub",
         "id": "gh",
-        "spec": {"validateRegex": r".+/.+/.+"},
-        "detect": {"regex": "^(https?://github.com/)?(?<repo>.*)"},
+        "spec": {"validateRegex": r"[^/]+/[^/]+/.+"},
+        "detect": {"regex": "^(https?://github.com/)?(?<repo>.*[^/])/?"},
         "repo": {
             "label": "GitHub repository name or URL",
             "placeholder": "example: yuvipanda/requirements or https://github.com/yuvipanda/requirements",
@@ -1112,8 +1112,8 @@ class GistRepoProvider(GitHubRepoProvider):
     display_config = {
         "displayName": "GitHub Gist",
         "id": "gist",
-        "spec": {"validateRegex": r".+/.+(/.+)"},
-        "detect": {"regex": "^(https?://gist.github.com/)?(?<repo>.*)"},
+        "spec": {"validateRegex": r"[^/]+/[^/]+(/[^/]+)?"},
+        "detect": {"regex": "^(https?://gist.github.com/)?(?<repo>.*[^/])/?"},
         "repo": {
             "label": "Gist ID (username/gistId) or URL",
             "placeholder": "",

binderhub/tests/test_repoproviders.py

@@ -610,3 +610,83 @@ def test_gist_secret():
 
     provider = GistRepoProvider(spec=spec, allow_secret_gist=True)
     assert IOLoop().run_sync(provider.get_resolved_ref) is not None
+
+
+def _js_regex(pat):
+    """compile a javascript regular expression
+
+    converts js '?<name>' to Python '?P<name>' syntax
+    """
+    return re.compile(pat.replace("?<", "?P<"))
+
+
+@pytest.mark.parametrize(
+    "provider, repo, expected_spec",
+    [
+        (
+            GitHubRepoProvider,
+            "https://github.com/org/repo",
+            "org/repo",
+        ),
+        (
+            GitHubRepoProvider,
+            "http://github.com/org/repo/",
+            "org/repo",
+        ),
+        (
+            GitHubRepoProvider,
+            "http://github.com/org/",
+            "org",
+        ),
+        (
+            GitHubRepoProvider,
+            "org/repo",
+            "org/repo",
+        ),
+        (
+            GitHubRepoProvider,
+            "org/repo/",
+            "org/repo",
+        ),
+        (
+            GitLabRepoProvider,
+            "https://gitlab.com/org/repo",
+            "org/repo",
+        ),
+        (
+            GitLabRepoProvider,
+            "https://gitlab.com/org/repo/",
+            "org/repo",
+        ),
+        (
+            GitLabRepoProvider,
+            "org/repo/",
+            "org/repo",
+        ),
+        (
+            GitLabRepoProvider,
+            "org/repo",
+            "org/repo",
+        ),
+        (
+            GistRepoProvider,
+            "user/12345",
+            "user/12345",
+        ),
+        (
+            GistRepoProvider,
+            "user/12345/",
+            "user/12345",
+        ),
+    ],
+)
+def test_detect_regex(provider, repo, expected_spec):
+    config = provider.display_config
+    detect_regex = _js_regex(config["detect"]["regex"])
+
+    match = detect_regex.match(repo)
+    if expected_spec is None and match is None:
+        # ok, no match expected
+        return
+    repo = match.group("repo")
+    assert repo == expected_spec