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

Fix when Session, Authentication or Message middlewares are not present #667

Merged
merged 1 commit into from
Aug 15, 2024
Merged
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
16 changes: 16 additions & 0 deletions project/tests/test_silky_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from django.urls import reverse

from silk.config import SilkyConfig
from silk.errors import SilkNotConfigured
from silk.middleware import SilkyMiddleware, _should_intercept
from silk.models import Request

Expand Down Expand Up @@ -100,6 +101,21 @@ def test_no_mappings(self):
]
middleware._apply_dynamic_mappings() # Just checking no crash

def test_raise_if_authentication_is_enable_but_no_middlewares(self):
SilkyConfig().SILKY_AUTHENTICATION = True
with self.modify_settings(MIDDLEWARE={
'remove': [
'django.contrib.sessions.middleware.SessionMiddleware',
'django.contrib.auth.middleware.AuthenticationMiddleware',
'django.contrib.messages.middleware.MessageMiddleware',
],
}):
with self.assertRaisesMessage(
SilkNotConfigured,
"SILKY_AUTHENTICATION can not be enabled without Session, Authentication or Message Django's middlewares"
albertyw marked this conversation as resolved.
Show resolved Hide resolved
):
SilkyMiddleware(fake_get_response)


class TestShouldIntercept(TestCase):
def test_should_intercept_non_silk_request(self):
Expand Down
46 changes: 46 additions & 0 deletions project/tests/test_view_profiling.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from django.test import TestCase

from silk.middleware import silky_reverse
from silk.views.profiling import ProfilingView

from .test_lib.assertion import dict_contains
Expand Down Expand Up @@ -92,3 +93,48 @@ def test_get(self):
'options_func_names': ProfilingView()._get_function_names()
}, context))
self.assertIn('results', context)

def test_view_without_session_and_auth_middlewares(self):
"""
Filters are not present because there is no `session` to store them.
"""
with self.modify_settings(MIDDLEWARE={
'remove': [
'django.contrib.sessions.middleware.SessionMiddleware',
'django.contrib.auth.middleware.AuthenticationMiddleware',
'django.contrib.messages.middleware.MessageMiddleware',
],
}):
# test filters on GET
show = 10
func_name = 'func_name'
name = 'name'
order_by = 'Time'
response = self.client.get(silky_reverse('profiling'), {
'show': show,
'func_name': func_name,
'name': name,
'order_by': order_by
})
context = response.context
self.assertTrue(dict_contains({
'show': show,
'order_by': order_by,
'func_name': func_name,
'name': name,
'options_show': ProfilingView.show,
'options_order_by': ProfilingView.order_by,
'options_func_names': ProfilingView()._get_function_names()
}, context))

# test filters on POST
response = self.client.post(silky_reverse('profiling'), {
'filter-overalltime-value': 100,
'filter-overalltime-typ': 'TimeSpentOnQueriesFilter',
})
context = response.context
self.assertTrue(dict_contains({
'filters': {
'overalltime': {'typ': 'TimeSpentOnQueriesFilter', 'value': 100, 'str': 'DB Time >= 100'}
},
}, context))
39 changes: 39 additions & 0 deletions project/tests/test_view_requests.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,45 @@ def test_post(self):
self.assertQuerysetEqual(context['options_paths'], RequestsView()._get_paths())
self.assertIn('results', context)

def test_view_without_session_and_auth_middlewares(self):
"""
Filters are not present because there is no `session` to store them.
"""
with self.modify_settings(MIDDLEWARE={
'remove': [
'django.contrib.sessions.middleware.SessionMiddleware',
'django.contrib.auth.middleware.AuthenticationMiddleware',
'django.contrib.messages.middleware.MessageMiddleware',
],
}):
# test filters on GET
show = 10
path = '/path/to/somewhere/'
order_by = 'path'
response = self.client.get(silky_reverse('requests'), {
'show': show,
'path': path,
'order_by': order_by,
})
context = response.context
self.assertTrue(dict_contains({
'show': show,
'order_by': order_by,
'path': path,
}, context))

# test filters on POST
response = self.client.post(silky_reverse('requests'), {
'filter-overalltime-value': 100,
'filter-overalltime-typ': 'TimeSpentOnQueriesFilter',
})
context = response.context
self.assertTrue(dict_contains({
'filters': {
'overalltime': {'typ': 'TimeSpentOnQueriesFilter', 'value': 100, 'str': 'DB Time >= 100'}
},
}, context))


