From c1fd7e6c125acd46d3bee65f039a6c889d84097f Mon Sep 17 00:00:00 2001
From: Michael Woolnough <mw31@sanger.ac.uk>
Date: Mon, 25 Sep 2023 10:58:28 +0100
Subject: [PATCH 1/4] Add upload endpoint for builder to pass artifacts to be
 uploaded to repo.

---
 softpack_core/schemas/environment.py    | 10 +++--
 softpack_core/service.py                | 42 ++++++++++++++++++
 tests/integration/conftest.py           | 25 ++++++++++-
 tests/integration/test_builderupload.py | 58 +++++++++++++++++++++++++
 tests/integration/test_environment.py   | 24 +---------
 5 files changed, 132 insertions(+), 27 deletions(-)
 create mode 100644 tests/integration/test_builderupload.py

diff --git a/softpack_core/schemas/environment.py b/softpack_core/schemas/environment.py
index d9744d8..4113379 100644
--- a/softpack_core/schemas/environment.py
+++ b/softpack_core/schemas/environment.py
@@ -264,8 +264,8 @@ def create_new_env(
         already exists.
 
         Args:
-            env (EnvironmentInput): Details of the new environment. env_type
-            (str): One of Artifacts.built_by_softpack_file or
+            env (EnvironmentInput): Details of the new environment.
+            env_type (str): One of Artifacts.built_by_softpack_file or
             Artifacts.generated_from_module_file that denotes how the
             environment was made.
 
@@ -543,7 +543,11 @@ async def write_artifacts(
             new_files: List[Tuple[str, str]] = []
             for file in files:
                 contents = cast(str, (await file.read()).decode())
-                new_files.append((file.name, contents))
+                try:
+                    name = file.name
+                except BaseException:
+                    name = file.filename
+                new_files.append((name, contents))
 
             tree_oid = cls.artifacts.create_files(
                 Path(folder_path), new_files, overwrite=True
diff --git a/softpack_core/service.py b/softpack_core/service.py
index eaafe32..a477099 100644
--- a/softpack_core/service.py
+++ b/softpack_core/service.py
@@ -5,11 +5,22 @@
 """
 
 
+import urllib.parse
+from pathlib import Path
+from typing import List
+
 import typer
 import uvicorn
+from fastapi import APIRouter, Request, UploadFile
 from typer import Typer
 from typing_extensions import Annotated
 
+from softpack_core.schemas.environment import (
+    CreateEnvironmentSuccess,
+    Environment,
+    EnvironmentInput,
+)
+
 from .api import API
 from .app import app
 
@@ -19,6 +30,7 @@ class ServiceAPI(API):
 
     prefix = "/service"
     commands = Typer(help="Commands for managing core service.")
+    router = APIRouter()
 
     @staticmethod
     @commands.command(help="Start the SoftPack Core API service.")
@@ -46,3 +58,33 @@ def run(
             reload=reload,
             log_level="debug",
         )
+
+    @staticmethod
+    @router.post("/upload")
+    async def upload_artifacts(  # type: ignore[no-untyped-def]
+        file: List[UploadFile], request: Request
+    ):
+        """upload_artifacts is a POST fn that adds files to an environment.
+
+          The environment does not need to exist already.
+
+        Args:
+            file (List[MUploadFile]): The files to be uploaded.
+            request (Request): The POST request which contains the environment
+            path in the query.
+
+        Returns:
+            WriteArtifactResponse
+        """
+        env_path = urllib.parse.unquote(request.url.query)
+
+        if Environment.check_env_exists(Path(env_path)) is not None:
+            create_response = Environment.create_new_env(
+                EnvironmentInput.from_path(env_path),
+                Environment.artifacts.built_by_softpack,
+            )
+
+            if not isinstance(create_response, CreateEnvironmentSuccess):
+                return create_response
+
+        return await Environment.write_artifacts(env_path, file)
diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py
index 1ea552a..40c7857 100644
--- a/tests/integration/conftest.py
+++ b/tests/integration/conftest.py
@@ -9,7 +9,12 @@
 import pytest
 from starlette.datastructures import UploadFile
 
-from softpack_core.artifacts import app
+from softpack_core.artifacts import Artifacts, Package, app
+from softpack_core.schemas.environment import Environment, EnvironmentInput
+from tests.integration.utils import (
+    get_user_path_without_environments,
+    new_test_artifacts,
+)
 
 
 @pytest.fixture(scope="package", autouse=True)
@@ -41,3 +46,21 @@ def httpx_post(mocker):
 @pytest.fixture()
 def upload(mocker):
     return mocker.Mock(spec=UploadFile)
+
+
+@pytest.fixture
+def testable_env_input(mocker) -> EnvironmentInput:
+    ad = new_test_artifacts()
+    artifacts: Artifacts = ad["artifacts"]
+    user = ad["test_user"]
+
+    mocker.patch.object(Environment, 'artifacts', new=artifacts)
+
+    testable_env_input = EnvironmentInput(
+        name="test_env_create",
+        path=str(get_user_path_without_environments(artifacts, user)),
+        description="description",
+        packages=[Package(name="pkg_test")],
+    )
+
+    yield testable_env_input
diff --git a/tests/integration/test_builderupload.py b/tests/integration/test_builderupload.py
new file mode 100644
index 0000000..b250177
--- /dev/null
+++ b/tests/integration/test_builderupload.py
@@ -0,0 +1,58 @@
+"""Copyright (c) 2023 Genome Research Ltd.
+
+This source code is licensed under the MIT license found in the
+LICENSE file in the root directory of this source tree.
+"""
+
+from pathlib import Path
+
+import pytest
+from fastapi.testclient import TestClient
+
+from softpack_core.app import app
+from softpack_core.schemas.environment import Environment
+from softpack_core.service import ServiceAPI
+from tests.integration.utils import file_in_repo
+
+pytestmark = pytest.mark.repo
+
+
+def test_builder_upload(testable_env_input):
+    ServiceAPI.register()
+    client = TestClient(app.router)
+
+    env_parent = "groups/hgi"
+    env_name = "unknown-env"
+    env_path = env_parent + "/" + env_name
+
+    softpackYaml = "softpack.yaml"
+    softpackYamlContents = b"softpack yaml file"
+
+    spackLock = "spack.lock"
+    spackLockContents = b"spack lock file"
+
+    assert Environment.check_env_exists(Path(env_path)) is not None
+    resp = client.post(
+        url="/upload?" + env_path,
+        files=[
+            ("file", (softpackYaml, softpackYamlContents)),
+            ("file", (spackLock, spackLockContents)),
+        ],
+    )
+    assert resp.status_code == 200
+    assert resp.json().get("message") == "Successfully written artifact(s)"
+    assert Environment.check_env_exists(Path(env_path)) is None
+    assert file_in_repo(
+        Environment.artifacts,
+        Path(Environment.artifacts.environments_root, env_path, softpackYaml),
+    )
+    assert file_in_repo(
+        Environment.artifacts,
+        Path(Environment.artifacts.environments_root, env_path, spackLock),
+    )
+
+    tree = Environment.artifacts.get(env_parent, env_name)
+    assert tree is not None
+
+    assert tree[softpackYaml].data == softpackYamlContents
+    assert tree[spackLock].data == spackLockContents
diff --git a/tests/integration/test_environment.py b/tests/integration/test_environment.py
index 7cf5d85..060bc59 100644
--- a/tests/integration/test_environment.py
+++ b/tests/integration/test_environment.py
@@ -24,33 +24,11 @@
     UpdateEnvironmentSuccess,
     WriteArtifactSuccess,
 )
-from tests.integration.utils import (
-    file_in_remote,
-    get_user_path_without_environments,
-    new_test_artifacts,
-)
+from tests.integration.utils import file_in_remote
 
 pytestmark = pytest.mark.repo
 
 
-@pytest.fixture
-def testable_env_input(mocker) -> EnvironmentInput:
-    ad = new_test_artifacts()
-    artifacts: Artifacts = ad["artifacts"]
-    user = ad["test_user"]
-
-    mocker.patch.object(Environment, 'artifacts', new=artifacts)
-
-    testable_env_input = EnvironmentInput(
-        name="test_env_create",
-        path=str(get_user_path_without_environments(artifacts, user)),
-        description="description",
-        packages=[Package(name="pkg_test")],
-    )
-
-    yield testable_env_input
-
-
 def test_create(httpx_post, testable_env_input: EnvironmentInput) -> None:
     result = Environment.create(testable_env_input)
     assert isinstance(result, CreateEnvironmentSuccess)

From d649b6b47425ee06c2b07c1fa8c9f522b4fed635 Mon Sep 17 00:00:00 2001
From: Michael Woolnough <mw31@sanger.ac.uk>
Date: Mon, 25 Sep 2023 11:58:49 +0100
Subject: [PATCH 2/4] Use correct placeholder filename.

---
 softpack_core/service.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/softpack_core/service.py b/softpack_core/service.py
index a477099..35f21e2 100644
--- a/softpack_core/service.py
+++ b/softpack_core/service.py
@@ -81,7 +81,7 @@ async def upload_artifacts(  # type: ignore[no-untyped-def]
         if Environment.check_env_exists(Path(env_path)) is not None:
             create_response = Environment.create_new_env(
                 EnvironmentInput.from_path(env_path),
-                Environment.artifacts.built_by_softpack,
+                Environment.artifacts.built_by_softpack_file,
             )
 
             if not isinstance(create_response, CreateEnvironmentSuccess):

From 5d289dee8aab856899112c9606590cf536bca2a7 Mon Sep 17 00:00:00 2001
From: Michael Woolnough <mw31@sanger.ac.uk>
Date: Tue, 26 Sep 2023 10:19:14 +0100
Subject: [PATCH 3/4] Support large file uploads.

---
 softpack_core/artifacts.py            | 10 ++++--
 softpack_core/schemas/environment.py  | 45 ++++++++++++++++-----------
 softpack_core/service.py              | 14 ++++++---
 tests/integration/conftest.py         |  6 ----
 tests/integration/test_environment.py | 43 ++++++++++++++-----------
 tox.ini                               |  4 +--
 6 files changed, 68 insertions(+), 54 deletions(-)

diff --git a/softpack_core/artifacts.py b/softpack_core/artifacts.py
index 1b776ab..3d9d9b9 100644
--- a/softpack_core/artifacts.py
+++ b/softpack_core/artifacts.py
@@ -9,11 +9,12 @@
 from dataclasses import dataclass
 from enum import Enum
 from pathlib import Path
-from typing import Iterable, Iterator, Optional, Tuple
+from typing import Iterable, Iterator, List, Optional, Tuple, Union
 
 import pygit2
 import strawberry
 from box import Box
+from fastapi import UploadFile
 
 from softpack_core.spack import Spack
 
@@ -400,7 +401,7 @@ def create_file(
     def create_files(
         self,
         folder_path: Path,
-        files: list[Tuple[str, str]],
+        files: List[Tuple[str, Union[str, UploadFile]]],
         new_folder: bool = False,
         overwrite: bool = False,
     ) -> pygit2.Oid:
@@ -431,7 +432,10 @@ def create_files(
             new_treebuilder = self.repo.TreeBuilder(folder)
 
         for file_name, contents in files:
-            file_oid = self.repo.create_blob(contents.encode())
+            if isinstance(contents, str):
+                file_oid = self.repo.create_blob(contents)
+            else:
+                file_oid = self.repo.create_blob_fromiobase(contents.file)
             new_treebuilder.insert(
                 file_name, file_oid, pygit2.GIT_FILEMODE_BLOB
             )
diff --git a/softpack_core/schemas/environment.py b/softpack_core/schemas/environment.py
index 4113379..cd2b169 100644
--- a/softpack_core/schemas/environment.py
+++ b/softpack_core/schemas/environment.py
@@ -10,8 +10,9 @@
 from typing import Iterable, List, Optional, Tuple, Union, cast
 
 import httpx
+import starlette.datastructures
 import strawberry
-from starlette.datastructures import UploadFile
+from fastapi import UploadFile
 from strawberry.file_uploads import Upload
 
 from softpack_core.artifacts import Artifacts, Package, State
@@ -471,9 +472,15 @@ async def convert_module_file_to_artifacts(
         yml = ToSoftpackYML(env_name, contents)
         readme = GenerateEnvReadme(module_path)
 
-        module_file = UploadFile(file=io.BytesIO(contents))
-        softpack_file = UploadFile(file=io.BytesIO(yml))
-        readme_file = UploadFile(file=io.BytesIO(readme))
+        module_file = UploadFile(
+            filename=cls.artifacts.module_file, file=io.BytesIO(contents)
+        )
+        softpack_file = UploadFile(
+            filename=cls.artifacts.environments_file, file=io.BytesIO(yml)
+        )
+        readme_file = UploadFile(
+            filename=cls.artifacts.readme_file, file=io.BytesIO(readme)
+        )
 
         return await cls.write_module_artifacts(
             module_file=module_file,
@@ -531,7 +538,7 @@ async def write_artifact(
 
     @classmethod
     async def write_artifacts(
-        cls, folder_path: str, files: list[Upload]
+        cls, folder_path: str, files: list[Union[Upload, UploadFile]]
     ) -> WriteArtifactResponse:  # type: ignore
         """Add one or more files to the Artifacts repo.
 
@@ -540,14 +547,16 @@ async def write_artifacts(
             files: the files to add to the repo.
         """
         try:
-            new_files: List[Tuple[str, str]] = []
+            new_files: List[Tuple[str, Union[str, UploadFile]]] = []
             for file in files:
-                contents = cast(str, (await file.read()).decode())
-                try:
-                    name = file.name
-                except BaseException:
-                    name = file.filename
-                new_files.append((name, contents))
+                if isinstance(file, starlette.datastructures.UploadFile):
+                    new_files.append(
+                        (file.filename or "", cast(UploadFile, file))
+                    )
+                else:
+                    new_files.append(
+                        (file.name, cast(str, (await file.read()).decode()))
+                    )
 
             tree_oid = cls.artifacts.create_files(
                 Path(folder_path), new_files, overwrite=True
@@ -625,12 +634,12 @@ class Mutation:
         createEnvironment: CreateResponse = Environment.create  # type: ignore
         updateEnvironment: UpdateResponse = Environment.update  # type: ignore
         deleteEnvironment: DeleteResponse = Environment.delete  # type: ignore
-        writeArtifact: WriteArtifactResponse = (  # type: ignore
-            Environment.write_artifact
-        )
-        writeArtifacts: WriteArtifactResponse = (  # type: ignore
-            Environment.write_artifacts
-        )
+        # writeArtifact: WriteArtifactResponse = (  # type: ignore
+        #     Environment.write_artifact
+        # )
+        # writeArtifacts: WriteArtifactResponse = (  # type: ignore
+        #     Environment.write_artifacts
+        # )
         createFromModule: CreateResponse = (  # type: ignore
             Environment.create_from_module
         )
diff --git a/softpack_core/service.py b/softpack_core/service.py
index 35f21e2..6ea3829 100644
--- a/softpack_core/service.py
+++ b/softpack_core/service.py
@@ -7,7 +7,6 @@
 
 import urllib.parse
 from pathlib import Path
-from typing import List
 
 import typer
 import uvicorn
@@ -19,6 +18,7 @@
     CreateEnvironmentSuccess,
     Environment,
     EnvironmentInput,
+    WriteArtifactSuccess,
 )
 
 from .api import API
@@ -62,14 +62,15 @@ def run(
     @staticmethod
     @router.post("/upload")
     async def upload_artifacts(  # type: ignore[no-untyped-def]
-        file: List[UploadFile], request: Request
+        request: Request,
+        file: list[UploadFile],
     ):
         """upload_artifacts is a POST fn that adds files to an environment.
 
           The environment does not need to exist already.
 
         Args:
-            file (List[MUploadFile]): The files to be uploaded.
+            file (List[UploadFile]): The files to be uploaded.
             request (Request): The POST request which contains the environment
             path in the query.
 
@@ -77,7 +78,6 @@ async def upload_artifacts(  # type: ignore[no-untyped-def]
             WriteArtifactResponse
         """
         env_path = urllib.parse.unquote(request.url.query)
-
         if Environment.check_env_exists(Path(env_path)) is not None:
             create_response = Environment.create_new_env(
                 EnvironmentInput.from_path(env_path),
@@ -87,4 +87,8 @@ async def upload_artifacts(  # type: ignore[no-untyped-def]
             if not isinstance(create_response, CreateEnvironmentSuccess):
                 return create_response
 
-        return await Environment.write_artifacts(env_path, file)
+        resp = await Environment.write_artifacts(env_path, file)
+        if not isinstance(resp, WriteArtifactSuccess):
+            raise Exception(resp)
+
+        return resp
diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py
index 40c7857..56aee99 100644
--- a/tests/integration/conftest.py
+++ b/tests/integration/conftest.py
@@ -7,7 +7,6 @@
 import os
 
 import pytest
-from starlette.datastructures import UploadFile
 
 from softpack_core.artifacts import Artifacts, Package, app
 from softpack_core.schemas.environment import Environment, EnvironmentInput
@@ -43,11 +42,6 @@ def httpx_post(mocker):
     return post_mock
 
 
-@pytest.fixture()
-def upload(mocker):
-    return mocker.Mock(spec=UploadFile)
-
-
 @pytest.fixture
 def testable_env_input(mocker) -> EnvironmentInput:
     ad = new_test_artifacts()
diff --git a/tests/integration/test_environment.py b/tests/integration/test_environment.py
index 060bc59..11678e6 100644
--- a/tests/integration/test_environment.py
+++ b/tests/integration/test_environment.py
@@ -4,11 +4,13 @@
 LICENSE file in the root directory of this source tree.
 """
 
+import io
 from pathlib import Path
 from typing import Optional
 
 import pygit2
 import pytest
+from fastapi import UploadFile
 
 from softpack_core.artifacts import Artifacts
 from softpack_core.schemas.environment import (
@@ -150,10 +152,8 @@ def test_delete(httpx_post, testable_env_input) -> None:
 
 
 @pytest.mark.asyncio
-async def test_write_artifact(httpx_post, testable_env_input, upload):
-    upload.filename = "example.txt"
-    upload.content_type = "text/plain"
-    upload.read.return_value = b"mock data"
+async def test_write_artifact(httpx_post, testable_env_input):
+    upload = UploadFile(filename="example.txt", file=io.BytesIO(b"mock data"))
 
     result = await Environment.write_artifact(
         file=upload,
@@ -195,15 +195,16 @@ def test_iter(testable_env_input):
 
 
 @pytest.mark.asyncio
-async def test_states(httpx_post, testable_env_input, upload):
+async def test_states(httpx_post, testable_env_input):
     result = Environment.create(testable_env_input)
     assert isinstance(result, CreateEnvironmentSuccess)
     httpx_post.assert_called_once()
 
-    upload.filename = Artifacts.environments_file
-    upload.content_type = "text/plain"
-    upload.read.return_value = (
-        b"description: test env\n" b"packages:\n  - zlib@v1.1\n"
+    upload = UploadFile(
+        filename=Artifacts.environments_file,
+        file=io.BytesIO(
+            b"description: test env\n" b"packages:\n  - zlib@v1.1\n"
+        ),
     )
 
     result = await Environment.write_artifact(
@@ -220,9 +221,9 @@ async def test_states(httpx_post, testable_env_input, upload):
     assert env.type == Artifacts.built_by_softpack
     assert env.state == State.queued
 
-    upload.filename = Artifacts.module_file
-    upload.content_type = "text/plain"
-    upload.read.return_value = b"#%Module"
+    upload = UploadFile(
+        filename=Artifacts.module_file, file=io.BytesIO(b"#%Module")
+    )
 
     result = await Environment.write_artifact(
         file=upload,
@@ -243,14 +244,14 @@ def get_env_from_iter(name: str) -> Optional[Environment]:
 
 
 @pytest.mark.asyncio
-async def test_create_from_module(httpx_post, testable_env_input, upload):
+async def test_create_from_module(httpx_post, testable_env_input):
     test_files_dir = Path(__file__).parent.parent / "files" / "modules"
     test_file_path = test_files_dir / "shpc.mod"
 
     with open(test_file_path, "rb") as fh:
-        upload.filename = "shpc.mod"
-        upload.content_type = "text/plain"
-        upload.read.return_value = fh.read()
+        data = fh.read()
+
+    upload = UploadFile(filename="shpc.mod", file=io.BytesIO(data))
 
     env_name = "some-environment"
     name = "groups/hgi/" + env_name
@@ -264,6 +265,8 @@ async def test_create_from_module(httpx_post, testable_env_input, upload):
 
     assert isinstance(result, CreateEnvironmentSuccess)
 
+    upload = UploadFile(filename="shpc.mod", file=io.BytesIO(data))
+
     result = await Environment.create_from_module(
         file=upload,
         module_path=module_path,
@@ -313,9 +316,9 @@ async def test_create_from_module(httpx_post, testable_env_input, upload):
     test_modifiy_file_path = test_files_dir / "all_fields.mod"
 
     with open(test_modifiy_file_path, "rb") as fh:
-        upload.filename = "all_fields.mod"
-        upload.content_type = "text/plain"
-        upload.read.return_value = fh.read()
+        data = fh.read()
+
+    upload = UploadFile(filename="all_fields.mod", file=io.BytesIO(data))
 
     module_path = "HGI/common/all_fields"
 
@@ -339,6 +342,8 @@ async def test_create_from_module(httpx_post, testable_env_input, upload):
     assert env.type == Artifacts.generated_from_module
     assert env.state == State.ready
 
+    upload = UploadFile(filename="all_fields.mod", file=io.BytesIO(data))
+
     result = await Environment.update_from_module(
         file=upload,
         module_path=module_path,
diff --git a/tox.ini b/tox.ini
index bb1214b..14d2cf0 100644
--- a/tox.ini
+++ b/tox.ini
@@ -1,12 +1,10 @@
 [tox]
 isolated_build = true
-envlist = py39, py310, py311, format, lint, build
+envlist = py311, format, lint, build
 
 [gh-actions]
 python =
     3.11: py311, format, lint, build
-    3.10: py310
-    3.9: py39
 
 [flake8]
 max-line-length = 79

From 246f096cdff9a45fa33bd8ecff179a6a878c22d1 Mon Sep 17 00:00:00 2001
From: Michael Woolnough <mw31@sanger.ac.uk>
Date: Tue, 26 Sep 2023 10:34:34 +0100
Subject: [PATCH 4/4] Add python version requirement.

---
 README.md      | 4 +++-
 pyproject.toml | 2 --
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/README.md b/README.md
index 3992145..9a1e8dd 100644
--- a/README.md
+++ b/README.md
@@ -28,7 +28,9 @@ SoftPack Core - GraphQL backend service
 
 ### External dependencies
 
-SoftPack Core relies on Spack. Install that first:
+SoftPack Core requires Python version 3.11 or greater.
+
+This project also relies on Spack. Install that first:
 
 ``` console
 $ git clone -c feature.manyFiles=true --depth 1 https://github.com/spack/spack.git
diff --git a/pyproject.toml b/pyproject.toml
index e44e433..a3cb5d3 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -13,8 +13,6 @@ classifiers=[
     'License :: OSI Approved :: MIT License',
     'Natural Language :: English',
     'Programming Language :: Python :: 3',
-    'Programming Language :: Python :: 3.9',
-    'Programming Language :: Python :: 3.10',
     'Programming Language :: Python :: 3.11',
 ]
 packages = [