Refactored Node.get_absolute_url and related functions (such as MultiView.reverse...
authorStephen Burrows <stephen.r.burrows@gmail.com>
Wed, 2 Feb 2011 16:48:40 +0000 (11:48 -0500)
committerStephen Burrows <stephen.r.burrows@gmail.com>
Wed, 2 Feb 2011 16:56:55 +0000 (11:56 -0500)
contrib/penfield/models.py
contrib/penfield/utils.py
contrib/shipherd/models.py
contrib/waldo/models.py
exceptions.py
middleware.py
models/nodes.py
templatetags/nodes.py
views.py

index 7ca879d..27e5b5d 100644 (file)
@@ -111,49 +111,49 @@ class BlogView(MultiView, FeedMultiViewMixin):
                )
                if self.feeds_enabled:
                        urlpatterns += patterns('',
-                               url(r'^%s/(?P<tag_slugs>[-\w]+[-+/\w]*)/%s/' % (self.tag_permalink_base, self.feed_suffix), self.feed_view(self.get_entries_by_tag, 'entries_by_tag_feed'), name='entries_by_tag_feed'),
+                               url(r'^%s/(?P<tag_slugs>[-\w]+[-+/\w]*)/%s$' % (self.tag_permalink_base, self.feed_suffix), self.feed_view(self.get_entries_by_tag, 'entries_by_tag_feed'), name='entries_by_tag_feed'),
                        )
                urlpatterns += patterns('',
-                       url(r'^%s/(?P<tag_slugs>[-\w]+[-+/\w]*)/' % self.tag_permalink_base, self.page_view(self.get_entries_by_tag, self.tag_page), name='entries_by_tag')
+                       url(r'^%s/(?P<tag_slugs>[-\w]+[-+/\w]*)$' % self.tag_permalink_base, self.page_view(self.get_entries_by_tag, self.tag_page), name='entries_by_tag')
                )
                if self.tag_archive_page:
                        urlpatterns += patterns('',
-                               url((r'^(?:%s)/?$' % self.tag_permalink_base), self.tag_archive_view, 'tag_archive')
+                               url((r'^%s$' % self.tag_permalink_base), self.tag_archive_view, name='tag_archive')
                        )
                
                if self.entry_archive_page:
                        if self.entry_permalink_style in 'DMY':
                                urlpatterns += patterns('',
-                                       url(r'^(?P<year>\d{4})/', include(self.feed_patterns(self.get_entries_by_ymd, self.entry_archive_page, 'entries_by_year')))
+                                       url(r'^(?P<year>\d{4})', include(self.feed_patterns(self.get_entries_by_ymd, self.entry_archive_page, 'entries_by_year')))
                                )
                                if self.entry_permalink_style in 'DM':
                                        urlpatterns += patterns('',
-                                               url(r'^(?P<year>\d{4})/(?P<month>\d{2})/?$', include(self.feed_patterns(self.get_entries_by_ymd, self.entry_archive_page, 'entries_by_month'))),
+                                               url(r'^(?P<year>\d{4})/(?P<month>\d{2})$', include(self.feed_patterns(self.get_entries_by_ymd, self.entry_archive_page, 'entries_by_month'))),
                                        )
                                        if self.entry_permalink_style == 'D':
                                                urlpatterns += patterns('',
-                                                       url(r'^(?P<year>\d{4})/(?P<month>\d{2})/(?P<day>\d{2})/?$', include(self.feed_patterns(self.get_entries_by_ymd, self.entry_archive_page, 'entries_by_day')))
+                                                       url(r'^(?P<year>\d{4})/(?P<month>\d{2})/(?P<day>\d{2})$', include(self.feed_patterns(self.get_entries_by_ymd, self.entry_archive_page, 'entries_by_day')))
                                                )
                
                if self.entry_permalink_style == 'D':
                        urlpatterns += patterns('',
-                               url(r'^(?P<year>\d{4})/(?P<month>\d{2})/(?P<day>\d{2})/(?P<slug>[-\w]+)/?$', self.entry_view)
+                               url(r'^(?P<year>\d{4})/(?P<month>\d{2})/(?P<day>\d{2})/(?P<slug>[-\w]+)$', self.entry_view)
                        )
                elif self.entry_permalink_style == 'M':
                        urlpatterns += patterns('',
-                               url(r'^(?P<year>\d{4})/(?P<month>\d{2})/(?P<slug>[-\w]+)/?$', self.entry_view)
+                               url(r'^(?P<year>\d{4})/(?P<month>\d{2})/(?P<slug>[-\w]+)$', self.entry_view)
                        )
                elif self.entry_permalink_style == 'Y':
                        urlpatterns += patterns('',
-                               url(r'^(?P<year>\d{4})/(?P<slug>[-\w]+)/?$', self.entry_view)
+                               url(r'^(?P<year>\d{4})/(?P<slug>[-\w]+)$', self.entry_view)
                        )
                elif self.entry_permalink_style == 'B':
                        urlpatterns += patterns('',
-                               url((r'^(?:%s)/(?P<slug>[-\w]+)/?$' % self.entry_permalink_base), self.entry_view)
+                               url((r'^%s/(?P<slug>[-\w]+)$' % self.entry_permalink_base), self.entry_view)
                        )
                else:
                        urlpatterns = patterns('',
-                               url(r'^(?P<slug>[-\w]+)/?$', self.entry_view)
+                               url(r'^(?P<slug>[-\w]+)$', self.entry_view)
                        )
                return urlpatterns
        
