From a9a6a5e6a751b041482a79e385c01a9419465a15 Mon Sep 17 00:00:00 2001 From: jm Date: Tue, 18 Aug 2026 01:51:10 +0200 Subject: [PATCH 1/7] build: migrate to pyproject.toml and adopt shared tooling Replace setup.py with a PEP 621 pyproject.toml (static metadata, requests core dep, solr + dev optional-dependency groups) and mirror the consumer repo's toolchain so both share one standard: - .pre-commit-config.yaml (mixed-line-ending, autopep8, ruff, pylint) - [tool.ruff] / [tool.pylint] / [tool.autopep8] / [tool.mypy] config - ship a py.typed marker (PEP 561) via package-data + MANIFEST.in Drop requirements.txt (fpdf/textblob/setuptools were unused cruft; runtime deps are now declared in pyproject). requirements-test.txt is kept because the consumer repo's CI installs it. --- .pre-commit-config.yaml | 27 ++++++++ MANIFEST.in | 2 + dspace_rest_client/py.typed | 0 pyproject.toml | 121 ++++++++++++++++++++++++++++++++++++ requirements-test.txt | 2 +- requirements.txt | 6 -- setup.py | 32 ---------- 7 files changed, 151 insertions(+), 39 deletions(-) create mode 100644 .pre-commit-config.yaml create mode 100644 MANIFEST.in create mode 100644 dspace_rest_client/py.typed create mode 100644 pyproject.toml delete mode 100644 requirements.txt delete mode 100644 setup.py 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/MANIFEST.in b/MANIFEST.in new file mode 100644 index 0000000..bfad03c --- /dev/null +++ b/MANIFEST.in @@ -0,0 +1,2 @@ +include README.md +include dspace_rest_client/py.typed diff --git a/dspace_rest_client/py.typed b/dspace_rest_client/py.typed new file mode 100644 index 0000000..e69de29 diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 0000000..ecc9a00 --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,121 @@ +[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.10" +license = { text = "BSD-3-Clause" } +authors = [ + { name = "Kim Shepherd", email = "kim@the-library-code.de" }, +] +classifiers = [ + "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.setuptools.package-data] +dspace_rest_client = ["py.typed"] + +[tool.autopep8] +max_line_length = 90 + +[tool.ruff] +line-length = 90 +target-version = "py310" + +[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.10" +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", -) From bfe1e6f3b370edf080c83833534213078352cbb7 Mon Sep 17 00:00:00 2001 From: jm Date: Tue, 18 Aug 2026 01:51:11 +0200 Subject: [PATCH 2/7] ci: add lint and non-blocking typecheck jobs Add a ruff+pylint lint job and a mypy typecheck job (continue-on-error, since strict typing is being introduced incrementally) alongside the existing pytest matrix, all run via uvx. Key the pip cache off pyproject.toml now that setup.py is gone. --- .github/workflows/tests.yml | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index affd373..62e79a9 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,32 @@ 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 + # Non-blocking: strict typing is being introduced incrementally. + continue-on-error: true + steps: + - uses: actions/checkout@v6 + + - name: Install uv + uses: astral-sh/setup-uv@v6 + + - name: Mypy + run: uvx --with requests --with pysolr mypy dspace_rest_client From 7e562c06682dc5ecc2980867891859ea23179069 Mon Sep 17 00:00:00 2001 From: jm Date: Tue, 18 Aug 2026 01:51:11 +0200 Subject: [PATCH 3/7] refactor: clear pylint findings and add type hints Bring dspace_rest_client to a clean 10.00/10 under the shared pylint config without changing behavior (all 68 tests still pass): - replace wildcard imports with explicit imports + __all__ (client, __init__) - extract a _is_valid_uuid() helper; stop shadowing the builtin id - fix a dangerous mutable default arg (proxies), list()/dict() literals, no-else-return, consider-using-in, inconsistent-return-statements, logging-not-lazy, pointless-string-statement and unused variables - collapse a redundant WorkspaceItem override to plain inheritance - add type hints (from __future__ import annotations) across models/client --- dspace_rest_client/__init__.py | 48 +++++- dspace_rest_client/client.py | 262 +++++++++++++++++++-------------- dspace_rest_client/models.py | 116 ++++++++------- 3 files changed, 261 insertions(+), 165 deletions(-) diff --git a/dspace_rest_client/__init__.py b/dspace_rest_client/__init__.py index b6e690f..5300a47 100644 --- a/dspace_rest_client/__init__.py +++ b/dspace_rest_client/__init__.py @@ -1 +1,47 @@ -from . import * +"""DSpace 7 REST API client.""" + +from .client import DSpaceClient +from .models import ( + HALResource, + AddressableHALResource, + ExternalDataObject, + DSpaceObject, + SimpleDSpaceObject, + Item, + Community, + Collection, + Bundle, + Bitstream, + Group, + User, + InProgressSubmission, + WorkspaceItem, + EntityType, + RelationshipType, + License, + Label, + ResourcePolicy, +) + +__all__ = [ + "DSpaceClient", + "HALResource", + "AddressableHALResource", + "ExternalDataObject", + "DSpaceObject", + "SimpleDSpaceObject", + "Item", + "Community", + "Collection", + "Bundle", + "Bitstream", + "Group", + "User", + "InProgressSubmission", + "WorkspaceItem", + "EntityType", + "RelationshipType", + "License", + "Label", + "ResourcePolicy", +] diff --git a/dspace_rest_client/client.py b/dspace_rest_client/client.py index 9cacb08..d0daba6 100644 --- a/dspace_rest_client/client.py +++ b/dspace_rest_client/client.py @@ -14,15 +14,29 @@ @author Kim Shepherd """ -import json +from __future__ import annotations + +import json as _json import logging import os +from typing import Any, Optional from uuid import UUID import requests from requests import Request -from .models import * +from .models import ( + Bitstream, + Bundle, + Collection, + Community, + DSpaceObject, + Group, + Item, + ResourcePolicy, + SimpleDSpaceObject, + User, +) __all__ = ['DSpaceClient'] @@ -35,7 +49,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) @@ -53,6 +67,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 @@ -87,7 +110,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: @@ -96,8 +122,10 @@ 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: Optional[dict] = None, 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 @@ -110,7 +138,7 @@ 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 + self.proxies = proxies if proxies is not None else self.PROXY_DICT self.solr = None self._last_err = None self.timeout = timeout if timeout is not None else self.DEFAULT_TIMEOUT @@ -136,7 +164,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 @@ -157,9 +185,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 @@ -183,7 +210,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. """ @@ -202,7 +229,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 @@ -210,7 +237,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 @@ -227,7 +255,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. @@ -272,7 +301,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. @@ -302,7 +332,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. @@ -334,7 +365,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. @@ -366,7 +398,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. @@ -397,7 +429,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) @@ -415,8 +448,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 @@ -459,7 +492,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 @@ -506,7 +540,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 @@ -524,7 +558,7 @@ def fetch_resource(self, url, params=None): # ValueError / JSON handling moved to static method return parse_json(r) - def get_resourcepolicy(self, uuid, action='READ'): + def get_resourcepolicy(self, uuid: str, action: str = 'READ') -> Optional[list]: """ Fetch resource policies for a given resource UUID and action. @param uuid: resource UUID to search for @@ -533,7 +567,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: @@ -551,9 +585,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= @@ -595,7 +629,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. @@ -605,14 +639,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. @@ -631,7 +665,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. @@ -655,27 +689,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 @@ -707,15 +738,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. @@ -725,7 +756,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}' @@ -733,7 +764,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 @@ -765,7 +796,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 @@ -787,7 +818,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 @@ -798,7 +830,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'] @@ -820,17 +852,18 @@ 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}]') - bitstreams = list() + bitstreams = [] if '_embedded' in r_json and 'bitstreams' in r_json['_embedded']: for bitstream_resource in r_json['_embedded']['bitstreams']: bitstreams.append(Bitstream(bitstream_resource)) return bitstreams - def create_bitstream(self, bundle=None, name=None, path=None, mime=None, metadata=None, retry=False): + def create_bitstream(self, bundle=None, name=None, path=None, mime=None, + metadata=None, retry: bool = False) -> Optional[Bitstream]: """ Upload a file and create a bitstream for a specified parent bundle, from the uploaded file and the supplied metadata. @@ -858,7 +891,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) @@ -868,7 +901,7 @@ def create_bitstream(self, bundle=None, name=None, path=None, mime=None, metadat r = self.session.send(prepared_req, proxies=self.proxies, timeout=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): @@ -879,14 +912,13 @@ def create_bitstream(self, bundle=None, name=None, path=None, mime=None, metadat self.authenticate() return self.create_bitstream(bundle, name, path, mime, metadata, True) - 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: @@ -897,9 +929,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 @@ -917,15 +951,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 @@ -935,7 +966,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']: @@ -946,7 +977,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 @@ -961,7 +992,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) @@ -981,14 +1013,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']: @@ -998,7 +1028,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']: @@ -1011,7 +1041,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 @@ -1026,7 +1056,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 @@ -1034,7 +1064,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) @@ -1043,7 +1073,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. """ @@ -1066,14 +1096,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 @@ -1089,7 +1119,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 """ @@ -1103,7 +1133,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 @@ -1125,7 +1155,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. @@ -1137,7 +1167,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: @@ -1193,7 +1224,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 @@ -1218,9 +1249,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 @@ -1236,7 +1267,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 @@ -1250,7 +1281,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. """ @@ -1260,7 +1291,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. @@ -1289,13 +1320,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, @@ -1313,7 +1344,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 @@ -1331,7 +1362,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: @@ -1340,14 +1372,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: @@ -1358,7 +1391,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 @@ -1374,7 +1407,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 """ @@ -1384,8 +1417,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. """ @@ -1402,7 +1437,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 """ @@ -1411,7 +1446,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. """ @@ -1426,7 +1461,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. """ @@ -1443,7 +1479,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. @@ -1467,7 +1503,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 4427512..b0db7f2 100644 --- a/dspace_rest_client/models.py +++ b/dspace_rest_client/models.py @@ -9,7 +9,10 @@ @author Kim Shepherd """ +from __future__ import annotations + import json +from typing import Any __all__ = ['DSpaceObject', 'HALResource', 'ExternalDataObject', 'SimpleDSpaceObject', 'Community', @@ -23,7 +26,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 @@ -44,13 +47,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): @@ -63,14 +66,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: @@ -84,13 +87,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 @@ -111,14 +114,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() @@ -142,10 +149,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. @@ -160,7 +175,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 @@ -179,7 +194,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: @@ -192,7 +207,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 @@ -206,10 +221,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) @@ -228,9 +243,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 @@ -247,18 +266,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 @@ -268,7 +287,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(): @@ -282,7 +301,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 @@ -290,7 +309,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 @@ -307,7 +326,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 @@ -315,7 +334,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 @@ -331,7 +350,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 @@ -339,7 +358,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 @@ -363,7 +382,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 @@ -382,7 +401,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 @@ -401,7 +420,7 @@ 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 @@ -413,7 +432,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 @@ -436,7 +455,7 @@ 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 @@ -458,7 +477,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 @@ -475,7 +494,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'] @@ -486,7 +505,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, @@ -497,12 +516,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): """ @@ -510,7 +524,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'] @@ -521,14 +535,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' @@ -542,7 +556,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, @@ -557,7 +571,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 @@ -570,7 +584,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, @@ -584,7 +598,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') @@ -603,7 +617,7 @@ def __init__(self, api_resource: dict): self.groupName = api_resource['_embedded']['group'].get('name') self.groupUUID = api_resource['_embedded']['group'].get('uuid') - def as_dict(self): + def as_dict(self) -> dict[str, Any]: return { 'id': self.id, 'name': self.name, @@ -617,5 +631,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}]" From 8d1e7eca0bf24901eb3639c2c95586db3e798900 Mon Sep 17 00:00:00 2001 From: jm Date: Tue, 18 Aug 2026 01:53:36 +0200 Subject: [PATCH 4/7] ci: report the typecheck job green (step-level continue-on-error) Job-level continue-on-error still surfaces the job as a failed check on the PR; moving it to the mypy step lets the job pass green while mypy stays advisory. --- .github/workflows/tests.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 62e79a9..d52185c 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -56,8 +56,6 @@ jobs: typecheck: runs-on: ubuntu-latest - # Non-blocking: strict typing is being introduced incrementally. - continue-on-error: true steps: - uses: actions/checkout@v6 @@ -65,4 +63,7 @@ jobs: 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 From cc339803744634f9ee1a50197e90fcdb0d3ed42d Mon Sep 17 00:00:00 2001 From: jm Date: Tue, 18 Aug 2026 02:20:12 +0200 Subject: [PATCH 5/7] fix: address second Copilot review - get_resourcepolicy: annotate action as Optional[str]. None is a tested, supported value that omits the action filter, but the str annotation rejected it for typed consumers now that the package ships py.typed. - Copy the class-level PROXY_DICT default per instance so mutating one client's .proxies cannot leak into other default-constructed clients. - README: drop the deleted requirements.txt install step and the stale Python 3.8 requirement (now `pip install .` / Python 3.10+). --- README.md | 9 +++++---- dspace_rest_client/client.py | 7 +++++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 240cfc6..2841747 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.10+ +* 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/client.py b/dspace_rest_client/client.py index d0daba6..f58a688 100644 --- a/dspace_rest_client/client.py +++ b/dspace_rest_client/client.py @@ -138,7 +138,9 @@ def __init__(self, api_endpoint: str = API_ENDPOINT, username: str = USERNAME, self.USERNAME = username self.PASSWORD = password self.SOLR_ENDPOINT = solr_endpoint - self.proxies = proxies if proxies is not None else self.PROXY_DICT + # Copy the class-level default so per-instance mutation of `proxies` + # never leaks into other default-constructed clients. + self.proxies = proxies if proxies is not None else dict(self.PROXY_DICT) self.solr = None self._last_err = None self.timeout = timeout if timeout is not None else self.DEFAULT_TIMEOUT @@ -558,7 +560,8 @@ def fetch_resource(self, url: str, params=None) -> Any: # ValueError / JSON handling moved to static method return parse_json(r) - def get_resourcepolicy(self, uuid: str, action: str = 'READ') -> Optional[list]: + 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 From 170b617aea04ad01daa4e61b946d9e003a535690 Mon Sep 17 00:00:00 2001 From: jm Date: Tue, 18 Aug 2026 14:07:34 +0200 Subject: [PATCH 6/7] build: drop py.typed and MANIFEST.in The library is consumed as a vendored submodule on sys.path, so the type hints in the source are read directly and the PEP 561 marker adds nothing; MANIFEST.in only affects an sdist that is never built here. Remove both and the now-dangling package-data entry. --- MANIFEST.in | 2 -- dspace_rest_client/py.typed | 0 pyproject.toml | 3 --- 3 files changed, 5 deletions(-) delete mode 100644 MANIFEST.in delete mode 100644 dspace_rest_client/py.typed diff --git a/MANIFEST.in b/MANIFEST.in deleted file mode 100644 index bfad03c..0000000 --- a/MANIFEST.in +++ /dev/null @@ -1,2 +0,0 @@ -include README.md -include dspace_rest_client/py.typed diff --git a/dspace_rest_client/py.typed b/dspace_rest_client/py.typed deleted file mode 100644 index e69de29..0000000 diff --git a/pyproject.toml b/pyproject.toml index ecc9a00..9e1b523 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -41,9 +41,6 @@ Changelog = "https://github.com/the-library-code/dspace-rest-python/blob/main/CH [tool.setuptools] packages = ["dspace_rest_client"] -[tool.setuptools.package-data] -dspace_rest_client = ["py.typed"] - [tool.autopep8] max_line_length = 90 From b2af3a6f8b01a25f38887e41def6a50e61f54343 Mon Sep 17 00:00:00 2001 From: jm Date: Tue, 18 Aug 2026 14:18:18 +0200 Subject: [PATCH 7/7] build: drop the original author's stale email from pyproject --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 9e1b523..0459c25 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -10,7 +10,7 @@ readme = "README.md" requires-python = ">=3.10" license = { text = "BSD-3-Clause" } authors = [ - { name = "Kim Shepherd", email = "kim@the-library-code.de" }, + { name = "Kim Shepherd" }, ] classifiers = [ "Programming Language :: Python :: 3.10",