Merge pull request #42 from borgstrom/thread-safe

Implement thread safety

Author: borgstrom

.travis.yml

@@ -13,6 +13,7 @@ env:
   - DJANGO=Django==1.6
   - DJANGO=Django==1.7
   - DJANGO=Django==1.8
+  - DJANGO=Django==1.9
 
 matrix:
   exclude:
@@ -22,6 +23,12 @@ matrix:
     - python: 2.6
       env: DJANGO=Django==1.8
 
+    - python: 2.6
+      env: DJANGO=Django==1.9
+
+    - python: 3.3
+      env: DJANGO=Django==1.9
+
     - python: 3.5
       env: DJANGO=Django==1.6
 

README.rst

@@ -55,4 +55,4 @@ https://django-simple-menu.readthedocs.org
 Requirements
 ------------
 
-Django 1.3+
+Django 1.6+

menu/menu.py

@@ -1,6 +1,9 @@
+import copy
 import re
+import sys
 
 from django.conf import settings
+from django.utils.text import slugify
 
 try:
     from django.apps import apps
@@ -105,8 +108,9 @@ def process(c, request, name=None):
         if name not in c.items:
             return []
 
+        items = copy.deepcopy(c.items[name])
         curitem = None
-        for item in c.items[name]:
+        for item in items:
             item.process(request)
             if item.visible:
                 item.selected = False
@@ -117,19 +121,12 @@ def process(c, request, name=None):
         if curitem is not None:
             curitem.selected = True
 
-        def filter_visible(items):
-            return [
-                filter_visible_children(item)
-                for item in items
-                if item.visible
-            ]
-
-        def filter_visible_children(item):
-            item.children = filter_visible(item.children)
-            return item
-
         # return only visible items
-        visible = filter_visible(c.items[name])
+        visible = [
+            item
+            for item in items
+            if item.visible
+        ]
 
         # determine if we should apply 'selected' to parents when one of their
         # children is the 'selected' menu
