From feeb3997e7a4ae0ae33962cafa79447f49ff5729 Mon Sep 17 00:00:00 2001
From: jm <jm@maz>
Date: Mon, 17 Aug 2026 22:21:38 +0200
Subject: [PATCH 1/2] fix(client): correctness sweep - logging, timeouts, file
 handle, None guards

Python-hygiene fixes (no behaviour change on the happy path):

- Remove logging.basicConfig() at import - a library must not configure the
  root logger. Attach a NullHandler and route everything through the module
  _logger (dropped the stray root logging.* calls).
- Add a configurable per-request timeout (DEFAULT_TIMEOUT=60, `timeout=`
  constructor arg) on every session call, so a stalled server cannot hang the
  client forever.
- create_bitstream: open the upload file in a `with` block so the handle is
  always closed instead of leaked to the GC.
- Bitstream(None) no longer raises TypeError on the __init__ membership checks.
- create_clarinlruallowances: parameterise metadata_payload; drop the leftover
  hardcoded {"metadataValue":"Test"} debug data (refuses with no payload).
- models: modernise super(Cls, self) -> super() throughout.

Mutable class-attribute defaults were intentionally left untouched.

68 tests (5 new), no network.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 dspace_rest_client/client.py | 84 +++++++++++++++++++++---------------
 dspace_rest_client/models.py | 51 +++++++++++-----------
 tests/test_client_auth.py    | 15 +++++++
 tests/test_client_write.py   | 22 ++++++++++
 tests/test_models.py         |  7 +++
 5 files changed, 120 insertions(+), 59 deletions(-)

diff --git a/dspace_rest_client/client.py b/dspace_rest_client/client.py
index c298378..6b88223 100644
--- a/dspace_rest_client/client.py
+++ b/dspace_rest_client/client.py
@@ -26,8 +26,11 @@
 
 __all__ = ['DSpaceClient']
 
-logging.basicConfig(format='%(asctime)s - %(message)s', level=logging.INFO)
 _logger = logging.getLogger("dspace.client")
+# A library must not configure the root logger - that is the consuming
+# application's job. Attach a NullHandler so records are dropped unless the
+# application opts in to logging.
+_logger.addHandler(logging.NullHandler())
 
 
 def parse_json(response):
@@ -79,6 +82,9 @@ class DSpaceClient:
         USER_AGENT = os.environ['USER_AGENT']
     verbose = False
     ITER_PAGE_SIZE = 20
+    # 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()
 
     # Simple enum for patch operation types
@@ -89,7 +95,7 @@ class PatchOperation:
         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):
+                 solr_auth=SOLR_AUTH, fake_user_agent=False, proxies=PROXY_DICT, timeout=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
@@ -105,6 +111,7 @@ def __init__(self, api_endpoint=API_ENDPOINT, username=USERNAME, password=PASSWO
         self.proxies = proxies
         self.solr = None
         self._last_err = None
+        self.timeout = timeout if timeout is not None else self.DEFAULT_TIMEOUT
         try:
             import pysolr
             self.solr = pysolr.Solr(url=solr_endpoint, always_commit=True, timeout=300, auth=solr_auth)
@@ -137,7 +144,7 @@ def authenticate(self, retry=False):
         # Get and update CSRF token
         r = self.session.post(self.LOGIN_URL, data={'user': self.USERNAME, 'password': self.PASSWORD},
                               headers=self.auth_request_headers,
-                              proxies=self.proxies)
+                              proxies=self.proxies, timeout=self.timeout)
         self.update_token(r)
 
         if r.status_code == 403:
@@ -164,7 +171,7 @@ def authenticate(self, retry=False):
 
         # Get and check authentication status
         r = self.session.get(f'{self.API_ENDPOINT}/authn/status', headers=self.request_headers,
-                             proxies=self.proxies)
+                             proxies=self.proxies, timeout=self.timeout)
         if r.status_code == 200:
             r_json = parse_json(r)
             if 'authenticated' in r_json and r_json['authenticated'] is True:
@@ -214,7 +221,7 @@ def api_get(self, url, params=None, data=None, headers=None):
         if headers is None:
             headers = self.request_headers
         r = self.session.get(url, params=params, data=data, headers=headers,
-                             proxies=self.proxies)
+                             proxies=self.proxies, timeout=self.timeout)
         self.update_token(r)
         return r
 
