Shifted NavigationManager caching to use node instances rather than node pks. Improve...

diff --git a/contrib/shipherd/models.py b/contrib/shipherd/models.py
index a95546d..8be03e2 100644 (file)
--- a/contrib/shipherd/models.py
+++ b/contrib/shipherd/models.py
@@ -41,11 +41,9 @@ class NavigationManager(TreeManager):
                will be the most recent set of defined hosted navigation among the node's
                ancestors. Lookups are cached so that subsequent lookups for the same node
                don't hit the database.
                will be the most recent set of defined hosted navigation among the node's
                ancestors. Lookups are cached so that subsequent lookups for the same node
                don't hit the database.

-               
-               TODO: Should this create the auto-generated navigation in "physical" form?

                """
                try:
                """
                try:

-                       return self._get_from_cache(self.db, node)
+                       return self._get_cache_for(self.db, node)

                except KeyError:
                        # Find the most recent host!
                        ancestors = node.get_ancestors(ascending=True, include_self=True).annotate(num_navigation=models.Count("hosted_navigation"))
                except KeyError:
                        # Find the most recent host!
                        ancestors = node.get_ancestors(ascending=True, include_self=True).annotate(num_navigation=models.Count("hosted_navigation"))


@@ -56,57 +54,75 @@ class NavigationManager(TreeManager):
                        nodes_to_cache = []
                        host_node = None
                        for ancestor in ancestors:
                        nodes_to_cache = []
                        host_node = None
                        for ancestor in ancestors:

-                               if self.is_cached(ancestor) or ancestor.num_navigation > 0:
+                               if self.has_cache_for(ancestor) or ancestor.num_navigation > 0:

                                        host_node = ancestor
                                        break
                                else:
                                        nodes_to_cache.append(ancestor)
                        
                                        host_node = ancestor
                                        break
                                else:
                                        nodes_to_cache.append(ancestor)
                        

-                       if not self.is_cached(host_node):
+                       if not self.has_cache_for(host_node):

                                self._add_to_cache(self.db, host_node)
                        
                        # Cache the queryset instance for every node that was passed over, as well.
                                self._add_to_cache(self.db, host_node)
                        
                        # Cache the queryset instance for every node that was passed over, as well.

-                       hosted_navigation = self._get_from_cache(self.db, host_node)
+                       hosted_navigation = self._get_cache_for(self.db, host_node)

                        for node in nodes_to_cache:
                                self._add_to_cache(self.db, node, hosted_navigation)
                
                return hosted_navigation
        
                        for node in nodes_to_cache:
                                self._add_to_cache(self.db, node, hosted_navigation)
                
                return hosted_navigation
        

-       def is_cached(self, node):
-               return self._is_cached(self.db, node)
-       

        def _add_to_cache(self, using, node, qs=None):
        def _add_to_cache(self, using, node, qs=None):

-               key = getattr(node, 'pk', None)
+               if node.pk is None:
+                       return

                
                if qs is None:
                
                if qs is None:

-                       if key is None:
-                               roots = self.none()
-                       else:
-                               roots = node.hosted_navigation.select_related('target_node')
+                       roots = node.hosted_navigation.select_related('target_node')
+                       qs = self.none()

                        
                        for root in roots:
                                root_qs = root.get_descendants(include_self=True).complex_filter({'%s__lte' % root._mptt_meta.level_attr: root.get_level() + root.depth}).exclude(depth__isnull=True)
                        
                        for root in roots:
                                root_qs = root.get_descendants(include_self=True).complex_filter({'%s__lte' % root._mptt_meta.level_attr: root.get_level() + root.depth}).exclude(depth__isnull=True)

-                               if qs is None:
-                                       qs = root_qs
-                               else:
-                                       qs |= root_qs
-               
-                       if qs is None:
-                               qs = self.none()
+                               qs |= root_qs

                
                

-               self.__class__._cache.setdefault(using, {})[key] = qs
+               self.__class__._cache.setdefault(using, {})[node] = qs
+       
+       def has_cache(self):
+               return self.db in self.__class__._cache and self.__class__._cache[self.db]
+       
+       def _get_cache_for(self, using, node):
+               return self.__class__._cache[self.db][node]
+       
+       def is_cached(self, navigation):
+               return self._is_cached(self.db, navigation)
+       
+       def _is_cached(self, using, navigation):
+               cache = self.__class__._cache[using]
+               for qs in cache.values():
+                       if navigation in qs:
+                               return True
+               return False

        
        