@@ -355,44 +355,44 @@ class NewsletterView(MultiView, FeedMultiViewMixin):
        def urlpatterns(self):
                urlpatterns = patterns('',
                        url(r'^', include(self.feed_patterns(self.get_all_articles, self.index_page, 'index'))),
-                       url(r'^(?:%s)/(?P<numbering>.+)/' % self.issue_permalink_base, include(self.feed_patterns(self.get_articles_by_issue, self.issue_page, 'issue')))
+                       url(r'^%s/(?P<numbering>.+)' % self.issue_permalink_base, include(self.feed_patterns(self.get_articles_by_issue, self.issue_page, 'issue')))
                )
                if self.issue_archive_page:
                        urlpatterns += patterns('',
-                               url(r'^(?:%s)/$' % self.issue_permalink_base, self.issue_archive_view, 'issue_archive')
+                               url(r'^%s$' % self.issue_permalink_base, self.issue_archive_view, 'issue_archive')
                        )
                if self.article_archive_page:
                        urlpatterns += patterns('',
-                               url(r'^(?:%s)/' % self.article_permalink_base, include(self.feed_patterns(self.get_all_articles, self.article_archive_page, 'articles')))
+                               url(r'^%s' % self.article_permalink_base, include(self.feed_patterns(self.get_all_articles, self.article_archive_page, 'articles')))
                        )
                        if self.article_permalink_style in 'DMY':
                                urlpatterns += patterns('',
-                                       url(r'^(?:%s)/(?P<year>\d{4})/' % self.article_permalink_base, include(self.feed_patterns(self.get_articles_by_ymd, self.article_archive_page, 'articles_by_year')))
+                                       url(r'^%s/(?P<year>\d{4})' % self.article_permalink_base, include(self.feed_patterns(self.get_articles_by_ymd, self.article_archive_page, 'articles_by_year')))
                                )
                                if self.article_permalink_style in 'DM':
                                        urlpatterns += patterns('',
-                                               url(r'^(?:%s)/(?P<year>\d{4})/(?P<month>\d{2})/' % self.article_permalink_base, include(self.feed_patterns(self.get_articles_by_ymd, self.article_archive_page, 'articles_by_month')))
+                                               url(r'^%s/(?P<year>\d{4})/(?P<month>\d{2})' % self.article_permalink_base, include(self.feed_patterns(self.get_articles_by_ymd, self.article_archive_page, 'articles_by_month')))
                                        )
                                        if self.article_permalink_style == 'D':
                                                urlpatterns += patterns('',
-                                                       url(r'^(?:%s)/(?P<year>\d{4})/(?P<month>\d{2})/(?P<day>\d{2})/' % self.article_permalink_base, include(self.feed_patterns(self.get_articles_by_ymd, self.article_archive_page, 'articles_by_day')))
+                                                       url(r'^%s/(?P<year>\d{4})/(?P<month>\d{2})/(?P<day>\d{2})' % self.article_permalink_base, include(self.feed_patterns(self.get_articles_by_ymd, self.article_archive_page, 'articles_by_day')))
                                                )
                
                if self.article_permalink_style == 'Y':
                        urlpatterns += patterns('',
-                               url(r'^(?:%s)/(?P<year>\d{4})/(?P<slug>[\w-]+)/$' % self.article_permalink_base, self.article_view)
+                               url(r'^%s/(?P<year>\d{4})/(?P<slug>[\w-]+)$' % self.article_permalink_base, self.article_view)
                        )
                elif self.article_permalink_style == 'M':
                        urlpatterns += patterns('',
-                               url(r'^(?:%s)/(?P<year>\d{4})/(?P<month>\d{2})/(?P<slug>[\w-]+)/$' % self.article_permalink_base, self.article_view)
+                               url(r'^%s/(?P<year>\d{4})/(?P<month>\d{2})/(?P<slug>[\w-]+)$' % self.article_permalink_base, self.article_view)
                        )
                elif self.article_permalink_style == 'D':
                        urlpatterns += patterns('',
-                               url(r'^(?:%s)/(?P<year>\d{4})/(?P<month>\d{2})/(?P<day>\d{2})/(?P<slug>[\w-]+)/$' % self.article_permalink_base, self.article_view)
+                               url(r'^%s/(?P<year>\d{4})/(?P<month>\d{2})/(?P<day>\d{2})/(?P<slug>[\w-]+)$' % self.article_permalink_base, self.article_view)
                        )
                else:   
                        urlpatterns += patterns('',
-                               url(r'^(?:%s)/(?P<slug>[-\w]+)/?$' % self.article_permalink_base, self.article_view)
+                               url(r'^%s/(?P<slug>[-\w]+)$' % self.article_permalink_base, self.article_view)
                        )
                
                return urlpatterns
