refactor: move model attribute defaults from class level to __init__

Every model class kept its attribute defaults in the class body and relied on
them as fallbacks until __init__ (or an API resource) overwrote them. That is
what made the shared-mutable-dict bug possible in the first place: a class-level
`links = {}` / `checkSum = {...}` / `sections = {}` is one object shared by
every instance, so one object's mutation leaks into all the others.

All of them are now plain instance attributes assigned in __init__ -
HALResource (type, links, embedded), AddressableHALResource (id),
ExternalDataObject (id, display, value, externalSource, metadata), DSpaceObject
(id, uuid, name, handle, lastModified, parent, metadata, type), Item (inArchive,
discoverable, withdrawn), Community / Collection / Bundle / Bitstream / Group /
User (type and their own fields), InProgressSubmission (lastModified, step,
sections, type) and EntityType (label). No model class has a class-level
attribute left; a new test asserts that, so finding 7 cannot regress.

Verified against both repositories first: nothing reads these attributes off the
class (only `Item.from_dso`, a classmethod), nothing builds a model through
`object.__new__`, and `to_json`/`to_json_pretty` - the only readers of a model's
`__dict__` - have no callers. An exhaustive old-vs-new comparison over every
class, constructor form, `dso=` copy path and `as_dict()` reports two
differences, both deliberate: `DSpaceObject.id` and `EntityType.label` now
default to None instead of raising AttributeError when absent.

Behaviour deliberately preserved: Item still stamps `type = 'item'` only when
built from an API resource. Its class-level `type = 'item'` was dead - DSpaceObject
.__init__ always assigns self.type first, shadowing it on every instance.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Author: jm

CHANGELOG.md

@@ -18,9 +18,14 @@ Date: Unreleased
 5. `get_communities`, `get_collections` and `get_bundle_by_name` return `None` on a
    failed or non-JSON response instead of raising `TypeError`.
 6. `add_metadata` / `remove_metadata` return `None` (not the client) on invalid input.
-7. Model instances no longer share class-level `links`, `embedded`, `checkSum` or
-   `sections` dicts, `Group()` / `User()` accept a `None` API resource, and
-   `Item.from_dso` / `DSpaceObject(dso=...)` deep-copy metadata instead of aliasing it.
+7. Model attribute defaults moved from the class body into `__init__` as plain
+   instance attributes, so no instance can share (or mutate) a class-level
+   `links`, `embedded`, `metadata`, `checkSum` or `sections` dict. `Group()` /
+   `User()` accept a `None` API resource, and `Item.from_dso` /
+   `DSpaceObject(dso=...)` deep-copy metadata instead of aliasing it. Side effect:
+   `id` on a `DSpaceObject` (and its subclasses) and `label` on an `EntityType`
+   now default to `None` rather than raising `AttributeError` when the API
+   resource omits them.
 8. `models.__all__` exports the full model surface re-exported by the package.
 9. Moved direct Solr support to the documented `solr` optional dependency group;
    `solr_query()` raises an actionable `RuntimeError` when the extra is missing.

dspace_rest_client/models.py

@@ -27,16 +27,20 @@
 class HALResource:
     """
     Base class to represent HAL+JSON API resources
+
+    Every attribute is a plain instance attribute assigned here in __init__.
+    Nothing is declared at class level: a class-level default is shared by all
+    instances, which for a mutable default (links, embedded, metadata,
+    checkSum, sections) means one instance's mutation leaks into every other.
     """
-    links: dict[str, Any]
-    type: str | None = 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
         """
         self._from_d: dict[str, Any] | None = None
+        self.type: str | None = None
         self.links: dict[str, Any] = {}
         self.embedded: dict[str, Any] = {}
         if api_resource is not None:
@@ -52,10 +56,10 @@ def __init__(self, api_resource: dict[str, Any] | None = None) -> None:
 
 
 class AddressableHALResource(HALResource):
-    id: Any = None
 
     def __init__(self, api_resource: dict[str, Any] | None = None) -> None:
         super().__init__(api_resource)
+        self.id: Any = None
         if api_resource is not None:
             if 'id' in api_resource:
                 self.id = api_resource['id']
@@ -68,11 +72,6 @@ class ExternalDataObject(HALResource):
     """
     Generic External Data Object as configured in DSpace's external data providers framework
     """
-    id: Any = None
-    display: Any = None
-    value: Any = None
-    externalSource: Any = None
-    metadata: dict[str, Any]
 
     def __init__(self, api_resource: dict[str, Any] | None = None) -> None:
         """
