Repository: SciCatProject/scitacean

Issue:
Support Sample
We already have the models. All that's missing are functions in the clients.

Discussion:

Patch:
diff --git a/docs/release-notes.rst b/docs/release-notes.rst
index 95f32bf..9685594 100644
--- a/docs/release-notes.rst
+++ b/docs/release-notes.rst
@@ -41,6 +41,8 @@ Security
 Features
 ~~~~~~~~
 
+* Added functions for downloading and uploading samples: :meth:`Client.get_sample`, :meth:`Client.upload_new_sample_now`.
+
 Breaking changes
 ~~~~~~~~~~~~~~~~
 
diff --git a/src/scitacean/__init__.py b/src/scitacean/__init__.py
index 6dd85f7..50eb539 100644
--- a/src/scitacean/__init__.py
+++ b/src/scitacean/__init__.py
@@ -16,7 +16,7 @@ from .dataset import Dataset
 from .error import FileUploadError, IntegrityError, ScicatCommError, ScicatLoginError
 from .file import File
 from .filesystem import RemotePath
-from .model import Attachment, DatasetType
+from .model import Attachment, DatasetType, Sample
 from .pid import PID
 from .thumbnail import Thumbnail
 from .warning import VisibleDeprecationWarning
@@ -32,6 +32,7 @@ __all__ = (
     "OrigDatablock",
     "PID",
     "RemotePath",
+    "Sample",
     "ScicatCommError",
     "ScicatLoginError",
     "Thumbnail",
diff --git a/src/scitacean/client.py b/src/scitacean/client.py
index 9d786d9..20f5f99 100644
--- a/src/scitacean/client.py
+++ b/src/scitacean/client.py
@@ -213,13 +213,38 @@ class Client:
             attachment_models=attachment_models,
         )
 
