Skip to content

Commit

Permalink
Fix when Session, Authentication or Message middlewares are not present
Browse files Browse the repository at this point in the history
  • Loading branch information
mgaligniana committed Aug 14, 2023
1 parent 75cbbcf commit 753ced8
Show file tree
Hide file tree
Showing 9 changed files with 189 additions and 15 deletions.
18 changes: 18 additions & 0 deletions project/tests/test_silky_middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@

from django.test import TestCase, override_settings
from django.urls import reverse
from django.utils.translation import gettext_lazy as _

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 +102,22 @@ 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")
):
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
19 changes: 19 additions & 0 deletions silk/request_filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -230,3 +230,22 @@ 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
elif hasattr(request, 'silk_filters'):
request.silk_filters = filters

def get(self, request):
if hasattr(request, 'session'):
return request.session.get(self.key, {})
elif hasattr(request, 'silk_filters'):
return request.silk_filters

return {}

Check warning on line 251 in silk/request_filters.py

View check run for this annotation

Codecov / codecov/patch

silk/request_filters.py#L251

Added line #L251 was not covered by tests
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))

0 comments on commit 753ced8

Please sign in to comment.