diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index affd373..d52185c 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -14,8 +14,7 @@ jobs: strategy: fail-fast: false matrix: - # Matches the DSpace-ISstag-integration consumer CI matrix; the library - # itself declares support for >=3.8 (see setup.py). + # Matches the DSpace-ISstag-integration consumer CI matrix. python-version: ["3.10", "3.12"] steps: @@ -27,7 +26,7 @@ jobs: python-version: ${{ matrix.python-version }} cache: pip cache-dependency-path: | - setup.py + pyproject.toml requirements-test.txt - name: Install package + test deps @@ -38,3 +37,33 @@ jobs: - name: Run tests run: python -m pytest tests/ -v + + lint: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + + - name: Install uv + uses: astral-sh/setup-uv@v6 + + - name: Ruff + run: uvx ruff@0.12.7 check dspace_rest_client tests + + - name: Pylint + run: >- + uvx --with requests --with pysolr pylint@3.3.7 + --rcfile=pyproject.toml dspace_rest_client + + typecheck: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + + - name: Install uv + uses: astral-sh/setup-uv@v6 + + - name: Mypy + # Non-blocking: strict typing is being introduced incrementally, so a + # mypy failure is reported but does not fail the (green) job. + continue-on-error: true + run: uvx --with requests --with pysolr mypy dspace_rest_client diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000..de7c633 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,27 @@ +# Pre-commit hooks for local development. Mirrors the consumer repo +# (DSpace-ISstag-integration) so both repositories share one standard. +repos: + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v5.0.0 + hooks: + - id: check-json + - id: mixed-line-ending + args: ['--fix=lf'] + - id: check-added-large-files + - repo: https://github.com/hhatto/autopep8 + rev: v2.3.2 + hooks: + - id: autopep8 + args: ['-i', '--max-line-length=90', '--ignore=E402'] + - repo: https://github.com/astral-sh/ruff-pre-commit + rev: v0.12.7 + hooks: + - id: ruff + args: [--fix, --exit-non-zero-on-fix] + - repo: https://github.com/pylint-dev/pylint + rev: v3.3.7 + hooks: + - id: pylint + exclude: ^tests/ + args: ['-rn', '-sn', '--rcfile=pyproject.toml'] + additional_dependencies: [requests, pysolr] diff --git a/MAINTAINING.md b/MAINTAINING.md index 0efeed4..6036dab 100644 --- a/MAINTAINING.md +++ b/MAINTAINING.md @@ -7,12 +7,11 @@ These notes are for maintenance of the Git / PyPI source and releases, rather th All the tasks we need to do, in order, when releasing a new version: 1. - [ ] **Check the main branch!** - we should have all the changes we want to include merged/picked and tested -2. - [ ] **Update setup.py** - this might include other dependency or project description changes, but usually will just be a case of incrementing the version number, e.g. `0.1.9` -> `0.1.10`. Note the new number. -3. - [ ] **Update publish.sh** - this simple publish script performs the publish to PyPI and will need the new version number -4. - [ ] **Update CHANGELOG.md** - new versions go at the top of the file. See previous release blocks for formatting. I include a 'thanks' or 'reported by' attribution for PRs contributed or issues reported. The new version number from `setup.py` is used for the heading and the (future) PyPI URL +2. - [ ] **Update `project.version` in pyproject.toml** - this might include other dependency or project description changes, but usually will just be a case of incrementing the version number, e.g. `0.1.9` -> `0.1.10`. Note the new number. +3. - [ ] **Update CHANGELOG.md** - new versions go at the top of the file. See previous release blocks for formatting. I include a 'thanks' or 'reported by' attribution for PRs contributed or issues reported. The new version number from `pyproject.toml` is used for the heading and the (future) PyPI URL 4. - [ ] **Commit release preparation** - once you are happy with the steps above, commit with a message like 'Prepare release 0.1.10' 5. - [ ] **Push branch** - making sure github is up to date, (in future: CI) 6. - [ ] **Clear out build and dist directories**: OPTIONAL, but nice to start with a clean Python build environment before making this new version -7. - [ ] **Run publish script** - this will run `setup.py` to build a new version then upload to PyPI with twine - you will be prompted for credentials interactively +7. - [ ] **Run publish script** - this will run `python -m build` (needs `pip install build twine`) to build the sdist and wheel from `pyproject.toml`, check them and upload `dist/*` to PyPI with twine - you will be prompted for credentials interactively TODO: If we just keep a `version` file around some of these steps can be more easily automated or derived instead of updated by hand, but for now it's all pretty simple. \ No newline at end of file diff --git a/README.md b/README.md index 240cfc6..45980e0 100644 --- a/README.md +++ b/README.md @@ -10,8 +10,8 @@ Help with extending the scope and improving the code is always welcome! PyPI homepage: https://pypi.org/project/dspace-rest-client/ ## Requirements -* Python 3.x (developed using Python 3.8.5) -* Python Requests module (see `requirements.txt`) +* Python 3.8+ +* Python Requests module (installed automatically; declared in `pyproject.toml`) * Working DSpace 7 repository with an accessible REST API ## Installation @@ -20,10 +20,11 @@ To install with pip: (or `pip3` or `python -m pip` as appropriate to your environment) -To install manually, clone this repository and install the requirements: +To install manually, clone this repository and install the package: ```commandline git clone https://github.com/the-library-code/dspace-rest-python.git -pip install -r requirements.txt +cd dspace-rest-python +pip install . ``` diff --git a/dspace_rest_client/__init__.py b/dspace_rest_client/__init__.py index b6e690f..b955bbc 100644 --- a/dspace_rest_client/__init__.py +++ b/dspace_rest_client/__init__.py @@ -1 +1,61 @@ -from . import * +"""DSpace 7 REST API client.""" + +from typing import TYPE_CHECKING, Any + +from .models import ( + HALResource, + AddressableHALResource, + ExternalDataObject, + DSpaceObject, + SimpleDSpaceObject, + Item, + Community, + Collection, + Bundle, + Bitstream, + Group, + User, + InProgressSubmission, + WorkspaceItem, + EntityType, + RelationshipType, + License, + Label, + ResourcePolicy, +) + +if TYPE_CHECKING: + from .client import DSpaceClient + +__all__ = [ + "DSpaceClient", + "HALResource", + "AddressableHALResource", + "ExternalDataObject", + "DSpaceObject", + "SimpleDSpaceObject", + "Item", + "Community", + "Collection", + "Bundle", + "Bitstream", + "Group", + "User", + "InProgressSubmission", + "WorkspaceItem", + "EntityType", + "RelationshipType", + "License", + "Label", + "ResourcePolicy", +] + + +def __getattr__(name: str) -> Any: + # client reads its env-based class defaults (DSPACE_API_ENDPOINT, PROXY_URL, + # ...) at import time, so it is imported on first use, never as a side + # effect of importing the package or one of its submodules + if name == "DSpaceClient": + from .client import DSpaceClient # pylint: disable=import-outside-toplevel + return DSpaceClient + raise AttributeError(f"module {__name__!r} has no attribute {name!r}") diff --git a/dspace_rest_client/client.py b/dspace_rest_client/client.py index 8a53b57..0777548 100644 --- a/dspace_rest_client/client.py +++ b/dspace_rest_client/client.py @@ -14,18 +14,41 @@ @author Kim Shepherd """ -import json +from __future__ import annotations + +import json as _json import logging import os +from typing import Any, Optional from urllib.parse import urlparse from uuid import UUID import requests from requests import Request -from .models import * - -__all__ = ['DSpaceClient'] +from .models import ( + Bitstream, + Bundle, + Collection, + Community, + DSpaceObject, + ExternalDataObject, + Group, + HALResource, + Item, + ResourcePolicy, + SimpleDSpaceObject, + User, +) + +# the models are re-exported: `from .models import *` used to make them +# importable from here, and callers rely on it +__all__ = ['DSpaceClient', 'Bitstream', 'Bundle', 'Collection', 'Community', + 'DSpaceObject', 'ExternalDataObject', 'Group', 'HALResource', 'Item', + 'ResourcePolicy', 'SimpleDSpaceObject', 'User'] + +# default of DSpaceClient(proxies=...): use PROXY_DICT; distinct from None +_PROXY_DICT_DEFAULT: Any = object() # cap on the response body echoed into an error log line _ERR_BODY_MAX = 500 @@ -39,7 +62,7 @@ _logger.addHandler(logging.NullHandler()) -def parse_json(response): +def parse_json(response) -> Any: """ Simple static method to handle ValueError if JSON is invalid in response body @param response: the http response object (which should contain JSON) @@ -57,6 +80,15 @@ def parse_json(response): return response_json +def _is_valid_uuid(value: str) -> bool: + """Return True if `value` is a well-formed UUID string, else False.""" + try: + UUID(str(value)) + return True + except ValueError: + return False + + class DSpaceClient: """ Main class of the API client itself. This client uses request sessions to connect and authenticate to @@ -91,7 +123,10 @@ class DSpaceClient: # Default per-request timeout in seconds so a stalled server cannot hang the # client forever; override via the `timeout` constructor argument. DEFAULT_TIMEOUT = 60 - PROXY_DICT = dict(http=os.environ["PROXY_URL"],https=os.environ["PROXY_URL"]) if "PROXY_URL" in os.environ else dict() + PROXY_DICT = ( + {"http": os.environ["PROXY_URL"], "https": os.environ["PROXY_URL"]} + if "PROXY_URL" in os.environ else {} + ) # Simple enum for patch operation types class PatchOperation: @@ -100,8 +135,11 @@ class PatchOperation: REPLACE = 'replace' MOVE = 'move' - def __init__(self, api_endpoint=API_ENDPOINT, username=USERNAME, password=PASSWORD, solr_endpoint=SOLR_ENDPOINT, - solr_auth=SOLR_AUTH, fake_user_agent=False, proxies=PROXY_DICT, timeout=None): + def __init__(self, api_endpoint: str = API_ENDPOINT, username: str = USERNAME, + password: str = PASSWORD, solr_endpoint: str = SOLR_ENDPOINT, + solr_auth=SOLR_AUTH, fake_user_agent: bool = False, + proxies: Any = _PROXY_DICT_DEFAULT, + timeout: Optional[int] = None) -> None: """ Accept optional API endpoint, username, password arguments using the OS environment variables as defaults :param api_endpoint: base path to DSpace REST API, eg. http://localhost:8080/server/api @@ -117,7 +155,11 @@ def __init__(self, api_endpoint=API_ENDPOINT, username=USERNAME, password=PASSWO self.USERNAME = username self.PASSWORD = password self.SOLR_ENDPOINT = solr_endpoint - self.proxies = proxies + # Copy the class-level default so per-instance mutation of `proxies` + # never leaks into other default-constructed clients. An explicit None + # is kept: requests then resolves proxies itself. + self.proxies = (dict(self.PROXY_DICT) if proxies is _PROXY_DICT_DEFAULT + else proxies) self.solr = None self._last_err = None self.timeout = timeout if timeout is not None else self.DEFAULT_TIMEOUT @@ -143,7 +185,7 @@ def __init__(self, api_endpoint=API_ENDPOINT, username=USERNAME, password=PASSWO def last_err(self): return self._last_err - def authenticate(self, retry=False): + def authenticate(self, retry: bool = False) -> bool: """ Authenticate with the DSpace REST API. As with other operations, perform XSRF refreshes when necessary. After POST, check /authn/status and log success if the authenticated json property is true @@ -164,9 +206,8 @@ def authenticate(self, retry=False): if retry: _logger.error(f'Too many retries updating token: {r.status_code}: {r.text}') return False - else: - _logger.debug("Retrying request with updated CSRF token") - return self.authenticate(retry=True) + _logger.debug("Retrying request with updated CSRF token") + return self.authenticate(retry=True) if r.status_code == 401: # 401 Unauthorized @@ -190,7 +231,7 @@ def authenticate(self, retry=False): # Default, return false return False - def verify_response(self, r, id_str: str, as_json: bool = False): + def verify_response(self, r, id_str: str, as_json: bool = False) -> bool: """ Verify response from API. If response is not 200, log error and return False. """ @@ -209,7 +250,7 @@ def verify_response(self, r, id_str: str, as_json: bool = False): return True - def refresh_token(self): + def refresh_token(self) -> None: """ If the DSPACE-XSRF-TOKEN appears, we need to update our local stored token and re-send our API request @return: None @@ -217,7 +258,8 @@ def refresh_token(self): r = self.api_post(self.LOGIN_URL, None, None) self.update_token(r) - def api_get(self, url, params=None, data=None, headers=None): + def api_get(self, url: str, params=None, data=None, + headers=None) -> requests.Response: """ Perform a GET request. Refresh XSRF token if necessary. @param url: DSpace REST API URL @@ -234,7 +276,8 @@ def api_get(self, url, params=None, data=None, headers=None): self.update_token(r) return r - def api_post(self, url, params, json, retry=False, timeout=None): + def api_post(self, url: str, params, json: Any, retry: bool = False, + timeout=None) -> requests.Response: """ Perform a POST request. Refresh XSRF token if necessary. POSTs are typically used to create objects. @@ -279,7 +322,8 @@ def api_post(self, url, params, json, retry=False, timeout=None): return self.api_post(url, params=params, json=json, retry=True, timeout=timeout) return r - def api_post_uri(self, url, params, uri_list, retry=False): + def api_post_uri(self, url: str, params, uri_list, + retry: bool = False) -> requests.Response: """ Perform a POST request. Refresh XSRF token if necessary. POSTs are typically used to create objects. @@ -309,7 +353,8 @@ def api_post_uri(self, url, params, uri_list, retry=False): return r - def api_put(self, url, params, json, retry=False): + def api_put(self, url: str, params, json: Any, + retry: bool = False) -> requests.Response: """ Perform a PUT request. Refresh XSRF token if necessary. PUTs are typically used to update objects. @@ -341,7 +386,8 @@ def api_put(self, url, params, json, retry=False): return r - def api_put_uri(self, url, params, uri_list, retry=False): + def api_put_uri(self, url: str, params, uri_list, + retry: bool = False) -> requests.Response: """ Perform a PUT request. Refresh XSRF token if necessary. PUTs are typically used to update objects. @@ -373,7 +419,7 @@ def api_put_uri(self, url, params, uri_list, retry=False): return r - def api_delete(self, url, params, retry=False): + def api_delete(self, url: str, params, retry: bool = False) -> requests.Response: """ Perform a DELETE request. Refresh XSRF token if necessary. DELETES are typically used to update objects. @@ -404,7 +450,8 @@ def api_delete(self, url, params, retry=False): return r - def api_patch(self, url, operation, path, value, params=None, retry=False): + def api_patch(self, url: str, operation, path, value, params=None, + retry: bool = False) -> Optional[requests.Response]: """ @param url: DSpace REST API URL @param operation: 'add', 'remove', 'replace', or 'move' (see PatchOperation enumeration) @@ -422,8 +469,8 @@ def api_patch(self, url, operation, path, value, params=None, retry=False): if path is None: _logger.error('Need valid path eg. /withdrawn or /metadata/dc.title/0/language') return None - if (operation == self.PatchOperation.ADD or operation == self.PatchOperation.REPLACE - or operation == self.PatchOperation.MOVE) and value is None: + if operation in (self.PatchOperation.ADD, self.PatchOperation.REPLACE, + self.PatchOperation.MOVE) and value is None: # missing value required for add/replace/move operations _logger.error('Missing required "value" argument for add/replace/move operations') return None @@ -466,7 +513,8 @@ def api_patch(self, url, operation, path, value, params=None, retry=False): return r # PAGINATION - def search_objects(self, query=None, scope=None, filters=None, page=0, size=20, sort=None, dso_type=None, details=None): + def search_objects(self, query=None, scope=None, filters=None, page: int = 0, + size: int = 20, sort=None, dso_type=None, details=None) -> list: """ Do a basic search with optional query, filters and dsoType params. @param query: query string @@ -513,7 +561,7 @@ def search_objects(self, query=None, scope=None, filters=None, page=0, size=20, return dsos - def fetch_resource(self, url, params=None): + def fetch_resource(self, url: str, params=None) -> Any: """ Simple function for higher-level 'get' functions to use whenever they want to retrieve JSON resources from the API @@ -590,7 +638,8 @@ def _fetch_all_pages(self, url, params, key, what): f'[{url}]') return resources - def get_resourcepolicy(self, uuid, action='READ'): + def get_resourcepolicy(self, uuid: str, + action: Optional[str] = 'READ') -> Optional[list]: """ Fetch resource policies for a given resource UUID and action. @param uuid: resource UUID to search for @@ -599,7 +648,7 @@ def get_resourcepolicy(self, uuid, action='READ'): """ try: # Validate UUID - id = UUID(uuid).version + UUID(uuid) url = f'{self.API_ENDPOINT}/authz/resourcepolicies/search/resource' params = {'uuid': uuid} if action is not None: @@ -616,9 +665,9 @@ def get_resourcepolicy(self, uuid, action='READ'): return None def create_resourcepolicy( - self, resource_uuid, group_uuid, action='READ', + self, resource_uuid: str, group_uuid: str, action: str = 'READ', start_date=None, end_date=None, - ): + ) -> Optional[ResourcePolicy]: """ Create a new resource policy for a given DSpace resource. Uses POST /api/authz/resourcepolicies?resource=&group= @@ -660,7 +709,7 @@ def create_resourcepolicy( f'Failed to create resource policy: {r.status_code}: {r.text}') return None - def get_dso(self, url, uuid): + def get_dso(self, url: str, uuid: str) -> Optional[requests.Response]: """ Base 'get DSpace Object' function. Uses fetch_resource which itself calls parse_json on the raw response before returning. @@ -670,14 +719,14 @@ def get_dso(self, url, uuid): """ try: # Try to get UUID version to test validity - id = UUID(uuid).version + UUID(uuid) url = f'{url}/{uuid}' return self.api_get(url, None, None) except ValueError: _logger.error(f'Invalid DSO UUID: {uuid}') return None - def create_dso(self, url, params, data): + def create_dso(self, url: str, params, data) -> requests.Response: """ Base 'create DSpace Object' function. Takes JSON data and some POST parameters and returns the response. @@ -696,7 +745,7 @@ def create_dso(self, url, params, data): _logger.error(f'create operation failed: {r.status_code}: {r.text} ({url})') return r - def update_dso(self, dso, params=None): + def update_dso(self, dso, params=None) -> Optional[DSpaceObject]: """ Update DSpaceObject. Takes a DSpaceObject and any optional parameters. Will send a PUT update to the remote object and return the updated object, typed correctly. @@ -720,27 +769,24 @@ def update_dso(self, dso, params=None): if 'lastModified' in data: data.pop('lastModified') - """ - if 'id' in data: - data.pop('id') - if 'handle' in data: - data.pop('handle') - if 'uuid' in data: - data.pop('uuid') - if 'type' in data: - data.pop('type') - """ + # if 'id' in data: + # data.pop('id') + # if 'handle' in data: + # data.pop('handle') + # if 'uuid' in data: + # data.pop('uuid') + # if 'type' in data: + # data.pop('type') r = self.api_put(url, params=params, json=data) if r.status_code == 200: # 200 OK - success! updated_dso = dso_type(parse_json(r)) _logger.debug(f'{updated_dso.type} {updated_dso.uuid} updated successfully!') return updated_dso - else: - _logger.error(f'update operation failed: {r.status_code}: {r.text} ({url})') - return None + _logger.error(f'update operation failed: {r.status_code}: {r.text} ({url})') + return None - except ValueError as e: + except ValueError: _logger.error("Error parsing DSO response", exc_info=True) return None @@ -772,15 +818,15 @@ def delete_dso(self, dso=None, url=None, params=None): # 204 No Content - success! _logger.info(f'{url} was deleted successfully!') return r - else: - _logger.error(f'update operation failed: {r.status_code}: {r.text} ({url})') - return None + _logger.error(f'update operation failed: {r.status_code}: {r.text} ({url})') + return None except ValueError as e: _logger.error(f'Error deleting DSO {dso.uuid}: {e}') return None # PAGINATION - def get_bundles(self, parent=None, uuid=None, page=0, size=20, sort=None): + def get_bundles(self, parent=None, uuid=None, page: int = 0, size: int = 20, + sort=None) -> list: """ Get bundles for an item @param parent: python Item object, from which the UUID will be referenced in the URL. @@ -790,7 +836,7 @@ def get_bundles(self, parent=None, uuid=None, page=0, size=20, sort=None): """ # TODO: It is probably wise to allow the parent UUID to be simply passed as an alternative to having the full # python object as constructed by this REST client, for more flexible usage. - bundles = list() + bundles = [] single_result = False if uuid is not None: url = f'{self.API_ENDPOINT}/core/bundles/{uuid}' @@ -798,7 +844,7 @@ def get_bundles(self, parent=None, uuid=None, page=0, size=20, sort=None): elif parent is not None: url = f'{self.API_ENDPOINT}/core/items/{parent.uuid}/bundles' else: - return list() + return [] params = {} if size is not None: params['size'] = size @@ -832,7 +878,7 @@ def get_bundles(self, parent=None, uuid=None, page=0, size=20, sort=None): return bundles - def create_bundle(self, parent=None, name='ORIGINAL'): + def create_bundle(self, parent=None, name: str = 'ORIGINAL') -> Optional[Bundle]: """ Create new bundle in the specified item @param parent: Parent python Item, the UUID of which will be used in the URL path @@ -854,7 +900,8 @@ def create_bundle(self, parent=None, name='ORIGINAL'): return Bundle(api_resource=parse_json(r)) # PAGINATION - def get_bitstreams(self, uuid=None, bundle=None, page=0, size=20, sort=None): + def get_bitstreams(self, uuid=None, bundle=None, page: int = 0, size: int = 20, + sort=None) -> list: """ Get a specific bitstream UUID, or all bitstreams for a specific bundle @param uuid: UUID of a specific bitstream to retrieve @@ -865,7 +912,7 @@ def get_bitstreams(self, uuid=None, bundle=None, page=0, size=20, sort=None): """ url = f'{self.API_ENDPOINT}/core/bitstreams/{uuid}' if uuid is None and bundle is None: - return list() + return [] if uuid is None and isinstance(bundle, Bundle): if 'bitstreams' in bundle.links: url = bundle.links['bitstreams']['href'] @@ -887,13 +934,15 @@ def get_bitstreams(self, uuid=None, bundle=None, page=0, size=20, sort=None): # the bundle (or item) is gone - no bitstreams, a clean empty # result rather than a crash. Mirrors get_bundles. _logger.info(f'No bitstreams: resource not found (404) [{url}]') - return list() + return [] # a transient 5xx must NOT masquerade as "no bitstreams"; surface it # with status + url so the caller can retry, not an opaque TypeError. raise RuntimeError(f'Failed to fetch bitstreams: HTTP {status} [{url}]') return [Bitstream(bitstream_resource) for bitstream_resource in resources] - def create_bitstream(self, bundle=None, name=None, path=None, mime=None, metadata=None, retry=False, timeout=None): + def create_bitstream(self, bundle=None, name=None, path=None, mime=None, + metadata=None, retry: bool = False, + timeout=None) -> Optional[Bitstream]: """ Upload a file and create a bitstream for a specified parent bundle, from the uploaded file and the supplied metadata. @@ -925,7 +974,7 @@ def create_bitstream(self, bundle=None, name=None, path=None, mime=None, metadat with open(path, 'rb') as fh: files = {'file': (name, fh, mime)} properties = {'name': name, 'metadata': metadata, 'bundleName': bundle.name} - payload = {'properties': json.dumps(properties) + ';application/json'} + payload = {'properties': _json.dumps(properties) + ';application/json'} # copy the session headers so this request's Content-Encoding does # not leak onto every subsequent request (and across threads) h = dict(self.session.headers) @@ -936,7 +985,7 @@ def create_bitstream(self, bundle=None, name=None, path=None, mime=None, metadat timeout=timeout if timeout is not None else self.timeout) if 'DSPACE-XSRF-TOKEN' in r.headers: t = r.headers['DSPACE-XSRF-TOKEN'] - _logger.debug('Updating token to ' + t) + _logger.debug(f'Updating token to {t}') self.session.headers.update({'X-XSRF-Token': t}) self.session.cookies.update({'X-XSRF-Token': t}) if not retry and r.status_code in (401, 403): @@ -948,14 +997,13 @@ def create_bitstream(self, bundle=None, name=None, path=None, mime=None, metadat return self.create_bitstream(bundle=bundle, name=name, path=path, mime=mime, metadata=metadata, retry=True, timeout=timeout) - if r.status_code == 201 or r.status_code == 200: + if r.status_code in (201, 200): # Success return Bitstream(api_resource=parse_json(r)) - else: - _logger.error(f'Error creating bitstream: {r.status_code}: {r.text}') - return None + _logger.error(f'Error creating bitstream: {r.status_code}: {r.text}') + return None - def download_bitstream(self, uuid=None): + def download_bitstream(self, uuid=None) -> Optional[requests.Response]: """ Download bitstream and return full response object including headers, and content @param uuid: @@ -966,9 +1014,11 @@ def download_bitstream(self, uuid=None): r = self.api_get(url, headers=h) if r.status_code == 200: return r + return None # PAGINATION - def get_communities(self, uuid=None, page=0, size=20, sort=None, top=False): + def get_communities(self, uuid: Optional[str] = None, page: int = 0, size: int = 20, + sort=None, top: bool = False) -> Optional[list]: """ Get communities - either all, for single UUID, or all top-level (ie no sub-communities) @param uuid: string UUID if getting single community @@ -986,15 +1036,12 @@ def get_communities(self, uuid=None, page=0, size=20, sort=None, top=False): if sort is not None: params['sort'] = sort if uuid is not None: - try: - # This isn't used, but it'll throw a ValueError if not a valid UUID - id = UUID(uuid).version - # Set URL and parameters - url = f'{url}/{uuid}' - params = None - except ValueError: + if not _is_valid_uuid(uuid): _logger.error(f'Invalid community UUID: {uuid}') return None + # Set URL and parameters + url = f'{url}/{uuid}' + params = None if top: # Set new URL @@ -1004,7 +1051,7 @@ def get_communities(self, uuid=None, page=0, size=20, sort=None, top=False): # Perform actual get r_json = self.fetch_resource(url, params) # Empty list - communities = list() + communities = [] if '_embedded' in r_json: if 'communities' in r_json['_embedded']: for community_resource in r_json['_embedded']['communities']: @@ -1015,7 +1062,7 @@ def get_communities(self, uuid=None, page=0, size=20, sort=None, top=False): # Return list (populated or empty) return communities - def create_community(self, parent, data): + def create_community(self, parent, data) -> Community: """ Create a community, either top-level or beneath a given parent @param parent: (optional) parent UUID to pass as a parameter to create_dso @@ -1030,7 +1077,8 @@ def create_community(self, parent, data): params = {'parent': parent} return Community(api_resource=parse_json(self.create_dso(url, params, data))) - def get_collections(self, uuid=None, community=None, page=0, size=20, sort=None): + def get_collections(self, uuid: Optional[str] = None, community=None, page: int = 0, + size: int = 20, sort=None) -> Optional[list]: """ Get collections - all, or single UUID, or for a specific community @param uuid: UUID string. If present, just a single collection is returned (overrides community arg) @@ -1050,14 +1098,12 @@ def get_collections(self, uuid=None, community=None, page=0, size=20, sort=None) params['sort'] = sort # First, handle case of UUID. It overrides the other arguments as it is a request for a single collection if uuid is not None: - try: - id = UUID(uuid).version - # Update URL and parameters - url = f'{url}/{uuid}' - params = None - except ValueError: + if not _is_valid_uuid(uuid): _logger.error(f'Invalid collection UUID: {uuid}') return None + # Update URL and parameters + url = f'{url}/{uuid}' + params = None if community is not None: if 'collections' in community.links and 'href' in community.links['collections']: @@ -1067,7 +1113,7 @@ def get_collections(self, uuid=None, community=None, page=0, size=20, sort=None) # Perform the actual request. By now, our URL and parameter should be properly set r_json = self.fetch_resource(url, params=params) # Empty list - collections = list() + collections = [] if '_embedded' in r_json: # This is a list of collections if 'collections' in r_json['_embedded']: @@ -1080,7 +1126,7 @@ def get_collections(self, uuid=None, community=None, page=0, size=20, sort=None) # Return list (populated or empty) return collections - def create_collection(self, parent, data): + def create_collection(self, parent, data) -> Collection: """ Create collection beneath a given parent community. @param parent: UUID of parent community to pass as a parameter to create_dso @@ -1095,7 +1141,7 @@ def create_collection(self, parent, data): params = {'parent': parent} return Collection(api_resource=parse_json(self.create_dso(url, params, data))) - def get_item(self, uuid): + def get_item(self, uuid: str) -> Optional[Item]: """ Get an item, given its UUID @param uuid: the UUID of the item @@ -1103,7 +1149,7 @@ def get_item(self, uuid): """ url = f'{self.API_ENDPOINT}/core/items' try: - id = UUID(uuid).version + UUID(uuid) url = f'{url}/{uuid}' r = self.api_get(url, None, None) r_json = parse_json(response=r) @@ -1112,7 +1158,7 @@ def get_item(self, uuid): _logger.error(f'Invalid item UUID: {uuid}') return None - def get_item_by_handle(self, handle): + def get_item_by_handle(self, handle) -> Optional[Item]: """ Get item based on handle. """ @@ -1135,14 +1181,14 @@ def get_item_by_handle(self, handle): _logger.error(f'Invalid item handle: {handle}') return None - def get_items(self, page=0, size=20): + def get_items(self, page: int = 0, size: int = 20) -> list: """ Get all archived items for a logged-in administrator. Admin only! Usually you will want to use search or browse methods instead of this method @return: A list of items, or an error """ url = f'{self.API_ENDPOINT}/core/items' - items = list() + items = [] params = {} if size is not None: params['size'] = size @@ -1158,7 +1204,7 @@ def get_items(self, page=0, size=20): items.append(Item(r_json)) return items - def get_owningCollection(self, item_uuid): + def get_owningCollection(self, item_uuid: str) -> Optional[Collection]: """ Get owningCollection """ @@ -1172,7 +1218,7 @@ def get_owningCollection(self, item_uuid): _logger.error(f'Invalid owningCollection for UUID: {item_uuid}') return None - def create_item(self, parent, item): + def create_item(self, parent, item) -> Optional[Item]: """ Create an item beneath the given parent collection @param parent: UUID of parent collection to pass as a parameter to create_dso @@ -1194,7 +1240,7 @@ def create_item(self, parent, item): return None return Item(api_resource=parse_json(r)) - def update_item(self, item): + def update_item(self, item) -> Optional[DSpaceObject]: """ Update item. The Item passed to this method contains all the data, identifiers, links necessary to perform the update to the API. Note this is a full update, not a patch / partial update operation. @@ -1206,7 +1252,8 @@ def update_item(self, item): return None return self.update_dso(item, params=None) - def add_metadata(self, dso, field, value, language=None, authority=None, confidence=-1, place=''): + def add_metadata(self, dso, field, value, language=None, authority=None, + confidence: int = -1, place: str = ''): """ Add metadata to a DSO using the api_patch method (PUT, with path and operation and value) :param dso: @@ -1262,7 +1309,7 @@ def remove_metadata(self, dso, field, place=None): r = self.api_patch(url=url, operation=self.PatchOperation.REMOVE, path=path, value=None) return dso_type(api_resource=parse_json(r)) - def create_user(self, user, token=None): + def create_user(self, user, token=None) -> User: """ Create a user @param user: python User object or Python dict containing all the data and links expected by the REST API @@ -1287,9 +1334,9 @@ def delete_user(self, user): return self.delete_dso(user) # PAGINATION - def get_users(self, page=0, size=20, sort=None): + def get_users(self, page: int = 0, size: int = 20, sort=None) -> list: url = f'{self.API_ENDPOINT}/eperson/epersons' - users = list() + users = [] params = {} if size is not None: params['size'] = size @@ -1305,7 +1352,7 @@ def get_users(self, page=0, size=20, sort=None): users.append(User(user_resource)) return users - def create_group(self, group): + def create_group(self, group) -> Group: """ Create a group @param group: python Group object or Python dict containing all the data and links expected by the REST API @@ -1319,7 +1366,7 @@ def create_group(self, group): # that you see for other DSO types - still figuring out the best way return Group(api_resource=parse_json(self.create_dso(url, params=None, data=data))) - def create_submit_group(self, collection): + def create_submit_group(self, collection) -> Optional[Group]: """ Creates a submitter group for the given collection. """ @@ -1329,7 +1376,7 @@ def create_submit_group(self, collection): return Group(parse_json(r)) return None - def add_member(self, group, eperson): + def add_member(self, group, eperson) -> bool: """ Adds a user (EPerson) as a member of the specified group. @@ -1358,13 +1405,13 @@ def add_member(self, group, eperson): return False - def start_workflow(self, workspace_item): + def start_workflow(self, workspace_item) -> None: url = f'{self.API_ENDPOINT}/workflow/workflowitems' res = parse_json(self.api_post_uri(url, params=None, uri_list=workspace_item)) _logger.debug(res) # TODO: WIP - def update_token(self, r): + def update_token(self, r) -> None: """ Refresh / update the XSRF (aka. CSRF) token if DSPACE-XSRF-TOKEN found in response headers This is used by all the base methods like api_put, @@ -1382,7 +1429,7 @@ def update_token(self, r): self.session.headers.update({'X-XSRF-Token': t}) self.session.cookies.update({'X-XSRF-Token': t}) - def get_short_lived_token(self): + def get_short_lived_token(self) -> Optional[str]: """ Get a short-lived (2 min) token in order to request restricted bitstream downloads @return: short lived Authorization token @@ -1400,7 +1447,8 @@ def get_short_lived_token(self): _logger.error('Could not retrieve short-lived token') return None - def solr_query(self, query, filters=None, fields=None, start=0, rows=999999999): + def solr_query(self, query, filters=None, fields=None, start: int = 0, + rows: int = 999999999): if fields is None: fields = [] if filters is None: @@ -1409,14 +1457,15 @@ def solr_query(self, query, filters=None, fields=None, start=0, rows=999999999): 'fl': ','.join(fields) }) - def get_items_from_collection(self, collection_id, page=0, size=1000): + def get_items_from_collection(self, collection_id, page: int = 0, + size: int = 1000) -> list: """ Get all items @return: list of Item objects """ url = f'{self.API_ENDPOINT}/discover/search/objects?sort=dc.date.accessioned,DESC&page={page}&size={size}&scope={collection_id}&dsoType=ITEM&embed=thumbnail' - items = list() + items = [] r = self.api_get(url) r_json = parse_json(r) if '_embedded' in r_json: @@ -1427,7 +1476,7 @@ def get_items_from_collection(self, collection_id, page=0, size=1000): return items - def get_bundle_by_name(self, name, item_uuid): + def get_bundle_by_name(self, name, item_uuid: str) -> Optional[Bundle]: """ Get a bundle by name for a specific item @param name: Name of the bundle @@ -1443,7 +1492,7 @@ def get_bundle_by_name(self, name, item_uuid): return Bundle(bundle) return None - def get_resource_policy(self, bundle_uuid): + def get_resource_policy(self, bundle_uuid: str) -> Optional[dict]: """ Get a resource policy for a specific bundle """ @@ -1453,8 +1502,10 @@ def get_resource_policy(self, bundle_uuid): if '_embedded' in r_json: if 'resourcepolicies' in r_json['_embedded']: return r_json['_embedded']['resourcepolicies'][0] + return None - def create_resource_policy(self, resource_uuid, data, group_uuid=None, eperson_uuid=None): + def create_resource_policy(self, resource_uuid: str, data, group_uuid=None, + eperson_uuid=None) -> bool: """ Creates a resource policy by sending a POST request to the API endpoint. """ @@ -1471,7 +1522,7 @@ def create_resource_policy(self, resource_uuid, data, group_uuid=None, eperson_u return False - def update_resource_policy_group(self, policy_id, group_uuid): + def update_resource_policy_group(self, policy_id, group_uuid: str) -> requests.Response: """ Update a resource policy with a new group """ @@ -1480,7 +1531,7 @@ def update_resource_policy_group(self, policy_id, group_uuid): r = self.api_put_uri(url, None, body, False) return r - def get_clarinlruallowances(self): + def get_clarinlruallowances(self) -> Optional[list]: """ Fetch all clarinlruallowances. """ @@ -1495,7 +1546,8 @@ def get_clarinlruallowances(self): _logger.error(f"Error fetching CLARIN LRU allowances [{url}]: {e}") return None - def get_clarinlruallowances_by_bitstream_and_user(self, bitstream_uuid, user_uuid): + def get_clarinlruallowances_by_bitstream_and_user( + self, bitstream_uuid: str, user_uuid: str) -> Optional[list]: """ Fetch user allowances for a specific bitstream and user. """ @@ -1512,7 +1564,7 @@ def get_clarinlruallowances_by_bitstream_and_user(self, bitstream_uuid, user_uui return None - def create_clarinlruallowances(self, bitstream_uuid, metadata_payload=None): + def create_clarinlruallowances(self, bitstream_uuid: str, metadata_payload=None) -> bool: """ Create clarinlruallowances for a bitstream for the logged-in user by managing the bitstream's user metadata. @@ -1536,7 +1588,7 @@ def create_clarinlruallowances(self, bitstream_uuid, metadata_payload=None): return False - def get_user_by_email(self, email): + def get_user_by_email(self, email: str) -> Optional[User]: """ Retrieve user details using their email address. """ diff --git a/dspace_rest_client/models.py b/dspace_rest_client/models.py index ca8cb46..25acd0d 100644 --- a/dspace_rest_client/models.py +++ b/dspace_rest_client/models.py @@ -9,8 +9,11 @@ @author Kim Shepherd """ +from __future__ import annotations + import json import logging +from typing import Any _logger = logging.getLogger("dspace.models") # library: records are dropped unless the application configures logging @@ -28,7 +31,7 @@ class HALResource: links = {} type = None - def __init__(self, api_resource=None): + def __init__(self, api_resource: dict[str, Any] | None = None) -> None: """ Default constructor @param api_resource: optional API resource (JSON) from a GET response or successful POST can populate instance @@ -49,13 +52,13 @@ def __init__(self, api_resource=None): class AddressableHALResource(HALResource): id = None - def __init__(self, api_resource=None): + def __init__(self, api_resource: dict[str, Any] | None = None) -> None: super().__init__(api_resource) if api_resource is not None: if 'id' in api_resource: self.id = api_resource['id'] - def as_dict(self): + def as_dict(self) -> dict[str, Any]: return {'id': self.id} class ExternalDataObject(HALResource): @@ -68,14 +71,14 @@ class ExternalDataObject(HALResource): externalSource = None metadata = {} - def __init__(self, api_resource=None): + def __init__(self, api_resource: dict[str, Any] | None = None) -> None: """ Default constructor @param api_resource: optional API resource (JSON) from a GET response or successful POST can populate instance """ super().__init__(api_resource) - self.metadata = dict() + self.metadata = {} if api_resource is not None: if 'id' in api_resource: @@ -89,13 +92,13 @@ def __init__(self, api_resource=None): if 'metadata' in api_resource: self.metadata = api_resource['metadata'].copy() - def get_metadata_values(self, field): + def get_metadata_values(self, field: str) -> list: """ Return metadata values as simple list of strings @param field: DSpace field, eg. dc.creator @return: list of strings """ - values = list() + values = [] if field in self.metadata: values = self.metadata[field] return values @@ -116,14 +119,18 @@ class DSpaceObject(HALResource): type = None parent = None - def __init__(self, api_resource=None, dso=None): + def __init__( + self, + api_resource: dict[str, Any] | None = None, + dso: DSpaceObject | None = None, + ) -> None: """ Default constructor @param api_resource: optional API resource (JSON) from a GET response or successful POST can populate instance """ super().__init__(api_resource) self.type = None - self.metadata = dict() + self.metadata = {} if dso is not None: api_resource = dso.as_dict() @@ -147,10 +154,18 @@ def __init__(self, api_resource=None, dso=None): self.links = api_resource['_links'].copy() @property - def resourcePolicies(self): + def resourcePolicies(self) -> Any: return (self._from_d or {}).get('resourcePolicies') - def add_metadata(self, field, value, language=None, authority=None, confidence=-1, place=None): + def add_metadata( + self, + field: str, + value, + language=None, + authority=None, + confidence: int = -1, + place=None, + ) -> DSpaceObject | None: """ Add metadata to a DSO. This is performed on the local object only, it is not an API operation (see patch) This is useful when constructing new objects for ingest. @@ -165,7 +180,7 @@ def add_metadata(self, field, value, language=None, authority=None, confidence=- :return: """ if field is None or value is None: - return + return None if field in self.metadata: values = self.metadata[field] # Ensure we don't accidentally duplicate place value. If this place already exists, the user @@ -184,7 +199,7 @@ def add_metadata(self, field, value, language=None, authority=None, confidence=- # Return this as an easy way for caller to inspect or use return self - def clear_metadata(self, field=None, value=None): + def clear_metadata(self, field: str | None = None, value=None) -> None: if field is None: self.metadata = {} elif field in self.metadata: @@ -197,7 +212,7 @@ def clear_metadata(self, field=None, value=None): updated.append(v) self.metadata[field] = updated - def as_dict(self): + def as_dict(self) -> dict[str, Any]: """ Return custom dict of this DSpaceObject with specific attributes included (no _links, etc.) @return: dict of this DSpaceObject for API use @@ -211,10 +226,10 @@ def as_dict(self): 'type': self.type, } - def to_json(self): + def to_json(self) -> str: return json.dumps(self, default=lambda o: o.__dict__, sort_keys=True, indent=None) - def to_json_pretty(self): + def to_json_pretty(self) -> str: return json.dumps(self, default=lambda o: o.__dict__, sort_keys=True, indent=4) @@ -233,9 +248,13 @@ class Item(SimpleDSpaceObject): inArchive = False discoverable = False withdrawn = False - metadata = dict() + metadata = {} - def __init__(self, api_resource=None, dso=None): + def __init__( + self, + api_resource: dict[str, Any] | None = None, + dso: DSpaceObject | None = None, + ) -> None: """ Default constructor. Call DSpaceObject init then set item-specific attributes @param api_resource: API result object to use as initial data @@ -252,18 +271,18 @@ def __init__(self, api_resource=None, dso=None): self.discoverable = api_resource['discoverable'] if 'discoverable' in api_resource else False self.withdrawn = api_resource['withdrawn'] if 'withdrawn' in api_resource else False - def get_metadata_values(self, field): + def get_metadata_values(self, field: str) -> list: """ Return metadata values as simple list of strings @param field: DSpace field, eg. dc.creator @return: list of strings """ - values = list() + values = [] if field in self.metadata: values = self.metadata[field] return values - def as_dict(self): + def as_dict(self) -> dict[str, Any]: """ Return a dict representation of this Item, based on super with item-specific attributes added @return: dict of Item for API use @@ -273,7 +292,7 @@ def as_dict(self): return {**dso_dict, **item_dict} @classmethod - def from_dso(cls, dso: DSpaceObject): + def from_dso(cls, dso: DSpaceObject) -> Item: # Create new Item and copy everything over from this dso item = cls() for key, value in dso.__dict__.items(): @@ -287,7 +306,7 @@ class Community(SimpleDSpaceObject): """ type = 'community' - def __init__(self, api_resource=None): + def __init__(self, api_resource: dict[str, Any] | None = None) -> None: """ Default constructor. Call DSpaceObject init then set item-specific attributes @param api_resource: API result object to use as initial data @@ -295,7 +314,7 @@ def __init__(self, api_resource=None): super().__init__(api_resource) self.type = 'community' - def as_dict(self): + def as_dict(self) -> dict[str, Any]: """ Return a dict representation of this Community, based on super with community-specific attributes added @return: dict of Item for API use @@ -312,7 +331,7 @@ class Collection(SimpleDSpaceObject): """ type = 'collection' - def __init__(self, api_resource=None): + def __init__(self, api_resource: dict[str, Any] | None = None) -> None: """ Default constructor. Call DSpaceObject init then set collection-specific attributes @param api_resource: API result object to use as initial data @@ -320,7 +339,7 @@ def __init__(self, api_resource=None): super().__init__(api_resource) self.type = 'collection' - def as_dict(self): + def as_dict(self) -> dict[str, Any]: """ Return a dict representation of this Collection, based on super with collection-specific attributes added @return: dict of Item for API use @@ -336,7 +355,7 @@ class Bundle(DSpaceObject): """ type = 'bundle' - def __init__(self, api_resource=None): + def __init__(self, api_resource: dict[str, Any] | None = None) -> None: """ Default constructor. Call DSpaceObject init then set bundle-specific attributes @param api_resource: API result object to use as initial data @@ -344,7 +363,7 @@ def __init__(self, api_resource=None): super().__init__(api_resource) self.type = 'bundle' - def as_dict(self): + def as_dict(self) -> dict[str, Any]: """ Return a dict representation of this Bundle, based on super with bundle-specific attributes added @return: dict of Bundle for API use @@ -368,7 +387,7 @@ class Bitstream(DSpaceObject): } sequenceId = None - def __init__(self, api_resource=None): + def __init__(self, api_resource: dict[str, Any] | None = None) -> None: """ Default constructor. Call DSpaceObject init then set bitstream-specific attributes @param api_resource: API result object to use as initial data @@ -387,7 +406,7 @@ def __init__(self, api_resource=None): if 'sequenceId' in api_resource: self.sequenceId = api_resource['sequenceId'] - def as_dict(self): + def as_dict(self) -> dict[str, Any]: """ Return a dict representation of this Bitstream, based on super with bitstream-specific attributes added @return: dict of Bitstream for API use @@ -406,11 +425,12 @@ class Group(DSpaceObject): name = None permanent = False - def __init__(self, api_resource=None): + def __init__(self, api_resource: dict[str, Any] | None = None) -> None: """ Default constructor. Call DSpaceObject init then set group-specific attributes @param api_resource: API result object to use as initial data """ + api_resource = api_resource or {} super().__init__(api_resource) self.type = 'group' if 'name' in api_resource: @@ -418,7 +438,7 @@ def __init__(self, api_resource=None): if 'permanent' in api_resource: self.permanent = api_resource['permanent'] - def as_dict(self): + def as_dict(self) -> dict[str, Any]: """ Return a dict representation of this Group, based on super with group-specific attributes added @return: dict of Group for API use @@ -441,11 +461,12 @@ class User(SimpleDSpaceObject): requireCertificate = False selfRegistered = False - def __init__(self, api_resource=None): + def __init__(self, api_resource: dict[str, Any] | None = None) -> None: """ Default constructor. Call DSpaceObject init then set user-specific attributes @param api_resource: API result object to use as initial data """ + api_resource = api_resource or {} super().__init__(api_resource) self.type = 'user' if 'name' in api_resource: @@ -463,7 +484,7 @@ def __init__(self, api_resource=None): if 'selfRegistered' in api_resource: self.selfRegistered = api_resource['selfRegistered'] - def as_dict(self): + def as_dict(self) -> dict[str, Any]: """ Return a dict representation of this User, based on super with user-specific attributes added @return: dict of User for API use @@ -480,7 +501,7 @@ class InProgressSubmission(AddressableHALResource): sections = {} type = None - def __init__(self, api_resource): + def __init__(self, api_resource: dict[str, Any]) -> None: super().__init__(api_resource) if 'lastModified' in api_resource: self.lastModified = api_resource['lastModified'] @@ -491,7 +512,7 @@ def __init__(self, api_resource): if 'type' in api_resource: self.type = api_resource['type'] - def as_dict(self): + def as_dict(self) -> dict[str, Any]: parent_dict = super().as_dict() submission_dict = { 'lastModified': self.lastModified, @@ -502,12 +523,7 @@ def as_dict(self): return {**parent_dict, **submission_dict} class WorkspaceItem(InProgressSubmission): - - def __init__(self, api_resource): - super().__init__(api_resource) - - def as_dict(self): - return super().as_dict() + pass class EntityType(AddressableHALResource): """ @@ -515,7 +531,7 @@ class EntityType(AddressableHALResource): used in entities and relationships. For example, Publication, Person, Project and Journal are all common entity types used in DSpace 7+ """ - def __init__(self, api_resource): + def __init__(self, api_resource: dict[str, Any]) -> None: super().__init__(api_resource) if 'label' in api_resource: self.label = api_resource['label'] @@ -526,14 +542,14 @@ class RelationshipType(AddressableHALResource): """ TODO: RelationshipType """ - def __init__(self, api_resource): + def __init__(self, api_resource: dict[str, Any]) -> None: super().__init__(api_resource) class License(AddressableHALResource): """ Specific attributes and functions for licenses """ - def __init__(self, api_resource=None): + def __init__(self, api_resource: dict[str, Any] | None = None) -> None: super().__init__(api_resource) api_resource = api_resource or {} self.type = 'clarinlicense' @@ -547,7 +563,7 @@ def __init__(self, api_resource=None): api_resource.get('extendedClarinLicenseLabels', [])] self.bitstream = api_resource.get('bitstreams') - def to_dict(self): + def to_dict(self) -> dict[str, Any]: return { 'name': self.name, 'license_id': self.id, @@ -562,7 +578,7 @@ class Label(AddressableHALResource): """ Specific attributes and functions for licenses """ - def __init__(self, api_resource=None): + def __init__(self, api_resource: dict[str, Any] | None = None) -> None: """ Default constructor. Call DSpaceObject init then set label-specific attributes @param api_resource: API result object to use as initial data @@ -575,7 +591,7 @@ def __init__(self, api_resource=None): self.icon = api_resource.get('icon') self.extended = api_resource.get('extended', False) - def to_dict(self): + def to_dict(self) -> dict[str, Any]: return { 'label_id': self.id, 'label': self.label, @@ -589,7 +605,7 @@ class ResourcePolicy(AddressableHALResource): """ DQ specific. Extends Addressable HAL Resource to model a resource policy. """ - def __init__(self, api_resource: dict): + def __init__(self, api_resource: dict[str, Any]) -> None: super().__init__(api_resource) api_resource = api_resource or {} self.name = api_resource.get('name') @@ -614,7 +630,7 @@ def __init__(self, api_resource: dict): _logger.warning( f'Resource policy [{self.id}] has neither a group nor an eperson') - def as_dict(self): + def as_dict(self) -> dict[str, Any]: return { 'id': self.id, 'name': self.name, @@ -628,5 +644,5 @@ def as_dict(self): 'groupUUID': self.groupUUID, } - def __repr__(self): + def __repr__(self) -> str: return f"ResourcePolicy: {self.name} [{self.groupName}] [action: {self.action}] [type: {self.type}]" diff --git a/publish.sh b/publish.sh index 7462814..20f3ba1 100755 --- a/publish.sh +++ b/publish.sh @@ -1,3 +1,7 @@ #!/bin/bash -python setup.py bdist_wheel -twine upload dist/dspace_rest_client-0.1.10-py3-none-any.whl \ No newline at end of file +set -euo pipefail + +rm -rf build dist +python -m build +python -m twine check dist/* +python -m twine upload dist/* \ No newline at end of file diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 0000000..acd11a5 --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,120 @@ +[build-system] +requires = ["setuptools>=61.0"] +build-backend = "setuptools.build_meta" + +[project] +name = "dspace-rest-client" +version = "0.1.10" +description = "A DSpace 7 REST API client library" +readme = "README.md" +requires-python = ">=3.8" +license = { text = "BSD-3-Clause" } +authors = [ + { name = "Kim Shepherd" }, +] +classifiers = [ + "Programming Language :: Python :: 3.8", + "Programming Language :: Python :: 3.9", + "Programming Language :: Python :: 3.10", + "Programming Language :: Python :: 3.11", + "Programming Language :: Python :: 3.12", + "License :: OSI Approved :: BSD License", + "Operating System :: OS Independent", +] +dependencies = [ + "requests", +] + +[project.optional-dependencies] +solr = ["pysolr>=3.9"] +dev = [ + "pytest", + "requests-mock", + "ruff", + "pylint", + "mypy", +] + +[project.urls] +Documentation = "https://github.com/the-library-code/dspace-rest-python/blob/main/README.md" +GitHub = "https://github.com/the-library-code/dspace-rest-python" +Changelog = "https://github.com/the-library-code/dspace-rest-python/blob/main/CHANGELOG.md" + +[tool.setuptools] +packages = ["dspace_rest_client"] + +[tool.autopep8] +max_line_length = 90 + +[tool.ruff] +line-length = 90 +target-version = "py38" + +[tool.ruff.lint] +# E = pycodestyle, F = pyflakes, T20 = flake8-print (no print()) +select = ["E", "F", "T20"] +ignore = ["F403", "F405", "E501", "E402", "F841", "E741"] + +[tool.ruff.lint.per-file-ignores] +"tests/**" = ["T20"] + +[tool.mypy] +python_version = "3.10" +ignore_missing_imports = true + +[tool.pylint.main] +py-version = "3.8" +jobs = 0 + +[tool.pylint.basic] +# DSpace REST fields are camelCase (sizeBytes, checkSum, ...) mirrored verbatim +# as attributes; `id` is a valid short name here. +good-names = ["i", "j", "k", "ex", "Run", "_", "id", "d", "f"] +no-docstring-rgx = "^_" + +[tool.pylint.design] +max-args = 10 +max-attributes = 15 +max-branches = 15 +max-locals = 20 +max-public-methods = 20 +max-returns = 8 +max-statements = 60 + +[tool.pylint.format] +max-line-length = 90 +max-module-lines = 1200 + +[tool.pylint.imports] +allow-wildcard-with-all = false + +[tool.pylint.logging] +logging-format-style = "old" + +[tool.pylint."messages control"] +# camelCase attrs mirror the DSpace API (invalid-name); f-strings in log calls +# are mandated by the project (logging-fstring-interpolation); broad catches +# guard optional/fallback paths (broad-exception-caught). import-error: pysolr +# is an optional extra not present in every lint env. too-many-lines / +# too-many-public-methods: DSpaceClient is a wide REST facade (one method per +# endpoint); splitting into endpoint mixins is a tracked follow-up, not a +# blocker. fixme: upstream TODO markers are informational, not defects. +disable = [ + "missing-function-docstring", + "missing-class-docstring", + "missing-module-docstring", + "invalid-name", + "import-error", + "import-outside-toplevel", + "broad-exception-caught", + "logging-fstring-interpolation", + "line-too-long", + "too-few-public-methods", + "too-many-branches", + "too-many-locals", + "too-many-nested-blocks", + "too-many-positional-arguments", + "too-many-lines", + "too-many-public-methods", + "fixme", +] diff --git a/requirements-test.txt b/requirements-test.txt index 575ad6c..79774c3 100644 --- a/requirements-test.txt +++ b/requirements-test.txt @@ -1,7 +1,7 @@ # Test-only dependencies for the dspace_rest_client suite. # The test runner and an HTTP transport mock so tests never touch a real DSpace # server. `requests` is listed explicitly so `pip install -r requirements-test.txt` -# alone is enough to import the package (CI also `pip install .`s it via setup.py). +# alone is enough to import the package (CI also `pip install .`s it via pyproject.toml). pytest>=7.0 requests-mock>=1.11 requests diff --git a/requirements.txt b/requirements.txt deleted file mode 100644 index 298c07d..0000000 --- a/requirements.txt +++ /dev/null @@ -1,6 +0,0 @@ -requests~=2.27.1 - -setuptools~=47.1.0 -fpdf~=1.7.2 -textblob~=0.17.1 -pysolr~=3.9.0 \ No newline at end of file diff --git a/setup.py b/setup.py deleted file mode 100644 index 3256eb0..0000000 --- a/setup.py +++ /dev/null @@ -1,32 +0,0 @@ -import setuptools - -with open("README.md", "r") as fh: - long_description = fh.read() - -setuptools.setup( - name="dspace-rest-client", - version="0.1.10", - author="Kim Shepherd", - author_email="kim@the-library-code.de", - description="A DSpace 7 REST API client library", - license="BSD-3-Clause", - long_description=long_description, - long_description_content_type="text/markdown", - url="https://github.com/the-library-code/dspace-rest-client", - project_urls={ - 'Documentation': 'https://github.com/the-library-code/dspace-rest-python/blob/main/README.md', - 'GitHub': 'https://github.com/the-library-code/dspace-rest-python', - 'Changelog': 'https://github.com/the-library-code/dspace-rest-python/blob/main/CHANGELOG.md', - }, - classifiers=[ - "Programming Language :: Python :: 3.7", - "Programming Language :: Python :: 3.8", - "Programming Language :: Python :: 3.9", - "Programming Language :: Python :: 3.10", - "License :: OSI Approved :: BSD License", - "Operating System :: OS Independent", - ], - packages=["dspace_rest_client"], - install_requires=["requests"], - python_requires=">=3.8.0", -) diff --git a/tests/test_client_auth.py b/tests/test_client_auth.py index 08a1556..1ec3b42 100644 --- a/tests/test_client_auth.py +++ b/tests/test_client_auth.py @@ -6,6 +6,7 @@ so the True/False semantics here matter. """ import unittest +from unittest import mock import requests_mock @@ -32,6 +33,16 @@ def test_timeout_default_and_override(self): self.assertEqual(make_client().timeout, DSpaceClient.DEFAULT_TIMEOUT) self.assertEqual(DSpaceClient(API, "u", "p", timeout=5).timeout, 5) + def test_proxies_default_to_proxy_dict(self): + with mock.patch.object(DSpaceClient, "PROXY_DICT", {"http": "http://px:1"}): + self.assertEqual(make_client().proxies, {"http": "http://px:1"}) + + def test_explicit_proxies_none_is_kept(self): + # None lets requests resolve proxies itself (env vars); it must not be + # replaced by PROXY_DICT + with mock.patch.object(DSpaceClient, "PROXY_DICT", {"http": "http://px:1"}): + self.assertIsNone(DSpaceClient(API, "u", "p", proxies=None).proxies) + def test_library_does_not_hijack_root_logger(self): # importing the client must not call logging.basicConfig; the module # logger carries a NullHandler so records drop unless the app opts in. diff --git a/tests/test_models.py b/tests/test_models.py index 00a30a5..ceedd4b 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -12,7 +12,7 @@ import _helpers # noqa: F401 (bootstraps sys.path for direct runs) from _helpers import no_warnings_logged from dspace_rest_client.models import ( - Item, Community, Collection, Bundle, Bitstream, ResourcePolicy) + Item, Community, Collection, Bundle, Bitstream, ResourcePolicy, Group, User) class TestItem(unittest.TestCase): @@ -170,5 +170,16 @@ def test_policy_without_group_and_eperson_warns_with_id(self): self.assertIn("[4]", logs.records[0].getMessage()) +class TestGroupAndUserDefaults(unittest.TestCase): + + def test_constructible_without_api_resource(self): + # the signature allows None (the default); it must not raise TypeError + for cls in (Group, User): + with self.subTest(cls=cls.__name__): + obj = cls() + self.assertIsNone(obj.name) + self.assertIsNone(obj.uuid) + + if __name__ == "__main__": unittest.main() diff --git a/tests/test_packaging.py b/tests/test_packaging.py new file mode 100644 index 0000000..fd036d8 --- /dev/null +++ b/tests/test_packaging.py @@ -0,0 +1,54 @@ +""" +Import-time and public-surface contract of the package. + +``DSpaceClient`` reads its defaults (DSPACE_API_ENDPOINT, PROXY_URL, ...) from +the environment when ``dspace_rest_client.client`` is first imported, so the +package must not import it as a side effect of importing anything else. +""" +import os +import subprocess +import sys +import unittest + +import _helpers # noqa: F401 +from dspace_rest_client import client + +_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + + +def _run(code: str) -> str: + return subprocess.run([sys.executable, "-c", code], cwd=_ROOT, check=True, + capture_output=True, text=True).stdout.strip() + + +class TestLazyClientImport(unittest.TestCase): + + def test_importing_models_does_not_import_client(self): + out = _run("import sys, dspace_rest_client.models; " + "print('dspace_rest_client.client' in sys.modules)") + self.assertEqual(out, "False") + + def test_env_set_after_models_import_reaches_client_defaults(self): + out = _run("import os, dspace_rest_client.models; " + "os.environ['DSPACE_API_ENDPOINT'] = 'http://late/server/api'; " + "from dspace_rest_client.client import DSpaceClient; " + "print(DSpaceClient.API_ENDPOINT)") + self.assertEqual(out, "http://late/server/api") + + def test_client_still_reachable_from_package(self): + import dspace_rest_client + self.assertIs(dspace_rest_client.DSpaceClient, client.DSpaceClient) + + +class TestClientModuleReExports(unittest.TestCase): + + def test_models_previously_star_imported_into_client_stay_importable(self): + for name in ("DSpaceObject", "HALResource", "ExternalDataObject", + "SimpleDSpaceObject", "Community", "Collection", "Item", + "Bundle", "Bitstream", "User", "Group", "ResourcePolicy"): + with self.subTest(name=name): + self.assertTrue(hasattr(client, name)) + + +if __name__ == "__main__": + unittest.main()