+    def get_sample(
+        self,
+        sample_id: str,
+        strict_validation: bool = False,
+    ) -> model.Sample:
+        """Download a sample from SciCat.
+
+        Parameters
+        ----------
+        sample_id:
+            ID of the sample.
+        strict_validation:
+            If ``True``, the sample must pass validation.
+            If ``False``, a sample is still returned if validation fails.
+            Note that some sample fields may have a bad value or type.
+            A warning will be logged if validation fails.
+
+        Returns
+        -------
+        :
+            The downloaded sample.
+        """
+        sample_model = self.scicat.get_sample_model(
+            sample_id, strict_validation=strict_validation
+        )
+        return model.Sample.from_download_model(sample_model)
+
     def upload_new_dataset_now(self, dataset: Dataset) -> Dataset:
         """Upload a dataset as a new entry to SciCat immediately.
 
         The dataset is inserted as a new entry in the database and will
         never overwrite existing data.
-        To this end, the input dataset's ID is ignored and a new one is
-        assigned automatically.
 
         Attachments are also uploaded automatically.
         This happens after the upload of files and the dataset itself.
@@ -278,6 +303,34 @@ class Client:
             attachment_models=finalized_attachments,
         )
 
+    def upload_new_sample_now(self, sample: model.Sample) -> model.Sample:
+        """Upload a sample as a new entry to SciCat immediately.
+
+        The sample is inserted as a new entry in the database and will
+        never overwrite existing data.
+        To this end, the input sample's ID is ignored and a new one is
+        assigned automatically.
+
+        Parameters
+        ----------
+        sample:
+            The sample to upload.
+
+        Returns
+        -------
+        :
+            A copy of the input sample with fields adjusted
+            according to the response of the server.
+
+        Raises
+        ------
+        scitacean.ScicatCommError
+            If the upload to SciCat fails.
+        """
+        sample = dataclasses.replace(sample, sample_id=None)
+        finalized_model = self.scicat.create_sample_model(sample.make_upload_model())
+        return model.Sample.from_download_model(finalized_model)
+
     def _upload_orig_datablocks(
         self, orig_datablocks: Optional[List[model.UploadOrigDatablock]]
     ) -> List[model.DownloadOrigDatablock]:
@@ -725,6 +778,47 @@ class ScicatClient:
             for attachment in attachment_json
         ]
 
+    def get_sample_model(
+        self, sample_id: str, strict_validation: bool = False
+    ) -> model.DownloadSample:
+        """Fetch a sample from SciCat.
+
+        Parameters
+        ----------
+        sample_id:
+            ID of the sample to fetch.
+        strict_validation:
+            If ``True``, the sample must pass validation.
+            If ``False``, a sample is still returned if validation fails.
+            Note that some fields may have a bad value or type.
+            A warning will be logged if validation fails.
+
+        Returns
+        -------
+        :
+            A model of the sample.
+
+        Raises
+        ------
+        scitacean.ScicatCommError
+            If the sample does not exist or communication fails for some other reason.
+        """
+        sample_json = self._call_endpoint(
+            cmd="get",
+            url=f"samples/{quote_plus(sample_id)}",
+            operation="get_sample_model",
+        )
+        if not sample_json:
+            raise ScicatCommError(
+                f"Cannot get sample with {sample_id=}, "
+                f"no such sample in SciCat at {self._base_url}."
+            )
+        return model.construct(
+            model.DownloadSample,
+            _strict_validation=strict_validation,
+            **sample_json,
+        )
+
     def create_dataset_model(
         self, dset: Union[model.UploadDerivedDataset, model.UploadRawDataset]
     ) -> model.DownloadDataset:
@@ -843,6 +937,43 @@ class ScicatClient:
             model.DownloadAttachment, _strict_validation=False, **uploaded
         )
 
+    def create_sample_model(self, sample: model.UploadSample) -> model.DownloadSample:
+        """Create a new sample in SciCat.
+
+        The sample ID must be either
+
+        - ``None``, in which case SciCat assigns an ID,
+        - an unused id, in which case SciCat uses it for the new sample.
+
+        If the ID already exists, creation will fail without
+        modification to the database.
+        Unless the new sample is identical to the existing one,
+        in which case, nothing happens.
+
+        Parameters
+        ----------
+        sample:
+            Model of the sample to create.
+
+        Returns
+        -------
+        :
+            The uploaded sample as returned by SciCat.
+            This is the input plus some modifications.
+
+        Raises
+        ------
+        scitacean.ScicatCommError
+            If SciCat refuses the sample or communication
+            fails for some other reason.
+        """
+        uploaded = self._call_endpoint(
+            cmd="post", url="samples", data=sample, operation="create_sample_model"
+        )
+        return model.construct(
+            model.DownloadSample, _strict_validation=False, **uploaded
+        )
+
     def validate_dataset_model(
         self, dset: Union[model.UploadDerivedDataset, model.UploadRawDataset]
     ) -> None:


Test patch:
diff --git a/src/scitacean/testing/client.py b/src/scitacean/testing/client.py
index 2452a6c..acad7d7 100644
--- a/src/scitacean/testing/client.py
+++ b/src/scitacean/testing/client.py
@@ -126,6 +126,7 @@ class FakeClient(Client):
         self.datasets: Dict[PID, model.DownloadDataset] = {}
         self.orig_datablocks: Dict[PID, List[model.DownloadOrigDatablock]] = {}
         self.attachments: Dict[PID, List[model.DownloadAttachment]] = {}
+        self.samples: Dict[str, model.DownloadSample] = {}
 
     @classmethod
     def from_token(
@@ -206,6 +207,17 @@ class FakeScicatClient(ScicatClient):
         _ = strict_validation  # unused by fake
         return self.main.attachments.get(pid) or []
 
+    @_conditionally_disabled
+    def get_sample_model(
+        self, sample_id: str, strict_validation: bool = False
+    ) -> model.DownloadSample:
+        """Fetch a sample from SciCat."""
+        _ = strict_validation  # unused by fake
+        try:
+            return self.main.samples[sample_id]
+        except KeyError:
+            raise ScicatCommError(f"Unable to retrieve sample {sample_id}") from None
+
     @_conditionally_disabled
     def create_dataset_model(
         self, dset: Union[model.UploadDerivedDataset, model.UploadRawDataset]
@@ -213,6 +225,11 @@ class FakeScicatClient(ScicatClient):
         """Create a new dataset in SciCat."""
         ingested = _process_dataset(dset)
         pid: PID = ingested.pid  # type: ignore[assignment]
+        if pid in self.main.datasets:
+            raise ScicatCommError(
+                f"Cannot create dataset with pid '{pid}' "
+                "because there already is a dataset with this pid."
+            )
         self.main.datasets[pid] = ingested
         return ingested
 
@@ -249,6 +266,19 @@ class FakeScicatClient(ScicatClient):
         self.main.attachments.setdefault(dataset_id, []).append(ingested)
         return ingested
 
+    @_conditionally_disabled
+    def create_sample_model(self, sample: model.UploadSample) -> model.DownloadSample:
+        """Create a new sample in SciCat."""
+        ingested = _process_sample(sample)
+        if ingested.sampleId in self.main.samples:
+            raise ScicatCommError(
+                f"Cannot create sample with id '{ingested.sampleId}' "
+                "because there already is a sample with this id."
+            )
+        sample_id: str = ingested.sampleId  # type: ignore[assignment]
+        self.main.samples[sample_id] = ingested
+        return ingested
+
     @_conditionally_disabled
     def validate_dataset_model(
         self, dset: Union[model.UploadDerivedDataset, model.UploadRawDataset]
@@ -335,13 +365,13 @@ def _process_attachment(
     attachment: model.UploadAttachment, dataset_id: Optional[PID] = None
 ) -> model.DownloadAttachment:
     created_at = datetime.datetime.now(tz=datetime.timezone.utc)
-    # Using strict_validation=False because the input model should already be validated.
-    # If there are validation errors, it was probably intended by the user.
     fields = _model_dict(attachment)
     if fields.get("id") is None:
-        fields["id"] = uuid.uuid4().hex
+        fields["id"] = str(uuid.uuid4())
     if dataset_id is not None:
         fields["datasetId"] = dataset_id
+    # Using strict_validation=False because the input model should already be validated.
+    # If there are validation errors, it was probably intended by the user.
     return model.construct(
         model.DownloadAttachment,
         _strict_validation=False,
@@ -353,6 +383,24 @@ def _process_attachment(
     )
 
 
+def _process_sample(sample: model.UploadSample) -> model.DownloadSample:
+    created_at = datetime.datetime.now(tz=datetime.timezone.utc)
+    fields = _model_dict(sample)
+    if fields.get("sampleId") is None:
+        fields["sampleId"] = str(uuid.uuid4())
+    # Using strict_validation=False because the input model should already be validated.
+    # If there are validation errors, it was probably intended by the user.
+    return model.construct(
+        model.DownloadSample,
+        _strict_validation=False,
+        createdBy="fake",
+        createdAt=created_at,
+        updatedBy="fake",
+        updatedAt=created_at,
+        **fields,
+    )
+
+
 def process_uploaded_dataset(
     dataset: Union[model.UploadDerivedDataset, model.UploadRawDataset],
     orig_datablocks: Optional[List[model.UploadOrigDatablock]],
diff --git a/tests/client/sample_client_test.py b/tests/client/sample_client_test.py
new file mode 100644
index 0000000..3ca59ba
--- /dev/null
+++ b/tests/client/sample_client_test.py
@@ -0,0 +1,82 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright (c) 2023 SciCat Project (https://github.com/SciCatProject/scitacean)
+
+import dataclasses
+
+import pytest
+
+from scitacean import Client, ScicatCommError
+from scitacean.client import ScicatClient
+from scitacean.model import (
+    Sample,
+)
+
+
+@pytest.fixture()
+def scicat_client(client: Client) -> ScicatClient:
+    return client.scicat
+
+
+@pytest.fixture()
+def sample(scicat_access):
+    return Sample(
+        owner_group=scicat_access.user.group,
+        access_groups=["group1", "2nd_group"],
+        description="A test sample for Scitacean",
+        owner="ridcully",
+        sample_characteristics={"layers": ["H2O", "EtOH"], "mass": 2},
+    )
+
+
+def compare_sample_model_after_upload(uploaded, downloaded):
+    for key, expected in uploaded:
+        # The database populates a number of fields that are None in uploaded.
+        # But we don't want to test those here as we don't want to test the database.
+        if expected is not None:
+            assert expected == dict(downloaded)[key], f"key = {key}"
+
+
+def compare_sample_after_upload(uploaded, downloaded):
+    for field in dataclasses.fields(uploaded):
+        # The database populates a number of fields that are None in uploaded.
+        # But we don't want to test those here as we don't want to test the database.
+        expected = getattr(uploaded, field.name)
+        if expected is not None:
+            assert expected == getattr(downloaded, field.name), f"key = {field.name}"
+
+
+def test_create_sample_model_roundtrip(scicat_client, sample):
+    upload_sample = sample.make_upload_model()
+    finalized = scicat_client.create_sample_model(upload_sample)
+    downloaded = scicat_client.get_sample_model(finalized.sampleId)
+    compare_sample_model_after_upload(upload_sample, downloaded)
+    compare_sample_model_after_upload(finalized, downloaded)
+
+
+def test_create_sample_model_roundtrip_existing_id(scicat_client, sample):
+    upload_sample = sample.make_upload_model()
+    finalized = scicat_client.create_sample_model(upload_sample)
+    upload_sample.sampleId = finalized.sampleId
+    # SciCat might accept an identical sample, so change it.
+    upload_sample.description = "A new sample with the same id"
+    with pytest.raises(ScicatCommError):
+        scicat_client.create_sample_model(upload_sample)
+
+
+def test_create_sample_model_populates_id(scicat_client, sample):
+    upload_sample = sample.make_upload_model()
+    finalized = scicat_client.create_sample_model(upload_sample)
+    downloaded = scicat_client.get_sample_model(finalized.sampleId)
+    assert finalized.sampleId is not None
+    assert downloaded.sampleId == finalized.sampleId
+
+
+def test_upload_sample_roundtrip(client, sample):
+    finalized = client.upload_new_sample_now(sample)
+    compare_sample_after_upload(sample, finalized)
+
+
+def test_upload_sample_overrides_id(client, sample):
+    sample.sample_id = "my_sample-id"
+    finalized = client.upload_new_sample_now(sample)
+    assert finalized.sample_id != sample.sample_id