Cleaner way of implementing trailing slashes

This commit is contained in:
Andrew Morgan 2019-03-12 14:11:11 +00:00
parent 4868b12029
commit 0ea8582f8b
3 changed files with 66 additions and 70 deletions

View File

@ -52,8 +52,9 @@ class TransportLayerClient(object):
destination, room_id) destination, room_id)
path = _create_v1_path("/state/%s", room_id) path = _create_v1_path("/state/%s", room_id)
return self.client.get_json_with_trailing_slashes_on_404( return self.client.get_json(
destination, path=path, args={"event_id": event_id}, destination, path=path, args={"event_id": event_id},
try_trailing_slash_on_404=True,
) )
@log_function @log_function
@ -74,8 +75,9 @@ class TransportLayerClient(object):
destination, room_id) destination, room_id)
path = _create_v1_path("/state_ids/%s", room_id) path = _create_v1_path("/state_ids/%s", room_id)
return self.client.get_json_with_trailing_slashes_on_404( return self.client.get_json(
destination, path=path, args={"event_id": event_id}, destination, path=path, args={"event_id": event_id},
try_trailing_slash_on_404=True,
) )
@log_function @log_function
@ -96,8 +98,9 @@ class TransportLayerClient(object):
destination, event_id) destination, event_id)
path = _create_v1_path("/event/%s", event_id) path = _create_v1_path("/event/%s", event_id)
return self.client.get_json_with_trailing_slashes_on_404( return self.client.get_json(
destination, path=path, timeout=timeout, destination, path=path, timeout=timeout,
try_trailing_slash_on_404=True,
) )
@log_function @log_function
@ -130,10 +133,11 @@ class TransportLayerClient(object):
"limit": [str(limit)], "limit": [str(limit)],
} }
return self.client.get_json_with_trailing_slashes_on_404( return self.client.get_json(
destination, destination,
path=path, path=path,
args=args, args=args,
try_trailing_slash_on_404=True,
) )
@defer.inlineCallbacks @defer.inlineCallbacks
@ -171,13 +175,14 @@ class TransportLayerClient(object):
path = _create_v1_path("/send/%s", transaction.transaction_id) path = _create_v1_path("/send/%s", transaction.transaction_id)
response = yield self.client.put_json_with_trailing_slashes_on_404( response = yield self.client.put_json(
transaction.destination, transaction.destination,
path=path, path=path,
data=json_data, data=json_data,
json_data_callback=json_data_callback, json_data_callback=json_data_callback,
long_retries=True, long_retries=True,
backoff_on_404=True, # If we get a 404 the other side has gone backoff_on_404=True, # If we get a 404 the other side has gone
try_trailing_slash_on_404=True,
) )
defer.returnValue(response) defer.returnValue(response)

View File

