Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Make make_request actually render the request #8761

Merged
merged 5 commits into from
Nov 17, 2020
Merged

Make make_request actually render the request #8761

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1f41422
Fix the URL in the URL preview tests
richvdh Nov 14, 2020
129ae84
Make `make_request` actually render the request
richvdh Nov 15, 2020
be8fa65
Remove redundant calls to `render()`
richvdh Nov 15, 2020
acfe3b3
Remove redundant `HomeserverTestCase.render`
richvdh Nov 15, 2020
0ce31ef
changelog
richvdh Nov 15, 2020
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
4 changes: 2 additions & 2 deletions tests/replication/test_multi_media_repo.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,15 +68,15 @@ def _get_media_req(
the media which the caller should respond to.
"""
resource = hs.get_media_repository_resource().children[b"download"]
request, channel = make_request(
_, channel = make_request(
self.reactor,
FakeSite(resource),
"GET",
"/{}/{}".format(target, media_id),
shorthand=False,
access_token=self.access_token,
await_result=False,
)
request.render(resource)
self.pump()

clients = self.reactor.tcpClients
Expand Down
6 changes: 0 additions & 6 deletions tests/rest/admin/test_admin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,8 +231,6 @@ def _ensure_quarantined(self, admin_user_tok, server_and_media_id):
shorthand=False,
access_token=admin_user_tok,
)
request.render(self.download_resource)
self.pump(1.0)

# Should be quarantined
self.assertEqual(
Expand Down Expand Up @@ -301,8 +299,6 @@ def test_quarantine_media_by_id(self):
shorthand=False,
access_token=non_admin_user_tok,
)
request.render(self.download_resource)
self.pump(1.0)

# Should be successful
self.assertEqual(200, int(channel.code), msg=channel.result["body"])
Expand Down Expand Up @@ -478,8 +474,6 @@ def test_cannot_quarantine_safe_media(self):
shorthand=False,
access_token=non_admin_user_tok,
)
request.render(self.download_resource)
self.pump(1.0)

# Shouldn't be quarantined
self.assertEqual(
Expand Down
6 changes: 0 additions & 6 deletions tests/rest/admin/test_media.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,6 @@ def test_delete_media(self):
shorthand=False,
access_token=self.admin_user_tok,
)
request.render(download_resource)
self.pump(1.0)

# Should be successful
self.assertEqual(
Expand Down Expand Up @@ -172,8 +170,6 @@ def test_delete_media(self):
shorthand=False,
access_token=self.admin_user_tok,
)
request.render(download_resource)
self.pump(1.0)
self.assertEqual(
404,
channel.code,
Expand Down Expand Up @@ -548,8 +544,6 @@ def _access_media(self, server_and_media_id, expect_success=True):
shorthand=False,
access_token=self.admin_user_tok,
)
request.render(download_resource)
self.pump(1.0)

if expect_success:
self.assertEqual(
Expand Down
4 changes: 1 addition & 3 deletions tests/rest/client/v1/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ def upload_media(
"""
image_length = len(image_data)
path = "/_matrix/media/r0/upload?filename=%s" % (filename,)
request, channel = make_request(
_, channel = make_request(
self.hs.get_reactor(),
FakeSite(resource),
"POST",
Expand All @@ -319,8 +319,6 @@ def upload_media(
access_token=tok,
custom_headers=[(b"Content-Length", str(image_length))],
)
request.render(resource)
self.hs.get_reactor().pump([100])

assert channel.code == expect_code, "Expected: %d, got: %d, resp: %r" % (
expect_code,
Expand Down
4 changes: 0 additions & 4 deletions tests/rest/client/v2_alpha/test_account.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,8 +263,6 @@ def _validate_token(self, link):
path,
shorthand=False,
)
request.render(self.submit_token_resource)
self.pump()

self.assertEquals(200, channel.code, channel.result)

Expand All @@ -288,8 +286,6 @@ def _validate_token(self, link):
shorthand=False,
content_is_form=True,
)
request.render(self.submit_token_resource)
self.pump()
self.assertEquals(200, channel.code, channel.result)

def _get_link_from_email(self):
Expand Down
7 changes: 3 additions & 4 deletions tests/rest/media/v1/test_media_storage.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,8 @@ def _req(self, content_disposition):
"GET",
self.media_id,
shorthand=False,
await_result=False,
)
request.render(self.download_resource)
self.pump()

# We've made one fetch, to example.com, using the media URL, and asking
Expand Down Expand Up @@ -330,8 +330,8 @@ def _test_thumbnail(self, method, expected_body, expected_found):
"GET",
self.media_id + params,
shorthand=False,
await_result=False,
)
request.render(self.thumbnail_resource)
self.pump()

