diff --git a/AGENTS.md b/AGENTS.md index 349e4fe..31e34e5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -137,7 +137,8 @@ sp run ls --limit 5 | jq -r '.data[] | "\(.run_id) \(.platform) \(.status) Exit codes to branch on: `0` ok, `3` unreachable, `4` not found, `5` validation, `6` auth, `7` rate limited, `8` conflict, `9` `run wait` timed out, `10` `run -wait` broke before it could finish. Under `run wait`, `1` means a run failed or +wait` broke before it could finish, `11` `sample download` received bytes that +do not match the sample's SHA-256 (nothing is written; retry once). Under `run wait`, `1` means a run failed or was canceled and nothing else does, so `1` is a verdict about the code while `9`/`10` say only that the platform misbehaved — block on the first, retry the others. **`2` means you got the command wrong** — an unknown flag, a missing @@ -172,6 +173,11 @@ when stdout is not a terminal, so JSON on stdout is always parseable. fields. An unmatched line reports `level=info`, `source=web`. Do not treat `[ERROR]` in a build log as authoritative — ffmpeg banner output shows up there too. +- **`sample download` fetches from a signed storage URL**, not the API, so the + token is never sent there and the URL expires after a while: a + `download_failed` with `http_status` 400/403 means run it again. It refuses to + overwrite an existing file without `--force` (exit 8). Its `-d/--dest` is not + the global `-o`, which stays the output format. - **`include_stack=true` on infra errors needs admin or contributor role** and 403s otherwise. diff --git a/README.md b/README.md index 802361e..cfa3cf0 100644 --- a/README.md +++ b/README.md @@ -138,6 +138,7 @@ sp run ls --pr 2309 # ... just one pull request's runs sp run wait 9412 9413 # block until they finish; exit code is the verdict sp run create --commit --platform linux --repository owner/repo sp sample ls / show / details # media samples +sp sample download 188 -d ./media # fetch the file, verified against its SHA-256 sp regression ls / show # regression-test definitions sp category ls # categories, with test counts ``` diff --git a/sp_cli/commands/sample.py b/sp_cli/commands/sample.py index d7b6e0d..c8e49c2 100644 --- a/sp_cli/commands/sample.py +++ b/sp_cli/commands/sample.py @@ -1,17 +1,22 @@ -"""``sp sample`` — list and inspect media samples.""" +"""``sp sample`` — list, inspect and download media samples.""" -from typing import Optional +import os +from typing import Any, Dict, Optional, Tuple import click -from sp_cli.constants import (MAX_OFFSET, MAX_PAGE_LIMIT, PLATFORMS, +from sp_cli.client import ApiError +from sp_cli.constants import (EXIT_CHECKSUM_MISMATCH, MAX_OFFSET, + MAX_PAGE_LIMIT, PLATFORMS, SAMPLE_CATALOG_STATUSES, SAMPLE_STATUSES) +from sp_cli.download import ChecksumMismatch, download_to +from sp_cli.output import render, render_error from sp_cli.runner import clean_params, fetch_and_render @click.group() def sample() -> None: - """List and inspect media samples.""" + """List, inspect and download media samples.""" @sample.command('ls') @@ -84,3 +89,127 @@ def sample_history(ctx: click.Context, sample_id: int, platform: Optional[str], 'created_after': created_after, 'created_before': created_before, 'limit': limit, 'offset': offset}) fetch_and_render(ctx, f'/samples/{sample_id}/history', params) + + +@sample.command('download') +@click.argument('sample_id', type=int) +@click.option('--dest', '-d', 'dest', type=click.Path(file_okay=True, dir_okay=True), default=None, + help='Directory to save into, or a file path. Default: the current directory, ' + 'under the name the platform stores it as.') +@click.option('--media-info', 'media_info', is_flag=True, default=False, + help='Download the MediaInfo XML instead of the sample itself.') +@click.option('--extra-file', 'extra_file_id', type=click.IntRange(1), default=None, + help='Download this accompanying file (an id from `sp sample details`) instead.') +@click.option('--force', is_flag=True, default=False, help='Overwrite an existing file.') +@click.pass_context +def sample_download(ctx: click.Context, sample_id: int, dest: Optional[str], media_info: bool, + extra_file_id: Optional[int], force: bool) -> None: + """Download a sample's media file, verifying it against its SHA-256. + + Samples are stored under their SHA-256, so the file is checked against the + platform's record before it is moved into place. A mismatch leaves nothing + behind and exits 11. The MediaInfo XML and extra files have no recorded + digest and are written unverified (`verified: false`). + + The API hands out a short-lived signed storage URL; the bytes come straight + from storage and the platform token is never sent there. A byte counter is + drawn on stderr when it is a terminal, in either output mode. + """ + if media_info and extra_file_id is not None: + raise click.UsageError('--media-info and --extra-file are mutually exclusive.') + + client = ctx.obj['client'] + output = ctx.obj['output'] + try: + kind, located, expected_sha = _locate(client, sample_id, media_info, extra_file_id) + url = located.get('download_url') + if not url: + raise ApiError( + 'not_downloadable', + 'The platform has this file only on its own disk (storage_status=' + f"{located.get('storage_status')}), so there is no URL to fetch it from.", + 404, {'storage_status': located.get('storage_status')}) + # basename: the name comes from the server and must not pick the directory. + name = os.path.basename(str(located.get('filename') or f'sample-{sample_id}')) + path = _resolve_destination(dest, name) + if os.path.exists(path) and not force: + raise ApiError('file_exists', f'{path} already exists; pass --force to overwrite it.', + 409, {'path': path}) + written = download_to(url, path, expected_sha256=expected_sha, timeout=client.timeout) + except ChecksumMismatch as mismatch: + render_error(ApiError( + 'checksum_mismatch', + 'The downloaded file does not match the SHA-256 the platform has on record; ' + 'nothing was written. Retry once; a repeat means the stored copy is damaged.', + None, {'expected': mismatch.expected, 'actual': mismatch.actual}), output) + raise SystemExit(EXIT_CHECKSUM_MISMATCH) + except ApiError as error: + render_error(error, output) + raise SystemExit(error.exit_code) + except OSError as exc: + render_error(ApiError('write_failed', f'Could not write the file: {exc}'), output) + raise SystemExit(1) + + payload: Dict[str, Any] = { + 'sample_id': sample_id, + 'file': kind, + 'path': os.path.abspath(path), + 'bytes': written['bytes'], + 'sha256': written['sha256'], + 'verified': expected_sha is not None, + 'storage_status': located.get('storage_status'), + } + if extra_file_id is not None: + payload['extra_file_id'] = extra_file_id + render(payload, output, ctx.obj.get('color', False)) + + +def _locate(client: Any, sample_id: int, media_info: bool, + extra_file_id: Optional[int]) -> Tuple[str, Dict[str, Any], Optional[str]]: + """ + Ask the API where the requested file is, and what it should hash to. + + :param client: The configured API client. + :type client: Any + :param sample_id: Sample the file belongs to. + :type sample_id: int + :param media_info: Whether the MediaInfo XML was asked for. + :type media_info: bool + :param extra_file_id: Accompanying file asked for, if any. + :type extra_file_id: Optional[int] + :return: ``(kind, download envelope, expected sha256 or None)``. + :rtype: Tuple[str, Dict[str, Any], Optional[str]] + """ + if media_info: + return 'media_info', client.get(f'/samples/{sample_id}/media-info/download'), None + if extra_file_id is not None: + located = client.get(f'/samples/{sample_id}/extra-files/{extra_file_id}/download') + return 'extra_file', located, None + # The digest comes from the sample record, not the download envelope, so a + # wrong URL cannot vouch for itself. + expected = client.get(f'/samples/{sample_id}').get('sha') + return 'sample', client.get(f'/samples/{sample_id}/download'), expected + + +def _resolve_destination(dest: Optional[str], name: str) -> str: + """ + Turn ``--dest`` into the file path to write. + + :param dest: The option as given: a directory, a file path, or ``None``. + :type dest: Optional[str] + :param name: The platform's name for the file. + :type name: str + :raises click.BadParameter: when the parent directory does not exist. + :return: The file path to write. + :rtype: str + """ + if dest is None: + return name + if os.path.isdir(dest): + return os.path.join(dest, name) + if dest.endswith(os.sep) or (os.altsep and dest.endswith(os.altsep)): + raise click.BadParameter(f'directory {dest} does not exist.', param_hint='--dest') + parent = os.path.dirname(os.path.abspath(dest)) + if not os.path.isdir(parent): + raise click.BadParameter(f'directory {parent} does not exist.', param_hint='--dest') + return dest diff --git a/sp_cli/constants.py b/sp_cli/constants.py index a4218ab..d2c784f 100644 --- a/sp_cli/constants.py +++ b/sp_cli/constants.py @@ -45,6 +45,12 @@ #: do map to a code of their own (3 through 8) keep it. EXIT_WAIT_ABORTED = 10 +#: ``sp sample download`` exits with this when the received bytes do not hash to +#: the SHA-256 the platform has on record. Its own code, because a corrupt +#: transfer is neither "not found" nor a generic failure: retrying may fix it, +#: and a repeat mismatch means the stored sample itself is damaged. +EXIT_CHECKSUM_MISMATCH = 11 + #: How many failing tests ``sp run report`` lists per reference in table mode. #: The counts are the answer and the rows are evidence for it; JSON output is #: never truncated, so nothing is lost to a script. diff --git a/sp_cli/download.py b/sp_cli/download.py new file mode 100644 index 0000000..26d2ba8 --- /dev/null +++ b/sp_cli/download.py @@ -0,0 +1,160 @@ +"""Stream a file from a signed storage URL to disk, hashing it on the way. + +The API never serves sample bytes itself: it hands out a short-lived signed URL +to the storage bucket (see ``/samples/{id}/download``). This module fetches that +URL. It deliberately does **not** go through :class:`sp_cli.client.ApiClient`: +that client attaches the bearer token to every request, and a signed URL points +at a third-party host that has no business seeing the platform credential. +""" + +import hashlib +import os +import tempfile +from typing import Any, Dict, Optional + +import requests # type: ignore[import-untyped] + +from sp_cli.client import ApiError +from sp_cli.progress import Spinner + +#: Bytes read per chunk. Large enough that hashing, not syscalls, dominates. +_CHUNK_SIZE = 1024 * 1024 + + +class ChecksumMismatch(Exception): + """Raised when a downloaded file does not hash to the value the API reported.""" + + def __init__(self, expected: str, actual: str) -> None: + """ + Record both digests so the caller can report them. + + :param expected: The SHA-256 the platform has on record. + :type expected: str + :param actual: The SHA-256 of the bytes actually received. + :type actual: str + """ + super().__init__(f'SHA-256 mismatch: expected {expected}, got {actual}') + self.expected = expected + self.actual = actual + + +def download_to(url: str, destination: str, expected_sha256: Optional[str] = None, + timeout: int = 30, show_progress: bool = True) -> Dict[str, Any]: + """ + Download ``url`` into ``destination``, verifying it when a digest is known. + + The bytes go to a temporary file beside the destination and are moved into + place only once complete and verified, so an interrupted or corrupt transfer + never leaves a file that looks like a finished sample. + + :param url: Signed storage URL, fetched without any platform credential. + :type url: str + :param destination: Final file path. + :type destination: str + :param expected_sha256: Hex digest the file must match, or ``None`` to skip. + :type expected_sha256: Optional[str] + :param timeout: Connect/read timeout in seconds (per read, not total). + :type timeout: int + :param show_progress: Whether to draw a byte counter on stderr. + :type show_progress: bool + :raises ApiError: when storage cannot be reached or refuses the request. + :raises ChecksumMismatch: when the received bytes do not match. + :return: ``{bytes, sha256}`` for the file written. + :rtype: Dict[str, Any] + """ + directory = os.path.dirname(os.path.abspath(destination)) + digest = hashlib.sha256() + received = 0 + fd, partial = tempfile.mkstemp(prefix='.sp-download-', suffix='.part', dir=directory) + try: + with os.fdopen(fd, 'wb') as handle: + try: + response = requests.get(url, stream=True, timeout=timeout) + except requests.RequestException as exc: + raise ApiError('connection_error', f'Could not reach storage: {exc}') + with response: + if response.status_code >= 400: + # Not the API's error envelope: storage answers in its own + # format. An expired signature is a 400 or 403 here. + raise ApiError( + 'download_failed', + f'Storage refused the download with HTTP {response.status_code}; ' + 'the signed URL may have expired, so run the command again.', + None, {'http_status': response.status_code}) + total = _content_length(response) + label = os.path.basename(destination) + with Spinner(_progress_label(label, 0, total), enabled=show_progress) as spinner: + try: + for chunk in response.iter_content(chunk_size=_CHUNK_SIZE): + if not chunk: + continue + handle.write(chunk) + digest.update(chunk) + received += len(chunk) + spinner.label = _progress_label(label, received, total) + except requests.RequestException as exc: + raise ApiError('connection_error', + f'Download interrupted after {received} bytes: {exc}') + + actual = digest.hexdigest() + if expected_sha256 is not None and actual != expected_sha256.lower(): + raise ChecksumMismatch(expected_sha256, actual) + # mkstemp creates the file 0600; give it the mode any other new file gets. + os.chmod(partial, 0o666 & ~_umask()) + os.replace(partial, destination) + except BaseException: + if os.path.exists(partial): + os.remove(partial) + raise + + return {'bytes': received, 'sha256': actual} + + +def _umask() -> int: + """ + Read the process umask, which can only be done by setting it. + + :return: The current umask. + :rtype: int + """ + current = os.umask(0) + os.umask(current) + return current + + +def _content_length(response: Any) -> Optional[int]: + """ + Read the advertised size, if the server sent a usable one. + + :param response: The streaming response. + :type response: Any + :return: The size in bytes, or ``None`` when absent or malformed. + :rtype: Optional[int] + """ + headers = getattr(response, 'headers', None) or {} + value = headers.get('Content-Length') + if value is None: + return None + try: + return int(value) + except (TypeError, ValueError): + return None + + +def _progress_label(name: str, received: int, total: Optional[int]) -> str: + """ + Format the progress line shown beside the spinner. + + :param name: File being written. + :type name: str + :param received: Bytes written so far. + :type received: int + :param total: Expected size, when known. + :type total: Optional[int] + :return: e.g. ``abc.ts 12.0/340.5 MiB``. + :rtype: str + """ + mib = 1024 * 1024 + if total: + return f'{name} {received / mib:.1f}/{total / mib:.1f} MiB' + return f'{name} {received / mib:.1f} MiB' diff --git a/sp_cli/progress.py b/sp_cli/progress.py index 68a8680..9e30fd0 100644 --- a/sp_cli/progress.py +++ b/sp_cli/progress.py @@ -4,6 +4,10 @@ output is not a live terminal, whenever ``NO_COLOR`` is set, and always in JSON mode. It draws on **stderr**, so even when it does run it cannot contaminate the payload a caller is parsing on stdout. + +The one exception to the JSON-mode rule is ``sp sample download``: a transfer +of hundreds of megabytes with no sign of life reads as a hang, and the TTY check +alone already keeps the counter away from any script or agent. """ import itertools diff --git a/tests/test_download.py b/tests/test_download.py new file mode 100644 index 0000000..addc702 --- /dev/null +++ b/tests/test_download.py @@ -0,0 +1,269 @@ +"""Tests for ``sp sample download``, mocking the API client and storage.""" + +import hashlib +import json +import os +import shutil +import tempfile +import unittest +from unittest import mock + +from click.testing import CliRunner + +from sp_cli.client import ApiError +from sp_cli.constants import EXIT_CHECKSUM_MISMATCH +from sp_cli.main import cli +from tests import SESSION_SANDBOX # noqa: F401 + +CONTENT = b'\x47\x40\x11\x10' * 50000 +SHA = hashlib.sha256(CONTENT).hexdigest() +FILENAME = f'{SHA}.ts' +SIGNED_URL = 'https://storage.googleapis.com/bucket/TestFiles/x.ts?X-Goog-Signature=abc' + + +class _FakeResponse: + """Just enough of ``requests.Response`` for a streamed download.""" + + def __init__(self, body=CONTENT, status_code=200): + self.status_code = status_code + self.headers = {'Content-Length': str(len(body))} + self._body = body + + def iter_content(self, chunk_size=1): + for start in range(0, len(self._body), chunk_size): + yield self._body[start:start + chunk_size] + + def __enter__(self): + return self + + def __exit__(self, *exc): + return False + + +def _api(located=None, sha=SHA): + """Build a fake ``ApiClient.get`` answering the sample and download paths.""" + envelope = located if located is not None else { + 'sample_id': 188, 'filename': FILENAME, 'download_url': SIGNED_URL, 'storage_status': 'ok'} + + def get(path, params=None): + if path == '/samples/188': + return {'sample_id': 188, 'sha': sha, 'filename': FILENAME} + if path.endswith('/download'): + return envelope + raise AssertionError(f'unexpected path {path}') + return get + + +class SampleDownloadTests(unittest.TestCase): + """Exercise ``sp sample download`` end to end with storage mocked.""" + + def setUp(self): + """Create a runner and a scratch destination directory.""" + self.runner = CliRunner() + self.dir = tempfile.mkdtemp(prefix='sp-download-') + self.addCleanup(shutil.rmtree, self.dir, True) + + def _leftovers(self): + return [name for name in os.listdir(self.dir) if name.endswith('.part')] + + @mock.patch('sp_cli.download.requests.get') + @mock.patch('sp_cli.client.ApiClient.get') + def test_downloads_and_verifies(self, mock_get, mock_storage): + """The file lands under its platform name and is reported as verified.""" + mock_get.side_effect = _api() + mock_storage.return_value = _FakeResponse() + + result = self.runner.invoke(cli, ['--token', 't', 'sample', 'download', '188', '--dest', self.dir]) + + self.assertEqual(result.exit_code, 0, result.output) + path = os.path.join(self.dir, FILENAME) + with open(path, 'rb') as handle: + self.assertEqual(handle.read(), CONTENT) + payload = json.loads(result.output) + self.assertEqual(payload['path'], os.path.abspath(path)) + self.assertEqual(payload['sha256'], SHA) + self.assertEqual(payload['bytes'], len(CONTENT)) + self.assertTrue(payload['verified']) + self.assertEqual(payload['file'], 'sample') + self.assertEqual(self._leftovers(), []) + + @unittest.skipIf(os.name == 'nt', 'POSIX permissions') + @mock.patch('sp_cli.download.requests.get') + @mock.patch('sp_cli.client.ApiClient.get') + def test_file_gets_ordinary_permissions(self, mock_get, mock_storage): + """The temporary file's 0600 does not survive onto the finished sample.""" + mock_get.side_effect = _api() + mock_storage.return_value = _FakeResponse() + previous = os.umask(0o022) + self.addCleanup(os.umask, previous) + + self.runner.invoke(cli, ['sample', 'download', '188', '--dest', self.dir]) + + mode = os.stat(os.path.join(self.dir, FILENAME)).st_mode & 0o777 + self.assertEqual(mode, 0o644) + + @mock.patch('sp_cli.download.requests.get') + @mock.patch('sp_cli.client.ApiClient.get') + def test_storage_never_sees_the_token(self, mock_get, mock_storage): + """The signed URL is fetched bare: the bearer token stays with the API.""" + mock_get.side_effect = _api() + mock_storage.return_value = _FakeResponse() + + self.runner.invoke(cli, ['--token', 'secret', 'sample', 'download', '188', '--dest', self.dir]) + + args, kwargs = mock_storage.call_args + self.assertEqual(args, (SIGNED_URL,)) + self.assertNotIn('headers', kwargs) + self.assertTrue(kwargs['stream']) + + @mock.patch('sp_cli.download.requests.get') + @mock.patch('sp_cli.client.ApiClient.get') + def test_checksum_mismatch_fails_and_leaves_nothing(self, mock_get, mock_storage): + """Corrupt bytes exit 11 with both digests and no file on disk.""" + mock_get.side_effect = _api() + mock_storage.return_value = _FakeResponse(body=CONTENT[:-1] + b'\x00') + + result = self.runner.invoke(cli, ['sample', 'download', '188', '--dest', self.dir]) + + self.assertEqual(result.exit_code, EXIT_CHECKSUM_MISMATCH) + error = json.loads(result.stderr)['error'] + self.assertEqual(error['code'], 'checksum_mismatch') + self.assertEqual(error['details']['expected'], SHA) + self.assertEqual(os.listdir(self.dir), []) + + @mock.patch('sp_cli.download.requests.get') + @mock.patch('sp_cli.client.ApiClient.get') + def test_expired_url_is_reported_and_cleaned_up(self, mock_get, mock_storage): + """A storage refusal is a download_failed error, not a traceback.""" + mock_get.side_effect = _api() + mock_storage.return_value = _FakeResponse(body=b'', status_code=403) + + result = self.runner.invoke(cli, ['sample', 'download', '188', '--dest', self.dir]) + + self.assertEqual(result.exit_code, 1) + error = json.loads(result.stderr)['error'] + self.assertEqual(error['code'], 'download_failed') + self.assertEqual(error['details'], {'http_status': 403}) + self.assertEqual(os.listdir(self.dir), []) + + @mock.patch('sp_cli.download.requests.get') + @mock.patch('sp_cli.client.ApiClient.get') + def test_local_only_copy_is_not_downloadable(self, mock_get, mock_storage): + """A null download_url exits 4 without touching storage.""" + mock_get.side_effect = _api(located={ + 'sample_id': 188, 'filename': FILENAME, 'download_url': None, + 'storage_status': 'degraded'}) + + result = self.runner.invoke(cli, ['sample', 'download', '188', '--dest', self.dir]) + + self.assertEqual(result.exit_code, 4) + self.assertEqual(json.loads(result.stderr)['error']['code'], 'not_downloadable') + mock_storage.assert_not_called() + + @mock.patch('sp_cli.client.ApiClient.get') + def test_api_error_propagates_exit_code(self, mock_get): + """A missing sample exits 4 like every other not-found.""" + mock_get.side_effect = ApiError('not_found', 'Sample 188 not found.', 404) + + result = self.runner.invoke(cli, ['sample', 'download', '188', '--dest', self.dir]) + + self.assertEqual(result.exit_code, 4) + + @mock.patch('sp_cli.download.requests.get') + @mock.patch('sp_cli.client.ApiClient.get') + def test_refuses_to_overwrite_without_force(self, mock_get, mock_storage): + """An existing file is a conflict (exit 8) unless --force is given.""" + mock_get.side_effect = _api() + mock_storage.return_value = _FakeResponse() + path = os.path.join(self.dir, FILENAME) + with open(path, 'wb') as handle: + handle.write(b'old') + + refused = self.runner.invoke(cli, ['sample', 'download', '188', '--dest', self.dir]) + self.assertEqual(refused.exit_code, 8) + with open(path, 'rb') as handle: + self.assertEqual(handle.read(), b'old') + + forced = self.runner.invoke(cli, ['sample', 'download', '188', '--dest', self.dir, '--force']) + self.assertEqual(forced.exit_code, 0, forced.output) + with open(path, 'rb') as handle: + self.assertEqual(handle.read(), CONTENT) + + @mock.patch('sp_cli.download.requests.get') + @mock.patch('sp_cli.client.ApiClient.get') + def test_dest_may_name_the_file(self, mock_get, mock_storage): + """A --dest that is not a directory is the file path to write.""" + mock_get.side_effect = _api() + mock_storage.return_value = _FakeResponse() + target = os.path.join(self.dir, 'mine.ts') + + result = self.runner.invoke(cli, ['sample', 'download', '188', '--dest', target]) + + self.assertEqual(result.exit_code, 0, result.output) + self.assertTrue(os.path.isfile(target)) + + @mock.patch('sp_cli.client.ApiClient.get') + def test_missing_dest_directory_is_a_usage_error(self, mock_get): + """A --dest inside a directory that does not exist exits 2.""" + mock_get.side_effect = _api() + missing = os.path.join(self.dir, 'nope', 'file.ts') + + result = self.runner.invoke(cli, ['sample', 'download', '188', '--dest', missing]) + + self.assertEqual(result.exit_code, 2) + + @mock.patch('sp_cli.download.requests.get') + @mock.patch('sp_cli.client.ApiClient.get') + def test_server_filename_cannot_escape_dest(self, mock_get, mock_storage): + """A filename with path components is reduced to its basename.""" + mock_get.side_effect = _api(located={ + 'sample_id': 188, 'filename': '../../evil.ts', 'download_url': SIGNED_URL, + 'storage_status': 'ok'}) + mock_storage.return_value = _FakeResponse() + + result = self.runner.invoke(cli, ['sample', 'download', '188', '--dest', self.dir]) + + self.assertEqual(result.exit_code, 0, result.output) + self.assertEqual(os.listdir(self.dir), ['evil.ts']) + + @mock.patch('sp_cli.download.requests.get') + @mock.patch('sp_cli.client.ApiClient.get') + def test_media_info_is_unverified(self, mock_get, mock_storage): + """--media-info hits its own route and reports verified=false.""" + mock_get.return_value = {'sample_id': 188, 'filename': f'{SHA}.xml', + 'download_url': SIGNED_URL, 'storage_status': 'ok'} + mock_storage.return_value = _FakeResponse(body=b'') + + result = self.runner.invoke(cli, ['sample', 'download', '188', '--media-info', '--dest', self.dir]) + + self.assertEqual(result.exit_code, 0, result.output) + mock_get.assert_called_once_with('/samples/188/media-info/download') + payload = json.loads(result.output) + self.assertEqual(payload['file'], 'media_info') + self.assertFalse(payload['verified']) + + @mock.patch('sp_cli.download.requests.get') + @mock.patch('sp_cli.client.ApiClient.get') + def test_extra_file(self, mock_get, mock_storage): + """--extra-file targets the accompanying file's download route.""" + mock_get.return_value = {'sample_id': 188, 'filename': 'extra.srt', + 'download_url': SIGNED_URL, 'storage_status': 'ok'} + mock_storage.return_value = _FakeResponse(body=b'1\n') + + result = self.runner.invoke(cli, ['sample', 'download', '188', '--extra-file', '7', + '--dest', self.dir]) + + self.assertEqual(result.exit_code, 0, result.output) + mock_get.assert_called_once_with('/samples/188/extra-files/7/download') + self.assertEqual(json.loads(result.output)['extra_file_id'], 7) + + def test_media_info_and_extra_file_are_exclusive(self): + """Asking for both is a usage error.""" + result = self.runner.invoke(cli, ['sample', 'download', '188', '--media-info', + '--extra-file', '7']) + + self.assertEqual(result.exit_code, 2) + + +if __name__ == '__main__': + unittest.main()