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 19 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
102 changes: 100 additions & 2 deletions channels/routing.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
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, URLResolver, URLPattern
from django.urls import reverse as django_reverse

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


def _parse_resolver(child_url_pattern, parent_resolver, parent_regex, routes):
"""Parse resolver (returned by `include`) recurrsively
jjjkkkjjj marked this conversation as resolved.
Show resolved Hide resolved

Parameters
----------
child_url_pattern : URLResolver |
Copy link
Member

Choose a reason for hiding this comment

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

Why the trailing |?

Copy link
Author

@jjjkkkjjj jjjkkkjjj Oct 8, 2024

Choose a reason for hiding this comment

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

Ah, I forgot to append to Any.
This function receives the url_pattern returned by path, re_path (Any) and include (URLResolver)

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 Expand Up @@ -158,3 +234,25 @@ async def __call__(self, scope, receive, send):
raise ValueError(
"No application configured for channel name %r" % scope["channel"]
)


def reverse(*args, urlconf=None, **kwargs):
"""reverse wrapper for django's reverse function

Parameters
----------
urlconf : str, optional
The root path of the routings, by default None

See the django's
[reverse](https://docs.djangoproject.com/en/5.0/ref/urlresolvers/#reverse)
for more details of the other arguments

Returns
-------
str
The reversed url
"""
if urlconf is None:
urlconf = settings.ROOT_WEBSOCKET_URLCONF
return django_reverse(*args, urlconf=urlconf, **kwargs)
18 changes: 18 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import pytest
import sys
from django.conf import settings


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


@pytest.fixture
def root_urlconf(settings):
"""Set ROOT_WEBSOCKET_URLCONF."""
settings.ROOT_WEBSOCKET_URLCONF = "__src.routings"
return settings.ROOT_WEBSOCKET_URLCONF


@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