headers = {
Expand Down Expand Up @@ -359,7 +359,6 @@ def _test_thumbnail(self, method, expected_body, expected_found):
channel.json_body,
{
"errcode": "M_NOT_FOUND",
"error": "Not found [b'example.com', b'12345?width=32&height=32&method=%s']"
Copy link
Member

Choose a reason for hiding this comment

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

What caused this to change? It doesn't seem like it should have...

Copy link
Member Author

Choose a reason for hiding this comment

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

this error is built from request.postpath. Previously, we were manually setting postpath in make_request here, based on the full path of the request we are simulating.

we're now leaving request.process to populate postpath, which it does here based on request.path, which is set here and excludes the query-string.

In short: it now better matches the real behaviour.

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense! Thanks!

% method,
"error": "Not found [b'example.com', b'12345']",
},
)
93 changes: 45 additions & 48 deletions tests/rest/media/v1/test_url_preview.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,13 +133,18 @@ def resolveHostName(

self.reactor.nameResolver = Resolver()

def create_test_resource(self):
return self.hs.get_media_repository_resource()

def test_cache_returns_correct_type(self):
self.lookups["matrix.org"] = [(IPv4Address, "10.1.2.3")]

request, channel = self.make_request(
"GET", "url_preview?url=http://matrix.org", shorthand=False
"GET",
"preview_url?url=http://matrix.org",
shorthand=False,
await_result=False,
)
request.render(self.preview_url)
self.pump()

client = self.reactor.tcpClients[0][2].buildProtocol(None)
Expand All @@ -160,10 +165,8 @@ def test_cache_returns_correct_type(self):

# Check the cache returns the correct response
request, channel = self.make_request(
"GET", "url_preview?url=http://matrix.org", shorthand=False
"GET", "preview_url?url=http://matrix.org", shorthand=False
)
request.render(self.preview_url)
self.pump()

# Check the cache response has the same content
self.assertEqual(channel.code, 200)
Expand All @@ -178,10 +181,8 @@ def test_cache_returns_correct_type(self):

# Check the database cache returns the correct response
request, channel = self.make_request(
"GET", "url_preview?url=http://matrix.org", shorthand=False
"GET", "preview_url?url=http://matrix.org", shorthand=False
)
request.render(self.preview_url)
self.pump()

# Check the cache response has the same content
self.assertEqual(channel.code, 200)
Expand All @@ -201,9 +202,11 @@ def test_non_ascii_preview_httpequiv(self):
)

request, channel = self.make_request(
"GET", "url_preview?url=http://matrix.org", shorthand=False
"GET",
"preview_url?url=http://matrix.org",
shorthand=False,
await_result=False,
)
request.render(self.preview_url)
self.pump()

client = self.reactor.tcpClients[0][2].buildProtocol(None)
Expand Down Expand Up @@ -234,9 +237,11 @@ def test_non_ascii_preview_content_type(self):
)

request, channel = self.make_request(
"GET", "url_preview?url=http://matrix.org", shorthand=False
"GET",
"preview_url?url=http://matrix.org",
shorthand=False,
await_result=False,
)
request.render(self.preview_url)
self.pump()

client = self.reactor.tcpClients[0][2].buildProtocol(None)
Expand Down Expand Up @@ -267,9 +272,11 @@ def test_overlong_title(self):
)

request, channel = self.make_request(
"GET", "url_preview?url=http://matrix.org", shorthand=False
"GET",
"preview_url?url=http://matrix.org",
shorthand=False,
await_result=False,
)
request.render(self.preview_url)
self.pump()

client = self.reactor.tcpClients[0][2].buildProtocol(None)
Expand Down Expand Up @@ -298,9 +305,11 @@ def test_ipaddr(self):
self.lookups["example.com"] = [(IPv4Address, "10.1.2.3")]

request, channel = self.make_request(
"GET", "url_preview?url=http://example.com", shorthand=False
"GET",
"preview_url?url=http://example.com",
shorthand=False,
await_result=False,
)
request.render(self.preview_url)
self.pump()

client = self.reactor.tcpClients[0][2].buildProtocol(None)
Expand All @@ -326,10 +335,8 @@ def test_blacklisted_ip_specific(self):
self.lookups["example.com"] = [(IPv4Address, "192.168.1.1")]

request, channel = self.make_request(
"GET", "url_preview?url=http://example.com", shorthand=False
"GET", "preview_url?url=http://example.com", shorthand=False
)
request.render(self.preview_url)
self.pump()

# No requests made.
self.assertEqual(len(self.reactor.tcpClients), 0)
Expand All @@ -349,10 +356,8 @@ def test_blacklisted_ip_range(self):
self.lookups["example.com"] = [(IPv4Address, "1.1.1.2")]

