From 129ae841e5aebb34a980dd7d118140d08b0ff81d Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Sun, 15 Nov 2020 22:47:54 +0000 Subject: [PATCH] Make `make_request` actually render the request remove the stubbing out of `request.process`, so that `requestReceived` also renders the request via the appropriate resource. Replace render() with a stub for now. --- tests/replication/test_multi_media_repo.py | 4 +- tests/rest/admin/test_admin.py | 6 -- tests/rest/admin/test_media.py | 6 -- tests/rest/client/v1/utils.py | 4 +- tests/rest/client/v2_alpha/test_account.py | 4 -- tests/rest/media/v1/test_media_storage.py | 7 +-- tests/rest/media/v1/test_url_preview.py | 66 ++++++++++------------ tests/server.py | 20 ++++--- tests/unittest.py | 8 +++ 9 files changed, 57 insertions(+), 68 deletions(-) diff --git a/tests/replication/test_multi_media_repo.py b/tests/replication/test_multi_media_repo.py index a9ac4aeec1..48b574ccbe 100644 --- a/tests/replication/test_multi_media_repo.py +++ b/tests/replication/test_multi_media_repo.py @@ -68,15 +68,15 @@ class MediaRepoShardTestCase(BaseMultiWorkerStreamTestCase): 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 diff --git a/tests/rest/admin/test_admin.py b/tests/rest/admin/test_admin.py index 9e4b0bca53..961a5732b3 100644 --- a/tests/rest/admin/test_admin.py +++ b/tests/rest/admin/test_admin.py @@ -231,8 +231,6 @@ class QuarantineMediaTestCase(unittest.HomeserverTestCase): shorthand=False, access_token=admin_user_tok, ) - request.render(self.download_resource) - self.pump(1.0) # Should be quarantined self.assertEqual( @@ -301,8 +299,6 @@ class QuarantineMediaTestCase(unittest.HomeserverTestCase): 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"]) @@ -478,8 +474,6 @@ class QuarantineMediaTestCase(unittest.HomeserverTestCase): shorthand=False, access_token=non_admin_user_tok, ) - request.render(self.download_resource) - self.pump(1.0) # Shouldn't be quarantined self.assertEqual( diff --git a/tests/rest/admin/test_media.py b/tests/rest/admin/test_media.py index 36e07f1b36..64b7aa53ee 100644 --- a/tests/rest/admin/test_media.py +++ b/tests/rest/admin/test_media.py @@ -133,8 +133,6 @@ class DeleteMediaByIDTestCase(unittest.HomeserverTestCase): shorthand=False, access_token=self.admin_user_tok, ) - request.render(download_resource) - self.pump(1.0) # Should be successful self.assertEqual( @@ -172,8 +170,6 @@ class DeleteMediaByIDTestCase(unittest.HomeserverTestCase): shorthand=False, access_token=self.admin_user_tok, ) - request.render(download_resource) - self.pump(1.0) self.assertEqual( 404, channel.code, @@ -548,8 +544,6 @@ class DeleteMediaByDateSizeTestCase(unittest.HomeserverTestCase): shorthand=False, access_token=self.admin_user_tok, ) - request.render(download_resource) - self.pump(1.0) if expect_success: self.assertEqual( diff --git a/tests/rest/client/v1/utils.py b/tests/rest/client/v1/utils.py index 900852f85b..040a92d6f0 100644 --- a/tests/rest/client/v1/utils.py +++ b/tests/rest/client/v1/utils.py @@ -310,7 +310,7 @@ class RestHelper: """ 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", @@ -319,8 +319,6 @@ class RestHelper: 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, diff --git a/tests/rest/client/v2_alpha/test_account.py b/tests/rest/client/v2_alpha/test_account.py index 94a627b0a6..b871200909 100644 --- a/tests/rest/client/v2_alpha/test_account.py +++ b/tests/rest/client/v2_alpha/test_account.py @@ -263,8 +263,6 @@ class PasswordResetTestCase(unittest.HomeserverTestCase): path, shorthand=False, ) - request.render(self.submit_token_resource) - self.pump() self.assertEquals(200, channel.code, channel.result) @@ -288,8 +286,6 @@ class PasswordResetTestCase(unittest.HomeserverTestCase): 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): diff --git a/tests/rest/media/v1/test_media_storage.py b/tests/rest/media/v1/test_media_storage.py index 0fd31a0096..2a3b2a8f27 100644 --- a/tests/rest/media/v1/test_media_storage.py +++ b/tests/rest/media/v1/test_media_storage.py @@ -234,8 +234,8 @@ class MediaRepoTests(unittest.HomeserverTestCase): "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 @@ -330,8 +330,8 @@ class MediaRepoTests(unittest.HomeserverTestCase): "GET", self.media_id + params, shorthand=False, + await_result=False, ) - request.render(self.thumbnail_resource) self.pump() headers = { @@ -359,7 +359,6 @@ class MediaRepoTests(unittest.HomeserverTestCase): channel.json_body, { "errcode": "M_NOT_FOUND", - "error": "Not found [b'example.com', b'12345?width=32&height=32&method=%s']" - % method, + "error": "Not found [b'example.com', b'12345']", }, ) diff --git a/tests/rest/media/v1/test_url_preview.py b/tests/rest/media/v1/test_url_preview.py index e00ad61231..ccdc8c2ecf 100644 --- a/tests/rest/media/v1/test_url_preview.py +++ b/tests/rest/media/v1/test_url_preview.py @@ -140,9 +140,11 @@ class URLPreviewTests(unittest.HomeserverTestCase): self.lookups["matrix.org"] = [(IPv4Address, "10.1.2.3")] request, channel = self.make_request( - "GET", "preview_url?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) @@ -165,8 +167,6 @@ class URLPreviewTests(unittest.HomeserverTestCase): request, channel = self.make_request( "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) @@ -183,8 +183,6 @@ class URLPreviewTests(unittest.HomeserverTestCase): request, channel = self.make_request( "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) @@ -204,9 +202,11 @@ class URLPreviewTests(unittest.HomeserverTestCase): ) request, channel = self.make_request( - "GET", "preview_url?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) @@ -237,9 +237,11 @@ class URLPreviewTests(unittest.HomeserverTestCase): ) request, channel = self.make_request( - "GET", "preview_url?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) @@ -270,9 +272,11 @@ class URLPreviewTests(unittest.HomeserverTestCase): ) request, channel = self.make_request( - "GET", "preview_url?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) @@ -301,9 +305,11 @@ class URLPreviewTests(unittest.HomeserverTestCase): self.lookups["example.com"] = [(IPv4Address, "10.1.2.3")] request, channel = self.make_request( - "GET", "preview_url?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) @@ -331,8 +337,6 @@ class URLPreviewTests(unittest.HomeserverTestCase): request, channel = self.make_request( "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) @@ -354,8 +358,6 @@ class URLPreviewTests(unittest.HomeserverTestCase): request, channel = self.make_request( "GET", "preview_url?url=http://example.com", shorthand=False ) - request.render(self.preview_url) - self.pump() self.assertEqual(channel.code, 502) self.assertEqual( @@ -373,8 +375,6 @@ class URLPreviewTests(unittest.HomeserverTestCase): request, channel = self.make_request( "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) @@ -394,8 +394,6 @@ class URLPreviewTests(unittest.HomeserverTestCase): request, channel = self.make_request( "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( @@ -414,9 +412,11 @@ class URLPreviewTests(unittest.HomeserverTestCase): self.lookups["example.com"] = [(IPv4Address, "1.1.1.1")] request, channel = self.make_request( - "GET", "preview_url?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) @@ -451,8 +451,6 @@ class URLPreviewTests(unittest.HomeserverTestCase): request, channel = self.make_request( "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, @@ -473,8 +471,6 @@ class URLPreviewTests(unittest.HomeserverTestCase): request, channel = self.make_request( "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) @@ -496,8 +492,6 @@ class URLPreviewTests(unittest.HomeserverTestCase): request, channel = self.make_request( "GET", "preview_url?url=http://example.com", shorthand=False ) - request.render(self.preview_url) - self.pump() self.assertEqual(channel.code, 502) self.assertEqual( @@ -515,8 +509,6 @@ class URLPreviewTests(unittest.HomeserverTestCase): request, channel = self.make_request( "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, {}) @@ -528,9 +520,11 @@ class URLPreviewTests(unittest.HomeserverTestCase): # Build and make a request to the server request, channel = self.make_request( - "GET", "preview_url?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 @@ -603,8 +597,8 @@ class URLPreviewTests(unittest.HomeserverTestCase): "GET", "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) @@ -668,8 +662,8 @@ class URLPreviewTests(unittest.HomeserverTestCase): "GET", "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) diff --git a/tests/server.py b/tests/server.py index 5a1583a3e7..de7cb1d8b3 100644 --- a/tests/server.py +++ b/tests/server.py @@ -171,16 +171,18 @@ def make_request( shorthand=True, federation_auth_origin=None, content_is_form=False, + await_result: bool = True, custom_headers: Optional[ Iterable[Tuple[Union[bytes, str], Union[bytes, str]]] ] = None, ): """ - Make a web request using the given method and path, feed it the - content, and return the Request and the Channel underneath. + Make a web request using the given method, path and content, and render it + + Returns the Request and the Channel underneath. Args: - site: The twisted Site to associate with the Channel + site: The twisted Site to use to render the request method (bytes/unicode): The HTTP request method ("verb"). path (bytes/unicode): The HTTP path, suitably URL encoded (e.g. @@ -196,6 +198,10 @@ def make_request( custom_headers: (name, value) pairs to add as request headers + await_result: whether to wait for the request to complete rendering. If true, + will pump the reactor until the the renderer tells the channel the request + is finished. + Returns: Tuple[synapse.http.site.SynapseRequest, channel] """ @@ -225,11 +231,9 @@ def make_request( channel = FakeChannel(site, reactor) req = request(channel) - req.process = lambda: b"" req.content = BytesIO(content) # Twisted expects to be at the end of the content when parsing the request. req.content.seek(SEEK_END) - req.postpath = list(map(unquote, path[1:].split(b"/"))) if access_token: req.requestHeaders.addRawHeader( @@ -257,12 +261,14 @@ def make_request( req.requestReceived(method, path, b"1.1") + if await_result: + channel.await_result() + return req, channel def render(request, resource, clock): - request.render(resource) - request._channel.await_result() + pass @implementer(IReactorPluggableNameResolver) diff --git a/tests/unittest.py b/tests/unittest.py index e39cb8dec9..9c7eca3b6e 100644 --- a/tests/unittest.py +++ b/tests/unittest.py @@ -377,6 +377,7 @@ class HomeserverTestCase(TestCase): shorthand: bool = True, federation_auth_origin: str = None, content_is_form: bool = False, + await_result: bool = True, ) -> Tuple[SynapseRequest, FakeChannel]: ... @@ -391,6 +392,7 @@ class HomeserverTestCase(TestCase): shorthand: bool = True, federation_auth_origin: str = None, content_is_form: bool = False, + await_result: bool = True, ) -> Tuple[T, FakeChannel]: ... @@ -404,6 +406,7 @@ class HomeserverTestCase(TestCase): shorthand: bool = True, federation_auth_origin: str = None, content_is_form: bool = False, + await_result: bool = True, ) -> Tuple[T, FakeChannel]: """ Create a SynapseRequest at the path using the method and containing the @@ -422,6 +425,10 @@ class HomeserverTestCase(TestCase): content_is_form: Whether the content is URL encoded form data. Adds the 'Content-Type': 'application/x-www-form-urlencoded' header. + await_result: whether to wait for the request to complete rendering. If + true (the default), will pump the test reactor until the the renderer + tells the channel the request is finished. + Returns: Tuple[synapse.http.site.SynapseRequest, channel] """ @@ -436,6 +443,7 @@ class HomeserverTestCase(TestCase): shorthand, federation_auth_origin, content_is_form, + await_result, ) def render(self, request):