@@ -81,6 +80,10 @@ def __init__(self, api_resource: dict[str, Any] | None = None) -> None:
         """
         super().__init__(api_resource)
 
+        self.id: Any = None
+        self.display: Any = None
+        self.value: Any = None
+        self.externalSource: Any = None
         self.metadata: dict[str, Any] = {}
 
         if api_resource is not None:
@@ -114,13 +117,6 @@ class DSpaceObject(HALResource):
     operations are included in the dict returned by asDict(). Implements toJSON() as well.
     This class can be used on its own but is generally expected to be extended by other types: Item, Bitstream, etc.
     """
-    uuid: str | None = None
-    name: str | None = None
-    handle: str | None = None
-    metadata: dict[str, Any]
-    lastModified: Any = None
-    type: str | None = None
-    parent: Any = None
 
     def __init__(
         self,
@@ -133,6 +129,12 @@ def __init__(
         """
         super().__init__(api_resource)
         self.type = None
+        self.id: Any = None
+        self.uuid: str | None = None
+        self.name: str | None = None
+        self.handle: str | None = None
+        self.lastModified: Any = None
+        self.parent: Any = None
         self.metadata: dict[str, Any] = {}
 
         if dso is not None:
@@ -247,10 +249,6 @@ class Item(SimpleDSpaceObject):
     """
     Extends DSpaceObject to implement specific attributes and functions for items
     """
-    type = 'item'
-    inArchive = False
-    discoverable = False
-    withdrawn = False
 
     def __init__(
         self,
@@ -267,11 +265,20 @@ def __init__(
         else:
             super().__init__(api_resource)
 
+        # defaults for the no-api_resource case; a resource overrides them below.
+        # NB: unlike the other subclasses, Item only stamps `type` when it is
+        # built from a resource. DSpaceObject.__init__ has already set
+        # self.type = None, so the old class-level `type = 'item'` was shadowed
+        # on every instance and never readable - dropping it changes nothing.
+        self.inArchive = False
+        self.discoverable = False
+        self.withdrawn = False
+
         if api_resource is not None:
             self.type = 'item'
-            self.inArchive = api_resource['inArchive'] if 'inArchive' in api_resource else True
-            self.discoverable = api_resource['discoverable'] if 'discoverable' in api_resource else False
-            self.withdrawn = api_resource['withdrawn'] if 'withdrawn' in api_resource else False
+            self.inArchive = api_resource.get('inArchive', True)
+            self.discoverable = api_resource.get('discoverable', False)
+            self.withdrawn = api_resource.get('withdrawn', False)
 
     def get_metadata_values(self, field: str) -> list:
         """
@@ -306,7 +313,6 @@ class Community(SimpleDSpaceObject):
     """
     Extends DSpaceObject to implement specific attributes and functions for communities
     """
-    type = 'community'
 
     def __init__(self, api_resource: dict[str, Any] | None = None) -> None:
         """
@@ -321,7 +327,6 @@ class Collection(SimpleDSpaceObject):
     """
     Extends DSpaceObject to implement specific attributes and functions for collections
     """
-    type = 'collection'
 
     def __init__(self, api_resource: dict[str, Any] | None = None) -> None:
         """
@@ -336,7 +341,6 @@ class Bundle(DSpaceObject):
     """
     Extends DSpaceObject to implement specific attributes and functions for bundles
     """
-    type = 'bundle'
 
     def __init__(self, api_resource: dict[str, Any] | None = None) -> None:
         """
@@ -351,12 +355,6 @@ class Bitstream(DSpaceObject):
     """
     Extends DSpaceObject to implement specific attributes and functions for bundles
     """
-    type = 'bitstream'
-    # Bitstream has a few extra fields specific to file storage
-    bundleName: str | None = None
-    sizeBytes: int | None = None
-    checkSum: dict[str, Any]
-    sequenceId: int | None = None
 
     def __init__(self, api_resource: dict[str, Any] | None = None) -> None:
         """
@@ -365,10 +363,11 @@ def __init__(self, api_resource: dict[str, Any] | None = None) -> None:
         """
         super().__init__(api_resource)
         self.type = 'bitstream'
-        self.bundleName = None
-        self.sizeBytes = None
-        self.checkSum = {'checkSumAlgorithm': 'MD5', 'value': None}
-        self.sequenceId = None
+        # Bitstream has a few extra fields specific to file storage
+        self.bundleName: str | None = None
+        self.sizeBytes: int | None = None
+        self.checkSum: dict[str, Any] = {'checkSumAlgorithm': 'MD5', 'value': None}
+        self.sequenceId: int | None = None
         api_resource = api_resource or {}
         if 'bundleName' in api_resource:
             self.bundleName = api_resource['bundleName']
@@ -394,9 +393,6 @@ class Group(DSpaceObject):
     """
     Extends DSpaceObject to implement specific attributes and methods for groups (aka. EPersonGroups)
     """
-    type = 'group'
-    name = None
-    permanent = False
 
     def __init__(self, api_resource: dict[str, Any] | None = None) -> None:
         """
@@ -427,14 +423,6 @@ class User(SimpleDSpaceObject):
     """
     Extends DSpaceObject to implement specific attributes and methods for users (aka. EPersons)
     """
-    type = 'user'
-    name = None
-    netid = None
-    lastActive = None
-    canLogIn = False
-    email = None
-    requireCertificate = False
-    selfRegistered = False
 
     def __init__(self, api_resource: dict[str, Any] | None = None) -> None:
         """
@@ -479,15 +467,11 @@ def as_dict(self) -> dict[str, Any]:
 
 
 class InProgressSubmission(AddressableHALResource):
-    lastModified: Any = None
-    step: Any = None
-    sections: dict[str, Any]
-    type: str | None = None
 
     def __init__(self, api_resource: dict[str, Any] | None = None) -> None:
         super().__init__(api_resource)
-        self.lastModified = None
-        self.step = None
+        self.lastModified: Any = None
+        self.step: Any = None
         self.sections: dict[str, Any] = {}
         self.type = None
         api_resource = api_resource or {}
@@ -524,6 +508,7 @@ class EntityType(AddressableHALResource):
 
     def __init__(self, api_resource: dict[str, Any]) -> None:
         super().__init__(api_resource)
+        self.label: Any = None
         if 'label' in api_resource:
             self.label = api_resource['label']
         if 'type' in api_resource:

tests/test_models.py

@@ -10,6 +10,7 @@
 import unittest
 
 import _helpers  # noqa: F401  (bootstraps sys.path for direct runs)
+from dspace_rest_client import models as models_module
 from dspace_rest_client.models import (
     DSpaceObject, HALResource, Item, Community, Collection, Bundle, Bitstream,
     Group, User, InProgressSubmission, ResourcePolicy)
@@ -90,6 +91,39 @@ def test_submission_sections_are_isolated_between_instances(self):
 
         self.assertNotIn("license", second.sections)
 
+    def test_no_model_declares_attribute_defaults_at_class_level(self):
+        """The structural guarantee behind the three tests above.
+
+        A class-level default is shared by every instance, so a mutable one
+        (links, metadata, checkSum, sections) lets one object's mutation leak
+        into all the others. Every attribute is assigned in __init__ instead;
+        this test fails if anyone reintroduces a class-body default.
+        """
+        for cls in vars(models_module).values():
+            if not isinstance(cls, type) or cls.__module__ != models_module.__name__:
+                continue
+            declared = sorted(
+                name for name, value in vars(cls).items()
+                if not name.startswith("__")
+                and not isinstance(value, (property, classmethod, staticmethod))
+                and not callable(value))
+            with self.subTest(model=cls.__name__):
+                self.assertEqual(declared, [], (
+                    f"{cls.__name__} declares {declared} at class level; "
+                    "move the default into __init__ as self.<attr> = ..."))
+
+    def test_every_model_instance_defines_the_attributes_its_dict_reports(self):
+        # nothing may fall back to a class attribute: what __init__ assigns is
+        # the whole public surface of the instance.
+        for cls, args in ((HALResource, ()), (DSpaceObject, ()), (Item, ()),
+                          (Community, ()), (Collection, ()), (Bundle, ()),
+                          (Bitstream, ()), (Group, ()), (User, ()),
+                          (InProgressSubmission, ({},))):
+            with self.subTest(model=cls.__name__):
+                instance = cls(*args)
+                for name in ("type", "links", "embedded"):
+                    self.assertIn(name, vars(instance))
+
 
 class TestCommunityCollection(unittest.TestCase):