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

Feature/44 show content size #46

Open
wants to merge 2 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
3 changes: 3 additions & 0 deletions log_outgoing_requests/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ class OutgoingRequestsLogAdmin(admin.ModelAdmin):
"method",
"response_ms",
"timestamp",
"response_content_length",
)
list_filter = ("method", "timestamp", "status_code", "hostname")
search_fields = ("url", "params", "hostname")
Expand Down Expand Up @@ -57,6 +58,7 @@ class OutgoingRequestsLogAdmin(admin.ModelAdmin):
"res_headers",
"res_content_type",
"res_body_encoding",
"response_content_length",
"response_body",
)
},
Expand All @@ -76,6 +78,7 @@ class OutgoingRequestsLogAdmin(admin.ModelAdmin):
"response_ms",
"res_headers",
"res_content_type",
"response_content_length",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be nice if it's possible to sort by this field in the admin listview, that way it's easier to find requests with unexpectedly large response bodies. Since it's a computed field, you might have to annotate the queryset to implement this: https://docs.djangoproject.com/en/4.2/ref/models/querysets/#annotate

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm absolutely agree with making it sortable, but I think it is not possible to annotate the queryset because this property reads the length of another computed property response_body_decoded. I think it is therefore for this reason, I could be wrong.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm that's true, though I do wonder how useful this if we cannot sort by length.

I think it's only possible to make it sortable if we use res_body instead of response_body_decoded, although I do think the lengths won't be exactly correct depending on the content_type of the response.

Example for an HTML response

>>> len(large.res_body)
233210
>>> len(large.response_body_decoded)
233181

We could do something like this in that case:

from django.db.models import Func

class LengthFunction(Func):
    function = 'LENGTH'


def get_queryset(self, request):
        qs = super().get_queryset(request)
        return qs.annotate(
            annotated_response_length=LengthFunction('response_body_decoded')
        )

def response_content_length(self, obj):
    return obj.annotated_response_length
response_content_length.admin_order_field = 'annotated_response_length'

@alextreme what are your thought on this. Would it be okay if the response_content_length in the list view isn't exactly correct, but it would be sortable? Or would you rather have the exact length without sortability?

"response_body",
"trace",
)
Expand Down
9 changes: 9 additions & 0 deletions log_outgoing_requests/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,15 @@ def response_body_decoded(self) -> str:

response_body_decoded.short_description = _("Response body") # type: ignore

@cached_property
def response_content_length(self) -> int:
"""
Get Response content length by reading `len(response_body_decoded)`.
"""
return len(self.response_body_decoded)

response_content_length.short_description = _("Response content length") # type: ignore


def get_default_max_content_length():
"""
Expand Down
77 changes: 77 additions & 0 deletions tests/test_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,3 +131,80 @@ def test_list_url_is_not_trucated_under_200_chars(admin_client):
truncated_url = doc.find(".field-truncated_url").text()

assert truncated_url == "/a1b2c3d4e/some-path"


@pytest.mark.django_db
def test_response_content_length_empty_changelist_view(admin_client):
"""Assert the length of the content of the response is empty in changelist_view"""

log = OutgoingRequestsLog.objects.create(
id=1,
req_body=b"",
res_body=b"",
timestamp=timezone.now(),
)

url = reverse("admin:log_outgoing_requests_outgoingrequestslog_changelist")

response = admin_client.get(url)
assert response.status_code == 200

html = response.content.decode("utf-8")
doc = PyQuery(html)
content_length = doc.find(".field-response_content_length").text()

assert content_length == "0"
assert log.response_content_length == 0


@pytest.mark.django_db
def test_response_content_length_displayed_changelist_view(admin_client):
"""Assert the length of the content of the response is displayed in changelist_view"""

log = OutgoingRequestsLog.objects.create(
id=1,
req_body=b"Test request list view",
res_body=b"Test Response list view",
timestamp=timezone.now(),
)

url = reverse("admin:log_outgoing_requests_outgoingrequestslog_changelist")

response = admin_client.get(url)
assert response.status_code == 200

html = response.content.decode("utf-8")
doc = PyQuery(html)
content_length = doc.find(".field-response_content_length").text()

assert content_length == "23"
assert log.response_content_length == 23


@pytest.mark.django_db
def test_response_content_length_displayed_change_view(admin_client):
"""Assert the length of the content of the response is displayed in change_view"""

log = OutgoingRequestsLog.objects.create(
id=1,
req_body=b"Test request list view",
res_body=b"Test Response list view",
timestamp=timezone.now(),
)
url = reverse(
"admin:log_outgoing_requests_outgoingrequestslog_change", args=(log.pk,)
)

response = admin_client.get(url)
assert response.status_code == 200

html = response.content.decode("utf-8")
doc = PyQuery(html)
request_body = doc.find(".field-request_body .readonly").text()
response_body = doc.find(".field-response_body .readonly").text()
content_length = doc.find(".field-response_content_length .readonly").text()

assert request_body == "Test request list view"
assert response_body == "Test Response list view"
assert content_length == "23"
assert log.response_content_length == 23
Loading