Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support URLRouter with include #2110

Open
wants to merge 26 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 78 additions & 2 deletions channels/routing.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import importlib
import re

from django.conf import settings
from django.core.exceptions import ImproperlyConfigured
from django.urls.exceptions import Resolver404
from django.urls.resolvers import RegexPattern, RoutePattern, URLResolver
from django.urls.resolvers import RegexPattern, RoutePattern, URLPattern, URLResolver

"""
All Routing instances inside this file are also valid ASGI applications - with
Expand Down Expand Up @@ -52,6 +53,71 @@ async def __call__(self, scope, receive, send):
)


def _parse_resolver(child_url_pattern, parent_resolver, parent_regex, routes):
"""
Parse resolver (returned by `include`) recurrsively

Parameters
----------
child_url_pattern : URLResolver | Any
The child url pattern
parent_resolver : URLResolver
The parent resolver
parent_regex : Pattern
The parent regex pattern
routes : list[URLPattern]
The URLPattern's list that stores the routes

Returns
-------
list[URLPattern]
The URLPattern's list that stores the routes
"""
if isinstance(child_url_pattern, URLResolver):
# parse the urls resolved by django's `include` function
for url_pattern in child_url_pattern.url_patterns:
# call _parse_resolver recurrsively to parse nested URLResolver
routes.extend(
_parse_resolver(
url_pattern,
child_url_pattern,
parent_resolver.pattern.regex,
routes,
)
)
else:
# concatenate parent's url (route) and child's url (url_pattern)
regex = "".join(
x.pattern
for x in [
parent_regex,
parent_resolver.pattern.regex,
child_url_pattern.pattern.regex,
]
)

# Remove the redundant caret ^ which is appended by `path` function
regex = re.sub(r"(?<!^)\^", "", regex)

name = (
f"{parent_resolver.app_name}:{child_url_pattern.name}"
if child_url_pattern.name
else None
)
pattern = RegexPattern(regex, name=name, is_endpoint=True)

routes.append(
URLPattern(
pattern,
child_url_pattern.callback,
child_url_pattern.default_args,
name,
)
)

return routes


class URLRouter:
"""
Routes to different applications/consumers based on the URL path.
Expand All @@ -67,7 +133,17 @@ class URLRouter:
_path_routing = True

def __init__(self, routes):
self.routes = routes
new_routes = []
for route in routes:
if not route.callback and isinstance(route, URLResolver):
# parse the urls resolved by django's `include` function
for url_pattern in route.url_patterns:
new_routes.extend(
_parse_resolver(url_pattern, route, re.compile(r""), [])
)
else:
new_routes.append(route)
self.routes = new_routes

for route in self.routes:
# The inner ASGI app wants to do additional routing, route
Expand Down
25 changes: 22 additions & 3 deletions docs/topics/routing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,29 @@ would do this:

stream = self.scope["url_route"]["kwargs"]["stream"]

Please note that ``URLRouter`` nesting will not work properly with
``path()`` routes if inner routers are wrapped by additional middleware.
See `Issue #1428 <https://github.com/django/channels/issues/1428>`__.

You can use [include](https://docs.djangoproject.com/en/5.1/ref/urls/#include)
function for nested routings. This is similar as Django's URL routing system.

Here's an example for nested routings. When you configure the routings in parent ``routings.py``;

.. code-block:: python

urlpatterns = [
path("app1/", include("app1.routings"), name="app1"),
]

and in child ``app1/routings.py``;

.. code-block:: python

app_name = 'app1'

urlpatterns = [
re_path(r"chats/(\d+)/$", test_app, name="chats"),
]

This would resolve to a path such as ``/app1/chats/5/``.

ChannelNameRouter
-----------------
Expand Down
13 changes: 13 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import sys

import pytest
from django.conf import settings

Expand Down Expand Up @@ -38,3 +40,14 @@ def samesite(request, settings):
def samesite_invalid(settings):
"""Set samesite flag to strict."""
settings.SESSION_COOKIE_SAMESITE = "Hello"


@pytest.fixture(autouse=True)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not keen on the autouse. I can't see just from the code whether or when it's in play.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is for rolling back the changes by sys.module.

And then, should I use files instead of sys.modules?

@jjjkkkjjj hold off on that last. If you could add the drafts for the docs (don't have to be perfect) I can think it through properly

I ask you again.
Should I create the test files instead of using sys.module?

def mock_modules():
"""Save original modules for each test and clear a cache"""
original_modules = sys.modules.copy()
yield
sys.modules = original_modules
from django.urls.base import _get_cached_resolver

_get_cached_resolver.cache_clear()
Loading
Loading