Skip to content
This repository has been archived by the owner on Jul 24, 2018. It is now read-only.

Commit

Permalink
Merge pull request #6 from fedora-infra/feature/refactor
Browse files Browse the repository at this point in the history 
Refactor the main "recipients" api to be much easier to cache.
  • Loading branch information
@ralphbean
ralphbean committed May 15, 2014
2 parents 096c303 + c917681 commit a3db7d7
Show file tree
Hide file tree
Showing 3 changed files with 149 additions and 146 deletions.
77 changes: 61 additions & 16 deletions fmn/lib/__init__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import fmn.lib.models

import functools
import inspect
import logging
import re
Expand All @@ -10,6 +11,10 @@
import docutils.examples
import markupsafe

import fedmsg.utils

from collections import defaultdict

try:
from collections import OrderedDict
except ImportError:
Expand All @@ -22,35 +27,76 @@
gcm_regex = r'^[\w-]+$'


def recipients(session, config, valid_paths, message):
def recipients(preferences, message, valid_paths, config):
""" The main API function.
Accepts a fedmsg message as an argument.
Returns a dict mapping context names to lists of recipients.
"""

res = {}
results = defaultdict(list)
notified = set()

for preference in preferences:
user = preference['user']
context = preference['context']
if (user['openid'], context['name']) in notified:
continue

for context in session.query(fmn.lib.models.Context).all():
res[context.name] = recipients_for_context(
session, config, valid_paths, context, message)
filters = preference['filters']
for filter in preference['filters']:
if matches(filter, message, valid_paths, config):
for detail_value in preference['detail_values']:
results[context['name']].append({
'user': user['openid'],
context['detail_name']: detail_value,
'filter_name': filter['name'],
'filter_id': filter['id'],
})
notified.add((user['openid'], context['name']))
break

return res
return results


def recipients_for_context(session, config, valid_paths, context, message):
""" Returns the recipients for a given fedmsg message and stated context.
def matches(filter, message, valid_paths, config):
""" Returns True if the given filter matches the given message. """

Context may be either the name of a context or an instance of
fmn.lib.models.Context.
"""
if not filter['rules']:
return False

for rule in filter['rules']:
code_path = rule['code_path']
arguments = rule['arguments']

if isinstance(context, basestring):
context = session.query(fmn.lib.models.Context)\
.filter_by(name=context).one()
# First, validate the rule before doing anything
fmn.lib.models.Rule.validate_code_path(valid_paths, code_path)

return context.recipients(session, config, valid_paths, message)
# Next, instantiate it into a python callable.
# (This is a bit of a misnomer though, load_class can load anything.)
fn = fedmsg.utils.load_class(str(code_path))
# And, partially apply our keyword arguments.
fn = functools.partial(fn, **arguments)

try:
result = fn(config, message)
if result:
return True
except Exception as e:
log.exception(e)

# Then no rule on this filter matched..
return False


def load_preferences(session, config, valid_paths):
""" Every rule for every filter for every context for every user.
This is an expensive query that loads, practically, the whole database.
"""
preferences = session.query(fmn.lib.models.Preference).all()
return [preference.__json__() for preference in preferences]


def load_rules(root='fmn.rules'):
Expand All @@ -63,7 +109,6 @@ def load_rules(root='fmn.rules'):
obj = getattr(module, name)
if not callable(obj):
continue
log.debug("Found rule %r %r" % (name, obj))

doc = inspect.getdoc(obj)

Expand Down
147 changes: 44 additions & 103 deletions fmn/lib/models.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import pkg_resources

import datetime
import functools
import hashlib
import json
import logging
Expand All @@ -46,7 +45,6 @@
from sqlalchemy.orm import backref

import fedmsg
import fedmsg.utils

import fmn.lib.defaults

Expand All @@ -69,20 +67,6 @@ def notify(self, openid, context, changed):

log = logging.getLogger(__name__)

execute_error_template = """
exception
---------
%s
rule
----
%r
message
-------
%s
"""


