From df798425c0d7e36570483632c61b3ef6f88adc79 Mon Sep 17 00:00:00 2001 From: Jan-Lukas Wynen Date: Thu, 11 Jan 2024 13:53:19 +0100 Subject: [PATCH] Inject template config Before, the notebook functions would get the default app config and would not allow for overriding it in tests. --- src/sciwyrm/config.py | 6 ++- src/sciwyrm/main.py | 29 +++++++++++++- src/sciwyrm/notebook.py | 57 +++++++++++++++------------ src/sciwyrm/templates.py | 85 ++++++++++++++++++++++++---------------- 4 files changed, 114 insertions(+), 63 deletions(-) diff --git a/src/sciwyrm/config.py b/src/sciwyrm/config.py index 75c7fc2..2e2c952 100644 --- a/src/sciwyrm/config.py +++ b/src/sciwyrm/config.py @@ -81,7 +81,11 @@ def settings_customise_sources( @lru_cache(maxsize=1) def app_config() -> AppConfig: - """Return the application config.""" + """Return the application config. + + This function should only be called by the app. + Otherwise, tests cannot override the configuration. + """ from .logging import get_logger config = AppConfig() diff --git a/src/sciwyrm/main.py b/src/sciwyrm/main.py index bf7b9a4..4b5a894 100644 --- a/src/sciwyrm/main.py +++ b/src/sciwyrm/main.py @@ -6,12 +6,19 @@ from typing import Annotated from fastapi import Depends, FastAPI, Request, Response +from fastapi.exceptions import RequestValidationError from fastapi.responses import JSONResponse from fastapi.templating import Jinja2Templates +from pydantic import ValidationError from . import notebook from .config import AppConfig, app_config -from .templates import get_templates, list_notebook_templates, notebook_template_path +from .templates import ( + get_notebook_template_config, + get_templates, + list_notebook_templates, + notebook_template_path, +) app = FastAPI() @@ -24,11 +31,29 @@ async def list_templates( return JSONResponse(list_notebook_templates(config)) +def _inject_app_config( + spec: notebook.NotebookSpec, config: Annotated[AppConfig, Depends(app_config)] +) -> notebook.NotebookSpecWithConfig: + try: + return spec.with_config( + get_notebook_template_config( + spec.template_name, spec.template_version, config + ) + ) + except ValidationError as exc: + # FastAPI cannot handle a ValidationError at this point. + # So make it look like this came from the initial spec. + errors = exc.errors() + for error in errors: + error["input"].pop("config") + raise RequestValidationError(errors) from None + + @app.post("/notebook", response_class=JSONResponse) async def format_notebook( request: Request, - spec: notebook.NotebookSpec, templates: Annotated[Jinja2Templates, Depends(get_templates)], + spec: notebook.NotebookSpecWithConfig = Depends(_inject_app_config), # noqa: B008 ) -> Response: """Format and return a notebook.""" formatted = templates.TemplateResponse( diff --git a/src/sciwyrm/notebook.py b/src/sciwyrm/notebook.py index 9b69490..0f030d2 100644 --- a/src/sciwyrm/notebook.py +++ b/src/sciwyrm/notebook.py @@ -2,15 +2,16 @@ # Copyright (c) 2024 SciCat Project (https://github.com/SciCatProject/sciwyrm) """Notebook handling.""" +from __future__ import annotations + from datetime import datetime, timezone from typing import Any import jsonschema -from pydantic import BaseModel, ValidationInfo, field_validator +from pydantic import BaseModel, model_validator from pydantic_core import PydanticCustomError -from .config import app_config -from .templates import get_template_config, notebook_template_hash +from .templates import NotebookTemplateConfig class NotebookSpec(BaseModel): @@ -20,28 +21,36 @@ class NotebookSpec(BaseModel): template_version: str parameters: dict[str, Any] - @field_validator("parameters") - @classmethod - def validate_parameters( - cls, parameters: Any, info: ValidationInfo - ) -> dict[str, Any]: - """Validate parameters against the template schema.""" - if not isinstance(parameters, dict): - raise AssertionError("'parameters' must be a dict.") + def with_config(self, config: NotebookTemplateConfig) -> NotebookSpecWithConfig: + """Return a new spec with template config.""" + return NotebookSpecWithConfig( + **{**self.model_dump(), "config": config}, + ) + + +class NotebookSpecWithConfig(NotebookSpec): + """Internal spec with added template config. - schema = get_template_config( - info.data["template_name"], info.data["template_version"], app_config() - )["parameter_schema"] + This should not be exposed to the API because notebook template configs + are provided by the server. + """ + + config: NotebookTemplateConfig + + @model_validator(mode="after") + def validate_parameters(self) -> NotebookSpecWithConfig: + """Validate parameters against the template schema.""" + schema = self.config.parameter_schema try: - jsonschema.validate(parameters, schema) + jsonschema.validate(self.parameters, schema) except jsonschema.ValidationError as err: raise PydanticCustomError( "Validation Error", "{message}", { "message": err.message, - "template_name": info.data["template_name"], - "template_version": info.data["template_version"], + "template_name": self.template_name, + "template_version": self.template_version, "instance": err.instance, "jsonpath": err.json_path, "schema": err.schema, @@ -50,28 +59,24 @@ def validate_parameters( "validator_value": err.validator_value, }, ) from None - return parameters + return self -def render_context(spec: NotebookSpec) -> dict[str, Any]: +def render_context(spec: NotebookSpecWithConfig) -> dict[str, Any]: """Return a dict that can be used to render a notebook template.""" context = spec.parameters | notebook_metadata(spec) return {key.upper(): value for key, value in context.items()} -def notebook_metadata(spec: NotebookSpec) -> dict[str, Any]: +def notebook_metadata(spec: NotebookSpecWithConfig) -> dict[str, Any]: """Return metadata for a requested notebook. Here, metadata is any data that was not explicitly requested by the user. """ - app_conf = app_config() - config = get_template_config(spec.template_name, spec.template_version, app_conf) return { "template_name": spec.template_name, "template_version": spec.template_version, - "template_authors": config["authors"], + "template_authors": [author.model_dump() for author in spec.config.authors], "template_rendered": datetime.now(tz=timezone.utc).isoformat(), - "template_hash": notebook_template_hash( - spec.template_name, spec.template_version, app_conf - ), + "template_hash": spec.config.template_hash, } diff --git a/src/sciwyrm/templates.py b/src/sciwyrm/templates.py index a81783e..233b21f 100644 --- a/src/sciwyrm/templates.py +++ b/src/sciwyrm/templates.py @@ -11,29 +11,70 @@ from fastapi import Depends from fastapi.templating import Jinja2Templates from jinja2 import Environment, FileSystemLoader +from pydantic import BaseModel, EmailStr from .config import AppConfig, app_config +class Author(BaseModel): + """Author of the template.""" + + name: str + email: EmailStr | None = None + + +class NotebookTemplateConfig(BaseModel): + """Template configuration.""" + + name: str + version: str + description: str + authors: list[Author] + parameter_schema: dict[str, Any] + template_hash: str + + def get_templates(config: Annotated[AppConfig, Depends(app_config)]) -> Jinja2Templates: """Return a handler for loading and rendering templates.""" return _make_template_handler(config.template_dir) -def get_template_config( - name: str, version: str, config: AppConfig, category: str = "notebook" -) -> dict[str, Any]: - """Return a template configuration.""" - return _load_template_config( - config.template_dir.joinpath(category, f"{name}_v{version}.json") +@lru_cache(maxsize=1) +def _make_template_handler(template_dir: Path) -> Jinja2Templates: + from . import filters + from .logging import get_logger + + get_logger().info("Loading templates from %s", template_dir) + templates = Jinja2Templates( + env=Environment( + loader=FileSystemLoader(template_dir), + autoescape=True, + ) ) + templates.env.filters["quote"] = filters.quote + templates.env.filters["je"] = filters.json_escape + return templates + + +def get_notebook_template_config( + name: str, version: str, config: AppConfig +) -> NotebookTemplateConfig: + """Return a template configuration.""" + return _load_notebook_template_config(name, version, config.template_dir) @lru_cache() -def _load_template_config(path: Path) -> dict[str, Any]: +def _load_notebook_template_config( + name: str, version: str, template_dir: Path +) -> NotebookTemplateConfig: # AppConfig cannot be hashed and used with lru_cache. + path = template_dir.joinpath("notebook", f"{name}_v{version}.json") with path.open() as f: - return json.load(f) + fields = json.load(f) + fields["name"] = name + fields["version"] = version + fields["template_hash"] = _notebook_template_hash(path) + return NotebookTemplateConfig(**fields) def notebook_template_path(name: str, version: str) -> str: @@ -63,35 +104,11 @@ def list_notebook_templates(config: AppConfig) -> list[dict[str, str]]: ] -def notebook_template_hash(name: str, version: str, config: AppConfig) -> str: +def _notebook_template_hash(path: Path) -> str: """Return a hash for a notebook template.""" - return ( - "blake2b:" - + hashlib.blake2b( - config.template_dir.joinpath( - notebook_template_path(name, version) - ).read_bytes() - ).hexdigest() - ) + return "blake2b:" + hashlib.blake2b(path.read_bytes()).hexdigest() def _split_notebook_template_name(full_name: str) -> dict[str, str]: name, version = full_name.split("_v") return {"name": name, "version": version} - - -@lru_cache(maxsize=1) -def _make_template_handler(template_dir: Path) -> Jinja2Templates: - from . import filters - from .logging import get_logger - - get_logger().info("Loading templates from %s", template_dir) - templates = Jinja2Templates( - env=Environment( - loader=FileSystemLoader(template_dir), - autoescape=True, - ) - ) - templates.env.filters["quote"] = filters.quote - templates.env.filters["je"] = filters.json_escape - return templates