request, channel = self.make_request(
"GET", "url_preview?url=http://example.com", shorthand=False
"GET", "preview_url?url=http://example.com", shorthand=False
)
request.render(self.preview_url)
self.pump()

self.assertEqual(channel.code, 502)
self.assertEqual(
Expand All @@ -368,10 +373,8 @@ def test_blacklisted_ip_specific_direct(self):
Blacklisted IP addresses, accessed directly, are not spidered.
"""
request, channel = self.make_request(
"GET", "url_preview?url=http://192.168.1.1", shorthand=False
"GET", "preview_url?url=http://192.168.1.1", shorthand=False
)
request.render(self.preview_url)
self.pump()

# No requests made.
self.assertEqual(len(self.reactor.tcpClients), 0)
Expand All @@ -389,10 +392,8 @@ def test_blacklisted_ip_range_direct(self):
Blacklisted IP ranges, accessed directly, are not spidered.
"""
request, channel = self.make_request(
"GET", "url_preview?url=http://1.1.1.2", shorthand=False
"GET", "preview_url?url=http://1.1.1.2", shorthand=False
)
request.render(self.preview_url)
self.pump()

self.assertEqual(channel.code, 403)
self.assertEqual(
Expand All @@ -411,9 +412,11 @@ def test_blacklisted_ip_range_whitelisted_ip(self):
self.lookups["example.com"] = [(IPv4Address, "1.1.1.1")]

request, channel = self.make_request(
"GET", "url_preview?url=http://example.com", shorthand=False
"GET",
"preview_url?url=http://example.com",
shorthand=False,
await_result=False,
)
request.render(self.preview_url)
self.pump()

client = self.reactor.tcpClients[0][2].buildProtocol(None)
Expand Down Expand Up @@ -446,10 +449,8 @@ def test_blacklisted_ip_with_external_ip(self):
]

request, channel = self.make_request(
"GET", "url_preview?url=http://example.com", shorthand=False
"GET", "preview_url?url=http://example.com", shorthand=False
)
request.render(self.preview_url)
self.pump()
self.assertEqual(channel.code, 502)
self.assertEqual(
channel.json_body,
Expand All @@ -468,10 +469,8 @@ def test_blacklisted_ipv6_specific(self):
]

request, channel = self.make_request(
"GET", "url_preview?url=http://example.com", shorthand=False
"GET", "preview_url?url=http://example.com", shorthand=False
)
request.render(self.preview_url)
self.pump()

# No requests made.
self.assertEqual(len(self.reactor.tcpClients), 0)
Expand All @@ -491,10 +490,8 @@ def test_blacklisted_ipv6_range(self):
self.lookups["example.com"] = [(IPv6Address, "2001:800::1")]

request, channel = self.make_request(
"GET", "url_preview?url=http://example.com", shorthand=False
"GET", "preview_url?url=http://example.com", shorthand=False
)
request.render(self.preview_url)
self.pump()

self.assertEqual(channel.code, 502)
self.assertEqual(
Expand All @@ -510,10 +507,8 @@ def test_OPTIONS(self):
OPTIONS returns the OPTIONS.
"""
request, channel = self.make_request(
"OPTIONS", "url_preview?url=http://example.com", shorthand=False
"OPTIONS", "preview_url?url=http://example.com", shorthand=False
)
request.render(self.preview_url)
self.pump()
self.assertEqual(channel.code, 200)
self.assertEqual(channel.json_body, {})

Expand All @@ -525,9 +520,11 @@ def test_accept_language_config_option(self):

# Build and make a request to the server
request, channel = self.make_request(
"GET", "url_preview?url=http://example.com", shorthand=False
"GET",
"preview_url?url=http://example.com",
shorthand=False,
await_result=False,
)
request.render(self.preview_url)
self.pump()

# Extract Synapse's tcp client
Expand Down Expand Up @@ -598,10 +595,10 @@ def test_oembed_photo(self):

request, channel = self.make_request(
"GET",
"url_preview?url=http://twitter.com/matrixdotorg/status/12345",
"preview_url?url=http://twitter.com/matrixdotorg/status/12345",
shorthand=False,
await_result=False,
)
request.render(self.preview_url)
self.pump()

client = self.reactor.tcpClients[0][2].buildProtocol(None)
Expand Down Expand Up @@ -663,10 +660,10 @@ def test_oembed_rich(self):

request, channel = self.make_request(
"GET",
"url_preview?url=http://twitter.com/matrixdotorg/status/12345",
"preview_url?url=http://twitter.com/matrixdotorg/status/12345",
shorthand=False,
await_result=False,
)
request.render(self.preview_url)
self.pump()

client = self.reactor.tcpClients[0][2].buildProtocol(None)
Expand Down
Loading