-       def _get_from_cache(self, using, node):
-               key = getattr(node, 'pk', None)
-               return self.__class__._cache[self.db][key]
+       def has_cache_for(self, node):
+               return self._has_cache_for(self.db, node)

        
        

-       def _is_cached(self, using, node):
+       def _has_cache_for(self, using, node):

                try:
                try:

-                       self._get_from_cache(using, node)
+                       self._get_cache_for(using, node)

                except KeyError:
                        return False
                return True
        
                except KeyError:
                        return False
                return True
        

+       def clear_cache_for(self, node):
+               """Clear the cache for a node and all its descendants"""
+               self._clear_cache_for(self.db, node)
+       
+       def _clear_cache_for(self, using, node):
+               # Clear the cache for all descendants of the node. Ideally we would
+               # only clear up to another hosting node, but the complexity is not
+               # necessary and may not be possible.
+               descendants = node.get_descendants(include_self=True)
+               cache = self.__class__._cache[using]
+               for node in descendants:
+                       cache.pop(node, None)
+       

        def clear_cache(self, navigation=None):
                """
                Clear out the navigation cache. This needs to happen during database flushes
        def clear_cache(self, navigation=None):
                """
                Clear out the navigation cache. This needs to happen during database flushes


@@ -116,9 +132,9 @@ class NavigationManager(TreeManager):
                        self.__class__._cache.clear()
                elif self.db in self.__class__._cache:
                        cache = self.__class__._cache[self.db]
                        self.__class__._cache.clear()
                elif self.db in self.__class__._cache:
                        cache = self.__class__._cache[self.db]

-                       for pk, qs in cache.items():
+                       for node, qs in cache.items():

                                if navigation in qs:
                                if navigation in qs:

-                                       cache.pop(pk)
+                                       cache.pop(node)

 
 
 class Navigation(TreeEntity):
 
 
 class Navigation(TreeEntity):


@@ -175,23 +191,27 @@ class Navigation(TreeEntity):
        target_url = property(get_target_url)
        
        def is_active(self, request):
        target_url = property(get_target_url)
        
        def is_active(self, request):

-               # First check if this particular navigation is active. It is considered active if:
-               # - the requested node is this instance's target node and its subpath matches the requested path.
-               # - the requested node is a descendant of this instance's target node and this instance's target
-               #   node is not the hosting node of this navigation structure.
-               # - this instance has no target node and the url matches either the request path or the full url.
-               # - any of this instance's children are active.

                node = request.node
                
                node = request.node
                

-               if self.target_node == node:
-                       if self.target_url == request.path:
-                               return True
-               elif self.target_node is None:
-                       if self.url_or_subpath == request.path or self.url_or_subpath == "http%s://%s%s" % (request.is_secure() and 's' or '', request.get_host(), request.path):
-                               return True
-               elif self.target_node.is_ancestor_of(node) and self.target_node != self.hosting_node:
+               if self.target_url == request.path:
+                       # Handle the `default` case where the target_url and requested path
+                       # are identical.
+                       return True
+               
+               if self.target_node is None and self.url_or_subpath == "http%s://%s%s" % (request.is_secure() and 's' or '', request.get_host(), request.path):
+                       # If there's no target_node, double-check whether it's a full-url
+                       # match.

                        return True
                
                        return True
                

+               ancestors = node.get_ancestors(ascending=True, include_self=True).annotate(num_navigation=models.Count("hosted_navigation")).filter(num_navigation__gt=0)
+               if ancestors:
+                       # If the target node is an ancestor of the requested node, this is
+                       # active - unless the target node is the `home` node for this set of
+                       # navigation or this navigation points to some other url.
+                       host_node = ancestors[0]
+                       if self.target_node.is_ancestor_of(node) and self.target_node != host_node and not self.url_or_subpath:
+                               return True
+               

                # Always fall back to whether the node has active children.
                return self.has_active_children(request)
        
                # Always fall back to whether the node has active children.
                return self.has_active_children(request)
        


@@ -215,17 +235,22 @@ class Navigation(TreeEntity):
                
                if self._has_changed():
                        self._initial_data = model_to_dict(self)
                
                if self._has_changed():
                        self._initial_data = model_to_dict(self)

