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 @@ -69,7 +69,11 @@ def generate_metadata( - deposit_client: str, name: str, external_id: Optional[str], authors: List[str] + deposit_client: str, + name: str, + authors: List[str], + external_id: Optional[str] = None, + create_origin: Optional[str] = None, ) -> str: """Generate sword compliant xml metadata with the minimum required metadata. @@ -93,8 +97,8 @@ Args: deposit_client: Deposit client username, name: Software name - external_id: External identifier (slug) or generated one authors: List of author names + create_origin: Origin concerned by the deposit Returns: metadata xml string @@ -119,6 +123,14 @@ if external_id: document["atom:entry"]["codemeta:identifier"] = external_id + if create_origin: + document["atom:entry"][ + "@xmlns:swh" + ] = "https://www.softwareheritage.org/schema/2018/deposit" + document["atom:entry"]["swh:create_origin"] = { + "swh:origin": {"@url": create_origin} + } + logging.debug("Atom entry dict to generate as xml: %s", document) return xmltodict.unparse(document, pretty=True) @@ -144,6 +156,7 @@ metadata: Optional[str], collection: Optional[str], slug: Optional[str], + create_origin: Optional[str], partial: bool, deposit_id: Optional[int], swhid: Optional[str], @@ -187,7 +200,7 @@ "username": username "metadata": the metadata file to deposit "collection": the user's collection under which to put the deposit - "slug": the slug or external id identifying the deposit to make + "create_origin": the origin concerned by the deposit "in_progress": if the deposit is partial or not "url": deposit's server main entry point "deposit_id": optional deposit identifier @@ -198,7 +211,9 @@ if name and authors: metadata_path = os.path.join(temp_dir, "metadata.xml") logging.debug("Temporary file: %s", metadata_path) - metadata_xml = generate_metadata(username, name, slug, authors) + metadata_xml = generate_metadata( + username, name, authors, external_id=slug, create_origin=create_origin + ) logging.debug("Metadata xml generated: %s", metadata_xml) with open(metadata_path, "w") as f: f.write(metadata_xml) @@ -224,10 +239,11 @@ # TODO: this is a multipart deposit, we might want to check that # metadata are deposited at some point pass - elif name or authors: + elif name or authors or create_origin: raise InputError( - "Using --metadata flag is incompatible with both " - "--author and --name (Those are used to generate one metadata file)." + "Using --metadata flag is incompatible with " + "--author and --name and --create-origin (those are used to generate one " + "metadata file)." ) if not archive and not metadata: @@ -325,10 +341,18 @@ @click.option( "--slug", help=( - "(Optional) External system information identifier. " + "(Deprecated) (Optional) External system information identifier. " "If not provided, it will be generated" ), ) +@click.option( + "--create-origin", + help=( + "(Optional) Origin url to attach information to. To be used alongside " + "--name and --author. This will be generated alongside the metadata to " + "provide to the deposit server." + ), +) @click.option( "--partial/--no-partial", default=False, @@ -372,6 +396,7 @@ metadata_deposit: bool, collection: Optional[str], slug: Optional[str], + create_origin: Optional[str], partial: bool, deposit_id: Optional[int], swhid: Optional[str], @@ -403,6 +428,20 @@ DeprecationWarning, ) + if slug: + if create_origin and slug != create_origin: + raise InputError( + '"--slug" flag has been deprecated in favor of "--create-origin" flag. ' + "You mentioned both with different values, please only " + 'use "--create-origin".' + ) + + warnings.warn( + '"--slug" flag has been deprecated in favor of "--create-origin" flag. ' + 'Please, start using "--create-origin" instead of "--slug"', + DeprecationWarning, + ) + url = _url(url) client = PublicApiDepositClient(url=url, auth=(username, password)) @@ -416,6 +455,7 @@ metadata, collection, slug, + create_origin, partial, deposit_id, swhid, @@ -429,7 +469,13 @@ if verbose: logger.info("Parsed configuration: %s", config) - keys = ["archive", "collection", "in_progress", "metadata", "slug"] + keys = [ + "archive", + "collection", + "in_progress", + "metadata", + "slug", + ] if config["deposit_id"]: keys += ["deposit_id", "replace", "swhid"] data = client.deposit_update(**_subdict(config, keys)) 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 @@ -16,12 +16,7 @@ from swh.deposit.api.checks import check_metadata from swh.deposit.cli import deposit as cli -from swh.deposit.cli.client import ( - InputError, - _collection, - _url, - generate_metadata, -) +from swh.deposit.cli.client import InputError, _collection, _url, generate_metadata from swh.deposit.client import MaintenanceError, PublicApiDepositClient from swh.deposit.parsers import parse_xml from swh.model.exceptions import ValidationError @@ -101,6 +96,37 @@ _collection(mock_client) +def test_cli_upload_conflictual_flags( + datadir, requests_mock_datadir, cli_runner, atom_dataset, tmp_path, +): + """Post metadata-only deposit through cli with invalid swhid raises + + """ + api_url_basename = "deposit.test.metadataonly" + metadata = atom_dataset["entry-data-minimal"] + metadata_path = os.path.join(tmp_path, "entry-data-minimal.xml") + with open(metadata_path, "w") as f: + f.write(metadata) + + with pytest.raises(InputError, match="both with different values"): + # fmt: off + cli_runner.invoke( + cli, + [ + "upload", + "--url", f"https://{api_url_basename}/1", + "--username", TEST_USER["username"], + "--password", TEST_USER["password"], + "--metadata", metadata_path, + "--slug", "some-slug", # deprecated flag + "--create-origin", "some-other-slug", # conflictual value, so raise + "--format", "json", + ], + catch_exceptions=False, + ) + # fmt: on + + def test_cli_deposit_with_server_down_for_maintenance( sample_archive, caplog, client_mock_api_down, slug, patched_tmp_path, cli_runner ): @@ -139,7 +165,11 @@ """ actual_metadata_xml = generate_metadata( - "deposit-client", "project-name", "external-id", authors=["some", "authors"] + "deposit-client", + "project-name", + authors=["some", "authors"], + external_id="external-id", + create_origin="origin-url", ) actual_metadata = dict(parse_xml(actual_metadata_xml)) @@ -152,6 +182,33 @@ OrderedDict([("codemeta:name", "some")]), OrderedDict([("codemeta:name", "authors")]), ] + assert actual_metadata["swh:create_origin"]["swh:origin"]["@url"] == "origin-url" + + checks_ok, detail = check_metadata(actual_metadata) + + assert checks_ok is True + assert detail is None + + +def test_cli_client_generate_metadata_ok2(slug): + """Generated metadata is well formed and pass service side metadata checks + + """ + actual_metadata_xml = generate_metadata( + "deposit-client", "project-name", authors=["some", "authors"], + ) + + actual_metadata = dict(parse_xml(actual_metadata_xml)) + assert actual_metadata["atom:author"] == "deposit-client" + assert actual_metadata["atom:title"] == "project-name" + assert actual_metadata["atom:updated"] is not None + assert actual_metadata["codemeta:name"] == "project-name" + assert actual_metadata["codemeta:author"] == [ + OrderedDict([("codemeta:name", "some")]), + OrderedDict([("codemeta:name", "authors")]), + ] + assert actual_metadata.get("codemeta:identifier") is None + assert actual_metadata.get("swh:create_origin") is None checks_ok, detail = check_metadata(actual_metadata) @@ -310,8 +367,9 @@ logging.ERROR, ( "Problem during parsing options: " - "Using --metadata flag is incompatible with both " - "--author and --name (Those are used to generate one metadata file)." + "Using --metadata flag is incompatible with --author " + "and --name and --create-origin (those are used to generate " + "one metadata file)." ), ) assert expected_error_log_record in caplog.record_tuples