From a18b1a649c5dd7fbb42fa2b1af0530542b8c3873 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Mon, 18 Aug 2014 15:01:08 +0100 Subject: Added /matrix/content path, HS resource_for_content_repo attribute and FileUploadResource. Added stub methods. --- synapse/api/urls.py | 3 ++- synapse/app/homeserver.py | 12 +++++++++--- synapse/http/server.py | 28 ++++++++++++++++++++++++++++ synapse/server.py | 2 ++ 4 files changed, 41 insertions(+), 4 deletions(-) (limited to 'synapse') diff --git a/synapse/api/urls.py b/synapse/api/urls.py index 04970adb71..05ca000787 100644 --- a/synapse/api/urls.py +++ b/synapse/api/urls.py @@ -17,4 +17,5 @@ CLIENT_PREFIX = "/matrix/client/api/v1" FEDERATION_PREFIX = "/matrix/federation/v1" -WEB_CLIENT_PREFIX = "/matrix/client" \ No newline at end of file +WEB_CLIENT_PREFIX = "/matrix/client" +CONTENT_REPO_PREFIX = "/matrix/content" \ No newline at end of file diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index 3429a29a6b..e5bd13a6eb 100755 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -24,9 +24,11 @@ from twisted.python.log import PythonLoggingObserver from twisted.web.resource import Resource from twisted.web.static import File from twisted.web.server import Site -from synapse.http.server import JsonResource, RootRedirect +from synapse.http.server import JsonResource, RootRedirect, FileUploadResource from synapse.http.client import TwistedHttpClient -from synapse.api.urls import CLIENT_PREFIX, FEDERATION_PREFIX, WEB_CLIENT_PREFIX +from synapse.api.urls import ( + CLIENT_PREFIX, FEDERATION_PREFIX, WEB_CLIENT_PREFIX, CONTENT_REPO_PREFIX +) from daemonize import Daemonize @@ -53,6 +55,9 @@ class SynapseHomeServer(HomeServer): def build_resource_for_web_client(self): return File("webclient") # TODO configurable? + def build_resource_for_content_repo(self): + return FileUploadResource("uploads") + def build_db_pool(self): """ Set up all the dbs. Since all the *.sql have IF NOT EXISTS, so we don't have to worry about overwriting existing content. @@ -101,7 +106,8 @@ class SynapseHomeServer(HomeServer): # [ ("/aaa/bbb/cc", Resource1), ("/aaa/dummy", Resource2) ] desired_tree = [ (CLIENT_PREFIX, self.get_resource_for_client()), - (FEDERATION_PREFIX, self.get_resource_for_federation()) + (FEDERATION_PREFIX, self.get_resource_for_federation()), + (CONTENT_REPO_PREFIX, self.get_resource_for_content_repo()) ] if web_client: logger.info("Adding the web client.") diff --git a/synapse/http/server.py b/synapse/http/server.py index bad2738bde..f86151e51c 100644 --- a/synapse/http/server.py +++ b/synapse/http/server.py @@ -25,6 +25,7 @@ from twisted.web.server import NOT_DONE_YET from twisted.web.util import redirectTo import collections +import json import logging @@ -176,6 +177,33 @@ class RootRedirect(resource.Resource): return resource.Resource.getChild(self, name, request) +class FileUploadResource(resource.Resource): + isLeaf = True + + def __init__(self, directory): + resource.Resource.__init__(self) + self.directory = directory + + def render(self, request): + self._async_render(request) + return server.NOT_DONE_YET + + # @defer.inlineCallbacks + def _async_render(self, request): + request.setResponseCode(200) + request.setHeader(b"Content-Type", b"application/json") + + request.setHeader("Access-Control-Allow-Origin", "*") + request.setHeader("Access-Control-Allow-Methods", + "GET, POST, PUT, DELETE, OPTIONS") + request.setHeader("Access-Control-Allow-Headers", + "Origin, X-Requested-With, Content-Type, Accept") + + request.write(json.dumps({"url": "not_implemented"})) + request.finish() + defer.succeed("not implemented") + + def respond_with_json_bytes(request, code, json_bytes, send_cors=False): """Sends encoded JSON in response to the given request. diff --git a/synapse/server.py b/synapse/server.py index 0f7ac352ae..d4c2481483 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -72,6 +72,7 @@ class BaseHomeServer(object): 'resource_for_client', 'resource_for_federation', 'resource_for_web_client', + 'resource_for_content_repo', ] def __init__(self, hostname, **kwargs): @@ -140,6 +141,7 @@ class HomeServer(BaseHomeServer): resource_for_client resource_for_web_client resource_for_federation + resource_for_content_repo http_client db_pool """ -- cgit 1.5.1 From 35da1bf4a3ee6eeafec5965af05cefbb4bd3c0b5 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Mon, 18 Aug 2014 15:50:55 +0100 Subject: Auth content uploads. Added a mapping function from request > filename. Added exception handling for content uploads. webclient: Only prefix the client API path on doRequest, not doBaseRequest (this would've broken the identity server auth too). Added matrixService.uploadContent. May not require mFileUpload anymore. --- synapse/app/homeserver.py | 2 +- synapse/http/server.py | 44 ++++++++++++++++------ .../components/fileUpload/file-upload-service.js | 29 +++++++------- webclient/components/matrix/matrix-service.js | 18 +++++++-- 4 files changed, 62 insertions(+), 31 deletions(-) (limited to 'synapse') diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index e5bd13a6eb..f7c1da9201 100755 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -56,7 +56,7 @@ class SynapseHomeServer(HomeServer): return File("webclient") # TODO configurable? def build_resource_for_content_repo(self): - return FileUploadResource("uploads") + return FileUploadResource("uploads", self.auth) def build_db_pool(self): """ Set up all the dbs. Since all the *.sql have IF NOT EXISTS, so we diff --git a/synapse/http/server.py b/synapse/http/server.py index f86151e51c..7d6e225e74 100644 --- a/synapse/http/server.py +++ b/synapse/http/server.py @@ -180,28 +180,48 @@ class RootRedirect(resource.Resource): class FileUploadResource(resource.Resource): isLeaf = True - def __init__(self, directory): + def __init__(self, directory, auth, file_map_func=None): resource.Resource.__init__(self) self.directory = directory + self.auth = auth + if not file_map_func: + file_map_func = self.map_request_to_name + self.get_name_for_request = file_map_func + + @defer.inlineCallbacks + def map_request_to_name(self, request): + # auth the user + auth_user = yield self.auth.get_user_by_req(request) + logger.info("User %s is uploading a file.", auth_user) + defer.returnValue("boo2.png") def render(self, request): self._async_render(request) return server.NOT_DONE_YET - # @defer.inlineCallbacks + @defer.inlineCallbacks def _async_render(self, request): - request.setResponseCode(200) - request.setHeader(b"Content-Type", b"application/json") + try: + fname = yield self.get_name_for_request(request) - request.setHeader("Access-Control-Allow-Origin", "*") - request.setHeader("Access-Control-Allow-Methods", - "GET, POST, PUT, DELETE, OPTIONS") - request.setHeader("Access-Control-Allow-Headers", - "Origin, X-Requested-With, Content-Type, Accept") + with open(fname, "wb") as f: + f.write(request.content.read()) + + respond_with_json_bytes(request, 200, + json.dumps({"url": "not_implemented2"}), + send_cors=True) - request.write(json.dumps({"url": "not_implemented"})) - request.finish() - defer.succeed("not implemented") + except CodeMessageException as e: + logger.exception(e) + respond_with_json_bytes(request, e.code, + json.dumps(cs_exception(e))) + except Exception as e: + logger.error("Failed to store file: %s" % e) + respond_with_json_bytes( + request, + 500, + json.dumps({"error": "Internal server error"}), + send_cors=True) def respond_with_json_bytes(request, code, json_bytes, send_cors=False): diff --git a/webclient/components/fileUpload/file-upload-service.js b/webclient/components/fileUpload/file-upload-service.js index 5729d5da48..0826666fe4 100644 --- a/webclient/components/fileUpload/file-upload-service.js +++ b/webclient/components/fileUpload/file-upload-service.js @@ -16,11 +16,12 @@ 'use strict'; +// TODO determine if this is really required as a separate service to matrixService. /* * Upload an HTML5 file to a server */ angular.module('mFileUpload', []) -.service('mFileUpload', ['$http', '$q', function ($http, $q) { +.service('mFileUpload', ['matrixService', '$q', function (matrixService, $q) { /* * Upload an HTML5 file to a server and returned a promise @@ -28,20 +29,18 @@ angular.module('mFileUpload', []) */ this.uploadFile = function(file) { var deferred = $q.defer(); - - // @TODO: This service runs with the do_POST hacky implementation of /synapse/demos/webserver.py. - // This is temporary until we have a true file upload service - console.log("Uploading " + file.name + "..."); - $http.post(file.name, file) - .success(function(data, status, headers, config) { - deferred.resolve(location.origin + data.url); - console.log(" -> Successfully uploaded! Available at " + location.origin + data.url); - }). - error(function(data, status, headers, config) { - console.log(" -> Failed to upload" + file.name); - deferred.reject(); - }); + console.log("Uploading " + file.name + "... to /matrix/content"); + matrixService.uploadContent(file).then( + function(response) { + console.log(" -> Successfully uploaded! Available at " + location.origin + response.data.url); + deferred.resolve(location.origin + response.data.url); + }, + function(error) { + console.log(" -> Failed to upload " + file.name); + deferred.reject(error); + } + ); return deferred.promise; }; -}]); \ No newline at end of file +}]); diff --git a/webclient/components/matrix/matrix-service.js b/webclient/components/matrix/matrix-service.js index 47828993a1..b67beb007a 100644 --- a/webclient/components/matrix/matrix-service.js +++ b/webclient/components/matrix/matrix-service.js @@ -54,13 +54,14 @@ angular.module('matrixService', []) params.access_token = config.access_token; + if (path.indexOf(prefixPath) !== 0) { + path = prefixPath + path; + } + return doBaseRequest(config.homeserver, method, path, params, data, undefined); }; var doBaseRequest = function(baseUrl, method, path, params, data, headers) { - if (path.indexOf(prefixPath) !== 0) { - path = prefixPath + path; - } return $http({ method: method, url: baseUrl + path, @@ -319,6 +320,17 @@ angular.module('matrixService', []) return doBaseRequest(config.identityServer, "POST", path, {}, data, headers); }, + uploadContent: function(file) { + var path = "/matrix/content"; + var headers = { + "Content-Type": undefined // undefined means angular will figure it out + }; + var params = { + access_token: config.access_token + }; + return doBaseRequest(config.homeserver, "POST", path, params, file, headers); + }, + // start listening on /events getEventStream: function(from, timeout) { var path = "/events"; -- cgit 1.5.1 From 590ab24c8544553ae5c21b729e35dcda998f1cec Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Mon, 18 Aug 2014 16:18:11 +0100 Subject: hs: Make the uploads directory if it doesn't exist. Namespace uploads by the base64 encoded user id of the uploader. Make a reasonable attempt to retry clashing upload paths. Try to guess a sensible file extension depending on the content type. --- synapse/http/server.py | 51 +++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 46 insertions(+), 5 deletions(-) (limited to 'synapse') diff --git a/synapse/http/server.py b/synapse/http/server.py index 7d6e225e74..8502416fca 100644 --- a/synapse/http/server.py +++ b/synapse/http/server.py @@ -17,16 +17,19 @@ from syutil.jsonutil import ( encode_canonical_json, encode_pretty_printed_json ) -from synapse.api.errors import cs_exception, CodeMessageException +from synapse.api.errors import cs_exception, SynapseError, CodeMessageException +from synapse.util.stringutils import random_string from twisted.internet import defer, reactor from twisted.web import server, resource from twisted.web.server import NOT_DONE_YET from twisted.web.util import redirectTo +import base64 import collections import json import logging +import os logger = logging.getLogger(__name__) @@ -188,14 +191,52 @@ class FileUploadResource(resource.Resource): file_map_func = self.map_request_to_name self.get_name_for_request = file_map_func + if not os.path.isdir(self.directory): + os.mkdir(self.directory) + logger.info("FileUploadResource : Created %s directory.", + self.directory) + @defer.inlineCallbacks def map_request_to_name(self, request): # auth the user auth_user = yield self.auth.get_user_by_req(request) - logger.info("User %s is uploading a file.", auth_user) - defer.returnValue("boo2.png") - def render(self, request): + # namespace all file uploads on the user + prefix = base64.urlsafe_b64encode( + auth_user.to_string() + ).replace('=', '') + + # use a random string for the main portion + main_part = random_string(24) + + # suffix with a file extension if we can make one. This is nice to + # provide a hint to clients on the file information. + suffix = "" + if request.requestHeaders.hasHeader("Content-Type"): + content_type = request.requestHeaders.getRawHeaders( + "Content-Type")[0] + if (content_type.split("/")[0].lower() in + ["image", "video", "audio"]): + suffix = "." + content_type.split("/")[-1] + + file_path = os.path.join(self.directory, prefix + main_part + suffix) + logger.info("User %s is uploading a file to path %s", + auth_user.to_string(), + file_path) + + # keep trying to make a non-clashing file, with a sensible max attempts + attempts = 0 + while os.path.exists(file_path): + main_part = random_string(24) + file_path = os.path.join(self.directory, + prefix + main_part + suffix) + attempts += 1 + if attempts > 25: # really? Really? + raise SynapseError(500, "Unable to create file.") + + defer.returnValue(file_path) + + def render_POST(self, request): self._async_render(request) return server.NOT_DONE_YET @@ -208,7 +249,7 @@ class FileUploadResource(resource.Resource): f.write(request.content.read()) respond_with_json_bytes(request, 200, - json.dumps({"url": "not_implemented2"}), + json.dumps({"path": fname}), send_cors=True) except CodeMessageException as e: -- cgit 1.5.1 From 58548ab557bbbdd95644997e00777b0aec8532bc Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Mon, 18 Aug 2014 17:18:39 +0100 Subject: Implemented GETs for the ContentRepoResource. It all actually appears to be working. --- synapse/app/homeserver.py | 4 +- synapse/http/server.py | 71 ++++++++++++++++++---- .../components/fileUpload/file-upload-service.js | 5 +- 3 files changed, 65 insertions(+), 15 deletions(-) (limited to 'synapse') diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index f7c1da9201..ca102236cf 100755 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -24,7 +24,7 @@ from twisted.python.log import PythonLoggingObserver from twisted.web.resource import Resource from twisted.web.static import File from twisted.web.server import Site -from synapse.http.server import JsonResource, RootRedirect, FileUploadResource +from synapse.http.server import JsonResource, RootRedirect, ContentRepoResource from synapse.http.client import TwistedHttpClient from synapse.api.urls import ( CLIENT_PREFIX, FEDERATION_PREFIX, WEB_CLIENT_PREFIX, CONTENT_REPO_PREFIX @@ -56,7 +56,7 @@ class SynapseHomeServer(HomeServer): return File("webclient") # TODO configurable? def build_resource_for_content_repo(self): - return FileUploadResource("uploads", self.auth) + return ContentRepoResource("uploads", self.auth) def build_db_pool(self): """ Set up all the dbs. Since all the *.sql have IF NOT EXISTS, so we diff --git a/synapse/http/server.py b/synapse/http/server.py index 8502416fca..9b6ae993ab 100644 --- a/synapse/http/server.py +++ b/synapse/http/server.py @@ -17,10 +17,13 @@ from syutil.jsonutil import ( encode_canonical_json, encode_pretty_printed_json ) -from synapse.api.errors import cs_exception, SynapseError, CodeMessageException +from synapse.api.errors import ( + cs_exception, SynapseError, CodeMessageException, Codes, cs_error +) from synapse.util.stringutils import random_string from twisted.internet import defer, reactor +from twisted.protocols.basic import FileSender from twisted.web import server, resource from twisted.web.server import NOT_DONE_YET from twisted.web.util import redirectTo @@ -30,7 +33,7 @@ import collections import json import logging import os - +import re logger = logging.getLogger(__name__) @@ -180,16 +183,25 @@ class RootRedirect(resource.Resource): return resource.Resource.getChild(self, name, request) -class FileUploadResource(resource.Resource): +class ContentRepoResource(resource.Resource): + """Provides file uploading and downloading. + + Uploads are POSTed to wherever this Resource is linked to. This resource + returns a "content token" which can be used to GET this content again. The + token is typically a path, but it may not be. + + In this case, the token contains 3 sections: + - User ID base64d (for namespacing content to each user) + - random string + - Content type base64d (so we can return it when clients GET it) + + """ isLeaf = True - def __init__(self, directory, auth, file_map_func=None): + def __init__(self, directory, auth): resource.Resource.__init__(self) self.directory = directory self.auth = auth - if not file_map_func: - file_map_func = self.map_request_to_name - self.get_name_for_request = file_map_func if not os.path.isdir(self.directory): os.mkdir(self.directory) @@ -210,14 +222,19 @@ class FileUploadResource(resource.Resource): main_part = random_string(24) # suffix with a file extension if we can make one. This is nice to - # provide a hint to clients on the file information. + # provide a hint to clients on the file information. We will also reuse + # this info to spit back the content type to the client. suffix = "" if request.requestHeaders.hasHeader("Content-Type"): content_type = request.requestHeaders.getRawHeaders( "Content-Type")[0] + suffix = "." + base64.urlsafe_b64encode(content_type) if (content_type.split("/")[0].lower() in ["image", "video", "audio"]): - suffix = "." + content_type.split("/")[-1] + file_ext = content_type.split("/")[-1] + # be a little paranoid and only allow a-z + file_ext = re.sub("[^a-z]", "", file_ext) + suffix += "." + file_ext file_path = os.path.join(self.directory, prefix + main_part + suffix) logger.info("User %s is uploading a file to path %s", @@ -236,6 +253,37 @@ class FileUploadResource(resource.Resource): defer.returnValue(file_path) + def render_GET(self, request): + # no auth here on purpose, to allow anyone to view, even across home + # servers. + + # TODO: A little crude here, we could do this better. + filename = request.path.split(self.directory + "/")[1] + # be paranoid + filename = re.sub("[^0-9A-z.-_]", "", filename) + + file_path = self.directory + "/" + filename + if os.path.isfile(file_path): + # filename has the content type + base64_contentype = filename.split(".")[1] + content_type = base64.urlsafe_b64decode(base64_contentype) + logger.info("Sending file %s", file_path) + f = open(file_path, 'rb') + request.setHeader('Content-Type', content_type) + d = FileSender().beginFileTransfer(f, request) + def cbFinished(ignored): + f.close() + request.finish() + d.addCallback(cbFinished) + else: + respond_with_json_bytes( + request, + 404, + json.dumps(cs_error("Not found", code=Codes.NOT_FOUND)), + send_cors=True) + + return server.NOT_DONE_YET + def render_POST(self, request): self._async_render(request) return server.NOT_DONE_YET @@ -243,13 +291,14 @@ class FileUploadResource(resource.Resource): @defer.inlineCallbacks def _async_render(self, request): try: - fname = yield self.get_name_for_request(request) + fname = yield self.map_request_to_name(request) + # TODO I have a suspcious feeling this is just going to block with open(fname, "wb") as f: f.write(request.content.read()) respond_with_json_bytes(request, 200, - json.dumps({"path": fname}), + json.dumps({"content_token": fname}), send_cors=True) except CodeMessageException as e: diff --git a/webclient/components/fileUpload/file-upload-service.js b/webclient/components/fileUpload/file-upload-service.js index 0826666fe4..d620e6a4d0 100644 --- a/webclient/components/fileUpload/file-upload-service.js +++ b/webclient/components/fileUpload/file-upload-service.js @@ -32,8 +32,9 @@ angular.module('mFileUpload', []) console.log("Uploading " + file.name + "... to /matrix/content"); matrixService.uploadContent(file).then( function(response) { - console.log(" -> Successfully uploaded! Available at " + location.origin + response.data.url); - deferred.resolve(location.origin + response.data.url); + var content_url = location.origin + "/matrix/content/" + response.data.content_token; + console.log(" -> Successfully uploaded! Available at " + content_url); + deferred.resolve(content_url); }, function(error) { console.log(" -> Failed to upload " + file.name); -- cgit 1.5.1 From e37b040bc3a392f2705fbabf4123b61ad6ac05ee Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Mon, 18 Aug 2014 17:22:31 +0100 Subject: Small amounts of cleanup and bonus round comments. --- synapse/http/server.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) (limited to 'synapse') diff --git a/synapse/http/server.py b/synapse/http/server.py index 9b6ae993ab..42fb9f5e96 100644 --- a/synapse/http/server.py +++ b/synapse/http/server.py @@ -188,11 +188,13 @@ class ContentRepoResource(resource.Resource): Uploads are POSTed to wherever this Resource is linked to. This resource returns a "content token" which can be used to GET this content again. The - token is typically a path, but it may not be. + token is typically a path, but it may not be. Tokens can expire, be one-time + uses, etc. - In this case, the token contains 3 sections: + In this case, the token is a path to the file and contains 3 interesting + sections: - User ID base64d (for namespacing content to each user) - - random string + - random 24 char string - Content type base64d (so we can return it when clients GET it) """ @@ -205,7 +207,7 @@ class ContentRepoResource(resource.Resource): if not os.path.isdir(self.directory): os.mkdir(self.directory) - logger.info("FileUploadResource : Created %s directory.", + logger.info("ContentRepoResource : Created %s directory.", self.directory) @defer.inlineCallbacks @@ -271,6 +273,8 @@ class ContentRepoResource(resource.Resource): f = open(file_path, 'rb') request.setHeader('Content-Type', content_type) d = FileSender().beginFileTransfer(f, request) + + # after the file has been sent, clean up and finish the request def cbFinished(ignored): f.close() request.finish() -- cgit 1.5.1 From f48792eec43f893f4f893ffdcbf00f8958b6f6b5 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Tue, 19 Aug 2014 10:55:44 +0100 Subject: Reduce the amount of incredibly spammy stack traces. Expected errors (e.g. SynapseErrors) shouldn't have their full trace logged every time. Don't send responses to disconnected requests. --- synapse/http/server.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) (limited to 'synapse') diff --git a/synapse/http/server.py b/synapse/http/server.py index 42fb9f5e96..c28d9a33f9 100644 --- a/synapse/http/server.py +++ b/synapse/http/server.py @@ -132,7 +132,11 @@ class JsonResource(HttpServer, resource.Resource): {"error": "Unrecognized request"} ) except CodeMessageException as e: - logger.exception(e) + if isinstance(e, SynapseError): + logger.error("%s SynapseError: %s - %s", request, e.code, + e.msg) + else: + logger.exception(e) self._send_response( request, e.code, @@ -147,6 +151,14 @@ class JsonResource(HttpServer, resource.Resource): ) def _send_response(self, request, code, response_json_object): + # could alternatively use request.notifyFinish() and flip a flag when + # the Deferred fires, but since the flag is RIGHT THERE it seems like + # a waste. + if request._disconnected: + logger.warn( + "Not sending response to request %s, already disconnected.", + request) + return if not self._request_user_agent_is_curl(request): json_bytes = encode_canonical_json(response_json_object) -- cgit 1.5.1 From 6fafa878f6968af3041ae7265f0ef11854bb254f Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Mon, 18 Aug 2014 16:07:14 +0100 Subject: Deny __iter__ on UserID/RoomID/RoomName instances as it's a subtle bug that will bite you --- synapse/types.py | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'synapse') diff --git a/synapse/types.py b/synapse/types.py index 054b1e713c..b8e191bb3c 100644 --- a/synapse/types.py +++ b/synapse/types.py @@ -32,6 +32,12 @@ class DomainSpecificString( HomeServer as being its own """ + # Deny iteration because it will bite you if you try to create a singleton + # set by: + # users = set(user) + def __iter__(self): + raise ValueError("Attempted to iterate a %s" % (type(self).__name__)) + @classmethod def from_string(cls, s, hs): """Parse the string given by 's' into a structure object.""" -- cgit 1.5.1 From 83f031207e3ff1ceaacccaf83df76ea67aeefb1b Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Mon, 18 Aug 2014 16:43:18 +0100 Subject: Implement and test presence dropping of remote users --- synapse/handlers/presence.py | 8 ++++++-- tests/handlers/test_presence.py | 8 ++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) (limited to 'synapse') diff --git a/synapse/handlers/presence.py b/synapse/handlers/presence.py index e8cb83eddb..73ec45c9cb 100644 --- a/synapse/handlers/presence.py +++ b/synapse/handlers/presence.py @@ -463,9 +463,13 @@ class PresenceHandler(BaseHandler): deferreds = [] if target_user: - raise NotImplementedError("TODO: remove one user") + if target_user not in self._remote_recvmap: + return + target_users = set([target_user]) + else: + target_users = self._remote_recvmap.keys() - remoteusers = [u for u in self._remote_recvmap + remoteusers = [u for u in target_users if user in self._remote_recvmap[u]] remoteusers_by_domain = partition(remoteusers, lambda u: u.domain) diff --git a/tests/handlers/test_presence.py b/tests/handlers/test_presence.py index 86bd8bb3f2..a1856e46e2 100644 --- a/tests/handlers/test_presence.py +++ b/tests/handlers/test_presence.py @@ -383,6 +383,14 @@ class PresenceInvitesTestCase(unittest.TestCase): self.mock_stop.assert_called_with( self.u_apple, target_user=self.u_banana) + @defer.inlineCallbacks + def test_drop_remote(self): + yield self.handler.drop( + observer_user=self.u_apple, observed_user=self.u_cabbage) + + self.datastore.del_presence_list.assert_called_with( + "apple", "@cabbage:elsewhere") + @defer.inlineCallbacks def test_get_presence_list(self): self.datastore.get_presence_list.return_value = defer.succeed( -- cgit 1.5.1 From 88f7482b92564930c4ee6d5b4c0f32050e1b2e6b Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Mon, 18 Aug 2014 16:52:29 +0100 Subject: Perform the 'REST'-level tests of Presence against the real Presence handler as well, mocking out the datastore beneath it --- synapse/handlers/presence.py | 2 +- tests/rest/test_presence.py | 132 ++++++++++++++++++++++++++++++------------- 2 files changed, 93 insertions(+), 41 deletions(-) (limited to 'synapse') diff --git a/synapse/handlers/presence.py b/synapse/handlers/presence.py index 73ec45c9cb..7e725d1027 100644 --- a/synapse/handlers/presence.py +++ b/synapse/handlers/presence.py @@ -383,7 +383,7 @@ class PresenceHandler(BaseHandler): logger.debug("Start polling for presence from %s", user) if target_user: - target_users = set(target_user) + target_users = set([target_user]) else: presence = yield self.store.get_presence_list( user.localpart, accepted=True diff --git a/tests/rest/test_presence.py b/tests/rest/test_presence.py index 33817df889..0ba72addf6 100644 --- a/tests/rest/test_presence.py +++ b/tests/rest/test_presence.py @@ -24,6 +24,7 @@ import logging from ..utils import MockHttpResource from synapse.api.constants import PresenceState +from synapse.handlers.presence import PresenceHandler from synapse.server import HomeServer @@ -39,28 +40,48 @@ myid = "@apple:test" PATH_PREFIX = "/matrix/client/api/v1" +class JustPresenceHandlers(object): + def __init__(self, hs): + self.presence_handler = PresenceHandler(hs) + + class PresenceStateTestCase(unittest.TestCase): def setUp(self): self.mock_resource = MockHttpResource(prefix=PATH_PREFIX) - self.mock_handler = Mock(spec=[ - "get_state", - "set_state", - ]) hs = HomeServer("test", db_pool=None, + datastore=Mock(spec=[ + "get_presence_state", + "set_presence_state", + ]), http_client=None, resource_for_client=self.mock_resource, resource_for_federation=self.mock_resource, ) + hs.handlers = JustPresenceHandlers(hs) + + self.datastore = hs.get_datastore() + + def get_presence_list(*a, **kw): + return defer.succeed([]) + self.datastore.get_presence_list = get_presence_list def _get_user_by_token(token=None): return hs.parse_userid(myid) hs.get_auth().get_user_by_token = _get_user_by_token - hs.get_handlers().presence_handler = self.mock_handler + room_member_handler = hs.handlers.room_member_handler = Mock( + spec=[ + "get_rooms_for_user", + ] + ) + + def get_rooms_for_user(user): + return defer.succeed([]) + room_member_handler.get_rooms_for_user = get_rooms_for_user hs.register_servlets() @@ -68,9 +89,10 @@ class PresenceStateTestCase(unittest.TestCase): @defer.inlineCallbacks def test_get_my_status(self): - mocked_get = self.mock_handler.get_state + mocked_get = self.datastore.get_presence_state mocked_get.return_value = defer.succeed( - {"state": ONLINE, "status_msg": "Available"}) + {"state": ONLINE, "status_msg": "Available"} + ) (code, response) = yield self.mock_resource.trigger("GET", "/presence/%s/status" % (myid), None) @@ -78,47 +100,63 @@ class PresenceStateTestCase(unittest.TestCase): self.assertEquals(200, code) self.assertEquals({"state": ONLINE, "status_msg": "Available"}, response) - mocked_get.assert_called_with(target_user=self.u_apple, - auth_user=self.u_apple) + mocked_get.assert_called_with("apple") @defer.inlineCallbacks def test_set_my_status(self): - mocked_set = self.mock_handler.set_state - mocked_set.return_value = defer.succeed(()) + mocked_set = self.datastore.set_presence_state + mocked_set.return_value = defer.succeed({"state": OFFLINE}) (code, response) = yield self.mock_resource.trigger("PUT", "/presence/%s/status" % (myid), '{"state": "unavailable", "status_msg": "Away"}') self.assertEquals(200, code) - mocked_set.assert_called_with(target_user=self.u_apple, - auth_user=self.u_apple, - state={"state": UNAVAILABLE, "status_msg": "Away"}) + mocked_set.assert_called_with("apple", + {"state": UNAVAILABLE, "status_msg": "Away"}) class PresenceListTestCase(unittest.TestCase): def setUp(self): self.mock_resource = MockHttpResource(prefix=PATH_PREFIX) - self.mock_handler = Mock(spec=[ - "get_presence_list", - "send_invite", - "drop", - ]) hs = HomeServer("test", db_pool=None, + datastore=Mock(spec=[ + "has_presence_state", + "get_presence_state", + "allow_presence_visible", + "is_presence_visible", + "add_presence_list_pending", + "set_presence_list_accepted", + "del_presence_list", + "get_presence_list", + ]), http_client=None, resource_for_client=self.mock_resource, resource_for_federation=self.mock_resource ) + hs.handlers = JustPresenceHandlers(hs) + + self.datastore = hs.get_datastore() + + def has_presence_state(user_localpart): + return defer.succeed( + user_localpart in ("apple", "banana",) + ) + self.datastore.has_presence_state = has_presence_state def _get_user_by_token(token=None): return hs.parse_userid(myid) - hs.get_auth().get_user_by_token = _get_user_by_token + room_member_handler = hs.handlers.room_member_handler = Mock( + spec=[ + "get_rooms_for_user", + ] + ) - hs.get_handlers().presence_handler = self.mock_handler + hs.get_auth().get_user_by_token = _get_user_by_token hs.register_servlets() @@ -127,53 +165,67 @@ class PresenceListTestCase(unittest.TestCase): @defer.inlineCallbacks def test_get_my_list(self): - self.mock_handler.get_presence_list.return_value = defer.succeed( - [{"observed_user": self.u_banana}] + self.datastore.get_presence_list.return_value = defer.succeed( + [{"observed_user_id": "@banana:test"}], ) (code, response) = yield self.mock_resource.trigger("GET", "/presence_list/%s" % (myid), None) self.assertEquals(200, code) - self.assertEquals([{"user_id": "@banana:test"}], response) + self.assertEquals( + [{"user_id": "@banana:test", "state": OFFLINE}], response + ) + + self.datastore.get_presence_list.assert_called_with( + "apple", accepted=True + ) @defer.inlineCallbacks def test_invite(self): - self.mock_handler.send_invite.return_value = defer.succeed(()) + self.datastore.add_presence_list_pending.return_value = ( + defer.succeed(()) + ) + self.datastore.is_presence_visible.return_value = defer.succeed( + True + ) (code, response) = yield self.mock_resource.trigger("POST", - "/presence_list/%s" % (myid), - """{ - "invite": ["@banana:test"] - }""") + "/presence_list/%s" % (myid), + """{"invite": ["@banana:test"]}""" + ) self.assertEquals(200, code) - self.mock_handler.send_invite.assert_called_with( - observer_user=self.u_apple, observed_user=self.u_banana) + self.datastore.add_presence_list_pending.assert_called_with( + "apple", "@banana:test" + ) + self.datastore.set_presence_list_accepted.assert_called_with( + "apple", "@banana:test" + ) @defer.inlineCallbacks def test_drop(self): - self.mock_handler.drop.return_value = defer.succeed(()) + self.datastore.del_presence_list.return_value = ( + defer.succeed(()) + ) (code, response) = yield self.mock_resource.trigger("POST", - "/presence_list/%s" % (myid), - """{ - "drop": ["@banana:test"] - }""") + "/presence_list/%s" % (myid), + """{"drop": ["@banana:test"]}""" + ) self.assertEquals(200, code) - self.mock_handler.drop.assert_called_with( - observer_user=self.u_apple, observed_user=self.u_banana) + self.datastore.del_presence_list.assert_called_with( + "apple", "@banana:test" + ) class PresenceEventStreamTestCase(unittest.TestCase): def setUp(self): self.mock_resource = MockHttpResource(prefix=PATH_PREFIX) - # TODO: mocked data store - # HIDEOUS HACKERY # TODO(paul): This should be injected in via the HomeServer DI system from synapse.handlers.events import EventStreamHandler -- cgit 1.5.1 From ece7a6d995304752f093d8db2aafbf5baca93e90 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Tue, 19 Aug 2014 11:50:57 +0100 Subject: Unquote sender IDs. --- synapse/rest/room.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'synapse') diff --git a/synapse/rest/room.py b/synapse/rest/room.py index 035209a19d..c15eb9de48 100644 --- a/synapse/rest/room.py +++ b/synapse/rest/room.py @@ -235,7 +235,7 @@ class MessageRestServlet(RestServlet): msg_handler = self.handlers.message_handler msg = yield msg_handler.get_message(room_id=urllib.unquote(room_id), - sender_id=sender_id, + sender_id=urllib.unquote(sender_id), msg_id=msg_id, user_id=user.to_string(), ) @@ -250,7 +250,7 @@ class MessageRestServlet(RestServlet): def on_PUT(self, request, room_id, sender_id, msg_id): user = yield self.auth.get_user_by_req(request) - if user.to_string() != sender_id: + if user.to_string() != urllib.unquote(sender_id): raise SynapseError(403, "Must send messages as yourself.", errcode=Codes.FORBIDDEN) -- cgit 1.5.1 From caef65d819cc420d61c6926be36c8d97bfb81886 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Tue, 19 Aug 2014 12:30:28 +0100 Subject: More unquotes. Also, don't return the room_id on membership state changes, they already know it. --- synapse/rest/room.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'synapse') diff --git a/synapse/rest/room.py b/synapse/rest/room.py index c15eb9de48..db8f18e8b3 100644 --- a/synapse/rest/room.py +++ b/synapse/rest/room.py @@ -170,8 +170,10 @@ class RoomMemberRestServlet(RestServlet): user = yield self.auth.get_user_by_req(request) handler = self.handlers.room_member_handler - member = yield handler.get_room_member(room_id, target_user_id, - user.to_string()) + member = yield handler.get_room_member( + room_id, + urllib.unquote(target_user_id), + user.to_string()) if not member: raise SynapseError(404, "Member not found.", errcode=Codes.NOT_FOUND) @@ -183,7 +185,7 @@ class RoomMemberRestServlet(RestServlet): event = self.event_factory.create_event( etype=self.get_event_type(), - target_user_id=target_user_id, + target_user_id=urllib.unquote(target_user_id), room_id=urllib.unquote(roomid), user_id=user.to_string(), membership=Membership.LEAVE, @@ -210,7 +212,7 @@ class RoomMemberRestServlet(RestServlet): event = self.event_factory.create_event( etype=self.get_event_type(), - target_user_id=target_user_id, + target_user_id=urllib.unquote(target_user_id), room_id=urllib.unquote(roomid), user_id=user.to_string(), membership=content["membership"], @@ -218,8 +220,8 @@ class RoomMemberRestServlet(RestServlet): ) handler = self.handlers.room_member_handler - result = yield handler.change_membership(event, broadcast_msg=True) - defer.returnValue((200, result)) + yield handler.change_membership(event, broadcast_msg=True) + defer.returnValue((200, "")) class MessageRestServlet(RestServlet): -- cgit 1.5.1 From 992782b9f527f2c2cec083a46d4035d1a6e3673e Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Tue, 19 Aug 2014 14:24:53 +0100 Subject: Ensure that federation's .send_edu() returns a Deferred --- synapse/federation/replication.py | 1 + 1 file changed, 1 insertion(+) (limited to 'synapse') diff --git a/synapse/federation/replication.py b/synapse/federation/replication.py index bc9df2f214..c9f2e06b7b 100644 --- a/synapse/federation/replication.py +++ b/synapse/federation/replication.py @@ -158,6 +158,7 @@ class ReplicationLayer(object): # TODO, add errback, etc. self._transaction_queue.enqueue_edu(edu) + return defer.succeed(None) @log_function def make_query(self, destination, query_type, args): -- cgit 1.5.1