def init(db_url, alembic_ini=None, debug=False, create=False):
""" Create the tables in the database using the information from the
Expand Down Expand Up @@ -131,6 +115,16 @@ class Context(BASE):
icon = sa.Column(sa.String(32), nullable=False)
placeholder = sa.Column(sa.String(256))

def __json__(self):
return {
'name': self.name,
'detail_name': self.detail_name,
'description': self.description,
'created_on': self.created_on,
'icon': self.icon,
'placeholder': self.placeholder,
}

def get_confirmation(self, openid):
for confirmation in self.confirmations:
if confirmation.openid == openid:
Expand Down Expand Up @@ -171,29 +165,6 @@ def create(cls, session, name, description,
session.commit()
return context

def _recipients(self, session, config, valid_paths, message):
""" Returns the list of recipients for a message. """

for user in User.all(session):
pref = Preference.load(session, user, self)
if not pref or not pref.detail_values:
continue

flter = pref.prefers(session, config, valid_paths, message)
if not flter:
continue

for value in pref.detail_values:
yield {
'user': user.openid,
pref.context.detail_name: value.value,
'filter_name': flter.name,
'filter_id': flter.id,
}

def recipients(self, session, config, valid_paths, message):
return list(self._recipients(session, config, valid_paths, message))


class User(BASE):
__tablename__ = 'users'
Expand All @@ -204,6 +175,13 @@ class User(BASE):
api_key = sa.Column(sa.Text)
created_on = sa.Column(sa.DateTime, default=datetime.datetime.utcnow)

def __json__(self):
return {
'openid': self.openid,
'openid_url': self.openid_url,
'created_on': self.created_on,
}

def __repr__(self):
return "<fmn.lib.models.User %r %r>" % (self.openid, self.openid_url)

Expand Down Expand Up @@ -262,6 +240,13 @@ class Rule(BASE):
# JSON-encoded kw
_arguments = sa.Column(sa.String(256))

def __json__(self):
return {
'created_on': self.created_on,
'code_path': self.code_path,
'arguments': self.arguments,
}

def __repr__(self):
return "<fmn.lib.models.Rule %r(**%r)>" % (
self.code_path, self.arguments)
Expand All @@ -275,13 +260,6 @@ def arguments(self, kw):
if not kw is None:
self._arguments = json.dumps(kw)

def _instantiate_callable(self):
# This is a bit of a misnomer, load_class can load anything.
fn = fedmsg.utils.load_class(str(self.code_path))
# Now, partially apply our keyword arguments.
fn = functools.partial(fn, **self.arguments)
return fn

@staticmethod
def validate_code_path(valid_paths, code_path, **kw):
""" Raise an exception if code_path is not one of our
Expand Down Expand Up @@ -317,29 +295,6 @@ def doc(self, valid_paths, no_links=False):
else:
return valid_paths[root][name]['doc']

def execute(self, session, config, valid_paths, message):
""" Load our callable and execute it.
Note, we validate the code_path again here for the second time. Once
before it is inserted into the db, and once again before we execute it.
This is mitigation in case other code is vulnerable to injecting
arbitrary data into the db.
"""

Rule.validate_code_path(
valid_paths, self.code_path, **self.arguments)

fn = self._instantiate_callable()
try:
return fn(config, message)
except Exception as e:
log.error(execute_error_template % (
traceback.format_exc(),
self,
pprint.pformat(message),
))
return False


class Filter(BASE):
__tablename__ = 'filters'
Expand All @@ -352,6 +307,14 @@ class Filter(BASE):
sa.ForeignKey('preferences.id'))
preference = relation('Preference', backref=backref('filters'))

def __json__(self):
return {
'id': self.id,
'name': self.name,
'created_on': self.created_on,
'rules': [r.__json__() for r in self.rules]
}

def __repr__(self):
return "<fmn.lib.models.Filter %r>" % (self.name)

Expand Down Expand Up @@ -388,24 +351,6 @@ def remove_rule(self, session, code_path, **kw):

raise ValueError("No such rule found: %r" % code_path)

def matches(self, session, config, paths, message):
""" Return true if this filter matches the given message.
This is the case if *all* of the associated rules match.
...with one exception. If no rules are defined, the filter does not
match (even though technically, all of its zero rules match).
"""

if not self.rules:
return False

for rule in self.rules:
if not rule.execute(session, config, paths, message):
return False

return True


class DetailValue(BASE):
__tablename__ = 'detail_values'
Expand Down Expand Up @@ -472,6 +417,18 @@ class Preference(BASE):
sa.UniqueConstraint('openid', 'context_name'),
)

def __json__(self):
return {
'created_on': self.created_on,
'batch_delta': self.batch_delta,
'batch_count': self.batch_count,
'enabled': self.enabled,
'context': self.context.__json__(),
'user': self.user.__json__(),
'filters': [f.__json__() for f in self.filters],
'detail_values': [v.value for v in self.detail_values],
}

@property
def should_batch(self):
""" If the user has any batching preferences at all, then we should """
Expand Down Expand Up @@ -627,22 +584,6 @@ def get_filter(self, session, filter_id):

raise ValueError("No such filter %r" % filter_id)

def prefers(self, session, config, valid_paths, message):
""" Evaluate to true if this preference "prefers" this message.
That is the case if *any* of the associated filters match.
The first filter that matches the message is returned for bookkeeping.
If no filter matches, None is returned.
"""

for filter in self.filters:
if filter.matches(session, config, valid_paths, message):
return filter

return None


def hash_producer(*args, **kwargs):
""" Returns a random hash for a confirmation secret. """
Expand Down
Loading

0 comments on commit a3db7d7

Please sign in to comment.