Submodule, urlparse

.github/workflows/ci.yaml

@@ -24,6 +24,7 @@ jobs:
         run: |
           python -m pip install --upgrade pip
           pip install ".[test]"
+          git submodule update
       - name: Test with pytest
         run: |
           export AWS_DEFAULT_REGION=us-east-1; python -m pytest --cov-report xml --cov=src tests

.gitmodules

@@ -0,0 +1,3 @@
+[submodule "tests/test_data/mock_payloads/cumulus-aggregator-test-study"]
+	path = tests/test_data/mock_payloads/cumulus-aggregator-test-study
+	url = https://github.com/smart-on-fhir/cumulus-aggregator-test-study

src/dashboard/post_distribute/post_distribute.py

@@ -2,6 +2,7 @@
 
 import json
 import os
+import urllib
 
 import boto3
 import requests
@@ -13,8 +14,11 @@
 
 
 def validate_github_url(config):
-    if not config["url"].startswith("https://github.com/smart-on-fhir/") or any(
-        c not in valid_chars for c in config["url"]
+    parsed_url = urllib.parse.urlparse(config["url"])
+    if (
+        not parsed_url.netloc == "github.com"
+        or not parsed_url.path.startswith("/smart-on-fhir")
+        or any(c not in valid_chars for c in config["url"])
     ):
         raise ValueError(f"{config['url']} is not an official Cumulus study.")
     res = requests.get(config["url"], timeout=10)

src/dashboard/queue_distribute/queue_distribute.py

@@ -39,12 +39,10 @@
 
 def get_study_from_github(config):
     try:
-        subprocess.run(["/usr/bin/git", "clone", config["url"], f"{BASE_DIR}/studies"], check=True)  # noqa: S603
-        if "tag" in config and config["tag"] is not None:
-            subprocess.run(  # noqa: S603
-                ["/usr/bin/git", "checkout", "tag"],
-                cwd=f"{BASE_DIR}/studies/{config['url'].split('/')[-2]}",
-            )
+        args = ["--depth", "1", config["url"], f"{BASE_DIR}/studies"]
+        if config["tag"]:
+            args = ["--branch", config["tag"], *args]
+        subprocess.run(["/usr/bin/git", "clone", *args], check=True)  # noqa: S603
 
     except subprocess.CalledProcessError:
         # TODO: retry/backoff logic? or do we just have a user queue again?

tests/dashboard/test_queue_distribute.py

@@ -25,13 +25,13 @@ def error_callback(process):
     [
         (
             "test_study",
-            "https://github.com/smart-on-fhir/test_study/",
+            "https://github.com/smart-on-fhir/cumulus-aggregator-test-study/",
             None,
             200,
         ),
         (
             "test_study",
-            "https://github.com/smart-on-fhir/test_study/",
+            "https://github.com/smart-on-fhir/cumulus-aggregator-test-study/",
             "tag",
             200,
         ),
@@ -43,15 +43,23 @@ def test_process_github(
     mock_notification, tmp_path, name, url, tag, expected_status, monkeypatch, fp
 ):
     fp.allow_unregistered(True)  # fp is provided by pytest-subprocess
+
+    args = ["--depth", "1", url, f"{tmp_path}/studies"]
+    if tag:
+        args = ["--branch", tag, *args]
     if name == "invalid_study":
-        fp.register(["/usr/bin/git", "clone", url, f"{tmp_path}/studies"], callback=error_callback)
+        fp.register(["/usr/bin/git", "clone", *args], callback=error_callback)
     else:
-        fp.register(["/usr/bin/git", "clone", url, f"{tmp_path}/studies"])
+        fp.register(["/usr/bin/git", "clone", *args])
         (tmp_path / "studies").mkdir()
+        study_dir = tmp_path / f"studies/{name}"
         shutil.copytree(
-            pathlib.Path.cwd() / f"./tests/test_data/mock_payloads/{name}/",
-            tmp_path / f"studies/{name}",
+            pathlib.Path.cwd() / "./tests/test_data/mock_payloads/cumulus-aggregator-test-study",
+            study_dir,
         )
+        if tag:
+            # if we're checking out a tag, make sure we've set up an actual git repo
+            subprocess.run(["git", "checkout", "tag"], cwd=study_dir)
 
     monkeypatch.setattr(queue_distribute, "BASE_DIR", tmp_path)
     mock_sns = mock.MagicMock()
@@ -88,4 +96,5 @@ def test_process_github(
     if tag == "tag":
         files = [p for p in (tmp_path / f"studies/{name}").iterdir() if p.is_file()]
         files = [file.stem for file in files]
-        assert "tag" in files
+        print(type(files[0]))
+        assert "tag" not in files