@@ -230,7 +237,7 @@ def api_post(self, url, params, json, retry=False, timeout=None):
         """
         self._last_err = None
         r = self.session.post(url, json=json, params=params, headers=self.request_headers,
-                              proxies=self.proxies, timeout=timeout)
+                              proxies=self.proxies, timeout=timeout if timeout is not None else self.timeout)
         self.update_token(r)
 
         if r.status_code == 403:
@@ -252,10 +259,10 @@ def api_post(self, url, params, json, retry=False, timeout=None):
             r_json = parse_json(r)
             if 'message' in (r_json or {}) and 'Authentication is required' in r_json['message']:
                 if retry:
-                    logging.error(
+                    _logger.error(
                         'API Post: Already retried... something must be wrong')
                 else:
-                    logging.debug("API Post: Retrying request with updated CSRF token")
+                    _logger.debug("API Post: Retrying request with updated CSRF token")
                     # try to authenticate
                     self.authenticate()
                     # Try to authenticate and repeat the request 3 times -
@@ -275,7 +282,7 @@ def api_post_uri(self, url, params, uri_list, retry=False):
         """
         self._last_err = None
         r = self.session.post(url, data=uri_list, params=params, headers=self.list_request_headers,
-                              proxies=self.proxies)
+                              proxies=self.proxies, timeout=self.timeout)
         self.update_token(r)
 
         if r.status_code == 403:
@@ -305,7 +312,7 @@ def api_put(self, url, params, json, retry=False):
         """
         self._last_err = None
         r = self.session.put(url, params=params, json=json, headers=self.request_headers,
-                             proxies=self.proxies)
+                             proxies=self.proxies, timeout=self.timeout)
         self.update_token(r)
 
         if r.status_code == 403:
@@ -337,7 +344,7 @@ def api_put_uri(self, url, params, uri_list, retry=False):
         """
         self._last_err = None
         r = self.session.put(url, params=params, data=uri_list, headers=self.list_request_headers,
-                             proxies=self.proxies)
+                             proxies=self.proxies, timeout=self.timeout)
         self.update_token(r)
 
         if r.status_code == 403:
@@ -368,7 +375,7 @@ def api_delete(self, url, params, retry=False):
         """
         self._last_err = None
         r = self.session.delete(url, params=params, headers=self.request_headers,
-                                proxies=self.proxies)
+                                proxies=self.proxies, timeout=self.timeout)
         self.update_token(r)
 
         if r.status_code == 403:
@@ -401,15 +408,15 @@ def api_patch(self, url, operation, path, value, params=None, retry=False):
         """
         self._last_err = None
         if url is None:
-            logging.error('Missing required URL argument')
+            _logger.error('Missing required URL argument')
             return None
         if path is None:
-            logging.error('Need valid path eg. /withdrawn or /metadata/dc.title/0/language')
+            _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:
             # missing value required for add/replace/move operations
-            logging.error('Missing required "value" argument for add/replace/move operations')
+            _logger.error('Missing required "value" argument for add/replace/move operations')
             return None
 
         # compile patch data
@@ -426,7 +433,7 @@ def api_patch(self, url, operation, path, value, params=None, retry=False):
         # set headers
         # perform patch request
         r = self.session.patch(url, json=[data], params=params, headers=self.request_headers,
-                               proxies=self.proxies)
+                               proxies=self.proxies, timeout=self.timeout)
         self.update_token(r)
 
         if r.status_code == 403:
@@ -635,7 +642,7 @@ def update_dso(self, dso, params=None):
             return None
         dso_type = type(dso)
         if not isinstance(dso, SimpleDSpaceObject):
