diff --git a/gateway/sds_gateway/api_methods/helpers/temporal_filtering.py b/gateway/sds_gateway/api_methods/helpers/temporal_filtering.py index cc64718e8..51c1d0c9d 100644 --- a/gateway/sds_gateway/api_methods/helpers/temporal_filtering.py +++ b/gateway/sds_gateway/api_methods/helpers/temporal_filtering.py @@ -20,6 +20,7 @@ def drf_rf_filename_from_ms(ms: int) -> str: """Format ms as DRF rf data filename (canonical for range queries).""" + ms = int(ms) return f"rf@{ms // 1000}.{ms % 1000:03d}.h5" diff --git a/gateway/sds_gateway/api_methods/models.py b/gateway/sds_gateway/api_methods/models.py index 3eaaf492c..95a16c73e 100644 --- a/gateway/sds_gateway/api_methods/models.py +++ b/gateway/sds_gateway/api_methods/models.py @@ -1240,6 +1240,28 @@ def get_authors_display(self): # Authors should already be in new object format return self.authors + def get_institutions_display(self): + if not self.institutions or self.institutions == "": + return [] + + if not isinstance(self.institutions, list): + log.warning( + f"Dataset {self.uuid}: institutions field is not a list " + f"(type: {type(self.institutions).__name__})", + ) + return [] + + if self.institutions and isinstance(self.institutions[0], str): + log.warning( + f"Dataset {self.uuid}: institutions still in old string format, " + f"needs migration", + ) + # Convert old format for backward compatibility + return [{institution} for institution in self.institutions] + + # institutions should already be in new object format + return self.institutions + class TemporaryZipFile(BaseModel): """ diff --git a/gateway/sds_gateway/api_methods/serializers/dataset_serializers.py b/gateway/sds_gateway/api_methods/serializers/dataset_serializers.py index ce9c79438..b970e355d 100644 --- a/gateway/sds_gateway/api_methods/serializers/dataset_serializers.py +++ b/gateway/sds_gateway/api_methods/serializers/dataset_serializers.py @@ -24,10 +24,17 @@ READABLE_ISO_DATE_TIME: str = "%Y-%m-%d %H:%M:%S%z" +class DatasetAttachCaptureSerializer(serializers.Serializer): + """Validate a request to attach an existing capture to a dataset.""" + + capture_uuid = serializers.UUIDField(required=True) + + class DatasetGetSerializer(serializers.ModelSerializer[Dataset]): owner = UserGetSerializer(read_only=True) authors = serializers.SerializerMethodField() keywords = serializers.SerializerMethodField() + institutions = serializers.SerializerMethodField() created_at = serializers.DateTimeField( format=READABLE_ISO_DATE_TIME, read_only=True ) @@ -51,6 +58,13 @@ def get_authors(self, obj): """Return the full authors list using the model's get_authors_display method.""" return obj.get_authors_display() + def get_institutions(self, obj): + if not obj.institutions: + return [] + if isinstance(obj.institutions, list): + return obj.institutions + return [] + def get_keywords(self, obj): """Return a list of keyword names for the dataset.""" return [kw.name for kw in obj.keywords.filter(is_deleted=False)] diff --git a/gateway/sds_gateway/api_methods/tests/test_dataset_endpoints.py b/gateway/sds_gateway/api_methods/tests/test_dataset_endpoints.py index 6e247078a..39f8cc159 100644 --- a/gateway/sds_gateway/api_methods/tests/test_dataset_endpoints.py +++ b/gateway/sds_gateway/api_methods/tests/test_dataset_endpoints.py @@ -10,8 +10,11 @@ from sds_gateway.api_methods.models import Capture from sds_gateway.api_methods.models import CaptureType +from sds_gateway.api_methods.models import DatasetStatus from sds_gateway.api_methods.models import ItemType +from sds_gateway.api_methods.models import PermissionLevel from sds_gateway.api_methods.models import UserSharePermission +from sds_gateway.api_methods.tests.factories import CaptureFactory from sds_gateway.api_methods.tests.factories import DatasetFactory from sds_gateway.api_methods.tests.factories import MockMinIOContext from sds_gateway.api_methods.tests.factories import UserFactory @@ -808,3 +811,207 @@ def test_get_dataset_files_shared_capture_disabled_permission(self): # Should get 403 Forbidden because the share permission is disabled assert response.status_code == status.HTTP_403_FORBIDDEN + + +class DatasetAttachCaptureEndpointTestCase(TestCase): + """Tests for attaching owned captures to editable datasets.""" + + def setUp(self) -> None: + self.client = APIClient() + self.user = UserFactory() + self.client.force_authenticate(user=self.user) + self.dataset = DatasetFactory(owner=self.user, keywords=None) + self.capture = CaptureFactory(owner=self.user) + self.url = reverse( + "api:datasets-attach-capture", + kwargs={"pk": self.dataset.uuid}, + ) + + def test_owner_can_attach_capture_with_json(self) -> None: + response = self.client.post( + self.url, + {"capture_uuid": str(self.capture.uuid)}, + format="json", + ) + + assert response.status_code == status.HTTP_200_OK + assert response.data["added"] == [str(self.capture.uuid)] + assert response.data["skipped"] == [] + assert self.dataset.captures.filter(pk=self.capture.pk).exists() + + def test_attach_is_idempotent(self) -> None: + self.dataset.captures.add(self.capture) + + response = self.client.post( + self.url, + {"capture_uuid": str(self.capture.uuid)}, + format="json", + ) + + assert response.status_code == status.HTTP_200_OK + assert response.data["added"] == [] + assert response.data["skipped"] == [str(self.capture.uuid)] + assert self.dataset.captures.filter(pk=self.capture.pk).count() == 1 + + def test_missing_or_invalid_capture_uuid_returns_bad_request(self) -> None: + missing_response = self.client.post(self.url, {}, format="json") + invalid_response = self.client.post( + self.url, + {"capture_uuid": "not-a-uuid"}, + format="json", + ) + + assert missing_response.status_code == status.HTTP_400_BAD_REQUEST + assert invalid_response.status_code == status.HTTP_400_BAD_REQUEST + + def test_invalid_dataset_uuid_returns_bad_request(self) -> None: + url = reverse( + "api:datasets-attach-capture", + kwargs={"pk": "not-a-uuid"}, + ) + + response = self.client.post( + url, + {"capture_uuid": str(self.capture.uuid)}, + format="json", + ) + + assert response.status_code == status.HTTP_400_BAD_REQUEST + + def test_contributor_can_attach_owned_capture(self) -> None: + owner = UserFactory() + shared_dataset = DatasetFactory(owner=owner, keywords=None) + UserSharePermission.objects.create( + owner=owner, + shared_with=self.user, + item_type=ItemType.DATASET, + item_uuid=shared_dataset.uuid, + permission_level=PermissionLevel.CONTRIBUTOR, + ) + url = reverse( + "api:datasets-attach-capture", + kwargs={"pk": shared_dataset.uuid}, + ) + + response = self.client.post( + url, + {"capture_uuid": str(self.capture.uuid)}, + format="json", + ) + + assert response.status_code == status.HTTP_200_OK + assert shared_dataset.captures.filter(pk=self.capture.pk).exists() + + def test_viewer_cannot_attach_capture(self) -> None: + owner = UserFactory() + shared_dataset = DatasetFactory(owner=owner, keywords=None) + UserSharePermission.objects.create( + owner=owner, + shared_with=self.user, + item_type=ItemType.DATASET, + item_uuid=shared_dataset.uuid, + permission_level=PermissionLevel.VIEWER, + ) + url = reverse( + "api:datasets-attach-capture", + kwargs={"pk": shared_dataset.uuid}, + ) + + response = self.client.post( + url, + {"capture_uuid": str(self.capture.uuid)}, + format="json", + ) + + assert response.status_code == status.HTTP_403_FORBIDDEN + assert not shared_dataset.captures.filter(pk=self.capture.pk).exists() + + def test_user_cannot_attach_another_users_capture(self) -> None: + foreign_capture = CaptureFactory(owner=UserFactory()) + + response = self.client.post( + self.url, + {"capture_uuid": str(foreign_capture.uuid)}, + format="json", + ) + + assert response.status_code == status.HTTP_404_NOT_FOUND + assert not self.dataset.captures.filter(pk=foreign_capture.pk).exists() + + def test_published_dataset_cannot_be_modified(self) -> None: + for dataset in ( + DatasetFactory( + owner=self.user, + status=DatasetStatus.FINAL, + keywords=None, + ), + DatasetFactory(owner=self.user, is_public=True, keywords=None), + ): + with self.subTest(dataset=dataset.uuid): + url = reverse( + "api:datasets-attach-capture", + kwargs={"pk": dataset.uuid}, + ) + response = self.client.post( + url, + {"capture_uuid": str(self.capture.uuid)}, + format="json", + ) + + assert response.status_code == status.HTTP_403_FORBIDDEN + assert not dataset.captures.filter(pk=self.capture.pk).exists() + + def test_all_multi_channel_siblings_are_attached(self) -> None: + top_level_dir = "multi-channel-capture" + first = CaptureFactory( + owner=self.user, + top_level_dir=top_level_dir, + capture_type=CaptureType.DigitalRF, + channel="channel-1", + ) + second = CaptureFactory( + owner=self.user, + top_level_dir=top_level_dir, + capture_type=CaptureType.DigitalRF, + channel="channel-2", + ) + + response = self.client.post( + self.url, + {"capture_uuid": str(first.uuid)}, + format="json", + ) + + assert response.status_code == status.HTTP_200_OK + assert set(response.data["added"]) == {str(first.uuid), str(second.uuid)} + expected_capture_count = 2 + assert ( + self.dataset.captures.filter(pk__in=[first.pk, second.pk]).count() + == expected_capture_count + ) + + def test_attachment_rolls_back_when_database_add_fails(self) -> None: + manager_class = type(self.dataset.captures) + + def failing_add(manager, *captures, **_kwargs) -> None: + manager.through.objects.create( + dataset_id=manager.instance.pk, + capture_id=captures[0].pk, + ) + message = "simulated database failure" + raise RuntimeError(message) + + with patch.object( + manager_class, + "add", + autospec=True, + side_effect=failing_add, + ): + response = self.client.post( + self.url, + {"capture_uuid": str(self.capture.uuid)}, + format="json", + ) + + assert response.status_code == status.HTTP_500_INTERNAL_SERVER_ERROR + assert not self.dataset.captures.filter(pk=self.capture.pk).exists() diff --git a/gateway/sds_gateway/api_methods/views/dataset_endpoints.py b/gateway/sds_gateway/api_methods/views/dataset_endpoints.py index 2ef78566f..250c38fd6 100644 --- a/gateway/sds_gateway/api_methods/views/dataset_endpoints.py +++ b/gateway/sds_gateway/api_methods/views/dataset_endpoints.py @@ -3,7 +3,9 @@ from __future__ import annotations from typing import TYPE_CHECKING +from uuid import UUID +from django.db import transaction from django.shortcuts import get_object_or_404 from drf_spectacular.utils import OpenApiParameter from drf_spectacular.utils import OpenApiResponse @@ -15,10 +17,16 @@ from rest_framework.viewsets import ViewSet from sds_gateway.api_methods.authentication import APIKeyAuthentication +from sds_gateway.api_methods.models import Capture from sds_gateway.api_methods.models import Dataset +from sds_gateway.api_methods.models import DatasetStatus from sds_gateway.api_methods.models import File from sds_gateway.api_methods.models import ItemType +from sds_gateway.api_methods.models import UserSharePermission from sds_gateway.api_methods.models import user_has_access_to_item +from sds_gateway.api_methods.serializers.dataset_serializers import ( + DatasetAttachCaptureSerializer, +) from sds_gateway.api_methods.serializers.dataset_serializers import DatasetGetSerializer from sds_gateway.api_methods.serializers.file_serializers import FileGetSerializer from sds_gateway.api_methods.utils.asset_access_control import check_if_shared @@ -42,8 +50,6 @@ from sds_gateway.users.models import User if TYPE_CHECKING: - from uuid import UUID - from django.db.models import QuerySet from rest_framework.request import Request @@ -266,6 +272,123 @@ def get_dataset_files(self, request: Request, pk: str | None = None) -> Response return paginator.get_paginated_response(serializer.data) + @extend_schema( + parameters=[ + OpenApiParameter( + name="id", + description="Dataset UUID", + required=True, + type=str, + location=OpenApiParameter.PATH, + ), + ], + request=DatasetAttachCaptureSerializer, + responses={ + 200: OpenApiResponse(description="Capture attached to Dataset"), + 400: OpenApiResponse(description="Bad Request"), + 403: OpenApiResponse(description="Forbidden"), + 404: OpenApiResponse(description="Not Found"), + 500: OpenApiResponse(description="Error attaching Capture to Dataset"), + }, + description=("Attach a Capture to a Dataset."), + summary="Attach Capture to Dataset", + ) + @action( + detail=True, + methods=["post"], + url_path="attach-capture", + url_name="attach-capture", + ) + def attach_capture_to_dataset( + self, request: Request, pk: str | None = None + ) -> Response: + """Link a capture to a dataset.""" + try: + dataset_uuid = UUID(str(pk)) + except ValueError: + return Response( + {"detail": "Dataset UUID must be a valid UUID."}, + status=status.HTTP_400_BAD_REQUEST, + ) + + request_serializer = DatasetAttachCaptureSerializer(data=request.data) + if not request_serializer.is_valid(): + return Response( + request_serializer.errors, + status=status.HTTP_400_BAD_REQUEST, + ) + + target_dataset = get_object_or_404( + Dataset, + pk=dataset_uuid, + is_deleted=False, + ) + + assert isinstance(request.user, User), ( + "Expected request.user to be an instance of the custom User model" + ) + if not UserSharePermission.user_can_add_assets( + request.user, + target_dataset.uuid, + ItemType.DATASET, + ): + return Response( + {"detail": "You do not have permission to add assets to this dataset."}, + status=status.HTTP_403_FORBIDDEN, + ) + + if target_dataset.status == DatasetStatus.FINAL or target_dataset.is_public: + return Response( + {"detail": "Published datasets cannot be edited."}, + status=status.HTTP_403_FORBIDDEN, + ) + + capture_uuid = request_serializer.validated_data["capture_uuid"] + capture = get_object_or_404( + Capture, + pk=capture_uuid, + owner=request.user, + is_deleted=False, + ) + + candidates = ( + list( + Capture.objects.filter( + top_level_dir=capture.top_level_dir, + owner=request.user, + is_deleted=False, + capture_type=capture.capture_type, + ) + ) + if capture.is_multi_channel + else [capture] + ) + + existing_pks = set( + target_dataset.captures.filter( + pk__in=[c.pk for c in candidates] + ).values_list("pk", flat=True) + ) + + captures_to_add = [c for c in candidates if c.pk not in existing_pks] + try: + with transaction.atomic(): + target_dataset.captures.add(*captures_to_add) + except Exception: # noqa: BLE001 + return Response( + {"detail": f"Failed to attach Capture to Dataset {dataset_uuid}."}, + status=status.HTTP_500_INTERNAL_SERVER_ERROR, + ) + + return Response( + status=status.HTTP_200_OK, + data={ + "message": "Capture attached to Dataset.", + "added": [str(c.uuid) for c in captures_to_add], + "skipped": [str(c.uuid) for c in candidates if c.pk in existing_pks], + }, + ) + @extend_schema( parameters=[ OpenApiParameter( diff --git a/gateway/sds_gateway/templates/users/downloads_list.html b/gateway/sds_gateway/templates/users/downloads_list.html index a0b4dd30d..02a1cedfa 100644 --- a/gateway/sds_gateway/templates/users/downloads_list.html +++ b/gateway/sds_gateway/templates/users/downloads_list.html @@ -95,7 +95,7 @@