@ -196,7 +196,8 @@ class MatrixFederationHttpClient(object):
timeout=None, timeout=None,
long_retries=False, long_retries=False,
ignore_backoff=False, ignore_backoff=False,
backoff_on_404=False backoff_on_404=False,
try_trailing_slash_on_404=False,
): ):
""" """
Sends a request to the given server. Sends a request to the given server.
@ -212,6 +213,11 @@ class MatrixFederationHttpClient(object):
backoff_on_404 (bool): Back off if we get a 404 backoff_on_404 (bool): Back off if we get a 404
try_trailing_slash_on_404 (bool): True if on a 404 response we
should try appending a trailing slash to the end of the
request. This will be attempted before backing off if backing
off has been enabled.
Returns: Returns:
Deferred[twisted.web.client.Response]: resolves with the HTTP Deferred[twisted.web.client.Response]: resolves with the HTTP
response object on success. response object on success.
@ -473,7 +479,8 @@ class MatrixFederationHttpClient(object):
json_data_callback=None, json_data_callback=None,
long_retries=False, timeout=None, long_retries=False, timeout=None,
ignore_backoff=False, ignore_backoff=False,
backoff_on_404=False): backoff_on_404=False,
try_trailing_slash_on_404=False):
""" Sends the specifed json data using PUT """ Sends the specifed json data using PUT
Args: Args:
@ -493,7 +500,11 @@ class MatrixFederationHttpClient(object):
and try the request anyway. and try the request anyway.
backoff_on_404 (bool): True if we should count a 404 response as backoff_on_404 (bool): True if we should count a 404 response as
a failure of the server (and should therefore back off future a failure of the server (and should therefore back off future
requests) requests).
try_trailing_slash_on_404 (bool): True if on a 404 response we
should try appending a trailing slash to the end of the
request. This will be attempted before backing off if backing
off has been enabled.
Returns: Returns:
Deferred[dict|list]: Succeeds when we get a 2xx HTTP response. The Deferred[dict|list]: Succeeds when we get a 2xx HTTP response. The
@ -509,7 +520,6 @@ class MatrixFederationHttpClient(object):
RequestSendFailed: If there were problems connecting to the RequestSendFailed: If there were problems connecting to the
remote, due to e.g. DNS failures, connection timeouts etc. remote, due to e.g. DNS failures, connection timeouts etc.
""" """
request = MatrixFederationRequest( request = MatrixFederationRequest(
method="PUT", method="PUT",
destination=destination, destination=destination,
@ -519,13 +529,26 @@ class MatrixFederationHttpClient(object):
json=data, json=data,
) )
response = yield self._send_request( send_request_args = {
request, "request": request,
long_retries=long_retries, "long_retries": long_retries,
timeout=timeout, "timeout": timeout,
ignore_backoff=ignore_backoff, "ignore_backoff": ignore_backoff,
backoff_on_404=backoff_on_404, # Do not backoff on the initial request if we're trying with trailing slashes
) # Otherwise we may end up waiting to contact a server that is actually up
"backoff_on_404": False if try_trailing_slash_on_404 else backoff_on_404,
}
response = yield self._send_request(**send_request_args)
# If enabled, retry with a trailing slash if we received a 404
if try_trailing_slash_on_404 and response.code == 404:
args["path"] += "/"
# Re-enable backoff if enabled
send_request_args["backoff_on_404"] = backoff_on_404
response = yield self.get_json(**send_request_args)
body = yield _handle_json_response( body = yield _handle_json_response(
self.hs.get_reactor(), self.default_timeout, request, response, self.hs.get_reactor(), self.default_timeout, request, response,
@ -592,7 +615,8 @@ class MatrixFederationHttpClient(object):
@defer.inlineCallbacks @defer.inlineCallbacks
def get_json(self, destination, path, args=None, retry_on_dns_fail=True, def get_json(self, destination, path, args=None, retry_on_dns_fail=True,
timeout=None, ignore_backoff=False): timeout=None, ignore_backoff=False,
try_trailing_slash_on_404=False):
""" GETs some json from the given host homeserver and path """ GETs some json from the given host homeserver and path
Args: Args:
@ -606,6 +630,9 @@ class MatrixFederationHttpClient(object):
be retried. be retried.
ignore_backoff (bool): true to ignore the historical backoff data ignore_backoff (bool): true to ignore the historical backoff data
and try the request anyway. and try the request anyway.
try_trailing_slash_on_404 (bool): True if on a 404 response we
should try appending a trailing slash to the end of the
request.
Returns: Returns:
Deferred[dict|list]: Succeeds when we get a 2xx HTTP response. The Deferred[dict|list]: Succeeds when we get a 2xx HTTP response. The
result will be the decoded JSON body. result will be the decoded JSON body.
@ -631,63 +658,25 @@ class MatrixFederationHttpClient(object):
query=args, query=args,
) )
response = yield self._send_request( send_request_args = {
request, "request": request,
retry_on_dns_fail=retry_on_dns_fail, "retry_on_dns_fail": retry_on_dns_fail,
timeout=timeout, "timeout": timeout,
ignore_backoff=ignore_backoff, "ignore_backoff": ignore_backoff,
) }
response = yield self._send_request(**send_request_args)
# If enabled, retry with a trailing slash if we received a 404
if try_trailing_slash_on_404 and response.code == 404:
args["path"] += "/"
response = yield self._send_request(**send_request_args)
body = yield _handle_json_response( body = yield _handle_json_response(
self.hs.get_reactor(), self.default_timeout, request, response, self.hs.get_reactor(), self.default_timeout, request, response,
) )
defer.returnValue(body) defer.returnValue(body)
@defer.inlineCallbacks
def get_json_with_trailing_slashes_on_404(self, args={}):
"""Runs client.get_json under the hood, but if receiving a 404, tries
the request again with a trailing slash. This is a result of removing
trailing slashes from some federation endpoints and in an effort to
remain backwards compatible with older versions of Synapse, we try
again if a server requires a trailing slash.
Args:
args (dict): A dictionary of arguments matching those provided by put_json.
Returns:
Deferred[dict|list]: Succeeds when we get a 2xx HTTP response. The
result will be the decoded JSON body.
"""
response = yield self.get_json(**args)
# Retry with a trailing slash if we received a 404
if response.code == 404:
args["path"] += "/"
response = yield self.get_json(**args)
defer.returnValue(response)
@defer.inlineCallbacks
def put_json_with_trailing_slashes_on_404(self, args={}):
"""Runs client.put_json under the hood, but if receiving a 404, tries
the request again with a trailing slash.
See get_json_with_trailing_slashes_on_404 for more details.
Args:
args (dict): A dictionary of arguments matching those provided by put_json.
Returns:
Deferred[dict|list]: Succeeds when we get a 2xx HTTP response. The
result will be the decoded JSON body.
"""
response = yield self.put_json(**args)
# Retry with a trailing slash if we received a 404
if response.code == 404:
args["path"] += "/"
response = yield self.put_json(**args)
defer.returnValue(response)
@defer.inlineCallbacks @defer.inlineCallbacks
def delete_json(self, destination, path, long_retries=False, def delete_json(self, destination, path, long_retries=False,
timeout=None, ignore_backoff=False, args={}): timeout=None, ignore_backoff=False, args={}):

