From 80c73c21cd29821ea4bcb2eb635b8acd50cc851d Mon Sep 17 00:00:00 2001 From: Sean Quah Date: Fri, 1 Oct 2021 12:30:10 +0100 Subject: [PATCH 1/6] Fix error in `get_user_ip_and_agents` when fetching from the database --- changelog.d/10968.bugfix | 1 + synapse/storage/databases/main/client_ips.py | 4 +-- tests/storage/test_client_ips.py | 34 ++++++++++++++++++++ 3 files changed, 37 insertions(+), 2 deletions(-) create mode 100644 changelog.d/10968.bugfix diff --git a/changelog.d/10968.bugfix b/changelog.d/10968.bugfix new file mode 100644 index 000000000000..76624ed73c36 --- /dev/null +++ b/changelog.d/10968.bugfix @@ -0,0 +1 @@ +Fix `/admin/whois/{user_id}` endpoint, which was broken in v1.44.0rc1. diff --git a/synapse/storage/databases/main/client_ips.py b/synapse/storage/databases/main/client_ips.py index 0e1d97aaebe0..c77acc7c84c5 100644 --- a/synapse/storage/databases/main/client_ips.py +++ b/synapse/storage/databases/main/client_ips.py @@ -591,8 +591,8 @@ def get_recent(txn): ) results.update( - ((row["access_token"], row["ip"]), (row["user_agent"], row["last_seen"])) - for row in rows + ((access_token, ip), (user_agent, last_seen)) + for access_token, ip, user_agent, last_seen in rows ) return [ { diff --git a/tests/storage/test_client_ips.py b/tests/storage/test_client_ips.py index 1c2df54ecc53..3cc8038f1e65 100644 --- a/tests/storage/test_client_ips.py +++ b/tests/storage/test_client_ips.py @@ -15,9 +15,12 @@ from unittest.mock import Mock +from parameterized import parameterized + import synapse.rest.admin from synapse.http.site import XForwardedForRequest from synapse.rest.client import login +from synapse.types import UserID from tests import unittest from tests.server import make_request @@ -143,6 +146,37 @@ def test_insert_new_client_ip_none_device_id(self): ], ) + @parameterized.expand([(False,), (True,)]) + def test_get_user_ip_and_agents(self, after_persisting: bool): + """Test `get_user_ip_and_agents` for persisted and unpersisted data""" + self.reactor.advance(12345678) + + user_id = "@user:id" + user = UserID.from_string(user_id) + + # Insert a user IP + self.get_success( + self.store.insert_client_ip( + user_id, "access_token", "ip", "user_agent", "MY_DEVICE" + ) + ) + + if after_persisting: + # Trigger the storage loop + self.reactor.advance(10) + + self.assertEqual( + self.get_success(self.store.get_user_ip_and_agents(user)), + [ + { + "access_token": "access_token", + "ip": "ip", + "user_agent": "user_agent", + "last_seen": 12345678000, + }, + ], + ) + @override_config({"limit_usage_by_mau": False, "max_mau_value": 50}) def test_disabled_monthly_active_user(self): user_id = "@user:server" From cf20c86116d75e4a3d2930c8672fa837d1474bde Mon Sep 17 00:00:00 2001 From: Sean Quah Date: Fri, 1 Oct 2021 14:02:06 +0100 Subject: [PATCH 2/6] Fix inconsistent behavior of `get_last_client_by_ip` Make `get_last_client_by_ip` return the same dictionary structure regardless of whether the data has been persisted to the database. This change will allow slightly cleaner type hints to be applied later on. --- changelog.d/10970.misc | 1 + synapse/storage/databases/main/client_ips.py | 13 ++++-- tests/storage/test_client_ips.py | 43 ++++++++++++++++++++ 3 files changed, 53 insertions(+), 4 deletions(-) create mode 100644 changelog.d/10970.misc diff --git a/changelog.d/10970.misc b/changelog.d/10970.misc new file mode 100644 index 000000000000..bb75ea79a657 --- /dev/null +++ b/changelog.d/10970.misc @@ -0,0 +1 @@ +Fix inconsistent behavior of `get_last_client_by_ip` when reporting data that has not been stored in the database yet. diff --git a/synapse/storage/databases/main/client_ips.py b/synapse/storage/databases/main/client_ips.py index c77acc7c84c5..6c1ef0904973 100644 --- a/synapse/storage/databases/main/client_ips.py +++ b/synapse/storage/databases/main/client_ips.py @@ -538,15 +538,20 @@ async def get_last_client_ip_by_device( """ ret = await super().get_last_client_ip_by_device(user_id, device_id) - # Update what is retrieved from the database with data which is pending insertion. + # Update what is retrieved from the database with data which is pending + # insertion, as if it has already been stored in the database. for key in self._batch_row_update: - uid, access_token, ip = key + uid, _access_token, ip = key if uid == user_id: user_agent, did, last_seen = self._batch_row_update[key] + + if did is None: + # These updates don't make it to the `devices` table + continue + if not device_id or did == device_id: - ret[(user_id, device_id)] = { + ret[(user_id, did)] = { "user_id": user_id, - "access_token": access_token, "ip": ip, "user_agent": user_agent, "device_id": did, diff --git a/tests/storage/test_client_ips.py b/tests/storage/test_client_ips.py index 3cc8038f1e65..cb0cda820037 100644 --- a/tests/storage/test_client_ips.py +++ b/tests/storage/test_client_ips.py @@ -177,6 +177,49 @@ def test_get_user_ip_and_agents(self, after_persisting: bool): ], ) + @parameterized.expand([(False,), (True,)]) + def test_get_last_client_ip_by_device(self, after_persisting: bool): + """Test `get_last_client_ip_by_device` for persisted and unpersisted data""" + self.reactor.advance(12345678) + + user_id = "@user:id" + device_id = "MY_DEVICE" + + # Insert a user IP + self.get_success( + self.store.store_device( + user_id, + device_id, + "display name", + ) + ) + self.get_success( + self.store.insert_client_ip( + user_id, "access_token", "ip", "user_agent", device_id + ) + ) + + if after_persisting: + # Trigger the storage loop + self.reactor.advance(10) + + result = self.get_success( + self.store.get_last_client_ip_by_device(user_id, device_id) + ) + + self.assertEqual( + result, + { + (user_id, device_id): { + "user_id": user_id, + "device_id": device_id, + "ip": "ip", + "user_agent": "user_agent", + "last_seen": 12345678000, + }, + }, + ) + @override_config({"limit_usage_by_mau": False, "max_mau_value": 50}) def test_disabled_monthly_active_user(self): user_id = "@user:server" From 744756d6983e91626e0d3ff73e8be36d15b169c6 Mon Sep 17 00:00:00 2001 From: Sean Quah Date: Fri, 1 Oct 2021 15:20:55 +0100 Subject: [PATCH 3/6] Add type hints to `synapse.storage.databases.main.client_ips` --- changelog.d/10972.misc | 1 + mypy.ini | 4 + synapse/handlers/device.py | 15 +- synapse/module_api/__init__.py | 6 +- synapse/storage/databases/main/client_ips.py | 141 +++++++++++++------ 5 files changed, 122 insertions(+), 45 deletions(-) create mode 100644 changelog.d/10972.misc diff --git a/changelog.d/10972.misc b/changelog.d/10972.misc new file mode 100644 index 000000000000..f66a7beaf05f --- /dev/null +++ b/changelog.d/10972.misc @@ -0,0 +1 @@ +Add type hints to `synapse.storage.databases.main.client_ips`. diff --git a/mypy.ini b/mypy.ini index 568166db3300..020b31945ef2 100644 --- a/mypy.ini +++ b/mypy.ini @@ -53,6 +53,7 @@ files = synapse/storage/_base.py, synapse/storage/background_updates.py, synapse/storage/databases/main/appservice.py, + synapse/storage/databases/main/client_ips.py, synapse/storage/databases/main/events.py, synapse/storage/databases/main/keys.py, synapse/storage/databases/main/pusher.py, @@ -99,6 +100,9 @@ disallow_untyped_defs = True [mypy-synapse.rest.*] disallow_untyped_defs = True +[mypy-synapse.storage.databases.main.client_ips] +disallow_untyped_defs = True + [mypy-synapse.util.batching_queue] disallow_untyped_defs = True diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py index 35334725d76b..6f84da521650 100644 --- a/synapse/handlers/device.py +++ b/synapse/handlers/device.py @@ -14,7 +14,18 @@ # See the License for the specific language governing permissions and # limitations under the License. import logging -from typing import TYPE_CHECKING, Collection, Dict, Iterable, List, Optional, Set, Tuple +from typing import ( + TYPE_CHECKING, + Any, + Collection, + Dict, + Iterable, + List, + Mapping, + Optional, + Set, + Tuple, +) from synapse.api import errors from synapse.api.constants import EventTypes @@ -595,7 +606,7 @@ async def rehydrate_device( def _update_device_from_client_ips( - device: JsonDict, client_ips: Dict[Tuple[str, str], JsonDict] + device: JsonDict, client_ips: Mapping[Tuple[str, str], Mapping[str, Any]] ) -> None: ip = client_ips.get((device["user_id"], device["device_id"]), {}) device.update({"last_seen_ts": ip.get("last_seen"), "last_seen_ip": ip.get("ip")}) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index 8ae21bc43c23..b2a228c23178 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -773,9 +773,9 @@ async def get_user_ip_and_agents( # Sanitize some of the data. We don't want to return tokens. return [ UserIpAndAgent( - ip=str(data["ip"]), - user_agent=str(data["user_agent"]), - last_seen=int(data["last_seen"]), + ip=data["ip"], + user_agent=data["user_agent"], + last_seen=data["last_seen"], ) for data in raw_data ] diff --git a/synapse/storage/databases/main/client_ips.py b/synapse/storage/databases/main/client_ips.py index 6c1ef0904973..883ea2c07368 100644 --- a/synapse/storage/databases/main/client_ips.py +++ b/synapse/storage/databases/main/client_ips.py @@ -13,14 +13,26 @@ # limitations under the License. import logging -from typing import Dict, List, Optional, Tuple, Union +from typing import TYPE_CHECKING, Dict, List, Mapping, Optional, Tuple, Union, cast + +from typing_extensions import TypedDict from synapse.metrics.background_process_metrics import wrap_as_background_process from synapse.storage._base import SQLBaseStore -from synapse.storage.database import DatabasePool, make_tuple_comparison_clause -from synapse.types import UserID +from synapse.storage.database import ( + DatabasePool, + LoggingDatabaseConnection, + LoggingTransaction, + make_tuple_comparison_clause, +) +from synapse.storage.databases.main.monthly_active_users import MonthlyActiveUsersStore +from synapse.storage.types import Connection +from synapse.types import JsonDict, UserID from synapse.util.caches.lrucache import LruCache +if TYPE_CHECKING: + from synapse.server import HomeServer + logger = logging.getLogger(__name__) # Number of msec of granularity to store the user IP 'last seen' time. Smaller @@ -29,8 +41,32 @@ LAST_SEEN_GRANULARITY = 120 * 1000 +LastClientIpByDeviceEntry = TypedDict( + "LastClientIpByDeviceEntry", + { + # These types must match the columns in the `devices` table + "user_id": str, + "ip": Optional[str], + "user_agent": Optional[str], + "device_id": str, + "last_seen": Optional[int], + }, +) + +UserIpAndAgentsEntry = TypedDict( + "UserIpAndAgentsEntry", + { + # These types must match the columns in the `user_ips` table + "access_token": str, + "ip": str, + "user_agent": str, + "last_seen": int, + }, +) + + class ClientIpBackgroundUpdateStore(SQLBaseStore): - def __init__(self, database: DatabasePool, db_conn, hs): + def __init__(self, database: DatabasePool, db_conn: Connection, hs: "HomeServer"): super().__init__(database, db_conn, hs) self.db_pool.updates.register_background_index_update( @@ -81,8 +117,10 @@ def __init__(self, database: DatabasePool, db_conn, hs): "devices_last_seen", self._devices_last_seen_update ) - async def _remove_user_ip_nonunique(self, progress, batch_size): - def f(conn): + async def _remove_user_ip_nonunique( + self, progress: JsonDict, batch_size: int + ) -> int: + def f(conn: LoggingDatabaseConnection) -> None: txn = conn.cursor() txn.execute("DROP INDEX IF EXISTS user_ips_user_ip") txn.close() @@ -93,14 +131,14 @@ def f(conn): ) return 1 - async def _analyze_user_ip(self, progress, batch_size): + async def _analyze_user_ip(self, progress: JsonDict, batch_size: int) -> int: # Background update to analyze user_ips table before we run the # deduplication background update. The table may not have been analyzed # for ages due to the table locks. # # This will lock out the naive upserts to user_ips while it happens, but # the analyze should be quick (28GB table takes ~10s) - def user_ips_analyze(txn): + def user_ips_analyze(txn: LoggingTransaction) -> None: txn.execute("ANALYZE user_ips") await self.db_pool.runInteraction("user_ips_analyze", user_ips_analyze) @@ -109,16 +147,16 @@ def user_ips_analyze(txn): return 1 - async def _remove_user_ip_dupes(self, progress, batch_size): + async def _remove_user_ip_dupes(self, progress: JsonDict, batch_size: int) -> int: # This works function works by scanning the user_ips table in batches # based on `last_seen`. For each row in a batch it searches the rest of # the table to see if there are any duplicates, if there are then they # are removed and replaced with a suitable row. # Fetch the start of the batch - begin_last_seen = progress.get("last_seen", 0) + begin_last_seen: int = progress.get("last_seen", 0) - def get_last_seen(txn): + def get_last_seen(txn: LoggingTransaction) -> Optional[int]: txn.execute( """ SELECT last_seen FROM user_ips @@ -129,7 +167,7 @@ def get_last_seen(txn): """, (begin_last_seen, batch_size), ) - row = txn.fetchone() + row = cast(Optional[Tuple[int]], txn.fetchone()) if row: return row[0] else: @@ -149,7 +187,7 @@ def get_last_seen(txn): end_last_seen, ) - def remove(txn): + def remove(txn: LoggingTransaction) -> None: # This works by looking at all entries in the given time span, and # then for each (user_id, access_token, ip) tuple in that range # checking for any duplicates in the rest of the table (via a join). @@ -161,10 +199,12 @@ def remove(txn): # Define the search space, which requires handling the last batch in # a different way + args: Tuple[int, ...] if last: clause = "? <= last_seen" args = (begin_last_seen,) else: + assert end_last_seen is not None clause = "? <= last_seen AND last_seen < ?" args = (begin_last_seen, end_last_seen) @@ -189,7 +229,9 @@ def remove(txn): ), args, ) - res = txn.fetchall() + res = cast( + List[Tuple[str, str, str, Optional[str], str, int, int]], txn.fetchall() + ) # We've got some duplicates for i in res: @@ -278,13 +320,15 @@ def remove(txn): return batch_size - async def _devices_last_seen_update(self, progress, batch_size): + async def _devices_last_seen_update( + self, progress: JsonDict, batch_size: int + ) -> int: """Background update to insert last seen info into devices table""" - last_user_id = progress.get("last_user_id", "") - last_device_id = progress.get("last_device_id", "") + last_user_id: str = progress.get("last_user_id", "") + last_device_id: str = progress.get("last_device_id", "") - def _devices_last_seen_update_txn(txn): + def _devices_last_seen_update_txn(txn: LoggingTransaction) -> int: # This consists of two queries: # # 1. The sub-query searches for the next N devices and joins @@ -296,6 +340,7 @@ def _devices_last_seen_update_txn(txn): # we'll just end up updating the same device row multiple # times, which is fine. + where_args: List[Union[str, int]] where_clause, where_args = make_tuple_comparison_clause( [("user_id", last_user_id), ("device_id", last_device_id)], ) @@ -319,7 +364,7 @@ def _devices_last_seen_update_txn(txn): } txn.execute(sql, where_args + [batch_size]) - rows = txn.fetchall() + rows = cast(List[Tuple[int, str, str, str, str]], txn.fetchall()) if not rows: return 0 @@ -350,7 +395,7 @@ def _devices_last_seen_update_txn(txn): class ClientIpWorkerStore(ClientIpBackgroundUpdateStore): - def __init__(self, database: DatabasePool, db_conn, hs): + def __init__(self, database: DatabasePool, db_conn: Connection, hs: "HomeServer"): super().__init__(database, db_conn, hs) self.user_ips_max_age = hs.config.server.user_ips_max_age @@ -359,7 +404,7 @@ def __init__(self, database: DatabasePool, db_conn, hs): self._clock.looping_call(self._prune_old_user_ips, 5 * 1000) @wrap_as_background_process("prune_old_user_ips") - async def _prune_old_user_ips(self): + async def _prune_old_user_ips(self) -> None: """Removes entries in user IPs older than the configured period.""" if self.user_ips_max_age is None: @@ -394,9 +439,9 @@ async def _prune_old_user_ips(self): ) """ - timestamp = self.clock.time_msec() - self.user_ips_max_age + timestamp = self._clock.time_msec() - self.user_ips_max_age - def _prune_old_user_ips_txn(txn): + def _prune_old_user_ips_txn(txn: LoggingTransaction) -> None: txn.execute(sql, (timestamp,)) await self.db_pool.runInteraction( @@ -405,7 +450,7 @@ def _prune_old_user_ips_txn(txn): async def get_last_client_ip_by_device( self, user_id: str, device_id: Optional[str] - ) -> Dict[Tuple[str, str], dict]: + ) -> Dict[Tuple[str, str], LastClientIpByDeviceEntry]: """For each device_id listed, give the user_ip it was last seen on. The result might be slightly out of date as client IPs are inserted in batches. @@ -423,26 +468,32 @@ async def get_last_client_ip_by_device( if device_id is not None: keyvalues["device_id"] = device_id - res = await self.db_pool.simple_select_list( - table="devices", - keyvalues=keyvalues, - retcols=("user_id", "ip", "user_agent", "device_id", "last_seen"), + res = cast( + List[LastClientIpByDeviceEntry], + await self.db_pool.simple_select_list( + table="devices", + keyvalues=keyvalues, + retcols=("user_id", "ip", "user_agent", "device_id", "last_seen"), + ), ) return {(d["user_id"], d["device_id"]): d for d in res} -class ClientIpStore(ClientIpWorkerStore): - def __init__(self, database: DatabasePool, db_conn, hs): +class ClientIpStore(ClientIpWorkerStore, MonthlyActiveUsersStore): + def __init__(self, database: DatabasePool, db_conn: Connection, hs: "HomeServer"): - self.client_ip_last_seen = LruCache( + # (user_id, access_token, ip,) -> last_seen + self.client_ip_last_seen = LruCache[Tuple[str, str, str], int]( cache_name="client_ip_last_seen", max_size=50000 ) super().__init__(database, db_conn, hs) # (user_id, access_token, ip,) -> (user_agent, device_id, last_seen) - self._batch_row_update = {} + self._batch_row_update: Dict[ + Tuple[str, str, str], Tuple[str, Optional[str], int] + ] = {} self._client_ip_looper = self._clock.looping_call( self._update_client_ips_batch, 5 * 1000 @@ -452,8 +503,14 @@ def __init__(self, database: DatabasePool, db_conn, hs): ) async def insert_client_ip( - self, user_id, access_token, ip, user_agent, device_id, now=None - ): + self, + user_id: str, + access_token: str, + ip: str, + user_agent: str, + device_id: Optional[str], + now: Optional[int] = None, + ) -> None: if not now: now = int(self._clock.time_msec()) key = (user_id, access_token, ip) @@ -485,7 +542,11 @@ async def _update_client_ips_batch(self) -> None: "_update_client_ips_batch", self._update_client_ips_batch_txn, to_update ) - def _update_client_ips_batch_txn(self, txn, to_update): + def _update_client_ips_batch_txn( + self, + txn: LoggingTransaction, + to_update: Mapping[Tuple[str, str, str], Tuple[str, Optional[str], int]], + ) -> None: if "user_ips" in self.db_pool._unsafe_to_upsert_tables or ( not self.database_engine.can_native_upsert ): @@ -525,7 +586,7 @@ def _update_client_ips_batch_txn(self, txn, to_update): async def get_last_client_ip_by_device( self, user_id: str, device_id: Optional[str] - ) -> Dict[Tuple[str, str], dict]: + ) -> Dict[Tuple[str, str], LastClientIpByDeviceEntry]: """For each device_id listed, give the user_ip it was last seen on Args: @@ -561,12 +622,12 @@ async def get_last_client_ip_by_device( async def get_user_ip_and_agents( self, user: UserID, since_ts: int = 0 - ) -> List[Dict[str, Union[str, int]]]: + ) -> List[UserIpAndAgentsEntry]: """ Fetch IP/User Agent connection since a given timestamp. """ user_id = user.to_string() - results = {} + results: Dict[Tuple[str, str], Tuple[str, int]] = {} for key in self._batch_row_update: ( @@ -579,7 +640,7 @@ async def get_user_ip_and_agents( if last_seen >= since_ts: results[(access_token, ip)] = (user_agent, last_seen) - def get_recent(txn): + def get_recent(txn: LoggingTransaction) -> List[Tuple[str, str, str, int]]: txn.execute( """ SELECT access_token, ip, user_agent, last_seen FROM user_ips @@ -589,7 +650,7 @@ def get_recent(txn): """, (since_ts, user_id), ) - return txn.fetchall() + return cast(List[Tuple[str, str, str, int]], txn.fetchall()) rows = await self.db_pool.runInteraction( desc="get_user_ip_and_agents", func=get_recent From b452f8b9709879409ae7ee2547c7d8afdf3ebcf9 Mon Sep 17 00:00:00 2001 From: Sean Quah Date: Mon, 4 Oct 2021 14:40:42 +0100 Subject: [PATCH 4/6] Use classy TypedDicts and give them better names --- synapse/storage/databases/main/client_ips.py | 49 ++++++++++---------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/synapse/storage/databases/main/client_ips.py b/synapse/storage/databases/main/client_ips.py index 883ea2c07368..b81d9218ce18 100644 --- a/synapse/storage/databases/main/client_ips.py +++ b/synapse/storage/databases/main/client_ips.py @@ -41,28 +41,27 @@ LAST_SEEN_GRANULARITY = 120 * 1000 -LastClientIpByDeviceEntry = TypedDict( - "LastClientIpByDeviceEntry", - { - # These types must match the columns in the `devices` table - "user_id": str, - "ip": Optional[str], - "user_agent": Optional[str], - "device_id": str, - "last_seen": Optional[int], - }, -) +class DeviceLastConnectionInfo(TypedDict): + """Metadata for the last connection seen for a user and device combination""" -UserIpAndAgentsEntry = TypedDict( - "UserIpAndAgentsEntry", - { - # These types must match the columns in the `user_ips` table - "access_token": str, - "ip": str, - "user_agent": str, - "last_seen": int, - }, -) + # These types must match the columns in the `devices` table + user_id: str + device_id: str + + ip: Optional[str] + user_agent: Optional[str] + last_seen: Optional[int] + + +class LastConnectionInfo(TypedDict): + """Metadata for the last connection seen for an access token and IP combination""" + + # These types must match the columns in the `user_ips` table + access_token: str + ip: str + + user_agent: str + last_seen: int class ClientIpBackgroundUpdateStore(SQLBaseStore): @@ -450,7 +449,7 @@ def _prune_old_user_ips_txn(txn: LoggingTransaction) -> None: async def get_last_client_ip_by_device( self, user_id: str, device_id: Optional[str] - ) -> Dict[Tuple[str, str], LastClientIpByDeviceEntry]: + ) -> Dict[Tuple[str, str], DeviceLastConnectionInfo]: """For each device_id listed, give the user_ip it was last seen on. The result might be slightly out of date as client IPs are inserted in batches. @@ -469,7 +468,7 @@ async def get_last_client_ip_by_device( keyvalues["device_id"] = device_id res = cast( - List[LastClientIpByDeviceEntry], + List[DeviceLastConnectionInfo], await self.db_pool.simple_select_list( table="devices", keyvalues=keyvalues, @@ -586,7 +585,7 @@ def _update_client_ips_batch_txn( async def get_last_client_ip_by_device( self, user_id: str, device_id: Optional[str] - ) -> Dict[Tuple[str, str], LastClientIpByDeviceEntry]: + ) -> Dict[Tuple[str, str], DeviceLastConnectionInfo]: """For each device_id listed, give the user_ip it was last seen on Args: @@ -622,7 +621,7 @@ async def get_last_client_ip_by_device( async def get_user_ip_and_agents( self, user: UserID, since_ts: int = 0 - ) -> List[UserIpAndAgentsEntry]: + ) -> List[LastConnectionInfo]: """ Fetch IP/User Agent connection since a given timestamp. """ From 519bb6b6c2b40a0a89edb847f9b764bd95350955 Mon Sep 17 00:00:00 2001 From: Sean Quah Date: Tue, 12 Oct 2021 13:03:32 +0100 Subject: [PATCH 5/6] Clean up diff for tests/storage/test_client_ips.py --- tests/storage/test_client_ips.py | 60 ++++++++++++++++---------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/tests/storage/test_client_ips.py b/tests/storage/test_client_ips.py index d86ad12e651c..0e4013ebeaa7 100644 --- a/tests/storage/test_client_ips.py +++ b/tests/storage/test_client_ips.py @@ -147,17 +147,24 @@ def test_insert_new_client_ip_none_device_id(self): ) @parameterized.expand([(False,), (True,)]) - def test_get_user_ip_and_agents(self, after_persisting: bool): - """Test `get_user_ip_and_agents` for persisted and unpersisted data""" + def test_get_last_client_ip_by_device(self, after_persisting: bool): + """Test `get_last_client_ip_by_device` for persisted and unpersisted data""" self.reactor.advance(12345678) user_id = "@user:id" - user = UserID.from_string(user_id) + device_id = "MY_DEVICE" # Insert a user IP + self.get_success( + self.store.store_device( + user_id, + device_id, + "display name", + ) + ) self.get_success( self.store.insert_client_ip( - user_id, "access_token", "ip", "user_agent", "MY_DEVICE" + user_id, "access_token", "ip", "user_agent", device_id ) ) @@ -165,37 +172,35 @@ def test_get_user_ip_and_agents(self, after_persisting: bool): # Trigger the storage loop self.reactor.advance(10) + result = self.get_success( + self.store.get_last_client_ip_by_device(user_id, device_id) + ) + self.assertEqual( - self.get_success(self.store.get_user_ip_and_agents(user)), - [ - { - "access_token": "access_token", + result, + { + (user_id, device_id): { + "user_id": user_id, + "device_id": device_id, "ip": "ip", "user_agent": "user_agent", "last_seen": 12345678000, }, - ], + }, ) @parameterized.expand([(False,), (True,)]) - def test_get_last_client_ip_by_device(self, after_persisting: bool): - """Test `get_last_client_ip_by_device` for persisted and unpersisted data""" + def test_get_user_ip_and_agents(self, after_persisting: bool): + """Test `get_user_ip_and_agents` for persisted and unpersisted data""" self.reactor.advance(12345678) user_id = "@user:id" - device_id = "MY_DEVICE" + user = UserID.from_string(user_id) # Insert a user IP - self.get_success( - self.store.store_device( - user_id, - device_id, - "display name", - ) - ) self.get_success( self.store.insert_client_ip( - user_id, "access_token", "ip", "user_agent", device_id + user_id, "access_token", "ip", "user_agent", "MY_DEVICE" ) ) @@ -203,21 +208,16 @@ def test_get_last_client_ip_by_device(self, after_persisting: bool): # Trigger the storage loop self.reactor.advance(10) - result = self.get_success( - self.store.get_last_client_ip_by_device(user_id, device_id) - ) - self.assertEqual( - result, - { - (user_id, device_id): { - "user_id": user_id, - "device_id": device_id, + self.get_success(self.store.get_user_ip_and_agents(user)), + [ + { + "access_token": "access_token", "ip": "ip", "user_agent": "user_agent", "last_seen": 12345678000, }, - }, + ], ) @override_config({"limit_usage_by_mau": False, "max_mau_value": 50}) From 0acd03f6368888d8f3f40c45e44b95ff352f8c03 Mon Sep 17 00:00:00 2001 From: Sean Quah Date: Tue, 12 Oct 2021 13:05:04 +0100 Subject: [PATCH 6/6] Clean up diff --- changelog.d/10968.bugfix | 1 - 1 file changed, 1 deletion(-) delete mode 100644 changelog.d/10968.bugfix diff --git a/changelog.d/10968.bugfix b/changelog.d/10968.bugfix deleted file mode 100644 index 76624ed73c36..000000000000 --- a/changelog.d/10968.bugfix +++ /dev/null @@ -1 +0,0 @@ -Fix `/admin/whois/{user_id}` endpoint, which was broken in v1.44.0rc1.