diff --git a/docs/qppe-server.yaml b/docs/qppe-server.yaml index 283fb64..956ec06 100644 --- a/docs/qppe-server.yaml +++ b/docs/qppe-server.yaml @@ -63,13 +63,7 @@ paths: content: application/json: schema: - type: object - properties: - definition: - $ref: "#/components/schemas/OptionsFormDefinition" - form_data: - $ref: "#/components/schemas/FormData" - required: [ definition, form_data ] + $ref: "#/components/schemas/Options" 404: description: Package not found. 500: @@ -591,6 +585,17 @@ components: - OPT_1 - OPT_2 + Options: + allOf: + - type: object + properties: + definition: + $ref: "#/components/schemas/OptionsFormDefinition" + form_data: + $ref: "#/components/schemas/FormData" + required: [ definition, form_data ] + - $ref: '#/components/schemas/PackageDependencies' + RequestBaseData: type: object properties: @@ -757,98 +762,119 @@ components: required: [ variant ] Attempt: - type: object - properties: - variant: - type: integer - minimum: 1 - description: Which variant of this question, between 1 and question.num_variants. - question_summary: - type: string - nullable: true - default: null - description: Plain-text summary of the question. - right_answer_summary: - type: string - nullable: true - default: null - description: Plain-text summary of the right answer. - ui: - type: object + allOf: + - type: object properties: - fields: - type: array - items: - type: object - properties: - name: - type: string - type: - type: string - description: Type of the input field. - default: - type: string - nullable: true - validation_regex: - type: string - nullable: true - description: Validate response with this regex. If the response does not match, the response might not be scorable. - required: - type: boolean - description: This field is required. Otherwise the response will not be scorable. - correct_response: - type: string - nullable: true - required: [ name, type, default, validation_regex, required, correct_response ] - text: - type: string - format: text/html - description: Answer input area - example: '' - include_inline_css: + variant: + type: integer + minimum: 1 + description: Which variant of this question, between 1 and question.num_variants. + question_summary: type: string nullable: true - include_css_file: + default: null + description: Plain-text summary of the question. + right_answer_summary: type: string nullable: true - include_javascript_modules: - type: array - default: [ ] - items: - type: string - call_javascript: - type: array - default: [ ] - items: - type: object - properties: - module: - type: string - function: - type: string - args: - type: string - required: [ module, function, args ] - files: - type: array - default: [ ] - items: - type: object - properties: - name: - type: string - description: file name - data: - type: string - format: base64 - description: file data - mime_type: - type: string - nullable: true - required: [ name, data, mime_type ] - required: [ fields, text ] - readOnly: true - required: [ variant, ui ] + default: null + description: Plain-text summary of the right answer. + ui: + type: object + properties: + fields: + type: array + items: + type: object + properties: + name: + type: string + type: + type: string + description: Type of the input field. + default: + type: string + nullable: true + validation_regex: + type: string + nullable: true + description: Validate response with this regex. If the response does not match, the response might not be scorable. + required: + type: boolean + description: This field is required. Otherwise the response will not be scorable. + correct_response: + type: string + nullable: true + required: [ name, type, default, validation_regex, required, correct_response ] + text: + type: string + format: text/html + description: Answer input area + example: '' + include_inline_css: + type: string + nullable: true + include_css_file: + type: string + nullable: true + javascript_calls: + type: array + default: [ ] + items: + type: object + properties: + module: + type: string + pattern: '^[a-zA-Z0-9_$/@.]+$' + description: JS module name like @[package namespace]/[package short name]/[module].js + function: + type: string + pattern: '^[a-zA-Z0-9_$]+$' + description: Name of a callable value within the JS module. + data: + type: string + nullable: true + description: JSON data given as argument to the function + if_role: + type: string + nullable: true + description: Function is called only if the user has this role. + enum: + - TEACHER + - DEVELOPER + - SCORER + - PROCTOR + if_feedback_type: + type: string + nullable: true + description: Function is called only if the user is allowed to view this type of feedback. + enum: + - SPECIFIC_FEEDBACK + - GENERAL_FEEDBACK + - RIGHT_ANSWER + - HINT + required: [ module, function, data, if_role, if_feedback_type ] + files: + type: array + default: [ ] + items: + type: object + properties: + name: + type: string + description: file name + data: + type: string + format: base64 + description: file data + mime_type: + type: string + nullable: true + required: [ name, data, mime_type ] + required: [ fields, text ] + readOnly: true + required: [ variant, ui ] + - $ref: "#/components/schemas/PackageDependencies" AttemptStarted: allOf: @@ -999,6 +1025,28 @@ components: required: [ scoring_state, scoring_code, score ] - $ref: "#/components/schemas/Attempt" + PackageDependencies: + type: object + properties: + package_dependencies: + type: array + items: + type: object + properties: + namespace: + type: string + description: Namespace of a package. + short_name: + type: string + description: Short name of a package. + hash: + type: string + description: SHA256 hash of a package. + required: [ namespace, short_name, hash ] + description: Listing of all QuestionPy packages that were involved for this response. This information will + be used by the LMS to resolve a QPy-URI and other references. + required: [ package_dependencies ] + AttemptAsyncScoringStatus: type: object properties: diff --git a/questionpy_common/api/attempt.py b/questionpy_common/api/attempt.py index 4f4d2d5..e8346bc 100644 --- a/questionpy_common/api/attempt.py +++ b/questionpy_common/api/attempt.py @@ -2,7 +2,7 @@ # QuestionPy is free software released under terms of the MIT license. See LICENSE.md. # (c) Technische Universität Berlin, innoCampus -from enum import Enum +from enum import Enum, StrEnum from typing import Annotated from pydantic import BaseModel, Field @@ -17,7 +17,13 @@ "AttemptUi", "CacheControl", "ClassifiedResponse", + "DisplayRole", + "FeedbackType", + "JsModuleCall", "ScoreModel", + "ScoredInputModel", + "ScoredInputState", + "ScoredSubquestionModel", "ScoringCode", ] @@ -28,6 +34,33 @@ class CacheControl(Enum): NO_CACHE = "NO_CACHE" +class DisplayRole(StrEnum): + DEVELOPER = "DEVELOPER" + PROCTOR = "PROCTOR" + SCORER = "SCORER" + TEACHER = "TEACHER" + + +class FeedbackType(StrEnum): + GENERAL_FEEDBACK = "GENERAL_FEEDBACK" + SPECIFIC_FEEDBACK = "SPECIFIC_FEEDBACK" + RIGHT_ANSWER = "RIGHT_ANSWER" + HINT = "HINT" + + +class JsModuleCall(BaseModel): + module: Annotated[str, Field(pattern=r"^[a-zA-Z0-9_$/@.]+$")] + """JS module name like @[package namespace]/[package short name]/[module].js""" + function: Annotated[str, Field(pattern=r"^[a-zA-Z0-9_$]+$")] + """Name of a callable value within the JS module.""" + data: str | None + """JSON data given as argument to the function""" + if_role: DisplayRole | None + """Function is only called if the user has this role.""" + if_feedback_type: FeedbackType | None + """Function is only called if the user is allowed to view this feedback type.""" + + class AttemptFile(BaseModel): name: str mime_type: str | None = None @@ -47,6 +80,7 @@ class AttemptUi(BaseModel): placeholders: dict[str, str] = {} """Names and values of the `` Path: if not (self._path and self._path.is_file()): self._path = await self.sources.get_path() return self._path + + async def get_zip_package_location(self) -> ZipPackageLocation: + return ZipPackageLocation(await self.get_path(), self.hash) diff --git a/questionpy_server/web/_routes/_attempts.py b/questionpy_server/web/_routes/_attempts.py index 824d0c9..c44ac19 100644 --- a/questionpy_server/web/_routes/_attempts.py +++ b/questionpy_server/web/_routes/_attempts.py @@ -7,12 +7,18 @@ from aiohttp import web from questionpy_common.environment import RequestUser -from questionpy_server.models import AttemptScoreArguments, AttemptStartArguments, AttemptViewArguments +from questionpy_server.models import ( + AttemptResponse, + AttemptScoreArguments, + AttemptScoredResponse, + AttemptStartArguments, + AttemptStartedResponse, + AttemptViewArguments, +) from questionpy_server.package import Package from questionpy_server.web._decorators import ensure_required_parts from questionpy_server.web._utils import pydantic_json_response from questionpy_server.web.app import QPyServer -from questionpy_server.worker.runtime.package_location import ZipPackageLocation if TYPE_CHECKING: from questionpy_server.worker import Worker @@ -27,12 +33,14 @@ async def post_attempt_start( ) -> web.Response: qpyserver = request.app[QPyServer.APP_KEY] - package_path = await package.get_path() + location = await package.get_zip_package_location() worker: Worker - async with qpyserver.worker_pool.get_worker(ZipPackageLocation(package_path), 0, data.context) as worker: + async with qpyserver.worker_pool.get_worker(location, 0, data.context) as worker: attempt = await worker.start_attempt(RequestUser(["de", "en"]), question_state.decode(), data.variant) + packages = worker.get_loaded_packages() - return pydantic_json_response(data=attempt, status=201) + resp = AttemptStartedResponse(**dict(attempt), package_dependencies=packages) + return pydantic_json_response(data=resp, status=201) @attempt_routes.post(r"/packages/{package_hash:\w+}/attempt/view") @@ -42,9 +50,9 @@ async def post_attempt_view( ) -> web.Response: qpyserver = request.app[QPyServer.APP_KEY] - package_path = await package.get_path() + location = await package.get_zip_package_location() worker: Worker - async with qpyserver.worker_pool.get_worker(ZipPackageLocation(package_path), 0, data.context) as worker: + async with qpyserver.worker_pool.get_worker(location, 0, data.context) as worker: attempt = await worker.get_attempt( request_user=RequestUser(["de", "en"]), question_state=question_state.decode(), @@ -52,8 +60,10 @@ async def post_attempt_view( scoring_state=data.scoring_state, response=data.response, ) + packages = worker.get_loaded_packages() - return pydantic_json_response(data=attempt, status=201) + resp = AttemptResponse(**dict(attempt), package_dependencies=packages) + return pydantic_json_response(data=resp, status=201) @attempt_routes.post(r"/packages/{package_hash:\w+}/attempt/score") @@ -63,9 +73,9 @@ async def post_attempt_score( ) -> web.Response: qpyserver = request.app[QPyServer.APP_KEY] - package_path = await package.get_path() + location = await package.get_zip_package_location() worker: Worker - async with qpyserver.worker_pool.get_worker(ZipPackageLocation(package_path), 0, data.context) as worker: + async with qpyserver.worker_pool.get_worker(location, 0, data.context) as worker: attempt_scored = await worker.score_attempt( request_user=RequestUser(["de", "en"]), question_state=question_state.decode(), @@ -73,5 +83,7 @@ async def post_attempt_score( scoring_state=data.scoring_state, response=data.response, ) + packages = worker.get_loaded_packages() - return pydantic_json_response(data=attempt_scored, status=201) + resp = AttemptScoredResponse(**dict(attempt_scored), package_dependencies=packages) + return pydantic_json_response(data=resp, status=201) diff --git a/questionpy_server/web/_routes/_files.py b/questionpy_server/web/_routes/_files.py index 3c2f295..a601b36 100644 --- a/questionpy_server/web/_routes/_files.py +++ b/questionpy_server/web/_routes/_files.py @@ -9,7 +9,6 @@ from questionpy_server.package import Package from questionpy_server.web._decorators import ensure_package from questionpy_server.web.app import QPyServer -from questionpy_server.worker.runtime.package_location import ZipPackageLocation if TYPE_CHECKING: from questionpy_server.worker import Worker @@ -29,8 +28,9 @@ async def serve_static_file(request: web.Request, package: Package) -> web.Respo # TODO: Support static files in non-main packages by using namespace and short_name. raise HTTPNotImplemented(text="Static file retrieval from non-main packages is not supported yet.") + location = await package.get_zip_package_location() worker: Worker - async with qpy_server.worker_pool.get_worker(ZipPackageLocation(await package.get_path()), 0, None) as worker: + async with qpy_server.worker_pool.get_worker(location, 0, None) as worker: try: file = await worker.get_static_file(path) except FileNotFoundError as e: diff --git a/questionpy_server/web/_routes/_packages.py b/questionpy_server/web/_routes/_packages.py index d416702..f2cb4a1 100644 --- a/questionpy_server/web/_routes/_packages.py +++ b/questionpy_server/web/_routes/_packages.py @@ -13,7 +13,6 @@ from questionpy_server.web._decorators import ensure_package, ensure_required_parts from questionpy_server.web._utils import pydantic_json_response from questionpy_server.web.app import QPyServer -from questionpy_server.worker.runtime.package_location import ZipPackageLocation if TYPE_CHECKING: from questionpy_server.worker import Worker @@ -47,14 +46,17 @@ async def post_options( """Get the options form definition that allow a question creator to customize a question.""" qpyserver = request.app[QPyServer.APP_KEY] - package_path = await package.get_path() + location = await package.get_zip_package_location() worker: Worker - async with qpyserver.worker_pool.get_worker(ZipPackageLocation(package_path), 0, data.context) as worker: + async with qpyserver.worker_pool.get_worker(location, 0, data.context) as worker: definition, form_data = await worker.get_options_form( RequestUser(["de", "en"]), question_state.decode() if question_state else None ) + packages = worker.get_loaded_packages() - return pydantic_json_response(data=QuestionEditFormResponse(definition=definition, form_data=form_data)) + return pydantic_json_response( + data=QuestionEditFormResponse(definition=definition, form_data=form_data, package_dependencies=packages) + ) @package_routes.post(r"/packages/{package_hash:\w+}/question") @@ -64,9 +66,9 @@ async def post_question( ) -> web.Response: qpyserver = request.app[QPyServer.APP_KEY] - package_path = await package.get_path() + location = await package.get_zip_package_location() worker: Worker - async with qpyserver.worker_pool.get_worker(ZipPackageLocation(package_path), 0, data.context) as worker: + async with qpyserver.worker_pool.get_worker(location, 0, data.context) as worker: question = await worker.create_question_from_options( RequestUser(["de", "en"]), question_state.decode() if question_state else None, data.form_data ) diff --git a/questionpy_server/worker/__init__.py b/questionpy_server/worker/__init__.py index aaa81a8..e6b01e6 100644 --- a/questionpy_server/worker/__init__.py +++ b/questionpy_server/worker/__init__.py @@ -12,7 +12,7 @@ from questionpy_common.elements import OptionsFormDefinition from questionpy_common.environment import RequestUser, WorkerResourceLimits from questionpy_common.manifest import PackageFile -from questionpy_server.models import QuestionCreated +from questionpy_server.models import LoadedPackage, QuestionCreated from questionpy_server.utils.manifest import ComparableManifest from questionpy_server.worker.runtime.messages import MessageToServer, MessageToWorker from questionpy_server.worker.runtime.package_location import PackageLocation @@ -58,6 +58,8 @@ def __init__(self, package: PackageLocation, limits: WorkerResourceLimits | None self.package = package self.limits = limits self.state = WorkerState.NOT_RUNNING + self.loaded_packages: list[LoadedPackage] = [] + """All loaded packages in the worker.""" @abstractmethod async def start(self) -> None: @@ -188,3 +190,11 @@ async def get_static_file(self, path: str) -> PackageFileData: @abstractmethod async def get_static_file_index(self) -> dict[str, PackageFile]: """Returns the index of static files as declared in the package's manifest.""" + + def get_loaded_packages(self, *, only_with_hash: bool = True) -> list[LoadedPackage]: + """Get all loaded packages in the worker. + + Args: + only_with_hash: Only return packages that have a hash (i.e. only ZIP packages). + """ + return [p for p in self.loaded_packages if p.hash is not None or not only_with_hash] diff --git a/questionpy_server/worker/impl/_base.py b/questionpy_server/worker/impl/_base.py index e8701ed..46cc812 100644 --- a/questionpy_server/worker/impl/_base.py +++ b/questionpy_server/worker/impl/_base.py @@ -17,7 +17,7 @@ from questionpy_common.elements import OptionsFormDefinition from questionpy_common.environment import RequestUser from questionpy_common.manifest import Manifest, PackageFile -from questionpy_server.models import QuestionCreated +from questionpy_server.models import LoadedPackage, QuestionCreated from questionpy_server.utils.manifest import ComparableManifest from questionpy_server.worker import PackageFileData, Worker, WorkerState from questionpy_server.worker.exception import ( @@ -101,11 +101,15 @@ async def _initialize(self) -> None: InitWorker.Response, self._init_worker_timeout, ) - await self.send_and_wait_for_response( + loaded = await self.send_and_wait_for_response( LoadQPyPackage(location=self.package, main=True), LoadQPyPackage.Response, self._load_qpy_package_timeout, ) + packagehash = self.package.hash if isinstance(self.package, ZipPackageLocation) else None + self.loaded_packages.append( + LoadedPackage(namespace=loaded.nssn.namespace, short_name=loaded.nssn.short_name, hash=packagehash) + ) except BaseWorkerError as e: msg = "Worker has exited before or during initialization." raise WorkerStartError(msg, temporary=e.temporary) from e diff --git a/questionpy_server/worker/runtime/manager.py b/questionpy_server/worker/runtime/manager.py index 0f53141..5a9ffcc 100644 --- a/questionpy_server/worker/runtime/manager.py +++ b/questionpy_server/worker/runtime/manager.py @@ -130,7 +130,7 @@ def on_msg_load_qpy_package(self, msg: LoadQPyPackage) -> MessageToServer: nssn = PackageNamespaceAndShortName(package.manifest.namespace, package.manifest.short_name) self._loaded_packages[nssn] = package - return LoadQPyPackage.Response() + return LoadQPyPackage.Response(nssn=nssn) def on_msg_get_qpy_package_manifest(self, msg: GetQPyPackageManifest) -> MessageToServer: if not self._env: diff --git a/questionpy_server/worker/runtime/messages.py b/questionpy_server/worker/runtime/messages.py index 14c56fb..c314ac9 100644 --- a/questionpy_server/worker/runtime/messages.py +++ b/questionpy_server/worker/runtime/messages.py @@ -12,7 +12,7 @@ from questionpy_common.api.qtype import InvalidQuestionStateError, OptionsFormValidationError from questionpy_common.api.question import QuestionModel from questionpy_common.elements import OptionsFormDefinition -from questionpy_common.environment import RequestUser, WorkerResourceLimits +from questionpy_common.environment import PackageNamespaceAndShortName, RequestUser, WorkerResourceLimits from questionpy_common.error import QPyBaseError from questionpy_common.manifest import Manifest from questionpy_server.worker.runtime.package_location import PackageLocation @@ -113,6 +113,7 @@ class Response(MessageToServer): """Success message in return to LoadQPyPackage.""" message_id: ClassVar[MessageIds] = MessageIds.LOADED_QPY_PACKAGE + nssn: PackageNamespaceAndShortName class GetQPyPackageManifest(MessageToWorker): diff --git a/questionpy_server/worker/runtime/package_location.py b/questionpy_server/worker/runtime/package_location.py index 8213e4c..0916b97 100644 --- a/questionpy_server/worker/runtime/package_location.py +++ b/questionpy_server/worker/runtime/package_location.py @@ -14,10 +14,11 @@ class ZipPackageLocation: """The path to a 'regular', zip-formatted QuestionPy package.""" path: Path + hash: str kind: Literal["zip"] = field(default="zip", init=False) def __str__(self) -> str: - return str(self.path) + return f"{self.path} (sha256:{self.hash})" @dataclass diff --git a/tests/conftest.py b/tests/conftest.py index 4a7ada9..f0dda42 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -6,7 +6,6 @@ import tempfile from collections.abc import Iterable from dataclasses import dataclass -from hashlib import sha256 from pathlib import Path from zipfile import ZipFile @@ -14,8 +13,9 @@ from aiohttp.pytest_plugin import AiohttpClient from aiohttp.test_utils import TestClient -from questionpy_common.constants import DIST_DIR, MANIFEST_FILENAME, KiB, MiB +from questionpy_common.constants import DIST_DIR, MANIFEST_FILENAME, MiB from questionpy_common.manifest import PackageFile +from questionpy_server.hash import calculate_hash from questionpy_server.settings import ( CollectorSettings, GeneralSettings, @@ -33,22 +33,13 @@ from questionpy_server.worker.runtime.package_location import DirPackageLocation, ZipPackageLocation -def get_file_hash(path: Path) -> str: - hash_value = sha256() - with path.open("rb") as file: - while chunk := file.read(4 * KiB): - hash_value.update(chunk) - return hash_value.hexdigest() - - @dataclass class TestZipPackage(ZipPackageLocation): __test__ = False def __init__(self, path: Path): - super().__init__(path) + super().__init__(path, calculate_hash(path)) - self.hash = get_file_hash(self.path) with ZipFile(self.path) as package: self.manifest = ComparableManifest.model_validate_json(package.read(f"{DIST_DIR}/{MANIFEST_FILENAME}")) diff --git a/tests/questionpy_common/test_elements.py b/tests/questionpy_common/test_elements.py index 11ad839..ab5ed20 100644 --- a/tests/questionpy_common/test_elements.py +++ b/tests/questionpy_common/test_elements.py @@ -27,8 +27,9 @@ is_form_element, ) from questionpy_server.collector import PackageCollection +from questionpy_server.hash import calculate_hash from questionpy_server.models import RequestErrorCode -from tests.conftest import get_file_hash, package_dir, test_data_path +from tests.conftest import package_dir, test_data_path from .factories import ( CheckboxElementFactory, @@ -45,7 +46,7 @@ ) _PACKAGE = package_dir / "package_1.qpy" -_PACKAGE_HASH = get_file_hash(_PACKAGE) +_PACKAGE_HASH = calculate_hash(_PACKAGE) _METHOD = "POST" _URL = f"packages/{_PACKAGE_HASH}/options" diff --git a/tests/questionpy_server/collector/test_local_collector.py b/tests/questionpy_server/collector/test_local_collector.py index 9b662a7..8779b81 100644 --- a/tests/questionpy_server/collector/test_local_collector.py +++ b/tests/questionpy_server/collector/test_local_collector.py @@ -18,8 +18,9 @@ from questionpy_server import WorkerPool from questionpy_server.collector.indexer import Indexer from questionpy_server.collector.local_collector import LocalCollector +from questionpy_server.hash import calculate_hash from questionpy_server.package import Package -from tests.conftest import PACKAGE, PACKAGE_2, get_file_hash +from tests.conftest import PACKAGE, PACKAGE_2 def create_local_collector(tmp_path_factory: TempPathFactory) -> tuple[LocalCollector, Path]: @@ -115,7 +116,7 @@ async def test_package_exists_before_init(tmp_path_factory: TempPathFactory) -> actual_package_path = await local_collector.get_path(package) assert actual_package_path.is_file() assert str(actual_package_path) == package_path - assert get_file_hash(actual_package_path) == package.hash + assert calculate_hash(actual_package_path) == package.hash async def test_package_gets_created(tmp_path_factory: TempPathFactory) -> None: