Skip to content

Commit

Permalink
Merge pull request #4102 from NyanKiyoshi/perfs/lazy-middlewares
Browse files Browse the repository at this point in the history
Separate the legacy middleware from the GQL API middleware
  • Loading branch information
maarcingebala committed May 16, 2019
2 parents c6612aa + 81f7fb7 commit 94c0703
Show file tree
Hide file tree
Showing 6 changed files with 134 additions and 23 deletions.
2 changes: 1 addition & 1 deletion .isort.cfg

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ All notable, unreleased changes to this project will be documented in this file.
- Fix GATEWAYS_ENUM to always contain all implemented payment gateways - #4108 by @koradon
- Fix translation discard button - #4109 by @benekex2
- Change input style and improve Storybook stories - #4115 by @dominik-zeglen
- Separated the legacy middleware from the GQL API middleware - #4102 by @NyanKiyoshi

## 2.6.0

Expand Down
83 changes: 78 additions & 5 deletions saleor/core/middleware.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,22 @@
import logging
from datetime import date

from functools import wraps
from typing import Callable

import django.contrib.auth.middleware
import django.contrib.messages.middleware
import django.contrib.sessions.middleware
import django.middleware.common
import django.middleware.csrf
import django.middleware.locale
import django.middleware.security
import django_babel.middleware
import impersonate.middleware
import social_django.middleware
from django.conf import settings
from django.contrib.sites.models import Site
from django.core.exceptions import MiddlewareNotUsed
from django.urls import reverse
from django.utils.functional import SimpleLazyObject
from django.utils.translation import get_language
from django_countries.fields import Country
Expand All @@ -15,9 +29,66 @@
logger = logging.getLogger(__name__)


def django_only_request_handler(get_response: Callable, handler: Callable):
api_path = reverse("api")

@wraps(handler)
def handle_request(request):
if request.path == api_path:
return get_response(request)
return handler(request)

return handle_request


def django_only_middleware(middleware):
@wraps(middleware)
def wrapped(get_response):
handler = middleware(get_response)
return django_only_request_handler(get_response, handler)

return wrapped


social_auth_exception_middleware = django_only_middleware(
social_django.middleware.SocialAuthExceptionMiddleware
)
impersonate_middleware = django_only_middleware(
impersonate.middleware.ImpersonateMiddleware
)
babel_locale_middleware = django_only_middleware(
django_babel.middleware.LocaleMiddleware
)
django_locale_middleware = django_only_middleware(
django.middleware.locale.LocaleMiddleware
)
django_messages_middleware = django_only_middleware(
django.contrib.messages.middleware.MessageMiddleware
)
django_auth_middleware = django_only_middleware(
django.contrib.auth.middleware.AuthenticationMiddleware
)
django_csrf_view_middleware = django_only_middleware(
django.middleware.csrf.CsrfViewMiddleware
)
django_common_middleware = django_only_middleware(
django.middleware.common.CommonMiddleware
)
django_security_middleware = django_only_middleware(
django.middleware.security.SecurityMiddleware
)
django_session_middleware = django_only_middleware(
django.contrib.sessions.middleware.SessionMiddleware
)


@django_only_middleware
def google_analytics(get_response):
"""Report a page view to Google Analytics."""

if not settings.GOOGLE_ANALYTICS_TRACKING_ID:
raise MiddlewareNotUsed()

def middleware(request):
client_id = analytics.get_client_id(request)
path = request.path
Expand All @@ -38,10 +109,9 @@ def discounts(get_response):
"""Assign active discounts to `request.discounts`."""