class TestGetObjects(TestCase):
def assertSorted(self, objects, sort_field):
Expand Down
26 changes: 26 additions & 0 deletions project/tests/test_view_summary_view.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
from django.test import TestCase

from silk.middleware import silky_reverse
from silk.views.summary import SummaryView

from .test_lib.assertion import dict_contains
from .test_lib.mock_suite import MockSuite

mock_suite = MockSuite()
Expand All @@ -11,3 +13,27 @@ class TestSummaryView(TestCase):
def test_longest_query_by_view(self):
[mock_suite.mock_request() for _ in range(0, 10)]
print([x.time_taken for x in SummaryView()._longest_query_by_view([])])

def test_view_without_session_and_auth_middlewares(self):
"""
Filters are not present because there is no `session` to store them.
"""
with self.modify_settings(MIDDLEWARE={
'remove': [
'django.contrib.sessions.middleware.SessionMiddleware',
'django.contrib.auth.middleware.AuthenticationMiddleware',
'django.contrib.messages.middleware.MessageMiddleware',
],
}):
# test filters on POST
seconds = 3600
response = self.client.post(silky_reverse('summary'), {
'filter-seconds-value': seconds,
'filter-seconds-typ': 'SecondsFilter',
})
context = response.context
self.assertTrue(dict_contains({
'filters': {
'seconds': {'typ': 'SecondsFilter', 'value': seconds, 'str': f'>{seconds} seconds ago'}
}
}, context))
22 changes: 22 additions & 0 deletions silk/middleware.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
import logging
import random

from django.conf import settings
from django.db import DatabaseError, transaction
from django.db.models.sql.compiler import SQLCompiler
from django.urls import NoReverseMatch, reverse
from django.utils import timezone
from django.utils.translation import gettext_lazy as _

from silk.collector import DataCollector
from silk.config import SilkyConfig
from silk.errors import SilkNotConfigured
from silk.model_factory import RequestModelFactory, ResponseModelFactory
from silk.profiling import dynamic
from silk.profiling.profiler import silk_meta_profiler
Expand All @@ -32,6 +35,11 @@ def get_fpath():


config = SilkyConfig()
AUTH_AND_SESSION_MIDDLEWARES = [
'django.contrib.sessions.middleware.SessionMiddleware',
'django.contrib.auth.middleware.AuthenticationMiddleware',
'django.contrib.messages.middleware.MessageMiddleware',
]


def _should_intercept(request):
Expand Down Expand Up @@ -64,11 +72,25 @@ def process_request(self, request):

class SilkyMiddleware:
def __init__(self, get_response):
if config.SILKY_AUTHENTICATION and not (
set(AUTH_AND_SESSION_MIDDLEWARES) & set(settings.MIDDLEWARE)
):
raise SilkNotConfigured(
_("SILKY_AUTHENTICATION can not be enabled without Session, "
"Authentication or Message Django's middlewares")
)

self.get_response = get_response

def __call__(self, request):
self.process_request(request)

# To be able to persist filters when Session and Authentication
# middlewares are not present.
# Unlike session (which stores in DB) it won't persist filters
# after refresh the page.
request.silk_filters = {}

response = self.get_response(request)

response = self.process_response(request, response)
Expand Down
15 changes: 15 additions & 0 deletions silk/request_filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -230,3 +230,18 @@ def filters_from_request(request):
except FilterValidationError:
logger.warning(f'Validation error when processing filter {typ}({value})')
return filters


class FiltersManager:
def __init__(self, filters_key):
self.key = filters_key

def save(self, request, filters):
if hasattr(request, 'session'):
request.session[self.key] = filters
request.silk_filters = filters

def get(self, request):
if hasattr(request, 'session'):
return request.session.get(self.key, {})
return request.silk_filters
8 changes: 5 additions & 3 deletions silk/views/profiling.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

from silk.auth import login_possibly_required, permissions_possibly_required
from silk.models import Profile, Request
from silk.request_filters import BaseFilter, filters_from_request
from silk.request_filters import BaseFilter, FiltersManager, filters_from_request


class ProfilingView(View):
Expand All @@ -20,6 +20,7 @@ class ProfilingView(View):
'Time on queries']
defualt_order_by = 'Recent'
session_key_profile_filters = 'session_key_profile_filters'
filters_manager = FiltersManager(session_key_profile_filters)

