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

[philo.git] / contrib / shipherd / models.py

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