View File

@ -177,7 +177,7 @@ class TypingNotificationsTestCase(unittest.HomeserverTestCase):
timeout=20000, timeout=20000,
)) ))
put_json = self.hs.get_http_client().put_json_with_trailing_slashes_on_404 put_json = self.hs.get_http_client().put_json
put_json.assert_called_once_with( put_json.assert_called_once_with(
"farm", "farm",
path="/_matrix/federation/v1/send/1000000", path="/_matrix/federation/v1/send/1000000",
@ -192,6 +192,7 @@ class TypingNotificationsTestCase(unittest.HomeserverTestCase):
json_data_callback=ANY, json_data_callback=ANY,
long_retries=True, long_retries=True,
backoff_on_404=True, backoff_on_404=True,
trailing_slashes_on_404=True,
) )
def test_started_typing_remote_recv(self): def test_started_typing_remote_recv(self):
@ -254,7 +255,7 @@ class TypingNotificationsTestCase(unittest.HomeserverTestCase):
[call('typing_key', 1, rooms=[ROOM_ID])] [call('typing_key', 1, rooms=[ROOM_ID])]
) )
put_json = self.hs.get_http_client().put_json_with_trailing_slashes_on_404 put_json = self.hs.get_http_client().put_json
put_json.assert_called_once_with( put_json.assert_called_once_with(
"farm", "farm",
path="/_matrix/federation/v1/send/1000000", path="/_matrix/federation/v1/send/1000000",
@ -269,6 +270,7 @@ class TypingNotificationsTestCase(unittest.HomeserverTestCase):
json_data_callback=ANY, json_data_callback=ANY,
long_retries=True, long_retries=True,
backoff_on_404=True, backoff_on_404=True,
trailing_slashes_on_404=True,
) )
self.assertEquals(self.event_source.get_current_key(), 1) self.assertEquals(self.event_source.get_current_key(), 1)