index 43c7c91..2c550db 100644 (file)
@@ -56,13 +56,13 @@ class FeedMultiViewMixin(object):
                        current_site = Site.objects.get_current()
                        #Could this be done with request.path instead somehow?
                        feed_kwargs = {
-                               'link': 'http://%s/%s/%s/' % (current_site.domain, request.node.get_absolute_url().strip('/'), reverse(reverse_name, urlconf=self, kwargs=kwargs).strip('/'))
+                               'link': request.node.construct_url(subpath=self.reverse(reverse_name, kwargs=kwargs), request=request, with_domain=True)
                        }
                        feed = self.get_feed(feed_type, extra_context, feed_kwargs)
                        
                        for obj in objects:
                                kwargs = {
-                                       'link': 'http://%s/%s/%s/' % (current_site.domain, request.node.get_absolute_url().strip('/'), self.get_subpath(obj).strip('/'))
+                                       'link': request.node.construct_url(subpath=self.reverse(obj=obj), request=request, with_domain=True)
                                }
                                self.add_item(feed, obj, kwargs=kwargs)
        
@@ -93,7 +93,7 @@ class FeedMultiViewMixin(object):
                if self.feeds_enabled:
                        feed_name = '%s_feed' % base_name
                        urlpatterns = patterns('',
-                               url(r'^%s/$' % self.feed_suffix, self.feed_view(object_fetcher, feed_name), name=feed_name),
+                               url(r'^%s$' % self.feed_suffix, self.feed_view(object_fetcher, feed_name), name=feed_name),
                        ) + urlpatterns
                return urlpatterns
        
index dee16e9..010aa9f 100644 (file)
@@ -248,12 +248,12 @@ class NavigationItem(TreeEntity):
                                params = self.reversing_parameters
                                args = isinstance(params, list) and params or None
                                kwargs = isinstance(params, dict) and params or None
-                               return node.view.reverse(view_name, args=args, kwargs=kwargs, node=node)
+                               subpath = node.view.reverse(view_name, args=args, kwargs=kwargs)
                        else:
                                subpath = self.url_or_subpath
-                               while subpath and subpath[0] == '/':
-                                       subpath = subpath[1:]
-                               return '%s%s' % (node.get_absolute_url(), subpath)
+                               if subpath[0] != '/':
+                                       subpath = '/' + subpath
+                       return node.construct_url(subpath)
                elif node is not None:
                        return node.get_absolute_url()
                else:
