chore!: update DirBasedPackage to represent dist directory, not sou…

…rce directory

Use Pydantic's internal JSON parser consistently.

BREAKING CHANGE: Removed `manifest` from `DirBasedPackage` constructor arguments.

pyproject.toml

@@ -8,7 +8,7 @@ description = "QuestionPy application server"
 authors = ["Technische Universität Berlin, innoCampus <[email protected]>"]
 license = "MIT"
 homepage = "https://questionpy.org"
-version = "0.2.5"
+version = "0.2.6"
 packages = [
     { include = "questionpy_common" },
     { include = "questionpy_server" }

questionpy_server/worker/runtime/package.py

@@ -2,7 +2,6 @@
 #  The QuestionPy Server is free software released under terms of the MIT license. See LICENSE.md.
 #  (c) Technische Universität Berlin, innoCampus <[email protected]>
 import inspect
-import json
 import sys
 import zipfile
 from abc import ABC, abstractmethod
@@ -73,8 +72,8 @@ def __init__(self, path: Path):
     @cached_property
     def manifest(self) -> Manifest:
         """Load QuestionPy manifest from package."""
-        data = json.loads(self.read(f"{DIST_DIR}/{MANIFEST_FILENAME}"))
-        return Manifest.model_validate(data)
+        data = self.read(f"{DIST_DIR}/{MANIFEST_FILENAME}")
+        return Manifest.model_validate_json(data)
 
     def get_path(self, path: str) -> Traversable:
         # Intuitively, a path beginning with '/' should be absolute within the package, but ZipFile behaves differently.
@@ -98,23 +97,24 @@ def __repr__(self) -> str:
 
 
 class DirBasedPackage(ImportablePackage):
-    """A package's source directory to be used directly."""
+    """A package's dist directory to be used directly."""
 
-    def __init__(self, path: Path, manifest: Manifest) -> None:
+    def __init__(self, path: Path) -> None:
         self.path = path
-        self._manifest = manifest
 
-    @property
+    @cached_property
     def manifest(self) -> Manifest:
-        return self._manifest
+        """Load QuestionPy manifest from package."""
+        manifest_path = self.path / MANIFEST_FILENAME
+        return Manifest.model_validate_json(manifest_path.read_bytes())
 
     def get_path(self, path: str) -> Traversable:
         return self.path.joinpath(path)
 
     def setup_imports(self) -> None:
         for new_path in (
-            str(self.path / DIST_DIR / "dependencies" / "site-packages"),
-            str(self.path / DIST_DIR / "python"),
+            str(self.path / "dependencies" / "site-packages"),
+            str(self.path / "python"),
         ):
             if new_path not in sys.path:
                 sys.path.insert(0, new_path)
@@ -172,7 +172,7 @@ def load_package(location: PackageLocation) -> ImportablePackage:
     if isinstance(location, ZipPackageLocation):
         return ZipBasedPackage(location.path)
     if isinstance(location, DirPackageLocation):
-        return DirBasedPackage(location.path, location.manifest)
+        return DirBasedPackage(location.path)
     if isinstance(location, FunctionPackageLocation):
         return FunctionBasedPackage(location.module_name, location.function_name, location.manifest)
 

questionpy_server/worker/runtime/package_location.py

@@ -22,17 +22,10 @@ def __str__(self) -> str:
 
 @dataclass
 class DirPackageLocation:
-    """A package's source directory to be loaded directly."""
+    """A package's dist directory to be loaded directly."""
 
     path: Path
 
-    manifest: Manifest
-    """The manifest to use, probably loaded from the package config.
-
-    Since the source format of a manifest is YAML and we don't want the runtime to depend on PyYAML, the server should
-    load the manifest and pass it into the worker here.
-    """
-
     kind: Literal["dir"] = field(default="dir", init=False)
 
     def __str__(self) -> str:

questionpy_server/worker/worker/base.py

@@ -244,11 +244,10 @@ def _get_static_file_sync(self, path: str, manifest: Manifest) -> PackageFileDat
             log.info("Static file '%s' is not listed in package manifest.", path)
             raise FileNotFoundError(path) from e
 
-        dist_path = DIST_DIR + "/" + path
-
         reader: Callable[[], bytes]
         if isinstance(self.package, ZipPackageLocation):
             with ZipFile(self.package.path) as zip_file:
+                dist_path = f"{DIST_DIR}/{path}"
                 try:
                     zipinfo = zip_file.getinfo(dist_path)
                 except KeyError as e:
@@ -257,17 +256,20 @@ def _get_static_file_sync(self, path: str, manifest: Manifest) -> PackageFileDat
 
                 real_size = zipinfo.file_size
                 reader = partial(zip_file.read, dist_path)
+
         elif isinstance(self.package, DirPackageLocation):
-            full_path: Path = self.package.path / dist_path
-            if not full_path.exists():
+            full_path: Path = self.package.path / path
+            try:
+                real_size = full_path.stat().st_size
+                reader = full_path.read_bytes
+            except FileNotFoundError:
                 log.info("Static file '%s' is missing despite being listed in the manifest.", path)
-                raise FileNotFoundError(path)
+                raise
 
-            real_size = full_path.stat().st_size
-            reader = full_path.read_bytes
         elif isinstance(self.package, FunctionPackageLocation):
             msg = "Function-based packages don't serve static files."
             raise NotImplementedError(msg)
+
         else:
             raise TypeError(type(self.package).__name__)
 

tests/conftest.py

@@ -51,7 +51,7 @@ def __init__(self, path: Path):
     def as_dir_package(self) -> Iterator[DirPackageLocation]:
         with TemporaryDirectory() as tmp_dir, ZipFile(self.path) as package:
             package.extractall(tmp_dir)
-            yield DirPackageLocation(Path(tmp_dir), self.manifest)
+            yield DirPackageLocation(Path(tmp_dir) / DIST_DIR)
 
 
 test_data_path = Path(__file__).parent / "test_data"

tests/questionpy_server/worker/worker/test_base.py

@@ -3,13 +3,12 @@
 #  (c) Technische Universität Berlin, innoCampus <[email protected]>
 
 import mimetypes
-from copy import deepcopy
 from typing import TYPE_CHECKING
 
 import pytest
 
-from questionpy_common.constants import DIST_DIR, MANIFEST_FILENAME, MiB
-from questionpy_common.manifest import PackageFile
+from questionpy_common.constants import MANIFEST_FILENAME, MiB
+from questionpy_common.manifest import Manifest, PackageFile
 from questionpy_server import WorkerPool
 from questionpy_server.worker.runtime.package_location import DirPackageLocation
 from questionpy_server.worker.worker.base import StaticFileSizeMismatchError
@@ -36,15 +35,16 @@ async def test_should_get_manifest(pool: WorkerPool) -> None:
 
 
 def _inject_static_file_into_dist(package: DirPackageLocation, name: str, content: str) -> None:
-    full_path = package.path / DIST_DIR / name
+    full_path = package.path / name
     full_path.parent.mkdir(exist_ok=True)
     full_path.write_text(content)
 
 
 def _inject_static_file_into_manifest(package: DirPackageLocation, name: str, size: int) -> None:
-    package.manifest = deepcopy(package.manifest)
-    package.manifest.static_files[name] = PackageFile(mime_type=mimetypes.guess_type(name)[0], size=size)
-    (package.path / DIST_DIR / MANIFEST_FILENAME).write_text(package.manifest.model_dump_json())
+    manifest_path = package.path / MANIFEST_FILENAME
+    manifest = Manifest.model_validate_json(manifest_path.read_bytes())
+    manifest.static_files[name] = PackageFile(mime_type=mimetypes.guess_type(name)[0], size=size)
+    manifest_path.write_text(manifest.model_dump_json())
 
 
 _STATIC_FILE_NAME = "static/test_file.txt"