def middleware(request):
discounts = Sale.objects.active(date.today()).prefetch_related(
request.discounts = Sale.objects.active(date.today()).prefetch_related(
"products", "categories", "collections"
)
request.discounts = discounts
return get_response(request)

return middleware
Expand Down Expand Up @@ -83,9 +153,12 @@ def site(get_response):
the cache. Using this middleware solves this problem.
"""

def middleware(request):
def _get_site():
Site.objects.clear_cache()
request.site = Site.objects.get_current()
return Site.objects.get_current()

def middleware(request):
request.site = SimpleLazyObject(_get_site)
return get_response(request)

return middleware
Expand Down
37 changes: 30 additions & 7 deletions saleor/graphql/middleware.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,33 @@
from functools import wraps
from typing import Callable

from django.contrib.auth.models import AnonymousUser
from django.shortcuts import reverse
from django.urls import reverse
from graphene_django.settings import graphene_settings
from graphql_jwt.middleware import JSONWebTokenMiddleware


def api_only_request_handler(get_response: Callable, handler: Callable):
@wraps(handler)
def handle_request(request):
api_path = reverse("api")
if request.path != api_path:
return get_response(request)
return handler(request)

return handle_request


def api_only_middleware(middleware):
@wraps(middleware)
def wrapped(get_response):
handler = middleware(get_response)
return api_only_request_handler(get_response, handler)

return wrapped


@api_only_middleware
def jwt_middleware(get_response):
"""Authenticate user using JSONWebTokenMiddleware
ignoring the session-based authentication.
Expand All @@ -18,13 +42,12 @@ def jwt_middleware(get_response):
graphene_settings.MIDDLEWARE.remove(JSONWebTokenMiddleware)

def middleware(request):
if request.path == reverse("api"):
# clear user authenticated by AuthenticationMiddleware
request._cached_user = AnonymousUser()
request.user = AnonymousUser()
# clear user authenticated by AuthenticationMiddleware
request._cached_user = AnonymousUser()
request.user = AnonymousUser()

# authenticate using JWT middleware
jwt_middleware_inst.process_request(request)
# authenticate using JWT middleware
jwt_middleware_inst.process_request(request)
return get_response(request)

return middleware
20 changes: 10 additions & 10 deletions saleor/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,22 +195,22 @@ def get_bool_from_env(name, default_value):
SECRET_KEY = os.environ.get("SECRET_KEY")

MIDDLEWARE = [
"django.contrib.sessions.middleware.SessionMiddleware",
"django.middleware.security.SecurityMiddleware",
"django.middleware.common.CommonMiddleware",
"django.middleware.csrf.CsrfViewMiddleware",
"django.contrib.auth.middleware.AuthenticationMiddleware",
"django.contrib.messages.middleware.MessageMiddleware",
"django.middleware.locale.LocaleMiddleware",
"django_babel.middleware.LocaleMiddleware",
"saleor.core.middleware.django_session_middleware",
"saleor.core.middleware.django_security_middleware",
"saleor.core.middleware.django_common_middleware",
"saleor.core.middleware.django_csrf_view_middleware",
"saleor.core.middleware.django_auth_middleware",
"saleor.core.middleware.django_messages_middleware",
"saleor.core.middleware.django_locale_middleware",
"saleor.core.middleware.babel_locale_middleware",
"saleor.core.middleware.discounts",
"saleor.core.middleware.google_analytics",
"saleor.core.middleware.country",
"saleor.core.middleware.currency",
"saleor.core.middleware.site",
"saleor.core.middleware.taxes",
"social_django.middleware.SocialAuthExceptionMiddleware",
"impersonate.middleware.ImpersonateMiddleware",
"saleor.core.middleware.social_auth_exception_middleware",
"saleor.core.middleware.impersonate_middleware",
"saleor.graphql.middleware.jwt_middleware",
]

Expand Down
14 changes: 14 additions & 0 deletions tests/api/test_graphql.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,20 @@
from tests.api.utils import get_graphql_content


def test_middleware_dont_generate_sql_requests(
client, settings, django_assert_num_queries
):
"""When requesting on the GraphQL API endpoint, no SQL request should happen
indirectly. This test ensures that."""

# Enables the Graphql playground
settings.DEBUG = True

with django_assert_num_queries(0):
response = client.get(reverse("api"))
assert response.status_code == 200


def test_jwt_middleware(admin_user):
def get_response(request):
return HttpResponse()
Expand Down

0 comments on commit 94c0703

Please sign in to comment.