Pending Downloads

{% if download.downloaded_at %}{{ download.downloaded_at|date:"M j, Y, P" }}{% endif %} {% if download.status == "Ready" or download.status == "Downloaded" %} - Download + Download {% endif %} diff --git a/sdk/docs/mkdocs/changelog.md b/sdk/docs/mkdocs/changelog.md index 7883b5b01..b7ac23209 100644 --- a/sdk/docs/mkdocs/changelog.md +++ b/sdk/docs/mkdocs/changelog.md @@ -1,5 +1,10 @@ # SpectrumX SDK Changelog +## `0.2.4` - 2026-10-15 + ++ Features: + + **New SDK function**: Client.add_capture_to_dataset(dataset_uuid, capture_uuid) + ## `0.2.3` - 2026-08-27 + Fixes: diff --git a/sdk/docs/mkdocs/faq.md b/sdk/docs/mkdocs/faq.md index 666719ff8..89fa3fb83 100644 --- a/sdk/docs/mkdocs/faq.md +++ b/sdk/docs/mkdocs/faq.md @@ -12,7 +12,8 @@ And others not as much. + [How do I set up authentication?](#how-do-i-set-up-authentication) + [How to generate a secret token?](#how-to-generate-a-secret-token) + [Why can't I generate an API key?](#why-cant-i-generate-an-api-key) - + [Why is my data not uploaded? | What is dry-run mode?](#why-is-my-data-not-uploaded--what-is-dry-run-mode) + + + [Why is my data not uploaded? | What is dry-run mode?](#why-is-my-data-not-uploaded-what-is-dry-run-mode) + [Where can I find code examples?](#where-can-i-find-code-examples) + [File Operations](#file-operations) + [How do I upload files to the SDS?](#how-do-i-upload-files-to-the-sds) @@ -20,7 +21,8 @@ And others not as much. + [Can I resume interrupted uploads or downloads?](#can-i-resume-interrupted-uploads-or-downloads) + [How do I handle errors during file operations?](#how-do-i-handle-errors-during-file-operations) + [Asset Types and Organization](#asset-types-and-organization) - + [What is a File | Directory | Capture | Dataset | Experiment?](#what-is-a-file--directory--capture--dataset--experiment) + + + [What is a File | Directory | Capture | Dataset | Experiment?](#what-is-a-file-directory-capture-dataset-experiment) + [What are Draft and Final Datasets?](#what-are-draft-and-final-datasets) + [Can we share a draft dataset?](#can-we-share-a-draft-dataset) + [What's the difference between a Directory and a Dataset?](#whats-the-difference-between-a-directory-and-a-dataset) diff --git a/sdk/src/spectrumx/api/captures.py b/sdk/src/spectrumx/api/captures.py index 092476db4..9efc663b9 100644 --- a/sdk/src/spectrumx/api/captures.py +++ b/sdk/src/spectrumx/api/captures.py @@ -324,7 +324,9 @@ def delete( def revoke_share_permissions(self, capture_uuid: uuid.UUID) -> bool: """Revoke all direct share permissions on this capture (owner-only). - Use this (or the web portal) before :meth:`delete` when the capture is shared. + Use this (or the web portal) before + [`delete`][spectrumx.api.captures.CaptureAPI.delete] + when the capture is shared. """ if self.verbose: log.bind(cat=LogCategory.FILESYSTEM).debug( @@ -341,7 +343,8 @@ def revoke_share_permissions(self, capture_uuid: uuid.UUID) -> bool: def detach_from_datasets(self, capture_uuid: uuid.UUID) -> bool: """Remove this capture from all datasets (owner-only). - Use before :meth:`delete` when the capture is still linked to datasets. + Use before [`delete`][spectrumx.api.captures.CaptureAPI.delete] + when the capture is still linked to datasets. """ if self.verbose: log.bind(cat=LogCategory.FILESYSTEM).debug( diff --git a/sdk/src/spectrumx/api/datasets.py b/sdk/src/spectrumx/api/datasets.py index 8cf0ccd33..570e4aff0 100644 --- a/sdk/src/spectrumx/api/datasets.py +++ b/sdk/src/spectrumx/api/datasets.py @@ -44,7 +44,8 @@ def get(self, dataset_uuid: UUID) -> Dataset: Captures are returned in the same grouped shape as the capture list API (one entry per logical multi-channel capture where applicable). For every - file in the dataset (including capture-linked files), use :meth:`get_files` + file in the dataset (including capture-linked files), + use [`get_files`][spectrumx.api.datasets.DatasetAPI.get_files] instead, which calls the paginated dataset files manifest endpoint. """ if self.dry_run: @@ -61,7 +62,8 @@ def list_captures(self, dataset_uuid: UUID) -> list[dict[str, Any]]: """Return capture payloads linked to the dataset (raw JSON objects). Use this when you need composite capture fields (for example ``channels``) - without coercing through :class:`~spectrumx.models.datasets.DatasetCapture`. + without coercing through + [`DatasetCapture`][spectrumx.models.datasets.DatasetCapture]. """ if self.dry_run: log_user("Dry run enabled: returning an empty capture list") @@ -78,9 +80,10 @@ def list_captures(self, dataset_uuid: UUID) -> list[dict[str, Any]]: def list_artifact_files(self, dataset_uuid: UUID) -> list[dict[str, Any]]: """Return file rows linked directly to the dataset (artifacts), as JSON dicts. - These are the same objects embedded on :meth:`get` under the ``files`` key. + These are the same objects embedded on + [`get`][spectrumx.api.datasets.DatasetAPI.get] under the ``files`` key. For the full downloadable manifest (captures plus artifacts), use - :meth:`get_files`. + [`get_files`][spectrumx.api.datasets.DatasetAPI.get_files]. """ if self.dry_run: log_user("Dry run enabled: returning an empty artifact file list") @@ -182,7 +185,8 @@ def delete( def revoke_share_permissions(self, dataset_uuid: UUID) -> bool: """Revoke all direct share permissions on this dataset (owner-only). - Use this (or the web portal) before :meth:`delete` when the dataset is shared. + Use this (or the web portal) before + [`delete`][spectrumx.api.datasets.DatasetAPI.delete] when the dataset is shared. """ if self.verbose: log.bind(cat=LogCategory.FILESYSTEM).debug( @@ -195,3 +199,22 @@ def revoke_share_permissions(self, dataset_uuid: UUID) -> bool: return True self.gateway.revoke_dataset_share_permissions(dataset_uuid=dataset_uuid) return True + + def add_capture_to_dataset(self, dataset_uuid: UUID, capture_uuid: UUID) -> bool: + """Add an existing capture to an existing dataset.""" + if self.verbose: + log.bind(cat=LogCategory.FILESYSTEM).debug( + f"Attaching capture {capture_uuid} to dataset {dataset_uuid}" + ) + if self.dry_run: + log.bind(cat=LogCategory.FILESYSTEM).debug( + f"Dry run enabled: would attach capture {capture_uuid} " + f"to dataset {dataset_uuid}" + ) + return True + self.gateway.add_capture_to_dataset( + dataset_uuid=dataset_uuid, + capture_uuid=capture_uuid, + verbose=self.verbose, + ) + return True diff --git a/sdk/src/spectrumx/client.py b/sdk/src/spectrumx/client.py index dc25152d8..3583794c5 100644 --- a/sdk/src/spectrumx/client.py +++ b/sdk/src/spectrumx/client.py @@ -296,9 +296,9 @@ def download( Args: from_sds_path: The virtual directory on SDS to download files from. start_time: The start time to filter DRF capture files by - (optional, only applicable to DRF capture files). + (optional, only applicable to DRF capture files). end_time: The end time to filter DRF capture files by - (optional, only applicable to DRF capture files). + (optional, only applicable to DRF capture files). to_local_path: The local path to save the downloaded files to. files_to_download: A paginator or list (in dry run mode) of files to download. If not provided, all files in the directory will be @@ -784,8 +784,9 @@ def download_dataset( UUIDs are downloaded (dataset artifacts with no capture are excluded). top_level_dirs: If set, only files whose ``directory`` lies under one of these capture ``top_level_dir`` paths (as from - :meth:`list_dataset_captures`) are included. Leading/trailing slashes - are normalized. + [`list_dataset_captures`][spectrumx.client.Client.list_dataset_captures]) + are included. + Leading/trailing slashes are normalized. artifacts_only: If set, only return artifact files (not capture-linked files). skip_contents: When True, only the metadata is downloaded. @@ -884,6 +885,18 @@ def list_dataset_captures(self, dataset_uuid: UUID4 | str) -> list[dict[str, Any dataset_uuid = UUID(dataset_uuid) return self.datasets.list_captures(dataset_uuid) + def add_capture_to_dataset( + self, dataset_uuid: UUID4 | str, capture_uuid: UUID4 | str + ) -> Dataset: + """Attach a Capture to an existing Dataset.""" + if isinstance(dataset_uuid, str): + dataset_uuid = UUID(dataset_uuid) + if isinstance(capture_uuid, str): + capture_uuid = UUID(capture_uuid) + + self.datasets.add_capture_to_dataset(dataset_uuid, capture_uuid) + return self.datasets.get(dataset_uuid) + def list_dataset_artifact_files( self, dataset_uuid: UUID4 | str ) -> list[dict[str, Any]]: diff --git a/sdk/src/spectrumx/gateway.py b/sdk/src/spectrumx/gateway.py index fb033b13e..19ff40085 100644 --- a/sdk/src/spectrumx/gateway.py +++ b/sdk/src/spectrumx/gateway.py @@ -52,6 +52,7 @@ class Endpoints(StrEnum): ) DATASETS = "/assets/datasets" DATASET_FILES = "/assets/datasets/{uuid}/files" + DATASET_ATTACH_CAPTURE = "/assets/datasets/{uuid}/attach-capture" DATASET_REVOKE_SHARE_PERMISSIONS = ( "/assets/datasets/{uuid}/revoke-share-permissions" ) @@ -756,6 +757,33 @@ def revoke_capture_share_permissions( network.success_or_raise(response, ContextException=CaptureError) return response.content + def add_capture_to_dataset( + self, + *, + dataset_uuid: uuid.UUID, + capture_uuid: uuid.UUID, + verbose: bool = False, + ) -> bytes: + """Attach a capture to a specified dataset. + + Args: + dataset_uuid: UUID of the dataset. + capture_uuid: UUID of the capture. + Returns: + Raw response body (typically JSON with a success message). + Raises: + CaptureError: If the request fails. + """ + response = self._request( + method=HTTPMethods.POST, + endpoint=Endpoints.DATASET_ATTACH_CAPTURE, + endpoint_args={"uuid": dataset_uuid.hex}, + json={"capture_uuid": capture_uuid.hex}, + verbose=verbose, + ) + network.success_or_raise(response, ContextException=DatasetError) + return response.content + def detach_capture_from_datasets( self, *, diff --git a/sdk/src/spectrumx/models/datasets.py b/sdk/src/spectrumx/models/datasets.py index 61e913ca9..5b4690d78 100644 --- a/sdk/src/spectrumx/models/datasets.py +++ b/sdk/src/spectrumx/models/datasets.py @@ -23,6 +23,8 @@ class DatasetFile(BaseModel): class DatasetCapture(BaseModel): + """A Capture linked to a Dataset.""" + model_config = ConfigDict(extra="ignore") uuid: UUID4 | None = None @@ -45,10 +47,10 @@ class Dataset(BaseModel): abstract: str | None = None description: str | None = None doi: str | None = None - authors: list[str] | None = None + authors: list[dict[str, Any]] | list[str] | None = None license: str | None = None keywords: list[str] | None = None - institutions: list[str] | None = None + institutions: list[str] | str | None = None release_date: datetime | None = None repository: str | None = None version: int | None = None diff --git a/sdk/src/spectrumx/models/files/file.py b/sdk/src/spectrumx/models/files/file.py index ff2b2ae3b..2ecc92080 100644 --- a/sdk/src/spectrumx/models/files/file.py +++ b/sdk/src/spectrumx/models/files/file.py @@ -92,8 +92,6 @@ def compute_sum_blake3(self) -> str | None: If the file is currently being downloaded, this method will block until the download is complete or terminated. - Args: - block: when True, waits until the file contents are unlocked. Returns: The BLAKE3 checksum of the file, OR None if the file is not available locally. diff --git a/sdk/tests/conftest.py b/sdk/tests/conftest.py index 92363100b..7657d424b 100644 --- a/sdk/tests/conftest.py +++ b/sdk/tests/conftest.py @@ -292,6 +292,15 @@ def get_dataset_revoke_share_permissions_url(client: Client, dataset_id: str) -> ) +def get_dataset_attach_capture_url(client: Client, dataset_id: str) -> str: + """URL for POST attach-capture on a dataset.""" + return ( + client.base_url + + f"/api/{API_TARGET_VERSION}/assets/datasets/{dataset_id}/" + + "attach-capture/" + ) + + def get_content_check_endpoint(client: Client) -> str: """Returns the endpoint for the content check API.""" return ( diff --git a/sdk/tests/integration/test_file_ops.py b/sdk/tests/integration/test_file_ops.py index 1b471fcec..1de43b8a4 100644 --- a/sdk/tests/integration/test_file_ops.py +++ b/sdk/tests/integration/test_file_ops.py @@ -5,6 +5,7 @@ import uuid from datetime import UTC from datetime import datetime +from enum import IntEnum from pathlib import Path from pathlib import PurePosixPath from unittest.mock import patch @@ -14,6 +15,7 @@ from spectrumx.client import Client from spectrumx.errors import FileError from spectrumx.models.files import File +from spectrumx.ops import files from spectrumx.ops.files import construct_file from spectrumx.ops.files import get_valid_files from spectrumx.ops.files import is_valid_file @@ -28,6 +30,17 @@ DRF_SAMPLE_RF_CHUNK_COUNT: int = 16 +class LogLevels(IntEnum): + """Log levels for testing.""" + + TRACE = 5 + DEBUG = 10 + INFO = 20 + WARNING = 30 + ERROR = 40 + CRITICAL = 50 + + def test_is_valid_file_allowed(temp_file_with_text_contents) -> None: """Test the file validation function.""" allowed_mime_types = [ @@ -507,6 +520,75 @@ def test_download_single_file( download_path.unlink(missing_ok=True) +@pytest.mark.integration +@pytest.mark.usefixtures("_integration_setup_teardown") +@pytest.mark.usefixtures("_without_responses") +@pytest.mark.parametrize( + "_without_responses", + argvalues=[ + [ + *PassthruEndpoints.authentication(), + *PassthruEndpoints.file_content_checks(), + *PassthruEndpoints.file_meta_download_or_upload(), + *PassthruEndpoints.file_content_download(), + ] + ], + indirect=True, +) +def test_download_fails_for_invalid_files( + caplog: pytest.LogCaptureFixture, + client: Client, +) -> None: + """Ensures download() fails when an invalid local path is provided.""" + + caplog.set_level(LogLevels.ERROR) + sds_path = PurePosixPath("sds/custom/dir/") + local_path = Path("local/path") + + target_num_files = 2 + original_list = files.generate_random_files(num_files=target_num_files) + + def _get_problematic_list_of_files(num_files: int = 10) -> list[File]: + """Generates a list of fabricated files with issues for download.""" + # num_files is ignored — using pre-generated file list instead + altered_list = [ + ofile.model_copy() for ofile in original_list[:target_num_files] + ] + + # set an invalid directory (not a child of local_path) + altered_list[0].directory = local_path / "../../../" / "invalid" + + # can't download a file with missing uuid for now + altered_list[1].uuid = None + + return altered_list + + # Patch the files.generate_random_files method to return the problematic list + with patch.object( + files, + attribute="generate_random_files", + side_effect=_get_problematic_list_of_files, + ): + results = client.download(from_sds_path=sds_path, to_local_path=local_path) + assert len(results) == target_num_files, "Three files should be generated" + + successful_files = [result() for result in results if result] + error_infos = [result.error_info for result in results if not result] + log.error( + f"File count: successful={len(successful_files)}, " + f"errored={len(error_infos)}" + ) + for success in successful_files: + log.info(f"{success.name}: {success.uuid}") + for error in error_infos: + # simplified: error_info includes file object for error metadata + assert isinstance(error, dict) + if error: + file_obj: File = error["file"] + log.error(f"{file_obj.name}: uuid={file_obj.uuid}") + assert not any(successful_files), "No file should be successfully downloaded." + + @pytest.mark.integration @pytest.mark.usefixtures("_integration_setup_teardown") @pytest.mark.usefixtures("_without_responses") diff --git a/sdk/tests/ops/test_datasets.py b/sdk/tests/ops/test_datasets.py index 380a2d678..6f195b8c3 100644 --- a/sdk/tests/ops/test_datasets.py +++ b/sdk/tests/ops/test_datasets.py @@ -256,3 +256,42 @@ def test_revoke_share_permissions_dry_run(client: Client) -> None: dataset_uuid = uuidlib.uuid4() result = client.datasets.revoke_share_permissions(dataset_uuid) assert result is True + + +def test_add_capture_to_dataset_forwards_uuids_and_verbose() -> None: + """Dataset API forwards attachment arguments to the gateway.""" + api = DatasetAPI( + gateway=GatewayClient(host="example.invalid", api_key="test-key"), + dry_run=False, + verbose=True, + ) + dataset_uuid = uuidlib.uuid4() + capture_uuid = uuidlib.uuid4() + + with patch.object( + api.gateway, + "add_capture_to_dataset", + return_value=b'{"message": "attached"}', + ) as add_capture: + result = api.add_capture_to_dataset(dataset_uuid, capture_uuid) + + assert result is True + add_capture.assert_called_once_with( + dataset_uuid=dataset_uuid, + capture_uuid=capture_uuid, + verbose=True, + ) + + +def test_add_capture_to_dataset_dry_run_skips_gateway() -> None: + """Dry-run attachment reports success without making an HTTP request.""" + api = DatasetAPI( + gateway=GatewayClient(host="example.invalid", api_key="test-key"), + dry_run=True, + ) + + with patch.object(api.gateway, "add_capture_to_dataset") as add_capture: + result = api.add_capture_to_dataset(uuidlib.uuid4(), uuidlib.uuid4()) + + assert result is True + add_capture.assert_not_called() diff --git a/sdk/tests/ops/test_files.py b/sdk/tests/ops/test_files.py index 3fc8d132d..5edfecd58 100644 --- a/sdk/tests/ops/test_files.py +++ b/sdk/tests/ops/test_files.py @@ -671,6 +671,7 @@ def test_delete_file_str_uuid( def test_delete_file_dry_run(client: Client, responses: responses.RequestsMock) -> None: """Test file deletion in dry run mode.""" + client.dry_run = True # ARRANGE test_uuid = uuidlib.uuid4() assert client.dry_run is True, "Dry run must be enabled for this test." @@ -1937,6 +1938,7 @@ def test_upload_new_file_metadata_only_dry_run( client: Client, ) -> None: """Dry-run in __upload_new_file_metadata_only logs and returns early.""" + client.dry_run = True file_instance = File( name="dry_meta.txt", media_type="text/plain", diff --git a/sdk/tests/test_client.py b/sdk/tests/test_client.py index b62ba3d17..694ea3c4d 100644 --- a/sdk/tests/test_client.py +++ b/sdk/tests/test_client.py @@ -21,6 +21,7 @@ from spectrumx.config import CFG_NAME_LOOKUP from spectrumx.config import SDSConfig from spectrumx.errors import CaptureError +from spectrumx.errors import DatasetError from spectrumx.errors import Result from spectrumx.errors import SDSError from spectrumx.gateway import API_TARGET_VERSION @@ -181,6 +182,8 @@ def test_warning_for_unrecognized_config( def test_http_timeout_default() -> None: """Default timeout must be 300 when set via Client with no env overrides.""" client = Client(host="sds-test.example.com") + # This test is invalid because the Client will always load .env when it exists + # so if it contains HTTP_TIMEOUT then it will override the default assert client.gateway.timeout == _EXPECTED_DEFAULT_TIMEOUT, ( f"Expected default gateway timeout {_EXPECTED_DEFAULT_TIMEOUT}, " f"got {client.gateway.timeout}" @@ -280,60 +283,6 @@ def test_download_dry_run_happy_path( ) -def test_download_fails_for_invalid_files( - caplog: pytest.LogCaptureFixture, - client: Client, -) -> None: - """Ensures download() fails when an invalid local path is provided.""" - - caplog.set_level(LogLevels.ERROR) - sds_path = PurePosixPath("sds/custom/dir/") - local_path = Path("local/path") - - target_num_files = 2 - original_list = files.generate_random_files(num_files=target_num_files) - - def _get_problematic_list_of_files(num_files: int = 10) -> list[File]: - """Generates a list of fabricated files with issues for download.""" - # num_files is ignored — using pre-generated file list instead - altered_list = [ - ofile.model_copy() for ofile in original_list[:target_num_files] - ] - - # set an invalid directory (not a child of local_path) - altered_list[0].directory = local_path / "../../../" / "invalid" - - # can't download a file with missing uuid for now - altered_list[1].uuid = None - - return altered_list - - # Patch the files.generate_random_files method to return the problematic list - with patch.object( - files, - attribute="generate_random_files", - side_effect=_get_problematic_list_of_files, - ): - results = client.download(from_sds_path=sds_path, to_local_path=local_path) - assert len(results) == target_num_files, "Three files should be generated" - - successful_files = [result() for result in results if result] - error_infos = [result.error_info for result in results if not result] - log.error( - f"File count: successful={len(successful_files)}, " - f"errored={len(error_infos)}" - ) - for success in successful_files: - log.info(f"{success.name}: {success.uuid}") - for error in error_infos: - # simplified: error_info includes file object for error metadata - assert isinstance(error, dict) - if error: - file_obj: File = error["file"] - log.error(f"{file_obj.name}: uuid={file_obj.uuid}") - assert not any(successful_files), "No file should be successfully downloaded." - - def test_existing_local_file_no_overwrite_skips_download( tmp_path: Path, client: Client ) -> None: @@ -695,6 +644,7 @@ def test_download_dataset_dry_run( uuid_arg: uuid.UUID | str, ) -> None: """download_dataset works in dry run mode (lines 588-652).""" + client.dry_run = True results = client.download_dataset( dataset_uuid=uuid_arg, to_local_path=tmp_path, @@ -767,21 +717,67 @@ def test_download_dataset_filter_branches( # ====================================================================== -def test_get_dataset_string_uuid(client: Client) -> None: +@responses.activate +def test_get_dataset_string_uuid( + client: Client, + responses: responses.RequestsMock, +) -> None: """get_dataset accepts a string UUID (lines 662-664).""" - ds = client.get_dataset(dataset_uuid=str(uuid.uuid4())) + client.dry_run = False + dataset_uuid = uuid.uuid4() + responses.add( + method=responses.GET, + url=get_datasets_endpoint(client, dataset_id=dataset_uuid.hex), + status=200, + json={"uuid": str(dataset_uuid)}, + ) + ds = client.get_dataset(dataset_uuid=str(dataset_uuid)) assert ds.uuid is not None -def test_list_dataset_captures_string_uuid(client: Client) -> None: +@responses.activate +def test_list_dataset_captures_string_uuid( + client: Client, + responses: responses.RequestsMock, +) -> None: """list_dataset_captures accepts a string UUID (lines 668-670).""" - result = client.list_dataset_captures(dataset_uuid=str(uuid.uuid4())) + client.dry_run = False + ds_uuid_obj = uuid.uuid4() + responses.add( + method=responses.GET, + url=get_datasets_endpoint(client, dataset_id=ds_uuid_obj.hex), + status=200, + json={ + "uuid": ds_uuid_obj.hex, + "name": "test-dataset", + "captures": [], + "files": [], + }, + ) + result = client.list_dataset_captures(dataset_uuid=str(ds_uuid_obj)) assert result == [] -def test_list_dataset_artifact_files_string_uuid(client: Client) -> None: +@responses.activate +def test_list_dataset_artifact_files_string_uuid( + client: Client, + responses: responses.RequestsMock, +) -> None: """list_dataset_artifact_files accepts a string UUID (lines 676-678).""" - result = client.list_dataset_artifact_files(dataset_uuid=str(uuid.uuid4())) + client.dry_run = False + ds_uuid_obj = uuid.uuid4() + responses.add( + method=responses.GET, + url=get_datasets_endpoint(client, dataset_id=ds_uuid_obj.hex), + status=200, + json={ + "uuid": ds_uuid_obj.hex, + "name": "test-dataset", + "captures": [], + "files": [], + }, + ) + result = client.list_dataset_artifact_files(dataset_uuid=str(ds_uuid_obj)) assert result == [] @@ -1034,6 +1030,7 @@ def test_issue_user_alerts_no_dry_run(caplog: pytest.LogCaptureFixture) -> None: def test_delete_file_dry_run() -> None: """delete_file in dry-run mode returns True (line 270).""" client = Client(host="sds-test.example.com") + client.dry_run = True assert client.dry_run result = client.delete_file(file_uuid=uuid.uuid4()) assert result is True @@ -1148,23 +1145,6 @@ def test_download_single_file_path_not_relative( assert "not relative to" in str(exc) -def test_download_single_file_skip_contents( - tmp_path: Path, - client: Client, -) -> None: - """download_single_file with skip_contents=True succeeds.""" - file_info = files.generate_sample_file(uuid.uuid4()) - file_info.directory = PurePosixPath("remote/dir") - - result = client.download_single_file( - file_info=file_info, - to_local_path=tmp_path, - skip_contents=True, - overwrite=False, - ) - assert result - - # ====================================================================== # Additional coverage: _download_files_fallback (lines 581-598) # ====================================================================== @@ -1248,6 +1228,7 @@ def test_download_files_fallback_failure_path( def test_list_files_dry_run(client: Client) -> None: """list_files in dry-run mode returns a Paginator with files (line 713).""" + client.dry_run = True paginator = client.list_files( sds_path=PurePosixPath("/some/path"), verbose=False, @@ -1364,6 +1345,54 @@ def test_get_dataset_with_uuid_object( assert ds.uuid is not None +def test_add_capture_to_dataset_normalizes_uuids_and_returns_refreshed_dataset( + client: Client, +) -> None: + """Public attachment normalizes strings and returns the refreshed dataset.""" + dataset_uuid = uuid.uuid4() + capture_uuid = uuid.uuid4() + refreshed_dataset = MagicMock() + + with ( + patch.object( + client.datasets, + "add_capture_to_dataset", + return_value=True, + ) as add_capture, + patch.object( + client.datasets, + "get", + return_value=refreshed_dataset, + ) as get_dataset, + ): + result = client.add_capture_to_dataset( + str(dataset_uuid), + str(capture_uuid), + ) + + assert result is refreshed_dataset + add_capture.assert_called_once_with(dataset_uuid, capture_uuid) + get_dataset.assert_called_once_with(dataset_uuid) + + +def test_add_capture_to_dataset_does_not_refresh_after_attachment_error( + client: Client, +) -> None: + """An attachment error propagates without issuing a misleading refresh.""" + with ( + patch.object( + client.datasets, + "add_capture_to_dataset", + side_effect=DatasetError("attachment failed"), + ), + patch.object(client.datasets, "get") as get_dataset, + pytest.raises(DatasetError, match="attachment failed"), + ): + client.add_capture_to_dataset(uuid.uuid4(), uuid.uuid4()) + + get_dataset.assert_not_called() + + @responses.activate def test_list_dataset_captures_non_dry_run( client: Client, @@ -1421,6 +1450,7 @@ def test_list_dataset_artifact_files_non_dry_run( def test_dataset_revoke_share_permissions_dry_run(client: Client) -> None: """revoke_share_permissions in dry-run mode returns True.""" + client.dry_run = True result = client.datasets.revoke_share_permissions(dataset_uuid=uuid.uuid4()) assert result is True @@ -1450,6 +1480,7 @@ def test_dataset_revoke_share_permissions_non_dry_run( def test_detach_file_from_datasets_dry_run(client: Client) -> None: """detach_file_from_datasets in dry-run mode returns True.""" + client.dry_run = True result = client.detach_file_from_datasets(file_uuid=uuid.uuid4()) assert result is True @@ -1809,20 +1840,55 @@ def test_download_dataset_dry_run_mkdir(tmp_path: Path, client: Client) -> None: assert len(results) == _DRY_RUN_FILE_COUNT -def test_list_dataset_captures_uuid_object(client: Client) -> None: +@responses.activate +def test_list_dataset_captures_uuid_object( + client: Client, + responses: responses.RequestsMock, +) -> None: """list_dataset_captures with UUID object (line 883 else branch).""" - result = client.list_dataset_captures(dataset_uuid=uuid.uuid4()) + client.dry_run = False + ds_uuid_obj = uuid.uuid4() + responses.add( + method=responses.GET, + url=get_datasets_endpoint(client, dataset_id=ds_uuid_obj.hex), + status=200, + json={ + "uuid": ds_uuid_obj.hex, + "name": "test-dataset", + "captures": [], + "files": [], + }, + ) + result = client.list_dataset_captures(dataset_uuid=ds_uuid_obj) assert result == [] -def test_list_dataset_artifact_files_uuid_object(client: Client) -> None: +@responses.activate +def test_list_dataset_artifact_files_uuid_object( + client: Client, + responses: responses.RequestsMock, +) -> None: """list_dataset_artifact_files with UUID object (line 891 else branch).""" - result = client.list_dataset_artifact_files(dataset_uuid=uuid.uuid4()) + client.dry_run = False + ds_uuid_obj = uuid.uuid4() + responses.add( + method=responses.GET, + url=get_datasets_endpoint(client, dataset_id=ds_uuid_obj.hex), + status=200, + json={ + "uuid": ds_uuid_obj.hex, + "name": "test-dataset", + "captures": [], + "files": [], + }, + ) + result = client.list_dataset_artifact_files(dataset_uuid=ds_uuid_obj) assert result == [] def test_upload_file_dry_run(client: Client) -> None: """upload_file in dry-run mode (line 1016).""" + client.dry_run = True file_info = files.generate_sample_file(uuid.uuid4()) result = client.upload_file(local_file=file_info, sds_path="/") assert result.uuid == file_info.uuid diff --git a/sdk/tests/test_gateway.py b/sdk/tests/test_gateway.py index e65e89977..11faf5f59 100644 --- a/sdk/tests/test_gateway.py +++ b/sdk/tests/test_gateway.py @@ -22,6 +22,7 @@ from loguru import logger as loguru_logger from pytz import UTC from spectrumx.errors import AuthError +from spectrumx.errors import DatasetError from spectrumx.errors import FileError from spectrumx.errors import NetworkError from spectrumx.gateway import Endpoints @@ -397,6 +398,65 @@ def test_get_dataset_files_with_artifacts_only() -> None: assert result == b'{"files": []}' +# --------------------------------------------------------------------------- +# add_capture_to_dataset +# --------------------------------------------------------------------------- + + +@responses.activate +def test_add_capture_to_dataset_posts_json_to_dataset_action() -> None: + gw = _make_gateway() + dataset_uuid = uuid.uuid4() + capture_uuid = uuid.uuid4() + url = ( + f"http://localhost:80/api/v1/assets/datasets/{dataset_uuid.hex}/attach-capture/" + ) + responses.add( + responses.POST, + url, + body=b'{"message": "Capture attached to Dataset."}', + status=200, + ) + + result = gw.add_capture_to_dataset( + dataset_uuid=dataset_uuid, + capture_uuid=capture_uuid, + ) + + assert result == b'{"message": "Capture attached to Dataset."}' + assert len(responses.calls) == 1 + request = responses.calls[0].request + assert request.method == "POST" + assert request.url == url + request_body = request.body + if isinstance(request_body, bytes): + request_body = request_body.decode() + capture_uuid_string = str(capture_uuid.hex) + assert request_body == f'{{"capture_uuid": "{capture_uuid_string}"}}' + + +@responses.activate +def test_add_capture_to_dataset_raises_dataset_error() -> None: + gw = _make_gateway() + dataset_uuid = uuid.uuid4() + capture_uuid = uuid.uuid4() + url = ( + f"http://localhost:80/api/v1/assets/datasets/{dataset_uuid.hex}/attach-capture/" + ) + responses.add( + responses.POST, + url, + json={"detail": "Capture not found."}, + status=404, + ) + + with pytest.raises(DatasetError, match="Capture not found"): + gw.add_capture_to_dataset( + dataset_uuid=dataset_uuid, + capture_uuid=capture_uuid, + ) + + # --------------------------------------------------------------------------- # upload_new_file # --------------------------------------------------------------------------- diff --git a/sdk/tests/test_usage.py b/sdk/tests/test_usage.py index 77632c97d..f078ad855 100644 --- a/sdk/tests/test_usage.py +++ b/sdk/tests/test_usage.py @@ -34,6 +34,14 @@ def get_auth_endpoint(client: Client) -> str: return client.base_url + f"/api/{API_TARGET_VERSION}/auth/" +def test_dry_run_enabled_by_default(client: Client) -> None: + """Dry-run mode must be enabled by default.""" + assert client.dry_run is True, ( + "Dry-run must be enabled by default. " + "Check your SDK .env file for 'DRY_RUN = false'" + ) + + # ------------------------ # TESTS FOR AUTHENTICATION # ------------------------ @@ -82,11 +90,6 @@ def test_dry_run_setter(client: Client) -> None: assert client.dry_run is True, "Dry-run setter failed." -def test_dry_run_enabled_by_default(client: Client) -> None: - """Dry-run mode must be enabled by default.""" - assert client.dry_run is True, "Dry-run must be enabled by default." - - def test_dry_auth_does_not_request( client: Client, responses_dry_run: RequestsMock ) -> None: