Skip to content

Commit

Permalink
Merge pull request #1 from zulip/main
Browse files Browse the repository at this point in the history
 fixes the default view issue
  • Loading branch information
amaan784 authored Jun 16, 2023
2 parents cbde01e + 92c83c1 commit fd21fa9
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 39 deletions.
6 changes: 0 additions & 6 deletions zerver/lib/test_classes.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
from django.db.migrations.state import StateApps
from django.db.utils import IntegrityError
from django.http import HttpRequest, HttpResponse
from django.http.response import StreamingHttpResponse
from django.test import TestCase
from django.test.client import BOUNDARY, MULTIPART_CONTENT, encode_multipart
from django.test.testcases import SerializeMixin
Expand Down Expand Up @@ -1111,11 +1110,6 @@ def users_subscribed_to_stream(self, stream_name: str, realm: Realm) -> List[Use

return [subscription.user_profile for subscription in subscriptions]

def assert_streaming_content(self, response: "TestHttpResponse", result: bytes) -> None:
assert isinstance(response, StreamingHttpResponse)
data = b"".join(response.streaming_content)
self.assertEqual(result, data)

def assert_json_success(
self,
result: Union["TestHttpResponse", HttpResponse],
Expand Down
10 changes: 5 additions & 5 deletions zerver/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -3475,11 +3475,11 @@ def select_for_update_query() -> QuerySet["UserMessage"]:
the row locks acquired by a bulk update operation to modify
message flags using bitand/bitor.
This consistent ordering is important to prevent to prevent
deadlocks when 2 or more bulk updates to the same rows in the
UserMessage table race against each other (For example, if a
client submits simultaneous duplicate API requests to mark a
certain set of messages as read).
This consistent ordering is important to prevent deadlocks when
2 or more bulk updates to the same rows in the UserMessage table
race against each other (For example, if a client submits
simultaneous duplicate API requests to mark a certain set of
messages as read).
"""
return UserMessage.objects.select_for_update().order_by("message_id")

Expand Down
4 changes: 2 additions & 2 deletions zerver/tests/test_auth_backends.py
Original file line number Diff line number Diff line change
Expand Up @@ -6307,8 +6307,8 @@ def test_login_success_when_user_does_not_exist_with_valid_subdomain(self) -> No
"rb",
) as f:
example_avatar = f.read()
self.assert_streaming_content(
response, resize_avatar(example_avatar, DEFAULT_AVATAR_SIZE)
self.assertEqual(
response.getvalue(), resize_avatar(example_avatar, DEFAULT_AVATAR_SIZE)
)

@override_settings(AUTHENTICATION_BACKENDS=("zproject.backends.ZulipLDAPAuthBackend",))
Expand Down
2 changes: 1 addition & 1 deletion zerver/tests/test_realm_export.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ def fake_export_realm(
export_path = orjson.loads(extra_data).get("export_path")
response = self.client_get(export_path)
self.assertEqual(response.status_code, 200)
self.assert_streaming_content(response, b"zulip!")
self.assertEqual(response.getvalue(), b"zulip!")

result = self.client_get("/json/export/realm")
response_dict = self.assert_json_success(result)
Expand Down
40 changes: 18 additions & 22 deletions zerver/tests/test_upload.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

import orjson
from django.conf import settings
from django.http.response import StreamingHttpResponse
from django.utils.timezone import now as timezone_now
from PIL import Image
from urllib3 import encode_multipart_formdata
Expand Down Expand Up @@ -81,11 +80,11 @@ def test_rest_endpoint(self) -> None:
self.logout()
response = self.api_get(self.example_user("hamlet"), url)
self.assertEqual(response.status_code, 200)
self.assert_streaming_content(response, b"zulip!")
self.assertEqual(response.getvalue(), b"zulip!")

# Files uploaded through the API should be accessible via the web client
self.login("hamlet")
self.assert_streaming_content(self.client_get(url), b"zulip!")
self.assertEqual(self.client_get(url).getvalue(), b"zulip!")

def test_mobile_api_endpoint(self) -> None:
"""
Expand Down Expand Up @@ -114,7 +113,7 @@ def test_mobile_api_endpoint(self) -> None:

response = self.client_get(url, {"api_key": get_api_key(user_profile)})
self.assertEqual(response.status_code, 200)
self.assert_streaming_content(response, b"zulip!")
self.assertEqual(response.getvalue(), b"zulip!")

def test_file_too_big_failure(self) -> None:
"""
Expand Down Expand Up @@ -194,12 +193,12 @@ def test_file_upload_authed(self) -> None:

# In the future, local file requests will follow the same style as S3
# requests; they will be first authenticated and redirected
self.assert_streaming_content(self.client_get(url), b"zulip!")
self.assertEqual(self.client_get(url).getvalue(), b"zulip!")

# Check the download endpoint
download_url = url.replace("/user_uploads/", "/user_uploads/download/")
result = self.client_get(download_url)
self.assert_streaming_content(result, b"zulip!")
self.assertEqual(result.getvalue(), b"zulip!")
self.assertIn("attachment;", result.headers["Content-Disposition"])

# check if DB has attachment marked as unclaimed
Expand All @@ -223,7 +222,7 @@ def test_file_upload_authed(self) -> None:
# The generated URL has a token authorizing the requestor to access the file
# without being logged in.
self.logout()
self.assert_streaming_content(self.client_get(url_only_url), b"zulip!")
self.assertEqual(self.client_get(url_only_url).getvalue(), b"zulip!")
# The original url shouldn't work when logged out:
result = self.client_get(url)
self.assertEqual(result.status_code, 403)
Expand Down Expand Up @@ -314,7 +313,7 @@ def test_serve_local_file_unauthed_token_expires(self) -> None:
url_only_url = data["url"]

self.logout()
self.assert_streaming_content(self.client_get(url_only_url), b"zulip!")
self.assertEqual(self.client_get(url_only_url).getvalue(), b"zulip!")

# After over 60 seconds, the token should become invalid:
with mock.patch("django.core.signing.time.time", return_value=start_time + 61):
Expand Down Expand Up @@ -741,7 +740,7 @@ def create_user(email: str, realm_id: str) -> UserProfile:
self.login_user(user_1)
response = self.client_get(url, subdomain=test_subdomain)
self.assertEqual(response.status_code, 200)
self.assert_streaming_content(response, b"zulip!")
self.assertEqual(response.getvalue(), b"zulip!")
self.logout()

# Confirm other cross-realm users can't read it.
Expand Down Expand Up @@ -784,15 +783,15 @@ def test_file_download_authorization_invite_only(self) -> None:
with self.assert_database_query_count(5):
response = self.client_get(url)
self.assertEqual(response.status_code, 200)
self.assert_streaming_content(response, b"zulip!")
self.assertEqual(response.getvalue(), b"zulip!")
self.logout()

# Subscribed user who received the message should be able to view file
self.login_user(cordelia)
with self.assert_database_query_count(6):
response = self.client_get(url)
self.assertEqual(response.status_code, 200)
self.assert_streaming_content(response, b"zulip!")
self.assertEqual(response.getvalue(), b"zulip!")
self.logout()

def assert_cannot_access_file(user: UserProfile) -> None:
Expand Down Expand Up @@ -845,23 +844,23 @@ def test_file_download_authorization_invite_only_with_shared_history(self) -> No
with self.assert_database_query_count(5):
response = self.client_get(url)
self.assertEqual(response.status_code, 200)
self.assert_streaming_content(response, b"zulip!")
self.assertEqual(response.getvalue(), b"zulip!")
self.logout()

# Originally subscribed user should be able to view file
self.login_user(polonius)
with self.assert_database_query_count(6):
response = self.client_get(url)
self.assertEqual(response.status_code, 200)
self.assert_streaming_content(response, b"zulip!")
self.assertEqual(response.getvalue(), b"zulip!")
self.logout()

# Subscribed user who did not receive the message should also be able to view file
self.login_user(late_subscribed_user)
with self.assert_database_query_count(9):
response = self.client_get(url)
self.assertEqual(response.status_code, 200)
self.assert_streaming_content(response, b"zulip!")
self.assertEqual(response.getvalue(), b"zulip!")
self.logout()
# It takes a few extra queries to verify access because of shared history.

Expand Down Expand Up @@ -921,7 +920,7 @@ def test_multiple_message_attachment_file_download(self) -> None:
with self.assert_database_query_count(9):
response = self.client_get(url)
self.assertEqual(response.status_code, 200)
self.assert_streaming_content(response, b"zulip!")
self.assertEqual(response.getvalue(), b"zulip!")

with self.assert_database_query_count(6):
self.assertTrue(validate_attachment_request(user, fp_path_id))
Expand Down Expand Up @@ -949,7 +948,7 @@ def test_file_download_authorization_public(self) -> None:
for user in subscribed_users + unsubscribed_users:
self.login_user(user)
response = self.client_get(url)
self.assert_streaming_content(response, b"zulip!")
self.assertEqual(response.getvalue(), b"zulip!")
self.logout()

def test_serve_local(self) -> None:
Expand Down Expand Up @@ -1309,8 +1308,7 @@ def test_valid_avatars(self) -> None:

if rfname is not None:
response = self.client_get(url)
assert isinstance(response, StreamingHttpResponse)
data = b"".join(response.streaming_content)
data = response.getvalue()
self.assertEqual(Image.open(io.BytesIO(data)).size, (100, 100))

# Verify that the medium-size avatar was created
Expand Down Expand Up @@ -1590,8 +1588,7 @@ def test_valid_icons(self) -> None:

if rfname is not None:
response = self.client_get(url)
assert isinstance(response, StreamingHttpResponse)
data = b"".join(response.streaming_content)
data = response.getvalue()
self.assertEqual(Image.open(io.BytesIO(data)).size, (100, 100))

def test_invalid_icons(self) -> None:
Expand Down Expand Up @@ -1773,8 +1770,7 @@ def test_valid_logos(self) -> None:

if rfname is not None:
response = self.client_get(logo_url)
assert isinstance(response, StreamingHttpResponse)
data = b"".join(response.streaming_content)
data = response.getvalue()
# size should be 100 x 100 because thumbnail keeps aspect ratio
# while trying to fit in a 800 x 100 box without losing part of the image
self.assertEqual(Image.open(io.BytesIO(data)).size, (100, 100))
Expand Down
4 changes: 1 addition & 3 deletions zerver/tests/test_upload_local.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
from urllib.parse import urlparse

from django.conf import settings
from django.http.response import StreamingHttpResponse
from PIL import Image

import zerver.lib.upload
Expand Down Expand Up @@ -150,8 +149,7 @@ def test_avatar_url(self) -> None:
# We get a resized avatar from it
image_data = read_test_image_file("img.png")
resized_avatar = resize_avatar(image_data)
assert isinstance(result, StreamingHttpResponse)
self.assertEqual(resized_avatar, b"".join(result.streaming_content))
self.assertEqual(resized_avatar, result.getvalue())

with self.settings(DEVELOPMENT=False):
# In production, this is an X-Accel-Redirect to the
Expand Down

0 comments on commit fd21fa9

Please sign in to comment.