-            logging.error('Only SimpleDSpaceObject types (eg Item, Collection, Community) '
+            _logger.error('Only SimpleDSpaceObject types (eg Item, Collection, Community) '
                   'are supported by generic update_dso PUT.')
             return dso
         try:
@@ -682,11 +689,11 @@ def delete_dso(self, dso=None, url=None, params=None):
         """
         if dso is None:
             if url is None:
-                logging.error('Need a DSO or a URL to delete')
+                _logger.error('Need a DSO or a URL to delete')
                 return None
         else:
             if not isinstance(dso, SimpleDSpaceObject):
-                logging.error('Only SimpleDSpaceObject types (eg Item, Collection, Community, EPerson) '
+                _logger.error('Only SimpleDSpaceObject types (eg Item, Collection, Community, EPerson) '
                       'are supported by generic update_dso PUT.')
                 return dso
             # Get self URI from HAL links
@@ -844,15 +851,17 @@ def create_bitstream(self, bundle=None, name=None, path=None, mime=None, metadat
         if metadata is None:
             metadata = {}
         url = f'{self.API_ENDPOINT}/core/bundles/{bundle.uuid}/bitstreams'
-        file = (name, open(path, 'rb'), mime)
-        files = {'file': file}
-        properties = {'name': name, 'metadata': metadata, 'bundleName': bundle.name}
-        payload = {'properties': json.dumps(properties) + ';application/json'}
-        h = self.session.headers
-        h.update({'Content-Encoding': 'gzip', 'User-Agent': self.USER_AGENT})
-        req = Request('POST', url, data=payload, headers=h, files=files)
-        prepared_req = self.session.prepare_request(req)
-        r = self.session.send(prepared_req, proxies=self.proxies)
+        # open the file in a context manager so the handle is always closed,
+        # even if prepare/send raises (it was previously leaked to the GC).
+        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'}
+            h = self.session.headers
+            h.update({'Content-Encoding': 'gzip', 'User-Agent': self.USER_AGENT})
+            req = Request('POST', url, data=payload, headers=h, files=files)
+            prepared_req = self.session.prepare_request(req)
+            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)
@@ -1200,7 +1209,7 @@ def create_user(self, user, token=None):
 
     def delete_user(self, user):
         if not isinstance(user, User):
-            logging.error('Must be a valid user')
+            _logger.error('Must be a valid user')
             return None
         return self.delete_dso(user)
 
@@ -1430,16 +1439,21 @@ def get_clarinlruallowances_by_bitstream_and_user(self, bitstream_uuid, user_uui
         return None
 
 
-    def create_clarinlruallowances(self, bitstream_uuid):
+    def create_clarinlruallowances(self, bitstream_uuid, metadata_payload=None):
         """
-        Create clarinlruallowances for a bitstream for logged user
-        by managing user metadata of bitstream.
+        Create clarinlruallowances for a bitstream for the logged-in user by
+        managing the bitstream's user metadata.
+        @param bitstream_uuid:   target bitstream UUID
+        @param metadata_payload: list of {"metadataKey", "metadataValue"} dicts.
+                                 Required - there is no meaningful default (the
+                                 previous hardcoded "Test" value was leftover
+                                 debug data, not usable for real callers).
         """
+        if not metadata_payload:
+            _logger.error('create_clarinlruallowances requires a metadata_payload')
+            return False
         url = f'{self.API_ENDPOINT}/core/clarinusermetadata/manage'
         params = {'bitstreamUUID': bitstream_uuid}
-        metadata_payload = [
-            {"metadataKey": "NAME", "metadataValue": "Test"}
-        ]
         try:
             response = self.api_post(url, json=metadata_payload, params=params)
             if response.status_code == 200:
diff --git a/dspace_rest_client/models.py b/dspace_rest_client/models.py
index f09aa2c..a843bc1 100644
--- a/dspace_rest_client/models.py
+++ b/dspace_rest_client/models.py
@@ -237,9 +237,9 @@ def __init__(self, api_resource=None, dso=None):
         """
         if dso is not None:
             api_resource = dso.as_dict()
-            super(Item, self).__init__(dso=dso)
+            super().__init__(dso=dso)
         else:
-            super(Item, self).__init__(api_resource)
+            super().__init__(api_resource)
 
         if api_resource is not None:
             self.type = 'item'
@@ -263,7 +263,7 @@ def as_dict(self):
         Return a dict representation of this Item, based on super with item-specific attributes added
         @return: dict of Item for API use
         """
-        dso_dict = super(Item, self).as_dict()
+        dso_dict = super().as_dict()
         item_dict = {'inArchive': self.inArchive, 'discoverable': self.discoverable, 'withdrawn': self.withdrawn}
         return {**dso_dict, **item_dict}
 
@@ -287,7 +287,7 @@ def __init__(self, api_resource=None):
         Default constructor. Call DSpaceObject init then set item-specific attributes
         @param api_resource: API result object to use as initial data
         """
-        super(Community, self).__init__(api_resource)
+        super().__init__(api_resource)
         self.type = 'community'
 
     def as_dict(self):
@@ -295,7 +295,7 @@ def as_dict(self):
         Return a dict representation of this Community, based on super with community-specific attributes added
         @return: dict of Item for API use
         """
-        dso_dict = super(Community, self).as_dict()
+        dso_dict = super().as_dict()
         # TODO: More community-specific stuff
         community_dict = {}
         return {**dso_dict, **community_dict}
@@ -312,11 +312,11 @@ def __init__(self, api_resource=None):
         Default constructor. Call DSpaceObject init then set collection-specific attributes
         @param api_resource: API result object to use as initial data
         """
-        super(Collection, self).__init__(api_resource)
+        super().__init__(api_resource)
         self.type = 'collection'
 
     def as_dict(self):
-        dso_dict = super(Collection, self).as_dict()
+        dso_dict = super().as_dict()
         """
         Return a dict representation of this Collection, based on super with collection-specific attributes added
         @return: dict of Item for API use
@@ -336,7 +336,7 @@ def __init__(self, api_resource=None):
         Default constructor. Call DSpaceObject init then set bundle-specific attributes
         @param api_resource: API result object to use as initial data
         """
-        super(Bundle, self).__init__(api_resource)
+        super().__init__(api_resource)
         self.type = 'bundle'
 
     def as_dict(self):
@@ -344,7 +344,7 @@ def as_dict(self):
         Return a dict representation of this Bundle, based on super with bundle-specific attributes added
         @return: dict of Bundle for API use
         """
-        dso_dict = super(Bundle, self).as_dict()
+        dso_dict = super().as_dict()
         bundle_dict = {}
         return {**dso_dict, **bundle_dict}
 
@@ -368,8 +368,11 @@ def __init__(self, api_resource=None):
         Default constructor. Call DSpaceObject init then set bitstream-specific attributes
         @param api_resource: API result object to use as initial data
         """
-        super(Bitstream, self).__init__(api_resource)
+        super().__init__(api_resource)
         self.type = 'bitstream'
+        # tolerate Bitstream(None): other models guard this, and without it the
+        # membership tests below raise TypeError on a None api_resource.
+        api_resource = api_resource or {}
         if 'bundleName' in api_resource:
             self.bundleName = api_resource['bundleName']
         if 'sizeBytes' in api_resource:
@@ -384,7 +387,7 @@ def as_dict(self):
         Return a dict representation of this Bitstream, based on super with bitstream-specific attributes added
         @return: dict of Bitstream for API use
         """
-        dso_dict = super(Bitstream, self).as_dict()
+        dso_dict = super().as_dict()
         bitstream_dict = {'bundleName': self.bundleName, 'sizeBytes': self.sizeBytes, 'checkSum': self.checkSum,
                           'sequenceId': self.sequenceId}
         return {**dso_dict, **bitstream_dict}
@@ -403,7 +406,7 @@ def __init__(self, api_resource=None):
         Default constructor. Call DSpaceObject init then set group-specific attributes
         @param api_resource: API result object to use as initial data
         """
-        super(Group, self).__init__(api_resource)
+        super().__init__(api_resource)
         self.type = 'group'
         if 'name' in api_resource:
             self.name = api_resource['name']
@@ -415,7 +418,7 @@ def as_dict(self):
         Return a dict representation of this Group, based on super with group-specific attributes added
         @return: dict of Group for API use
         """
-        dso_dict = super(Group, self).as_dict()
+        dso_dict = super().as_dict()
         group_dict = {'name': self.name, 'permanent': self.permanent}
         return {**dso_dict, **group_dict}
 
@@ -438,7 +441,7 @@ def __init__(self, api_resource=None):
         Default constructor. Call DSpaceObject init then set user-specific attributes
         @param api_resource: API result object to use as initial data
         """
-        super(User, self).__init__(api_resource)
+        super().__init__(api_resource)
         self.type = 'user'
         if 'name' in api_resource:
             self.name = api_resource['name']
@@ -460,7 +463,7 @@ def as_dict(self):
         Return a dict representation of this User, based on super with user-specific attributes added
         @return: dict of User for API use
         """
-        dso_dict = super(User, self).as_dict()
+        dso_dict = super().as_dict()
         user_dict = {'name': self.name, 'netid': self.netid, 'lastActive': self.lastActive, 'canLogIn': self.canLogIn,
                      'email': self.email, 'requireCertificate': self.requireCertificate,
                      'selfRegistered': self.selfRegistered}
@@ -473,7 +476,7 @@ class InProgressSubmission(AddressableHALResource):
     type = None
 
     def __init__(self, api_resource):
-        super(InProgressSubmission, self).__init__(api_resource)
+        super().__init__(api_resource)
         if 'lastModified' in api_resource:
             self.lastModified = api_resource['lastModified']
         if 'step' in api_resource:
@@ -484,7 +487,7 @@ def __init__(self, api_resource):
             self.type = api_resource['type']
 
     def as_dict(self):
-        parent_dict = super(InProgressSubmission, self).as_dict()
+        parent_dict = super().as_dict()
         dict = {
             'lastModified': self.lastModified,
             'step': self.step,
@@ -496,10 +499,10 @@ def as_dict(self):
 class WorkspaceItem(InProgressSubmission):
 
     def __init__(self, api_resource):
-        super(WorkspaceItem, self).__init__(api_resource)
+        super().__init__(api_resource)
 
     def as_dict(self):
-        return super(WorkspaceItem, self).as_dict()
+        return super().as_dict()
 
 class EntityType(AddressableHALResource):
     """
@@ -508,7 +511,7 @@ class EntityType(AddressableHALResource):
     are all common entity types used in DSpace 7+
     """
     def __init__(self, api_resource):
-        super(EntityType, self).__init__(api_resource)
+        super().__init__(api_resource)
         if 'label' in api_resource:
             self.label = api_resource['label']
         if 'type' in api_resource:
@@ -519,14 +522,14 @@ class RelationshipType(AddressableHALResource):
     TODO: RelationshipType
     """
     def __init__(self, api_resource):
-        super(RelationshipType, self).__init__(api_resource)
+        super().__init__(api_resource)
 
 class License(AddressableHALResource):
     """
     Specific attributes and functions for licenses
     """
     def __init__(self, api_resource=None):
-        super(License, self).__init__(api_resource)
+        super().__init__(api_resource)
         api_resource = api_resource or {}
         self.type = 'clarinlicense'
         self.name = api_resource.get('name')
@@ -559,7 +562,7 @@ def __init__(self, api_resource=None):
         Default constructor. Call DSpaceObject init then set label-specific attributes
         @param api_resource: API result object to use as initial data
         """
-        super(Label, self).__init__(api_resource)
+        super().__init__(api_resource)
         api_resource = api_resource or {}
         self.type = 'clarinlicenselabel'
         self.label = api_resource.get('label')
@@ -582,7 +585,7 @@ class ResourcePolicy(AddressableHALResource):
         DQ specific. Extends Addressable HAL Resource to model a resource policy.
     """
     def __init__(self, api_resource: dict):
-        super(ResourcePolicy, self).__init__(api_resource)
+        super().__init__(api_resource)
         api_resource = api_resource or {}
         self.name = api_resource.get('name')
         self.description = api_resource.get('description')
diff --git a/tests/test_client_auth.py b/tests/test_client_auth.py
index dae22da..08a1556 100644
--- a/tests/test_client_auth.py
+++ b/tests/test_client_auth.py
@@ -9,8 +9,11 @@
 
 import requests_mock
 
+import logging
+
 import _helpers  # noqa: F401
 from _helpers import make_client, API
+from dspace_rest_client.client import DSpaceClient
 
 
 class TestConstructor(unittest.TestCase):
@@ -24,6 +27,18 @@ def test_endpoints_derived_from_api_endpoint(self):
     def test_default_last_err_is_none(self):
         self.assertIsNone(make_client().last_err)
 
+    def test_timeout_default_and_override(self):
+        # every request must be bounded so a stalled server can't hang forever
+        self.assertEqual(make_client().timeout, DSpaceClient.DEFAULT_TIMEOUT)
+        self.assertEqual(DSpaceClient(API, "u", "p", timeout=5).timeout, 5)
+
+    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.
+        lg = logging.getLogger("dspace.client")
+        self.assertTrue(
+            any(isinstance(h, logging.NullHandler) for h in lg.handlers))
+
 
 class TestAuthenticate(unittest.TestCase):
 
diff --git a/tests/test_client_write.py b/tests/test_client_write.py
index 28231e6..9e4815c 100644
--- a/tests/test_client_write.py
+++ b/tests/test_client_write.py
@@ -193,5 +193,27 @@ def test_server_error_returns_none(self):
                 mime="application/pdf"))
 
 
