TireSwingSoftware/openassign-server — Issue #58: Ideas for improving tests that use auth tokens
Author: jc0n
Created: 2011-12-09 20:00:45+00:00
State: open
Labels: behavior

I came up with two ideas for reducing the amount of token passing required for the service methods and improving readability of manager tests. Conceptually, it makes sense to think of the service methods as operations on a context where the auth_token defines that context. It follows that the context should be established as a pre-condition to those operations and all subsequent operations perform within that context. In most cases the same auth token is used for multiple operations.

#### First Idea
Accept an auth_token as a constructor argument in `ObjectManager` that way unless one is specified as a keyword argument to the service method, the default (constructor argument) is used. Usage of the managers would then look a bit like..

```python
user_manager = managers.UserManager(default_token)
user = user_manager.create('username', 'first_name', ... ) # create with user privileges
user_manager.update(user.id, {'first_name': 'joe'})
user_manager.delete(user.id)

admin_user_manager = managers.UserManager(admin_token)
admin_user_manager.create('otheruser', 'other_name'...) # create with admin privileges
```

#### Second idea
Slightly more "magic" than above but has the benefit of being a drop in solution that doesn't have to be applied everywhere. It also wont require changes in object managers; only the call site of the service methods. The other benefit is that you can have the same manager instance decoupled from the auth token. My thoughts on this were that test cases which *must* perform an operation (ie. to seed test data) setup and call `self.admin_abcxyz_manager` which would be wrapped with an admin auth token.

I like this because it also helps with readability of the tests which could be drastically improved so that we know what we're actually testing. When you see a call to `admin_abcxyz_manager` you *know* it must succeed and that we are not testing the operation with this call. You also know that all calls to `abcxyz_manager` no longer assume the auth token is for admin which most currently do. The idea stems from cleanup I am working on for #39 to re-use all of the existing tests for verifying different roles. The tests will be written in a way which makes no assumption about the auth token being for the admin.

```python

class ManagerAuthTokenWrapper(object):
"""
Wrap ObjectManager methods to automatically provide a specified auth token
"""

WRAP_METHODS = frozenset(('create', 'batch_create', 'update', 'get_filtered'))

def __init__(self, manager, token_getter):
assert isinstance(manager, ObjectManager)
assert callable(token_getter)
self.manager = manager
self.token_getter = token_getter

def _wrapped_method(self, method):
@functools.wraps(method)
def _wrapper(*args, **kwargs):
if args and isinstance(args[0], facade.models.AuthToken):
return method(*args, **kwargs)
token = kwargs.pop('auth_token', self.token_getter())
return method(token, *args, **kwargs)

return _wrapper

def __getattr__(self, name):
attr = getattr(self.manager, name)
if name in self.WRAP_METHODS:
return self._wrapped_method(attr)
else:
return attr
```

Usage of this approach would make code like...

```python
user_manager = ManagerAuthTokenWrapper(user_manager, default_token)
admin_user_manager = ManagerAuthTokenWrapper(user_manager, admin_token)

user = user_manager.create('username', 'first_name', ... ) # create with user privileges
user_manager.update(user.id, {'first_name': 'joe'})
user_manager.delete(user.id)
```