@@ -176,10 +173,8 @@ def __init__(self, title, url, children=[], weight=1, check=None,
 
         self.url = url
         self.title = title
-        self._title = None
         self.visible = visible
         self.children = children
-        self._children = None
         self.weight = weight
         self.check = check
         self.slug = slug
@@ -191,39 +186,57 @@ def __init__(self, title, url, children=[], weight=1, check=None,
         for k in kwargs:
             setattr(self, k, kwargs[k])
 
-        # if title is a callable store a reference to it for later
-        # then we'll process it at runtime
-        if callable(title):
-            self.title = ""
-            self._title = title
-
     def process(self, request):
         """
         process determines if this item should visible, if its selected, etc...
         """
-        self.check_check(request)
+        # evaluate our check
+        if callable(self.check):
+            self.visible = self.check(request)
+
+        # if we're not visible we return since we don't need to do anymore processing
         if not self.visible:
             return
 
-        # evaluate title
-        self.check_title(request)
+        # evaluate our title
+        if callable(self.title):
+            self.title = self.title(request)
+
+        # if no title is set turn it into a slug
+        if self.slug is None:
+            # in python3 we don't need to convert to unicode, in python2 slugify
+            # requires a unicode string
+            if sys.version_info > (3, 0):
+                self.slug = slugify(self.title)
+            else:
+                self.slug = slugify(unicode(self.title))
 
         # evaluate children
-        visible_children = []
-        self.check_children(request)
+        if callable(self.children):
+            children = list(self.children(request))
+        else:
+            children = list(self.children)
 
-        for child in self.children:
+        for child in children:
+            child.parent = self
             child.process(request)
-            if child.visible:
-                visible_children.append(child)
 
+        self.children = [
+            child
+            for child in children
+            if child.visible
+        ]
+        self.children.sort(key=lambda child: child.weight)
+
+        # if we have no children and MENU_HIDE_EMPTY then we are not visible and should return
         hide_empty = getattr(settings, 'MENU_HIDE_EMPTY', False)
-        if hide_empty and not self.check and not len(visible_children):
+        if hide_empty and len(self.children) == 0:
             self.visible = False
             return
 
+        # find out if one of our children is selected, and mark it as such
         curitem = None
-        for item in visible_children:
+        for item in self.children:
             item.selected = False
 
             if item.match_url(request):
@@ -244,36 +257,3 @@ def match_url(self, request):
         elif re.match("%s" % self.url, request.path):
             matched = True
         return matched
-
-    def check_children(self, request):
-        """
-        Check children against the given request
-        """
-        if callable(self._children):
-            children = self._children(request)
-        elif callable(self.children):
-            children = self.children(request)
-            self._children = self.children
-        else:
-            children = self.children
-
-        children = [child for child in children]
-        children.sort(key=lambda child: child.weight)
-        for child in children:
-            child.parent = self
-
-        self.children = children
-
-    def check_check(self, request):
-        """
-        Set our visibility based on our check against the given request
-        """
-        if callable(self.check):
-            self.visible = self.check(request)
-
-    def check_title(self, request):
-        if callable(self._title):
-            self.title = self._title(request)
-        if self.slug is None:
-            self.slug = re.sub(r'[^a-zA-Z0-9\-]+', '_',
-                               self.title.lower()).strip('_')

menu/tests/test_menu.py

@@ -1,4 +1,12 @@
+import signal
+import threading
 import unittest
+import sys
+
+if sys.version_info > (3, 0):
+    from queue import Queue
+else:
+    from Queue import Queue
 
 from django.conf import settings
 from django.template import Template, Context
@@ -8,7 +16,6 @@
 from menu import Menu, MenuItem
 
 # XXX TODO: test MENU_HIDE_EMPTY
-# XXX TODO: test check_children
 
 class MenuTests(TestCase):
     """
@@ -52,7 +59,7 @@ def kids3_1(request):
             ]
 
         kids3 = (
-            MenuItem("kids3-1", "/parent3/kids3-1", children=kids3_1),
+            MenuItem("kids3-1", "/parent3/kids3-1", children=kids3_1, slug="salty"),
             MenuItem(kids3_2_title, "/parent3/kids3-2")
         )
 
@@ -68,74 +75,107 @@ def kids3_1(request):
 
         self.factory = RequestFactory()
 
+    def test_thread_safety_and_checks(self):
+        """
+        Ensure our thread safety works, this also ensures our checks work
+        """
+        # this shouldn't ever take more than 5 seconds, add a safety in case someting breaks
+        signal.alarm(5)
+
+        def t1(results):
+            "Closure for thread 1"
+            request = self.factory.get('/kids2-2/visible')
+            items = Menu.process(request, 'test')
+            results.put_nowait(len(items[0].children) == 2)
+
+        def t2(results):
+            "Closure for thread 2"
+            request = self.factory.get('/kids2-2/hidden')
+            items = Menu.process(request, 'test')
+            results.put_nowait(len(items[0].children) == 1)
+
+        results = Queue()
+        for _ in range(50):
+            threads = [
+                threading.Thread(target=t1, args=(results,)),
+                threading.Thread(target=t2, args=(results,))
+            ]
+            for thread in threads:
+                thread.start()
+            for thread in threads:
+                thread.join()
+
+
+        self.assertTrue(all([
+            results.get()
+            for _ in range(100)
+        ]))
+
+    def test_slug(self):
+        """
+        Ensure our slugification works as expected
+        """
+        request = self.factory.get('/parent3/kids3-1')
+        items = Menu.process(request, 'test')
+        self.assertEqual(items[1].slug, "parent-3")
+        self.assertEqual(items[1].children[0].slug, "salty")
+
     def test_exact_url(self):
         """
         Ensure that the exact_url setting works
         """
         # the extra stuff will still cause kids3-2 to be selected
         request = self.factory.get('/parent3/kids3-2/extra_stuff_here')
-        Menu.process(request, 'test')
-        self.assertEqual(Menu.items['test'][1].children[1].selected, True)
+        items = Menu.process(request, 'test')
+        self.assertEqual(items[1].children[1].selected, True)
 
         # but here it won't, because exact_url is set
         request = self.factory.get('/parent3/kids3-1/kid1/extra_stuff_here')
-        Menu.process(request, 'test')
-        self.assertEqual(Menu.items['test'][1].children[0].children[0].selected, False)
+        items = Menu.process(request, 'test')
+        self.assertEqual(items[1].children[0].children[0].selected, False)
 
     def test_callable_title(self):
         """
         Ensure callable titles work
         """
         self.kids3_2_desired_title = "fun"
         request = self.factory.get('/parent3')
-        Menu.process(request, 'test')
-        self.assertEqual(Menu.items['test'][1].children[1].title, "/parent3-fun")
-
-    def test_checks(self):
-        """
-        Ensure checks on menus work
-        """
-        request = self.factory.get('/kids2-2/visible')
-        Menu.process(request, 'test')
-        self.assertEqual(len(Menu.items['test'][0].children), 2)
-
-        request = self.factory.get('/kids2-2/hidden')
-        Menu.process(request, 'test')
-        self.assertEqual(len(Menu.items['test'][0].children), 1)
+        items = Menu.process(request, 'test')
+        self.assertEqual(items[1].children[1].title, "/parent3-fun")
 
     def test_select_parents(self):
         """
         Ensure the MENU_SELECT_PARENTS setting works
         """
         settings.MENU_SELECT_PARENTS = False
         request = self.factory.get('/parent2/kids2-1')
-        Menu.process(request, 'test')
-        self.assertEqual(Menu.items['test'][0].selected, True)
-        self.assertEqual(Menu.items['test'][0].children[1].selected, True)
-        self.assertEqual(Menu.items['test'][1].selected, False)
+        items = Menu.process(request, 'test')
+        self.assertEqual(items[0].selected, True)
+        self.assertEqual(items[0].children[1].selected, True)
+        self.assertEqual(items[1].selected, False)
 
         request = self.factory.get('/kids2-2')
-        Menu.process(request, 'test')
-        self.assertEqual(Menu.items['test'][0].selected, False)
-        self.assertEqual(Menu.items['test'][0].children[0].selected, True)
-        self.assertEqual(Menu.items['test'][1].selected, False)
+        items = Menu.process(request, 'test')
+        self.assertEqual(items[0].selected, False)
+        self.assertEqual(items[0].children[0].selected, True)
+        self.assertEqual(items[1].selected, False)
 
         settings.MENU_SELECT_PARENTS = True
         request = self.factory.get('/kids2-2')
-        Menu.process(request, 'test')
-        self.assertEqual(Menu.items['test'][0].selected, True)
-        self.assertEqual(Menu.items['test'][0].children[0].selected, True)
-        self.assertEqual(Menu.items['test'][1].selected, False)
+        items = Menu.process(request, 'test')
+        self.assertEqual(items[0].selected, True)
+        self.assertEqual(items[0].children[0].selected, True)
+        self.assertEqual(items[1].selected, False)
 
         request = self.factory.get('/parent3/kids3-1/kid1')
-        Menu.process(request, 'test')
-        self.assertEqual(Menu.items['test'][0].selected, False)
-        self.assertEqual(Menu.items['test'][0].children[1].selected, False)
-        self.assertEqual(Menu.items['test'][1].selected, True)
-        self.assertEqual(Menu.items['test'][1].children[0].selected, True)
-        self.assertEqual(Menu.items['test'][1].children[0].children[0].selected, True)
-        self.assertEqual(Menu.items['test'][1].children[1].selected, False)
-        self.assertEqual(Menu.items['test'][2].selected, False)
+        items = Menu.process(request, 'test')
+        self.assertEqual(items[0].selected, False)
+        self.assertEqual(items[0].children[1].selected, False)
+        self.assertEqual(items[1].selected, True)
+        self.assertEqual(items[1].children[0].selected, True)
+        self.assertEqual(items[1].children[0].children[0].selected, True)
+        self.assertEqual(items[1].children[1].selected, False)
+        self.assertEqual(items[2].selected, False)
 
     def test_template_tag(self):
         """