+class TestCreateClarinAllowances(unittest.TestCase):
+
+    def test_requires_metadata_payload(self):
+        # the previous hardcoded {"metadataValue":"Test"} is gone; with no
+        # payload the call refuses and makes no request.
+        c = make_client()
+        with requests_mock.Mocker() as m:
+            self.assertFalse(c.create_clarinlruallowances(BITSTREAM_UUID))
+            self.assertEqual(m.call_count, 0)
+
+    def test_posts_supplied_payload(self):
+        c = make_client()
+        payload = [{"metadataKey": "NAME", "metadataValue": "real value"}]
+        with requests_mock.Mocker() as m:
+            m.post(f"{API}/core/clarinusermetadata/manage", status_code=200,
+                   json={})
+            self.assertTrue(c.create_clarinlruallowances(BITSTREAM_UUID, payload))
+            self.assertEqual(m.last_request.json(), payload)
+            self.assertEqual(sent_params(m.last_request)["bitstreamUUID"],
+                             [BITSTREAM_UUID])
+
+
 if __name__ == "__main__":
     unittest.main()
diff --git a/tests/test_models.py b/tests/test_models.py
index 2e06059..8167548 100644
--- a/tests/test_models.py
+++ b/tests/test_models.py
@@ -102,6 +102,13 @@ def test_bitstream_file_fields(self):
         self.assertEqual(d["checkSum"]["value"], "deadbeef")
         self.assertEqual(d["sequenceId"], 3)
 
