feat: use endpoints compatible with LFS server discovery

Additionally, add support for the <organization> in the URI to contain slashes, as some git repositories(e.g. GitLab) can contain multiple levels of organization groups/namespaces.
datopian · Mar 8, 2024 · cf62ee5 · cf62ee5
1 parent 422fad8
commit cf62ee5
Show file tree

Hide file tree

Showing 6 changed files with 24 additions and 18 deletions.
diff --git a/giftless/transfer/basic_streaming.py b/giftless/transfer/basic_streaming.py
@@ -33,7 +33,7 @@ class VerifyView(BaseView):
     make the test structures a little less weird?
     """
 
-    route_base = "<organization>/<repo>/objects/storage"
+    route_base = "objects/storage"
 
     def __init__(self, storage: VerifiableStorage) -> None:
         self.storage = storage
@@ -75,7 +75,7 @@ def get_verify_url(
 class ObjectsView(BaseView):
     """Provides methods for object storage management."""
 
-    route_base = "<organization>/<repo>/objects/storage"
+    route_base = "objects/storage"
 
     def __init__(self, storage: StreamingStorage) -> None:
         self.storage = storage

diff --git a/giftless/view.py b/giftless/view.py
@@ -23,6 +23,11 @@ class BaseView(FlaskView):
         "flask-classful/default": representation.output_git_lfs_json,
     }
 
+    route_prefix: ClassVar = "<path:organization>/<repo>.git/info/lfs/"
+    # [flask-classful bug/feat?] Placeholders in route_prefix not skipped for
+    # building the final rule for methods with them (FlaskView.build_rule).
+    base_args: ClassVar = ["organization", "repo"]
+
     trailing_slash = False
 
     @classmethod
@@ -64,7 +69,7 @@ def _is_authorized(
 class BatchView(BaseView):
     """Batch operations."""
 
-    route_base = "<organization>/<repo>/objects/batch"
+    route_base = "objects/batch"
 
     def post(self, organization: str, repo: str) -> dict[str, Any]:
         """Batch operations."""

diff --git a/tests/test_batch_api.py b/tests/test_batch_api.py
@@ -12,7 +12,7 @@ def test_upload_batch_request(test_client: FlaskClient) -> None:
     """Test basic batch API with a basic successful upload request."""
     request_payload = batch_request_payload(operation="upload")
     response = test_client.post(
-        "/myorg/myrepo/objects/batch", json=request_payload
+        "/myorg/myrepo.git/info/lfs/objects/batch", json=request_payload
     )
 
     assert response.status_code == 200
@@ -40,7 +40,7 @@ def test_download_batch_request(
     create_file_in_storage(storage_path, "myorg", "myrepo", oid, size=8)
 
     response = test_client.post(
-        "/myorg/myrepo/objects/batch", json=request_payload
+        "/myorg/myrepo.git/info/lfs/objects/batch", json=request_payload
     )
 
     assert response.status_code == 200
@@ -70,7 +70,7 @@ def test_download_batch_request_two_files_one_missing(
     request_payload["objects"].append({"oid": "12345679", "size": 5555})
 
     response = test_client.post(
-        "/myorg/myrepo/objects/batch", json=request_payload
+        "/myorg/myrepo.git/info/lfs/objects/batch", json=request_payload
     )
 
     assert response.status_code == 200
@@ -104,7 +104,7 @@ def test_download_batch_request_two_files_missing(
     request_payload["objects"].append({"oid": "12345679", "size": 5555})
 
     response = test_client.post(
-        "/myorg/myrepo/objects/batch", json=request_payload
+        "/myorg/myrepo.git/info/lfs/objects/batch", json=request_payload
     )
 
     assert response.status_code == 404
@@ -139,7 +139,7 @@ def test_download_batch_request_two_files_one_mismatch(
     )
 
     response = test_client.post(
-        "/myorg/myrepo/objects/batch", json=request_payload
+        "/myorg/myrepo.git/info/lfs/objects/batch", json=request_payload
     )
 
     assert response.status_code == 200
@@ -177,7 +177,7 @@ def test_download_batch_request_one_file_mismatch(
     )
 
     response = test_client.post(
-        "/myorg/myrepo/objects/batch", json=request_payload
+        "/myorg/myrepo.git/info/lfs/objects/batch", json=request_payload
     )
 
     assert response.status_code == 422
@@ -206,7 +206,7 @@ def test_download_batch_request_two_files_different_errors(
     )
 
     response = test_client.post(
-        "/myorg/myrepo/objects/batch", json=request_payload
+        "/myorg/myrepo.git/info/lfs/objects/batch", json=request_payload
     )
 
     assert response.status_code == 422

diff --git a/tests/test_error_responses.py b/tests/test_error_responses.py
@@ -7,7 +7,7 @@
 def test_error_response_422(test_client: FlaskClient) -> None:
     """Test an invalid payload error."""
     response = test_client.post(
-        "/myorg/myrepo/objects/batch",
+        "/myorg/myrepo.git/info/lfs/objects/batch",
         json=batch_request_payload(delete_keys=["operation"]),
     )
 
@@ -30,7 +30,7 @@ def test_error_response_403(test_client: FlaskClient) -> None:
     read-only setup.
     """
     response = test_client.post(
-        "/myorg/myrepo/objects/batch",
+        "/myorg/myrepo.git/info/lfs/objects/batch",
         json=batch_request_payload(operation="upload"),
     )
 

