From 8e7c9306ad3f3f9a1d1f22e046be4f911ba9c2db Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Wed, 22 Apr 2015 10:13:59 +0100 Subject: [PATCH] RoutablePage: Change way URL resolver is created Fixes #1168 Prevously, we looked up the URLs directly from the subpage_urls attribute and gave the value this returned to Djangos get_resolver function. get_resolver implements an infinite LRU cache so if this attribute was a property, every time a different instance of RoutablePage called it, a new item was added to the cache. This would repeat until the process was restarted or crashed. This commit makes defining subpage_urls as a property raise an error so this memory leak can't happen. The implementation was taken from #1161 to make it forwards-compatible. The changes are: - Added a new classmethod called get_resolver. This builds a RegexURLResolver itself and caches it on the class - No longer using Djangos get_resolver so url configs don't get cached twice - Added a new classmethod called get_subpage_urls. This can be overridden by subclasses to add more URLs --- wagtail/contrib/wagtailroutablepage/models.py | 33 +++++++++++-------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/wagtail/contrib/wagtailroutablepage/models.py b/wagtail/contrib/wagtailroutablepage/models.py index 2463af146a..9ad9dce48c 100644 --- a/wagtail/contrib/wagtailroutablepage/models.py +++ b/wagtail/contrib/wagtailroutablepage/models.py @@ -3,8 +3,7 @@ from __future__ import unicode_literals from six import string_types from django.http import Http404 -from django.core.urlresolvers import get_resolver -from django.core.exceptions import ImproperlyConfigured +from django.core.urlresolvers import RegexURLResolver from wagtail.wagtailcore.models import Page from wagtail.wagtailcore.url_routing import RouteResult @@ -18,28 +17,36 @@ class RoutablePageMixin(object): #: Set this to a tuple of ``django.conf.urls.url`` objects. subpage_urls = None + @classmethod + def get_subpage_urls(cls): + if cls.subpage_urls: + return cls.subpage_urls + + return () + + @classmethod + def get_resolver(cls): + if '_routablepage_urlresolver' not in cls.__dict__: + subpage_urls = cls.get_subpage_urls() + cls._routablepage_urlresolver = RegexURLResolver(r'^/', subpage_urls) + + return cls._routablepage_urlresolver + def reverse_subpage(self, name, args=None, kwargs=None): """ - This method does the same job as Djangos' built in "urlresolvers.reverse()" function for subpage urlconfs. + This method does the same job as Djangos' built in + "urlresolvers.reverse()" function for subpage urlconfs. """ args = args or [] kwargs = kwargs or {} - if self.subpage_urls is None: - raise ImproperlyConfigured("You must set 'subpage_urls' on " + type(self).__name__) - - resolver = get_resolver(self.subpage_urls) - return resolver.reverse(name, *args, **kwargs) + return self.get_resolver().reverse(name, *args, **kwargs) def resolve_subpage(self, path): """ This finds a view method/function from a URL path. """ - if self.subpage_urls is None: - raise ImproperlyConfigured("You must set 'subpage_urls' on " + type(self).__name__) - - resolver = get_resolver(self.subpage_urls) - view, args, kwargs = resolver.resolve(path) + view, args, kwargs = self.get_resolver().resolve(path) # If view is a string, find it as an attribute of self if isinstance(view, string_types):