Skip to content

Commit 719a831

Browse files
authored
Refactor Translator to use less global state (#165)
In the past, there was one `Translator`, which was a global singleton, and would be permanently modified any time a new object class with an `_item_type` was created. Recently, in v2.0.0a1, we tried out a change where the global translator would be modified any time a new object class with an `_item_type` OR with a baseclass with an `_item_type` was created. This means that every subclass operation mutated global state. We thought this would make it easier for developers to add their own classes to the SDK. In retrospect, it makes it much harder to write tests (because any temporary subclasses created, including by a mock library, will modify global state for the rest of the test run), and makes it impossible to intentionally create a subclass that shouldn't be registered in the translator. So we are reverting this behavior for v2.0.0a2. Furthermore, this adds the ability to create non-global translators (which are, by default, used on `BoxSession` objects), and the ability to add non-global registrations to these non-global translators. This is now the publicly recommended way for developers to register types, outside of the SDK itself. For now, the old mechanism of implicitly registering the official SDK classes with `_item_type` is retained. But we can experiment with the new system, and see if we prefer to switch to the explicit registration, and delete the implicit registration system, in v2.0.0a3. Also fix a bug that I discovered in `ExtendableEnumMeta`.
1 parent e325da5 commit 719a831

26 files changed

+470
-194
lines changed

.pylintrc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ max-branches=12
226226
max-statements=50
227227

228228
# Maximum number of parents for a class (see R0901).
229-
max-parents=7
229+
max-parents=14
230230

231231
# Maximum number of attributes for a class (see R0902).
232232
max-attributes=15

HISTORY.rst

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,17 @@ Release History
2424

2525
**Features**
2626

27-
- Added ability to create custom subclasses of SDK objects with ``_item_type`` defined.
27+
- Added more flexibility to the object translation system:
28+
29+
- Can create non-global ``Translator`` instances, which can extend or
30+
not-extend the global default ``Translator``.
31+
- Can initialize ``BoxSession`` with a custom ``Translator``.
32+
- Can register custom subclasses on the ``Translator`` which is associated
33+
with a ``BoxSession`` or a ``Client``.
34+
- All translation of API responses now use the ``Translator`` that is
35+
referenced by the ``BoxSession``, instead of directly using the global
36+
default ``Translator``.
37+
2838
- Added an ``Event`` class.
2939

3040
**Other**
@@ -36,6 +46,7 @@ Release History
3646
``BaseObject`` is the parent of all objects that are a part of the REST API. Another subclass of
3747
``BaseAPIJSONObject``, ``APIJSONObject``, was created to represent pseudo-smart objects such as ``Event`` that are not
3848
directly accessible through an API endpoint.
49+
- Fixed an exception that was being raised from ``ExtendableEnumMeta.__dir__()``.
3950

4051
1.5.3 (2016-05-26)
4152
++++++++++++++++++

boxsdk/client/client.py

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
from ..object.search import Search
1212
from ..object.events import Events
1313
from ..util.shared_link import get_shared_link_header
14-
from ..util.translator import Translator
1514

1615

1716
class Client(Cloneable):
@@ -62,6 +61,14 @@ def session(self):
6261
"""
6362
return self._session
6463

64+
@property
65+
def translator(self):
66+
"""The translator used for translating Box API JSON responses into `BaseAPIJSONObject` smart objects.
67+
68+
:rtype: :class:`Translator`
69+
"""
70+
return self._session.translator
71+
6572
def folder(self, folder_id):
6673
"""
6774
Initialize a :class:`Folder` object, whose box id is folder_id.
@@ -75,7 +82,7 @@ def folder(self, folder_id):
7582
:rtype:
7683
:class:`Folder`
7784
"""
78-
return Translator().translate('folder')(session=self._session, object_id=folder_id)
85+
return self.translator.translate('folder')(session=self._session, object_id=folder_id)
7986

8087
def file(self, file_id):
8188
"""
@@ -90,7 +97,7 @@ def file(self, file_id):
9097
:rtype:
9198
:class:`File`
9299
"""
93-
return Translator().translate('file')(session=self._session, object_id=file_id)
100+
return self.translator.translate('file')(session=self._session, object_id=file_id)
94101

95102
def user(self, user_id='me'):
96103
"""
@@ -105,7 +112,7 @@ def user(self, user_id='me'):
105112
:rtype:
106113
:class:`User`
107114
"""
108-
return Translator().translate('user')(session=self._session, object_id=user_id)
115+
return self.translator.translate('user')(session=self._session, object_id=user_id)
109116

110117
def group(self, group_id):
111118
"""
@@ -120,7 +127,7 @@ def group(self, group_id):
120127
:rtype:
121128
:class:`Group`
122129
"""
123-
return Translator().translate('group')(session=self._session, object_id=group_id)
130+
return self.translator.translate('group')(session=self._session, object_id=group_id)
124131

125132
def collaboration(self, collab_id):
126133
"""
@@ -135,7 +142,7 @@ def collaboration(self, collab_id):
135142
:rtype:
136143
:class:`Collaboration`
137144
"""
138-
return Translator().translate('collaboration')(session=self._session, object_id=collab_id)
145+
return self.translator.translate('collaboration')(session=self._session, object_id=collab_id)
139146

140147
@api_call
141148
def users(self, limit=None, offset=0, filter_term=None):
@@ -167,7 +174,7 @@ def users(self, limit=None, offset=0, filter_term=None):
167174
params['filter_term'] = filter_term
168175
box_response = self._session.get(url, params=params)
169176
response = box_response.json()
170-
user_class = Translator().translate('user')
177+
user_class = self.translator.translate('user')
171178
return [user_class(
172179
session=self._session,
173180
object_id=item['id'],
@@ -256,7 +263,7 @@ def group_membership(self, group_membership_id):
256263
:rtype:
257264
:class:`GroupMembership`
258265
"""
259-
return Translator().translate('group_membership')(
266+
return self.translator.translate('group_membership')(
260267
session=self._session,
261268
object_id=group_membership_id,
262269
)
@@ -274,7 +281,7 @@ def groups(self):
274281
url = '{0}/groups'.format(API.BASE_API_URL)
275282
box_response = self._session.get(url)
276283
response = box_response.json()
277-
group_class = Translator().translate('group')
284+
group_class = self.translator.translate('group')
278285
return [group_class(
279286
session=self._session,
280287
object_id=item['id'],
@@ -303,7 +310,7 @@ def create_group(self, name):
303310
}
304311
box_response = self._session.post(url, data=json.dumps(body_attributes))
305312
response = box_response.json()
306-
return Translator().translate('group')(
313+
return self.translator.translate('group')(
307314
session=self._session,
308315
object_id=response['id'],
309316
response_object=response,
@@ -334,7 +341,7 @@ def get_shared_item(self, shared_link, password=None):
334341
'{0}/shared_items'.format(API.BASE_API_URL),
335342
headers=get_shared_link_header(shared_link, password),
336343
).json()
337-
return Translator().translate(response['type'])(
344+
return self.translator.translate(response['type'])(
338345
session=self._session.with_shared_link(shared_link, password),
339346
object_id=response['id'],
340347
response_object=response,
@@ -389,7 +396,7 @@ def create_user(self, name, login=None, **user_attributes):
389396
user_attributes['is_platform_access_only'] = True
390397
box_response = self._session.post(url, data=json.dumps(user_attributes))
391398
response = box_response.json()
392-
return Translator().translate('user')(
399+
return self.translator.translate('user')(
393400
session=self._session,
394401
object_id=response['id'],
395402
response_object=response,

boxsdk/object/base_api_json_object.py

Lines changed: 52 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,22 +9,68 @@
99

1010
class BaseAPIJSONObjectMeta(type):
1111
"""
12-
Metaclass for Box API objects. Registers classes so that API responses can be translated to the correct type.
13-
Relies on the _item_type field defined on the classes to match the type property of the response json.
14-
But the type-class mapping will only be registered if the module of the class is imported.
15-
So it's also important to add the module name to __all__ in object/__init__.py.
12+
Metaclass for Box API objects.
13+
14+
Registers classes with the default translator, so that API responses can be
15+
translated to the correct type. This relies on the _item_type field, which
16+
must be defined in the class's namespace dict (and must be re-defined, in
17+
order to register a custom subclass), to match the 'type' field of the
18+
response json. But the type-class mapping will only be registered if the
19+
module of the class is imported.
20+
21+
For example, events returned from the API look like
22+
23+
.. code-block:: json
24+
25+
{'type': 'event', ...}
26+
27+
so a class for that type could be created and registered with the default
28+
translator like this:
29+
30+
.. code-block:: python
31+
32+
class Event(BaseAPIJSONObject):
33+
_item_type = 'event'
34+
...
35+
36+
NOTE: The default translator registration functionality is a private
37+
implementation detail of the SDK, to make it easy to register the default
38+
API object classes with the default translator. For convenience and
39+
backwards-compatability, developers are allowed to re-define the _item_type
40+
field in their own custom subclasses in order to take advantage of this
41+
functionality, but are encouraged not to. Since this is a private
42+
implementation detail, it may change or be removed in any major or minor
43+
release. Additionally, it has the usual hazards of mutable global state.
44+
The supported and recommended ways for registering custom subclasses are:
45+
46+
- Constructing a new :class:`Translator`, calling `Translator.register()`
47+
as necessary, and passing it to the :class:`BoxSession` constructor.
48+
- Calling `session.translator.register()` on an existing
49+
:class:`BoxSession`.
50+
- Calling `client.translator.register()` on an existing :class:`Client`.
1651
"""
52+
1753
def __init__(cls, name, bases, attrs):
1854
super(BaseAPIJSONObjectMeta, cls).__init__(name, bases, attrs)
19-
item_type = getattr(cls, '_item_type', None)
55+
item_type = attrs.get('_item_type', None)
2056
if item_type is not None:
21-
Translator().register(item_type, cls)
57+
Translator._default_translator.register(item_type, cls) # pylint:disable=protected-access
2258

2359

2460
@six.add_metaclass(BaseAPIJSONObjectMeta)
2561
class BaseAPIJSONObject(object):
2662
"""Base class containing basic logic shared between true REST objects and other objects (such as an Event)"""
2763

64+
# :attr _item_type:
65+
# (protected) The Box resource type that this class represents.
66+
# For API object/resource classes this should equal the expected value
67+
# of the 'type' field in API JSON responses. Otherwise, this should be
68+
# `None`.
69+
# :type _item_type: `unicode` or `None`
70+
#
71+
# NOTE: When defining a leaf class with an _item_type in this SDK, it's
72+
# also important to add the module name to __all__ in object/__init__.py,
73+
# so that it will be imported and registered with the default translator.
2874
_item_type = None
2975

3076
def __init__(self, response_object=None, **kwargs):

boxsdk/object/base_endpoint.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,14 @@ def session(self):
3232
"""
3333
return self._session
3434

35+
@property
36+
def translator(self):
37+
"""The translator used for translating Box API JSON responses into `BaseAPIJSONObject` smart objects.
38+
39+
:rtype: :class:`Translator`
40+
"""
41+
return self._session.translator
42+
3543
def get_url(self, endpoint, *args):
3644
"""
3745
Return the URL used to access the endpoint.

boxsdk/object/base_object.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55

66
from .base_endpoint import BaseEndpoint
77
from .base_api_json_object import BaseAPIJSONObject
8-
from ..util.translator import Translator
98
from ..util.api_call_decorator import api_call
109

1110

@@ -208,7 +207,7 @@ def _paging_wrapper(self, url, starting_index, limit, factory=None):
208207
for index_in_current_page, item in enumerate(response['entries']):
209208
instance_factory = factory
210209
if not instance_factory:
211-
instance_factory = Translator().translate(item['type'])
210+
instance_factory = self.translator.translate(item['type'])
212211
instance = instance_factory(self._session, item['id'], item)
213212
yield instance, current_page_size, index_in_current_page
214213

boxsdk/object/events.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
from ..util.enum import ExtendableEnumMeta
1010
from ..util.lru_cache import LRUCache
1111
from ..util.text_enum import TextEnum
12-
from ..util.translator import Translator
1312

1413

1514
# pylint:disable=too-many-ancestors
@@ -94,7 +93,7 @@ def get_events(self, limit=100, stream_position=0, stream_type=UserEventsStreamT
9493
box_response = self._session.get(url, params=params)
9594
response = box_response.json().copy()
9695
if 'entries' in response:
97-
response['entries'] = [Translator().translate(item['type'])(item) for item in response['entries']]
96+
response['entries'] = [self.translator.translate(item['type'])(item) for item in response['entries']]
9897
return response
9998

10099
@api_call

boxsdk/object/folder.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
from boxsdk.object.user import User
1212
from boxsdk.util.api_call_decorator import api_call
1313
from boxsdk.util.text_enum import TextEnum
14-
from boxsdk.util.translator import Translator
1514

1615

1716
class FolderSyncState(TextEnum):
@@ -153,7 +152,7 @@ def get_items(self, limit, offset=0, fields=None):
153152
params['fields'] = ','.join(fields)
154153
box_response = self._session.get(url, params=params)
155154
response = box_response.json()
156-
return [Translator().translate(item['type'])(self._session, item['id'], item) for item in response['entries']]
155+
return [self.translator.translate(item['type'])(self._session, item['id'], item) for item in response['entries']]
157156

158157
@api_call
159158
def upload_stream(
@@ -218,7 +217,7 @@ def upload_stream(
218217
box_response = self._session.post(url, data=data, files=files, expect_json_response=False)
219218
file_response = box_response.json()['entries'][0]
220219
file_id = file_response['id']
221-
return Translator().translate(file_response['type'])(
220+
return self.translator.translate(file_response['type'])(
222221
session=self._session,
223222
object_id=file_id,
224223
response_object=file_response,
@@ -364,7 +363,7 @@ def add_collaborator(self, collaborator, role, notify=False):
364363
box_response = self._session.post(url, expect_json_response=True, data=data, params=params)
365364
collaboration_response = box_response.json()
366365
collab_id = collaboration_response['id']
367-
return Translator().translate(collaboration_response['type'])(
366+
return self.translator.translate(collaboration_response['type'])(
368367
session=self._session,
369368
object_id=collab_id,
370369
response_object=collaboration_response,

boxsdk/object/group.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66

77
from .base_object import BaseObject
88
from ..config import API
9-
from ..util.translator import Translator
109
from ..util.api_call_decorator import api_call
1110

1211

@@ -48,7 +47,7 @@ def membership(self, starting_index=0, limit=100, include_page_info=False):
4847
"""
4948
url = self.get_url('memberships')
5049

51-
membership_factory = partial(Translator().translate("group_membership"), group=self)
50+
membership_factory = partial(self.translator.translate("group_membership"), group=self)
5251
for group_membership_tuple in self._paging_wrapper(url, starting_index, limit, membership_factory):
5352
if include_page_info:
5453
yield group_membership_tuple
@@ -83,4 +82,4 @@ def add_member(self, user, role):
8382
box_response = self._session.post(url, data=json.dumps(body_attributes))
8483
response = box_response.json()
8584

86-
return Translator().translate(response['type'])(self._session, response['id'], response, user=user, group=self)
85+
return self.translator.translate(response['type'])(self._session, response['id'], response, user=user, group=self)

boxsdk/object/group_membership.py

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
from __future__ import unicode_literals, absolute_import
44

55
from .base_object import BaseObject
6-
from ..util.translator import Translator
76

87

98
class GroupMembership(BaseObject):
@@ -70,8 +69,8 @@ def _init_user_and_group_instances(session, response_object, user, group):
7069
user_info = response_object.get('user')
7170
group_info = response_object.get('group')
7271

73-
user = user or Translator().translate(user_info['type'])(session, user_info['id'], user_info)
74-
group = group or Translator().translate(group_info['type'])(session, group_info['id'], group_info)
72+
user = user or session.translator.translate(user_info['type'])(session, user_info['id'], user_info)
73+
group = group or session.translator.translate(group_info['type'])(session, group_info['id'], group_info)
7574

7675
return user, group
7776

0 commit comments

Comments
 (0)