You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@allura.apache.org by tv...@apache.org on 2013/06/08 01:42:08 UTC
[13/27] git commit: [#6325] Added tests for PPL, refactored,
and fixed issue with partial ordering of EP overrides
[#6325] Added tests for PPL, refactored, and fixed issue with partial ordering of EP overrides
Project: http://git-wip-us.apache.org/repos/asf/incubator-allura/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-allura/commit/af325d92
Tree: http://git-wip-us.apache.org/repos/asf/incubator-allura/tree/af325d92
Diff: http://git-wip-us.apache.org/repos/asf/incubator-allura/diff/af325d92
Branch: refs/heads/db/6276
Commit: af325d9223adc5238a953bdba4a0ef279705f2bd
Parents: b2093db
Author: Cory Johns <cj...@slashdotmedia.com>
Authored: Mon Jun 3 14:23:32 2013 +0000
Committer: Cory Johns <cj...@slashdotmedia.com>
Committed: Tue Jun 4 20:51:31 2013 +0000
----------------------------------------------------------------------
Allura/allura/lib/helpers.py | 65 +++++
Allura/allura/lib/package_path_loader.py | 215 +++++++++------
.../allura/tests/unit/test_package_path_loader.py | 203 ++++++++++++++
requirements-common.txt | 2 +-
4 files changed, 395 insertions(+), 90 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/af325d92/Allura/allura/lib/helpers.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/helpers.py b/Allura/allura/lib/helpers.py
index c821301..6c0a863 100644
--- a/Allura/allura/lib/helpers.py
+++ b/Allura/allura/lib/helpers.py
@@ -28,6 +28,7 @@ import logging
import cPickle as pickle
from hashlib import sha1
from datetime import datetime, timedelta
+from collections import defaultdict
import tg
import genshi.template
@@ -725,3 +726,67 @@ def log_output(log):
finally:
sys.stdout = _stdout
sys.stderr = _stderr
+
+def topological_sort(items, partial_order):
+ """Perform topological sort.
+ items is a list of items to be sorted.
+ partial_order is a list of pairs. If pair (a,b) is in it, it means
+ that item a should appear before item b.
+ Returns a list of the items in one of the possible orders, or None
+ if partial_order contains a loop.
+
+ Modified from: http://www.bitformation.com/art/python_toposort.html
+ """
+
+ def add_arc(graph, fromnode, tonode):
+ """Add an arc to a graph. Can create multiple arcs.
+ The end nodes must already exist."""
+ graph[fromnode].append(tonode)
+ # Update the count of incoming arcs in tonode.
+ graph[tonode][0] = graph[tonode][0] + 1
+
+ # step 1 - create a directed graph with an arc a->b for each input
+ # pair (a,b).
+ # The graph is represented by a dictionary. The dictionary contains
+ # a pair item:list for each node in the graph. /item/ is the value
+ # of the node. /list/'s 1st item is the count of incoming arcs, and
+ # the rest are the destinations of the outgoing arcs. For example:
+ # {'a':[0,'b','c'], 'b':[1], 'c':[1]}
+ # represents the graph: c <-- a --> b
+ # The graph may contain loops and multiple arcs.
+ # Note that our representation does not contain reference loops to
+ # cause GC problems even when the represented graph contains loops,
+ # because we keep the node names rather than references to the nodes.
+ graph = defaultdict(lambda:[0])
+ for a,b in partial_order:
+ add_arc(graph, a, b)
+
+ # Step 2 - find all roots (nodes with zero incoming arcs).
+ roots = [n for n in items if graph[n][0] == 0]
+ roots.reverse() # keep sort stable
+
+ # step 3 - repeatedly emit a root and remove it from the graph. Removing
+ # a node may convert some of the node's direct children into roots.
+ # Whenever that happens, we append the new roots to the list of
+ # current roots.
+ sorted = []
+ while roots:
+ # If len(roots) is always 1 when we get here, it means that
+ # the input describes a complete ordering and there is only
+ # one possible output.
+ # When len(roots) > 1, we can choose any root to send to the
+ # output; this freedom represents the multiple complete orderings
+ # that satisfy the input restrictions. We arbitrarily take one of
+ # the roots using pop(). Note that for the algorithm to be efficient,
+ # this operation must be done in O(1) time.
+ root = roots.pop()
+ sorted.append(root)
+ for child in graph[root][1:]:
+ graph[child][0] = graph[child][0] - 1
+ if graph[child][0] == 0:
+ roots.append(child)
+ del graph[root]
+ if len(graph) > 0:
+ # There is a loop in the input.
+ return None
+ return sorted
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/af325d92/Allura/allura/lib/package_path_loader.py
----------------------------------------------------------------------
diff --git a/Allura/allura/lib/package_path_loader.py b/Allura/allura/lib/package_path_loader.py
index 53b6fb7..0efde59 100644
--- a/Allura/allura/lib/package_path_loader.py
+++ b/Allura/allura/lib/package_path_loader.py
@@ -1,3 +1,19 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
'''
A Jinja template loader which allows for:
- dotted-notation package loading
@@ -93,8 +109,12 @@ The positioners are:
'''
import pkg_resources
import os
+from collections import defaultdict
import jinja2
+from ming.utils import LazyProperty
+
+from allura.lib.helpers import topological_sort
class PackagePathLoader(jinja2.BaseLoader):
@@ -113,7 +133,7 @@ class PackagePathLoader(jinja2.BaseLoader):
# TODO: How does one handle project-theme?
if default_paths is None:
default_paths = [
- #['projec-theme', None],
+ #['project-theme', None],
['site-theme', None],
['allura', '/'],
]
@@ -122,72 +142,108 @@ class PackagePathLoader(jinja2.BaseLoader):
self.default_paths = default_paths
self.override_root = override_root
- # Finally instantiate the loader
- self.fs_loader = jinja2.FileSystemLoader(self.init_paths())
+ @LazyProperty
+ def fs_loader(self):
+ return jinja2.FileSystemLoader(self.init_paths())
+
+ def _load_paths(self):
+ """
+ Load all the paths to be processed, including defaults, in the default order.
+ """
+ paths = self.default_paths[:] # copy default_paths
+ paths[-1:0] = [ # insert all eps just before last item, by default
+ [ep.name, pkg_resources.resource_filename(ep.module_name, "")]
+ for ep in pkg_resources.iter_entry_points(self.override_entrypoint)
+ ]
+ return paths
+
+ def _load_rules(self):
+ """
+ Load and pre-process the rules from the entry points.
+
+ Rules are specified per-tool as a list of the form:
+
+ template_path_rules = [
+ ['>', 'tool1'], # this tool must be resolved before tool1
+ ['<', 'tool2'], # this tool must be resolved after tool2
+ ['=', 'tool3'], # this tool replaces all of tool3's templates
+ ]
+
+ Returns two lists of rules, order_rules and replacement_rules.
+
+ order_rules represents all of the '>' and '<' rules and are returned
+ as a list of pairs of the form ('a', 'b') indicating that path 'a' must
+ come before path 'b'.
+
+ replacement_rules represent all of the '=' rules and are returned as
+ a dictionary mapping the paths to replace to the paths to replace with.
+ """
+ order_rules = []
+ replacement_rules = {}
+ for ep in pkg_resources.iter_entry_points(self.override_entrypoint):
+ for rule in getattr(ep.load(), 'template_path_rules', []):
+ if rule[0] == '>':
+ order_rules.append((ep.name, rule[1]))
+ elif rule[0] == '=':
+ replacement_rules[rule[1]] = ep.name
+ elif rule[0] == '<':
+ order_rules.append((rule[1], ep.name))
+ else:
+ raise jinja2.TemplateError(
+ 'Unknown template path rule in %s: %s' % (
+ ep.name, ' '.join(rule)))
+ return order_rules, replacement_rules
+
+ def _sort_paths(self, paths, rules):
+ """
+ Process all '>' and '<' rules, providing a partial ordering
+ of the paths based on the given rules.
+
+ The rules should already have been pre-processed by _load_rules
+ to a list of partial ordering pairs ('a', 'b') indicating that
+ path 'a' should come before path 'b'.
+ """
+ names = [p[0] for p in paths]
+ # filter rules that reference non-existent paths to prevent "loops" in the graph
+ rules = [r for r in rules if r[0] in names and r[1] in names]
+ ordered_paths = topological_sort(names, rules)
+ if ordered_paths is None:
+ raise jinja2.TemplateError(
+ 'Loop detected in ordering of overrides')
+ return paths.sort(key=lambda p: ordered_paths.index(p[0]))
+
+ def _replace_signposts(self, paths, rules):
+ """
+ Process all '=' rules, replacing the rule target's path value with
+ the rule's entry's path value.
+
+ Multiple entries replacing the same signpost can cause indeterminate
+ behavior, as the order of the entries is not entirely defined.
+ However, if _sort_by_rules is called first, the partial ordering is
+ respected.
+
+ This mutates paths.
+ """
+ p_idx = lambda n: [e[0] for e in paths].index(n)
+ for target, replacement in rules.items():
+ try:
+ removed = paths.pop(p_idx(replacement))
+ paths[p_idx(target)][1] = removed[1]
+ except ValueError:
+ # target or replacement missing (may not be installed)
+ pass
def init_paths(self):
'''
Set up the setuptools entry point-based paths.
'''
- paths = self.default_paths[:]
-
- '''
- Iterate through the overriders.
- TODO: Can this be moved to allura.app_globals.Globals, or is this
- executed before that is available?
- '''
- epoints = pkg_resources.iter_entry_points(self.override_entrypoint)
- for epoint in epoints:
- overrider = epoint.load()
- # Get the path of the module
- tmpl_path = pkg_resources.resource_filename(
- overrider.__module__,
- ""
- )
- # Default insert position is right before allura(/)
- insert_position = len(paths) - 1
-
- rules = getattr(overrider, 'template_path_rules', [])
-
- # Check each of the rules for this overrider
- for direction, signpost in rules:
- sp_location = None
-
- # Find the signpost
- try:
- sp_location = [path[0] for path in paths].index(signpost)
- except ValueError:
- # Couldn't find it, hope they specified another one, or
- # that the default is ok.
- continue
-
- if direction == '=':
- # Set a signpost. Behavior if already set is undetermined,
- # as entry point ordering is undetermined
- paths[sp_location][1] = tmpl_path
- # already inserted! our work is done here
- insert_position = None
- break
- elif direction == '>':
- # going to put it right before the signpost
- insert_position = min(sp_location, insert_position)
- elif direction == '<':
- # going to put it right after the signpost
- insert_position = min(sp_location + 1, insert_position)
- else:
- # don't know what that is!
- raise jinja2.TemplateError(
- 'Unknown template path rule in %s: %s' % (
- overrider, direction))
+ paths = self._load_paths()
+ order_rules, repl_rules = self._load_rules()
- # in the case that we've already replaced a signpost, carry on
- if insert_position is not None:
- # TODO: wouldn't OrderedDict be better? the allura.lib one
- # doesn't support ordering like the markdown one
- paths.insert(insert_position, (epoint.name, tmpl_path))
+ self._sort_paths(paths, order_rules)
+ self._replace_signposts(paths, repl_rules)
- # Get rid of None paths... not useful
- return [path for name, path in paths if path is not None]
+ return [p[1] for p in paths if p[1] is not None]
def get_source(self, environment, template):
'''
@@ -195,40 +251,21 @@ class PackagePathLoader(jinja2.BaseLoader):
- path/to/template.html
- module:path/to/template.html
'''
- package, path = None, None
src = None
- bits = template.split(':')
-
- if len(bits) == 2:
- # splitting out the Python module name from the template string...
- # the default allura behavior
- package, path = template.split(':')
- # TODO: is there a better way to do this?
- path_fragment = os.path.join(self.override_root, package, path)
- elif len(bits) == 1:
- # TODO: is this even useful?
- path = bits[0]
- path_fragment = os.path.join(self.override_root, path)
- else:
- raise jinja2.TemplateNotFound(template)
# look in all of the customized search locations...
try:
- src = self.fs_loader.get_source(environment, path_fragment)
+ parts = [self.override_root] + template.split(':')
+ if len(parts) > 2:
+ parts[1:2] = parts[1].split('.')
+ return self.fs_loader.get_source(environment, os.path.join(*parts))
except jinja2.TemplateNotFound:
- # ...this doesn't mean it's really not found... it's probably
- # just specified in the Allura-normal manner
+ # fall-back to attempt non-override loading
pass
- # ...but if you don't find anything, fall back to the explicit package
- # approach
- if src is None and package is not None:
- # gets the absolute filename of the template
+ if ':' in template:
+ package, path = template.split(':', 2)
filename = pkg_resources.resource_filename(package, path)
- # get the filename relative to the root: (default '/').. if this
- # fails this error is not caught, so should get propagated
- # normally
- src = self.fs_loader.get_source(environment, filename)
- elif src is None:
- raise jinja2.TemplateNotFound(template)
- return src
+ return self.fs_loader.get_source(environment, filename)
+ else:
+ return self.fs_loader.get_source(environment, template)
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/af325d92/Allura/allura/tests/unit/test_package_path_loader.py
----------------------------------------------------------------------
diff --git a/Allura/allura/tests/unit/test_package_path_loader.py b/Allura/allura/tests/unit/test_package_path_loader.py
new file mode 100644
index 0000000..a5055ac
--- /dev/null
+++ b/Allura/allura/tests/unit/test_package_path_loader.py
@@ -0,0 +1,203 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from unittest import TestCase
+from collections import defaultdict
+
+import jinja2
+from nose.tools import assert_equal, assert_in, assert_raises
+import mock
+
+from allura.lib.package_path_loader import PackagePathLoader
+
+
+class TestPackagePathLoader(TestCase):
+
+ @mock.patch('pkg_resources.resource_filename')
+ @mock.patch('pkg_resources.iter_entry_points')
+ def test_load_paths(self, iter_entry_points, resource_filename):
+ eps = iter_entry_points.return_value.__iter__.return_value = [
+ mock.Mock(ep_name='ep0', module_name='eps.ep0'),
+ mock.Mock(ep_name='ep1', module_name='eps.ep1'),
+ mock.Mock(ep_name='ep2', module_name='eps.ep2'),
+ ]
+ for ep in eps:
+ ep.name = ep.ep_name
+ resource_filename.side_effect = lambda m, r: 'path:'+m
+
+ paths = PackagePathLoader()._load_paths()
+
+ assert_equal(paths, [
+ ['site-theme', None],
+ ['ep0', 'path:eps.ep0'],
+ ['ep1', 'path:eps.ep1'],
+ ['ep2', 'path:eps.ep2'],
+ ['allura', '/'],
+ ])
+ assert_equal(type(paths[0]), list)
+ assert_equal(resource_filename.call_args_list, [
+ mock.call('eps.ep0', ''),
+ mock.call('eps.ep1', ''),
+ mock.call('eps.ep2', ''),
+ ])
+
+ @mock.patch('pkg_resources.iter_entry_points')
+ def test_load_rules(self, iter_entry_points):
+ eps = iter_entry_points.return_value.__iter__.return_value = [
+ mock.Mock(ep_name='ep0', rules=[('>', 'allura')]),
+ mock.Mock(ep_name='ep1', rules=[('=', 'allura')]),
+ mock.Mock(ep_name='ep2', rules=[('<', 'allura')]),
+ ]
+ for ep in eps:
+ ep.name = ep.ep_name
+ ep.load.return_value.template_path_rules = ep.rules
+
+ order_rules, replacement_rules = PackagePathLoader()._load_rules()
+
+ assert_equal(order_rules, [('ep0', 'allura'), ('allura', 'ep2')])
+ assert_equal(replacement_rules, {'allura': 'ep1'})
+
+ eps = iter_entry_points.return_value.__iter__.return_value = [
+ mock.Mock(ep_name='ep0', rules=[('?', 'allura')]),
+ ]
+ for ep in eps:
+ ep.name = ep.ep_name
+ ep.load.return_value.template_path_rules = ep.rules
+ assert_raises(jinja2.TemplateError, PackagePathLoader()._load_rules)
+
+ def test_replace_signposts(self):
+ ppl = PackagePathLoader()
+ ppl._replace_signpost = mock.Mock()
+ paths = [
+ ['site-theme', None],
+ ['ep0', '/ep0'],
+ ['ep1', '/ep1'],
+ ['ep2', '/ep2'],
+ ['allura', '/'],
+ ]
+ rules = {
+ 'allura': 'ep2',
+ 'site-theme': 'ep1',
+ 'foo': 'ep1',
+ 'ep0': 'bar',
+ }
+
+ ppl._replace_signposts(paths, rules)
+
+ assert_equal(paths, [
+ ['site-theme', '/ep1'],
+ ['ep0', '/ep0'],
+ ['allura', '/ep2'],
+ ]);
+
+ def test_sort_paths(self):
+ paths = [
+ ['site-theme', None],
+ ['ep0', '/ep0'],
+ ['ep1', '/ep1'],
+ ['ep2', '/ep2'],
+ ['ep3', '/ep3'],
+ ['allura', '/'],
+ ]
+ rules = [
+ ('allura', 'ep0'),
+ ('ep3', 'ep1'),
+ ('ep2', 'ep1'),
+ ('ep4', 'ep1'), # rules referencing missing paths
+ ('ep2', 'ep5'),
+ ]
+
+ PackagePathLoader()._sort_paths(paths, rules)
+
+ assert_equal(paths, [
+ ['site-theme', None],
+ ['ep2', '/ep2'],
+ ['ep3', '/ep3'],
+ ['ep1', '/ep1'],
+ ['allura', '/'],
+ ['ep0', '/ep0'],
+ ])
+
+ def test_init_paths(self):
+ paths = [
+ ['root', '/'],
+ ['none', None],
+ ['tail', '/tail'],
+ ]
+ ppl = PackagePathLoader()
+ ppl._load_paths = mock.Mock(return_value=paths)
+ ppl._load_rules = mock.Mock(return_value=('order_rules','repl_rules'))
+ ppl._replace_signposts = mock.Mock()
+ ppl._sort_paths = mock.Mock()
+
+ output = ppl.init_paths()
+
+ ppl._load_paths.assert_called_once_with()
+ ppl._load_rules.assert_called_once_with()
+ ppl._sort_paths.assert_called_once_with(paths, 'order_rules')
+ ppl._replace_signposts.assert_called_once_with(paths, 'repl_rules')
+
+ assert_equal(output, ['/', '/tail'])
+
+ @mock.patch('jinja2.FileSystemLoader')
+ def test_fs_loader(self, FileSystemLoader):
+ ppl = PackagePathLoader()
+ ppl.init_paths = mock.Mock(return_value=['path1', 'path2'])
+ FileSystemLoader.return_value = 'fs_loader'
+
+ output1 = ppl.fs_loader
+ output2 = ppl.fs_loader
+
+ ppl.init_paths.assert_called_once_with()
+ FileSystemLoader.assert_called_once_with(['path1', 'path2'])
+ assert_equal(output1, 'fs_loader')
+ assert output1 is output2
+
+ @mock.patch('jinja2.FileSystemLoader')
+ def test_get_source(self, fs_loader):
+ ppl = PackagePathLoader()
+ ppl.init_paths = mock.Mock()
+ fs_loader().get_source.return_value = 'fs_load'
+
+ # override exists
+ output = ppl.get_source('env', 'allura.ext.admin:templates/audit.html')
+
+ assert_equal(output, 'fs_load')
+ fs_loader().get_source.assert_called_once_with('env', 'override/allura/ext/admin/templates/audit.html')
+
+ fs_loader().get_source.reset_mock()
+ fs_loader().get_source.side_effect = [jinja2.TemplateNotFound('test'), 'fs_load']
+
+ with mock.patch('pkg_resources.resource_filename') as rf:
+ rf.return_value = 'resource'
+ # no override, ':' in template
+ output = ppl.get_source('env', 'allura.ext.admin:templates/audit.html')
+ rf.assert_called_once_with('allura.ext.admin', 'templates/audit.html')
+
+ assert_equal(output, 'fs_load')
+ assert_equal(fs_loader().get_source.call_count, 2)
+ fs_loader().get_source.assert_called_with('env', 'resource')
+
+ fs_loader().get_source.reset_mock()
+ fs_loader().get_source.side_effect = [jinja2.TemplateNotFound('test'), 'fs_load']
+
+ # no override, ':' not in template
+ output = ppl.get_source('env', 'templates/audit.html')
+
+ assert_equal(output, 'fs_load')
+ assert_equal(fs_loader().get_source.call_count, 2)
+ fs_loader().get_source.assert_called_with('env', 'templates/audit.html')
http://git-wip-us.apache.org/repos/asf/incubator-allura/blob/af325d92/requirements-common.txt
----------------------------------------------------------------------
diff --git a/requirements-common.txt b/requirements-common.txt
index f80e240..7bb0de3 100644
--- a/requirements-common.txt
+++ b/requirements-common.txt
@@ -67,7 +67,7 @@ smmap==0.8.1
# testing & development
datadiff==1.1.5
ipython==0.11
-mock==0.8.0
+mock==1.0.1
nose==1.1.2
pyflakes==0.5.0
WebTest==1.4.0