-                       if self.is_cached():
-                               Navigation.objects.clear_cache(self)
-                       else:
-                               for navigation in self.get_ancestors():
-                                       if navigation.hosting_node and navigation.is_cached() and self.get_level() <= (navigation.get_level() + navigation.depth):
-                                               Navigation.objects.clear_cache(navigation)
+                       if Navigation.objects.has_cache():
+                               if self.is_cached():
+                                       Navigation.objects.clear_cache(self)
+                               else:
+                                       for navigation in self.get_ancestors():
+                                               if navigation.hosting_node and navigation.is_cached() and self.get_level() <= (navigation.get_level() + navigation.depth):
+                                                       Navigation.objects.clear_cache(navigation)
+                                       
+                                       if self.hosting_node and Navigation.objects.has_cache_for(self.hosting_node):
+                                               Navigation.objects.clear_cache_for(self.hosting_node)

        
        def delete(self, *args, **kwargs):
                super(Navigation, self).delete(*args, **kwargs)
                Navigation.objects.clear_cache(self)
        
        class Meta:
        
        def delete(self, *args, **kwargs):
                super(Navigation, self).delete(*args, **kwargs)
                Navigation.objects.clear_cache(self)
        
        class Meta:

-               ordering = ['order']
+               # Should I even try ordering?
+               ordering = ['order', 'lft']

                verbose_name_plural = 'navigation'
\ No newline at end of file
                verbose_name_plural = 'navigation'
\ No newline at end of file

diff --git a/contrib/shipherd/templatetags/shipherd.py b/contrib/shipherd/templatetags/shipherd.py
index 8adef3e..db15d46 100644 (file)
--- a/contrib/shipherd/templatetags/shipherd.py
+++ b/contrib/shipherd/templatetags/shipherd.py
@@ -2,6 +2,7 @@ from django import template
 from django.conf import settings
 from django.utils.safestring import mark_safe
 from philo.contrib.shipherd.models import Navigation
 from django.conf import settings
 from django.utils.safestring import mark_safe
 from philo.contrib.shipherd.models import Navigation

+from philo.models import Node

 from mptt.templatetags.mptt_tags import RecurseTreeNode, cache_tree_children
 
 
 from mptt.templatetags.mptt_tags import RecurseTreeNode, cache_tree_children
 
 


@@ -33,7 +34,16 @@ class RecurseNavigationNode(RecurseTreeNode):
                        return ''
                
                instance = self.instance_var.resolve(context)
                        return ''
                
                instance = self.instance_var.resolve(context)

-               roots = cache_tree_children(Navigation.objects.closest_navigation(instance))
+               
+               if isinstance(instance, Node):
+                       qs = Navigation.objects.closest_navigation(instance)
+               elif hasattr(instance, '__iter__'):
+                       # Is this the right way to check?
+                       qs = instance
+               else:
+                       return settings.TEMPLATE_STRING_IF_INVALID
+               
+               roots = cache_tree_children(qs)

                bits = [self._render_node(context, node, request) for node in roots]
                return ''.join(bits)
 
                bits = [self._render_node(context, node, request) for node in roots]
                return ''.join(bits)
 


@@ -64,7 +74,7 @@ def recursenavigation(parser, token):
        if len(bits) != 2:
                raise template.TemplateSyntaxError(_('%s tag requires an instance') % bits[0])
        
        if len(bits) != 2:
                raise template.TemplateSyntaxError(_('%s tag requires an instance') % bits[0])
        

-       instance_var = template.Variable(bits[1])
+       instance_var = parser.compile_filter(bits[1])

        
        template_nodes = parser.parse(('endrecursenavigation',))
        parser.delete_first_token()
        
        template_nodes = parser.parse(('endrecursenavigation',))
        parser.delete_first_token()


@@ -74,4 +84,9 @@ def recursenavigation(parser, token):
 
 @register.filter
 def has_navigation(node):
 
 @register.filter
 def has_navigation(node):

-       return bool(Navigation.objects.closest_navigation(node).count())
\ No newline at end of file
+       return bool(Navigation.objects.closest_navigation(node).count())
+
+
+@register.filter
+def targeting_navigation(node):
+       return Navigation.objects.closest_navigation(node).filter(target_node=node).order_by('level', 'lft')
\ No newline at end of file