From ce55a8cc4bb26c4518875743a04a06e792ad7ebf Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Wed, 10 Sep 2014 15:42:15 +0100 Subject: [PATCH 01/35] Move database preparing code out of homserver.py into storage where it belongs --- synapse/app/homeserver.py | 73 +++++-------------------------------- synapse/server.py | 1 + synapse/storage/__init__.py | 61 +++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 64 deletions(-) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index d675d8c8f..b63ecd4b5 100755 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -14,7 +14,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -from synapse.storage import read_schema +from synapse.storage import prepare_database from synapse.server import HomeServer @@ -36,7 +36,6 @@ from daemonize import Daemonize import twisted.manhole.telnet import logging -import sqlite3 import os import re import sys @@ -44,22 +43,6 @@ import sys logger = logging.getLogger(__name__) -SCHEMAS = [ - "transactions", - "pdu", - "users", - "profiles", - "presence", - "im", - "room_aliases", -] - - -# Remember to update this number every time an incompatible change is made to -# database schema files, so the users will be informed on server restarts. -SCHEMA_VERSION = 3 - - class SynapseHomeServer(HomeServer): def build_http_client(self): @@ -80,52 +63,12 @@ class SynapseHomeServer(HomeServer): ) 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. - """ - logging.info("Preparing database: %s...", self.db_name) - - with sqlite3.connect(self.db_name) as db_conn: - c = db_conn.cursor() - c.execute("PRAGMA user_version") - row = c.fetchone() - - if row and row[0]: - user_version = row[0] - - if user_version > SCHEMA_VERSION: - raise ValueError("Cannot use this database as it is too " + - "new for the server to understand" - ) - elif user_version < SCHEMA_VERSION: - logging.info("Upgrading database from version %d", - user_version - ) - - # Run every version since after the current version. - for v in range(user_version + 1, SCHEMA_VERSION + 1): - sql_script = read_schema("delta/v%d" % (v)) - c.executescript(sql_script) - - db_conn.commit() - - else: - for sql_loc in SCHEMAS: - sql_script = read_schema(sql_loc) - - c.executescript(sql_script) - db_conn.commit() - c.execute("PRAGMA user_version = %d" % SCHEMA_VERSION) - - c.close() - - logging.info("Database prepared in %s.", self.db_name) - - pool = adbapi.ConnectionPool( - 'sqlite3', self.db_name, check_same_thread=False, - cp_min=1, cp_max=1) - - return pool + return adbapi.ConnectionPool( + "sqlite3", self.get_db_name(), + check_same_thread=False, + cp_min=1, + cp_max=1 + ) def create_resource_tree(self, web_client, redirect_root_to_web_client): """Create the resource tree for this Home Server. @@ -270,6 +213,8 @@ def setup(): ) hs.start_listening(config.bind_port, config.unsecure_port) + prepare_database(hs.get_db_name()) + hs.get_db_pool() if config.manhole: diff --git a/synapse/server.py b/synapse/server.py index 83368ea5a..1ba13f3df 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -57,6 +57,7 @@ class BaseHomeServer(object): DEPENDENCIES = [ 'clock', 'http_client', + 'db_name', 'db_pool', 'persistence_service', 'replication_layer', diff --git a/synapse/storage/__init__.py b/synapse/storage/__init__.py index ad2a484c1..2543fb12b 100644 --- a/synapse/storage/__init__.py +++ b/synapse/storage/__init__.py @@ -43,10 +43,28 @@ from .keys import KeyStore import json import logging import os +import sqlite3 logger = logging.getLogger(__name__) + +SCHEMAS = [ + "transactions", + "pdu", + "users", + "profiles", + "presence", + "im", + "room_aliases", +] + + +# Remember to update this number every time an incompatible change is made to +# database schema files, so the users will be informed on server restarts. +SCHEMA_VERSION = 3 + + class _RollbackButIsFineException(Exception): """ This exception is used to rollback a transaction without implying something went wrong. @@ -350,3 +368,46 @@ def read_schema(schema): """ with open(schema_path(schema)) as schema_file: return schema_file.read() + + +def prepare_database(db_name): + """ Set up all the dbs. Since all the *.sql have IF NOT EXISTS, so we + don't have to worry about overwriting existing content. + """ + logging.info("Preparing database: %s...", db_name) + + with sqlite3.connect(db_name) as db_conn: + c = db_conn.cursor() + c.execute("PRAGMA user_version") + row = c.fetchone() + + if row and row[0]: + user_version = row[0] + + if user_version > SCHEMA_VERSION: + raise ValueError("Cannot use this database as it is too " + + "new for the server to understand" + ) + elif user_version < SCHEMA_VERSION: + logging.info("Upgrading database from version %d", + user_version + ) + + # Run every version since after the current version. + for v in range(user_version + 1, SCHEMA_VERSION + 1): + sql_script = read_schema("delta/v%d" % (v)) + c.executescript(sql_script) + + db_conn.commit() + + else: + for sql_loc in SCHEMAS: + sql_script = read_schema(sql_loc) + + c.executescript(sql_script) + db_conn.commit() + c.execute("PRAGMA user_version = %d" % SCHEMA_VERSION) + + c.close() + + logging.info("Database prepared in %s.", db_name) From 6c1f0055dc18038deb133ffad7718705e298c146 Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Wed, 10 Sep 2014 16:07:44 +0100 Subject: [PATCH 02/35] No need for a tiny run() function any more, just use reactor.run() directly --- synapse/app/homeserver.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index b63ecd4b5..e9a632102 100755 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -173,10 +173,6 @@ class SynapseHomeServer(HomeServer): logger.info("Synapse now listening on port %d", unsecure_port) -def run(): - reactor.run() - - def setup(): config = HomeServerConfig.load_config( "Synapse Homeserver", @@ -229,7 +225,7 @@ def setup(): daemon = Daemonize( app="synapse-homeserver", pid=config.pid_file, - action=run, + action=reactor.run, auto_close_fds=False, verbose=True, logger=logger, @@ -237,7 +233,7 @@ def setup(): daemon.start() else: - run() + reactor.run() if __name__ == '__main__': From 2faffc52eed70df7cf1adc021633f4a427917c90 Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Wed, 10 Sep 2014 16:16:24 +0100 Subject: [PATCH 03/35] Make sure not to open our TCP ports until /after/ the DB is nicely prepared ready for use --- synapse/app/homeserver.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index e9a632102..e6377e306 100755 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -207,7 +207,6 @@ def setup(): web_client=config.webclient, redirect_root_to_web_client=True, ) - hs.start_listening(config.bind_port, config.unsecure_port) prepare_database(hs.get_db_name()) @@ -220,6 +219,8 @@ def setup(): f.namespace['hs'] = hs reactor.listenTCP(config.manhole, f, interface='127.0.0.1') + hs.start_listening(config.bind_port, config.unsecure_port) + if config.daemonize: print config.pid_file daemon = Daemonize( From 55397f634770f2b91cd4567e6b40507944144b67 Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Wed, 10 Sep 2014 16:23:58 +0100 Subject: [PATCH 04/35] prepare_database() on db_conn, not plain name, so we can pass in the connection from outside --- synapse/app/homeserver.py | 10 ++++++- synapse/storage/__init__.py | 59 +++++++++++++++++-------------------- 2 files changed, 36 insertions(+), 33 deletions(-) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index e6377e306..2f1b95490 100755 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -39,6 +39,7 @@ import logging import os import re import sys +import sqlite3 logger = logging.getLogger(__name__) @@ -208,7 +209,14 @@ def setup(): redirect_root_to_web_client=True, ) - prepare_database(hs.get_db_name()) + db_name = hs.get_db_name() + + logging.info("Preparing database: %s...", db_name) + + with sqlite3.connect(db_name) as db_conn: + prepare_database(db_conn) + + logging.info("Database prepared in %s.", db_name) hs.get_db_pool() diff --git a/synapse/storage/__init__.py b/synapse/storage/__init__.py index 2543fb12b..6b273a030 100644 --- a/synapse/storage/__init__.py +++ b/synapse/storage/__init__.py @@ -43,7 +43,6 @@ from .keys import KeyStore import json import logging import os -import sqlite3 logger = logging.getLogger(__name__) @@ -370,44 +369,40 @@ def read_schema(schema): return schema_file.read() -def prepare_database(db_name): +def prepare_database(db_conn): """ Set up all the dbs. Since all the *.sql have IF NOT EXISTS, so we don't have to worry about overwriting existing content. """ - logging.info("Preparing database: %s...", db_name) + c = db_conn.cursor() + c.execute("PRAGMA user_version") + row = c.fetchone() - with sqlite3.connect(db_name) as db_conn: - c = db_conn.cursor() - c.execute("PRAGMA user_version") - row = c.fetchone() + if row and row[0]: + user_version = row[0] - if row and row[0]: - user_version = row[0] - - if user_version > SCHEMA_VERSION: - raise ValueError("Cannot use this database as it is too " + - "new for the server to understand" - ) - elif user_version < SCHEMA_VERSION: - logging.info("Upgrading database from version %d", - user_version - ) - - # Run every version since after the current version. - for v in range(user_version + 1, SCHEMA_VERSION + 1): - sql_script = read_schema("delta/v%d" % (v)) - c.executescript(sql_script) - - db_conn.commit() - - else: - for sql_loc in SCHEMAS: - sql_script = read_schema(sql_loc) + if user_version > SCHEMA_VERSION: + raise ValueError("Cannot use this database as it is too " + + "new for the server to understand" + ) + elif user_version < SCHEMA_VERSION: + logging.info("Upgrading database from version %d", + user_version + ) + # Run every version since after the current version. + for v in range(user_version + 1, SCHEMA_VERSION + 1): + sql_script = read_schema("delta/v%d" % (v)) c.executescript(sql_script) + db_conn.commit() - c.execute("PRAGMA user_version = %d" % SCHEMA_VERSION) - c.close() + else: + for sql_loc in SCHEMAS: + sql_script = read_schema(sql_loc) + + c.executescript(sql_script) + db_conn.commit() + c.execute("PRAGMA user_version = %d" % SCHEMA_VERSION) + + c.close() - logging.info("Database prepared in %s.", db_name) From 6081f4947e9c4d7dfbdaf6439e662b173d309a8e Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Wed, 10 Sep 2014 16:42:31 +0100 Subject: [PATCH 05/35] Tiny trivial PoC unit-test using SQLite in :memory: mode --- tests/storage/test_profile.py | 78 +++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 tests/storage/test_profile.py diff --git a/tests/storage/test_profile.py b/tests/storage/test_profile.py new file mode 100644 index 000000000..fff0a8c4f --- /dev/null +++ b/tests/storage/test_profile.py @@ -0,0 +1,78 @@ +# -*- coding: utf-8 -*- +# Copyright 2014 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. + + +from twisted.trial import unittest +from twisted.internet import defer + +from mock import Mock, call +from twisted.enterprise.adbapi import ConnectionPool + +from synapse.server import HomeServer +from synapse.storage import prepare_database +from synapse.storage.profile import ProfileStore + + +class SQLiteMemoryDbPool(ConnectionPool, object): + def __init__(self): + super(SQLiteMemoryDbPool, self).__init__( + "sqlite3", ":memory:", + cp_min=1, + cp_max=1, + ) + + def prepare(self): + return self.runWithConnection(prepare_database) + + #def runInteraction(self, interaction, *args, **kwargs): + # # Just use a cursor as the txn directly + # txn = self.db.cursor() + + # def _on_success(result): + # txn.commit() + # return result + # def _on_failure(failure): + # txn.rollback() + # raise failure + + # d = interaction(txn, *args, **kwargs) + # d.addCallbacks(_on_success, _on_failure) + # return d + + +class ProfileStoreTestCase(unittest.TestCase): + def setUp(self): + hs = HomeServer("test", + db_pool=SQLiteMemoryDbPool(), + ) + hs.get_db_pool().prepare() + + self.store = ProfileStore(hs) + + self.u_frank = hs.parse_userid("@frank:test") + + @defer.inlineCallbacks + def test_displayname(self): + yield self.store.create_profile( + self.u_frank.localpart + ) + + yield self.store.set_profile_displayname( + self.u_frank.localpart, "Frank" + ) + + name = yield self.store.get_profile_displayname(self.u_frank.localpart) + + self.assertEquals("Frank", name) From 53d0f69dc3b4a3f19f9e48c63a5dbc704b944c0c Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Wed, 10 Sep 2014 16:49:34 +0100 Subject: [PATCH 06/35] Also test avatar_url profile field --- tests/storage/test_profile.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/storage/test_profile.py b/tests/storage/test_profile.py index fff0a8c4f..bca056b29 100644 --- a/tests/storage/test_profile.py +++ b/tests/storage/test_profile.py @@ -76,3 +76,17 @@ class ProfileStoreTestCase(unittest.TestCase): name = yield self.store.get_profile_displayname(self.u_frank.localpart) self.assertEquals("Frank", name) + + @defer.inlineCallbacks + def test_avatar_url(self): + yield self.store.create_profile( + self.u_frank.localpart + ) + + yield self.store.set_profile_avatar_url( + self.u_frank.localpart, "http://my.site/here" + ) + + name = yield self.store.get_profile_avatar_url(self.u_frank.localpart) + + self.assertEquals("http://my.site/here", name) From 9774949cc989e2a24e2e8070bb4bd299335891ab Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Wed, 10 Sep 2014 16:50:09 +0100 Subject: [PATCH 07/35] It's considered polite to actually wait for DB prepare before running tests --- tests/storage/test_profile.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/storage/test_profile.py b/tests/storage/test_profile.py index bca056b29..45c69dafa 100644 --- a/tests/storage/test_profile.py +++ b/tests/storage/test_profile.py @@ -53,11 +53,14 @@ class SQLiteMemoryDbPool(ConnectionPool, object): class ProfileStoreTestCase(unittest.TestCase): + + @defer.inlineCallbacks def setUp(self): hs = HomeServer("test", db_pool=SQLiteMemoryDbPool(), ) - hs.get_db_pool().prepare() + + yield hs.get_db_pool().prepare() self.store = ProfileStore(hs) From 08f5c48fc81753ac29d4f76080a44bcd5aa52ece Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Wed, 10 Sep 2014 16:56:02 +0100 Subject: [PATCH 08/35] Move SQLiteMemoryDbPool implementation into tests.utils --- tests/storage/test_profile.py | 28 +--------------------------- tests/utils.py | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 27 deletions(-) diff --git a/tests/storage/test_profile.py b/tests/storage/test_profile.py index 45c69dafa..82e0c33be 100644 --- a/tests/storage/test_profile.py +++ b/tests/storage/test_profile.py @@ -18,38 +18,12 @@ from twisted.trial import unittest from twisted.internet import defer from mock import Mock, call -from twisted.enterprise.adbapi import ConnectionPool from synapse.server import HomeServer from synapse.storage import prepare_database from synapse.storage.profile import ProfileStore - -class SQLiteMemoryDbPool(ConnectionPool, object): - def __init__(self): - super(SQLiteMemoryDbPool, self).__init__( - "sqlite3", ":memory:", - cp_min=1, - cp_max=1, - ) - - def prepare(self): - return self.runWithConnection(prepare_database) - - #def runInteraction(self, interaction, *args, **kwargs): - # # Just use a cursor as the txn directly - # txn = self.db.cursor() - - # def _on_success(result): - # txn.commit() - # return result - # def _on_failure(failure): - # txn.rollback() - # raise failure - - # d = interaction(txn, *args, **kwargs) - # d.addCallbacks(_on_success, _on_failure) - # return d +from tests.utils import SQLiteMemoryDbPool class ProfileStoreTestCase(unittest.TestCase): diff --git a/tests/utils.py b/tests/utils.py index d90214e41..bc5d35e56 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -16,12 +16,14 @@ from synapse.http.server import HttpServer from synapse.api.errors import cs_error, CodeMessageException, StoreError from synapse.api.constants import Membership +from synapse.storage import prepare_database from synapse.api.events.room import ( RoomMemberEvent, MessageEvent ) from twisted.internet import defer, reactor +from twisted.enterprise.adbapi import ConnectionPool from collections import namedtuple from mock import patch, Mock @@ -120,6 +122,18 @@ class MockClock(object): self.now += secs +class SQLiteMemoryDbPool(ConnectionPool, object): + def __init__(self): + super(SQLiteMemoryDbPool, self).__init__( + "sqlite3", ":memory:", + cp_min=1, + cp_max=1, + ) + + def prepare(self): + return self.runWithConnection(prepare_database) + + class MemoryDataStore(object): Room = namedtuple( From dc7f39677f19be9f53054e97e095055166836acc Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Wed, 10 Sep 2014 16:56:52 +0100 Subject: [PATCH 09/35] Remember to kill now-dead import in test_profile.py --- tests/storage/test_profile.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/storage/test_profile.py b/tests/storage/test_profile.py index 82e0c33be..7e082091b 100644 --- a/tests/storage/test_profile.py +++ b/tests/storage/test_profile.py @@ -20,7 +20,6 @@ from twisted.internet import defer from mock import Mock, call from synapse.server import HomeServer -from synapse.storage import prepare_database from synapse.storage.profile import ProfileStore from tests.utils import SQLiteMemoryDbPool From dd1a9100c5eae1b2a55893de0f3e13b2877dfc77 Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Wed, 10 Sep 2014 17:51:05 +0100 Subject: [PATCH 10/35] Added unit tests for PresenceDataStore too --- tests/storage/test_presence.py | 166 +++++++++++++++++++++++++++++++++ tests/storage/test_profile.py | 2 - 2 files changed, 166 insertions(+), 2 deletions(-) create mode 100644 tests/storage/test_presence.py diff --git a/tests/storage/test_presence.py b/tests/storage/test_presence.py new file mode 100644 index 000000000..f0a04ae83 --- /dev/null +++ b/tests/storage/test_presence.py @@ -0,0 +1,166 @@ +# -*- coding: utf-8 -*- +# Copyright 2014 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. + + +from twisted.trial import unittest +from twisted.internet import defer + +from synapse.server import HomeServer +from synapse.storage.presence import PresenceStore + +from tests.utils import SQLiteMemoryDbPool, MockClock + + +class PresenceStoreTestCase(unittest.TestCase): + + @defer.inlineCallbacks + def setUp(self): + hs = HomeServer("test", + clock=MockClock(), + db_pool=SQLiteMemoryDbPool(), + ) + + yield hs.get_db_pool().prepare() + + self.store = PresenceStore(hs) + + self.u_apple = hs.parse_userid("@apple:test") + self.u_banana = hs.parse_userid("@banana:test") + + @defer.inlineCallbacks + def test_state(self): + yield self.store.create_presence( + self.u_apple.localpart + ) + + state = yield self.store.get_presence_state( + self.u_apple.localpart + ) + + self.assertEquals( + {"state": None, "status_msg": None, "mtime": None}, state + ) + + yield self.store.set_presence_state( + self.u_apple.localpart, {"state": "online", "status_msg": "Here"} + ) + + state = yield self.store.get_presence_state( + self.u_apple.localpart + ) + + self.assertEquals( + {"state": "online", "status_msg": "Here", "mtime": 1000000}, state + ) + + @defer.inlineCallbacks + def test_visibility(self): + self.assertFalse((yield self.store.is_presence_visible( + observed_localpart=self.u_apple.localpart, + observer_userid=self.u_banana.to_string(), + ))) + + yield self.store.allow_presence_visible( + observed_localpart=self.u_apple.localpart, + observer_userid=self.u_banana.to_string(), + ) + + self.assertTrue((yield self.store.is_presence_visible( + observed_localpart=self.u_apple.localpart, + observer_userid=self.u_banana.to_string(), + ))) + + yield self.store.disallow_presence_visible( + observed_localpart=self.u_apple.localpart, + observer_userid=self.u_banana.to_string(), + ) + + self.assertFalse((yield self.store.is_presence_visible( + observed_localpart=self.u_apple.localpart, + observer_userid=self.u_banana.to_string(), + ))) + + @defer.inlineCallbacks + def test_presence_list(self): + self.assertEquals( + [], + (yield self.store.get_presence_list( + observer_localpart=self.u_apple.localpart, + )) + ) + self.assertEquals( + [], + (yield self.store.get_presence_list( + observer_localpart=self.u_apple.localpart, + accepted=True, + )) + ) + + yield self.store.add_presence_list_pending( + observer_localpart=self.u_apple.localpart, + observed_userid=self.u_banana.to_string(), + ) + + self.assertEquals( + [{"observed_user_id": "@banana:test", "accepted": 0}], + (yield self.store.get_presence_list( + observer_localpart=self.u_apple.localpart, + )) + ) + self.assertEquals( + [], + (yield self.store.get_presence_list( + observer_localpart=self.u_apple.localpart, + accepted=True, + )) + ) + + yield self.store.set_presence_list_accepted( + observer_localpart=self.u_apple.localpart, + observed_userid=self.u_banana.to_string(), + ) + + self.assertEquals( + [{"observed_user_id": "@banana:test", "accepted": 1}], + (yield self.store.get_presence_list( + observer_localpart=self.u_apple.localpart, + )) + ) + self.assertEquals( + [{"observed_user_id": "@banana:test", "accepted": 1}], + (yield self.store.get_presence_list( + observer_localpart=self.u_apple.localpart, + accepted=True, + )) + ) + + yield self.store.del_presence_list( + observer_localpart=self.u_apple.localpart, + observed_userid=self.u_banana.to_string(), + ) + + self.assertEquals( + [], + (yield self.store.get_presence_list( + observer_localpart=self.u_apple.localpart, + )) + ) + self.assertEquals( + [], + (yield self.store.get_presence_list( + observer_localpart=self.u_apple.localpart, + accepted=True, + )) + ) diff --git a/tests/storage/test_profile.py b/tests/storage/test_profile.py index 7e082091b..fa96abf03 100644 --- a/tests/storage/test_profile.py +++ b/tests/storage/test_profile.py @@ -17,8 +17,6 @@ from twisted.trial import unittest from twisted.internet import defer -from mock import Mock, call - from synapse.server import HomeServer from synapse.storage.profile import ProfileStore From 79fe6083ebaf734fbba82cdace54c1cb3e3603fe Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Wed, 10 Sep 2014 18:11:32 +0100 Subject: [PATCH 11/35] Test ProfileHandler against the real datastore layer using SQLite :memory: --- tests/handlers/test_profile.py | 50 +++++++++++++++++----------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/tests/handlers/test_profile.py b/tests/handlers/test_profile.py index 8e7a89b47..8b9685a52 100644 --- a/tests/handlers/test_profile.py +++ b/tests/handlers/test_profile.py @@ -24,6 +24,8 @@ from synapse.api.errors import AuthError from synapse.server import HomeServer from synapse.handlers.profile import ProfileHandler +from tests.utils import SQLiteMemoryDbPool + logging.getLogger().addHandler(logging.NullHandler()) @@ -36,6 +38,7 @@ class ProfileHandlers(object): class ProfileTestCase(unittest.TestCase): """ Tests profile management. """ + @defer.inlineCallbacks def setUp(self): self.mock_federation = Mock(spec=[ "make_query", @@ -47,26 +50,24 @@ class ProfileTestCase(unittest.TestCase): self.mock_federation.register_query_handler = register_query_handler hs = HomeServer("test", - db_pool=None, + db_pool=SQLiteMemoryDbPool(), http_client=None, - datastore=Mock(spec=[ - "get_profile_displayname", - "set_profile_displayname", - "get_profile_avatar_url", - "set_profile_avatar_url", - ]), handlers=None, resource_for_federation=Mock(), replication_layer=self.mock_federation, ) hs.handlers = ProfileHandlers(hs) - self.datastore = hs.get_datastore() + yield hs.get_db_pool().prepare() + + self.store = hs.get_datastore() self.frank = hs.parse_userid("@1234ABCD:test") self.bob = hs.parse_userid("@4567:test") self.alice = hs.parse_userid("@alice:remote") + yield self.store.create_profile(self.frank.localpart) + self.handler = hs.get_handlers().profile_handler # TODO(paul): Icky signal declarings.. booo @@ -74,22 +75,22 @@ class ProfileTestCase(unittest.TestCase): @defer.inlineCallbacks def test_get_my_name(self): - mocked_get = self.datastore.get_profile_displayname - mocked_get.return_value = defer.succeed("Frank") + yield self.store.set_profile_displayname( + self.frank.localpart, "Frank" + ) displayname = yield self.handler.get_displayname(self.frank) self.assertEquals("Frank", displayname) - mocked_get.assert_called_with("1234ABCD") @defer.inlineCallbacks def test_set_my_name(self): - mocked_set = self.datastore.set_profile_displayname - mocked_set.return_value = defer.succeed(()) - yield self.handler.set_displayname(self.frank, self.frank, "Frank Jr.") - mocked_set.assert_called_with("1234ABCD", "Frank Jr.") + self.assertEquals( + (yield self.store.get_profile_displayname(self.frank.localpart)), + "Frank Jr." + ) @defer.inlineCallbacks def test_set_my_name_noauth(self): @@ -114,32 +115,31 @@ class ProfileTestCase(unittest.TestCase): @defer.inlineCallbacks def test_incoming_fed_query(self): - mocked_get = self.datastore.get_profile_displayname - mocked_get.return_value = defer.succeed("Caroline") + yield self.store.create_profile("caroline") + yield self.store.set_profile_displayname("caroline", "Caroline") response = yield self.query_handlers["profile"]( {"user_id": "@caroline:test", "field": "displayname"} ) self.assertEquals({"displayname": "Caroline"}, response) - mocked_get.assert_called_with("caroline") @defer.inlineCallbacks def test_get_my_avatar(self): - mocked_get = self.datastore.get_profile_avatar_url - mocked_get.return_value = defer.succeed("http://my.server/me.png") + yield self.store.set_profile_avatar_url( + self.frank.localpart, "http://my.server/me.png" + ) avatar_url = yield self.handler.get_avatar_url(self.frank) self.assertEquals("http://my.server/me.png", avatar_url) - mocked_get.assert_called_with("1234ABCD") @defer.inlineCallbacks def test_set_my_avatar(self): - mocked_set = self.datastore.set_profile_avatar_url - mocked_set.return_value = defer.succeed(()) - yield self.handler.set_avatar_url(self.frank, self.frank, "http://my.server/pic.gif") - mocked_set.assert_called_with("1234ABCD", "http://my.server/pic.gif") + self.assertEquals( + (yield self.store.get_profile_avatar_url(self.frank.localpart)), + "http://my.server/pic.gif" + ) From d83202b938a67cc3120efb7b77e607d5584ad31c Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Thu, 11 Sep 2014 11:32:46 +0100 Subject: [PATCH 12/35] Added unit tests of DirectoryStore --- tests/storage/test_directory.py | 66 +++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 tests/storage/test_directory.py diff --git a/tests/storage/test_directory.py b/tests/storage/test_directory.py new file mode 100644 index 000000000..49c41700f --- /dev/null +++ b/tests/storage/test_directory.py @@ -0,0 +1,66 @@ +# -*- coding: utf-8 -*- +# Copyright 2014 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. + + +from twisted.trial import unittest +from twisted.internet import defer + +from synapse.server import HomeServer +from synapse.storage.directory import DirectoryStore + +from tests.utils import SQLiteMemoryDbPool + + +class DirectoryStoreTestCase(unittest.TestCase): + + @defer.inlineCallbacks + def setUp(self): + hs = HomeServer("test", + db_pool=SQLiteMemoryDbPool(), + ) + + yield hs.get_db_pool().prepare() + + self.store = DirectoryStore(hs) + + self.room = hs.parse_roomid("!abcde:test") + self.alias = hs.parse_roomalias("#my-room:test") + + @defer.inlineCallbacks + def test_room_to_alias(self): + yield self.store.create_room_alias_association( + room_alias=self.alias, + room_id=self.room.to_string(), + servers=["test"], + ) + + aliases = yield self.store.get_aliases_for_room(self.room.to_string()) + + self.assertEquals(["#my-room:test"], aliases) + + @defer.inlineCallbacks + def test_alias_to_room(self): + yield self.store.create_room_alias_association( + room_alias=self.alias, + room_id=self.room.to_string(), + servers=["test"], + ) + + mapping = yield self.store.get_association_from_room_alias( + self.alias + ) + + self.assertEquals(self.room.to_string(), mapping.room_id) + self.assertEquals(["test"], mapping.servers) From d13d0bba511dfc5c688f8971a6cd91d52bb7e769 Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Thu, 11 Sep 2014 11:59:48 +0100 Subject: [PATCH 13/35] Unit-test DirectoryHandler against (real) SQLite memory store, not mocked storage layer --- tests/handlers/test_directory.py | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/tests/handlers/test_directory.py b/tests/handlers/test_directory.py index 72a2b1443..58ecf561f 100644 --- a/tests/handlers/test_directory.py +++ b/tests/handlers/test_directory.py @@ -25,6 +25,8 @@ from synapse.http.client import HttpClient from synapse.handlers.directory import DirectoryHandler from synapse.storage.directory import RoomAliasMapping +from tests.utils import SQLiteMemoryDbPool + logging.getLogger().addHandler(logging.NullHandler()) @@ -37,6 +39,7 @@ class DirectoryHandlers(object): class DirectoryTestCase(unittest.TestCase): """ Tests the directory service. """ + @defer.inlineCallbacks def setUp(self): self.mock_federation = Mock(spec=[ "make_query", @@ -48,32 +51,27 @@ class DirectoryTestCase(unittest.TestCase): self.mock_federation.register_query_handler = register_query_handler hs = HomeServer("test", - datastore=Mock(spec=[ - "get_association_from_room_alias", - "get_joined_hosts_for_room", - ]), + db_pool=SQLiteMemoryDbPool(), http_client=None, resource_for_federation=Mock(), replication_layer=self.mock_federation, ) hs.handlers = DirectoryHandlers(hs) + yield hs.get_db_pool().prepare() + self.handler = hs.get_handlers().directory_handler - self.datastore = hs.get_datastore() - - def hosts(room_id): - return defer.succeed([]) - self.datastore.get_joined_hosts_for_room.side_effect = hosts + self.store = hs.get_datastore() self.my_room = hs.parse_roomalias("#my-room:test") + self.your_room = hs.parse_roomalias("#your-room:test") self.remote_room = hs.parse_roomalias("#another:remote") @defer.inlineCallbacks def test_get_local_association(self): - mocked_get = self.datastore.get_association_from_room_alias - mocked_get.return_value = defer.succeed( - RoomAliasMapping("!8765qwer:test", "#my-room:test", ["test"]) + yield self.store.create_room_alias_association( + self.my_room, "!8765qwer:test", ["test"] ) result = yield self.handler.get_association(self.my_room) @@ -106,9 +104,8 @@ class DirectoryTestCase(unittest.TestCase): @defer.inlineCallbacks def test_incoming_fed_query(self): - mocked_get = self.datastore.get_association_from_room_alias - mocked_get.return_value = defer.succeed( - RoomAliasMapping("!8765asdf:test", "#your-room:test", ["test"]) + yield self.store.create_room_alias_association( + self.your_room, "!8765asdf:test", ["test"] ) response = yield self.query_handlers["directory"]( From 4385eadc283abd697abbfe5d4a7a2008364dc646 Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Thu, 11 Sep 2014 13:57:17 +0100 Subject: [PATCH 14/35] Start of converting PresenceHandler unit tests to use SQLiteMemoryDbPool - just the 'State' test case for now --- tests/handlers/test_presence.py | 73 ++++++++++++--------------------- 1 file changed, 27 insertions(+), 46 deletions(-) diff --git a/tests/handlers/test_presence.py b/tests/handlers/test_presence.py index 9eb8b6909..c98f877e8 100644 --- a/tests/handlers/test_presence.py +++ b/tests/handlers/test_presence.py @@ -21,7 +21,9 @@ from mock import Mock, call, ANY import logging import json -from ..utils import MockHttpResource, MockClock, DeferredMockCallable +from tests.utils import ( + MockHttpResource, MockClock, DeferredMockCallable, SQLiteMemoryDbPool +) from synapse.server import HomeServer from synapse.api.constants import PresenceState @@ -64,30 +66,20 @@ class JustPresenceHandlers(object): class PresenceStateTestCase(unittest.TestCase): """ Tests presence management. """ + @defer.inlineCallbacks def setUp(self): hs = HomeServer("test", - clock=MockClock(), - db_pool=None, - datastore=Mock(spec=[ - "get_presence_state", - "set_presence_state", - "add_presence_list_pending", - "set_presence_list_accepted", - ]), - handlers=None, - resource_for_federation=Mock(), - http_client=None, - ) + clock=MockClock(), + db_pool=SQLiteMemoryDbPool(), + handlers=None, + resource_for_federation=Mock(), + http_client=None, + ) hs.handlers = JustPresenceHandlers(hs) - self.datastore = hs.get_datastore() + yield hs.get_db_pool().prepare() - def is_presence_visible(observed_localpart, observer_userid): - allow = (observed_localpart == "apple" and - observer_userid == "@banana:test" - ) - return defer.succeed(allow) - self.datastore.is_presence_visible = is_presence_visible + self.store = hs.get_datastore() # Mock the RoomMemberHandler room_member_handler = Mock(spec=[]) @@ -99,6 +91,11 @@ class PresenceStateTestCase(unittest.TestCase): self.u_banana = hs.parse_userid("@banana:test") self.u_clementine = hs.parse_userid("@clementine:test") + yield self.store.create_presence(self.u_apple.localpart) + yield self.store.set_presence_state( + self.u_apple.localpart, {"state": ONLINE, "status_msg": "Online"} + ) + self.handler = hs.get_handlers().presence_handler self.room_members = [] @@ -122,7 +119,7 @@ class PresenceStateTestCase(unittest.TestCase): shared = all(map(lambda i: i in room_member_ids, userlist)) return defer.succeed(shared) - self.datastore.user_rooms_intersect = user_rooms_intersect + self.store.user_rooms_intersect = user_rooms_intersect self.mock_start = Mock() self.mock_stop = Mock() @@ -132,11 +129,6 @@ class PresenceStateTestCase(unittest.TestCase): @defer.inlineCallbacks def test_get_my_state(self): - mocked_get = self.datastore.get_presence_state - mocked_get.return_value = defer.succeed( - {"state": ONLINE, "status_msg": "Online"} - ) - state = yield self.handler.get_state( target_user=self.u_apple, auth_user=self.u_apple ) @@ -145,13 +137,12 @@ class PresenceStateTestCase(unittest.TestCase): {"presence": ONLINE, "status_msg": "Online"}, state ) - mocked_get.assert_called_with("apple") @defer.inlineCallbacks def test_get_allowed_state(self): - mocked_get = self.datastore.get_presence_state - mocked_get.return_value = defer.succeed( - {"state": ONLINE, "status_msg": "Online"} + yield self.store.allow_presence_visible( + observed_localpart=self.u_apple.localpart, + observer_userid=self.u_banana.to_string(), ) state = yield self.handler.get_state( @@ -162,15 +153,9 @@ class PresenceStateTestCase(unittest.TestCase): {"presence": ONLINE, "status_msg": "Online"}, state ) - mocked_get.assert_called_with("apple") @defer.inlineCallbacks def test_get_same_room_state(self): - mocked_get = self.datastore.get_presence_state - mocked_get.return_value = defer.succeed( - {"state": ONLINE, "status_msg": "Online"} - ) - self.room_members = [self.u_apple, self.u_clementine] state = yield self.handler.get_state( @@ -184,11 +169,6 @@ class PresenceStateTestCase(unittest.TestCase): @defer.inlineCallbacks def test_get_disallowed_state(self): - mocked_get = self.datastore.get_presence_state - mocked_get.return_value = defer.succeed( - {"state": ONLINE, "status_msg": "Online"} - ) - self.room_members = [] yield self.assertFailure( @@ -200,16 +180,17 @@ class PresenceStateTestCase(unittest.TestCase): @defer.inlineCallbacks def test_set_my_state(self): - mocked_set = self.datastore.set_presence_state - mocked_set.return_value = defer.succeed({"state": OFFLINE}) - yield self.handler.set_state( target_user=self.u_apple, auth_user=self.u_apple, state={"presence": UNAVAILABLE, "status_msg": "Away"}) - mocked_set.assert_called_with("apple", - {"state": UNAVAILABLE, "status_msg": "Away"} + self.assertEquals( + {"state": UNAVAILABLE, + "status_msg": "Away", + "mtime": 1000000}, + (yield self.store.get_presence_state(self.u_apple.localpart)) ) + self.mock_start.assert_called_with(self.u_apple, state={ "presence": UNAVAILABLE, From 493b1e6d3ccb9fd806e6d4c22daa1b6657c6ae7f Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Thu, 11 Sep 2014 15:21:15 +0100 Subject: [PATCH 15/35] Need to prepare() the SQLiteMemoryDbPool before passing it to HomeServer constructor, as DataStore's constructor will want it ready --- tests/handlers/test_directory.py | 7 ++++--- tests/handlers/test_presence.py | 7 ++++--- tests/handlers/test_profile.py | 7 ++++--- tests/storage/test_directory.py | 9 +++++---- tests/storage/test_presence.py | 7 ++++--- tests/storage/test_profile.py | 9 +++++---- 6 files changed, 26 insertions(+), 20 deletions(-) diff --git a/tests/handlers/test_directory.py b/tests/handlers/test_directory.py index 58ecf561f..f7eb7b4f0 100644 --- a/tests/handlers/test_directory.py +++ b/tests/handlers/test_directory.py @@ -50,16 +50,17 @@ class DirectoryTestCase(unittest.TestCase): self.query_handlers[query_type] = handler self.mock_federation.register_query_handler = register_query_handler + db_pool = SQLiteMemoryDbPool() + yield db_pool.prepare() + hs = HomeServer("test", - db_pool=SQLiteMemoryDbPool(), + db_pool=db_pool, http_client=None, resource_for_federation=Mock(), replication_layer=self.mock_federation, ) hs.handlers = DirectoryHandlers(hs) - yield hs.get_db_pool().prepare() - self.handler = hs.get_handlers().directory_handler self.store = hs.get_datastore() diff --git a/tests/handlers/test_presence.py b/tests/handlers/test_presence.py index c98f877e8..61e9cc704 100644 --- a/tests/handlers/test_presence.py +++ b/tests/handlers/test_presence.py @@ -68,17 +68,18 @@ class PresenceStateTestCase(unittest.TestCase): @defer.inlineCallbacks def setUp(self): + db_pool = SQLiteMemoryDbPool() + yield db_pool.prepare() + hs = HomeServer("test", clock=MockClock(), - db_pool=SQLiteMemoryDbPool(), + db_pool=db_pool, handlers=None, resource_for_federation=Mock(), http_client=None, ) hs.handlers = JustPresenceHandlers(hs) - yield hs.get_db_pool().prepare() - self.store = hs.get_datastore() # Mock the RoomMemberHandler diff --git a/tests/handlers/test_profile.py b/tests/handlers/test_profile.py index 8b9685a52..63c929594 100644 --- a/tests/handlers/test_profile.py +++ b/tests/handlers/test_profile.py @@ -49,8 +49,11 @@ class ProfileTestCase(unittest.TestCase): self.query_handlers[query_type] = handler self.mock_federation.register_query_handler = register_query_handler + db_pool = SQLiteMemoryDbPool() + yield db_pool.prepare() + hs = HomeServer("test", - db_pool=SQLiteMemoryDbPool(), + db_pool=db_pool, http_client=None, handlers=None, resource_for_federation=Mock(), @@ -58,8 +61,6 @@ class ProfileTestCase(unittest.TestCase): ) hs.handlers = ProfileHandlers(hs) - yield hs.get_db_pool().prepare() - self.store = hs.get_datastore() self.frank = hs.parse_userid("@1234ABCD:test") diff --git a/tests/storage/test_directory.py b/tests/storage/test_directory.py index 49c41700f..c4c321dda 100644 --- a/tests/storage/test_directory.py +++ b/tests/storage/test_directory.py @@ -27,11 +27,12 @@ class DirectoryStoreTestCase(unittest.TestCase): @defer.inlineCallbacks def setUp(self): - hs = HomeServer("test", - db_pool=SQLiteMemoryDbPool(), - ) + db_pool = SQLiteMemoryDbPool() + yield db_pool.prepare() - yield hs.get_db_pool().prepare() + hs = HomeServer("test", + db_pool=db_pool, + ) self.store = DirectoryStore(hs) diff --git a/tests/storage/test_presence.py b/tests/storage/test_presence.py index f0a04ae83..f3fab4fe7 100644 --- a/tests/storage/test_presence.py +++ b/tests/storage/test_presence.py @@ -27,13 +27,14 @@ class PresenceStoreTestCase(unittest.TestCase): @defer.inlineCallbacks def setUp(self): + db_pool = SQLiteMemoryDbPool() + yield db_pool.prepare() + hs = HomeServer("test", clock=MockClock(), - db_pool=SQLiteMemoryDbPool(), + db_pool=db_pool, ) - yield hs.get_db_pool().prepare() - self.store = PresenceStore(hs) self.u_apple = hs.parse_userid("@apple:test") diff --git a/tests/storage/test_profile.py b/tests/storage/test_profile.py index fa96abf03..185527804 100644 --- a/tests/storage/test_profile.py +++ b/tests/storage/test_profile.py @@ -27,11 +27,12 @@ class ProfileStoreTestCase(unittest.TestCase): @defer.inlineCallbacks def setUp(self): - hs = HomeServer("test", - db_pool=SQLiteMemoryDbPool(), - ) + db_pool = SQLiteMemoryDbPool() + yield db_pool.prepare() - yield hs.get_db_pool().prepare() + hs = HomeServer("test", + db_pool=db_pool, + ) self.store = ProfileStore(hs) From fb93a4a9e319d4cf553a6819448cbd70a055e8ee Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Thu, 11 Sep 2014 16:22:44 +0100 Subject: [PATCH 16/35] Perform PresenceInvitesTestCase against real SQLiteMemoryDbPool --- tests/handlers/test_presence.py | 180 ++++++++++++++++++-------------- 1 file changed, 100 insertions(+), 80 deletions(-) diff --git a/tests/handlers/test_presence.py b/tests/handlers/test_presence.py index 61e9cc704..d57ee563c 100644 --- a/tests/handlers/test_presence.py +++ b/tests/handlers/test_presence.py @@ -209,50 +209,34 @@ class PresenceStateTestCase(unittest.TestCase): class PresenceInvitesTestCase(unittest.TestCase): """ Tests presence management. """ + @defer.inlineCallbacks def setUp(self): self.mock_http_client = Mock(spec=[]) self.mock_http_client.put_json = DeferredMockCallable() self.mock_federation_resource = MockHttpResource() - hs = HomeServer("test", - clock=MockClock(), - db_pool=None, - datastore=Mock(spec=[ - "has_presence_state", - "allow_presence_visible", - "add_presence_list_pending", - "set_presence_list_accepted", - "get_presence_list", - "del_presence_list", + db_pool = SQLiteMemoryDbPool() + yield db_pool.prepare() - # Bits that Federation needs - "prep_send_transaction", - "delivered_txn", - "get_received_txn_response", - "set_received_txn_response", - ]), - handlers=None, - resource_for_client=Mock(), - resource_for_federation=self.mock_federation_resource, - http_client=self.mock_http_client, - ) + hs = HomeServer("test", + clock=MockClock(), + db_pool=db_pool, + handlers=None, + resource_for_client=Mock(), + resource_for_federation=self.mock_federation_resource, + http_client=self.mock_http_client, + ) 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_received_txn_response(*args): - return defer.succeed(None) - self.datastore.get_received_txn_response = get_received_txn_response + self.store = hs.get_datastore() # Some local users to test with self.u_apple = hs.parse_userid("@apple:test") self.u_banana = hs.parse_userid("@banana:test") + yield self.store.create_presence(self.u_apple.localpart) + yield self.store.create_presence(self.u_banana.localpart) + # ID of a local user that does not exist self.u_durian = hs.parse_userid("@durian:test") @@ -275,12 +259,16 @@ class PresenceInvitesTestCase(unittest.TestCase): yield self.handler.send_invite( observer_user=self.u_apple, observed_user=self.u_banana) - self.datastore.add_presence_list_pending.assert_called_with( - "apple", "@banana:test") - self.datastore.allow_presence_visible.assert_called_with( - "banana", "@apple:test") - self.datastore.set_presence_list_accepted.assert_called_with( - "apple", "@banana:test") + self.assertEquals( + [{"observed_user_id": "@banana:test", "accepted": 1}], + (yield self.store.get_presence_list(self.u_apple.localpart)) + ) + self.assertTrue( + (yield self.store.is_presence_visible( + observed_localpart=self.u_banana.localpart, + observer_userid=self.u_apple.to_string(), + )) + ) self.mock_start.assert_called_with( self.u_apple, target_user=self.u_banana) @@ -290,10 +278,10 @@ class PresenceInvitesTestCase(unittest.TestCase): yield self.handler.send_invite( observer_user=self.u_apple, observed_user=self.u_durian) - self.datastore.add_presence_list_pending.assert_called_with( - "apple", "@durian:test") - self.datastore.del_presence_list.assert_called_with( - "apple", "@durian:test") + self.assertEquals( + [], + (yield self.store.get_presence_list(self.u_apple.localpart)) + ) @defer.inlineCallbacks def test_invite_remote(self): @@ -314,8 +302,10 @@ class PresenceInvitesTestCase(unittest.TestCase): yield self.handler.send_invite( observer_user=self.u_apple, observed_user=self.u_cabbage) - self.datastore.add_presence_list_pending.assert_called_with( - "apple", "@cabbage:elsewhere") + self.assertEquals( + [{"observed_user_id": "@cabbage:elsewhere", "accepted": 0}], + (yield self.store.get_presence_list(self.u_apple.localpart)) + ) yield put_json.await_calls() @@ -347,8 +337,12 @@ class PresenceInvitesTestCase(unittest.TestCase): ) ) - self.datastore.allow_presence_visible.assert_called_with( - "apple", "@cabbage:elsewhere") + self.assertTrue( + (yield self.store.is_presence_visible( + observed_localpart=self.u_apple.localpart, + observer_userid=self.u_cabbage.to_string(), + )) + ) yield put_json.await_calls() @@ -382,6 +376,11 @@ class PresenceInvitesTestCase(unittest.TestCase): @defer.inlineCallbacks def test_accepted_remote(self): + yield self.store.add_presence_list_pending( + observer_localpart=self.u_apple.localpart, + observed_userid=self.u_cabbage.to_string(), + ) + yield self.mock_federation_resource.trigger("PUT", "/_matrix/federation/v1/send/1000000/", _make_edu_json("elsewhere", "m.presence_accept", @@ -392,14 +391,21 @@ class PresenceInvitesTestCase(unittest.TestCase): ) ) - self.datastore.set_presence_list_accepted.assert_called_with( - "apple", "@cabbage:elsewhere") + self.assertEquals( + [{"observed_user_id": "@cabbage:elsewhere", "accepted": 1}], + (yield self.store.get_presence_list(self.u_apple.localpart)) + ) self.mock_start.assert_called_with( self.u_apple, target_user=self.u_cabbage) @defer.inlineCallbacks def test_denied_remote(self): + yield self.store.add_presence_list_pending( + observer_localpart=self.u_apple.localpart, + observed_userid="@eggplant:elsewhere", + ) + yield self.mock_federation_resource.trigger("PUT", "/_matrix/federation/v1/send/1000000/", _make_edu_json("elsewhere", "m.presence_deny", @@ -410,32 +416,65 @@ class PresenceInvitesTestCase(unittest.TestCase): ) ) - self.datastore.del_presence_list.assert_called_with( - "apple", "@eggplant:elsewhere") + self.assertEquals( + [], + (yield self.store.get_presence_list(self.u_apple.localpart)) + ) @defer.inlineCallbacks def test_drop_local(self): - yield self.handler.drop( - observer_user=self.u_apple, observed_user=self.u_banana) + yield self.store.add_presence_list_pending( + observer_localpart=self.u_apple.localpart, + observed_userid=self.u_banana.to_string(), + ) + yield self.store.set_presence_list_accepted( + observer_localpart=self.u_apple.localpart, + observed_userid=self.u_banana.to_string(), + ) - self.datastore.del_presence_list.assert_called_with( - "apple", "@banana:test") + yield self.handler.drop( + observer_user=self.u_apple, + observed_user=self.u_banana, + ) + + self.assertEquals( + [], + (yield self.store.get_presence_list(self.u_apple.localpart)) + ) 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) + yield self.store.add_presence_list_pending( + observer_localpart=self.u_apple.localpart, + observed_userid=self.u_cabbage.to_string(), + ) + yield self.store.set_presence_list_accepted( + observer_localpart=self.u_apple.localpart, + observed_userid=self.u_cabbage.to_string(), + ) - self.datastore.del_presence_list.assert_called_with( - "apple", "@cabbage:elsewhere") + yield self.handler.drop( + observer_user=self.u_apple, + observed_user=self.u_cabbage, + ) + + self.assertEquals( + [], + (yield self.store.get_presence_list(self.u_apple.localpart)) + ) @defer.inlineCallbacks def test_get_presence_list(self): - self.datastore.get_presence_list.return_value = defer.succeed( - [{"observed_user_id": "@banana:test"}] + yield self.store.add_presence_list_pending( + observer_localpart=self.u_apple.localpart, + observed_userid=self.u_banana.to_string(), + ) + yield self.store.set_presence_list_accepted( + observer_localpart=self.u_apple.localpart, + observed_userid=self.u_banana.to_string(), ) presence = yield self.handler.get_presence_list( @@ -443,29 +482,10 @@ class PresenceInvitesTestCase(unittest.TestCase): self.assertEquals([ {"observed_user": self.u_banana, - "presence": OFFLINE}, + "presence": OFFLINE, + "accepted": 1}, ], presence) - self.datastore.get_presence_list.assert_called_with("apple", - accepted=None - ) - - self.datastore.get_presence_list.return_value = defer.succeed( - [{"observed_user_id": "@banana:test"}] - ) - - presence = yield self.handler.get_presence_list( - observer_user=self.u_apple, accepted=True - ) - - self.assertEquals([ - {"observed_user": self.u_banana, - "presence": OFFLINE}, - ], presence) - - self.datastore.get_presence_list.assert_called_with("apple", - accepted=True) - class PresencePushTestCase(unittest.TestCase): """ Tests steady-state presence status updates. From 3d6aee079ef7cb44a7c5a7ea4f13f67a6850128a Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Thu, 11 Sep 2014 17:44:00 +0100 Subject: [PATCH 17/35] Unit-test for RegistrationStore using SQLiteMemoryDbPool --- tests/storage/test_registration.py | 69 ++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 tests/storage/test_registration.py diff --git a/tests/storage/test_registration.py b/tests/storage/test_registration.py new file mode 100644 index 000000000..5e11ce351 --- /dev/null +++ b/tests/storage/test_registration.py @@ -0,0 +1,69 @@ +# -*- coding: utf-8 -*- +# Copyright 2014 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. + + +from twisted.trial import unittest +from twisted.internet import defer + +from synapse.server import HomeServer +from synapse.storage.registration import RegistrationStore + +from tests.utils import SQLiteMemoryDbPool + + +class RegistrationStoreTestCase(unittest.TestCase): + + @defer.inlineCallbacks + def setUp(self): + db_pool = SQLiteMemoryDbPool() + yield db_pool.prepare() + + hs = HomeServer("test", + db_pool=db_pool, + ) + + self.store = RegistrationStore(hs) + + self.user_id = "@my-user:test" + self.tokens = ["AbCdEfGhIjKlMnOpQrStUvWxYz", + "BcDeFgHiJkLmNoPqRsTuVwXyZa"] + self.pwhash = "{xx1}123456789" + + @defer.inlineCallbacks + def test_register(self): + yield self.store.register(self.user_id, self.tokens[0], self.pwhash) + + self.assertEquals( + # TODO(paul): Surely this field should be 'user_id', not 'name' + # Additionally surely it shouldn't come in a 1-element list + [{"name": self.user_id, "password_hash": self.pwhash}], + (yield self.store.get_user_by_id(self.user_id)) + ) + + self.assertEquals( + self.user_id, + (yield self.store.get_user_by_token(self.tokens[0])) + ) + + @defer.inlineCallbacks + def test_add_tokens(self): + yield self.store.register(self.user_id, self.tokens[0], self.pwhash) + yield self.store.add_access_token_to_user(self.user_id, self.tokens[1]) + + self.assertEquals( + self.user_id, + (yield self.store.get_user_by_token(self.tokens[1])) + ) + From aaf9ab68c6969d69150b7aa1f6ebddcf1c496050 Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Thu, 11 Sep 2014 18:44:04 +0100 Subject: [PATCH 18/35] Rename _store_room_member_txn to _store_room_member_from_event_txn so we can create another, more sensible function of that name --- synapse/storage/__init__.py | 2 +- synapse/storage/roommember.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/storage/__init__.py b/synapse/storage/__init__.py index 6b273a030..822806927 100644 --- a/synapse/storage/__init__.py +++ b/synapse/storage/__init__.py @@ -154,7 +154,7 @@ class DataStore(RoomMemberStore, RoomStore, @log_function def _persist_event_txn(self, txn, event, backfilled, stream_ordering=None): if event.type == RoomMemberEvent.TYPE: - self._store_room_member_txn(txn, event) + self._store_room_member_from_event_txn(txn, event) elif event.type == FeedbackEvent.TYPE: self._store_feedback_txn(txn, event) elif event.type == RoomNameEvent.TYPE: diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py index 9a393e256..437ff03a7 100644 --- a/synapse/storage/roommember.py +++ b/synapse/storage/roommember.py @@ -26,7 +26,7 @@ logger = logging.getLogger(__name__) class RoomMemberStore(SQLBaseStore): - def _store_room_member_txn(self, txn, event): + def _store_room_member_from_event_txn(self, txn, event): """Store a room member in the database. """ target_user_id = event.state_key From 249e8f227799c2b5f1adcd17a471ff9773b43f14 Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Thu, 11 Sep 2014 18:52:35 +0100 Subject: [PATCH 19/35] Add a better _store_room_member_txn() method that takes separated fields instead of an event object; also add FIXME comment about a big bug in the logic --- synapse/storage/roommember.py | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py index 437ff03a7..b357dc305 100644 --- a/synapse/storage/roommember.py +++ b/synapse/storage/roommember.py @@ -27,36 +27,49 @@ logger = logging.getLogger(__name__) class RoomMemberStore(SQLBaseStore): def _store_room_member_from_event_txn(self, txn, event): + self._store_room_member_txn(txn, + target_user_id=event.state_key, + sender_user_id=event.user_id, + room_id=event.room_id, + event_id=event.event_id, + membership=event.membership, + ) + + def _store_room_member_txn(self, txn, target_user_id, sender_user_id, + room_id, event_id, membership): """Store a room member in the database. """ - target_user_id = event.state_key domain = self.hs.parse_userid(target_user_id).domain self._simple_insert_txn( txn, "room_memberships", { - "event_id": event.event_id, + "event_id": event_id, "user_id": target_user_id, - "sender": event.user_id, - "room_id": event.room_id, - "membership": event.membership, + "sender": sender_user_id, + "room_id": room_id, + "membership": membership, } ) # Update room hosts table - if event.membership == Membership.JOIN: + # TODO(paul): This code is massively broken currently as it doesn't + # count users per room - meaning it'll delete on the FIRST user to + # have a membership other than JOIN - say, LEAVE, or even INVITE. + # FIXME + if membership == Membership.JOIN: sql = ( "INSERT OR IGNORE INTO room_hosts (room_id, host) " "VALUES (?, ?)" ) - txn.execute(sql, (event.room_id, domain)) + txn.execute(sql, (room_id, domain)) else: sql = ( "DELETE FROM room_hosts WHERE room_id = ? AND host = ?" ) - txn.execute(sql, (event.room_id, domain)) + txn.execute(sql, (room_id, domain)) @defer.inlineCallbacks def get_room_member(self, user_id, room_id): From e53d77b5017e823506484bbb95964b4d97f3e2a1 Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Fri, 12 Sep 2014 13:57:24 +0100 Subject: [PATCH 20/35] Add a .runInteraction() method on SQLBaseStore itself to wrap the .db_pool --- synapse/storage/__init__.py | 4 ++-- synapse/storage/_base.py | 20 ++++++++++++-------- synapse/storage/pdu.py | 24 ++++++++++++------------ synapse/storage/registration.py | 4 ++-- synapse/storage/room.py | 4 ++-- synapse/storage/roommember.py | 5 +++++ synapse/storage/stream.py | 2 +- synapse/storage/transactions.py | 12 ++++++------ 8 files changed, 42 insertions(+), 33 deletions(-) diff --git a/synapse/storage/__init__.py b/synapse/storage/__init__.py index 822806927..629c110be 100644 --- a/synapse/storage/__init__.py +++ b/synapse/storage/__init__.py @@ -94,7 +94,7 @@ class DataStore(RoomMemberStore, RoomStore, stream_ordering = self.min_token try: - yield self._db_pool.runInteraction( + yield self.runInteraction( self._persist_pdu_event_txn, pdu=pdu, event=event, @@ -297,7 +297,7 @@ class DataStore(RoomMemberStore, RoomStore, prev_state_pdu=prev_state_pdu, ) - return self._db_pool.runInteraction(_snapshot) + return self.runInteraction(_snapshot) class Snapshot(object): diff --git a/synapse/storage/_base.py b/synapse/storage/_base.py index 803722507..8a36f0bc6 100644 --- a/synapse/storage/_base.py +++ b/synapse/storage/_base.py @@ -34,6 +34,10 @@ class SQLBaseStore(object): self.event_factory = hs.get_event_factory() self._clock = hs.get_clock() + def runInteraction(self, txn, *args, **kwargs): + """Wraps the .runInteraction() method on the underlying db_pool.""" + return self._db_pool.runInteraction(txn, *args, **kwargs) + def cursor_to_dict(self, cursor): """Converts a SQL cursor into an list of dicts. @@ -71,7 +75,7 @@ class SQLBaseStore(object): else: return cursor.fetchall() - return self._db_pool.runInteraction(interaction) + return self.runInteraction(interaction) def _execute_and_decode(self, query, *args): return self._execute(self.cursor_to_dict, query, *args) @@ -87,7 +91,7 @@ class SQLBaseStore(object): values : dict of new column names and values for them or_replace : bool; if True performs an INSERT OR REPLACE """ - return self._db_pool.runInteraction( + return self.runInteraction( self._simple_insert_txn, table, values, or_replace=or_replace ) @@ -164,7 +168,7 @@ class SQLBaseStore(object): txn.execute(sql, keyvalues.values()) return txn.fetchall() - res = yield self._db_pool.runInteraction(func) + res = yield self.runInteraction(func) defer.returnValue([r[0] for r in res]) @@ -187,7 +191,7 @@ class SQLBaseStore(object): txn.execute(sql, keyvalues.values()) return self.cursor_to_dict(txn) - return self._db_pool.runInteraction(func) + return self.runInteraction(func) def _simple_update_one(self, table, keyvalues, updatevalues, retcols=None): @@ -255,7 +259,7 @@ class SQLBaseStore(object): raise StoreError(500, "More than one row matched") return ret - return self._db_pool.runInteraction(func) + return self.runInteraction(func) def _simple_delete_one(self, table, keyvalues): """Executes a DELETE query on the named table, expecting to delete a @@ -276,7 +280,7 @@ class SQLBaseStore(object): raise StoreError(404, "No row found") if txn.rowcount > 1: raise StoreError(500, "more than one row matched") - return self._db_pool.runInteraction(func) + return self.runInteraction(func) def _simple_max_id(self, table): """Executes a SELECT query on the named table, expecting to return the @@ -294,7 +298,7 @@ class SQLBaseStore(object): return 0 return max_id - return self._db_pool.runInteraction(func) + return self.runInteraction(func) def _parse_event_from_row(self, row_dict): d = copy.deepcopy({k: v for k, v in row_dict.items() if v}) @@ -313,7 +317,7 @@ class SQLBaseStore(object): ) def _parse_events(self, rows): - return self._db_pool.runInteraction(self._parse_events_txn, rows) + return self.runInteraction(self._parse_events_txn, rows) def _parse_events_txn(self, txn, rows): events = [self._parse_event_from_row(r) for r in rows] diff --git a/synapse/storage/pdu.py b/synapse/storage/pdu.py index 3cbce2d0a..f770a82bc 100644 --- a/synapse/storage/pdu.py +++ b/synapse/storage/pdu.py @@ -42,7 +42,7 @@ class PduStore(SQLBaseStore): PduTuple: If the pdu does not exist in the database, returns None """ - return self._db_pool.runInteraction( + return self.runInteraction( self._get_pdu_tuple, pdu_id, origin ) @@ -94,7 +94,7 @@ class PduStore(SQLBaseStore): list: A list of PduTuples """ - return self._db_pool.runInteraction( + return self.runInteraction( self._get_current_state_for_context, context ) @@ -142,7 +142,7 @@ class PduStore(SQLBaseStore): pdu_origin (str) """ - return self._db_pool.runInteraction( + return self.runInteraction( self._mark_as_processed, pdu_id, pdu_origin ) @@ -151,7 +151,7 @@ class PduStore(SQLBaseStore): def get_all_pdus_from_context(self, context): """Get a list of all PDUs for a given context.""" - return self._db_pool.runInteraction( + return self.runInteraction( self._get_all_pdus_from_context, context, ) @@ -178,7 +178,7 @@ class PduStore(SQLBaseStore): Return: list: A list of PduTuples """ - return self._db_pool.runInteraction( + return self.runInteraction( self._get_backfill, context, pdu_list, limit ) @@ -239,7 +239,7 @@ class PduStore(SQLBaseStore): txn context (str) """ - return self._db_pool.runInteraction( + return self.runInteraction( self._get_min_depth_for_context, context ) @@ -345,7 +345,7 @@ class PduStore(SQLBaseStore): bool """ - return self._db_pool.runInteraction( + return self.runInteraction( self._is_pdu_new, pdu_id=pdu_id, origin=origin, @@ -498,7 +498,7 @@ class StatePduStore(SQLBaseStore): ) def get_unresolved_state_tree(self, new_state_pdu): - return self._db_pool.runInteraction( + return self.runInteraction( self._get_unresolved_state_tree, new_state_pdu ) @@ -537,7 +537,7 @@ class StatePduStore(SQLBaseStore): def update_current_state(self, pdu_id, origin, context, pdu_type, state_key): - return self._db_pool.runInteraction( + return self.runInteraction( self._update_current_state, pdu_id, origin, context, pdu_type, state_key ) @@ -576,7 +576,7 @@ class StatePduStore(SQLBaseStore): PduEntry """ - return self._db_pool.runInteraction( + return self.runInteraction( self._get_current_state_pdu, context, pdu_type, state_key ) @@ -638,7 +638,7 @@ class StatePduStore(SQLBaseStore): PduIdTuple: A pdu that we are missing, or None if we have all the pdus required to do the conflict resolution. """ - return self._db_pool.runInteraction( + return self.runInteraction( self._get_next_missing_pdu, new_pdu ) @@ -682,7 +682,7 @@ class StatePduStore(SQLBaseStore): Returns: bool: True if the new_pdu clobbered the current state, False if not """ - return self._db_pool.runInteraction( + return self.runInteraction( self._handle_new_state, new_pdu ) diff --git a/synapse/storage/registration.py b/synapse/storage/registration.py index fd762bc64..db20b1daa 100644 --- a/synapse/storage/registration.py +++ b/synapse/storage/registration.py @@ -62,7 +62,7 @@ class RegistrationStore(SQLBaseStore): Raises: StoreError if the user_id could not be registered. """ - yield self._db_pool.runInteraction(self._register, user_id, token, + yield self.runInteraction(self._register, user_id, token, password_hash) def _register(self, txn, user_id, token, password_hash): @@ -99,7 +99,7 @@ class RegistrationStore(SQLBaseStore): Raises: StoreError if no user was found. """ - user_id = yield self._db_pool.runInteraction(self._query_for_auth, + user_id = yield self.runInteraction(self._query_for_auth, token) defer.returnValue(user_id) diff --git a/synapse/storage/room.py b/synapse/storage/room.py index 017169ce0..5adf8cdf1 100644 --- a/synapse/storage/room.py +++ b/synapse/storage/room.py @@ -149,7 +149,7 @@ class RoomStore(SQLBaseStore): defer.returnValue(None) def get_power_level(self, room_id, user_id): - return self._db_pool.runInteraction( + return self.runInteraction( self._get_power_level, room_id, user_id, ) @@ -182,7 +182,7 @@ class RoomStore(SQLBaseStore): return None def get_ops_levels(self, room_id): - return self._db_pool.runInteraction( + return self.runInteraction( self._get_ops_levels, room_id, ) diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py index b357dc305..8cbc15356 100644 --- a/synapse/storage/roommember.py +++ b/synapse/storage/roommember.py @@ -71,6 +71,11 @@ class RoomMemberStore(SQLBaseStore): txn.execute(sql, (room_id, domain)) + def store_room_member(self, user_id, room_id, event_id, membership): + return self.runInteraction(self._store_room_member_txn, + user_id, user_id, room_id, event_id, membership + ) + @defer.inlineCallbacks def get_room_member(self, user_id, room_id): """Retrieve the current state of a room member. diff --git a/synapse/storage/stream.py b/synapse/storage/stream.py index aff6dc985..8c766b8a0 100644 --- a/synapse/storage/stream.py +++ b/synapse/storage/stream.py @@ -286,7 +286,7 @@ class StreamStore(SQLBaseStore): defer.returnValue(ret) def get_room_events_max_id(self): - return self._db_pool.runInteraction(self._get_room_events_max_id_txn) + return self.runInteraction(self._get_room_events_max_id_txn) def _get_room_events_max_id_txn(self, txn): txn.execute( diff --git a/synapse/storage/transactions.py b/synapse/storage/transactions.py index 7467e1035..ab4599b46 100644 --- a/synapse/storage/transactions.py +++ b/synapse/storage/transactions.py @@ -41,7 +41,7 @@ class TransactionStore(SQLBaseStore): this transaction or a 2-tuple of (int, dict) """ - return self._db_pool.runInteraction( + return self.runInteraction( self._get_received_txn_response, transaction_id, origin ) @@ -72,7 +72,7 @@ class TransactionStore(SQLBaseStore): response_json (str) """ - return self._db_pool.runInteraction( + return self.runInteraction( self._set_received_txn_response, transaction_id, origin, code, response_dict ) @@ -104,7 +104,7 @@ class TransactionStore(SQLBaseStore): list: A list of previous transaction ids. """ - return self._db_pool.runInteraction( + return self.runInteraction( self._prep_send_transaction, transaction_id, destination, ts, pdu_list ) @@ -159,7 +159,7 @@ class TransactionStore(SQLBaseStore): code (int) response_json (str) """ - return self._db_pool.runInteraction( + return self.runInteraction( self._delivered_txn, transaction_id, destination, code, response_dict ) @@ -184,7 +184,7 @@ class TransactionStore(SQLBaseStore): Returns: list: A list of `ReceivedTransactionsTable.EntryType` """ - return self._db_pool.runInteraction( + return self.runInteraction( self._get_transactions_after, transaction_id, destination ) @@ -214,7 +214,7 @@ class TransactionStore(SQLBaseStore): Returns list: A list of PduTuple """ - return self._db_pool.runInteraction( + return self.runInteraction( self._get_pdus_after_transaction, transaction_id, destination ) From 1c2024988457c5cdb9c0137a99e687e56e74e14b Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Fri, 12 Sep 2014 14:37:55 +0100 Subject: [PATCH 21/35] Logging of all SQL queries via the 'synapse.storage.SQL' logger --- synapse/storage/_base.py | 45 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/synapse/storage/_base.py b/synapse/storage/_base.py index 8a36f0bc6..a46f2c660 100644 --- a/synapse/storage/_base.py +++ b/synapse/storage/_base.py @@ -25,6 +25,44 @@ import json logger = logging.getLogger(__name__) +sql_logger = logging.getLogger("synapse.storage.SQL") + + +class LoggingTransaction(object): + """An object that almost-transparently proxies for the 'txn' object + passed to the constructor. Adds logging to the .execute() method.""" + __slots__ = ["txn"] + + def __init__(self, txn): + object.__setattr__(self, "txn", txn) + + def __getattribute__(self, name): + if name == "execute": + return object.__getattribute__(self, "execute") + + return getattr(object.__getattribute__(self, "txn"), name) + + def __setattr__(self, name, value): + setattr(object.__getattribute__(self, "txn"), name, value) + + def execute(self, sql, *args, **kwargs): + # TODO(paul): Maybe use 'info' and 'debug' for values? + sql_logger.debug("[SQL] %s", sql) + try: + if args and args[0]: + values = args[0] + sql_logger.debug("[SQL values] " + + ", ".join(("<%s>",) * len(values)), *values) + except: + # Don't let logging failures stop SQL from working + pass + + # TODO(paul): Here would be an excellent place to put some timing + # measurements, and log (warning?) slow queries. + return object.__getattribute__(self, "txn").execute( + sql, *args, **kwargs + ) + class SQLBaseStore(object): @@ -34,9 +72,12 @@ class SQLBaseStore(object): self.event_factory = hs.get_event_factory() self._clock = hs.get_clock() - def runInteraction(self, txn, *args, **kwargs): + def runInteraction(self, func, *args, **kwargs): """Wraps the .runInteraction() method on the underlying db_pool.""" - return self._db_pool.runInteraction(txn, *args, **kwargs) + def inner_func(txn, *args, **kwargs): + return func(LoggingTransaction(txn), *args, **kwargs) + + return self._db_pool.runInteraction(inner_func, *args, **kwargs) def cursor_to_dict(self, cursor): """Converts a SQL cursor into an list of dicts. From a840ff8f3fb0c58737a09cd326247fde4d75e2e3 Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Fri, 12 Sep 2014 14:38:27 +0100 Subject: [PATCH 22/35] Now don't need the other logger.debug() call in _execute --- synapse/storage/_base.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/synapse/storage/_base.py b/synapse/storage/_base.py index a46f2c660..7006c1995 100644 --- a/synapse/storage/_base.py +++ b/synapse/storage/_base.py @@ -104,11 +104,6 @@ class SQLBaseStore(object): Returns: The result of decoder(results) """ - logger.debug( - "[SQL] %s Args=%s Func=%s", - query, args, decoder.__name__ if decoder else None - ) - def interaction(txn): cursor = txn.execute(query, args) if decoder: From a87eac4308e2230c2f79f41e2e1817636da4b208 Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Fri, 12 Sep 2014 15:51:51 +0100 Subject: [PATCH 23/35] Revert recent changes to RoomMemberStore --- synapse/storage/__init__.py | 2 +- synapse/storage/roommember.py | 36 +++++++++-------------------------- 2 files changed, 10 insertions(+), 28 deletions(-) diff --git a/synapse/storage/__init__.py b/synapse/storage/__init__.py index 629c110be..0dbae504b 100644 --- a/synapse/storage/__init__.py +++ b/synapse/storage/__init__.py @@ -154,7 +154,7 @@ class DataStore(RoomMemberStore, RoomStore, @log_function def _persist_event_txn(self, txn, event, backfilled, stream_ordering=None): if event.type == RoomMemberEvent.TYPE: - self._store_room_member_from_event_txn(txn, event) + self._store_room_member_txn(txn, event) elif event.type == FeedbackEvent.TYPE: self._store_feedback_txn(txn, event) elif event.type == RoomNameEvent.TYPE: diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py index 8cbc15356..9a393e256 100644 --- a/synapse/storage/roommember.py +++ b/synapse/storage/roommember.py @@ -26,55 +26,37 @@ logger = logging.getLogger(__name__) class RoomMemberStore(SQLBaseStore): - def _store_room_member_from_event_txn(self, txn, event): - self._store_room_member_txn(txn, - target_user_id=event.state_key, - sender_user_id=event.user_id, - room_id=event.room_id, - event_id=event.event_id, - membership=event.membership, - ) - - def _store_room_member_txn(self, txn, target_user_id, sender_user_id, - room_id, event_id, membership): + def _store_room_member_txn(self, txn, event): """Store a room member in the database. """ + target_user_id = event.state_key domain = self.hs.parse_userid(target_user_id).domain self._simple_insert_txn( txn, "room_memberships", { - "event_id": event_id, + "event_id": event.event_id, "user_id": target_user_id, - "sender": sender_user_id, - "room_id": room_id, - "membership": membership, + "sender": event.user_id, + "room_id": event.room_id, + "membership": event.membership, } ) # Update room hosts table - # TODO(paul): This code is massively broken currently as it doesn't - # count users per room - meaning it'll delete on the FIRST user to - # have a membership other than JOIN - say, LEAVE, or even INVITE. - # FIXME - if membership == Membership.JOIN: + if event.membership == Membership.JOIN: sql = ( "INSERT OR IGNORE INTO room_hosts (room_id, host) " "VALUES (?, ?)" ) - txn.execute(sql, (room_id, domain)) + txn.execute(sql, (event.room_id, domain)) else: sql = ( "DELETE FROM room_hosts WHERE room_id = ? AND host = ?" ) - txn.execute(sql, (room_id, domain)) - - def store_room_member(self, user_id, room_id, event_id, membership): - return self.runInteraction(self._store_room_member_txn, - user_id, user_id, room_id, event_id, membership - ) + txn.execute(sql, (event.room_id, domain)) @defer.inlineCallbacks def get_room_member(self, user_id, room_id): From aa525e4a634a2a5932995dab3faead5a1b91c5a7 Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Fri, 12 Sep 2014 16:43:49 +0100 Subject: [PATCH 24/35] More accurate docs / clearer paramter names in RoomMemberStore --- synapse/storage/roommember.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/synapse/storage/roommember.py b/synapse/storage/roommember.py index 9a393e256..8357a0edd 100644 --- a/synapse/storage/roommember.py +++ b/synapse/storage/roommember.py @@ -120,7 +120,7 @@ class RoomMemberStore(SQLBaseStore): membership_list (list): A list of synapse.api.constants.Membership values which the user must be in. Returns: - A list of dicts with "room_id" and "membership" keys. + A list of RoomMemberEvent objects """ if not membership_list: return defer.succeed(None) @@ -165,10 +165,11 @@ class RoomMemberStore(SQLBaseStore): defer.returnValue(results) @defer.inlineCallbacks - def user_rooms_intersect(self, user_list): - """ Checks whether a list of users share a room. + def user_rooms_intersect(self, user_id_list): + """ Checks whether all the users whose IDs are given in a list share a + room. """ - user_list_clause = " OR ".join(["m.user_id = ?"] * len(user_list)) + user_list_clause = " OR ".join(["m.user_id = ?"] * len(user_id_list)) sql = ( "SELECT m.room_id FROM room_memberships as m " "INNER JOIN current_state_events as c " @@ -178,8 +179,8 @@ class RoomMemberStore(SQLBaseStore): "GROUP BY m.room_id HAVING COUNT(m.room_id) = ?" ) % {"clause": user_list_clause} - args = user_list - args.append(len(user_list)) + args = list(user_id_list) + args.append(len(user_id_list)) rows = yield self._execute(None, sql, *args) From 2026942b056ea46cb0f14336ea00c4c7d8b14311 Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Fri, 12 Sep 2014 16:44:07 +0100 Subject: [PATCH 25/35] Initial hack at some RoomMemberStore unit tests --- tests/storage/test_roommember.py | 109 +++++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+) create mode 100644 tests/storage/test_roommember.py diff --git a/tests/storage/test_roommember.py b/tests/storage/test_roommember.py new file mode 100644 index 000000000..5a6e93d4d --- /dev/null +++ b/tests/storage/test_roommember.py @@ -0,0 +1,109 @@ +# -*- coding: utf-8 -*- +# Copyright 2014 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. + + +from twisted.trial import unittest +from twisted.internet import defer + +from synapse.server import HomeServer +from synapse.api.constants import Membership +from synapse.api.events.room import RoomMemberEvent + +from tests.utils import SQLiteMemoryDbPool + + +class RoomMemberStoreTestCase(unittest.TestCase): + + @defer.inlineCallbacks + def setUp(self): + db_pool = SQLiteMemoryDbPool() + yield db_pool.prepare() + + hs = HomeServer("test", + db_pool=db_pool, + ) + + # We can't test the RoomMemberStore on its own without the other event + # storage logic + self.store = hs.get_datastore() + self.event_factory = hs.get_event_factory() + + self.u_alice = hs.parse_userid("@alice:test") + self.u_bob = hs.parse_userid("@bob:test") + + self.room = hs.parse_roomid("!abc123:test") + + @defer.inlineCallbacks + def inject_room_member(self, room, user, membership): + # Have to create a join event using the eventfactory + yield self.store.persist_event( + self.event_factory.create_event( + etype=RoomMemberEvent.TYPE, + user_id=user.to_string(), + state_key=user.to_string(), + room_id=room.to_string(), + membership=membership, + content={"membership": membership}, + depth=1, + ) + ) + + @defer.inlineCallbacks + def test_one_member(self): + yield self.inject_room_member(self.room, self.u_alice, Membership.JOIN) + + self.assertEquals( + Membership.JOIN, + (yield self.store.get_room_member( + user_id=self.u_alice.to_string(), + room_id=self.room.to_string(), + )).membership + ) + self.assertEquals( + [self.u_alice.to_string()], + [m.user_id for m in ( + yield self.store.get_room_members(self.room.to_string()) + )] + ) + self.assertEquals( + [self.room.to_string()], + [m.room_id for m in ( + yield self.store.get_rooms_for_user_where_membership_is( + self.u_alice.to_string(), [Membership.JOIN] + )) + ] + ) + self.assertFalse( + (yield self.store.user_rooms_intersect( + [self.u_alice.to_string(), self.u_bob.to_string()] + )) + ) + + @defer.inlineCallbacks + def test_two_members(self): + yield self.inject_room_member(self.room, self.u_alice, Membership.JOIN) + yield self.inject_room_member(self.room, self.u_bob, Membership.JOIN) + + self.assertEquals( + {self.u_alice.to_string(), self.u_bob.to_string()}, + {m.user_id for m in ( + yield self.store.get_room_members(self.room.to_string()) + )} + ) + self.assertTrue( + (yield self.store.user_rooms_intersect( + [self.u_alice.to_string(), self.u_bob.to_string()] + )) + ) From ae7dfeb5b6e8ee491f137d9a706cc83b9294b86f Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Mon, 15 Sep 2014 14:19:16 +0100 Subject: [PATCH 26/35] Use new 'tests.unittest' in new storage level tests --- tests/storage/test_directory.py | 2 +- tests/storage/test_presence.py | 2 +- tests/storage/test_profile.py | 2 +- tests/storage/test_registration.py | 2 +- tests/storage/test_roommember.py | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/storage/test_directory.py b/tests/storage/test_directory.py index c4c321dda..9a80bf6c0 100644 --- a/tests/storage/test_directory.py +++ b/tests/storage/test_directory.py @@ -14,7 +14,7 @@ # limitations under the License. -from twisted.trial import unittest +from tests import unittest from twisted.internet import defer from synapse.server import HomeServer diff --git a/tests/storage/test_presence.py b/tests/storage/test_presence.py index f3fab4fe7..9655d3cf4 100644 --- a/tests/storage/test_presence.py +++ b/tests/storage/test_presence.py @@ -14,7 +14,7 @@ # limitations under the License. -from twisted.trial import unittest +from tests import unittest from twisted.internet import defer from synapse.server import HomeServer diff --git a/tests/storage/test_profile.py b/tests/storage/test_profile.py index 185527804..21df2babd 100644 --- a/tests/storage/test_profile.py +++ b/tests/storage/test_profile.py @@ -14,7 +14,7 @@ # limitations under the License. -from twisted.trial import unittest +from tests import unittest from twisted.internet import defer from synapse.server import HomeServer diff --git a/tests/storage/test_registration.py b/tests/storage/test_registration.py index 5e11ce351..91e221d53 100644 --- a/tests/storage/test_registration.py +++ b/tests/storage/test_registration.py @@ -14,7 +14,7 @@ # limitations under the License. -from twisted.trial import unittest +from tests import unittest from twisted.internet import defer from synapse.server import HomeServer diff --git a/tests/storage/test_roommember.py b/tests/storage/test_roommember.py index 5a6e93d4d..dd18291b4 100644 --- a/tests/storage/test_roommember.py +++ b/tests/storage/test_roommember.py @@ -14,7 +14,7 @@ # limitations under the License. -from twisted.trial import unittest +from tests import unittest from twisted.internet import defer from synapse.server import HomeServer From 1aaa4290810dbc39c427af5cce8da1ee041d7a74 Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Mon, 15 Sep 2014 15:00:14 +0100 Subject: [PATCH 27/35] Also unittest RoomMemberStore's joined_hosts_for_room() --- tests/storage/test_roommember.py | 48 ++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/storage/test_roommember.py b/tests/storage/test_roommember.py index dd18291b4..eae278ee8 100644 --- a/tests/storage/test_roommember.py +++ b/tests/storage/test_roommember.py @@ -43,6 +43,9 @@ class RoomMemberStoreTestCase(unittest.TestCase): self.u_alice = hs.parse_userid("@alice:test") self.u_bob = hs.parse_userid("@bob:test") + # User elsewhere on another host + self.u_charlie = hs.parse_userid("@charlie:elsewhere") + self.room = hs.parse_roomid("!abc123:test") @defer.inlineCallbacks @@ -107,3 +110,48 @@ class RoomMemberStoreTestCase(unittest.TestCase): [self.u_alice.to_string(), self.u_bob.to_string()] )) ) + + @defer.inlineCallbacks + def test_room_hosts(self): + yield self.inject_room_member(self.room, self.u_alice, Membership.JOIN) + + self.assertEquals( + ["test"], + (yield self.store.get_joined_hosts_for_room(self.room.to_string())) + ) + + # Should still have just one host after second join from it + yield self.inject_room_member(self.room, self.u_bob, Membership.JOIN) + + self.assertEquals( + ["test"], + (yield self.store.get_joined_hosts_for_room(self.room.to_string())) + ) + + # Should now have two hosts after join from other host + yield self.inject_room_member(self.room, self.u_charlie, Membership.JOIN) + + self.assertEquals( + {"test", "elsewhere"}, + set((yield + self.store.get_joined_hosts_for_room(self.room.to_string()) + )) + ) + + # Should still have both hosts + yield self.inject_room_member(self.room, self.u_alice, Membership.LEAVE) + + self.assertEquals( + {"test", "elsewhere"}, + set((yield + self.store.get_joined_hosts_for_room(self.room.to_string()) + )) + ) + + # Should have only one host after other leaves + yield self.inject_room_member(self.room, self.u_charlie, Membership.LEAVE) + + self.assertEquals( + ["test"], + (yield self.store.get_joined_hosts_for_room(self.room.to_string())) + ) From e32cfed1d8778ce59f406853686675bb8185dfb4 Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Mon, 15 Sep 2014 18:41:24 +0100 Subject: [PATCH 28/35] Initial pass at a RoomStore test --- tests/storage/test_room.py | 53 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) create mode 100644 tests/storage/test_room.py diff --git a/tests/storage/test_room.py b/tests/storage/test_room.py new file mode 100644 index 000000000..2477f965e --- /dev/null +++ b/tests/storage/test_room.py @@ -0,0 +1,53 @@ +# -*- coding: utf-8 -*- +# Copyright 2014 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. + + +from tests import unittest +from twisted.internet import defer + +from synapse.server import HomeServer +from synapse.storage.room import RoomStore + +from tests.utils import SQLiteMemoryDbPool + + +class RoomStoreTestCase(unittest.TestCase): + + @defer.inlineCallbacks + def setUp(self): + db_pool = SQLiteMemoryDbPool() + yield db_pool.prepare() + + hs = HomeServer("test", + db_pool=db_pool, + ) + + self.store = RoomStore(hs) + + self.room = hs.parse_roomid("!abcde:test") + self.u_creator = hs.parse_userid("@creator:test") + + @defer.inlineCallbacks + def test_store_room(self): + yield self.store.store_room(self.room.to_string(), + room_creator_user_id=self.u_creator.to_string(), + is_public=True + ) + + room = yield self.store.get_room(self.room.to_string()) + + self.assertEquals(self.room.to_string(), room.room_id) + self.assertEquals(self.u_creator.to_string(), room.creator) + self.assertTrue(room.is_public) From 9973298e2ac4039b96e923faa984b400ea720b7f Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Wed, 17 Sep 2014 15:27:45 +0100 Subject: [PATCH 29/35] Print expected-vs-actual data types on typecheck failure from check_json() --- synapse/api/events/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/api/events/__init__.py b/synapse/api/events/__init__.py index 72c493db5..4fe060801 100644 --- a/synapse/api/events/__init__.py +++ b/synapse/api/events/__init__.py @@ -154,7 +154,8 @@ class SynapseEvent(JsonEncodedObject): return "Missing %s key" % key if type(content[key]) != type(template[key]): - return "Key %s is of the wrong type." % key + return "Key %s is of the wrong type (got %s, want %s)" % ( + key, type(content[key]), type(template[key])) if type(content[key]) == dict: # we must go deeper From de1485323749b5aed0d0143c695180eb25ee8808 Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Wed, 17 Sep 2014 15:33:10 +0100 Subject: [PATCH 30/35] More RoomStore tests --- tests/storage/test_room.py | 44 ++++++++++++++++++++++++++++++++++---- 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/tests/storage/test_room.py b/tests/storage/test_room.py index 2477f965e..8d85d56b9 100644 --- a/tests/storage/test_room.py +++ b/tests/storage/test_room.py @@ -18,7 +18,6 @@ from tests import unittest from twisted.internet import defer from synapse.server import HomeServer -from synapse.storage.room import RoomStore from tests.utils import SQLiteMemoryDbPool @@ -34,20 +33,57 @@ class RoomStoreTestCase(unittest.TestCase): db_pool=db_pool, ) - self.store = RoomStore(hs) + # We can't test RoomStore on its own without the DirectoryStore, for + # management of the 'room_aliases' table + self.store = hs.get_datastore() self.room = hs.parse_roomid("!abcde:test") + self.alias = hs.parse_roomalias("#a-room-name:test") self.u_creator = hs.parse_userid("@creator:test") - @defer.inlineCallbacks - def test_store_room(self): yield self.store.store_room(self.room.to_string(), room_creator_user_id=self.u_creator.to_string(), is_public=True ) + @defer.inlineCallbacks + def test_get_room(self): room = yield self.store.get_room(self.room.to_string()) self.assertEquals(self.room.to_string(), room.room_id) self.assertEquals(self.u_creator.to_string(), room.creator) self.assertTrue(room.is_public) + + @defer.inlineCallbacks + def test_store_room_config(self): + yield self.store.store_room_config(self.room.to_string(), + visibility=False + ) + + room = yield self.store.get_room(self.room.to_string()) + + self.assertFalse(room.is_public) + + @defer.inlineCallbacks + def test_get_rooms(self): + # get_rooms does an INNER JOIN on the room_aliases table :( + + rooms = yield self.store.get_rooms(is_public=True) + # Should be empty before we add the alias + self.assertEquals([], rooms) + + yield self.store.create_room_alias_association( + room_alias=self.alias, + room_id=self.room.to_string(), + servers=["test"] + ) + + rooms = yield self.store.get_rooms(is_public=True) + + self.assertEquals(1, len(rooms)) + self.assertEquals({ + "name": None, + "room_id": self.room.to_string(), + "topic": None, + "aliases": [self.alias.to_string()], + }, rooms[0]) From 7aacd6834a17c84dacba875f362a8236aaaa2fb0 Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Wed, 17 Sep 2014 15:56:40 +0100 Subject: [PATCH 31/35] Added a useful unit test primitive for asserting object attributes --- tests/unittest.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/unittest.py b/tests/unittest.py index fb97fb114..a9c0e0554 100644 --- a/tests/unittest.py +++ b/tests/unittest.py @@ -71,6 +71,17 @@ class TestCase(unittest.TestCase): logging.getLogger().setLevel(level) return orig() + def assertObjectHasAttributes(self, attrs, obj): + """Asserts that the given object has each of the attributes given, and + that the value of each matches according to assertEquals.""" + for (key, value) in attrs.items(): + if not hasattr(obj, key): + raise AssertionError("Expected obj to have a '.%s'" % key) + try: + self.assertEquals(attrs[key], getattr(obj, key)) + except AssertionError as e: + raise (type(e))(e.message + " for '.%s'" % key) + def DEBUG(target): """A decorator to set the .loglevel attribute to logging.DEBUG. From ba41ca45fa40cb23bd8f28f6802788f7f38a46b7 Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Wed, 17 Sep 2014 16:04:05 +0100 Subject: [PATCH 32/35] Use new assertObjectHasAttributes() in tests/storage/test_room.py --- tests/storage/test_room.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/storage/test_room.py b/tests/storage/test_room.py index 8d85d56b9..9e794074d 100644 --- a/tests/storage/test_room.py +++ b/tests/storage/test_room.py @@ -50,9 +50,12 @@ class RoomStoreTestCase(unittest.TestCase): def test_get_room(self): room = yield self.store.get_room(self.room.to_string()) - self.assertEquals(self.room.to_string(), room.room_id) - self.assertEquals(self.u_creator.to_string(), room.creator) - self.assertTrue(room.is_public) + self.assertObjectHasAttributes( + {"room_id": self.room.to_string(), + "creator": self.u_creator.to_string(), + "is_public": True}, + room + ) @defer.inlineCallbacks def test_store_room_config(self): From b588ce920df77a4cfd3af9dd806d17c7bcf1d4cb Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Wed, 17 Sep 2014 16:31:11 +0100 Subject: [PATCH 33/35] Unit tests for (some) room events via the RoomStore --- tests/storage/test_room.py | 85 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/tests/storage/test_room.py b/tests/storage/test_room.py index 9e794074d..9979be2f6 100644 --- a/tests/storage/test_room.py +++ b/tests/storage/test_room.py @@ -18,6 +18,9 @@ from tests import unittest from twisted.internet import defer from synapse.server import HomeServer +from synapse.api.events.room import ( + RoomNameEvent, RoomTopicEvent +) from tests.utils import SQLiteMemoryDbPool @@ -90,3 +93,85 @@ class RoomStoreTestCase(unittest.TestCase): "topic": None, "aliases": [self.alias.to_string()], }, rooms[0]) + + +class RoomEventsStoreTestCase(unittest.TestCase): + + @defer.inlineCallbacks + def setUp(self): + db_pool = SQLiteMemoryDbPool() + yield db_pool.prepare() + + hs = HomeServer("test", + db_pool=db_pool, + ) + + # Room events need the full datastore, for persist_event() and + # get_room_state() + self.store = hs.get_datastore() + self.event_factory = hs.get_event_factory(); + + self.room = hs.parse_roomid("!abcde:test") + + yield self.store.store_room(self.room.to_string(), + room_creator_user_id="@creator:text", + is_public=True + ) + + @defer.inlineCallbacks + def inject_room_event(self, **kwargs): + yield self.store.persist_event( + self.event_factory.create_event( + room_id=self.room.to_string(), + **kwargs + ) + ) + + @defer.inlineCallbacks + def test_room_name(self): + name = u"A-Room-Name" + + yield self.inject_room_event( + etype=RoomNameEvent.TYPE, + name=name, + content={"name": name}, + depth=1, + ) + + state = yield self.store.get_current_state( + room_id=self.room.to_string() + ) + + self.assertEquals(1, len(state)) + self.assertObjectHasAttributes( + {"type": "m.room.name", + "room_id": self.room.to_string(), + "name": name}, + state[0] + ) + + @defer.inlineCallbacks + def test_room_name(self): + topic = u"A place for things" + + yield self.inject_room_event( + etype=RoomTopicEvent.TYPE, + topic=topic, + content={"topic": topic}, + depth=1, + ) + + state = yield self.store.get_current_state( + room_id=self.room.to_string() + ) + + self.assertEquals(1, len(state)) + self.assertObjectHasAttributes( + {"type": "m.room.topic", + "room_id": self.room.to_string(), + "topic": topic}, + state[0] + ) + + # Not testing the various 'level' methods for now because there's lots + # of them and need coalescing; see JIRA SPEC-11 From bcf512193705a0aaca2da3bbfd62ce6f4cb65980 Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Wed, 17 Sep 2014 16:58:59 +0100 Subject: [PATCH 34/35] Neaten more of the storage layer tests with assertObjectHasAttributes; more standardisation on test layout --- tests/storage/test_directory.py | 17 +++++++++-------- tests/storage/test_profile.py | 14 ++++++++------ tests/storage/test_room.py | 11 +++++------ 3 files changed, 22 insertions(+), 20 deletions(-) diff --git a/tests/storage/test_directory.py b/tests/storage/test_directory.py index 9a80bf6c0..7e8e7e1e8 100644 --- a/tests/storage/test_directory.py +++ b/tests/storage/test_directory.py @@ -47,9 +47,10 @@ class DirectoryStoreTestCase(unittest.TestCase): servers=["test"], ) - aliases = yield self.store.get_aliases_for_room(self.room.to_string()) - - self.assertEquals(["#my-room:test"], aliases) + self.assertEquals( + ["#my-room:test"], + (yield self.store.get_aliases_for_room(self.room.to_string())) + ) @defer.inlineCallbacks def test_alias_to_room(self): @@ -59,9 +60,9 @@ class DirectoryStoreTestCase(unittest.TestCase): servers=["test"], ) - mapping = yield self.store.get_association_from_room_alias( - self.alias - ) - self.assertEquals(self.room.to_string(), mapping.room_id) - self.assertEquals(["test"], mapping.servers) + self.assertObjectHasAttributes( + {"room_id": self.room.to_string(), + "servers": ["test"]}, + (yield self.store.get_association_from_room_alias(self.alias)) + ) diff --git a/tests/storage/test_profile.py b/tests/storage/test_profile.py index 21df2babd..5d36723c2 100644 --- a/tests/storage/test_profile.py +++ b/tests/storage/test_profile.py @@ -48,9 +48,10 @@ class ProfileStoreTestCase(unittest.TestCase): self.u_frank.localpart, "Frank" ) - name = yield self.store.get_profile_displayname(self.u_frank.localpart) - - self.assertEquals("Frank", name) + self.assertEquals( + "Frank", + (yield self.store.get_profile_displayname(self.u_frank.localpart)) + ) @defer.inlineCallbacks def test_avatar_url(self): @@ -62,6 +63,7 @@ class ProfileStoreTestCase(unittest.TestCase): self.u_frank.localpart, "http://my.site/here" ) - name = yield self.store.get_profile_avatar_url(self.u_frank.localpart) - - self.assertEquals("http://my.site/here", name) + self.assertEquals( + "http://my.site/here", + (yield self.store.get_profile_avatar_url(self.u_frank.localpart)) + ) diff --git a/tests/storage/test_room.py b/tests/storage/test_room.py index 9979be2f6..369a73d91 100644 --- a/tests/storage/test_room.py +++ b/tests/storage/test_room.py @@ -51,13 +51,11 @@ class RoomStoreTestCase(unittest.TestCase): @defer.inlineCallbacks def test_get_room(self): - room = yield self.store.get_room(self.room.to_string()) - self.assertObjectHasAttributes( {"room_id": self.room.to_string(), "creator": self.u_creator.to_string(), "is_public": True}, - room + (yield self.store.get_room(self.room.to_string())) ) @defer.inlineCallbacks @@ -66,9 +64,10 @@ class RoomStoreTestCase(unittest.TestCase): visibility=False ) - room = yield self.store.get_room(self.room.to_string()) - - self.assertFalse(room.is_public) + self.assertObjectHasAttributes( + {"is_public": False}, + (yield self.store.get_room(self.room.to_string())) + ) @defer.inlineCallbacks def test_get_rooms(self): From bfae582fa3e92e4b6279ce7d5004ae933fe579a9 Mon Sep 17 00:00:00 2001 From: "Paul \"LeoNerd\" Evans" Date: Wed, 17 Sep 2014 18:27:30 +0100 Subject: [PATCH 35/35] Remark on remaining storage modules that still need unit tests --- tests/storage/TESTS_NEEDED_FOR | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 tests/storage/TESTS_NEEDED_FOR diff --git a/tests/storage/TESTS_NEEDED_FOR b/tests/storage/TESTS_NEEDED_FOR new file mode 100644 index 000000000..8e5d0cbdc --- /dev/null +++ b/tests/storage/TESTS_NEEDED_FOR @@ -0,0 +1,5 @@ +synapse/storage/feedback.py +synapse/storage/keys.py +synapse/storage/pdu.py +synapse/storage/stream.py +synapse/storage/transactions.py