This repository has been archived by the owner on Apr 26, 2024. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Store Promise<Response> instead of Response for HTTP API transactions #1624
Merged
Merged
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
2771447
Store Promise<Response> instead of Response for HTTP API transactions
kegsay 8a8ad46
Flake8
kegsay c7daf31
Use observable deferreds because they are sane
kegsay 42c43cf
Use ObservableDeferreds instead of Deferreds as they behave as intended
kegsay a88bc67
Flake8 and fix whoopsie
kegsay f6c4880
More flake8
kegsay 8ecaff5
Review comments
kegsay af4a1ba
Move .observe() up to the cache to make things neater
kegsay 3991b4c
Clean transactions based on time. Add HttpTransactionCache tests.
kegsay File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,85 @@ | ||
# -*- coding: utf-8 -*- | ||
# Copyright 2014-2016 OpenMarket Ltd | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# http://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
|
||
"""This module contains logic for storing HTTP PUT transactions. This is used | ||
to ensure idempotency when performing PUTs using the REST API.""" | ||
import logging | ||
|
||
from synapse.api.auth import get_access_token_from_request | ||
from synapse.util.async import ObservableDeferred | ||
|
||
logger = logging.getLogger(__name__) | ||
|
||
|
||
def get_transaction_key(request): | ||
"""A helper function which returns a transaction key that can be used | ||
with TransactionCache for idempotent requests. | ||
|
||
Idempotency is based on the returned key being the same for separate | ||
requests to the same endpoint. The key is formed from the HTTP request | ||
path and the access_token for the requesting user. | ||
|
||
Args: | ||
request (twisted.web.http.Request): The incoming request. Must | ||
contain an access_token. | ||
Returns: | ||
str: A transaction key | ||
""" | ||
token = get_access_token_from_request(request) | ||
return request.path + "/" + token | ||
|
||
|
||
class HttpTransactionCache(object): | ||
|
||
def __init__(self): | ||
self.transactions = { | ||
# $txn_key: ObservableDeferred<(res_code, res_json_body)> | ||
} | ||
|
||
def fetch_or_execute_request(self, request, fn, *args, **kwargs): | ||
"""A helper function for fetch_or_execute which extracts | ||
a transaction key from the given request. | ||
|
||
See: | ||
fetch_or_execute | ||
""" | ||
return self.fetch_or_execute( | ||
get_transaction_key(request), fn, *args, **kwargs | ||
) | ||
|
||
def fetch_or_execute(self, txn_key, fn, *args, **kwargs): | ||
"""Fetches the response for this transaction, or executes the given function | ||
to produce a response for this transaction. | ||
|
||
Args: | ||
txn_key (str): A key to ensure idempotency should fetch_or_execute be | ||
called again at a later point in time. | ||
fn (function): A function which returns a tuple of | ||
(response_code, response_dict)d | ||
*args: Arguments to pass to fn. | ||
**kwargs: Keyword arguments to pass to fn. | ||
Returns: | ||
synapse.util.async.ObservableDeferred which resolves to a tuple | ||
of (response_code, response_dict). | ||
""" | ||
try: | ||
return self.transactions[txn_key] | ||
except KeyError: | ||
pass # execute the function instead. | ||
|
||
deferred = fn(*args, **kwargs) | ||
observable = ObservableDeferred(deferred) | ||
self.transactions[txn_key] = observable | ||
return observable | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -55,17 +55,11 @@ def register(self, http_server): | |
|
||
@defer.inlineCallbacks | ||
def on_PUT(self, request, txn_id): | ||
try: | ||
defer.returnValue( | ||
self.txns.get_client_transaction(request, txn_id) | ||
) | ||
except KeyError: | ||
pass | ||
|
||
response = yield self.on_POST(request) | ||
|
||
self.txns.store_client_transaction(request, txn_id, response) | ||
defer.returnValue(response) | ||
observable = self.txns.fetch_or_execute_request( | ||
request, self.on_POST, request | ||
) | ||
res = yield observable.observe() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I'd move this def on_PUT(self, request, txn_id):
return self.txns.fetch_or_execute_request(
request, self.on_POST, request
) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
defer.returnValue(res) | ||
|
||
@defer.inlineCallbacks | ||
def on_POST(self, request): | ||
|
@@ -216,17 +210,11 @@ def on_GET(self, request, room_id, event_type, txn_id): | |
|
||
@defer.inlineCallbacks | ||
def on_PUT(self, request, room_id, event_type, txn_id): | ||
try: | ||
defer.returnValue( | ||
self.txns.get_client_transaction(request, txn_id) | ||
) | ||
except KeyError: | ||
pass | ||
|
||
response = yield self.on_POST(request, room_id, event_type, txn_id) | ||
|
||
self.txns.store_client_transaction(request, txn_id, response) | ||
defer.returnValue(response) | ||
observable = self.txns.fetch_or_execute_request( | ||
request, self.on_POST, request, room_id, event_type, txn_id | ||
) | ||
res = yield observable.observe() | ||
defer.returnValue(res) | ||
|
||
|
||
# TODO: Needs unit testing for room ID + alias joins | ||
|
@@ -285,17 +273,11 @@ def on_POST(self, request, room_identifier, txn_id=None): | |
|
||
@defer.inlineCallbacks | ||
def on_PUT(self, request, room_identifier, txn_id): | ||
try: | ||
defer.returnValue( | ||
self.txns.get_client_transaction(request, txn_id) | ||
) | ||
except KeyError: | ||
pass | ||
|
||
response = yield self.on_POST(request, room_identifier, txn_id) | ||
|
||
self.txns.store_client_transaction(request, txn_id, response) | ||
defer.returnValue(response) | ||
observable = self.txns.fetch_or_execute_request( | ||
request, self.on_POST, request, room_identifier, txn_id | ||
) | ||
res = yield observable.observe() | ||
defer.returnValue(res) | ||
|
||
|
||
# TODO: Needs unit testing | ||
|
@@ -539,19 +521,11 @@ def on_POST(self, request, room_id, txn_id=None): | |
|
||
@defer.inlineCallbacks | ||
def on_PUT(self, request, room_id, txn_id): | ||
try: | ||
defer.returnValue( | ||
self.txns.get_client_transaction(request, txn_id) | ||
) | ||
except KeyError: | ||
pass | ||
|
||
response = yield self.on_POST( | ||
request, room_id, txn_id | ||
observable = self.txns.fetch_or_execute_request( | ||
request, self.on_POST, request, room_id, txn_id | ||
) | ||
|
||
self.txns.store_client_transaction(request, txn_id, response) | ||
defer.returnValue(response) | ||
res = yield observable.observe() | ||
defer.returnValue(res) | ||
|
||
|
||
# TODO: Needs unit testing | ||
|
@@ -625,19 +599,11 @@ def _has_3pid_invite_keys(self, content): | |
|
||
@defer.inlineCallbacks | ||
def on_PUT(self, request, room_id, membership_action, txn_id): | ||
try: | ||
defer.returnValue( | ||
self.txns.get_client_transaction(request, txn_id) | ||
) | ||
except KeyError: | ||
pass | ||
|
||
response = yield self.on_POST( | ||
request, room_id, membership_action, txn_id | ||
observable = self.txns.fetch_or_execute_request( | ||
request, self.on_POST, request, room_id, membership_action, txn_id | ||
) | ||
|
||
self.txns.store_client_transaction(request, txn_id, response) | ||
defer.returnValue(response) | ||
res = yield observable.observe() | ||
defer.returnValue(res) | ||
|
||
|
||
class RoomRedactEventRestServlet(ClientV1RestServlet): | ||
|
@@ -671,17 +637,11 @@ def on_POST(self, request, room_id, event_id, txn_id=None): | |
|
||
@defer.inlineCallbacks | ||
def on_PUT(self, request, room_id, event_id, txn_id): | ||
try: | ||
defer.returnValue( | ||
self.txns.get_client_transaction(request, txn_id) | ||
) | ||
except KeyError: | ||
pass | ||
|
||
response = yield self.on_POST(request, room_id, event_id, txn_id) | ||
|
||
self.txns.store_client_transaction(request, txn_id, response) | ||
defer.returnValue(response) | ||
observable = self.txns.fetch_or_execute_request( | ||
request, self.on_POST, request, room_id, event_id, txn_id | ||
) | ||
res = yield observable.observe() | ||
defer.returnValue(res) | ||
|
||
|
||
class RoomTypingRestServlet(ClientV1RestServlet): | ||
|
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you need a
.observe()
on the endThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.