+    def test_bitstream_from_none_does_not_crash(self):
+        # some cache/None paths construct Bitstream(None); it must not raise the
+        # way it used to on the membership checks in __init__.
+        b = Bitstream(None)
+        self.assertEqual(b.type, "bitstream")
+        self.assertIsNone(b.uuid)
+
 
 class TestResourcePolicy(unittest.TestCase):
 

From e9eef068414d5ba71a2a44ad2d62356be9c021b1 Mon Sep 17 00:00:00 2001
From: jm <jm@maz>
Date: Tue, 18 Aug 2026 00:29:01 +0200
Subject: [PATCH 2/2] fix: address Copilot review (handler guard, header copy,
 docstrings)

- guard the NullHandler registration so reloads/re-imports don't accumulate
  duplicate handlers
- create_bitstream: copy the session headers before adding Content-Encoding so
  it doesn't leak onto every subsequent request / across threads
- move Collection.as_dict's docstring to the first statement (it was a no-op
  string literal after code); rename the `dict` builtin-shadow in
  InProgressSubmission.as_dict

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 dspace_rest_client/client.py | 12 ++++++++----
 dspace_rest_client/models.py |  6 +++---
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/dspace_rest_client/client.py b/dspace_rest_client/client.py
index 6b88223..9cacb08 100644
--- a/dspace_rest_client/client.py
+++ b/dspace_rest_client/client.py
@@ -28,9 +28,11 @@
 
 _logger = logging.getLogger("dspace.client")
 # A library must not configure the root logger - that is the consuming
