diff --git a/swh/deposit/api/private/deposit_check.py b/swh/deposit/api/private/deposit_check.py --- a/swh/deposit/api/private/deposit_check.py +++ b/swh/deposit/api/private/deposit_check.py @@ -1,4 +1,4 @@ -# Copyright (C) 2017-2018 The Software Heritage developers +# Copyright (C) 2017-2019 The Software Heritage developers # See the AUTHORS file at the top-level directory of this distribution # License: GNU General Public License version 3, or any later version # See top-level LICENSE file for more information @@ -19,7 +19,6 @@ MANDATORY_FIELDS_MISSING = 'Mandatory fields are missing' ALTERNATE_FIELDS_MISSING = 'Mandatory alternate fields are missing' -INCOMPATIBLE_URL_FIELDS = "At least one url field must be compatible with the client's domain name" # noqa MANDATORY_ARCHIVE_UNREADABLE = 'At least one of its associated archives is not readable' # noqa MANDATORY_ARCHIVE_INVALID = 'Mandatory archive is invalid (i.e contains only one archive)' # noqa MANDATORY_ARCHIVE_UNSUPPORTED = 'Mandatory archive type is not supported' @@ -163,35 +162,6 @@ 'metadata': detail } - def _check_url(self, client_domain, metadata): - """Check compatibility between client_domain and url field in metadata - - Args: - client_domain (str): url associated with the deposit's client - metadata (dict): Metadata where to find url - - Returns: - tuple (status, error_detail): True, None if url associated - with the deposit's client is ok, (False, - ) otherwise. - - """ - url_fields = [] - for field in metadata: - if 'url' in field: - if client_domain in metadata[field]: - return True, None - url_fields.append(field) - - detail = { - 'url': { - 'summary': INCOMPATIBLE_URL_FIELDS, - } - } - if url_fields: - detail['url']['fields'] = url_fields - return False, detail - def process_get(self, req, collection_name, deposit_id): """Build a unique tarball from the multiple received and stream that content to the client. @@ -206,7 +176,6 @@ """ deposit = Deposit.objects.get(pk=deposit_id) - client_domain = deposit.client.domain metadata = self._metadata_get(deposit) problems = {} # will check each deposit's associated request (both of type @@ -219,11 +188,7 @@ if not metadata_status: problems.update(error_detail) - url_status, error_detail = self._check_url(client_domain, metadata) - if not url_status: - problems.update(error_detail) - - deposit_status = archives_status and metadata_status and url_status + deposit_status = archives_status and metadata_status # if any problems arose, the deposit is rejected if not deposit_status: diff --git a/swh/deposit/cli/deposit.py b/swh/deposit/cli/deposit.py --- a/swh/deposit/cli/deposit.py +++ b/swh/deposit/cli/deposit.py @@ -3,11 +3,13 @@ # License: GNU General Public License version 3, or any later version # See top-level LICENSE file for more information -import logging import os +import logging +import tempfile import uuid import click +import xmltodict from swh.deposit.client import PublicApiDepositClient from swh.deposit.cli import cli @@ -30,11 +32,69 @@ return str(uuid.uuid4()) +def generate_metadata_file(name, external_id, authors): + """Generate a temporary metadata file with the minimum required metadata + + This generates a xml file in a temporary location and returns the + path to that file. + + This is up to the client of that function to clean up the + temporary file. + + Args: + name (str): Software's name + external_id (str): External identifier (slug) or generated one + authors (List[str]): List of author names + + Returns: + Filepath to the metadata generated file + + """ + _, tmpfile = tempfile.mkstemp(prefix='swh.deposit.cli.') + + # generate a metadata file with the minimum required metadata + codemetadata = { + 'entry': { + '@xmlns': "http://www.w3.org/2005/Atom", + '@xmlns:codemeta': "https://doi.org/10.5063/SCHEMA/CODEMETA-2.0", + 'codemeta:name': name, + 'codemeta:identifier': external_id, + 'codemeta:author': [{ + 'codemeta:name': author_name + } for author_name in authors], + }, + } + + logging.debug('Temporary file: %s', tmpfile) + logging.debug('Metadata dict to generate as xml: %s', codemetadata) + s = xmltodict.unparse(codemetadata, pretty=True) + logging.debug('Metadata dict as xml generated: %s', s) + with open(tmpfile, 'w') as fp: + fp.write(s) + return tmpfile + + +def _cleanup_tempfile(config): + """Clean up the temporary metadata file generated. + + Args: + + config (Dict): A configuration dict with 2 important keys for + that routine, 'cleanup_tempfile' (bool) and 'metadata' (path + to eventually clean up) + + """ + if config['cleanup_tempfile']: + path = config['metadata'] + if os.path.exists(path): + os.unlink(path) + + def client_command_parse_input( username, password, archive, metadata, archive_deposit, metadata_deposit, collection, slug, partial, deposit_id, replace, - url, status): + url, status, name, authors): """Parse the client subcommand options and make sure the combination is acceptable*. If not, an InputError exception is raised explaining the issue. @@ -74,82 +134,98 @@ 'deposit_id': optional deposit identifier """ - if status and not deposit_id: - raise InputError("Deposit id must be provided for status check") - - if status and deposit_id: # status is higher priority over deposit - archive_deposit = False - metadata_deposit = False - archive = None - metadata = None - - if archive_deposit and metadata_deposit: - # too many flags use, remove redundant ones (-> multipart deposit) - archive_deposit = False - metadata_deposit = False - - if archive and not os.path.exists(archive): - raise InputError('Software Archive %s must exist!' % archive) - - if archive and not metadata: - metadata = '%s.metadata.xml' % archive - - if metadata_deposit: - archive = None - - if archive_deposit: - metadata = None - - if metadata_deposit and not metadata: - raise InputError( - "Metadata deposit filepath must be provided for metadata deposit") - - if metadata and not os.path.exists(metadata): - raise InputError('Software Archive metadata %s must exist!' % metadata) - - if not status and not archive and not metadata: - raise InputError( - 'Please provide an actionable command. See --help for more ' - 'information.') + cleanup_tempfile = False - if replace and not deposit_id: - raise InputError( - 'To update an existing deposit, you must provide its id') - - client = PublicApiDepositClient({ - 'url': url, - 'auth': { + try: + if status and not deposit_id: + raise InputError("Deposit id must be provided for status check") + + if status and deposit_id: # status is higher priority over deposit + archive_deposit = False + metadata_deposit = False + archive = None + metadata = None + + if archive_deposit and metadata_deposit: + # too many flags use, remove redundant ones (-> multipart deposit) + archive_deposit = False + metadata_deposit = False + + if archive and not os.path.exists(archive): + raise InputError('Software Archive %s must exist!' % archive) + + if not slug: # generate one as this is mandatory + slug = generate_slug() + + if archive and not metadata: # we need to have the metadata + if name and authors: + metadata = generate_metadata_file(name, slug, authors) + cleanup_tempfile = True + else: + raise InputError('Either metadata deposit file or (`--name` ' + ' and `--author`) fields must be provided') + + if metadata_deposit: + archive = None + + if archive_deposit: + metadata = None + + if metadata_deposit and not metadata: + raise InputError( + "Metadata deposit filepath must be provided for metadata " + "deposit") + + if metadata and not os.path.exists(metadata): + raise InputError('Software Archive metadata %s must exist!' % ( + metadata, )) + + if not status and not archive and not metadata: + raise InputError( + 'Please provide an actionable command. See --help for more ' + 'information.') + + if replace and not deposit_id: + raise InputError( + 'To update an existing deposit, you must provide its id') + + client = PublicApiDepositClient({ + 'url': url, + 'auth': { + 'username': username, + 'password': password + }, + }) + + if not collection: + # retrieve user's collection + sd_content = client.service_document() + if 'error' in sd_content: + raise InputError('Service document retrieval: %s' % ( + sd_content['error'], )) + collection = sd_content[ + 'service']['workspace']['collection']['sword:name'] + + return { + 'archive': archive, 'username': username, - 'password': password - }, - }) - - if not collection: - # retrieve user's collection - sd_content = client.service_document() - if 'error' in sd_content: - raise InputError('Service document retrieval: %s' % ( - sd_content['error'], )) - collection = sd_content[ - 'service']['workspace']['collection']['sword:name'] - - if not slug: - # generate slug - slug = generate_slug() - - return { - 'archive': archive, - 'username': username, - 'password': password, - 'metadata': metadata, - 'collection': collection, - 'slug': slug, - 'in_progress': partial, - 'client': client, - 'url': url, - 'deposit_id': deposit_id, - 'replace': replace, - } + 'password': password, + 'metadata': metadata, + 'cleanup_tempfile': cleanup_tempfile, + 'collection': collection, + 'slug': slug, + 'in_progress': partial, + 'client': client, + 'url': url, + 'deposit_id': deposit_id, + 'replace': replace, + } + except Exception: # to be clean, cleanup prior to raise + _cleanup_tempfile({ + 'cleanup_tempfile': cleanup_tempfile, + 'metadata': metadata + }) + raise def _subdict(d, keys): @@ -219,6 +295,11 @@ help="(Optional) Deposit's status") @click.option('--verbose/--no-verbose', default=False, help='Verbose mode') +@click.option('--name', + help='Software name') +@click.option('--author', multiple=True, + help='Software author(s), this can be repeated as many times' + ' as there are authors') @click.pass_context def deposit(ctx, username, password, archive=None, metadata=None, @@ -226,7 +307,7 @@ collection=None, slug=None, partial=False, deposit_id=None, replace=False, status=False, url='https://deposit.softwareheritage.org/1', - verbose=False): + verbose=False, name=None, author=None): """Software Heritage Public Deposit Client Create/Update deposit through the command line or access its @@ -243,8 +324,7 @@ config = client_command_parse_input( username, password, archive, metadata, archive_deposit, metadata_deposit, collection, slug, partial, deposit_id, - replace, url, status) - + replace, url, status, name, author) except InputError as e: msg = 'Problem during parsing options: %s' % e r = { @@ -253,17 +333,21 @@ logger.info(r) return 1 - if verbose: - logger.info("Parsed configuration: %s" % ( - config, )) + try: + if verbose: + logger.info("Parsed configuration: %s" % ( + config, )) + + deposit_id = config['deposit_id'] - deposit_id = config['deposit_id'] + if status and deposit_id: + r = deposit_status(config, logger) + elif not status and deposit_id: + r = deposit_update(config, logger) + elif not status and not deposit_id: + r = deposit_create(config, logger) - if status and deposit_id: - r = deposit_status(config, logger) - elif not status and deposit_id: - r = deposit_update(config, logger) - elif not status and not deposit_id: - r = deposit_create(config, logger) + logger.info(r) - logger.info(r) + finally: + _cleanup_tempfile(config) diff --git a/swh/deposit/tests/api/test_deposit_check.py b/swh/deposit/tests/api/test_deposit_check.py --- a/swh/deposit/tests/api/test_deposit_check.py +++ b/swh/deposit/tests/api/test_deposit_check.py @@ -16,7 +16,7 @@ ) from swh.deposit.api.private.deposit_check import ( SWHChecksDeposit, MANDATORY_ARCHIVE_INVALID, - MANDATORY_FIELDS_MISSING, INCOMPATIBLE_URL_FIELDS, + MANDATORY_FIELDS_MISSING, MANDATORY_ARCHIVE_UNSUPPORTED, ALTERNATE_FIELDS_MISSING, MANDATORY_ARCHIVE_MISSING ) @@ -141,8 +141,6 @@ alternate = details['metadata'][1] self.assertEqual(alternate['summary'], ALTERNATE_FIELDS_MISSING) self.assertEqual(alternate['fields'], ['name or title']) - # url check failure - self.assertEqual(details['url']['summary'], INCOMPATIBLE_URL_FIELDS) deposit = Deposit.objects.get(pk=deposit.id) self.assertEqual(deposit.status, DEPOSIT_STATUS_REJECTED)