Skip to content
This repository has been archived by the owner on Mar 28, 2019. It is now read-only.

Commit

Permalink
Pass current request along every backends methods.
Browse files Browse the repository at this point in the history
**Rationale*:

This small refactor paves the way for:

* persisting backend connections on current request (instead of pull/push from queue for each operation performed with one request, e.g batched requests)
* having one transaction per request (with a tween to connect / commit). Particularly interesting for batched requests. (see Kinto/kinto#194)
* have one event per request, with every created/updated/deleted records of a batch

Of course, it may seem ugly to pass the request, but it is advocated [in Pyramid docs](http://pyramid.readthedocs.org/en/1.5-branch/narr/threadlocals.html#why-you-shouldn-t-abuse-thread-locals):

> get_current_request function should never be called because it’s “easier” or “more elegant” to think about calling it **than to pass a request through a series of function calls when creating some API design**. Your application should instead almost certainly pass data derived from the request around rather than relying on being able to call this function to obtain the request in places that actually have no business knowing about it.

We could pass a derived info like `request.bound_data`, but I like the fact to pass the request object.

Especially because it gives a lot of freedom in possible backend implementation (*imagine a box.com storage backend where original request headers are manipulated before being sent to third party service...*)
  • Loading branch information
leplatrem committed Oct 20, 2015
1 parent c076368 commit 543643f
Show file tree
Hide file tree
Showing 15 changed files with 216 additions and 148 deletions.
34 changes: 24 additions & 10 deletions cliquet/cache/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,66 +19,80 @@ def initialize_schema(self):
"""
raise NotImplementedError

def flush(self):
"""Delete every values."""
def flush(self, request=None):
"""Delete every values.
:param request: Optional current request object.
:type request: :class:`~pyramid:pyramid.request.Request`
"""
raise NotImplementedError

def ping(self, request):
"""Test that cache backend is operationnal.
:param request: current request object
:param request: Optional current request object.
:type request: :class:`~pyramid:pyramid.request.Request`
:returns: ``True`` is everything is ok, ``False`` otherwise.
:rtype: bool
"""
try:
if random.random() < _HEARTBEAT_DELETE_RATE:
self.delete(_HEARTBEAT_KEY)
self.delete(_HEARTBEAT_KEY, request=request)
else:
self.set(_HEARTBEAT_KEY, 'alive', _HEARTBEAT_TTL_SECONDS)
self.set(_HEARTBEAT_KEY, 'alive', _HEARTBEAT_TTL_SECONDS,
request=request)
return True
except:
return False

def ttl(self, key):
def ttl(self, key, request=None):
"""Obtain the expiration value of the specified `key`.
:param str key: key
:param request: Optional current request object.
:type request: :class:`~pyramid:pyramid.request.Request`
:returns: number of seconds or negative if no TTL.
:rtype: float
"""
raise NotImplementedError

def expire(self, key, ttl):
def expire(self, key, ttl, request=None):
"""Set the expiration value `ttl` for the specified `key`.
:param str key: key
:param float ttl: number of seconds
:param request: Optional current request object.
:type request: :class:`~pyramid:pyramid.request.Request`
"""
raise NotImplementedError

def set(self, key, value, ttl=None):
def set(self, key, value, ttl=None, request=None):
"""Store a value with the specified `key`. If `ttl` is provided,
set an expiration value.
:param str key: key
:param str value: value to store
:param float ttl: expire after number of seconds
:param request: Optional current request object.
:type request: :class:`~pyramid:pyramid.request.Request`
"""
raise NotImplementedError

def get(self, key):
def get(self, key, request=None):
"""Obtain the value of the specified `key`.
:param str key: key
:param request: Optional current request object.
:type request: :class:`~pyramid:pyramid.request.Request`
:returns: the stored value or None if missing.
:rtype: str
"""
raise NotImplementedError

def delete(self, key):
def delete(self, key, request=None):
"""Delete the value of the specified `key`.
:param str key: key
:param request: Optional current request object.
:type request: :class:`~pyramid:pyramid.request.Request`
"""
raise NotImplementedError
12 changes: 6 additions & 6 deletions cliquet/cache/memory.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,32 +20,32 @@ def initialize_schema(self):
# Nothing to do.
pass

def flush(self):
def flush(self, request=None):
self._ttl = {}
self._store = {}

def ttl(self, key):
def ttl(self, key, request=None):
ttl = self._ttl.get(key)
if ttl is not None:
return (ttl - utils.msec_time()) / 1000.0
return -1

def expire(self, key, ttl):
def expire(self, key, ttl, request=None):
self._ttl[key] = utils.msec_time() + int(ttl * 1000.0)

def set(self, key, value, ttl=None):
def set(self, key, value, ttl=None, request=None):
self._store[key] = value
if ttl is not None:
self.expire(key, ttl)

def get(self, key):
def get(self, key, request=None):
current = utils.msec_time()
expired = [k for k, v in self._ttl.items() if current > v]
for key in expired:
self.delete(key)
return self._store.get(key)

def delete(self, key):
def delete(self, key, request=None):
self._ttl.pop(key, None)
self._store.pop(key, None)

Expand Down
10 changes: 5 additions & 5 deletions cliquet/cache/postgresql/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def flush(self):
cursor.execute(query)
logger.debug('Flushed PostgreSQL cache tables')

def ttl(self, key):
def ttl(self, key, request=None):
query = """
SELECT EXTRACT(SECOND FROM (ttl - now())) AS ttl
FROM cache
Expand All @@ -81,14 +81,14 @@ def ttl(self, key):
return cursor.fetchone()['ttl']
return -1

def expire(self, key, ttl):
def expire(self, key, ttl, request=None):
query = """
UPDATE cache SET ttl = sec2ttl(%s) WHERE key = %s;
"""
with self.connect() as cursor:
cursor.execute(query, (ttl, key,))

def set(self, key, value, ttl=None):
def set(self, key, value, ttl=None, request=None):
query = """
WITH upsert AS (
UPDATE cache SET value = %(value)s, ttl = sec2ttl(%(ttl)s)
Expand All @@ -102,7 +102,7 @@ def set(self, key, value, ttl=None):
with self.connect() as cursor:
cursor.execute(query, dict(key=key, value=value, ttl=ttl))

def get(self, key):
def get(self, key, request=None):
purge = "DELETE FROM cache WHERE ttl IS NOT NULL AND now() > ttl;"
query = "SELECT value FROM cache WHERE key = %s;"
with self.connect() as cursor:
Expand All @@ -112,7 +112,7 @@ def get(self, key):
value = cursor.fetchone()['value']
return json.loads(value)

def delete(self, key):
def delete(self, key, request=None):
query = "DELETE FROM cache WHERE key = %s"
with self.connect() as cursor:
cursor.execute(query, (key,))
Expand Down
12 changes: 6 additions & 6 deletions cliquet/cache/redis.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,34 +38,34 @@ def initialize_schema(self):
pass

@wrap_redis_error
def flush(self):
def flush(self, request=None):
self._client.flushdb()

@wrap_redis_error
def ttl(self, key):
def ttl(self, key, request=None):
return self._client.ttl(key)

@wrap_redis_error
def expire(self, key, ttl):
def expire(self, key, ttl, request=None):
self._client.pexpire(key, int(ttl * 1000))

@wrap_redis_error
def set(self, key, value, ttl=None):
def set(self, key, value, ttl=None, request=None):
value = json.dumps(value)
if ttl:
self._client.psetex(key, int(ttl * 1000), value)
else:
self._client.set(key, value)

@wrap_redis_error
def get(self, key):
def get(self, key, request=None):
value = self._client.get(key)
if value:
value = value.decode('utf-8')
return json.loads(value)

@wrap_redis_error
def delete(self, key):
def delete(self, key, request=None):
self._client.delete(key)


Expand Down
18 changes: 9 additions & 9 deletions cliquet/collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class Collection(object):
"""Name of `deleted` field in deleted records"""

def __init__(self, storage, id_generator=None, collection_id='',
parent_id='', auth=None):
parent_id='', request=None):
"""
:param storage: an instance of storage
:type storage: :class:`cliquet.storage.Storage`
Expand All @@ -34,7 +34,7 @@ def __init__(self, storage, id_generator=None, collection_id='',
self.id_generator = id_generator
self.parent_id = parent_id
self.collection_id = collection_id
self.auth = auth
self.request = request

def timestamp(self, parent_id=None):
"""Fetch the collection current timestamp.
Expand All @@ -46,7 +46,7 @@ def timestamp(self, parent_id=None):
return self.storage.collection_timestamp(
collection_id=self.collection_id,
parent_id=parent_id,
auth=self.auth)
request=self.request)

def get_records(self, filters=None, sorting=None, pagination_rules=None,
limit=None, include_deleted=False, parent_id=None):
Expand Down Expand Up @@ -96,7 +96,7 @@ def get_records(self, filters=None, sorting=None, pagination_rules=None,
id_field=self.id_field,
modified_field=self.modified_field,
deleted_field=self.deleted_field,
auth=self.auth)
request=self.request)
return records, total_records

def delete_records(self, filters=None, parent_id=None):
Expand All @@ -121,7 +121,7 @@ def delete_records(self, filters=None, parent_id=None):
id_field=self.id_field,
modified_field=self.modified_field,
deleted_field=self.deleted_field,
auth=self.auth)
request=self.request)

def get_record(self, record_id, parent_id=None):
"""Fetch current view related record, and raise 404 if missing.
Expand All @@ -138,7 +138,7 @@ def get_record(self, record_id, parent_id=None):
object_id=record_id,
id_field=self.id_field,
modified_field=self.modified_field,
auth=self.auth)
request=self.request)

def create_record(self, record, parent_id=None, unique_fields=None):
"""Create a record in the collection.
Expand Down Expand Up @@ -169,7 +169,7 @@ def create_record(self, record):
unique_fields=unique_fields,
id_field=self.id_field,
modified_field=self.modified_field,
auth=self.auth)
request=self.request)

def update_record(self, record, parent_id=None, unique_fields=None):
"""Update a record in the collection.
Expand Down Expand Up @@ -202,7 +202,7 @@ def update_record(self, record, parent_id=None,unique_fields=None):
unique_fields=unique_fields,
id_field=self.id_field,
modified_field=self.modified_field,
auth=self.auth)
request=self.request)

def delete_record(self, record, parent_id=None):
"""Delete a record in the collection.
Expand Down Expand Up @@ -232,7 +232,7 @@ def delete_record(self, record):
id_field=self.id_field,
modified_field=self.modified_field,
deleted_field=self.deleted_field,
auth=self.auth)
request=self.request)


class ProtectedCollection(Collection):
Expand Down
Loading

0 comments on commit 543643f

Please sign in to comment.