index 2f40da7..3286aa0 100644 (file)
@@ -41,32 +41,31 @@ class LoginMultiView(MultiView):
        @property
        def urlpatterns(self):
                urlpatterns = patterns('',
-                       url(r'^login/$', self.login, name='login'),
-                       url(r'^logout/$', self.logout, name='logout'),
+                       url(r'^login$', self.login, name='login'),
+                       url(r'^logout$', self.logout, name='logout'),
                        
-                       url(r'^password/reset/$', csrf_protect(self.password_reset), name='password_reset'),
-                       url(r'^password/reset/(?P<uidb36>\w+)/(?P<token>[^/]+)/$', self.password_reset_confirm, name='password_reset_confirm'),
+                       url(r'^password/reset$', csrf_protect(self.password_reset), name='password_reset'),
+                       url(r'^password/reset/(?P<uidb36>\w+)/(?P<token>[^/]+)$', self.password_reset_confirm, name='password_reset_confirm'),
                        
-                       url(r'^register/$', csrf_protect(self.register), name='register'),
-                       url(r'^register/(?P<uidb36>\w+)/(?P<token>[^/]+)/$', self.register_confirm, name='register_confirm')
+                       url(r'^register$', csrf_protect(self.register), name='register'),
+                       url(r'^register/(?P<uidb36>\w+)/(?P<token>[^/]+)$', self.register_confirm, name='register_confirm')
                )
                
                if self.password_change_page:
                        urlpatterns += patterns('',
-                               url(r'^password/change/$', csrf_protect(self.login_required(self.password_change)), name='password_change'),
+                               url(r'^password/change$', csrf_protect(self.login_required(self.password_change)), name='password_change'),
                        )
                
                return urlpatterns
        
        def make_confirmation_link(self, confirmation_view, token_generator, user, node, token_args=None, reverse_kwargs=None):
-               current_site = Site.objects.get_current()
                token = token_generator.make_token(user, *(token_args or []))
                kwargs = {
                        'uidb36': int_to_base36(user.id),
                        'token': token
                }
                kwargs.update(reverse_kwargs or {})
-               return 'http://%s%s' % (current_site.domain, self.reverse(confirmation_view, kwargs=kwargs, node=node))
+               return node.construct_url(subpath=self.reverse(confirmation_view, kwargs=kwargs), with_domain=True)
        
        def display_login_page(self, request, message, extra_context=None):
                request.session.set_test_cookie()
@@ -336,8 +335,8 @@ class AccountMultiView(LoginMultiView):
        def urlpatterns(self):
                urlpatterns = super(AccountMultiView, self).urlpatterns
                urlpatterns += patterns('',
-                       url(r'^account/$', self.login_required(self.account_view), name='account'),
-                       url(r'^account/email/(?P<uidb36>\w+)/(?P<email>[\w.]+[+][\w.]+)/(?P<token>[^/]+)/$', self.email_change_confirm, name='email_change_confirm')
+                       url(r'^account$', self.login_required(self.account_view), name='account'),
+                       url(r'^account/email/(?P<uidb36>\w+)/(?P<email>[\w.]+[+][\w.]+)/(?P<token>[^/]+)$', self.email_change_confirm, name='email_change_confirm')
                )
                return urlpatterns
        
