diff --git a/swh/deposit/cli/client.py b/swh/deposit/cli/client.py
--- a/swh/deposit/cli/client.py
+++ b/swh/deposit/cli/client.py
@@ -141,6 +141,7 @@
slug: Optional[str],
partial: bool,
deposit_id: Optional[int],
+ swhid: Optional[str],
replace: bool,
url: str,
name: Optional[str],
@@ -188,6 +189,7 @@
'url': deposit's server main entry point
'deposit_type': deposit's type (binary, multipart, metadata)
'deposit_id': optional deposit identifier
+ 'swhid': optional deposit swhid
"""
if archive_deposit and metadata_deposit:
@@ -264,6 +266,7 @@
"client": client,
"url": url,
"deposit_id": deposit_id,
+ "swhid": swhid,
"replace": replace,
}
@@ -299,6 +302,7 @@
"slug",
"in_progress",
"replace",
+ "swhid",
)
return client.deposit_update(**_subdict(config, keys))
@@ -355,6 +359,11 @@
default=None,
help="(Optional) Update an existing partial deposit with its identifier",
) # noqa
+@click.option(
+ "--swhid",
+ default=None,
+ help="(Optional) Update existing completed deposit (status done) with new metadata",
+)
@click.option(
"--replace/--no-replace",
default=False,
@@ -397,6 +406,7 @@
slug: Optional[str] = None,
partial: bool = False,
deposit_id: Optional[int] = None,
+ swhid: Optional[str] = None,
replace: bool = False,
url: str = "https://deposit.softwareheritage.org",
verbose: bool = False,
@@ -433,6 +443,7 @@
slug,
partial,
deposit_id,
+ swhid,
replace,
url,
name,
diff --git a/swh/deposit/client.py b/swh/deposit/client.py
--- a/swh/deposit/client.py
+++ b/swh/deposit/client.py
@@ -11,7 +11,7 @@
import hashlib
import logging
import os
-from typing import Any, Dict
+from typing import Any, Dict, Optional
from urllib.parse import urljoin
import requests
@@ -22,6 +22,64 @@
logger = logging.getLogger(__name__)
+def compute_unified_information(
+ collection: str,
+ in_progress: bool,
+ slug: str,
+ *,
+ filepath: Optional[str] = None,
+ swhid: Optional[str] = None,
+ **kwargs,
+) -> Dict[str, Any]:
+ """Given a filepath, compute necessary information on that file.
+
+ Args:
+ collection: Deposit collection
+ in_progress: do we finalize the deposit?
+ slug: external id to use
+ filepath: Path to the file to compute the necessary information out of
+ swhid: Deposit swhid if any
+
+ Returns:
+ dict with keys:
+
+ 'slug': external id to use
+ 'in_progress': do we finalize the deposit?
+ 'content-type': content type associated
+ 'md5sum': md5 sum
+ 'filename': filename
+ 'filepath': filepath
+ 'swhid': deposit swhid
+
+ """
+ result: Dict[str, Any] = {
+ "slug": slug,
+ "in_progress": in_progress,
+ "swhid": swhid,
+ }
+ content_type: Optional[str] = None
+ md5sum: Optional[str] = None
+
+ if filepath:
+ filename = os.path.basename(filepath)
+ md5sum = hashlib.md5(open(filepath, "rb").read()).hexdigest()
+ extension = filename.split(".")[-1]
+ if "zip" in extension:
+ content_type = "application/zip"
+ else:
+ content_type = "application/x-tar"
+ result.update(
+ {
+ "content-type": content_type,
+ "md5sum": md5sum,
+ "filename": filename,
+ "filepath": filepath,
+ }
+ )
+
+ return result
+
+
class MaintenanceError(ValueError):
"""Informational maintenance error exception
@@ -243,7 +301,7 @@
"""
pass
- def compute_information(self, *args, **kwargs):
+ def compute_information(self, *args, **kwargs) -> Dict[str, Any]:
"""Compute some more information given the inputs (e.g http headers,
...)
@@ -415,50 +473,7 @@
],
)
- def _compute_information(
- self, collection, filepath, in_progress, slug, is_archive=True
- ):
- """Given a filepath, compute necessary information on that file.
-
- Args:
- filepath (str): Path to a file
- is_archive (bool): is it an archive or not?
-
- Returns:
- dict with keys:
- 'content-type': content type associated
- 'md5sum': md5 sum
- 'filename': filename
- """
- filename = os.path.basename(filepath)
-
- if is_archive:
- md5sum = hashlib.md5(open(filepath, "rb").read()).hexdigest()
- extension = filename.split(".")[-1]
- if "zip" in extension:
- content_type = "application/zip"
- else:
- content_type = "application/x-tar"
- else:
- content_type = None
- md5sum = None
-
- return {
- "slug": slug,
- "in_progress": in_progress,
- "content-type": content_type,
- "md5sum": md5sum,
- "filename": filename,
- "filepath": filepath,
- }
-
- def compute_information(
- self, collection, filepath, in_progress, slug, is_archive=True, **kwargs
- ):
- info = self._compute_information(
- collection, filepath, in_progress, slug, is_archive=is_archive
- )
- info["headers"] = self.compute_headers(info)
+ def compute_headers(self, info: Dict[str, Any]) -> Dict[str, Any]:
return info
def do_execute(self, method, url, info):
@@ -478,6 +493,13 @@
"CONTENT-DISPOSITION": "attachment; filename=%s" % (info["filename"],),
}
+ def compute_information(self, *args, **kwargs) -> Dict[str, Any]:
+ info = compute_unified_information(
+ *args, filepath=kwargs["archive_path"], **kwargs
+ )
+ info["headers"] = self.compute_headers(info)
+ return info
+
class UpdateArchiveDepositClient(CreateArchiveDepositClient):
"""Update (add/replace) an archive (binary) deposit client."""
@@ -499,17 +521,37 @@
"CONTENT-TYPE": "application/atom+xml;type=entry",
}
+ def compute_information(self, *args, **kwargs) -> Dict[str, Any]:
+ info = compute_unified_information(
+ *args, filepath=kwargs["metadata_path"], **kwargs
+ )
+ info["headers"] = self.compute_headers(info)
+ return info
+
-class UpdateMetadataDepositClient(CreateMetadataDepositClient):
- """Update (add/replace) a metadata deposit client."""
+class UpdateMetadataOnPartialDepositClient(CreateMetadataDepositClient):
+ """Update (add/replace) metadata on partial deposit scenario."""
def compute_url(self, collection, *args, deposit_id=None, **kwargs):
- return "/%s/%s/metadata/" % (collection, deposit_id)
+ return f"/{collection}/{deposit_id}/metadata/"
- def compute_method(self, *args, replace=False, **kwargs):
+ def compute_method(self, *args, replace: bool = False, **kwargs) -> str:
return "put" if replace else "post"
+class UpdateMetadataOnDoneDepositClient(UpdateMetadataOnPartialDepositClient):
+ """Update metadata on "done" deposit. This requires the deposit swhid."""
+
+ def compute_headers(self, info: Dict[str, Any]) -> Dict[str, Any]:
+ return {
+ "CONTENT-TYPE": "application/atom+xml;type=entry",
+ "X_CHECK_SWHID": info["swhid"],
+ }
+
+ def compute_method(self, *args, **kwargs) -> str:
+ return "put"
+
+
class CreateMultipartDepositClient(BaseCreateDepositClient):
"""Create a multipart deposit client."""
@@ -537,12 +579,10 @@
return files, headers
- def compute_information(
- self, collection, archive, metadata, in_progress, slug, **kwargs
- ):
- info = self._compute_information(collection, archive, in_progress, slug)
- info_meta = self._compute_information(
- collection, metadata, in_progress, slug, is_archive=False
+ def compute_information(self, *args, **kwargs) -> Dict[str, Any]:
+ info = compute_unified_information(*args, filepath=kwargs["archive_path"],)
+ info_meta = compute_unified_information(
+ *args, filepath=kwargs["metadata_path"],
)
files, headers = self._multipart_info(info, info_meta)
return {"files": files, "headers": headers}
@@ -568,36 +608,46 @@
"""Retrieve service document endpoint's information."""
return ServiceDocumentDepositClient(self.config).execute()
- def deposit_status(self, collection, deposit_id):
+ def deposit_status(self, collection: str, deposit_id: int):
"""Retrieve status information on a deposit."""
return StatusDepositClient(self.config).execute(collection, deposit_id)
def deposit_create(
- self, collection, slug, archive=None, metadata=None, in_progress=False
+ self,
+ collection: str,
+ slug: str,
+ archive: Optional[str] = None,
+ metadata: Optional[str] = None,
+ in_progress: bool = False,
):
"""Create a new deposit (archive, metadata, both as multipart)."""
if archive and not metadata:
return CreateArchiveDepositClient(self.config).execute(
- collection, archive, in_progress, slug
+ collection, in_progress, slug, archive_path=archive
)
elif not archive and metadata:
return CreateMetadataDepositClient(self.config).execute(
- collection, metadata, in_progress, slug, is_archive=False
+ collection, in_progress, slug, metadata_path=metadata
)
else:
return CreateMultipartDepositClient(self.config).execute(
- collection, archive, metadata, in_progress, slug
+ collection,
+ in_progress,
+ slug,
+ archive_path=archive,
+ metadata_path=metadata,
)
def deposit_update(
self,
- collection,
- deposit_id,
- slug,
- archive=None,
- metadata=None,
- in_progress=False,
- replace=False,
+ collection: str,
+ deposit_id: int,
+ slug: str,
+ archive: Optional[str] = None,
+ metadata: Optional[str] = None,
+ in_progress: bool = False,
+ replace: bool = False,
+ swhid: Optional[str] = None,
):
"""Update (add/replace) existing deposit (archive, metadata, both)."""
r = self.deposit_status(collection, deposit_id)
@@ -605,39 +655,55 @@
return r
status = r["deposit_status"]
- if status != "partial":
+ if swhid is None and status != "partial":
return {
"error": "You can only act on deposit with status 'partial'",
- "detail": "The deposit %s has status '%s'" % (deposit_id, status),
+ "detail": f"The deposit {deposit_id} has status '{status}'",
+ "deposit_status": status,
+ "deposit_id": deposit_id,
+ }
+ if swhid is not None and status != "done":
+ return {
+ "error": "You can only update metadata on deposit with status 'done'",
+ "detail": f"The deposit {deposit_id} has status '{status}'",
"deposit_status": status,
"deposit_id": deposit_id,
}
if archive and not metadata:
r = UpdateArchiveDepositClient(self.config).execute(
collection,
- archive,
in_progress,
slug,
deposit_id=deposit_id,
+ archive_path=archive,
replace=replace,
)
- elif not archive and metadata:
- r = UpdateMetadataDepositClient(self.config).execute(
+ elif not archive and metadata and swhid is None:
+ r = UpdateMetadataOnPartialDepositClient(self.config).execute(
collection,
- metadata,
in_progress,
slug,
deposit_id=deposit_id,
+ metadata_path=metadata,
replace=replace,
)
+ elif not archive and metadata and swhid is not None:
+ r = UpdateMetadataOnDoneDepositClient(self.config).execute(
+ collection,
+ in_progress,
+ slug,
+ deposit_id=deposit_id,
+ metadata_path=metadata,
+ swhid=swhid,
+ )
else:
r = UpdateMultipartDepositClient(self.config).execute(
collection,
- archive,
- metadata,
in_progress,
slug,
deposit_id=deposit_id,
+ archive_path=archive,
+ metadata_path=metadata,
replace=replace,
)
diff --git a/swh/deposit/tests/cli/test_client.py b/swh/deposit/tests/cli/test_client.py
--- a/swh/deposit/tests/cli/test_client.py
+++ b/swh/deposit/tests/cli/test_client.py
@@ -599,7 +599,7 @@
)
with open(deposit_status_xml_path, "r") as f:
deposit_status_xml = f.read()
- expected_deposit_dict = dict(parse_xml(deposit_status_xml))
+ expected_deposit_status = dict(parse_xml(deposit_status_xml))
result = cli_runner.invoke(
cli,
@@ -627,4 +627,97 @@
result_output = result.output
actual_deposit = callable_fn(result_output)
- assert actual_deposit == expected_deposit_dict
+ assert actual_deposit == expected_deposit_status
+
+
+def test_cli_update_metadata_with_swhid_on_completed_deposit(
+ datadir, requests_mock_datadir, cli_runner
+):
+ """Update new metadata on a completed deposit (status done) is ok
+ """
+ api_url_basename = "deposit.test.updateswhid"
+ deposit_id = 123
+ deposit_status_xml_path = os.path.join(
+ datadir, f"https_{api_url_basename}", f"1_test_{deposit_id}_status"
+ )
+ with open(deposit_status_xml_path, "r") as f:
+ deposit_status_xml = f.read()
+ expected_deposit_status = dict(parse_xml(deposit_status_xml))
+
+ assert expected_deposit_status["deposit_status"] == "done"
+ assert expected_deposit_status["deposit_swh_id"] is not None
+
+ result = cli_runner.invoke(
+ cli,
+ [
+ "upload",
+ "--url",
+ f"https://{api_url_basename}/1",
+ "--username",
+ TEST_USER["username"],
+ "--password",
+ TEST_USER["password"],
+ "--name",
+ "test-project",
+ "--author",
+ "John Doe",
+ "--deposit-id",
+ deposit_id,
+ "--swhid",
+ expected_deposit_status["deposit_swh_id"],
+ "--format",
+ "json",
+ ],
+ )
+ assert result.exit_code == 0, result.output
+ actual_deposit_status = json.loads(result.output)
+ assert "error" not in actual_deposit_status
+ assert actual_deposit_status == expected_deposit_status
+
+
+def test_cli_update_metadata_with_swhid_on_other_status_deposit(
+ datadir, requests_mock_datadir, cli_runner
+):
+ """Update new metadata with swhid on other deposit status is not possible
+ """
+ api_url_basename = "deposit.test.updateswhid"
+ deposit_id = 321
+ deposit_status_xml_path = os.path.join(
+ datadir, f"https_{api_url_basename}", f"1_test_{deposit_id}_status"
+ )
+ with open(deposit_status_xml_path, "r") as f:
+ deposit_status_xml = f.read()
+ expected_deposit_status = dict(parse_xml(deposit_status_xml))
+ assert expected_deposit_status["deposit_status"] != "done"
+
+ result = cli_runner.invoke(
+ cli,
+ [
+ "upload",
+ "--url",
+ f"https://{api_url_basename}/1",
+ "--username",
+ TEST_USER["username"],
+ "--password",
+ TEST_USER["password"],
+ "--name",
+ "test-project",
+ "--author",
+ "John Doe",
+ "--deposit-id",
+ deposit_id,
+ "--swhid",
+ "swh:1:dir:ef04a768181417fbc5eef4243e2507915f24deea",
+ "--format",
+ "json",
+ ],
+ )
+ assert result.exit_code == 0, result.output
+ actual_result = json.loads(result.output)
+ assert "error" in actual_result
+ assert actual_result == {
+ "error": "You can only update metadata on deposit with status 'done'",
+ "detail": "The deposit 321 has status 'partial'",
+ "deposit_status": "partial",
+ "deposit_id": 321,
+ }
diff --git a/swh/deposit/tests/data/https_deposit.test.updateswhid/1_servicedocument b/swh/deposit/tests/data/https_deposit.test.updateswhid/1_servicedocument
new file mode 100644
--- /dev/null
+++ b/swh/deposit/tests/data/https_deposit.test.updateswhid/1_servicedocument
@@ -0,0 +1,26 @@
+
+
+
+ 2.0
+ 209715200
+
+
+ The Software Heritage (SWH) Archive
+
+ test Software Collection
+ application/zip
+ application/x-tar
+ Collection Policy
+ Software Heritage Archive
+ Collect, Preserve, Share
+ false
+ false
+ http://purl.org/net/sword/package/SimpleZip
+ https://deposit.test.updateswhid/1/test/
+ test
+
+
+
diff --git a/swh/deposit/tests/data/https_deposit.test.updateswhid/1_test_123_metadata b/swh/deposit/tests/data/https_deposit.test.updateswhid/1_test_123_metadata
new file mode 100644
--- /dev/null
+++ b/swh/deposit/tests/data/https_deposit.test.updateswhid/1_test_123_metadata
@@ -0,0 +1,10 @@
+
+ 123
+ done
+ The deposit has been successfully loaded into the Software Heritage archive
+ swh:1:dir:ef04a768181417fbc5eef4243e2507915f24deea
+ swh:1:dir:ef04a768181417fbc5eef4243e2507915f24deea;origin=https://www.softwareheritage.org/check-deposit-2020-10-08T13:52:34.509655;visit=swh:1:snp:c477c6ef51833127b13a86ece7d75e5b3cc4e93d;anchor=swh:1:rev:f26f3960c175f15f6e24200171d446b86f6f7230;path=/
+ check-deposit-2020-10-08T13:52:34.509655
+
diff --git a/swh/deposit/tests/data/https_deposit.test.updateswhid/1_test_123_status b/swh/deposit/tests/data/https_deposit.test.updateswhid/1_test_123_status
new file mode 100644
--- /dev/null
+++ b/swh/deposit/tests/data/https_deposit.test.updateswhid/1_test_123_status
@@ -0,0 +1,10 @@
+
+ 123
+ done
+ The deposit has been successfully loaded into the Software Heritage archive
+ swh:1:dir:ef04a768181417fbc5eef4243e2507915f24deea
+ swh:1:dir:ef04a768181417fbc5eef4243e2507915f24deea;origin=https://www.softwareheritage.org/check-deposit-2020-10-08T13:52:34.509655;visit=swh:1:snp:c477c6ef51833127b13a86ece7d75e5b3cc4e93d;anchor=swh:1:rev:f26f3960c175f15f6e24200171d446b86f6f7230;path=/
+ check-deposit-2020-10-08T13:52:34.509655
+
diff --git a/swh/deposit/tests/data/https_deposit.test.updateswhid/1_test_321_status b/swh/deposit/tests/data/https_deposit.test.updateswhid/1_test_321_status
new file mode 100644
--- /dev/null
+++ b/swh/deposit/tests/data/https_deposit.test.updateswhid/1_test_321_status
@@ -0,0 +1,8 @@
+
+ 321
+ partial
+ The deposit is in partial state
+ check-deposit-2020-10-08T13:52:34.509655
+