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

Fix media repo breaking #5593

Merged
merged 8 commits into from
Jul 2, 2019
Merged
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
1 change: 1 addition & 0 deletions changelog.d/5593.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix regression in 1.1rc1 where OPTIONS requests to the media repo would fail.
26 changes: 16 additions & 10 deletions synapse/http/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ def wrap_json_request_handler(h):
The handler method must have a signature of "handle_foo(self, request)",
where "request" must be a SynapseRequest.

The handler must return a deferred. If the deferred succeeds we assume that
a response has been sent. If the deferred fails with a SynapseError we use
The handler must return a deferred or a coroutine. If the deferred succeeds
we assume that a response has been sent. If the deferred fails with a SynapseError we use
it to send a JSON response with the appropriate HTTP reponse code. If the
deferred fails with any other type of error we send a 500 reponse.
"""
Expand Down Expand Up @@ -353,16 +353,22 @@ def render(self, request):
"""
Render the request, using an asynchronous render handler if it exists.
"""
render_callback_name = "_async_render_" + request.method.decode("ascii")
async_render_callback_name = "_async_render_" + request.method.decode("ascii")

if hasattr(self, render_callback_name):
# Call the handler
callback = getattr(self, render_callback_name)
defer.ensureDeferred(callback(request))
# Try and get the async renderer
callback = getattr(self, async_render_callback_name, None)

return NOT_DONE_YET
else:
super().render(request)
# No async renderer for this request method.
if not callback:
return super().render(request)

resp = callback(request)

# If it's a coroutine, turn it into a Deferred
if isinstance(resp, types.CoroutineType):
richvdh marked this conversation as resolved.
Show resolved Hide resolved
defer.ensureDeferred(resp)

return NOT_DONE_YET
hawkowl marked this conversation as resolved.
Show resolved Hide resolved


def _options_handler(request):
Expand Down
1 change: 1 addition & 0 deletions synapse/rest/media/v1/preview_url_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ def __init__(self, hs, media_repo, media_storage):
)

def render_OPTIONS(self, request):
request.setHeader(b"Allow", b"OPTIONS, GET")
Copy link
Member

Choose a reason for hiding this comment

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

feels a bit odd to fix this here and not any of the other render_OPTIONS and on_OPTIONS tbh. Don't mind, though.

return respond_with_json(request, 200, {}, send_cors=True)

@wrap_json_request_handler
Expand Down
9 changes: 7 additions & 2 deletions synapse/util/logcontext.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

import logging
import threading
import types

from twisted.internet import defer, threads

Expand Down Expand Up @@ -528,8 +529,9 @@ def run_in_background(f, *args, **kwargs):
return from the function, and that the sentinel context is set once the
deferred returned by the function completes.

Useful for wrapping functions that return a deferred which you don't yield
on (for instance because you want to pass it to deferred.gatherResults()).
Useful for wrapping functions that return a deferred or coroutine, which you don't
yield or await on (for instance because you want to pass it to
deferred.gatherResults()).

Note that if you completely discard the result, you should make sure that
`f` doesn't raise any deferred exceptions, otherwise a scary-looking
Expand All @@ -544,6 +546,9 @@ def run_in_background(f, *args, **kwargs):
# by synchronous exceptions, so let's turn them into Failures.
return defer.fail()

if isinstance(res, types.CoroutineType):
hawkowl marked this conversation as resolved.
Show resolved Hide resolved
res = defer.ensureDeferred(res)

if not isinstance(res, defer.Deferred):
return res

Expand Down
12 changes: 12 additions & 0 deletions tests/rest/media/v1/test_url_preview.py
Original file line number Diff line number Diff line change
Expand Up @@ -460,3 +460,15 @@ def test_blacklisted_ipv6_range(self):
"error": "DNS resolution failure during URL preview generation",
},
)

def test_OPTIONS(self):
"""
OPTIONS returns the OPTIONS.
"""
request, channel = self.make_request(
"OPTIONS", "url_preview?url=http://example.com", shorthand=False
)
request.render(self.preview_url)
self.pump()
self.assertEqual(channel.code, 200)
self.assertEqual(channel.json_body, {})
33 changes: 21 additions & 12 deletions tests/util/test_logcontext.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,24 +39,17 @@ def _test_run_in_background(self, function):

callback_completed = [False]

def test():
with LoggingContext() as context_one:
context_one.request = "one"
d = function()

# fire off function, but don't wait on it.
d2 = logcontext.run_in_background(function)

def cb(res):
self._check_test_key("one")
callback_completed[0] = True
return res

d.addCallback(cb)

return d

with LoggingContext() as context_one:
context_one.request = "one"

# fire off function, but don't wait on it.
logcontext.run_in_background(test)
d2.addCallback(cb)

self._check_test_key("one")

Expand Down Expand Up @@ -105,6 +98,22 @@ def testfunc():

return self._test_run_in_background(testfunc)

def test_run_in_background_with_coroutine(self):
async def testfunc():
self._check_test_key("one")
d = Clock(reactor).sleep(0)
self.assertIs(LoggingContext.current_context(), LoggingContext.sentinel)
await d
self._check_test_key("one")

return self._test_run_in_background(testfunc)

def test_run_in_background_with_nonblocking_coroutine(self):
async def testfunc():
self._check_test_key("one")

return self._test_run_in_background(testfunc)

@defer.inlineCallbacks
def test_make_deferred_yieldable(self):
# a function which retuns an incomplete deferred, but doesn't follow
Expand Down