index 1e4b9d9..f53083d 100644 (file)
@@ -5,12 +5,12 @@ MIDDLEWARE_NOT_CONFIGURED = ImproperlyConfigured("""Philo requires the RequestNo
 
 
 class ViewDoesNotProvideSubpaths(Exception):
-       """ Raised by get_subpath when the View does not provide subpaths (the default). """
+       """ Raised by View.reverse when the View does not provide subpaths (the default). """
        silent_variable_failure = True
 
 
 class ViewCanNotProvideSubpath(Exception):
-       """ Raised by get_subpath when the View can not provide a subpath for the supplied object. """
+       """ Raised by View.reverse when the View can not provide a subpath for the supplied arguments. """
        silent_variable_failure = True
 
 
index ad660ec..832bd16 100644 (file)
@@ -20,6 +20,10 @@ class LazyNode(object):
                        except Node.DoesNotExist:
                                node = None
                        
+                       if subpath is None:
+                               subpath = ""
+                       subpath = "/" + subpath
+                       
                        if node:
                                node.subpath = subpath
                        
index 526400c..ab8adf7 100644 (file)
@@ -1,9 +1,8 @@
 from django.db import models
 from django.contrib.contenttypes.models import ContentType
 from django.contrib.contenttypes import generic
-from django.contrib.sites.models import Site
-from django.http import HttpResponse, HttpResponseServerError, HttpResponseRedirect
-from django.core.exceptions import ViewDoesNotExist
+from django.contrib.sites.models import Site, RequestSite
+from django.http import HttpResponse, HttpResponseServerError, HttpResponseRedirect, Http404
 from django.core.servers.basehttp import FileWrapper
 from django.core.urlresolvers import resolve, clear_url_caches, reverse, NoReverseMatch
 from django.template import add_to_builtins as register_templatetags
@@ -30,23 +29,53 @@ class Node(TreeEntity):
                        return self.view.accepts_subpath
                return False
        
+       def handles_subpath(self, subpath):
+               return self.view.handles_subpath(subpath)
+       
        def render_to_response(self, request, extra_context=None):
                return self.view.render_to_response(request, extra_context)
        
-       def get_absolute_url(self):
+       def get_absolute_url(self, request=None, with_domain=False, secure=False):
+               return self.construct_url(request=request, with_domain=with_domain, secure=secure)
+       
+       def construct_url(self, subpath=None, request=None, with_domain=False, secure=False):
+               """
+               This method will construct a URL based on the Node's location.
+               If a request is passed in, that will be used as a backup in case
+               the Site lookup fails. The Site lookup takes precedence because
+               it's what's used to find the root node. This will raise:
+               - NoReverseMatch if philo-root is not reverseable
+               - Site.DoesNotExist if a domain is requested but not buildable.
+               - AncestorDoesNotExist if the root node of the site isn't an
+                 ancestor of this instance.
+               """
+               # Try reversing philo-root first, since we can't do anything if that fails.
+               root_url = reverse('philo-root')
+               
                try:
-                       root = Site.objects.get_current().root_node
+                       current_site = Site.objects.get_current()
                except Site.DoesNotExist:
-                       root = None
+                       if request is not None:
+                               current_site = RequestSite(request)
+                       elif with_domain:
+                               # If they want a domain and we can't figure one out,
+                               # best to reraise the error to let them know.
+                               raise
+                       else:
+                               current_site = None
                
-               try:
-                       path = self.get_path(root=root)
-                       if path:
-                               path += '/'
-                       root_url = reverse('philo-root')
-                       return '%s%s' % (root_url, path)
-               except AncestorDoesNotExist, ViewDoesNotExist:
-                       return None
+               root = getattr(current_site, 'root_node', None)
+               path = self.get_path(root=root)
+               
+               if current_site and with_domain:
+                       domain = "http%s://%s" % (secure and "s" or "", current_site.domain)
+               else:
+                       domain = ""
+               
+               if not path:
+                       subpath = subpath[1:]
+               
+               return '%s%s%s%s' % (domain, root_url, path, subpath or "")
        
        class Meta:
                app_label = 'philo'
@@ -61,15 +90,34 @@ class View(Entity):
        
        accepts_subpath = False
        
-       def get_subpath(self, obj):
+       def handles_subpath(self, subpath):
+               if not self.accepts_subpath and subpath != "/":
+                       return False
+               return True
+       
+       def reverse(self, view_name=None, args=None, kwargs=None, node=None, obj=None):
+               """Shortcut method to handle the common pattern of getting the
+               absolute url for a view's subpaths."""
                if not self.accepts_subpath:
                        raise ViewDoesNotProvideSubpaths
                
-               view_name, args, kwargs = self.get_reverse_params(obj)
+               if obj is not None:
+                       # Perhaps just override instead of combining?
+                       obj_view_name, obj_args, obj_kwargs = self.get_reverse_params(obj)
+                       if view_name is None:
+                               view_name = obj_view_name
+                       args = list(obj_args) + list(args or [])
+                       obj_kwargs.update(kwargs or {})
+                       kwargs = obj_kwargs
+               
                try:
-                       return reverse(view_name, args=args, kwargs=kwargs, urlconf=self)
+                       subpath = reverse(view_name, urlconf=self, args=args or [], kwargs=kwargs or {})
                except NoReverseMatch:
                        raise ViewCanNotProvideSubpath
+               
+               if node is not None:
+                       return node.construct_url(subpath)
+               return subpath
        
        def get_reverse_params(self, obj):
                """This method should return a view_name, args, kwargs tuple suitable for reversing a url for the given obj using self as the urlconf."""
@@ -105,12 +153,18 @@ class MultiView(View):
        def urlpatterns(self):
                raise NotImplementedError("MultiView subclasses must implement urlpatterns.")
        
+       def handles_subpath(self, subpath):
+               if not super(MultiView, self).handles_subpath(subpath):
+                       return False
+               try:
+                       resolve(subpath, urlconf=self)
+               except Http404:
+                       return False
+               return True
+       
        def actually_render_to_response(self, request, extra_context=None):
                clear_url_caches()
                subpath = request.node.subpath
-               if not subpath:
-                       subpath = ""
-               subpath = "/" + subpath
                view, args, kwargs = resolve(subpath, urlconf=self)
                view_args = getargspec(view)
                if extra_context is not None and ('extra_context' in view_args[0] or view_args[2] is not None):
@@ -119,14 +173,6 @@ class MultiView(View):
                        kwargs['extra_context'] = extra_context
                return view(request, *args, **kwargs)
        
-       def reverse(self, view_name, args=None, kwargs=None, node=None):
-               """Shortcut method to handle the common pattern of getting the absolute url for a multiview's
-               subpaths."""
-               subpath = reverse(view_name, urlconf=self, args=args or [], kwargs=kwargs or {})
-               if node is not None:
-                       return '/%s/%s/' % (node.get_absolute_url().strip('/'), subpath.strip('/'))
-               return subpath
-       
        def get_context(self):
                """Hook for providing instance-specific context - such as the value of a Field - to all views."""
                return {}
index 73492d4..5ae507d 100644 (file)
@@ -55,10 +55,7 @@ class NodeURLNode(template.Node):
                                                raise
                                        return settings.TEMPLATE_STRING_IF_INVALID
                        else:
-                               if subpath[0] == '/':
-                                       subpath = subpath[1:]
-                               
-                               url = node.get_absolute_url() + subpath
+                               url = node.construct_url(subpath)
                
                if self.as_var:
                        context[self.as_var] = url
index 255e54e..cec9c57 100644 (file)
--- a/views.py
+++ b/views.py
@@ -1,5 +1,6 @@
 from django.conf import settings
-from django.http import Http404
+from django.core.urlresolvers import resolve
+from django.http import Http404, HttpResponseRedirect
 from django.views.decorators.vary import vary_on_headers
 from philo.exceptions import MIDDLEWARE_NOT_CONFIGURED
 
@@ -15,6 +16,30 @@ def node_view(request, path=None, **kwargs):
        node = request.node
        subpath = request.node.subpath
        
-       if subpath and not node.accepts_subpath:
-               raise Http404
+       if not node.handles_subpath(subpath):
+               # If the subpath isn't handled, check settings.APPEND_SLASH. If
+               # it's True, try to correct the subpath.
+               if not settings.APPEND_SLASH:
+                       raise Http404
+               
+               if subpath[-1] == "/":
+                       subpath = subpath[:-1]
+               else:
+                       subpath += "/"
+               
+               redirect_url = node.construct_url(subpath)
+               
+               if node.handles_subpath(subpath):
+                       return HttpResponseRedirect(redirect_url)
+               
+               # Perhaps there is a non-philo view at this address. Can we
+               # resolve *something* there besides node_view? If not,
+               # raise a 404.
+               view, args, kwargs = resolve(redirect_url)
+               
+               if view == node_view:
+                       raise Http404
+               else:
+                       return HttpResponseRedirect(redirect_url)
+       
        return node.render_to_response(request, kwargs)
\ No newline at end of file