def __init__(self, **kwargs):
super().__init__(**kwargs)
Expand Down Expand Up @@ -90,7 +91,7 @@ def _create_context(self, request, *args, **kwargs):
show = int(show)
func_name = request.GET.get('func_name', None)
name = request.GET.get('name', None)
filters = request.session.get(self.session_key_profile_filters, {})
filters = self.filters_manager.get(request)
context = {
'show': show,
'order_by': order_by,
Expand Down Expand Up @@ -127,5 +128,6 @@ def get(self, request, *args, **kwargs):
@method_decorator(permissions_possibly_required)
def post(self, request):
filters = filters_from_request(request)
request.session[self.session_key_profile_filters] = {ident: f.as_dict() for ident, f in filters.items()}
filters_as_dict = {ident: f.as_dict() for ident, f in filters.items()}
self.filters_manager.save(request, filters_as_dict)
return render(request, 'silk/profiling.html', self._create_context(request))
17 changes: 9 additions & 8 deletions silk/views/requests.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

from silk.auth import login_possibly_required, permissions_possibly_required
from silk.models import Request, Response
from silk.request_filters import BaseFilter, filters_from_request
from silk.request_filters import BaseFilter, FiltersManager, filters_from_request

__author__ = 'mtford'

Expand Down Expand Up @@ -59,6 +59,7 @@ class RequestsView(View):
default_view_style = 'card'

session_key_request_filters = 'request_filters'
filters_manager = FiltersManager(session_key_request_filters)

@property
def options_order_by(self):
Expand Down Expand Up @@ -130,7 +131,7 @@ def _get_objects(self, show=None, order_by=None, order_dir=None, path=None, filt
return query_set[:show]

def _create_context(self, request):
raw_filters = request.session.get(self.session_key_request_filters, {}).copy()
raw_filters = self.filters_manager.get(request).copy()
show = raw_filters.pop('show', self.default_show)
order_by = raw_filters.pop('order_by', self.default_order_by)
order_dir = raw_filters.pop('order_dir', self.default_order_dir)
Expand Down Expand Up @@ -169,22 +170,22 @@ def get(self, request):
if request.GET:
filters = {
# filters from previous session
**request.session.get(self.session_key_request_filters, {}),
**self.filters_manager.get(request),
# new filters from GET, overriding old
**{k: v for k, v in request.GET.items() if k in ['show', 'order_by', 'order_dir', 'view_style']},
}
request.session[self.session_key_request_filters] = filters
self.filters_manager.save(request, filters)
return render(request, 'silk/requests.html', self._create_context(request))

@method_decorator(login_possibly_required)
@method_decorator(permissions_possibly_required)
def post(self, request):
previous_session = request.session.get(self.session_key_request_filters, {})
filters = filters_from_request(request)
request.session[self.session_key_request_filters] = {
previous_session = self.filters_manager.get(request)
filters = {
# filters from previous session but only GET values
**{k: v for k, v in previous_session.items() if k in ['show', 'order_by', 'order_dir', 'view_style']},
# new filters from POST, overriding old
**{ident: f.as_dict() for ident, f in filters.items()},
**{ident: f.as_dict() for ident, f in filters_from_request(request).items()},
}
self.filters_manager.save(request, filters)
return render(request, 'silk/requests.html', self._create_context(request))
9 changes: 5 additions & 4 deletions silk/views/summary.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@

from silk import models
from silk.auth import login_possibly_required, permissions_possibly_required
from silk.request_filters import BaseFilter, filters_from_request
from silk.request_filters import BaseFilter, FiltersManager, filters_from_request


class SummaryView(View):
filters_key = 'summary_filters'
filters_manager = FiltersManager(filters_key)

def _avg_num_queries(self, filters):
queries__aggregate = models.Request.objects.filter(*filters).annotate(num_queries=Count('queries')).aggregate(num=Avg('num_queries'))
Expand Down Expand Up @@ -54,7 +55,7 @@ def _num_queries_by_view(self, filters):
return sorted(requests, key=lambda item: item.t, reverse=True)

def _create_context(self, request):
raw_filters = request.session.get(self.filters_key, {})
raw_filters = self.filters_manager.get(request)
filters = [BaseFilter.from_dict(filter_d) for _, filter_d in raw_filters.items()]
avg_overall_time = self._avg_num_queries(filters)
c = {
Expand All @@ -81,6 +82,6 @@ def get(self, request):
@method_decorator(login_possibly_required)
@method_decorator(permissions_possibly_required)
def post(self, request):
filters = filters_from_request(request)
request.session[self.filters_key] = {ident: f.as_dict() for ident, f in filters.items()}
filters = {ident: f.as_dict() for ident, f in filters_from_request(request).items()}
self.filters_manager.save(request, filters)
return render(request, 'silk/summary.html', self._create_context(request))
Loading