Skip to content

Commit

Permalink
Get rid of duplicate code, fixing a big performance problem in the pr…
Browse files Browse the repository at this point in the history
…ocess
  • Loading branch information
Denis Krienbühl committed Feb 4, 2015
1 parent 566d5c8 commit 9b83dd6
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 18 deletions.
6 changes: 6 additions & 0 deletions docs/api.rst
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ Context / Registry
.. autoclass:: libres.context.context.Context
:members:

.. autoclass:: libres.context.context.ContextServicesMixin
:members:

.. autoclass:: libres.context.context.StoppableService
:members:

Database Access
---------------

Expand Down
41 changes: 41 additions & 0 deletions libres/context/context.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import libres
import threading

from cached_property import cached_property
from contextlib import contextmanager
from libres.modules import errors

Expand All @@ -22,6 +23,40 @@ def stop_service(self):
pass


class ContextServicesMixin(object):
""" Provides access methods to the context's services. Expects
the class that uses the mixin to provide self.context.
The results are cached for performance.
"""

@cached_property
def is_allocation_exposed(self):
return self.context.get_service('exposure').is_allocation_exposed

@cached_property
def generate_uuid(self):
return self.context.get_service('uuid_generator')

@cached_property
def validate_email(self):
return self.context.get_service('email_validator')

def clear_cache(self):
""" Clears the cache of the mixin. """

properties = (
'is_allocation_exposed',
'generate_uuid',
'validate_email'
)

for p in properties:
if p in self.__dict__:
del self.__dict__[p]


class Context(object):
""" Used throughout Libres, the context holds settings like the database
connection string and services like the json dumps/loads functions that
Expand All @@ -40,6 +75,12 @@ class Context(object):
master_context is used instead. In other words, the custom context
inherits from the master context.
Note that contexts not meant to be changed often. Classes talking to the
database usually cache data form the context freely. That means basically
that after changing the context you should get a fresh
:class:`~libres.db.scheduler.Scheduler` instance or call
:meth:`~.ContextServicesMixin.clear_cache`.
A context may be registered as follows::
from libres import registry
Expand Down
9 changes: 2 additions & 7 deletions libres/db/queries.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@

from libres.modules import errors, events, calendar
from libres.db.models import Allocation, Reservation, ReservedSlot
from libres.context.context import ContextServicesMixin
from libres.context.session import serialized, Serializable


class Queries(Serializable):
class Queries(Serializable, ContextServicesMixin):
""" Contains helper methods independent of the resource (as owned by
:class:`.scheduler.Scheduler`)
Expand All @@ -31,12 +32,6 @@ def all_allocations_in_range(self, start, end):
self.session.query(Allocation), start, end
)

@property
def is_allocation_exposed(self):
# XXX this is copied over from the Scheduler. A ContextServicesMixin
# could be responsible for this in a single class
return self.context.get_service('exposure').is_allocation_exposed

@staticmethod
def allocations_in_range(query, start, end):
""" Takes an allocation query and limits it to the allocations
Expand Down
13 changes: 3 additions & 10 deletions libres/db/scheduler.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from datetime import datetime, timedelta

from libres.context.context import ContextServicesMixin
from libres.context.session import serialized, Serializable
from libres.db.models import ORMBase, Allocation, ReservedSlot, Reservation
from libres.db.queries import Queries
Expand All @@ -19,7 +20,7 @@
missing = object()


class Scheduler(Serializable):
class Scheduler(Serializable, ContextServicesMixin):
""" The Scheduler is responsible for talking to the backend of the given
context to create reservations. It is the main part of the API.
"""
Expand Down Expand Up @@ -74,7 +75,7 @@ def resource(self):
on the namespace uuid defined in :ref:`settings.uuid_namespace`
"""
return self.context.get_service('uuid_generator')(self.name)
return self.generate_uuid(self.name)

def prepare_date(self, date):
return calendar.normalize_date(date, self.timezone)
Expand All @@ -92,14 +93,6 @@ def prepare_range(self, start, end):
def setup_database(self):
ORMBase.metadata.create_all(self.session.bind)

@property
def validate_email(self):
return self.context.get_service('email_validator')

@property
def is_allocation_exposed(self):
return self.context.get_service('exposure').is_allocation_exposed

def managed_allocations(self):
""" The allocations managed by this scheduler / resource. """
query = self.session.query(Allocation)
Expand Down
4 changes: 3 additions & 1 deletion libres/tests/test_scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -1523,7 +1523,7 @@ def test_search_allocations(scheduler):
assert len(scheduler.search_allocations(*daterange, days=['su'])) == 1
assert len(scheduler.search_allocations(*daterange, days=['mo'])) == 0

# make sure the exposure is taken into account
# make sure the exposure is taken into account..
class MockExposure(object):

def __init__(self, return_value):
Expand All @@ -1533,9 +1533,11 @@ def is_allocation_exposed(self, allocation):
return self.return_value

scheduler.context.set_service('exposure', lambda ctx: MockExposure(False))
scheduler.clear_cache()
assert len(scheduler.search_allocations(*daterange)) == 0

scheduler.context.set_service('exposure', lambda ctx: MockExposure(True))
scheduler.clear_cache()
assert len(scheduler.search_allocations(*daterange)) == 1

# test available only
Expand Down

0 comments on commit 9b83dd6

Please sign in to comment.