From b22b9da5d03d6a1727538710d30bbc0b1fb70f29 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= <sybren@stuvel.eu>
Date: Thu, 28 Feb 2019 12:32:07 +0100
Subject: [PATCH] Shaman: Moved URL parsing into a separate function

This allows other code to just call that function, rather than copying
the behaviour.
---
 blender_asset_tracer/cli/pack.py             | 18 +-------
 blender_asset_tracer/pack/shaman/__init__.py | 22 ++++++++++
 tests/test_shaman.py                         | 45 ++++++++++++++++++++
 3 files changed, 69 insertions(+), 16 deletions(-)
 create mode 100644 tests/test_shaman.py

diff --git a/blender_asset_tracer/cli/pack.py b/blender_asset_tracer/cli/pack.py
index e718b8e..8dcbe56 100644
--- a/blender_asset_tracer/cli/pack.py
+++ b/blender_asset_tracer/cli/pack.py
@@ -145,29 +145,15 @@ def create_shamanpacker(bpath: pathlib.Path, ppath: pathlib.Path, tpath: str) ->
     This uses HTTPS to connect to the server. To connect using HTTP, use:
         shaman+http://hostname/base-url#jobID
     """
-
-    import urllib.parse
     from blender_asset_tracer.pack import shaman
 
-    urlparts = urllib.parse.urlparse(str(tpath))
-
-    if urlparts.scheme in {'shaman', 'shaman+https'}:
-        scheme = 'https'
-    elif urlparts.scheme == 'shaman+http':
-        scheme = 'http'
-    else:
-        raise SystemExit('Invalid scheme %r, choose shaman:// or shaman+http://', urlparts.scheme)
-
-    checkout_id = urlparts.fragment
+    endpoint, checkout_id = shaman.parse_endpoint(tpath)
     if not checkout_id:
         log.warning('No checkout ID given on the URL. Going to send BAT pack to Shaman, '
                     'but NOT creating a checkout')
 
-    new_urlparts = (scheme, *urlparts[1:-1], '')
-    endpoint = urllib.parse.urlunparse(new_urlparts)
-
     log.info('Uploading to Shaman server %s with job %s', endpoint, checkout_id)
-    return shaman.ShamanPacker(bpath, ppath, tpath, endpoint=endpoint, checkout_id=checkout_id)
+    return shaman.ShamanPacker(bpath, ppath, '/', endpoint=endpoint, checkout_id=checkout_id)
 
 
 def paths_from_cli(args) -> typing.Tuple[pathlib.Path, pathlib.Path, str]:
diff --git a/blender_asset_tracer/pack/shaman/__init__.py b/blender_asset_tracer/pack/shaman/__init__.py
index 8ad4dea..d2129c6 100644
--- a/blender_asset_tracer/pack/shaman/__init__.py
+++ b/blender_asset_tracer/pack/shaman/__init__.py
@@ -20,6 +20,7 @@
 """Shaman Client interface."""
 import logging
 import pathlib
+import typing
 import urllib.parse
 
 import requests
@@ -93,3 +94,24 @@ class ShamanPacker(bat_pack.Packer):
             log.exception('Error communicating with Shaman')
             self.abort(str(ex))
             self._check_aborted()
+
+
+def parse_endpoint(shaman_url: str) -> typing.Tuple[str, str]:
+    """Convert shaman://hostname/path#checkoutID into endpoint URL + checkout ID."""
+
+    urlparts = urllib.parse.urlparse(str(shaman_url))
+
+    if urlparts.scheme in {'shaman', 'shaman+https'}:
+        scheme = 'https'
+    elif urlparts.scheme == 'shaman+http':
+        scheme = 'http'
+    else:
+        raise ValueError('Invalid scheme %r, choose shaman:// or shaman+http://', urlparts.scheme)
+
+    checkout_id = urllib.parse.unquote(urlparts.fragment)
+
+    path = urlparts.path or '/'
+    new_urlparts = (scheme, urlparts.netloc, path, *urlparts[3:-1], '')
+    endpoint = urllib.parse.urlunparse(new_urlparts)
+
+    return endpoint, checkout_id
diff --git a/tests/test_shaman.py b/tests/test_shaman.py
new file mode 100644
index 0000000..f349e49
--- /dev/null
+++ b/tests/test_shaman.py
@@ -0,0 +1,45 @@
+import unittest
+
+from blender_asset_tracer.pack import shaman
+
+
+class ParseEndpointTest(unittest.TestCase):
+    def test_path_slashyness(self):
+        self.assertEqual(
+            ('https://endpoint/', '123'),
+            shaman.parse_endpoint('shaman://endpoint#123'),
+        )
+        self.assertEqual(
+            ('https://endpoint/', '123'),
+            shaman.parse_endpoint('shaman://endpoint/#123'),
+        )
+        self.assertEqual(
+            ('https://endpoint/root', '123'),
+            shaman.parse_endpoint('shaman://endpoint/root#123'),
+        )
+        self.assertEqual(
+            ('https://endpoint/root/is/longer/', '123'),
+            shaman.parse_endpoint('shaman://endpoint/root/is/longer/#123'),
+        )
+
+    def test_schemes_with_plus(self):
+        self.assertEqual(
+            ('https://endpoint/', '123'),
+            shaman.parse_endpoint('shaman+https://endpoint/#123'),
+        )
+        self.assertEqual(
+            ('http://endpoint/', '123'),
+            shaman.parse_endpoint('shaman+http://endpoint/#123'),
+        )
+
+    def test_checkout_ids(self):
+        self.assertEqual(
+            ('https://endpoint/', ''),
+            shaman.parse_endpoint('shaman+https://endpoint/'),
+        )
+
+        # Not a valid ID, but the parser should handle it gracefully anyway
+        self.assertEqual(
+            ('http://endpoint/', 'ïđ'),
+            shaman.parse_endpoint('shaman+http://endpoint/#%C3%AF%C4%91'),
+        )