diff --git a/tests/test_middleware.py b/tests/test_middleware.py
@@ -44,17 +44,18 @@ def test_upload_request_with_x_forwarded_middleware(
     """
     request_payload = batch_request_payload(operation="upload")
     response = test_client.post(
-        "/myorg/myrepo/objects/batch", json=request_payload
+        "/myorg/myrepo.git/info/lfs/objects/batch", json=request_payload
     )
 
     assert response.status_code == 200
     json = cast(dict[str, Any], response.json)
     upload_action = json["objects"][0]["actions"]["upload"]
     href = upload_action["href"]
-    assert href == "http://localhost/myorg/myrepo/objects/storage/12345678"
+    base_uri = "myorg/myrepo.git/info/lfs"
+    assert href == f"http://localhost/{base_uri}/objects/storage/12345678"
 
     response = test_client.post(
-        "/myorg/myrepo/objects/batch",
+        "/myorg/myrepo.git/info/lfs/objects/batch",
         json=request_payload,
         headers={
             "X-Forwarded-Host": "mycompany.xyz",
@@ -70,5 +71,5 @@ def test_upload_request_with_x_forwarded_middleware(
     href = upload_action["href"]
     assert (
         href
-        == "https://mycompany.xyz:1234/lfs/myorg/myrepo/objects/storage/12345678"
+        == "https://mycompany.xyz:1234/lfs/myorg/myrepo.git/info/lfs/objects/storage/12345678"
     )
diff --git a/tests/transfer/test_basic_external_adapter.py b/tests/transfer/test_basic_external_adapter.py
@@ -45,7 +45,7 @@ def test_upload_action_new_file() -> None:
                 "expires_in": 900,
             },
             "verify": {
-                "href": "http://giftless.local/myorg/myrepo/objects/storage/verify",
+                "href": "http://giftless.local/myorg/myrepo.git/info/lfs/objects/storage/verify",
                 "header": {},
                 "expires_in": 43200,
             },
@@ -73,7 +73,7 @@ def test_upload_action_extras_are_passed() -> None:
                 "expires_in": 900,
             },
             "verify": {
-                "href": "http://giftless.local/myorg/myrepo/objects/storage/verify",
+                "href": "http://giftless.local/myorg/myrepo.git/info/lfs/objects/storage/verify",
                 "header": {},
                 "expires_in": 43200,
             },