Address comments

setup.py

@@ -44,11 +44,13 @@ setup(
 
         # twisted warns about about the absence of this
         "service_identity>=1.0.0",
+
+        "phonenumbers",
+        "pyopenssl",
+
         "pynacl",
         "pyyaml",
         "netaddr",
-        "phonenumbers",
-        "pyopenssl",
     ],
     # make sure we package the sql files
     include_package_data=True,

sydent/db/invite_tokens.py

@@ -33,14 +33,15 @@ class JoinTokenStore(object):
         :type sender: str
         :param token: The token itself.
         :type token: str
-        :param originServer: The server this invite originated from (if coming from replication).
-        :type originServer: str
+        :param originServer: The server this invite originated from (if
+            coming from replication).
+        :type originServer: str, None
         :param originId: The id of the token in the DB of originServer. Used
         for determining if we've already received a token or not.
-        :type originId: int
-        :param commit: Whether DB changes should be committed by this function (or an external one).
+        :type originId: int, None
+        :param commit: Whether DB changes should be committed by this
+            function (or an external one).
         :type commit: bool
-        :return:
         """
         cur = self.sydent.db.cursor()
 
@@ -58,7 +59,8 @@ class JoinTokenStore(object):
         :type medium: str
         :param address: The address of the 3PID.
         :type address: str
-        :return a list of invite tokens.
+        :returns a list of invite tokens, or an empty list if no tokens found.
+        :rtype: list[Dict[str, str]]
         """
         cur = self.sydent.db.cursor()
 
@@ -83,14 +85,18 @@ class JoinTokenStore(object):
 
         return ret
 