-# application's job. Attach a NullHandler so records are dropped unless the
-# application opts in to logging.
-_logger.addHandler(logging.NullHandler())
+# application's job. Attach a NullHandler (once) so records are dropped unless
+# the application opts in to logging - guarded so reloads/re-imports don't
+# accumulate duplicate handlers.
+if not any(isinstance(h, logging.NullHandler) for h in _logger.handlers):
+    _logger.addHandler(logging.NullHandler())
 
 
 def parse_json(response):
@@ -857,7 +859,9 @@ def create_bitstream(self, bundle=None, name=None, path=None, mime=None, metadat
             files = {'file': (name, fh, mime)}
             properties = {'name': name, 'metadata': metadata, 'bundleName': bundle.name}
             payload = {'properties': json.dumps(properties) + ';application/json'}
-            h = self.session.headers
+            # 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)
             h.update({'Content-Encoding': 'gzip', 'User-Agent': self.USER_AGENT})
             req = Request('POST', url, data=payload, headers=h, files=files)
             prepared_req = self.session.prepare_request(req)
diff --git a/dspace_rest_client/models.py b/dspace_rest_client/models.py
index a843bc1..4427512 100644
--- a/dspace_rest_client/models.py
+++ b/dspace_rest_client/models.py
@@ -316,11 +316,11 @@ def __init__(self, api_resource=None):
         self.type = 'collection'
 
     def as_dict(self):
-        dso_dict = super().as_dict()
         """
         Return a dict representation of this Collection, based on super with collection-specific attributes added
         @return: dict of Item for API use
         """
+        dso_dict = super().as_dict()
         collection_dict = {}
         return {**dso_dict, **collection_dict}
 
@@ -488,13 +488,13 @@ def __init__(self, api_resource):
 
     def as_dict(self):
         parent_dict = super().as_dict()
-        dict = {
+        submission_dict = {
             'lastModified': self.lastModified,
             'step': self.step,
             'sections': self.sections,
             'type': self.type
         }
-        return {**parent_dict, **dict}
+        return {**parent_dict, **submission_dict}
 
 class WorkspaceItem(InProgressSubmission):