diff --git a/alembic/versions/a3f1c7d94e02_normalise_ids_metadata_to_list.py b/alembic/versions/a3f1c7d94e02_normalise_ids_metadata_to_list.py new file mode 100644 index 00000000..96173b3d --- /dev/null +++ b/alembic/versions/a3f1c7d94e02_normalise_ids_metadata_to_list.py @@ -0,0 +1,92 @@ +"""normalise_ids_metadata_to_list + +Convert the ``ids`` and ``input_ids`` simulation metadata from their display-string +form, ``"[core_profiles, equilibrium]"``, to a real list of IDS names. + +Revision ID: a3f1c7d94e02 +Revises: 6fb9b8fbac38 +Create Date: 2026-09-01 00:00:00.000000 + +""" + +import json +from typing import Any, Sequence, Union + +from sqlalchemy import text + +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = "a3f1c7d94e02" +down_revision: Union[str, Sequence[str], None] = "6fb9b8fbac38" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + +IDS_KEYS = ("ids", "input_ids") + +_SELECT = text("SELECT id, metadata FROM simulations WHERE metadata IS NOT NULL") +_UPDATE = text("UPDATE simulations SET metadata = :metadata WHERE id = :sim_id") + + +def _as_dict(value: Any) -> Any: + """Return the metadata column value as a dict. + + SQLite hands back the raw JSON text while PostgreSQL decodes JSONB for us. + """ + if isinstance(value, (bytes, bytearray, memoryview)): + value = bytes(value).decode("utf-8") + if isinstance(value, str): + try: + value = json.loads(value) + except ValueError: + return None + return value if isinstance(value, dict) else None + + +def _split_ids(value: str) -> list: + text_value = value.strip() + if text_value.startswith("[") and text_value.endswith("]"): + text_value = text_value[1:-1] + return [name.strip() for name in text_value.split(",") if name.strip()] + + +def _convert(convert_value) -> None: + conn = op.get_bind() + rows = conn.execute(_SELECT).fetchall() + + for sim_id, metadata in rows: + meta_dict = _as_dict(metadata) + if not meta_dict: + continue + + changed = False + for key in IDS_KEYS: + if key not in meta_dict: + continue + new_value = convert_value(meta_dict[key]) + if new_value is not None and new_value != meta_dict[key]: + meta_dict[key] = new_value + changed = True + + if changed: + conn.execute(_UPDATE, {"metadata": json.dumps(meta_dict), "sim_id": sim_id}) + + +def upgrade() -> None: + """Turn stringified IDS lists into real lists.""" + + def to_list(value: Any) -> Any: + return _split_ids(value) if isinstance(value, str) else None + + _convert(to_list) + + +def downgrade() -> None: + """Restore the display-string form of the IDS lists.""" + + def to_string(value: Any) -> Any: + if isinstance(value, list): + return "[{}]".format(", ".join(str(el) for el in value)) + return None + + _convert(to_string) diff --git a/src/simdb/cli/commands/remote.py b/src/simdb/cli/commands/remote.py index 1fdbf49a..c1ba61e3 100644 --- a/src/simdb/cli/commands/remote.py +++ b/src/simdb/cli/commands/remote.py @@ -11,6 +11,7 @@ from simdb.cli.remote_api import RemoteAPI from simdb.database.models.simulation import Simulation +from simdb.database.models.watcher import Watcher from simdb.notifications import Notification from . import check_meta_args, pass_config @@ -270,6 +271,13 @@ def config_set_option(config: "Config", name: str, option: str, value: str): config.save() +_NOTIFICATION_NAMES = { + value: notification.name + for notification, value in Watcher.NOTIFICATION_CHOICES.items() +} +"""Notification names by the single character the remote reports them as.""" + + @remote.group(cls=RemoteSubGroup) def watcher(): """Manage simulation watchers on REMOTE SimDB server.""" @@ -285,7 +293,10 @@ def list_watchers(api: RemoteAPI, sim_id: str): if watchers: click.echo(f"Watchers for simulation {sim_id}:") for watcher in watchers: - click.echo(watcher) + notification = _NOTIFICATION_NAMES.get( + watcher.notification, watcher.notification + ) + click.echo(f"{watcher.username} <{watcher.email}> ({notification})") else: click.echo(f"no watchers found for simulation {sim_id}") diff --git a/src/simdb/cli/commands/simulation.py b/src/simdb/cli/commands/simulation.py index 59bd0bdd..c6f523b3 100644 --- a/src/simdb/cli/commands/simulation.py +++ b/src/simdb/cli/commands/simulation.py @@ -3,7 +3,7 @@ import urllib.parse from itertools import chain from pathlib import Path -from typing import Any, List, Optional, Tuple, Type +from typing import Any, List, Optional, Tuple import appdirs import click @@ -200,19 +200,22 @@ def simulation_ingest(config: Config, manifest_file: str, alias: str): click.echo("ALIAS: " + simulation.alias + "\nUUID: " + str(simulation.uuid)) -def n_required_args_adaptor(n) -> Type[click.Command]: - class NRequiredArgs(click.Command): - NArgs = n +class OptionalRemoteCommand(click.Command): + """A command declared as `[REMOTE] ARG...` whose REMOTE may be left out.""" - def parse_args(self, ctx, args): - if len(args) == self.NArgs: - args.insert(0, "") - super().parse_args(ctx, args) + def parse_args(self, ctx, args): + arguments = [p for p in self.get_params(ctx) if isinstance(p, click.Argument)] + if self._count_values_given(ctx, args, arguments) < len(arguments): + args = ["", *args] + super().parse_args(ctx, args) - return NRequiredArgs + def _count_values_given(self, ctx, args, arguments) -> int: + """Count how many of the ARGUMENTS the command line provides a value for.""" + values = self.make_parser(ctx).parse_args(list(args))[0] + return sum(1 for argument in arguments if values.get(argument.name) is not None) -@simulation.command("push", cls=n_required_args_adaptor(1)) +@simulation.command("push", cls=OptionalRemoteCommand) @pass_config @click.argument("remote", required=False) @click.argument("sim_id") @@ -257,7 +260,7 @@ def simulation_push( click.echo(f"Successfully pushed simulation {simulation.uuid}") -@simulation.command("pull", cls=n_required_args_adaptor(2)) +@simulation.command("pull", cls=OptionalRemoteCommand) @pass_config @click.argument("remote", required=False) @click.argument("sim_id") @@ -380,7 +383,7 @@ def simulation_query( ) -@simulation.command("data", cls=n_required_args_adaptor(2)) +@simulation.command("data", cls=OptionalRemoteCommand) @pass_config @click.argument("remote", required=False) @click.argument("sim_id") @@ -418,37 +421,36 @@ def simulation_data( except Exception as err: raise click.ClickException(str(err)) from err - click.echo(f"simulation : {result['simulation']}") - click.echo(f"path : {result['path']} (occurrence {result['occurrence']})") + click.echo(f"simulation : {result.simulation}") + click.echo(f"path : {result.path} (occurrence {result.occurrence})") - coordinates = result.get("coordinates") or [] + field = result.field + coordinates = result.coordinates plot_coordinate = next( ( coord for coord in coordinates - if isinstance(coord.get("data"), list) - and isinstance(result["field"].get("data"), list) - and len(coord["data"]) == len(result["field"]["data"]) + if isinstance(coord.data, list) + and isinstance(field.data, list) + and len(coord.data) == len(field.data) ), None, ) - field_is_1d = is_numeric_1d(result["field"].get("data")) + field_is_1d = is_numeric_1d(field.data) if field_is_1d: - show_quantity_textual_plot( - result["field"], label="field", x_quantity=plot_coordinate - ) + show_quantity_textual_plot(field, label="field", x_quantity=plot_coordinate) else: - print_quantity(result["field"], label="field") + print_quantity(field, label="field") if config.verbose and coordinates: for coord in coordinates: - if field_is_1d and is_numeric_1d(coord.get("data")): + if field_is_1d and is_numeric_1d(coord.data): continue - if isinstance(coord.get("data"), list): - print_quantity(coord, label=f"coord {coord['name']}", show_stats=False) + if isinstance(coord.data, list): + print_quantity(coord, label=f"coord {coord.name}", show_stats=False) -@simulation.command("validate", cls=n_required_args_adaptor(1)) +@simulation.command("validate", cls=OptionalRemoteCommand) @pass_config @click.argument("remote", required=False) @click.argument("sim_id") diff --git a/src/simdb/cli/commands/utils.py b/src/simdb/cli/commands/utils.py index df460ae3..71d0b38a 100644 --- a/src/simdb/cli/commands/utils.py +++ b/src/simdb/cli/commands/utils.py @@ -8,6 +8,8 @@ from rich.table import Table from rich.text import Text +from simdb.remote.models import QuantityData, SimulationTraceData + if TYPE_CHECKING: # Only importing these for type checking and documentation generation in order to # speed up runtime startup. @@ -50,9 +52,9 @@ def is_numeric_1d(data: Any) -> bool: return isinstance(data, list) and bool(data) and all(_is_numeric(v) for v in data) -def _quantity_axis_label(q: dict, fallback: str = "") -> str: - name = q.get("name") or fallback - units = q.get("units") or "-" +def _quantity_axis_label(q: QuantityData, fallback: str = "") -> str: + name = q.name or fallback + units = q.units or "-" label = str(name).rsplit("/", 1)[-1] or str(name) return f"{label} [{units}]" @@ -140,14 +142,14 @@ def _plot_panel( def show_quantity_textual_plot( - q: dict, + q: QuantityData, label: str = "", - x_quantity: Optional[dict] = None, + x_quantity: Optional[QuantityData] = None, ) -> None: - """Print line plot for a 1-D numeric QuantityData dict.""" - name = q["name"] - units = q["units"] or "-" - data = q["data"] + """Print line plot for a 1-D numeric QuantityData.""" + name = q.name + units = q.units or "-" + data = q.data if not is_numeric_1d(data): print_quantity(q, label=label) return @@ -158,10 +160,10 @@ def show_quantity_textual_plot( xlabel = "index [-]" if ( x_quantity - and is_numeric_1d(x_quantity.get("data")) - and len(x_quantity["data"]) == len(y_values) + and is_numeric_1d(x_quantity.data) + and len(x_quantity.data) == len(y_values) ): - x_values = [float(value) for value in x_quantity["data"]] + x_values = [float(value) for value in x_quantity.data] xlabel = _quantity_axis_label(x_quantity, fallback="x") title = label or name @@ -191,11 +193,11 @@ def show_quantity_textual_plot( print_quantity(q, label=label) -def print_quantity(q: dict, label: str = "", show_stats: bool = True) -> None: - """Print a QuantityData dict with array display and stats.""" - name = q["name"] - units = q["units"] or "-" - data = q["data"] +def print_quantity(q: QuantityData, label: str = "", show_stats: bool = True) -> None: + """Print a QuantityData with array display and stats.""" + name = q.name + units = q.units or "-" + data = q.data title = f"[bold]{label or name}[/bold] [dim]\\[{units}][/dim]" if not isinstance(data, list): @@ -241,7 +243,10 @@ def _format_meta_value(meta_value: Any, max_len: int) -> str: if isinstance(meta_value, list): values = [] for i, v in enumerate(meta_value): - values.append(f"{v:.2f}") + if isinstance(v, bool) or not isinstance(v, (int, float)): + values.append(str(v)) + else: + values.append(f"{v:.2f}") if i >= max_len - 1: values.append("...") break @@ -334,44 +339,36 @@ def print_simulations( ) -def _print_trace_sim(trace_data: dict, indentation: int): +def _print_trace_sim(trace_data: SimulationTraceData, indentation: int): spaces = " " * indentation - if "error" in trace_data: - error = trace_data["error"] - click.echo(f"{spaces}{error}") - return - - uuid = trace_data["uuid"] - alias = trace_data["alias"] - status = trace_data.get("status", "unknown") + status = trace_data.status or "unknown" - click.echo(f"{spaces}Simulation: {uuid}") - click.echo(f"{spaces} Alias: {alias}") + click.echo(f"{spaces}Simulation: {trace_data.uuid}") + click.echo(f"{spaces} Alias: {trace_data.alias}") click.echo(f"{spaces} Status: {status}") - status_on_name = status + "_on" - if status_on_name in trace_data: - status_on = trace_data[status_on_name] + status_on_name = status.replace(" ", "_") + "_on" + status_on = getattr(trace_data, status_on_name, None) + if status_on is not None: label = status_on_name.replace("_", " ").capitalize() click.echo(f"{spaces}{label}: {status_on}") - if "replaces" in trace_data: - if "replaces_reason" in trace_data: - replaces_reason = trace_data["replaces_reason"] - click.echo(f"{spaces}Replaces: (reason: {replaces_reason})") + if trace_data.replaces is not None: + if trace_data.replaces_reason is not None: + click.echo(f"{spaces}Replaces: (reason: {trace_data.replaces_reason})") else: click.echo(f"{spaces}Replaces:") - _print_trace_sim(trace_data["replaces"], indentation + 2) + _print_trace_sim(trace_data.replaces, indentation + 2) -def print_trace(trace_data: dict) -> None: +def print_trace(trace_data: Optional[SimulationTraceData]) -> None: """ Print the simulation trace data to the console. - :param trace_data: A dictionary containing the simulation trace data. + :param trace_data: The trace data of the simulation. :return: None """ - if not trace_data: + if trace_data is None: click.echo("No simulations trace found") return diff --git a/src/simdb/cli/remote_api.py b/src/simdb/cli/remote_api.py index 5456cde8..cf3774cb 100644 --- a/src/simdb/cli/remote_api.py +++ b/src/simdb/cli/remote_api.py @@ -4,13 +4,13 @@ import hashlib import io import itertools -import json import os import pickle import shutil import sys import uuid from collections import defaultdict +from contextlib import contextmanager from io import BytesIO from pathlib import Path from typing import ( @@ -19,7 +19,9 @@ Any, Callable, Dict, + FrozenSet, Iterable, + Iterator, List, Optional, Tuple, @@ -30,14 +32,51 @@ import appdirs import click import requests +from pydantic import BaseModel, ValidationError from requests.auth import AuthBase from semantic_version import Version from simdb.config import Config from simdb.database.models import Simulation from simdb.imas.utils import SimDBUrl, imas_files -from simdb.json import CustomDecoder, CustomEncoder -from simdb.remote import CLIENT_API_VERSIONS, APIConstants +from simdb.remote import CLIENT_API_VERSIONS +from simdb.remote.models import ( + ChunkInfo, + FileGetDataResponse, + FileRegistrationData, + FileRegistrationItem, + FileUploadData, + ImasDataQueryParams, + ImasDataResponse, + IndexResponse, + MetadataDataList, + MetadataDeleteData, + MetadataDeleteResponse, + MetadataPatchData, + MetadataValue, + PaginatedResponse, + PaginationData, + SimulationData, + SimulationDataResponse, + SimulationDeleteResponse, + SimulationListItem, + SimulationPatchResponse, + SimulationPostData, + SimulationTraceData, + StagingDirectoryResponse, + StatusPatchData, + TokenResponse, + UploadOptions, + ValidationResult, + ValidationSchemaList, + WatcherData, + WatcherDeleteRequest, + WatcherDeleteResponse, + WatcherGetResponse, + WatcherPostRequest, + WatcherPostResponse, + coerce_ids_list, +) from .manifest import DataType @@ -93,6 +132,18 @@ def wrapped_func(*args, **kwargs): This might indicate an invalid SimDB URL or the existence of a firewall. """ ) from None + except ValidationError as ex: + if any(error["type"] == "json_invalid" for error in ex.errors()): + raise FailedConnection( + """\ +Invalid JSON returned from request endpoint + +This might indicate an invalid SimDB URL or the existence of a firewall. + """ + ) from None + raise RemoteError( + f"Unexpected data exchanged with the remote:\n{ex}" + ) from None return wrapped_func @@ -124,10 +175,14 @@ def _push_simulation_v1_2(self, ...): compatible with. This keeps the current protocol as the primary, most-visible code path and confines backwards-compatibility handling to clearly named shims. + A method that does not support the negotiated version sends its requests to the + highest version it does support that the remote provides, so ``@versioned_method + ("v1.2")`` keeps working against a v1.3 remote. + The full set of supported versions is recorded on the method as ``_api_versions``, - and calling the method with a negotiated version that no implementation serves - raises a RemoteError. While no version has been negotiated yet, the default - implementation is used. + and calling the method against a remote that provides none of them raises a + RemoteError. While no version has been negotiated yet, the default implementation + is used. """ def decorator(default: Callable) -> Callable: @@ -136,17 +191,25 @@ def decorator(default: Callable) -> Callable: @functools.wraps(default) def wrapper(self, *args, **kwargs): - selected = getattr(self, "_api_version", None) + selected = getattr(self, "_request_version", None) or getattr( + self, "_api_version", None + ) if selected is None: return default(self, *args, **kwargs) - impl = registry.get(selected) - if impl is None: + + if selected in registry: + return registry[selected](self, *args, **kwargs) + + version = select_api_version(self._server_versions, registry) + if version is None: raise RemoteError( - f"'{default_name}' is not supported by the negotiated API " - f"version '{selected}'. It requires one of: " - f"{', '.join(sorted(registry))}." + f"'{default_name}' requires one of the API versions " + f"{', '.join(sorted(registry))}, none of which is provided by " + f"remote '{self._remote}' (it provides " + f"{', '.join(sorted(self._server_versions))})." ) - return impl(self, *args, **kwargs) + with self.api_version(version): + return registry[version](self, *args, **kwargs) def register(*impl_versions: str) -> Callable: def do_register(func: Callable) -> Callable: @@ -214,6 +277,23 @@ def select_api_version( return max(common_versions, key=lambda v: Version.coerce(v.lstrip("v"))) +RequestData = Optional[BaseModel] +"""Body of a request, described by the pydantic model of the endpoint.""" + + +def serialise_request_data(data: RequestData) -> Union[str, Dict]: + """ + Serialise the body of a request to JSON. + + @param data: the request body as the pydantic model of the endpoint, or None + for a request without a body. + @return: the JSON encoded body, or an empty dict if there is no body. + """ + if data is None: + return {} + return data.model_dump_json() + + def check_return(res: "requests.Response") -> None: if res.status_code != 200: try: @@ -226,6 +306,47 @@ def check_return(res: "requests.Response") -> None: res.raise_for_status() +def _pagination_headers(limit: int, page: int = 1) -> Dict[str, str]: + """ + Return the pagination request headers, leaving out the server side defaults. + """ + pagination = PaginationData.model_validate({"limit": limit, "page": page}) + return { + name: str(value) + for name, value in pagination.model_dump( + by_alias=True, exclude_defaults=True + ).items() + } + + +def _meta_list(value: Any) -> List[Any]: + """ + Return an IDS metadata value as the list of names the files endpoint expects. + """ + value = coerce_ids_list(value) + if value is None: + return [] + if isinstance(value, (list, tuple)): + return list(value) + return [value] + + +def _simulation_from_list_item(item: SimulationListItem) -> "Simulation": + """ + Build a Simulation from the summary returned by the simulation list endpoint. + """ + return Simulation.from_data_model( + SimulationData.model_validate( + { + "uuid": item.uuid, + "alias": item.alias, + "datetime": item.datetime, + "metadata": item.metadata, + } + ) + ) + + def _get_paths(file: "File") -> Iterable[Path]: if file.type == DataType.FILE: if file.uri and file.uri.path: @@ -303,7 +424,10 @@ def __init__( if self._firewall is not None: self._load_cookies(remote, username, password) - self._api_url: str = f"{self._url}/" + self._base_url: str = f"{self._url}/" + self._api_version: Optional[str] = None + self._request_version: Optional[str] = None + self._server_versions: FrozenSet[str] = frozenset() self._server_auth = self.get_server_authentication() if self._firewall: self._server_auth = "None" @@ -328,6 +452,7 @@ def __init__( endpoints = self.get_endpoints() endpoint_versions = [endpoint.split("/")[-1] for endpoint in endpoints] + self._server_versions = frozenset(endpoint_versions) selected_version = select_api_version(endpoint_versions) if selected_version is None: @@ -341,7 +466,6 @@ def __init__( print(f"Selected API version {selected_version}") self._api_version = selected_version - self._api_url += f"{selected_version}/" self.version = Version.coerce(selected_version.lstrip("v")) self.server_version = Version.coerce(self.get_server_version()) @@ -394,6 +518,42 @@ def _load_cookies( else: raise ValueError(f"Unknown firewall option {self._firewall}") + @property + def _api_url(self) -> str: + """ + Return the base URL of the API version the current request targets. + """ + version = self._request_version or self._api_version + if version is None: + return self._base_url + return f"{self._base_url}{version}/" + + @contextmanager + def api_version(self, version: str) -> Iterator[None]: + """ + Send the requests made inside this context to the given API version. + + @param version: the API version, as it appears in the endpoint URL. + """ + if version not in CLIENT_API_VERSIONS: + raise RemoteError( + f"API version '{version}' is not supported by this client. It " + f"supports: {', '.join(CLIENT_API_VERSIONS)}." + ) + if version not in self._server_versions: + raise RemoteError( + f"API version '{version}' is not provided by remote " + f"'{self._remote}'. It provides: " + f"{', '.join(sorted(self._server_versions)) or 'none'}." + ) + + previous = self._request_version + self._request_version = version + try: + yield + finally: + self._request_version = previous + @property def remote(self) -> str: """ @@ -423,6 +583,7 @@ def get( headers: Optional[Dict] = None, authenticate: Optional[bool] = True, stream: Optional[bool] = False, + base_url: Optional[str] = None, ) -> "requests.Response": """ Perform an HTTP GET request. @@ -433,17 +594,20 @@ def get( :param authenticate: True if we should send authentication headers with the request. :param stream: True to enable streaming. + :param base_url: the base URL to resolve the request against, defaulting + to the API version the current request targets. """ params = params if params is not None else {} headers = headers if headers is not None else {} headers["Accept-encoding"] = "gzip" headers["User-Agent"] = "it_script_basic" + request_url = (base_url if base_url is not None else self._api_url) + url # Get token api expected basic auth in request if authenticate and self._server_auth != "None": res = requests.get( - self._api_url + url, + request_url, params=params, auth=self._get_auth(), headers=headers, @@ -452,7 +616,7 @@ def get( ) else: res = requests.get( - self._api_url + url, + request_url, params=params, headers=headers, cookies=self._cookies, @@ -462,49 +626,24 @@ def get( check_return(res) return res - def get_root( - self, - headers: Optional[Dict] = None, - stream: bool = False, - ) -> "requests.Response": - """ - Perform an HTTP GET request to the server root endpoint. - - @param headers: additional headers to send with the request. - @param stream: True to enable streaming. - @return: - """ - - headers = headers or {} - headers["Accept-encoding"] = "gzip" - headers["User-Agent"] = "it_script_basic" - - res = requests.get( - self._url, - headers=headers, - cookies=self._cookies, - stream=stream, - ) - check_return(res) - return res - - def put(self, url: str, data: Dict, **kwargs) -> "requests.Response": + def put(self, url: str, data: RequestData, **kwargs) -> "requests.Response": """ Perform an HTTP PUT request. @param url: the URL of the request. - @param data: the PUT data to send. + @param data: the PUT data to send, as the pydantic model of the endpoint. @param kwargs: any additional keyword arguments to add to the request. @return: """ headers = {"Content-type": "application/json"} headers["User-Agent"] = "it_script_basic" + put_data = serialise_request_data(data) if self._server_auth != "None": res = requests.put( self._api_url + url, - data=json.dumps(data, cls=CustomEncoder), + data=put_data, headers=headers, auth=self._get_auth(), cookies=self._cookies, @@ -513,7 +652,7 @@ def put(self, url: str, data: Dict, **kwargs) -> "requests.Response": else: res = requests.put( self._api_url + url, - data=json.dumps(data, cls=CustomEncoder), + data=put_data, headers=headers, cookies=self._cookies, **kwargs, @@ -522,12 +661,12 @@ def put(self, url: str, data: Dict, **kwargs) -> "requests.Response": check_return(res) return res - def post(self, url: str, data: Dict, **kwargs) -> "requests.Response": + def post(self, url: str, data: RequestData, **kwargs) -> "requests.Response": """ Perform an HTTP POST request. @param url: the URL of the request. - @param data: the POST data to send. + @param data: the POST data to send, as the pydantic model of the endpoint. @param kwargs: any additional keyword arguments to add to the request. @return: """ @@ -538,7 +677,7 @@ def post(self, url: str, data: Dict, **kwargs) -> "requests.Response": headers = {} else: headers = {"Content-type": "application/json"} - post_data = json.dumps(data, cls=CustomEncoder, indent=2) if data else {} + post_data = serialise_request_data(data) headers["User-Agent"] = "it_script_basic" # Compress the data if it is larger than 2 MB and the URL is for simulations @@ -575,23 +714,24 @@ def post(self, url: str, data: Dict, **kwargs) -> "requests.Response": check_return(res) return res - def patch(self, url: str, data: Dict, **kwargs) -> "requests.Response": + def patch(self, url: str, data: RequestData, **kwargs) -> "requests.Response": """ Perform an HTTP PATCH request. @param url: the URL of the request. - @param data: the PATCH data to send. + @param data: the PATCH data to send, as the pydantic model of the endpoint. @param kwargs: any additional keyword arguments to add to the request. @return: """ headers = {"Content-type": "application/json"} headers["User-Agent"] = "it_script_basic" + patch_data = serialise_request_data(data) if self._server_auth != "None": res = requests.patch( self._api_url + url, - data=json.dumps(data, cls=CustomEncoder), + data=patch_data, headers=headers, auth=self._get_auth(), cookies=self._cookies, @@ -600,7 +740,7 @@ def patch(self, url: str, data: Dict, **kwargs) -> "requests.Response": else: res = requests.patch( self._api_url + url, - data=json.dumps(data, cls=CustomEncoder), + data=patch_data, headers=headers, cookies=self._cookies, **kwargs, @@ -609,23 +749,24 @@ def patch(self, url: str, data: Dict, **kwargs) -> "requests.Response": check_return(res) return res - def delete(self, url: str, data: Dict[Any, Any], **kwargs) -> "requests.Response": + def delete(self, url: str, data: RequestData, **kwargs) -> "requests.Response": """ Perform an HTTP DELETE request. @param url: the URL of the request. - @param data: the DELETE data to send. + @param data: the DELETE data to send, as the pydantic model of the endpoint. @param kwargs: any additional keyword arguments to add to the request. @return: """ headers = {"Content-type": "application/json"} headers["User-Agent"] = "it_script_basic" + delete_data = serialise_request_data(data) if self._server_auth != "None": res = requests.delete( self._api_url + url, - data=json.dumps(data, cls=CustomEncoder), + data=delete_data, headers=headers, auth=self._get_auth(), cookies=self._cookies, @@ -634,7 +775,7 @@ def delete(self, url: str, data: Dict[Any, Any], **kwargs) -> "requests.Response else: res = requests.delete( self._api_url + url, - data=json.dumps(data, cls=CustomEncoder), + data=delete_data, headers=headers, cookies=self._cookies, **kwargs, @@ -645,54 +786,55 @@ def delete(self, url: str, data: Dict[Any, Any], **kwargs) -> "requests.Response def has_url(self) -> bool: return bool(self._url) + @try_request + def _get_index(self, base_url: Optional[str] = None) -> IndexResponse: + """ + Return the index of an endpoint. + + @param base_url: the base URL of the index to read, defaulting to the index + of the API version the current request targets. + """ + res = self.get("", authenticate=False, base_url=base_url) + return IndexResponse.model_validate_json(res.content) + @versioned_method("v1.2", "v1.3") @try_request def get_token(self) -> str: res = self.get("token") - data = res.json() - return data["token"] + return TokenResponse.model_validate_json(res.content).token @versioned_method("v1.2", "v1.3") - @try_request def get_endpoints(self) -> List[str]: - res = self.get("", authenticate=False) - data = res.json() - return data["endpoints"] + return self._get_index().endpoints @versioned_method("v1.2", "v1.3") - @try_request def get_server_authentication(self) -> Optional[str]: - res = self.get("", authenticate=False) - data = res.json() - return data.get("authentication") + return self._get_index().authentication @versioned_method("v1.2", "v1.3") - @try_request def get_server_version(self) -> str: - try: - res = self.get_root().json() - if (server_version := res.get("server_version")) is not None: - return server_version - except Exception: - pass - data = self.get("", authenticate=False).json() - return data["server_version"] + # The server root is the primary source; remotes that predate the move of + # the server version off the versioned API index still report it there. + version = self._get_index(base_url=self._base_url).server_version + if version is None: + version = self._get_index().server_version + if version is None: + raise RemoteError( + f"Remote '{self._remote}' did not report a server version." + ) + return version @versioned_method("v1.2", "v1.3") @try_request def get_validation_schemas(self) -> List[Dict]: res = self.get("validation_schema") - return res.json() + return ValidationSchemaList.model_validate_json(res.content).root @versioned_method("v1.2", "v1.3") @try_request - def get_upload_options(self) -> Dict[str, Any]: - try: - res = self.get("upload_options") - return res.json() - except FailedConnection: - # old remotes may not provide this endpoint - return {} + def get_upload_options(self) -> UploadOptions: + res = self.get("upload_options") + return UploadOptions.model_validate_json(res.content) @versioned_method("v1.2", "v1.3") @try_request @@ -700,22 +842,24 @@ def list_simulations( self, meta: Optional[List[str]] = None, limit: int = 0 ) -> List["Simulation"]: args = "?" + "&".join(meta) if meta else "" - headers = {"simdb-result-limit": str(limit)} + headers = _pagination_headers(limit) res = self.get("simulations" + args, headers=headers) - data = res.json(cls=CustomDecoder) - return [Simulation.from_data(sim) for sim in data["results"]] + data = PaginatedResponse[SimulationListItem].model_validate_json(res.content) + return [_simulation_from_list_item(sim) for sim in data.results] @versioned_method("v1.2", "v1.3") @try_request def get_simulation(self, sim_id: str) -> "Simulation": res = self.get("simulation/" + sim_id) - return Simulation.from_data(res.json(cls=CustomDecoder)) + return Simulation.from_data_model( + SimulationDataResponse.model_validate_json(res.content) + ) @versioned_method("v1.2", "v1.3") @try_request - def trace_simulation(self, sim_id: str) -> dict: + def trace_simulation(self, sim_id: str) -> SimulationTraceData: res = self.get("trace/" + sim_id) - return res.json(cls=CustomDecoder) + return SimulationTraceData.model_validate_json(res.content) @versioned_method("v1.2", "v1.3") @try_request @@ -727,92 +871,104 @@ def query_simulations( (key, value) = item.split("=") params[key].append(value) args = "?" + "&".join(meta) if meta else "" - headers = { - APIConstants.LIMIT_HEADER: str(limit), - APIConstants.PAGE_HEADER: str(1), - } + headers = _pagination_headers(limit, page=1) res = self.get("simulations" + args, params, headers=headers) - data = res.json(cls=CustomDecoder) - return [Simulation.from_data(sim) for sim in data["results"]] + data = PaginatedResponse[SimulationListItem].model_validate_json(res.content) + return [_simulation_from_list_item(sim) for sim in data.results] @versioned_method("v1.2", "v1.3") @try_request - def delete_simulation(self, sim_id: str) -> Dict: - res = self.delete("simulation/" + sim_id, {}) - return res.json() + def delete_simulation(self, sim_id: str) -> SimulationDeleteResponse: + res = self.delete("simulation/" + sim_id, None) + return SimulationDeleteResponse.model_validate_json(res.content) @versioned_method("v1.2", "v1.3") @try_request def update_simulation(self, sim_id: str, update_type: "Simulation.Status") -> None: - self.patch("simulation/" + sim_id, {"status": update_type.value}) + res = self.patch( + "simulation/" + sim_id, StatusPatchData(status=update_type.value) + ) + SimulationPatchResponse.model_validate_json(res.content) @versioned_method("v1.2", "v1.3") @try_request def validate_simulation(self, sim_id: str) -> Tuple[bool, str]: - res = self.post("validate/" + sim_id, {}) - data = res.json() - if data["passed"]: + res = self.post("validate/" + sim_id, None) + result = ValidationResult.model_validate_json(res.content) + if result.passed: return True, "" else: - return False, data["error"] + return False, result.error or "" @versioned_method("v1.2", "v1.3") @try_request def add_watcher( self, sim_id: str, user: str, email: str, notification: "Watcher.Notification" ) -> None: - self.post( + res = self.post( "watchers/" + sim_id, - {"user": user, "email": email, "notification": notification.name}, + WatcherPostRequest(user=user, email=email, notification=notification.name), ) + WatcherPostResponse.model_validate_json(res.content) @versioned_method("v1.2", "v1.3") @try_request def remove_watcher(self, sim_id: str, user: str) -> None: - self.delete("watchers/" + sim_id, {"user": user}) + res = self.delete("watchers/" + sim_id, WatcherDeleteRequest(user=user)) + WatcherDeleteResponse.model_validate_json(res.content) @versioned_method("v1.2", "v1.3") @try_request - def list_watchers(self, sim_id: str) -> List[Tuple]: + def list_watchers(self, sim_id: str) -> List[WatcherData]: res = self.get("watchers/" + sim_id) - return [(d["username"], d["email"], d["notification"]) for d in res.json()] + return WatcherGetResponse.model_validate_json(res.content).root @versioned_method("v1.2", "v1.3") @try_request def set_metadata( self, sim_id: str, key: str, value: Union[str, uuid.UUID, int, float] - ) -> List[str]: - res = self.patch("simulation/metadata/" + sim_id, {"key": key, "value": value}) - return [data["value"] for data in res.json()] + ) -> List[MetadataValue]: + """ + Set a metadata value on the remote, returning the values it replaced. + """ + res = self.patch( + "simulation/metadata/" + sim_id, + MetadataPatchData(key=key, value=str(value)), + ) + return [ + meta.value + for meta in MetadataDataList.model_validate_json(res.content).root + ] @versioned_method("v1.2", "v1.3") @try_request - def delete_metadata(self, sim_id: str, key: str) -> List[str]: - res = self.delete("simulation/metadata/" + sim_id, {"key": key}) - return [data["value"] for data in res.json()] + def delete_metadata(self, sim_id: str, key: str) -> None: + res = self.delete("simulation/metadata/" + sim_id, MetadataDeleteData(key=key)) + MetadataDeleteResponse.model_validate_json(res.content) @versioned_method("v1.3") @try_request def get_simulation_data( self, sim_id: str, path: str, dd_version: Optional[str] = None - ) -> Dict[str, Any]: - params = {"path": path} - if dd_version is not None: - params["dd_version"] = dd_version - res = self.get(f"simulation/{sim_id}/data", params=params) - return res.json() + ) -> ImasDataResponse: + query = ImasDataQueryParams(path=path, dd_version=dd_version) + res = self.get( + f"simulation/{sim_id}/data", + params=query.model_dump(exclude_none=True), + ) + return ImasDataResponse.model_validate_json(res.content) @try_request - def get_directory(self) -> str: + def get_directory(self) -> Path: res = self.get("staging_dir") - return res.json()["staging_dir"] + return StagingDirectoryResponse.model_validate_json(res.content).staging_dir def _push_file( self, path: Path, uuid: uuid.UUID, file_type: str, - sim_data: Dict[str, Any], + sim_data: SimulationData, chunk_size: int, out_stream: IO, type: DataType, @@ -832,18 +988,18 @@ def _push_file( if type == DataType.FILE: self.post( "files", - data={ - "simulation": sim_data, - "obj_type": DataType.FILE, - "files": [ - { - "chunks": num_chunks, - "file_type": file_type, - "file_uuid": uuid.hex, - "ids_list": None, - } + data=FileRegistrationData( + simulation=sim_data, + obj_type=DataType.FILE, + files=[ + FileRegistrationItem( + chunks=num_chunks, + file_type=file_type, + file_uuid=uuid, + ids_list=None, + ) ], - }, + ), ) print(f"\r{msg}", file=out_stream, end="") print( @@ -859,27 +1015,27 @@ def _send_chunk( chunk_size: int, uuid: uuid.UUID, file_type: str, - sim_data: dict, + sim_data: SimulationData, ): - data = { - "simulation": sim_data, - "file_type": file_type, - "chunk_info": {uuid.hex: {"chunk_size": chunk_size, "chunk": chunk_index}}, - } + data = FileUploadData( + simulation=sim_data, + file_type=file_type, + chunk_info={uuid.hex: ChunkInfo(chunk_size=chunk_size, chunk=chunk_index)}, + ) files: List[Tuple[str, Tuple[str, bytes, str]]] = [ ( "data", ( "data", - json.dumps(data, cls=CustomEncoder).encode(), + data.model_dump_json().encode(), "text/json", ), ), ("files", (uuid.hex, chunk, "application/octet-stream")), ] - self.post("files", data={}, files=files) + self.post("files", data=None, files=files) - @versioned_method("v1.2", "v1.3") + @versioned_method("v1.2") @try_request def push_simulation( self, @@ -893,19 +1049,19 @@ def push_simulation( First we upload any files associated with the simulation, then push the simulation metadata. + Only supported on the v1.2 API: the chunked file upload is to be replaced by + a resumable HTTP upload, so it has not been ported to v1.3. + :param simulation: The Simulation to push to remote server :param out_stream: The IO stream to write messages to the user (default: stdout) :param add_watcher: Add the current user as a watcher of the simulation on the remote server """ - sim_data = simulation.data(recurse=True) + sim_data = simulation.to_model(recurse=True) try: - sim_json = json.dumps( - sim_data, cls=CustomEncoder, separators=(",", ":") - ).encode("utf-8") - sim_json_size = len(sim_json) + sim_json_size = len(sim_data.model_dump_json().encode("utf-8")) except Exception: sim_json_size = 0 @@ -921,17 +1077,17 @@ def push_simulation( ) options = self.get_upload_options() - if options.get("copy_files", True): + if options.copy_files: chunk_size = allowed_chunk # 10 MB limit on ITER network - copy_ids = options.get("copy_ids", True) + copy_ids = options.copy_ids for file in simulation.inputs: if file.type == DataType.IMAS: if not copy_ids: print(f"Skipping IDS data {file}", file=out_stream, flush=True) continue - ids_list = simulation.meta_dict().get("input_ids", []) + ids_list = _meta_list(simulation.meta_dict().get("input_ids")) for path in imas_files(file.uri): # Check if hdf5 ids_name is in ids_list ids_name = Path(path).name.split(".") @@ -942,9 +1098,9 @@ def push_simulation( ): continue sim_file = next( - f for f in sim_data["inputs"] if f.get("uuid") == file.uuid + f for f in sim_data.inputs.root if f.uuid == file.uuid ) - sim_file["uri"] = f"file:{path}" + sim_file.uri = f"file:{path}" self._push_file( path, file.uuid, @@ -957,17 +1113,17 @@ def push_simulation( self.post( "files", - data={ - "simulation": simulation.data(recurse=True), - "obj_type": file.type, - "files": [ - { - "file_type": "input", - "file_uuid": file.uuid.hex, - "ids_list": ids_list, - } + data=FileRegistrationData( + simulation=simulation.to_model(recurse=True), + obj_type=file.type, + files=[ + FileRegistrationItem( + file_type="input", + file_uuid=file.uuid, + ids_list=ids_list, + ) ], - }, + ), ) else: @@ -988,7 +1144,7 @@ def push_simulation( print(f"Skipping IDS data {file}", file=out_stream, flush=True) continue - ids_list = simulation.meta_dict().get("ids", []) + ids_list = _meta_list(simulation.meta_dict().get("ids")) for path in imas_files(file.uri): # Check if hdf5 ids_name is in ids_list ids_name = Path(path).name.split(".") @@ -999,15 +1155,11 @@ def push_simulation( ): continue sim_file = next( - ( - f - for f in sim_data["outputs"] - if f.get("uuid") == file.uuid - ), + (f for f in sim_data.outputs.root if f.uuid == file.uuid), None, ) if sim_file: - sim_file["uri"] = f"file:{path}" + sim_file.uri = f"file:{path}" self._push_file( path, file.uuid, @@ -1020,17 +1172,17 @@ def push_simulation( self.post( "files", - data={ - "simulation": simulation.data(recurse=True), - "obj_type": file.type, - "files": [ - { - "file_type": "output", - "file_uuid": file.uuid.hex, - "ids_list": ids_list, - } + data=FileRegistrationData( + simulation=simulation.to_model(recurse=True), + obj_type=file.type, + files=[ + FileRegistrationItem( + file_type="output", + file_uuid=file.uuid, + ids_list=ids_list, + ) ], - }, + ), ) else: if file.uri and file.uri.path: @@ -1044,24 +1196,22 @@ def push_simulation( file.type, ) - sim_data = simulation.data(recurse=True) - uploaded_by = simulation.meta_dict().get("uploaded_by", None) + uploaded_by = simulation.meta_dict().get("uploaded_by") print("Uploading simulation data ... ", file=out_stream, end="", flush=True) self.post( "simulations", - data={ - "simulation": sim_data, - "add_watcher": add_watcher, - "uploaded_by": uploaded_by, - }, + data=SimulationPostData( + simulation=simulation.to_model(recurse=True), + add_watcher=add_watcher, + uploaded_by=str(uploaded_by) if uploaded_by is not None else None, + ), ) print("Success", file=out_stream, flush=True) def _get_file_info(self, uuid: uuid.UUID) -> List[Tuple[Path, str]]: - r = self.get(f"file/{uuid.hex}") - data = r.json() - files = data["files"] - return [(Path(file["path"]), file["checksum"]) for file in files] + res = self.get(f"file/{uuid.hex}") + data = FileGetDataResponse.model_validate_json(res.content) + return [(file.path, file.checksum) for file in data.files] def _pull_file( self, @@ -1132,6 +1282,12 @@ def pull_simulation( all_paths = [] + if len(simulation.inputs) + len(simulation.outputs) == 0: + raise RemoteError( + f"Simulation '{sim_id}' on remote has no input or output files " + "registered, so there is nothing to download." + ) + for file in itertools.chain(simulation.inputs, simulation.outputs): info = self._get_file_info(file.uuid) all_paths += [path for (path, _) in info] @@ -1169,4 +1325,4 @@ def pull_simulation( @versioned_method("v1.2", "v1.3") @try_request def reset_database(self) -> None: - self.post("reset", {}) + self.post("reset", None) diff --git a/src/simdb/database/models/simulation.py b/src/simdb/database/models/simulation.py index f8212032..495873b2 100644 --- a/src/simdb/database/models/simulation.py +++ b/src/simdb/database/models/simulation.py @@ -17,12 +17,14 @@ from simdb.enums import IngestionStatus from simdb.imas.utils import SimDBUrl from simdb.remote.models import ( + IDS_LIST_KEYS, FileDataList, MetadataData, MetadataDataList, SimulationData, SimulationDataResponse, SimulationTraceData, + coerce_ids_list, ) if "sphinx" in sys.modules: @@ -164,6 +166,12 @@ def _get_metadata_dict(self) -> Dict[str, Any]: return self._metadata def _set_metadata_dict(self, meta_dict: Dict[str, Any]) -> None: + # Repair simulations pulled with odd formatted ids arrays. This is the only + # coercion on the raw-dict path (v1/v1.1 endpoints), which never builds a + # MetadataData. + for key in IDS_LIST_KEYS: + if key in meta_dict: + meta_dict[key] = coerce_ids_list(meta_dict[key]) self._metadata = meta_dict def __init__( @@ -214,7 +222,7 @@ def __init__( self.inputs.append(file) if all_input_idss: - self.set_meta("input_ids", "[{}]".format(", ".join(all_input_idss))) + self.set_meta("input_ids", all_input_idss) all_output_idss = [] @@ -242,7 +250,7 @@ def __init__( self.outputs.append(file) if all_output_idss: - self.set_meta("ids", "[{}]".format(", ".join(all_output_idss))) + self.set_meta("ids", all_output_idss) flattened_dict = flatten_dict(manifest.metadata) @@ -286,6 +294,10 @@ def __str__(self): first_line = False elif isinstance(value, dict) and "min" in value and "max" in value: result += f" {element}: [{value['min']}, {value['max']}]\n" + elif isinstance(value, list): + result += " {}: [{}]\n".format( + element, ", ".join(str(el) for el in value) + ) else: result += f" {element}: {value}\n" result += "inputs:\n" diff --git a/src/simdb/database/models/watcher.py b/src/simdb/database/models/watcher.py index c7532b72..c64e9cd8 100644 --- a/src/simdb/database/models/watcher.py +++ b/src/simdb/database/models/watcher.py @@ -62,4 +62,12 @@ def data(self, recurse: bool = False) -> Dict[str, str]: return data def to_model(self) -> WatcherData: - return WatcherData.model_validate(self.data()) + return WatcherData.model_validate( + { + "username": self.username, + "email": self.email, + "notification": self.NOTIFICATION_CHOICES[ + Notification(self.notification) + ], + } + ) diff --git a/src/simdb/imas/utils.py b/src/simdb/imas/utils.py index 73a4c73e..9ac4795e 100644 --- a/src/simdb/imas/utils.py +++ b/src/simdb/imas/utils.py @@ -331,6 +331,15 @@ def imas_files(uri: SimDBUrl) -> List[Path]: path = _get_path(uri) + if backend == "uda": + query_backend = dict(uri.query_params()).get("backend") + if query_backend is None: + raise ValueError( + "Invalid IMAS URI - 'backend' query argument not provided for UDA " + "backend" + ) + backend = query_backend + if backend == "hdf5": return [p.absolute() for p in path.glob("*.h5")] elif backend == "mdsplus": diff --git a/src/simdb/remote/apis/files.py b/src/simdb/remote/apis/files.py index cd7cad5e..7f0e7a56 100644 --- a/src/simdb/remote/apis/files.py +++ b/src/simdb/remote/apis/files.py @@ -58,15 +58,11 @@ def _verify_file( path_value = qs.get("path") if path_value is None: raise ValueError("The 'path' key is missing in the URI query") - if common_root == Path("/"): - path_value = str(staging_dir) + path_value - elif common_root is not None and common_root == path_value: - path_value = path_value.replace(str(common_root), str(staging_dir)) - - else: - path_value = str(staging_dir) + staged_dir = secure_path( + Path(path_value), common_root, staging_dir, is_file=False + ) new_uri = uri.build( - scheme=uri.scheme, path=uri.path, query=f"path={path_value}" + scheme=uri.scheme, path=uri.path, query=f"path={staged_dir.as_posix()}" ) checksum = imas_checksum(new_uri, ids_list or []) if sim_file.checksum != checksum: diff --git a/src/simdb/remote/apis/v1_2/simulations.py b/src/simdb/remote/apis/v1_2/simulations.py index a8594490..62e14f13 100644 --- a/src/simdb/remote/apis/v1_2/simulations.py +++ b/src/simdb/remote/apis/v1_2/simulations.py @@ -20,7 +20,11 @@ from simdb.remote.core.auth import User, requires_auth from simdb.remote.core.cache import cache, cache_key, clear_cache from simdb.remote.core.errors import error -from simdb.remote.core.path import find_common_root, secure_path +from simdb.remote.core.path import ( + UnsafePathError, + find_common_root, + secure_path, +) from simdb.remote.core.pydantic_utils import ( Body, Header, @@ -163,6 +167,16 @@ def get_meta_val(key, default=None): return data +def _staging_path( + path: Path, common_root: Optional[Path], staging_dir: Path, is_file: bool = True +) -> Path: + """secure_path, reporting a path it refuses to stage as a client error.""" + try: + return secure_path(path, common_root, staging_dir, is_file=is_file) + except UnsafePathError as err: + raise ResponseException(str(err)) from err + + @api.route("/simulations") class SimulationList(Resource): @requires_auth() @@ -267,7 +281,7 @@ def post( and sim_file.uri.scheme == "file" and sim_file.uri.path is not None ): - path = secure_path( + path = _staging_path( Path(sim_file.uri.path), common_root, staging_dir ) if not path.exists(): @@ -278,11 +292,11 @@ def post( elif sim_file.uri.scheme == "imas": qs = dict(sim_file.uri.query_params()) if copy_files: - path = secure_path( + path = _staging_path( Path(qs["path"]), common_root, staging_dir, - is_file=common_root is not None, + is_file=False, ) else: path = Path(qs["path"]) diff --git a/src/simdb/remote/core/path.py b/src/simdb/remote/core/path.py index bb02508d..505340a7 100644 --- a/src/simdb/remote/core/path.py +++ b/src/simdb/remote/core/path.py @@ -5,17 +5,61 @@ from werkzeug.utils import secure_filename +class UnsafePathError(ValueError): + """Raised for an uploaded path that would be staged outside the staging dir.""" + + +def _staged_relative(path: Path, common_root: Path) -> Path: + """ + Return *path* relative to *common_root*, as a path that stays inside it. + + Path.relative_to is purely lexical and keeps ".." segments, so "/data/../etc" + relative to "/data" is "../etc", which would stage the file outside the staging + directory entirely. The paths come from the uploaded simulation, and neither + they nor the common root derived from them are to be trusted. + """ + try: + relative = path.relative_to(common_root) + except ValueError as err: + raise UnsafePathError(f"path {path} is not below {common_root}") from err + + # normpath resolves the ".." segments that do stay inside, so that a path + # written as "run/../run" stages next to the one written as "run". + normalised = Path(os.path.normpath(relative)) + if normalised.is_absolute() or ".." in normalised.parts: + raise UnsafePathError(f"path {path} escapes {common_root}") + return normalised + + def secure_path( path: Path, common_root: Optional[Path], staging_dir: Path, is_file=True ) -> Path: + """ + Return the location under *staging_dir* that *path* is uploaded to. + + Directories keep their name so that the directory form agrees with the file + form: the parent of a staged file is the staged form of its directory. + + :param path: the path on the client, from the simulation being uploaded. + :param common_root: the root the uploaded paths share, or None if they share + none, in which case everything is flattened into *staging_dir*. + :param staging_dir: the directory the simulation is staged in. + :param is_file: True if *path* is a file, False if it is a directory. + :raises UnsafePathError: if *path* would be staged outside *staging_dir*. + """ + if not is_file: + if common_root is None: + return staging_dir + return staging_dir / _staged_relative(path, common_root) + + # secure_filename() empties a name that is entirely unusable, such as "..", + # which would otherwise hand back the directory holding the file. + name = secure_filename(path.name) + if not name: + raise UnsafePathError(f"path {path} has no usable file name") if common_root is None: - directory = staging_dir - else: - directory = staging_dir / path.parent.relative_to(common_root) - if is_file: - return directory / secure_filename(path.name) - else: - return directory + return staging_dir / name + return staging_dir / _staged_relative(path.parent, common_root) / name def find_common_root(paths: Collection[Path]) -> Optional[Path]: diff --git a/src/simdb/remote/models.py b/src/simdb/remote/models.py index ae9565aa..eb345ef9 100644 --- a/src/simdb/remote/models.py +++ b/src/simdb/remote/models.py @@ -12,6 +12,7 @@ List, Literal, Optional, + Tuple, TypeVar, Union, ) @@ -224,6 +225,31 @@ def _serialize_numpy(o: np.ndarray) -> dict: validation.""" +IDS_LIST_KEYS: Tuple[str, ...] = ("ids", "input_ids") +"""Metadata keys whose value is a list of IDS names.""" + + +def coerce_ids_list(value: Any) -> Any: + """Repair an IDS list written as the display string ``"[a, b, c]"``. + + SimDB <= 1.2 stored the :data:`IDS_LIST_KEYS` metadata that way rather than as a + list (#119), so both forms have to be accepted wherever such a value crosses into + the models. Values that are not strings are returned unchanged. + + Kept as a single definition because it is applied at three separate boundaries: + :meth:`MetadataData.fix_ids_list_string` on the way out to the API, + ``Simulation._set_metadata_dict`` on the way in to the ORM, and the + ``a3f1c7d94e02`` migration for rows already at rest. The migration carries its own + copy on purpose -- migrations must stay frozen against later changes here. + """ + if not isinstance(value, str): + return value + text = value.strip() + if text.startswith("[") and text.endswith("]"): + text = text[1:-1] + return [name.strip() for name in text.split(",") if name.strip()] + + class MetadataData(BaseModel): """Key-value pair for simulation metadata.""" @@ -240,6 +266,22 @@ def convert_array_to_range(cls, data: Any) -> Any: data["value"] = _array_to_range(data["value"]) return data + @model_validator(mode="before") + @classmethod + def fix_ids_list_string(cls, data: Any) -> Any: + """Convert funky ids list strings to actual lists. + + Applies to every key in :data:`IDS_LIST_KEYS`; see :func:`coerce_ids_list`. + """ + if ( + isinstance(data, dict) + and "value" in data + and "element" in data + and data["element"] in IDS_LIST_KEYS + ): + data["value"] = coerce_ids_list(data["value"]) + return data + def as_dict(self) -> dict: """Convert to dictionary.""" return {self.element: self.value} @@ -521,8 +563,11 @@ class FileUploadResponse(BaseModel): class FileRegistrationItem(BaseModel): """A single file entry in the file registration payload.""" - chunks: int - """The amount of chunks to be processed.""" + chunks: Optional[int] = None + """The amount of chunks to be processed. + + Only sent for plain file uploads; omitted for IMAS registrations, where a + single item covers the many files pushed under one UUID.""" file_type: str """The file type.""" file_uuid: HexUUID @@ -662,6 +707,58 @@ class ImasDataResponse(BaseModel): """Coordinates for each dimension of *field*, in dimension order.""" +class IndexResponse(BaseModel): + """Response from an API index endpoint. + + Both the server root (``/``) and the versioned API roots (``/v1.x/``) are + described by this model; each populates only the fields it provides, so every + field carries a default. + """ + + api: Optional[str] = None + """Name of the API.""" + api_version: Optional[str] = None + """Version of the API served by this endpoint.""" + server_version: Optional[str] = None + """Version of the SimDB server.""" + endpoints: List[str] = [] + """URLs of the endpoints provided by this endpoint.""" + documentation: Optional[str] = None + """URL of the API documentation.""" + authentication: Optional[str] = None + """Name of the authenticator the server uses by default.""" + authenticators: List[str] = [] + """Names of all the authenticators the server accepts.""" + + +class TokenResponse(BaseModel): + """Response from the get token endpoint.""" + + token: str + """The JWT token to authenticate subsequent requests with.""" + status: Optional[str] = None + """Status of the token request.""" + + +class UploadOptions(BaseModel): + """Response from the upload options endpoint. + + Describes how the server expects simulation data to be uploaded. + """ + + copy_files: bool = True + """Whether files have to be uploaded to the server.""" + copy_ids: bool = True + """Whether IMAS data has to be uploaded to the server.""" + + +class ValidationSchemaList(RootModel): + """Response from the validation schema endpoint.""" + + root: List[Dict[str, Any]] = [] + """The validation schemas the server validates simulations against.""" + + class ErrorResponse(BaseModel): """Response model for server errors.""" diff --git a/tests/cli/cli_helpers.py b/tests/cli/cli_helpers.py new file mode 100644 index 00000000..18a69a92 --- /dev/null +++ b/tests/cli/cli_helpers.py @@ -0,0 +1,34 @@ +"""Helpers shared by the CLI tests. + +Kept out of ``conftest.py`` because pytest imports every ``conftest`` under +the same module name, so importing from it directly picks up whichever one +was loaded first. +""" + +from typing import Optional +from unittest import mock + + +def make_simulation( + alias: str, + uuid: str = "0123456789abcdef0123456789abcdef", + datetime: str = "2000-01-01 00:00:00", + status: str = "not validated", + meta: Optional[dict] = None, +) -> mock.Mock: + """Build a stand-in for a :class:`~simdb.database.models.Simulation`. + + Only the attributes the CLI display code touches are set; ``find_meta`` + answers from ``meta`` the same way the real model does (a list of objects + with a ``value``, empty when the name is unknown). + """ + simulation = mock.Mock() + simulation.alias = alias + simulation.uuid = uuid + simulation.datetime = datetime + simulation.status = status + meta = meta or {} + simulation.find_meta.side_effect = lambda name: ( + [mock.Mock(value=meta[name])] if name in meta else [] + ) + return simulation diff --git a/tests/cli/conftest.py b/tests/cli/conftest.py new file mode 100644 index 00000000..ff2c5342 --- /dev/null +++ b/tests/cli/conftest.py @@ -0,0 +1,159 @@ +"""Shared fixtures for the ``simdb`` command line interface tests. + +Every test here drives the CLI through :class:`click.testing.CliRunner`, so the +fixtures below take care of the two things that would otherwise leak between +tests and into the developer's machine: the configuration that the CLI reads at +startup, and the handshake :class:`~simdb.cli.remote_api.RemoteAPI` performs +against a remote when it is constructed. +""" + +import os +from pathlib import Path +from types import SimpleNamespace +from unittest import mock + +import pytest +from click.testing import CliRunner, Result + +from simdb.cli.remote_api import RemoteAPI +from simdb.cli.simdb import cli + +REMOTE_NAME = "test" +REMOTE_URL = "http://0.0.0.0:5000/" +REMOTE_TOKEN = "123ABC" + +SERVER_ENDPOINTS = ["v1", "v1.1", "v1.1.1", "v1.2", "v1.3"] +"""API versions the fake remote advertises.""" + + +@pytest.fixture(autouse=True) +def isolated_config_environment(tmp_path, monkeypatch): + """Point the CLI at throw-away site and user configuration files. + + :class:`~simdb.config.config.Config` reads ``simdb.cfg`` from the platform + config directories unless ``SIMDB_SITE_CONFIG_PATH``/ + ``SIMDB_USER_CONFIG_PATH`` say otherwise. Without this fixture the outcome of + a test depends on whether the machine running it happens to have a real + SimDB configuration, which is exactly the kind of difference that makes a + suite pass locally and fail in CI. + """ + for variable in [name for name in os.environ if name.startswith("SIMDB_")]: + monkeypatch.delenv(variable) + monkeypatch.setenv("SIMDB_SITE_CONFIG_PATH", str(tmp_path / "site-simdb.cfg")) + monkeypatch.setenv("SIMDB_USER_CONFIG_PATH", str(tmp_path / "user-simdb.cfg")) + + +@pytest.fixture +def config_file(tmp_path) -> Path: + """A configuration file declaring a single, default, token-authenticated remote.""" + config_path = tmp_path / "simdb.cfg" + config_path.write_text( + f'[remote "{REMOTE_NAME}"]\n' + f"url = {REMOTE_URL}\n" + "default = True\n" + f"token = {REMOTE_TOKEN}\n" + "\n" + "[db]\n" + # Keep any command that reaches the real database away from the local + # one in the user's data directory. + f"file = {tmp_path / 'sim.db'}\n" + ) + return config_path + + +@pytest.fixture +def runner() -> CliRunner: + return CliRunner() + + +@pytest.fixture +def invoke(runner, config_file): + """Invoke the ``simdb`` CLI against the throw-away :func:`config_file`. + + ``invoke("simulation", "list")`` runs ``simdb --config-file=... simulation + list``. Any keyword argument is forwarded to + :meth:`click.testing.CliRunner.invoke`, so ``input=`` can be used to answer + prompts. + """ + + def _invoke(*args: str, **kwargs) -> Result: + return runner.invoke(cli, [f"--config-file={config_file}", *args], **kwargs) + + return _invoke + + +@pytest.fixture +def remote_handshake(): + """Stub the requests :class:`RemoteAPI` makes while it is being constructed. + + ``RemoteAPI.__init__`` asks the remote for its authentication scheme, its + endpoints, and its server version before any command specific request is + made. Tests that only care about the command itself get all three stubbed + here, and can still assert on them through the returned namespace:: + + def test_something(invoke, remote_handshake): + ... + assert remote_handshake.get_endpoints.called + """ + with mock.patch.object( + RemoteAPI, "get_server_authentication", return_value="None" + ) as get_server_authentication, mock.patch.object( + RemoteAPI, "get_endpoints", return_value=list(SERVER_ENDPOINTS) + ) as get_endpoints, mock.patch.object( + RemoteAPI, "get_server_version", return_value="0.11" + ) as get_server_version: + yield SimpleNamespace( + get_server_authentication=get_server_authentication, + get_endpoints=get_endpoints, + get_server_version=get_server_version, + ) + + +@pytest.fixture +def local_db(): + """Replace the local database with a mock in every module that looks it up. + + ``get_local_db`` is imported into each command module, so patching a single + import site silently leaves the other commands talking to the real database + in the user's data directory. + """ + db = mock.Mock() + with mock.patch( + "simdb.cli.commands.alias.get_local_db", return_value=db + ), mock.patch("simdb.cli.commands.simulation.get_local_db", return_value=db): + yield db + + +@pytest.fixture +def data_file(tmp_path) -> Path: + """A small file a manifest can reference as an input or output.""" + path = tmp_path / "data.txt" + path.write_text("simulation data\n") + return path + + +@pytest.fixture +def manifest_file(tmp_path, data_file) -> Path: + """A minimal, valid manifest referencing only local files.""" + manifest_path = tmp_path / "manifest.yaml" + manifest_path.write_text( + f"""\ +manifest_version: 2 +alias: simulation-alias + +inputs: + - uri: file://{data_file} + +outputs: + - uri: file://{data_file} + +metadata: +- values: + workflow: + name: Workflow Name + git: ssh://git@git.iter.org/wf/workflow.git + branch: master + commit: 079e84d5ae8a0eec6dcf3819c98f3c05f48e952f +""" + ) + return manifest_path diff --git a/tests/cli/test_cli_alias_command.py b/tests/cli/test_cli_alias_command.py index c173dfd6..a85a6efd 100644 --- a/tests/cli/test_cli_alias_command.py +++ b/tests/cli/test_cli_alias_command.py @@ -1,80 +1,110 @@ -from unittest import mock +"""Tests for ``simdb alias``.""" -from click.testing import CliRunner -from utils import config_test_file +from unittest import mock -from simdb.cli.simdb import cli +import pytest +from cli_helpers import make_simulation LOCAL_ALIASES = ["hello", "world", "foo-123"] REMOTE_ALIASES = ["foo#1", "bar", "barfoo", "123foo", "barbaz"] -def _generate_mock_data(get_local_db, remote_list_simulations): - simulations = [] - - for alias in REMOTE_ALIASES: - sim = mock.Mock() - sim.alias = alias - simulations.append(sim) - remote_list_simulations.return_value = simulations - simulations = [] - - for alias in LOCAL_ALIASES: - sim = mock.Mock() - sim.alias = alias - simulations.append(sim) - get_local_db.return_value.list_simulations.return_value = simulations - - -@mock.patch("simdb.cli.commands.alias.get_local_db") -@mock.patch("simdb.cli.remote_api.RemoteAPI.list_simulations") -@mock.patch("simdb.cli.remote_api.RemoteAPI.__init__") -def test_alias_search_command(init, remote_list_simulations, get_local_db): - init.return_value = None - _generate_mock_data(get_local_db, remote_list_simulations) - - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke( - cli, [f"--config-file={config_file}", "alias", "search", "foo"] - ) - assert result.exception is None - expected_sims = ["foo#1", "barfoo", "123foo", "foo-123"] - assert "\n".join(expected_sims) in result.output - - -@mock.patch("simdb.cli.commands.alias.get_local_db") -@mock.patch("simdb.cli.remote_api.RemoteAPI.list_simulations") -@mock.patch("simdb.cli.remote_api.RemoteAPI.has_url") -@mock.patch("simdb.cli.remote_api.RemoteAPI.__init__") -def test_alias_list_command(init, has_url, remote_list_simulations, get_local_db): - init.return_value = None - has_url.return_value = True - _generate_mock_data(get_local_db, remote_list_simulations) - - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke(cli, [f"--config-file={config_file}", "alias", "list"]) - assert result.exception is None +@pytest.fixture +def aliases(local_db, remote_handshake): + """A local database and a remote, each holding a known set of aliases.""" + local_db.list_simulations.return_value = [ + make_simulation(alias) for alias in LOCAL_ALIASES + ] + with mock.patch( + "simdb.cli.remote_api.RemoteAPI.list_simulations", + return_value=[make_simulation(alias) for alias in REMOTE_ALIASES], + ), mock.patch("simdb.cli.remote_api.RemoteAPI.has_url", return_value=True): + yield + + +def test_search_returns_local_and_remote_matches(invoke, aliases): + result = invoke("alias", "search", "foo") + + assert result.exit_code == 0 + assert "\n".join(["foo#1", "barfoo", "123foo", "foo-123"]) in result.output + + +def test_search_without_matches_prints_nothing(invoke, aliases): + result = invoke("alias", "search", "nothing-matches-this") + + assert result.exit_code == 0 + for alias in LOCAL_ALIASES + REMOTE_ALIASES: + assert alias not in result.output + + +def test_list_shows_the_local_aliases(invoke, aliases): + result = invoke("alias", "list") + + assert result.exit_code == 0 assert "\n ".join(LOCAL_ALIASES) in result.output -@mock.patch("simdb.cli.commands.alias.get_local_db") -@mock.patch("simdb.cli.remote_api.RemoteAPI.list_simulations") -@mock.patch("simdb.cli.remote_api.RemoteAPI.has_url") -@mock.patch("simdb.cli.remote_api.RemoteAPI.__init__") -def test_alias_list_command_with_remote_name( - init, has_url, remote_list_simulations, get_local_db -): - init.return_value = None - has_url.return_value = True - _generate_mock_data(get_local_db, remote_list_simulations) - - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke( - cli, [f"--config-file={config_file}", "alias", "test", "list"] - ) - assert result.exception is None +def test_list_with_a_remote_name_shows_both_sides(invoke, aliases): + result = invoke("alias", "test", "list") + + assert result.exit_code == 0 assert "\n ".join(REMOTE_ALIASES) in result.output assert "\n ".join(LOCAL_ALIASES) in result.output + + +def test_list_can_skip_the_remote(invoke, aliases): + result = invoke("alias", "list", "--local") + + assert result.exit_code == 0 + assert "Remote:" not in result.output + assert "\n ".join(LOCAL_ALIASES) in result.output + + +def test_list_explains_a_remote_without_a_url(invoke, local_db, remote_handshake): + local_db.list_simulations.return_value = [] + with mock.patch("simdb.cli.remote_api.RemoteAPI.has_url", return_value=False): + result = invoke("alias", "list") + + assert result.exit_code == 0 + assert "The Remote Server has not been specified" in result.output + + +def test_make_unique_returns_an_unused_alias_unchanged(invoke, aliases): + result = invoke("alias", "make-unique", "brand-new") + + assert result.exit_code == 0 + assert result.output.strip() == "brand-new" + + +def test_make_unique_appends_a_counter_to_a_taken_alias(invoke, aliases): + result = invoke("alias", "make-unique", "bar") + + assert result.exit_code == 0 + assert result.output.strip() == "bar-1" + + +def test_make_unique_replaces_reserved_characters(invoke, aliases): + result = invoke("alias", "make-unique", "a#b/c(d)e=f,g*h%i") + + assert result.exit_code == 0 + assert result.output.strip() == "a_b_c_d_e_f_g_h_i" + + +def test_make_unique_keeps_counting_past_a_taken_suffix(invoke, aliases, local_db): + local_db.list_simulations.return_value = [ + make_simulation("bar"), + make_simulation("bar-1"), + ] + + result = invoke("alias", "make-unique", "bar") + + assert result.exit_code == 0 + assert result.output.strip() == "bar-2" + + +def test_the_group_shows_help_when_no_subcommand_is_given(invoke): + result = invoke("alias") + + assert result.exit_code == 0 + assert "Query remote and local aliases." in result.output + assert "make-unique" in result.output diff --git a/tests/cli/test_cli_config_command.py b/tests/cli/test_cli_config_command.py index 1d278d9b..853c1b10 100644 --- a/tests/cli/test_cli_config_command.py +++ b/tests/cli/test_cli_config_command.py @@ -1,36 +1,96 @@ +"""Tests for ``simdb config``.""" + from unittest import mock -from click.testing import CliRunner -from utils import config_test_file +import pytest from simdb.cli.simdb import cli -@mock.patch("simdb.config.config.Config.get_option") -def test_config_get(get_option): - config_file = config_test_file() - get_option.return_value = "bar" - runner = CliRunner() - result = runner.invoke( - cli, [f"--config-file={config_file}", "config", "get", "foo"] - ) - assert result.exception is None +def test_get_prints_the_option_value(invoke): + with mock.patch( + "simdb.config.config.Config.get_option", return_value="bar" + ) as get_option: + result = invoke("config", "get", "foo") + + assert result.exit_code == 0 assert "bar" in result.output - (args, kwargs) = get_option.call_args - assert args == ("foo",) - assert kwargs == {} - - -@mock.patch("simdb.config.config.Config.save") -@mock.patch("simdb.config.config.Config.set_option") -def test_config_set(set_option, save): - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke( - cli, [f"--config-file={config_file}", "config", "set", "foo", "bar"] - ) - assert result.exception is None - (args, kwargs) = set_option.call_args - assert args == ("foo", "bar") - assert kwargs == {} + # Config.load also reads options, so only the final call is the command's. + assert get_option.call_args.args == ("foo",) + + +def test_get_reads_from_the_loaded_config_file(invoke): + result = invoke("config", "get", "remote.test.url") + + assert result.exit_code == 0 + assert "http://0.0.0.0:5000/" in result.output + + +def test_get_fails_for_an_unknown_option(invoke): + result = invoke("config", "get", "no.such.option") + + assert result.exit_code != 0 + + +def test_set_stores_the_option_and_saves(invoke): + with mock.patch("simdb.config.config.Config.save") as save, mock.patch( + "simdb.config.config.Config.set_option" + ) as set_option: + result = invoke("config", "set", "foo", "bar") + + assert result.exit_code == 0 + # Config.load sets options for the SIMDB_* environment variables first. + assert set_option.call_args.args == ("foo", "bar") + assert save.called + + +def test_delete_removes_the_option_and_saves(invoke): + with mock.patch("simdb.config.config.Config.save") as save, mock.patch( + "simdb.config.config.Config.delete_option" + ) as delete_option: + result = invoke("config", "delete", "foo") + + assert result.exit_code == 0 + assert "Success." in result.output + delete_option.assert_called_once_with("foo") assert save.called + + +def test_list_shows_the_configured_options(invoke): + result = invoke("config", "list") + + assert result.exit_code == 0 + assert "remote.test.url: http://0.0.0.0:5000/" in result.output + + +def test_list_masks_remote_tokens(invoke): + """A token in the configuration must never be echoed back in full.""" + result = invoke("config", "list") + + assert result.exit_code == 0 + assert "remote.test.token: ********" in result.output + assert "123ABC" not in result.output + + +def test_path_prints_the_config_file_that_was_loaded(invoke, tmp_path): + """``--config-file`` replaces the user configuration rather than adding to it.""" + result = invoke("config", "path") + + assert result.exit_code == 0 + assert str(tmp_path / "simdb.cfg") in result.output + + +def test_path_falls_back_to_the_user_configuration(runner, tmp_path): + """Without ``--config-file`` the location comes from SIMDB_USER_CONFIG_PATH.""" + result = runner.invoke(cli, ["config", "path"]) + + assert result.exit_code == 0 + assert str(tmp_path / "user-simdb.cfg") in result.output + + +@pytest.mark.parametrize("subcommand", ["get", "set", "delete"]) +def test_missing_arguments_are_reported(invoke, subcommand): + result = invoke("config", subcommand) + + assert result.exit_code == 2 + assert "Missing argument" in result.output diff --git a/tests/cli/test_cli_display.py b/tests/cli/test_cli_display.py new file mode 100644 index 00000000..f88bf18b --- /dev/null +++ b/tests/cli/test_cli_display.py @@ -0,0 +1,337 @@ +"""Tests for the console output helpers in ``simdb.cli.commands.utils``. + +These are the functions that turn simulations and IDS quantities into what the +user actually sees. They are pure enough to call directly, so they are tested +here without going through a command. +""" + +import pytest +from cli_helpers import make_simulation +from rich.console import Console + +from simdb.cli.commands import utils +from simdb.cli.commands.utils import ( + is_numeric_1d, + print_quantity, + print_simulations, + print_trace, + show_quantity_textual_plot, +) +from simdb.remote.models import QuantityData, SimulationTraceData + + +@pytest.fixture(autouse=True) +def fixed_width_console(monkeypatch): + """Give the rich output a stable width so assertions do not depend on the + terminal the tests happen to run in.""" + monkeypatch.setattr( + utils, "_RICH_CONSOLE", Console(width=120, legacy_windows=False) + ) + + +def quantity(data, name="grid/rho_tor_norm", units="-") -> QuantityData: + return QuantityData(name=name, units=units, data=data) + + +# --------------------------------------------------------------------------- +# is_numeric_1d +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + ("data", "expected"), + [ + ([1, 2, 3], True), + ([1.0, 2.5], True), + ([], False), + ([[1, 2], [3, 4]], False), + (["a", "b"], False), + # bool is an int subclass, but plotting booleans is not what the caller + # means by "numeric". + ([True, False], False), + (1.0, False), + (None, False), + ], +) +def test_is_numeric_1d(data, expected): + assert is_numeric_1d(data) is expected + + +# --------------------------------------------------------------------------- +# print_quantity +# --------------------------------------------------------------------------- + + +def test_print_quantity_renders_a_scalar(capsys): + print_quantity(quantity(1.23456789, name="time", units="s")) + + output = capsys.readouterr().out + assert "1.23457" in output + assert "scalar" in output + + +def test_print_quantity_renders_a_one_dimensional_array_with_stats(capsys): + print_quantity(quantity([0.0, 0.5, 1.0])) + + output = capsys.readouterr().out + assert "shape (3,)" in output + for column in ("n", "min", "max", "mean", "std", "median"): + assert column in output + assert "0.5" in output + + +def test_print_quantity_truncates_long_rows(capsys): + print_quantity(quantity(list(range(100)))) + + output = capsys.readouterr().out + assert "..." in output + assert "shape (100,)" in output + # Head and tail are kept, the middle is not. + assert "0 1 2" in output + assert "97 98 99" in output + assert "50" not in output + + +def test_print_quantity_renders_a_two_dimensional_array(capsys): + print_quantity(quantity([[1, 2], [3, 4]])) + + output = capsys.readouterr().out + assert "shape (2, 2)" in output + + +def test_print_quantity_truncates_tall_two_dimensional_arrays(capsys): + print_quantity(quantity([[row, row] for row in range(20)])) + + output = capsys.readouterr().out + assert "shape (20, 2)" in output + assert "..." in output + + +def test_print_quantity_summarises_higher_dimensional_arrays(capsys): + print_quantity(quantity([[[1, 2], [3, 4]], [[5, 6], [7, 8]]])) + + output = capsys.readouterr().out + assert "3-D array" in output + + +def test_print_quantity_can_omit_the_stats_table(capsys): + print_quantity(quantity([0.0, 0.5, 1.0]), show_stats=False) + + output = capsys.readouterr().out + assert "median" not in output + + +def test_print_quantity_uses_the_label_over_the_name(capsys): + print_quantity(quantity([1, 2, 3], name="grid/rho_tor_norm"), label="field") + + output = capsys.readouterr().out + assert "field" in output + assert "rho_tor_norm" not in output + + +def test_print_quantity_falls_back_to_a_dash_for_missing_units(capsys): + print_quantity(quantity(1.0, units="")) + + assert "[-]" in capsys.readouterr().out + + +def test_stats_are_omitted_for_a_single_value(capsys): + print_quantity(quantity([42.0])) + + output = capsys.readouterr().out + assert "shape (1,)" in output + assert "median" not in output + + +# --------------------------------------------------------------------------- +# show_quantity_textual_plot +# --------------------------------------------------------------------------- + + +def test_plot_is_drawn_for_a_numeric_field(capsys): + show_quantity_textual_plot(quantity([0.0, 1.0, 4.0, 9.0]), label="field") + + output = capsys.readouterr().out + assert "index [-]" in output + assert "shape (4,)" in output + + +def test_plot_uses_a_matching_coordinate_as_the_x_axis(capsys): + show_quantity_textual_plot( + quantity([0.0, 1.0, 4.0, 9.0]), + label="field", + x_quantity=quantity([0, 1, 2, 3], name="profiles_1d/time", units="s"), + ) + + output = capsys.readouterr().out + assert "time [s]" in output + assert "index [-]" not in output + + +def test_plot_ignores_a_coordinate_of_a_different_length(capsys): + show_quantity_textual_plot( + quantity([0.0, 1.0, 4.0]), + x_quantity=quantity([0, 1], name="time", units="s"), + ) + + assert "index [-]" in capsys.readouterr().out + + +def test_non_numeric_data_is_printed_instead_of_plotted(capsys): + show_quantity_textual_plot(quantity(["a", "b"]), label="field") + + output = capsys.readouterr().out + assert "index [-]" not in output + assert "shape (2,)" in output + + +# --------------------------------------------------------------------------- +# print_simulations +# --------------------------------------------------------------------------- + + +def test_print_simulations_reports_an_empty_list(capsys): + print_simulations([]) + + assert "No simulations found" in capsys.readouterr().out + + +def test_print_simulations_prints_a_single_alias_column(capsys): + print_simulations([make_simulation("first"), make_simulation("second")]) + + output = capsys.readouterr().out + assert "alias" in output + assert "first" in output + assert "second" in output + assert "UUID" not in output + assert "status" not in output + + +def test_print_simulations_adds_datetime_and_status_when_verbose(capsys): + print_simulations([make_simulation("first", status="passed")], verbose=True) + + output = capsys.readouterr().out + assert "datetime" in output + assert "status" in output + assert "passed" in output + + +def test_print_simulations_adds_the_uuid_column_on_request(capsys): + print_simulations([make_simulation("first", uuid="abcd1234")], show_uuid=True) + + output = capsys.readouterr().out + assert "UUID" in output + assert "abcd1234" in output + + +def test_print_simulations_adds_a_column_per_metadata_name(capsys): + simulations = [ + make_simulation("first", meta={"pulse": 134173}), + make_simulation("second", meta={}), + ] + + print_simulations(simulations, metadata_names=["pulse"]) + + output = capsys.readouterr().out + assert "pulse" in output + assert "134173" in output + # A simulation without the metadata still gets a row. + assert "second" in output + + +def test_print_simulations_handles_a_missing_alias(capsys): + print_simulations([make_simulation(None, uuid="abcd1234")], show_uuid=True) + + assert "abcd1234" in capsys.readouterr().out + + +def test_print_simulations_hints_at_the_limit_when_a_full_page_is_returned(capsys): + simulations = [make_simulation(f"sim{index}") for index in range(100)] + + print_simulations(simulations) + + assert "first 100 entries shown" in capsys.readouterr().out + + +def test_print_simulations_does_not_hint_below_the_limit(capsys): + print_simulations([make_simulation(f"sim{index}") for index in range(99)]) + + assert "first 100 entries shown" not in capsys.readouterr().out + + +@pytest.mark.parametrize( + ("value", "expected"), + [ + ({"min": 1, "max": 5}, "[1, 5]"), + ([1.0, 2.0], "[1.00, 2.00]"), + ([True, False], "[True, False]"), + (["a", "b"], "[a, b]"), + # Long lists are truncated to five entries. + (list(range(10)), "[0.00, 1.00, 2.00, 3.00, 4.00, ...]"), + ("plain", "plain"), + ], +) +def test_metadata_values_are_formatted_for_the_table(capsys, value, expected): + print_simulations( + [make_simulation("sim", meta={"field": value})], metadata_names=["field"] + ) + + assert expected in capsys.readouterr().out + + +# --------------------------------------------------------------------------- +# print_trace +# --------------------------------------------------------------------------- + + +def test_print_trace_reports_a_missing_trace(capsys): + print_trace(None) + + assert "No simulations trace found" in capsys.readouterr().out + + +def test_print_trace_prints_the_simulation_and_its_status_date(capsys): + trace = SimulationTraceData( + alias="current", status="passed", passed_on="2024-01-01" + ) + + print_trace(trace) + + output = capsys.readouterr().out + assert "current" in output + assert "passed" in output + assert "Passed on: 2024-01-01" in output + + +def test_print_trace_reports_an_unknown_status(capsys): + print_trace(SimulationTraceData(alias="current")) + + assert "Status: unknown" in capsys.readouterr().out + + +def test_print_trace_indents_the_replaced_simulation(capsys): + trace = SimulationTraceData( + alias="current", + status="passed", + replaces=SimulationTraceData(alias="older", status="deprecated"), + replaces_reason="superseded", + ) + + print_trace(trace) + + output = capsys.readouterr().out + assert "Replaces: (reason: superseded)" in output + assert " Simulation:" in output + assert "older" in output + + +def test_print_trace_handles_a_replacement_without_a_reason(capsys): + trace = SimulationTraceData( + alias="current", replaces=SimulationTraceData(alias="older") + ) + + print_trace(trace) + + output = capsys.readouterr().out + assert "Replaces:" in output + assert "reason" not in output diff --git a/tests/cli/test_cli_manifest_command.py b/tests/cli/test_cli_manifest_command.py index efc96740..6408ab21 100644 --- a/tests/cli/test_cli_manifest_command.py +++ b/tests/cli/test_cli_manifest_command.py @@ -1,54 +1,65 @@ -from unittest import mock +"""Tests for ``simdb manifest``.""" -from click.testing import CliRunner -from utils import config_test_file, create_manifest, get_file_path +import yaml -from simdb.cli.simdb import cli +from simdb.cli.manifest import Manifest -@mock.patch("simdb.cli.commands.manifest.Manifest") -def test_manifest_check_command(manifest): - config_file = config_test_file() - runner = CliRunner() - file_name = create_manifest() - result = runner.invoke( - cli, [f"--config-file={config_file}", "manifest", "check", str(file_name)] - ) - assert result.exception is None - assert "ok" in result.output - assert manifest.load_from_file.called - (args, kwargs) = manifest.load_from_file.call_args - assert str(args[0]) == str(file_name) - assert kwargs == {} - - -def test_manifest_check_command_integration(): - """Integration test that actually runs the manifest check without mocking.""" - config_file = config_test_file() - runner = CliRunner() - file_name = create_manifest() - result = runner.invoke( - cli, [f"--config-file={config_file}", "manifest", "check", str(file_name)] - ) - assert result.exception is None, f"Unexpected exception: {result.exception}" - assert result.exit_code == 0, ( - f"Exit code: {result.exit_code}, Output: {result.output}" - ) +def test_check_accepts_a_valid_manifest(invoke, manifest_file): + result = invoke("manifest", "check", str(manifest_file)) + + assert result.exit_code == 0 assert "ok" in result.output -@mock.patch("simdb.cli.commands.manifest.Manifest") -def test_manifest_create_command(manifest): - config_file = config_test_file() - runner = CliRunner() - file_name = get_file_path("manifest.yaml") - result = runner.invoke( - cli, [f"--config-file={config_file}", "manifest", "create", str(file_name)] - ) - assert result.exception is None - assert str(file_name) in result.output - assert manifest.from_template.called - assert manifest.from_template().save.called - (args, kwargs) = manifest.from_template().save.call_args - assert args[0].name == str(file_name) - assert kwargs == {} +def test_check_rejects_an_invalid_manifest(invoke, tmp_path): + manifest_file = tmp_path / "broken.yaml" + manifest_file.write_text("manifest_version: 2\nalias: 'not a valid alias'\n") + + result = invoke("manifest", "check", str(manifest_file)) + + assert result.exit_code != 0 + assert "illegal characters in alias" in str(result.exception) + + +def test_check_requires_the_file_to_exist(invoke, tmp_path): + result = invoke("manifest", "check", str(tmp_path / "missing.yaml")) + + assert result.exit_code == 2 + assert "does not exist" in result.output + + +def test_create_writes_a_manifest_from_the_template(invoke, tmp_path): + manifest_file = tmp_path / "new-manifest.yaml" + + result = invoke("manifest", "create", str(manifest_file)) + + assert result.exit_code == 0 + assert str(manifest_file) in result.output + assert manifest_file.exists() + assert yaml.safe_load(manifest_file.read_text())["manifest_version"] == 2 + + +def test_a_created_manifest_carries_the_template_placeholders(invoke, tmp_path): + """``create`` writes a skeleton, so ``check`` still has something to report. + + The template points at ``/home/user/path/to/a/file1`` and friends, which the + user is expected to replace; checking it unedited must say so rather than + pass silently. + """ + manifest_file = tmp_path / "new-manifest.yaml" + assert invoke("manifest", "create", str(manifest_file)).exit_code == 0 + + result = invoke("manifest", "check", str(manifest_file)) + + assert result.exit_code != 0 + assert "No files found matching path" in str(result.exception) + + +def test_check_loads_the_manifest_it_was_given(invoke, manifest_file): + """``check`` reports on the requested file, not on a default one.""" + loaded = Manifest.load_from_file(manifest_file) + + assert loaded.alias == "simulation-alias" + assert len(loaded.inputs) == 1 + assert len(loaded.outputs) == 1 diff --git a/tests/cli/test_cli_optional_remote_argument.py b/tests/cli/test_cli_optional_remote_argument.py new file mode 100644 index 00000000..25d9e4a1 --- /dev/null +++ b/tests/cli/test_cli_optional_remote_argument.py @@ -0,0 +1,105 @@ +"""The optional ``REMOTE`` argument of ``simdb simulation`` sub-commands. + +``simulation push``, ``pull``, ``data`` and ``validate`` all take an optional +REMOTE before their required arguments. Click cannot express "optional first +argument", so :class:`OptionalRemoteCommand` fills in an empty REMOTE when the +command line does not provide a value for every argument. + +Counting raw command line entries instead is what made ``push SIM_ID --replaces +=x`` fail with "Missing argument 'SIM_ID'": the option was counted as an +argument, so no empty REMOTE was inserted. +""" + +from unittest import mock + +import pytest +from cli_helpers import make_simulation + +SUBCOMMANDS = [ + ("push", ()), + ("validate", ()), + ("pull", ("directory",)), + ("data", ("core_profiles/time",)), +] +"""Each sub-command and the arguments that follow its SIM_ID.""" + + +@pytest.fixture +def remote_api(): + with mock.patch("simdb.cli.commands.simulation.RemoteAPI") as remote_api_cls: + remote_api_cls.return_value.get_validation_schemas.return_value = [] + yield remote_api_cls + + +@pytest.fixture(autouse=True) +def a_simulation_exists(local_db): + local_db.get_simulation.return_value = make_simulation("sim") + local_db.get_simulation.side_effect = None + return local_db + + +@pytest.mark.parametrize(("subcommand", "trailing"), SUBCOMMANDS) +@pytest.mark.parametrize( + "options", [("--username", "bob"), ()], ids=["username", "no-options"] +) +@pytest.mark.parametrize( + "options_first", [True, False], ids=["options-first", "options-last"] +) +@pytest.mark.parametrize( + "remote", [("test",), ()], ids=["named-remote", "default-remote"] +) +def test_the_remote_may_be_omitted( + invoke, remote_api, remote, options_first, options, subcommand, trailing +): + """REMOTE may be left out, wherever the options appear on the command line.""" + arguments = (*remote, "sim", *trailing) + argv = (*options, *arguments) if options_first else (*arguments, *options) + + result = invoke("simulation", subcommand, *argv) + + assert remote_api.called, result.output + used_remote, used_username = remote_api.call_args.args[:2] + assert used_remote == (remote[0] if remote else "") + assert used_username == ("bob" if options else None) + + +@pytest.mark.parametrize( + ("subcommand", "arguments", "option"), + [ + ("push", ["sim"], "--replaces=older"), + ("push", ["sim"], "--add-watcher"), + ("data", ["sim", "core_profiles/time"], "--dd-version=4.1.1"), + ], +) +def test_a_command_specific_option_does_not_consume_the_remote( + invoke, remote_api, subcommand, arguments, option +): + """Options that are not shared by every sub-command must count the same way.""" + result = invoke("simulation", subcommand, *arguments, option) + + assert "Missing argument" not in result.output + assert remote_api.call_args.args[0] == "" + + +@pytest.mark.parametrize( + ("subcommand", "trailing"), + [(subcommand, trailing) for subcommand, trailing in SUBCOMMANDS if trailing], +) +def test_a_genuinely_missing_argument_is_still_reported( + invoke, remote_api, subcommand, trailing +): + """Filling in the REMOTE must not paper over an argument the user forgot.""" + result = invoke("simulation", subcommand, "sim", *trailing[:-1]) + + assert result.exit_code == 2 + assert "Missing argument" in result.output + assert not remote_api.called + + +@pytest.mark.parametrize("subcommand", [subcommand for subcommand, _ in SUBCOMMANDS]) +def test_a_command_with_no_arguments_at_all_is_reported(invoke, remote_api, subcommand): + result = invoke("simulation", subcommand) + + assert result.exit_code == 2 + assert "Missing argument" in result.output + assert not remote_api.called diff --git a/tests/cli/test_cli_provenance_command.py b/tests/cli/test_cli_provenance_command.py index a68026f7..7d4cfe20 100644 --- a/tests/cli/test_cli_provenance_command.py +++ b/tests/cli/test_cli_provenance_command.py @@ -1,22 +1,48 @@ -from unittest import mock - -from click.testing import CliRunner -from utils import config_test_file, get_file_path - -from simdb.cli.simdb import cli - - -@mock.patch("yaml.dump") -def test_provenance_command(dump): - config_file = config_test_file() - runner = CliRunner() - file_name = get_file_path("provenance.yaml") - result = runner.invoke( - cli, [f"--config-file={config_file}", "provenance", str(file_name)] - ) - assert result.exception is None - assert str(file_name) in result.output - assert dump.called - (args, kwargs) = dump.call_args - assert args[1].name == str(file_name) - assert kwargs == {"default_flow_style": False} +"""Tests for ``simdb provenance``.""" + +import yaml + + +def test_provenance_writes_a_yaml_description_of_the_system(invoke, tmp_path): + provenance_file = tmp_path / "provenance.yaml" + + result = invoke("provenance", str(provenance_file)) + + assert result.exit_code == 0 + assert str(provenance_file) in result.output + + provenance = yaml.safe_load(provenance_file.read_text()) + assert set(provenance) == {"environment", "platform"} + assert provenance["platform"]["system"] + assert provenance["platform"]["python_version"] + + +def test_path_like_environment_variables_are_split_into_lists( + invoke, tmp_path, monkeypatch +): + monkeypatch.setenv("SIMDB_TEST_PATH", "/first:/second") + provenance_file = tmp_path / "provenance.yaml" + + assert invoke("provenance", str(provenance_file)).exit_code == 0 + + environment = yaml.safe_load(provenance_file.read_text())["environment"] + assert environment["SIMDB_TEST_PATH"] == ["/first", "/second"] + + +def test_other_environment_variables_are_kept_as_strings(invoke, tmp_path, monkeypatch): + monkeypatch.setenv("SIMDB_TEST_VALUE", "plain") + provenance_file = tmp_path / "provenance.yaml" + + assert invoke("provenance", str(provenance_file)).exit_code == 0 + + environment = yaml.safe_load(provenance_file.read_text())["environment"] + assert environment["SIMDB_TEST_VALUE"] == "plain" + + +def test_the_file_is_overwritten_on_a_second_run(invoke, tmp_path): + provenance_file = tmp_path / "provenance.yaml" + provenance_file.write_text("stale: true\n") + + assert invoke("provenance", str(provenance_file)).exit_code == 0 + + assert "stale" not in provenance_file.read_text() diff --git a/tests/cli/test_cli_remote_api_client.py b/tests/cli/test_cli_remote_api_client.py new file mode 100644 index 00000000..39116a9c --- /dev/null +++ b/tests/cli/test_cli_remote_api_client.py @@ -0,0 +1,374 @@ +"""Tests for the HTTP layer of :class:`simdb.cli.remote_api.RemoteAPI`. + +The other CLI tests stub ``RemoteAPI`` methods so they can concentrate on the +commands. Here it is the client itself that is under test, so only ``requests`` +is replaced: construction, authentication, URL building, error translation and +response parsing all run for real. +""" + +import json +from unittest import mock + +import pytest +import requests +from pydantic import ValidationError + +from simdb.cli.remote_api import ( + FailedConnection, + RemoteAPI, + RemoteError, + check_return, + try_request, +) +from simdb.config import Config +from simdb.remote.models import TokenResponse + +INDEX = { + "api": "SimDB", + "api_version": "1.3", + "server_version": "0.11", + "endpoints": ["http://remote.test/v1.2", "http://remote.test/v1.3"], + "authentication": "None", +} + + +def response(payload=None, status=200, content=None) -> requests.Response: + """Build a real :class:`requests.Response` around a JSON payload.""" + res = requests.Response() + res.status_code = status + res.url = "http://remote.test/" + res.reason = "OK" if status == 200 else "Error" + if content is None: + content = json.dumps(payload if payload is not None else {}).encode() + res._content = content + return res + + +class FakeHttp: + """Answer ``requests.get``/``post`` from a URL suffix to payload mapping.""" + + def __init__(self, index=None): + self.routes = {"": index if index is not None else dict(INDEX)} + self.calls = [] + + def route(self, suffix, payload=None, status=200, content=None): + self.routes[suffix] = response(payload, status=status, content=content) + + def _respond(self, method, url, **kwargs): + self.calls.append(mock.call(method, url, **kwargs)) + suffix = url.split("/v1.3/", 1)[-1] if "/v1.3/" in url else "" + if url.rstrip("/") in ("http://remote.test", "http://remote.test/"): + suffix = "" + route = self.routes.get(suffix) + if route is None: + return response({"error": f"no route for {url!r}"}, status=404) + return route if isinstance(route, requests.Response) else response(route) + + def request_for(self, suffix): + """The recorded call whose URL ends with the given suffix.""" + for call in self.calls: + if call.args[1].endswith(suffix): + return call + raise AssertionError(f"no request was made to {suffix!r}") + + +@pytest.fixture +def http(monkeypatch): + fake = FakeHttp() + for method in ("get", "post", "put", "delete"): + monkeypatch.setattr( + requests, + method, + lambda url, _method=method, **kwargs: fake._respond(_method, url, **kwargs), + ) + return fake + + +def make_config(**options) -> Config: + config = Config() + config.set_option("remote.test.url", "http://remote.test") + for name, value in options.items(): + config.set_option(f"remote.test.{name}", value) + return config + + +# --------------------------------------------------------------------------- +# construction +# --------------------------------------------------------------------------- + + +def test_the_negotiated_version_is_used_for_requests(http): + api = RemoteAPI("test", None, None, make_config()) + + assert api._api_url == "http://remote.test/v1.3/" + assert api.remote == "test" + assert api.has_url() is True + + +def test_the_default_remote_is_used_when_no_name_is_given(http): + config = make_config() + config.default_remote = "test" + + api = RemoteAPI(None, None, None, config) + + assert api.remote == "test" + + +def test_a_missing_remote_name_without_a_default_is_reported(http): + with pytest.raises(KeyError, match="no default remote"): + RemoteAPI(None, None, None, Config()) + + +def test_an_unknown_remote_is_reported(http): + with pytest.raises(ValueError, match="Remote 'other' not found"): + RemoteAPI("other", None, None, make_config()) + + +def test_a_password_without_a_username_is_rejected(http): + with pytest.raises(ValueError, match="Password given but no username"): + RemoteAPI("test", None, "secret", make_config()) + + +def test_authentication_without_credentials_or_a_token_is_rejected(http): + http.routes[""] = response({**INDEX, "authentication": "LDAP"}) + + with pytest.raises(ValueError, match="No username or password given"): + RemoteAPI("test", None, None, make_config()) + + +def test_a_remote_without_a_usable_version_is_reported(http): + http.routes[""] = response({**INDEX, "endpoints": ["http://remote.test/v9"]}) + + with pytest.raises(RemoteError, match="No compatible API version"): + RemoteAPI("test", None, None, make_config()) + + +def test_the_api_version_comes_from_the_negotiated_endpoint(http): + http.routes[""] = response({**INDEX, "api_version": None}) + + api = RemoteAPI("test", None, None, make_config()) + + # The negotiated endpoint decides the version, so a remote that reports no + # api_version of its own is still usable. + assert str(api.version) == "1.3.0" + + +def test_a_remote_that_reports_no_server_version_is_reported(http): + http.routes[""] = response({**INDEX, "server_version": None}) + + with pytest.raises(RemoteError, match="did not report a server version"): + RemoteAPI("test", None, None, make_config()) + + +def test_the_selected_version_is_announced_when_verbose(http, capsys): + config = make_config() + config.verbose = True + + RemoteAPI("test", None, None, config) + + assert "Selected API version v1.3" in capsys.readouterr().out + + +# --------------------------------------------------------------------------- +# authentication +# --------------------------------------------------------------------------- + + +def test_no_credentials_are_sent_to_an_unauthenticated_remote(http): + api = RemoteAPI("test", None, None, make_config()) + http.route("validation_schema", []) + + api.get_validation_schemas() + + assert "auth" not in http.request_for("validation_schema").kwargs + + +def test_a_token_is_sent_as_a_jwt_header(http): + http.routes[""] = response({**INDEX, "authentication": "LDAP"}) + api = RemoteAPI("test", None, None, make_config(token="123ABC")) + http.route("validation_schema", []) + + api.get_validation_schemas() + + auth = http.request_for("validation_schema").kwargs["auth"] + request = auth(mock.Mock(headers={})) + assert request.headers["Authorization"] == "JWT-Token 123ABC" + + +def test_a_username_and_password_are_sent_as_basic_auth(http): + http.routes[""] = response({**INDEX, "authentication": "LDAP"}) + api = RemoteAPI("test", "user", "secret", make_config()) + http.route("validation_schema", []) + + api.get_validation_schemas() + + assert http.request_for("validation_schema").kwargs["auth"] == ("user", "secret") + + +def test_credentials_are_prompted_for_when_the_remote_authenticates(http): + http.routes[""] = response({**INDEX, "authentication": "LDAP"}) + + with mock.patch("click.prompt", side_effect=["user", "secret"]) as prompt: + api = RemoteAPI("test", None, None, make_config(), use_token=False) + + assert prompt.call_count == 2 + assert api._username == "user" + assert api._password == "secret" + + +# --------------------------------------------------------------------------- +# requests and responses +# --------------------------------------------------------------------------- + + +def test_requests_are_sent_to_the_versioned_url(http): + api = RemoteAPI("test", None, None, make_config()) + http.route("token", {"token": "NEWTOKEN"}) + + assert api.get_token() == "NEWTOKEN" + assert http.request_for("token").args[1] == "http://remote.test/v1.3/token" + + +def test_the_api_version_context_redirects_requests(http): + api = RemoteAPI("test", None, None, make_config()) + http.route("validation_schema", []) + + with api.api_version("v1.2"): + assert api._api_url == "http://remote.test/v1.2/" + + assert api._api_url == "http://remote.test/v1.3/" + + +def test_list_simulations_sends_the_pagination_headers(http): + api = RemoteAPI("test", None, None, make_config()) + http.route( + "simulations", + { + "count": 1, + "page": 1, + "limit": 10, + "results": [ + { + "uuid": {"_type": "uuid.UUID", "hex": "0" * 32}, + "alias": "sim", + "datetime": "2000-01-01", + } + ], + }, + ) + + simulations = api.list_simulations(limit=10) + + assert [simulation.alias for simulation in simulations] == ["sim"] + headers = http.request_for("simulations").kwargs["headers"] + assert headers["simdb-result-limit"] == "10" + + +def test_list_simulations_appends_the_requested_metadata(http): + api = RemoteAPI("test", None, None, make_config()) + http.route( + "simulations?pulse&run", {"count": 0, "page": 1, "limit": 0, "results": []} + ) + + api.list_simulations(meta=["pulse", "run"]) + + assert http.request_for("simulations?pulse&run") + + +def test_an_error_payload_becomes_a_remote_error(http): + api = RemoteAPI("test", None, None, make_config()) + http.route("validation_schema", {"error": "you shall not pass"}, status=403) + + with pytest.raises(RemoteError, match="you shall not pass"): + api.get_validation_schemas() + + +def test_an_error_without_a_payload_becomes_a_failed_connection(http): + api = RemoteAPI("test", None, None, make_config()) + http.route("validation_schema", status=500, content=b"oops") + + with pytest.raises(FailedConnection, match="HTTP error 500"): + api.get_validation_schemas() + + +def test_a_non_json_response_becomes_a_failed_connection(http): + """A firewall login page is HTML where the API promised JSON.""" + api = RemoteAPI("test", None, None, make_config()) + http.route("validation_schema", content=b"login") + + with pytest.raises(FailedConnection, match="Invalid JSON"): + api.get_validation_schemas() + + +def test_unexpected_json_becomes_a_remote_error(http): + api = RemoteAPI("test", None, None, make_config()) + http.route("token", {"not_a_token": True}) + + with pytest.raises(RemoteError, match="Unexpected data exchanged"): + api.get_token() + + +# --------------------------------------------------------------------------- +# check_return and try_request, tested on their own +# --------------------------------------------------------------------------- + + +def test_check_return_accepts_a_successful_response(): + assert check_return(response({"ok": True})) is None + + +def test_check_return_raises_for_status_without_an_error_field(): + with pytest.raises(requests.HTTPError): + check_return(response({"detail": "nope"}, status=404)) + + +def test_try_request_translates_a_connection_error(): + request = mock.Mock(url="http://remote.test/v1.3/simulations") + + @try_request + def failing(): + raise requests.ConnectionError(request=request) + + with pytest.raises(FailedConnection, match="Connection failed to"): + failing() + + +def test_try_request_reports_an_unknown_url_for_a_request_less_error(): + @try_request + def failing(): + raise requests.ConnectionError(request=None) + + with pytest.raises(FailedConnection, match="undefined"): + failing() + + +def test_try_request_translates_a_json_decode_error(): + @try_request + def failing(): + raise requests.JSONDecodeError("bad", "doc", 0) + + with pytest.raises(FailedConnection, match="Invalid JSON"): + failing() + + +def test_try_request_translates_a_validation_error(): + @try_request + def failing(): + TokenResponse.model_validate({"wrong": "shape"}) + + with pytest.raises(RemoteError, match="Unexpected data exchanged"): + failing() + + +def test_try_request_passes_a_successful_call_through(): + @try_request + def succeeding(value): + return value + + assert succeeding(42) == 42 + + +def test_a_validation_error_is_not_swallowed_as_something_else(): + """Only ``json_invalid`` errors mean the response was not JSON at all.""" + with pytest.raises(ValidationError): + TokenResponse.model_validate_json(b"{}") diff --git a/tests/cli/test_cli_remote_command.py b/tests/cli/test_cli_remote_command.py index 63905285..b03d7941 100644 --- a/tests/cli/test_cli_remote_command.py +++ b/tests/cli/test_cli_remote_command.py @@ -1,305 +1,469 @@ +"""Tests for ``simdb remote``. + +The remote handshake is stubbed by the ``remote_handshake`` fixture; each test +stubs only the one API call its command makes. +""" + +import uuid from unittest import mock -from click.testing import CliRunner -from utils import config_test_file +import pytest +from cli_helpers import make_simulation -from simdb.cli.simdb import cli +from simdb.cli.remote_api import RemoteAPI +from simdb.database.models.simulation import Simulation from simdb.notifications import Notification +from simdb.remote.models import SimulationTraceData, WatcherData +pytestmark = pytest.mark.usefixtures("remote_handshake") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_server_authentication") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_endpoints") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_server_version") -@mock.patch("simdb.cli.remote_api.RemoteAPI.list_watchers") -def test_remote_watchers_list_command( - list_watchers, - get_server_version, - get_endpoints, - get_server_authentication, -): - get_endpoints.return_value = ["v1", "v1.1", "v1.1.1", "v1.2"] - get_server_version.return_value = "0.11" - get_server_authentication.return_value = "None" - sim_id = "acbd1234" - watchers = ["a", "b", "c"] - list_watchers.return_value = watchers - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke( - cli, [f"--config-file={config_file}", "remote", "watcher", "list", sim_id] - ) - assert result.exception is None - assert sim_id in result.output + +@pytest.fixture +def api_call(): + """Stub a single :class:`RemoteAPI` method by name. + + ``api_call("list_simulations", return_value=[...])`` patches the method for + the duration of the test and hands back the mock to assert on. + """ + patches = [] + + def _api_call(name, **kwargs): + patcher = mock.patch.object(RemoteAPI, name, **kwargs) + patches.append(patcher) + return patcher.start() + + yield _api_call + + for patcher in reversed(patches): + patcher.stop() + + +# --------------------------------------------------------------------------- +# remote test / directory +# --------------------------------------------------------------------------- + + +def test_test_command_reports_the_remote_api_version(invoke, remote_handshake): + result = invoke("remote", "test") + + assert result.exit_code == 0 + assert "Remote is valid" in result.output + assert "1.3" in result.output + + +def test_directory_prints_the_remote_storage_directory(invoke, api_call): + api_call("get_directory", return_value="/srv/simdb/data") + + result = invoke("remote", "directory") + + assert result.exit_code == 0 + assert "/srv/simdb/data" in result.output + + +# --------------------------------------------------------------------------- +# remote watcher +# --------------------------------------------------------------------------- + + +def test_watcher_list_prints_every_watcher(invoke, api_call, remote_handshake): + watchers = [ + WatcherData(username="a", email="a@simdb.test", notification="A"), + WatcherData(username="b", email="b@simdb.test", notification="V"), + WatcherData(username="c", email="c@simdb.test", notification="R"), + ] + list_watchers = api_call("list_watchers", return_value=watchers) + + result = invoke("remote", "watcher", "list", "acbd1234") + + assert result.exit_code == 0 + assert "acbd1234" in result.output for watcher in watchers: - assert watcher in result.output + assert watcher.username in result.output + assert watcher.email in result.output assert list_watchers.called + assert remote_handshake.get_endpoints.called -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_server_authentication") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_endpoints") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_server_version") -@mock.patch("simdb.cli.remote_api.RemoteAPI.remove_watcher") -def test_remote_watcher_remove_command( - remove_watcher, - get_server_version, - get_endpoints, - get_server_authentication, -): - get_endpoints.return_value = ["v1", "v1.1", "v1.1.1", "v1.2"] - get_server_version.return_value = "0.11" - get_server_authentication.return_value = "None" - user = "test" - sim_id = "acbd1234" - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke( - cli, - [ - f"--config-file={config_file}", - "remote", - "watcher", - "remove", - sim_id, - f"--user={user}", - ], +def test_watcher_list_reports_no_watchers(invoke, api_call): + api_call("list_watchers", return_value=[]) + + result = invoke("remote", "watcher", "list", "acbd1234") + + assert result.exit_code == 0 + assert "no watchers found for simulation acbd1234" in result.output + + +def test_watcher_remove_passes_the_user_on(invoke, api_call): + remove_watcher = api_call("remove_watcher") + + result = invoke("remote", "watcher", "remove", "acbd1234", "--user=test") + + assert result.exit_code == 0 + assert "acbd1234" in result.output + assert remove_watcher.call_args.args == ("acbd1234", "test") + + +def test_watcher_remove_fails_without_a_user(invoke, api_call): + remove_watcher = api_call("remove_watcher") + + result = invoke("remote", "watcher", "remove", "acbd1234") + + assert result.exit_code != 0 + assert not remove_watcher.called + + +@pytest.mark.xfail( + strict=True, + reason=( + "remove_watcher calls get_string_option('user.name') without default=None, " + "so the config lookup raises KeyError before its own error message is built" + ), +) +def test_watcher_remove_explains_a_missing_user(invoke, api_call): + """``watcher add`` reports this cleanly; ``watcher remove`` should match it.""" + api_call("remove_watcher") + + result = invoke("remote", "watcher", "remove", "acbd1234") + + assert "User not provided and user.name not found in config" in result.output + + +def test_watcher_add_passes_user_email_and_notification(invoke, api_call): + add_watcher = api_call("add_watcher") + + result = invoke( + "remote", + "watcher", + "add", + "acbd1234", + "--user=test", + "--email=test@iter.org", + "--notification=all", ) - assert result.exception is None - assert sim_id in result.output - assert remove_watcher.called - (args, kwargs) = remove_watcher.call_args - assert args == (sim_id, user) - assert kwargs == {} - - -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_server_authentication") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_endpoints") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_server_version") -@mock.patch("simdb.cli.remote_api.RemoteAPI.add_watcher") -def test_remote_watcher_add_command( - add_watcher, - get_server_version, - get_endpoints, - get_server_authentication, -): - get_endpoints.return_value = ["v1", "v1.1", "v1.1.1", "v1.2"] - get_server_version.return_value = "0.11" - get_server_authentication.return_value = "None" - user = "test" - email = "test@iter.org" - sim_id = "acbd1234" - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke( - cli, - [ - f"--config-file={config_file}", - "remote", - "watcher", - "add", - sim_id, - f"--user={user}", - f"--email={email}", - "--notification=all", - ], + + assert result.exit_code == 0 + assert "acbd1234" in result.output + assert add_watcher.call_args.args == ( + "acbd1234", + "test", + "test@iter.org", + Notification.ALL, ) - assert result.exception is None - assert sim_id in result.output - assert add_watcher.called - (args, kwargs) = add_watcher.call_args - assert args == (sim_id, user, email, Notification.ALL) - assert kwargs == {} - - -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_server_authentication") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_endpoints") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_server_version") -@mock.patch("simdb.cli.remote_api.RemoteAPI.list_simulations") -def test_remote_list_command( - list_simulations, - get_server_version, - get_endpoints, - get_server_authentication, -): - get_endpoints.return_value = ["v1", "v1.1", "v1.1.1", "v1.2"] - get_server_version.return_value = "0.11" - get_server_authentication.return_value = "None" - data = [ - ("abcd1234", "test"), - ("abcd5678", "test"), - ("abcd4321", "test"), - ] - sims = [] - for el in data: - sim = mock.Mock() - sim.uuid = el[0] - sim.alias = el[1] - sims.append(sim) - list_simulations.return_value = sims - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke( - cli, [f"--config-file={config_file}", "remote", "list", "--uuid"] + + +def test_watcher_add_needs_an_email(invoke, api_call): + add_watcher = api_call("add_watcher") + + result = invoke("remote", "watcher", "add", "acbd1234", "--user=test") + + assert result.exit_code != 0 + assert "Email not provided and user.email not found in config" in result.output + assert not add_watcher.called + + +def test_watcher_add_rejects_an_unknown_notification(invoke, api_call): + add_watcher = api_call("add_watcher") + + result = invoke( + "remote", "watcher", "add", "acbd1234", "--user=t", "-e=t@x", "-n=sometimes" ) - assert result.exception is None - assert list_simulations.called - for el in data: - for i in el: - assert i in result.output - - -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_server_authentication") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_endpoints") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_server_version") -@mock.patch("simdb.cli.remote_api.RemoteAPI.list_simulations") -def test_remote_list_command_with_verbose( - list_simulations, - get_server_version, - get_endpoints, - get_server_authentication, -): - get_endpoints.return_value = ["v1", "v1.1", "v1.1.1", "v1.2"] - get_server_version.return_value = "0.11" - get_server_authentication.return_value = "None" - data = [ - ("abcd1234", "test", "2000-01-01-01", "Validated"), - ("abcd5678", "test", "2000-02-02-02", "Validated"), - ("abcd4321", "test", "2000-03-03-03", "Validated"), + + assert result.exit_code == 2 + assert not add_watcher.called + + +# --------------------------------------------------------------------------- +# remote list / info / query +# --------------------------------------------------------------------------- + + +def test_list_prints_the_returned_simulations(invoke, api_call, remote_handshake): + simulations = [ + make_simulation("test", uuid="abcd1234"), + make_simulation("test", uuid="abcd5678"), + make_simulation("test", uuid="abcd4321"), ] - sims = [] - for el in data: - sim = mock.Mock() - sim.uuid = el[0] - sim.alias = el[1] - sim.datetime = el[2] - sim.status = el[3] - sims.append(sim) - list_simulations.return_value = sims - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke( - cli, [f"--config-file={config_file}", "--verbose", "remote", "list", "--uuid"] - ) - assert result.exception is None + list_simulations = api_call("list_simulations", return_value=simulations) + + result = invoke("remote", "list", "--uuid") + + assert result.exit_code == 0 + for simulation in simulations: + assert simulation.uuid in result.output assert list_simulations.called - for el in data: - for i in el: - assert i in result.output - - -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_server_authentication") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_endpoints") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_server_version") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_simulation") -def test_remote_info_command( - get_simulation, - get_server_version, - get_endpoints, - get_server_authentication, -): - get_endpoints.return_value = ["v1", "v1.1", "v1.1.1", "v1.2"] - get_server_version.return_value = "0.11" - get_server_authentication.return_value = "None" - sim_id = "abcd1234" - sim = ("abcd1234", "test", "2000-01-01-01", "Validated") - get_simulation.return_value = sim - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke( - cli, [f"--config-file={config_file}", "remote", "info", sim_id] + assert remote_handshake.get_endpoints.called + + +def test_list_shows_the_extra_columns_when_verbose(invoke, api_call): + api_call( + "list_simulations", + return_value=[ + make_simulation( + "test", uuid="abcd1234", datetime="2000-01-01-01", status="passed" + ) + ], ) - assert result.exception is None - assert str(sim) in result.output - assert get_simulation.called - (args, kwargs) = get_simulation.call_args - assert args == (sim_id,) - assert kwargs == {} - - -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_server_authentication") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_endpoints") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_server_version") -@mock.patch("simdb.cli.remote_api.RemoteAPI.query_simulations") -def test_remote_query_command( - query_simulations, - get_server_version, - get_endpoints, - get_server_authentication, -): - get_endpoints.return_value = ["v1", "v1.1", "v1.1.1", "v1.2"] - get_server_version.return_value = "0.11" - get_server_authentication.return_value = "None" - data = [ - ("abcd1234", "123"), - ] - sims = [] - for el in data: - sim = mock.Mock() - sim.uuid = el[0] - sim.alias = el[1] - sim.find_meta.return_value = [] - sims.append(sim) + + result = invoke("--verbose", "remote", "list", "--uuid") + + assert result.exit_code == 0 + assert "2000-01-01-01" in result.output + assert "passed" in result.output + + +def test_list_rejects_a_negative_limit(invoke, api_call): + list_simulations = api_call("list_simulations") + + result = invoke("remote", "list", "--limit=-1") + + assert result.exit_code == 2 + assert not list_simulations.called + + +def test_info_prints_the_simulation(invoke, api_call): + get_simulation = api_call("get_simulation", return_value="simulation description") + + result = invoke("remote", "info", "abcd1234") + + assert result.exit_code == 0 + assert "simulation description" in result.output + assert get_simulation.call_args.args == ("abcd1234",) + + +def test_query_forwards_the_constraints(invoke, api_call): constraints = ("alias=123", "description=in:test") - query_simulations.return_value = sims - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke( - cli, [f"--config-file={config_file}", "remote", "query", "--uuid", *constraints] + query_simulations = api_call( + "query_simulations", return_value=[make_simulation("123", uuid="abcd1234")] ) - assert result.exception is None - for el in data: - for i in el: - assert i in result.output - assert query_simulations.called - (args, kwargs) = query_simulations.call_args - assert args == (constraints, (), 100) - assert kwargs == {} - - -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_server_authentication") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_endpoints") -@mock.patch("simdb.cli.remote_api.RemoteAPI.get_server_version") -@mock.patch("simdb.cli.remote_api.RemoteAPI.query_simulations") -def test_remote_query_command_with_verbose( - query_simulations, - get_server_version, - get_endpoints, - get_server_authentication, -): - get_endpoints.return_value = ["v1", "v1.1", "v1.1.1", "v1.2"] - get_server_version.return_value = "0.11" - get_server_authentication.return_value = "None" - data = [ - ("abcd1234", "123", "2000-01-01-01", "Validated"), - ] - sims = [] - for el in data: - sim = mock.Mock() - sim.uuid = el[0] - sim.alias = el[1] - sim.datetime = el[2] - sim.status = el[3] - sim.find_meta.return_value = [] - sims.append(sim) - constraints = ("alias=123", "description=in:test") - query_simulations.return_value = sims - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke( - cli, - [ - f"--config-file={config_file}", - "--verbose", - "remote", - "query", - "--uuid", - *constraints, + + result = invoke("remote", "query", "--uuid", *constraints) + + assert result.exit_code == 0 + assert "abcd1234" in result.output + assert query_simulations.call_args.args == (constraints, (), 100) + + +def test_query_shows_the_extra_columns_when_verbose(invoke, api_call): + api_call( + "query_simulations", + return_value=[ + make_simulation( + "123", uuid="abcd1234", datetime="2000-01-01-01", status="passed" + ) ], ) - assert result.exception is None - for el in data: - for i in el: - assert i in result.output - assert query_simulations.called - (args, kwargs) = query_simulations.call_args - assert args == (constraints, (), 100) - assert kwargs == {} + + result = invoke("--verbose", "remote", "query", "--uuid", "alias=123") + + assert result.exit_code == 0 + assert "2000-01-01-01" in result.output + assert "passed" in result.output + + +# --------------------------------------------------------------------------- +# remote version / trace +# --------------------------------------------------------------------------- + + +def test_version_prints_the_remote_simdb_version(invoke): + result = invoke("remote", "version") + + assert result.exit_code == 0 + assert "Remote 'test' SimDB version: 0.11" in result.output + + +def test_trace_prints_the_provenance_chain(invoke, api_call): + trace_simulation = api_call( + "trace_simulation", + return_value=SimulationTraceData( + alias="current", + status="passed", + replaces=SimulationTraceData(alias="older", status="deprecated"), + ), + ) + + result = invoke("remote", "trace", "abcd1234") + + assert result.exit_code == 0 + assert "current" in result.output + assert "older" in result.output + assert trace_simulation.call_args.args == ("abcd1234",) + + +def test_trace_reports_a_missing_trace(invoke, api_call): + api_call("trace_simulation", return_value=None) + + result = invoke("remote", "trace", "abcd1234") + + assert result.exit_code == 0 + assert "No simulations trace found" in result.output + + +# --------------------------------------------------------------------------- +# remote schema +# --------------------------------------------------------------------------- + + +def test_schema_prints_the_validation_schemas(invoke, api_call): + api_call("get_validation_schemas", return_value=[{"alias": {"required": True}}]) + + result = invoke("remote", "schema") + + assert result.exit_code == 0 + assert "alias" in result.output + + +def test_schema_rejects_a_non_positive_depth(invoke, api_call): + get_validation_schemas = api_call("get_validation_schemas", return_value=[]) + + result = invoke("remote", "schema", "--depth=0") + + assert result.exit_code == 2 + assert "must be greater than zero" in result.output + assert not get_validation_schemas.called + + +# --------------------------------------------------------------------------- +# remote token +# --------------------------------------------------------------------------- + + +def test_token_new_stores_the_token_in_the_configuration(invoke, api_call, config_file): + api_call("get_token", return_value="NEWTOKEN") + + result = invoke("remote", "token", "new") + + assert result.exit_code == 0 + assert "Token added for remote test." in result.output + assert "NEWTOKEN" in config_file.read_text() + + +def test_token_delete_removes_the_token(invoke, config_file): + result = invoke("remote", "token", "delete") + + assert result.exit_code == 0 + assert "Token for remote test deleted." in result.output + assert "123ABC" not in config_file.read_text() + + +def test_token_delete_can_be_repeated(invoke): + assert invoke("remote", "token", "delete").exit_code == 0 + + result = invoke("remote", "token", "delete") + + assert result.exit_code == 0 + + +@pytest.mark.xfail( + strict=True, + reason=( + "Config.delete_option only raises KeyError for a missing section; " + "configparser.remove_option returns False for a missing option instead " + "of raising, so the deletion is reported as successful" + ), +) +def test_token_delete_says_when_there_was_no_token(invoke): + assert invoke("remote", "token", "delete").exit_code == 0 + + result = invoke("remote", "token", "delete") + + assert "No token for remote test found." in result.output + + +# --------------------------------------------------------------------------- +# remote admin +# --------------------------------------------------------------------------- + + +def test_admin_set_meta_reports_an_update(invoke, api_call): + set_metadata = api_call("set_metadata", return_value="old") + + result = invoke("remote", "admin", "set-meta", "abcd1234", "pulse", "134173") + + assert result.exit_code == 0 + assert "old -> 134173" in result.output + assert set_metadata.call_args.args == ("abcd1234", "pulse", "134173") + + +def test_admin_set_meta_reports_a_new_value(invoke, api_call): + api_call("set_metadata", return_value=None) + + result = invoke("remote", "admin", "set-meta", "abcd1234", "pulse", "134173") + + assert result.exit_code == 0 + assert "Added pulse for simulation abcd1234 with value '134173'" in result.output + + +@pytest.mark.parametrize( + ("type_name", "value", "expected"), + [ + ("int", "42", 42), + ("float", "1.5", 1.5), + ("string", "42", "42"), + ], +) +def test_admin_set_meta_converts_the_value( + invoke, api_call, type_name, value, expected +): + set_metadata = api_call("set_metadata", return_value=None) + + result = invoke( + "remote", "admin", "set-meta", "abcd1234", "key", value, f"--type={type_name}" + ) + + assert result.exit_code == 0 + assert set_metadata.call_args.args[2] == expected + + +def test_admin_set_meta_converts_a_uuid(invoke, api_call): + set_metadata = api_call("set_metadata", return_value=None) + value = "12345678123456781234567812345678" + + result = invoke( + "remote", "admin", "set-meta", "abcd1234", "key", value, "--type=UUID" + ) + + assert result.exit_code == 0 + assert set_metadata.call_args.args[2] == uuid.UUID(value) + + +def test_admin_set_status_updates_the_status(invoke, api_call): + update_simulation = api_call("update_simulation", return_value="not validated") + + result = invoke("remote", "admin", "set-status", "abcd1234", "PASSED") + + assert result.exit_code == 0 + assert "not validated -> PASSED" in result.output + assert update_simulation.call_args.args == ( + "abcd1234", + Simulation.Status.PASSED, + ) + + +def test_admin_set_status_rejects_an_unknown_status(invoke, api_call): + update_simulation = api_call("update_simulation") + + result = invoke("remote", "admin", "set-status", "abcd1234", "excellent") + + assert result.exit_code == 2 + assert not update_simulation.called + + +def test_admin_del_meta_removes_the_key(invoke, api_call): + delete_metadata = api_call("delete_metadata") + + result = invoke("remote", "admin", "del-meta", "abcd1234", "pulse") + + assert result.exit_code == 0 + assert "Deleted pulse for simulation abcd1234" in result.output + assert delete_metadata.call_args.args == ("abcd1234", "pulse") + + +def test_admin_delete_removes_the_simulation(invoke, api_call): + delete_simulation = api_call("delete_simulation") + + result = invoke("remote", "admin", "delete", "abcd1234") + + assert result.exit_code == 0 + assert "Deleted simulation abcd1234" in result.output + assert delete_simulation.call_args.args == ("abcd1234",) diff --git a/tests/cli/test_cli_remote_config_command.py b/tests/cli/test_cli_remote_config_command.py new file mode 100644 index 00000000..02751570 --- /dev/null +++ b/tests/cli/test_cli_remote_config_command.py @@ -0,0 +1,152 @@ +"""Tests for ``simdb remote config``. + +These commands only touch the configuration file, so no remote is involved. The +file they write back to is the throw-away one the ``config_file`` fixture +creates inside ``tmp_path``. +""" + +import configparser + +import pytest + + +@pytest.fixture +def saved_config(config_file): + """Read back what ``Config.save`` wrote. + + ``Config.load`` treats a ``--config-file`` as the user configuration, so + that is the file ``save`` writes back to. + """ + + def _saved_config() -> configparser.ConfigParser: + parser = configparser.ConfigParser() + parser.read(config_file) + return parser + + return _saved_config + + +def test_list_shows_the_configured_remotes(invoke): + result = invoke("remote", "config", "list") + + assert result.exit_code == 0 + assert "test: http://0.0.0.0:5000/ (default)" in result.output + + +def test_list_shows_the_username_and_firewall_of_a_remote(invoke): + assert ( + invoke("remote", "config", "set-option", "test", "username", "me").exit_code + == 0 + ) + assert ( + invoke("remote", "config", "set-option", "test", "firewall", "F5").exit_code + == 0 + ) + + result = invoke("remote", "config", "list") + + assert result.exit_code == 0 + assert "firewall: F5" in result.output + assert "username: me" in result.output + + +def test_default_prints_the_default_remote(invoke): + result = invoke("remote", "config", "default") + + assert result.exit_code == 0 + assert result.output.strip() == "test" + + +def test_get_default_prints_the_default_remote(invoke): + result = invoke("remote", "config", "get-default") + + assert result.exit_code == 0 + assert result.output.strip() == "test" + + +def test_new_adds_a_remote(invoke, saved_config): + result = invoke("remote", "config", "new", "other", "http://other.test") + + assert result.exit_code == 0 + assert saved_config()['remote "other"']["url"] == "http://other.test" + + +def test_new_can_record_a_username_and_firewall(invoke, saved_config): + result = invoke( + "remote", + "config", + "new", + "other", + "http://other.test", + "--username=me", + "--firewall=F5", + ) + + assert result.exit_code == 0 + section = saved_config()['remote "other"'] + assert section["username"] == "me" + assert section["firewall"] == "F5" + + +def test_new_rejects_an_unknown_firewall(invoke): + result = invoke( + "remote", "config", "new", "other", "http://other.test", "--firewall=nope" + ) + + assert result.exit_code == 2 + + +def test_new_can_make_the_remote_the_default(invoke, saved_config): + result = invoke( + "remote", "config", "new", "other", "http://other.test", "--default" + ) + + assert result.exit_code == 0 + config = saved_config() + assert config.getboolean('remote "other"', "default") is True + assert config.getboolean('remote "test"', "default") is False + + +def test_set_default_switches_the_default_remote(invoke, saved_config): + assert ( + invoke("remote", "config", "new", "other", "http://other.test").exit_code == 0 + ) + + result = invoke("remote", "config", "set-default", "other") + + assert result.exit_code == 0 + assert saved_config().getboolean('remote "other"', "default") is True + + +def test_delete_removes_a_remote(invoke, saved_config): + assert ( + invoke("remote", "config", "new", "other", "http://other.test").exit_code == 0 + ) + + result = invoke("remote", "config", "delete", "other") + + assert result.exit_code == 0 + assert 'remote "other"' not in saved_config().sections() + + +def test_set_option_stores_an_arbitrary_option(invoke, saved_config): + result = invoke("remote", "config", "set-option", "test", "token", "NEWTOKEN") + + assert result.exit_code == 0 + assert saved_config()['remote "test"']["token"] == "NEWTOKEN" + + +@pytest.mark.parametrize( + ("subcommand", "arguments"), + [ + ("new", ["only-a-name"]), + ("delete", []), + ("set-default", []), + ("set-option", ["test", "token"]), + ], +) +def test_missing_arguments_are_reported(invoke, subcommand, arguments): + result = invoke("remote", "config", subcommand, *arguments) + + assert result.exit_code == 2 + assert "Missing argument" in result.output diff --git a/tests/cli/test_cli_root.py b/tests/cli/test_cli_root.py new file mode 100644 index 00000000..99000ef5 --- /dev/null +++ b/tests/cli/test_cli_root.py @@ -0,0 +1,133 @@ +"""Tests for the top level ``simdb`` group in :mod:`simdb.cli.simdb`.""" + +from unittest import mock + +import pytest +from cli_helpers import make_simulation + +from simdb import __version__ +from simdb.cli import simdb as simdb_cli +from simdb.cli.simdb import cli + + +def test_version_option_reports_the_package_version(runner): + result = runner.invoke(cli, ["--version"]) + + assert result.exit_code == 0 + assert __version__ in result.output + + +def test_help_lists_every_command(runner): + result = runner.invoke(cli, ["--help"]) + + assert result.exit_code == 0 + commands = ("alias", "config", "manifest", "provenance", "remote", "simulation") + for command in commands: + assert command in result.output + + +def test_commands_are_listed_in_alphabetical_order(): + listed = cli.list_commands(None) + + assert listed == sorted(listed) + assert "sim" in listed + + +def test_sim_is_an_alias_for_simulation(runner): + result = runner.invoke(cli, ["--help"]) + + assert "Alias for simulation." in result.output + assert cli.get_command(None, "sim") is not None + + +def test_the_alias_reaches_the_same_command(invoke, local_db): + local_db.list_simulations.return_value = [] + + assert invoke("sim", "list").exit_code == 0 + assert local_db.list_simulations.called + + +def test_unknown_commands_are_rejected(runner): + result = runner.invoke(cli, ["nonsense"]) + + assert result.exit_code == 2 + + +def test_hidden_dump_help_prints_help_for_every_subcommand(runner): + result = runner.invoke(cli, ["dump-help"]) + + assert result.exit_code == 0 + # Both a group and one of its sub-commands are documented. + assert "Manage ingested simulations." in result.output + assert "List ingested simulations." in result.output + assert "Query/update application configuration." in result.output + + +def test_dump_help_is_hidden_from_the_command_list(runner): + assert "dump-help" not in runner.invoke(cli, ["--help"]).output + + +def test_config_file_option_is_loaded(invoke): + """The remote declared in the config file is visible to the commands.""" + result = invoke("remote", "config", "list") + + assert result.exit_code == 0 + assert "test: http://0.0.0.0:5000/" in result.output + assert "(default)" in result.output + + +def test_a_missing_config_file_is_reported(runner, tmp_path): + result = runner.invoke( + cli, [f"--config-file={tmp_path / 'nope.cfg'}", "config", "path"] + ) + + assert result.exit_code == 2 + assert "No such file or directory" in result.output + + +def test_verbose_flag_reaches_the_commands(invoke, local_db): + """``--verbose`` is what turns on the extra columns of ``simulation list``.""" + local_db.list_simulations.return_value = [make_simulation("sim")] + + assert "status" not in invoke("simulation", "list").output + assert "status" in invoke("--verbose", "simulation", "list").output + + +class TestMain: + """``main`` is the console-script entry point and the CLI's last error handler.""" + + def test_successful_runs_do_not_raise(self): + with mock.patch.object(simdb_cli, "cli") as command: + simdb_cli.main() + + assert command.called + + def test_errors_are_reported_and_exit_non_zero(self, capsys): + with mock.patch.object( + simdb_cli, "cli", side_effect=RuntimeError("boom") + ), pytest.raises(SystemExit) as exit_info: + simdb_cli.main() + + assert exit_info.value.code == 1 + assert "Error: boom" in capsys.readouterr().err + + def test_debug_mode_re_raises_for_a_traceback(self, monkeypatch): + monkeypatch.setattr(simdb_cli, "g_debug", True) + + with mock.patch.object( + simdb_cli, "cli", side_effect=RuntimeError("boom") + ), pytest.raises(RuntimeError, match="boom"): + simdb_cli.main() + + def test_the_debug_flag_switches_debug_mode_on(self, invoke): + """``-d`` is what makes :func:`main` re-raise instead of printing. + + ``--help`` is eager and exits before the group callback runs, so the + flag only takes effect when a command is actually invoked. + """ + invoke("-d", "config", "path") + assert simdb_cli.g_debug is True + + # Leave the module global as the other tests expect to find it. + invoke("config", "path") + assert simdb_cli.g_debug is False diff --git a/tests/cli/test_cli_simulation_command.py b/tests/cli/test_cli_simulation_command.py index 0120fc02..7541cca1 100644 --- a/tests/cli/test_cli_simulation_command.py +++ b/tests/cli/test_cli_simulation_command.py @@ -1,87 +1,496 @@ +"""Tests for ``simdb simulation``. + +The local database and the remote are stubbed; what is exercised here is the +command layer itself: argument parsing, the branches each command takes, the +calls it makes, and what it reports back to the user. +""" + from unittest import mock -from click.testing import CliRunner -from utils import config_test_file - -from simdb.cli.simdb import cli - - -@mock.patch("simdb.database.get_local_db") -@mock.patch("simdb.cli.remote_api.RemoteAPI") -def test_simulation_alias_command(remote_api, get_local_db): - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke(cli, [f"--config-file={config_file}", "simulation"]) - assert result.exception is None - - -@mock.patch("simdb.database.get_local_db") -@mock.patch("simdb.cli.remote_api.RemoteAPI") -def test_simulation_delete_command(remote_api, get_local_db): - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke(cli, [f"--config-file={config_file}", "simulation"]) - assert result.exception is None - - -@mock.patch("simdb.database.get_local_db") -@mock.patch("simdb.cli.remote_api.RemoteAPI") -def test_simulation_info_command(remote_api, get_local_db): - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke(cli, [f"--config-file={config_file}", "simulation"]) - assert result.exception is None - - -@mock.patch("simdb.database.get_local_db") -@mock.patch("simdb.cli.remote_api.RemoteAPI") -def test_simulation_list_command(remote_api, get_local_db): - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke(cli, [f"--config-file={config_file}", "simulation"]) - assert result.exception is None - - -@mock.patch("simdb.database.get_local_db") -@mock.patch("simdb.cli.remote_api.RemoteAPI") -def test_simulation_modify_command(remote_api, get_local_db): - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke(cli, [f"--config-file={config_file}", "simulation"]) - assert result.exception is None - - -@mock.patch("simdb.database.get_local_db") -@mock.patch("simdb.cli.remote_api.RemoteAPI") -def test_simulation_new_command(remote_api, get_local_db): - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke(cli, [f"--config-file={config_file}", "simulation"]) - assert result.exception is None - - -@mock.patch("simdb.database.get_local_db") -@mock.patch("simdb.cli.remote_api.RemoteAPI") -def test_simulation_push_command(remote_api, get_local_db): - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke(cli, [f"--config-file={config_file}", "simulation"]) - assert result.exception is None - - -@mock.patch("simdb.database.get_local_db") -@mock.patch("simdb.cli.remote_api.RemoteAPI") -def test_simulation_query_command(remote_api, get_local_db): - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke(cli, [f"--config-file={config_file}", "simulation"]) - assert result.exception is None - - -@mock.patch("simdb.database.get_local_db") -@mock.patch("simdb.cli.remote_api.RemoteAPI") -def test_simulation_validate_command(remote_api, get_local_db): - config_file = config_test_file() - runner = CliRunner() - result = runner.invoke(cli, [f"--config-file={config_file}", "simulation"]) - assert result.exception is None +import pytest +from cli_helpers import make_simulation + +from simdb.cli.remote_api import RemoteError +from simdb.database import DatabaseError +from simdb.remote.models import ImasDataResponse, QuantityData +from simdb.validation import ValidationError + + +@pytest.fixture +def remote_api(): + """Replace the ``RemoteAPI`` the simulation commands construct.""" + with mock.patch("simdb.cli.commands.simulation.RemoteAPI") as remote_api_cls: + yield remote_api_cls.return_value + + +# --------------------------------------------------------------------------- +# simulation list +# --------------------------------------------------------------------------- + + +def test_list_prints_every_alias(invoke, local_db): + local_db.list_simulations.return_value = [ + make_simulation("first"), + make_simulation("second"), + ] + + result = invoke("simulation", "list") + + assert result.exit_code == 0 + assert "first" in result.output + assert "second" in result.output + assert local_db.list_simulations.call_args.kwargs == { + "meta_keys": (), + "limit": 100, + } + + +def test_list_reports_an_empty_database(invoke, local_db): + local_db.list_simulations.return_value = [] + + result = invoke("simulation", "list") + + assert result.exit_code == 0 + assert "No simulations found" in result.output + + +def test_list_shows_uuid_and_metadata_columns_on_request(invoke, local_db): + local_db.list_simulations.return_value = [ + make_simulation("first", uuid="abcd1234", meta={"pulse": 134173}) + ] + + result = invoke("simulation", "list", "--uuid", "--meta-data=pulse") + + assert result.exit_code == 0 + assert "abcd1234" in result.output + assert "134173" in result.output + assert local_db.list_simulations.call_args.kwargs["meta_keys"] == ("pulse",) + + +def test_list_passes_the_requested_limit(invoke, local_db): + local_db.list_simulations.return_value = [] + + assert invoke("simulation", "list", "--limit=5").exit_code == 0 + + assert local_db.list_simulations.call_args.kwargs["limit"] == 5 + + +def test_list_rejects_a_negative_limit(invoke, local_db): + result = invoke("simulation", "list", "--limit=-1") + + assert result.exit_code == 2 + assert "must be non-negative" in result.output + assert not local_db.list_simulations.called + + +# --------------------------------------------------------------------------- +# simulation modify +# --------------------------------------------------------------------------- + + +def test_modify_sets_a_new_alias(invoke, local_db): + simulation = make_simulation("old") + local_db.get_simulation.return_value = simulation + + result = invoke("simulation", "modify", "old", "--alias=new") + + assert result.exit_code == 0 + assert "alias updated" in result.output + assert simulation.alias == "new" + assert local_db.session.commit.called + + +def test_modify_sets_metadata(invoke, local_db): + simulation = make_simulation("sim") + local_db.get_simulation.return_value = simulation + + result = invoke("simulation", "modify", "sim", "--set-meta=pulse=134173") + + assert result.exit_code == 0 + assert "metadata updated" in result.output + simulation.set_meta.assert_called_once_with("pulse", "134173") + assert local_db.session.commit.called + + +def test_modify_rejects_metadata_without_a_value(invoke, local_db): + result = invoke("simulation", "modify", "sim", "--set-meta=pulse") + + assert result.exit_code == 2 + assert "must be of form NAME=VALUE" in result.output + assert not local_db.session.commit.called + + +def test_modify_deletes_metadata(invoke, local_db): + simulation = make_simulation("sim") + local_db.get_simulation.return_value = simulation + + result = invoke("simulation", "modify", "sim", "--del-meta=pulse") + + assert result.exit_code == 0 + assert "metadata deleted" in result.output + simulation.remove_meta.assert_called_once_with("pulse") + + +def test_modify_without_options_changes_nothing(invoke, local_db): + result = invoke("simulation", "modify", "sim") + + assert result.exit_code == 0 + assert "nothing to do" in result.output + assert not local_db.get_simulation.called + + +# --------------------------------------------------------------------------- +# simulation delete +# --------------------------------------------------------------------------- + + +def test_delete_removes_a_single_simulation(invoke, local_db): + simulation = make_simulation("sim") + simulation.uuid = mock.Mock(hex="abcd1234") + local_db.delete_simulation.return_value = simulation + + result = invoke("simulation", "delete", "sim") + + assert result.exit_code == 0 + assert "abcd1234 deleted" in result.output + local_db.delete_simulation.assert_called_once_with("sim") + + +def test_delete_requires_a_simulation_or_all(invoke, local_db): + result = invoke("simulation", "delete") + + assert result.exit_code != 0 + assert "Either SIM_ID or --all must be provided" in result.output + + +def test_delete_all_removes_the_database_file_once_confirmed( + invoke, local_db, tmp_path +): + database_file = tmp_path / "sim.db" + database_file.write_text("not really a database") + + with mock.patch( + "simdb.cli.commands.simulation.Confirm.ask", return_value=True + ) as ask: + result = invoke("simulation", "delete", "--all") + + assert result.exit_code == 0 + assert "Local database reset." in result.output + assert ask.called + assert not database_file.exists() + + +def test_delete_all_keeps_the_database_when_not_confirmed(invoke, local_db, tmp_path): + database_file = tmp_path / "sim.db" + database_file.write_text("not really a database") + + with mock.patch("simdb.cli.commands.simulation.Confirm.ask", return_value=False): + result = invoke("simulation", "delete", "--all") + + # Declining the reset falls through to the single-simulation path, which has + # no SIM_ID to work with. + assert result.exit_code != 0 + assert "Either SIM_ID or --all must be provided" in result.output + assert database_file.exists() + + +# --------------------------------------------------------------------------- +# simulation info +# --------------------------------------------------------------------------- + + +def test_info_prints_the_simulation(invoke, local_db): + local_db.get_simulation.return_value = "simulation description" + + result = invoke("simulation", "info", "sim") + + assert result.exit_code == 0 + assert "simulation description" in result.output + local_db.get_simulation.assert_called_once_with("sim") + + +def test_info_fails_when_the_simulation_is_unknown(invoke, local_db): + local_db.get_simulation.return_value = None + + result = invoke("simulation", "info", "sim") + + assert result.exit_code != 0 + assert isinstance(result.exception, KeyError) + + +# --------------------------------------------------------------------------- +# simulation ingest +# --------------------------------------------------------------------------- + + +def test_ingest_stores_the_manifest_and_reports_the_alias( + invoke, local_db, manifest_file +): + result = invoke("simulation", "ingest", str(manifest_file)) + + assert result.exit_code == 0 + assert "ALIAS: simulation-alias" in result.output + assert local_db.insert_simulation.called + (simulation,) = local_db.insert_simulation.call_args.args + assert simulation.alias == "simulation-alias" + assert len(simulation.inputs) == 1 + assert len(simulation.outputs) == 1 + + +def test_ingest_alias_option_overrides_the_manifest(invoke, local_db, manifest_file): + result = invoke("simulation", "ingest", str(manifest_file), "--alias=override") + + assert result.exit_code == 0 + assert "ALIAS: override" in result.output + (simulation,) = local_db.insert_simulation.call_args.args + assert simulation.alias == "override" + + +def test_ingest_rejects_an_alias_with_reserved_characters( + invoke, local_db, manifest_file +): + """The manifest refuses the alias before the simulation is built. + + ``simulation_ingest`` also has a "warning: alias contains reserved + characters" branch, but ``Manifest.validate_alias`` applies the identical + ``urllib.parse.quote`` check to both the manifest alias and the ``--alias`` + override, so that branch cannot be reached. + """ + result = invoke("simulation", "ingest", str(manifest_file), "--alias=with space") + + assert result.exit_code != 0 + assert "illegal characters in alias" in str(result.exception) + assert not local_db.insert_simulation.called + + +def test_ingest_requires_an_existing_manifest(invoke, local_db, tmp_path): + result = invoke("simulation", "ingest", str(tmp_path / "missing.yaml")) + + assert result.exit_code == 2 + assert not local_db.insert_simulation.called + + +# --------------------------------------------------------------------------- +# simulation query +# --------------------------------------------------------------------------- + + +def test_query_parses_constraints_and_prints_matches(invoke, local_db): + local_db.query_meta.return_value = [make_simulation("match")] + + result = invoke("simulation", "query", "pulse=gt:1000", "run=0") + + assert result.exit_code == 0 + assert "match" in result.output + (constraints,) = local_db.query_meta.call_args.args + assert [(name, value) for name, value, _ in constraints] == [ + ("pulse", "1000"), + ("run", "0"), + ] + + +def test_query_requires_a_constraint(invoke, local_db): + result = invoke("simulation", "query") + + assert result.exit_code != 0 + assert "At least one constraint must be provided" in result.output + assert not local_db.query_meta.called + + +def test_query_rejects_a_constraint_without_a_value(invoke, local_db): + result = invoke("simulation", "query", "pulse") + + assert result.exit_code != 0 + assert "Invalid constraint pulse" in result.output + assert not local_db.query_meta.called + + +# --------------------------------------------------------------------------- +# simulation push +# --------------------------------------------------------------------------- + + +def test_push_validates_and_uploads_the_simulation(invoke, local_db, remote_api): + simulation = make_simulation("sim") + local_db.get_simulation.return_value = simulation + remote_api.get_validation_schemas.return_value = [] + + result = invoke("simulation", "push", "sim") + + assert result.exit_code == 0 + assert "Successfully pushed simulation" in result.output + assert remote_api.push_simulation.call_args.args == (simulation,) + assert remote_api.push_simulation.call_args.kwargs["add_watcher"] is False + + +def test_push_records_the_replaced_simulation(invoke, local_db, remote_api): + simulation = make_simulation("sim") + local_db.get_simulation.return_value = simulation + remote_api.get_validation_schemas.return_value = [] + + result = invoke("simulation", "push", "sim", "--replaces=older") + + assert result.exit_code == 0 + simulation.set_meta.assert_called_once_with("replaces", "older") + + +def test_push_adds_a_watcher_on_request(invoke, local_db, remote_api): + local_db.get_simulation.return_value = make_simulation("sim") + remote_api.get_validation_schemas.return_value = [] + + result = invoke("simulation", "push", "sim", "--add-watcher") + + assert result.exit_code == 0 + assert remote_api.push_simulation.call_args.kwargs["add_watcher"] is True + + +def test_push_fails_when_the_simulation_is_unknown(invoke, local_db, remote_api): + local_db.get_simulation.return_value = None + + result = invoke("simulation", "push", "sim") + + assert result.exit_code != 0 + assert "Failed to find simulation: sim" in result.output + assert not remote_api.push_simulation.called + + +def test_push_reports_validation_failures_without_uploading( + invoke, local_db, remote_api +): + local_db.get_simulation.return_value = make_simulation("sim") + remote_api.get_validation_schemas.return_value = [{"alias": {"type": "string"}}] + + with mock.patch("simdb.cli.commands.simulation.Validator") as validator: + validator.return_value.validate.side_effect = ValidationError("bad metadata") + result = invoke("simulation", "push", "sim") + + assert result.exit_code != 0 + assert "Simulation does not validate: bad metadata" in result.output + assert not remote_api.push_simulation.called + + +# --------------------------------------------------------------------------- +# simulation pull +# --------------------------------------------------------------------------- + + +def test_pull_stores_the_simulation_locally(invoke, local_db, remote_api, tmp_path): + local_db.get_simulation.side_effect = DatabaseError("not found") + pulled = make_simulation("pulled") + remote_api.pull_simulation.return_value = pulled + + result = invoke("simulation", "pull", "sim", str(tmp_path / "out")) + + assert result.exit_code == 0 + assert "Successfully pulled simulation" in result.output + local_db.insert_simulation.assert_called_once_with(pulled) + + +def test_pull_refuses_to_overwrite_an_existing_simulation( + invoke, local_db, remote_api, tmp_path +): + local_db.get_simulation.return_value = make_simulation("sim") + + result = invoke("simulation", "pull", "sim", str(tmp_path / "out")) + + assert result.exit_code != 0 + assert "already exists" in result.output + assert not remote_api.pull_simulation.called + assert not local_db.insert_simulation.called + + +def test_pull_reports_remote_errors(invoke, local_db, remote_api, tmp_path): + local_db.get_simulation.side_effect = DatabaseError("not found") + remote_api.pull_simulation.side_effect = RemoteError("remote is down") + + result = invoke("simulation", "pull", "sim", str(tmp_path / "out")) + + assert result.exit_code != 0 + assert "remote is down" in result.output + assert not local_db.insert_simulation.called + + +# --------------------------------------------------------------------------- +# simulation data +# --------------------------------------------------------------------------- + + +def _data_response(field_data, coordinates=()): + return ImasDataResponse( + simulation="abcd1234", + path="core_profiles/profiles_1d[0]/grid/rho_tor_norm", + occurrence=0, + field=QuantityData(name="grid/rho_tor_norm", units="-", data=field_data), + coordinates=list(coordinates), + ) + + +def test_data_plots_a_one_dimensional_field(invoke, remote_api): + remote_api.get_simulation_data.return_value = _data_response([0.0, 0.5, 1.0]) + + result = invoke("simulation", "data", "sim", "core_profiles/grid/rho_tor_norm") + + assert result.exit_code == 0 + assert "abcd1234" in result.output + assert "occurrence 0" in result.output + assert remote_api.get_simulation_data.call_args.args == ( + "sim", + "core_profiles/grid/rho_tor_norm", + ) + assert remote_api.get_simulation_data.call_args.kwargs == {"dd_version": None} + + +def test_data_forwards_the_requested_dd_version(invoke, remote_api): + remote_api.get_simulation_data.return_value = _data_response(1.5) + + result = invoke( + "simulation", "data", "sim", "core_profiles/time", "--dd-version=4.1.1" + ) + + assert result.exit_code == 0 + assert remote_api.get_simulation_data.call_args.kwargs == {"dd_version": "4.1.1"} + + +def test_data_reports_remote_failures_as_a_clean_error(invoke, remote_api): + remote_api.get_simulation_data.side_effect = RemoteError("no such field") + + result = invoke("simulation", "data", "sim", "core_profiles/nope") + + assert result.exit_code != 0 + assert "no such field" in result.output + + +# --------------------------------------------------------------------------- +# simulation validate +# --------------------------------------------------------------------------- + + +def test_validate_checks_metadata_and_file_checksums(invoke, local_db, remote_api): + file = mock.Mock(uri="file:///data", checksum="abc") + file.generate_checksum.return_value = "abc" + simulation = make_simulation("sim") + simulation.inputs = [file] + simulation.outputs = [] + local_db.get_simulation.return_value = simulation + remote_api.get_validation_schemas.return_value = [] + + result = invoke("simulation", "validate", "sim") + + assert result.exit_code == 0 + assert "validation successful" in result.output + + +def test_validate_fails_on_a_checksum_mismatch(invoke, local_db, remote_api): + file = mock.Mock(uri="file:///data", checksum="abc") + file.generate_checksum.return_value = "different" + simulation = make_simulation("sim") + simulation.inputs = [file] + simulation.outputs = [] + local_db.get_simulation.return_value = simulation + remote_api.get_validation_schemas.return_value = [] + + result = invoke("simulation", "validate", "sim") + + assert result.exit_code != 0 + assert isinstance(result.exception, ValidationError) + assert "file:///data" in str(result.exception) diff --git a/tests/cli/test_cli_validators.py b/tests/cli/test_cli_validators.py new file mode 100644 index 00000000..614516e4 --- /dev/null +++ b/tests/cli/test_cli_validators.py @@ -0,0 +1,28 @@ +"""Tests for the shared click parameter validators.""" + +import click +import pytest + +from simdb.cli.commands.validators import validate_non_negative, validate_positive + + +@pytest.mark.parametrize("value", [0, 1, 100]) +def test_non_negative_accepts_zero_and_above(value): + assert validate_non_negative(None, None, value) == value + + +@pytest.mark.parametrize("value", [-1, -100]) +def test_non_negative_rejects_negative_values(value): + with pytest.raises(click.BadParameter, match="must be non-negative"): + validate_non_negative(None, None, value) + + +@pytest.mark.parametrize("value", [1, 100]) +def test_positive_accepts_values_above_zero(value): + assert validate_positive(None, None, value) == value + + +@pytest.mark.parametrize("value", [0, -1]) +def test_positive_rejects_zero_and_below(value): + with pytest.raises(click.BadParameter, match="must be greater than zero"): + validate_positive(None, None, value) diff --git a/tests/cli/test_remote_api_helpers.py b/tests/cli/test_remote_api_helpers.py new file mode 100644 index 00000000..73c76cae --- /dev/null +++ b/tests/cli/test_remote_api_helpers.py @@ -0,0 +1,42 @@ +"""Tests for the IDS metadata helper used when pushing IMAS data.""" + +import pytest + +from simdb.cli.remote_api import _meta_list + + +@pytest.mark.parametrize( + ("value", "expected"), + [ + # The list form written since #119. + (["core_profiles", "equilibrium"], ["core_profiles", "equilibrium"]), + (("core_profiles", "equilibrium"), ["core_profiles", "equilibrium"]), + # The display string written by SimDB <= 1.2, which reaches the client + # from a remote that has not been migrated. + ("[core_profiles, equilibrium]", ["core_profiles", "equilibrium"]), + ("core_profiles", ["core_profiles"]), + # Nothing to filter on. + (None, []), + ("", []), + ("[]", []), + ], +) +def test_meta_list_normalises_both_stored_forms(value, expected): + assert _meta_list(value) == expected + + +@pytest.mark.parametrize( + "value", [["core_profiles", "equilibrium"], "[core_profiles, equilibrium]"] +) +def test_ids_names_match_however_the_value_was_stored(value): + """push_simulation skips any IDS whose name is not in this list. + + Wrapping the display string instead of splitting it yields + ``["[core_profiles, equilibrium]"]``, which matches no IDS name at all and so + silently skips every file of the simulation. + """ + ids_list = _meta_list(value) + + assert "core_profiles" in ids_list + assert "equilibrium" in ids_list + assert "core_sources" not in ids_list diff --git a/tests/cli/test_remote_api_version.py b/tests/cli/test_remote_api_version.py index 08b0b44b..1ed2b887 100644 --- a/tests/cli/test_remote_api_version.py +++ b/tests/cli/test_remote_api_version.py @@ -1,4 +1,14 @@ -from simdb.cli.remote_api import select_api_version +import io +import json +from unittest import mock + +import pytest + +from simdb.cli.manifest import Manifest +from simdb.cli.remote_api import RemoteAPI, RemoteError, select_api_version +from simdb.config import Config +from simdb.database.models import Simulation +from simdb.remote.models import UploadOptions def test_selects_highest_common_version(): @@ -16,3 +26,142 @@ def test_no_common_version_returns_none(): def test_versions_compare_semantically_not_lexicographically(): assert select_api_version(["v1.2", "v1.10"], ("v1.2", "v1.10")) == "v1.10" + + +def _remote_api(endpoints=("v1.2", "v1.3")): + config = Config() + config.set_option("remote.test.url", "http://remote.test") + config.set_option("remote.test.token", "123ABC") + + with mock.patch.object( + RemoteAPI, "get_server_authentication", return_value="None" + ), mock.patch.object( + RemoteAPI, "get_endpoints", return_value=list(endpoints) + ), mock.patch.object(RemoteAPI, "get_server_version", return_value="0.11"): + return RemoteAPI("test", None, None, config) + + +def test_requests_use_the_negotiated_version(): + api = _remote_api() + + assert api._api_url == "http://remote.test/v1.3/" + + +def test_api_version_switches_the_requested_version(): + api = _remote_api() + + with api.api_version("v1.2"): + assert api._api_url == "http://remote.test/v1.2/" + + assert api._api_url == "http://remote.test/v1.3/" + + +def test_api_version_not_provided_by_remote_raises(): + api = _remote_api(endpoints=("v1.3",)) + + with pytest.raises(RemoteError, match="not provided by remote"), api.api_version( + "v1.2" + ): + pass + + +def test_api_version_unknown_to_client_raises(): + api = _remote_api() + + with pytest.raises( + RemoteError, match="not supported by this client" + ), api.api_version("v1.1"): + pass + + +def test_push_simulation_uses_v1_2_when_v1_3_is_negotiated(): + api = _remote_api() + simulation = Simulation(Manifest()) + used_urls = [] + + with mock.patch.object( + RemoteAPI, "get_upload_options", return_value=UploadOptions(copy_files=False) + ), mock.patch.object( + RemoteAPI, "post", side_effect=lambda *a, **kw: used_urls.append(api._api_url) + ): + api.push_simulation(simulation, out_stream=io.StringIO()) + + assert used_urls == ["http://remote.test/v1.2/"] + assert api._api_url == "http://remote.test/v1.3/" + + +def test_push_simulation_requires_v1_2_on_the_remote(): + api = _remote_api(endpoints=("v1.3",)) + + with pytest.raises(RemoteError, match=r"requires one of the API versions v1\.2"): + api.push_simulation(Simulation(Manifest()), out_stream=io.StringIO()) + + +def _remote_api_serving(root_index, versioned_index): + """ + Build a RemoteAPI against a remote whose root and versioned API indices return + the given payloads, leaving the index reading itself unmocked. + """ + config = Config() + config.set_option("remote.test.url", "http://remote.test") + config.set_option("remote.test.token", "123ABC") + + def fake_get( + self, + url, + params=None, + headers=None, + authenticate=True, + stream=False, + base_url=None, + ): + target = (base_url if base_url is not None else self._api_url) + url + body = root_index if target == "http://remote.test/" else versioned_index + return mock.Mock(content=json.dumps(body).encode()) + + with mock.patch.object(RemoteAPI, "get", fake_get): + return RemoteAPI("test", None, None, config) + + +_ENDPOINTS = ["http://remote.test/v1.2", "http://remote.test/v1.3"] + + +def test_api_version_comes_from_the_negotiated_version(): + api = _remote_api_serving( + {"endpoints": _ENDPOINTS, "authentication": "None", "server_version": "1.4.0"}, + {"api": "simdb", "api_version": "9.9", "endpoints": []}, + ) + + # The negotiated version decides, not whatever the index happens to report. + assert str(api.version) == "1.3.0" + + +def test_server_version_is_read_from_the_root_index(): + api = _remote_api_serving( + {"endpoints": _ENDPOINTS, "authentication": "None", "server_version": "1.4.0"}, + {"api": "simdb", "api_version": "1.3", "endpoints": []}, + ) + + assert str(api.server_version) == "1.4.0" + + +def test_server_version_falls_back_to_the_versioned_index(): + api = _remote_api_serving( + {"endpoints": _ENDPOINTS, "authentication": "None"}, + { + "api": "simdb", + "api_version": "1.3", + "server_version": "1.2.0", + "endpoints": [], + }, + ) + + assert str(api.server_version) == "1.2.0" + + +def test_server_version_missing_everywhere_raises(): + with pytest.raises(RemoteError, match="did not report a server version"): + _remote_api_serving( + {"endpoints": _ENDPOINTS, "authentication": "None"}, + {"api": "simdb", "api_version": "1.3", "endpoints": []}, + ) diff --git a/tests/cli/utils.py b/tests/cli/utils.py deleted file mode 100644 index 7c99c36f..00000000 --- a/tests/cli/utils.py +++ /dev/null @@ -1,51 +0,0 @@ -from pathlib import Path - - -def config_test_file() -> Path: - config = """\ -[remote "test"] -url = http://0.0.0.0:5000/ -default = True -token = 123ABC -""" - config_file = Path(__file__).parent / "test.cfg" - config_file.write_text(config) - return config_file - - -def create_manifest() -> Path: - manifest = """\ -manifest_version: 2 -alias: simulation-alias - -# Data and configuration files -inputs: -# - uri: simdb://simdb.iter.org/123e4567-e89b-12d3-a456-426655440000 - - uri: file:///$MANIFEST_DIR/utils.py - - uri: imas:///user?shot=10000&run=0&database=west -# - uri: imas+uda:///TOKAMAK?shot=10000&run=0&server=uda.server.org:56565 - -# Data and log files. -outputs: - - uri: file:///$MANIFEST_DIR/utils.py - - uri: imas:///user?shot=10000&run=1&database=west - -metadata: -- values: - workflow: - name: Workflow Name - git: ssh://git@git.iter.org/wf/workflow.git - branch: master - commit: 079e84d5ae8a0eec6dcf3819c98f3c05f48e952f - codes: - - Code 1: - git: ssh://git@git.iter.org/eq/code.git - commit: 079e84d5ae8a0eec6dcf3819c98f3c05f48e952f -""" - manifest_file = Path(__file__).parent / "manifest.yaml" - manifest_file.write_text(manifest) - return manifest_file - - -def get_file_path(file_name) -> Path: - return Path(__file__).parent / file_name diff --git a/tests/remote/api/test_files.py b/tests/remote/api/test_files.py index e4e6e328..29d6c20d 100644 --- a/tests/remote/api/test_files.py +++ b/tests/remote/api/test_files.py @@ -3,6 +3,7 @@ import io import json import tarfile +import uuid from datetime import datetime, timezone from pathlib import Path @@ -189,3 +190,17 @@ def test_download_file(client): assert rv.status_code == 200 assert rv.data == file_content + + +def test_file_registration_item_without_chunks(): + """The IMAS registration payload omits ``chunks``. + + A single item covers all the files pushed under one file UUID, so the + client has no meaningful chunk count to report and leaves it unset. + """ + item = FileRegistrationItem( + file_type="input", + file_uuid=uuid.uuid4(), + ids_list=["core_profiles"], + ) + assert item.chunks is None diff --git a/tests/remote/api/test_metadata.py b/tests/remote/api/test_metadata.py index dbd99cee..82ee1b32 100644 --- a/tests/remote/api/test_metadata.py +++ b/tests/remote/api/test_metadata.py @@ -1,10 +1,16 @@ +import pytest from conftest import ( HEADERS, generate_simulation_data, post_simulation, ) -from simdb.remote.models import MetadataKeyInfoList, MetadataValueList, RangeValue +from simdb.remote.models import ( + MetadataData, + MetadataKeyInfoList, + MetadataValueList, + RangeValue, +) def test_get_metadata_keys(client): @@ -85,3 +91,16 @@ def test_get_metadata_values_nonexistent_key(client): assert rv.status_code == 200 # Should return an empty list or list without the key assert isinstance(rv.json, list) + + +@pytest.mark.parametrize("element", ["ids", "input_ids"]) +def test_ids_list_display_string_is_coerced(element): + """Both IDS list keys accept the display-string form written by SimDB <= 1.2. + + These were stored as ``"[core_profiles, equilibrium]"`` rather than a list + (#119), and the coercion has to cover ``ids`` as well as ``input_ids``. + """ + meta = MetadataData.model_validate( + {"element": element, "value": "[core_profiles, equilibrium]"} + ) + assert meta.value == ["core_profiles", "equilibrium"] diff --git a/tests/remote/api/test_remote_api_client.py b/tests/remote/api/test_remote_api_client.py new file mode 100644 index 00000000..340c85b6 --- /dev/null +++ b/tests/remote/api/test_remote_api_client.py @@ -0,0 +1,345 @@ +"""Round-trip tests for the CLI RemoteAPI client against the real server. + +The client talks to the Flask test client instead of the network, so every +request body, query parameter and response is validated by the pydantic models +of both sides. +""" + +import base64 +import gzip +import io +import json +import os +import shutil +import tempfile +from functools import partial +from pathlib import Path +from unittest import mock +from urllib.parse import urlencode + +import pytest +from conftest import TEST_PASSWORD, has_flask + +from simdb.cli.manifest import Manifest +from simdb.cli.remote_api import FailedConnection, RemoteAPI, RemoteError +from simdb.config import Config +from simdb.database.models import Simulation +from simdb.notifications import Notification +from simdb.remote.app import create_app +from simdb.remote.models import ( + SimulationDeleteResponse, + SimulationTraceData, + UploadOptions, + WatcherData, +) + +REMOTE_URL = "http://remote.test" + +VALIDATION_SCHEMA = """\ +status: + type: string +""" + + +class _Response: + """The parts of a requests.Response the RemoteAPI uses.""" + + def __init__(self, response, url): + self.status_code = response.status_code + self.headers = response.headers + self.url = url + content = response.data + if (response.headers.get("Content-Encoding") or "").lower() == "gzip": + # requests transparently decodes this for the real client + content = gzip.decompress(content) + self.content = content + + def json(self, **kwargs): + return json.loads(self.content.decode(), **kwargs) + + def raise_for_status(self): + if self.status_code >= 400: + raise AssertionError(f"HTTP {self.status_code} for {self.url}") + + def iter_content(self, chunk_size=1): + for start in range(0, len(self.content), chunk_size): + yield self.content[start : start + chunk_size] + + +def _requests_shim(client): + """Return a stand-in for the requests module routed to *client*.""" + + def request( + method, + url, + params=None, + data=None, + headers=None, + auth=None, + files=None, + **_kwargs, + ): + path = url[len(REMOTE_URL) :] + query = None + if "?" in path: + path, _, query = path.partition("?") + if params: + extra = urlencode(params, doseq=True) + query = f"{query}&{extra}" if query else extra + + body = data + if files: + fields = {} + for key, (name, content, _content_type) in files: + fields.setdefault(key, []).append((io.BytesIO(content), name)) + body = {k: v[0] if len(v) == 1 else v for k, v in fields.items()} + + request_headers = dict(headers or {}) + if isinstance(auth, tuple): + credentials = base64.b64encode(f"{auth[0]}:{auth[1]}".encode()).decode() + request_headers["Authorization"] = f"Basic {credentials}" + + return _Response( + client.open( + path, + method=method, + query_string=query, + data=body, + headers=request_headers, + ), + url, + ) + + shim = mock.MagicMock() + for method in ("get", "post", "put", "patch", "delete"): + shim.configure_mock(**{method: partial(request, method.upper())}) + shim.ConnectionError = ConnectionError + shim.HTTPError = type("HTTPError", (Exception,), {}) + shim.JSONDecodeError = json.JSONDecodeError + return shim + + +@pytest.fixture(scope="function") +def remote(tmp_path): + """Yield a (RemoteAPI, admin RemoteAPI) pair talking to a live server.""" + if not has_flask: + pytest.skip("Flask not installed") + + schema_file = tmp_path / "validation.yaml" + schema_file.write_text(VALIDATION_SCHEMA) + + db_fd, db_file = tempfile.mkstemp() + upload_dir = tempfile.mkdtemp() + + config = Config() + config.load() + config.set_option("database.type", "sqlite") + config.set_option("database.file", db_file) + config.set_option("server.admin_password", TEST_PASSWORD) + config.set_option("server.upload_folder", upload_dir) + config.set_option("authentication.type", "None") + config.set_option("server.copy_files", True) + config.set_option("role.admin.users", "admin") + config.set_option("validation.path", str(schema_file)) + config.set_option("flask.secret_key", "test-secret") + app = create_app(config=config, testing=True, debug=True) + app.testing = True + + client_config = Config() + client_config.set_option(f"remote.{'test'}.url", REMOTE_URL) + + with app.test_client() as client, mock.patch( + "simdb.cli.remote_api.requests", _requests_shim(client) + ): + api = RemoteAPI("test", None, None, client_config, use_token=False) + admin = RemoteAPI( + "test", "admin", TEST_PASSWORD, client_config, use_token=False + ) + yield api, admin + + os.close(db_fd) + Path(db_file).unlink() + shutil.rmtree(upload_dir) + + +def _push_simulation(api, tmp_path, alias="client-sim"): + data_file = tmp_path / "input.txt" + data_file.write_text("hello simdb\n") + manifest_file = tmp_path / "manifest.yaml" + manifest_file.write_text( + "manifest_version: 2\n" + f"alias: {alias}\n" + "inputs:\n" + f" - uri: file://{data_file}\n" + "outputs:\n" + f" - uri: file://{data_file}\n" + "metadata:\n" + "- values:\n" + " code: my-code\n" + ) + simulation = Simulation(Manifest.load_from_file(manifest_file)) + api.push_simulation(simulation, out_stream=io.StringIO(), add_watcher=False) + return simulation + + +def test_index_endpoints(remote): + api, _ = remote + + assert str(api.version).startswith("1.3") + assert any(url.endswith("simulations") for url in api.get_endpoints()) + assert api.get_upload_options() == UploadOptions(copy_files=True, copy_ids=True) + assert isinstance(api.get_directory(), Path) + assert api.get_validation_schemas() == [{"status": {"type": "string"}}] + + +def test_token(remote): + _, admin = remote + + assert admin.get_token() + + +def test_push_and_get_simulation(remote, tmp_path): + api, _ = remote + + simulation = _push_simulation(api, tmp_path) + pushed = api.get_simulation(simulation.uuid.hex) + + assert pushed.uuid == simulation.uuid + assert pushed.alias == "client-sim" + assert pushed.meta_dict()["values"]["code"] == "my-code" + # the file has been staged on the server, so the URI now points there + assert len(pushed.inputs) == 1 + assert Path(pushed.inputs[0].uri.path).read_text() == "hello simdb\n" + + +def test_list_and_query_simulations(remote, tmp_path): + api, _ = remote + + simulation = _push_simulation(api, tmp_path) + + listed = api.list_simulations(limit=0) + assert [sim.uuid for sim in listed] == [simulation.uuid] + + queried = api.query_simulations(["values.code=my-code"], ["values.code"]) + assert [sim.uuid for sim in queried] == [simulation.uuid] + assert queried[0].find_meta("values.code") == ["my-code"] + + assert api.query_simulations(["values.code=other"], []) == [] + + +def test_trace_simulation(remote, tmp_path): + api, _ = remote + + simulation = _push_simulation(api, tmp_path) + trace = api.trace_simulation(simulation.uuid.hex) + + assert isinstance(trace, SimulationTraceData) + assert trace.uuid == simulation.uuid + assert trace.status == "not validated" + assert trace.replaces is None + + +def test_validate_simulation(remote, tmp_path): + api, _ = remote + + simulation = _push_simulation(api, tmp_path) + + assert api.validate_simulation(simulation.uuid.hex) == (True, "") + + +def test_watchers(remote, tmp_path): + api, _ = remote + + simulation = _push_simulation(api, tmp_path) + sim_id = simulation.uuid.hex + + assert api.list_watchers(sim_id) == [] + + api.add_watcher(sim_id, "watcher1", "example@iter.org", Notification.ALL) + assert api.list_watchers(sim_id) == [ + WatcherData(username="watcher1", email="example@iter.org", notification="A") + ] + + api.remove_watcher(sim_id, "watcher1") + assert api.list_watchers(sim_id) == [] + + +def test_metadata(remote, tmp_path): + api, admin = remote + + simulation = _push_simulation(api, tmp_path) + sim_id = simulation.uuid.hex + + assert admin.set_metadata(sim_id, "code", "first") == [] + assert admin.set_metadata(sim_id, "code", "second") == ["first"] + + assert admin.delete_metadata(sim_id, "code") is None + assert "code" not in api.get_simulation(sim_id).meta_dict() + + +def test_update_simulation_status(remote, tmp_path): + api, admin = remote + + simulation = _push_simulation(api, tmp_path) + sim_id = simulation.uuid.hex + + admin.update_simulation(sim_id, Simulation.Status.ACCEPTED) + + assert api.get_simulation(sim_id).status == Simulation.Status.ACCEPTED + + +def test_delete_simulation(remote, tmp_path): + api, admin = remote + + simulation = _push_simulation(api, tmp_path) + + deleted = admin.delete_simulation(simulation.uuid.hex) + + assert isinstance(deleted, SimulationDeleteResponse) + assert deleted.deleted.simulation == simulation.uuid + assert deleted.deleted.files + assert api.list_simulations() == [] + + +def test_pull_simulation(remote, tmp_path): + api, _ = remote + + simulation = _push_simulation(api, tmp_path) + + pulled = api.pull_simulation( + simulation.uuid.hex, tmp_path / "pulled", out_stream=io.StringIO() + ) + + assert pulled.uuid == simulation.uuid + assert Path(pulled.inputs[0].uri.path).read_text() == "hello simdb\n" + + +def test_simulation_data_reports_remote_errors(remote, tmp_path): + """The v1.3 IMAS data endpoint reports errors as a RemoteError, not a model.""" + api, _ = remote + + simulation = _push_simulation(api, tmp_path) + + with pytest.raises(RemoteError): + # the simulation holds no IMAS data, so the remote reports an error + api.get_simulation_data(simulation.uuid.hex, "core_profiles/time") + + +def test_invalid_json_from_remote_is_reported(remote): + """A non-JSON body is reported as a failed connection, not a pydantic error.""" + api, _ = remote + + with mock.patch.object( + RemoteAPI, "get", return_value=mock.Mock(content=b"login") + ), pytest.raises(FailedConnection, match="Invalid JSON"): + api.get_simulation("does-not-matter") + + +def test_unexpected_response_from_remote_is_reported(remote): + """A JSON body that does not match the model is reported as a remote error.""" + api, _ = remote + + with mock.patch.object( + RemoteAPI, "get", return_value=mock.Mock(content=b'{"unexpected": true}') + ), pytest.raises(RemoteError, match="Unexpected data exchanged"): + api.get_simulation("does-not-matter") diff --git a/tests/remote/api/test_watchers.py b/tests/remote/api/test_watchers.py index 7ec11a01..68f02565 100644 --- a/tests/remote/api/test_watchers.py +++ b/tests/remote/api/test_watchers.py @@ -26,7 +26,29 @@ def test_get_watchers(client): ) assert rv.status_code == 200 - WatcherGetResponse.model_validate(rv.json) + assert WatcherGetResponse.model_validate(rv.json).root == [] + + # Watchers that have been added are reported with the notification character + post_data = WatcherPostRequest( + user="testuser", email="example@iter.org", notification="ALL" + ) + rv_add = client.post( + f"/v1.2/watchers/{simulation_data.simulation.uuid.hex}", + json=post_data.model_dump(mode="json"), + headers=HEADERS, + content_type="application/json", + ) + assert rv_add.status_code == 200 + + rv = client.get( + f"/v1.2/watchers/{simulation_data.simulation.uuid.hex}", headers=HEADERS + ) + + assert rv.status_code == 200 + watchers = WatcherGetResponse.model_validate(rv.json).root + assert [(w.username, w.email, w.notification) for w in watchers] == [ + ("testuser", "example@iter.org", "A") + ] def test_post_watchers(client):