-    def getTokensAfterId(self, afterId, limit):
+    def getInviteTokensAfterId(self, afterId, limit):
         """Retrieves max `limit` invite tokens after a given DB id.
         
-        :param afterId: A database id to act as an offset. Tokens after this id are returned.
+        :param afterId: A database id to act as an offset. Tokens after this
+            id are returned.
         :type afterId: int
         :param limit: Max amount of database rows to return.
-        :type limit: int
-        :returns a tuple consisting of a list of invite tokens and the maximum DB id that was extracted.
+        :type limit: int, None
+        :returns a tuple consisting of a dict of invite tokens (with key
+            being the token's DB id) and the maximum DB id that was extracted.
+            Otherwise returns ({}, None) if no tokens are found.
+        :rtype: Tuple[Dict[int, Dict], int|None]
         """
         cur = self.sydent.db.cursor()
         res = cur.execute(
@@ -103,7 +109,7 @@ class JoinTokenStore(object):
         # Dict of "id": {content}
         invite_tokens = {}
 
-        maxId = 0
+        maxId = None
 
         for row in rows:
             maxId, medium, address, room_id, sender, token = row
@@ -125,14 +131,16 @@ class JoinTokenStore(object):
         :param server: The name of the origin server.
         :type server: str
         :returns a database id marking the last known invite token received
-        from the given server.
+            from the given server. Returns 0 if no tokens have been received from
+            this server.
+        :rtype: int
         """
         cur = self.sydent.db.cursor()
         res = cur.execute("select max(origin_id), count(origin_id) from invite_tokens"
                           " where origin_server = ?", (server,))
         row = res.fetchone()
 
-        if row[1] == 0:
+        if not row or row[1] == 0:
             return 0
 
         return row[0]
@@ -145,7 +153,6 @@ class JoinTokenStore(object):
         :type medium: str
         :param address: The address of the token.
         :type address: str
-        :return:
         """
         cur = self.sydent.db.cursor()
 
@@ -162,14 +169,15 @@ class JoinTokenStore(object):
         :type publicKey: str
         :param persistenceTs: 
         :type persistenceTs: int
-        :param originServer: the server this key was received from (if retrieved through replication).
+        :param originServer: the server this key was received from (if
+            retrieved through replication).
         :type originServer: str
         :param originId: The id of the key in the DB of originServer. Used
+            for determining if we've already received a key or not.
         :type originId: int
-        for determining if we've already received a key or not.
-        :param commit: Whether DB changes should be committed by this function (or an external one).
+        :param commit: Whether DB changes should be committed by this
+            function (or an external one).
         :type commit: bool
-        :return:
         """
         if not persistenceTs:
             persistenceTs = int(time.time())
@@ -188,7 +196,9 @@ class JoinTokenStore(object):
         
         :param publicKey: An ephemeral public key.
         :type publicKey: str
-        :returns true or false depending on whether validation was successful.
+        :returns true or false depending on whether validation was
+            successful.
+        :rtype: bool
         """
         cur = self.sydent.db.cursor()
         cur.execute(
@@ -203,12 +213,15 @@ class JoinTokenStore(object):
     def getEphemeralPublicKeysAfterId(self, afterId, limit):
         """Retrieves max `limit` ephemeral public keys after a given DB id.
         
-        :param afterId: A database id to act as an offset. Keys after this id are returned.
+        :param afterId: A database id to act as an offset. Keys after this id
+            are returned.
         :type afterId: int
         :param limit: Max amount of database rows to return.
         :type limit: int
-        :returns a tuple consisting of a list of ephemeral public keys and
-        the maximum table id that was extracted.
+        :returns a tuple consisting of a list of ephemeral public keys (with
+            key being the token's DB id) and the maximum table id that was
+            extracted. Otherwise returns ({}, None) if no keys are found.
+        :rtype: Tuple[Dict[int, Dict], int|None]
         """
         cur = self.sydent.db.cursor()
         res = cur.execute(
@@ -219,19 +232,19 @@ class JoinTokenStore(object):
         rows = res.fetchall()
 
         # Dict of "id": {content}
-        epheremal_keys = {}
+        ephemeral_keys = {}
 
-        maxId = 0
+        maxId = None
 
         for row in rows:
             maxId, public_key, verify_count, persistence_ts = row
-            epheremal_keys[maxId] = {
+            ephemeral_keys[maxId] = {
                 "public_key": public_key,
                 "verify_count": verify_count,
                 "persistence_ts": persistence_ts,
             }
 
-        return (epheremal_keys, maxId)
+        return (ephemeral_keys, maxId)
 
     def getLastEphemeralPublicKeyIdFromServer(self, server):
         """Returns the last known ephemeral public key that was received from
@@ -239,14 +252,16 @@ class JoinTokenStore(object):
 
         :param server: The name of the origin server.
         :type server: str
-        :returns the last known public key id received from the given server.
+        :returns the last known DB id received from the given server, or 0 if
+            none have been received.
+        :rtype: int
         """
         cur = self.sydent.db.cursor()
         res = cur.execute("select max(origin_id),count(origin_id) from ephemeral_public_keys"
                           " where origin_server = ?", (server,))
         row = res.fetchone()
 
-        if row[1] == 0:
+        if not row or row[1] == 0:
             return 0
 
         return row[0]
@@ -256,7 +271,9 @@ class JoinTokenStore(object):
         
         :param token: The invite token.
         :type token: str
-        :returns the sender of a given invite token or None if there isn't one.
+        :returns the sender of a given invite token or None if there isn't
+            one.
+        :rtype: str, None
         """
         cur = self.sydent.db.cursor()
         res = cur.execute(

sydent/db/invite_tokens.sql

@@ -21,10 +21,10 @@ CREATE TABLE IF NOT EXISTS invite_tokens (
     room_id varchar(256) not null,
     sender varchar(256) not null,
     token varchar(256) not null,
-    origin_id INTEGER, -- original id in homeserver's DB that this was replicated from (if applicable)
-    origin_server TEXT, -- homeserver this was replicated from (if applicable)
-    received_ts BIGINT, -- When the invite was received by us from the homeserver
-    sent_ts BIGINT -- When the token was sent by us to the user
+    received_ts bigint, -- When the invite was received by us from the homeserver
+    sent_ts bigint, -- When the token was sent by us to the user
+    origin_id integer, -- original id in homeserver's DB that this was replicated from (if applicable)
+    origin_server text -- homeserver this was replicated from (if applicable)
 );
 CREATE INDEX IF NOT EXISTS invite_token_medium_address on invite_tokens(medium, address);
 CREATE INDEX IF NOT EXISTS invite_token_token on invite_tokens(token);

sydent/db/peers.py

@@ -23,7 +23,9 @@ class PeerStore:
 
     def getPeerByName(self, name):
         cur = self.sydent.db.cursor()
-        res = cur.execute("select p.name, p.port, p.lastSentAssocsId, p.lastSentInviteTokensId, p.lastSentEphemeralKeysId, p.shadow, pk.alg, pk.key from peers p, peer_pubkeys pk "
+        res = cur.execute("select p.name, p.port, "
+                          "p.lastSentAssocsId, p.lastSentInviteTokensId, p.lastSentEphemeralKeysId, "
+                          "p.shadow, pk.alg, pk.key from peers p, peer_pubkeys pk "
                           "where p.name = ? and pk.peername = p.name and p.active = 1", (name,))
 
         serverName = None
@@ -57,7 +59,9 @@ class PeerStore:
 
     def getAllPeers(self):
         cur = self.sydent.db.cursor()
-        res = cur.execute("select p.name, p.port, p.lastSentAssocsId, p.lastSentInviteTokensId, p.lastSentEphemeralKeysId, p.shadow, pk.alg, pk.key from peers p, peer_pubkeys pk "
+        res = cur.execute("select p.name, p.port, "
+                          "p.lastSentAssocsId, p.lastSentInviteTokensId, p.lastSentEphemeralKeysId, "
+                          "p.shadow, pk.alg, pk.key from peers p, peer_pubkeys pk "
                           "where pk.peername = p.name and p.active = 1")
 
         peers = []
@@ -104,25 +108,28 @@ class PeerStore:
     def setLastSentIdAndPokeSucceeded(self, peerName, ids, lastPokeSucceeded):
         """Set last successful replication of data to this peer.
 
+        If an id for a replicated database table is None, the last sent value
+        will not be updated.
+
         :param peerName: The name of the peer.
         :type peerName: str
         :param ids: A Dictionary of ids that represent the last database
-        :type ids: dict
-        table ids that were replicated to this peer.
+            table ids that were replicated to this peer.
+        :type ids: Dict[str, int]
         :param lastPokeSucceeded: The time of when the last successful
-        replication succeeded (even if no actual replication of data was necessary).
+            replication succeeded (even if no actual replication of data was
+            necessary).
         :type lastPokeSucceeded: int
-        :return:
         """
 
         cur = self.sydent.db.cursor()
-        if "sg_assocs" in ids and ids["sg_assocs"] != 0:
+        if "sg_assocs" in ids and ids["sg_assocs"]:
             cur.execute("update peers set lastSentAssocsId = ?, lastPokeSucceededAt = ? "
                         "where name = ?", (ids["sg_assocs"], lastPokeSucceeded, peerName))
-        if "invite_tokens" in ids and ids["invite_tokens"] != 0:
+        if "invite_tokens" in ids and ids["invite_tokens"]:
             cur.execute("update peers set lastSentInviteTokensId = ?, lastPokeSucceededAt = ? "
                         "where name = ?", (ids["invite_tokens"], lastPokeSucceeded, peerName))
-        if "ephemeral_public_keys" in ids and ids["ephemeral_public_keys"] != 0:
+        if "ephemeral_public_keys" in ids and ids["ephemeral_public_keys"]:
             cur.execute("update peers set lastSentEphemeralKeysId = ?, lastPokeSucceededAt = ? "
                         "where name = ?", (ids["ephemeral_public_keys"], lastPokeSucceeded, peerName))
         self.sydent.db.commit()

sydent/db/threepid_associations.py

@@ -178,9 +178,9 @@ class GlobalAssociationStore:
 
     def addAssociation(self, assoc, rawSgAssoc, originServer, originId, commit=True):
         """
-        :param assoc: (sydent.threepid.GlobalThreepidAssociation) The association to add as a high level object
-        :param sgAssoc The original raw bytes of the signed association
-        :return:
+        :param assoc: (sydent.threepid.GlobalThreepidAssociation) The
+            association to add as a high level object.
+        :param sgAssoc: The original raw bytes of the signed association.
         """
         cur = self.sydent.db.cursor()
         res = cur.execute("insert or ignore into global_threepid_associations "

sydent/hs_federation/verifier.py

@@ -88,11 +88,11 @@ class Verifier(object):
         to do perspectives checks.
 
         :param acceptable_server_names: If provided and not None,
-        only signatures from servers in this list will be accepted.
-        :type acceptable_server_names: list of strings
+            only signatures from servers in this list will be accepted.
+        :type acceptable_server_names: list[str]
 
         :return a tuple of the server name and key name that was
-        successfully verified. If the json cannot be verified,
+            successfully verified. If the json cannot be verified,
         raises SignatureVerifyException.
         """
         if 'signatures' not in signed_json:

sydent/http/httpclient.py

@@ -130,9 +130,10 @@ def matrix_federation_endpoint(reactor, destination, ssl_context_factory=None,
     :param reactor: Twisted reactor.
     :param destination: The name of the server to connect to.
     :type destination: bytes
-    :param ssl_context_factory: Factory which generates SSL contexts to use for TLS.
+    :param ssl_context_factory: Factory which generates SSL contexts to use
+        for TLS.
     :type ssl_context_factory: twisted.internet.ssl.ContextFactory
-    :param timeout (int): connection timeout in seconds
+    :param timeout: connection timeout in seconds
     :type timeout: int
     """
 

sydent/replication/peer.py

@@ -38,14 +38,6 @@ class Peer(object):
         self.pubkeys = pubkeys
         self.shadow = False
 
-    def pushUpdates(self, sgAssocs):
-        """
-        :param sgAssocs: Sequence of (originId, (sgAssoc, shadowSgAssoc)) tuples where originId
-            is the id on the creating server and sgAssoc is the json object of the signed association
-        :return:
-        """
-        pass
-
 
 class LocalPeer(Peer):
     """
@@ -133,9 +125,11 @@ class RemotePeer(Peer):
     def pushUpdates(self, data):
         """Push updates to a remote peer.
 
-        :param data: A dictionary of possible `sg_assocs`, `invite_tokens` and `ephemeral_public_keys` keys.
-        :type data: dict
+        :param data: A dictionary of possible `sg_assocs`, `invite_tokens`
+            and `ephemeral_public_keys` keys.
+        :type data: Dict
         :returns a deferred.
+        :rtype: Deferred
         """
 
         # sgAssocs is comprised of tuples (sgAssoc, shadowSgAssoc)

sydent/replication/pusher.py

@@ -47,12 +47,15 @@ class Pusher:
         """Return max `limit` associations from the database after a given
         DB table id.
 
-        :param afterId: A database id to act as an offset. Rows after this id are returned.
+        :param afterId: A database id to act as an offset. Rows after this id
+            are returned.
         :type afterId: int
         :param limit: Max amount of database rows to return.
         :type limit: int
-        :returns a tuple with the first item being a list of associations, and the
-        second being the maximum table id of the returned associations.
+        :returns a tuple with the first item being a list of associations,
+            and the second being the maximum table id of the returned
+            associations.
+        :rtype: Tuple[list[Tuple[Dict, Dict]], int|None]
         """
         assocs = {}
 
@@ -84,37 +87,37 @@ class Pusher:
         """Return max `limit` invite tokens from the database after a given
         DB table id.
 
-        :param afterId: A database id to act as an offset. Rows after this id are returned.
+        :param afterId: A database id to act as an offset. Rows after this id
+            are returned.
         :type afterId: int
         :param limit: Max amount of database rows to return.
         :type limit: int
         :returns a tuple with the first item being a list of tokens, and the
-        second being the maximum table id of the returned tokens.
+            second being the maximum table id of the returned tokens.
+        :rtype: Tuple[Dict[int, Dict], int|None]
         """
-        join_token_store = JoinTokenStore(self.sydent)
-        (invite_tokens, maxId) = join_token_store.getTokensAfterId(afterId, limit)
-
         # TODO: Do something for shadow servers?
 
-        return (invite_tokens, maxId)
+        join_token_store = JoinTokenStore(self.sydent)
+        return join_token_store.getInviteTokensAfterId(afterId, limit)
 
     def getEphemeralPublicKeysAfterId(self, afterId, limit):
         """Return max `limit` ephemeral keys from the database after a given
         table id.
 
-        :param afterId: A database id to act as an offset. Rows after this id are returned.
+        :param afterId: A database id to act as an offset. Rows after this id
+            are returned.
         :type afterId: int
         :param limit: Max amount of database rows to return.
         :type limit: int
         :returns a tuple with the first item being a list of keys, and the
-        second being the maximum table id of the returned keys.
+            second being the maximum table id of the returned keys.
+        :rtype: Tuple[Dict[int, Dict], int|None]
         """
-        join_token_store = JoinTokenStore(self.sydent)
-        (ephemeral_public_keys, maxId) = join_token_store.getEphemeralPublicKeysAfterId(afterId, limit)
-
         # TODO: Do something for shadow servers?
 
-        return (ephemeral_public_keys, maxId)
+        join_token_store = JoinTokenStore(self.sydent)
+        return join_token_store.getEphemeralPublicKeysAfterId(afterId, limit)
 
     def doLocalPush(self):
         """

sydent/threepid/__init__.py

@@ -21,12 +21,20 @@ def threePidAssocFromDict(d):
 class ThreepidAssociation:
     def __init__(self, medium, address, mxid, ts, not_before, not_after):
         """
-        :param medium: The medium of the 3pid (eg. email)
-        :param address: The identifier (eg. email address)
-        :param mxid: The matrix ID the 3pid is associated with
-        :param ts: The creation timestamp of this association, ms
-        :param not_before: The timestamp, in ms, at which this association becomes valid
-        :param not_after: The timestamp, in ms, at which this association ceases to be valid
+        :param medium: The medium of the 3pid (eg. email).
+        :type medium: str
+        :param address: The identifier (eg. email address).
+        :type address: str
+        :param mxid: The matrix ID the 3pid is associated with.
+        :type mxid: str
+        :param ts: The creation timestamp of this association, ms.
+        :type ts: int
+        :param not_before: The timestamp, in ms, at which this association
+            becomes valid.
+        :type not_before: int
+        :param not_after: The timestamp, in ms, at which this association
+            ceases to be valid.
+        :type not_after: int
         """
         self.medium = medium
         self.address = address