Skip to content

Commit

Permalink
Merge pull request #16 from plone/maurits-minor-fixes-and-additions
Browse files Browse the repository at this point in the history
Support storage.clear and storage.update (plus minor fixes)
  • Loading branch information
gforcada authored Feb 15, 2019
2 parents 4db8952 + 85e3d88 commit 3796c60
Show file tree
Hide file tree
Showing 9 changed files with 79 additions and 38 deletions.
1 change: 1 addition & 0 deletions news/12.bugfix
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
Turned doctests into unittests.
Removed no longer needed test_suite functions.
[maurits]
2 changes: 2 additions & 0 deletions news/13.bugfix → news/13.feature
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,7 @@ Support using the 'in' operator for paths.
Support using storage[old_path] to get the new path, possibly raising KeyError.
Support using storage[old_path] to set or delete new paths.
Support using len(storage) to get the number of paths.
Support storage.clear() to clear out all data.
Support storage.update() for bulk updates.
Added performance tests. Call with ``export PLONE_APP_REDIRECTOR_PERFORMANCE_NUMBER=100000`` to enable.
[maurits]
9 changes: 9 additions & 0 deletions plone/app/redirector/storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ class RedirectionStorage(Persistent):
"""

def __init__(self):
self.clear()

def clear(self):
# If the data already exists, we could call 'clear' on all BTrees,
# but making them fresh seems cleaner and faster.
self._paths = OOBTree()
self._rpaths = OOBTree()

Expand Down Expand Up @@ -61,6 +66,10 @@ def add(self, old_path, new_path):

__setitem__ = add

def update(self, info):
for key, value in info.items():
self.add(key, value)

def remove(self, old_path):
old_path = self._canonical(old_path)
new_path = self._paths.get(old_path, None)
Expand Down
6 changes: 0 additions & 6 deletions plone/app/redirector/tests/test_browser.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,3 @@ def test_no_redirect_on_creation(self):
self.browser.getControl('Rename').click()
self.assertListEqual(list(storage), ['/plone/foo'])
self.assertEqual(storage.get('/plone/foo'), '/plone/bar')


def test_suite():
suite = unittest.TestSuite()
suite.addTest(unittest.makeSuite(TestBrowser))
return suite
6 changes: 0 additions & 6 deletions plone/app/redirector/tests/test_events.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,3 @@ def test_add_doesnt_create_storage_entry(self):
self.folder.invokeFactory('Document', 'p1')
transaction.savepoint(1)
self.assertEqual(0, len(list(self.storage)) - orig_len)


def test_suite():
suite = unittest.TestSuite()
suite.addTest(unittest.makeSuite(TestRedirectorEvents))
return suite
57 changes: 48 additions & 9 deletions plone/app/redirector/tests/test_performance.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,16 @@
VERBOSE = False


def pretty_number(num):
if num < 1000:
return num
num = int(num / 1000)
if num < 1000:
return '{0} thousand'.format(num)
num = int(num / 1000)
return '{0} million'.format(num)


class TestStoragePerformance(unittest.TestCase):
"""Test the performance of the RedirectionStorage class.
"""
Expand Down Expand Up @@ -66,13 +76,48 @@ def test_storage_performance(self):
st = RedirectionStorage()
if VERBOSE:
print('\nRunning plone.app.redirector storage performance tests.')
print('Inserting {0} paths...'.format(NUMBER))

# Can take long. But 10.000 per second should be no problem.
with self.timeit('Inserting', NUMBER / 10000.0):
for i in range(NUMBER):
# Take one tenth of the items at first.
num = max(int(NUMBER / 10), 1)
with self.timeit(
'Inserting {0} individual items'.format(pretty_number(num)),
num / 10000.0,
):
for i in range(num):
st['/old/{0}'.format(i)] = '/new/{0}'.format(i)

# I expected this to be almost instantaneous because we replace
# the data with new OOBTrees, but it still takes time:
# for ten million items it take 0.3 seconds.
with self.timeit('Clearing storage', num / 1000000.0):
st.clear()

# Should be fairly quick.
with self.timeit(
'Preparing {0} items for bulk import'.format(
pretty_number(NUMBER)
),
NUMBER / 100000.0,
):
info = {
'/old/{0}'.format(i): '/new/{0}'.format(i)
for i in range(NUMBER)
}

# Can take long. But 10.000 per second should be no problem.
with self.timeit(
'Inserting {0} prepared items in bulk'.format(
pretty_number(NUMBER)
),
NUMBER / 10000.0,
):
# Prepare input:
info = {}
for i in range(NUMBER):
info['/old/{0}'.format(i)] = '/new/{0}'.format(i)
st.update(info)

# Should be almost instantaneous.
with self.timeit('Getting length'):
self.assertEqual(len(st), NUMBER)
Expand All @@ -91,9 +136,3 @@ def test_storage_performance(self):
):
for key in st:
st[key]


def test_suite():
suite = unittest.TestSuite()
suite.addTest(unittest.makeSuite(TestStoragePerformance))
return suite
6 changes: 0 additions & 6 deletions plone/app/redirector/tests/test_setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,3 @@ def test_utility(self):
def test_view(self):
view = self.portal.restrictedTraverse('@@plone_redirector_view')
self.assertNotEqual(None, view)


def test_suite():
suite = unittest.TestSuite()
suite.addTest(unittest.makeSuite(TestRedirectorSetup))
return suite
24 changes: 19 additions & 5 deletions plone/app/redirector/tests/test_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,19 @@ def test_storage_two_redirects_pythonic(self):
self.assertIn('/baz', st)
self.assertNotIn('/bar', st)

def test_storage_clear(self):
# Clear all information.
st = RedirectionStorage()
st['/foo'] = '/bar'
st['/baz'] = '/bar'
st.clear()
self.assertNotIn('/foo', st)
self.assertNotIn('/baz', st)
self.assertEqual(len(st.redirects('/bar')), 0)
# Test the internal structures directly
self.assertEqual(len(st._paths), 0)
self.assertEqual(len(st._rpaths), 0)

def test_storage_update_redirect(self):
# Update a redirect
st = RedirectionStorage()
Expand Down Expand Up @@ -272,8 +285,9 @@ def test_storage_three_step_circular_rename(self):
self.assertListEqual(st.redirects('second'), [])
self.assertListEqual(st.redirects('third'), [])


def test_suite():
suite = unittest.TestSuite()
suite.addTest(unittest.makeSuite(TestStorage))
return suite
def test_storage_non_string_path_fails(self):
st = RedirectionStorage()
with self.assertRaises(AttributeError):
st[0] = '/bar'
with self.assertRaises(AttributeError):
st['/foo'] = 0
6 changes: 0 additions & 6 deletions plone/app/redirector/tests/test_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,9 +204,3 @@ def test_search_blacklisted(self):
urls = sorted([b.getURL() for b in view.search_for_similar()])
self.assertEqual(1, len(urls))
self.assertEqual(fu + '/f2', urls[0])


def test_suite():
suite = unittest.TestSuite()
suite.addTest(unittest.makeSuite(TestRedirectorView))
return suite

0 comments on commit 3796c60

Please sign in to comment.